Skip to content

Commit

Permalink
prevent use of common password using the haveibeenpwned password api (#…
Browse files Browse the repository at this point in the history
…958)

* prevent use of common password using the haveibeenpwned password api

* made requested chages
1. Reduced the core logic by extracting it in custom function in auth.server.ts
2. added timeout to the request to skip the check if request takes more than 1s
3. skips the check if request fails
4. added msw mock so that we don't depend upon it in development and testing

* Update app/utils/auth.server.ts

Co-authored-by: Kent C. Dodds <[email protected]>

* requested changes made
1. Added warning in case if the error is not AbortError to get more info about the error
2. updated the msw mock to follow convention of other msw mocks

* renamed the common-password to pwnedpasswords

---------

Co-authored-by: Kent C. Dodds <[email protected]>
  • Loading branch information
AdityaKirad and kentcdodds authored Mar 6, 2025
1 parent 468c5e5 commit 832c24c
Show file tree
Hide file tree
Showing 5 changed files with 72 additions and 4 deletions.
15 changes: 14 additions & 1 deletion app/routes/_auth+/onboarding.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,12 @@ import { z } from 'zod'
import { CheckboxField, ErrorList, Field } from '#app/components/forms.tsx'
import { Spacer } from '#app/components/spacer.tsx'
import { StatusButton } from '#app/components/ui/status-button.tsx'
import { requireAnonymous, sessionKey, signup } from '#app/utils/auth.server.ts'
import {
checkCommonPassword,
requireAnonymous,
sessionKey,
signup,
} from '#app/utils/auth.server.ts'
import { prisma } from '#app/utils/db.server.ts'
import { checkHoneypot } from '#app/utils/honeypot.server.ts'
import { useIsPending } from '#app/utils/misc.tsx'
Expand Down Expand Up @@ -72,6 +77,14 @@ export async function action({ request }: Route.ActionArgs) {
})
return
}
const isCommonPassword = await checkCommonPassword(data.password)
if (isCommonPassword) {
ctx.addIssue({
path: ['password'],
code: 'custom',
message: 'Password is too common',
})
}
}).transform(async (data) => {
if (intent !== null) return { ...data, session: null }

Expand Down
20 changes: 17 additions & 3 deletions app/routes/_auth+/reset-password.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,11 @@ import { data, redirect, Form } from 'react-router'
import { GeneralErrorBoundary } from '#app/components/error-boundary.tsx'
import { ErrorList, Field } from '#app/components/forms.tsx'
import { StatusButton } from '#app/components/ui/status-button.tsx'
import { requireAnonymous, resetUserPassword } from '#app/utils/auth.server.ts'
import {
checkCommonPassword,
requireAnonymous,
resetUserPassword,
} from '#app/utils/auth.server.ts'
import { useIsPending } from '#app/utils/misc.tsx'
import { PasswordAndConfirmPasswordSchema } from '#app/utils/user-validation.ts'
import { verifySessionStorage } from '#app/utils/verification.server.ts'
Expand Down Expand Up @@ -41,8 +45,18 @@ export async function loader({ request }: Route.LoaderArgs) {
export async function action({ request }: Route.ActionArgs) {
const resetPasswordUsername = await requireResetPasswordUsername(request)
const formData = await request.formData()
const submission = parseWithZod(formData, {
schema: ResetPasswordSchema,
const submission = await parseWithZod(formData, {
schema: ResetPasswordSchema.superRefine(async ({ password }, ctx) => {
const isCommonPassword = await checkCommonPassword(password)
if (isCommonPassword) {
ctx.addIssue({
path: ['password'],
code: 'custom',
message: 'Password is too common',
})
}
}),
async: true,
})
if (submission.status !== 'success') {
return data(
Expand Down
32 changes: 32 additions & 0 deletions app/utils/auth.server.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import crypto from 'node:crypto'
import { type Connection, type Password, type User } from '@prisma/client'
import bcrypt from 'bcryptjs'
import { redirect } from 'react-router'
Expand Down Expand Up @@ -255,3 +256,34 @@ export async function verifyUserPassword(

return { id: userWithPassword.id }
}

export async function checkCommonPassword(password: string) {
const hash = crypto
.createHash('sha1')
.update(password, 'utf8')
.digest('hex')
.toUpperCase()

const [prefix, suffix] = [hash.slice(0, 5), hash.slice(5)]

const controller = new AbortController()

try {
const timeoutId = setTimeout(() => controller.abort(), 1000)

const res = await fetch(`https://api.pwnedpasswords.com/range/${prefix}`)

clearTimeout(timeoutId)

if (!res.ok) false

const data = await res.text()
return data.split('/\r?\n/').some((line) => line.includes(suffix))
} catch (error) {
if (error instanceof Error && error.name === 'AbortError') {
console.warn('Password check timed out')
}
console.warn('unknow error during password check', error)
return false
}
}
2 changes: 2 additions & 0 deletions tests/mocks/index.ts
Original file line number Diff line number Diff line change
@@ -1,13 +1,15 @@
import closeWithGrace from 'close-with-grace'
import { setupServer } from 'msw/node'
import { handlers as githubHandlers } from './github.ts'
import { handlers as pwnedPasswordApiHandlers } from './pwnedpasswords.ts'
import { handlers as resendHandlers } from './resend.ts'
import { handlers as tigrisHandlers } from './tigris.ts'

export const server = setupServer(
...resendHandlers,
...githubHandlers,
...tigrisHandlers,
...pwnedPasswordApiHandlers,
)

server.listen({
Expand Down
7 changes: 7 additions & 0 deletions tests/mocks/pwnedpasswords.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
import { http, HttpResponse } from 'msw'

export const handlers = [
http.get('https://api.pwnedpasswords.com/range/:prefix', () => {
return new HttpResponse('', { status: 200 })
}),
]

0 comments on commit 832c24c

Please sign in to comment.