diff --git a/apps/sim/app/api/credentials/[id]/route.test.ts b/apps/sim/app/api/credentials/[id]/route.test.ts new file mode 100644 index 00000000000..f3c7ea98e94 --- /dev/null +++ b/apps/sim/app/api/credentials/[id]/route.test.ts @@ -0,0 +1,58 @@ +/** + * @vitest-environment node + */ +import { authMockFns, createMockRequest } from '@sim/testing' +import { beforeEach, describe, expect, it, vi } from 'vitest' +import { CredentialAccessRequiredError } from '@/lib/credentials/application/authorized-credential-use-case' + +const mocks = vi.hoisted(() => ({ + read: vi.fn(), + update: vi.fn(), + remove: vi.fn(), +})) + +vi.mock('@/lib/credentials/application/credential-crud', () => ({ + CredentialProviderOperationError: class CredentialProviderOperationError extends Error {}, + getWorkspaceCredentialUseCase: { + operation: { id: 'credentials.read' }, + execute: mocks.read, + }, + updateWorkspaceCredentialUseCase: { + operation: { id: 'credentials.update' }, + execute: mocks.update, + }, +})) + +vi.mock('@/lib/credentials/application/service-account', () => ({ + deleteCredentialUseCase: { + operation: { id: 'credentials.delete' }, + execute: mocks.remove, + }, +})) + +import { GET } from '@/app/api/credentials/[id]/route' + +const CREDENTIAL_ID = 'credential-1' +const routeContext = { params: Promise.resolve({ id: CREDENTIAL_ID }) } + +describe('GET /api/credentials/[id]', () => { + beforeEach(() => { + vi.clearAllMocks() + authMockFns.mockGetSession.mockResolvedValue({ + user: { id: 'writer-1' }, + session: { id: 'session-1' }, + }) + }) + + it('preserves the generic denial for a workspace writer without credential access', async () => { + mocks.read.mockRejectedValue(new CredentialAccessRequiredError()) + + const response = await GET( + createMockRequest('GET', undefined, {}, `http://localhost/api/credentials/${CREDENTIAL_ID}`), + routeContext + ) + + expect(response.status).toBe(403) + expect(await response.json()).toEqual({ error: 'Forbidden' }) + }) +}) diff --git a/apps/sim/app/api/credentials/[id]/route.ts b/apps/sim/app/api/credentials/[id]/route.ts index d99ad8382c4..0010a8af252 100644 --- a/apps/sim/app/api/credentials/[id]/route.ts +++ b/apps/sim/app/api/credentials/[id]/route.ts @@ -10,6 +10,7 @@ import { } from '@/lib/api/server/routes' import { credentialValidationParseOptions, + internalCredentialDetailErrorPolicy, internalCredentialErrorPolicy, } from '@/lib/credentials/api/route-policies' import { @@ -27,7 +28,7 @@ export const GET = defineInternalJsonRoute({ auth: internalSessionAuth, operation: credentialOperations.read, rateLimit, - errorPolicy: internalCredentialErrorPolicy, + errorPolicy: internalCredentialDetailErrorPolicy, parseOptions: credentialValidationParseOptions, mapInput: ({ params }) => ({ credentialId: params.id }), useCase: getWorkspaceCredentialUseCase, diff --git a/apps/sim/lib/credentials/api/route-policies.ts b/apps/sim/lib/credentials/api/route-policies.ts index 8e97f55c2ce..717766dec51 100644 --- a/apps/sim/lib/credentials/api/route-policies.ts +++ b/apps/sim/lib/credentials/api/route-policies.ts @@ -7,6 +7,7 @@ import { getValidationErrorMessage, validationErrorResponse } from '@/lib/api/se import { NoWorkspaceAccessError } from '@/lib/core/application' import { ForbiddenOperationError } from '@/lib/core/application/forbidden' import { OrchestrationError } from '@/lib/core/orchestration/types' +import { CredentialAccessRequiredError } from '@/lib/credentials/application/authorized-credential-use-case' import { CredentialProviderOperationError } from '@/lib/credentials/application/credential-crud' export const credentialValidationParseOptions = { @@ -25,6 +26,14 @@ export const internalCredentialErrorPolicy = extendInternalErrorPolicy( } ) +export const internalCredentialDetailErrorPolicy = extendInternalErrorPolicy( + internalCredentialErrorPolicy, + (error) => + error instanceof CredentialAccessRequiredError + ? internalErrorResponse(403, { error: 'Forbidden' }) + : null +) + export const internalCredentialMemberListErrorPolicy = extendInternalErrorPolicy( internalCredentialErrorPolicy, (error) => { diff --git a/apps/sim/lib/credentials/application/authorized-credential-use-case.test.ts b/apps/sim/lib/credentials/application/authorized-credential-use-case.test.ts index dbfe9f99157..b0a1939edfd 100644 --- a/apps/sim/lib/credentials/application/authorized-credential-use-case.test.ts +++ b/apps/sim/lib/credentials/application/authorized-credential-use-case.test.ts @@ -3,7 +3,10 @@ */ import { beforeEach, describe, expect, it, vi } from 'vitest' import { defineWorkspaceOperation } from '@/lib/core/application' -import { defineAuthorizedCredentialUseCase } from '@/lib/credentials/application/authorized-credential-use-case' +import { + CredentialAccessRequiredError, + defineAuthorizedCredentialUseCase, +} from '@/lib/credentials/application/authorized-credential-use-case' import { defineCredentialOperation } from '@/lib/credentials/application/operations' const mocks = vi.hoisted(() => ({ @@ -85,6 +88,20 @@ describe('defineAuthorizedCredentialUseCase', () => { expect(mocks.execute).toHaveBeenCalledOnce() }) + it('denies member-level reads without credential membership', async () => { + mocks.getActor.mockResolvedValue({ + credential, + member: null, + hasWorkspaceAccess: true, + isAdmin: false, + }) + + await expect( + createUseCase(memberOperation).execute({ principal, input: undefined }) + ).rejects.toBeInstanceOf(CredentialAccessRequiredError) + expect(mocks.execute).not.toHaveBeenCalled() + }) + it('requires credential admin independently of workspace read access', async () => { await expect( createUseCase(adminOperation).execute({ principal, input: undefined }) diff --git a/apps/sim/lib/credentials/application/authorized-credential-use-case.ts b/apps/sim/lib/credentials/application/authorized-credential-use-case.ts index 635fc9eb621..57e6d109993 100644 --- a/apps/sim/lib/credentials/application/authorized-credential-use-case.ts +++ b/apps/sim/lib/credentials/application/authorized-credential-use-case.ts @@ -17,6 +17,13 @@ export interface CredentialAuthorizationContext extends WorkspaceAuthorizationCo credentialAccess?: CredentialActorContext } +export class CredentialAccessRequiredError extends OrchestrationError { + constructor() { + super('forbidden', 'Credential access required') + this.name = 'CredentialAccessRequiredError' + } +} + export function requireCredentialAccess( context: CredentialAuthorizationContext ): CredentialActorContext { @@ -61,7 +68,7 @@ export function defineAuthorizedCredentialUseCase< switch (definition.operation.minimumCredentialRole) { case 'member': if (!actor.member && !actor.isAdmin) { - throw new OrchestrationError('forbidden', 'Credential access required') + throw new CredentialAccessRequiredError() } return case 'admin':