Skip to content

Commit 531cb5b

Browse files
fix(auth): harden subject delegation and cookies
1 parent 266f4fc commit 531cb5b

4 files changed

Lines changed: 104 additions & 8 deletions

File tree

apps/sim/lib/auth/internal.test.ts

Lines changed: 72 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,9 +2,11 @@
22
* @vitest-environment node
33
*/
44

5+
import { serializePrincipal } from '@sim/auth/principal'
56
import { resetEnvMock } from '@sim/testing'
6-
import { decodeJwt } from 'jose'
7+
import { decodeJwt, SignJWT } from 'jose'
78
import { afterAll, describe, expect, it, vi } from 'vitest'
9+
import { env } from '@/lib/core/config/env'
810

911
vi.unmock('@/lib/auth/internal')
1012

@@ -135,7 +137,32 @@ describe('internal executor delegation claims', () => {
135137
})
136138
})
137139

138-
it('rejects laundering actorless or external principals into a Sim user subject', async () => {
140+
it('round-trips an authenticated chat subject without inventing a Sim user', async () => {
141+
const token = await generateInternalDelegationToken({
142+
workflowId: 'workflow-1',
143+
principal: {
144+
kind: 'system',
145+
serviceId: 'chat',
146+
workspaceId: 'workspace-1',
147+
workflowId: 'workflow-1',
148+
subject: { kind: 'authenticated_email', email: 'person@example.com' },
149+
},
150+
})
151+
152+
await expect(verifyInternalDelegationToken(token)).resolves.toMatchObject({
153+
workflowId: 'workflow-1',
154+
principal: {
155+
kind: 'system',
156+
serviceId: 'chat',
157+
workspaceId: 'workspace-1',
158+
workflowId: 'workflow-1',
159+
subject: { kind: 'authenticated_email', email: 'person@example.com' },
160+
},
161+
})
162+
expect(decodeJwt(token).sub).toBeUndefined()
163+
})
164+
165+
it('rejects laundering actorless or non-Sim principals into a Sim user subject', async () => {
139166
await expect(
140167
generateInternalDelegationToken({
141168
subjectUserId: 'billing-owner',
@@ -167,7 +194,49 @@ describe('internal executor delegation claims', () => {
167194
},
168195
},
169196
})
170-
).rejects.toThrow('External workflow subjects cannot be represented as Sim users')
197+
).rejects.toThrow('Non-Sim workflow subjects cannot be represented as Sim users')
198+
199+
await expect(
200+
generateInternalDelegationToken({
201+
subjectUserId: 'unrelated-user',
202+
workflowId: 'workflow-1',
203+
principal: {
204+
kind: 'system',
205+
serviceId: 'chat',
206+
workspaceId: 'workspace-1',
207+
workflowId: 'workflow-1',
208+
subject: { kind: 'authenticated_email', email: 'person@example.com' },
209+
},
210+
})
211+
).rejects.toThrow('Non-Sim workflow subjects cannot be represented as Sim users')
212+
})
213+
214+
it('rejects a signed delegation that pairs a non-Sim principal with a Sim user subject', async () => {
215+
const issuedAt = Math.floor(Date.now() / 1000)
216+
const token = await new SignJWT({
217+
type: 'internal_delegation',
218+
serviceId: 'executor',
219+
workflowId: 'workflow-1',
220+
principal: serializePrincipal({
221+
kind: 'system',
222+
serviceId: 'chat',
223+
workspaceId: 'workspace-1',
224+
workflowId: 'workflow-1',
225+
subject: { kind: 'authenticated_email', email: 'person@example.com' },
226+
}),
227+
})
228+
.setProtectedHeader({ alg: 'HS256' })
229+
.setJti('delegation-1')
230+
.setSubject('unrelated-user')
231+
.setIssuedAt(issuedAt)
232+
.setExpirationTime(issuedAt + 5 * 60)
233+
.setIssuer('sim-internal')
234+
.setAudience('sim-api')
235+
.sign(new TextEncoder().encode(env.INTERNAL_API_SECRET))
236+
237+
await expect(verifyInternalDelegationToken(token)).rejects.toBeInstanceOf(
238+
InvalidInternalDelegationTokenError
239+
)
171240
})
172241

173242
it('derives issued-at and expiry from one timestamp', async () => {

apps/sim/lib/auth/internal.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -107,8 +107,8 @@ export async function generateInternalDelegationToken(
107107
? requireNonEmptyDelegationClaim(input.subjectUserId, 'subjectUserId')
108108
: undefined
109109
const principalSubject = input.principal ? resolvePrincipalSubject(input.principal) : null
110-
if (principalSubject?.kind === 'external_user' && suppliedSubjectUserId) {
111-
throw new Error('External workflow subjects cannot be represented as Sim users')
110+
if (principalSubject && principalSubject.kind !== 'sim_user' && suppliedSubjectUserId) {
111+
throw new Error('Non-Sim workflow subjects cannot be represented as Sim users')
112112
}
113113
if (!principalSubject && input.principal && suppliedSubjectUserId) {
114114
throw new Error('Actorless workflow principals cannot be represented as Sim users')
@@ -204,7 +204,7 @@ export async function verifyInternalDelegationToken(
204204
if (
205205
(!principal && !subjectUserId) ||
206206
(principalSubject?.kind === 'sim_user' && principalSubject.userId !== subjectUserId) ||
207-
(principalSubject?.kind === 'external_user' && subjectUserId) ||
207+
(principalSubject && principalSubject.kind !== 'sim_user' && subjectUserId) ||
208208
(principal && !principalSubject && subjectUserId)
209209
) {
210210
throw new InvalidInternalDelegationTokenError()

apps/sim/lib/core/security/deployment.test.ts

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,12 @@
11
/**
22
* @vitest-environment node
33
*/
4+
5+
import { sha256Hex } from '@sim/security/hash'
6+
import { hmacSha256Hex } from '@sim/security/hmac'
47
import type { NextResponse } from 'next/server'
58
import { describe, expect, it, vi } from 'vitest'
9+
import { env } from '@/lib/core/config/env'
610
import {
711
isEmailAllowed,
812
readDeploymentAuthToken,
@@ -20,6 +24,17 @@ function mintDeploymentAuthToken(
2024
return set.mock.calls[0][0].value
2125
}
2226

27+
function mintLegacyDeploymentAuthToken(
28+
deploymentId: string,
29+
authType: string,
30+
encryptedPassword?: string
31+
): string {
32+
const passwordSlot = encryptedPassword ? sha256Hex(encryptedPassword).slice(0, 8) : ''
33+
const payload = `${deploymentId}:${authType}:${Date.now()}:${passwordSlot}`
34+
const signature = hmacSha256Hex(payload, env.BETTER_AUTH_SECRET)
35+
return Buffer.from(`${payload}:${signature}`).toString('base64')
36+
}
37+
2338
describe('deployment auth tokens', () => {
2439
it('round-trips the normalized email proven by OTP authentication', () => {
2540
const token = mintDeploymentAuthToken('chat-1', 'email', ' Person@Example.com ')
@@ -35,6 +50,18 @@ describe('deployment auth tokens', () => {
3550
expect(readDeploymentAuthToken(token, 'chat-1', 'password')).toEqual({})
3651
})
3752

53+
it('accepts a valid legacy password token without inventing identity', () => {
54+
const token = mintLegacyDeploymentAuthToken('chat-1', 'password', 'encrypted-password')
55+
56+
expect(readDeploymentAuthToken(token, 'chat-1', 'password', 'encrypted-password')).toEqual({})
57+
})
58+
59+
it('rejects a legacy email token that cannot prove an email identity', () => {
60+
const token = mintLegacyDeploymentAuthToken('chat-1', 'email')
61+
62+
expect(readDeploymentAuthToken(token, 'chat-1', 'email')).toBeNull()
63+
})
64+
3865
it('rejects a token outside its bound deployment and authentication type', () => {
3966
const token = mintDeploymentAuthToken('chat-1', 'email', 'person@example.com')
4067

apps/sim/lib/core/security/deployment.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -67,8 +67,8 @@ export function readDeploymentAuthToken(
6767
if (!safeCompare(sig, signPayload(payload))) return null
6868

6969
const parts = payload.split(':')
70-
if (parts.length !== 5) return null
71-
const [storedId, storedType, timestamp, storedPwSlot, storedEmailSlot] = parts
70+
if (parts.length !== 4 && parts.length !== 5) return null
71+
const [storedId, storedType, timestamp, storedPwSlot, storedEmailSlot = ''] = parts
7272

7373
if (storedId !== deploymentId || storedType !== authType) return null
7474
if (storedPwSlot !== passwordSlot(encryptedPassword)) return null

0 commit comments

Comments
 (0)