Compatibility with Spring Boot 4#1214
Conversation
jdaugherty
left a comment
There was a problem hiding this comment.
I absolutely love the clean up on the tests, but can you help me understand the background for the compat library?
| } | ||
|
|
||
| @CompileStatic | ||
| class SecurityConfig implements ConfigAttribute { |
There was a problem hiding this comment.
Newer versions of spring security removed the centralized config and split them up. This can capture stuff on the grails side, but what if they configured spring security defaults using the spring configuration keys? That config won't make it into this centralized configuration.
@jdaugherty The Longer term might want to look at other solutions. |
From my understanding, the compat library is a shim then. I don't think this is a complete solution if that's the intent. Spring split the configuration into separate namespaces so while it continues to work with this change, if you configure it in the shared config, it doesn't get passed to Spring Security. Isn't that a major problem? |
We do not need to exclude any autoconfigurations as spring boot 7 has extracted them to specific starter modules that we don't use.
Parent `DefaultAuthenticationEventPublisher` creates its own no-op publisher when constructed without arguments.
Restores the auto-configuration excluder removed in 0c2e328 with the correct Spring Boot 4 class names. Spring Boot 4 split the security auto-configurations into the spring-boot-security and spring-boot-security-oauth2-client modules and re-packaged them under `org.springframework.boot.security.autoconfigure.*` and `org.springframework.boot.security.oauth2.client.autoconfigure.*`. A user who explicitly adds spring-boot-starter-security or spring-boot-starter-oauth2-client would otherwise re-introduce a parallel SecurityFilterChain stack alongside the Grails plugin's own filter chain. Addresses jdaugherty's review feedback on PR #1214 about the plugin's "centralized" security configuration not composing with Spring's split namespaces. The mitigation here is mutual exclusion of the two stacks rather than property bridging: the plugin owns the security config and the excluder prevents Boot's auto-config from doubling it up. Class names were verified against the META-INF/spring/org.springframework.boot.autoconfigure.AutoConfiguration.imports files in spring-boot-security-4.0.5.jar and spring-boot-security-oauth2-client-4.0.5.jar. Also updates SpringSecurityBeanFactoryPostProcessor.AUTOCONFIG_NAME to the SB4 location of SecurityFilterAutoConfiguration so the belt-and-suspenders bean cleanup keeps working. Updates README.md to provide both Grails 8 / SB4 and Grails 7 / SB3 manual fallback exclusion examples. Assisted-by: claude-code:claude-4.6-opus
- Adds class-level Groovydoc to MutableRoleHierarchy explaining why the class exists (RoleHierarchyImpl became immutable in Spring Security 6 and only exposes a static factory) and how its setHierarchy contract works (per jdaugherty's request to document the class). - Updates spring-security-compat module pomDescription per matrei's agreed rephrasing: 'Compatibility classes that make Grails Spring Security 8 work with newer Spring Security versions, such as 7 and later.' - Adds an attribution comment to the AbstractSecurityInterceptor compat shim noting it is based on the class of the same name in Spring Security (removed in 7), matching the precedent set by WebExpressionVoter and FilterProcessUrlRequestMatcher in this codebase. Assisted-by: claude-code:claude-4.6-opus
…dit GSPs
Replaces the bare <input type="submit"> markup in the two outliers with
<g:submitButton ... /> to match the rest of the functional-test-app GSPs
(create.gsp variants already use g:submitButton). The rendered HTML is
equivalent (g:submitButton emits an <input type="submit">) so the Geb
$('input', value: 'Update') page selectors in EditRolePage and
EditRequestmapPage are unaffected.
Per jdaugherty's PR #1214 review note about consistent submit-button
markup.
Assisted-by: claude-code:claude-4.6-opus
…den contract
Multi-agent post-implementation review identified gaps in the initial
SB4 port. This commit:
- Adds 4 missing SB4 servlet auto-configuration exclusions verified
against actual jar contents (`META-INF/spring/org.springframework.boot.
autoconfigure.AutoConfiguration.imports`):
* spring-boot-security-oauth2-resource-server: OAuth2ResourceServerAutoConfiguration
* spring-boot-security-saml2: Saml2RelyingPartyAutoConfiguration
* spring-boot-security-oauth2-authorization-server: OAuth2AuthorizationServerAutoConfiguration,
OAuth2AuthorizationServerJwtAutoConfiguration
- Documents the configuration contract in both class-level Javadoc and
README/installation.adoc: while the plugin is active,
`grails.plugin.springsecurity.*` is the authoritative configuration
source and `spring.security.*` is not bridged. This addresses
jdaugherty's PR #1214 review concern about the two namespaces not
being combined.
- Adds a startup WARN log via @slf4j when `excludeSpringSecurityAuto
Configuration=false`, so users do not silently lose the plugin's
security guarantees.
- Restores @SInCE 7.0.2 (the version the original excluder was first
introduced in via PR #1205) instead of 8.0.0.
- Retabs the new Groovy files to match repo convention (tabs, not
spaces).
- Adds @unroll spec coverage for the 4 new exclusions and for the SB4
reactive variants which we intentionally let pass through (servlet
plugin only).
README.md and installation.adoc list all 11 manually-equivalent
exclusions and call out the configuration contract and the WARN-on-disable
behaviour.
Assisted-by: claude-code:claude-4.6-opus
Multi-agent post-implementation review pointed out that the original integration assertions were too weak - `SecurityFilterChain` count `<= 1` and `UserDetailsService` count `>= 1` would silently pass even if Boot security auto-configurations sneaked through and registered their own beans. This commit replaces the count-based assertions with concrete checks: - For every excluded auto-configuration class name (the 11-entry list in SecurityAutoConfigurationExcluder.EXCLUDED_AUTO_CONFIGURATIONS), assert that `applicationContext` does not contain a bean definition for that class. - Assert that no SecurityFilterChain bean has a class name starting with `org.springframework.boot.security.` (i.e. came from Boot's auto-config). - Assert that no UserDetailsService bean has a class name starting with `org.springframework.boot.security.`, and that Boot's `inMemoryUserDetailsManager` bean name in particular is absent. These are still trivial-pass assertions while the integration-test-app does not include any spring-boot-security* module on its classpath; the follow-up to add a Boot security starter to that test app and exercise the real exclusion path is left as a separate task because it touches the example app's runtime dependency graph. Also retabs to match repo convention (tabs, not spaces). Assisted-by: claude-code:claude-4.6-opus
Spring Boot 4 / Grails 8 SNAPSHOT dependencies pulled in classes that
reference `org.jline.reader.LineReader` (jline 3.x), but the
plugin-core compile classpath only had `jline:jline:2.14.6` (jline 2.x,
package `jline.console`). Groovy's static type checker fails to
canonicalize types when it encounters a reference to a class whose
required types are not on the classpath:
General error during canonicalization:
java.lang.NoClassDefFoundError: org.jline.reader.LineReader
This regression was not caused by PR #1214 or this PR; the same failure
reproduces when CI is re-run against the unmodified `grails8-sb4` HEAD
(see workflow run 24935487319). Older successful runs were masked by
Gradle build caching of pre-regression compile outputs.
Adding `compileOnly 'org.jline:jline-reader:3.30.9'` makes the
required types available to the compile classpath while keeping it out
of the runtime classpath of plugin consumers (their own Groovy/Grails
distribution provides jline at runtime).
Verified locally: `./gradlew :core-plugin:check --max-workers=2 --continue`
BUILD SUCCESSFUL with all unit tests passing.
Assisted-by: claude-code:claude-4.6-opus
… oauth2-plugin Follow-up to the previous CI fix to align with the project's dependency- management convention: - Defines `jlineReaderVersion=3.30.9` in `gradle.properties` alongside the other version properties (unboundidLdapSdkVersion, nimbusVersion, scribejavaVersion, etc.) since `org.jline:jline-reader` is not managed by any BOM on the compile classpath (grails-bom, spring-boot- bom, groovy-bom none provide it). - Updates `plugin-core/plugin/build.gradle` to use "org.jline:jline-reader:$jlineReaderVersion" instead of the hardcoded version. - Applies the same fix to `plugin-oauth2/plugin/build.gradle`, which hit the identical `org.jline.reader.LineReader` `NoClassDefFoundError` during `./gradlew check`. Verified locally: `./gradlew :core-plugin:compileGroovy :oauth2-plugin:compileGroovy --rerun-tasks` BUILD SUCCESSFUL. Assisted-by: claude-code:claude-4.6-opus
…guration Spring Security 5.7 deprecated and Spring Security 6 removed WebSecurityConfigurerAdapter, replacing it with the component-based configuration model described in https://spring.io/blog/2022/02/21/spring-security-without-the-websecurityconfigureradapter This plugin pre-dates that model and provides equivalent functionality through the `grails.plugin.springsecurity.*` namespace, but the behaviour when users define those component beans alongside the plugin was previously undocumented. This commit adds an explicit coexistence matrix to the SecurityAutoConfigurationExcluder Javadoc, README.md, and installation.adoc covering each pattern from the Spring blog: - `@Bean SecurityFilterChain` is created but never services requests; use `grails.plugin.springsecurity.filterChain.chainMap` / `filterNames` and `staticRules` instead. - `@Bean WebSecurityCustomizer` is a no-op (the plugin does not use Spring's `WebSecurity` builder); use `staticRules` with `permitAll` or `ipRestrictions` instead. - `@Bean AuthenticationManager` conflicts with the plugin's `authenticationManager` (`ProviderManager`) bean; register custom `AuthenticationProvider` beans and add their names to `grails.plugin.springsecurity.providerNames`. - `@Bean InMemoryUserDetailsManager` / `JdbcUserDetailsManager` coexists in the context but is unused; configure `grails.plugin.springsecurity.userLookup.userDomainClassName` or replace the `userDetailsService` bean. - LDAP factory beans (`EmbeddedLdapServerContextSourceFactoryBean`, `LdapBindAuthenticationManagerFactory`, `LdapPasswordComparisonAuthenticationManagerFactory`) coexist but are not wired into the plugin's authentication providers; use the `grails-spring-security-ldap` plugin and the `grails.plugin.springsecurity.ldap.*` configuration. upgrading7x.adoc now links to the Spring blog post and references the new coexistence section, so users migrating from Spring Security 5.x WebSecurityConfigurerAdapter usage know what their custom beans will (and will not) do under this plugin. Verified locally: `./gradlew :docs:asciidoctor` BUILD SUCCESSFUL with the full coexistence table rendered in the generated HTML. Assisted-by: claude-code:claude-4.6-opus
Per Copilot PR #1215 review feedback (comments 3142335908 and 3142335915): - `ApplicationContext.getBeanFactory()` is not part of the `ApplicationContext` API; the previous code relied on Groovy dynamic dispatch / a specific context implementation. - Switch the autowired field type to `ConfigurableApplicationContext` and call `getBeanDefinition(...)` / `containsBeanDefinition(...)` on the returned `ConfigurableListableBeanFactory`. This makes the required Spring API explicit at the source level. Verified locally: `./gradlew :core-examples-integration-test-app:compileIntegrationTestGroovy` BUILD SUCCESSFUL. Assisted-by: claude-code:claude-4.6-opus
… Spring Security beans Adds `grails.plugin.springsecurity.componentbased.ComponentBasedConfigBlender` and `ChainedUserDetailsService` to make the Grails plugin's `grails.plugin.springsecurity.*` configuration coexist with the component-based Spring Security configuration model recommended by https://spring.io/blog/2022/02/21/spring-security-without-the-websecurityconfigureradapter Whether security is configured via Grails plugin keys, via Spring Security component beans, or both, the effective configuration is now the union of both sources rather than the plugin silently winning. Blending behaviour (each enabled by default, opt-out via `grails.plugin.springsecurity.componentBased.<flag>: false`): - `@Bean SecurityFilterChain` -> auto-merged into the plugin's `FilterChainProxy`. User chains are *prepended* (higher precedence) so their typically more-specific request matchers win against the plugin's catch-all chain. Opt-out: `autoMergeSecurityFilterChain`. - `@Bean AuthenticationProvider` -> auto-merged into the plugin's `authenticationManager` (`ProviderManager`). User providers are *appended* so the plugin's primary GORM-backed provider runs first; providers already declared via `providerNames` are not re-added. Opt-out: `autoMergeAuthenticationProviders`. - `@Bean InMemoryUserDetailsManager` / `JdbcUserDetailsManager` / any extra `UserDetailsService` -> auto-chained behind the plugin's primary `GormUserDetailsService` via `ChainedUserDetailsService`. The plugin's GORM lookup runs first; if it throws `UsernameNotFoundException`, each additional bean is queried in bean-name order. The chained service is wired into `daoAuthenticationProvider`. Opt-out: `autoChainUserDetailsServices`. - `spring.security.user.name` / `spring.security.user.password` / `spring.security.user.roles` -> if `spring.security.user.name` is set, an `InMemoryUserDetailsManager` is created from those properties (mimicking what Spring Boot's `UserDetailsServiceAutoConfiguration` would have done) and chained behind the plugin's primary user lookup. Opt-out: `bridgeSpringSecurityUserProperties`. `@Bean WebSecurityCustomizer` is still a no-op (the plugin does not use Spring's `WebSecurity` builder); use `staticRules` with `permitAll` or `ipRestrictions` instead. `@Bean AuthenticationManager` still conflicts with the plugin's `authenticationManager` bean by name; use `@Bean AuthenticationProvider` (auto-merged) or `grails.plugin.springsecurity.providerNames` instead. Wired into `SpringSecurityCoreGrailsPlugin.doWithApplicationContext` after the plugin's own filter chains and authentication providers are populated, so blending sees the final plugin state. `SecurityAutoConfigurationExcluder` Javadoc, `README.md` and `installation.adoc` updated to document the new blending behaviour (replacing the earlier "not blended" coexistence notes). Tests: 10 new unit tests in `ComponentBasedConfigBlenderSpec` covering prepend/append ordering, idempotent dedup, chained UDS query order, `UsernameNotFoundException` propagation, and the `spring.security.user.*` property bridge. Verified locally: `./gradlew :core-plugin:check` and `./gradlew :docs:asciidoctor` BUILD SUCCESSFUL. Assisted-by: claude-code:claude-4.6-opus
… mutating the existing one
The previous commit attempted to wire a chained UserDetailsService into
the existing `daoAuthenticationProvider` via property assignment. That
worked in pre-Spring-Security-7 but Spring Security 7 made
`DaoAuthenticationProvider.userDetailsService` a final
constructor-only field, so the assignment fails at startup with:
groovy.lang.ReadOnlyPropertyException: Cannot set readonly property:
userDetailsService for class:
org.springframework.security.authentication.dao.DaoAuthenticationProvider
This caused `ldap-examples-functional-test-app:integrationTest` to fail
the application context startup (the LDAP example app exposes an
`ldapUserDetailsService` bean which the blender then tried to chain).
Fix: instead of mutating the existing `daoAuthenticationProvider`,
create a NEW `DaoAuthenticationProvider` per additional
`UserDetailsService` (each preserving the existing application
`passwordEncoder` if present) and append them to the
`authenticationManager` providers list. The plugin's primary
GORM-backed provider remains first, so the GORM lookup still wins for
known users; if that throws an authentication exception, each
additional provider is tried in turn.
Same observable behaviour, no readonly-field mutation. Verified locally:
./gradlew :ldap-examples-functional-test-app:integrationTest --rerun-tasks
-> BUILD SUCCESSFUL
Docs (Javadoc, README, installation.adoc) updated to describe the
per-UDS-provider approach and to call out the final-field constraint
that motivated it.
Assisted-by: claude-code:claude-4.6-opus
Address review feedback on PR #1214 (Spring Boot 4 compat)
|
@matrei From discussion with @jamesfredley on his other PR, I think we can move forward with this - we've documented that any spring security related defaults just won't be set now & that only the grails config names will be used. We'll likely have some regressions because of this, but we can at least get the basic case working for the milestone / Grails 8 |
OK, great! Approve and merge at will! |
Copilot generated PR description:
This pull request introduces several significant updates to modernize the codebase, improve test reliability, and enhance maintainability, especially around the functional test infrastructure and build configuration. Key changes include upgrading to Java 21, updating project versions, refactoring and expanding the functional test pages, and simplifying test configuration for better stability.
Build and Environment Upgrades:
.github/workflows/gradle.yml,.github/workflows/rat.yml,.github/workflows/release.yml,.sdkmanrc, andgradle.propertiesto ensure the project builds and tests with the latest LTS Java, and synchronized Grails and project versions to 8.0.0-SNAPSHOT. [1] [2] [3] [4] [5] [6] [7] [8] [9]-PgebAtCheckWaitingproperty from Gradle commands and enabledatCheckWaitingand timeouts for Geb tests directly ingradle/test-config.gradlefor improved CI reliability. [1] [2] [3]Functional Test Improvements:
AccessDeniedPage,DeleteReportPage,LogoutPage,ResetDataPage), improved URL handling, and enhanced content selectors and utility methods for better test clarity and maintainability. [1] [2] [3] [4] [5] [6] [7] [8] [9] [10]ErrorsController.groovyto provide more accurate 404 error messages by correctly resolving the request URI.Test Code Refactoring:
AbstractSecuritySpecto use the new page objects and navigation methods, improving test setup and login/logout reliability.AdminFunctionalSpecby adopting new page objects, using Spock's@Unrollfor parameterized tests, and updating test assertions for better readability and maintainability. [1] [2]Build/Publishing Configuration:
spring-security-compatto the list of published projects ingradle/publish-root-config.gradle.These changes collectively modernize the codebase, improve test reliability and maintainability, and ensure compatibility with the latest Java and Grails versions.