Skip to content

Commit baca4bf

Browse files
authored
fix(api): refuse workspace keys and enforce personal_api_key.use on v1 audit logs (#8025)
1 parent 96ab25e commit baca4bf

8 files changed

Lines changed: 271 additions & 20 deletions

File tree

‎apps/sim/app/api/v1/audit-logs/[id]/route.test.ts‎

Lines changed: 29 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -9,12 +9,12 @@ import { beforeEach, describe, expect, it, vi } from 'vitest'
99

1010
const {
1111
mockCheckRateLimit,
12-
mockValidateEnterpriseAuditAccess,
12+
mockValidateV1EnterpriseAuditAccess,
1313
mockBuildOrgScopeCondition,
1414
mockGetOrgWorkspaceIds,
1515
} = vi.hoisted(() => ({
1616
mockCheckRateLimit: vi.fn(),
17-
mockValidateEnterpriseAuditAccess: vi.fn(),
17+
mockValidateV1EnterpriseAuditAccess: vi.fn(),
1818
mockBuildOrgScopeCondition: vi.fn(),
1919
mockGetOrgWorkspaceIds: vi.fn(),
2020
}))
@@ -25,7 +25,7 @@ vi.mock('@/app/api/v1/middleware', () => ({
2525
}))
2626

2727
vi.mock('@/app/api/v1/audit-logs/auth', () => ({
28-
validateEnterpriseAuditAccess: mockValidateEnterpriseAuditAccess,
28+
validateV1EnterpriseAuditAccess: mockValidateV1EnterpriseAuditAccess,
2929
}))
3030

3131
vi.mock('@/lib/audit-logs/query', () => ({
@@ -76,8 +76,9 @@ describe('GET /api/v1/audit-logs/[id]', () => {
7676
beforeEach(() => {
7777
vi.clearAllMocks()
7878
mockCheckRateLimit.mockResolvedValue({ allowed: true, userId: 'admin-1' })
79-
mockValidateEnterpriseAuditAccess.mockResolvedValue({
79+
mockValidateV1EnterpriseAuditAccess.mockResolvedValue({
8080
success: true,
81+
userId: 'admin-1',
8182
context: { organizationId: ORG_ID, orgMemberIds: MEMBER_IDS },
8283
})
8384
mockGetOrgWorkspaceIds.mockResolvedValue(ORG_WORKSPACE_IDS)
@@ -124,4 +125,28 @@ describe('GET /api/v1/audit-logs/[id]', () => {
124125
expect(body.data.ipAddress).toBeUndefined()
125126
expect(body.data.userAgent).toBeUndefined()
126127
})
128+
129+
it('returns the refusal for a workspace key without querying', async () => {
130+
mockCheckRateLimit.mockResolvedValue({
131+
allowed: true,
132+
userId: 'admin-1',
133+
keyType: 'workspace',
134+
workspaceId: 'ws-org-1',
135+
})
136+
const denied = new Response(
137+
JSON.stringify({ error: 'Audit logs require a personal API key' }),
138+
{
139+
status: 403,
140+
}
141+
)
142+
mockValidateV1EnterpriseAuditAccess.mockResolvedValue({ success: false, response: denied })
143+
144+
const response = await callRoute('log-1')
145+
146+
expect(response.status).toBe(403)
147+
expect(mockValidateV1EnterpriseAuditAccess).toHaveBeenCalledWith(
148+
expect.objectContaining({ keyType: 'workspace' })
149+
)
150+
expect(dbChainMockFns.select).not.toHaveBeenCalled()
151+
})
127152
})

‎apps/sim/app/api/v1/audit-logs/[id]/route.ts‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ import { v1GetAuditLogContract } from '@/lib/api/contracts/v1/audit-logs'
2222
import { parseRequest } from '@/lib/api/server'
2323
import { buildOrgScopeCondition, getOrgWorkspaceIds } from '@/lib/audit-logs/query'
2424
import { withRouteHandler } from '@/lib/core/utils/with-route-handler'
25-
import { validateEnterpriseAuditAccess } from '@/app/api/v1/audit-logs/auth'
25+
import { validateV1EnterpriseAuditAccess } from '@/app/api/v1/audit-logs/auth'
2626
import { formatAuditLogEntry } from '@/app/api/v1/audit-logs/format'
2727
import { createApiResponse, getUserLimits } from '@/app/api/v1/logs/meta'
2828
import { checkRateLimit, createRateLimitResponse } from '@/app/api/v1/middleware'
@@ -49,7 +49,6 @@ export const GET = withRouteHandler(
4949
return createRateLimitResponse(rateLimit)
5050
}
5151

52-
const userId = rateLimit.userId!
5352
const parsed = await parseRequest(v1GetAuditLogContract, request, context, {
5453
validationErrorResponse: () =>
5554
NextResponse.json({ error: 'Invalid audit log ID' }, { status: 400 }),
@@ -58,11 +57,12 @@ export const GET = withRouteHandler(
5857

5958
const { id } = parsed.data.params
6059

61-
const authResult = await validateEnterpriseAuditAccess(userId)
60+
const authResult = await validateV1EnterpriseAuditAccess(rateLimit)
6261
if (!authResult.success) {
6362
return authResult.response
6463
}
6564

65+
const { userId } = authResult
6666
const { organizationId, orgMemberIds } = authResult.context
6767

6868
const orgWorkspaceIds = await getOrgWorkspaceIds(organizationId)

‎apps/sim/app/api/v1/audit-logs/auth.test.ts‎

Lines changed: 85 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -11,21 +11,34 @@ import {
1111
} from '@sim/testing'
1212
import { afterAll, beforeEach, describe, expect, it, vi } from 'vitest'
1313

14-
const { mockIsOrganizationBillingBlocked } = vi.hoisted(() => ({
15-
mockIsOrganizationBillingBlocked: vi.fn(),
16-
}))
14+
const { mockIsOrganizationBillingBlocked, mockCheckOrganizationPersonalKeyRefusal } = vi.hoisted(
15+
() => ({
16+
mockIsOrganizationBillingBlocked: vi.fn(),
17+
mockCheckOrganizationPersonalKeyRefusal: vi.fn(),
18+
})
19+
)
1720

1821
vi.mock('@/lib/billing/core/access', () => ({
1922
isOrganizationBillingBlocked: mockIsOrganizationBillingBlocked,
2023
}))
2124

22-
import { validateEnterpriseAuditAccess } from '@/app/api/v1/audit-logs/auth'
25+
vi.mock('@/app/api/v1/middleware', () => ({
26+
capabilityGovernedUserId: (rateLimit: { keyType?: string; userId?: string }) =>
27+
rateLimit.keyType === 'personal' ? (rateLimit.userId ?? null) : null,
28+
checkOrganizationPersonalKeyRefusal: mockCheckOrganizationPersonalKeyRefusal,
29+
}))
30+
31+
import {
32+
validateEnterpriseAuditAccess,
33+
validateV1EnterpriseAuditAccess,
34+
} from '@/app/api/v1/audit-logs/auth'
2335

2436
describe('enterprise audit access', () => {
2537
beforeEach(() => {
2638
vi.clearAllMocks()
2739
resetDbChainMock()
2840
mockIsOrganizationBillingBlocked.mockResolvedValue(false)
41+
mockCheckOrganizationPersonalKeyRefusal.mockResolvedValue(null)
2942
})
3043

3144
afterAll(() => {
@@ -112,4 +125,72 @@ describe('enterprise audit access', () => {
112125
})
113126
})
114127
})
128+
129+
describe('v1 API-key access', () => {
130+
const personalKey = {
131+
allowed: true,
132+
remaining: 1,
133+
limit: 1,
134+
resetAt: new Date(),
135+
userId: 'viewer',
136+
keyType: 'personal' as const,
137+
}
138+
139+
beforeEach(() => {
140+
setEnvFlags({ isBillingEnabled: false, isAuditLogsEnabled: true })
141+
})
142+
143+
it('refuses a workspace key before resolving its creator as the subject', async () => {
144+
const result = await validateV1EnterpriseAuditAccess({
145+
...personalKey,
146+
keyType: 'workspace',
147+
workspaceId: 'workspace-a',
148+
})
149+
150+
if (result.success) throw new Error('Expected the workspace key to be refused')
151+
expect(result.response.status).toBe(403)
152+
await expect(result.response.json()).resolves.toEqual({
153+
error: 'Audit logs require a personal API key',
154+
})
155+
expect(dbChainMockFns.where).not.toHaveBeenCalled()
156+
expect(mockCheckOrganizationPersonalKeyRefusal).not.toHaveBeenCalled()
157+
})
158+
159+
it('authorizes a personal key held by an organization admin', async () => {
160+
queueTableRows(schemaMock.member, [{ organizationId: 'org-1', role: 'admin' }])
161+
queueTableRows(schemaMock.member, [{ userId: 'viewer' }])
162+
163+
await expect(validateV1EnterpriseAuditAccess(personalKey)).resolves.toEqual({
164+
success: true,
165+
userId: 'viewer',
166+
context: { organizationId: 'org-1', orgMemberIds: ['viewer'] },
167+
})
168+
expect(mockCheckOrganizationPersonalKeyRefusal).toHaveBeenCalledWith(personalKey)
169+
})
170+
171+
it('refuses a personal key its permission group withholds', async () => {
172+
queueTableRows(schemaMock.member, [{ organizationId: 'org-1', role: 'admin' }])
173+
queueTableRows(schemaMock.member, [{ userId: 'viewer' }])
174+
const refusal = new Response(null, { status: 403 })
175+
mockCheckOrganizationPersonalKeyRefusal.mockResolvedValue(refusal)
176+
177+
const result = await validateV1EnterpriseAuditAccess(personalKey)
178+
179+
if (result.success) throw new Error('Expected the withheld personal key to be refused')
180+
expect(result.response).toBe(refusal)
181+
})
182+
183+
it('answers a non-admin with the role refusal, not the group configuration', async () => {
184+
queueTableRows(schemaMock.member, [{ organizationId: 'org-1', role: 'member' }])
185+
mockCheckOrganizationPersonalKeyRefusal.mockResolvedValue(new Response(null, { status: 403 }))
186+
187+
const result = await validateV1EnterpriseAuditAccess(personalKey)
188+
189+
if (result.success) throw new Error('Expected the non-admin to be refused')
190+
await expect(result.response.json()).resolves.toEqual({
191+
error: 'Organization admin or owner role required',
192+
})
193+
expect(mockCheckOrganizationPersonalKeyRefusal).not.toHaveBeenCalled()
194+
})
195+
})
115196
})

‎apps/sim/app/api/v1/audit-logs/auth.ts‎

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,11 +3,20 @@ import {
33
type EnterpriseAuditContext,
44
resolveEnterpriseAuditAccess,
55
} from '@/lib/audit-logs/authorization'
6+
import {
7+
capabilityGovernedUserId,
8+
checkOrganizationPersonalKeyRefusal,
9+
type RateLimitResult,
10+
} from '@/app/api/v1/middleware'
611

712
type AuthResult =
813
| { success: true; context: EnterpriseAuditContext }
914
| { success: false; response: NextResponse }
1015

16+
type V1AuthResult =
17+
| { success: true; userId: string; context: EnterpriseAuditContext }
18+
| { success: false; response: NextResponse }
19+
1120
/**
1221
* v1 wrapper: renders {@link resolveEnterpriseAuditAccess} as the v1 `{ error }`
1322
* response body.
@@ -23,3 +32,37 @@ export async function validateEnterpriseAuditAccess(
2332
response: NextResponse.json({ error: result.message }, { status: result.status }),
2433
}
2534
}
35+
36+
/**
37+
* Authorizes a v1 API-key read of the organization audit trail with the same
38+
* policy as `auditLogOperations`, which v1 does not route through.
39+
*
40+
* Workspace keys are refused (`workspaceApiKey: 'deny'`): their `userId` is the
41+
* key's creator, so authorizing it would let a credential scoped to one
42+
* workspace read every workspace in the organization whenever its creator is an
43+
* organization admin. A personal key is then held to the user-global
44+
* `personal_api_key.use` group decision, checked after the admin role so the
45+
* refusal never describes an organization's configuration to a non-admin.
46+
*/
47+
export async function validateV1EnterpriseAuditAccess(
48+
rateLimit: RateLimitResult
49+
): Promise<V1AuthResult> {
50+
const userId = capabilityGovernedUserId(rateLimit)
51+
if (!userId) {
52+
return {
53+
success: false,
54+
response: NextResponse.json(
55+
{ error: 'Audit logs require a personal API key' },
56+
{ status: 403 }
57+
),
58+
}
59+
}
60+
61+
const access = await validateEnterpriseAuditAccess(userId)
62+
if (!access.success) return access
63+
64+
const personalKeyRefusal = await checkOrganizationPersonalKeyRefusal(rateLimit)
65+
if (personalKeyRefusal) return { success: false, response: personalKeyRefusal }
66+
67+
return { success: true, userId, context: access.context }
68+
}

‎apps/sim/app/api/v1/audit-logs/route.test.ts‎

Lines changed: 30 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -9,14 +9,14 @@ import { beforeEach, describe, expect, it, vi } from 'vitest'
99

1010
const {
1111
mockCheckRateLimit,
12-
mockValidateEnterpriseAuditAccess,
12+
mockValidateV1EnterpriseAuditAccess,
1313
mockBuildOrgScopeCondition,
1414
mockGetOrgWorkspaceIds,
1515
mockQueryAuditLogs,
1616
mockBuildFilterConditions,
1717
} = vi.hoisted(() => ({
1818
mockCheckRateLimit: vi.fn(),
19-
mockValidateEnterpriseAuditAccess: vi.fn(),
19+
mockValidateV1EnterpriseAuditAccess: vi.fn(),
2020
mockBuildOrgScopeCondition: vi.fn(),
2121
mockGetOrgWorkspaceIds: vi.fn(),
2222
mockQueryAuditLogs: vi.fn(),
@@ -31,7 +31,7 @@ vi.mock('@/app/api/v1/middleware', () => ({
3131
}))
3232

3333
vi.mock('@/app/api/v1/audit-logs/auth', () => ({
34-
validateEnterpriseAuditAccess: mockValidateEnterpriseAuditAccess,
34+
validateV1EnterpriseAuditAccess: mockValidateV1EnterpriseAuditAccess,
3535
}))
3636

3737
vi.mock('@/lib/audit-logs/query', () => ({
@@ -61,8 +61,9 @@ describe('GET /api/v1/audit-logs', () => {
6161
beforeEach(() => {
6262
vi.clearAllMocks()
6363
mockCheckRateLimit.mockResolvedValue({ allowed: true, userId: 'admin-1' })
64-
mockValidateEnterpriseAuditAccess.mockResolvedValue({
64+
mockValidateV1EnterpriseAuditAccess.mockResolvedValue({
6565
success: true,
66+
userId: 'admin-1',
6667
context: { organizationId: ORG_ID, orgMemberIds: MEMBER_IDS },
6768
})
6869
mockGetOrgWorkspaceIds.mockResolvedValue(ORG_WORKSPACE_IDS)
@@ -122,11 +123,35 @@ describe('GET /api/v1/audit-logs', () => {
122123

123124
it('returns the auth failure response when enterprise access is denied', async () => {
124125
const denied = new Response(JSON.stringify({ error: 'nope' }), { status: 403 })
125-
mockValidateEnterpriseAuditAccess.mockResolvedValue({ success: false, response: denied })
126+
mockValidateV1EnterpriseAuditAccess.mockResolvedValue({ success: false, response: denied })
126127

127128
const response = await GET(makeRequest(''))
128129

129130
expect(response.status).toBe(403)
130131
expect(mockQueryAuditLogs).not.toHaveBeenCalled()
131132
})
133+
134+
it('returns the refusal for a workspace key without querying', async () => {
135+
mockCheckRateLimit.mockResolvedValue({
136+
allowed: true,
137+
userId: 'admin-1',
138+
keyType: 'workspace',
139+
workspaceId: 'ws-org-1',
140+
})
141+
const denied = new Response(
142+
JSON.stringify({ error: 'Audit logs require a personal API key' }),
143+
{
144+
status: 403,
145+
}
146+
)
147+
mockValidateV1EnterpriseAuditAccess.mockResolvedValue({ success: false, response: denied })
148+
149+
const response = await GET(makeRequest('?workspaceId=ws-org-2'))
150+
151+
expect(response.status).toBe(403)
152+
expect(mockValidateV1EnterpriseAuditAccess).toHaveBeenCalledWith(
153+
expect.objectContaining({ keyType: 'workspace' })
154+
)
155+
expect(mockQueryAuditLogs).not.toHaveBeenCalled()
156+
})
132157
})

‎apps/sim/app/api/v1/audit-logs/route.ts‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ import {
3232
queryAuditLogs,
3333
} from '@/lib/audit-logs/query'
3434
import { withRouteHandler } from '@/lib/core/utils/with-route-handler'
35-
import { validateEnterpriseAuditAccess } from '@/app/api/v1/audit-logs/auth'
35+
import { validateV1EnterpriseAuditAccess } from '@/app/api/v1/audit-logs/auth'
3636
import { formatAuditLogEntry } from '@/app/api/v1/audit-logs/format'
3737
import { createApiResponse, getUserLimits } from '@/app/api/v1/logs/meta'
3838
import {
@@ -63,13 +63,12 @@ export const GET = withRouteHandler(async (request: NextRequest) => {
6363
return createRateLimitResponse(rateLimit)
6464
}
6565

66-
const userId = rateLimit.userId!
67-
68-
const authResult = await validateEnterpriseAuditAccess(userId)
66+
const authResult = await validateV1EnterpriseAuditAccess(rateLimit)
6967
if (!authResult.success) {
7068
return authResult.response
7169
}
7270

71+
const { userId } = authResult
7372
const { organizationId, orgMemberIds } = authResult.context
7473

7574
const parsed = await parseRequest(

0 commit comments

Comments
 (0)