fix(saml): require signed SAML responses by default

[mbiuki]

Fixes dotCMS/private-issues#725

Proposed Changes

• SAML app descriptor (dotsaml-config.yml): Validation Type defaults to Response & Assertion; the None option is removed and the hint says to select exactly the parts the IdP signs (exact matching is what the bundle core ships today requires).
• DotIdentityProviderConfigurationImpl: a missing or unsupported signatureValidationType (including a stored none) resolves to responseandassertion, and stored verify.signature.credentials / verify.signature.profile extra parameters are ignored, so the SAML bundle always uses its defaults for them (true). A warning naming the site is logged once per site when either applies (the configuration is rebuilt per request). This takes effect with the SAML bundle version core ships today.
• DotAbstractSamlConfigurationServiceImpl: the same two keys are also ignored in the SAML defaults properties file, so they can't switch signature checks off there either.
• dotAuth
  • SamlProtocolHandler stores only response, assertion or responseandassertion (normalized) and answers 400 for anything else.
  • SAML screen: the last signature toggle that is on can't be switched off, the payload never carries none (neither toggle on maps to responseandassertion), and a stored none or unknown value is shown as both signatures required, matching what the backend enforces.

Checklist

• Tests: DotIdentityProviderConfigurationImplTest (new, 6), DotAbstractSamlConfigurationServiceImplTest (new, 1), SamlProtocolHandlerTest (+3), dot-auth mapper spec (+2, 1 updated). Java: 28/28; dot-auth lib: 129/129; strict TypeScript gate, Prettier and ESLint pass on the changed frontend files.
• Translations: n/a
• Security Implications Contemplated

Additional Info

• 25.07 LTS counterpart: fix(saml): require signed SAML responses by default (25.07 LTS) #37871.
• Companion plugin PR: fix(saml): validate SAML responses against the Web Browser SSO profile com.dotcms.dotsaml#25, which validates the response itself (Destination, Issuer, Conditions, AudienceRestriction, SubjectConfirmation, InResponseTo, replay). This PR doesn't depend on it and can merge first. Once the new bundle is published to repo.dotcms.com, the com.dotcms.samlbundle pin in osgi-base/system-bundles/pom.xml moves in two separate PRs, one for main and one for release-25.07.10_lts.
• Behaviour change for release notes: a site whose SAML configuration still has Validation Type None needs its IdP to sign the response and the assertion after upgrading, or an admin to save the Validation Type that matches what the IdP signs.

🤖 Generated with Claude Code

This PR fixes: #725

[claude]

Claude finished @mbiuki's task in 1m 46s —— View job

---

Code review: SAML validation defaults

Re-checked the five findings from the prior CHANGES_REQUESTED review against the latest commit (ea76527a). All five are addressed, and I found no new blocking issues.

Resolved

• ✅ Strict TS gate — dot-auth-config.mappers.spec.ts:349 now casts the out-of-range array as unknown as DotAuthSignatureValidation[], so TS2322 is cleared. (Confirm with the CI re-run, as swicken asked.)
• ✅ Warning log flood — DotIdentityProviderConfigurationImpl guards each warning with a static LOGGED_WARNINGS set via firstTime() keyed on host.getIdentifier() + '|' + warning, so a legacy none / stored verify.signature.* logs once per site+value instead of per request. Short-circuit order (condition && firstTime(...)) also keeps the set from being polluted for valid configs.
• ✅ Defaults file not covered — DotAbstractSamlConfigurationServiceImpl now skips SIGNATURE_CHECKS (verify.signature.credentials / verify.signature.profile) when loading dotcms-saml-default.properties, so a =false there no longer turns the check off. New DotAbstractSamlConfigurationServiceImplTest proves the two keys stay true while an unrelated default (clock.skew) still applies.
• ✅ Test gap — DotIdentityProviderConfigurationImplTest adds signature_check_overrides_are_hidden_from_the_bundle (both ignored keys return null / containsOptionalProperty==false) and other_extra_parameters_still_reach_the_bundle (allow.unsolicited.responses, clock.skew pass through), covering getOptionalProperty/containsOptionalProperty as requested.
• ✅ Wording — the Validation Type hint was rewritten to explain the setting (dotsaml-config.yml:40).

Smaller point still open (non-blocking)

• 🟡 Minor: dotCMS/src/main/resources/apps/dotsaml-config.yml:40 — finding wezell's issue #5 also suggested noting that a signature the IdP sends on the non-selected part is verified too (the bundle's behaviour after fix(saml): validate SAML responses against the Web Browser SSO profile com.dotcms.dotsaml#25), so admins picking "Only Assertion" for an IdP that signs both aren't surprised. The hint now explains the setting but doesn't add that. Optional, and arguably belongs with the Removing system host permissions from a role creates error on backend #25 release rather than here.

Notes verified while reviewing

• SamlProtocolHandler.validSignatureValidationType reuses DotIdentityProviderConfigurationImpl.resolveSignatureValidationType as the single source of truth and throws javax.ws.rs.BadRequestException (imported at line 18) for none/unknown — server-side 400 matches the frontend. Stored value is normalized (lowercased/trimmed), confirmed by buildSecrets_stores_supported_signature_validation_type_normalized.
• Frontend fromView / toPayload round-trip is consistent: a stored none/unknown shows as both toggles on, and neither-box-checked serializes to responseandassertion, never none. The two [disabled] guards in dot-auth-saml-config.component.html keep the last on-toggle from being switched off.

