Skip to content

Commit 4147323

Browse files
committed
improvement(chat): let Chat review organization access requests
Organization admins can list, preview, approve and deny access requests from an organization conversation through the settings tool's new requests section. The admin list, preview and resolve operations now admit organization-delegated Copilot principals under the sim:settings audience; role, audit, outbox notice and fingerprint checks are unchanged. Request settings stay session-only.
1 parent 133c671 commit 4147323

11 files changed

Lines changed: 424 additions & 29 deletions

File tree

‎apps/sim/ee/access-requests/lib/application/authorization.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@ import { authorizeOrganizationOperation } from '@/lib/core/application/organizat
1212
import {
1313
authorizeWorkspaceOperation,
1414
PrincipalKindAuthorizationError,
15-
requireAllowedWorkspacePrincipal,
1615
} from '@/lib/core/application/workspace-authorization'
1716
import { OrchestrationError } from '@/lib/core/orchestration/types'
1817
import type { DbOrTx } from '@/lib/db/types'
@@ -21,6 +20,7 @@ import {
2120
type AccessRequestOperation,
2221
type AccessRequestPrincipal,
2322
} from '@/ee/access-requests/lib/application/operations'
23+
import { requireAccessRequestPrincipal } from '@/ee/access-requests/lib/application/principal'
2424
import type { AccessRequestScope } from '@/ee/access-requests/lib/targets'
2525

