diff --git a/_bmad-output/implementation-artifacts/tech-spec-audit-trail-mvp.md b/_bmad-output/implementation-artifacts/tech-spec-audit-trail-mvp.md new file mode 100644 index 0000000..9cf3184 --- /dev/null +++ b/_bmad-output/implementation-artifacts/tech-spec-audit-trail-mvp.md @@ -0,0 +1,469 @@ +--- +title: 'Audit Trail MVP' +slug: 'audit-trail-mvp' +created: '2026-03-29' +status: 'completed' +stepsCompleted: [1, 2, 3, 4] +tech_stack: + [ + 'NestJS', + 'BullMQ', + 'MikroORM', + 'PostgreSQL', + 'nestjs-cls', + 'Zod', + 'Passport/JWT', + ] +files_to_modify: + - 'src/configurations/common/queue-names.ts' + - 'src/modules/index.module.ts' + - 'src/entities/audit-log.entity.ts (NEW)' + - 'src/entities/index.entity.ts' + - 'src/modules/audit/audit.module.ts (NEW)' + - 'src/modules/audit/audit.service.ts (NEW)' + - 'src/modules/audit/audit.processor.ts (NEW)' + - 'src/modules/audit/audit-action.enum.ts (NEW)' + - 'src/modules/audit/decorators/audited.decorator.ts (NEW)' + - 'src/modules/audit/interceptors/audit.interceptor.ts (NEW)' + - 'src/modules/audit/dto/audit-job-message.dto.ts (NEW)' + - 'src/modules/auth/auth.service.ts' + - 'src/modules/auth/auth.controller.ts' + - 'src/modules/moodle/controllers/moodle-sync.controller.ts' + - 'src/modules/questionnaires/questionnaire.controller.ts' + - 'src/modules/analysis/analysis.controller.ts' + - 'migration file (NEW)' +code_patterns: + - 'BullMQ queue-per-type: QueueName const enum in queue-names.ts, BullModule.registerQueue() in module' + - 'Processor: @Processor(QueueName.X, { concurrency }) extends WorkerHost, em.fork() then create() + flush()' + - 'CLS context: CurrentUserService.get() for user, RequestMetadataService.get() for IP/browser/OS' + - 'Append-only entity: SyncLog pattern — no CustomBaseEntity, no soft delete, own PK/timestamps' + - 'Custom decorator: SetMetadata(KEY, value) with exported KEY constant' + - 'Composite decorator: applyDecorators() to combine multiple decorators' + - 'Job enqueueing: @InjectQueue(QueueName.X), queue.add(name, envelope, { jobId, attempts, backoff })' + - 'Direct emit for non-interceptor contexts (auth failures in catch blocks)' +test_patterns: + - 'NestJS TestingModule with Jest mocks: { provide: Dep, useValue: { method: jest.fn() } }' + - 'Auth tests mock CustomJwtService, UnitOfWork, CurrentUserService, RequestMetadataService' + - 'Controller tests override JWT/role guards' + - 'Strategy tests validate error handling paths' +--- + +# Tech-Spec: Audit Trail MVP + +**Created:** 2026-03-29 + +## Overview + +### Problem Statement + +The platform has no visibility into who performed security-sensitive actions — auth events, admin configuration changes, or sensitive data mutations. There is no audit log for compliance, security incident investigation, or operational accountability. + +### Solution + +Add an append-only `AuditLog` entity backed by a dedicated BullMQ `AUDIT` queue. Capture audit events through two emission paths: + +1. **Interceptor path** — An `@Audited({ action, resource? })` decorator on endpoints triggers an interceptor that auto-captures context (user, IP, route params) and enqueues an audit event post-response. Used for authenticated endpoints where CLS context is available. +2. **Direct emit path** — `AuditService.Emit()` called explicitly in service code for contexts where the interceptor can't capture full context (e.g., login success/failure where no JWT exists, token refresh, catch blocks). + +Both paths feed the same queue, processor, and entity. Write-only pipeline for the MVP. + +### Scope + +**In Scope:** + +- `AuditLog` entity (append-only, immutable, no soft delete) +- BullMQ `AUDIT` queue and processor (concurrency: 1) +- `AuditService` with `Emit()` method for direct emission +- `@Audited()` decorator and `AuditInterceptor` for endpoint-based capture +- 11 MVP endpoints tagged across three categories: + - **Auth events**: login success/failure, logout, token refresh + - **Admin actions**: sync schedule changes, config updates, user management + - **Sensitive data mutations**: questionnaire submissions, analysis job dispatch + +**Out of Scope:** + +- Admin query/filter endpoint for audit logs +- Before/after diff capture on entity updates +- Retention policy / cleanup job +- Broader CRUD auditing beyond MVP endpoints +- Export/download of audit logs + +## Context for Development + +### Codebase Patterns + +- **BullMQ queue-per-type**: Each analysis type gets its own queue registered via `BullModule.registerQueue()` in the module. Queue names are centralized in `src/configurations/common/queue-names.ts`. +- **Processor pattern**: Analysis processors extend `RunPodBatchProcessor` (which extends `WorkerHost` from `@nestjs/bullmq`). The audit processor will NOT use this hierarchy (no HTTP dispatch needed) — it extends `WorkerHost` directly and writes to the DB. +- **CLS context**: `nestjs-cls` provides request-scoped state. `CurrentUserService` holds the authenticated user; `RequestMetadataService` holds IP/browser/OS. Both are available in interceptors. +- **Append-only entity**: `SyncLog` is the precedent — no `CustomBaseEntity` extension, no soft delete, owns its own schema. +- **Decorator + interceptor**: The `@Audited()` decorator sets Reflector metadata on the handler. The `AuditInterceptor` reads it post-response and enqueues. Note: `MetaDataInterceptor` is NOT provided by `AuditModule` — it is `@Injectable()` and resolved from the host module's DI scope (each host module imports `CommonModule` which provides it). `AuditInterceptor` IS exported by `AuditModule` (global). + +### Files to Reference + +| File | Purpose | +| ------------------------------------------------------------- | -------------------------------------------------------------------------------------------------------------------------------- | +| `src/configurations/common/queue-names.ts` | Queue name enum — add `AUDIT` here. Uses `as const` pattern with derived type. | +| `src/modules/analysis/analysis.module.ts` | Pattern for `BullModule.registerQueue({ name: QueueName.X })` and provider registration | +| `src/modules/analysis/processors/sentiment.processor.ts` | Pattern for `@Processor(QueueName.X, { concurrency })`, `WorkerHost` extension, `em.fork()` | +| `src/modules/analysis/analysis.service.ts` | Pattern for `@InjectQueue()`, envelope format `{ jobId, version, type, metadata, publishedAt }`, job options | +| `src/entities/sync-log.entity.ts` | **Primary pattern**: append-only entity, no `CustomBaseEntity`, no soft delete, own `@PrimaryKey()` + timestamps | +| `src/entities/base.entity.ts` | `CustomBaseEntity` — audit entity does NOT extend this | +| `src/modules/common/interceptors/metadata.interceptor.ts` | Extracts IP (x-forwarded-for fallback), browser, OS via UAParser; stores in CLS | +| `src/modules/common/interceptors/current-user.interceptor.ts` | Loads User entity from DataLoader, stores in CLS via `CurrentUserService.set()` | +| `src/modules/common/cls/request-metadata.service.ts` | `RequestMetadata = { browserName, os, ipAddress }`, wraps `ClsService` with typed get/set | +| `src/modules/common/cls/current-user.service.ts` | `get()` returns `User \| null`, `getUserId()` extracts from JWT payload | +| `src/modules/common/cls/cls.module.ts` | `AppClsModule` exports both CLS services | +| `src/modules/index.module.ts` | `ApplicationModules` array — add `AuditModule` here | +| `src/security/decorators/roles.decorator.ts` | Pattern for `SetMetadata(KEY, value)` custom decorator | +| `src/security/decorators/index.ts` | Pattern for `applyDecorators()` composite decorator (`UseJwtGuard`) | +| `src/modules/auth/auth.controller.ts` | MVP endpoints: `POST /login`, `POST /logout`, `POST /refresh` | +| `src/modules/auth/auth.service.ts` | Login strategy execution, failure paths at lines 51-54 (no strategy match), refresh token validation | +| `src/modules/auth/strategies/local-login.strategy.ts` | Throws `UnauthorizedException` on invalid credentials — direct emit audit point | +| `src/modules/auth/strategies/moodle-login.strategy.ts` | Catches `MoodleConnectivityError` — direct emit audit point | +| `src/modules/moodle/controllers/moodle-sync.controller.ts` | MVP endpoints: `POST /moodle/sync` (manual trigger, superadmin), `PUT /moodle/sync/schedule` (superadmin) | +| `src/modules/questionnaires/questionnaire.controller.ts` | MVP endpoints: `POST /submissions`, `POST /ingest` (bulk CSV, superadmin), `DELETE /versions/:id/submissions` (wipe, superadmin) | +| `src/modules/analysis/analysis.controller.ts` | MVP endpoints: `POST /pipelines` (create), `POST /pipelines/:id/confirm`, `POST /pipelines/:id/cancel` | + +### Technical Decisions + +- **Entity does NOT extend `CustomBaseEntity`**: Audit records are immutable and never soft-deleted. Following the `SyncLog` precedent. +- **Processor does NOT extend `RunPodBatchProcessor` or `BaseAnalysisProcessor`**: No HTTP dispatch to external workers. Simple DB persist via `const fork = em.fork(); fork.create(AuditLog, {...}); await fork.flush();` — matching the codebase's `create()` + `flush()` pattern (not `persistAndFlush()`). +- **Concurrency: 1**: Audit inserts are lightweight. Single concurrency avoids contention and is sufficient for MVP volume. +- **JSONB `metadata` field**: Flexible bag for action-specific details. Keeps schema stable across action types. Expected shapes per action: + + | Action | Metadata Shape | Source | + | -------------------------------- | ----------------------------------------------------------------- | ------------------------- | + | `auth.login.success` | `{ strategyUsed: string }` | Direct emit | + | `auth.login.failure` | `{ username: string, reason: string }` | Direct emit | + | `auth.logout` | `{}` (no route params or query) | Interceptor | + | `auth.token.refresh` | `{}` (no extra context) | Direct emit | + | `admin.sync.trigger` | `{}` (no route params) | Interceptor | + | `admin.sync-schedule.update` | `{}` (body-only endpoint, no route params or query) | Interceptor | + | `questionnaire.submit` | `{}` (no route params) | Interceptor | + | `questionnaire.ingest` | `{}` (body excluded, no route params) | Interceptor | + | `questionnaire.submissions.wipe` | `{ versionId: string }` | Interceptor (from params) | + | `analysis.pipeline.create` | `{}` (no route params — pipeline ID is in response, not captured) | Interceptor | + | `analysis.pipeline.confirm` | `{ id: string }` | Interceptor (from params) | + | `analysis.pipeline.cancel` | `{ id: string }` | Interceptor (from params) | + +- **Denormalized `actorUsername`**: Users can be renamed; audit records preserve the username at time of action. +- **Two emission paths**: Interceptor for standard authenticated endpoints; direct `Emit()` for edge cases (failed logins, service-level events). + +## Implementation Plan + +### Tasks + +#### Phase 1: Foundation (Entity + Queue + Processor) + +- [x] Task 1: Add `AUDIT` queue name + - File: `src/configurations/common/queue-names.ts` + - Action: Add `AUDIT: 'audit'` to the `QueueName` const object + - Notes: Follows existing pattern (`SENTIMENT: 'sentiment'`, etc.) + +- [x] Task 2: Create `AuditLog` entity + - File: `src/entities/audit-log.entity.ts` (NEW) + - Action: Create append-only entity following `SyncLog` pattern (no `CustomBaseEntity`). Fields: + - `id: string` — `@PrimaryKey()`, default `v4()` + - `action: string` — e.g. `auth.login.success`, `admin.sync-schedule.update` + - `actorId: string` — nullable, **plain string column, NOT a `@ManyToOne` relation** (unlike `SyncLog.triggeredBy` which uses `@ManyToOne(() => User)`). We intentionally avoid a foreign key so audit records survive user deletion. + - `actorUsername: string` — nullable, denormalized for historical accuracy + - `resourceType: string` — nullable, e.g. `User`, `QuestionnaireSubmission` + - `resourceId: string` — nullable, UUID of affected resource + - `metadata: Record` — `@Property({ type: 'jsonb', nullable: true })` + - `browserName: string` — nullable + - `os: string` — nullable + - `ipAddress: string` — nullable + - `occurredAt: Date` — `@Index()`, **required (no JS-side default)**. The processor MUST always set this from the job payload. Omitting it should cause a runtime error, not a silent wrong timestamp. The PostgreSQL migration adds `DEFAULT now()` as a DB-level safety net only. + - Notes: No `updatedAt`, no `deletedAt`. Add comment on the entity class: "Audit records are never soft-deleted. Queries must use `filters: { softDelete: false }` to bypass the global filter. See SyncLog for precedent." No custom repository for MVP — follows `SyncLog` pattern which uses `filters: { softDelete: false }` at call sites. + +- [x] Task 3: Register entity in entity index + - File: `src/entities/index.entity.ts` + - Action: Export `AuditLog` from the entity barrel file AND add it to the `entities` array (used by MikroORM for schema discovery/migrations). Without this, `npx mikro-orm migration:create` will not detect the new entity. + +- [x] Task 4: Create database migration + - Command: `npx mikro-orm migration:create` + - Action: Create `audit_log` table with all columns from Task 2. Add index on `occurred_at`. Add index on `action`. + - Notes: Run `npx mikro-orm migration:create` after entity is created, verify generated SQL. Add a PostgreSQL-level `DEFAULT now()` on the `occurred_at` column as a second safety net (the entity default is JS-side only and fires at instantiation, not at DB insert time). + +- [x] Task 5: Create `AuditProcessor` + - File: `src/modules/audit/audit.processor.ts` (NEW) + - Action: Create processor that extends `WorkerHost` (NOT `BaseAnalysisProcessor`): + - `@Processor(QueueName.AUDIT, { concurrency: 1 })` + - Inject `EntityManager` + - `process(job: Job)`: extract fields from job data, `const fork = this.em.fork(); fork.create(AuditLog, { ...fields, occurredAt: new Date(job.data.occurredAt) }); await fork.flush();` — **set `occurredAt` from the job payload** (not `new Date()` — the entity default is only a safety net; the payload timestamp reflects actual event time, not delayed queue processing time). Use `create()` + `flush()` pattern to match codebase convention. + - `@OnWorkerEvent('failed')`: log error with job ID, attempt count, and **non-PII fields only**: `action`, `actorId`, `resourceType`, `resourceId`, `occurredAt`. Do NOT log `metadata` (may contain usernames from failed logins or other PII). This preserves audit traceability in application logs without leaking sensitive data. + - Notes: No HTTP dispatch. Direct DB write only. `attempts: 1` is set at enqueue time in `AuditService.Emit()` (Task 7) — the processor does NOT configure attempts. The `@Processor()` decorator only accepts `concurrency`, `stalledInterval`, etc. Add a `Logger.log` on successful processing (e.g., `Persisted audit log: ${job.data.action}`) for basic throughput observability — since `removeOnComplete: true` leaves no trace in Redis. + +- [x] Task 6: Create `AuditJobMessage` DTO + - File: `src/modules/audit/dto/audit-job-message.dto.ts` (NEW) + - Action: Define TypeScript interface for the audit queue envelope: + ```typescript + interface AuditJobMessage { + action: AuditAction; + actorId?: string; + actorUsername?: string; + resourceType?: string; + resourceId?: string; + metadata?: Record; + browserName?: string; + os?: string; + ipAddress?: string; + occurredAt: string; // ISO timestamp + } + ``` + - Notes: Interface only — no Zod schema needed. The audit data comes from the trusted `AuditService.Emit()` call (internal), not from external input. Adding Zod validation in the processor would be dead code. + +#### Phase 2: Service + Decorator + Interceptor + +- [x] Task 7: Create `AuditService` + - File: `src/modules/audit/audit.service.ts` (NEW) + - Action: Create service with: + - `@InjectQueue(QueueName.AUDIT) private readonly auditQueue: Queue` + - `async Emit(params: { action: AuditAction; actorId?: string; actorUsername?: string; resourceType?: string; resourceId?: string; metadata?: Record; browserName?: string; os?: string; ipAddress?: string }): Promise` + - Build envelope with `occurredAt: new Date().toISOString()` + - Enqueue via `this.auditQueue.add('audit', envelope, { attempts: 1, removeOnComplete: true, removeOnFail: 100 })` + - Wrap in try/catch — audit failures MUST NOT break the request. Log with `Logger.warn` including **non-PII fields only**: `action`, `actorId`, `resourceType`, `resourceId`, `occurredAt`. Do NOT log `metadata` (may contain usernames from failed logins — same PII policy as the processor's `@OnWorkerEvent('failed')` handler in Task 5). + - Notes: Fire-and-forget. Set `attempts: 1` on job options (no retries). Audit inserts are idempotent-safe and not critical enough to retry. + +- [x] Task 8a: Create `AuditAction` const enum + - File: `src/modules/audit/audit-action.enum.ts` (NEW) + - Action: Define all MVP audit actions as a const object (same pattern as `QueueName`): + ```typescript + export const AuditAction = { + AUTH_LOGIN_SUCCESS: 'auth.login.success', + AUTH_LOGIN_FAILURE: 'auth.login.failure', + AUTH_LOGOUT: 'auth.logout', + AUTH_TOKEN_REFRESH: 'auth.token.refresh', + ADMIN_SYNC_TRIGGER: 'admin.sync.trigger', + ADMIN_SYNC_SCHEDULE_UPDATE: 'admin.sync-schedule.update', + QUESTIONNAIRE_SUBMIT: 'questionnaire.submit', + QUESTIONNAIRE_INGEST: 'questionnaire.ingest', + QUESTIONNAIRE_SUBMISSIONS_WIPE: 'questionnaire.submissions.wipe', + ANALYSIS_PIPELINE_CREATE: 'analysis.pipeline.create', + ANALYSIS_PIPELINE_CONFIRM: 'analysis.pipeline.confirm', + ANALYSIS_PIPELINE_CANCEL: 'analysis.pipeline.cancel', + } as const; + export type AuditAction = (typeof AuditAction)[keyof typeof AuditAction]; + ``` + - Notes: Prevents typos in action strings. All tasks referencing action strings must use this enum. + +- [x] Task 8b: Create `@Audited()` decorator + - File: `src/modules/audit/decorators/audited.decorator.ts` (NEW) + - Action: Create decorator using `SetMetadata` that accepts an options object: + ```typescript + export const AUDIT_META_KEY = 'audit:meta'; + export interface AuditedOptions { + action: AuditAction; + resource?: string; // e.g. 'User', 'QuestionnaireSubmission' + } + export const Audited = (options: AuditedOptions) => + SetMetadata(AUDIT_META_KEY, options); + ``` + - Notes: Extended from single string to options object. `resource` is optional and populates `resourceType` in the audit log. The interceptor extracts `resourceId` from the first UUID route param (`request.params`). When `resourceId` is null but `resource` is set (e.g., `POST /moodle/sync` with `resource: 'SyncLog'`), it means the resource is **created by the action** and doesn't exist yet at audit capture time. This is expected and acceptable for MVP. + +- [x] Task 9: Create `AuditInterceptor` + - File: `src/modules/audit/interceptors/audit.interceptor.ts` (NEW) + - Action: Create NestJS interceptor: + - Inject `Reflector`, `AuditService`, `CurrentUserService`, `RequestMetadataService`. Type the request as `AuthenticatedRequest` (from `src/modules/common/interceptors/http/authenticated-request.ts`) for typed access to `req.user?.userId`. + - In `intercept()`: + 1. Read `AUDIT_META_KEY` from handler metadata via `Reflector.get()` — returns `AuditedOptions | undefined` + 2. If no metadata, pass through (`return next.handle()`) + 3. **CRITICAL: Use RxJS `tap()` operator, NOT `finalize()`** — `tap` only fires on successful `next` emissions. `finalize` fires on both success and error, which would log failed requests as audited actions. This distinction is essential for correct login audit behavior. + 4. Return `next.handle().pipe(tap(() => { ... }))` — emit audit event **after** successful response + 5. In the `tap` callback: + - Read user from `CurrentUserService.get()` — if null, **fall back to `req.user?.userId` from the JWT payload** (available on all `@UseJwtGuard()` endpoints without needing `CurrentUserInterceptor`). `actorUsername` will be null in this fallback case (acceptable — username is a convenience field, not critical). + - Read request metadata from `RequestMetadataService.get()` — if null, log `Logger.warn` with controller/handler name indicating missing CLS metadata + - Extract `resourceId` from `request.params` — use the first param value matching a full UUID v4 regex (`/^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i`), or null if none. Note: this is a best-effort heuristic — for `DELETE /versions/:versionId/submissions`, it will capture the `versionId` which is the parent resource, not the submissions being deleted. This is semantically correct for that action (see Task 15 notes). + - Extract `resourceType` from `AuditedOptions.resource` + - Capture `metadata` from `request.params` and `request.query` (shallow merge) — do NOT capture `request.body` (too large/sensitive) + - Call `AuditService.Emit()` with action, actorId, actorUsername, resourceType, resourceId, metadata, browserName, os, ipAddress + 6. Wrap the entire `tap` callback in try/catch — errors must be logged, never propagated to the response + +#### Phase 3: Module Wiring + +- [x] Task 10: Create `AuditModule` (global) + - File: `src/modules/audit/audit.module.ts` (NEW) + - Action: Create **global** module — audit is a cross-cutting concern (same pattern as `JwtModule`, `CacheModule`): + ```typescript + @Global() + @Module({ + imports: [ + BullModule.registerQueue({ name: QueueName.AUDIT }), + MikroOrmModule.forFeature([AuditLog]), + AppClsModule, + ], + providers: [AuditService, AuditProcessor, AuditInterceptor], + exports: [AuditService, AuditInterceptor], + }) + export class AuditModule {} + ``` + - Notes: `@Global()` makes `AuditService` and `AuditInterceptor` injectable in all modules without explicit imports. This is a new convention — no existing application module uses `@Global()`. The infrastructure modules (`JwtModule`, `CacheModule`, `ClsModule`) achieve global scope via `isGlobal: true` / `global: true` config options, not `@Global()` decorator. `@Global()` is justified here because audit is a cross-cutting concern consumed by many modules, and explicit imports in every host module add friction with no benefit. Import `AppClsModule` (not `CommonModule`) — audit only needs `CurrentUserService` and `RequestMetadataService`. + +- [x] Task 11: Register `AuditModule` in application modules + - File: `src/modules/index.module.ts` + - Action: Import `AuditModule` and add to `ApplicationModules` array + +#### Phase 4: Tag MVP Endpoints + +- [x] Task 12: Tag auth logout endpoint (interceptor path) + - File: `src/modules/auth/auth.controller.ts` + - Action: + - Add `@Audited({ action: AuditAction.AUTH_LOGOUT, resource: 'User' })` to `POST /logout` handler + - **REPLACE** the existing `@UseInterceptors(CurrentUserInterceptor)` on `POST /logout` with `@UseInterceptors(MetaDataInterceptor, CurrentUserInterceptor, AuditInterceptor)` — do NOT add a second decorator (would cause `CurrentUserInterceptor` to run twice). **Ordering matters**: metadata first, user second, audit last. + - Notes: Only logout uses the interceptor path for auth. Login (success + failure) and refresh use direct emit (Task 13) because there is no authenticated user in CLS for login, and `CurrentUserInterceptor` is not wired for refresh. Note: the logout endpoint currently has NO `MetaDataInterceptor` — this is a pre-existing gap (login and refresh already have it). Adding it here incidentally fixes that gap. Side effect: `MetaDataInterceptor` performs `Logger.log()` on every request with IP/browser/OS. Adding it to endpoints that didn't have it will increase log volume slightly — this is expected and acceptable. + +- [x] Task 13: Add direct audit emit for auth events (login success, login failure, token refresh) + - File: `src/modules/auth/auth.service.ts` + - Action: + - Inject `AuditService` using `@Optional()` decorator. Guard all `Emit()` calls with `this.auditService?.Emit(...)` (optional chaining). Note: `@Optional()` documents the contract that auth does not require audit — it handles the case where the provider isn't registered. However, if `AuditModule` fails during bootstrap (e.g., Redis down), NestJS will crash the whole app regardless. The real runtime protection is the try/catch inside `Emit()`, not `@Optional()`. + - **Login failure**: Inside the `if (!strategy)` block (around line 51), BEFORE the `throw new UnauthorizedException()`, call `void this.auditService?.Emit({ action: AuditAction.AUTH_LOGIN_FAILURE, metadata: { username, reason: 'no_matching_strategy' }, browserName, os, ipAddress })`. Also wrap `strategy.Execute()` in a try/catch — emit with `reason: error.message` on failure before re-throwing. Use `void` (fire-and-forget) — do NOT `await` audit emits in failure paths to avoid delaying error responses. + - **Login success**: **Refactor `Login()` to emit AFTER the transaction.** The current method does `return await this.unitOfWork.runInTransaction(...)` — everything is inside the transaction callback and variables are scoped within it. Restructure to: + ```typescript + const result = await this.unitOfWork.runInTransaction(async (em) => { + // ...existing strategy execution, token generation... + return { response: LoginResponse.Map(...), userId: user.id, username: user.username, strategyName: strategy.Name ?? strategy.constructor.name }; + }); + void this.auditService?.Emit({ action: AuditAction.AUTH_LOGIN_SUCCESS, actorId: result.userId, actorUsername: result.username, metadata: { strategyUsed: result.strategyName }, browserName, os, ipAddress }); + return result.response; + ``` + Note: `strategy.constructor.name` may be mangled in production builds with minification. Prefer a static `Name` property on each strategy (e.g., `get Name() { return 'local'; }`). Fall back to `constructor.name` if the property doesn't exist. + **CRITICAL**: The transaction must return the data the audit emit needs. The emit fires AFTER the transaction closes. Never emit inside the transaction — Redis latency would hold the DB transaction open. + - **Token refresh**: **Same refactoring pattern for `RefreshToken()`.** Current method also does `return await this.unitOfWork.runInTransaction(...)`. Restructure to: + ```typescript + const result = await this.unitOfWork.runInTransaction(async (em) => { + // ...existing refresh logic... + return { response: TokenResponse.Map(...), userId: user.id, username: user.username }; + }); + void this.auditService?.Emit({ action: AuditAction.AUTH_TOKEN_REFRESH, actorId: result.userId, actorUsername: result.username, browserName, os, ipAddress }); + return result.response; + ``` + - Pull browser/OS/IP from `RequestMetadataService.get()` with null-safe destructure: `const { browserName, os, ipAddress } = this.requestMetadataService.get() ?? {};` — guards against null if `MetaDataInterceptor` didn't run. + - Notes: All audit emits use `void` (fire-and-forget, no `await`) — this prevents audit from blocking the response or holding transactions open. `actorId`/`actorUsername` will be undefined for failed logins. `RequestMetadataService` IS available since `MetaDataInterceptor` runs before auth logic. The `@Optional()` injection with optional chaining prevents audit from being a hard dependency of auth. + +- [x] Task 14: Tag moodle sync endpoints + - File: `src/modules/moodle/controllers/moodle-sync.controller.ts` + - Action: + - Add `@Audited({ action: AuditAction.ADMIN_SYNC_TRIGGER, resource: 'SyncLog' })` to `POST /moodle/sync` + - Add `@Audited({ action: AuditAction.ADMIN_SYNC_SCHEDULE_UPDATE, resource: 'SystemConfig' })` to `PUT /moodle/sync/schedule` + - For `POST /moodle/sync`: **REPLACE** existing `@UseInterceptors(CurrentUserInterceptor)` with `@UseInterceptors(MetaDataInterceptor, CurrentUserInterceptor, AuditInterceptor)` — this endpoint already has `CurrentUserInterceptor`, so keep it. + - For `PUT /moodle/sync/schedule`: Add `@UseInterceptors(MetaDataInterceptor, AuditInterceptor)` — this endpoint has no existing interceptors. `CurrentUserInterceptor` not required (JWT fallback in AuditInterceptor). + - Notes: Both are superadmin-only endpoints. + +- [x] Task 15: Tag questionnaire endpoints + - File: `src/modules/questionnaires/questionnaire.controller.ts` + - Action: + - Add `@Audited({ action: AuditAction.QUESTIONNAIRE_SUBMIT, resource: 'QuestionnaireSubmission' })` and `@UseInterceptors(MetaDataInterceptor, CurrentUserInterceptor, AuditInterceptor)` to `POST /submissions` (no existing interceptors). **Exception to the "no CurrentUserInterceptor" rule**: this is the highest-volume audited action — every student submission. The extra DB hit from `UserLoader.load()` is justified here so that `actorUsername` is populated (not null), making the audit log meaningfully readable for the most frequent event. + - Add `@Audited({ action: AuditAction.QUESTIONNAIRE_INGEST, resource: 'QuestionnaireSubmission' })` to `POST /ingest`. **IMPORTANT**: This endpoint already has `@UseInterceptors(FileInterceptor('file', { fileFilter: csvFileFilter, limits: { fileSize: 5 * 1024 * 1024 } }))`. **REPLACE** it with a single merged decorator preserving the existing FileInterceptor config: `@UseInterceptors(MetaDataInterceptor, FileInterceptor('file', { fileFilter: csvFileFilter, limits: { fileSize: 5 * 1024 * 1024 } }), AuditInterceptor)` — `FileInterceptor` must run before `AuditInterceptor` (it parses the multipart body). Do NOT add `CurrentUserInterceptor` (endpoint never had it — JWT fallback handles actor ID). Do NOT add a second `@UseInterceptors` decorator. + - Add `@Audited({ action: AuditAction.QUESTIONNAIRE_SUBMISSIONS_WIPE, resource: 'QuestionnaireVersion' })` and `@UseInterceptors(MetaDataInterceptor, AuditInterceptor)` to `DELETE /versions/:versionId/submissions`. Note: `resourceId` will be the `versionId` (the parent resource whose submissions are being wiped), and `resourceType` is `QuestionnaireVersion` — this correctly identifies what resource the wipe is scoped to, not the individual submissions being deleted. + - Notes: Interceptor stacks are specified per-endpoint above (each bullet includes its own `@UseInterceptors` call). `CurrentUserInterceptor` is NOT added to endpoints that didn't already have it — the `AuditInterceptor` falls back to `req.user.userId` from the JWT payload (see Task 9). Submission wipe is the highest-risk action. For bulk ingestion (`POST /ingest`), the interceptor fires **once per HTTP request** (not per record) — one audit event with route params, not thousands. `POST /ingest` is accessible by 4 roles (SUPER_ADMIN, ADMIN, DEAN, CHAIRPERSON), not just superadmin. Note: `POST /ingest` has no route params or query params — interceptor-captured metadata will be `{}`. The actual ingestion context (version, record count) lives in the multipart body which is intentionally excluded. For richer audit data on this endpoint, a future iteration could add a direct emit enrichment inside the handler. Document this explicitly so nobody adds per-record auditing inside the ingestion engine later. + +- [x] Task 16: Tag analysis endpoints + - File: `src/modules/analysis/analysis.controller.ts` + - Action: + - Add `@Audited({ action: AuditAction.ANALYSIS_PIPELINE_CREATE, resource: 'AnalysisPipeline' })` to `POST /pipelines` + - Add `@Audited({ action: AuditAction.ANALYSIS_PIPELINE_CONFIRM, resource: 'AnalysisPipeline' })` to `POST /pipelines/:id/confirm` + - Add `@Audited({ action: AuditAction.ANALYSIS_PIPELINE_CANCEL, resource: 'AnalysisPipeline' })` to `POST /pipelines/:id/cancel` + - Add `@UseInterceptors(MetaDataInterceptor, AuditInterceptor)` to each tagged method — **ordering: metadata first, audit last**. `CurrentUserInterceptor` is NOT required — the `AuditInterceptor` falls back to `req.user.userId` from the JWT payload. This avoids adding an extra DB hit (`UserLoader.load()`) to endpoints that never had it. + - Notes: `AnalysisController` currently has no interceptors — only `@UseJwtGuard()` at the class level. + +#### Phase 5: Tests + +- [x] Task 17: Unit test `AuditService` + - File: `src/modules/audit/audit.service.spec.ts` (NEW) + - Action: Test that `Emit()` calls `queue.add()` with correct envelope; test that Redis errors are caught and logged (not thrown) + +- [x] Task 18: Unit test `AuditProcessor` + - File: `src/modules/audit/audit.processor.spec.ts` (NEW) + - Action: Test that `process()` creates `AuditLog` entity with correct field mapping; test `em.fork()` is called; test malformed job data logs error + +- [x] Task 19: Unit test `AuditInterceptor` + - File: `src/modules/audit/interceptors/audit.interceptor.spec.ts` (NEW) + - Action: Test interceptor reads `@Audited()` metadata and calls `AuditService.Emit()` after response; test no-op when no `@Audited()` metadata; test errors in emit don't propagate + +- [x] Task 20: Update auth service tests + - File: `src/modules/auth/auth.service.spec.ts` + - Action: Add mock for `AuditService`; verify `Emit()` called with `auth.login.failure` on failed login; verify `Emit()` called with `auth.login.success` after transaction returns (not inside); verify `Emit()` called with `auth.token.refresh` after transaction returns; add test case where `AuditService` is `undefined` (not provided via `@Optional()`) and verify login and refresh still complete without throwing (logout uses interceptor path, not direct emit — not relevant here) + +### Acceptance Criteria + +#### Core Pipeline + +- [x] AC 1: Given the application starts, when the AUDIT queue is registered, then BullMQ connects to Redis and the `audit` queue is available for job dispatch +- [x] AC 2: Given a valid `AuditJobMessage` is enqueued, when the `AuditProcessor` processes it, then an `AuditLog` row is persisted with all fields correctly mapped +- [x] AC 3: Given the Redis connection fails during `AuditService.Emit()`, when an audited action occurs, then the error is logged but the original request completes successfully (audit never breaks the app) + +#### Interceptor Path + +- [x] AC 4: Given a controller method decorated with `@Audited({ action: AuditAction.AUTH_LOGOUT, resource: 'User' })`, when the endpoint returns a successful response, then an audit event is enqueued with `action='auth.logout'`, the current user's ID (from CLS or JWT fallback), `resourceType: 'User'`, and request metadata (IP, browser, OS) +- [x] AC 5: Given a controller method WITHOUT `@Audited()`, when the `AuditInterceptor` is applied, then no audit event is emitted (pass-through) +- [x] AC 6: Given the `AuditInterceptor` fails to emit (e.g., service error), when the endpoint handler succeeds, then the original response is still returned to the client + +#### Direct Emit Path + +- [x] AC 7: Given a user submits invalid credentials, when the login strategy throws `UnauthorizedException`, then `AuditService.Emit()` is called with `action='auth.login.failure'`, the attempted username in metadata, and the request IP address +- [x] AC 8: Given a user logs in successfully, when the auth service returns tokens, then `AuditService.Emit()` is called via the **direct emit path** inside `AuthService` with `action=AuditAction.AUTH_LOGIN_SUCCESS`, the authenticated user's ID/username, and the strategy used in metadata + +#### Entity Integrity + +- [x] AC 9: Given the `audit_log` table exists, when a query is run without `filters: { softDelete: false }`, then the global soft-delete filter does not exclude audit records (entity has no `deletedAt` field, but queries must still bypass the filter) +- [x] AC 10: Given an `AuditLog` record is created, then it has no `updatedAt` or `deletedAt` fields — it is immutable and append-only + +#### MVP Endpoint Coverage + +- [x] AC 11: Given the MVP is complete, when inspecting the codebase, then: `POST /auth/logout`, `POST /moodle/sync`, `PUT /moodle/sync/schedule`, `POST /questionnaires/submissions`, `POST /questionnaires/ingest`, `DELETE /questionnaires/versions/:id/submissions`, `POST /analysis/pipelines`, `POST /analysis/pipelines/:id/confirm`, `POST /analysis/pipelines/:id/cancel` have `@Audited()` decorators (interceptor path); and `POST /auth/login` (success + failure) and `POST /auth/refresh` use direct `AuditService.Emit()` calls (direct emit path) + +## Review Notes + +- Adversarial review completed (15 findings) +- Findings: 15 total, 9 fixed, 6 skipped (4 noise, 2 undecided) +- Resolution approach: auto-fix +- Fixed: metadata size cap (F1), sanitized error reasons (F2), moved failure emits outside transaction (F3), added actorId index (F6), added onFailed handler test (F8), added metadata capture tests (F9), moved EmitParams to dto/ (F10), added failure-path @Optional test (F14), extracted shared test helpers (F15) +- Skipped as noise: processor validation (F5, spec decision), soft-delete filter risk (F7, matches SyncLog), spoofable x-forwarded-for (F12, pre-existing), occurredAt accuracy (F13, negligible) +- Skipped as undecided: strategy.constructor.name (F4, NestJS doesn't minify), composite decorator (F11, out of MVP scope) + +## Additional Context + +### Dependencies + +- `@nestjs/bullmq` / `bullmq` (already installed) +- `nestjs-cls` (already installed) +- Redis (already running via docker-compose) +- No new external dependencies required + +### Testing Strategy + +**Unit Tests (NestJS TestingModule + Jest):** + +- `audit.service.spec.ts` — Test `Emit()` enqueues correct envelope to AUDIT queue; test error handling for Redis connection failures +- `audit.processor.spec.ts` — Test `process()` persists `AuditLog` entity with correct fields via `em.fork().persistAndFlush()`; test malformed job data handling +- `audit.interceptor.spec.ts` — Test interceptor reads `@Audited()` metadata, calls `AuditService.Emit()` post-response with correct action/context/resource; test no-op when decorator is absent; test that when `RequestMetadataService.get()` returns null, a `Logger.warn` fires but the audit event still emits with null IP/browser/OS; test that route params are captured in metadata; test that `tap()` does NOT fire on error responses + +**Integration Points (existing test files to update):** + +- `auth.service.spec.ts` — Verify `AuditService.Emit()` is called on login success (with user ID and strategy name), login failure (with username and reason), and token refresh (with user ID) +- Controller tests — Verify `@Audited()` decorator is present on tagged endpoints (metadata reflection test) + +### Notes + +**High-Risk Items:** + +- **Global soft-delete filter bypass**: `AuditLog` has no `deletedAt` field, but the global MikroORM filter in `mikro-orm.config.ts` applies to all entities. Any direct query on `AuditLog` must use `filters: { softDelete: false }`. The processor's `em.fork()` handles writes fine, but future read queries must remember this. +- **Auth service coupling**: Injecting `AuditService` into `AuthService` creates a new dependency. `@Optional()` documents the contract but does NOT protect against bootstrap failure — if `AuditModule` fails to initialize (e.g., Redis unreachable), the entire app crashes (same as existing analysis queues). The real runtime protection is the try/catch inside `Emit()` and `void` (fire-and-forget) call pattern. +- **CLS context availability**: `RequestMetadataService` is populated by `MetaDataInterceptor` — all MVP interceptor-path endpoints now include it. `CurrentUserInterceptor` is only added where it already existed (logout, moodle sync trigger). For other endpoints, `AuditInterceptor` falls back to `req.user.userId` from the JWT payload, avoiding unnecessary DB hits from `UserLoader`. +- **Test environment Redis**: Any test that imports `AuditModule` (directly or transitively) will attempt to connect to Redis via BullMQ. Unit tests should mock the queue; integration/E2E tests need Redis running (already required for other queues in the test environment). + +**Known Limitations:** + +- Auth failure auditing captures the attempted username from the DTO but cannot resolve to a user ID (user may not exist). +- For "create" actions (`analysis.pipeline.create`, `questionnaire.submit`), `resourceId` will be null because the resource ID is generated inside the handler and returned in the response body — the interceptor captures `request.params` only, not the response. A future iteration could extend the interceptor to optionally read `resourceId` from the response. +- No before/after diffs on entity mutations — only the action and resource ID are recorded. +- Audit logs grow unbounded — no retention policy in MVP. +- Failed BullMQ audit jobs are retained in Redis (`removeOnFail: 100`). During prolonged DB outages, up to 100 failed jobs accumulate. Operators can inspect failed jobs via `redis-cli` (`ZRANGE bull:audit:failed 0 -1`) or a BullMQ dashboard. Acceptable for MVP — add monitoring/cleanup in future iterations if needed. + +**Future Considerations (out of scope):** + +- Admin query endpoint with filtering by action, actor, date range, resource type +- Retention/archival job (similar to `RefreshTokenCleanupJob` pattern) +- Before/after entity snapshots using MikroORM lifecycle hooks +- Broader CRUD auditing via global interceptor +- Export to external log aggregation (ELK, Datadog, etc.) diff --git a/docs/architecture/audit-trail.md b/docs/architecture/audit-trail.md new file mode 100644 index 0000000..e8e98f7 --- /dev/null +++ b/docs/architecture/audit-trail.md @@ -0,0 +1,117 @@ +# Audit Trail + +The `AuditModule` provides an append-only audit log for security-sensitive actions. It captures who did what, when, and from where — for compliance, incident investigation, and operational accountability. + +## Architecture + +```mermaid +flowchart LR + subgraph Interceptor Path + A["@Audited() decorator"] --> B[AuditInterceptor] + B -->|post-response tap| C[AuditService.Emit] + end + + subgraph Direct Emit Path + D[AuthService] -->|fire-and-forget| C + end + + C -->|enqueue| E[AUDIT queue] + E --> F[AuditProcessor] + F -->|em.fork + create + flush| G[(audit_log table)] +``` + +### Two Emission Paths + +| Path | When | Context Source | +| --------------- | ----------------------------------------------------------------------------------- | ------------------------------------------------------------------------------ | +| **Interceptor** | Standard authenticated endpoints (logout, sync, submissions, pipelines) | CLS (`CurrentUserService`, `RequestMetadataService`) with JWT payload fallback | +| **Direct emit** | Auth events where CLS context is unavailable (login success/failure, token refresh) | Explicit params from `AuthService` | + +Both paths feed the same `AuditService.Emit()` method, which enqueues a job to the `AUDIT` BullMQ queue. + +## AuditLog Entity + +Append-only, immutable. Does **not** extend `CustomBaseEntity` (no `updatedAt`, no `deletedAt`). Follows the `SyncLog` precedent. + +| Column | Type | Notes | +| --------------- | ------------------ | ------------------------------------------------------------------------------------------------- | +| `id` | `varchar` PK | UUID v4, auto-generated | +| `action` | `varchar` | Indexed. Dot-notation action code (e.g., `auth.login.success`) | +| `actorId` | `varchar` nullable | Indexed. Plain string, **not** a FK — survives user deletion | +| `actorUsername` | `varchar` nullable | Denormalized for historical accuracy | +| `resourceType` | `varchar` nullable | Entity name (e.g., `User`, `AnalysisPipeline`) | +| `resourceId` | `varchar` nullable | UUID of affected resource | +| `metadata` | `jsonb` nullable | Action-specific details (capped at 4KB from interceptor) | +| `browserName` | `varchar` nullable | From `MetaDataInterceptor` via CLS | +| `os` | `varchar` nullable | From `MetaDataInterceptor` via CLS | +| `ipAddress` | `varchar` nullable | From `x-forwarded-for` or socket | +| `occurredAt` | `timestamptz` | Indexed. Set from job payload (event time, not processing time). DB default `now()` as safety net | + +Queries must use `filters: { softDelete: false }` to bypass the global soft-delete filter. + +## MVP Actions + +```typescript +export const AuditAction = { + AUTH_LOGIN_SUCCESS: 'auth.login.success', + AUTH_LOGIN_FAILURE: 'auth.login.failure', + AUTH_LOGOUT: 'auth.logout', + AUTH_TOKEN_REFRESH: 'auth.token.refresh', + ADMIN_SYNC_TRIGGER: 'admin.sync.trigger', + ADMIN_SYNC_SCHEDULE_UPDATE: 'admin.sync-schedule.update', + QUESTIONNAIRE_SUBMIT: 'questionnaire.submit', + QUESTIONNAIRE_INGEST: 'questionnaire.ingest', + QUESTIONNAIRE_SUBMISSIONS_WIPE: 'questionnaire.submissions.wipe', + ANALYSIS_PIPELINE_CREATE: 'analysis.pipeline.create', + ANALYSIS_PIPELINE_CONFIRM: 'analysis.pipeline.confirm', + ANALYSIS_PIPELINE_CANCEL: 'analysis.pipeline.cancel', +} as const; +``` + +## Interceptor Path Detail + +Endpoints are tagged with the `@Audited({ action, resource? })` decorator, which sets Reflector metadata. The `AuditInterceptor` reads this metadata and, on successful response (RxJS `tap`, not `finalize`), enqueues an audit event. + +Interceptor ordering matters: `MetaDataInterceptor` (IP/browser/OS) must run before `AuditInterceptor`. When `CurrentUserInterceptor` is present, it runs between them to populate the CLS user. + +```typescript +@UseInterceptors(MetaDataInterceptor, CurrentUserInterceptor, AuditInterceptor) +``` + +The interceptor extracts `resourceId` from route params using a UUID v4 regex heuristic. Metadata captures route params and query params (not request body), capped at 4KB. + +## Direct Emit Path Detail + +Used in `AuthService` for login success, login failure, and token refresh. These events occur before JWT authentication is established, so CLS user context is unavailable. + +- **Login success**: Emitted after the transaction returns, with `actorId`, `actorUsername`, and `strategyUsed` metadata. +- **Login failure**: Emitted after the transaction rejects, with `username` and a sanitized `reason` code (`no_matching_strategy` or `strategy_execution_failed`). Raw error messages are never persisted. +- **Token refresh**: Emitted after the transaction returns, with `actorId` and `actorUsername`. + +All direct emits use `void this.auditService?.Emit(...)` — fire-and-forget, never inside a transaction. + +## Queue & Processor + +| Property | Value | +| ------------------ | --------------------------------- | +| Queue name | `audit` | +| Concurrency | 1 | +| Retry attempts | 1 (no retries) | +| `removeOnComplete` | `true` | +| `removeOnFail` | 100 (keep last 100 for debugging) | + +The `AuditProcessor` extends `WorkerHost` directly (no HTTP dispatch). It forks the `EntityManager`, creates an `AuditLog` entity, and flushes. The `@OnWorkerEvent('failed')` handler logs non-PII fields only (no `metadata`). + +## Module Design + +`AuditModule` is `@Global()` — the only application module using this decorator. This makes `AuditService` and `AuditInterceptor` injectable everywhere without explicit imports. Justified because audit is a cross-cutting concern consumed by many modules. + +`AuditService` is injected with `@Optional()` in `AuthService` to avoid making audit a hard dependency of authentication. All `Emit()` calls use optional chaining. + +## Error Handling + +Audit failures never break the request: + +1. `AuditService.Emit()` wraps `queue.add()` in try/catch — logs a warning, returns void. +2. `AuditInterceptor` wraps the entire `tap` callback in try/catch — errors are logged, never propagated. +3. The `.catch()` on the `Emit()` promise handles async rejections. diff --git a/docs/architecture/core-components.md b/docs/architecture/core-components.md index bd2e6cf..c636a95 100644 --- a/docs/architecture/core-components.md +++ b/docs/architecture/core-components.md @@ -55,6 +55,7 @@ classDiagram DimensionsModule FacultyModule CurriculumModule + AuditModule } AppModule --> InfrastructureModules : "imports" @@ -68,6 +69,8 @@ classDiagram QuestionnaireModule --> CommonModule : "uses UnitOfWork" AnalysisModule --> BullModule : "uses BullMQ queues" AnalyticsModule --> BullModule : "uses BullMQ queue" + AuditModule --> BullModule : "uses BullMQ queue" + AuthModule --> AuditModule : "uses AuditService" AnalyticsModule --> CommonModule : "uses ScopeResolverService" FacultyModule --> CommonModule : "uses ScopeResolverService" CurriculumModule --> CommonModule : "uses ScopeResolverService" @@ -118,6 +121,13 @@ classDiagram +IngestionEngine +IngestionMapperService } + + class AuditModule { + <> + +AuditService + +AuditProcessor + +AuditInterceptor + } ``` ## 4. Login Strategy Pattern @@ -205,7 +215,7 @@ Each stage has a corresponding `RunStatus` (`PENDING` → `PROCESSING` → `COMP ### Queue Architecture -Six BullMQ queues with independent concurrency. Queue names are centralized in `src/configurations/common/queue-names.ts`. +Seven BullMQ queues with independent concurrency. Queue names are centralized in `src/configurations/common/queue-names.ts`. | Queue | Processor | Concurrency Default | Module | | ------------------- | --------------------------- | ------------------- | --------------- | @@ -215,6 +225,7 @@ Six BullMQ queues with independent concurrency. Queue names are centralized in ` | `topic-model` | `TopicModelProcessor` | 1 | AnalysisModule | | `recommendations` | `RecommendationsProcessor` | 1 | AnalysisModule | | `analytics-refresh` | `AnalyticsRefreshProcessor` | 1 | AnalyticsModule | +| `audit` | `AuditProcessor` | 1 | AuditModule | ### REST Endpoints diff --git a/docs/decisions/decisions.md b/docs/decisions/decisions.md index 9a68851..6ebe9c3 100644 --- a/docs/decisions/decisions.md +++ b/docs/decisions/decisions.md @@ -237,6 +237,36 @@ The `MoodleSyncScheduler` was rewritten from a static `@Cron(CronExpression.EVER - **Soft-delete filter bypass:** Since `SyncLog` has no `deletedAt` column, MikroORM's global `softDelete` filter would fail at query time. Queries must use `filters: { softDelete: false }`. The `@Filter` decorator approach (`cond: {}, default: false`) was found to be insufficient at runtime. - **Trade-off:** Admin schedule changes don't survive process restarts unless persisted to the database (which they are, via `SystemConfig`). The scheduler reads from DB on init, so restarts pick up the latest admin-configured interval. +## 34. Append-Only Audit Entity (No CustomBaseEntity) + +The `AuditLog` entity does not extend `CustomBaseEntity`. It has no `updatedAt` or `deletedAt` — records are immutable and never soft-deleted. The `actorId` column is a plain string, not a `@ManyToOne` FK, so audit records survive user deletion. + +- **Rationale:** Audit logs must be tamper-evident and permanent. Soft delete semantics would allow "hiding" audit records. FK constraints would cause cascade failures when users are deleted, creating a perverse incentive to retain user data solely for audit integrity. +- **Precedent:** Follows the `SyncLog` entity pattern. Queries must use `filters: { softDelete: false }` to bypass the global filter. +- **Trade-off:** No ORM-level relationship to `User` — joins require manual `actorId` matching. Acceptable because audit query endpoints (future) will use raw SQL or query builder, not entity relationships. + +## 35. Global AuditModule with @Global() Decorator + +`AuditModule` uses the `@Global()` class decorator — the only application module to do so. Infrastructure modules achieve global scope via config options (`isGlobal: true`), but `@Global()` is appropriate here because audit is a cross-cutting concern consumed by many modules. + +- **Rationale:** Without `@Global()`, every module that uses `@Audited()` endpoints would need to explicitly import `AuditModule`. Since the interceptor is applied per-endpoint (not per-module), this friction discourages adoption with no compensating benefit. +- **Trade-off:** `AuditService` is injectable everywhere, which could lead to misuse. Mitigated by the fire-and-forget API — `Emit()` has no return value and catches all errors internally. + +## 36. Dual Audit Emission Paths (Interceptor + Direct) + +Audit events are captured through two paths: an interceptor for standard authenticated endpoints, and direct `AuditService.Emit()` calls for auth events. + +- **Rationale:** The interceptor path requires CLS context (`CurrentUserService`, `RequestMetadataService`), which is unavailable during login (no JWT yet) and inconsistently available during token refresh. Rather than forcing all audit events through one path, two paths allow each context to use the most natural capture mechanism. +- **Convergence:** Both paths feed the same `AuditService.Emit()` → AUDIT queue → `AuditProcessor` → `audit_log` table pipeline. The entity schema is identical regardless of emission path. +- **Trade-off:** Two integration patterns to understand. Mitigated by clear separation — interceptor path is decorator-driven (declarative), direct path is explicit method calls in `AuthService` only. + +## 37. Sanitized Audit Metadata (No Raw Error Messages) + +Login failure audit events store a fixed reason code (`no_matching_strategy`, `strategy_execution_failed`) instead of the raw `error.message`. + +- **Rationale:** Raw error messages may contain connection strings, hostnames, SQL fragments, or stack traces — especially from Moodle connectivity errors or database driver failures. Persisting these in an immutable, append-only table creates a permanent information disclosure risk. +- **Trade-off:** Less diagnostic detail in audit logs. Full error details are still available in application logs (which are rotatable and not permanent). + ## 30. Semester Code Parsing for Display Labels The Moodle category sync now parses semester codes (e.g., `S22526`) into human-readable `label` ("Semester 2") and `academicYear` ("2025-2026") fields on the `Semester` entity. diff --git a/docs/workflows/auth-hydration.md b/docs/workflows/auth-hydration.md index 77a0b69..69872fe 100644 --- a/docs/workflows/auth-hydration.md +++ b/docs/workflows/auth-hydration.md @@ -25,7 +25,11 @@ flowchart TD G --> N[Issue JWT + RefreshToken] L --> N - N --> O[200 OK Tokens] + N --> P[Audit: auth.login.success] + P --> O[200 OK Tokens] + + J --> Q[Audit: auth.login.failure] + M --> Q ``` ## Moodle Login Flow (Detail) @@ -72,6 +76,22 @@ sequenceDiagram AuthController-->>Client: 200 OK (Tokens) ``` +## Audit Events + +Auth events are captured via the direct emit path (not the interceptor) because CLS user context is unavailable during login. All emits are fire-and-forget (`void`) and occur **outside** the database transaction. + +| Event | Action Code | When | Metadata | +| ------------------------------ | -------------------- | ---------------------------- | --------------------------------------------------- | +| Login success | `auth.login.success` | After transaction commits | `{ strategyUsed }` | +| Login failure (no strategy) | `auth.login.failure` | After transaction rejects | `{ username, reason: 'no_matching_strategy' }` | +| Login failure (strategy threw) | `auth.login.failure` | After transaction rejects | `{ username, reason: 'strategy_execution_failed' }` | +| Token refresh | `auth.token.refresh` | After transaction commits | _(none)_ | +| Logout | `auth.logout` | Via `@Audited()` interceptor | _(route params)_ | + +`AuditService` is injected with `@Optional()` — auth works even if the audit module fails to bootstrap. + +See [Audit Trail Architecture](../architecture/audit-trail.md) for the full audit system design. + ## Institutional Role Resolution The system detects institutional management roles from Moodle category capabilities. Roles have a `source` field (`auto` or `manual`) that determines whether hydration can manage them. diff --git a/src/configurations/common/queue-names.ts b/src/configurations/common/queue-names.ts index e39cbe8..7bba1e4 100644 --- a/src/configurations/common/queue-names.ts +++ b/src/configurations/common/queue-names.ts @@ -5,6 +5,7 @@ export const QueueName = { RECOMMENDATIONS: 'recommendations', MOODLE_SYNC: 'moodle-sync', ANALYTICS_REFRESH: 'analytics-refresh', + AUDIT: 'audit', } as const; export type QueueName = (typeof QueueName)[keyof typeof QueueName]; diff --git a/src/entities/audit-log.entity.ts b/src/entities/audit-log.entity.ts new file mode 100644 index 0000000..8532b53 --- /dev/null +++ b/src/entities/audit-log.entity.ts @@ -0,0 +1,44 @@ +import { Entity, Index, Opt, PrimaryKey, Property } from '@mikro-orm/core'; +import { v4 } from 'uuid'; + +// Audit records are never soft-deleted. Queries must use +// `filters: { softDelete: false }` to bypass the global filter. +// See SyncLog for precedent. +@Entity() +export class AuditLog { + @PrimaryKey() + id: string & Opt = v4(); + + @Index() + @Property() + action!: string; + + @Index() + @Property({ nullable: true }) + actorId?: string; + + @Property({ nullable: true }) + actorUsername?: string; + + @Property({ nullable: true }) + resourceType?: string; + + @Property({ nullable: true }) + resourceId?: string; + + @Property({ type: 'jsonb', nullable: true }) + metadata?: Record; + + @Property({ nullable: true }) + browserName?: string; + + @Property({ nullable: true }) + os?: string; + + @Property({ nullable: true }) + ipAddress?: string; + + @Index() + @Property() + occurredAt!: Date; +} diff --git a/src/entities/index.entity.ts b/src/entities/index.entity.ts index 56ba360..3311046 100644 --- a/src/entities/index.entity.ts +++ b/src/entities/index.entity.ts @@ -30,6 +30,7 @@ import { TopicAssignment } from './topic-assignment.entity'; import { Section } from './section.entity'; import { TopicModelRun } from './topic-model-run.entity'; import { SyncLog } from './sync-log.entity'; +import { AuditLog } from './audit-log.entity'; export { ChatKitThread, @@ -64,6 +65,7 @@ export { TopicAssignment, TopicModelRun, SyncLog, + AuditLog, }; export const entities = [ @@ -99,4 +101,5 @@ export const entities = [ TopicAssignment, TopicModelRun, SyncLog, + AuditLog, ]; diff --git a/src/migrations/.snapshot-faculytics_db.json b/src/migrations/.snapshot-faculytics_db.json index 20e2b63..79d790a 100644 --- a/src/migrations/.snapshot-faculytics_db.json +++ b/src/migrations/.snapshot-faculytics_db.json @@ -4,6 +4,166 @@ ], "name": "public", "tables": [ + { + "columns": { + "id": { + "name": "id", + "type": "varchar(255)", + "unsigned": false, + "autoincrement": false, + "primary": false, + "nullable": false, + "length": 255, + "mappedType": "string" + }, + "action": { + "name": "action", + "type": "varchar(255)", + "unsigned": false, + "autoincrement": false, + "primary": false, + "nullable": false, + "length": 255, + "mappedType": "string" + }, + "actor_id": { + "name": "actor_id", + "type": "varchar(255)", + "unsigned": false, + "autoincrement": false, + "primary": false, + "nullable": true, + "length": 255, + "mappedType": "string" + }, + "actor_username": { + "name": "actor_username", + "type": "varchar(255)", + "unsigned": false, + "autoincrement": false, + "primary": false, + "nullable": true, + "length": 255, + "mappedType": "string" + }, + "resource_type": { + "name": "resource_type", + "type": "varchar(255)", + "unsigned": false, + "autoincrement": false, + "primary": false, + "nullable": true, + "length": 255, + "mappedType": "string" + }, + "resource_id": { + "name": "resource_id", + "type": "varchar(255)", + "unsigned": false, + "autoincrement": false, + "primary": false, + "nullable": true, + "length": 255, + "mappedType": "string" + }, + "metadata": { + "name": "metadata", + "type": "jsonb", + "unsigned": false, + "autoincrement": false, + "primary": false, + "nullable": true, + "mappedType": "json" + }, + "browser_name": { + "name": "browser_name", + "type": "varchar(255)", + "unsigned": false, + "autoincrement": false, + "primary": false, + "nullable": true, + "length": 255, + "mappedType": "string" + }, + "os": { + "name": "os", + "type": "varchar(255)", + "unsigned": false, + "autoincrement": false, + "primary": false, + "nullable": true, + "length": 255, + "mappedType": "string" + }, + "ip_address": { + "name": "ip_address", + "type": "varchar(255)", + "unsigned": false, + "autoincrement": false, + "primary": false, + "nullable": true, + "length": 255, + "mappedType": "string" + }, + "occurred_at": { + "name": "occurred_at", + "type": "timestamptz", + "unsigned": false, + "autoincrement": false, + "primary": false, + "nullable": false, + "length": 6, + "mappedType": "datetime" + } + }, + "name": "audit_log", + "schema": "public", + "indexes": [ + { + "columnNames": [ + "action" + ], + "composite": false, + "keyName": "audit_log_action_index", + "constraint": false, + "primary": false, + "unique": false + }, + { + "columnNames": [ + "actor_id" + ], + "composite": false, + "keyName": "audit_log_actor_id_index", + "constraint": false, + "primary": false, + "unique": false + }, + { + "columnNames": [ + "occurred_at" + ], + "composite": false, + "keyName": "audit_log_occurred_at_index", + "constraint": false, + "primary": false, + "unique": false + }, + { + "keyName": "audit_log_pkey", + "columnNames": [ + "id" + ], + "composite": false, + "constraint": true, + "primary": true, + "unique": true + } + ], + "checks": [], + "foreignKeys": {}, + "nativeEnums": {} + }, { "columns": { "id": { diff --git a/src/migrations/Migration20260329201139.ts b/src/migrations/Migration20260329201139.ts new file mode 100644 index 0000000..020dafb --- /dev/null +++ b/src/migrations/Migration20260329201139.ts @@ -0,0 +1,17 @@ +import { Migration } from '@mikro-orm/migrations'; + +export class Migration20260329201139 extends Migration { + + override async up(): Promise { + this.addSql(`create table "audit_log" ("id" varchar(255) not null, "action" varchar(255) not null, "actor_id" varchar(255) null, "actor_username" varchar(255) null, "resource_type" varchar(255) null, "resource_id" varchar(255) null, "metadata" jsonb null, "browser_name" varchar(255) null, "os" varchar(255) null, "ip_address" varchar(255) null, "occurred_at" timestamptz not null, constraint "audit_log_pkey" primary key ("id"));`); + this.addSql(`alter table "audit_log" alter column "occurred_at" set default now();`); + this.addSql(`create index "audit_log_action_index" on "audit_log" ("action");`); + this.addSql(`create index "audit_log_actor_id_index" on "audit_log" ("actor_id");`); + this.addSql(`create index "audit_log_occurred_at_index" on "audit_log" ("occurred_at");`); + } + + override async down(): Promise { + this.addSql(`drop table if exists "audit_log" cascade;`); + } + +} diff --git a/src/modules/analysis/analysis.controller.spec.ts b/src/modules/analysis/analysis.controller.spec.ts index 97667ee..9279332 100644 --- a/src/modules/analysis/analysis.controller.spec.ts +++ b/src/modules/analysis/analysis.controller.spec.ts @@ -2,6 +2,10 @@ import { Test, TestingModule } from '@nestjs/testing'; import { AnalysisController } from './analysis.controller'; import { PipelineOrchestratorService } from './services/pipeline-orchestrator.service'; import { PipelineStatus } from './enums'; +import { + auditTestProviders, + overrideAuditInterceptors, +} from '../audit/testing/audit-test.helpers'; const makeMockPipeline = ( overrides: Partial> = {}, @@ -47,15 +51,18 @@ describe('AnalysisController', () => { GetRecommendations: jest.fn(), }; - const module: TestingModule = await Test.createTestingModule({ + const builder = Test.createTestingModule({ controllers: [AnalysisController], providers: [ { provide: PipelineOrchestratorService, useValue: mockOrchestrator, }, + ...auditTestProviders(), ], - }).compile(); + }); + const module: TestingModule = + await overrideAuditInterceptors(builder).compile(); controller = module.get(AnalysisController); }); diff --git a/src/modules/analysis/analysis.controller.ts b/src/modules/analysis/analysis.controller.ts index 1344e5b..356dfa0 100644 --- a/src/modules/analysis/analysis.controller.ts +++ b/src/modules/analysis/analysis.controller.ts @@ -1,6 +1,18 @@ -import { Body, Controller, Get, Param, Post, Req } from '@nestjs/common'; +import { + Body, + Controller, + Get, + Param, + Post, + Req, + UseInterceptors, +} from '@nestjs/common'; import { ApiTags, ApiOperation } from '@nestjs/swagger'; import { UseJwtGuard } from 'src/security/decorators'; +import { MetaDataInterceptor } from '../common/interceptors/metadata.interceptor'; +import { AuditInterceptor } from '../audit/interceptors/audit.interceptor'; +import { Audited } from '../audit/decorators/audited.decorator'; +import { AuditAction } from '../audit/audit-action.enum'; import type { AuthenticatedRequest } from '../common/interceptors/http/authenticated-request'; import { PipelineOrchestratorService } from './services/pipeline-orchestrator.service'; import { CreatePipelineDto } from './dto/create-pipeline.dto'; @@ -13,6 +25,11 @@ export class AnalysisController { constructor(private readonly orchestrator: PipelineOrchestratorService) {} @Post('pipelines') + @Audited({ + action: AuditAction.ANALYSIS_PIPELINE_CREATE, + resource: 'AnalysisPipeline', + }) + @UseInterceptors(MetaDataInterceptor, AuditInterceptor) @ApiOperation({ summary: 'Create an analysis pipeline' }) async CreatePipeline( @Body() body: CreatePipelineDto, @@ -26,6 +43,11 @@ export class AnalysisController { } @Post('pipelines/:id/confirm') + @Audited({ + action: AuditAction.ANALYSIS_PIPELINE_CONFIRM, + resource: 'AnalysisPipeline', + }) + @UseInterceptors(MetaDataInterceptor, AuditInterceptor) @ApiOperation({ summary: 'Confirm and start pipeline execution' }) async ConfirmPipeline(@Param('id') id: string) { const pipeline = await this.orchestrator.ConfirmPipeline(id); @@ -33,6 +55,11 @@ export class AnalysisController { } @Post('pipelines/:id/cancel') + @Audited({ + action: AuditAction.ANALYSIS_PIPELINE_CANCEL, + resource: 'AnalysisPipeline', + }) + @UseInterceptors(MetaDataInterceptor, AuditInterceptor) @ApiOperation({ summary: 'Cancel a non-terminal pipeline' }) async CancelPipeline(@Param('id') id: string) { const pipeline = await this.orchestrator.CancelPipeline(id); diff --git a/src/modules/audit/audit-action.enum.ts b/src/modules/audit/audit-action.enum.ts new file mode 100644 index 0000000..d057556 --- /dev/null +++ b/src/modules/audit/audit-action.enum.ts @@ -0,0 +1,16 @@ +export const AuditAction = { + AUTH_LOGIN_SUCCESS: 'auth.login.success', + AUTH_LOGIN_FAILURE: 'auth.login.failure', + AUTH_LOGOUT: 'auth.logout', + AUTH_TOKEN_REFRESH: 'auth.token.refresh', + ADMIN_SYNC_TRIGGER: 'admin.sync.trigger', + ADMIN_SYNC_SCHEDULE_UPDATE: 'admin.sync-schedule.update', + QUESTIONNAIRE_SUBMIT: 'questionnaire.submit', + QUESTIONNAIRE_INGEST: 'questionnaire.ingest', + QUESTIONNAIRE_SUBMISSIONS_WIPE: 'questionnaire.submissions.wipe', + ANALYSIS_PIPELINE_CREATE: 'analysis.pipeline.create', + ANALYSIS_PIPELINE_CONFIRM: 'analysis.pipeline.confirm', + ANALYSIS_PIPELINE_CANCEL: 'analysis.pipeline.cancel', +} as const; + +export type AuditAction = (typeof AuditAction)[keyof typeof AuditAction]; diff --git a/src/modules/audit/audit.module.ts b/src/modules/audit/audit.module.ts new file mode 100644 index 0000000..f9b51f2 --- /dev/null +++ b/src/modules/audit/audit.module.ts @@ -0,0 +1,21 @@ +import { Global, Module } from '@nestjs/common'; +import { BullModule } from '@nestjs/bullmq'; +import { MikroOrmModule } from '@mikro-orm/nestjs'; +import { QueueName } from 'src/configurations/common/queue-names'; +import { AuditLog } from 'src/entities/audit-log.entity'; +import { AppClsModule } from '../common/cls/cls.module'; +import { AuditService } from './audit.service'; +import { AuditProcessor } from './audit.processor'; +import { AuditInterceptor } from './interceptors/audit.interceptor'; + +@Global() +@Module({ + imports: [ + BullModule.registerQueue({ name: QueueName.AUDIT }), + MikroOrmModule.forFeature([AuditLog]), + AppClsModule, + ], + providers: [AuditService, AuditProcessor, AuditInterceptor], + exports: [AuditService, AuditInterceptor], +}) +export class AuditModule {} diff --git a/src/modules/audit/audit.processor.spec.ts b/src/modules/audit/audit.processor.spec.ts new file mode 100644 index 0000000..806d458 --- /dev/null +++ b/src/modules/audit/audit.processor.spec.ts @@ -0,0 +1,125 @@ +import { Test, TestingModule } from '@nestjs/testing'; +import { EntityManager } from '@mikro-orm/postgresql'; +import { AuditProcessor } from './audit.processor'; +import { AuditLog } from 'src/entities/audit-log.entity'; +import { AuditAction } from './audit-action.enum'; +import type { AuditJobMessage } from './dto/audit-job-message.dto'; +import type { Job } from 'bullmq'; + +describe('AuditProcessor', () => { + let processor: AuditProcessor; + let mockFork: { create: jest.Mock; flush: jest.Mock }; + let mockEm: { fork: jest.Mock }; + + beforeEach(async () => { + mockFork = { + create: jest.fn(), + flush: jest.fn().mockResolvedValue(undefined), + }; + mockEm = { fork: jest.fn().mockReturnValue(mockFork) }; + + const module: TestingModule = await Test.createTestingModule({ + providers: [AuditProcessor, { provide: EntityManager, useValue: mockEm }], + }).compile(); + + processor = module.get(AuditProcessor); + }); + + it('should be defined', () => { + expect(processor).toBeDefined(); + }); + + it('should persist an AuditLog entity with correct fields', async () => { + const jobData: AuditJobMessage = { + action: AuditAction.AUTH_LOGIN_SUCCESS, + actorId: 'user-1', + actorUsername: 'admin', + resourceType: 'User', + resourceId: 'user-1', + metadata: { strategyUsed: 'LocalLoginStrategy' }, + browserName: 'Chrome', + os: 'Linux', + ipAddress: '127.0.0.1', + occurredAt: '2026-03-29T12:00:00.000Z', + }; + + const mockJob = { data: jobData } as Job; + + await processor.process(mockJob); + + expect(mockEm.fork).toHaveBeenCalled(); + expect(mockFork.create).toHaveBeenCalledWith(AuditLog, { + action: AuditAction.AUTH_LOGIN_SUCCESS, + actorId: 'user-1', + actorUsername: 'admin', + resourceType: 'User', + resourceId: 'user-1', + metadata: { strategyUsed: 'LocalLoginStrategy' }, + browserName: 'Chrome', + os: 'Linux', + ipAddress: '127.0.0.1', + occurredAt: new Date('2026-03-29T12:00:00.000Z'), + }); + expect(mockFork.flush).toHaveBeenCalled(); + }); + + it('should fork the entity manager for each job', async () => { + const jobData: AuditJobMessage = { + action: AuditAction.AUTH_LOGOUT, + occurredAt: new Date().toISOString(), + }; + + await processor.process({ data: jobData } as Job); + await processor.process({ data: jobData } as Job); + + expect(mockEm.fork).toHaveBeenCalledTimes(2); + }); + + it('should handle job with minimal fields', async () => { + const jobData: AuditJobMessage = { + action: AuditAction.AUTH_LOGOUT, + occurredAt: '2026-03-29T12:00:00.000Z', + }; + + await processor.process({ data: jobData } as Job); + + expect(mockFork.create).toHaveBeenCalledWith(AuditLog, { + action: AuditAction.AUTH_LOGOUT, + actorId: undefined, + actorUsername: undefined, + resourceType: undefined, + resourceId: undefined, + metadata: undefined, + browserName: undefined, + os: undefined, + ipAddress: undefined, + occurredAt: new Date('2026-03-29T12:00:00.000Z'), + }); + }); + + describe('onFailed', () => { + it('should log non-PII fields on failure', () => { + const logSpy = jest.spyOn(processor['logger'], 'error'); + const jobData: AuditJobMessage = { + action: AuditAction.AUTH_LOGIN_FAILURE, + actorId: 'user-1', + resourceType: 'User', + resourceId: 'user-1', + metadata: { username: 'sensitive-data' }, + occurredAt: '2026-03-29T12:00:00.000Z', + }; + + processor.onFailed( + { id: 'job-1', data: jobData, attemptsMade: 1 } as Job, + new Error('DB connection lost'), + ); + + expect(logSpy).toHaveBeenCalledTimes(1); + const logMessage = logSpy.mock.calls[0][0] as string; + expect(logMessage).toContain('action=auth.login.failure'); + expect(logMessage).toContain('actorId=user-1'); + expect(logMessage).toContain('DB connection lost'); + expect(logMessage).not.toContain('sensitive-data'); + }); + }); +}); diff --git a/src/modules/audit/audit.processor.ts b/src/modules/audit/audit.processor.ts new file mode 100644 index 0000000..34ad970 --- /dev/null +++ b/src/modules/audit/audit.processor.ts @@ -0,0 +1,58 @@ +import { Logger } from '@nestjs/common'; +import { Processor, WorkerHost, OnWorkerEvent } from '@nestjs/bullmq'; +import { Job } from 'bullmq'; +import { EntityManager } from '@mikro-orm/postgresql'; +import { QueueName } from 'src/configurations/common/queue-names'; +import { AuditLog } from 'src/entities/audit-log.entity'; +import type { AuditJobMessage } from './dto/audit-job-message.dto'; + +@Processor(QueueName.AUDIT, { concurrency: 1 }) +export class AuditProcessor extends WorkerHost { + private readonly logger = new Logger(AuditProcessor.name); + + constructor(private readonly em: EntityManager) { + super(); + } + + async process(job: Job): Promise { + const { + action, + actorId, + actorUsername, + resourceType, + resourceId, + metadata, + browserName, + os, + ipAddress, + occurredAt, + } = job.data; + + const fork = this.em.fork(); + fork.create(AuditLog, { + action, + actorId, + actorUsername, + resourceType, + resourceId, + metadata, + browserName, + os, + ipAddress, + occurredAt: new Date(occurredAt), + }); + await fork.flush(); + + this.logger.log(`Persisted audit log: ${action}`); + } + + @OnWorkerEvent('failed') + onFailed(job: Job, error: Error) { + this.logger.error( + `Audit job ${job.id} failed (attempt ${job.attemptsMade}): ` + + `action=${job.data.action}, actorId=${job.data.actorId}, ` + + `resourceType=${job.data.resourceType}, resourceId=${job.data.resourceId}, ` + + `occurredAt=${job.data.occurredAt} — ${error.message}`, + ); + } +} diff --git a/src/modules/audit/audit.service.spec.ts b/src/modules/audit/audit.service.spec.ts new file mode 100644 index 0000000..c5e645b --- /dev/null +++ b/src/modules/audit/audit.service.spec.ts @@ -0,0 +1,83 @@ +import { Test, TestingModule } from '@nestjs/testing'; +import { getQueueToken } from '@nestjs/bullmq'; +import { QueueName } from 'src/configurations/common/queue-names'; +import { AuditService } from './audit.service'; +import { AuditAction } from './audit-action.enum'; + +describe('AuditService', () => { + let service: AuditService; + let mockQueue: { add: jest.Mock }; + + beforeEach(async () => { + mockQueue = { add: jest.fn().mockResolvedValue({ id: 'job-1' }) }; + + const module: TestingModule = await Test.createTestingModule({ + providers: [ + AuditService, + { provide: getQueueToken(QueueName.AUDIT), useValue: mockQueue }, + ], + }).compile(); + + service = module.get(AuditService); + }); + + it('should be defined', () => { + expect(service).toBeDefined(); + }); + + it('should enqueue an audit event with correct envelope', async () => { + await service.Emit({ + action: AuditAction.AUTH_LOGIN_SUCCESS, + actorId: 'user-1', + actorUsername: 'admin', + metadata: { strategyUsed: 'LocalLoginStrategy' }, + browserName: 'Chrome', + os: 'Linux', + ipAddress: '127.0.0.1', + }); + + expect(mockQueue.add).toHaveBeenCalledTimes(1); + const [name, envelope, opts] = mockQueue.add.mock.calls[0] as [ + string, + Record, + Record, + ]; + expect(name).toBe('audit'); + expect(envelope.action).toBe(AuditAction.AUTH_LOGIN_SUCCESS); + expect(envelope.actorId).toBe('user-1'); + expect(envelope.actorUsername).toBe('admin'); + expect(envelope.metadata).toEqual({ + strategyUsed: 'LocalLoginStrategy', + }); + expect(envelope.occurredAt).toBeDefined(); + expect(opts).toEqual({ + attempts: 1, + removeOnComplete: true, + removeOnFail: 100, + }); + }); + + it('should catch and log Redis errors without throwing', async () => { + mockQueue.add.mockRejectedValue(new Error('ECONNREFUSED')); + + await expect( + service.Emit({ + action: AuditAction.AUTH_LOGOUT, + actorId: 'user-1', + }), + ).resolves.toBeUndefined(); + }); + + it('should pass optional fields as undefined when not provided', async () => { + await service.Emit({ action: AuditAction.AUTH_LOGOUT }); + + const [, envelope] = mockQueue.add.mock.calls[0] as [ + string, + Record, + ]; + expect(envelope.actorId).toBeUndefined(); + expect(envelope.actorUsername).toBeUndefined(); + expect(envelope.resourceType).toBeUndefined(); + expect(envelope.resourceId).toBeUndefined(); + }); +}); diff --git a/src/modules/audit/audit.service.ts b/src/modules/audit/audit.service.ts new file mode 100644 index 0000000..57f2014 --- /dev/null +++ b/src/modules/audit/audit.service.ts @@ -0,0 +1,36 @@ +import { Injectable, Logger } from '@nestjs/common'; +import { InjectQueue } from '@nestjs/bullmq'; +import { Queue } from 'bullmq'; +import { QueueName } from 'src/configurations/common/queue-names'; +import type { AuditJobMessage } from './dto/audit-job-message.dto'; +import type { EmitParams } from './dto/emit-params.dto'; + +@Injectable() +export class AuditService { + private readonly logger = new Logger(AuditService.name); + + constructor( + @InjectQueue(QueueName.AUDIT) private readonly auditQueue: Queue, + ) {} + + async Emit(params: EmitParams): Promise { + const envelope: AuditJobMessage = { + ...params, + occurredAt: new Date().toISOString(), + }; + + try { + await this.auditQueue.add('audit', envelope, { + attempts: 1, + removeOnComplete: true, + removeOnFail: 100, + }); + } catch (error) { + this.logger.warn( + `Failed to enqueue audit event: action=${params.action}, ` + + `actorId=${params.actorId}, resourceType=${params.resourceType}, ` + + `resourceId=${params.resourceId} — ${(error as Error).message}`, + ); + } + } +} diff --git a/src/modules/audit/decorators/audited.decorator.ts b/src/modules/audit/decorators/audited.decorator.ts new file mode 100644 index 0000000..e64c4ec --- /dev/null +++ b/src/modules/audit/decorators/audited.decorator.ts @@ -0,0 +1,12 @@ +import { SetMetadata } from '@nestjs/common'; +import type { AuditAction } from '../audit-action.enum'; + +export const AUDIT_META_KEY = 'audit:meta'; + +export interface AuditedOptions { + action: AuditAction; + resource?: string; +} + +export const Audited = (options: AuditedOptions) => + SetMetadata(AUDIT_META_KEY, options); diff --git a/src/modules/audit/dto/audit-job-message.dto.ts b/src/modules/audit/dto/audit-job-message.dto.ts new file mode 100644 index 0000000..a6b64aa --- /dev/null +++ b/src/modules/audit/dto/audit-job-message.dto.ts @@ -0,0 +1,14 @@ +import type { AuditAction } from '../audit-action.enum'; + +export interface AuditJobMessage { + action: AuditAction; + actorId?: string; + actorUsername?: string; + resourceType?: string; + resourceId?: string; + metadata?: Record; + browserName?: string; + os?: string; + ipAddress?: string; + occurredAt: string; // ISO timestamp +} diff --git a/src/modules/audit/dto/emit-params.dto.ts b/src/modules/audit/dto/emit-params.dto.ts new file mode 100644 index 0000000..16030b8 --- /dev/null +++ b/src/modules/audit/dto/emit-params.dto.ts @@ -0,0 +1,13 @@ +import type { AuditAction } from '../audit-action.enum'; + +export interface EmitParams { + action: AuditAction; + actorId?: string; + actorUsername?: string; + resourceType?: string; + resourceId?: string; + metadata?: Record; + browserName?: string; + os?: string; + ipAddress?: string; +} diff --git a/src/modules/audit/interceptors/audit.interceptor.spec.ts b/src/modules/audit/interceptors/audit.interceptor.spec.ts new file mode 100644 index 0000000..350384f --- /dev/null +++ b/src/modules/audit/interceptors/audit.interceptor.spec.ts @@ -0,0 +1,249 @@ +import { ExecutionContext, CallHandler } from '@nestjs/common'; +import { Reflector } from '@nestjs/core'; +import { of, throwError } from 'rxjs'; +import { AuditInterceptor } from './audit.interceptor'; +import { AuditService } from '../audit.service'; +import { CurrentUserService } from 'src/modules/common/cls/current-user.service'; +import { RequestMetadataService } from 'src/modules/common/cls/request-metadata.service'; +import { AuditAction } from '../audit-action.enum'; +import { AUDIT_META_KEY } from '../decorators/audited.decorator'; + +describe('AuditInterceptor', () => { + let interceptor: AuditInterceptor; + let reflector: jest.Mocked; + let auditService: { Emit: jest.Mock }; + let currentUserService: { get: jest.Mock }; + let requestMetadataService: { get: jest.Mock }; + + const mockHandler = (): jest.Mocked => ({ + handle: jest.fn().mockReturnValue(of({ success: true })), + }); + + const mockContext = ( + params: Record = {}, + query: Record = {}, + user?: { userId: string }, + ): jest.Mocked => + ({ + getHandler: jest.fn().mockReturnValue(() => {}), + getClass: jest.fn().mockReturnValue({ name: 'TestController' }), + switchToHttp: jest.fn().mockReturnValue({ + getRequest: jest.fn().mockReturnValue({ + params, + query, + user, + }), + }), + }) as unknown as jest.Mocked; + + beforeEach(() => { + reflector = { get: jest.fn() } as unknown as jest.Mocked; + auditService = { Emit: jest.fn().mockResolvedValue(undefined) }; + currentUserService = { get: jest.fn().mockReturnValue(null) }; + requestMetadataService = { + get: jest.fn().mockReturnValue({ + browserName: 'Chrome', + os: 'Linux', + ipAddress: '127.0.0.1', + }), + }; + + interceptor = new AuditInterceptor( + reflector, + auditService as unknown as AuditService, + currentUserService as unknown as CurrentUserService, + requestMetadataService as unknown as RequestMetadataService, + ); + }); + + it('should pass through when no @Audited() metadata', (done) => { + reflector.get.mockReturnValue(undefined); + const handler = mockHandler(); + const context = mockContext(); + + interceptor.intercept(context, handler).subscribe({ + complete: () => { + expect(auditService.Emit).not.toHaveBeenCalled(); + done(); + }, + }); + }); + + it('should emit audit event after successful response', (done) => { + reflector.get.mockReturnValue({ + action: AuditAction.AUTH_LOGOUT, + resource: 'User', + }); + currentUserService.get.mockReturnValue({ + id: 'user-1', + userName: 'admin', + }); + + const handler = mockHandler(); + const context = mockContext({}, {}, { userId: 'user-1' }); + + interceptor.intercept(context, handler).subscribe({ + complete: () => { + expect(auditService.Emit).toHaveBeenCalledWith( + expect.objectContaining({ + action: AuditAction.AUTH_LOGOUT, + actorId: 'user-1', + actorUsername: 'admin', + resourceType: 'User', + browserName: 'Chrome', + os: 'Linux', + ipAddress: '127.0.0.1', + }), + ); + done(); + }, + }); + }); + + it('should extract UUID resourceId from route params', (done) => { + reflector.get.mockReturnValue({ + action: AuditAction.ANALYSIS_PIPELINE_CONFIRM, + resource: 'AnalysisPipeline', + }); + + const pipelineId = '550e8400-e29b-41d4-a716-446655440000'; + const handler = mockHandler(); + const context = mockContext({ id: pipelineId }, {}, { userId: 'user-1' }); + + interceptor.intercept(context, handler).subscribe({ + complete: () => { + expect(auditService.Emit).toHaveBeenCalledWith( + expect.objectContaining({ + resourceId: pipelineId, + resourceType: 'AnalysisPipeline', + }), + ); + done(); + }, + }); + }); + + it('should fall back to req.user.userId when CLS user is null', (done) => { + reflector.get.mockReturnValue({ + action: AuditAction.AUTH_LOGOUT, + resource: 'User', + }); + currentUserService.get.mockReturnValue(null); + + const handler = mockHandler(); + const context = mockContext({}, {}, { userId: 'jwt-user-id' }); + + interceptor.intercept(context, handler).subscribe({ + complete: () => { + expect(auditService.Emit).toHaveBeenCalledWith( + expect.objectContaining({ + actorId: 'jwt-user-id', + actorUsername: undefined, + }), + ); + done(); + }, + }); + }); + + it('should not emit on error responses (tap, not finalize)', (done) => { + reflector.get.mockReturnValue({ + action: AuditAction.AUTH_LOGOUT, + resource: 'User', + }); + + const handler: jest.Mocked = { + handle: jest.fn().mockReturnValue(throwError(() => new Error('fail'))), + }; + const context = mockContext(); + + interceptor.intercept(context, handler).subscribe({ + error: () => { + expect(auditService.Emit).not.toHaveBeenCalled(); + done(); + }, + }); + }); + + it('should not propagate errors from AuditService.Emit()', (done) => { + reflector.get.mockReturnValue({ + action: AuditAction.AUTH_LOGOUT, + resource: 'User', + }); + auditService.Emit.mockRejectedValue(new Error('Redis down')); + + const handler = mockHandler(); + const context = mockContext({}, {}, { userId: 'user-1' }); + + interceptor.intercept(context, handler).subscribe({ + next: (value) => { + expect(value).toEqual({ success: true }); + }, + complete: () => { + done(); + }, + }); + }); + + it('should capture route params and query in metadata', (done) => { + reflector.get.mockReturnValue({ + action: AuditAction.QUESTIONNAIRE_SUBMISSIONS_WIPE, + resource: 'QuestionnaireVersion', + }); + + const versionId = '550e8400-e29b-41d4-a716-446655440000'; + const handler = mockHandler(); + const context = mockContext( + { versionId }, + { dryRun: 'true' }, + { userId: 'user-1' }, + ); + + interceptor.intercept(context, handler).subscribe({ + complete: () => { + expect(auditService.Emit).toHaveBeenCalledWith( + expect.objectContaining({ + metadata: { versionId, dryRun: 'true' }, + resourceId: versionId, + }), + ); + done(); + }, + }); + }); + + it('should set metadata to undefined when params and query are empty', (done) => { + reflector.get.mockReturnValue({ + action: AuditAction.AUTH_LOGOUT, + resource: 'User', + }); + + const handler = mockHandler(); + const context = mockContext({}, {}, { userId: 'user-1' }); + + interceptor.intercept(context, handler).subscribe({ + complete: () => { + expect(auditService.Emit).toHaveBeenCalledWith( + expect.objectContaining({ + metadata: undefined, + }), + ); + done(); + }, + }); + }); + + it('should read metadata from Reflector with AUDIT_META_KEY', () => { + reflector.get.mockReturnValue(undefined); + const handler = mockHandler(); + const context = mockContext(); + + interceptor.intercept(context, handler); + + // eslint-disable-next-line @typescript-eslint/unbound-method + expect(reflector.get).toHaveBeenCalledWith( + AUDIT_META_KEY, + context.getHandler(), + ); + }); +}); diff --git a/src/modules/audit/interceptors/audit.interceptor.ts b/src/modules/audit/interceptors/audit.interceptor.ts new file mode 100644 index 0000000..3f59290 --- /dev/null +++ b/src/modules/audit/interceptors/audit.interceptor.ts @@ -0,0 +1,104 @@ +import { + CallHandler, + ExecutionContext, + Injectable, + Logger, + NestInterceptor, +} from '@nestjs/common'; +import { Reflector } from '@nestjs/core'; +import { tap } from 'rxjs'; +import { AuditService } from '../audit.service'; +import { CurrentUserService } from 'src/modules/common/cls/current-user.service'; +import { RequestMetadataService } from 'src/modules/common/cls/request-metadata.service'; +import { + AUDIT_META_KEY, + type AuditedOptions, +} from '../decorators/audited.decorator'; +import type { AuthenticatedRequest } from 'src/modules/common/interceptors/http/authenticated-request'; + +const UUID_V4_REGEX = + /^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i; + +const MAX_METADATA_BYTES = 4096; + +@Injectable() +export class AuditInterceptor implements NestInterceptor { + private readonly logger = new Logger(AuditInterceptor.name); + + constructor( + private readonly reflector: Reflector, + private readonly auditService: AuditService, + private readonly currentUserService: CurrentUserService, + private readonly requestMetadataService: RequestMetadataService, + ) {} + + intercept(context: ExecutionContext, next: CallHandler) { + const auditMeta = this.reflector.get( + AUDIT_META_KEY, + context.getHandler(), + ); + + if (!auditMeta) { + return next.handle(); + } + + const request: AuthenticatedRequest = context.switchToHttp().getRequest(); + + return next.handle().pipe( + tap(() => { + try { + const user = this.currentUserService.get(); + const actorId = user?.id ?? request.user?.userId; + const actorUsername = user?.userName ?? undefined; + + const meta = this.requestMetadataService.get(); + if (!meta) { + this.logger.warn( + `Missing CLS metadata for ${context.getClass().name}.${context.getHandler().name}`, + ); + } + + const params: Record = + (request.params as Record) ?? {}; + const query: Record = + (request.query as Record) ?? {}; + const rawMetadata: Record = { + ...params, + ...query, + }; + + const resourceId = + Object.values(params).find((v) => UUID_V4_REGEX.test(v)) ?? + undefined; + + let metadata: Record | undefined; + if (Object.keys(rawMetadata).length > 0) { + const serialized = JSON.stringify(rawMetadata); + metadata = + serialized.length <= MAX_METADATA_BYTES ? rawMetadata : undefined; + } + + this.auditService + .Emit({ + action: auditMeta.action, + actorId, + actorUsername, + resourceType: auditMeta.resource, + resourceId, + metadata, + browserName: meta?.browserName, + os: meta?.os, + ipAddress: meta?.ipAddress, + }) + .catch((err: Error) => { + this.logger.error(`Audit emit error: ${err.message}`); + }); + } catch (error) { + this.logger.error( + `Audit interceptor error: ${(error as Error).message}`, + ); + } + }), + ); + } +} diff --git a/src/modules/audit/testing/audit-test.helpers.ts b/src/modules/audit/testing/audit-test.helpers.ts new file mode 100644 index 0000000..a724f68 --- /dev/null +++ b/src/modules/audit/testing/audit-test.helpers.ts @@ -0,0 +1,33 @@ +import type { TestingModuleBuilder } from '@nestjs/testing'; +import type { Provider } from '@nestjs/common'; +import { RequestMetadataService } from 'src/modules/common/cls/request-metadata.service'; +import { AuditService } from '../audit.service'; +import { MetaDataInterceptor } from 'src/modules/common/interceptors/metadata.interceptor'; +import { AuditInterceptor } from '../interceptors/audit.interceptor'; + +const noop = { + intercept: (_ctx: unknown, next: { handle: () => unknown }) => next.handle(), +}; + +export function auditTestProviders(): Provider[] { + return [ + { + provide: RequestMetadataService, + useValue: { get: jest.fn(), set: jest.fn() }, + }, + { + provide: AuditService, + useValue: { Emit: jest.fn().mockResolvedValue(undefined) }, + }, + ]; +} + +export function overrideAuditInterceptors( + builder: TestingModuleBuilder, +): TestingModuleBuilder { + return builder + .overrideInterceptor(MetaDataInterceptor) + .useValue(noop) + .overrideInterceptor(AuditInterceptor) + .useValue(noop); +} diff --git a/src/modules/auth/auth.controller.ts b/src/modules/auth/auth.controller.ts index d0d6d0e..6e9ef63 100644 --- a/src/modules/auth/auth.controller.ts +++ b/src/modules/auth/auth.controller.ts @@ -12,6 +12,9 @@ import { LoginRequest } from './dto/requests/login.request.dto'; import { CurrentUserInterceptor } from '../common/interceptors/current-user.interceptor'; import { Throttle, UseJwtGuard } from 'src/security/decorators'; import { MetaDataInterceptor } from '../common/interceptors/metadata.interceptor'; +import { AuditInterceptor } from '../audit/interceptors/audit.interceptor'; +import { Audited } from '../audit/decorators/audited.decorator'; +import { AuditAction } from '../audit/audit-action.enum'; import { JwtRefreshGuard } from 'src/security/guards/refresh-jwt-auth.guard'; import type { RefreshTokenRequest } from '../common/interceptors/http/refresh-token-request'; import { RefreshTokenRequestBody } from './dto/requests/refresh-token.request.dto'; @@ -50,7 +53,12 @@ export class AuthController { @Post('logout') @UseJwtGuard() - @UseInterceptors(CurrentUserInterceptor) + @Audited({ action: AuditAction.AUTH_LOGOUT, resource: 'User' }) + @UseInterceptors( + MetaDataInterceptor, + CurrentUserInterceptor, + AuditInterceptor, + ) async Logout() { await this.authService.Logout(); return { message: 'Logged out successfully' }; diff --git a/src/modules/auth/auth.service.spec.ts b/src/modules/auth/auth.service.spec.ts index e77f742..8e49ab7 100644 --- a/src/modules/auth/auth.service.spec.ts +++ b/src/modules/auth/auth.service.spec.ts @@ -10,6 +10,8 @@ import { EntityManager } from '@mikro-orm/postgresql'; import { RefreshToken } from '../../entities/refresh-token.entity'; import { CurrentUserService } from '../common/cls/current-user.service'; import { RequestMetadataService } from '../common/cls/request-metadata.service'; +import { AuditService } from '../audit/audit.service'; +import { AuditAction } from '../audit/audit-action.enum'; import { validate } from 'class-validator'; import { plainToInstance } from 'class-transformer'; import { LoginRequest } from './dto/requests/login.request.dto'; @@ -32,6 +34,7 @@ describe('AuthService', () => { let unitOfWork: UnitOfWork; let mockLocalStrategy: jest.Mocked; let mockMoodleStrategy: jest.Mocked; + let mockAuditService: { Emit: jest.Mock }; beforeEach(async () => { mockLocalStrategy = { @@ -46,6 +49,8 @@ describe('AuthService', () => { Execute: jest.fn(), }; + mockAuditService = { Emit: jest.fn().mockResolvedValue(undefined) }; + const module: TestingModule = await Test.createTestingModule({ providers: [ AuthService, @@ -90,6 +95,10 @@ describe('AuthService', () => { provide: RequestMetadataService, useValue: mockRequestMetadataService, }, + { + provide: AuditService, + useValue: mockAuditService, + }, ], }).compile(); @@ -296,6 +305,102 @@ describe('AuthService', () => { service.Login({ username: 'admin', password: 'wrong-password' }), ).rejects.toThrow(UnauthorizedException); }); + + it('should emit auth.login.failure audit event when no strategy matches', async () => { + const mockEm = { findOne: jest.fn().mockResolvedValue(null) }; + + (unitOfWork.runInTransaction as jest.Mock).mockImplementation( + (cb: (em: EntityManager) => unknown) => + cb(mockEm as unknown as EntityManager), + ); + + mockLocalStrategy.CanHandle.mockReturnValue(false); + mockMoodleStrategy.CanHandle.mockReturnValue(false); + + await expect( + service.Login({ username: 'unknown', password: 'password' }), + ).rejects.toThrow(UnauthorizedException); + + expect(mockAuditService.Emit).toHaveBeenCalledWith( + expect.objectContaining({ + action: AuditAction.AUTH_LOGIN_FAILURE, + metadata: expect.objectContaining({ + username: 'unknown', + reason: 'no_matching_strategy', + }) as Record, + }), + ); + }); + + it('should emit auth.login.failure audit event when strategy throws', async () => { + const mockUser = new User(); + mockUser.userName = 'admin'; + mockUser.password = 'hashed'; + + const mockEm = { findOne: jest.fn().mockResolvedValue(mockUser) }; + + (unitOfWork.runInTransaction as jest.Mock).mockImplementation( + (cb: (em: EntityManager) => unknown) => + cb(mockEm as unknown as EntityManager), + ); + + mockLocalStrategy.CanHandle.mockReturnValue(true); + mockLocalStrategy.Execute.mockRejectedValue( + new UnauthorizedException('Invalid credentials'), + ); + + await expect( + service.Login({ username: 'admin', password: 'wrong' }), + ).rejects.toThrow(UnauthorizedException); + + expect(mockAuditService.Emit).toHaveBeenCalledWith( + expect.objectContaining({ + action: AuditAction.AUTH_LOGIN_FAILURE, + metadata: expect.objectContaining({ + username: 'admin', + reason: 'strategy_execution_failed', + }) as Record, + }), + ); + }); + + it('should emit auth.login.success audit event after transaction completes', async () => { + const mockUser = new User(); + mockUser.id = 'user-id'; + mockUser.userName = 'admin'; + mockUser.moodleUserId = 1; + + const mockEm = { + findOne: jest.fn().mockResolvedValue(mockUser), + getRepository: jest.fn().mockReturnValue({}), + }; + + (unitOfWork.runInTransaction as jest.Mock).mockImplementation( + (cb: (em: EntityManager) => unknown) => + cb(mockEm as unknown as EntityManager), + ); + + mockLocalStrategy.CanHandle.mockReturnValue(true); + mockLocalStrategy.Execute.mockResolvedValue({ user: mockUser }); + + (jwtService.CreateSignedTokens as jest.Mock).mockResolvedValue({ + token: 'access', + refreshToken: 'refresh', + }); + + await service.Login({ username: 'admin', password: 'password123' }); + + expect(mockAuditService.Emit).toHaveBeenCalledWith( + expect.objectContaining({ + action: AuditAction.AUTH_LOGIN_SUCCESS, + actorId: 'user-id', + actorUsername: 'admin', + metadata: expect.objectContaining({ + strategyUsed: expect.any(String) as string, + }) as Record, + }), + ); + }); }); describe('RefreshToken', () => { @@ -363,11 +468,9 @@ describe('AuthService', () => { }); const mockEm = { - getRepository: jest - .fn() - .mockReturnValue({ - find: jest.fn().mockResolvedValue([storedToken]), - }), + getRepository: jest.fn().mockReturnValue({ + find: jest.fn().mockResolvedValue([storedToken]), + }), findOneOrFail: jest.fn(), }; @@ -399,6 +502,42 @@ describe('AuthService', () => { ).rejects.toThrow(UnauthorizedException); }); + it('should emit auth.token.refresh audit event after transaction completes', async () => { + const hashedToken = await bcrypt.hash(rawRefreshToken, 10); + const storedToken = createMockToken({ tokenHash: hashedToken }); + + const mockUser = new User(); + mockUser.id = userId; + mockUser.userName = 'admin'; + mockUser.moodleUserId = 1; + + const mockFind = jest.fn().mockResolvedValue([storedToken]); + const mockEm = { + getRepository: jest.fn().mockReturnValue({ find: mockFind }), + findOneOrFail: jest.fn().mockResolvedValue(mockUser), + }; + + (unitOfWork.runInTransaction as jest.Mock).mockImplementation( + (cb: (em: EntityManager) => unknown) => + cb(mockEm as unknown as EntityManager), + ); + + (jwtService.CreateSignedTokens as jest.Mock).mockResolvedValue({ + token: 'new-access', + refreshToken: 'new-refresh', + }); + + await service.RefreshToken(userId, rawRefreshToken); + + expect(mockAuditService.Emit).toHaveBeenCalledWith( + expect.objectContaining({ + action: AuditAction.AUTH_TOKEN_REFRESH, + actorId: userId, + actorUsername: 'admin', + }), + ); + }); + it('should not use synchronous bcrypt.compareSync', async () => { const hashedToken = await bcrypt.hash(rawRefreshToken, 10); const storedToken = createMockToken({ tokenHash: hashedToken }); @@ -432,6 +571,205 @@ describe('AuthService', () => { }); }); +describe('AuthService without AuditService', () => { + it('should complete login successfully when AuditService is not provided', async () => { + const mockUser = new User(); + mockUser.id = 'user-id'; + mockUser.userName = 'admin'; + mockUser.moodleUserId = 1; + + const mockLocalStrategy: jest.Mocked = { + priority: 10, + CanHandle: jest.fn().mockReturnValue(true), + Execute: jest.fn().mockResolvedValue({ user: mockUser }), + }; + + const module: TestingModule = await Test.createTestingModule({ + providers: [ + AuthService, + { + provide: LOGIN_STRATEGIES, + useValue: [mockLocalStrategy], + }, + { + provide: CustomJwtService, + useValue: { + CreateSignedTokens: jest.fn().mockResolvedValue({ + token: 'access', + refreshToken: 'refresh', + }), + }, + }, + { + provide: UnitOfWork, + useValue: { + runInTransaction: jest + .fn() + .mockImplementation((cb: (em: EntityManager) => unknown) => + cb({ + findOne: jest.fn().mockResolvedValue(mockUser), + } as unknown as EntityManager), + ), + }, + }, + { + provide: CurrentUserService, + useValue: { + get: jest.fn(), + getOrFail: jest.fn().mockReturnValue(mockUser), + getUserId: jest.fn().mockReturnValue('user-id'), + set: jest.fn(), + setJwtPayload: jest.fn(), + }, + }, + { + provide: RequestMetadataService, + useValue: mockRequestMetadataService, + }, + // AuditService intentionally NOT provided + ], + }).compile(); + + const svc = module.get(AuthService); + const result = await svc.Login({ + username: 'admin', + password: 'password123', + }); + + expect(result).toBeDefined(); + expect(result.token).toBe('access'); + }); + + it('should complete refresh successfully when AuditService is not provided', async () => { + const mockUser = new User(); + mockUser.id = 'user-id'; + mockUser.userName = 'admin'; + mockUser.moodleUserId = 1; + + const rawRefreshToken = 'raw-refresh-token'; + const hashedToken = await bcrypt.hash(rawRefreshToken, 10); + + const storedToken = new RefreshToken(); + storedToken.id = 'token-id'; + storedToken.tokenHash = hashedToken; + storedToken.userId = 'user-id'; + storedToken.expiresAt = new Date(Date.now() + 7 * 24 * 60 * 60 * 1000); + storedToken.isActive = true; + storedToken.browserName = 'test'; + storedToken.os = 'test'; + storedToken.ipAddress = '127.0.0.1'; + + const module: TestingModule = await Test.createTestingModule({ + providers: [ + AuthService, + { + provide: LOGIN_STRATEGIES, + useValue: [], + }, + { + provide: CustomJwtService, + useValue: { + CreateSignedTokens: jest.fn().mockResolvedValue({ + token: 'new-access', + refreshToken: 'new-refresh', + }), + }, + }, + { + provide: UnitOfWork, + useValue: { + runInTransaction: jest + .fn() + .mockImplementation((cb: (em: EntityManager) => unknown) => + cb({ + getRepository: jest.fn().mockReturnValue({ + find: jest.fn().mockResolvedValue([storedToken]), + }), + findOneOrFail: jest.fn().mockResolvedValue(mockUser), + } as unknown as EntityManager), + ), + }, + }, + { + provide: CurrentUserService, + useValue: { + get: jest.fn(), + getOrFail: jest.fn().mockReturnValue(mockUser), + getUserId: jest.fn().mockReturnValue('user-id'), + set: jest.fn(), + setJwtPayload: jest.fn(), + }, + }, + { + provide: RequestMetadataService, + useValue: mockRequestMetadataService, + }, + // AuditService intentionally NOT provided + ], + }).compile(); + + const svc = module.get(AuthService); + const result = await svc.RefreshToken('user-id', rawRefreshToken); + + expect(result).toBeDefined(); + expect(result.token).toBe('new-access'); + }); + + it('should still throw UnauthorizedException on login failure when AuditService is not provided', async () => { + const mockLocalStrategy: jest.Mocked = { + priority: 10, + CanHandle: jest.fn().mockReturnValue(false), + Execute: jest.fn(), + }; + + const module: TestingModule = await Test.createTestingModule({ + providers: [ + AuthService, + { + provide: LOGIN_STRATEGIES, + useValue: [mockLocalStrategy], + }, + { + provide: CustomJwtService, + useValue: { CreateSignedTokens: jest.fn() }, + }, + { + provide: UnitOfWork, + useValue: { + runInTransaction: jest + .fn() + .mockImplementation((cb: (em: EntityManager) => unknown) => + cb({ + findOne: jest.fn().mockResolvedValue(null), + } as unknown as EntityManager), + ), + }, + }, + { + provide: CurrentUserService, + useValue: { + get: jest.fn(), + getOrFail: jest.fn(), + getUserId: jest.fn(), + set: jest.fn(), + setJwtPayload: jest.fn(), + }, + }, + { + provide: RequestMetadataService, + useValue: mockRequestMetadataService, + }, + // AuditService intentionally NOT provided + ], + }).compile(); + + const svc = module.get(AuthService); + await expect( + svc.Login({ username: 'bad', password: 'bad' }), + ).rejects.toThrow(UnauthorizedException); + }); +}); + describe('LoginRequest DTO validation', () => { const toDto = (plain: Record) => plainToInstance(LoginRequest, plain); diff --git a/src/modules/auth/auth.service.ts b/src/modules/auth/auth.service.ts index 292ae0b..f455bfc 100644 --- a/src/modules/auth/auth.service.ts +++ b/src/modules/auth/auth.service.ts @@ -2,6 +2,7 @@ import { Inject, Injectable, Logger, + Optional, UnauthorizedException, } from '@nestjs/common'; import { LoginRequest } from './dto/requests/login.request.dto'; @@ -16,9 +17,15 @@ import { v4 } from 'uuid'; import { RefreshToken } from 'src/entities/refresh-token.entity'; import * as bcrypt from 'bcrypt'; import { RefreshTokenRepository } from 'src/repositories/refresh-token.repository'; -import { LOGIN_STRATEGIES, LoginStrategy } from './strategies'; +import { + LOGIN_STRATEGIES, + LoginStrategy, + type LoginStrategyResult, +} from './strategies'; import { CurrentUserService } from '../common/cls/current-user.service'; import { RequestMetadataService } from '../common/cls/request-metadata.service'; +import { AuditService } from '../audit/audit.service'; +import { AuditAction } from '../audit/audit-action.enum'; @Injectable() export class AuthService { @@ -33,6 +40,7 @@ export class AuthService { private readonly unitOfWork: UnitOfWork, private readonly currentUserService: CurrentUserService, private readonly requestMetadataService: RequestMetadataService, + @Optional() private readonly auditService?: AuditService, ) { this.sortedStrategies = [...loginStrategies].sort( (a, b) => a.priority - b.priority, @@ -40,38 +48,81 @@ export class AuthService { } async Login(body: LoginRequest) { - return await this.unitOfWork.runInTransaction(async (em) => { - const localUser = await em.findOne(User, { userName: body.username }); + const { browserName, os, ipAddress } = + this.requestMetadataService.get() ?? {}; - const strategy = this.sortedStrategies.find((s) => - s.CanHandle(localUser, body), - ); + let failureReason: string | undefined; + + try { + const result = await this.unitOfWork.runInTransaction(async (em) => { + const localUser = await em.findOne(User, { userName: body.username }); - if (!strategy) { - this.logger.warn( - 'Login attempt failed: no matching authentication strategy', + const strategy = this.sortedStrategies.find((s) => + s.CanHandle(localUser, body), ); - throw new UnauthorizedException('Invalid credentials'); - } - const result = await strategy.Execute(em, localUser, body); + if (!strategy) { + this.logger.warn( + 'Login attempt failed: no matching authentication strategy', + ); + failureReason = 'no_matching_strategy'; + throw new UnauthorizedException('Invalid credentials'); + } + + let strategyResult: LoginStrategyResult; + try { + strategyResult = await strategy.Execute(em, localUser, body); + } catch (error) { + failureReason = 'strategy_execution_failed'; + throw error; + } + + const jwtPayload = JwtPayload.Create( + strategyResult.user.id, + strategyResult.user.moodleUserId, + ); + const refreshTokenPayload = RefreshJwtPayload.Create( + strategyResult.user.id, + v4(), + ); + const signedTokens = await this.jwtService.CreateSignedTokens({ + jwt: jwtPayload, + refreshJwt: refreshTokenPayload, + userId: strategyResult.user.id, + }); + + return { + response: LoginResponse.Map(signedTokens), + userId: strategyResult.user.id, + username: strategyResult.user.userName, + strategyName: strategy.constructor.name, + }; + }); - const jwtPayload = JwtPayload.Create( - result.user.id, - result.user.moodleUserId, - ); - const refreshTokenPayload = RefreshJwtPayload.Create( - result.user.id, - v4(), - ); - const signedTokens = await this.jwtService.CreateSignedTokens({ - jwt: jwtPayload, - refreshJwt: refreshTokenPayload, - userId: result.user.id, + void this.auditService?.Emit({ + action: AuditAction.AUTH_LOGIN_SUCCESS, + actorId: result.userId, + actorUsername: result.username, + metadata: { strategyUsed: result.strategyName }, + browserName, + os, + ipAddress, }); - return LoginResponse.Map(signedTokens); - }); + return result.response; + } catch (error) { + void this.auditService?.Emit({ + action: AuditAction.AUTH_LOGIN_FAILURE, + metadata: { + username: body.username, + reason: failureReason ?? 'unknown', + }, + browserName, + os, + ipAddress, + }); + throw error; + } } Me() { @@ -80,7 +131,10 @@ export class AuthService { } async RefreshToken(userId: string, refreshToken: string) { - return await this.unitOfWork.runInTransaction(async (em) => { + const { browserName, os, ipAddress } = + this.requestMetadataService.get() ?? {}; + + const result = await this.unitOfWork.runInTransaction(async (em) => { const refreshTokenRepository: RefreshTokenRepository = em.getRepository(RefreshToken); @@ -119,8 +173,23 @@ export class AuthService { matchingToken.replacedByTokenId = refreshTokenPayload.jti; - return LoginResponse.Map(signedTokens); + return { + response: LoginResponse.Map(signedTokens), + userId: user.id, + username: user.userName, + }; + }); + + void this.auditService?.Emit({ + action: AuditAction.AUTH_TOKEN_REFRESH, + actorId: result.userId, + actorUsername: result.username, + browserName, + os, + ipAddress, }); + + return result.response; } async Logout() { diff --git a/src/modules/index.module.ts b/src/modules/index.module.ts index ce4307e..e63ac58 100644 --- a/src/modules/index.module.ts +++ b/src/modules/index.module.ts @@ -21,6 +21,7 @@ import { DimensionsModule } from './dimensions/dimensions.module'; import { FacultyModule } from './faculty/faculty.module'; import { CurriculumModule } from './curriculum/curriculum.module'; import { AdminModule } from './admin/admin.module'; +import { AuditModule } from './audit/audit.module'; import { ThrottlerModule } from '@nestjs/throttler'; import { ThrottlerStorageRedisService } from '@nest-lab/throttler-storage-redis'; import { LoggerModule } from 'nestjs-pino'; @@ -43,6 +44,7 @@ export const ApplicationModules = [ FacultyModule, CurriculumModule, AdminModule, + AuditModule, ]; export const InfrastructureModules = [ diff --git a/src/modules/moodle/controllers/moodle-sync.controller.spec.ts b/src/modules/moodle/controllers/moodle-sync.controller.spec.ts index 59658c3..d18e8e7 100644 --- a/src/modules/moodle/controllers/moodle-sync.controller.spec.ts +++ b/src/modules/moodle/controllers/moodle-sync.controller.spec.ts @@ -7,6 +7,10 @@ import { QueueName } from 'src/configurations/common/queue-names'; import { RolesGuard } from 'src/security/guards/roles.guard'; import { CurrentUserService } from 'src/modules/common/cls/current-user.service'; import { CurrentUserInterceptor } from 'src/modules/common/interceptors/current-user.interceptor'; +import { + auditTestProviders, + overrideAuditInterceptors, +} from 'src/modules/audit/testing/audit-test.helpers'; import { validate } from 'class-validator'; import { MoodleSyncController } from './moodle-sync.controller'; import { MoodleSyncScheduler } from '../schedulers/moodle-sync.scheduler'; @@ -60,7 +64,7 @@ describe('MoodleSyncController', () => { getOrFail: jest.fn().mockReturnValue({ id: 'user-1' }), }; - const module: TestingModule = await Test.createTestingModule({ + const builder = Test.createTestingModule({ controllers: [MoodleSyncController], providers: [ { @@ -79,18 +83,22 @@ describe('MoodleSyncController', () => { provide: CurrentUserService, useValue: mockCurrentUserService, }, + ...auditTestProviders(), ], - }) - .overrideGuard(AuthGuard('jwt')) - .useValue({ canActivate: () => true }) - .overrideGuard(RolesGuard) - .useValue({ canActivate: () => true }) - .overrideInterceptor(CurrentUserInterceptor) - .useValue({ - intercept: (_ctx: unknown, next: { handle: () => unknown }) => - next.handle(), - }) - .compile(); + }); + + const module: TestingModule = await overrideAuditInterceptors( + builder + .overrideGuard(AuthGuard('jwt')) + .useValue({ canActivate: () => true }) + .overrideGuard(RolesGuard) + .useValue({ canActivate: () => true }) + .overrideInterceptor(CurrentUserInterceptor) + .useValue({ + intercept: (_ctx: unknown, next: { handle: () => unknown }) => + next.handle(), + }), + ).compile(); controller = module.get(MoodleSyncController); }); diff --git a/src/modules/moodle/controllers/moodle-sync.controller.ts b/src/modules/moodle/controllers/moodle-sync.controller.ts index bf9720f..8889ab4 100644 --- a/src/modules/moodle/controllers/moodle-sync.controller.ts +++ b/src/modules/moodle/controllers/moodle-sync.controller.ts @@ -23,6 +23,10 @@ import { Queue } from 'bullmq'; import { EntityManager } from '@mikro-orm/core'; import { QueueName } from 'src/configurations/common/queue-names'; import { UseJwtGuard } from 'src/security/decorators'; +import { Audited } from 'src/modules/audit/decorators/audited.decorator'; +import { AuditAction } from 'src/modules/audit/audit-action.enum'; +import { AuditInterceptor } from 'src/modules/audit/interceptors/audit.interceptor'; +import { MetaDataInterceptor } from 'src/modules/common/interceptors/metadata.interceptor'; import { UserRole } from 'src/modules/auth/roles.enum'; import { CurrentUserService } from 'src/modules/common/cls/current-user.service'; import { CurrentUserInterceptor } from 'src/modules/common/interceptors/current-user.interceptor'; @@ -105,7 +109,12 @@ export class MoodleSyncController { description: 'Sync already in progress or queued', }) @ApiResponse({ status: 503, description: 'Sync queue unavailable' }) - @UseInterceptors(CurrentUserInterceptor) + @Audited({ action: AuditAction.ADMIN_SYNC_TRIGGER, resource: 'SyncLog' }) + @UseInterceptors( + MetaDataInterceptor, + CurrentUserInterceptor, + AuditInterceptor, + ) async TriggerSync(): Promise { try { const [activeCount, waitingCount] = await Promise.all([ @@ -198,6 +207,11 @@ export class MoodleSyncController { @Put('sync/schedule') @UseJwtGuard(UserRole.SUPER_ADMIN) + @Audited({ + action: AuditAction.ADMIN_SYNC_SCHEDULE_UPDATE, + resource: 'SystemConfig', + }) + @UseInterceptors(MetaDataInterceptor, AuditInterceptor) @ApiOperation({ summary: 'Update sync schedule interval' }) @ApiBearerAuth() @ApiResponse({ status: 200, type: SyncScheduleResponseDto }) diff --git a/src/modules/questionnaires/questionnaire.controller.spec.ts b/src/modules/questionnaires/questionnaire.controller.spec.ts index a7c83b0..39b3d34 100644 --- a/src/modules/questionnaires/questionnaire.controller.spec.ts +++ b/src/modules/questionnaires/questionnaire.controller.spec.ts @@ -17,6 +17,10 @@ import { } from './lib/questionnaire.types'; import { RolesGuard } from 'src/security/guards/roles.guard'; import { CurrentUserInterceptor } from '../common/interceptors/current-user.interceptor'; +import { + auditTestProviders, + overrideAuditInterceptors, +} from '../audit/testing/audit-test.helpers'; import { AuthGuard } from '@nestjs/passport'; describe('QuestionnaireController - checkSubmission', () => { @@ -24,7 +28,7 @@ describe('QuestionnaireController - checkSubmission', () => { let questionnaireService: jest.Mocked; beforeEach(async () => { - const module: TestingModule = await Test.createTestingModule({ + const builder = Test.createTestingModule({ controllers: [QuestionnaireController], providers: [ { @@ -58,17 +62,21 @@ describe('QuestionnaireController - checkSubmission', () => { provide: CSVAdapter, useValue: new CSVAdapter(), }, + ...auditTestProviders(), ], - }) - .overrideGuard(AuthGuard('jwt')) - .useValue({ canActivate: () => true }) - .overrideGuard(RolesGuard) - .useValue({ canActivate: () => true }) - .overrideInterceptor(CurrentUserInterceptor) - .useValue({ - intercept: (_ctx: ExecutionContext, next: CallHandler) => next.handle(), - }) - .compile(); + }); + const module: TestingModule = await overrideAuditInterceptors( + builder + .overrideGuard(AuthGuard('jwt')) + .useValue({ canActivate: () => true }) + .overrideGuard(RolesGuard) + .useValue({ canActivate: () => true }) + .overrideInterceptor(CurrentUserInterceptor) + .useValue({ + intercept: (_ctx: ExecutionContext, next: CallHandler) => + next.handle(), + }), + ).compile(); controller = module.get(QuestionnaireController); questionnaireService = module.get(QuestionnaireService); @@ -188,7 +196,7 @@ describe('QuestionnaireController - IngestCsv', () => { }) as Express.Multer.File; beforeEach(async () => { - const module: TestingModule = await Test.createTestingModule({ + const builder = Test.createTestingModule({ controllers: [QuestionnaireController], providers: [ { @@ -221,17 +229,21 @@ describe('QuestionnaireController - IngestCsv', () => { provide: CSVAdapter, useValue: new CSVAdapter(), }, + ...auditTestProviders(), ], - }) - .overrideGuard(AuthGuard('jwt')) - .useValue({ canActivate: () => true }) - .overrideGuard(RolesGuard) - .useValue({ canActivate: () => true }) - .overrideInterceptor(CurrentUserInterceptor) - .useValue({ - intercept: (_ctx: ExecutionContext, next: CallHandler) => next.handle(), - }) - .compile(); + }); + const module: TestingModule = await overrideAuditInterceptors( + builder + .overrideGuard(AuthGuard('jwt')) + .useValue({ canActivate: () => true }) + .overrideGuard(RolesGuard) + .useValue({ canActivate: () => true }) + .overrideInterceptor(CurrentUserInterceptor) + .useValue({ + intercept: (_ctx: ExecutionContext, next: CallHandler) => + next.handle(), + }), + ).compile(); controller = module.get(QuestionnaireController); questionnaireService = module.get(QuestionnaireService); @@ -437,7 +449,7 @@ describe('QuestionnaireController - wipeSubmissions', () => { const VERSION_ID = '550e8400-e29b-41d4-a716-446655440000'; beforeEach(async () => { - const module: TestingModule = await Test.createTestingModule({ + const builder = Test.createTestingModule({ controllers: [QuestionnaireController], providers: [ { @@ -469,17 +481,21 @@ describe('QuestionnaireController - wipeSubmissions', () => { provide: CSVAdapter, useValue: new CSVAdapter(), }, + ...auditTestProviders(), ], - }) - .overrideGuard(AuthGuard('jwt')) - .useValue({ canActivate: () => true }) - .overrideGuard(RolesGuard) - .useValue({ canActivate: () => true }) - .overrideInterceptor(CurrentUserInterceptor) - .useValue({ - intercept: (_ctx: ExecutionContext, next: CallHandler) => next.handle(), - }) - .compile(); + }); + const module: TestingModule = await overrideAuditInterceptors( + builder + .overrideGuard(AuthGuard('jwt')) + .useValue({ canActivate: () => true }) + .overrideGuard(RolesGuard) + .useValue({ canActivate: () => true }) + .overrideInterceptor(CurrentUserInterceptor) + .useValue({ + intercept: (_ctx: ExecutionContext, next: CallHandler) => + next.handle(), + }), + ).compile(); controller = module.get(QuestionnaireController); questionnaireService = module.get(QuestionnaireService); @@ -567,7 +583,7 @@ describe('QuestionnaireController - GetCsvTemplate', () => { }; beforeEach(async () => { - const module: TestingModule = await Test.createTestingModule({ + const builder = Test.createTestingModule({ controllers: [QuestionnaireController], providers: [ { @@ -598,17 +614,21 @@ describe('QuestionnaireController - GetCsvTemplate', () => { provide: CSVAdapter, useValue: new CSVAdapter(), }, + ...auditTestProviders(), ], - }) - .overrideGuard(AuthGuard('jwt')) - .useValue({ canActivate: () => true }) - .overrideGuard(RolesGuard) - .useValue({ canActivate: () => true }) - .overrideInterceptor(CurrentUserInterceptor) - .useValue({ - intercept: (_ctx: ExecutionContext, next: CallHandler) => next.handle(), - }) - .compile(); + }); + const module: TestingModule = await overrideAuditInterceptors( + builder + .overrideGuard(AuthGuard('jwt')) + .useValue({ canActivate: () => true }) + .overrideGuard(RolesGuard) + .useValue({ canActivate: () => true }) + .overrideInterceptor(CurrentUserInterceptor) + .useValue({ + intercept: (_ctx: ExecutionContext, next: CallHandler) => + next.handle(), + }), + ).compile(); controller = module.get(QuestionnaireController); questionnaireService = module.get(QuestionnaireService); @@ -741,7 +761,7 @@ describe('QuestionnaireController - mutation DTO mapping', () => { }; beforeEach(async () => { - const module: TestingModule = await Test.createTestingModule({ + const builder = Test.createTestingModule({ controllers: [QuestionnaireController], providers: [ { @@ -776,17 +796,21 @@ describe('QuestionnaireController - mutation DTO mapping', () => { provide: CSVAdapter, useValue: new CSVAdapter(), }, + ...auditTestProviders(), ], - }) - .overrideGuard(AuthGuard('jwt')) - .useValue({ canActivate: () => true }) - .overrideGuard(RolesGuard) - .useValue({ canActivate: () => true }) - .overrideInterceptor(CurrentUserInterceptor) - .useValue({ - intercept: (_ctx: ExecutionContext, next: CallHandler) => next.handle(), - }) - .compile(); + }); + const module: TestingModule = await overrideAuditInterceptors( + builder + .overrideGuard(AuthGuard('jwt')) + .useValue({ canActivate: () => true }) + .overrideGuard(RolesGuard) + .useValue({ canActivate: () => true }) + .overrideInterceptor(CurrentUserInterceptor) + .useValue({ + intercept: (_ctx: ExecutionContext, next: CallHandler) => + next.handle(), + }), + ).compile(); controller = module.get(QuestionnaireController); questionnaireService = module.get(QuestionnaireService); diff --git a/src/modules/questionnaires/questionnaire.controller.ts b/src/modules/questionnaires/questionnaire.controller.ts index 94830c0..84ee63b 100644 --- a/src/modules/questionnaires/questionnaire.controller.ts +++ b/src/modules/questionnaires/questionnaire.controller.ts @@ -44,6 +44,10 @@ import { IngestionResultDto } from './ingestion/dto/ingestion-result.dto'; import { UseJwtGuard } from 'src/security/decorators'; import { UserRole } from '../auth/roles.enum'; import { CurrentUserInterceptor } from '../common/interceptors/current-user.interceptor'; +import { MetaDataInterceptor } from '../common/interceptors/metadata.interceptor'; +import { AuditInterceptor } from '../audit/interceptors/audit.interceptor'; +import { Audited } from '../audit/decorators/audited.decorator'; +import { AuditAction } from '../audit/audit-action.enum'; import { IngestionEngine } from './ingestion/services/ingestion-engine.service'; import { CSVAdapter } from './ingestion/adapters/csv.adapter'; import { CSVAdapterConfig } from './ingestion/types/csv-adapter-config.type'; @@ -273,6 +277,15 @@ export class QuestionnaireController { @Post('submissions') @UseJwtGuard() + @Audited({ + action: AuditAction.QUESTIONNAIRE_SUBMIT, + resource: 'QuestionnaireSubmission', + }) + @UseInterceptors( + MetaDataInterceptor, + CurrentUserInterceptor, + AuditInterceptor, + ) @ApiOperation({ summary: 'Submit a completed questionnaire' }) async submitQuestionnaire( @Body() data: SubmitQuestionnaireRequest, @@ -348,11 +361,17 @@ export class QuestionnaireController { UserRole.DEAN, UserRole.CHAIRPERSON, ) + @Audited({ + action: AuditAction.QUESTIONNAIRE_INGEST, + resource: 'QuestionnaireSubmission', + }) @UseInterceptors( + MetaDataInterceptor, FileInterceptor('file', { fileFilter: csvFileFilter, limits: { fileSize: 5 * 1024 * 1024 }, }), + AuditInterceptor, ) @ApiOperation({ summary: 'Ingest questionnaire submissions from CSV' }) @ApiConsumes('multipart/form-data') @@ -427,6 +446,11 @@ export class QuestionnaireController { @Delete('versions/:versionId/submissions') @UseJwtGuard(UserRole.SUPER_ADMIN) + @Audited({ + action: AuditAction.QUESTIONNAIRE_SUBMISSIONS_WIPE, + resource: 'QuestionnaireVersion', + }) + @UseInterceptors(MetaDataInterceptor, AuditInterceptor) @ApiOperation({ summary: 'Wipe all submissions for a version' }) @ApiResponse({ status: 200,