github-starred/better-auth
2
0
Fork 0
mirror of https://github.com/better-auth/better-auth.git synced 2026-10-18 11:32:15 -05:00
Code Issues 2.5k Packages Projects Releases 112 Wiki Activity

[GH-ISSUE #8607] SAML InResponseTo validation always fails — extract.inResponseTo is undefined, should be extract.response?.inResponseTo #11134

New Issue
Open
opened 2026-04-13 07:30:15 -05:00 by GiteaMirror · 2 comments
GiteaMirror commented 2026-04-13 07:30:15 -05:00
Owner
Copy Link
Copy Source

Originally created by @evanenochs on GitHub (Mar 14, 2026).
Original GitHub issue: https://github.com/better-auth/better-auth/issues/8607

Is this suited for github?

  • Yes, this is suited for github

To Reproduce

  1. Configure the SSO plugin with strict InResponseTo validation:
sso({
  saml: {
    enableInResponseToValidation: true,
    allowIdpInitiated: false,
  },
})
  1. Register a SAML provider (e.g., Microsoft Entra ID / Azure AD)
  2. Initiate an SP-initiated SSO login from your application (not from the IdP portal)
  3. Azure AD authenticates the user and sends a SAML Response back to the ACS endpoint with a valid InResponseTo attribute
  4. Login is rejected with: SAML IdP-initiated SSO rejected: InResponseTo missing and allowIdpInitiated is false

Current vs. Expected behavior

Expected: SP-initiated SAML login succeeds. The plugin reads InResponseTo from the parsed SAML Response, matches it against the stored AuthnRequest ID, and allows the login.

Actual: Every SP-initiated login is rejected. The plugin reads extract.inResponseTo which is always undefined. The InResponseTo value from Azure AD is actually present at extract.response.inResponseTo — samlify's extractor nests it under the response key. Because the plugin sees undefined, it misclassifies every SP-initiated response as IdP-initiated and rejects it.

The flow is SP-initiated. Azure AD correctly includes InResponseTo in its SAML Response. The plugin simply reads the wrong field path, so it can never find the value.

Proof using samlify's extractor directly:

const { extract, loginResponseFields } = require('samlify/build/src/extractor');

const xml = `<samlp:Response xmlns:samlp="urn:oasis:names:tc:SAML:2.0:protocol"
  xmlns:saml="urn:oasis:names:tc:SAML:2.0:assertion"
  ID="_resp1" Version="2.0" IssueInstant="2026-01-01T00:00:00Z"
  Destination="http://localhost/acs" InResponseTo="_req1">
  <saml:Issuer>https://idp.example.com</saml:Issuer>
  <samlp:Status><samlp:StatusCode Value="urn:oasis:names:tc:SAML:2.0:status:Success"/></samlp:Status>
  <saml:Assertion ID="_a1" Version="2.0" IssueInstant="2026-01-01T00:00:00Z">
    <saml:Issuer>https://idp.example.com</saml:Issuer>
    <saml:Subject><saml:NameID>user@example.com</saml:NameID></saml:Subject>
    <saml:Conditions NotBefore="2026-01-01T00:00:00Z" NotOnOrAfter="2026-01-01T01:00:00Z">
      <saml:AudienceRestriction><saml:Audience>http://localhost</saml:Audience></saml:AudienceRestriction>
    </saml:Conditions>
    <saml:AuthnStatement AuthnInstant="2026-01-01T00:00:00Z" SessionIndex="_s1"/>
  </saml:Assertion>
</samlp:Response>`;

const result = extract(xml, loginResponseFields(null));
console.log(result.inResponseTo);           // undefined  ← what the plugin reads
console.log(result.response.inResponseTo);  // "_req1"    ← where the value actually is

The logout response handler in the same file does it correctly:

// handleLogoutResponse — correct
const inResponseTo = extract?.response?.inResponseTo;

But both login handlers (callbackSSOSAML and acsEndpoint) do it wrong:

// callbackSSOSAML — incorrect
const inResponseTo = extract.inResponseTo;

// acsEndpoint — incorrect
const inResponseToAcs = extract.inResponseTo;

Suggested fix:

- const inResponseTo = extract.inResponseTo;
+ const inResponseTo = extract.response?.inResponseTo;

What version of Better Auth are you using?

1.5.5

System info

**System info:**
- `@better-auth/sso`: 1.5.5
- `samlify`: 2.10.2
- IdP: Microsoft Entra ID (Azure AD) with SAML 2.0
- Node.js 24+
- macOS

Which area(s) are affected? (Select all that apply)

Package

Auth config (if applicable)

Additional context

Additional context:

This is reproducible locally. The bug likely affects all environments since it's a code path issue, not environment-specific. The feature (enableInResponseToValidation) defaults to false, which is probably why it hasn't been reported — most users never enable strict validation. I verified the extract structure by running samlify's extractor directly against a valid SAML Response XML (reproduction script above).

Originally created by @evanenochs on GitHub (Mar 14, 2026). Original GitHub issue: https://github.com/better-auth/better-auth/issues/8607 ### Is this suited for github? - [x] Yes, this is suited for github ### To Reproduce 1. Configure the SSO plugin with strict InResponseTo validation: ```ts sso({ saml: { enableInResponseToValidation: true, allowIdpInitiated: false, }, }) ``` 2. Register a SAML provider (e.g., Microsoft Entra ID / Azure AD) 3. Initiate an SP-initiated SSO login from your application (not from the IdP portal) 4. Azure AD authenticates the user and sends a SAML Response back to the ACS endpoint with a valid `InResponseTo` attribute 5. Login is rejected with: `SAML IdP-initiated SSO rejected: InResponseTo missing and allowIdpInitiated is false` ### Current vs. Expected behavior **Expected:** SP-initiated SAML login succeeds. The plugin reads `InResponseTo` from the parsed SAML Response, matches it against the stored AuthnRequest ID, and allows the login. **Actual:** Every SP-initiated login is rejected. The plugin reads `extract.inResponseTo` which is always `undefined`. The `InResponseTo` value from Azure AD is actually present at `extract.response.inResponseTo` — `samlify`'s extractor nests it under the `response` key. Because the plugin sees `undefined`, it misclassifies every SP-initiated response as IdP-initiated and rejects it. **The flow is SP-initiated.** Azure AD correctly includes `InResponseTo` in its SAML Response. The plugin simply reads the wrong field path, so it can never find the value. Proof using samlify's extractor directly: ```js const { extract, loginResponseFields } = require('samlify/build/src/extractor'); const xml = `<samlp:Response xmlns:samlp="urn:oasis:names:tc:SAML:2.0:protocol" xmlns:saml="urn:oasis:names:tc:SAML:2.0:assertion" ID="_resp1" Version="2.0" IssueInstant="2026-01-01T00:00:00Z" Destination="http://localhost/acs" InResponseTo="_req1"> <saml:Issuer>https://idp.example.com</saml:Issuer> <samlp:Status><samlp:StatusCode Value="urn:oasis:names:tc:SAML:2.0:status:Success"/></samlp:Status> <saml:Assertion ID="_a1" Version="2.0" IssueInstant="2026-01-01T00:00:00Z"> <saml:Issuer>https://idp.example.com</saml:Issuer> <saml:Subject><saml:NameID>user@example.com</saml:NameID></saml:Subject> <saml:Conditions NotBefore="2026-01-01T00:00:00Z" NotOnOrAfter="2026-01-01T01:00:00Z"> <saml:AudienceRestriction><saml:Audience>http://localhost</saml:Audience></saml:AudienceRestriction> </saml:Conditions> <saml:AuthnStatement AuthnInstant="2026-01-01T00:00:00Z" SessionIndex="_s1"/> </saml:Assertion> </samlp:Response>`; const result = extract(xml, loginResponseFields(null)); console.log(result.inResponseTo); // undefined ← what the plugin reads console.log(result.response.inResponseTo); // "_req1" ← where the value actually is ``` The logout response handler in the same file does it correctly: ```js // handleLogoutResponse — correct const inResponseTo = extract?.response?.inResponseTo; ``` But both login handlers (callbackSSOSAML and acsEndpoint) do it wrong: ```js // callbackSSOSAML — incorrect const inResponseTo = extract.inResponseTo; // acsEndpoint — incorrect const inResponseToAcs = extract.inResponseTo; ``` **Suggested fix:** ```diff - const inResponseTo = extract.inResponseTo; + const inResponseTo = extract.response?.inResponseTo; ``` ### What version of Better Auth are you using? 1.5.5 ### System info ```bash **System info:** - `@better-auth/sso`: 1.5.5 - `samlify`: 2.10.2 - IdP: Microsoft Entra ID (Azure AD) with SAML 2.0 - Node.js 24+ - macOS ``` ### Which area(s) are affected? (Select all that apply) Package ### Auth config (if applicable) ```typescript ``` ### Additional context Additional context: This is reproducible locally. The bug likely affects all environments since it's a code path issue, not environment-specific. The feature (enableInResponseToValidation) defaults to false, which is probably why it hasn't been reported — most users never enable strict validation. I verified the extract structure by running samlify's extractor directly against a valid SAML Response XML (reproduction script above).
GiteaMirror added the bugidentity labels 2026-04-13 07:30:15 -05:00
GiteaMirror commented 2026-04-13 07:30:16 -05:00
Author
Owner
Copy Link
Copy Source

@dosubot[bot] commented on GitHub (Mar 14, 2026):

Hi @evanenochs! I'm Dosu and I’m helping the better-auth team.

I can confirm this bug exists. Looking at the code in packages/sso/src/routes/sso.ts, both callbackSSOSAML and acsEndpoint incorrectly read:

const inResponseTo = (extract as SAMLAssertionExtract).inResponseTo as
    | string
    | undefined;

Meanwhile, handleLogoutResponse correctly uses:

const inResponseTo = extract?.response?.inResponseTo;

Your suggested fix is correct — the login handlers should use extract.response?.inResponseTo to match the samlify structure and the existing logout handler pattern. This bug effectively breaks the replay attack protection that was introduced in PR #6557.

To reply, just mention @dosu.

How did I do? Good | Irrelevant | Incorrect | Verbose | Hallucination | Report 🐛 | Other

Chat with Dosu Open in Cursor Join Discord Share on X

<!-- gh-comment-id:4061132551 --> @dosubot[bot] commented on GitHub (Mar 14, 2026): <!-- Greeting --> Hi @evanenochs! I'm [Dosu](https://go.dosu.dev/dosubot) and I’m helping the better-auth team. <!-- Answer --> I can confirm this bug exists. Looking at the code in [`packages/sso/src/routes/sso.ts`](https://github.com/better-auth/better-auth/blob/64339f06459b0ff940cae8bbda705371c216de39/packages/sso/src/routes/sso.ts#L2176-L2178), both `callbackSSOSAML` and `acsEndpoint` incorrectly read: ```typescript const inResponseTo = (extract as SAMLAssertionExtract).inResponseTo as | string | undefined; ``` Meanwhile, [`handleLogoutResponse`](https://github.com/better-auth/better-auth/blob/64339f06459b0ff940cae8bbda705371c216de39/packages/sso/src/routes/sso.ts#L3112) correctly uses: ```typescript const inResponseTo = extract?.response?.inResponseTo; ``` Your suggested fix is correct — the login handlers should use `extract.response?.inResponseTo` to match the samlify structure and the existing logout handler pattern. This bug effectively breaks the replay attack protection that was introduced in [PR #6557](https://github.com/better-auth/better-auth/pull/6557). <!-- Dosu Comment Footer --> *To reply, just mention [@dosu](https://go.dosu.dev/dosubot?utm_source=github&utm_medium=bot-comment&utm_campaign=github-assistant&utm_content=brand-link).* --- <sup>How did I do? [Good](https://app.dosu.dev/response-feedback/f077d0b4-d5ab-4089-96e2-14d427113c77?feedback_type=great_response&utm_source=github&utm_medium=bot-comment&utm_campaign=github-assistant&utm_content=feedback-great_response) | [Irrelevant](https://app.dosu.dev/response-feedback/f077d0b4-d5ab-4089-96e2-14d427113c77?feedback_type=irrelevant_answer&utm_source=github&utm_medium=bot-comment&utm_campaign=github-assistant&utm_content=feedback-irrelevant_answer) | [Incorrect](https://app.dosu.dev/response-feedback/f077d0b4-d5ab-4089-96e2-14d427113c77?feedback_type=incorrect_sources&utm_source=github&utm_medium=bot-comment&utm_campaign=github-assistant&utm_content=feedback-incorrect_sources) | [Verbose](https://app.dosu.dev/response-feedback/f077d0b4-d5ab-4089-96e2-14d427113c77?feedback_type=too_verbose&utm_source=github&utm_medium=bot-comment&utm_campaign=github-assistant&utm_content=feedback-too_verbose) | [Hallucination](https://app.dosu.dev/response-feedback/f077d0b4-d5ab-4089-96e2-14d427113c77?feedback_type=hallucination&utm_source=github&utm_medium=bot-comment&utm_campaign=github-assistant&utm_content=feedback-hallucination) | [Report 🐛](https://app.dosu.dev/response-feedback/f077d0b4-d5ab-4089-96e2-14d427113c77?feedback_type=bug_report&utm_source=github&utm_medium=bot-comment&utm_campaign=github-assistant&utm_content=feedback-bug_report) | [Other](https://app.dosu.dev/response-feedback/f077d0b4-d5ab-4089-96e2-14d427113c77?feedback_type=other&utm_source=github&utm_medium=bot-comment&utm_campaign=github-assistant&utm_content=feedback-other)</sup> [![Chat with Dosu](https://dosu.dev/dosu-chat-badge.svg)](https://app.dosu.dev/cdda13d9-dd27-4d31-b09a-5d8bec92de21/ask?utm_source=github&utm_medium=bot-comment&utm_campaign=github-assistant&utm_content=chat-badge)&nbsp;[![Open in Cursor](https://dosu.dev/dosu-in-cursor.svg)](https://cursor.com/link/prompt?text=I%20can%20confirm%20this%20bug%20exists.%20Looking%20at%20the%20code%20in%20%5B%60packages/sso/src/routes/sso.ts%60%5D%28https%3A//github.com/better-auth/better-auth/blob/64339f06459b0ff940cae8bbda705371c216de39/packages/sso/src/routes/sso.ts%23L2176-L2178%29%2C%20both%20%60callbackSSOSAML%60%20and%20%60acsEndpoint%60%20incorrectly%20read%3A%0A%0A%60%60%60typescript%0Aconst%20inResponseTo%20%3D%20%28extract%20as%20SAMLAssertionExtract%29.inResponseTo%20as%0A%20%20%20%20%7C%20string%0A%20%20%20%20%7C%20undefined%3B%0A%60%60%60%0A%0AMeanwhile%2C%20%5B%60handleLogoutResponse%60%5D%28https%3A//github.com/better-auth/better-auth/blob/64339f06459b0ff940cae8bbda705371c216de39/packages/sso/src/routes/sso.ts%23L3112%29%20correctly%20uses%3A%0A%0A%60%60%60typescript%0Aconst%20inResponseTo%20%3D%20extract%3F.response%3F.inResponseTo%3B%0A%60%60%60%0A%0AYour%20suggested%20fix%20is%20correct%20%E2%80%94%20the%20login%20handlers%20should%20use%20%60extract.response%3F.inResponseTo%60%20to%20match%20the%20samlify%20structure%20and%20the%20existing%20logout%20handler%20pattern.%20This%20bug%20effectively%20breaks%20the%20replay%20attack%20protection%20that%20was%20introduced%20in%20%5BPR%20%236557%5D%28https%3A//github.com/better-auth/better-auth/pull/6557%29.)&nbsp;[![Join Discord](https://img.shields.io/badge/join-5865F2?logo=discord&logoColor=white&label=)](https://go.dosu.dev/discord-bot?utm_source=github&utm_medium=bot-comment&utm_campaign=github-assistant&utm_content=join-discord)&nbsp;[![Share on X](https://img.shields.io/badge/X-share-black)](https://twitter.com/intent/tweet?text=%40dosu_ai%20helped%20me%20solve%20this%20issue!&url=https%3A//github.com/better-auth/better-auth/issues/8607)
GiteaMirror commented 2026-04-13 07:30:17 -05:00
Author
Owner
Copy Link
Copy Source

@evanenochs commented on GitHub (Mar 14, 2026):

@dosu I have a pr for it https://github.com/better-auth/better-auth/pull/8608

<!-- gh-comment-id:4061167801 --> @evanenochs commented on GitHub (Mar 14, 2026): @dosu I have a pr for it https://github.com/better-auth/better-auth/pull/8608
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
adapter
astro
awaiting external contributor
blocked
breaking
breaking change
bug
c-devops
core
credentials
database
dependencies
devops
devtools
docs
documentation
duplicate
elysia
enhancement
enterprise
expo
express
fastify
good first issue
help wanted
hono
identity
infra
integration
invalid
javascript
locked
maintenance
need-more-information
needs: info
needs: repro
nextjs
nuxt
oauth
organization
P0
payments
perf
platform
plugin
pull-request
Mirrored from GitHub Pull Request
 question
ready
regression
remix
security
social-provider
solid
stale
svelte
tanstack-start
tracking
version-bump
vue
wontfix
No labels bug identity
Milestone
No items
No Milestone
Projects
Clear projects
No projects
Assignees
Clear assignees
GiteaMirror ninjasurge (Kaehlen Rog)
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: github-starred/better-auth#11134