feat(profile): implement password management with OWASP validation #95
Conversation
- Add password change functionality with current/new/confirm fields - Implement OWASP password validation (8+ chars, 3 of 4 character types) - Add real-time password strength indicator with visual feedback - Implement password reset email functionality for current user - Add 2FA status display and external setup link - Use signal-based state management for reactive updates - Add comprehensive form validation and error handling - Include data-testid attributes for all interactive elements LFXV2-528 Signed-off-by: Asitha de Silva <asithade@gmail.com>
WalkthroughRefactors profile edit form to use a shared control-touch utility. Replaces the profile password placeholder with a fully functional, signal-driven password management UI and logic. Adds password/2FA API methods to UserService. Introduces shared interfaces for password requests, strength, and two-factor settings. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor U as User
participant C as ProfilePasswordComponent
participant US as UserService
participant API as Backend API
participant MS as MessageService
U->>C: Click "Change Password"
C->>C: Validate form (match/strength)
alt Invalid
C->>U: Show form validation errors
else Valid
C->>US: changePassword({current,new})
US->>API: POST /api/profile/change-password
API-->>US: { message } or error
US-->>C: Success or error
opt Finalize
C->>C: Reset loading state
end
alt Success
C->>MS: showSuccess(message)
C->>C: Reset/untouch form
else Error
C->>MS: showError(error)
end
end
sequenceDiagram
autonumber
actor U as User
participant C as ProfilePasswordComponent
participant US as UserService
participant API as Backend API
participant MS as MessageService
Note over C: Two-Factor settings load (on init or refresh)
C->>US: getTwoFactorSettings()
US->>API: GET /api/profile/2fa-settings
API-->>US: TwoFactorSettings or error
US-->>C: TwoFactorSettings|null
C->>C: Update signals for UI state
U->>C: Click "Send Reset Link"
C->>US: sendPasswordResetEmail()
US->>API: POST /api/profile/reset-password
API-->>US: { message } or error
US-->>C: Response
alt Success
C->>MS: showSuccess(message)
else Error
C->>MS: showError(error)
end
C->>C: Reset sending state
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests
Comment |
github-actions
bot
requested review from
andrest50,
audigregorie,
dealako and
mauriciozanettisalomao
There was a problem hiding this comment.
Pull Request Overview
This PR implements comprehensive password management functionality for user profiles with OWASP-compliant validation, adding password change forms, strength indicators, reset functionality, and 2FA integration.
Key Changes
- Password Management Interface: Added comprehensive TypeScript interfaces for password operations and 2FA settings
- Service Layer Enhancement: Extended UserService with password change, reset, and 2FA endpoints
- Complete UI Implementation: Built full password management component with real-time validation, strength indicators, and 2FA status display
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/shared/src/interfaces/user-profile.interface.ts | Adds TypeScript interfaces for password operations and 2FA settings |
| apps/lfx-one/src/app/shared/services/user.service.ts | Extends UserService with password management and 2FA API methods |
| apps/lfx-one/src/app/modules/profile/password/profile-password.component.ts | Implements complete password management component with validation and state management |
| apps/lfx-one/src/app/modules/profile/password/profile-password.component.html | Creates comprehensive UI for password change, reset, and 2FA management |
| apps/lfx-one/src/app/modules/profile/edit/profile-edit.component.ts | Refactors to use shared utility function for form validation |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
apps/lfx-one/src/app/modules/profile/password/profile-password.component.ts
Show resolved
Hide resolved
apps/lfx-one/src/app/modules/profile/password/profile-password.component.html
Show resolved
Hide resolved
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (8)
packages/shared/src/interfaces/user-profile.interface.ts (3)
132-134: PasswordResetRequest doesn’t match current API usage.Service sends an empty body. Make
Apply one of these diffs:
-export interface PasswordResetRequest { - email: string; -} +// Not used by current API (server infers email). Keep optional for future use. +export interface PasswordResetRequest { + email?: string; +}—or—
-/** - * Request to send password reset email - */ -export interface PasswordResetRequest { - email: string; -} +// Removed: API infers user email; no payload needed.
155-160: backup_codes_count should be optional to match template checks.Template guards with
!== undefined. Mark the field optional to reflect API variability.-export interface TwoFactorSettings { +export interface TwoFactorSettings { enabled: boolean; method: 'app' | 'sms' | 'email' | null; - backup_codes_count: number; + backup_codes_count?: number; last_used: string | null; }
141-149: Prefer enums over string literal unions for shared contracts.Improves discoverability and refactor safety across apps.
+export enum PasswordStrengthLabel { + weak = 'weak', + fair = 'fair', + good = 'good', + strong = 'strong', +} + +export enum TwoFactorMethod { + app = 'app', + sms = 'sms', + email = 'email', +} export interface PasswordStrength { score: number; // 0-4 (weak to strong) - label: 'weak' | 'fair' | 'good' | 'strong'; + label: PasswordStrengthLabel; requirements: { minLength: boolean; hasLowercase: boolean; hasUppercase: boolean; hasNumbers: boolean; hasSpecialChars: boolean; meetsCriteria: boolean; // true if 3 of 4 character types are present }; } export interface TwoFactorSettings { enabled: boolean; - method: 'app' | 'sms' | 'email' | null; + method: TwoFactorMethod | null; backup_codes_count?: number; last_used: string | null; }Please confirm no downstream code relies on string unions before we switch to enums.
Also applies to: 156-158
apps/lfx-one/src/app/shared/services/user.service.ts (1)
41-43: Consistency nit: favortake(1)for one-shot HTTP calls.Other methods use it; align
getCurrentUserProfile(and optionallygetUserEmails) for consistency.- public getCurrentUserProfile(): Observable<CombinedProfile> { - return this.http.get<CombinedProfile>('/api/profile'); - } + public getCurrentUserProfile(): Observable<CombinedProfile> { + return this.http.get<CombinedProfile>('/api/profile').pipe(take(1)); + }Also consider giving
updateProfileDetailsa concrete return type instead ofany.Can the backend return the updated
ProfileDetails? If yes, we can type it.Also applies to: 64-66, 55-57
apps/lfx-one/src/app/modules/profile/password/profile-password.component.html (1)
48-49: [class] overrides static classes; use [ngClass] to merge.Current pattern risks dropping
text-gray-400 hover:text-gray-600.- <i [class]="showCurrentPassword() ? 'fa-light fa-eye-slash' : 'fa-light fa-eye'" class="text-gray-400 hover:text-gray-600"></i> + <i [ngClass]="showCurrentPassword() ? 'fa-light fa-eye-slash' : 'fa-light fa-eye'" class="text-gray-400 hover:text-gray-600" aria-hidden="true"></i> - <i [class]="showNewPassword() ? 'fa-light fa-eye-slash' : 'fa-light fa-eye'" class="text-gray-400 hover:text-gray-600"></i> + <i [ngClass]="showNewPassword() ? 'fa-light fa-eye-slash' : 'fa-light fa-eye'" class="text-gray-400 hover:text-gray-600" aria-hidden="true"></i> - <i [class]="showConfirmPassword() ? 'fa-light fa-eye-slash' : 'fa-light fa-eye'" class="text-gray-400 hover:text-gray-600"></i> + <i [ngClass]="showConfirmPassword() ? 'fa-light fa-eye-slash' : 'fa-light fa-eye'" class="text-gray-400 hover:text-gray-600" aria-hidden="true"></i>Also applies to: 73-74, 157-158
apps/lfx-one/src/app/modules/profile/password/profile-password.component.ts (3)
100-137: Success/error flows are clean; small UX tweak.After success, consider focusing the first field or showing a brief toast duration to improve feedback.
170-174: Prevent reverse‑tabnabbing when opening external links.Add
noopener,noreferrer.- window.open('https://docs.example.com/2fa-setup', '_blank'); + window.open('https://docs.example.com/2fa-setup', '_blank', 'noopener,noreferrer');
195-223: UI text vs. regex discrepancy for “special characters.”Regex allows a wider set than the checklist text. Align them to avoid confusion.
Option A (update copy): “Special character (e.g., !@#$%^&*)”.
Option B (restrict regex to displayed set):- hasSpecialChars: /[!@#$%^&*(),.?":{}|<>]/.test(password), + hasSpecialChars: /[!@#$%^&*]/.test(password),Confirm which set the policy intends to enforce.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (5)
apps/lfx-one/src/app/modules/profile/edit/profile-edit.component.ts(2 hunks)apps/lfx-one/src/app/modules/profile/password/profile-password.component.html(1 hunks)apps/lfx-one/src/app/modules/profile/password/profile-password.component.ts(1 hunks)apps/lfx-one/src/app/shared/services/user.service.ts(2 hunks)packages/shared/src/interfaces/user-profile.interface.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
packages/shared/src/interfaces/**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Place all TypeScript interfaces in the shared package at packages/shared/src/interfaces
Files:
packages/shared/src/interfaces/user-profile.interface.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Use TypeScript interfaces instead of union types for better maintainability
When defining PrimeNG-related types, reference the official PrimeNG component interfaces
Files:
packages/shared/src/interfaces/user-profile.interface.tsapps/lfx-one/src/app/shared/services/user.service.tsapps/lfx-one/src/app/modules/profile/edit/profile-edit.component.tsapps/lfx-one/src/app/modules/profile/password/profile-password.component.ts
**/*.{ts,tsx,js,jsx,mjs,cjs,html,css,scss}
📄 CodeRabbit inference engine (CLAUDE.md)
Include required license headers on all source files
Files:
packages/shared/src/interfaces/user-profile.interface.tsapps/lfx-one/src/app/shared/services/user.service.tsapps/lfx-one/src/app/modules/profile/edit/profile-edit.component.tsapps/lfx-one/src/app/modules/profile/password/profile-password.component.htmlapps/lfx-one/src/app/modules/profile/password/profile-password.component.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Do not nest ternary expressions
Files:
packages/shared/src/interfaces/user-profile.interface.tsapps/lfx-one/src/app/shared/services/user.service.tsapps/lfx-one/src/app/modules/profile/edit/profile-edit.component.tsapps/lfx-one/src/app/modules/profile/password/profile-password.component.ts
apps/lfx-one/src/**/*.html
📄 CodeRabbit inference engine (CLAUDE.md)
apps/lfx-one/src/**/*.html: Always add data-testid attributes when creating new Angular components for reliable test targeting
Use data-testid naming convention [section]-[component]-[element]
Files:
apps/lfx-one/src/app/modules/profile/password/profile-password.component.html
🧬 Code graph analysis (3)
apps/lfx-one/src/app/shared/services/user.service.ts (1)
packages/shared/src/interfaces/user-profile.interface.ts (2)
ChangePasswordRequest(124-127)TwoFactorSettings(155-160)
apps/lfx-one/src/app/modules/profile/edit/profile-edit.component.ts (1)
packages/shared/src/utils/form.utils.ts (1)
markFormControlsAsTouched(16-19)
apps/lfx-one/src/app/modules/profile/password/profile-password.component.ts (3)
apps/lfx-one/src/app/modules/profile/edit/profile-edit.component.ts (1)
Component(21-243)packages/shared/src/interfaces/user-profile.interface.ts (3)
TwoFactorSettings(155-160)ChangePasswordRequest(124-127)PasswordStrength(139-150)packages/shared/src/utils/form.utils.ts (1)
markFormControlsAsTouched(16-19)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Code Quality Checks
🔇 Additional comments (5)
apps/lfx-one/src/app/modules/profile/edit/profile-edit.component.ts (2)
8-8: Good move: centralize touched-state logic via shared util.Importing
markFormControlsAsTouchedfrom shared keeps behavior consistent across forms.
120-124: Form invalid handling is correct.Using the shared
markFormControlsAsTouched(this.profileForm)before early‑return is aligned with our patterns.apps/lfx-one/src/app/shared/services/user.service.ts (1)
104-125: Password/2FA endpoints look good and typed.Methods are concise, use POST/GET appropriately, and
take(1)ensures completion.apps/lfx-one/src/app/modules/profile/password/profile-password.component.html (1)
21-181: Template structure and test hooks are solid.Comprehensive
data-testidcoverage, a11y labels on toggles, and clear state handling.Also applies to: 186-216, 220-316
apps/lfx-one/src/app/modules/profile/password/profile-password.component.ts (1)
225-256: Validators correctly enforce OWASP basics (min 8 + 3 of 4).Group-level match validator and field-level strength validator are succinct and readable.
✅ E2E Tests PassedBrowser: chromium All E2E tests passed successfully. Test Configuration
|
🧹 Deployment RemovedThe deployment for PR #95 has been removed. |
Summary
Implements comprehensive password management functionality for user profiles with OWASP-compliant validation.
Changes
Technical Implementation
Testing
password-[section]-[element]JIRA
LFXV2-528
Checklist