Skip to content

Commit d3d511a

Browse files
committed
Hardening batch: terrain toggle, access lifecycle, auth matrix helpers, identity audit
1 parent 861c07c commit d3d511a

14 files changed

Lines changed: 194 additions & 14 deletions

File tree

AGENTS.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,3 +7,4 @@
77
- Any modal/popover that can open on top of another dialog must use `tier="raised"` in `ModalOverlay`.
88
- When catching UI errors, use `getUiErrorMessage()` from `src/lib/uiError.ts` for consistent messaging.
99
- Do not leave backlog tasks in ambiguous state. Use `[x]` only when code and verification are done.
10+
- Do not start user-added backlog items without explicit user confirmation in the current thread.

docs/BACKLOG.md

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ State: stabilization pass (no net-new product features unless explicitly approve
2424
- [x] 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.
2525
- [x] What does the reject button currently do?
2626
- [x] 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
27-
- [ ] User should be able to select if
27+
- [ ] User should be able to select if e-mail should be visible to everyone or only admins
2828

2929
## Active stabilization backlog
3030

@@ -37,7 +37,9 @@ State: stabilization pass (no net-new product features unless explicitly approve
3737
- [ ] Restrict sign-ups and approval transitions with explicit lifecycle states
3838
- [ ] Add dedicated auth/permission tests for critical flows
3939
- Progress: baseline tests added for auth source resolution and error mapping; endpoint permission matrix still pending.
40+
- Progress: added access-policy helper matrix tests (`functions/_lib/access.test.ts`) and endpoint gating helpers in users endpoints.
4041
- [ ] Add dedicated identity reconciliation tests + audit logging coverage
42+
- Progress: identity reconciliation audit events now persisted in `user_identity_audit`; dedicated reconciliation tests still pending.
4143
- [x] Add observability for Cloudflare Access auth header/JWT variants
4244

4345
### Data and storage safety
@@ -73,6 +75,7 @@ State: stabilization pass (no net-new product features unless explicitly approve
7375
- [ ] Improve explanatory info for FSPL / TwoRay / ITM and defaults
7476
- [ ] Document map sampling strategies clearly in UI help
7577
- [ ] Recheck pass/fail interpretation and communication around terrain blocking
78+
- [x] Add terrain overlay visibility toggle (visual only; simulation still uses loaded terrain)
7679

7780
### Security and access hardening
7881
- [x] Productize Access policy templates in-app docs and setup checklist

functions/_lib/access.test.ts

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
import { describe, expect, it } from "vitest";
2+
import {
3+
canDeleteUserAccount,
4+
canListUsers,
5+
canUpdateUserApproval,
6+
canUpdateUserRole,
7+
deriveAccountState,
8+
type AccessUserLike,
9+
} from "./access";
10+
11+
const makeUser = (patch: Partial<AccessUserLike>): AccessUserLike => ({
12+
id: patch.id ?? "u1",
13+
isAdmin: patch.isAdmin ?? false,
14+
isApproved: patch.isApproved ?? false,
15+
approvedAt: patch.approvedAt ?? null,
16+
approvedByUserId: patch.approvedByUserId ?? null,
17+
});
18+
19+
describe("access matrix", () => {
20+
it("derives account lifecycle deterministically", () => {
21+
expect(deriveAccountState(makeUser({ isApproved: false, approvedAt: null }))).toBe("pending");
22+
expect(deriveAccountState(makeUser({ isApproved: true }))).toBe("approved");
23+
expect(deriveAccountState(makeUser({ isAdmin: true, isApproved: false }))).toBe("approved");
24+
expect(deriveAccountState(makeUser({ isApproved: false, approvedAt: "2026-01-01T00:00:00Z" }))).toBe(
25+
"revoked",
26+
);
27+
});
28+
29+
it("enforces admin-only list and disallows self-moderation", () => {
30+
const admin = makeUser({ id: "admin", isAdmin: true, isApproved: true });
31+
const normal = makeUser({ id: "user", isAdmin: false, isApproved: true });
32+
33+
expect(canListUsers(admin)).toBe(true);
34+
expect(canListUsers(normal)).toBe(false);
35+
36+
expect(canUpdateUserRole(admin, "user")).toBe(true);
37+
expect(canUpdateUserRole(admin, "admin")).toBe(false);
38+
expect(canUpdateUserRole(normal, "admin")).toBe(false);
39+
40+
expect(canUpdateUserApproval(admin, "user")).toBe(true);
41+
expect(canUpdateUserApproval(admin, "admin")).toBe(false);
42+
expect(canUpdateUserApproval(normal, "admin")).toBe(false);
43+
44+
expect(canDeleteUserAccount(admin, "user")).toBe(true);
45+
expect(canDeleteUserAccount(admin, "admin")).toBe(false);
46+
expect(canDeleteUserAccount(normal, "admin")).toBe(false);
47+
});
48+
});
49+

functions/_lib/access.ts

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
export type AccessUserLike = {
2+
id: string;
3+
isAdmin: boolean;
4+
isApproved: boolean;
5+
approvedAt?: string | null;
6+
approvedByUserId?: string | null;
7+
};
8+
9+
export type AccountState = "pending" | "approved" | "revoked";
10+
11+
export const deriveAccountState = (user: AccessUserLike): AccountState => {
12+
if (user.isAdmin || user.isApproved) return "approved";
13+
if (user.approvedAt || user.approvedByUserId) return "revoked";
14+
return "pending";
15+
};
16+
17+
export const canListUsers = (user: AccessUserLike): boolean => user.isAdmin;
18+
19+
export const canUpdateUserRole = (actor: AccessUserLike, targetUserId: string): boolean =>
20+
actor.isAdmin && actor.id !== targetUserId;
21+
22+
export const canUpdateUserApproval = (actor: AccessUserLike, targetUserId: string): boolean =>
23+
actor.isAdmin && actor.id !== targetUserId;
24+
25+
export const canDeleteUserAccount = (actor: AccessUserLike, targetUserId: string): boolean =>
26+
actor.isAdmin && actor.id !== targetUserId;
27+

functions/_lib/db.ts

Lines changed: 59 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,8 @@ const VISIBILITIES: Visibility[] = ["private", "public_read", "public_write"];
44
const ROLES: ResourceRole[] = ["viewer", "editor", "admin"];
55

66
let schemaReady: Promise<void> | null = null;
7-
const SCHEMA_VERSION = "2026-03-12";
7+
const SCHEMA_VERSION = "2026-03-12b";
8+
type AccountState = "pending" | "approved" | "revoked";
89

910
const sanitizeVisibility = (value: unknown): Visibility =>
1011
typeof value === "string" && VISIBILITIES.includes(value as Visibility)
@@ -169,6 +170,16 @@ const REQUIRED_COLUMNS: Record<string, string[]> = {
169170
site_roles: ["site_id", "user_id", "role", "created_at"],
170171
simulation_roles: ["simulation_id", "user_id", "role", "created_at"],
171172
resource_changes: ["id", "resource_kind", "resource_id", "action", "actor_user_id", "changed_at", "note"],
173+
user_identity_audit: [
174+
"id",
175+
"event_type",
176+
"target_user_id",
177+
"source_user_id",
178+
"actor_user_id",
179+
"idp_email",
180+
"details_json",
181+
"created_at",
182+
],
172183
};
173184

174185
export const getSchemaDiagnostics = async (env: Env): Promise<{
@@ -278,13 +289,26 @@ const ensureSchema = async (env: Env): Promise<void> => {
278289
note TEXT
279290
)`,
280291
),
292+
env.DB.prepare(
293+
`CREATE TABLE IF NOT EXISTS user_identity_audit (
294+
id INTEGER PRIMARY KEY AUTOINCREMENT,
295+
event_type TEXT NOT NULL,
296+
target_user_id TEXT NOT NULL,
297+
source_user_id TEXT,
298+
actor_user_id TEXT,
299+
idp_email TEXT,
300+
details_json TEXT,
301+
created_at TEXT NOT NULL
302+
)`,
303+
),
281304
env.DB.prepare("CREATE INDEX IF NOT EXISTS idx_sites_owner ON sites(owner_user_id)"),
282305
env.DB.prepare("CREATE INDEX IF NOT EXISTS idx_sites_visibility ON sites(visibility)"),
283306
env.DB.prepare("CREATE INDEX IF NOT EXISTS idx_site_roles_user ON site_roles(user_id)"),
284307
env.DB.prepare("CREATE INDEX IF NOT EXISTS idx_simulations_owner ON simulations(owner_user_id)"),
285308
env.DB.prepare("CREATE INDEX IF NOT EXISTS idx_simulations_visibility ON simulations(visibility)"),
286309
env.DB.prepare("CREATE INDEX IF NOT EXISTS idx_simulation_roles_user ON simulation_roles(user_id)"),
287310
env.DB.prepare("CREATE INDEX IF NOT EXISTS idx_resource_changes_lookup ON resource_changes(resource_kind, resource_id, changed_at DESC)"),
311+
env.DB.prepare("CREATE INDEX IF NOT EXISTS idx_identity_audit_target ON user_identity_audit(target_user_id, created_at DESC)"),
288312
]);
289313

290314
const diagnostics = await getSchemaDiagnostics(env);
@@ -327,6 +351,12 @@ const toUserProfile = (row: UserRow) => ({
327351
avatarUrl: row.avatar_url ?? "",
328352
isAdmin: row.is_admin === 1,
329353
isApproved: row.is_approved === 1,
354+
accountState:
355+
row.is_admin === 1 || row.is_approved === 1
356+
? ("approved" as AccountState)
357+
: row.approved_at || row.approved_by_user_id
358+
? ("revoked" as AccountState)
359+
: ("pending" as AccountState),
330360
approvedAt: row.approved_at,
331361
approvedByUserId: row.approved_by_user_id,
332362
createdAt: row.created_at,
@@ -407,6 +437,27 @@ const reconcileUserIdentityByIdpEmail = async (
407437
env.DB.prepare("UPDATE simulation_roles SET user_id = ? WHERE user_id = ?").bind(userId, existing.id),
408438
env.DB.prepare("UPDATE resource_changes SET actor_user_id = ? WHERE actor_user_id = ?").bind(userId, existing.id),
409439
]);
440+
441+
await env.DB
442+
.prepare(
443+
`INSERT INTO user_identity_audit
444+
(event_type, target_user_id, source_user_id, actor_user_id, idp_email, details_json, created_at)
445+
VALUES (?, ?, ?, ?, ?, ?, ?)`,
446+
)
447+
.bind(
448+
"reconciled_by_verified_idp_email",
449+
userId,
450+
existing.id,
451+
userId,
452+
normalized,
453+
JSON.stringify({
454+
mergedFromUserId: existing.id,
455+
mergedFromIsAdmin: existing.is_admin === 1,
456+
mergedFromIsApproved: existing.is_approved === 1,
457+
}),
458+
now,
459+
)
460+
.run();
410461
};
411462

412463
export const ensureUser = async (
@@ -498,7 +549,10 @@ export const fetchUserProfile = async (env: Env, userId: string) => {
498549
export const assertUserAccess = async (env: Env, userId: string) => {
499550
const user = await fetchUserProfile(env, userId);
500551
if (!user) throw new Error("Unauthorized");
501-
if (!user.isApproved && !user.isAdmin) {
552+
if (user.accountState === "revoked") {
553+
throw new Error("Account access revoked by admin");
554+
}
555+
if (user.accountState !== "approved") {
502556
throw new Error("Account pending approval");
503557
}
504558
return user;
@@ -585,8 +639,8 @@ export const setUserApproval = async (
585639
.prepare(
586640
`UPDATE users
587641
SET is_approved = ?,
588-
approved_at = CASE WHEN ? = 1 THEN ? ELSE NULL END,
589-
approved_by_user_id = CASE WHEN ? = 1 THEN ? ELSE NULL END,
642+
approved_at = CASE WHEN ? = 1 THEN COALESCE(approved_at, ?) ELSE approved_at END,
643+
approved_by_user_id = CASE WHEN ? = 1 THEN COALESCE(approved_by_user_id, ?) ELSE approved_by_user_id END,
590644
updated_at = ?
591645
WHERE id = ?`,
592646
)
@@ -649,7 +703,7 @@ export const listPendingApprovalUsers = async (
649703
.prepare(
650704
`SELECT id, username, email, created_at, access_request_note
651705
FROM users
652-
WHERE is_approved = 0
706+
WHERE is_approved = 0 AND approved_at IS NULL
653707
ORDER BY created_at ASC
654708
LIMIT 200`,
655709
)

functions/_lib/http.test.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ describe("http error normalization", () => {
55
it("maps known auth/access errors to stable statuses", () => {
66
expect(statusFromErrorMessage("Schema out of date")).toBe(503);
77
expect(statusFromErrorMessage("Session revoked by admin")).toBe(401);
8+
expect(statusFromErrorMessage("Account access revoked by admin")).toBe(403);
89
expect(statusFromErrorMessage("Unauthorized")).toBe(401);
910
expect(statusFromErrorMessage("Account pending approval")).toBe(403);
1011
expect(statusFromErrorMessage("Forbidden")).toBe(403);
@@ -15,6 +16,7 @@ describe("http error normalization", () => {
1516
it("normalizes common messages", () => {
1617
expect(normalizeApiErrorMessage("Unauthorized token")).toBe("Unauthorized.");
1718
expect(normalizeApiErrorMessage("pending approval for account")).toBe("Account pending approval.");
19+
expect(normalizeApiErrorMessage("account access revoked by admin")).toBe("Account access revoked by admin.");
1820
expect(normalizeApiErrorMessage("")).toBe("Request failed.");
1921
});
2022
});

functions/_lib/http.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@ export const handleOptions = (request: Request): Response =>
4040
export const normalizeApiErrorMessage = (message: string): string => {
4141
const lower = message.toLowerCase();
4242
if (lower.includes("session revoked by admin")) return "Session revoked by admin.";
43+
if (lower.includes("access revoked by admin")) return "Account access revoked by admin.";
4344
if (lower.includes("pending approval")) return "Account pending approval.";
4445
if (lower.includes("unauthorized")) return "Unauthorized.";
4546
if (lower.includes("forbidden")) return "Forbidden.";
@@ -54,6 +55,7 @@ export const statusFromErrorMessage = (message: string, fallback = 500): number
5455
const lower = message.toLowerCase();
5556
if (lower.includes("schema out of date")) return 503;
5657
if (lower.includes("session revoked by admin")) return 401;
58+
if (lower.includes("access revoked by admin")) return 403;
5759
if (lower.includes("unauthorized")) return 401;
5860
if (lower.includes("pending approval")) return 403;
5961
if (lower.includes("forbidden")) return 403;

functions/api/users.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import { verifyAuth } from "../_lib/auth";
2+
import { canListUsers } from "../_lib/access";
23
import { assertUserAccess, ensureUser, fetchUserProfile, listUsers } from "../_lib/db";
34
import { errorResponse, handleOptions, json, withCors } from "../_lib/http";
45
import type { Env } from "../_lib/types";
@@ -14,7 +15,7 @@ export const onRequestGet: PagesFunction<Env> = async ({ request, env }) => {
1415
await assertUserAccess(env, auth.userId);
1516
const me = await fetchUserProfile(env, auth.userId);
1617
if (!me) return withCors(request, json({ error: "Unauthorized" }, { status: 401 }));
17-
if (!me.isAdmin) return withCors(request, json({ error: "Forbidden" }, { status: 403 }));
18+
if (!canListUsers(me)) return withCors(request, json({ error: "Forbidden" }, { status: 403 }));
1819

1920
const users = await listUsers(env);
2021
return withCors(request, json({ users }));

functions/api/users/[id].ts

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,9 @@
11
import { verifyAuth } from "../../_lib/auth";
2+
import {
3+
canDeleteUserAccount,
4+
canUpdateUserApproval,
5+
canUpdateUserRole,
6+
} from "../../_lib/access";
27
import {
38
assertUserAccess,
49
deleteUser,
@@ -101,12 +106,15 @@ export const onRequestPatch: PagesFunction<Env> = async ({ request, env, params
101106
});
102107
}
103108
if (body.isAdmin !== undefined) {
104-
if (targetId === auth.userId) {
109+
if (!canUpdateUserRole(me, targetId)) {
105110
return withCors(request, json({ error: "Users cannot change their own admin role." }, { status: 400 }));
106111
}
107112
user = await setUserAdminFlag(env, targetId, body.isAdmin);
108113
}
109114
if (body.isApproved !== undefined) {
115+
if (!canUpdateUserApproval(me, targetId)) {
116+
return withCors(request, json({ error: "Users cannot change their own approval state." }, { status: 400 }));
117+
}
110118
user = await setUserApproval(env, targetId, body.isApproved, auth.userId);
111119
}
112120

@@ -129,7 +137,7 @@ export const onRequestDelete: PagesFunction<Env> = async ({ request, env, params
129137

130138
const targetId = typeof params.id === "string" ? params.id : "";
131139
if (!targetId) return withCors(request, json({ error: "Missing user id" }, { status: 400 }));
132-
if (targetId === auth.userId) {
140+
if (!canDeleteUserAccount(me, targetId)) {
133141
return withCors(request, json({ error: "Admin cannot delete own account." }, { status: 400 }));
134142
}
135143

src/components/AppShell.tsx

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,10 @@ export function AppShell() {
2525
void (async () => {
2626
try {
2727
const me = await fetchMe();
28+
if (me.accountState === "revoked") {
29+
setAccessState("locked");
30+
return;
31+
}
2832
setAccessState(me.isAdmin || me.isApproved ? "granted" : "pending");
2933
} catch (error) {
3034
const message = getUiErrorMessage(error);

0 commit comments

Comments
 (0)