fix(auth): make account provisioning invitation-only

apps/backend/AGENTS.md

@@ -74,7 +74,11 @@ Automatic on push to `main` touching `apps/backend/lambdas/**` or `shared/types/
 
 **`branch.users.is_admin` is the single source of truth for admin.** There is no promotion from a Cognito group, and no pre-token-generation trigger, so `is_admin` is not a JWT claim — `GET /auth/me` is the only way a client can learn it.
 
-**A `branch.users` row with `cognito_sub IS NULL` is a pending invitation**, created by the `db/seed.sql` rows or by admin `POST /users`. `POST /auth/register` claims such a row (setting `cognito_sub`, never touching `is_admin`) instead of returning 409. Registration only 409s when the row is already claimed.
+**Account provisioning is invitation-only.** A `branch.users` row with `cognito_sub IS NULL` is a pending invitation, created by `db/seed.sql` or by admin `POST /users` (ADMIN-gated). `POST /auth/register` is public, so it deliberately **cannot create a row** — it only claims an existing invitation, setting `cognito_sub` and never touching `is_admin`. An email with no pending invitation gets 403 `INVITATION_REQUIRED`; an already-claimed one gets 409.
+
+The 403 is intentionally identical whether or not the address exists, so registration cannot be used to enumerate staff emails.
+
+This is the real control, not the Cognito pool config: `authenticateRequest` rejects any Cognito identity whose `sub` has no `branch.users` row, so a Cognito user created out of band is inert. Full flow: admin `POST /users` → invitee `POST /auth/register` with that email → `POST /auth/verify-email` with the emailed code → `POST /auth/login`.
 
 **Bootstrapping the first admin** is a manual SQL statement in every environment, because `is_admin` can only be set by an existing admin: `make grant-admin EMAIL=…` locally, or the equivalent `UPDATE` against RDS in production.
 

apps/backend/lambdas/auth/handler.ts

@@ -666,7 +666,27 @@ async function handleRegister(event: any): Promise<APIGatewayProxyResult> {
       return json(409, { message: 'User with this email already exists' });
     }
 
-    const claimingUserId: number | null = existingUser ? existingUser.user_id : null;
+    // REGISTRATION IS INVITATION-ONLY. This endpoint is public and
+    // unauthenticated, so without this gate anyone could create a working
+    // account for themselves. An account is only meaningful once a branch.users
+    // row exists -- authenticateRequest rejects any Cognito identity whose sub
+    // has no row -- so refusing to create that row here is the control.
+    //
+    // The invitation must be created first by an admin via the ADMIN-gated
+    // POST /users, which inserts a row with a NULL cognito_sub.
+    //
+    // 403 rather than 404: this endpoint must not become an oracle for which
+    // email addresses have been invited, so the response is deliberately the
+    // same whether or not the address is known.
+    if (!existingUser) {
+      return json(403, {
+        message:
+          'Registration is by invitation only. Ask an administrator to create your account.',
+        code: 'INVITATION_REQUIRED',
+      });
+    }
+
+    const claimingUserId: number = existingUser.user_id;
 
     // Prepare Cognito SignUp parameters
     const signUpParams: SignUpCommandInput = {
@@ -700,7 +720,7 @@ async function handleRegister(event: any): Promise<APIGatewayProxyResult> {
         // SignUp can never hand us a sub. Happens routinely in local dev: `make
         // down-v` wipes Postgres while the shared dev pool keeps the user. Link
         // the existing Cognito identity instead of dead-ending on a 409.
