Bind OAuth account linking to verified provider identity

.agents/inbox/2026-10-27-oauth-linking-needs-provider-proof-and-atomicity.md

@@ -0,0 +1,74 @@
+# OAuth email linking needs provider proof and an isolated transaction
+
+On 2026-10-27, the #184 OAuth review traced live `oauth-strategies.ts` into
+`findOrCreateUserFromOAuth` at token head `5aa307c`. Synthetic owned SQLite
+controls established two failures: unverified email linked to the existing
+admin fixture, and an intentionally rejected provider insert left one new user
+instead of zero. No live provider or shared user database was touched.
+
+## Evidence and correction
+
+`npm test --workspace @graphdone/server -- tests/oauth-identity.test.ts` on the
+unchanged implementation produced two failures and one disabled-subject control
+pass after correcting the fixture's initially wrong status-method name. The
+first run's fixture error is not evidence of an application defect.
+
+The fixed helper requires explicit verified-email/authority evidence for new
+links, selects bound provider+subject before email, rejects inactive accounts,
+and checks the authenticated revision again. Dedicated file-backed SQLite
+transactions make user creation and provider linking atomic. Actual provider
+strategy tests exposed important adapter contracts: GitHub's default mapping
+drops verification flags; LinkedIn's short callback skips userinfo. The revised
+configuration retains GitHub metadata, requests LinkedIn userinfo with the
+nine-argument callback, checks its subject and uses its documented issuer.
+Google third-party email verification alone is not ownership authority.
+
+The first concurrent transaction implementation used native busy waits on eight
+connections; the five-second regression timed out. SQLite waits can occupy the
+native worker pool needed by the transaction holding the lock. Serialize this
+process's OAuth writes by database file, keep the transaction on its own
+connection, hash before acquiring the write lock, and use bounded asynchronous
+busy retries. Do not put an async transaction on the shared auth connection:
+unrelated writes could join its rollback. The regression now proves eight first
+logins create one user/binding, and an unrelated rollback cannot erase that user.
+No test timeout increase was used as the correction.
+
+Node 22.18.0 focused validation passed 38 cases across `oauth-identity`,
+`oauth-profiles` and `oauth-strategies`, including actual local HTTP callback
+issuance and stale-revision rejection. Initial build errors from the GitHub
+state declaration, an unused token import and the project's augmented Express
+request type were corrected without weakening runtime checks. The callback
+uses contextual `RequestHandler` typing. Final full gates are recorded on the PR.
+
+## Limits and recovery
+
+The [provider contract and recovery policy](../../docs/quality/OAUTH_IDENTITY.md)
+links official sources and describes existing bindings, absent proof, legacy
+credential retention and synthetic-test limits. Existing bindings are preserved,
+not retrospectively certified. New provider tokens/raw profiles are not stored;
+callback logs no longer expose credential-bearing redirects. Private traces and
+raw tokens must stay out of repository notes. No deployed provider acceptance
+or hosted account clearance follows from these local tests.
+
+## Independent review corrections
+
+Review of `c4561ae` found a preregistration takeover path: an existing unverified
+local account kept its earlier password and JWT when verified OAuth linked to
+its email. The added negative uses the actual password-login resolver and
+current-account JWT resolver before attempting the link; it failed against
+`c4561ae` because the link succeeded. The corrected transaction rejects an
+unverified existing email match. A separate verified-account positive remains.
+No account transfer or credential reset is silently performed.
+
+The review also drove the actual OIDC strategy with asynchronous provider
+callbacks. Malformed userinfo JSON reached an installed library catch block
+that calls undefined `done`, causing an uncaught ReferenceError. GitHub's email
+loader also parses JSON outside a safe error boundary. The final implementation
+disables those library profile loaders and fetches their fixed HTTPS endpoints
+with bounded time/body size, redirect rejection and explicit JSON/status checks.
+It retains the authenticated ID-token subject for LinkedIn and exact GitHub
+verification metadata. Asynchronous malformed/error/oversize/abort controls now
+fail authentication without account lookup. The synchronous first fixtures had
+not exercised this exception path; provider doubles must preserve asynchronous
+callback boundaries. The focused corrected set passed 49 cases. Full final
+review and gates remain in the PR rather than inferred from this count.

.agents/issues/INDEX.md

@@ -2,6 +2,7 @@
 
 | Symptom | Canonical lesson and regression |
 | --- | --- |
