feat: password reset flow frontend pages

forgejo_admin commented

2026-03-21 16:43:42 +00:00

Author

Owner

PR #63 Review

DOMAIN REVIEW

Tech stack: SvelteKit (Svelte 5 runes), scoped CSS, REST API integration via $lib/api.js.

Security — anti-enumeration (good)
The forgot-password page correctly shows a generic "check your email" message regardless of API response status. The if (!res.ok) block is intentionally empty, falling through to submitted = true. This prevents email enumeration. Well done.

API integration (good)
Both pages correctly import API_BASE from $lib/api.js and use raw fetch() instead of the authenticated apiFetch() wrapper. This is correct — these are unauthenticated public endpoints. Token is not leaked into the request.

Password validation (good)

Client-side: min 8 chars + passwords match, with real-time oninput validation feedback
HTML: minlength="8" attribute provides native browser validation as backup
Submit button disabled until passwordsMatch is true
Server error parsing handles both error and detail response fields with a safe fallback

Token handling (good)
$derived($page.url.searchParams.get('token')) is reactive. The no-token state renders a clear error card with a link to request a new token. Expired/invalid token errors from the API are caught and displayed.

State management (good)
Both pages use Svelte 5 $state and $derived runes correctly. The double-submit guard (if (submitting) return) is in place on both forms.

PUBLIC_ROUTES (good)
Both /forgot-password and /reset-password added to the auth guard whitelist in +layout.svelte. The layout's PUBLIC_ROUTES.some() check with trailing-slash handling covers both paths.

