From 085071987ec6b12bf738309d796d19b7e5810058 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Sat, 19 Sep 2026 17:16:10 -0700 Subject: [PATCH 1/9] fix(sso): encrypt SSO provider secrets at rest The OIDC client secret, and the SAML signing and decryption keys, were stored as plain JSON in sso_provider while every sibling credential is encrypted with ENCRYPTION_KEY, so a copy of the database exposed them without the key. They are now encrypted field-by-field at the Better Auth adapter, which is the only seam the SSO plugin's reads and writes both pass through; the surrounding config stays readable JSON. Values written before this keep working and are encrypted the next time the provider is saved. The providers API also redacts the SAML key material it used to return in full. --- .../content/docs/platform/enterprise/sso.mdx | 2 +- .../platform/self-hosting/architecture.mdx | 2 +- .../docs/platform/self-hosting/docker.mdx | 2 +- .../self-hosting/environment-variables.mdx | 2 +- .../docs/platform/self-hosting/kubernetes.mdx | 4 +- .../docs/platform/self-hosting/security.mdx | 2 +- .../platform/self-hosting/troubleshooting.mdx | 2 + .../docs/platform/self-hosting/upgrades.mdx | 2 +- .../app/api/auth/sso/providers/route.test.ts | 66 ++++++- apps/sim/app/api/auth/sso/providers/route.ts | 62 +++++-- .../app/api/auth/sso/register/route.test.ts | 57 ++++++ apps/sim/app/api/auth/sso/register/route.ts | 10 +- apps/sim/lib/auth/sim-auth-adapter.ts | 5 +- ...o-provider-secret-adapter.postgres.test.ts | 148 +++++++++++++++ .../auth/sso-provider-secret-adapter.test.ts | 173 ++++++++++++++++++ .../lib/auth/sso-provider-secret-adapter.ts | 123 +++++++++++++ .../sim/lib/auth/sso/provider-secrets.test.ts | 112 ++++++++++++ apps/sim/lib/auth/sso/provider-secrets.ts | 113 ++++++++++++ packages/db/scripts/register-sso-provider.ts | 62 ++++++- 19 files changed, 917 insertions(+), 32 deletions(-) create mode 100644 apps/sim/lib/auth/sso-provider-secret-adapter.postgres.test.ts create mode 100644 apps/sim/lib/auth/sso-provider-secret-adapter.test.ts create mode 100644 apps/sim/lib/auth/sso-provider-secret-adapter.ts create mode 100644 apps/sim/lib/auth/sso/provider-secrets.test.ts create mode 100644 apps/sim/lib/auth/sso/provider-secrets.ts diff --git a/apps/docs/content/docs/platform/enterprise/sso.mdx b/apps/docs/content/docs/platform/enterprise/sso.mdx index ff7a6fb4002..4f7f1092d42 100644 --- a/apps/docs/content/docs/platform/enterprise/sso.mdx +++ b/apps/docs/content/docs/platform/enterprise/sso.mdx @@ -101,7 +101,7 @@ Click **Save**. To test, sign out and use the **Sign in with SSO** button on the ## Editing and advanced configuration -For a saved connection, open **Sign-in**, select the provider, and select **Edit**. The Provider ID remains fixed. **Delete** removes that sign-in path only: accounts and memberships it admitted stay. If you delete the primary provider and the domain has another verified provider, that one becomes primary; otherwise people at the domain sign in another way until a provider serves it again. A saved OIDC client secret appears as a mask with a suffix when available; **Replace** lets you enter a new secret, and **Keep saved** cancels that replacement. Select **Update** to save the provider, or **Discard** to abandon changes. +For a saved connection, open **Sign-in**, select the provider, and select **Edit**. The Provider ID remains fixed. **Delete** removes that sign-in path only: accounts and memberships it admitted stay. If you delete the primary provider and the domain has another verified provider, that one becomes primary; otherwise people at the domain sign in another way until a provider serves it again. A saved OIDC client secret appears as a mask with a suffix when available; **Replace** lets you enter a new secret, and **Keep saved** cancels that replacement. Provider secrets — the OIDC client secret, and SAML signing and decryption keys — are encrypted with `ENCRYPTION_KEY` before they are stored, so a copy of the database alone does not expose them. Select **Update** to save the provider, or **Discard** to abandon changes. **Advanced options** contains OIDC scopes and optional authorization, token, and JWKS endpoint overrides. For SAML, it contains Audience, Callback URL override, signed-assertion requirements, NameID format, and optional IdP metadata XML. **Attribute mapping** lets either protocol override the email, name, and stable user-ID claim names. Leave a mapping blank to use the protocol default. diff --git a/apps/docs/content/docs/platform/self-hosting/architecture.mdx b/apps/docs/content/docs/platform/self-hosting/architecture.mdx index 7002b69976d..90618398286 100644 --- a/apps/docs/content/docs/platform/self-hosting/architecture.mdx +++ b/apps/docs/content/docs/platform/self-hosting/architecture.mdx @@ -73,7 +73,7 @@ Three places once the deployment is configured for production. Everything else i - `ENCRYPTION_KEY` is not recoverable and not derivable. It encrypts workspace and personal environment variables, stored provider API keys, MCP OAuth credentials, and deployment/chat secrets at rest — a database restore paired with a *different* key yields a working app in which none of that can be decrypted. Back it up separately from the database, and never rotate it casually. + `ENCRYPTION_KEY` is not recoverable and not derivable. It encrypts workspace and personal environment variables, stored provider API keys, MCP OAuth credentials, deployment/chat secrets, and SSO provider secrets at rest — a database restore paired with a *different* key yields a working app in which none of that can be decrypted. Back it up separately from the database, and never rotate it casually. Redis is a cache and message bus. Losing it drops in-flight live updates; it does not lose committed data. diff --git a/apps/docs/content/docs/platform/self-hosting/docker.mdx b/apps/docs/content/docs/platform/self-hosting/docker.mdx index 56cfe3a5027..dc17f64c85b 100644 --- a/apps/docs/content/docs/platform/self-hosting/docker.mdx +++ b/apps/docs/content/docs/platform/self-hosting/docker.mdx @@ -48,7 +48,7 @@ EOF - Save `ENCRYPTION_KEY` and `API_ENCRYPTION_KEY` somewhere outside this server. `ENCRYPTION_KEY` encrypts workspace and personal environment variables, stored provider API keys, MCP OAuth credentials, and deployment/chat secrets; `API_ENCRYPTION_KEY` encrypts user-generated Sim API keys. Neither can be regenerated — a database restore paired with a different key leaves the data it protected permanently unreadable. + Save `ENCRYPTION_KEY` and `API_ENCRYPTION_KEY` somewhere outside this server. `ENCRYPTION_KEY` encrypts workspace and personal environment variables, stored provider API keys, MCP OAuth credentials, deployment/chat secrets, and SSO provider secrets; `API_ENCRYPTION_KEY` encrypts user-generated Sim API keys. Neither can be regenerated — a database restore paired with a different key leaves the data it protected permanently unreadable. The compose file refuses to start if `BETTER_AUTH_SECRET`, `ENCRYPTION_KEY`, `INTERNAL_API_SECRET`, or `POSTGRES_PASSWORD` is missing, rather than booting with empty or well-known values. Postgres applies `POSTGRES_PASSWORD` only when it first creates the database volume — see [Postgres on Compose](/platform/self-hosting/security#postgres-on-compose) before changing it on an existing install. `CRON_SECRET` is treated more gently: without it the `cron` service prints what to set and exits, leaving the rest of the stack running — so upgrading from a compose file that predates the scheduler still works. diff --git a/apps/docs/content/docs/platform/self-hosting/environment-variables.mdx b/apps/docs/content/docs/platform/self-hosting/environment-variables.mdx index 23c80273d81..0f6f973287c 100644 --- a/apps/docs/content/docs/platform/self-hosting/environment-variables.mdx +++ b/apps/docs/content/docs/platform/self-hosting/environment-variables.mdx @@ -19,7 +19,7 @@ import { Callout } from 'fumadocs-ui/components/callout' `openssl rand -hex 32` prints 64 hex characters. `ENCRYPTION_KEY` and `API_ENCRYPTION_KEY` must be exactly that — a value of any other shape throws the first time Sim encrypts or decrypts, not at startup. The rest are secrets of no fixed shape and only need 32 characters or more. The Sim app never checks — it runs its env schema with validation skipped — but the realtime service validates `BETTER_AUTH_SECRET` and `INTERNAL_API_SECRET` at boot and refuses to start if either is shorter. - `ENCRYPTION_KEY` and `API_ENCRYPTION_KEY` cannot be rotated or recovered. Losing either makes the data it protects permanently unreadable — workspace and personal environment variables, stored provider API keys, MCP OAuth credentials, and deployment/chat secrets in the first case, user-generated Sim API keys in the second. Back them up separately from the database. + `ENCRYPTION_KEY` and `API_ENCRYPTION_KEY` cannot be rotated or recovered. Losing either makes the data it protects permanently unreadable — workspace and personal environment variables, stored provider API keys, MCP OAuth credentials, deployment/chat secrets, and SSO provider secrets in the first case, user-generated Sim API keys in the second. Back them up separately from the database. ## Strongly recommended diff --git a/apps/docs/content/docs/platform/self-hosting/kubernetes.mdx b/apps/docs/content/docs/platform/self-hosting/kubernetes.mdx index 7d2953476c8..00c26014a47 100644 --- a/apps/docs/content/docs/platform/self-hosting/kubernetes.mdx +++ b/apps/docs/content/docs/platform/self-hosting/kubernetes.mdx @@ -51,7 +51,7 @@ helm install sim oci://ghcr.io/simstudioai/charts/sim \ ``` - Save all six values somewhere durable before moving on. `ENCRYPTION_KEY` in particular cannot be regenerated — losing it makes workspace environment variables and stored provider keys permanently unreadable. + Save all six values somewhere durable before moving on. `ENCRYPTION_KEY` in particular cannot be regenerated — losing it makes workspace environment variables, stored provider keys, and SSO provider secrets permanently unreadable. `API_ENCRYPTION_KEY` is optional, and the failure mode is silent: leave it unset and Sim stores user-generated API keys **in plain text**, logging one warning and nothing else. Set it at install time — it must be a 64-character hex string, which is exactly what `openssl rand -hex 32` produces — and back it up like `ENCRYPTION_KEY`. @@ -102,7 +102,7 @@ Signing is Sigstore-only — there is no GPG `.prov` file, so `helm install --ve ## Cloud-Specific Values -These are cloud-tuned **alternatives** to the generic install above — pick one path, don't run both. The commands reuse the `$BETTER_AUTH_SECRET`, `$ENCRYPTION_KEY`, `$INTERNAL_API_SECRET`, `$API_ENCRYPTION_KEY`, `$CRON_SECRET`, and `$POSTGRES_PASSWORD` variables generated in [Installation](#installation) above, so run that block's `openssl` lines first in the same shell. They use `helm upgrade --install`, so they work whether or not a release exists yet. Two caveats when converting an existing generic install rather than starting fresh: (1) **reuse the original secret values** — recover them with `helm get values sim -n simstudio` if your shell no longer has them; supplying a newly generated `ENCRYPTION_KEY` makes every previously encrypted value (workspace environment variables, stored provider keys, MCP OAuth credentials) undecryptable. (2) The cloud values rename the bundled PostgreSQL database to `simstudio`, but Postgres only applies that setting on first initialization — add `--set postgresql.auth.database=sim` to keep your existing database. If you'd rather start clean, `helm uninstall sim -n simstudio`, delete its PVCs, and run the cloud command fresh. +These are cloud-tuned **alternatives** to the generic install above — pick one path, don't run both. The commands reuse the `$BETTER_AUTH_SECRET`, `$ENCRYPTION_KEY`, `$INTERNAL_API_SECRET`, `$API_ENCRYPTION_KEY`, `$CRON_SECRET`, and `$POSTGRES_PASSWORD` variables generated in [Installation](#installation) above, so run that block's `openssl` lines first in the same shell. They use `helm upgrade --install`, so they work whether or not a release exists yet. Two caveats when converting an existing generic install rather than starting fresh: (1) **reuse the original secret values** — recover them with `helm get values sim -n simstudio` if your shell no longer has them; supplying a newly generated `ENCRYPTION_KEY` makes every previously encrypted value (workspace environment variables, stored provider keys, MCP OAuth credentials, SSO provider secrets) undecryptable. (2) The cloud values rename the bundled PostgreSQL database to `simstudio`, but Postgres only applies that setting on first initialization — add `--set postgresql.auth.database=sim` to keep your existing database. If you'd rather start clean, `helm uninstall sim -n simstudio`, delete its PVCs, and run the cloud command fresh. ```bash # The example values files are not part of the packaged chart, so fetch the one diff --git a/apps/docs/content/docs/platform/self-hosting/security.mdx b/apps/docs/content/docs/platform/self-hosting/security.mdx index a60da61710a..738e78c4902 100644 --- a/apps/docs/content/docs/platform/self-hosting/security.mdx +++ b/apps/docs/content/docs/platform/self-hosting/security.mdx @@ -13,7 +13,7 @@ Five secrets drive the security of a deployment. Generate each with `openssl ran | Secret | Protects | Rotatable | |---|---|---| | `BETTER_AUTH_SECRET` | Session tokens | Yes — invalidates all sessions | -| `ENCRYPTION_KEY` | Workspace env vars, stored provider keys, MCP OAuth credentials, deployment/chat secrets | **No** — see below | +| `ENCRYPTION_KEY` | Workspace env vars, stored provider keys, MCP OAuth credentials, deployment/chat secrets, SSO provider secrets | **No** — see below | | `API_ENCRYPTION_KEY` | Reversible stored copy of user-generated API keys | **No** — existing keys keep authenticating, but their stored copy can no longer be displayed | | `INTERNAL_API_SECRET` | Service-to-service calls | Yes — roll app and realtime together | | `CRON_SECRET` | Background job endpoints | Yes — roll app and cron together | diff --git a/apps/docs/content/docs/platform/self-hosting/troubleshooting.mdx b/apps/docs/content/docs/platform/self-hosting/troubleshooting.mdx index 1bfa5e25130..7298cd751a0 100644 --- a/apps/docs/content/docs/platform/self-hosting/troubleshooting.mdx +++ b/apps/docs/content/docs/platform/self-hosting/troubleshooting.mdx @@ -258,6 +258,8 @@ A document that fails with `vector 0 has N unexpected dimensions` means `EMBEDDI Integrations show as connected but fail, or provider keys error on decrypt. `ENCRYPTION_KEY` does not match the value in use when the backup was taken. There is no recovery — the original key must be restored. +SSO sign-in fails the same way, since provider secrets are encrypted with the same key. A provider whose secret cannot be decrypted refuses the sign-in rather than sending an unusable secret to the identity provider; re-enter the client secret in organization settings once the correct key is in place. + ## Kubernetes: App Pods Never Become Ready Check the migrations init container first — a failed migration deliberately blocks the rollout: diff --git a/apps/docs/content/docs/platform/self-hosting/upgrades.mdx b/apps/docs/content/docs/platform/self-hosting/upgrades.mdx index 1006325ae40..4e6293796af 100644 --- a/apps/docs/content/docs/platform/self-hosting/upgrades.mdx +++ b/apps/docs/content/docs/platform/self-hosting/upgrades.mdx @@ -233,7 +233,7 @@ There is a short window where the app is unavailable while containers restart. C ### Verify -Run the [verification checklist](/platform/self-hosting/verify). At minimum: sign in, open a workflow, execute it, upload a file, and confirm the [background jobs](/platform/self-hosting/background-jobs) are still firing. +Run the [verification checklist](/platform/self-hosting/verify). At minimum: sign in, open a workflow, execute it, upload a file, and confirm the [background jobs](/platform/self-hosting/background-jobs) are still firing. If the deployment uses [SSO](/platform/enterprise/sso), complete one SSO sign-in too — provider secrets are encrypted with `ENCRYPTION_KEY`, so a key that does not match the one they were saved under surfaces here. diff --git a/apps/sim/app/api/auth/sso/providers/route.test.ts b/apps/sim/app/api/auth/sso/providers/route.test.ts index 47c7cfdcae1..eb3b8186934 100644 --- a/apps/sim/app/api/auth/sso/providers/route.test.ts +++ b/apps/sim/app/api/auth/sso/providers/route.test.ts @@ -11,19 +11,33 @@ import { } from '@sim/testing' import { beforeEach, describe, expect, it, vi } from 'vitest' -const { mockGetSession } = vi.hoisted(() => ({ mockGetSession: vi.fn() })) +const { mockGetSession, mockDecryptSecret } = vi.hoisted(() => ({ + mockGetSession: vi.fn(), + mockDecryptSecret: vi.fn(), +})) vi.mock('@sim/db', () => ({ ...dbChainMock, ...schemaMock })) vi.mock('@/lib/auth', () => ({ getSession: mockGetSession })) +/** The shared env mock's ENCRYPTION_KEY is not 64 hex characters, so real crypto would throw. */ +vi.mock('@/lib/core/security/encryption', () => ({ + encryptSecret: vi.fn(), + decryptSecret: mockDecryptSecret, +})) import { GET } from '@/app/api/auth/sso/providers/route' +const IV = 'a'.repeat(32) +const TAG = 'b'.repeat(32) +const sealed = (secret: string) => `${IV}:${Buffer.from(secret).toString('hex')}:${TAG}` + +const CLIENT_SECRET = 'a-long-client-secret-wxyz' + const providerRow = { id: 'row-1', providerId: 'acme-okta', domain: 'acme.com', issuer: 'https://acme.okta.test', - oidcConfig: JSON.stringify({ clientId: 'client', clientSecret: 'a-long-client-secret-wxyz' }), + oidcConfig: JSON.stringify({ clientId: 'client', clientSecret: sealed(CLIENT_SECRET) }), samlConfig: null, userId: 'user-1', organizationId: 'org-1', @@ -38,6 +52,9 @@ describe('GET /api/auth/sso/providers', () => { vi.clearAllMocks() resetDbChainMock() mockGetSession.mockResolvedValue({ user: { id: 'user-1' } }) + mockDecryptSecret.mockImplementation(async (value: string) => ({ + decrypted: Buffer.from(value.split(':')[1], 'hex').toString('utf8'), + })) }) it('refuses a caller without a session before reading any provider', async () => { @@ -56,10 +73,55 @@ describe('GET /api/auth/sso/providers', () => { expect(providers[0]).toMatchObject({ providerId: 'acme-okta', providerType: 'oidc' }) expect(JSON.parse(providers[0].oidcConfig)).toMatchObject({ clientSecretHint: 'wxyz' }) expect(providers[0].oidcConfig).not.toContain('a-long-client-secret') + expect(providers[0].oidcConfig).not.toContain(sealed(CLIENT_SECRET)) const condition = JSON.stringify(dbChainMockFns.where.mock.calls[0][0]) expect(condition).toContain('user-1') }) + it('hints a secret stored before encryption existed', async () => { + queueTableRows(schemaMock.ssoProvider, [ + { + ...providerRow, + oidcConfig: JSON.stringify({ clientId: 'client', clientSecret: CLIENT_SECRET }), + }, + ]) + + const res = await GET(createMockRequest('GET')) + + const { providers } = await res.json() + expect(JSON.parse(providers[0].oidcConfig)).toMatchObject({ clientSecretHint: 'wxyz' }) + expect(providers[0].oidcConfig).not.toContain(CLIENT_SECRET) + expect(mockDecryptSecret).not.toHaveBeenCalled() + }) + + it('redacts SAML key material and keeps the certificate', async () => { + queueTableRows(schemaMock.ssoProvider, [ + { + ...providerRow, + oidcConfig: null, + samlConfig: JSON.stringify({ + cert: 'public-cert', + entryPoint: 'https://acme.okta.test/sso', + privateKey: sealed('sp-signing-key'), + decryptionPvk: sealed('sp-decryption-key'), + }), + }, + ]) + + const res = await GET(createMockRequest('GET')) + + const { providers } = await res.json() + const samlConfig = JSON.parse(providers[0].samlConfig) + expect(samlConfig).toMatchObject({ + cert: 'public-cert', + entryPoint: 'https://acme.okta.test/sso', + privateKey: '[REDACTED]', + decryptionPvk: '[REDACTED]', + }) + expect(providers[0].samlConfig).not.toContain('sp-signing-key') + expect(providers[0].providerType).toBe('saml') + }) + it('refuses an organization the caller does not administer', async () => { queueTableRows(schemaMock.member, [{ organizationId: 'org-1', role: 'member' }]) const res = await GET( diff --git a/apps/sim/app/api/auth/sso/providers/route.ts b/apps/sim/app/api/auth/sso/providers/route.ts index 9a447da9df0..7fbce82a370 100644 --- a/apps/sim/app/api/auth/sso/providers/route.ts +++ b/apps/sim/app/api/auth/sso/providers/route.ts @@ -11,6 +11,7 @@ import { listSsoProvidersContract } from '@/lib/api/contracts/auth' import { parseRequest } from '@/lib/api/server' import { getSession } from '@/lib/auth' import { markSignInProviders } from '@/lib/auth/sso/primary-provider' +import { decryptProviderConfig } from '@/lib/auth/sso/provider-secrets' import { REDACTED_MARKER } from '@/lib/core/security/redaction' import { withRouteHandler } from '@/lib/core/utils/with-route-handler' @@ -31,6 +32,44 @@ function buildClientSecretHint(clientSecret: unknown): string | null { return clientSecret.slice(-4) } +/** + * Replaces the stored client secret with the redaction marker, keeping a hint + * built from the real secret. The stored value is decrypted first: a hint taken + * from the envelope would be four characters of the auth tag, which says nothing + * about the secret and changes every time the row is rewritten. + */ +async function redactOidcConfig(oidcConfig: string | null): Promise { + if (!oidcConfig) return oidcConfig + try { + const parsed = JSON.parse((await decryptProviderConfig(oidcConfig, 'oidcConfig')) as string) + const hint = buildClientSecretHint(parsed.clientSecret) + parsed.clientSecret = REDACTED_MARKER + if (hint) parsed.clientSecretHint = hint + return JSON.stringify(parsed) + } catch { + return null + } +} + +/** + * Drops the SAML key material an admin never needs back. Unlike the OIDC secret + * these carry no hint: they are the service provider's own signing and + * decryption keys, they are only ever set by the operator registration script, + * and the settings form does not read them. + */ +function redactSamlConfig(samlConfig: string | null): string | null { + if (!samlConfig) return samlConfig + try { + const parsed = JSON.parse(samlConfig) + for (const field of ['privateKey', 'decryptionPvk']) { + if (typeof parsed[field] === 'string' && parsed[field] !== '') parsed[field] = REDACTED_MARKER + } + return JSON.stringify(parsed) + } catch { + return null + } +} + /** * Lists the identity providers the caller administers: an organization's when an * owner or admin names it, otherwise the ones the caller registered. @@ -90,25 +129,14 @@ export const GET = withRouteHandler(async (request: NextRequest) => { .where(whereClause) .orderBy(asc(ssoProvider.providerId)) - const providers = markSignInProviders(results).map((provider) => { - let oidcConfig = provider.oidcConfig - if (oidcConfig) { - try { - const parsed = JSON.parse(oidcConfig) - const hint = buildClientSecretHint(parsed.clientSecret) - parsed.clientSecret = REDACTED_MARKER - if (hint) parsed.clientSecretHint = hint - oidcConfig = JSON.stringify(parsed) - } catch { - oidcConfig = null - } - } - return { + const providers = await Promise.all( + markSignInProviders(results).map(async (provider) => ({ ...provider, - oidcConfig, + oidcConfig: await redactOidcConfig(provider.oidcConfig), + samlConfig: redactSamlConfig(provider.samlConfig), providerType: (provider.samlConfig ? 'saml' : 'oidc') as 'oidc' | 'saml', - } - }) + })) + ) logger.info('Fetched SSO providers', { userId, providerCount: providers.length }) diff --git a/apps/sim/app/api/auth/sso/register/route.test.ts b/apps/sim/app/api/auth/sso/register/route.test.ts index e64e7a3487a..058107b07d9 100644 --- a/apps/sim/app/api/auth/sso/register/route.test.ts +++ b/apps/sim/app/api/auth/sso/register/route.test.ts @@ -22,6 +22,7 @@ const { mockHasSSOAccess, mockValidateUrlWithDNS, mockSecureFetchWithPinnedIP, + mockDecryptSecret, } = vi.hoisted(() => ({ mockGetSession: vi.fn(), mockRegisterSSOProvider: vi.fn(), @@ -29,6 +30,7 @@ const { mockHasSSOAccess: vi.fn(), mockValidateUrlWithDNS: vi.fn(), mockSecureFetchWithPinnedIP: vi.fn(), + mockDecryptSecret: vi.fn(), })) vi.mock('@sim/db', () => ({ ...dbChainMock, ...schemaMock })) @@ -76,6 +78,12 @@ vi.mock('@sim/utils/sso-domain', () => ({ }, })) +/** The shared env mock's ENCRYPTION_KEY is not 64 hex characters, so real crypto would throw. */ +vi.mock('@/lib/core/security/encryption', () => ({ + encryptSecret: vi.fn(), + decryptSecret: mockDecryptSecret, +})) + vi.mock('@/lib/core/security/input-validation.server', () => ({ validateUrlWithDNS: mockValidateUrlWithDNS, secureFetchWithPinnedIP: mockSecureFetchWithPinnedIP, @@ -115,6 +123,9 @@ describe('POST /api/auth/sso/register', () => { mockHasSSOAccess.mockResolvedValue(true) mockValidateUrlWithDNS.mockResolvedValue({ isValid: true, resolvedIP: '1.2.3.4' }) mockSecureFetchWithPinnedIP.mockRejectedValue(new Error('discovery not mocked for this test')) + mockDecryptSecret.mockImplementation(async (value: string) => ({ + decrypted: Buffer.from(value.split(':')[1], 'hex').toString('utf8'), + })) mockRegisterSSOProvider.mockResolvedValue({ id: 'row-1', providerId: 'acme-oidc' }) mockUpdateSSOProvider.mockResolvedValue({ providerId: 'acme-oidc' }) // The trust UPDATE reports the row it matched; by default the provider exists. @@ -288,6 +299,52 @@ describe('POST /api/auth/sso/register', () => { }) }) + /** + * Leaving the secret field blank sends the redaction marker back, and the + * route lifts the stored secret into the new config. It reads the column + * directly rather than through Better Auth, so it decrypts it itself. + */ + it('reuses the stored client secret, decrypting it first', async () => { + const sealed = `${'a'.repeat(32)}:${Buffer.from('stored-secret').toString('hex')}:${'b'.repeat(32)}` + queueMembers([{ organizationId: 'org1', role: 'owner' }]) + // In route order: providerId conflict and domain refusal, the reuse read, + // both checks again before the write, then the pre-image being updated. + queueTableRows(schemaMock.ssoProvider, []) + queueTableRows(schemaMock.ssoProvider, []) + queueTableRows(schemaMock.ssoProvider, [ + { oidcConfig: JSON.stringify({ clientId: 'client', clientSecret: sealed }) }, + ]) + queueTableRows(schemaMock.ssoProvider, []) + queueTableRows(schemaMock.ssoProvider, []) + queueTableRows(schemaMock.ssoProvider, [{ id: 'p1' }]) + + const res = await POST(request({ ...OIDC_BODY, clientSecret: '[REDACTED]' })) + + expect(res.status).toBe(200) + const sent = mockUpdateSSOProvider.mock.calls[0][0].body + expect(sent.oidcConfig.clientSecret).toBe('stored-secret') + }) + + it('reuses a client secret stored before encryption existed', async () => { + queueMembers([{ organizationId: 'org1', role: 'owner' }]) + queueTableRows(schemaMock.ssoProvider, []) + queueTableRows(schemaMock.ssoProvider, []) + queueTableRows(schemaMock.ssoProvider, [ + { oidcConfig: JSON.stringify({ clientId: 'client', clientSecret: 'legacy-plain-secret' }) }, + ]) + queueTableRows(schemaMock.ssoProvider, []) + queueTableRows(schemaMock.ssoProvider, []) + queueTableRows(schemaMock.ssoProvider, [{ id: 'p1' }]) + + const res = await POST(request({ ...OIDC_BODY, clientSecret: '[REDACTED]' })) + + expect(res.status).toBe(200) + expect(mockUpdateSSOProvider.mock.calls[0][0].body.oidcConfig.clientSecret).toBe( + 'legacy-plain-secret' + ) + expect(mockDecryptSecret).not.toHaveBeenCalled() + }) + /** updateSSOProvider resets domainVerified to false whenever the domain changes. */ it('re-marks the provider domain-verified after an update', async () => { queueMembers([{ organizationId: 'org1', role: 'owner' }]) diff --git a/apps/sim/app/api/auth/sso/register/route.ts b/apps/sim/app/api/auth/sso/register/route.ts index 04c3c94c800..7765720eaaf 100644 --- a/apps/sim/app/api/auth/sso/register/route.ts +++ b/apps/sim/app/api/auth/sso/register/route.ts @@ -8,6 +8,7 @@ import { type NextRequest, NextResponse } from 'next/server' import { ssoRegistrationContract } from '@/lib/api/contracts/auth' import { getValidationErrorMessage, parseRequest } from '@/lib/api/server' import { auth, getSession } from '@/lib/auth' +import { decryptProviderConfig } from '@/lib/auth/sso/provider-secrets' import { invalidateSsoPolicyCache } from '@/lib/auth/sso-policy' import { hasSSOAccess } from '@/lib/billing' import { isSsoEnabled } from '@/lib/core/config/env-flags' @@ -311,7 +312,8 @@ export const POST = withRouteHandler(async (request: NextRequest) => { ) } try { - clientSecret = JSON.parse(existing.oidcConfig).clientSecret + const stored = await decryptProviderConfig(existing.oidcConfig, 'oidcConfig') + clientSecret = JSON.parse(stored as string).clientSecret } catch { return NextResponse.json( { @@ -695,6 +697,12 @@ export const POST = withRouteHandler(async (request: NextRequest) => { }) if (existingOwnedProvider) { + /** + * Restores the columns exactly as they were read, including whatever + * encoding their secrets were stored in. Re-encrypting would wrap an + * already-encrypted value twice; decrypting would downgrade the row to + * plain text. + */ const revertProviderUpdate = async (): Promise => { await db .update(ssoProvider) diff --git a/apps/sim/lib/auth/sim-auth-adapter.ts b/apps/sim/lib/auth/sim-auth-adapter.ts index b98fe240bbf..23c8b374b61 100644 --- a/apps/sim/lib/auth/sim-auth-adapter.ts +++ b/apps/sim/lib/auth/sim-auth-adapter.ts @@ -7,6 +7,7 @@ import { type AuthDatabase, guardOAuthProviderWrites, } from '@/lib/auth/oauth-provider-adapter-guard' +import { encryptSsoProviderSecrets } from '@/lib/auth/sso-provider-secret-adapter' import { guardSubscriptionPlanWrites } from '@/lib/auth/stripe-adapter-guard' type BetterAuthAdapter = ReturnType> @@ -25,7 +26,9 @@ export function createSimAuthAdapter( schema, transaction: false, })(options) - const guarded = guardSubscriptionPlanWrites(guardOAuthProviderWrites(base, database)) + const guarded = guardSubscriptionPlanWrites( + guardOAuthProviderWrites(encryptSsoProviderSecrets(base), database) + ) if (inTransaction) return guarded guarded.transaction = (callback) => diff --git a/apps/sim/lib/auth/sso-provider-secret-adapter.postgres.test.ts b/apps/sim/lib/auth/sso-provider-secret-adapter.postgres.test.ts new file mode 100644 index 00000000000..ad97b226cec --- /dev/null +++ b/apps/sim/lib/auth/sso-provider-secret-adapter.postgres.test.ts @@ -0,0 +1,148 @@ +/** + * @vitest-environment node + */ +import { sso } from '@better-auth/sso' +import * as schema from '@sim/db/schema' +import { withUtcTimestamps } from '@sim/db/timestamps' +import { resetEnvMock, setEnv } from '@sim/testing' +import { generateId } from '@sim/utils/id' +import type { BetterAuthOptions } from 'better-auth' +import { drizzle } from 'drizzle-orm/postgres-js' +import postgres from 'postgres' +import { afterAll, beforeAll, describe, expect, it, vi } from 'vitest' +import { createSimAuthAdapter } from '@/lib/auth/sim-auth-adapter' + +vi.unmock('@sim/db/schema') +vi.unmock('drizzle-orm') + +/** The guard suites cover authorization; this exercises the secret encoding against the real table. */ +vi.mock('@/lib/auth/oauth-provider-adapter-guard', () => ({ + guardOAuthProviderWrites: (adapter: object) => adapter, +})) +vi.mock('@/lib/auth/stripe-adapter-guard', () => ({ + guardSubscriptionPlanWrites: (adapter: object) => adapter, +})) + +const OPTIONS: BetterAuthOptions = { plugins: [sso()] } +const databaseUrl = process.env.OAUTH_TOKEN_FAMILY_TEST_DATABASE_URL + +const CLIENT_SECRET = 'super-secret-value' +const oidcConfig = (clientSecret: string) => + JSON.stringify({ clientId: 'client', clientSecret, pkce: true }) + +describe.skipIf(!databaseUrl)('SSO provider secrets in PostgreSQL', () => { + const client = postgres( + databaseUrl ?? '', + withUtcTimestamps({ max: 1, prepare: false, fetch_types: false }) + ) + const database = drizzle(client, { schema }) + const adapter = createSimAuthAdapter(OPTIONS, database) + const userId = generateId() + + beforeAll(async () => { + /** The shared env mock's ENCRYPTION_KEY is not usable AES material. */ + setEnv({ ENCRYPTION_KEY: '0'.repeat(64) }) + await database.insert(schema.user).values({ + id: userId, + name: 'SSO Secret Owner', + email: `${userId}@example.test`, + emailVerified: true, + createdAt: new Date(), + updatedAt: new Date(), + }) + }) + + afterAll(async () => { + await client`DELETE FROM sso_provider WHERE user_id = ${userId}` + await client`DELETE FROM "user" WHERE id = ${userId}` + await client.end() + resetEnvMock() + }) + + async function storedConfig(providerId: string): Promise { + const [row] = await client<{ oidc_config: string }[]>` + SELECT oidc_config FROM sso_provider WHERE provider_id = ${providerId} + ` + return row.oidc_config + } + + /** Cleanup is by `user_id`, so each case just needs a distinct provider id. */ + function register(): string { + return `acme-${generateId()}` + } + + it('writes the client secret encrypted and reads it back usable', async () => { + const providerId = register() + + await adapter.create({ + model: 'ssoProvider', + data: { + id: generateId(), + providerId, + issuer: 'https://idp.example.test', + domain: 'example.test', + userId, + oidcConfig: oidcConfig(CLIENT_SECRET), + }, + forceAllowId: true, + }) + + const atRest = JSON.parse(await storedConfig(providerId)) + expect(atRest.clientSecret).not.toBe(CLIENT_SECRET) + expect(atRest.clientSecret).toMatch(/^[0-9a-f]{32}:[0-9a-f]+:[0-9a-f]{32}$/) + expect(atRest.clientId).toBe('client') + + const loaded = await adapter.findOne<{ oidcConfig: string }>({ + model: 'ssoProvider', + where: [{ field: 'providerId', value: providerId }], + }) + expect(JSON.parse(loaded!.oidcConfig).clientSecret).toBe(CLIENT_SECRET) + }) + + it('re-encrypts on update without double-wrapping the value it read', async () => { + const providerId = register() + await adapter.create({ + model: 'ssoProvider', + data: { + id: generateId(), + providerId, + issuer: 'https://idp.example.test', + domain: 'example.test', + userId, + oidcConfig: oidcConfig(CLIENT_SECRET), + }, + forceAllowId: true, + }) + + /** What Better Auth's own update merge does: read the row, write it back. */ + const current = await adapter.findOne<{ oidcConfig: string }>({ + model: 'ssoProvider', + where: [{ field: 'providerId', value: providerId }], + }) + const updated = await adapter.update<{ oidcConfig: string }>({ + model: 'ssoProvider', + where: [{ field: 'providerId', value: providerId }], + update: { oidcConfig: current!.oidcConfig }, + }) + + expect(JSON.parse(updated!.oidcConfig).clientSecret).toBe(CLIENT_SECRET) + const atRest = JSON.parse(await storedConfig(providerId)) + expect(atRest.clientSecret).toMatch(/^[0-9a-f]{32}:[0-9a-f]+:[0-9a-f]{32}$/) + }) + + it('reads a row written before the secret was encrypted', async () => { + const providerId = register() + await client` + INSERT INTO sso_provider (id, issuer, domain, oidc_config, user_id, provider_id) + VALUES (${generateId()}, 'https://idp.example.test', 'example.test', + ${oidcConfig(CLIENT_SECRET)}, ${userId}, ${providerId}) + ` + + const loaded = await adapter.findOne<{ oidcConfig: string }>({ + model: 'ssoProvider', + where: [{ field: 'providerId', value: providerId }], + }) + + expect(JSON.parse(loaded!.oidcConfig).clientSecret).toBe(CLIENT_SECRET) + }) +}) diff --git a/apps/sim/lib/auth/sso-provider-secret-adapter.test.ts b/apps/sim/lib/auth/sso-provider-secret-adapter.test.ts new file mode 100644 index 00000000000..f75bbb5fc2f --- /dev/null +++ b/apps/sim/lib/auth/sso-provider-secret-adapter.test.ts @@ -0,0 +1,173 @@ +/** + * @vitest-environment node + */ +import { beforeEach, describe, expect, it, vi } from 'vitest' + +const { mockEncryptSecret, mockDecryptSecret } = vi.hoisted(() => ({ + mockEncryptSecret: vi.fn(), + mockDecryptSecret: vi.fn(), +})) + +/** The shared env mock's ENCRYPTION_KEY is not 64 hex characters, so real crypto would throw. */ +vi.mock('@/lib/core/security/encryption', () => ({ + encryptSecret: mockEncryptSecret, + decryptSecret: mockDecryptSecret, +})) + +import { encryptSsoProviderSecrets } from '@/lib/auth/sso-provider-secret-adapter' + +const IV = 'a'.repeat(32) +const TAG = 'b'.repeat(32) +const sealed = (secret: string) => `${IV}:${Buffer.from(secret).toString('hex')}:${TAG}` + +const PLAIN_OIDC = JSON.stringify({ clientId: 'client', clientSecret: 'super-secret' }) +const SEALED_OIDC = JSON.stringify({ clientId: 'client', clientSecret: sealed('super-secret') }) + +function createBaseAdapter() { + return { + create: vi.fn(async (input: { data: unknown }) => input.data), + update: vi.fn(async (input: { update: unknown }) => input.update), + findOne: vi.fn(async () => ({ id: 'p1', oidcConfig: SEALED_OIDC })), + findMany: vi.fn(async () => [{ id: 'p1', oidcConfig: SEALED_OIDC }]), + consumeOne: vi.fn(async () => ({ id: 'p1', oidcConfig: SEALED_OIDC })), + incrementOne: vi.fn(async () => ({ id: 'p1', oidcConfig: SEALED_OIDC })), + transaction: vi.fn(async (callback: (trx: unknown) => Promise) => + callback(createBaseAdapter()) + ), + } +} + +// double-cast-allowed: test double implements only the adapter subset the decorator touches +const asAdapter = (adapter: ReturnType) => + encryptSsoProviderSecrets(adapter as unknown as Parameters[0]) + +describe('encryptSsoProviderSecrets', () => { + beforeEach(() => { + vi.clearAllMocks() + mockEncryptSecret.mockImplementation(async (secret: string) => ({ encrypted: sealed(secret) })) + mockDecryptSecret.mockImplementation(async (value: string) => ({ + decrypted: Buffer.from(value.split(':')[1], 'hex').toString('utf8'), + })) + }) + + it('encrypts the secret on create and returns the row decrypted', async () => { + const adapter = createBaseAdapter() + + const created = await asAdapter(adapter).create({ + model: 'ssoProvider', + data: { providerId: 'acme-okta', oidcConfig: PLAIN_OIDC }, + } as never) + + const written = adapter.create.mock.calls[0][0] as { data: { oidcConfig: string } } + expect(JSON.parse(written.data.oidcConfig).clientSecret).toBe(sealed('super-secret')) + expect(written.data.oidcConfig).not.toContain('super-secret') + expect(JSON.parse((created as { oidcConfig: string }).oidcConfig).clientSecret).toBe( + 'super-secret' + ) + }) + + it('encrypts the secret on update and returns the updated row decrypted', async () => { + const adapter = createBaseAdapter() + + const updated = await asAdapter(adapter).update({ + model: 'ssoProvider', + where: [{ field: 'providerId', value: 'acme-okta' }], + update: { oidcConfig: PLAIN_OIDC }, + } as never) + + const written = adapter.update.mock.calls[0][0] as { update: { oidcConfig: string } } + expect(written.update.oidcConfig).not.toContain('super-secret') + expect(JSON.parse((updated as { oidcConfig: string }).oidcConfig).clientSecret).toBe( + 'super-secret' + ) + }) + + it('encrypts SAML key material but not the certificate', async () => { + const adapter = createBaseAdapter() + + await asAdapter(adapter).create({ + model: 'ssoProvider', + data: { samlConfig: JSON.stringify({ cert: 'public-cert', privateKey: 'sp-key' }) }, + } as never) + + const written = adapter.create.mock.calls[0][0] as { data: { samlConfig: string } } + const parsed = JSON.parse(written.data.samlConfig) + expect(parsed.cert).toBe('public-cert') + expect(parsed.privateKey).toBe(sealed('sp-key')) + }) + + it.each(['findOne', 'consumeOne', 'incrementOne'] as const)( + 'decrypts the row returned by %s', + async (method) => { + const adapter = createBaseAdapter() + + const row = await asAdapter(adapter)[method]({ model: 'ssoProvider', where: [] } as never) + + expect(JSON.parse((row as { oidcConfig: string }).oidcConfig).clientSecret).toBe( + 'super-secret' + ) + } + ) + + it('decrypts every row returned by findMany', async () => { + const adapter = createBaseAdapter() + + const rows = await asAdapter(adapter).findMany({ model: 'ssoProvider' } as never) + + expect(JSON.parse((rows as { oidcConfig: string }[])[0].oidcConfig).clientSecret).toBe( + 'super-secret' + ) + }) + + it('returns a value written before encryption existed unchanged', async () => { + const adapter = createBaseAdapter() + adapter.findOne.mockResolvedValue({ id: 'p1', oidcConfig: PLAIN_OIDC }) + + const row = await asAdapter(adapter).findOne({ model: 'ssoProvider', where: [] } as never) + + expect((row as { oidcConfig: string }).oidcConfig).toBe(PLAIN_OIDC) + expect(mockDecryptSecret).not.toHaveBeenCalled() + }) + + it.each([ + ['a row without the config columns', { id: 'p1' }], + ['a null column', { id: 'p1', oidcConfig: null }], + ['no row at all', null], + ])('tolerates %s', async (_label, stored) => { + const adapter = createBaseAdapter() + adapter.findOne.mockResolvedValue(stored as never) + + await expect( + asAdapter(adapter).findOne({ model: 'ssoProvider', where: [] } as never) + ).resolves.toEqual(stored) + }) + + it('leaves writes and reads on other models untouched', async () => { + const adapter = createBaseAdapter() + const guarded = asAdapter(adapter) + + await guarded.create({ model: 'user', data: { oidcConfig: PLAIN_OIDC } } as never) + await guarded.findOne({ model: 'user', where: [] } as never) + + const written = adapter.create.mock.calls[0][0] as { data: { oidcConfig: string } } + expect(written.data.oidcConfig).toBe(PLAIN_OIDC) + expect(mockEncryptSecret).not.toHaveBeenCalled() + expect(mockDecryptSecret).not.toHaveBeenCalled() + }) + + it('applies the same encoding to writes inside a transaction', async () => { + const inner = createBaseAdapter() + const adapter = createBaseAdapter() + adapter.transaction.mockImplementation(async (callback) => callback(inner)) + + await asAdapter(adapter).transaction(async (trx) => + (trx as ReturnType).create({ + model: 'ssoProvider', + data: { oidcConfig: PLAIN_OIDC }, + } as never) + ) + + const written = inner.create.mock.calls[0][0] as { data: { oidcConfig: string } } + expect(written.data.oidcConfig).not.toContain('super-secret') + }) +}) diff --git a/apps/sim/lib/auth/sso-provider-secret-adapter.ts b/apps/sim/lib/auth/sso-provider-secret-adapter.ts new file mode 100644 index 00000000000..13516cedf80 --- /dev/null +++ b/apps/sim/lib/auth/sso-provider-secret-adapter.ts @@ -0,0 +1,123 @@ +import type { drizzleAdapter } from 'better-auth/adapters/drizzle' +import { + decryptProviderConfig, + encryptProviderConfig, + type SsoConfigColumn, +} from '@/lib/auth/sso/provider-secrets' + +type BetterAuthAdapter = ReturnType> + +const SSO_PROVIDER_MODEL = 'ssoProvider' +const SECRET_COLUMNS: readonly SsoConfigColumn[] = ['oidcConfig', 'samlConfig'] + +/** + * Encrypts the secrets inside `sso_provider.oidc_config` / `saml_config` at the + * database adapter, and decrypts them on the way back out. + * + * The adapter is the only seam that covers both directions. Better Auth's SSO + * plugin owns the sign-in path end to end: it loads the provider row itself and + * passes `oidcConfig.clientSecret` straight into the authorization URL and the + * token exchange, so encrypting writes anywhere else would break every sign-in. + * Registration, the plugin's own update merge, and sign-in all pass through + * here, which keeps one encoding for all of them. + * + * Only the secret fields are encrypted, not the whole column, so the JSON stays + * parseable for the readers that want the public parts of the config. Reads + * tolerate values stored before this existed; see `provider-secrets.ts`. + * + * Sim also reads and writes these columns directly with Drizzle in a few places + * that bypass this adapter — the register route's reuse and rollback branches, + * the providers list, and the operator registration script — and each of those + * handles the envelope itself. + * + * Transactions are wrapped recursively so writes inside `adapter.transaction(...)` + * callbacks go through the same encoding. + */ +export function encryptSsoProviderSecrets(adapter: BetterAuthAdapter): BetterAuthAdapter { + const guarded: BetterAuthAdapter = { + ...adapter, + ...encryptSecretSurface(adapter), + } + + const transaction = adapter.transaction + if (typeof transaction === 'function') { + guarded.transaction = (callback) => + transaction((trx) => callback({ ...trx, ...encryptSecretSurface(trx) })) + } + + return guarded +} + +type SecretSurface = Pick< + BetterAuthAdapter, + 'create' | 'update' | 'findOne' | 'findMany' | 'consumeOne' | 'incrementOne' +> + +function encryptSecretSurface(adapter: TAdapter): SecretSurface { + return { + create: async (input) => { + if (input.model !== SSO_PROVIDER_MODEL) return adapter.create(input) + const data = await encryptColumns(input.data as Record) + return decryptRow(await adapter.create({ ...input, data: data as never })) as never + }, + update: async (input) => { + if (input.model !== SSO_PROVIDER_MODEL) return adapter.update(input) + const update = await encryptColumns(input.update as Record) + return decryptRow(await adapter.update({ ...input, update: update as never })) as never + }, + findOne: async (input) => { + const row = await adapter.findOne(input) + return (input.model === SSO_PROVIDER_MODEL ? await decryptRow(row) : row) as never + }, + findMany: async (input) => { + const rows = await adapter.findMany(input) + if (input.model !== SSO_PROVIDER_MODEL || !Array.isArray(rows)) return rows as never + return (await Promise.all(rows.map((row) => decryptRow(row)))) as never + }, + consumeOne: async (input) => { + const row = await adapter.consumeOne(input) + return (input.model === SSO_PROVIDER_MODEL ? await decryptRow(row) : row) as never + }, + incrementOne: async (input) => { + const row = await adapter.incrementOne(input) + return (input.model === SSO_PROVIDER_MODEL ? await decryptRow(row) : row) as never + }, + } +} + +/** + * Rewrites only the secret columns a write actually carries. A payload that + * omits them — the plugin marking a domain verified, say — is left untouched. + */ +async function encryptColumns(payload: Record): Promise> { + let next: Record | null = null + for (const column of SECRET_COLUMNS) { + const value = payload[column] + if (typeof value !== 'string') continue + const encrypted = await encryptProviderConfig(value, column) + if (encrypted === value) continue + next ??= { ...payload } + next[column] = encrypted + } + return next ?? payload +} + +/** + * Decrypts the secret columns of a returned row. Rows are also returned by + * `update`, `consumeOne` and `incrementOne`, and a `select` projection can omit + * the columns entirely, so every shape has to be tolerated. + */ +async function decryptRow(row: TRow): Promise { + if (!row || typeof row !== 'object' || Array.isArray(row)) return row + const record = row as Record + let next: Record | null = null + for (const column of SECRET_COLUMNS) { + const value = record[column] + if (typeof value !== 'string') continue + const decrypted = await decryptProviderConfig(value, column) + if (decrypted === value) continue + next ??= { ...record } + next[column] = decrypted + } + return (next ?? row) as TRow +} diff --git a/apps/sim/lib/auth/sso/provider-secrets.test.ts b/apps/sim/lib/auth/sso/provider-secrets.test.ts new file mode 100644 index 00000000000..376f58ac8d2 --- /dev/null +++ b/apps/sim/lib/auth/sso/provider-secrets.test.ts @@ -0,0 +1,112 @@ +/** + * @vitest-environment node + */ +import { beforeEach, describe, expect, it, vi } from 'vitest' + +const { mockEncryptSecret, mockDecryptSecret } = vi.hoisted(() => ({ + mockEncryptSecret: vi.fn(), + mockDecryptSecret: vi.fn(), +})) + +/** The shared env mock's ENCRYPTION_KEY is not 64 hex characters, so real crypto would throw. */ +vi.mock('@/lib/core/security/encryption', () => ({ + encryptSecret: mockEncryptSecret, + decryptSecret: mockDecryptSecret, +})) + +import { decryptProviderConfig, encryptProviderConfig } from '@/lib/auth/sso/provider-secrets' + +const IV = 'a'.repeat(32) +const TAG = 'b'.repeat(32) +const envelope = (ciphertext: string) => `${IV}:${ciphertext}:${TAG}` + +describe('provider secrets', () => { + beforeEach(() => { + vi.clearAllMocks() + mockEncryptSecret.mockImplementation(async (secret: string) => ({ + encrypted: envelope(Buffer.from(secret).toString('hex')), + })) + mockDecryptSecret.mockImplementation(async (value: string) => ({ + decrypted: Buffer.from(value.split(':')[1], 'hex').toString('utf8'), + })) + }) + + it('encrypts only the client secret and leaves the rest of the config readable', async () => { + const stored = await encryptProviderConfig( + JSON.stringify({ clientId: 'client', clientSecret: 'super-secret', pkce: true }), + 'oidcConfig' + ) + + const parsed = JSON.parse(stored as string) + expect(parsed.clientSecret).toBe(envelope(Buffer.from('super-secret').toString('hex'))) + expect(parsed).toMatchObject({ clientId: 'client', pkce: true }) + expect(stored).not.toContain('super-secret') + }) + + it('round-trips a config through encrypt and decrypt', async () => { + const config = JSON.stringify({ clientId: 'client', clientSecret: 'super-secret' }) + + const stored = await encryptProviderConfig(config, 'oidcConfig') + const loaded = await decryptProviderConfig(stored, 'oidcConfig') + + expect(JSON.parse(loaded as string)).toEqual({ + clientId: 'client', + clientSecret: 'super-secret', + }) + }) + + it('encrypts both SAML key fields and ignores the public certificate', async () => { + const stored = await encryptProviderConfig( + JSON.stringify({ cert: 'public-cert', privateKey: 'sp-key', decryptionPvk: 'pvk' }), + 'samlConfig' + ) + + const parsed = JSON.parse(stored as string) + expect(parsed.cert).toBe('public-cert') + expect(parsed.privateKey).toBe(envelope(Buffer.from('sp-key').toString('hex'))) + expect(parsed.decryptionPvk).toBe(envelope(Buffer.from('pvk').toString('hex'))) + }) + + it('leaves a value stored before encryption existed untouched', async () => { + const legacy = JSON.stringify({ clientId: 'client', clientSecret: 'plain-text-secret' }) + + await expect(decryptProviderConfig(legacy, 'oidcConfig')).resolves.toBe(legacy) + expect(mockDecryptSecret).not.toHaveBeenCalled() + }) + + it('does not encrypt a value that is already encrypted', async () => { + const stored = JSON.stringify({ clientSecret: envelope('deadbeef') }) + + await expect(encryptProviderConfig(stored, 'oidcConfig')).resolves.toBe(stored) + expect(mockEncryptSecret).not.toHaveBeenCalled() + }) + + it.each([ + ['a null column', null], + ['an empty column', ''], + ['an undefined column', undefined], + ])('passes through %s', async (_label, value) => { + await expect(encryptProviderConfig(value, 'oidcConfig')).resolves.toBe(value) + await expect(decryptProviderConfig(value, 'oidcConfig')).resolves.toBe(value) + }) + + it.each([ + ['a config without the secret field', JSON.stringify({ clientId: 'client' })], + ['a config whose secret is empty', JSON.stringify({ clientSecret: '' })], + ['a config whose secret is not a string', JSON.stringify({ clientSecret: { a: 1 } })], + ['a value that is not JSON', 'not json at all'], + ['a JSON array', '[1,2,3]'], + ])('passes through %s unchanged', async (_label, value) => { + await expect(encryptProviderConfig(value, 'oidcConfig')).resolves.toBe(value) + await expect(decryptProviderConfig(value, 'oidcConfig')).resolves.toBe(value) + expect(mockEncryptSecret).not.toHaveBeenCalled() + }) + + it('fails loudly when a stored secret cannot be decrypted', async () => { + mockDecryptSecret.mockRejectedValue(new Error('auth tag mismatch')) + + await expect( + decryptProviderConfig(JSON.stringify({ clientSecret: envelope('deadbeef') }), 'oidcConfig') + ).rejects.toThrow(/different ENCRYPTION_KEY/) + }) +}) diff --git a/apps/sim/lib/auth/sso/provider-secrets.ts b/apps/sim/lib/auth/sso/provider-secrets.ts new file mode 100644 index 00000000000..4e6fbf9482f --- /dev/null +++ b/apps/sim/lib/auth/sso/provider-secrets.ts @@ -0,0 +1,113 @@ +import { createLogger } from '@sim/logger' +import { toError } from '@sim/utils/errors' +import { decryptSecret, encryptSecret } from '@/lib/core/security/encryption' + +const logger = createLogger('SsoProviderSecrets') + +/** + * The secret-bearing fields inside each provider config column. Everything else + * an IdP config carries — endpoints, scopes, the SAML certificate, attribute + * mapping — is public configuration and stays readable, so the column remains + * ordinary JSON for the readers that only need those. + */ +const SECRET_FIELDS = { + oidcConfig: ['clientSecret'], + samlConfig: ['privateKey', 'decryptionPvk'], +} as const + +export type SsoConfigColumn = keyof typeof SECRET_FIELDS + +/** + * The shape {@link encryptSecret} produces: a 16-byte IV and a 16-byte GCM auth + * tag around hex ciphertext. Matching it exactly is what lets a value written + * before these fields were encrypted be recognized as legacy plain text and + * returned unchanged — the same tolerance `decryptApiKey` gives API keys. + */ +const ENVELOPE = /^[0-9a-f]{32}:[0-9a-f]+:[0-9a-f]{32}$/ + +function isEnvelope(value: string): boolean { + return ENVELOPE.test(value) +} + +function parseConfig(config: string): Record | null { + try { + const parsed: unknown = JSON.parse(config) + if (!parsed || typeof parsed !== 'object' || Array.isArray(parsed)) return null + return parsed as Record + } catch { + return null + } +} + +/** + * Maps the secret fields of one provider config through `transform`, leaving the + * rest of the JSON byte-identical in field order. A column that is null, not an + * object, or not JSON at all is returned unchanged: this runs on every read of + * every provider row, and a config Sim does not recognize is the storage + * layer's to pass through, not to reject. + */ +async function mapSecretFields( + config: string | null | undefined, + column: SsoConfigColumn, + transform: (value: string) => Promise +): Promise { + if (!config) return config + const parsed = parseConfig(config) + if (!parsed) return config + + let changed = false + for (const field of SECRET_FIELDS[column]) { + const value = parsed[field] + if (typeof value !== 'string' || value === '') continue + const next = await transform(value) + if (next === value) continue + parsed[field] = next + changed = true + } + + return changed ? JSON.stringify(parsed) : config +} + +/** + * Encrypts the secret fields of a provider config for storage. Already-encrypted + * values are left alone, so a config that Better Auth merged from a stored row + * (`mergeOIDCConfig` carries the existing `clientSecret` forward when an update + * omits it) is never wrapped twice. + */ +export function encryptProviderConfig( + config: string | null | undefined, + column: SsoConfigColumn +): Promise { + return mapSecretFields(config, column, async (value) => + isEnvelope(value) ? value : (await encryptSecret(value)).encrypted + ) +} + +/** + * Decrypts the secret fields of a stored provider config. Values written before + * these fields were encrypted lack the envelope shape and are returned as-is. + * + * A value that IS an envelope but fails to decrypt — a wrong or rotated + * `ENCRYPTION_KEY`, a tampered row — throws rather than degrading to ciphertext. + * Handing ciphertext to an IdP as a client secret would fail the token exchange + * with an opaque `invalid_client`; failing here names the real cause. + */ +export function decryptProviderConfig( + config: string | null | undefined, + column: SsoConfigColumn +): Promise { + return mapSecretFields(config, column, async (value) => { + if (!isEnvelope(value)) return value + try { + return (await decryptSecret(value, { logFailure: false })).decrypted + } catch (error) { + logger.error('Failed to decrypt an SSO provider secret', { + column, + error: toError(error).message, + }) + throw new Error( + `Could not decrypt the stored ${column} secret. It was encrypted with a different ENCRYPTION_KEY.` + ) + } + }) +} diff --git a/packages/db/scripts/register-sso-provider.ts b/packages/db/scripts/register-sso-provider.ts index ef4f2c12694..27afb033840 100644 --- a/packages/db/scripts/register-sso-provider.ts +++ b/packages/db/scripts/register-sso-provider.ts @@ -10,6 +10,8 @@ * Usage: bun run packages/db/scripts/register-sso-provider.ts * * Required Environment Variables: + * ENCRYPTION_KEY= (provider secrets are stored encrypted + * with it; a different key makes them unreadable at sign-in) * SSO_ENABLED=true * SSO_PROVIDER_TYPE=oidc|saml * SSO_PROVIDER_ID=your-provider-id @@ -38,6 +40,7 @@ * SSO_SAML_WANT_ASSERTIONS_SIGNED=true (optional, defaults to false) */ +import { createCipheriv, createDecipheriv, randomBytes } from 'node:crypto' import { getErrorMessage } from '@sim/utils/errors' import { generateId } from '@sim/utils/id' import { normalizeSSODomain } from '@sim/utils/sso-domain' @@ -147,6 +150,51 @@ if (!CONNECTION_STRING) { process.exit(1) } +const ENCRYPTION_KEY = process.env.ENCRYPTION_KEY +if (!ENCRYPTION_KEY || !/^[0-9a-f]{64}$/i.test(ENCRYPTION_KEY)) { + console.error( + '❌ ENCRYPTION_KEY must be set to the 64-character hex key the app uses; provider secrets are stored encrypted with it' + ) + process.exit(1) +} + +const ENCRYPTION_KEY_BUFFER = Buffer.from(ENCRYPTION_KEY, 'hex') + +/** + * AES-256-GCM in the `iv:ciphertext:authTag` envelope the app reads back. This + * package cannot import from `apps/*`, so the primitive is repeated here rather + * than shared; {@link assertCryptoRoundTrip} proves the key produces a readable + * value before any row is written. + */ +function encryptSecretValue(secret: string): string { + const iv = randomBytes(16) + const cipher = createCipheriv('aes-256-gcm', ENCRYPTION_KEY_BUFFER, iv, { authTagLength: 16 }) + let encrypted = cipher.update(secret, 'utf8', 'hex') + encrypted += cipher.final('hex') + return `${iv.toString('hex')}:${encrypted}:${cipher.getAuthTag().toString('hex')}` +} + +function decryptSecretValue(envelope: string): string { + const [ivHex, ciphertext, authTagHex] = envelope.split(':') + const decipher = createDecipheriv( + 'aes-256-gcm', + ENCRYPTION_KEY_BUFFER, + Buffer.from(ivHex, 'hex'), + { + authTagLength: 16, + } + ) + decipher.setAuthTag(Buffer.from(authTagHex, 'hex')) + return decipher.update(ciphertext, 'hex', 'utf8') + decipher.final('utf8') +} + +function assertCryptoRoundTrip(): void { + const sample = 'sso-provider-registration-self-test' + if (decryptSecretValue(encryptSecretValue(sample)) !== sample) { + throw new Error('Crypto self-test failed; refusing to write provider secrets') + } +} + const postgresClient = postgres(CONNECTION_STRING, { prepare: false, idle_timeout: 20, @@ -590,7 +638,7 @@ async function registerSSOProvider(): Promise { const oidcConfig = { issuer: ssoConfig.issuer, clientId: ssoConfig.oidcConfig.clientId, - clientSecret: ssoConfig.oidcConfig.clientSecret, + clientSecret: encryptSecretValue(ssoConfig.oidcConfig.clientSecret), authorizationEndpoint: ssoConfig.oidcConfig.authorizationEndpoint, tokenEndpoint: ssoConfig.oidcConfig.tokenEndpoint, // Default to client_secret_post: better-auth sends client_secret_basic @@ -624,8 +672,12 @@ async function registerSSOProvider(): Promise { signatureAlgorithm: ssoConfig.samlConfig.signatureAlgorithm, digestAlgorithm: ssoConfig.samlConfig.digestAlgorithm, identifierFormat: ssoConfig.samlConfig.identifierFormat, - privateKey: ssoConfig.samlConfig.privateKey, - decryptionPvk: ssoConfig.samlConfig.decryptionPvk, + privateKey: ssoConfig.samlConfig.privateKey + ? encryptSecretValue(ssoConfig.samlConfig.privateKey) + : undefined, + decryptionPvk: ssoConfig.samlConfig.decryptionPvk + ? encryptSecretValue(ssoConfig.samlConfig.decryptionPvk) + : undefined, additionalParams: ssoConfig.samlConfig.additionalParams, mapping: ssoConfig.mapping, } @@ -762,6 +814,10 @@ async function main() { console.log('This script directly inserts SSO provider records into the database.') console.log("It follows Better Auth's exact registerSSOProvider logic.\n") + // Before any row is written: a key that cannot round-trip would store secrets + // the app can never read back, and SSO would fail only at the first sign-in. + assertCryptoRoundTrip() + const success = await registerSSOProvider() if (success) { From bdb8785c9daa8322d9cdf7f88f4dd5447a172bf5 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Sat, 19 Sep 2026 18:03:22 -0700 Subject: [PATCH 2/9] fix(sso): mark encrypted provider secrets with an explicit prefix Detecting ciphertext by its iv:ciphertext:authTag shape was ambiguous: a client secret is an arbitrary string chosen at the identity provider, so one shaped like an envelope would have been read back as ciphertext and broken that provider. Encrypted values now carry a versioned prefix. The providers list also lets a decryption failure surface instead of reporting the provider as having no config, and the helper moved next to the adapter that uses it. --- .../app/api/auth/sso/providers/route.test.ts | 12 +++++++++- apps/sim/app/api/auth/sso/providers/route.ts | 10 ++++++-- .../app/api/auth/sso/register/route.test.ts | 2 +- apps/sim/app/api/auth/sso/register/route.ts | 2 +- ...o-provider-secret-adapter.postgres.test.ts | 4 ++-- .../auth/sso-provider-secret-adapter.test.ts | 6 +++-- .../lib/auth/sso-provider-secret-adapter.ts | 2 +- ...s.test.ts => sso-provider-secrets.test.ts} | 22 ++++++++++++++--- ...der-secrets.ts => sso-provider-secrets.ts} | 24 ++++++++++++------- packages/db/scripts/register-sso-provider.ts | 11 ++++++--- 10 files changed, 70 insertions(+), 25 deletions(-) rename apps/sim/lib/auth/{sso/provider-secrets.test.ts => sso-provider-secrets.test.ts} (81%) rename apps/sim/lib/auth/{sso/provider-secrets.ts => sso-provider-secrets.ts} (78%) diff --git a/apps/sim/app/api/auth/sso/providers/route.test.ts b/apps/sim/app/api/auth/sso/providers/route.test.ts index eb3b8186934..3c326dae1dd 100644 --- a/apps/sim/app/api/auth/sso/providers/route.test.ts +++ b/apps/sim/app/api/auth/sso/providers/route.test.ts @@ -28,7 +28,7 @@ import { GET } from '@/app/api/auth/sso/providers/route' const IV = 'a'.repeat(32) const TAG = 'b'.repeat(32) -const sealed = (secret: string) => `${IV}:${Buffer.from(secret).toString('hex')}:${TAG}` +const sealed = (secret: string) => `sim.sso.v1:${IV}:${Buffer.from(secret).toString('hex')}:${TAG}` const CLIENT_SECRET = 'a-long-client-secret-wxyz' @@ -94,6 +94,16 @@ describe('GET /api/auth/sso/providers', () => { expect(mockDecryptSecret).not.toHaveBeenCalled() }) + it('fails the request when a stored secret cannot be decrypted', async () => { + mockDecryptSecret.mockRejectedValue(new Error('auth tag mismatch')) + queueTableRows(schemaMock.ssoProvider, [providerRow]) + + const res = await GET(createMockRequest('GET')) + + /** Reporting a key problem as a provider with no config would hide it behind a 200. */ + expect(res.status).toBe(500) + }) + it('redacts SAML key material and keeps the certificate', async () => { queueTableRows(schemaMock.ssoProvider, [ { diff --git a/apps/sim/app/api/auth/sso/providers/route.ts b/apps/sim/app/api/auth/sso/providers/route.ts index 7fbce82a370..b930f7f6c8a 100644 --- a/apps/sim/app/api/auth/sso/providers/route.ts +++ b/apps/sim/app/api/auth/sso/providers/route.ts @@ -11,7 +11,7 @@ import { listSsoProvidersContract } from '@/lib/api/contracts/auth' import { parseRequest } from '@/lib/api/server' import { getSession } from '@/lib/auth' import { markSignInProviders } from '@/lib/auth/sso/primary-provider' -import { decryptProviderConfig } from '@/lib/auth/sso/provider-secrets' +import { decryptProviderConfig } from '@/lib/auth/sso-provider-secrets' import { REDACTED_MARKER } from '@/lib/core/security/redaction' import { withRouteHandler } from '@/lib/core/utils/with-route-handler' @@ -40,8 +40,14 @@ function buildClientSecretHint(clientSecret: unknown): string | null { */ async function redactOidcConfig(oidcConfig: string | null): Promise { if (!oidcConfig) return oidcConfig + /** + * Outside the catch: a config that will not decrypt is a key problem, and + * reporting it as a provider with no config would hide it behind a healthy + * 200. Unreadable JSON stays tolerated below, as it was before. + */ + const decrypted = await decryptProviderConfig(oidcConfig, 'oidcConfig') try { - const parsed = JSON.parse((await decryptProviderConfig(oidcConfig, 'oidcConfig')) as string) + const parsed = JSON.parse(decrypted as string) const hint = buildClientSecretHint(parsed.clientSecret) parsed.clientSecret = REDACTED_MARKER if (hint) parsed.clientSecretHint = hint diff --git a/apps/sim/app/api/auth/sso/register/route.test.ts b/apps/sim/app/api/auth/sso/register/route.test.ts index 058107b07d9..39ee56497d5 100644 --- a/apps/sim/app/api/auth/sso/register/route.test.ts +++ b/apps/sim/app/api/auth/sso/register/route.test.ts @@ -305,7 +305,7 @@ describe('POST /api/auth/sso/register', () => { * directly rather than through Better Auth, so it decrypts it itself. */ it('reuses the stored client secret, decrypting it first', async () => { - const sealed = `${'a'.repeat(32)}:${Buffer.from('stored-secret').toString('hex')}:${'b'.repeat(32)}` + const sealed = `sim.sso.v1:${'a'.repeat(32)}:${Buffer.from('stored-secret').toString('hex')}:${'b'.repeat(32)}` queueMembers([{ organizationId: 'org1', role: 'owner' }]) // In route order: providerId conflict and domain refusal, the reuse read, // both checks again before the write, then the pre-image being updated. diff --git a/apps/sim/app/api/auth/sso/register/route.ts b/apps/sim/app/api/auth/sso/register/route.ts index 7765720eaaf..defe1c420a2 100644 --- a/apps/sim/app/api/auth/sso/register/route.ts +++ b/apps/sim/app/api/auth/sso/register/route.ts @@ -8,8 +8,8 @@ import { type NextRequest, NextResponse } from 'next/server' import { ssoRegistrationContract } from '@/lib/api/contracts/auth' import { getValidationErrorMessage, parseRequest } from '@/lib/api/server' import { auth, getSession } from '@/lib/auth' -import { decryptProviderConfig } from '@/lib/auth/sso/provider-secrets' import { invalidateSsoPolicyCache } from '@/lib/auth/sso-policy' +import { decryptProviderConfig } from '@/lib/auth/sso-provider-secrets' import { hasSSOAccess } from '@/lib/billing' import { isSsoEnabled } from '@/lib/core/config/env-flags' import { runWithOutboundOrganization } from '@/lib/core/network/context.server' diff --git a/apps/sim/lib/auth/sso-provider-secret-adapter.postgres.test.ts b/apps/sim/lib/auth/sso-provider-secret-adapter.postgres.test.ts index ad97b226cec..6c7671dc50b 100644 --- a/apps/sim/lib/auth/sso-provider-secret-adapter.postgres.test.ts +++ b/apps/sim/lib/auth/sso-provider-secret-adapter.postgres.test.ts @@ -89,7 +89,7 @@ describe.skipIf(!databaseUrl)('SSO provider secrets in PostgreSQL', () => { const atRest = JSON.parse(await storedConfig(providerId)) expect(atRest.clientSecret).not.toBe(CLIENT_SECRET) - expect(atRest.clientSecret).toMatch(/^[0-9a-f]{32}:[0-9a-f]+:[0-9a-f]{32}$/) + expect(atRest.clientSecret).toMatch(/^sim\.sso\.v1:[0-9a-f]{32}:[0-9a-f]+:[0-9a-f]{32}$/) expect(atRest.clientId).toBe('client') const loaded = await adapter.findOne<{ oidcConfig: string }>({ @@ -127,7 +127,7 @@ describe.skipIf(!databaseUrl)('SSO provider secrets in PostgreSQL', () => { expect(JSON.parse(updated!.oidcConfig).clientSecret).toBe(CLIENT_SECRET) const atRest = JSON.parse(await storedConfig(providerId)) - expect(atRest.clientSecret).toMatch(/^[0-9a-f]{32}:[0-9a-f]+:[0-9a-f]{32}$/) + expect(atRest.clientSecret).toMatch(/^sim\.sso\.v1:[0-9a-f]{32}:[0-9a-f]+:[0-9a-f]{32}$/) }) it('reads a row written before the secret was encrypted', async () => { diff --git a/apps/sim/lib/auth/sso-provider-secret-adapter.test.ts b/apps/sim/lib/auth/sso-provider-secret-adapter.test.ts index f75bbb5fc2f..0b3bb7d0fbe 100644 --- a/apps/sim/lib/auth/sso-provider-secret-adapter.test.ts +++ b/apps/sim/lib/auth/sso-provider-secret-adapter.test.ts @@ -18,7 +18,9 @@ import { encryptSsoProviderSecrets } from '@/lib/auth/sso-provider-secret-adapte const IV = 'a'.repeat(32) const TAG = 'b'.repeat(32) -const sealed = (secret: string) => `${IV}:${Buffer.from(secret).toString('hex')}:${TAG}` +/** What `encryptSecret` returns; `provider-secrets` adds the prefix around it. */ +const raw = (secret: string) => `${IV}:${Buffer.from(secret).toString('hex')}:${TAG}` +const sealed = (secret: string) => `sim.sso.v1:${raw(secret)}` const PLAIN_OIDC = JSON.stringify({ clientId: 'client', clientSecret: 'super-secret' }) const SEALED_OIDC = JSON.stringify({ clientId: 'client', clientSecret: sealed('super-secret') }) @@ -44,7 +46,7 @@ const asAdapter = (adapter: ReturnType) => describe('encryptSsoProviderSecrets', () => { beforeEach(() => { vi.clearAllMocks() - mockEncryptSecret.mockImplementation(async (secret: string) => ({ encrypted: sealed(secret) })) + mockEncryptSecret.mockImplementation(async (secret: string) => ({ encrypted: raw(secret) })) mockDecryptSecret.mockImplementation(async (value: string) => ({ decrypted: Buffer.from(value.split(':')[1], 'hex').toString('utf8'), })) diff --git a/apps/sim/lib/auth/sso-provider-secret-adapter.ts b/apps/sim/lib/auth/sso-provider-secret-adapter.ts index 13516cedf80..b6f5b65bef1 100644 --- a/apps/sim/lib/auth/sso-provider-secret-adapter.ts +++ b/apps/sim/lib/auth/sso-provider-secret-adapter.ts @@ -3,7 +3,7 @@ import { decryptProviderConfig, encryptProviderConfig, type SsoConfigColumn, -} from '@/lib/auth/sso/provider-secrets' +} from '@/lib/auth/sso-provider-secrets' type BetterAuthAdapter = ReturnType> diff --git a/apps/sim/lib/auth/sso/provider-secrets.test.ts b/apps/sim/lib/auth/sso-provider-secrets.test.ts similarity index 81% rename from apps/sim/lib/auth/sso/provider-secrets.test.ts rename to apps/sim/lib/auth/sso-provider-secrets.test.ts index 376f58ac8d2..70b5aa06307 100644 --- a/apps/sim/lib/auth/sso/provider-secrets.test.ts +++ b/apps/sim/lib/auth/sso-provider-secrets.test.ts @@ -14,17 +14,19 @@ vi.mock('@/lib/core/security/encryption', () => ({ decryptSecret: mockDecryptSecret, })) -import { decryptProviderConfig, encryptProviderConfig } from '@/lib/auth/sso/provider-secrets' +import { decryptProviderConfig, encryptProviderConfig } from '@/lib/auth/sso-provider-secrets' const IV = 'a'.repeat(32) const TAG = 'b'.repeat(32) -const envelope = (ciphertext: string) => `${IV}:${ciphertext}:${TAG}` +/** What `encryptSecret` returns; the module under test adds the prefix. */ +const raw = (ciphertext: string) => `${IV}:${ciphertext}:${TAG}` +const envelope = (ciphertext: string) => `sim.sso.v1:${raw(ciphertext)}` describe('provider secrets', () => { beforeEach(() => { vi.clearAllMocks() mockEncryptSecret.mockImplementation(async (secret: string) => ({ - encrypted: envelope(Buffer.from(secret).toString('hex')), + encrypted: raw(Buffer.from(secret).toString('hex')), })) mockDecryptSecret.mockImplementation(async (value: string) => ({ decrypted: Buffer.from(value.split(':')[1], 'hex').toString('utf8'), @@ -74,6 +76,20 @@ describe('provider secrets', () => { expect(mockDecryptSecret).not.toHaveBeenCalled() }) + it('treats a legacy secret shaped like an envelope as plain text', async () => { + /** A client secret is chosen at the identity provider and can be any string. */ + const lookalike = `${'a'.repeat(32)}:${'c'.repeat(16)}:${'b'.repeat(32)}` + const legacy = JSON.stringify({ clientSecret: lookalike }) + + await expect(decryptProviderConfig(legacy, 'oidcConfig')).resolves.toBe(legacy) + expect(mockDecryptSecret).not.toHaveBeenCalled() + + const stored = await encryptProviderConfig(legacy, 'oidcConfig') + expect(JSON.parse(stored as string).clientSecret).toBe( + envelope(Buffer.from(lookalike).toString('hex')) + ) + }) + it('does not encrypt a value that is already encrypted', async () => { const stored = JSON.stringify({ clientSecret: envelope('deadbeef') }) diff --git a/apps/sim/lib/auth/sso/provider-secrets.ts b/apps/sim/lib/auth/sso-provider-secrets.ts similarity index 78% rename from apps/sim/lib/auth/sso/provider-secrets.ts rename to apps/sim/lib/auth/sso-provider-secrets.ts index 4e6fbf9482f..84274196533 100644 --- a/apps/sim/lib/auth/sso/provider-secrets.ts +++ b/apps/sim/lib/auth/sso-provider-secrets.ts @@ -18,15 +18,20 @@ const SECRET_FIELDS = { export type SsoConfigColumn = keyof typeof SECRET_FIELDS /** - * The shape {@link encryptSecret} produces: a 16-byte IV and a 16-byte GCM auth - * tag around hex ciphertext. Matching it exactly is what lets a value written - * before these fields were encrypted be recognized as legacy plain text and - * returned unchanged — the same tolerance `decryptApiKey` gives API keys. + * Marks a value this module encrypted. An explicit prefix, rather than matching + * the `iv:ciphertext:authTag` shape, is what makes the distinction unambiguous: + * a client secret is an arbitrary string chosen at the identity provider, and + * one that happened to look like an envelope would otherwise be read back as + * ciphertext and fail to decrypt. Values without the prefix were stored before + * these fields were encrypted and are passed through unchanged, the tolerance + * `decryptApiKey` gives API keys. + * + * The version lets a future encoding change be told apart from this one. */ -const ENVELOPE = /^[0-9a-f]{32}:[0-9a-f]+:[0-9a-f]{32}$/ +const ENVELOPE_PREFIX = 'sim.sso.v1:' function isEnvelope(value: string): boolean { - return ENVELOPE.test(value) + return value.startsWith(ENVELOPE_PREFIX) } function parseConfig(config: string): Record | null { @@ -79,13 +84,13 @@ export function encryptProviderConfig( column: SsoConfigColumn ): Promise { return mapSecretFields(config, column, async (value) => - isEnvelope(value) ? value : (await encryptSecret(value)).encrypted + isEnvelope(value) ? value : `${ENVELOPE_PREFIX}${(await encryptSecret(value)).encrypted}` ) } /** * Decrypts the secret fields of a stored provider config. Values written before - * these fields were encrypted lack the envelope shape and are returned as-is. + * these fields were encrypted lack the prefix and are returned as-is. * * A value that IS an envelope but fails to decrypt — a wrong or rotated * `ENCRYPTION_KEY`, a tampered row — throws rather than degrading to ciphertext. @@ -99,7 +104,8 @@ export function decryptProviderConfig( return mapSecretFields(config, column, async (value) => { if (!isEnvelope(value)) return value try { - return (await decryptSecret(value, { logFailure: false })).decrypted + return (await decryptSecret(value.slice(ENVELOPE_PREFIX.length), { logFailure: false })) + .decrypted } catch (error) { logger.error('Failed to decrypt an SSO provider secret', { column, diff --git a/packages/db/scripts/register-sso-provider.ts b/packages/db/scripts/register-sso-provider.ts index 27afb033840..48f3d9bab7b 100644 --- a/packages/db/scripts/register-sso-provider.ts +++ b/packages/db/scripts/register-sso-provider.ts @@ -161,21 +161,26 @@ if (!ENCRYPTION_KEY || !/^[0-9a-f]{64}$/i.test(ENCRYPTION_KEY)) { const ENCRYPTION_KEY_BUFFER = Buffer.from(ENCRYPTION_KEY, 'hex') /** - * AES-256-GCM in the `iv:ciphertext:authTag` envelope the app reads back. This + * AES-256-GCM in the prefixed envelope the app reads back — the prefix is what + * marks a value as encrypted, so a secret that merely looks like one is not + * mistaken for it. Keep it in step with `ENVELOPE_PREFIX` in + * `apps/sim/lib/auth/sso/provider-secrets.ts`. This * package cannot import from `apps/*`, so the primitive is repeated here rather * than shared; {@link assertCryptoRoundTrip} proves the key produces a readable * value before any row is written. */ +const ENVELOPE_PREFIX = 'sim.sso.v1:' + function encryptSecretValue(secret: string): string { const iv = randomBytes(16) const cipher = createCipheriv('aes-256-gcm', ENCRYPTION_KEY_BUFFER, iv, { authTagLength: 16 }) let encrypted = cipher.update(secret, 'utf8', 'hex') encrypted += cipher.final('hex') - return `${iv.toString('hex')}:${encrypted}:${cipher.getAuthTag().toString('hex')}` + return `${ENVELOPE_PREFIX}${iv.toString('hex')}:${encrypted}:${cipher.getAuthTag().toString('hex')}` } function decryptSecretValue(envelope: string): string { - const [ivHex, ciphertext, authTagHex] = envelope.split(':') + const [ivHex, ciphertext, authTagHex] = envelope.slice(ENVELOPE_PREFIX.length).split(':') const decipher = createDecipheriv( 'aes-256-gcm', ENCRYPTION_KEY_BUFFER, From c73789c88e7dc66eb49b6528fb592a981e3749c6 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Sat, 19 Sep 2026 18:43:48 -0700 Subject: [PATCH 3/9] feat(sso): support encrypted SAML assertions and retire the registration scripts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Encrypted assertions are now configured in the settings UI and the registration API: the service provider certificate is published in the SP metadata Sim serves, and the matching private key is stored encrypted and handed to the SAML library for decryption. Request signing is deliberately absent — Better Auth's service provider neither signs requests nor publishes a signing certificate, so a key field for it would do nothing. The operator registration scripts are removed along with the env vars and docs that drove them; the UI covers registration, and the scripts were a second write path with their own copy of the crypto. The field they wrote for assertion decryption was never read by the SAML library, so it never worked. Also: run the new adapter PostgreSQL suite in CI, reject a stored OIDC config whose client secret is missing rather than saving a provider without one, and cover the nested service-provider keys in both encryption at rest and API redaction. --- .github/workflows/test-build.yml | 1 + .../content/docs/platform/enterprise/sso.mdx | 63 +- apps/sim/app/api/auth/sso/providers/route.ts | 24 +- .../app/api/auth/sso/register/route.test.ts | 149 +++ apps/sim/app/api/auth/sso/register/route.ts | 125 ++- .../sso/components/sso-provider-settings.tsx | 150 ++++ .../ee/sso/components/sso-settings.test.tsx | 80 +- apps/sim/lib/api/contracts/auth.ts | 64 +- .../sim/lib/auth/sso-provider-secrets.test.ts | 43 + apps/sim/lib/auth/sso-provider-secrets.ts | 50 +- apps/sim/lib/core/config/env.ts | 38 +- apps/sim/scripts/create-oauth-client.ts | 2 +- .../db/scripts/deregister-sso-provider.ts | 189 ---- packages/db/scripts/register-sso-provider.ts | 845 ------------------ packages/sim-setup/src/checks.ts | 1 - 15 files changed, 663 insertions(+), 1161 deletions(-) delete mode 100644 packages/db/scripts/deregister-sso-provider.ts delete mode 100644 packages/db/scripts/register-sso-provider.ts diff --git a/.github/workflows/test-build.yml b/.github/workflows/test-build.yml index 22c14043fa5..6019411aa9c 100644 --- a/.github/workflows/test-build.yml +++ b/.github/workflows/test-build.yml @@ -116,6 +116,7 @@ jobs: ee/scim/lib/managed-membership.postgres.test.ts lib/auth/sso/application/admit-sso-user.postgres.test.ts lib/auth/sso/primary-provider.postgres.test.ts + lib/auth/sso-provider-secret-adapter.postgres.test.ts - name: Verify billing and organization activity in PostgreSQL working-directory: apps/sim diff --git a/apps/docs/content/docs/platform/enterprise/sso.mdx b/apps/docs/content/docs/platform/enterprise/sso.mdx index 4f7f1092d42..d5d23baf917 100644 --- a/apps/docs/content/docs/platform/enterprise/sso.mdx +++ b/apps/docs/content/docs/platform/enterprise/sso.mdx @@ -103,7 +103,22 @@ Click **Save**. To test, sign out and use the **Sign in with SSO** button on the For a saved connection, open **Sign-in**, select the provider, and select **Edit**. The Provider ID remains fixed. **Delete** removes that sign-in path only: accounts and memberships it admitted stay. If you delete the primary provider and the domain has another verified provider, that one becomes primary; otherwise people at the domain sign in another way until a provider serves it again. A saved OIDC client secret appears as a mask with a suffix when available; **Replace** lets you enter a new secret, and **Keep saved** cancels that replacement. Provider secrets — the OIDC client secret, and SAML signing and decryption keys — are encrypted with `ENCRYPTION_KEY` before they are stored, so a copy of the database alone does not expose them. Select **Update** to save the provider, or **Discard** to abandon changes. -**Advanced options** contains OIDC scopes and optional authorization, token, and JWKS endpoint overrides. For SAML, it contains Audience, Callback URL override, signed-assertion requirements, NameID format, and optional IdP metadata XML. **Attribute mapping** lets either protocol override the email, name, and stable user-ID claim names. Leave a mapping blank to use the protocol default. +**Advanced options** contains OIDC scopes and optional authorization, token, and JWKS endpoint overrides. For SAML, it contains Audience, Callback URL override, signed-assertion requirements, encrypted assertions, NameID format, and optional IdP metadata XML. **Attribute mapping** lets either protocol override the email, name, and stable user-ID claim names. Leave a mapping blank to use the protocol default. + +### Encrypted assertions + +Turn on **Encrypt SAML assertions** when your identity provider encrypts the assertion, which some organizations require for assertions carrying personal data. It takes a key pair you generate: + +- **Service provider certificate** — the public half. Sim publishes it in its service provider metadata, and you upload it to the identity provider as the encryption certificate. +- **Service provider private key** — the half Sim decrypts with. It is encrypted with `ENCRYPTION_KEY` before it is stored, and the form shows only a mask afterwards; **Replace** takes a new key. + +Generate a pair with `openssl req -x509 -newkey rsa:2048 -keyout sp-key.pem -out sp-cert.pem -days 3650 -nodes`. Turning the setting off clears the stored key. + + + Signing the authentication request Sim sends is not supported. Identity providers that + can require signed requests — Entra ID's **Require verification certificates**, for + example — must leave that off for Sim's application, which is their default. + SCIM settings save immediately in the **Provisioning** tab. Its **Disable just-in-time provisioning** rule overrides Automatic first-sign-in membership while the connection is active and entitled. Existing members can still sign in. See [directory provisioning](/platform/enterprise/scim#provisioning-and-sso-together). @@ -417,48 +432,4 @@ SSO_TRUSTED_PROVIDER_IDS=custom-oidc,partner-saml depend on your IdP asserting `email_verified`. -You can register providers through the **Settings UI** (same as cloud) or by running the registration script directly against your database. - -### Script-based registration - -Use this when you need to register an SSO provider without going through the UI — for example, during initial deployment or CI/CD automation. - -```bash -# OIDC example (Okta) -SSO_ENABLED=true \ -NEXT_PUBLIC_APP_URL=https://your-instance.com \ -SSO_PROVIDER_TYPE=oidc \ -SSO_PROVIDER_ID=okta \ -SSO_ISSUER=https://dev-1234567.okta.com \ -SSO_DOMAIN=company.com \ -SSO_USER_EMAIL=admin@company.com \ -SSO_OIDC_CLIENT_ID=your-client-id \ -SSO_OIDC_CLIENT_SECRET=your-client-secret \ -bun run packages/db/scripts/register-sso-provider.ts -``` - -```bash -# SAML example (ADFS) -SSO_ENABLED=true \ -NEXT_PUBLIC_APP_URL=https://your-instance.com \ -SSO_PROVIDER_TYPE=saml \ -SSO_PROVIDER_ID=adfs \ -SSO_ISSUER=https://adfs.company.com/adfs/services/trust \ -SSO_SAML_AUDIENCE=https://your-instance.com \ -SSO_DOMAIN=company.com \ -SSO_USER_EMAIL=admin@company.com \ -SSO_SAML_ENTRY_POINT=https://adfs.company.com/adfs/ls \ -SSO_SAML_CERT="-----BEGIN CERTIFICATE----- -... ------END CERTIFICATE-----" \ -bun run packages/db/scripts/register-sso-provider.ts -``` - -The script outputs the callback URL to configure in your IdP once it completes. - -To remove a provider: - -```bash -SSO_USER_EMAIL=admin@company.com \ -bun run packages/db/scripts/deregister-sso-provider.ts -``` +Register providers through the **Settings UI**, the same flow as cloud. Verify the email domain first: a provider is only saved against a domain the organization has verified, which is what authorizes it to sign people in. diff --git a/apps/sim/app/api/auth/sso/providers/route.ts b/apps/sim/app/api/auth/sso/providers/route.ts index b930f7f6c8a..eacf5f780f0 100644 --- a/apps/sim/app/api/auth/sso/providers/route.ts +++ b/apps/sim/app/api/auth/sso/providers/route.ts @@ -11,7 +11,11 @@ import { listSsoProvidersContract } from '@/lib/api/contracts/auth' import { parseRequest } from '@/lib/api/server' import { getSession } from '@/lib/auth' import { markSignInProviders } from '@/lib/auth/sso/primary-provider' -import { decryptProviderConfig } from '@/lib/auth/sso-provider-secrets' +import { + decryptProviderConfig, + resolveHolder, + SECRET_FIELDS, +} from '@/lib/auth/sso-provider-secrets' import { REDACTED_MARKER } from '@/lib/core/security/redaction' import { withRouteHandler } from '@/lib/core/utils/with-route-handler' @@ -58,17 +62,23 @@ async function redactOidcConfig(oidcConfig: string | null): Promise { expect(mockDecryptSecret).not.toHaveBeenCalled() }) + it.each([ + ['no client secret', JSON.stringify({ clientId: 'client' })], + ['an empty client secret', JSON.stringify({ clientId: 'client', clientSecret: '' })], + ])('refuses to reuse a stored config with %s', async (_label, oidcConfig) => { + queueMembers([{ organizationId: 'org1', role: 'owner' }]) + queueTableRows(schemaMock.ssoProvider, []) + queueTableRows(schemaMock.ssoProvider, []) + queueTableRows(schemaMock.ssoProvider, [{ oidcConfig }]) + + const res = await POST(request({ ...OIDC_BODY, clientSecret: '[REDACTED]' })) + + expect(res.status).toBe(400) + await expect(res.json()).resolves.toMatchObject({ + error: expect.stringContaining('Re-enter your client secret'), + }) + expect(mockUpdateSSOProvider).not.toHaveBeenCalled() + }) + + describe('SAML encrypted assertions', () => { + const SP_CERT = `-----BEGIN CERTIFICATE-----\nQUJD\n-----END CERTIFICATE-----` + const SP_KEY = `-----BEGIN PRIVATE KEY-----\nREVG\n-----END PRIVATE KEY-----` + const samlBody = (overrides: Record = {}) => ({ + providerType: 'saml' as const, + providerId: 'acme-saml', + issuer: 'https://idp.acme.com', + domain: 'acme.com', + orgId: 'org1', + entryPoint: 'https://idp.acme.com/sso', + cert: 'IDP-CERT', + ...overrides, + }) + + it('publishes the certificate and keeps the private key for decryption', async () => { + queueMembers([{ organizationId: 'org1', role: 'owner' }]) + queueProviders([]) + + const res = await POST( + request( + samlBody({ encryptAssertions: true, spEncryptionCert: SP_CERT, spDecryptionKey: SP_KEY }) + ) + ) + + expect(res.status).toBe(200) + const { samlConfig } = mockRegisterSSOProvider.mock.calls[0][0].body + expect(samlConfig.spMetadata).toMatchObject({ + isAssertionEncrypted: true, + encPrivateKey: SP_KEY, + encryptionCert: SP_CERT, + }) + /** The certificate travels in the metadata document, stripped of its PEM armor. */ + expect(samlConfig.spMetadata.metadata).toContain('use="encryption"') + expect(samlConfig.spMetadata.metadata).toContain('QUJD') + expect(samlConfig.spMetadata.metadata).not.toContain('BEGIN CERTIFICATE') + expect(samlConfig.spMetadata.metadata).not.toContain('REVG') + }) + + it('leaves the metadata and key material alone when encryption is off', async () => { + queueMembers([{ organizationId: 'org1', role: 'owner' }]) + queueProviders([]) + + const res = await POST(request(samlBody())) + + expect(res.status).toBe(200) + const { samlConfig } = mockRegisterSSOProvider.mock.calls[0][0].body + expect(samlConfig.spMetadata).not.toHaveProperty('encPrivateKey') + expect(samlConfig.spMetadata).not.toHaveProperty('isAssertionEncrypted') + expect(samlConfig.spMetadata).not.toHaveProperty('encryptionCert') + expect(samlConfig.spMetadata.metadata).not.toContain('use="encryption"') + }) + + it.each([ + ['no certificate', { spDecryptionKey: SP_KEY }], + ['no private key', { spEncryptionCert: SP_CERT }], + ])('refuses to enable encryption with %s', async (_label, overrides) => { + queueMembers([{ organizationId: 'org1', role: 'owner' }]) + queueProviders([]) + + const res = await POST(request(samlBody({ encryptAssertions: true, ...overrides }))) + + expect(res.status).toBe(400) + expect(mockRegisterSSOProvider).not.toHaveBeenCalled() + }) + + it.each([ + [ + 'a certificate that is not PEM', + { spEncryptionCert: 'not-a-cert', spDecryptionKey: SP_KEY }, + ], + ['a private key that is not PEM', { spEncryptionCert: SP_CERT, spDecryptionKey: 'nope' }], + ])('refuses %s', async (_label, overrides) => { + queueMembers([{ organizationId: 'org1', role: 'owner' }]) + queueProviders([]) + + const res = await POST(request(samlBody({ encryptAssertions: true, ...overrides }))) + + expect(res.status).toBe(400) + await expect(res.json()).resolves.toMatchObject({ error: expect.stringContaining('PEM') }) + expect(mockRegisterSSOProvider).not.toHaveBeenCalled() + }) + + it('keeps the stored private key when the update sends the marker', async () => { + queueMembers([{ organizationId: 'org1', role: 'owner' }]) + queueTableRows(schemaMock.ssoProvider, []) + queueTableRows(schemaMock.ssoProvider, []) + queueTableRows(schemaMock.ssoProvider, [ + { samlConfig: JSON.stringify({ spMetadata: { encPrivateKey: SP_KEY } }) }, + ]) + queueTableRows(schemaMock.ssoProvider, []) + queueTableRows(schemaMock.ssoProvider, []) + queueTableRows(schemaMock.ssoProvider, [{ id: 'p1' }]) + + const res = await POST( + request( + samlBody({ + encryptAssertions: true, + spEncryptionCert: SP_CERT, + spDecryptionKey: '[REDACTED]', + }) + ) + ) + + expect(res.status).toBe(200) + const { samlConfig } = mockUpdateSSOProvider.mock.calls[0][0].body + expect(samlConfig.spMetadata.encPrivateKey).toBe(SP_KEY) + }) + + it('refuses the marker when no key is stored', async () => { + queueMembers([{ organizationId: 'org1', role: 'owner' }]) + queueTableRows(schemaMock.ssoProvider, []) + queueTableRows(schemaMock.ssoProvider, []) + queueTableRows(schemaMock.ssoProvider, [{ samlConfig: JSON.stringify({ spMetadata: {} }) }]) + + const res = await POST( + request( + samlBody({ + encryptAssertions: true, + spEncryptionCert: SP_CERT, + spDecryptionKey: '[REDACTED]', + }) + ) + ) + + expect(res.status).toBe(400) + await expect(res.json()).resolves.toMatchObject({ + error: expect.stringContaining('no stored service provider private key'), + }) + }) + }) + /** updateSSOProvider resets domainVerified to false whenever the domain changes. */ it('re-marks the provider domain-verified after an update', async () => { queueMembers([{ organizationId: 'org1', role: 'owner' }]) diff --git a/apps/sim/app/api/auth/sso/register/route.ts b/apps/sim/app/api/auth/sso/register/route.ts index defe1c420a2..7395b0cf75c 100644 --- a/apps/sim/app/api/auth/sso/register/route.ts +++ b/apps/sim/app/api/auth/sso/register/route.ts @@ -29,7 +29,7 @@ type TokenEndpointAuthMethod = 'client_secret_basic' | 'client_secret_post' * Prefers client_secret_post over client_secret_basic when an IdP supports both: * better-auth sends client_secret_basic credentials without URL-encoding per * RFC 6749 §2.3.1, so a '+' in the client secret is decoded as a space, causing - * invalid_client errors. Matches the same default in register-sso-provider.ts. + * invalid_client errors. */ function selectTokenEndpointAuthMethod( supportedMethods: unknown, @@ -84,6 +84,47 @@ async function fetchOIDCDiscoveryDocument(discoveryUrl: string): Promise { try { if (!isSsoEnabled) { @@ -120,8 +161,8 @@ export const POST = withRouteHandler(async (request: NextRequest) => { const { providerId, issuer, providerType, mapping, orgId, jitProvisioningEnabled } = body /** - * Always org-scoped: an org-less provider has no `sso_domain` proof, so only - * operators create one, via `packages/db/scripts/register-sso-provider.ts`. + * Always org-scoped: an org-less provider has no `sso_domain` proof, and the + * verified domain is what authorizes a provider to sign anyone in. */ const [membership] = await db .select({ organizationId: member.organizationId, role: member.role }) @@ -313,7 +354,16 @@ export const POST = withRouteHandler(async (request: NextRequest) => { } try { const stored = await decryptProviderConfig(existing.oidcConfig, 'oidcConfig') - clientSecret = JSON.parse(stored as string).clientSecret + const storedSecret = JSON.parse(stored as string).clientSecret + /** + * A stored config without a usable secret cannot be reused: letting it + * through would save the provider with no client secret at all, and the + * failure would only appear at the next sign-in. + */ + if (typeof storedSecret !== 'string' || storedSecret === '') { + throw new Error('stored OIDC config has no client secret') + } + clientSecret = storedSecret } catch { return NextResponse.json( { @@ -516,8 +566,43 @@ export const POST = withRouteHandler(async (request: NextRequest) => { digestAlgorithm, identifierFormat, idpMetadata, + encryptAssertions, + spEncryptionCert, + spDecryptionKey, } = body + /** + * The private key half of the encryption pair. Like the OIDC client + * secret, an update may send the redaction marker to keep the stored one + * rather than re-pasting it. + */ + let decryptionKey = spDecryptionKey + if (encryptAssertions && spDecryptionKey === REDACTED_MARKER) { + const [existing] = await db + .select({ samlConfig: ssoProvider.samlConfig }) + .from(ssoProvider) + .where(ownerClause) + .limit(1) + const storedKey = existing?.samlConfig + ? readStoredDecryptionKey(await decryptProviderConfig(existing.samlConfig, 'samlConfig')) + : null + if (!storedKey) { + return NextResponse.json( + { + error: + 'Cannot update: no stored service provider private key. Re-enter the key to keep encrypted assertions on.', + }, + { status: 400 } + ) + } + decryptionKey = storedKey + } + + if (encryptAssertions) { + const pemProblem = describePemProblem(spEncryptionCert, decryptionKey) + if (pemProblem) return NextResponse.json({ error: pemProblem }, { status: 400 }) + } + const computedCallbackUrl = callbackUrl || `${getBaseUrl()}/api/auth/sso/saml2/callback/${providerId}` @@ -539,9 +624,20 @@ export const POST = withRouteHandler(async (request: NextRequest) => { } }) + /** + * Published so the identity provider can encrypt assertions to Sim. Only + * the certificate goes in the document; the matching private key stays in + * the provider row, encrypted. + */ + const encryptionKeyDescriptor = + encryptAssertions && spEncryptionCert + ? ` + ${escapeXml(stripPemArmor(spEncryptionCert))}` + : '' + const spMetadataXml = ` - + ${encryptionKeyDescriptor} ` @@ -550,8 +646,27 @@ export const POST = withRouteHandler(async (request: NextRequest) => { entryPoint, cert, callbackUrl: computedCallbackUrl, + /** + * Rebuilt on every save, and Better Auth replaces the whole object + * rather than merging its keys, so turning encryption off here clears + * the key material with it. + */ spMetadata: { metadata: spMetadataXml, + ...(encryptAssertions && decryptionKey + ? { + isAssertionEncrypted: true, + encPrivateKey: decryptionKey, + /** + * The certificate as the admin pasted it. The metadata document + * carries it stripped of its PEM armor, which is what the + * identity provider reads; keeping the original lets the + * settings form show it back without parsing that XML. Better + * Auth ignores keys it does not know. + */ + encryptionCert: spEncryptionCert, + } + : {}), }, } diff --git a/apps/sim/ee/sso/components/sso-provider-settings.tsx b/apps/sim/ee/sso/components/sso-provider-settings.tsx index 2bfd5af853e..e4759225a41 100644 --- a/apps/sim/ee/sso/components/sso-provider-settings.tsx +++ b/apps/sim/ee/sso/components/sso-provider-settings.tsx @@ -154,6 +154,23 @@ function ClientSecretField({ } /** Reads a string from stored provider JSON, tolerating malformed legacy configurations. */ +/** + * Whether a saved SAML provider holds a service-provider private key. The API + * returns only its sentinel, so presence is all a client can see — and all it + * needs, to offer "keep the saved key" rather than demand a fresh paste. + */ +function hasStoredSpDecryptionKey(samlConfig: string | null | undefined): boolean { + if (!samlConfig) return false + try { + const config: unknown = JSON.parse(samlConfig) + if (!isRecordLike(config)) return false + const spMetadata = config.spMetadata + return isRecordLike(spMetadata) && typeof spMetadata.encPrivateKey === 'string' + } catch { + return false + } +} + function readProviderConfigString( serialized: string | null | undefined, field: string @@ -185,6 +202,9 @@ const DEFAULT_FORM_DATA = { mapEmail: '', mapName: '', identifierFormat: '', + encryptAssertions: false, + spEncryptionCert: '', + spDecryptionKey: '', authorizationEndpoint: '', tokenEndpoint: '', jwksEndpoint: '', @@ -231,6 +251,7 @@ export function SsoProviderSettings({ const [showErrors, setShowErrors] = useState(false) const [isReplacingClientSecret, setIsReplacingClientSecret] = useState(false) + const [isReplacingDecryptionKey, setIsReplacingDecryptionKey] = useState(false) /** * Editing an OIDC provider always means a secret is stored — the contract @@ -243,6 +264,15 @@ export function SsoProviderSettings({ ? readProviderConfigString(existingProvider?.oidcConfig, 'clientSecretHint') : null + /** + * A SAML provider saved with encrypted assertions already holds the private + * key, and the API returns only its sentinel. Blank therefore means "keep it". + */ + const hasStoredDecryptionKey = + isEditing && + existingProvider?.providerType === 'saml' && + hasStoredSpDecryptionKey(existingProvider?.samlConfig) + const hasChanges = (Object.keys(formData) as (keyof typeof formData)[]).some( (k) => formData[k] !== originalFormData[k] ) @@ -318,6 +348,17 @@ export function SsoProviderSettings({ newErrors.entryPoint = ['Entry Point URL is required for SAML providers'] } newErrors.cert = validateRequired('Certificate', data.cert) + if (data.encryptAssertions) { + newErrors.spEncryptionCert = validateRequired( + 'Service provider certificate', + data.spEncryptionCert + ) + /** Skipped only while the stored key is being kept, as for the client secret. */ + newErrors.spDecryptionKey = + hasStoredDecryptionKey && !isReplacingDecryptionKey + ? [] + : validateRequired('Service provider private key', data.spDecryptionKey) + } } return newErrors @@ -402,6 +443,17 @@ export function SsoProviderSettings({ ...(formData.audience ? { audience: formData.audience } : {}), ...(formData.idpMetadata ? { idpMetadata: formData.idpMetadata } : {}), identifierFormat: formData.identifierFormat, + encryptAssertions: formData.encryptAssertions, + ...(formData.encryptAssertions + ? { + spEncryptionCert: formData.spEncryptionCert, + /** Unchanged on an edit: the marker keeps the stored key. */ + spDecryptionKey: + hasStoredDecryptionKey && !isReplacingDecryptionKey + ? REDACTED_MARKER + : formData.spDecryptionKey, + } + : {}), } await configureSSOMutation.mutateAsync(requestBody) @@ -462,6 +514,8 @@ export function SsoProviderSettings({ /** Blank means "use the protocol default", so only carry over a stored value that differs — otherwise editing rewrites a default as an explicit override. */ let mapping: { id?: string; email?: string; name?: string } = {} let identifierFormat = '' + let encryptAssertions = false + let spEncryptionCert = '' let authorizationEndpoint = '' let tokenEndpoint = '' let jwksEndpoint = '' @@ -489,6 +543,9 @@ export function SsoProviderSettings({ : (config.idpMetadata?.metadata ?? '') mapping = config.mapping ?? {} identifierFormat = config.identifierFormat || '' + encryptAssertions = config.spMetadata?.isAssertionEncrypted === true + /** The certificate is public and kept beside the metadata document so it can be shown back. */ + spEncryptionCert = config.spMetadata?.encryptionCert || '' } const defaults = @@ -514,6 +571,9 @@ export function SsoProviderSettings({ mapEmail: overrideOf(mapping.email, defaults.email), mapName: overrideOf(mapping.name, defaults.name), identifierFormat, + encryptAssertions, + spEncryptionCert, + spDecryptionKey: '', authorizationEndpoint, tokenEndpoint, jwksEndpoint, @@ -525,6 +585,7 @@ export function SsoProviderSettings({ setShowErrors(false) setShowAdvanced(false) setIsReplacingClientSecret(false) + setIsReplacingDecryptionKey(false) setShowMapping(Boolean(snapshot.mapId || snapshot.mapEmail || snapshot.mapName)) } catch (err) { logger.error('Failed to parse provider config', { error: err }) @@ -1057,6 +1118,95 @@ export function SsoProviderSettings({ /> +
+ + + handleInputChange('encryptAssertions', checked) + } + /> +
+ + {formData.encryptAssertions && ( + <> + 0 + ? errors.spEncryptionCert.join(' ') + : undefined + } + > + + handleInputChange('spEncryptionCert', e.target.value) + } + className='min-h-20' + error={showErrors && errors.spEncryptionCert?.length > 0} + rows={3} + /> +

+ Upload this certificate to your identity provider so it can encrypt + assertions to Sim. +

+ + + 0 + ? errors.spDecryptionKey.join(' ') + : undefined + } + > + {hasStoredDecryptionKey && !isReplacingDecryptionKey ? ( +
+ + setIsReplacingDecryptionKey(true)}> + Replace + +
+ ) : ( + + handleInputChange('spDecryptionKey', e.target.value) + } + className='min-h-20' + error={showErrors && errors.spDecryptionKey?.length > 0} + rows={3} + /> + )} + + + )} + ({ ), ChipTextarea: ({ + id, value, onChange, }: { + id?: string value?: string onChange?: ChangeEventHandler - }) =>