From e8cc734db519e1f365859f6ca905ee398a356ffb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Paola=20Estefan=C3=ADa=20de=20Campos?= <84341268+Paola3stefania@users.noreply.github.com> Date: Mon, 8 Dec 2025 00:25:27 -0300 Subject: [PATCH] fix(sso): safely parse provider configs on registration (#6550) Co-authored-by: Bereket Engida Co-authored-by: Bereket Engida <86073083+Bekacru@users.noreply.github.com> --- packages/sso/src/routes/sso.ts | 38 +------ packages/sso/src/saml.test.ts | 181 +++++++++++++++++++++++++++++++++ packages/sso/src/utils.ts | 31 ++++++ 3 files changed, 217 insertions(+), 33 deletions(-) diff --git a/packages/sso/src/routes/sso.ts b/packages/sso/src/routes/sso.ts index a41f3e2551..f6e998ff5c 100644 --- a/packages/sso/src/routes/sso.ts +++ b/packages/sso/src/routes/sso.ts @@ -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(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 = (options: O) => { return ctx.json({ ...provider, - oidcConfig: JSON.parse( + oidcConfig: safeJsonParse( provider.oidcConfig as unknown as string, - ) as OIDCConfig, - samlConfig: JSON.parse( + ), + samlConfig: safeJsonParse( provider.samlConfig as unknown as string, - ) as SAMLConfig, + ), redirectURI: `${ctx.context.baseURL}/sso/callback/${provider.providerId}`, ...(options?.domainVerification?.enabled ? { domainVerified } : {}), ...(options?.domainVerification?.enabled diff --git a/packages/sso/src/saml.test.ts b/packages/sso/src/saml.test.ts index 9f94b193f6..1799106a73 100644 --- a/packages/sso/src/saml.test.ts +++ b/packages/sso/src/saml.test.ts @@ -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(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(obj); + expect(result).toBe(obj); + }); + + it("handles empty string JSON", () => { + const result = safeJsonParse>("{}"); + 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"); + }); +}); diff --git a/packages/sso/src/utils.ts b/packages/sso/src/utils.ts index 8f08325dfc..6d465aed82 100644 --- a/packages/sso/src/utils.ts +++ b/packages/sso/src/utils.ts @@ -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( + 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();