+| OAuth links an unverified email, changes accounts after a provider email change, or leaves an orphan user after a link failure | [Provider proof and isolated atomic linking](../inbox/2026-10-27-oauth-linking-needs-provider-proof-and-atomicity.md); `oauth-identity`, `oauth-profiles` and actual `oauth-strategies` tests |
 | Login and reset links cross-use, or a reset/reactivation revives old sessions | [Bind purpose and session revision at the write](../inbox/2026-10-27-email-purpose-and-revision-are-auth-boundaries.md); real SQLite/HTTP/WebSocket `auth-link-boundary.test.ts` |
 | CAPTCHA widget is visible but an immediate verification event leaves proof empty | [Attach widget listeners during commit](../inbox/2026-10-27-widget-events-need-commit-phase-listeners.md); `AuthCaptcha.test.tsx` Profiler regression |
 | CI development quick login fails required proof and exhausts login attempts with NODE_ENV unset | [Declare mode in the dev entry point without overriding production](../inbox/2026-10-27-dev-launch-must-declare-mode.md); `development-startup.test.ts` and actual dev smoke |

.agents/issues/security-audit-index.md

@@ -2,6 +2,7 @@
 
 | Symptom | Evidence and corrective rule |
 | --- | --- |
+| OAuth email metadata loses verification or callbacks log credential-bearing redirects | [Provider proof and isolated atomic linking](../inbox/2026-10-27-oauth-linking-needs-provider-proof-and-atomicity.md); [provider/recovery contract](../../docs/quality/OAUTH_IDENTITY.md) |
 | Login and reset links cross-use, or a reset/reactivation revives old sessions | [Bind purpose and session revision at the write](../inbox/2026-10-27-email-purpose-and-revision-are-auth-boundaries.md); real SQLite/HTTP/WebSocket `auth-link-boundary.test.ts` |
 | CAPTCHA validates only a client string's format or skips missing GraphQL proof | [Server policy, signed action/expiry and bounded replay](../inbox/2026-10-27-captcha-proof-must-be-server-issued.md); `packages/server/tests/captcha*.test.ts`, `qc:captcha` |
 | Historical CSS audit findings disappear while the lock is unchanged | [Reconcile official backport ranges with actual consumers](../inbox/2026-10-27-reconcile-advisory-backports-with-locked-consumers.md); locked-path and Tailwind/parser regressions |

docs/quality/OAUTH_IDENTITY.md

