GRAILSPLUGINS-2505#1
Closed
marcusb wants to merge 1 commit into
Closed
Conversation
This is so that it can be initialized multiple times, which happens if both integration and functional test phases are run. Signed-off-by: Marcus Better <marcus@better.se>
Contributor
|
This was fixed for the 1.1 release (the issue # is now http://jira.grails.org/browse/GPSPRINGSECURITYCORE-42) |
jamesfredley
added a commit
to jamesfredley/grails-spring-security
that referenced
this pull request
May 22, 2026
…ded evidence Investigated all 22 PMC open questions from the §14 ratification gate using parallel code-evidence agents across plugin-core, plugin-rest, plugin-oauth2, plugin-cas, plugin-ldap, plugin-ui, spring-security-compat, and external references (Nimbus JOSE+JWT canonical docs, upstream Spring Security 5.x RunAsManagerImpl at SHA 4942459, ASF threat-model rubric). §14 reshaped from "open questions" to "Ratification log". Every question has a resolution paragraph with file:line citations. Provenance tags promoted from (inferred) to (maintainer) for 18 inline claims tied to the 22 resolved questions. Wave 1 (scope/intended use) - resolved: - Q1: Five caller roles in §2 are correct primitives; anonymous-token posture is a sub-state of unauthenticated, not a separate principal. - Q2: UI plugin endpoints ship unprotected by design - zero @secured across all 13 controllers, zero default Requestmap rows, zero authz wiring. - Q3: alg=none acceptance confirmed reachable when operator sets useSignedJwt:false (default true) + useEncryptedJwt:false + null secret + null keyProvider. JwtService.groovy:71-76 PlainJWT branch falls through. - Q4: cas.useSingleSignout:true is default and forces useSessionFixationPrevention=false. SpringSecurityCasGrailsPlugin.groovy:85-89. - Q5: security.ui.encodePassword:false default is intentional - s2-quickstart generates Person with PasswordEncoderListener that hashes on beforeInsert/beforeUpdate; flipping default would double-hash. - Q6: bcrypt.logrounds=10 is the production default (matches Spring Security upstream); 4 is test-only. §10 apache#1 ≥12 hardening recommendation stands. - Q7: spring.security.user.name bridging is intentional; the {noop} prefix mirrors Spring Boot's UserDetailsServiceAutoConfiguration semantics. Wave 2 (trust boundaries) - resolved: - Q8: IpAddressFilter uses request.remoteAddr only (line 109,130) - BY-DESIGN: property-disclaimed. - Q9: PortResolverImpl uses request.serverPort only (lines 31-33) - BY-DESIGN: property-disclaimed. SecurityRequestHolderFilter:83-113 asymmetry now explicitly documented in §9 false-friend. - Q10: OAuth2 state uses java.util.Random (line 103) - VALID-HARDENING. - Q11: PKCE absent - ServiceBuilder.withPkce() never invoked - VALID-HARDENING. - Q12: Refresh-token reused verbatim (RestOauthController.groovy:173-181) - BY-DESIGN; operators mitigate via short refreshExpiration. - Q13: JwtTokenStorageService.removeToken throws unconditionally (lines 124-128); RestLogoutFilter returns HTTP 404 - BY-DESIGN. - Q14: JwtService.parse() dispatches on Nimbus Java type, no algorithm allow-list - VALID-HARDENING. Wave 3 (misuse / §11a curation) - resolved: - Q15: Compat shim's RunAsImplAuthenticationProvider is MORE permissive than upstream Spring Security 5.x. The key field is declared in both classes but never read; upstream performs a keyHash equality check via hashCode() (verified at SHA 4942459 of spring-projects/spring-security). The §11a KNOWN-NON-FINDING stands because useRunAs:false is the default, but the shim's permissiveness is now explicitly recorded. - Q16: AclClass.className feeds Class.forName directly (GormAclLookupStrategy.groovy:296-299) - §6 trusted-input. Nuance: AclClassController ships without @secured (Q2), so the write path is unprotected unless the operator adds a rule. - Q17: Domain class Serializable implementations are standard - all 22 examples use serialVersionUID=1L with no custom readObject/writeObject - KNOWN-NON-FINDING. - Q18: AbstractS2UiDomainController.ajaxSearch concatenates params.paramName into raw HQL order-by string at doSearch line 171 with only double-quote wrapping - VALID-HARDENING. - Q19: §13 disposition table confirmed closed and complete. Meta - resolved: - Q20: Repo-root file, maintained by PMC, revised on §12 triggers; release branches fork with own version binding. - Q21: SECURITY.md is disclosure-process artifact; this file is the model; SECURITY.md already cross-references §3-§11a. - Q22: Per-plugin AsciiDoc remains end-user-facing; Appendix A back-map enumerates the citations. Code-level follow-up items identified during ratification (F1-F10), tracked as separate work items: - F1 (Q3): Startup-time rejection of useSignedJwt:false + useEncryptedJwt:false + null secret + null keyProvider combination. - F2 (Q4): Doc note in plugin-cas configuration.adoc linking useSingleSignout=true to forced useSessionFixationPrevention=false. - F3 (Q5): Startup WARN in SpringSecurityUiService.initialize() when encodePassword:false and no PasswordEncoderListener detected. - F4 (Q10): Migrate OAuth2 state from java.util.Random to SecureRandom. - F5 (Q11): Invoke ServiceBuilder.withPkce() in buildScribeService. - F6 (Q12): Doc WARNING citing RFC 6749 §10.4 on refresh-token replay. - F7 (Q14): Algorithm allow-list pinning at JwtService.parse(). - F8 (Q15): Port upstream Spring Security 5.x keyHash check into compat shim. - F9 (Q16): Allow-list / package-prefix check on AclClass.className before Class.forName. - F10 (Q18): Allow-list validation on AbstractS2UiDomainController.ajaxSearch paramName; closed-set check on params.order. threat-model.yaml updated with parallel ratification_log block, followup_items list, and updated provenance counts (22 documented / 18 maintainer / 57 inferred). §1 status promoted from "DRAFT - not yet ratified" to "DRAFT - awaiting PMC review". Assisted-by: claude-code:claude-4.7-opus
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Reset static state when initializing plugin. This is so that it can be initialized multiple times, which happens if
both integration and functional test phases are run.