Repository navigation
fix: refresh deps for best-effort auth logout, /logout never 500s (closes #14) - #17
Merged
Merged
Conversation
…oses #14) campus-api-python eb1c876 (PR nyjc-computing/campus-api-python#58) makes AuthAPI.logout() treat remote login-session revocation as best-effort: on failure it logs a warning and clears the stored login session id instead of raising NotFoundError, so /logout always completes with a local logout + redirect. Lock refresh also picks up campus-suite weekly d0c2ce4 (redirect_uri enforcement, RFC 7009 token revocation, refresh-token grant, and campus#652 which makes PUBLIC_URL required — already set on Railway and documented in .env.example/README).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #14.
Root cause
GET /logout→ flask_campus →campus.auth.logout()→LoginSessions.Login.revoke()→DELETE /auth/v1/logins/<id>. The auth service's DELETE route (_verify_session_idincampus/auth/resources/login.py) requires the auth-service-side{provider}_session_idcookie of the requesting client and 404s with "No session ID in client" when it's absent. TheCampusclient keeps that cookie in a per-process in-memoryrequests.Sessionjar — since campus-admin runs 2 gunicorn workers (PR #12), the login POST and the logout DELETE can land on different workers, the cookie isn't there, the DELETE 404s, andraise_for_status()turned/logoutinto a 500 while the user stayed signed in.Fix
Upstream, in nyjc-computing/campus-api-python#58 (merged,
eb1c876):AuthAPI.logout()now treats remote revocation as best-effort — it logs a warning and clears the stored login session id instead of raising, so logout always completes locally with a redirect. The auth-service-side strictness is filed upstream as nyjc-computing/campus#692 (revocation of sessions created by the same client credentials should not depend on a volatile per-process cookie).This PR refreshes
poetry.lockto that commit.Verification (local, against dev auth service)
Campusclients (simulating two workers): worker 1 creates a real login session, worker 2 (empty cookie jar) logs out — the DELETE now returns 404 "No session ID in client", which is caught and logged; the local session key is cleared and the flow returns cleanly. Previously this raisedNotFoundError→ 500.GET /logoutwith no session → 302/.GET /logoutwith a stalelogins_login_id→ 302, key cleared, landing page renders signed-out state.ruff checkclean; app-factory smoke (create_app) passes.Drift note
The refresh also pulls campus-suite weekly
484a1bb → d0c2ce4(redirect_uri fail-closed enforcement, RFC 7009 token revocation, refresh-token grant, and campus#652 which now requiresPUBLIC_URL).PUBLIC_URLis already set on the Railway service and documented in.env.example/README, so no action needed.