From bd9e3ce854da0e34ac2967298dd49d61b9a01f25 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Tue, 10 Nov 2026 12:40:53 -0800 Subject: [PATCH 1/6] feat(api): update and remove workspace members through the v2 API Add PATCH and DELETE /api/v2/workspaces/{workspaceId}/members/{userId}, backed by the existing permission-update and member-removal use cases. Both operations now admit personal API keys and OAuth tokens with api:write; workspace API keys stay refused. PATCH returns the member exactly as the roster lists them, read before the write so a committed change is never left unaudited. DELETE reports whether leaving removed the organization membership. A directory-managed member's refusal keeps its SCIM_MANAGED_MEMBERSHIP code through the store's HTTP wrapper. The v2 member policy answers the billing-account removal refusal as 409 and exhausted transaction contention as 503 instead of 500. The CLI gains `workspaces members update` and `workspaces members remove`; the listing becomes `workspaces members list` and shows user ids. --- apps/docs/content/docs/cli/reference.mdx | 66 +++- apps/docs/content/docs/cli/workspaces.mdx | 64 +++- apps/docs/openapi-v2-resources.json | 322 ++++++++++++++++++ .../scripts/fixtures/content-baseline.json | 42 ++- .../[workspaceId]/members/[userId]/route.ts | 47 +++ .../workspaces/[workspaceId]/members/route.ts | 11 +- .../api/workspaces/[id]/permissions/route.ts | 2 +- .../app/api/workspaces/members/[id]/route.ts | 6 +- .../lib/api/contracts/v2/openapi/resources.ts | 67 ++++ apps/sim/lib/api/contracts/v2/workspaces.ts | 62 +++- .../lib/api/mcp/generated/v2-operations.ts | 24 ++ .../server/routes/v2-route-table.generated.ts | 4 + .../lib/api/server/workspace-presenters.ts | 15 + apps/sim/lib/workspaces/api/route-policies.ts | 20 ++ .../application/manage-permissions.ts | 182 ++++++---- .../application/member-operations.ts | 42 +++ .../workspaces/application/remove-member.ts | 13 +- .../permissions/management-store.ts | 8 +- apps/sim/lib/workspaces/public-queries.ts | 24 +- packages/sim-cli/src/contract/commands.ts | 15 +- packages/sim-cli/src/generated/v2-api.ts | 77 +++++ packages/sim-cli/src/runtime/execute.ts | 2 +- scripts/check-unused-exports.baseline.json | 1 - 23 files changed, 1005 insertions(+), 111 deletions(-) create mode 100644 apps/sim/app/api/v2/workspaces/[workspaceId]/members/[userId]/route.ts create mode 100644 apps/sim/lib/api/server/workspace-presenters.ts create mode 100644 apps/sim/lib/workspaces/application/member-operations.ts diff --git a/apps/docs/content/docs/cli/reference.mdx b/apps/docs/content/docs/cli/reference.mdx index ff141b26964..8d077ff6c59 100644 --- a/apps/docs/content/docs/cli/reference.mdx +++ b/apps/docs/content/docs/cli/reference.mdx @@ -8849,12 +8849,12 @@ sim workspaces fork-resources [options] API request and response schemas: [listWorkspaceForkResources](/api-reference/workspace-sync/listWorkspaceForkResources). -### sim workspaces members +### sim workspaces members list List workspace members ```bash -sim workspaces members [options] +sim workspaces members list [options] ``` **Options** @@ -8869,6 +8869,68 @@ sim workspaces members [options] API request and response schemas: [listWorkspaceMembers](/api-reference/workspaces/listWorkspaceMembers). +### sim workspaces members remove + +Remove workspace member (OAuth login or personal API key required) + +This removes the member’s workspace access. If it was their last workspace in the organization, their organization membership is removed and their sessions end. + +```bash +sim workspaces members remove [options] +``` + +**Arguments** + + + +| Argument | Required | Description | +| --- | --- | --- | +| `userId` | Yes | User identifier of the workspace member, as returned by List workspace members. | + + + +**Options** + + + +| Option | Required | Description | +| --- | --- | --- | +| `-y, --yes` | Yes | Confirm this operation. | + + + +API request and response schemas: [removeWorkspaceMember](/api-reference/workspaces/removeWorkspaceMember). + +### sim workspaces members update + +Update workspace member (OAuth login or personal API key required) + +```bash +sim workspaces members update [options] +``` + +**Arguments** + + + +| Argument | Required | Description | +| --- | --- | --- | +| `userId` | Yes | User identifier of the workspace member, as returned by List workspace members. | + + + +**Options** + + + +| Option | Required | Description | +| --- | --- | --- | +| `--role ` | Yes | New workspace role. Inherited organization Admin access, the workspace owner, and the workspace billing account keep Admin and cannot be changed here. Accepted values: `admin`, `write`, `read`. | + + + +API request and response schemas: [updateWorkspaceMember](/api-reference/workspaces/updateWorkspaceMember). + ### sim workspaces list List workspaces diff --git a/apps/docs/content/docs/cli/workspaces.mdx b/apps/docs/content/docs/cli/workspaces.mdx index 826b0d42133..cf195f61761 100644 --- a/apps/docs/content/docs/cli/workspaces.mdx +++ b/apps/docs/content/docs/cli/workspaces.mdx @@ -44,7 +44,7 @@ API request and response schemas: [getWorkspace](/api-reference/workspaces/getWo ### List workspace members ```bash -sim workspaces members [options] +sim workspaces members list [options] ``` **Options** @@ -59,6 +59,68 @@ sim workspaces members [options] API request and response schemas: [listWorkspaceMembers](/api-reference/workspaces/listWorkspaceMembers). +### Update workspace member + +OAuth login or personal API key required. + +```bash +sim workspaces members update [options] +``` + +**Arguments** + + + +| Argument | Required | Description | +| --- | --- | --- | +| `userId` | Yes | User identifier of the workspace member, as returned by List workspace members. | + + + +**Options** + + + +| Option | Required | Description | +| --- | --- | --- | +| `--role ` | Yes | New workspace role. Inherited organization Admin access, the workspace owner, and the workspace billing account keep Admin and cannot be changed here. Accepted values: `admin`, `write`, `read`. | + + + +API request and response schemas: [updateWorkspaceMember](/api-reference/workspaces/updateWorkspaceMember). + +### Remove workspace member + +OAuth login or personal API key required. + +This removes the member’s workspace access. If it was their last workspace in the organization, their organization membership is removed and their sessions end. + +```bash +sim workspaces members remove [options] +``` + +**Arguments** + + + +| Argument | Required | Description | +| --- | --- | --- | +| `userId` | Yes | User identifier of the workspace member, as returned by List workspace members. | + + + +**Options** + + + +| Option | Required | Description | +| --- | --- | --- | +| `-y, --yes` | Yes | Confirm this operation. | + + + +API request and response schemas: [removeWorkspaceMember](/api-reference/workspaces/removeWorkspaceMember). + ### Create workspace invitations OAuth login or personal API key required. diff --git a/apps/docs/openapi-v2-resources.json b/apps/docs/openapi-v2-resources.json index 89d0ab247db..3c9bba988dc 100644 --- a/apps/docs/openapi-v2-resources.json +++ b/apps/docs/openapi-v2-resources.json @@ -421,6 +421,239 @@ } } }, + "/api/v2/workspaces/{workspaceId}/members/{userId}": { + "patch": { + "operationId": "updateWorkspaceMember", + "summary": "Update workspace member", + "description": "Change one existing member’s workspace role. Requires workspace Admin access. Only existing members can be changed; add people with Create workspace invitations. You cannot lower your own role, and organization Admins, the workspace owner, and the workspace billing account always keep Admin. Members whose organization membership an identity provider manages cannot be changed here. Requesting the role a member already holds changes nothing and records no audit entry. Workspace API keys return `403`; use a personal API key or scoped OAuth token.\n\nOAuth scope: `api:write`.", + "x-sim-operation": "workspaces.permissions.update", + "x-oauth-scope": "api:write", + "tags": ["Workspaces"], + "security": [ + { + "apiKey": [] + }, + { + "oauthBearer": ["api:write"] + } + ], + "parameters": [ + { + "name": "workspaceId", + "in": "path", + "required": true, + "description": "Workspace to retrieve.", + "schema": { + "type": "string", + "minLength": 1, + "maxLength": 128, + "description": "Workspace to retrieve." + } + }, + { + "name": "userId", + "in": "path", + "required": true, + "description": "User identifier of the workspace member, as returned by List workspace members.", + "schema": { + "type": "string", + "minLength": 1, + "maxLength": 128, + "description": "User identifier of the workspace member, as returned by List workspace members." + } + } + ], + "requestBody": { + "required": true, + "description": "The new workspace role for this member.", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/UpdateWorkspaceMemberBody" + }, + "examples": { + "example1": { + "summary": "Request example 1", + "value": { + "role": "write" + } + } + } + } + } + }, + "responses": { + "200": { + "description": "The member with their updated workspace role.", + "headers": { + "X-RateLimit-Limit": { + "$ref": "#/components/headers/X-RateLimit-Limit" + }, + "X-RateLimit-Remaining": { + "$ref": "#/components/headers/X-RateLimit-Remaining" + }, + "X-RateLimit-Reset": { + "$ref": "#/components/headers/X-RateLimit-Reset" + } + }, + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/UpdateWorkspaceMemberResponse" + }, + "examples": { + "example1": { + "summary": "Response example 1", + "value": { + "data": { + "userId": "user-123", + "email": "jane@example.com", + "name": "Jane Smith", + "image": null, + "role": "write", + "isExternal": false, + "joinedAt": "2026-01-15T10:30:00.000Z" + } + } + } + } + } + } + }, + "400": { + "$ref": "#/components/responses/BadRequest" + }, + "401": { + "$ref": "#/components/responses/Unauthorized" + }, + "403": { + "$ref": "#/components/responses/Forbidden" + }, + "404": { + "$ref": "#/components/responses/NotFound" + }, + "409": { + "$ref": "#/components/responses/Conflict" + }, + "413": { + "$ref": "#/components/responses/PayloadTooLarge" + }, + "415": { + "$ref": "#/components/responses/UnsupportedMediaType" + }, + "429": { + "$ref": "#/components/responses/RateLimited" + }, + "500": { + "$ref": "#/components/responses/InternalError" + }, + "503": { + "$ref": "#/components/responses/ServiceUnavailable" + } + } + }, + "delete": { + "operationId": "removeWorkspaceMember", + "summary": "Remove workspace member", + "description": "Remove a member’s workspace access, or leave the workspace by naming yourself. Removing someone else requires workspace Admin access; any member may leave. The last Admin cannot leave, organization Admins hold access through their organization role and cannot be removed here, and the workspace billing account cannot be removed until billing is reassigned. Removing the workspace owner transfers ownership to the billing account. When this was the member’s last workspace in its organization, their organization membership and seat are removed too and all of their sessions end. Workspace API keys return `403`; use a personal API key or scoped OAuth token.\n\nOAuth scope: `api:write`.", + "x-sim-operation": "workspaces.members.remove", + "x-oauth-scope": "api:write", + "tags": ["Workspaces"], + "security": [ + { + "apiKey": [] + }, + { + "oauthBearer": ["api:write"] + } + ], + "parameters": [ + { + "name": "workspaceId", + "in": "path", + "required": true, + "description": "Workspace to retrieve.", + "schema": { + "type": "string", + "minLength": 1, + "maxLength": 128, + "description": "Workspace to retrieve." + } + }, + { + "name": "userId", + "in": "path", + "required": true, + "description": "User identifier of the workspace member, as returned by List workspace members.", + "schema": { + "type": "string", + "minLength": 1, + "maxLength": 128, + "description": "User identifier of the workspace member, as returned by List workspace members." + } + } + ], + "responses": { + "200": { + "description": "Acknowledgement that workspace access was removed.", + "headers": { + "X-RateLimit-Limit": { + "$ref": "#/components/headers/X-RateLimit-Limit" + }, + "X-RateLimit-Remaining": { + "$ref": "#/components/headers/X-RateLimit-Remaining" + }, + "X-RateLimit-Reset": { + "$ref": "#/components/headers/X-RateLimit-Reset" + } + }, + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/RemoveWorkspaceMemberResponse" + }, + "examples": { + "example1": { + "summary": "Response example 1", + "value": { + "data": { + "userId": "user-123", + "deleted": true, + "organizationMembershipRemoved": false + } + } + } + } + } + } + }, + "400": { + "$ref": "#/components/responses/BadRequest" + }, + "401": { + "$ref": "#/components/responses/Unauthorized" + }, + "403": { + "$ref": "#/components/responses/Forbidden" + }, + "404": { + "$ref": "#/components/responses/NotFound" + }, + "409": { + "$ref": "#/components/responses/Conflict" + }, + "429": { + "$ref": "#/components/responses/RateLimited" + }, + "500": { + "$ref": "#/components/responses/InternalError" + }, + "503": { + "$ref": "#/components/responses/ServiceUnavailable" + } + } + } + }, "/api/v2/mcp-servers": { "get": { "operationId": "listMcpServers", @@ -14836,6 +15069,95 @@ } ] }, + "UpdateWorkspaceMemberResponse": { + "type": "object", + "properties": { + "data": { + "description": "Response data.", + "$ref": "#/components/schemas/V2WorkspaceMember" + } + }, + "required": ["data"], + "additionalProperties": false, + "title": "Update workspace member response", + "description": "The member with their updated workspace role.", + "examples": [ + { + "data": { + "userId": "user-123", + "email": "jane@example.com", + "name": "Jane Smith", + "image": null, + "role": "write", + "isExternal": false, + "joinedAt": "2026-01-15T10:30:00.000Z" + } + } + ] + }, + "UpdateWorkspaceMemberBody": { + "type": "object", + "properties": { + "role": { + "type": "string", + "enum": ["admin", "write", "read"], + "description": "New workspace role. Inherited organization Admin access, the workspace owner, and the workspace billing account keep Admin and cannot be changed here." + } + }, + "required": ["role"], + "additionalProperties": false, + "title": "Update workspace member body", + "description": "The new workspace role for this member.", + "examples": [ + { + "role": "write" + } + ] + }, + "V2WorkspaceMemberDeletion": { + "type": "object", + "properties": { + "userId": { + "type": "string", + "description": "User removed from the workspace." + }, + "deleted": { + "type": "boolean", + "const": true, + "description": "Whether workspace access was removed." + }, + "organizationMembershipRemoved": { + "type": "boolean", + "description": "Whether this was the member’s last workspace in the organization, so their organization membership and seat were removed too." + } + }, + "required": ["userId", "deleted", "organizationMembershipRemoved"], + "additionalProperties": false, + "title": "Workspace member removal", + "description": "Acknowledges removal of a workspace member." + }, + "RemoveWorkspaceMemberResponse": { + "type": "object", + "properties": { + "data": { + "description": "Response data.", + "$ref": "#/components/schemas/V2WorkspaceMemberDeletion" + } + }, + "required": ["data"], + "additionalProperties": false, + "title": "Remove workspace member response", + "description": "Acknowledgement that workspace access was removed.", + "examples": [ + { + "data": { + "userId": "user-123", + "deleted": true, + "organizationMembershipRemoved": false + } + } + ] + }, "V2McpServer": { "type": "object", "properties": { diff --git a/apps/docs/scripts/fixtures/content-baseline.json b/apps/docs/scripts/fixtures/content-baseline.json index b192484a92d..ca730d5600a 100644 --- a/apps/docs/scripts/fixtures/content-baseline.json +++ b/apps/docs/scripts/fixtures/content-baseline.json @@ -8555,8 +8555,8 @@ "depth": 3 }, { - "title": "sim workspaces members", - "url": "#sim-workspaces-members", + "title": "sim workspaces members list", + "url": "#sim-workspaces-members-list", "depth": 3 }, { @@ -13178,9 +13178,23 @@ { "lang": "bash", "meta": null, - "sha256": "ea1cc85c4e5a1b39b5b26bca357d22583d993fbd321ba62dc529b811a553b050", - "bytes": 32, - "line": 8776 + "sha256": "210e8ab3d8af173ce648501ca50c828325ac1f4452006ad4485cb7a24d760ae4", + "bytes": 37, + "line": 8852 + }, + { + "lang": "bash", + "meta": null, + "sha256": "cdff2d4272d5bc3a18729e63e5ece9053d33ac0f8faf35cfaa3a289baa2dccc1", + "bytes": 48, + "line": 8874 + }, + { + "lang": "bash", + "meta": null, + "sha256": "c307203dd609208b90cbc0372e3401a75e943421bdf77e4e10779ed2c89ca4ff", + "bytes": 48, + "line": 8904 }, { "lang": "bash", @@ -17732,10 +17746,24 @@ { "lang": "bash", "meta": null, - "sha256": "ea1cc85c4e5a1b39b5b26bca357d22583d993fbd321ba62dc529b811a553b050", - "bytes": 32, + "sha256": "210e8ab3d8af173ce648501ca50c828325ac1f4452006ad4485cb7a24d760ae4", + "bytes": 37, "line": 42 }, + { + "lang": "bash", + "meta": null, + "sha256": "c307203dd609208b90cbc0372e3401a75e943421bdf77e4e10779ed2c89ca4ff", + "bytes": 48, + "line": 62 + }, + { + "lang": "bash", + "meta": null, + "sha256": "cdff2d4272d5bc3a18729e63e5ece9053d33ac0f8faf35cfaa3a289baa2dccc1", + "bytes": 48, + "line": 94 + }, { "lang": "bash", "meta": null, diff --git a/apps/sim/app/api/v2/workspaces/[workspaceId]/members/[userId]/route.ts b/apps/sim/app/api/v2/workspaces/[workspaceId]/members/[userId]/route.ts new file mode 100644 index 00000000000..df4ca574f2b --- /dev/null +++ b/apps/sim/app/api/v2/workspaces/[workspaceId]/members/[userId]/route.ts @@ -0,0 +1,47 @@ +import { + v2RemoveWorkspaceMemberContract, + v2UpdateWorkspaceMemberContract, +} from '@/lib/api/contracts/v2/workspaces' +import { defineV2JsonRoute, v2ApiKeyAuth, v2RateLimits } from '@/lib/api/server/routes' +import { presentWorkspaceMember } from '@/lib/api/server/workspace-presenters' +import { v2WorkspaceErrorPolicies } from '@/lib/workspaces/api/route-policies' +import { updateWorkspaceMember } from '@/lib/workspaces/application/manage-permissions' +import { + removeWorkspaceMemberOperation, + workspacePermissionOperations, +} from '@/lib/workspaces/application/member-operations' +import { removeWorkspaceMember } from '@/lib/workspaces/application/remove-member' + +/** PATCH /api/v2/workspaces/[workspaceId]/members/[userId] — Change one member's role. */ +export const PATCH = defineV2JsonRoute({ + contract: v2UpdateWorkspaceMemberContract, + operation: workspacePermissionOperations.update, + auth: v2ApiKeyAuth, + rateLimit: v2RateLimits.publicApi, + errorPolicy: v2WorkspaceErrorPolicies.manageWorkspaceMembers, + mapInput: ({ params, body }) => ({ + workspaceId: params.workspaceId, + userId: params.userId, + role: body.role, + }), + useCase: updateWorkspaceMember, + present: ({ member }) => ({ data: presentWorkspaceMember(member) }), +}) + +/** DELETE /api/v2/workspaces/[workspaceId]/members/[userId] — Remove a member, or leave. */ +export const DELETE = defineV2JsonRoute({ + contract: v2RemoveWorkspaceMemberContract, + operation: removeWorkspaceMemberOperation, + auth: v2ApiKeyAuth, + rateLimit: v2RateLimits.publicApi, + errorPolicy: v2WorkspaceErrorPolicies.manageWorkspaceMembers, + mapInput: ({ params }) => params, + useCase: removeWorkspaceMember, + present: (result) => ({ + data: { + userId: result.removedUserId, + deleted: true as const, + organizationMembershipRemoved: result.organizationRemoval, + }, + }), +}) diff --git a/apps/sim/app/api/v2/workspaces/[workspaceId]/members/route.ts b/apps/sim/app/api/v2/workspaces/[workspaceId]/members/route.ts index b12db8d4594..f047d313d36 100644 --- a/apps/sim/app/api/v2/workspaces/[workspaceId]/members/route.ts +++ b/apps/sim/app/api/v2/workspaces/[workspaceId]/members/route.ts @@ -4,6 +4,7 @@ import { } from '@/lib/api/contracts/v2/workspaces' import { cursorRoute, cursorScopeKey, UNREADABLE_CURSOR_MESSAGE } from '@/lib/api/cursor-binding' import { defineV2JsonRoute, v2ApiKeyAuth, v2RateLimits } from '@/lib/api/server/routes' +import { presentWorkspaceMember } from '@/lib/api/server/workspace-presenters' import { OrchestrationError } from '@/lib/core/orchestration/types' import { v2WorkspaceErrorPolicies } from '@/lib/workspaces/api/route-policies' import { listPublicWorkspaceMembers } from '@/lib/workspaces/application/list-public-workspace-members' @@ -50,15 +51,7 @@ export const GET = defineV2JsonRoute({ }, useCase: listPublicWorkspaceMembers, present: ({ page }, { params }) => ({ - data: page.members.map((member) => ({ - userId: member.userId, - email: member.email, - name: member.name, - image: member.image, - role: member.role, - isExternal: member.isExternal, - joinedAt: member.joinedAt.toISOString(), - })), + data: page.members.map(presentWorkspaceMember), nextCursor: page.nextEmail ? encodeScopedCursor( memberCursorScope(params.workspaceId), diff --git a/apps/sim/app/api/workspaces/[id]/permissions/route.ts b/apps/sim/app/api/workspaces/[id]/permissions/route.ts index a8b28c7547f..a8f749ae12e 100644 --- a/apps/sim/app/api/workspaces/[id]/permissions/route.ts +++ b/apps/sim/app/api/workspaces/[id]/permissions/route.ts @@ -22,8 +22,8 @@ import { captureServerEvent } from '@/lib/posthog/server' import { readWorkspacePermissions, updateWorkspacePermissions, - workspacePermissionOperations, } from '@/lib/workspaces/application/manage-permissions' +import { workspacePermissionOperations } from '@/lib/workspaces/application/member-operations' import { WorkspacePermissionError } from '@/lib/workspaces/permissions/management-store' function permissionErrorResponse(error: unknown): NextResponse { diff --git a/apps/sim/app/api/workspaces/members/[id]/route.ts b/apps/sim/app/api/workspaces/members/[id]/route.ts index 0c9a109c23c..d86f23b39e4 100644 --- a/apps/sim/app/api/workspaces/members/[id]/route.ts +++ b/apps/sim/app/api/workspaces/members/[id]/route.ts @@ -11,10 +11,8 @@ import { NoWorkspaceAccessError, } from '@/lib/core/application/workspace-authorization' import { captureServerEvent } from '@/lib/posthog/server' -import { - removeWorkspaceMember, - removeWorkspaceMemberOperation, -} from '@/lib/workspaces/application/remove-member' +import { removeWorkspaceMemberOperation } from '@/lib/workspaces/application/member-operations' +import { removeWorkspaceMember } from '@/lib/workspaces/application/remove-member' import { WorkspaceBillingAccountRemovalError } from '@/lib/workspaces/utils' export const DELETE = defineInternalJsonRoute({ diff --git a/apps/sim/lib/api/contracts/v2/openapi/resources.ts b/apps/sim/lib/api/contracts/v2/openapi/resources.ts index b8416de9b3a..febb8965f27 100644 --- a/apps/sim/lib/api/contracts/v2/openapi/resources.ts +++ b/apps/sim/lib/api/contracts/v2/openapi/resources.ts @@ -91,6 +91,8 @@ import { v2GetWorkspaceContract, v2ListWorkspaceMembersContract, v2ListWorkspacesContract, + v2RemoveWorkspaceMemberContract, + v2UpdateWorkspaceMemberContract, } from '@/lib/api/contracts/v2/workspaces' import { defineOpenApiDocument, @@ -105,6 +107,10 @@ import { sandboxOperations } from '@/lib/sandboxes/application/operations' import { secretOperations } from '@/lib/secrets/application/operations' import { skillOperations } from '@/lib/skills/application/operations' import { toolExecutionOperations } from '@/lib/tool-execution/application/operations' +import { + removeWorkspaceMemberOperation, + workspacePermissionOperations, +} from '@/lib/workspaces/application/member-operations' import { workspaceOperations } from '@/lib/workspaces/application/operations' const BLOCK_SUMMARY_EXAMPLE = { @@ -752,6 +758,67 @@ const declaredRoutes = [ ), } ), + defineOpenApiRoute( + v2UpdateWorkspaceMemberContract, + resourceOperation('Workspaces', { + applicationOperation: workspacePermissionOperations.update, + operationId: 'updateWorkspaceMember', + summary: 'Update workspace member', + description: `Change one existing member’s workspace role. Requires workspace Admin access. Only existing members can be changed; add people with Create workspace invitations. You cannot lower your own role, and organization Admins, the workspace owner, and the workspace billing account always keep Admin. Members whose organization membership an identity provider manages cannot be changed here. Requesting the role a member already holds changes nothing and records no audit entry. ${WORKSPACE_API_KEY_DENIED}`, + errors: RESOURCE_CONFLICT_ERRORS, + success: { description: 'The member with their updated workspace role.' }, + }), + { + params: documentedSchema( + v2UpdateWorkspaceMemberContract.params, + 'UpdateWorkspaceMemberParams', + 'Update workspace member parameters', + 'Workspace and member identifiers.' + ), + query: v2UpdateWorkspaceMemberContract.query, + body: documentedSchema( + v2UpdateWorkspaceMemberContract.body, + 'UpdateWorkspaceMemberBody', + 'Update workspace member body', + 'The new workspace role for this member.', + [{ role: 'write' }] + ), + response: documentedSchema( + v2UpdateWorkspaceMemberContract.response.schema, + 'UpdateWorkspaceMemberResponse', + 'Update workspace member response', + 'The member with their updated workspace role.', + [{ data: { ...WORKSPACE_MEMBER_EXAMPLE, role: 'write' } }] + ), + } + ), + defineOpenApiRoute( + v2RemoveWorkspaceMemberContract, + resourceOperation('Workspaces', { + applicationOperation: removeWorkspaceMemberOperation, + operationId: 'removeWorkspaceMember', + summary: 'Remove workspace member', + description: `Remove a member’s workspace access, or leave the workspace by naming yourself. Removing someone else requires workspace Admin access; any member may leave. The last Admin cannot leave, organization Admins hold access through their organization role and cannot be removed here, and the workspace billing account cannot be removed until billing is reassigned. Removing the workspace owner transfers ownership to the billing account. When this was the member’s last workspace in its organization, their organization membership and seat are removed too and all of their sessions end. ${WORKSPACE_API_KEY_DENIED}`, + errors: RESOURCE_CONFLICT_ERRORS, + success: { description: 'Acknowledgement that workspace access was removed.' }, + }), + { + params: documentedSchema( + v2RemoveWorkspaceMemberContract.params, + 'RemoveWorkspaceMemberParams', + 'Remove workspace member parameters', + 'Workspace and member identifiers.' + ), + query: v2RemoveWorkspaceMemberContract.query, + response: documentedSchema( + v2RemoveWorkspaceMemberContract.response.schema, + 'RemoveWorkspaceMemberResponse', + 'Remove workspace member response', + 'Acknowledgement that workspace access was removed.', + [{ data: { userId: 'user-123', deleted: true, organizationMembershipRemoved: false } }] + ), + } + ), defineOpenApiRoute( v2ListMcpServersContract, resourceOperation('MCP Servers', { diff --git a/apps/sim/lib/api/contracts/v2/workspaces.ts b/apps/sim/lib/api/contracts/v2/workspaces.ts index e11e5599ff8..9823ecaba67 100644 --- a/apps/sim/lib/api/contracts/v2/workspaces.ts +++ b/apps/sim/lib/api/contracts/v2/workspaces.ts @@ -1,5 +1,10 @@ import { z } from 'zod' -import { noInputSchema, workspaceIdSchema } from '@/lib/api/contracts/primitives' +import { + MAX_ID_LENGTH, + noInputSchema, + requiredFieldSchema, + workspaceIdSchema, +} from '@/lib/api/contracts/primitives' import { defineRouteContract } from '@/lib/api/contracts/types' import { v2CursorListResponse, @@ -115,3 +120,58 @@ export const v2ListWorkspaceMembersContract = defineRouteContract({ query: v2ListWorkspaceMembersQuerySchema, response: { mode: 'json', schema: v2CursorListResponse(v2WorkspaceMemberSchema) }, }) + +const v2WorkspaceMemberParamsSchema = v2WorkspaceParamsSchema + .extend({ + userId: requiredFieldSchema('User ID is required') + .max(MAX_ID_LENGTH, 'User ID is too long') + .describe('User identifier of the workspace member, as returned by List workspace members.'), + }) + .strict() + +const v2UpdateWorkspaceMemberBodySchema = z + .object({ + role: z + .enum(['admin', 'write', 'read']) + .describe( + 'New workspace role. Inherited organization Admin access, the workspace owner, and the workspace billing account keep Admin and cannot be changed here.' + ), + }) + .strict() + +export const v2UpdateWorkspaceMemberContract = defineRouteContract({ + method: 'PATCH', + path: '/api/v2/workspaces/[workspaceId]/members/[userId]', + params: v2WorkspaceMemberParamsSchema, + query: noInputSchema, + body: v2UpdateWorkspaceMemberBodySchema, + response: { mode: 'json', schema: v2DataResponse(v2WorkspaceMemberSchema) }, +}) + +export const v2RemoveWorkspaceMemberContract = defineRouteContract({ + method: 'DELETE', + path: '/api/v2/workspaces/[workspaceId]/members/[userId]', + params: v2WorkspaceMemberParamsSchema, + query: noInputSchema, + response: { + mode: 'json', + schema: v2DataResponse( + z + .object({ + userId: z.string().describe('User removed from the workspace.'), + deleted: z.literal(true).describe('Whether workspace access was removed.'), + organizationMembershipRemoved: z + .boolean() + .describe( + 'Whether this was the member’s last workspace in the organization, so their organization membership and seat were removed too.' + ), + }) + .strict() + .meta({ + id: 'V2WorkspaceMemberDeletion', + title: 'Workspace member removal', + description: 'Acknowledges removal of a workspace member.', + }) + ), + }, +}) diff --git a/apps/sim/lib/api/mcp/generated/v2-operations.ts b/apps/sim/lib/api/mcp/generated/v2-operations.ts index e21082c9268..561d0e7d591 100644 --- a/apps/sim/lib/api/mcp/generated/v2-operations.ts +++ b/apps/sim/lib/api/mcp/generated/v2-operations.ts @@ -357,6 +357,8 @@ import { v2GetWorkspaceContract, v2ListWorkspaceMembersContract, v2ListWorkspacesContract, + v2RemoveWorkspaceMemberContract, + v2UpdateWorkspaceMemberContract, } from '@/lib/api/contracts/v2/workspaces' import type { V2McpOperation } from '@/lib/api/mcp/types' @@ -2444,6 +2446,17 @@ export const V2_MCP_OPERATIONS = { '@/app/api/v2/organizations/[organizationId]/permission-groups/[groupId]/members/[userId]/route' ).then((route) => route.DELETE), }, + removeWorkspaceMember: { + contract: v2RemoveWorkspaceMemberContract, + summary: 'Remove workspace member', + description: + 'Remove a member’s workspace access, or leave the workspace by naming yourself. Removing someone else requires workspace Admin access; any member may leave. The last Admin cannot leave, organization Admins hold access through their organization role and cannot be removed here, and the workspace billing account cannot be removed until billing is reassigned. Removing the workspace owner transfers ownership to the billing account. When this was the member’s last workspace in its organization, their organization membership and seat are removed too and all of their sessions end. Workspace API keys return `403`; use a personal API key or scoped OAuth token.\n\nOAuth scope: `api:write`.', + workspaceKeyUnsupported: true, + handler: () => + import('@/app/api/v2/workspaces/[workspaceId]/members/[userId]/route').then( + (route) => route.DELETE + ), + }, renameFile: { contract: v2RenameFileContract, summary: 'Rename file', @@ -3008,6 +3021,17 @@ export const V2_MCP_OPERATIONS = { (route) => route.PUT ), }, + updateWorkspaceMember: { + contract: v2UpdateWorkspaceMemberContract, + summary: 'Update workspace member', + description: + 'Change one existing member’s workspace role. Requires workspace Admin access. Only existing members can be changed; add people with Create workspace invitations. You cannot lower your own role, and organization Admins, the workspace owner, and the workspace billing account always keep Admin. Members whose organization membership an identity provider manages cannot be changed here. Requesting the role a member already holds changes nothing and records no audit entry. Workspace API keys return `403`; use a personal API key or scoped OAuth token.\n\nOAuth scope: `api:write`.', + workspaceKeyUnsupported: true, + handler: () => + import('@/app/api/v2/workspaces/[workspaceId]/members/[userId]/route').then( + (route) => route.PATCH + ), + }, upsertCredentialMember: { contract: v2UpsertCredentialMemberContract, summary: 'Upsert credential member', diff --git a/apps/sim/lib/api/server/routes/v2-route-table.generated.ts b/apps/sim/lib/api/server/routes/v2-route-table.generated.ts index e4072fe5a65..8047e84737b 100644 --- a/apps/sim/lib/api/server/routes/v2-route-table.generated.ts +++ b/apps/sim/lib/api/server/routes/v2-route-table.generated.ts @@ -964,6 +964,10 @@ export const V2_ROUTES: readonly V2RouteEntry[] = [ pattern: '/api/v2/workspaces/{workspaceId}/members', load: () => import('@/app/api/v2/workspaces/[workspaceId]/members/route'), }, + { + pattern: '/api/v2/workspaces/{workspaceId}/members/{userId}', + load: () => import('@/app/api/v2/workspaces/[workspaceId]/members/[userId]/route'), + }, { pattern: '/api/v2/workspaces/{workspaceId}/operations', load: () => import('@/app/api/v2/workspaces/[workspaceId]/operations/route'), diff --git a/apps/sim/lib/api/server/workspace-presenters.ts b/apps/sim/lib/api/server/workspace-presenters.ts new file mode 100644 index 00000000000..91fc0ee5a17 --- /dev/null +++ b/apps/sim/lib/api/server/workspace-presenters.ts @@ -0,0 +1,15 @@ +import type { V2WorkspaceMember } from '@/lib/api/contracts/v2/workspaces' +import type { WorkspaceMemberRow } from '@/lib/workspaces/public-queries' + +/** One effective member as every v2 member response renders them. */ +export function presentWorkspaceMember(member: WorkspaceMemberRow): V2WorkspaceMember { + return { + userId: member.userId, + email: member.email, + name: member.name, + image: member.image, + role: member.role, + isExternal: member.isExternal, + joinedAt: member.joinedAt.toISOString(), + } +} diff --git a/apps/sim/lib/workspaces/api/route-policies.ts b/apps/sim/lib/workspaces/api/route-policies.ts index e1c4f6115af..65854edc379 100644 --- a/apps/sim/lib/workspaces/api/route-policies.ts +++ b/apps/sim/lib/workspaces/api/route-policies.ts @@ -3,6 +3,9 @@ import { createV2ResourceConcealmentPolicy, internalOrchestrationErrorPolicy, } from '@/lib/api/server/routes' +import { isRetryableTransactionError } from '@/lib/db/transaction' +import { WorkspaceBillingAccountRemovalError } from '@/lib/workspaces/utils' +import { v2CaughtOrchestrationError, v2Error } from '@/app/api/v2/lib/response' /** * A caller naming a workspace it cannot reach must not be able to tell that @@ -15,6 +18,23 @@ export const v2WorkspaceErrorPolicies = { concealWorkspaceAuthorization: createV2ResourceConcealmentPolicy({ notFoundMessage: 'Workspace not found', }), + /** + * Member administration adds two failures a caller can act on that the + * shared projection would answer as 500: an owner whose removal leaves no + * billing account to inherit the workspace (a state conflict, resolved by + * reassigning billing), and contention that outlived the transaction (resolved + * by waiting). + */ + manageWorkspaceMembers: createV2ResourceConcealmentPolicy({ + notFoundMessage: 'Workspace not found', + render(error) { + if (error instanceof WorkspaceBillingAccountRemovalError) + return v2Error('CONFLICT', error.message) + if (isRetryableTransactionError(error)) + return v2Error('SERVICE_UNAVAILABLE', 'The workspace is busy; retry in a moment') + return v2CaughtOrchestrationError(error) + }, + }), } as const export const internalWorkspaceErrorPolicies = { diff --git a/apps/sim/lib/workspaces/application/manage-permissions.ts b/apps/sim/lib/workspaces/application/manage-permissions.ts index 57507c2710e..fdc82569f9b 100644 --- a/apps/sim/lib/workspaces/application/manage-permissions.ts +++ b/apps/sim/lib/workspaces/application/manage-permissions.ts @@ -1,10 +1,11 @@ import { AuditAction, AuditResourceType } from '@sim/audit' import { type Principal, resolvePrincipalSubject } from '@sim/auth/principal' -import { defineAuthorizedWorkspaceUseCase } from '@/lib/core/application' -import { defineWorkspaceOperation } from '@/lib/core/application/workspace-operation' +import type { PermissionType } from '@sim/platform-authz/workspace' +import { defineAuthorizedWorkspaceUseCase, ForbiddenOperationError } from '@/lib/core/application' import { OrchestrationError } from '@/lib/core/orchestration/types' import { HttpError } from '@/lib/core/utils/http-error' import { resolveActiveWorkspaceApplicationContext } from '@/lib/workspaces/application/context' +import { workspacePermissionOperations } from '@/lib/workspaces/application/member-operations' import { type WorkspacePermissionUpdate, workspacePermissionUpdatesSchema, @@ -14,29 +15,7 @@ import { updateWorkspacePermissionRecords, } from '@/lib/workspaces/permissions/management-store' import { getWorkspacePermissionsForViewer } from '@/lib/workspaces/permissions/utils' - -export const workspacePermissionOperations = { - /** permission-group-exempt: the roster describes the workspace the caller belongs to. */ - read: defineWorkspaceOperation({ - id: 'workspaces.permissions.read', - minimumRole: 'read', - workspaceApiKey: 'deny', - principalKinds: ['session', 'delegated'], - delegatedServices: ['copilot'], - capability: 'none', - contentDelivery: 'none', - }), - /** permission-group-exempt: workspace administrators manage existing collaborator roles. */ - update: defineWorkspaceOperation({ - id: 'workspaces.permissions.update', - minimumRole: 'admin', - workspaceApiKey: 'deny', - principalKinds: ['session', 'delegated'], - delegatedServices: ['copilot'], - capability: 'none', - contentDelivery: 'none', - }), -} as const +import { getPublicWorkspaceMember } from '@/lib/workspaces/public-queries' export interface ReadWorkspacePermissionsInput { workspaceId: string @@ -75,56 +54,127 @@ export const readWorkspacePermissions = defineAuthorizedWorkspaceUseCase({ }, }) +/** + * The caller-safe failure a store error stands for, or the error itself when it + * names nothing a caller may see. + * + * This store's typed errors promise caller-safe wording; driver failures remain + * private. A coded refusal anywhere in the chain wins over the HTTP error that + * wraps it, so a directory-managed member keeps its `SCIM_MANAGED_MEMBERSHIP` + * code instead of collapsing into a bare forbidden. + */ +function callerSafeStoreFailure(error: unknown): unknown { + const chain: Error[] = [] + for ( + let cause: unknown = error; + cause instanceof Error && !chain.includes(cause); + cause = cause.cause + ) { + chain.push(cause) + } + const refusal = chain.find((cause) => cause instanceof ForbiddenOperationError) + if (refusal) return refusal + for (const cause of chain) { + if (!(cause instanceof HttpError)) continue + const code = + cause.statusCode === 400 + ? 'validation' + : cause.statusCode === 403 + ? 'forbidden' + : cause.statusCode === 404 + ? 'not_found' + : cause.statusCode === 409 + ? 'conflict' + : undefined + if (code) { + const failure = new OrchestrationError(code, cause.message) + failure.cause = cause + return failure + } + } + return error +} + +async function applyPermissionUpdates( + workspaceId: string, + principal: Principal, + updates: WorkspacePermissionUpdate[] +) { + const parsed = workspacePermissionUpdatesSchema.safeParse({ updates }) + if (!parsed.success) throw new OrchestrationError('validation', parsed.error.issues[0].message) + return updateWorkspacePermissionRecords( + workspaceId, + actorUserId(principal), + parsed.data.updates + ).catch((error: unknown) => { + throw callerSafeStoreFailure(error) + }) +} + +type PermissionChange = Awaited>[number] + +function projectPermissionChangeAudit(workspaceId: string, changes: PermissionChange[]) { + return changes.map((change) => ({ + action: AuditAction.MEMBER_ROLE_CHANGED, + resourceType: AuditResourceType.WORKSPACE, + resourceId: workspaceId, + resourceName: change.targetEmail ?? change.targetUserId, + description: `Changed permissions for ${change.targetEmail ?? change.targetUserId} from ${change.previousRole ?? 'unknown'} to ${change.newRole}`, + metadata: change, + })) +} + export const updateWorkspacePermissions = defineAuthorizedWorkspaceUseCase({ operation: workspacePermissionOperations.update, resolveContext: ({ input }: { input: UpdateWorkspacePermissionsInput }) => resolveContext({ input }), authorizationOptions, async execute({ principal, input, context }) { - const parsed = workspacePermissionUpdatesSchema.safeParse(input) - if (!parsed.success) throw new OrchestrationError('validation', parsed.error.issues[0].message) - const changes = await updateWorkspacePermissionRecords( - context.workspaceId, - actorUserId(principal), - parsed.data.updates - ).catch((error: unknown) => { - // This store's typed errors promise caller-safe wording; driver failures remain private. - let cause: unknown = error - const seen = new Set() - while (cause instanceof Error && !seen.has(cause)) { - seen.add(cause) - if (cause instanceof HttpError) { - const code = - cause.statusCode === 400 - ? 'validation' - : cause.statusCode === 403 - ? 'forbidden' - : cause.statusCode === 404 - ? 'not_found' - : cause.statusCode === 409 - ? 'conflict' - : undefined - if (code) { - const failure = new OrchestrationError(code, cause.message) - failure.cause = cause - throw failure - } - } - cause = cause.cause - } - throw error - }) + const changes = await applyPermissionUpdates(context.workspaceId, principal, input.updates) return { message: 'Permissions updated successfully', changes } }, projectAudit: ({ context, result }) => - result.changes.map((change) => ({ - action: AuditAction.MEMBER_ROLE_CHANGED, - resourceType: AuditResourceType.WORKSPACE, - resourceId: context.workspaceId, - resourceName: change.targetEmail ?? change.targetUserId, - description: `Changed permissions for ${change.targetEmail ?? change.targetUserId} from ${change.previousRole ?? 'unknown'} to ${change.newRole}`, - metadata: change, - })), + projectPermissionChangeAudit(context.workspaceId, result.changes), + afterSuccess: ({ principal, context }) => + reconcileWorkspacePermissionCredentials(context.workspaceId, actorUserId(principal)), +}) + +export interface UpdateWorkspaceMemberInput extends ReadWorkspacePermissionsInput { + userId: string + role: PermissionType +} + +/** + * One member's role change, answered with that member as the roster shows them + * afterwards. The same operation and store as {@link updateWorkspacePermissions}; + * a request for the role the member already holds changes and audits nothing. + * + * The member is read before the write: audit is projected only once `execute` + * returns, so a read failing after the commit would leave a durable role change + * with no audit entry. A successful write leaves exactly the requested role, + * because the store refuses inherited (organization admin) roles outright. + */ +export const updateWorkspaceMember = defineAuthorizedWorkspaceUseCase({ + operation: workspacePermissionOperations.update, + resolveContext: ({ input }: { input: UpdateWorkspaceMemberInput }) => resolveContext({ input }), + authorizationOptions, + async execute({ principal, input, context }) { + const before = await getPublicWorkspaceMember(context.workspaceId, input.userId) + const changes = await applyPermissionUpdates(context.workspaceId, principal, [ + { userId: input.userId, permissions: input.role }, + ]) + /** Granted between the read and the locked write; the store vetted them as a member. */ + const member = before ?? (await getPublicWorkspaceMember(context.workspaceId, input.userId)) + if (!member) { + throw new OrchestrationError( + 'conflict', + "This member's access just changed. Refresh and try again." + ) + } + return { changes, member: { ...member, role: input.role } } + }, + projectAudit: ({ context, result }) => + projectPermissionChangeAudit(context.workspaceId, result.changes), afterSuccess: ({ principal, context }) => reconcileWorkspacePermissionCredentials(context.workspaceId, actorUserId(principal)), }) diff --git a/apps/sim/lib/workspaces/application/member-operations.ts b/apps/sim/lib/workspaces/application/member-operations.ts new file mode 100644 index 00000000000..6c339c1ca02 --- /dev/null +++ b/apps/sim/lib/workspaces/application/member-operations.ts @@ -0,0 +1,42 @@ +import { defineWorkspaceOperation } from '@/lib/core/application/workspace-operation' + +/** + * Workspace membership administration, kept free of the stores so route + * declarations and the OpenAPI registry can name the operations without + * loading their implementations. + */ +export const workspacePermissionOperations = { + /** permission-group-exempt: the roster describes the workspace the caller belongs to. */ + read: defineWorkspaceOperation({ + id: 'workspaces.permissions.read', + minimumRole: 'read', + workspaceApiKey: 'deny', + principalKinds: ['session', 'delegated'], + delegatedServices: ['copilot'], + capability: 'none', + contentDelivery: 'none', + }), + /** permission-group-exempt: workspace administrators manage existing collaborator roles. */ + update: defineWorkspaceOperation({ + id: 'workspaces.permissions.update', + minimumRole: 'admin', + workspaceApiKey: 'deny', + oauthScope: 'api:write', + principalKinds: ['session', 'personal_api_key', 'oauth_access_token', 'delegated'], + delegatedServices: ['copilot'], + capability: 'none', + contentDelivery: 'none', + }), +} as const + +/** permission-group-exempt: members may leave; workspace administrators may remove existing collaborators. */ +export const removeWorkspaceMemberOperation = defineWorkspaceOperation({ + id: 'workspaces.members.remove', + minimumRole: 'read', + workspaceApiKey: 'deny', + capability: 'none', + contentDelivery: 'none', + oauthScope: 'api:write', + principalKinds: ['session', 'personal_api_key', 'oauth_access_token', 'delegated'], + delegatedServices: ['copilot'], +}) diff --git a/apps/sim/lib/workspaces/application/remove-member.ts b/apps/sim/lib/workspaces/application/remove-member.ts index 9fc09180883..8a94e698694 100644 --- a/apps/sim/lib/workspaces/application/remove-member.ts +++ b/apps/sim/lib/workspaces/application/remove-member.ts @@ -1,22 +1,11 @@ import { AuditAction, AuditResourceType } from '@sim/audit' import { requirePrincipalSubjectUserId } from '@sim/auth/principal' import { defineAuthorizedWorkspaceUseCase, requireCurrentHumanRole } from '@/lib/core/application' -import { defineWorkspaceOperation } from '@/lib/core/application/workspace-operation' import { OrchestrationError } from '@/lib/core/orchestration/types' import { resolveActiveWorkspaceApplicationContext } from '@/lib/workspaces/application/context' +import { removeWorkspaceMemberOperation } from '@/lib/workspaces/application/member-operations' import { removeWorkspaceMemberRecord } from '@/lib/workspaces/permissions/member-removal-store' -/** permission-group-exempt: members may leave; workspace administrators may remove existing collaborators. */ -export const removeWorkspaceMemberOperation = defineWorkspaceOperation({ - id: 'workspaces.members.remove', - minimumRole: 'read', - workspaceApiKey: 'deny', - capability: 'none', - contentDelivery: 'none', - principalKinds: ['session', 'delegated'], - delegatedServices: ['copilot'], -}) - export interface RemoveWorkspaceMemberInput { workspaceId: string userId: string diff --git a/apps/sim/lib/workspaces/permissions/management-store.ts b/apps/sim/lib/workspaces/permissions/management-store.ts index 3f996df28eb..f2abc5524e6 100644 --- a/apps/sim/lib/workspaces/permissions/management-store.ts +++ b/apps/sim/lib/workspaces/permissions/management-store.ts @@ -87,12 +87,16 @@ class WorkspaceBusyError extends HttpError { } } -/** A target whose membership the organization's directory owns; the guard's own wording names the remedy. */ +/** + * A target whose membership the organization's directory owns; the guard's own + * wording names the remedy. The coded refusal stays its `cause` so surfaces that + * publish `details.code` can still name it. + */ class DirectoryManagedMemberError extends HttpError { readonly statusCode = 403 constructor(cause: ForbiddenOperationError) { - super(cause.message) + super(cause.message, { cause }) this.name = 'DirectoryManagedMemberError' } } diff --git a/apps/sim/lib/workspaces/public-queries.ts b/apps/sim/lib/workspaces/public-queries.ts index 14c81417b6e..68dd082d10b 100644 --- a/apps/sim/lib/workspaces/public-queries.ts +++ b/apps/sim/lib/workspaces/public-queries.ts @@ -13,7 +13,7 @@ export interface PublicWorkspaceDetail { updatedAt: Date } -interface WorkspaceMemberRow { +export interface WorkspaceMemberRow { userId: string email: string name: string @@ -26,6 +26,8 @@ interface WorkspaceMemberRow { interface QueryWorkspaceMembersOptions { limit: number afterEmail?: string + /** Restricts both member sources to one user, for a single-member read. */ + userId?: string } export interface WorkspaceMemberPage { @@ -153,6 +155,7 @@ export async function queryPublicWorkspaceMembers( const emailOrder = sql`${user.email} COLLATE "C"` const emailCursor = options.afterEmail ? gt(emailOrder, options.afterEmail) : undefined + const userFilter = options.userId ? eq(user.id, options.userId) : undefined const sourceLimit = options.limit + 1 const hasRelevantOrganizationMembership = workspaceRow.organizationId ? sql`EXISTS ( @@ -183,7 +186,8 @@ export async function queryPublicWorkspaceMembers( and( eq(permissions.entityType, 'workspace'), eq(permissions.entityId, workspaceId), - emailCursor + emailCursor, + userFilter ) ) .orderBy(asc(emailOrder)) @@ -204,7 +208,8 @@ export async function queryPublicWorkspaceMembers( and( eq(member.organizationId, workspaceRow.organizationId), inArray(member.role, [...ORG_ADMIN_ROLES]), - emailCursor + emailCursor, + userFilter ) ) .orderBy(asc(emailOrder)) @@ -259,3 +264,16 @@ export async function queryPublicWorkspaceMembers( nextEmail: hasMore ? (members.at(-1)?.email ?? null) : null, } } + +/** + * One effective member, projected exactly as the roster projects them, or null + * when the user holds neither an explicit grant nor inherited organization + * admin access (or the workspace is archived). + */ +export async function getPublicWorkspaceMember( + workspaceId: string, + userId: string +): Promise { + const page = await queryPublicWorkspaceMembers(workspaceId, { limit: 1, userId }) + return page?.members[0] ?? null +} diff --git a/packages/sim-cli/src/contract/commands.ts b/packages/sim-cli/src/contract/commands.ts index 8118dd99ca6..9b1cd36404c 100644 --- a/packages/sim-cli/src/contract/commands.ts +++ b/packages/sim-cli/src/contract/commands.ts @@ -1518,10 +1518,13 @@ export const CLI_CONTRACT: CliContract = { ], }, listWorkspaceMembers: { - command: 'workspaces members', + command: 'workspaces members list', describe: 'List workspace members', profileWorkspacePath: true, + // `members update` and `members remove` take the user id, and the listing + // is where a caller finds it. columns: [ + { header: 'user', path: 'userId' }, { header: 'email' }, { header: 'name' }, { header: 'role' }, @@ -1529,6 +1532,16 @@ export const CLI_CONTRACT: CliContract = { { header: 'joined', path: 'joinedAt', format: 'timestamp' }, ], }, + updateWorkspaceMember: { + command: 'workspaces members update', + profileWorkspacePath: true, + }, + removeWorkspaceMember: { + command: 'workspaces members remove', + profileWorkspacePath: true, + confirm: + 'This removes the member’s workspace access. If it was their last workspace in the organization, their organization membership is removed and their sessions end.', + }, listAuditLogs: { allWorkspaces: true, diff --git a/packages/sim-cli/src/generated/v2-api.ts b/packages/sim-cli/src/generated/v2-api.ts index 992fa1a7246..24176d780d3 100644 --- a/packages/sim-cli/src/generated/v2-api.ts +++ b/packages/sim-cli/src/generated/v2-api.ts @@ -12677,6 +12677,24 @@ export type RemovePermissionGroupMemberResponse = { data: RemovePermissionGroupMemberResponseRef0 } +/** `DELETE /api/v2/workspaces/[workspaceId]/members/[userId]` */ +export type RemoveWorkspaceMemberParams = { + workspaceId: string + userId: string +} + +export type RemoveWorkspaceMemberQuery = Record + +type RemoveWorkspaceMemberResponseRef0 = { + userId: string + deleted: true + organizationMembershipRemoved: boolean +} + +export type RemoveWorkspaceMemberResponse = { + data: RemoveWorkspaceMemberResponseRef0 +} + /** `PATCH /api/v2/files/[fileId]` */ export type RenameFileParams = { fileId: string @@ -15854,6 +15872,32 @@ export type UpdateWorkspaceForkMappingsResponse = { data: UpdateWorkspaceForkMappingsResponseRef0 } +/** `PATCH /api/v2/workspaces/[workspaceId]/members/[userId]` */ +export type UpdateWorkspaceMemberParams = { + workspaceId: string + userId: string +} + +export type UpdateWorkspaceMemberQuery = Record + +export type UpdateWorkspaceMemberBody = { + role: 'admin' | 'write' | 'read' +} + +type UpdateWorkspaceMemberResponseRef0 = { + userId: string + email: string + name: string + image: string | null + role: 'admin' | 'write' | 'read' + isExternal: boolean + joinedAt: string +} + +export type UpdateWorkspaceMemberResponse = { + data: UpdateWorkspaceMemberResponseRef0 +} + /** `POST /api/v2/knowledge/[knowledgeBaseId]/documents` */ export type UploadKnowledgeDocumentParams = { knowledgeBaseId: string @@ -23102,6 +23146,18 @@ export const V2_OPERATIONS = { summary: 'Remove permission group member', workspaceKeyUnsupported: true, }, + removeWorkspaceMember: { + method: 'DELETE', + path: '/api/v2/workspaces/[workspaceId]/members/[userId]', + pathParams: ['workspaceId', 'userId'] as const, + pathParamDocs: { + workspaceId: 'Workspace to retrieve.', + userId: 'User identifier of the workspace member, as returned by List workspace members.', + }, + responseMode: 'json', + summary: 'Remove workspace member', + workspaceKeyUnsupported: true, + }, renameFile: { method: 'PATCH', path: '/api/v2/files/[fileId]', @@ -25053,6 +25109,27 @@ export const V2_OPERATIONS = { }, }, }, + updateWorkspaceMember: { + method: 'PATCH', + path: '/api/v2/workspaces/[workspaceId]/members/[userId]', + pathParams: ['workspaceId', 'userId'] as const, + pathParamDocs: { + workspaceId: 'Workspace to retrieve.', + userId: 'User identifier of the workspace member, as returned by List workspace members.', + }, + responseMode: 'json', + summary: 'Update workspace member', + workspaceKeyUnsupported: true, + body: { + role: { + kind: 'enum', + required: true, + values: ['admin', 'write', 'read'] as const, + describe: + 'New workspace role. Inherited organization Admin access, the workspace owner, and the workspace billing account keep Admin and cannot be changed here.', + }, + }, + }, uploadKnowledgeDocument: { method: 'POST', path: '/api/v2/knowledge/[knowledgeBaseId]/documents', diff --git a/packages/sim-cli/src/runtime/execute.ts b/packages/sim-cli/src/runtime/execute.ts index 8c765eedd11..47efd80c360 100644 --- a/packages/sim-cli/src/runtime/execute.ts +++ b/packages/sim-cli/src/runtime/execute.ts @@ -459,7 +459,7 @@ export async function executeOperation( const omitsWorkspace = commandSpec.allWorkspaces && requestFlags.allWorkspaces === true /** * A workspace carried in the path is resolved exactly like one carried in a - * field. `workspaces get` and `workspaces members` take theirs as a path + * field. `workspaces get` and `workspaces members list` take theirs as a path * parameter, so they skipped `requireWorkspace` and fell into `buildRequest`'s * own fallback: a second wording for the same precondition, and — because * `requireWorkspace` checks the key first — advice to set a workspace on an diff --git a/scripts/check-unused-exports.baseline.json b/scripts/check-unused-exports.baseline.json index dff09edea85..999b30ba486 100644 --- a/scripts/check-unused-exports.baseline.json +++ b/scripts/check-unused-exports.baseline.json @@ -3808,7 +3808,6 @@ "apps/sim/lib/api/contracts/v2/workspaces.ts#V2ListWorkspaceMembersQuery", "apps/sim/lib/api/contracts/v2/workspaces.ts#V2ListWorkspacesQuery", "apps/sim/lib/api/contracts/v2/workspaces.ts#V2Workspace", - "apps/sim/lib/api/contracts/v2/workspaces.ts#V2WorkspaceMember", "apps/sim/lib/api/contracts/v2/workspaces.ts#V2WorkspaceMemberCursor", "apps/sim/lib/api/contracts/v2/workspaces.ts#V2WorkspaceParams", "apps/sim/lib/api/contracts/v2/workspaces.ts#V2WorkspaceSortBy", From f95583c0abb1b1a0d909903609a1905172162058 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Tue, 10 Nov 2026 12:41:02 -0800 Subject: [PATCH 2/6] test(workspaces): prove v2 workspace member management end to end Add a real-HTTP E2E script for the new member update and removal endpoints, reporting to WORKSPACE_MEMBERS_E2E_REPORT_PATH: workspace-key and api:read-only OAuth refusals, role and tenant denials, non-member targets, self-demotion, protected owner/billing/org-admin roles, no-op writes without audit, last-admin leave, the 409 owner removal, self-leave with a personal key, input validation, and the key owner as audit actor. The use-case test now refuses a workspace key, which the operations still reject, rather than a personal key, which they now admit, and pins that a coded directory refusal survives the store's HTTP wrapper. --- .../application/manage-permissions.test.ts | 33 +- .../sim/scripts/test-workspace-members-e2e.ts | 522 ++++++++++++++++++ 2 files changed, 549 insertions(+), 6 deletions(-) create mode 100644 apps/sim/scripts/test-workspace-members-e2e.ts diff --git a/apps/sim/lib/workspaces/application/manage-permissions.test.ts b/apps/sim/lib/workspaces/application/manage-permissions.test.ts index 8af2a28ecbd..edabdbf03b5 100644 --- a/apps/sim/lib/workspaces/application/manage-permissions.test.ts +++ b/apps/sim/lib/workspaces/application/manage-permissions.test.ts @@ -1,8 +1,8 @@ import type { Principal } from '@sim/auth/principal' import { createDelegatedPrincipal, - createPersonalApiKeyPrincipal, createSessionPrincipal, + createWorkspaceApiKeyPrincipal, } from '@sim/testing/factories/principal.factory' import { auditMock, auditMockFns } from '@sim/testing/mocks/audit.mock' import { permissionsMock, permissionsMockFns } from '@sim/testing/mocks/permissions.mock' @@ -27,6 +27,8 @@ vi.mock('@/lib/workspaces/permissions/management-store', async (importOriginal) reconcileWorkspacePermissionCredentials: hoisted.reconcile, })) +import { ForbiddenOperationError, forbiddenErrorDetails } from '@/lib/core/application/forbidden' +import { HttpError } from '@/lib/core/utils/http-error' import { COPILOT_APPLICATION_SYSTEM_ERROR_MESSAGE, messageForCopilotApplicationError, @@ -118,14 +120,16 @@ describe('workspace permission operations', () => { expect(mocks.update).not.toHaveBeenCalled() }) - it('rejects unsupported principals before canonical loading', async () => { - await expect( - updateWorkspacePermissions.execute({ - principal: createPersonalApiKeyPrincipal({ userId: 'actor', keyId: 'key' }), + it('rejects a workspace key, even for its own workspace, before canonical loading', async () => { + const result = await updateWorkspacePermissions + .execute({ + principal: createWorkspaceApiKeyPrincipal({ workspaceId: 'workspace', keyId: 'key' }), input, }) - ).rejects.toThrow() + .catch((error: unknown) => error) + expect(forbiddenErrorDetails(result)).toEqual({ code: 'WORKSPACE_KEY_OPERATION_NOT_PERMITTED' }) expect(mocks.context).not.toHaveBeenCalled() + expect(mocks.update).not.toHaveBeenCalled() }) it('rejects duplicate direct application updates before the mutation', async () => { @@ -161,6 +165,23 @@ describe('workspace permission operations', () => { } ) + it('keeps a coded refusal that the store wraps in an HTTP error', async () => { + class DirectoryRefusal extends HttpError { + readonly statusCode = 403 + } + const refusal = new ForbiddenOperationError( + 'SCIM_MANAGED_MEMBERSHIP', + 'Your identity provider manages this membership' + ) + mocks.update.mockRejectedValue(new DirectoryRefusal(refusal.message, { cause: refusal })) + const result = await updateWorkspacePermissions + .execute({ principal, input }) + .catch((error: unknown) => error) + expect(forbiddenErrorDetails(result)).toEqual({ code: 'SCIM_MANAGED_MEMBERSHIP' }) + expect(result).toMatchObject({ code: 'forbidden', message: refusal.message }) + expect(auditMockFns.mockRecordAudit).not.toHaveBeenCalled() + }) + it.each([ new Error('private SQL and credentials'), Object.assign(new Error('forged HTTP error'), { statusCode: 403 }), diff --git a/apps/sim/scripts/test-workspace-members-e2e.ts b/apps/sim/scripts/test-workspace-members-e2e.ts new file mode 100644 index 00000000000..b304f267fa5 --- /dev/null +++ b/apps/sim/scripts/test-workspace-members-e2e.ts @@ -0,0 +1,522 @@ +import assert from 'node:assert/strict' +import { writeFile } from 'node:fs/promises' +import { assertDisposableTestDatabaseUrl } from '@sim/db/testing/test-infrastructure' +import { createLogger } from '@sim/logger' +import { sha256Hex } from '@sim/security/hash' +import { getErrorMessage } from '@sim/utils/errors' +import { sleep } from '@sim/utils/helpers' +import { generateId, generateShortId } from '@sim/utils/id' +import { isRecordLike } from '@sim/utils/object' +import { truncate } from '@sim/utils/string' +import postgres from 'postgres' + +/** + * Exercises `PATCH` and `DELETE /api/v2/workspaces/{workspaceId}/members/{userId}` + * over real HTTP against a running local app and a disposable PostgreSQL + * database. SQL seeds identities, credentials, and workspaces, and inspects + * persistence and audit rows; every member operation crosses the HTTP boundary. + * + * Required: WORKSPACE_MEMBERS_E2E_BASE_URL (loopback origin), + * WORKSPACE_MEMBERS_E2E_DATABASE_URL (loopback, database name containing + * "test"), WORKSPACE_MEMBERS_E2E_REPORT_PATH (JSON report destination). + */ +const logger = createLogger('WorkspaceMembersE2E') + +function required(name: string): string { + const value = process.env[name] + assert(value, `${name} must be explicitly provided`) + return value +} + +const baseUrl = new URL(required('WORKSPACE_MEMBERS_E2E_BASE_URL')) +assert(['localhost', '127.0.0.1', '[::1]'].includes(baseUrl.hostname), 'Use a loopback app host') +assert.equal(baseUrl.protocol, 'http:', 'The app must use local HTTP') +assert.equal(baseUrl.pathname, '/', 'App URL must be an origin without a path') +const databaseUrl = assertDisposableTestDatabaseUrl(required('WORKSPACE_MEMBERS_E2E_DATABASE_URL')) +const reportPath = required('WORKSPACE_MEMBERS_E2E_REPORT_PATH') + +const sql = postgres(databaseUrl.toString(), { max: 2 }) +const checks: { name: string; status: 'passed' | 'failed'; durationMs: number; error?: string }[] = + [] +const requests: { method: string; path: string; status: number }[] = [] +const startedAt = new Date().toISOString() + +const runId = generateId().replaceAll('-', '') +const domain = `workspace-members-e2e-${runId}.test` +const orgId = generateId() +const oauthClientId = generateId() + +/** The primary organization workspace and two personal workspaces for the edge states. */ +const workspaceId = generateId() +const lastAdminWorkspaceId = generateId() +const unresolvedOwnerWorkspaceId = generateId() +const workspaceIds = [workspaceId, lastAdminWorkspaceId, unresolvedOwnerWorkspaceId] + +const users = { + orgOwner: generateId(), + admin: generateId(), + wsOwner: generateId(), + billing: generateId(), + writer: generateId(), + reader: generateId(), + outsider: generateId(), +} as const +type UserName = keyof typeof users + +const personalKeys = { + admin: `sk-sim-e2e-${generateId()}`, + writer: `sk-sim-e2e-${generateId()}`, + reader: `sk-sim-e2e-${generateId()}`, + outsider: `sk-sim-e2e-${generateId()}`, +} +const workspaceKey = `sk-sim-e2e-ws-${generateId()}` +const oauthReadToken = `sim_oat_${generateShortId(32)}` +const oauthWriteToken = `sim_oat_${generateShortId(32)}` + +type Credential = { apiKey: string } | { bearer: string } + +interface V2ErrorBody { + code: string + message: string + details?: Record +} + +function record(value: unknown): Record { + assert(isRecordLike(value), 'Expected a JSON object') + return value +} + +function emailOf(name: UserName): string { + return `${name}@${domain}` +} + +function memberPath(targetUserId: string, workspace = workspaceId): string { + return `/api/v2/workspaces/${workspace}/members/${encodeURIComponent(targetUserId)}` +} + +async function call( + credential: Credential, + method: 'GET' | 'PATCH' | 'DELETE', + path: string, + expected: number, + body?: unknown +): Promise> { + const headers = new Headers({ 'User-Agent': 'Sim-local-workspace-members-E2E' }) + if ('apiKey' in credential) headers.set('X-API-Key', credential.apiKey) + else headers.set('Authorization', `Bearer ${credential.bearer}`) + if (body !== undefined) headers.set('Content-Type', 'application/json') + const response = await fetch(new URL(path, baseUrl), { + method, + headers, + body: body === undefined ? undefined : JSON.stringify(body), + redirect: 'error', + signal: AbortSignal.timeout(60_000), + }) + requests.push({ method, path, status: response.status }) + const text = await response.text() + assert.equal( + response.status, + expected, + `${method} ${path}: ${response.status} ${truncate(text, 500)}` + ) + assert.match(response.headers.get('cache-control') ?? '', /no-store/, 'v2 responses are no-store') + return record(JSON.parse(text)) +} + +/** Asserts the v2 failure envelope and returns its `error`. */ +async function fails( + credential: Credential, + method: 'PATCH' | 'DELETE', + path: string, + expected: number, + body?: unknown +): Promise { + const response = await call(credential, method, path, expected, body) + assert.deepEqual(Object.keys(response), ['error'], 'A failure carries only the error envelope') + const error = record(response.error) + assert.equal(typeof error.code, 'string') + assert.equal(typeof error.message, 'string') + return { + code: String(error.code), + message: String(error.message), + ...(error.details === undefined ? {} : { details: record(error.details) }), + } +} + +const key = (name: keyof typeof personalKeys): Credential => ({ apiKey: personalKeys[name] }) + +async function permissionOf(userId: string, workspace = workspaceId): Promise { + const [row] = await sql<{ permission_type: string }[]>`select permission_type from permissions + where entity_type = 'workspace' and entity_id = ${workspace} and user_id = ${userId}` + return row?.permission_type ?? null +} + +async function auditRows(action: string, workspace = workspaceId) { + return sql< + { actor_id: string | null; description: string | null; metadata: Record }[] + >`select actor_id, description, metadata from audit_log + where workspace_id = ${workspace} and action = ${action} order by created_at asc` +} + +/** Audit writes may land just after the response; poll briefly for the expected count. */ +async function awaitAuditCount(action: string, count: number, workspace = workspaceId) { + const deadline = performance.now() + 5_000 + for (;;) { + const rows = await auditRows(action, workspace) + if (rows.length >= count || performance.now() > deadline) return rows + await sleep(100) + } +} + +async function check(name: string, run: () => Promise) { + const started = performance.now() + try { + await run() + checks.push({ name, status: 'passed', durationMs: Math.round(performance.now() - started) }) + logger.info(`PASS ${name}`) + } catch (error) { + checks.push({ + name, + status: 'failed', + durationMs: Math.round(performance.now() - started), + error: getErrorMessage(error), + }) + throw error + } +} + +async function seed() { + await sql.begin(async (tx) => { + for (const [name, id] of Object.entries(users) as [UserName, string][]) { + await tx`insert into "user" (id, name, email, normalized_email, email_verified, created_at, updated_at) + values (${id}, ${`E2E ${name}`}, ${emailOf(name)}, ${emailOf(name)}, true, now(), now())` + await tx`insert into user_stats (id, user_id) values (${generateId()}, ${id})` + } + await tx`insert into organization (id, name, slug, created_at) + values (${orgId}, 'Workspace members E2E', ${`workspace-members-e2e-${runId}`}, now())` + await tx`insert into member (id, user_id, organization_id, role) values + (${generateId()}, ${users.orgOwner}, ${orgId}, 'owner'), + (${generateId()}, ${users.admin}, ${orgId}, 'member'), + (${generateId()}, ${users.wsOwner}, ${orgId}, 'member'), + (${generateId()}, ${users.billing}, ${orgId}, 'member'), + (${generateId()}, ${users.writer}, ${orgId}, 'member'), + (${generateId()}, ${users.reader}, ${orgId}, 'member')` + + await tx`insert into project (id, name, owner_id, organization_id) + values (${workspaceId}, 'Members E2E', ${users.wsOwner}, ${orgId})` + await tx`insert into workspace (id, project_id, name, owner_id, billed_account_user_id, organization_id) + values (${workspaceId}, ${workspaceId}, 'Members E2E', ${users.wsOwner}, ${users.billing}, ${orgId})` + await tx`insert into permissions (id, user_id, entity_type, entity_id, permission_type) values + (${generateId()}, ${users.admin}, 'workspace', ${workspaceId}, 'admin'), + (${generateId()}, ${users.wsOwner}, 'workspace', ${workspaceId}, 'admin'), + (${generateId()}, ${users.billing}, 'workspace', ${workspaceId}, 'admin'), + (${generateId()}, ${users.writer}, 'workspace', ${workspaceId}, 'write'), + (${generateId()}, ${users.reader}, 'workspace', ${workspaceId}, 'read')` + + /** Personal workspace whose only explicit admin is `admin`; its owner holds no grant. */ + await tx`insert into project (id, name, owner_id) + values (${lastAdminWorkspaceId}, 'Members E2E last admin', ${users.billing})` + await tx`insert into workspace (id, project_id, name, owner_id, billed_account_user_id) + values (${lastAdminWorkspaceId}, ${lastAdminWorkspaceId}, 'Members E2E last admin', ${users.billing}, ${users.billing})` + await tx`insert into permissions (id, user_id, entity_type, entity_id, permission_type) + values (${generateId()}, ${users.admin}, 'workspace', ${lastAdminWorkspaceId}, 'admin')` + + /** + * Personal workspace whose billing account holds no grant, so removing its + * owner leaves nobody to inherit it: the revocation refuses inside the + * transaction with the billing-account removal error. + */ + await tx`insert into project (id, name, owner_id) + values (${unresolvedOwnerWorkspaceId}, 'Members E2E unresolved owner', ${users.wsOwner})` + await tx`insert into workspace (id, project_id, name, owner_id, billed_account_user_id) + values (${unresolvedOwnerWorkspaceId}, ${unresolvedOwnerWorkspaceId}, 'Members E2E unresolved owner', ${users.wsOwner}, ${users.billing})` + await tx`insert into permissions (id, user_id, entity_type, entity_id, permission_type) values + (${generateId()}, ${users.wsOwner}, 'workspace', ${unresolvedOwnerWorkspaceId}, 'admin'), + (${generateId()}, ${users.admin}, 'workspace', ${unresolvedOwnerWorkspaceId}, 'admin')` + + for (const [name, value] of Object.entries(personalKeys) as [ + keyof typeof personalKeys, + string, + ][]) { + await tx`insert into api_key (id, user_id, name, key, key_hash, type) + values (${generateId()}, ${users[name]}, 'Members E2E', ${value}, ${sha256Hex(value)}, 'personal')` + } + await tx`insert into api_key (id, user_id, workspace_id, created_by, name, key, key_hash, type) + values (${generateId()}, ${users.admin}, ${workspaceId}, ${users.admin}, 'Members E2E workspace key', + ${workspaceKey}, ${sha256Hex(workspaceKey)}, 'workspace')` + + await tx`insert into oauth_client (id, client_id, name, public, token_endpoint_auth_method, grant_types, redirect_uris) + values (${oauthClientId}, ${oauthClientId}, 'Members E2E OAuth client', true, 'none', + ${['authorization_code', 'refresh_token']}, ${['http://127.0.0.1/callback']})` + for (const [token, scopes] of [ + [oauthReadToken, ['api:read']], + [oauthWriteToken, ['api:write']], + ] as const) { + await tx`insert into oauth_access_token (id, token, client_id, user_id, scopes, created_at, expires_at) + values (${generateId()}, ${sha256Hex(token.slice('sim_oat_'.length))}, ${oauthClientId}, + ${users.admin}, ${[...scopes]}, now(), now() + interval '1 hour')` + } + + /** A live session for `reader`, which leaving their last organization workspace must end. */ + await tx`insert into session (id, token, user_id, expires_at, created_at, updated_at) + values (${generateId()}, ${generateShortId()}, ${users.reader}, now() + interval '1 day', now(), now())` + }) +} + +async function cleanup() { + const userIds = Object.values(users) + await sql.begin(async (tx) => { + await tx`delete from audit_log where workspace_id in ${tx(workspaceIds)}` + await tx`delete from api_key where user_id in ${tx(userIds)}` + await tx`delete from oauth_client where client_id = ${oauthClientId}` + await tx`delete from permissions where entity_type = 'workspace' and entity_id in ${tx(workspaceIds)}` + await tx`delete from workspace where id in ${tx(workspaceIds)}` + await tx`delete from project where id in ${tx(workspaceIds)}` + await tx`delete from member where organization_id = ${orgId}` + await tx`delete from organization where id = ${orgId}` + await tx`delete from "user" where id in ${tx(userIds)}` + }) +} + +async function run() { + await seed() + const admin = key('admin') + + await check('A workspace API key is refused on both methods with a coded 403', async () => { + for (const [method, body] of [ + ['PATCH', { role: 'write' }], + ['DELETE', undefined], + ] as const) { + const error = await fails( + { apiKey: workspaceKey }, + method, + memberPath(users.reader), + 403, + body + ) + assert.equal(error.code, 'FORBIDDEN') + assert( + ['WORKSPACE_KEY_OPERATION_NOT_PERMITTED', 'PRINCIPAL_KIND_NOT_PERMITTED'].includes( + String(error.details?.code) + ), + `Unexpected workspace-key refusal code ${String(error.details?.code)}` + ) + } + assert.equal(await permissionOf(users.reader), 'read') + }) + + await check('An OAuth token without api:write is refused with INSUFFICIENT_SCOPE', async () => { + for (const [method, body] of [ + ['PATCH', { role: 'write' }], + ['DELETE', undefined], + ] as const) { + const error = await fails( + { bearer: oauthReadToken }, + method, + memberPath(users.reader), + 403, + body + ) + assert.equal(error.details?.code, 'INSUFFICIENT_SCOPE') + } + assert.equal(await permissionOf(users.reader), 'read') + }) + + await check('A non-admin cannot change or remove someone else', async () => { + const patch = await fails(key('writer'), 'PATCH', memberPath(users.reader), 403, { + role: 'write', + }) + assert.equal(patch.details?.code, 'INSUFFICIENT_WORKSPACE_ROLE') + const remove = await fails(key('writer'), 'DELETE', memberPath(users.reader), 403) + assert.equal(remove.details?.code, 'INSUFFICIENT_WORKSPACE_ROLE') + assert.equal(await permissionOf(users.reader), 'read') + }) + + await check('An unreachable or unknown workspace answers an identical 404', async () => { + const unreachable = await fails(key('outsider'), 'PATCH', memberPath(users.reader), 404, { + role: 'write', + }) + const unknown = await fails(admin, 'PATCH', memberPath(users.reader, generateId()), 404, { + role: 'write', + }) + assert.deepEqual(unreachable, unknown) + assert.deepEqual(unreachable, { code: 'NOT_FOUND', message: 'Workspace not found' }) + await fails(key('outsider'), 'DELETE', memberPath(users.reader), 404) + }) + + await check('A non-member target is a 400 to change and a 404 to remove', async () => { + await fails(admin, 'PATCH', memberPath(users.outsider), 400, { role: 'write' }) + await fails(admin, 'DELETE', memberPath(users.outsider), 404) + assert.equal(await permissionOf(users.outsider), null) + }) + + await check('Invalid bodies and query strings are rejected before any write', async () => { + await fails(admin, 'PATCH', memberPath(users.reader), 400, { role: 'owner' }) + await fails(admin, 'PATCH', memberPath(users.reader), 400, { role: 'write', extra: true }) + await fails(admin, 'PATCH', memberPath(users.reader), 400, {}) + await fails(admin, 'PATCH', `${memberPath(users.reader)}?force=1`, 400, { role: 'write' }) + await fails(admin, 'DELETE', `${memberPath(users.reader)}?force=1`, 400) + await fails(admin, 'PATCH', memberPath('x'.repeat(200)), 400, { role: 'write' }) + assert.equal(await permissionOf(users.reader), 'read') + }) + + await check('An admin cannot lower their own role', async () => { + const error = await fails(admin, 'PATCH', memberPath(users.admin), 400, { role: 'write' }) + assert.match(error.message, /own admin/) + assert.equal(await permissionOf(users.admin), 'admin') + }) + + await check('Owner, billing account, and organization admins keep Admin', async () => { + await fails(admin, 'PATCH', memberPath(users.wsOwner), 400, { role: 'read' }) + await fails(admin, 'PATCH', memberPath(users.billing), 400, { role: 'read' }) + await fails(admin, 'PATCH', memberPath(users.orgOwner), 400, { role: 'write' }) + await fails(admin, 'DELETE', memberPath(users.orgOwner), 400) + await fails(admin, 'DELETE', memberPath(users.billing), 400) + assert.equal(await permissionOf(users.wsOwner), 'admin') + assert.equal(await permissionOf(users.billing), 'admin') + }) + + await check( + 'A role change returns the roster member and audits the key owner as actor', + async () => { + const before = (await auditRows('member.role_changed')).length + const response = await call(admin, 'PATCH', memberPath(users.reader), 200, { + role: 'write', + }) + assert.deepEqual(Object.keys(response), ['data']) + const roster = await call(admin, 'GET', `/api/v2/workspaces/${workspaceId}/members`, 200) + assert(Array.isArray(roster.data)) + const listed = roster.data.map(record).find((entry) => entry.userId === users.reader) + assert.deepEqual(response.data, listed, 'PATCH returns the member exactly as the list does') + assert.equal(record(response.data).role, 'write') + assert.equal(await permissionOf(users.reader), 'write') + + const rows = await awaitAuditCount('member.role_changed', before + 1) + assert.equal(rows.length, before + 1) + const entry = rows[rows.length - 1] + assert.equal(entry.actor_id, users.admin) + assert.equal(record(entry.metadata).targetUserId, users.reader) + assert.equal(record(entry.metadata).previousRole, 'read') + assert.equal(record(record(entry.metadata).actor).kind, 'personal_api_key') + } + ) + + await check('An OAuth token with api:write can change a role', async () => { + const response = await call( + { bearer: oauthWriteToken }, + 'PATCH', + memberPath(users.reader), + 200, + { + role: 'read', + } + ) + assert.equal(record(response.data).role, 'read') + assert.equal(await permissionOf(users.reader), 'read') + const rows = await awaitAuditCount('member.role_changed', 2) + assert.equal(record(record(rows[rows.length - 1].metadata).actor).kind, 'oauth_access_token') + assert.equal(rows[rows.length - 1].actor_id, users.admin) + }) + + await check('Requesting the current role writes nothing and records no audit', async () => { + const before = await auditRows('member.role_changed') + const [{ updated_at: updatedBefore }] = await sql<{ updated_at: Date }[]>` + select updated_at from permissions + where entity_type = 'workspace' and entity_id = ${workspaceId} and user_id = ${users.reader}` + const response = await call(admin, 'PATCH', memberPath(users.reader), 200, { role: 'read' }) + assert.equal(record(response.data).role, 'read') + await sleep(1_000) + assert.equal((await auditRows('member.role_changed')).length, before.length) + const [{ updated_at: updatedAfter }] = await sql<{ updated_at: Date }[]>` + select updated_at from permissions + where entity_type = 'workspace' and entity_id = ${workspaceId} and user_id = ${users.reader}` + assert.equal(updatedAfter.getTime(), updatedBefore.getTime()) + }) + + await check('The last explicit admin cannot leave', async () => { + const error = await fails(admin, 'DELETE', memberPath(users.admin, lastAdminWorkspaceId), 400) + assert.match(error.message, /last admin/) + assert.equal(await permissionOf(users.admin, lastAdminWorkspaceId), 'admin') + }) + + await check('An owner removal with no successor is a 409, not a 500', async () => { + const error = await fails( + admin, + 'DELETE', + memberPath(users.wsOwner, unresolvedOwnerWorkspaceId), + 409 + ) + assert.equal(error.code, 'CONFLICT') + assert.match(error.message, /billing account/) + assert.equal(await permissionOf(users.wsOwner, unresolvedOwnerWorkspaceId), 'admin') + const [owner] = await sql<{ owner_id: string }[]>`select owner_id from workspace + where id = ${unresolvedOwnerWorkspaceId}` + assert.equal(owner.owner_id, users.wsOwner) + }) + + await check('An admin removes another member and the removal is audited', async () => { + const before = (await auditRows('member.removed')).length + const response = await call(admin, 'DELETE', memberPath(users.writer), 200) + const data = record(response.data) + assert.equal(data.userId, users.writer) + assert.equal(data.deleted, true) + assert.equal(data.organizationMembershipRemoved, true, 'This was their only org workspace') + assert.equal(await permissionOf(users.writer), null) + const membership = await sql`select id from member + where organization_id = ${orgId} and user_id = ${users.writer}` + assert.equal(membership.length, 0) + const rows = await awaitAuditCount('member.removed', before + 1) + assert.equal(rows[rows.length - 1].actor_id, users.admin) + assert.equal(record(rows[rows.length - 1].metadata).removedUserId, users.writer) + }) + + await check( + 'A member leaves with a personal key, losing org membership and sessions', + async () => { + const response = await call(key('reader'), 'DELETE', memberPath(users.reader), 200) + const data = record(response.data) + assert.equal(data.userId, users.reader) + assert.equal(data.organizationMembershipRemoved, true) + assert.equal(await permissionOf(users.reader), null) + const membership = await sql`select id from member + where organization_id = ${orgId} and user_id = ${users.reader}` + assert.equal(membership.length, 0) + const sessions = await sql`select id from session where user_id = ${users.reader}` + assert.equal(sessions.length, 0, 'Leaving the last org workspace ends every session') + const rows = await awaitAuditCount('member.removed', 2) + const entry = rows[rows.length - 1] + assert.equal(entry.actor_id, users.reader) + assert.equal(record(entry.metadata).selfRemoval, true) + await fails(key('reader'), 'DELETE', memberPath(users.reader), 404) + } + ) +} + +try { + await run() +} catch (error) { + logger.error('Workspace members E2E failed', { error: getErrorMessage(error) }) + process.exitCode = 1 +} finally { + try { + await cleanup() + logger.info('Removed workspace members E2E fixtures') + } catch (error) { + logger.error('Workspace members E2E fixture cleanup failed', { error: getErrorMessage(error) }) + process.exitCode = 1 + } + await sql.end() + const report = { + startedAt, + completedAt: new Date().toISOString(), + status: process.exitCode ? 'failed' : 'passed', + transport: 'Real HTTP against a local Next.js app and PostgreSQL', + requestCount: requests.length, + checks, + requests, + } + await writeFile(reportPath, `${JSON.stringify(report, null, 2)}\n`) + logger.info( + `Workspace members E2E ${report.status}: ${checks.filter((item) => item.status === 'passed').length}/${checks.length} checks, ${requests.length} HTTP requests` + ) +} From 34a5ca6ea5fbc4c46fcd3463b07a3a2c61f4dfe7 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Tue, 10 Nov 2026 12:51:10 -0800 Subject: [PATCH 3/6] test(workspaces): prove v2 member management in-process against PostgreSQL Replace the running-app E2E script with an integration suite that drives the real v2 member route handlers with hashed personal keys, a workspace key, and OAuth tokens against PostgreSQL, including a SCIM-managed member. Simplify the store-failure mapping with findCause and a status table, and keep the store's HTTP error as the cause of a coded refusal so the internal permissions route renders it exactly as before. Spread params and body in the PATCH route like the organization member route, trim comments, and list the renamed member commands in the Teammates settings section. --- .../[workspaceId]/members/[userId]/route.ts | 6 +- apps/sim/lib/settings/sections.ts | 7 +- .../__integration__/members.integration.ts | 527 ++++++++++++++++++ apps/sim/lib/workspaces/api/route-policies.ts | 8 +- .../application/manage-permissions.test.ts | 2 + .../application/manage-permissions.ts | 80 ++- .../sim/scripts/test-workspace-members-e2e.ts | 522 ----------------- 7 files changed, 574 insertions(+), 578 deletions(-) create mode 100644 apps/sim/lib/workspaces/__integration__/members.integration.ts delete mode 100644 apps/sim/scripts/test-workspace-members-e2e.ts diff --git a/apps/sim/app/api/v2/workspaces/[workspaceId]/members/[userId]/route.ts b/apps/sim/app/api/v2/workspaces/[workspaceId]/members/[userId]/route.ts index df4ca574f2b..00e4339a2d2 100644 --- a/apps/sim/app/api/v2/workspaces/[workspaceId]/members/[userId]/route.ts +++ b/apps/sim/app/api/v2/workspaces/[workspaceId]/members/[userId]/route.ts @@ -19,11 +19,7 @@ export const PATCH = defineV2JsonRoute({ auth: v2ApiKeyAuth, rateLimit: v2RateLimits.publicApi, errorPolicy: v2WorkspaceErrorPolicies.manageWorkspaceMembers, - mapInput: ({ params, body }) => ({ - workspaceId: params.workspaceId, - userId: params.userId, - role: body.role, - }), + mapInput: ({ params, body }) => ({ ...params, ...body }), useCase: updateWorkspaceMember, present: ({ member }) => ({ data: presentWorkspaceMember(member) }), }) diff --git a/apps/sim/lib/settings/sections.ts b/apps/sim/lib/settings/sections.ts index 1e524c8a04c..f287e6c199c 100644 --- a/apps/sim/lib/settings/sections.ts +++ b/apps/sim/lib/settings/sections.ts @@ -112,7 +112,12 @@ export const settingsSections: Record ({ ...envFlagsMock, isScimEnabled: true })) + +const orgId = generateId() +const scimConnectionId = generateId() +const workspaceId = generateId() +/** Personal workspace whose only explicit admin is `admin`; its owner holds no grant. */ +const lastAdminWorkspaceId = generateId() +/** Personal workspace whose billing account holds no grant, so its owner has no successor. */ +const unresolvedOwnerWorkspaceId = generateId() +const workspaceIds = [workspaceId, lastAdminWorkspaceId, unresolvedOwnerWorkspaceId] + +const users = { + orgOwner: generateId(), + admin: generateId(), + wsOwner: generateId(), + billing: generateId(), + writer: generateId(), + reader: generateId(), + managed: generateId(), + outsider: generateId(), +} +const userIds = Object.values(users) +const personalKeys = { + admin: `sk-sim-fixture-${generateId()}`, + writer: `sk-sim-fixture-${generateId()}`, + reader: `sk-sim-fixture-${generateId()}`, + outsider: `sk-sim-fixture-${generateId()}`, +} +const workspaceKey = `sk-sim-fixture-${generateId()}` +const oauthClientId = generateId() +const oauthReadToken = `sim_oat_${generateShortId(32)}` +const oauthWriteToken = `sim_oat_${generateShortId(32)}` + +type Credential = { apiKey: string } | { bearer: string } +const key = (name: keyof typeof personalKeys): Credential => ({ apiKey: personalKeys[name] }) +const admin = key('admin') + +async function call( + credential: Credential, + method: 'PATCH' | 'DELETE', + target: string, + options: { workspace?: string; body?: unknown; query?: string } = {} +) { + const scope = options.workspace ?? workspaceId + const headers = new Headers({ 'x-forwarded-for': '127.0.0.1' }) + if ('apiKey' in credential) headers.set('X-API-Key', credential.apiKey) + else headers.set('Authorization', `Bearer ${credential.bearer}`) + if (options.body !== undefined) headers.set('Content-Type', 'application/json') + const request = new NextRequest( + `http://localhost/api/v2/workspaces/${scope}/members/${target}${options.query ?? ''}`, + { + method, + headers, + ...(options.body === undefined ? {} : { body: JSON.stringify(options.body) }), + } + ) + const context = { params: Promise.resolve({ workspaceId: scope, userId: target }) } + const response = + method === 'PATCH' ? await updateMember(request, context) : await removeMember(request, context) + expect(response.headers.get('cache-control')).toMatch(/no-store/) + return { status: response.status, body: await response.json() } +} + +async function roleOf(target: string, scope = workspaceId) { + const [row] = await db + .select({ role: permissions.permissionType }) + .from(permissions) + .where( + and( + eq(permissions.entityType, 'workspace'), + eq(permissions.entityId, scope), + eq(permissions.userId, target) + ) + ) + return row?.role ?? null +} + +function auditRows(action: string, scope = workspaceId) { + return db + .select() + .from(auditLog) + .where(and(eq(auditLog.workspaceId, scope), eq(auditLog.action, action))) + .orderBy(asc(auditLog.createdAt)) +} + +/** Audit is written after the response; wait for the expected row count. */ +async function awaitAudit(action: string, count: number, scope = workspaceId) { + return vi.waitFor( + async () => { + const rows = await auditRows(action, scope) + expect(rows).toHaveLength(count) + return rows + }, + { timeout: 5_000, interval: 100 } + ) +} + +async function orgMembership(target: string) { + return db + .select({ id: member.id }) + .from(member) + .where(and(eq(member.organizationId, orgId), eq(member.userId, target))) +} + +/** Drives the real v2 member route handlers with hashed credentials against PostgreSQL. */ +describe('v2 workspace member management against PostgreSQL', () => { + beforeAll(async () => { + const now = new Date() + await db.insert(user).values( + Object.entries(users).map(([name, id]) => ({ + id, + name: `Fixture ${name}`, + email: `${id}@members.test`, + normalizedEmail: `${id}@members.test`, + emailVerified: true, + createdAt: now, + updatedAt: now, + })) + ) + await db.insert(userStats).values(userIds.map((id) => ({ id: generateId(), userId: id }))) + await db + .insert(organization) + .values({ id: orgId, name: 'Members fixture', slug: `members-${orgId}`, createdAt: now }) + await db.insert(member).values( + (['orgOwner', 'admin', 'wsOwner', 'billing', 'writer', 'reader', 'managed'] as const).map( + (name) => ({ + id: generateId(), + userId: users[name], + organizationId: orgId, + role: name === 'orgOwner' ? 'owner' : 'member', + }) + ) + ) + await insertWorkspaceFixture(db, [ + { + id: workspaceId, + name: 'Members fixture', + ownerId: users.wsOwner, + billedAccountUserId: users.billing, + organizationId: orgId, + allowPersonalApiKeys: true, + }, + { + id: lastAdminWorkspaceId, + name: 'Last admin fixture', + ownerId: users.billing, + billedAccountUserId: users.billing, + allowPersonalApiKeys: true, + }, + { + id: unresolvedOwnerWorkspaceId, + name: 'Unresolved owner fixture', + ownerId: users.wsOwner, + billedAccountUserId: users.billing, + allowPersonalApiKeys: true, + }, + ]) + const grant = (target: string, scope: string, role: 'admin' | 'write' | 'read') => ({ + id: generateId(), + userId: target, + entityType: 'workspace', + entityId: scope, + permissionType: role, + }) + await db + .insert(permissions) + .values([ + grant(users.admin, workspaceId, 'admin'), + grant(users.wsOwner, workspaceId, 'admin'), + grant(users.billing, workspaceId, 'admin'), + grant(users.writer, workspaceId, 'write'), + grant(users.reader, workspaceId, 'read'), + grant(users.managed, workspaceId, 'write'), + grant(users.admin, lastAdminWorkspaceId, 'admin'), + grant(users.wsOwner, unresolvedOwnerWorkspaceId, 'admin'), + grant(users.admin, unresolvedOwnerWorkspaceId, 'admin'), + ]) + await db.insert(apiKey).values([ + ...(Object.entries(personalKeys) as [keyof typeof personalKeys, string][]).map( + ([name, value]) => ({ + id: generateId(), + userId: users[name], + name: 'Members fixture', + key: value, + keyHash: hashApiKey(value), + type: 'personal', + }) + ), + { + id: generateId(), + userId: users.admin, + workspaceId, + createdBy: users.admin, + name: 'Members workspace fixture', + key: workspaceKey, + keyHash: hashApiKey(workspaceKey), + type: 'workspace', + }, + ]) + await db.insert(oauthClient).values({ + id: oauthClientId, + clientId: oauthClientId, + name: 'Members fixture client', + public: true, + tokenEndpointAuthMethod: 'none', + grantTypes: ['authorization_code', 'refresh_token'], + redirectUris: ['http://127.0.0.1/callback'], + }) + await db.insert(oauthConsent).values({ + id: generateId(), + clientId: oauthClientId, + userId: users.admin, + scopes: ['api:read', 'api:write'], + createdAt: now, + updatedAt: now, + }) + await db.insert(oauthAccessToken).values( + ( + [ + [oauthReadToken, ['api:read']], + [oauthWriteToken, ['api:write']], + ] as const + ).map(([token, scopes]) => ({ + id: generateId(), + token: hashOAuthToken(token.slice('sim_oat_'.length)), + clientId: oauthClientId, + userId: users.admin, + scopes: [...scopes], + createdAt: now, + expiresAt: new Date(now.getTime() + 60 * 60 * 1000), + })) + ) + await db.insert(scimConnection).values({ + id: scimConnectionId, + organizationId: orgId, + status: 'active', + settings: { lockManualMembership: true }, + }) + const managedEmail = `${users.managed}@members.test` + await db.insert(scimUser).values({ + id: generateId(), + connectionId: scimConnectionId, + userId: users.managed, + userName: managedEmail, + orderKey: users.managed, + attributes: { + userName: managedEmail, + active: true, + displayName: 'Managed user', + name: { formatted: 'Managed user' }, + emails: [{ value: managedEmail, primary: true, type: 'work' }], + }, + }) + await db.insert(session).values({ + id: generateId(), + token: generateShortId(32), + userId: users.reader, + expiresAt: new Date(now.getTime() + 24 * 60 * 60 * 1000), + createdAt: now, + updatedAt: now, + }) + }) + + afterAll(async () => { + await db.delete(auditLog).where(inArray(auditLog.workspaceId, workspaceIds)) + await db.delete(apiKey).where(inArray(apiKey.userId, userIds)) + await db.delete(oauthClient).where(eq(oauthClient.clientId, oauthClientId)) + await db + .delete(permissions) + .where( + and(eq(permissions.entityType, 'workspace'), inArray(permissions.entityId, workspaceIds)) + ) + await deleteWorkspaceFixture(db, inArray(workspace.id, workspaceIds)) + await db.delete(scimConnection).where(eq(scimConnection.id, scimConnectionId)) + await db.delete(member).where(eq(member.organizationId, orgId)) + await db.delete(organization).where(eq(organization.id, orgId)) + await db.delete(session).where(inArray(session.userId, userIds)) + await db.delete(userStats).where(inArray(userStats.userId, userIds)) + await db.delete(user).where(inArray(user.id, userIds)) + await db.$client.end() + }) + + it('refuses a workspace API key on both methods with a coded 403', async () => { + for (const [method, body] of [ + ['PATCH', { role: 'write' }], + ['DELETE', undefined], + ] as const) { + const response = await call({ apiKey: workspaceKey }, method, users.reader, { body }) + expect(response).toMatchObject({ + status: 403, + body: { + error: { code: 'FORBIDDEN', details: { code: 'WORKSPACE_KEY_OPERATION_NOT_PERMITTED' } }, + }, + }) + } + expect(await roleOf(users.reader)).toBe('read') + }) + + it('refuses an OAuth token without api:write', async () => { + for (const [method, body] of [ + ['PATCH', { role: 'write' }], + ['DELETE', undefined], + ] as const) { + const response = await call({ bearer: oauthReadToken }, method, users.reader, { body }) + expect(response).toMatchObject({ + status: 403, + body: { error: { details: { code: 'INSUFFICIENT_SCOPE' } } }, + }) + } + expect(await roleOf(users.reader)).toBe('read') + }) + + it('refuses a non-admin changing or removing someone else', async () => { + for (const [method, body] of [ + ['PATCH', { role: 'write' }], + ['DELETE', undefined], + ] as const) { + const response = await call(key('writer'), method, users.reader, { body }) + expect(response).toMatchObject({ + status: 403, + body: { error: { details: { code: 'INSUFFICIENT_WORKSPACE_ROLE' } } }, + }) + } + expect(await roleOf(users.reader)).toBe('read') + }) + + it('answers an unreachable and an unknown workspace with the same 404', async () => { + const unreachable = await call(key('outsider'), 'PATCH', users.reader, { + body: { role: 'write' }, + }) + const unknown = await call(admin, 'PATCH', users.reader, { + workspace: generateId(), + body: { role: 'write' }, + }) + expect(unreachable).toEqual(unknown) + expect(unreachable).toEqual({ + status: 404, + body: { error: { code: 'NOT_FOUND', message: 'Workspace not found' } }, + }) + expect((await call(key('outsider'), 'DELETE', users.reader)).status).toBe(404) + }) + + it('treats a non-member target as a 400 to change and a 404 to remove', async () => { + expect((await call(admin, 'PATCH', users.outsider, { body: { role: 'write' } })).status).toBe( + 400 + ) + expect((await call(admin, 'DELETE', users.outsider)).status).toBe(404) + expect(await roleOf(users.outsider)).toBeNull() + }) + + it('rejects invalid bodies, query strings, and ids before any write', async () => { + const invalid = [ + call(admin, 'PATCH', users.reader, { body: { role: 'owner' } }), + call(admin, 'PATCH', users.reader, { body: { role: 'write', extra: true } }), + call(admin, 'PATCH', users.reader, { body: {} }), + call(admin, 'PATCH', users.reader, { body: { role: 'write' }, query: '?force=1' }), + call(admin, 'DELETE', users.reader, { query: '?force=1' }), + call(admin, 'PATCH', 'x'.repeat(200), { body: { role: 'write' } }), + ] + for (const response of await Promise.all(invalid)) { + expect(response).toMatchObject({ status: 400, body: { error: { code: 'BAD_REQUEST' } } }) + } + expect(await roleOf(users.reader)).toBe('read') + }) + + it('keeps the SCIM_MANAGED_MEMBERSHIP code for a directory-managed member', async () => { + const response = await call(admin, 'PATCH', users.managed, { body: { role: 'read' } }) + expect(response).toMatchObject({ + status: 403, + body: { error: { code: 'FORBIDDEN', details: { code: 'SCIM_MANAGED_MEMBERSHIP' } } }, + }) + expect(await roleOf(users.managed)).toBe('write') + }) + + it('refuses an admin lowering their own role', async () => { + const response = await call(admin, 'PATCH', users.admin, { body: { role: 'write' } }) + expect(response.status).toBe(400) + expect(response.body.error.message).toMatch(/own admin/) + expect(await roleOf(users.admin)).toBe('admin') + }) + + it('keeps Admin for the owner, the billing account, and organization admins', async () => { + const refusals = await Promise.all([ + call(admin, 'PATCH', users.wsOwner, { body: { role: 'read' } }), + call(admin, 'PATCH', users.billing, { body: { role: 'read' } }), + call(admin, 'PATCH', users.orgOwner, { body: { role: 'write' } }), + call(admin, 'DELETE', users.orgOwner), + call(admin, 'DELETE', users.billing), + ]) + expect(refusals.map((response) => response.status)).toEqual([400, 400, 400, 400, 400]) + expect(await roleOf(users.wsOwner)).toBe('admin') + expect(await roleOf(users.billing)).toBe('admin') + }) + + it('returns the roster member and audits the key owner as actor', async () => { + const response = await call(admin, 'PATCH', users.reader, { body: { role: 'write' } }) + expect(response.status).toBe(200) + expect(Object.keys(response.body)).toEqual(['data']) + const roster = await listMembers( + new NextRequest(`http://localhost/api/v2/workspaces/${workspaceId}/members`, { + headers: { 'X-API-Key': personalKeys.admin, 'x-forwarded-for': '127.0.0.1' }, + }), + { params: Promise.resolve({ workspaceId }) } + ) + const listed = (await roster.json()).data.find( + (entry: { userId: string }) => entry.userId === users.reader + ) + expect(response.body.data).toEqual(listed) + expect(listed.role).toBe('write') + expect(await roleOf(users.reader)).toBe('write') + + const [entry] = await awaitAudit('member.role_changed', 1) + expect(entry.actorId).toBe(users.admin) + expect(entry.metadata).toMatchObject({ + targetUserId: users.reader, + previousRole: 'read', + actor: { kind: 'personal_api_key' }, + }) + }) + + it('lets an OAuth token with api:write change a role', async () => { + const response = await call({ bearer: oauthWriteToken }, 'PATCH', users.reader, { + body: { role: 'read' }, + }) + expect(response).toMatchObject({ status: 200, body: { data: { role: 'read' } } }) + expect(await roleOf(users.reader)).toBe('read') + const rows = await awaitAudit('member.role_changed', 2) + expect(rows[1].actorId).toBe(users.admin) + expect(rows[1].metadata).toMatchObject({ actor: { kind: 'oauth_access_token' } }) + }) + + it('writes and audits nothing when the requested role is already held', async () => { + const grantRow = () => + db + .select({ updatedAt: permissions.updatedAt }) + .from(permissions) + .where(and(eq(permissions.entityId, workspaceId), eq(permissions.userId, users.reader))) + const [before] = await grantRow() + const response = await call(admin, 'PATCH', users.reader, { body: { role: 'read' } }) + expect(response).toMatchObject({ status: 200, body: { data: { role: 'read' } } }) + await new Promise((settle) => setTimeout(settle, 500)) + expect(await auditRows('member.role_changed')).toHaveLength(2) + expect(await grantRow()).toEqual([before]) + }) + + it('refuses the last explicit admin leaving', async () => { + const response = await call(admin, 'DELETE', users.admin, { workspace: lastAdminWorkspaceId }) + expect(response.status).toBe(400) + expect(response.body.error.message).toMatch(/last admin/) + expect(await roleOf(users.admin, lastAdminWorkspaceId)).toBe('admin') + }) + + it('answers an owner removal with no successor as 409, not 500', async () => { + const response = await call(admin, 'DELETE', users.wsOwner, { + workspace: unresolvedOwnerWorkspaceId, + }) + expect(response.status).toBe(409) + expect(response.body.error.code).toBe('CONFLICT') + expect(response.body.error.message).toMatch(/billing account/) + expect(await roleOf(users.wsOwner, unresolvedOwnerWorkspaceId)).toBe('admin') + const [row] = await db + .select({ ownerId: workspace.ownerId }) + .from(workspace) + .where(eq(workspace.id, unresolvedOwnerWorkspaceId)) + expect(row.ownerId).toBe(users.wsOwner) + }) + + it('removes another member and audits the removal', async () => { + const response = await call(admin, 'DELETE', users.writer) + expect(response).toEqual({ + status: 200, + body: { data: { userId: users.writer, deleted: true, organizationMembershipRemoved: true } }, + }) + expect(await roleOf(users.writer)).toBeNull() + expect(await orgMembership(users.writer)).toHaveLength(0) + const [entry] = await awaitAudit('member.removed', 1) + expect(entry.actorId).toBe(users.admin) + expect(entry.metadata).toMatchObject({ removedUserId: users.writer }) + }) + + it('lets a member leave with a personal key, ending org membership and sessions', async () => { + const response = await call(key('reader'), 'DELETE', users.reader) + expect(response).toEqual({ + status: 200, + body: { data: { userId: users.reader, deleted: true, organizationMembershipRemoved: true } }, + }) + expect(await roleOf(users.reader)).toBeNull() + expect(await orgMembership(users.reader)).toHaveLength(0) + expect(await db.select().from(session).where(eq(session.userId, users.reader))).toHaveLength(0) + const rows = await awaitAudit('member.removed', 2) + expect(rows[1].actorId).toBe(users.reader) + expect(rows[1].metadata).toMatchObject({ selfRemoval: true }) + expect((await call(key('reader'), 'DELETE', users.reader)).status).toBe(404) + }) +}) diff --git a/apps/sim/lib/workspaces/api/route-policies.ts b/apps/sim/lib/workspaces/api/route-policies.ts index 65854edc379..02505f26fad 100644 --- a/apps/sim/lib/workspaces/api/route-policies.ts +++ b/apps/sim/lib/workspaces/api/route-policies.ts @@ -19,11 +19,9 @@ export const v2WorkspaceErrorPolicies = { notFoundMessage: 'Workspace not found', }), /** - * Member administration adds two failures a caller can act on that the - * shared projection would answer as 500: an owner whose removal leaves no - * billing account to inherit the workspace (a state conflict, resolved by - * reassigning billing), and contention that outlived the transaction (resolved - * by waiting). + * Member administration adds two actionable failures the shared projection + * would answer as 500: an owner removal with no billing account to inherit + * (409) and contention that outlived the transaction (503). */ manageWorkspaceMembers: createV2ResourceConcealmentPolicy({ notFoundMessage: 'Workspace not found', diff --git a/apps/sim/lib/workspaces/application/manage-permissions.test.ts b/apps/sim/lib/workspaces/application/manage-permissions.test.ts index edabdbf03b5..8c2ed6c2418 100644 --- a/apps/sim/lib/workspaces/application/manage-permissions.test.ts +++ b/apps/sim/lib/workspaces/application/manage-permissions.test.ts @@ -179,6 +179,8 @@ describe('workspace permission operations', () => { .catch((error: unknown) => error) expect(forbiddenErrorDetails(result)).toEqual({ code: 'SCIM_MANAGED_MEMBERSHIP' }) expect(result).toMatchObject({ code: 'forbidden', message: refusal.message }) + // The internal route renders the store's HTTP error, so it must stay the cause. + expect((result as Error).cause).toBeInstanceOf(DirectoryRefusal) expect(auditMockFns.mockRecordAudit).not.toHaveBeenCalled() }) diff --git a/apps/sim/lib/workspaces/application/manage-permissions.ts b/apps/sim/lib/workspaces/application/manage-permissions.ts index fdc82569f9b..d8d8a660813 100644 --- a/apps/sim/lib/workspaces/application/manage-permissions.ts +++ b/apps/sim/lib/workspaces/application/manage-permissions.ts @@ -1,8 +1,9 @@ import { AuditAction, AuditResourceType } from '@sim/audit' import { type Principal, resolvePrincipalSubject } from '@sim/auth/principal' import type { PermissionType } from '@sim/platform-authz/workspace' +import { findCause } from '@sim/utils/errors' import { defineAuthorizedWorkspaceUseCase, ForbiddenOperationError } from '@/lib/core/application' -import { OrchestrationError } from '@/lib/core/orchestration/types' +import { OrchestrationError, type OrchestrationErrorCode } from '@/lib/core/orchestration/types' import { HttpError } from '@/lib/core/utils/http-error' import { resolveActiveWorkspaceApplicationContext } from '@/lib/workspaces/application/context' import { workspacePermissionOperations } from '@/lib/workspaces/application/member-operations' @@ -54,45 +55,37 @@ export const readWorkspacePermissions = defineAuthorizedWorkspaceUseCase({ }, }) +const STORE_FAILURE_CODES: Partial> = { + 400: 'validation', + 403: 'forbidden', + 404: 'not_found', + 409: 'conflict', +} + /** * The caller-safe failure a store error stands for, or the error itself when it - * names nothing a caller may see. - * - * This store's typed errors promise caller-safe wording; driver failures remain - * private. A coded refusal anywhere in the chain wins over the HTTP error that - * wraps it, so a directory-managed member keeps its `SCIM_MANAGED_MEMBERSHIP` - * code instead of collapsing into a bare forbidden. + * names nothing a caller may see: the store's typed errors promise caller-safe + * wording, driver failures stay private. A coded refusal the HTTP error wraps + * keeps its code (`SCIM_MANAGED_MEMBERSHIP`), and the HTTP error stays the + * `cause` the internal route renders. */ function callerSafeStoreFailure(error: unknown): unknown { - const chain: Error[] = [] - for ( - let cause: unknown = error; - cause instanceof Error && !chain.includes(cause); - cause = cause.cause - ) { - chain.push(cause) - } - const refusal = chain.find((cause) => cause instanceof ForbiddenOperationError) - if (refusal) return refusal - for (const cause of chain) { - if (!(cause instanceof HttpError)) continue - const code = - cause.statusCode === 400 - ? 'validation' - : cause.statusCode === 403 - ? 'forbidden' - : cause.statusCode === 404 - ? 'not_found' - : cause.statusCode === 409 - ? 'conflict' - : undefined - if (code) { - const failure = new OrchestrationError(code, cause.message) - failure.cause = cause - return failure - } - } - return error + const httpError = findCause( + error, + (cause): cause is HttpError => + cause instanceof HttpError && STORE_FAILURE_CODES[cause.statusCode] !== undefined + ) + const code = httpError && STORE_FAILURE_CODES[httpError.statusCode] + if (!httpError || !code) return error + const refusal = findCause( + httpError.cause, + (cause): cause is ForbiddenOperationError => cause instanceof ForbiddenOperationError + ) + const failure = refusal + ? new ForbiddenOperationError(refusal.detailCode, httpError.message) + : new OrchestrationError(code, httpError.message) + failure.cause = httpError + return failure } async function applyPermissionUpdates( @@ -145,14 +138,11 @@ export interface UpdateWorkspaceMemberInput extends ReadWorkspacePermissionsInpu } /** - * One member's role change, answered with that member as the roster shows them - * afterwards. The same operation and store as {@link updateWorkspacePermissions}; - * a request for the role the member already holds changes and audits nothing. - * - * The member is read before the write: audit is projected only once `execute` - * returns, so a read failing after the commit would leave a durable role change - * with no audit entry. A successful write leaves exactly the requested role, - * because the store refuses inherited (organization admin) roles outright. + * One member's role change through the same operation and store as + * {@link updateWorkspacePermissions}, answered with the member as the roster + * shows them; requesting the current role changes and audits nothing. The + * member is read before the write so a failed read cannot leave a committed, + * unaudited change. */ export const updateWorkspaceMember = defineAuthorizedWorkspaceUseCase({ operation: workspacePermissionOperations.update, @@ -163,7 +153,7 @@ export const updateWorkspaceMember = defineAuthorizedWorkspaceUseCase({ const changes = await applyPermissionUpdates(context.workspaceId, principal, [ { userId: input.userId, permissions: input.role }, ]) - /** Granted between the read and the locked write; the store vetted them as a member. */ + // Granted between the read and the locked write; the store vetted them as a member. const member = before ?? (await getPublicWorkspaceMember(context.workspaceId, input.userId)) if (!member) { throw new OrchestrationError( diff --git a/apps/sim/scripts/test-workspace-members-e2e.ts b/apps/sim/scripts/test-workspace-members-e2e.ts deleted file mode 100644 index b304f267fa5..00000000000 --- a/apps/sim/scripts/test-workspace-members-e2e.ts +++ /dev/null @@ -1,522 +0,0 @@ -import assert from 'node:assert/strict' -import { writeFile } from 'node:fs/promises' -import { assertDisposableTestDatabaseUrl } from '@sim/db/testing/test-infrastructure' -import { createLogger } from '@sim/logger' -import { sha256Hex } from '@sim/security/hash' -import { getErrorMessage } from '@sim/utils/errors' -import { sleep } from '@sim/utils/helpers' -import { generateId, generateShortId } from '@sim/utils/id' -import { isRecordLike } from '@sim/utils/object' -import { truncate } from '@sim/utils/string' -import postgres from 'postgres' - -/** - * Exercises `PATCH` and `DELETE /api/v2/workspaces/{workspaceId}/members/{userId}` - * over real HTTP against a running local app and a disposable PostgreSQL - * database. SQL seeds identities, credentials, and workspaces, and inspects - * persistence and audit rows; every member operation crosses the HTTP boundary. - * - * Required: WORKSPACE_MEMBERS_E2E_BASE_URL (loopback origin), - * WORKSPACE_MEMBERS_E2E_DATABASE_URL (loopback, database name containing - * "test"), WORKSPACE_MEMBERS_E2E_REPORT_PATH (JSON report destination). - */ -const logger = createLogger('WorkspaceMembersE2E') - -function required(name: string): string { - const value = process.env[name] - assert(value, `${name} must be explicitly provided`) - return value -} - -const baseUrl = new URL(required('WORKSPACE_MEMBERS_E2E_BASE_URL')) -assert(['localhost', '127.0.0.1', '[::1]'].includes(baseUrl.hostname), 'Use a loopback app host') -assert.equal(baseUrl.protocol, 'http:', 'The app must use local HTTP') -assert.equal(baseUrl.pathname, '/', 'App URL must be an origin without a path') -const databaseUrl = assertDisposableTestDatabaseUrl(required('WORKSPACE_MEMBERS_E2E_DATABASE_URL')) -const reportPath = required('WORKSPACE_MEMBERS_E2E_REPORT_PATH') - -const sql = postgres(databaseUrl.toString(), { max: 2 }) -const checks: { name: string; status: 'passed' | 'failed'; durationMs: number; error?: string }[] = - [] -const requests: { method: string; path: string; status: number }[] = [] -const startedAt = new Date().toISOString() - -const runId = generateId().replaceAll('-', '') -const domain = `workspace-members-e2e-${runId}.test` -const orgId = generateId() -const oauthClientId = generateId() - -/** The primary organization workspace and two personal workspaces for the edge states. */ -const workspaceId = generateId() -const lastAdminWorkspaceId = generateId() -const unresolvedOwnerWorkspaceId = generateId() -const workspaceIds = [workspaceId, lastAdminWorkspaceId, unresolvedOwnerWorkspaceId] - -const users = { - orgOwner: generateId(), - admin: generateId(), - wsOwner: generateId(), - billing: generateId(), - writer: generateId(), - reader: generateId(), - outsider: generateId(), -} as const -type UserName = keyof typeof users - -const personalKeys = { - admin: `sk-sim-e2e-${generateId()}`, - writer: `sk-sim-e2e-${generateId()}`, - reader: `sk-sim-e2e-${generateId()}`, - outsider: `sk-sim-e2e-${generateId()}`, -} -const workspaceKey = `sk-sim-e2e-ws-${generateId()}` -const oauthReadToken = `sim_oat_${generateShortId(32)}` -const oauthWriteToken = `sim_oat_${generateShortId(32)}` - -type Credential = { apiKey: string } | { bearer: string } - -interface V2ErrorBody { - code: string - message: string - details?: Record -} - -function record(value: unknown): Record { - assert(isRecordLike(value), 'Expected a JSON object') - return value -} - -function emailOf(name: UserName): string { - return `${name}@${domain}` -} - -function memberPath(targetUserId: string, workspace = workspaceId): string { - return `/api/v2/workspaces/${workspace}/members/${encodeURIComponent(targetUserId)}` -} - -async function call( - credential: Credential, - method: 'GET' | 'PATCH' | 'DELETE', - path: string, - expected: number, - body?: unknown -): Promise> { - const headers = new Headers({ 'User-Agent': 'Sim-local-workspace-members-E2E' }) - if ('apiKey' in credential) headers.set('X-API-Key', credential.apiKey) - else headers.set('Authorization', `Bearer ${credential.bearer}`) - if (body !== undefined) headers.set('Content-Type', 'application/json') - const response = await fetch(new URL(path, baseUrl), { - method, - headers, - body: body === undefined ? undefined : JSON.stringify(body), - redirect: 'error', - signal: AbortSignal.timeout(60_000), - }) - requests.push({ method, path, status: response.status }) - const text = await response.text() - assert.equal( - response.status, - expected, - `${method} ${path}: ${response.status} ${truncate(text, 500)}` - ) - assert.match(response.headers.get('cache-control') ?? '', /no-store/, 'v2 responses are no-store') - return record(JSON.parse(text)) -} - -/** Asserts the v2 failure envelope and returns its `error`. */ -async function fails( - credential: Credential, - method: 'PATCH' | 'DELETE', - path: string, - expected: number, - body?: unknown -): Promise { - const response = await call(credential, method, path, expected, body) - assert.deepEqual(Object.keys(response), ['error'], 'A failure carries only the error envelope') - const error = record(response.error) - assert.equal(typeof error.code, 'string') - assert.equal(typeof error.message, 'string') - return { - code: String(error.code), - message: String(error.message), - ...(error.details === undefined ? {} : { details: record(error.details) }), - } -} - -const key = (name: keyof typeof personalKeys): Credential => ({ apiKey: personalKeys[name] }) - -async function permissionOf(userId: string, workspace = workspaceId): Promise { - const [row] = await sql<{ permission_type: string }[]>`select permission_type from permissions - where entity_type = 'workspace' and entity_id = ${workspace} and user_id = ${userId}` - return row?.permission_type ?? null -} - -async function auditRows(action: string, workspace = workspaceId) { - return sql< - { actor_id: string | null; description: string | null; metadata: Record }[] - >`select actor_id, description, metadata from audit_log - where workspace_id = ${workspace} and action = ${action} order by created_at asc` -} - -/** Audit writes may land just after the response; poll briefly for the expected count. */ -async function awaitAuditCount(action: string, count: number, workspace = workspaceId) { - const deadline = performance.now() + 5_000 - for (;;) { - const rows = await auditRows(action, workspace) - if (rows.length >= count || performance.now() > deadline) return rows - await sleep(100) - } -} - -async function check(name: string, run: () => Promise) { - const started = performance.now() - try { - await run() - checks.push({ name, status: 'passed', durationMs: Math.round(performance.now() - started) }) - logger.info(`PASS ${name}`) - } catch (error) { - checks.push({ - name, - status: 'failed', - durationMs: Math.round(performance.now() - started), - error: getErrorMessage(error), - }) - throw error - } -} - -async function seed() { - await sql.begin(async (tx) => { - for (const [name, id] of Object.entries(users) as [UserName, string][]) { - await tx`insert into "user" (id, name, email, normalized_email, email_verified, created_at, updated_at) - values (${id}, ${`E2E ${name}`}, ${emailOf(name)}, ${emailOf(name)}, true, now(), now())` - await tx`insert into user_stats (id, user_id) values (${generateId()}, ${id})` - } - await tx`insert into organization (id, name, slug, created_at) - values (${orgId}, 'Workspace members E2E', ${`workspace-members-e2e-${runId}`}, now())` - await tx`insert into member (id, user_id, organization_id, role) values - (${generateId()}, ${users.orgOwner}, ${orgId}, 'owner'), - (${generateId()}, ${users.admin}, ${orgId}, 'member'), - (${generateId()}, ${users.wsOwner}, ${orgId}, 'member'), - (${generateId()}, ${users.billing}, ${orgId}, 'member'), - (${generateId()}, ${users.writer}, ${orgId}, 'member'), - (${generateId()}, ${users.reader}, ${orgId}, 'member')` - - await tx`insert into project (id, name, owner_id, organization_id) - values (${workspaceId}, 'Members E2E', ${users.wsOwner}, ${orgId})` - await tx`insert into workspace (id, project_id, name, owner_id, billed_account_user_id, organization_id) - values (${workspaceId}, ${workspaceId}, 'Members E2E', ${users.wsOwner}, ${users.billing}, ${orgId})` - await tx`insert into permissions (id, user_id, entity_type, entity_id, permission_type) values - (${generateId()}, ${users.admin}, 'workspace', ${workspaceId}, 'admin'), - (${generateId()}, ${users.wsOwner}, 'workspace', ${workspaceId}, 'admin'), - (${generateId()}, ${users.billing}, 'workspace', ${workspaceId}, 'admin'), - (${generateId()}, ${users.writer}, 'workspace', ${workspaceId}, 'write'), - (${generateId()}, ${users.reader}, 'workspace', ${workspaceId}, 'read')` - - /** Personal workspace whose only explicit admin is `admin`; its owner holds no grant. */ - await tx`insert into project (id, name, owner_id) - values (${lastAdminWorkspaceId}, 'Members E2E last admin', ${users.billing})` - await tx`insert into workspace (id, project_id, name, owner_id, billed_account_user_id) - values (${lastAdminWorkspaceId}, ${lastAdminWorkspaceId}, 'Members E2E last admin', ${users.billing}, ${users.billing})` - await tx`insert into permissions (id, user_id, entity_type, entity_id, permission_type) - values (${generateId()}, ${users.admin}, 'workspace', ${lastAdminWorkspaceId}, 'admin')` - - /** - * Personal workspace whose billing account holds no grant, so removing its - * owner leaves nobody to inherit it: the revocation refuses inside the - * transaction with the billing-account removal error. - */ - await tx`insert into project (id, name, owner_id) - values (${unresolvedOwnerWorkspaceId}, 'Members E2E unresolved owner', ${users.wsOwner})` - await tx`insert into workspace (id, project_id, name, owner_id, billed_account_user_id) - values (${unresolvedOwnerWorkspaceId}, ${unresolvedOwnerWorkspaceId}, 'Members E2E unresolved owner', ${users.wsOwner}, ${users.billing})` - await tx`insert into permissions (id, user_id, entity_type, entity_id, permission_type) values - (${generateId()}, ${users.wsOwner}, 'workspace', ${unresolvedOwnerWorkspaceId}, 'admin'), - (${generateId()}, ${users.admin}, 'workspace', ${unresolvedOwnerWorkspaceId}, 'admin')` - - for (const [name, value] of Object.entries(personalKeys) as [ - keyof typeof personalKeys, - string, - ][]) { - await tx`insert into api_key (id, user_id, name, key, key_hash, type) - values (${generateId()}, ${users[name]}, 'Members E2E', ${value}, ${sha256Hex(value)}, 'personal')` - } - await tx`insert into api_key (id, user_id, workspace_id, created_by, name, key, key_hash, type) - values (${generateId()}, ${users.admin}, ${workspaceId}, ${users.admin}, 'Members E2E workspace key', - ${workspaceKey}, ${sha256Hex(workspaceKey)}, 'workspace')` - - await tx`insert into oauth_client (id, client_id, name, public, token_endpoint_auth_method, grant_types, redirect_uris) - values (${oauthClientId}, ${oauthClientId}, 'Members E2E OAuth client', true, 'none', - ${['authorization_code', 'refresh_token']}, ${['http://127.0.0.1/callback']})` - for (const [token, scopes] of [ - [oauthReadToken, ['api:read']], - [oauthWriteToken, ['api:write']], - ] as const) { - await tx`insert into oauth_access_token (id, token, client_id, user_id, scopes, created_at, expires_at) - values (${generateId()}, ${sha256Hex(token.slice('sim_oat_'.length))}, ${oauthClientId}, - ${users.admin}, ${[...scopes]}, now(), now() + interval '1 hour')` - } - - /** A live session for `reader`, which leaving their last organization workspace must end. */ - await tx`insert into session (id, token, user_id, expires_at, created_at, updated_at) - values (${generateId()}, ${generateShortId()}, ${users.reader}, now() + interval '1 day', now(), now())` - }) -} - -async function cleanup() { - const userIds = Object.values(users) - await sql.begin(async (tx) => { - await tx`delete from audit_log where workspace_id in ${tx(workspaceIds)}` - await tx`delete from api_key where user_id in ${tx(userIds)}` - await tx`delete from oauth_client where client_id = ${oauthClientId}` - await tx`delete from permissions where entity_type = 'workspace' and entity_id in ${tx(workspaceIds)}` - await tx`delete from workspace where id in ${tx(workspaceIds)}` - await tx`delete from project where id in ${tx(workspaceIds)}` - await tx`delete from member where organization_id = ${orgId}` - await tx`delete from organization where id = ${orgId}` - await tx`delete from "user" where id in ${tx(userIds)}` - }) -} - -async function run() { - await seed() - const admin = key('admin') - - await check('A workspace API key is refused on both methods with a coded 403', async () => { - for (const [method, body] of [ - ['PATCH', { role: 'write' }], - ['DELETE', undefined], - ] as const) { - const error = await fails( - { apiKey: workspaceKey }, - method, - memberPath(users.reader), - 403, - body - ) - assert.equal(error.code, 'FORBIDDEN') - assert( - ['WORKSPACE_KEY_OPERATION_NOT_PERMITTED', 'PRINCIPAL_KIND_NOT_PERMITTED'].includes( - String(error.details?.code) - ), - `Unexpected workspace-key refusal code ${String(error.details?.code)}` - ) - } - assert.equal(await permissionOf(users.reader), 'read') - }) - - await check('An OAuth token without api:write is refused with INSUFFICIENT_SCOPE', async () => { - for (const [method, body] of [ - ['PATCH', { role: 'write' }], - ['DELETE', undefined], - ] as const) { - const error = await fails( - { bearer: oauthReadToken }, - method, - memberPath(users.reader), - 403, - body - ) - assert.equal(error.details?.code, 'INSUFFICIENT_SCOPE') - } - assert.equal(await permissionOf(users.reader), 'read') - }) - - await check('A non-admin cannot change or remove someone else', async () => { - const patch = await fails(key('writer'), 'PATCH', memberPath(users.reader), 403, { - role: 'write', - }) - assert.equal(patch.details?.code, 'INSUFFICIENT_WORKSPACE_ROLE') - const remove = await fails(key('writer'), 'DELETE', memberPath(users.reader), 403) - assert.equal(remove.details?.code, 'INSUFFICIENT_WORKSPACE_ROLE') - assert.equal(await permissionOf(users.reader), 'read') - }) - - await check('An unreachable or unknown workspace answers an identical 404', async () => { - const unreachable = await fails(key('outsider'), 'PATCH', memberPath(users.reader), 404, { - role: 'write', - }) - const unknown = await fails(admin, 'PATCH', memberPath(users.reader, generateId()), 404, { - role: 'write', - }) - assert.deepEqual(unreachable, unknown) - assert.deepEqual(unreachable, { code: 'NOT_FOUND', message: 'Workspace not found' }) - await fails(key('outsider'), 'DELETE', memberPath(users.reader), 404) - }) - - await check('A non-member target is a 400 to change and a 404 to remove', async () => { - await fails(admin, 'PATCH', memberPath(users.outsider), 400, { role: 'write' }) - await fails(admin, 'DELETE', memberPath(users.outsider), 404) - assert.equal(await permissionOf(users.outsider), null) - }) - - await check('Invalid bodies and query strings are rejected before any write', async () => { - await fails(admin, 'PATCH', memberPath(users.reader), 400, { role: 'owner' }) - await fails(admin, 'PATCH', memberPath(users.reader), 400, { role: 'write', extra: true }) - await fails(admin, 'PATCH', memberPath(users.reader), 400, {}) - await fails(admin, 'PATCH', `${memberPath(users.reader)}?force=1`, 400, { role: 'write' }) - await fails(admin, 'DELETE', `${memberPath(users.reader)}?force=1`, 400) - await fails(admin, 'PATCH', memberPath('x'.repeat(200)), 400, { role: 'write' }) - assert.equal(await permissionOf(users.reader), 'read') - }) - - await check('An admin cannot lower their own role', async () => { - const error = await fails(admin, 'PATCH', memberPath(users.admin), 400, { role: 'write' }) - assert.match(error.message, /own admin/) - assert.equal(await permissionOf(users.admin), 'admin') - }) - - await check('Owner, billing account, and organization admins keep Admin', async () => { - await fails(admin, 'PATCH', memberPath(users.wsOwner), 400, { role: 'read' }) - await fails(admin, 'PATCH', memberPath(users.billing), 400, { role: 'read' }) - await fails(admin, 'PATCH', memberPath(users.orgOwner), 400, { role: 'write' }) - await fails(admin, 'DELETE', memberPath(users.orgOwner), 400) - await fails(admin, 'DELETE', memberPath(users.billing), 400) - assert.equal(await permissionOf(users.wsOwner), 'admin') - assert.equal(await permissionOf(users.billing), 'admin') - }) - - await check( - 'A role change returns the roster member and audits the key owner as actor', - async () => { - const before = (await auditRows('member.role_changed')).length - const response = await call(admin, 'PATCH', memberPath(users.reader), 200, { - role: 'write', - }) - assert.deepEqual(Object.keys(response), ['data']) - const roster = await call(admin, 'GET', `/api/v2/workspaces/${workspaceId}/members`, 200) - assert(Array.isArray(roster.data)) - const listed = roster.data.map(record).find((entry) => entry.userId === users.reader) - assert.deepEqual(response.data, listed, 'PATCH returns the member exactly as the list does') - assert.equal(record(response.data).role, 'write') - assert.equal(await permissionOf(users.reader), 'write') - - const rows = await awaitAuditCount('member.role_changed', before + 1) - assert.equal(rows.length, before + 1) - const entry = rows[rows.length - 1] - assert.equal(entry.actor_id, users.admin) - assert.equal(record(entry.metadata).targetUserId, users.reader) - assert.equal(record(entry.metadata).previousRole, 'read') - assert.equal(record(record(entry.metadata).actor).kind, 'personal_api_key') - } - ) - - await check('An OAuth token with api:write can change a role', async () => { - const response = await call( - { bearer: oauthWriteToken }, - 'PATCH', - memberPath(users.reader), - 200, - { - role: 'read', - } - ) - assert.equal(record(response.data).role, 'read') - assert.equal(await permissionOf(users.reader), 'read') - const rows = await awaitAuditCount('member.role_changed', 2) - assert.equal(record(record(rows[rows.length - 1].metadata).actor).kind, 'oauth_access_token') - assert.equal(rows[rows.length - 1].actor_id, users.admin) - }) - - await check('Requesting the current role writes nothing and records no audit', async () => { - const before = await auditRows('member.role_changed') - const [{ updated_at: updatedBefore }] = await sql<{ updated_at: Date }[]>` - select updated_at from permissions - where entity_type = 'workspace' and entity_id = ${workspaceId} and user_id = ${users.reader}` - const response = await call(admin, 'PATCH', memberPath(users.reader), 200, { role: 'read' }) - assert.equal(record(response.data).role, 'read') - await sleep(1_000) - assert.equal((await auditRows('member.role_changed')).length, before.length) - const [{ updated_at: updatedAfter }] = await sql<{ updated_at: Date }[]>` - select updated_at from permissions - where entity_type = 'workspace' and entity_id = ${workspaceId} and user_id = ${users.reader}` - assert.equal(updatedAfter.getTime(), updatedBefore.getTime()) - }) - - await check('The last explicit admin cannot leave', async () => { - const error = await fails(admin, 'DELETE', memberPath(users.admin, lastAdminWorkspaceId), 400) - assert.match(error.message, /last admin/) - assert.equal(await permissionOf(users.admin, lastAdminWorkspaceId), 'admin') - }) - - await check('An owner removal with no successor is a 409, not a 500', async () => { - const error = await fails( - admin, - 'DELETE', - memberPath(users.wsOwner, unresolvedOwnerWorkspaceId), - 409 - ) - assert.equal(error.code, 'CONFLICT') - assert.match(error.message, /billing account/) - assert.equal(await permissionOf(users.wsOwner, unresolvedOwnerWorkspaceId), 'admin') - const [owner] = await sql<{ owner_id: string }[]>`select owner_id from workspace - where id = ${unresolvedOwnerWorkspaceId}` - assert.equal(owner.owner_id, users.wsOwner) - }) - - await check('An admin removes another member and the removal is audited', async () => { - const before = (await auditRows('member.removed')).length - const response = await call(admin, 'DELETE', memberPath(users.writer), 200) - const data = record(response.data) - assert.equal(data.userId, users.writer) - assert.equal(data.deleted, true) - assert.equal(data.organizationMembershipRemoved, true, 'This was their only org workspace') - assert.equal(await permissionOf(users.writer), null) - const membership = await sql`select id from member - where organization_id = ${orgId} and user_id = ${users.writer}` - assert.equal(membership.length, 0) - const rows = await awaitAuditCount('member.removed', before + 1) - assert.equal(rows[rows.length - 1].actor_id, users.admin) - assert.equal(record(rows[rows.length - 1].metadata).removedUserId, users.writer) - }) - - await check( - 'A member leaves with a personal key, losing org membership and sessions', - async () => { - const response = await call(key('reader'), 'DELETE', memberPath(users.reader), 200) - const data = record(response.data) - assert.equal(data.userId, users.reader) - assert.equal(data.organizationMembershipRemoved, true) - assert.equal(await permissionOf(users.reader), null) - const membership = await sql`select id from member - where organization_id = ${orgId} and user_id = ${users.reader}` - assert.equal(membership.length, 0) - const sessions = await sql`select id from session where user_id = ${users.reader}` - assert.equal(sessions.length, 0, 'Leaving the last org workspace ends every session') - const rows = await awaitAuditCount('member.removed', 2) - const entry = rows[rows.length - 1] - assert.equal(entry.actor_id, users.reader) - assert.equal(record(entry.metadata).selfRemoval, true) - await fails(key('reader'), 'DELETE', memberPath(users.reader), 404) - } - ) -} - -try { - await run() -} catch (error) { - logger.error('Workspace members E2E failed', { error: getErrorMessage(error) }) - process.exitCode = 1 -} finally { - try { - await cleanup() - logger.info('Removed workspace members E2E fixtures') - } catch (error) { - logger.error('Workspace members E2E fixture cleanup failed', { error: getErrorMessage(error) }) - process.exitCode = 1 - } - await sql.end() - const report = { - startedAt, - completedAt: new Date().toISOString(), - status: process.exitCode ? 'failed' : 'passed', - transport: 'Real HTTP against a local Next.js app and PostgreSQL', - requestCount: requests.length, - checks, - requests, - } - await writeFile(reportPath, `${JSON.stringify(report, null, 2)}\n`) - logger.info( - `Workspace members E2E ${report.status}: ${checks.filter((item) => item.status === 'passed').length}/${checks.length} checks, ${requests.length} HTTP requests` - ) -} From ebdce71a5eb8eb1b98d94f73b8cc9b88f0171be9 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Tue, 10 Nov 2026 13:03:59 -0800 Subject: [PATCH 4/6] fix(api): align v2 workspace member errors and keep the old CLI spelling - Answer a role change for a non-member with 404 "Member not found", as the organization member route does, and drop the post-write re-read. - Answer role-change contention with 503 and Retry-After, like removal; the internal route keeps its 409. - Keep `sim workspaces members` listing members as the default subcommand of the new group. - Correct the member route prose and the workspaceId parameter wording. - Give each integration test its own fixture and cover contention. --- apps/docs/openapi-v2-resources.json | 12 +- .../lib/api/contracts/v2/openapi/resources.ts | 4 +- apps/sim/lib/api/contracts/v2/workspaces.ts | 5 +- .../lib/api/mcp/generated/v2-operations.ts | 4 +- .../__integration__/members.integration.ts | 634 ++++++++++-------- apps/sim/lib/workspaces/api/route-policies.ts | 11 +- .../application/manage-permissions.test.ts | 22 +- .../application/manage-permissions.ts | 11 +- .../permissions/management-store.ts | 2 +- packages/sim-cli/src/contract/commands.ts | 3 + packages/sim-cli/src/generated/v2-api.ts | 4 +- .../sim-cli/src/runtime/group-default.test.ts | 20 + 12 files changed, 405 insertions(+), 327 deletions(-) diff --git a/apps/docs/openapi-v2-resources.json b/apps/docs/openapi-v2-resources.json index 3c9bba988dc..ede1e4ba70b 100644 --- a/apps/docs/openapi-v2-resources.json +++ b/apps/docs/openapi-v2-resources.json @@ -425,7 +425,7 @@ "patch": { "operationId": "updateWorkspaceMember", "summary": "Update workspace member", - "description": "Change one existing member’s workspace role. Requires workspace Admin access. Only existing members can be changed; add people with Create workspace invitations. You cannot lower your own role, and organization Admins, the workspace owner, and the workspace billing account always keep Admin. Members whose organization membership an identity provider manages cannot be changed here. Requesting the role a member already holds changes nothing and records no audit entry. Workspace API keys return `403`; use a personal API key or scoped OAuth token.\n\nOAuth scope: `api:write`.", + "description": "Change one existing member’s workspace role. Requires workspace Admin access. Only existing members can be changed; anyone else is Not found, and people are added with Create workspace invitations. You cannot lower your own role, and organization Admins, the workspace owner, and the workspace billing account always keep Admin. Members whose organization membership an identity provider manages cannot be changed here. Requesting the role a member already holds changes nothing and records no audit entry, except that an organization Admin is refused whatever role is requested. Workspace API keys return `403`; use a personal API key or scoped OAuth token.\n\nOAuth scope: `api:write`.", "x-sim-operation": "workspaces.permissions.update", "x-oauth-scope": "api:write", "tags": ["Workspaces"], @@ -442,12 +442,12 @@ "name": "workspaceId", "in": "path", "required": true, - "description": "Workspace to retrieve.", + "description": "Workspace the member belongs to.", "schema": { "type": "string", "minLength": 1, "maxLength": 128, - "description": "Workspace to retrieve." + "description": "Workspace the member belongs to." } }, { @@ -555,7 +555,7 @@ "delete": { "operationId": "removeWorkspaceMember", "summary": "Remove workspace member", - "description": "Remove a member’s workspace access, or leave the workspace by naming yourself. Removing someone else requires workspace Admin access; any member may leave. The last Admin cannot leave, organization Admins hold access through their organization role and cannot be removed here, and the workspace billing account cannot be removed until billing is reassigned. Removing the workspace owner transfers ownership to the billing account. When this was the member’s last workspace in its organization, their organization membership and seat are removed too and all of their sessions end. Workspace API keys return `403`; use a personal API key or scoped OAuth token.\n\nOAuth scope: `api:write`.", + "description": "Remove a member’s workspace access, or leave the workspace by naming yourself. Removing someone else requires workspace Admin access; any member may leave. The last Admin cannot leave, organization Admins hold access through their organization role and cannot be removed here, and the workspace billing account cannot be removed until billing is reassigned. Removing the workspace owner transfers ownership to the billing account. When this was the member’s last workspace in its organization, their organization membership and seat are removed too and all of their sessions end, since this endpoint is called with an API key or OAuth token rather than from a signed-in browser session. Workspace API keys return `403`; use a personal API key or scoped OAuth token.\n\nOAuth scope: `api:write`.", "x-sim-operation": "workspaces.members.remove", "x-oauth-scope": "api:write", "tags": ["Workspaces"], @@ -572,12 +572,12 @@ "name": "workspaceId", "in": "path", "required": true, - "description": "Workspace to retrieve.", + "description": "Workspace the member belongs to.", "schema": { "type": "string", "minLength": 1, "maxLength": 128, - "description": "Workspace to retrieve." + "description": "Workspace the member belongs to." } }, { diff --git a/apps/sim/lib/api/contracts/v2/openapi/resources.ts b/apps/sim/lib/api/contracts/v2/openapi/resources.ts index febb8965f27..2652cd824fd 100644 --- a/apps/sim/lib/api/contracts/v2/openapi/resources.ts +++ b/apps/sim/lib/api/contracts/v2/openapi/resources.ts @@ -764,7 +764,7 @@ const declaredRoutes = [ applicationOperation: workspacePermissionOperations.update, operationId: 'updateWorkspaceMember', summary: 'Update workspace member', - description: `Change one existing member’s workspace role. Requires workspace Admin access. Only existing members can be changed; add people with Create workspace invitations. You cannot lower your own role, and organization Admins, the workspace owner, and the workspace billing account always keep Admin. Members whose organization membership an identity provider manages cannot be changed here. Requesting the role a member already holds changes nothing and records no audit entry. ${WORKSPACE_API_KEY_DENIED}`, + description: `Change one existing member’s workspace role. Requires workspace Admin access. Only existing members can be changed; anyone else is Not found, and people are added with Create workspace invitations. You cannot lower your own role, and organization Admins, the workspace owner, and the workspace billing account always keep Admin. Members whose organization membership an identity provider manages cannot be changed here. Requesting the role a member already holds changes nothing and records no audit entry, except that an organization Admin is refused whatever role is requested. ${WORKSPACE_API_KEY_DENIED}`, errors: RESOURCE_CONFLICT_ERRORS, success: { description: 'The member with their updated workspace role.' }, }), @@ -798,7 +798,7 @@ const declaredRoutes = [ applicationOperation: removeWorkspaceMemberOperation, operationId: 'removeWorkspaceMember', summary: 'Remove workspace member', - description: `Remove a member’s workspace access, or leave the workspace by naming yourself. Removing someone else requires workspace Admin access; any member may leave. The last Admin cannot leave, organization Admins hold access through their organization role and cannot be removed here, and the workspace billing account cannot be removed until billing is reassigned. Removing the workspace owner transfers ownership to the billing account. When this was the member’s last workspace in its organization, their organization membership and seat are removed too and all of their sessions end. ${WORKSPACE_API_KEY_DENIED}`, + description: `Remove a member’s workspace access, or leave the workspace by naming yourself. Removing someone else requires workspace Admin access; any member may leave. The last Admin cannot leave, organization Admins hold access through their organization role and cannot be removed here, and the workspace billing account cannot be removed until billing is reassigned. Removing the workspace owner transfers ownership to the billing account. When this was the member’s last workspace in its organization, their organization membership and seat are removed too and all of their sessions end, since this endpoint is called with an API key or OAuth token rather than from a signed-in browser session. ${WORKSPACE_API_KEY_DENIED}`, errors: RESOURCE_CONFLICT_ERRORS, success: { description: 'Acknowledgement that workspace access was removed.' }, }), diff --git a/apps/sim/lib/api/contracts/v2/workspaces.ts b/apps/sim/lib/api/contracts/v2/workspaces.ts index 9823ecaba67..70e115098cb 100644 --- a/apps/sim/lib/api/contracts/v2/workspaces.ts +++ b/apps/sim/lib/api/contracts/v2/workspaces.ts @@ -121,8 +121,9 @@ export const v2ListWorkspaceMembersContract = defineRouteContract({ response: { mode: 'json', schema: v2CursorListResponse(v2WorkspaceMemberSchema) }, }) -const v2WorkspaceMemberParamsSchema = v2WorkspaceParamsSchema - .extend({ +const v2WorkspaceMemberParamsSchema = z + .object({ + workspaceId: workspaceIdSchema.describe('Workspace the member belongs to.'), userId: requiredFieldSchema('User ID is required') .max(MAX_ID_LENGTH, 'User ID is too long') .describe('User identifier of the workspace member, as returned by List workspace members.'), diff --git a/apps/sim/lib/api/mcp/generated/v2-operations.ts b/apps/sim/lib/api/mcp/generated/v2-operations.ts index 561d0e7d591..20a0a4b34b8 100644 --- a/apps/sim/lib/api/mcp/generated/v2-operations.ts +++ b/apps/sim/lib/api/mcp/generated/v2-operations.ts @@ -2450,7 +2450,7 @@ export const V2_MCP_OPERATIONS = { contract: v2RemoveWorkspaceMemberContract, summary: 'Remove workspace member', description: - 'Remove a member’s workspace access, or leave the workspace by naming yourself. Removing someone else requires workspace Admin access; any member may leave. The last Admin cannot leave, organization Admins hold access through their organization role and cannot be removed here, and the workspace billing account cannot be removed until billing is reassigned. Removing the workspace owner transfers ownership to the billing account. When this was the member’s last workspace in its organization, their organization membership and seat are removed too and all of their sessions end. Workspace API keys return `403`; use a personal API key or scoped OAuth token.\n\nOAuth scope: `api:write`.', + 'Remove a member’s workspace access, or leave the workspace by naming yourself. Removing someone else requires workspace Admin access; any member may leave. The last Admin cannot leave, organization Admins hold access through their organization role and cannot be removed here, and the workspace billing account cannot be removed until billing is reassigned. Removing the workspace owner transfers ownership to the billing account. When this was the member’s last workspace in its organization, their organization membership and seat are removed too and all of their sessions end, since this endpoint is called with an API key or OAuth token rather than from a signed-in browser session. Workspace API keys return `403`; use a personal API key or scoped OAuth token.\n\nOAuth scope: `api:write`.', workspaceKeyUnsupported: true, handler: () => import('@/app/api/v2/workspaces/[workspaceId]/members/[userId]/route').then( @@ -3025,7 +3025,7 @@ export const V2_MCP_OPERATIONS = { contract: v2UpdateWorkspaceMemberContract, summary: 'Update workspace member', description: - 'Change one existing member’s workspace role. Requires workspace Admin access. Only existing members can be changed; add people with Create workspace invitations. You cannot lower your own role, and organization Admins, the workspace owner, and the workspace billing account always keep Admin. Members whose organization membership an identity provider manages cannot be changed here. Requesting the role a member already holds changes nothing and records no audit entry. Workspace API keys return `403`; use a personal API key or scoped OAuth token.\n\nOAuth scope: `api:write`.', + 'Change one existing member’s workspace role. Requires workspace Admin access. Only existing members can be changed; anyone else is Not found, and people are added with Create workspace invitations. You cannot lower your own role, and organization Admins, the workspace owner, and the workspace billing account always keep Admin. Members whose organization membership an identity provider manages cannot be changed here. Requesting the role a member already holds changes nothing and records no audit entry, except that an organization Admin is refused whatever role is requested. Workspace API keys return `403`; use a personal API key or scoped OAuth token.\n\nOAuth scope: `api:write`.', workspaceKeyUnsupported: true, handler: () => import('@/app/api/v2/workspaces/[workspaceId]/members/[userId]/route').then( diff --git a/apps/sim/lib/workspaces/__integration__/members.integration.ts b/apps/sim/lib/workspaces/__integration__/members.integration.ts index af5e4dc2763..47aa1a511e4 100644 --- a/apps/sim/lib/workspaces/__integration__/members.integration.ts +++ b/apps/sim/lib/workspaces/__integration__/members.integration.ts @@ -20,7 +20,7 @@ import { envFlagsMock } from '@sim/testing/mocks/env-flags.mock' import { generateId, generateShortId } from '@sim/utils/id' import { and, asc, eq, inArray } from 'drizzle-orm' import { NextRequest } from 'next/server' -import { afterAll, beforeAll, describe, expect, it, vi } from 'vitest' +import { afterAll, afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import { hashApiKey } from '@/lib/api-key/crypto' import { hashOAuthToken } from '@/lib/auth/oauth-access-token' import { @@ -32,40 +32,51 @@ import { GET as listMembers } from '@/app/api/v2/workspaces/[workspaceId]/member /** Directory provisioning on a self-hosted deployment, so a locked SCIM member is refused. */ vi.mock('@/lib/core/config/env-flags', () => ({ ...envFlagsMock, isScimEnabled: true })) -const orgId = generateId() -const scimConnectionId = generateId() -const workspaceId = generateId() -/** Personal workspace whose only explicit admin is `admin`; its owner holds no grant. */ -const lastAdminWorkspaceId = generateId() -/** Personal workspace whose billing account holds no grant, so its owner has no successor. */ -const unresolvedOwnerWorkspaceId = generateId() -const workspaceIds = [workspaceId, lastAdminWorkspaceId, unresolvedOwnerWorkspaceId] - -const users = { - orgOwner: generateId(), - admin: generateId(), - wsOwner: generateId(), - billing: generateId(), - writer: generateId(), - reader: generateId(), - managed: generateId(), - outsider: generateId(), -} -const userIds = Object.values(users) -const personalKeys = { - admin: `sk-sim-fixture-${generateId()}`, - writer: `sk-sim-fixture-${generateId()}`, - reader: `sk-sim-fixture-${generateId()}`, - outsider: `sk-sim-fixture-${generateId()}`, +const USER_NAMES = [ + 'orgOwner', + 'admin', + 'wsOwner', + 'billing', + 'writer', + 'reader', + 'managed', + 'outsider', +] as const +const KEY_HOLDERS = ['admin', 'writer', 'reader', 'outsider'] as const +type KeyHolder = (typeof KEY_HOLDERS)[number] + +/** A fresh organization, users, workspaces, and credentials for every test. */ +function newFixture() { + const workspaceId = generateId() + /** Personal workspace whose only explicit admin is `admin`; its owner holds no grant. */ + const lastAdminWorkspaceId = generateId() + /** Personal workspace whose billing account holds no grant, so its owner has no successor. */ + const unresolvedOwnerWorkspaceId = generateId() + return { + orgId: generateId(), + scimConnectionId: generateId(), + oauthClientId: generateId(), + workspaceId, + lastAdminWorkspaceId, + unresolvedOwnerWorkspaceId, + workspaceIds: [workspaceId, lastAdminWorkspaceId, unresolvedOwnerWorkspaceId], + users: Object.fromEntries(USER_NAMES.map((name) => [name, generateId()])) as Record< + (typeof USER_NAMES)[number], + string + >, + keys: Object.fromEntries( + KEY_HOLDERS.map((name) => [name, `sk-sim-fixture-${generateId()}`]) + ) as Record, + workspaceKey: `sk-sim-fixture-${generateId()}`, + oauthReadToken: `sim_oat_${generateShortId(32)}`, + oauthWriteToken: `sim_oat_${generateShortId(32)}`, + } } -const workspaceKey = `sk-sim-fixture-${generateId()}` -const oauthClientId = generateId() -const oauthReadToken = `sim_oat_${generateShortId(32)}` -const oauthWriteToken = `sim_oat_${generateShortId(32)}` + +let f = newFixture() type Credential = { apiKey: string } | { bearer: string } -const key = (name: keyof typeof personalKeys): Credential => ({ apiKey: personalKeys[name] }) -const admin = key('admin') +const key = (name: KeyHolder): Credential => ({ apiKey: f.keys[name] }) async function call( credential: Credential, @@ -73,7 +84,7 @@ async function call( target: string, options: { workspace?: string; body?: unknown; query?: string } = {} ) { - const scope = options.workspace ?? workspaceId + const scope = options.workspace ?? f.workspaceId const headers = new Headers({ 'x-forwarded-for': '127.0.0.1' }) if ('apiKey' in credential) headers.set('X-API-Key', credential.apiKey) else headers.set('Authorization', `Bearer ${credential.bearer}`) @@ -90,10 +101,14 @@ async function call( const response = method === 'PATCH' ? await updateMember(request, context) : await removeMember(request, context) expect(response.headers.get('cache-control')).toMatch(/no-store/) - return { status: response.status, body: await response.json() } + return { + status: response.status, + retryAfter: response.headers.get('retry-after'), + body: await response.json(), + } } -async function roleOf(target: string, scope = workspaceId) { +async function roleOf(target: string, scope = f.workspaceId) { const [row] = await db .select({ role: permissions.permissionType }) .from(permissions) @@ -107,7 +122,7 @@ async function roleOf(target: string, scope = workspaceId) { return row?.role ?? null } -function auditRows(action: string, scope = workspaceId) { +function auditRows(action: string, scope = f.workspaceId) { return db .select() .from(auditLog) @@ -116,7 +131,7 @@ function auditRows(action: string, scope = workspaceId) { } /** Audit is written after the response; wait for the expected row count. */ -async function awaitAudit(action: string, count: number, scope = workspaceId) { +async function awaitAudit(action: string, count: number, scope = f.workspaceId) { return vi.waitFor( async () => { const rows = await auditRows(action, scope) @@ -131,184 +146,193 @@ async function orgMembership(target: string) { return db .select({ id: member.id }) .from(member) - .where(and(eq(member.organizationId, orgId), eq(member.userId, target))) + .where(and(eq(member.organizationId, f.orgId), eq(member.userId, target))) } -/** Drives the real v2 member route handlers with hashed credentials against PostgreSQL. */ -describe('v2 workspace member management against PostgreSQL', () => { - beforeAll(async () => { - const now = new Date() - await db.insert(user).values( - Object.entries(users).map(([name, id]) => ({ - id, - name: `Fixture ${name}`, - email: `${id}@members.test`, - normalizedEmail: `${id}@members.test`, - emailVerified: true, - createdAt: now, - updatedAt: now, - })) - ) - await db.insert(userStats).values(userIds.map((id) => ({ id: generateId(), userId: id }))) - await db - .insert(organization) - .values({ id: orgId, name: 'Members fixture', slug: `members-${orgId}`, createdAt: now }) - await db.insert(member).values( - (['orgOwner', 'admin', 'wsOwner', 'billing', 'writer', 'reader', 'managed'] as const).map( - (name) => ({ - id: generateId(), - userId: users[name], - organizationId: orgId, - role: name === 'orgOwner' ? 'owner' : 'member', - }) - ) - ) - await insertWorkspaceFixture(db, [ - { - id: workspaceId, - name: 'Members fixture', - ownerId: users.wsOwner, - billedAccountUserId: users.billing, - organizationId: orgId, - allowPersonalApiKeys: true, - }, - { - id: lastAdminWorkspaceId, - name: 'Last admin fixture', - ownerId: users.billing, - billedAccountUserId: users.billing, - allowPersonalApiKeys: true, - }, - { - id: unresolvedOwnerWorkspaceId, - name: 'Unresolved owner fixture', - ownerId: users.wsOwner, - billedAccountUserId: users.billing, - allowPersonalApiKeys: true, - }, - ]) - const grant = (target: string, scope: string, role: 'admin' | 'write' | 'read') => ({ +async function seed() { + const { users } = f + const now = new Date() + const userIds = Object.values(users) + await db.insert(user).values( + Object.entries(users).map(([name, id]) => ({ + id, + name: `Fixture ${name}`, + email: `${id}@members.test`, + normalizedEmail: `${id}@members.test`, + emailVerified: true, + createdAt: now, + updatedAt: now, + })) + ) + await db.insert(userStats).values(userIds.map((id) => ({ id: generateId(), userId: id }))) + await db + .insert(organization) + .values({ id: f.orgId, name: 'Members fixture', slug: `members-${f.orgId}`, createdAt: now }) + await db.insert(member).values( + USER_NAMES.filter((name) => name !== 'outsider').map((name) => ({ id: generateId(), - userId: target, - entityType: 'workspace', - entityId: scope, - permissionType: role, - }) - await db - .insert(permissions) - .values([ - grant(users.admin, workspaceId, 'admin'), - grant(users.wsOwner, workspaceId, 'admin'), - grant(users.billing, workspaceId, 'admin'), - grant(users.writer, workspaceId, 'write'), - grant(users.reader, workspaceId, 'read'), - grant(users.managed, workspaceId, 'write'), - grant(users.admin, lastAdminWorkspaceId, 'admin'), - grant(users.wsOwner, unresolvedOwnerWorkspaceId, 'admin'), - grant(users.admin, unresolvedOwnerWorkspaceId, 'admin'), - ]) - await db.insert(apiKey).values([ - ...(Object.entries(personalKeys) as [keyof typeof personalKeys, string][]).map( - ([name, value]) => ({ - id: generateId(), - userId: users[name], - name: 'Members fixture', - key: value, - keyHash: hashApiKey(value), - type: 'personal', - }) - ), - { - id: generateId(), - userId: users.admin, - workspaceId, - createdBy: users.admin, - name: 'Members workspace fixture', - key: workspaceKey, - keyHash: hashApiKey(workspaceKey), - type: 'workspace', - }, + userId: users[name], + organizationId: f.orgId, + role: name === 'orgOwner' ? 'owner' : 'member', + })) + ) + await insertWorkspaceFixture(db, [ + { + id: f.workspaceId, + name: 'Members fixture', + ownerId: users.wsOwner, + billedAccountUserId: users.billing, + organizationId: f.orgId, + allowPersonalApiKeys: true, + }, + { + id: f.lastAdminWorkspaceId, + name: 'Last admin fixture', + ownerId: users.billing, + billedAccountUserId: users.billing, + allowPersonalApiKeys: true, + }, + { + id: f.unresolvedOwnerWorkspaceId, + name: 'Unresolved owner fixture', + ownerId: users.wsOwner, + billedAccountUserId: users.billing, + allowPersonalApiKeys: true, + }, + ]) + const grant = (target: string, scope: string, role: 'admin' | 'write' | 'read') => ({ + id: generateId(), + userId: target, + entityType: 'workspace', + entityId: scope, + permissionType: role, + }) + await db + .insert(permissions) + .values([ + grant(users.admin, f.workspaceId, 'admin'), + grant(users.wsOwner, f.workspaceId, 'admin'), + grant(users.billing, f.workspaceId, 'admin'), + grant(users.writer, f.workspaceId, 'write'), + grant(users.reader, f.workspaceId, 'read'), + grant(users.managed, f.workspaceId, 'write'), + grant(users.admin, f.lastAdminWorkspaceId, 'admin'), + grant(users.wsOwner, f.unresolvedOwnerWorkspaceId, 'admin'), + grant(users.admin, f.unresolvedOwnerWorkspaceId, 'admin'), ]) - await db.insert(oauthClient).values({ - id: oauthClientId, - clientId: oauthClientId, - name: 'Members fixture client', - public: true, - tokenEndpointAuthMethod: 'none', - grantTypes: ['authorization_code', 'refresh_token'], - redirectUris: ['http://127.0.0.1/callback'], - }) - await db.insert(oauthConsent).values({ + await db.insert(apiKey).values([ + ...KEY_HOLDERS.map((name) => ({ id: generateId(), - clientId: oauthClientId, - userId: users.admin, - scopes: ['api:read', 'api:write'], - createdAt: now, - updatedAt: now, - }) - await db.insert(oauthAccessToken).values( - ( - [ - [oauthReadToken, ['api:read']], - [oauthWriteToken, ['api:write']], - ] as const - ).map(([token, scopes]) => ({ - id: generateId(), - token: hashOAuthToken(token.slice('sim_oat_'.length)), - clientId: oauthClientId, - userId: users.admin, - scopes: [...scopes], - createdAt: now, - expiresAt: new Date(now.getTime() + 60 * 60 * 1000), - })) - ) - await db.insert(scimConnection).values({ - id: scimConnectionId, - organizationId: orgId, - status: 'active', - settings: { lockManualMembership: true }, - }) - const managedEmail = `${users.managed}@members.test` - await db.insert(scimUser).values({ + userId: users[name], + name: 'Members fixture', + key: f.keys[name], + keyHash: hashApiKey(f.keys[name]), + type: 'personal', + })), + { id: generateId(), - connectionId: scimConnectionId, - userId: users.managed, - userName: managedEmail, - orderKey: users.managed, - attributes: { - userName: managedEmail, - active: true, - displayName: 'Managed user', - name: { formatted: 'Managed user' }, - emails: [{ value: managedEmail, primary: true, type: 'work' }], - }, - }) - await db.insert(session).values({ + userId: users.admin, + workspaceId: f.workspaceId, + createdBy: users.admin, + name: 'Members workspace fixture', + key: f.workspaceKey, + keyHash: hashApiKey(f.workspaceKey), + type: 'workspace', + }, + ]) + await db.insert(oauthClient).values({ + id: f.oauthClientId, + clientId: f.oauthClientId, + name: 'Members fixture client', + public: true, + tokenEndpointAuthMethod: 'none', + grantTypes: ['authorization_code', 'refresh_token'], + redirectUris: ['http://127.0.0.1/callback'], + }) + await db.insert(oauthConsent).values({ + id: generateId(), + clientId: f.oauthClientId, + userId: users.admin, + scopes: ['api:read', 'api:write'], + createdAt: now, + updatedAt: now, + }) + await db.insert(oauthAccessToken).values( + ( + [ + [f.oauthReadToken, ['api:read']], + [f.oauthWriteToken, ['api:write']], + ] as const + ).map(([token, scopes]) => ({ id: generateId(), - token: generateShortId(32), - userId: users.reader, - expiresAt: new Date(now.getTime() + 24 * 60 * 60 * 1000), + token: hashOAuthToken(token.slice('sim_oat_'.length)), + clientId: f.oauthClientId, + userId: users.admin, + scopes: [...scopes], createdAt: now, - updatedAt: now, - }) + expiresAt: new Date(now.getTime() + 60 * 60 * 1000), + })) + ) + await db.insert(scimConnection).values({ + id: f.scimConnectionId, + organizationId: f.orgId, + status: 'active', + settings: { lockManualMembership: true }, + }) + const managedEmail = `${users.managed}@members.test` + await db.insert(scimUser).values({ + id: generateId(), + connectionId: f.scimConnectionId, + userId: users.managed, + userName: managedEmail, + orderKey: users.managed, + attributes: { + userName: managedEmail, + active: true, + displayName: 'Managed user', + name: { formatted: 'Managed user' }, + emails: [{ value: managedEmail, primary: true, type: 'work' }], + }, + }) + await db.insert(session).values({ + id: generateId(), + token: generateShortId(32), + userId: users.reader, + expiresAt: new Date(now.getTime() + 24 * 60 * 60 * 1000), + createdAt: now, + updatedAt: now, + }) +} + +async function cleanup() { + const userIds = Object.values(f.users) + await db.delete(auditLog).where(inArray(auditLog.workspaceId, f.workspaceIds)) + await db.delete(apiKey).where(inArray(apiKey.userId, userIds)) + await db.delete(oauthClient).where(eq(oauthClient.clientId, f.oauthClientId)) + await db + .delete(permissions) + .where( + and(eq(permissions.entityType, 'workspace'), inArray(permissions.entityId, f.workspaceIds)) + ) + await deleteWorkspaceFixture(db, inArray(workspace.id, f.workspaceIds)) + await db.delete(scimConnection).where(eq(scimConnection.id, f.scimConnectionId)) + await db.delete(member).where(eq(member.organizationId, f.orgId)) + await db.delete(organization).where(eq(organization.id, f.orgId)) + await db.delete(session).where(inArray(session.userId, userIds)) + await db.delete(userStats).where(inArray(userStats.userId, userIds)) + await db.delete(user).where(inArray(user.id, userIds)) +} + +/** Drives the real v2 member route handlers with hashed credentials against PostgreSQL. */ +describe('v2 workspace member management against PostgreSQL', () => { + beforeEach(async () => { + f = newFixture() + await seed() }) + afterEach(cleanup) + afterAll(async () => { - await db.delete(auditLog).where(inArray(auditLog.workspaceId, workspaceIds)) - await db.delete(apiKey).where(inArray(apiKey.userId, userIds)) - await db.delete(oauthClient).where(eq(oauthClient.clientId, oauthClientId)) - await db - .delete(permissions) - .where( - and(eq(permissions.entityType, 'workspace'), inArray(permissions.entityId, workspaceIds)) - ) - await deleteWorkspaceFixture(db, inArray(workspace.id, workspaceIds)) - await db.delete(scimConnection).where(eq(scimConnection.id, scimConnectionId)) - await db.delete(member).where(eq(member.organizationId, orgId)) - await db.delete(organization).where(eq(organization.id, orgId)) - await db.delete(session).where(inArray(session.userId, userIds)) - await db.delete(userStats).where(inArray(userStats.userId, userIds)) - await db.delete(user).where(inArray(user.id, userIds)) await db.$client.end() }) @@ -317,15 +341,18 @@ describe('v2 workspace member management against PostgreSQL', () => { ['PATCH', { role: 'write' }], ['DELETE', undefined], ] as const) { - const response = await call({ apiKey: workspaceKey }, method, users.reader, { body }) + const response = await call({ apiKey: f.workspaceKey }, method, f.users.reader, { body }) expect(response).toMatchObject({ status: 403, body: { - error: { code: 'FORBIDDEN', details: { code: 'WORKSPACE_KEY_OPERATION_NOT_PERMITTED' } }, + error: { + code: 'FORBIDDEN', + details: { code: 'WORKSPACE_KEY_OPERATION_NOT_PERMITTED' }, + }, }, }) } - expect(await roleOf(users.reader)).toBe('read') + expect(await roleOf(f.users.reader)).toBe('read') }) it('refuses an OAuth token without api:write', async () => { @@ -333,13 +360,13 @@ describe('v2 workspace member management against PostgreSQL', () => { ['PATCH', { role: 'write' }], ['DELETE', undefined], ] as const) { - const response = await call({ bearer: oauthReadToken }, method, users.reader, { body }) + const response = await call({ bearer: f.oauthReadToken }, method, f.users.reader, { body }) expect(response).toMatchObject({ status: 403, body: { error: { details: { code: 'INSUFFICIENT_SCOPE' } } }, }) } - expect(await roleOf(users.reader)).toBe('read') + expect(await roleOf(f.users.reader)).toBe('read') }) it('refuses a non-admin changing or removing someone else', async () => { @@ -347,118 +374,157 @@ describe('v2 workspace member management against PostgreSQL', () => { ['PATCH', { role: 'write' }], ['DELETE', undefined], ] as const) { - const response = await call(key('writer'), method, users.reader, { body }) + const response = await call(key('writer'), method, f.users.reader, { body }) expect(response).toMatchObject({ status: 403, body: { error: { details: { code: 'INSUFFICIENT_WORKSPACE_ROLE' } } }, }) } - expect(await roleOf(users.reader)).toBe('read') + expect(await roleOf(f.users.reader)).toBe('read') }) it('answers an unreachable and an unknown workspace with the same 404', async () => { - const unreachable = await call(key('outsider'), 'PATCH', users.reader, { + const unreachable = await call(key('outsider'), 'PATCH', f.users.reader, { body: { role: 'write' }, }) - const unknown = await call(admin, 'PATCH', users.reader, { + const unknown = await call(key('admin'), 'PATCH', f.users.reader, { workspace: generateId(), body: { role: 'write' }, }) expect(unreachable).toEqual(unknown) expect(unreachable).toEqual({ status: 404, + retryAfter: null, body: { error: { code: 'NOT_FOUND', message: 'Workspace not found' } }, }) - expect((await call(key('outsider'), 'DELETE', users.reader)).status).toBe(404) + expect((await call(key('outsider'), 'DELETE', f.users.reader)).status).toBe(404) }) - it('treats a non-member target as a 400 to change and a 404 to remove', async () => { - expect((await call(admin, 'PATCH', users.outsider, { body: { role: 'write' } })).status).toBe( - 400 - ) - expect((await call(admin, 'DELETE', users.outsider)).status).toBe(404) - expect(await roleOf(users.outsider)).toBeNull() + it('answers a non-member target with 404 on both methods and writes nothing', async () => { + const patch = await call(key('admin'), 'PATCH', f.users.outsider, { body: { role: 'write' } }) + expect(patch).toMatchObject({ + status: 404, + body: { error: { code: 'NOT_FOUND', message: 'Member not found' } }, + }) + expect((await call(key('admin'), 'DELETE', f.users.outsider)).status).toBe(404) + expect(await roleOf(f.users.outsider)).toBeNull() + expect(await auditRows('member.role_changed')).toHaveLength(0) }) it('rejects invalid bodies, query strings, and ids before any write', async () => { + const admin = key('admin') const invalid = [ - call(admin, 'PATCH', users.reader, { body: { role: 'owner' } }), - call(admin, 'PATCH', users.reader, { body: { role: 'write', extra: true } }), - call(admin, 'PATCH', users.reader, { body: {} }), - call(admin, 'PATCH', users.reader, { body: { role: 'write' }, query: '?force=1' }), - call(admin, 'DELETE', users.reader, { query: '?force=1' }), + call(admin, 'PATCH', f.users.reader, { body: { role: 'owner' } }), + call(admin, 'PATCH', f.users.reader, { body: { role: 'write', extra: true } }), + call(admin, 'PATCH', f.users.reader, { body: {} }), + call(admin, 'PATCH', f.users.reader, { body: { role: 'write' }, query: '?force=1' }), + call(admin, 'DELETE', f.users.reader, { query: '?force=1' }), call(admin, 'PATCH', 'x'.repeat(200), { body: { role: 'write' } }), ] for (const response of await Promise.all(invalid)) { expect(response).toMatchObject({ status: 400, body: { error: { code: 'BAD_REQUEST' } } }) } - expect(await roleOf(users.reader)).toBe('read') + expect(await roleOf(f.users.reader)).toBe('read') }) it('keeps the SCIM_MANAGED_MEMBERSHIP code for a directory-managed member', async () => { - const response = await call(admin, 'PATCH', users.managed, { body: { role: 'read' } }) + const response = await call(key('admin'), 'PATCH', f.users.managed, { body: { role: 'read' } }) expect(response).toMatchObject({ status: 403, body: { error: { code: 'FORBIDDEN', details: { code: 'SCIM_MANAGED_MEMBERSHIP' } } }, }) - expect(await roleOf(users.managed)).toBe('write') + expect(await roleOf(f.users.managed)).toBe('write') }) + it('answers a role change held up by a competing writer with a retryable 503', async () => { + let lockTaken!: () => void + let release!: () => void + const taken = new Promise((resolve) => { + lockTaken = resolve + }) + const released = new Promise((resolve) => { + release = resolve + }) + // Held from its own async context, so the request is not nested inside this transaction. + const competingWriter = db.transaction(async (tx) => { + await tx + .select({ id: member.id }) + .from(member) + .where(and(eq(member.organizationId, f.orgId), eq(member.userId, f.users.reader))) + .for('update') + lockTaken() + await released + }) + await taken + const response = await call(key('admin'), 'PATCH', f.users.reader, { + body: { role: 'write' }, + }).finally(release) + await competingWriter + expect(response).toMatchObject({ + status: 503, + body: { error: { code: 'SERVICE_UNAVAILABLE' } }, + }) + expect(Number(response.retryAfter)).toBeGreaterThan(0) + expect(await roleOf(f.users.reader)).toBe('read') + }, 30_000) + it('refuses an admin lowering their own role', async () => { - const response = await call(admin, 'PATCH', users.admin, { body: { role: 'write' } }) + const response = await call(key('admin'), 'PATCH', f.users.admin, { body: { role: 'write' } }) expect(response.status).toBe(400) expect(response.body.error.message).toMatch(/own admin/) - expect(await roleOf(users.admin)).toBe('admin') + expect(await roleOf(f.users.admin)).toBe('admin') }) it('keeps Admin for the owner, the billing account, and organization admins', async () => { + const admin = key('admin') const refusals = await Promise.all([ - call(admin, 'PATCH', users.wsOwner, { body: { role: 'read' } }), - call(admin, 'PATCH', users.billing, { body: { role: 'read' } }), - call(admin, 'PATCH', users.orgOwner, { body: { role: 'write' } }), - call(admin, 'DELETE', users.orgOwner), - call(admin, 'DELETE', users.billing), + call(admin, 'PATCH', f.users.wsOwner, { body: { role: 'read' } }), + call(admin, 'PATCH', f.users.billing, { body: { role: 'read' } }), + call(admin, 'PATCH', f.users.orgOwner, { body: { role: 'write' } }), + call(admin, 'PATCH', f.users.orgOwner, { body: { role: 'admin' } }), + call(admin, 'DELETE', f.users.orgOwner), + call(admin, 'DELETE', f.users.billing), ]) - expect(refusals.map((response) => response.status)).toEqual([400, 400, 400, 400, 400]) - expect(await roleOf(users.wsOwner)).toBe('admin') - expect(await roleOf(users.billing)).toBe('admin') + expect(refusals.map((response) => response.status)).toEqual([400, 400, 400, 400, 400, 400]) + expect(await roleOf(f.users.wsOwner)).toBe('admin') + expect(await roleOf(f.users.billing)).toBe('admin') }) it('returns the roster member and audits the key owner as actor', async () => { - const response = await call(admin, 'PATCH', users.reader, { body: { role: 'write' } }) + const response = await call(key('admin'), 'PATCH', f.users.reader, { body: { role: 'write' } }) expect(response.status).toBe(200) expect(Object.keys(response.body)).toEqual(['data']) const roster = await listMembers( - new NextRequest(`http://localhost/api/v2/workspaces/${workspaceId}/members`, { - headers: { 'X-API-Key': personalKeys.admin, 'x-forwarded-for': '127.0.0.1' }, + new NextRequest(`http://localhost/api/v2/workspaces/${f.workspaceId}/members`, { + headers: { 'X-API-Key': f.keys.admin, 'x-forwarded-for': '127.0.0.1' }, }), - { params: Promise.resolve({ workspaceId }) } + { params: Promise.resolve({ workspaceId: f.workspaceId }) } ) const listed = (await roster.json()).data.find( - (entry: { userId: string }) => entry.userId === users.reader + (entry: { userId: string }) => entry.userId === f.users.reader ) expect(response.body.data).toEqual(listed) expect(listed.role).toBe('write') - expect(await roleOf(users.reader)).toBe('write') + expect(await roleOf(f.users.reader)).toBe('write') const [entry] = await awaitAudit('member.role_changed', 1) - expect(entry.actorId).toBe(users.admin) + expect(entry.actorId).toBe(f.users.admin) expect(entry.metadata).toMatchObject({ - targetUserId: users.reader, + targetUserId: f.users.reader, previousRole: 'read', actor: { kind: 'personal_api_key' }, }) }) it('lets an OAuth token with api:write change a role', async () => { - const response = await call({ bearer: oauthWriteToken }, 'PATCH', users.reader, { - body: { role: 'read' }, + const response = await call({ bearer: f.oauthWriteToken }, 'PATCH', f.users.reader, { + body: { role: 'write' }, }) - expect(response).toMatchObject({ status: 200, body: { data: { role: 'read' } } }) - expect(await roleOf(users.reader)).toBe('read') - const rows = await awaitAudit('member.role_changed', 2) - expect(rows[1].actorId).toBe(users.admin) - expect(rows[1].metadata).toMatchObject({ actor: { kind: 'oauth_access_token' } }) + expect(response).toMatchObject({ status: 200, body: { data: { role: 'write' } } }) + expect(await roleOf(f.users.reader)).toBe('write') + const [entry] = await awaitAudit('member.role_changed', 1) + expect(entry.actorId).toBe(f.users.admin) + expect(entry.metadata).toMatchObject({ actor: { kind: 'oauth_access_token' } }) }) it('writes and audits nothing when the requested role is already held', async () => { @@ -466,62 +532,70 @@ describe('v2 workspace member management against PostgreSQL', () => { db .select({ updatedAt: permissions.updatedAt }) .from(permissions) - .where(and(eq(permissions.entityId, workspaceId), eq(permissions.userId, users.reader))) + .where(and(eq(permissions.entityId, f.workspaceId), eq(permissions.userId, f.users.reader))) const [before] = await grantRow() - const response = await call(admin, 'PATCH', users.reader, { body: { role: 'read' } }) + const response = await call(key('admin'), 'PATCH', f.users.reader, { body: { role: 'read' } }) expect(response).toMatchObject({ status: 200, body: { data: { role: 'read' } } }) await new Promise((settle) => setTimeout(settle, 500)) - expect(await auditRows('member.role_changed')).toHaveLength(2) + expect(await auditRows('member.role_changed')).toHaveLength(0) expect(await grantRow()).toEqual([before]) }) it('refuses the last explicit admin leaving', async () => { - const response = await call(admin, 'DELETE', users.admin, { workspace: lastAdminWorkspaceId }) + const response = await call(key('admin'), 'DELETE', f.users.admin, { + workspace: f.lastAdminWorkspaceId, + }) expect(response.status).toBe(400) expect(response.body.error.message).toMatch(/last admin/) - expect(await roleOf(users.admin, lastAdminWorkspaceId)).toBe('admin') + expect(await roleOf(f.users.admin, f.lastAdminWorkspaceId)).toBe('admin') }) it('answers an owner removal with no successor as 409, not 500', async () => { - const response = await call(admin, 'DELETE', users.wsOwner, { - workspace: unresolvedOwnerWorkspaceId, + const response = await call(key('admin'), 'DELETE', f.users.wsOwner, { + workspace: f.unresolvedOwnerWorkspaceId, }) expect(response.status).toBe(409) expect(response.body.error.code).toBe('CONFLICT') expect(response.body.error.message).toMatch(/billing account/) - expect(await roleOf(users.wsOwner, unresolvedOwnerWorkspaceId)).toBe('admin') + expect(await roleOf(f.users.wsOwner, f.unresolvedOwnerWorkspaceId)).toBe('admin') const [row] = await db .select({ ownerId: workspace.ownerId }) .from(workspace) - .where(eq(workspace.id, unresolvedOwnerWorkspaceId)) - expect(row.ownerId).toBe(users.wsOwner) + .where(eq(workspace.id, f.unresolvedOwnerWorkspaceId)) + expect(row.ownerId).toBe(f.users.wsOwner) }) it('removes another member and audits the removal', async () => { - const response = await call(admin, 'DELETE', users.writer) - expect(response).toEqual({ + const response = await call(key('admin'), 'DELETE', f.users.writer) + expect(response).toMatchObject({ status: 200, - body: { data: { userId: users.writer, deleted: true, organizationMembershipRemoved: true } }, + body: { + data: { userId: f.users.writer, deleted: true, organizationMembershipRemoved: true }, + }, }) - expect(await roleOf(users.writer)).toBeNull() - expect(await orgMembership(users.writer)).toHaveLength(0) + expect(await roleOf(f.users.writer)).toBeNull() + expect(await orgMembership(f.users.writer)).toHaveLength(0) const [entry] = await awaitAudit('member.removed', 1) - expect(entry.actorId).toBe(users.admin) - expect(entry.metadata).toMatchObject({ removedUserId: users.writer }) + expect(entry.actorId).toBe(f.users.admin) + expect(entry.metadata).toMatchObject({ removedUserId: f.users.writer }) }) it('lets a member leave with a personal key, ending org membership and sessions', async () => { - const response = await call(key('reader'), 'DELETE', users.reader) - expect(response).toEqual({ + const response = await call(key('reader'), 'DELETE', f.users.reader) + expect(response).toMatchObject({ status: 200, - body: { data: { userId: users.reader, deleted: true, organizationMembershipRemoved: true } }, + body: { + data: { userId: f.users.reader, deleted: true, organizationMembershipRemoved: true }, + }, }) - expect(await roleOf(users.reader)).toBeNull() - expect(await orgMembership(users.reader)).toHaveLength(0) - expect(await db.select().from(session).where(eq(session.userId, users.reader))).toHaveLength(0) - const rows = await awaitAudit('member.removed', 2) - expect(rows[1].actorId).toBe(users.reader) - expect(rows[1].metadata).toMatchObject({ selfRemoval: true }) - expect((await call(key('reader'), 'DELETE', users.reader)).status).toBe(404) + expect(await roleOf(f.users.reader)).toBeNull() + expect(await orgMembership(f.users.reader)).toHaveLength(0) + expect(await db.select().from(session).where(eq(session.userId, f.users.reader))).toHaveLength( + 0 + ) + const [entry] = await awaitAudit('member.removed', 1) + expect(entry.actorId).toBe(f.users.reader) + expect(entry.metadata).toMatchObject({ selfRemoval: true }) + expect((await call(key('reader'), 'DELETE', f.users.reader)).status).toBe(404) }) }) diff --git a/apps/sim/lib/workspaces/api/route-policies.ts b/apps/sim/lib/workspaces/api/route-policies.ts index 02505f26fad..d333d04dc37 100644 --- a/apps/sim/lib/workspaces/api/route-policies.ts +++ b/apps/sim/lib/workspaces/api/route-policies.ts @@ -1,9 +1,11 @@ +import { findCause } from '@sim/utils/errors' import { createInternalResourceConcealmentPolicy, createV2ResourceConcealmentPolicy, internalOrchestrationErrorPolicy, } from '@/lib/api/server/routes' import { isRetryableTransactionError } from '@/lib/db/transaction' +import { WorkspaceBusyError } from '@/lib/workspaces/permissions/management-store' import { WorkspaceBillingAccountRemovalError } from '@/lib/workspaces/utils' import { v2CaughtOrchestrationError, v2Error } from '@/app/api/v2/lib/response' @@ -14,6 +16,10 @@ import { v2CaughtOrchestrationError, v2Error } from '@/app/api/v2/lib/response' * `get-public-workspace` and `list-public-workspace-members` already uses, so * the two responses are byte-identical. */ +function isWorkspaceBusyError(error: unknown): error is WorkspaceBusyError { + return error instanceof WorkspaceBusyError +} + export const v2WorkspaceErrorPolicies = { concealWorkspaceAuthorization: createV2ResourceConcealmentPolicy({ notFoundMessage: 'Workspace not found', @@ -21,14 +27,15 @@ export const v2WorkspaceErrorPolicies = { /** * Member administration adds two actionable failures the shared projection * would answer as 500: an owner removal with no billing account to inherit - * (409) and contention that outlived the transaction (503). + * (409) and contention that outlived the transaction (503, whether the + * removal's driver error or the role store's busy refusal surfaces it). */ manageWorkspaceMembers: createV2ResourceConcealmentPolicy({ notFoundMessage: 'Workspace not found', render(error) { if (error instanceof WorkspaceBillingAccountRemovalError) return v2Error('CONFLICT', error.message) - if (isRetryableTransactionError(error)) + if (isRetryableTransactionError(error) || findCause(error, isWorkspaceBusyError)) return v2Error('SERVICE_UNAVAILABLE', 'The workspace is busy; retry in a moment') return v2CaughtOrchestrationError(error) }, diff --git a/apps/sim/lib/workspaces/application/manage-permissions.test.ts b/apps/sim/lib/workspaces/application/manage-permissions.test.ts index 8c2ed6c2418..681d1b2bd87 100644 --- a/apps/sim/lib/workspaces/application/manage-permissions.test.ts +++ b/apps/sim/lib/workspaces/application/manage-permissions.test.ts @@ -27,8 +27,7 @@ vi.mock('@/lib/workspaces/permissions/management-store', async (importOriginal) reconcileWorkspacePermissionCredentials: hoisted.reconcile, })) -import { ForbiddenOperationError, forbiddenErrorDetails } from '@/lib/core/application/forbidden' -import { HttpError } from '@/lib/core/utils/http-error' +import { forbiddenErrorDetails } from '@/lib/core/application/forbidden' import { COPILOT_APPLICATION_SYSTEM_ERROR_MESSAGE, messageForCopilotApplicationError, @@ -165,25 +164,6 @@ describe('workspace permission operations', () => { } ) - it('keeps a coded refusal that the store wraps in an HTTP error', async () => { - class DirectoryRefusal extends HttpError { - readonly statusCode = 403 - } - const refusal = new ForbiddenOperationError( - 'SCIM_MANAGED_MEMBERSHIP', - 'Your identity provider manages this membership' - ) - mocks.update.mockRejectedValue(new DirectoryRefusal(refusal.message, { cause: refusal })) - const result = await updateWorkspacePermissions - .execute({ principal, input }) - .catch((error: unknown) => error) - expect(forbiddenErrorDetails(result)).toEqual({ code: 'SCIM_MANAGED_MEMBERSHIP' }) - expect(result).toMatchObject({ code: 'forbidden', message: refusal.message }) - // The internal route renders the store's HTTP error, so it must stay the cause. - expect((result as Error).cause).toBeInstanceOf(DirectoryRefusal) - expect(auditMockFns.mockRecordAudit).not.toHaveBeenCalled() - }) - it.each([ new Error('private SQL and credentials'), Object.assign(new Error('forged HTTP error'), { statusCode: 403 }), diff --git a/apps/sim/lib/workspaces/application/manage-permissions.ts b/apps/sim/lib/workspaces/application/manage-permissions.ts index d8d8a660813..4a286d32eea 100644 --- a/apps/sim/lib/workspaces/application/manage-permissions.ts +++ b/apps/sim/lib/workspaces/application/manage-permissions.ts @@ -149,18 +149,11 @@ export const updateWorkspaceMember = defineAuthorizedWorkspaceUseCase({ resolveContext: ({ input }: { input: UpdateWorkspaceMemberInput }) => resolveContext({ input }), authorizationOptions, async execute({ principal, input, context }) { - const before = await getPublicWorkspaceMember(context.workspaceId, input.userId) + const member = await getPublicWorkspaceMember(context.workspaceId, input.userId) + if (!member) throw new OrchestrationError('not_found', 'Member not found') const changes = await applyPermissionUpdates(context.workspaceId, principal, [ { userId: input.userId, permissions: input.role }, ]) - // Granted between the read and the locked write; the store vetted them as a member. - const member = before ?? (await getPublicWorkspaceMember(context.workspaceId, input.userId)) - if (!member) { - throw new OrchestrationError( - 'conflict', - "This member's access just changed. Refresh and try again." - ) - } return { changes, member: { ...member, role: input.role } } }, projectAudit: ({ context, result }) => diff --git a/apps/sim/lib/workspaces/permissions/management-store.ts b/apps/sim/lib/workspaces/permissions/management-store.ts index f2abc5524e6..ee2b035d86b 100644 --- a/apps/sim/lib/workspaces/permissions/management-store.ts +++ b/apps/sim/lib/workspaces/permissions/management-store.ts @@ -78,7 +78,7 @@ class WorkspaceContextChangedError extends HttpError { * reach the client as a generic server error, which is what bounding the lock * wait would otherwise have produced. */ -class WorkspaceBusyError extends HttpError { +export class WorkspaceBusyError extends HttpError { readonly statusCode = 409 constructor() { diff --git a/packages/sim-cli/src/contract/commands.ts b/packages/sim-cli/src/contract/commands.ts index 9b1cd36404c..4c18a788516 100644 --- a/packages/sim-cli/src/contract/commands.ts +++ b/packages/sim-cli/src/contract/commands.ts @@ -1519,6 +1519,9 @@ export const CLI_CONTRACT: CliContract = { }, listWorkspaceMembers: { command: 'workspaces members list', + // `workspaces members` shipped as this list before the group gained update + // and remove, so naming the group still lists. + defaultSubcommand: true, describe: 'List workspace members', profileWorkspacePath: true, // `members update` and `members remove` take the user id, and the listing diff --git a/packages/sim-cli/src/generated/v2-api.ts b/packages/sim-cli/src/generated/v2-api.ts index 24176d780d3..17621517fa6 100644 --- a/packages/sim-cli/src/generated/v2-api.ts +++ b/packages/sim-cli/src/generated/v2-api.ts @@ -23151,7 +23151,7 @@ export const V2_OPERATIONS = { path: '/api/v2/workspaces/[workspaceId]/members/[userId]', pathParams: ['workspaceId', 'userId'] as const, pathParamDocs: { - workspaceId: 'Workspace to retrieve.', + workspaceId: 'Workspace the member belongs to.', userId: 'User identifier of the workspace member, as returned by List workspace members.', }, responseMode: 'json', @@ -25114,7 +25114,7 @@ export const V2_OPERATIONS = { path: '/api/v2/workspaces/[workspaceId]/members/[userId]', pathParams: ['workspaceId', 'userId'] as const, pathParamDocs: { - workspaceId: 'Workspace to retrieve.', + workspaceId: 'Workspace the member belongs to.', userId: 'User identifier of the workspace member, as returned by List workspace members.', }, responseMode: 'json', diff --git a/packages/sim-cli/src/runtime/group-default.test.ts b/packages/sim-cli/src/runtime/group-default.test.ts index 9fa9c58eee4..cd0d0064a73 100644 --- a/packages/sim-cli/src/runtime/group-default.test.ts +++ b/packages/sim-cli/src/runtime/group-default.test.ts @@ -43,6 +43,26 @@ describe('a nested group that defaults to its list', () => { expect(await searchOf(['credentials', 'providers', '--search', 'github'])).toEqual(['github']) }) + it('lists workspace members under the shipped `workspaces members` spelling, options included', async () => { + const pathsAndLimits = async (argv: string[]) => { + const seen: string[] = [] + await runEmbeddedCli(argv, { + ...identity, + transport: async (url) => { + const parsed = new URL(String(url)) + seen.push(`${parsed.pathname}?limit=${parsed.searchParams.get('limit')}`) + return Response.json({ data: [], nextCursor: null }) + }, + }) + return seen + } + const expected = ['/api/v2/workspaces/workspace/members?limit=5'] + expect(await pathsAndLimits(['workspaces', 'members', '--limit', '5'])).toEqual(expected) + expect(await pathsAndLimits(['workspaces', 'members', 'list', '--limit', '5'])).toEqual( + expected + ) + }) + it('leaves a group without a default printing its usage, sending nothing', async () => { const group = await run(['credentials']) expect(group.paths).toEqual([]) From 1b62796c4d04ca4290b7d12dcec95d71c81b8555 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Tue, 10 Nov 2026 13:13:50 -0800 Subject: [PATCH 5/6] fix(api): keep workspace member operations in the operations registry and append the CLI user column --- .../[workspaceId]/members/[userId]/route.ts | 6 +-- .../api/workspaces/[id]/permissions/route.ts | 2 +- .../app/api/workspaces/members/[id]/route.ts | 4 +- .../lib/api/contracts/v2/openapi/resources.ts | 8 ++-- .../application/manage-permissions.ts | 2 +- .../application/member-operations.ts | 42 ------------------- .../lib/workspaces/application/operations.ts | 38 +++++++++++++++++ .../workspaces/application/remove-member.ts | 4 +- packages/sim-cli/src/contract/commands.ts | 5 ++- 9 files changed, 54 insertions(+), 57 deletions(-) delete mode 100644 apps/sim/lib/workspaces/application/member-operations.ts diff --git a/apps/sim/app/api/v2/workspaces/[workspaceId]/members/[userId]/route.ts b/apps/sim/app/api/v2/workspaces/[workspaceId]/members/[userId]/route.ts index 00e4339a2d2..8fd8f472889 100644 --- a/apps/sim/app/api/v2/workspaces/[workspaceId]/members/[userId]/route.ts +++ b/apps/sim/app/api/v2/workspaces/[workspaceId]/members/[userId]/route.ts @@ -7,9 +7,9 @@ import { presentWorkspaceMember } from '@/lib/api/server/workspace-presenters' import { v2WorkspaceErrorPolicies } from '@/lib/workspaces/api/route-policies' import { updateWorkspaceMember } from '@/lib/workspaces/application/manage-permissions' import { - removeWorkspaceMemberOperation, + workspaceMemberOperations, workspacePermissionOperations, -} from '@/lib/workspaces/application/member-operations' +} from '@/lib/workspaces/application/operations' import { removeWorkspaceMember } from '@/lib/workspaces/application/remove-member' /** PATCH /api/v2/workspaces/[workspaceId]/members/[userId] — Change one member's role. */ @@ -27,7 +27,7 @@ export const PATCH = defineV2JsonRoute({ /** DELETE /api/v2/workspaces/[workspaceId]/members/[userId] — Remove a member, or leave. */ export const DELETE = defineV2JsonRoute({ contract: v2RemoveWorkspaceMemberContract, - operation: removeWorkspaceMemberOperation, + operation: workspaceMemberOperations.remove, auth: v2ApiKeyAuth, rateLimit: v2RateLimits.publicApi, errorPolicy: v2WorkspaceErrorPolicies.manageWorkspaceMembers, diff --git a/apps/sim/app/api/workspaces/[id]/permissions/route.ts b/apps/sim/app/api/workspaces/[id]/permissions/route.ts index a8f749ae12e..d165badf853 100644 --- a/apps/sim/app/api/workspaces/[id]/permissions/route.ts +++ b/apps/sim/app/api/workspaces/[id]/permissions/route.ts @@ -23,7 +23,7 @@ import { readWorkspacePermissions, updateWorkspacePermissions, } from '@/lib/workspaces/application/manage-permissions' -import { workspacePermissionOperations } from '@/lib/workspaces/application/member-operations' +import { workspacePermissionOperations } from '@/lib/workspaces/application/operations' import { WorkspacePermissionError } from '@/lib/workspaces/permissions/management-store' function permissionErrorResponse(error: unknown): NextResponse { diff --git a/apps/sim/app/api/workspaces/members/[id]/route.ts b/apps/sim/app/api/workspaces/members/[id]/route.ts index d86f23b39e4..5c0b9cb6cff 100644 --- a/apps/sim/app/api/workspaces/members/[id]/route.ts +++ b/apps/sim/app/api/workspaces/members/[id]/route.ts @@ -11,14 +11,14 @@ import { NoWorkspaceAccessError, } from '@/lib/core/application/workspace-authorization' import { captureServerEvent } from '@/lib/posthog/server' -import { removeWorkspaceMemberOperation } from '@/lib/workspaces/application/member-operations' +import { workspaceMemberOperations } from '@/lib/workspaces/application/operations' import { removeWorkspaceMember } from '@/lib/workspaces/application/remove-member' import { WorkspaceBillingAccountRemovalError } from '@/lib/workspaces/utils' export const DELETE = defineInternalJsonRoute({ contract: removeWorkspaceMemberContract, auth: internalSessionAuth, - operation: removeWorkspaceMemberOperation, + operation: workspaceMemberOperations.remove, rateLimit: internalRateLimits.none({ reason: 'Preserve existing workspace member removal policy.', }), diff --git a/apps/sim/lib/api/contracts/v2/openapi/resources.ts b/apps/sim/lib/api/contracts/v2/openapi/resources.ts index 2652cd824fd..db013686a2c 100644 --- a/apps/sim/lib/api/contracts/v2/openapi/resources.ts +++ b/apps/sim/lib/api/contracts/v2/openapi/resources.ts @@ -108,10 +108,10 @@ import { secretOperations } from '@/lib/secrets/application/operations' import { skillOperations } from '@/lib/skills/application/operations' import { toolExecutionOperations } from '@/lib/tool-execution/application/operations' import { - removeWorkspaceMemberOperation, + workspaceMemberOperations, + workspaceOperations, workspacePermissionOperations, -} from '@/lib/workspaces/application/member-operations' -import { workspaceOperations } from '@/lib/workspaces/application/operations' +} from '@/lib/workspaces/application/operations' const BLOCK_SUMMARY_EXAMPLE = { id: 'slack', @@ -795,7 +795,7 @@ const declaredRoutes = [ defineOpenApiRoute( v2RemoveWorkspaceMemberContract, resourceOperation('Workspaces', { - applicationOperation: removeWorkspaceMemberOperation, + applicationOperation: workspaceMemberOperations.remove, operationId: 'removeWorkspaceMember', summary: 'Remove workspace member', description: `Remove a member’s workspace access, or leave the workspace by naming yourself. Removing someone else requires workspace Admin access; any member may leave. The last Admin cannot leave, organization Admins hold access through their organization role and cannot be removed here, and the workspace billing account cannot be removed until billing is reassigned. Removing the workspace owner transfers ownership to the billing account. When this was the member’s last workspace in its organization, their organization membership and seat are removed too and all of their sessions end, since this endpoint is called with an API key or OAuth token rather than from a signed-in browser session. ${WORKSPACE_API_KEY_DENIED}`, diff --git a/apps/sim/lib/workspaces/application/manage-permissions.ts b/apps/sim/lib/workspaces/application/manage-permissions.ts index 4a286d32eea..2b5f75fb71d 100644 --- a/apps/sim/lib/workspaces/application/manage-permissions.ts +++ b/apps/sim/lib/workspaces/application/manage-permissions.ts @@ -6,7 +6,7 @@ import { defineAuthorizedWorkspaceUseCase, ForbiddenOperationError } from '@/lib import { OrchestrationError, type OrchestrationErrorCode } from '@/lib/core/orchestration/types' import { HttpError } from '@/lib/core/utils/http-error' import { resolveActiveWorkspaceApplicationContext } from '@/lib/workspaces/application/context' -import { workspacePermissionOperations } from '@/lib/workspaces/application/member-operations' +import { workspacePermissionOperations } from '@/lib/workspaces/application/operations' import { type WorkspacePermissionUpdate, workspacePermissionUpdatesSchema, diff --git a/apps/sim/lib/workspaces/application/member-operations.ts b/apps/sim/lib/workspaces/application/member-operations.ts deleted file mode 100644 index 6c339c1ca02..00000000000 --- a/apps/sim/lib/workspaces/application/member-operations.ts +++ /dev/null @@ -1,42 +0,0 @@ -import { defineWorkspaceOperation } from '@/lib/core/application/workspace-operation' - -/** - * Workspace membership administration, kept free of the stores so route - * declarations and the OpenAPI registry can name the operations without - * loading their implementations. - */ -export const workspacePermissionOperations = { - /** permission-group-exempt: the roster describes the workspace the caller belongs to. */ - read: defineWorkspaceOperation({ - id: 'workspaces.permissions.read', - minimumRole: 'read', - workspaceApiKey: 'deny', - principalKinds: ['session', 'delegated'], - delegatedServices: ['copilot'], - capability: 'none', - contentDelivery: 'none', - }), - /** permission-group-exempt: workspace administrators manage existing collaborator roles. */ - update: defineWorkspaceOperation({ - id: 'workspaces.permissions.update', - minimumRole: 'admin', - workspaceApiKey: 'deny', - oauthScope: 'api:write', - principalKinds: ['session', 'personal_api_key', 'oauth_access_token', 'delegated'], - delegatedServices: ['copilot'], - capability: 'none', - contentDelivery: 'none', - }), -} as const - -/** permission-group-exempt: members may leave; workspace administrators may remove existing collaborators. */ -export const removeWorkspaceMemberOperation = defineWorkspaceOperation({ - id: 'workspaces.members.remove', - minimumRole: 'read', - workspaceApiKey: 'deny', - capability: 'none', - contentDelivery: 'none', - oauthScope: 'api:write', - principalKinds: ['session', 'personal_api_key', 'oauth_access_token', 'delegated'], - delegatedServices: ['copilot'], -}) diff --git a/apps/sim/lib/workspaces/application/operations.ts b/apps/sim/lib/workspaces/application/operations.ts index f7d23bb3493..118f382157e 100644 --- a/apps/sim/lib/workspaces/application/operations.ts +++ b/apps/sim/lib/workspaces/application/operations.ts @@ -39,3 +39,41 @@ export const workspaceOperations = { delegatedServices: ['copilot'], }), } as const + +export const workspacePermissionOperations = { + /** permission-group-exempt: the roster describes the workspace the caller belongs to. */ + read: defineWorkspaceOperation({ + id: 'workspaces.permissions.read', + minimumRole: 'read', + workspaceApiKey: 'deny', + principalKinds: ['session', 'delegated'], + delegatedServices: ['copilot'], + capability: 'none', + contentDelivery: 'none', + }), + /** permission-group-exempt: workspace administrators manage existing collaborator roles. */ + update: defineWorkspaceOperation({ + id: 'workspaces.permissions.update', + minimumRole: 'admin', + workspaceApiKey: 'deny', + oauthScope: 'api:write', + principalKinds: ['session', 'personal_api_key', 'oauth_access_token', 'delegated'], + delegatedServices: ['copilot'], + capability: 'none', + contentDelivery: 'none', + }), +} as const + +export const workspaceMemberOperations = { + /** permission-group-exempt: members may leave; workspace administrators may remove existing collaborators. */ + remove: defineWorkspaceOperation({ + id: 'workspaces.members.remove', + minimumRole: 'read', + workspaceApiKey: 'deny', + capability: 'none', + contentDelivery: 'none', + oauthScope: 'api:write', + principalKinds: ['session', 'personal_api_key', 'oauth_access_token', 'delegated'], + delegatedServices: ['copilot'], + }), +} as const diff --git a/apps/sim/lib/workspaces/application/remove-member.ts b/apps/sim/lib/workspaces/application/remove-member.ts index 8a94e698694..b176619cff8 100644 --- a/apps/sim/lib/workspaces/application/remove-member.ts +++ b/apps/sim/lib/workspaces/application/remove-member.ts @@ -3,7 +3,7 @@ import { requirePrincipalSubjectUserId } from '@sim/auth/principal' import { defineAuthorizedWorkspaceUseCase, requireCurrentHumanRole } from '@/lib/core/application' import { OrchestrationError } from '@/lib/core/orchestration/types' import { resolveActiveWorkspaceApplicationContext } from '@/lib/workspaces/application/context' -import { removeWorkspaceMemberOperation } from '@/lib/workspaces/application/member-operations' +import { workspaceMemberOperations } from '@/lib/workspaces/application/operations' import { removeWorkspaceMemberRecord } from '@/lib/workspaces/permissions/member-removal-store' export interface RemoveWorkspaceMemberInput { @@ -12,7 +12,7 @@ export interface RemoveWorkspaceMemberInput { } export const removeWorkspaceMember = defineAuthorizedWorkspaceUseCase({ - operation: removeWorkspaceMemberOperation, + operation: workspaceMemberOperations.remove, resolveContext: ({ input }: { input: RemoveWorkspaceMemberInput }) => resolveActiveWorkspaceApplicationContext(input.workspaceId), authorizationOptions: { delegation: { audience: 'sim:settings', isWithinScope: () => true } }, diff --git a/packages/sim-cli/src/contract/commands.ts b/packages/sim-cli/src/contract/commands.ts index 4c18a788516..831ebd1c710 100644 --- a/packages/sim-cli/src/contract/commands.ts +++ b/packages/sim-cli/src/contract/commands.ts @@ -1525,14 +1525,15 @@ export const CLI_CONTRACT: CliContract = { describe: 'List workspace members', profileWorkspacePath: true, // `members update` and `members remove` take the user id, and the listing - // is where a caller finds it. + // is where a caller finds it. Appended last so the shipped text columns keep + // their positions for scripts that read them by field. columns: [ - { header: 'user', path: 'userId' }, { header: 'email' }, { header: 'name' }, { header: 'role' }, { header: 'external', path: 'isExternal', format: 'bool' }, { header: 'joined', path: 'joinedAt', format: 'timestamp' }, + { header: 'user', path: 'userId' }, ], }, updateWorkspaceMember: { From 84ac1111df404fe191762c34da0cd693c858a65e Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Tue, 10 Nov 2026 13:21:07 -0800 Subject: [PATCH 6/6] fix(docs): keep the shipped workspaces members anchor and classify the member update - Pin the CLI reference heading for `sim workspaces members list` to the `#sim-workspaces-members` anchor it shipped under, through a reference-page counterpart of the existing legacy anchor map, and restore that heading in the docs compatibility baseline. - Classify `updateWorkspaceMember` as non-destructive alongside `updateOrganizationMember`. - Cover an inherited organization admin with no grant row: PATCH answers the documented 400 for every requested role and writes nothing. --- apps/docs/content/docs/cli/reference.mdx | 2 +- .../scripts/fixtures/content-baseline.json | 4 ++-- .../__integration__/members.integration.ts | 23 ++++++++++++++++--- packages/sim-cli/src/http/client.test.ts | 1 + scripts/generate-cli-docs.ts | 13 ++++++++++- 5 files changed, 36 insertions(+), 7 deletions(-) diff --git a/apps/docs/content/docs/cli/reference.mdx b/apps/docs/content/docs/cli/reference.mdx index 8d077ff6c59..121b9b1557a 100644 --- a/apps/docs/content/docs/cli/reference.mdx +++ b/apps/docs/content/docs/cli/reference.mdx @@ -8849,7 +8849,7 @@ sim workspaces fork-resources [options] API request and response schemas: [listWorkspaceForkResources](/api-reference/workspace-sync/listWorkspaceForkResources). -### sim workspaces members list +### sim workspaces members list [#sim-workspaces-members] List workspace members diff --git a/apps/docs/scripts/fixtures/content-baseline.json b/apps/docs/scripts/fixtures/content-baseline.json index ca730d5600a..f6563814165 100644 --- a/apps/docs/scripts/fixtures/content-baseline.json +++ b/apps/docs/scripts/fixtures/content-baseline.json @@ -8555,8 +8555,8 @@ "depth": 3 }, { - "title": "sim workspaces members list", - "url": "#sim-workspaces-members-list", + "title": "sim workspaces members", + "url": "#sim-workspaces-members", "depth": 3 }, { diff --git a/apps/sim/lib/workspaces/__integration__/members.integration.ts b/apps/sim/lib/workspaces/__integration__/members.integration.ts index 47aa1a511e4..749e28e625f 100644 --- a/apps/sim/lib/workspaces/__integration__/members.integration.ts +++ b/apps/sim/lib/workspaces/__integration__/members.integration.ts @@ -480,16 +480,33 @@ describe('v2 workspace member management against PostgreSQL', () => { const refusals = await Promise.all([ call(admin, 'PATCH', f.users.wsOwner, { body: { role: 'read' } }), call(admin, 'PATCH', f.users.billing, { body: { role: 'read' } }), - call(admin, 'PATCH', f.users.orgOwner, { body: { role: 'write' } }), - call(admin, 'PATCH', f.users.orgOwner, { body: { role: 'admin' } }), call(admin, 'DELETE', f.users.orgOwner), call(admin, 'DELETE', f.users.billing), ]) - expect(refusals.map((response) => response.status)).toEqual([400, 400, 400, 400, 400, 400]) + expect(refusals.map((response) => response.status)).toEqual([400, 400, 400, 400]) expect(await roleOf(f.users.wsOwner)).toBe('admin') expect(await roleOf(f.users.billing)).toBe('admin') }) + it('refuses an inherited organization admin, who holds no grant row, whatever role is requested', async () => { + expect(await roleOf(f.users.orgOwner)).toBeNull() + for (const role of ['admin', 'write', 'read'] as const) { + const response = await call(key('admin'), 'PATCH', f.users.orgOwner, { body: { role } }) + expect(response).toEqual({ + status: 400, + retryAfter: null, + body: { + error: { + code: 'BAD_REQUEST', + message: 'Organization admins are workspace admins and their role cannot be changed', + }, + }, + }) + } + expect(await roleOf(f.users.orgOwner)).toBeNull() + expect(await auditRows('member.role_changed')).toHaveLength(0) + }) + it('returns the roster member and audits the key owner as actor', async () => { const response = await call(key('admin'), 'PATCH', f.users.reader, { body: { role: 'write' } }) expect(response.status).toBe(200) diff --git a/packages/sim-cli/src/http/client.test.ts b/packages/sim-cli/src/http/client.test.ts index 97fafa33d86..3d61ffca49f 100644 --- a/packages/sim-cli/src/http/client.test.ts +++ b/packages/sim-cli/src/http/client.test.ts @@ -447,6 +447,7 @@ describe('destructive operations are gated', () => { 'createOrganizationInvitation', 'resendOrganizationInvitation', 'updateOrganizationMember', + 'updateWorkspaceMember', 'createPermissionGroup', 'updatePermissionGroup', 'addPermissionGroupMember', diff --git a/scripts/generate-cli-docs.ts b/scripts/generate-cli-docs.ts index 762bf4ee069..9d442e36519 100644 --- a/scripts/generate-cli-docs.ts +++ b/scripts/generate-cli-docs.ts @@ -466,6 +466,15 @@ const LEGACY_COMMAND_ANCHORS: Record = { 'wait-for-copy-and-deployment-readiness-exit-3-for-configuration-1-for-failure-or-4-for-timeout', } +/** + * Reference-page anchors a renamed command keeps, so links to the spelling it + * shipped under still land on it. `workspaces members` became a group whose + * default subcommand is `list`. + */ +const LEGACY_REFERENCE_ANCHORS: Record = { + 'workspaces members list': 'sim-workspaces-members', +} + const VERB_ORDER = [ 'list', 'ls', @@ -913,7 +922,9 @@ function renderCommand(entry: DocumentedCommand, options: RenderOptions = {}): s const description = entry.command.description() const operation = operationOf(entry.command) const spec = operation ? CLI_CONTRACT[operation] : undefined - const anchor = commandHeadings ? undefined : LEGACY_COMMAND_ANCHORS[entry.path.join(' ')] + const anchor = (commandHeadings ? LEGACY_REFERENCE_ANCHORS : LEGACY_COMMAND_ANCHORS)[ + entry.path.join(' ') + ] const lines = [`${'#'.repeat(level)} ${escapeProse(heading)}${anchor ? ` [#${anchor}]` : ''}`, ''] if (commandHeadings && description) lines.push(escapeProse(sentenceCase(description)), '') if (!commandHeadings) {