Skip to content

Commit 8441031

Browse files
waleedlatif1waleedlatif1
committed
improvement(executor): resolve credential tokens in-process instead of hopping the oauth token route
1 parent beb24b2 commit 8441031

8 files changed

Lines changed: 346 additions & 197 deletions

File tree

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ vi.mock('@/lib/workspaces/permissions/utils', () => ({
1616
vi.mock('@/lib/oauth/credential-service', () => authOAuthUtilsMock)
1717

1818
vi.mock('@/executor/utils/credential-token', () => ({
19-
fetchCredentialAccessToken: vi.fn().mockResolvedValue('mock-access-token'),
19+
resolveExecutorCredentialToken: vi.fn().mockResolvedValue({ accessToken: 'mock-access-token' }),
2020
}))
2121

2222
vi.mock('@/lib/credentials/access', () => ({

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ vi.mock('@/lib/oauth/credential-service', () => authOAuthUtilsMock)
2222
vi.mock('@/lib/core/security/encryption', () => encryptionMock)
2323

2424
vi.mock('@/executor/utils/credential-token', () => ({
25-
fetchCredentialAccessToken: vi.fn().mockResolvedValue('mock-access-token'),
25+
resolveExecutorCredentialToken: vi.fn().mockResolvedValue({ accessToken: 'mock-access-token' }),
2626
}))
2727

2828
vi.mock('@/lib/credentials/access', () => ({

‎apps/sim/executor/utils/credential-token.test.ts‎

Lines changed: 146 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,146 @@
1+
/**
2+
* @vitest-environment node
3+
*/
4+
import { beforeEach, describe, expect, it, vi } from 'vitest'
5+
import type { ExecutorDelegationOrigin } from '@/executor/types'
6+
7+
const { mockBindExecutorManagedOAuthDelegation, mockResolveCredentialAccessToken } = vi.hoisted(
8+
() => ({
9+
mockBindExecutorManagedOAuthDelegation: vi.fn(),
10+
mockResolveCredentialAccessToken: vi.fn(),
11+
})
12+
)
13+
14+
vi.mock('@/lib/oauth/token-resolution', () => ({
15+
resolveCredentialAccessToken: mockResolveCredentialAccessToken,
16+
}))
17+
18+
vi.mock('@/lib/credentials/application/managed-oauth-delegation', () => ({
19+
bindExecutorManagedOAuthDelegation: mockBindExecutorManagedOAuthDelegation,
20+
}))
21+
22+
import { resolveExecutorCredentialToken } from '@/executor/utils/credential-token'
23+
24+
const ORIGIN: ExecutorDelegationOrigin = {
25+
subjectUserId: 'user-1',
26+
workflowId: 'wf-1',
27+
executionId: 'exec-1',
28+
currentWorkflow: { workflowId: 'wf-1' },
29+
} as ExecutorDelegationOrigin
30+
31+
describe('resolveExecutorCredentialToken', () => {
32+
beforeEach(() => {
33+
vi.clearAllMocks()
34+
mockResolveCredentialAccessToken.mockResolvedValue({
35+
ok: true,
36+
token: { accessToken: 'fresh' },
37+
})
38+
})
39+
40+
it('dispatches with an internal-JWT auth result for the executing user', async () => {
41+
await resolveExecutorCredentialToken({
42+
requestId: 'req-1',
43+
credentialId: 'cred-1',
44+
userId: 'user-1',
45+
workflowId: 'wf-1',
46+
toolId: 'gmail_read',
47+
})
48+
49+
const input = mockResolveCredentialAccessToken.mock.calls[0][0]
50+
expect(input).toMatchObject({
51+
requestId: 'req-1',
52+
credentialId: 'cred-1',
53+
workflowId: 'wf-1',
54+
toolId: 'gmail_read',
55+
})
56+
await expect(input.authenticate()).toEqual({
57+
success: true,
58+
userId: 'user-1',
59+
authType: 'internal_jwt',
60+
})
61+
expect(input.resolveManagedPrincipal).toBeUndefined()
62+
})
63+
64+
it('asserts the caller only when the run enforces credential access', async () => {
65+
await resolveExecutorCredentialToken({
66+
requestId: 'req-1',
67+
credentialId: 'cred-1',
68+
userId: 'user-1',
69+
})
70+
expect(mockResolveCredentialAccessToken.mock.calls[0][0].callerUserId).toBeUndefined()
71+
72+
await resolveExecutorCredentialToken({
73+
requestId: 'req-2',
74+
credentialId: 'cred-1',
75+
userId: 'user-1',
76+
enforceCredentialAccess: true,
77+
})
78+
expect(mockResolveCredentialAccessToken.mock.calls[1][0].callerUserId).toBe('user-1')
79+
})
80+
81+
it('wires the managed delegation binder only when the run carries an origin', async () => {
82+
mockBindExecutorManagedOAuthDelegation.mockResolvedValue({ kind: 'delegated' })
83+
84+
await resolveExecutorCredentialToken({
85+
requestId: 'req-1',
86+
credentialId: 'cred-1',
87+
userId: 'user-1',
88+
executorDelegationOrigin: ORIGIN,
89+
})
90+
91+
const input = mockResolveCredentialAccessToken.mock.calls[0][0]
92+
expect(input.resolveManagedPrincipal).toBeTypeOf('function')
93+
await input.resolveManagedPrincipal('managed-1')
94+
expect(mockBindExecutorManagedOAuthDelegation).toHaveBeenCalledWith(ORIGIN, 'managed-1')
95+
})
96+
97+
it('fails before dispatch when the origin lacks current workflow authority', async () => {
98+
await expect(
99+
resolveExecutorCredentialToken({
100+
requestId: 'req-1',
101+
credentialId: 'cred-1',
102+
userId: 'user-1',
103+
executorDelegationOrigin: { ...ORIGIN, currentWorkflow: undefined },
104+
})
105+
).rejects.toThrow('Managed credential delegation is missing current workflow authority')
106+
expect(mockResolveCredentialAccessToken).not.toHaveBeenCalled()
107+
})
108+
109+
it('throws the executeTool error contract with the tool label on failure', async () => {
110+
mockResolveCredentialAccessToken.mockResolvedValue({
111+
ok: false,
112+
status: 401,
113+
error: 'Failed to refresh access token',
114+
})
115+
116+
await expect(
117+
resolveExecutorCredentialToken({
118+
requestId: 'req-1',
119+
credentialId: 'cred-1',
120+
userId: 'user-1',
121+
toolLabel: 'Gmail Read',
122+
})
123+
).rejects.toThrow('Failed to obtain credential for Gmail Read: Failed to refresh access token')
124+
})
125+
126+
it('returns the full token payload untouched', async () => {
127+
const token = {
128+
accessToken: 'fresh',
129+
idToken: 'id-1',
130+
instanceUrl: 'https://contoso.crm.dynamics.com',
131+
apiDomain: 'desk.zoho.com',
132+
cloudId: 'cloud-1',
133+
domain: 'example.atlassian.net',
134+
authStyle: 'x-api-token',
135+
}
136+
mockResolveCredentialAccessToken.mockResolvedValue({ ok: true, token })
137+
138+
await expect(
139+
resolveExecutorCredentialToken({
140+
requestId: 'req-1',
141+
credentialId: 'cred-1',
142+
userId: 'user-1',
143+
})
144+
).resolves.toEqual(token)
145+
})
146+
})

‎apps/sim/executor/utils/credential-token.ts‎

Lines changed: 61 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -1,67 +1,79 @@
11
import { createLogger } from '@sim/logger'
2-
import { generateInternalToken } from '@/lib/auth/internal'
3-
import { getInternalApiBaseUrl } from '@/lib/core/utils/urls'
2+
import { AuthType } from '@/lib/auth/hybrid'
3+
import { bindExecutorManagedOAuthDelegation } from '@/lib/credentials/application/managed-oauth-delegation'
4+
import {
5+
type CredentialTokenPayload,
6+
resolveCredentialAccessToken,
7+
} from '@/lib/oauth/token-resolution'
8+
import type { ExecutorDelegationOrigin } from '@/executor/types'
49

510
const logger = createLogger('ExecutorCredentialToken')
611

7-
/**
8-
* Fetches a credential's access token from the app rather than resolving it here.
9-
*
10-
* Refreshing an OAuth token needs the provider's client id and secret, read through
11-
* `requireOAuthClientCapability`, which THROWS when they are absent. Only the app
12-
* container loads those (from `SIM_ENV_SECRET_ID`); workflow execution runs in a
13-
* Trigger.dev worker whose environment does not carry them. Resolving in-process there
14-
* turns every credential whose access token has expired into a refresh failure, and a
15-
* still-valid token hides it until the token lapses.
16-
*
17-
* See `.claude/rules/sim-architecture.md`, "The app/worker runtime boundary".
18-
*
19-
* The route authorizes the credential itself, so this never widens access.
20-
*/
21-
export async function fetchCredentialAccessToken(params: {
12+
export interface ResolveExecutorCredentialTokenParams {
2213
requestId: string
2314
credentialId: string
24-
userId: string
15+
userId?: string
2516
workflowId?: string
26-
}): Promise<string> {
27-
const { requestId, credentialId, userId, workflowId } = params
17+
/** Tool consuming the token; required by the managed-OAuth scope policy. */
18+
toolId?: string
19+
/** Display label for the thrown failure ("Failed to obtain credential for X: ..."). */
20+
toolLabel?: string
21+
scopes?: string[]
22+
impersonateEmail?: string
23+
/** Asserts the acting user alongside the credential lookup, mirroring the HTTP surface. */
24+
enforceCredentialAccess?: boolean
25+
/** Proves managed-credential delegations in-process when the run carries one. */
26+
executorDelegationOrigin?: ExecutorDelegationOrigin
27+
}
2828

29-
const url = new URL('/api/auth/oauth/token', getInternalApiBaseUrl())
30-
if (workflowId) url.searchParams.set('workflowId', workflowId)
29+
/**
30+
* Resolves a credential's access token in-process for server-side workflow
31+
* execution. Goes through the same authorized application dispatch as
32+
* `POST /api/auth/oauth/token` (`resolveCredentialAccessToken`), replacing the
33+
* HTTP hop the executor used to make to its own route — both runtimes hold the
34+
* OAuth client config the refresh branch needs, so authorization, refresh, and
35+
* audit run identically without the round trip.
36+
*/
37+
export async function resolveExecutorCredentialToken(
38+
params: ResolveExecutorCredentialTokenParams
39+
): Promise<CredentialTokenPayload> {
40+
const { requestId, credentialId, userId, workflowId, toolId, executorDelegationOrigin } = params
3141

32-
const headers: Record<string, string> = { 'Content-Type': 'application/json' }
33-
try {
34-
headers.Authorization = `Bearer ${await generateInternalToken(userId)}`
35-
} catch (_e) {
36-
// Swallow mint errors; the request then fails authentication and reports upstream.
42+
if (executorDelegationOrigin && !executorDelegationOrigin.currentWorkflow) {
43+
throw new Error('Managed credential delegation is missing current workflow authority')
3744
}
3845

39-
// boundary-raw-fetch: same-origin token route, authenticated by the internal JWT minted above
40-
const response = await fetch(url.toString(), {
41-
method: 'POST',
42-
headers,
43-
body: JSON.stringify({ credentialId, ...(workflowId ? { workflowId } : {}) }),
46+
const result = await resolveCredentialAccessToken({
47+
requestId,
48+
credentialId,
49+
workflowId,
50+
toolId,
51+
scopes: params.scopes,
52+
impersonateEmail: params.impersonateEmail,
53+
callerUserId: userId && params.enforceCredentialAccess ? userId : undefined,
54+
authenticate: () => ({
55+
success: true,
56+
...(userId ? { userId } : {}),
57+
authType: AuthType.INTERNAL_JWT,
58+
}),
59+
...(executorDelegationOrigin
60+
? {
61+
resolveManagedPrincipal: (managedCredentialId: string) =>
62+
bindExecutorManagedOAuthDelegation(executorDelegationOrigin, managedCredentialId),
63+
}
64+
: {}),
4465
})
4566

46-
if (!response.ok) {
47-
const errorText = await response.text()
48-
logger.error(`[${requestId}] Credential token request failed`, {
49-
status: response.status,
67+
if (!result.ok) {
68+
logger.error(`[${requestId}] Credential token resolution failed`, {
69+
status: result.status,
5070
credentialId,
71+
...(result.code ? { code: result.code } : {}),
5172
})
52-
let message = errorText
53-
try {
54-
const parsed = JSON.parse(errorText)
55-
if (parsed.error) message = parsed.error
56-
} catch {
57-
// Use raw text
58-
}
59-
throw new Error(message)
73+
throw new Error(
74+
`Failed to obtain credential for ${params.toolLabel ?? credentialId}: ${result.error}`
75+
)
6076
}
6177

62-
const { accessToken } = (await response.json()) as { accessToken?: string }
63-
if (!accessToken) {
64-
throw new Error('Credential token response carried no access token')
65-
}
66-
return accessToken
78+
return result.token
6779
}

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

Lines changed: 7 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -7,12 +7,12 @@ const {
77
mockGetCredentialActorContext,
88
mockGetServiceAccountToken,
99
mockRefreshTokenIfNeeded,
10-
mockFetchCredentialAccessToken,
10+
mockResolveExecutorCredentialToken,
1111
} = vi.hoisted(() => ({
1212
mockGetCredentialActorContext: vi.fn(),
1313
mockGetServiceAccountToken: vi.fn(),
1414
mockRefreshTokenIfNeeded: vi.fn(),
15-
mockFetchCredentialAccessToken: vi.fn(),
15+
mockResolveExecutorCredentialToken: vi.fn(),
1616
}))
1717

1818
vi.mock('@/lib/credentials/access', () => ({
@@ -25,7 +25,7 @@ vi.mock('@/lib/oauth/credential-service', () => ({
2525
refreshTokenIfNeeded: mockRefreshTokenIfNeeded,
2626
}))
2727
vi.mock('@/executor/utils/credential-token', () => ({
28-
fetchCredentialAccessToken: mockFetchCredentialAccessToken,
28+
resolveExecutorCredentialToken: mockResolveExecutorCredentialToken,
2929
}))
3030

3131
import { resolveVertexCredential } from '@/executor/utils/vertex-credential'
@@ -104,10 +104,6 @@ describe('resolveVertexCredential workspace binding', () => {
104104
})
105105
})
106106

107-
/**
108-
* This resolver runs inside the Trigger.dev worker, whose environment carries no OAuth
109-
* client config — an in-process refresh throws there once the stored token expires.
110-
*/
111107
describe('resolveVertexCredential OAuth branch', () => {
112108
const oauthContext = {
113109
credential: { id: 'cred-o', workspaceId: 'workspace-a', type: 'oauth', accountId: 'acct-1' },
@@ -120,10 +116,10 @@ describe('resolveVertexCredential OAuth branch', () => {
120116
beforeEach(() => {
121117
vi.clearAllMocks()
122118
mockGetCredentialActorContext.mockResolvedValue(oauthContext)
123-
mockFetchCredentialAccessToken.mockResolvedValue('oauth-access-token')
119+
mockResolveExecutorCredentialToken.mockResolvedValue({ accessToken: 'oauth-access-token' })
124120
})
125121

126-
it('fetches the token from the app instead of refreshing in-process', async () => {
122+
it('resolves the token through the shared in-process resolver', async () => {
127123
await expect(
128124
resolveVertexCredential({
129125
credentialId: 'cred-o',
@@ -134,7 +130,7 @@ describe('resolveVertexCredential OAuth branch', () => {
134130
).resolves.toBe('oauth-access-token')
135131

136132
expect(mockRefreshTokenIfNeeded).not.toHaveBeenCalled()
137-
expect(mockFetchCredentialAccessToken).toHaveBeenCalledWith(
133+
expect(mockResolveExecutorCredentialToken).toHaveBeenCalledWith(
138134
expect.objectContaining({ credentialId: 'cred-o', userId: 'user-1', workflowId: 'wf-1' })
139135
)
140136
})
@@ -153,6 +149,6 @@ describe('resolveVertexCredential OAuth branch', () => {
153149
})
154150
).rejects.toThrow()
155151

156-
expect(mockFetchCredentialAccessToken).not.toHaveBeenCalled()
152+
expect(mockResolveExecutorCredentialToken).not.toHaveBeenCalled()
157153
})
158154
})

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

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import { createLogger } from '@sim/logger'
22
import { canUseCredential, getCredentialActorContext } from '@/lib/credentials/access'
33
import { getServiceAccountToken } from '@/lib/oauth/credential-service'
4-
import { fetchCredentialAccessToken } from '@/executor/utils/credential-token'
4+
import { resolveExecutorCredentialToken } from '@/executor/utils/credential-token'
55

66
const logger = createLogger('VertexCredential')
77

@@ -65,17 +65,12 @@ export async function resolveVertexCredential({
6565
throw new Error(`Vertex AI credential is not a valid OAuth credential: ${credentialId}`)
6666
}
6767

68-
/**
69-
* Fetched from the app rather than refreshed here: this runs inside the Trigger.dev
70-
* worker, whose environment carries no OAuth client config, so an in-process refresh
71-
* throws once the stored access token expires. The service-account branch above needs
72-
* no such config and stays in-process.
73-
*/
74-
const accessToken = await fetchCredentialAccessToken({
68+
const { accessToken } = await resolveExecutorCredentialToken({
7569
requestId,
7670
credentialId,
7771
userId: actingUserId,
7872
workflowId,
73+
toolLabel: 'Vertex AI',
7974
})
8075

8176
if (!accessToken) {

0 commit comments

Comments
 (0)