perf(auth): resolve the caller in one query instead of two

apps/backend/lambdas/auth/controllers/auth.ts

@@ -9,8 +9,6 @@ import {
 } from '@aws-sdk/client-cognito-identity-provider';
 import { json, parseBody } from '@branch/lambda-http';
 import type { RouteHandler } from '@branch/lambda-http';
-import { authenticateRequest } from '../auth';
-import db from '../db';
 import { resolveProfileImage } from '../photos';
 import {
   cognitoClient,
@@ -197,16 +195,7 @@ export const handleMe: RouteHandler = async ({ auth }) => {
   // `access: 'authenticated'` on the route means dispatch has already verified
   // the session and loaded the subject; re-doing either here would be a second
   // token verification and a second memberships query per request.
-  const user = auth.context.user;
-  if (!user) {
-    return json(401, { message: 'Authentication required' });
-  }
-
-  const me = await db
-    .selectFrom('branch.users')
-    .where('cognito_sub', '=', user.cognitoSub)
-    .select(['user_id', 'cognito_sub', 'email', 'name', 'is_admin', 'profile_image'])
-    .executeTakeFirst();
+  const me = auth.context.user?.dbUser;
 
   // Defensive: authenticateRequest already rejects a token whose sub has no row,
   // so this is unreachable today. Kept so a future refactor cannot turn a
@@ -218,12 +207,12 @@ export const handleMe: RouteHandler = async ({ auth }) => {
   }
 
   return json(200, {
-    userId: me.user_id,
-    cognitoSub: me.cognito_sub,
+    userId: me.userId,
+    cognitoSub: me.cognitoSub,
     email: me.email,
     name: me.name,
-    isAdmin: me.is_admin === true,
-    profileImage: await resolveProfileImage(me.profile_image),
+    isAdmin: me.isAdmin,
+    profileImage: await resolveProfileImage(me.profileImage),
     // The RBAC subject travels with identity so the browser evaluates the same
     // policy against the same facts the lambdas used. Without it the frontend
     // would have to re-derive "is a director" and "which projects am I on"

apps/backend/lambdas/auth/test/auth.login.unit.test.ts

@@ -421,32 +421,38 @@ describe('GET /me', () => {
     expect(mockExecuteTakeFirst).not.toHaveBeenCalled();
   });
 
-  it('returns 401 when the token verifies but no branch.users row exists', async () => {
+  it('returns 401 when the context carries no branch.users row', async () => {
+    // Unreachable today -- authentication rejects a token whose sub has no row.
+    // The guard keeps a future refactor from turning a missing row into a 500,
+    // and 401-not-404 keeps /me from being a user-existence oracle.
     mockAuthenticateRequest.mockResolvedValue({
       isAuthenticated: true,
       user: { cognitoSub: 'sub-1', isAdmin: false },
     });
-    mockExecuteTakeFirst.mockResolvedValue(undefined);
 
     const res = await handler(event('/me', 'GET'));
 
     expect(res.statusCode).toBe(401);
   });
 
-  it('sources isAdmin from the database row, not the auth context', async () => {
-    // Regression guard: /auth/me is the only place the frontend can learn
-    // isAdmin, and it must reflect branch.users rather than any token claim.
+  it('answers from the row authentication already read, with no query of its own', async () => {
+    // Was three round trips: identity, memberships, then this handler
+    // re-reading the identity row by the same key for a different column list.
     mockAuthenticateRequest.mockResolvedValue({
       isAuthenticated: true,
-      user: { cognitoSub: 'sub-1', isAdmin: false },
-    });
-    mockExecuteTakeFirst.mockResolvedValue({
-      user_id: 7,
-      cognito_sub: 'sub-1',
-      email: 'a@b.com',
-      name: 'Ada',
-      is_admin: true,
-      profile_image: null,
+      user: {
+        cognitoSub: 'sub-1',
+        userId: 7,
+        isAdmin: true,
+        dbUser: {
+          userId: 7,
+          cognitoSub: 'sub-1',
+          email: 'a@b.com',
+          name: 'Ada',
+          isAdmin: true,
+          profileImage: 'https://s3/pic.png',
+        },
+      },
     });
 
     const res = await handler(event('/me', 'GET', undefined, { Authorization: 'Bearer t' }));
@@ -458,30 +464,61 @@ describe('GET /me', () => {
       email: 'a@b.com',
       name: 'Ada',
       isAdmin: true,
-      profileImage: null,
+      profileImage: 'https://s3/pic.png',
       // The authorization subject rides along with identity so the browser can
       // evaluate @branch/rbac without a second round trip.
       rbac: mockSubject,
     });
+    expect(mockExecuteTakeFirst).not.toHaveBeenCalled();
   });
 
-  it('coerces a non-boolean is_admin to false', async () => {
+  it('reports the email column, not the token email claim', async () => {
+    // The two fields are populated from different places on purpose: an access
+    // token's claim can be stale or absent, branch.users.email cannot.
     mockAuthenticateRequest.mockResolvedValue({
       isAuthenticated: true,
-      user: { cognitoSub: 'sub-1', isAdmin: true },
+      user: {
+        cognitoSub: 'sub-1',
+        userId: 7,
+        email: 'stale-claim@b.com',
+        isAdmin: false,
+        dbUser: {
+          userId: 7,
+          cognitoSub: 'sub-1',
+          email: 'column@b.com',
+          name: 'Ada',
+          isAdmin: false,
+          profileImage: null,
+        },
+      },
     });
-    mockExecuteTakeFirst.mockResolvedValue({
-      user_id: 7,
-      cognito_sub: 'sub-1',
-      email: 'a@b.com',
-      name: 'Ada',
-      is_admin: null,
-      profile_image: null,
+
+    const res = await handler(event('/me', 'GET'));
+
+    expect(JSON.parse(res.body).email).toBe('column@b.com');
+  });
+
+  it('sources isAdmin from the database row', async () => {
+    mockAuthenticateRequest.mockResolvedValue({
+      isAuthenticated: true,
+      user: {
+        cognitoSub: 'sub-1',
+        userId: 7,
+        isAdmin: false,
+        dbUser: {
+          userId: 7,
+          cognitoSub: 'sub-1',
+          email: 'a@b.com',
+          name: 'Ada',
+          isAdmin: true,
+          profileImage: null,
+        },
+      },
     });
 
     const res = await handler(event('/me', 'GET'));
 
-    expect(JSON.parse(res.body).isAdmin) .toBe(false);
+    expect(JSON.parse(res.body).isAdmin).toBe(true);
   });
 });
 

shared/lambda-auth/src/authenticate.ts

@@ -1,5 +1,5 @@
 import { CognitoJwtVerifier } from 'aws-jwt-verify';
-import type { AuthContext, AuthenticatedUser } from './types';
+import type { AuthContext, AuthenticatedUser, AuthMembership } from './types';
 
 // Minimal structural type — avoids a hard dependency on kysely at compile time.
 // Parameter typed as `any` so Kysely<DB>'s constrained overload is assignable.
@@ -42,6 +42,35 @@ export function extractToken(event: any): string | null {
   return authHeader;
 }
 
+/**
+ * One row of the identity query. The LEFT JOIN fans the single `branch.users`
+ * row out to one row per membership, so the user columns repeat and the
+ * membership columns are NULL for a user who holds none.
+ */
+interface CallerRow {
+  user_id: number;
+  cognito_sub: string | null;
+  email: string;
+  name: string;
+  is_admin: boolean | null;
+  profile_image: string | null;
+  project_id: number | null;
+  role: string | null;
+}
+
+/**
+ * Resolve the caller in a single round trip: verify the token, then read the
+ * `branch.users` row *and* its memberships with one LEFT JOIN.
+ *
+ * It used to be two strictly serial queries — identity, then memberships keyed
+ * on the user_id the first one returned — which put two RTTs in front of every
+ * guarded request in all six lambdas. The join is LEFT because a user with no
+ * memberships must still authenticate; an inner join would sign them out.
+ *
+ * The columns are the union of what authentication and `GET /auth/me` need, so
+ * `/auth/me` can answer from this context instead of re-reading the same row by
+ * the same key.
+ */
 export async function authenticateRequest(
   db: QueryableDb,
   event: any,
@@ -63,12 +92,25 @@ export async function authenticateRequest(
 
   // Uncaught on purpose: a DB outage is not a 401. Catching it hid an unreachable
   // RDS behind "Authentication required" and logged users out. Handlers map to 500.
-  const dbUser = await db
-    .selectFrom('branch.users')
-    .where('cognito_sub', '=', payload.sub)
-    .selectAll()
-    .executeTakeFirst();
+  const rows: CallerRow[] = await db
+    .selectFrom('branch.users as u')
+    .leftJoin('branch.project_memberships as pm', 'pm.user_id', 'u.user_id')
+    .where('u.cognito_sub', '=', payload.sub)
+    .select([
+      'u.user_id',
+      'u.cognito_sub',
+      'u.email',
+      'u.name',
+      'u.is_admin',
+      'u.profile_image',
+      'pm.project_id',
+      'pm.role',
+    ])
+    .execute();
 
+  // No rows at all means no `branch.users` row — the LEFT JOIN guarantees at
+  // least one row for a user that exists, membership or not.
+  const dbUser = rows[0];
   if (!dbUser) {
     console.warn(
       'User authenticated with Cognito but not found in database:',
@@ -77,18 +119,47 @@ export async function authenticateRequest(
     return { isAuthenticated: false };
   }
 
+  const isAdmin = dbUser.is_admin === true;
+
   const user: AuthenticatedUser = {
     cognitoSub: payload.sub,
     userId: dbUser.user_id,
     email: payload.email as string | undefined,
-    isAdmin: dbUser.is_admin === true,
+    isAdmin,
     // Informational only. We deliberately do NOT promote on a Cognito
     // "Admins" group: branch.users.is_admin is the single source of truth.
     // A second source would make demotion via PATCH /users/{userId} silently
     // ineffective, nothing in this codebase writes group membership, and no
     // aws_cognito_user_group is defined in infrastructure/aws/cognito.tf.
     cognitoGroups: payload['cognito:groups'] as string[] | undefined,
+    // The same row, under the names `GET /auth/me` reports. `email` above stays
+    // the token claim; this one is the column.
+    dbUser: {
+      userId: dbUser.user_id,
+      cognitoSub: dbUser.cognito_sub,
+      email: dbUser.email,
+      name: dbUser.name,
+      isAdmin,
+      profileImage: dbUser.profile_image,
+    },
+    memberships: collectMemberships(rows),
   };
 
   return { user, isAuthenticated: true };
 }
+
+/**
+ * Dedupe the fanned-out rows down to the memberships.
+ *
+ * A user with no memberships arrives as exactly one row whose membership
+ * columns are NULL; dropping those is what keeps the join from inventing a
+ * phantom membership on a null project.
+ */
+function collectMemberships(rows: readonly CallerRow[]): AuthMembership[] {
+  const memberships: AuthMembership[] = [];
+  for (const row of rows) {
+    if (row.project_id == null || row.role == null) continue;
+    memberships.push({ project_id: row.project_id, role: row.role });
+  }
+  return memberships;
+}

shared/lambda-auth/src/index.ts

@@ -1,6 +1,6 @@
 export * from './types';
 export { extractToken, authenticateRequest } from './authenticate';
-export { loadRbacSubject } from './rbac';
+export { loadRbacSubject, preloadedSubject } from './rbac';
 // The policy itself is re-exported so a lambda needs one import for auth and
 // authorization, the same way this package already re-exports the auth DTOs.
 export * from '@branch/rbac';

shared/lambda-auth/src/rbac.ts

@@ -8,9 +8,28 @@ interface QueryableDb {
 }
 
 /**
- * Build the authorization subject for an authenticated request: one query at the
- * edge, because nearly every rule needs the caller's memberships. The result is
- * also what `GET /auth/me` ships to the browser.
+ * The subject for a caller whose memberships `authenticateRequest` already
+ * joined in, or `null` when they were not loaded and somebody has to query.
+ *
+ * `null` and "member of nothing" are different answers, which is why this reads
+ * the presence of the array rather than its length: a hand-built context (every
+ * lambda's tests build one) must still fall through to `loadRbacSubject`.
+ */
+export function preloadedSubject(authContext: AuthContext): RbacSubject | null {
+  if (!authContext.isAuthenticated || !authContext.user?.userId) return null;
+  const memberships = authContext.user.memberships;
+  if (!memberships) return null;
+  return buildSubject(authContext.user, memberships);
+}
+
+/**
+ * Build the authorization subject for an authenticated request by reading the
+ * memberships from Postgres, because nearly every rule needs them. The result
+ * is also what `GET /auth/me` ships to the browser.
+ *
+ * `authenticateRequest` now fetches the same rows in the query that resolves the
+ * identity, so the request path takes `preloadedSubject` and never gets here.
+ * This remains the loader for a context that arrived without them.
  *
  * The assembly lives in `buildSubject` in @branch/rbac; this is the "read it
  * from Postgres" half.
@@ -22,6 +41,9 @@ export async function loadRbacSubject(
   const userId = authContext.user?.userId;
   if (!authContext.isAuthenticated || !userId) return ANONYMOUS;
 
+  const preloaded = preloadedSubject(authContext);
+  if (preloaded) return preloaded;
+
   const memberships: MembershipRow[] = await db
     .selectFrom('branch.project_memberships')
     .where('user_id', '=', userId)

shared/lambda-auth/src/types.ts

@@ -1,3 +1,8 @@
 // Declared once in @branch/types and re-exported here so runtime consumers get
 // the DTOs from the package that produces them.
-export type { AuthContext, AuthenticatedUser } from '@branch/types';
+export type {
+  AuthContext,
+  AuthenticatedUser,
+  AuthenticatedDbUser,
+  AuthMembership,
+} from '@branch/types';