fix(users): CSRF-protect browser auth requests; logout retires the browser session

CHANGELOG.md

@@ -15,6 +15,13 @@ All notable changes to this project are documented in this file. The format is b
 ## [Unreleased]
 
 ### Security
+- **Behaviour change:** local browser auth mutations now refuse `Sec-Fetch-Site: cross-site`
+  and require the session-bound CSRF token (no Origin/Referer equality check, so TLS proxies work), extending the shared token support (#504).
+  Native API clients without browser headers retain their existing workflow.
+- **Behaviour change:** local logout retires only the current browser session,
+  using the existing account-cache invalidation channel (#318). Migration
+  `e805a1b2c3d4` adds expiring session retirement records. Sessions issued before
+  the upgrade remain compatible until expiry or next sign-in.
 - **Audit hardening:** audit entries no longer record credential values.
   Redacted fields retain change indicators using `[redacted]`; credential
   primary-key identities are masked too. Migration `b8d4f2a6c901` redacts

docs/database/migrations.md

@@ -71,8 +71,8 @@ The history is **not** a single linear chain. It has several roots and several h
 
 | Head | Owning module | Label Alembic reports |
 |---|---|---|
-| `e731a1b2c3d4` | users (unique `lower(email)`, #731) | `api_tokens`, `background_tasks`, `file_storage`, `users` |
-| `a7c2e91d4b58` | audit_log | `audit_log` |
+| `e805a1b2c3d4` | users (browser session retirement) | `api_tokens`, `background_tasks`, `file_storage`, `users` |
+| `b8d4f2a6c901` | audit_log | `audit_log` |
 | `f1a9c3d7b2e4` | tenants | `tenants` |
 | `d263ea727663` | framework (`sm_lease`) | `leases` |
 | `f6a3b9d2e8c4` | keycloak | `keycloak` |

docs/modules/users.md

@@ -33,7 +33,7 @@ fires `on_after_verify` followed by `on_after_update`, then signs the user in.
 | Method + path | Body | Notes |
 |---|---|---|
 | `POST /api/users/auth/login` | `OAuth2PasswordRequestForm` | sets `sm_auth` cookie + `session["user_id"]`; `400 LOGIN_BAD_CREDENTIALS`; `429` + `Retry-After` while locked — see [Login lockout](#login-lockout) |
-| `POST /api/users/auth/logout` | — | clears the `sm_auth` cookie and the session |
+| `POST /api/users/auth/logout` | — | retires this browser session, clears the `sm_auth` cookie and the session |
 | `POST /api/users/auth/register` | `UserCreate` | gated by `users.allow_signup`; rate-limited |
 | `POST /api/users/auth/forgot-password` | `PasswordReset` | rate-limited |
 | `POST /api/users/auth/reset-password` | `{token, password}` | |
@@ -77,7 +77,7 @@ fires `on_after_verify` followed by `on_after_update`, then signs the user in.
 Public:
 
 - `GET /users/login` → `Users/Login` (shows dev-account buttons in dev)
-- `POST /users/logout` → 303 redirect, clears cookie
+- `POST /users/logout` → 303 redirect, retires this browser session and clears cookies
 - `GET /users/register` → `Users/Register` (404 if signup disabled)
 - `GET /users/forgot-password` → `Users/ForgotPassword`
 - `GET /users/reset-password` → `Users/ResetPassword`
@@ -547,6 +547,63 @@ In the admin UI, the user list and edit page show an **"External · SSO"** badge
 
 Anything else without a session redirects to `/users/login` (or returns 401 JSON for `/api/*` / bearer requests).
 
+## Browser auth requests and logout
+
+**Behaviour change:** anonymous auth pages issue a session-bound `csrfToken` prop.
+Browser mutations under `/api/users/auth/` (password sign-in, demo sign-in,
+registration, forgot/reset password, verification, invite acceptance and API
+logout) must echo it as `X-CSRF-Token`. The bearer-token routes
+(`/api/users/auth/token*`) set no cookie and need no token, so native clients
+and cross-origin clients allowed by `cors_allow_origins` can use them. The shared typed API
+client already does this, including sign-in after a password reset. Read the prop
+from `GET /users/login` through Inertia when building another browser client;
+retain its session cookie. Signing in rotates the token: read the next page's
+prop before another mutation. Missing or mismatched tokens return 403.
+
+Requests with `Origin`, `Referer` or `Sec-Fetch-Site` are treated as browser
+requests and must carry the session token. `Sec-Fetch-Site: cross-site` is refused
+even with a token. `Origin`/`Referer` are not compared with the request's own
+origin, so deployments behind a TLS-terminating proxy work without extra
+configuration; the token is what defeats login CSRF. A cross-site form always
+carries `Origin` in current browsers, so it cannot skip the token; a very old
+browser sending none of the three headers would be treated as a native client,
+which is why the session cookie stays `SameSite=Lax`. The Inertia
+`POST /users/logout` applies only the cross-site refusal and needs no token
+header, so the existing menu submission continues to work.
+
+Native JSON/form API clients without browser headers continue to work without
+first fetching an HTML page. If they explicitly send `X-CSRF-Token`, it is
+validated against their session too. This compatibility exception is why the
+browser guard combines source validation with the framework's `RequiresCsrf`
+primitive rather than requiring a token unconditionally. Browser clients must
+keep these auth pages private and uncached.
+
+The users OAuth, OIDC and Keycloak GET login starters only establish a login
+transaction and redirect to the identity provider. Their callbacks validate the
+session-bound state before signing in; OIDC and Keycloak additionally validate
+nonce and use PKCE. These redirects continue to accept external navigation.
+
+**Behaviour change:** both logout routes retire this browser's session ID
+server-side and revoke its `sm_auth` access-token row, then clear the cookies.
+Other devices stay signed in. Password, invite, demo and users OAuth sign-ins
+mint a fresh ID through the shared users sign-in hook. Migration
+`e805a1b2c3d4` adds `users_revoked_session`, indexed by account and expiry;
+logout lazily removes expired records. Records remain until the session's
+original absolute deadline, including the remembered window.
+
+Retired IDs share the account's session-version cache entry and
+`session_version_cache_ttl_seconds` (default 30 seconds). Logout publishes on the
+existing `users.session_version` invalidation channel **after commit**. A Redis
+invalidation transport propagates eviction to other workers; without one, or
+if a message is lost, their TTL bounds the delay. Warm requests add no database
+query. The existing availability policy still applies: a cache miss during a
+database outage preserves an already-cached principal until a successful read.
+
+Sessions issued before this upgrade have no ID. They continue to work until
+expiry or next sign-in; logout clears their browser cookies but cannot retire
+another copy individually. Password changes and “Sign out everywhere” still
+retire all sessions through the account version counter.
+
 ## Roles cache
 
 `roles_cache.py` keeps an in-memory list of `RoleSummary(id, name)` on `app.state.users.roles_cache`. Refreshed at startup and on demand via `refresh_roles_cache(app)`. When the role set changes (the setup wizard's first administrator creates the `admin` role), `publish_roles_changed(app)` announces it on the `users.roles` invalidation channel: each subscribed worker empties its cache and `get_roles_cache` refills it on the next read (other workers hear it where the Redis invalidation transport is installed). Because delivery is not guaranteed, the cache also expires on its own after `TTL_SECONDS` (300 s, stamped as `roles_cache_loaded_at` on the users state) and is refetched on the next read. Used by the admin UI to render role pickers without hitting the DB on every request.

framework/hosting/tests/test_hosting_csrf_shared_prop.py

@@ -47,13 +47,12 @@ async def _probe() -> dict[str, bool]:
 
 
 async def test_anonymous_page_has_no_token(client) -> None:
-    resp = await client.get("/users/login", headers=_INERTIA)
-    assert resp.status_code == 200, resp.text
+    # The shared prop never mints a token for anonymous pages; the error page
+    # renders through the same shared-prop pipeline. (Auth pages such as
+    # /users/login pass their own token deliberately, for the sign-in post.)
+    resp = await client.get("/no-such-page", headers=_INERTIA)
+    assert resp.json()["component"] == "Error", resp.text
     assert "csrfToken" not in resp.json()["props"]
-    # The full document of a page a module may mark cacheable stays token-free too.
-    doc = await client.get("/users/login", headers={"Accept": "text/html"})
-    assert doc.status_code == 200
-    assert "csrfToken" not in doc.text
 
 
 def test_helper_needs_a_signed_in_session() -> None:

host/migrations/versions/e805a1b2c3d4_users_browser_sessions.py

@@ -0,0 +1,34 @@
+"""Track retired local browser sessions.
+
+Revision ID: e805a1b2c3d4
+Revises: e731a1b2c3d4
+"""
+
+import sqlalchemy as sa
+from alembic import op
+from fastapi_users_db_sqlalchemy.generics import GUID
+from simple_module_db.types import AwareDateTime
+
+revision = "e805a1b2c3d4"
+down_revision = "e731a1b2c3d4"
+branch_labels = None
+depends_on = None
+
+_TABLE = "users_revoked_session"
+
+
+def upgrade() -> None:
+    op.create_table(
+        _TABLE,
+        sa.Column("session_id", sa.String(64), primary_key=True, nullable=False),
+        sa.Column(
+            "user_id", GUID(), sa.ForeignKey("users_user.id", ondelete="CASCADE"), nullable=False
+        ),
+        sa.Column("expires_at", AwareDateTime(), nullable=False),
+    )
+    op.create_index("ix_users_revoked_session_user_id", _TABLE, ["user_id"])
+    op.create_index("ix_users_revoked_session_expires_at", _TABLE, ["expires_at"])
+
+
+def downgrade() -> None:
+    op.drop_table(_TABLE)

host/tests/test_migration_graph.py

@@ -44,6 +44,7 @@
     "a625f0b7c912": "users",  # refresh-token families (#625)
     "a626b1c8d023": "users",  # hashed bearer tokens (#626)
     "e731a1b2c3d4": "users",  # unique lower(email) (#731)
+    "e805a1b2c3d4": "users",  # browser session retirement
     "51b2058d8afd": "api_tokens",
     "168a2882f443": "keycloak",
     "f6a3b9d2e8c4": "keycloak",
@@ -70,7 +71,7 @@
 # head revision -> module owning the tables it creates or alters.
 _HEADS = {
     "b8d4f2a6c901": "audit_log",
-    "e731a1b2c3d4": "users",  # also carries ``api_tokens``, ``background_tasks``, ``file_storage``
+    "e805a1b2c3d4": "users",  # also carries ``api_tokens``, ``background_tasks``, ``file_storage``
     "f1a9c3d7b2e4": "tenants",
     "f6a3b9d2e8c4": "keycloak",
     "c9d2e4f6a810": "oidc",
@@ -118,6 +119,7 @@
     "b31a70971001": "api_tokens",
     "f5b1c3e8a207": "api_tokens",
     "e731a1b2c3d4": "api_tokens",
+    "e805a1b2c3d4": "api_tokens",
     "a626b1c8d023": "api_tokens",  # also inherits ``background_tasks``
     "51b2058d8afd": "api_tokens",  # ancestors of a labelled revision inherit its label too
 }

modules/users/tests-js/users_Login.test.tsx

@@ -12,6 +12,7 @@ vi.mock('@inertiajs/react', () => ({
   usePage: () => ({ props: { branding: { appName: 'Acme', logoUrl: null } } }),
 }));
 
+import { setCsrfToken } from '@simple-module-py/ui/lib/api';
 import Login from '../users/pages/Login';
 import { json, sent, stubFetch } from './users_fixtures';
 
@@ -37,6 +38,7 @@ function submit() {
 }
 
 afterEach(() => {
+  setCsrfToken(undefined);
   vi.unstubAllGlobals();
   vi.clearAllMocks();
 });
@@ -59,13 +61,15 @@ describe('users Login', () => {
   });
 
   it('posts the form-encoded credentials and follows the server redirect on 204', async () => {
+    setCsrfToken('anonymous-session-token');
     const mock = stubFetch(() => new Response(null, { status: 204 }));
     render(<Login {...props} />);
     submit();
     await waitFor(() => expect(visit).toHaveBeenCalledWith('/dashboard'));
     const call = await sent(mock);
     expect(call.path).toBe('/api/users/auth/login');
     expect(call.method).toBe('POST');
+    expect(call.csrf).toBe('anonymous-session-token');
     expect(call.contentType).toContain('application/x-www-form-urlencoded');
     expect(call.credentials).toBe('same-origin');
     const form = new URLSearchParams(call.text);

modules/users/tests-js/users_fixtures.ts

@@ -18,6 +18,7 @@ export async function sent(mock: ReturnType<typeof stubFetch>, index = 0) {
     method: request.method,
     contentType: request.headers.get('Content-Type'),
     credentials: request.credentials,
+    csrf: request.headers.get('X-CSRF-Token'),
     text: await request.clone().text(),
   };
 }

modules/users/tests/test_session_version_hardening.py

@@ -238,5 +238,5 @@ def test_the_module_level_cache_is_the_one_the_helpers_use():
 
     configure_session_version_cache(7)
     store_session_version("dana", 1)
-    assert cache_module._SESSION_VERSIONS.get("dana") == 1
+    assert cache_module.peek_session_version("dana") == 1
     assert cache_module._SESSION_VERSIONS.ttl == 7

modules/users/tests/test_users_browser_auth_boundary.py

@@ -0,0 +1,108 @@
+"""Browser auth mutations use the anonymous session's token."""
+
+import pytest
+
+LOGIN = "/api/users/auth/login"
+CREDENTIALS = {"username": "admin@example.com", "password": "AdminPass1!"}
+_INERTIA = {"X-Inertia": "true", "X-Inertia-Version": "1.0"}
+
+
+async def _login_token(client) -> str:
+    return (await client.get("/users/login", headers=_INERTIA)).json()["props"]["csrfToken"]
+
+
+@pytest.mark.parametrize(
+    "path",
+    [
+        LOGIN,
+        "/api/users/auth/register",
+        "/api/users/auth/forgot-password",
+        "/api/users/auth/reset-password",
+        "/api/users/auth/request-verify-token",
+        "/api/users/auth/verify",
+        "/api/users/auth/accept-invite",
+        "/api/users/auth/demo/user",
+        "/api/users/auth/logout",
+    ],
+)
+async def test_browser_mutations_require_session_token(anon_client, path):
+    for headers in [
+        {"Origin": "https://other.example"},
+        {"Origin": "null"},
+        {"Referer": "https://other.example/form"},
+        {"Sec-Fetch-Site": "cross-site"},
+        {"Sec-Fetch-Site": "same-site"},
+        {"Origin": "http://testserver"},
+    ]:
+        response = await anon_client.post(path, headers=headers, data=CREDENTIALS)
+        assert response.status_code == 403, (headers, response.text)
+        assert "sm_auth=" not in response.headers.get("set-cookie", "")
+
+
+async def test_anonymous_page_token_allows_login_and_rotates(anon_client):
+    token = await _login_token(anon_client)
+    headers = {"Origin": "http://testserver", "X-CSRF-Token": token}
+    assert (await anon_client.post(LOGIN, data=CREDENTIALS, headers=headers)).status_code == 204
+    assert (await anon_client.post(LOGIN, data=CREDENTIALS, headers=headers)).status_code == 403
+
+
+async def test_cross_site_fetch_metadata_refused_even_with_token(anon_client):
+    token = await _login_token(anon_client)
+    response = await anon_client.post(
+        LOGIN,
+        data=CREDENTIALS,
+        headers={"Sec-Fetch-Site": "cross-site", "X-CSRF-Token": token},
+    )
+    assert response.status_code == 403
+
+
+async def test_proxied_origin_with_token_is_accepted(anon_client):
+    token = await _login_token(anon_client)
+    response = await anon_client.post(
+        LOGIN,
+        data=CREDENTIALS,
+        headers={"Origin": "https://app.example", "X-CSRF-Token": token},
+    )
+    assert response.status_code == 204
+
+
+async def test_non_browser_password_client_keeps_working(anon_client):
+    assert (await anon_client.post(LOGIN, data=CREDENTIALS)).status_code == 204
+
+
+async def test_view_logout_accepts_same_origin_inertia_post(anon_client):
+    assert (await anon_client.post(LOGIN, data=CREDENTIALS)).status_code == 204
+    refused = await anon_client.post("/users/logout", headers={"Sec-Fetch-Site": "cross-site"})
+    assert refused.status_code == 403
+    assert (
+        await anon_client.post("/users/logout", headers={"Origin": "http://testserver"})
+    ).status_code == 303
+
+
+@pytest.mark.parametrize(
+    "path",
+    [
+        "/users/login",
+        "/users/register",
+        "/users/forgot-password",
+        "/users/reset-password",
+        "/users/verify",
+        "/users/invite/accept",
+    ],
+)
+async def test_anonymous_auth_pages_supply_token(anon_client_signup, path):
+    response = await anon_client_signup.get(path, headers=_INERTIA)
+    assert response.status_code == 200
+    assert response.json()["props"]["csrfToken"]
+
+
+async def test_bearer_token_login_needs_no_session_token(anon_client):
+    """A cross-origin API client gets tokens in the body; no cookie, no CSRF."""
+    response = await anon_client.post(
+        "/api/users/auth/token",
+        headers={"Origin": "https://spa.example", "Sec-Fetch-Site": "cross-site"},
+        json={"email": CREDENTIALS["username"], "password": CREDENTIALS["password"]},
+    )
+    assert response.status_code == 200, response.text
+    assert response.json()["access_token"]
+    assert "set-cookie" not in response.headers

modules/users/tests/test_users_browser_fetch_metadata.py

@@ -0,0 +1,44 @@
+"""Fetch-metadata refusal and token validation at the browser boundary."""
+
+import pytest
+from fastapi import HTTPException, Request
+from simple_module_hosting.csrf import get_csrf_token
+from users.auth_local.browser_guard import require_browser_csrf
+
+
+@pytest.mark.parametrize(
+    ("headers", "allowed"),
+    [
+        ({"origin": "http://testserver:80"}, True),
+        ({"origin": "https://app.example"}, True),  # proxy: scheme/host differ from base_url
+        ({"referer": "http://testserver/login?next=%2F"}, True),
+        ({"sec-fetch-site": "cross-site"}, False),
+        ({"sec-fetch-site": "same-site"}, True),
+    ],
+)
+async def test_browser_source_with_valid_session_token(headers, allowed):
+    request = Request(
+        {
+            "type": "http",
+            "method": "POST",
+            "scheme": "http",
+            "path": "/api/users/auth/login",
+            "root_path": "",
+            "server": ("testserver", 80),
+            "session": {},
+            "headers": [(b"host", b"testserver")],
+        }
+    )
+    token = get_csrf_token(request)
+    request.scope["headers"].extend(
+        (name.encode(), value.encode())
+        for name, value in {**headers, "x-csrf-token": token}.items()
+    )
+    # Request.headers may already be cached by URL construction.
+    request = Request(request.scope)
+    if allowed:
+        await require_browser_csrf(request)
+    else:
+        with pytest.raises(HTTPException) as error:
+            await require_browser_csrf(request)
+        assert error.value.status_code == 403