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

Return the login result even when the profile can't be fetched

Aperta
  • Qualcosa non va?
#189 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à
4/5
Tempo stimato
3-5 giorni
Idoneità per principianti
35/100
Tipo di issue
Funzionalità
Chiarezza
Abbastanza chiara
Stato di attività
Attiva
Stack tecnologico
fastapi, openapi, python
Ambito
api, authentication, backend, documentation, testing

Direzione di ricerca

Wait for maintainer approval, then read CONTRIBUTING.md and inspect PESUAcademy.authenticate in app/pesu.py plus RequestModel and ResponseModel. Review app/docs/authenticate.py and tests/unit/test_openapi_docs.py before running the relevant tests. Done means the approved optional behavior, unchanged default behavior, documentation, models, metrics, and failure-type tests are covered.

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

Descrizione

api enhancement question student profile

🔄 Updated for v5.0.0 (2026-10-08). Still open, and still needs a maintainer decision. 5.0.0 (#241) narrowed the problem: a single changed or missing profile field is now null rather than a failure, so only a structural change or PESU refusing the profile call fails the request. The background and the example message are updated to the new code.

💬 Needs a maintainer decision before implementation. This changes the API contract. Discuss the design below in the comments; once a maintainer approves it, the issue can be assigned.

Summary

When a caller asks for the profile and the profile step fails after a successful login, pesu-auth fails the whole request: a 502 if the page couldn't be fetched, and a 422 if it couldn't be parsed. The caller loses the one thing it most needs, which is confirmation that the credentials are valid.

Since 5.0.0, a single field PESU stops sending or sends in an unexpected shape no longer fails the request: it comes back as null. But if PESU Academy changes the profile response's structure, or stops serving it, every profile: true login fails until pesu-auth is fixed, and apps that use pesu-auth to sign students in are locked out completely, even though logging in still works.

Background

PESUAcademy.authenticate (app/pesu.py): the login succeeds, then _fetch_profile raises ProfileFetchError (502: PESU couldn't be reached, answered with a non-200, or declined to serve the profile, as it does for a non-student account) or ProfileParseError (422: the response's structure changed). Just before that, a login response without an access token raises UpstreamError (502). Any of these propagates out of authenticate. ResponseModel has no way to say "logged in, but no profile".

Proposed design (for discussion)

Add an optional request flag, keeping today's behaviour as the default so nothing breaks:

{ "username": "...", "password": "...", "profile": true, "profileRequired": false }
  • profileRequired: true (the default): unchanged, and profile failures fail the request.
  • profileRequired: false: a profile failure after a successful login returns 200 with status: true, no profile, and a new profileError object:
{
  "status": true,
  "message": "Login successful. Profile data could not be retrieved.",
  "timestamp": "...",
  "profileError": { "message": "Failed to parse the profile response from PESU Academy." }
}

Alternatives to weigh in the comments:

  1. Always degrade, with no flag. That's simpler, but it's a breaking change and needs a major version.
  2. Return a status other than 200 (for example 207). Unusual for JSON APIs, and clients would need to change.

Implementation plan (once approved)

  1. RequestModel: add profile_required: bool = True (camelCase alias profileRequired), with a description and an example.
  2. ResponseModel: add an optional profile_error sub-model (message only; never raw exception text or HTML).
  3. authenticate: when profile is true and profile_required is false, catch ProfileFetchError / ProfileParseError (and the missing-token UpstreamError, which only happens after a successful login), keep status: True, and set profileError from the exception's public message, never its log-only detail. Metrics: the existing profile_parse_errors_total still counts the parse failure; authentication_results_total{result="success"} is correct here.
  4. Docs: the README request and response tables, and app/docs/authenticate.py (a new example, which tests/unit/test_openapi_docs.py checks).
  5. Tests: both flags with each failure type, the default unchanged, the response validates against the models, and OpenAPI examples.

Acceptance criteria

  • A maintainer has approved a design in this issue
  • Callers can opt in to getting a login result without a profile; the default behaviour is unchanged
  • README, Swagger and tests cover the new fields

⚠️ 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.

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 #189 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
22h 4m
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.