Skip to content

Commit 5f6bb43

Browse files
committed
fix(credentials): authorize shared credentials without requiring a workflow
`authorizeCredentialUse` could only reach its member-based sharing branch when a `workflowId` was supplied, so non-workflow surfaces — knowledge base connectors, credential management — fell through to an owner-only path and rejected everyone but the user who ran the OAuth flow. A workflow now only pins which workspace a legacy account id resolves through; it never grants access on its own. Access itself is decided by one rule everywhere: active credential member, or derived credential admin. - resolve legacy account ids through whichever workspace credential rows the caller can reach, instead of an owner-only fallback - extract `canUseCredential` and replace the predicate hand-inlined at five sites - reuse `resolveCredentialTokenIdentity` for owner resolution instead of a second local copy of the same invariant - keep the workflow-pinned path from crossing a workspace boundary
1 parent 1dd85eb commit 5f6bb43

9 files changed

Lines changed: 381 additions & 190 deletions

File tree

apps/sim/app/api/auth/oauth/credentials/route.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ import { getValidationErrorMessage } from '@/lib/api/server'
99
import { checkSessionOrInternalAuth } from '@/lib/auth/hybrid'
1010
import { generateRequestId } from '@/lib/core/utils/request'
1111
import { withRouteHandler } from '@/lib/core/utils/with-route-handler'
12-
import { getCredentialActorContext } from '@/lib/credentials/access'
12+
import { canUseCredential, getCredentialActorContext } from '@/lib/credentials/access'
1313
import { syncWorkspaceOAuthCredentialsForUser } from '@/lib/credentials/oauth'
1414
import {
1515
getCanonicalScopesForProvider,
@@ -155,7 +155,7 @@ export const GET = withRouteHandler(async (request: NextRequest) => {
155155

156156
if (!workflowId) {
157157
const access = await getCredentialActorContext(platformCredential.id, requesterUserId)
158-
if (!access.hasWorkspaceAccess || (!access.member && !access.isAdmin)) {
158+
if (!canUseCredential(access)) {
159159
return NextResponse.json({ error: 'Forbidden' }, { status: 403 })
160160
}
161161
}
@@ -187,7 +187,7 @@ export const GET = withRouteHandler(async (request: NextRequest) => {
187187
}
188188
} else {
189189
const access = await getCredentialActorContext(platformCredential.id, requesterUserId)
190-
if (!access.hasWorkspaceAccess || (!access.member && !access.isAdmin)) {
190+
if (!canUseCredential(access)) {
191191
return NextResponse.json({ error: 'Forbidden' }, { status: 403 })
192192
}
193193
}

apps/sim/app/api/credentials/[id]/route.ts

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,11 @@ import { updateWorkspaceCredentialContract } from '@/lib/api/contracts/credentia
44
import { getValidationErrorMessage, parseRequest } from '@/lib/api/server'
55
import { getSession } from '@/lib/auth'
66
import { withRouteHandler } from '@/lib/core/utils/with-route-handler'
7-
import { type CredentialActorContext, getCredentialActorContext } from '@/lib/credentials/access'
7+
import {
8+
type CredentialActorContext,
9+
canUseCredential,
10+
getCredentialActorContext,
11+
} from '@/lib/credentials/access'
812
import {
913
isProviderOutageCode,
1014
performDeleteCredential,
@@ -49,7 +53,7 @@ export const GET = withRouteHandler(
4953
if (!access.credential) {
5054
return NextResponse.json({ error: 'Credential not found' }, { status: 404 })
5155
}
52-
if (!access.hasWorkspaceAccess || (!access.member && !access.isAdmin)) {
56+
if (!canUseCredential(access)) {
5357
return NextResponse.json({ error: 'Forbidden' }, { status: 403 })
5458
}
5559

apps/sim/executor/handlers/evaluator/evaluator-handler.test.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,8 @@ const { mockResolveAutoModel } = vi.hoisted(() => ({
1111
vi.mock('@/lib/oauth/credential-service', () => authOAuthUtilsMock)
1212

1313
vi.mock('@/lib/credentials/access', () => ({
14+
canUseCredential: (access: { hasWorkspaceAccess: boolean; member: unknown; isAdmin: boolean }) =>
15+
access.hasWorkspaceAccess && (Boolean(access.member) || access.isAdmin),
1416
getCredentialActorContext: vi.fn().mockResolvedValue({
1517
credential: {
1618
id: 'test-vertex-credential-id',

apps/sim/executor/handlers/router/router-handler.test.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,8 @@ vi.mock('@/lib/oauth/credential-service', () => authOAuthUtilsMock)
1717
vi.mock('@/lib/core/security/encryption', () => encryptionMock)
1818

1919
vi.mock('@/lib/credentials/access', () => ({
20+
canUseCredential: (access: { hasWorkspaceAccess: boolean; member: unknown; isAdmin: boolean }) =>
21+
access.hasWorkspaceAccess && (Boolean(access.member) || access.isAdmin),
2022
getCredentialActorContext: vi.fn().mockResolvedValue({
2123
credential: {
2224
id: 'test-vertex-credential',

apps/sim/executor/utils/vertex-credential.test.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,8 @@ const { mockGetCredentialActorContext, mockGetServiceAccountToken, mockRefreshTo
1212

1313
vi.mock('@/lib/credentials/access', () => ({
1414
getCredentialActorContext: mockGetCredentialActorContext,
15+
canUseCredential: (access: { hasWorkspaceAccess: boolean; member: unknown; isAdmin: boolean }) =>
16+
access.hasWorkspaceAccess && (Boolean(access.member) || access.isAdmin),
1517
}))
1618
vi.mock('@/lib/oauth/credential-service', () => ({
1719
getServiceAccountToken: mockGetServiceAccountToken,

apps/sim/executor/utils/vertex-credential.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import { db } from '@sim/db'
22
import { account } from '@sim/db/schema'
33
import { createLogger } from '@sim/logger'
44
import { eq } from 'drizzle-orm'
5-
import { getCredentialActorContext } from '@/lib/credentials/access'
5+
import { canUseCredential, getCredentialActorContext } from '@/lib/credentials/access'
66
import { getServiceAccountToken, refreshTokenIfNeeded } from '@/lib/oauth/credential-service'
77

88
const logger = createLogger('VertexCredential')
@@ -48,7 +48,7 @@ export async function resolveVertexCredential({
4848
})
4949
throw new Error('Credential is not accessible from this workflow workspace')
5050
}
51-
if (!access.hasWorkspaceAccess || (!access.member && !access.isAdmin)) {
51+
if (!canUseCredential(access)) {
5252
throw new Error('Not authorized to use this Vertex AI credential')
5353
}
5454

Lines changed: 238 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,238 @@
1+
/**
2+
* @vitest-environment node
3+
*/
4+
import { account, credential, credentialMember, workflow } from '@sim/db/schema'
5+
import { createMockRequest, queueTableRows, resetDbChainMock } from '@sim/testing'
6+
import { afterAll, beforeEach, describe, expect, it, vi } from 'vitest'
7+
8+
const { mockCheckSessionOrInternalAuth, mockResolveWorkspaceAccess, mockGetUserEntityPermissions } =
9+
vi.hoisted(() => ({
10+
mockCheckSessionOrInternalAuth: vi.fn(),
11+
mockResolveWorkspaceAccess: vi.fn(),
12+
mockGetUserEntityPermissions: vi.fn(),
13+
}))
14+
15+
vi.mock('@/lib/auth/hybrid', () => ({
16+
AuthType: { SESSION: 'session', API_KEY: 'api_key', INTERNAL_JWT: 'internal_jwt' },
17+
checkSessionOrInternalAuth: mockCheckSessionOrInternalAuth,
18+
}))
19+
20+
vi.mock('@/lib/workspaces/permissions/utils', () => ({
21+
checkWorkspaceAccess: mockResolveWorkspaceAccess,
22+
getUserEntityPermissions: mockGetUserEntityPermissions,
23+
resolveWorkspaceAccess: mockResolveWorkspaceAccess,
24+
}))
25+
26+
import { authorizeCredentialUse } from '@/lib/auth/credential-access'
27+
28+
afterAll(resetDbChainMock)
29+
30+
const OWNER = 'owner-user'
31+
const WORKSPACE = 'ws-1'
32+
const ACCOUNT_ID = 'acct-1'
33+
34+
const workspaceAdmin = { hasAccess: true, canWrite: true, canAdmin: true }
35+
const workspaceWriter = { hasAccess: true, canWrite: true, canAdmin: false }
36+
const noWorkspaceAccess = { hasAccess: false, canWrite: false, canAdmin: false }
37+
38+
const platformCredential = {
39+
id: 'cred-1',
40+
workspaceId: WORKSPACE,
41+
type: 'oauth',
42+
accountId: ACCOUNT_ID,
43+
}
44+
45+
function actAs(userId: string) {
46+
mockCheckSessionOrInternalAuth.mockResolvedValue({ success: true, userId, authType: 'session' })
47+
}
48+
49+
/** The rows `getCredentialActorContext` reads: the credential, then the caller's membership. */
50+
function queueActorContext(
51+
credentialRow: Record<string, unknown>,
52+
membership: { role: string }[] = []
53+
) {
54+
queueTableRows(credential, [credentialRow])
55+
queueTableRows(credentialMember, membership)
56+
}
57+
58+
/** The rows `resolveCredentialTokenIdentity` reads: the credential, then its account. */
59+
function queueTokenIdentity(
60+
credentialRow: Record<string, unknown> | null,
61+
ownerUserId: string | null
62+
) {
63+
queueTableRows(credential, credentialRow ? [credentialRow] : [])
64+
queueTableRows(account, ownerUserId ? [{ userId: ownerUserId }] : [])
65+
}
66+
67+
function authorize(credentialId: string, workflowId?: string) {
68+
return authorizeCredentialUse(createMockRequest('POST'), { credentialId, workflowId })
69+
}
70+
71+
describe('authorizeCredentialUse', () => {
72+
beforeEach(() => {
73+
vi.clearAllMocks()
74+
resetDbChainMock()
75+
actAs('acting-user')
76+
mockGetUserEntityPermissions.mockResolvedValue('admin')
77+
mockResolveWorkspaceAccess.mockResolvedValue(workspaceWriter)
78+
})
79+
80+
describe('workspace-scoped credentials, without a workflow', () => {
81+
it('authorizes a workspace admin who did not run the OAuth flow', async () => {
82+
queueActorContext(platformCredential)
83+
queueTokenIdentity(platformCredential, OWNER)
84+
mockResolveWorkspaceAccess.mockResolvedValue(workspaceAdmin)
85+
86+
const result = await authorize('cred-1')
87+
88+
expect(result.ok).toBe(true)
89+
expect(result.credentialOwnerUserId).toBe(OWNER)
90+
expect(result.resolvedCredentialId).toBe(ACCOUNT_ID)
91+
expect(result.workspaceId).toBe(WORKSPACE)
92+
})
93+
94+
it('authorizes an active credential member who did not run the OAuth flow', async () => {
95+
queueActorContext(platformCredential, [{ role: 'member' }])
96+
queueTokenIdentity(platformCredential, OWNER)
97+
98+
const result = await authorize('cred-1')
99+
100+
expect(result.ok).toBe(true)
101+
expect(result.credentialOwnerUserId).toBe(OWNER)
102+
})
103+
104+
it('rejects a workspace member who is not a credential member', async () => {
105+
queueActorContext(platformCredential)
106+
107+
const result = await authorize('cred-1')
108+
109+
expect(result.ok).toBe(false)
110+
expect(result.error).toContain('add you as a member')
111+
})
112+
113+
it('rejects a caller who has lost access to the credential workspace', async () => {
114+
queueActorContext(platformCredential)
115+
mockResolveWorkspaceAccess.mockResolvedValue(noWorkspaceAccess)
116+
117+
const result = await authorize('cred-1')
118+
119+
expect(result.ok).toBe(false)
120+
expect(result.error).toBe('You do not have access to this workspace.')
121+
})
122+
123+
it('rejects when the credential owner has lost access to the workspace', async () => {
124+
queueActorContext(platformCredential)
125+
queueTokenIdentity(platformCredential, OWNER)
126+
mockResolveWorkspaceAccess.mockResolvedValue(workspaceAdmin)
127+
mockGetUserEntityPermissions.mockResolvedValue(null)
128+
129+
const result = await authorize('cred-1')
130+
131+
expect(result.ok).toBe(false)
132+
expect(result.error).toBe('Unauthorized')
133+
})
134+
})
135+
136+
describe('workflow scope', () => {
137+
it('rejects a credential belonging to another workspace', async () => {
138+
queueTableRows(workflow, [{ workspaceId: 'other-ws' }])
139+
queueActorContext(platformCredential)
140+
141+
const result = await authorize('cred-1', 'wf-1')
142+
143+
expect(result.ok).toBe(false)
144+
expect(result.error).toBe('Credential is not accessible from this workflow workspace')
145+
})
146+
})
147+
148+
describe('legacy account ids', () => {
149+
const sharedRow = { id: 'cred-1', workspaceId: WORKSPACE, type: 'oauth' }
150+
151+
it('resolves through an accessible workspace credential without a workflow', async () => {
152+
queueTableRows(credential, []) // platform lookup miss
153+
queueTableRows(credential, [sharedRow]) // shared rows wrapping the account
154+
queueActorContext(sharedRow)
155+
queueTokenIdentity(null, OWNER)
156+
mockResolveWorkspaceAccess.mockResolvedValue(workspaceAdmin)
157+
158+
const result = await authorize(ACCOUNT_ID)
159+
160+
expect(result.ok).toBe(true)
161+
expect(result.credentialOwnerUserId).toBe(OWNER)
162+
expect(result.workspaceId).toBe(WORKSPACE)
163+
expect(result.resolvedCredentialId).toBe(ACCOUNT_ID)
164+
})
165+
166+
it('rejects when no workspace credential is reachable by the caller', async () => {
167+
queueTableRows(credential, [])
168+
queueTableRows(credential, [sharedRow])
169+
queueActorContext(sharedRow)
170+
queueTableRows(account, [{ userId: OWNER }])
171+
172+
const result = await authorize(ACCOUNT_ID)
173+
174+
expect(result.ok).toBe(false)
175+
expect(result.error).toContain('add you as a member')
176+
})
177+
178+
it('still authorizes the owner when a shared row rejects them', async () => {
179+
actAs(OWNER)
180+
queueTableRows(credential, [])
181+
queueTableRows(credential, [{ id: 'cred-1', workspaceId: 'other-ws', type: 'oauth' }])
182+
queueActorContext({ id: 'cred-1', workspaceId: 'other-ws', type: 'oauth' })
183+
queueTableRows(account, [{ userId: OWNER }])
184+
185+
const result = await authorize(ACCOUNT_ID)
186+
187+
expect(result.ok).toBe(true)
188+
expect(result.credentialOwnerUserId).toBe(OWNER)
189+
})
190+
191+
it('does not fall back to the owner path when a workflow pins the workspace', async () => {
192+
actAs(OWNER)
193+
queueTableRows(workflow, [{ workspaceId: WORKSPACE }])
194+
queueTableRows(credential, [])
195+
queueTableRows(credential, [])
196+
queueTableRows(account, [{ userId: OWNER }])
197+
198+
const result = await authorize(ACCOUNT_ID, 'wf-1')
199+
200+
expect(result.ok).toBe(false)
201+
expect(result.error).toBe('Credential not found')
202+
})
203+
204+
it('keeps an unshared account private to its owner', async () => {
205+
queueTableRows(credential, [])
206+
queueTableRows(credential, [])
207+
queueTableRows(account, [{ userId: OWNER }])
208+
209+
const result = await authorize(ACCOUNT_ID)
210+
211+
expect(result.ok).toBe(false)
212+
expect(result.error).toBe('Unauthorized')
213+
})
214+
215+
it('authorizes the owner of an unshared account', async () => {
216+
actAs(OWNER)
217+
queueTableRows(credential, [])
218+
queueTableRows(credential, [])
219+
queueTableRows(account, [{ userId: OWNER }])
220+
221+
const result = await authorize(ACCOUNT_ID)
222+
223+
expect(result.ok).toBe(true)
224+
expect(result.credentialOwnerUserId).toBe(OWNER)
225+
})
226+
227+
it('reports an unknown credential id', async () => {
228+
queueTableRows(credential, [])
229+
queueTableRows(credential, [])
230+
queueTableRows(account, [])
231+
232+
const result = await authorize('nope')
233+
234+
expect(result.ok).toBe(false)
235+
expect(result.error).toBe('Credential not found')
236+
})
237+
})
238+
})

0 commit comments

Comments
 (0)