rafraa/TX
0
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Record more sensitive admin actions in the audit log (Task #93)

Browse Source 
Expanded audit_logs coverage from "force deletions only" to a wider set
of sensitive admin actions. The Audit Log view's action filter dropdown
picks them up automatically (it queries distinct actions from the table).

New audit actions written:
- user.delete (every delete; metadata includes username, email, force,
  and dependent counts when applicable). Replaces user.force_delete for
  new entries; old rows remain.
- role.create / role.update / role.delete (in artifacts/api-server/src/
  routes/roles.ts). role.update only fires when fields actually change
  and records previousName + changes.
- role.permissions.replace / role.permission.add / role.permission.remove
  for the PUT /roles/:id/permissions, POST /roles/:id/permissions, and
  DELETE /roles/:id/permissions/:permissionId endpoints. Replace records
  added[] and removed[] diffs; add/remove records permissionId and
  permissionName so the affected permission stays identifiable even if
  later renamed/deleted. No-op writes (same permission re-added, replace
  with identical set) skip the audit.
- group.create / group.update / group.delete (group.delete replaces
  group.force_delete for new entries; metadata always includes force +
  counts). group.update records changed fields and added/removed lists
  for members, apps, and roles when those collections were touched.
- group.user.add / group.user.remove / group.app.add / group.app.remove
  / group.role.add / group.role.remove for the sub-resource
  POST/DELETE /groups/:id/:kind/:targetId endpoints (only when something
  actually changed, via .returning() guard).
- auth.issue_reset_link in routes/auth.ts admin issue-reset-link.
- app.create / app.update / app.delete (app.delete replaces
  app.force_delete for new entries with force flag in metadata).
  app.update only fires when something actually changed and records a
  per-field {from, to} diff.
- settings.update with per-field {from, to} diff (covers
  registrationOpen and any other UpdateAppSettingsBody field).

Out of scope / deviations:
- "App-permission changes" is in the task description, but no admin
  endpoints exist yet to write app_permissions. A separate task ("Let
  admins manage which permission an app requires from the UI") will add
  them. Filed follow-up #113 to add the audit hooks once those endpoints
  exist. Also filed #114 (readable summaries for new actions) and #115
  (automated tests for the expanded coverage).
- The pre-existing standalone task "Record an audit trail when a role's
  permissions change" was folded into this task at code-review request,
  since the same task description called out role permissions. That
  separate task is now obsolete.

Verified end-to-end via curl against the running API: every new action
appears in /api/admin/audit-logs and in the actions[] dropdown list,
including all three role.permission* actions.

Replit-Task-Id: f078e4fd-afdf-4c71-b29c-c0cae026af1b
This commit is contained in:
riyadhafraa
2026-04-28 20:47:24 +00:00
parent bdc19d5012
commit 675a4a418a
6 changed files with 318 additions and 58 deletions
Download Patch File Download Diff File Expand all files Collapse all files
artifacts/api-server/src/routes/apps.ts
+56 -15
View File
@@ -175,6 +175,19 @@ router.post("/apps", requireAdmin, async (req, res): Promise<void> => {
}
const [app] = await db.insert(appsTable).values(parsed.data).returning();
await db.insert(auditLogsTable).values({
actorUserId: req.session.userId ?? null,
action: "app.create",
targetType: "app",
targetId: app.id,
metadata: {
slug: app.slug,
nameAr: app.nameAr,
nameEn: app.nameEn,
route: app.route,
isActive: app.isActive,
},
});
res.status(201).json(app);
});
@@ -211,6 +224,11 @@ router.patch("/apps/:id", requireAdmin, async (req, res): Promise<void> => {
return;
}
const [previous] = await db
.select()
.from(appsTable)
.where(eq(appsTable.id, params.data.id));
const [app] = await db
.update(appsTable)
.set(parsed.data)
@@ -222,6 +240,29 @@ router.patch("/apps/:id", requireAdmin, async (req, res): Promise<void> => {
return;
}
if (previous) {
const changes: Record<string, { from: unknown; to: unknown }> = {};
for (const [key, value] of Object.entries(parsed.data)) {
const prevValue = (previous as Record<string, unknown>)[key];
if (prevValue !== value) {
changes[key] = { from: prevValue ?? null, to: value ?? null };
}
}
if (Object.keys(changes).length > 0) {
await db.insert(auditLogsTable).values({
actorUserId: req.session.userId ?? null,
action: "app.update",
targetType: "app",
targetId: app.id,
metadata: {
slug: app.slug,
nameEn: app.nameEn,
changes,
},
});
}
}
res.json(app);
});
@@ -297,21 +338,21 @@ router.delete("/apps/:id", requireAdmin, async (req, res): Promise<void> => {
await db.delete(appsTable).where(eq(appsTable.id, appId));
if (hasDeps && force) {
await db.insert(auditLogsTable).values({
actorUserId: req.session.userId ?? null,
action: "app.force_delete",
targetType: "app",
targetId: appId,
metadata: {
slug: existing.slug,
nameEn: existing.nameEn,
groupCount,
restrictionCount,
openCount,
},
});
}
await db.insert(auditLogsTable).values({
actorUserId: req.session.userId ?? null,
action: "app.delete",
targetType: "app",
targetId: appId,
metadata: {
slug: existing.slug,
nameAr: existing.nameAr,
nameEn: existing.nameEn,
force,
groupCount,
restrictionCount,
openCount,
},
});
res.sendStatus(204);
});
artifacts/api-server/src/routes/auth.ts
+13
View File
@@ -11,6 +11,7 @@ import {
passwordResetTokensTable,
groupsTable,
userGroupsTable,
auditLogsTable,
} from "@workspace/db";
import { requireAuth, requireAdmin, getUserRoles } from "../middlewares/auth";
import { saveSession, destroySession } from "../lib/session";
@@ -319,6 +320,18 @@ router.post(
`${req.protocol}://${req.get("host")}`;
const resetUrl = `${origin.replace(/\/$/, "")}/reset-password?token=${rawToken}`;
await db.insert(auditLogsTable).values({
actorUserId: req.session.userId ?? null,
action: "auth.issue_reset_link",
targetType: "user",
targetId: user.id,
metadata: {
username: user.username,
email: user.email,
expiresAt: expiresAt.toISOString(),
},
});
res.json({ resetUrl, expiresAt: expiresAt.toISOString() });
},
);
artifacts/api-server/src/routes/groups.ts
+123 -26
View File
@@ -240,6 +240,20 @@ router.post("/groups", requireAdmin, async (req, res): Promise<void> => {
);
return g;
});
await db.insert(auditLogsTable).values({
actorUserId: req.session.userId ?? null,
action: "group.create",
targetType: "group",
targetId: group.id,
metadata: {
name: group.name,
descriptionAr: group.descriptionAr,
descriptionEn: group.descriptionEn,
memberCount: parsed.data.userIds?.length ?? 0,
appCount: parsed.data.appIds?.length ?? 0,
roleCount: parsed.data.roleIds?.length ?? 0,
},
});
res.status(201).json({
...serializeGroup(group),
memberCount: parsed.data.userIds?.length ?? 0,
@@ -274,6 +288,18 @@ router.patch("/groups/:id", requireAdmin, async (req, res): Promise<void> => {
return;
}
const previousMemberIds = await getGroupMemberIds(id);
const previousAppIds = (
await db
.select({ appId: groupAppsTable.appId })
.from(groupAppsTable)
.where(eq(groupAppsTable.groupId, id))
).map((r) => r.appId);
const previousRoleIds = (
await db
.select({ roleId: groupRolesTable.roleId })
.from(groupRolesTable)
.where(eq(groupRolesTable.groupId, id))
).map((r) => r.roleId);
const updates: Partial<typeof groupsTable.$inferInsert> = {};
if (parsed.data.name !== undefined) updates.name = parsed.data.name;
if (parsed.data.descriptionAr !== undefined) updates.descriptionAr = parsed.data.descriptionAr;
@@ -306,6 +332,8 @@ router.patch("/groups/:id", requireAdmin, async (req, res): Promise<void> => {
.where(eq(userGroupsTable.groupId, id));
const currentMemberIds = users.map((u) => u.userId);
const currentAppIds = apps.map((a) => a.appId);
const currentRoleIds = roles.map((r) => r.roleId);
const membershipChanged = parsed.data.userIds !== undefined;
const grantsChanged =
parsed.data.appIds !== undefined || parsed.data.roleIds !== undefined;
@@ -320,6 +348,42 @@ router.patch("/groups/:id", requireAdmin, async (req, res): Promise<void> => {
await emitAppsChangedToUsers(Array.from(affected));
}
function diff(prev: number[], next: number[]) {
const prevSet = new Set(prev);
const nextSet = new Set(next);
return {
added: next.filter((n) => !prevSet.has(n)),
removed: prev.filter((n) => !nextSet.has(n)),
};
}
const fieldsChanged = Object.keys(updates).length > 0;
if (fieldsChanged || membershipChanged || grantsChanged) {
const meta: Record<string, unknown> = {
name: group.name,
};
if (fieldsChanged) {
meta.previousName = existing.name;
meta.changes = updates;
}
if (membershipChanged) {
meta.members = diff(previousMemberIds, currentMemberIds);
}
if (parsed.data.appIds !== undefined) {
meta.apps = diff(previousAppIds, currentAppIds);
}
if (parsed.data.roleIds !== undefined) {
meta.roles = diff(previousRoleIds, currentRoleIds);
}
await db.insert(auditLogsTable).values({
actorUserId: req.session.userId ?? null,
action: "group.update",
targetType: "group",
targetId: id,
metadata: meta,
});
}
res.json({
...serializeGroup(group),
appIds: apps.map((a) => a.appId),
@@ -365,25 +429,42 @@ router.post("/groups/:id/:kind/:targetId", requireAdmin, async (req, res): Promi
res.status(404).json({ error: `${kind.slice(0, -1)} not found` });
return;
}
let inserted: { groupId: number }[] = [];
if (kind === "users") {
await db
inserted = await db
.insert(userGroupsTable)
.values({ groupId: id, userId: targetId })
.onConflictDoNothing();
.onConflictDoNothing()
.returning({ groupId: userGroupsTable.groupId });
await emitAppsChangedToUsers([targetId]);
} else if (kind === "apps") {
await db
inserted = await db
.insert(groupAppsTable)
.values({ groupId: id, appId: targetId })
.onConflictDoNothing();
.onConflictDoNothing()
.returning({ groupId: groupAppsTable.groupId });
await emitAppsChangedToUsers(await getGroupMemberIds(id));
} else {
await db
inserted = await db
.insert(groupRolesTable)
.values({ groupId: id, roleId: targetId })
.onConflictDoNothing();
.onConflictDoNothing()
.returning({ groupId: groupRolesTable.groupId });
await emitAppsChangedToUsers(await getGroupMemberIds(id));
}
if (inserted.length > 0) {
const subject = kind === "users" ? "user" : kind === "apps" ? "app" : "role";
await db.insert(auditLogsTable).values({
actorUserId: req.session.userId ?? null,
action: `group.${subject}.add`,
targetType: "group",
targetId: id,
metadata: {
groupName: group.name,
[`${subject}Id`]: targetId,
},
});
}
res.sendStatus(204);
});
@@ -400,24 +481,41 @@ router.delete("/groups/:id/:kind/:targetId", requireAdmin, async (req, res): Pro
res.status(404).json({ error: "Group not found" });
return;
}
let removed: { groupId: number }[] = [];
if (kind === "users") {
await db
removed = await db
.delete(userGroupsTable)
.where(sql`${userGroupsTable.groupId} = ${id} and ${userGroupsTable.userId} = ${targetId}`);
.where(sql`${userGroupsTable.groupId} = ${id} and ${userGroupsTable.userId} = ${targetId}`)
.returning({ groupId: userGroupsTable.groupId });
await emitAppsChangedToUsers([targetId]);
} else if (kind === "apps") {
const memberIds = await getGroupMemberIds(id);
await db
removed = await db
.delete(groupAppsTable)
.where(sql`${groupAppsTable.groupId} = ${id} and ${groupAppsTable.appId} = ${targetId}`);
.where(sql`${groupAppsTable.groupId} = ${id} and ${groupAppsTable.appId} = ${targetId}`)
.returning({ groupId: groupAppsTable.groupId });
await emitAppsChangedToUsers(memberIds);
} else {
const memberIds = await getGroupMemberIds(id);
await db
removed = await db
.delete(groupRolesTable)
.where(sql`${groupRolesTable.groupId} = ${id} and ${groupRolesTable.roleId} = ${targetId}`);
.where(sql`${groupRolesTable.groupId} = ${id} and ${groupRolesTable.roleId} = ${targetId}`)
.returning({ groupId: groupRolesTable.groupId });
await emitAppsChangedToUsers(memberIds);
}
if (removed.length > 0) {
const subject = kind === "users" ? "user" : kind === "apps" ? "app" : "role";
await db.insert(auditLogsTable).values({
actorUserId: req.session.userId ?? null,
action: `group.${subject}.remove`,
targetType: "group",
targetId: id,
metadata: {
groupName: group.name,
[`${subject}Id`]: targetId,
},
});
}
res.sendStatus(204);
});
@@ -460,20 +558,19 @@ router.delete("/groups/:id", requireAdmin, async (req, res): Promise<void> => {
return;
}
await db.delete(groupsTable).where(eq(groupsTable.id, id));
if (isNonEmpty && force) {
await db.insert(auditLogsTable).values({
actorUserId: req.session.userId ?? null,
action: "group.force_delete",
targetType: "group",
targetId: id,
metadata: {
name: existing.name,
memberCount,
appCount,
roleCount,
},
});
}
await db.insert(auditLogsTable).values({
actorUserId: req.session.userId ?? null,
action: "group.delete",
targetType: "group",
targetId: id,
metadata: {
name: existing.name,
force,
memberCount,
appCount,
roleCount,
},
});
await emitAppsChangedToUsers(memberIds);
res.sendStatus(204);
});
artifacts/api-server/src/routes/roles.ts
+87 -1
View File
@@ -7,6 +7,7 @@ import {
groupRolesTable,
rolePermissionsTable,
permissionsTable,
auditLogsTable,
} from "@workspace/db";
import { requireAdmin } from "../middlewares/auth";
import {
@@ -81,6 +82,17 @@ router.post("/roles", requireAdmin, async (req, res): Promise<void> => {
descriptionEn: parsed.data.descriptionEn ?? null,
})
.returning();
await db.insert(auditLogsTable).values({
actorUserId: req.session.userId ?? null,
action: "role.create",
targetType: "role",
targetId: role.id,
metadata: {
name: role.name,
descriptionAr: role.descriptionAr,
descriptionEn: role.descriptionEn,
},
});
res.status(201).json(serializeRole(role));
});
@@ -129,6 +141,17 @@ router.patch("/roles/:id", requireAdmin, async (req, res): Promise<void> => {
if (parsed.data.descriptionEn !== undefined) updates.descriptionEn = parsed.data.descriptionEn;
if (Object.keys(updates).length > 0) {
await db.update(rolesTable).set(updates).where(eq(rolesTable.id, id));
await db.insert(auditLogsTable).values({
actorUserId: req.session.userId ?? null,
action: "role.update",
targetType: "role",
targetId: id,
metadata: {
previousName: existing.name,
name: updates.name ?? existing.name,
changes: updates,
},
});
}
const [role] = await db.select().from(rolesTable).where(eq(rolesTable.id, id));
res.json(serializeRole(role));
@@ -171,6 +194,17 @@ router.delete("/roles/:id", requireAdmin, async (req, res): Promise<void> => {
await tx.delete(rolePermissionsTable).where(eq(rolePermissionsTable.roleId, id));
await tx.delete(rolesTable).where(eq(rolesTable.id, id));
});
await db.insert(auditLogsTable).values({
actorUserId: req.session.userId ?? null,
action: "role.delete",
targetType: "role",
targetId: id,
metadata: {
name: existing.name,
descriptionAr: existing.descriptionAr,
descriptionEn: existing.descriptionEn,
},
});
res.sendStatus(204);
});
@@ -236,6 +270,12 @@ router.put("/roles/:id/permissions", requireAdmin, async (req, res): Promise<voi
return;
}
}
const previousIds = (
await db
.select({ permissionId: rolePermissionsTable.permissionId })
.from(rolePermissionsTable)
.where(eq(rolePermissionsTable.roleId, id))
).map((r) => r.permissionId);
await db.transaction(async (tx) => {
await tx.delete(rolePermissionsTable).where(eq(rolePermissionsTable.roleId, id));
if (requestedIds.length > 0) {
@@ -245,6 +285,23 @@ router.put("/roles/:id/permissions", requireAdmin, async (req, res): Promise<voi
}
});
await emitAppsChangedToRoleHolders(id);
const previousSet = new Set(previousIds);
const requestedSet = new Set(requestedIds);
const added = requestedIds.filter((p) => !previousSet.has(p));
const removed = previousIds.filter((p) => !requestedSet.has(p));
if (added.length > 0 || removed.length > 0) {
await db.insert(auditLogsTable).values({
actorUserId: req.session.userId ?? null,
action: "role.permissions.replace",
targetType: "role",
targetId: id,
metadata: {
roleName: role.name,
added,
removed,
},
});
}
const rows = await db
.select({
id: permissionsTable.id,
@@ -281,7 +338,10 @@ router.post("/roles/:id/permissions", requireAdmin, async (req, res): Promise<vo
.json({ error: "Cannot modify a system role's permissions", code: "system_role_permissions" });
return;
}
const [perm] = await db.select({ id: permissionsTable.id }).from(permissionsTable).where(eq(permissionsTable.id, permissionId));
const [perm] = await db
.select({ id: permissionsTable.id, name: permissionsTable.name })
.from(permissionsTable)
.where(eq(permissionsTable.id, permissionId));
if (!perm) {
res.status(404).json({ error: "Permission not found" });
return;
@@ -293,6 +353,17 @@ router.post("/roles/:id/permissions", requireAdmin, async (req, res): Promise<vo
if (existing.length === 0) {
await db.insert(rolePermissionsTable).values({ roleId: id, permissionId });
await emitAppsChangedToRoleHolders(id);
await db.insert(auditLogsTable).values({
actorUserId: req.session.userId ?? null,
action: "role.permission.add",
targetType: "role",
targetId: id,
metadata: {
roleName: role.name,
permissionId,
permissionName: perm.name,
},
});
}
res.status(201).json({ roleId: id, permissionId });
});
@@ -315,6 +386,10 @@ router.delete("/roles/:id/permissions/:permissionId", requireAdmin, async (req,
.json({ error: "Cannot modify a system role's permissions", code: "system_role_permissions" });
return;
}
const [perm] = await db
.select({ name: permissionsTable.name })
.from(permissionsTable)
.where(eq(permissionsTable.id, permissionId));
const deleted = await db
.delete(rolePermissionsTable)
.where(
@@ -323,6 +398,17 @@ router.delete("/roles/:id/permissions/:permissionId", requireAdmin, async (req,
.returning();
if (deleted.length > 0) {
await emitAppsChangedToRoleHolders(id);
await db.insert(auditLogsTable).values({
actorUserId: req.session.userId ?? null,
action: "role.permission.remove",
targetType: "role",
targetId: id,
metadata: {
roleName: role.name,
permissionId,
permissionName: perm?.name ?? null,
},
});
}
res.sendStatus(204);
});
artifacts/api-server/src/routes/settings.ts
+25 -1
View File
@@ -1,7 +1,7 @@
import { Router, type IRouter } from "express";
import { eq } from "drizzle-orm";
import { db } from "@workspace/db";
import { appSettingsTable } from "@workspace/db";
import { appSettingsTable, auditLogsTable } from "@workspace/db";
import { requireAdmin } from "../middlewares/auth";
import { UpdateAppSettingsBody } from "@workspace/api-zod";
@@ -31,11 +31,35 @@ router.patch("/settings", requireAdmin, async (req, res): Promise<void> => {
return;
}
await ensureSettingsRow();
const [previous] = await db
.select()
.from(appSettingsTable)
.where(eq(appSettingsTable.id, 1));
const [updated] = await db
.update(appSettingsTable)
.set(parsed.data)
.where(eq(appSettingsTable.id, 1))
.returning();
if (previous) {
const changes: Record<string, { from: unknown; to: unknown }> = {};
for (const [key, value] of Object.entries(parsed.data)) {
const prevValue = (previous as Record<string, unknown>)[key];
if (prevValue !== value) {
changes[key] = { from: prevValue ?? null, to: value ?? null };
}
}
if (Object.keys(changes).length > 0) {
await db.insert(auditLogsTable).values({
actorUserId: req.session.userId ?? null,
action: "settings.update",
targetType: "settings",
targetId: 1,
metadata: { changes },
});
}
}
res.json(updated);
});
artifacts/api-server/src/routes/users.ts
+14 -15
View File
@@ -531,21 +531,20 @@ router.delete("/users/:id", requireAdmin, async (req, res): Promise<void> => {
await tx.delete(usersTable).where(eq(usersTable.id, userId));
});
if (hasDeps && force) {
await db.insert(auditLogsTable).values({
actorUserId: req.session.userId ?? null,
action: "user.force_delete",
targetType: "user",
targetId: userId,
metadata: {
username: existing.username,
noteCount,
orderCount,
conversationCount,
messageCount,
},
});
}
await db.insert(auditLogsTable).values({
actorUserId: req.session.userId ?? null,
action: "user.delete",
targetType: "user",
targetId: userId,
metadata: {
username: existing.username,
email: existing.email,
force,
...(hasDeps
? { noteCount, orderCount, conversationCount, messageCount }
: {}),
},
});
res.sendStatus(204);
});