feat: production-grade RBAC with editor role, custom roles, API key scoping, and audit log (#89)

* feat(rbac): add editor role, 3 new permissions, ownership helper

* feat(rbac): add audit_log table, apiKeys.permissions column, editor role to schema

* feat(rbac): wire requirePermission into all routes, add editor role support

* refactor(rbac): replace ad-hoc role checks with permission-based ownership

* feat(rbac): add audit log DB writes + query endpoint

Dual-write audit events to stdout (existing) and SQLite audit_log table.
Add GET /api/v1/audit-log with pagination, action filter, and date range
filtering, gated behind audit:read permission.

* feat(rbac): add API key permission scoping with ceiling enforcement

* feat(rbac): add escalation prevention and last-admin protection

* feat(rbac): add editor role to UI, API key permission scoping in settings

* test(rbac): add full permission matrix integration test

* test(rbac): add editor role E2E tests

* feat(rbac): add custom roles with CRUD API and DB-backed permission lookup

* feat(rbac): add API key expiration

* feat(rbac): add roles management UI and API key expiration to settings

* feat(rbac): add audit log UI to settings

* fix: remove any cast in API key permission validation

* test(rbac): add unit tests for username validation rules

* test(rbac): add unit tests for effective permissions and ownership

* test(rbac): add comprehensive route permission matrix (all routes × all roles)

* test(rbac): add auth route edge case tests (login failures, session expiry, password side effects)

* test(rbac): add escalation prevention tests (register, update, self-demote, last-admin)

* test(rbac): add ownership enforcement tests (files, pipelines, editor access, cross-user isolation)

* test(rbac): add API key edge cases (name validation, delete behavior, key revocation)

* test(rbac): add audit log edge cases (all events, pagination clamping, structure)

* test(rbac): add custom roles edge case tests (validation, CRUD, functional permissions)

* test(rbac): add comprehensive E2E tests (roles UI, audit log, custom role, API key scoping)
This commit is contained in:
Ashim
2026-04-22 18:10:04 +08:00
committed by GitHub
parent 2d7a61c18f
commit 5a45bcbc8f
40 changed files with 5054 additions and 87 deletions
+155
View File
@@ -0,0 +1,155 @@
/**
* Unit tests for audit event mapping and actor extraction logic.
*
* Since deriveTargetType is not exported from audit.ts, we reproduce the
* mapping logic here so we can verify every event type maps correctly.
* Actor ID and username extraction logic is tested via the same rules
* used in auditLog().
*/
import { describe, expect, it } from "vitest";
// ---------------------------------------------------------------------------
// Reproduce the private deriveTargetType logic so we can test its mapping
// ---------------------------------------------------------------------------
type AuditEvent =
| "LOGIN_SUCCESS"
| "LOGIN_FAILED"
| "LOGOUT"
| "PASSWORD_CHANGED"
| "PASSWORD_RESET"
| "USER_CREATED"
| "USER_DELETED"
| "USER_UPDATED"
| "FILE_UPLOADED"
| "FILE_DELETED"
| "API_KEY_CREATED"
| "API_KEY_DELETED"
| "ROLE_CREATED"
| "ROLE_UPDATED"
| "ROLE_DELETED"
| "SETTINGS_UPDATED";
function deriveTargetType(event: AuditEvent): string | null {
if (
event.startsWith("USER_") ||
event.startsWith("LOGIN") ||
event.startsWith("PASSWORD") ||
event === "LOGOUT"
)
return "user";
if (event.startsWith("API_KEY")) return "api_key";
if (event.startsWith("FILE")) return "file";
if (event.startsWith("ROLE")) return "role";
if (event === "SETTINGS_UPDATED") return "setting";
return null;
}
// ---------------------------------------------------------------------------
// Reproduce the actor extraction logic from auditLog()
// ---------------------------------------------------------------------------
function extractActorId(details: Record<string, unknown>): string | null {
return (details.userId as string) ?? (details.adminId as string) ?? null;
}
function extractActorUsername(details: Record<string, unknown>): string {
return (
(details.username as string) ??
(details.newUsername as string) ??
"system"
);
}
// ---------------------------------------------------------------------------
// Tests
// ---------------------------------------------------------------------------
describe("audit helpers", () => {
describe("deriveTargetType", () => {
it.each<[AuditEvent, string]>([
["LOGIN_SUCCESS", "user"],
["LOGIN_FAILED", "user"],
["LOGOUT", "user"],
["PASSWORD_CHANGED", "user"],
["PASSWORD_RESET", "user"],
["USER_CREATED", "user"],
["USER_DELETED", "user"],
["USER_UPDATED", "user"],
])("%s -> %s", (event, expected) => {
expect(deriveTargetType(event)).toBe(expected);
});
it.each<[AuditEvent, string]>([
["FILE_UPLOADED", "file"],
["FILE_DELETED", "file"],
])("%s -> %s", (event, expected) => {
expect(deriveTargetType(event)).toBe(expected);
});
it.each<[AuditEvent, string]>([
["API_KEY_CREATED", "api_key"],
["API_KEY_DELETED", "api_key"],
])("%s -> %s", (event, expected) => {
expect(deriveTargetType(event)).toBe(expected);
});
it.each<[AuditEvent, string]>([
["ROLE_CREATED", "role"],
["ROLE_UPDATED", "role"],
["ROLE_DELETED", "role"],
])("%s -> %s", (event, expected) => {
expect(deriveTargetType(event)).toBe(expected);
});
it("SETTINGS_UPDATED -> setting", () => {
expect(deriveTargetType("SETTINGS_UPDATED")).toBe("setting");
});
});
describe("extractActorId", () => {
it("returns userId when present", () => {
expect(extractActorId({ userId: "u-123" })).toBe("u-123");
});
it("falls back to adminId when userId is absent", () => {
expect(extractActorId({ adminId: "a-456" })).toBe("a-456");
});
it("prefers userId over adminId when both are present", () => {
expect(extractActorId({ userId: "u-123", adminId: "a-456" })).toBe(
"u-123",
);
});
it("returns null when neither userId nor adminId is present", () => {
expect(extractActorId({})).toBeNull();
});
it("returns null for an empty details object", () => {
expect(extractActorId({})).toBeNull();
});
});
describe("extractActorUsername", () => {
it("returns username when present", () => {
expect(extractActorUsername({ username: "alice" })).toBe("alice");
});
it("falls back to newUsername when username is absent", () => {
expect(extractActorUsername({ newUsername: "bob" })).toBe("bob");
});
it("prefers username over newUsername when both are present", () => {
expect(
extractActorUsername({ username: "alice", newUsername: "bob" }),
).toBe("alice");
});
it('returns "system" when neither username nor newUsername is present', () => {
expect(extractActorUsername({})).toBe("system");
});
it('returns "system" for an empty details object', () => {
expect(extractActorUsername({})).toBe("system");
});
});
});
@@ -0,0 +1,276 @@
/**
* Unit tests for effective permissions logic.
*
* Covers hasEffectivePermission (role + API key scoping),
* getPermissions edge cases, and hasPermission edge cases.
*/
import type { Permission, Role } from "@ashim/shared";
import { describe, expect, it, vi } from "vitest";
// Mock the auth plugin to avoid transitively opening a SQLite connection
vi.mock("../../../apps/api/src/plugins/auth.js", () => ({
getAuthUser: () => null,
}));
import {
getPermissions,
hasEffectivePermission,
hasPermission,
} from "../../../apps/api/src/permissions.js";
import type { AuthUser } from "../../../apps/api/src/plugins/auth.js";
// ── Helpers ──────────────────────────────────────────────────────────
function makeUser(overrides: Partial<AuthUser> & { role: string }): AuthUser {
return {
id: "u-1",
username: "testuser",
...overrides,
};
}
// ── hasEffectivePermission ───────────────────────────────────────────
describe("hasEffectivePermission", () => {
describe("without API key scoping (apiKeyPermissions undefined)", () => {
it("admin can use any permission", () => {
const admin = makeUser({ role: "admin" });
const allAdmin = getPermissions("admin");
for (const perm of allAdmin) {
expect(hasEffectivePermission(admin, perm)).toBe(true);
}
});
it("editor can use all editor permissions", () => {
const editor = makeUser({ role: "editor" });
const editorPerms = getPermissions("editor");
for (const perm of editorPerms) {
expect(hasEffectivePermission(editor, perm)).toBe(true);
}
});
it("editor cannot use admin-only permissions", () => {
const editor = makeUser({ role: "editor" });
expect(hasEffectivePermission(editor, "users:manage")).toBe(false);
expect(hasEffectivePermission(editor, "settings:write")).toBe(false);
expect(hasEffectivePermission(editor, "teams:manage")).toBe(false);
expect(hasEffectivePermission(editor, "branding:manage")).toBe(false);
expect(hasEffectivePermission(editor, "features:manage")).toBe(false);
expect(hasEffectivePermission(editor, "system:health")).toBe(false);
expect(hasEffectivePermission(editor, "audit:read")).toBe(false);
});
it("user can use all user permissions", () => {
const user = makeUser({ role: "user" });
const userPerms = getPermissions("user");
for (const perm of userPerms) {
expect(hasEffectivePermission(user, perm)).toBe(true);
}
});
it("user cannot use editor or admin permissions", () => {
const user = makeUser({ role: "user" });
expect(hasEffectivePermission(user, "files:all")).toBe(false);
expect(hasEffectivePermission(user, "pipelines:all")).toBe(false);
expect(hasEffectivePermission(user, "users:manage")).toBe(false);
expect(hasEffectivePermission(user, "settings:write")).toBe(false);
});
it("unknown role has no effective permissions", () => {
const unknown = makeUser({ role: "ghost" });
expect(hasEffectivePermission(unknown, "tools:use")).toBe(false);
expect(hasEffectivePermission(unknown, "users:manage")).toBe(false);
});
});
describe("API key scoping restricts permissions", () => {
it("admin scoped to tools:use can only use tools:use", () => {
const admin = makeUser({
role: "admin",
apiKeyPermissions: ["tools:use"],
});
expect(hasEffectivePermission(admin, "tools:use")).toBe(true);
expect(hasEffectivePermission(admin, "users:manage")).toBe(false);
expect(hasEffectivePermission(admin, "files:all")).toBe(false);
});
it("editor scoped to files:own and tools:use only has those", () => {
const editor = makeUser({
role: "editor",
apiKeyPermissions: ["files:own", "tools:use"],
});
expect(hasEffectivePermission(editor, "files:own")).toBe(true);
expect(hasEffectivePermission(editor, "tools:use")).toBe(true);
expect(hasEffectivePermission(editor, "files:all")).toBe(false);
expect(hasEffectivePermission(editor, "settings:read")).toBe(false);
});
it("user scoped to settings:read only has that", () => {
const user = makeUser({
role: "user",
apiKeyPermissions: ["settings:read"],
});
expect(hasEffectivePermission(user, "settings:read")).toBe(true);
expect(hasEffectivePermission(user, "tools:use")).toBe(false);
expect(hasEffectivePermission(user, "files:own")).toBe(false);
});
});
describe("API key cannot grant permissions the role lacks", () => {
it("user with apiKeyPermissions including users:manage still denied", () => {
const user = makeUser({
role: "user",
apiKeyPermissions: ["tools:use", "users:manage"],
});
expect(hasEffectivePermission(user, "users:manage")).toBe(false);
// But role-granted permission that is also in the key works
expect(hasEffectivePermission(user, "tools:use")).toBe(true);
});
it("editor with apiKeyPermissions including settings:write still denied", () => {
const editor = makeUser({
role: "editor",
apiKeyPermissions: ["settings:write", "files:all"],
});
expect(hasEffectivePermission(editor, "settings:write")).toBe(false);
// files:all is in editor role, so it works
expect(hasEffectivePermission(editor, "files:all")).toBe(true);
});
it("unknown role gains nothing even with full apiKeyPermissions", () => {
const unknown = makeUser({
role: "nobody",
apiKeyPermissions: ["tools:use", "files:own", "users:manage", "settings:write"],
});
expect(hasEffectivePermission(unknown, "tools:use")).toBe(false);
expect(hasEffectivePermission(unknown, "users:manage")).toBe(false);
});
});
describe("empty apiKeyPermissions blocks everything", () => {
it("admin with empty array has no effective permissions", () => {
const admin = makeUser({ role: "admin", apiKeyPermissions: [] });
const allAdmin = getPermissions("admin");
for (const perm of allAdmin) {
expect(hasEffectivePermission(admin, perm)).toBe(false);
}
});
it("user with empty array has no effective permissions", () => {
const user = makeUser({ role: "user", apiKeyPermissions: [] });
expect(hasEffectivePermission(user, "tools:use")).toBe(false);
expect(hasEffectivePermission(user, "files:own")).toBe(false);
});
});
describe("undefined apiKeyPermissions inherits all role permissions", () => {
it("admin without apiKeyPermissions gets full admin access", () => {
const admin = makeUser({ role: "admin" });
expect(admin.apiKeyPermissions).toBeUndefined();
expect(hasEffectivePermission(admin, "users:manage")).toBe(true);
expect(hasEffectivePermission(admin, "audit:read")).toBe(true);
});
it("user without apiKeyPermissions gets full user access", () => {
const user = makeUser({ role: "user" });
expect(user.apiKeyPermissions).toBeUndefined();
expect(hasEffectivePermission(user, "tools:use")).toBe(true);
expect(hasEffectivePermission(user, "pipelines:own")).toBe(true);
});
});
});
// ── getPermissions ───────────────────────────────────────────────────
describe("getPermissions", () => {
describe("exact counts for built-in roles", () => {
it("admin has exactly 15 permissions", () => {
expect(getPermissions("admin")).toHaveLength(15);
});
it("editor has exactly 7 permissions", () => {
expect(getPermissions("editor")).toHaveLength(7);
});
it("user has exactly 5 permissions", () => {
expect(getPermissions("user")).toHaveLength(5);
});
});
describe("invalid and edge-case role names", () => {
it("empty string returns empty array", () => {
expect(getPermissions("")).toEqual([]);
});
it("null coerced to string returns empty array", () => {
expect(getPermissions(null as unknown as Role)).toEqual([]);
});
it("undefined coerced to string returns empty array", () => {
expect(getPermissions(undefined as unknown as Role)).toEqual([]);
});
it("case-sensitive: Admin (capitalized) returns empty array", () => {
expect(getPermissions("Admin" as Role)).toEqual([]);
});
it("case-sensitive: ADMIN (uppercase) returns empty array", () => {
expect(getPermissions("ADMIN" as Role)).toEqual([]);
});
it("case-sensitive: User (capitalized) returns empty array", () => {
expect(getPermissions("User" as Role)).toEqual([]);
});
it("whitespace-padded role name returns empty array", () => {
expect(getPermissions(" admin " as Role)).toEqual([]);
});
});
describe("role permission subsets", () => {
it("editor permissions are a subset of admin permissions", () => {
const adminPerms = getPermissions("admin");
const editorPerms = getPermissions("editor");
for (const perm of editorPerms) {
expect(adminPerms).toContain(perm);
}
});
it("user permissions are a subset of admin permissions", () => {
const adminPerms = getPermissions("admin");
const userPerms = getPermissions("user");
for (const perm of userPerms) {
expect(adminPerms).toContain(perm);
}
});
it("user permissions are a subset of editor permissions", () => {
const editorPerms = getPermissions("editor");
const userPerms = getPermissions("user");
for (const perm of userPerms) {
expect(editorPerms).toContain(perm);
}
});
});
});
// ── hasPermission edge cases ─────────────────────────────────────────
describe("hasPermission edge cases", () => {
it("returns false for a non-existent permission string", () => {
expect(hasPermission("admin", "fake:perm" as Permission)).toBe(false);
});
it("returns false for an empty string permission", () => {
expect(hasPermission("admin", "" as Permission)).toBe(false);
});
it("returns false for unknown role even with valid permission", () => {
expect(hasPermission("visitor" as Role, "tools:use")).toBe(false);
});
it("returns false for both unknown role and unknown permission", () => {
expect(hasPermission("visitor" as Role, "x:y" as Permission)).toBe(false);
});
});
+5 -2
View File
@@ -19,9 +19,9 @@ import { getPermissions, hasPermission } from "../../../apps/api/src/permissions
describe("permissions", () => {
describe("getPermissions", () => {
it("returns all 12 permissions for admin", () => {
it("returns all 15 permissions for admin", () => {
const perms = getPermissions("admin");
expect(perms).toHaveLength(12);
expect(perms).toHaveLength(15);
expect(perms).toContain("tools:use");
expect(perms).toContain("files:own");
expect(perms).toContain("files:all");
@@ -34,6 +34,9 @@ describe("permissions", () => {
expect(perms).toContain("users:manage");
expect(perms).toContain("teams:manage");
expect(perms).toContain("branding:manage");
expect(perms).toContain("features:manage");
expect(perms).toContain("system:health");
expect(perms).toContain("audit:read");
});
it("returns only basic permissions for user role", () => {
+53
View File
@@ -0,0 +1,53 @@
import { describe, expect, it } from "vitest";
import { getPermissions, hasPermission } from "../../../apps/api/src/permissions.js";
describe("role permissions", () => {
it("admin has all 15 permissions", () => {
const perms = getPermissions("admin");
expect(perms).toContain("tools:use");
expect(perms).toContain("files:all");
expect(perms).toContain("users:manage");
expect(perms).toContain("features:manage");
expect(perms).toContain("system:health");
expect(perms).toContain("audit:read");
expect(perms.length).toBe(15);
});
it("editor has collaborative but not admin permissions", () => {
const perms = getPermissions("editor");
expect(perms).toContain("tools:use");
expect(perms).toContain("files:own");
expect(perms).toContain("files:all");
expect(perms).toContain("pipelines:all");
expect(perms).toContain("settings:read");
expect(perms).not.toContain("users:manage");
expect(perms).not.toContain("settings:write");
expect(perms).not.toContain("teams:manage");
expect(perms).not.toContain("features:manage");
expect(perms).not.toContain("system:health");
expect(perms).not.toContain("audit:read");
});
it("user has basic permissions only", () => {
const perms = getPermissions("user");
expect(perms).toContain("tools:use");
expect(perms).toContain("files:own");
expect(perms).toContain("apikeys:own");
expect(perms).toContain("pipelines:own");
expect(perms).toContain("settings:read");
expect(perms).not.toContain("files:all");
expect(perms).not.toContain("users:manage");
});
it("unknown role returns empty permissions", () => {
const perms = getPermissions("bogus" as any);
expect(perms).toEqual([]);
});
it("hasPermission checks correctly", () => {
expect(hasPermission("admin", "users:manage")).toBe(true);
expect(hasPermission("editor", "users:manage")).toBe(false);
expect(hasPermission("user", "tools:use")).toBe(true);
expect(hasPermission("user", "files:all")).toBe(false);
});
});
@@ -0,0 +1,87 @@
/**
* Unit tests for username validation rules.
*
* The validateUsername function is not exported from auth.ts,
* so we reproduce its logic here to test the rules directly.
*/
import { describe, expect, it } from "vitest";
/**
* Reproduce the validateUsername logic from apps/api/src/plugins/auth.ts
* so we can unit-test the rules without importing the module (which
* transitively opens a SQLite connection).
*/
function validateUsername(username: string): string | null {
if (username.length < 3 || username.length > 50) {
return "Username must be between 3 and 50 characters";
}
if (!/^[a-zA-Z0-9_.-]+$/.test(username)) {
return "Username can only contain letters, numbers, dots, hyphens, and underscores";
}
return null;
}
describe("validateUsername", () => {
describe("valid usernames", () => {
it.each([
["alice", "lowercase letters"],
["bob123", "letters and digits"],
["user.name", "dots"],
["user-name", "hyphens"],
["user_name", "underscores"],
["abc", "minimum length (3)"],
["a".repeat(50), "maximum length (50)"],
["A.B-C_D", "mixed separators and uppercase"],
["123", "digits only"],
])("accepts %s (%s)", (username) => {
expect(validateUsername(username)).toBeNull();
});
});
describe("too short", () => {
it.each([
["ab", "2 characters"],
["a", "1 character"],
["", "empty string"],
])("rejects %s (%s)", (username) => {
expect(validateUsername(username)).toBe("Username must be between 3 and 50 characters");
});
});
describe("too long", () => {
it("rejects a 51-character username", () => {
expect(validateUsername("a".repeat(51))).toBe("Username must be between 3 and 50 characters");
});
});
describe("invalid characters", () => {
it.each([
["has space", "space"],
["user@name", "@ symbol"],
["user#name", "# symbol"],
["user/name", "forward slash"],
["<script>", "angle brackets"],
["user\nname", "newline"],
["user!name", "exclamation mark"],
["user name", "tab character"],
])("rejects '%s' (%s)", (username) => {
expect(validateUsername(username)).toBe(
"Username can only contain letters, numbers, dots, hyphens, and underscores",
);
});
});
describe("unicode characters", () => {
it.each([
["élève", "accented Latin letters"],
["用户名", "CJK characters"],
["пользователь", "Cyrillic characters"],
["üser", "umlaut"],
])("rejects '%s' (%s)", (username) => {
expect(validateUsername(username)).toBe(
"Username can only contain letters, numbers, dots, hyphens, and underscores",
);
});
});
});