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 ff7a6fb4002..d5d23baf917 100644 --- a/apps/docs/content/docs/platform/enterprise/sso.mdx +++ b/apps/docs/content/docs/platform/enterprise/sso.mdx @@ -101,9 +101,24 @@ 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. +**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/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..d55cbed34b1 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) => `sim.sso.v1:${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,71 @@ 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('still lists a provider whose secret cannot be decrypted', async () => { + mockDecryptSecret.mockRejectedValue(new Error('auth tag mismatch')) + queueTableRows(schemaMock.ssoProvider, [providerRow]) + + const res = await GET(createMockRequest('GET')) + + /** + * The settings form stays reachable, which is where an admin replaces the + * secret; the decryption failure is logged rather than 500ing the page. + */ + expect(res.status).toBe(200) + const { providers } = await res.json() + expect(providers).toHaveLength(1) + expect(providers[0]).toMatchObject({ providerId: 'acme-okta', oidcConfig: null }) + }) + + 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..5f5ff6b10ef 100644 --- a/apps/sim/app/api/auth/sso/providers/route.ts +++ b/apps/sim/app/api/auth/sso/providers/route.ts @@ -11,6 +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, + 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' @@ -31,6 +36,58 @@ 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 { + /** + * A secret that will not decrypt — a lost or rotated `ENCRYPTION_KEY` — + * reports as a provider with no config rather than failing the request. + * `decryptProviderConfig` has already logged the cause, and listing the + * provider is what keeps the settings form reachable: it still offers + * Replace, which is how an admin restores a working secret. A 500 here + * would take the whole page down and leave no way back. + */ + 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: the service provider's + * own signing and decryption keys. Unlike the OIDC secret these carry no hint — + * the admin holds the key pair already, and the certificate half stays readable. + * The same field list drives encryption at rest, so the two cannot drift. + */ +function redactSamlConfig(samlConfig: string | null): string | null { + if (!samlConfig) return samlConfig + try { + const parsed = JSON.parse(samlConfig) + if (!parsed || typeof parsed !== 'object') return null + for (const path of SECRET_FIELDS.samlConfig) { + const holder = resolveHolder(parsed, path) + if (!holder) continue + const field = path[path.length - 1] + if (typeof holder[field] === 'string' && holder[field] !== '') { + holder[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 +147,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..d4a6b3de766 100644 --- a/apps/sim/app/api/auth/sso/register/route.test.ts +++ b/apps/sim/app/api/auth/sso/register/route.test.ts @@ -1,6 +1,12 @@ /** * @vitest-environment node */ + +import { execFileSync } from 'node:child_process' +import { X509Certificate } from 'node:crypto' +import { mkdtempSync, readFileSync, rmSync } from 'node:fs' +import { tmpdir } from 'node:os' +import path from 'node:path' import { createMockRequest, dbChainMock, @@ -13,6 +19,7 @@ import { setEnv, setEnvFlags, } from '@sim/testing' +import { loggerMock } from '@sim/testing/mocks/logger.mock' import { afterAll, beforeEach, describe, expect, it, vi } from 'vitest' const { @@ -22,6 +29,7 @@ const { mockHasSSOAccess, mockValidateUrlWithDNS, mockSecureFetchWithPinnedIP, + mockDecryptSecret, } = vi.hoisted(() => ({ mockGetSession: vi.fn(), mockRegisterSSOProvider: vi.fn(), @@ -29,6 +37,7 @@ const { mockHasSSOAccess: vi.fn(), mockValidateUrlWithDNS: vi.fn(), mockSecureFetchWithPinnedIP: vi.fn(), + mockDecryptSecret: vi.fn(), })) vi.mock('@sim/db', () => ({ ...dbChainMock, ...schemaMock })) @@ -76,6 +85,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, @@ -83,6 +98,17 @@ vi.mock('@/lib/core/security/input-validation.server', () => ({ import { POST } from '@/app/api/auth/sso/register/route' +type MockLogger = { info: { mock: { calls: unknown[][] } } } + +/** The logger the route built at import time, so its calls can be inspected. */ +const routeLogger = loggerMock.createLogger.mock.calls.reduce( + (found, call, index) => + call[0] === 'SSORegisterRoute' + ? (loggerMock.createLogger.mock.results[index].value as MockLogger) + : found, + null +) + const OIDC_BODY = { providerType: 'oidc' as const, providerId: 'acme-oidc', @@ -115,6 +141,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 +317,374 @@ 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 = `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. + 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() + }) + + 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() + }) + + /** The SAML branch carries a superRefine now; these prove the union still narrows cleanly. */ + it.each([ + ['an empty certificate', { cert: '' }, /Certificate is required for SAML/], + ['a malformed entry point', { entryPoint: 'not-a-url' }, /[Ee]ntry point/], + /** A missing field reports the type error; the point is that it narrows to SAML at all. */ + ['a missing certificate', { cert: undefined }, /expected string/], + ])('rejects a SAML body with %s', async (_label, overrides, expected) => { + queueMembers([{ organizationId: 'org1', role: 'owner' }]) + queueProviders([]) + + const res = await POST( + request({ + providerType: 'saml', + providerId: 'acme-saml', + issuer: 'https://idp.acme.com', + domain: 'acme.com', + orgId: 'org1', + entryPoint: 'https://idp.acme.com/sso', + cert: 'IDP-CERT', + ...overrides, + }) + ) + + expect(res.status).toBe(400) + await expect(res.json()).resolves.toMatchObject({ error: expect.stringMatching(expected) }) + expect(mockRegisterSSOProvider).not.toHaveBeenCalled() + }) + + describe('SAML encrypted assertions', () => { + /** + * Real key material, because the route parses both halves and checks they + * belong together. Generated per run rather than committed: a private key + * in the repository is exactly what this PR is about not doing. + */ + const keyPair = (subject: string) => { + const dir = mkdtempSync(path.join(tmpdir(), 'sim-sso-keys-')) + execFileSync('openssl', [ + 'req', + '-x509', + '-newkey', + 'rsa:2048', + '-keyout', + path.join(dir, 'key.pem'), + '-out', + path.join(dir, 'cert.pem'), + '-days', + '2', + '-nodes', + '-subj', + `/CN=${subject}`, + ]) + const pair = { + cert: readFileSync(path.join(dir, 'cert.pem'), 'utf8'), + key: readFileSync(path.join(dir, 'key.pem'), 'utf8'), + } + rmSync(dir, { recursive: true, force: true }) + return pair + } + + const SP = keyPair('sim-test-sp') + const OTHER = keyPair('sim-test-other') + const SP_CERT = SP.cert + const SP_KEY = SP.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( + SP_CERT.replace(/-----(BEGIN|END) CERTIFICATE-----/g, '').replace(/\s+/g, '') + ) + expect(samlConfig.spMetadata.metadata).not.toContain('BEGIN CERTIFICATE') + expect(samlConfig.spMetadata.metadata).not.toContain('PRIVATE KEY') + }) + + it('publishes only the certificate bytes, never the key, in the metadata', async () => { + queueMembers([{ organizationId: 'org1', role: 'owner' }]) + queueProviders([]) + + await POST( + request( + samlBody({ encryptAssertions: true, spEncryptionCert: SP_CERT, spDecryptionKey: SP_KEY }) + ) + ) + + const { samlConfig } = mockRegisterSSOProvider.mock.calls[0][0].body + const published = samlConfig.spMetadata.metadata + /** Built from the parsed certificate's own DER, so it cannot echo pasted input. */ + expect(published).toContain(new X509Certificate(SP_CERT).raw.toString('base64')) + expect(published).not.toContain( + SP_KEY.replace(/-----(BEGIN|END) PRIVATE KEY-----/g, '') + .replace(/\s+/g, '') + .slice(0, 40) + ) + }) + + it('never writes the private key to a log line', async () => { + queueMembers([{ organizationId: 'org1', role: 'owner' }]) + queueProviders([]) + + await POST( + request( + samlBody({ encryptAssertions: true, spEncryptionCert: SP_CERT, spDecryptionKey: SP_KEY }) + ) + ) + + /** The route logs its resolved provider config; the key must be redacted there. */ + const logged = (routeLogger?.info.mock.calls ?? []) + .map((call) => JSON.stringify(call)) + .join('\n') + expect(logged).not.toContain('PRIVATE KEY') + expect(logged).toContain('[REDACTED]') + }) + + 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' }], + [ + 'a key pair whose halves do not match', + { spEncryptionCert: SP_CERT, spDecryptionKey: OTHER.key }, + ], + /** + * A private key satisfies a public-key comparison, so anything short of + * X.509 parsing would accept it here and then publish it as the + * certificate in service provider metadata. + */ + [ + 'a private key pasted into the certificate field', + { spEncryptionCert: SP_KEY, spDecryptionKey: SP_KEY }, + ], + ])('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.stringMatching(/PEM|matching pair/), + }) + 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('reuses a key left behind by the retired registration script', async () => { + queueMembers([{ organizationId: 'org1', role: 'owner' }]) + queueTableRows(schemaMock.ssoProvider, []) + queueTableRows(schemaMock.ssoProvider, []) + /** Rows the script wrote kept the key flat, where the SAML library never read it. */ + queueTableRows(schemaMock.ssoProvider, [ + { samlConfig: JSON.stringify({ decryptionPvk: 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('asks for the key again when the stored one cannot be decrypted', async () => { + mockDecryptSecret.mockRejectedValue(new Error('auth tag mismatch')) + queueMembers([{ organizationId: 'org1', role: 'owner' }]) + queueTableRows(schemaMock.ssoProvider, []) + queueTableRows(schemaMock.ssoProvider, []) + queueTableRows(schemaMock.ssoProvider, [ + { + samlConfig: JSON.stringify({ + spMetadata: { encPrivateKey: `sim.sso.v1:${'a'.repeat(32)}:dead:${'b'.repeat(32)}` }, + }), + }, + ]) + + const res = await POST( + request( + samlBody({ + encryptAssertions: true, + spEncryptionCert: SP_CERT, + spDecryptionKey: '[REDACTED]', + }) + ) + ) + + /** A key the app can no longer read is an operator action, not a server fault. */ + expect(res.status).toBe(400) + await expect(res.json()).resolves.toMatchObject({ + error: expect.stringContaining('Re-enter it'), + }) + }) + + 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 04c3c94c800..fb0acd7c4b3 100644 --- a/apps/sim/app/api/auth/sso/register/route.ts +++ b/apps/sim/app/api/auth/sso/register/route.ts @@ -1,3 +1,4 @@ +import { createPrivateKey, createPublicKey, X509Certificate } from 'node:crypto' import { db, member, ssoDomain, ssoProvider } from '@sim/db' import { keepDomainSignInProvider, ssoProviderDomainKey } from '@sim/db/sso-primary-provider' import { createLogger } from '@sim/logger' @@ -9,6 +10,7 @@ import { ssoRegistrationContract } from '@/lib/api/contracts/auth' import { getValidationErrorMessage, parseRequest } from '@/lib/api/server' import { auth, getSession } from '@/lib/auth' 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' @@ -28,7 +30,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, @@ -83,6 +85,78 @@ async function fetchOIDCDiscoveryDocument(discoveryUrl: string): Promise { try { if (!isSsoEnabled) { @@ -119,8 +193,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 }) @@ -310,9 +384,20 @@ export const POST = withRouteHandler(async (request: NextRequest) => { { status: 400 } ) } + let storedSecret: unknown try { - clientSecret = JSON.parse(existing.oidcConfig).clientSecret + const stored = await decryptProviderConfig(existing.oidcConfig, 'oidcConfig') + storedSecret = JSON.parse(stored as string).clientSecret } catch { + storedSecret = null + } + + /** + * Unreadable, or readable but holding no secret: either way there is + * nothing to carry forward, and saving without one would surface only at + * the next sign-in. + */ + if (typeof storedSecret !== 'string' || storedSecret === '') { return NextResponse.json( { error: 'Cannot update: failed to read existing secret. Re-enter your client secret.', @@ -320,6 +405,7 @@ export const POST = withRouteHandler(async (request: NextRequest) => { { status: 400 } ) } + clientSecret = storedSecret } const oidcConfig: any = { @@ -514,8 +600,59 @@ 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) + let storedKey: string | null = null + if (existing?.samlConfig) { + try { + storedKey = readStoredDecryptionKey( + await decryptProviderConfig(existing.samlConfig, 'samlConfig') + ) + } catch { + /** A key the app can no longer read is re-entered, not a 500. */ + return NextResponse.json( + { + error: + 'Cannot update: failed to read the saved service provider private key. Re-enter it.', + }, + { status: 400 } + ) + } + } + 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 + } + + let encryptionCertificate: X509Certificate | null = null + if (encryptAssertions) { + const keyPair = checkKeyPair(spEncryptionCert, decryptionKey) + if ('error' in keyPair) return NextResponse.json({ error: keyPair.error }, { status: 400 }) + encryptionCertificate = keyPair.certificate + } + const computedCallbackUrl = callbackUrl || `${getBaseUrl()}/api/auth/sso/saml2/callback/${providerId}` @@ -537,9 +674,19 @@ 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 = encryptionCertificate + ? ` + ${encryptionCertificate.raw.toString('base64')}` + : '' + const spMetadataXml = ` - + ${encryptionKeyDescriptor} ` @@ -548,8 +695,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, + } + : {}), }, } @@ -594,6 +760,15 @@ export const POST = withRouteHandler(async (request: NextRequest) => { ? { ...providerConfig.samlConfig, cert: REDACTED_MARKER, + /** The service provider's own private key never reaches a log line. */ + ...(providerConfig.samlConfig.spMetadata?.encPrivateKey + ? { + spMetadata: { + ...providerConfig.samlConfig.spMetadata, + encPrivateKey: REDACTED_MARKER, + }, + } + : {}), } : undefined, }, @@ -695,6 +870,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/ee/sso/components/sso-provider-settings.tsx b/apps/sim/ee/sso/components/sso-provider-settings.tsx index 2bfd5af853e..d523509b1e2 100644 --- a/apps/sim/ee/sso/components/sso-provider-settings.tsx +++ b/apps/sim/ee/sso/components/sso-provider-settings.tsx @@ -13,7 +13,6 @@ import { Expandable, ExpandableContent, Label, - Switch, toast, } from '@sim/emcn' import { ArrowLeft, ChevronDown, Eye, EyeOff } from '@sim/emcn/icons' @@ -61,6 +60,12 @@ const CLIENT_SECRET_FIELD_ID = 'sso-client-secret' /** Fixed width, so the mask never leaks how long the stored secret is. */ const CLIENT_SECRET_MASK = '••••••••••••' +/** On/off options for the SAML toggles, the chip equivalent of a boolean switch. */ +const TOGGLE_OPTIONS = [ + { value: 'on', label: 'On' }, + { value: 'off', label: 'Off' }, +] as const + interface ClientSecretFieldProps { /** A secret is already saved, so the field opens as a masked fact rather than an input. */ hasStoredSecret: boolean @@ -154,6 +159,25 @@ 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 + if (isRecordLike(spMetadata) && typeof spMetadata.encPrivateKey === 'string') return true + /** Rows from the retired registration script kept the key flat; the server reuses it too. */ + return typeof config.decryptionPvk === 'string' + } catch { + return false + } +} + function readProviderConfigString( serialized: string | null | undefined, field: string @@ -185,6 +209,9 @@ const DEFAULT_FORM_DATA = { mapEmail: '', mapName: '', identifierFormat: '', + encryptAssertions: false, + spEncryptionCert: '', + spDecryptionKey: '', authorizationEndpoint: '', tokenEndpoint: '', jwksEndpoint: '', @@ -231,6 +258,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 +271,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 +355,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 +450,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 +521,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 +550,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 +578,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 +592,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 }) @@ -1045,18 +1113,121 @@ export function SsoProviderSettings({
- - - handleInputChange('wantAssertionsSigned', checked) + + + handleInputChange('wantAssertionsSigned', value === 'on') + } + /> +
+ +
+ + + handleInputChange('encryptAssertions', value === 'on') } />
+ {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} + /> + {/** The pair to Replace, as on the client secret: put the saved key back. */} + {hasStoredDecryptionKey && ( + { + setIsReplacingDecryptionKey(false) + handleInputChange('spDecryptionKey', '') + }} + > + Keep saved + + )} +
+ )} +
+ + )} + ({ options, value, onChange, + 'aria-label': ariaLabel, }: { options: Array<{ label: string; value: string }> value: string onChange: (value: string) => void + 'aria-label'?: string }) => ( -
+
{options.map((option) => (
), ChipTextarea: ({ + id, value, onChange, }: { + id?: string value?: string onChange?: ChangeEventHandler - }) =>