From 7996395d31b72787d9897e2f192edb7f348d948b Mon Sep 17 00:00:00 2001 From: rudrabjoshi Date: Mon, 24 Aug 2026 11:12:20 -0700 Subject: [PATCH 1/5] Add Flask password-sync utility FlaskPasswordSync calls Flask's internal sync endpoint (POST /api/internal/sync-password) with a shared secret (INTERNAL_SYNC_KEY), so a password reset completed on Spring also lands on the Flask account for the same uid. Not wired up to any caller yet -- that's the OAuth-verified reset flow, next PR in the stack. Best-effort: a sync failure is logged, not fatal to whatever already-successful operation triggered it. Co-Authored-By: Claude Sonnet 5 --- .../spring/mvc/person/FlaskPasswordSync.java | 84 +++++++++++++++++++ 1 file changed, 84 insertions(+) create mode 100644 src/main/java/com/open/spring/mvc/person/FlaskPasswordSync.java diff --git a/src/main/java/com/open/spring/mvc/person/FlaskPasswordSync.java b/src/main/java/com/open/spring/mvc/person/FlaskPasswordSync.java new file mode 100644 index 00000000..ac393ba0 --- /dev/null +++ b/src/main/java/com/open/spring/mvc/person/FlaskPasswordSync.java @@ -0,0 +1,84 @@ +package com.open.spring.mvc.person; + +import java.net.URI; +import java.net.http.HttpClient; +import java.net.http.HttpRequest; +import java.net.http.HttpResponse; +import java.nio.charset.StandardCharsets; +import java.time.Duration; + +import org.json.JSONObject; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import io.github.cdimascio.dotenv.Dotenv; + +// Server-to-server call into Flask's /api/internal/sync-password, so a password +// reset completed here (OAuth + student ID verified) also lands on the Flask +// account for the same uid. Gated by a shared secret (INTERNAL_SYNC_KEY) that +// must match Flask's own config -- see GoogleIdTokenVerifier for the same +// env-then-dotenv resolution pattern used here. +public class FlaskPasswordSync { + private static final Logger logger = LoggerFactory.getLogger(FlaskPasswordSync.class); + private static final HttpClient HTTP_CLIENT = HttpClient.newBuilder() + .connectTimeout(Duration.ofSeconds(10)) + .build(); + + private static String resolve(String envKey, String fallback) { + String value = System.getenv(envKey); + if (value != null && !value.isBlank()) { + return value; + } + try { + Dotenv dotenv = Dotenv.configure().ignoreIfMissing().load(); + value = dotenv.get(envKey); + if (value != null && !value.isBlank()) { + return value; + } + } catch (Exception e) { + // fall through to default + } + return fallback; + } + + // Best-effort: the Spring-side reset has already succeeded by the time this is + // called, so a Flask sync failure is logged and swallowed rather than failing + // the whole request -- the user's new password is already live on Spring, + // which is the backend this feature actually verified identity against. + public static boolean syncPassword(String uid, String newPassword) { + String syncKey = resolve("INTERNAL_SYNC_KEY", null); + String flaskUri = resolve("FLASK_URI", "http://localhost:8587"); + + if (syncKey == null) { + logger.warn("AUDIT flask_password_sync_skipped uid={} reason=no_sync_key_configured", uid); + return false; + } + + try { + JSONObject payload = new JSONObject(); + payload.put("uid", uid); + payload.put("password", newPassword); + + HttpRequest request = HttpRequest.newBuilder() + .uri(URI.create(flaskUri + "/api/internal/sync-password")) + .header("Content-Type", "application/json") + .header("X-Internal-Sync-Key", syncKey) + .timeout(Duration.ofSeconds(10)) + .POST(HttpRequest.BodyPublishers.ofString(payload.toString(), StandardCharsets.UTF_8)) + .build(); + + HttpResponse response = HTTP_CLIENT.send(request, HttpResponse.BodyHandlers.ofString()); + + if (response.statusCode() == 200) { + logger.info("AUDIT flask_password_sync_succeeded uid={}", uid); + return true; + } + + logger.warn("AUDIT flask_password_sync_failed uid={} status={}", uid, response.statusCode()); + return false; + } catch (Exception e) { + logger.warn("AUDIT flask_password_sync_failed uid={} reason=exception msg={}", uid, e.getMessage()); + return false; + } + } +} From f57ed8dd8ab4b8901142bf1a552e35345248c97c Mon Sep 17 00:00:00 2001 From: rudrabjoshi Date: Mon, 24 Aug 2026 11:16:48 -0700 Subject: [PATCH 2/5] Add OAuth + student ID verified password reset flow New security-driven self-service reset: a student proves account ownership by signing in with their @stu.powayusd.com school Google account, and the trailing 5 digits of that (server-verified) email must match the last 5 digits of the account's stored sid before a reset is allowed. GoogleIdTokenVerifier verifies the Google Identity Services ID token server-side via Google's tokeninfo endpoint (checks aud, iss, email_verified) -- the existing signup flow only ever decoded this token client-side and never verified it, fine as a UX nicety but not acceptable as an actual security gate. POST /mvc/person/reset/oauth/verify reuses the admin/default-account guards and rate limiting from the existing email-based /reset/start flow, and on success issues a single-use token via the existing ResetCode infrastructure. POST /mvc/person/reset/oauth/complete spends that token and sets the new password, then best-effort syncs it to Flask via FlaskPasswordSync (added in the previous PR in this stack). Also invalidates JWTs when a password changes (Person.tokenVersion, bumped in PersonDetailsService.save whenever samePassword is false; JwtTokenUtil stamps and checks it), fixes ResetCode's RESET_TOKEN_SECRET resolution so it actually sees Spring's .env-imported value instead of silently signing tokens with a random per-restart key, and repoints login.html's "Forgot Password?" link at the new verified reset wizard on the pages site. Co-Authored-By: Claude Sonnet 5 --- docs/forgot-password-pipeline.md | 193 ++++++++++++++++++ .../spring/mvc/person/Email/ResetCode.java | 52 ++++- .../mvc/person/GoogleIdTokenVerifier.java | 82 ++++++++ .../com/open/spring/mvc/person/Person.java | 9 + .../mvc/person/PersonDetailsService.java | 3 + .../mvc/person/PersonViewController.java | 136 ++++++++++++ .../open/spring/security/JwtTokenUtil.java | 34 ++- .../spring/security/MvcSecurityConfig.java | 4 + src/main/resources/templates/login.html | 12 +- 9 files changed, 508 insertions(+), 17 deletions(-) create mode 100644 docs/forgot-password-pipeline.md create mode 100644 src/main/java/com/open/spring/mvc/person/GoogleIdTokenVerifier.java diff --git a/docs/forgot-password-pipeline.md b/docs/forgot-password-pipeline.md new file mode 100644 index 00000000..ef419b86 --- /dev/null +++ b/docs/forgot-password-pipeline.md @@ -0,0 +1,193 @@ +# Forgot Password Pipeline + +How password reset works across `pages`, `spring`, and `flask` — the three independently-versioned pieces of the same full-stack app. Current as of 2026-08-20. + +> ### ⚠️ REQUIRED BEFORE THIS DEPLOYS TO PRODUCTION +> +> The `token_version` migration (see "Session/token invalidation" below) has only been applied to the **local dev SQLite databases**. Production Flask runs **MySQL** (`__init__.py` — `SQLALCHEMY_DATABASE_URI` switches to MySQL whenever `DB_ENDPOINT`/`DB_USERNAME`/`DB_PASSWORD` are set), which is a completely separate database this session had no access to. **Someone must run this against production before the Flask code ships, or every login there will start throwing errors on the missing column:** +> +> ```sql +> ALTER TABLE users ADD COLUMN token_version INTEGER NOT NULL DEFAULT 0; +> ``` +> +> Spring's production schema needs the equivalent, run against whatever database backs that deployment (`RESET_TOKEN_SECRET`-style `.env`/`Dotenv` resolution doesn't apply here — this is a raw DB migration, not an env var): +> +> ```sql +> ALTER TABLE person ADD COLUMN token_version bigint NOT NULL DEFAULT 0; +> ``` +> +> The `reset_ticket` table itself (Spring, the "Escape hatch" feature below) is also new and won't exist in production yet either — this one's a full table, not a single column: +> +> ```sql +> CREATE TABLE IF NOT EXISTS "reset_ticket" ( +> "attempts_granted" integer not null, +> "created_at" varchar(255), +> "name" varchar(255), +> "resolved" boolean not null, +> "resolved_at" varchar(255), +> "uid" varchar(255) not null, +> "id" integer, +> primary key ("id") +> ); +> ``` +> +> (SQLite syntax shown, matching what's in local dev — adjust types for whatever production actually runs.) + +## Summary + +There are **three distinct reset paths** live in the system today, plus an escape hatch for when the primary path's rate limit is hit. Only one of them is the advertised, primary path; the other two remain reachable but unlinked from the main UI. + +| Path | Verifies identity via | User picks own password? | Status | +|---|---|---|---| +| OAuth + Student ID (primary) | Google Sign-In + student ID digit match | Yes | Live, advertised everywhere | +| Email code (legacy) | Emailed one-time code | No — reset to `DEFAULT_PASSWORD` | Live, reachable only by direct link | +| Admin reset | Admin's own session | No — reset to `DEFAULT_PASSWORD` | Live, admin portal only | +| Reset ticket (escape hatch) | None (admin manually approves) | N/A — grants more attempts, doesn't reset | Live, triggers when rate-limited | + +**Before treating any of this as hardened:** see "Known gaps" below. Session/token invalidation on password change (the most significant item) has since been fixed on both backends' JWT paths and on Flask's session path — see "Security mechanisms." Spring's separate MVC session path (`HttpSession`, form login under `/mvc/**`) remains open; that one's scoped out for now, not fixed. + +## System roles + +- **`pages`** — the only frontend. Owns the reset wizard UI (`navigation/authentication/support.md`) and the two "Forgot Password?" entry points (its own `login.md`, plus Spring's server-rendered `login.html`). +- **`spring`** — the identity authority. Verifies who's requesting a reset, issues signed reset tokens, and is the only system that decides whether a reset is allowed. +- **`flask`** — a downstream mirror, nothing more. It has no reset logic of its own; it exists to keep its own password copy in sync with whatever Spring just verified. This is intentional, not a gap. + +## Primary flow: OAuth + Student ID verified reset + +```mermaid +sequenceDiagram + participant U as User (browser) + participant P as pages (support.md) + participant S as spring + participant G as Google + participant F as flask + + U->>P: Click "Forgot Password?" + P->>U: Step 1 — enter GitHub uid + U->>P: Submit uid + P->>U: Step 2 — "Sign in with school account" + U->>G: Google Identity Services sign-in + G-->>P: idToken (credential) + P->>S: POST /mvc/person/reset/oauth/verify {uid, idToken} + S->>S: Check rate limit (3 / 15 min) + S->>G: Verify idToken server-side (tokeninfo) + S->>S: Match email domain + last 5 digits vs sid + alt verified + S-->>P: {verified: true, resetToken} + P->>U: Step 3 — choose new password + U->>P: Submit new password + P->>S: POST /mvc/person/reset/oauth/complete {uid, resetToken, newPassword} + S->>S: Validate + consume token, BCrypt-hash, save + S->>F: POST /api/internal/sync-password {uid, password} + F->>F: PBKDF2-hash, save (shared-secret auth) + S-->>P: 200 OK + P->>U: Redirect to /login + else denied or rate-limited + S-->>P: 403 / 429 (identical body either way) + P->>U: Generic failure message + "Request a Ticket Instead" (on 429) + end +``` + +### Step by step + +1. **Entry points** — `pages/_layouts/profile.html` and `pages/navigation/authentication/login.md` both link to `/support?topic=reset`, which deep-links straight into the wizard. Spring's own `login.html` (served at `/login` on the Spring origin) links to the same wizard cross-origin, resolving the `pages` host from `localhost:4000` or `pages.opencodingsociety.com` depending on environment. + +2. **Step 1 — identify the account.** User enters their GitHub `uid` in `support.md`. Nothing is sent to the server yet. + +3. **Step 2 — prove ownership via Google.** Google Identity Services renders a sign-in button. On success, the browser gets an `idToken` and the frontend POSTs `{uid, idToken}` to `spring`'s `POST /mvc/person/reset/oauth/verify`. + + Server-side, in order: + - Rejects unknown uids, admin accounts, and seeded default accounts. + - Checks the shared rate limiter (`ResetCode.canIssueResetCode`) — 3 requests per rolling 15-minute window per uid, plus one active token at a time. Fails with `429` if exceeded. + - Verifies the Google ID token server-side against Google's tokeninfo endpoint (`GoogleIdTokenVerifier`) — checks `aud`, `iss`, and `email_verified`. + - Regex-matches the verified email against the school domain pattern and extracts the trailing 5 digits. + - Compares those digits to the last 5 digits of the account's `sid` on file. + - On success, issues a single-use, HMAC-SHA256-signed reset token (5-minute TTL) via `ResetCode.GenerateResetCode`. + + **Every denial path returns an identical response body** (`{"verified":false}`), regardless of which check failed — this is deliberate, so the endpoint can't be used to enumerate valid uid/sid pairs. The specific reason is only ever written to the server log. + +4. **Step 3 — set a new password.** The frontend POSTs `{uid, resetToken, newPassword}` to `POST /mvc/person/reset/oauth/complete`. Spring validates and consumes the token (single-use — a second attempt with the same token fails), requires the password be at least 8 characters, BCrypt-hashes it, and saves it. + +5. **Cross-backend sync.** Immediately after saving, Spring calls `FlaskPasswordSync.syncPassword(uid, newPassword)` — a server-to-server POST to `flask`'s `POST /api/internal/sync-password`, authenticated by a static shared secret (`X-Internal-Sync-Key` header, compared via constant-time `hmac.compare_digest`). Flask re-hashes the password with PBKDF2-SHA256 and updates its own row. This call is best-effort: if it fails, Spring's reset has already succeeded and the request still returns success to the user — Flask just falls behind until the next successful reset syncs it again. + +## Escape hatch: reset tickets + +If a user exhausts the rate limit (3 attempts / 15 min) before getting through, the wizard shows a **"Request a Ticket Instead"** button in place of the normal retry message. + +1. Frontend POSTs `{uid}` to `POST /mvc/person/reset/ticket`. Spring creates a `ResetTicket` row (or silently reuses an existing open one for that uid — idempotent, no duplicate tickets). This endpoint is unauthenticated and takes an arbitrary uid, so per-uid idempotency alone doesn't stop someone from paging through many *different* real uids to spam the admin queue — it's additionally rate-limited to 5 requests per 15 minutes per caller IP (`ResetCode.canRequestTicket`), separately from the global per-request `RateLimitFilter` (which is tuned for gross abuse, not this specific pattern). +2. An admin sees open tickets in a **"Password Reset Tickets"** panel at the top of the person-admin portal (`/mvc/person/read`). The panel only renders when at least one ticket is open. +3. Clicking **"Grant 5 Attempts"** calls `POST /mvc/person/reset/ticket/{id}/grant`. This calls `ResetCode.grantBonusAttempts(uid, 5)`, which raises that uid's allowed-requests ceiling by 5 for the current window, and marks the ticket resolved. +4. The user can now retry the OAuth flow immediately — granting doesn't reset a password itself, it just lifts the block so the normal flow can run again. + +No identity re-verification happens at ticket-request time — the ticket only *asks* for help; the actual identity check still happens in the normal OAuth flow once the user retries. Admin approval is the trust boundary here, same as it is for the direct admin-reset button. + +⚠️ **Was broken for its entire actual purpose until this was caught:** `POST /mvc/person/reset/ticket` was never added to `MvcSecurityConfig`'s `permitAll()` list, so an anonymous request — the *only* kind this endpoint should ever see, since a rate-limited user is by definition not logged in — got redirected to `/login` (`302`) instead of creating a ticket. Every test of it this session used an authenticated admin session (curl with a saved cookie jar), which never exercised the real caller path and completely masked the bug. Found by writing `scripts/inject_reset_tickets.py` to call it the way a real locked-out user would (no session) — its first run reported a false "200 created" because Python's `urllib` followed the redirect to `/login` and reported *that* page's `200`. Fixed by adding the route to `permitAll()` and by making the script refuse to follow redirects, so this exact class of bug can't hide again. `POST /mvc/person/reset/ticket/{id}/grant` (admin-only) was correctly left off `permitAll()` the whole time — it falls through to `anyRequest().authenticated()` plus the controller's own `ROLE_ADMIN` check, same pattern as `/mvc/person/reset/admin/{id}`. + +## Legacy paths (still live, no longer advertised) + +**"Unlinked" is not "disabled."** Nothing below has been removed or gated off — de-linking the old "Forgot Password?" button only stops people from *discovering* these routes through the UI. Anyone who already has the URL, or finds it in browser history / an old bookmark / this document, can still hit them directly and they work exactly as before. That's security through obscurity, not a control. If these flows are genuinely no longer needed, actually disabling the endpoints (403 them, or delete the code) would be the stronger fix; that hasn't been done. + +**Email-code reset** (`Flow A`) — `GET /mvc/person/reset` → `POST /mvc/person/reset/start` emails a signed code via FormSubmit.co → `POST /mvc/person/reset/check` verifies it. On success, the account's password is set to the env-configured `DEFAULT_PASSWORD`, **not** a user-chosen password — the user is expected to log in and change it. This flow shares the same `ResetCode` rate limiter and token machinery as the OAuth flow. It's no longer linked from any "Forgot Password?" button (Spring's `login.html` was repointed to the OAuth wizard), but the routes themselves are untouched and still work for anyone with a direct link. + +This flow also routes the reset code through **FormSubmit.co**, a third-party form-relay service — the code is effectively a bearer credential in transit through infrastructure this project doesn't control. Their logging/retention policy hasn't been reviewed here; worth checking before treating this path as low-risk. + +**Admin reset** (`Flow B`) — two independent implementations, both admin-only, both reset straight to `DEFAULT_PASSWORD` with no token involved: +- Spring: `POST /mvc/person/reset/admin/{id}` (the "Reset Password" button in the person-admin table). +- Flask: `POST /users/reset_password/`. + +They don't call each other — an admin using Spring's button does not sync to Flask, and vice versa. + +## Security mechanisms + +- **Token signing** — `ResetCode` signs tokens as `base64(uid).expiresAt.nonce.HMAC-SHA256(uid.expiresAt.nonce)`, keyed by `RESET_TOKEN_SECRET`. The signing key is resolved the same way as other cross-service secrets in this codebase (env var, then `.env` via the Dotenv library) and the app **refuses to sign tokens** if it's unset, rather than falling back to a randomly generated key — an earlier version of this code did fall back silently, which meant every restart silently invalidated all outstanding tokens without anyone noticing. +- **Rate limiting** — 3 requests per rolling 15-minute window per uid, shared between the OAuth and email-code flows, tracked in-process (acceptable given this deploys as a single Spring instance, per its `docker-compose.yml`). +- **Enumeration resistance** — the OAuth verify endpoint's failure responses are indistinguishable regardless of cause (unknown uid vs. sid mismatch vs. bad token all return the same shape). +- **Password storage** — Spring hashes with BCrypt; Flask hashes independently with PBKDF2-SHA256. They're separate hashes of the same plaintext, computed at sync time — not shared or convertible between the two backends. +- **Inter-service auth** — the Spring → Flask sync call is gated by a static shared secret (`INTERNAL_SYNC_KEY`), compared with a constant-time comparison on the Flask side. If unset, Flask returns `401` (fails closed) and Spring logs a warning rather than pretending to succeed. The bigger question for this call was never the auth — it's transport: the request body is `{uid, password}` with the **new password in plaintext**, and the shared secret alone doesn't protect data-in-transit the way TLS would. Whether that matters depends entirely on whether the call ever leaves loopback in production; the deployment evidence (see below) points to same-host today, but nothing enforced it. **Fixed:** `FlaskPasswordSync` now refuses the sync (logs and skips, doesn't fail the reset) unless `FLASK_URI` resolves to loopback (`localhost`/`127.0.0.1`, host parsed via `java.net.URI`, not string-prefix matching — a prefix check would wrongly pass a lookalike like `http://localhost.attacker.com`) or the scheme is `https://`. So even if `FLASK_URI` is ever pointed at a public host over plain HTTP, the plaintext password no longer goes out over the wire — the sync just gets skipped and logged instead. Deployment evidence for why loopback is the current reality: `spring/nginx_spring_8585_8589.conf` and `flask/nginx_flask_8587.conf` both front the *same* public IP and both proxy back to `localhost:` — the standard single-box, two-app pattern, reinforced by both READMEs describing production deploys through the same "cockpit" admin panel. Neither repo contains any tracked config (`render.yaml`, `Procfile`, CI/CD, `.env.example`) that actually sets `FLASK_URI` for production, so this was inference from infra files, not a confirmed value — which is exactly why the code-level check was worth adding rather than just trusting the inference. +- **API hardening** — Flask's general-purpose user endpoints (`GET /api/user`, create/update/delete responses) previously included the PBKDF2 password hash in their JSON bodies, readable by any logged-in user, not just admins. Fixed by stripping the `password` field before those responses go out; the admin-only backup/export endpoints still include it, since restoring from a backup needs the hash to round-trip. +- **Ticket-creation rate limiting** — `POST /mvc/person/reset/ticket` is capped at 5 requests per 15 minutes per caller IP (see "Escape hatch" above). Also worth noting: the endpoint silently 500'd on every real request until this was verified, because `ResetTicket`'s `@GeneratedValue(strategy = GenerationType.AUTO)` resolved to sequence-table ID generation on this SQLite dialect, and no such sequence table exists (`ddl-auto=none`, schema managed by hand). Fixed by switching to `GenerationType.IDENTITY`, matching the convention every other SQLite-backed entity in this codebase already uses (e.g. `GameAttempt`). This had gone unnoticed because earlier manual testing inserted ticket rows directly via SQL rather than through the real endpoint. +- **Session/token invalidation on password change.** ⚠️ **The schema migration this needs has only run against local dev databases — see the callout at the very top of this document before deploying either backend.** Previously neither backend tied an issued credential to a specific password: Spring's JWT embedded only `sub` + `roles` and was checked for username-match + a static 12h expiry, nothing password-derived, no revocation registry; Flask's JWT had **no `exp` claim at all**, and Flask-Login sessions carried only a bare user id. A stolen JWT or Flask session cookie kept working indefinitely after the legitimate user reset their password specifically because they suspected compromise. **Fixed on both backends**, at the single funnel every password-change path already goes through (`PersonDetailsService.save`/`User.set_password`): a `tokenVersion`/`token_version` counter is bumped on every real password change (not on idempotent re-saves of the same hash) and embedded in issued JWTs (both backends) and in Flask-Login's session id (`get_id()` returns `"id:token_version"`, checked in `load_user`). Verified live end to end on both: fresh JWT/session → 200, password reset → the old JWT gets `401` (with an explicit "password has changed" message on Flask), the old Flask session gets redirected to `/login`, fresh login after the reset works again. **Scope note:** this covers `/api/**` on Spring (the JWT-validated surface — `JwtRequestFilter` only runs JWT checks for `/api/**` requests) and both auth paths on Flask. Spring's separate MVC session (`HttpSession`, form login under `/mvc/**`, e.g. the admin portal) is *not* covered — closing that would need Spring Security's concurrent-session/`SessionRegistry` machinery, scoped out as a larger, separate piece of work. + +## Known gaps (not yet addressed) + +These are real, open issues — not by-design tradeoffs: + +- ⚠️ **Production databases don't have the `token_version` column or the `reset_ticket` table yet.** Both were only migrated against local dev SQLite. Production Flask runs MySQL — a completely different database this session never touched. **See the callout at the top of this document for the exact SQL to run before deploying.** Until that runs, production logins/reset-ticket usage will break on the missing schema, not silently degrade. +- **Spring's MVC `HttpSession` path isn't invalidated on password change** (see the scope note above) — only the JWT path is. +- **`DEFAULT_PASSWORD`'s exposure window is unbounded.** There is no forced-password-change flag or mechanism anywhere in `Person`/`PersonDetailsService` — confirmed by grepping for it. An account reset via the email-code or admin-reset flow sits at the shared, guessable `DEFAULT_PASSWORD` indefinitely, until the user happens to log in and manually changes it. Being env-configured rather than hardcoded (see below) limits *some* exposure, but doesn't bound the window in time — an attacker who can trigger a reset (or knows one already happened) has an open account-takeover race against the legitimate user for as long as that user hasn't logged back in. + +## Deliberate non-issues + +A few things that look like gaps but are intentional: + +- **`DEFAULT_PASSWORD` is shared and predictable** across the email-code and admin-reset flows. This is by design — it's environment-configured per deployment, not hardcoded. (The fact that it's env-configured is the intentional part; the unbounded exposure window above is not — see "Known gaps.") +- **Flask has no reset logic of its own.** It's meant to be a pure mirror of whatever Spring decides — adding independent verification to Flask would duplicate the trust boundary, not strengthen it. +- **Rate-limit state is in-memory**, not database-backed. This means a Spring restart clears everyone's rate-limit counters and any in-flight (≤5 min old) tokens. Acceptable given the single-instance deployment; would need revisiting if this ever runs behind a load balancer with multiple instances. + +## Key files + +| System | File | Role | +|---|---|---| +| pages | `navigation/authentication/support.md` | Reset wizard UI, ticket-request button | +| pages | `navigation/authentication/login.md` | "Forgot Password?" entry point | +| pages | `_layouts/profile.html` | "Forgot Password?" entry point (profile page) | +| pages | `assets/js/api/config.js` | Shared `javaURI`, `GOOGLE_CLIENT_ID` | +| spring | `mvc/person/PersonViewController.java` | All reset endpoints (OAuth, email-code, admin, tickets) | +| spring | `mvc/person/Email/ResetCode.java` | Token signing, rate limiting, bonus-attempt grants | +| spring | `mvc/person/GoogleIdTokenVerifier.java` | Server-side Google ID token verification | +| spring | `mvc/person/FlaskPasswordSync.java` | Server-to-server sync call to Flask | +| spring | `mvc/person/ResetTicket.java` / `ResetTicketJpaRepository.java` | Reset-ticket persistence | +| spring | `security/MvcSecurityConfig.java` | `permitAll()` list — see the ⚠️ note under "Escape hatch" | +| spring | `scripts/inject_reset_tickets.py` | Test-creates tickets via the real (unauthenticated) endpoint | +| spring | `templates/person/read.html` | Admin portal — reset-password button, ticket panel | +| spring | `templates/login.html` | Server-rendered login page | +| spring | `security/JwtTokenUtil.java` | JWT issuance/validation, `tokenVersion` claim | +| spring | `security/JwtRequestFilter.java` | JWT validation on `/api/**` only — see MVC-session scope note | +| spring | `security/RateLimitFilter.java` | Global per-request rate limiter (not endpoint-specific) | +| flask | `api/user.py` (`_InternalPasswordSync`) | Receives the sync call from Spring | +| flask | `api/user.py` (`_Security`) | JWT issuance, `token_version` + `exp` claims | +| flask | `api/authorize.py` | Session/JWT validation (`auth_required`), `token_version` check | +| flask | `main.py` (`load_user`, `reset_password`) | Flask-Login session validation, admin reset-to-default route | +| flask | `model/user.py` | Password hashing, `token_version`, `get_id()` | +| spring | `nginx_spring_8585_8589.conf` | Evidence for same-host topology (see "Inter-service auth") | +| flask | `nginx_flask_8587.conf` | Evidence for same-host topology (see "Inter-service auth") | diff --git a/src/main/java/com/open/spring/mvc/person/Email/ResetCode.java b/src/main/java/com/open/spring/mvc/person/Email/ResetCode.java index 15a7e5de..779657f1 100644 --- a/src/main/java/com/open/spring/mvc/person/Email/ResetCode.java +++ b/src/main/java/com/open/spring/mvc/person/Email/ResetCode.java @@ -15,6 +15,8 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import io.github.cdimascio.dotenv.Dotenv; + public class ResetCode { private static final Logger logger = LoggerFactory.getLogger(ResetCode.class); @@ -27,7 +29,7 @@ public class ResetCode { private static final Map> resetRequestTimesByUid = new ConcurrentHashMap<>(); private static final Map lastIssueReasonByUid = new ConcurrentHashMap<>(); - private static final byte[] secret = loadSecret(); + private static volatile byte[] cachedSecret; private static class ResetTokenRecord { private final String token; @@ -39,16 +41,44 @@ private ResetTokenRecord(String token, long expiresAtEpoch) { } } - private static byte[] loadSecret() { - String envSecret = System.getenv("RESET_TOKEN_SECRET"); - if (envSecret != null && !envSecret.isBlank()) { - return envSecret.getBytes(StandardCharsets.UTF_8); + // Same env-then-.env resolution order as FlaskPasswordSync/GoogleIdTokenVerifier: plain + // System.getenv() only sees real OS environment variables, not Spring's own + // spring.config.import=.env mechanism, so a Dotenv fallback is required for local dev + // where the secret only lives in .env. + private static String resolveConfiguredSecret() { + String value = System.getenv("RESET_TOKEN_SECRET"); + if (value != null && !value.isBlank()) { + return value; + } + try { + Dotenv dotenv = Dotenv.configure().ignoreIfMissing().load(); + value = dotenv.get("RESET_TOKEN_SECRET"); + if (value != null && !value.isBlank()) { + return value; + } + } catch (Exception e) { + // fall through } + return null; + } - byte[] generated = new byte[32]; - random.nextBytes(generated); - logger.warn("AUDIT reset_secret_fallback using ephemeral in-memory secret because RESET_TOKEN_SECRET is not set"); - return generated; + // Deliberately fails closed instead of falling back to an ephemeral per-restart secret: + // a randomly generated fallback would silently invalidate every outstanding reset token + // (and undermine the HMAC's whole purpose) on every deploy, without anyone noticing. + private static byte[] getSecret() { + byte[] local = cachedSecret; + if (local != null) { + return local; + } + String configured = resolveConfiguredSecret(); + if (configured == null) { + throw new IllegalStateException( + "RESET_TOKEN_SECRET is not set. Password reset cannot issue or validate tokens " + + "without it -- set RESET_TOKEN_SECRET in the environment or .env file."); + } + local = configured.getBytes(StandardCharsets.UTF_8); + cachedSecret = local; + return local; } private static String base64Url(byte[] value) { @@ -58,8 +88,10 @@ private static String base64Url(byte[] value) { private static String hmacSha256(String payload) { try { Mac mac = Mac.getInstance("HmacSHA256"); - mac.init(new SecretKeySpec(secret, "HmacSHA256")); + mac.init(new SecretKeySpec(getSecret(), "HmacSHA256")); return base64Url(mac.doFinal(payload.getBytes(StandardCharsets.UTF_8))); + } catch (IllegalStateException e) { + throw e; } catch (Exception e) { throw new IllegalStateException("Unable to sign reset token", e); } diff --git a/src/main/java/com/open/spring/mvc/person/GoogleIdTokenVerifier.java b/src/main/java/com/open/spring/mvc/person/GoogleIdTokenVerifier.java new file mode 100644 index 00000000..bfe2e815 --- /dev/null +++ b/src/main/java/com/open/spring/mvc/person/GoogleIdTokenVerifier.java @@ -0,0 +1,82 @@ +package com.open.spring.mvc.person; + +import java.net.URLEncoder; +import java.nio.charset.StandardCharsets; +import java.util.HashMap; +import java.util.Map; + +import org.json.JSONObject; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import com.open.spring.mvc.person.HttpRequest.HttpSender; + +import io.github.cdimascio.dotenv.Dotenv; + +// Verifies a Google Identity Services ID token server-side via Google's tokeninfo +// endpoint, so callers never trust an email a client merely claims to have signed in with. +public class GoogleIdTokenVerifier { + private static final Logger logger = LoggerFactory.getLogger(GoogleIdTokenVerifier.class); + + // Same public client ID hardcoded in navigation/authentication/login.md's GOOGLE_CLIENT_ID. + // Client IDs are not secret; this is only used to check the token's "aud" claim. + private static final String DEFAULT_CLIENT_ID = "65827797404-ccjleg7jg4g2an8ddpmhnlca4ii2gk8q.apps.googleusercontent.com"; + + private static String resolveClientId() { + String value = System.getenv("GOOGLE_CLIENT_ID"); + if (value != null && !value.isBlank()) { + return value; + } + try { + Dotenv dotenv = Dotenv.configure().ignoreIfMissing().load(); + value = dotenv.get("GOOGLE_CLIENT_ID"); + if (value != null && !value.isBlank()) { + return value; + } + } catch (Exception e) { + // fall through to default + } + return DEFAULT_CLIENT_ID; + } + + // Returns the verified email address, or null if the token is missing, expired, + // mis-signed, issued for a different client, or not marked email_verified by Google. + public static String verifyAndGetEmail(String idToken) { + if (idToken == null || idToken.isBlank()) { + return null; + } + + try { + String encoded = URLEncoder.encode(idToken, StandardCharsets.UTF_8); + Map response = HttpSender.sendRequest( + "https://oauth2.googleapis.com/tokeninfo?id_token=" + encoded, + "GET", + new HashMap<>() + ); + + if (!"200".equals(response.get("responseCode"))) { + logger.warn("AUDIT google_token_verify_failed reason=non_200_response code={}", response.get("responseCode")); + return null; + } + + JSONObject claims = new JSONObject(response.get("content")); + String aud = claims.optString("aud", null); + String issuer = claims.optString("iss", null); + boolean emailVerified = "true".equals(claims.optString("email_verified", null)); + String email = claims.optString("email", null); + + boolean issuerOk = "accounts.google.com".equals(issuer) || "https://accounts.google.com".equals(issuer); + boolean audOk = aud != null && aud.equals(resolveClientId()); + + if (!audOk || !issuerOk || !emailVerified || email == null || email.isBlank()) { + logger.warn("AUDIT google_token_verify_failed reason=claim_check_failed"); + return null; + } + + return email; + } catch (Exception e) { + logger.warn("AUDIT google_token_verify_failed reason=exception msg={}", e.getMessage()); + return null; + } + } +} diff --git a/src/main/java/com/open/spring/mvc/person/Person.java b/src/main/java/com/open/spring/mvc/person/Person.java index 8172cca8..c06a522b 100644 --- a/src/main/java/com/open/spring/mvc/person/Person.java +++ b/src/main/java/com/open/spring/mvc/person/Person.java @@ -97,6 +97,15 @@ public class Person extends Submitter implements Comparable { @JsonProperty(access = JsonProperty.Access.WRITE_ONLY) private String password; + // Bumped every time the password actually changes (PersonDetailsService.save, when + // samePassword is false -- the single funnel every reset/update path goes through). + // Embedded in issued JWTs and checked on every /api/** request (JwtTokenUtil); a + // mismatch means the token predates the current password and is rejected, so a + // stolen JWT stops working the moment its owner resets their password instead of + // staying valid for the rest of its 12h lifetime. + @JsonProperty(access = JsonProperty.Access.WRITE_ONLY) + private Long tokenVersion = 0L; + @NotEmpty @Size(min = 1) @Column(unique = true, nullable = false) diff --git a/src/main/java/com/open/spring/mvc/person/PersonDetailsService.java b/src/main/java/com/open/spring/mvc/person/PersonDetailsService.java index e3d88449..c10d33f2 100644 --- a/src/main/java/com/open/spring/mvc/person/PersonDetailsService.java +++ b/src/main/java/com/open/spring/mvc/person/PersonDetailsService.java @@ -97,6 +97,9 @@ public void save(Person person, Boolean samePassword) { if (!samePassword) { // Encode the password only if it's not the same as before person.setPassword(passwordEncoder.encode(person.getPassword())); + // Invalidates every JWT already issued to this person -- see the field + // comment on Person.tokenVersion. + person.setTokenVersion((person.getTokenVersion() == null ? 0L : person.getTokenVersion()) + 1); } personJpaRepository.save(person); // Save the person to the database } diff --git a/src/main/java/com/open/spring/mvc/person/PersonViewController.java b/src/main/java/com/open/spring/mvc/person/PersonViewController.java index 40f5b3bf..03316d82 100644 --- a/src/main/java/com/open/spring/mvc/person/PersonViewController.java +++ b/src/main/java/com/open/spring/mvc/person/PersonViewController.java @@ -32,6 +32,8 @@ import java.util.Arrays; import java.util.Collections; +import java.util.regex.Matcher; +import java.util.regex.Pattern; import lombok.Getter; import org.slf4j.Logger; @@ -503,6 +505,140 @@ public ResponseEntity adminResetPassword(@PathVariable Long id, Authenti return new ResponseEntity<>(HttpStatus.OK); } + // Matches student emails shaped like "firstnamelastinitial12345@stu.powayusd.com" and + // captures the trailing 5 digits, which must match the last 5 digits of the account's sid. + private static final Pattern SCHOOL_EMAIL_DIGITS_PATTERN = + Pattern.compile("^[a-z]+([0-9]{5})@stu\\.powayusd\\.com$"); + + private ResponseEntity oauthResetDenied(HttpStatus status) { + HttpHeaders responseHeaders = new HttpHeaders(); + responseHeaders.setContentType(MediaType.APPLICATION_JSON); + String body = "{\"verified\":false}"; + return new ResponseEntity(body, responseHeaders, status); + } + + @Getter + public static class PersonOAuthResetVerifyBody { + private String uid; + private String idToken; + } + + // Step 1 of the OAuth-verified reset: caller proves ownership of the account by signing + // in with their school Google account. The last 5 digits of that (server-verified) email + // must match the last 5 digits of the sid already on file for this uid before a reset + // token is issued. Denial responses are intentionally identical regardless of which check + // failed, so a caller can't use this endpoint to enumerate uid/sid pairs; the specific + // reason is only ever written to the server log. + @PostMapping("/reset/oauth/verify") + public ResponseEntity resetPasswordOAuthVerify(@RequestBody PersonOAuthResetVerifyBody requestBody) { + if (requestBody == null || requestBody.getUid() == null || requestBody.getUid().isBlank()) { + return new ResponseEntity(HttpStatus.BAD_REQUEST); + } + + Person personToReset = repository.getByUid(requestBody.getUid()); + + //person not found + if (personToReset == null) { + return new ResponseEntity(HttpStatus.NO_CONTENT); + } + + //don't allow people to reset the passwords of admins + if (personToReset.getRoles().stream().anyMatch(role -> "ROLE_ADMIN".equals(role.getName()))) { + return new ResponseEntity(HttpStatus.UNAUTHORIZED); + } + + //dont allow people to reset password of default users (such as toby) + Person[] databasePersons = Person.init(); + for (Person person : databasePersons) { + if (person.getUid().equals(personToReset.getUid())) { + return new ResponseEntity(HttpStatus.UNAUTHORIZED); + } + } + + // enforce active-token and rolling-window rate limits, same as the email-based flow + if (!ResetCode.canIssueResetCode(personToReset.getUid())) { + return new ResponseEntity(HttpStatus.TOO_MANY_REQUESTS); + } + + String verifiedEmail = GoogleIdTokenVerifier.verifyAndGetEmail(requestBody.getIdToken()); + if (verifiedEmail == null) { + logger.warn("AUDIT oauth_reset_denied uid={} reason=invalid_token", personToReset.getUid()); + return oauthResetDenied(HttpStatus.FORBIDDEN); + } + + Matcher matcher = SCHOOL_EMAIL_DIGITS_PATTERN.matcher(verifiedEmail.toLowerCase()); + if (!matcher.matches()) { + logger.warn("AUDIT oauth_reset_denied uid={} reason=email_format", personToReset.getUid()); + return oauthResetDenied(HttpStatus.FORBIDDEN); + } + + String emailDigits = matcher.group(1); + String sid = personToReset.getSid(); + if (sid == null || sid.length() < 5) { + logger.warn("AUDIT oauth_reset_denied uid={} reason=no_sid", personToReset.getUid()); + return oauthResetDenied(HttpStatus.FORBIDDEN); + } + + String sidDigits = sid.substring(sid.length() - 5); + if (!emailDigits.equals(sidDigits)) { + logger.warn("AUDIT oauth_reset_denied uid={} reason=sid_mismatch", personToReset.getUid()); + return oauthResetDenied(HttpStatus.FORBIDDEN); + } + + String resetToken = ResetCode.GenerateResetCode(personToReset.getUid()); + if (resetToken == null) { + return oauthResetDenied(HttpStatus.TOO_MANY_REQUESTS); + } + + logger.info("AUDIT oauth_reset_verified uid={}", personToReset.getUid()); + + HttpHeaders responseHeaders = new HttpHeaders(); + responseHeaders.setContentType(MediaType.APPLICATION_JSON); + String body = "{\"verified\":true,\"resetToken\":\"" + resetToken + "\"}"; + return new ResponseEntity(body, responseHeaders, HttpStatus.OK); + } + + @Getter + public static class PersonOAuthResetCompleteBody { + private String uid; + private String resetToken; + private String newPassword; + } + + // Step 2: spends the single-use token issued by /reset/oauth/verify to actually set the + // new password. The token, not the client's earlier "verified" claim, is what's trusted here. + @PostMapping("/reset/oauth/complete") + public ResponseEntity resetPasswordOAuthComplete(@RequestBody PersonOAuthResetCompleteBody requestBody) { + if (requestBody == null || requestBody.getUid() == null || requestBody.getUid().isBlank()) { + return new ResponseEntity(HttpStatus.BAD_REQUEST); + } + + Person personToReset = repository.getByUid(requestBody.getUid()); + if (personToReset == null) { + return new ResponseEntity(HttpStatus.NO_CONTENT); + } + + if (requestBody.getNewPassword() == null || requestBody.getNewPassword().length() < 8) { + return new ResponseEntity(HttpStatus.BAD_REQUEST); + } + + if (!ResetCode.validateAndConsume(personToReset.getUid(), requestBody.getResetToken())) { + logger.warn("AUDIT oauth_reset_complete_denied uid={} reason=invalid_token", personToReset.getUid()); + return new ResponseEntity(HttpStatus.FORBIDDEN); + } + + personToReset.setPassword(requestBody.getNewPassword()); + repository.save(personToReset, false); + + logger.info("AUDIT oauth_reset_completed uid={}", personToReset.getUid()); + + // Best-effort sync to Flask so both backends' passwords stay in sync for this + // account; failure here doesn't roll back or fail the Spring-side reset above. + FlaskPasswordSync.syncPassword(personToReset.getUid(), requestBody.getNewPassword()); + + return new ResponseEntity(HttpStatus.OK); + } + /////////////////////////////////////////////////////////////////////////////////////////// /// "Cookie-Clicker" Post and Get mappings /// diff --git a/src/main/java/com/open/spring/security/JwtTokenUtil.java b/src/main/java/com/open/spring/security/JwtTokenUtil.java index d1a75d6e..1f709385 100644 --- a/src/main/java/com/open/spring/security/JwtTokenUtil.java +++ b/src/main/java/com/open/spring/security/JwtTokenUtil.java @@ -1,4 +1,5 @@ package com.open.spring.security; +import org.springframework.beans.factory.annotation.Autowired; import org.springframework.beans.factory.annotation.Value; import org.springframework.security.core.userdetails.UserDetails; import org.springframework.stereotype.Component; @@ -13,6 +14,9 @@ import io.jsonwebtoken.security.Keys; import javax.crypto.*; +import com.open.spring.mvc.person.Person; +import com.open.spring.mvc.person.PersonJpaRepository; + @Component public class JwtTokenUtil { @@ -22,6 +26,9 @@ public class JwtTokenUtil { @Value("${jwt.secret}") private String secret; + @Autowired + private PersonJpaRepository personJpaRepository; + private SecretKey getSecretKey() { byte[] ptsecret = Base64.getDecoder().decode(this.secret); SecretKey k = Keys.hmacShaKeyFor(ptsecret); @@ -55,15 +62,21 @@ private Boolean isTokenExpired(String token) { // Generate token for user public String generateToken(UserDetails userDetails, List roles) { - // Create a map to store JWT claims, + // Create a map to store JWT claims, // ... a claim is information asserted about the logged in user Map claims = new HashMap<>(); - + // Add a custom claim to the JWT token, - // ... roles are added for the client/frontend to know what the user can do, + // ... roles are added for the client/frontend to know what the user can do, // ... adding roles to login avoides an extra request to the server claims.put("roles", roles); - + + // Stamp the token with the person's current tokenVersion so a later password + // change (which bumps it) invalidates this token on the next validateToken call, + // instead of it staying valid for the rest of its JWT_TOKEN_VALIDITY lifetime. + Person person = personJpaRepository.findByUid(userDetails.getUsername()); + claims.put("tokenVersion", person != null && person.getTokenVersion() != null ? person.getTokenVersion() : 0L); + // Call doGenerateToken method to create the JWT token and set the standard claims // "sub" (subject) will be the username of the user. // "iat" (issued at) will be the current time. @@ -86,6 +99,17 @@ private String doGenerateToken(Map claims, String subject) { //validate token public Boolean validateToken(String token, UserDetails userDetails) { final String username = getUsernameFromToken(token); - return (username.equals(userDetails.getUsername()) && !isTokenExpired(token)); + if (!username.equals(userDetails.getUsername()) || isTokenExpired(token)) { + return false; + } + + Long tokenVersionClaim = getClaimFromToken(token, claims -> claims.get("tokenVersion", Long.class)); + long claimedVersion = tokenVersionClaim != null ? tokenVersionClaim : 0L; + + Person person = personJpaRepository.findByUid(username); + long currentVersion = (person != null && person.getTokenVersion() != null) ? person.getTokenVersion() : 0L; + + // A mismatch means the password changed since this token was issued. + return claimedVersion == currentVersion; } } \ No newline at end of file diff --git a/src/main/java/com/open/spring/security/MvcSecurityConfig.java b/src/main/java/com/open/spring/security/MvcSecurityConfig.java index aa1cef24..4d5aa061 100644 --- a/src/main/java/com/open/spring/security/MvcSecurityConfig.java +++ b/src/main/java/com/open/spring/security/MvcSecurityConfig.java @@ -77,6 +77,8 @@ public SecurityFilterChain mvcSecurityFilterChain(HttpSecurity http) throws Exce .requestMatchers(HttpMethod.GET, "/mvc/person/reset/check").permitAll() .requestMatchers(HttpMethod.POST, "/mvc/person/reset/start").permitAll() .requestMatchers(HttpMethod.POST, "/mvc/person/reset/check").permitAll() + .requestMatchers(HttpMethod.POST, "/mvc/person/reset/oauth/verify").permitAll() + .requestMatchers(HttpMethod.POST, "/mvc/person/reset/oauth/complete").permitAll() .requestMatchers("/mvc/person/read/**").authenticated() .requestMatchers("/mvc/person/cookie-clicker").authenticated() .requestMatchers(HttpMethod.GET,"/mvc/person/update/user").authenticated() @@ -191,6 +193,8 @@ public Map mvcEndpointRolePolicy() { policy.put("GET /mvc/person/reset/check", "permitAll"); policy.put("POST /mvc/person/reset/start", "permitAll"); policy.put("POST /mvc/person/reset/check", "permitAll"); + policy.put("POST /mvc/person/reset/oauth/verify", "permitAll"); + policy.put("POST /mvc/person/reset/oauth/complete", "permitAll"); policy.put("GET /mvc/person/update/user", "authenticated"); policy.put("POST /mvc/person/update", "authenticated (+ controller ownership checks)"); policy.put("POST /mvc/person/update/role", "ROLE_ADMIN"); diff --git a/src/main/resources/templates/login.html b/src/main/resources/templates/login.html index 2856d841..e7651dd9 100644 --- a/src/main/resources/templates/login.html +++ b/src/main/resources/templates/login.html @@ -48,8 +48,11 @@

