diff --git a/dotCMS/src/main/java/com/dotcms/saml/Attributes.java b/dotCMS/src/main/java/com/dotcms/saml/Attributes.java index a55a9179f9e5..9e272e0fd4d1 100644 --- a/dotCMS/src/main/java/com/dotcms/saml/Attributes.java +++ b/dotCMS/src/main/java/com/dotcms/saml/Attributes.java @@ -31,7 +31,7 @@ public class Attributes implements Serializable { private final Object roles; // Saml object with the NameID. - private final Object nameID; + private final Serializable nameID; // SAML Session Index private final String sessionIndex; @@ -76,7 +76,7 @@ public Object getRoles() return roles; } - public Object getNameID() + public Serializable getNameID() { return nameID; } @@ -109,7 +109,7 @@ public static final class Builder { String firstName = ""; boolean addRoles = false; Object roles = null; - Object nameID = null; + Serializable nameID = null; String sessionIndex; Map additionalAttributes; @@ -149,7 +149,7 @@ public Builder roles(final Object roles) return this; } - public Builder nameID(final Object nameID) + public Builder nameID(final Serializable nameID) { this.nameID = nameID; return this; @@ -187,7 +187,7 @@ public Object getRoles() return roles; } - public Object getNameID() + public Serializable getNameID() { return nameID; } diff --git a/dotCMS/src/main/java/com/dotcms/saml/SamlNameID.java b/dotCMS/src/main/java/com/dotcms/saml/SamlNameID.java new file mode 100644 index 000000000000..0fbf7b4aad34 --- /dev/null +++ b/dotCMS/src/main/java/com/dotcms/saml/SamlNameID.java @@ -0,0 +1,136 @@ +package com.dotcms.saml; + +import java.io.IOException; +import java.io.InvalidObjectException; +import java.io.ObjectInputStream; +import java.io.Serializable; +import java.util.Objects; + +/** + * A serializable holder for a SAML NameID's XML representation. + * + *

OpenSAML's concrete NameID implementation does not implement {@link Serializable}, + * which prevents it from being stored in Redis-managed sessions. This class lives in the + * webapp classpath (core) so that Tomcat's session deserializer can always find it — + * unlike plugin bundle classes, which are invisible to the webapp classloader. + * + *

The full XML string is stored so that the plugin can reconstruct a live + * NameID on demand via {@code SamlUtils.toNameID(SamlNameID)}. + * + *

Usage in the plugin: + * 

+ *     // Store:
+ *     attrBuilder.nameID(new SamlNameID(SamlUtils.toXMLObjectString(nameID)));
+ *
+ *     // Reconstruct:
+ *     NameID nameID = SamlUtils.toNameID((SamlNameID) attributes.getNameID());
+ *
+ * + * @author jsanca + */ +public final class SamlNameID implements Serializable { + + private static final long serialVersionUID = 1L; + + /** Maximum permitted length of the XML string (guards against heap exhaustion on corrupt sessions). */ + private static final int MAX_XML_LENGTH = 8192; + + /** Maximum permitted length of the plain NameID value. */ + private static final int MAX_VALUE_LENGTH = 1024; + + private final String xmlString; + + /** The plain NameID value (email, persistent ID, etc.) for cheap retrieval without XML parsing. */ + private final String value; + + /** + * Constructs a {@code SamlNameID}. + * + * @param xmlString the full XML string of the NameID; must not be {@code null} + * @param value the plain NameID value (e.g. email or opaque ID); must not be {@code null} + */ + public SamlNameID(final String xmlString, final String value) { + this.xmlString = Objects.requireNonNull(xmlString, "xmlString must not be null"); + this.value = Objects.requireNonNull(value, "value must not be null"); + } + + /** + * Returns the XML string representation of the NameID, used to reconstruct a live + * NameID object in the SAML plugin via {@code SamlUtils.toNameID(SamlNameID)}. + * + *

Security warning: the XML string contains the user's SAML identity + * value (email address, persistent ID, etc.) and is therefore PII. It must never be + * written to logs, stored in plaintext audit trails, or returned in API responses. + * Use {@link #getValue()} only when the raw identity string is genuinely required + * (e.g. for hashing before storage), and {@link #toString()} for any diagnostic output. + * + * @return the XML string; never {@code null} + */ + public String getXmlString() { + return xmlString; + } + + /** + * Returns the plain NameID value (e.g. email address or opaque persistent ID). + * + *

Security warning: this is raw PII. Pass it only to operations that + * require the actual identity (e.g. {@code SAMLHelper.hashIt()}) — never write it directly + * to logs or API responses. If a correlation token is needed for debugging, hash the value + * first and use only the first few characters of the hash. + * + * @return the NameID value; never {@code null} + */ + public String getValue() { + return value; + } + + /** + * Validates invariants after Java deserialization. + * + *

Length caps are enforced here in addition to the null/blank checks because + * {@code defaultReadObject()} can restore arbitrarily large strings from a corrupt or + * malicious Redis entry, potentially exhausting heap memory before any application code + * runs. + */ + private void readObject(final ObjectInputStream in) throws IOException, ClassNotFoundException { + in.defaultReadObject(); + if (xmlString == null || xmlString.isBlank()) { + throw new InvalidObjectException("SamlNameID: xmlString must not be null or blank"); + } + if (xmlString.length() > MAX_XML_LENGTH) { + throw new InvalidObjectException("SamlNameID: xmlString exceeds maximum length of " + MAX_XML_LENGTH); + } + if (value == null || value.isBlank()) { + throw new InvalidObjectException("SamlNameID: value must not be null or blank"); + } + if (value.length() > MAX_VALUE_LENGTH) { + throw new InvalidObjectException("SamlNameID: value exceeds maximum length of " + MAX_VALUE_LENGTH); + } + } + + @Override + public boolean equals(final Object o) { + if (this == o) { + return true; + } + if (!(o instanceof SamlNameID)) { + return false; + } + final SamlNameID other = (SamlNameID) o; + return Objects.equals(xmlString, other.xmlString); + } + + @Override + public int hashCode() { + return Objects.hash(xmlString); + } + + /** + * Returns a redacted representation to prevent accidental PII leakage in logs. + * The XML string contains the user's SAML identity value and must never appear in logs. + */ + @Override + public String toString() { + return "SamlNameID{xmlString='[REDACTED]'}"; + } +} diff --git a/dotcms-integration/src/test/java/com/dotcms/auth/providers/saml/v1/SAMLHelperTest.java b/dotcms-integration/src/test/java/com/dotcms/auth/providers/saml/v1/SAMLHelperTest.java index cf2da501d001..d91726404c2f 100644 --- a/dotcms-integration/src/test/java/com/dotcms/auth/providers/saml/v1/SAMLHelperTest.java +++ b/dotcms-integration/src/test/java/com/dotcms/auth/providers/saml/v1/SAMLHelperTest.java @@ -11,6 +11,7 @@ import com.dotcms.datagen.UserDataGen; import com.dotcms.saml.Attributes; import com.dotcms.saml.DotSamlConstants; +import com.dotcms.saml.SamlNameID; import com.dotcms.saml.IdentityProviderConfiguration; import com.dotcms.saml.SamlAuthenticationService; import com.dotcms.saml.SamlConfigurationService; @@ -108,6 +109,9 @@ public void renderMetadataXML(Writer writer, IdentityProviderConfiguration ident @Override public String getValue(Object samlObject) { + if (samlObject instanceof SamlNameID) { + return ((SamlNameID) samlObject).getValue(); + } return samlObject.toString(); } @@ -118,6 +122,14 @@ public List getValues(Object samlObject) { } } + /** + * Creates a minimal {@link SamlNameID} suitable for tests that do not need real SAML XML. + * The XML stub is not valid OpenSAML XML; it only satisfies the non-blank invariant. + */ + private static SamlNameID testNameID(final String value) { + return new SamlNameID("" + value + "", value); + } + /** * Method to test: testing the {@link SAMLHelper#doLogin(HttpServletRequest, HttpServletResponse, IdentityProviderConfiguration, User, LoginServiceAPI)} * Given Scenario: tries to log in the admin user @@ -204,7 +216,7 @@ public void testResolveUser() throws DotDataException, DotSecurityException, IOE additionalAttributes.put("prop-key2", "prop-value2"); final Attributes nativeUserAttributes = new Attributes.Builder().firstName(nativeFirstName + "Updated") - .lastName(nativeLastName).nameID(nativeUser.getUserId()).email(nativeEmailAddress).additionalAttributes(additionalAttributes).build(); + .lastName(nativeLastName).nameID(testNameID(nativeUser.getUserId())).email(nativeEmailAddress).additionalAttributes(additionalAttributes).build(); // recover with SAML the native user final User recoveredNativeUser = samlHelper.resolveUser(nativeUserAttributes, identityProviderConfiguration); @@ -220,7 +232,7 @@ public void testResolveUser() throws DotDataException, DotSecurityException, IOE // creates an user from saml final Attributes samlUserAttributes = new Attributes.Builder().firstName(samlFirstName) - .lastName(samlLastName).nameID(samlNameId).email(samlEmailAddress).build(); + .lastName(samlLastName).nameID(testNameID(samlNameId)).email(samlEmailAddress).build(); // recover with SAML the new saml user final User samlUser = samlHelper.resolveUser(samlUserAttributes, identityProviderConfiguration); @@ -283,7 +295,7 @@ public void testResolveUserEmailRepeated() throws DotDataException, DotSecurityE // creates an user from saml final Attributes samlUserAttributes = new Attributes.Builder().firstName(nativeFirstName) - .lastName(nativeLastName).nameID("123" + nativeNameId).email(email).build(); // diff id, same email + .lastName(nativeLastName).nameID(testNameID("123" + nativeNameId)).email(email).build(); // diff id, same email // recover with SAML the new user final User recoveredNewUser = samlHelper.resolveUser(samlUserAttributes, identityProviderConfiguration); @@ -338,7 +350,7 @@ public void testResolveUser_byEmailAddress() throws DotDataException, DotSecurit .emailAddress(nativeEmailAddress).nextPersisted(); final Attributes nativeUserAttributes = new Attributes.Builder().firstName(nativeFirstName + "Updated") - .lastName(nativeLastName).nameID("xxxxxxxxxxx").email(nativeEmailAddress).build(); + .lastName(nativeLastName).nameID(testNameID("xxxxxxxxxxx")).email(nativeEmailAddress).build(); // recover with SAML the native user final User recoveredNativeUser = samlHelper.resolveUser(nativeUserAttributes, identityProviderConfiguration); @@ -423,12 +435,13 @@ public void test_createNewUser_non_Attrs() throws NoSuchAlgorithmException { company.setAuthType(Company.AUTH_TYPE_EA); when(companyAPI.getDefaultCompany()).thenReturn(company); final IdentityProviderConfiguration identityProviderConfiguration = mock(IdentityProviderConfiguration.class); - final Attributes attrs = new Attributes.Builder().nameID(UUID.randomUUID()).build(); + final String nameIdValue = UUID.randomUUID().toString(); + final Attributes attrs = new Attributes.Builder().nameID(testNameID(nameIdValue)).build(); final User user = samlHelper.createNewUser(APILocator.systemUser(), attrs, identityProviderConfiguration); Assert.assertNotNull(user); - Assert.assertEquals(samlHelper.hashIt(attrs.getNameID().toString()), user.getUserId()); + Assert.assertEquals(samlHelper.hashIt(nameIdValue), user.getUserId()); } /** @@ -449,12 +462,12 @@ public void test_createNewUser_known_Attrs() throws NoSuchAlgorithmException { when(companyAPI.getDefaultCompany()).thenReturn(company); final IdentityProviderConfiguration identityProviderConfiguration = mock(IdentityProviderConfiguration.class); final String uuid = UUID.randomUUID().toString(); - final Attributes attrs = new Attributes.Builder().nameID(uuid).email(uuid+"@dotcms.com.cr").firstName("John").lastName("Sn").build(); + final Attributes attrs = new Attributes.Builder().nameID(testNameID(uuid)).email(uuid+"@dotcms.com.cr").firstName("John").lastName("Sn").build(); final User user = samlHelper.createNewUser(APILocator.systemUser(), attrs, identityProviderConfiguration); Assert.assertNotNull(user); - Assert.assertEquals(samlHelper.hashIt(attrs.getNameID().toString()), user.getUserId()); + Assert.assertEquals(samlHelper.hashIt(uuid), user.getUserId()); Assert.assertEquals(uuid+"@dotcms.com.cr", attrs.getEmail()); Assert.assertEquals("John", attrs.getFirstName()); Assert.assertEquals("Sn", attrs.getLastName()); diff --git a/hotfix_tracking.md b/hotfix_tracking.md index d84f0fa2a0f0..af5103c60e28 100644 --- a/hotfix_tracking.md +++ b/hotfix_tracking.md @@ -50,3 +50,4 @@ Release-25.07.10 LTS 44. https://github.com/dotCMS/private-issues/issues/651 : Stored XSS → RCE: low-priv backend user name bypasses Xss filter, runs in admin session, deploys OSGi bundle #651 45. https://github.com/dotCMS/private-issues/issues/642 : sec: Privilege Escalation + RCE via OSGi bundle upload (low-priv backend user → CMS Administrator) #642 46. https://github.com/dotCMS/core/issues/36851 : Native, configurable HTML minification in the core rendering engine #36851 +47. https://github.com/dotCMS/core/issues/37085 : Upgrade BouncyCastle to 1.85 across all three bundled locations (CVE-2026-59638) #37085