Skip to content

Commit b34b81d

Browse files
Bill LeoutsakosBill Leoutsakos
Bill Leoutsakos
authored and
Bill Leoutsakos
committed
refactor(oci): isolate credential handoff
1 parent b33c42c commit b34b81d

6 files changed

Lines changed: 37 additions & 155 deletions

File tree

‎apps/sim/app/api/auth/oauth/token/route.test.ts‎

Lines changed: 8 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -256,23 +256,19 @@ describe('OAuth Token API Routes', () => {
256256
describe('service account path', () => {
257257
it('threads the NetSuite SuiteTalk instance URL into the token response', async () => {
258258
const instanceUrl = 'https://1234567.suitetalk.api.netsuite.com'
259-
const resolvedCredential = {
259+
authOAuthUtilsMockFns.mockResolveOAuthAccountId.mockResolvedValueOnce({
260260
accountId: '',
261261
credentialId: 'netsuite-credential-id',
262262
credentialType: 'service_account',
263263
providerId: 'netsuite-service-account',
264264
workspaceId: 'workspace-id',
265265
usedCredentialTable: true,
266-
} as const
267-
authOAuthUtilsMockFns.mockResolveOAuthAccountId
268-
.mockResolvedValueOnce(resolvedCredential)
269-
.mockResolvedValueOnce(resolvedCredential)
266+
})
270267
mockAuthorizeCredentialUse.mockResolvedValueOnce({
271268
ok: true,
272269
authType: 'session',
273270
requesterUserId: 'test-user-id',
274271
workspaceId: 'workspace-id',
275-
resolvedCredentialId: 'netsuite-credential-id',
276272
})
277273
mockResolveServiceAccountToken.mockResolvedValueOnce({
278274
accessToken: 'netsuite-token',
@@ -289,23 +285,19 @@ describe('OAuth Token API Routes', () => {
289285
})
290286

291287
it('should thread authStyle from the resolver into the response', async () => {
292-
const resolvedCredential = {
288+
authOAuthUtilsMockFns.mockResolveOAuthAccountId.mockResolvedValueOnce({
293289
accountId: '',
294290
credentialId: 'sa-credential-id',
295291
credentialType: 'service_account',
296292
providerId: 'pipedrive-service-account',
297293
workspaceId: 'workspace-id',
298294
usedCredentialTable: true,
299-
} as const
300-
authOAuthUtilsMockFns.mockResolveOAuthAccountId
301-
.mockResolvedValueOnce(resolvedCredential)
302-
.mockResolvedValueOnce(resolvedCredential)
295+
})
303296
mockAuthorizeCredentialUse.mockResolvedValueOnce({
304297
ok: true,
305298
authType: 'session',
306299
requesterUserId: 'test-user-id',
307300
workspaceId: 'workspace-id',
308-
resolvedCredentialId: 'sa-credential-id',
309301
})
310302
mockResolveServiceAccountToken.mockResolvedValueOnce({
311303
accessToken: 'pasted-api-token',
@@ -323,23 +315,19 @@ describe('OAuth Token API Routes', () => {
323315
})
324316

325317
it('should omit authStyle for Bearer token-paste providers', async () => {
326-
const resolvedCredential = {
318+
authOAuthUtilsMockFns.mockResolveOAuthAccountId.mockResolvedValueOnce({
327319
accountId: '',
328320
credentialId: 'sa-credential-id',
329321
credentialType: 'service_account',
330322
providerId: 'hubspot-service-account',
331323
workspaceId: 'workspace-id',
332324
usedCredentialTable: true,
333-
} as const
334-
authOAuthUtilsMockFns.mockResolveOAuthAccountId
335-
.mockResolvedValueOnce(resolvedCredential)
336-
.mockResolvedValueOnce(resolvedCredential)
325+
})
337326
mockAuthorizeCredentialUse.mockResolvedValueOnce({
338327
ok: true,
339328
authType: 'session',
340329
requesterUserId: 'test-user-id',
341330
workspaceId: 'workspace-id',
342-
resolvedCredentialId: 'sa-credential-id',
343331
})
344332
mockResolveServiceAccountToken.mockResolvedValueOnce({
345333
accessToken: 'pat-token',
@@ -362,23 +350,19 @@ describe('OAuth Token API Routes', () => {
362350
] as const)(
363351
'surfaces the %s error code with status %i when the mint fails',
364352
async (code, status) => {
365-
const resolvedCredential = {
353+
authOAuthUtilsMockFns.mockResolveOAuthAccountId.mockResolvedValueOnce({
366354
accountId: '',
367355
credentialId: 'sa-credential-id',
368356
credentialType: 'service_account',
369357
providerId: 'salesforce-service-account',
370358
workspaceId: 'workspace-id',
371359
usedCredentialTable: true,
372-
} as const
373-
authOAuthUtilsMockFns.mockResolveOAuthAccountId
374-
.mockResolvedValueOnce(resolvedCredential)
375-
.mockResolvedValueOnce(resolvedCredential)
360+
})
376361
mockAuthorizeCredentialUse.mockResolvedValueOnce({
377362
ok: true,
378363
authType: 'session',
379364
requesterUserId: 'test-user-id',
380365
workspaceId: 'workspace-id',
381-
resolvedCredentialId: 'sa-credential-id',
382366
})
383367
mockResolveServiceAccountToken.mockRejectedValueOnce(
384368
new TokenServiceAccountValidationError(code, status, { step: 'mint' })

‎apps/sim/lib/oauth/token-resolution.test.ts‎

Lines changed: 1 addition & 59 deletions
Original file line numberDiff line numberDiff line change
@@ -287,20 +287,7 @@ describe('resolveCredentialToken', () => {
287287
})
288288

289289
it('surfaces the classified service-account failure code', async () => {
290-
mockAuthorizeCredentialUseForAuth.mockResolvedValue({
291-
ok: true,
292-
requesterUserId: 'user-1',
293-
workspaceId: 'ws-1',
294-
resolvedCredentialId: 'sa-authoritative',
295-
})
296-
mockResolveOAuthAccountId.mockResolvedValue({
297-
credentialType: 'service_account',
298-
credentialId: 'sa-authoritative',
299-
providerId: 'atlassian',
300-
workspaceId: 'ws-1',
301-
accountId: '',
302-
usedCredentialTable: true,
303-
})
290+
mockAuthorizeCredentialUseForAuth.mockResolvedValue({ ok: true, requesterUserId: 'user-1' })
304291
mockResolveServiceAccountToken.mockRejectedValue(
305292
new TokenServiceAccountValidationError('invalid_credentials', 401)
306293
)
@@ -324,12 +311,6 @@ describe('resolveCredentialToken', () => {
324311
code: 'invalid_credentials',
325312
error: 'Credential rejected by the provider — reconnect the credential',
326313
})
327-
expect(mockResolveServiceAccountToken).toHaveBeenCalledWith(
328-
'sa-authoritative',
329-
'atlassian',
330-
[],
331-
undefined
332-
)
333314
})
334315

335316
it('rejects a malformed impersonation subject before touching the credential', async () => {
@@ -518,45 +499,6 @@ describe('resolveCredentialAccessToken', () => {
518499
expect(mockResolveServiceAccountToken).not.toHaveBeenCalled()
519500
})
520501

521-
it('cannot use a non-OCI alias to bypass trusted OCI tool metadata checks', async () => {
522-
const supplied = {
523-
credentialType: 'service_account',
524-
credentialId: 'caller-controlled-alias',
525-
providerId: 'google-service-account',
526-
workspaceId: 'ws-1',
527-
accountId: '',
528-
usedCredentialTable: true,
529-
} as const
530-
const authoritative = {
531-
...supplied,
532-
credentialId: 'credential-authoritative',
533-
providerId: 'oci-api-key-service-account',
534-
} as const
535-
mockResolveOAuthAccountId.mockResolvedValueOnce(supplied).mockResolvedValueOnce(authoritative)
536-
mockGetToolMetadata.mockReturnValue({
537-
oauth: { required: true, provider: 'slack', credentialKind: 'service-account' },
538-
})
539-
mockGetServiceConfigByServiceId.mockReturnValue({
540-
serviceAccountProviderId: 'slack-custom-bot',
541-
})
542-
mockAuthorizeCredentialUseForAuth.mockResolvedValue({
543-
ok: true,
544-
requesterUserId: 'user-1',
545-
workspaceId: 'ws-1',
546-
resolvedCredentialId: 'credential-authoritative',
547-
})
548-
549-
await expect(
550-
resolveCredentialAccessToken({
551-
requestId: 'req-oci',
552-
credentialId: 'caller-controlled-alias',
553-
toolId: 'non_oci_tool',
554-
authenticate,
555-
})
556-
).resolves.toEqual({ ok: false, status: 403, error: 'Unauthorized' })
557-
expect(mockResolveServiceAccountToken).not.toHaveBeenCalled()
558-
})
559-
560502
it('rejects a managed credential when no delegation resolver is wired', async () => {
561503
mockResolveOAuthAccountId.mockResolvedValue(MANAGED_RESOLVED)
562504

‎apps/sim/lib/oauth/token-resolution.ts‎

Lines changed: 25 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -60,8 +60,8 @@ export interface ResolveCredentialTokenInput {
6060
auditRequest?: CredentialAuditRequest
6161
/** Credential lookup already performed by {@link resolveCredentialAccessToken}'s dispatch. */
6262
resolvedCredential: ResolvedCredential | null
63-
/** Trusted provider binding derived from registered tool metadata. */
64-
expectedServiceAccountProviderId?: string
63+
/** Trusted OCI provider binding derived from registered tool metadata. */
64+
expectedServiceAccountProviderId?: typeof OCI_API_KEY_SERVICE_ACCOUNT_PROVIDER_ID
6565
}
6666

6767
export type ResolveCredentialTokenResult =
@@ -215,27 +215,31 @@ export async function resolveCredentialToken(
215215
return { ok: false, status: 403, error: authz.error || 'Unauthorized' }
216216
}
217217

218-
const authoritativeId = authz.resolvedCredentialId
219-
if (!authoritativeId) return { ok: false, status: 403, error: 'Unauthorized' }
220-
const authoritative = await resolveOAuthAccountId(authoritativeId)
221-
if (
222-
authoritative?.credentialType !== 'service_account' ||
223-
authoritative.credentialId !== authoritativeId ||
224-
(authoritative.providerId === OCI_API_KEY_SERVICE_ACCOUNT_PROVIDER_ID &&
225-
input.expectedServiceAccountProviderId !== OCI_API_KEY_SERVICE_ACCOUNT_PROVIDER_ID) ||
226-
(input.expectedServiceAccountProviderId !== undefined &&
227-
authoritative.providerId !== input.expectedServiceAccountProviderId)
228-
) {
229-
return { ok: false, status: 403, error: 'Unauthorized' }
230-
}
231-
218+
let serviceAccountCredentialId = resolved.credentialId
219+
let serviceAccountProviderId = resolved.providerId
232220
const saActorId = authz.requesterUserId
233-
const saWorkspaceId = authz.workspaceId ?? null
221+
let saWorkspaceId = resolved.workspaceId ?? authz.workspaceId ?? null
222+
223+
if (input.expectedServiceAccountProviderId === OCI_API_KEY_SERVICE_ACCOUNT_PROVIDER_ID) {
224+
const authoritativeId = authz.resolvedCredentialId
225+
if (!authoritativeId) return { ok: false, status: 403, error: 'Unauthorized' }
226+
const authoritative = await resolveOAuthAccountId(authoritativeId)
227+
if (
228+
authoritative?.credentialType !== 'service_account' ||
229+
authoritative.credentialId !== authoritativeId ||
230+
authoritative.providerId !== OCI_API_KEY_SERVICE_ACCOUNT_PROVIDER_ID
231+
) {
232+
return { ok: false, status: 403, error: 'Unauthorized' }
233+
}
234+
serviceAccountCredentialId = authoritativeId
235+
serviceAccountProviderId = authoritative.providerId
236+
saWorkspaceId = authz.workspaceId ?? null
237+
}
234238

235239
try {
236240
const result = await resolveServiceAccountToken(
237-
authoritativeId,
238-
authoritative.providerId,
241+
serviceAccountCredentialId,
242+
serviceAccountProviderId,
239243
scopes ?? [],
240244
impersonateEmail
241245
)
@@ -244,8 +248,8 @@ export async function resolveCredentialToken(
244248
recordCredentialAccess({
245249
actorId: saActorId,
246250
workspaceId: saWorkspaceId,
247-
resourceId: authoritativeId,
248-
providerId: authoritative.providerId,
251+
resourceId: serviceAccountCredentialId,
252+
providerId: serviceAccountProviderId,
249253
credentialType: 'service_account',
250254
auditRequest,
251255
})

‎apps/sim/lib/selectors/server/credentials.test.ts‎

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
*/
44

55
import { credential } from '@sim/db/schema'
6-
import { dbChainMockFns, queueTableRows, resetDbChainMock } from '@sim/testing'
6+
import { queueTableRows, resetDbChainMock } from '@sim/testing'
77
import { beforeEach, describe, expect, it, vi } from 'vitest'
88

99
const mocks = vi.hoisted(() => ({
@@ -89,9 +89,6 @@ describe('authorizeSelectorCredential', () => {
8989
workspaceId: 'workspace-1',
9090
})
9191
)
92-
const providerPredicate = JSON.stringify(dbChainMockFns.where.mock.calls.at(-1)?.[0])
93-
expect(providerPredicate).toContain('account-1')
94-
expect(providerPredicate).not.toContain('credential-1')
9592
})
9693

9794
it('promotes a hidden fixed token to an authentication secret at every length', async () => {

‎apps/sim/lib/selectors/server/credentials.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -130,13 +130,13 @@ export async function authorizeSelectorCredential(input: {
130130
...(input.scope.kind === 'workspace' ? { workspaceId: input.workspaceId } : {}),
131131
}
132132
)
133-
if (!access.ok || access.workspaceId !== input.workspaceId || !access.resolvedCredentialId) {
133+
if (!access.ok || access.workspaceId !== input.workspaceId) {
134134
throw new SelectorConnectionUnavailableError()
135135
}
136136
input.protectedValues.add(access.resolvedCredentialId, 'reference')
137137

138138
const providerId = await requireCredentialProviderBinding(
139-
access.resolvedCredentialId,
139+
suppliedId,
140140
access,
141141
input.policy.serviceIds
142142
)

‎apps/sim/tools/index.test.ts‎

Lines changed: 0 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -3125,51 +3125,6 @@ describe('Internal Route Trust', () => {
31253125
}
31263126
})
31273127

3128-
it('unconditionally overwrites a caller-supplied hidden credential value', async () => {
3129-
const toolId = 'test_hidden_credential_authority'
3130-
const mockTool = {
3131-
id: toolId,
3132-
name: 'Hidden Credential Authority Test',
3133-
description: 'Verifies authoritative hidden credential injection',
3134-
version: '1.0.0',
3135-
oauth: {
3136-
required: true,
3137-
provider: 'google',
3138-
credentialKind: 'oauth' as const,
3139-
},
3140-
params: {
3141-
accessToken: { type: 'string', required: true, visibility: 'hidden' },
3142-
},
3143-
request: {
3144-
url: () => 'https://www.googleapis.com/test',
3145-
method: 'GET' as const,
3146-
headers: (params: Record<string, unknown>) => ({
3147-
Authorization: `Bearer ${params.accessToken}`,
3148-
}),
3149-
},
3150-
transformResponse: vi.fn().mockResolvedValue({ success: true, output: {} }),
3151-
}
3152-
;(tools as Record<string, unknown>)[toolId] = mockTool
3153-
mockResolveExecutorCredentialToken.mockResolvedValue({
3154-
accessToken: 'authorized-value',
3155-
credentialType: 'oauth',
3156-
})
3157-
3158-
try {
3159-
const result = await executeTool(toolId, {
3160-
credential: 'selected-credential',
3161-
accessToken: 'caller-forged-value',
3162-
})
3163-
3164-
expect(result.success).toBe(true)
3165-
const requestOptions = mockSecureFetchWithPinnedIP.mock.calls.at(-1)?.[2]
3166-
expect(requestOptions?.headers.authorization).toBe('Bearer authorized-value')
3167-
expect(JSON.stringify(requestOptions)).not.toContain('caller-forged-value')
3168-
} finally {
3169-
Reflect.deleteProperty(tools, toolId)
3170-
}
3171-
})
3172-
31733128
it('transports only active provenance selected for an internal model input', async () => {
31743129
const registry = new ResolvedSecretTraceRegistry([
31753130
{

0 commit comments

Comments
 (0)