Enhance signup and password reset flows#344
Merged
Merged
Conversation
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
✅ Deploy Preview for develop-devlovers ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughCentralized signup/password constraints and a UTF‑8 byte-length utility were added. Server Zod schemas now enforce tightened password/name/email rules and surface first-field validation messages. Client forms and field components gained live validation, trimmed-on-blur behavior, expanded props, inline error messaging, and submit gating. i18n keys for new validations were added. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant ClientForm as Client Form
participant ClientValidate as Client Validation
participant API as API Route
participant ServerValidate as Server Validation
participant DB as Database
User->>ClientForm: fill fields / blur
ClientForm->>ClientValidate: validate (length, regex, bytes)
ClientValidate-->>ClientForm: update errors / disable submit
User->>ClientForm: submit
ClientForm->>ClientValidate: final check
alt client valid
ClientForm->>API: POST (name,email,password,confirm)
API->>ServerValidate: parse & Zod-validate using shared constraints
alt server valid
ServerValidate->>DB: create/update user, hash password
DB-->>API: success
API-->>ClientForm: success
else server invalid
ServerValidate-->>API: first-field error
API-->>ClientForm: field-specific error
end
else client invalid
ClientForm->>ClientForm: block submission, show errors
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
frontend/app/api/auth/signup/route.ts (1)
61-65:⚠️ Potential issue | 🟡 MinorSame field-error shape mismatch as in the password-reset route.
parsed.error.flatten().fieldErrorsreturns an object, but the client (SignupForm.tsxLine 169–171) only checkstypeof data?.error === 'string', so all Zod field-level messages are replaced with a genericsignupFailedstring. Consider returning a flat error string (e.g. the first issue) or updating the client to render the field-level map.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/app/api/auth/signup/route.ts` around lines 61 - 65, The route currently returns parsed.error.flatten().fieldErrors (an object) which the client expects to be a string; update the failure response in the parsed.success branch to return a single flat error string instead (e.g. extract the first field error by using parsed.error.flatten().fieldErrors, take Object.values(...).flat()[0] or similar) and return { error: "<firstErrorString>" } with status 400 so SignupForm (which checks typeof data?.error === 'string') receives a string. Ensure you keep the existing NextResponse.json call and only change the payload shape.
🧹 Nitpick comments (9)
frontend/app/api/auth/password-reset/confirm/route.ts (1)
17-29: Duplicated password validation chain — extract a shared schema.The password Zod chain (
.min(),.max(), two.regex(),.regex(PASSWORD_POLICY_REGEX)) is identical to the one infrontend/app/api/auth/signup/route.ts(Lines 37–43). Extract a sharedpasswordSchemafromsignup-constraints.tsto keep the rules in sync and avoid divergence.♻️ Example: shared password schema
In
frontend/lib/auth/signup-constraints.ts:import { z } from 'zod'; export const passwordSchema = z .string() .min(PASSWORD_MIN_LEN, `Password must be at least ${PASSWORD_MIN_LEN} characters`) .max(PASSWORD_MAX_LEN, `Password must be at most ${PASSWORD_MAX_LEN} characters`) .regex(/[A-Z]/, 'Password must contain at least one capital letter') .regex(/[^A-Za-z0-9]/, 'Password must contain at least one special character') .regex(PASSWORD_POLICY_REGEX, 'Password does not meet the required policy');Then in both route files:
- password: z - .string() - .min(...) - ... + password: passwordSchema,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/app/api/auth/password-reset/confirm/route.ts` around lines 17 - 29, Extract the duplicated Zod chain used for the password field into a shared exported passwordSchema in signup-constraints.ts and import it into both confirm route and signup route; replace the inline chain (the password: z.string().min(...).max(...).regex(...).regex(...).regex(PASSWORD_POLICY_REGEX,...)) with a reference to passwordSchema so both routes use the same validation constants (PASSWORD_MIN_LEN, PASSWORD_MAX_LEN, PASSWORD_POLICY_REGEX) and stay in sync.frontend/components/auth/fields/PasswordField.tsx (2)
50-60: Hardcoded English validation messages break i18n consistency.
tooLong(Line 51) andpatternMismatch(Lines 57–58) use hardcoded English strings, whilevalueMissingandtooShortuset(...)translations. These should be localized the same way. The same inconsistency appears inNameField.tsxandEmailField.tsx.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/components/auth/fields/PasswordField.tsx` around lines 50 - 60, The validation messages in PasswordField.tsx use hardcoded English strings for the tooLong and patternMismatch branches; update the input.setCustomValidity calls inside the input.validity.tooLong (uses maxLength) and input.validity.patternMismatch checks to use the same i18n translation function t(...) as used for valueMissing/tooShort so messages are localized, and apply the same change in NameField.tsx and EmailField.tsx replacing their hardcoded setCustomValidity strings with appropriate t(...) keys that match existing translation keys or add new keys if needed.
63-66:handleInputre-appliescustomErroron every keystroke.If a parent ever passes a non-empty
customError, the input will remain invalid after every keystroke becausehandleInputre-setscustomErrorinstead of clearing it. Current callers don't passcustomError, so this is dormant, but the API contract is surprising — consumers would need to reactively clearcustomErroron their ownonChangeto avoid this.Consider clearing validity unconditionally in
handleInputand only applyingcustomErrorexternally (e.g., via auseEffector on blur).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/components/auth/fields/PasswordField.tsx` around lines 63 - 66, The handleInput function currently re-applies the passed-in customError on every keystroke which keeps the control invalid; change handleInput (in PasswordField) to always clear validity (call setCustomValidity('')) instead of setting customError, and move application of customError to an external place (e.g., a useEffect that runs when customError changes or the input's onBlur handler) so consumers can pass a persistent customError without it being re-set on each keystroke; update references to handleInput and ensure any existing useEffect/onBlur applies input.setCustomValidity(customError) when appropriate.frontend/components/auth/ResetPasswordForm.tsx (1)
37-43: Hardcoded English UI strings — not localized.
passwordRequirementsText(Line 37–38),passwordErrorText(Lines 40–43), andplaceholder="New password"(Line 96) are all hardcoded in English. The rest of the form already usesuseTranslationsfor labels and messages. These should go throught(...)as well for i18n consistency.Also applies to: 96-96
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/components/auth/ResetPasswordForm.tsx` around lines 37 - 43, The hardcoded English strings in ResetPasswordForm should be localized: replace the constants passwordRequirementsText and passwordErrorText so they call the useTranslations() t(...) function (e.g., t('resetPassword.passwordRequirements') and t('resetPassword.passwordError')) and change the input placeholder="New password" to placeholder={t('resetPassword.newPasswordPlaceholder')}; update ResetPasswordForm to import/use the existing useTranslations instance (or the same t used elsewhere in the file) and use those keys, ensuring string interpolation for passwordErrorText still uses the localized requirements text.frontend/components/auth/SignupForm.tsx (3)
264-296: Confirm password field should not independently enforce the pattern.Passing
pattern={PASSWORD_POLICY_REGEX.source}(Line 289) to the confirm-passwordPasswordFieldtriggers browser-native pattern validation and the "must include at least one capital letter…" tooltip fromPasswordField'shandleInvalid. For a confirm field the only meaningful constraint is matching the primary password. Consider omittingpattern,minLength, andmaxLengthfrom the confirm field and validating only via the match check.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/components/auth/SignupForm.tsx` around lines 264 - 296, The confirm-password PasswordField (id/name "confirmPassword") should not enforce the password policy via pattern/minLength/maxLength; remove the pattern={PASSWORD_POLICY_REGEX.source}, minLength={PASSWORD_MIN_LEN} and maxLength={PASSWORD_MAX_LEN} props from the confirm PasswordField instance and rely only on the match validation that sets confirmPasswordErrorText (keep onChange={setConfirmPasswordValue} and onBlur={() => setConfirmPasswordTouched(true)}); ensure the existing match-checking logic (where confirmPasswordErrorText is produced) covers length/policy mismatches by comparing to the primary password rather than using browser-native validation on the confirm field.
77-82:confirmPasswordPolicyOkis redundant — only match matters.If the primary password already passes the policy and
passwordsMatchis true, the confirm password passes the policy by definition. Validating the confirm field independently against the policy produces confusing UX: users see "Repeat password must meet requirements" while they're still typing to match the first field, instead of the simpler "Passwords do not match." RemoveconfirmPasswordPolicyOkand its derived error text; rely onpasswordsMatchalone for the confirm field.♻️ Suggested simplification
- const confirmPasswordPolicyOk = useMemo(() => { - if (!confirmPasswordValue) return false; - if (confirmPasswordValue.length < PASSWORD_MIN_LEN) return false; - if (confirmPasswordValue.length > PASSWORD_MAX_LEN) return false; - return PASSWORD_POLICY_REGEX.test(confirmPasswordValue); - }, [confirmPasswordValue]); - const passwordsMatch = useMemo(() => { if (!passwordValue || !confirmPasswordValue) return false; return passwordValue === confirmPasswordValue; }, [passwordValue, confirmPasswordValue]); ... - const confirmPolicyErrorText = - confirmPasswordTouched && !confirmPasswordPolicyOk - ? `Repeat password must meet requirements: ${passwordRequirementsText}` - : null; - - const mismatchErrorText = - confirmPasswordTouched && - passwordTouched && - passwordValue.length > 0 && - confirmPasswordValue.length > 0 && - !passwordsMatch - ? 'Passwords do not match.' - : null; - - const confirmPasswordErrorText = - mismatchErrorText ?? confirmPolicyErrorText ?? null; + const confirmPasswordErrorText = + confirmPasswordTouched && + passwordTouched && + confirmPasswordValue.length > 0 && + !passwordsMatch + ? 'Passwords do not match.' + : null; const submitDisabled = loading || !nameLooksValid || !emailLooksValid || !passwordPolicyOk || - !confirmPasswordPolicyOk || !passwordsMatch;Also applies to: 118-121, 132-133
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/components/auth/SignupForm.tsx` around lines 77 - 82, Remove the redundant confirmPasswordPolicyOk logic in the SignupForm component: delete the useMemo that computes confirmPasswordPolicyOk (and any references to PASSWORD_MIN_LEN, PASSWORD_MAX_LEN, PASSWORD_POLICY_REGEX inside it) and stop rendering the confirm-field-specific policy error message; instead rely solely on the existing passwordsMatch boolean to show the "passwords do not match" error for the confirm field. Ensure any code branches or JSX that conditionally display confirmPasswordPolicyOk-based errors (e.g., the error text shown when confirmPasswordPolicyOk is false) are replaced to depend only on passwordsMatch, and remove leftover imports/variables that become unused after this change.
58-61: Zod schema is re-instantiated insideuseMemo— hoist it.
z.string().email()creates a new Zod schema object every timeemailTrimmedchanges. It's cheap, but easy to hoist above the component as a module-level constant.♻️ Hoist the schema
+const emailSchema = z.string().email(); + export function SignupForm({ locale, returnTo }: SignupFormProps) { ... const emailFormatOk = useMemo(() => { if (!emailTrimmed) return false; - return z.string().email().safeParse(emailTrimmed).success; + return emailSchema.safeParse(emailTrimmed).success; }, [emailTrimmed]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/components/auth/SignupForm.tsx` around lines 58 - 61, The Zod schema is being re-created inside the useMemo for emailFormatOk; hoist the schema to a module-level constant (e.g., EMAIL_SCHEMA = z.string().email()) and then update the useMemo to call EMAIL_SCHEMA.safeParse(emailTrimmed).success so emailTrimmed and emailFormatOk behavior is unchanged but the schema object is not re-instantiated on every change; reference the existing emailFormatOk useMemo, emailTrimmed variable, and z.string().email() when making the change.frontend/lib/auth/signup-constraints.ts (1)
10-11: Length bounds in the regex duplicatePASSWORD_MIN_LEN/PASSWORD_MAX_LEN.
PASSWORD_POLICY_REGEXhardcodes.{8,128}while the constants on Lines 7–8 express the same bounds. If either value changes the regex silently drifts. Consider building the regex dynamically or dropping the length quantifier from the regex and relying solely on the constants (the Zod schemas and HTML attributes already enforce length).♻️ Option: derive regex from constants
-export const PASSWORD_POLICY_REGEX = - /^(?=.*[A-Z])(?=.*[^A-Za-z0-9]).{8,128}$/; +export const PASSWORD_POLICY_REGEX = new RegExp( + `^(?=.*[A-Z])(?=.*[^A-Za-z0-9]).{${PASSWORD_MIN_LEN},${PASSWORD_MAX_LEN}}$` +);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/lib/auth/signup-constraints.ts` around lines 10 - 11, PASSWORD_POLICY_REGEX currently hardcodes the length quantifier ".{8,128}" which duplicates PASSWORD_MIN_LEN and PASSWORD_MAX_LEN; update the implementation so the regex length is derived from those constants (e.g., create the regex via new RegExp(`^(?=.*[A-Z])(?=.*[^A-Za-z0-9]).{${PASSWORD_MIN_LEN},${PASSWORD_MAX_LEN}}$`) at module runtime) or remove the length quantifier entirely and rely on PASSWORD_MIN_LEN / PASSWORD_MAX_LEN (and Zod/HTML attrs) for length checks; adjust the exported PASSWORD_POLICY_REGEX definition accordingly so it remains consistent when the constants change.frontend/components/auth/fields/NameField.tsx (1)
45-55:onChangefires twice with different values — on keystroke (raw) and on blur (trimmed).The
onChangecallback on Line 69 fires the raw value on every keystroke, whilehandleBlur(Line 53) fires the trimmed value on blur. This works because the parent (SignupForm) applies.trim()in its ownuseMemo, but callers that trustonChangeto always deliver the final value may be surprised. Consider either removing theonChangecall fromhandleBlur(relying on the input's ownonChange) or documenting this dual-fire behavior.Also applies to: 69-69
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/components/auth/fields/NameField.tsx` around lines 45 - 55, The handleBlur in NameField.tsx currently calls onChange with the trimmed value and also calls onBlur, causing onChange to fire twice (raw on keystroke and trimmed on blur); to fix, remove the onChange?.(trimmed) call from the handleBlur function so the component only emits keystroke values via the input's onChange handler and emits blur via onBlur, or alternatively document this dual-fire behavior—locate handleBlur and adjust/remove the onChange invocation there (references: handleBlur, onChange, onBlur, SignupForm useMemo behavior).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@frontend/app/api/auth/password-reset/confirm/route.ts`:
- Around line 36-40: The route currently returns field-level errors as an object
(parsed.error.flatten().fieldErrors) but the client ResetPasswordForm.tsx
expects a string; change the response in the failing branch (where
parsed.success is false) to produce a single error string that the client can
consume: extract the first available field message from
parsed.error.flatten().fieldErrors (e.g., first non-empty array value) and
return NextResponse.json({ error: "<first message>" }, { status: 400 }) so
ResetPasswordForm.tsx sees a string instead of an object; alternatively, if you
prefer per-field UI, update ResetPasswordForm.tsx to handle an object-shaped
error payload, but pick one approach and make the route's response shape
(NextResponse.json) match ResetPasswordForm.tsx expectations.
In `@frontend/components/auth/SignupForm.tsx`:
- Around line 94-108: The email error logic in the useMemo for emailErrorText
currently returns null when emailTrimmed is empty, so when a user blurs the
field without typing (emailTouched true) no inline "required" message is shown;
update the logic in the emailErrorText useMemo (referencing emailTouched,
emailTrimmed, EMAIL_MAX_LEN, emailFormatOk) to return an explicit required
message when emailTouched is true and emailTrimmed is empty (instead of
returning null), while keeping the existing length and format checks afterwards.
---
Outside diff comments:
In `@frontend/app/api/auth/signup/route.ts`:
- Around line 61-65: The route currently returns
parsed.error.flatten().fieldErrors (an object) which the client expects to be a
string; update the failure response in the parsed.success branch to return a
single flat error string instead (e.g. extract the first field error by using
parsed.error.flatten().fieldErrors, take Object.values(...).flat()[0] or
similar) and return { error: "<firstErrorString>" } with status 400 so
SignupForm (which checks typeof data?.error === 'string') receives a string.
Ensure you keep the existing NextResponse.json call and only change the payload
shape.
---
Nitpick comments:
In `@frontend/app/api/auth/password-reset/confirm/route.ts`:
- Around line 17-29: Extract the duplicated Zod chain used for the password
field into a shared exported passwordSchema in signup-constraints.ts and import
it into both confirm route and signup route; replace the inline chain (the
password:
z.string().min(...).max(...).regex(...).regex(...).regex(PASSWORD_POLICY_REGEX,...))
with a reference to passwordSchema so both routes use the same validation
constants (PASSWORD_MIN_LEN, PASSWORD_MAX_LEN, PASSWORD_POLICY_REGEX) and stay
in sync.
In `@frontend/components/auth/fields/NameField.tsx`:
- Around line 45-55: The handleBlur in NameField.tsx currently calls onChange
with the trimmed value and also calls onBlur, causing onChange to fire twice
(raw on keystroke and trimmed on blur); to fix, remove the onChange?.(trimmed)
call from the handleBlur function so the component only emits keystroke values
via the input's onChange handler and emits blur via onBlur, or alternatively
document this dual-fire behavior—locate handleBlur and adjust/remove the
onChange invocation there (references: handleBlur, onChange, onBlur, SignupForm
useMemo behavior).
In `@frontend/components/auth/fields/PasswordField.tsx`:
- Around line 50-60: The validation messages in PasswordField.tsx use hardcoded
English strings for the tooLong and patternMismatch branches; update the
input.setCustomValidity calls inside the input.validity.tooLong (uses maxLength)
and input.validity.patternMismatch checks to use the same i18n translation
function t(...) as used for valueMissing/tooShort so messages are localized, and
apply the same change in NameField.tsx and EmailField.tsx replacing their
hardcoded setCustomValidity strings with appropriate t(...) keys that match
existing translation keys or add new keys if needed.
- Around line 63-66: The handleInput function currently re-applies the passed-in
customError on every keystroke which keeps the control invalid; change
handleInput (in PasswordField) to always clear validity (call
setCustomValidity('')) instead of setting customError, and move application of
customError to an external place (e.g., a useEffect that runs when customError
changes or the input's onBlur handler) so consumers can pass a persistent
customError without it being re-set on each keystroke; update references to
handleInput and ensure any existing useEffect/onBlur applies
input.setCustomValidity(customError) when appropriate.
In `@frontend/components/auth/ResetPasswordForm.tsx`:
- Around line 37-43: The hardcoded English strings in ResetPasswordForm should
be localized: replace the constants passwordRequirementsText and
passwordErrorText so they call the useTranslations() t(...) function (e.g.,
t('resetPassword.passwordRequirements') and t('resetPassword.passwordError'))
and change the input placeholder="New password" to
placeholder={t('resetPassword.newPasswordPlaceholder')}; update
ResetPasswordForm to import/use the existing useTranslations instance (or the
same t used elsewhere in the file) and use those keys, ensuring string
interpolation for passwordErrorText still uses the localized requirements text.
In `@frontend/components/auth/SignupForm.tsx`:
- Around line 264-296: The confirm-password PasswordField (id/name
"confirmPassword") should not enforce the password policy via
pattern/minLength/maxLength; remove the pattern={PASSWORD_POLICY_REGEX.source},
minLength={PASSWORD_MIN_LEN} and maxLength={PASSWORD_MAX_LEN} props from the
confirm PasswordField instance and rely only on the match validation that sets
confirmPasswordErrorText (keep onChange={setConfirmPasswordValue} and onBlur={()
=> setConfirmPasswordTouched(true)}); ensure the existing match-checking logic
(where confirmPasswordErrorText is produced) covers length/policy mismatches by
comparing to the primary password rather than using browser-native validation on
the confirm field.
- Around line 77-82: Remove the redundant confirmPasswordPolicyOk logic in the
SignupForm component: delete the useMemo that computes confirmPasswordPolicyOk
(and any references to PASSWORD_MIN_LEN, PASSWORD_MAX_LEN, PASSWORD_POLICY_REGEX
inside it) and stop rendering the confirm-field-specific policy error message;
instead rely solely on the existing passwordsMatch boolean to show the
"passwords do not match" error for the confirm field. Ensure any code branches
or JSX that conditionally display confirmPasswordPolicyOk-based errors (e.g.,
the error text shown when confirmPasswordPolicyOk is false) are replaced to
depend only on passwordsMatch, and remove leftover imports/variables that become
unused after this change.
- Around line 58-61: The Zod schema is being re-created inside the useMemo for
emailFormatOk; hoist the schema to a module-level constant (e.g., EMAIL_SCHEMA =
z.string().email()) and then update the useMemo to call
EMAIL_SCHEMA.safeParse(emailTrimmed).success so emailTrimmed and emailFormatOk
behavior is unchanged but the schema object is not re-instantiated on every
change; reference the existing emailFormatOk useMemo, emailTrimmed variable, and
z.string().email() when making the change.
In `@frontend/lib/auth/signup-constraints.ts`:
- Around line 10-11: PASSWORD_POLICY_REGEX currently hardcodes the length
quantifier ".{8,128}" which duplicates PASSWORD_MIN_LEN and PASSWORD_MAX_LEN;
update the implementation so the regex length is derived from those constants
(e.g., create the regex via new
RegExp(`^(?=.*[A-Z])(?=.*[^A-Za-z0-9]).{${PASSWORD_MIN_LEN},${PASSWORD_MAX_LEN}}$`)
at module runtime) or remove the length quantifier entirely and rely on
PASSWORD_MIN_LEN / PASSWORD_MAX_LEN (and Zod/HTML attrs) for length checks;
adjust the exported PASSWORD_POLICY_REGEX definition accordingly so it remains
consistent when the constants change.
kryvosheyin
commented
Feb 19, 2026
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (3)
frontend/app/api/auth/password-reset/confirm/route.ts (1)
27-32:PASSWORD_POLICY_REGEXcheck is redundant and its message is unreachable.
PASSWORD_POLICY_REGEXenforces uppercase, special character, and length 8–128 — all of which are already individually validated by the preceding.min(),.max(), and two.regex()calls. If those pass, the policy regex always passes too; if they fail, the more specific messages fire first. The final check produces no additional validation and'Password does not meet the required policy'is never returned as the first (or only) error.Consider removing it to avoid confusion when the policy regex is updated in the future (the individual checks would also need updating to stay in sync):
♻️ Proposed simplification
.regex(/[A-Z]/, 'Password must contain at least one capital letter') .regex( /[^A-Za-z0-9]/, 'Password must contain at least one special character' - ) - .regex(PASSWORD_POLICY_REGEX, 'Password does not meet the required policy'), + ),
PASSWORD_POLICY_REGEXcan still be imported and used in client-side form components where a single catch-all regex check is preferred, but here the individual checks already cover everything.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/app/api/auth/password-reset/confirm/route.ts` around lines 27 - 32, The final .regex(PASSWORD_POLICY_REGEX, 'Password does not meet the required policy') in the password validation chain is redundant because .min(), .max(), and the two prior .regex() calls already enforce length, uppercase and special-character rules; remove the PASSWORD_POLICY_REGEX check (and its message) from the validator in frontend/app/api/auth/password-reset/confirm/route.ts so the individual, more specific validations remain, and update any local imports/usages if you also remove the PASSWORD_POLICY_REGEX import — keep PASSWORD_POLICY_REGEX available for client-side single-regex checks if needed.frontend/components/auth/SignupForm.tsx (2)
130-131: Consider showing the policy error before the mismatch error.When both fields contain non-empty but policy-failing values that also don't match, the current priority (
mismatchErrorText ?? confirmPolicyErrorText) surfaces "Passwords do not match" rather than the more actionable "Password must meet requirements." Fixing the policy issue first would naturally bring the passwords into a comparable state.♻️ Suggested inversion
- const confirmPasswordErrorText = - mismatchErrorText ?? confirmPolicyErrorText ?? null; + const confirmPasswordErrorText = + confirmPolicyErrorText ?? mismatchErrorText ?? null;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/components/auth/SignupForm.tsx` around lines 130 - 131, In SignupForm.tsx the confirmPasswordErrorText currently prefers mismatchErrorText over confirmPolicyErrorText; invert that priority so policy failures show before mismatch by changing the evaluation order for confirmPasswordErrorText (referencing the confirmPasswordErrorText variable and the mismatchErrorText and confirmPolicyErrorText symbols) to use confirmPolicyErrorText first, then fallback to mismatchErrorText, and finally null.
250-254:setEmail(value)in theEmailFieldonChangeis redundant.
formData.get('email')inonSubmit(line 149–150), and the only consumer is the post-submit verification banner (line 214). CallingsetEmailon every keystroke is unnecessary. The trailing blank line inside the arrow function body is also a minor style artifact.♻️ Suggested cleanup
- onChange={value => { - setEmailValueLive(value); - setEmail(value); - - }} + onChange={setEmailValueLive}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/components/auth/SignupForm.tsx` around lines 250 - 254, The EmailField's onChange handler redundantly calls setEmail(value) on every keystroke even though the canonical email state is set once from formData.get('email') in onSubmit and only used for the post-submit verification banner; remove the unnecessary setEmail(value) call and the trailing blank line, leaving just setEmailValueLive(value) in the onChange arrow function so only the live input state is updated and the persistent email remains set on submit (update the EmailField onChange handler where setEmailValueLive and setEmail are referenced).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@frontend/app/api/auth/password-reset/confirm/route.ts`:
- Around line 19-26: PASSWORD_MAX_LEN (set to 128) exceeds bcrypt's 72-byte
effective input limit causing silent truncation; either reduce PASSWORD_MAX_LEN
to 72 in the password validation chain (the .max(PASSWORD_MAX_LEN, ...) usage in
route.ts) or implement a consistent SHA-256 pre-hash step before calling bcrypt
for both the reset flow (password-reset/confirm route) and the
login/verification flow so all paths hash the same input; update any constants,
input validation, and the bcrypt usage locations to reflect the chosen approach
and ensure tests cover both reset and login verification.
In `@frontend/components/auth/SignupForm.tsx`:
- Line 266: Remove the hardcoded placeholder strings on the PasswordField
instances in SignupForm (the main password field and the confirm-password field)
so they don't bypass PasswordField's internal fallback (placeholder ??
t('password')); specifically, delete the literal placeholder="Password" and the
hardcoded confirm placeholder, then either let PasswordField fall back to
t('password') for the first field and to a new i18n key for the confirm field,
or pass a translated string using t('confirmPassword') (ensure you add the
confirmPassword translation key).
- Around line 92-106: The emailErrorText useMemo currently lacks a branch for
the minimum-length rule so short-but-syntactically-valid addresses produce no
inline error; update the emailErrorText computation (the useMemo that depends on
emailTouched, emailTrimmed, emailFormatOk) to check if emailTrimmed.length <
EMAIL_MIN_LEN and return a clear message like "Email must be at least
{EMAIL_MIN_LEN} characters" before checking format/other rules so the UI
explains why emailLooksValid is false; keep the existing checks for required,
max length and format in the same order and ensure the useMemo includes any
variables it reads (emailTrimmed, EMAIL_MIN_LEN if it's not a true constant) so
the message updates correctly.
---
Nitpick comments:
In `@frontend/app/api/auth/password-reset/confirm/route.ts`:
- Around line 27-32: The final .regex(PASSWORD_POLICY_REGEX, 'Password does not
meet the required policy') in the password validation chain is redundant because
.min(), .max(), and the two prior .regex() calls already enforce length,
uppercase and special-character rules; remove the PASSWORD_POLICY_REGEX check
(and its message) from the validator in
frontend/app/api/auth/password-reset/confirm/route.ts so the individual, more
specific validations remain, and update any local imports/usages if you also
remove the PASSWORD_POLICY_REGEX import — keep PASSWORD_POLICY_REGEX available
for client-side single-regex checks if needed.
In `@frontend/components/auth/SignupForm.tsx`:
- Around line 130-131: In SignupForm.tsx the confirmPasswordErrorText currently
prefers mismatchErrorText over confirmPolicyErrorText; invert that priority so
policy failures show before mismatch by changing the evaluation order for
confirmPasswordErrorText (referencing the confirmPasswordErrorText variable and
the mismatchErrorText and confirmPolicyErrorText symbols) to use
confirmPolicyErrorText first, then fallback to mismatchErrorText, and finally
null.
- Around line 250-254: The EmailField's onChange handler redundantly calls
setEmail(value) on every keystroke even though the canonical email state is set
once from formData.get('email') in onSubmit and only used for the post-submit
verification banner; remove the unnecessary setEmail(value) call and the
trailing blank line, leaving just setEmailValueLive(value) in the onChange arrow
function so only the live input state is updated and the persistent email
remains set on submit (update the EmailField onChange handler where
setEmailValueLive and setEmail are referenced).
…d policy with bcrypt 72-byte limit, confirm-password match, and blur-gated inline errors
kryvosheyin
force-pushed
the
feat/auth-enhance-registration
branch
from
fd36d4f to
4a1c109
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (4)
frontend/app/api/auth/signup/route.ts (2)
30-32: The.regex(PASSWORD_POLICY_REGEX, …)check is redundant.Lines 30–31 already individually assert uppercase and special character presence, and line 26 asserts minimum length — all of which are the only constraints in
PASSWORD_POLICY_REGEX. The catch-all regex can never surface a new failure that the individual checks above it don't already catch, making the third.regex(...)dead validation code.♻️ Proposed simplification
- .regex(/[A-Z]/, 'Password must contain at least one capital letter') - .regex(/[^A-Za-z0-9]/, 'Password must contain at least one special character') - .regex(PASSWORD_POLICY_REGEX, 'Password does not meet the required policy') + .regex(/[A-Z]/, 'Password must contain at least one capital letter') + .regex(/[^A-Za-z0-9]/, 'Password must contain at least one special character')🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/app/api/auth/signup/route.ts` around lines 30 - 32, Remove the redundant .regex(PASSWORD_POLICY_REGEX, ...) call in the password validation chain inside the signup route; the individual .regex(/[A-Z]/, ...), .regex(/[^A-Za-z0-9]/, ...) and the minimum-length check already enforce everything PASSWORD_POLICY_REGEX covers, so delete the .regex(PASSWORD_POLICY_REGEX, 'Password does not meet the required policy') invocation (leave the other validations intact).
52-52:confirmPasswordshould not apply the full password policy schema.Attaching
passwordSchemato the confirm field means Zod validates policy compliance independently on both fields. A user who types a policy-violating value in the confirm field will see policy errors (e.g., "Password must contain at least one capital letter") on the confirm field rather than "Passwords do not match" — which is confusing UX. The confirm field only needs to be a raw string; thesuperRefinealready handles equality.♻️ Proposed fix
- confirmPassword: passwordSchema, + confirmPassword: z.string(),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/app/api/auth/signup/route.ts` at line 52, The confirmPassword field in the signup Zod schema is incorrectly using passwordSchema; change confirmPassword to a plain string schema (e.g., z.string()) instead of passwordSchema so only the main password is validated against the policy and the existing superRefine equality check (superRefine on the schema) handles the "Passwords do not match" error; update the schema where confirmPassword is defined and leave passwordSchema and the superRefine equality logic intact.frontend/app/api/auth/password-reset/confirm/route.ts (1)
11-23: Password schema is duplicated verbatim withsignup/route.ts— extract to a shared module.The identical
.min/.regex×3 /.refine(Buffer.byteLength)chain exists in both route files. A future constraint change requires editing two places.♻️ Suggested extraction
Create
frontend/lib/auth/server-password-schema.ts:import { z } from 'zod'; import { PASSWORD_MAX_BYTES, PASSWORD_MIN_LEN, PASSWORD_POLICY_REGEX, } from '@/lib/auth/signup-constraints'; export const serverPasswordSchema = z .string() .min(PASSWORD_MIN_LEN, `Password must be at least ${PASSWORD_MIN_LEN} characters`) .regex(/[A-Z]/, 'Password must contain at least one capital letter') .regex(/[^A-Za-z0-9]/, 'Password must contain at least one special character') .refine( (val) => Buffer.byteLength(val, 'utf8') <= PASSWORD_MAX_BYTES, `Password must be at most ${PASSWORD_MAX_BYTES} bytes` );Then import
serverPasswordSchemain both route files.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/app/api/auth/password-reset/confirm/route.ts` around lines 11 - 23, The password validation logic is duplicated in the `schema` object in this file and `signup/route.ts`; extract the shared zod chain into a single exported symbol (e.g. `serverPasswordSchema`) in a new module (suggested name: frontend/lib/auth/server-password-schema.ts) that imports the existing constants `PASSWORD_MIN_LEN`, `PASSWORD_MAX_BYTES`, and `PASSWORD_POLICY_REGEX` from `signup-constraints`; replace the inline `.min/.regex/.regex/.regex/.refine(Buffer.byteLength...)` chain in both `schema` (this file) and `signup/route.ts` by importing and using `serverPasswordSchema` and keep the surrounding object shape (use `z.object({ token: z.string().uuid(), password: serverPasswordSchema })` here).frontend/components/auth/SignupForm.tsx (1)
31-33:utf8ByteLengthduplicates the existingpassword-bytes.tsutility — import instead.♻️ Proposed fix
-function utf8ByteLength(value: string): number { - return new TextEncoder().encode(value).length; -}Add import at the top of the file:
+import { utf8ByteLength } from '@/lib/auth/password-bytes';🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/components/auth/SignupForm.tsx` around lines 31 - 33, The function utf8ByteLength duplicates an existing utility; remove this local duplicate and import the centralized implementation from password-bytes.ts instead. Replace the local utf8ByteLength definition in SignupForm.tsx with an import of the utility (use the exported name from password-bytes.ts) and update any local references to call that imported function (ensure the import is added at the top and the duplicate function removed).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@frontend/components/auth/fields/EmailField.tsx`:
- Around line 28-41: In handleInvalid, replace hardcoded English messages for
tooShort and tooLong with calls to the translation function (t(...)) using the
same keys/format as other branches (referencing input.validity.tooShort,
minLength and input.validity.tooLong, maxLength), and reorder the checks so
tooLong is evaluated before typeMismatch (i.e., move the input.validity.tooLong
branch above the typeMismatch branch) to ensure length errors are surfaced prior
to invalid-format errors.
In `@frontend/components/auth/fields/PasswordField.tsx`:
- Around line 46-51: Replace the hardcoded English message in the
patternMismatch branch inside PasswordField.tsx with a call to the i18n
translator (t) using a new key (e.g., "auth.password.patternMismatch"); update
the patternMismatch branch in the validation handler to call
t('auth.password.patternMismatch') instead of the literal string, and add that
key with appropriate translated values to en.json, pl.json, and uk.json so the
message is localized.
In `@frontend/components/auth/ResetPasswordForm.tsx`:
- Around line 31-36: The client-side length check in ResetPasswordForm.tsx uses
passwordValue.length which counts characters not UTF-8 bytes, causing mismatches
with the server's bcrypt byte limit; import and use the existing utf8ByteLength
utility from frontend/lib/auth/password-bytes.ts and replace the guard condition
"passwordValue.length > PASSWORD_MAX_BYTES" with "utf8ByteLength(passwordValue)
> PASSWORD_MAX_BYTES" inside the passwordPolicyOk useMemo (keeping the other
checks and PASSWORD_MIN_LEN, PASSWORD_POLICY_REGEX intact) so client validation
matches server byte-based validation.
In `@frontend/components/auth/SignupForm.tsx`:
- Around line 119-127: password and confirm password show both generic and
byte-length errors; update the computed passwordErrorText and
confirmPolicyErrorText to handle the byte-length case explicitly (use
utf8ByteLength(passwordValue) > PASSWORD_MAX_BYTES to return a specific
"password too long" translation instead of the generic invalidPassword) and
remove the separate byte-length <p> blocks rendered later (or alternatively make
those explicit checks conditional on !passwordPolicyOk and !patternFails etc.);
apply the same change for confirmPasswordErrorText so the byte-length message is
shown exclusively and the duplicate messages are eliminated (refer to
passwordPolicyOk, confirmPasswordPolicyOk, passwordErrorText,
confirmPolicyErrorText, utf8ByteLength, and PASSWORD_MAX_BYTES).
In `@frontend/lib/auth/password-bytes.ts`:
- Around line 1-3: ResetPasswordForm.tsx currently validates password length
using character count (passwordValue.length) which allows multibyte UTF-8
passwords to bypass the byte-limit; replace that check to use the
utf8ByteLength(value) utility from frontend/lib/auth/password-bytes.ts and
import utf8ByteLength at the top of ResetPasswordForm.tsx, comparing
utf8ByteLength(passwordValue) > PASSWORD_MAX_BYTES. Also remove the duplicated
UTF‑8 byte-length implementation in SignupForm.tsx and import utf8ByteLength
there instead so both forms use the single exported function.
In `@frontend/lib/auth/signup-constraints.ts`:
- Around line 4-5: The EMAIL_MIN_LEN constant is set to 8 which rejects valid
short emails; update the constraint by lowering EMAIL_MIN_LEN to 6 (or remove
the minimum entirely and rely on the existing email format validator) and keep
EMAIL_MAX_LEN = 254; also search for usages of EMAIL_MIN_LEN in signup
validation/UX and update any tests or error messages that assert the old minimum
so they reflect the new minimum or the removal.
In `@frontend/messages/en.json`:
- Line 1072: Update the "passwordRequirements" translation entries so they
correctly describe the bcrypt byte limit instead of implying a character limit:
change the wording for the {PASSWORD_MAX_BYTES} portion (key
"passwordRequirements") to mention bytes (e.g.,
"…{PASSWORD_MIN_LEN}-{PASSWORD_MAX_BYTES} characters (max {PASSWORD_MAX_BYTES}
bytes), at least one capital letter, and at least one special character" or a
user-friendly alternative like "…{PASSWORD_MIN_LEN}–{PASSWORD_MAX_BYTES} chars
(up to {PASSWORD_MAX_BYTES} bytes) …"). Apply the same wording change to the
corresponding "passwordRequirements" keys in the other locales (Polish and
Ukrainian).
---
Duplicate comments:
In `@frontend/app/api/auth/password-reset/confirm/route.ts`:
- Around line 25-45: The PR fixes two issues: ensure the first validation error
is surfaced and avoid bcrypt's 72-byte truncation; leave the
firstFieldErrorMessage function as implemented (it returns the first non-empty
string from fieldErrors) and ensure the POST handler continues to use
schema.safeParse(body) and returns NextResponse.json({ error:
firstFieldErrorMessage(flattened) ?? 'Invalid request' }, { status: 400 }) when
parsed.success is false; also keep or enforce the PASSWORD_MAX_BYTES = 72 cap in
the ResetPasswordForm/schema validation so password inputs are validated to that
byte limit to prevent silent bcrypt truncation.
In `@frontend/components/auth/SignupForm.tsx`:
- Around line 99-112: emailErrorText is missing a branch for too-short addresses
causing no feedback when emailLooksValid returns false for length <
EMAIL_MIN_LEN; update the useMemo for emailErrorText (the function named
emailErrorText) to check if emailTrimmed.length < EMAIL_MIN_LEN and return the
appropriate localized message (e.g., tf('validation.emailTooShort', {
EMAIL_MIN_LEN })) before the format check, and ensure EMAIL_MIN_LEN is
referenced in the dependency list if needed.
In `@frontend/messages/pl.json`:
- Line 1072: The Polish translation for the password length message currently
uses "znaków" (characters) while {PASSWORD_MAX_BYTES} is a byte limit; update
the string under the passwordRequirements key to indicate bytes (e.g., replace
"znaków" with "bajtów" or a phrasing like "do {PASSWORD_MAX_BYTES} bajtów") so
it matches the byte-based limit semantics; make the analogous change in en.json
if it also implies characters.
In `@frontend/messages/uk.json`:
- Line 1072: The translation for the passwordRequirements key uses the
placeholder {PASSWORD_MAX_BYTES} but labels it as characters ("символи"),
causing a byte-vs-character mismatch; update the string for passwordRequirements
to either (A) reflect bytes by replacing "символи" with "байтів" and adjust
surrounding grammar so it reads as a UTF-8 byte limit, or (B) if the UI should
show character count instead, change the placeholder to {PASSWORD_MAX_CHARS} and
ensure the code supplies that value; target the passwordRequirements entry and
the placeholders PASSWORD_MAX_BYTES / PASSWORD_MAX_CHARS accordingly so the
label and variable type match.
---
Nitpick comments:
In `@frontend/app/api/auth/password-reset/confirm/route.ts`:
- Around line 11-23: The password validation logic is duplicated in the `schema`
object in this file and `signup/route.ts`; extract the shared zod chain into a
single exported symbol (e.g. `serverPasswordSchema`) in a new module (suggested
name: frontend/lib/auth/server-password-schema.ts) that imports the existing
constants `PASSWORD_MIN_LEN`, `PASSWORD_MAX_BYTES`, and `PASSWORD_POLICY_REGEX`
from `signup-constraints`; replace the inline
`.min/.regex/.regex/.regex/.refine(Buffer.byteLength...)` chain in both `schema`
(this file) and `signup/route.ts` by importing and using `serverPasswordSchema`
and keep the surrounding object shape (use `z.object({ token: z.string().uuid(),
password: serverPasswordSchema })` here).
In `@frontend/app/api/auth/signup/route.ts`:
- Around line 30-32: Remove the redundant .regex(PASSWORD_POLICY_REGEX, ...)
call in the password validation chain inside the signup route; the individual
.regex(/[A-Z]/, ...), .regex(/[^A-Za-z0-9]/, ...) and the minimum-length check
already enforce everything PASSWORD_POLICY_REGEX covers, so delete the
.regex(PASSWORD_POLICY_REGEX, 'Password does not meet the required policy')
invocation (leave the other validations intact).
- Line 52: The confirmPassword field in the signup Zod schema is incorrectly
using passwordSchema; change confirmPassword to a plain string schema (e.g.,
z.string()) instead of passwordSchema so only the main password is validated
against the policy and the existing superRefine equality check (superRefine on
the schema) handles the "Passwords do not match" error; update the schema where
confirmPassword is defined and leave passwordSchema and the superRefine equality
logic intact.
In `@frontend/components/auth/SignupForm.tsx`:
- Around line 31-33: The function utf8ByteLength duplicates an existing utility;
remove this local duplicate and import the centralized implementation from
password-bytes.ts instead. Replace the local utf8ByteLength definition in
SignupForm.tsx with an import of the utility (use the exported name from
password-bytes.ts) and update any local references to call that imported
function (ensure the import is added at the top and the duplicate function
removed).
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
frontend/app/api/auth/signup/route.ts (1)
80-91:⚠️ Potential issue | 🟡 MinorTOCTOU: concurrent signups with the same email can bypass the existence check.
The select (lines 80-84) and insert (lines 95-105) are not atomic. A duplicate-email insert will throw if the DB has a unique constraint on email (confirmed), but the user would see "Signup failed" instead of "Email already in use". Consider catching the unique-constraint DB error specifically, or wrapping the check+insert in a transaction with a serializable isolation level.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/app/api/auth/signup/route.ts` around lines 80 - 91, The current TOCTOU check using db.select(...).where(eq(users.email, normalizedEmail)) before inserting can be bypassed concurrently; modify the signup flow to handle unique-email races by catching the database unique-constraint error from the insert (the db.insert on the users table) and returning the 409 JSON response ("Email already in use") when that specific constraint error occurs, or alternatively perform the existence check and insert inside a transaction with serializable isolation; reference the existingUser check, the users table/insert call, normalizedEmail, and the NextResponse.json 409 response when implementing the specific error handling or transactional fix.
🧹 Nitpick comments (3)
frontend/components/auth/SignupForm.tsx (2)
137-157: Form submission reads fromFormDatawhile validation uses React state — potential for divergence.
onSubmitreadsname,password, andconfirmPasswordfromFormData(the DOM) but validation flags (nameLooksValid,passwordPolicyOk, etc.) derive from React state set viaonChange. If a field component's internal DOM value ever diverges from theonChange-reported value (e.g., browser autofill not triggering React'sonChange), submission could bypass or mismatch the validated state.Consider either using the React state values directly in the submission payload or re-validating the
FormDatavalues before sending.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/components/auth/SignupForm.tsx` around lines 137 - 157, onSubmit currently builds the payload from FormData while validation (nameLooksValid, passwordPolicyOk, etc.) comes from React state, which can diverge (e.g., autofill); fix by using the React state values (e.g., name, password, confirmPassword, email state updated by inputs) when constructing the JSON body in onSubmit (or alternatively re-run your existing validation helpers against the FormData and update validation flags before sending), and ensure you still call setEmail only from the authoritative source (state) so setLoading/setError behavior remains correct.
162-168: Server-side field-level validation errors are silently swallowed.The server returns
{ error: { name: [...], email: [...], ... } }(ZodfieldErrorsobject) on validation failure, but the client only surfacesdata.errorwhen it's astring(line 164). When it's an object, the user just sees the generict('errors.signupFailed'). Since client-side validation is the primary gate, this is a soft-landing fallback, but consider logging or displaying the first field error for robustness.♻️ Proposed enhancement
if (!res.ok) { const msg = typeof data?.error === 'string' ? data.error + : typeof data?.error === 'object' && data.error !== null + ? Object.values(data.error).flat().filter(Boolean)[0] as string + ?? t('errors.signupFailed') : t('errors.signupFailed'); setError(msg); return; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/components/auth/SignupForm.tsx` around lines 162 - 168, The signup error handling in SignupForm.tsx currently only shows data.error when it's a string and otherwise shows a generic t('errors.signupFailed'); update the logic inside the response failure branch (the block that checks if (!res.ok) in the submit/handleSignup function) to detect when data.error is an object (Zod fieldErrors), pick the first field key and its first message (or join messages) and pass that to setError so users see the specific field validation message; also log the full error object (e.g., console.error or a logger) for debugging while keeping the fallback to t('errors.signupFailed') if no field messages exist.frontend/app/api/auth/signup/route.ts (1)
24-36: Remove redundantPASSWORD_POLICY_REGEXcheck — it duplicates earlier validators and its error message is unreachable.The regex
/^(?=.*[A-Z])(?=.*[^A-Za-z0-9]).{8,}$/on line 32 validates exactly the same three conditions already checked individually: minimum 8 characters (line 26–29), at least one uppercase letter (line 30), and at least one special character (line 31). If all three earlier validators pass, the regex is guaranteed to pass; if any fails, the user sees a more specific error message. The generic "Password does not meet the required policy" message on line 32 can never be displayed.♻️ Proposed fix — remove the redundant regex
const passwordSchema = z .string() .min( PASSWORD_MIN_LEN, `Password must be at least ${PASSWORD_MIN_LEN} characters` ) .regex(/[A-Z]/, 'Password must contain at least one capital letter') .regex(/[^A-Za-z0-9]/, 'Password must contain at least one special character') - .regex(PASSWORD_POLICY_REGEX, 'Password does not meet the required policy') .refine( val => Buffer.byteLength(val, 'utf8') <= PASSWORD_MAX_BYTES, `Password must be at most ${PASSWORD_MAX_BYTES} bytes` );The
PASSWORD_POLICY_REGEXconstant should remain exported for client-side validation (used inSignupForm.tsxandResetPasswordForm.tsx), but it is redundant in the server-side schema.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/app/api/auth/signup/route.ts` around lines 24 - 36, The server-side password schema (passwordSchema) includes a redundant .regex(PASSWORD_POLICY_REGEX, ...) check that duplicates earlier validators (.min, .regex(/[A-Z]/), .regex(/[^A-Za-z0-9]/)) making its error unreachable; remove the .regex(PASSWORD_POLICY_REGEX, 'Password does not meet the required policy') entry from passwordSchema so the schema relies on the specific validators while leaving the exported PASSWORD_POLICY_REGEX constant unchanged for client-side use (SignupForm.tsx, ResetPasswordForm.tsx).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@frontend/components/auth/SignupForm.tsx`:
- Around line 31-33: Remove the local duplicate function utf8ByteLength in
SignupForm.tsx and replace it with an import from the centralized utility
(import { utf8ByteLength } from '@/lib/auth/password-bytes'); update any calls
to the local function to use the imported symbol and delete the local
declaration to avoid duplication and keep DRY.
---
Outside diff comments:
In `@frontend/app/api/auth/signup/route.ts`:
- Around line 80-91: The current TOCTOU check using
db.select(...).where(eq(users.email, normalizedEmail)) before inserting can be
bypassed concurrently; modify the signup flow to handle unique-email races by
catching the database unique-constraint error from the insert (the db.insert on
the users table) and returning the 409 JSON response ("Email already in use")
when that specific constraint error occurs, or alternatively perform the
existence check and insert inside a transaction with serializable isolation;
reference the existingUser check, the users table/insert call, normalizedEmail,
and the NextResponse.json 409 response when implementing the specific error
handling or transactional fix.
---
Duplicate comments:
In `@frontend/components/auth/SignupForm.tsx`:
- Around line 93-106: The email error logic in the useMemo for emailErrorText is
missing a check for EMAIL_MIN_LEN which causes submit to be disabled without
showing a message; update the emailErrorText computation (the useMemo that
references emailTouched, emailTrimmed, emailFormatOk, tf) to add a branch that
when emailTrimmed.length < EMAIL_MIN_LEN returns a localized short-email message
(e.g., tf('validation.emailTooShort', { EMAIL_MIN_LEN })); place this check
after the empty checks and before the format/max-length checks so
short-but-formatted addresses produce the correct error.
- Around line 113-119: The generic and byte-length password errors can both show
because passwordPolicyOk is false when utf8ByteLength(passwordValue) >
PASSWORD_MAX_BYTES; update the passwordErrorText computation to prefer the
byte-length message when utf8ByteLength(passwordValue) > PASSWORD_MAX_BYTES (use
passwordTouched and utf8ByteLength(passwordValue) > PASSWORD_MAX_BYTES to return
the specific byte-limit tf message), falling back to the existing
tf('validation.invalidPassword', { passwordRequirementsText }) otherwise, and
remove the separate passwordBytesTooLong rendering block so only
passwordErrorText is displayed; reference variables to change:
passwordErrorText, passwordTouched, passwordPolicyOk,
utf8ByteLength(passwordValue), PASSWORD_MAX_BYTES, passwordBytesTooLong, and the
standalone render block that currently uses passwordBytesTooLong.
---
Nitpick comments:
In `@frontend/app/api/auth/signup/route.ts`:
- Around line 24-36: The server-side password schema (passwordSchema) includes a
redundant .regex(PASSWORD_POLICY_REGEX, ...) check that duplicates earlier
validators (.min, .regex(/[A-Z]/), .regex(/[^A-Za-z0-9]/)) making its error
unreachable; remove the .regex(PASSWORD_POLICY_REGEX, 'Password does not meet
the required policy') entry from passwordSchema so the schema relies on the
specific validators while leaving the exported PASSWORD_POLICY_REGEX constant
unchanged for client-side use (SignupForm.tsx, ResetPasswordForm.tsx).
In `@frontend/components/auth/SignupForm.tsx`:
- Around line 137-157: onSubmit currently builds the payload from FormData while
validation (nameLooksValid, passwordPolicyOk, etc.) comes from React state,
which can diverge (e.g., autofill); fix by using the React state values (e.g.,
name, password, confirmPassword, email state updated by inputs) when
constructing the JSON body in onSubmit (or alternatively re-run your existing
validation helpers against the FormData and update validation flags before
sending), and ensure you still call setEmail only from the authoritative source
(state) so setLoading/setError behavior remains correct.
- Around line 162-168: The signup error handling in SignupForm.tsx currently
only shows data.error when it's a string and otherwise shows a generic
t('errors.signupFailed'); update the logic inside the response failure branch
(the block that checks if (!res.ok) in the submit/handleSignup function) to
detect when data.error is an object (Zod fieldErrors), pick the first field key
and its first message (or join messages) and pass that to setError so users see
the specific field validation message; also log the full error object (e.g.,
console.error or a logger) for debugging while keeping the fallback to
t('errors.signupFailed') if no field messages exist.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
frontend/app/api/auth/password-reset/confirm/route.ts (1)
23-32: RedundantPASSWORD_POLICY_REGEXcheck — its error message is unreachable.
PASSWORD_POLICY_REGEX = /^(?=.*[A-Z])(?=.*[^A-Za-z0-9]).{8,}$/tests the exact same three conditions as the three preceding validators (.min,/[A-Z]/,/[^A-Za-z0-9]/). Because Zod short-circuits on the first failure, a password that passes checks 1–3 always passes check 4, and a password that fails check 4 also fails at least one of checks 1–3 first. The error string'Password does not meet the required policy'can therefore never be shown.♻️ Suggested cleanup
- .regex(PASSWORD_POLICY_REGEX, 'Password does not meet the required policy') - .refine( + .refine(🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/app/api/auth/password-reset/confirm/route.ts` around lines 23 - 32, Remove the redundant .regex(PASSWORD_POLICY_REGEX, 'Password does not meet the required policy') validator because PASSWORD_POLICY_REGEX duplicates the checks already enforced by .min(8), .regex(/[A-Z]/, ...) and .regex(/[^A-Za-z0-9]/, ...); update the password schema by deleting that .regex call so the remaining validators (.min, the two .regex checks and the .refine for byte length) continue to provide the required policy and reachable error messages.frontend/components/auth/ResetPasswordForm.tsx (1)
21-23: Remove the localutf8ByteLengthfunction and import from the shared utility module.The function is already exported from
@/lib/auth/password-bytes.tswith identical implementation. Eliminate this duplication by importing it instead of defining it locally.♻️ Proposed fix
Remove the local definition:
-function utf8ByteLength(value: string): number { - return new TextEncoder().encode(value).length; -} -Add the import at the top of the file:
+import { utf8ByteLength } from '@/lib/auth/password-bytes';🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/components/auth/ResetPasswordForm.tsx` around lines 21 - 23, Remove the local utf8ByteLength function in ResetPasswordForm.tsx and instead import the shared implementation from "@/lib/auth/password-bytes.ts"; specifically delete the function declaration for utf8ByteLength and add an import for the exported symbol utf8ByteLength at the top of the file, then ensure all uses in the component (e.g., where utf8ByteLength(...) is called) refer to the imported function.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@frontend/components/auth/ResetPasswordForm.tsx`:
- Around line 49-56: passwordErrorText currently shows whenever passwordTouched
&& !passwordPolicyOk, which causes it to render alongside the specific
bytes-too-long message when utf8ByteLength(passwordValue) > PASSWORD_MAX_BYTES;
update the passwordErrorText condition to exclude the bytes-too-long case (use
the existing passwordBytesTooLong predicate or inline the utf8ByteLength check)
so that passwordErrorText only appears when passwordTouched && !passwordPolicyOk
&& !passwordBytesTooLong, leaving the dedicated passwordBytesTooLong paragraph
to show exclusively for byte-limit violations.
---
Nitpick comments:
In `@frontend/app/api/auth/password-reset/confirm/route.ts`:
- Around line 23-32: Remove the redundant .regex(PASSWORD_POLICY_REGEX,
'Password does not meet the required policy') validator because
PASSWORD_POLICY_REGEX duplicates the checks already enforced by .min(8),
.regex(/[A-Z]/, ...) and .regex(/[^A-Za-z0-9]/, ...); update the password schema
by deleting that .regex call so the remaining validators (.min, the two .regex
checks and the .refine for byte length) continue to provide the required policy
and reachable error messages.
In `@frontend/components/auth/ResetPasswordForm.tsx`:
- Around line 21-23: Remove the local utf8ByteLength function in
ResetPasswordForm.tsx and instead import the shared implementation from
"@/lib/auth/password-bytes.ts"; specifically delete the function declaration for
utf8ByteLength and add an import for the exported symbol utf8ByteLength at the
top of the file, then ensure all uses in the component (e.g., where
utf8ByteLength(...) is called) refer to the imported function.
ViktorSvertoka
approved these changes
Feb 20, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
New Features
Bug Fixes
Localization