Skip to content

Commit 05c4a4e

Browse files
Bill LeoutsakosBill Leoutsakos
Bill Leoutsakos
authored and
Bill Leoutsakos
committed
refactor(oci): isolate credential handoff
1 parent 71d8330 commit 05c4a4e

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
@@ -328,23 +328,19 @@ describe('OAuth Token API Routes', () => {
328328
describe('service account path', () => {
329329
it('threads the NetSuite SuiteTalk instance URL into the token response', async () => {
330330
const instanceUrl = 'https://1234567.suitetalk.api.netsuite.com'
331-
const resolvedCredential = {
331+
authOAuthUtilsMockFns.mockResolveOAuthAccountId.mockResolvedValueOnce({
332332
accountId: '',
333333
credentialId: 'netsuite-credential-id',
334334
credentialType: 'service_account',
335335
providerId: 'netsuite-service-account',
336336
workspaceId: 'workspace-id',
337337
usedCredentialTable: true,
338-
} as const
339-
authOAuthUtilsMockFns.mockResolveOAuthAccountId
340-
.mockResolvedValueOnce(resolvedCredential)
341-
.mockResolvedValueOnce(resolvedCredential)
338+
})
342339
mockAuthorizeCredentialUse.mockResolvedValueOnce({
343340
ok: true,
344341
authType: 'session',
345342
requesterUserId: 'test-user-id',
346343
workspaceId: 'workspace-id',
347-
resolvedCredentialId: 'netsuite-credential-id',
348344
})
349345
mockResolveServiceAccountToken.mockResolvedValueOnce({
350346
accessToken: 'netsuite-token',
@@ -361,23 +357,19 @@ describe('OAuth Token API Routes', () => {
361357
})
362358

363359
it('should thread authStyle from the resolver into the response', async () => {
364-
const resolvedCredential = {
360+
authOAuthUtilsMockFns.mockResolveOAuthAccountId.mockResolvedValueOnce({
365361
accountId: '',
366362
credentialId: 'sa-credential-id',
367363
credentialType: 'service_account',
368364
providerId: 'pipedrive-service-account',
369365
workspaceId: 'workspace-id',
370366
usedCredentialTable: true,
371-
} as const
372-
authOAuthUtilsMockFns.mockResolveOAuthAccountId
373-
.mockResolvedValueOnce(resolvedCredential)
374-
.mockResolvedValueOnce(resolvedCredential)
367+
})
375368
mockAuthorizeCredentialUse.mockResolvedValueOnce({
376369
ok: true,
377370
authType: 'session',
378371
requesterUserId: 'test-user-id',
379372
workspaceId: 'workspace-id',
380-
resolvedCredentialId: 'sa-credential-id',
381373
})
382374
mockResolveServiceAccountToken.mockResolvedValueOnce({
383375
accessToken: 'pasted-api-token',
@@ -395,23 +387,19 @@ describe('OAuth Token API Routes', () => {
395387
})
396388

397389
it('should omit authStyle for Bearer token-paste providers', async () => {
398-
const resolvedCredential = {
390+
authOAuthUtilsMockFns.mockResolveOAuthAccountId.mockResolvedValueOnce({
399391
accountId: '',
400392
credentialId: 'sa-credential-id',
401393
credentialType: 'service_account',
402394
providerId: 'hubspot-service-account',
403395
workspaceId: 'workspace-id',
404396
usedCredentialTable: true,
405-
} as const
406-
authOAuthUtilsMockFns.mockResolveOAuthAccountId
407-
.mockResolvedValueOnce(resolvedCredential)
408-
.mockResolvedValueOnce(resolvedCredential)
397+
})
409398
mockAuthorizeCredentialUse.mockResolvedValueOnce({
410399
ok: true,
411400
authType: 'session',
412401
requesterUserId: 'test-user-id',
413402
workspaceId: 'workspace-id',
414-
resolvedCredentialId: 'sa-credential-id',
415403
})
416404
mockResolveServiceAccountToken.mockResolvedValueOnce({
417405
accessToken: 'pat-token',
@@ -434,23 +422,19 @@ describe('OAuth Token API Routes', () => {
434422
] as const)(
435423
'surfaces the %s error code with status %i when the mint fails',
436424
async (code, status) => {
437-
const resolvedCredential = {
425+
authOAuthUtilsMockFns.mockResolveOAuthAccountId.mockResolvedValueOnce({
438426
accountId: '',
439427
credentialId: 'sa-credential-id',
440428
credentialType: 'service_account',
441429
providerId: 'salesforce-service-account',
442430
workspaceId: 'workspace-id',
443431
usedCredentialTable: true,
444-
} as const
445-
authOAuthUtilsMockFns.mockResolveOAuthAccountId
446-
.mockResolvedValueOnce(resolvedCredential)
447-
.mockResolvedValueOnce(resolvedCredential)
432+
})
448433
mockAuthorizeCredentialUse.mockResolvedValueOnce({
449434
ok: true,
450435
authType: 'session',
451436
requesterUserId: 'test-user-id',
452437
workspaceId: 'workspace-id',
453-
resolvedCredentialId: 'sa-credential-id',
454438
})
455439
mockResolveServiceAccountToken.mockRejectedValueOnce(
456440
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
@@ -320,20 +320,7 @@ describe('resolveCredentialToken', () => {
320320
})
321321

322322
it('surfaces the classified service-account failure code', async () => {
323-
mockAuthorizeCredentialUseForAuth.mockResolvedValue({
324-
ok: true,
325-
requesterUserId: 'user-1',
326-
workspaceId: 'ws-1',
327-
resolvedCredentialId: 'sa-authoritative',
328-
})
329-
mockResolveOAuthAccountId.mockResolvedValue({
330-
credentialType: 'service_account',
331-
credentialId: 'sa-authoritative',
332-
providerId: 'atlassian',
333-
workspaceId: 'ws-1',
334-
accountId: '',
335-
usedCredentialTable: true,
336-
})
323+
mockAuthorizeCredentialUseForAuth.mockResolvedValue({ ok: true, requesterUserId: 'user-1' })
337324
mockResolveServiceAccountToken.mockRejectedValue(
338325
new TokenServiceAccountValidationError('invalid_credentials', 401)
339326
)
@@ -357,12 +344,6 @@ describe('resolveCredentialToken', () => {
357344
code: 'invalid_credentials',
358345
error: 'Credential rejected by the provider — reconnect the credential',
359346
})
360-
expect(mockResolveServiceAccountToken).toHaveBeenCalledWith(
361-
'sa-authoritative',
362-
'atlassian',
363-
[],
364-
undefined
365-
)
366347
})
367348

368349
it('rejects a malformed impersonation subject before touching the credential', async () => {
@@ -551,45 +532,6 @@ describe('resolveCredentialAccessToken', () => {
551532
expect(mockResolveServiceAccountToken).not.toHaveBeenCalled()
552533
})
553534

554-
it('cannot use a non-OCI alias to bypass trusted OCI tool metadata checks', async () => {
555-
const supplied = {
556-
credentialType: 'service_account',
557-
credentialId: 'caller-controlled-alias',
558-
providerId: 'google-service-account',
559-
workspaceId: 'ws-1',
560-
accountId: '',
561-
usedCredentialTable: true,
562-
} as const
563-
const authoritative = {
564-
...supplied,
565-
credentialId: 'credential-authoritative',
566-
providerId: 'oci-api-key-service-account',
567-
} as const
568-
mockResolveOAuthAccountId.mockResolvedValueOnce(supplied).mockResolvedValueOnce(authoritative)
569-
mockGetToolMetadata.mockReturnValue({
570-
oauth: { required: true, provider: 'slack', credentialKind: 'service-account' },
571-
})
572-
mockGetServiceConfigByServiceId.mockReturnValue({
573-
serviceAccountProviderId: 'slack-custom-bot',
574-
})
575-
mockAuthorizeCredentialUseForAuth.mockResolvedValue({
576-
ok: true,
577-
requesterUserId: 'user-1',
578-
workspaceId: 'ws-1',
579-
resolvedCredentialId: 'credential-authoritative',
580-
})
581-
582-
await expect(
583-
resolveCredentialAccessToken({
584-
requestId: 'req-oci',
585-
credentialId: 'caller-controlled-alias',
586-
toolId: 'non_oci_tool',
587-
authenticate,
588-
})
589-
).resolves.toEqual({ ok: false, status: 403, error: 'Unauthorized' })
590-
expect(mockResolveServiceAccountToken).not.toHaveBeenCalled()
591-
})
592-
593535
it('rejects a managed credential when no delegation resolver is wired', async () => {
594536
mockResolveOAuthAccountId.mockResolvedValue(MANAGED_RESOLVED)
595537

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

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

6969
export type ResolveCredentialTokenResult =
@@ -265,27 +265,31 @@ export async function resolveCredentialToken(
265265
return { ok: false, status: 403, error: authz.error || 'Unauthorized' }
266266
}
267267

268-
const authoritativeId = authz.resolvedCredentialId
269-
if (!authoritativeId) return { ok: false, status: 403, error: 'Unauthorized' }
270-
const authoritative = await resolveOAuthAccountId(authoritativeId)
271-
if (
272-
authoritative?.credentialType !== 'service_account' ||
273-
authoritative.credentialId !== authoritativeId ||
274-
(authoritative.providerId === OCI_API_KEY_SERVICE_ACCOUNT_PROVIDER_ID &&
275-
input.expectedServiceAccountProviderId !== OCI_API_KEY_SERVICE_ACCOUNT_PROVIDER_ID) ||
276-
(input.expectedServiceAccountProviderId !== undefined &&
277-
authoritative.providerId !== input.expectedServiceAccountProviderId)
278-
) {
279-
return { ok: false, status: 403, error: 'Unauthorized' }
280-
}
281-
268+
let serviceAccountCredentialId = resolved.credentialId
269+
let serviceAccountProviderId = resolved.providerId
282270
const saActorId = authz.requesterUserId
283-
const saWorkspaceId = authz.workspaceId ?? null
271+
let saWorkspaceId = resolved.workspaceId ?? authz.workspaceId ?? null
272+
273+
if (input.expectedServiceAccountProviderId === OCI_API_KEY_SERVICE_ACCOUNT_PROVIDER_ID) {
274+
const authoritativeId = authz.resolvedCredentialId
275+
if (!authoritativeId) return { ok: false, status: 403, error: 'Unauthorized' }
276+
const authoritative = await resolveOAuthAccountId(authoritativeId)
277+
if (
278+
authoritative?.credentialType !== 'service_account' ||
279+
authoritative.credentialId !== authoritativeId ||
280+
authoritative.providerId !== OCI_API_KEY_SERVICE_ACCOUNT_PROVIDER_ID
281+
) {
282+
return { ok: false, status: 403, error: 'Unauthorized' }
283+
}
284+
serviceAccountCredentialId = authoritativeId
285+
serviceAccountProviderId = authoritative.providerId
286+
saWorkspaceId = authz.workspaceId ?? null
287+
}
284288

285289
try {
286290
const result = await resolveServiceAccountToken(
287-
authoritativeId,
288-
authoritative.providerId,
291+
serviceAccountCredentialId,
292+
serviceAccountProviderId,
289293
scopes ?? [],
290294
impersonateEmail
291295
)
@@ -294,8 +298,8 @@ export async function resolveCredentialToken(
294298
recordCredentialAccess({
295299
actorId: saActorId,
296300
workspaceId: saWorkspaceId,
297-
resourceId: authoritativeId,
298-
providerId: authoritative.providerId,
301+
resourceId: serviceAccountCredentialId,
302+
providerId: serviceAccountProviderId,
299303
credentialType: 'service_account',
300304
auditRequest,
301305
})

‎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
@@ -3176,51 +3176,6 @@ describe('Internal Route Trust', () => {
31763176
}
31773177
})
31783178

3179-
it('unconditionally overwrites a caller-supplied hidden credential value', async () => {
3180-
const toolId = 'test_hidden_credential_authority'
3181-
const mockTool = {
3182-
id: toolId,
3183-
name: 'Hidden Credential Authority Test',
3184-
description: 'Verifies authoritative hidden credential injection',
3185-
version: '1.0.0',
3186-
oauth: {
3187-
required: true,
3188-
provider: 'google',
3189-
credentialKind: 'oauth' as const,
3190-
},
3191-
params: {
3192-
accessToken: { type: 'string', required: true, visibility: 'hidden' },
3193-
},
3194-
request: {
3195-
url: () => 'https://www.googleapis.com/test',
3196-
method: 'GET' as const,
3197-
headers: (params: Record<string, unknown>) => ({
3198-
Authorization: `Bearer ${params.accessToken}`,
3199-
}),
3200-
},
3201-
transformResponse: vi.fn().mockResolvedValue({ success: true, output: {} }),
3202-
}
3203-
;(tools as Record<string, unknown>)[toolId] = mockTool
3204-
mockResolveExecutorCredentialToken.mockResolvedValue({
3205-
accessToken: 'authorized-value',
3206-
credentialType: 'oauth',
3207-
})
3208-
3209-
try {
3210-
const result = await executeTool(toolId, {
3211-
credential: 'selected-credential',
3212-
accessToken: 'caller-forged-value',
3213-
})
3214-
3215-
expect(result.success).toBe(true)
3216-
const requestOptions = mockSecureFetchWithPinnedIP.mock.calls.at(-1)?.[2]
3217-
expect(requestOptions?.headers.authorization).toBe('Bearer authorized-value')
3218-
expect(JSON.stringify(requestOptions)).not.toContain('caller-forged-value')
3219-
} finally {
3220-
Reflect.deleteProperty(tools, toolId)
3221-
}
3222-
})
3223-
32243179
it('transports only active provenance selected for an internal model input', async () => {
32253180
const registry = new ResolvedSecretTraceRegistry([
32263181
{

0 commit comments

Comments
 (0)