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
11 changes: 5 additions & 6 deletions apps/sim/lib/core/application/workspace-authorization.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -488,20 +488,19 @@ describe('authorizeWorkspaceOperation personal API key policy', () => {
})

/**
* The workspace column keeps its fail-fast: it is a property of the workspace
* rather than of any group, so it is not the organization-configuration
* oracle the group key would be, and three call-site suites pin the ordering.
* The workspace column follows membership too: the context is loaded before
* authorization, so refusing the column first would tell a non-member that the
* workspace, and the resource it named in it, exist.
*/
it('keeps refusing the workspace personal-key column before the role lookup', async () => {
it('conceals the workspace before refusing the workspace personal-key column', async () => {
mocks.resolvePermission.mockResolvedValue(null)

await expect(
authorizeWorkspaceOperation(personalKeyPrincipal, personalKeyOperation, {
...context,
allowPersonalApiKeys: false,
})
).rejects.toBeInstanceOf(PersonalApiKeysDisabledError)
expect(mocks.resolvePermission).not.toHaveBeenCalled()
).rejects.toBeInstanceOf(NoWorkspaceAccessError)
})
})

Expand Down
19 changes: 9 additions & 10 deletions apps/sim/lib/core/application/workspace-authorization.ts
Original file line number Diff line number Diff line change
Expand Up @@ -380,33 +380,32 @@ export async function authorizeWorkspaceOperation<C extends WorkspaceAuthorizati
* narrows it further for one cohort inside an enterprise organization.
* Either one saying no is a no.
*
* The column is checked before the role, deliberately: it is a property of
* the workspace rather than of any group, it needs no query, and refusing
* a key the workspace has switched off is the answer whatever the caller's
* role turns out to be. The group key is not — it runs AFTER the role
* check, for the reason {@link requireCurrentHumanRole} gives: it answers
* with a `403` naming how an organization configured one cohort, and
* running it ahead of the concealed {@link NoWorkspaceAccessError} would
* hand that to a caller with no reach into the workspace at all.
* Both run AFTER the role check, for the reason {@link requireCurrentHumanRole}
* gives: each answers with a `403` naming how the workspace or its
* organization is configured, and running it ahead of the concealed
* {@link NoWorkspaceAccessError} would hand that to a caller with no reach
* into the workspace. The context is loaded before authorization, so a
* `403` there would also tell a non-member that the resource it named
* exists, where a missing one answers `404`.
*
* `requireCurrentHumanAccess` is unrolled below so the group's
* personal-key refusal sits between the role check and the operation's own
* capability: the remedies differ, and the narrower one is worth naming
* first.
*/
await requireCurrentHumanRole(principal.userId, context, operation.minimumRole, options)
if (!context.allowPersonalApiKeys) {
throw new PersonalApiKeysDisabledError()
}
await requireCurrentHumanRole(principal.userId, context, operation.minimumRole, options)
await requirePersonalApiKeysAllowed(principal.userId, context, options?.executor)
await requireCapability(principal.userId, context, operation, options?.executor)
return
/** OAuth scopes and expiry were checked before loading protected context. */
case 'oauth_access_token': {
await requireCurrentHumanRole(principal.userId, context, operation.minimumRole, options)
if (!context.allowPersonalApiKeys) {
throw new PersonalApiKeysDisabledError()
}
await requireCurrentHumanRole(principal.userId, context, operation.minimumRole, options)
await requireUserCredentialCapabilities(principal, context, options?.executor)
await requireCapability(principal.userId, context, operation, options?.executor)
return
Expand Down
7 changes: 4 additions & 3 deletions apps/sim/lib/table/application/authorization.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import { beforeEach, describe, expect, it, vi } from 'vitest'

vi.mock('@sim/platform-authz/workspace', () => workspaceAuthzMock)

import { PersonalApiKeysDisabledError } from '@/lib/core/application'
import type { OrchestrationError } from '@/lib/core/orchestration/types'
import { authorizeTableOperation } from '@/lib/table/application/authorization'
import { tableOperations } from '@/lib/table/application/operations'
Expand Down Expand Up @@ -65,14 +66,14 @@ describe('table operation authorization', () => {
await expectForbidden(createSessionPrincipal())
})

it('rejects disabled personal keys before permission lookup', async () => {
it("refuses a member's personal key when the workspace disables them", async () => {
resolvePermission.mockResolvedValue('admin')
await expect(
authorizeTableOperation(createPersonalApiKeyPrincipal(), tableOperations.updateRow, {
...authorizationContext,
allowPersonalApiKeys: false,
})
).rejects.toMatchObject<Partial<OrchestrationError>>({ code: 'forbidden' })
expect(resolvePermission).not.toHaveBeenCalled()
).rejects.toBeInstanceOf(PersonalApiKeysDisabledError)
})

it('allows workspace keys only in their credential workspace', async () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import { beforeEach, describe, expect, it, vi } from 'vitest'

vi.mock('@sim/platform-authz/workspace', () => workspaceAuthzMock)

import { PersonalApiKeysDisabledError } from '@/lib/core/application'
import type { OrchestrationError } from '@/lib/core/orchestration/types'
import { authorizeWorkspaceFileAccess } from '@/lib/workspace-files/application/authorization'
import { fileOperations } from '@/lib/workspace-files/application/operations'
Expand Down Expand Up @@ -53,14 +54,14 @@ describe('file operation authorization', () => {
})
})

it('fails a personal key immediately when the workspace disables it', async () => {
it("refuses a member's personal key when the workspace disables them", async () => {
resolvePermission.mockResolvedValue('admin')
await expect(
authorizeWorkspaceFileAccess(createPersonalApiKeyPrincipal(), fileOperations.rename, {
...authorizationContext,
allowPersonalApiKeys: false,
})
).rejects.toMatchObject<Partial<OrchestrationError>>({ code: 'forbidden' })
expect(resolvePermission).not.toHaveBeenCalled()
).rejects.toBeInstanceOf(PersonalApiKeysDisabledError)
})

it('rejects a disallowed principal kind before principal-specific authorization', async () => {
Expand Down
Loading