WEB-628: Standardize password minimum length validation and error handling - #3234
Conversation
|
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:
Note
|
| Cohort / File(s) | Summary |
|---|---|
Environment configuration src/environments/environment.ts, src/environments/environment.prod.ts, src/assets/env.js, src/assets/env.template.js |
Introduce passwordRegex and compute minPasswordLength via a resolved/fallback value (default 8); surface min/passwordRegex via runtime env variables. |
Password utility src/app/core/utils/passwords-utility.ts |
PasswordsUtility.minPasswordLength and PASSWORD_REGEX now use environment.minPasswordLength / environment.passwordRegex; added DEFAULT_PASSWORD_REGEX. Removed inline ` |
Login form src/app/login/login-form/login-form.component.ts, src/app/login/login-form/login-form.component.html |
Expose minPasswordLength from environment; FormControl minlength uses env value; template and getErrorMessage updated to show minlength error and pass requiredLength/min to translations. |
Translations (i18n) src/assets/translations/* src/assets/translations/en-US.json, src/assets/translations/cs-CS.json, src/assets/translations/de-DE.json, src/assets/translations/es-CL.json, src/assets/translations/es-MX.json, src/assets/translations/fr-FR.json, src/assets/translations/it-IT.json, src/assets/translations/ko-KO.json, src/assets/translations/lt-LT.json, src/assets/translations/lv-LV.json, src/assets/translations/ne-NE.json, src/assets/translations/pt-PT.json, src/assets/translations/sw-SW.json |
Updated "Password is required" messages to include a {{min}} placeholder for displaying configured minimum length. |
Sequence Diagram(s)
sequenceDiagram
participant User
participant LoginForm as Login Form
participant Env as Environment
participant Validators as PasswordsUtility
participant I18n as Translations
User->>LoginForm: enters password / triggers validation
LoginForm->>Env: read minPasswordLength & passwordRegex
LoginForm->>Validators: validate(password, minPasswordLength, passwordRegex)
Validators-->>LoginForm: result (valid | minlength | pattern)
LoginForm->>I18n: request error message with params (min / requiredLength)
I18n-->>LoginForm: localized message with {{min}} populated
LoginForm-->>User: display validation message
Estimated code review effort
🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
- WEB-562:fix(auth) align password validation with backend regex requir… #2990: Overlaps on PASSWORD_REGEX and password validation initialization in
src/app/core/utils/passwords-utility.ts.
Suggested reviewers
- IOhacker
- gkbishnoi07
- alberto-art3ch
🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
| Check name | Status | Explanation |
|---|---|---|
| Description Check | ✅ Passed | Check skipped - CodeRabbit’s high-level summary is enabled. |
| Title check | ✅ Passed | The title clearly and specifically describes the main change: standardizing password minimum length validation and error handling across the codebase, which aligns with the primary objective of the PR. |
| Docstring Coverage | ✅ Passed | No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. |
✏️ Tip: You can configure your own custom pre-merge checks in the settings.
✨ Finishing Touches
🧪 Generate unit tests (beta)
- Create PR with unit tests
- Post copyable unit tests in a comment
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.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/app/login/login-form/login-form.component.ts (1)
148-151: KeepgetErrorMessage()localized to avoid future message drift.Line 150 returns a hardcoded English message; consider using translated keys here too for consistency with template-driven errors.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/login/login-form/login-form.component.ts` around lines 148 - 151, The hardcoded English message inside getErrorMessage() should be replaced with a localized message: use the app's i18n/translation facility (e.g., TranslateService or i18n key lookup) in getErrorMessage() to return a translated string key like 'errors.minLength' with the requiredLength as a parameter (reference control and the requiredLength extraction logic already in getErrorMessage()); ensure getErrorMessage() returns TranslateService.instant('errors.minLength', {count: requiredLength}) or the equivalent translation helper used across the component so the minlength message is localized consistently with other template errors.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/app/login/login-form/login-form.component.html`:
- Around line 62-63: Replace the hardcoded English minlength message in the
login-form template with a localized string: change the <mat-error> that uses
loginForm.controls.password.errors?.minlength?.requiredLength to render a
translation key (e.g., labels.commons["Password minimum length must be"]) and
pass the requiredLength as the interpolation param (min). Update or add the
corresponding entry in your translations (e.g.,
src/assets/translations/en-US.json under labels.commons) so the key maps to
"Minimum length is {{min}}".
---
Nitpick comments:
In `@src/app/login/login-form/login-form.component.ts`:
- Around line 148-151: The hardcoded English message inside getErrorMessage()
should be replaced with a localized message: use the app's i18n/translation
facility (e.g., TranslateService or i18n key lookup) in getErrorMessage() to
return a translated string key like 'errors.minLength' with the requiredLength
as a parameter (reference control and the requiredLength extraction logic
already in getErrorMessage()); ensure getErrorMessage() returns
TranslateService.instant('errors.minLength', {count: requiredLength}) or the
equivalent translation helper used across the component so the minlength message
is localized consistently with other template errors.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
src/app/core/utils/passwords-utility.tssrc/app/login/login-form/login-form.component.htmlsrc/app/login/login-form/login-form.component.tssrc/assets/translations/en-US.jsonsrc/environments/environment.prod.tssrc/environments/environment.ts
IOhacker
left a comment
There was a problem hiding this comment.
Add the missing i18n files, include the REGEX for the parameters as a global variable and also do the proper changes in the env.js and env.template.js files
There was a problem hiding this comment.
Actionable comments posted: 11
♻️ Duplicate comments (1)
src/app/login/login-form/login-form.component.html (1)
57-65:⚠️ Potential issue | 🟠 MajorFix password error localization and key mismatch in the template.
This block still hardcodes the minlength error and appears to use a non-aligned required-message key, so users can get untranslated/misaligned validation text.
💡 Suggested fix
`@if` (loginForm.controls.password.hasError('required')) { <mat-error> - {{ 'labels.inputs.Password' | translate }} - <strong> - {{ 'labels.commons.pass is required' | translate: { min: minPasswordLength } }} - </strong> + <strong>{{ 'errors.Password is required' | translate: { min: minPasswordLength } }}</strong> </mat-error> } `@else` if (loginForm.controls.password.hasError('minlength')) { - <mat-error> Minimum length is {{ loginForm.controls.password.errors?.minlength?.requiredLength }} </mat-error> + <mat-error> + {{ + 'labels.commons.Password minimum length must be' + | translate: { min: loginForm.controls.password.errors?.['minlength']?.requiredLength } + }} + </mat-error> }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/login/login-form/login-form.component.html` around lines 57 - 65, The template is showing a hardcoded minlength message and a mismatched translation key for the required message; update the password validation block that checks loginForm.controls.password so both branches use the translate pipe with correct keys and parameters: replace the incorrect 'labels.commons.pass is required' key with the correct required-key used elsewhere (e.g., 'labels.errors.passwordRequired' or your app's canonical password-required key) and replace the hardcoded "Minimum length is ..." text with a translate key (e.g., 'labels.errors.passwordMinLength') passing { min: loginForm.controls.password.errors?.minlength?.requiredLength } so the minlength message is localized and the keys align with existing translation entries.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/assets/translations/cs-CS.json`:
- Line 19: The Czech translation for the key "Password is required" contains
English text ("min length {{min}}"); update the value for "Password is required"
in src/assets/translations/cs-CS.json to a fully Czech message such as "Je
vyžadováno heslo (minimální délka {{min}})" or "Je vyžadováno heslo (minimální
délka {{min}} znaků)" so the placeholder {{min}} remains but the surrounding
text is translated.
In `@src/assets/translations/de-DE.json`:
- Line 19: The translation for the key "Password is required" currently contains
English text ("min length {{min}}"); update the de-DE value to a fully German
phrase and keep the placeholder {{min}} intact (e.g., replace with a German
fragment such as "Passwort wird benötigt (mindestens {{min}} Zeichen)" or
similar) so the entire string is localized while preserving the {{min}} token.
In `@src/assets/translations/es-CL.json`:
- Line 19: Update the Spanish (Chile) translation value for the "Password is
required" key so it is fully localized and uses sentence casing; replace the
mixed English phrase "min length {{min}}" with a Spanish equivalent such as "se
requiere contraseña (longitud mínima {{min}})" or "se requiere contraseña
(mínimo {{min}} caracteres)" and keep the {{min}} placeholder intact; edit the
value for the "Password is required" entry in src/assets/translations/es-CL.json
accordingly.
In `@src/assets/translations/es-MX.json`:
- Line 19: The translation for the key "Password is required" in es-MX currently
contains the English phrase "min length" — update the value so the entire hint
is Spanish (e.g., replace "se requiere contraseña (min length {{min}})" with a
fully Spanish string such as "se requiere contraseña (longitud mínima
{{min}})"), ensuring you keep the placeholder {{min}} intact and preserve
punctuation.
In `@src/assets/translations/fr-FR.json`:
- Line 19: The French translation for the key "Password is required" mixes
English ("min length {{min}}"); update the value in the fr-FR translations so
the entire message is in French and preserves the interpolation token {{min}}
(e.g., use a fully translated phrase like "Mot de passe requis (longueur
minimale {{min}})" or another natural French wording) so the key "Password is
required" in src/assets/translations/fr-FR.json is consistent and still accepts
the {{min}} parameter.
In `@src/assets/translations/it-IT.json`:
- Line 19: Replace the incorrect Italian value for the JSON key "Password is
required": fix grammar and fully localize the message, e.g. use "La password è
richiesta (lunghezza minima {{min}})" or similar correct Italian with the
{{min}} placeholder preserved; update the value associated with "Password is
required" so it contains only proper Italian text and correct
punctuation/diacritics.
In `@src/assets/translations/lt-LT.json`:
- Line 19: The translation for the key "Password is required" currently combines
required and minlength text; change the value for the "Password is required" key
to a required-only message (e.g., "reikalingas slaptažodis") and add a new
separate minlength key (e.g., "Password must be at least {{min}} characters" or
its Lithuanian equivalent) so required and minlength validations are handled by
distinct keys; update the JSON by keeping the existing key name "Password is
required" and adding a new key such as "Password minlength" (or your project's
minlength key convention) with the {{min}} interpolation.
In `@src/assets/translations/lv-LV.json`:
- Line 19: The translation for the key "Password is required" in the lv-LV
locale mixes English ("min length {{min}}") with Latvian; replace the English
fragment with a fully Latvian phrase (e.g., "paroles garums vismaz {{min}}
rakstzīmes" or similar) so the entire value is proper Latvian and retains the
{{min}} interpolation token unchanged; update the value for "Password is
required" to a fully localized string.
In `@src/assets/translations/ne-NE.json`:
- Line 19: The Nepali translation for the "Password is required" key currently
contains English text ("min length {{min}}"); update the value for the "Password
is required" key in src/assets/translations/ne-NE.json to use a fully Nepali
suffix (e.g., replace "min length {{min}}" with the appropriate Nepali phrase
including the {{min}} placeholder) so the entire message is localized and the
placeholder remains unchanged.
In `@src/assets/translations/pt-PT.json`:
- Line 19: The pt-PT translation for the key "Password is required" mixes
Brazilian/English wording; update the value to a consistent European Portuguese
validation message (translate "min length {{min}}" and use pt-PT phrasing).
Locate the string "Password is required" in src/assets/translations/pt-PT.json
and replace the value with a pt-PT message (e.g., "Senha obrigatória
(comprimento mínimo {{min}})" or another equivalent pt-PT phrasing) so the
entire message is in Portuguese (pt-PT).
In `@src/assets/translations/sw-SW.json`:
- Line 19: The Swahili translation for the key "Password is required" contains
English text ("min length"); update the value to a fully localized Swahili
string while preserving the interpolation token {{min}} — for example replace
"Nenosiri linahitajika (min length {{min}})" with "Nenosiri linahitajika (urefu
wa chini {{min}})" or similar natural Swahili, ensuring the key "Password is
required" and the {{min}} placeholder remain unchanged.
---
Duplicate comments:
In `@src/app/login/login-form/login-form.component.html`:
- Around line 57-65: The template is showing a hardcoded minlength message and a
mismatched translation key for the required message; update the password
validation block that checks loginForm.controls.password so both branches use
the translate pipe with correct keys and parameters: replace the incorrect
'labels.commons.pass is required' key with the correct required-key used
elsewhere (e.g., 'labels.errors.passwordRequired' or your app's canonical
password-required key) and replace the hardcoded "Minimum length is ..." text
with a translate key (e.g., 'labels.errors.passwordMinLength') passing { min:
loginForm.controls.password.errors?.minlength?.requiredLength } so the minlength
message is localized and the keys align with existing translation entries.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (15)
src/app/login/login-form/login-form.component.htmlsrc/app/login/login-form/login-form.component.tssrc/assets/translations/cs-CS.jsonsrc/assets/translations/de-DE.jsonsrc/assets/translations/en-US.jsonsrc/assets/translations/es-CL.jsonsrc/assets/translations/es-MX.jsonsrc/assets/translations/fr-FR.jsonsrc/assets/translations/it-IT.jsonsrc/assets/translations/ko-KO.jsonsrc/assets/translations/lt-LT.jsonsrc/assets/translations/lv-LV.jsonsrc/assets/translations/ne-NE.jsonsrc/assets/translations/pt-PT.jsonsrc/assets/translations/sw-SW.json
✅ Files skipped from review due to trivial changes (1)
- src/assets/translations/ko-KO.json
🚧 Files skipped from review as they are similar to previous changes (1)
- src/assets/translations/en-US.json
| "Email not valid": "E-Mail ungültig", | ||
| "Filling Details": "Klicken Sie hier, um mit dem Ausfüllen der Details zu beginnen. * Markierungsfelder sind erforderlich. Für weitere Details klicken Sie:", | ||
| "Password is required": "Passwort wird benötigt", | ||
| "Password is required": "Passwort wird benötigt (min length {{min}})", |
There was a problem hiding this comment.
Localize the new min-length fragment in German.
The updated value mixes German and English (min length). Please keep this string fully localized for de-DE.
✍️ Suggested wording
- "Password is required": "Passwort wird benötigt (min length {{min}})",
+ "Password is required": "Passwort wird benötigt (Mindestlänge {{min}} Zeichen)",📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "Password is required": "Passwort wird benötigt (min length {{min}})", | |
| "Password is required": "Passwort wird benötigt (Mindestlänge {{min}} Zeichen)", |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/assets/translations/de-DE.json` at line 19, The translation for the key
"Password is required" currently contains English text ("min length {{min}}");
update the de-DE value to a fully German phrase and keep the placeholder {{min}}
intact (e.g., replace with a German fragment such as "Passwort wird benötigt
(mindestens {{min}} Zeichen)" or similar) so the entire string is localized
while preserving the {{min}} token.
| "Email not valid": "Correo electrónico no válido", | ||
| "Filling Details": "Haga clic para comenzar a completar los detalles. * Los campos de marca son necesarios. Para más detalles haga clic en:", | ||
| "Password is required": "se requiere contraseña", | ||
| "Password is required": "se requiere contraseña (min length {{min}})", |
There was a problem hiding this comment.
Localize the min-length phrase for es-CL.
Line 19 mixes Spanish and English (min length {{min}}), which makes the UI copy inconsistent for this locale. Use fully localized wording and sentence casing.
✍️ Proposed fix
- "Password is required": "se requiere contraseña (min length {{min}})",
+ "Password is required": "Se requiere contraseña (longitud mínima {{min}})",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/assets/translations/es-CL.json` at line 19, Update the Spanish (Chile)
translation value for the "Password is required" key so it is fully localized
and uses sentence casing; replace the mixed English phrase "min length {{min}}"
with a Spanish equivalent such as "se requiere contraseña (longitud mínima
{{min}})" or "se requiere contraseña (mínimo {{min}} caracteres)" and keep the
{{min}} placeholder intact; edit the value for the "Password is required" entry
in src/assets/translations/es-CL.json accordingly.
| "Email not valid": "Correo electrónico no válido", | ||
| "Filling Details": "Haga clic para comenzar a completar los detalles. * Los campos de marca son necesarios. Para más detalles haga clic en:", | ||
| "Password is required": "se requiere contraseña", | ||
| "Password is required": "se requiere contraseña (min length {{min}})", |
There was a problem hiding this comment.
Translate the minimum-length hint fully to Spanish in es-MX.
Line 19 keeps min length in English, which creates mixed-language UX in a Spanish locale.
💡 Suggested fix
- "Password is required": "se requiere contraseña (min length {{min}})",
+ "Password is required": "Se requiere contraseña (mínimo {{min}} caracteres)",📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "Password is required": "se requiere contraseña (min length {{min}})", | |
| "Password is required": "Se requiere contraseña (mínimo {{min}} caracteres)", |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/assets/translations/es-MX.json` at line 19, The translation for the key
"Password is required" in es-MX currently contains the English phrase "min
length" — update the value so the entire hint is Spanish (e.g., replace "se
requiere contraseña (min length {{min}})" with a fully Spanish string such as
"se requiere contraseña (longitud mínima {{min}})"), ensuring you keep the
placeholder {{min}} intact and preserve punctuation.
| "Email not valid": "E-pasts nav derīgs", | ||
| "Filling Details": "Noklikšķiniet, lai sāktu aizpildīt informāciju. * ir nepieciešami atzīmju lauki. Lai iegūtu sīkāku informāciju, noklikšķiniet:", | ||
| "Password is required": "nepieciešama parole", | ||
| "Password is required": "nepieciešama parole (min length {{min}})", |
There was a problem hiding this comment.
Localize the new min-length phrase for lv-LV.
Line 19 mixes Latvian with English (min length), so users get a partially untranslated validation message.
✏️ Proposed fix
- "Password is required": "nepieciešama parole (min length {{min}})",
+ "Password is required": "Nepieciešama parole (minimālais garums: {{min}})",📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "Password is required": "nepieciešama parole (min length {{min}})", | |
| "Password is required": "Nepieciešama parole (minimálais garums: {{min}})", |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/assets/translations/lv-LV.json` at line 19, The translation for the key
"Password is required" in the lv-LV locale mixes English ("min length {{min}}")
with Latvian; replace the English fragment with a fully Latvian phrase (e.g.,
"paroles garums vismaz {{min}} rakstzīmes" or similar) so the entire value is
proper Latvian and retains the {{min}} interpolation token unchanged; update the
value for "Password is required" to a fully localized string.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/assets/env.js (1)
115-118: Derive regex minimum fromminPasswordLengthto avoid future drift.Today both are 8, but this becomes easy to desync when updating config later.
Suggested refactor
// Password Configuration window['env']['minPasswordLength'] = 8; window['env']['passwordRegex'] = - '^(?!.*(.)\\1)(?!.*\\s)(?=.*\\d)(?=.*[a-z])(?=.*[A-Z])(?=.*[^\\w\\s]).{8,50}$'; + `^(?!.*(.)\\1)(?!.*\\s)(?=.*\\d)(?=.*[a-z])(?=.*[A-Z])(?=.*[^\\w\\s]).{${window['env']['minPasswordLength']},50}$`;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/assets/env.js` around lines 115 - 118, The password regex is hardcoded to {8,50} and can drift from window['env']['minPasswordLength']; update the code that sets window['env']['passwordRegex'] to build the pattern dynamically using the min value (window['env']['minPasswordLength']) so the quantifier becomes `{<minPasswordLength>,50}` (or use a configurable max) and assign that string/RegExp to window['env']['passwordRegex'] instead of the fixed literal; locate the assignment of window['env']['passwordRegex'] and replace the static pattern with a constructed pattern that inserts the min length.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/assets/env.template.js`:
- Around line 94-96: The password regex is hardcoded to {8,50} while
minPasswordLength is set from MIFOS_MIN_PASSWORD_LENGTH; update the assignment
for window['env']['passwordRegex'] so it dynamically uses the configured min
length (window['env']['minPasswordLength'] or the MIFOS_MIN_PASSWORD_LENGTH
value) instead of the literal 8, e.g., build the regex string with the min
length interpolated and preserve the existing rules and max (50) so the
validator and message remain consistent with window['env']['minPasswordLength'].
---
Nitpick comments:
In `@src/assets/env.js`:
- Around line 115-118: The password regex is hardcoded to {8,50} and can drift
from window['env']['minPasswordLength']; update the code that sets
window['env']['passwordRegex'] to build the pattern dynamically using the min
value (window['env']['minPasswordLength']) so the quantifier becomes
`{<minPasswordLength>,50}` (or use a configurable max) and assign that
string/RegExp to window['env']['passwordRegex'] instead of the fixed literal;
locate the assignment of window['env']['passwordRegex'] and replace the static
pattern with a constructed pattern that inserts the min length.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
src/app/core/utils/passwords-utility.tssrc/assets/env.jssrc/assets/env.template.jssrc/environments/environment.prod.tssrc/environments/environment.ts
1bc8a16 to
0e76ce1
Compare
|
Hi @IOhacker, The requested changes have been addressed:
Please let me know if any further refinements are needed. |
0e76ce1 to
3b6008e
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (4)
src/assets/env.template.js (1)
94-95:⚠️ Potential issue | 🟠 Major
passwordRegexis still not wired in env.template.Line 95 reassigns
minPasswordLengthagain and never setswindow['env']['passwordRegex'], so runtime regex override from env is not available.Suggested fix
- // Min Password length - window['env']['minPasswordLength'] = '$MIFOS_MIN_PASSWORD_LENGTH'; - - // Password Regex - window['env']['minPasswordLength'] = '$MIFOS_MIN_PASSWORD_LENGTH'; + // Password configuration + const minPasswordLength = Number('$MIFOS_MIN_PASSWORD_LENGTH') || 8; + window['env']['minPasswordLength'] = minPasswordLength; + window['env']['passwordRegex'] = + '$MIFOS_PASSWORD_REGEX' || + `^(?!.*(.)\\1)(?!.*\\s)(?=.*\\d)(?=.*[a-z])(?=.*[A-Z])(?=.*[^\\w\\s]).{${minPasswordLength},50}$`;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/assets/env.template.js` around lines 94 - 95, The template mistakenly reassigns window['env']['minPasswordLength'] a second time and never sets window['env']['passwordRegex', so update the block where minPasswordLength is assigned (the code that currently sets window['env']['minPasswordLength'] = '$MIFOS_MIN_PASSWORD_LENGTH') to instead set window['env']['passwordRegex'] = '$MIFOS_PASSWORD_REGEX' (and ensure minPasswordLength remains assigned only once); locate the assignment(s) to window['env']['minPasswordLength'] and replace the duplicate with a proper window['env']['passwordRegex'] assignment so the runtime regex override is available.src/assets/translations/cs-CS.json (1)
19-19:⚠️ Potential issue | 🟡 MinorLocalize the inserted min-length text in Czech.
Line 19 still mixes Czech and English (
min length {{min}}), so Czech users get inconsistent messaging.Suggested fix
- "Password is required": "Je vyžadováno heslo (min length {{min}})", + "Password is required": "Heslo je povinné (minimální délka {{min}} znaků)",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/assets/translations/cs-CS.json` at line 19, The translation for the key "Password is required" mixes English; replace the English fragment `min length {{min}}` with a Czech phrase such as `minimální délka {{min}}` (e.g., change the value to "Je vyžadováno heslo (minimální délka {{min}})" or "Je vyžadováno heslo (minimální délka {{min}} znaků)") so the entire message is fully localized.src/assets/translations/fr-FR.json (1)
19-19:⚠️ Potential issue | 🟡 MinorLocalize the min-length phrase in French.
Line 19 still mixes English in
fr-FR(min length {{min}}). Please keep it fully French while preserving interpolation, e.g.longueur minimale : {{min}}.💡 Suggested fix
- "Password is required": "Mot de passe requis (min length {{min}})", + "Password is required": "Mot de passe requis (longueur minimale : {{min}})",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/assets/translations/fr-FR.json` at line 19, The French translation for the key "Password is required" contains English text "min length {{min}}"; update the value to fully French while preserving the interpolation token {{min}} (for example use a French phrase such as "longueur minimale : {{min}}" or "longueur minimale : {{min}}") so the translated string remains correct and the interpolation still works.src/assets/translations/sw-SW.json (1)
19-19:⚠️ Potential issue | 🟡 MinorLocalize the min-length phrase fully in Swahili.
Line 19 still mixes English (
min length) in thesw-SWlocale string. Keep{{min}}, but localize the phrase.💡 Suggested fix
- "Password is required": "Nenosiri linahitajika (min length {{min}})", + "Password is required": "Nenosiri linahitajika (urefu wa chini {{min}})",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/assets/translations/sw-SW.json` at line 19, Update the Swahili translation for the key "Password is required" so it does not mix English; replace the English phrase "min length {{min}}" with a fully localized Swahili phrase while keeping the {{min}} placeholder (for example: "Nenosiri linahitajika (urefu wa chini {{min}})"), and save the change in the sw-SW JSON entry for "Password is required".
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/environments/environment.ts`:
- Around line 111-114: Normalize and validate loadedEnv.minPasswordLength before
embedding it into passwordRegex: parse loadedEnv.minPasswordLength to an integer
(e.g., using Number/parseInt), check it is a finite positive integer (fallback
to 8 if invalid), and use that validated numeric value when constructing the
passwordRegex string (reference symbols: loadedEnv.minPasswordLength,
minPasswordLength, passwordRegex) so the generated regex is always syntactically
valid even when the env var is a non-numeric string.
---
Duplicate comments:
In `@src/assets/env.template.js`:
- Around line 94-95: The template mistakenly reassigns
window['env']['minPasswordLength'] a second time and never sets
window['env']['passwordRegex', so update the block where minPasswordLength is
assigned (the code that currently sets window['env']['minPasswordLength'] =
'$MIFOS_MIN_PASSWORD_LENGTH') to instead set window['env']['passwordRegex'] =
'$MIFOS_PASSWORD_REGEX' (and ensure minPasswordLength remains assigned only
once); locate the assignment(s) to window['env']['minPasswordLength'] and
replace the duplicate with a proper window['env']['passwordRegex'] assignment so
the runtime regex override is available.
In `@src/assets/translations/cs-CS.json`:
- Line 19: The translation for the key "Password is required" mixes English;
replace the English fragment `min length {{min}}` with a Czech phrase such as
`minimální délka {{min}}` (e.g., change the value to "Je vyžadováno heslo
(minimální délka {{min}})" or "Je vyžadováno heslo (minimální délka {{min}}
znaků)") so the entire message is fully localized.
In `@src/assets/translations/fr-FR.json`:
- Line 19: The French translation for the key "Password is required" contains
English text "min length {{min}}"; update the value to fully French while
preserving the interpolation token {{min}} (for example use a French phrase such
as "longueur minimale : {{min}}" or "longueur minimale : {{min}}") so the
translated string remains correct and the interpolation still works.
In `@src/assets/translations/sw-SW.json`:
- Line 19: Update the Swahili translation for the key "Password is required" so
it does not mix English; replace the English phrase "min length {{min}}" with a
fully localized Swahili phrase while keeping the {{min}} placeholder (for
example: "Nenosiri linahitajika (urefu wa chini {{min}})"), and save the change
in the sw-SW JSON entry for "Password is required".
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (20)
src/app/core/utils/passwords-utility.tssrc/app/login/login-form/login-form.component.htmlsrc/app/login/login-form/login-form.component.tssrc/assets/env.jssrc/assets/env.template.jssrc/assets/translations/cs-CS.jsonsrc/assets/translations/de-DE.jsonsrc/assets/translations/en-US.jsonsrc/assets/translations/es-CL.jsonsrc/assets/translations/es-MX.jsonsrc/assets/translations/fr-FR.jsonsrc/assets/translations/it-IT.jsonsrc/assets/translations/ko-KO.jsonsrc/assets/translations/lt-LT.jsonsrc/assets/translations/lv-LV.jsonsrc/assets/translations/ne-NE.jsonsrc/assets/translations/pt-PT.jsonsrc/assets/translations/sw-SW.jsonsrc/environments/environment.prod.tssrc/environments/environment.ts
🚧 Files skipped from review as they are similar to previous changes (13)
- src/app/core/utils/passwords-utility.ts
- src/assets/translations/ko-KO.json
- src/assets/translations/es-CL.json
- src/assets/translations/de-DE.json
- src/assets/translations/lt-LT.json
- src/assets/translations/it-IT.json
- src/assets/translations/pt-PT.json
- src/assets/translations/lv-LV.json
- src/assets/translations/en-US.json
- src/assets/translations/es-MX.json
- src/app/login/login-form/login-form.component.html
- src/environments/environment.prod.ts
- src/assets/translations/ne-NE.json
|
@YousufFFFF I think that the REGEX fallback value can be set to match the current policy for backward compatibility. The new env var variables can be set for making the proper changes for new setups and also make sure about the impact in the User management section (new user, edit user, edit/change password). This should only have a change for the Basic authorization and have a special review in the OAUTH and OIDC because we have to match the policies of those systems. |
|
Thanks for the clarification. I will ensure the regex fallback matches the current policy for backward compatibility and restrict the changes to Basic authentication only. I’ll also review the impact on user management and confirm no unintended changes affect OAuth/OIDC flows. |
3b6008e to
86888e1
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (3)
src/assets/env.template.js (1)
94-96:⚠️ Potential issue | 🟠 MajorAdd
passwordRegexhere instead of reassigningminPasswordLength.Line 95 duplicates
minPasswordLengthand never setswindow['env']['passwordRegex'], so this template cannot expose regex configuration as intended.Suggested fix
- // Password Regex - window['env']['minPasswordLength'] = '$MIFOS_MIN_PASSWORD_LENGTH'; + // Password Configuration + const minPasswordLength = Number('$MIFOS_MIN_PASSWORD_LENGTH') || 8; + window['env']['minPasswordLength'] = minPasswordLength; + window['env']['passwordRegex'] = + `^(?!.*(.)\\1)(?!.*\\s)(?=.*\\d)(?=.*[a-z])(?=.*[A-Z])(?=.*[^\\w\\s]).{${minPasswordLength},50}$`;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/assets/env.template.js` around lines 94 - 96, The template currently reassigns window['env']['minPasswordLength'] twice and never sets the password regex; change the second assignment so that it sets window['env']['passwordRegex'] (e.g., use the MIFOS_PASSWORD_REGEX template variable) instead of overwriting minPasswordLength; update the assignment at the location where window['env']['minPasswordLength'] is duplicated to use window['env']['passwordRegex'] so the template exposes the regex configuration via passwordRegex.src/assets/translations/pt-PT.json (1)
19-19:⚠️ Potential issue | 🟡 MinorUse consistent pt-PT wording for this validation string.
Line 19 mixes Brazilian/English wording in a pt-PT file.
Suggested fix
- "Password is required": "Senha requerida (min length {{min}})", + "Password is required": "Palavra-passe obrigatória (comprimento mínimo {{min}})",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/assets/translations/pt-PT.json` at line 19, The translation value for the key "Password is required" mixes Brazilian/English; update the value to consistent pt-PT phrasing (e.g., use "Palavra-passe obrigatória (mín. {{min}})" or "Palavra-passe obrigatória (comprimento mínimo {{min}})") by replacing the current string for the "Password is required" key so it uses pt-PT vocabulary ("Palavra-passe", "obrigatória", "mín." or "comprimento mínimo") and keeps the {{min}} interpolation.src/environments/environment.ts (1)
20-20:⚠️ Potential issue | 🟠 MajorApply the same
minPasswordLengthinteger sanitization here as well.This file mirrors the same parsing/regex construction risk and should use the same guard.
Also applies to: 113-116
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/environments/environment.ts` at line 20, The code uses resolvedMinPasswordLength = Number(loadedEnv.minPasswordLength) || 8 which can mis-handle non-numeric or zero values; update this to explicitly parse and validate the integer (e.g., parseInt/Number, verify Number.isFinite and Number.isInteger and enforce a sensible minimum like 1) and fall back to 8 if validation fails, then use that sanitized value wherever minPasswordLength is consumed (including other occurrences in this file that build regexes or rely on it around the later minPasswordLength usages) to prevent injection/regex-construction issues.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/environments/environment.prod.ts`:
- Line 17: The value for resolvedMinPasswordLength must be converted to a
positive integer and clamped before it’s used to build a regex; replace the
current Number(loadedEnv.minPasswordLength) || 8 logic with parsing to an
integer (e.g. parse or Number), applying Math.floor (or equivalent), defaulting
to 8 when NaN, and then clamping with Math.max(1, ...) so negative or fractional
inputs become a safe positive integer; update the resolvedMinPasswordLength
assignment and the other occurrences referenced around lines 99-102 to use the
same sanitization logic for loadedEnv.minPasswordLength.
---
Duplicate comments:
In `@src/assets/env.template.js`:
- Around line 94-96: The template currently reassigns
window['env']['minPasswordLength'] twice and never sets the password regex;
change the second assignment so that it sets window['env']['passwordRegex']
(e.g., use the MIFOS_PASSWORD_REGEX template variable) instead of overwriting
minPasswordLength; update the assignment at the location where
window['env']['minPasswordLength'] is duplicated to use
window['env']['passwordRegex'] so the template exposes the regex configuration
via passwordRegex.
In `@src/assets/translations/pt-PT.json`:
- Line 19: The translation value for the key "Password is required" mixes
Brazilian/English; update the value to consistent pt-PT phrasing (e.g., use
"Palavra-passe obrigatória (mín. {{min}})" or "Palavra-passe obrigatória
(comprimento mínimo {{min}})") by replacing the current string for the "Password
is required" key so it uses pt-PT vocabulary ("Palavra-passe", "obrigatória",
"mín." or "comprimento mínimo") and keeps the {{min}} interpolation.
In `@src/environments/environment.ts`:
- Line 20: The code uses resolvedMinPasswordLength =
Number(loadedEnv.minPasswordLength) || 8 which can mis-handle non-numeric or
zero values; update this to explicitly parse and validate the integer (e.g.,
parseInt/Number, verify Number.isFinite and Number.isInteger and enforce a
sensible minimum like 1) and fall back to 8 if validation fails, then use that
sanitized value wherever minPasswordLength is consumed (including other
occurrences in this file that build regexes or rely on it around the later
minPasswordLength usages) to prevent injection/regex-construction issues.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (20)
src/app/core/utils/passwords-utility.tssrc/app/login/login-form/login-form.component.htmlsrc/app/login/login-form/login-form.component.tssrc/assets/env.jssrc/assets/env.template.jssrc/assets/translations/cs-CS.jsonsrc/assets/translations/de-DE.jsonsrc/assets/translations/en-US.jsonsrc/assets/translations/es-CL.jsonsrc/assets/translations/es-MX.jsonsrc/assets/translations/fr-FR.jsonsrc/assets/translations/it-IT.jsonsrc/assets/translations/ko-KO.jsonsrc/assets/translations/lt-LT.jsonsrc/assets/translations/lv-LV.jsonsrc/assets/translations/ne-NE.jsonsrc/assets/translations/pt-PT.jsonsrc/assets/translations/sw-SW.jsonsrc/environments/environment.prod.tssrc/environments/environment.ts
🚧 Files skipped from review as they are similar to previous changes (15)
- src/assets/translations/en-US.json
- src/assets/env.js
- src/assets/translations/it-IT.json
- src/assets/translations/ko-KO.json
- src/assets/translations/fr-FR.json
- src/assets/translations/es-CL.json
- src/assets/translations/lt-LT.json
- src/assets/translations/cs-CS.json
- src/assets/translations/lv-LV.json
- src/assets/translations/de-DE.json
- src/assets/translations/ne-NE.json
- src/app/login/login-form/login-form.component.html
- src/assets/translations/sw-SW.json
- src/app/core/utils/passwords-utility.ts
- src/assets/translations/es-MX.json
|
Hi @IOhacker , Please let me know if you’d like any further adjustments. |
IOhacker
left a comment
There was a problem hiding this comment.
Kindly see my comments and do a squash and commit (1 commit for PR) please.
d6f43b8 to
3f78091
Compare
… length and regex configuration WEB-628: Sanitize minPasswordLength to positive integer
3f78091 to
908d3dd
Compare
|
Hi @IOhacker,
Please let me know if anything else needs adjustment. |
|
@YousufFFFF you got the point! Thank you for your efforts, you have keep backward compatibility and implement a new feature, I will open a new PR for adding the new variables in the README.md |
|
Thank you @IOhacker for the guidance and review throughout this PR, I really appreciate it. I’ve been primarily working on frontend contributions (including Apache Superset’s frontend codebase), and through WEB-628 I’ve become quite familiar with the Mifos web app structure as well. I’m looking forward to contributing more on the frontend side going forward. |
Description
This PR standardizes password minimum length validation in the login module to ensure consistency between UI validation, environment configuration, and displayed error messages.
Previously, the minimum password length was inconsistently handled across validation logic and error messaging. This caused mismatches between the configured minimum length and the validation feedback shown to users.
This change ensures:
No additional dependencies are required.
Related issues and discussion
Closes #WEB-628
Screenshots / Demo
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
Bug Fixes
Localization