diff --git a/scripts/inject_reset_tickets.py b/scripts/inject_reset_tickets.py new file mode 100644 index 00000000..fa1d84f6 --- /dev/null +++ b/scripts/inject_reset_tickets.py @@ -0,0 +1,152 @@ +#!/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 caller IP +(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 per run -- see rate limit note below)", + ) + 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 per caller IP 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() + + if len(args.uids) > 5: + print( + f"Note: {len(args.uids)} uids given, but the endpoint only allows 5 " + "ticket-creation requests per 15 min per caller IP -- the rest will " + "come back 429 in this same run.\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..82d6ca0c 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,33 @@ 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 and uid-idempotent (one open ticket per uid), so + // that alone doesn't stop a caller from paging through many *different* uids to spam the + // admin queue -- rate-limit by caller IP instead, separately from the uid-keyed limits + // above. + private static final long TICKET_RATE_WINDOW_SECONDS = 15 * 60; + private static final int MAX_TICKET_REQUESTS_PER_WINDOW = 5; + private static final Map> ticketRequestTimesByIp = new ConcurrentHashMap<>(); + + public static synchronized boolean canRequestTicket(String ip) { + long now = Instant.now().getEpochSecond(); + Deque requestTimes = ticketRequestTimesByIp.computeIfAbsent(ip, 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,7 +113,8 @@ public static synchronized boolean canIssueResetCode(String uid) { } Deque requestTimes = resetRequestTimesByUid.computeIfAbsent(uid, key -> new ArrayDeque<>()); - if (requestTimes.size() >= MAX_REQUESTS_PER_WINDOW) { + int allowedRequests = MAX_REQUESTS_PER_WINDOW + bonusAttemptsByUid.getOrDefault(uid, 0); + if (requestTimes.size() >= allowedRequests) { lastIssueReasonByUid.put(uid, "rate-limit"); return false; } @@ -102,6 +127,14 @@ 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, so the user can retry immediately. + 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..333cc69e 100644 --- a/src/main/java/com/open/spring/mvc/person/PersonViewController.java +++ b/src/main/java/com/open/spring/mvc/person/PersonViewController.java @@ -37,6 +37,8 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import jakarta.servlet.http.HttpServletRequest; + // Built using article: https://docs.spring.io/spring-framework/docs/3.2.x/spring-framework-reference/html/mvc.html // or similar: https://asbnotebook.com/2020/04/11/spring-boot-thymeleaf-form-validation-example/ @Controller @@ -51,6 +53,9 @@ public class PersonViewController { @Autowired private PasswordEncoder passwordEncoder; + @Autowired + private ResetTicketJpaRepository ticketRepository; + //@Autowired //private PersonJpaRepository find; @@ -71,6 +76,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 +509,72 @@ 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 and takes an arbitrary uid, so it's also rate-limited per caller IP + // (separately from the global RateLimitFilter) -- otherwise a caller could page through + // many different real uids and spam the admin's ticket queue without ever tripping the + // per-uid idempotency check above. + @PostMapping("/reset/ticket") + public ResponseEntity requestResetTicket(@RequestBody ResetTicketRequestBody requestBody, + HttpServletRequest servletRequest) { + if (requestBody == null || requestBody.getUid() == null || requestBody.getUid().isBlank()) { + return new ResponseEntity<>(HttpStatus.BAD_REQUEST); + } + + if (!ResetCode.canRequestTicket(servletRequest.getRemoteAddr())) { + return new ResponseEntity<>(HttpStatus.TOO_MANY_REQUESTS); + } + + Person personToReset = repository.getByUid(requestBody.getUid()); + if (personToReset == null) { + return new ResponseEntity<>(HttpStatus.NO_CONTENT); + } + + 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..aabd9805 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 + +
+
+
+
@@ -191,6 +224,26 @@ } + +