diff --git a/LICENSE b/LICENSE new file mode 100644 index 0000000..aa5473f --- /dev/null +++ b/LICENSE @@ -0,0 +1,21 @@ +MIT License + +Copyright (c) 2026 Ankur Mhatre + +Permission is hereby granted, free of charge, to any person obtaining a copy +of this software and associated documentation files (the "Software"), to deal +in the Software without restriction, including without limitation the rights +to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +copies of the Software, and to permit persons to whom the Software is +furnished to do so, subject to the following conditions: + +The above copyright notice and this permission notice shall be included in all +copies or substantial portions of the Software. + +THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE +SOFTWARE. diff --git a/README.md b/README.md index 04cf481..e9fecf4 100644 --- a/README.md +++ b/README.md @@ -1,131 +1,39 @@ # spring-security-demo -Companion repo for [Spring Security Context Propagation: The Complete -Guide](https://ankurm.com/spring-security-context-propagation-complete-guide/) on ankurm.com. -Eight small, dependency-light programs and one JUnit test suite that answer one question each: -does a Spring Security `SecurityContext` survive a specific thread, scheduler, or request -hand-off? Every scenario sets an `Authentication` somewhere and checks whether the other side -of the hand-off can see it -- against real executors, a real Reactor pipeline, a real -`TaskScheduler`, and real servlet filter classes (via Spring Test's mock request/response, no -running server needed). +Companion code for the Spring Security series on [ankurm.com](https://ankurm.com). Each +directory is a self-contained Maven project for one article, with its own `pom.xml`, its own +numbered documentation chapters, and its own captured output under `docs/output/` — regenerated +by that module's `scripts/run-all.sh`, never typed by hand. -Every example the post shows -- `@Async`, `ExecutorService`, `CompletableFuture`, virtual -threads, `StructuredTaskScope`, `ReactiveSecurityContextHolder`/WebFlux, -`DelegatingSecurityContextTaskScheduler` and scheduled tasks, and the -`SecurityContextHolderFilter`/`SecurityContextPersistenceFilter` servlet distinction -- has a -runnable demo here, plus edge cases the post doesn't have room for. See the [edge-case -index](docs/08-testing-contract.md#edge-case-index) for the full list with links. +| Module | Article | What it demonstrates | +|---|---|---| +| [`context-propagation/`](context-propagation/README.md) | [Spring Security Context Propagation: The Complete Guide](https://ankurm.com/spring-security-context-propagation-complete-guide/) | Whether a `SecurityContext` survives `@Async`, executors, virtual threads, `StructuredTaskScope`, Reactor, schedulers and the servlet filter chain | +| [`method-security/`](method-security/README.md) | [Method Security in Spring Security 7: `@PreAuthorize`, `@PostAuthorize` and the Proxy Traps](https://ankurm.com/spring-security-7-method-security-proxy-traps/) | What the method-security annotations do, the full SpEL surface, and the cases where the check silently does not run | -## Verified versions +The two are related more closely than they look. Method security reads the `Authentication` +from `SecurityContextHolder` on the calling thread; the context-propagation module is about +getting it there. An `@Async` method carrying `@PreAuthorize` fails with +`AuthenticationCredentialsNotFoundException` for reasons that belong to the first module, not +the second. -| Component | Version | -|---|---| -| JDK | 25 (Temurin 25.0.4.1+1), LTS, GA 2025-09-16 | -| Spring Boot (reference target) | 4.1.1 | -| Spring Framework | 7.0.9 | -| Spring Security | 7.1.1 | -| Spring Security Test | 7.1.1 | -| `io.micrometer:context-propagation` | 1.2.1 (as managed by Boot 4.1.1's BOM) | -| Reactor Core / Reactor Test | 3.8.7 (as managed by Boot 4.1.1's `reactor-bom` 2025.0.7) | -| `jakarta.servlet-api` | 6.1.0 | -| JUnit Jupiter | 6.0.3 | -| AssertJ | 3.27.7 | +## Common ground -`StructuredTaskScope` is a **preview API** on JDK 25 (JEP 505, fifth preview) and remains -preview through JDK 26 (JEP 525, sixth preview) -- every build/run/test command below needs -`--enable-preview`. +Both modules target the same verified stack: **JDK 25** (Temurin 25.0.4.1+1), +**Spring Framework 7.0.9**, **Spring Security 7.1.1** — the versions Spring Boot **4.1.1** +manages. Versions were taken from `maven-metadata.xml` on Maven Central rather than from +release announcements. -## Quickstart +`context-propagation` additionally needs `--enable-preview`, because `StructuredTaskScope` is +still a preview API on JDK 25. `method-security` does not. + +## Running either module ```bash -mvn dependency:build-classpath -Dmdep.outputFile=cp.txt -javac --release 25 --enable-preview -cp "$(cat cp.txt)" -d target/classes $(find src/main -name '*.java') -java --enable-preview -cp "target/classes:$(cat cp.txt)" com.ankurm.vt.Demo1PlainThreadLocal +cd method-security # or context-propagation +./scripts/run-all.sh # every demo plus the test suite, regenerating docs/output/ +mvn test # just the assertions ``` -Or just run everything -- all seven demos plus the test suite -- and regenerate the captured -output: `scripts/run-all.sh`. To run only the JUnit contract tests: `mvn test` (the -`--enable-preview` flag is already wired into `pom.xml`'s surefire `argLine`, no extra flags -needed). +## License -## What each demo shows - -| Demo | Question | Chapter | -|---|---|---| -| `Demo1PlainThreadLocal` | Does `InheritableThreadLocal` behave differently for a pooled platform thread vs. a fresh virtual thread? (No Spring.) | [docs/01](docs/01-inheritable-threadlocal.md) | -| `Demo2AsyncVirtualThreads` | Does the Boot-4.1-style virtual-thread `@Async` executor propagate `SecurityContext`, and what four fixes change? | [docs/02](docs/02-async-virtual-threads.md) | -| `Demo3StructuredConcurrency` | Does a `StructuredTaskScope.fork()` subtask see the parent's `SecurityContext`? | [docs/03](docs/03-structured-concurrency.md) | -| `Demo4ExecutorWrapping` | Do `DelegatingSecurityContextExecutorService`/`Executor`/`AsyncTaskExecutor` propagate context on a classic *pooled platform-thread* executor, and does a reused worker leak between tasks the way `InheritableThreadLocal` did in Demo 1? | [docs/04](docs/04-executor-wrapping.md) | -| `Demo5ReactiveContext` | Does `ReactiveSecurityContextHolder` survive a scheduler hop that kills plain `ThreadLocal`-based `SecurityContextHolder`? | [docs/05](docs/05-reactive-context.md) | -| `Demo6ScheduledSystemIdentity` | What does `DelegatingSecurityContextTaskScheduler` actually capture, and when -- and how does the post's `createSystemContext()` pattern fix the fact that there's no real caller to propagate from? | [docs/06](docs/06-scheduled-tasks.md) | -| `Demo7ServletFilterPersistence` | Does `SecurityContextHolderFilter` really never save, while `SecurityContextPersistenceFilter` does -- proven against real filter instances and a real `HttpSession`? | [docs/07](docs/07-servlet-filter-persistence.md) | -| `SecurityContextPropagationContractTest` (JUnit, `src/test`) | Same ten claims above, pinned as assertions instead of printed lines; includes a `TestSecurityContextHolder`-based test reproducing the post's own "Testing Security Context Propagation" section | [docs/08](docs/08-testing-contract.md) | - -## Endpoints / entry points - -There's no web server in this repo (see the top of this file), so "entry points" means: every -class above has a runnable `main()`, and the whole suite runs end to end via -`scripts/run-all.sh`. `Demo5ReactiveContext` reproduces the post's `/profile` -(`ReactiveSecurityContextHolder`) behavior as a plain `Mono` chain rather than a bound HTTP -route, and `Demo7ServletFilterPersistence` reproduces the filter chain's request-scoped -behavior against `MockHttpServletRequest`/`MockHttpServletResponse` rather than a bound -servlet container -- both keep the "no web server, no HTTP" property the original three demos -established, so the whole repo still runs in well under a second with zero open ports. - -## Captured output - -Every number and log line in the blog post traces back to one of these, produced by -`scripts/run-all.sh`, not retyped: - -- [docs/output/demo1.txt](docs/output/demo1.txt) -- [docs/output/demo2.txt](docs/output/demo2.txt) -- [docs/output/demo3.txt](docs/output/demo3.txt) -- [docs/output/demo4.txt](docs/output/demo4.txt) -- [docs/output/demo5.txt](docs/output/demo5.txt) -- [docs/output/demo6.txt](docs/output/demo6.txt) -- [docs/output/demo7.txt](docs/output/demo7.txt) -- [docs/output/tests.txt](docs/output/tests.txt) -- `mvn test` surefire summary for the ten - contract tests - -## Doc chapters - -Numbered, cross-linked, each with prev/next navigation at the top: - -1. [InheritableThreadLocal across thread models](docs/01-inheritable-threadlocal.md) -2. [@Async, DelegatingSecurityContextExecutor, and virtual threads](docs/02-async-virtual-threads.md) -3. [StructuredTaskScope and SecurityContext](docs/03-structured-concurrency.md) -4. [Executor, ExecutorService, and AsyncTaskExecutor wrapping](docs/04-executor-wrapping.md) -5. [ReactiveSecurityContextHolder and Reactor Context](docs/05-reactive-context.md) -6. [DelegatingSecurityContextTaskScheduler and the synthetic system identity](docs/06-scheduled-tasks.md) -7. [SecurityContextHolderFilter vs. SecurityContextPersistenceFilter](docs/07-servlet-filter-persistence.md) -8. [Testing contract + edge-case index](docs/08-testing-contract.md) - -## Edge cases - -Thirteen reproducible edge cases were found building this repository -- pooled-worker leaks -the `Delegating*` classes don't have, the common `ForkJoinPool` trap, why -`MODE_INHERITABLETHREADLOCAL` is a JVM-wide instrument, why there's no -`DelegatingSecurityContextStructuredTaskScope` and never will be, a real `NullPointerException` -hit writing the reactive test, `ReactiveSecurityContextHolder` completing empty rather than -erroring, per-call (not per-construction) context capture in -`DelegatingSecurityContextTaskScheduler`, why a synthetic `SYSTEM` principal isn't -"anonymous", and the precise load-vs-save split between the two servlet filters. Full list, -each with the chapter that reproduces it: [docs/08 § Edge-case -index](docs/08-testing-contract.md#edge-case-index). - -## The one-line summary of all eight chapters - -`SecurityContextHolder` is a `ThreadLocal`. Nothing about virtual threads, structured -concurrency, reactive streams, schedulers, or servlet filters changes that fact -- what changes -between them is *how* (or whether) anything carries that `ThreadLocal`'s value across the -boundary each one introduces. Virtual threads are never pooled, so -`MODE_INHERITABLETHREADLOCAL`'s old danger (stale context on a reused pool worker) doesn't -apply to them, but it's still a JVM-wide setting. The `Delegating*` wrapper family solves the -same pooled-worker problem by a completely different mechanism -- explicit push/pop per task, -never thread inheritance -- which is why it has worked, unchanged, since long before virtual -threads existed. Reactive code doesn't have a `ThreadLocal`-compatible thread to begin with, so -`ReactiveSecurityContextHolder` uses Reactor's own `Context` instead. Scheduled tasks have no -caller at all, so the fix isn't propagation, it's minting an identity. And the servlet filter -that used to auto-save the context for you was replaced by one that only loads -- a change -worth knowing about before it's the reason a custom filter's write silently doesn't survive to -the next request. +MIT — see [LICENSE](LICENSE). diff --git a/context-propagation/README.md b/context-propagation/README.md new file mode 100644 index 0000000..b5a7ddf --- /dev/null +++ b/context-propagation/README.md @@ -0,0 +1,141 @@ +# context-propagation + +Companion module for [Spring Security Context Propagation: The Complete +Guide](https://ankurm.com/spring-security-context-propagation-complete-guide/) on ankurm.com. + +Part of [spring-security-demo](../README.md); the sibling module is +[method-security](../method-security/README.md), which covers what `@PreAuthorize` does with +the `Authentication` once it has reached the right thread -- and the cases where it silently +does nothing. + +Eight small, dependency-light programs and one JUnit test suite that answer one question each: +does a Spring Security `SecurityContext` survive a specific thread, scheduler, or request +hand-off? Every scenario sets an `Authentication` somewhere and checks whether the other side +of the hand-off can see it -- against real executors, a real Reactor pipeline, a real +`TaskScheduler`, and real servlet filter classes (via Spring Test's mock request/response, no +running server needed). + +Every example the post shows -- `@Async`, `ExecutorService`, `CompletableFuture`, virtual +threads, `StructuredTaskScope`, `ReactiveSecurityContextHolder`/WebFlux, +`DelegatingSecurityContextTaskScheduler` and scheduled tasks, and the +`SecurityContextHolderFilter`/`SecurityContextPersistenceFilter` servlet distinction -- has a +runnable demo here, plus edge cases the post doesn't have room for. See the [edge-case +index](docs/08-testing-contract.md#edge-case-index) for the full list with links. + +## Verified versions + +| Component | Version | +|---|---| +| JDK | 25 (Temurin 25.0.4.1+1), LTS, GA 2025-09-16 | +| Spring Boot (reference target) | 4.1.1 | +| Spring Framework | 7.0.9 | +| Spring Security | 7.1.1 | +| Spring Security Test | 7.1.1 | +| `io.micrometer:context-propagation` | 1.2.1 (as managed by Boot 4.1.1's BOM) | +| Reactor Core / Reactor Test | 3.8.7 (as managed by Boot 4.1.1's `reactor-bom` 2025.0.7) | +| `jakarta.servlet-api` | 6.1.0 | +| JUnit Jupiter | 6.0.3 | +| AssertJ | 3.27.7 | + +`StructuredTaskScope` is a **preview API** on JDK 25 (JEP 505, fifth preview) and remains +preview through JDK 26 (JEP 525, sixth preview) -- every build/run/test command below needs +`--enable-preview`. + +## Quickstart + +```bash +mvn dependency:build-classpath -Dmdep.outputFile=cp.txt +javac --release 25 --enable-preview -cp "$(cat cp.txt)" -d target/classes $(find src/main -name '*.java') +java --enable-preview -cp "target/classes:$(cat cp.txt)" com.ankurm.vt.Demo1PlainThreadLocal +``` + +Or just run everything -- all seven demos plus the test suite -- and regenerate the captured +output: `scripts/run-all.sh`. To run only the JUnit contract tests: `mvn test` (the +`--enable-preview` flag is already wired into `pom.xml`'s surefire `argLine`, no extra flags +needed). + +## What each demo shows + +| Demo | Question | Chapter | +|---|---|---| +| `Demo1PlainThreadLocal` | Does `InheritableThreadLocal` behave differently for a pooled platform thread vs. a fresh virtual thread? (No Spring.) | [docs/01](docs/01-inheritable-threadlocal.md) | +| `Demo2AsyncVirtualThreads` | Does the Boot-4.1-style virtual-thread `@Async` executor propagate `SecurityContext`, and what four fixes change? | [docs/02](docs/02-async-virtual-threads.md) | +| `Demo3StructuredConcurrency` | Does a `StructuredTaskScope.fork()` subtask see the parent's `SecurityContext`? | [docs/03](docs/03-structured-concurrency.md) | +| `Demo4ExecutorWrapping` | Do `DelegatingSecurityContextExecutorService`/`Executor`/`AsyncTaskExecutor` propagate context on a classic *pooled platform-thread* executor, and does a reused worker leak between tasks the way `InheritableThreadLocal` did in Demo 1? | [docs/04](docs/04-executor-wrapping.md) | +| `Demo5ReactiveContext` | Does `ReactiveSecurityContextHolder` survive a scheduler hop that kills plain `ThreadLocal`-based `SecurityContextHolder`? | [docs/05](docs/05-reactive-context.md) | +| `Demo6ScheduledSystemIdentity` | What does `DelegatingSecurityContextTaskScheduler` actually capture, and when -- and how does the post's `createSystemContext()` pattern fix the fact that there's no real caller to propagate from? | [docs/06](docs/06-scheduled-tasks.md) | +| `Demo7ServletFilterPersistence` | Does `SecurityContextHolderFilter` really never save, while `SecurityContextPersistenceFilter` does -- proven against real filter instances and a real `HttpSession`? | [docs/07](docs/07-servlet-filter-persistence.md) | +| `SecurityContextPropagationContractTest` (JUnit, `src/test`) | Same ten claims above, pinned as assertions instead of printed lines; includes a `TestSecurityContextHolder`-based test reproducing the post's own "Testing Security Context Propagation" section | [docs/08](docs/08-testing-contract.md) | + +## Endpoints / entry points + +There's no web server in this repo (see the top of this file), so "entry points" means: every +class above has a runnable `main()`, and the whole suite runs end to end via +`scripts/run-all.sh`. `Demo5ReactiveContext` reproduces the post's `/profile` +(`ReactiveSecurityContextHolder`) behavior as a plain `Mono` chain rather than a bound HTTP +route, and `Demo7ServletFilterPersistence` reproduces the filter chain's request-scoped +behavior against `MockHttpServletRequest`/`MockHttpServletResponse` rather than a bound +servlet container -- both keep the "no web server, no HTTP" property the original three demos +established, so the whole repo still runs in well under a second with zero open ports. + +## Captured output + +Every number and log line in the blog post traces back to one of these, produced by +`scripts/run-all.sh`, not retyped: + +- [docs/output/demo1.txt](docs/output/demo1.txt) +- [docs/output/demo2.txt](docs/output/demo2.txt) +- [docs/output/demo3.txt](docs/output/demo3.txt) +- [docs/output/demo4.txt](docs/output/demo4.txt) +- [docs/output/demo5.txt](docs/output/demo5.txt) +- [docs/output/demo6.txt](docs/output/demo6.txt) +- [docs/output/demo7.txt](docs/output/demo7.txt) +- [docs/output/tests.txt](docs/output/tests.txt) -- `mvn test` surefire summary for the ten + contract tests + +## Doc chapters + +Numbered, cross-linked, each with prev/next navigation at the top: + +1. [InheritableThreadLocal across thread models](docs/01-inheritable-threadlocal.md) +2. [@Async, DelegatingSecurityContextExecutor, and virtual threads](docs/02-async-virtual-threads.md) +3. [StructuredTaskScope and SecurityContext](docs/03-structured-concurrency.md) +4. [Executor, ExecutorService, and AsyncTaskExecutor wrapping](docs/04-executor-wrapping.md) +5. [ReactiveSecurityContextHolder and Reactor Context](docs/05-reactive-context.md) +6. [DelegatingSecurityContextTaskScheduler and the synthetic system identity](docs/06-scheduled-tasks.md) +7. [SecurityContextHolderFilter vs. SecurityContextPersistenceFilter](docs/07-servlet-filter-persistence.md) +8. [Testing contract + edge-case index](docs/08-testing-contract.md) + +## Edge cases + +Thirteen reproducible edge cases were found building this repository -- pooled-worker leaks +the `Delegating*` classes don't have, the common `ForkJoinPool` trap, why +`MODE_INHERITABLETHREADLOCAL` is a JVM-wide instrument, why there's no +`DelegatingSecurityContextStructuredTaskScope` and never will be, a real `NullPointerException` +hit writing the reactive test, `ReactiveSecurityContextHolder` completing empty rather than +erroring, per-call (not per-construction) context capture in +`DelegatingSecurityContextTaskScheduler`, why a synthetic `SYSTEM` principal isn't +"anonymous", and the precise load-vs-save split between the two servlet filters. Full list, +each with the chapter that reproduces it: [docs/08 § Edge-case +index](docs/08-testing-contract.md#edge-case-index). + +## The one-line summary of all eight chapters + +`SecurityContextHolder` is a `ThreadLocal`. Nothing about virtual threads, structured +concurrency, reactive streams, schedulers, or servlet filters changes that fact -- what changes +between them is *how* (or whether) anything carries that `ThreadLocal`'s value across the +boundary each one introduces. Virtual threads are never pooled, so +`MODE_INHERITABLETHREADLOCAL`'s old danger (stale context on a reused pool worker) doesn't +apply to them, but it's still a JVM-wide setting. The `Delegating*` wrapper family solves the +same pooled-worker problem by a completely different mechanism -- explicit push/pop per task, +never thread inheritance -- which is why it has worked, unchanged, since long before virtual +threads existed. Reactive code doesn't have a `ThreadLocal`-compatible thread to begin with, so +`ReactiveSecurityContextHolder` uses Reactor's own `Context` instead. Scheduled tasks have no +caller at all, so the fix isn't propagation, it's minting an identity. And the servlet filter +that used to auto-save the context for you was replaced by one that only loads -- a change +worth knowing about before it's the reason a custom filter's write silently doesn't survive to +the next request. + +## License + +MIT -- see [LICENSE](../LICENSE). diff --git a/docs/01-inheritable-threadlocal.md b/context-propagation/docs/01-inheritable-threadlocal.md similarity index 100% rename from docs/01-inheritable-threadlocal.md rename to context-propagation/docs/01-inheritable-threadlocal.md diff --git a/docs/02-async-virtual-threads.md b/context-propagation/docs/02-async-virtual-threads.md similarity index 100% rename from docs/02-async-virtual-threads.md rename to context-propagation/docs/02-async-virtual-threads.md diff --git a/docs/03-structured-concurrency.md b/context-propagation/docs/03-structured-concurrency.md similarity index 100% rename from docs/03-structured-concurrency.md rename to context-propagation/docs/03-structured-concurrency.md diff --git a/docs/04-executor-wrapping.md b/context-propagation/docs/04-executor-wrapping.md similarity index 100% rename from docs/04-executor-wrapping.md rename to context-propagation/docs/04-executor-wrapping.md diff --git a/docs/05-reactive-context.md b/context-propagation/docs/05-reactive-context.md similarity index 100% rename from docs/05-reactive-context.md rename to context-propagation/docs/05-reactive-context.md diff --git a/docs/06-scheduled-tasks.md b/context-propagation/docs/06-scheduled-tasks.md similarity index 100% rename from docs/06-scheduled-tasks.md rename to context-propagation/docs/06-scheduled-tasks.md diff --git a/docs/07-servlet-filter-persistence.md b/context-propagation/docs/07-servlet-filter-persistence.md similarity index 100% rename from docs/07-servlet-filter-persistence.md rename to context-propagation/docs/07-servlet-filter-persistence.md diff --git a/docs/08-testing-contract.md b/context-propagation/docs/08-testing-contract.md similarity index 100% rename from docs/08-testing-contract.md rename to context-propagation/docs/08-testing-contract.md diff --git a/docs/output/demo1.txt b/context-propagation/docs/output/demo1.txt similarity index 100% rename from docs/output/demo1.txt rename to context-propagation/docs/output/demo1.txt diff --git a/docs/output/demo2.txt b/context-propagation/docs/output/demo2.txt similarity index 100% rename from docs/output/demo2.txt rename to context-propagation/docs/output/demo2.txt diff --git a/docs/output/demo3.txt b/context-propagation/docs/output/demo3.txt similarity index 100% rename from docs/output/demo3.txt rename to context-propagation/docs/output/demo3.txt diff --git a/docs/output/demo4.txt b/context-propagation/docs/output/demo4.txt similarity index 100% rename from docs/output/demo4.txt rename to context-propagation/docs/output/demo4.txt diff --git a/docs/output/demo5.txt b/context-propagation/docs/output/demo5.txt similarity index 100% rename from docs/output/demo5.txt rename to context-propagation/docs/output/demo5.txt diff --git a/docs/output/demo6.txt b/context-propagation/docs/output/demo6.txt similarity index 100% rename from docs/output/demo6.txt rename to context-propagation/docs/output/demo6.txt diff --git a/docs/output/demo7.txt b/context-propagation/docs/output/demo7.txt similarity index 100% rename from docs/output/demo7.txt rename to context-propagation/docs/output/demo7.txt diff --git a/docs/output/tests.txt b/context-propagation/docs/output/tests.txt similarity index 100% rename from docs/output/tests.txt rename to context-propagation/docs/output/tests.txt diff --git a/pom.xml b/context-propagation/pom.xml similarity index 100% rename from pom.xml rename to context-propagation/pom.xml diff --git a/scripts/run-all.sh b/context-propagation/scripts/run-all.sh similarity index 100% rename from scripts/run-all.sh rename to context-propagation/scripts/run-all.sh diff --git a/src/main/java/com/ankurm/vt/Demo1PlainThreadLocal.java b/context-propagation/src/main/java/com/ankurm/vt/Demo1PlainThreadLocal.java similarity index 100% rename from src/main/java/com/ankurm/vt/Demo1PlainThreadLocal.java rename to context-propagation/src/main/java/com/ankurm/vt/Demo1PlainThreadLocal.java diff --git a/src/main/java/com/ankurm/vt/Demo2AsyncVirtualThreads.java b/context-propagation/src/main/java/com/ankurm/vt/Demo2AsyncVirtualThreads.java similarity index 100% rename from src/main/java/com/ankurm/vt/Demo2AsyncVirtualThreads.java rename to context-propagation/src/main/java/com/ankurm/vt/Demo2AsyncVirtualThreads.java diff --git a/src/main/java/com/ankurm/vt/Demo3StructuredConcurrency.java b/context-propagation/src/main/java/com/ankurm/vt/Demo3StructuredConcurrency.java similarity index 100% rename from src/main/java/com/ankurm/vt/Demo3StructuredConcurrency.java rename to context-propagation/src/main/java/com/ankurm/vt/Demo3StructuredConcurrency.java diff --git a/src/main/java/com/ankurm/vt/Demo4ExecutorWrapping.java b/context-propagation/src/main/java/com/ankurm/vt/Demo4ExecutorWrapping.java similarity index 100% rename from src/main/java/com/ankurm/vt/Demo4ExecutorWrapping.java rename to context-propagation/src/main/java/com/ankurm/vt/Demo4ExecutorWrapping.java diff --git a/src/main/java/com/ankurm/vt/Demo5ReactiveContext.java b/context-propagation/src/main/java/com/ankurm/vt/Demo5ReactiveContext.java similarity index 100% rename from src/main/java/com/ankurm/vt/Demo5ReactiveContext.java rename to context-propagation/src/main/java/com/ankurm/vt/Demo5ReactiveContext.java diff --git a/src/main/java/com/ankurm/vt/Demo6ScheduledSystemIdentity.java b/context-propagation/src/main/java/com/ankurm/vt/Demo6ScheduledSystemIdentity.java similarity index 100% rename from src/main/java/com/ankurm/vt/Demo6ScheduledSystemIdentity.java rename to context-propagation/src/main/java/com/ankurm/vt/Demo6ScheduledSystemIdentity.java diff --git a/src/main/java/com/ankurm/vt/Demo7ServletFilterPersistence.java b/context-propagation/src/main/java/com/ankurm/vt/Demo7ServletFilterPersistence.java similarity index 100% rename from src/main/java/com/ankurm/vt/Demo7ServletFilterPersistence.java rename to context-propagation/src/main/java/com/ankurm/vt/Demo7ServletFilterPersistence.java diff --git a/src/test/java/com/ankurm/vt/SecurityContextPropagationContractTest.java b/context-propagation/src/test/java/com/ankurm/vt/SecurityContextPropagationContractTest.java similarity index 100% rename from src/test/java/com/ankurm/vt/SecurityContextPropagationContractTest.java rename to context-propagation/src/test/java/com/ankurm/vt/SecurityContextPropagationContractTest.java diff --git a/method-security/README.md b/method-security/README.md new file mode 100644 index 0000000..1fbd2e1 --- /dev/null +++ b/method-security/README.md @@ -0,0 +1,95 @@ +# method-security + +Companion module for [Method Security in Spring Security 7: `@PreAuthorize`, `@PostAuthorize` +and the Proxy Traps](https://ankurm.com/spring-security-7-method-security-proxy-traps/) +on ankurm.com. + +Nine small programs and a JUnit suite, each answering one question about what +`@PreAuthorize` and its siblings actually do at runtime — including the three cases where they +do nothing at all and say nothing about it. No web layer, no Boot application, no server: a +plain `AnnotationConfigApplicationContext`, a `SecurityContextHolder`, and real proxied beans, +so every result is about method security and not about a filter chain. + +Part of [spring-security-demo](../README.md); the sibling module is +[context-propagation](../context-propagation/README.md). + +## Verified versions + +| Component | Version | +|---|---| +| JDK | 25 (Temurin 25.0.4.1+1), LTS | +| Spring Boot (reference target) | 4.1.1 | +| Spring Framework | 7.0.9 | +| Spring Security | 7.1.1 (`-core`, `-config`, `-test`) | +| AspectJ Weaver | 1.9.25 (only for `exposeProxy` in Demo 2) | +| Spring Data Commons | 4.1.1 (only so Demo 5 can filter a real `Page`) | +| H2 | 2.4.240 (Demo 6's transaction rollback proof) | +| JUnit Jupiter | 6.0.3 | +| AssertJ | 3.27.7 | + +Latest GA on Maven Central at the time of writing, taken from `maven-metadata.xml`. +`4.2.0-M1` / `7.2.0-M1` exist as milestones only. + +## Quickstart + +```bash +mvn dependency:build-classpath -Dmdep.outputFile=cp.txt +javac --release 25 -parameters -cp "$(cat cp.txt)" -d target/classes $(find src/main -name '*.java') +java -cp "target/classes:$(cat cp.txt)" com.ankurm.methodsec.Demo2SelfInvocation +``` + +`-parameters` matters: without it every `#parameterName` expression in this module silently +stops working, which is what Demo 9 is about. + +Everything at once, regenerating `docs/output/`: `scripts/run-all.sh`. +Just the assertions: `mvn test`. + +## What each demo shows + +| Demo | Question | Chapter | +|---|---|---| +| `Demo1AnnotationsInAction` | What do all six annotation families do on the happy path, and what is thrown when they deny? | [docs/01](docs/01-how-method-security-runs.md) | +| `Demo2SelfInvocation` | Why does an annotated method called from inside its own class run unchecked, and what are the three fixes? | [docs/03](docs/03-self-invocation.md) | +| `Demo3NonProxyable` | Which of `final` / `static` / `private` / package-private / final-class / interface-only actually get advised? | [docs/04](docs/04-non-proxyable-methods.md) | +| `Demo4SpelReference` | Every expression you can write inside the annotation, evaluated as two different users | [docs/02](docs/02-spel-reference.md) | +| `Demo5FilteringTraps` | Which container types can `@PreFilter` / `@PostFilter` handle, and what happens on an immutable one? | [docs/05](docs/05-filtering.md) | +| `Demo6InterceptorOrder` | Where does the security advice sit relative to `@Transactional`, and does a denial roll back? | [docs/07](docs/07-ordering-and-transactions.md) | +| `Demo7DeniedHandling` | What is actually thrown, and how do `@HandleAuthorizationDenied` and `@AuthorizeReturnObject` change it? | [docs/06](docs/06-denied-handling.md) | +| `Demo8MetaAnnotations` | Do `{value}` templates need a bean? What does a method-level rule do to a class-level one? | [docs/08](docs/08-meta-annotations.md) | +| `Demo9ParameterNames` | The same class compiled with and without `-parameters` | [docs/02](docs/02-spel-reference.md) | +| `MethodSecurityTrapsTest` | 14 assertions pinning every claim above | [docs/09](docs/09-audit-checklist.md) | + +## Captured output + +| File | From | +|---|---| +| [`docs/output/demo1.txt`](docs/output/demo1.txt) … [`demo8.txt`](docs/output/demo8.txt) | the eight demos above | +| [`docs/output/demo9-with-parameters.txt`](docs/output/demo9-with-parameters.txt) | Demo 9, compiled with `-parameters` | +| [`docs/output/demo9-without-parameters.txt`](docs/output/demo9-without-parameters.txt) | Demo 9, same source, compiled without it | +| [`docs/output/tests.txt`](docs/output/tests.txt) | `mvn test` | + +Regenerate all of it with `scripts/run-all.sh`. Nothing in `docs/` or in the article is +hand-typed output. + +## Documentation corrections found while building this + +Each of these disagrees with the current reference documentation or with widely repeated +guidance, and each was verified by running the code or reading the 7.1.1 source: + +- `@EnableMethodSecurity` has an `offset` attribute, not `order`. +- `AuthorizationProxyFactory` is in `org.springframework.security.authorization`, not + `…authorization.method`. +- `AbstractSecurityExpressionHandler.setRoleHierarchy(..)` is deprecated in 7.1; + `AuthorizationManagerFactory` is where role hierarchy and role prefix now live. +- `{value}` meta-annotation templates work **without** an `AnnotationTemplateExpressionDefaults` + bean. +- Conflicting `@PreAuthorize` inherited from two interfaces fails at **call time**, not at + startup. +- `@PreFilter` on an immutable collection is a **silent no-op**, not an exception. +- Package-private methods **are** advised by a CGLIB proxy. + +See [docs/09](docs/09-audit-checklist.md) for the full index. + +## License + +MIT — see [LICENSE](../LICENSE). diff --git a/method-security/docs/01-how-method-security-runs.md b/method-security/docs/01-how-method-security-runs.md new file mode 100644 index 0000000..36836b5 --- /dev/null +++ b/method-security/docs/01-how-method-security-runs.md @@ -0,0 +1,76 @@ +[← chapter index](README.md) · [next: the SpEL reference →](02-spel-reference.md) + +# 01 · How method security actually runs + +Run: `java -cp target/classes:$(cat cp.txt) com.ankurm.methodsec.Demo1AnnotationsInAction` +Output: [`output/demo1.txt`](output/demo1.txt) +Source: [`Demo1AnnotationsInAction.java`](../src/main/java/com/ankurm/methodsec/Demo1AnnotationsInAction.java) + +## The one-sentence model + +`@PreAuthorize` is not a keyword the JVM understands. It is an annotation that a Spring AOP +**advisor** matches, on a **proxy** that wraps your bean, intercepting calls that arrive from +outside. Every trap in this repository follows from that sentence. + +## What `@EnableMethodSecurity` switches on + +Nothing happens without it. Spring Boot's security auto-configuration does not enable method +security; the annotation is yours to add. Its attributes, read out of the `AnnotationDefault` +attributes in `spring-security-config-7.1.1.jar` rather than from documentation: + +| Attribute | Default | Effect | +|---|---|---| +| `prePostEnabled` | `true` | `@PreAuthorize`, `@PostAuthorize`, `@PreFilter`, `@PostFilter` | +| `securedEnabled` | `false` | `@Secured` | +| `jsr250Enabled` | `false` | `@RolesAllowed`, `@PermitAll`, `@DenyAll` | +| `proxyTargetClass` | `false` | Force CGLIB even when the bean implements interfaces | +| `mode` | `AdviceMode.PROXY` | The alternative is `ASPECTJ`, which sidesteps chapters 03 and 04 entirely | +| `offset` | `0` | Shifts every security advisor's order by this amount | + +Note the third row. `@Secured("ROLE_ADMIN")` and `@RolesAllowed("ADMIN")` compile, look +correct in review, and do nothing at all until you switch them on. That is the zeroth silent +failure, and unlike the two the article is named for it takes one attribute to fix. + +There is no `order` attribute, despite what several guides say. It is `offset`, and it moves +all the interceptors together — see [chapter 07](07-ordering-and-transactions.md). + +## The call path + +``` +caller + └─ proxy (CGLIB subclass, or JDK dynamic proxy) + └─ @PreFilter advisor, order 100 mutates the argument collection + └─ @PreAuthorize advisor, order 200 evaluates SpEL, throws or proceeds + └─ @PostAuthorize advisor, order 500 + └─ @PostFilter advisor, order 600 + └─ your method body +``` + +Each advisor builds a `MethodSecurityExpressionRoot` over the `Authentication` from +`SecurityContextHolder` plus the `MethodInvocation`, hands it to a +`MethodSecurityExpressionHandler`, and evaluates the annotation's expression against it. + +## What a denial looks like + +`Demo1` runs the same service as three identities. Two details from +[`output/demo1.txt`](output/demo1.txt) are worth internalising: + +- With an `Authentication` present but insufficient, you get `AuthorizationDeniedException`. +- With **no** `Authentication` at all you get `AuthenticationCredentialsNotFoundException` + instead — a different exception, from a different place, which an `@ExceptionHandler` written + only for `AccessDeniedException` will not catch. `@PermitAll` still returns normally, because + it never asks for the `Authentication`. + +In a servlet application both are normally translated by `ExceptionTranslationFilter`, so you +see 403 and 401 respectively. Outside a request — a scheduled job, a message listener, a test — +nothing translates them and they surface raw. + +## Where the `Authentication` comes from + +`SecurityContextHolder`, on the calling thread. If the call happens on a thread that never +received the context, method security does not fail open; it throws +`AuthenticationCredentialsNotFoundException`. Getting the context onto that thread is a +separate topic with its own module in this repository — see +[`../context-propagation/`](../context-propagation/README.md). + +[← chapter index](README.md) · [next: the SpEL reference →](02-spel-reference.md) diff --git a/method-security/docs/02-spel-reference.md b/method-security/docs/02-spel-reference.md new file mode 100644 index 0000000..d547c18 --- /dev/null +++ b/method-security/docs/02-spel-reference.md @@ -0,0 +1,117 @@ +[← 01 · how it runs](01-how-method-security-runs.md) · [chapter index](README.md) · [next: self-invocation →](03-self-invocation.md) + +# 02 · The SpEL reference + +Run: `java -cp target/classes:$(cat cp.txt) com.ankurm.methodsec.Demo4SpelReference` +Output: [`output/demo4.txt`](output/demo4.txt), [`output/demo9-with-parameters.txt`](output/demo9-with-parameters.txt), [`output/demo9-without-parameters.txt`](output/demo9-without-parameters.txt) +Source: [`Demo4SpelReference.java`](../src/main/java/com/ankurm/methodsec/Demo4SpelReference.java), [`Demo9ParameterNames.java`](../src/main/java/com/ankurm/methodsec/Demo9ParameterNames.java) + +Every row below is a real annotated method on a real proxied bean in `Demo4`, invoked as two +different users. Nothing here is transcribed from documentation. + +## Predicates + +| Expression | Meaning | +|---|---| +| `hasRole('ADMIN')` | authority `ROLE_ADMIN` (prefix configurable) | +| `hasAnyRole('A','B')` | any of them | +| `hasAllRoles('A','B')` | all of them | +| `hasAuthority('report:read')` | the authority string, verbatim, no prefix | +| `hasAnyAuthority(..)` / `hasAllAuthorities(..)` | as above | +| `isAuthenticated()` | authenticated and not anonymous | +| `isFullyAuthenticated()` | authenticated, not anonymous, not remember-me | +| `isRememberMe()` / `isAnonymous()` | the two it excludes | +| `permitAll` / `denyAll` | constants; also callable as `permitAll()` / `denyAll()` | +| `hasPermission(target, permission)` | delegates to a `PermissionEvaluator` bean | +| `hasPermission(id, 'type', permission)` | the id/type overload | + +`hasAllRoles` and `hasAllAuthorities` are easy to miss — they are on +`SecurityExpressionRoot` alongside the `any` variants. + +## The root object + +`MethodSecurityExpressionRoot` (package-private, extends +`SecurityExpressionRoot`) exposes exactly: + +| Reference | Available in | What it is | +|---|---|---| +| `authentication` | everywhere | the `Authentication` | +| `principal` | everywhere | `authentication.getPrincipal()` | +| `returnObject` | `@PostAuthorize`, `@PostFilter` | the value the method returned | +| `filterObject` | `@PreFilter`, `@PostFilter` | the element currently being tested | +| `#root.this` | everywhere | the target object | +| `read`, `write`, `create`, `delete`, `admin` | everywhere | `String` constants for `hasPermission(..)` | + +There is **no positional access to arguments**. `#root.args[0]` fails with + +``` +EL1008E: Property or field 'args' cannot be found on object of type +'org.springframework.security.access.expression.method.MethodSecurityExpressionRoot' +``` + +Arguments are bound by name, which brings us to the flag. + +## `#parameterName` needs `-parameters` + +`@PreAuthorize("#owner == authentication.name")` resolves `#owner` through a +`ParameterNameDiscoverer`. Java only keeps parameter names in the class file when `javac` is +given `-parameters`. Without it the name is `arg0`, `#owner` resolves to nothing, and the +comparison is false. + +The same class, same expression, two compilations — +[`demo9-with-parameters.txt`](output/demo9-with-parameters.txt) versus +[`demo9-without-parameters.txt`](output/demo9-without-parameters.txt): + +``` +compiled with -parameters : true compiled with -parameters : false +byParameterName param[0] : owner byParameterName param[0] : arg0 +alice → #owner == name : ALLOWED alice → #owner == name : DENIED +``` + +It fails closed, which is the good direction, but it fails **silently** — the rule being +enforced is not the rule you wrote. Spring Boot's Maven and Gradle plugins set `-parameters` +for you; a hand-rolled build, a shaded jar, or a module compiled by a different toolchain may +not. `@P("alias")` from `org.springframework.security.core.parameters` does not depend on the +flag, because the name lives in the annotation. + +## Beans and types + +`@beanName.method(...)` resolves a bean from the context, which is the cleanest way to move a +non-trivial rule out of a string literal and into testable code: + +```java +@PreAuthorize("@accountPolicy.canRead(authentication, #id)") +public Account read(long id) { ... } +``` + +`T(java.time.LocalDate).now().year >= 2020` works too. It is legal, and it is a warning sign: +an expression that needs a type reference is an expression that wants to be a bean method. + +## Role prefix and hierarchy, in 7.1 + +Both moved. `AbstractSecurityExpressionHandler.setRoleHierarchy(..)` is **deprecated** in +Spring Security 7.1 — the compiler says so, which is how this module found out. The current +knob is an `AuthorizationManagerFactory`: + +```java +@Bean +static AuthorizationManagerFactory authorizationManagerFactory() { + DefaultAuthorizationManagerFactory factory = new DefaultAuthorizationManagerFactory<>(); + factory.setRoleHierarchy(RoleHierarchyImpl.withDefaultRolePrefix() + .role("ADMIN").implies("USER") + .role("USER").implies("GUEST") + .build()); + factory.setRolePrefix("ROLE_"); + return factory; +} +``` + +`PrePostMethodSecurityConfiguration` autowires both an `AuthorizationManagerFactory` and a +bare `RoleHierarchy` bean (`@Autowired(required = false)` on each), so a plain `RoleHierarchy` +bean still works. The factory is where the prefix and the hierarchy now live together, and it +is the one that is not deprecated. + +Verified in [`output/demo4.txt`](output/demo4.txt): with the hierarchy above, `root` holding +only `ROLE_ADMIN` passes `hasRole('GUEST')`. + +[← 01 · how it runs](01-how-method-security-runs.md) · [chapter index](README.md) · [next: self-invocation →](03-self-invocation.md) diff --git a/method-security/docs/03-self-invocation.md b/method-security/docs/03-self-invocation.md new file mode 100644 index 0000000..96fdb0a --- /dev/null +++ b/method-security/docs/03-self-invocation.md @@ -0,0 +1,90 @@ +[← 02 · SpEL reference](02-spel-reference.md) · [chapter index](README.md) · [next: non-proxyable methods →](04-non-proxyable-methods.md) + +# 03 · Self-invocation + +Run: `java -cp target/classes:$(cat cp.txt) com.ankurm.methodsec.Demo2SelfInvocation` +Output: [`output/demo2.txt`](output/demo2.txt) +Source: [`Demo2SelfInvocation.java`](../src/main/java/com/ankurm/methodsec/Demo2SelfInvocation.java) + +## The shape + +```java +@PreAuthorize("hasRole('ADMIN')") +public String adminReport() { return "TOP SECRET REVENUE NUMBERS"; } + +public String userEntryPoint() { + return adminReport(); // no proxy involved +} +``` + +A caller invoking `adminReport()` from outside goes through the proxy and is denied. A caller +invoking `userEntryPoint()` reaches the target object, and `this.adminReport()` is a plain +virtual call on the target. The advisor never sees it. + +From [`output/demo2.txt`](output/demo2.txt), as a user holding only `ROLE_USER`: + +``` +reports.adminReport() (via proxy) DENIED -> AuthorizationDeniedException +reports.userEntryPoint() (this.adminReport()) ALLOWED -> TOP SECRET REVENUE NUMBERS +``` + +## Why it survives review + +The same run prints: + +``` +ReportService.adminReport() @PreAuthorize -> @PreAuthorize("hasRole('ADMIN')") +bean is an AOP proxy -> true +proxy class -> ...ReportService$$SpringCGLIB$$0 +``` + +The annotation is present. The bean *is* proxied. Reflection agrees with the source. Every +individual check a reviewer would run passes; only the composition is wrong. Nothing logs at +any level, no metric moves, and the method returns the right answer to the wrong person. + +## Three fixes, in order of preference + +**Move the method to a different bean.** A call between two beans is an external call by +definition. This is the boring answer and it is usually the right one, because a method that +needs its own authorization rule is usually a different responsibility. + +**Inject the bean into itself through an `ObjectProvider`.** + +```java +private final ObjectProvider self; + +public String viaSelfInjection() { + return this.self.getObject().adminReport(); +} +``` + +A direct field of the bean's own type is a circular reference the container refuses in a +constructor; `ObjectProvider` resolves lazily at call time and hands back the proxy. + +**`AopContext.currentProxy()`.** + +```java +public String viaAopContext() { + return ((ReportService) AopContext.currentProxy()).adminReport(); +} +``` + +This needs `@EnableAspectJAutoProxy(exposeProxy = true)` — without it the call throws +`IllegalStateException: Cannot find current proxy: Set 'exposeProxy' property on Advised to +'true' to make it available`. It also couples the method to the fact that it is proxied, which +is why it is last on this list. + +All three are verified in the same run; all three produce `AuthorizationDeniedException`. + +## The structural fix + +`@EnableMethodSecurity(mode = AdviceMode.ASPECTJ)` weaves the advice into the bytecode +instead of wrapping the object, and self-invocation stops being a thing. It also requires +AspectJ weaving in the build, which is a large change to make for one class of bug. Consider it +if you are already weaving. + +The cheaper structural answer is not to depend on method security alone: keep a catch-all +`authorizeHttpRequests` rule (`anyRequest().authenticated()`) so that an inner method that +escapes its own check is still behind a request-level one. + +[← 02 · SpEL reference](02-spel-reference.md) · [chapter index](README.md) · [next: non-proxyable methods →](04-non-proxyable-methods.md) diff --git a/method-security/docs/04-non-proxyable-methods.md b/method-security/docs/04-non-proxyable-methods.md new file mode 100644 index 0000000..7db88c5 --- /dev/null +++ b/method-security/docs/04-non-proxyable-methods.md @@ -0,0 +1,85 @@ +[← 03 · self-invocation](03-self-invocation.md) · [chapter index](README.md) · [next: filtering →](05-filtering.md) + +# 04 · Methods the proxy cannot advise + +Run: `java -cp target/classes:$(cat cp.txt) com.ankurm.methodsec.Demo3NonProxyable` +Output: [`output/demo3.txt`](output/demo3.txt) +Source: [`Demo3NonProxyable.java`](../src/main/java/com/ankurm/methodsec/Demo3NonProxyable.java) + +A CGLIB proxy is a generated subclass. It intercepts a method by overriding it. Anything that +cannot be overridden cannot be advised. + +## The results + +Same class, same `@PreAuthorize("hasRole('ADMIN')")` on every method, same `ROLE_USER` caller: + +| Declaration | Advised? | How you find out | +|---|---|---| +| `public` | yes | denied, as intended | +| `public final` | **no** | a `WARNING` from `CglibAopProxy` at startup | +| `static` | **no** | nothing at all | +| package-private | yes | denied — the generated subclass is in the same package | +| `private` | **no** | nothing at all; also unreachable except by self-invocation | +| `final class` | n/a | the context **fails to start** | + +The package-private row is the surprise. Received wisdom is "only public methods are secured"; +the generated subclass lands in the same package as the target, so it can and does override a +package-private method. Verified by reflection in the same run: + +``` +publicAdminOnly declared final=false overridden by proxy=true +finalAdminOnly declared final=true overridden by proxy=false +packagePrivateAdminOnly declared final=false overridden by proxy=true +``` + +## `final` is the loud one, `static` and `private` are not + +Spring does warn about a public `final` method — `CglibAopProxy.doValidateClass` logs: + +``` +WARNING: Public final method [public final java.lang.String ...Vault.finalAdminOnly()] +cannot get proxied via CGLIB, consider removing the final marker or using interface-based +JDK proxies. +``` + +It is in [`output/demo3.txt`](output/demo3.txt), at the very top, at startup, mixed in with +everything else an application logs while booting. It is easy to miss and it is more than you +get for `static` and `private`, which produce nothing. + +A `final` **class** is different again: CGLIB cannot subclass it at all, so the container +refuses to start with +`IllegalArgumentException: Cannot subclass final class ...SealedVault`. That is the friendliest +failure in this chapter. It is also the reason a Java `record` cannot carry method security on +its own methods — records are final. If you need a secured getter on a returned object (see +[chapter 06](06-denied-handling.md)), it has to be a non-final class. + +## Interfaces and JDK proxies + +`@EnableMethodSecurity(proxyTargetClass = false)` is the default, so a bean that implements an +interface gets a **JDK dynamic proxy**, which implements only the interfaces. A public +annotated method that is not on the interface is then not merely unadvised — it is not on the +proxy at all: + +``` +proxy is a JDK proxy -> true +proxied interfaces -> [interface ...LedgerOperations] +cast proxy to Ledger impl -> ClassCastException: class jdk.proxy2.$Proxy18 cannot be cast to ...Ledger +``` + +In a Spring Boot application this is usually moot: Boot sets `proxyTargetClass = true` globally +via `spring.aop.proxy-target-class`, which defaults to `true`. In a plain Spring context, or +with that property flipped, it is live. Either way, the `ClassCastException` is loud — the +dangerous version is the one where the interface method *is* annotated and the implementation +carries a second, different annotation; see [chapter 08](08-meta-annotations.md). + +## What to actually do + +- Do not put `@PreAuthorize` on anything `private` or `static`. Neither is reachable through a + proxy, and neither will tell you. +- Remove `final` from methods that carry security annotations, or make peace with the fact that + the annotation is documentation. +- If you use records or other final classes as return values, secure the method that returns + them, not the accessors on them. +- Grep for the combination. [Chapter 09](09-audit-checklist.md) has the patterns. + +[← 03 · self-invocation](03-self-invocation.md) · [chapter index](README.md) · [next: filtering →](05-filtering.md) diff --git a/method-security/docs/05-filtering.md b/method-security/docs/05-filtering.md new file mode 100644 index 0000000..c1cd7fe --- /dev/null +++ b/method-security/docs/05-filtering.md @@ -0,0 +1,120 @@ +[← 04 · non-proxyable methods](04-non-proxyable-methods.md) · [chapter index](README.md) · [next: denial handling →](06-denied-handling.md) + +# 05 · Filtering, `filterObject`, and the third silent failure + +Run: `java -cp target/classes:$(cat cp.txt) com.ankurm.methodsec.Demo5FilteringTraps` +Output: [`output/demo5.txt`](output/demo5.txt) +Source: [`Demo5FilteringTraps.java`](../src/main/java/com/ankurm/methodsec/Demo5FilteringTraps.java) + +`@PreFilter` and `@PostFilter` evaluate their expression once per element, with the element +bound to `filterObject`, and drop the ones that evaluate false. + +## `@PreFilter` mutates the caller's collection + +It does not hand your method a filtered copy. It clears the collection the caller passed and +adds the survivors back into it. From [`output/demo1.txt`](output/demo1.txt): + +``` +caller's list before the call : [Account[1,alice,100], Account[2,bob,200], Account[3,alice,300]] +method body saw : [Account[1,alice,150], Account[3,alice,350]] +caller's list after the call : [Account[1,alice,150], Account[3,alice,350]] +``` + +The caller's own list lost an element. If that list was a field, a cache, or a collection +shared with anything else, it lost the element there too. + +## The silent failure: an immutable argument + +``` +method body saw: [Account[1,alice,100], Account[2,bob,200], Account[3,alice,300]] +List.of(..) (immutable) ALLOWED -> (void) +``` + +Bob's account reached the method body. No exception, no warning, no 403. + +The mechanism, from +`DefaultMethodSecurityExpressionHandler.filterCollection` in Spring Security 7.1.1: + +```java +try { + filterTarget.clear(); + filterTarget.addAll(retain); + return filterTarget; +} +catch (UnsupportedOperationException readonly) { + this.logger.trace("Collection threw exception: ... Will return a new instance instead of mutating its state."); + return retain; +} +``` + +It computes the right answer, cannot write it back, and returns a fresh list instead. And +`PreFilterAuthorizationMethodInterceptor.invoke` is: + +```java +Object filterTarget = findFilterTarget(attribute.getFilterTarget(), ctx, mi); +expressionHandler.filter(filterTarget, attribute.getExpression(), ctx); +return mi.proceed(); +``` + +The return value of `filter(..)` is discarded. `@PreFilter` is *entirely* dependent on in-place +mutation working. When it does not, the filter is a no-op and the unfiltered elements go +straight into the method body. + +The only trace is at TRACE level, and the demo turns it on so you can see what it looks like: + +``` +TRACE Retaining elements: [Account[1,alice,100], Account[3,alice,300]] +TRACE Collection threw exception: null. Will return a new instance instead of mutating its state. +method body saw: [Account[1,alice,100], Account[2,bob,200], Account[3,alice,300]] +``` + +`List.of(..)`, `List.copyOf(..)`, `Collections.unmodifiableList(..)`, `Arrays.asList(..)` (which +refuses `clear()`), a Guava `ImmutableList`, and the result of `Stream.toList()` are all +immutable. `Stream.toList()` is the one that catches people, because it looks like a neutral +terminal operation. `.collect(Collectors.toList())` returns a mutable `ArrayList`. + +`@PostFilter` does not have this problem, because there the new list *is* the return value — +the same immutable input filters correctly. Verified in the same run. + +## Container types + +| Return / argument type | `@PreFilter` | `@PostFilter` | `filterObject` is | +|---|---|---|---| +| `Collection` (mutable) | works | works | the element | +| `Collection` (immutable) | **silent no-op** | works | the element | +| Array | `IllegalStateException` | works | the element | +| `Map` | works | works | the `Map.Entry` — use `filterObject.value.…` | +| `Stream` | works | works | the element | +| Anything else | `IllegalArgumentException` | `IllegalArgumentException` | — | + +`Optional` is in the "anything else" row, and so is Spring Data's `Page` — a real `PageImpl`, +not a stand-in: + +``` +Optional IllegalArgumentException: ... but was Optional[Account[1,alice,100]] +Page (real Spring Data PageImpl) IllegalArgumentException: ... but was Page 1 of 1 containing ...Account instances +``` + +`PageImpl` implements `Slice` → `Streamable` → `Iterable`, but not `Collection`, so it falls +through every branch of `DefaultMethodSecurityExpressionHandler.filter`. `@PostFilter` on a +repository method returning `Page` therefore throws at runtime, not at startup. And +filtering a page in memory would give you the wrong page size anyway — filter in the query. + +## Two more sharp edges + +**More than one argument needs `filterTarget`.** `@PreFilter` with two parameters and no +`filterTarget` throws `IllegalStateException: Unable to determine the method argument for +filtering. Specify the filter target.` — at invocation time, not at startup, so a rarely +exercised method can ship broken. Name the argument: +`@PreFilter(value = "…", filterTarget = "accounts")`. + +**`@PostFilter` returns the same instance it filtered.** Verified: + +``` +returned == the list the method returned : true +``` + +If the method returned a cached or shared collection, `@PostFilter` has now deleted elements +from it for every future caller. Return a defensive copy from any method you filter. + +[← 04 · non-proxyable methods](04-non-proxyable-methods.md) · [chapter index](README.md) · [next: denial handling →](06-denied-handling.md) diff --git a/method-security/docs/06-denied-handling.md b/method-security/docs/06-denied-handling.md new file mode 100644 index 0000000..a64678c --- /dev/null +++ b/method-security/docs/06-denied-handling.md @@ -0,0 +1,104 @@ +[← 05 · filtering](05-filtering.md) · [chapter index](README.md) · [next: ordering and transactions →](07-ordering-and-transactions.md) + +# 06 · Denial: what is thrown, and how to change it + +Run: `java -cp target/classes:$(cat cp.txt) com.ankurm.methodsec.Demo7DeniedHandling` +Output: [`output/demo7.txt`](output/demo7.txt) +Source: [`Demo7DeniedHandling.java`](../src/main/java/com/ankurm/methodsec/Demo7DeniedHandling.java) + +## The exception + +``` +thrown -> org.springframework.security.authorization.AuthorizationDeniedException +is AccessDeniedException -> true +is AuthorizationDeniedException -> true +carries an AuthorizationResult -> ExpressionAuthorizationDecision granted=false +``` + +Documentation and older posts say "throws `AccessDeniedException`", and code written against +that still catches it. But the concrete type is `AuthorizationDeniedException`, and it carries +the `AuthorizationResult` that explains the denial — for a SpEL rule, an +`ExpressionAuthorizationDecision` that knows which expression failed. That is the difference +between an audit log line saying "access denied" and one saying which rule denied it. + +## Returning something instead of throwing + +`@HandleAuthorizationDenied` names a `MethodAuthorizationDeniedHandler` bean, which gets the +`MethodInvocation` and the `AuthorizationResult` and returns a value in place of the throw: + +```java +@PreAuthorize("hasRole('FINANCE')") +@HandleAuthorizationDenied(handlerClass = MaskingHandler.class) +public String maskedBalance() { return "1,204,993.22"; } +``` + +``` +maskedBalance() (alice, no ROLE_FINANCE) ALLOWED -> ***masked*** +maskedList() (alice, no ROLE_FINANCE) ALLOWED -> [] +maskedBalance() (cfo, has ROLE_FINANCE) ALLOWED -> 1,204,993.22 +``` + +The returned value must be assignable to the method's declared return type, so a handler shared +across methods has to inspect it — the demo's handler returns `List.of()` for a `List` return +type and a masked string otherwise. A handler that gets this wrong fails with a +`ClassCastException` at the call site, which is worse than the denial it was replacing. + +Use this where a partial answer is genuinely correct — a masked field on a shared DTO, an empty +list for a section the user cannot see. Do not use it to make an authorization failure invisible +to your own logs. + +## `@AuthorizeReturnObject` + +Moves the check from the method that returns an object onto the object's own accessors: + +```java +public class Customer { + public String getName() { return this.name; } + + @PreAuthorize("hasAuthority('pii:read')") + public String getEmail() { return this.email; } +} + +@AuthorizeReturnObject +public Customer findCustomer(String name) { ... } +``` + +``` +returned instance -> ...Customer$$SpringCGLIB$$0 +customer.getName() (no authority needed) ALLOWED -> alice +customer.getEmail() (needs 'pii:read') DENIED -> AuthorizationDeniedException +``` + +The returned object is CGLIB-proxied, which means [chapter 04](04-non-proxyable-methods.md) +applies to it in full: the class cannot be `final`, so **a record will not work**, and a `final` +getter is not advised. The `Customer` in this demo is deliberately a plain class for exactly +that reason. + +The same thing without the annotation, through the container's `AuthorizationProxyFactory`: + +```java +AuthorizationProxyFactory factory = ctx.getBean(AuthorizationProxyFactory.class); +Customer wrapped = factory.proxy(raw); // generic, no cast needed +``` + +``` +raw.getEmail() (unproxied object) ALLOWED -> alice@example.com +wrapped.getEmail() (proxied object) DENIED -> AuthorizationDeniedException +``` + +**Package correction:** `AuthorizationProxyFactory` lives in +`org.springframework.security.authorization`, not `org.springframework.security.authorization.method` +where the reference documentation places it. The implementation, +`AuthorizationAdvisorProxyFactory`, is in `…authorization.method`; the interface is one package +up. `proxy(T)` is generic and returns `T`. + +Two more things worth knowing before you reach for it: + +- The `authorizeReturnObject` advisor is registered at order **450** + (`AuthorizationInterceptorsOrder.SECURE_RESULT`) whether or not anything in your application + uses `@AuthorizeReturnObject` — visible in [`output/demo6.txt`](output/demo6.txt). +- At class level it proxies *every* return value, including `String` and boxed primitives. + Publish `AuthorizationAdvisorProxyFactory.TargetVisitor.defaultsSkipValueTypes()` if you go + that route. + +[← 05 · filtering](05-filtering.md) · [chapter index](README.md) · [next: ordering and transactions →](07-ordering-and-transactions.md) diff --git a/method-security/docs/07-ordering-and-transactions.md b/method-security/docs/07-ordering-and-transactions.md new file mode 100644 index 0000000..6bc5124 --- /dev/null +++ b/method-security/docs/07-ordering-and-transactions.md @@ -0,0 +1,121 @@ +[← 06 · denial handling](06-denied-handling.md) · [chapter index](README.md) · [next: meta-annotations →](08-meta-annotations.md) + +# 07 · Ordering, and `@PostAuthorize` vs `@Transactional` + +Run: `java -cp target/classes:$(cat cp.txt) com.ankurm.methodsec.Demo6InterceptorOrder` +Output: [`output/demo6.txt`](output/demo6.txt) +Source: [`Demo6InterceptorOrder.java`](../src/main/java/com/ankurm/methodsec/Demo6InterceptorOrder.java) + +## The numbers, read from the enum + +`AuthorizationInterceptorsOrder`, printed from the running JVM rather than transcribed: + +| Constant | `getOrder()` | +|---|---| +| `FIRST` | `-2147483648` | +| `PRE_FILTER` | `100` | +| `PRE_AUTHORIZE` | `200` | +| `SECURED` | `300` | +| `JSR250` | `400` | +| `SECURE_RESULT` | `450` | +| `POST_AUTHORIZE` | `500` | +| `POST_FILTER` | `600` | +| `LAST` | `2147483647` | + +And the advisor beans `@EnableMethodSecurity` actually registers: + +``` + 100 preFilterAuthorizationMethodInterceptor + 200 preAuthorizeAuthorizationMethodInterceptor + 450 authorizeReturnObjectMethodInterceptor + 500 postAuthorizeAuthorizationMethodInterceptor + 600 postFilterAuthorizationMethodInterceptor +``` + +## Lower order means further out — which flips for "after" advice + +Lower order = higher precedence = further *out* in the chain. For the two "before" +annotations that reads the obvious way: `@PreFilter` (100) runs before `@PreAuthorize` (200). + +For the two "after" annotations it reads backwards. `@PostAuthorize` (500) sits **further out** +than `@PostFilter` (600), so on the way back out `@PostFilter` finishes first and +`@PostAuthorize` evaluates `returnObject` against the **already-filtered** value. + +Two methods, identical except for the expected size, prove it: + +```java +@PostAuthorize("returnObject.size() == 3") +@PostFilter("filterObject != 'c'") +public List expectsThree(List in) { return in; } +``` + +``` +@PostAuthorize returnObject.size() == 3 DENIED +@PostAuthorize returnObject.size() == 2 ALLOWED -> [a, b] +``` + +Three elements went in, the method returned three, and `@PostAuthorize` saw two. If you have +both annotations on one method, `returnObject` is not what the method returned. + +## The transaction problem + +Spring's `@Transactional` advisor defaults to `Ordered.LOWEST_PRECEDENCE` (`2147483647`), +which is larger than every security order above. So **security wraps transactions**: the +transaction commits, and only then does `@PostAuthorize` decide the caller may not see the +result. + +```java +@Transactional +@PostAuthorize("returnObject.owner == authentication.name") +public Account recordAndReturn(String owner) { + this.jdbc.update("insert into audit(owner) values (?)", owner); + return new Account(1, owner, 100); +} +``` + +Against a real H2 database, as a caller who is denied: + +``` +rows before : 0 +recordAndReturn("bob") @PostAuthorize DENIED -> AuthorizationDeniedException +rows after the denial : 1 +``` + +The row is there. The caller got a 403 and the write happened anyway. + +## Fixing it + +Move the transaction advisor outside the security advisor, so the +`AuthorizationDeniedException` propagates through it and triggers the normal +rollback-on-`RuntimeException` rule: + +```java +@EnableTransactionManagement(order = Integer.MIN_VALUE) +``` + +Same run, same denial: + +``` +rows before : 0 +recordAndReturn("bob") @PostAuthorize DENIED -> AuthorizationDeniedException +rows after the denial : 0 +``` + +`@EnableMethodSecurity(offset = ...)` shifts every security interceptor by a fixed amount and +gets you to the same place from the other side. Use whichever you can reason about later; +`@EnableTransactionManagement(order = ...)` names the thing you are actually moving. + +**But prefer not to need it.** Rolling back on an authorization failure means you have already +done the work and are undoing it, and rollback is not universal — a message you published, a +file you wrote, an outbound HTTP call are all still gone. The reference documentation's advice +holds: do not combine `@PostAuthorize` with a method that writes. Authorize on the way in with +`@PreAuthorize` and arguments, or read with `@PostAuthorize` and write from a separate method. + +## While you are here: `@Transactional` has the same two traps + +Everything in [chapter 03](03-self-invocation.md) and +[chapter 04](04-non-proxyable-methods.md) applies to `@Transactional` unchanged — same proxy, +same overriding rules. A `private @Transactional` method is exactly as inert as a `private +@PreAuthorize` one. If you find one, look for the other. + +[← 06 · denial handling](06-denied-handling.md) · [chapter index](README.md) · [next: meta-annotations →](08-meta-annotations.md) diff --git a/method-security/docs/08-meta-annotations.md b/method-security/docs/08-meta-annotations.md new file mode 100644 index 0000000..9b1cba0 --- /dev/null +++ b/method-security/docs/08-meta-annotations.md @@ -0,0 +1,99 @@ +[← 07 · ordering and transactions](07-ordering-and-transactions.md) · [chapter index](README.md) · [next: the audit checklist →](09-audit-checklist.md) + +# 08 · Meta-annotations, templates, class-level rules, ambiguity + +Run: `java -cp target/classes:$(cat cp.txt) com.ankurm.methodsec.Demo8MetaAnnotations` +Output: [`output/demo8.txt`](output/demo8.txt) +Source: [`Demo8MetaAnnotations.java`](../src/main/java/com/ankurm/methodsec/Demo8MetaAnnotations.java) + +## Naming a rule + +```java +@Target({ ElementType.METHOD, ElementType.TYPE }) +@Retention(RetentionPolicy.RUNTIME) +@PreAuthorize("hasRole('ADMIN')") +public @interface IsAdmin { } +``` + +`@IsAdmin` now works anywhere `@PreAuthorize("hasRole('ADMIN')")` would, with no configuration. +The value is not brevity — it is that the rule exists in one place, so changing it is an edit +rather than a grep. + +## Templated meta-annotations, and a documentation correction + +```java +@PreAuthorize("hasRole('{value}')") +public @interface HasRole { String value(); } +``` + +The reference documentation says you must publish an `AnnotationTemplateExpressionDefaults` +bean for `{value}` to be substituted. **In Spring Security 7.1.1 you do not.** +[`output/demo8.txt`](output/demo8.txt) shows `@HasRole("ADMIN")` discriminating correctly +between an admin and a non-admin in a context with no such bean. + +The reason is in `PreAuthorizeExpressionAttributeRegistry`, which initialises its scanner as +`SecurityAnnotationScanners.requireUnique(PreAuthorize.class)` — and that overload constructs +`new AnnotationTemplateExpressionDefaults()` internally. The bean is autowired +`@Autowired(required = false)` and only replaces that default. Its single knob is +`setIgnoreUnknown(false)`, which turns an unrecognised placeholder into an error instead of +leaving it in the expression. That is worth publishing the bean for; making templates work at +all is not. + +One caveat, read from the source rather than executed here: +`SecurityAnnotationScanners.requireUnique` memoises scanners in a static map keyed by +annotation type. Within a single JVM the first configuration for a given annotation type is the +one that sticks — relevant if you run several differently-configured contexts in one test JVM. + +Multi-value templates need the quotes inside the attribute, which is as awkward as it looks: + +```java +@PreAuthorize("hasAnyRole({roles})") +public @interface HasAnyRole { String[] roles(); } + +@HasAnyRole(roles = { "'USER'", "'ADMIN'" }) +``` + +## Class level + +`@PreAuthorize` on the class applies to every method. A method-level `@PreAuthorize` **replaces** +it rather than adding to it — the nearest declaration wins: + +``` +inherited from the class (needs ADMIN) DENIED +method-level overrides it (needs USER) ALLOWED +``` + +Different annotation *types* are ANDed. A class-level `@PreAuthorize` and a method-level +`@PostAuthorize` both have to pass. + +The trap is the override. Adding `@PreAuthorize("hasRole('USER')")` to one method of an +`@PreAuthorize("hasRole('ADMIN')")` class looks like a tightening and is a widening. + +## Two interfaces, two rules + +```java +public interface ReadsAsUser { @PreAuthorize("hasRole('USER')") String read(); } +public interface ReadsAsAdmin { @PreAuthorize("hasRole('ADMIN')") String read(); } +public class Ambiguous implements ReadsAsUser, ReadsAsAdmin { public String read() { ... } } +``` + +Spring Security refuses to pick one: + +``` +AnnotationConfigurationException: Please ensure there is one unique annotation of type +[interface ...PreAuthorize] attributed to public abstract java.lang.String ...ReadsAsUser.read(). +Found 2 competing annotations: [@PreAuthorize("hasRole('USER')"), @PreAuthorize("hasRole('ADMIN')")] +``` + +**Correction to the common description:** this is *not* a startup failure. The context +refreshes, the bean is proxied, and the exception appears the first time the method is called. +Guidance that says "if a class inherits the same annotation from two interfaces, startup fails" +describes a friendlier framework than the one you are running. Verified in +[`output/demo8.txt`](output/demo8.txt). + +The fix is to annotate the implementation method, which is the nearest declaration and wins +outright. The broader lesson is to put authorization rules in one layer and keep them there. +Repeating the same annotation twice on one method is also unsupported — combine with `and`/`or` +inside a single expression, or delegate to a bean. + +[← 07 · ordering and transactions](07-ordering-and-transactions.md) · [chapter index](README.md) · [next: the audit checklist →](09-audit-checklist.md) diff --git a/method-security/docs/09-audit-checklist.md b/method-security/docs/09-audit-checklist.md new file mode 100644 index 0000000..b88aee9 --- /dev/null +++ b/method-security/docs/09-audit-checklist.md @@ -0,0 +1,97 @@ +[← 08 · meta-annotations](08-meta-annotations.md) · [chapter index](README.md) + +# 09 · The audit checklist + +What to run against an existing codebase, roughly in order of how often it finds something. + +## Grep for it + +```bash +# annotations on methods a proxy cannot advise +grep -rnE '@(Pre|Post)(Authorize|Filter)' --include='*.java' -A3 . \ + | grep -E '(private|static|final) .*\(' + +# final classes carrying method security (context will refuse to start, but check anyway) +grep -rlE '@(Pre|Post)Authorize' --include='*.java' . | xargs grep -lE '^public final class' + +# @Secured / JSR-250 in a codebase that never enabled them +grep -rlE '@(Secured|RolesAllowed|PermitAll|DenyAll)' --include='*.java' . >/dev/null \ + && grep -rn 'EnableMethodSecurity' --include='*.java' . + +# @PreFilter reached with a list that may be immutable +grep -rn '@PreFilter' --include='*.java' -A5 . # then check every caller + +# @PostAuthorize on a method that writes +grep -rn '@PostAuthorize' --include='*.java' -B3 . | grep -i 'transactional' +``` + +Self-invocation does not grep well. The signal is a public method with no annotation calling an +annotated method on the same class; an IDE "find usages" on each annotated method, filtered to +its own file, finds them faster than a regex. + +## Check at runtime + +Print the advisor chain for a bean you believe is secured. If the bean has zero advisors, none +of its annotations are doing anything: + +```java +if (bean instanceof Advised advised) { + for (Advisor a : advised.getAdvisors()) { + System.out.println(((Ordered) a).getOrder() + " " + a); + } +} +``` + +Then confirm the build is passing `-parameters` — one reflection call answers it: + +```java +SomeService.class.getMethod("byOwner", String.class).getParameters()[0].isNamePresent() +``` + +`false` means every `#parameterName` expression in the application is comparing against +nothing. See [chapter 02](02-spel-reference.md). + +## Test for it + +The thing worth asserting is the **negative**: that an unauthorised caller is refused. A test +that only checks the happy path passes identically whether or not the annotation is being +applied at all, which makes it worse than no test. `MethodSecurityTrapsTest` in this module is +14 such assertions — +[`src/test/java/com/ankurm/methodsec/MethodSecurityTrapsTest.java`](../src/test/java/com/ankurm/methodsec/MethodSecurityTrapsTest.java), +output in [`output/tests.txt`](output/tests.txt). + +Use `@WithMockUser(roles = "USER")` and assert `AuthorizationDeniedException`. Call the method +through the injected bean, never through `new`. + +## Edge-case index + +Everything this module demonstrates, with the file that proves it: + +| # | Behaviour | Where | +|---|---|---| +| 1 | `@Secured` / JSR-250 inert unless enabled | [01](01-how-method-security-runs.md), [`demo1.txt`](output/demo1.txt) | +| 2 | No `Authentication` gives `AuthenticationCredentialsNotFoundException`, not a denial | [01](01-how-method-security-runs.md), [`demo1.txt`](output/demo1.txt) | +| 3 | `hasAllRoles` / `hasAllAuthorities` exist | [02](02-spel-reference.md), [`demo4.txt`](output/demo4.txt) | +| 4 | `#root.args[0]` does not exist | [02](02-spel-reference.md), [`demo4.txt`](output/demo4.txt) | +| 5 | `#parameterName` needs `-parameters` | [02](02-spel-reference.md), [`demo9-*.txt`](output/) | +| 6 | `setRoleHierarchy` deprecated; `AuthorizationManagerFactory` is the 7.1 knob | [02](02-spel-reference.md), [`demo4.txt`](output/demo4.txt) | +| 7 | Self-invocation bypasses the check | [03](03-self-invocation.md), [`demo2.txt`](output/demo2.txt) | +| 8 | `final` / `static` / `private` methods are not advised | [04](04-non-proxyable-methods.md), [`demo3.txt`](output/demo3.txt) | +| 9 | Package-private methods **are** advised | [04](04-non-proxyable-methods.md), [`demo3.txt`](output/demo3.txt) | +| 10 | `final` class fails at startup | [04](04-non-proxyable-methods.md), [`demo3.txt`](output/demo3.txt) | +| 11 | JDK proxy hides non-interface methods entirely | [04](04-non-proxyable-methods.md), [`demo3.txt`](output/demo3.txt) | +| 12 | `@PreFilter` on an immutable collection is a silent no-op | [05](05-filtering.md), [`demo5.txt`](output/demo5.txt) | +| 13 | `@PreFilter` mutates the caller's own collection | [05](05-filtering.md), [`demo1.txt`](output/demo1.txt) | +| 14 | `@PreFilter` needs `filterTarget` past one argument; rejects arrays | [05](05-filtering.md), [`demo1.txt`](output/demo1.txt), [`demo5.txt`](output/demo5.txt) | +| 15 | `@PostFilter` returns the same instance it filtered | [05](05-filtering.md), [`demo5.txt`](output/demo5.txt) | +| 16 | `Optional` and `Page` are not filterable | [05](05-filtering.md), [`demo5.txt`](output/demo5.txt) | +| 17 | `AuthorizationDeniedException` carries an `AuthorizationResult` | [06](06-denied-handling.md), [`demo7.txt`](output/demo7.txt) | +| 18 | `@AuthorizeReturnObject` cannot secure a record | [06](06-denied-handling.md), [`demo7.txt`](output/demo7.txt) | +| 19 | `AuthorizationProxyFactory` package correction | [06](06-denied-handling.md), [`demo7.txt`](output/demo7.txt) | +| 20 | `@PostAuthorize` sees the already-filtered return value | [07](07-ordering-and-transactions.md), [`demo6.txt`](output/demo6.txt) | +| 21 | A denied `@PostAuthorize` does not roll back by default | [07](07-ordering-and-transactions.md), [`demo6.txt`](output/demo6.txt) | +| 22 | `{value}` templates work without the defaults bean | [08](08-meta-annotations.md), [`demo8.txt`](output/demo8.txt) | +| 23 | Method-level `@PreAuthorize` replaces the class-level one | [08](08-meta-annotations.md), [`demo8.txt`](output/demo8.txt) | +| 24 | Conflicting inherited annotations fail at call time, not startup | [08](08-meta-annotations.md), [`demo8.txt`](output/demo8.txt) | + +[← 08 · meta-annotations](08-meta-annotations.md) · [chapter index](README.md) diff --git a/method-security/docs/README.md b/method-security/docs/README.md new file mode 100644 index 0000000..4ca5a0c --- /dev/null +++ b/method-security/docs/README.md @@ -0,0 +1,19 @@ +# Method security: the chapters + +Companion notes for [Method Security in Spring Security 7](https://ankurm.com/spring-security-7-method-security-proxy-traps/) +on ankurm.com. Read in order, or jump to whichever failure you are currently staring at. + +| # | Chapter | Answers | +|---|---|---| +| 01 | [How method security actually runs](01-how-method-security-runs.md) | What `@EnableMethodSecurity` registers, and what happens between the caller and the method body | +| 02 | [The SpEL reference](02-spel-reference.md) | Everything you can write inside the annotation, evaluated for real | +| 03 | [Self-invocation](03-self-invocation.md) | Silent failure #1, why it happens, and three fixes | +| 04 | [Methods the proxy cannot advise](04-non-proxyable-methods.md) | Silent failure #2: `final`, `static`, `private`, interfaces, final classes | +| 05 | [Filtering and `filterObject`](05-filtering.md) | Silent failure #3: `@PreFilter` on an immutable argument, and which container types work | +| 06 | [Denial: what is thrown, and how to change it](06-denied-handling.md) | `AuthorizationDeniedException`, `@HandleAuthorizationDenied`, `@AuthorizeReturnObject` | +| 07 | [Ordering, and `@PostAuthorize` vs `@Transactional`](07-ordering-and-transactions.md) | Why a denied `@PostAuthorize` does not roll anything back by default | +| 08 | [Meta-annotations and templates](08-meta-annotations.md) | Custom annotations, `{value}` templates, class-level rules, ambiguity | +| 09 | [The audit checklist](09-audit-checklist.md) | What to grep for in an existing codebase, plus the edge-case index | + +Every claim in these chapters has a file under [`output/`](output/) behind it, regenerated by +`scripts/run-all.sh`. diff --git a/method-security/docs/output/demo1.txt b/method-security/docs/output/demo1.txt new file mode 100644 index 0000000..68fd603 --- /dev/null +++ b/method-security/docs/output/demo1.txt @@ -0,0 +1,50 @@ +============================================================================== +Demo 1 -- the four pre/post annotations, @Secured and JSR-250, all switched on +============================================================================== +SLF4J(W): No SLF4J providers were found. +SLF4J(W): Defaulting to no-operation (NOP) logger implementation +SLF4J(W): See https://www.slf4j.org/codes.html#noProviders for further details. + +as alice (ROLE_USER) +-------------------- + @PreAuthorize hasRole('ADMIN') DENIED -> AuthorizationDeniedException: Access Denied + @PreAuthorize #owner == authentication.name ALLOWED -> [Account[1,alice,100], Account[3,alice,300]] + @PreAuthorize #owner == authentication.name DENIED -> AuthorizationDeniedException: Access Denied + @PostAuthorize returnObject.owner == ...name ALLOWED -> Account[1,alice,100] + @PostAuthorize returnObject.owner == ...name DENIED -> AuthorizationDeniedException: Access Denied + @PostFilter filterObject.owner == ...name ALLOWED -> [Account[1,alice,100], Account[3,alice,300]] + @Secured("ROLE_ADMIN") DENIED -> AuthorizationDeniedException: Access Denied + @RolesAllowed("ADMIN") DENIED -> AuthorizationDeniedException: Access Denied + @PermitAll ALLOWED -> open payload + @DenyAll DENIED -> AuthorizationDeniedException: Access Denied + +as root (ROLE_ADMIN, ROLE_USER) +------------------------------- + @PreAuthorize hasRole('ADMIN') ALLOWED -> the admin console + @Secured("ROLE_ADMIN") ALLOWED -> secured payload + @RolesAllowed("ADMIN") ALLOWED -> jsr250 payload + @PostFilter filterObject.owner == ...name ALLOWED -> [] + +with no Authentication at all (SecurityContextHolder cleared) +------------------------------------------------------------- + @PreAuthorize hasRole('ADMIN') DENIED -> AuthenticationCredentialsNotFoundException: An Authentication object was not found in the SecurityContext + @PermitAll ALLOWED -> open payload + +@PreFilter -- filtering the ARGUMENT, as alice +---------------------------------------------- + caller's list before the call : [Account[1,alice,100], Account[2,bob,200], Account[3,alice,300]] + method body saw : [Account[1,alice,150], Account[3,alice,350]] + caller's list after the call : [Account[1,alice,150], Account[3,alice,350]] + + @PreFilter did not hand the method a copy. It removed bob's account from + the caller's own list, in place, before the method body ever ran. That is + why Demo 5's immutable List.of(..) blows up. + +@PreFilter on a method with more than one argument +-------------------------------------------------- + no filterTarget, 2 args DENIED -> IllegalStateException: Unable to determine the method argument for filtering. Specify the filter target. + method body saw : [Account[4,alice,60]] + filterTarget = "accounts" ALLOWED -> (void) + + This one is loud, not silent -- but it only fires when the method is + actually called, so a rarely-exercised path can ship broken. diff --git a/method-security/docs/output/demo2.txt b/method-security/docs/output/demo2.txt new file mode 100644 index 0000000..b578b11 --- /dev/null +++ b/method-security/docs/output/demo2.txt @@ -0,0 +1,27 @@ +============================================================================== +Demo 2 -- self-invocation: the annotation is there, the check is not +============================================================================== +SLF4J(W): No SLF4J providers were found. +SLF4J(W): Defaulting to no-operation (NOP) logger implementation +SLF4J(W): See https://www.slf4j.org/codes.html#noProviders for further details. + +alice has ROLE_USER only. adminReport() requires ROLE_ADMIN. +------------------------------------------------------------ + reports.adminReport() (via proxy) DENIED -> AuthorizationDeniedException: Access Denied + reports.userEntryPoint() (this.adminReport()) ALLOWED -> TOP SECRET REVENUE NUMBERS + +Is the annotation actually there? (reflection on the target class) +------------------------------------------------------------------ + ReportService.adminReport() @PreAuthorize -> @org.springframework.security.access.prepost.PreAuthorize("hasRole('ADMIN')") + bean is an AOP proxy -> true + proxy class -> com.ankurm.methodsec.Demo2SelfInvocation$ReportService$$SpringCGLIB$$0 + target class -> com.ankurm.methodsec.Demo2SelfInvocation$ReportService + + The annotation is present, the bean IS proxied, and the call was still + not checked. The proxy only sees calls that arrive from outside. + +Three ways to make the inner call go through the proxy +------------------------------------------------------ + self-injection (ObjectProvider) DENIED -> AuthorizationDeniedException: Access Denied + AopContext.currentProxy() DENIED -> AuthorizationDeniedException: Access Denied + call a different bean (collaborator) DENIED -> AuthorizationDeniedException: Access Denied diff --git a/method-security/docs/output/demo3.txt b/method-security/docs/output/demo3.txt new file mode 100644 index 0000000..0c01464 --- /dev/null +++ b/method-security/docs/output/demo3.txt @@ -0,0 +1,37 @@ +============================================================================== +Demo 3 -- @PreAuthorize on methods the proxy cannot override +============================================================================== +SLF4J(W): No SLF4J providers were found. +SLF4J(W): Defaulting to no-operation (NOP) logger implementation +SLF4J(W): See https://www.slf4j.org/codes.html#noProviders for further details. + +alice has ROLE_USER. Every method below says hasRole('ADMIN'). +-------------------------------------------------------------- + public (overridable) DENIED -> AuthorizationDeniedException: Access Denied + public final (NOT overridable) ALLOWED -> final payload + static (NOT overridable) ALLOWED -> static payload + package-private (overridable, same package) DENIED -> AuthorizationDeniedException: Access Denied + private, reached via a public wrapper ALLOWED -> private payload + +What the proxy actually overrode +-------------------------------- + publicAdminOnly declared final=false overridden by proxy=true + finalAdminOnly declared final=true overridden by proxy=false + packagePrivateAdminOnly declared final=false overridden by proxy=true + proxy class -> com.ankurm.methodsec.Demo3NonProxyable$Vault$$SpringCGLIB$$0 + +A JDK dynamic proxy only advises methods that are ON the interface +------------------------------------------------------------------ + proxy is a JDK proxy -> true + proxied interfaces -> [interface com.ankurm.methodsec.Demo3NonProxyable$LedgerOperations] + onTheInterface() (advised) DENIED -> AuthorizationDeniedException: Access Denied + notOnTheInterface() is public and annotated, but the JDK proxy does not + implement it at all -- a caller cannot even reach it without casting to + the implementation class, and that cast throws ClassCastException. + cast proxy to Ledger impl class DENIED -> ClassCastException: class jdk.proxy2.$Proxy18 cannot be cast to class com.ankurm.methodsec.Demo3NonProxyable$Ledger (jdk.proxy2.$Proxy18 is in module jdk.proxy2 of loader 'app'; com.ankurm.methodsec.Demo3NonProxyable$Ledger is in unnamed module of loader 'app') + +A final CLASS is the loud one +----------------------------- + startup FAILED -> BeanCreationException + root cause -> java.lang.IllegalArgumentException + message -> Cannot subclass final class com.ankurm.methodsec.Demo3NonProxyable$SealedVault diff --git a/method-security/docs/output/demo4.txt b/method-security/docs/output/demo4.txt new file mode 100644 index 0000000..dc4bea9 --- /dev/null +++ b/method-security/docs/output/demo4.txt @@ -0,0 +1,69 @@ +============================================================================== +Demo 4 -- what you can actually write inside @PreAuthorize / @PostAuthorize +============================================================================== +SLF4J(W): No SLF4J providers were found. +SLF4J(W): Defaulting to no-operation (NOP) logger implementation +SLF4J(W): See https://www.slf4j.org/codes.html#noProviders for further details. + +alice: ROLE_USER, ROLE_AUDITOR, plus the authority 'report:read' +---------------------------------------------------------------- + permitAll ALLOWED -> ok + denyAll DENIED -> AuthorizationDeniedException: Access Denied + isAuthenticated() ALLOWED -> ok + isAnonymous() DENIED -> AuthorizationDeniedException: Access Denied + isFullyAuthenticated() ALLOWED -> ok + isRememberMe() DENIED -> AuthorizationDeniedException: Access Denied + hasRole('ADMIN') DENIED -> AuthorizationDeniedException: Access Denied + hasAnyRole('ADMIN','AUDITOR') ALLOWED -> ok + hasAllRoles('USER','AUDITOR') ALLOWED -> ok + hasAuthority('report:read') ALLOWED -> ok + hasAnyAuthority('report:read','x') ALLOWED -> ok + hasAllAuthorities('report:read','x') DENIED -> AuthorizationDeniedException: Access Denied + authentication.name == 'alice' ALLOWED -> ok + principal == 'alice' ALLOWED -> ok + +root: ROLE_ADMIN only (RoleHierarchy says ADMIN > USER > GUEST) +--------------------------------------------------------------- + permitAll ALLOWED -> ok + denyAll DENIED -> AuthorizationDeniedException: Access Denied + isAuthenticated() ALLOWED -> ok + isAnonymous() DENIED -> AuthorizationDeniedException: Access Denied + isFullyAuthenticated() ALLOWED -> ok + isRememberMe() DENIED -> AuthorizationDeniedException: Access Denied + hasRole('ADMIN') ALLOWED -> ok + hasAnyRole('ADMIN','AUDITOR') ALLOWED -> ok + hasAllRoles('USER','AUDITOR') DENIED -> AuthorizationDeniedException: Access Denied + hasAuthority('report:read') DENIED -> AuthorizationDeniedException: Access Denied + hasAnyAuthority('report:read','x') DENIED -> AuthorizationDeniedException: Access Denied + hasAllAuthorities('report:read','x') DENIED -> AuthorizationDeniedException: Access Denied + authentication.name == 'alice' DENIED -> AuthorizationDeniedException: Access Denied + principal == 'alice' DENIED -> AuthorizationDeniedException: Access Denied + +Role prefix and hierarchy +------------------------- + hasRole('USER') -> ROLE_USER ALLOWED -> ok + hasAuthority('USER') -> literal 'USER' DENIED -> AuthorizationDeniedException: Access Denied + hasAuthority('ROLE_USER') ALLOWED -> ok + root hasRole('GUEST') via RoleHierarchy ALLOWED -> ok + +Method arguments, the return value, and bean references +------------------------------------------------------- + #owner == authentication.name ("alice") ALLOWED -> ok + #owner == authentication.name ("bob") DENIED -> AuthorizationDeniedException: Access Denied + @P("o") alias, #o == ...name ("alice") ALLOWED -> ok + #root.this (the target object) ALLOWED -> ok + #root.args[0] -- no such property DENIED -> IllegalArgumentException: Failed to evaluate expression '#root.args[0] == authentication.name' [cause: SpelEvaluationException: EL1008E: Property or field 'args' cannot be found on object of type 'org.springframework.security.access.expression.method.MethodSecurityExpressionRoot' - maybe not public or not valid?] + @policy.canRead(authentication, #id) id=1 ALLOWED -> ok + @policy.canRead(authentication, #id) id=9 DENIED -> AuthorizationDeniedException: Access Denied + hasPermission(#id, 'account', 'read') id=1 ALLOWED -> ok + hasPermission(#id, 'account', 'read') id=9 DENIED -> AuthorizationDeniedException: Access Denied + T(java.time.LocalDate) type reference ALLOWED -> ok + @PostAuthorize returnObject.owner == ...name ALLOWED -> Account[1,alice,100] + @PostAuthorize returnObject.owner == ...name DENIED -> AuthorizationDeniedException: Access Denied + +The literal constants on SecurityExpressionRoot +----------------------------------------------- + permitAll / denyAll exist as BOTH a boolean field and a no-arg method, + and read/write/create/delete/admin are String constants meant for + hasPermission(..) -- e.g. hasPermission(#id, 'account', read). + hasPermission(#id, 'account', read) id=1 ALLOWED -> ok diff --git a/method-security/docs/output/demo5.txt b/method-security/docs/output/demo5.txt new file mode 100644 index 0000000..54eb7a8 --- /dev/null +++ b/method-security/docs/output/demo5.txt @@ -0,0 +1,58 @@ +============================================================================== +Demo 5 -- filterObject: mutability, container types, and the silent no-op +============================================================================== +SLF4J(W): No SLF4J providers were found. +SLF4J(W): Defaulting to no-operation (NOP) logger implementation +SLF4J(W): See https://www.slf4j.org/codes.html#noProviders for further details. + +@PreFilter needs a MUTABLE argument -- and does not tell you when it is not +--------------------------------------------------------------------------- + method body saw: [Account[1,alice,100], Account[3,alice,300]] + new ArrayList<>(..) ALLOWED -> (void) + method body saw: [Account[1,alice,100], Account[2,bob,200], Account[3,alice,300]] + List.of(..) (immutable) ALLOWED -> (void) + method body saw: [Account[1,alice,100], Account[2,bob,200], Account[3,alice,300]] + List.copyOf(..) (immutable) ALLOWED -> (void) + method body saw: [Account[1,alice,100], Account[2,bob,200], Account[3,alice,300]] + Arrays.asList(..) ALLOWED -> (void) + method body saw: [Account[1,alice,100], Account[2,bob,200], Account[3,alice,300]] + Collections.unmodifiableList(..) ALLOWED -> (void) + method body saw: [Account[1,alice,100], Account[2,bob,200], Account[3,alice,300]] + stream().toList() (unmodifiable since 16) ALLOWED -> (void) + method body saw: [Account[1,alice,100], Account[3,alice,300]] + stream().collect(toList()) (ArrayList) ALLOWED -> (void) + Account[] (arrays rejected outright) DENIED -> IllegalStateException: Pre-filtering on array types is not supported. Using a Collection will solve this problem. + + Read the second and third lines again: bob's account reached the method + body. @PreFilter filters by CLEARING the caller's collection and adding + the survivors back. On an immutable list that throws, and + DefaultMethodSecurityExpressionHandler.filterCollection catches the + UnsupportedOperationException and returns a fresh list instead -- which + PreFilterAuthorizationMethodInterceptor.invoke then discards, because it + ignores filter()'s return value entirely. No exception, no WARN, no 403. + +The only trace it leaves (same call, logger at TRACE) +----------------------------------------------------- + method body saw: [Account[1,alice,100], Account[2,bob,200], Account[3,alice,300]] + List.of(..) with TRACE on ALLOWED -> (void) + +What @PostFilter accepts as a return type +----------------------------------------- + List ALLOWED -> [Account[1,alice,100], Account[3,alice,300]] + Account[] ALLOWED -> [Account[1,alice,100], Account[3,alice,300]] + Stream (collected here) ALLOWED -> [alice, alice] + Map ALLOWED -> {acct-1=Account[1,alice,100], acct-3=Account[3,alice,300]} + Optional (alice's) DENIED -> IllegalArgumentException: Filter target must be a collection, array, map or stream type, but was Optional[Account[1,alice,100]] + Optional (bob's) DENIED -> IllegalArgumentException: Filter target must be a collection, array, map or stream type, but was Optional[Account[2,bob,200]] + List.of(..) (immutable return) ALLOWED -> [Account[1,alice,100], Account[3,alice,300]] + Ledger (a type Spring Security does not know) DENIED -> IllegalArgumentException: Filter target must be a collection, array, map or stream type, but was Ledger[Account[1,alice,100], Account[2,bob,200], Account[3,alice,300]] + Page (real Spring Data PageImpl) DENIED -> IllegalArgumentException: Filter target must be a collection, array, map or stream type, but was Page 1 of 1 containing com.ankurm.methodsec.Account instances + +Identity: does @PostFilter hand back the same object? +----------------------------------------------------- + returned == the list the method returned : true + the method's own list, after filtering : [Account[1,alice,100], Account[3,alice,300]] + + @PostFilter mutates the returned collection in place and hands the same + reference back. If that collection is a cached or shared instance, you + have just deleted rows from it for every future caller. diff --git a/method-security/docs/output/demo6.txt b/method-security/docs/output/demo6.txt new file mode 100644 index 0000000..daa0466 --- /dev/null +++ b/method-security/docs/output/demo6.txt @@ -0,0 +1,58 @@ +============================================================================== +Demo 6 -- interceptor order, and @PostAuthorize vs @Transactional +============================================================================== + +AuthorizationInterceptorsOrder, read from the enum itself +--------------------------------------------------------- + CONSTANT getOrder() + FIRST -2147483648 + PRE_FILTER 100 + PRE_AUTHORIZE 200 + SECURED 300 + JSR250 400 + SECURE_RESULT 450 + POST_AUTHORIZE 500 + POST_FILTER 600 + LAST 2147483647 + + Lower order = higher precedence = further OUT in the chain. Spring's own + @Transactional advisor defaults to Ordered.LOWEST_PRECEDENCE (2147483647), + which is larger than every number above -- so security wraps transactions, + not the other way round. +SLF4J(W): No SLF4J providers were found. +SLF4J(W): Defaulting to no-operation (NOP) logger implementation +SLF4J(W): See https://www.slf4j.org/codes.html#noProviders for further details. + +The advisor chain on a bean carrying all four annotations +--------------------------------------------------------- + advisors applied to the proxy: 4 + ORDER ADVISOR BEAN (as registered by @EnableMethodSecurity) + 100 preFilterAuthorizationMethodInterceptor + 200 preAuthorizeAuthorizationMethodInterceptor + 450 authorizeReturnObjectMethodInterceptor + 500 postAuthorizeAuthorizationMethodInterceptor + 600 postFilterAuthorizationMethodInterceptor + (authorizeReturnObject sits at SECURE_RESULT = 450 and is registered + whether or not anything in the app uses @AuthorizeReturnObject.) + + For BEFORE advice, a lower order runs earlier: @PreFilter (100) really + does run before @PreAuthorize (200). For AFTER advice the same numbers + mean the opposite. @PostAuthorize (500) sits FURTHER OUT than + @PostFilter (600), so on the way back out @PostFilter finishes first + and @PostAuthorize evaluates returnObject on the ALREADY-FILTERED list. + + Both methods below return the same 3 elements and filter one away: + @PostAuthorize returnObject.size() == 3 DENIED -> AuthorizationDeniedException: Access Denied + @PostAuthorize returnObject.size() == 2 ALLOWED -> [a, b] + +Default order: @PostAuthorize denies AFTER the transaction commits +------------------------------------------------------------------ + rows before : 0 + recordAndReturn("bob") @PostAuthorize DENIED -> AuthorizationDeniedException: Access Denied + rows after the denial : 1 + +@EnableTransactionManagement(order = FIRST): the write rolls back +----------------------------------------------------------------- + rows before : 0 + recordAndReturn("bob") @PostAuthorize DENIED -> AuthorizationDeniedException: Access Denied + rows after the denial : 0 diff --git a/method-security/docs/output/demo7.txt b/method-security/docs/output/demo7.txt new file mode 100644 index 0000000..95c4460 --- /dev/null +++ b/method-security/docs/output/demo7.txt @@ -0,0 +1,34 @@ +============================================================================== +Demo 7 -- @HandleAuthorizationDenied and @AuthorizeReturnObject +============================================================================== +SLF4J(W): No SLF4J providers were found. +SLF4J(W): Defaulting to no-operation (NOP) logger implementation +SLF4J(W): See https://www.slf4j.org/codes.html#noProviders for further details. + +The exception type you actually catch +------------------------------------- + thrown -> org.springframework.security.authorization.AuthorizationDeniedException + is AccessDeniedException -> true + is AuthorizationDeniedException -> true + carries an AuthorizationResult -> ExpressionAuthorizationDecision granted=false + + Handlers written against AccessDeniedException still work -- but the + concrete type carries the AuthorizationResult that explains the denial. + +@HandleAuthorizationDenied: return something instead of throwing +---------------------------------------------------------------- + maskedBalance() (alice, no ROLE_FINANCE) ALLOWED -> ***masked*** + maskedList() (alice, no ROLE_FINANCE) ALLOWED -> [] + maskedBalance() (cfo, has ROLE_FINANCE) ALLOWED -> 1,204,993.22 + +@AuthorizeReturnObject: the check moves onto the returned object +---------------------------------------------------------------- + returned instance -> com.ankurm.methodsec.Demo7DeniedHandling$Customer$$SpringCGLIB$$0 + customer.getName() (no authority needed) ALLOWED -> alice + customer.getEmail() (needs 'pii:read') DENIED -> AuthorizationDeniedException: Access Denied + customer.getEmail() (has 'pii:read') ALLOWED -> alice@example.com + +Same thing without the annotation, via AuthorizationProxyFactory +---------------------------------------------------------------- + raw.getEmail() (unproxied object) ALLOWED -> alice@example.com + wrapped.getEmail() (proxied object) DENIED -> AuthorizationDeniedException: Access Denied diff --git a/method-security/docs/output/demo8.txt b/method-security/docs/output/demo8.txt new file mode 100644 index 0000000..0240e26 --- /dev/null +++ b/method-security/docs/output/demo8.txt @@ -0,0 +1,47 @@ +============================================================================== +Demo 8 -- meta-annotations, templates, class-level rules, ambiguity +============================================================================== + +A plain meta-annotation needs no extra configuration +---------------------------------------------------- +SLF4J(W): No SLF4J providers were found. +SLF4J(W): Defaulting to no-operation (NOP) logger implementation +SLF4J(W): See https://www.slf4j.org/codes.html#noProviders for further details. + @IsAdmin (alice, ROLE_USER) DENIED -> AuthorizationDeniedException: Access Denied + @IsAdmin (root, ROLE_ADMIN) ALLOWED -> ok + +A TEMPLATED meta-annotation, with NO AnnotationTemplateExpressionDefaults bean +------------------------------------------------------------------------------ + @HasRole("ADMIN") as root (ROLE_ADMIN) ALLOWED -> ok + @HasRole("ADMIN") as alice (ROLE_USER) DENIED -> AuthorizationDeniedException: Access Denied + + '{value}' was substituted anyway. The reference documentation says you + must publish an AnnotationTemplateExpressionDefaults bean for templated + meta-annotations to work; in 7.1.1 you do not. + PreAuthorizeExpressionAttributeRegistry initialises its scanner with + SecurityAnnotationScanners.requireUnique(PreAuthorize.class), and that + overload constructs a default AnnotationTemplateExpressionDefaults for + you. Publishing the bean only changes ignoreUnknown. + +The same annotation WITH the AnnotationTemplateExpressionDefaults bean +---------------------------------------------------------------------- + @HasRole("ADMIN") as root (ROLE_ADMIN) ALLOWED -> ok + @HasRole("ADMIN") as alice (ROLE_USER) DENIED -> AuthorizationDeniedException: Access Denied + @HasRole("USER") as alice (ROLE_USER) ALLOWED -> ok + +Class-level rules, and what a method-level one does to them +----------------------------------------------------------- + inherited from the class (needs ADMIN) DENIED -> AuthorizationDeniedException: Access Denied + method-level overrides it (needs USER) ALLOWED -> ok + class @PreAuthorize AND method @PostAuthorize DENIED -> AuthorizationDeniedException: Access Denied + +Two interfaces, two different @PreAuthorize on the same method +-------------------------------------------------------------- + context started fine. + bean type -> jdk.proxy2.$Proxy21 + read() -- inherits two conflicting rules DENIED -> AnnotationConfigurationException: Please ensure there is one unique annotation of type [interface org.springframework.security.access.prepost.PreAuthorize] attributed to public abstract java.lang.String com.ankurm.methodsec.Demo8MetaAnnotations$ReadsAsUser.read(). Found 2 competing annotations: [@org.springframework.security.access.prepost.PreAuthorize("hasRole('USER')"), @org.springframework.security.access.prepost.PreAuthorize("hasRole('ADMIN')")] + + It is not a startup failure: the context refreshes, the bean is + proxied, and the conflict only surfaces when the method is called. + The fix is to put @PreAuthorize on the implementation method, which + is the nearest declaration and therefore wins outright. diff --git a/method-security/docs/output/demo9-with-parameters.txt b/method-security/docs/output/demo9-with-parameters.txt new file mode 100644 index 0000000..05a1407 --- /dev/null +++ b/method-security/docs/output/demo9-with-parameters.txt @@ -0,0 +1,24 @@ +============================================================================== +Demo 9 -- #parameterName and the -parameters compiler flag +============================================================================== + compiled with -parameters : true + byParameterName param[0] : owner + byParameterAlias param[0] : owner (annotated @P("o")) +SLF4J(W): No SLF4J providers were found. +SLF4J(W): Defaulting to no-operation (NOP) logger implementation +SLF4J(W): See https://www.slf4j.org/codes.html#noProviders for further details. + +alice calling with her own name +------------------------------- + #owner == authentication.name ALLOWED -> ok + #o == authentication.name (@P("o")) ALLOWED -> ok + +alice calling with somebody else's name +--------------------------------------- + #owner == authentication.name DENIED -> AuthorizationDeniedException: Access Denied + #o == authentication.name (@P("o")) DENIED -> AuthorizationDeniedException: Access Denied + + Without -parameters the first expression denies BOTH calls -- it fails + closed, which is the good direction, but it fails silently in the sense + that nothing tells you the rule is not the rule you wrote. @P("o") does + not depend on the flag, because the name is in the class file either way. diff --git a/method-security/docs/output/demo9-without-parameters.txt b/method-security/docs/output/demo9-without-parameters.txt new file mode 100644 index 0000000..83161a0 --- /dev/null +++ b/method-security/docs/output/demo9-without-parameters.txt @@ -0,0 +1,24 @@ +============================================================================== +Demo 9 -- #parameterName and the -parameters compiler flag +============================================================================== + compiled with -parameters : false + byParameterName param[0] : arg0 + byParameterAlias param[0] : arg0 (annotated @P("o")) +SLF4J(W): No SLF4J providers were found. +SLF4J(W): Defaulting to no-operation (NOP) logger implementation +SLF4J(W): See https://www.slf4j.org/codes.html#noProviders for further details. + +alice calling with her own name +------------------------------- + #owner == authentication.name DENIED -> AuthorizationDeniedException: Access Denied + #o == authentication.name (@P("o")) ALLOWED -> ok + +alice calling with somebody else's name +--------------------------------------- + #owner == authentication.name DENIED -> AuthorizationDeniedException: Access Denied + #o == authentication.name (@P("o")) DENIED -> AuthorizationDeniedException: Access Denied + + Without -parameters the first expression denies BOTH calls -- it fails + closed, which is the good direction, but it fails silently in the sense + that nothing tells you the rule is not the rule you wrote. @P("o") does + not depend on the flag, because the name is in the class file either way. diff --git a/method-security/docs/output/tests.txt b/method-security/docs/output/tests.txt new file mode 100644 index 0000000..c18bff5 --- /dev/null +++ b/method-security/docs/output/tests.txt @@ -0,0 +1,6 @@ +mvn test -- MethodSecurityTrapsTest (14 tests pinning every claim the demos print) + +------------------------------------------------------------------------------- +Test set: com.ankurm.methodsec.MethodSecurityTrapsTest +------------------------------------------------------------------------------- +Tests run: 14, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 1.076 s -- in com.ankurm.methodsec.MethodSecurityTrapsTest diff --git a/method-security/pom.xml b/method-security/pom.xml new file mode 100644 index 0000000..e9a4abd --- /dev/null +++ b/method-security/pom.xml @@ -0,0 +1,119 @@ + + 4.0.0 + com.ankurm + method-security-verify + 1.0 + jar + + + 25 + UTF-8 + + + + + org.springframework.security + spring-security-core + 7.1.1 + + + org.springframework.security + spring-security-config + 7.1.1 + + + org.springframework + spring-context + 7.0.9 + + + org.springframework + spring-aop + 7.0.9 + + + org.springframework + spring-tx + 7.0.9 + + + org.springframework + spring-jdbc + 7.0.9 + + + + org.aspectj + aspectjweaver + 1.9.25 + + + com.h2database + h2 + 2.4.240 + + + + org.springframework.data + spring-data-commons + 4.1.1 + + + jakarta.annotation + jakarta.annotation-api + 3.0.0 + + + + org.springframework + spring-test + 7.0.9 + test + + + org.springframework.security + spring-security-test + 7.1.1 + test + + + org.junit.jupiter + junit-jupiter + 6.0.3 + test + + + org.assertj + assertj-core + 3.27.7 + test + + + + + + + org.apache.maven.plugins + maven-compiler-plugin + 3.13.0 + + 25 + + + -parameters + + + + + org.apache.maven.plugins + maven-surefire-plugin + 3.5.2 + + + + diff --git a/method-security/scripts/run-all.sh b/method-security/scripts/run-all.sh new file mode 100755 index 0000000..eb887ed --- /dev/null +++ b/method-security/scripts/run-all.sh @@ -0,0 +1,50 @@ +#!/usr/bin/env bash +# Compiles this module twice -- once with -parameters and once without -- runs all nine demos +# plus the JUnit suite, and regenerates every file in docs/output/. +# +# Requires JDK 25 (no preview features needed; the sibling context-propagation module does). +set -euo pipefail +cd "$(dirname "$0")/.." + +mvn -q dependency:build-classpath -Dmdep.outputFile=cp.txt +CP=$(cat cp.txt) + +rm -rf target/classes target/classes-noparams +mkdir -p target/classes target/classes-noparams docs/output + +# -parameters is what makes #owner resolve to "owner" instead of "arg0". Demo 9 is run from +# both builds so the difference is a diff, not a claim. +javac --release 25 -parameters -cp "$CP" -d target/classes $(find src/main -name '*.java') +javac --release 25 -cp "$CP" -d target/classes-noparams $(find src/main -name '*.java') + +run() { # run [classes-dir] + local demo="$1" out="docs/output/$2" dir="${3:-target/classes}" + echo "Running $demo -> $out" + # stderr is kept: Spring's own CGLIB warning about final methods is part of the evidence. + java -cp "$dir:$CP" "com.ankurm.methodsec.$demo" 2>&1 \ + | grep -v '^Picked up JAVA_TOOL_OPTIONS' > "$out" +} + +run Demo1AnnotationsInAction demo1.txt +run Demo2SelfInvocation demo2.txt +run Demo3NonProxyable demo3.txt +run Demo4SpelReference demo4.txt +run Demo5FilteringTraps demo5.txt +run Demo6InterceptorOrder demo6.txt +run Demo7DeniedHandling demo7.txt +run Demo8MetaAnnotations demo8.txt +run Demo9ParameterNames demo9-with-parameters.txt target/classes +run Demo9ParameterNames demo9-without-parameters.txt target/classes-noparams + +echo "Running JUnit trap suite -> docs/output/tests.txt" +mvn -q test > /tmp/ms-mvn-test.txt 2>&1 || true +{ + echo "mvn test -- MethodSecurityTrapsTest (14 tests pinning every claim the demos print)" + echo + cat target/surefire-reports/com.ankurm.methodsec.MethodSecurityTrapsTest.txt +} > docs/output/tests.txt +rm -f /tmp/ms-mvn-test.txt + +echo +echo "Regenerated:" +ls -1 docs/output/ diff --git a/method-security/src/main/java/com/ankurm/methodsec/Account.java b/method-security/src/main/java/com/ankurm/methodsec/Account.java new file mode 100644 index 0000000..d1a77dd --- /dev/null +++ b/method-security/src/main/java/com/ankurm/methodsec/Account.java @@ -0,0 +1,51 @@ +package com.ankurm.methodsec; + +/** + * The one domain object every demo in this module operates on. + * + *

