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 #9097] [MERGED] fix(sso): unify SAML response processing and fix bugs #25335

New Issue
Closed
opened 2026-04-15 22:50:34 -05:00 by GiteaMirror · 0 comments
GiteaMirror commented 2026-04-15 22:50:34 -05:00
Owner
Copy Link

📋 Pull Request Information

Original PR: https://github.com/better-auth/better-auth/pull/9097
Author: @gustavovalverde
Created: 4/10/2026
Status: ✅ Merged
Merged: 4/10/2026
Merged by: @gustavovalverde

Base: main ← Head: fix/sso-saml-hardening

📝 Commits (9)

  • d8c8b9b fix(sso)!: unify SAML response processing, fix provider lookup and config bugs
  • ee14282 chore: add changeset for SSO SAML hardening
  • 960662c fix(sso): break circular dependency, unexport internal function
  • c85e5c8 fix(sso): sanitize error redirects, tighten entryPoint validation
  • b856238 fix(sso): align docs with actual callbackUrl semantics, restore ACS redirects
  • 9acc5a9 fix(sso): preserve existing query params in error redirect URL
  • a05e8d3 docs: expand changeset with behavioral changes and migration notes
  • 2b2b813 fix(sso): preserve backward-compatible error codes in ACS redirects
  • 4546e4d fix(sso): remove process-specific comment from test block

📊 Changes

10 files changed (+1335 additions, -1411 deletions)

View changed files

➕ .changeset/sso-saml-hardening.md (+25 -0)
📝 docs/content/docs/guides/saml-sso-with-okta.mdx (+2 -3)
📝 docs/content/docs/guides/supabase-migration-guide.mdx (+44 -19)
📝 docs/content/docs/plugins/sso.mdx (+209 -248)
📝 packages/sso/src/routes/helpers.ts (+40 -16)
➕ packages/sso/src/routes/saml-pipeline.ts (+529 -0)
📝 packages/sso/src/routes/sso.ts (+85 -1125)
📝 packages/sso/src/saml.test.ts (+315 -0)
➕ packages/sso/src/saml/timestamp.ts (+78 -0)
📝 packages/sso/src/types.ts (+8 -0)

📄 Description

Summary

  • Extract duplicated SAML response handling from callbackSSOSAML and acsEndpoint into a shared processSAMLResponse pipeline
  • Complete createSP/createIdP helpers with all encryption and signing fields so both endpoints produce identical SP/IdP objects
  • Fix SP metadata endpoint using internal row ID instead of providerId in ACS URL fallback
  • Fix acsEndpoint skipping DB lookup when defaultSSO was configured, and falling back to defaultSSO[0] for unmatched providerIds
  • Fix acsEndpoint missing encryption fields (isAssertionEncrypted, encPrivateKey) causing silent decryption failures
  • Add registration-time validation: reject configs with no usable IdP entry point
  • Fix Supabase migration guide generating unsupported metadataUrl field; fetch IdP metadata XML inline instead
  • Fix contradictory callbackUrl guidance across SSO docs
  • Add TDD tests for provider lookup fallback, registration validation, and RelayState priority

Summary by cubic

Unifies SAML response handling in @better-auth/sso, restores browser-friendly ACS redirects with sanitized, query-safe URLs, and aligns docs to treat callbackUrl as the ACS URL. Keeps ACS error codes in backward‑compatible lowercase.

  • Refactors

    • Added shared processSAMLResponse for ACS and callback, removing duplicate logic.
    • Completed createSP/createIdP with full signing/encryption, nameIDFormat, SLO, RelayState; createSP now falls back to a generated ACS URL when callbackUrl is missing.
    • Extracted validateSAMLTimestamp to saml/timestamp.ts (re-exported) and made extractAssertionId internal.

  • Bug Fixes

    • Corrected ACS URL/provider lookup: SP metadata uses public providerId; ACS always queries the DB and never falls back to defaultSSO[0]; fixed defaultSSO parsing in the callback path.
    • Added missing encryption fields (isAssertionEncrypted, encPrivateKey) to prevent silent decryption failures.
    • Registration now requires IdP metadata XML, singleSignOnService, and a valid entryPoint (validated via new URL()).
    • Restored browser-friendly ACS error redirects, sanitized redirect targets, preserved existing query params, and kept error codes in lowercase snake_case (e.g., error=multiple_assertions) for compatibility.

