mirror of
https://github.com/better-auth/better-auth.git
synced 2026-07-26 22:42:35 -05:00
fix(sso)!: harden SAML response validation (InResponseTo, Audience, SessionIndex) (#9055)
This commit is contained in:
@@ -0,0 +1,20 @@
|
||||
---
|
||||
"@better-auth/sso": minor
|
||||
---
|
||||
|
||||
fix(sso)!: harden SAML response validation (InResponseTo, Audience, SessionIndex)
|
||||
|
||||
### Breaking Changes
|
||||
|
||||
- **`allowIdpInitiated` now defaults to `false`** — IdP-initiated SSO (unsolicited SAML responses) is disabled by default. Set `saml.allowIdpInitiated: true` to restore the previous behavior. This aligns with the SAML2Int interoperability profile which recommends against IdP-initiated SSO due to its susceptibility to injection attacks.
|
||||
|
||||
### Bug Fixes
|
||||
|
||||
- **InResponseTo validation was completely non-functional** — The code read `extract.inResponseTo` (always `undefined`) instead of samlify's actual path `extract.response.inResponseTo`. SP-initiated InResponseTo validation now works as intended in both ACS handlers.
|
||||
- **Audience Restriction was never validated** — SAML assertions issued for a different service provider were accepted without checking the `<AudienceRestriction>` element. Audience is now validated against the configured `samlConfig.audience` value per SAML 2.0 Core §2.5.1.
|
||||
- **SessionIndex stored as object instead of string** — samlify returns `sessionIndex` from login responses as `{ authnInstant, sessionNotOnOrAfter, sessionIndex }`, but the code stored the whole object. SLO session-index comparisons always failed silently. The correct inner `sessionIndex` string is now extracted.
|
||||
|
||||
### Improvements
|
||||
|
||||
- Extracted shared `validateInResponseTo()` and `validateAudience()` into `packages/sso/src/saml/response-validation.ts`, eliminating ~160 lines of duplicated validation logic between the two ACS handlers.
|
||||
- Fixed `SAMLAssertionExtract` type to match samlify's actual extractor output shape.
|
||||
+50
-162
@@ -43,7 +43,9 @@ import {
|
||||
mapDiscoveryErrorToAPIError,
|
||||
} from "../oidc";
|
||||
import {
|
||||
validateAudience,
|
||||
validateConfigAlgorithms,
|
||||
validateInResponseTo,
|
||||
validateSAMLAlgorithms,
|
||||
validateSingleAssertion,
|
||||
} from "../saml";
|
||||
@@ -2181,88 +2183,28 @@ export const callbackSSOSAML = (options?: SSOOptions) => {
|
||||
logger: ctx.context.logger,
|
||||
});
|
||||
|
||||
const inResponseTo = (extract as SAMLAssertionExtract).inResponseTo as
|
||||
| string
|
||||
| undefined;
|
||||
const shouldValidateInResponseTo =
|
||||
options?.saml?.enableInResponseToValidation !== false;
|
||||
const samlRedirectUrl =
|
||||
relayState?.callbackURL ||
|
||||
parsedSamlConfig.callbackUrl ||
|
||||
ctx.context.baseURL;
|
||||
|
||||
if (shouldValidateInResponseTo) {
|
||||
const allowIdpInitiated = options?.saml?.allowIdpInitiated !== false;
|
||||
await validateInResponseTo(ctx, {
|
||||
extract: extract as SAMLAssertionExtract,
|
||||
providerId: provider.providerId,
|
||||
options: {
|
||||
enableInResponseToValidation:
|
||||
options?.saml?.enableInResponseToValidation,
|
||||
allowIdpInitiated: options?.saml?.allowIdpInitiated,
|
||||
},
|
||||
redirectUrl: samlRedirectUrl,
|
||||
});
|
||||
|
||||
if (inResponseTo) {
|
||||
let storedRequest: AuthnRequestRecord | null = null;
|
||||
|
||||
const verification =
|
||||
await ctx.context.internalAdapter.findVerificationValue(
|
||||
`${constants.AUTHN_REQUEST_KEY_PREFIX}${inResponseTo}`,
|
||||
);
|
||||
if (verification) {
|
||||
try {
|
||||
storedRequest = JSON.parse(
|
||||
verification.value,
|
||||
) as AuthnRequestRecord;
|
||||
if (storedRequest && storedRequest.expiresAt < Date.now()) {
|
||||
storedRequest = null;
|
||||
}
|
||||
} catch {
|
||||
storedRequest = null;
|
||||
}
|
||||
}
|
||||
|
||||
if (!storedRequest) {
|
||||
ctx.context.logger.error(
|
||||
"SAML InResponseTo validation failed: unknown or expired request ID",
|
||||
{ inResponseTo, providerId: provider.providerId },
|
||||
);
|
||||
const redirectUrl =
|
||||
relayState?.callbackURL ||
|
||||
parsedSamlConfig.callbackUrl ||
|
||||
ctx.context.baseURL;
|
||||
throw ctx.redirect(
|
||||
`${redirectUrl}?error=invalid_saml_response&error_description=Unknown+or+expired+request+ID`,
|
||||
);
|
||||
}
|
||||
|
||||
if (storedRequest.providerId !== provider.providerId) {
|
||||
ctx.context.logger.error(
|
||||
"SAML InResponseTo validation failed: provider mismatch",
|
||||
{
|
||||
inResponseTo,
|
||||
expectedProvider: storedRequest.providerId,
|
||||
actualProvider: provider.providerId,
|
||||
},
|
||||
);
|
||||
|
||||
await ctx.context.internalAdapter.deleteVerificationByIdentifier(
|
||||
`${constants.AUTHN_REQUEST_KEY_PREFIX}${inResponseTo}`,
|
||||
);
|
||||
const redirectUrl =
|
||||
relayState?.callbackURL ||
|
||||
parsedSamlConfig.callbackUrl ||
|
||||
ctx.context.baseURL;
|
||||
throw ctx.redirect(
|
||||
`${redirectUrl}?error=invalid_saml_response&error_description=Provider+mismatch`,
|
||||
);
|
||||
}
|
||||
|
||||
await ctx.context.internalAdapter.deleteVerificationByIdentifier(
|
||||
`${constants.AUTHN_REQUEST_KEY_PREFIX}${inResponseTo}`,
|
||||
);
|
||||
} else if (!allowIdpInitiated) {
|
||||
ctx.context.logger.error(
|
||||
"SAML IdP-initiated SSO rejected: InResponseTo missing and allowIdpInitiated is false",
|
||||
{ providerId: provider.providerId },
|
||||
);
|
||||
const redirectUrl =
|
||||
relayState?.callbackURL ||
|
||||
parsedSamlConfig.callbackUrl ||
|
||||
ctx.context.baseURL;
|
||||
throw ctx.redirect(
|
||||
`${redirectUrl}?error=unsolicited_response&error_description=IdP-initiated+SSO+not+allowed`,
|
||||
);
|
||||
}
|
||||
}
|
||||
validateAudience(ctx, {
|
||||
extract: extract as SAMLAssertionExtract,
|
||||
expectedAudience: parsedSamlConfig.audience,
|
||||
providerId: provider.providerId,
|
||||
redirectUrl: samlRedirectUrl,
|
||||
});
|
||||
|
||||
// Assertion Replay Protection
|
||||
const samlContent = (parsedResponse as any).samlContent as
|
||||
@@ -2450,7 +2392,8 @@ export const callbackSSOSAML = (options?: SSOOptions) => {
|
||||
sessionId: session.id,
|
||||
providerId: provider.providerId,
|
||||
nameID: extract.nameID,
|
||||
sessionIndex: (extract as SAMLAssertionExtract).sessionIndex,
|
||||
sessionIndex: (extract as SAMLAssertionExtract).sessionIndex
|
||||
?.sessionIndex,
|
||||
};
|
||||
await ctx.context.internalAdapter
|
||||
.createVerificationValue({
|
||||
@@ -2703,87 +2646,28 @@ export const acsEndpoint = (options?: SSOOptions) => {
|
||||
logger: ctx.context.logger,
|
||||
});
|
||||
|
||||
const inResponseToAcs = (extract as SAMLAssertionExtract).inResponseTo as
|
||||
| string
|
||||
| undefined;
|
||||
const shouldValidateInResponseToAcs =
|
||||
options?.saml?.enableInResponseToValidation !== false;
|
||||
const acsRedirectUrl =
|
||||
relayState?.callbackURL ||
|
||||
parsedSamlConfig.callbackUrl ||
|
||||
ctx.context.baseURL;
|
||||
|
||||
if (shouldValidateInResponseToAcs) {
|
||||
const allowIdpInitiated = options?.saml?.allowIdpInitiated !== false;
|
||||
await validateInResponseTo(ctx, {
|
||||
extract: extract as SAMLAssertionExtract,
|
||||
providerId,
|
||||
options: {
|
||||
enableInResponseToValidation:
|
||||
options?.saml?.enableInResponseToValidation,
|
||||
allowIdpInitiated: options?.saml?.allowIdpInitiated,
|
||||
},
|
||||
redirectUrl: acsRedirectUrl,
|
||||
});
|
||||
|
||||
if (inResponseToAcs) {
|
||||
let storedRequest: AuthnRequestRecord | null = null;
|
||||
|
||||
const verification =
|
||||
await ctx.context.internalAdapter.findVerificationValue(
|
||||
`${constants.AUTHN_REQUEST_KEY_PREFIX}${inResponseToAcs}`,
|
||||
);
|
||||
if (verification) {
|
||||
try {
|
||||
storedRequest = JSON.parse(
|
||||
verification.value,
|
||||
) as AuthnRequestRecord;
|
||||
if (storedRequest && storedRequest.expiresAt < Date.now()) {
|
||||
storedRequest = null;
|
||||
}
|
||||
} catch {
|
||||
storedRequest = null;
|
||||
}
|
||||
}
|
||||
|
||||
if (!storedRequest) {
|
||||
ctx.context.logger.error(
|
||||
"SAML InResponseTo validation failed: unknown or expired request ID",
|
||||
{ inResponseTo: inResponseToAcs, providerId },
|
||||
);
|
||||
const redirectUrl =
|
||||
relayState?.callbackURL ||
|
||||
parsedSamlConfig.callbackUrl ||
|
||||
ctx.context.baseURL;
|
||||
throw ctx.redirect(
|
||||
`${redirectUrl}?error=invalid_saml_response&error_description=Unknown+or+expired+request+ID`,
|
||||
);
|
||||
}
|
||||
|
||||
if (storedRequest.providerId !== providerId) {
|
||||
ctx.context.logger.error(
|
||||
"SAML InResponseTo validation failed: provider mismatch",
|
||||
{
|
||||
inResponseTo: inResponseToAcs,
|
||||
expectedProvider: storedRequest.providerId,
|
||||
actualProvider: providerId,
|
||||
},
|
||||
);
|
||||
await ctx.context.internalAdapter.deleteVerificationByIdentifier(
|
||||
`${constants.AUTHN_REQUEST_KEY_PREFIX}${inResponseToAcs}`,
|
||||
);
|
||||
const redirectUrl =
|
||||
relayState?.callbackURL ||
|
||||
parsedSamlConfig.callbackUrl ||
|
||||
ctx.context.baseURL;
|
||||
throw ctx.redirect(
|
||||
`${redirectUrl}?error=invalid_saml_response&error_description=Provider+mismatch`,
|
||||
);
|
||||
}
|
||||
|
||||
await ctx.context.internalAdapter.deleteVerificationByIdentifier(
|
||||
`${constants.AUTHN_REQUEST_KEY_PREFIX}${inResponseToAcs}`,
|
||||
);
|
||||
} else if (!allowIdpInitiated) {
|
||||
ctx.context.logger.error(
|
||||
"SAML IdP-initiated SSO rejected: InResponseTo missing and allowIdpInitiated is false",
|
||||
{ providerId },
|
||||
);
|
||||
const redirectUrl =
|
||||
relayState?.callbackURL ||
|
||||
parsedSamlConfig.callbackUrl ||
|
||||
ctx.context.baseURL;
|
||||
throw ctx.redirect(
|
||||
`${redirectUrl}?error=unsolicited_response&error_description=IdP-initiated+SSO+not+allowed`,
|
||||
);
|
||||
}
|
||||
}
|
||||
validateAudience(ctx, {
|
||||
extract: extract as SAMLAssertionExtract,
|
||||
expectedAudience: parsedSamlConfig.audience,
|
||||
providerId,
|
||||
redirectUrl: acsRedirectUrl,
|
||||
});
|
||||
|
||||
// Assertion Replay Protection
|
||||
const samlContentAcs = Buffer.from(SAMLResponse, "base64").toString(
|
||||
@@ -2971,7 +2855,8 @@ export const acsEndpoint = (options?: SSOOptions) => {
|
||||
sessionId: session.id,
|
||||
providerId,
|
||||
nameID: extract.nameID,
|
||||
sessionIndex: (extract as SAMLAssertionExtract).sessionIndex,
|
||||
sessionIndex: (extract as SAMLAssertionExtract).sessionIndex
|
||||
?.sessionIndex,
|
||||
};
|
||||
await ctx.context.internalAdapter
|
||||
.createVerificationValue({
|
||||
@@ -3186,7 +3071,10 @@ async function handleLogoutRequest(
|
||||
}
|
||||
|
||||
const { nameID } = parsed.extract;
|
||||
const sessionIndex = (parsed.extract as SAMLAssertionExtract).sessionIndex;
|
||||
// LogoutRequest's sessionIndex is a plain string (text node), unlike the
|
||||
// login response's multi-attribute object. Don't cast to SAMLAssertionExtract.
|
||||
const sessionIndex = (parsed.extract as { sessionIndex?: string })
|
||||
.sessionIndex;
|
||||
|
||||
const key = `${constants.SAML_SESSION_KEY_PREFIX}${providerId}:${nameID}`;
|
||||
const stored = await ctx.context.internalAdapter.findVerificationValue(key);
|
||||
|
||||
+190
-33
@@ -1011,6 +1011,12 @@ describe("SAML SSO", async () => {
|
||||
attributes: userInfo.attributes,
|
||||
};
|
||||
}),
|
||||
saml: {
|
||||
// Disable for the shared instance — InResponseTo validation has
|
||||
// dedicated tests. Enabling it here would require every test to
|
||||
// go through the SP-initiated flow to store an AuthnRequest first.
|
||||
enableInResponseToValidation: false,
|
||||
},
|
||||
};
|
||||
|
||||
const auth = betterAuth({
|
||||
@@ -1586,7 +1592,7 @@ describe("SAML SSO", async () => {
|
||||
|
||||
it("should initiate SAML login and fallback to callbackUrl on invalid RelayState", async () => {
|
||||
const { auth, signInWithTestUser } = await getTestInstance({
|
||||
plugins: [sso()],
|
||||
plugins: [sso({ saml: { enableInResponseToValidation: false } })],
|
||||
});
|
||||
|
||||
const { headers } = await signInWithTestUser();
|
||||
@@ -1732,7 +1738,12 @@ describe("SAML SSO", async () => {
|
||||
it("should reject SAML sign-in when disableImplicitSignUp is true and user doesn't exist", async () => {
|
||||
const { auth: authWithDisabledSignUp, signInWithTestUser } =
|
||||
await getTestInstance({
|
||||
plugins: [sso({ disableImplicitSignUp: true })],
|
||||
plugins: [
|
||||
sso({
|
||||
disableImplicitSignUp: true,
|
||||
saml: { enableInResponseToValidation: false },
|
||||
}),
|
||||
],
|
||||
});
|
||||
|
||||
const { headers } = await signInWithTestUser();
|
||||
@@ -1796,7 +1807,12 @@ describe("SAML SSO", async () => {
|
||||
it("should reject SAML ACS (IdP-initiated) when disableImplicitSignUp is true and user doesn't exist", async () => {
|
||||
const { auth: authWithDisabledSignUp, signInWithTestUser } =
|
||||
await getTestInstance({
|
||||
plugins: [sso({ disableImplicitSignUp: true })],
|
||||
plugins: [
|
||||
sso({
|
||||
disableImplicitSignUp: true,
|
||||
saml: { enableInResponseToValidation: false },
|
||||
}),
|
||||
],
|
||||
});
|
||||
|
||||
const { headers } = await signInWithTestUser();
|
||||
@@ -1862,7 +1878,7 @@ describe("SAML SSO", async () => {
|
||||
trustedProviders: [],
|
||||
},
|
||||
},
|
||||
plugins: [sso()],
|
||||
plugins: [sso({ saml: { enableInResponseToValidation: false } })],
|
||||
});
|
||||
|
||||
const { headers } = await signInWithTestUser();
|
||||
@@ -1941,7 +1957,7 @@ describe("SAML SSO", async () => {
|
||||
trustedProviders: ["trusted-saml-provider"],
|
||||
},
|
||||
},
|
||||
plugins: [sso()],
|
||||
plugins: [sso({ saml: { enableInResponseToValidation: false } })],
|
||||
},
|
||||
);
|
||||
|
||||
@@ -2076,10 +2092,10 @@ describe("SAML SSO", async () => {
|
||||
|
||||
expect(response.status).toBe(302);
|
||||
const redirectLocation = response.headers.get("location") || "";
|
||||
expect(redirectLocation).toContain("error=unsolicited_response");
|
||||
expect(redirectLocation).toContain("error=invalid_saml_response");
|
||||
});
|
||||
|
||||
it("should allow unsolicited SAML response when allowIdpInitiated is true (default)", async () => {
|
||||
it("should allow unsolicited SAML response when allowIdpInitiated is explicitly true", async () => {
|
||||
const { auth, signInWithTestUser } = await getTestInstance({
|
||||
plugins: [
|
||||
sso({
|
||||
@@ -2277,7 +2293,10 @@ describe("SAML SSO", async () => {
|
||||
|
||||
expect(response.status).toBe(302);
|
||||
const redirectLocation = response.headers.get("location") || "";
|
||||
expect(redirectLocation).toContain("error=unsolicited_response");
|
||||
// The mock IdP response includes InResponseTo, but no matching
|
||||
// AuthnRequest was stored — so it's rejected as unknown request ID,
|
||||
// not as unsolicited (which would require InResponseTo to be absent).
|
||||
expect(redirectLocation).toContain("error=invalid_saml_response");
|
||||
});
|
||||
|
||||
it("should use verification table for InResponseTo validation", async () => {
|
||||
@@ -2343,10 +2362,148 @@ describe("SAML SSO", async () => {
|
||||
),
|
||||
);
|
||||
|
||||
// Should reject unsolicited response, proving validation is active
|
||||
// The mock response has InResponseTo but no matching stored request
|
||||
expect(response.status).toBe(302);
|
||||
const redirectLocation = response.headers.get("location") || "";
|
||||
expect(redirectLocation).toContain("error=unsolicited_response");
|
||||
expect(redirectLocation).toContain("error=invalid_saml_response");
|
||||
});
|
||||
|
||||
/**
|
||||
* @see https://github.com/better-auth/better-auth/issues/8607
|
||||
*/
|
||||
it("should reject SAML response with mismatched audience restriction", async () => {
|
||||
const { auth, signInWithTestUser } = await getTestInstance({
|
||||
plugins: [
|
||||
sso({
|
||||
saml: {
|
||||
enableInResponseToValidation: false,
|
||||
},
|
||||
}),
|
||||
],
|
||||
});
|
||||
|
||||
const { headers } = await signInWithTestUser();
|
||||
|
||||
await auth.api.registerSSOProvider({
|
||||
body: {
|
||||
providerId: "audience-mismatch-provider",
|
||||
issuer: "http://localhost:8081",
|
||||
domain: "http://localhost:8081",
|
||||
samlConfig: {
|
||||
entryPoint: "http://localhost:8081/api/sso/saml2/idp/post",
|
||||
cert: certificate,
|
||||
callbackUrl: "http://localhost:3000/dashboard",
|
||||
audience: "https://wrong-audience.example.com",
|
||||
wantAssertionsSigned: false,
|
||||
signatureAlgorithm: "sha256",
|
||||
digestAlgorithm: "sha256",
|
||||
idpMetadata: {
|
||||
metadata: idpMetadata,
|
||||
},
|
||||
spMetadata: {
|
||||
metadata: spMetadata,
|
||||
binding: "post",
|
||||
},
|
||||
},
|
||||
},
|
||||
headers,
|
||||
});
|
||||
|
||||
let samlResponse: any;
|
||||
await betterFetch("http://localhost:8081/api/sso/saml2/idp/post", {
|
||||
onSuccess: async (context) => {
|
||||
samlResponse = await context.data;
|
||||
},
|
||||
});
|
||||
|
||||
const response = await auth.handler(
|
||||
new Request(
|
||||
"http://localhost:3000/api/auth/sso/saml2/callback/audience-mismatch-provider",
|
||||
{
|
||||
method: "POST",
|
||||
headers: {
|
||||
"Content-Type": "application/x-www-form-urlencoded",
|
||||
},
|
||||
body: new URLSearchParams({
|
||||
SAMLResponse: samlResponse.samlResponse,
|
||||
}),
|
||||
},
|
||||
),
|
||||
);
|
||||
|
||||
expect(response.status).toBe(302);
|
||||
const redirectLocation = response.headers.get("location") || "";
|
||||
expect(redirectLocation).toContain(
|
||||
"error=invalid_saml_response&error_description=Audience+mismatch",
|
||||
);
|
||||
});
|
||||
|
||||
it("should accept SAML response when audience matches configured value", async () => {
|
||||
const { auth, signInWithTestUser } = await getTestInstance({
|
||||
plugins: [
|
||||
sso({
|
||||
saml: {
|
||||
enableInResponseToValidation: false,
|
||||
},
|
||||
}),
|
||||
],
|
||||
});
|
||||
|
||||
const { headers } = await signInWithTestUser();
|
||||
|
||||
// The mock IdP's SP entity ID is the audience it sends in assertions
|
||||
// Configure the provider's audience to match it
|
||||
await auth.api.registerSSOProvider({
|
||||
body: {
|
||||
providerId: "audience-match-provider",
|
||||
issuer: "http://localhost:8081",
|
||||
domain: "http://localhost:8081",
|
||||
samlConfig: {
|
||||
entryPoint: "http://localhost:8081/api/sso/saml2/idp/post",
|
||||
cert: certificate,
|
||||
callbackUrl: "http://localhost:3000/dashboard",
|
||||
audience: "http://localhost:3001/api/sso/saml2/sp/metadata",
|
||||
wantAssertionsSigned: false,
|
||||
signatureAlgorithm: "sha256",
|
||||
digestAlgorithm: "sha256",
|
||||
idpMetadata: {
|
||||
metadata: idpMetadata,
|
||||
},
|
||||
spMetadata: {
|
||||
metadata: spMetadata,
|
||||
binding: "post",
|
||||
},
|
||||
},
|
||||
},
|
||||
headers,
|
||||
});
|
||||
|
||||
let samlResponse: any;
|
||||
await betterFetch("http://localhost:8081/api/sso/saml2/idp/post", {
|
||||
onSuccess: async (context) => {
|
||||
samlResponse = await context.data;
|
||||
},
|
||||
});
|
||||
|
||||
const response = await auth.handler(
|
||||
new Request(
|
||||
"http://localhost:3000/api/auth/sso/saml2/callback/audience-match-provider",
|
||||
{
|
||||
method: "POST",
|
||||
headers: {
|
||||
"Content-Type": "application/x-www-form-urlencoded",
|
||||
},
|
||||
body: new URLSearchParams({
|
||||
SAMLResponse: samlResponse.samlResponse,
|
||||
}),
|
||||
},
|
||||
),
|
||||
);
|
||||
|
||||
// Should not redirect to an error — the audience matches
|
||||
expect(response.status).toBe(302);
|
||||
const redirectLocation = response.headers.get("location") || "";
|
||||
expect(redirectLocation).not.toContain("error=");
|
||||
});
|
||||
|
||||
/**
|
||||
@@ -2354,7 +2511,7 @@ describe("SAML SSO", async () => {
|
||||
*/
|
||||
it("should correctly parse verification-ID-based RelayState on ACS route (SP-initiated)", async () => {
|
||||
const { auth, signInWithTestUser } = await getTestInstance({
|
||||
plugins: [sso()],
|
||||
plugins: [sso({ saml: { enableInResponseToValidation: false } })],
|
||||
});
|
||||
|
||||
const { headers } = await signInWithTestUser();
|
||||
@@ -2439,7 +2596,7 @@ describe("SAML SSO", async () => {
|
||||
|
||||
it("should redirect to signIn callbackURL when relay_state cookie is missing on callback route (cross-site POST)", async () => {
|
||||
const { auth, signInWithTestUser } = await getTestInstance({
|
||||
plugins: [sso()],
|
||||
plugins: [sso({ saml: { enableInResponseToValidation: false } })],
|
||||
});
|
||||
|
||||
const { headers } = await signInWithTestUser();
|
||||
@@ -2454,7 +2611,6 @@ describe("SAML SSO", async () => {
|
||||
cert: certificate,
|
||||
callbackUrl:
|
||||
"http://localhost:3000/api/auth/sso/saml2/callback/saml-provider",
|
||||
audience: "http://localhost:3000",
|
||||
wantAssertionsSigned: false,
|
||||
signatureAlgorithm: "sha256",
|
||||
digestAlgorithm: "sha256",
|
||||
@@ -2518,7 +2674,7 @@ describe("SAML SSO", async () => {
|
||||
|
||||
it("should redirect to signIn callbackURL when relay_state cookie is missing on ACS route (cross-site POST)", async () => {
|
||||
const { auth, signInWithTestUser } = await getTestInstance({
|
||||
plugins: [sso()],
|
||||
plugins: [sso({ saml: { enableInResponseToValidation: false } })],
|
||||
});
|
||||
|
||||
const { headers } = await signInWithTestUser();
|
||||
@@ -2533,7 +2689,6 @@ describe("SAML SSO", async () => {
|
||||
cert: certificate,
|
||||
callbackUrl:
|
||||
"http://localhost:3000/api/auth/sso/saml2/sp/acs/saml-provider",
|
||||
audience: "http://localhost:3000",
|
||||
wantAssertionsSigned: false,
|
||||
signatureAlgorithm: "sha256",
|
||||
digestAlgorithm: "sha256",
|
||||
@@ -2989,7 +3144,7 @@ describe("SSO Provider Config Parsing", () => {
|
||||
describe("SAML SSO - IdP Initiated Flow", () => {
|
||||
it("should handle IdP-initiated flow with GET after POST redirect", async () => {
|
||||
const { auth, signInWithTestUser } = await getTestInstance({
|
||||
plugins: [sso()],
|
||||
plugins: [sso({ saml: { enableInResponseToValidation: false } })],
|
||||
});
|
||||
|
||||
const { headers } = await signInWithTestUser();
|
||||
@@ -3106,7 +3261,7 @@ describe("SAML SSO - IdP Initiated Flow", () => {
|
||||
|
||||
it("should prevent redirect loop when callbackUrl points to callback route", async () => {
|
||||
const { auth, signInWithTestUser } = await getTestInstance({
|
||||
plugins: [sso()],
|
||||
plugins: [sso({ saml: { enableInResponseToValidation: false } })],
|
||||
});
|
||||
|
||||
const { headers } = await signInWithTestUser();
|
||||
@@ -3178,7 +3333,7 @@ describe("SAML SSO - IdP Initiated Flow", () => {
|
||||
|
||||
it("should handle GET request with RelayState in query", async () => {
|
||||
const { auth, signInWithTestUser } = await getTestInstance({
|
||||
plugins: [sso()],
|
||||
plugins: [sso({ saml: { enableInResponseToValidation: false } })],
|
||||
});
|
||||
|
||||
const { headers } = await signInWithTestUser();
|
||||
@@ -3260,7 +3415,7 @@ describe("SAML SSO - IdP Initiated Flow", () => {
|
||||
|
||||
it("should handle GET request when POST redirects to callback URL (original issue scenario)", async () => {
|
||||
const { auth, signInWithTestUser } = await getTestInstance({
|
||||
plugins: [sso()],
|
||||
plugins: [sso({ saml: { enableInResponseToValidation: false } })],
|
||||
});
|
||||
|
||||
const { headers } = await signInWithTestUser();
|
||||
@@ -3348,7 +3503,7 @@ describe("SAML SSO - IdP Initiated Flow", () => {
|
||||
|
||||
it("should prevent open redirect with malicious RelayState URL", async () => {
|
||||
const { auth, signInWithTestUser } = await getTestInstance({
|
||||
plugins: [sso()],
|
||||
plugins: [sso({ saml: { enableInResponseToValidation: false } })],
|
||||
});
|
||||
|
||||
const { headers } = await signInWithTestUser();
|
||||
@@ -3422,7 +3577,7 @@ describe("SAML SSO - IdP Initiated Flow", () => {
|
||||
|
||||
it("should prevent open redirect via GET with malicious RelayState", async () => {
|
||||
const { auth, signInWithTestUser } = await getTestInstance({
|
||||
plugins: [sso()],
|
||||
plugins: [sso({ saml: { enableInResponseToValidation: false } })],
|
||||
});
|
||||
|
||||
const { headers } = await signInWithTestUser();
|
||||
@@ -3508,7 +3663,7 @@ describe("SAML SSO - IdP Initiated Flow", () => {
|
||||
|
||||
it("should allow relative path redirects", async () => {
|
||||
const { auth, signInWithTestUser } = await getTestInstance({
|
||||
plugins: [sso()],
|
||||
plugins: [sso({ saml: { enableInResponseToValidation: false } })],
|
||||
});
|
||||
|
||||
const { headers } = await signInWithTestUser();
|
||||
@@ -3577,7 +3732,7 @@ describe("SAML SSO - IdP Initiated Flow", () => {
|
||||
|
||||
it("should block protocol-relative URL attacks (//evil.com)", async () => {
|
||||
const { auth, signInWithTestUser } = await getTestInstance({
|
||||
plugins: [sso()],
|
||||
plugins: [sso({ saml: { enableInResponseToValidation: false } })],
|
||||
});
|
||||
|
||||
const { headers } = await signInWithTestUser();
|
||||
@@ -3883,7 +4038,7 @@ describe("SAML ACS Origin Check Bypass", () => {
|
||||
describe("Positive: SAML endpoints allow external IdP origins", () => {
|
||||
it("should allow SAML callback POST from external IdP origin", async () => {
|
||||
const { auth, signInWithTestUser } = await getTestInstance({
|
||||
plugins: [sso()],
|
||||
plugins: [sso({ saml: { enableInResponseToValidation: false } })],
|
||||
});
|
||||
const { headers } = await signInWithTestUser();
|
||||
|
||||
@@ -3937,7 +4092,7 @@ describe("SAML ACS Origin Check Bypass", () => {
|
||||
|
||||
it("should allow ACS endpoint POST from external IdP origin", async () => {
|
||||
const { auth, signInWithTestUser } = await getTestInstance({
|
||||
plugins: [sso()],
|
||||
plugins: [sso({ saml: { enableInResponseToValidation: false } })],
|
||||
});
|
||||
const { headers } = await signInWithTestUser();
|
||||
|
||||
@@ -3990,7 +4145,7 @@ describe("SAML ACS Origin Check Bypass", () => {
|
||||
describe("Negative: Non-SAML endpoints remain protected", () => {
|
||||
it("should block POST to sign-up with untrusted origin when origin check is enabled", async () => {
|
||||
const { auth } = await getTestInstance({
|
||||
plugins: [sso()],
|
||||
plugins: [sso({ saml: { enableInResponseToValidation: false } })],
|
||||
advanced: {
|
||||
disableCSRFCheck: false,
|
||||
disableOriginCheck: false,
|
||||
@@ -4214,7 +4369,7 @@ describe("SAML SSO - Size Limit Validation", () => {
|
||||
describe("SAML SSO - Assertion Replay Protection", () => {
|
||||
it("should reject replayed SAML assertion (same assertion submitted twice)", async () => {
|
||||
const { auth, signInWithTestUser } = await getTestInstance({
|
||||
plugins: [sso()],
|
||||
plugins: [sso({ saml: { enableInResponseToValidation: false } })],
|
||||
});
|
||||
|
||||
const { headers } = await signInWithTestUser();
|
||||
@@ -4294,7 +4449,7 @@ describe("SAML SSO - Assertion Replay Protection", () => {
|
||||
|
||||
it("should reject replayed SAML assertion on ACS endpoint", async () => {
|
||||
const { auth, signInWithTestUser } = await getTestInstance({
|
||||
plugins: [sso()],
|
||||
plugins: [sso({ saml: { enableInResponseToValidation: false } })],
|
||||
});
|
||||
|
||||
const { headers } = await signInWithTestUser();
|
||||
@@ -4374,7 +4529,7 @@ describe("SAML SSO - Assertion Replay Protection", () => {
|
||||
|
||||
it("should reject cross-endpoint replay (callback → ACS)", async () => {
|
||||
const { auth, signInWithTestUser } = await getTestInstance({
|
||||
plugins: [sso()],
|
||||
plugins: [sso({ saml: { enableInResponseToValidation: false } })],
|
||||
});
|
||||
|
||||
const { headers } = await signInWithTestUser();
|
||||
@@ -4455,7 +4610,7 @@ describe("SAML SSO - Assertion Replay Protection", () => {
|
||||
describe("SAML SSO - Single Assertion Validation", () => {
|
||||
it("should reject SAML response with multiple assertions on callback endpoint", async () => {
|
||||
const { auth, signInWithTestUser } = await getTestInstance({
|
||||
plugins: [sso()],
|
||||
plugins: [sso({ saml: { enableInResponseToValidation: false } })],
|
||||
});
|
||||
|
||||
const { headers } = await signInWithTestUser();
|
||||
@@ -4529,7 +4684,7 @@ describe("SAML SSO - Single Assertion Validation", () => {
|
||||
|
||||
it("should reject SAML response with multiple assertions on ACS endpoint", async () => {
|
||||
const { auth, signInWithTestUser } = await getTestInstance({
|
||||
plugins: [sso()],
|
||||
plugins: [sso({ saml: { enableInResponseToValidation: false } })],
|
||||
});
|
||||
|
||||
const { headers } = await signInWithTestUser();
|
||||
@@ -4742,7 +4897,7 @@ describe("SAML SSO - Single Assertion Validation", () => {
|
||||
|
||||
it("should accept valid SAML response with exactly one assertion", async () => {
|
||||
const { auth, signInWithTestUser } = await getTestInstance({
|
||||
plugins: [sso()],
|
||||
plugins: [sso({ saml: { enableInResponseToValidation: false } })],
|
||||
});
|
||||
|
||||
const { headers } = await signInWithTestUser();
|
||||
@@ -4801,7 +4956,7 @@ describe("SAML SSO - Single Assertion Validation", () => {
|
||||
|
||||
it("should normalize email to lowercase in SAML authentication to prevent duplicate creation", async () => {
|
||||
const { auth, client, signInWithTestUser, db } = await getTestInstance({
|
||||
plugins: [sso()],
|
||||
plugins: [sso({ saml: { enableInResponseToValidation: false } })],
|
||||
});
|
||||
|
||||
const { headers } = await signInWithTestUser();
|
||||
@@ -5544,6 +5699,7 @@ describe("SAML provisionUser should only be called for new users", async () => {
|
||||
plugins: [
|
||||
sso({
|
||||
provisionUser: provisionUserFn,
|
||||
saml: { enableInResponseToValidation: false },
|
||||
}),
|
||||
],
|
||||
});
|
||||
@@ -5659,6 +5815,7 @@ describe("SAML provisionUserOnEveryLogin should call provisionUser on every sign
|
||||
sso({
|
||||
provisionUser: provisionUserFn,
|
||||
provisionUserOnEveryLogin: true,
|
||||
saml: { enableInResponseToValidation: false },
|
||||
}),
|
||||
],
|
||||
});
|
||||
|
||||
@@ -11,3 +11,5 @@ export {
|
||||
} from "./algorithms";
|
||||
|
||||
export { validateSingleAssertion } from "./assertions";
|
||||
|
||||
export { validateAudience, validateInResponseTo } from "./response-validation";
|
||||
|
||||
@@ -0,0 +1,197 @@
|
||||
import type { GenericEndpointContext } from "@better-auth/core";
|
||||
import { AUTHN_REQUEST_KEY_PREFIX } from "../constants";
|
||||
import type { SAMLAssertionExtract } from "../types";
|
||||
|
||||
function errorRedirectUrl(
|
||||
base: string,
|
||||
error: string,
|
||||
description: string,
|
||||
): string {
|
||||
try {
|
||||
const url = new URL(base);
|
||||
url.searchParams.set("error", error);
|
||||
url.searchParams.set("error_description", description);
|
||||
return url.toString();
|
||||
} catch {
|
||||
// Relative URL — fall back to manual construction.
|
||||
// Split off any fragment so query params stay before the hash.
|
||||
const hashIdx = base.indexOf("#");
|
||||
const path = hashIdx >= 0 ? base.slice(0, hashIdx) : base;
|
||||
const hash = hashIdx >= 0 ? base.slice(hashIdx + 1) : undefined;
|
||||
const separator = path.includes("?") ? "&" : "?";
|
||||
const query = `error=${encodeURIComponent(error)}&error_description=${encodeURIComponent(description)}`;
|
||||
return `${path}${separator}${query}${hash ? `#${hash}` : ""}`;
|
||||
}
|
||||
}
|
||||
|
||||
interface AuthnRequestRecord {
|
||||
id: string;
|
||||
providerId: string;
|
||||
createdAt: number;
|
||||
expiresAt: number;
|
||||
}
|
||||
|
||||
export interface InResponseToValidationContext {
|
||||
extract: SAMLAssertionExtract;
|
||||
providerId: string;
|
||||
options: {
|
||||
enableInResponseToValidation?: boolean;
|
||||
allowIdpInitiated?: boolean;
|
||||
};
|
||||
redirectUrl: string;
|
||||
}
|
||||
|
||||
/**
|
||||
* Validates the InResponseTo attribute of a SAML Response.
|
||||
*
|
||||
* This binds the IdP's Response to a specific SP-initiated AuthnRequest,
|
||||
* preventing replay attacks, unsolicited response injection, and
|
||||
* cross-provider assertion swaps.
|
||||
*
|
||||
* The InResponseTo value lives at `extract.response.inResponseTo` in
|
||||
* samlify's parsed output (not at the top level).
|
||||
*/
|
||||
export async function validateInResponseTo(
|
||||
c: GenericEndpointContext,
|
||||
ctx: InResponseToValidationContext,
|
||||
): Promise<void> {
|
||||
if (ctx.options.enableInResponseToValidation === false) {
|
||||
return;
|
||||
}
|
||||
|
||||
const inResponseTo = ctx.extract.response?.inResponseTo;
|
||||
const allowIdpInitiated = ctx.options.allowIdpInitiated ?? false;
|
||||
|
||||
if (inResponseTo) {
|
||||
let storedRequest: AuthnRequestRecord | null = null;
|
||||
|
||||
const verification = await c.context.internalAdapter.findVerificationValue(
|
||||
`${AUTHN_REQUEST_KEY_PREFIX}${inResponseTo}`,
|
||||
);
|
||||
|
||||
if (verification) {
|
||||
try {
|
||||
storedRequest = JSON.parse(verification.value) as AuthnRequestRecord;
|
||||
if (storedRequest && storedRequest.expiresAt < Date.now()) {
|
||||
storedRequest = null;
|
||||
}
|
||||
} catch {
|
||||
storedRequest = null;
|
||||
}
|
||||
}
|
||||
|
||||
if (!storedRequest) {
|
||||
c.context.logger.error(
|
||||
"SAML InResponseTo validation failed: unknown or expired request ID",
|
||||
{ inResponseTo, providerId: ctx.providerId },
|
||||
);
|
||||
throw c.redirect(
|
||||
errorRedirectUrl(
|
||||
ctx.redirectUrl,
|
||||
"invalid_saml_response",
|
||||
"Unknown or expired request ID",
|
||||
),
|
||||
);
|
||||
}
|
||||
|
||||
if (storedRequest.providerId !== ctx.providerId) {
|
||||
c.context.logger.error(
|
||||
"SAML InResponseTo validation failed: provider mismatch",
|
||||
{
|
||||
inResponseTo,
|
||||
expectedProvider: storedRequest.providerId,
|
||||
actualProvider: ctx.providerId,
|
||||
},
|
||||
);
|
||||
await c.context.internalAdapter.deleteVerificationByIdentifier(
|
||||
`${AUTHN_REQUEST_KEY_PREFIX}${inResponseTo}`,
|
||||
);
|
||||
throw c.redirect(
|
||||
errorRedirectUrl(
|
||||
ctx.redirectUrl,
|
||||
"invalid_saml_response",
|
||||
"Provider mismatch",
|
||||
),
|
||||
);
|
||||
}
|
||||
|
||||
// Single-use: delete the stored request after successful validation
|
||||
await c.context.internalAdapter.deleteVerificationByIdentifier(
|
||||
`${AUTHN_REQUEST_KEY_PREFIX}${inResponseTo}`,
|
||||
);
|
||||
} else if (!allowIdpInitiated) {
|
||||
c.context.logger.error(
|
||||
"SAML IdP-initiated SSO rejected: InResponseTo missing and allowIdpInitiated is false",
|
||||
{ providerId: ctx.providerId },
|
||||
);
|
||||
throw c.redirect(
|
||||
errorRedirectUrl(
|
||||
ctx.redirectUrl,
|
||||
"unsolicited_response",
|
||||
"IdP-initiated SSO not allowed",
|
||||
),
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
export interface AudienceValidationContext {
|
||||
extract: SAMLAssertionExtract;
|
||||
expectedAudience: string | undefined;
|
||||
providerId: string;
|
||||
redirectUrl: string;
|
||||
}
|
||||
|
||||
/**
|
||||
* Validates the AudienceRestriction of a SAML assertion.
|
||||
*
|
||||
* Per SAML 2.0 Core §2.5.1, an assertion's Audience element specifies
|
||||
* the intended recipient SP. Without this check, an assertion issued
|
||||
* for a different SP (e.g., another application sharing the same IdP)
|
||||
* could be accepted.
|
||||
*/
|
||||
export function validateAudience(
|
||||
c: GenericEndpointContext,
|
||||
ctx: AudienceValidationContext,
|
||||
): void {
|
||||
if (!ctx.expectedAudience) {
|
||||
return;
|
||||
}
|
||||
|
||||
const audience = ctx.extract.audience;
|
||||
|
||||
if (!audience) {
|
||||
c.context.logger.error(
|
||||
"SAML assertion missing AudienceRestriction but audience is configured — rejecting",
|
||||
{ providerId: ctx.providerId },
|
||||
);
|
||||
throw c.redirect(
|
||||
errorRedirectUrl(
|
||||
ctx.redirectUrl,
|
||||
"invalid_saml_response",
|
||||
"Audience restriction missing",
|
||||
),
|
||||
);
|
||||
}
|
||||
|
||||
// samlify returns a string for a single Audience element, or an array
|
||||
// when multiple <Audience> values are present in the restriction.
|
||||
const audiences = Array.isArray(audience) ? audience : [audience];
|
||||
|
||||
if (!audiences.includes(ctx.expectedAudience)) {
|
||||
c.context.logger.error(
|
||||
"SAML audience mismatch: assertion was issued for a different service provider",
|
||||
{
|
||||
expected: ctx.expectedAudience,
|
||||
received: audiences,
|
||||
providerId: ctx.providerId,
|
||||
},
|
||||
);
|
||||
throw c.redirect(
|
||||
errorRedirectUrl(
|
||||
ctx.redirectUrl,
|
||||
"invalid_saml_response",
|
||||
"Audience mismatch",
|
||||
),
|
||||
);
|
||||
}
|
||||
}
|
||||
@@ -95,15 +95,39 @@ export interface SAMLSessionRecord {
|
||||
sessionIndex?: string;
|
||||
}
|
||||
|
||||
/** Parsed SAML assertion extract from samlify */
|
||||
/**
|
||||
* Parsed SAML login response extract from samlify.
|
||||
*
|
||||
* samlify's extractor nests multi-attribute XML elements as objects:
|
||||
* - `response` (Response/@ID, @IssueInstant, @Destination, @InResponseTo)
|
||||
* - `sessionIndex` (AuthnStatement/@AuthnInstant, @SessionNotOnOrAfter, @SessionIndex)
|
||||
* - `conditions` (Conditions/@NotBefore, @NotOnOrAfter)
|
||||
*
|
||||
* Single-value elements remain as strings: `nameID`, `audience`.
|
||||
*/
|
||||
export interface SAMLAssertionExtract {
|
||||
nameID?: string;
|
||||
sessionIndex?: string;
|
||||
inResponseTo?: string;
|
||||
/**
|
||||
* From `<AuthnStatement>` — samlify extracts all 3 attributes as an object.
|
||||
* To get the SessionIndex string, read `sessionIndex.sessionIndex`.
|
||||
*/
|
||||
sessionIndex?: {
|
||||
authnInstant?: string;
|
||||
sessionNotOnOrAfter?: string;
|
||||
sessionIndex?: string;
|
||||
};
|
||||
conditions?: {
|
||||
notBefore?: string;
|
||||
notOnOrAfter?: string;
|
||||
};
|
||||
response?: {
|
||||
id?: string;
|
||||
issueInstant?: string;
|
||||
destination?: string;
|
||||
inResponseTo?: string;
|
||||
};
|
||||
/** Single string or array when multiple `<Audience>` elements are present. */
|
||||
audience?: string | string[];
|
||||
}
|
||||
|
||||
type BaseSSOProvider = {
|
||||
@@ -323,9 +347,13 @@ export interface SSOOptions {
|
||||
* When true, responses without InResponseTo are accepted.
|
||||
* When false, all responses must correlate to a stored AuthnRequest.
|
||||
*
|
||||
* IdP-initiated SSO is a known attack vector — the SAML2Int
|
||||
* interoperability profile recommends against it. Only enable
|
||||
* this if your IdP requires it and you understand the risks.
|
||||
*
|
||||
* Only applies when InResponseTo validation is enabled.
|
||||
*
|
||||
* @default true
|
||||
* @default false
|
||||
*/
|
||||
allowIdpInitiated?: boolean;
|
||||
/**
|
||||
|
||||
Reference in New Issue
Block a user