2626
export interface AccessRequestContext {
@@ -94,7 +94,7 @@ export async function authorizeAccessRequestScope(
9494
forUpdate = false,
9595
expected?: Pick<AccessRequestContext, 'organizationId' | 'membershipId'>
9696
): Promise<AccessRequestContext> {
97-
requireAllowedWorkspacePrincipal(principal, operation)
97+
requireAccessRequestPrincipal(principal, operation)
9898
if (operation.admin && scope.kind !== 'organization') {
9999
throw new OrchestrationError('forbidden', 'Organization administrator access is required')
100100
}

‎apps/sim/ee/access-requests/lib/application/authorized-use-case.ts‎

Lines changed: 2 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,11 @@
1-
import { type Principal, requirePrincipalSubjectUserId } from '@sim/auth/principal'
1+
import { requirePrincipalSubjectUserId } from '@sim/auth/principal'
22
import { db } from '@sim/db'
33
import { acquireOrganizationMutationLock } from '@/lib/billing/organizations/membership'
44
import {
55
recordProjectedUseCaseAuditEntries,
66
type WorkspaceUseCaseAuditEntry,
77
} from '@/lib/core/application/authorized-workspace-use-case'
88
import type { OperationUseCase } from '@/lib/core/application/operation'
9-
import { requireAllowedWorkspacePrincipal } from '@/lib/core/application/workspace-authorization'
109
import { runWithOutboundOrganization } from '@/lib/core/network/context.server'
1110
import type { DbClient, DbOrTx, DbTransaction } from '@/lib/db/types'
1211
import {
@@ -18,6 +17,7 @@ import {
1817
type AccessRequestOperation,
1918
type AccessRequestPrincipal,
2019
} from '@/ee/access-requests/lib/application/operations'
20+
import { requireAccessRequestPrincipal } from '@/ee/access-requests/lib/application/principal'
2121
import type { AccessRequestScope } from '@/ee/access-requests/lib/targets'
2222

2323
interface AccessRequestPreparationArgs<I> {
@@ -64,13 +64,6 @@ type ReadAccessRequestUseCase<I, R, P> = AccessRequestUseCase<I, R, P, DbClient>
6464
mutation?: false
6565
}
6666

67-
function requireAccessRequestPrincipal(
68-
principal: Principal,
69-
operation: AccessRequestOperation
70-
): asserts principal is AccessRequestPrincipal {
71-
requireAllowedWorkspacePrincipal(principal, operation)
72-
}
73-
7467
/** Runs preparation before any transaction opens and binds its result to `execute`. */
7568
async function prepareExecution<I, R, P, E extends DbOrTx>(
7669
definition: AccessRequestUseCase<I, R, P, E>,
Lines changed: 275 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,275 @@
1+
/** Organization Chat reviewing access requests through the generic settings tool. */
2+
3+
import { db } from '@sim/db'
4+
import {
5+
auditLog,
6+
copilotChats,
7+
member,
8+
organization,
9+
outboxEvent,
10+
permissionAccessRequest,
11+
permissionGroup,
12+
permissionGroupWorkspace,
13+
permissions,
14+
user,
15+
workspace,
16+
} from '@sim/db/schema'
17+
import { deleteWorkspaceFixture, insertWorkspaceFixture } from '@sim/db/testing/workspace-fixtures'
18+
import { envFlagsMock } from '@sim/testing/mocks/env-flags.mock'
19+
import { generateId } from '@sim/utils/id'
20+
import { and, eq, inArray, sql } from 'drizzle-orm'
21+
import { afterAll, beforeAll, describe, expect, it, vi } from 'vitest'
22+
23+
vi.mock('@/lib/core/config/env-flags', () => ({ ...envFlagsMock, isAccessControlEnabled: true }))
24+
25+
import { settingsServerTool } from '@/lib/mothership/tools/server/settings'
26+
import { createAccessRequest } from '@/ee/access-requests/lib/application/requests'
27+
import { PERMISSION_ACCESS_REQUEST_DECIDED_EVENT } from '@/ee/access-requests/lib/notification-events'
28+
29+
const organizationId = generateId()
30+
const workspaceId = generateId()
31+
const adminId = generateId()
32+
const memberId = generateId()
33+
const requesterId = generateId()
34+
const groupId = generateId()
35+
const chats = new Map<string, string>()
36+
const requestIds: Record<'tables' | 'files', string> = { tables: '', files: '' }
37+
38+
function chatContext(userId: string) {
39+
const chatId = chats.get(userId)
40+
if (!chatId) throw new Error('Fixture chat missing')
41+
return {
42+
userId,
43+
organizationId,
44+
chatId,
45+
toolCallId: generateId(),
46+
requestMode: 'agent',
47+
copilotToolExecution: true,
48+
}
49+
}
50+
51+
function asChat(userId: string) {
52+
return (operation: string, input: Record<string, unknown>) =>
53+
settingsServerTool.execute(
54+
{ scope: 'organization', action: 'execute', section: 'requests', operation, input },
55+
chatContext(userId)
56+
) as Promise<{ result: Record<string, unknown> }>
57+
}
58+
59+
function readRequest(id: string) {
60+
return db
61+
.select({
62+
status: permissionAccessRequest.status,
63+
decidedBy: permissionAccessRequest.decidedBy,
64+
decisionReason: permissionAccessRequest.decisionReason,
65+
})
66+
.from(permissionAccessRequest)
67+
.where(eq(permissionAccessRequest.id, id))
68+
.then(([row]) => row)
69+
}
70+
71+
async function readGroupConfig() {
72+
const [group] = await db
73+
.select({ config: permissionGroup.config })
74+
.from(permissionGroup)
75+
.where(eq(permissionGroup.id, groupId))
76+
return group.config
77+
}
78+
79+
function decidedEvents(id: string) {
80+
return db
81+
.select({ id: outboxEvent.id })
82+
.from(outboxEvent)
83+
.where(
84+
and(
85+
eq(outboxEvent.eventType, PERMISSION_ACCESS_REQUEST_DECIDED_EVENT),
86+
sql`${outboxEvent.payload}->>'requestId' = ${id}`
87+
)
88+
)
89+
}
90+
91+
async function waitForAudit(resourceId: string, action: string) {
92+
await vi.waitFor(async () => {
93+
const [audit] = await db
94+
.select({ actorId: auditLog.actorId, metadata: auditLog.metadata })
95+
.from(auditLog)
96+
.where(and(eq(auditLog.resourceId, resourceId), eq(auditLog.action, action)))
97+
expect(audit).toMatchObject({
98+
actorId: adminId,
99+
metadata: {
100+
operation: 'access_requests.resolve',
101+
actor: { kind: 'organization_delegated', serviceId: 'copilot', subjectUserId: adminId },
102+
},
103+
})
104+
})
105+
}
106+
107+
describe('organization Chat review of access requests', () => {
108+
beforeAll(async () => {
109+
const now = new Date()
110+
const people = [adminId, memberId, requesterId]
111+
await db.insert(user).values(
112+
people.map((id) => ({
113+
id,
114+
name: 'Access review fixture',
115+
email: `${id}@access-review.test`,
116+
emailVerified: true,
117+
createdAt: now,
118+
updatedAt: now,
119+
}))
120+
)
121+
await db.insert(organization).values({
122+
id: organizationId,
123+
name: 'Access review fixture',
124+
slug: organizationId,
125+
createdAt: now,
126+
updatedAt: now,
127+
})
128+
await db.insert(member).values(
129+
people.map((userId) => ({
130+
id: generateId(),
131+
userId,
132+
organizationId,
133+
role: userId === adminId ? 'admin' : 'member',
134+
createdAt: now,
135+
}))
136+
)
137+
await insertWorkspaceFixture(db, [
138+
{
139+
id: workspaceId,
140+
name: 'Access review fixture',
141+
ownerId: adminId,
142+
billedAccountUserId: adminId,
143+
organizationId,
144+
workspaceMode: 'organization' as const,
145+
},
146+
])
147+
await db.insert(permissions).values({
148+
id: generateId(),
149+
userId: requesterId,
150+
entityType: 'workspace',
151+
entityId: workspaceId,
152+
permissionType: 'write' as const,
153+
})
154+
await db.insert(permissionGroup).values({
155+
id: groupId,
156+
organizationId,
157+
name: 'Tables and files withheld',
158+
createdBy: adminId,
159+
config: { hideTablesTab: true, hideFilesTab: true },
160+
})
161+
await db.insert(permissionGroupWorkspace).values({
162+
id: generateId(),
163+
permissionGroupId: groupId,
164+
workspaceId,
165+
organizationId,
166+
})
167+
for (const userId of [adminId, memberId]) {
168+
const [chat] = await db
169+
.insert(copilotChats)
170+
.values({
171+
userId,
172+
organizationId,
173+
type: 'mothership',
174+
config: { conversationMode: 'agent' },
175+
model: 'fixture-model',
176+
})
177+
.returning({ id: copilotChats.id })
178+
chats.set(userId, chat.id)
179+
}
180+
const requester = { kind: 'session', userId: requesterId, sessionId: 'fixture' } as const
181+
for (const [key, configKey] of [
182+
['tables', 'hideTablesTab'],
183+
['files', 'hideFilesTab'],
184+
] as const) {
185+
const { request } = await createAccessRequest.execute({
186+
principal: requester,
187+
input: {
188+
scope: { kind: 'workspace', workspaceId },
189+
target: { kind: 'feature', configKey },
190+
reason: 'Need it for a report',
191+
},
192+
})
193+
requestIds[key] = request.id
194+
}
195+
})
196+
197+
afterAll(async () => {
198+
const ids = Object.values(requestIds)
199+
await db.delete(outboxEvent).where(inArray(sql`${outboxEvent.payload}->>'requestId'`, ids))
200+
await db
201+
.delete(permissionAccessRequest)
202+
.where(eq(permissionAccessRequest.organizationId, organizationId))
203+
await db.delete(auditLog).where(inArray(auditLog.resourceId, [...ids, groupId]))
204+
await db.delete(copilotChats).where(eq(copilotChats.organizationId, organizationId))
205+
await deleteWorkspaceFixture(db, eq(workspace.id, workspaceId))
206+
await db.delete(organization).where(eq(organization.id, organizationId))
207+
await db.delete(user).where(inArray(user.id, [adminId, memberId, requesterId]))
208+
})
209+
210+
it("refuses a non-admin member's Chat every review operation", async () => {
211+
const asMember = asChat(memberId)
212+
const fingerprint = 'f'.repeat(64)
213+
214+
await expect(asMember('list', {})).rejects.toMatchObject({ code: 'forbidden' })
215+
await expect(asMember('preview', { requestId: requestIds.tables })).rejects.toMatchObject({
216+
code: 'forbidden',
217+
})
218+
await expect(
219+
asMember('approve', { requestId: requestIds.tables, expectedFingerprint: fingerprint })
220+
).rejects.toMatchObject({ code: 'forbidden' })
221+
await expect(
222+
asMember('deny', { requestId: requestIds.files, reason: 'No' })
223+
).rejects.toMatchObject({ code: 'forbidden' })
224+
225+
expect(await readRequest(requestIds.tables)).toMatchObject({ status: 'pending' })
226+
expect(await readRequest(requestIds.files)).toMatchObject({ status: 'pending' })
227+
expect(await readGroupConfig()).toMatchObject({ hideTablesTab: true, hideFilesTab: true })
228+
})
229+
230+
it("lets an admin's Chat list, preview and approve a request, granting it", async () => {
231+
const asAdmin = asChat(adminId)
232+
233+
const section = (await settingsServerTool.execute(
234+
{ scope: 'organization', action: 'get', section: 'requests' },
235+
chatContext(adminId)
236+
)) as { value: { requests: { id: string; status: string }[] } }
237+
expect(section.value.requests.map(({ id }) => id).sort()).toEqual(
238+
Object.values(requestIds).sort()
239+
)
240+
241+
const { result: preview } = await asAdmin('preview', { requestId: requestIds.tables })
242+
expect(preview).toMatchObject({ canApply: true, resolutionKind: 'permission' })
243+
244+
const { result } = await asAdmin('approve', {
245+
requestId: requestIds.tables,
246+
expectedFingerprint: preview.fingerprint,
247+
})
248+
249+
expect(result).toMatchObject({ changed: true, request: { status: 'fulfilled' } })
250+
expect(await readRequest(requestIds.tables)).toMatchObject({
251+
status: 'fulfilled',
252+
decidedBy: adminId,
253+
})
254+
expect(await readGroupConfig()).toMatchObject({ hideTablesTab: false, hideFilesTab: true })
255+
expect(await decidedEvents(requestIds.tables)).toHaveLength(1)
256+
await waitForAudit(requestIds.tables, 'permission_access_request.fulfilled')
257+
})
258+
259+
it("lets an admin's Chat deny a request with a reason, changing no policy", async () => {
260+
const { result } = await asChat(adminId)('deny', {
261+
requestId: requestIds.files,
262+
reason: 'Files stay restricted this quarter',
263+
})
264+
265+
expect(result).toMatchObject({ changed: true, request: { status: 'declined' } })
266+
expect(await readRequest(requestIds.files)).toEqual({
267+
status: 'declined',
268+
decidedBy: adminId,
269+
decisionReason: 'Files stay restricted this quarter',
270+
})
271+
expect(await readGroupConfig()).toMatchObject({ hideTablesTab: false, hideFilesTab: true })
272+
expect(await decidedEvents(requestIds.files)).toHaveLength(1)
273+
await waitForAudit(requestIds.files, 'permission_access_request.declined')
274+
})
275+
})

0 commit comments

Comments
 (0)