Skip to content

fix(auth): accept the same passwords on reset, sign-up and Profile #1892

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.

Sign up for GitHub

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

Draft
dawsontoth wants to merge 1 commit into from
+79 −11
Draft

fix(auth): accept the same passwords on reset, sign-up and Profile #1892

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
30 changes: 23 additions & 7 deletions src/features/auth/ResetPassword.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
/**
* @vitest-environment jsdom
*/
import { act, cleanup, fireEvent, render, screen } from '@testing-library/react';
import { act, cleanup, fireEvent, render, screen, waitFor } from '@testing-library/react';
import { afterEach, describe, expect, it, vi } from 'vitest';

vi.mock('@tanstack/react-router', () => ({
Expand All @@ -19,6 +19,14 @@ afterEach(() => {
vi.clearAllMocks();
});

async function submitNewPassword(value: string) {
await act(async () => {
fireEvent.change(screen.getByLabelText('New Password'), { target: { value } });
fireEvent.change(screen.getByLabelText('Confirm Password'), { target: { value } });
fireEvent.click(screen.getByRole('button', { name: 'Submit New Password' }));
});
}

describe('ResetPassword', () => {
// maxlength would silently clamp a pasted password, leaving the password manager holding one that never worked.
it('lets an over-long password be typed in full and explains the limit instead', async () => {
Expand All @@ -28,15 +36,23 @@ describe('ResetPassword', () => {
expect(password.maxLength).toBe(-1);
expect(confirmation.maxLength).toBe(-1);

await act(async () => {
fireEvent.change(password, { target: { value: 'p'.repeat(51) } });
fireEvent.change(confirmation, { target: { value: 'p'.repeat(51) } });
fireEvent.click(screen.getByRole('button', { name: 'Submit New Password' }));
});
await submitNewPassword('p'.repeat(257));

expect(await screen.findByText('Password cannot be longer than 50 characters.')).toBeTruthy();
expect(password.value).toHaveLength(257);
expect(await screen.findByText('Password cannot be longer than 256 characters.')).toBeTruthy();
expect(mutate).not.toHaveBeenCalled();
});

it.each([51, 256])('accepts a %i-character password, as sign-up does', async (length) => {
render(<ResetPassword />);
const value = 'p'.repeat(length);

await submitNewPassword(value);

await waitFor(() => expect(mutate).toHaveBeenCalledTimes(1));
expect(mutate.mock.calls[0][0]).toEqual({ token: 'reset-token', password: value });
expect(screen.queryByText(/cannot be longer than/)).toBeNull();
});
});

describe('ResetPassword password fields', () => {
Expand Down
4 changes: 2 additions & 2 deletions src/features/auth/ResetPassword.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import { FormField } from '@/components/ui/form/FormField';
import { FormItem } from '@/components/ui/form/FormItem';
import { FormLabel } from '@/components/ui/form/FormLabel';
import { FormMessage } from '@/components/ui/form/FormMessage';
import { zodRequirePassword } from '@/lib/zod/password';
import { ACCOUNT_PASSWORD_MAX_LENGTH, ACCOUNT_PASSWORD_TOO_LONG_ERROR, zodRequirePassword } from '@/lib/zod/password';
import { zodResolver } from '@hookform/resolvers/zod';
import { useNavigate, useSearch } from '@tanstack/react-router';
import { useCallback, useEffect } from 'react';
Expand All @@ -18,7 +18,7 @@ const ResetPasswordSchema = z
.object({
password: zodRequirePassword
.min(8, { error: 'Password must be at least 8 characters long.' })
.max(50, { error: 'Password cannot be longer than 50 characters.' }),
.max(ACCOUNT_PASSWORD_MAX_LENGTH, { error: ACCOUNT_PASSWORD_TOO_LONG_ERROR }),
confirmPassword: z.string(),
})
.refine((data) => data.password === data.confirmPassword, {
Expand Down
23 changes: 23 additions & 0 deletions src/features/auth/SignUp.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,29 @@ describe('SignUp', () => {
expect(maxLengthOf('Confirm Password')).toBe(-1);
});

it('refuses a password longer than central manager accepts before sending it', async () => {
renderSignUp();
fillValidForm();
fireEvent.change(screen.getByLabelText('Password'), { target: { value: 'p'.repeat(257) } });
fireEvent.change(screen.getByLabelText('Confirm Password'), { target: { value: 'p'.repeat(257) } });
submit();

expect(await screen.findByText('Password cannot be longer than 256 characters.')).toBeTruthy();
expect(post).not.toHaveBeenCalled();
});

it('sends a password right at central manager’s limit', async () => {
post.mockResolvedValue({ data: { id: 'usr-1', email: 'taken@example.com' } });
renderSignUp();
fillValidForm();
fireEvent.change(screen.getByLabelText('Password'), { target: { value: 'p'.repeat(256) } });
fireEvent.change(screen.getByLabelText('Confirm Password'), { target: { value: 'p'.repeat(256) } });
submit();

await waitFor(() => expect(post).toHaveBeenCalledTimes(1));
expect(post.mock.calls[0][1]).toMatchObject({ password: 'p'.repeat(256) });
});

it('reveals each password independently without submitting or losing field values', () => {
renderSignUp();
const password = screen.getByLabelText('Password') as HTMLInputElement;
Expand Down
5 changes: 3 additions & 2 deletions src/features/auth/SignUp.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import { parseCompanyFromEmail } from '@/lib/string/parseCompanyFromEmail';
import { personNameRegex } from '@/lib/string/regex/personNameRegex';
import { clearUtmParamsFromUrl } from '@/lib/urls/clearUtmParams';
import { zodRequireEmail } from '@/lib/zod/email';
import { zodRequirePassword } from '@/lib/zod/password';
import { ACCOUNT_PASSWORD_MAX_LENGTH, ACCOUNT_PASSWORD_TOO_LONG_ERROR, zodRequirePassword } from '@/lib/zod/password';
import { zodResolver } from '@hookform/resolvers/zod';
import { Link, useNavigate, useSearch } from '@tanstack/react-router';
import { MouseEvent, useCallback, useEffect, useState } from 'react';
Expand Down Expand Up @@ -50,7 +50,8 @@ const SignUpSchema = z.object({
)
.max(80, { error: 'Last name cannot be longer than 80 characters.' }),
password: zodRequirePassword
.min(8, { error: 'Password must be at least 8 characters long.' }),
.min(8, { error: 'Password must be at least 8 characters long.' })
.max(ACCOUNT_PASSWORD_MAX_LENGTH, { error: ACCOUNT_PASSWORD_TOO_LONG_ERROR }),
confirmPassword: z.string(),
acceptTerms: z.boolean().refine(val => val === true, {
message: 'You must accept the Privacy Policy and Terms of Service.',
Expand Down
20 changes: 20 additions & 0 deletions src/features/profile/ProfileIndex.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,26 @@ describe('ProfileIndex', () => {
await waitFor(() => expect(saveButton().disabled).toBe(false));
});

it('refuses a new password longer than sign-up accepts once the field is left', async () => {
render(<ProfileIndex />);
await type('New Password', 'p'.repeat(257));
await leave('New Password');

expect(await screen.findByText('Password cannot be longer than 256 characters.')).toBeTruthy();
expect(saveButton().disabled).toBe(true);
});

it('accepts a new password as long as sign-up allows', async () => {
render(<ProfileIndex />);
await type('New Password', 'p'.repeat(256));
await leave('New Password');
await type('Confirm New Password', 'p'.repeat(256));
await leave('Confirm New Password');

await waitFor(() => expect(saveButton().disabled).toBe(false));
expect(screen.queryByText(/cannot be longer than/)).toBeNull();
});

it('explains a short new password when Enter is pressed in it', async () => {
render(<ProfileIndex />);
await type('New Password', 'short');
Expand Down
2 changes: 2 additions & 0 deletions src/features/profile/mutations/updateUserSchema.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { ACCOUNT_PASSWORD_MAX_LENGTH, ACCOUNT_PASSWORD_TOO_LONG_ERROR } from '@/lib/zod/password';
import { z } from 'zod';

export const UpdateUserSchema = z
Expand All @@ -20,6 +21,7 @@ export const UpdateUserSchema = z
error: 'Please enter your new password.',
})
.min(8, { error: 'Password must be 8 characters or more.' })
.max(ACCOUNT_PASSWORD_MAX_LENGTH, { error: ACCOUNT_PASSWORD_TOO_LONG_ERROR })
.or(z.string().max(0)),
Comment on lines 23 to 25

@gemini-code-assist gemini-code-assist Bot Oct 11, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

Using .or() to allow an empty string creates a ZodUnion. When validation fails (e.g., if the password is too long or too short), Zod evaluates both branches and returns errors for both, which can lead to multiple or confusing error messages in the UI.

We can avoid the union entirely by using a single ZodString with a .refine() check. This ensures only a single, predictable error message is generated. Note that in Zod v4, .refine() accepts error as the unified message parameter.

			.max(ACCOUNT_PASSWORD_MAX_LENGTH, { error: ACCOUNT_PASSWORD_TOO_LONG_ERROR })
			.refine((val) => val.length === 0 || val.length >= 8, {
				error: 'Password must be 8 characters or more.',
			}),
References
  1. In Zod v4, '.refine()' accepts 'error' as the unified message parameter.

@dawsontoth dawsontoth Oct 12, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Declining: the form only ever shows one message here, and it is the right one. Replaying the schema through zodResolver(UpdateUserSchema) (resolvers 5.9.1, zod 4.6.5): a 257-character password yields message: "Password cannot be longer than 256 characters." and "short" yields "Password must be 8 characters or more."; the empty and valid cases yield no error. Each failing branch has exactly one issue, and the resolver surfaces the closest branch with ties going to the first (@hookform/resolvers/zod/src/zod.ts:117-131), so the .max(0) branch's text never wins. The second branch's text appears only in types under criteriaMode: 'all', which no form sets, and FormMessage renders error.message alone (src/components/ui/form/FormMessage.tsx:7). ProfileIndex.test.tsx pins both messages through the real form. The .or(z.string().max(0)) union also predates this PR, which only adds the .max.

🤖 Generated by Claude Code (Claude Opus 5.5); posted via @dawsontoth.

confirmNewPassword: z
.string()
Expand Down
6 changes: 6 additions & 0 deletions src/lib/zod/password.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,3 +3,9 @@ import { z } from 'zod';
export const zodRequirePassword = z
.string()
.nonempty({ error: 'Please enter your password.' });

// Central manager's `PASSWORD_MAX_LENGTH`, which it enforces only on sign-up. Reset and Profile use it
// too, so every form that sets a Fabric account password accepts the same passwords.
export const ACCOUNT_PASSWORD_MAX_LENGTH = 256;
export const ACCOUNT_PASSWORD_TOO_LONG_ERROR =
`Password cannot be longer than ${ACCOUNT_PASSWORD_MAX_LENGTH} characters.`;
Loading