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

[PR #7684] [CLOSED] fix(sso): correct IdentityProvider configuration in signInSSO #7491

New Issue
Closed
opened 2026-03-13 13:38:55 -05:00 by GiteaMirror · 0 comments
GiteaMirror commented 2026-03-13 13:38:55 -05:00
Owner
Copy Link

📋 Pull Request Information

Original PR: https://github.com/better-auth/better-auth/pull/7684
Author: @theNailz
Created: 1/29/2026
Status: ❌ Closed

Base: main ← Head: canary

📝 Commits (10+)

  • 2a80e02 docs: document disableOriginCheck in options.mdx (#7199)
  • 064ee71 fix: set Location header on redirected responses (#6232)
  • bdd3062 fix(anonymous): prevent Convex cleanup from deleting fresh sessions (#5825)
  • 870acfc chore: fix missing AnonymousSession import (#7203)
  • d7621cc fix(passkey): add error logs during client verification error (#7193)
  • 6ac030c chore: clean up unused import (#7212)
  • ca9dfd7 fix(one-tap): respect user dismiss actions in prompt retry logic (#7211)
  • d3c5de4 fix(email-verification): sending email verification of another user fails with EMAIL_ALREADY_VERIFIED (#7215)
  • 706a422 docs(one-tap): add missing option and update to clearer description (#7214)
  • a0f4ff1 fix(oauth): handle unencrypted tokens when encryptOAuthTokens is enabled (#7210)

📊 Changes

741 files changed (+52732 additions, -22092 deletions)

View changed files

➕ .claude/rules/release.md (+70 -0)
📝 .cspell/alt-languages.txt (+9 -1)
📝 .cspell/company-names.txt (+2 -1)
📝 .cspell/names.txt (+21 -1)
📝 .cspell/tech-terms.txt (+5 -1)
📝 .cspell/third-party.txt (+2 -1)
➖ .github/workflows/adapter-tests.yml (+0 -71)
➖ .github/workflows/auto-cherry-pick-to-main.yml (+0 -337)
➖ .github/workflows/cherry-pick-to-main.yml (+0 -325)
📝 .github/workflows/ci.yml (+23 -20)
➕ .github/workflows/claude.yml (+53 -0)
📝 .github/workflows/e2e.yml (+85 -25)
📝 .github/workflows/preview.yml (+7 -5)
📝 .github/workflows/release.yml (+9 -2)
📝 .gitignore (+2 -0)
➖ .npmrc (+0 -1)
📝 .nvmrc (+1 -1)
➕ .postmortem/client-side-import-server.md (+141 -0)
➕ .postmortem/tanstack-start-server-core.md (+104 -0)
➕ .remarkignore (+9 -0)

...and 80 more files

📄 Description

Problem

The signInSSO endpoint incorrectly constructs the SAML IdentityProvider, causing:

  • AssertionConsumerServiceURL to contain invalid data instead of the callback URL
  • <saml:Issuer> to contain invalid data instead of the issuer URL

This results in Azure AD (and likely other IdPs) rejecting the SAML AuthnRequest with:
AADSTS7500511: XML attribute 'AssertionConsumerServiceURL' in the SAML message must be a URI

Root Cause

In signInSSO, the IdentityProvider is constructed with:

  1. Wrong field name: encryptCert instead of signingCert - samlify ignores encryptCert for signing
  2. Missing fallback: entityID doesn't fall back to parsedSamlConfig.issuer
  3. Missing fallback: singleSignOnService doesn't fall back to parsedSamlConfig.entryPoint

The callback handlers (callbackSSOSAML, acsEndpoint) already implement this correctly.

Solution

Align the signInSSO implementation with the callback handlers by:

  1. Using signingCert instead of encryptCert
  2. Adding fallback to parsedSamlConfig.issuer for entityID
  3. Adding fallback to parsedSamlConfig.entryPoint for singleSignOnService
  4. Handling both metadata and non-metadata IdP configurations

Testing

  • Tested with Azure AD SAML SSO
  • SAML AuthnRequest now contains correct AssertionConsumerServiceURL and Issuer
  • Response validation works correctly

Fixes #6609

Summary by cubic

Fixes SAML SSO AuthnRequest construction in signInSSO so AssertionConsumerServiceURL and Issuer are correct, resolving Azure AD rejections. Also streamlines CI, docs, and demos for better DX.

  • Bug Fixes

    • Use signingCert instead of encryptCert when building the IdP.
    • Add fallbacks: entityID -> issuer and singleSignOnService -> entryPoint.
    • Support both metadata and manual IdP configs in signInSSO.
    • Validated with Azure AD; AuthnRequest and response validation now pass. Fixes #6609.

  • Refactors

    • CI: honor .nvmrc, add format check, unify Turbo remote cache, remove legacy cherry-pick workflows, add Claude Code workflow.
    • Docs: replace static sitemap with dynamic route; rebuild code tabs (Next.js/Nuxt/SvelteKit); accessibility and content fixes.
    • Demos: polish Next.js UI, expand CORS methods, integrate Dash/SCIM/SSO, add ripple background; update packages/configs.
    • Tooling: add CLAUDE.md, release rules, remark config; bump Node to 22.22.0; add Redis to docker-compose.

Written for commit 26d7916692. Summary will update on new commits.

🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.

## 📋 Pull Request Information **Original PR:** https://github.com/better-auth/better-auth/pull/7684 **Author:** [@theNailz](https://github.com/theNailz) **Created:** 1/29/2026 **Status:** ❌ Closed **Base:** `main` ← **Head:** `canary` --- ### 📝 Commits (10+) - [`2a80e02`](https://github.com/better-auth/better-auth/commit/2a80e02ad40080ca5b210cb823517290270eef4b) docs: document `disableOriginCheck` in options.mdx (#7199) - [`064ee71`](https://github.com/better-auth/better-auth/commit/064ee71b7f1d96cbd7855d2407ece8f31d347e09) fix: set Location header on redirected responses (#6232) - [`bdd3062`](https://github.com/better-auth/better-auth/commit/bdd3062b30ba09a4fdd0ed5922afb4366324a91d) fix(anonymous): prevent Convex cleanup from deleting fresh sessions (#5825) - [`870acfc`](https://github.com/better-auth/better-auth/commit/870acfcd5c1548ac2e4da5f284c53231749b1075) chore: fix missing AnonymousSession import (#7203) - [`d7621cc`](https://github.com/better-auth/better-auth/commit/d7621ccaf5bc51b7e49898b72747c694b9c8c89c) fix(passkey): add error logs during client verification error (#7193) - [`6ac030c`](https://github.com/better-auth/better-auth/commit/6ac030c3c57cc1f4c5149fd900681672eb6337b0) chore: clean up unused import (#7212) - [`ca9dfd7`](https://github.com/better-auth/better-auth/commit/ca9dfd78e6770fcfd0b299228c971dc70de7453a) fix(one-tap): respect user dismiss actions in prompt retry logic (#7211) - [`d3c5de4`](https://github.com/better-auth/better-auth/commit/d3c5de4a5f05f92e44c47a6a1dacadc422d1162e) fix(email-verification): sending email verification of another user fails with EMAIL_ALREADY_VERIFIED (#7215) - [`706a422`](https://github.com/better-auth/better-auth/commit/706a422f09799e79819cfb8f5cdc991b3c278d51) docs(one-tap): add missing option and update to clearer description (#7214) - [`a0f4ff1`](https://github.com/better-auth/better-auth/commit/a0f4ff1ea24d76369e7ed380c4ddc139c68aa9d2) fix(oauth): handle unencrypted tokens when encryptOAuthTokens is enabled (#7210) ### 📊 Changes **741 files changed** (+52732 additions, -22092 deletions) <details> <summary>View changed files</summary> ➕ `.claude/rules/release.md` (+70 -0) 📝 `.cspell/alt-languages.txt` (+9 -1) 📝 `.cspell/company-names.txt` (+2 -1) 📝 `.cspell/names.txt` (+21 -1) 📝 `.cspell/tech-terms.txt` (+5 -1) 📝 `.cspell/third-party.txt` (+2 -1) ➖ `.github/workflows/adapter-tests.yml` (+0 -71) ➖ `.github/workflows/auto-cherry-pick-to-main.yml` (+0 -337) ➖ `.github/workflows/cherry-pick-to-main.yml` (+0 -325) 📝 `.github/workflows/ci.yml` (+23 -20) ➕ `.github/workflows/claude.yml` (+53 -0) 📝 `.github/workflows/e2e.yml` (+85 -25) 📝 `.github/workflows/preview.yml` (+7 -5) 📝 `.github/workflows/release.yml` (+9 -2) 📝 `.gitignore` (+2 -0) ➖ `.npmrc` (+0 -1) 📝 `.nvmrc` (+1 -1) ➕ `.postmortem/client-side-import-server.md` (+141 -0) ➕ `.postmortem/tanstack-start-server-core.md` (+104 -0) ➕ `.remarkignore` (+9 -0) _...and 80 more files_ </details> ### 📄 Description ## Problem The `signInSSO` endpoint incorrectly constructs the SAML `IdentityProvider`, causing: - `AssertionConsumerServiceURL` to contain invalid data instead of the callback URL - `<saml:Issuer>` to contain invalid data instead of the issuer URL This results in Azure AD (and likely other IdPs) rejecting the SAML AuthnRequest with: `AADSTS7500511: XML attribute 'AssertionConsumerServiceURL' in the SAML message must be a URI` ## Root Cause In `signInSSO`, the `IdentityProvider` is constructed with: 1. Wrong field name: `encryptCert` instead of `signingCert` - samlify ignores `encryptCert` for signing 2. Missing fallback: `entityID` doesn't fall back to `parsedSamlConfig.issuer` 3. Missing fallback: `singleSignOnService` doesn't fall back to `parsedSamlConfig.entryPoint` The callback handlers (`callbackSSOSAML`, `acsEndpoint`) already implement this correctly. ## Solution Align the `signInSSO` implementation with the callback handlers by: 1. Using `signingCert` instead of `encryptCert` 2. Adding fallback to `parsedSamlConfig.issuer` for `entityID` 3. Adding fallback to `parsedSamlConfig.entryPoint` for `singleSignOnService` 4. Handling both metadata and non-metadata IdP configurations ## Testing - Tested with Azure AD SAML SSO - SAML AuthnRequest now contains correct `AssertionConsumerServiceURL` and `Issuer` - Response validation works correctly Fixes #6609 <!-- This is an auto-generated description by cubic. --> --- ## Summary by cubic Fixes SAML SSO AuthnRequest construction in signInSSO so AssertionConsumerServiceURL and Issuer are correct, resolving Azure AD rejections. Also streamlines CI, docs, and demos for better DX. - Bug Fixes - Use signingCert instead of encryptCert when building the IdP. - Add fallbacks: entityID -> issuer and singleSignOnService -> entryPoint. - Support both metadata and manual IdP configs in signInSSO. - Validated with Azure AD; AuthnRequest and response validation now pass. Fixes #6609. - Refactors - CI: honor .nvmrc, add format check, unify Turbo remote cache, remove legacy cherry-pick workflows, add Claude Code workflow. - Docs: replace static sitemap with dynamic route; rebuild code tabs (Next.js/Nuxt/SvelteKit); accessibility and content fixes. - Demos: polish Next.js UI, expand CORS methods, integrate Dash/SCIM/SSO, add ripple background; update packages/configs. - Tooling: add CLAUDE.md, release rules, remark config; bump Node to 22.22.0; add Redis to docker-compose. <sup>Written for commit 26d79166926483f1425ca7bc8335f78f024610f6. Summary will update on new commits.</sup> <!-- End of auto-generated description by cubic. --> --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>
GiteaMirror added the pull-request label 2026-03-13 13:38:55 -05:00
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 Label pull-request
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
GiteaMirror ninjasurge
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: github-starred/better-auth#7491
Delete Branch "%!s()"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?