Skip to content

Remove single IdP special-case for /sso/get-by-email#5291

Draft
supersven wants to merge 3 commits into
developfrom
sventennie/remove-single-idp-specialcase
Draft

Remove single IdP special-case for /sso/get-by-email#5291
supersven wants to merge 3 commits into
developfrom
sventennie/remove-single-idp-specialcase

Conversation

@supersven

Copy link
Copy Markdown
Contributor

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

Checklist

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

@supersven supersven requested a review from Copilot June 25, 2026 14:32
@zebot zebot added the ok-to-test Approved for running tests in CI, overrides not-ok-to-test if both labels exist label Jun 25, 2026

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 updates the IdP selection logic for /sso/get-by-email so that a single configured IdP is no longer automatically returned; instead, the IdP must match the requested domain (including the Nothing/unset case).

Changes:

  • Removed the “single IdP always matches” branch in getSsoCodeByEmailImpl, delegating all non-empty cases to domain-based matching.
  • Updated the unit property test to assert Just idpId only when the IdP’s configured domain matches the requested domain.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
libs/wire-subsystems/src/Wire/IdPSubsystem/Interpreter.hs Removes the singleton-IdP shortcut and always performs domain-based IdP selection for non-empty IdP lists.
libs/wire-subsystems/test/unit/Wire/IdPSubsystem/InterpreterSpec.hs Adjusts the property test expectations to align with the updated domain-matching behavior.

[idp] -> (pure . pure) (idp ^. SAML.idpId)
pure Nothing
selectIdP idps =
-- Multiple IdPs: find by domain if host provided
…ress mode

When multi-ingress is active, each IdP has a domain field that ties it to a specific ingress point. Reject initiate-login requests where the incoming Z-Host header does not match the IdP's configured domain (404 idp-not-for-this-domain). Single-ingress deployments unaffected.

Changes:
- services/spar/src/Spar/Error.hs: add SparIdPNotForThisDomain error
- services/spar/src/Spar/API.hs: pass samlConfig to authreq, add domain validation
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.

3 participants