Skip to content

Commit bdb8785

Browse files
committed
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.
1 parent 0850719 commit bdb8785

10 files changed

Lines changed: 70 additions & 25 deletions

‎apps/sim/app/api/auth/sso/providers/route.test.ts‎

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ import { GET } from '@/app/api/auth/sso/providers/route'
2828

2929
const IV = 'a'.repeat(32)
3030
const TAG = 'b'.repeat(32)
31-
const sealed = (secret: string) => `${IV}:${Buffer.from(secret).toString('hex')}:${TAG}`
31+
const sealed = (secret: string) => `sim.sso.v1:${IV}:${Buffer.from(secret).toString('hex')}:${TAG}`
3232

3333
const CLIENT_SECRET = 'a-long-client-secret-wxyz'
3434

@@ -94,6 +94,16 @@ describe('GET /api/auth/sso/providers', () => {
9494
expect(mockDecryptSecret).not.toHaveBeenCalled()
9595
})
9696

97+
it('fails the request when a stored secret cannot be decrypted', async () => {
98+
mockDecryptSecret.mockRejectedValue(new Error('auth tag mismatch'))
99+
queueTableRows(schemaMock.ssoProvider, [providerRow])
100+
101+
const res = await GET(createMockRequest('GET'))
102+
103+
/** Reporting a key problem as a provider with no config would hide it behind a 200. */
104+
expect(res.status).toBe(500)
105+
})
106+
97107
it('redacts SAML key material and keeps the certificate', async () => {
98108
queueTableRows(schemaMock.ssoProvider, [
99109
{

‎apps/sim/app/api/auth/sso/providers/route.ts‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ import { listSsoProvidersContract } from '@/lib/api/contracts/auth'
1111
import { parseRequest } from '@/lib/api/server'
1212
import { getSession } from '@/lib/auth'
1313
import { markSignInProviders } from '@/lib/auth/sso/primary-provider'
14-
import { decryptProviderConfig } from '@/lib/auth/sso/provider-secrets'
14+
import { decryptProviderConfig } from '@/lib/auth/sso-provider-secrets'
1515
import { REDACTED_MARKER } from '@/lib/core/security/redaction'
1616
import { withRouteHandler } from '@/lib/core/utils/with-route-handler'
1717

@@ -40,8 +40,14 @@ function buildClientSecretHint(clientSecret: unknown): string | null {
4040
*/
4141
async function redactOidcConfig(oidcConfig: string | null): Promise<string | null> {
4242
if (!oidcConfig) return oidcConfig
43+
/**
44+
* Outside the catch: a config that will not decrypt is a key problem, and
45+
* reporting it as a provider with no config would hide it behind a healthy
46+
* 200. Unreadable JSON stays tolerated below, as it was before.
47+
*/
48+
const decrypted = await decryptProviderConfig(oidcConfig, 'oidcConfig')
4349
try {
44-
const parsed = JSON.parse((await decryptProviderConfig(oidcConfig, 'oidcConfig')) as string)
50+
const parsed = JSON.parse(decrypted as string)
4551
const hint = buildClientSecretHint(parsed.clientSecret)
4652
parsed.clientSecret = REDACTED_MARKER
4753
if (hint) parsed.clientSecretHint = hint

‎apps/sim/app/api/auth/sso/register/route.test.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -305,7 +305,7 @@ describe('POST /api/auth/sso/register', () => {
305305
* directly rather than through Better Auth, so it decrypts it itself.
306306
*/
307307
it('reuses the stored client secret, decrypting it first', async () => {
308-
const sealed = `${'a'.repeat(32)}:${Buffer.from('stored-secret').toString('hex')}:${'b'.repeat(32)}`
308+
const sealed = `sim.sso.v1:${'a'.repeat(32)}:${Buffer.from('stored-secret').toString('hex')}:${'b'.repeat(32)}`
309309
queueMembers([{ organizationId: 'org1', role: 'owner' }])
310310
// In route order: providerId conflict and domain refusal, the reuse read,
311311
// both checks again before the write, then the pre-image being updated.

‎apps/sim/app/api/auth/sso/register/route.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,8 @@ import { type NextRequest, NextResponse } from 'next/server'
88
import { ssoRegistrationContract } from '@/lib/api/contracts/auth'
99
import { getValidationErrorMessage, parseRequest } from '@/lib/api/server'
1010
import { auth, getSession } from '@/lib/auth'
11-
import { decryptProviderConfig } from '@/lib/auth/sso/provider-secrets'
1211
import { invalidateSsoPolicyCache } from '@/lib/auth/sso-policy'
12+
import { decryptProviderConfig } from '@/lib/auth/sso-provider-secrets'
1313
import { hasSSOAccess } from '@/lib/billing'
1414
import { isSsoEnabled } from '@/lib/core/config/env-flags'
1515
import { runWithOutboundOrganization } from '@/lib/core/network/context.server'

‎apps/sim/lib/auth/sso-provider-secret-adapter.postgres.test.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -89,7 +89,7 @@ describe.skipIf(!databaseUrl)('SSO provider secrets in PostgreSQL', () => {
8989

9090
const atRest = JSON.parse(await storedConfig(providerId))
9191
expect(atRest.clientSecret).not.toBe(CLIENT_SECRET)
92-
expect(atRest.clientSecret).toMatch(/^[0-9a-f]{32}:[0-9a-f]+:[0-9a-f]{32}$/)
92+
expect(atRest.clientSecret).toMatch(/^sim\.sso\.v1:[0-9a-f]{32}:[0-9a-f]+:[0-9a-f]{32}$/)
9393
expect(atRest.clientId).toBe('client')
9494

9595
const loaded = await adapter.findOne<{ oidcConfig: string }>({
@@ -127,7 +127,7 @@ describe.skipIf(!databaseUrl)('SSO provider secrets in PostgreSQL', () => {
127127

128128
expect(JSON.parse(updated!.oidcConfig).clientSecret).toBe(CLIENT_SECRET)
129129
const atRest = JSON.parse(await storedConfig(providerId))
130-
expect(atRest.clientSecret).toMatch(/^[0-9a-f]{32}:[0-9a-f]+:[0-9a-f]{32}$/)
130+
expect(atRest.clientSecret).toMatch(/^sim\.sso\.v1:[0-9a-f]{32}:[0-9a-f]+:[0-9a-f]{32}$/)
131131
})
132132

133133
it('reads a row written before the secret was encrypted', async () => {

‎apps/sim/lib/auth/sso-provider-secret-adapter.test.ts‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,9 @@ import { encryptSsoProviderSecrets } from '@/lib/auth/sso-provider-secret-adapte
1818

1919
const IV = 'a'.repeat(32)
2020
const TAG = 'b'.repeat(32)
21-
const sealed = (secret: string) => `${IV}:${Buffer.from(secret).toString('hex')}:${TAG}`
21+
/** What `encryptSecret` returns; `provider-secrets` adds the prefix around it. */
22+
const raw = (secret: string) => `${IV}:${Buffer.from(secret).toString('hex')}:${TAG}`
23+
const sealed = (secret: string) => `sim.sso.v1:${raw(secret)}`
2224

2325
const PLAIN_OIDC = JSON.stringify({ clientId: 'client', clientSecret: 'super-secret' })
2426
const SEALED_OIDC = JSON.stringify({ clientId: 'client', clientSecret: sealed('super-secret') })
@@ -44,7 +46,7 @@ const asAdapter = (adapter: ReturnType<typeof createBaseAdapter>) =>
4446
describe('encryptSsoProviderSecrets', () => {
4547
beforeEach(() => {
4648
vi.clearAllMocks()
47-
mockEncryptSecret.mockImplementation(async (secret: string) => ({ encrypted: sealed(secret) }))
49+
mockEncryptSecret.mockImplementation(async (secret: string) => ({ encrypted: raw(secret) }))
4850
mockDecryptSecret.mockImplementation(async (value: string) => ({
4951
decrypted: Buffer.from(value.split(':')[1], 'hex').toString('utf8'),
5052
}))

‎apps/sim/lib/auth/sso-provider-secret-adapter.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ import {
33
decryptProviderConfig,
44
encryptProviderConfig,
55
type SsoConfigColumn,
6-
} from '@/lib/auth/sso/provider-secrets'
6+
} from '@/lib/auth/sso-provider-secrets'
77

88
type BetterAuthAdapter = ReturnType<ReturnType<typeof drizzleAdapter>>
99

apps/sim/lib/auth/sso/provider-secrets.test.ts renamed to apps/sim/lib/auth/sso-provider-secrets.test.ts

Lines changed: 19 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14,17 +14,19 @@ vi.mock('@/lib/core/security/encryption', () => ({
1414
decryptSecret: mockDecryptSecret,
1515
}))
1616

17-
import { decryptProviderConfig, encryptProviderConfig } from '@/lib/auth/sso/provider-secrets'
17+
import { decryptProviderConfig, encryptProviderConfig } from '@/lib/auth/sso-provider-secrets'
1818

1919
const IV = 'a'.repeat(32)
2020
const TAG = 'b'.repeat(32)
21-
const envelope = (ciphertext: string) => `${IV}:${ciphertext}:${TAG}`
21+
/** What `encryptSecret` returns; the module under test adds the prefix. */
22+
const raw = (ciphertext: string) => `${IV}:${ciphertext}:${TAG}`
23+
const envelope = (ciphertext: string) => `sim.sso.v1:${raw(ciphertext)}`
2224

2325
describe('provider secrets', () => {
2426
beforeEach(() => {
2527
vi.clearAllMocks()
2628
mockEncryptSecret.mockImplementation(async (secret: string) => ({
27-
encrypted: envelope(Buffer.from(secret).toString('hex')),
29+
encrypted: raw(Buffer.from(secret).toString('hex')),
2830
}))
2931
mockDecryptSecret.mockImplementation(async (value: string) => ({
3032
decrypted: Buffer.from(value.split(':')[1], 'hex').toString('utf8'),
@@ -74,6 +76,20 @@ describe('provider secrets', () => {
7476
expect(mockDecryptSecret).not.toHaveBeenCalled()
7577
})
7678

79+
it('treats a legacy secret shaped like an envelope as plain text', async () => {
80+
/** A client secret is chosen at the identity provider and can be any string. */
81+
const lookalike = `${'a'.repeat(32)}:${'c'.repeat(16)}:${'b'.repeat(32)}`
82+
const legacy = JSON.stringify({ clientSecret: lookalike })
83+
84+
await expect(decryptProviderConfig(legacy, 'oidcConfig')).resolves.toBe(legacy)
85+
expect(mockDecryptSecret).not.toHaveBeenCalled()
86+
87+
const stored = await encryptProviderConfig(legacy, 'oidcConfig')
88+
expect(JSON.parse(stored as string).clientSecret).toBe(
89+
envelope(Buffer.from(lookalike).toString('hex'))
90+
)
91+
})
92+
7793
it('does not encrypt a value that is already encrypted', async () => {
7894
const stored = JSON.stringify({ clientSecret: envelope('deadbeef') })
7995

Lines changed: 15 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -18,15 +18,20 @@ const SECRET_FIELDS = {
1818
export type SsoConfigColumn = keyof typeof SECRET_FIELDS
1919

2020
/**
21-
* The shape {@link encryptSecret} produces: a 16-byte IV and a 16-byte GCM auth
22-
* tag around hex ciphertext. Matching it exactly is what lets a value written
23-
* before these fields were encrypted be recognized as legacy plain text and
24-
* returned unchanged — the same tolerance `decryptApiKey` gives API keys.
21+
* Marks a value this module encrypted. An explicit prefix, rather than matching
22+
* the `iv:ciphertext:authTag` shape, is what makes the distinction unambiguous:
23+
* a client secret is an arbitrary string chosen at the identity provider, and
24+
* one that happened to look like an envelope would otherwise be read back as
25+
* ciphertext and fail to decrypt. Values without the prefix were stored before
26+
* these fields were encrypted and are passed through unchanged, the tolerance
27+
* `decryptApiKey` gives API keys.
28+
*
29+
* The version lets a future encoding change be told apart from this one.
2530
*/
26-
const ENVELOPE = /^[0-9a-f]{32}:[0-9a-f]+:[0-9a-f]{32}$/
31+
const ENVELOPE_PREFIX = 'sim.sso.v1:'
2732

2833
function isEnvelope(value: string): boolean {
29-
return ENVELOPE.test(value)
34+
return value.startsWith(ENVELOPE_PREFIX)
3035
}
3136

3237
function parseConfig(config: string): Record<string, unknown> | null {
@@ -79,13 +84,13 @@ export function encryptProviderConfig(
7984
column: SsoConfigColumn
8085
): Promise<string | null | undefined> {
8186
return mapSecretFields(config, column, async (value) =>
82-
isEnvelope(value) ? value : (await encryptSecret(value)).encrypted
87+
isEnvelope(value) ? value : `${ENVELOPE_PREFIX}${(await encryptSecret(value)).encrypted}`
8388
)
8489
}
8590

8691
/**
8792
* Decrypts the secret fields of a stored provider config. Values written before
88-
* these fields were encrypted lack the envelope shape and are returned as-is.
93+
* these fields were encrypted lack the prefix and are returned as-is.
8994
*
9095
* A value that IS an envelope but fails to decrypt — a wrong or rotated
9196
* `ENCRYPTION_KEY`, a tampered row — throws rather than degrading to ciphertext.
@@ -99,7 +104,8 @@ export function decryptProviderConfig(
99104
return mapSecretFields(config, column, async (value) => {
100105
if (!isEnvelope(value)) return value
101106
try {
102-
return (await decryptSecret(value, { logFailure: false })).decrypted
107+
return (await decryptSecret(value.slice(ENVELOPE_PREFIX.length), { logFailure: false }))
108+
.decrypted
103109
} catch (error) {
104110
logger.error('Failed to decrypt an SSO provider secret', {
105111
column,

‎packages/db/scripts/register-sso-provider.ts‎

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -161,21 +161,26 @@ if (!ENCRYPTION_KEY || !/^[0-9a-f]{64}$/i.test(ENCRYPTION_KEY)) {
161161
const ENCRYPTION_KEY_BUFFER = Buffer.from(ENCRYPTION_KEY, 'hex')
162162

163163
/**
164-
* AES-256-GCM in the `iv:ciphertext:authTag` envelope the app reads back. This
164+
* AES-256-GCM in the prefixed envelope the app reads back — the prefix is what
165+
* marks a value as encrypted, so a secret that merely looks like one is not
166+
* mistaken for it. Keep it in step with `ENVELOPE_PREFIX` in
167+
* `apps/sim/lib/auth/sso/provider-secrets.ts`. This
165168
* package cannot import from `apps/*`, so the primitive is repeated here rather
166169
* than shared; {@link assertCryptoRoundTrip} proves the key produces a readable
167170
* value before any row is written.
168171
*/
172+
const ENVELOPE_PREFIX = 'sim.sso.v1:'
173+
169174
function encryptSecretValue(secret: string): string {
170175
const iv = randomBytes(16)
171176
const cipher = createCipheriv('aes-256-gcm', ENCRYPTION_KEY_BUFFER, iv, { authTagLength: 16 })
172177
let encrypted = cipher.update(secret, 'utf8', 'hex')
173178
encrypted += cipher.final('hex')
174-
return `${iv.toString('hex')}:${encrypted}:${cipher.getAuthTag().toString('hex')}`
179+
return `${ENVELOPE_PREFIX}${iv.toString('hex')}:${encrypted}:${cipher.getAuthTag().toString('hex')}`
175180
}
176181

177182
function decryptSecretValue(envelope: string): string {
178-
const [ivHex, ciphertext, authTagHex] = envelope.split(':')
183+
const [ivHex, ciphertext, authTagHex] = envelope.slice(ENVELOPE_PREFIX.length).split(':')
179184
const decipher = createDecipheriv(
180185
'aes-256-gcm',
181186
ENCRYPTION_KEY_BUFFER,

0 commit comments

Comments
 (0)