Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ vi.mock('@sim/platform-authz/workspace', () => ({

import { AuditAction, AuditResourceType } from '@sim/audit'
import { defineAuthorizedWorkspaceUseCase, defineWorkspaceOperation } from '@/lib/core/application'
import { recordProjectedUseCaseAuditEntries } from '@/lib/core/application/authorized-workspace-use-case'
import { resolveCurrentOutboundRoute } from '@/lib/core/network/context.server'
import type { OrchestrationError } from '@/lib/core/orchestration/types'
import { CREDENTIAL_GROUP_CREDENTIAL_USE_ACTION } from '@/lib/resource-policies/registry'
Expand Down Expand Up @@ -494,3 +495,34 @@ describe('defineAuthorizedWorkspaceUseCase', () => {
)
})
})

describe('projected audit workspace attribution', () => {
beforeEach(() => vi.clearAllMocks())

it.each([
{ override: undefined, expected: 'workspace-1' },
{ override: 'workspace-2', expected: 'workspace-2' },
{ override: null, expected: null },
])('records the canonical workspace override $override', ({ override, expected }) => {
recordProjectedUseCaseAuditEntries(
operation,
'workspace-1',
sessionPrincipal,
undefined,
[
{
action: AuditAction.FILE_UPDATED,
resourceType: AuditResourceType.FILE,
workspaceId: override,
},
],
'organization-1'
)
expect(mocks.recordAudit).toHaveBeenCalledExactlyOnceWith(
expect.objectContaining({
workspaceId: expected,
metadata: expect.objectContaining({ organizationId: 'organization-1' }),
})
)
})
})
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,8 @@ import type { OrchestrationRequestContext } from '@/lib/core/orchestration/types
import type { ResourcePolicyBinding } from '@/lib/resource-policies/registry'

