Skip to content

Commit b394cd6

Browse files
committed
fix(organizations): preserve member administration compatibility
1 parent d520146 commit b394cd6

10 files changed

Lines changed: 95 additions & 37 deletions

File tree

‎apps/docs/openapi-v2-resources.json‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6376,7 +6376,7 @@
63766376
"post": {
63776377
"operationId": "createOrganizationInvitation",
63786378
"summary": "Create Organization Invitation",
6379-
"description": "Email an invitation to join the organization as a member or administrator. Requires organization administrator access, invitations enabled, and an available seat on an eligible plan. This grants no workspace-specific permissions. A unexpired pending invitation for the email conflicts; use Resend Organization Invitation to send it again. Workspace API keys return `403`; use a personal API key or scoped OAuth token.\n\nOAuth scope: `api:write`.",
6379+
"description": "Email an invitation to join the organization as a member or administrator. Requires organization administrator access, invitations enabled, and an available seat on an eligible plan. This grants no workspace-specific permissions. An unexpired pending invitation for the email conflicts; use Resend Organization Invitation to send it again. Workspace API keys return `403`; use a personal API key or scoped OAuth token.\n\nOAuth scope: `api:write`.",
63806380
"x-sim-operation": "organizations.invitations.create",
63816381
"x-oauth-scope": "api:write",
63826382
"tags": ["Organizations"],
@@ -6539,7 +6539,7 @@
65396539
"delete": {
65406540
"operationId": "revokeOrganizationInvitation",
65416541
"summary": "Revoke Organization Invitation",
6542-
"description": "Cancel a unexpired pending invitation and all its workspace grants so it can no longer be accepted. Requires organization administrator access. This does not remove a person who already accepted; use Remove Organization Member for that. Workspace API keys return `403`; use a personal API key or scoped OAuth token.\n\nOAuth scope: `api:write`.",
6542+
"description": "Cancel an unexpired pending invitation and all its workspace grants so it can no longer be accepted. Requires organization administrator access. This does not remove a person who already accepted; use Remove Organization Member for that. Workspace API keys return `403`; use a personal API key or scoped OAuth token.\n\nOAuth scope: `api:write`.",
65436543
"x-sim-operation": "invitations.revoke",
65446544
"x-oauth-scope": "api:write",
65456545
"tags": ["Organizations"],
@@ -6620,7 +6620,7 @@
66206620
"post": {
66216621
"operationId": "resendOrganizationInvitation",
66226622
"summary": "Resend Organization Invitation",
6623-
"description": "Email a unexpired pending invitation again, renew its expiry, and replace its previous acceptance link. Requires organization administrator access and current invitation eligibility. Retrying sends another email; inspect the invitation after a delivery failure before retrying. Workspace API keys return `403`; use a personal API key or scoped OAuth token.\n\nOAuth scope: `api:write`.",
6623+
"description": "Email an unexpired pending invitation again, renew its expiry, and replace its previous acceptance link. Requires organization administrator access and current invitation eligibility. Retrying sends another email; inspect the invitation after a delivery failure before retrying. Workspace API keys return `403`; use a personal API key or scoped OAuth token.\n\nOAuth scope: `api:write`.",
66246624
"x-sim-operation": "invitations.resend",
66256625
"x-oauth-scope": "api:write",
66266626
"tags": ["Organizations"],
@@ -14842,7 +14842,7 @@
1484214842
},
1484314843
"slug": {
1484414844
"type": "string",
14845-
"description": "Organization slug, or null when unset."
14845+
"description": "Organization slug."
1484614846
},
1484714847
"logo": {
1484814848
"anyOf": [

‎apps/sim/lib/api/contracts/v2/openapi/organizations.ts‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -316,7 +316,7 @@ export const organizationOpenApiRoutes = [
316316
applicationOperation: organizationOperations.createInvitation,
317317
operationId: 'createOrganizationInvitation',
318318
summary: 'Create Organization Invitation',
319-
description: `Email an invitation to join the organization as a member or administrator. Requires organization administrator access, invitations enabled, and an available seat on an eligible plan. This grants no workspace-specific permissions. A unexpired pending invitation for the email conflicts; use Resend Organization Invitation to send it again. ${WORKSPACE_API_KEY_DENIED}`,
319+
description: `Email an invitation to join the organization as a member or administrator. Requires organization administrator access, invitations enabled, and an available seat on an eligible plan. This grants no workspace-specific permissions. An unexpired pending invitation for the email conflicts; use Resend Organization Invitation to send it again. ${WORKSPACE_API_KEY_DENIED}`,
320320
tags: ['Organizations'],
321321
errors: RESOURCE_CONFLICT_ERRORS,
322322
success: {
@@ -410,7 +410,7 @@ export const organizationOpenApiRoutes = [
410410
applicationOperation: invitationOperations.resend,
411411
operationId: 'resendOrganizationInvitation',
412412
summary: 'Resend Organization Invitation',
413-
description: `Email a unexpired pending invitation again, renew its expiry, and replace its previous acceptance link. Requires organization administrator access and current invitation eligibility. Retrying sends another email; inspect the invitation after a delivery failure before retrying. ${WORKSPACE_API_KEY_DENIED}`,
413+
description: `Email an unexpired pending invitation again, renew its expiry, and replace its previous acceptance link. Requires organization administrator access and current invitation eligibility. Retrying sends another email; inspect the invitation after a delivery failure before retrying. ${WORKSPACE_API_KEY_DENIED}`,
414414
tags: ['Organizations'],
415415
errors: RESOURCE_CONFLICT_ERRORS,
416416
success: {
@@ -462,7 +462,7 @@ export const organizationOpenApiRoutes = [
462462
applicationOperation: invitationOperations.revoke,
463463
operationId: 'revokeOrganizationInvitation',
464464
summary: 'Revoke Organization Invitation',
465-
description: `Cancel a unexpired pending invitation and all its workspace grants so it can no longer be accepted. Requires organization administrator access. This does not remove a person who already accepted; use Remove Organization Member for that. ${WORKSPACE_API_KEY_DENIED}`,
465+
description: `Cancel an unexpired pending invitation and all its workspace grants so it can no longer be accepted. Requires organization administrator access. This does not remove a person who already accepted; use Remove Organization Member for that. ${WORKSPACE_API_KEY_DENIED}`,
466466
tags: ['Organizations'],
467467
errors: RESOURCE_CONFLICT_ERRORS,
468468
success: {

‎apps/sim/lib/api/contracts/v2/organizations.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ export const v2OrganizationSchema = z
3434
.object({
3535
id: z.string().describe('Organization identifier.'),
3636
name: z.string().describe('Organization display name.'),
37-
slug: z.string().describe('Organization slug, or null when unset.'),
37+
slug: z.string().describe('Organization slug.'),
3838
logo: z.string().nullable().describe('Organization logo URL, or null when unset.'),
3939
role: organizationRoleSchema.describe('The acting user’s role in this organization.'),
4040
createdAt: v2TimestampSchema.describe('When the organization was created.'),

‎apps/sim/lib/api/mcp/generated/v2-operations.ts‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -653,7 +653,7 @@ export const V2_MCP_OPERATIONS = {
653653
contract: v2CreateOrganizationInvitationContract,
654654
summary: 'Create Organization Invitation',
655655
description:
656-
'Email an invitation to join the organization as a member or administrator. Requires organization administrator access, invitations enabled, and an available seat on an eligible plan. This grants no workspace-specific permissions. A unexpired pending invitation for the email conflicts; use Resend Organization Invitation to send it again. Workspace API keys return `403`; use a personal API key or scoped OAuth token.\n\nOAuth scope: `api:write`.',
656+
'Email an invitation to join the organization as a member or administrator. Requires organization administrator access, invitations enabled, and an available seat on an eligible plan. This grants no workspace-specific permissions. An unexpired pending invitation for the email conflicts; use Resend Organization Invitation to send it again. Workspace API keys return `403`; use a personal API key or scoped OAuth token.\n\nOAuth scope: `api:write`.',
657657
workspaceKeyUnsupported: true,
658658
handler: () =>
659659
import('@/app/api/v2/organizations/[organizationId]/invitations/route').then(
@@ -2070,7 +2070,7 @@ export const V2_MCP_OPERATIONS = {
20702070
contract: v2ResendOrganizationInvitationContract,
20712071
summary: 'Resend Organization Invitation',
20722072
description:
2073-
'Email a unexpired pending invitation again, renew its expiry, and replace its previous acceptance link. Requires organization administrator access and current invitation eligibility. Retrying sends another email; inspect the invitation after a delivery failure before retrying. Workspace API keys return `403`; use a personal API key or scoped OAuth token.\n\nOAuth scope: `api:write`.',
2073+
'Email an unexpired pending invitation again, renew its expiry, and replace its previous acceptance link. Requires organization administrator access and current invitation eligibility. Retrying sends another email; inspect the invitation after a delivery failure before retrying. Workspace API keys return `403`; use a personal API key or scoped OAuth token.\n\nOAuth scope: `api:write`.',
20742074
workspaceKeyUnsupported: true,
20752075
handler: () =>
20762076
import(
@@ -2157,7 +2157,7 @@ export const V2_MCP_OPERATIONS = {
21572157
contract: v2RevokeOrganizationInvitationContract,
21582158
summary: 'Revoke Organization Invitation',
21592159
description:
2160-
'Cancel a unexpired pending invitation and all its workspace grants so it can no longer be accepted. Requires organization administrator access. This does not remove a person who already accepted; use Remove Organization Member for that. Workspace API keys return `403`; use a personal API key or scoped OAuth token.\n\nOAuth scope: `api:write`.',
2160+
'Cancel an unexpired pending invitation and all its workspace grants so it can no longer be accepted. Requires organization administrator access. This does not remove a person who already accepted; use Remove Organization Member for that. Workspace API keys return `403`; use a personal API key or scoped OAuth token.\n\nOAuth scope: `api:write`.',
21612161
workspaceKeyUnsupported: true,
21622162
handler: () =>
21632163
import('@/app/api/v2/organizations/[organizationId]/invitations/[invitationId]/route').then(

‎apps/sim/lib/billing/organizations/membership-external-removal.test.ts‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
import { credential, knowledgeBase, member, workspaceFiles } from '@sim/db/schema'
55
import { dbChainMockFns, hasMockCondition, queueTableRows, resetDbChainMock } from '@sim/testing'
66
import { beforeEach, describe, expect, it, vi } from 'vitest'
7+
import { OrchestrationError } from '@/lib/core/orchestration/types'
78

89
const { mockSetOrgMemberUsageLimit } = vi.hoisted(() => ({
910
mockSetOrgMemberUsageLimit: vi.fn(),
@@ -102,11 +103,34 @@ describe('external organization access removal', () => {
102103
organizationId: 'org-1',
103104
userId: 'target',
104105
memberId: 'membership',
106+
onError: 'throw',
105107
})
106108
).rejects.toBe(failure)
107109
}
108110
)
109111

112+
it.each([
113+
Object.assign(new Error('retry transaction'), { code: '40001' }),
114+
Object.assign(new Error('retry transaction'), { code: '40P01' }),
115+
Object.assign(new Error('retry transaction'), { code: '55P03' }),
116+
new OrchestrationError('conflict', 'The membership changed before removal'),
117+
])('preserves failure results for legacy compound callers on $code', async (failure) => {
118+
queueTableRows(member, [{ id: 'membership', userId: 'target', role: 'member' }])
119+
dbChainMockFns.transaction.mockRejectedValueOnce(failure)
120+
121+
await expect(
122+
removeUserFromOrganization({
123+
organizationId: 'org-1',
124+
userId: 'target',
125+
memberId: 'membership',
126+
})
127+
).resolves.toMatchObject({
128+
success: false,
129+
error: 'Failed to remove user from organization',
130+
})
131+
expect(dbChainMockFns.delete).not.toHaveBeenCalled()
132+
})
133+
110134
it('rejects an actor demoted before the locked removal', async () => {
111135
queueTableRows(member, [{ id: 'membership', userId: 'target', role: 'member' }])
112136
queueTableRows(member, [{ id: 'actor-membership', role: 'member' }])
@@ -116,6 +140,7 @@ describe('external organization access removal', () => {
116140
userId: 'target',
117141
memberId: 'membership',
118142
actorUserId: 'actor',
143+
onError: 'throw',
119144
})
120145
).rejects.toMatchObject({ code: 'forbidden' })
121146
expect(dbChainMockFns.delete).not.toHaveBeenCalled()

‎apps/sim/lib/billing/organizations/membership.ts‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -488,6 +488,8 @@ export interface RemoveMemberParams {
488488
spareSessionId?: string
489489
/** Acting member whose management authority is rechecked under the mutation lock. */
490490
actorUserId?: string
491+
/** Legacy compound callers consume failure results; application use cases propagate errors. */
492+
onError?: 'return-failure' | 'throw'
491493
/**
492494
* Only remove the member when they hold no remaining permission on any of the
493495
* org's workspaces, evaluated atomically under the membership lock. Used by
@@ -1288,6 +1290,7 @@ export async function removeUserFromOrganization(
12881290
spareSessionToken,
12891291
spareSessionId,
12901292
actorUserId,
1293+
onError = 'return-failure',
12911294
} = params
12921295

12931296
const billingActions = {
@@ -1529,10 +1532,10 @@ export async function removeUserFromOrganization(
15291532

15301533
return { success: true, removed: true, billingActions }
15311534
} catch (error) {
1532-
if (error instanceof OrchestrationError || isRetryableTransactionError(error)) throw error
15331535
if (error instanceof WorkspaceBillingAccountRemovalError) {
15341536
return { success: false, error: error.message, billingActions }
15351537
}
1538+
if (onError === 'throw') throw error
15361539

15371540
logger.error('Failed to remove user from organization', {
15381541
userId,

‎apps/sim/lib/organizations/application/members.ts‎

Lines changed: 12 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -20,23 +20,18 @@ export const updateOrganizationMember = defineAuthorizedOrganizationUseCase({
2020
input: { organizationId: string; userId: string; role: 'member' | 'admin' | 'owner' }
2121
context: { userId: string }
2222
}) => updateOrganizationMemberRecord({ ...input, actorUserId: context.userId }),
23-
projectAudit: ({ input, result }) =>
24-
result.changed
25-
? [
26-
{
27-
action: AuditAction.ORG_MEMBER_ROLE_CHANGED,
28-
resourceType: AuditResourceType.ORGANIZATION,
29-
resourceId: input.organizationId,
30-
description: `Changed role for member ${input.userId} to ${input.role}`,
31-
metadata: {
32-
targetUserId: input.userId,
33-
targetEmail: result.member.userEmail,
34-
targetName: result.member.userName,
35-
changes: [{ field: 'role', from: result.previousRole, to: input.role }],
36-
},
37-
},
38-
]
39-
: [],
23+
projectAudit: ({ input, result }) => ({
24+
action: AuditAction.ORG_MEMBER_ROLE_CHANGED,
25+
resourceType: AuditResourceType.ORGANIZATION,
26+
resourceId: input.organizationId,
27+
description: `Changed role for member ${input.userId} to ${input.role}`,
28+
metadata: {
29+
targetUserId: input.userId,
30+
targetEmail: result.member.userEmail,
31+
targetName: result.member.userName,
32+
changes: [{ field: 'role', from: result.previousRole, to: input.role }],
33+
},
34+
}),
4035
})
4136

4237
export const removeOrganizationMember = defineAuthorizedOrganizationUseCase({

‎apps/sim/lib/organizations/application/use-cases.test.ts‎

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -161,10 +161,26 @@ describe('organization application operations', () => {
161161
expect(dbChainMockFns.select).not.toHaveBeenCalled()
162162
})
163163

164-
it('does not audit an unchanged role or failed write', async () => {
164+
it('preserves audit on a successful same-role request', async () => {
165165
queueTableRows(member, [{ role: 'admin' }])
166-
mocks.update.mockResolvedValueOnce({ member: target, changed: false })
166+
mocks.update.mockResolvedValueOnce({
167+
member: { ...target, role: 'admin' },
168+
previousRole: 'admin',
169+
changed: false,
170+
})
167171
await updateOrganizationMember.execute({ principal: session, input: roleInput })
172+
expect(recordAudit).toHaveBeenCalledExactlyOnceWith(
173+
expect.objectContaining({
174+
actorId: 'actor',
175+
resourceId: 'org',
176+
metadata: expect.objectContaining({
177+
changes: [{ field: 'role', from: 'admin', to: 'admin' }],
178+
}),
179+
})
180+
)
181+
})
182+
183+
it('does not audit a failed role write', async () => {
168184
queueTableRows(member, [{ role: 'admin' }])
169185
mocks.update.mockRejectedValueOnce(new Error('database unavailable'))
170186
await expect(

‎apps/sim/lib/organizations/member-manager.test.ts‎

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/** @vitest-environment node */
2-
import { member } from '@sim/db/schema'
2+
import { member, user } from '@sim/db/schema'
33
import { dbChainMockFns, queueTableRows, resetDbChainMock } from '@sim/testing'
44
import { beforeEach, describe, expect, it, vi } from 'vitest'
55

@@ -96,8 +96,21 @@ describe('organization member managers', () => {
9696
seatReduction: { changed: false },
9797
})
9898
expect(mocks.remove).toHaveBeenCalledWith(
99-
expect.objectContaining({ actorUserId: 'actor', memberId: 'membership' })
99+
expect.objectContaining({ actorUserId: 'actor', memberId: 'membership', onError: 'throw' })
100100
)
101101
expect(mocks.scim).not.toHaveBeenCalled()
102102
})
103+
104+
it('reports membership added before external-removal preflight as a conflict', async () => {
105+
queueTableRows(member, [])
106+
queueTableRows(user, [{ id: 'target', name: 'Person', email: 'person@example.com' }])
107+
mocks.external.mockResolvedValue({ success: false, error: 'User is an organization member' })
108+
109+
await expect(removeOrganizationMemberRecord(input)).rejects.toMatchObject({
110+
code: 'conflict',
111+
message: 'User is an organization member',
112+
})
113+
expect(mocks.remove).not.toHaveBeenCalled()
114+
expect(mocks.seats).not.toHaveBeenCalled()
115+
})
103116
})

‎apps/sim/lib/organizations/member-manager.ts‎

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -72,9 +72,11 @@ export async function removeOrganizationMemberRecord(input: {
7272
const code =
7373
message === 'External workspace member not found'
7474
? 'not_found'
75-
: message === WORKSPACE_BILLING_ACCOUNT_REMOVAL_ERROR
76-
? 'validation'
77-
: 'internal'
75+
: message === 'User is an organization member'
76+
? 'conflict'
77+
: message === WORKSPACE_BILLING_ACCOUNT_REMOVAL_ERROR
78+
? 'validation'
79+
: 'internal'
7880
throw new OrchestrationError(code, message)
7981
}
8082
return {
@@ -84,7 +86,11 @@ export async function removeOrganizationMemberRecord(input: {
8486
seatReduction: null,
8587
}
8688
}
87-
const removal = await removeUserFromOrganization({ ...input, memberId: target.id })
89+
const removal = await removeUserFromOrganization({
90+
...input,
91+
memberId: target.id,
92+
onError: 'throw',
93+
})
8894
if (!removal.success) {
8995
const message = removal.error || 'Failed to remove user from organization'
9096
const code =

0 commit comments

Comments
 (0)