Login To Account

Sign Up - - Forgot Password? + + Forgot Password? @@ -61,6 +64,11 @@

Login To Account

form.addEventListener("submit", (event) => { event.submitter.innerHTML = ""; }) + + const pagesOrigin = (location.hostname === "localhost" || location.hostname === "127.0.0.1") + ? "http://localhost:4000" + : "https://pages.opencodingsociety.com"; + document.getElementById("forgot-password-link").href = pagesOrigin + "/support?topic=reset"; From a23d2ab4f213f1eca59a7e70e803d3ceaa61bf79 Mon Sep 17 00:00:00 2001 From: rudrabjoshi Date: Fri, 28 Aug 2026 10:36:43 -0700 Subject: [PATCH 3/5] Enforce password complexity in the reset pipeline Hook Person.checkPassword() (8+ chars, upper/lower/digit/special) into PersonDetailsService.save(), guarded so unrelated profile edits that re-save an existing password hash aren't re-validated. Check complexity in /reset/oauth/complete before consuming the reset token. Fix two non-compliant default-password fallback literals that would otherwise break seed init and reset-to-default once complexity is enforced. Co-Authored-By: Claude Sonnet 5 --- .../com/open/spring/mvc/person/Person.java | 21 ++++++- .../mvc/person/PersonDetailsService.java | 15 ++++- .../mvc/person/PersonViewController.java | 6 +- .../com/open/spring/system/ModelInit.java | 2 +- .../spring/mvc/person/PasswordCheckTest.java | 56 +++++++++++++++++++ 5 files changed, 96 insertions(+), 4 deletions(-) create mode 100644 src/test/java/com/open/spring/mvc/person/PasswordCheckTest.java diff --git a/src/main/java/com/open/spring/mvc/person/Person.java b/src/main/java/com/open/spring/mvc/person/Person.java index c06a522b..522a5291 100644 --- a/src/main/java/com/open/spring/mvc/person/Person.java +++ b/src/main/java/com/open/spring/mvc/person/Person.java @@ -338,7 +338,9 @@ public static Person[] init() { ArrayList people = new ArrayList<>(); final Dotenv dotenv = Dotenv.load(); - String defaultPassword = envOrDefault(dotenv, "DEFAULT_PASSWORD", "defaultPassword123"); + // Must satisfy checkPassword()'s complexity rule -- seed init goes through + // PersonDetailsService.save() like everything else. + String defaultPassword = envOrDefault(dotenv, "DEFAULT_PASSWORD", "DefaultPassword123!"); // JSON-like list of person data using Map.ofEntries List> personData = Arrays.asList( @@ -512,4 +514,21 @@ public static List getAllSubmissions(Person person) { } return all; } + + // Complexity rule shared across the whole password-reset pipeline (this field, + // flask's model/user.py validate_password, and pages' getPasswordStrength in + // support.md) -- keep the required special-character set identical across all + // three so a password accepted by one backend is never rejected by the other. + public boolean checkPassword() { + if (password == null || password.length() < 8) { + return false; + } + + boolean hasUpper = password.matches(".*[A-Z].*"); + boolean hasLower = password.matches(".*[a-z].*"); + boolean hasNumber = password.matches(".*[0-9].*"); + boolean hasSpecial = password.matches(".*[`~!@#$%^&*()].*"); + + return hasUpper && hasLower && hasNumber && hasSpecial; + } } \ No newline at end of file diff --git a/src/main/java/com/open/spring/mvc/person/PersonDetailsService.java b/src/main/java/com/open/spring/mvc/person/PersonDetailsService.java index c10d33f2..749ec5ea 100644 --- a/src/main/java/com/open/spring/mvc/person/PersonDetailsService.java +++ b/src/main/java/com/open/spring/mvc/person/PersonDetailsService.java @@ -86,15 +86,28 @@ public void save(Person person) { if (person.getPassword() == null || person.getPassword().isEmpty()) { throw new IllegalArgumentException("Password cannot be null or empty"); } + // Always a plaintext password here (new-account creation / seed init), so + // this is the only place in this overload where complexity applies. + if (!person.checkPassword()) { + throw new IllegalArgumentException("Password does not meet complexity requirements"); + } person.setPassword(passwordEncoder.encode(person.getPassword())); personJpaRepository.save(person); } - + public void save(Person person, Boolean samePassword) { if (person.getPassword() == null) { // this will occur if ADMIN_PASSWORD and DEFAULT_PASSWORD are not set in .env throw new IllegalArgumentException("Password cannot be null"); } if (!samePassword) { + // Only check complexity when the password is actually changing -- when + // samePassword is true, person.getPassword() holds the existing BCrypt + // hash (re-saved unchanged for an unrelated profile edit), not a + // plaintext candidate, and checking a hash against these rules would + // fail unpredictably depending on its byte content. + if (!person.checkPassword()) { + throw new IllegalArgumentException("Password does not meet complexity requirements"); + } // Encode the password only if it's not the same as before person.setPassword(passwordEncoder.encode(person.getPassword())); // Invalidates every JWT already issued to this person -- see the field diff --git a/src/main/java/com/open/spring/mvc/person/PersonViewController.java b/src/main/java/com/open/spring/mvc/person/PersonViewController.java index 03316d82..a475cea7 100644 --- a/src/main/java/com/open/spring/mvc/person/PersonViewController.java +++ b/src/main/java/com/open/spring/mvc/person/PersonViewController.java @@ -618,7 +618,11 @@ public ResponseEntity resetPasswordOAuthComplete(@RequestBody PersonOAut return new ResponseEntity(HttpStatus.NO_CONTENT); } - if (requestBody.getNewPassword() == null || requestBody.getNewPassword().length() < 8) { + // Check complexity (same rule as PersonDetailsService.save) before consuming + // the single-use reset token, so a rejected password doesn't burn the token. + Person passwordCheck = new Person(); + passwordCheck.setPassword(requestBody.getNewPassword()); + if (!passwordCheck.checkPassword()) { return new ResponseEntity(HttpStatus.BAD_REQUEST); } diff --git a/src/main/java/com/open/spring/system/ModelInit.java b/src/main/java/com/open/spring/system/ModelInit.java index 0b45144b..f1c8b534 100644 --- a/src/main/java/com/open/spring/system/ModelInit.java +++ b/src/main/java/com/open/spring/system/ModelInit.java @@ -192,7 +192,7 @@ CommandLineRunner run() { // Ensure password is not null or empty if (person.getPassword() == null || person.getPassword().isEmpty()) { - person.setPassword("defaultPassword123"); // Set a default password or handle differently + person.setPassword("DefaultPassword123!"); // Must satisfy Person.checkPassword() } personDetailsService.save(person); diff --git a/src/test/java/com/open/spring/mvc/person/PasswordCheckTest.java b/src/test/java/com/open/spring/mvc/person/PasswordCheckTest.java new file mode 100644 index 00000000..29651ff6 --- /dev/null +++ b/src/test/java/com/open/spring/mvc/person/PasswordCheckTest.java @@ -0,0 +1,56 @@ +package com.open.spring.mvc.person; + +import org.junit.jupiter.api.Test; + +import static org.junit.jupiter.api.Assertions.*; + +public class PasswordCheckTest { + + @Test + public void testStrongPassword() { + Person person = new Person(); + person.setPassword("Password1!"); + + assertTrue(person.checkPassword()); + } + + @Test + public void testNoUppercase() { + Person person = new Person(); + person.setPassword("password1!"); + + assertFalse(person.checkPassword()); + } + + @Test + public void testNoLowercase() { + Person person = new Person(); + person.setPassword("PASSWORD1!"); + + assertFalse(person.checkPassword()); + } + + @Test + public void testNoNumber() { + Person person = new Person(); + person.setPassword("Password!"); + + assertFalse(person.checkPassword()); + } + + @Test + public void testNoSpecialCharacter() { + Person person = new Person(); + person.setPassword("Password1"); + + assertFalse(person.checkPassword()); + } + + @Test + public void testTooShort() { + Person person = new Person(); + person.setPassword("Pa1!"); + + assertFalse(person.checkPassword()); + } +} From 7c69943f40514253924095a8bd279f79a9195720 Mon Sep 17 00:00:00 2001 From: rudrabjoshi Date: Mon, 31 Aug 2026 10:44:54 -0700 Subject: [PATCH 4/5] Force-logout MVC sessions on OAuth password reset Addresses jm1021's review feedback on this PR/flask #74 ("tied to Profile Password reset") reinterpreted for the current architecture: the profile page's direct password field was removed in a later PR in the same stack in favor of routing everyone through this OAuth wizard, so "tied to profile reset" now means "tied to the wizard's completion". Track MVC logins in a SessionRegistry (tracking only, no session cap) and force-expire a uid's sessions after /reset/oauth/complete succeeds, closing the previously-documented gap where the JWT path invalidated on password change but the MVC HttpSession path didn't. Co-Authored-By: Claude Sonnet 5 --- .../mvc/person/PersonViewController.java | 30 +++++++++++++++++++ .../spring/security/MvcSecurityConfig.java | 29 +++++++++++++++++- 2 files changed, 58 insertions(+), 1 deletion(-) diff --git a/src/main/java/com/open/spring/mvc/person/PersonViewController.java b/src/main/java/com/open/spring/mvc/person/PersonViewController.java index a475cea7..0cb5a5ea 100644 --- a/src/main/java/com/open/spring/mvc/person/PersonViewController.java +++ b/src/main/java/com/open/spring/mvc/person/PersonViewController.java @@ -17,6 +17,8 @@ import org.springframework.beans.factory.annotation.Autowired; import org.springframework.security.core.Authentication; +import org.springframework.security.core.session.SessionInformation; +import org.springframework.security.core.session.SessionRegistry; import org.springframework.security.core.userdetails.UserDetails; import org.springframework.security.crypto.password.PasswordEncoder; import org.springframework.stereotype.Controller; @@ -53,6 +55,9 @@ public class PersonViewController { @Autowired private PasswordEncoder passwordEncoder; + @Autowired + private SessionRegistry sessionRegistry; + //@Autowired //private PersonJpaRepository find; @@ -640,9 +645,34 @@ public ResponseEntity resetPasswordOAuthComplete(@RequestBody PersonOAut // account; failure here doesn't roll back or fail the Spring-side reset above. FlaskPasswordSync.syncPassword(personToReset.getUid(), requestBody.getNewPassword()); + // Force-logout: kill any MVC HttpSession this uid currently holds (e.g. an admin + // portal tab logged in as this account elsewhere), closing the gap where password + // resets invalidated JWTs (tokenVersion) but not this session-based auth path. The + // requesting browser itself gets an explicit /logout call from the frontend on + // success (see support.md) since that's the only way to also clear its cookies. + invalidateActiveSessions(personToReset.getUid()); + return new ResponseEntity(HttpStatus.OK); } + // Marks every SessionRegistry-tracked HttpSession for this uid as expired. Expiry isn't + // instant server-side destruction -- ConcurrentSessionFilter enforces it the next time + // that session is used, which is sufficient: the account is unusable via the old session + // from that point on, matching what a real logout accomplishes. + private void invalidateActiveSessions(String uid) { + for (Object principal : sessionRegistry.getAllPrincipals()) { + String principalUid = (principal instanceof UserDetails userDetails) + ? userDetails.getUsername() + : String.valueOf(principal); + if (!uid.equals(principalUid)) { + continue; + } + for (SessionInformation sessionInformation : sessionRegistry.getAllSessions(principal, false)) { + sessionInformation.expireNow(); + } + } + } + /////////////////////////////////////////////////////////////////////////////////////////// /// "Cookie-Clicker" Post and Get mappings /// diff --git a/src/main/java/com/open/spring/security/MvcSecurityConfig.java b/src/main/java/com/open/spring/security/MvcSecurityConfig.java index 4d5aa061..1c835bf8 100644 --- a/src/main/java/com/open/spring/security/MvcSecurityConfig.java +++ b/src/main/java/com/open/spring/security/MvcSecurityConfig.java @@ -7,6 +7,7 @@ import org.springframework.beans.factory.annotation.Autowired; import org.springframework.beans.factory.annotation.Value; +import org.springframework.boot.web.servlet.ServletListenerRegistrationBean; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; import org.springframework.core.annotation.Order; @@ -17,8 +18,11 @@ import org.springframework.security.config.annotation.web.builders.HttpSecurity; import org.springframework.security.config.http.SessionCreationPolicy; import org.springframework.security.core.GrantedAuthority; +import org.springframework.security.core.session.SessionRegistry; +import org.springframework.security.core.session.SessionRegistryImpl; import org.springframework.security.core.userdetails.UserDetails; import org.springframework.security.web.SecurityFilterChain; +import org.springframework.security.web.session.HttpSessionEventPublisher; /* * MvcSecurityConfig.java @@ -56,6 +60,21 @@ public class MvcSecurityConfig { @Autowired private JwtTokenUtil jwtTokenUtil; + // Tracks every live MVC HttpSession by principal so a password reset can force-expire + // whatever session(s) that uid currently holds -- previously a known gap (the JWT path + // was covered via tokenVersion, but this form-login/session path was not). Registering + // HttpSessionEventPublisher is required for the registry to actually see session + // creation/destruction events. + @Bean + public SessionRegistry sessionRegistry() { + return new SessionRegistryImpl(); + } + + @Bean + public ServletListenerRegistrationBean httpSessionEventPublisher() { + return new ServletListenerRegistrationBean<>(new HttpSessionEventPublisher()); + } + /** * MVC security: form login, session-based. */ @@ -68,7 +87,15 @@ public SecurityFilterChain mvcSecurityFilterChain(HttpSecurity http) throws Exce .securityMatcher("/**") .cors(Customizer.withDefaults()) .csrf(csrf -> csrf.disable()) - .sessionManagement(session -> session.sessionCreationPolicy(SessionCreationPolicy.IF_REQUIRED)) + .sessionManagement(session -> session + .sessionCreationPolicy(SessionCreationPolicy.IF_REQUIRED) + // maximumSessions(-1) means no cap is enforced -- this exists purely to get + // every session registered in sessionRegistry() so it can be force-expired + // elsewhere (PersonViewController, after a password reset), not to limit + // concurrent logins. + .sessionConcurrency(concurrency -> concurrency + .sessionRegistry(sessionRegistry()) + .maximumSessions(-1))) .authorizeHttpRequests(auth -> auth .requestMatchers("/mvc/person/search/**").authenticated() .requestMatchers(HttpMethod.GET, "/mvc/person/create").permitAll() From ab25d214e04cfe6f9c52b7c3f96c9284ff4f81f2 Mon Sep 17 00:00:00 2001 From: rudrabjoshi Date: Mon, 31 Aug 2026 12:04:02 -0700 Subject: [PATCH 5/5] Fix /reset/oauth/verify enumeration and read.html rel attributes Route the unknown-uid (was 204) and admin/default-account (was 401) denial paths in /reset/oauth/verify through the same oauthResetDenied() helper as every other check, so they're no longer distinguishable from a real failed verification by status code or body shape. 429 (rate limit) still necessarily differs -- the frontend needs it to show "Request a Ticket Instead". Also add rel="noopener noreferrer" to this file's two target="_blank" links (unrelated pre-existing template code, general hygiene). Co-Authored-By: Claude Sonnet 5 --- .../open/spring/mvc/person/PersonViewController.java | 12 ++++++++---- src/main/resources/templates/person/read.html | 4 ++-- 2 files changed, 10 insertions(+), 6 deletions(-) diff --git a/src/main/java/com/open/spring/mvc/person/PersonViewController.java b/src/main/java/com/open/spring/mvc/person/PersonViewController.java index 0cb5a5ea..03ba3f04 100644 --- a/src/main/java/com/open/spring/mvc/person/PersonViewController.java +++ b/src/main/java/com/open/spring/mvc/person/PersonViewController.java @@ -542,21 +542,25 @@ public ResponseEntity resetPasswordOAuthVerify(@RequestBody PersonOAuthR Person personToReset = repository.getByUid(requestBody.getUid()); - //person not found + //person not found -- same {"verified":false}/403 shape as every other denial below, + //so an unknown uid can't be distinguished from a real one that failed verification if (personToReset == null) { - return new ResponseEntity(HttpStatus.NO_CONTENT); + logger.warn("AUDIT oauth_reset_denied uid={} reason=not_found", requestBody.getUid()); + return oauthResetDenied(HttpStatus.FORBIDDEN); } //don't allow people to reset the passwords of admins if (personToReset.getRoles().stream().anyMatch(role -> "ROLE_ADMIN".equals(role.getName()))) { - return new ResponseEntity(HttpStatus.UNAUTHORIZED); + logger.warn("AUDIT oauth_reset_denied uid={} reason=admin_account", personToReset.getUid()); + return oauthResetDenied(HttpStatus.FORBIDDEN); } //dont allow people to reset password of default users (such as toby) Person[] databasePersons = Person.init(); for (Person person : databasePersons) { if (person.getUid().equals(personToReset.getUid())) { - return new ResponseEntity(HttpStatus.UNAUTHORIZED); + logger.warn("AUDIT oauth_reset_denied uid={} reason=default_account", personToReset.getUid()); + return oauthResetDenied(HttpStatus.FORBIDDEN); } } diff --git a/src/main/resources/templates/person/read.html b/src/main/resources/templates/person/read.html index 3b63bce3..701e3e29 100644 --- a/src/main/resources/templates/person/read.html +++ b/src/main/resources/templates/person/read.html @@ -68,7 +68,7 @@

Person Viewer

Person ID - User UID Name @@ -79,7 +79,7 @@

Person Viewer

- +