fix(auth): check a workspace's personal-key switch after membership (#9540)

* fix(auth): check a workspace's personal-key switch after membership, so a non-member never learns what it named exists

* test(auth): import the key-switch error from the application barrel

Author: waleedlatif1

apps/sim/lib/core/application/workspace-authorization.test.ts

@@ -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)
   })
 })
 

apps/sim/lib/core/application/workspace-authorization.ts

@@ -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

apps/sim/lib/table/application/authorization.test.ts

@@ -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'
@@ -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 () => {

apps/sim/lib/workspace-files/application/authorization.test.ts

@@ -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'
@@ -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 () => {