From 98e41441e90def647eb70ab99403fa15a765508a Mon Sep 17 00:00:00 2001 From: Anthony Ettinger Date: Tue, 27 Oct 2026 03:38:16 +0000 Subject: [PATCH] fix(api): never trust the CoinPay email on Sign in with CoinPay CoinPay does not verify email addresses, so its userinfo email proves nothing. The callback stored it in the unique users.email column and set email_verified from `info.email_verified ?? !!info.email`, which treated an omitted flag as verified. A CoinPay account could squat an address before its owner signed up (their signup then 409'd), and a CoinPay login whose email matched an existing account failed with a unique-violation 500. Now the account is resolved by sub only; a new CoinPay account never takes an email another account holds and is never marked verified; a password signup can reclaim an address held only by an unverified, password-less CoinPay account. Co-Authored-By: Claude Opus 5.5 --- services/api/src/coinpay-login.test.ts | 81 ++++++++++++++++++++++++++ services/api/src/coinpay-login.ts | 70 ++++++++++++++++++++++ services/api/src/db.ts | 5 ++ services/api/src/index.ts | 19 +++--- 4 files changed, 166 insertions(+), 9 deletions(-) create mode 100644 services/api/src/coinpay-login.test.ts create mode 100644 services/api/src/coinpay-login.ts diff --git a/services/api/src/coinpay-login.test.ts b/services/api/src/coinpay-login.test.ts new file mode 100644 index 0000000..ee2fb91 --- /dev/null +++ b/services/api/src/coinpay-login.test.ts @@ -0,0 +1,81 @@ +import { describe, expect, it } from 'vitest'; +import { coinpayEmail, releasableEmailHolder, resolveCoinpayUser, type CoinpayUser } from './coinpay-login.js'; + +function fakeDb(seed: CoinpayUser[]) { + const rows = [...seed]; + let n = 0; + return { + rows, + deps: { + userByCoinpaySub: async (sub: string) => rows.find((r) => r.coinpay_sub === sub) ?? null, + userByEmail: async (email: string) => rows.find((r) => r.email === email) ?? null, + createUser: async (u: { id: string; authMethod: string; coinpaySub: string; email: string | null; emailVerified: boolean }) => { + if (u.email && rows.some((r) => r.email === u.email)) throw new Error('duplicate key value violates unique constraint "users_email_key"'); + rows.push({ id: u.id, auth_method: u.authMethod, coinpay_sub: u.coinpaySub, email: u.email, email_verified: u.emailVerified ? 1 : 0 }); + }, + newId: () => `new-${++n}`, + }, + }; +} + +const victim: CoinpayUser = { + id: 'victim', auth_method: 'password', coinpay_sub: null, email: 'victim@example.com', email_verified: 1, password_hash: 'hash', +}; + +describe('resolveCoinpayUser', () => { + it('does not hand a CoinPay login the account of a victim with the same email', async () => { + const { deps, rows } = fakeDb([victim]); + const user = await resolveCoinpayUser(deps, 'attacker-sub', { sub: 'attacker-sub', email: 'victim@example.com', email_verified: true }); + expect(user.id).not.toBe('victim'); + expect(user.email).toBeNull(); + expect(user.email_verified).toBe(0); + expect(rows.find((r) => r.id === 'victim')).toEqual(victim); + }); + + it('does not link to an account bound to another CoinPay sub', async () => { + const other: CoinpayUser = { id: 'other', auth_method: 'coinpay', coinpay_sub: 'sub-a', email: 'a@example.com', email_verified: 0 }; + const { deps } = fakeDb([other]); + const user = await resolveCoinpayUser(deps, 'sub-b', { email: 'a@example.com' }); + expect(user.id).not.toBe('other'); + expect(user.coinpay_sub).toBe('sub-b'); + }); + + it('returns the account bound to the sub', async () => { + const mine: CoinpayUser = { id: 'mine', auth_method: 'coinpay', coinpay_sub: 'sub-1', email: 'x@example.com', email_verified: 0 }; + const { deps } = fakeDb([mine, victim]); + expect((await resolveCoinpayUser(deps, 'sub-1', { email: 'victim@example.com' })).id).toBe('mine'); + }); + + it('never marks a CoinPay email verified, even when userinfo claims it or omits the flag', async () => { + const { deps, rows } = fakeDb([]); + await resolveCoinpayUser(deps, 's1', { email: 'one@example.com', email_verified: true }); + await resolveCoinpayUser(deps, 's2', { email: 'two@example.com' }); + expect(rows.map((r) => [r.email, r.email_verified])).toEqual([['one@example.com', 0], ['two@example.com', 0]]); + }); + + it('falls back to no email when it loses a race for the address', async () => { + const { deps, rows } = fakeDb([]); + const racing = { ...deps, userByEmail: async () => null }; + rows.push({ ...victim }); + const user = await resolveCoinpayUser(racing, 's3', { email: 'victim@example.com' }); + expect(user.email).toBeNull(); + }); +}); + +describe('releasableEmailHolder', () => { + it('releases only an unverified, password-less CoinPay account', () => { + expect(releasableEmailHolder({ id: 'c', auth_method: 'coinpay', coinpay_sub: 's', email: 'e@x.io', email_verified: 0, password_hash: null })).toBe(true); + expect(releasableEmailHolder({ id: 'c', auth_method: 'coinpay', coinpay_sub: 's', email: 'e@x.io', email_verified: 1, password_hash: null })).toBe(false); + expect(releasableEmailHolder(victim)).toBe(false); + expect(releasableEmailHolder({ ...victim, email_verified: 0 })).toBe(false); + expect(releasableEmailHolder(null)).toBe(false); + }); +}); + +describe('coinpayEmail', () => { + it('trims and rejects junk', () => { + expect(coinpayEmail({ email: ' a@b.co ' })).toBe('a@b.co'); + expect(coinpayEmail({ email: 'nope' })).toBeNull(); + expect(coinpayEmail({})).toBeNull(); + }); +}); diff --git a/services/api/src/coinpay-login.ts b/services/api/src/coinpay-login.ts new file mode 100644 index 0000000..b361f05 --- /dev/null +++ b/services/api/src/coinpay-login.ts @@ -0,0 +1,70 @@ +/** + * Which TronBrowser account a "Sign in with CoinPay" callback lands in. + * + * CoinPay does not verify email addresses: anyone can open a CoinPay account + * under someone else's address, and userinfo reports email_verified=false (or + * omits it). So the only identity CoinPay proves is its `sub`. + * + * - The account bound to this sub is the one we sign into. Nothing else is. + * - A new CoinPay account never takes an email another account already holds, + * and its email is never marked verified, whatever CoinPay says. + * - Because that email is unproven, a password signup for the same address + * may take it back from a CoinPay-only account that never verified it + * (see `releasableEmailHolder`), so a squatter cannot block the real owner. + */ + +export interface CoinpayUser { + id: string; + auth_method: string; + coinpay_sub: string | null; + email: string | null; + email_verified: number; + password_hash?: string | null; +} + +export interface CoinpayLoginDeps { + userByCoinpaySub(sub: string): Promise; + userByEmail(email: string): Promise; + createUser(u: { id: string; authMethod: string; coinpaySub: string; email: string | null; emailVerified: boolean }): Promise; + newId(): string; +} + +/** The CoinPay userinfo email, trimmed, or null. Never treated as verified. */ +export function coinpayEmail(info: Record): string | null { + const e = typeof info.email === 'string' ? info.email.trim() : ''; + return e && e.length <= 254 && e.includes('@') ? e : null; +} + +export async function resolveCoinpayUser( + deps: CoinpayLoginDeps, + sub: string, + info: Record, +): Promise { + const bound = await deps.userByCoinpaySub(sub); + if (bound) return bound; + + // Never link to, and never collide with, an account that already has this + // address: keep the CoinPay account separate and email-less instead. + let email = coinpayEmail(info); + if (email && (await deps.userByEmail(email))) email = null; + + const id = deps.newId(); + try { + await deps.createUser({ id, authMethod: 'coinpay', coinpaySub: sub, email, emailVerified: false }); + } catch (err) { + // Lost a race for the unique email: create the account without it. + if (!email) throw err; + email = null; + await deps.createUser({ id, authMethod: 'coinpay', coinpaySub: sub, email: null, emailVerified: false }); + } + return { id, auth_method: 'coinpay', coinpay_sub: sub, email, email_verified: 0 }; +} + +/** + * True when `holder` keeps `email` only on CoinPay's say-so: a CoinPay-only + * account (no password) whose email was never verified by us. A password + * signup for that address may release it from the holder. + */ +export function releasableEmailHolder(holder: CoinpayUser | null): boolean { + return !!holder && holder.auth_method === 'coinpay' && !holder.password_hash && !holder.email_verified; +} diff --git a/services/api/src/db.ts b/services/api/src/db.ts index cbda2cd..b0c2b45 100644 --- a/services/api/src/db.ts +++ b/services/api/src/db.ts @@ -84,6 +84,11 @@ export async function createUser(u: { }); } +/** Drop an unverified email from an account (see releasableEmailHolder). */ +export async function releaseEmail(userId: string): Promise { + await db().execute({ sql: "UPDATE users SET email = NULL, updated_at = datetime('now') WHERE id = ? AND email_verified = 0", args: [userId] }); +} + export async function setEmailVerified(userId: string): Promise { await db().execute({ sql: 'UPDATE users SET email_verified = 1, updated_at = datetime(\'now\') WHERE id = ?', args: [userId] }); } diff --git a/services/api/src/index.ts b/services/api/src/index.ts index 7eaf707..fd9c7a8 100644 --- a/services/api/src/index.ts +++ b/services/api/src/index.ts @@ -5,7 +5,7 @@ import { cors } from 'hono/cors'; import { getCookie, setCookie, deleteCookie } from 'hono/cookie'; import { createUser, userByCoinpaySub, userByEmail, createSession, userBySession, - deleteSession, putEmailToken, consumeEmailToken, setEmailVerified, + deleteSession, putEmailToken, consumeEmailToken, setEmailVerified, releaseEmail, getSettings, putSettings, type User, } from './db.js'; import { token, uuid, hashPassword, verifyPassword, SESSION_TTL, EMAIL_TOKEN_TTL } from './auth.js'; @@ -16,6 +16,7 @@ import { dnsRoutes } from './dns.js'; import { tronRelay } from './mcp/tron.js'; import { safeRedirect } from './redirect.js'; import { extLoginTarget } from './ext-login.js'; +import { resolveCoinpayUser, releasableEmailHolder } from './coinpay-login.js'; import { assertDatabaseUrl, db } from './db.js'; import { pushService } from './push/routes.js'; import { createCrawlGate, crawlCheck } from './crawl-gate.js'; @@ -157,13 +158,9 @@ app.get('/api/auth/coinpay/callback', async (c) => { const sub = info.sub || info.id || tok.sub; if (!sub) return c.text('coinpay userinfo missing subject', 502); - const emailVerified = info.email_verified ?? !!info.email; - let user = await userByCoinpaySub(String(sub)); - if (!user) { - const id = uuid(); - await createUser({ id, authMethod: 'coinpay', coinpaySub: String(sub), email: info.email ?? null, emailVerified }); - user = { id, auth_method: 'coinpay', coinpay_sub: String(sub), email: info.email ?? null, email_verified: emailVerified ? 1 : 0 }; - } + // Bound to the CoinPay sub only. CoinPay never verifies emails, so its email + // never links to an existing account and is never marked verified. + const user = await resolveCoinpayUser({ userByCoinpaySub, userByEmail, createUser, newId: uuid }, String(sub), info); deleteCookie(c, 'cp_state'); deleteCookie(c, 'cp_redirect'); const r = await startSession(c, user.id, redirect); return redirect ? r : c.redirect(`${APP_URL}/?signedin=1`); @@ -177,7 +174,11 @@ app.post('/api/auth/signup', async (c) => { // The shared profile fields (first + last name required), checked before any account exists. const invalid = signupProfileErrors(body); if (invalid) return c.json(invalid, 400); - if (await userByEmail(email)) return c.json({ error: 'email already registered' }, 409); + const holder = await userByEmail(email); + if (holder && !releasableEmailHolder(holder)) return c.json({ error: 'email already registered' }, 409); + // A CoinPay-only account holding this address on CoinPay's unverified word + // gives it up; the new account still has to verify it by email. + if (holder) await releaseEmail(holder.id); const id = uuid(); await createUser({ id, authMethod: 'password', email, passwordHash: hashPassword(password), emailVerified: false }); await saveSignup(profileStore(), id, body, email);