[mbiuki]

Part of dotCMS/private-issues#725. Companion core PR: dotCMS/core#37865.

What

Brings SAML Response consumption in line with the SAML 2.0 Web Browser SSO profile (Profiles §4.1.4.2–4.1.4.3, Core §2.5.1, Bindings §3.5.5.2).

Changes

Signatures (CredentialServiceImpl, SamlCoreServiceImpl)

• An unrecognized signatureValidationType (blank, none, a typo) is treated as responseandassertion.
• A required signature must be present, and any signature that is present is verified. Previously, a signature on an element whose validation was not selected caused the response to be rejected.
• verify.signature.credentials and verify.signature.profile can no longer be set to false; the value is ignored and logged.
• The response must contain exactly one assertion, encrypted or not as configured, instead of taking the first one.
• Signature algorithm allow-list, read from the SignedInfo that is verified: RSA or ECDSA with SHA-256/384/512, and SHA-256/384/512 reference digests. RSA-SHA1 and SHA-1 digests are accepted only with allow.sha1.signatures=true (logged as a warning); HMAC, MD5, DSA and unknown algorithms are always rejected.

Profile checks (SamlResponseValidator, new; runs after the signature checks in HttpPostAssertionResolverHandlerImpl)

• Response Destination equals the assertion consumer URL (required when the Response is signed). Compared with the configured ACS URL, plus its /api/v1 form, rather than the servlet request URL, which differs behind a TLS-terminating proxy and after the /dotsaml/login rewrite.
• Response Issuer (when present) and Assertion Issuer equal the IdP entityID from the metadata.
• Assertion Conditions NotBefore/NotOnOrAfter, and every AudienceRestriction includes the SP Issuer ID.
• A bearer SubjectConfirmation whose SubjectConfirmationData has an unexpired NotOnOrAfter, a Recipient equal to the ACS URL, and an InResponseTo equal to the Response's.
• At least one AuthnStatement; SessionNotOnOrAfter honored.
• Each assertion ID is accepted once, on any node and across restarts (DatabaseAssertionReplayStore, new): the bundle creates a dotsaml_assertion_replay table in the dotCMS database (via com.dotmarketing.db, exported to the system OSGi framework on main and 25.07 LTS) and, on its own auto-commit connection, deletes expired rows and inserts the assertion key with ON CONFLICT DO NOTHING. One inserted row means first use. A database error rejects the response.

Request binding (AuthnRequestStateCookie, new)

• The three authentication handlers keep one __Host-dotsaml_req_<IdP configuration> cookie (HttpOnly; Secure; SameSite=None; Path=/) holding the five most recent AuthnRequest IDs, each with its issue time. A Response's InResponseTo must match an unexpired one, which is then removed. A cookie is used rather than the HTTP session because the session cookie is SameSite=Lax and is not sent on the IdP's cross-site POST.
• A login that starts on a host other than the Service Provider Endpoint Hostname (a site alias, or a site using the System Host configuration) is first redirected to the ACS URL on the endpoint host, where a GET starts it again (EndpointHostRedirect, from OpenSamlAuthenticationServiceImpl.authentication). The cookie is then set on the host the IdP posts back to. Users already end up on the endpoint host after login today. A marker parameter prevents a second redirect, and hosts are compared without ports.
• Responses without InResponseTo (IdP-initiated login) are rejected unless allow.unsolicited.responses=true.

Other

• SamlUtils.toXMLObjectString serializes an existing DOM instead of marshalling again. Logging an unsigned, unencrypted assertion re-marshalled it, which released the Response's DOM; OpenSAML reads isSigned() from the DOM, so a Response-signed IdP with a plain assertion was rejected as unsigned (with "does not match" before this PR).
• Assertion decryption no longer logs the SP credential's private key.
• A SAML message that isn't a Response posted to the ACS is rejected cleanly instead of failing with a ClassCastException (a 500).
• validateDestinationAndLifetime renamed to validateMessageLifetime (it only ever checked the lifetime).
• verifyStatus no longer throws a NullPointerException for a missing status or a status without a sub-code.
• README: new properties, validation steps, a troubleshooting table from log message to fix, clock.skew / message.life.time units corrected to milliseconds.

New optional properties

• allow.unsolicited.responses (default false).
• authn.request.max.age, seconds an AuthnRequest stays valid (default 900, clamped to 60–3600).
• allow.sha1.signatures (default false).

