Skip to content

Commit d69f177

Browse files
committed
fix(mothership): let Chat share credentials and request access as the delegating member
1 parent 13461e3 commit d69f177

16 files changed

Lines changed: 625 additions & 122 deletions

File tree

‎apps/sim/app/api/v2/access-requests.test.ts‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,12 @@ vi.mock('@/ee/access-requests/lib/application/requests', () => ({
3030
},
3131
createAccessRequest: { operation: { id: 'access_requests.create' }, execute: mocks.create },
3232
cancelAccessRequest: { operation: { id: 'access_requests.cancel' }, execute: mocks.cancel },
33+
workspaceAccessRequestUseCases: {
34+
discover: { operation: { id: 'access_requests.discover' }, execute: mocks.discover },
35+
listMine: { operation: { id: 'access_requests.list_mine' }, execute: mocks.listMine },
36+
create: { operation: { id: 'access_requests.create' }, execute: mocks.create },
37+
cancel: { operation: { id: 'access_requests.cancel' }, execute: mocks.cancel },
38+
},
3339
listOrganizationAccessRequests: {
3440
operation: { id: 'access_requests.list_organization' },
3541
execute: mocks.listOrganization,

‎apps/sim/app/api/v2/workspaces/[workspaceId]/access-requests/[requestId]/cancel/route.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import { v2CancelWorkspaceAccessRequestContract } from '@/lib/api/contracts/v2/a
22
import { defineV2JsonRoute, v2ApiKeyAuth, v2RateLimits } from '@/lib/api/server/routes'
33
import { v2AccessRequestErrorPolicy } from '@/lib/api/server/routes/access-requests'
44
import { accessRequestOperations } from '@/ee/access-requests/lib/application/operations'
5-
import { cancelAccessRequest } from '@/ee/access-requests/lib/application/requests'
5+
import { workspaceAccessRequestUseCases } from '@/ee/access-requests/lib/application/requests'
66

77
export const POST = defineV2JsonRoute({
88
contract: v2CancelWorkspaceAccessRequestContract,
@@ -14,6 +14,6 @@ export const POST = defineV2JsonRoute({
1414
requestId: params.requestId,
1515
scope: { kind: 'workspace' as const, workspaceId: params.workspaceId },
1616
}),
17-
useCase: cancelAccessRequest,
17+
useCase: workspaceAccessRequestUseCases.cancel,
1818
present: ({ request }) => ({ data: request }),
1919
})

‎apps/sim/app/api/v2/workspaces/[workspaceId]/access-requests/discovery/route.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ import { defineV2JsonRoute, v2ApiKeyAuth, v2RateLimits } from '@/lib/api/server/
44
import { v2AccessRequestErrorPolicy } from '@/lib/api/server/routes/access-requests'
55
import { cursorSortKey, decodeOffsetCursor, encodeOffsetCursor } from '@/app/api/v2/lib/response'
66
import { accessRequestOperations } from '@/ee/access-requests/lib/application/operations'
7-
import { discoverAccessRequests } from '@/ee/access-requests/lib/application/requests'
7+
import { workspaceAccessRequestUseCases } from '@/ee/access-requests/lib/application/requests'
88

99
function cursorFilters(
1010
params: { workspaceId: string },
@@ -33,7 +33,7 @@ export const GET = defineV2JsonRoute({
3333
cursorFilters(params, query)
3434
),
3535
}),
36-
useCase: discoverAccessRequests,
36+
useCase: workspaceAccessRequestUseCases.discover,
3737
present: ({ entries, hasMore }, { params, query }) => ({
3838
data: entries,
3939
nextCursor: hasMore

‎apps/sim/app/api/v2/workspaces/[workspaceId]/access-requests/route.ts‎

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -7,10 +7,7 @@ import { defineV2JsonRoute, v2ApiKeyAuth, v2RateLimits } from '@/lib/api/server/
77
import { v2AccessRequestErrorPolicy } from '@/lib/api/server/routes/access-requests'
88
import { readSortedCursor, writeSortedCursor } from '@/app/api/v2/lib/response'
99
import { accessRequestOperations } from '@/ee/access-requests/lib/application/operations'
10-
import {
11-
createAccessRequest,
12-
listMyAccessRequests,
13-
} from '@/ee/access-requests/lib/application/requests'
10+
import { workspaceAccessRequestUseCases } from '@/ee/access-requests/lib/application/requests'
1411

1512
function cursorFilters(params: { workspaceId: string }, query: { status?: string }) {
1613
return cursorScopeKey(cursorRoute(v2ListMyWorkspaceAccessRequestsContract, params), {
@@ -40,7 +37,7 @@ export const GET = defineV2JsonRoute({
4037
),
4138
},
4239
}),
43-
useCase: listMyAccessRequests,
40+
useCase: workspaceAccessRequestUseCases.listMine,
4441
present: ({ requests, nextCursorKeys }, { params, query }) => ({
4542
data: requests,
4643
nextCursor: writeSortedCursor(
@@ -62,6 +59,6 @@ export const POST = defineV2JsonRoute({
6259
...body,
6360
scope: { kind: 'workspace' as const, workspaceId: params.workspaceId },
6461
}),
65-
useCase: createAccessRequest,
62+
useCase: workspaceAccessRequestUseCases.create,
6663
present: ({ request }) => ({ data: request }),
6764
})

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

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import { db } from '@sim/db'
22
import { member, permissions, user, workspace } from '@sim/db/schema'
33
import { dbChainMockFns, queueTableRows, resetDbChainMock } from '@sim/testing'
44
import {
5+
createDelegatedPrincipal,
56
createPersonalApiKeyPrincipal,
67
createSessionPrincipal,
78
} from '@sim/testing/factories/principal.factory'
@@ -185,6 +186,18 @@ describe('access request scope authorization', () => {
185186
expect(dbChainMockFns.select).not.toHaveBeenCalled()
186187
})
187188

189+
it("refuses Chat on a member's organization-scoped request before any lookup", async () => {
190+
const chat = createDelegatedPrincipal({
191+
subjectUserId: 'person',
192+
workspaceId: 'workspace',
193+
audience: 'sim:settings',
194+
})
195+
await expect(
196+
authorizeAccessRequestScope(chat, accessRequestOperations.create, organizationScope)
197+
).rejects.toMatchObject({ detailCode: 'PRINCIPAL_KIND_NOT_PERMITTED' })
198+
expect(dbChainMockFns.select).not.toHaveBeenCalled()
199+
})
200+
188201
it('conceals an archived or removed workspace before membership lookup', async () => {
189202
queueTableRows(workspace, [])
190203
await expect(

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

Lines changed: 16 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import { requirePrincipalSubjectUserId } from '@sim/auth/principal'
12
import { db } from '@sim/db'
23
import { member, permissions, user, workspace } from '@sim/db/schema'
34
import {
@@ -10,13 +11,15 @@ import { isAccountBlocked } from '@/lib/auth/ban'
1011
import { authorizeOrganizationOperation } from '@/lib/core/application/organization-authorization'
1112
import {
1213
authorizeWorkspaceOperation,
14+
PrincipalKindAuthorizationError,
1315
requireAllowedWorkspacePrincipal,
1416
} from '@/lib/core/application/workspace-authorization'
1517
import { OrchestrationError } from '@/lib/core/orchestration/types'
1618
import type { DbOrTx } from '@/lib/db/types'
17-
import type {
18-
AccessRequestOperation,
19-
AccessRequestPrincipal,
19+
import {
20+
ACCESS_REQUEST_DELEGATION_AUDIENCE,
21+
type AccessRequestOperation,
22+
type AccessRequestPrincipal,
2023
} from '@/ee/access-requests/lib/application/operations'
2124
import type { AccessRequestScope } from '@/ee/access-requests/lib/targets'
2225

@@ -95,6 +98,10 @@ export async function authorizeAccessRequestScope(
9598
if (operation.admin && scope.kind !== 'organization') {
9699
throw new OrchestrationError('forbidden', 'Organization administrator access is required')
97100
}
101+
if (principal.kind === 'delegated' && scope.kind !== 'workspace') {
102+
throw new PrincipalKindAuthorizationError(principal.kind, operation.id)
103+
}
104+
const actorUserId = requirePrincipalSubjectUserId(principal)
98105
let canonicalWorkspace:
99106
| Pick<typeof workspace.$inferSelect, 'id' | 'organizationId' | 'allowPersonalApiKeys'>
100107
| undefined
@@ -119,7 +126,7 @@ export async function authorizeAccessRequestScope(
119126
}
120127
const membership = await loadAccessRequestMembership(
121128
executor,
122-
principal.userId,
129+
actorUserId,
123130
scope,
124131
organizationId,
125132
forUpdate
@@ -136,7 +143,11 @@ export async function authorizeAccessRequestScope(
136143
workspaceOrganizationId: canonicalWorkspace.organizationId,
137144
allowPersonalApiKeys: canonicalWorkspace.allowPersonalApiKeys,
138145
},
139-
{ executor, forUpdate }
146+
{
147+
executor,
148+
forUpdate,
149+
delegation: { audience: ACCESS_REQUEST_DELEGATION_AUDIENCE, isWithinScope: () => true },
150+
}
140151
)
141152
} else if (scope.kind === 'organization') {
142153
await authorizeOrganizationOperation(

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

Lines changed: 20 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import type { Principal } from '@sim/auth/principal'
22
import { db } from '@sim/db'
33
import {
4+
createDelegatedPrincipal,
45
createPersonalApiKeyPrincipal,
56
createSessionPrincipal,
67
createWorkspaceApiKeyPrincipal,
@@ -95,6 +96,11 @@ describe('authorized access request execution', () => {
9596
scopes: ['api:write'],
9697
expiresAt: new Date('2099-01-01'),
9798
},
99+
createDelegatedPrincipal({
100+
subjectUserId: 'requester',
101+
workspaceId: 'workspace',
102+
audience: 'sim:settings',
103+
}),
98104
])(
99105
'preserves the $kind actor through preparation, transactional reauthorization and audit',
100106
async (caller) => {
@@ -110,7 +116,12 @@ describe('authorized access request execution', () => {
110116
projectAudit: () => audit,
111117
})
112118
await expect(useCase.execute({ principal: caller, input })).resolves.toBe('result')
113-
expect(prepare).toHaveBeenCalledWith({ principal: caller, input, context })
119+
expect(prepare).toHaveBeenCalledWith({
120+
principal: caller,
121+
actorUserId: 'requester',
122+
input,
123+
context,
124+
})
114125
expect(mocks.authorize).toHaveBeenNthCalledWith(
115126
2,
116127
caller,
@@ -122,6 +133,7 @@ describe('authorized access request execution', () => {
122133
)
123134
expect(execute).toHaveBeenCalledWith({
124135
principal: caller,
136+
actorUserId: 'requester',
125137
input,
126138
context,
127139
executor: transaction,
@@ -194,7 +206,12 @@ describe('authorized access request execution', () => {
194206
execute,
195207
})
196208
await expect(useCase.execute({ principal, input })).resolves.toEqual({ id: 'request' })
197-
expect(prepare).toHaveBeenCalledExactlyOnceWith({ principal, input, context })
209+
expect(prepare).toHaveBeenCalledExactlyOnceWith({
210+
principal,
211+
actorUserId: 'requester',
212+
input,
213+
context,
214+
})
198215
expect(prepare.mock.invocationCallOrder[0]).toBeLessThan(
199216
vi.mocked(db.transaction).mock.invocationCallOrder[0]
200217
)
@@ -213,6 +230,7 @@ describe('authorized access request execution', () => {
213230
)
214231
expect(execute).toHaveBeenCalledExactlyOnceWith({
215232
principal,
233+
actorUserId: 'requester',
216234
input,
217235
context,
218236
executor: transaction,

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

Lines changed: 30 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import type { Principal } from '@sim/auth/principal'
1+
import { type Principal, requirePrincipalSubjectUserId } from '@sim/auth/principal'
22
import { db } from '@sim/db'
33
import { acquireOrganizationMutationLock } from '@/lib/billing/organizations/membership'
44
import {
@@ -13,14 +13,17 @@ import {
1313
type AccessRequestContext,
1414
authorizeAccessRequestScope,
1515
} from '@/ee/access-requests/lib/application/authorization'
16-
import type {
17-
AccessRequestOperation,
18-
AccessRequestPrincipal,
16+
import {
17+
ACCESS_REQUEST_DELEGATION_AUDIENCE,
18+
type AccessRequestOperation,
19+
type AccessRequestPrincipal,
1920
} from '@/ee/access-requests/lib/application/operations'
2021
import type { AccessRequestScope } from '@/ee/access-requests/lib/targets'
2122

2223
interface AccessRequestPreparationArgs<I> {
2324
principal: AccessRequestPrincipal
25+
/** The person acting: the caller, or the member Chat acts for. */
26+
actorUserId: string
2427
input: I
2528
context: AccessRequestContext
2629
}
@@ -107,7 +110,8 @@ export function defineAuthorizedAccessRequestUseCase<I, R, P = undefined>(
107110
const scope = definition.scope(input)
108111
const initial = await authorizeAccessRequestScope(principal, definition.operation, scope)
109112
return runWithOutboundOrganization(initial.organizationId, async () => {
110-
const preparation = { principal, input, context: initial }
113+
const actorUserId = requirePrincipalSubjectUserId(principal)
114+
const preparation = { principal, actorUserId, input, context: initial }
111115
let context = initial
112116
let result: R
113117
if (definition.mutation) {
@@ -124,19 +128,26 @@ export function defineAuthorizedAccessRequestUseCase<I, R, P = undefined>(
124128
true,
125129
initial
126130
)
127-
return execute({ principal, input, context, executor })
131+
return execute({ principal, actorUserId, input, context, executor })
128132
})
129133
} else {
130134
const execute = await prepareExecution(definition, preparation)
131-
result = await execute({ principal, input, context, executor: db })
135+
result = await execute({ principal, actorUserId, input, context, executor: db })
132136
}
133137
if (definition.projectAudit) {
134138
recordProjectedUseCaseAuditEntries(
135139
definition.operation,
136140
context.workspaceId,
137141
principal,
138142
request,
139-
definition.projectAudit({ principal, input, context, executor: db, result }),
143+
definition.projectAudit({
144+
principal,
145+
actorUserId,
146+
input,
147+
context,
148+
executor: db,
149+
result,
150+
}),
140151
context.organizationId ?? undefined
141152
)
142153
}
@@ -145,3 +156,14 @@ export function defineAuthorizedAccessRequestUseCase<I, R, P = undefined>(
145156
},
146157
}
147158
}
159+
160+
/**
161+
* The workspace routes' copy of a member use case. Declaring the audience admits Chat acting for
162+
* the requester, pinned to its own workspace; the organization routes keep the shared use case,
163+
* so Chat never reaches them.
164+
*/
165+
export function admitWorkspaceDelegation<I, R>(
166+
useCase: OperationUseCase<AccessRequestOperation, I, R>
167+
): OperationUseCase<AccessRequestOperation, I, R> {
168+
return Object.freeze({ ...useCase, delegationAudience: ACCESS_REQUEST_DELEGATION_AUDIENCE })
169+
}

0 commit comments

Comments
 (0)