Written for commit 4546e4d09b. 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/9097 **Author:** [@gustavovalverde](https://github.com/gustavovalverde) **Created:** 4/10/2026 **Status:** ✅ Merged **Merged:** 4/10/2026 **Merged by:** [@gustavovalverde](https://github.com/gustavovalverde) **Base:** `main` ← **Head:** `fix/sso-saml-hardening` --- ### 📝 Commits (9) - [`d8c8b9b`](https://github.com/better-auth/better-auth/commit/d8c8b9b319aec4131dc00277f0de81d289e92c0b) fix(sso)!: unify SAML response processing, fix provider lookup and config bugs - [`ee14282`](https://github.com/better-auth/better-auth/commit/ee142826c4eb5d803319e192a6c0d051d8b93592) chore: add changeset for SSO SAML hardening - [`960662c`](https://github.com/better-auth/better-auth/commit/960662ce40f363c648ab80df86b6c400fb71d15f) fix(sso): break circular dependency, unexport internal function - [`c85e5c8`](https://github.com/better-auth/better-auth/commit/c85e5c8b1a332d9156024c987fbc999484c41c9e) fix(sso): sanitize error redirects, tighten entryPoint validation - [`b856238`](https://github.com/better-auth/better-auth/commit/b8562386529ca22bc8f03846b2a04d516cc04639) fix(sso): align docs with actual callbackUrl semantics, restore ACS redirects - [`9acc5a9`](https://github.com/better-auth/better-auth/commit/9acc5a93ac136b871ee721ca45d18b83813886a5) fix(sso): preserve existing query params in error redirect URL - [`a05e8d3`](https://github.com/better-auth/better-auth/commit/a05e8d38bccd9267f229fa6b95b9931983d04bc9) docs: expand changeset with behavioral changes and migration notes - [`2b2b813`](https://github.com/better-auth/better-auth/commit/2b2b813f29c2c444a0a4dbce0a0db05bcff84e9a) fix(sso): preserve backward-compatible error codes in ACS redirects - [`4546e4d`](https://github.com/better-auth/better-auth/commit/4546e4d09b11af11d42d9a112e16d2390060f076) fix(sso): remove process-specific comment from test block ### 📊 Changes **10 files changed** (+1335 additions, -1411 deletions) <details> <summary>View changed files</summary> ➕ `.changeset/sso-saml-hardening.md` (+25 -0) 📝 `docs/content/docs/guides/saml-sso-with-okta.mdx` (+2 -3) 📝 `docs/content/docs/guides/supabase-migration-guide.mdx` (+44 -19) 📝 `docs/content/docs/plugins/sso.mdx` (+209 -248) 📝 `packages/sso/src/routes/helpers.ts` (+40 -16) ➕ `packages/sso/src/routes/saml-pipeline.ts` (+529 -0) 📝 `packages/sso/src/routes/sso.ts` (+85 -1125) 📝 `packages/sso/src/saml.test.ts` (+315 -0) ➕ `packages/sso/src/saml/timestamp.ts` (+78 -0) 📝 `packages/sso/src/types.ts` (+8 -0) </details> ### 📄 Description ## Summary - Extract duplicated SAML response handling from `callbackSSOSAML` and `acsEndpoint` into a shared `processSAMLResponse` pipeline - Complete `createSP`/`createIdP` helpers with all encryption and signing fields so both endpoints produce identical SP/IdP objects - Fix SP metadata endpoint using internal row ID instead of `providerId` in ACS URL fallback - Fix `acsEndpoint` skipping DB lookup when `defaultSSO` was configured, and falling back to `defaultSSO[0]` for unmatched providerIds - Fix `acsEndpoint` missing encryption fields (`isAssertionEncrypted`, `encPrivateKey`) causing silent decryption failures - Add registration-time validation: reject configs with no usable IdP entry point - Fix Supabase migration guide generating unsupported `metadataUrl` field; fetch IdP metadata XML inline instead - Fix contradictory `callbackUrl` guidance across SSO docs - Add TDD tests for provider lookup fallback, registration validation, and RelayState priority <!-- This is an auto-generated description by cubic. --> --- ## Summary by cubic Unifies SAML response handling in `@better-auth/sso`, restores browser-friendly ACS redirects with sanitized, query-safe URLs, and aligns docs to treat `callbackUrl` as the ACS URL. Keeps ACS error codes in backward‑compatible lowercase. - **Refactors** - Added shared `processSAMLResponse` for ACS and callback, removing duplicate logic. - Completed `createSP`/`createIdP` with full signing/encryption, `nameIDFormat`, SLO, RelayState; `createSP` now falls back to a generated ACS URL when `callbackUrl` is missing. - Extracted `validateSAMLTimestamp` to `saml/timestamp.ts` (re-exported) and made `extractAssertionId` internal. - **Bug Fixes** - Corrected ACS URL/provider lookup: SP metadata uses public `providerId`; ACS always queries the DB and never falls back to `defaultSSO[0]`; fixed `defaultSSO` parsing in the callback path. - Added missing encryption fields (`isAssertionEncrypted`, `encPrivateKey`) to prevent silent decryption failures. - Registration now requires IdP metadata XML, `singleSignOnService`, and a valid `entryPoint` (validated via `new URL()`). - Restored browser-friendly ACS error redirects, sanitized redirect targets, preserved existing query params, and kept error codes in lowercase snake_case (e.g., `error=multiple_assertions`) for compatibility. <sup>Written for commit 4546e4d09b11af11d42d9a112e16d2390060f076. 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-04-15 22:50:34 -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#25335
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?