Skip to content

Commit 54e9e8a

Browse files
committed
Tighten API key authorization
1 parent a71e488 commit 54e9e8a

8 files changed

Lines changed: 324 additions & 15 deletions

File tree

README.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -333,6 +333,7 @@ Personal API keys for automation clients:
333333
- Create and manage keys from the authenticated account API: `GET /auth/api-keys`, `POST /auth/api-keys`, and `DELETE /auth/api-keys/:id`.
334334
- `POST /auth/api-keys` accepts `{ "name": "Obsidian" }` and returns the plaintext token once. Store it immediately; ExcaliDash stores only a hash and metadata.
335335
- Automation clients such as the Obsidian plugin should call normal drawing and collection APIs with `Authorization: Bearer <key>`, for example `Authorization: Bearer exd_...`.
336+
- API keys are scoped automation credentials for `/drawings` and `/collections` only. They cannot manage account settings, create/revoke API keys, call admin endpoints, or use import/export APIs.
336337
- API-key Bearer requests without browser `Origin`/`Referer` headers do not require CSRF tokens. Cookie-authenticated browser requests keep the existing CSRF behavior.
337338

338339
</details>

backend/src/__tests__/api-key-auth.integration.ts

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ describe("API key authentication", () => {
1111
let userId: string;
1212
let apiKeyId: string;
1313
let apiKeyToken: string;
14+
let adminApiKeyToken: string;
1415

1516
beforeAll(async () => {
1617
setupTestDb();
@@ -50,6 +51,29 @@ describe("API key authentication", () => {
5051
select: { id: true },
5152
});
5253
apiKeyId = apiKey.id;
54+
55+
const adminUser = await prisma.user.create({
56+
data: {
57+
email: "api-key-admin@test.local",
58+
passwordHash,
59+
name: "API Key Admin",
60+
role: "ADMIN",
61+
isActive: true,
62+
},
63+
select: { id: true },
64+
});
65+
const adminGenerated = generateApiKey();
66+
adminApiKeyToken = adminGenerated.token;
67+
await prisma.apiKey.create({
68+
data: {
69+
userId: adminUser.id,
70+
name: "Admin automation",
71+
keyId: adminGenerated.keyId,
72+
tokenHash: adminGenerated.tokenHash,
73+
prefix: adminGenerated.prefix,
74+
scopes: serializeApiKeyScopes(),
75+
},
76+
});
5377
});
5478

5579
afterAll(async () => {
@@ -70,6 +94,35 @@ describe("API key authentication", () => {
7094
expect(stored?.lastUsedAt).toBeInstanceOf(Date);
7195
});
7296

97+
it("accepts API key bearer auth for allowed read routes", async () => {
98+
const collectionsResponse = await request(app)
99+
.get("/collections")
100+
.set("Authorization", `Bearer ${apiKeyToken}`);
101+
const drawingsResponse = await request(app)
102+
.get("/drawings")
103+
.set("Authorization", `Bearer ${apiKeyToken}`);
104+
105+
expect(collectionsResponse.status).toBe(200);
106+
expect(drawingsResponse.status).toBe(200);
107+
});
108+
109+
it("rejects API key management with API key auth", async () => {
110+
const response = await request(app)
111+
.get("/auth/api-keys")
112+
.set("Authorization", `Bearer ${apiKeyToken}`);
113+
114+
expect(response.status).toBe(403);
115+
});
116+
117+
it("rejects admin actions with admin-owned API key auth", async () => {
118+
const response = await request(app)
119+
.get("/auth/users")
120+
.set("Authorization", `Bearer ${adminApiKeyToken}`)
121+
.send();
122+
123+
expect(response.status).toBe(403);
124+
});
125+
73126
it("stores only hashed API keys and metadata", async () => {
74127
const stored = await prisma.apiKey.findUnique({ where: { id: apiKeyId } });
75128

backend/src/auth/apiKeys.test.ts

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
import { describe, expect, it } from "vitest";
2+
import { API_KEY_PREFIX, extractApiKeyId } from "./apiKeys";
3+
4+
describe("API key helpers", () => {
5+
it("extracts generated-format key IDs that contain underscores", () => {
6+
const keyId = "abcd_efgh_123456";
7+
const token = `${API_KEY_PREFIX}${keyId}_secret_value`;
8+
9+
expect(extractApiKeyId(token)).toBe(keyId);
10+
});
11+
});

backend/src/auth/apiKeys.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ export const isApiKeyToken = (token: string): boolean => token.startsWith(API_KE
3434
export const extractApiKeyId = (token: string): string | null => {
3535
if (!isApiKeyToken(token)) return null;
3636
const withoutPrefix = token.slice(API_KEY_PREFIX.length);
37-
const separatorIndex = withoutPrefix.indexOf("_");
37+
const separatorIndex = withoutPrefix[16] === "_" ? 16 : withoutPrefix.indexOf("_");
3838
if (separatorIndex <= 0) return null;
3939
const keyId = withoutPrefix.slice(0, separatorIndex);
4040
return /^[A-Za-z0-9_-]{8,64}$/.test(keyId) ? keyId : null;

backend/src/middleware/auth.test.ts

Lines changed: 118 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
44
import { config } from "../config";
55
import { createAuthMiddleware } from "./auth";
66
import { BOOTSTRAP_USER_ID } from "../auth/authMode";
7-
import { generateApiKey } from "../auth/apiKeys";
7+
import { generateApiKey, serializeApiKeyScopes } from "../auth/apiKeys";
88

99
const createRequest = (overrides?: Partial<Request>): Request =>
1010
({
@@ -203,6 +203,7 @@ describe("auth middleware", () => {
203203
prisma.apiKey.findUnique.mockResolvedValue({
204204
id: "api-key-1",
205205
tokenHash: generated.tokenHash,
206+
scopes: serializeApiKeyScopes(),
206207
revokedAt: null,
207208
user: {
208209
id: "user-1",
@@ -240,6 +241,122 @@ describe("auth middleware", () => {
240241
});
241242
});
242243

244+
it("allows valid API key auth when lastUsedAt update fails", async () => {
245+
const { prisma, authModeService } = createDeps();
246+
authModeService.getAuthEnabled.mockResolvedValue(true);
247+
const generated = generateApiKey();
248+
prisma.apiKey.findUnique.mockResolvedValue({
249+
id: "api-key-1",
250+
tokenHash: generated.tokenHash,
251+
scopes: serializeApiKeyScopes(),
252+
revokedAt: null,
253+
user: {
254+
id: "user-1",
255+
username: "user1",
256+
email: "user-1@test.local",
257+
name: "User One",
258+
role: "USER",
259+
mustResetPassword: false,
260+
isActive: true,
261+
},
262+
});
263+
prisma.apiKey.update.mockRejectedValue(new Error("write failed"));
264+
const warn = vi.spyOn(console, "warn").mockImplementation(() => undefined);
265+
const { requireAuth } = createAuthMiddleware({ prisma, authModeService });
266+
267+
const req = createRequest({
268+
method: "GET",
269+
originalUrl: "/drawings",
270+
headers: {
271+
authorization: `Bearer ${generated.token}`,
272+
},
273+
});
274+
const res = createResponse();
275+
const next = vi.fn() as NextFunction;
276+
277+
await requireAuth(req, res, next);
278+
279+
expect(next).toHaveBeenCalledTimes(1);
280+
expect(res.status).not.toHaveBeenCalled();
281+
warn.mockRestore();
282+
});
283+
284+
it("rejects API keys for routes outside drawings and collections", async () => {
285+
const { prisma, authModeService } = createDeps();
286+
authModeService.getAuthEnabled.mockResolvedValue(true);
287+
const generated = generateApiKey();
288+
prisma.apiKey.findUnique.mockResolvedValue({
289+
id: "api-key-1",
290+
tokenHash: generated.tokenHash,
291+
scopes: serializeApiKeyScopes(),
292+
revokedAt: null,
293+
user: {
294+
id: "admin-1",
295+
username: "admin",
296+
email: "admin@test.local",
297+
name: "Admin",
298+
role: "ADMIN",
299+
mustResetPassword: false,
300+
isActive: true,
301+
},
302+
});
303+
prisma.apiKey.update.mockResolvedValue({});
304+
const { requireAuth } = createAuthMiddleware({ prisma, authModeService });
305+
306+
const req = createRequest({
307+
method: "GET",
308+
originalUrl: "/auth/api-keys",
309+
headers: {
310+
authorization: `Bearer ${generated.token}`,
311+
},
312+
});
313+
const res = createResponse();
314+
const next = vi.fn() as NextFunction;
315+
316+
await requireAuth(req, res, next);
317+
318+
expect(res.status).toHaveBeenCalledWith(403);
319+
expect(next).not.toHaveBeenCalled();
320+
});
321+
322+
it("rejects API keys missing the required resource scope", async () => {
323+
const { prisma, authModeService } = createDeps();
324+
authModeService.getAuthEnabled.mockResolvedValue(true);
325+
const generated = generateApiKey();
326+
prisma.apiKey.findUnique.mockResolvedValue({
327+
id: "api-key-1",
328+
tokenHash: generated.tokenHash,
329+
scopes: serializeApiKeyScopes(["drawings:read"]),
330+
revokedAt: null,
331+
user: {
332+
id: "user-1",
333+
username: "user1",
334+
email: "user-1@test.local",
335+
name: "User One",
336+
role: "USER",
337+
mustResetPassword: false,
338+
isActive: true,
339+
},
340+
});
341+
prisma.apiKey.update.mockResolvedValue({});
342+
const { requireAuth } = createAuthMiddleware({ prisma, authModeService });
343+
344+
const req = createRequest({
345+
method: "POST",
346+
originalUrl: "/drawings",
347+
headers: {
348+
authorization: `Bearer ${generated.token}`,
349+
},
350+
});
351+
const res = createResponse();
352+
const next = vi.fn() as NextFunction;
353+
354+
await requireAuth(req, res, next);
355+
356+
expect(res.status).toHaveBeenCalledWith(403);
357+
expect(next).not.toHaveBeenCalled();
358+
});
359+
243360
it("rejects revoked API keys", async () => {
244361
const { prisma, authModeService } = createDeps();
245362
authModeService.getAuthEnabled.mockResolvedValue(true);

backend/src/middleware/auth.ts

Lines changed: 53 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import {
1313
apiKeyHashMatches,
1414
extractApiKeyId,
1515
isApiKeyToken,
16+
parseApiKeyScopes,
1617
} from "../auth/apiKeys";
1718

1819
declare global {
@@ -123,6 +124,36 @@ const isAllowedWhileMustResetPassword = (req: Request): boolean => {
123124
return false;
124125
};
125126

127+
const getRequiredApiKeyScope = (req: Request): string | null => {
128+
const path = normalizeRequestPath(req);
129+
const resource = path === "/drawings" || path.startsWith("/drawings/")
130+
? "drawings"
131+
: path === "/collections" || path.startsWith("/collections/")
132+
? "collections"
133+
: null;
134+
if (!resource) return null;
135+
136+
const access = req.method === "GET" || req.method === "HEAD" ? "read" : "write";
137+
return `${resource}:${access}`;
138+
};
139+
140+
const authorizeApiKeyRequest = (
141+
req: Request,
142+
res: Response,
143+
scopes: string[],
144+
): boolean => {
145+
const requiredScope = getRequiredApiKeyScope(req);
146+
if (requiredScope && scopes.includes(requiredScope)) {
147+
return true;
148+
}
149+
150+
res.status(403).json({
151+
error: "Forbidden",
152+
message: "API key is not authorized for this route",
153+
});
154+
return false;
155+
};
156+
126157
export type AuthMiddlewareDeps = {
127158
prisma: PrismaClient;
128159
authModeService: AuthModeService;
@@ -169,12 +200,16 @@ export const createAuthMiddleware = ({
169200
if (!apiKeyHashMatches(token, apiKey.tokenHash)) return null;
170201
if (!apiKey.user.isActive) return null;
171202

172-
await prisma.apiKey.update({
173-
where: { id: apiKey.id },
174-
data: { lastUsedAt: new Date() },
175-
});
203+
try {
204+
await prisma.apiKey.update({
205+
where: { id: apiKey.id },
206+
data: { lastUsedAt: new Date() },
207+
});
208+
} catch (error) {
209+
console.warn("Failed to update API key lastUsedAt:", error);
210+
}
176211

177-
return apiKey.user;
212+
return { user: apiKey.user, scopes: parseApiKeyScopes(apiKey.scopes) };
178213
};
179214

180215
const shouldReconcileOidcRole = async (
@@ -283,14 +318,19 @@ export const createAuthMiddleware = ({
283318

284319
if (extracted.source === "bearer" && isApiKeyToken(extracted.token)) {
285320
try {
286-
const user = await authenticateApiKey(extracted.token);
287-
if (!user) {
321+
const result = await authenticateApiKey(extracted.token);
322+
if (!result) {
288323
res.status(401).json({
289324
error: "Unauthorized",
290325
message: "Invalid or revoked API key",
291326
});
292327
return;
293328
}
329+
const { user, scopes } = result;
330+
331+
if (!authorizeApiKeyRequest(req, res, scopes)) {
332+
return;
333+
}
294334

295335
if (user.mustResetPassword && !isAllowedWhileMustResetPassword(req)) {
296336
res.status(403).json({
@@ -419,8 +459,12 @@ export const createAuthMiddleware = ({
419459

420460
if (extracted.source === "bearer" && isApiKeyToken(extracted.token)) {
421461
try {
422-
const user = await authenticateApiKey(extracted.token);
423-
if (user) {
462+
const result = await authenticateApiKey(extracted.token);
463+
if (result) {
464+
if (!authorizeApiKeyRequest(req, res, result.scopes)) {
465+
return;
466+
}
467+
const { user } = result;
424468
req.user = {
425469
id: user.id,
426470
username: user.username,

0 commit comments

Comments
 (0)