multi-ingress: cross-IdP SSO

[supersven]

I've covered this usecase in the documentation. To avoid duplication, please look at the docs in this PR.

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

Security notes:

• This requires a final sign off by @wireapp/security
• The algorithm for multi-ingress SSO cross-IdP logins has been discussed with @emil-wire
• This PR also lowers the condition to be found by /sso/get-by-email from SCIM SSO to SSO user

Further reading:

• https://docs.wire.com/latest/developer/reference/config-options.html?h=multi+ingress#settings-in-cannon
• https://docs.wire.com/latest/how-to/install/multi-ingress.html#what-is-a-multi-ingress-setup

Checklist

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

[Copilot]

Pull request overview

Adds support for multi-ingress “cross-IdP” SSO logins in Spar by falling back to a team-wide lookup (by email NameID) when the primary (issuer, NameID) lookup fails, and updates /sso/get-by-email discovery to work for any SSO user (not only SCIM-managed).

Changes:

• Extend Spar SAML finalize-login handling to support cross-IdP migration in multi-ingress mode (and add a dedicated config error for non-email NameIDs).
• Relax /sso/get-by-email lookup condition from “SCIM+SSO” to “SSO user”.
• Add integration and unit tests plus documentation/changelog updates covering the new behavior.

Reviewed changes

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

Show a summary per file

File	Description
services/spar/test-integration/Test/Spar/AppSpec.hs	Updates integration test helper to call updated verdictHandler signature with SAML config.
services/spar/src/Spar/Error.hs	Adds and renders a new error for invalid multi-ingress cross-IdP configuration (non-email NameID).
services/spar/src/Spar/App.hs	Implements multi-ingress cross-IdP fallback flow + structured logging changes.
services/spar/src/Spar/API.hs	Plumbs SAML config/host info into finalize-login handling and updates logger constraints.
libs/wire-subsystems/test/unit/Wire/IdPSubsystem/InterpreterSpec.hs	Adjusts unit test expectation to “non SSO user” behavior for get-by-email.
libs/wire-subsystems/src/Wire/IdPSubsystem/Interpreter.hs	Changes /sso/get-by-email eligibility check from SCIM+SSO to SSO.
integration/test/Test/Spar/MultiIngressCrossIdpSso.hs	Adds new integration test suite for cross-IdP migration and related error cases.
integration/integration.cabal	Registers new integration test module.
docs/src/developer/reference/config-options.md	Documents cross-IdP fallback algorithm and updates get-by-email criteria.
changelog.d/2-features/multi-ingress-cross-IdP-SSO	Adds changelog entry describing cross-IdP SSO behavior.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[supersven]

This might be subject to change in case the customer decides to go for SCIM. Then, we could make this condition stricter (requiring SSO AND SCIM).

[fisx]

I read the docs and have some questions, more feedback coming up!

[fisx]

[nit] admins need to manually guarantee that email addresses are unique, so why can't they do the same thing here?

[supersven]

@emil-wire and I were uncertain about this as well. I made the hand-wavy decision, because in my experience with other web services, my email address is much less likely to conflict than my desired username when I register somewhere.

However, the customer is not much into emails as fetching those may reveal relationships as well AND @julialongtin promised that all data will be located in one database (used by all IdPs)

So, this could actually be subject to change. 🤔

[fisx]

I think the best approach is to allow nicks / usernames, but put big red warnings stickers in all the right places and emphasize that emails are best here, and nicks with mandatory, instance-specific pre- or suffixes are second-best.

If you decide against my intuition that's also fine, if anything supporting only emails would make it safer.

[Copilot]

Pull request overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated 4 comments.

[Copilot]

Pull request overview

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

[fisx]

I skimmed through the integration tests and thoroughly reviewed the rest, looks intact! I've hidden a few possibly valuable change requests in a large number of nit-picks to make processing the review more fun. :-)

I'm pre-approving because none of the change requests affect correctness and security IMO.

[fisx]

I think the best approach is to allow nicks / usernames, but put big red warnings stickers in all the right places and emphasize that emails are best here, and nicks with mandatory, instance-specific pre- or suffixes are second-best.

If you decide against my intuition that's also fine, if anything supporting only emails would make it safer.

[fisx]

It's probably worth adding this warning to the UI in team management, at the point where multi-ingress is enabled. Has this happened? Is there a ticket for this?

