You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@grails.apache.org by "jdaugherty (via GitHub)" <gi...@apache.org> on 2026/10/23 16:42:02 UTC
[PR] Honour observeOncePerRequest in the compat FilterSecurityInterceptor [grails-core]
jdaugherty opened a new pull request, #16743:
URL: https://github.com/apache/grails-core/pull/16743
## Description
Fixes #16739
Spring Security 7 removed `FilterSecurityInterceptor`, and the replacement in `grails-spring-security-compat` authorized every dispatch of a request. The plugin still wires `fii.observeOncePerRequest` (default `true`) into the bean, but the compat `doFilter` never read it, so the `ERROR` dispatch that renders an error page was authorized against the error page's own rule. An application whose rules do not permit `/error` then answered every container error page sent to an unauthenticated caller with a redirect to the login page, where Grails 7.x served the page.
The interceptor now marks the request with the attribute Spring Security 6 used, before authorizing it, and passes later dispatches of a marked request through when `observeOncePerRequest` is set. Marking before the authorization lets the error page of a denied request through as well.
### Tests
- `FilterSecurityInterceptorSpec` (plugin) gains four features: a second dispatch of an authorized request, of a public request and of a denied request, and the flag turned off. The first three fail without the fix.
- The core functional-test-app gains an `oncePerRequest` `TESTCONFIG` with no rule for `/error`, a public `SendErrorController.notFound` action, and `OncePerRequestSpec`: the 404 page of a public action and the 403 page of a denied request are served, while a direct request to `/error` and a secured URL are still authorized on their first dispatch. Both error-page features fail without the fix. The configuration leaves `adh.errorPage` unset on purpose: with the default denied page, an `ERROR` dispatch that is denied again is forwarded to `/login/denied` and Grails still renders the 403 view for the error status, which hides the regression in the 403 case.
- The new configuration is added to the `securityConfig` matrix. `OncePerRequestSpec` and `NoAnonymousFilterSpec` share `AbstractHttpClientSecuritySpec` for the user seeding, the Basic Auth credentials and the client that does not follow redirects.
### Documentation
The guide's description of `fii.observeOncePerRequest` and its configuration-metadata description now name the `ERROR` dispatch, and the guide says the skip covers every dispatcher type the chain is registered for, `REQUEST` and `ERROR` by default.
### Verification
Run locally: `FilterSecurityInterceptorSpec`, the compat module's test suite, `OncePerRequestSpec` under `-DTESTCONFIG=oncePerRequest`, `NoAnonymousFilterSpec` under `-DTESTCONFIG=noAnonymous`, `:grails-spring-security-compat:codeStyle` and `validateRepositoryConventions`. The plugin's full suite and the other `TESTCONFIG` runs are left to CI.
## Contributor Checklist
### Issue and Scope
- [x] This PR is linked to an existing issue that has been **acknowledged or approved** by the project team. If no approved issue exists, please give background on why this change is necessary. Tickets are preferred for release change log history.
- [x] This PR addresses the **complete scope** of the linked issue. Partial implementations or unfinished work should not be submitted for review.
- [x] This PR contains a **single, focused change**. Unrelated changes should be submitted as separate pull requests.
- [x] This PR targets the **correct branch** for the type of change:
- **Patch release branches** (e.g., `7.0.x`): Bug fixes only. No new features or API changes.
- **Minor release branches** (e.g., `7.1.x`): New features are welcome, but breaking existing APIs must be avoided.
- **Major release branches** (e.g., `8.0.x`): Reserved for major changes. Breaking API changes are permitted.
### Code Quality
- [x] I have **added or updated tests** that cover the changes introduced in this PR. All code contributions are expected to include appropriate test coverage.
- [ ] I have verified that all existing tests pass by running `./gradlew build --rerun-tasks`.
- [x] My code follows the project's **code style** guidelines. I have run `./gradlew codeStyle` and resolved any violations. See [Code Style](../CONTRIBUTING.md#code-style) for details.
- [x] This PR does **not** include mass reformatting, style-only changes, or large-scale refactoring unless it was **explicitly approved** in the linked issue. Unsolicited reformatting will not be accepted.
- [ ] If generative AI tooling was used in preparing this contribution, a quality model was used to ensure contributions are **consistent with the project's quality standards**.
### Licensing and Attribution
- [x] All contributed code is provided under the [Apache License 2.0](https://www.apache.org/licenses/LICENSE-2.0), and new source files include the appropriate **Apache license header**.
- [ ] I have the necessary rights to submit this contribution and confirm it is my own original work (see [Legal Notice](../CONTRIBUTING.md#i-want-to-contribute)).
- [ ] If generative AI tooling was used in preparing this contribution, I have followed the [Apache Software Foundation's policy on generative tooling](https://www.apache.org/legal/generative-tooling.html) and have properly attributed its use.
### Documentation
- [x] If this PR introduces user-facing changes, I have included or updated the relevant documentation.
- [ ] If this PR adds a new feature, I have updated the **What's New** section of the Grails Guide.
- [ ] If this PR introduces breaking changes or changes that require user action during an upgrade, I have updated the **Upgrade Notes** for the corresponding version in the Grails Guide.
- [x] The PR description clearly explains **what** was changed and **why**.
--
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: notifications-unsubscribe@grails.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Honour observeOncePerRequest in the compat FilterSecurityInterceptor [grails-core]
Posted by "codecov[bot] (via GitHub)" <gi...@apache.org>.
codecov[bot] commented on PR #16743:
URL: https://github.com/apache/grails-core/pull/16743#issuecomment-6371184222
## [Codecov](https://app.codecov.io/gh/apache/grails-core/pull/16743?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
:x: Patch coverage is `0%` with `5 lines` in your changes missing coverage. Please review.
:white_check_mark: Project coverage is 62.2094%. Comparing base ([`39daa9b`](https://app.codecov.io/gh/apache/grails-core/commit/39daa9b0ee47f2ba93a637cb583703a0b639aa21?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`47a761c`](https://app.codecov.io/gh/apache/grails-core/commit/47a761c365764a3d05b80fa932cb39b03f253125?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
:warning: Report is 63 commits behind head on 8.0.x.
| [Files with missing lines](https://app.codecov.io/gh/apache/grails-core/pull/16743?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Patch % | Lines |
|---|---|---|
| [.../access/intercept/FilterSecurityInterceptor.groovy](https://app.codecov.io/gh/apache/grails-core/pull/16743?src=pr&el=tree&filepath=grails-spring-security%2Fcompat%2Fsrc%2Fmain%2Fgroovy%2Forg%2Fspringframework%2Fsecurity%2Fweb%2Faccess%2Fintercept%2FFilterSecurityInterceptor.groovy&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-Z3JhaWxzLXNwcmluZy1zZWN1cml0eS9jb21wYXQvc3JjL21haW4vZ3Jvb3Z5L29yZy9zcHJpbmdmcmFtZXdvcmsvc2VjdXJpdHkvd2ViL2FjY2Vzcy9pbnRlcmNlcHQvRmlsdGVyU2VjdXJpdHlJbnRlcmNlcHRvci5ncm9vdnk=) | 0.0000% | [5 Missing :warning: ](https://app.codecov.io/gh/apache/grails-core/pull/16743?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
<details><summary>Additional details and impacted files</summary>
[](https://app.codecov.io/gh/apache/grails-core/pull/16743?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
```diff
@@ Coverage Diff @@
## 8.0.x #16743 +/- ##
===================================================
+ Coverage 46.0040% 62.2094% +16.2054%
- Complexity 873 26950 +26077
===================================================
Files 86 2244 +2158
Lines 4980 110824 +105844
Branches 874 20230 +19356
===================================================
+ Hits 2291 68943 +66652
- Misses 2377 32617 +30240
- Partials 312 9264 +8952
```
| [Files with missing lines](https://app.codecov.io/gh/apache/grails-core/pull/16743?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [.../access/intercept/FilterSecurityInterceptor.groovy](https://app.codecov.io/gh/apache/grails-core/pull/16743?src=pr&el=tree&filepath=grails-spring-security%2Fcompat%2Fsrc%2Fmain%2Fgroovy%2Forg%2Fspringframework%2Fsecurity%2Fweb%2Faccess%2Fintercept%2FFilterSecurityInterceptor.groovy&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-Z3JhaWxzLXNwcmluZy1zZWN1cml0eS9jb21wYXQvc3JjL21haW4vZ3Jvb3Z5L29yZy9zcHJpbmdmcmFtZXdvcmsvc2VjdXJpdHkvd2ViL2FjY2Vzcy9pbnRlcmNlcHQvRmlsdGVyU2VjdXJpdHlJbnRlcmNlcHRvci5ncm9vdnk=) | `0.0000% <0.0000%> (ø)` | |
... and [2159 files with indirect coverage changes](https://app.codecov.io/gh/apache/grails-core/pull/16743/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
</details>
<details><summary> :rocket: New features to boost your workflow: </summary>
- :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
- :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
</details>
--
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: notifications-unsubscribe@grails.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Honour observeOncePerRequest in the compat FilterSecurityInterceptor [grails-core]
Posted by "jdaugherty (via GitHub)" <gi...@apache.org>.
jdaugherty merged PR #16743:
URL: https://github.com/apache/grails-core/pull/16743
--
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: notifications-unsubscribe@grails.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org