Skip to content

Commit efe9746

Browse files
committed
Hardening batch: auth observability, diagnostics endpoint, unified API errors, backend tests
1 parent 789d576 commit efe9746

14 files changed

Lines changed: 229 additions & 65 deletions

File tree

config/vitest.config.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ import { defineConfig } from "vitest/config";
33
export default defineConfig({
44
test: {
55
environment: "node",
6-
include: ["../src/**/*.test.ts"],
6+
include: ["src/**/*.test.ts", "functions/**/*.test.ts"],
77
coverage: {
88
provider: "v8",
99
reporter: ["text", "html"],

docs/BACKLOG.md

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,8 @@ State: stabilization pass (no net-new product features unless explicitly approve
2222
- [ ] Instead of showing actions on the users in the admin panel, show a simple list of users and make open the profile popover when clicking the names. This should be the same popover that appears anywhere else. Admin gets extra moderation buttons.
2323
- [ ] Access request note should be a one time thing for the users. Admins should still be able to see it even after approval, but it shouldn't be an editable field.
2424
- [ ] What does the reject button currently do?
25+
- [ ] Show profile pictures to other users when you click open the user popover. Also show a small one next to any user name in the UI
26+
- [ ] User should be able to select if
2527

2628
## Active stabilization backlog
2729

@@ -33,8 +35,9 @@ State: stabilization pass (no net-new product features unless explicitly approve
3335
- [x] Clarify pending-account UX text and flow end-to-end
3436
- [ ] Restrict sign-ups and approval transitions with explicit lifecycle states
3537
- [ ] Add dedicated auth/permission tests for critical flows
38+
- Progress: baseline tests added for auth source resolution and error mapping; endpoint permission matrix still pending.
3639
- [ ] Add dedicated identity reconciliation tests + audit logging coverage
37-
- [ ] Add observability for Cloudflare Access auth header/JWT variants
40+
- [x] Add observability for Cloudflare Access auth header/JWT variants
3841

3942
### Data and storage safety
4043
- [ ] Replace avatar data URLs in D1 with object storage flow (R2) + thumbnails
@@ -54,6 +57,7 @@ State: stabilization pass (no net-new product features unless explicitly approve
5457
- [ ] Clean sidebar information density and progressive disclosure
5558
- [ ] Unify labels/buttons across libraries and managers
5659
- [ ] Standardize error messages across endpoints and UI surfaces
60+
- Progress: backend endpoints now use centralized error normalization and status mapping; UI surface pass still pending.
5761

5862
### Simulation quality clarity
5963
- [ ] Improve explanatory info for FSPL / TwoRay / ITM and defaults
@@ -73,10 +77,10 @@ State: stabilization pass (no net-new product features unless explicitly approve
7377
- Path: add API integration tests for self-role block, pending-user lock, approval/revocation, admin-only mutations, cross-user denial, and delete safeguards.
7478
- [ ] Identity reconciliation hardening
7579
- Path: define deterministic merge/link matrix (idp subject, verified email, legacy local email), add immutable audit events for link/merge decisions, and test every branch.
76-
- [ ] Cloudflare Access auth observability
80+
- [x] Cloudflare Access auth observability
7781
- Path: add structured auth logs with reason codes for 401/403, include parsed identity source and header/JWT shape, and expose admin diagnostics endpoint/view.
7882

7983
## Next batch plan
80-
1. Pending-account UX clarity pass + wording consistency pass.
81-
2. Notification center expansion (history, filtering, moderation handoff).
84+
1. Auth/permission endpoint matrix tests (admin/user/pending/revoked/deleted sessions).
85+
2. Runtime migration extraction from request path into explicit migration step.
8286
3. Admin utilities scope draft replacing manual SQL operations.

functions/_lib/auth.test.ts

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
import { describe, expect, it } from "vitest";
2+
import { inspectAuthRequest, verifyAuth } from "./auth";
3+
import type { Env } from "./types";
4+
5+
const makeEnv = (overrides?: Partial<Env>): Env =>
6+
({
7+
DB: {} as D1Database,
8+
...overrides,
9+
}) as Env;
10+
11+
describe("auth inspection", () => {
12+
it("detects available Cloudflare auth headers", () => {
13+
const request = new Request("https://example.test/api/me", {
14+
headers: {
15+
"Cf-Access-Authenticated-User-Email": "user@example.com",
16+
"Cf-Access-Authenticated-User-Id": "sub-123",
17+
},
18+
});
19+
const inspected = inspectAuthRequest(request);
20+
expect(inspected.hasEmailHeader).toBe(true);
21+
expect(inspected.hasUserIdHeader).toBe(true);
22+
expect(inspected.hasJwtAssertion).toBe(false);
23+
});
24+
});
25+
26+
describe("verifyAuth", () => {
27+
it("accepts header-based auth when user headers are present", async () => {
28+
const request = new Request("https://example.test/api/me", {
29+
headers: {
30+
"Cf-Access-Authenticated-User-Email": "user@example.com",
31+
},
32+
});
33+
const auth = await verifyAuth(request, makeEnv());
34+
expect(auth?.userId).toBe("user@example.com");
35+
expect(auth?.source).toBe("headers");
36+
});
37+
38+
it("falls back to insecure dev auth when enabled", async () => {
39+
const request = new Request("https://example.test/api/me");
40+
const auth = await verifyAuth(
41+
request,
42+
makeEnv({ ALLOW_INSECURE_DEV_AUTH: "true", DEV_AUTH_USER_ID: "local-dev" }),
43+
);
44+
expect(auth?.userId).toBe("local-dev");
45+
expect(auth?.source).toBe("dev");
46+
});
47+
});

functions/_lib/auth.ts

Lines changed: 52 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,37 @@ const readHeaderUserName = (request: Request): string => {
6666
return name.trim();
6767
};
6868

69+
export const inspectAuthRequest = (request: Request) => {
70+
const jwt =
71+
request.headers.get("cf-access-jwt-assertion") ??
72+
request.headers.get("Cf-Access-Jwt-Assertion") ??
73+
"";
74+
const email =
75+
request.headers.get("cf-access-authenticated-user-email") ??
76+
request.headers.get("Cf-Access-Authenticated-User-Email") ??
77+
"";
78+
const userId =
79+
request.headers.get("cf-access-authenticated-user-id") ??
80+
request.headers.get("Cf-Access-Authenticated-User-Id") ??
81+
"";
82+
const userName =
83+
request.headers.get("cf-access-authenticated-user-name") ??
84+
request.headers.get("Cf-Access-Authenticated-User-Name") ??
85+
"";
86+
return {
87+
hasJwtAssertion: Boolean(jwt.trim()),
88+
hasEmailHeader: Boolean(email.trim()),
89+
hasUserIdHeader: Boolean(userId.trim()),
90+
hasUserNameHeader: Boolean(userName.trim()),
91+
};
92+
};
93+
94+
const emitAuthLog = (env: Env, payload: Record<string, unknown>) => {
95+
const enabled = (env.AUTH_OBSERVABILITY ?? "true").trim().toLowerCase();
96+
if (enabled === "false" || enabled === "0" || enabled === "off") return;
97+
console.info(JSON.stringify({ event: "auth", ...payload }));
98+
};
99+
69100
const verifyCloudflareAccessJwt = async (
70101
token: string,
71102
request: Request,
@@ -111,6 +142,7 @@ const verifyCloudflareAccessJwt = async (
111142
email: typeof lastPayload.email === "string" && lastPayload.email.trim() ? lastPayload.email : readHeaderEmail(request),
112143
name: typeof lastPayload.name === "string" && lastPayload.name.trim() ? lastPayload.name : readHeaderUserName(request),
113144
},
145+
source: "jwt",
114146
};
115147
};
116148

@@ -123,6 +155,7 @@ const verifyByHeadersOnly = (request: Request): AuthContext | null => {
123155
email: readHeaderEmail(request),
124156
name: readHeaderUserName(request),
125157
},
158+
source: "headers",
126159
};
127160
};
128161

@@ -133,10 +166,12 @@ const allowInsecureDevAuth = (env: Env): AuthContext | null => {
133166
return {
134167
userId,
135168
tokenPayload: { devAuth: true },
169+
source: "dev",
136170
};
137171
};
138172

139173
export const verifyAuth = async (request: Request, env: Env): Promise<AuthContext | null> => {
174+
const authSignals = inspectAuthRequest(request);
140175
try {
141176
const token =
142177
request.headers.get("cf-access-jwt-assertion") ??
@@ -145,13 +180,27 @@ export const verifyAuth = async (request: Request, env: Env): Promise<AuthContex
145180

146181
if (token.trim()) {
147182
const jwtVerified = await verifyCloudflareAccessJwt(token.trim(), request, env);
148-
if (jwtVerified) return jwtVerified;
183+
if (jwtVerified) {
184+
emitAuthLog(env, { result: "ok", source: jwtVerified.source, ...authSignals });
185+
return jwtVerified;
186+
}
187+
emitAuthLog(env, { result: "fail", reason: "jwt_verify_failed", ...authSignals });
149188
}
150189

151190
const byHeader = verifyByHeadersOnly(request);
152-
if (byHeader) return byHeader;
191+
if (byHeader) {
192+
emitAuthLog(env, { result: "ok", source: byHeader.source, ...authSignals });
193+
return byHeader;
194+
}
153195
} catch {
196+
emitAuthLog(env, { result: "fail", reason: "auth_exception", ...authSignals });
154197
// Fail closed to header/dev fallback instead of surfacing auth internals as 500.
155198
}
156-
return allowInsecureDevAuth(env);
199+
const dev = allowInsecureDevAuth(env);
200+
if (dev) {
201+
emitAuthLog(env, { result: "ok", source: dev.source, ...authSignals });
202+
return dev;
203+
}
204+
emitAuthLog(env, { result: "fail", reason: "no_auth_context", ...authSignals });
205+
return null;
157206
};

functions/_lib/http.test.ts

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
import { describe, expect, it } from "vitest";
2+
import { normalizeApiErrorMessage, statusFromErrorMessage } from "./http";
3+
4+
describe("http error normalization", () => {
5+
it("maps known auth/access errors to stable statuses", () => {
6+
expect(statusFromErrorMessage("Session revoked by admin")).toBe(401);
7+
expect(statusFromErrorMessage("Unauthorized")).toBe(401);
8+
expect(statusFromErrorMessage("Account pending approval")).toBe(403);
9+
expect(statusFromErrorMessage("Forbidden")).toBe(403);
10+
expect(statusFromErrorMessage("User not found")).toBe(404);
11+
expect(statusFromErrorMessage("Name is required")).toBe(400);
12+
});
13+
14+
it("normalizes common messages", () => {
15+
expect(normalizeApiErrorMessage("Unauthorized token")).toBe("Unauthorized.");
16+
expect(normalizeApiErrorMessage("pending approval for account")).toBe("Account pending approval.");
17+
expect(normalizeApiErrorMessage("")).toBe("Request failed.");
18+
});
19+
});

functions/_lib/http.ts

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,3 +36,40 @@ export const handleOptions = (request: Request): Response =>
3636
status: 204,
3737
headers: corsHeaders(request),
3838
});
39+
40+
export const normalizeApiErrorMessage = (message: string): string => {
41+
const lower = message.toLowerCase();
42+
if (lower.includes("session revoked by admin")) return "Session revoked by admin.";
43+
if (lower.includes("pending approval")) return "Account pending approval.";
44+
if (lower.includes("unauthorized")) return "Unauthorized.";
45+
if (lower.includes("forbidden")) return "Forbidden.";
46+
if (lower.includes("not found")) return "Not found.";
47+
if (lower.includes("required") || lower.includes("must be valid") || lower.includes("missing ")) {
48+
return message;
49+
}
50+
return message || "Request failed.";
51+
};
52+
53+
export const statusFromErrorMessage = (message: string, fallback = 500): number => {
54+
const lower = message.toLowerCase();
55+
if (lower.includes("session revoked by admin")) return 401;
56+
if (lower.includes("unauthorized")) return 401;
57+
if (lower.includes("pending approval")) return 403;
58+
if (lower.includes("forbidden")) return 403;
59+
if (lower.includes("not found")) return 404;
60+
if (lower.includes("required") || lower.includes("must be valid") || lower.includes("invalid")) return 400;
61+
return fallback;
62+
};
63+
64+
export const errorResponse = (request: Request, error: unknown, fallback = 500): Response => {
65+
const message = error instanceof Error ? error.message : String(error);
66+
return withCors(
67+
request,
68+
json(
69+
{
70+
error: normalizeApiErrorMessage(message),
71+
},
72+
{ status: statusFromErrorMessage(message, fallback) },
73+
),
74+
);
75+
};

functions/_lib/types.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ export type Env = {
2323
DB: D1Database;
2424
ACCESS_TEAM_DOMAIN?: string;
2525
ACCESS_AUD?: string;
26+
AUTH_OBSERVABILITY?: string;
2627
ALLOW_INSECURE_DEV_AUTH?: string;
2728
DEV_AUTH_USER_ID?: string;
2829
ADMIN_USER_IDS?: string;
@@ -32,4 +33,5 @@ export type Env = {
3233
export type AuthContext = {
3334
userId: string;
3435
tokenPayload: Record<string, unknown>;
36+
source?: "jwt" | "headers" | "dev";
3537
};

functions/api/auth-diagnostics.ts

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
import { verifyAuth, inspectAuthRequest } from "../_lib/auth";
2+
import { assertUserAccess, ensureUser, fetchUserProfile } from "../_lib/db";
3+
import { errorResponse, handleOptions, json, withCors } from "../_lib/http";
4+
import type { Env } from "../_lib/types";
5+
6+
export const onRequestOptions: PagesFunction<Env> = async ({ request }) => handleOptions(request);
7+
8+
export const onRequestGet: PagesFunction<Env> = async ({ request, env }) => {
9+
try {
10+
const auth = await verifyAuth(request, env);
11+
if (!auth) return withCors(request, json({ error: "Unauthorized" }, { status: 401 }));
12+
13+
await ensureUser(env, auth.userId, auth.tokenPayload);
14+
await assertUserAccess(env, auth.userId);
15+
const me = await fetchUserProfile(env, auth.userId);
16+
if (!me) return withCors(request, json({ error: "Unauthorized" }, { status: 401 }));
17+
if (!me.isAdmin) return withCors(request, json({ error: "Forbidden" }, { status: 403 }));
18+
19+
const claims = auth.tokenPayload;
20+
return withCors(
21+
request,
22+
json({
23+
auth: {
24+
source: auth.source ?? "unknown",
25+
userId: auth.userId,
26+
signals: inspectAuthRequest(request),
27+
claims: {
28+
iss: typeof claims.iss === "string" ? claims.iss : null,
29+
sub: typeof claims.sub === "string" ? claims.sub : null,
30+
email: typeof claims.email === "string" ? claims.email : null,
31+
name: typeof claims.name === "string" ? claims.name : null,
32+
iat: typeof claims.iat === "number" ? claims.iat : null,
33+
exp: typeof claims.exp === "number" ? claims.exp : null,
34+
},
35+
config: {
36+
accessAudConfigured: Boolean((env.ACCESS_AUD ?? "").trim()),
37+
accessTeamDomainConfigured: Boolean((env.ACCESS_TEAM_DOMAIN ?? "").trim()),
38+
insecureDevAuthEnabled: (env.ALLOW_INSECURE_DEV_AUTH ?? "").trim().toLowerCase() === "true",
39+
authObservabilityEnabled: (env.AUTH_OBSERVABILITY ?? "true").trim().toLowerCase() !== "false",
40+
},
41+
},
42+
}),
43+
);
44+
} catch (error) {
45+
return errorResponse(request, error, 500);
46+
}
47+
};

functions/api/changes.ts

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import { verifyAuth } from "../_lib/auth";
22
import { assertUserAccess, ensureUser, fetchResourceChanges } from "../_lib/db";
3-
import { handleOptions, json, withCors } from "../_lib/http";
3+
import { errorResponse, handleOptions, json, withCors } from "../_lib/http";
44
import type { Env } from "../_lib/types";
55

66
export const onRequestOptions: PagesFunction<Env> = async ({ request }) => handleOptions(request);
@@ -25,8 +25,6 @@ export const onRequestGet: PagesFunction<Env> = async ({ request, env }) => {
2525
const changes = await fetchResourceChanges(env, kind, resourceId);
2626
return withCors(request, json({ changes }));
2727
} catch (error) {
28-
const message = error instanceof Error ? error.message : String(error);
29-
const status = message.includes("pending approval") || message.includes("removed by admin") ? 403 : 500;
30-
return withCors(request, json({ error: message }, { status }));
28+
return errorResponse(request, error, 500);
3129
}
3230
};

functions/api/library.ts

Lines changed: 3 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import { verifyAuth } from "../_lib/auth";
22
import { assertUserAccess, ensureUser, fetchLibraryForUser, upsertLibrarySnapshot } from "../_lib/db";
3-
import { handleOptions, json, withCors } from "../_lib/http";
3+
import { errorResponse, handleOptions, json, withCors } from "../_lib/http";
44
import type { CloudResourceRecord, Env, LibrarySnapshotPayload } from "../_lib/types";
55

66
const normalizeArray = (value: unknown): CloudResourceRecord[] => (Array.isArray(value) ? (value as CloudResourceRecord[]) : []);
@@ -22,9 +22,7 @@ export const onRequestGet: PagesFunction<Env> = async ({ request, env }) => {
2222
}),
2323
);
2424
} catch (error) {
25-
const message = error instanceof Error ? error.message : String(error);
26-
const status = message.includes("pending approval") || message.includes("removed by admin") ? 403 : 500;
27-
return withCors(request, json({ error: message }, { status }));
25+
return errorResponse(request, error, 500);
2826
}
2927
};
3028

@@ -52,11 +50,6 @@ export const onRequestPut: PagesFunction<Env> = async ({ request, env }) => {
5250
}),
5351
);
5452
} catch (error) {
55-
const message = error instanceof Error ? error.message : String(error);
56-
const status =
57-
message.includes("pending approval") || message.includes("removed by admin")
58-
? 403
59-
: 400;
60-
return withCors(request, json({ error: message }, { status }));
53+
return errorResponse(request, error, 400);
6154
}
6255
};

0 commit comments

Comments
 (0)