From fdc1609946b2ae13dfcd85baf31a36bb60f29171 Mon Sep 17 00:00:00 2001 From: isra el Date: Sun, 2 Aug 2026 14:29:31 +0300 Subject: [PATCH] fix(auth): check Google token audience and align reset responses The tokeninfo response was trusted without checking who the token was issued for, so a token minted for any other Google OAuth client was accepted. The audience is now matched against GOOGLE_CLIENT_ID and the verified-email flag is required. The audience check is skipped with a logged warning when the variable is unset, so an unconfigured environment does not lose Google sign-in. Password reset requests now return the same response whether or not the address is registered, an unknown address on reset reports the same error as a bad code, a non-string email no longer reaches the query, and the code is drawn from crypto.randomInt. Co-Authored-By: Claude Opus 5 (1M context) --- api/.env.example | 7 ++- api/src/auth/auth.service.spec.ts | 101 ++++++++++++++++++++++++++++++ api/src/auth/auth.service.ts | 70 ++++++++++++++++++--- 3 files changed, 170 insertions(+), 8 deletions(-) diff --git a/api/.env.example b/api/.env.example index 46ea35d..825335e 100644 --- a/api/.env.example +++ b/api/.env.example @@ -39,4 +39,9 @@ WEBHOOK_AUTO_DISABLE_MIN_FAILURE_RATE=0.50 USE_SMS_QUEUE=false REDIS_URL=redis://localhost:6379 # if queue is enabled, redis url is required -CLOUDFLARE_TURNSTILE_SECRET_KEY=1x0000000000000000000000000000000AA \ No newline at end of file +CLOUDFLARE_TURNSTILE_SECRET_KEY=1x0000000000000000000000000000000AA + +# OAuth client id(s) that Google sign-in tokens must be issued for. Must match +# the web app's NEXT_PUBLIC_GOOGLE_CLIENT_ID. Comma-separate for several clients. +# When unset, the audience is not checked and a warning is logged. +GOOGLE_CLIENT_ID= \ No newline at end of file diff --git a/api/src/auth/auth.service.spec.ts b/api/src/auth/auth.service.spec.ts index 6698800..cfd9c68 100644 --- a/api/src/auth/auth.service.spec.ts +++ b/api/src/auth/auth.service.spec.ts @@ -1,6 +1,7 @@ import { HttpException } from '@nestjs/common' import * as bcrypt from 'bcryptjs' import { createHash } from 'crypto' +import axios from 'axios' import { AuthService } from './auth.service' const sha256 = (value: string) => @@ -575,5 +576,105 @@ describe('AuthService', () => { { attempts: { $exists: false } }, ]) }) + + it('reports an unknown address the same way as a bad code', async () => { + const ctx = build() + ctx.usersService.findOne.mockResolvedValue(null) + + await expect( + ctx.service.resetPassword({ + email: 'nobody@b.com', + otp: '123456', + newPassword: 'Str0ng!pass', + } as any), + ).rejects.toMatchObject({ + response: { error: 'Invalid OTP' }, + status: 400, + }) + }) + + it('does not pass a non-string email to the user lookup', async () => { + const ctx = build() + + await expect( + ctx.service.resetPassword({ + email: { $ne: null } as any, + otp: '123456', + newPassword: 'Str0ng!pass', + } as any), + ).rejects.toThrow(HttpException) + expect(ctx.usersService.findOne).not.toHaveBeenCalled() + }) + }) + + // tokeninfo only proves Google signed the token. Without these checks a token + // minted for any other Google OAuth client would be accepted. + describe('loginWithGoogle', () => { + const ORIGINAL_ENV = process.env.GOOGLE_CLIENT_ID + const OURS = 'our-client-id.apps.googleusercontent.com' + + const stageTokenInfo = (data: any) => { + jest.spyOn(axios, 'get').mockResolvedValue({ data } as any) + } + + beforeEach(() => { + process.env.GOOGLE_CLIENT_ID = OURS + }) + + afterEach(() => { + process.env.GOOGLE_CLIENT_ID = ORIGINAL_ENV + jest.restoreAllMocks() + }) + + it('rejects a token minted for a different OAuth client', async () => { + const ctx = build() + stageTokenInfo({ + aud: 'someone-elses-client.apps.googleusercontent.com', + email: 'victim@example.com', + email_verified: 'true', + sub: 'g1', + }) + + await expect(ctx.service.loginWithGoogle('tok')).rejects.toThrow( + HttpException, + ) + expect(ctx.usersService.findOne).not.toHaveBeenCalled() + }) + + it('rejects a token whose email is not verified', async () => { + const ctx = build() + stageTokenInfo({ + aud: OURS, + email: 'victim@example.com', + email_verified: 'false', + sub: 'g1', + }) + + await expect(ctx.service.loginWithGoogle('tok')).rejects.toThrow( + HttpException, + ) + expect(ctx.usersService.findOne).not.toHaveBeenCalled() + }) + + it('accepts our own audience with a verified email', async () => { + const ctx = build() + stageTokenInfo({ + aud: OURS, + email: 'ada@example.com', + email_verified: 'true', + sub: 'g1', + name: 'Ada', + }) + ctx.usersService.findOne.mockResolvedValue({ + _id: 'user_1', + email: 'ada@example.com', + save: jest.fn().mockResolvedValue(undefined), + toObject: () => ({ _id: 'user_1', email: 'ada@example.com' }), + }) + + const result = await ctx.service.loginWithGoogle('tok') + + expect(result.accessToken).toBe('signed-jwt') + }) }) }) diff --git a/api/src/auth/auth.service.ts b/api/src/auth/auth.service.ts index 2f0e4dc..210ab16 100644 --- a/api/src/auth/auth.service.ts +++ b/api/src/auth/auth.service.ts @@ -82,11 +82,47 @@ export class AuthService { } } + // tokeninfo only proves Google signed the token, not that it was issued for + // this app, so the audience and the verified-email flag are checked here. + private assertGoogleTokenIsForThisApp(tokenInfo: { + aud?: string + email_verified?: boolean | string + }) { + const allowedAudiences = (process.env.GOOGLE_CLIENT_ID ?? '') + .split(',') + .map((value) => value.trim()) + .filter(Boolean) + + if (!allowedAudiences.length) { + // Unset in this environment: log rather than reject, so a missing config + // value cannot take Google sign-in down. Set GOOGLE_CLIENT_ID to enable. + console.error( + 'loginWithGoogle: GOOGLE_CLIENT_ID is not set, skipping audience check', + ) + } else if (!allowedAudiences.includes(tokenInfo.aud)) { + throw new HttpException({ error: 'Unauthorized' }, HttpStatus.UNAUTHORIZED) + } + + if ( + tokenInfo.email_verified !== true && + tokenInfo.email_verified !== 'true' + ) { + throw new HttpException( + { error: 'Google account email is not verified' }, + HttpStatus.UNAUTHORIZED, + ) + } + } + async loginWithGoogle(idToken: string) { const response = await axios.get( - `https://oauth2.googleapis.com/tokeninfo?id_token=${idToken}`, + `https://oauth2.googleapis.com/tokeninfo?id_token=${encodeURIComponent( + idToken, + )}`, ) + this.assertGoogleTokenIsForThisApp(response.data) + const { sub: googleId, name, email, picture } = response.data let user = await this.usersService.findOne({ email }) @@ -168,11 +204,20 @@ export class AuthService { }: RequestResetPasswordInputDTO) { await this.turnstileService.verify(turnstileToken) + // Both branches below return this, so the response says nothing about + // whether the address is registered. + const acceptedResponse = { + message: 'If email is found you will receive a password reset email', + } + + // Guards against a non-string reaching the query as a Mongo operator. + if (typeof email !== 'string') { + return acceptedResponse + } + const user = await this.usersService.findOne({ email }) if (!user) { - return { - message: 'If email is found you will receive a password reset email', - } + return acceptedResponse } // Check if user has requested password reset more than 5 times in the last 24 hours @@ -189,7 +234,7 @@ export class AuthService { ) } - const otp = Math.floor(100000 + Math.random() * 900000).toString() + const otp = randomInt(100000, 1000000).toString() const expiresAt = new Date(Date.now() + 20 * 60 * 1000) const hashedOtp = await bcrypt.hash(otp, 10) @@ -209,13 +254,24 @@ export class AuthService { context: { name: user.name, resetLink, otp }, }) - return { message: 'Password reset email sent' } + return acceptedResponse } async resetPassword({ email, otp, newPassword }: ResetPasswordInputDTO) { + // Matches the other failure paths below so an unknown address is not + // distinguishable from a bad code. + const invalidOtp = new HttpException( + { error: 'Invalid OTP' }, + HttpStatus.BAD_REQUEST, + ) + + if (typeof email !== 'string') { + throw invalidOtp + } + const user = await this.usersService.findOne({ email }) if (!user) { - throw new HttpException({ error: 'User not found' }, HttpStatus.NOT_FOUND) + throw invalidOtp } const latestReset = await this.passwordResetModel.findOne( {