Repository navigation
Fix for when Grails 8 returns 500 error instead of 401 for an unauthenticated request - #16654
Conversation
### Cause and fix The compatibility `FilterSecurityInterceptor` passed null authentication to authorization voters, causing an `AssertionError` instead of a handled authentication failure. It now throws `AuthenticationCredentialsNotFoundException` before invoking the decision manager. This lets the configured exception-translation filter return 401 for API entry points, while preserving login redirects and public-request behavior. ### Coverage - Reproduced the reported failure before applying the fix. - Added tests for missing authentication, HTTP 401/200/403 responses, public requests, access denial, and committed responses. - Updated filter-chain documentation.
jdaugherty
left a comment
jdaugherty
left a comment
There was a problem hiding this comment.
Let's create a functional test for this issue instead of a targeted change. Can you please reproduce and then I'll review?
…16632) A filter chain that drops anonymousAuthenticationFilter reaches the filter security interceptor with no Authentication in the security context. The core functional-test-app gains a noAnonymous TESTCONFIG whose Basic Auth and form-login chains drop that filter, and a spec that expects the configured entry point to answer such requests (401 with a Basic challenge, or a redirect to the login page), that valid Basic Auth credentials are still authorized or refused through the same chain, and that a chain which keeps the anonymous filter still serves public URLs. Against the previous compat FilterSecurityInterceptor the two unauthenticated cases fail with a 500, which is the regression the issue reports. The new configuration is added to the securityConfig matrix.
|
@jdaugherty, thank you for adding the functional test. You beat me to it! |
jdaugherty
left a comment
jdaugherty
left a comment
There was a problem hiding this comment.
The guard is the right shape. It is the check Spring's AbstractSecurityInterceptor.beforeInvocation performed and the one AuthorizationFilter still performs, and the compat MethodSecurityInterceptor already carries the identical check, so this brings the two shims in line.
Verified at b1012aa: :grails-spring-security:test (228 tests) and :grails-spring-security-compat:test (16) pass, as does codeStyle for both modules. With the 8.0.x version of FilterSecurityInterceptor.groovy swapped back in, the two "missing authentication" cases of the new spec fail with the AssertionError from the issue and the other seven pass.
I pushed the functional reproduction to this branch in f899432: a noAnonymous TESTCONFIG for the core functional-test-app whose Basic Auth and form-login chains drop anonymousAuthenticationFilter, with a spec that expects a 401 with the Basic challenge, a redirect to the login page, 200 and 403 for valid Basic credentials with and without the role, and 200 for a public URL on a chain that keeps the filter. Against the 8.0.x interceptor the two unauthenticated cases get a 500. The configuration is added to the securityConfig matrix in gradle.yml.
I will merge this once the tests finish locally.
| } | ||
|
|
||
| def authentication = SecurityContextHolder.context?.authentication | ||
| if (authentication == null) { |
There was a problem hiding this comment.
Same guard and same message as the compat MethodSecurityInterceptor, which is the consistency we want between the two shims.
Probe: with the 8.0.x version of this file, the missing authentication row of the new chain spec fails with java.lang.AssertionError: authentication cannot be null out of WebExpressionVoter.vote, the exact stack from #16632. With this change it is a 401.
2 participants
Description
Cause
The compatibility
FilterSecurityInterceptorpassed null authentication to authorization voters, causing anAssertionErrorinstead of a handled authentication failure.Fix
It now throws
AuthenticationCredentialsNotFoundExceptionbefore invoking the decision manager. This lets the configured exception-translation filter return 401 for API entry points, while preserving login redirects and public-request behavior.Coverage
Fixes #16632
Generated with GPT 6 Astra via Cline API Provider in IntelliJ IDEA 2025.3.3
Contributor Checklist
Please review the following checklist before submitting your pull request. Pull requests that do not meet these requirements may be closed without review.
Issue and Scope
7.0.x): Bug fixes only. No new features or API changes.7.1.x): New features are welcome, but breaking existing APIs must be avoided.8.0.x): Reserved for major changes. Breaking API changes are permitted.Code Quality
./gradlew build --rerun-tasks../gradlew codeStyleand resolved any violations. See Code Style for details.Licensing and Attribution
Documentation