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', () => {