-        if (claimingUserId !== null) {
+        {
           try {
             // AdminGetUser is SigV4-signed and needs cognito-idp:AdminGetUser
             // (granted in infrastructure/aws/lambda.tf). With no AWS credentials
@@ -746,29 +766,19 @@ async function handleRegister(event: any): Promise<APIGatewayProxyResult> {
 
     // Create user in database, or claim the pending invitation
     try {
-      if (claimingUserId !== null) {
-        // is_admin is deliberately NOT touched: it was set by whoever created the
-        // invitation (a seed, or an admin via POST /users) and must not be
-        // settable from a public, unauthenticated endpoint. The cognito_sub IS
-        // NULL predicate makes a concurrent claim a no-op rather than an
-        // overwrite; UNIQUE(cognito_sub) is the backstop.
-        await db
-          .updateTable('branch.users')
-          .set({ cognito_sub: cognitoUserSub, name: name.trim() })
-          .where('user_id', '=', claimingUserId)
-          .where('cognito_sub', 'is', null)
-          .execute();
-      } else {
-        await db
-          .insertInto('branch.users')
-          .values({
-            cognito_sub: cognitoUserSub,
-            email: email.toLowerCase(),
-            name: name.trim(),
-            is_admin: false,
-          })
-          .execute();
-      }
+      // Claim the invitation. is_admin is deliberately NOT touched: it was set
+      // by whoever created the invitation (a seed, or an admin via POST /users)
+      // and must never be settable from a public, unauthenticated endpoint.
+      // There is no insert path here -- registration cannot mint a new row, only
+      // claim one an admin already approved. The cognito_sub IS NULL predicate
+      // makes a concurrent claim a no-op rather than an overwrite;
+      // UNIQUE(cognito_sub) is the backstop.
+      await db
+        .updateTable('branch.users')
+        .set({ cognito_sub: cognitoUserSub, name: name.trim() })
+        .where('user_id', '=', claimingUserId)
+        .where('cognito_sub', 'is', null)
+        .execute();
     } catch (dbError: any) {
       console.error('Database insert error:', dbError);
 
@@ -795,7 +805,7 @@ async function handleRegister(event: any): Promise<APIGatewayProxyResult> {
       name: name.trim(),
       emailVerificationRequired: true,
       details: 'Please check your email for verification code',
-      ...(claimingUserId !== null ? { claimed: true } : {}),
+      claimed: true,
     });
   } catch (error: any) {
     console.error('Registration error:', error);

apps/backend/lambdas/auth/openapi.yaml

@@ -21,8 +21,13 @@ paths:
 
   /register:
     post:
-      summary: Register a new user
-      description: Creates a new user account in Cognito and the database
+      summary: Claim an invitation and activate an account
+      description: >
+        INVITATION-ONLY. This endpoint is public and unauthenticated, so it
+        cannot mint new accounts: it only activates a branch.users row that an
+        admin already created via the ADMIN-gated POST /users (such a row has a
+        NULL cognito_sub). An email with no pending invitation gets 403.
+        is_admin is never written here.
       requestBody:
         required: true
         content:
@@ -87,8 +92,24 @@ paths:
                   message:
                     type: string
                     example: Invalid email format
+        '403':
+          description: >
+            No pending invitation for this email. Deliberately indistinguishable
+            from the response for an address that is not in the system at all,
+            so this endpoint cannot be used to enumerate staff emails.
+          content:
+            application/json:
+              schema:
+                type: object
+                properties:
+                  message:
+                    type: string
+                    example: Registration is by invitation only. Ask an administrator to create your account.
+                  code:
+                    type: string
+                    example: INVITATION_REQUIRED
         '409':
-          description: User already exists
+          description: The invitation has already been claimed
           content:
             application/json:
               schema:

apps/backend/lambdas/auth/test/auth.e2e.test.ts

@@ -23,7 +23,7 @@ beforeAll(async () => {
 
 beforeEach(async () => {
   const testName = expect.getState().currentTestName;
-  if (testName && (testName.includes('duplicate') || testName.includes('normalization') || testName.includes('register with valid'))) {
+  if (testName && (testName.includes('duplicate') || testName.includes('normalization') || testName.includes('uninvited') || testName.includes('seeded admins'))) {
     const client = await pool.connect();
     try {
       await resetData(client);
@@ -38,12 +38,13 @@ afterAll(async () => {
 });
 
 test("duplicate email returns 409", async () => {
-  // Insert a user with lowercase email directly in DB
+  // A CLAIMED row (cognito_sub set) is a genuine conflict. A row without one is
+  // a pending invitation and would be claimed instead -- see the test below.
   const client = await pool.connect();
   try {
     await client.query(
-      'INSERT INTO branch.users (email, name, is_admin) VALUES ($1, $2, $3)',
-      ['existing@example.com', 'Existing User', false]
+      'INSERT INTO branch.users (email, name, is_admin, cognito_sub) VALUES ($1, $2, $3, $4)',
+      ['existing@example.com', 'Existing User', false, 'existing-sub-123']
     );
   } finally {
     client.release();
@@ -69,8 +70,8 @@ test("email normalization uppercase matches lowercase in DB", async () => {
   const client = await pool.connect();
   try {
     await client.query(
-      'INSERT INTO branch.users (email, name, is_admin) VALUES ($1, $2, $3)',
-      ['lowercase@example.com', 'Existing User', false]
+      'INSERT INTO branch.users (email, name, is_admin, cognito_sub) VALUES ($1, $2, $3, $4)',
+      ['lowercase@example.com', 'Existing User', false, 'lowercase-sub-123']
     );
   } finally {
     client.release();
@@ -91,9 +92,12 @@ test("email normalization uppercase matches lowercase in DB", async () => {
   expect(body.message).toContain("already exists");
 });
 
-test("register with valid data (requires Cognito)", async () => {
+test("uninvited email is refused before Cognito is ever called", async () => {
+  // Registration is invitation-only: with no branch.users row for this address
+  // the request must be rejected outright. Previously this created a working
+  // account for any caller.
   const uniqueEmail = `test${Date.now()}${Math.random().toString(36).substring(7)}@example.com`;
-  
+
   const res = await fetch("http://localhost:3000/auth/register", {
     method: "POST",
     body: JSON.stringify({
@@ -103,30 +107,44 @@ test("register with valid data (requires Cognito)", async () => {
     })
   });
 
-  if (res.status === 201) {
-    const body = await res.json();
-    expect(body.message).toBe("User registered successfully");
-    expect(body.userId).toBeDefined();
-    expect(body.email).toBe(uniqueEmail);
-    expect(body.name).toBe("Test User");
-    expect(body.emailVerificationRequired).toBe(true);
+  expect(res.status).toBe(403);
+  const body = await res.json();
+  expect(body.code).toBe("INVITATION_REQUIRED");
 
-    const client = await pool.connect();
-    try {
-      const result = await client.query(
-        'SELECT * FROM branch.users WHERE email = $1',
-        [uniqueEmail]
-      );
-      expect(result.rows.length).toBe(1);
-      expect(result.rows[0].cognito_sub).toBe(body.userId);
-      expect(result.rows[0].is_admin).toBe(false);
-    } finally {
-      client.release();
+  // And nothing was written.
+  const client = await pool.connect();
+  try {
+    const { rows } = await client.query(
+      'SELECT 1 FROM branch.users WHERE email = $1',
+      [uniqueEmail]
+    );
+    expect(rows).toHaveLength(0);
+  } finally {
+    client.release();
+  }
+});
+
+test("seeded admins are pending invitations, not claimed accounts", async () => {
+  // The claim path depends on this contract: a seeded admin has a row but no
+  // Cognito identity, so /auth/register activates it instead of 409ing. Before
+  // claim-on-register these three could never sign in at all.
+  const client = await pool.connect();
+  try {
+    const { rows } = await client.query(
+      'SELECT email, user_id, is_admin, cognito_sub FROM branch.users ORDER BY user_id'
+    );
+
+    expect(rows).toHaveLength(3);
+    for (const row of rows) {
+      expect(row.cognito_sub).toBeNull();
+      expect(row.is_admin).toBe(true);
     }
-  } else if (res.status === 500) {
-    console.log('Skipping Cognito test - Cognito not configured');
-    expect(res.status).toBe(500);
-  } else {
-    throw new Error(`Unexpected status code: ${res.status}`);
+    expect(rows.map((r: { email: string }) => r.email)).toEqual([
+      'ashley@branch.org',
+      'renee@branch.org',
+      'nour@branch.org',
+    ]);
+  } finally {
+    client.release();
   }
 });

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

@@ -474,6 +474,14 @@ describe('GET /me', () => {
 describe('POST /register — claim-on-register', () => {
   const validBody = { email: 'Ashley@branch.org', password: 'Password123', name: 'Ashley' };
 
+  /** An admin-created invitation: the row exists, but no Cognito identity yet. */
+  const invitation = {
+    user_id: 1,
+    email: 'ashley@branch.org',
+    cognito_sub: null,
+    is_admin: true,
+  };
+
   it('claims a pending invitation (cognito_sub IS NULL) instead of returning 409', async () => {
     mockExecuteTakeFirst.mockResolvedValue({
       user_id: 1,
@@ -521,22 +529,37 @@ describe('POST /register — claim-on-register', () => {
     expect(mockSend).not.toHaveBeenCalled();
   });
 
-  it('inserts a new row when no invitation exists', async () => {
+  it('refuses to create an account for an uninvited email', async () => {
+    // Registration is invitation-only. This endpoint is public, so without the
+    // gate anyone on the internet could mint themselves a working account --
+    // and several list endpoints authorize on `isAuthenticated` alone.
     mockExecuteTakeFirst.mockResolvedValue(undefined);
-    mockSend.mockResolvedValue({ UserSub: 'new-sub' });
-    mockExecute.mockResolvedValue(undefined);
 
     const res = await handler(event('/register', 'POST', validBody));
 
-    expect(res.statusCode).toBe(201);
-    expect(JSON.parse(res.body).claimed).toBeUndefined();
-    expect(mockValues).toHaveBeenCalledWith(
-      expect.objectContaining({ cognito_sub: 'new-sub', is_admin: false }),
+    expect(res.statusCode).toBe(403);
+    expect(JSON.parse(res.body).code).toBe('INVITATION_REQUIRED');
+    // No Cognito user and no DB row: nothing is created at all.
+    expect(mockSend).not.toHaveBeenCalled();
+    expect(mockValues).not.toHaveBeenCalled();
+    expect(mockExecute).not.toHaveBeenCalled();
+  });
+
+  it('does not reveal whether an uninvited email is known', async () => {
+    // Same response shape for an unknown address as for a known-but-uninvited
+    // one, so /register cannot be used to enumerate staff email addresses.
+    mockExecuteTakeFirst.mockResolvedValue(undefined);
+
+    const unknown = await handler(
+      event('/register', 'POST', { ...validBody, email: 'stranger@example.com' }),
     );
+
+    expect(unknown.statusCode).toBe(403);
+    expect(JSON.parse(unknown.body).message).not.toMatch(/not found|no such|unknown/i);
   });
 
   it('rolls back the Cognito user when the database write fails', async () => {
-    mockExecuteTakeFirst.mockResolvedValue(undefined);
+    mockExecuteTakeFirst.mockResolvedValue(invitation);
     mockSend
       .mockResolvedValueOnce({ UserSub: 'new-sub' }) // SignUp
       .mockResolvedValueOnce({}); // AdminDeleteUser