#37085 include in 25.07.10 LTS — backport SAML NameID serialization API - #37089
Open
fabrizzio-dotCMS wants to merge 1 commit into
Open
#37085 include in 25.07.10 LTS — backport SAML NameID serialization API#37089fabrizzio-dotCMS wants to merge 1 commit into
fabrizzio-dotCMS wants to merge 1 commit into
Conversation
Backport the SAML NameID serialization API from #34700 (PR #35122) so the LTS line can run the Maven-built SAML bundle that carries BouncyCastle 1.85. - Add com.dotcms.saml.SamlNameID: a serializable, OpenSAML-free holder for a NameID's XML string. It must live on the webapp classpath so Tomcat's session deserializer can resolve it; plugin bundle classes are invisible to that classloader. - Narrow Attributes.nameID (field, getter and builder) from Object to Serializable. This is not cosmetic: the bundle's bytecode references Attributes.getNameID()Ljava/io/Serializable; and Attributes$Builder.nameID(Ljava/io/Serializable;), so shipping SamlNameID alone would still fail with NoSuchMethodError on the SAML authentication path. - Update SAMLHelperTest to build NameIDs through the new type. No production code in core reads the narrowed members with an incompatible type: all eight call sites in SAMLHelper and DotSamlResource pass the value into Object-typed parameters or back into the builder. SAMLHelper is byte-identical to main. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
13 tasks
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
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.
Proposed Changes
Backport of the SAML NameID serialization API from #34700 (PR #35122) to the 25.07.10 LTS line. This is the enabling step for #37085: it lets the LTS line run the Maven-built SAML bundle that carries BouncyCastle 1.85.
com.dotcms.saml.SamlNameID— a serializable, OpenSAML-free holder for a NameID's XML string. It has to live on the webapp classpath so Tomcat's session deserializer can resolve it; plugin bundle classes are invisible to that classloader. File is byte-identical tomain.Attributes.nameID(field, getter, and both builder members) fromObjecttoSerializable.SAMLHelperTestto build NameIDs through the new type.hotfix_tracking.md.Why the
Attributeschange is required, not cosmeticShipping
SamlNameIDalone would still fail at runtime. The SAML bundle is compiled against the newer core, so its bytecode references:The LTS core declares both as
Object, so the bundle would throwNoSuchMethodErroron the SAML authentication path. Compiling the bundle's source against the LTS jar does not surface this — only the shipped descriptors do.Verification
dotcms-core-25.07.10_lts_v17. Three real gaps: theSamlNameIDclass and the two descriptors above. One false positive,WebInterceptorDelegate.addFirst, which is inherited fromWebInterceptorAwareand is present in LTS with the exact descriptor.javac --release 11clean on both changed classes — no Java 11 target violations.nameIDcall sites inSAMLHelperandDotSamlResourcepass the value intoObject-typed parameters or back into the builder; none break under the narrowing.SAMLHelper.javais byte-identical between this branch andmain, somain's green build covers those call sites..toString()on the NameID — worth stating explicitly becauseSamlNameID.toString()is redacted, and hashing that redacted string would collide every SAML user onto one ID.Checklist
SAMLHelperTestupdated and carried over from fix: serialize SAML NameID for Redis session storage #34700 #35122SamlNameIDcarries PII (the SAML identity value). ItstoString()is redacted andreadObjectenforces non-blank plus length caps so a corrupt or hostile Redis session entry cannot exhaust heap. Behavior is unchanged from what already ships onmain.Additional Info
Not yet covered by any of the above: a real SAML login + logout smoke test on an LTS image. That is a QA step.
This PR does not bump
com.dotcms.samlbundleinosgi-base/system-bundles/pom.xml. That happens once the LTS-targeted bundle version is published and its number is agreed with release.Related to #37085.