diff --git a/.claude/skills/audit/SKILL.md b/.claude/skills/audit/SKILL.md new file mode 100644 index 0000000..a1c30d7 --- /dev/null +++ b/.claude/skills/audit/SKILL.md @@ -0,0 +1,229 @@ +--- +name: audit +description: > + Run a comprehensive code quality audit and automatically create GitHub issues for every finding. + Produces one .md file per issue, creates the GitHub issues with labels, adds them to the project + board, then writes an audit-summary.md. Trigger when user asks for a code audit, quality review, + tech debt scan, security review, or anti-pattern analysis. Accepts an optional path argument to + scope the audit (e.g. /audit src/modules/auth). Without args, audits the full src/ directory. +allowed-tools: Read, Grep, Glob, Bash, Write, Agent +--- + +# Code Quality Audit + +> **Recommended settings:** Use **Opus** model with **extended thinking** enabled. +> Audits require multi-file reasoning and catching subtle patterns — Sonnet will miss more. +> For scoped single-module runs (`/audit src/modules/auth`), Sonnet is acceptable. + +You are a senior software engineer performing a comprehensive code quality audit. +Your job is to find real technical debt, risk, and maintainability problems — then +file them as GitHub issues in one automated pass. + +## Scope + +The path to audit is: `$ARGUMENTS` (defaults to `src/` if not provided) + +--- + +## PHASE 1 — Setup + +1. Read `CLAUDE.md` (root and any subproject CLAUDE.md) for codebase context, intentional patterns, and known trade-offs. Do NOT flag patterns documented as intentional. + +2. Ensure the required labels exist in the repo. Run: + + ```bash + gh label list --repo CtrlAltElite-Devs/api.faculytics --json name --jq '.[].name' + ``` + + For each of the following that is missing, create it: + + ```bash + gh label create "tech-debt" --color "e4c217" --repo CtrlAltElite-Devs/api.faculytics + gh label create "performance" --color "e08b17" --repo CtrlAltElite-Devs/api.faculytics + gh label create "security" --color "b60205" --repo CtrlAltElite-Devs/api.faculytics + gh label create "refactor" --color "0075ca" --repo CtrlAltElite-Devs/api.faculytics + ``` + +3. Create the `audit/` working directory if it doesn't exist. + +--- + +## PHASE 2 — Audit + +Use an `Explore` subagent (or explore directly) to examine the scoped code for the four categories below. Pass the full CLAUDE.md context to the agent so it can avoid flagging intentional patterns. + +### 1. Code Smells + +- Duplicated / copy-pasted logic (DRY violations) +- Long methods or functions (doing too much) +- Large classes / god objects +- Deep nesting (arrow code) +- Magic numbers and magic strings +- Dead code (unreachable, unused exports, stale flags) +- Feature envy (a module heavily using another module's internals) +- Inappropriate intimacy between modules +- Inconsistent abstraction levels within the same file or function +- Missing or misleading comments that contradict the code + +### 2. Anti-Patterns + +- God objects / god modules +- Premature optimization +- Cargo cult code (code with no clear purpose) +- Spaghetti control flow +- Overuse of inheritance where composition fits better +- Shotgun surgery risk (one change requires edits across many unrelated files) +- Divergent change risk (one class changed for many different reasons) +- Primitive obsession (using raw primitives instead of domain types) +- Anemic domain model (objects with no behavior, just getters/setters) +- Speculative generality (over-engineering for hypothetical future needs) + +### 3. Performance Issues + +- N+1 query patterns +- Missing indexes on frequent query paths (flag the query, not the schema) +- Unbounded loops over large datasets +- Synchronous blocking operations that should be async +- Missing pagination on list endpoints +- Unnecessary data fetching (over-fetching, missing field selection) +- Cache invalidation problems or missing caching on hot paths +- Memory leaks (event listeners not cleaned up, closures holding references) + +### 4. Security Issues + +- Unsanitized user input used in queries, commands, or file paths +- Hardcoded secrets, credentials, or API keys +- Overly permissive CORS or access control configurations +- Missing authentication/authorization checks on sensitive endpoints +- Insecure direct object references (IDOR) +- Sensitive data logged to stdout/files +- Missing rate limiting on public-facing endpoints + +--- + +## PHASE 3 — Write Issue Files + +For each finding, write a file to `audit/issue-NNN-short-slug.md` (zero-padded, e.g. `issue-001-...`). + +Each file must follow this exact template: + +````md +--- +title: '' +severity: Critical | Major | Minor +category: Code Smell | Anti-Pattern | Performance | Security +labels: [] +--- + +## Summary + +One or two sentences describing the problem and why it matters. + +## Location + +- **File(s):** `path/to/file.ts` (lines X–Y if applicable) +- **Function/Class:** `functionName` / `ClassName` + +## Problem + +Explain the specific issue in detail. Include the problematic code snippet. + +```language +// relevant code here +``` +```` + +## Impact + +- What breaks or degrades if left unaddressed? +- Who is affected (users, developers, infrastructure)? +- Is this a latent bug, performance risk, security exposure, or maintainability burden? + +## Suggested Fix + +Describe the recommended approach. Include a corrected code snippet if possible. + +```language +// suggested fix here +``` + +## Acceptance Criteria + +- [ ] Specific, testable condition 1 +- [ ] Specific, testable condition 2 +- [ ] Existing tests still pass / new tests added for the fix + +```` + +**Label mapping** (use in frontmatter `labels` field): + +| Category | Labels | +|----------|--------| +| Security | `security`, `bug` | +| Performance | `performance`, `enhancement` | +| Code Smell | `tech-debt`, `refactor` | +| Anti-Pattern | `tech-debt`, `refactor` | + +--- + +## PHASE 4 — Create GitHub Issues + +For each `.md` file written to `audit/`, create a GitHub issue: + +```bash +ISSUE_URL=$(gh issue create \ + --repo CtrlAltElite-Devs/api.faculytics \ + --title "" \ + --body "$(cat audit/issue-NNN-short-slug.md)" \ + --label "<comma-separated labels from frontmatter>") +```` + +Capture the returned issue URL. Then add the issue to the project board under Backlog: + +```bash +gh project item-add 1 \ + --owner CtrlAltElite-Devs \ + --url "$ISSUE_URL" +``` + +> **Note:** `1` is the project **number** (not the GraphQL node ID). The project number +> is visible in the project URL or via `gh project list --owner CtrlAltElite-Devs`. + +--- + +## PHASE 5 — Summary + +Write `audit/audit-summary.md`: + +```md +# Audit Summary + +Scope: `<path audited>` +Date: <today's date> + +| # | Title | Severity | Category | File | Issue | +| --- | ----- | -------- | -------- | ------- | ------------ | +| 001 | ... | Critical | Security | src/... | <GitHub URL> | +``` + +Then print the summary table to the conversation so the user sees results at a glance. + +--- + +## SEVERITY GUIDE + +| Severity | Meaning | +| ------------ | -------------------------------------------------------------------------------------------------------- | +| **Critical** | Exploitable security vulnerability, data loss risk, or production-breaking bug. Fix before next release. | +| **Major** | Significant tech debt degrading performance, correctness, or developer velocity. Next sprint. | +| **Minor** | Low-risk code smell reducing readability or long-term maintainability. Backlog candidate. | + +--- + +## RULES + +- Do NOT flag style preferences (formatting, naming) unless they introduce real ambiguity or risk. +- Do NOT create duplicate issues for the same root cause — one issue, all locations listed. +- Do NOT flag third-party library internals, only the calling code. +- Every issue must reference real file paths and real code from this codebase. +- If no issues are found in a category, skip it silently. diff --git a/.gitignore b/.gitignore index 9b00a41..c097f02 100644 --- a/.gitignore +++ b/.gitignore @@ -48,6 +48,9 @@ openapi.yaml .temp .tmp +# Audit output (ephemeral — real output is GitHub issues) +/audit/ + # Runtime data pids *.pid 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<string, unknown>` — `@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<AuditJobMessage>)`: 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<string, unknown>; + 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<string, unknown>; browserName?: string; os?: string; ipAddress?: string }): Promise<void>` + - 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 { + <<Global>> + +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<string, unknown>; + + @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<void> { + 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<void> { + 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<Record<string, unknown>> = {}, @@ -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>(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>(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<AuditJobMessage>; + + 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<AuditJobMessage>); + await processor.process({ data: jobData } as Job<AuditJobMessage>); + + 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<AuditJobMessage>); + + 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<AuditJobMessage>, + 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<AuditJobMessage>): Promise<void> { + 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<AuditJobMessage>, 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>(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<string, unknown>, + Record<string, unknown>, + ]; + 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<string, unknown>, + ]; + 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<void> { + 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<string, unknown>; + 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<string, unknown>; + 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<Reflector>; + let auditService: { Emit: jest.Mock }; + let currentUserService: { get: jest.Mock }; + let requestMetadataService: { get: jest.Mock }; + + const mockHandler = (): jest.Mocked<CallHandler> => ({ + handle: jest.fn().mockReturnValue(of({ success: true })), + }); + + const mockContext = ( + params: Record<string, string> = {}, + query: Record<string, unknown> = {}, + user?: { userId: string }, + ): jest.Mocked<ExecutionContext> => + ({ + 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<ExecutionContext>; + + beforeEach(() => { + reflector = { get: jest.fn() } as unknown as jest.Mocked<Reflector>; + 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<CallHandler> = { + 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<AuditedOptions | undefined>( + 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<string, string> = + (request.params as Record<string, string>) ?? {}; + const query: Record<string, unknown> = + (request.query as Record<string, unknown>) ?? {}; + const rawMetadata: Record<string, unknown> = { + ...params, + ...query, + }; + + const resourceId = + Object.values(params).find((v) => UUID_V4_REGEX.test(v)) ?? + undefined; + + let metadata: Record<string, unknown> | 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 acf7777..8e49ab7 100644 --- a/src/modules/auth/auth.service.spec.ts +++ b/src/modules/auth/auth.service.spec.ts @@ -7,8 +7,14 @@ import * as bcrypt from 'bcrypt'; import { UnauthorizedException } from '@nestjs/common'; import { LOGIN_STRATEGIES, LoginStrategy } from './strategies'; 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'; const mockMetadata = { browserName: 'test', @@ -28,6 +34,7 @@ describe('AuthService', () => { let unitOfWork: UnitOfWork; let mockLocalStrategy: jest.Mocked<LoginStrategy>; let mockMoodleStrategy: jest.Mocked<LoginStrategy>; + let mockAuditService: { Emit: jest.Mock }; beforeEach(async () => { mockLocalStrategy = { @@ -42,6 +49,8 @@ describe('AuthService', () => { Execute: jest.fn(), }; + mockAuditService = { Emit: jest.fn().mockResolvedValue(undefined) }; + const module: TestingModule = await Test.createTestingModule({ providers: [ AuthService, @@ -86,6 +95,10 @@ describe('AuthService', () => { provide: RequestMetadataService, useValue: mockRequestMetadataService, }, + { + provide: AuditService, + useValue: mockAuditService, + }, ], }).compile(); @@ -292,5 +305,509 @@ 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<string, unknown>, + }), + ); + }); + + 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<string, unknown>, + }), + ); + }); + + 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<string, unknown>, + }), + ); + }); + }); + + describe('RefreshToken', () => { + const userId = 'user-id'; + const rawRefreshToken = 'raw-refresh-token'; + + function createMockToken( + overrides: Partial<RefreshToken> = {}, + ): RefreshToken { + const token = new RefreshToken(); + token.id = overrides.id ?? 'token-id'; + token.tokenHash = overrides.tokenHash ?? 'hashed-token'; + token.userId = overrides.userId ?? userId; + token.expiresAt = + overrides.expiresAt ?? new Date(Date.now() + 7 * 24 * 60 * 60 * 1000); + token.isActive = overrides.isActive ?? true; + token.browserName = 'test'; + token.os = 'test'; + token.ipAddress = '127.0.0.1'; + return token; + } + + it('should successfully refresh with a valid token', async () => { + const hashedToken = await bcrypt.hash(rawRefreshToken, 10); + const storedToken = createMockToken({ tokenHash: hashedToken }); + + const mockUser = new User(); + mockUser.id = userId; + 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', + }); + + const result = await service.RefreshToken(userId, rawRefreshToken); + + expect(result.token).toBe('new-access'); + expect(storedToken.isActive).toBe(false); + expect(storedToken.revokedAt).toBeDefined(); + expect(mockFind).toHaveBeenCalledWith( + expect.objectContaining({ + userId, + isActive: true, + // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment + expiresAt: { $gt: expect.any(Date) }, + }), + ); + }); + + it('should throw UnauthorizedException when no tokens match', async () => { + const storedToken = createMockToken({ + tokenHash: await bcrypt.hash('different-token', 10), + }); + + const mockEm = { + getRepository: jest.fn().mockReturnValue({ + find: jest.fn().mockResolvedValue([storedToken]), + }), + findOneOrFail: jest.fn(), + }; + + (unitOfWork.runInTransaction as jest.Mock).mockImplementation( + (cb: (em: EntityManager) => unknown) => + cb(mockEm as unknown as EntityManager), + ); + + await expect( + service.RefreshToken(userId, rawRefreshToken), + ).rejects.toThrow(UnauthorizedException); + }); + + it('should throw UnauthorizedException when no active tokens exist', async () => { + const mockEm = { + getRepository: jest + .fn() + .mockReturnValue({ find: jest.fn().mockResolvedValue([]) }), + findOneOrFail: jest.fn(), + }; + + (unitOfWork.runInTransaction as jest.Mock).mockImplementation( + (cb: (em: EntityManager) => unknown) => + cb(mockEm as unknown as EntityManager), + ); + + await expect( + service.RefreshToken(userId, rawRefreshToken), + ).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 }); + + const mockUser = new User(); + mockUser.id = userId; + + 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: 'access', + refreshToken: 'refresh', + }); + + // RefreshToken should return a promise (async), not block synchronously. + // If compareSync were used, this would still resolve, but we verify + // the method is async by checking it returns a promise. + const result = service.RefreshToken(userId, rawRefreshToken); + expect(result).toBeInstanceOf(Promise); + await result; + }); + }); +}); + +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<LoginStrategy> = { + 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>(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>(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<LoginStrategy> = { + 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>(AuthService); + await expect( + svc.Login({ username: 'bad', password: 'bad' }), + ).rejects.toThrow(UnauthorizedException); + }); +}); + +describe('LoginRequest DTO validation', () => { + const toDto = (plain: Record<string, unknown>) => + plainToInstance(LoginRequest, plain); + + it('should pass with valid username and password', async () => { + const errors = await validate( + toDto({ username: 'admin', password: 'password123' }), + ); + expect(errors).toHaveLength(0); + }); + + it('should fail when username is empty', async () => { + const errors = await validate( + toDto({ username: '', password: 'password123' }), + ); + expect(errors.length).toBeGreaterThan(0); + expect(errors[0].property).toBe('username'); + }); + + it('should fail when password is empty', async () => { + const errors = await validate(toDto({ username: 'admin', password: '' })); + expect(errors.length).toBeGreaterThan(0); + expect(errors[0].property).toBe('password'); + }); + + it('should fail when username exceeds max length', async () => { + const errors = await validate( + toDto({ username: 'a'.repeat(101), password: 'password123' }), + ); + expect(errors.length).toBeGreaterThan(0); + expect(errors[0].property).toBe('username'); + }); + + it('should fail when password exceeds max length', async () => { + const errors = await validate( + toDto({ username: 'admin', password: 'a'.repeat(256) }), + ); + expect(errors.length).toBeGreaterThan(0); + expect(errors[0].property).toBe('password'); }); }); diff --git a/src/modules/auth/auth.service.ts b/src/modules/auth/auth.service.ts index 3bdd189..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,20 +131,28 @@ 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); const storedTokens = await refreshTokenRepository.find({ userId, isActive: true, + expiresAt: { $gt: new Date() }, }); - const matchingToken = storedTokens.find((token) => - bcrypt.compareSync(refreshToken, token.tokenHash), + const comparisons = await Promise.all( + storedTokens.map(async (token) => ({ + token, + isMatch: await bcrypt.compare(refreshToken, token.tokenHash), + })), ); + const matchingToken = comparisons.find((c) => c.isMatch)?.token; - if (!matchingToken || matchingToken.expiresAt < new Date()) { + if (!matchingToken) { throw new UnauthorizedException(); } @@ -114,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/auth/dto/requests/login.request.dto.ts b/src/modules/auth/dto/requests/login.request.dto.ts index a25fbf3..91bc338 100644 --- a/src/modules/auth/dto/requests/login.request.dto.ts +++ b/src/modules/auth/dto/requests/login.request.dto.ts @@ -1,9 +1,13 @@ -import { IsString } from 'class-validator'; +import { IsNotEmpty, IsString, MaxLength } from 'class-validator'; export class LoginRequest { @IsString() + @IsNotEmpty() + @MaxLength(100) username: string; @IsString() + @IsNotEmpty() + @MaxLength(255) password: string; } diff --git a/src/modules/auth/strategies/moodle-login.strategy.ts b/src/modules/auth/strategies/moodle-login.strategy.ts index db6beee..3820bdc 100644 --- a/src/modules/auth/strategies/moodle-login.strategy.ts +++ b/src/modules/auth/strategies/moodle-login.strategy.ts @@ -57,7 +57,7 @@ export class MoodleLoginStrategy implements LoginStrategy { } catch (error) { if (error instanceof MoodleConnectivityError) { this.logger.error( - `Moodle connectivity failure during login for user "${body.username}": ${error.message}`, + `Moodle connectivity failure during login: ${error.message}`, error.cause?.stack, ); throw new UnauthorizedException( diff --git a/src/modules/curriculum/curriculum.controller.ts b/src/modules/curriculum/curriculum.controller.ts index f9d1955..10c9989 100644 --- a/src/modules/curriculum/curriculum.controller.ts +++ b/src/modules/curriculum/curriculum.controller.ts @@ -7,9 +7,9 @@ import { CurriculumService } from './services/curriculum.service'; import { ListDepartmentsQueryDto } from './dto/requests/list-departments-query.dto'; import { ListProgramsQueryDto } from './dto/requests/list-programs-query.dto'; import { ListCoursesQueryDto } from './dto/requests/list-courses-query.dto'; -import { DepartmentItemResponseDto } from './dto/responses/department-item.response.dto'; -import { ProgramItemResponseDto } from './dto/responses/program-item.response.dto'; -import { CourseItemResponseDto } from './dto/responses/course-item.response.dto'; +import { DepartmentListResponseDto } from './dto/responses/department-list.response.dto'; +import { ProgramListResponseDto } from './dto/responses/program-list.response.dto'; +import { CourseListResponseDto } from './dto/responses/course-list.response.dto'; @ApiTags('Curriculum') @Controller('curriculum') @@ -20,28 +20,28 @@ export class CurriculumController { @Get('departments') @ApiOperation({ summary: 'List departments scoped to caller role' }) - @ApiResponse({ status: 200, type: [DepartmentItemResponseDto] }) + @ApiResponse({ status: 200, type: DepartmentListResponseDto }) async ListDepartments( @Query() query: ListDepartmentsQueryDto, - ): Promise<DepartmentItemResponseDto[]> { + ): Promise<DepartmentListResponseDto> { return this.curriculumService.ListDepartments(query); } @Get('programs') @ApiOperation({ summary: 'List programs scoped to caller role' }) - @ApiResponse({ status: 200, type: [ProgramItemResponseDto] }) + @ApiResponse({ status: 200, type: ProgramListResponseDto }) async ListPrograms( @Query() query: ListProgramsQueryDto, - ): Promise<ProgramItemResponseDto[]> { + ): Promise<ProgramListResponseDto> { return this.curriculumService.ListPrograms(query); } @Get('courses') @ApiOperation({ summary: 'List courses scoped to caller role' }) - @ApiResponse({ status: 200, type: [CourseItemResponseDto] }) + @ApiResponse({ status: 200, type: CourseListResponseDto }) async ListCourses( @Query() query: ListCoursesQueryDto, - ): Promise<CourseItemResponseDto[]> { + ): Promise<CourseListResponseDto> { return this.curriculumService.ListCourses(query); } } diff --git a/src/modules/curriculum/dto/requests/list-courses-query.dto.ts b/src/modules/curriculum/dto/requests/list-courses-query.dto.ts index 49a8049..8d92178 100644 --- a/src/modules/curriculum/dto/requests/list-courses-query.dto.ts +++ b/src/modules/curriculum/dto/requests/list-courses-query.dto.ts @@ -6,8 +6,9 @@ import { MaxLength, } from 'class-validator'; import { ApiProperty, ApiPropertyOptional } from '@nestjs/swagger'; +import { PaginationQueryDto } from 'src/modules/common/dto/pagination-query.dto'; -export class ListCoursesQueryDto { +export class ListCoursesQueryDto extends PaginationQueryDto { @ApiProperty({ description: 'Semester UUID to scope course list' }) @IsUUID() @IsNotEmpty() diff --git a/src/modules/curriculum/dto/requests/list-departments-query.dto.ts b/src/modules/curriculum/dto/requests/list-departments-query.dto.ts index 030d7c2..cfb9a3b 100644 --- a/src/modules/curriculum/dto/requests/list-departments-query.dto.ts +++ b/src/modules/curriculum/dto/requests/list-departments-query.dto.ts @@ -6,8 +6,9 @@ import { MaxLength, } from 'class-validator'; import { ApiProperty, ApiPropertyOptional } from '@nestjs/swagger'; +import { PaginationQueryDto } from 'src/modules/common/dto/pagination-query.dto'; -export class ListDepartmentsQueryDto { +export class ListDepartmentsQueryDto extends PaginationQueryDto { @ApiProperty({ description: 'Semester UUID to scope department list' }) @IsUUID() @IsNotEmpty() diff --git a/src/modules/curriculum/dto/requests/list-programs-query.dto.ts b/src/modules/curriculum/dto/requests/list-programs-query.dto.ts index 3d0f95b..487d07d 100644 --- a/src/modules/curriculum/dto/requests/list-programs-query.dto.ts +++ b/src/modules/curriculum/dto/requests/list-programs-query.dto.ts @@ -6,8 +6,9 @@ import { MaxLength, } from 'class-validator'; import { ApiProperty, ApiPropertyOptional } from '@nestjs/swagger'; +import { PaginationQueryDto } from 'src/modules/common/dto/pagination-query.dto'; -export class ListProgramsQueryDto { +export class ListProgramsQueryDto extends PaginationQueryDto { @ApiProperty({ description: 'Semester UUID to scope program list' }) @IsUUID() @IsNotEmpty() diff --git a/src/modules/curriculum/dto/responses/course-list.response.dto.ts b/src/modules/curriculum/dto/responses/course-list.response.dto.ts new file mode 100644 index 0000000..50874d4 --- /dev/null +++ b/src/modules/curriculum/dto/responses/course-list.response.dto.ts @@ -0,0 +1,11 @@ +import { ApiProperty } from '@nestjs/swagger'; +import { PaginationMeta } from 'src/modules/common/dto/pagination.dto'; +import { CourseItemResponseDto } from './course-item.response.dto'; + +export class CourseListResponseDto { + @ApiProperty({ type: [CourseItemResponseDto] }) + data: CourseItemResponseDto[]; + + @ApiProperty({ type: PaginationMeta }) + meta: PaginationMeta; +} diff --git a/src/modules/curriculum/dto/responses/department-list.response.dto.ts b/src/modules/curriculum/dto/responses/department-list.response.dto.ts new file mode 100644 index 0000000..2fdf206 --- /dev/null +++ b/src/modules/curriculum/dto/responses/department-list.response.dto.ts @@ -0,0 +1,11 @@ +import { ApiProperty } from '@nestjs/swagger'; +import { PaginationMeta } from 'src/modules/common/dto/pagination.dto'; +import { DepartmentItemResponseDto } from './department-item.response.dto'; + +export class DepartmentListResponseDto { + @ApiProperty({ type: [DepartmentItemResponseDto] }) + data: DepartmentItemResponseDto[]; + + @ApiProperty({ type: PaginationMeta }) + meta: PaginationMeta; +} diff --git a/src/modules/curriculum/dto/responses/program-list.response.dto.ts b/src/modules/curriculum/dto/responses/program-list.response.dto.ts new file mode 100644 index 0000000..e2878c1 --- /dev/null +++ b/src/modules/curriculum/dto/responses/program-list.response.dto.ts @@ -0,0 +1,11 @@ +import { ApiProperty } from '@nestjs/swagger'; +import { PaginationMeta } from 'src/modules/common/dto/pagination.dto'; +import { ProgramItemResponseDto } from './program-item.response.dto'; + +export class ProgramListResponseDto { + @ApiProperty({ type: [ProgramItemResponseDto] }) + data: ProgramItemResponseDto[]; + + @ApiProperty({ type: PaginationMeta }) + meta: PaginationMeta; +} diff --git a/src/modules/curriculum/services/curriculum.service.spec.ts b/src/modules/curriculum/services/curriculum.service.spec.ts index 68b4e78..65fe3c5 100644 --- a/src/modules/curriculum/services/curriculum.service.spec.ts +++ b/src/modules/curriculum/services/curriculum.service.spec.ts @@ -10,7 +10,7 @@ import { ScopeResolverService } from 'src/modules/common/services/scope-resolver describe('CurriculumService', () => { let service: CurriculumService; - let em: { findOne: jest.Mock; find: jest.Mock }; + let em: { findOne: jest.Mock; findAndCount: jest.Mock }; let scopeResolver: { ResolveDepartmentIds: jest.Mock }; const semesterId = 'semester-1'; @@ -22,7 +22,7 @@ describe('CurriculumService', () => { beforeEach(async () => { em = { findOne: jest.fn(), - find: jest.fn(), + findAndCount: jest.fn(), }; scopeResolver = { @@ -44,6 +44,14 @@ describe('CurriculumService', () => { em.findOne.mockResolvedValueOnce({ id: semesterId }); } + const emptyMeta = (page = 1, limit = 10) => ({ + totalItems: 0, + itemCount: 0, + itemsPerPage: limit, + totalPages: 0, + currentPage: page, + }); + // ─── ListDepartments ────────────────────────────────────────────── describe('ListDepartments', () => { @@ -55,14 +63,16 @@ describe('CurriculumService', () => { { id: deptId, code: 'CCS', name: 'College of Computer Studies' }, { id: deptId2, code: 'CBA', name: 'College of Business Admin' }, ]; - em.find.mockResolvedValue(departments); + em.findAndCount.mockResolvedValue([departments, 2]); const result = await service.ListDepartments({ semesterId }); - expect(result).toHaveLength(2); - expect(result[0].id).toBe(deptId); - expect(result[0].code).toBe('CCS'); - expect(result[0].name).toBe('College of Computer Studies'); + expect(result.data).toHaveLength(2); + expect(result.data[0].id).toBe(deptId); + expect(result.data[0].code).toBe('CCS'); + expect(result.data[0].name).toBe('College of Computer Studies'); + expect(result.meta.totalItems).toBe(2); + expect(result.meta.currentPage).toBe(1); expect(scopeResolver.ResolveDepartmentIds).toHaveBeenCalledWith( semesterId, ); @@ -75,37 +85,38 @@ describe('CurriculumService', () => { const departments = [ { id: deptId, code: 'CCS', name: 'College of Computer Studies' }, ]; - em.find.mockResolvedValue(departments); + em.findAndCount.mockResolvedValue([departments, 1]); const result = await service.ListDepartments({ semesterId }); - expect(result).toHaveLength(1); - expect(result[0].code).toBe('CCS'); + expect(result.data).toHaveLength(1); + expect(result.data[0].code).toBe('CCS'); // Verify scope filter was applied - const findCall = em.find.mock.calls[0] as unknown[]; + const findCall = em.findAndCount.mock.calls[0] as unknown[]; expect(findCall[1]).toEqual( expect.objectContaining({ id: { $in: [deptId] } }), ); }); - it('should return [] when dean has empty scope', async () => { + it('should return empty page when dean has empty scope', async () => { setupSemesterFound(); scopeResolver.ResolveDepartmentIds.mockResolvedValue([]); const result = await service.ListDepartments({ semesterId }); - expect(result).toEqual([]); - expect(em.find).not.toHaveBeenCalled(); + expect(result.data).toEqual([]); + expect(result.meta).toEqual(emptyMeta()); + expect(em.findAndCount).not.toHaveBeenCalled(); }); it('should filter by search on code and name (OR, ILIKE)', async () => { setupSemesterFound(); scopeResolver.ResolveDepartmentIds.mockResolvedValue(null); - em.find.mockResolvedValue([]); + em.findAndCount.mockResolvedValue([[], 0]); await service.ListDepartments({ semesterId, search: 'Comp' }); - const findCall = em.find.mock.calls[0] as unknown[]; + const findCall = em.findAndCount.mock.calls[0] as unknown[]; expect(findCall[1]).toEqual( expect.objectContaining({ $and: [ @@ -123,11 +134,11 @@ describe('CurriculumService', () => { it('should escape LIKE wildcards in search', async () => { setupSemesterFound(); scopeResolver.ResolveDepartmentIds.mockResolvedValue(null); - em.find.mockResolvedValue([]); + em.findAndCount.mockResolvedValue([[], 0]); await service.ListDepartments({ semesterId, search: '%admin_test' }); - const findCall = em.find.mock.calls[0] as unknown[]; + const findCall = em.findAndCount.mock.calls[0] as unknown[]; expect(findCall[1]).toEqual( expect.objectContaining({ $and: [ @@ -150,24 +161,25 @@ describe('CurriculumService', () => { ); }); - it('should return [] when no departments match', async () => { + it('should return empty page when no departments match', async () => { setupSemesterFound(); scopeResolver.ResolveDepartmentIds.mockResolvedValue(null); - em.find.mockResolvedValue([]); + em.findAndCount.mockResolvedValue([[], 0]); const result = await service.ListDepartments({ semesterId }); - expect(result).toEqual([]); + expect(result.data).toEqual([]); + expect(result.meta.totalItems).toBe(0); }); it('should apply both scope restriction and search simultaneously', async () => { setupSemesterFound(); scopeResolver.ResolveDepartmentIds.mockResolvedValue([deptId]); - em.find.mockResolvedValue([]); + em.findAndCount.mockResolvedValue([[], 0]); await service.ListDepartments({ semesterId, search: 'CCS' }); - const findCall = em.find.mock.calls[0] as unknown[]; + const findCall = em.findAndCount.mock.calls[0] as unknown[]; expect(findCall[1]).toEqual( expect.objectContaining({ id: { $in: [deptId] }, @@ -186,11 +198,64 @@ describe('CurriculumService', () => { it('should handle department with null name', async () => { setupSemesterFound(); scopeResolver.ResolveDepartmentIds.mockResolvedValue(null); - em.find.mockResolvedValue([{ id: deptId, code: 'CCS', name: undefined }]); + em.findAndCount.mockResolvedValue([ + [{ id: deptId, code: 'CCS', name: undefined }], + 1, + ]); const result = await service.ListDepartments({ semesterId }); - expect(result[0].name).toBeNull(); + expect(result.data[0].name).toBeNull(); + }); + + it('should pass limit and offset to findAndCount with default pagination', async () => { + setupSemesterFound(); + scopeResolver.ResolveDepartmentIds.mockResolvedValue(null); + em.findAndCount.mockResolvedValue([[], 0]); + + await service.ListDepartments({ semesterId }); + + const findCall = em.findAndCount.mock.calls[0] as unknown[]; + expect(findCall[2]).toEqual( + expect.objectContaining({ limit: 10, offset: 0 }), + ); + }); + + it('should pass custom page and limit to findAndCount', async () => { + setupSemesterFound(); + scopeResolver.ResolveDepartmentIds.mockResolvedValue(null); + em.findAndCount.mockResolvedValue([[], 0]); + + await service.ListDepartments({ semesterId, page: 3, limit: 5 }); + + const findCall = em.findAndCount.mock.calls[0] as unknown[]; + expect(findCall[2]).toEqual( + expect.objectContaining({ limit: 5, offset: 10 }), + ); + }); + + it('should compute pagination meta correctly', async () => { + setupSemesterFound(); + scopeResolver.ResolveDepartmentIds.mockResolvedValue(null); + + const departments = [ + { id: deptId, code: 'CCS', name: 'College of Computer Studies' }, + ]; + em.findAndCount.mockResolvedValue([departments, 25]); + + const result = await service.ListDepartments({ + semesterId, + page: 2, + limit: 10, + }); + + expect(result.meta).toEqual({ + totalItems: 25, + itemCount: 1, + itemsPerPage: 10, + totalPages: 3, + currentPage: 2, + }); }); }); @@ -215,27 +280,28 @@ describe('CurriculumService', () => { department: { id: deptId }, }, ]; - em.find.mockResolvedValue(programs); + em.findAndCount.mockResolvedValue([programs, 2]); const result = await service.ListPrograms({ semesterId }); - expect(result).toHaveLength(2); - expect(result[0].departmentId).toBe(deptId); + expect(result.data).toHaveLength(2); + expect(result.data[0].departmentId).toBe(deptId); + expect(result.meta.totalItems).toBe(2); }); - it('should return [] for super admin with non-existent departmentId', async () => { + it('should return empty page for super admin with non-existent departmentId', async () => { setupSemesterFound(); scopeResolver.ResolveDepartmentIds.mockResolvedValue(null); - em.find.mockResolvedValue([]); + em.findAndCount.mockResolvedValue([[], 0]); const result = await service.ListPrograms({ semesterId, departmentId: 'non-existent', }); - expect(result).toEqual([]); + expect(result.data).toEqual([]); // Verify filter includes the departmentId - const findCall = em.find.mock.calls[0] as unknown[]; + const findCall = em.findAndCount.mock.calls[0] as unknown[]; expect(findCall[1]).toEqual( expect.objectContaining({ // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment @@ -256,21 +322,21 @@ describe('CurriculumService', () => { department: { id: deptId }, }, ]; - em.find.mockResolvedValue(programs); + em.findAndCount.mockResolvedValue([programs, 1]); const result = await service.ListPrograms({ semesterId }); - expect(result).toHaveLength(1); + expect(result.data).toHaveLength(1); }); it('should narrow results with departmentId within scope', async () => { setupSemesterFound(); scopeResolver.ResolveDepartmentIds.mockResolvedValue([deptId, deptId2]); - em.find.mockResolvedValue([]); + em.findAndCount.mockResolvedValue([[], 0]); await service.ListPrograms({ semesterId, departmentId: deptId }); - const findCall = em.find.mock.calls[0] as unknown[]; + const findCall = em.findAndCount.mock.calls[0] as unknown[]; expect(findCall[1]).toEqual( expect.objectContaining({ // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment @@ -291,11 +357,11 @@ describe('CurriculumService', () => { it('should filter by search on code and name (OR)', async () => { setupSemesterFound(); scopeResolver.ResolveDepartmentIds.mockResolvedValue(null); - em.find.mockResolvedValue([]); + em.findAndCount.mockResolvedValue([[], 0]); await service.ListPrograms({ semesterId, search: 'BS' }); - const findCall = em.find.mock.calls[0] as unknown[]; + const findCall = em.findAndCount.mock.calls[0] as unknown[]; expect(findCall[1]).toEqual( expect.objectContaining({ $and: [ @@ -307,24 +373,39 @@ describe('CurriculumService', () => { ); }); - it('should return [] when no programs match', async () => { + it('should return empty page when no programs match', async () => { setupSemesterFound(); scopeResolver.ResolveDepartmentIds.mockResolvedValue(null); - em.find.mockResolvedValue([]); + em.findAndCount.mockResolvedValue([[], 0]); const result = await service.ListPrograms({ semesterId }); - expect(result).toEqual([]); + expect(result.data).toEqual([]); + expect(result.meta.totalItems).toBe(0); }); - it('should return [] when dean has empty scope and no departmentId', async () => { + it('should return empty page when dean has empty scope and no departmentId', async () => { setupSemesterFound(); scopeResolver.ResolveDepartmentIds.mockResolvedValue([]); const result = await service.ListPrograms({ semesterId }); - expect(result).toEqual([]); - expect(em.find).not.toHaveBeenCalled(); + expect(result.data).toEqual([]); + expect(result.meta).toEqual(emptyMeta()); + expect(em.findAndCount).not.toHaveBeenCalled(); + }); + + it('should pass limit and offset with custom pagination', async () => { + setupSemesterFound(); + scopeResolver.ResolveDepartmentIds.mockResolvedValue(null); + em.findAndCount.mockResolvedValue([[], 0]); + + await service.ListPrograms({ semesterId, page: 2, limit: 15 }); + + const findCall = em.findAndCount.mock.calls[0] as unknown[]; + expect(findCall[2]).toEqual( + expect.objectContaining({ limit: 15, offset: 15 }), + ); }); }); @@ -359,15 +440,16 @@ describe('CurriculumService', () => { isActive: false, }, ]; - em.find.mockResolvedValue(courses); + em.findAndCount.mockResolvedValue([courses, 2]); const result = await service.ListCourses({ semesterId, departmentId: deptId, }); - expect(result).toHaveLength(2); - expect(result[0].programId).toBe(programId); + expect(result.data).toHaveLength(2); + expect(result.data[0].programId).toBe(programId); + expect(result.meta.totalItems).toBe(2); }); it('should return courses for dean with programId within scope', async () => { @@ -388,14 +470,14 @@ describe('CurriculumService', () => { isActive: true, }, ]; - em.find.mockResolvedValue(courses); + em.findAndCount.mockResolvedValue([courses, 1]); const result = await service.ListCourses({ semesterId, programId, }); - expect(result).toHaveLength(1); + expect(result.data).toHaveLength(1); }); it('should throw 403 when dean provides programId outside scope', async () => { @@ -444,7 +526,7 @@ describe('CurriculumService', () => { it('should filter by search on shortname and fullname (OR)', async () => { setupSemesterFound(); scopeResolver.ResolveDepartmentIds.mockResolvedValue(null); - em.find.mockResolvedValue([]); + em.findAndCount.mockResolvedValue([[], 0]); await service.ListCourses({ semesterId, @@ -452,7 +534,7 @@ describe('CurriculumService', () => { search: 'NET', }); - const findCall = em.find.mock.calls[0] as unknown[]; + const findCall = em.findAndCount.mock.calls[0] as unknown[]; expect(findCall[1]).toEqual( expect.objectContaining({ $and: [ @@ -487,19 +569,19 @@ describe('CurriculumService', () => { isActive: false, }, ]; - em.find.mockResolvedValue(courses); + em.findAndCount.mockResolvedValue([courses, 2]); const result = await service.ListCourses({ semesterId, departmentId: deptId, }); - expect(result).toHaveLength(2); - expect(result[0].isActive).toBe(true); - expect(result[1].isActive).toBe(false); + expect(result.data).toHaveLength(2); + expect(result.data[0].isActive).toBe(true); + expect(result.data[1].isActive).toBe(false); // Verify no isActive filter was applied - const findCall = em.find.mock.calls[0] as unknown[]; + const findCall = em.findAndCount.mock.calls[0] as unknown[]; expect(findCall[1]).not.toHaveProperty('isActive'); }); @@ -526,17 +608,18 @@ describe('CurriculumService', () => { ).rejects.toThrow(ForbiddenException); }); - it('should return [] when no courses match', async () => { + it('should return empty page when no courses match', async () => { setupSemesterFound(); scopeResolver.ResolveDepartmentIds.mockResolvedValue(null); - em.find.mockResolvedValue([]); + em.findAndCount.mockResolvedValue([[], 0]); const result = await service.ListCourses({ semesterId, departmentId: deptId, }); - expect(result).toEqual([]); + expect(result.data).toEqual([]); + expect(result.meta.totalItems).toBe(0); }); it('should throw 404 for non-existent semesterId', async () => { @@ -547,7 +630,7 @@ describe('CurriculumService', () => { ).rejects.toThrow(NotFoundException); }); - it('should return [] when dean has empty scope with departmentId', async () => { + it('should return empty page when dean has empty scope with departmentId', async () => { setupSemesterFound(); scopeResolver.ResolveDepartmentIds.mockResolvedValue([]); @@ -588,7 +671,7 @@ describe('CurriculumService', () => { isActive: true, }, ]; - em.find.mockResolvedValue(courses); + em.findAndCount.mockResolvedValue([courses, 1]); const result = await service.ListCourses({ semesterId, @@ -596,11 +679,11 @@ describe('CurriculumService', () => { programId, }); - expect(result).toHaveLength(1); - expect(result[0].shortname).toBe('FREAI'); + expect(result.data).toHaveLength(1); + expect(result.data[0].shortname).toBe('FREAI'); // Verify filter includes both constraints - const findCall = em.find.mock.calls[0] as unknown[]; + const findCall = em.findAndCount.mock.calls[0] as unknown[]; expect(findCall[1]).toEqual( expect.objectContaining({ // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment @@ -608,5 +691,54 @@ describe('CurriculumService', () => { }), ); }); + + it('should pass limit and offset with custom pagination', async () => { + setupSemesterFound(); + scopeResolver.ResolveDepartmentIds.mockResolvedValue(null); + em.findAndCount.mockResolvedValue([[], 0]); + + await service.ListCourses({ + semesterId, + departmentId: deptId, + page: 4, + limit: 25, + }); + + const findCall = em.findAndCount.mock.calls[0] as unknown[]; + expect(findCall[2]).toEqual( + expect.objectContaining({ limit: 25, offset: 75 }), + ); + }); + + it('should compute pagination meta correctly for courses', async () => { + setupSemesterFound(); + scopeResolver.ResolveDepartmentIds.mockResolvedValue(null); + + const courses = [ + { + id: 'c1', + shortname: 'FREAI', + fullname: 'Free Elective AI', + program: { id: programId }, + isActive: true, + }, + ]; + em.findAndCount.mockResolvedValue([courses, 50]); + + const result = await service.ListCourses({ + semesterId, + departmentId: deptId, + page: 3, + limit: 10, + }); + + expect(result.meta).toEqual({ + totalItems: 50, + itemCount: 1, + itemsPerPage: 10, + totalPages: 5, + currentPage: 3, + }); + }); }); }); diff --git a/src/modules/curriculum/services/curriculum.service.ts b/src/modules/curriculum/services/curriculum.service.ts index 527413a..b04562e 100644 --- a/src/modules/curriculum/services/curriculum.service.ts +++ b/src/modules/curriculum/services/curriculum.service.ts @@ -15,8 +15,11 @@ import { ListDepartmentsQueryDto } from '../dto/requests/list-departments-query. import { ListProgramsQueryDto } from '../dto/requests/list-programs-query.dto'; import { ListCoursesQueryDto } from '../dto/requests/list-courses-query.dto'; import { DepartmentItemResponseDto } from '../dto/responses/department-item.response.dto'; +import { DepartmentListResponseDto } from '../dto/responses/department-list.response.dto'; import { ProgramItemResponseDto } from '../dto/responses/program-item.response.dto'; +import { ProgramListResponseDto } from '../dto/responses/program-list.response.dto'; import { CourseItemResponseDto } from '../dto/responses/course-item.response.dto'; +import { CourseListResponseDto } from '../dto/responses/course-list.response.dto'; @Injectable() export class CurriculumService { @@ -27,9 +30,13 @@ export class CurriculumService { async ListDepartments( query: ListDepartmentsQueryDto, - ): Promise<DepartmentItemResponseDto[]> { + ): Promise<DepartmentListResponseDto> { await this.ValidateSemester(query.semesterId); + const page = query.page ?? 1; + const limit = query.limit ?? 10; + const offset = (page - 1) * limit; + const departmentIds = await this.scopeResolverService.ResolveDepartmentIds( query.semesterId, ); @@ -40,37 +47,44 @@ export class CurriculumService { if (departmentIds !== null) { if (departmentIds.length === 0) { - return []; + return this.BuildEmptyPage(page, limit); } Object.assign(filter, { id: { $in: departmentIds } }); } - if (query.search) { - const escaped = this.EscapeLikeWildcards(query.search); - Object.assign(filter, { - $and: [ - { - $or: [ - { code: { $ilike: `%${escaped}%` } }, - { name: { $ilike: `%${escaped}%` } }, - ], - }, - ], - }); - } + this.ApplySearchFilter(filter, query.search, ['code', 'name']); - const departments = await this.em.find(Department, filter, { - orderBy: { name: QueryOrder.ASC_NULLS_LAST }, - }); + const [departments, totalItems] = await this.em.findAndCount( + Department, + filter, + { + orderBy: { name: QueryOrder.ASC_NULLS_LAST }, + limit, + offset, + }, + ); - return departments.map((d) => DepartmentItemResponseDto.Map(d)); + return { + data: departments.map((d) => DepartmentItemResponseDto.Map(d)), + meta: { + totalItems, + itemCount: departments.length, + itemsPerPage: limit, + totalPages: Math.ceil(totalItems / limit), + currentPage: page, + }, + }; } async ListPrograms( query: ListProgramsQueryDto, - ): Promise<ProgramItemResponseDto[]> { + ): Promise<ProgramListResponseDto> { await this.ValidateSemester(query.semesterId); + const page = query.page ?? 1; + const limit = query.limit ?? 10; + const offset = (page - 1) * limit; + const departmentIds = await this.scopeResolverService.ResolveDepartmentIds( query.semesterId, ); @@ -83,7 +97,7 @@ export class CurriculumService { } } - const departmentFilter: Record<string, unknown> = { + const departmentFilter: FilterQuery<Department> = { semester: query.semesterId, }; @@ -91,40 +105,39 @@ export class CurriculumService { departmentFilter.id = query.departmentId; } else if (departmentIds !== null) { if (departmentIds.length === 0) { - return []; + return this.BuildEmptyPage(page, limit); } departmentFilter.id = { $in: departmentIds }; } const filter: FilterQuery<Program> = { department: departmentFilter, - } as FilterQuery<Program>; - - if (query.search) { - const escaped = this.EscapeLikeWildcards(query.search); - Object.assign(filter, { - $and: [ - { - $or: [ - { code: { $ilike: `%${escaped}%` } }, - { name: { $ilike: `%${escaped}%` } }, - ], - }, - ], - }); - } + }; - const programs = await this.em.find(Program, filter, { + this.ApplySearchFilter(filter, query.search, ['code', 'name']); + + const [programs, totalItems] = await this.em.findAndCount(Program, filter, { populate: ['department'], orderBy: { name: QueryOrder.ASC_NULLS_LAST }, + limit, + offset, }); - return programs.map((p) => ProgramItemResponseDto.Map(p)); + return { + data: programs.map((p) => ProgramItemResponseDto.Map(p)), + meta: { + totalItems, + itemCount: programs.length, + itemsPerPage: limit, + totalPages: Math.ceil(totalItems / limit), + currentPage: page, + }, + }; } async ListCourses( query: ListCoursesQueryDto, - ): Promise<CourseItemResponseDto[]> { + ): Promise<CourseListResponseDto> { await this.ValidateSemester(query.semesterId); if (!query.programId && !query.departmentId) { @@ -133,6 +146,10 @@ export class CurriculumService { ); } + const page = query.page ?? 1; + const limit = query.limit ?? 10; + const offset = (page - 1) * limit; + const departmentIds = await this.scopeResolverService.ResolveDepartmentIds( query.semesterId, ); @@ -177,7 +194,7 @@ export class CurriculumService { } // Build filter - const departmentFilter: Record<string, unknown> = { + const departmentFilter: FilterQuery<Department> = { semester: query.semesterId, }; @@ -185,12 +202,12 @@ export class CurriculumService { departmentFilter.id = query.departmentId; } else if (departmentIds !== null) { if (departmentIds.length === 0) { - return []; + return this.BuildEmptyPage(page, limit); } departmentFilter.id = { $in: departmentIds }; } - const programFilter: Record<string, unknown> = { + const programFilter: FilterQuery<Program> = { department: departmentFilter, }; @@ -200,28 +217,27 @@ export class CurriculumService { const filter: FilterQuery<Course> = { program: programFilter, - } as FilterQuery<Course>; - - if (query.search) { - const escaped = this.EscapeLikeWildcards(query.search); - Object.assign(filter, { - $and: [ - { - $or: [ - { shortname: { $ilike: `%${escaped}%` } }, - { fullname: { $ilike: `%${escaped}%` } }, - ], - }, - ], - }); - } + }; - const courses = await this.em.find(Course, filter, { + this.ApplySearchFilter(filter, query.search, ['shortname', 'fullname']); + + const [courses, totalItems] = await this.em.findAndCount(Course, filter, { populate: ['program'], orderBy: { shortname: QueryOrder.ASC }, + limit, + offset, }); - return courses.map((c) => CourseItemResponseDto.Map(c)); + return { + data: courses.map((c) => CourseItemResponseDto.Map(c)), + meta: { + totalItems, + itemCount: courses.length, + itemsPerPage: limit, + totalPages: Math.ceil(totalItems / limit), + currentPage: page, + }, + }; } private async ValidateSemester(semesterId: string): Promise<void> { @@ -233,10 +249,41 @@ export class CurriculumService { } } + private ApplySearchFilter( + filter: Record<string, unknown>, + search: string | undefined, + fields: [string, string], + ): void { + if (!search) return; + const escaped = this.EscapeLikeWildcards(search); + Object.assign(filter, { + $and: [ + { + $or: fields.map((field) => ({ + [field]: { $ilike: `%${escaped}%` }, + })), + }, + ], + }); + } + private EscapeLikeWildcards(input: string): string { return input .replace(/\\/g, '\\\\') .replace(/%/g, '\\%') .replace(/_/g, '\\_'); } + + private BuildEmptyPage(page: number, limit: number) { + return { + data: [], + meta: { + totalItems: 0, + itemCount: 0, + itemsPerPage: limit, + totalPages: 0, + currentPage: page, + }, + }; + } } diff --git a/src/modules/enrollments/enrollments.service.spec.ts b/src/modules/enrollments/enrollments.service.spec.ts index 3c9afd6..179c508 100644 --- a/src/modules/enrollments/enrollments.service.spec.ts +++ b/src/modules/enrollments/enrollments.service.spec.ts @@ -2,6 +2,8 @@ import { Test, TestingModule } from '@nestjs/testing'; import { EnrollmentsService } from './enrollments.service'; import { EntityManager } from '@mikro-orm/core'; import { User } from 'src/entities/user.entity'; +import { Enrollment } from 'src/entities/enrollment.entity'; +import { QuestionnaireSubmission } from 'src/entities/questionnaire-submission.entity'; import { CacheService } from '../common/cache/cache.service'; import { CurrentUserService } from '../common/cls/current-user.service'; @@ -92,10 +94,11 @@ describe('EnrollmentsService', () => { ]; (em.findAndCount as jest.Mock).mockResolvedValue([mockEnrollments, 1]); - // Promise.all order: getFacultyByCourseIds first, getSubmissionStatusByCourseIds second - (em.find as jest.Mock) - .mockResolvedValueOnce(mockFacultyEnrollments) - .mockResolvedValueOnce([]); + (em.find as jest.Mock).mockImplementation((entity: unknown) => { + if (entity === Enrollment) return Promise.resolve(mockFacultyEnrollments); + if (entity === QuestionnaireSubmission) return Promise.resolve([]); + return Promise.resolve([]); + }); const result = await service.getMyEnrollments({ page: 1, limit: 10 }); @@ -156,9 +159,12 @@ describe('EnrollmentsService', () => { const mockSubmissions = [{ course: { id: 'c1' }, submittedAt }]; (em.findAndCount as jest.Mock).mockResolvedValue([mockEnrollments, 1]); - (em.find as jest.Mock) - .mockResolvedValueOnce([]) // faculty - .mockResolvedValueOnce(mockSubmissions); // submissions + (em.find as jest.Mock).mockImplementation((entity: unknown) => { + if (entity === Enrollment) return Promise.resolve([]); + if (entity === QuestionnaireSubmission) + return Promise.resolve(mockSubmissions); + return Promise.resolve([]); + }); const result = await service.getMyEnrollments({ page: 1, limit: 10 }); @@ -225,6 +231,58 @@ describe('EnrollmentsService', () => { expect(result.data[0].semester).toBeNull(); }); + it('should return faculty data for teacher role (not just editingteacher)', async () => { + const mockEnrollments = [ + { + id: 'e1', + role: 'student', + course: { + id: 'c1', + moodleCourseId: 101, + shortname: 'CS101', + fullname: 'Intro to CS', + courseImage: null, + program: { + department: { + semester: { + id: 'sem-1', + code: 'S12526', + label: '1st Semester', + academicYear: '2025-2026', + }, + }, + }, + }, + }, + ]; + + const mockFacultyEnrollments = [ + { + course: { id: 'c1' }, + user: { + id: 'faculty-2', + fullName: 'Prof. Jones', + userName: 'EMP002', + userProfilePicture: null, + }, + }, + ]; + + (em.findAndCount as jest.Mock).mockResolvedValue([mockEnrollments, 1]); + (em.find as jest.Mock) + .mockResolvedValueOnce(mockFacultyEnrollments) + .mockResolvedValueOnce([]); + + const result = await service.getMyEnrollments({ page: 1, limit: 10 }); + + expect(result.data[0].faculty).toEqual({ + id: 'faculty-2', + fullName: 'Prof. Jones', + employeeNumber: 'EMP002', + profilePicture: undefined, + }); + }); + it('should not query faculty or submissions when no enrollments exist', async () => { (em.findAndCount as jest.Mock).mockResolvedValue([[], 0]); diff --git a/src/modules/enrollments/enrollments.service.ts b/src/modules/enrollments/enrollments.service.ts index d8264d2..42481a9 100644 --- a/src/modules/enrollments/enrollments.service.ts +++ b/src/modules/enrollments/enrollments.service.ts @@ -8,6 +8,7 @@ import { CurrentUserService } from '../common/cls/current-user.service'; import { MyEnrollmentsQueryDto } from './dto/requests/my-enrollments-query.dto'; import { FacultyShortResponseDto } from './dto/responses/faculty-short.response.dto'; import { MyEnrollmentsResponseDto } from './dto/responses/my-enrollments.response.dto'; +import { EnrollmentRole } from '../questionnaires/lib/questionnaire.types'; @Injectable() export class EnrollmentsService { @@ -124,8 +125,23 @@ export class EnrollmentsService { const facultyEnrollments = await this.em.find( Enrollment, - { course: { $in: courseIds }, role: 'editingteacher', isActive: true }, - { populate: ['user', 'course'] }, + { + course: { $in: courseIds }, + role: { $in: [EnrollmentRole.EDITING_TEACHER, EnrollmentRole.TEACHER] }, + isActive: true, + }, + { + populate: ['user'], + fields: [ + 'course', + 'user.id', + 'user.fullName', + 'user.firstName', + 'user.lastName', + 'user.userName', + 'user.userProfilePicture', + ], + }, ); for (const enrollment of facultyEnrollments) { diff --git a/src/modules/faculty/services/faculty.service.ts b/src/modules/faculty/services/faculty.service.ts index bcdf2cc..6915c85 100644 --- a/src/modules/faculty/services/faculty.service.ts +++ b/src/modules/faculty/services/faculty.service.ts @@ -17,6 +17,7 @@ import { FacultyCardResponseDto } from '../dto/responses/faculty-card.response.d import { SubmissionCountResponseDto } from '../dto/responses/submission-count.response.dto'; import { Course } from 'src/entities/course.entity'; import { FilterQuery } from '@mikro-orm/core'; +import { EnrollmentRole } from 'src/modules/questionnaires/lib/questionnaire.types'; @Injectable() export class FacultyService { @@ -127,7 +128,9 @@ export class FacultyService { Enrollment, { user: { $in: userIds }, - role: { $in: ['editingteacher', 'teacher'] }, + role: { + $in: [EnrollmentRole.EDITING_TEACHER, EnrollmentRole.TEACHER], + }, isActive: true, course: this.BuildCourseFilter(query, departmentIds), }, 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<TriggerSyncResponseDto> { 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/lib/questionnaire.types.ts b/src/modules/questionnaires/lib/questionnaire.types.ts index 3b8a40c..d1a77f3 100644 --- a/src/modules/questionnaires/lib/questionnaire.types.ts +++ b/src/modules/questionnaires/lib/questionnaire.types.ts @@ -21,6 +21,7 @@ export enum RespondentRole { export enum EnrollmentRole { STUDENT = 'student', EDITING_TEACHER = 'editingteacher', + TEACHER = 'teacher', } export interface QuestionNode { 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<QuestionnaireService>; 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,