Behaviour changes (release notes)

• IdP-initiated login stops working by default. Core routes IdP-initiated logins in DotSamlResource since July (8e943ee0a1), so some sites rely on it. They need allow.unsolicited.responses=true.
• Destination and Recipient must exactly match the ACS URL derived from the Service Provider Endpoint Hostname (https://<hostname>/dotsaml/login/<site id>, or its /api/v1 form). Sites whose IdP posts to another hostname are rejected.
• Configurations with Validation Type none require a signed response and assertion.
• verify.signature.credentials=false / verify.signature.profile=false no longer have any effect.
• Fixed: an IdP that signs only the Response and sends an unencrypted assertion could not log in (rejected as "does not match"); it now works.
• Logins started on a site alias are redirected to the Service Provider Endpoint Hostname before going to the IdP.
• IdPs that sign with RSA-SHA1 or SHA-1 digests are rejected unless allow.sha1.signatures=true.

Support can map each rejection's log message to its fix with the README's troubleshooting table.

Notes

• OpenSAML 3.3.1 ships SAML20AssertionValidator, but not the parameters for valid issuers or the expected InResponseTo (VALID_ISSUERS, SC_VALID_IN_RESPONSE_TO, both 4.0). It also treats a missing Conditions, AudienceRestriction, Recipient or NotOnOrAfter as valid, and resolves SubjectConfirmationData@Address through DNS. So the checks are implemented explicitly.
• The bearer check tolerates a SubjectConfirmationData NotBefore (Profiles §4.1.4.2 says there shouldn't be one) and only checks that it isn't in the future, like OpenSAML.
• The plugin version is not bumped here; that happens in the release step, followed by the com.dotcms.samlbundle bump in core.

Testing

./mvnw test: 92 tests pass, 71 of them new.

• TestSamlResponseValidator (27): each profile check, clock skew, URL normalization, replay, request binding, unsolicited handling.
• TestSamlSignatureVerification (15): fail-closed validation type, signatures that can't be disabled, required vs. present signatures with real RSA signatures, algorithm allow-list (SHA-1 signature and digest, SHA-1 opt-in, stronger RSA, HMAC), assertion count.
• TestHttpPostAssertionResolverHandler (10, including a posted message that isn't a Response): end to end through HTTP-POST decoding: assertion-signed, Response-signed with a plain and with an encrypted assertion, both signed (under each Validation Type), Response-only signature when the assertion must be signed, an altered signed Response, the unsigned response under "none".
• TestDatabaseAssertionReplayStore (6), TestAuthnRequestStateCookie (7), TestEndpointHostRedirect (5) and TestLoginStartHost (1, a login started on an alias is redirected before any AuthnRequest or cookie). The replay-store SQL was also run against Postgres 16 (first insert 1 row, replay 0 rows, expired rows deleted).

Not yet tested against a live IdP. QA should cover Entra ID and Okta, SP- and IdP-initiated.

🤖 Generated with Claude Code

[swicken]

Thanks, this is a solid piece of work. The profile checks follow the spec closely, comparing Destination and Recipient against the configured ACS URL rather than the request URL is the right call behind a TLS-terminating proxy, and the cookie-based request binding is a good fit given that JSESSIONID is SameSite=Lax. Making unrecognized validation types fail closed and removing the ways to switch signature checks off is the direction we want. I ran ./mvnw test on the branch: 66 tests, all passing.

Two things need to change before this can merge, plus some smaller points.

Needs changes

1. The replay cache has to be shared across the cluster. SamlResponseValidator keeps used assertion IDs in a MemoryStorageService on each node. Most of our deployments run two or more nodes behind a load balancer, so an assertion that one node has already accepted can be accepted again by another node, and by any node after a restart, until it expires. The request-state cookie doesn't cover this, because its name and value are not secret. With allow.unsolicited.responses=true, the replay cache is the only protection against replay at all.

A suggestion for the store: a small table that the bundle creates itself, accessed through DotConnect. Use the assertion ID as the primary key, store an expiry in epoch millis, write with INSERT ... ON CONFLICT (assertion_id) DO NOTHING (an affected-row count of 0 means a replay), and delete expired rows as part of the same check. Why this option:

• com.dotmarketing.common.db is exported to the system OSGi framework on main and on the 25.07 LTS, and core is Postgres-only on both, so the fix stays bundle-only.
• The Lettuce Redis client is not exported to that framework.
• DotCacheAdministrator only sends invalidations to other nodes, never the stored values, so it can't be used either.

2. No test covers a signed Response. Every signed fixture in TestSamlSignatureVerification and TestHttpPostAssertionResolverHandler signs the assertion only. Shibboleth signs the Response and encrypts the assertion by default, and some Entra ID setups sign only the Response. For those IdPs, the path through verifySignature for the Response is untested: the setIdAttribute on the Response DOM, the algorithm check on the Response's SignedInfo, and the profile and credential checks. Please add end-to-end happy paths through TestHttpPostAssertionResolverHandler for a Response-signed IdP, ideally one with an unencrypted assertion and one with an encrypted assertion.

Smaller points

3. The OpenSAML rationale is slightly off. The Javadoc of SamlResponseValidator and the PR description say SAML20AssertionValidator isn't available in 3.3.x. It is: it's in opensaml-saml-api-3.3.1.jar. What 3.3.1 lacks are the issuer and InResponseTo parameters (VALID_ISSUERS, SC_VALID_IN_RESPONSE_TO, both added in 4.0). It also treats a missing Conditions, AudienceRestriction, Recipient or NotOnOrAfter as valid, and it resolves SubjectConfirmationData@Address through DNS. Those are good reasons to implement the checks explicitly, so only the explanation needs correcting.

4. Cookies per AuthnRequest. Each login attempt adds its own dotsaml_req_<id> cookie for up to 15 minutes. The ACS returns an error rather than redirecting, so this only grows with repeated manual retries. Still, a bound would be cheap: for example, one cookie per IdP configuration holding the latest request ID, or a cap on how many are kept. Separately, the cookie already meets the __Host- prefix rules (Secure, Path=/, no Domain). Adding the prefix would stop a sibling subdomain from planting one.

5. Release notes. Beyond the four behaviour changes already listed in the description, two need prominent mention:

• IdP-initiated login stops working by default. Core added routing specifically for IdP-initiated logins in DotSamlResource this July (8e943ee0a1), so some sites rely on it, and support will need allow.unsolicited.responses documented.
• Destination and Recipient must exactly match the ACS URL derived from the Service Provider Endpoint Hostname. Sites whose IdP posts to a different hostname will be rejected.

The log messages are clear enough that a short mapping from message to fix would make those support tickets quick.

6. Release order with dotCMS/core#37865. That PR rewrites a stored none to responseandassertion. With the bundle core ships today, which requires an exact signature match, every IdP that doesn't sign both the Response and the Assertion would be rejected. So this bundle needs to ship before that core change, or together with it.

7. Nit: the bearer check accepts a SubjectConfirmationData NotBefore as long as it isn't in the future, although Profiles §4.1.4.2 says a bearer confirmation must not have one. Being lenient seems fine for compatibility, but a short comment would make that look deliberate rather than an oversight.

[swicken]

Correction to point 6: the release order with dotCMS/core#37865 doesn't matter. With the current bundle, none only works for IdPs that sign nothing, so core mapping it to responseandassertion breaks only those sites (which this bundle breaks too), and it closes the none path even before this bundle ships.

[mbiuki]

Thanks, Scott. Addressed in bfb829d:

1. Replay cache: now DatabaseAssertionReplayStore, along the lines you suggested. The bundle creates dotsaml_assertion_replay (assertion_key VARCHAR(64) primary key, a SHA-256 of IdP entityID + assertion ID; expires_at BIGINT, indexed). On its own auto-commit connection from DbConnectionFactory.getDataSource() (com.dotmarketing.db, exported to the system framework on main and 25.07), it deletes expired rows and inserts with ON CONFLICT DO NOTHING: one row means first use. A database error rejects the response. It's unit-tested against a fake JDBC layer, and I ran the SQL against Postgres 16.
2. Signed Response: added end-to-end cases for a Response-signed IdP with a plain and with an encrypted assertion, both signed under each Validation Type, a Response-only signature when the assertion must be signed, and an altered signed Response. The plain-assertion case found a bug: logging the unsigned assertion through SamlUtils.toXMLObjectString re-marshalled it, which released the Response's DOM, and OpenSAML's isSigned() reads the DOM, so the Response looked unsigned. toXMLObjectString now serializes the existing DOM. The bundle we ship today has the same bug (it reports "does not match"), so an IdP that signs only the Response and sends an unencrypted assertion can't log in with it.
3. OpenSAML: you're right, I missed it (my search pattern skipped class names containing digits). The Javadoc and the description now give the actual reasons.
4. Cookie: one __Host-dotsaml_req_<config> cookie per IdP configuration holding the five most recent request IDs, each with its issue time and expiring on its own.
5. Release notes: both added to the description, with the IdP-initiated note pointing at 8e943ee0a1. The README has a table from log message to fix for support.
6. Noted, thanks for the correction.
7. Comment added.

I also removed a log line in assertion decryption that printed the SP credential's private key.

85 tests pass.

[swicken]

Thanks, Mehdi, bfb829d addresses everything from the first review. I re-checked it:

• All 85 tests pass locally.
• I ran the replay-table statements against Postgres 16. The first use inserts a row, a replay inserts none, expired rows are deleted and unexpired ones kept, and repeating the table creation is harmless.
• I reproduced the SamlUtils bug: with the old toXMLObjectString, three of the new handler tests fail, including the Response-signed IdP with a plain assertion. Good catch; that fix is worth a line in the release notes.

One more issue came out of a final pass, and it needs either a fix or an explicit decision before release.

A login that starts on a different host than the configured endpoint host is rejected. AuthnRequestStateCookie.remember sets the cookie on the response to whatever host the user started the login on. The AuthnRequest, though, always names https://<Service Provider Endpoint Hostname>/dotsaml/login/<id> as the ACS (see EndpointServiceImpl.getAssertionConsumerEndpoint). When the two hosts differ, the browser doesn't send the cookie to the ACS, and consume rejects the response as "not started from this browser". The __Host- prefix isn't the cause; the first version's cookie had no Domain attribute either, so it was also tied to one host.

This affects users who browse a site alias, and sites that use the System Host's SAML configuration while being served under a different hostname. Those logins work today and end on the endpoint host. I traced this through core (SamlWebInterceptor, DotSamlResource.doLogin) and the bundle, and found nothing that moves the user to the endpoint host before the AuthnRequest is built. I haven't reproduced it on a live instance.

A possible fix: when the request host differs from the endpoint host, redirect first to https://<endpoint host>/dotsaml/login/<id> (DotSamlResource.doLogin), passing the return path along. The AuthnRequest and its cookie are then issued by the host that receives the response. Users already end up on the endpoint host after login today, so they shouldn't notice any difference. If you'd rather not change the flow now, it needs a release note and an entry in the support table. A handler or validator test with different start and ACS hosts would pin it down either way.

Optional, not blocking: the static settable clock in AuthnRequestStateCookie could be passed in rather than held as mutable static state. Also, a pre-existing issue: posting a SAML message that isn't a Response to the ACS throws a ClassCastException in HttpPostAssertionResolverHandlerImpl, which shows up as a 500 rather than a clean rejection.

[mbiuki]

Thanks, Scott. Went with your suggestion, in bfb5d40:

• Logins started on another host: before any AuthnRequest or cookie is issued, OpenSamlAuthenticationServiceImpl.authentication checks whether the request host differs from the endpoint host. If it does, the browser is redirected to the configured ACS URL, where a GET (DotSamlResource.doLogin, via the /dotsaml/login rewrite) starts the login again, so the cookie lands on the host the IdP posts back to. The target is always the configured ACS URL, never request input. A dotsaml_host_redirect marker stops a second redirect, so a proxy that rewrites the Host header can't loop. Hosts are compared without ports, since cookies ignore ports. It stays in the bundle: core already routes GET /dotsaml/login/<id> to doLogin. The post-login destination is the same as today on an alias (the endpoint host's default), because the original target lives in the alias host's session. Carrying it across would need a doLogin change in core, so I left that out.
• Tests: TestEndpointHostRedirect (same host, case and port, alias, marker, relative/missing ACS) and TestLoginStartHost, which goes through authentication with a different start host and asserts a redirect with no AuthnRequest or cookie.
• Non-Response posts: rejected with a SamlException instead of the ClassCastException, with a test.
• Clock: AuthnRequestStateCookie now takes the clock as a parameter; there's no static state.
• The Response-signed fix is in the release notes now.

92 tests pass.