diff --git a/packages/audit/lib/audit.ts b/packages/audit/lib/audit.ts index e848558497..f3ab38fe2c 100644 --- a/packages/audit/lib/audit.ts +++ b/packages/audit/lib/audit.ts @@ -66,21 +66,25 @@ export class AuditClient { /** * Account-scoped, most-recent-first. `cursor` is the opaque `nextCursor` of a previous page; - * `from`/`to` optionally bound the window and combine with the cursor. Empty when audit isn't wired - * to a backend. + * `from`/`to` and the resource filters optionally narrow the set and combine with the cursor. Empty + * when audit isn't wired to a backend. */ async listAuditTrailEvents({ accountId, limit, cursor, from, - to + to, + resources, + actions }: { accountId: number; limit: number; cursor?: string | undefined; from?: string | undefined; to?: string | undefined; + resources?: string[] | undefined; + actions?: string[] | undefined; }): Promise> { let before: AuditTrailCursor | undefined; if (cursor) { @@ -91,7 +95,7 @@ export class AuditClient { before = decoded; } - return (await this.reader.list({ accountId, limit, before, from, to })).map((page) => ({ + return (await this.reader.list({ accountId, limit, before, from, to, resources, actions })).map((page) => ({ events: page.events, nextCursor: page.nextCursor ? encodeCursor(page.nextCursor) : null })); diff --git a/packages/audit/lib/migrate.integration.test.ts b/packages/audit/lib/migrate.integration.test.ts index 5861dcb48a..012e32bf78 100644 --- a/packages/audit/lib/migrate.integration.test.ts +++ b/packages/audit/lib/migrate.integration.test.ts @@ -43,6 +43,7 @@ describe('audit migrate', () => { expect(await tables()).toEqual(['audit_trail_events', 'migrations']); expect(await appliedMigrations()).toEqual([ expect.stringMatching(/^20260729000001_create_audit_trail_events\.[jt]s$/), + expect.stringMatching(/^20260731000001_add_resource_action_search_columns\.[jt]s$/), expect.stringMatching(/^20260731000002_allow_account_id_zero\.[jt]s$/) ]); }); @@ -52,7 +53,19 @@ describe('audit migrate', () => { // Deliberately no FINAL: re-applying inserts a second row for the same name, which FINAL would hide. const res = await admin.query({ query: `SELECT count() AS count FROM ${database}.migrations`, format: 'JSONEachRow' }); - expect(Number((await res.json<{ count: string }>())[0]!.count)).toBe(2); + expect(Number((await res.json<{ count: string }>())[0]!.count)).toBe(3); + }); + + it('creates a set index for each of the resource search columns', async () => { + const res = await admin.query({ + query: `SELECT name, type_full AS type FROM system.data_skipping_indices WHERE database = {db:String} AND table = 'audit_trail_events' ORDER BY name`, + format: 'JSONEachRow', + query_params: { db: database } + }); + expect(await res.json<{ name: string; type: string }>()).toEqual([ + { name: 'idx_resource', type: 'set(0)' }, + { name: 'idx_resource_action', type: 'set(0)' } + ]); }); it('rejects an accountId that is missing, malformed or negative, and accepts account 0', async () => { diff --git a/packages/audit/lib/migrations/20260731000001_add_resource_action_search_columns.ts b/packages/audit/lib/migrations/20260731000001_add_resource_action_search_columns.ts new file mode 100644 index 0000000000..d481d5e053 --- /dev/null +++ b/packages/audit/lib/migrations/20260731000001_add_resource_action_search_columns.ts @@ -0,0 +1,14 @@ +// Non-throwing extracts, unlike the ORDER BY keys: a blob these can't read is still a storable event. +// `resource_action` is its own column because two `set` indexes AND-ed prune per column, not per pair. +export const sql = [ + ` + ALTER TABLE {database:Identifier}.audit_trail_events + ADD COLUMN IF NOT EXISTS resource LowCardinality(String) MATERIALIZED JSONExtractString(event, 'resource'), + ADD COLUMN IF NOT EXISTS resource_action LowCardinality(String) MATERIALIZED concat(JSONExtractString(event, 'resource'), '.', JSONExtractString(event, 'action')) + `, + ` + ALTER TABLE {database:Identifier}.audit_trail_events + ADD INDEX IF NOT EXISTS idx_resource resource TYPE set(0) GRANULARITY 1, + ADD INDEX IF NOT EXISTS idx_resource_action resource_action TYPE set(0) GRANULARITY 1 + ` +]; diff --git a/packages/audit/lib/store.integration.test.ts b/packages/audit/lib/store.integration.test.ts index 7658418dc2..79772e1bb4 100644 --- a/packages/audit/lib/store.integration.test.ts +++ b/packages/audit/lib/store.integration.test.ts @@ -18,7 +18,19 @@ let client: ClickHouseClient; let store: ClickhouseAuditStore; // Known ids so the read assertions below are deterministic. -async function insertEvent({ id, accountId, occurredAt }: { id: string; accountId: number; occurredAt: string }) { +async function insertEvent({ + id, + accountId, + occurredAt, + resource = 'connection', + action = 'deleted' +}: { + id: string; + accountId: number; + occurredAt: string; + resource?: string; + action?: string; +}) { const event = { id, version: '2026-07-16', @@ -26,8 +38,8 @@ async function insertEvent({ id, accountId, occurredAt }: { id: string; accountI accountId, environment: null, actor: { type: 'user', id: '5', display: 'a@b.co' }, - resource: 'connection', - action: 'deleted', + resource, + action, targets: [{ type: 'connection', id: '10' }], context: {}, outcome: 'success' @@ -55,6 +67,12 @@ beforeAll(async () => { await insertEvent({ id: '22222222-2222-2222-2222-222222222222', accountId: 1, occurredAt: at(1000) }); await insertEvent({ id: '33333333-3333-3333-3333-333333333333', accountId: 1, occurredAt: at(2000) }); await insertEvent({ id: '99999999-9999-9999-9999-999999999999', accountId: 2, occurredAt: at(1500) }); + + // account 3: one event per resource/action pair the filter tests select on + await insertEvent({ id: 'aaaaaaaa-0000-0000-0000-000000000001', accountId: 3, occurredAt: at(0), resource: 'connection', action: 'deleted' }); + await insertEvent({ id: 'aaaaaaaa-0000-0000-0000-000000000002', accountId: 3, occurredAt: at(1000), resource: 'connection', action: 'updated' }); + await insertEvent({ id: 'aaaaaaaa-0000-0000-0000-000000000003', accountId: 3, occurredAt: at(2000), resource: 'api_key', action: 'deleted' }); + await insertEvent({ id: 'aaaaaaaa-0000-0000-0000-000000000004', accountId: 3, occurredAt: at(3000), resource: 'sync', action: 'enabled' }); }); afterAll(async () => { @@ -94,6 +112,56 @@ describe('ClickhouseAuditStore.list', () => { }); }); +describe('ClickhouseAuditStore.list resource filters', () => { + const resourceActionOf = (events: { resource: string; action: string }[]) => events.map((e) => `${e.resource}.${e.action}`).sort(); + + it('filters by resource', async () => { + const { events } = (await store.list({ accountId: 3, limit: 10, resources: ['connection'] })).unwrap(); + expect(resourceActionOf(events)).toEqual(['connection.deleted', 'connection.updated']); + expect(events.every((e) => e.accountId === 3)).toBe(true); + }); + + it('filters by several resources at once', async () => { + const { events } = (await store.list({ accountId: 3, limit: 10, resources: ['api_key', 'sync'] })).unwrap(); + expect(resourceActionOf(events)).toEqual(['api_key.deleted', 'sync.enabled']); + }); + + it('narrows a resource to a single action', async () => { + const { events } = (await store.list({ accountId: 3, limit: 10, resources: ['connection'], actions: ['deleted'] })).unwrap(); + expect(resourceActionOf(events)).toEqual(['connection.deleted']); + }); + + // The pairs are the cross product, so an action belonging to another resource must not widen the match. + it('matches actions against their own resource only', async () => { + const onSync = (await store.list({ accountId: 3, limit: 10, resources: ['sync'], actions: ['enabled'] })).unwrap(); + expect(resourceActionOf(onSync.events)).toEqual(['sync.enabled']); + + // Same action, a resource that never records it: the pair matches nothing rather than falling + // back to either half. + const onConnection = (await store.list({ accountId: 3, limit: 10, resources: ['connection'], actions: ['enabled'] })).unwrap(); + expect(onConnection.events).toHaveLength(0); + }); + + it('ignores actions given without a resource, since a pair needs both halves', async () => { + const { events } = (await store.list({ accountId: 3, limit: 10, actions: ['deleted'] })).unwrap(); + expect(resourceActionOf(events)).toEqual(['api_key.deleted', 'connection.deleted', 'connection.updated', 'sync.enabled']); + }); + + it('combines with the date window and paginates', async () => { + const page1 = (await store.list({ accountId: 3, limit: 1, resources: ['connection'], from: at(0), to: at(1000) })).unwrap(); + expect(resourceActionOf(page1.events)).toEqual(['connection.updated']); + expect(page1.nextCursor).not.toBeNull(); + + const page2 = (await store.list({ accountId: 3, limit: 1, resources: ['connection'], from: at(0), to: at(1000), before: page1.nextCursor! })).unwrap(); + expect(resourceActionOf(page2.events)).toEqual(['connection.deleted']); + }); + + it('returns nothing for a resource that was never recorded', async () => { + const { events } = (await store.list({ accountId: 3, limit: 10, resources: ['team'] })).unwrap(); + expect(events).toHaveLength(0); + }); +}); + describe('AuditClient.record through ClickhouseAuditStore', () => { it('writes an emitted event that reads back with the id + version stamped at emit', async () => { const event: AuditEvent = { diff --git a/packages/audit/lib/store.ts b/packages/audit/lib/store.ts index a4f28adc15..91b54253c3 100644 --- a/packages/audit/lib/store.ts +++ b/packages/audit/lib/store.ts @@ -24,6 +24,9 @@ export interface ListAuditTrailEventsParams { before?: AuditTrailCursor | undefined; from?: string | undefined; to?: string | undefined; + resources?: string[] | undefined; + /** Narrows `resources`; ignored on its own, since a match needs both halves of `resource.action`. */ + actions?: string[] | undefined; } export interface AuditTrailPage { @@ -94,9 +97,20 @@ export class ClickhouseAuditStore implements AuditWriter, AuditBatchWriter, Audi } } - async list({ accountId, limit, before, from, to }: ListAuditTrailEventsParams): Promise> { + async list({ accountId, limit, before, from, to, resources, actions }: ListAuditTrailEventsParams): Promise> { const params: Record = { account_id: accountId, limit: limit + 1 }; const conditions = ['account_id = {account_id:Int64}']; + if (resources?.length) { + if (actions?.length) { + // Every requested pair, matched against the materialized concatenation. Two separate + // conditions would prune per column and let a granule through on a pair it doesn't hold. + conditions.push('resource_action IN {resource_actions:Array(String)}'); + params['resource_actions'] = resources.flatMap((resource) => actions.map((action) => `${resource}.${action}`)); + } else { + conditions.push('resource IN {resources:Array(String)}'); + params['resources'] = resources; + } + } if (from) { conditions.push('occurred_at >= parseDateTime64BestEffortOrNull({from:String}, 3)'); params['from'] = from; diff --git a/packages/server/lib/controllers/v1/audit-trail/getAuditTrail.integration.test.ts b/packages/server/lib/controllers/v1/audit-trail/getAuditTrail.integration.test.ts index f806994443..5e9038c385 100644 --- a/packages/server/lib/controllers/v1/audit-trail/getAuditTrail.integration.test.ts +++ b/packages/server/lib/controllers/v1/audit-trail/getAuditTrail.integration.test.ts @@ -5,7 +5,7 @@ import { seeders } from '@nangohq/shared'; import { authenticateUser, isSuccess, runServer } from '../../../utils/tests.js'; -import type { AuditEvent } from '@nangohq/audit'; +import type { AuditEvent, AuditResourceAction } from '@nangohq/audit'; let api: Awaited>; let auditClient: ReturnType; @@ -18,17 +18,16 @@ async function authAdmin() { return { session, account, env }; } -function auditEvent(accountId: number, occurredAt: string): AuditEvent { +function auditEvent(accountId: number, occurredAt: string, resourceAction: AuditResourceAction = { resource: 'connection', action: 'deleted' }): AuditEvent { return { occurredAt, accountId, environment: null, actor: { type: 'user', id: '5', display: 'a@b.co' }, - resource: 'connection', - action: 'deleted', targets: [{ type: 'connection', id: '10' }], context: {}, - outcome: 'success' + outcome: 'success', + ...resourceAction }; } @@ -111,6 +110,52 @@ describe('GET /api/v1/audit-trail', () => { expect(event.action).toBe('deleted'); }); + it('caps how many values one filter param can carry', async () => { + const { session } = await authAdmin(); + const values = (n: number) => Array.from({ length: n }, (_, i) => `resource_${i}`).join(','); + + const atCap = await api.fetch('/api/v1/audit-trail', { method: 'GET', session, query: { resources: values(50) } }); + expect(atCap.res.status).toBe(200); + + const overCap = await api.fetch('/api/v1/audit-trail', { method: 'GET', session, query: { resources: values(51) } }); + expect(overCap.res.status).toBe(400); + }); + + it('rejects `actions` sent without `resources` with 400', async () => { + const { session } = await authAdmin(); + const res = await api.fetch('/api/v1/audit-trail', { method: 'GET', session, query: { actions: 'deleted' } }); + expect(res.res.status).toBe(400); + }); + + it('filters by resource, and by a resource narrowed to an action', async () => { + const { session, account } = await authAdmin(); + (await emitter.record(auditEvent(account.id, '2026-07-16T10:00:00.000Z', { resource: 'connection', action: 'deleted' }))).unwrap(); + (await emitter.record(auditEvent(account.id, '2026-07-16T10:00:01.000Z', { resource: 'connection', action: 'updated' }))).unwrap(); + (await emitter.record(auditEvent(account.id, '2026-07-16T10:00:02.000Z', { resource: 'api_key', action: 'deleted' }))).unwrap(); + + const byResource = await api.fetch('/api/v1/audit-trail', { method: 'GET', session, query: { resources: 'connection' } }); + expect(byResource.res.status).toBe(200); + isSuccess(byResource.json); + expect(byResource.json.data.map((e) => `${e.resource}.${e.action}`).sort()).toEqual(['connection.deleted', 'connection.updated']); + + const byPair = await api.fetch('/api/v1/audit-trail', { method: 'GET', session, query: { resources: 'connection', actions: 'deleted' } }); + expect(byPair.res.status).toBe(200); + isSuccess(byPair.json); + expect(byPair.json.data.map((e) => `${e.resource}.${e.action}`)).toEqual(['connection.deleted']); + }); + + it('filters by several resources at once', async () => { + const { session, account } = await authAdmin(); + (await emitter.record(auditEvent(account.id, '2026-07-16T10:00:00.000Z', { resource: 'connection', action: 'deleted' }))).unwrap(); + (await emitter.record(auditEvent(account.id, '2026-07-16T10:00:01.000Z', { resource: 'api_key', action: 'deleted' }))).unwrap(); + (await emitter.record(auditEvent(account.id, '2026-07-16T10:00:02.000Z', { resource: 'team', action: 'updated' }))).unwrap(); + + const res = await api.fetch('/api/v1/audit-trail', { method: 'GET', session, query: { resources: 'connection,api_key' } }); + expect(res.res.status).toBe(200); + isSuccess(res.json); + expect(res.json.data.map((e) => e.resource).sort()).toEqual(['api_key', 'connection']); + }); + it('paginates via the opaque cursor', async () => { const { session, account } = await authAdmin(); // 26 events one second apart (oldest → newest) — one more than the fixed page size of 25. diff --git a/packages/server/lib/controllers/v1/audit-trail/getAuditTrail.ts b/packages/server/lib/controllers/v1/audit-trail/getAuditTrail.ts index 87d80176e2..6e838540c1 100644 --- a/packages/server/lib/controllers/v1/audit-trail/getAuditTrail.ts +++ b/packages/server/lib/controllers/v1/audit-trail/getAuditTrail.ts @@ -10,15 +10,30 @@ import type { GetAuditTrail } from '@nangohq/types'; const PAGE_SIZE = 25; +const MAX_FILTER_VALUES = 50; + +// Comma-separated list (`?resources=a,b`). Safe to split on a comma because both vocabularies are +// snake_case identifiers. Unlike the enum-valued query params elsewhere the values aren't checked +// against a vocabulary — the audit one has no runtime form — so an unknown value matches nothing. +const csvParam = z + .string() + .transform((value) => value.split(',')) + .pipe(z.array(z.string().min(1)).max(MAX_FILTER_VALUES)); + const queryStringValidation = z .object({ cursor: z.string().optional(), from: z.iso.datetime().optional(), - to: z.iso.datetime().optional() + to: z.iso.datetime().optional(), + resources: csvParam.optional(), + actions: csvParam.optional() }) // Account-scoped endpoint (no `env`). Not strict: any stray query param is stripped rather than 400'd, so a read never fails over an extra key. // Surface an inverted range as a 400 rather than a silently empty result. - .refine((q) => !q.from || !q.to || new Date(q.from) <= new Date(q.to), { message: '`from` must be before or equal to `to`', path: ['from'] }); + .refine((q) => !q.from || !q.to || new Date(q.from) <= new Date(q.to), { message: '`from` must be before or equal to `to`', path: ['from'] }) + // An action is only meaningful attached to a resource, so reject the pairless form rather than + // dropping it and returning a wider result set than the caller asked for. + .refine((q) => !q.actions?.length || Boolean(q.resources?.length), { message: '`actions` requires `resources`', path: ['actions'] }); export const getAuditTrail = asyncWrapper(async (req, res) => { const query = queryStringValidation.safeParse(req.query); @@ -28,9 +43,9 @@ export const getAuditTrail = asyncWrapper(async (req, res) => { } const { account } = res.locals; - const { cursor, from, to } = query.data; + const { cursor, from, to, resources, actions } = query.data; - const result = await audit.listAuditTrailEvents({ accountId: account.id, limit: PAGE_SIZE, cursor, from, to }); + const result = await audit.listAuditTrailEvents({ accountId: account.id, limit: PAGE_SIZE, cursor, from, to, resources, actions }); if (result.isErr()) { if (result.error instanceof InvalidAuditCursorError) { res.status(400).send({ error: { code: 'invalid_query_params', message: 'Invalid cursor' } }); diff --git a/packages/types/lib/audit-trail/api.ts b/packages/types/lib/audit-trail/api.ts index bd3566b1bd..9068176b7a 100644 --- a/packages/types/lib/audit-trail/api.ts +++ b/packages/types/lib/audit-trail/api.ts @@ -26,11 +26,15 @@ export type GetAuditTrail = ApiEndpoint<{ Method: 'GET'; Path: '/api/v1/audit-trail'; Querystring: { - // Account-scoped endpoint: no `env`. `cursor` encodes position only, not the filter window — resend the - // same `from`/`to` on every page or subsequent pages paginate the unfiltered set past the cursor. + // Account-scoped endpoint: no `env`. `cursor` encodes position only, not the filter window — resend + // every other param on each page or subsequent pages paginate the unfiltered set past the cursor. cursor?: string; from?: string; to?: string; + // Comma-separated `AuditResource` / `AuditAction` values (`?resources=connection,sync`). + // `actions` requires `resources`: the pair is matched as one `resource.action` value. + resources?: string; + actions?: string; }; Success: { data: ApiAuditTrailEvent[]; diff --git a/packages/webapp/src/app/router.tsx b/packages/webapp/src/app/router.tsx index ff65bfa7c3..cdc1c86d87 100644 --- a/packages/webapp/src/app/router.tsx +++ b/packages/webapp/src/app/router.tsx @@ -189,7 +189,7 @@ export const router = sentryCreateBrowserRouter([ { path: '/team/audit', element: , - handle: { breadcrumb: 'Audit log' } as BreadcrumbHandle + handle: { breadcrumb: 'Audit trail' } as BreadcrumbHandle }, { path: '/:env', diff --git a/packages/webapp/src/components/patterns/FilterMultiSelect.tsx b/packages/webapp/src/components/patterns/FilterMultiSelect.tsx index e685ed3aa6..f6810593df 100644 --- a/packages/webapp/src/components/patterns/FilterMultiSelect.tsx +++ b/packages/webapp/src/components/patterns/FilterMultiSelect.tsx @@ -26,6 +26,7 @@ interface FilterMultiSelectProps { width?: string; open?: boolean; onOpenChange?: (open: boolean) => void; + disabled?: boolean; } export function FilterMultiSelect({ @@ -40,7 +41,8 @@ export function FilterMultiSelect({ max, width = 'w-56', open: controlledOpen, - onOpenChange: controlledOnOpenChange + onOpenChange: controlledOnOpenChange, + disabled = false }: FilterMultiSelectProps) { const [internalOpen, setInternalOpen] = useState(false); const [search, setSearch] = useState(''); @@ -48,7 +50,7 @@ export function FilterMultiSelect({ const searchInputRef = useRef(null); const isControlled = controlledOpen !== undefined; - const open = isControlled ? controlledOpen : internalOpen; + const open = (isControlled ? controlledOpen : internalOpen) && !disabled; const setOpen = useCallback( (val: boolean) => { @@ -179,7 +181,7 @@ export function FilterMultiSelect({ return ( -