Make tokens revocable and move the refresh token out of localStorage (0.46.0)
F5 — A token was valid until it expired, full stop. Resetting a compromised account's password changed nothing for whoever held its tokens (up to 30 days for a refresh token), demoting or disabling an account only took effect once the same clock ran out, and logout was purely client-side. Every account now has a token_version, every token is minted carrying it, and every request compares the two. Bumping it is the revoke switch, pulled on the three changes that alter what an account may do: password, role, active flag. "Sign out everywhere" in the user menu bumps your own. Plain "Sign out" only drops the cookie, because signing out on your phone should not kill your desktop session. The refresh token left localStorage for an httpOnly cookie (SameSite=Lax, scoped to /api/auth), and the access token is now held in memory only. A successful XSS can still act inside the open page but can no longer walk off with 30 days of access. The cookie is marked Secure only when the request arrived over HTTPS — request.url.scheme is trustworthy since the F4 fix — so a plain-HTTP homelab keeps working. Any refresh token an older build left in localStorage is deleted on first load. Scripted clients that cannot hold a cookie can still ask for it in the body with ?in_body=true. F9 comes with it, as predicted: the WebSocket helpers read the role off the live user instead of the token's claim. /ws/exec is root-equivalent on the host, and a token minted while the account was an admin stayed syntactically valid after a demotion. The sharp edge was the migration, not the feature. _ensure_model_columns emits ADD COLUMN without a DEFAULT, so SQLite would have filled token_version with NULL on every existing install, every version check would have failed against it, and the upgrade would have locked out every user everywhere. The helper now renders NOT NULL DEFAULT <literal> for scalar defaults; test_schema_migration builds a genuinely old-shaped user table and asserts the backfill. The version comparison also tolerates NULL as 1, so a database migrated by some other route still works. Writing that test surfaced an undocumented precondition: _ensure_model_columns does nothing unless `models` has been imported, since SQLModel.metadata is empty until then. It holds in production because init_db imports first; now it says so. The authorization matrix did its job — adding two auth routes failed the suite until both were classified, which is exactly the review moment it exists for. 30 new tests (698 total). Upgrading signs everyone out once. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Dk43rmEeRfYi5wsLDbmfyG
This commit is contained in:
+115
-12
@@ -4,7 +4,7 @@ from __future__ import annotations
|
||||
import time
|
||||
from collections import defaultdict, deque
|
||||
|
||||
from fastapi import APIRouter, Depends, HTTPException, Request, status
|
||||
from fastapi import APIRouter, Depends, HTTPException, Request, Response, status
|
||||
from sqlmodel import Session, select
|
||||
|
||||
import auth as auth_mod
|
||||
@@ -18,6 +18,7 @@ from models.user import (
|
||||
UserRead,
|
||||
UserUpdate,
|
||||
)
|
||||
from config import settings
|
||||
from services import audit_service
|
||||
|
||||
router = APIRouter(prefix="/api/auth", tags=["auth"])
|
||||
@@ -41,10 +42,44 @@ def _check_rate_limit(ip: str) -> None:
|
||||
hits.append(now)
|
||||
|
||||
|
||||
def _tokens_for(user: User) -> TokenPair:
|
||||
#: The refresh cookie is scoped to the two endpoints that consume it, so it is
|
||||
#: not attached to every API call the way a "/" cookie would be.
|
||||
REFRESH_COOKIE = "stackpilot_refresh"
|
||||
REFRESH_COOKIE_PATH = "/api/auth"
|
||||
|
||||
|
||||
def _issue(
|
||||
user: User, response: Response, request: Request, in_body: bool = False
|
||||
) -> TokenPair:
|
||||
"""Mint a token pair, putting the refresh token in an httpOnly cookie.
|
||||
|
||||
Keeping the long-lived token out of JavaScript's reach means a successful
|
||||
XSS can no longer walk off with 30 days of access — it is limited to
|
||||
whatever it can do in the live page. The short-lived access token still
|
||||
goes to the client, which holds it in memory only.
|
||||
|
||||
``in_body`` returns it in the response as well, for scripted clients that
|
||||
have no cookie jar.
|
||||
"""
|
||||
refresh = auth_mod.create_refresh_token(user)
|
||||
response.set_cookie(
|
||||
REFRESH_COOKIE,
|
||||
refresh,
|
||||
max_age=settings.REFRESH_TOKEN_EXPIRE_DAYS * 24 * 3600,
|
||||
httponly=True,
|
||||
# Lax rather than Strict so following a link into StackPilot keeps you
|
||||
# signed in; the cookie is only ever read by same-site POSTs anyway.
|
||||
samesite="lax",
|
||||
# Only when the request actually arrived over TLS — marking it Secure on
|
||||
# a plain-HTTP homelab deployment would make the browser drop it and
|
||||
# nobody could stay signed in. request.url.scheme is trustworthy here
|
||||
# because uvicorn runs with --proxy-headers.
|
||||
secure=request.url.scheme == "https",
|
||||
path=REFRESH_COOKIE_PATH,
|
||||
)
|
||||
return TokenPair(
|
||||
access_token=auth_mod.create_access_token(user),
|
||||
refresh_token=auth_mod.create_refresh_token(user),
|
||||
refresh_token=refresh if in_body else None,
|
||||
)
|
||||
|
||||
|
||||
@@ -56,7 +91,11 @@ def needs_setup(session: Session = Depends(get_session)) -> dict:
|
||||
|
||||
@router.post("/setup", response_model=TokenPair)
|
||||
def setup(
|
||||
body: UserCreate, session: Session = Depends(get_session)
|
||||
body: UserCreate,
|
||||
request: Request,
|
||||
response: Response,
|
||||
in_body: bool = False,
|
||||
session: Session = Depends(get_session),
|
||||
) -> TokenPair:
|
||||
if auth_mod.users_exist(session):
|
||||
raise HTTPException(status_code=400, detail="Setup already completed")
|
||||
@@ -71,13 +110,15 @@ def setup(
|
||||
audit_service.record(
|
||||
session, user=user.username, action="user.setup", target=user.username
|
||||
)
|
||||
return _tokens_for(user)
|
||||
return _issue(user, response, request, in_body)
|
||||
|
||||
|
||||
@router.post("/login", response_model=TokenPair)
|
||||
def login(
|
||||
body: LoginRequest,
|
||||
request: Request,
|
||||
response: Response,
|
||||
in_body: bool = False,
|
||||
session: Session = Depends(get_session),
|
||||
) -> TokenPair:
|
||||
ip = request.client.host if request.client else "unknown"
|
||||
@@ -91,20 +132,71 @@ def login(
|
||||
audit_service.record(
|
||||
session, user=user.username, action="auth.login", target=user.username, ip=ip
|
||||
)
|
||||
return _tokens_for(user)
|
||||
return _issue(user, response, request, in_body)
|
||||
|
||||
|
||||
@router.post("/refresh", response_model=TokenPair)
|
||||
def refresh(
|
||||
body: RefreshRequest, session: Session = Depends(get_session)
|
||||
request: Request,
|
||||
response: Response,
|
||||
body: RefreshRequest | None = None,
|
||||
in_body: bool = False,
|
||||
session: Session = Depends(get_session),
|
||||
) -> TokenPair:
|
||||
payload = auth_mod.decode_token(body.refresh_token, "refresh")
|
||||
user = auth_mod.get_user(session, payload.get("sub", ""))
|
||||
if not user or not user.is_active:
|
||||
"""Exchange a refresh token for a fresh pair.
|
||||
|
||||
Reads the httpOnly cookie; a body is accepted as a fallback for clients
|
||||
that cannot hold one. The token is re-validated against the live user, so a
|
||||
password reset or a disabled account takes effect here too rather than at
|
||||
the end of the token's 30-day life.
|
||||
"""
|
||||
token = request.cookies.get(REFRESH_COOKIE) or (body.refresh_token if body else None)
|
||||
if not token:
|
||||
raise HTTPException(
|
||||
status_code=status.HTTP_401_UNAUTHORIZED, detail="Invalid refresh token"
|
||||
status_code=status.HTTP_401_UNAUTHORIZED, detail="No refresh token"
|
||||
)
|
||||
return _tokens_for(user)
|
||||
payload = auth_mod.decode_token(token, "refresh")
|
||||
user = auth_mod.resolve_token_user(session, payload)
|
||||
if not user:
|
||||
response.delete_cookie(REFRESH_COOKIE, path=REFRESH_COOKIE_PATH)
|
||||
raise HTTPException(
|
||||
status_code=status.HTTP_401_UNAUTHORIZED,
|
||||
detail="Session is no longer valid — sign in again",
|
||||
)
|
||||
return _issue(user, response, request, in_body)
|
||||
|
||||
|
||||
@router.post("/logout")
|
||||
def logout(request: Request, response: Response) -> dict:
|
||||
"""End the session on this device by dropping the refresh cookie.
|
||||
|
||||
Deliberately does not bump ``token_version``: signing out on your phone
|
||||
should not kill the session on your desktop. Use "sign out everywhere"
|
||||
for that. The access token is held in memory by the client and dies with
|
||||
the tab; it stays technically valid for the rest of its hour, which is why
|
||||
it is short-lived.
|
||||
"""
|
||||
response.delete_cookie(REFRESH_COOKIE, path=REFRESH_COOKIE_PATH)
|
||||
return {"ok": True}
|
||||
|
||||
|
||||
@router.post("/logout-everywhere")
|
||||
def logout_everywhere(
|
||||
request: Request,
|
||||
response: Response,
|
||||
session: Session = Depends(get_session),
|
||||
user: User = Depends(auth_mod.get_current_user),
|
||||
) -> dict:
|
||||
"""Revoke every token this account holds, on every device."""
|
||||
auth_mod.bump_token_version(user)
|
||||
session.add(user)
|
||||
session.commit()
|
||||
response.delete_cookie(REFRESH_COOKIE, path=REFRESH_COOKIE_PATH)
|
||||
audit_service.record(
|
||||
session, user=user.username, action="auth.logout_everywhere",
|
||||
target=user.username, ip=_ip(request),
|
||||
)
|
||||
return {"ok": True}
|
||||
|
||||
|
||||
@router.get("/me", response_model=UserRead)
|
||||
@@ -175,6 +267,15 @@ def update_user(
|
||||
).first()
|
||||
if not other_admins:
|
||||
raise HTTPException(status_code=400, detail="Cannot demote or disable the last active admin")
|
||||
# Any of these three changes what this account is allowed to do, so the
|
||||
# tokens it already holds must stop working. Without the bump a password
|
||||
# reset was cosmetic: whoever had the old tokens kept full access for up to
|
||||
# 30 days, and a demotion or a disable only took effect once they expired.
|
||||
authority_changed = (
|
||||
bool(body.password)
|
||||
or (body.role is not None and body.role != user.role)
|
||||
or (body.is_active is not None and body.is_active != user.is_active)
|
||||
)
|
||||
if body.password:
|
||||
user.hashed_password = auth_mod.hash_password(body.password)
|
||||
if body.role is not None:
|
||||
@@ -183,6 +284,8 @@ def update_user(
|
||||
user.role = body.role
|
||||
if body.is_active is not None:
|
||||
user.is_active = body.is_active
|
||||
if authority_changed:
|
||||
auth_mod.bump_token_version(user)
|
||||
session.add(user)
|
||||
session.commit()
|
||||
session.refresh(user)
|
||||
|
||||
+26
-14
@@ -13,7 +13,7 @@ from fastapi import APIRouter, Query, WebSocket, WebSocketDisconnect
|
||||
from jose import JWTError
|
||||
from sqlmodel import Session
|
||||
|
||||
from auth import decode_token
|
||||
from auth import decode_token, resolve_token_user
|
||||
from database import engine
|
||||
from models.agent import Agent
|
||||
from models.setting import EVENT_PULL_FAILED, EVENT_STACK_ERROR, EVENT_STACK_START
|
||||
@@ -30,14 +30,30 @@ logger = logging.getLogger("stackpilot.ws")
|
||||
router = APIRouter(tags=["ws"])
|
||||
|
||||
|
||||
def _user_for(token: str | None):
|
||||
"""The live user behind a socket's token, or None.
|
||||
|
||||
Resolves against the database rather than reading the role straight off the
|
||||
JWT: a socket can outlive a demotion, a disabled account or a password
|
||||
reset, and the exec endpoint below is root-equivalent on the host. Same
|
||||
check the HTTP routes make.
|
||||
"""
|
||||
if not token:
|
||||
return None
|
||||
try:
|
||||
payload = decode_token(token, "access")
|
||||
except (JWTError, Exception): # noqa: BLE001
|
||||
return None
|
||||
with Session(engine) as session:
|
||||
user = resolve_token_user(session, payload)
|
||||
if user:
|
||||
session.expunge(user)
|
||||
return user
|
||||
|
||||
|
||||
async def _authorize(websocket: WebSocket, token: str | None) -> bool:
|
||||
"""Validate the JWT supplied as a query param. Closes socket on failure."""
|
||||
if not token:
|
||||
await websocket.close(code=4401)
|
||||
return False
|
||||
try:
|
||||
decode_token(token, "access")
|
||||
except (JWTError, Exception): # noqa: BLE001
|
||||
if _user_for(token) is None:
|
||||
await websocket.close(code=4401)
|
||||
return False
|
||||
return True
|
||||
@@ -47,15 +63,11 @@ async def _authorize_admin(websocket: WebSocket, token: str | None) -> bool:
|
||||
"""Like _authorize but also requires the admin role (exec is root-equivalent).
|
||||
|
||||
Closes 4401 on a missing/invalid token, 4403 on a valid non-admin token."""
|
||||
if not token:
|
||||
user = _user_for(token)
|
||||
if user is None:
|
||||
await websocket.close(code=4401)
|
||||
return False
|
||||
try:
|
||||
payload = decode_token(token, "access")
|
||||
except (JWTError, Exception): # noqa: BLE001
|
||||
await websocket.close(code=4401)
|
||||
return False
|
||||
if payload.get("role") != "admin":
|
||||
if user.role != "admin":
|
||||
await websocket.close(code=4403)
|
||||
return False
|
||||
return True
|
||||
|
||||
Reference in New Issue
Block a user