fix(saml): use connection samlAudienceOverride as Issuer in SLO LogoutRequest - #4075
Open
LeighBriody24 wants to merge 1 commit into
Open
fix(saml): use connection samlAudienceOverride as Issuer in SLO LogoutRequest#4075LeighBriody24 wants to merge 1 commit into
LeighBriody24 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 |
LeighBriody24
marked this pull request as ready for review
August 10, 2026 10:27
Collaborator
|
Thanks @LeighBriody24, the team will review your PR soon. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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