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/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; + } + } +} 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";