diff --git a/.agents/inbox/2026-10-27-oauth-linking-needs-provider-proof-and-atomicity.md b/.agents/inbox/2026-10-27-oauth-linking-needs-provider-proof-and-atomicity.md new file mode 100644 index 00000000..87f06c08 --- /dev/null +++ b/.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. diff --git a/.agents/issues/INDEX.md b/.agents/issues/INDEX.md index 5cca8f79..58a1dbed 100644 --- a/.agents/issues/INDEX.md +++ b/.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 | diff --git a/.agents/issues/security-audit-index.md b/.agents/issues/security-audit-index.md index 9c5e04c1..51005c6b 100644 --- a/.agents/issues/security-audit-index.md +++ b/.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 | diff --git a/docs/quality/OAUTH_IDENTITY.md b/docs/quality/OAUTH_IDENTITY.md new file mode 100644 index 00000000..52586946 --- /dev/null +++ b/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. diff --git a/packages/server/src/auth/oauth-account.ts b/packages/server/src/auth/oauth-account.ts new file mode 100644 index 00000000..07da88e5 --- /dev/null +++ b/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>(); + +async function serializeWrite(filename: string, write: () => Promise): Promise { + const previous = pending.get(filename) ?? Promise.resolve(); + let release!: () => void; + const current = new Promise(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>((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((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((resolve, reject) => { + connection.run(sql, values, error => error ? reject(error) : resolve()); + }); + const get = (sql: string, values: unknown[] = []) => new Promise((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((resolve, reject) => connection.close(error => error ? reject(error) : resolve())); } + } + }); +} diff --git a/packages/server/src/auth/oauth-callback.ts b/packages/server/src/auth/oauth-callback.ts new file mode 100644 index 00000000..257e5eb9 --- /dev/null +++ b/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' }); + } +}; diff --git a/packages/server/src/auth/oauth-identity.ts b/packages/server/src/auth/oauth-identity.ts new file mode 100644 index 00000000..6b2caebb --- /dev/null +++ b/packages/server/src/auth/oauth-identity.ts @@ -0,0 +1,63 @@ +export interface OAuthIdentity { + provider: string; + providerId: string; + email?: string; + name: string; + avatar?: string; + emailVerified: boolean; + canLinkByEmail: boolean; +} + +function subject(value: unknown): string { + if (typeof value !== 'string' || !value.trim() || value.length > 255) throw new Error('Invalid provider identity'); + return value; +} + +function email(value: unknown): string | undefined { + if (typeof value !== 'string' || value.length > 254 || !/^[^\s@]+@[^\s@]+\.[^\s@]+$/.test(value)) return undefined; + return value.toLowerCase(); +} + +function name(value: unknown, fallback: string): string { + return typeof value === 'string' && value.trim() ? value.trim().slice(0, 200) : fallback; +} + +export function googleIdentity(profile: any): OAuthIdentity { + const claims = profile?._json; + const providerId = subject(claims?.sub); + if (providerId !== profile.id) throw new Error('Provider subject mismatch'); + const address = email(claims.email); + const emailVerified = claims.email_verified === true; + const authoritative = address?.endsWith('@gmail.com') || + (typeof claims.hd === 'string' && claims.hd.trim().length > 0); + return { provider: 'google', providerId, email: address, + name: name(profile.displayName, 'Google user'), + emailVerified, canLinkByEmail: emailVerified && !!authoritative }; +} + +export function githubIdentity(profile: any): OAuthIdentity { + const providerId = subject(profile?.id); + const candidates = Array.isArray(profile.emails) + ? profile.emails.filter((entry: any) => entry.primary === true && entry.verified === true && email(entry.value)) : []; + const address = candidates.length === 1 ? email(candidates[0].value) : undefined; + return { provider: 'github', providerId, email: address, + name: name(profile.displayName || profile.username, 'GitHub user'), + emailVerified: !!address, canLinkByEmail: !!address }; +} + +export function linkedinIdentity(issuer: string, userInfo: any, idProfile: any): OAuthIdentity { + if (issuer !== 'https://www.linkedin.com') throw new Error('Invalid provider issuer'); + const providerId = subject(idProfile?.id); + const claims = userInfo?._json; + if (subject(claims?.sub) !== providerId || userInfo.id !== providerId) throw new Error('Provider subject mismatch'); + const address = email(claims.email); + const emailVerified = claims.email_verified === true; + return { provider: 'linkedin', providerId, email: address, + name: name(userInfo.displayName, 'LinkedIn user'), emailVerified, canLinkByEmail: emailVerified }; +} + +export function validateOAuthIdentity(identity: OAuthIdentity): void { + if (!['google', 'github', 'linkedin'].includes(identity.provider)) throw new Error('Invalid provider identity'); + subject(identity.providerId); + if (identity.email !== undefined && email(identity.email) !== identity.email) throw new Error('Invalid provider email'); +} diff --git a/packages/server/src/auth/oauth-profile-fetch.ts b/packages/server/src/auth/oauth-profile-fetch.ts new file mode 100644 index 00000000..50e8f4ff --- /dev/null +++ b/packages/server/src/auth/oauth-profile-fetch.ts @@ -0,0 +1,40 @@ +import { githubIdentity, linkedinIdentity, type OAuthIdentity } from './oauth-identity.js'; + +async function providerJSON(url: string, accessToken: string): Promise { + try { + const response = await fetch(url, { + headers: { Authorization: `Bearer ${accessToken}`, Accept: 'application/json' }, + redirect: 'error', signal: AbortSignal.timeout(5000), + }); + if (!response.ok || !response.body || !response.headers.get('content-type')?.includes('application/json')) { + throw new Error('Invalid provider response'); + } + const reader = response.body.getReader(); + const chunks: Uint8Array[] = []; + let size = 0; + try { + while (true) { + const { value, done } = await reader.read(); + if (done) break; + size += value.byteLength; + if (size > 65536) throw new Error('Provider response too large'); + chunks.push(value); + } + } finally { await reader.cancel().catch(() => {}); } + return JSON.parse(Buffer.concat(chunks).toString('utf8')); + } catch { throw new Error('Provider profile unavailable'); } +} + +export async function fetchGitHubIdentity(accessToken: string): Promise { + const profile = await providerJSON('https://api.github.com/user', accessToken); + const emails = await providerJSON('https://api.github.com/user/emails?per_page=100', accessToken); + if (!profile || !Array.isArray(emails)) throw new Error('Invalid provider profile'); + const id = typeof profile.id === 'number' && Number.isSafeInteger(profile.id) ? String(profile.id) : profile.id; + return githubIdentity({ id, displayName: profile.name, username: profile.login, + emails: emails.map(entry => ({ value: entry?.email, primary: entry?.primary, verified: entry?.verified })) }); +} + +export async function fetchLinkedInIdentity(issuer: string, idProfile: unknown, accessToken: string): Promise { + const profile = await providerJSON('https://api.linkedin.com/v2/userinfo', accessToken); + return linkedinIdentity(issuer, { id: profile?.sub, displayName: profile?.name, _json: profile }, idProfile); +} diff --git a/packages/server/src/auth/oauth-strategies.ts b/packages/server/src/auth/oauth-strategies.ts index 1b04f49a..0575e79a 100644 --- a/packages/server/src/auth/oauth-strategies.ts +++ b/packages/server/src/auth/oauth-strategies.ts @@ -3,127 +3,67 @@ import { Strategy as GoogleStrategy } from 'passport-google-oauth20'; import { Strategy as OpenIDConnectStrategy } from 'passport-openidconnect'; import { Strategy as GitHubStrategy } from 'passport-github2'; import { sqliteAuthStore } from './sqlite-auth.js'; +import { googleIdentity, type OAuthIdentity } from './oauth-identity.js'; +import { fetchGitHubIdentity, fetchLinkedInIdentity } from './oauth-profile-fetch.js'; -export function configureOAuthStrategies() { - passport.use( - new GoogleStrategy( - { - clientID: process.env.GOOGLE_CLIENT_ID || '', - clientSecret: process.env.GOOGLE_CLIENT_SECRET || '', - callbackURL: process.env.GOOGLE_CALLBACK_URL || 'https://localhost:4128/auth/google/callback', - }, - async (_accessToken, _refreshToken, profile, done) => { - try { - const email = profile.emails?.[0]?.value; - if (!email) { - return done(new Error('No email found in Google profile')); - } +type Complete = (error: Error | null, user?: any) => void; - const user = await sqliteAuthStore.findOrCreateUserFromOAuth({ - provider: 'google', - providerId: profile.id, - email: email, - name: profile.displayName || email.split('@')[0], - avatar: profile.photos?.[0]?.value, - accessToken: _accessToken, - refreshToken: _refreshToken, - profile: profile._json, - }); - - return done(null, user); - } catch (error) { - return done(error as Error); - } - } - ) - ); - - passport.use('linkedin', - new OpenIDConnectStrategy( - { - issuer: 'https://www.linkedin.com/oauth', - authorizationURL: 'https://www.linkedin.com/oauth/v2/authorization', - tokenURL: 'https://www.linkedin.com/oauth/v2/accessToken', - userInfoURL: 'https://api.linkedin.com/v2/userinfo', - clientID: process.env.LINKEDIN_CLIENT_ID || '', - clientSecret: process.env.LINKEDIN_CLIENT_SECRET || '', - callbackURL: process.env.LINKEDIN_CALLBACK_URL || 'http://localhost:4127/auth/linkedin/callback', - scope: ['openid', 'profile', 'email'], - }, - async (_issuer: string, _profile: any, done: any) => { - try { - console.log('🔍 LinkedIn profile received:', JSON.stringify(_profile, null, 2)); - - const email = _profile.email || _profile.emails?.[0]?.value || _profile._json?.email; - if (!email) { - console.error('❌ No email in profile. Profile keys:', Object.keys(_profile)); - return done(new Error('No email found in LinkedIn profile')); - } - - const user = await sqliteAuthStore.findOrCreateUserFromOAuth({ - provider: 'linkedin', - providerId: _profile.sub || _profile.id, - email: email, - name: _profile.name || _profile.displayName || email.split('@')[0], - avatar: _profile.picture || _profile.photos?.[0]?.value, - accessToken: '', - refreshToken: '', - profile: _profile, - }); - - return done(null, user); - } catch (error) { - console.error('❌ LinkedIn OAuth error:', error); - return done(error as Error); - } - } - ) - ); - - passport.use( - new GitHubStrategy( - { - clientID: process.env.GITHUB_CLIENT_ID || '', - clientSecret: process.env.GITHUB_CLIENT_SECRET || '', - callbackURL: process.env.GITHUB_CALLBACK_URL || 'https://localhost:4128/auth/github/callback', - scope: ['user:email'], - }, - async (_accessToken: string, _refreshToken: string, profile: any, done: any) => { - try { - const email = profile.emails?.[0]?.value; - if (!email) { - return done(new Error('No email found in GitHub profile')); - } +async function authenticateIdentity(readIdentity: () => OAuthIdentity | Promise, done: Complete) { + try { + done(null, await sqliteAuthStore.findOrCreateUserFromOAuth(await readIdentity())); + } catch (error) { + done(error as Error); + } +} - const user = await sqliteAuthStore.findOrCreateUserFromOAuth({ - provider: 'github', - providerId: profile.id, - email: email, - name: profile.displayName || profile.username || email.split('@')[0], - avatar: profile.photos?.[0]?.value, - accessToken: _accessToken, - refreshToken: _refreshToken, - profile: profile._json, - }); +export function configureOAuthStrategies(authenticator: Pick = passport) { + if (process.env.GOOGLE_CLIENT_ID) { + if (!process.env.GOOGLE_CLIENT_SECRET) throw new Error('Google OAuth client secret required'); + authenticator.use(new GoogleStrategy({ + clientID: process.env.GOOGLE_CLIENT_ID, + clientSecret: process.env.GOOGLE_CLIENT_SECRET, + callbackURL: process.env.GOOGLE_CALLBACK_URL || 'https://localhost:4128/auth/google/callback', + state: true, + }, (_accessToken, _refreshToken, profile, done) => { + void authenticateIdentity(() => googleIdentity(profile), done); + })); + } - return done(null, user); - } catch (error) { - return done(error as Error); - } - } - ) - ); + if (process.env.LINKEDIN_CLIENT_ID) { + if (!process.env.LINKEDIN_CLIENT_SECRET) throw new Error('LinkedIn OAuth client secret required'); + authenticator.use('linkedin', new OpenIDConnectStrategy({ + issuer: 'https://www.linkedin.com', + authorizationURL: 'https://www.linkedin.com/oauth/v2/authorization', + tokenURL: 'https://www.linkedin.com/oauth/v2/accessToken', + userInfoURL: 'https://api.linkedin.com/v2/userinfo', + clientID: process.env.LINKEDIN_CLIENT_ID, + clientSecret: process.env.LINKEDIN_CLIENT_SECRET, + callbackURL: process.env.LINKEDIN_CALLBACK_URL || 'http://localhost:4127/auth/linkedin/callback', + scope: ['openid', 'profile', 'email'], + skipUserProfile: true, + }, (issuer: string, _userInfo: any, idProfile: any, _context: unknown, _idToken: string, + accessToken: string, _refreshToken: string, _params: unknown, done: Complete) => { + void authenticateIdentity(() => fetchLinkedInIdentity(issuer, idProfile, accessToken), done); + })); + } - passport.serializeUser((user: any, done) => { - done(null, user.id); - }); + if (process.env.GITHUB_CLIENT_ID) { + if (!process.env.GITHUB_CLIENT_SECRET) throw new Error('GitHub OAuth client secret required'); + authenticator.use(new GitHubStrategy({ + clientID: process.env.GITHUB_CLIENT_ID, + clientSecret: process.env.GITHUB_CLIENT_SECRET, + callbackURL: process.env.GITHUB_CALLBACK_URL || 'https://localhost:4128/auth/github/callback', + scope: ['user:email'], + skipUserProfile: true, + state: true as any, + }, (accessToken: string, _refreshToken: string, _profile: any, done: Complete) => { + void authenticateIdentity(() => fetchGitHubIdentity(accessToken), done); + })); + } - passport.deserializeUser(async (id: string, done) => { - try { - const user = await sqliteAuthStore.findUserById(id); - done(null, user); - } catch (error) { - done(error); - } + authenticator.serializeUser((user: any, done) => done(null, user.id)); + authenticator.deserializeUser(async (id: string, done) => { + try { done(null, await sqliteAuthStore.findUserById(id)); } + catch (error) { done(error); } }); } diff --git a/packages/server/src/auth/sqlite-auth.ts b/packages/server/src/auth/sqlite-auth.ts index 9fa5f416..5b03eda6 100644 --- a/packages/server/src/auth/sqlite-auth.ts +++ b/packages/server/src/auth/sqlite-auth.ts @@ -1,4 +1,6 @@ import {randomBytes} from 'node:crypto'; +import { resolveOAuthAccount } from './oauth-account.js'; +import type { OAuthIdentity } from './oauth-identity.js'; import sqlite3 from 'sqlite3'; import bcrypt from 'bcryptjs'; import { v4 as uuidv4 } from 'uuid'; @@ -1208,57 +1210,12 @@ class SQLiteAuthStore { }); } - async findOrCreateUserFromOAuth(oauthData: { - provider: string; - providerId: string; - email: string; - name: string; - avatar?: string; - accessToken?: string; - refreshToken?: string; - profile?: any; - }): Promise { + async findOrCreateUserFromOAuth(identity: OAuthIdentity): Promise { await this.initialize(); - const db = await this.getDb(); - - const existingUser = await this.findUserByOAuthProvider(oauthData.provider, oauthData.providerId); - if (existingUser) { - await this.updateOAuthProvider(existingUser.id, oauthData); - return existingUser; - } - - const userByEmail = await this.findUserByEmailOrUsername(oauthData.email); - if (userByEmail) { - await this.linkOAuthProvider(userByEmail.id, oauthData); - return userByEmail; - } - - const userId = uuidv4(); - const now = new Date().toISOString(); - const username = oauthData.email.split('@')[0] + '_' + Math.random().toString(36).substring(7); - const passwordHash = await bcrypt.hash(uuidv4(), 10); - - return new Promise((resolve, reject) => { - db.serialize(() => { - db.run(`INSERT INTO users (id, email, username, name, role, passwordHash, createdAt, updatedAt, isActive, isEmailVerified) - VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?)`, - [userId, oauthData.email.toLowerCase(), username, oauthData.name, 'USER', passwordHash, now, now, 1, 1], - async (err) => { - if (err) { - reject(err); - return; - } - - try { - await this.linkOAuthProvider(userId, oauthData); - const newUser = await this.findUserById(userId); - resolve(newUser!); - } catch (linkErr) { - reject(linkErr); - } - }); - }); - }); + const snapshot = await resolveOAuthAccount(await this.getDb(), identity); + const user = await this.findUserById(snapshot.id); + if (!user || user.authVersion !== snapshot.authVersion) throw new Error('Account changed during authentication'); + return user; } async linkOAuthProvider(userId: string, oauthData: { diff --git a/packages/server/src/index.ts b/packages/server/src/index.ts index a35e1ece..e4750502 100644 --- a/packages/server/src/index.ts +++ b/packages/server/src/index.ts @@ -30,7 +30,7 @@ import { driver, NEO4J_URI } from './db.js'; import { sqliteAuthStore } from './auth/sqlite-auth.js'; import { ensureGraphIdentity } from './auth/graph-identity.js'; import { configureOAuthStrategies } from './auth/oauth-strategies.js'; -import { generateToken } from './utils/auth.js'; +import { oauthLoginCallback } from './auth/oauth-callback.js'; import { createTlsConfig, validateTlsConfig, type TlsConfig } from './config/tls.js'; import { emailService } from './auth/email-service.js'; import { exec } from 'child_process'; @@ -661,17 +661,7 @@ async function startServer() { app.get( '/auth/google/callback', passport.authenticate('google', { failureRedirect: `${process.env.CLIENT_URL || 'http://localhost:3127'}/login?error=google` }), - (req, res) => { - console.log('🔐 Google OAuth callback received'); // eslint-disable-line no-console - const user = req.user as any; - console.log('👤 User from OAuth:', user?.email || 'No user'); // eslint-disable-line no-console - const token = generateToken(user.id, user.email, user.role, user.authVersion); - console.log('🎫 Generated token:', token?.substring(0, 20) + '...'); // eslint-disable-line no-console - const clientUrl = process.env.CLIENT_URL || 'http://localhost:3127'; - const redirectUrl = `${clientUrl}/login?token=${token}`; - console.log('↪️ Redirecting to:', redirectUrl); // eslint-disable-line no-console - res.redirect(redirectUrl); - } + oauthLoginCallback ); app.get('/auth/linkedin', passport.authenticate('linkedin', { scope: ['openid', 'profile', 'email'] })); @@ -679,17 +669,7 @@ async function startServer() { app.get( '/auth/linkedin/callback', passport.authenticate('linkedin', { failureRedirect: `${process.env.CLIENT_URL || 'http://localhost:3127'}/login?error=linkedin` }), - (req, res) => { - console.log('🔐 LinkedIn OAuth callback received'); // eslint-disable-line no-console - const user = req.user as any; - console.log('👤 User from OAuth:', user?.email || 'No user'); // eslint-disable-line no-console - const token = generateToken(user.id, user.email, user.role, user.authVersion); - console.log('🎫 Generated token:', token?.substring(0, 20) + '...'); // eslint-disable-line no-console - const clientUrl = process.env.CLIENT_URL || 'http://localhost:3127'; - const redirectUrl = `${clientUrl}/login?token=${token}`; - console.log('↪️ Redirecting to:', redirectUrl); // eslint-disable-line no-console - res.redirect(redirectUrl); - } + oauthLoginCallback ); app.get('/auth/github', passport.authenticate('github', { scope: ['user:email'] })); @@ -697,17 +677,7 @@ async function startServer() { app.get( '/auth/github/callback', passport.authenticate('github', { failureRedirect: `${process.env.CLIENT_URL || 'http://localhost:3127'}/login?error=github` }), - (req, res) => { - console.log('🔐 GitHub OAuth callback received'); // eslint-disable-line no-console - const user = req.user as any; - console.log('👤 User from OAuth:', user?.email || 'No user'); // eslint-disable-line no-console - const token = generateToken(user.id, user.email, user.role, user.authVersion); - console.log('🎫 Generated token:', token?.substring(0, 20) + '...'); // eslint-disable-line no-console - const clientUrl = process.env.CLIENT_URL || 'http://localhost:3127'; - const redirectUrl = `${clientUrl}/login?token=${token}`; - console.log('↪️ Redirecting to:', redirectUrl); // eslint-disable-line no-console - res.redirect(redirectUrl); - } + oauthLoginCallback ); const magicLinkCorsOptions = { diff --git a/packages/server/src/types/passport-openidconnect.d.ts b/packages/server/src/types/passport-openidconnect.d.ts index a8e47f2b..c64e6dd0 100644 --- a/packages/server/src/types/passport-openidconnect.d.ts +++ b/packages/server/src/types/passport-openidconnect.d.ts @@ -11,13 +11,20 @@ declare module 'passport-openidconnect' { clientSecret: string; callbackURL: string; scope?: string[]; + skipUserProfile?: boolean; } export type VerifyCallback = (error: Error | null, user?: any, info?: any) => void; export type VerifyFunction = ( issuer: string, - profile: any, + userInfo: any, + idProfile: any, + context: unknown, + idToken: string, + accessToken: string, + refreshToken: string, + params: unknown, done: VerifyCallback ) => void; diff --git a/packages/server/tests/oauth-identity.test.ts b/packages/server/tests/oauth-identity.test.ts new file mode 100644 index 00000000..178a852d --- /dev/null +++ b/packages/server/tests/oauth-identity.test.ts @@ -0,0 +1,217 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import { mkdtemp, rm } from 'node:fs/promises'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import sqlite3 from 'sqlite3'; +import express from 'express'; +import { createServer } from 'node:http'; +import { sqliteAuthStore } from '../src/auth/sqlite-auth'; +import { oauthLoginCallback } from '../src/auth/oauth-callback'; +import { verifyToken } from '../src/utils/auth'; +import passport from 'passport'; +import { configureOAuthStrategies } from '../src/auth/oauth-strategies'; +import { sqliteAuthResolvers } from '../src/resolvers/sqlite-auth'; +import { resolveActiveAuthUser } from '../src/middleware/auth'; + +let directory: string; +let database: sqlite3.Database; +const store = sqliteAuthStore as unknown as { db: sqlite3.Database | null; initialized: boolean }; +const execute = (sql: string, values: unknown[] = []) => new Promise((resolve, reject) => { + database.run(sql, values, error => error ? reject(error) : resolve()); +}); +const row = (sql: string, values: unknown[] = []) => new Promise((resolve, reject) => { + database.get(sql, values, (error, result: T) => error ? reject(error) : resolve(result)); +}); +const identity = (changes = {}) => ({ provider: 'github', providerId: 'owned-provider-subject', + email: 'owned-oauth@example.invalid', name: 'Owned OAuth user', emailVerified: true, canLinkByEmail: true, ...changes }); + +beforeEach(async () => { + vi.stubEnv('NODE_ENV', 'test'); + vi.stubEnv('ADMIN_EMAIL', 'oauth-admin@example.invalid'); + vi.stubEnv('ADMIN_PASSWORD', 'OwnedOAuthFixture!2026'); + vi.stubEnv('CLIENT_URL', 'http://localhost:3127'); + vi.spyOn(console, 'log').mockImplementation(() => {}); + directory = await mkdtemp(join(tmpdir(), 'graphdone-oauth-')); + database = await new Promise((resolve, reject) => { + const connection = new sqlite3.Database(join(directory, 'auth.db'), error => error ? reject(error) : resolve(connection)); + }); + store.db = database; store.initialized = false; + await sqliteAuthStore.initialize(); +}); + +afterEach(async () => { + await new Promise((resolve, reject) => database.close(error => error ? reject(error) : resolve())); + store.db = null; store.initialized = false; + await rm(directory, { recursive: true, force: true }); + vi.restoreAllMocks(); + vi.unstubAllEnvs(); + vi.unstubAllGlobals(); +}); + +describe('OAuth account identity boundary', () => { + it('rejects unverified email collisions without linking an existing account', async () => { + await expect(sqliteAuthStore.findOrCreateUserFromOAuth(identity({ email: 'oauth-admin@example.invalid', emailVerified: false }))) + .rejects.toThrow(); + expect(await row('SELECT count(*) AS count FROM oauth_providers')).toEqual({ count: 0 }); + }); + + it('does not relink a disabled provider subject through a different active email', async () => { + const first = await sqliteAuthStore.findOrCreateUserFromOAuth(identity()); + await sqliteAuthStore.updateUserStatus(first.id, false); + await expect(sqliteAuthStore.findOrCreateUserFromOAuth(identity({ email: 'oauth-admin@example.invalid' }))).rejects.toThrow(); + expect(await row('SELECT userId FROM oauth_providers WHERE providerId = ?', ['owned-provider-subject'])) + .toEqual({ userId: first.id }); + }); + + it('rolls back a new user if provider linking fails', async () => { + await execute("CREATE TRIGGER reject_owned_link BEFORE INSERT ON oauth_providers BEGIN SELECT RAISE(ABORT, 'Owned link failure'); END"); + await expect(sqliteAuthStore.findOrCreateUserFromOAuth(identity())).rejects.toThrow(); + expect(await row('SELECT count(*) AS count FROM users WHERE email = ?', ['owned-oauth@example.invalid'])).toEqual({ count: 0 }); + }); + + it('preserves a bound subject when its email changes or is absent', async () => { + const first = await sqliteAuthStore.findOrCreateUserFromOAuth(identity()); + for (const address of ['oauth-admin@example.invalid', undefined]) { + const user = await sqliteAuthStore.findOrCreateUserFromOAuth(identity({ email: address, emailVerified: false, canLinkByEmail: false })); + expect(user.id).toBe(first.id); + expect(user.email).toBe(first.email); + } + expect(await row('SELECT count(*) AS count FROM users WHERE email = ?', [first.email])).toEqual({ count: 1 }); + }); + + it('requires exact verified and authoritative claims for a new binding', async () => { + for (const changes of [{ emailVerified: 'true' }, { canLinkByEmail: false }, { email: undefined }, { providerId: '' }]) { + await expect(sqliteAuthStore.findOrCreateUserFromOAuth(identity(changes))).rejects.toThrow(); + } + expect(await row('SELECT count(*) AS count FROM oauth_providers')).toEqual({ count: 0 }); + }); + + it('links a verified email without confusing a matching username for ownership', async () => { + const admin = await sqliteAuthStore.findUserByEmailOrUsername('oauth-admin@example.invalid'); + expect(admin!.isEmailVerified).toBe(true); + await execute('UPDATE users SET username = ? WHERE id = ?', ['owned-oauth@example.invalid', admin!.id]); + const created = await sqliteAuthStore.findOrCreateUserFromOAuth(identity()); + expect(created.id).not.toBe(admin!.id); + const linked = await sqliteAuthStore.findOrCreateUserFromOAuth(identity({ providerId: 'owned-admin-subject', email: admin!.email })); + expect(linked.id).toBe(admin!.id); + }); + + it('rejects linking to a preregistered unverified account with another password and session', async () => { + const account = await sqliteAuthStore.createUser({ email: 'owned-oauth@example.invalid', username: 'preregistered', + password: 'PreRegisteredPassword!2026', name: 'Unverified registration' }); + expect(account.isEmailVerified).toBe(false); + const login = await sqliteAuthResolvers.Mutation.login(null, + { input: { emailOrUsername: account.email, password: 'PreRegisteredPassword!2026' } }, { req: { ip: '127.0.0.77' } } as any); + expect((await resolveActiveAuthUser(`Bearer ${login.token}`))?.userId).toBe(account.id); + await expect(sqliteAuthStore.findOrCreateUserFromOAuth(identity())).rejects.toThrow('Existing account recovery required'); + expect(await row('SELECT count(*) AS count FROM oauth_providers WHERE userId = ?', [account.id])).toEqual({ count: 0 }); + expect((await sqliteAuthStore.findUserById(account.id))!.isEmailVerified).toBe(false); + }); + + it('rejects verified email linking to a disabled account', async () => { + const admin = await sqliteAuthStore.findUserByEmailOrUsername('oauth-admin@example.invalid'); + await sqliteAuthStore.updateUserStatus(admin!.id, false); + await expect(sqliteAuthStore.findOrCreateUserFromOAuth(identity({ email: admin!.email }))).rejects.toThrow('Account unavailable'); + }); + + it('serializes simultaneous first logins without orphan users or duplicate bindings', async () => { + const users = await Promise.all(Array.from({ length: 8 }, () => sqliteAuthStore.findOrCreateUserFromOAuth(identity()))); + expect(new Set(users.map(user => user.id)).size).toBe(1); + expect(await row('SELECT count(*) AS count FROM users WHERE email = ?', ['owned-oauth@example.invalid'])).toEqual({ count: 1 }); + expect(await row('SELECT count(*) AS count FROM oauth_providers')).toEqual({ count: 1 }); + }); + + it('rejects a revision changed after the identity transaction instead of minting a newer session', async () => { + const find = sqliteAuthStore.findUserById.bind(sqliteAuthStore); + vi.spyOn(sqliteAuthStore, 'findUserById').mockImplementationOnce(async id => { + await execute('UPDATE users SET authVersion = authVersion + 1 WHERE id = ?', [id]); + return find(id); + }); + await expect(sqliteAuthStore.findOrCreateUserFromOAuth(identity())).rejects.toThrow('Account changed'); + }); + + it('waits outside an unrelated transaction without joining its rollback', async () => { + await execute('BEGIN IMMEDIATE'); + const original = sqlite3.Database.prototype.run; + let attempted = false; + vi.spyOn(sqlite3.Database.prototype, 'run').mockImplementation(function (this: sqlite3.Database, ...args: any[]) { + if (this !== database && args[0] === 'BEGIN IMMEDIATE') attempted = true; + return original.apply(this, args as any); + }); + const authentication = sqliteAuthStore.findOrCreateUserFromOAuth(identity()); + try { + await vi.waitFor(() => expect(attempted).toBe(true)); + await execute('UPDATE users SET name = ? WHERE email = ?', ['Uncommitted change', 'oauth-admin@example.invalid']); + } finally { await execute('ROLLBACK'); } + const user = await authentication; + expect(await sqliteAuthStore.findUserById(user.id)).not.toBeNull(); + expect((await sqliteAuthStore.findUserByEmailOrUsername('oauth-admin@example.invalid'))!.name).not.toBe('Uncommitted change'); + }); + + it('issues the actual HTTP callback token without logging credentials and rejects stale snapshots', async () => { + let authenticated = await sqliteAuthStore.findOrCreateUserFromOAuth(identity()); + const app = express(); + app.get('/callback', (req, _res, next) => { req.user = authenticated; next(); }, oauthLoginCallback); + const server = createServer(app); + await new Promise(resolve => server.listen(0, '127.0.0.1', resolve)); + const address = server.address(); + if (!address || typeof address === 'string') throw new Error('Owned callback server has no address'); + const url = `http://127.0.0.1:${address.port}/callback`; + const log = vi.mocked(console.log); + log.mockClear(); + const error = vi.spyOn(console, 'error').mockImplementation(() => {}); + const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}); + try { + const result = await fetch(url, { redirect: 'manual' }); + expect(result.status).toBe(302); + expect(result.headers.get('cache-control')).toBe('no-store'); + expect(result.headers.get('referrer-policy')).toBe('no-referrer'); + const redirect = new URL(result.headers.get('location')!); + expect(redirect.origin).toBe('http://localhost:3127'); + expect(verifyToken(redirect.searchParams.get('token')!)).toMatchObject({ userId: authenticated.id, authVersion: 0 }); + expect(log).not.toHaveBeenCalled(); expect(error).not.toHaveBeenCalled(); expect(warn).not.toHaveBeenCalled(); + await sqliteAuthStore.updateUserStatus(authenticated.id, false); + expect((await fetch(url, { redirect: 'manual' })).status).toBe(401); + await sqliteAuthStore.updateUserStatus(authenticated.id, true); + expect((await fetch(url, { redirect: 'manual' })).status).toBe(401); + authenticated = await sqliteAuthStore.findOrCreateUserFromOAuth(identity()); + expect((await fetch(url, { redirect: 'manual' })).status).toBe(302); + } finally { + await new Promise((resolve, reject) => server.close(error => error ? reject(error) : resolve())); + } + }); + + it.each(['google', 'github', 'linkedin'])('enforces verified linking and subject continuity in the %s callback', async provider => { + for (const name of ['GOOGLE', 'GITHUB', 'LINKEDIN']) { + vi.stubEnv(`${name}_CLIENT_ID`, name.toLowerCase() === provider ? 'owned-client' : ''); + vi.stubEnv(`${name}_CLIENT_SECRET`, 'owned-client-secret'); + } + const authenticator = new passport.Passport(); + configureOAuthStrategies(authenticator); + const strategy = (authenticator as any)._strategy(provider); + const authenticate = (subject: string, address: string | undefined, verified: boolean) => new Promise((resolve, reject) => { + const profile = { id: subject, displayName: 'Owned provider user', + _json: { sub: subject, email: address, email_verified: verified, hd: 'example.invalid' }, + emails: address ? [{ value: address, primary: true, verified }] : [], + }; + vi.stubGlobal('fetch', vi.fn(async (url: string) => { + await new Promise(resolve => setImmediate(resolve)); + const data = url.includes('/user/emails') ? (address ? [{ email: address, primary: true, verified }] : []) : + { ...profile._json, id: subject, name: profile.displayName }; + return new Response(JSON.stringify(data), { headers: { 'content-type': 'application/json' } }); + })); + const done = (error: Error | null, user: unknown) => error ? reject(error) : resolve(user); + if (provider === 'linkedin') strategy._verify('https://www.linkedin.com', profile, { id: subject }, {}, '', '', '', {}, done); + else strategy._verify('owned-access-token', 'owned-refresh-token', profile, done); + }); + await expect(authenticate('new-unproved-subject', 'oauth-admin@example.invalid', false)).rejects.toThrow(); + await expect(authenticate('new-missing-email', undefined, false)).rejects.toThrow(); + const user = await authenticate('owned-subject', 'owned-oauth@example.invalid', true); + expect((await authenticate('owned-subject', undefined, false)).id).toBe(user.id); + expect((await authenticate('owned-subject', 'oauth-admin@example.invalid', true)).id).toBe(user.id); + expect(await row('SELECT accessToken, refreshToken, profile FROM oauth_providers WHERE provider = ?', [provider])) + .toEqual({ accessToken: null, refreshToken: null, profile: null }); + await sqliteAuthStore.updateUserStatus(user.id, false); + await expect(authenticate('owned-subject', 'oauth-admin@example.invalid', true)).rejects.toThrow('Account unavailable'); + }); +}); diff --git a/packages/server/tests/oauth-profiles.test.ts b/packages/server/tests/oauth-profiles.test.ts new file mode 100644 index 00000000..7c34952f --- /dev/null +++ b/packages/server/tests/oauth-profiles.test.ts @@ -0,0 +1,50 @@ +import { describe, expect, it } from 'vitest'; +import { googleIdentity, githubIdentity, linkedinIdentity } from '../src/auth/oauth-identity'; + +describe('provider email evidence', () => { + it.each([true, false, undefined, 'true'])('requires exact Google verified=true (%s)', verified => { + const result = googleIdentity({ id: 'google-sub', _json: { sub: 'google-sub', email: 'Owned@Gmail.com', email_verified: verified } }); + expect(result.email).toBe('owned@gmail.com'); + expect(result.canLinkByEmail).toBe(verified === true); + }); + + it('requires Google authority for third-party addresses', () => { + const profile = { id: 'google-sub', _json: { sub: 'google-sub', email: 'owned@example.invalid', email_verified: true } }; + expect(googleIdentity(profile).canLinkByEmail).toBe(false); + expect(googleIdentity({ ...profile, _json: { ...profile._json, hd: 'example.invalid' } }).canLinkByEmail).toBe(true); + expect(() => googleIdentity({ ...profile, id: 'different-sub' })).toThrow('subject mismatch'); + }); + + it('selects the sole verified primary GitHub email from authenticated email metadata', () => { + const result = githubIdentity({ id: '1234', _json: { email: 'unproved@example.invalid' }, emails: [ + { value: 'unproved@example.invalid', primary: false, verified: false }, + { value: 'Owned@example.invalid', primary: true, verified: true }, + ] }); + expect(result.email).toBe('owned@example.invalid'); + expect(result.canLinkByEmail).toBe(true); + }); + + it.each([ + undefined, + [{ value: 'owned@example.invalid' }], + [{ value: 'owned@example.invalid', primary: true, verified: false }], + [{ value: 'owned@example.invalid', primary: true, verified: 'true' }], + [{ value: 'owned@example.invalid', primary: false, verified: true }], + [{ value: 'a@example.invalid', primary: true, verified: true }, { value: 'b@example.invalid', primary: true, verified: true }], + ])('does not infer GitHub verification from incomplete or ambiguous emails', emails => { + expect(githubIdentity({ id: '1234', emails }).canLinkByEmail).toBe(false); + }); + + it.each([true, false, undefined, 'true'])('requires exact LinkedIn userinfo email verification (%s)', verified => { + const result = linkedinIdentity('https://www.linkedin.com', + { id: 'linkedin-sub', _json: { sub: 'linkedin-sub', email: 'owned@example.invalid', email_verified: verified } }, { id: 'linkedin-sub' }); + expect(result.canLinkByEmail).toBe(verified === true); + }); + + it('binds LinkedIn userinfo to the authenticated issuer and ID-token subject', () => { + const profile = { id: 'linkedin-sub', _json: { sub: 'linkedin-sub', email_verified: true } }; + expect(() => linkedinIdentity('https://www.linkedin.com/oauth', profile, { id: 'linkedin-sub' })).toThrow('issuer'); + expect(() => linkedinIdentity('https://www.linkedin.com', profile, { id: 'different-sub' })).toThrow('subject mismatch'); + expect(linkedinIdentity('https://www.linkedin.com', profile, { id: 'linkedin-sub' }).email).toBeUndefined(); + }); +}); diff --git a/packages/server/tests/oauth-strategies.test.ts b/packages/server/tests/oauth-strategies.test.ts new file mode 100644 index 00000000..f31966ba --- /dev/null +++ b/packages/server/tests/oauth-strategies.test.ts @@ -0,0 +1,116 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import passport from 'passport'; +import { configureOAuthStrategies } from '../src/auth/oauth-strategies'; +import { sqliteAuthStore } from '../src/auth/sqlite-auth'; + +beforeEach(() => { + for (const provider of ['GOOGLE', 'GITHUB', 'LINKEDIN']) { + vi.stubEnv(`${provider}_CLIENT_ID`, ''); + vi.stubEnv(`${provider}_CLIENT_SECRET`, ''); + } +}); +afterEach(() => { vi.restoreAllMocks(); vi.unstubAllEnvs(); vi.unstubAllGlobals(); }); + +function strategyFor(provider: string) { + vi.stubEnv(`${provider.toUpperCase()}_CLIENT_ID`, 'owned-client'); + vi.stubEnv(`${provider.toUpperCase()}_CLIENT_SECRET`, 'owned-client-secret'); + const authenticator = new passport.Passport(); + configureOAuthStrategies(authenticator); + return (authenticator as any)._strategy(provider); +} + +async function authorize(strategy: any) { + const session = {}; + const url = await new Promise((resolve, reject) => { + strategy.redirect = resolve; + strategy.error = reject; + strategy.authenticate({ query: {}, session, headers: { host: 'localhost' }, connection: {} }, {}); + }); + const state = new URL(url).searchParams.get('state'); + expect(state).toBeTruthy(); + return { session, state }; +} + +function callback(strategy: any, session: object, state: string | null) { + return new Promise<{ user?: unknown; failed?: boolean }>((resolve, reject) => { + strategy.success = (user: unknown) => resolve({ user }); + strategy.fail = () => resolve({ failed: true }); + strategy.error = reject; + strategy.authenticate({ query: { code: 'owned-code', state }, session, headers: { host: 'localhost' }, connection: {} }, {}); + }); +} + +describe('actual Passport provider flows', () => { + it.each(['google', 'github', 'linkedin'])('requires session-bound state before %s token exchange', async provider => { + const strategy = strategyFor(provider); + const { session } = await authorize(strategy); + const exchange = vi.fn(); + strategy._oauth2.getOAuthAccessToken = exchange; + expect(await callback(strategy, session, 'wrong-state')).toEqual({ failed: true }); + expect(exchange).not.toHaveBeenCalled(); + }); + + it.each(['google', 'github', 'linkedin'])('loads and checks real %s profile parsing before account lookup', async provider => { + const strategy = strategyFor(provider); + const { session, state } = await authorize(strategy); + const store = vi.spyOn(sqliteAuthStore, 'findOrCreateUserFromOAuth').mockResolvedValue({ id: 'owned-user' } as any); + const claims = { iss: 'https://www.linkedin.com', sub: 'owned-subject', aud: 'owned-client', + exp: Math.floor(Date.now() / 1000) + 300, iat: Math.floor(Date.now() / 1000) }; + const idToken = ['eyJhbGciOiJSUzI1NiJ9', Buffer.from(JSON.stringify(claims)).toString('base64url'), 'owned-signature'].join('.'); + strategy._oauth2.getOAuthAccessToken = vi.fn((_code, _options, done) => setImmediate(() => done(null, 'owned-access-token', 'owned-refresh-token', { id_token: idToken }))); + const request = vi.fn((url: string, _token: string, done: (...args: any[]) => void) => { + const data = url.includes('/user/emails') ? [{ email: 'owned@gmail.com', primary: true, verified: true }] : + { ...claims, id: 'owned-subject', email: 'owned@gmail.com', email_verified: true, name: 'Owned user' }; + setImmediate(() => done(null, JSON.stringify(data), {})); + }); + strategy._oauth2.get = request; + const fetchProfile = vi.fn(async (url: string) => { + await new Promise(resolve => setImmediate(resolve)); + const data = url.includes('/user/emails') ? [{ email: 'owned@gmail.com', primary: true, verified: true }] : + { ...claims, id: 'owned-subject', email: 'owned@gmail.com', email_verified: true, name: 'Owned user' }; + return new Response(JSON.stringify(data), { headers: { 'content-type': 'application/json' } }); + }); + vi.stubGlobal('fetch', fetchProfile); + expect(await callback(strategy, session, state)).toEqual({ user: { id: 'owned-user' } }); + expect(store).toHaveBeenCalledTimes(1); + expect(store.mock.calls[0][0]).toMatchObject({ provider, providerId: 'owned-subject', email: 'owned@gmail.com', emailVerified: true, canLinkByEmail: true }); + expect(store.mock.calls[0][0]).not.toHaveProperty('accessToken'); + expect(store.mock.calls[0][0]).not.toHaveProperty('refreshToken'); + expect(store.mock.calls[0][0]).not.toHaveProperty('profile'); + if (provider === 'github') expect(fetchProfile.mock.calls.some(([url]) => url === 'https://api.github.com/user/emails?per_page=100')).toBe(true); + if (provider === 'linkedin') expect(fetchProfile.mock.calls.some(([url]) => url === 'https://api.linkedin.com/v2/userinfo')).toBe(true); + if (provider !== 'google') expect(request).not.toHaveBeenCalled(); + expect(await callback(strategy, session, state)).toEqual({ failed: true }); + expect(store).toHaveBeenCalledTimes(1); + }); + + it('fails startup for a partially configured provider without requiring other providers', () => { + vi.stubEnv('GITHUB_CLIENT_ID', 'owned-client'); + expect(() => configureOAuthStrategies(new passport.Passport())).toThrow('GitHub OAuth client secret required'); + }); + + for (const provider of ['github', 'linkedin']) { + it.each(['malformed', 'http-error', 'content-type', 'oversized', 'aborted'])('fails closed on asynchronous %s ' + provider + ' profile responses', async failure => { + const strategy = strategyFor(provider); + const { session, state } = await authorize(strategy); + const store = vi.spyOn(sqliteAuthStore, 'findOrCreateUserFromOAuth'); + const claims = { iss: 'https://www.linkedin.com', sub: 'owned-subject', aud: 'owned-client', + exp: Math.floor(Date.now() / 1000) + 300, iat: Math.floor(Date.now() / 1000) }; + const idToken = ['eyJhbGciOiJSUzI1NiJ9', Buffer.from(JSON.stringify(claims)).toString('base64url'), 'owned-signature'].join('.'); + strategy._oauth2.getOAuthAccessToken = (_code: unknown, _options: unknown, done: (...args: any[]) => void) => + setImmediate(() => done(null, 'owned-access-token', '', { id_token: idToken })); + vi.stubGlobal('fetch', vi.fn(async (_url: string, options: RequestInit) => { + expect(options.redirect).toBe('error'); + expect(options.signal).toBeInstanceOf(AbortSignal); + await new Promise(resolve => setImmediate(resolve)); + if (failure === 'aborted') throw new DOMException('Owned timeout', 'AbortError'); + return new Response(failure === 'oversized' ? 'x'.repeat(65537) : '', { + status: failure === 'http-error' ? 503 : 200, + headers: { 'content-type': failure === 'content-type' ? 'text/html' : 'application/json' }, + }); + })); + await expect(callback(strategy, session, state)).rejects.toThrow('Provider profile unavailable'); + expect(store).not.toHaveBeenCalled(); + }); + } +});