mirror of
https://github.com/better-auth/better-auth.git
synced 2026-10-24 00:15:17 -05:00
fix(sso): update samlify to 2.13.1 for signed-assertion XML injection (#9821)
This commit is contained in:
@@ -0,0 +1,9 @@
|
||||
---
|
||||
"@better-auth/sso": patch
|
||||
---
|
||||
|
||||
Fix a high-severity XML injection in signed SAML assertions (GHSA-34r5-q4jw-r36m) by updating `samlify` from 2.10.2 to 2.13.1. A crafted `AttributeValue` could escalate privileges.
|
||||
|
||||
samlify 2.11 replaced `node-forge` with Node's native crypto, which parses private keys through OpenSSL 3 and rejects PEM blocks that carry leading whitespace. SAML private keys are now normalized before they reach samlify, so a key pasted with indentation (for example from an indented YAML or JSON config) keeps loading.
|
||||
|
||||
IdP-initiated Single Logout now derives its response from the parsed logout request, which fixes response generation under samlify 2.13. When mapping SAML attributes to user fields, a multi-valued attribute is read by its first value.
|
||||
@@ -67,7 +67,7 @@
|
||||
"dependencies": {
|
||||
"fast-xml-parser": "^5.8.0",
|
||||
"jose": "^6.1.3",
|
||||
"samlify": "~2.10.2",
|
||||
"samlify": "^2.13.1",
|
||||
"tldts": "^6.1.0",
|
||||
"zod": "^4.3.6"
|
||||
},
|
||||
|
||||
@@ -1,7 +1,7 @@
|
||||
import type { DBAdapter } from "@better-auth/core/db/adapter";
|
||||
import { saml } from "../samlify";
|
||||
import type { SAMLConfig, SSOOptions, SSOProvider } from "../types";
|
||||
import { safeJsonParse } from "../utils";
|
||||
import { normalizePem, safeJsonParse } from "../utils";
|
||||
|
||||
export async function findSAMLProvider(
|
||||
providerId: string,
|
||||
@@ -83,10 +83,10 @@ export function createSP(
|
||||
wantLogoutResponseSigned:
|
||||
opts?.sloOptions?.wantLogoutResponseSigned ?? false,
|
||||
metadata: spData?.metadata,
|
||||
privateKey: spData?.privateKey || config.privateKey,
|
||||
privateKey: normalizePem(spData?.privateKey || config.privateKey),
|
||||
privateKeyPass: spData?.privateKeyPass,
|
||||
isAssertionEncrypted: spData?.isAssertionEncrypted || false,
|
||||
encPrivateKey: spData?.encPrivateKey,
|
||||
encPrivateKey: normalizePem(spData?.encPrivateKey),
|
||||
encPrivateKeyPass: spData?.encPrivateKeyPass,
|
||||
nameIDFormat: config.identifierFormat
|
||||
? [config.identifierFormat]
|
||||
@@ -100,10 +100,10 @@ export function createIdP(config: SAMLConfig) {
|
||||
if (idpData?.metadata) {
|
||||
return saml.IdentityProvider({
|
||||
metadata: idpData.metadata,
|
||||
privateKey: idpData.privateKey,
|
||||
privateKey: normalizePem(idpData.privateKey),
|
||||
privateKeyPass: idpData.privateKeyPass,
|
||||
isAssertionEncrypted: idpData.isAssertionEncrypted,
|
||||
encPrivateKey: idpData.encPrivateKey,
|
||||
encPrivateKey: normalizePem(idpData.encPrivateKey),
|
||||
encPrivateKeyPass: idpData.encPrivateKeyPass,
|
||||
});
|
||||
}
|
||||
@@ -119,7 +119,7 @@ export function createIdP(config: SAMLConfig) {
|
||||
signingCert: idpData?.cert || config.cert,
|
||||
wantAuthnRequestsSigned: config.authnRequestsSigned || false,
|
||||
isAssertionEncrypted: idpData?.isAssertionEncrypted || false,
|
||||
encPrivateKey: idpData?.encPrivateKey,
|
||||
encPrivateKey: normalizePem(idpData?.encPrivateKey),
|
||||
encPrivateKeyPass: idpData?.encPrivateKeyPass,
|
||||
});
|
||||
}
|
||||
|
||||
@@ -376,6 +376,13 @@ export async function processSAMLResponse(
|
||||
const attributes = extract.attributes || {};
|
||||
const mapping = parsedSamlConfig.mapping ?? {};
|
||||
|
||||
// samlify >= 2.13 types attribute values as `string | string[]` to support
|
||||
// multi-valued attributes. The identity fields below are single-valued.
|
||||
const attr = (key: string): string | undefined => {
|
||||
const value = attributes[key];
|
||||
return Array.isArray(value) ? value[0] : value;
|
||||
};
|
||||
|
||||
const userInfo = {
|
||||
...Object.fromEntries(
|
||||
Object.entries(mapping.extraFields || {}).map(([key, value]) => [
|
||||
@@ -383,22 +390,24 @@ export async function processSAMLResponse(
|
||||
attributes[value as string],
|
||||
]),
|
||||
),
|
||||
id: attributes[mapping.id || "nameID"] || extract.nameID,
|
||||
id: attr(mapping.id || "nameID") || extract.nameID,
|
||||
email: (
|
||||
attributes[mapping.email || "email"] || extract.nameID
|
||||
attr(mapping.email || "email") ||
|
||||
extract.nameID ||
|
||||
""
|
||||
).toLowerCase(),
|
||||
name:
|
||||
[
|
||||
attributes[mapping.firstName || "givenName"],
|
||||
attributes[mapping.lastName || "surname"],
|
||||
attr(mapping.firstName || "givenName"),
|
||||
attr(mapping.lastName || "surname"),
|
||||
]
|
||||
.filter(Boolean)
|
||||
.join(" ") ||
|
||||
attributes[mapping.name || "displayName"] ||
|
||||
attr(mapping.name || "displayName") ||
|
||||
extract.nameID,
|
||||
emailVerified:
|
||||
options?.trustEmailVerified && mapping.emailVerified
|
||||
? ((attributes[mapping.emailVerified] || false) as boolean)
|
||||
? ((attr(mapping.emailVerified) || false) as boolean)
|
||||
: false,
|
||||
};
|
||||
if (!userInfo.id || !userInfo.email) {
|
||||
|
||||
@@ -20,6 +20,7 @@ import { handleOAuthUserInfo } from "better-auth/oauth2";
|
||||
import { decodeJwt } from "jose";
|
||||
import type { BindingContext } from "samlify/types/src/entity";
|
||||
import type { IdentityProvider } from "samlify/types/src/entity-idp";
|
||||
import type { RequestInfo } from "samlify/types/src/types";
|
||||
import * as z from "zod";
|
||||
import * as constants from "../constants";
|
||||
import { assignOrganizationFromProvider } from "../linking";
|
||||
@@ -45,7 +46,12 @@ import type {
|
||||
SSOOptions,
|
||||
SSOProvider,
|
||||
} from "../types";
|
||||
import { domainMatches, safeJsonParse, validateEmailDomain } from "../utils";
|
||||
import {
|
||||
domainMatches,
|
||||
normalizePem,
|
||||
safeJsonParse,
|
||||
validateEmailDomain,
|
||||
} from "../utils";
|
||||
import { getVerificationIdentifier } from "./domain-verification";
|
||||
import {
|
||||
createIdP,
|
||||
@@ -1291,9 +1297,10 @@ export const signInSSO = (options?: SSOOptions) => {
|
||||
const sp = saml.ServiceProvider({
|
||||
metadata: metadata,
|
||||
allowCreate: true,
|
||||
privateKey:
|
||||
privateKey: normalizePem(
|
||||
parsedSamlConfig.spMetadata?.privateKey ||
|
||||
parsedSamlConfig.privateKey,
|
||||
parsedSamlConfig.privateKey,
|
||||
),
|
||||
privateKeyPass: parsedSamlConfig.spMetadata?.privateKeyPass,
|
||||
relayState,
|
||||
});
|
||||
@@ -1313,16 +1320,16 @@ export const signInSSO = (options?: SSOOptions) => {
|
||||
wantAuthnRequestsSigned:
|
||||
parsedSamlConfig.authnRequestsSigned || false,
|
||||
isAssertionEncrypted: idpData?.isAssertionEncrypted || false,
|
||||
encPrivateKey: idpData?.encPrivateKey,
|
||||
encPrivateKey: normalizePem(idpData?.encPrivateKey),
|
||||
encPrivateKeyPass: idpData?.encPrivateKeyPass,
|
||||
});
|
||||
} else {
|
||||
idp = saml.IdentityProvider({
|
||||
metadata: idpData.metadata,
|
||||
privateKey: idpData.privateKey,
|
||||
privateKey: normalizePem(idpData.privateKey),
|
||||
privateKeyPass: idpData.privateKeyPass,
|
||||
isAssertionEncrypted: idpData.isAssertionEncrypted,
|
||||
encPrivateKey: idpData.encPrivateKey,
|
||||
encPrivateKey: normalizePem(idpData.encPrivateKey),
|
||||
encPrivateKeyPass: idpData.encPrivateKeyPass,
|
||||
});
|
||||
}
|
||||
@@ -2235,17 +2242,17 @@ async function handleLogoutRequest(
|
||||
|
||||
deleteSessionCookie(ctx);
|
||||
|
||||
const requestId = parsed.extract.request?.id || "";
|
||||
// Pass the parsed request so samlify links `InResponseTo` and fills the
|
||||
// response template (ID, Issuer, IssueInstant, Destination, StatusCode).
|
||||
const res = sp.createLogoutResponse(
|
||||
idp,
|
||||
null,
|
||||
parsed as unknown as RequestInfo,
|
||||
binding,
|
||||
relayState || "",
|
||||
(template: string) =>
|
||||
template
|
||||
.replace("{InResponseTo}", requestId)
|
||||
.replace("{StatusCode}", constants.SAML_STATUS_SUCCESS),
|
||||
) as { context: string; entityEndpoint?: string };
|
||||
) as {
|
||||
context: string;
|
||||
entityEndpoint?: string;
|
||||
};
|
||||
|
||||
if (binding === "post" && res.entityEndpoint) {
|
||||
return createSAMLPostForm(
|
||||
|
||||
@@ -20,6 +20,7 @@ import type {
|
||||
Response as ExpressResponse,
|
||||
} from "express";
|
||||
import express from "express";
|
||||
import type { RequestInfo } from "samlify/types/src/types";
|
||||
import {
|
||||
afterAll,
|
||||
afterEach,
|
||||
@@ -34,6 +35,7 @@ import { sso, validateSAMLTimestamp } from ".";
|
||||
import { ssoClient } from "./client";
|
||||
import { DEFAULT_CLOCK_SKEW_MS } from "./constants";
|
||||
import { saml } from "./samlify";
|
||||
import { normalizePem } from "./utils";
|
||||
|
||||
const spMetadata = `
|
||||
<md:EntityDescriptor xmlns:md="urn:oasis:names:tc:SAML:2.0:metadata" entityID="http://localhost:3001/api/sso/saml2/sp/metadata">
|
||||
@@ -109,7 +111,18 @@ const idpMetadata = `
|
||||
</md:ContactPerson>
|
||||
</md:EntityDescriptor>
|
||||
`;
|
||||
const idPk = `
|
||||
/**
|
||||
* Strips the indentation that keeps these PEM fixtures readable in the source.
|
||||
* samlify >= 2.11 parses keys with Node's native crypto (OpenSSL 3), which
|
||||
* rejects leading whitespace in PEM blocks; node-forge previously tolerated it.
|
||||
*/
|
||||
const dedentPem = (strings: TemplateStringsArray) =>
|
||||
`${(strings[0] ?? "")
|
||||
.split("\n")
|
||||
.map((line) => line.trim())
|
||||
.join("\n")
|
||||
.trim()}\n`;
|
||||
const idPk = dedentPem`
|
||||
-----BEGIN RSA PRIVATE KEY-----
|
||||
MIIJKgIBAAKCAgEA+YIi6C8hA+NSB7dEcQf5OseCtL8wCohnnD8nnUTUdaMor9zm
|
||||
JLhuY4ExNsHD2XO5A9RIOXerYdAVh8v6L2jUuZpIJzleoypDGlluLAjEpy52lstL
|
||||
@@ -163,7 +176,7 @@ const idPk = `
|
||||
-----END RSA PRIVATE KEY-----
|
||||
|
||||
`;
|
||||
const spPrivateKey = `
|
||||
const spPrivateKey = dedentPem`
|
||||
-----BEGIN RSA PRIVATE KEY-----
|
||||
Proc-Type: 4,ENCRYPTED
|
||||
DEK-Info: DES-EDE3-CBC,9C86371F0420A091
|
||||
@@ -195,7 +208,7 @@ const spPrivateKey = `
|
||||
jnnjCjsK5YzTa4hmbHhPZIW262KoFV9TqxYKkhP5ab7AXRSakrdrY2cwACWN4AMT
|
||||
-----END RSA PRIVATE KEY-----
|
||||
`;
|
||||
const idpPrivateKey = `
|
||||
const idpPrivateKey = dedentPem`
|
||||
-----BEGIN RSA PRIVATE KEY-----
|
||||
Proc-Type: 4,ENCRYPTED
|
||||
DEK-Info: DES-EDE3-CBC,116B0EBB2F2F0A9D
|
||||
@@ -250,7 +263,7 @@ const certificate = `
|
||||
yyoWAJDUHiAmvFA=
|
||||
-----END CERTIFICATE-----
|
||||
`;
|
||||
const idpEncryptionKey = `
|
||||
const idpEncryptionKey = dedentPem`
|
||||
-----BEGIN RSA PRIVATE KEY-----
|
||||
Proc-Type: 4,ENCRYPTED
|
||||
DEK-Info: DES-EDE3-CBC,860FDB9F3BE14699
|
||||
@@ -282,7 +295,7 @@ const idpEncryptionKey = `
|
||||
ISbutnQPUN5fsaIsgKDIV3T7n6519t6brobcW5bdigmf5ebFeZJ16/lYy6V77UM5
|
||||
-----END RSA PRIVATE KEY-----
|
||||
`;
|
||||
const spEncryptionKey = `
|
||||
const spEncryptionKey = dedentPem`
|
||||
-----BEGIN RSA PRIVATE KEY-----
|
||||
Proc-Type: 4,ENCRYPTED
|
||||
DEK-Info: DES-EDE3-CBC,860FDB9F3BE14699
|
||||
@@ -493,13 +506,13 @@ const createMockSAMLIdP = (port: number, options: MockIdPOptions = {}) => {
|
||||
emailAddress: emailValue,
|
||||
famName: "hello world",
|
||||
};
|
||||
const { context, entityEndpoint } = await idp.createLoginResponse(
|
||||
const { context, entityEndpoint } = (await idp.createLoginResponse(
|
||||
sp,
|
||||
{} as any,
|
||||
saml.Constants.wording.binding.post,
|
||||
user,
|
||||
createTemplateCallback(idp, sp, user.emailAddress),
|
||||
);
|
||||
)) as { context: string; entityEndpoint?: string };
|
||||
res.status(200).json({ samlResponse: context, entityEndpoint });
|
||||
} catch (error) {
|
||||
res.status(500).json({
|
||||
@@ -589,6 +602,21 @@ afterAll(async () => {
|
||||
await sharedMockIdP.stop();
|
||||
});
|
||||
|
||||
describe("private key normalization", () => {
|
||||
// samlify >= 2.11 parses keys with native crypto (OpenSSL 3), which rejects
|
||||
// indented PEM that node-forge tolerated. normalizePem keeps such keys working.
|
||||
it("loads PEM keys pasted with leading indentation", () => {
|
||||
const indented = idPk
|
||||
.split("\n")
|
||||
.map((line) => ` ${line}`)
|
||||
.join("\n");
|
||||
expect(() => createPrivateKey({ key: indented })).toThrow();
|
||||
expect(() =>
|
||||
createPrivateKey({ key: normalizePem(indented) }),
|
||||
).not.toThrow();
|
||||
});
|
||||
});
|
||||
|
||||
describe("SAML SSO with defaultSSO array", async () => {
|
||||
const data = {
|
||||
user: [],
|
||||
@@ -5443,7 +5471,7 @@ describe("SAML Single Logout (SLO)", () => {
|
||||
|
||||
const logoutResponse = idp.createLogoutResponse(
|
||||
sp,
|
||||
null,
|
||||
null as unknown as RequestInfo,
|
||||
saml.Constants.wording.binding.redirect,
|
||||
callbackUrl,
|
||||
) as { context: string };
|
||||
@@ -5527,7 +5555,7 @@ describe("SAML Single Logout (SLO)", () => {
|
||||
|
||||
const logoutResponse = idp.createLogoutResponse(
|
||||
sp,
|
||||
null,
|
||||
null as unknown as RequestInfo,
|
||||
saml.Constants.wording.binding.redirect,
|
||||
"",
|
||||
) as { context: string };
|
||||
|
||||
@@ -74,6 +74,23 @@ export function parseCertificate(certPem: string) {
|
||||
};
|
||||
}
|
||||
|
||||
/**
|
||||
* samlify >= 2.11 parses private keys with Node's native crypto (OpenSSL 3),
|
||||
* which rejects PEM blocks carrying leading indentation that the previous
|
||||
* node-forge implementation tolerated. Trim each line so keys pasted with
|
||||
* surrounding whitespace (e.g. indented YAML/JSON config) still load.
|
||||
*/
|
||||
export function normalizePem(key: string): string;
|
||||
export function normalizePem(key: string | undefined): string | undefined;
|
||||
export function normalizePem(key: string | undefined): string | undefined {
|
||||
if (!key) return key;
|
||||
return `${key
|
||||
.split("\n")
|
||||
.map((line) => line.trim())
|
||||
.join("\n")
|
||||
.trim()}\n`;
|
||||
}
|
||||
|
||||
export function getHostnameFromDomain(domain: string): string | null {
|
||||
return getHostname(domain) || null;
|
||||
}
|
||||
|
||||
Generated
+12
-15
@@ -1466,8 +1466,8 @@ importers:
|
||||
specifier: ^6.1.3
|
||||
version: 6.1.3
|
||||
samlify:
|
||||
specifier: ~2.10.2
|
||||
version: 2.10.2
|
||||
specifier: ^2.13.1
|
||||
version: 2.13.1
|
||||
tldts:
|
||||
specifier: ^6.1.0
|
||||
version: 6.1.86
|
||||
@@ -11656,9 +11656,6 @@ packages:
|
||||
pako@0.2.9:
|
||||
resolution: {integrity: sha512-NUcwaKxUxWrZLpDG+z/xZaCgQITkA/Dv4V/T6bw7VON6l1Xz/VnrBqrYjZQ12TamKHzITTfOEIYUj48y2KXImA==}
|
||||
|
||||
pako@1.0.11:
|
||||
resolution: {integrity: sha512-4hLB8Py4zZce5s4yd9XzopqwVv/yGNhV1Bl8NTmCq1763HeK2+EwVTv+leGeL13Dnh2wfbqowVPXCIO0z4taYw==}
|
||||
|
||||
parent-module@1.0.1:
|
||||
resolution: {integrity: sha512-GQ2EWRpQV8/o+Aw8YqtfZZPfNRWZYkbidE9k5rpl/hC3vtHHBfGm2Ifi6qWV+coDGkrUKZAxE3Lot5kcsRlh+g==}
|
||||
engines: {node: '>=6'}
|
||||
@@ -12655,8 +12652,8 @@ packages:
|
||||
safer-buffer@2.1.2:
|
||||
resolution: {integrity: sha512-YZo3K82SD7Riyi0E1EQPojLz7kpepnSQI9IyPbHHg1XXXevb5dJI7tpyN2ADxGcQbHG7vcyRHk0cbwqcQriUtg==}
|
||||
|
||||
samlify@2.10.2:
|
||||
resolution: {integrity: sha512-y5s1cHwclqwP8h7K2Wj9SfP1q+1S9+jrs5OAegYTLAiuFi7nDvuKqbiXLmUTvYPMpzHcX94wTY2+D604jgTKvA==}
|
||||
samlify@2.13.1:
|
||||
resolution: {integrity: sha512-vdYr/zohDGBbfWNU4miEzc1jmWOtkLySPViapC6nfGkv9KxzLq4UlGkKyryzwLw4jVlZk88Rw93HaCRVpe+t+g==}
|
||||
|
||||
satori@0.21.0:
|
||||
resolution: {integrity: sha512-PV5II6Z+gsFFLq4jp6mslt0ObNRzcTi5Xq+8JtJ3ReqBS9k4nF4ajqITFXygPZgRA6l5+22NYIS1WRTqTfDp5g==}
|
||||
@@ -14347,6 +14344,10 @@ packages:
|
||||
resolution: {integrity: sha512-NNXnzrkDrAzalLhIUc01jO2mOzXGXh1JwPgkihcLLzw98c0WgYDmmjSh1Kl3wzaxSVWMuA+fe0WTWOBDWCBmNA==}
|
||||
engines: {node: '>=0.6.0'}
|
||||
|
||||
xpath@0.0.34:
|
||||
resolution: {integrity: sha512-FxF6+rkr1rNSQrhUNYrAFJpRXNzlDoMxeXN5qI84939ylEv3qqPFKa85Oxr6tDaJKqwW6KKyo2v26TSv3k6LeA==}
|
||||
engines: {node: '>=0.6.0'}
|
||||
|
||||
xtend@4.0.2:
|
||||
resolution: {integrity: sha512-LKYU1iAXJXUgAXn9URjiu+MWhyUXHsvfp7mcuYm9dSUKK0/CjtrUwFAxD82/mCWbtLsGjFIad0wIsod4zrTAEQ==}
|
||||
engines: {node: '>=0.4'}
|
||||
@@ -26009,8 +26010,6 @@ snapshots:
|
||||
|
||||
pako@0.2.9: {}
|
||||
|
||||
pako@1.0.11: {}
|
||||
|
||||
parent-module@1.0.1:
|
||||
dependencies:
|
||||
callsites: 3.1.0
|
||||
@@ -27414,19 +27413,15 @@ snapshots:
|
||||
|
||||
safer-buffer@2.1.2: {}
|
||||
|
||||
samlify@2.10.2:
|
||||
samlify@2.13.1:
|
||||
dependencies:
|
||||
'@authenio/xml-encryption': 2.0.2
|
||||
'@xmldom/xmldom': 0.8.13
|
||||
camelcase: 6.3.0
|
||||
node-forge: 1.4.0
|
||||
node-rsa: 1.1.1
|
||||
pako: 1.0.11
|
||||
uuid: 11.1.1
|
||||
xml: 1.0.1
|
||||
xml-crypto: 6.1.2
|
||||
xml-escape: 1.1.0
|
||||
xpath: 0.0.32
|
||||
xpath: 0.0.34
|
||||
|
||||
satori@0.21.0:
|
||||
dependencies:
|
||||
@@ -29289,6 +29284,8 @@ snapshots:
|
||||
|
||||
xpath@0.0.33: {}
|
||||
|
||||
xpath@0.0.34: {}
|
||||
|
||||
xtend@4.0.2: {}
|
||||
|
||||
y18n@4.0.3: {}
|
||||
|
||||
Reference in New Issue
Block a user