From dd2771870b68c275ee5684e1a457c1cbf7d891fa Mon Sep 17 00:00:00 2001 From: Qisthi Ramadhani Date: Sat, 7 Nov 2026 11:05:41 +0700 Subject: [PATCH] Google sign-in links only verified emails and claims unverified accounts loginExternal() links an existing account only when the provider says the email is verified. If the local account itself was never verified, its password is replaced and its sessions revoked before the provider's user signs in. The callback flashes a message when linking is refused. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_016ZbncuXneGAhsgTaizfjVi --- app/actions/auth/controller.tsx | 20 ++++++++++++++------ app/repositories/user.repository.ts | 4 ++++ app/services/auth.service.ts | 18 +++++++++++++++++- test/services/auth.service.test.ts | 26 +++++++++++++++++++++++++- 4 files changed, 60 insertions(+), 8 deletions(-) diff --git a/app/actions/auth/controller.tsx b/app/actions/auth/controller.tsx index ca565f9..7120de6 100644 --- a/app/actions/auth/controller.tsx +++ b/app/actions/auth/controller.tsx @@ -8,6 +8,7 @@ import { redirect } from 'remix/response/redirect' import { Session } from 'remix/session' import { config } from '../../config.ts' +import { DomainError } from '../../services/errors.ts' import { requireGuest, setSessionUser } from '../../middleware/auth.ts' import { getCsrfToken } from '../../middleware/csrf.ts' import { rateLimit } from '../../middleware/rate-limit.ts' @@ -170,12 +171,19 @@ export default function authController(rateLimits: boolean) { context.session.flash('error', 'Google account has no email address') return redirect(routes.auth.loginPage.href(), 303) } - let user = await context.services.auth.loginExternal({ - email: result.profile.email, - name: result.profile.name ?? null, - avatar: result.profile.picture ?? null, - emailVerified: result.profile.email_verified ?? false, - }) + let user + try { + user = await context.services.auth.loginExternal({ + email: result.profile.email, + name: result.profile.name ?? null, + avatar: result.profile.picture ?? null, + emailVerified: result.profile.email_verified ?? false, + }) + } catch (error) { + if (!(error instanceof DomainError)) throw error + context.session.flash('error', error.message) + return redirect(routes.auth.loginPage.href(), 303) + } let session = completeAuth(context) setSessionUser(session, user) return redirect(routes.app.dashboard.href(), 303) diff --git a/app/repositories/user.repository.ts b/app/repositories/user.repository.ts index 30215f7..9ccd6eb 100644 --- a/app/repositories/user.repository.ts +++ b/app/repositories/user.repository.ts @@ -103,6 +103,10 @@ export class UserRepository { this.db.run(`update users set ${sets.join(', ')} where id = ?`, params) } + markVerified(id: string): void { + this.db.run('update users set is_verified = 1, updated_at = ? where id = ?', [Date.now(), id]) + } + updatePassword(id: string, hashedPassword: string): void { this.db.run('update users set password = ?, updated_at = ? where id = ?', [ hashedPassword, diff --git a/app/services/auth.service.ts b/app/services/auth.service.ts index 5e597dd..3a85006 100644 --- a/app/services/auth.service.ts +++ b/app/services/auth.service.ts @@ -69,7 +69,14 @@ export class AuthService { return toAuthUser(user) } - /** Login/registration via a verified external identity provider (e.g. Google). */ + /** + * Login/registration via an external identity provider (e.g. Google). + * + * An existing local account is only linked when the provider says the email is verified. + * If that local account was never verified, nobody has proven they own the address, so + * whoever registered it may not be the provider's user: the old password is replaced with an + * unusable one and its sessions are revoked before the provider's user is signed in. + */ async loginExternal(profile: { email: string name?: string | null @@ -78,6 +85,15 @@ export class AuthService { }): Promise { let email = profile.email.trim().toLowerCase() let user = this.users.findByEmail(email) + if (user && !profile.emailVerified) { + throw errors.unauthorized('Sign in with your password to use this account') + } + if (user && !user.is_verified) { + this.users.updatePassword(user.id, await hashPassword(randomUUID())) + this.users.markVerified(user.id) + this.sessions.deleteByUserId(user.id) + user = this.users.findById(user.id)! + } if (!user) { user = this.users.create({ id: randomUUID(), diff --git a/test/services/auth.service.test.ts b/test/services/auth.service.test.ts index a8b3c1e..01b167b 100644 --- a/test/services/auth.service.test.ts +++ b/test/services/auth.service.test.ts @@ -79,10 +79,34 @@ describe('AuthService', () => { it('external login creates the account once', async () => { let a = await auth.loginExternal({ email: 'g@x.com', name: 'G', emailVerified: true }) - let b = await auth.loginExternal({ email: 'G@x.com' }) + let b = await auth.loginExternal({ email: 'G@x.com', emailVerified: true }) assert.equal(a.id, b.id) assert.equal(a.is_verified, true) }) + + it('external login does not link an account when the provider has not verified the email', async () => { + await auth.register({ name: 'A', email: 'a@x.com', password: 'secret1' }) + await assert.rejects( + auth.loginExternal({ email: 'a@x.com', emailVerified: false }), + (e: unknown) => e instanceof DomainError && e.kind === 'unauthorized', + ) + }) + + it('external login takes over an unverified local account from whoever registered it', async () => { + let squatter = await auth.register({ name: 'S', email: 'owner@x.com', password: 'squatter1' }) + assert.equal(squatter.is_verified, false) + repos.sessions.upsert('squatter-session', squatter.id, '[{},{}]', Date.now() + 10000) + + let owner = await auth.loginExternal({ email: 'owner@x.com', name: 'Owner', emailVerified: true }) + + assert.equal(owner.id, squatter.id) + assert.equal(owner.is_verified, true) + assert.equal(repos.sessions.countByUserId(owner.id), 0) + await assert.rejects( + auth.login({ identifier: 'owner@x.com', password: 'squatter1' }), + (e: unknown) => e instanceof DomainError && e.kind === 'unauthorized', + ) + }) }) describe('UserService', () => {