mirror of
https://github.com/better-auth/better-auth.git
synced 2026-05-26 00:46:44 -05:00
fix(sso): safely parse provider configs on registration (#6550)
Co-authored-by: Bereket Engida <Bekacru@gmail.com> Co-authored-by: Bereket Engida <86073083+Bekacru@users.noreply.github.com>
This commit is contained in:
committed by
GitHub
parent
c7bcab1c55
commit
e8cc734db5
@@ -22,35 +22,7 @@ import type { IdentityProvider } from "samlify/types/src/entity-idp";
|
||||
import type { FlowResult } from "samlify/types/src/flow";
|
||||
import * as z from "zod/v4";
|
||||
import type { OIDCConfig, SAMLConfig, SSOOptions, SSOProvider } from "../types";
|
||||
import { validateEmailDomain } from "../utils";
|
||||
|
||||
/**
|
||||
* Safely parses a value that might be a JSON string or already a parsed object
|
||||
* This handles cases where ORMs like Drizzle might return already parsed objects
|
||||
* instead of JSON strings from TEXT/JSON columns
|
||||
*/
|
||||
function safeJsonParse<T>(value: string | T | null | undefined): T | null {
|
||||
if (!value) return null;
|
||||
|
||||
// If it's already an object (not a string), return it as-is
|
||||
if (typeof value === "object") {
|
||||
return value as T;
|
||||
}
|
||||
|
||||
// If it's a string, try to parse it
|
||||
if (typeof value === "string") {
|
||||
try {
|
||||
return JSON.parse(value) as T;
|
||||
} catch (error) {
|
||||
// If parsing fails, this might indicate the string is not valid JSON
|
||||
throw new Error(
|
||||
`Failed to parse JSON: ${error instanceof Error ? error.message : "Unknown error"}`,
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
return null;
|
||||
}
|
||||
import { safeJsonParse, validateEmailDomain } from "../utils";
|
||||
|
||||
const spMetadataQuerySchema = z.object({
|
||||
providerId: z.string(),
|
||||
@@ -683,12 +655,12 @@ export const registerSSOProvider = <O extends SSOOptions>(options: O) => {
|
||||
|
||||
return ctx.json({
|
||||
...provider,
|
||||
oidcConfig: JSON.parse(
|
||||
oidcConfig: safeJsonParse<OIDCConfig>(
|
||||
provider.oidcConfig as unknown as string,
|
||||
) as OIDCConfig,
|
||||
samlConfig: JSON.parse(
|
||||
),
|
||||
samlConfig: safeJsonParse<SAMLConfig>(
|
||||
provider.samlConfig as unknown as string,
|
||||
) as SAMLConfig,
|
||||
),
|
||||
redirectURI: `${ctx.context.baseURL}/sso/callback/${provider.providerId}`,
|
||||
...(options?.domainVerification?.enabled ? { domainVerified } : {}),
|
||||
...(options?.domainVerification?.enabled
|
||||
|
||||
@@ -1490,3 +1490,184 @@ describe("SAML SSO with custom fields", () => {
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
import { safeJsonParse } from "./utils";
|
||||
|
||||
describe("safeJsonParse", () => {
|
||||
it("returns object as-is when value is already an object", () => {
|
||||
const obj = { a: 1, nested: { b: 2 } };
|
||||
const result = safeJsonParse<typeof obj>(obj);
|
||||
expect(result).toBe(obj); // same reference
|
||||
expect(result).toEqual({ a: 1, nested: { b: 2 } });
|
||||
});
|
||||
|
||||
it("parses stringified JSON when value is a string", () => {
|
||||
const json = '{"a":1,"nested":{"b":2}}';
|
||||
const result = safeJsonParse<{ a: number; nested: { b: number } }>(json);
|
||||
expect(result).toEqual({ a: 1, nested: { b: 2 } });
|
||||
});
|
||||
|
||||
it("returns null for null input", () => {
|
||||
const result = safeJsonParse<{ a: number }>(null);
|
||||
expect(result).toBeNull();
|
||||
});
|
||||
|
||||
it("returns null for undefined input", () => {
|
||||
const result = safeJsonParse<{ a: number }>(undefined);
|
||||
expect(result).toBeNull();
|
||||
});
|
||||
|
||||
it("throws error for invalid JSON string", () => {
|
||||
expect(() => safeJsonParse<{ a: number }>("not valid json")).toThrow(
|
||||
"Failed to parse JSON",
|
||||
);
|
||||
});
|
||||
|
||||
it("handles empty object", () => {
|
||||
const obj = {};
|
||||
const result = safeJsonParse<typeof obj>(obj);
|
||||
expect(result).toBe(obj);
|
||||
});
|
||||
|
||||
it("handles empty string JSON", () => {
|
||||
const result = safeJsonParse<Record<string, never>>("{}");
|
||||
expect(result).toEqual({});
|
||||
});
|
||||
});
|
||||
|
||||
describe("SSO Provider Config Parsing", () => {
|
||||
it("returns parsed SAML config and avoids [object Object] in response", async () => {
|
||||
const data = {
|
||||
user: [] as any[],
|
||||
session: [] as any[],
|
||||
verification: [] as any[],
|
||||
account: [] as any[],
|
||||
ssoProvider: [] as any[],
|
||||
};
|
||||
|
||||
const memory = memoryAdapter(data);
|
||||
|
||||
const auth = betterAuth({
|
||||
database: memory,
|
||||
baseURL: "http://localhost:3000",
|
||||
emailAndPassword: { enabled: true },
|
||||
plugins: [sso()],
|
||||
});
|
||||
|
||||
const authClient = createAuthClient({
|
||||
baseURL: "http://localhost:3000",
|
||||
plugins: [bearer(), ssoClient()],
|
||||
fetchOptions: {
|
||||
customFetchImpl: async (url, init) =>
|
||||
auth.handler(new Request(url, init)),
|
||||
},
|
||||
});
|
||||
|
||||
const headers = new Headers();
|
||||
await authClient.signUp.email({
|
||||
email: "test@example.com",
|
||||
password: "password123",
|
||||
name: "Test User",
|
||||
});
|
||||
await authClient.signIn.email(
|
||||
{ email: "test@example.com", password: "password123" },
|
||||
{ onSuccess: setCookieToHeader(headers) },
|
||||
);
|
||||
|
||||
const provider = await auth.api.registerSSOProvider({
|
||||
body: {
|
||||
providerId: "saml-config-provider",
|
||||
issuer: "http://localhost:8081",
|
||||
domain: "example.com",
|
||||
samlConfig: {
|
||||
entryPoint: "http://localhost:8081/sso",
|
||||
cert: "test-cert",
|
||||
callbackUrl: "http://localhost:3000/callback",
|
||||
spMetadata: {
|
||||
entityID: "test-entity",
|
||||
},
|
||||
},
|
||||
},
|
||||
headers,
|
||||
});
|
||||
|
||||
expect(provider.samlConfig).toBeDefined();
|
||||
expect(typeof provider.samlConfig).toBe("object");
|
||||
expect(provider.samlConfig?.entryPoint).toBe("http://localhost:8081/sso");
|
||||
expect(provider.samlConfig?.cert).toBe("test-cert");
|
||||
|
||||
const serialized = JSON.stringify(provider.samlConfig);
|
||||
expect(serialized).not.toContain("[object Object]");
|
||||
|
||||
expect(provider.samlConfig?.spMetadata?.entityID).toBe("test-entity");
|
||||
});
|
||||
|
||||
it("returns parsed OIDC config and avoids [object Object] in response", async () => {
|
||||
const data = {
|
||||
user: [] as any[],
|
||||
session: [] as any[],
|
||||
verification: [] as any[],
|
||||
account: [] as any[],
|
||||
ssoProvider: [] as any[],
|
||||
};
|
||||
|
||||
const memory = memoryAdapter(data);
|
||||
|
||||
const auth = betterAuth({
|
||||
database: memory,
|
||||
baseURL: "http://localhost:3000",
|
||||
emailAndPassword: { enabled: true },
|
||||
plugins: [sso()],
|
||||
});
|
||||
|
||||
const authClient = createAuthClient({
|
||||
baseURL: "http://localhost:3000",
|
||||
plugins: [bearer(), ssoClient()],
|
||||
fetchOptions: {
|
||||
customFetchImpl: async (url, init) =>
|
||||
auth.handler(new Request(url, init)),
|
||||
},
|
||||
});
|
||||
|
||||
const headers = new Headers();
|
||||
await authClient.signUp.email({
|
||||
email: "test@example.com",
|
||||
password: "password123",
|
||||
name: "Test User",
|
||||
});
|
||||
await authClient.signIn.email(
|
||||
{ email: "test@example.com", password: "password123" },
|
||||
{ onSuccess: setCookieToHeader(headers) },
|
||||
);
|
||||
|
||||
const provider = await auth.api.registerSSOProvider({
|
||||
body: {
|
||||
providerId: "oidc-config-provider",
|
||||
issuer: "http://localhost:8080",
|
||||
domain: "example.com",
|
||||
oidcConfig: {
|
||||
clientId: "test-client",
|
||||
clientSecret: "test-secret",
|
||||
discoveryEndpoint:
|
||||
"http://localhost:8080/.well-known/openid-configuration",
|
||||
mapping: {
|
||||
id: "sub",
|
||||
email: "email",
|
||||
name: "name",
|
||||
},
|
||||
},
|
||||
},
|
||||
headers,
|
||||
});
|
||||
|
||||
expect(provider.oidcConfig).toBeDefined();
|
||||
expect(typeof provider.oidcConfig).toBe("object");
|
||||
expect(provider.oidcConfig?.clientId).toBe("test-client");
|
||||
expect(provider.oidcConfig?.clientSecret).toBe("test-secret");
|
||||
|
||||
const serialized = JSON.stringify(provider.oidcConfig);
|
||||
expect(serialized).not.toContain("[object Object]");
|
||||
|
||||
expect(provider.oidcConfig?.mapping?.id).toBe("sub");
|
||||
});
|
||||
});
|
||||
|
||||
@@ -1,3 +1,34 @@
|
||||
/**
|
||||
* Safely parses a value that might be a JSON string or already a parsed object.
|
||||
* This handles cases where ORMs like Drizzle might return already parsed objects
|
||||
* instead of JSON strings from TEXT/JSON columns.
|
||||
*
|
||||
* @param value - The value to parse (string, object, null, or undefined)
|
||||
* @returns The parsed object or null
|
||||
* @throws Error if string parsing fails
|
||||
*/
|
||||
export function safeJsonParse<T>(
|
||||
value: string | T | null | undefined,
|
||||
): T | null {
|
||||
if (!value) return null;
|
||||
|
||||
if (typeof value === "object") {
|
||||
return value as T;
|
||||
}
|
||||
|
||||
if (typeof value === "string") {
|
||||
try {
|
||||
return JSON.parse(value) as T;
|
||||
} catch (error) {
|
||||
throw new Error(
|
||||
`Failed to parse JSON: ${error instanceof Error ? error.message : "Unknown error"}`,
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
return null;
|
||||
}
|
||||
|
||||
export const validateEmailDomain = (email: string, domain: string) => {
|
||||
const emailDomain = email.split("@")[1]?.toLowerCase();
|
||||
const providerDomain = domain.toLowerCase();
|
||||
|
||||
Reference in New Issue
Block a user