feat(auth): update password reset page UI and policy - #149
Conversation
- Replace email field outline with filled/disabled style - Add password visibility toggle with eye icon - Add confirm field match indicator (empty circle / green check / red X) - Add password requirements list derived from config - Add help link below the card - Disable submit button until all requirements pass and fields match - Update card title and subheader to FN branding - Fix min password length to 10 (was 8) and add + to allowed special chars pattern - Add AUTH_PASSWORD_MIN_LENGTH, AUTH_PASSWORD_SHAPE_LIST, and AUTH_PASSWORD_ALLOWED_SPECIAL_CHARACTERS_TEXT config keys - Update local dev docs: queue drain command and config cache notes
📝 WalkthroughWalkthroughThe password reset flow now exposes additional password-policy text, provides visibility and confirmation feedback in the frontend, tightens submission validation, updates styling and copy, and documents local asset, captcha, queue, log, and configuration workflows. ChangesPassword reset experience
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant ResetPage as reset.blade.php
participant ResetUI as reset_password.js
participant Config as config/auth.php
Config->>ResetPage: provide password policy settings
ResetPage->>ResetUI: pass passwordPolicy
User->>ResetUI: enter password and confirmation
ResetUI->>User: show visibility and match indicators
ResetUI->>User: enable submission only when fields are valid
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
📝 Generate docstrings
🧪 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 |
|
📘 OpenAPI / Swagger preview ➡️ https://OpenStackweb.github.io/openstackid/openapi/pr-149/ This page is automatically updated on each push to this PR. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@config/auth.php`:
- Around line 106-109: Update the default fallback for AUTH_PASSWORD_MIN_LENGTH
in the password configuration to 10 instead of 8, preserving the existing
environment-variable override behavior.
In `@resources/js/reset_password/reset_password.js`:
- Line 211: Render passwordPolicy.allowed_special_characters_text as a normal
React child in resources/js/reset_password/reset_password.js:211, removing
dangerouslySetInnerHTML. In resources/views/auth/passwords/reset.blade.php:27,
serialize the configured value with Illuminate\Support\Js::from(...) before
embedding it in the script block.
- Around line 151-157: Update the password visibility IconButton to provide a
dynamic accessible name reflecting show versus hide state. Add
screen-reader-visible status text for the confirmation indicator covering
matching, mismatching, and empty confirmation states, while preserving the
existing visual icons and behavior.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: e0d6a357-a410-4cd2-b08b-d5afc11d090d
📒 Files selected for processing (6)
.env.exampleLOCAL_DEVELOPMENT_HOWTO.mdconfig/auth.phpresources/js/reset_password/reset_password.jsresources/js/reset_password/reset_password.module.scssresources/views/auth/passwords/reset.blade.php
💤 Files with no reviewable changes (1)
- .env.example
- Add aria-label to password visibility toggle reflecting show/hide state - Add sr-only aria-live region for confirm field match status - Replace dangerouslySetInnerHTML with plain React child for special characters text; use Js::from() in blade template to safely serialize the value without HTML entity encoding - Update password_min_length default from 8 to 10 to match policy
|
📘 OpenAPI / Swagger preview ➡️ https://OpenStackweb.github.io/openstackid/openapi/pr-149/ This page is automatically updated on each push to this PR. |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@resources/js/reset_password/reset_password.module.scss`:
- Line 90: In the .sr_only style rule, replace the deprecated clip declaration
with clip-path: inset(50%) while preserving the existing visually hidden
behavior and other declarations.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 6e14e557-4608-46e6-b98d-6ce84565ae49
📒 Files selected for processing (4)
config/auth.phpresources/js/reset_password/reset_password.jsresources/js/reset_password/reset_password.module.scssresources/views/auth/passwords/reset.blade.php
🚧 Files skipped from review as they are similar to previous changes (3)
- config/auth.php
- resources/views/auth/passwords/reset.blade.php
- resources/js/reset_password/reset_password.js
Replace test passwords (8–9 chars) with a new one (10 chars) across TestSeeder and four test files so the suite passes against the updated auth.password_min_length default.
|
📘 OpenAPI / Swagger preview ➡️ https://OpenStackweb.github.io/openstackid/openapi/pr-149/ This page is automatically updated on each push to this PR. |
|
📘 OpenAPI / Swagger preview ➡️ https://OpenStackweb.github.io/openstackid/openapi/pr-149/ This page is automatically updated on each push to this PR. |
|
|
||
| AUTH_PASSWORD_RESET_LIFETIME=1800 | ||
|
|
||
| AUTH_PASSWORD_SHAPE_PATTERN="^(?=.*?[A-Z])(?=.*?[a-z])(?=.*?[0-9])(?=.*?[#?!@$%^&*-])" |
There was a problem hiding this comment.
@mulldug .env.example still ships the pre-PR password pattern, overriding the new default this PR introduces.
config/auth.php:107 now defaults to ^(?=.*?[A-Z])(?=.*?[a-z])(?=.*?[0-9])(?=.*?[#?!@$%^&*+\-])[A-Za-z0-9#?!@$%^&*+\-]+$ (adds +, enforces the allowed charset), but this line still sets the old lookahead-only pattern without +. Any new environment copying the template gets a concrete mismatch: a password whose only special character is + (e.g. Abcdefgh1+) is rejected server-side, while the UI's "Allowed: #?!@$%^&*+-" hint (which falls back to the new config default, since the template doesn't set AUTH_PASSWORD_ALLOWED_SPECIAL_CHARACTERS_TEXT) tells the user + is fine. The old pattern also enforces no allowed-charset body at all. Deployed envs are unaffected (openstackid-docker/app/.env.production and argocd-apps/openstackid/values-prod.yaml don't override AUTH_PASSWORD_SHAPE_PATTERN), so this only bites local dev and new environments — but it directly contradicts the PR's stated goal of adding + to the pattern.
Suggested fix — update the template to match the new defaults (or drop both lines so the config defaults apply), and document the new floor while here:
AUTH_PASSWORD_MIN_LENGTH=10
AUTH_PASSWORD_SHAPE_PATTERN="^(?=.*?[A-Z])(?=.*?[a-z])(?=.*?[0-9])(?=.*?[#?!@$%^&*+\-])[A-Za-z0-9#?!@$%^&*+\-]+$"
AUTH_PASSWORD_SHAPE_WARNING="Password must include at least one uppercase letter, one lowercase letter, one number, and one special character (#?!@$%^&*+-)."
| shape_warning: '{{ Config::get("auth.password_shape_warning") }}' | ||
| allowed_special_characters_text: {{ Illuminate\Support\Js::from(Config::get("auth.password_allowed_special_characters_text")) }}, | ||
| shape_warning: '{{ Config::get("auth.password_shape_warning") }}', | ||
| shape_list: '{{ Config::get("auth.password_shape_list") }}' |
There was a problem hiding this comment.
@mulldug shape_list is interpolated raw into a single-quoted JS string, unlike allowed_special_characters_text two lines above, which was hardened with Js::from(...) during this PR's review cycle.
Not exploitable as XSS (Blade's {{ }} escapes with ENT_QUOTES, and <script> bodies aren't entity-decoded), but it is a correctness trap for a deployment-configurable value: a quote or & in AUTH_PASSWORD_SHAPE_LIST renders as garbled literal text (') in the requirements list, and a trailing backslash breaks the JS parse entirely — which kills the whole page, since passwordRequirementItems in reset_password.js derives from this value at render time.
Suggested fix — same pattern as the sibling line:
shape_list: {{ Illuminate\Support\Js::from(Config::get("auth.password_shape_list")) }}
| fullWidth | ||
| size="small" | ||
| label="Email Address" | ||
| hiddenLabel |
There was a problem hiding this comment.
@mulldug The email field lost its accessible name: this change replaces label="Email Address" with hiddenLabel without adding an aria-label, so screen readers announce the FNid value with no field name.
This is inconsistent with the accessibility work already in this PR — the visibility toggle has a state-aware aria-label and the confirm indicator has an aria-live announcer — so the page is one attribute away from being fully covered. hiddenLabel only suppresses the visual label; it doesn't provide a replacement accessible name.
Suggested fix:
inputProps={{ 'aria-label': 'Email address' }}on the email <TextField>.
There was a problem hiding this comment.
Pull request overview
Updates the password reset experience and password policy defaults to match the new UX requirements (visibility toggle, live match indicator, requirements list) while aligning backend-configured password constraints (notably min length 10) with tests and seed data.
Changes:
- Revamps the reset password UI (email field styling, password visibility toggle, match indicators, requirements list, help link, submit disabling).
- Updates password policy configuration defaults and introduces additional config/env keys for UI display.
- Aligns test fixtures and seed user passwords with the new minimum-length policy; expands local development documentation.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/UserApiTest.php | Updates test password to meet new minimum length. |
| tests/PasswordChangeRevokeTokenTest.php | Updates current password values to meet new minimum length. |
| tests/OIDCProtocolTestCase.php | Updates login fixture password strings to match seeded/test policy. |
| resources/views/auth/passwords/reset.blade.php | Adds new password policy fields into the reset-password page config payload. |
| resources/js/reset_password/reset_password.module.scss | Adds styles for the new reset-password UI elements. |
| resources/js/reset_password/reset_password.js | Implements new reset-password UI behaviors and submission gating. |
| LOCAL_DEVELOPMENT_HOWTO.md | Adds frontend build/watch notes, captcha test keys guidance, queue draining, and config cache notes. |
| database/seeds/TestSeeder.php | Updates seeded test users’ passwords to satisfy new password policy. |
| config/auth.php | Raises default min length to 10 and adds new config/env keys for password policy display. |
| .env.example | Minor change in the auth password section (needs expansion to include new keys). |
Suppressed comments (2)
resources/js/reset_password/reset_password.js:21
- Typography is imported but no longer used in this component, which can cause lint/build warnings and adds unnecessary code to the bundle.
import Typography from "@material-ui/core/Typography";
resources/js/reset_password/reset_password.js:261
- There are two elements with id="email" (the disabled TextField and this hidden input). Duplicate IDs are invalid HTML and can break label associations and automated tests. Keep the hidden field for submission, but remove/rename its id.
<input type="hidden" value={initialValues.email} id="email" name="email"/>
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| import Grid from "@material-ui/core/Grid"; | ||
| import IconButton from "@material-ui/core/IconButton"; | ||
| import InputAdornment from "@material-ui/core/InputAdornment"; | ||
| import InfoOutlinedIcon from "@material-ui/icons/InfoOutlined"; |
| fullWidth | ||
| size="small" | ||
| label="Email Address" | ||
| hiddenLabel |
| AUTH_PASSWORD_RESET_LIFETIME=1800 | ||
|
|
||
| AUTH_PASSWORD_SHAPE_PATTERN="^(?=.*?[A-Z])(?=.*?[a-z])(?=.*?[0-9])(?=.*?[#?!@$%^&*-])" | ||
| AUTH_PASSWORD_SHAPE_WARNING="Password must include at least one uppercase letter, one lowercase letter, one number, and one special character." | ||
|
|
| shape_warning: '{{ Config::get("auth.password_shape_warning") }}' | ||
| allowed_special_characters_text: {{ Illuminate\Support\Js::from(Config::get("auth.password_allowed_special_characters_text")) }}, | ||
| shape_warning: '{{ Config::get("auth.password_shape_warning") }}', | ||
| shape_list: '{{ Config::get("auth.password_shape_list") }}' |
ref: https://app.clickup.com/t/9014802374/86b9txpuj
Summary by CodeRabbit
New Features
Documentation