Skip to content

Commit 063ff6e

Browse files
fix(credentials): revalidate policy before grant persistence
1 parent 45bcf01 commit 063ff6e

2 files changed

Lines changed: 127 additions & 3 deletions

File tree

apps/sim/lib/credential-groups/oauth.test.ts

Lines changed: 77 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,13 @@
11
/**
22
* @vitest-environment node
33
*/
4-
import { dbChainMockFns, resetDbChainMock } from '@sim/testing'
4+
import {
5+
dbChainMock,
6+
dbChainMockFns,
7+
queueTableRows,
8+
resetDbChainMock,
9+
schemaMock,
10+
} from '@sim/testing'
511
import { beforeEach, describe, expect, it, vi } from 'vitest'
612

713
const { adapter } = vi.hoisted(() => ({
@@ -54,6 +60,18 @@ const CONTEXT = {
5460
options: [],
5561
}
5662

63+
const GROUP = {
64+
status: 'active' as const,
65+
options: [
66+
{
67+
...CONTEXT.option,
68+
authorizationAppId: POLICY.authorizationAppId,
69+
requiredScopes: POLICY.requiredScopes,
70+
scopeVersion: POLICY.scopeVersion,
71+
},
72+
],
73+
}
74+
5775
describe('credential group OAuth persistence', () => {
5876
beforeEach(() => {
5977
vi.clearAllMocks()
@@ -104,7 +122,9 @@ describe('credential group OAuth persistence', () => {
104122
})
105123

106124
it('preserves completed enrollment state when an account reconnects', async () => {
107-
dbChainMockFns.limit.mockResolvedValueOnce([{ status: 'completed' }]).mockResolvedValueOnce([
125+
dbChainMockFns.limit.mockResolvedValueOnce([{ status: 'completed' }])
126+
queueTableRows(schemaMock.credentialGroup, [GROUP])
127+
queueTableRows(schemaMock.credential, [
108128
{
109129
id: 'credential-1',
110130
providerSubjectId: 'google-subject-1',
@@ -142,4 +162,59 @@ describe('credential group OAuth persistence', () => {
142162
)
143163
expect(enrollmentUpdate).not.toHaveProperty('completedAt')
144164
})
165+
166+
it('rejects an exchanged grant when the group policy changed before persistence', async () => {
167+
const nextPolicy = {
168+
...POLICY,
169+
requiredScopes: [...POLICY.requiredScopes, 'https://www.googleapis.com/auth/gmail.readonly'],
170+
scopeVersion: 2,
171+
}
172+
adapter.getPolicy.mockResolvedValueOnce(POLICY).mockResolvedValueOnce(nextPolicy)
173+
dbChainMockFns.limit.mockResolvedValueOnce([{ status: 'completed' }])
174+
queueTableRows(schemaMock.credentialGroup, [
175+
{
176+
...GROUP,
177+
options: [
178+
{
179+
...GROUP.options[0],
180+
requiredScopes: nextPolicy.requiredScopes,
181+
scopeVersion: nextPolicy.scopeVersion,
182+
},
183+
],
184+
},
185+
])
186+
187+
await expect(
188+
completeCredentialGroupOAuth(
189+
{ ...CONTEXT, enrollmentStatus: 'completed' },
190+
{
191+
state: 'state-1',
192+
provider: 'gmail',
193+
nonceHash: 'nonce-hash',
194+
enrollmentId: CONTEXT.enrollmentId,
195+
credentialGroupId: CONTEXT.credentialGroupId,
196+
optionId: CONTEXT.option.id,
197+
authorizationAppId: POLICY.authorizationAppId,
198+
scopeVersion: POLICY.scopeVersion,
199+
requiredScopes: POLICY.requiredScopes,
200+
redirectUri: 'https://sim.ai/api/credential-groups/oauth/gmail/callback',
201+
codeVerifier: 'verifier',
202+
invitationToken: 'invitation-token',
203+
createdAt: Date.now(),
204+
},
205+
'authorization-code'
206+
)
207+
).rejects.toThrow('This credential option changed.')
208+
209+
expect(adapter.getPolicy).toHaveBeenLastCalledWith(
210+
expect.objectContaining({ id: 'option-1' }),
211+
{
212+
workspaceId: CONTEXT.workspaceId,
213+
credentialGroupId: CONTEXT.credentialGroupId,
214+
executor: dbChainMock.db,
215+
}
216+
)
217+
expect(dbChainMockFns.update).not.toHaveBeenCalled()
218+
expect(dbChainMockFns.insert).not.toHaveBeenCalled()
219+
})
145220
})

apps/sim/lib/credential-groups/oauth.ts

Lines changed: 50 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { db } from '@sim/db'
2-
import { credential, credentialGroupEnrollment } from '@sim/db/schema'
2+
import { credential, credentialGroup, credentialGroupEnrollment } from '@sim/db/schema'
33
import { generateId } from '@sim/utils/id'
44
import { and, eq, ne, sql } from 'drizzle-orm'
55
import {
@@ -35,6 +35,19 @@ function scopesEqual(left: string[], right: string[]): boolean {
3535
)
3636
}
3737

38+
function policiesEqual(
39+
left: CredentialGroupProviderPolicy,
40+
right: CredentialGroupProviderPolicy
41+
): boolean {
42+
return (
43+
left.provider === right.provider &&
44+
left.providerId === right.providerId &&
45+
left.authorizationAppId === right.authorizationAppId &&
46+
left.scopeVersion === right.scopeVersion &&
47+
scopesEqual(left.requiredScopes, right.requiredScopes)
48+
)
49+
}
50+
3851
function getOptionAdapter(context: CredentialGroupOAuthContext): CredentialGroupProviderAdapter {
3952
if (!isCredentialGroupProvider(context.option.provider)) {
4053
throw new Error(`Unsupported Credential Group provider: ${context.option.provider}`)
@@ -114,6 +127,42 @@ async function persistGrant(
114127
throw new CredentialGroupOAuthError('This account invitation was revoked.', 409)
115128
}
116129

130+
const [group] = await tx
131+
.select({ status: credentialGroup.status, options: credentialGroup.options })
132+
.from(credentialGroup)
133+
.where(
134+
and(
135+
eq(credentialGroup.id, context.credentialGroupId),
136+
eq(credentialGroup.workspaceId, context.workspaceId)
137+
)
138+
)
139+
.limit(1)
140+
.for('update')
141+
const currentOption = group?.options.find((option) => option.id === context.option.id)
142+
if (
143+
!group ||
144+
group.status !== 'active' ||
145+
!currentOption ||
146+
currentOption.status !== 'active' ||
147+
currentOption.provider !== adapter.provider
148+
) {
149+
throw new CredentialGroupOAuthError(
150+
'This credential option changed. Reload the invitation and try again.',
151+
409
152+
)
153+
}
154+
const currentPolicy = await adapter.getPolicy(currentOption, {
155+
workspaceId: context.workspaceId,
156+
credentialGroupId: context.credentialGroupId,
157+
executor: tx,
158+
})
159+
if (!policiesEqual(currentPolicy, policy)) {
160+
throw new CredentialGroupOAuthError(
161+
'This credential option changed. Reload the invitation and try again.',
162+
409
163+
)
164+
}
165+
117166
const [existing] = await tx
118167
.select({
119168
id: credential.id,

0 commit comments

Comments
 (0)