Release FAC-87–98: Auth Security Fixes, Curriculum Pagination & Audit Trail MVP#217
Merged
Release FAC-87–98: Auth Security Fixes, Curriculum Pagination & Audit Trail MVP#217
Conversation
…hToken #199 Replace bcrypt.compareSync with concurrent async bcrypt.compare via Promise.all to avoid blocking the event loop (#195). Add expiresAt filter to the DB query so expired tokens are excluded before running expensive bcrypt comparisons (#197). Closes #195 Closes #197 https://claude.ai/code/session_01XttX7NLr6HY7TDLddDk2AG Co-authored-by: Claude <noreply@anthropic.com>
#201 Empty strings and arbitrarily long payloads were passing validation on the login endpoint. Add @isnotempty() and @maxlength() decorators to both username and password fields, consistent with RefreshTokenRequestBody. Add DTO validation tests covering empty and oversized inputs. Closes #196
…ollmentRole enum #206 getFacultyByCourseIds() only queried 'editingteacher', missing faculty with the 'teacher' Moodle role. Now queries both roles via the EnrollmentRole enum, matching the pattern already used in FacultyService. Closes #202, Closes #203 https://claude.ai/code/session_01LNfjqKQstgZ7skvJ2oXAkf Co-authored-by: Claude <noreply@anthropic.com>
Drop eager Course populate (FK already on Enrollment) and restrict User populate to the 5 columns actually used in the mapping, reducing DB I/O. https://claude.ai/code/session_01U2k8WWJ9EJ9b3Rzg4Vq81S Co-authored-by: Claude <noreply@anthropic.com>
…call order #208 Replace mockResolvedValueOnce chains with mockImplementation that matches on entity class (Enrollment vs QuestionnaireSubmission), making the test resilient to Promise.all reordering in the service. Closes #205 https://claude.ai/code/session_012UsMjq61dUvFhErpChg9cR Co-authored-by: Claude <noreply@anthropic.com>
…rch deduplication#212 Replace `Record<string, unknown>` with typed `FilterQuery<Department>` and `FilterQuery<Program>` intermediates, removing all `as FilterQuery<T>` casts in ListPrograms and ListCourses (#210). Extract duplicated $and/$or/$ilike search block into a shared ApplySearchFilter helper used by all three list methods (#211). Closes #210 Closes #211 https://claude.ai/code/session_01Y6eWhKjmgLfPJQMJa4VQQB Co-authored-by: Claude <noreply@anthropic.com>
Curriculum list endpoints (/departments, /programs, /courses) returned
unbounded result sets. Add page/limit query params via PaginationQueryDto
and return paginated { data, meta } responses using findAndCount, matching
the pattern used by admin, faculty, and dimensions modules.
https://claude.ai/code/session_01Ug6JLCMPhy42Skcq7tWyVV
Co-authored-by: Claude <noreply@anthropic.com>
* docs: add audit trail MVP tech-spec WIP Initialize tech-spec for audit trail feature covering auth events, admin actions, and sensitive data mutations via BullMQ queue. https://claude.ai/code/session_01EXwPk6t54GxvuoMNtD1SSP * docs: update audit trail tech-spec with deep investigation findings Add detailed file references, code patterns, MVP endpoint targets, and testing strategy from Step 2 codebase investigation. https://claude.ai/code/session_01EXwPk6t54GxvuoMNtD1SSP * docs: generate full implementation plan for audit trail tech-spec Add 20 tasks across 5 phases, 11 acceptance criteria, testing strategy, and risk analysis. Step 3 of quick-spec workflow complete. https://claude.ai/code/session_01EXwPk6t54GxvuoMNtD1SSP * docs: finalize audit trail MVP tech-spec as ready-for-dev Rename WIP to tech-spec-audit-trail-mvp.md and mark status as ready-for-dev. Quick-spec workflow complete (steps 1-4). https://claude.ai/code/session_01EXwPk6t54GxvuoMNtD1SSP * docs: apply pre-mortem hardening to audit trail tech-spec Add 6 preventions from pre-mortem analysis: @optional() AuditService injection in AuthService, Logger.warn on failed emissions, attempts:1 on audit jobs, CLS null-safety warnings, bulk ingestion documentation, and full payload logging on processor failures. https://claude.ai/code/session_01EXwPk6t54GxvuoMNtD1SSP * docs: add team refinements to audit trail tech-spec Log full emission params on failure, add @optional() undefined test case for auth service, add CLS null-metadata warning test for interceptor. https://claude.ai/code/session_01EXwPk6t54GxvuoMNtD1SSP * docs: apply all 23 adversarial review fixes to audit trail spec Critical: @global() AuditModule, explicit interceptor stacks on all MVP endpoints, fix DI scoping for host modules. High: move login/refresh to direct emit, extend @Audited() with resource option, add AuditAction enum, tap vs finalize docs. Medium: drop repo stub, fix occurredAt, add attempts:1 in code, separate browserName/os fields, fix role claim, complete file list. Low: clarify emit location, entity array registration, test Redis, interceptor ordering, endpoint count. https://claude.ai/code/session_01EXwPk6t54GxvuoMNtD1SSP * docs: apply round 2 adversarial review fixes to audit trail spec Fix 14 findings: emit audit AFTER transactions with void (fire-and- forget), replace existing @UseInterceptors instead of adding duplicates, AuditInterceptor falls back to JWT payload (no CurrentUserInterceptor required), import AppClsModule instead of CommonModule, PII-safe logging in failed handler, PostgreSQL DEFAULT on occurred_at, fix AC 4 syntax and Task 6 Zod contradiction, document empty metadata for /ingest and null resourceId for created-by-action scenarios. https://claude.ai/code/session_01EXwPk6t54GxvuoMNtD1SSP * docs: apply round 3 adversarial review fixes to audit trail spec Critical: add explicit transaction refactoring pattern with code snippets for Login() and RefreshToken() — emit after transaction returns, not inside. High: correct @optional() resilience claim, fix resourceType for wipe endpoint, add per-endpoint interceptor bullets, acknowledge logout MetaDataInterceptor gap. Medium: add metadata schema table, full UUID regex, AuthenticatedRequest type, justify @global() convention, full FileInterceptor config, fix /ingest CurrentUserInterceptor contradiction. Low: create()+flush() pattern, processor log counter, fix test description, base class name. https://claude.ai/code/session_01EXwPk6t54GxvuoMNtD1SSP * docs: apply round 4 adversarial review fixes to audit trail spec Fix metadata table accuracy (4 rows showed wrong shapes), add null- safe destructure for RequestMetadataService in direct emit, strategy Name fallback for minification, type action as AuditAction end-to-end, consistent PII redaction policy, add CurrentUserInterceptor to POST /submissions (highest-volume), make occurredAt required (no JS default), document MetaDataInterceptor DI resolution, clarify attempts config, note create-action resourceId gap, Redis failed job monitoring guidance, actorId is plain string not @manytoone, logging side effect note. https://claude.ai/code/session_01EXwPk6t54GxvuoMNtD1SSP * FAC-98 feat: add audit trail MVP Implement append-only AuditLog entity backed by a BullMQ AUDIT queue with dual emission paths — interceptor for authenticated endpoints and direct emit for auth events where CLS context is unavailable. - AuditLog entity (no CustomBaseEntity, no soft delete, no FK to User) - AuditService with fire-and-forget Emit() that never breaks requests - @Audited() decorator + AuditInterceptor (RxJS tap, post-response) - @global() AuditModule — cross-cutting concern, no explicit imports - 11 MVP endpoints tagged: auth (login/logout/refresh), moodle sync, questionnaire (submit/ingest/wipe), analysis (create/confirm/cancel) - Sanitized failure metadata (fixed reason codes, no raw error.message) - Metadata size cap (4KB) on interceptor-captured params/query - actorId index for "what did user X do?" queries - Shared audit test helpers to DRY controller spec boilerplate - 622 tests passing, lint clean --------- Co-authored-by: Claude <noreply@anthropic.com>
y4nder
added a commit
that referenced
this pull request
Mar 30, 2026
… Trail MVP (#217) * chore: add /audit skill with auto GitHub issue creation * FAC-87 FAC-88 fix: async bcrypt and expired token filtering in RefreshToken #199 Replace bcrypt.compareSync with concurrent async bcrypt.compare via Promise.all to avoid blocking the event loop (#195). Add expiresAt filter to the DB query so expired tokens are excluded before running expensive bcrypt comparisons (#197). Closes #195 Closes #197 https://claude.ai/code/session_01XttX7NLr6HY7TDLddDk2AG Co-authored-by: Claude <noreply@anthropic.com> * FAC-89 fix: username logged in plaintext in Moodle login error path #200 Closes #198 * FAC-90 fix: LoginRequest DTO missing @isnotempty and length constraints #201 Empty strings and arbitrarily long payloads were passing validation on the login endpoint. Add @isnotempty() and @maxlength() decorators to both username and password fields, consistent with RefreshTokenRequestBody. Add DTO validation tests covering empty and oversized inputs. Closes #196 * FAC-91 FAC-92 fix: include teacher role in faculty lookup and use EnrollmentRole enum #206 getFacultyByCourseIds() only queried 'editingteacher', missing faculty with the 'teacher' Moodle role. Now queries both roles via the EnrollmentRole enum, matching the pattern already used in FacultyService. Closes #202, Closes #203 https://claude.ai/code/session_01LNfjqKQstgZ7skvJ2oXAkf Co-authored-by: Claude <noreply@anthropic.com> * FAC-93 perf: limit fields in getFacultyByCourseIds query #207 Drop eager Course populate (FK already on Enrollment) and restrict User populate to the 5 columns actually used in the mapping, reducing DB I/O. https://claude.ai/code/session_01U2k8WWJ9EJ9b3Rzg4Vq81S Co-authored-by: Claude <noreply@anthropic.com> * FAC-94 refactor: enrollment test relies on implicit Promise.all mock call order #208 Replace mockResolvedValueOnce chains with mockImplementation that matches on entity class (Enrollment vs QuestionnaireSubmission), making the test resilient to Promise.all reordering in the service. Closes #205 https://claude.ai/code/session_012UsMjq61dUvFhErpChg9cR Co-authored-by: Claude <noreply@anthropic.com> * FAC-95 FAC-96 refactor: curriculum service filter type safety and search deduplication#212 Replace `Record<string, unknown>` with typed `FilterQuery<Department>` and `FilterQuery<Program>` intermediates, removing all `as FilterQuery<T>` casts in ListPrograms and ListCourses (#210). Extract duplicated $and/$or/$ilike search block into a shared ApplySearchFilter helper used by all three list methods (#211). Closes #210 Closes #211 https://claude.ai/code/session_01Y6eWhKjmgLfPJQMJa4VQQB Co-authored-by: Claude <noreply@anthropic.com> * FAC-97 perf: add pagination to curriculum list endpoints#213 Curriculum list endpoints (/departments, /programs, /courses) returned unbounded result sets. Add page/limit query params via PaginationQueryDto and return paginated { data, meta } responses using findAndCount, matching the pattern used by admin, faculty, and dimensions modules. https://claude.ai/code/session_01Ug6JLCMPhy42Skcq7tWyVV Co-authored-by: Claude <noreply@anthropic.com> * FAC-98 feat: add audit trail MVP (#215) * docs: add audit trail MVP tech-spec WIP Initialize tech-spec for audit trail feature covering auth events, admin actions, and sensitive data mutations via BullMQ queue. https://claude.ai/code/session_01EXwPk6t54GxvuoMNtD1SSP * docs: update audit trail tech-spec with deep investigation findings Add detailed file references, code patterns, MVP endpoint targets, and testing strategy from Step 2 codebase investigation. https://claude.ai/code/session_01EXwPk6t54GxvuoMNtD1SSP * docs: generate full implementation plan for audit trail tech-spec Add 20 tasks across 5 phases, 11 acceptance criteria, testing strategy, and risk analysis. Step 3 of quick-spec workflow complete. https://claude.ai/code/session_01EXwPk6t54GxvuoMNtD1SSP * docs: finalize audit trail MVP tech-spec as ready-for-dev Rename WIP to tech-spec-audit-trail-mvp.md and mark status as ready-for-dev. Quick-spec workflow complete (steps 1-4). https://claude.ai/code/session_01EXwPk6t54GxvuoMNtD1SSP * docs: apply pre-mortem hardening to audit trail tech-spec Add 6 preventions from pre-mortem analysis: @optional() AuditService injection in AuthService, Logger.warn on failed emissions, attempts:1 on audit jobs, CLS null-safety warnings, bulk ingestion documentation, and full payload logging on processor failures. https://claude.ai/code/session_01EXwPk6t54GxvuoMNtD1SSP * docs: add team refinements to audit trail tech-spec Log full emission params on failure, add @optional() undefined test case for auth service, add CLS null-metadata warning test for interceptor. https://claude.ai/code/session_01EXwPk6t54GxvuoMNtD1SSP * docs: apply all 23 adversarial review fixes to audit trail spec Critical: @global() AuditModule, explicit interceptor stacks on all MVP endpoints, fix DI scoping for host modules. High: move login/refresh to direct emit, extend @Audited() with resource option, add AuditAction enum, tap vs finalize docs. Medium: drop repo stub, fix occurredAt, add attempts:1 in code, separate browserName/os fields, fix role claim, complete file list. Low: clarify emit location, entity array registration, test Redis, interceptor ordering, endpoint count. https://claude.ai/code/session_01EXwPk6t54GxvuoMNtD1SSP * docs: apply round 2 adversarial review fixes to audit trail spec Fix 14 findings: emit audit AFTER transactions with void (fire-and- forget), replace existing @UseInterceptors instead of adding duplicates, AuditInterceptor falls back to JWT payload (no CurrentUserInterceptor required), import AppClsModule instead of CommonModule, PII-safe logging in failed handler, PostgreSQL DEFAULT on occurred_at, fix AC 4 syntax and Task 6 Zod contradiction, document empty metadata for /ingest and null resourceId for created-by-action scenarios. https://claude.ai/code/session_01EXwPk6t54GxvuoMNtD1SSP * docs: apply round 3 adversarial review fixes to audit trail spec Critical: add explicit transaction refactoring pattern with code snippets for Login() and RefreshToken() — emit after transaction returns, not inside. High: correct @optional() resilience claim, fix resourceType for wipe endpoint, add per-endpoint interceptor bullets, acknowledge logout MetaDataInterceptor gap. Medium: add metadata schema table, full UUID regex, AuthenticatedRequest type, justify @global() convention, full FileInterceptor config, fix /ingest CurrentUserInterceptor contradiction. Low: create()+flush() pattern, processor log counter, fix test description, base class name. https://claude.ai/code/session_01EXwPk6t54GxvuoMNtD1SSP * docs: apply round 4 adversarial review fixes to audit trail spec Fix metadata table accuracy (4 rows showed wrong shapes), add null- safe destructure for RequestMetadataService in direct emit, strategy Name fallback for minification, type action as AuditAction end-to-end, consistent PII redaction policy, add CurrentUserInterceptor to POST /submissions (highest-volume), make occurredAt required (no JS default), document MetaDataInterceptor DI resolution, clarify attempts config, note create-action resourceId gap, Redis failed job monitoring guidance, actorId is plain string not @manytoone, logging side effect note. https://claude.ai/code/session_01EXwPk6t54GxvuoMNtD1SSP * FAC-98 feat: add audit trail MVP Implement append-only AuditLog entity backed by a BullMQ AUDIT queue with dual emission paths — interceptor for authenticated endpoints and direct emit for auth events where CLS context is unavailable. - AuditLog entity (no CustomBaseEntity, no soft delete, no FK to User) - AuditService with fire-and-forget Emit() that never breaks requests - @Audited() decorator + AuditInterceptor (RxJS tap, post-response) - @global() AuditModule — cross-cutting concern, no explicit imports - 11 MVP endpoints tagged: auth (login/logout/refresh), moodle sync, questionnaire (submit/ingest/wipe), analysis (create/confirm/cancel) - Sanitized failure metadata (fixed reason codes, no raw error.message) - Metadata size cap (4KB) on interceptor-captured params/query - actorId index for "what did user X do?" queries - Shared audit test helpers to DRY controller spec boilerplate - 622 tests passing, lint clean --------- Co-authored-by: Claude <noreply@anthropic.com> --------- Co-authored-by: Claude <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Cherry-picks develop commits FAC-87 through FAC-98 (plus chore) onto staging for pre-production validation.
Commits Included
@IsNotEmptyand length constraintsgetFacultyByCourseIdsqueryTest Plan