Optional "Sign in with Google" with auto-provisioning, fully config-gated: without GOOGLE_CLIENT_ID/SECRET and OAUTH_REDIRECT_URL the routes 404 and the button is hidden, so deployments without SSO are unaffected. - migration 028: users.google_sub (unique) + email; password_hash nullable for OAuth-only accounts - src/auth/google-oauth.js: lazy google-auth-library, ID-token verify, GOOGLE_ALLOWED_DOMAIN enforcement, requires email_verified === true - auth routes: /auth/google (state-CSRF redirect), /auth/google/callback, /auth/google/enabled; reuses establishSession - web-ui: "Sign in with Google" on the login screen (shown only when enabled), friendly callback error handling - .env.example documents all new vars Security hardening (from review of this + the TOTP work): - resolveGoogleUser links ONLY by google_sub, never by email — a Google login can never seize a pre-existing local account (account-takeover fix) - a Google-linked account with TOTP still requires the second factor (ticket in session, /?mfa=1 step) instead of bypassing it - /login/totp now applies the per-IP login backoff - recovery-code consumption is atomic (WHERE used_at IS NULL + rowCount) - concurrent first-login race on google_sub is caught and re-resolved - tests: google-oauth config helpers + google-link takeover/dedup regression Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
63 lines
3.4 KiB
JavaScript
63 lines
3.4 KiB
JavaScript
// Security regression test for resolveGoogleUser: a Google sign-in must NEVER
|
|
// adopt a pre-existing local account by matching email (that would be account
|
|
// takeover). It links only by google_sub, otherwise provisions a fresh viewer.
|
|
// Skips without TEST_DATABASE_URL.
|
|
import { test } from 'node:test';
|
|
import assert from 'node:assert/strict';
|
|
import { isTestDbConfigured, setupTestDb } from '../helpers/setup-db.js';
|
|
import { hashPassword } from '../../src/auth/passwords.js';
|
|
|
|
const SKIP = !isTestDbConfigured() && 'TEST_DATABASE_URL not set';
|
|
|
|
async function loadResolve() {
|
|
process.env.DATABASE_URL = process.env.TEST_DATABASE_URL;
|
|
return (await import('../../src/routes/auth.js?v=' + Date.now())).resolveGoogleUser;
|
|
}
|
|
|
|
test('a Google login with an email matching a local admin does NOT take over that account', { skip: SKIP }, async () => {
|
|
const pool = await setupTestDb();
|
|
process.env.DATABASE_URL = process.env.TEST_DATABASE_URL;
|
|
try {
|
|
// Pre-existing local admin with a password and the same email the attacker controls.
|
|
const adminId = (await pool.query(
|
|
`INSERT INTO users (username, password_hash, role, email)
|
|
VALUES ('boss', $1, 'admin', 'boss@wilddragon.net') RETURNING id`,
|
|
[await hashPassword('a-real-password-12')])).rows[0].id;
|
|
|
|
const resolveGoogleUser = await loadResolve();
|
|
const user = await resolveGoogleUser({ sub: 'google-attacker-sub', email: 'boss@wilddragon.net', name: 'Not The Boss' });
|
|
|
|
// Must be a brand-new account, NOT the admin.
|
|
assert.notEqual(user.id, adminId, 'Google login must not resolve to the existing admin');
|
|
const row = (await pool.query(`SELECT role, google_sub FROM users WHERE id = $1`, [user.id])).rows[0];
|
|
assert.equal(row.role, 'viewer', 'auto-provisioned account must be a viewer');
|
|
assert.equal(row.google_sub, 'google-attacker-sub');
|
|
// The admin row is untouched (no google_sub linked onto it).
|
|
const admin = (await pool.query(`SELECT google_sub FROM users WHERE id = $1`, [adminId])).rows[0];
|
|
assert.equal(admin.google_sub, null, 'the existing admin must not have been linked');
|
|
} finally { await pool.end(); }
|
|
});
|
|
|
|
test('a returning Google user resolves to the same account by google_sub', { skip: SKIP }, async () => {
|
|
const pool = await setupTestDb();
|
|
process.env.DATABASE_URL = process.env.TEST_DATABASE_URL;
|
|
try {
|
|
const resolveGoogleUser = await loadResolve();
|
|
const first = await resolveGoogleUser({ sub: 'sub-123', email: 'alice@wilddragon.net', name: 'Alice' });
|
|
const second = await resolveGoogleUser({ sub: 'sub-123', email: 'alice@wilddragon.net', name: 'Alice' });
|
|
assert.equal(first.id, second.id, 'same google_sub must map to the same user');
|
|
const count = (await pool.query(`SELECT COUNT(*)::int AS n FROM users WHERE google_sub = 'sub-123'`)).rows[0].n;
|
|
assert.equal(count, 1, 'must not create a duplicate on second login');
|
|
} finally { await pool.end(); }
|
|
});
|
|
|
|
test('username collisions get a numeric suffix', { skip: SKIP }, async () => {
|
|
const pool = await setupTestDb();
|
|
process.env.DATABASE_URL = process.env.TEST_DATABASE_URL;
|
|
try {
|
|
await pool.query(`INSERT INTO users (username, password_hash, role) VALUES ('sam', 'x', 'viewer')`);
|
|
const resolveGoogleUser = await loadResolve();
|
|
const u = await resolveGoogleUser({ sub: 'sub-sam', email: 'sam@wilddragon.net', name: 'Sam' });
|
|
assert.match(u.username, /^sam\d+$/, 'colliding username should get a numeric suffix');
|
|
} finally { await pool.end(); }
|
|
});
|