ory / ory/polis

Single Logout uses global samlAudience as Issuer instead of the connection's samlAudienceOverride

Open
#4,076 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
TypeScript
Stars
2.3k
Forks
233
PR merge metrics
No merged PRs in 30d

Description

Issue Summary

When Polis builds the SAML LogoutRequest, it uses the global opts.samlAudience as the <saml:Issuer> and never reads the connection's samlAudienceOverride. The login path does use the override (connection.samlAudienceOverride ?? samlAudience), so any connection with a samlAudienceOverride sends one Issuer at login and a different one at logout.

Whether that mismatch actually breaks logout comes down to how strictly the IdP checks the session participant. Microsoft Entra / Azure AD checks it strictly: it recorded the participant under the override Issuer at login, then gets a logout claiming the global Issuer, can't find a matching participant, and returns AADSTS50068: "Signout failed. The initiating application is not a participant in the current session." The user hits an error page and the IdP session is never closed. More lenient IdPs accept the mismatched Issuer, so logout looks fine and nobody notices.

So the wrong Issuer goes out for every override connection. Strict IdPs like Entra are just where it turns into a visible logout failure.

Steps to Reproduce
  1. Create a SAML connection with a samlAudienceOverride (e.g. https://saml.boxyhq.com/custom-sp-entity-id), registered at the IdP as the SP entity ID / identifier.
  2. Log in via SSO. The AuthnRequest <saml:Issuer> is the override, and the IdP records it as the session participant.
  3. Trigger SLO through LogoutController.createRequest.
  4. Decode the generated LogoutRequest. Its <saml:Issuer> is the global samlAudience, not the override.
  5. Against a strict IdP (Entra/Azure) the logout is rejected with AADSTS50068 and the IdP session stays open. Against a lenient IdP the mismatch is accepted silently.

Expected: the logout Issuer should match the one used at login (the override), so the IdP recognises the participant and completes SLO. You can check this directly on the emitted <saml:Issuer> without needing a specific IdP.

Technical details
  • Root cause: npm/src/controller/logout.ts, createRequest sets providerName: this.opts.samlAudience!, which becomes the <saml:Issuer>. It never reads samlAudienceOverride, even though the connection is loaded a few lines above.
  • Login path for comparison: npm/src/controller/sso-handler.ts uses connection.samlAudienceOverride ?? samlAudience (same pattern on the OAuth path in oauth.ts).
  • The LogoutRequest also leaves out <samlp:SessionIndex>. That isn't what's failing here (the Issuer mismatch fails first), but some strict IdPs use SessionIndex to pick the exact session, so it may be worth a look once the Issuer is fixed.
  • Node.js version: 22.x, reproduced locally against the npm test suite.
  • Reproduced with a unit test that inflates the LogoutRequest and asserts the <saml:Issuer>.
Proposed fix

I've opened #4075 with a fix. It makes logout read the override with the same fallback the login path uses:

providerName: samlAudienceOverride ?? this.opts.samlAudience!,

It also adds the optional samlAudienceOverride field to SAMLConnection, plus two tests: with an override the logout Issuer is the override, and without one it falls back to the global samlAudience (no regression). Connections without an override produce the same output as today.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start at npm/src/controller/logout.ts, specifically LogoutController.createRequest, and compare its issuer handling with npm/src/controller/sso-handler.ts. Inspect the SAMLConnection definition and the existing unit test that inflates the LogoutRequest, then run the npm test suite and verify the emitted Issuer for both override and fallback cases.

Written by the indexing model from the issue text.

Assessment

Tech stack
nodejs, typescript
Domain
authentication
Issue type
Bug
Difficulty
2/5
Estimated time
1-3 hours
Activity status
Stale
Clarity
Clearly specified
Newbie friendliness
25/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.