WEB-562:fix(auth) align password validation with backend regex requir… - #2990
Conversation
|
Note
|
| Cohort / File(s) | Summary |
|---|---|
Core Password Validation Utilities src/app/core/utils/password.validator.ts, src/app/core/utils/passwords-utility.ts |
Updated regex checks: replaced prior consecutive-character logic with (.)\\1-based negative lookahead, broadened special-character set to any non-alphanumeric non-space character, and added whitespace rejection. Minor comment removal. |
Component Password Validation src/app/zitadel/shared/change-password-dialog/change-password-dialog.component.ts, src/app/zitadel/users/create-user/create-user.component.ts |
Aligned form validators with updated core pattern: disallow repeated characters, forbid whitespace, require at least one non-word/non-space special character, and preserve existing length and case/digit requirements. |
Estimated code review effort
🎯 3 (Moderate) | ⏱️ ~20 minutes
Suggested reviewers
- alberto-art3ch
- IOhacker
🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 warning)
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage | Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. | Write docstrings for the functions missing them to satisfy the coverage threshold. |
✅ Passed checks (2 passed)
| Check name | Status | Explanation |
|---|---|---|
| Description Check | ✅ Passed | Check skipped - CodeRabbit’s high-level summary is enabled. |
| Title check | ✅ Passed | The title clearly identifies the main objective: aligning frontend password validation with backend regex requirements to fix authentication validation issues. |
✏️ Tip: You can configure your own custom pre-merge checks in the settings.
✨ Finishing touches
- 📝 Generate docstrings
📜 Recent review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/app/core/utils/password.validator.tssrc/app/core/utils/passwords-utility.tssrc/app/zitadel/shared/change-password-dialog/change-password-dialog.component.tssrc/app/zitadel/users/create-user/create-user.component.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- src/app/core/utils/passwords-utility.ts
- src/app/zitadel/shared/change-password-dialog/change-password-dialog.component.ts
- src/app/zitadel/users/create-user/create-user.component.ts
🧰 Additional context used
📓 Path-based instructions (1)
src/app/**
⚙️ CodeRabbit configuration file
src/app/**: For Angular code: verify component separation, trackBy on *ngFor,
strict type safety, and clean observable patterns.
Files:
src/app/core/utils/password.validator.ts
🔇 Additional comments (3)
src/app/core/utils/password.validator.ts (3)
34-37: LGTM!The
(.)\1pattern correctly detects consecutive repeating characters. This is a clean simplification from what was likely a more complex pattern before.
42-45: LGTM!The whitespace check using
\scorrectly rejects all whitespace characters (spaces, tabs, newlines). This aligns with the updated special character check which also uses\sfor consistency.
38-41: Regex correctly excludes underscore as special character, but consistency with backend requirements cannot be verified from frontend code alone.The observation is accurate: the regex
[^\w\s]excludes underscore from special characters because\wincludes[A-Za-z0-9_]. A password likePassword_123would fail validation. However, this pattern is consistently applied across the codebase—thePASSWORD_REGEXinpasswords-utility.ts(line 21) and the Validators.pattern inchange-password-dialog.component.ts(line 63) all use the same[^\w\s]pattern.Note: The suggested alternative
[^A-Za-z0-9\s]does not change this behavior; it also excludes underscore. If underscore should be accepted as a special character, the pattern should be[^A-Za-z0-9\s_]instead.The frontend implementation is internally consistent, but verification against backend password requirements is not possible from this repository alone. Backend validation rules should be checked in the backend codebase to ensure alignment.
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 help to get the list of available commands and usage tips.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @src/app/core/utils/password.validator.ts:
- Around line 34-37: The validation message assigned to errors['repeated'] is
grammatically incorrect; update the string set in the repeated-check branch (the
if (/(.)\1/.test(value)) block that assigns errors['repeated']) to a correct
phrasing such as "Password must not contain consecutive repeating characters" or
"Password must not have consecutive repeating characters" so the error message
is clear and grammatically correct.
🧹 Nitpick comments (3)
src/app/zitadel/users/create-user/create-user.component.ts (2)
146-154: Consider using the injectedPasswordsUtilityinstead of inline validators.The regex pattern is correct and aligns with backend validation. However,
PasswordsUtilityis already injected (line 57) and providesgetPasswordValidators()which includes the same validation rules pluspasswordValidator()for detailed error messages.Using the shared utility would reduce duplication and ensure validation rules stay synchronized across the codebase.
♻️ Suggested refactor
password: [ '', - [ - Validators.required, - Validators.minLength(12), - Validators.maxLength(50), - Validators.pattern(/^(?!.*(.)\1)(?!.*\s)(?=.*[a-z])(?=.*[A-Z])(?=.*\d)(?=.*[^\w\s]).+$/) - ] + this.passwordsUtility.getPasswordValidators() ],
176-183: Unmanaged subscription may cause memory leak.The subscription to
valueChangesis not unsubscribed when the component is destroyed. Consider usingtakeUntilDestroyed()or storing the subscription to unsubscribe inngOnDestroy. As per coding guidelines, clean observable patterns should be used.♻️ Example using takeUntilDestroyed
+import { DestroyRef, inject } from '@angular/core'; +import { takeUntilDestroyed } from '@angular/core/rxjs-interop'; export class CreateUserComponent implements OnInit, AfterViewInit { + private destroyRef = inject(DestroyRef); // ... setStaffData() { - this.userForm.get('officeId').valueChanges.subscribe((officeId: string) => { + this.userForm.get('officeId').valueChanges.pipe( + takeUntilDestroyed(this.destroyRef) + ).subscribe((officeId: string) => { this.staffData = []; this.usersService2.getStaff(officeId).subscribe((staff: any) => { this.staffData = staff; }); }); }src/app/zitadel/shared/change-password-dialog/change-password-dialog.component.ts (1)
57-65: Consider usingPasswordsUtilityfor consistent validation.The regex pattern is correct and aligns with the backend validation requirements. However, this is the same pattern duplicated in
CreateUserComponentand similar toPasswordsUtility.PASSWORD_REGEX.For maintainability, consider injecting
PasswordsUtilityand usinggetPasswordValidators()to ensure all password fields use the same validation rules.♻️ Suggested refactor
+import { PasswordsUtility } from 'app/core/utils/passwords-utility'; export class ChangePasswordDialogComponent implements OnInit { + private passwordsUtility = inject(PasswordsUtility); // ... createChangePasswordForm(): void { this.changePasswordForm = this.formBuilder.group( { currentPassword: ['', Validators.required], password: [ '', - [ - Validators.required, - Validators.minLength(this.minPasswordLength), - Validators.maxLength(50), - Validators.pattern(/^(?!.*(.)\1)(?!.*\s)(?=.*[a-z])(?=.*[A-Z])(?=.*\d)(?=.*[^\w\s]).+$/) - ] + this.passwordsUtility.getPasswordValidators() ],
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/app/core/utils/password.validator.tssrc/app/core/utils/passwords-utility.tssrc/app/zitadel/shared/change-password-dialog/change-password-dialog.component.tssrc/app/zitadel/users/create-user/create-user.component.ts
🧰 Additional context used
📓 Path-based instructions (1)
src/app/**
⚙️ CodeRabbit configuration file
src/app/**: For Angular code: verify component separation, trackBy on *ngFor,
strict type safety, and clean observable patterns.
Files:
src/app/core/utils/password.validator.tssrc/app/zitadel/users/create-user/create-user.component.tssrc/app/zitadel/shared/change-password-dialog/change-password-dialog.component.tssrc/app/core/utils/passwords-utility.ts
🔇 Additional comments (2)
src/app/core/utils/passwords-utility.ts (1)
20-23: LGTM - Regex updated correctly for repeated character detection.The change from
(?!.*(.)\1{1,})to(?!.*(.)\1)correctly simplifies the detection of consecutive repeating characters. Both patterns achieve the same result, but the new version is more concise.Consider extracting this regex to be reused by other components (
CreateUserComponent,ChangePasswordDialogComponent) that currently define their own inline patterns, to avoid duplication and ensure consistency.src/app/core/utils/password.validator.ts (1)
38-45: LGTM - Special character and whitespace validation correctly implemented.The special character check
[^\w\s]properly requires a non-alphanumeric, non-space character (excluding underscore from valid special characters). The whitespace check ensures passwords cannot contain spaces. Both align with the backend validation requirements.
…ements fix the grammar
7116cf7 to
4db9499
Compare
2 participants
…ements
This PR fixes password validation in the frontend to align with the backend validation rules defined in Fineract. Previously, the frontend was rejecting valid passwords that contained special characters like #, ^, (, ), etc., and had incorrect logic for detecting repeating characters.
WEB-562
Checklist
Please make sure these boxes are checked before submitting your pull request - thanks!
If you have multiple commits please combine them into one commit by squashing them.
Read and understood the contribution guidelines at
web-app/.github/CONTRIBUTING.md.Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.