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
33 changes: 29 additions & 4 deletions apps/sim/app/api/v1/audit-logs/[id]/route.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,12 @@ import { beforeEach, describe, expect, it, vi } from 'vitest'

const {
mockCheckRateLimit,
mockValidateEnterpriseAuditAccess,
mockValidateV1EnterpriseAuditAccess,
mockBuildOrgScopeCondition,
mockGetOrgWorkspaceIds,
} = vi.hoisted(() => ({
mockCheckRateLimit: vi.fn(),
mockValidateEnterpriseAuditAccess: vi.fn(),
mockValidateV1EnterpriseAuditAccess: vi.fn(),
mockBuildOrgScopeCondition: vi.fn(),
mockGetOrgWorkspaceIds: vi.fn(),
}))
Expand All @@ -25,7 +25,7 @@ vi.mock('@/app/api/v1/middleware', () => ({
}))

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

vi.mock('@/lib/audit-logs/query', () => ({
Expand Down Expand Up @@ -76,8 +76,9 @@ describe('GET /api/v1/audit-logs/[id]', () => {
beforeEach(() => {
vi.clearAllMocks()
mockCheckRateLimit.mockResolvedValue({ allowed: true, userId: 'admin-1' })
mockValidateEnterpriseAuditAccess.mockResolvedValue({
mockValidateV1EnterpriseAuditAccess.mockResolvedValue({
success: true,
userId: 'admin-1',
context: { organizationId: ORG_ID, orgMemberIds: MEMBER_IDS },
})
mockGetOrgWorkspaceIds.mockResolvedValue(ORG_WORKSPACE_IDS)
Expand Down Expand Up @@ -124,4 +125,28 @@ describe('GET /api/v1/audit-logs/[id]', () => {
expect(body.data.ipAddress).toBeUndefined()
expect(body.data.userAgent).toBeUndefined()
})

it('returns the refusal for a workspace key without querying', async () => {
mockCheckRateLimit.mockResolvedValue({
allowed: true,
userId: 'admin-1',
keyType: 'workspace',
workspaceId: 'ws-org-1',
})
const denied = new Response(
JSON.stringify({ error: 'Audit logs require a personal API key' }),
{
status: 403,
}
)
mockValidateV1EnterpriseAuditAccess.mockResolvedValue({ success: false, response: denied })

const response = await callRoute('log-1')

expect(response.status).toBe(403)
expect(mockValidateV1EnterpriseAuditAccess).toHaveBeenCalledWith(
expect.objectContaining({ keyType: 'workspace' })
)
expect(dbChainMockFns.select).not.toHaveBeenCalled()
})
})
6 changes: 3 additions & 3 deletions apps/sim/app/api/v1/audit-logs/[id]/route.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ import { v1GetAuditLogContract } from '@/lib/api/contracts/v1/audit-logs'
import { parseRequest } from '@/lib/api/server'
import { buildOrgScopeCondition, getOrgWorkspaceIds } from '@/lib/audit-logs/query'
import { withRouteHandler } from '@/lib/core/utils/with-route-handler'
import { validateEnterpriseAuditAccess } from '@/app/api/v1/audit-logs/auth'
import { validateV1EnterpriseAuditAccess } from '@/app/api/v1/audit-logs/auth'
import { formatAuditLogEntry } from '@/app/api/v1/audit-logs/format'
import { createApiResponse, getUserLimits } from '@/app/api/v1/logs/meta'
import { checkRateLimit, createRateLimitResponse } from '@/app/api/v1/middleware'
Expand All @@ -49,7 +49,6 @@ export const GET = withRouteHandler(
return createRateLimitResponse(rateLimit)
}

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

const { id } = parsed.data.params

const authResult = await validateEnterpriseAuditAccess(userId)
const authResult = await validateV1EnterpriseAuditAccess(rateLimit)
if (!authResult.success) {
return authResult.response
}

const { userId } = authResult
const { organizationId, orgMemberIds } = authResult.context

const orgWorkspaceIds = await getOrgWorkspaceIds(organizationId)
Expand Down
89 changes: 85 additions & 4 deletions apps/sim/app/api/v1/audit-logs/auth.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,21 +11,34 @@ import {
} from '@sim/testing'
import { afterAll, beforeEach, describe, expect, it, vi } from 'vitest'

const { mockIsOrganizationBillingBlocked } = vi.hoisted(() => ({
mockIsOrganizationBillingBlocked: vi.fn(),
}))
const { mockIsOrganizationBillingBlocked, mockCheckOrganizationPersonalKeyRefusal } = vi.hoisted(
() => ({
mockIsOrganizationBillingBlocked: vi.fn(),
mockCheckOrganizationPersonalKeyRefusal: vi.fn(),
})
)

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

import { validateEnterpriseAuditAccess } from '@/app/api/v1/audit-logs/auth'
vi.mock('@/app/api/v1/middleware', () => ({
capabilityGovernedUserId: (rateLimit: { keyType?: string; userId?: string }) =>
rateLimit.keyType === 'personal' ? (rateLimit.userId ?? null) : null,
checkOrganizationPersonalKeyRefusal: mockCheckOrganizationPersonalKeyRefusal,
}))

import {
validateEnterpriseAuditAccess,
validateV1EnterpriseAuditAccess,
} from '@/app/api/v1/audit-logs/auth'

describe('enterprise audit access', () => {
beforeEach(() => {
vi.clearAllMocks()
resetDbChainMock()
mockIsOrganizationBillingBlocked.mockResolvedValue(false)
mockCheckOrganizationPersonalKeyRefusal.mockResolvedValue(null)
})

afterAll(() => {
Expand Down Expand Up @@ -112,4 +125,72 @@ describe('enterprise audit access', () => {
})
})
})

describe('v1 API-key access', () => {
const personalKey = {
allowed: true,
remaining: 1,
limit: 1,
resetAt: new Date(),
userId: 'viewer',
keyType: 'personal' as const,
}

beforeEach(() => {
setEnvFlags({ isBillingEnabled: false, isAuditLogsEnabled: true })
})

it('refuses a workspace key before resolving its creator as the subject', async () => {
const result = await validateV1EnterpriseAuditAccess({
...personalKey,
keyType: 'workspace',
workspaceId: 'workspace-a',
})

if (result.success) throw new Error('Expected the workspace key to be refused')
expect(result.response.status).toBe(403)
await expect(result.response.json()).resolves.toEqual({
error: 'Audit logs require a personal API key',
})
expect(dbChainMockFns.where).not.toHaveBeenCalled()
expect(mockCheckOrganizationPersonalKeyRefusal).not.toHaveBeenCalled()
})

it('authorizes a personal key held by an organization admin', async () => {
queueTableRows(schemaMock.member, [{ organizationId: 'org-1', role: 'admin' }])
queueTableRows(schemaMock.member, [{ userId: 'viewer' }])

await expect(validateV1EnterpriseAuditAccess(personalKey)).resolves.toEqual({
success: true,
userId: 'viewer',
context: { organizationId: 'org-1', orgMemberIds: ['viewer'] },
})
expect(mockCheckOrganizationPersonalKeyRefusal).toHaveBeenCalledWith(personalKey)
})

it('refuses a personal key its permission group withholds', async () => {
queueTableRows(schemaMock.member, [{ organizationId: 'org-1', role: 'admin' }])
queueTableRows(schemaMock.member, [{ userId: 'viewer' }])
const refusal = new Response(null, { status: 403 })
mockCheckOrganizationPersonalKeyRefusal.mockResolvedValue(refusal)

const result = await validateV1EnterpriseAuditAccess(personalKey)

if (result.success) throw new Error('Expected the withheld personal key to be refused')
expect(result.response).toBe(refusal)
})

it('answers a non-admin with the role refusal, not the group configuration', async () => {
queueTableRows(schemaMock.member, [{ organizationId: 'org-1', role: 'member' }])
mockCheckOrganizationPersonalKeyRefusal.mockResolvedValue(new Response(null, { status: 403 }))

const result = await validateV1EnterpriseAuditAccess(personalKey)

if (result.success) throw new Error('Expected the non-admin to be refused')
await expect(result.response.json()).resolves.toEqual({
error: 'Organization admin or owner role required',
})
expect(mockCheckOrganizationPersonalKeyRefusal).not.toHaveBeenCalled()
})
})
})
43 changes: 43 additions & 0 deletions apps/sim/app/api/v1/audit-logs/auth.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,20 @@ import {
type EnterpriseAuditContext,
resolveEnterpriseAuditAccess,
} from '@/lib/audit-logs/authorization'
import {
capabilityGovernedUserId,
checkOrganizationPersonalKeyRefusal,
type RateLimitResult,
} from '@/app/api/v1/middleware'

type AuthResult =
| { success: true; context: EnterpriseAuditContext }
| { success: false; response: NextResponse }

type V1AuthResult =
| { success: true; userId: string; context: EnterpriseAuditContext }
| { success: false; response: NextResponse }

/**
* v1 wrapper: renders {@link resolveEnterpriseAuditAccess} as the v1 `{ error }`
* response body.
Expand All @@ -23,3 +32,37 @@ export async function validateEnterpriseAuditAccess(
response: NextResponse.json({ error: result.message }, { status: result.status }),
}
}

/**
* Authorizes a v1 API-key read of the organization audit trail with the same
* policy as `auditLogOperations`, which v1 does not route through.
*
* Workspace keys are refused (`workspaceApiKey: 'deny'`): their `userId` is the
* key's creator, so authorizing it would let a credential scoped to one
* workspace read every workspace in the organization whenever its creator is an
* organization admin. A personal key is then held to the user-global
* `personal_api_key.use` group decision, checked after the admin role so the
* refusal never describes an organization's configuration to a non-admin.
*/
export async function validateV1EnterpriseAuditAccess(
rateLimit: RateLimitResult
): Promise<V1AuthResult> {
const userId = capabilityGovernedUserId(rateLimit)
if (!userId) {
return {
success: false,
response: NextResponse.json(
{ error: 'Audit logs require a personal API key' },
{ status: 403 }
),
}
}

const access = await validateEnterpriseAuditAccess(userId)
if (!access.success) return access

const personalKeyRefusal = await checkOrganizationPersonalKeyRefusal(rateLimit)
if (personalKeyRefusal) return { success: false, response: personalKeyRefusal }

return { success: true, userId, context: access.context }
}
35 changes: 30 additions & 5 deletions apps/sim/app/api/v1/audit-logs/route.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,14 +9,14 @@ import { beforeEach, describe, expect, it, vi } from 'vitest'

const {
mockCheckRateLimit,
mockValidateEnterpriseAuditAccess,
mockValidateV1EnterpriseAuditAccess,
mockBuildOrgScopeCondition,
mockGetOrgWorkspaceIds,
mockQueryAuditLogs,
mockBuildFilterConditions,
} = vi.hoisted(() => ({
mockCheckRateLimit: vi.fn(),
mockValidateEnterpriseAuditAccess: vi.fn(),
mockValidateV1EnterpriseAuditAccess: vi.fn(),
mockBuildOrgScopeCondition: vi.fn(),
mockGetOrgWorkspaceIds: vi.fn(),
mockQueryAuditLogs: vi.fn(),
Expand All @@ -31,7 +31,7 @@ vi.mock('@/app/api/v1/middleware', () => ({
}))

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

vi.mock('@/lib/audit-logs/query', () => ({
Expand Down Expand Up @@ -61,8 +61,9 @@ describe('GET /api/v1/audit-logs', () => {
beforeEach(() => {
vi.clearAllMocks()
mockCheckRateLimit.mockResolvedValue({ allowed: true, userId: 'admin-1' })
mockValidateEnterpriseAuditAccess.mockResolvedValue({
mockValidateV1EnterpriseAuditAccess.mockResolvedValue({
success: true,
userId: 'admin-1',
context: { organizationId: ORG_ID, orgMemberIds: MEMBER_IDS },
})
mockGetOrgWorkspaceIds.mockResolvedValue(ORG_WORKSPACE_IDS)
Expand Down Expand Up @@ -122,11 +123,35 @@ describe('GET /api/v1/audit-logs', () => {

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

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

expect(response.status).toBe(403)
expect(mockQueryAuditLogs).not.toHaveBeenCalled()
})

it('returns the refusal for a workspace key without querying', async () => {
mockCheckRateLimit.mockResolvedValue({
allowed: true,
userId: 'admin-1',
keyType: 'workspace',
workspaceId: 'ws-org-1',
})
const denied = new Response(
JSON.stringify({ error: 'Audit logs require a personal API key' }),
{
status: 403,
}
)
mockValidateV1EnterpriseAuditAccess.mockResolvedValue({ success: false, response: denied })

const response = await GET(makeRequest('?workspaceId=ws-org-2'))

expect(response.status).toBe(403)
expect(mockValidateV1EnterpriseAuditAccess).toHaveBeenCalledWith(
expect.objectContaining({ keyType: 'workspace' })
)
expect(mockQueryAuditLogs).not.toHaveBeenCalled()
})
})
7 changes: 3 additions & 4 deletions apps/sim/app/api/v1/audit-logs/route.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ import {
queryAuditLogs,
} from '@/lib/audit-logs/query'
import { withRouteHandler } from '@/lib/core/utils/with-route-handler'
import { validateEnterpriseAuditAccess } from '@/app/api/v1/audit-logs/auth'
import { validateV1EnterpriseAuditAccess } from '@/app/api/v1/audit-logs/auth'
import { formatAuditLogEntry } from '@/app/api/v1/audit-logs/format'
import { createApiResponse, getUserLimits } from '@/app/api/v1/logs/meta'
import {
Expand Down Expand Up @@ -63,13 +63,12 @@ export const GET = withRouteHandler(async (request: NextRequest) => {
return createRateLimitResponse(rateLimit)
}

const userId = rateLimit.userId!

const authResult = await validateEnterpriseAuditAccess(userId)
const authResult = await validateV1EnterpriseAuditAccess(rateLimit)
if (!authResult.success) {
return authResult.response
}

const { userId } = authResult
const { organizationId, orgMemberIds } = authResult.context

const parsed = await parseRequest(
Expand Down
Loading
Loading