[emil-wire]

Hm, please correct me if I'm wrong (my Haskell isn't great) but doesn't https://github.com/wireapp/wire-server/blob/sventennie/multi-ingress-sso_cross_idp/services/spar/src/Spar/App.hs#L528
asum $ map tryIdP idps return the first matching (issuer, email NameID) across team IdPs? Meaning if there are more than one match, the "Exactly one" condition isn't satisfied?

Failure scenario:

• Team has IdP A and IdP B.
• Victim exists under IdP A with email x@example.
• A different person or malicious IdP-side actor authenticates via IdP B with NameID = x@example.
• If no primary (B, x@example) user exists, fallback searches all team IdPs and can select the victim’s IdP A mapping.
• Outcome: attacker logs into victim’s Wire account and the SSO identity is migrated to IdP B.

This is exacerbated by the fact that we assume (but never check in code): "Different SAML IdPs must have a same user set” as a prerequisite.

[supersven]

asum $ map tryIdP idps return the first matching (issuer, email NameID) across team IdPs? Meaning if there are more than one match, the "Exactly one" condition isn't satisfied?

Yes, if you take this precisely for this expression: Yes, there could be e.g. 2 matching IdPs and the last one found would be ignored. Probably, this item is a bit misleading: SSO users are always bound to one IdP. The systems doesn't allow for more than one, though.
I will update the documentation accordingly.

Failure scenario:

Team has IdP A and IdP B.
Victim exists under IdP A with email x@example.
A different person or malicious IdP-side actor authenticates via IdP B with NameID = x@example.
If no primary (B, x@example) user exists, fallback searches all team IdPs and can select the victim’s IdP A mapping.
Outcome: attacker logs into victim’s Wire account and the SSO identity is migrated to IdP B.

This is exacerbated by the fact that we assume (but never check in code): "Different SAML IdPs must have a same user set” as a prerequisite.

Yes, that's exactly the scenario we're dealing with here (if you change the wording a bit, that's exactly the usecase - just s/(attacker|victim)/user/g 😉 ).

Unfortunately, we cannot ensure on our end that two IdPs contain the same user data.
Why do we need multiple IdP configurations (the representation of an IdP at Wire backend) then? A common IdP endpoint would leak information about the relationship of ingress-domains.

The countermeasures for this are:

• Logging on IdP configuration change (https://wearezeta.atlassian.net/browse/WPB-22124)
• Emails on IdP configuration change (https://wearezeta.atlassian.net/browse/WPB-22124)
• IdP fingerprint allowlist in spar's application config (https://wearezeta.atlassian.net/browse/WPB-26150 , with Jonas' analyses https://wearezeta.atlassian.net/browse/WPB-25378?focusedCommentId=163359)

[emil-wire]

Not something this PR introduced, but its close enough to it that I'd rather raise it here than lose track of it and forget about it.

I don't think the rate limit on /sso/get-by-email is actually in force. The generic - path: /sso entry sits above the - path: /sso/get-by-email$ one, and both get rendered as regex locations by this template. nginx picks the first regex location that matches rather than the most specific one, so /v15/sso/get-by-email gets served by the /sso block and never reaches the block with the tighter regex.

I rendered the chart to check (helm template charts/nginz), both blocks land in the same server block in this order:

location ~* ^(/v[0-9]+)?/sso {                  ->  limit_req zone=reqs_per_addr_sso burst=10 nodelay;
location ~* ^(/v[0-9]+)?/sso/get-by-email$ {    ->  limit_req zone=reqs_per_addr_sso_get_by_email burst=15 nodelay;

So the endpoint ends up on 50r/s instead of the intended 5r/m. The $ doesn't do anything because the regex above already matched.

initiate-login and finalize-login are already above /sso, which suggests this was understood at the time and get-by-email$ just got appended at the end when it came in with #5024. Moving it above the /sso entry should be all it needs.

raisin it here because the two things interact: this PR widens which users the endpoint answers for (SCIM+SSO -> any SSO user), and config sets enableIdPByEmailDiscovery: true for every multi-ingress deployment, so more accounts become reachable through an endpoint whose brute force protection isn't running. Fixing the ordering doesn't close the endpoint as an oracle for a known address, but it does make enumeration slower.