fix(saml): use connection samlAudienceOverride as Issuer in SLO LogoutRequest - #4075
fix(saml): use connection samlAudienceOverride as Issuer in SLO LogoutRequest#4075LeighBriody24 wants to merge 1 commit into
Conversation
…tRequest Single Logout fails for any SAML connection with a samlAudienceOverride set. The LogoutRequest was built with the global samlAudience as its <saml:Issuer>, ignoring the per-connection override used at login. Strict IdPs (e.g. Microsoft Entra) reject the mismatched Issuer with AADSTS50068, leaving the session open. Read samlAudienceOverride off the connection when building the LogoutRequest, with the same fallback to the global audience as the login path. Connections without an override are unaffected. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
📝 WalkthroughWalkthroughThe SAML connection supports an optional audience override. Logout request generation uses the override when configured and falls back to the global audience. Tests decode the compressed request and verify both issuer values. ChangesSAML logout audience selection
Estimated code review effort: 2 (Simple) | ~10 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Thanks @LeighBriody24, the team will review your PR soon. |
|
|
Fixes #4076
Summary
When Polis builds the SAML
LogoutRequest, it uses the globalsamlAudienceas the<saml:Issuer>and ignores the connection'ssamlAudienceOverride. The login path uses the override (connection.samlAudienceOverride ?? samlAudience), so any connection that setssamlAudienceOverridesends a different Issuer at login and at logout.Whether that breaks logout depends on the IdP. Strict IdPs (Microsoft Entra / Azure AD in particular) reject it, because the Issuer doesn't match the participant recorded at login, returning
AADSTS50068and leaving the IdP session open (screenshot below). Lenient IdPs accept the mismatch, which is why it isn't always visible.What does this PR do?
For SLO to work, login and logout have to send the same SP
Issuer. The login path already handles this: it usesconnection.samlAudienceOverride ?? samlAudience. The logout path doesn't; it always sendsopts.samlAudience.This change makes logout read
samlAudienceOverrideoff the connection when building theLogoutRequest, falling back to the global audience just like login does. Connections without an override are unaffected.Problem
LogoutController.createRequestbuilt the request like this:When a connection has a
samlAudienceOverride, the IdP records that override as the session participant at login. Logout then arrives claiming the global audience instead, so the IdP has no matching participant to sign out (the error above).Solution
Same behaviour as the login path.
SAMLConnectionalso gains the optionalsamlAudienceOverridefield.Type of change
How should this be tested?
Two cases added to
test/sso/logout.test.ts:With
samlAudienceOverrideset, theLogoutRequest<saml:Issuer>is the override value.Without it, the
Issuerfalls back to the globalsamlAudience(no regression).New and existing unit tests pass locally with my changes
Existing unit tests
Checklist