Review follow-ups: PR #553 — fix: revoke JWTs on password change + logout-all (Gitea #795) #800

New Issue

2026-09-23T20:11:18Z

gambit-admin commented

2026-09-23 20:11:18 +00:00

Non-blocking findings from the auto code review of PR #553 (fix: revoke JWTs on password change + logout-all (Gitea #795)) — captured for batch cleanup rather than blocking the PR.
Commit df9d5ce · https://github.com/Gambit-Inc/gambit/pull/553

SHOULD-FIX:

backend/app/routes/auth.py:1090 (/change-password) — Only the new JS handles the re-issued token; nothing else does. Older mobile builds and any other open web tab or session keep the old, now-revoked token. Their next request gets 401 "Token revoked", and the user is signed out right after a successful password change. Nothing here says which mobile/web versions are affected or whether that is accepted. Worth noting in the PR and release notes, and treating the OTA as the mitigation for mobile.
backend/alembic/versions/0050_users_token_version.py:39 — The _has_column() guard calls sa.inspect(op.get_bind()), which fails in alembic upgrade --sql offline mode. Only matters if offline SQL generation is ever used for this repo. Low risk.
Deploy ordering (backend/app/models.py:413, migration 0050) — Staging refreshes from the image at merge. It boots on create_all only, so it will 500 on every User query until someone runs alembic upgrade head there. The migration docstring says this. It is an operational step that is easy to miss, and nothing enforces it. Production is protected by the migration guard, which halts the auto-promote until someone promotes manually.

NITS:
none

Filed from a Claude Code session

Non-blocking findings from the auto code review of **PR #553** (fix: revoke JWTs on password change + logout-all (Gitea #795)) — captured for batch cleanup rather than blocking the PR. Commit `df9d5ce` · https://github.com/Gambit-Inc/gambit/pull/553 SHOULD-FIX: - `backend/app/routes/auth.py:1090` (`/change-password`) — Only the new JS handles the re-issued token; nothing else does. Older mobile builds and any other open web tab or session keep the old, now-revoked token. Their next request gets 401 "Token revoked", and the user is signed out right after a successful password change. Nothing here says which mobile/web versions are affected or whether that is accepted. Worth noting in the PR and release notes, and treating the OTA as the mitigation for mobile. - `backend/alembic/versions/0050_users_token_version.py:39` — The `_has_column()` guard calls `sa.inspect(op.get_bind())`, which fails in `alembic upgrade --sql` offline mode. Only matters if offline SQL generation is ever used for this repo. Low risk. - Deploy ordering (`backend/app/models.py:413`, migration 0050) — Staging refreshes from the image at merge. It boots on `create_all` only, so it will 500 on every `User` query until someone runs `alembic upgrade head` there. The migration docstring says this. It is an operational step that is easy to miss, and nothing enforces it. Production is protected by the migration guard, which halts the auto-promote until someone promotes manually. NITS: none --- Filed from a Claude Code session

gambit-admin added the enhancement label 2026-09-23 20:11:18 +00:00

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: gambit/gambit#800