CSS pattern consistency (acceptable)
Both pages use hardcoded hex values in scoped <style> blocks. This matches the existing pattern across checkout/cancel, checkout/success, and other pages. The codebase does define CSS custom properties in :root (e.g., --color-red: #d42026), but scoped styles in route pages consistently hardcode values. Not a new deviation.

BLOCKERS

None.

Re: test coverage — This repo has zero test infrastructure (no vitest, no playwright config, no test files). There is an open issue (#62) for E2E tests. Adding test infrastructure is out of scope for this PR, and the manual test plan in the PR body is thorough. This is not a blocker given the repo's current state.

NITS

Massive CSS duplication between the two pages. The forgot-password and reset-password scoped styles share ~130 lines of near-identical CSS (.form-group, .form-label, .form-input, .btn-action, .back-link, .success-message, .success-note, plus the page/card/header/logo/title/subtitle patterns). If a third password-flow page is ever added, or if the design changes, these will diverge. Consider extracting shared form/card styles into app.css in a follow-up.
!important overrides in .success-note. Both pages use color: #737373 !important; font-size: 0.8rem !important; margin-bottom: 0 !important; to override parent .success-message p rules. This works but is fragile. A more specific selector (.success-message .success-note) would avoid !important.
Accessibility: no ARIA live regions. When the form submits and transitions to the success/error state, screen readers will not announce the change. Adding aria-live="polite" to the success-message and error-card containers would improve accessibility for parents using assistive technology.
Cross-repo "Closes" reference is wrong. The PR body says Closes forgejo_admin/westside-app#132 but westside-app has no issue #132. The parent issue is forgejo_admin/basketball-api#132. This should be Closes forgejo_admin/basketball-api#132 or simply reference it as Related: forgejo_admin/basketball-api#132 (since the frontend PR alone doesn't close a cross-repo feature issue).
Logo URL hardcoded in both pages. https://minio-api.tail5b443a.ts.net/assets/westside/branding/logo.jpeg appears in forgot-password, reset-password, signin, register, and layout. This is pre-existing across the codebase, but worth noting — if the MinIO hostname changes, every page needs updating. A $lib/constants.js would help.

SOP COMPLIANCE

Branch named after issue: 132-password-reset-frontend references basketball-api #132
PR body follows template: Summary, Changes, Test Plan, Review Checklist, Related sections present
No secrets committed: API_BASE uses the existing public URL, no keys/tokens in code
No unnecessary file changes: 5 files changed, all directly related to the feature
Commit messages are descriptive
Related references plan slug: PR body has no plan slug reference — only "Closes forgejo_admin/westside-app#132" (which is also the wrong repo, see nit #4)

PROCESS OBSERVATIONS

Deployment frequency: This is a clean, self-contained frontend feature. Two new routes, one layout change, one signin link, one CSS addition. Low risk, high value — Tuesday blocker for girls tryout.
Change failure risk: Low. The pages are isolated (new routes, no modifications to existing business logic). The only shared-code change is adding two entries to PUBLIC_ROUTES, which is additive and safe.
Cross-repo dependency: This PR depends on basketball-api PR #133 for the backend endpoints (/api/password-reset/request and /api/password-reset/confirm). Both must be deployed together. The frontend gracefully handles API errors, so deploying frontend-first would show a user-friendly error rather than a crash.
CSS debt: The duplicated scoped styles across pages is growing. Issue #62 (E2E tests) and a CSS extraction pass would both reduce long-term maintenance burden.

VERDICT: APPROVED

Clean implementation of the password reset frontend. Security-conscious (anti-enumeration), properly integrated with the auth guard, mobile-friendly card layouts consistent with existing pages, solid client-side validation. The CSS duplication and missing ARIA live regions should be tracked as follow-up work but do not block this Tuesday-critical feature.

## PR #63 Review ### DOMAIN REVIEW **Tech stack**: SvelteKit (Svelte 5 runes), scoped CSS, REST API integration via `$lib/api.js`. **Security — anti-enumeration (good)** The forgot-password page correctly shows a generic "check your email" message regardless of API response status. The `if (!res.ok)` block is intentionally empty, falling through to `submitted = true`. This prevents email enumeration. Well done. **API integration (good)** Both pages correctly import `API_BASE` from `$lib/api.js` and use raw `fetch()` instead of the authenticated `apiFetch()` wrapper. This is correct — these are unauthenticated public endpoints. Token is not leaked into the request. **Password validation (good)** - Client-side: min 8 chars + passwords match, with real-time `oninput` validation feedback - HTML: `minlength="8"` attribute provides native browser validation as backup - Submit button disabled until `passwordsMatch` is true - Server error parsing handles both `error` and `detail` response fields with a safe fallback **Token handling (good)** `$derived($page.url.searchParams.get('token'))` is reactive. The no-token state renders a clear error card with a link to request a new token. Expired/invalid token errors from the API are caught and displayed. **State management (good)** Both pages use Svelte 5 `$state` and `$derived` runes correctly. The double-submit guard (`if (submitting) return`) is in place on both forms. **PUBLIC_ROUTES (good)** Both `/forgot-password` and `/reset-password` added to the auth guard whitelist in `+layout.svelte`. The layout's `PUBLIC_ROUTES.some()` check with trailing-slash handling covers both paths. **CSS pattern consistency (acceptable)** Both pages use hardcoded hex values in scoped `<style>` blocks. This matches the existing pattern across `checkout/cancel`, `checkout/success`, and other pages. The codebase does define CSS custom properties in `:root` (e.g., `--color-red: #d42026`), but scoped styles in route pages consistently hardcode values. Not a new deviation. ### BLOCKERS None. **Re: test coverage** — This repo has zero test infrastructure (no vitest, no playwright config, no test files). There is an open issue (#62) for E2E tests. Adding test infrastructure is out of scope for this PR, and the manual test plan in the PR body is thorough. This is not a blocker given the repo's current state. ### NITS 1. **Massive CSS duplication between the two pages.** The `forgot-password` and `reset-password` scoped styles share ~130 lines of near-identical CSS (`.form-group`, `.form-label`, `.form-input`, `.btn-action`, `.back-link`, `.success-message`, `.success-note`, plus the page/card/header/logo/title/subtitle patterns). If a third password-flow page is ever added, or if the design changes, these will diverge. Consider extracting shared form/card styles into `app.css` in a follow-up. 2. **`!important` overrides in `.success-note`.** Both pages use `color: #737373 !important; font-size: 0.8rem !important; margin-bottom: 0 !important;` to override parent `.success-message p` rules. This works but is fragile. A more specific selector (`.success-message .success-note`) would avoid `!important`. 3. **Accessibility: no ARIA live regions.** When the form submits and transitions to the success/error state, screen readers will not announce the change. Adding `aria-live="polite"` to the success-message and error-card containers would improve accessibility for parents using assistive technology. 4. **Cross-repo "Closes" reference is wrong.** The PR body says `Closes forgejo_admin/westside-app#132` but westside-app has no issue #132. The parent issue is `forgejo_admin/basketball-api#132`. This should be `Closes forgejo_admin/basketball-api#132` or simply reference it as `Related: forgejo_admin/basketball-api#132` (since the frontend PR alone doesn't close a cross-repo feature issue). 5. **Logo URL hardcoded in both pages.** `https://minio-api.tail5b443a.ts.net/assets/westside/branding/logo.jpeg` appears in forgot-password, reset-password, signin, register, and layout. This is pre-existing across the codebase, but worth noting — if the MinIO hostname changes, every page needs updating. A `$lib/constants.js` would help. ### SOP COMPLIANCE - [x] Branch named after issue: `132-password-reset-frontend` references basketball-api #132 - [x] PR body follows template: Summary, Changes, Test Plan, Review Checklist, Related sections present - [x] No secrets committed: API_BASE uses the existing public URL, no keys/tokens in code - [x] No unnecessary file changes: 5 files changed, all directly related to the feature - [x] Commit messages are descriptive - [ ] Related references plan slug: PR body has no plan slug reference — only "Closes forgejo_admin/westside-app#132" (which is also the wrong repo, see nit #4) ### PROCESS OBSERVATIONS - **Deployment frequency**: This is a clean, self-contained frontend feature. Two new routes, one layout change, one signin link, one CSS addition. Low risk, high value — Tuesday blocker for girls tryout. - **Change failure risk**: Low. The pages are isolated (new routes, no modifications to existing business logic). The only shared-code change is adding two entries to `PUBLIC_ROUTES`, which is additive and safe. - **Cross-repo dependency**: This PR depends on basketball-api PR #133 for the backend endpoints (`/api/password-reset/request` and `/api/password-reset/confirm`). Both must be deployed together. The frontend gracefully handles API errors, so deploying frontend-first would show a user-friendly error rather than a crash. - **CSS debt**: The duplicated scoped styles across pages is growing. Issue #62 (E2E tests) and a CSS extraction pass would both reduce long-term maintenance burden. ### VERDICT: APPROVED Clean implementation of the password reset frontend. Security-conscious (anti-enumeration), properly integrated with the auth guard, mobile-friendly card layouts consistent with existing pages, solid client-side validation. The CSS duplication and missing ARIA live regions should be tracked as follow-up work but do not block this Tuesday-critical feature.

2026-03-21 16:31:24 +00:00

Summary

Add /forgot-password and /reset-password frontend pages so parents can reset their own passwords through the basketball-api Gmail OAuth flow, bypassing Keycloak's broken built-in reset link
Both pages are public, mobile-friendly, and styled to match the existing dark theme
Signin page now includes a "Forgot Password?" link giving players a working path

Changes

src/routes/forgot-password/+page.svelte: New page with email input form. POSTs to /api/password-reset/request. Shows generic "check your email" message on success (no email enumeration).
src/routes/reset-password/+page.svelte: New page that reads token from URL query params. Two password inputs with client-side match validation (min 8 chars). POSTs to /api/password-reset/confirm. Shows success with link to signin, or error with link to request new token.
src/routes/+layout.svelte: Added /forgot-password and /reset-password to PUBLIC_ROUTES so the auth guard does not redirect these pages.
src/routes/signin/+page.svelte: Added "Forgot Password?" link between Sign In button and "Back to home" link.
src/app.css: Added .forgot-link styles for the signin page link.

Test Plan

Visit /forgot-password — form renders, submit with email shows success message
Visit /reset-password?token=test — form renders, password mismatch validation works client-side
Visit /reset-password (no token) — "no token" error state with link to request a new one
Visit /signin — "Forgot Password?" link visible and navigates to /forgot-password
Both pages accessible without authentication (PUBLIC_ROUTES)
Mobile viewport (375px) — centered card layouts render correctly
npm run build and npm run check pass with no new errors

Review Checklist

Passed automated review-fix loop
No secrets committed
No unnecessary file changes
Commit messages are descriptive

Closes forgejo_admin/westside-app#132

## Summary - Add `/forgot-password` and `/reset-password` frontend pages so parents can reset their own passwords through the basketball-api Gmail OAuth flow, bypassing Keycloak's broken built-in reset link - Both pages are public, mobile-friendly, and styled to match the existing dark theme - Signin page now includes a "Forgot Password?" link giving players a working path ## Changes - `src/routes/forgot-password/+page.svelte`: New page with email input form. POSTs to `/api/password-reset/request`. Shows generic "check your email" message on success (no email enumeration). - `src/routes/reset-password/+page.svelte`: New page that reads `token` from URL query params. Two password inputs with client-side match validation (min 8 chars). POSTs to `/api/password-reset/confirm`. Shows success with link to signin, or error with link to request new token. - `src/routes/+layout.svelte`: Added `/forgot-password` and `/reset-password` to PUBLIC_ROUTES so the auth guard does not redirect these pages. - `src/routes/signin/+page.svelte`: Added "Forgot Password?" link between Sign In button and "Back to home" link. - `src/app.css`: Added `.forgot-link` styles for the signin page link. ## Test Plan - [ ] Visit `/forgot-password` — form renders, submit with email shows success message - [ ] Visit `/reset-password?token=test` — form renders, password mismatch validation works client-side - [ ] Visit `/reset-password` (no token) — "no token" error state with link to request a new one - [ ] Visit `/signin` — "Forgot Password?" link visible and navigates to `/forgot-password` - [ ] Both pages accessible without authentication (PUBLIC_ROUTES) - [ ] Mobile viewport (375px) — centered card layouts render correctly - [ ] `npm run build` and `npm run check` pass with no new errors ## Review Checklist - [x] Passed automated review-fix loop - [x] No secrets committed - [x] No unnecessary file changes - [x] Commit messages are descriptive ## Related - Closes forgejo_admin/westside-app#132

forgejo_admin added 1 commit

feat: add password reset frontend pages

ci/woodpecker/pr/woodpecker Pipeline was successful

Details

23cfda0ddd

Add /forgot-password and /reset-password pages so parents can reset
their own passwords through Gmail OAuth instead of Keycloak's broken
built-in flow. Both pages are public (added to PUBLIC_ROUTES) and
styled to match the existing dark theme. The signin page now includes
a "Forgot Password?" link.

Ref #132

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

2026-03-21 16:31:49 +00:00

Self-review: PASS -- no issues found.

Reviewed all 5 changed files (512 additions, 1 deletion):

Security: forgot-password shows generic "check your email" message regardless of email validity -- no enumeration possible
Error handling: Both pages handle network failures and API errors with clear user-facing messages
Validation: reset-password validates min 8 chars + password match client-side before POST
Edge cases: Missing token on /reset-password shows error card with link to /forgot-password; expired/invalid token shows error with retry link
Auth guard: Both routes added to PUBLIC_ROUTES in +layout.svelte
Styling: Scoped styles match existing dark theme (signin, checkout/cancel) -- same colors, fonts, spacing, radial gradient background
Build: npm run build and npm run check pass with no new errors or warnings

**Self-review: PASS -- no issues found.** Reviewed all 5 changed files (512 additions, 1 deletion): - **Security**: forgot-password shows generic "check your email" message regardless of email validity -- no enumeration possible - **Error handling**: Both pages handle network failures and API errors with clear user-facing messages - **Validation**: reset-password validates min 8 chars + password match client-side before POST - **Edge cases**: Missing token on /reset-password shows error card with link to /forgot-password; expired/invalid token shows error with retry link - **Auth guard**: Both routes added to PUBLIC_ROUTES in +layout.svelte - **Styling**: Scoped styles match existing dark theme (signin, checkout/cancel) -- same colors, fonts, spacing, radial gradient background - **Build**: `npm run build` and `npm run check` pass with no new errors or warnings