export interface WorkspaceUseCaseAuditEntry {
/** Canonical workspace affected by a cross-workspace mutation, when different from its authorization scope. */
workspaceId?: string
/** Canonical affected workspace; null keeps an organization event outside the authorization workspace. */
workspaceId?: string | null
action: AuditActionType
resourceType: AuditResourceTypeValue
resourceId?: string
Expand Down Expand Up @@ -106,7 +106,7 @@ export function recordProjectedUseCaseAuditEntries(
const attribution: PrincipalAuditAttribution = resolvePrincipalAuditAttribution(principal)
for (const entry of entries) {
recordAudit({
workspaceId: entry.workspaceId ?? workspaceId,
workspaceId: entry.workspaceId === undefined ? workspaceId : entry.workspaceId,
actorId: attribution.actorId,
actorName: attribution.actorName,
action: entry.action,
Expand Down
2 changes: 1 addition & 1 deletion apps/sim/lib/permission-access-requests/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ Members request access from locked features, the block picker, or **My access re
- Fulfillment updates the current governing group. The preview lists every required change, including parent restrictions, and a conservative upper bound of affected people/workspaces. It does not create individual grants or move members between groups.
- Approval rechecks administrator authority, requester membership identity, workspace ownership, entitlement at admission, organization preference, group resolution, and the preview fingerprint. Membership, policy, or scope changes require a fresh review or request.
- The group/credit-limit update, final request record, and notification enqueue share one database transaction. The final decision stores its original change and impact for history; later policy edits do not rewrite it.
- One pending request per requester/scope/target is enforced by a database index and organization serialization. Member cap requests share an organization-wide key. Submission is bounded to 25 requests per rolling day and 100 pending requests per requester/organization, in addition to HTTP rate admission.
- One pending request per requester/scope/target is enforced by a database index and organization serialization. Member cap requests share an organization-wide key. Submission is bounded to 100 requests per rolling 24 hours and 100 pending requests per requester/organization, in addition to HTTP rate admission.
- A usage request increases the existing member credit cap. It does not change the pooled organization budget, buy credits, or alter temporary request-rate limits.
- Notifications recheck current membership and reviewer authority. Outbox fan-out is bounded and replay-safe; delivery to an email provider remains at-least-once across a crash after send.

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -285,7 +285,7 @@ describe('create access requests', () => {

it.each([
{ pending: 100, daily: 0, message: '100 pending requests' },
{ pending: 0, daily: 25, message: 'maximum of 25 requests today' },
{ pending: 0, daily: 100, message: '100 requests in the last 24 hours' },
])('enforces bounded admissions ($message)', async ({ pending, daily, message }) => {
queueTableRows(permissionAccessRequest, [])
queueTableRows(permissionAccessRequest, [{ total: pending }])
Expand All @@ -297,6 +297,22 @@ describe('create access requests', () => {
expect(mocks.outbox).not.toHaveBeenCalled()
})

it.each([25, 99])(
'allows a new request after %s submissions in the rolling window',
async (daily) => {
queueTableRows(permissionAccessRequest, [])
queueTableRows(permissionAccessRequest, [{ total: 0 }])
queueTableRows(permissionAccessRequest, [{ total: daily }])
dbChainMockFns.returning.mockResolvedValueOnce([stored()])
const result = await createAccessRequest.execute({ principal, input: { scope, target } })
expect(result.changed).toBe(true)
expect(dbChainMockFns.insert).toHaveBeenCalledWith(permissionAccessRequest)
expect(mocks.audit.mock.calls[0][4]).toEqual([
expect.objectContaining({ workspaceId: 'workspace', resourceId: 'request' }),
])
}
)

it('normalizes member cap requests to one organization-wide request across workspaces', async () => {
const cap = stored({
target: { kind: 'usage_limit', id: 'member' },
Expand All @@ -315,6 +331,9 @@ describe('create access requests', () => {
input: { scope, target: { kind: 'usage_limit', id: 'member' } },
})
expect(first.changed).toBe(true)
expect(mocks.audit.mock.calls[0][4]).toEqual([
expect.objectContaining({ workspaceId: null, resourceId: cap.id }),
])
expect(dbChainMockFns.values).toHaveBeenCalledWith(
expect.objectContaining({
workspaceId: null,
Expand Down Expand Up @@ -489,6 +508,27 @@ describe('discovery and request history', () => {
})

describe('cancel access requests', () => {
it.each(['workspace', null])(
'keeps cancellation in the stored request scope %s',
async (workspaceId) => {
const row = stored({
workspaceId,
...(workspaceId === null
? {
scopeKey: 'organization:organization:member-limit',
target: { kind: 'usage_limit', id: 'member' },
}
: {}),
})
mocks.stored.mockResolvedValue(row)
dbChainMockFns.returning.mockResolvedValueOnce([{ ...row, status: 'cancelled' }])
await cancelAccessRequest.execute({ principal, input: { scope, requestId: row.id } })
expect(mocks.audit.mock.calls[0][4]).toEqual([
expect.objectContaining({ workspaceId, resourceId: row.id }),
])
}
)

it('allows cancellation while disabled and does not resend a decision for terminal requests', async () => {
mocks.enabled.mockResolvedValue(false)
dbChainMockFns.returning.mockResolvedValueOnce([stored({ status: 'cancelled' })])
Expand Down
23 changes: 17 additions & 6 deletions apps/sim/lib/permission-access-requests/application/requests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,11 @@ import {
listAccessRequestTargets,
loadAccessRequestCatalog,
} from '@/lib/permission-access-requests/catalog'
import {
ACCESS_REQUEST_MAX_DAILY_SUBMISSIONS,
ACCESS_REQUEST_MAX_PENDING,
ACCESS_REQUEST_SUBMISSION_WINDOW_MS,
} from '@/lib/permission-access-requests/constants'
import {
PERMISSION_ACCESS_REQUEST_CREATED_EVENT,
PERMISSION_ACCESS_REQUEST_DECIDED_EVENT,
Expand Down Expand Up @@ -151,7 +156,7 @@ export const discoverAccessRequests = defineAuthorizedAccessRequestUseCase({
eq(permissionAccessRequest.status, 'pending')
)
)
.limit(100)
.limit(ACCESS_REQUEST_MAX_PENDING)
const pendingMemberLimit = pending.find((row) => row.targetKey === 'usage_limit:member')
const memberLimitMembershipMatches = pendingMemberLimit
? await hasCurrentMemberLimitMembership(
Expand Down Expand Up @@ -351,10 +356,10 @@ export const createAccessRequest = defineAuthorizedAccessRequestUseCase({
eq(permissionAccessRequest.status, 'pending')
)
)
if ((outstanding?.total ?? 0) >= 100)
if ((outstanding?.total ?? 0) >= ACCESS_REQUEST_MAX_PENDING)
throw new OrchestrationError(
'conflict',
'You have 100 pending requests. Cancel an existing request before sending another.'
`You have ${ACCESS_REQUEST_MAX_PENDING} pending requests. Cancel an existing request before sending another.`
)
const [daily] = await executor
.select({ total: count() })
Expand All @@ -363,13 +368,16 @@ export const createAccessRequest = defineAuthorizedAccessRequestUseCase({
and(
eq(permissionAccessRequest.organizationId, organizationId),
eq(permissionAccessRequest.requesterId, principal.userId),
gte(permissionAccessRequest.createdAt, new Date(Date.now() - 86_400_000))
gte(
permissionAccessRequest.createdAt,
new Date(Date.now() - ACCESS_REQUEST_SUBMISSION_WINDOW_MS)
)
)
)
if ((daily?.total ?? 0) >= 25)
if ((daily?.total ?? 0) >= ACCESS_REQUEST_MAX_DAILY_SUBMISSIONS)
throw new OrchestrationError(
'conflict',
'You have sent the maximum of 25 requests today. Try again tomorrow.'
`You have sent ${ACCESS_REQUEST_MAX_DAILY_SUBMISSIONS} requests in the last 24 hours. Try again later.`
)
const [row] = await executor
.insert(permissionAccessRequest)
Expand Down Expand Up @@ -402,6 +410,7 @@ export const createAccessRequest = defineAuthorizedAccessRequestUseCase({
action: AuditAction.PERMISSION_ACCESS_REQUEST_CLOSED,
resourceType: AuditResourceType.PERMISSION_ACCESS_REQUEST,
resourceId: result.closedRequest.id,
workspaceId: result.closedRequest.workspaceId,
Comment thread
waleedlatif1 marked this conversation as resolved.
metadata: { target: result.closedRequest.target },
},
]
Expand All @@ -413,6 +422,7 @@ export const createAccessRequest = defineAuthorizedAccessRequestUseCase({
: AuditAction.PERMISSION_ACCESS_REQUEST_CREATED,
resourceType: AuditResourceType.PERMISSION_ACCESS_REQUEST,
resourceId: result.request.id,
workspaceId: result.request.workspaceId,
metadata: { target: result.request.target },
},
]
Expand Down Expand Up @@ -492,6 +502,7 @@ export const cancelAccessRequest = defineAuthorizedAccessRequestUseCase({
action: AuditAction.PERMISSION_ACCESS_REQUEST_CANCELLED,
resourceType: AuditResourceType.PERMISSION_ACCESS_REQUEST,
resourceId: result.request.id,
workspaceId: result.request.workspaceId,
},
]
: [],
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
/**
* @vitest-environment node
*/
import { AuditAction } from '@sim/audit'
import { db } from '@sim/db'
import {
organizationMemberUsageLimit,
Expand Down Expand Up @@ -407,6 +408,44 @@ describe('permission request review', () => {
expect(mocks.outbox).toHaveBeenCalledOnce()
})

it.each(['workspace', null])(
'attributes a declined request to its stored scope %s',
async (workspaceId) => {
mocks.stored.mockResolvedValue(stored({ workspaceId }))
dbChainMockFns.returning.mockResolvedValueOnce([stored({ workspaceId, status: 'declined' })])
await resolveAccessRequest.execute({
principal,
input: { ...input, decision: { action: 'decline', reason: 'Use the existing provider.' } },
})
expect(mocks.audit.mock.calls[0][4]).toEqual([
expect.objectContaining({
action: AuditAction.PERMISSION_ACCESS_REQUEST_DECLINED,
workspaceId,
}),
])
}
)

it('keeps request fulfillment workspace-scoped and the group change organization-scoped', async () => {
const before = await preview()
queueWorkspace()
dbChainMockFns.returning.mockResolvedValueOnce([stored({ status: 'fulfilled' })])
await resolveAccessRequest.execute({
principal,
input: { ...input, decision: { action: 'apply', expectedFingerprint: before.fingerprint } },
})
const [, defaultWorkspaceId, , , entries] = mocks.audit.mock.calls[0]
expect(defaultWorkspaceId).toBeNull()
expect(entries).toEqual([
expect.objectContaining({
action: AuditAction.PERMISSION_ACCESS_REQUEST_FULFILLED,
workspaceId: 'workspace',
}),
expect.objectContaining({ action: AuditAction.PERMISSION_GROUP_UPDATED }),
])
expect(entries[1]).not.toHaveProperty('workspaceId')
})

it('does not apply or notify a second time after resolution', async () => {
mocks.stored.mockResolvedValue(stored({ status: 'fulfilled' }))
const result = await resolveAccessRequest.execute({
Expand All @@ -417,6 +456,7 @@ describe('permission request review', () => {
expect(dbChainMockFns.update).not.toHaveBeenCalled()
expect(mocks.outbox).not.toHaveBeenCalled()
expect(mocks.catalog).not.toHaveBeenCalled()
expect(mocks.audit.mock.calls[0][4]).toEqual([])
})

it('reads fulfilled history from its stored decision without rebuilding the catalog', async () => {
Expand Down Expand Up @@ -483,7 +523,11 @@ describe('member limit review', () => {
})
queueLimit(2000)
dbChainMockFns.returning.mockResolvedValueOnce([
stored({ status: 'fulfilled', target: { kind: 'usage_limit', id: 'member' } }),
stored({
workspaceId: null,
status: 'fulfilled',
target: { kind: 'usage_limit', id: 'member' },
}),
])
await resolveAccessRequest.execute({
principal,
Expand All @@ -498,6 +542,16 @@ describe('member limit review', () => {
})
expect(mocks.setLimit).toHaveBeenCalledWith('organization', 'requester', 15, 'admin', db)
expect(dbChainMockFns.update).not.toHaveBeenCalledWith(permissionGroup)
const [, defaultWorkspaceId, , , entries] = mocks.audit.mock.calls[0]
expect(defaultWorkspaceId).toBeNull()
expect(entries).toEqual([
expect.objectContaining({
action: AuditAction.PERMISSION_ACCESS_REQUEST_FULFILLED,
workspaceId: null,
}),
expect.objectContaining({ action: AuditAction.ORG_MEMBER_USAGE_LIMIT_CHANGED }),
])
expect(entries[1]).not.toHaveProperty('workspaceId')
expect(mocks.outbox).toHaveBeenCalledWith(db, PERMISSION_ACCESS_REQUEST_DECIDED_EVENT, {
requestId: 'request',
})
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -363,6 +363,7 @@ export const resolveAccessRequest = defineAuthorizedAccessRequestUseCase({
: AuditAction.PERMISSION_ACCESS_REQUEST_DECLINED,
resourceType: AuditResourceType.PERMISSION_ACCESS_REQUEST,
resourceId: result.request.id,
workspaceId: result.request.workspaceId,
metadata: {
target: result.request.target,
requesterId: result.request.requester.id,
Expand Down
3 changes: 3 additions & 0 deletions apps/sim/lib/permission-access-requests/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,3 +2,6 @@ export const ACCESS_REQUEST_LIST_PAGE_SIZE = 25
export const ACCESS_REQUEST_MAX_OFFSET = 1_000_000
export const ACCESS_REQUEST_MAX_SEARCH_LENGTH = 200
export const ACCESS_REQUEST_MAX_ID_LENGTH = 128
export const ACCESS_REQUEST_MAX_DAILY_SUBMISSIONS = 100
export const ACCESS_REQUEST_MAX_PENDING = 100
export const ACCESS_REQUEST_SUBMISSION_WINDOW_MS = 24 * 60 * 60 * 1000
Loading
Loading