fix(19): BL-04 keep context oidcIss/oidcSub null for local users (no fabricated identity sentinels)

This commit is contained in:
Lucas Berger
2026-06-17 20:24:01 -04:00
parent 71537601ce
commit 40666e1cc5
3 changed files with 62 additions and 12 deletions
+21 -4
View File
@@ -44,15 +44,32 @@ export const DEV_USER = {
color: COLOR_PALETTE[0], // '#4A90D9' — first palette slot color: COLOR_PALETTE[0], // '#4A90D9' — first palette slot
} as const; } as const;
/**
* The shape stored on c.get('user') across the bypass, local-session, and OIDC paths.
*
* BL-04: oidcIss/oidcSub are NULLABLE. Local users have null OIDC fields, and
* localAuthMiddleware must NOT fabricate sentinel ('local'/String(id)) values — those
* share the uniqueness domain (uniq_oidc_identity) with real OIDC identities and could
* collide with a genuine (iss,sub) pair if ever persisted. DEV_USER carries non-null
* 'dev'/'dev-user' values and remains assignable to this widened shape.
*/
export interface ContextUser {
id: number;
oidcIss: string | null;
oidcSub: string | null;
displayName: string | null;
color: string;
}
/** /**
* Extend Hono's ContextVariableMap so that c.get('user') / c.set('user', ...) * Extend Hono's ContextVariableMap so that c.get('user') / c.set('user', ...)
* are statically typed throughout the app. The value type is the DEV_USER shape, * are statically typed throughout the app. The value type is ContextUser — the shape
* which is compatible with both the bypass path and any future app-level user object * shared by the dev-bypass path, the local-session path (nullable oidc fields), and any
* stored on context (they share the same id/displayName/color subset). * future app-level user object stored on context.
*/ */
declare module 'hono' { declare module 'hono' {
interface ContextVariableMap { interface ContextVariableMap {
user: typeof DEV_USER; user: ContextUser;
} }
} }
+11 -8
View File
@@ -33,7 +33,7 @@ import { eq } from 'drizzle-orm';
import { db } from '../db/client.js'; import { db } from '../db/client.js';
import { users } from '../db/schema.js'; import { users } from '../db/schema.js';
import { verifyLocalSessionCookie } from './localSession.js'; import { verifyLocalSessionCookie } from './localSession.js';
import type { DEV_USER } from './devBypass.js'; import type { ContextUser } from './devBypass.js';
/** /**
* Returns a Hono MiddlewareHandler that: * Returns a Hono MiddlewareHandler that:
@@ -81,17 +81,20 @@ export function localAuthMiddleware(): MiddlewareHandler {
return; return;
} }
// Populate c.get('user') with the same shape as DEV_USER (devBypass.ts ContextVariableMap). // Populate c.get('user') with the ContextUser shape (devBypass.ts ContextVariableMap).
// oidcIss/oidcSub: local users have nullable oidcIss/oidcSub — use fallback strings so the // BL-04: keep oidcIss/oidcSub as NULL for local users — do NOT fabricate
// shape is compatible with typeof DEV_USER at runtime. Cast required because ContextVariableMap // 'local'/String(id) sentinels. Those values share the uniq_oidc_identity uniqueness
// is narrowed to the const DEV_USER literal type. // domain with real OIDC identities, so persisting them (e.g. a future upsertUser call
// using these context values) would let two local users collide or a local user shadow
// a genuine OIDC identity. ContextUser widens oidcIss/oidcSub to string | null so no
// cast is needed.
c.set('user', { c.set('user', {
id: row.id, id: row.id,
oidcIss: row.oidcIss ?? 'local', oidcIss: row.oidcIss ?? null,
oidcSub: row.oidcSub ?? String(row.id), oidcSub: row.oidcSub ?? null,
displayName: row.displayName ?? null, displayName: row.displayName ?? null,
color: row.color ?? '#4A90D9', color: row.color ?? '#4A90D9',
} as typeof DEV_USER); } satisfies ContextUser);
await next(); await next();
}; };
@@ -155,6 +155,36 @@ describe('localAuthMiddleware', () => {
expect(u.color).toBe('#FF5733'); expect(u.color).toBe('#FF5733');
}); });
it('Test 1c (BL-04): local user with null DB oidc fields → context oidcIss/oidcSub are NULL (no fabricated sentinels)', async () => {
mockVerifyResult = 9;
mockDbSelectResult.push({
id: 9,
oidcIss: null, // local user — no OIDC identity bound
oidcSub: null,
displayName: 'Local Member',
color: '#22AA88',
});
const localAuthMiddleware = await getMiddleware();
const app = new Hono();
let capturedUser: unknown;
app.use('/api/*', localAuthMiddleware());
app.get('/api/test', (c) => {
capturedUser = c.get('user');
return c.json({ ok: true });
});
const res = await app.request('/api/test');
expect(res.status).toBe(200);
const u = capturedUser as { id: number; oidcIss: string | null; oidcSub: string | null };
expect(u.id).toBe(9);
// BL-04: must be null — NOT 'local' / String(id) sentinels that share the
// uniq_oidc_identity domain with real OIDC identities.
expect(u.oidcIss).toBeNull();
expect(u.oidcSub).toBeNull();
});
it('Test 2: no cookie → pure passthrough; c.get("user") remains unset (Pitfall-1 guard)', async () => { it('Test 2: no cookie → pure passthrough; c.get("user") remains unset (Pitfall-1 guard)', async () => {
mockVerifyResult = null; // No cookie / invalid mockVerifyResult = null; // No cookie / invalid