diff --git a/CHANGELOG.md b/CHANGELOG.md index 9a269335a..fad9ae450 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,23 @@ All notable changes to this project are documented in this file. The format is b ## [Unreleased] +### Added +- `tenants.contracts.UsageMeter`: an implementation-free usage-metering protocol next to + `EntitlementProvider` (`check`, `consume`, `record`, `refund`, `reserve`/`commit`/`release`, + `summary`) with `UsageOutcome`/`UsageReservation`/`UsageSummaryRow`, `UsageExceededError`, + an `UnlimitedUsageMeter` default on `app.state.tenants.usage` and `get_usage_meter(app)` (#616). +- `simple_module_hosting.csrf.rotate_csrf_token(request)` for principal changes that keep the + session (#615). + +### Security +- Sign-in now rotates the session: users' `on_after_login` (password, OAuth, accept-invite) + clears it and keeps only the pending post-login redirect, so a CSRF token (or any other + session state) issued before sign-in no longer survives it. Keycloak sign-in rotates the CSRF + token (#615). **Behaviour change:** session keys a module stored + before sign-in are dropped at sign-in. + `simple_module_hosting.session.preserve_across_sign_in(app, key)` lets a module keep browser-bound + gate state across it; site_lock uses it so the unlock marker survives sign-in. + ## [0.0.39] — 2026-10-24 ### Added diff --git a/docs/framework-conventions.md b/docs/framework-conventions.md index 1703508dc..b81dd5f84 100644 --- a/docs/framework-conventions.md +++ b/docs/framework-conventions.md @@ -493,6 +493,19 @@ Callers echo the token back as `X-CSRF-Token` on `POST`/`PUT`/`PATCH`/`DELETE`; safe methods are never checked, and apps without `SessionMiddleware` (bare test apps) are exempt. +Signing in through users (password, OAuth, invite acceptance or demo) clears +and rebuilds the signed-cookie session, preserving only the pending login +redirect and any key a module registered with +`simple_module_hosting.session.preserve_across_sign_in(app, key)` (browser-bound +gate state such as the site-lock unlock marker). Locale lives in a separate cookie. This discards the pre-login CSRF +token and cached principal; the next Inertia render supplies a fresh +`csrfToken` shared prop. Clients must read that prop again after sign-in. +Keycloak sign-in retains the session and calls +`rotate_csrf_token(request)` to discard its token (switching the active tenant +keeps the same principal and the same token). Use this public helper for +other principal changes that intentionally keep the session; clearing the +session already rotates the token. A stale `X-CSRF-Token` receives `403`. + ### Error responses (HTML vs JSON) 401, 403, 404, 419, 422, 429, 500 and 503 are content-negotiated. Requests diff --git a/docs/modules/tenants.md b/docs/modules/tenants.md index e342cc407..ad2994870 100644 --- a/docs/modules/tenants.md +++ b/docs/modules/tenants.md @@ -102,6 +102,42 @@ The module ships no billing, but a billing module needs nothing more from it: `TenantService.delete_tenant` — subscribe to clean up per-tenant state), `MembershipAdded`, `MembershipRemoved`, `InvitationCreated`. +### Usage metering + +`tenants.contracts` exports the runtime-checkable `UsageMeter` protocol and +frozen value objects `UsageOutcome`, `UsageReservation`, and `UsageSummaryRow`. +Consumers import these contracts instead of importing billing. A billing module +installs its implementation during startup with `app.state.tenants.usage = provider`. +`get_usage_meter(app)` returns that provider, or an `UnlimitedUsageMeter` when +tenants state is absent. The tenants services container also defaults to this +unlimited provider: every check and consumption is allowed with `limit=None` +and `used=0`, reservations are issued, accounting operations do nothing, and +summaries are empty. No billing storage or enforcement is included here. + +All protocol methods are async and use string tenant IDs and meter keys such as +`"jobs.concurrent"`: + +- `check(tenant_id, meter, quantity=1)` inspects capacity without side effects. +- `consume(tenant_id, meter, quantity=1, *, idempotency_key=None)` atomically + checks and increments usage. A rejection returns `allowed=False` and records + nothing; a successful outcome's `used` includes the consumed quantity. +- `record(tenant_id, meter, quantity, *, idempotency_key=None)` unconditionally + records usage after the fact; `refund` has the same signature and returns usage. +- `reserve(tenant_id, meter, quantity=1, *, ttl=None)` holds capacity and returns + a `UsageReservation`, or `None` if the limit would be exceeded. `ttl` is an + optional `timedelta`. Pass the reservation back to the same provider's + `commit(reservation)` to record it, or `release(reservation)` to return capacity. +- `summary(tenant_id)` returns a sequence of `UsageSummaryRow` objects with + `meter`, `used`, `limit`, and `resets_at`. + +`UsageOutcome` carries `meter`, `allowed`, `limit`, `used`, `requested`, and +`resets_at`. `remaining` is `None` for unlimited meters, otherwise +`max(0, limit - used)`. Reset and expiry timestamps may be `None` when absent. +Providers own persistence, period boundaries, idempotency and hold expiry. +`UsageExceededError(outcome)` carries a denied outcome for implementers that +choose to raise it and map it to HTTP 402; the protocol's `consume` returns a +decision rather than requiring an exception. + ## Models `tenants_tenant` (id = the value stored in every `tenant_id`, slug, name, diff --git a/framework/hosting/simple_module_hosting/csrf.py b/framework/hosting/simple_module_hosting/csrf.py index ee089f853..21d1df23d 100644 --- a/framework/hosting/simple_module_hosting/csrf.py +++ b/framework/hosting/simple_module_hosting/csrf.py @@ -19,7 +19,9 @@ This lifts the design the pagebuilder module shipped first (``pagebuilder/security.py``) into the framework, so every module shares -one header name and one token-discovery convention. +one header name and one token-discovery convention. Sign-in clears the users +session, rotating its token; principal changes that retain the session must +call ``rotate_csrf_token`` before rendering the next page. """ from __future__ import annotations @@ -28,7 +30,7 @@ from fastapi import HTTPException, Request -__all__ = ["CSRF_HEADER", "RequiresCsrf", "get_csrf_token"] +__all__ = ["CSRF_HEADER", "RequiresCsrf", "get_csrf_token", "rotate_csrf_token"] CSRF_HEADER = "X-CSRF-Token" _SESSION_KEY = "sm_csrf_token" @@ -61,6 +63,17 @@ def get_csrf_token(request: Request) -> str: return token +def rotate_csrf_token(request: Request) -> None: + """Discard the old token; the next page render mints a fresh one. + + Use when changing the principal while deliberately retaining the session. + Clearing the whole session already discards the token. No session is a no-op. + """ + session = request.scope.get("session") + if session is not None: + session.pop(_SESSION_KEY, None) + + class RequiresCsrf: """FastAPI dependency enforcing the CSRF header on unsafe methods. diff --git a/framework/hosting/simple_module_hosting/session.py b/framework/hosting/simple_module_hosting/session.py index e5bb559b2..8a199b6f5 100644 --- a/framework/hosting/simple_module_hosting/session.py +++ b/framework/hosting/simple_module_hosting/session.py @@ -72,6 +72,28 @@ which would leave the hole this key was added to close. """ +_SIGN_IN_SURVIVORS_ATTR = "sign_in_preserved_session_keys" + + +def preserve_across_sign_in(app: Any, key: str) -> None: + """Keep ``key`` when sign-in replaces the session (it is not user-bound state). + + For gate state that belongs to the browser rather than the principal, such + as the site-lock unlock marker: clearing it at sign-in would re-prompt the + visitor who just passed the gate. Call from ``register_settings``. + """ + keys = getattr(app.state, _SIGN_IN_SURVIVORS_ATTR, None) + if keys is None: + keys = set() + setattr(app.state, _SIGN_IN_SURVIVORS_ATTR, keys) + keys.add(key) + + +def sign_in_preserved_keys(app: Any) -> frozenset[str]: + """Keys this app registered through :func:`preserve_across_sign_in`.""" + return frozenset(getattr(app.state, _SIGN_IN_SURVIVORS_ATTR, ())) + + SESSION_COOKIE_NAME = "session" """The session cookie's name (Starlette's default, pinned so the OpenAPI export can cite it).""" diff --git a/framework/hosting/tests/test_hosting_csrf.py b/framework/hosting/tests/test_hosting_csrf.py index d45030f13..4f6f77368 100644 --- a/framework/hosting/tests/test_hosting_csrf.py +++ b/framework/hosting/tests/test_hosting_csrf.py @@ -96,3 +96,32 @@ async def test_enforcement_is_skipped_and_token_empty(self) -> None: async with httpx.AsyncClient(transport=transport, base_url="http://test") as c: assert (await c.get("/token")).json()["token"] == "" assert (await c.post("/things")).status_code == 200 + + +@pytest.mark.parametrize("with_session", [False, True]) +def test_rotation_discards_only_csrf_state(with_session): + from simple_module_hosting.csrf import rotate_csrf_token + + scope = {"type": "http"} + if with_session: + scope["session"] = {"user_id": "keep"} + request = Request(scope) + old = get_csrf_token(request) + rotate_csrf_token(request) + rotate_csrf_token(request) # safe even before another token has been minted + if with_session: + assert request.session == {"user_id": "keep"} + assert get_csrf_token(request) != old + else: + assert get_csrf_token(request) == "" + + +def test_sign_in_survivors_registry(): + from types import SimpleNamespace + + from simple_module_hosting.session import preserve_across_sign_in, sign_in_preserved_keys + + app = SimpleNamespace(state=SimpleNamespace()) + assert sign_in_preserved_keys(app) == frozenset() + preserve_across_sign_in(app, "gate_marker") + assert sign_in_preserved_keys(app) == {"gate_marker"} diff --git a/modules/keycloak/keycloak/endpoints/api.py b/modules/keycloak/keycloak/endpoints/api.py index 820a31e3f..22c543321 100644 --- a/modules/keycloak/keycloak/endpoints/api.py +++ b/modules/keycloak/keycloak/endpoints/api.py @@ -8,6 +8,7 @@ from fastapi import APIRouter, HTTPException, Request from simple_module_core.redirect_safety import SESSION_NEXT_KEY, safe_next_or_none +from simple_module_hosting.csrf import rotate_csrf_token from starlette.responses import RedirectResponse if TYPE_CHECKING: @@ -84,6 +85,7 @@ async def oidc_callback(request: Request): cache_id = await provider._upsert_user_cache(request, claims) user_ctx = provider._claims_to_user_context(claims, cache_id=cache_id) + rotate_csrf_token(request) request.session[_SESSION_USER_CTX] = user_ctx.to_session_dict() request.session[_SESSION_ID_TOKEN] = id_token diff --git a/modules/keycloak/tests/test_keycloak_csrf_rotation.py b/modules/keycloak/tests/test_keycloak_csrf_rotation.py new file mode 100644 index 000000000..c6e2ce9d6 --- /dev/null +++ b/modules/keycloak/tests/test_keycloak_csrf_rotation.py @@ -0,0 +1,46 @@ +"""A successful OIDC callback must discard the previous principal's CSRF token.""" + +from types import SimpleNamespace +from unittest.mock import AsyncMock + +from keycloak.endpoints import api +from simple_module_hosting.csrf import get_csrf_token +from starlette.requests import Request + + +async def test_oidc_callback_rotates_csrf(monkeypatch): + context = SimpleNamespace(to_session_dict=lambda: {"id": "new-user"}) + provider = SimpleNamespace( + _upsert_user_cache=AsyncMock(return_value="new-user"), + _claims_to_user_context=lambda claims, cache_id: context, + ) + app = SimpleNamespace( + state=SimpleNamespace( + keycloak=SimpleNamespace( + settings=SimpleNamespace(login_redirect_url="/dashboard/"), + jwks_cache=SimpleNamespace(validate_jwt=AsyncMock(return_value={"sub": "new"})), + ), + auth=SimpleNamespace(auth_provider=provider), + ) + ) + request = Request( + { + "type": "http", + "app": app, + "query_string": b"code=code&state=valid", + "session": {"keycloak_oidc_state": "valid", "next": "/deep-link/"}, + } + ) + request.url_for = lambda name: "https://test/callback" + monkeypatch.setattr( + api, + "_get_oidc_client", + lambda req: SimpleNamespace( + exchange_code=AsyncMock(return_value={"access_token": "access", "id_token": "id"}) + ), + ) + old = get_csrf_token(request) + response = await api.oidc_callback(request) + assert response.headers["location"] == "/deep-link/" + assert request.session["user_ctx"] == {"id": "new-user"} + assert get_csrf_token(request) != old diff --git a/modules/site_lock/site_lock/module.py b/modules/site_lock/site_lock/module.py index 8395aac1a..88cab113f 100644 --- a/modules/site_lock/site_lock/module.py +++ b/modules/site_lock/site_lock/module.py @@ -6,6 +6,7 @@ from simple_module_core.module import ModuleBase, ModuleMeta from simple_module_core.module_settings import register_module_settings +from simple_module_hosting.session import preserve_across_sign_in from site_lock import constants as c @@ -28,6 +29,8 @@ def register_settings(self, app: FastAPI) -> None: from site_lock.settings import SiteLockSettings from site_lock.state import SiteLockState + # Signing in replaces the session; the unlock marker must outlive it. + preserve_across_sign_in(app, c.SESSION_KEY) register_module_settings( app, c.MODULE_PACKAGE, diff --git a/modules/tenants/tenants/contracts/__init__.py b/modules/tenants/tenants/contracts/__init__.py index 6ec62af3e..d13268d33 100644 --- a/modules/tenants/tenants/contracts/__init__.py +++ b/modules/tenants/tenants/contracts/__init__.py @@ -6,10 +6,26 @@ UnlimitedEntitlements, ensure_within_limit, ) +from tenants.contracts.usage import ( + UnlimitedUsageMeter, + UsageExceededError, + UsageMeter, + UsageOutcome, + UsageReservation, + UsageSummaryRow, + get_usage_meter, +) __all__ = [ "EntitlementExceededError", "EntitlementProvider", "UnlimitedEntitlements", + "UnlimitedUsageMeter", + "UsageExceededError", + "UsageMeter", + "UsageOutcome", + "UsageReservation", + "UsageSummaryRow", "ensure_within_limit", + "get_usage_meter", ] diff --git a/modules/tenants/tenants/contracts/usage.py b/modules/tenants/tenants/contracts/usage.py new file mode 100644 index 000000000..5bafccc2d --- /dev/null +++ b/modules/tenants/tenants/contracts/usage.py @@ -0,0 +1,159 @@ +"""Usage metering seam: consumers depend on this contract, never on billing. + +A provider installs itself as ``app.state.tenants.usage``. Without one, +``UnlimitedUsageMeter`` allows everything and keeps no usage state. +""" + +from __future__ import annotations + +from collections.abc import Sequence +from dataclasses import dataclass +from datetime import UTC, datetime, timedelta +from typing import TYPE_CHECKING, Protocol, runtime_checkable +from uuid import uuid4 + +if TYPE_CHECKING: + from fastapi import FastAPI + + +@dataclass(frozen=True, slots=True) +class UsageOutcome: + """A usage decision; ``used`` is the total observed when returning. + + Successful consumption includes the consumed quantity in ``used``. + ``requested`` describes this operation and is not subtracted again from + ``remaining``. ``None`` for ``limit`` means unlimited; ``resets_at`` is + absent for meters without a scheduled reset. + """ + + meter: str + allowed: bool + limit: int | None + used: int + requested: int + resets_at: datetime | None + + @property + def remaining(self) -> int | None: + """Available capacity, floored at zero; ``None`` means unlimited.""" + return None if self.limit is None else max(0, self.limit - self.used) + + +@dataclass(frozen=True, slots=True) +class UsageReservation: + """An opaque hold owned by the provider that issued it.""" + + id: str + meter: str + quantity: int + expires_at: datetime | None + + +@dataclass(frozen=True, slots=True) +class UsageSummaryRow: + """One meter's current usage and limit.""" + + meter: str + used: int + limit: int | None + resets_at: datetime | None + + +class UsageExceededError(Exception): + """A rejected usage decision; implementers may map this to HTTP 402.""" + + def __init__(self, outcome: UsageOutcome) -> None: + super().__init__(f"Usage limit reached for '{outcome.meter}' ({outcome.limit})") + self.outcome = outcome + + +@runtime_checkable +class UsageMeter(Protocol): + """Async usage operations keyed by tenant and meter (e.g. ``jobs.concurrent``). + + Persistence, period boundaries, idempotency and reservation lifecycle belong + to the provider. Consumers pass reservations back to the issuing provider. + """ + + async def check(self, tenant_id: str, meter: str, quantity: int = 1) -> UsageOutcome: + """Check capacity without recording usage or reserving it.""" + ... + + async def consume( + self, tenant_id: str, meter: str, quantity: int = 1, *, idempotency_key: str | None = None + ) -> UsageOutcome: + """Atomically check and increment; a denial records nothing.""" + ... + + async def record( + self, tenant_id: str, meter: str, quantity: int, *, idempotency_key: str | None = None + ) -> None: + """Record after-the-fact usage unconditionally, even over the limit.""" + ... + + async def refund( + self, tenant_id: str, meter: str, quantity: int, *, idempotency_key: str | None = None + ) -> None: + """Return previously recorded usage.""" + ... + + async def reserve( + self, tenant_id: str, meter: str, quantity: int = 1, *, ttl: timedelta | None = None + ) -> UsageReservation | None: + """Atomically hold capacity, or return ``None`` when it would exceed.""" + ... + + async def commit(self, reservation: UsageReservation) -> None: + """Convert a hold into recorded usage.""" + ... + + async def release(self, reservation: UsageReservation) -> None: + """Return held capacity without recording usage.""" + ... + + async def summary(self, tenant_id: str) -> Sequence[UsageSummaryRow]: + """Return the tenant's per-meter usage totals.""" + ... + + +class UnlimitedUsageMeter: + """Default provider: unlimited capacity and no accounting or stored holds.""" + + async def check(self, tenant_id: str, meter: str, quantity: int = 1) -> UsageOutcome: + return UsageOutcome(meter, True, None, 0, quantity, None) + + async def consume( + self, tenant_id: str, meter: str, quantity: int = 1, *, idempotency_key: str | None = None + ) -> UsageOutcome: + return await self.check(tenant_id, meter, quantity) + + async def record( + self, tenant_id: str, meter: str, quantity: int, *, idempotency_key: str | None = None + ) -> None: + pass + + async def refund( + self, tenant_id: str, meter: str, quantity: int, *, idempotency_key: str | None = None + ) -> None: + pass + + async def reserve( + self, tenant_id: str, meter: str, quantity: int = 1, *, ttl: timedelta | None = None + ) -> UsageReservation: + expires_at = datetime.now(UTC) + ttl if ttl is not None else None + return UsageReservation(str(uuid4()), meter, quantity, expires_at) + + async def commit(self, reservation: UsageReservation) -> None: + pass + + async def release(self, reservation: UsageReservation) -> None: + pass + + async def summary(self, tenant_id: str) -> Sequence[UsageSummaryRow]: + return [] + + +def get_usage_meter(app: FastAPI) -> UsageMeter: + """The tenants usage provider, or an unlimited fallback when absent.""" + provider = getattr(getattr(app.state, "tenants", None), "usage", None) + return provider if provider is not None else UnlimitedUsageMeter() diff --git a/modules/tenants/tenants/services.py b/modules/tenants/tenants/services.py index e2c551afe..42ca76d43 100644 --- a/modules/tenants/tenants/services.py +++ b/modules/tenants/tenants/services.py @@ -5,6 +5,7 @@ from dataclasses import dataclass, field from tenants.contracts.entitlements import EntitlementProvider, UnlimitedEntitlements +from tenants.contracts.usage import UnlimitedUsageMeter, UsageMeter from tenants.settings import TenantsSettings @@ -14,7 +15,9 @@ class TenantsServices: ``entitlements`` is the billing seam: a billing module assigns its own :class:`EntitlementProvider` here during its ``on_startup``. + ``usage`` similarly accepts a billing module's :class:`UsageMeter`. """ settings: TenantsSettings entitlements: EntitlementProvider = field(default_factory=UnlimitedEntitlements) + usage: UsageMeter = field(default_factory=UnlimitedUsageMeter) diff --git a/modules/tenants/tests/test_tenants_usage_contract.py b/modules/tenants/tests/test_tenants_usage_contract.py new file mode 100644 index 000000000..66f673f5d --- /dev/null +++ b/modules/tenants/tests/test_tenants_usage_contract.py @@ -0,0 +1,154 @@ +"""Public usage contracts and the default, unlimited provider.""" + +from collections.abc import Sequence +from dataclasses import FrozenInstanceError +from datetime import UTC, datetime, timedelta + +import pytest +from fastapi import FastAPI +from tenants.contracts import ( + UnlimitedUsageMeter, + UsageExceededError, + UsageMeter, + UsageOutcome, + UsageReservation, + UsageSummaryRow, + get_usage_meter, +) +from tenants.module import TenantsModule +from tenants.services import TenantsServices +from tenants.settings import TenantsSettings + + +class FakeUsageMeter: + """An independent provider needs no inheritance from the contract.""" + + async def check(self, tenant_id: str, meter: str, quantity: int = 1) -> UsageOutcome: + return UsageOutcome(meter, False, 0, 0, quantity, None) + + async def consume( + self, tenant_id: str, meter: str, quantity: int = 1, *, idempotency_key: str | None = None + ) -> UsageOutcome: + return await self.check(tenant_id, meter, quantity) + + async def record( + self, tenant_id: str, meter: str, quantity: int, *, idempotency_key: str | None = None + ) -> None: + pass + + async def refund( + self, tenant_id: str, meter: str, quantity: int, *, idempotency_key: str | None = None + ) -> None: + pass + + async def reserve( + self, tenant_id: str, meter: str, quantity: int = 1, *, ttl: timedelta | None = None + ) -> UsageReservation | None: + return None + + async def commit(self, reservation: UsageReservation) -> None: + pass + + async def release(self, reservation: UsageReservation) -> None: + pass + + async def summary(self, tenant_id: str) -> Sequence[UsageSummaryRow]: + return [UsageSummaryRow("jobs.concurrent", 0, 0, None)] + + +def test_runtime_protocol_conformance(): + default: UsageMeter = UnlimitedUsageMeter() + fake: UsageMeter = FakeUsageMeter() + assert isinstance(default, UsageMeter) + assert isinstance(fake, UsageMeter) + assert not isinstance(object(), UsageMeter) + + +@pytest.mark.parametrize( + "limit,used,remaining", [(None, 9, None), (10, 3, 7), (0, 0, 0), (2, 3, 0)] +) +def test_outcome_remaining(limit, used, remaining): + outcome = UsageOutcome("jobs.concurrent", True, limit, used, 1, None) + assert outcome.remaining == remaining + + +@pytest.mark.parametrize( + "value", + [ + UsageOutcome("jobs.concurrent", True, None, 0, 1, None), + UsageReservation("hold-1", "jobs.concurrent", 1, None), + UsageSummaryRow("jobs.concurrent", 0, None, None), + ], +) +def test_value_objects_are_frozen(value): + with pytest.raises(FrozenInstanceError): + value.meter = "changed" + assert not hasattr(value, "__dict__") + + +def test_exceeded_error_carries_outcome(): + outcome = UsageOutcome("jobs.concurrent", False, 5, 5, 1, None) + error = UsageExceededError(outcome) + assert error.outcome is outcome + assert outcome.meter in str(error) + + +async def test_unlimited_operations(): + provider: UsageMeter = UnlimitedUsageMeter() + for quantity in (1, 100): + expected = UsageOutcome("jobs.concurrent", True, None, 0, quantity, None) + assert await provider.check("tenant-1", "jobs.concurrent", quantity) == expected + assert ( + await provider.consume( + "tenant-1", "jobs.concurrent", quantity, idempotency_key="consume-1" + ) + == expected + ) + assert ( + await provider.record("tenant-1", "jobs.concurrent", 4, idempotency_key="record-1") is None + ) + assert ( + await provider.refund("tenant-1", "jobs.concurrent", 2, idempotency_key="refund-1") is None + ) + assert await provider.summary("tenant-1") == [] + assert (await provider.check("tenant-1", "jobs.concurrent")).used == 0 + assert (await provider.consume("tenant-1", "jobs.concurrent")).requested == 1 + + +async def test_unlimited_reservations(): + provider = UnlimitedUsageMeter() + start = datetime.now(UTC) + reservation = await provider.reserve("tenant-1", "jobs.concurrent", 3, ttl=timedelta(minutes=5)) + end = datetime.now(UTC) + assert reservation.meter == "jobs.concurrent" + assert reservation.quantity == 3 + assert reservation.expires_at is not None + assert start + timedelta(minutes=5) <= reservation.expires_at <= end + timedelta(minutes=5) + other = await provider.reserve("tenant-1", "jobs.concurrent") + assert reservation.id and other.id and reservation.id != other.id + assert other.quantity == 1 + assert other.expires_at is None + assert await provider.commit(reservation) is None + assert await provider.release(other) is None + + +def test_lookup_without_tenants(): + assert isinstance(get_usage_meter(FastAPI()), UnlimitedUsageMeter) + + +def test_services_default_and_override(): + app = FastAPI() + services = TenantsServices(TenantsSettings()) + app.state.tenants = services + assert isinstance(services.usage, UnlimitedUsageMeter) + assert get_usage_meter(app) is services.usage + replacement = FakeUsageMeter() + services.usage = replacement + assert get_usage_meter(app) is replacement + + +def test_module_registers_usage(): + app = FastAPI() + TenantsModule().register_settings(app) + assert isinstance(app.state.tenants.usage, UnlimitedUsageMeter) + assert get_usage_meter(app) is app.state.tenants.usage diff --git a/modules/users/tests/test_users_login_session_rotation.py b/modules/users/tests/test_users_login_session_rotation.py new file mode 100644 index 000000000..b3a8c02f9 --- /dev/null +++ b/modules/users/tests/test_users_login_session_rotation.py @@ -0,0 +1,136 @@ +"""Signing in must discard pre-login state and refresh Inertia's CSRF prop.""" + +import pytest +from _users_oauth_support import install_fake_provider, oauth_callback, seed_password_user +from fastapi import Depends, Request +from simple_module_hosting.csrf import CSRF_HEADER, RequiresCsrf, get_csrf_token + +_INERTIA = {"X-Inertia": "true", "X-Inertia-Version": "1.0"} +_PROBE = "/api/users/auth/rotation-probe" + + +def _install_probe(app): + @app.get(_PROBE) + async def seed(request: Request): + request.session["untrusted"] = "discard me" + return {"token": get_csrf_token(request)} + + @app.post(_PROBE, dependencies=[Depends(RequiresCsrf())]) + async def mutation(): + return {"ok": True} + + @app.get(f"{_PROBE}/session") + async def session(request: Request): + return dict(request.session) + + +async def _assert_rotated(client, old): + # Reload the page through Inertia as Login.tsx's router.visit does. + page = await client.get("/users/login", headers=_INERTIA) + assert page.status_code == 200, page.text + new = page.json()["props"]["csrfToken"] + assert new != old + assert (await client.post(_PROBE, headers={CSRF_HEADER: old})).status_code == 403 + assert (await client.post(_PROBE, headers={CSRF_HEADER: new})).status_code == 200 + session = (await client.get(f"{_PROBE}/session")).json() + assert "untrusted" not in session + assert "session_version" in session + assert "expires_at" in session + assert "is_demo" not in session + return new + + +@pytest.mark.parametrize("method", ["password", "oauth", "invite"]) +async def test_sign_in_rotates_session_and_page_token(users_app, anon_client, method): + _install_probe(users_app) + old = (await anon_client.get(_PROBE)).json()["token"] + anon_client.cookies.set("locale", "en") + if method == "password": + response = await anon_client.post( + "/api/users/auth/login", + data={"username": "admin@example.com", "password": "AdminPass1!"}, + ) + assert response.status_code == 204, response.text + elif method == "oauth": + await seed_password_user(users_app, "oauth@example.com", is_verified=True) + install_fake_provider(users_app, "ms-rotation", "oauth@example.com") + # AuthMiddleware supplies the return target, not the test route. + await anon_client.get("/admin/users/?page=2", follow_redirects=False) + response = await oauth_callback(anon_client) + assert response.status_code == 303, response.text + assert response.headers["location"] == "/admin/users/?page=2" + else: + from users.db_adapter import UserDatabaseWithRoles + from users.manager import UserManager + from users.models import User + + await seed_password_user(users_app, "invite@example.com", is_verified=False) + async with users_app.state.sm.db.session_factory() as db: + adapter = UserDatabaseWithRoles(db, User) + user = await adapter.get_by_email("invite@example.com") + manager = UserManager(adapter, None, users_app.state.users.settings) + token = manager.mint_invite_token(user) + response = await anon_client.post( + "/api/users/auth/accept-invite", json={"token": token, "password": "NewSecurePass1!"} + ) + assert response.status_code == 204, response.text + await _assert_rotated(anon_client, old) + session = (await anon_client.get(f"{_PROBE}/session")).json() + assert "next" not in session + assert anon_client.cookies.get("locale") == "en" + + +async def test_switching_users_discards_previous_context(users_app, anon_client): + _install_probe(users_app) + await seed_password_user(users_app, "second@example.com", is_verified=True) + assert ( + await anon_client.post( + "/api/users/auth/login", + data={"username": "admin@example.com", "password": "AdminPass1!", "remember": "true"}, + ) + ).status_code == 204 + old = (await anon_client.get("/users/login", headers=_INERTIA)).json()["props"]["csrfToken"] + assert ( + await anon_client.post( + "/api/users/auth/login", + data={"username": "second@example.com", "password": "SecurePass1!"}, + ) + ).status_code == 204 + await _assert_rotated(anon_client, old) + session = (await anon_client.get(f"{_PROBE}/session")).json() + assert "remember" not in session + assert (await anon_client.get("/api/users/me")).json()["email"] == "second@example.com" + + +async def test_demo_sign_in_rotates_token_and_restamps_marker(demo_app, demo_client): + _install_probe(demo_app) + old = (await demo_client.get(_PROBE)).json()["token"] + assert (await demo_client.post("/api/users/auth/demo/user")).status_code == 204 + props = (await demo_client.get("/users/login", headers=_INERTIA)).json()["props"] + assert props["csrfToken"] != old + assert props["demo"]["active"] is True + session = (await demo_client.get(f"{_PROBE}/session")).json() + assert "untrusted" not in session + + +async def test_registered_gate_keys_survive_sign_in(users_app, anon_client): + from simple_module_hosting.session import preserve_across_sign_in + + preserve_across_sign_in(users_app, "gate_marker") + + @users_app.get(f"{_PROBE}/gate") + async def gate(request: Request): + request.session["gate_marker"] = "unlocked" + request.session["untrusted"] = "discard me" + return {} + + _install_probe(users_app) + await anon_client.get(f"{_PROBE}/gate") + response = await anon_client.post( + "/api/users/auth/login", + data={"username": "admin@example.com", "password": "AdminPass1!"}, + ) + assert response.status_code == 204, response.text + session = (await anon_client.get(f"{_PROBE}/session")).json() + assert session["gate_marker"] == "unlocked" + assert "untrusted" not in session diff --git a/modules/users/users/constants.py b/modules/users/users/constants.py index 5ddcbf1c4..1a621437b 100644 --- a/modules/users/users/constants.py +++ b/modules/users/users/constants.py @@ -2,6 +2,8 @@ import uuid +from simple_module_core.redirect_safety import SESSION_NEXT_KEY + ADMIN_ROLE_ID = uuid.UUID("00000000-0000-0000-0000-000000000001") USER_ROLE_ID = uuid.UUID("00000000-0000-0000-0000-000000000002") @@ -17,6 +19,10 @@ PERM_USERS_MANAGE = "users.manage" PERM_USERS_SELF_PROFILE = "users.self.profile" +# Only the pending redirect belongs to both the anonymous and signed-in session. +# Locale is a separate cookie; cached principal, OAuth and CSRF state must go. +LOGIN_SESSION_PRESERVED_KEYS = (SESSION_NEXT_KEY,) + # Session keys SESSION_USER_ID_KEY = "user_id" # Revocation stamp copied out of ``User.session_version`` at login. A session diff --git a/modules/users/users/manager.py b/modules/users/users/manager.py index ad893640d..a378c8b7d 100644 --- a/modules/users/users/manager.py +++ b/modules/users/users/manager.py @@ -11,11 +11,11 @@ from fastapi import Depends, Request from fastapi_users import BaseUserManager, UUIDIDMixin, exceptions from fastapi_users.jwt import generate_jwt -from simple_module_hosting.session import stamp_session_expiry +from simple_module_hosting.session import sign_in_preserved_keys, stamp_session_expiry from users.constants import ( + LOGIN_SESSION_PRESERVED_KEYS, OAUTH_REGISTRATION_REQUEST_FLAG, - SESSION_DEMO_KEY, SESSION_USER_ID_KEY, SESSION_VERSION_KEY, ) @@ -192,14 +192,16 @@ async def on_after_login( # explicitly. Password / accept-invite flows already set this in their # wrappers — re-assigning the same value here is a harmless no-op. if request is not None: + preserved = { + key: request.session[key] + for key in (*LOGIN_SESSION_PRESERVED_KEYS, *sign_in_preserved_keys(request.app)) + if key in request.session + } + # Signed-cookie sessions have no server-side id: replace their state + # on sign-in, including CSRF, cached principal and remember/demo flags. + request.session.clear() + request.session.update(preserved) request.session[SESSION_USER_ID_KEY] = str(user.id) - # Whoever signs in here is not the shared demo account: the demo - # endpoint re-stamps this *after* calling the hook, and every other - # path must clear a marker the browser is still carrying from an - # earlier demo visit. Left behind, ``DemoReadOnlyMiddleware`` would - # treat this real session as read-only — and refuse the very - # ``/api/users/auth/login`` that created it. - request.session.pop(SESSION_DEMO_KEY, None) # Stamp the session with the revocation counter it was minted # under. ``UsersAuthProvider`` refuses any session whose stamp has # fallen behind the account's, which is what makes "sign out