From 1e7882d529549e617fb9c5a931531f726dfa709a Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 4 Jul 2026 07:15:13 +0000 Subject: [PATCH] fix(admin): re-check the allowlist on every cookie-session request (revocation) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The NIP-98 API path already re-checked allowlist membership on every request, but the browser COOKIE-session path (whoami) checked membership only at login/elevation — so removing an identity from the committed allowlist would not revoke a live browser session until it expired (up to the 8h absolute bound). The governance model requires revocation (commit + deploy) to take effect on the identity's NEXT request. Add resolveAllowlistedAdmin(): the single per-request enforcement choke point for cookie-authenticated admin routes. It resolves the session (idle/absolute bounds, as before) AND re-checks the identity against the current allowlist, DESTROYING the session if the identity has been de-listed. whoami's browser path now routes through it; future privileged cookie routes (Phases 5/6) must too — never resolveAdminSessionId alone. No stored membership/role snapshot to go stale: ADMIN_SESSIONS holds identity only, so the check is always live. Logout stays de-escalation-only (a revoked admin may still clear its cookie). Tests: mid-session revocation for both methods — a live session whose identity is removed from the allowlist gets 401 on its next request AND is destroyed (the original allowlist can't resurrect it). Closes the gap that had no coverage before. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01LVPyHAtQ1cdK76RpFasafP --- worker/admin/router.ts | 46 ++++++++++++++++++++++++++++----- worker/tests/admin-auth.test.ts | 30 +++++++++++++++++++++ 2 files changed, 70 insertions(+), 6 deletions(-) diff --git a/worker/admin/router.ts b/worker/admin/router.ts index ce32682d..2da47d93 100644 --- a/worker/admin/router.ts +++ b/worker/admin/router.ts @@ -162,17 +162,51 @@ async function openAdminSession( ); } +/** True when a proven subject is STILL a member of the allowlist for its method. */ +function isSubjectAllowed(allowlist: AdminAllowlist, method: AdminMethod, subject: string): boolean { + return method === 'nostr' + ? isAllowedNostrPubkey(allowlist, subject) + : isAllowedBlueskyDid(allowlist, subject); +} + +/** + * The single per-request enforcement choke point for every COOKIE-authenticated + * admin route. Resolves the session (idle/absolute bounds) AND re-checks that its + * identity is STILL allowlisted — so removing an identity from the committed + * allowlist revokes every live session on its NEXT request, not merely at expiry + * (matching the governance model: revocation = commit + deploy, effective next + * request). A session whose identity has been de-listed is destroyed, not just + * rejected. The NIP-98 API path enforces membership inline (see whoami Path 2); + * this covers the browser path and any future privileged cookie route — route + * privileged reads/writes through this, never through resolveAdminSessionId alone. + * Returns null → caller responds 401. + */ +async function resolveAllowlistedAdmin( + request: Request, + env: AdminEnv & { ADMIN_SESSIONS: KVNamespace }, + deps: AdminDeps, +): Promise<{ id: string; record: { subject: string; method: AdminMethod } } | null> { + const session = await resolveAdminSessionId(env.ADMIN_SESSIONS, readCookie(request, ADMIN_SESSION_COOKIE)); + if (!session) return null; + const { record } = session; + if (!isSubjectAllowed(deps.allowlist, record.method, record.subject)) { + await destroyAdminSession(env.ADMIN_SESSIONS, session.id); // de-listed → kill the session + return null; + } + return session; +} + async function whoami(request: Request, env: AdminEnv, deps: AdminDeps): Promise { if (!adminConfigured(env)) return authError(503, 'admin not configured'); - // Path 1 — browser: the admin cookie session (idle/absolute checks + refresh - // live in resolveAdminSessionId). The CSRF token rides along so a reloaded tab - // can still make its next state-changing call; the response is same-origin- - // readable only (authJson sets no CORS) and the token is useless without the - // HttpOnly cookie beside it. + // Path 1 — browser: the admin cookie session, re-checked against the allowlist on + // EVERY request (resolveAllowlistedAdmin) so a revoked identity loses access on its + // next call, not at expiry. The CSRF token rides along so a reloaded tab can still + // make its next state-changing call; the response is same-origin-readable only + // (authJson sets no CORS) and the token is useless without the HttpOnly cookie. const cookieId = readCookie(request, ADMIN_SESSION_COOKIE); if (cookieId) { - const session = await resolveAdminSessionId(env.ADMIN_SESSIONS, cookieId); + const session = await resolveAllowlistedAdmin(request, env, deps); if (!session) return authError(); const { record } = session; const csrfRes = await coord(env.ADMIN_COORD, record.method, record.subject, '/csrf', { sessionId: session.id }); diff --git a/worker/tests/admin-auth.test.ts b/worker/tests/admin-auth.test.ts index 536d58a3..97ddfa81 100644 --- a/worker/tests/admin-auth.test.ts +++ b/worker/tests/admin-auth.test.ts @@ -226,6 +226,36 @@ test('allowlisted NIP-98 login mints a Strict __Host- admin session + csrf', asy assert.equal(whoBody.csrf, body.csrf); }); +test('mid-session REVOCATION (Nostr): de-listing an identity kills its live cookie session on the next request', async () => { + const env = fakeEnv(); + const sk = generateSecretKey(); + const pk = getPublicKey(sk); + const allowed = { allowlist: { nostrPubkeys: [pk], blueskyDids: [] } }; + const id = sessionIdOf(await nostrLogin(env, sk, allowed)); + // While the session is still well within its idle+absolute bounds, the identity + // is removed from the allowlist (simulating a revocation commit+deploy). + const revoked = { allowlist: { nostrPubkeys: [], blueskyDids: [] } }; + assert.equal((await routeAdmin(withAdminCookie('/api/admin/whoami', id), env, revoked)).status, 401); + // …and the session is destroyed, not merely rejected: even the ORIGINAL (still- + // allowlisted) deps can't resurrect it. + assert.equal((await routeAdmin(withAdminCookie('/api/admin/whoami', id), env, allowed)).status, 401); +}); + +test('mid-session REVOCATION (Bluesky): de-listing a DID kills its elevated cookie session next request', async () => { + const env = fakeEnv(); + const did = 'did:plc:revokeme'; + const allowed = { allowlist: { nostrPubkeys: [], blueskyDids: [did] } }; + const sid = await blueskyUserSession(env, did); + const elevated = await routeAdmin(req('/api/admin/elevate/bluesky', { + method: 'POST', headers: { cookie: `${SESSION_COOKIE}=${sid}` }, + }), env, allowed); + const id = sessionIdOf(elevated); + assert.equal((await routeAdmin(withAdminCookie('/api/admin/whoami', id), env, allowed)).status, 200); + const revoked = { allowlist: { nostrPubkeys: [], blueskyDids: [] } }; + assert.equal((await routeAdmin(withAdminCookie('/api/admin/whoami', id), env, revoked)).status, 401); + assert.equal((await routeAdmin(withAdminCookie('/api/admin/whoami', id), env, allowed)).status, 401); // destroyed +}); + test('a replayed login (same challenge + token) fails: challenges are single-use', async () => { const env = fakeEnv(); const sk = generateSecretKey();