fix: enforce consistent password validation across all flows#785
Conversation
Password rules (8+ chars, lowercase, uppercase, number, special char) were only fully enforced on sign-up. Add Member, Edit Member, Change Password, and Set Password forms had weaker or no validation. - Create shared password schema (lib/shared/schemas/password.ts) - Add usePasswordValidation hook for consistent UI display - Update all 5 frontend forms to use shared schema + ValidationCheckList - Add backend validation in set_member_password and update_user_password - Add missing specialChar translation key for changePassword requirements
There was a problem hiding this comment.
Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (5)
📝 WalkthroughWalkthroughThis PR centralizes password validation by adding a shared password schema module and a usePasswordValidation hook, replacing inline password rules with Zod-based schemas across sign-up, account settings, and member management forms. It adds runtime password validation UI (ValidationCheckList) driven by the hook, updates server-side Convex handlers to validate passwords before hashing, introduces unit tests for password rules, and adds a translation key for the special-character requirement. Public component and function signatures were not changed. Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
📝 Coding Plan
Comment |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@services/platform/app/features/settings/organization/components/member-edit-dialog.tsx`:
- Around line 73-80: The password schema created by createOptionalPasswordSchema
is always validated even when updatePassword is false, causing hidden/invalid
passwords to block submission; update the form so password is only
required/validated when updatePassword is true—either (A) modify the Zod schema
for the member-edit-dialog form to use .superRefine() on the parent object to
conditionally require/validate password based on the sibling updatePassword flag
(reference: createOptionalPasswordSchema, updatePassword, password), or (B) when
the updatePassword checkbox toggles off call react-hook-form's
resetField('password') or unregister('password') to remove validation/state for
password so it no longer blocks submit.
In `@services/platform/convex/users/set_member_password.ts`:
- Around line 92-96: The password validity check
(isPasswordValid(args.newPassword)) currently runs after several adapter/DB
reads; move this check to immediately after successful authentication in the
setMemberPassword (or set_member_password) handler so we fail fast for invalid
input and avoid unnecessary adapter queries. Concretely, after verifying auth
(the block that validates the caller/session) and before any
adapter.get/adapter.query calls, add the existing isPasswordValid condition and
throw the same Error if it fails; keep the exact error message and use
args.newPassword as before.
In `@services/platform/convex/users/update_user_password.ts`:
- Around line 24-30: Move the authentication call before performing password
validation to avoid leaking password-policy info to unauthenticated clients:
call authComponent.getAuth(createAuth, ctx) (and destructure { auth, headers })
prior to invoking isPasswordValid(args.newPassword), and only throw the password
policy Error after successful authentication; update the function containing
these symbols (isPasswordValid, authComponent.getAuth, createAuth, ctx) so auth
is obtained first, then validate the password and throw the existing error if
invalid.
In `@services/platform/lib/shared/schemas/password.test.ts`:
- Around line 154-162: The tests 'rejects an invalid non-empty password' and
'rejects password missing only special char' only assert result.success is
false; update both cases to also assert the validation error message so
optional-schema mapping is correct—after calling schema.safeParse('weak') and
schema.safeParse('Abcdefg1') inspect result.error (e.g.
result.error.errors[0].message) and assert it equals the expected password
validation message used by your schema (the same message other tests expect),
ensuring the wrong error-message mapping for optional passwords is caught;
reference schema.safeParse and the two test names when making the change.
In `@services/platform/lib/shared/schemas/password.ts`:
- Around line 67-73: The optional password .refine currently always reports
messages.minLength for any failure; update the refine on the password schema to
return an appropriate complexity message instead of the static minLength one —
i.e., change the error payload used in the .refine that calls
isPasswordValid(val) from messages.minLength to a specific complexity message
(e.g., messages.passwordComplexity) or determine the correct message from
isPasswordValid, so that failures for uppercase/number/special-char surface the
proper error; reference the .refine call and the isPasswordValid function when
making this change.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 0c4fe145-17d4-4243-802c-b8d0fb309ada
📒 Files selected for processing (10)
services/platform/app/features/settings/account/components/account-form.tsxservices/platform/app/features/settings/organization/components/member-add-dialog.tsxservices/platform/app/features/settings/organization/components/member-edit-dialog.tsxservices/platform/app/hooks/use-password-validation.tsservices/platform/app/routes/_auth/sign-up.tsxservices/platform/convex/users/set_member_password.tsservices/platform/convex/users/update_user_password.tsservices/platform/lib/shared/schemas/password.test.tsservices/platform/lib/shared/schemas/password.tsservices/platform/messages/en.json
- Use superRefine in createOptionalPasswordSchema for specific error messages - Reset password field when edit member checkbox is toggled off - Move password validation after auth check to prevent policy probing - Move validation before DB reads in set_member_password for fail-fast - Add error message assertions in optional schema tests
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
Summary
Fixes #784
lib/shared/schemas/password.ts) with all 5 rules: min 8 chars, lowercase, uppercase, number, special characterset_member_passwordandupdate_user_passwordConvex functionsusePasswordValidationhook for consistentValidationCheckListdisplay across all formsBefore
After
All flows enforce all 5 rules consistently, both on frontend (Zod schemas) and backend.
Test plan
Summary by CodeRabbit
New Features
Tests