Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

prevent use of common password using the haveibeenpwned password api #958

Merged
merged 6 commits into from
Mar 6, 2025
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
31 changes: 31 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,33 @@ 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')
}
return false
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It may be a good idea to add a warning if any other kind of error is thrown so we at least have some visibility into whether things are working.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah completely forgot about it

}
}
8 changes: 8 additions & 0 deletions tests/mocks/common-password.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
import { http, HttpResponse } from 'msw'

export const pwnedPasswordApiHandler = http.get(
'https://api.pwnedpasswords.com/range/:prefix',
() => {
return new HttpResponse('', { status: 200 })
},
)
2 changes: 2 additions & 0 deletions tests/mocks/index.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import closeWithGrace from 'close-with-grace'
import { setupServer } from 'msw/node'
import { pwnedPasswordApiHandler } from './common-password.ts'
import { handlers as githubHandlers } from './github.ts'
import { handlers as resendHandlers } from './resend.ts'
import { handlers as tigrisHandlers } from './tigris.ts'
Expand All @@ -8,6 +9,7 @@ export const server = setupServer(
...resendHandlers,
...githubHandlers,
...tigrisHandlers,
pwnedPasswordApiHandler,
)

server.listen({
Expand Down