Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 2 additions & 3 deletions apps/sim/app/api/v2/files/[fileId]/content/route.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ vi.mock('@/lib/users/queries', () => ({
requireResolvedUserEmail: (emails: Map<string, string>, userId: string) => emails.get(userId)!,
}))

import { NoWorkspaceAccessError } from '@/lib/core/application'
import { OrchestrationError } from '@/lib/core/orchestration/types'
import { PUT } from '@/app/api/v2/files/[fileId]/content/route'

Expand Down Expand Up @@ -110,9 +111,7 @@ describe('PUT /api/v2/files/[fileId]/content', () => {
})

it('performs authenticated admission before parsing a large or malformed body', async () => {
mocks.admit.mockRejectedValue(
new OrchestrationError('forbidden', 'Insufficient workspace permissions')
)
mocks.admit.mockRejectedValue(new NoWorkspaceAccessError())

const response = await callPut('{not-json')

Expand Down
6 changes: 2 additions & 4 deletions apps/sim/app/api/v2/files/[fileId]/metadata/route.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ vi.mock('@/lib/users/queries', () => ({
requireResolvedUserEmail: (emails: Map<string, string>, userId: string) => emails.get(userId)!,
}))

import { OrchestrationError } from '@/lib/core/orchestration/types'
import { NoWorkspaceAccessError } from '@/lib/core/application'
import { GET } from '@/app/api/v2/files/[fileId]/metadata/route'

const WORKSPACE_ID = 'workspace-1'
Expand Down Expand Up @@ -114,9 +114,7 @@ describe('GET /api/v2/files/[fileId]/metadata', () => {
})

it('conceals an authorization failure as not found', async () => {
mocks.readMetadata.mockRejectedValue(
new OrchestrationError('forbidden', 'Insufficient workspace permissions')
)
mocks.readMetadata.mockRejectedValue(new NoWorkspaceAccessError())

const response = await callGet(`workspaceId=${WORKSPACE_ID}`)

Expand Down
31 changes: 24 additions & 7 deletions apps/sim/app/api/v2/files/[fileId]/route.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,10 @@ vi.mock('@/lib/users/queries', () => ({
requireResolvedUserEmail: (emails: Map<string, string>, userId: string) => emails.get(userId)!,
}))

import {
InsufficientWorkspacePermissionsError,
NoWorkspaceAccessError,
} from '@/lib/core/application'
import { OrchestrationError } from '@/lib/core/orchestration/types'
import { DELETE, GET, PATCH } from '@/app/api/v2/files/[fileId]/route'

Expand Down Expand Up @@ -136,9 +140,7 @@ describe('v2 single-file routes', () => {
})

it('conceals download authorization failures', async () => {
mocks.download.mockRejectedValue(
new OrchestrationError('forbidden', 'Insufficient workspace permissions')
)
mocks.download.mockRejectedValue(new NoWorkspaceAccessError())

const response = await GET(
new NextRequest(`http://localhost:3000/api/v2/files/${FILE_ID}?workspaceId=${WORKSPACE_ID}`),
Expand Down Expand Up @@ -166,7 +168,7 @@ describe('v2 single-file routes', () => {
})
})

it('maps rename conflicts and conceals authorization failures', async () => {
it('maps rename conflicts and conceals absent workspace access', async () => {
mocks.rename.mockRejectedValueOnce(new OrchestrationError('conflict', 'Name exists'))
const conflict = await PATCH(
new NextRequest(`http://localhost:3000/api/v2/files/${FILE_ID}`, {
Expand All @@ -178,9 +180,7 @@ describe('v2 single-file routes', () => {
)
expect(conflict.status).toBe(409)

mocks.rename.mockRejectedValueOnce(
new OrchestrationError('forbidden', 'Insufficient workspace permissions')
)
mocks.rename.mockRejectedValueOnce(new NoWorkspaceAccessError())
const concealed = await PATCH(
new NextRequest(`http://localhost:3000/api/v2/files/${FILE_ID}`, {
method: 'PATCH',
Expand All @@ -192,6 +192,23 @@ describe('v2 single-file routes', () => {
expect(concealed.status).toBe(404)
})

it('returns forbidden when the current workspace role cannot rename the file', async () => {
mocks.rename.mockRejectedValueOnce(new InsufficientWorkspacePermissionsError())
const response = await PATCH(
new NextRequest(`http://localhost:3000/api/v2/files/${FILE_ID}`, {
method: 'PATCH',
headers: { 'Content-Type': 'application/json' },
body: JSON.stringify({ workspaceId: WORKSPACE_ID, name: 'renamed.csv' }),
}),
context
)

expect(response.status).toBe(403)
expect(await response.json()).toEqual({
error: { code: 'FORBIDDEN', message: 'Insufficient workspace permissions' },
})
})

it('archives through the same principal and operation pipeline', async () => {
const request = new NextRequest(
`http://localhost:3000/api/v2/files/${FILE_ID}?workspaceId=${WORKSPACE_ID}`,
Expand Down
9 changes: 3 additions & 6 deletions apps/sim/app/api/v2/workflows/[id]/route.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,10 +36,7 @@ vi.mock('@/lib/core/rate-limiter', () => ({
}))
vi.mock('@/app/api/v2/lib/gate', () => ({ v2ApiGateError: mocks.gate }))

import {
InsufficientWorkspacePermissionsError,
PersonalApiKeysDisabledError,
} from '@/lib/core/application'
import { NoWorkspaceAccessError, PersonalApiKeysDisabledError } from '@/lib/core/application'
import { DELETE, GET, PATCH } from '@/app/api/v2/workflows/[id]/route'

const WORKSPACE_ID = 'workspace-1'
Expand Down Expand Up @@ -124,8 +121,8 @@ describe('/api/v2/workflows/[id]', () => {
})
})

it('conceals typed insufficient authorization as workflow absence', async () => {
mocks.readWorkflow.mockRejectedValue(new InsufficientWorkspacePermissionsError())
it('conceals absent workspace access as workflow absence', async () => {
mocks.readWorkflow.mockRejectedValue(new NoWorkspaceAccessError())
const response = await GET(
new NextRequest(`http://localhost/api/v2/workflows/${WORKFLOW_ID}`),
routeContext
Expand Down
15 changes: 9 additions & 6 deletions apps/sim/app/api/v2/workflows/[id]/runs/[runId]/route.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,10 @@ vi.mock('@/lib/workflows/application/cancel-run', () => ({
},
}))

import { InsufficientWorkspacePermissionsError } from '@/lib/core/application'
import {
InsufficientWorkspacePermissionsError,
NoWorkspaceAccessError,
} from '@/lib/core/application'
import { POST as cancelPost } from '@/app/api/v2/workflows/[id]/runs/[runId]/cancel/route'
import { GET } from '@/app/api/v2/workflows/[id]/runs/[runId]/route'

Expand Down Expand Up @@ -194,7 +197,7 @@ describe('v2 run detail and cancel adapters', () => {
})

it('conceals canonical run authorization failures as absence', async () => {
mocks.readRun.mockRejectedValueOnce(new InsufficientWorkspacePermissionsError())
mocks.readRun.mockRejectedValueOnce(new NoWorkspaceAccessError())

const response = await callStatus()

Expand Down Expand Up @@ -263,17 +266,17 @@ describe('v2 run detail and cancel adapters', () => {
expect(mocks.cancel).not.toHaveBeenCalled()
})

it('conceals cancellation authorization failures using canonical run policy', async () => {
it('returns forbidden when the current workspace role cannot cancel the run', async () => {
mocks.cancel.mockRejectedValueOnce(new InsufficientWorkspacePermissionsError())

const response = await cancelPost(createMockRequest('POST', undefined, {}), {
params: Promise.resolve({ id: 'workflow-1', runId: 'run-1' }),
})

expect(response.status).toBe(404)
expect(response.status).toBe(403)
expect((await response.json()).error).toMatchObject({
code: 'NOT_FOUND',
message: 'Run not found',
code: 'FORBIDDEN',
message: 'Insufficient workspace permissions',
})
expect(mocks.capture).not.toHaveBeenCalled()
})
Expand Down
7 changes: 2 additions & 5 deletions apps/sim/app/api/v2/workflows/[id]/runs/route.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,10 +35,7 @@ vi.mock('@/lib/workflows/application/list-workflow-runs', () => ({
},
}))

import {
InsufficientWorkspacePermissionsError,
PersonalApiKeysDisabledError,
} from '@/lib/core/application'
import { NoWorkspaceAccessError, PersonalApiKeysDisabledError } from '@/lib/core/application'
import { GET } from '@/app/api/v2/workflows/[id]/runs/route'

const principal = {
Expand Down Expand Up @@ -176,7 +173,7 @@ describe('GET /api/v2/workflows/[id]/runs', () => {
})

it('conceals workflow authorization failures as absence', async () => {
mocks.listRuns.mockRejectedValueOnce(new InsufficientWorkspacePermissionsError())
mocks.listRuns.mockRejectedValueOnce(new NoWorkspaceAccessError())

const response = await callGet()

Expand Down
19 changes: 15 additions & 4 deletions apps/sim/lib/api/server/routes/v2-resource-concealment.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import type { V2ErrorPolicy } from '@/lib/api/server/routes'
import {
DelegatedWorkspaceAuthorizationError,
InsufficientWorkspacePermissionsError,
NoWorkspaceAccessError,
PersonalApiKeysDisabledError,
PrincipalKindAuthorizationError,
WorkspaceApiKeyAuthorizationError,
Expand Down Expand Up @@ -59,7 +60,7 @@ const policies: Array<{
]

const resourceAuthorizationErrors = [
new InsufficientWorkspacePermissionsError(),
new NoWorkspaceAccessError(),
new WorkspaceApiKeyAuthorizationError(),
new DelegatedWorkspaceAuthorizationError(),
new PrincipalKindAuthorizationError('workspace_api_key', 'resources.read'),
Expand Down Expand Up @@ -88,11 +89,21 @@ describe.each(policies)('$domain resource concealment', ({ policy, notFoundMessa
})
})

it('preserves unrelated forbidden business failures', async () => {
const response = policy.render(new OrchestrationError('forbidden', 'Business rule denied'))
it('preserves insufficient workspace role as forbidden', async () => {
const response = policy.render(new InsufficientWorkspacePermissionsError())
expect(response?.status).toBe(403)
await expect(response?.json()).resolves.toEqual({
error: { code: 'FORBIDDEN', message: 'Business rule denied' },
error: { code: 'FORBIDDEN', message: 'Insufficient workspace permissions' },
})
})

it('does not classify generic forbidden errors by message', async () => {
const response = policy.render(
new OrchestrationError('forbidden', 'Insufficient workspace permissions')
)
expect(response?.status).toBe(403)
await expect(response?.json()).resolves.toEqual({
error: { code: 'FORBIDDEN', message: 'Insufficient workspace permissions' },
})
})
})
4 changes: 2 additions & 2 deletions apps/sim/lib/api/server/routes/v2-resource-concealment.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import type { V2ErrorPolicy } from '@/lib/api/server/routes/v2-json-route'
import {
DelegatedWorkspaceAuthorizationError,
InsufficientWorkspacePermissionsError,
NoWorkspaceAccessError,
PrincipalKindAuthorizationError,
WorkspaceApiKeyAuthorizationError,
} from '@/lib/core/application'
Expand All @@ -12,7 +12,7 @@ type V2ErrorRenderer = V2ErrorPolicy['render']
function isResourceAuthorizationError(error: unknown): boolean {
return (
error instanceof DelegatedWorkspaceAuthorizationError ||
error instanceof InsufficientWorkspacePermissionsError ||
error instanceof NoWorkspaceAccessError ||
error instanceof PrincipalKindAuthorizationError ||
error instanceof WorkspaceApiKeyAuthorizationError
)
Expand Down
1 change: 1 addition & 0 deletions apps/sim/lib/core/application/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ export {
DelegatedServiceAuthorizationError,
DelegatedWorkspaceAuthorizationError,
InsufficientWorkspacePermissionsError,
NoWorkspaceAccessError,
PersonalApiKeysDisabledError,
PrincipalKindAuthorizationError,
requireAllowedWorkspacePrincipal,
Expand Down
73 changes: 73 additions & 0 deletions apps/sim/lib/core/application/workspace-authorization.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
/**
* @vitest-environment node
*/
import type { SessionPrincipal } from '@sim/auth/principal'
import { beforeEach, describe, expect, it, vi } from 'vitest'

const mocks = vi.hoisted(() => ({
resolvePermission: vi.fn(),
}))

vi.mock('@sim/platform-authz/workspace', () => ({
permissionSatisfies: (actual: string, required: string) => {
const rank = { read: 1, write: 2, admin: 3 } as const
return rank[actual as keyof typeof rank] >= rank[required as keyof typeof rank]
},
resolveEffectiveWorkspacePermission: mocks.resolvePermission,
}))

import {
authorizeWorkspaceOperation,
defineWorkspaceOperation,
InsufficientWorkspacePermissionsError,
NoWorkspaceAccessError,
} from '@/lib/core/application'

const writeOperation = defineWorkspaceOperation({
id: 'test.write',
minimumRole: 'write',
workspaceApiKey: 'deny',
principalKinds: ['session'],
})

const principal: SessionPrincipal = {
kind: 'session',
userId: 'user-1',
sessionId: 'session-1',
}

const context = {
workspaceId: 'workspace-1',
workspaceOrganizationId: 'organization-1',
allowPersonalApiKeys: true,
}

describe('authorizeWorkspaceOperation', () => {
beforeEach(() => {
vi.clearAllMocks()
})

it('rejects a null effective permission as no workspace access', async () => {
mocks.resolvePermission.mockResolvedValue(null)

await expect(
authorizeWorkspaceOperation(principal, writeOperation, context)
).rejects.toBeInstanceOf(NoWorkspaceAccessError)
})

it('rejects a readable workspace as an insufficient role for a write operation', async () => {
mocks.resolvePermission.mockResolvedValue('read')

await expect(
authorizeWorkspaceOperation(principal, writeOperation, context)
).rejects.toBeInstanceOf(InsufficientWorkspacePermissionsError)
})

it('authorizes the write operation when the current role satisfies it', async () => {
mocks.resolvePermission.mockResolvedValue('write')

await expect(
authorizeWorkspaceOperation(principal, writeOperation, context)
).resolves.toBeUndefined()
})
})
10 changes: 10 additions & 0 deletions apps/sim/lib/core/application/workspace-authorization.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,13 @@ export class InsufficientWorkspacePermissionsError extends OrchestrationError {
}
}

export class NoWorkspaceAccessError extends OrchestrationError {
constructor() {
super('forbidden', 'Insufficient workspace permissions')
this.name = 'NoWorkspaceAccessError'
}
}

export class PersonalApiKeysDisabledError extends OrchestrationError {
constructor() {
super('forbidden', 'Personal API keys are not allowed for this workspace')
Expand Down Expand Up @@ -89,6 +96,9 @@ export function requireAllowedWorkspacePrincipal<O extends WorkspaceOperation>(
}

function requirePermission(permission: PermissionType | null, required: PermissionType): void {
if (permission === null) {
throw new NoWorkspaceAccessError()
}
if (!permissionSatisfies(permission, required)) {
throw new InsufficientWorkspacePermissionsError()
}
Expand Down
4 changes: 2 additions & 2 deletions apps/sim/lib/knowledge/api/route-policies.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
import { describe, expect, it } from 'vitest'
import {
DelegatedWorkspaceAuthorizationError,
InsufficientWorkspacePermissionsError,
NoWorkspaceAccessError,
PersonalApiKeysDisabledError,
PrincipalKindAuthorizationError,
WorkspaceApiKeyAuthorizationError,
Expand All @@ -15,7 +15,7 @@ import { v2KnowledgeErrorPolicies } from '@/lib/knowledge/api/route-policies'

describe('v2 knowledge error policies', () => {
it.each([
new InsufficientWorkspacePermissionsError(),
new NoWorkspaceAccessError(),
new WorkspaceApiKeyAuthorizationError(),
new DelegatedWorkspaceAuthorizationError(),
new PrincipalKindAuthorizationError('workspace_api_key', 'knowledge.read'),
Expand Down
Loading
Loading