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) <noreply@anthropic.com>
This commit is contained in:
isra el
2026-08-02 14:29:31 +03:00
parent 7347f2b730
commit fdc1609946
3 changed files with 170 additions and 8 deletions

View File

@@ -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
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=

View File

@@ -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')
})
})
})

View File

@@ -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(
{