The two release-coordination items from the PR body and swicken's comment (publishing plugin dotCMS/com.dotcms.dotsaml#25 to repo.dotcms.com, then the two com.dotcms.samlbundle pin PRs for main and release-25.07.10_lts) are tracking notes, not code in this diff.

[swicken]

Thanks, this looks right overall. It's also useful on its own: with the SAML bundle core ships today (26.08.21), mapping a stored none to responseandassertion already means unsigned responses are rejected for those sites. So this PR closes the none path even before dotCMS/com.dotcms.dotsaml#25 is released, and the two don't have to ship in a particular order. The sites it breaks are the ones that only worked because their IdP signs nothing, and those break under the new bundle anyway. The dotAuth side hangs together too: the server rejects any other type with a 400, the last signature toggle can't be switched off, and a legacy value is shown the way the backend enforces it.

Two things need fixing first.

Needs changes

1. CI: the strict TypeScript gate fails. The Frontend Unit Tests job fails in strict-gate with TS2322: Type 'string' is not assignable to type 'DotAuthSignatureValidation | undefined', and everything after it was cancelled. It comes from the new spec in dot-auth-config.mappers.spec.ts: for (const signatureValidationType of ['none', '', 'signature']) infers string, which is then assigned to values.signatureValidationType. Since the test feeds out-of-range values on purpose, a cast on the array should fix it, e.g. as unknown as DotAuthSignatureValidation[]. I haven't run the gate locally, so please confirm with a CI re-run.

2. The warning in the constructor will flood the logs. warnAboutOverriddenSettings() runs in the DotIdentityProviderConfigurationImpl constructor. DotIdentityProviderConfigurationFactoryImpl builds a new instance on every lookup, and SamlWebInterceptor.intercept looks one up for each request it handles. So a busy site with a legacy none, or with a stored verify.signature.* parameter, logs a WARN on every intercepted request until someone re-saves its configuration. Please log once per site, for example by keeping a set of site identifiers already warned, or by moving the check to save time and startup.

Smaller points

3. The verify.signature.* filter doesn't cover the defaults file. IGNORED_PROPERTIES hides these keys when they are stored as App extra parameters. But DotAbstractSamlConfigurationServiceImpl also loads saml/dotcms-saml-default.properties from the assets folder, and that file accepts these keys too. So with the current bundle, a verify.signature.credentials=false there still turns credential checks off. The bundle PR ignores these keys altogether, so this closes once both are out. Either cover the defaults file here as well, or note in the release notes that only the stored extra parameters are filtered until the bundle ships.

4. Tests. DotIdentityProviderConfigurationImplTest only covers the static resolveSignatureValidationType. Please add a test that getOptionalProperty and containsOptionalProperty hide the two ignored keys while passing others through. Other extra parameters, such as the new allow.unsolicited.responses, have to keep reaching the bundle.

5. Wording, minor. The new Validation Type hint ("dotCMS rejects a response unless the selected parts carry a valid IDP signature") reads well. It's worth adding that a signature the IdP sends on the other part is verified too, since that's how the bundle behaves after #25, and admins choosing "Only Assertion" for IdPs that sign both will wonder.

[swicken]

Note for the release: this PR doesn't change the SAML bundle version core ships, so a separate pin change is needed once the new plugin version (from dotCMS/com.dotcms.dotsaml#25) is published.

• The only pin is osgi-base/system-bundles/pom.xml (com.dotcms.samlbundle, currently 26.08.21); the build copies that jar into WEB-INF/felix-system/bundle, so nothing else needs editing.
• As with the Bouncy Castle update, that takes two PRs: one for main (like fix(security): reference dotSAML bundle 26.08.21 (BouncyCastle 1.85) #37159) and one for release-25.07.10_lts (like #37085 include in 25.07.10 LTS — reference dotSAML bundle 26.08.21 (BouncyCastle 1.85) #37161). The LTS has been on the same 26.08.21 bundle since v25.07.10_lts_v18, so it can take the new version as a straight pin change.
• The plugin has to be published to repo.dotcms.com first, because core's build downloads it from there.

[mbiuki]

Thanks, Scott. Addressed in ea76527:

1. Strict gate: cast added. I ran the gate locally: it fails without the cast and passes with it.
2. Warnings: logged once per site and value (a static set keyed by site identifier).
3. Defaults file: DotAbstractSamlConfigurationServiceImpl now ignores the two keys when it loads the defaults file, with a warning, so this PR covers the current bundle there too.
4. Tests: added a package-private constructor that takes the AppSecrets. Tests cover the two keys being hidden, other extra parameters (including allow.unsolicited.responses) still passing through, a stored none, and repeated construction, plus a defaults-file test.
5. Hint: I went with "Select exactly the parts of the SAML Response your IDP signs…" rather than describing Removing system host permissions from a role creates error on backend #25's behaviour. This PR can ship before the new bundle, and with today's bundle a mismatch is rejected. Once Removing system host permissions from a role creates error on backend #25 is out, selecting a subset also works, so the hint stays correct.

25.07 LTS counterpart: #37871.