fix: revoke API keys on password rotation and audit key owners

This commit is contained in:
TIANYAO ZHANG committed 2026-10-03 17:17:34 +08:00
1 parent 79fb8443be
commit b9c79c8138
7 files changed
+110 -20

No files matched your search

+13 -3
View File
@@ -5,7 +5,7 @@ import request from "supertest";
import { createDatabase, type BotDatabase } from "../../data/database.js";
import { createUserStore } from "../../data/users.js";
import { createSessionStore, type SessionStore } from "../../data/sessions.js";
import { createAuditStore } from "../../data/audit.js";
import { createAuditStore, type AuditStore } from "../../data/audit.js";
import { createApiKeyStore, MAX_API_KEYS_PER_USER, type ApiKeyStore } from "../../data/api-keys.js";
import { createPermissionStore } from "../../data/permissions.js";
import { createRequireAuth } from "../middleware/requireAuth.js";
@@ -17,6 +17,7 @@ describe("api-keys router", () => {
let app: express.Express;
let sessions: SessionStore;
let apiKeys: ApiKeyStore;
let audit: AuditStore;
let adminId: string;
let memberId: string;
let adminToken: string;
@@ -26,7 +27,7 @@ describe("api-keys router", () => {
botDb = createDatabase(":memory:");
const users = createUserStore(botDb.db);
sessions = createSessionStore(botDb.db);
const audit = createAuditStore(botDb.db);
audit = createAuditStore(botDb.db);
const permissions = createPermissionStore(botDb.db);
apiKeys = createApiKeyStore(botDb.db);
const admin = await users.createUser("alice", "pw-alice", "admin");
@@ -112,11 +113,20 @@ describe("api-keys router", () => {
expect(apiKeys.listForUser(adminId)).toHaveLength(1);
});
it("an admin can delete another user's key", async () => {
it("an admin revoking another user's key audits that key's owner", async () => {
const { key } = apiKeys.create(memberId, "member-key")!;
const res = await asAdmin().delete(`/api/keys/${key.id}`);
expect(res.status).toBe(200);
expect(apiKeys.listForUser(memberId)).toHaveLength(0);
expect(audit.list(10, 0)).toEqual([
expect.objectContaining({
actorId: adminId,
actorUsername: "alice",
targetUserId: memberId,
targetUsername: "bob",
action: "api_key.deleted",
}),
]);
});
it("admin can list all keys with ?all=1, members cannot", async () => {
+6 -9
View File
@@ -7,8 +7,8 @@ import type { Logger } from "../../logger.js";
/**
* API-key management (list / create / revoke), mounted at /api/keys.
* Only interactive sessions may manage keys: a leaked key must never be able
* to mint its own replacements.
* Only interactive sessions may call these endpoints. Administrator keys
* retain user-management authority through /api/users.
*/
export function createApiKeysRouter(apiKeys: ApiKeyStore, audit: AuditStore, logger: Logger): Router {
const router = Router();
@@ -63,11 +63,8 @@ export function createApiKeysRouter(apiKeys: ApiKeyStore, audit: AuditStore, log
// DELETE /api/keys/:id — revoke; members only their own, admins any.
router.delete("/:id", (req, res) => {
const user = req.user!;
const ok =
user.role === "admin"
? apiKeys.delete(req.params.id)
: apiKeys.delete(req.params.id, user.id);
if (!ok) {
const key = apiKeys.findById(req.params.id);
if (!key || !apiKeys.delete(key.id, user.role === "admin" ? undefined : user.id)) {
res.status(404).json({ error: "API key not found" });
return;
}
@@ -75,8 +72,8 @@ export function createApiKeysRouter(apiKeys: ApiKeyStore, audit: AuditStore, log
audit.record({
actorId: user.id,
actorUsername: user.username,
targetUserId: user.id,
targetUsername: user.username,
targetUserId: key.userId,
targetUsername: key.username,
action: "api_key.deleted",
});
} catch (auditErr) {
+66 -2
View File
@@ -6,6 +6,7 @@ import pino from "pino";
import { createDatabase, type BotDatabase } from "../../data/database.js";
import { createUserStore, type UserStore } from "../../data/users.js";
import { createSessionStore, type SessionStore } from "../../data/sessions.js";
import { createApiKeyStore, type ApiKeyStore } from "../../data/api-keys.js";
import { createAuditStore } from "../../data/audit.js";
import { createPermissionStore } from "../../data/permissions.js";
import { getDefaultConfig, type GuestModeConfig } from "../../data/config.js";
@@ -13,7 +14,7 @@ import type { GuestPermissions, BotAccess } from "../../data/permissions.js";
import { createSessionRouter } from "./session.js";
import { SESSION_COOKIE_NAME } from "../auth/validateSession.js";
function makeApp(botDb: BotDatabase, users: UserStore, sessions: SessionStore) {
function makeApp(botDb: BotDatabase, users: UserStore, sessions: SessionStore, apiKeys?: ApiKeyStore) {
const app = express();
app.use(express.json());
app.use(cookieParser());
@@ -27,7 +28,8 @@ function makeApp(botDb: BotDatabase, users: UserStore, sessions: SessionStore) {
audit,
pino({ level: "silent" }),
permissions,
() => getDefaultConfig().guestMode
() => getDefaultConfig().guestMode,
apiKeys
)
);
return app;
@@ -176,6 +178,68 @@ describe("session router", () => {
}, 20000);
});
describe("session router — API key revocation", () => {
let botDb: BotDatabase;
let users: UserStore;
let sessions: SessionStore;
let apiKeys: ApiKeyStore;
let app: express.Express;
let userId: string;
let currentCookie: string;
let rawKey: string;
beforeEach(async () => {
botDb = createDatabase(":memory:");
users = createUserStore(botDb.db);
sessions = createSessionStore(botDb.db);
apiKeys = createApiKeyStore(botDb.db);
const member = await users.createUser("alice", "old-password", "member");
userId = member.id;
currentCookie = `${SESSION_COOKIE_NAME}=${sessions.createSession(userId).token}`;
rawKey = apiKeys.create(userId, "integration")!.rawKey;
app = makeApp(botDb, users, sessions, apiKeys);
});
afterEach(() => botDb.close());
it("successful password change revokes all owned keys and preserves only the active browser session", async () => {
const secondKey = apiKeys.create(userId, "another-integration")!.rawKey;
const otherSession = `${SESSION_COOKIE_NAME}=${sessions.createSession(userId).token}`;
const otherUser = await users.createUser("bob", "other-password", "member");
const otherUserKey = apiKeys.create(otherUser.id, "other-user-integration")!.rawKey;
const changed = await request(app).post("/api/session/change-password")
.set("Cookie", currentCookie)
.send({ oldPassword: "old-password", newPassword: "new-password" });
expect(changed.status).toBe(204);
expect(apiKeys.validateAndTouch(rawKey)).toBeNull();
expect(apiKeys.validateAndTouch(secondKey)).toBeNull();
expect(apiKeys.listForUser(userId)).toEqual([]);
expect(apiKeys.validateAndTouch(otherUserKey)?.userId).toBe(otherUser.id);
expect((await request(app).get("/api/session/me").set("Cookie", currentCookie)).status).toBe(200);
expect((await request(app).get("/api/session/me").set("Cookie", otherSession)).status).toBe(401);
}, 20_000);
it.each([
{ oldPassword: "wrong-password", newPassword: "new-password", status: 401 },
{ oldPassword: "old-password", newPassword: "short", status: 400 },
])("failed password change ($status) leaves API keys valid", async ({ oldPassword, newPassword, status }) => {
const changed = await request(app).post("/api/session/change-password")
.set("Cookie", currentCookie)
.send({ oldPassword, newPassword });
expect(changed.status).toBe(status);
expect(apiKeys.validateAndTouch(rawKey)?.userId).toBe(userId);
expect((await request(app).get("/api/session/me").set("Cookie", currentCookie)).status).toBe(200);
});
it("unauthenticated password change leaves API keys valid", async () => {
const changed = await request(app).post("/api/session/change-password")
.send({ oldPassword: "old-password", newPassword: "new-password" });
expect(changed.status).toBe(401);
expect(apiKeys.validateAndTouch(rawKey)?.userId).toBe(userId);
});
});
describe("session router — guest mode", () => {
let botDb: BotDatabase;
+4 -1
View File
@@ -3,6 +3,7 @@ import type { Request, Response, NextFunction } from "express";
import type { Logger } from "../../logger.js";
import type { UserStore } from "../../data/users.js";
import type { SessionStore } from "../../data/sessions.js";
import type { ApiKeyStore } from "../../data/api-keys.js";
import type { AuditStore } from "../../data/audit.js";
import { resolvePermissionContext, type PermissionStore } from "../../data/permissions.js";
import { SESSION_TTL_MS, GUEST_SESSION_TTL_MS } from "../../data/sessions.js";
@@ -54,7 +55,8 @@ export function createSessionRouter(
audit: AuditStore,
logger: Logger,
permissions: PermissionStore,
getGuestConfig: () => GuestModeConfig
getGuestConfig: () => GuestModeConfig,
apiKeys?: ApiKeyStore
): Router {
const router = Router();
@@ -202,6 +204,7 @@ export function createSessionRouter(
await users.changePassword(u.id, newPassword);
const currentToken = parseTokenFromCookie(req.headers.cookie);
sessions.deleteAllForUser(u.id, currentToken ?? undefined);
apiKeys?.deleteAllForUser(u.id);
try {
audit.record({
actorId: u.id, actorUsername: u.username,
+1 -1
View File
@@ -131,7 +131,7 @@ export function createWebServer(options: WebServerOptions): WebServer {
app.use("/api/session/login", loginLimit);
app.use("/api/session/setup", setupLimit);
app.use("/api/session", createSessionRouter(users, sessions, audit, logger, permissions, () => options.config.guestMode));
app.use("/api/session", createSessionRouter(users, sessions, audit, logger, permissions, () => options.config.guestMode, apiKeys));
// ─── Gates for everything else under /api ───────────────────────────────
const requireAuth = createRequireAuth(sessions, permissions, () => options.config.guestMode, apiKeys);