feat(audit): add admin audit log for sensitive actions#34
Open
kimanicode wants to merge 1 commit into
Open
Conversation
- Add AuditLog entity, migration, service, and admin-only controller - Log USER_ROLE_CHANGED, USER_DELETED, USER_LOGIN_FAILED events - Add GET /admin/audit-logs with pagination and filtering by action, actorId, and date range - Add AdminGuard for audit endpoints (role lookup from DB) - Add PATCH /users/:id/role endpoint, wired to audit logging - Redact sensitive fields (passwords, tokens) from logged metadata - Add indexes on createdAt, action, actorId - Add docs/audit-log.md - Add unit and e2e tests for service, controller, and admin-only access Closes MyFanss#25
Contributor
|
@kimanicode Please fix the CI to pass |
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
Adds an append-only audit trail for sensitive admin/user actions (role changes, user deletions, failed logins), exposed via a paginated, filterable admin-only endpoint.
Closes #25
What's included
AuditLogentity + migration —actorId,action,targetType,targetId,metadata(jsonb),ipAddress,createdAt. Indexes oncreatedAt,action,actorId.AuditService— injectablelog()method, used across modules. Stripspasswords/tokens/secrets from metadata before persisting; no update/delete methods exist
on the service or entity.
GET /admin/audit-logs— admin-only (via newAdminGuard), paginated, filterable byaction,actorId, and date range. No PATCH/PUT/DELETE routes — logs are immutable.PATCH /users/:id/roleendpoint logsUSER_ROLE_CHANGEDwithbefore/afterrolein metadata
USER_DELETEDUSER_LOGIN_FAILED(admin-visible only, never returned tothe end user) with
ipAddresscaptureddocs/audit-log.md— schema, action list, endpoint usage, redaction policyAuditServiceandAuditController, e2e tests for admin-onlyaccess and immutability
Deviations from the issue spec
AdminGuardinsrc/audit/admin.guard.ts.It looks up the user's role from the DB since the JWT payload doesn't carry role. Happy to
move this somewhere more central (e.g.
src/common/guards/) if that's preferred — flaggingfor review.
PATCH /users/:id/roleas a new admin-onlyroute, since the existing
updateUser()is unguarded and a poor fit for audit-logged rolechanges. Open to feedback on the route shape.
actorIdis nullable — the issue lists it as part of the core fields, but also impliesUSER_LOGIN_FAILEDmay have no known actor (e.g. unknown/invalid username). Made itnullable to support that case.
page/limit), not the cursor-based pagination usedelsewhere for users. Audit logs are append-only and queried by filters/date range rather
than infinite-scroll, so keyset pagination didn't seem worth the added complexity — reused
the existing
PaginatedResponseDtowrapper either way.Testing
tsc --noEmit— no type errorstest/audit.e2e-spec.ts) — written and included, but could not run locallydue to no Postgres test DB available in this sandbox (
my_fans_teston localhost). Pleaserun in CI or let me know if you'd like me to set up a local DB and re-verify before merge.
Checklist (against acceptance criteria)