Hacktoberfest 2026: le issue che i maintainer hanno segnato per ottobre, aperte e adatte ai principianti. Sfoglia le issue Hacktoberfest

Rate-limit /authenticate per client IP and per username

Aperta
  • Qualcosa non va?
#190 1 commento 0 reazioni 0 assegnatari Vedi su GitHub

I maintainer di solito rispondono entro 1 giorno

Nessuno ha ancora preso questa issue.

Valutazione

Difficoltà
5/5
Tempo stimato
Più di una settimana
Idoneità per principianti
30/100
Tipo di issue
Funzionalità
Chiarezza
Abbastanza chiara
Stato di attività
Attiva
Stack tecnologico
fastapi, python
Ambito
api, authentication, backend, security

Direzione di ricerca

Read CONTRIBUTING.md first, then inspect main() in app/app.py, the error pattern in app/metrics/auth.py, and the authentication documentation in app/docs/authenticate.py. Confirm Render's forwarded-IP behavior with a maintainer on staging before designing the limiter; done means bounded configurable limits, documented 429 responses, passing targeted and live tests, and no IP leakage in logs or metrics.

Scritto dal modello di indicizzazione a partire dal testo della issue.

Descrizione

api authentication enhancement

🔄 Updated for v5.0.0 (2026-10-08). Still open and unchanged in substance. Since 5.0.0 (#241) pesu-auth signs in through PESU Academy's mobile API, so there is no CSRF token to protect; a wrong password is still a 401, which the per-username limit counts. References to the CSRF token and to #181 (now closed) are updated, along with the live-test note.

⚠️ PES University students only. This change sits in the login path and has to be tested end to end against PESU Academy with real PESU credentials (see "Tests that need credentials" in CONTRIBUTING.md). Run the whole suite with your credentials in .env and paste the summary line into your PR. Every run makes real logins to PESU Academy, so run it when you need it, not in a loop.

Summary

/authenticate is public, anonymous and unlimited. Anyone can use pesu-auth to try password after password against any student's PESU account, hidden behind pesu-auth's IP address. That's a risk to students, and a risk to pesu-auth itself: if PESU Academy notices the abuse and blocks Render's IP, pesu-auth goes down for every app that uses it. Add rate limits per client IP and per username, returning 429 Too Many Requests with Retry-After.

Background

  • There is no rate limiting anywhere in app/.
  • pesu-auth can't see the real client IP yet. On Render, requests arrive through Render's proxy. uvicorn only trusts X-Forwarded-For from the addresses in forwarded_allow_ips, which defaults to the FORWARDED_ALLOW_IPS environment variable or 127.0.0.1. main() in app/app.py doesn't set it, so today request.client.host is Render's proxy address for every request. A per-IP limit would be one shared limit for all users.
  • Setting FORWARDED_ALLOW_IPS="*" makes uvicorn trust the header from anyone, and uvicorn may then take the leftmost address, which a client can forge by sending its own X-Forwarded-For. The trustworthy address is the one Render adds. Check which position that is and don't assume it.
  • pesu-auth runs one process on Render's free tier, so in-memory state is enough. Say so in comments; a shared store such as Redis would only be needed with several instances.

Proposed change

1. Get the real client IP (do this first, verified on staging with a maintainer)
  • Work out exactly what Render sends: deploy a temporary DEBUG log of the X-Forwarded-For chain to staging only (a maintainer has to deploy it). Send requests with and without a forged X-Forwarded-For, and record which entry is Render's. Remove the log afterwards; IP addresses are personal data and must not be logged in production.
  • Implement a small client_ip(request) helper that takes the address Render appended (the rightmost entry that isn't a trusted proxy, if that's what the experiment shows), and falls back to request.client.host when there's no header (local runs, Docker, tests). Document how to configure it for people running the Docker image behind their own proxy.
2. Rate limiter
  • A small in-memory limiter (sliding window or token bucket) in its own module, for example app/ratelimit.py, with an injectable clock for tests. It needs no new dependency. slowapi exists, but it renders its own error shape and would need to be adapted to {status, message, timestamp}; propose it only if you can justify it.
  • Per IP, on every /authenticate request: for example 20 per minute.
  • Per username (username.casefold()), counting failed logins (401) only: for example 5 per 15 minutes. This is what stops password guessing, even from many IPs.
  • Bounded memory: expire old entries, and cap the number of tracked keys (for example 10,000, evicting the oldest).
  • Limits come from environment variables with sensible defaults (read once at startup, like METRICS_TOKEN in app/metrics/auth.py), documented in .env.example and the README.
3. Response
  • A new PESUAcademyError subclass, RateLimitError: status 429, message "Too many requests. Please try again later.", header Retry-After: <seconds> (use headers= on the exception like MetricsAuthorizationError does).
  • Check the limits before calling PESU Academy. A rejected request must not make any upstream call.
  • Log the rejection at WARNING with the username (usernames are logged by design) and never the IP. No IP or username in metric labels; errors_total{type="RateLimitError"} already counts rejections.
  • Add a 429 row to the README's /authenticate responses table and an example to app/docs/authenticate.py.

Tests

  • The limiter: allows up to the limit, rejects after it, recovers after the window, expires and evicts keys (fake clock).
  • client_ip: takes the right entry, ignores a forged leftmost entry, falls back without the header.
  • /authenticate: 429 with Retry-After and the standard body; no upstream call made when limited; the per-username limit counts only 401s.
  • Existing tests aren't affected (configure high limits in the test fixtures).
  • The live suite passes.

Acceptance criteria

  • Client IPs are identified correctly on Render (evidence from staging in the PR), and a forged header can't bypass the limit
  • Both limits work, are configurable and are documented
  • Memory is bounded
  • No IP address in logs or metrics

Related

#183 (bulkhead and circuit breaker) is a related protection; keep the two consistent. #181 (serialising logins per username) was closed: PESU Academy's mobile API allows several sessions per account, so concurrent logins no longer collide, and this issue's per-username limit is the remaining protection for a single account.

How to contribute

  • Wait until you're assigned. Comment to ask for this issue, then wait for a maintainer to assign it to you before writing any code. PRs for unassigned issues are closed without review.
  • Read CONTRIBUTING.md first. It covers forking, setting up with uv, and running the tests.
  • Branch from dev in your fork, and open the PR against dev with a Conventional Commit title and Closes #190 in the description.
  • Bump the minor version in pyproject.toml and run uv lock (CI checks this). uv run pre-commit run --all-files must pass, and coverage must stay at 100%.
  • Update the README, app/docs/ and comments in the same PR as the behaviour they describe.
  • Never log, print, store or echo a password or token, including in tests, PR descriptions and screenshots.
Lingua principale
Python
Stelle
22
Fork
29
Merge medio
1g 3h
PR unite (30g)
19

Preparare l'ambiente

Come iniziare

  1. Leggi tutta la issue e poi la guida ai contributi del progetto.
  2. Commenta sulla issue per dire che te ne occupi tu — evita che due persone facciano lo stesso lavoro.
  3. Fai un fork del repository e lavora su un branch.
  4. Apri una pull request che faccia riferimento al numero della issue.

Altre issue di pesu-dev/auth

Tutte le issue di pesu-dev/auth

Issue simili

Altre issue su Python

Ricevi le nuove issue nella tua casella

Un breve riepilogo di issue GitHub adatte ai principianti.