ali-jawad opened a new issue, #16739:
URL: https://github.com/apache/grails-core/issues/16739
### Expected Behavior
As on Grails 7.2.x, and as documented for
`grails.plugin.springsecurity.fii.observeOncePerRequest` (default `true`), the
`filterInvocationInterceptor` authorizes a request once. Later dispatches of
the same request, such as the ERROR dispatch that renders its error page, are
not authorized again.
An unauthenticated request to a public action that answers 404 gets the 404
error page. A direct request to `/error` is still authorized against its own
rule.
### Actual Behaviour
On 8.0.0 every dispatch is authorized again, ERROR dispatches included. The
security filter chain is registered for `REQUEST` and `ERROR` dispatches, so
the ERROR dispatch to `/error` is checked against `/error`'s rule. When
`/error` is not public, the anonymous caller is denied and
`ExceptionTranslationFilter` sends a 302 to the login page.
In our application, after upgrading from 7.2.4 to 8.0.0, every container
error page (404, 410, 413) sent to an unauthenticated caller became a redirect
to the login page. The compat interceptor never reads
`fii.observeOncePerRequest`, so the setting has no effect.
### Steps To Reproduce
1. Create a Grails 8.0.0 app with the Spring Security plugin:
```
sdk install grails 8.0.0
grails -t forge create-app example
cd example
```
Add `implementation "org.apache.grails:grails-spring-security"` to
`dependencies` in `build.gradle`, then run `./grailsw s2-quickstart example
User Role`. It generates `User`, `Role` and `UserRole` in
`grails-app/domain/example/` and the security settings in
`grails-app/conf/application.groovy`. `grails -t forge create-app example
--features=grails-spring-security` generates the equivalent without the
quickstart.
For the comparison, repeat with `sdk install grails 7.2.4`. Its Forge has
no `grails-spring-security` feature; the BOM resolves the plugin to 7.2.4.
2. In `grails-app/conf/application.groovy`, delete the `/error` entry
(`[pattern: '/error', access: ['permitAll']],`) from
`grails.plugin.springsecurity.controllerAnnotations.staticRules`. With
`rejectIfNoRule = true` (the default), `/error` is then denied.
3. Add `grails-app/controllers/example/PublicController.groovy`:
```groovy
package example
import grails.plugin.springsecurity.annotation.Secured
@Secured('permitAll')
class PublicController {
def missing() {
response.sendError(404)
}
}
```
4. Start the app with `./gradlew bootRun`, then `curl -i
http://localhost:8080/public/missing`:
- Grails 7.2.4: `404` with the application's not-found page (`<h1>Error:
Page Not Found (404)</h1>`)
- Grails 8.0.0: `302`, `Location: http://localhost:8080/login/auth`,
empty body
5. `curl -i http://localhost:8080/error` answers `302` with `Location:
http://localhost:8080/login/auth` on both versions, which is expected: it is
that request's first dispatch.
With the `/error` entry kept, step 4 answers `404` on 8.0.0 too.
The `/error` rule that the quickstart
([s2-quickstart.groovy#L229](https://github.com/apache/grails-core/blob/081cd1c93ab753a234e111e2828f089ebdd810ee/grails-spring-security/plugin/src/main/scripts/s2-quickstart.groovy#L229)),
the Forge `grails-spring-security` feature and the functional test app
([application.groovy#L31](https://github.com/apache/grails-core/blob/e4b22127dc63341050a290d8c1d738c7df2b2ff5/grails-test-examples/spring-security/core/functional-test-app/grails-app/conf/application.groovy#L31))
all ship hides the problem. Applications that never permitted `/error`,
because they relied on once-per-request semantics, are affected.
### Cause
Spring Security 7 removed `FilterSecurityInterceptor`, and Grails 8 ships a
replacement in `grails-spring-security-compat`, in Spring's own package. The
plugin still wires `observeOncePerRequest` into it from
`fii.observeOncePerRequest`
([SpringSecurityCoreGrailsPlugin.groovy#L399-L410](https://github.com/apache/grails-core/blob/081cd1c93ab753a234e111e2828f089ebdd810ee/grails-spring-security/plugin/src/main/groovy/grails/plugin/springsecurity/SpringSecurityCoreGrailsPlugin.groovy#L399-L410),
default `true` in
[DefaultSecurityConfig.groovy#L333-L340](https://github.com/apache/grails-core/blob/081cd1c93ab753a234e111e2828f089ebdd810ee/grails-spring-security/plugin/grails-app/conf/DefaultSecurityConfig.groovy#L333-L340)).
The compat `AbstractSecurityInterceptor` declares the property
([AbstractSecurityInterceptor.groovy#L46](https://github.com/apache/grails-core/blob/081cd1c93ab753a234e111e2828f089ebdd810ee/grails-spring-security/compat/src/main/groovy/org/springframework/security/acc
ess/intercept/AbstractSecurityInterceptor.groovy#L46)). But the compat
`doFilter` never reads it and authorizes every dispatch
([FilterSecurityInterceptor.groovy#L41-L58](https://github.com/apache/grails-core/blob/081cd1c93ab753a234e111e2828f089ebdd810ee/grails-spring-security/compat/src/main/groovy/org/springframework/security/web/access/intercept/FilterSecurityInterceptor.groovy#L41-L58)).
The same holds on the current `8.0.x` after the #16632 fix
([L42-L62](https://github.com/apache/grails-core/blob/e4b22127dc63341050a290d8c1d738c7df2b2ff5/grails-spring-security/compat/src/main/groovy/org/springframework/security/web/access/intercept/FilterSecurityInterceptor.groovy#L42-L62)).
The chain is registered for `ERROR` and `REQUEST` dispatches
([SpringSecurityCoreGrailsPlugin.groovy#L230-L235](https://github.com/apache/grails-core/blob/081cd1c93ab753a234e111e2828f089ebdd810ee/grails-spring-security/plugin/src/main/groovy/grails/plugin/springsecurity/SpringSecurityCoreGrailsPlugin.groovy#L230-L235)),
so the error page's dispatch reaches the interceptor.
Grails 7.2.4 has the same registration and wiring ([7.2.4
L399-L410](https://github.com/apache/grails-core/blob/ea4ddc768f362a77fceb926ced14491c18524ca3/grails-spring-security/plugin/src/main/groovy/grails/plugin/springsecurity/SpringSecurityCoreGrailsPlugin.groovy#L399-L410)).
It worked there because Spring Security 6's `FilterSecurityInterceptor`
honours the flag: it marks the request with `FILTER_APPLIED` and skips
authorization on later dispatches ([Spring Security 6.5.11
FilterSecurityInterceptor.java#L104-L128](https://github.com/spring-projects/spring-security/blob/73b077790fcb04ac3712033d3e939daf42264545/web/src/main/java/org/springframework/security/web/access/intercept/FilterSecurityInterceptor.java#L104-L128)).
### Suggested fix
Honour `observeOncePerRequest` the way Spring Security 6 did, using the same
request attribute. Mark the request before authorizing, so the ERROR dispatch
of a denied request (e.g. a 403 error page) is not authorized again either:
```groovy
private static final String FILTER_APPLIED =
'__spring_security_filterSecurityInterceptor_filterApplied'
@Override
void doFilter(ServletRequest request, ServletResponse response, FilterChain
chain) throws IOException, ServletException {
if (observeOncePerRequest && request.getAttribute(FILTER_APPLIED) !=
null) {
chain.doFilter(request, response)
return
}
if (observeOncePerRequest) {
request.setAttribute(FILTER_APPLIED, Boolean.TRUE)
}
// existing body unchanged
}
```
Tests that would cover it:
- `FilterSecurityInterceptorSpec`:
- a second dispatch of the same request does not call `decide`
- the ERROR dispatch after a denied first dispatch does not call `decide`
- with `observeOncePerRequest = false`, the second dispatch calls `decide`
again
- `grails-test-examples/spring-security/core/functional-test-app`: an
environment without the `/error` rule in which an unauthenticated
`sendError(404)` from a public action answers 404, while a direct `GET /error`
still redirects to the login page.
The same bean is also wired with `alwaysReauthenticate`,
`publishAuthorizationSuccess`, `runAsManager` and `afterInvocationManager`, and
the compat `doFilter` uses none of them. That may deserve a separate look; this
issue is only about `observeOncePerRequest`.
### Workaround
Replace the interceptor's class through the plugin's `<beanName>BeanClass`
override
([BeanTypeResolver.groovy#L48-L54](https://github.com/apache/grails-core/blob/081cd1c93ab753a234e111e2828f089ebdd810ee/grails-spring-security/plugin/src/main/groovy/grails/plugin/springsecurity/BeanTypeResolver.groovy#L48-L54)).
The plugin keeps wiring all its properties. In
`src/main/groovy/example/OncePerRequestFilterSecurityInterceptor.groovy`:
```groovy
package example
import jakarta.servlet.FilterChain
import jakarta.servlet.ServletException
import jakarta.servlet.ServletRequest
import jakarta.servlet.ServletResponse
import
org.springframework.security.web.access.intercept.FilterSecurityInterceptor
class OncePerRequestFilterSecurityInterceptor extends
FilterSecurityInterceptor {
private static final String FILTER_APPLIED =
'__spring_security_filterSecurityInterceptor_filterApplied'
@Override
void doFilter(ServletRequest request, ServletResponse response,
FilterChain chain) throws IOException, ServletException {
if (observeOncePerRequest && request.getAttribute(FILTER_APPLIED) !=
null) {
chain.doFilter(request, response)
return
}
if (observeOncePerRequest) {
request.setAttribute(FILTER_APPLIED, Boolean.TRUE)
}
super.doFilter(request, response, chain)
}
}
```
```groovy
// grails-app/conf/application.groovy
grails.plugin.springsecurity.filterInvocationInterceptorBeanClass =
example.OncePerRequestFilterSecurityInterceptor
```
Permitting `/error` also hides the symptom, but it makes a direct `GET
/error` public as well.
### Environment Information
- Grails 8.0.0 (Spring Boot 4.1.1, Spring Security 7.1.1, Tomcat 11.0.24,
Groovy 5.1.3), Java 21, Gradle 9.8.0
- Works on Grails 7.2.4 (Spring Boot 3.5.16, Spring Security 6.5.11, Tomcat
10.1.55, Groovy 4.0.33)
### Example Application
None; the steps above create a minimal one from the Forge templates.
### Version
8.0.0
Related: #16632 / #16654 (same compat class, missing null-`Authentication`
guard).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
