fix(auth): make logout revoke, and keep the JWT out of the URL (#72)

Logging out only deleted the cookie. A refresh token copied out of the
browser stayed valid for its full seven days, so the endpoint did nothing
at all against the one person it was meant to stop.

A JWT cannot be withdrawn once issued, so each refresh token now carries the
`ver` it was minted under and `GitHubUser.token_version` is bumped on
logout. Cheaper than a denylist: nothing stored per token, nothing to
expire. It invalidates all of the user's sessions rather than just this
browser's, which for a tool holding GitHub and Claude credentials is the
behaviour someone clicking logout after losing a laptop expects. Logout is
authenticated now, because revocation has to know whose tokens to kill.

Tokens issued before the column existed carry no `ver` claim and are
rejected the same way, logging current users out once. That is the intent,
not a side effect.

Separately, the OAuth callback put the access token in the redirect URL,
where it landed in browser history, in the Referer of whatever the page
loaded next, and in every proxy log along the way -- for the credential that
authenticates the entire API. The redirect carries no token now: it sets the
httpOnly refresh cookie, and the frontend trades that for an access token
through the `refreshToken()` helper the API client already had for silent
renewal.

426 backend tests, 66 frontend.

Author: mariuspruvot

CLAUDE.md

@@ -132,7 +132,8 @@ Key additions for production: `ENVIRONMENT=production`, `ADMIN_PASSWORD`, `CORS_
 - **Coolify `--project-directory`**: Coolify sets `--project-directory` to the repo root, not the compose file location. Relative paths in the compose (`context`, `volumes`) must be relative to the repo root (`./apps/api`, not `../../apps/api`).
 - **Coolify domain persistence**: Domains set in the Coolify UI may be cleared on redeploy/reload. Verify after each deploy. If persistent issues, add Traefik labels directly in the compose.
 - **`.dockerignore` vs `pyproject.toml`**: `apps/api/.dockerignore` excludes `*.md` but `pyproject.toml` references `readme = "README.md"` — `!README.md` exception is required in `.dockerignore` or `uv sync` fails.
-- **OAuth callback dual flow**: `GET /api/v1/auth/github/callback` accepts both OAuth login (with `state` CSRF param) and GitHub App installation redirect (with `installation_id`, no `state`). The `state` parameter is optional.
+- **OAuth callback dual flow**: `GET /api/v1/auth/github/callback` accepts both OAuth login (with `state` CSRF param) and GitHub App installation redirect (with `installation_id`, no `state`). The `state` parameter is optional. It redirects with **no token in the URL** — it sets the httpOnly refresh cookie, and the frontend trades that for an access token via `POST /auth/refresh` (`refreshToken()` in `shared/api/client.ts`).
+- **Refresh tokens are revocable**: each carries a `ver` claim matching `GitHubUser.token_version`; logout bumps the column, which invalidates every outstanding token for that user. `POST /auth/logout` is authenticated because revocation needs to know whose tokens to kill.
 - **GitHub App PEM key**: must be stored as raw PEM (multi-line) in Coolify env vars, NOT base64-encoded. The code passes it directly to `jwt.encode()`.
 - **OAuth tokens must be single-line**: Claude OAuth tokens pasted with line breaks cause `Invalid bearer token` errors. Frontend should strip whitespace/newlines from token input.
 

apps/api/alembic/versions/5b500a24f018_add_token_version_to_github_users.py

@@ -0,0 +1,35 @@
+"""Add token_version to github_users.
+
+Refresh tokens are JWTs, so they cannot be withdrawn once issued. Each one
+now carries the version it was minted under, and logout bumps this column --
+invalidating every outstanding token for that user without storing anything
+per token or needing a job to expire it.
+
+Existing rows default to 0. Tokens issued before this migration carry no
+``ver`` claim and are rejected on their next refresh, logging current users
+out once. That is the intent rather than a side effect: the point of the
+change is that tokens minted under the old rules stop being honoured.
+
+Revision ID: 5b500a24f018
+Revises: c4d5e6f7a0b1
+Create Date: 2026-08-01
+
+"""
+
+from collections.abc import Sequence
+
+import sqlalchemy as sa
+from alembic import op
+
+revision: str = "5b500a24f018"
+down_revision: str | None = "c4d5e6f7a0b1"
+branch_labels: str | Sequence[str] | None = None
+depends_on: str | Sequence[str] | None = None
+
+
+def upgrade() -> None:
+    op.add_column("github_users", sa.Column("token_version", sa.Integer(), server_default="0", nullable=False))
+
+
+def downgrade() -> None:
+    op.drop_column("github_users", "token_version")

apps/api/src/helprs/modules/identity/models.py

@@ -1,6 +1,6 @@
 """User identity ORM models."""
 
-from sqlalchemy import BigInteger, String
+from sqlalchemy import BigInteger, Integer, String
 from sqlalchemy.orm import Mapped, mapped_column
 
 from helprs.core.database import Base
@@ -16,3 +16,8 @@ class GitHubUser(Base):
     email: Mapped[str | None] = mapped_column(String(255), nullable=True)
     avatar_url: Mapped[str | None] = mapped_column(String(512), nullable=True)
     github_access_token_enc: Mapped[str] = mapped_column(String(512), nullable=False)
+    # Bumped on logout to invalidate outstanding refresh tokens. A JWT cannot
+    # be withdrawn once issued, so the token carries the version it was minted
+    # under and refresh compares the two. Cheaper than a denylist: nothing to
+    # store per token and nothing to expire.
+    token_version: Mapped[int] = mapped_column(Integer, nullable=False, default=0, server_default="0")

apps/api/src/helprs/modules/identity/router.py

@@ -15,7 +15,12 @@
 from helprs.core.exceptions import UnauthorizedError
 from helprs.core.middleware import limiter
 from helprs.modules.identity.schemas import TokenResponse, UserResponse, UserStatsResponse
-from helprs.modules.identity.service import authenticate_with_code, get_user_stats, refresh_tokens
+from helprs.modules.identity.service import (
+    authenticate_with_code,
+    get_user_stats,
+    refresh_tokens,
+    revoke_refresh_tokens,
+)
 
 router = APIRouter(prefix="/auth", tags=["auth"])
 
@@ -94,7 +99,12 @@ async def github_callback(
 
     _, tokens = await authenticate_with_code(session, code, settings)
 
-    response = RedirectResponse(url=f"{settings.APP_BASE_URL}/auth/callback?access_token={tokens.access_token}")
+    # No token in the URL. A redirect target lands in browser history, in the
+    # Referer of whatever the page loads next, and in every proxy log on the
+    # way -- for a credential that authenticates the whole API. The frontend
+    # trades the httpOnly refresh cookie set below for an access token by
+    # calling POST /auth/refresh, which puts it in a response body instead.
+    response = RedirectResponse(url=f"{settings.APP_BASE_URL}/auth/callback")
     _set_refresh_cookie(response, tokens.refresh_token, settings)
     response.delete_cookie(_OAUTH_STATE_COOKIE)
     return response
@@ -139,8 +149,21 @@ async def get_me(request: Request, user: CurrentUser) -> UserResponse:
 
 @router.post("/logout")
 @limiter.limit("30/minute")
-async def logout(request: Request, settings: GetSettings) -> Response:
-    """Clear the refresh cookie."""
+async def logout(
+    request: Request,
+    session: DbSession,
+    settings: GetSettings,
+    user: CurrentUser,
+) -> Response:
+    """Revoke the user's refresh tokens and clear the cookie.
+
+    Authenticated on purpose: revocation needs to know whose tokens to
+    invalidate. Clearing the cookie alone left a copied refresh token valid
+    for its full lifetime, which made this endpoint a no-op against anyone
+    who had actually taken one.
+    """
+    await revoke_refresh_tokens(session, user)
+
     response = Response(content='{"status":"ok"}', media_type="application/json")
     # Attributes must match those used to set it, or the browser keeps it.
     response.delete_cookie(

apps/api/src/helprs/modules/identity/service.py

@@ -91,7 +91,9 @@ def create_token_pair(user: GitHubUser, settings: Settings) -> TokenPair:
             settings.SECRET_KEY.get_secret_value(),
         ),
         refresh_token=create_access_token(
-            {"sub": str(user.id), "type": "refresh"},
+            # ver pins the token to the user's current version, so logout can
+            # invalidate every outstanding one by bumping it.
+            {"sub": str(user.id), "type": "refresh", "ver": user.token_version},
             settings.SECRET_KEY.get_secret_value(),
             REFRESH_TOKEN_LIFETIME,
         ),
@@ -112,9 +114,29 @@ async def refresh_tokens(refresh_token: str, session: AsyncSession, settings: Se
     if not user:
         raise UnauthorizedError("User not found")
 
+    if payload.get("ver") != user.token_version:
+        # Issued before a logout. Tokens minted before this field existed have
+        # no "ver" claim and are rejected the same way, which is the intent:
+        # the point of adding it is that older tokens stop being honoured.
+        raise UnauthorizedError("Refresh token has been revoked")
+
     return create_token_pair(user, settings)
 
 
+async def revoke_refresh_tokens(session: AsyncSession, user: GitHubUser) -> None:
+    """Invalidate every refresh token outstanding for this user.
+
+    Logout previously only cleared the cookie, so a token copied out of the
+    browser stayed valid for its full lifetime -- a "log out" that logged
+    nobody out. Bumping the version invalidates all of the user's sessions,
+    not just this browser's: for a tool that holds GitHub and Claude
+    credentials, logging out everywhere is the behaviour worth having, and it
+    is what someone clicking logout after losing a laptop expects.
+    """
+    user.token_version += 1
+    await session.flush()
+
+
 def _parse_subject(subject: object) -> uuid_mod.UUID:
     """Read the ``sub`` claim as a UUID, rejecting anything else."""
     if not isinstance(subject, str):

apps/api/tests/modules/identity/test_router.py

@@ -114,7 +114,12 @@ def _client(*args, **kwargs):
             resp = await client.get(f"/api/v1/auth/github/callback?code=test_code&state={state}")
 
         assert resp.status_code == 307
-        assert "access_token=" in resp.headers["location"]
+        # The token is deliberately NOT in the URL: a redirect target lands in
+        # browser history, in the next request's Referer, and in proxy logs.
+        assert "access_token=" not in resp.headers["location"]
+        assert resp.headers["location"].endswith("/auth/callback")
+        # It is reachable instead by trading the httpOnly cookie set here.
+        assert "refresh_token=" in resp.headers.get("set-cookie", "")
         assert "refresh_token" in resp.headers.get("set-cookie", "")
 
     async def test_invalid_state(self, app_with_db):
@@ -160,7 +165,7 @@ async def test_valid_refresh(self, authed_client, app_with_db):
         settings = get_settings()
 
         refresh_token = create_access_token(
-            {"sub": str(user_id), "type": "refresh"},
+            {"sub": str(user_id), "type": "refresh", "ver": 0},
             settings.SECRET_KEY.get_secret_value(),
             timedelta(days=7),
         )
@@ -185,11 +190,23 @@ async def test_missing_refresh_cookie(self, app_with_db):
 
 
 class TestLogout:
-    async def test_clears_cookie(self, app_with_db):
+    async def test_requires_authentication(self, app_with_db):
+        """Revocation has to know whose tokens to invalidate."""
         async with AsyncClient(
             transport=ASGITransport(app=app_with_db),
             base_url="http://test",
         ) as client:
             resp = await client.post("/api/v1/auth/logout")
-            assert resp.status_code == 200
-            assert resp.json() == {"status": "ok"}
+            assert resp.status_code == 401
+
+    async def test_clears_cookie_and_revokes(self, app_with_db, authed_client):
+        client, user_id = authed_client
+
+        resp = await client.post("/api/v1/auth/logout")
+
+        assert resp.status_code == 200
+        assert resp.json() == {"status": "ok"}
+        async with app_with_db.state.session_factory() as session:
+            refreshed = await session.get(GitHubUser, user_id)
+        # Every refresh token minted under version 0 is now dead.
+        assert refreshed.token_version == 1

apps/api/tests/modules/identity/test_service.py

@@ -20,17 +20,19 @@
     create_token_pair,
     get_decrypted_github_token,
     refresh_tokens,
+    revoke_refresh_tokens,
     sync_user,
 )
 
 
 class StoredUser:
     """Stand-in for a GitHubUser row where no database is needed."""
 
-    def __init__(self, *, encrypted_token: str = "", user_id: uuid.UUID | None = None) -> None:
+    def __init__(self, *, encrypted_token: str = "", user_id: uuid.UUID | None = None, token_version: int = 0) -> None:
         self.id = user_id or uuid.uuid4()
         self.github_login = "testuser"
         self.github_access_token_enc = encrypted_token
+        self.token_version = token_version
 
 
 def _serve_oauth(monkeypatch, *, profile_id: int = 99999999, login: str = "newuser") -> None:
@@ -124,13 +126,15 @@ def test_access_and_refresh_differ_and_carry_the_right_claims(self, settings):
         assert access_claims["github_login"] == "testuser"
         assert "type" not in access_claims
         assert refresh_claims["type"] == "refresh"
+        # Pins the token to the user's version so logout can revoke it.
+        assert refresh_claims["ver"] == user.token_version
 
 
 class TestRefreshTokens:
     async def test_valid_refresh_returns_a_new_pair(self, db_session, settings, test_user):
         user, _ = test_user
         refresh_token = create_access_token(
-            {"sub": str(user.id), "type": "refresh"},
+            {"sub": str(user.id), "type": "refresh", "ver": user.token_version},
             settings.SECRET_KEY.get_secret_value(),
             timedelta(days=7),
         )
@@ -140,6 +144,44 @@ async def test_valid_refresh_returns_a_new_pair(self, db_session, settings, test
         assert isinstance(pair, TokenPair)
         assert decode_access_token(pair.access_token, settings.SECRET_KEY.get_secret_value())["sub"] == str(user.id)
 
+    async def test_a_token_issued_before_logout_is_rejected(self, db_session, settings, test_user):
+        """A refresh token cannot be withdrawn once issued, so logout bumps
+        the version it was minted under. Before this, clearing the cookie was
+        the whole of "logout" and a copied token stayed valid for a week."""
+        user, _ = test_user
+        refresh_token = create_access_token(
+            {"sub": str(user.id), "type": "refresh", "ver": user.token_version},
+            settings.SECRET_KEY.get_secret_value(),
+            timedelta(days=7),
+        )
+
+        await revoke_refresh_tokens(db_session, user)
+
+        with pytest.raises(UnauthorizedError, match="revoked"):
+            await refresh_tokens(refresh_token, db_session, settings)
+
+    async def test_a_token_with_no_version_claim_is_rejected(self, db_session, settings, test_user):
+        """Tokens minted before the field existed. Rejecting them logs current
+        users out once, which is the point of adding it."""
+        user, _ = test_user
+        legacy = create_access_token(
+            {"sub": str(user.id), "type": "refresh"},
+            settings.SECRET_KEY.get_secret_value(),
+            timedelta(days=7),
+        )
+
+        with pytest.raises(UnauthorizedError, match="revoked"):
+            await refresh_tokens(legacy, db_session, settings)
+
+    async def test_a_fresh_token_still_works_after_revocation(self, db_session, settings, test_user):
+        user, _ = test_user
+        await revoke_refresh_tokens(db_session, user)
+
+        reissued = create_token_pair(user, settings)
+
+        pair = await refresh_tokens(reissued.refresh_token, db_session, settings)
+        assert isinstance(pair, TokenPair)
+
     async def test_invalid_refresh_token(self, db_session, settings):
         with pytest.raises(UnauthorizedError, match="Invalid or expired"):
             await refresh_tokens("invalid_token", db_session, settings)