feat: Audit log#314
Merged
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
Adds a user/account audit logging feature to OpenShock, including persistence, retrieval APIs, and retention cleanup, and wires logging into several security-sensitive account/token flows.
Changes:
- Introduces
UserAuditLogentity +audit_actionPostgres enum, EF mappings, and migration. - Adds
IAuditService/AuditServicefor writing and paging audit log entries, plus API endpoints to read them (user + admin). - Emits audit events from login/logout, email/password/username changes, OAuth connect/disconnect, token create/delete, and admin deactivate/reactivate/delete actions; adds a cron job to clear old audit logs.
Reviewed changes
Copilot reviewed 32 out of 33 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| Cron/Jobs/ClearOldAuditLogsJob.cs | Cron job to delete audit logs older than retention cutoff. |
| Common/Services/Audit/IAuditService.cs | Audit service contract for logging and paged retrieval. |
| Common/Services/Audit/AuditService.cs | Implementation for writing audit entries and querying with pagination/sorting. |
| Common/OpenShockServiceHelper.cs | Registers IAuditService in DI. |
| Common/OpenShockDb/UserAuditLog.cs | New EF entity representing an audit log row. |
| Common/OpenShockDb/User.cs | Adds navigation collections for audit logs (subject + actor). |
| Common/OpenShockDb/OpenShockContext.cs | Adds DbSet, enum registration, and entity mapping for audit logs. |
| Common/OpenShockControllerBase.cs | Logs Login audit event when creating a session. |
| Common/Models/AuditMetadata.cs | Adds polymorphic metadata records for audit events. |
| Common/Models/AuditAction.cs | Adds AuditAction enum with PgName mappings. |
| Common/Migrations/OpenShockContextModelSnapshot.cs | Updates snapshot to include audit enum/table. |
| Common/Migrations/20260612135418_AddUserAuditLog.Designer.cs | Migration designer for audit enum/table. |
| Common/Migrations/20260612135418_AddUserAuditLog.cs | Migration creating user_audit_logs and audit_action enum. |
| API/Services/Token/IApiTokenService.cs | Adds method to fetch minimal token info for audit logging on delete. |
| API/Services/Token/ApiTokenService.cs | Implements token audit info lookup. |
| API/Services/Account/IAccountService.cs | Changes TryVerifyEmailAsync to return data needed for audit logging. |
| API/Services/Account/AccountService.cs | Returns old/new email details from verify flow for auditing. |
| API/Models/Response/AuditLogEntryResponse.cs | API response DTO for audit log entries. |
| API/Controller/Tokens/Tokens.cs | Logs ApiTokenCreated audit events for token creation endpoints. |
| API/Controller/Tokens/DeleteToken.cs | Logs ApiTokenDeleted audit events on successful token deletion. |
| API/Controller/OAuth/HandOff.cs | Logs OAuthConnected audit event after linking. |
| API/Controller/Admin/ReactivateUser.cs | Logs AccountReactivated on successful admin reactivation. |
| API/Controller/Admin/GetAuditLog.cs | Adds admin endpoint to page audit logs across users with filters. |
| API/Controller/Admin/DeleteUser.cs | Logs AccountDeleted on successful admin deletion. |
| API/Controller/Admin/DeactivateUser.cs | Logs AccountDeactivated on successful admin deactivation (with metadata). |
| API/Controller/Account/VerifyEmail.cs | Logs EmailChanged on successful email verification. |
| API/Controller/Account/Logout.cs | Logs Logout when a session is successfully deleted. |
| API/Controller/Account/Authenticated/OAuthConnectionRemove.cs | Logs OAuthDisconnected when a connection is removed. |
| API/Controller/Account/Authenticated/GetAuditLog.cs | Adds user endpoint to page the current user’s audit log. |
| API/Controller/Account/Authenticated/Deactivate.cs | Logs AccountDeactivated for self-deactivation. |
| API/Controller/Account/Authenticated/ChangeUsername.cs | Logs UsernameChanged including old/new username metadata. |
| API/Controller/Account/Authenticated/ChangePassword.cs | Logs PasswordChanged on successful password update. |
| API/Controller/Account/Authenticated/ChangeEmail.cs | Logs EmailChangeRequested with new email metadata. |
Files not reviewed (1)
- Common/Migrations/20260612135418_AddUserAuditLog.Designer.cs: Generated file
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Resolve conflicts and adapt the audit-log feature to develop's OpenShockDb reorg + centralized Postgres enum registration (0f5c323): - OpenShockContext: keep develop's .RegisterPgEnums() instead of the manual HasPostgresEnum chain. - Move AuditAction into OpenShockDb/Enums with [PgEnum("audit_action")] and register it in NpgsqlEnumExtensions.Enums, so it flows through both MapPgEnums() and RegisterPgEnums(). This makes the audit_action column map as a real Postgres enum (addresses Copilot review comment #1). - Move UserAuditLog into OpenShockDb/Models for folder consistency. - Point controllers/response models at the new OpenShock.Common.OpenShockDb namespace for AuditAction/RoleType. Recreate the migration on top of develop: - Regenerate AddUserAuditLog (now 20260701144432, last in order) via EF so its designer + model snapshot reflect develop's refactored enums; the old 20260612 migration sorted before AddEmailOutbox/RefactorPgEnums and left the snapshot inconsistent. 'has-pending-model-changes' now reports clean. Address remaining Copilot review comments: - Email verify: use the stored UserEmailChange.OldEmail rather than the user's current email, which can change between request and verification. - Token audit metadata: persist canonical permission names via PermissionTypeBindings instead of PermissionType.ToString() (Shockers_Use).
|
Ready to review this PR? Stage has broken it down into 8 individual chapters for you: 
Chapters generated by Stage for commit fdd2468 on Jul 9, 2026 10:40am UTC. |
The audit-log controller methods gained an auditService parameter but lacked matching <param> doc tags, failing the build under warnings-as-errors (CS1573). Also removes duplicate tokenId/cancellationToken tags in DeleteToken.
TryRemoveConnectionAsync wrote and committed an OAuthDisconnected audit entry even when no connection was linked (nDeleted == 0), recording a disconnection that never happened. Return early before logging so the no-op delete rolls back on transaction disposal. LogoutSessionAsync deleted the Redis session before writing the audit entry, so a failed audit write left the session gone while the caller saw an error and no audit trail. Log first; the session delete is idempotent, so the caller can safely retry.
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 37 out of 38 changed files in this pull request and generated 5 comments.
Files not reviewed (1)
- Common/Migrations/20260701144432_AddUserAuditLog.Designer.cs: Generated file
Comments suppressed due to low confidence (1)
Common/OpenShockControllerBase.cs:8
using OpenShock.Common.OpenShockDb;appears unused in this file and will trigger CS8019 (Debug builds treat warnings as errors). Remove it.
using Microsoft.AspNetCore.Mvc;
using OpenShock.Common.Constants;
using OpenShock.Common.Models;
using OpenShock.Common.OpenShockDb;
using OpenShock.Common.Options;
using OpenShock.Common.Problems;
using OpenShock.Common.Services.Session;
using OpenShock.Common.Utils;
CompletePasswordResetFlowAsync changed the password via ExecuteUpdate but emitted no PasswordChanged audit entry, leaving the email-token reset flow (the one an account takeover would use) unaudited while the authenticated change was logged. Wrap the apply in a transaction and log PasswordChanged with a null actor (unauthenticated token flow, actor defaults to subject). Token creation relied on LogAsync's actorId default; pass actorId: userId explicitly to match every other call site.
Replace the non-nullable actor_id + `?? userId` coalesce with a nullable actor_id whose value carries the meaning directly: null = system/automated, == user_id = self-service, otherwise the moderator's id. No discriminator column needed, and "performed by X" filtering stays a simple equality since self actions store actor_id == user_id. Add an optional reason column (varchar 512) for moderator accountability, wired through the admin deactivate/reactivate/delete endpoints as a length-capped query param. Self and system actions leave it null. LogAsync now takes an explicit Guid? actorId (no silent self-coalesce) plus reason. actor_id FK is ON DELETE SET NULL so deleting a moderator never cascades away another user's audit history. Regenerated the (unmerged) AddUserAuditLog migration; snapshot reports no pending model changes.
Drop dead 'using System.Net;' from IAuditService/AuditService and the unused 'using OpenShock.Common.OpenShockDb;' from HandOff.
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.
No description provided.