@@ -0,0 +1,100 @@
+# OAuth identity and account linking
+
+This is the Core OAuth portion of #184. The separate
+[email-link lifecycle](AUTH_LINK_LIFECYCLE.md) defines token purpose and session
+revision. These are local implementation contracts, not deployed-provider
+acceptance.
+
+## Provider proof
+
+| Provider | Stable identity | Proof required for a new account or automatic email link |
+| --- | --- | --- |
+| Google | Authenticated userinfo `sub`, matching Passport's parsed ID | Exact boolean `email_verified`, plus Gmail address or a nonempty hosted-domain claim |
+| GitHub | Authenticated profile ID | Exactly one primary email with boolean `verified: true` from authenticated `/user/emails` metadata |
+| LinkedIn | ID-token subject matching authenticated userinfo subject, under issuer `https://www.linkedin.com` | Userinfo email and exact boolean `email_verified: true` |
+
+[Google's guidance](https://developers.google.com/identity/gsi/web/guides/verify-google-id-token)
+distinguishes Gmail/Workspace authority from third-party addresses whose
+ownership may have changed since verification. Core applies the conservative
+authority requirement to both account creation and automatic linking. It uses
+the existing server-side authorization-code strategy and authenticated
+userinfo, not a client-submitted ID token.
+
+[GitHub documents](https://docs.github.com/en/rest/users/emails#list-email-addresses-for-the-authenticated-user)
+the authenticated email endpoint and required `user:email` scope. Core fetches
+that metadata directly and retains verification/primary flags. Missing,
+unverified or ambiguous primary candidates cannot establish a new link. There
+is no fallback to the public profile email. The bounded request uses
+`per_page=100`; pagination beyond that response is not added. Absent proof fails
+closed rather than guessing ownership.
+
+[LinkedIn documents](https://learn.microsoft.com/en-us/linkedin/consumer/integrations/self-serve/sign-in-with-linkedin-v2)
+the issuer, userinfo endpoint and optional boolean verification field. The
+nine-argument OIDC callback exposes the authenticated ID-token profile. Core
+fetches userinfo safely and compares both subjects before using its email claim.
+The library's own userinfo loader is explicitly disabled: its malformed-JSON
+handler references an undefined callback. This authenticates a provider account,
+not a person's legal identity.
+
+GitHub and LinkedIn profile requests use fixed HTTPS endpoints, reject redirects,
+require successful JSON responses, enforce a five-second deadline and cap each
+body at 64 KiB. Errors fail authentication without returning provider response
+bodies or tokens. Google retains its existing parsed authenticated userinfo path.
+
+All configured providers use session-bound state. Google/GitHub enable the
+existing Passport state store; LinkedIn retains its default session store.
+Only configured providers are constructed; a configured ID without a secret
+fails startup. The GitHub type package incorrectly narrows the runtime state
+option to string; the localized cast preserves boolean `true`. Actual strategy
+tests verify state rejection before token exchange and single-use consumption.
+
+## Linking and recovery
+
+An existing provider-and-subject binding is authoritative for account selection.
+Changing or omitting its email cannot switch the user to another account.
+Disabled bound accounts fail before email fallback. New bindings require the
+proof above and match only account email, never username. Disabled, unverified or ambiguous
+case-folded email matches fail. Verified provider proof cannot attach the mailbox
+owner to a preregistered local account whose earlier password/session remains
+controlled by someone else. Both helper return and HTTP callback recheck the
+active account and the authenticated session revision; they cannot upgrade an
+old snapshot after reset or reactivation.
+
+Existing accounts and provider bindings are retained without a schema migration.
+This does not retrospectively prove how an old link was established. An existing
+bound subject can continue signing in when its email is absent or unverified.
+For a new unproved binding, existing password or email sign-in/recovery methods
+remain available; no implicit merge or account transfer occurs. Ordinary local
+accounts currently have no implemented verification/linking flow that upgrades
+them for automatic OAuth linking. Unverified local accounts therefore cannot add
+a new OAuth binding in this change. A future explicit recovery/linking flow must
+establish ownership and handle prior credentials/sessions; merely setting a
+verification flag is not a recovery procedure. Administrator review of
+a disputed historical binding remains a separate action. Old stored provider
+credentials are not deleted by this change; new callbacks neither persist
+access/refresh tokens or raw profiles nor log token-bearing redirects.
+
+New users and provider bindings commit together on a dedicated connection to
+the persistent SQLite authentication file. A link failure rolls back the user.
+The transaction cannot join a caller's unrelated transaction. In-process writes
+are serialized per file; bounded asynchronous SQLite busy retries avoid parking
+the native worker pool. The database transaction/uniqueness constraint also
+protects competing processes. Password hashing happens before acquiring the
+write lock. In-memory injected databases are not an OAuth production target.
+
+## Evidence and limits
+
+`oauth-profiles.test.ts` covers exact boolean claims, authority, missing and
+multiple email candidates, issuer and subject mismatch. `oauth-strategies.test.ts`
+drives the actual Passport authorization/callback paths with synthetic token
+exchange and profile responses. `oauth-identity.test.ts` uses owned temporary
+SQLite files, actual strategy callbacks and a local HTTP completion route. It
+covers collisions, subject continuity, disabled/reactivated accounts, concurrent
+creation, link rollback, unrelated transaction rollback, revision races, token
+issuance and absence of credential logging.
+
+Synthetic provider fixtures do not test live credentials, provider availability,
+consent, TLS, signature-key rotation or hosted D1. Tokens in the browser callback
+URL retain the existing application contract; removing server logging and adding
+no-store/no-referrer does not claim elimination of all browser-history exposure.
+Final exact-head review, CI and production browser results belong in the PR.

packages/server/src/auth/oauth-account.ts

@@ -0,0 +1,90 @@
+import sqlite3 from 'sqlite3';
+import bcrypt from 'bcryptjs';
+import { randomUUID } from 'node:crypto';
+import { setTimeout as delay } from 'node:timers/promises';
+import { validateOAuthIdentity, type OAuthIdentity } from './oauth-identity.js';
+
+const pending = new Map<string, Promise<void>>();
+
+async function serializeWrite<T>(filename: string, write: () => Promise<T>): Promise<T> {
+  const previous = pending.get(filename) ?? Promise.resolve();
+  let release!: () => void;
+  const current = new Promise<void>(resolve => { release = resolve; });
+  pending.set(filename, current);
+  await previous;
+  try { return await write(); }
+  finally {
+    release();
+    if (pending.get(filename) === current) pending.delete(filename);
+  }
+}
+
+export async function resolveOAuthAccount(database: sqlite3.Database, identity: OAuthIdentity): Promise<{ id: string; authVersion: number }> {
+  validateOAuthIdentity(identity);
+  const files = await new Promise<Array<{ name: string; file: string }>>((resolve, reject) => {
+    database.all('PRAGMA database_list', (error, rows: Array<{ name: string; file: string }>) => error ? reject(error) : resolve(rows));
+  });
+  const filename = files.find(file => file.name === 'main')?.file;
+  if (!filename) throw new Error('OAuth account linking requires a persistent authentication database');
+  return serializeWrite(filename, async () => {
+    const passwordHash = await bcrypt.hash(randomUUID(), 10);
+    const connection = await new Promise<sqlite3.Database>((resolve, reject) => {
+      const opened = new sqlite3.Database(filename, sqlite3.OPEN_READWRITE, error => error ? reject(error) : resolve(opened));
+    });
+    connection.configure('busyTimeout', 0);
+    const run = (sql: string, values: unknown[] = []) => new Promise<void>((resolve, reject) => {
+      connection.run(sql, values, error => error ? reject(error) : resolve());
+    });
+    const get = <T>(sql: string, values: unknown[] = []) => new Promise<T | undefined>((resolve, reject) => {
+      connection.get(sql, values, (error, row: T) => error ? reject(error) : resolve(row));
+    });
+    const writeBoundary = async (sql: string) => {
+      const deadline = Date.now() + 5000;
+      while (true) {
+        try { await run(sql); return; }
+        catch (error) {
+          if ((error as { code?: string }).code !== 'SQLITE_BUSY' || Date.now() >= deadline) throw error;
+          await delay(10);
+        }
+      }
+    };
+    let transaction = false;
+    try {
+      await run('PRAGMA foreign_keys = ON');
+      await writeBoundary('BEGIN IMMEDIATE');
+      transaction = true;
+      const bound = await get<{ id: string; authVersion: number; isActive: number }>(
+        'SELECT u.id, u.authVersion, u.isActive FROM oauth_providers op JOIN users u ON u.id = op.userId WHERE op.provider = ? AND op.providerId = ?',
+        [identity.provider, identity.providerId]);
+      let account = bound;
+      if (bound && !bound.isActive) throw new Error('Account unavailable');
+      if (!account) {
+        if (!identity.email || identity.emailVerified !== true || identity.canLinkByEmail !== true) {
+          throw new Error('Verified provider email required; use an existing sign-in method');
+        }
+        const matches = await get<{ count: number }>('SELECT count(*) AS count FROM users WHERE lower(email) = ?', [identity.email]);
+        if (matches && matches.count > 1) throw new Error('Account unavailable');
+        const existing = await get<{ id: string; authVersion: number; isActive: number; isEmailVerified: number }>(
+          'SELECT id, authVersion, isActive, isEmailVerified FROM users WHERE lower(email) = ?', [identity.email]);
+        account = existing;
+        if (account && !account.isActive) throw new Error('Account unavailable');
+        if (existing && existing.isEmailVerified !== 1) throw new Error('Existing account recovery required');
+        const now = new Date().toISOString();
+        if (!account) {
+          const id = randomUUID();
+          await run('INSERT INTO users (id, email, username, name, role, passwordHash, createdAt, updatedAt, isActive, isEmailVerified) VALUES (?, ?, ?, ?, ?, ?, ?, ?, 1, 1)',
+            [id, identity.email, `oauth_${randomUUID()}`, identity.name, 'USER', passwordHash, now, now]);
+          account = { id, authVersion: 0, isActive: 1 };
+        }
+        await run('INSERT INTO oauth_providers (id, userId, provider, providerId, email, name, createdAt, updatedAt) VALUES (?, ?, ?, ?, ?, ?, ?, ?)',
+          [randomUUID(), account.id, identity.provider, identity.providerId, identity.email, identity.name, now, now]);
+      }
+      await writeBoundary('COMMIT');
+      transaction = false;
+      return { id: account.id, authVersion: account.authVersion };
+    } finally {
+      try { if (transaction) await run('ROLLBACK'); }
+      finally { await new Promise<void>((resolve, reject) => connection.close(error => error ? reject(error) : resolve())); }
+    }
+  });
+}

packages/server/src/auth/oauth-callback.ts

@@ -0,0 +1,21 @@
+import type { RequestHandler } from 'express';
+import { generateToken } from '../utils/auth.js';
+import { sqliteAuthStore } from './sqlite-auth.js';
+
+export const oauthLoginCallback: RequestHandler = async (req, res) => {
+  const authenticated = req.user as { id?: string; authVersion?: number } | undefined;
+  if (!authenticated?.id) { res.status(401).json({ error: 'Authentication failed' }); return; }
+  try {
+    const user = await sqliteAuthStore.findUserById(authenticated.id);
+    if (!user || user.authVersion !== authenticated.authVersion) {
+      res.status(401).json({ error: 'Authentication failed' });
+      return;
+    }
+    const token = generateToken(user.id, user.email, user.role, user.authVersion);
+    res.setHeader('Cache-Control', 'no-store');
+    res.setHeader('Referrer-Policy', 'no-referrer');
+    res.redirect(`${process.env.CLIENT_URL || 'http://localhost:3127'}/login?token=${encodeURIComponent(token)}`);
+  } catch {
+    res.status(401).json({ error: 'Authentication failed' });
+  }
+};