Skip to content

idpCertFingerprintAllowlist mandatory for multi-ingress SSO#5327

Merged
supersven merged 22 commits into
developfrom
sventennie/allowlist-usage
Jul 9, 2026
Merged

idpCertFingerprintAllowlist mandatory for multi-ingress SSO#5327
supersven merged 22 commits into
developfrom
sventennie/allowlist-usage

Conversation

@supersven

@supersven supersven commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

In #5272 I introduced the idpCertFingerprintAllowlist config setting which contains fingerprints of allowed IdP x509 certificates. If it was set, IdP related requests only worked for allow-listed certificates. This becomes necessary, because #5212 allows auto-migrating users between IdPs. So, multi-ingress needs to be stricter regarding IdP management.

This PR goes one step further and requires the idpCertFingerprintAllowlist to be not empty, to prevent configuration mistakes; i.e. to prevent forgetting to set this flag.
This certainly captures the intent of the security team better.

I could have added this to the application startup code, but this is currently almost impossible to test with our current infrastructure and tests for details really matter in the multi-ingress labyrinth.

Are we breaking the API here?
Yes, but only from multi-ingress (one customer) where we control the configuration and can make the required changes to make this invisible to end-users. As we cannot control which versions of clients they (or a potential attacker) use, I see no other way.

Ticket: https://wearezeta.atlassian.net/browse/WPB-26150

Checklist

  • Add a new entry in an appropriate subdirectory of changelog.d
  • Read and follow the PR guidelines

@zebot zebot added the ok-to-test Approved for running tests in CI, overrides not-ok-to-test if both labels exist label Jul 8, 2026
@supersven supersven force-pushed the sventennie/allowlist-usage branch from 19275af to 4b98da1 Compare July 8, 2026 11:08

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR tightens IdP certificate fingerprint enforcement by making idpCertFingerprintAllowlist effectively mandatory for multi-ingress SSO, preventing insecure/mistaken configurations where the allowlist is omitted or empty.

Changes:

  • Update IdP certificate allowlist enforcement to reject multi-ingress IdP requests when the allowlist is Nothing or empty.
  • Extend Spar unit tests and integration tests to cover the multi-ingress “empty allowlist” rejection behavior and to configure allowlists where needed.
  • Update documentation and add a changelog entry describing the new multi-ingress requirement.

Reviewed changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
services/spar/src/Spar/API.hs Enforces “non-empty allowlist required” behavior for multi-ingress via assertCertsAllowlisted, with dedicated log message.
services/spar/test/Test/Spar/Saml/IdPSpec.hs Adds/adjusts unit tests to cover multi-ingress rejection on empty/absent allowlist across create/update/authresp flows.
libs/saml2-web-sso/src/SAML2/WebSSO/Test/Util/TestSP.hs Refactors test IdP metadata generation to support reusing a provided certificate for allowlist-driven tests.
integration/test/Testlib/Certs.hs Adds a shared fingerprintHex helper for canonical SHA-1 fingerprint formatting.
integration/test/Test/Spar/MultiIngressSSO.hs Updates multi-ingress SSO integration test setup to configure the allowlist with the generated IdP cert fingerprint.
integration/test/Test/Spar/MultiIngressIdp.hs Updates multi-ingress IdP integration tests to configure allowlist fingerprints and generate IdPs with matching certs.
integration/test/Test/Spar/GetByEmail.hs Updates multi-ingress get-by-email tests to configure allowlist fingerprints and generate IdPs with matching certs.
integration/test/Test/Spar/CertFingerprintAllowlist.hs Reuses shared fingerprintHex helper (removes local duplicate).
docs/src/developer/reference/config-options.md Documents that the allowlist is mandatory for multi-ingress setups and optional otherwise.
changelog.d/1-api-changes/multi-ingress-mandatory-allowlist Announces the breaking change for multi-ingress and its security motivation.

Comment thread services/spar/src/Spar/API.hs Outdated
Comment thread changelog.d/1-api-changes/multi-ingress-mandatory-allowlist Outdated
supersven and others added 3 commits July 8, 2026 15:26
@supersven supersven changed the title allowlist usage idpCertFingerprintAllowlist mandatory for multi-ingress SSO Jul 8, 2026
@supersven supersven marked this pull request as ready for review July 8, 2026 14:35
@supersven supersven requested review from a team as code owners July 8, 2026 14:35
@supersven supersven merged commit d27ea44 into develop Jul 9, 2026
11 checks passed
@supersven supersven deleted the sventennie/allowlist-usage branch July 9, 2026 05:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ok-to-test Approved for running tests in CI, overrides not-ok-to-test if both labels exist

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants