apps/api/src/routes/oidc.ts

@@ -1,6 +1,6 @@
 // SPDX-License-Identifier: AGPL-3.0-or-later
 // Sign in with OpenID Connect, connect and disconnect an OpenID account (12 §3.2, §3.3).
-import { and, eq, gt, isNull, notExists } from 'drizzle-orm';
+import { and, eq, gt, isNull, ne, notExists } from 'drizzle-orm';
 import type { FastifyInstance, FastifyReply, FastifyRequest } from 'fastify';
 import { z } from 'zod';
 import { decrypt, encrypt } from '../crypto/secrets.ts';
@@ -15,7 +15,7 @@ import {
 } from '../errors.ts';
 import { requireAdmin, requireAuth } from '../auth/guards.ts';
 import { isLoginBlocked, recordLoginFailure } from '../auth/login-limiter.ts';
-import { createSession, setSessionCookie } from '../auth/sessions.ts';
+import { COOKIE, createSession, getSessionUser, setSessionCookie } from '../auth/sessions.ts';
 import { audit } from '../services/audit.ts';
 import { getSetting } from '../services/settings.ts';
 import { decide, verifiedEmail } from '../sso/claims.ts';
@@ -146,7 +146,13 @@ export async function oidcRoutes(app: FastifyInstance) {
       .where(and(eq(userIdentities.kind, 'oidc'), eq(userIdentities.subject, r.subject)));
 
     if (connecting) {
-      // Connect (12 §3.3): to the user of the session that asked, if it's still signed in.
+      // Connect (12 §3.3): to the user of the session that asked, if it's still signed in, and
+      // only in that browser: a planted sp_oidc can't connect someone else's account (08 §7 #8).
+      const here = req.cookies[COOKIE]
+        ? await getSessionUser(db, req.cookies[COOKIE], false)
+        : null;
+      if (here?.sessionId !== connecting)
+        return fail('expired', 'connect finished in another browser than the one that asked');
       const [sess] = await db
         .select({ userId: sessions.userId, email: users.email })
         .from(sessions)
@@ -186,7 +192,8 @@ export async function oidcRoutes(app: FastifyInstance) {
 
     let userId = owner?.userId;
     if (!userId) {
-      // A user without a password or an OpenID account yet, by verified email (12 §3.3 step 2).
+      // A user without a password or any connected account yet, by verified email (12 §3.3
+      // step 2). Never an admin: a provider account with their email isn't them (08 §7 #2).
       const v = verifiedEmail(r.claims, s.trustEmail);
       const [u] = v
         ? await db
@@ -196,13 +203,12 @@ export async function oidcRoutes(app: FastifyInstance) {
               and(
                 eq(users.email, v),
                 isNull(users.passwordHash),
+                ne(users.role, 'admin'),
                 notExists(
                   db
                     .select({ id: userIdentities.id })
                     .from(userIdentities)
-                    .where(
-                      and(eq(userIdentities.userId, users.id), eq(userIdentities.kind, 'oidc')),
-                    ),
+                    .where(eq(userIdentities.userId, users.id)),
                 ),
               ),
             )
@@ -322,8 +328,15 @@ export async function oidcRoutes(app: FastifyInstance) {
       recordLoginFailure(req.ip, login);
       throw badRequest('The directory refused that username or password');
     }
-    if ((await connectDirectory(db, req.log, req.user!.id, entry, req.ip)) === 'taken')
+    const r = await connectDirectory(db, req.log, req.user!.id, entry, req.ip);
+    if (r === 'taken')
       throw new HttpError(409, 'taken', 'That directory account belongs to another user');
+    if (r === 'connected_other')
+      throw new HttpError(
+        409,
+        'connected_other',
+        'Another directory account is connected. Ask an admin to disconnect it first',
+      );
     return { ok: true };
   });
   app.delete('/api/users/:id/identities/:kind', { preHandler: requireAdmin }, (req, reply) => {

apps/api/src/sso/directory.ts

@@ -1,7 +1,7 @@
 // SPDX-License-Identifier: AGPL-3.0-or-later
 // A directory entry → its SelfPhoneNG user (12 §4.3): connected before, or by the directory's
 // email. Never a new user (owner, 2026-10-25).
-import { and, eq } from 'drizzle-orm';
+import { and, eq, notExists } from 'drizzle-orm';
 import { deleteUserSessions } from '../auth/sessions.ts';
 import type { Db } from '../db/client.ts';
 import { userIdentities, users } from '../db/schema.ts';
@@ -22,7 +22,21 @@ export async function directoryUserFor(db: Db, e: DirectoryEntry): Promise<UserR
     .where(and(eq(userIdentities.kind, 'ldap'), eq(userIdentities.subject, e.id)));
   if (byId) return byId.u;
   if (!e.email) return null;
-  const [byEmail] = await db.select().from(users).where(eq(users.email, e.email));
+  // Nobody with another way in either: an OpenID user's account is theirs to connect (08 §7 #1).
+  const [byEmail] = await db
+    .select()
+    .from(users)
+    .where(
+      and(
+        eq(users.email, e.email),
+        notExists(
+          db
+            .select({ id: userIdentities.id })
+            .from(userIdentities)
+            .where(eq(userIdentities.userId, users.id)),
+        ),
+      ),
+    );
   return byEmail && byEmailOk(byEmail) ? byEmail : null;
 }
 
@@ -35,19 +49,27 @@ export async function directoryUserFor(db: Db, e: DirectoryEntry): Promise<UserR
 const byEmailOk = (u: UserRow) =>
   u.role !== 'admin' && (u.passwordHash === null || u.mustChangePassword);
 
-/** Connect a directory account the person proved (My settings, 12 §4.3). */
+/**
+ * Connect a directory account the person proved (My settings, 12 §4.3). Not another one in place
+ * of the one they have: the leaver check follows it, so swapping is an admin's job (08 §7 #5).
+ */
 export async function connectDirectory(
   db: Db,
   log: { info(o: object, m: string): void },
   userId: string,
   e: DirectoryEntry,
   ip: string,
-): Promise<'connected' | 'taken'> {
+): Promise<'connected' | 'taken' | 'connected_other'> {
   const [owner] = await db
     .select({ userId: userIdentities.userId })
     .from(userIdentities)
     .where(and(eq(userIdentities.kind, 'ldap'), eq(userIdentities.subject, e.id)));
   if (owner && owner.userId !== userId) return 'taken';
+  const [mine] = await db
+    .select({ subject: userIdentities.subject })
+    .from(userIdentities)
+    .where(and(eq(userIdentities.userId, userId), eq(userIdentities.kind, 'ldap')));
+  if (mine && mine.subject !== e.id) return 'connected_other';
   await db.transaction(async (tx) => {
     await tx
       .delete(userIdentities)
@@ -101,11 +123,8 @@ export async function directorySignIn(
       entityId: u.id,
       summary: `${u.email} connected directory account ${e.dn}`,
     });
-  } else if (mine.subject !== e.id) {
-    // Their email matches, but another directory account is already theirs: not this one.
-    log.info({ userId: u.id }, `directory entry ${e.dn} matches by email but isn't theirs`);
-    return null;
   } else
+    // Found by this account's id (directoryUserFor never matches by email someone connected).
     await db
       .update(userIdentities)
       .set({ email: e.email, lastUsedAt: new Date() })

apps/api/test/directory-signin.test.ts

@@ -1,12 +1,13 @@
 // SPDX-License-Identifier: AGPL-3.0-or-later
 // 12 §4.3: who a directory entry may connect to by email, who may disconnect it (§4.5), and a
-// directory that's down counting as a failed sign-in (§4.2).
+// directory that's down counting as a failed sign-in (§4.2). 08 §7: no OpenID user by email, no
+// swapping the connected account.
 import { eq } from 'drizzle-orm';
 import { afterAll, beforeAll, describe, expect, it } from 'vitest';
 import { resetLoginLimiter } from '../src/auth/login-limiter.ts';
 import { userIdentities, users } from '../src/db/schema.ts';
 import { setSetting } from '../src/services/settings.ts';
-import { directorySignIn } from '../src/sso/directory.ts';
+import { connectDirectory, directorySignIn } from '../src/sso/directory.ts';
 import type { DirectoryEntry } from '../src/sso/ldap.ts';
 import { hasTestDb } from './db.ts';
 import { ADMIN, H, createTestApp } from './app.ts';
@@ -52,6 +53,38 @@ describe.skipIf(!hasTestDb)('directory accounts by email (12 §4.3)', () => {
     }
   });
 
+  it('never connects by email someone who signs in with OpenID (08 §7 #1)', async () => {
+    // A directory that lets people edit their own mail: Mallory's entry, the victim's email.
+    const victim = await t.addUser('oidc-only@example.com', 'unused-pw-2');
+    await t.db.update(users).set({ passwordHash: null }).where(eq(users.id, victim.id));
+    await t.db
+      .insert(userIdentities)
+      .values({ userId: victim.id, kind: 'oidc', subject: 'https://idp victim-sub' });
+    expect(await directorySignIn(t.db, log, entry(victim.email), '127.0.0.1')).toBeNull();
+    expect(await connected(victim.id)).toBe(1); // the OpenID account only
+  });
+
+  it('a connected directory account is only swapped by an admin (08 §7 #5)', async () => {
+    const u = await t.addUser('swapper@example.com', 'swapper-password-1');
+    expect(await connectDirectory(t.db, log, u.id, entry(u.email), '127.0.0.1')).toBe('connected');
+    const [first] = await t.db.select().from(userIdentities).where(eq(userIdentities.userId, u.id));
+    // The same account again is fine; another one (a shared mailbox) is not.
+    expect(
+      await connectDirectory(
+        t.db,
+        log,
+        u.id,
+        { ...entry(u.email), id: first!.subject },
+        '127.0.0.1',
+      ),
+    ).toBe('connected');
+    expect(await connectDirectory(t.db, log, u.id, entry('shared@example.com'), '127.0.0.1')).toBe(
+      'connected_other',
+    );
+    const now = await t.db.select().from(userIdentities).where(eq(userIdentities.userId, u.id));
+    expect(now.map((i) => i.subject)).toEqual([first!.subject]);
+  });
+
   it('only an admin can disconnect a directory account (a leaver must stay checked)', async () => {
     const u = await t.addUser('leaver@example.com', 'leaver-password-1');
     await t.db.insert(userIdentities).values({ userId: u.id, kind: 'ldap', subject: 'leaver-id' });

apps/api/test/oidc.test.ts

@@ -207,6 +207,56 @@ for (const alg of ['RS256', 'ES256'] as const)
       expect(ids).toMatchObject([{ kind: 'oidc', subject: `${idp.issuer} u-cara` }]);
     });
 
+    it('never matches by email an admin, or someone with a directory account (08 §7 #2)', async () => {
+      const [boss, dir] = await t.db
+        .insert(users)
+        .values([
+          { email: 'diradmin@example.com', displayName: 'Boss', passwordHash: null, role: 'admin' },
+          { email: 'diruser@example.com', displayName: 'Dir', passwordHash: null },
+        ])
+        .returning();
+      await t.db
+        .insert(userIdentities)
+        .values({ userId: dir!.id, kind: 'ldap', subject: 'dir-user-id' });
+      for (const u of [boss!, dir!]) {
+        idp.st.user = { sub: `u-${u.email}`, email: u.email, email_verified: true };
+        const { callback } = await flow();
+        expect(errorOf(callback!)).toBe('not_connected');
+        expect(session(callback!)).toBeNull();
+      }
+    });
+
+    it('connect finishes only in the browser whose session asked (08 §7 #8)', async () => {
+      // Mallory starts Connect, then plants her sp_oidc cookie in Vic's browser and sends him on.
+      await t.addUser('mallory@example.com', 'mallory-password-1');
+      const [m] = await t.db.select().from(users).where(eq(users.email, 'mallory@example.com'));
+      await t.addUser('vic@example.com', 'vic-password-123');
+      const mallory = await t.login('mallory@example.com', 'mallory-password-1');
+      const vic = await t.login('vic@example.com', 'vic-password-123');
+      idp.st.user = { sub: 'u-vic', email: 'vic@example.com', email_verified: true };
+      for (const theirs of [vic, '']) {
+        const start = await t.app.inject({
+          method: 'POST',
+          url: '/api/auth/oidc/connect',
+          headers: { ...H, cookie: mallory },
+          payload: {},
+        });
+        const pending = start.cookies.find((c) => c.name === 'sp_oidc')!;
+        const atIdp = await fetch((start.json() as { url: string }).url, { redirect: 'manual' });
+        const back = new URL(atIdp.headers.get('location')!);
+        const callback = await t.app.inject({
+          url: back.pathname + back.search,
+          headers: { cookie: [`sp_oidc=${pending.value}`, theirs].filter(Boolean).join('; ') },
+        });
+        expect(errorOf(callback)).toBe('expired');
+        const ids = await t.db
+          .select()
+          .from(userIdentities)
+          .where(eq(userIdentities.userId, m!.id));
+        expect(ids).toEqual([]);
+      }
+    });
+
     it('trustEmail matches an unverified email', async () => {
       await t.db
         .insert(users)

docs/spec/12-sso.md

@@ -199,11 +199,12 @@ Failed callbacks count in the IP login limiter (04 §2.2).
 Signing in, in this order:
 
 1. A `user_identities` row (`oidc`, `"<iss> <sub>"`) → its user.
-2. Else, a user with that email (case-insensitive) who has **no password and no OpenID account
-   yet** (made by an admin in `oidc` mode, or whose password an admin removed), **and the email is
-   verified** (`email_verified === true`, or `trustEmail`) → connect it now (step 3's row) and sign
-   in. Users who have a password are never matched by email: they connect from My account, so a
-   provider where people can set their own email can't hand over an account.
+2. Else, a user with that email (case-insensitive) who is **not an admin** and has **no password
+   and no connected account of any kind yet** (made by an admin in `oidc` mode, or whose password
+   an admin removed), **and the email is verified** (`email_verified === true`, or `trustEmail`)
+   → connect it now (step 3's row) and sign in. Users who have a password, an admin, or someone
+   who signs in through the directory are never matched by email: they connect from My account,
+   so a provider where people can set their own email can't hand over an account (08 §7 #2).
 3. Else `not_connected`.
 
 Connecting (from My account, already signed in, any mode but `password`):
@@ -211,6 +212,9 @@ Connecting (from My account, already signed in, any mode but `password`):
 - After the callback, the session's user gets the `user_identities` row (email and groups as
   sent). Refused (`taken`) when that `iss sub` belongs to another user; a user who already has one
   replaces it (confirm in the UI). Other sessions stay. Audit `sso_connected`.
+- The callback must arrive in the browser whose session asked: its `sp_session` is the session in
+  `sp_oidc`, else `expired`. A planted `sp_oidc` (from a sibling subdomain) can't connect the
+  victim's provider account to someone else's user (08 §7 #8).
 - **Disconnect** (`DELETE /api/me/identities/oidc`; admins also on Users,
   `DELETE /api/users/:id/identities/oidc`): ends that user's `oidc` sessions. Refused (409
   `last_way_in`) when it would leave them no way to sign in in the current mode (no password, or
@@ -376,11 +380,13 @@ A user can have both a SelfPhoneNG password and a directory account: either pass
 
 1. A `user_identities` row (`ldap`, id) → its user. Their email here follows the directory's when
    it changes (unless another user has it: kept, logged).
-2. Else, a user with the directory email who is **not an admin** and has **no password or only the
-   temporary one** from Users (`must_change_password`) → connect it (add the row) and sign in,
+2. Else, a user with the directory email who is **not an admin**, has **no password or only the
+   temporary one** from Users (`must_change_password`) and **no connected account of any kind**
+   (an OpenID user's account is theirs, 08 §7 #1) → connect it (add the row) and sign in,
    audit `ldap_connected`. Anyone else connects on My settings → Sign-in → **Connect directory
    account** (`POST /api/me/identities/ldap`, `{ login, password }`, checked by the directory and
-   counted by the login limiter; 409 `taken` when another user has it). Security audit,
+   counted by the login limiter; 409 `taken` when another user has it, 409 `connected_other` when
+   they already have a different one: an admin disconnects it first, 08 §7 #5). Security audit,
    2026-10-26: a directory where people can edit their own `mail` (common OpenLDAP access rules)
    must not hand out the admin's account.
 3. Else → 401 "Wrong email or password" (no account is made, §1), with `login_failed` audited as