Skip to content

Commit 5027685

Browse files
Bill LeoutsakosBill Leoutsakos
authored andcommitted
revert(credentials): defer platform-wide provider enforcement
1 parent a9659a7 commit 5027685

5 files changed

Lines changed: 2 additions & 307 deletions

File tree

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

Lines changed: 0 additions & 169 deletions
Original file line numberDiff line numberDiff line change
@@ -6,11 +6,8 @@ import { beforeEach, describe, expect, it, vi } from 'vitest'
66
const {
77
mockAuthorizeCredentialUseForAuth,
88
mockCaptureServerEvent,
9-
mockCredentialProviderMatchesService,
109
mockExecuteManagedToken,
1110
mockGetCredential,
12-
mockGetServiceConfigByProviderId,
13-
mockGetServiceConfigByServiceId,
1411
mockGetToolMetadata,
1512
mockRecordAudit,
1613
mockRefreshTokenIfNeeded,
@@ -19,11 +16,8 @@ const {
1916
} = vi.hoisted(() => ({
2017
mockAuthorizeCredentialUseForAuth: vi.fn(),
2118
mockCaptureServerEvent: vi.fn(),
22-
mockCredentialProviderMatchesService: vi.fn(),
2319
mockExecuteManagedToken: vi.fn(),
2420
mockGetCredential: vi.fn(),
25-
mockGetServiceConfigByProviderId: vi.fn(),
26-
mockGetServiceConfigByServiceId: vi.fn(),
2721
mockGetToolMetadata: vi.fn(),
2822
mockRecordAudit: vi.fn(),
2923
mockRefreshTokenIfNeeded: vi.fn(),
@@ -84,10 +78,7 @@ vi.mock('@/tools/metadata', () => ({
8478
}))
8579

8680
vi.mock('@/lib/oauth/utils', () => ({
87-
credentialProviderMatchesService: mockCredentialProviderMatchesService,
8881
getCanonicalScopesForProvider: vi.fn().mockReturnValue([]),
89-
getServiceConfigByProviderId: mockGetServiceConfigByProviderId,
90-
getServiceConfigByServiceId: mockGetServiceConfigByServiceId,
9182
}))
9283

9384
import { OrchestrationError } from '@/lib/core/orchestration/types'
@@ -355,12 +346,6 @@ describe('resolveCredentialAccessToken', () => {
355346
beforeEach(() => {
356347
vi.clearAllMocks()
357348
mockResolveOAuthAccountId.mockResolvedValue(null)
358-
mockCredentialProviderMatchesService.mockReturnValue(true)
359-
mockGetServiceConfigByServiceId.mockReturnValue({
360-
providerId: 'google',
361-
serviceAccountProviderId: 'google-service-account',
362-
})
363-
mockGetServiceConfigByProviderId.mockReturnValue(null)
364349
authenticate.mockResolvedValue(INTERNAL_AUTH)
365350
resolveManagedPrincipal.mockResolvedValue(EXECUTOR_PRINCIPAL)
366351
mockGetToolMetadata.mockReturnValue({
@@ -426,160 +411,6 @@ describe('resolveCredentialAccessToken', () => {
426411
})
427412
})
428413

429-
it('rejects a service-account credential with no provider before authentication', async () => {
430-
mockResolveOAuthAccountId.mockResolvedValue({
431-
credentialType: 'service_account',
432-
credentialId: 'service-account-1',
433-
workspaceId: 'ws-1',
434-
accountId: '',
435-
usedCredentialTable: true,
436-
})
437-
mockGetToolMetadata.mockReturnValue({
438-
oauth: {
439-
required: true,
440-
provider: 'google',
441-
credentialKind: 'service-account',
442-
},
443-
})
444-
445-
await expect(
446-
resolveCredentialAccessToken({
447-
requestId: 'req-1',
448-
credentialId: 'service-account-1',
449-
toolId: 'google_service_account_tool',
450-
authenticate,
451-
})
452-
).resolves.toEqual({
453-
ok: false,
454-
status: 403,
455-
code: 'CREDENTIAL_PROVIDER_MISMATCH',
456-
error: 'Credential belongs to another service',
457-
})
458-
expect(authenticate).not.toHaveBeenCalled()
459-
expect(mockResolveServiceAccountToken).not.toHaveBeenCalled()
460-
})
461-
462-
it('rejects a service-account credential from another provider before authentication', async () => {
463-
mockResolveOAuthAccountId.mockResolvedValue({
464-
credentialType: 'service_account',
465-
credentialId: 'service-account-1',
466-
providerId: 'atlassian-service-account',
467-
workspaceId: 'ws-1',
468-
accountId: '',
469-
usedCredentialTable: true,
470-
})
471-
mockGetToolMetadata.mockReturnValue({
472-
oauth: {
473-
required: true,
474-
provider: 'google',
475-
credentialKind: 'service-account',
476-
},
477-
})
478-
mockCredentialProviderMatchesService.mockReturnValue(false)
479-
480-
await expect(
481-
resolveCredentialAccessToken({
482-
requestId: 'req-1',
483-
credentialId: 'service-account-1',
484-
toolId: 'google_service_account_tool',
485-
authenticate,
486-
})
487-
).resolves.toEqual({
488-
ok: false,
489-
status: 403,
490-
code: 'CREDENTIAL_PROVIDER_MISMATCH',
491-
error: 'Credential belongs to another service',
492-
})
493-
expect(authenticate).not.toHaveBeenCalled()
494-
expect(mockResolveServiceAccountToken).not.toHaveBeenCalled()
495-
})
496-
497-
it('accepts a non-Oracle service account whose provider matches the tool service', async () => {
498-
mockResolveOAuthAccountId.mockResolvedValue({
499-
credentialType: 'service_account',
500-
credentialId: 'service-account-1',
501-
providerId: 'google-service-account',
502-
workspaceId: 'ws-1',
503-
accountId: '',
504-
usedCredentialTable: true,
505-
})
506-
mockGetToolMetadata.mockReturnValue({
507-
oauth: {
508-
required: true,
509-
provider: 'google-email',
510-
requiredScopes: ['scope-a'],
511-
credentialKind: 'service-account',
512-
},
513-
})
514-
mockGetServiceConfigByServiceId.mockReturnValue(null)
515-
mockGetServiceConfigByProviderId.mockReturnValue({
516-
providerId: 'google-email',
517-
serviceAccountProviderId: 'google-service-account',
518-
})
519-
mockAuthorizeCredentialUseForAuth.mockResolvedValue({
520-
ok: true,
521-
requesterUserId: 'user-1',
522-
workspaceId: 'ws-1',
523-
})
524-
mockResolveServiceAccountToken.mockResolvedValue({ accessToken: 'service-account-token' })
525-
526-
await expect(
527-
resolveCredentialAccessToken({
528-
requestId: 'req-1',
529-
credentialId: 'service-account-1',
530-
toolId: 'gmail_read',
531-
scopes: ['scope-a'],
532-
authenticate,
533-
})
534-
).resolves.toMatchObject({
535-
ok: true,
536-
token: { accessToken: 'service-account-token', credentialType: 'service_account' },
537-
})
538-
expect(mockGetServiceConfigByProviderId).toHaveBeenCalledWith('google-email')
539-
expect(mockResolveServiceAccountToken).toHaveBeenCalledWith(
540-
'service-account-1',
541-
'google-service-account',
542-
['scope-a'],
543-
undefined
544-
)
545-
})
546-
547-
it.each([
548-
['an OAuth credential for a service-account-only tool', undefined, 'service-account'],
549-
['a service account for an OAuth-only tool', 'service_account', 'oauth'],
550-
])('rejects %s before authentication', async (_label, credentialType, requiredKind) => {
551-
mockResolveOAuthAccountId.mockResolvedValue({
552-
...(credentialType ? { credentialType } : {}),
553-
credentialId: 'credential-1',
554-
providerId: credentialType ? 'google-service-account' : undefined,
555-
workspaceId: 'ws-1',
556-
accountId: credentialType ? '' : 'account-1',
557-
usedCredentialTable: true,
558-
})
559-
mockGetToolMetadata.mockReturnValue({
560-
oauth: {
561-
required: true,
562-
provider: 'google',
563-
credentialKind: requiredKind,
564-
},
565-
})
566-
567-
const result = await resolveCredentialAccessToken({
568-
requestId: 'req-1',
569-
credentialId: 'credential-1',
570-
toolId: 'kind_restricted_tool',
571-
authenticate,
572-
})
573-
574-
expect(result).toEqual({
575-
ok: false,
576-
status: 403,
577-
code: 'CREDENTIAL_PROVIDER_MISMATCH',
578-
error: 'Credential belongs to another service',
579-
})
580-
expect(authenticate).not.toHaveBeenCalled()
581-
})
582-
583414
it('rejects a managed credential when no delegation resolver is wired', async () => {
584415
mockResolveOAuthAccountId.mockResolvedValue(MANAGED_RESOLVED)
585416

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

Lines changed: 2 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -24,12 +24,7 @@ import {
2424
MICROSOFT_DATAVERSE_PROVIDER_ID,
2525
} from '@/lib/oauth/microsoft-dataverse'
2626
import { extractSalesforceInstanceUrl, isSalesforceOAuthProviderId } from '@/lib/oauth/salesforce'
27-
import {
28-
credentialProviderMatchesService,
29-
getCanonicalScopesForProvider,
30-
getServiceConfigByProviderId,
31-
getServiceConfigByServiceId,
32-
} from '@/lib/oauth/utils'
27+
import { getCanonicalScopesForProvider } from '@/lib/oauth/utils'
3328
import { captureServerEvent } from '@/lib/posthog/server'
3429
import { getToolMetadata } from '@/tools/metadata'
3530
import { extractZohoDeskBaseFromScope } from '@/tools/zoho_desk/host-allowlist'
@@ -337,46 +332,6 @@ export interface ResolveCredentialAccessTokenInput
337332
resolveManagedPrincipal?: (credentialId: string) => Promise<DelegatedPrincipal>
338333
}
339334

340-
function credentialProviderMismatch(): ResolveCredentialTokenResult {
341-
return {
342-
ok: false,
343-
status: 403,
344-
code: 'CREDENTIAL_PROVIDER_MISMATCH',
345-
error: 'Credential belongs to another service',
346-
}
347-
}
348-
349-
function validateToolCredentialBinding(
350-
resolved: ResolvedCredential | null,
351-
toolId: string | undefined,
352-
toolMetadata: ReturnType<typeof getToolMetadata>
353-
): ResolveCredentialTokenResult | null {
354-
if (!resolved || !toolId) return null
355-
356-
const oauth = toolMetadata?.oauth
357-
const isServiceAccount = resolved.credentialType === 'service_account'
358-
if (
359-
oauth?.credentialKind === 'service-account'
360-
? !isServiceAccount
361-
: oauth?.credentialKind === 'oauth' && isServiceAccount
362-
) {
363-
return credentialProviderMismatch()
364-
}
365-
if (!isServiceAccount) return null
366-
367-
const service = oauth?.required
368-
? (getServiceConfigByServiceId(oauth.provider) ?? getServiceConfigByProviderId(oauth.provider))
369-
: null
370-
if (
371-
!resolved.providerId ||
372-
!service ||
373-
!credentialProviderMatchesService(resolved.providerId, service)
374-
) {
375-
return credentialProviderMismatch()
376-
}
377-
return null
378-
}
379-
380335
/**
381336
* Authorized application dispatch behind `POST /api/auth/oauth/token`. Every server
382337
* surface that needs a credential token — the route and the in-process tool
@@ -388,10 +343,7 @@ export async function resolveCredentialAccessToken(
388343
): Promise<ResolveCredentialTokenResult> {
389344
const { requestId, credentialId, toolId, auditRequest } = input
390345

391-
const toolMetadata = toolId ? getToolMetadata(toolId) : undefined
392346
const resolved = credentialId ? await resolveOAuthAccountId(credentialId) : null
393-
const bindingError = validateToolCredentialBinding(resolved, toolId, toolMetadata)
394-
if (bindingError) return bindingError
395347

396348
if (resolved?.credentialType !== 'managed_oauth' || !resolved.credentialId) {
397349
const auth = await input.authenticate()
@@ -443,6 +395,7 @@ export async function resolveCredentialAccessToken(
443395
}
444396
}
445397

398+
const toolMetadata = getToolMetadata(toolId)
446399
if (!toolMetadata?.oauth?.required) {
447400
logger.error(`[${requestId}] Tool is not configured for managed OAuth`, { toolId })
448401
return {

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

Lines changed: 0 additions & 77 deletions
Original file line numberDiff line numberDiff line change
@@ -125,83 +125,6 @@ describe('authorizeSelectorCredential', () => {
125125
expect(mocks.authorizeCredentialUse).not.toHaveBeenCalled()
126126
})
127127

128-
it('rejects a fixed token for a service-account-only selector', async () => {
129-
await expect(
130-
authorizeSelectorCredential({
131-
principal,
132-
context: { oauthCredential: 'xoxb-a' },
133-
scope: { kind: 'workspace', workspaceId: 'workspace-1' },
134-
workspaceId: 'workspace-1',
135-
policy: {
136-
kind: 'stored-or-fixed-token',
137-
field: 'oauthCredential',
138-
serviceIds: ['slack'],
139-
tokenPrefixes: ['xoxb-'],
140-
credentialKind: 'service-account',
141-
},
142-
protectedValues: createSelectorProtectedValues(),
143-
references: new Map(),
144-
})
145-
).rejects.toEqual(new SelectorConnectionUnavailableError())
146-
expect(mocks.authorizeCredentialUse).not.toHaveBeenCalled()
147-
})
148-
149-
it.each([
150-
['oauth', 'service-account'],
151-
['service_account', 'oauth'],
152-
] as const)(
153-
'rejects a %s credential for a %s-only selector before provider resolution',
154-
async (credentialType, credentialKind) => {
155-
mocks.authorizeCredentialUse.mockResolvedValue({
156-
ok: true,
157-
workspaceId: 'workspace-1',
158-
credentialOwnerUserId: 'owner-1',
159-
resolvedCredentialId: 'credential-1',
160-
credentialType,
161-
})
162-
163-
await expect(
164-
authorizeSelectorCredential({
165-
principal,
166-
context: { oauthCredential: 'credential-1' },
167-
scope: { kind: 'workspace', workspaceId: 'workspace-1' },
168-
workspaceId: 'workspace-1',
169-
policy: { ...policy, credentialKind },
170-
protectedValues: createSelectorProtectedValues(),
171-
references: new Map(),
172-
})
173-
).rejects.toEqual(new SelectorConnectionUnavailableError())
174-
expect(mocks.credentialProviderMatchesService).not.toHaveBeenCalled()
175-
}
176-
)
177-
178-
it.each([
179-
['oauth', 'oauth'],
180-
['service_account', 'service-account'],
181-
] as const)('accepts a matching %s credential kind', async (credentialType, credentialKind) => {
182-
mocks.authorizeCredentialUse.mockResolvedValue({
183-
ok: true,
184-
workspaceId: 'workspace-1',
185-
credentialOwnerUserId: 'owner-1',
186-
resolvedCredentialId: 'credential-1',
187-
credentialType,
188-
})
189-
queueTableRows(credential, [{ accountId: 'account-1', providerId: 'google' }])
190-
mocks.credentialProviderMatchesService.mockReturnValue(true)
191-
192-
await expect(
193-
authorizeSelectorCredential({
194-
principal,
195-
context: { oauthCredential: 'credential-1' },
196-
scope: { kind: 'workspace', workspaceId: 'workspace-1' },
197-
workspaceId: 'workspace-1',
198-
policy: { ...policy, credentialKind },
199-
protectedValues: createSelectorProtectedValues(),
200-
references: new Map(),
201-
})
202-
).resolves.toMatchObject({ access: { credentialType } })
203-
})
204-
205128
it('conceals a stored credential whose trusted provider does not match the selector service', async () => {
206129
mocks.authorizeCredentialUse.mockResolvedValue({
207130
ok: true,

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

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -111,9 +111,6 @@ export async function authorizeSelectorCredential(input: {
111111
input.policy.kind === 'stored-or-fixed-token' &&
112112
input.policy.tokenPrefixes.some((prefix) => suppliedId.startsWith(prefix))
113113
) {
114-
if (input.policy.credentialKind === 'service-account') {
115-
throw new SelectorConnectionUnavailableError()
116-
}
117114
const reference = input.references.get(input.policy.field)
118115
if (reference && !reference.visible) {
119116
input.protectedValues.add(suppliedId, 'secret')
@@ -136,13 +133,6 @@ export async function authorizeSelectorCredential(input: {
136133
if (!access.ok || access.workspaceId !== input.workspaceId) {
137134
throw new SelectorConnectionUnavailableError()
138135
}
139-
if (
140-
input.policy.credentialKind === 'service-account'
141-
? access.credentialType !== 'service_account'
142-
: input.policy.credentialKind === 'oauth' && access.credentialType === 'service_account'
143-
) {
144-
throw new SelectorConnectionUnavailableError()
145-
}
146136
input.protectedValues.add(access.resolvedCredentialId, 'reference')
147137

148138
const providerId = await requireCredentialProviderBinding(

0 commit comments

Comments
 (0)