Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
152 changes: 152 additions & 0 deletions scripts/inject_reset_tickets.py
Original file line number Diff line number Diff line change
@@ -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())
35 changes: 34 additions & 1 deletion src/main/java/com/open/spring/mvc/person/Email/ResetCode.java
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,33 @@ public class ResetCode {
private static final Map<String, ResetTokenRecord> activeTokensByUid = new ConcurrentHashMap<>();
private static final Map<String, Deque<Long>> resetRequestTimesByUid = new ConcurrentHashMap<>();
private static final Map<String, String> 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<String, Integer> 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<String, Deque<Long>> ticketRequestTimesByIp = new ConcurrentHashMap<>();

public static synchronized boolean canRequestTicket(String ip) {
long now = Instant.now().getEpochSecond();
Deque<Long> 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;
Expand Down Expand Up @@ -89,7 +113,8 @@ public static synchronized boolean canIssueResetCode(String uid) {
}

Deque<Long> 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;
}
Expand All @@ -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));
Expand Down
72 changes: 72 additions & 0 deletions src/main/java/com/open/spring/mvc/person/PersonViewController.java
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -51,6 +53,9 @@ public class PersonViewController {
@Autowired
private PasswordEncoder passwordEncoder;

@Autowired
private ResetTicketJpaRepository ticketRepository;

//@Autowired
//private PersonJpaRepository find;

Expand All @@ -71,6 +76,7 @@ public String person(Authentication authentication, Model model) {
if (isAdmin == true){
List<Person> 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
Expand Down Expand Up @@ -503,6 +509,72 @@ public ResponseEntity<Object> 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<Object> 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<Object> 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
///
Expand Down
55 changes: 55 additions & 0 deletions src/main/java/com/open/spring/mvc/person/ResetTicket.java
Original file line number Diff line number Diff line change
@@ -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);
}
}
Original file line number Diff line number Diff line change
@@ -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<ResetTicket, Long> {
List<ResetTicket> findByResolvedFalseOrderByIdDesc();
List<ResetTicket> findByUidAndResolvedFalse(String uid);
}
7 changes: 7 additions & 0 deletions src/main/java/com/open/spring/security/MvcSecurityConfig.java
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down Expand Up @@ -191,6 +196,8 @@ public Map<String, String> 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");
Expand Down
Loading