{@code owner} is deliberately a plain {@code String} that matches + * {@code authentication.getName()}, because that is what makes expressions like + * {@code returnObject.owner == authentication.name} readable in the SpEL reference + * (docs/02-spel-reference.md). + * + *

Not a record: {@link Demo7DeniedHandling} needs a CGLIB-proxyable, non-final class for + * {@code @AuthorizeReturnObject}, and records are final. That restriction is itself one of the + * findings -- see + * docs/06-denied-handling.md. + */ +public class Account { + + private final long id; + + private final String owner; + + private long balanceMinor; + + public Account(long id, String owner, long balanceMinor) { + this.id = id; + this.owner = owner; + this.balanceMinor = balanceMinor; + } + + public long getId() { + return this.id; + } + + public String getOwner() { + return this.owner; + } + + public long getBalanceMinor() { + return this.balanceMinor; + } + + public void setBalanceMinor(long balanceMinor) { + this.balanceMinor = balanceMinor; + } + + @Override + public String toString() { + return "Account[" + this.id + "," + this.owner + "," + this.balanceMinor + "]"; + } + +} diff --git a/method-security/src/main/java/com/ankurm/methodsec/Demo1AnnotationsInAction.java b/method-security/src/main/java/com/ankurm/methodsec/Demo1AnnotationsInAction.java new file mode 100644 index 0000000..efd8ae9 --- /dev/null +++ b/method-security/src/main/java/com/ankurm/methodsec/Demo1AnnotationsInAction.java @@ -0,0 +1,174 @@ +package com.ankurm.methodsec; + +import java.util.ArrayList; +import java.util.List; + +import jakarta.annotation.security.DenyAll; +import jakarta.annotation.security.PermitAll; +import jakarta.annotation.security.RolesAllowed; + +import org.springframework.context.annotation.AnnotationConfigApplicationContext; +import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.Configuration; +import org.springframework.security.access.annotation.Secured; +import org.springframework.security.access.prepost.PostAuthorize; +import org.springframework.security.access.prepost.PostFilter; +import org.springframework.security.access.prepost.PreAuthorize; +import org.springframework.security.access.prepost.PreFilter; +import org.springframework.security.config.annotation.method.configuration.EnableMethodSecurity; + +/** + * Demo 1 -- every method-security annotation, on one service, against a real Spring context. + * + *

The point of this demo is to be boring: it establishes what the happy path looks like so + * the later demos can be about the ways it silently does not happen. Chapter: + * docs/01-how-method-security-runs.md. + * + *

Note {@code securedEnabled} and {@code jsr250Enabled} are {@code false} by default on + * {@link EnableMethodSecurity} -- verified by reading the {@code AnnotationDefault} attributes + * out of {@code spring-security-config-7.1.1.jar}, not from prose. {@code @Secured} and + * {@code @RolesAllowed} are therefore inert unless you switch them on, which is silent failure + * number zero. + */ +public class Demo1AnnotationsInAction { + + public static void main(String[] args) { + Support.banner("Demo 1 -- the four pre/post annotations, @Secured and JSR-250, all switched on"); + + try (var ctx = new AnnotationConfigApplicationContext(Config.class)) { + BankService bank = ctx.getBean(BankService.class); + + Support.heading("as alice (ROLE_USER)"); + Support.login("alice", "ROLE_USER"); + Support.attempt("@PreAuthorize hasRole('ADMIN')", () -> bank.adminOnly()); + Support.attempt("@PreAuthorize #owner == authentication.name", () -> bank.accountsOf("alice")); + Support.attempt("@PreAuthorize #owner == authentication.name", () -> bank.accountsOf("bob")); + Support.attempt("@PostAuthorize returnObject.owner == ...name", () -> bank.readAccount(1)); + Support.attempt("@PostAuthorize returnObject.owner == ...name", () -> bank.readAccount(2)); + Support.attempt("@PostFilter filterObject.owner == ...name", () -> bank.allAccounts()); + Support.attempt("@Secured(\"ROLE_ADMIN\")", () -> bank.securedAdminOnly()); + Support.attempt("@RolesAllowed(\"ADMIN\")", () -> bank.jsr250AdminOnly()); + Support.attempt("@PermitAll", () -> bank.jsr250Open()); + Support.attempt("@DenyAll", () -> bank.jsr250Closed()); + + Support.heading("as root (ROLE_ADMIN, ROLE_USER)"); + Support.login("root", "ROLE_ADMIN", "ROLE_USER"); + Support.attempt("@PreAuthorize hasRole('ADMIN')", () -> bank.adminOnly()); + Support.attempt("@Secured(\"ROLE_ADMIN\")", () -> bank.securedAdminOnly()); + Support.attempt("@RolesAllowed(\"ADMIN\")", () -> bank.jsr250AdminOnly()); + Support.attempt("@PostFilter filterObject.owner == ...name", () -> bank.allAccounts()); + + Support.heading("with no Authentication at all (SecurityContextHolder cleared)"); + Support.logout(); + Support.attempt("@PreAuthorize hasRole('ADMIN')", () -> bank.adminOnly()); + Support.attempt("@PermitAll", () -> bank.jsr250Open()); + + Support.heading("@PreFilter -- filtering the ARGUMENT, as alice"); + Support.login("alice", "ROLE_USER"); + List batch = new ArrayList<>(List.of(new Account(1, "alice", 100), + new Account(2, "bob", 200), new Account(3, "alice", 300))); + System.out.println(" caller's list before the call : " + batch); + bank.deposit(batch); + System.out.println(" caller's list after the call : " + batch); + System.out.println(); + System.out.println(" @PreFilter did not hand the method a copy. It removed bob's account from"); + System.out.println(" the caller's own list, in place, before the method body ever ran. That is"); + System.out.println(" why Demo 5's immutable List.of(..) blows up."); + + Support.heading("@PreFilter on a method with more than one argument"); + List two = new ArrayList<>(List.of(new Account(4, "alice", 10), new Account(5, "bob", 20))); + Support.attemptVoid("no filterTarget, 2 args", () -> bank.depositAmbiguous(two, 50)); + Support.attemptVoid("filterTarget = \"accounts\"", () -> bank.depositDisambiguated(two, 50)); + System.out.println(); + System.out.println(" This one is loud, not silent -- but it only fires when the method is"); + System.out.println(" actually called, so a rarely-exercised path can ship broken."); + } + finally { + Support.logout(); + } + } + + @Configuration + @EnableMethodSecurity(securedEnabled = true, jsr250Enabled = true) + static class Config { + + @Bean + BankService bankService() { + return new BankService(); + } + + } + + /** + * The annotated service the whole module is built around. Every trap demo later reuses + * these expressions so the difference is always the plumbing, never the rule. + */ + public static class BankService { + + private final List ledger = new ArrayList<>( + List.of(new Account(1, "alice", 100), new Account(2, "bob", 200), new Account(3, "alice", 300))); + + @PreAuthorize("hasRole('ADMIN')") + public String adminOnly() { + return "the admin console"; + } + + @PreAuthorize("#owner == authentication.name") + public List accountsOf(String owner) { + return this.ledger.stream().filter((a) -> a.getOwner().equals(owner)).toList(); + } + + @PostAuthorize("returnObject.owner == authentication.name") + public Account readAccount(long id) { + return this.ledger.stream().filter((a) -> a.getId() == id).findFirst().orElseThrow(); + } + + @PostFilter("filterObject.owner == authentication.name") + public List allAccounts() { + return new ArrayList<>(this.ledger); + } + + @PreFilter("filterObject.owner == authentication.name") + public void deposit(List accounts) { + accounts.forEach((a) -> a.setBalanceMinor(a.getBalanceMinor() + 50)); + System.out.println(" method body saw : " + accounts); + } + + /** + * Two arguments and no {@code filterTarget}: Spring Security cannot guess which one to + * filter, and throws {@code IllegalStateException} at invocation time -- not at startup. + */ + @PreFilter("filterObject.owner == authentication.name") + public void depositAmbiguous(List accounts, long amountMinor) { + accounts.forEach((a) -> a.setBalanceMinor(a.getBalanceMinor() + amountMinor)); + } + + @PreFilter(value = "filterObject.owner == authentication.name", filterTarget = "accounts") + public void depositDisambiguated(List accounts, long amountMinor) { + accounts.forEach((a) -> a.setBalanceMinor(a.getBalanceMinor() + amountMinor)); + System.out.println(" method body saw : " + accounts); + } + + @Secured("ROLE_ADMIN") + public String securedAdminOnly() { + return "secured payload"; + } + + @RolesAllowed("ADMIN") + public String jsr250AdminOnly() { + return "jsr250 payload"; + } + + @PermitAll + public String jsr250Open() { + return "open payload"; + } + + @DenyAll + public String jsr250Closed() { + return "unreachable"; + } + + } + +} diff --git a/method-security/src/main/java/com/ankurm/methodsec/Demo2SelfInvocation.java b/method-security/src/main/java/com/ankurm/methodsec/Demo2SelfInvocation.java new file mode 100644 index 0000000..bcc5cb8 --- /dev/null +++ b/method-security/src/main/java/com/ankurm/methodsec/Demo2SelfInvocation.java @@ -0,0 +1,136 @@ +package com.ankurm.methodsec; + +import java.lang.reflect.Method; + +import org.springframework.aop.framework.AopContext; +import org.springframework.aop.support.AopUtils; +import org.springframework.beans.factory.ObjectProvider; +import org.springframework.context.annotation.AnnotationConfigApplicationContext; +import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.Configuration; +import org.springframework.context.annotation.EnableAspectJAutoProxy; +import org.springframework.security.access.prepost.PreAuthorize; +import org.springframework.security.config.annotation.method.configuration.EnableMethodSecurity; + +/** + * Demo 2 -- silent failure #1: self-invocation. + * + *

{@code @PreAuthorize} is advice on a proxy. A call that starts inside the target object + * never touches the proxy, so the advice never runs. Nothing logs, nothing throws, and the + * annotation is still visibly there in the source and in reflection -- which is exactly what + * makes it survive code review. + * + *

Chapter: + * docs/03-self-invocation.md. + */ +public class Demo2SelfInvocation { + + public static void main(String[] args) { + Support.banner("Demo 2 -- self-invocation: the annotation is there, the check is not"); + + try (var ctx = new AnnotationConfigApplicationContext(Config.class)) { + ReportService reports = ctx.getBean(ReportService.class); + + Support.login("alice", "ROLE_USER"); + + Support.heading("alice has ROLE_USER only. adminReport() requires ROLE_ADMIN."); + Support.attempt("reports.adminReport() (via proxy)", () -> reports.adminReport()); + Support.attempt("reports.userEntryPoint() (this.adminReport())", () -> reports.userEntryPoint()); + + Support.heading("Is the annotation actually there? (reflection on the target class)"); + try { + Method m = ReportService.class.getDeclaredMethod("adminReport"); + System.out.println(" ReportService.adminReport() @PreAuthorize -> " + + m.getAnnotation(PreAuthorize.class)); + System.out.println(" bean is an AOP proxy -> " + AopUtils.isAopProxy(reports)); + System.out.println(" proxy class -> " + reports.getClass().getName()); + System.out.println(" target class -> " + + AopUtils.getTargetClass(reports).getName()); + } + catch (NoSuchMethodException ex) { + throw new IllegalStateException(ex); + } + System.out.println(); + System.out.println(" The annotation is present, the bean IS proxied, and the call was still"); + System.out.println(" not checked. The proxy only sees calls that arrive from outside."); + + Support.heading("Three ways to make the inner call go through the proxy"); + Support.attempt("self-injection (ObjectProvider)", () -> reports.viaSelfInjection()); + Support.attempt("AopContext.currentProxy()", () -> reports.viaAopContext()); + Support.attempt("call a different bean (collaborator)", () -> ctx.getBean(FacadeService.class).run()); + } + finally { + Support.logout(); + } + } + + @Configuration + @EnableMethodSecurity + // exposeProxy = true is what makes AopContext.currentProxy() work at all. Without it that + // call throws IllegalStateException("Cannot find current proxy: Set 'exposeProxy' to true"). + @EnableAspectJAutoProxy(exposeProxy = true) + static class Config { + + @Bean + ReportService reportService(ObjectProvider self) { + return new ReportService(self); + } + + @Bean + FacadeService facadeService(ReportService reports) { + return new FacadeService(reports); + } + + } + + public static class ReportService { + + /** + * An {@code ObjectProvider} rather than a field of the bean's own type: injecting the + * bean into itself directly is a circular reference the container will refuse in a + * constructor. The provider resolves lazily, at call time, and hands back the proxy. + */ + private final ObjectProvider self; + + ReportService(ObjectProvider self) { + this.self = self; + } + + @PreAuthorize("hasRole('ADMIN')") + public String adminReport() { + return "TOP SECRET REVENUE NUMBERS"; + } + + /** The bug. {@code this} is the raw target object, so no advice runs. */ + public String userEntryPoint() { + return adminReport(); + } + + /** Fix 1 -- route the inner call back through the container-managed proxy. */ + public String viaSelfInjection() { + return this.self.getObject().adminReport(); + } + + /** Fix 2 -- ask AOP for the proxy that is currently handling this invocation. */ + public String viaAopContext() { + return ((ReportService) AopContext.currentProxy()).adminReport(); + } + + } + + /** Fix 3 -- the boring one. A different bean means a real, external, proxied call. */ + public static class FacadeService { + + private final ReportService reports; + + FacadeService(ReportService reports) { + this.reports = reports; + } + + public String run() { + return this.reports.adminReport(); + } + + } + +} diff --git a/method-security/src/main/java/com/ankurm/methodsec/Demo3NonProxyable.java b/method-security/src/main/java/com/ankurm/methodsec/Demo3NonProxyable.java new file mode 100644 index 0000000..d70f632 --- /dev/null +++ b/method-security/src/main/java/com/ankurm/methodsec/Demo3NonProxyable.java @@ -0,0 +1,199 @@ +package com.ankurm.methodsec; + +import java.lang.reflect.Method; + +import org.springframework.aop.framework.Advised; +import org.springframework.aop.support.AopUtils; +import org.springframework.context.annotation.AnnotationConfigApplicationContext; +import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.Configuration; +import org.springframework.security.access.prepost.PreAuthorize; +import org.springframework.security.config.annotation.method.configuration.EnableMethodSecurity; + +/** + * Demo 3 -- silent failure #2: methods Spring AOP cannot advise. + * + *

A CGLIB proxy is a generated subclass. It can only intercept a method it is allowed to + * override. {@code final}, {@code static} and {@code private} methods cannot be overridden, so + * the annotation on them is decoration. Nothing warns you. + * + *

The two surprises here are that package-private methods ARE advised (the + * generated subclass lands in the same package), and that a {@code final} class is the one + * variant that fails loudly at startup instead of silently at runtime. + * + *

Chapter: + * docs/04-non-proxyable-methods.md. + */ +public class Demo3NonProxyable { + + public static void main(String[] args) { + Support.banner("Demo 3 -- @PreAuthorize on methods the proxy cannot override"); + + Support.login("alice", "ROLE_USER"); + try { + try (var ctx = new AnnotationConfigApplicationContext(Config.class)) { + Vault vault = ctx.getBean(Vault.class); + + Support.heading("alice has ROLE_USER. Every method below says hasRole('ADMIN')."); + Support.attempt("public (overridable)", () -> vault.publicAdminOnly()); + Support.attempt("public final (NOT overridable)", () -> vault.finalAdminOnly()); + Support.attempt("static (NOT overridable)", () -> Vault.staticAdminOnly()); + Support.attempt("package-private (overridable, same package)", () -> vault.packagePrivateAdminOnly()); + Support.attempt("private, reached via a public wrapper", () -> vault.callsPrivate()); + + Support.heading("What the proxy actually overrode"); + Class proxyClass = vault.getClass(); + for (String name : new String[] { "publicAdminOnly", "finalAdminOnly", "packagePrivateAdminOnly" }) { + Method target = find(Vault.class, name); + Method onProxy = find(proxyClass, name); + boolean overridden = onProxy != null && !onProxy.getDeclaringClass().equals(Vault.class); + System.out.printf(" %-26s declared final=%-5s overridden by proxy=%s%n", name, + java.lang.reflect.Modifier.isFinal(target.getModifiers()), overridden); + } + System.out.println(" proxy class -> " + proxyClass.getName()); + } + + Support.heading("A JDK dynamic proxy only advises methods that are ON the interface"); + try (var ctx = new AnnotationConfigApplicationContext(JdkProxyConfig.class)) { + LedgerOperations ops = ctx.getBean(LedgerOperations.class); + System.out.println(" proxy is a JDK proxy -> " + AopUtils.isJdkDynamicProxy(ops)); + System.out.println(" proxied interfaces -> " + + java.util.Arrays.toString(((Advised) ops).getProxiedInterfaces())); + Support.attempt("onTheInterface() (advised)", () -> ops.onTheInterface()); + System.out.println(" notOnTheInterface() is public and annotated, but the JDK proxy does not"); + System.out.println(" implement it at all -- a caller cannot even reach it without casting to"); + System.out.println(" the implementation class, and that cast throws ClassCastException."); + Support.attempt("cast proxy to Ledger impl class", () -> ((Ledger) ops).notOnTheInterface()); + } + + Support.heading("A final CLASS is the loud one"); + try (var ctx = new AnnotationConfigApplicationContext(FinalClassConfig.class)) { + System.out.println(" context started, bean = " + ctx.getBean(SealedVault.class).getClass().getName()); + } + catch (RuntimeException ex) { + System.out.println(" startup FAILED -> " + ex.getClass().getSimpleName()); + Throwable root = ex; + while (root.getCause() != null) { + root = root.getCause(); + } + System.out.println(" root cause -> " + root.getClass().getName()); + System.out.println(" message -> " + String.valueOf(root.getMessage()).split("\n")[0]); + } + } + finally { + Support.logout(); + } + } + + private static Method find(Class type, String name) { + for (Method m : type.getMethods()) { + if (m.getName().equals(name)) { + return m; + } + } + for (Method m : type.getDeclaredMethods()) { + if (m.getName().equals(name)) { + return m; + } + } + return null; + } + + @Configuration + @EnableMethodSecurity + static class Config { + + @Bean + Vault vault() { + return new Vault(); + } + + } + + @Configuration + @EnableMethodSecurity + static class JdkProxyConfig { + + @Bean + LedgerOperations ledger() { + return new Ledger(); + } + + } + + @Configuration + @EnableMethodSecurity + static class FinalClassConfig { + + @Bean + SealedVault sealedVault() { + return new SealedVault(); + } + + } + + public static class Vault { + + @PreAuthorize("hasRole('ADMIN')") + public String publicAdminOnly() { + return "public payload"; + } + + @PreAuthorize("hasRole('ADMIN')") + public final String finalAdminOnly() { + return "final payload"; + } + + @PreAuthorize("hasRole('ADMIN')") + public static String staticAdminOnly() { + return "static payload"; + } + + @PreAuthorize("hasRole('ADMIN')") + String packagePrivateAdminOnly() { + return "package-private payload"; + } + + @PreAuthorize("hasRole('ADMIN')") + private String privateAdminOnly() { + return "private payload"; + } + + public String callsPrivate() { + return privateAdminOnly(); + } + + } + + public interface LedgerOperations { + + String onTheInterface(); + + } + + public static class Ledger implements LedgerOperations { + + @Override + @PreAuthorize("hasRole('ADMIN')") + public String onTheInterface() { + return "interface payload"; + } + + @PreAuthorize("hasRole('ADMIN')") + public String notOnTheInterface() { + return "impl-only payload"; + } + + } + + /** {@code final} class: CGLIB has nothing to subclass. */ + public static final class SealedVault { + + @PreAuthorize("hasRole('ADMIN')") + public String adminOnly() { + return "sealed payload"; + } + + } + +} diff --git a/method-security/src/main/java/com/ankurm/methodsec/Demo4SpelReference.java b/method-security/src/main/java/com/ankurm/methodsec/Demo4SpelReference.java new file mode 100644 index 0000000..7c5d797 --- /dev/null +++ b/method-security/src/main/java/com/ankurm/methodsec/Demo4SpelReference.java @@ -0,0 +1,318 @@ +package com.ankurm.methodsec; + +import java.util.List; + +import org.springframework.context.annotation.AnnotationConfigApplicationContext; +import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.Configuration; +import org.springframework.security.access.PermissionEvaluator; +import org.springframework.security.access.expression.method.DefaultMethodSecurityExpressionHandler; +import org.springframework.security.access.expression.method.MethodSecurityExpressionHandler; +import org.aopalliance.intercept.MethodInvocation; + +import org.springframework.security.access.hierarchicalroles.RoleHierarchyImpl; +import org.springframework.security.access.prepost.PreAuthorize; +import org.springframework.security.config.annotation.method.configuration.EnableMethodSecurity; +import org.springframework.security.authorization.AuthorizationManagerFactory; +import org.springframework.security.authorization.DefaultAuthorizationManagerFactory; +import org.springframework.security.core.Authentication; + +/** + * Demo 4 -- the SpEL surface of method security, evaluated for real. + * + *

Every row this prints is a real annotated method on a real proxied bean, invoked twice + * under two different identities. The reference table in the article is generated from this + * output, so if Spring Security changes one of these, the table is wrong on the next run + * rather than wrong forever. + * + *

Chapter: + * docs/02-spel-reference.md. + */ +public class Demo4SpelReference { + + public static void main(String[] args) { + Support.banner("Demo 4 -- what you can actually write inside @PreAuthorize / @PostAuthorize"); + + try (var ctx = new AnnotationConfigApplicationContext(Config.class)) { + Spel s = ctx.getBean(Spel.class); + + Support.heading("alice: ROLE_USER, ROLE_AUDITOR, plus the authority 'report:read'"); + Support.login("alice", "ROLE_USER", "ROLE_AUDITOR", "report:read"); + runAll(s); + + Support.heading("root: ROLE_ADMIN only (RoleHierarchy says ADMIN > USER > GUEST)"); + Support.login("root", "ROLE_ADMIN"); + runAll(s); + + Support.heading("Role prefix and hierarchy"); + Support.login("alice", "ROLE_USER", "ROLE_AUDITOR", "report:read"); + Support.attempt("hasRole('USER') -> ROLE_USER", () -> s.hasRoleUser()); + Support.attempt("hasAuthority('USER') -> literal 'USER'", () -> s.hasAuthorityUserNoPrefix()); + Support.attempt("hasAuthority('ROLE_USER')", () -> s.hasAuthorityRoleUser()); + Support.login("root", "ROLE_ADMIN"); + Support.attempt("root hasRole('GUEST') via RoleHierarchy", () -> s.hasRoleGuest()); + + Support.heading("Method arguments, the return value, and bean references"); + Support.login("alice", "ROLE_USER", "ROLE_AUDITOR", "report:read"); + Support.attempt("#owner == authentication.name (\"alice\")", () -> s.byParameterName("alice")); + Support.attempt("#owner == authentication.name (\"bob\")", () -> s.byParameterName("bob")); + Support.attempt("@P(\"o\") alias, #o == ...name (\"alice\")", () -> s.byParameterAlias("alice")); + Support.attempt("#root.this (the target object)", () -> s.byRootThis()); + Support.attempt("#root.args[0] -- no such property", () -> s.byPositionalArg("alice")); + Support.attempt("@policy.canRead(authentication, #id) id=1", () -> s.byBeanReference(1)); + Support.attempt("@policy.canRead(authentication, #id) id=9", () -> s.byBeanReference(9)); + Support.attempt("hasPermission(#id, 'account', 'read') id=1", () -> s.byPermissionEvaluator(1)); + Support.attempt("hasPermission(#id, 'account', 'read') id=9", () -> s.byPermissionEvaluator(9)); + Support.attempt("T(java.time.LocalDate) type reference", () -> s.byTypeReference()); + Support.attempt("@PostAuthorize returnObject.owner == ...name", () -> s.postAuthorizeReturnObject("alice")); + Support.attempt("@PostAuthorize returnObject.owner == ...name", () -> s.postAuthorizeReturnObject("bob")); + + Support.heading("The literal constants on SecurityExpressionRoot"); + System.out.println(" permitAll / denyAll exist as BOTH a boolean field and a no-arg method,"); + System.out.println(" and read/write/create/delete/admin are String constants meant for"); + System.out.println(" hasPermission(..) -- e.g. hasPermission(#id, 'account', read)."); + Support.attempt("hasPermission(#id, 'account', read) id=1", () -> s.byPermissionConstant(1)); + } + finally { + Support.logout(); + } + } + + private static void runAll(Spel s) { + Support.attempt("permitAll", () -> s.permitAll()); + Support.attempt("denyAll", () -> s.denyAll()); + Support.attempt("isAuthenticated()", () -> s.isAuthenticated()); + Support.attempt("isAnonymous()", () -> s.isAnonymous()); + Support.attempt("isFullyAuthenticated()", () -> s.isFullyAuthenticated()); + Support.attempt("isRememberMe()", () -> s.isRememberMe()); + Support.attempt("hasRole('ADMIN')", () -> s.hasRoleAdmin()); + Support.attempt("hasAnyRole('ADMIN','AUDITOR')", () -> s.hasAnyRole()); + Support.attempt("hasAllRoles('USER','AUDITOR')", () -> s.hasAllRoles()); + Support.attempt("hasAuthority('report:read')", () -> s.hasAuthorityReportRead()); + Support.attempt("hasAnyAuthority('report:read','x')", () -> s.hasAnyAuthority()); + Support.attempt("hasAllAuthorities('report:read','x')", () -> s.hasAllAuthorities()); + Support.attempt("authentication.name == 'alice'", () -> s.authenticationName()); + Support.attempt("principal == 'alice'", () -> s.principalEquals()); + } + + @Configuration + @EnableMethodSecurity + static class Config { + + @Bean + Spel spel() { + return new Spel(); + } + + @Bean + AccountPolicy policy() { + return new AccountPolicy(); + } + + /** + * Spring Security 7.1 routes {@code hasRole}, {@code hasAuthority}, + * {@code authenticated()} and friends through an + * {@link AuthorizationManagerFactory}. Role hierarchy and role prefix are set here, + * NOT on the expression handler -- {@code AbstractSecurityExpressionHandler}'s + * {@code setRoleHierarchy(..)} is deprecated in 7.1 (the compiler says so; that is + * how this demo found out). + */ + @Bean + static AuthorizationManagerFactory authorizationManagerFactory() { + DefaultAuthorizationManagerFactory factory = new DefaultAuthorizationManagerFactory<>(); + factory.setRoleHierarchy(RoleHierarchyImpl.withDefaultRolePrefix() + .role("ADMIN").implies("USER") + .role("USER").implies("GUEST") + .build()); + // The prefix hasRole('X') expands with. Left at the default so the printed table + // is the one readers reproduce; declared to show where the knob moved to. + factory.setRolePrefix("ROLE_"); + return factory; + } + + /** hasPermission(..) still needs a PermissionEvaluator on the expression handler. */ + @Bean + static MethodSecurityExpressionHandler methodSecurityExpressionHandler( + AuthorizationManagerFactory authorizationManagerFactory) { + DefaultMethodSecurityExpressionHandler handler = new DefaultMethodSecurityExpressionHandler(); + handler.setAuthorizationManagerFactory(authorizationManagerFactory); + handler.setPermissionEvaluator(new AccountPermissionEvaluator()); + return handler; + } + + } + + /** Every method is one SpEL expression and nothing else. */ + public static class Spel { + + @PreAuthorize("permitAll") + public String permitAll() { + return "ok"; + } + + @PreAuthorize("denyAll") + public String denyAll() { + return "ok"; + } + + @PreAuthorize("isAuthenticated()") + public String isAuthenticated() { + return "ok"; + } + + @PreAuthorize("isAnonymous()") + public String isAnonymous() { + return "ok"; + } + + @PreAuthorize("isFullyAuthenticated()") + public String isFullyAuthenticated() { + return "ok"; + } + + @PreAuthorize("isRememberMe()") + public String isRememberMe() { + return "ok"; + } + + @PreAuthorize("hasRole('ADMIN')") + public String hasRoleAdmin() { + return "ok"; + } + + @PreAuthorize("hasRole('USER')") + public String hasRoleUser() { + return "ok"; + } + + @PreAuthorize("hasRole('GUEST')") + public String hasRoleGuest() { + return "ok"; + } + + @PreAuthorize("hasAnyRole('ADMIN','AUDITOR')") + public String hasAnyRole() { + return "ok"; + } + + @PreAuthorize("hasAllRoles('USER','AUDITOR')") + public String hasAllRoles() { + return "ok"; + } + + @PreAuthorize("hasAuthority('report:read')") + public String hasAuthorityReportRead() { + return "ok"; + } + + @PreAuthorize("hasAuthority('USER')") + public String hasAuthorityUserNoPrefix() { + return "ok"; + } + + @PreAuthorize("hasAuthority('ROLE_USER')") + public String hasAuthorityRoleUser() { + return "ok"; + } + + @PreAuthorize("hasAnyAuthority('report:read','report:write')") + public String hasAnyAuthority() { + return "ok"; + } + + @PreAuthorize("hasAllAuthorities('report:read','report:write')") + public String hasAllAuthorities() { + return "ok"; + } + + @PreAuthorize("authentication.name == 'alice'") + public String authenticationName() { + return "ok"; + } + + @PreAuthorize("principal == 'alice'") + public String principalEquals() { + return "ok"; + } + + @PreAuthorize("#owner == authentication.name") + public String byParameterName(String owner) { + return "ok"; + } + + @PreAuthorize("#o == authentication.name") + public String byParameterAlias(@org.springframework.security.core.parameters.P("o") String owner) { + return "ok"; + } + + @PreAuthorize("#root.this != null") + public String byRootThis() { + return "ok"; + } + + /** + * There is no positional access to arguments. {@code MethodSecurityExpressionRoot} + * exposes {@code filterObject}, {@code returnObject} and {@code this} and nothing + * else; arguments are bound by NAME into the evaluation context. Kept here because + * the failure is worth seeing. + */ + @PreAuthorize("#root.args[0] == authentication.name") + public String byPositionalArg(String owner) { + return "ok"; + } + + @PreAuthorize("@policy.canRead(authentication, #id)") + public String byBeanReference(long id) { + return "ok"; + } + + @PreAuthorize("hasPermission(#id, 'account', 'read')") + public String byPermissionEvaluator(long id) { + return "ok"; + } + + @PreAuthorize("hasPermission(#id, 'account', read)") + public String byPermissionConstant(long id) { + return "ok"; + } + + @PreAuthorize("T(java.time.LocalDate).now().year >= 2020") + public String byTypeReference() { + return "ok"; + } + + @org.springframework.security.access.prepost.PostAuthorize("returnObject.owner == authentication.name") + public Account postAuthorizeReturnObject(String owner) { + return new Account(1, owner, 100); + } + + } + + /** A plain bean, reachable from SpEL as {@code @policy}. */ + public static class AccountPolicy { + + private final List readable = List.of(1L, 2L, 3L); + + public boolean canRead(Authentication authentication, long id) { + return this.readable.contains(id) && authentication.isAuthenticated(); + } + + } + + /** Wired into the expression handler; backs {@code hasPermission(..)}. */ + static class AccountPermissionEvaluator implements PermissionEvaluator { + + @Override + public boolean hasPermission(Authentication authentication, Object targetDomainObject, Object permission) { + return false; + } + + @Override + public boolean hasPermission(Authentication authentication, java.io.Serializable targetId, String targetType, + Object permission) { + return "account".equals(targetType) && "read".equals(permission) && ((Long) targetId) <= 3L; + } + + } + +} diff --git a/method-security/src/main/java/com/ankurm/methodsec/Demo5FilteringTraps.java b/method-security/src/main/java/com/ankurm/methodsec/Demo5FilteringTraps.java new file mode 100644 index 0000000..4c13591 --- /dev/null +++ b/method-security/src/main/java/com/ankurm/methodsec/Demo5FilteringTraps.java @@ -0,0 +1,229 @@ +package com.ankurm.methodsec; + +import java.util.ArrayList; +import java.util.LinkedHashMap; +import java.util.List; +import java.util.Map; +import java.util.Optional; +import java.util.stream.Stream; + +import org.springframework.context.annotation.AnnotationConfigApplicationContext; +import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.Configuration; +import org.springframework.security.access.prepost.PostFilter; +import org.springframework.security.access.prepost.PreFilter; +import org.springframework.security.config.annotation.method.configuration.EnableMethodSecurity; + +/** + * Demo 5 -- {@code filterObject}, and the container types filtering does and does not accept. + * + *

Filtering is the part of method security that mutates your data. {@code @PreFilter} + * removes elements from the caller's own argument, in place; {@code @PostFilter} rebuilds the + * return value. Both need a mutable container, and both quietly do nothing to a type they do + * not recognise -- which is the third silent failure in this module. + * + *

Chapter: + * docs/05-filtering.md. + */ +public class Demo5FilteringTraps { + + public static void main(String[] args) { + Support.banner("Demo 5 -- filterObject: mutability, container types, and the silent no-op"); + + try (var ctx = new AnnotationConfigApplicationContext(Config.class)) { + Filters f = ctx.getBean(Filters.class); + Support.login("alice", "ROLE_USER"); + + Support.heading("@PreFilter needs a MUTABLE argument -- and does not tell you when it is not"); + List mutable = new ArrayList<>(alicesAndBobs()); + Support.attemptVoid("new ArrayList<>(..)", () -> f.consume(mutable)); + Support.attemptVoid("List.of(..) (immutable)", () -> f.consume(alicesAndBobs())); + Support.attemptVoid("List.copyOf(..) (immutable)", () -> f.consume(List.copyOf(alicesAndBobs()))); + Support.attemptVoid("Arrays.asList(..)", () -> f.consume( + java.util.Arrays.asList(alicesAndBobs().toArray(new Account[0])))); + Support.attemptVoid("Collections.unmodifiableList(..)", () -> f.consume( + java.util.Collections.unmodifiableList(new ArrayList<>(alicesAndBobs())))); + Support.attemptVoid("stream().toList() (unmodifiable since 16)", () -> f.consume( + alicesAndBobs().stream().toList())); + Support.attemptVoid("stream().collect(toList()) (ArrayList)", () -> f.consume( + alicesAndBobs().stream().collect(java.util.stream.Collectors.toList()))); + Support.attemptVoid("Account[] (arrays rejected outright)", () -> f.consumeArray( + alicesAndBobs().toArray(new Account[0]))); + System.out.println(); + System.out.println(" Read the second and third lines again: bob's account reached the method"); + System.out.println(" body. @PreFilter filters by CLEARING the caller's collection and adding"); + System.out.println(" the survivors back. On an immutable list that throws, and"); + System.out.println(" DefaultMethodSecurityExpressionHandler.filterCollection catches the"); + System.out.println(" UnsupportedOperationException and returns a fresh list instead -- which"); + System.out.println(" PreFilterAuthorizationMethodInterceptor.invoke then discards, because it"); + System.out.println(" ignores filter()'s return value entirely. No exception, no WARN, no 403."); + + Support.heading("The only trace it leaves (same call, logger at TRACE)"); + enableTraceOn("org.springframework.security.access.expression.method.DefaultMethodSecurityExpressionHandler"); + Support.attemptVoid("List.of(..) with TRACE on", () -> f.consume(alicesAndBobs())); + resetLogging(); + + Support.heading("What @PostFilter accepts as a return type"); + Support.attempt("List", () -> f.returnsList()); + Support.attempt("Account[]", () -> f.returnsArray()); + Support.attempt("Stream (collected here)", () -> f.returnsStream().map(Account::getOwner).toList()); + Support.attempt("Map", () -> f.returnsMap()); + Support.attempt("Optional (alice's)", () -> f.returnsOptional("alice")); + Support.attempt("Optional (bob's)", () -> f.returnsOptional("bob")); + Support.attempt("List.of(..) (immutable return)", () -> f.returnsImmutableList()); + Support.attempt("Ledger (a type Spring Security does not know)", () -> f.returnsCustomContainer()); + Support.attempt("Page (real Spring Data PageImpl)", () -> f.returnsPage()); + + Support.heading("Identity: does @PostFilter hand back the same object?"); + List source = new ArrayList<>(alicesAndBobs()); + List filtered = f.returnsGivenList(source); + System.out.println(" returned == the list the method returned : " + (filtered == source)); + System.out.println(" the method's own list, after filtering : " + source); + System.out.println(); + System.out.println(" @PostFilter mutates the returned collection in place and hands the same"); + System.out.println(" reference back. If that collection is a cached or shared instance, you"); + System.out.println(" have just deleted rows from it for every future caller."); + } + finally { + Support.logout(); + } + } + + /** Route the named logger to stdout at FINEST so the TRACE message is part of the capture. */ + private static void enableTraceOn(String loggerName) { + java.util.logging.Logger logger = java.util.logging.Logger.getLogger(loggerName); + logger.setLevel(java.util.logging.Level.FINEST); + logger.setUseParentHandlers(false); + java.util.logging.Handler handler = new java.util.logging.StreamHandler(System.out, + new java.util.logging.Formatter() { + @Override + public String format(java.util.logging.LogRecord record) { + return " TRACE " + record.getMessage() + System.lineSeparator(); + } + }) { + @Override + public synchronized void publish(java.util.logging.LogRecord record) { + super.publish(record); + flush(); + } + }; + handler.setLevel(java.util.logging.Level.FINEST); + logger.addHandler(handler); + } + + private static void resetLogging() { + java.util.logging.Logger logger = java.util.logging.Logger + .getLogger("org.springframework.security.access.expression.method.DefaultMethodSecurityExpressionHandler"); + for (java.util.logging.Handler h : logger.getHandlers()) { + logger.removeHandler(h); + } + logger.setLevel(null); + logger.setUseParentHandlers(true); + } + + private static List alicesAndBobs() { + return List.of(new Account(1, "alice", 100), new Account(2, "bob", 200), new Account(3, "alice", 300)); + } + + @Configuration + @EnableMethodSecurity + static class Config { + + @Bean + Filters filters() { + return new Filters(); + } + + } + + public static class Filters { + + @PreFilter("filterObject.owner == authentication.name") + public void consume(List accounts) { + System.out.println(" method body saw: " + accounts); + } + + @PreFilter("filterObject.owner == authentication.name") + public void consumeArray(Account[] accounts) { + System.out.println(" method body saw: " + List.of(accounts)); + } + + @PostFilter("filterObject.owner == authentication.name") + public List returnsList() { + return new ArrayList<>(alicesAndBobs()); + } + + @PostFilter("filterObject.owner == authentication.name") + public List returnsGivenList(List accounts) { + return accounts; + } + + @PostFilter("filterObject.owner == authentication.name") + public List returnsImmutableList() { + return alicesAndBobs(); + } + + @PostFilter("filterObject.owner == authentication.name") + public Account[] returnsArray() { + return alicesAndBobs().toArray(new Account[0]); + } + + @PostFilter("filterObject.owner == authentication.name") + public Stream returnsStream() { + return alicesAndBobs().stream(); + } + + /** For a Map, {@code filterObject} is a {@code Map.Entry}, not the value. */ + @PostFilter("filterObject.value.owner == authentication.name") + public Map returnsMap() { + Map map = new LinkedHashMap<>(); + for (Account a : alicesAndBobs()) { + map.put("acct-" + a.getId(), a); + } + return map; + } + + @PostFilter("filterObject.owner == authentication.name") + public Optional returnsOptional(String owner) { + return alicesAndBobs().stream().filter((a) -> a.getOwner().equals(owner)).findFirst(); + } + + /** + * A real Spring Data {@code Page}. {@code PageImpl} implements {@code Slice} -> + * {@code Streamable} -> {@code Iterable}, but NOT {@code Collection}, so it falls + * through every branch of {@code DefaultMethodSecurityExpressionHandler.filter}. + */ + @PostFilter("filterObject.owner == authentication.name") + public org.springframework.data.domain.Page returnsPage() { + return new org.springframework.data.domain.PageImpl<>(new ArrayList<>(alicesAndBobs())); + } + + /** A container type Spring Security has no visitor for. */ + @PostFilter("filterObject.owner == authentication.name") + public Ledger returnsCustomContainer() { + return new Ledger(new ArrayList<>(alicesAndBobs())); + } + + } + + /** Deliberately not a Collection -- this is the Spring Data {@code Page} shape in miniature. */ + public static class Ledger { + + private final List content; + + public Ledger(List content) { + this.content = content; + } + + public List getContent() { + return this.content; + } + + @Override + public String toString() { + return "Ledger" + this.content; + } + + } + +} diff --git a/method-security/src/main/java/com/ankurm/methodsec/Demo6InterceptorOrder.java b/method-security/src/main/java/com/ankurm/methodsec/Demo6InterceptorOrder.java new file mode 100644 index 0000000..76e1c54 --- /dev/null +++ b/method-security/src/main/java/com/ankurm/methodsec/Demo6InterceptorOrder.java @@ -0,0 +1,223 @@ +package com.ankurm.methodsec; + +import javax.sql.DataSource; + +import org.springframework.aop.Advisor; +import org.springframework.aop.framework.Advised; +import org.springframework.context.annotation.AnnotationConfigApplicationContext; +import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.Configuration; +import org.springframework.core.Ordered; +import org.springframework.jdbc.core.JdbcTemplate; +import org.springframework.jdbc.datasource.DataSourceTransactionManager; +import org.springframework.jdbc.datasource.embedded.EmbeddedDatabaseBuilder; +import org.springframework.jdbc.datasource.embedded.EmbeddedDatabaseType; +import org.springframework.security.access.prepost.PostAuthorize; +import org.springframework.security.access.prepost.PostFilter; +import org.springframework.security.access.prepost.PreAuthorize; +import org.springframework.security.access.prepost.PreFilter; +import org.springframework.security.authorization.method.AuthorizationInterceptorsOrder; +import org.springframework.security.config.annotation.method.configuration.EnableMethodSecurity; +import org.springframework.transaction.PlatformTransactionManager; +import org.springframework.transaction.annotation.EnableTransactionManagement; +import org.springframework.transaction.annotation.Transactional; + +/** + * Demo 6 -- where the security advice sits in the interceptor chain, and why that decides + * whether a denied {@code @PostAuthorize} rolls anything back. + * + *

Everything printed here is read out of the running container: the real + * {@link AuthorizationInterceptorsOrder} constants, the real advisor list on a real proxy, and + * a real H2 row count after a real denial. + * + *

Chapter: + * docs/07-ordering-and-transactions.md. + */ +public class Demo6InterceptorOrder { + + public static void main(String[] args) { + Support.banner("Demo 6 -- interceptor order, and @PostAuthorize vs @Transactional"); + + Support.heading("AuthorizationInterceptorsOrder, read from the enum itself"); + System.out.printf(" %-16s %s%n", "CONSTANT", "getOrder()"); + for (AuthorizationInterceptorsOrder value : AuthorizationInterceptorsOrder.values()) { + System.out.printf(" %-16s %d%n", value.name(), value.getOrder()); + } + System.out.println(); + System.out.println(" Lower order = higher precedence = further OUT in the chain. Spring's own"); + System.out.println(" @Transactional advisor defaults to Ordered.LOWEST_PRECEDENCE (" + Ordered.LOWEST_PRECEDENCE + + "),"); + System.out.println(" which is larger than every number above -- so security wraps transactions,"); + System.out.println(" not the other way round."); + + Support.login("alice", "ROLE_USER"); + try { + try (var ctx = new AnnotationConfigApplicationContext(ChainConfig.class)) { + Support.heading("The advisor chain on a bean carrying all four annotations"); + AllFour bean = ctx.getBean(AllFour.class); + System.out.println(" advisors applied to the proxy: " + ((Advised) bean).getAdvisors().length); + System.out.printf(" %6s %s%n", "ORDER", "ADVISOR BEAN (as registered by @EnableMethodSecurity)"); + ctx.getBeansOfType(Advisor.class) + .entrySet() + .stream() + // each interceptor is registered twice, once under a "...Advisor" alias + .filter((e) -> !e.getKey().endsWith("Advisor")) + .sorted(java.util.Comparator + .comparingInt((java.util.Map.Entry e) -> (e.getValue() instanceof Ordered o) + ? o.getOrder() : Integer.MAX_VALUE)) + .forEach((e) -> System.out.printf(" %6d %s%n", + (e.getValue() instanceof Ordered o) ? o.getOrder() : Integer.MAX_VALUE, e.getKey())); + System.out.println(" (authorizeReturnObject sits at SECURE_RESULT = 450 and is registered"); + System.out.println(" whether or not anything in the app uses @AuthorizeReturnObject.)"); + System.out.println(); + System.out.println(" For BEFORE advice, a lower order runs earlier: @PreFilter (100) really"); + System.out.println(" does run before @PreAuthorize (200). For AFTER advice the same numbers"); + System.out.println(" mean the opposite. @PostAuthorize (500) sits FURTHER OUT than"); + System.out.println(" @PostFilter (600), so on the way back out @PostFilter finishes first"); + System.out.println(" and @PostAuthorize evaluates returnObject on the ALREADY-FILTERED list."); + System.out.println(); + System.out.println(" Both methods below return the same 3 elements and filter one away:"); + Support.attempt("@PostAuthorize returnObject.size() == 3", () -> bean.expectsThree(abc())); + Support.attempt("@PostAuthorize returnObject.size() == 2", () -> bean.expectsTwo(abc())); + } + + Support.heading("Default order: @PostAuthorize denies AFTER the transaction commits"); + try (var ctx = new AnnotationConfigApplicationContext(DefaultOrderConfig.class)) { + runTransfer(ctx); + } + + Support.heading("@EnableTransactionManagement(order = FIRST): the write rolls back"); + try (var ctx = new AnnotationConfigApplicationContext(TxOuterConfig.class)) { + runTransfer(ctx); + } + } + finally { + Support.logout(); + } + } + + private static java.util.List abc() { + return new java.util.ArrayList<>(java.util.List.of("a", "b", "c")); + } + + private static void runTransfer(AnnotationConfigApplicationContext ctx) { + Ledger ledger = ctx.getBean(Ledger.class); + JdbcTemplate jdbc = ctx.getBean(JdbcTemplate.class); + System.out.println(" rows before : " + jdbc.queryForObject("select count(*) from audit", Integer.class)); + Support.attempt("recordAndReturn(\"bob\") @PostAuthorize", () -> ledger.recordAndReturn("bob")); + System.out.println(" rows after the denial : " + jdbc.queryForObject("select count(*) from audit", Integer.class)); + } + + @Configuration + @EnableMethodSecurity + static class ChainConfig { + + @Bean + AllFour allFour() { + return new AllFour(); + } + + } + + public static class AllFour { + + @PreFilter("filterObject != null") + @PreAuthorize("isAuthenticated()") + @PostAuthorize("returnObject.size() == 3") + @PostFilter("filterObject != 'c'") + public java.util.List expectsThree(java.util.List in) { + return in; + } + + @PreFilter("filterObject != null") + @PreAuthorize("isAuthenticated()") + @PostAuthorize("returnObject.size() == 2") + @PostFilter("filterObject != 'c'") + public java.util.List expectsTwo(java.util.List in) { + return in; + } + + } + + @Configuration + @EnableMethodSecurity + @EnableTransactionManagement + static class DefaultOrderConfig extends BaseDbConfig { + + @Override + String dbName() { + return "audit-default-order"; + } + + } + + @Configuration + @EnableMethodSecurity + // FIRST is Integer.MIN_VALUE, so the transaction advisor becomes the OUTERMOST one and the + // AuthorizationDeniedException thrown by @PostAuthorize propagates through it as a rollback. + @EnableTransactionManagement(order = Integer.MIN_VALUE) + static class TxOuterConfig extends BaseDbConfig { + + @Override + String dbName() { + return "audit-tx-outer"; + } + + } + + abstract static class BaseDbConfig { + + abstract String dbName(); + + @Bean + DataSource dataSource() { + // A fixed name rather than generateUniqueName(true): the two contexts in this + // demo are opened and closed in sequence, and a stable URL keeps the captured + // output identical between runs. + return new EmbeddedDatabaseBuilder().setType(EmbeddedDatabaseType.H2) + .setName(dbName()) + .build(); + } + + @Bean + JdbcTemplate jdbcTemplate(DataSource dataSource) { + JdbcTemplate jdbc = new JdbcTemplate(dataSource); + jdbc.execute("create table audit (id identity primary key, owner varchar(64))"); + return jdbc; + } + + @Bean + PlatformTransactionManager transactionManager(DataSource dataSource) { + return new DataSourceTransactionManager(dataSource); + } + + @Bean + Ledger ledger(JdbcTemplate jdbc) { + return new Ledger(jdbc); + } + + } + + public static class Ledger { + + private final JdbcTemplate jdbc; + + Ledger(JdbcTemplate jdbc) { + this.jdbc = jdbc; + } + + /** + * Writes an audit row, then returns an account the caller may not be allowed to see. + * The write is the point: it happens inside the transaction, before the security + * check that rejects the return value ever runs. + */ + @Transactional + @PostAuthorize("returnObject.owner == authentication.name") + public Account recordAndReturn(String owner) { + this.jdbc.update("insert into audit(owner) values (?)", owner); + return new Account(1, owner, 100); + } + + } + +} diff --git a/method-security/src/main/java/com/ankurm/methodsec/Demo7DeniedHandling.java b/method-security/src/main/java/com/ankurm/methodsec/Demo7DeniedHandling.java new file mode 100644 index 0000000..b644a52 --- /dev/null +++ b/method-security/src/main/java/com/ankurm/methodsec/Demo7DeniedHandling.java @@ -0,0 +1,182 @@ +package com.ankurm.methodsec; + +import java.util.List; +import java.util.Optional; + +import org.aopalliance.intercept.MethodInvocation; + +import org.springframework.context.annotation.AnnotationConfigApplicationContext; +import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.Configuration; +import org.springframework.security.access.prepost.PostAuthorize; +import org.springframework.security.access.prepost.PreAuthorize; +import org.springframework.security.authorization.AuthorizationDeniedException; +import org.springframework.security.authorization.AuthorizationProxyFactory; +import org.springframework.security.authorization.AuthorizationResult; +import org.springframework.security.authorization.method.AuthorizeReturnObject; +import org.springframework.security.authorization.method.HandleAuthorizationDenied; +import org.springframework.security.authorization.method.MethodAuthorizationDeniedHandler; +import org.springframework.security.config.annotation.method.configuration.EnableMethodSecurity; + +/** + * Demo 7 -- what happens on denial, and how to change it. + * + *

Two mechanisms that are newer than most of the material written about method security: + * {@code @HandleAuthorizationDenied}, which lets a denial return a masked value instead of + * throwing, and {@code @AuthorizeReturnObject}, which pushes the check down onto the returned + * object's own getters. + * + *

Chapter: + * docs/06-denied-handling.md. + */ +public class Demo7DeniedHandling { + + public static void main(String[] args) { + Support.banner("Demo 7 -- @HandleAuthorizationDenied and @AuthorizeReturnObject"); + + try (var ctx = new AnnotationConfigApplicationContext(Config.class)) { + Support.login("alice", "ROLE_USER"); + + Support.heading("The exception type you actually catch"); + Accounts accounts = ctx.getBean(Accounts.class); + try { + accounts.adminOnly(); + } + catch (RuntimeException ex) { + System.out.println(" thrown -> " + ex.getClass().getName()); + System.out.println(" is AccessDeniedException -> " + + (ex instanceof org.springframework.security.access.AccessDeniedException)); + System.out.println(" is AuthorizationDeniedException -> " + (ex instanceof AuthorizationDeniedException)); + if (ex instanceof AuthorizationDeniedException denied) { + AuthorizationResult result = denied.getAuthorizationResult(); + System.out.println(" carries an AuthorizationResult -> " + result.getClass().getSimpleName() + + " granted=" + result.isGranted()); + } + } + System.out.println(); + System.out.println(" Handlers written against AccessDeniedException still work -- but the"); + System.out.println(" concrete type carries the AuthorizationResult that explains the denial."); + + Support.heading("@HandleAuthorizationDenied: return something instead of throwing"); + Support.attempt("maskedBalance() (alice, no ROLE_FINANCE)", () -> accounts.maskedBalance()); + Support.attempt("maskedList() (alice, no ROLE_FINANCE)", () -> accounts.maskedList()); + Support.login("cfo", "ROLE_FINANCE"); + Support.attempt("maskedBalance() (cfo, has ROLE_FINANCE)", () -> accounts.maskedBalance()); + + Support.heading("@AuthorizeReturnObject: the check moves onto the returned object"); + Support.login("alice", "ROLE_USER"); + Customer proxied = accounts.findCustomer("alice"); + System.out.println(" returned instance -> " + proxied.getClass().getName()); + Support.attempt("customer.getName() (no authority needed)", () -> proxied.getName()); + Support.attempt("customer.getEmail() (needs 'pii:read')", () -> proxied.getEmail()); + Support.login("privacy-officer", "pii:read"); + Customer allowed = accounts.findCustomer("alice"); + Support.attempt("customer.getEmail() (has 'pii:read')", () -> allowed.getEmail()); + + Support.heading("Same thing without the annotation, via AuthorizationProxyFactory"); + Support.login("alice", "ROLE_USER"); + // NOTE the package: org.springframework.security.authorization, not + // ...authorization.method, which is where the reference docs place it. + AuthorizationProxyFactory factory = ctx.getBean(AuthorizationProxyFactory.class); + Customer raw = new Customer("alice", "alice@example.com"); + Customer wrapped = factory.proxy(raw); + Support.attempt("raw.getEmail() (unproxied object)", () -> raw.getEmail()); + Support.attempt("wrapped.getEmail() (proxied object)", () -> wrapped.getEmail()); + } + finally { + Support.logout(); + } + } + + @Configuration + @EnableMethodSecurity + static class Config { + + @Bean + Accounts accounts() { + return new Accounts(); + } + + @Bean + MaskingHandler maskingHandler() { + return new MaskingHandler(); + } + + } + + public static class Accounts { + + @PreAuthorize("hasRole('ADMIN')") + public String adminOnly() { + return "admin payload"; + } + + @PreAuthorize("hasRole('FINANCE')") + @HandleAuthorizationDenied(handlerClass = MaskingHandler.class) + public String maskedBalance() { + return "1,204,993.22"; + } + + @PreAuthorize("hasRole('FINANCE')") + @HandleAuthorizationDenied(handlerClass = MaskingHandler.class) + public List maskedList() { + return List.of("a", "b"); + } + + @AuthorizeReturnObject + public Customer findCustomer(String name) { + return new Customer(name, name + "@example.com"); + } + + @AuthorizeReturnObject + public Optional findOptionalCustomer(String name) { + return Optional.of(new Customer(name, name + "@example.com")); + } + + } + + /** + * Note the class is not final and {@code getEmail()} is not final -- the returned object is + * proxied with CGLIB, so the same rules as Demo 3 apply to it. A record here would fail. + */ + public static class Customer { + + private final String name; + + private final String email; + + public Customer(String name, String email) { + this.name = name; + this.email = email; + } + + public String getName() { + return this.name; + } + + @PreAuthorize("hasAuthority('pii:read')") + public String getEmail() { + return this.email; + } + + } + + /** + * Returns a masked value rather than throwing. The return type must be assignable to the + * method's declared return type, which is why this one inspects it. + */ + static class MaskingHandler implements MethodAuthorizationDeniedHandler { + + @Override + public Object handleDeniedInvocation(MethodInvocation methodInvocation, + AuthorizationResult authorizationResult) { + Class returnType = methodInvocation.getMethod().getReturnType(); + if (List.class.isAssignableFrom(returnType)) { + return List.of(); + } + return "***masked***"; + } + + } + +} diff --git a/method-security/src/main/java/com/ankurm/methodsec/Demo8MetaAnnotations.java b/method-security/src/main/java/com/ankurm/methodsec/Demo8MetaAnnotations.java new file mode 100644 index 0000000..7021263 --- /dev/null +++ b/method-security/src/main/java/com/ankurm/methodsec/Demo8MetaAnnotations.java @@ -0,0 +1,248 @@ +package com.ankurm.methodsec; + +import java.lang.annotation.ElementType; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; +import java.lang.annotation.Target; + +import org.springframework.context.annotation.AnnotationConfigApplicationContext; +import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.Configuration; +import org.springframework.security.access.prepost.PreAuthorize; +import org.springframework.security.config.annotation.method.configuration.EnableMethodSecurity; +import org.springframework.security.core.annotation.AnnotationTemplateExpressionDefaults; + +/** + * Demo 8 -- meta-annotations, expression templates, class-level rules, and the one + * configuration mistake method security refuses to start with. + * + *

Chapter: + * docs/08-meta-annotations.md. + */ +public class Demo8MetaAnnotations { + + public static void main(String[] args) { + Support.banner("Demo 8 -- meta-annotations, templates, class-level rules, ambiguity"); + + Support.login("alice", "ROLE_USER"); + try { + Support.heading("A plain meta-annotation needs no extra configuration"); + try (var ctx = new AnnotationConfigApplicationContext(PlainConfig.class)) { + Plain plain = ctx.getBean(Plain.class); + Support.attempt("@IsAdmin (alice, ROLE_USER)", () -> plain.adminOnly()); + Support.login("root", "ROLE_ADMIN"); + Support.attempt("@IsAdmin (root, ROLE_ADMIN)", () -> plain.adminOnly()); + Support.login("alice", "ROLE_USER"); + } + + Support.heading("A TEMPLATED meta-annotation, with NO AnnotationTemplateExpressionDefaults bean"); + try (var ctx = new AnnotationConfigApplicationContext(NoTemplateConfig.class)) { + Templated t = ctx.getBean(Templated.class); + Support.login("root", "ROLE_ADMIN"); + Support.attempt("@HasRole(\"ADMIN\") as root (ROLE_ADMIN)", () -> t.needsAdminRole()); + Support.login("alice", "ROLE_USER"); + Support.attempt("@HasRole(\"ADMIN\") as alice (ROLE_USER)", () -> t.needsAdminRole()); + } + + System.out.println(); + System.out.println(" '{value}' was substituted anyway. The reference documentation says you"); + System.out.println(" must publish an AnnotationTemplateExpressionDefaults bean for templated"); + System.out.println(" meta-annotations to work; in 7.1.1 you do not."); + System.out.println(" PreAuthorizeExpressionAttributeRegistry initialises its scanner with"); + System.out.println(" SecurityAnnotationScanners.requireUnique(PreAuthorize.class), and that"); + System.out.println(" overload constructs a default AnnotationTemplateExpressionDefaults for"); + System.out.println(" you. Publishing the bean only changes ignoreUnknown."); + + Support.heading("The same annotation WITH the AnnotationTemplateExpressionDefaults bean"); + try (var ctx = new AnnotationConfigApplicationContext(TemplateConfig.class)) { + Templated t = ctx.getBean(Templated.class); + Support.login("root", "ROLE_ADMIN"); + Support.attempt("@HasRole(\"ADMIN\") as root (ROLE_ADMIN)", () -> t.needsAdminRole()); + Support.login("alice", "ROLE_USER"); + Support.attempt("@HasRole(\"ADMIN\") as alice (ROLE_USER)", () -> t.needsAdminRole()); + Support.attempt("@HasRole(\"USER\") as alice (ROLE_USER)", () -> t.needsRole()); + } + + Support.heading("Class-level rules, and what a method-level one does to them"); + try (var ctx = new AnnotationConfigApplicationContext(ClassLevelConfig.class)) { + ClassLevel c = ctx.getBean(ClassLevel.class); + Support.attempt("inherited from the class (needs ADMIN)", () -> c.inherited()); + Support.attempt("method-level overrides it (needs USER)", () -> c.overridden()); + Support.attempt("class @PreAuthorize AND method @PostAuthorize", () -> c.andedWithPostAuthorize()); + } + + Support.heading("Two interfaces, two different @PreAuthorize on the same method"); + try (var ctx = new AnnotationConfigApplicationContext(AmbiguousConfig.class)) { + System.out.println(" context started fine."); + ReadsAsUser bean = ctx.getBean(ReadsAsUser.class); + System.out.println(" bean type -> " + bean.getClass().getName()); + Support.attempt("read() -- inherits two conflicting rules", () -> bean.read()); + System.out.println(); + System.out.println(" It is not a startup failure: the context refreshes, the bean is"); + System.out.println(" proxied, and the conflict only surfaces when the method is called."); + System.out.println(" The fix is to put @PreAuthorize on the implementation method, which"); + System.out.println(" is the nearest declaration and therefore wins outright."); + } + catch (RuntimeException ex) { + Throwable root = ex; + while (root.getCause() != null) { + root = root.getCause(); + } + System.out.println(" startup FAILED -> " + root.getClass().getSimpleName()); + System.out.println(" message -> " + String.valueOf(root.getMessage()).split("\n")[0]); + } + } + finally { + Support.logout(); + } + } + + @Target({ ElementType.METHOD, ElementType.TYPE }) + @Retention(RetentionPolicy.RUNTIME) + @PreAuthorize("hasRole('ADMIN')") + public @interface IsAdmin { + + } + + @Target({ ElementType.METHOD, ElementType.TYPE }) + @Retention(RetentionPolicy.RUNTIME) + @PreAuthorize("hasRole('{value}')") + public @interface HasRole { + + String value(); + + } + + @Configuration + @EnableMethodSecurity + static class PlainConfig { + + @Bean + Plain plain() { + return new Plain(); + } + + } + + @Configuration + @EnableMethodSecurity + static class NoTemplateConfig { + + @Bean + Templated templated() { + return new Templated(); + } + + } + + @Configuration + @EnableMethodSecurity + static class TemplateConfig { + + @Bean + Templated templated() { + return new Templated(); + } + + /** + * The documented prerequisite for {@code {value}} templates. Verified NOT to be one: + * the scanner already builds its own default. The bean's only job is + * {@code setIgnoreUnknown(false)}, which turns an unrecognised placeholder into an + * error instead of leaving it in the expression. + */ + @Bean + static AnnotationTemplateExpressionDefaults templateDefaults() { + return new AnnotationTemplateExpressionDefaults(); + } + + } + + @Configuration + @EnableMethodSecurity + static class ClassLevelConfig { + + @Bean + ClassLevel classLevel() { + return new ClassLevel(); + } + + } + + @Configuration + @EnableMethodSecurity + static class AmbiguousConfig { + + @Bean + Ambiguous ambiguous() { + return new Ambiguous(); + } + + } + + public static class Plain { + + @IsAdmin + public String adminOnly() { + return "ok"; + } + + } + + public static class Templated { + + @HasRole("USER") + public String needsRole() { + return "ok"; + } + + @HasRole("ADMIN") + public String needsAdminRole() { + return "ok"; + } + + } + + @PreAuthorize("hasRole('ADMIN')") + public static class ClassLevel { + + public String inherited() { + return "ok"; + } + + @PreAuthorize("hasRole('USER')") + public String overridden() { + return "ok"; + } + + @PreAuthorize("hasRole('USER')") + @org.springframework.security.access.prepost.PostAuthorize("returnObject == 'never'") + public String andedWithPostAuthorize() { + return "ok"; + } + + } + + public interface ReadsAsUser { + + @PreAuthorize("hasRole('USER')") + String read(); + + } + + public interface ReadsAsAdmin { + + @PreAuthorize("hasRole('ADMIN')") + String read(); + + } + + public static class Ambiguous implements ReadsAsUser, ReadsAsAdmin { + + @Override + public String read() { + return "ok"; + } + + } + +} diff --git a/method-security/src/main/java/com/ankurm/methodsec/Demo9ParameterNames.java b/method-security/src/main/java/com/ankurm/methodsec/Demo9ParameterNames.java new file mode 100644 index 0000000..f48337e --- /dev/null +++ b/method-security/src/main/java/com/ankurm/methodsec/Demo9ParameterNames.java @@ -0,0 +1,88 @@ +package com.ankurm.methodsec; + +import java.lang.reflect.Method; + +import org.springframework.context.annotation.AnnotationConfigApplicationContext; +import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.Configuration; +import org.springframework.security.access.prepost.PreAuthorize; +import org.springframework.security.config.annotation.method.configuration.EnableMethodSecurity; +import org.springframework.security.core.parameters.P; + +/** + * Demo 9 -- {@code #parameterName} depends on a compiler flag. + * + *

{@code @PreAuthorize("#owner == authentication.name")} resolves {@code #owner} by looking + * up the method's parameter names through a {@code ParameterNameDiscoverer}. Parameter names + * survive compilation only when {@code javac} is given {@code -parameters}. Without it the + * name is {@code arg0}, {@code #owner} resolves to nothing, and the comparison is false -- + * every call is denied. + * + *

{@code scripts/run-all.sh} compiles this module twice and runs this class from both + * builds, so {@code docs/output/demo9-with-parameters.txt} and + * {@code docs/output/demo9-without-parameters.txt} are the same code under the two flags. + * + *

Chapter: + * docs/02-spel-reference.md. + */ +public class Demo9ParameterNames { + + public static void main(String[] args) throws Exception { + Support.banner("Demo 9 -- #parameterName and the -parameters compiler flag"); + + Method byName = Owned.class.getMethod("byParameterName", String.class); + Method byAlias = Owned.class.getMethod("byParameterAlias", String.class); + System.out.println(" compiled with -parameters : " + byName.getParameters()[0].isNamePresent()); + System.out.println(" byParameterName param[0] : " + byName.getParameters()[0].getName()); + System.out.println(" byParameterAlias param[0] : " + byAlias.getParameters()[0].getName() + + " (annotated @P(\"o\"))"); + + try (var ctx = new AnnotationConfigApplicationContext(Config.class)) { + Owned owned = ctx.getBean(Owned.class); + Support.login("alice", "ROLE_USER"); + + Support.heading("alice calling with her own name"); + Support.attempt("#owner == authentication.name", () -> owned.byParameterName("alice")); + Support.attempt("#o == authentication.name (@P(\"o\"))", () -> owned.byParameterAlias("alice")); + + Support.heading("alice calling with somebody else's name"); + Support.attempt("#owner == authentication.name", () -> owned.byParameterName("bob")); + Support.attempt("#o == authentication.name (@P(\"o\"))", () -> owned.byParameterAlias("bob")); + } + finally { + Support.logout(); + } + + System.out.println(); + System.out.println(" Without -parameters the first expression denies BOTH calls -- it fails"); + System.out.println(" closed, which is the good direction, but it fails silently in the sense"); + System.out.println(" that nothing tells you the rule is not the rule you wrote. @P(\"o\") does"); + System.out.println(" not depend on the flag, because the name is in the class file either way."); + } + + @Configuration + @EnableMethodSecurity + static class Config { + + @Bean + Owned owned() { + return new Owned(); + } + + } + + public static class Owned { + + @PreAuthorize("#owner == authentication.name") + public String byParameterName(String owner) { + return "ok"; + } + + @PreAuthorize("#o == authentication.name") + public String byParameterAlias(@P("o") String owner) { + return "ok"; + } + + } + +} diff --git a/method-security/src/main/java/com/ankurm/methodsec/Support.java b/method-security/src/main/java/com/ankurm/methodsec/Support.java new file mode 100644 index 0000000..b8c6c07 --- /dev/null +++ b/method-security/src/main/java/com/ankurm/methodsec/Support.java @@ -0,0 +1,128 @@ +package com.ankurm.methodsec; + +import java.util.List; +import java.util.function.Supplier; + +import org.springframework.security.authentication.UsernamePasswordAuthenticationToken; +import org.springframework.security.core.Authentication; +import org.springframework.security.core.authority.AuthorityUtils; +import org.springframework.security.core.context.SecurityContextHolder; + +/** + * Shared plumbing for every demo in this module: log in as somebody, run something, and + * report what happened in one line. + * + *

Nothing here is Spring Security API worth learning -- it exists so the demos can be read + * as a list of claims rather than a list of try/catch blocks. See + * docs/01-how-method-security-runs.md + * for what actually happens between {@code run(..)} and the annotated method. + */ +public final class Support { + + static { + tidyLogging(); + } + + private Support() { + } + + /** + * Strip timestamps and source-method lines out of JUL output so captured files diff + * cleanly between runs, and silence the embedded-database chatter. Spring's own WARNING + * about un-proxyable final methods is deliberately kept -- it is evidence, not noise. + */ + private static void tidyLogging() { + java.util.logging.Logger root = java.util.logging.Logger.getLogger(""); + for (java.util.logging.Handler handler : root.getHandlers()) { + handler.setFormatter(new java.util.logging.Formatter() { + @Override + public String format(java.util.logging.LogRecord record) { + return record.getLevel() + " " + shortName(record.getLoggerName()) + ": " + + formatMessage(record) + System.lineSeparator(); + } + }); + } + java.util.logging.Logger.getLogger("org.springframework.jdbc.datasource.embedded.EmbeddedDatabaseFactory") + .setLevel(java.util.logging.Level.WARNING); + } + + private static String shortName(String loggerName) { + if (loggerName == null) { + return "?"; + } + int dot = loggerName.lastIndexOf('.'); + return (dot < 0) ? loggerName : loggerName.substring(dot + 1); + } + + /** Put an authenticated user with the given authorities into the {@code SecurityContextHolder}. */ + public static void login(String name, String... authorities) { + Authentication auth = UsernamePasswordAuthenticationToken.authenticated(name, "n/a", + AuthorityUtils.createAuthorityList(authorities)); + SecurityContextHolder.getContext().setAuthentication(auth); + } + + /** Clear the context -- an unauthenticated caller, not an anonymous one. */ + public static void logout() { + SecurityContextHolder.clearContext(); + } + + /** + * Invoke {@code body} and print one line saying whether it returned or was denied. + * Returns the value on success and {@code null} on denial, so callers can keep going. + */ + public static T attempt(String label, Supplier body) { + try { + T value = body.get(); + System.out.printf(" %-46s ALLOWED -> %s%n", label, render(value)); + return value; + } + catch (RuntimeException ex) { + Throwable root = ex; + while (root.getCause() != null) { + root = root.getCause(); + } + String detail = firstLine(ex.getMessage()); + if (root != ex) { + detail += " [cause: " + root.getClass().getSimpleName() + ": " + firstLine(root.getMessage()) + "]"; + } + System.out.printf(" %-46s DENIED -> %s: %s%n", label, ex.getClass().getSimpleName(), detail); + return null; + } + } + + /** Same as {@link #attempt} for a void call. */ + public static void attemptVoid(String label, Runnable body) { + attempt(label, () -> { + body.run(); + return "(void)"; + }); + } + + public static void heading(String title) { + System.out.println(); + System.out.println(title); + System.out.println("-".repeat(title.length())); + } + + public static void banner(String title) { + System.out.println("=".repeat(78)); + System.out.println(title); + System.out.println("=".repeat(78)); + } + + private static String render(Object value) { + if (value instanceof Object[] array) { + return List.of(array).toString(); + } + return String.valueOf(value); + } + + private static String firstLine(String message) { + if (message == null) { + return "(no message)"; + } + int newline = message.indexOf('\n'); + return (newline < 0) ? message : message.substring(0, newline) + " ..."; + } + +} diff --git a/method-security/src/test/java/com/ankurm/methodsec/MethodSecurityTrapsTest.java b/method-security/src/test/java/com/ankurm/methodsec/MethodSecurityTrapsTest.java new file mode 100644 index 0000000..7562632 --- /dev/null +++ b/method-security/src/test/java/com/ankurm/methodsec/MethodSecurityTrapsTest.java @@ -0,0 +1,297 @@ +package com.ankurm.methodsec; + +import java.util.ArrayList; +import java.util.List; + +import org.aopalliance.intercept.MethodInvocation; +import org.junit.jupiter.api.Test; + +import org.springframework.aop.framework.AopContext; +import org.springframework.beans.factory.ObjectProvider; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.context.annotation.AnnotationConfigApplicationContext; +import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.Configuration; +import org.springframework.context.annotation.EnableAspectJAutoProxy; +import org.springframework.security.access.prepost.PostAuthorize; +import org.springframework.security.access.prepost.PostFilter; +import org.springframework.security.access.prepost.PreAuthorize; +import org.springframework.security.access.prepost.PreFilter; +import org.springframework.security.authorization.AuthorizationDeniedException; +import org.springframework.security.authorization.method.AuthorizationInterceptorsOrder; +import org.springframework.security.config.annotation.method.configuration.EnableMethodSecurity; +import org.springframework.security.test.context.support.WithMockUser; +import org.springframework.test.context.junit.jupiter.SpringJUnitConfig; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatCode; +import static org.assertj.core.api.Assertions.assertThatExceptionOfType; + +/** + * The claims this module makes, as assertions. + * + *

Each test is named after the thing it pins. If Spring Security changes one of these + * behaviours, this suite fails and the article is wrong -- which is the point of having it. + * The prose walkthrough of every case is in + * the doc chapters. + */ +@SpringJUnitConfig(MethodSecurityTrapsTest.TestConfig.class) +class MethodSecurityTrapsTest { + + @Autowired + Service service; + + @Autowired + Collaborator collaborator; + + // --- the happy path, so the failures below mean something ------------------------------ + + @Test + @WithMockUser(roles = "USER") + void externalCallToAnnotatedMethodIsChecked() { + assertThatExceptionOfType(AuthorizationDeniedException.class).isThrownBy(this.service::adminOnly); + } + + @Test + @WithMockUser(roles = "ADMIN") + void externalCallByAnAdminIsAllowed() { + assertThat(this.service.adminOnly()).isEqualTo("secret"); + } + + @Test + @WithMockUser(roles = "USER") + void deniedCallThrowsAuthorizationDeniedWhichIsAnAccessDeniedException() { + assertThatExceptionOfType(AuthorizationDeniedException.class).isThrownBy(this.service::adminOnly) + .isInstanceOf(org.springframework.security.access.AccessDeniedException.class) + .satisfies((ex) -> assertThat(ex.getAuthorizationResult().isGranted()).isFalse()); + } + + // --- trap 1: self-invocation ------------------------------------------------------------ + + @Test + @WithMockUser(roles = "USER") + void selfInvocationSkipsTheCheckEntirely() { + assertThat(this.service.selfInvokes()).isEqualTo("secret"); + } + + @Test + @WithMockUser(roles = "USER") + void routingTheInnerCallThroughTheProxyRestoresTheCheck() { + assertThatExceptionOfType(AuthorizationDeniedException.class).isThrownBy(this.service::viaSelfInjection); + assertThatExceptionOfType(AuthorizationDeniedException.class).isThrownBy(this.service::viaAopContext); + assertThatExceptionOfType(AuthorizationDeniedException.class).isThrownBy(this.collaborator::callsAdminOnly); + } + + // --- trap 2: methods the proxy cannot override ------------------------------------------- + + @Test + @WithMockUser(roles = "USER") + void finalMethodIsNotAdvised() { + assertThat(this.service.finalAdminOnly()).isEqualTo("secret"); + } + + @Test + @WithMockUser(roles = "USER") + void staticMethodIsNotAdvised() { + assertThat(Service.staticAdminOnly()).isEqualTo("secret"); + } + + @Test + @WithMockUser(roles = "USER") + void packagePrivateMethodIsAdvisedBecauseTheCglibSubclassSharesThePackage() { + assertThatExceptionOfType(AuthorizationDeniedException.class) + .isThrownBy(this.service::packagePrivateAdminOnly); + } + + // --- trap 3: @PreFilter silently gives up on an immutable argument ----------------------- + + @Test + @WithMockUser(username = "alice", roles = "USER") + void preFilterRemovesElementsFromTheCallersOwnMutableList() { + List accounts = new ArrayList<>(ledger()); + this.service.consume(accounts); + assertThat(accounts).hasSize(2).allSatisfy((a) -> assertThat(a.getOwner()).isEqualTo("alice")); + } + + @Test + @WithMockUser(username = "alice", roles = "USER") + void preFilterOnAnImmutableListDoesNothingAndDoesNotThrow() { + List immutable = ledger(); + assertThatCode(() -> this.service.consume(immutable)).doesNotThrowAnyException(); + // note: lastSeen() is a METHOD call, not a field read -- reading service.lastSeen + // directly would read the CGLIB proxy's own uninitialised field, not the target's. + assertThat(this.service.lastSeen()).hasSize(3); + assertThat(this.service.lastSeen()).anySatisfy((a) -> assertThat(a.getOwner()).isEqualTo("bob")); + } + + @Test + @WithMockUser(username = "alice", roles = "USER") + void postFilterOnAnImmutableListDoesWorkBecauseTheNewListIsTheReturnValue() { + assertThat(this.service.immutableResults()).hasSize(2); + } + + // --- trap 4: ordering --------------------------------------------------------------------- + + @Test + void interceptorOrderIsPreFilterPreAuthorizeSecuredJsr250SecureResultPostAuthorizePostFilter() { + assertThat(AuthorizationInterceptorsOrder.PRE_FILTER.getOrder()).isEqualTo(100); + assertThat(AuthorizationInterceptorsOrder.PRE_AUTHORIZE.getOrder()).isEqualTo(200); + assertThat(AuthorizationInterceptorsOrder.SECURED.getOrder()).isEqualTo(300); + assertThat(AuthorizationInterceptorsOrder.JSR250.getOrder()).isEqualTo(400); + assertThat(AuthorizationInterceptorsOrder.SECURE_RESULT.getOrder()).isEqualTo(450); + assertThat(AuthorizationInterceptorsOrder.POST_AUTHORIZE.getOrder()).isEqualTo(500); + assertThat(AuthorizationInterceptorsOrder.POST_FILTER.getOrder()).isEqualTo(600); + } + + @Test + @WithMockUser(username = "alice", roles = "USER") + void postAuthorizeSeesTheAlreadyFilteredReturnValue() { + // three elements go in, @PostFilter removes one, and @PostAuthorize -- which sits + // further OUT and therefore runs LAST on the way back -- sees two, not three. + assertThatExceptionOfType(AuthorizationDeniedException.class) + .isThrownBy(() -> this.service.expectsThree(abc())); + assertThat(this.service.expectsTwo(abc())).containsExactly("a", "b"); + } + + // --- trap 5: parameter names ------------------------------------------------------------- + + @Test + @WithMockUser(username = "alice", roles = "USER") + void parameterNameExpressionsNeedTheParametersCompilerFlag() { + // This module is compiled WITH -parameters (see pom.xml), so the name resolves. + assertThat(Service.class.getDeclaredMethods()).anySatisfy((m) -> { + if (m.getName().equals("byParameterName")) { + assertThat(m.getParameters()[0].isNamePresent()).isTrue(); + assertThat(m.getParameters()[0].getName()).isEqualTo("owner"); + } + }); + assertThat(this.service.byParameterName("alice")).isEqualTo("ok"); + assertThatExceptionOfType(AuthorizationDeniedException.class) + .isThrownBy(() -> this.service.byParameterName("bob")); + } + + private static List ledger() { + return List.of(new Account(1, "alice", 100), new Account(2, "bob", 200), new Account(3, "alice", 300)); + } + + private static List abc() { + return new ArrayList<>(List.of("a", "b", "c")); + } + + @Configuration + @EnableMethodSecurity + @EnableAspectJAutoProxy(exposeProxy = true) + static class TestConfig { + + @Bean + Service service(ObjectProvider self) { + return new Service(self); + } + + @Bean + Collaborator collaborator(Service service) { + return new Collaborator(service); + } + + } + + public static class Service { + + private final ObjectProvider self; + + private List lastSeen = List.of(); + + Service(ObjectProvider self) { + this.self = self; + } + + @PreAuthorize("hasRole('ADMIN')") + public String adminOnly() { + return "secret"; + } + + public String selfInvokes() { + return adminOnly(); + } + + public String viaSelfInjection() { + return this.self.getObject().adminOnly(); + } + + public String viaAopContext() { + return ((Service) AopContext.currentProxy()).adminOnly(); + } + + @PreAuthorize("hasRole('ADMIN')") + public final String finalAdminOnly() { + return "secret"; + } + + @PreAuthorize("hasRole('ADMIN')") + public static String staticAdminOnly() { + return "secret"; + } + + @PreAuthorize("hasRole('ADMIN')") + String packagePrivateAdminOnly() { + return "secret"; + } + + @PreFilter("filterObject.owner == authentication.name") + public void consume(List accounts) { + this.lastSeen = List.copyOf(accounts); + } + + public List lastSeen() { + return this.lastSeen; + } + + @PostFilter("filterObject.owner == authentication.name") + public List immutableResults() { + return List.of(new Account(1, "alice", 100), new Account(2, "bob", 200), new Account(3, "alice", 300)); + } + + @PostAuthorize("returnObject.size() == 3") + @PostFilter("filterObject != 'c'") + public List expectsThree(List in) { + return in; + } + + @PostAuthorize("returnObject.size() == 2") + @PostFilter("filterObject != 'c'") + public List expectsTwo(List in) { + return in; + } + + @PreAuthorize("#owner == authentication.name") + public String byParameterName(String owner) { + return "ok"; + } + + } + + public static class Collaborator { + + private final Service service; + + Collaborator(Service service) { + this.service = service; + } + + public String callsAdminOnly() { + return this.service.adminOnly(); + } + + } + + /** Referenced only to keep the import list honest about what the chain looks like. */ + interface UnusedMarker extends MethodInvocation { + + } + + /** Same, for the standalone context used by the demos. */ + static AnnotationConfigApplicationContext unusedContextFactory() { + return new AnnotationConfigApplicationContext(); + } + +}