multi-ingress: cross-IdP SSO by supersven · Pull Request #5212 · wireapp/wire-server

supersven · 2026-05-01T10:00:28Z

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

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 · 2026-05-12T15:23:38Z

+    -- This used to check if the user is SCIM AND SSO! The RFC ("2025-05-12
+    -- RFC: Default SSO flow for team by host domain") is not really
+    -- unambiguous about this. The customer currently provisions non-SCIM.
+    isSsoUser :: User -> Bool
+    isSsoUser = isJust . userSSOId


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 · 2026-05-13T06:30:23Z

+1. **NameID must be an email address.** Username-based `NameID`s are rejected
+   to avoid ambiguity across authenticating IdPs.


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

@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. 🤔

Copilot

Pull request overview

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

Allow users to login with all team IdPs. This is required because there's usually one IdP per ingress in a team and users should be able to login on all ingresses.

Copilot

Pull request overview

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

`'` vs `"`: Let's take `'` as this needs no additional quoting and is good enough.

zebot added the ok-to-test Approved for running tests in CI, overrides not-ok-to-test if both labels exist label May 1, 2026

supersven force-pushed the sventennie/multi-ingress-sso_cross_idp branch 3 times, most recently from 70c63ab to 63c5b7d Compare May 5, 2026 17:18

supersven force-pushed the sventennie/multi-ingress-sso_cross_idp branch 3 times, most recently from c70220c to e7f3bf2 Compare May 12, 2026 08:33

supersven changed the title ~~POC: multi ingress sso cross idp~~ multi-ingress: cross-IdP SSO May 12, 2026

supersven force-pushed the sventennie/multi-ingress-sso_cross_idp branch from e7f3bf2 to d27326c Compare May 12, 2026 09:37

supersven requested review from Copilot and sanojwr May 12, 2026 10:06

Copilot started reviewing on behalf of supersven May 12, 2026 10:07 View session

supersven marked this pull request as ready for review May 12, 2026 10:07

supersven requested review from a team as code owners May 12, 2026 10:07

supersven requested a review from emil-wire May 12, 2026 10:08

Copilot AI reviewed May 12, 2026

View reviewed changes

Comment thread services/spar/src/Spar/App.hs

Comment thread docs/src/developer/reference/config-options.md Outdated

Comment thread docs/src/developer/reference/config-options.md Outdated

Comment thread changelog.d/2-features/multi-ingress-cross-IdP-SSO Outdated

supersven commented May 12, 2026

View reviewed changes

fisx reviewed May 13, 2026

View reviewed changes

supersven force-pushed the sventennie/multi-ingress-sso_cross_idp branch 2 times, most recently from a149340 to 4cce453 Compare June 9, 2026 06:17

supersven force-pushed the sventennie/multi-ingress-sso_cross_idp branch 2 times, most recently from 82b25a8 to f9fea47 Compare July 6, 2026 16:15

supersven mentioned this pull request Jul 8, 2026

idpCertFingerprintAllowlist mandatory for multi-ingress SSO #5327

Merged

2 tasks

supersven force-pushed the sventennie/multi-ingress-sso_cross_idp branch from f9fea47 to c134d5d Compare July 9, 2026 09:55

supersven commented Jul 9, 2026

View reviewed changes

Comment thread services/spar/src/Spar/App.hs

supersven commented Jul 9, 2026

View reviewed changes

Comment thread services/spar/src/Spar/App.hs

supersven commented Jul 9, 2026

View reviewed changes

Comment thread services/spar/src/Spar/App.hs

supersven force-pushed the sventennie/multi-ingress-sso_cross_idp branch from 4113c24 to 9a88a59 Compare July 9, 2026 15:27

supersven requested a review from Copilot July 9, 2026 15:28

Copilot started reviewing on behalf of supersven July 9, 2026 15:29 View session

Copilot AI reviewed Jul 9, 2026

View reviewed changes

Comment thread services/spar/src/Spar/App.hs

Comment thread integration/test/Test/Spar/MultiIngressCrossIdpSso.hs

Comment thread changelog.d/2-features/multi-ingress-cross-IdP-SSO Outdated

Comment thread docs/src/developer/reference/config-options.md Outdated

supersven force-pushed the sventennie/multi-ingress-sso_cross_idp branch from 9a88a59 to cc9b20e Compare July 9, 2026 16:10

supersven requested a review from Copilot July 9, 2026 16:16

Copilot started reviewing on behalf of supersven July 9, 2026 16:16 View session

multi-ingress: Cross-IdP migration

9e877f6

Allow users to login with all team IdPs. This is required because there's usually one IdP per ingress in a team and users should be able to login on all ingresses.

supersven force-pushed the sventennie/multi-ingress-sso_cross_idp branch from e54062a to 9e877f6 Compare July 9, 2026 16:20

Copilot AI reviewed Jul 9, 2026

View reviewed changes

Comment thread services/spar/src/Spar/App.hs

Comment thread services/spar/src/Spar/App.hs

supersven requested a review from fisx July 10, 2026 05:49

Fix test by being consistent about quotes

c38e88b

`'` vs `"`: Let's take `'` as this needs no additional quoting and is good enough.

		1. NameID must be an email address. Username-based `NameID`s are rejected
		to avoid ambiguity across authenticating IdPs.

Uh oh!

Conversation

supersven commented May 1, 2026 • edited by fisx Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Checklist

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

supersven May 12, 2026

Choose a reason for hiding this comment

Uh oh!

fisx left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

fisx May 13, 2026

Choose a reason for hiding this comment

Uh oh!

supersven Jun 9, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

supersven commented May 1, 2026 •

edited by fisx

Loading