Skip to content

Commit 29ab3d0

Browse files
committed
fix(admin): gate whitelabel on entitlement and emit detach audits post-commit
The Admin whitelabel PATCH skipped the entitlement check the settings UI runs, so an admin key could set branding the product had not granted the org. detachOrganizationWorkspacesTx also wrote its audit rows inside the caller's transaction, contradicting its own doc comment — a rolled-back delete would have left audit history describing detachments that never happened. It now returns the rows and callers emit them after commit.
1 parent 7d55f38 commit 29ab3d0

4 files changed

Lines changed: 45 additions & 19 deletions

File tree

apps/sim/app/api/v1/admin/organizations/[id]/route.test.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ vi.mock('@/app/api/v1/admin/auth', () => ({
2222

2323
vi.mock('@sim/audit', () => ({
2424
recordAudit: vi.fn(),
25+
recordAuditBatch: vi.fn(),
2526
AuditAction: {
2627
ORGANIZATION_UPDATED: 'organization.updated',
2728
ORGANIZATION_DELETED: 'organization.deleted',
@@ -57,6 +58,8 @@ describe('admin organization DELETE', () => {
5758
mockDetachOrganizationWorkspacesTx.mockResolvedValue({
5859
detachedWorkspaceIds: ['ws-1', 'ws-2'],
5960
billedAccountUserId: 'user-1',
61+
/** Returned rather than written, so the caller can emit them post-commit. */
62+
auditEntries: [],
6063
})
6164
mockDelete.mockClear()
6265
})

apps/sim/app/api/v1/admin/organizations/[id]/route.ts

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@
3030
* Response: AdminSingleResponse<{ success, organizationId, slug, membersRemoved, workspacesDetached }>
3131
*/
3232

33-
import { AuditAction, AuditResourceType, recordAudit } from '@sim/audit'
33+
import { AuditAction, AuditResourceType, recordAudit, recordAuditBatch } from '@sim/audit'
3434
import { db } from '@sim/db'
3535
import { member, organization, subscription } from '@sim/db/schema'
3636
import { createLogger } from '@sim/logger'
@@ -292,12 +292,14 @@ export const DELETE = withRouteHandler(
292292
* leave workspaces re-billed to their owners while the organization,
293293
* its members, and its settings survived a failed delete.
294294
*/
295-
const { detachedWorkspaceIds } = await db.transaction(async (tx) => {
295+
const { detachedWorkspaceIds, auditEntries } = await db.transaction(async (tx) => {
296296
const detached = await detachOrganizationWorkspacesTx(tx, organizationId)
297297
await tx.delete(organization).where(eq(organization.id, organizationId))
298298
return detached
299299
})
300300

301+
recordAuditBatch(auditEntries)
302+
301303
logger.info(`Admin API: Deleted organization ${organizationId}`, {
302304
slug: existing.slug,
303305
membersRemoved: memberCountRow?.value ?? 0,

apps/sim/app/api/v1/admin/organizations/[id]/whitelabel/route.ts

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,12 +18,15 @@ import { createLogger } from '@sim/logger'
1818
import { eq } from 'drizzle-orm'
1919
import { adminV1UpdateOrganizationWhitelabelContract } from '@/lib/api/contracts/v1/admin'
2020
import { parseRequest } from '@/lib/api/server'
21+
import { isOrganizationFeatureEntitled } from '@/lib/billing/core/subscription'
2122
import type { OrganizationWhitelabelSettings } from '@/lib/branding/types'
23+
import { isBillingEnabled, isWhitelabelingEnabled } from '@/lib/core/config/env-flags'
2224
import { withRouteHandler } from '@/lib/core/utils/with-route-handler'
2325
import { withAdminAuthParams } from '@/app/api/v1/admin/middleware'
2426
import {
2527
adminInvalidJsonResponse,
2628
adminValidationErrorResponse,
29+
forbiddenResponse,
2730
internalErrorResponse,
2831
notFoundResponse,
2932
singleResponse,
@@ -62,6 +65,20 @@ export const PATCH = withRouteHandler(
6265
return notFoundResponse('Organization')
6366
}
6467

68+
/**
69+
* Same entitlement gate the settings UI applies. An admin key
70+
* authenticates an operator, not an entitlement, so without this it would
71+
* be a way to set branding the product has not granted this organization.
72+
*/
73+
const entitled = await isOrganizationFeatureEntitled(organizationId, isWhitelabelingEnabled)
74+
if (!entitled) {
75+
return forbiddenResponse(
76+
isBillingEnabled
77+
? 'Whitelabeling is available on Enterprise plans only'
78+
: 'Whitelabeling is disabled. Set ENTERPRISE_ENABLED or WHITELABELING_ENABLED to enable it.'
79+
)
80+
}
81+
6582
const merged: OrganizationWhitelabelSettings = { ...(existing.whitelabelSettings ?? {}) }
6683
for (const key of Object.keys(incoming) as Array<keyof typeof incoming>) {
6784
const value = incoming[key]

apps/sim/lib/workspaces/organization-workspaces.ts

Lines changed: 21 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,8 @@ export interface AttachOwnedWorkspacesToOrganizationTxResult
3232
export interface DetachOrganizationWorkspacesResult {
3333
detachedWorkspaceIds: string[]
3434
billedAccountUserId: string | null
35+
/** Emit with `recordAuditBatch` once the surrounding transaction has committed. */
36+
auditEntries: Parameters<typeof recordAuditBatch>[0]
3537
}
3638

3739
export class WorkspaceOrganizationMembershipConflictError extends Error {
@@ -336,7 +338,9 @@ export async function attachOwnedWorkspacesToOrganizationTx(
336338
export async function detachOrganizationWorkspaces(
337339
organizationId: string
338340
): Promise<DetachOrganizationWorkspacesResult> {
339-
return db.transaction((tx) => detachOrganizationWorkspacesTx(tx, organizationId))
341+
const result = await db.transaction((tx) => detachOrganizationWorkspacesTx(tx, organizationId))
342+
recordAuditBatch(result.auditEntries)
343+
return result
340344
}
341345

342346
/**
@@ -345,8 +349,10 @@ export async function detachOrganizationWorkspaces(
345349
* detach that committed on its own would leave workspaces re-billed while the
346350
* organization it was meant to empty still exists.
347351
*
348-
* Audit rows are emitted by the caller after commit; `recordAuditBatch` is
349-
* fire-and-forget and must not describe a transaction that may still roll back.
352+
* Returns its audit rows in `auditEntries` rather than writing them. Callers
353+
* pass them to `recordAuditBatch` only after their transaction commits: the
354+
* write is fire-and-forget, so emitting it here would leave audit history
355+
* describing detachments that a later rollback undid.
350356
*/
351357
export async function detachOrganizationWorkspacesTx(
352358
tx: DbOrTx,
@@ -427,14 +433,23 @@ export async function detachOrganizationWorkspacesTx(
427433
return [...workspaceIds].sort()
428434
})()
429435

436+
logger.info('Detached organization workspaces', {
437+
organizationId,
438+
detachedWorkspaceCount: detachedWorkspaceIds.length,
439+
billedAccountUserId: organizationOwnerId,
440+
})
441+
430442
const workspacesById = new Map(
431443
organizationWorkspaces.map((organizationWorkspace) => [
432444
organizationWorkspace.id,
433445
organizationWorkspace,
434446
])
435447
)
436-
recordAuditBatch(
437-
detachedWorkspaceIds.map((detachedWorkspaceId) => {
448+
449+
return {
450+
detachedWorkspaceIds,
451+
billedAccountUserId: organizationOwnerId,
452+
auditEntries: detachedWorkspaceIds.map((detachedWorkspaceId) => {
438453
const detachedWorkspace = workspacesById.get(detachedWorkspaceId)
439454
return {
440455
workspaceId: detachedWorkspaceId,
@@ -450,17 +465,6 @@ export async function detachOrganizationWorkspacesTx(
450465
newBilledAccountUserId: organizationOwnerId ?? detachedWorkspace?.ownerId ?? null,
451466
},
452467
}
453-
})
454-
)
455-
456-
logger.info('Detached organization workspaces', {
457-
organizationId,
458-
detachedWorkspaceCount: detachedWorkspaceIds.length,
459-
billedAccountUserId: organizationOwnerId,
460-
})
461-
462-
return {
463-
detachedWorkspaceIds,
464-
billedAccountUserId: organizationOwnerId,
468+
}),
465469
}
466470
}

0 commit comments

Comments
 (0)