diff --git a/scripts/inject_reset_tickets.py b/scripts/inject_reset_tickets.py new file mode 100644 index 00000000..ae3891ae --- /dev/null +++ b/scripts/inject_reset_tickets.py @@ -0,0 +1,154 @@ +#!/usr/bin/env python3 +"""Create test reset tickets by calling the real POST /mvc/person/reset/ticket +endpoint, instead of hand-writing SQL against reset_ticket. + +Goes through the actual endpoint on purpose: it's idempotent per uid (won't +double-create), rate-limited to 5 requests / 15 min per uid +(ResetCode.canRequestTicket), and its schema (GenerationType.IDENTITY) has +already bitten one direct-SQL testing pass this session that never exercised +the endpoint itself -- see forgot-password-pipeline.md's "Ticket-creation +rate limiting" section for that story. Hitting the endpoint is what actually +proves the whole path (idempotency, rate limit, admin panel query) works, +not just that a row exists. + +Usage: + python3 scripts/inject_reset_tickets.py hop niko + python3 scripts/inject_reset_tickets.py --db-check hop + BASE_URL=http://localhost:8585 python3 scripts/inject_reset_tickets.py hop + +After running, open /mvc/person/read as an admin to see the "Password Reset +Tickets" panel, or use --db-check to confirm without a browser. +""" + +from __future__ import annotations + +import argparse +import os +import sqlite3 +import sys +from pathlib import Path +from urllib import request + +BASE_URL = os.getenv("BASE_URL", "http://localhost:8585") +PROJECT_ROOT = Path(__file__).resolve().parent.parent +DEFAULT_DB = PROJECT_ROOT / "volumes" / "sqlite.db" + + +def parse_args() -> argparse.Namespace: + parser = argparse.ArgumentParser( + description="Create test reset tickets via the real reset-ticket endpoint." + ) + parser.add_argument( + "uids", + nargs="+", + help="GitHub uid(s) to raise a reset ticket for (max 5 requests per uid per 15 min)", + ) + parser.add_argument( + "--db-check", + action="store_true", + help="After creating, print each uid's open-ticket row from the DB", + ) + parser.add_argument( + "--db", + default=str(DEFAULT_DB), + help=f"SQLite DB path for --db-check (default: {DEFAULT_DB})", + ) + return parser.parse_args() + + +class NoRedirectHandler(request.HTTPRedirectHandler): + """Turn a 3xx into a raised HTTPError instead of silently following it. + + This endpoint must be reachable with zero auth (see the security-config + comment in MvcSecurityConfig.java) -- if it's ever accidentally dropped + from permitAll again, Spring redirects an anonymous POST to /login (302) + instead of rejecting it, and urllib's default opener follows that + transparently and reports the login page's 200 as if the ticket had been + created. That exact bug shipped once already; this handler is what + would have caught it immediately instead of needing a manual curl -i. + """ + + def redirect_request(self, req, fp, code, msg, headers, newurl): + return None + + +OPENER = request.build_opener(NoRedirectHandler) + + +def create_ticket(uid: str) -> tuple[int, str]: + url = f"{BASE_URL}/mvc/person/reset/ticket" + body = ('{"uid":"%s"}' % uid).encode("utf-8") + req = request.Request( + url, data=body, method="POST", headers={"Content-Type": "application/json"} + ) + try: + with OPENER.open(req) as resp: + return resp.status, resp.read().decode("utf-8", errors="replace") + except Exception as exc: + status = getattr(exc, "code", 0) + body_bytes = exc.read() if hasattr(exc, "read") else b"" + return status, body_bytes.decode("utf-8", errors="replace") + + +STATUS_MEANING = { + 200: "created (or an open ticket already existed for this uid)", + 204: "no such uid -- person not found", + 400: "bad request -- uid missing/blank", + 302: "REDIRECTED TO LOGIN -- endpoint is requiring auth, nothing was created. " + "Check MvcSecurityConfig has POST /mvc/person/reset/ticket in permitAll().", + 429: "rate-limited: 5 ticket-creation requests / 15 min for this uid already used", +} + + +def print_db_check(db_path: Path, uids: list[str]) -> None: + if not db_path.exists(): + print(f"\n--db-check: database file not found: {db_path}") + return + + conn = sqlite3.connect(str(db_path)) + try: + cur = conn.cursor() + print(f"\n--db-check ({db_path}):") + for uid in uids: + cur.execute( + 'SELECT id, resolved, created_at FROM reset_ticket WHERE uid = ? ORDER BY id DESC LIMIT 1', + (uid,), + ) + row = cur.fetchone() + if row is None: + print(f" {uid}: no reset_ticket row found") + else: + ticket_id, resolved, created_at = row + state = "open" if not resolved else "resolved" + print(f" {uid}: ticket #{ticket_id} ({state}), created {created_at}") + finally: + conn.close() + + +def main() -> int: + args = parse_args() + + from collections import Counter + repeated = [uid for uid, count in Counter(args.uids).items() if count > 5] + if repeated: + print( + f"Note: {repeated} repeated more than 5 times, but the endpoint only " + "allows 5 ticket-creation requests per 15 min per uid -- the rest will " + "come back 429 in this same run. Different uids don't share a budget.\n" + ) + + for uid in args.uids: + status, body = create_ticket(uid) + meaning = STATUS_MEANING.get(status, "unexpected status") + print(f"{uid}: POST /mvc/person/reset/ticket -> {status} ({meaning})") + if status not in (200,) and body: + print(f" body: {body[:300]}") + + if args.db_check: + print_db_check(Path(args.db).expanduser().resolve(), args.uids) + + return 0 + + +if __name__ == "__main__": + sys.exit(main()) 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..2c5fd162 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 @@ -26,9 +26,36 @@ public class ResetCode { private static final Map activeTokensByUid = new ConcurrentHashMap<>(); private static final Map> resetRequestTimesByUid = new ConcurrentHashMap<>(); private static final Map lastIssueReasonByUid = new ConcurrentHashMap<>(); + // Bumped by an admin from the reset-ticket queue when a rate-limited user needs more + // attempts; each grant adds one batch on top of MAX_REQUESTS_PER_WINDOW. + private static final Map bonusAttemptsByUid = new ConcurrentHashMap<>(); private static final byte[] secret = loadSecret(); + // Ticket creation is unauthenticated, so this exists to stop one uid from being spammed + // with repeat requests -- it is NOT the security boundary against ticket spam in general. + // That boundary is the admin: a ticket does nothing on its own, it only grants bonus + // attempts once a human clicks "Grant" on it, so a burst of tickets for many different + // uids is queue noise for the admin to dismiss, not an actual bypass of anything. Keyed + // by uid (not caller IP) so testing/admin tooling running from one machine against many + // different uids in a short window doesn't trip this at all. + private static final long TICKET_RATE_WINDOW_SECONDS = 15 * 60; + private static final int MAX_TICKET_REQUESTS_PER_WINDOW = 5; + private static final Map> ticketRequestTimesByUid = new ConcurrentHashMap<>(); + + public static synchronized boolean canRequestTicket(String uid) { + long now = Instant.now().getEpochSecond(); + Deque requestTimes = ticketRequestTimesByUid.computeIfAbsent(uid, key -> new ArrayDeque<>()); + while (!requestTimes.isEmpty() && requestTimes.peekFirst() <= now - TICKET_RATE_WINDOW_SECONDS) { + requestTimes.removeFirst(); + } + if (requestTimes.size() >= MAX_TICKET_REQUESTS_PER_WINDOW) { + return false; + } + requestTimes.addLast(now); + return true; + } + private static class ResetTokenRecord { private final String token; private final long expiresAtEpoch; @@ -89,11 +116,24 @@ public static synchronized boolean canIssueResetCode(String uid) { } Deque requestTimes = resetRequestTimesByUid.computeIfAbsent(uid, key -> new ArrayDeque<>()); - if (requestTimes.size() >= MAX_REQUESTS_PER_WINDOW) { + int bonus = bonusAttemptsByUid.getOrDefault(uid, 0); + int allowedRequests = MAX_REQUESTS_PER_WINDOW + bonus; + if (requestTimes.size() >= allowedRequests) { lastIssueReasonByUid.put(uid, "rate-limit"); return false; } + // This request only succeeds because of a bonus grant if the base window is already + // exhausted -- consume one bonus attempt in that case, so a grant is a one-time batch + // that runs out, not a permanent raise of the per-window ceiling. + if (requestTimes.size() >= MAX_REQUESTS_PER_WINDOW && bonus > 0) { + if (bonus <= 1) { + bonusAttemptsByUid.remove(uid); + } else { + bonusAttemptsByUid.put(uid, bonus - 1); + } + } + lastIssueReasonByUid.remove(uid); return true; } @@ -102,6 +142,16 @@ public static String getLastIssueReason(String uid) { return lastIssueReasonByUid.get(uid); } + // Called by an admin resolving a reset ticket: lifts the rate limit by one batch of + // extraAttempts on top of the standard window. Each attempt drawn from this batch (i.e. + // each issuance beyond MAX_REQUESTS_PER_WINDOW) is consumed one at a time in + // canIssueResetCode, so this is a one-time allowance, not a permanent ceiling raise. + public static synchronized void grantBonusAttempts(String uid, int extraAttempts) { + bonusAttemptsByUid.merge(uid, extraAttempts, Integer::sum); + logger.info("AUDIT reset_bonus_attempts_granted uid={} extraAttempts={} totalBonus={}", + uid, extraAttempts, bonusAttemptsByUid.get(uid)); + } + public static synchronized String GenerateResetCode(String uid){ if (!canIssueResetCode(uid)) { logger.warn("AUDIT reset_token_issue_blocked uid={} reason={}", uid, getLastIssueReason(uid)); 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..97b1ba3a 100644 --- a/src/main/java/com/open/spring/mvc/person/PersonViewController.java +++ b/src/main/java/com/open/spring/mvc/person/PersonViewController.java @@ -51,6 +51,9 @@ public class PersonViewController { @Autowired private PasswordEncoder passwordEncoder; + @Autowired + private ResetTicketJpaRepository ticketRepository; + //@Autowired //private PersonJpaRepository find; @@ -71,6 +74,7 @@ public String person(Authentication authentication, Model model) { if (isAdmin == true){ List list = repository.listAll(); // Fetch all persons model.addAttribute("list", list); // Add the list to the model for the view + model.addAttribute("tickets", ticketRepository.findByResolvedFalseOrderByIdDesc()); } else { Person person = repository.getByUid(userDetails.getUsername()); // Fetch the person by email @@ -503,6 +507,74 @@ public ResponseEntity adminResetPassword(@PathVariable Long id, Authenti return new ResponseEntity<>(HttpStatus.OK); } + private static final int TICKET_GRANT_BATCH_SIZE = 5; + + @Getter + public static class ResetTicketRequestBody { + private String uid; + } + + // Raised by the frontend's reset wizard when a uid hits the reset rate limit, so an + // admin can step in from the person/read portal instead of the user waiting out the + // window. Idempotent: a uid with an existing open ticket won't get a second one. + // Unauthenticated, so it's also rate-limited per uid (separately from the global + // RateLimitFilter and from the uid-keyed reset-request limits above) -- this only + // guards against one uid being spammed with repeat requests. Admin approval, not this + // limiter, is what actually gates a bypass; see the comment on canRequestTicket. + @PostMapping("/reset/ticket") + public ResponseEntity requestResetTicket(@RequestBody ResetTicketRequestBody requestBody) { + if (requestBody == null || requestBody.getUid() == null || requestBody.getUid().isBlank()) { + return new ResponseEntity<>(HttpStatus.BAD_REQUEST); + } + + if (!ResetCode.canRequestTicket(requestBody.getUid())) { + return new ResponseEntity<>(HttpStatus.TOO_MANY_REQUESTS); + } + + Person personToReset = repository.getByUid(requestBody.getUid()); + if (personToReset == null) { + // Same 200 as a real ticket creation -- an unknown uid must not be + // distinguishable from a known one on this unauthenticated endpoint. + logger.warn("AUDIT reset_ticket_unknown_uid uid={}", requestBody.getUid()); + return new ResponseEntity<>(HttpStatus.OK); + } + + if (ticketRepository.findByUidAndResolvedFalse(personToReset.getUid()).isEmpty()) { + ticketRepository.save(new ResetTicket(personToReset.getUid(), personToReset.getName())); + logger.info("AUDIT reset_ticket_created uid={}", personToReset.getUid()); + } + + return new ResponseEntity<>(HttpStatus.OK); + } + + // Admin resolves a reset ticket from the portal: grants the uid one batch of extra + // reset attempts (lifting the rate limit) and closes the ticket. If the user still + // needs more attempts after that, they raise a new ticket. + @PostMapping("/reset/ticket/{id}/grant") + public ResponseEntity grantResetTicket(@PathVariable Long id, Authentication authentication) { + boolean isAdmin = authentication.getAuthorities().stream() + .anyMatch(authority -> "ROLE_ADMIN".equals(authority.getAuthority())); + if (!isAdmin) { + return new ResponseEntity<>(HttpStatus.FORBIDDEN); + } + + ResetTicket ticket = ticketRepository.findById(id).orElse(null); + if (ticket == null) { + return new ResponseEntity<>(HttpStatus.NOT_FOUND); + } + if (ticket.isResolved()) { + return new ResponseEntity<>(HttpStatus.OK); + } + + ResetCode.grantBonusAttempts(ticket.getUid(), TICKET_GRANT_BATCH_SIZE); + ticket.markResolved(TICKET_GRANT_BATCH_SIZE); + ticketRepository.save(ticket); + + logger.warn("AUDIT reset_ticket_granted admin={} target_uid={} batch={}", + authentication.getName(), ticket.getUid(), TICKET_GRANT_BATCH_SIZE); + return new ResponseEntity<>(HttpStatus.OK); + } + /////////////////////////////////////////////////////////////////////////////////////////// /// "Cookie-Clicker" Post and Get mappings /// diff --git a/src/main/java/com/open/spring/mvc/person/ResetTicket.java b/src/main/java/com/open/spring/mvc/person/ResetTicket.java new file mode 100644 index 00000000..90f7eb58 --- /dev/null +++ b/src/main/java/com/open/spring/mvc/person/ResetTicket.java @@ -0,0 +1,55 @@ +package com.open.spring.mvc.person; + +import java.time.LocalDateTime; +import java.time.format.DateTimeFormatter; + +import jakarta.persistence.Entity; +import jakarta.persistence.GeneratedValue; +import jakarta.persistence.GenerationType; +import jakarta.persistence.Id; +import jakarta.validation.constraints.NotNull; +import lombok.AllArgsConstructor; +import lombok.Data; +import lombok.NoArgsConstructor; + +// Raised by the frontend when a user hits the reset rate limit and asks for admin help +// instead. An admin resolves it from the person/read portal, which grants the uid a batch +// of extra reset attempts via ResetCode.grantBonusAttempts. +@Data +@NoArgsConstructor +@AllArgsConstructor +@Entity +public class ResetTicket { + @Id + @GeneratedValue(strategy = GenerationType.IDENTITY) + private Long id; + + @NotNull + private String uid; + + // Snapshot of the person's name at request time, so the ticket stays readable even if + // the account is later renamed or removed. + private String name; + + private boolean resolved = false; + + private String createdAt; + + private String resolvedAt; + + private int attemptsGranted = 0; + + private static final DateTimeFormatter FORMATTER = DateTimeFormatter.ofPattern("yyyy-MM-dd HH:mm:ss"); + + public ResetTicket(String uid, String name) { + this.uid = uid; + this.name = name; + this.createdAt = LocalDateTime.now().format(FORMATTER); + } + + public void markResolved(int attemptsGranted) { + this.resolved = true; + this.attemptsGranted = attemptsGranted; + this.resolvedAt = LocalDateTime.now().format(FORMATTER); + } +} diff --git a/src/main/java/com/open/spring/mvc/person/ResetTicketJpaRepository.java b/src/main/java/com/open/spring/mvc/person/ResetTicketJpaRepository.java new file mode 100644 index 00000000..84d111f8 --- /dev/null +++ b/src/main/java/com/open/spring/mvc/person/ResetTicketJpaRepository.java @@ -0,0 +1,10 @@ +package com.open.spring.mvc.person; + +import java.util.List; + +import org.springframework.data.jpa.repository.JpaRepository; + +public interface ResetTicketJpaRepository extends JpaRepository { + List findByResolvedFalseOrderByIdDesc(); + List findByUidAndResolvedFalse(String uid); +} diff --git a/src/main/java/com/open/spring/security/MvcSecurityConfig.java b/src/main/java/com/open/spring/security/MvcSecurityConfig.java index aa1cef24..d45816ea 100644 --- a/src/main/java/com/open/spring/security/MvcSecurityConfig.java +++ b/src/main/java/com/open/spring/security/MvcSecurityConfig.java @@ -77,6 +77,11 @@ 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() + // Must be public: raised by a user who's rate-limited and, by definition, + // not logged in. /reset/ticket/{id}/grant is deliberately NOT here -- it + // falls through to anyRequest().authenticated() + the controller's own + // ROLE_ADMIN check below, same as /mvc/person/reset/admin/{id}. + .requestMatchers(HttpMethod.POST, "/mvc/person/reset/ticket").permitAll() .requestMatchers("/mvc/person/read/**").authenticated() .requestMatchers("/mvc/person/cookie-clicker").authenticated() .requestMatchers(HttpMethod.GET,"/mvc/person/update/user").authenticated() @@ -191,6 +196,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/ticket", "permitAll"); + policy.put("POST /mvc/person/reset/ticket/{id}/grant", "authenticated + ROLE_ADMIN (controller check)"); 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/person/read.html b/src/main/resources/templates/person/read.html index 3b63bce3..7efdb166 100644 --- a/src/main/resources/templates/person/read.html +++ b/src/main/resources/templates/person/read.html @@ -34,6 +34,39 @@

Person Viewer

+ +
+
Password Reset Tickets
+
+ + + + + + + + + + + + + + + + + +
RequestedUIDNameAction
Requested At + User UID + Name + +
+
+
+
@@ -68,7 +101,7 @@

Person Viewer

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

Person Viewer

- + Profile PictureReset Password } + +