From 5ad4c3e89b6e981bef59dfbf74d4d256f52017c9 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 19 Sep 2026 13:57:02 +0000 Subject: [PATCH 1/3] feat(users): one-click demo account for showcase instances MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds `demo_mode` to the users module: turn it on and the sign-in card grows an "Explore the demo" button that signs a visitor in as a shared, pre-seeded account — no email, no password, no signup. Intended for hosting a public instance that shows the framework off. Three pieces, because a demo account is a *published* credential and the account alone would be a footgun: - `users.demo` resolves the configured account from settings and reconciles the row on every boot and every settings reload. Flipping `demo_role` from admin back to user demotes the existing row rather than leaving the grant behind. A blank `demo_password` seeds a random secret, so the account is reachable only through the button — the password never reaches the browser, unlike the dev quick-fill buttons, which paste real credentials and stay development-only. - `users.demo_guard.DemoReadOnlyMiddleware` refuses every unsafe HTTP method from a demo session while `demo_read_only` is on (the default), except signing out. It reads the session rather than `request.state.user`, so it needs nothing from AuthMiddleware and works wrapped outside it; matching on the user id as well as the session stamp catches someone who signed in with a published `demo_password` through the ordinary form. Inertia requests get the protocol's 409 + X-Inertia-Location rather than a raw 403 body, which Inertia would render as an error modal. - A `demo` shared prop drives `DemoBanner`, a standing bar in every app shell. Per session, not per install: an operator signed in to their own account on a demo instance is doing real work and is unaffected, by the banner and by the guard alike. `demo_role = "admin"` with `demo_read_only = false` is a writable public superuser. It is allowed — a database rebuilt on a timer is a legitimate reason — but logs `users.demo.writable_admin` at WARNING and is called out in the docs. Also extracts two self-contained decisions out of `UsersModule.on_startup` into `users.startup`; the hook was one line under the 300-line file cap. Claude-Session: https://claude.ai/code/session_015SeBCuMuwvpfANx9FHv4qn --- docs/modules/users.md | 73 ++++++ .../hosting/tests/test_middleware_order.py | 10 + modules/users/README.md | 22 ++ modules/users/tests/_demo_support.py | 32 +++ modules/users/tests/conftest.py | 2 +- modules/users/tests/test_demo_account.py | 238 ++++++++++++++++++ modules/users/tests/test_demo_guard.py | 117 +++++++++ .../users/tests/test_users_shared_props.py | 73 +++++- .../auth_local/components/DemoSignIn.tsx | 50 ++++ modules/users/users/auth_local/demo_api.py | 79 ++++++ modules/users/users/auth_local/views.py | 16 ++ modules/users/users/demo.py | 190 ++++++++++++++ modules/users/users/demo_guard.py | 107 ++++++++ modules/users/users/locales/en.json | 8 +- modules/users/users/module.py | 65 +++-- modules/users/users/pages/Login.tsx | 53 +++- modules/users/users/settings.py | 54 ++++ modules/users/users/shared_props.py | 20 +- modules/users/users/startup.py | 67 +++++ modules/users/users/state.py | 6 + packages/i18n/src/generated-resources.ts | 8 + packages/i18n/src/keys.generated.ts | 10 + packages/ui/locales/en.json | 4 + packages/ui/locales/es.json | 4 + packages/ui/src/components/DemoBanner.tsx | 33 +++ packages/ui/src/layouts/SidebarLayout.tsx | 2 + packages/ui/src/types.ts | 5 + 27 files changed, 1294 insertions(+), 54 deletions(-) create mode 100644 modules/users/tests/_demo_support.py create mode 100644 modules/users/tests/test_demo_account.py create mode 100644 modules/users/tests/test_demo_guard.py create mode 100644 modules/users/users/auth_local/components/DemoSignIn.tsx create mode 100644 modules/users/users/auth_local/demo_api.py create mode 100644 modules/users/users/demo.py create mode 100644 modules/users/users/demo_guard.py create mode 100644 modules/users/users/startup.py create mode 100644 packages/ui/src/components/DemoBanner.tsx diff --git a/docs/modules/users.md b/docs/modules/users.md index 2c5539d3..c95dd9f8 100644 --- a/docs/modules/users.md +++ b/docs/modules/users.md @@ -33,6 +33,7 @@ The module is built on [`fastapi-users`](https://fastapi-users.github.io/) for p | `POST /api/users/auth/request-verify-token` | `RequestVerifyToken` | rate-limited | | `POST /api/users/auth/verify` | `VerifyRequest` | | | `POST /api/users/auth/accept-invite` | `AcceptInviteRequest` | sets password + signs the user in | +| `POST /api/users/auth/demo` | — | one-click sign-in as the shared demo account; `404` unless `demo_mode` is on; rate-limited — see [Demo mode](#demo-mode-hosting-a-showcase-instance) | | `POST /api/users/auth/token` | `TokenRequest` (email + password) | bearer login for mobile / API clients → `{access_token, refresh_token, token_type, expires_in}`; `401` for external/SSO users | | `POST /api/users/auth/token/refresh` | `RefreshRequest` (refresh_token) | rotates a refresh token into a new pair (old one revoked) | | `DELETE /api/users/auth/token` | `RefreshRequest` (refresh_token) | revokes a refresh token (idempotent) | @@ -195,6 +196,12 @@ Everything else is DB-backed (initial values are pydantic defaults; edit under U | `auth_rate_limit_attempts` | `10` | | `auth_rate_limit_window_seconds` | `300` | | `bootstrap_email`, `bootstrap_password`, `bootstrap_user_email`, `bootstrap_user_password` | `""` — see [Bootstrap](#bootstrap-the-first-admin) | +| `demo_mode` | `False` — see [Demo mode](#demo-mode-hosting-a-showcase-instance) | +| `demo_email` | `"demo@example.com"` | +| `demo_password` | `""` (a random one is generated, so the account is reachable only through the button) | +| `demo_full_name` | `"Demo User"` | +| `demo_role` | `"user"` (or `"admin"`) | +| `demo_read_only` | `True` | | `oauth_google_client_id` / `oauth_google_client_secret` | `""` — Google OAuth | | `oauth_github_client_id` / `oauth_github_client_secret` | `""` — GitHub OAuth | | `oauth_microsoft_client_id` / `oauth_microsoft_client_secret` / `oauth_microsoft_tenant` | `""` / `""` / `"common"` — Microsoft (Entra ID) | @@ -252,6 +259,72 @@ Two paths to seed the first admin: 1. **CLI** — `smpy users create-admin ...`. 2. **Env vars** — set `SM_USERS_BOOTSTRAP_EMAIL` + `SM_USERS_BOOTSTRAP_PASSWORD` before first `make dev`. `bootstrap_admin_from_env(app)` runs at startup and creates the admin if the `users_user` table is empty. Optionally seed a non-admin too via `SM_USERS_BOOTSTRAP_USER_EMAIL` + `SM_USERS_BOOTSTRAP_USER_PASSWORD`. +## Demo mode (hosting a showcase instance) + +Turn `demo_mode` on and the sign-in card grows an **Explore the demo** button. +One click signs the visitor in as a shared, pre-seeded account — no email, no +password, no signup. + +``` +users.demo_mode = true +users.demo_email = demo@example.com +users.demo_role = admin # what actually shows off the framework +users.demo_read_only = true # default; leave it on +``` + +Set these under Users at `/admin/settings/`, or seed them in the settings store +before first boot. Changes apply live — the `SettingsReloaded` handler +re-seeds the account and refreshes the cached id, so there is no restart. + +### What it does + +- **Seeds the account** on every boot and every settings reload + (`users.demo.ensure_demo_user`). Idempotent, and it *reconciles*: changing + `demo_role` from `admin` back to `user` demotes the existing row and drops + the admin grant rather than leaving it in place. +- **Never sends the password to the browser.** The button posts to + `POST /api/users/auth/demo` with an empty body and the server resolves the + account itself. This is the difference between demo mode and the dev + quick-login buttons, which paste real credentials into the form and are + therefore development-only. Leave `demo_password` blank and the account is + seeded with a random secret nobody — including you — can type; set it only + if you also intend to publish the credentials (for an API demo, say). +- **Refuses writes** while `demo_read_only` is on. + `DemoReadOnlyMiddleware` rejects every unsafe HTTP method from a demo + session with `403 {"detail": "DEMO_READ_ONLY"}`, except signing out. Inertia + requests get the protocol's `409` + `X-Inertia-Location` instead, so a + blocked save re-renders the page rather than throwing up an error modal. +- **Says so.** `DemoBanner` renders a standing bar in every app shell, driven + by the `demo` shared prop. It is **per session**, not per install — you, + signed in to your own account on the same instance, do not see it, and your + writes are not touched. + +### `demo_role = "admin"` and `demo_read_only = false` + +This combination hands anyone who can reach your sign-in page a writable +superuser: the settings editor (including the SMTP password and OAuth client +secrets), the user table, and maintenance mode. It is allowed — an instance +whose database is rebuilt on a timer has a legitimate reason to want a +writable demo — but it logs `users.demo.writable_admin` at WARNING on every +settings load. Do not run it against a database you care about. + +### Limits worth knowing + +- The account is **shared**. Two visitors exploring at once see each other's + state, and with `demo_read_only = false` they can overwrite each other. + Periodically resetting the database is the only real answer. +- `demo_read_only` matches on the **session** — the stamp the demo endpoint + writes, or a session whose `user_id` is the demo account's. A bearer token + is not covered directly, but minting one is itself a `POST`, so a read-only + demo cannot get hold of one. +- The endpoint shares the `auth_rate_limit_*` budget with the other + credential-adjacent routes. Each click mints a session row, so a demo + instance under real traffic may want that raised. +- With `demo_role = "admin"`, the demo account **satisfies the first-run setup + gate** — an administrator exists, so `/setup` never appears. Seed your own + admin first (`smpy users create-admin`, or the `SM_USERS_BOOTSTRAP_*` vars), + because a read-only demo session cannot complete the wizard. + ## Mailer backends `mailer/` ships two implementations of the `Mailer` protocol: diff --git a/framework/hosting/tests/test_middleware_order.py b/framework/hosting/tests/test_middleware_order.py index 650193c4..4beaef82 100644 --- a/framework/hosting/tests/test_middleware_order.py +++ b/framework/hosting/tests/test_middleware_order.py @@ -32,6 +32,14 @@ to be fully hidden. That inversion breaks the feature without failing any site_lock unit test, which is why the order is pinned here. +DemoReadOnlyMiddleware is outermost of the module middlewares — ``users`` +sorts last, so it wraps SiteLock and Auth. That is where it wants to be: it +refuses a demo session's writes before any handler, middleware side-effect or +DB session runs, and it reads the demo marker straight off the session rather +than ``request.state.user``, so it needs nothing Auth provides. It also +cannot hide SiteLock from an anonymous visitor, because acquiring a demo +session means POSTing to the demo endpoint, which SiteLock blocks first. + Maintenance sits after InertiaLayoutData because its 503 page renders through Inertia and needs the shared props (auth, menus, i18n) — placed any further out it would render bare, with no layout and untranslated copy. It is @@ -64,6 +72,7 @@ "GZipMiddleware", "SecurityHeadersMiddleware", "SessionMiddleware", + "DemoReadOnlyMiddleware", "SiteLockMiddleware", "AuthMiddleware", "TenantMiddleware", @@ -81,6 +90,7 @@ "GZipMiddleware", "SecurityHeadersMiddleware", "SessionMiddleware", + "DemoReadOnlyMiddleware", "SiteLockMiddleware", "AuthMiddleware", "LocaleMiddleware", diff --git a/modules/users/README.md b/modules/users/README.md index 64b4cccf..3b1499f8 100644 --- a/modules/users/README.md +++ b/modules/users/README.md @@ -19,6 +19,7 @@ Pre-wired into any app scaffolded with `smpy new`. - `smpy users create-admin` CLI for ad-hoc admin creation. - Inertia pages for login/register/invite-accept/admin-invite. - Console mailer (logs to stdout) or SMTP mailer (`SM_USERS_MAILER=smtp`). +- Demo mode — a one-click, read-only shared account on the sign-in card, for public showcase instances. ## Usage @@ -51,6 +52,27 @@ async def profile(user: CurrentUser): - `simple_module_core`, `simple_module_db`, `simple_module_hosting`, `simple_module_settings`, `simple_module_auth` - `fastapi-users[sqlalchemy,oauth]>=15,<16`, `aiosmtplib`, `cachetools`, `typer` +## Demo mode + +For hosting a public showcase instance. Set under **/admin/settings/ → Users**: + +``` +demo_mode = true +demo_email = demo@example.com +demo_role = admin # "user" by default +demo_read_only = true # default — keep it on +``` + +The sign-in card grows an **Explore the demo** button that posts to +`POST /api/users/auth/demo`; the account's password never reaches the browser. +The account is seeded (and reconciled) at boot and on every settings reload, +and while `demo_read_only` is on, `DemoReadOnlyMiddleware` refuses every unsafe +HTTP method from the demo session — everyone else on the instance is unaffected. +A standing banner tells the visitor they are in a demo. + +Full detail, including the `demo_role = "admin"` + `demo_read_only = false` +warning, is in [docs/modules/users.md](../../docs/modules/users.md#demo-mode-hosting-a-showcase-instance). + ## Social sign-in (Google, GitHub, Microsoft, OIDC) OAuth providers are configured in the admin UI at **/settings/modules → Users** diff --git a/modules/users/tests/_demo_support.py b/modules/users/tests/_demo_support.py new file mode 100644 index 00000000..d3c08789 --- /dev/null +++ b/modules/users/tests/_demo_support.py @@ -0,0 +1,32 @@ +"""Fixtures for the demo-account tests, shared by the seeding and guard suites. + +Loaded as a pytest plugin from ``conftest.py`` — same mechanism as +``_middleware_support``. Not named ``test_*`` so pytest does not collect it. +""" + +from __future__ import annotations + +import httpx +import pytest +from _users_app_builders import _build_users_app +from users.demo import ensure_demo_user + +DEMO_EMAIL = "demo@example.com" + + +@pytest.fixture +async def demo_app(monkeypatch): + """A users app with demo mode on and the demo account seeded.""" + application, ctx = await _build_users_app(monkeypatch, allow_signup=False) + application.state.users.settings.demo_mode = True + await ensure_demo_user(application) + yield application + await ctx.__aexit__(None, None, None) + + +@pytest.fixture +async def demo_client(demo_app): + """Anonymous client against ``demo_app`` — it signs itself in via the button.""" + transport = httpx.ASGITransport(app=demo_app) + async with httpx.AsyncClient(transport=transport, base_url="http://testserver") as client: + yield client diff --git a/modules/users/tests/conftest.py b/modules/users/tests/conftest.py index f76f40ec..e6f3b702 100644 --- a/modules/users/tests/conftest.py +++ b/modules/users/tests/conftest.py @@ -162,4 +162,4 @@ async def users_db(users_app) -> AsyncGenerator[AsyncSession, None]: # Fixtures consumed by the users.middleware unit tests live in # _middleware_support.py (imported as a pytest plugin below). -pytest_plugins = ["_middleware_support"] +pytest_plugins = ["_middleware_support", "_demo_support"] diff --git a/modules/users/tests/test_demo_account.py b/modules/users/tests/test_demo_account.py new file mode 100644 index 00000000..b468a4cf --- /dev/null +++ b/modules/users/tests/test_demo_account.py @@ -0,0 +1,238 @@ +"""Demo account: how it is configured, seeded, and entered. + +Two of the ways the feature can fail open live here — a demo button that +appears when no account is configured, and an endpoint that signs someone in +when the feature is off. The third, a demo session that is allowed to write, +is ``test_demo_guard``. +""" + +from __future__ import annotations + +from _demo_support import DEMO_EMAIL +from sqlalchemy import select +from users.constants import ADMIN_ROLE_NAME, USER_ROLE_NAME +from users.demo import ( + DemoAccount, + ensure_demo_user, + reconcile_demo_user, + resolve_demo_account, +) +from users.models import Role, User, UserRole +from users.settings import UsersSettings + + +def _settings(**overrides) -> UsersSettings: + base = { + "reset_password_token_secret": "test-reset-secret-32-bytes-xxxxx", + "verification_token_secret": "test-verify-secret-32-bytes-xxxxx", + } + return UsersSettings(**base, **overrides) + + +# ── resolve_demo_account ──────────────────────────────────────────────────── + + +def test_demo_is_off_by_default(): + assert resolve_demo_account(_settings()) is None + + +def test_blank_email_reads_as_off_not_as_an_error(): + """A half-finished edit in the admin UI must not break the sign-in page.""" + assert resolve_demo_account(_settings(demo_mode=True, demo_email=" ")) is None + + +def test_an_unknown_role_falls_back_to_the_standard_one(): + """Never silently upgrade: an unrecognised value must not mean admin. + + ``demo_role`` carries a pydantic pattern, so this can only come from a + value written straight into the settings table — which is exactly the case + the fallback is there for, and why it is tested through ``model_construct`` + rather than the validated constructor. + """ + raw = UsersSettings.model_construct( + demo_mode=True, + demo_email=DEMO_EMAIL, + demo_password="", + demo_full_name="Demo User", + demo_role="superuser", + demo_read_only=True, + ) + account = resolve_demo_account(raw) + assert account is not None + assert account.role == USER_ROLE_NAME + + +def test_read_only_is_the_default_posture(): + account = resolve_demo_account(_settings(demo_mode=True)) + assert account is not None and account.read_only is True + + +# ── seeding ───────────────────────────────────────────────────────────────── + + +async def _demo_row(app) -> User | None: + async with app.state.sm.db.session_factory() as session: + return ( + await session.execute(select(User).where(User.email == DEMO_EMAIL)) + ).scalar_one_or_none() + + +async def test_ensure_demo_user_seeds_the_account(users_app): + users_app.state.users.settings.demo_mode = True + user_id = await ensure_demo_user(users_app) + + assert user_id is not None + assert users_app.state.users.demo_user_id == user_id + row = await _demo_row(users_app) + assert row is not None + assert row.is_active and row.is_verified and not row.is_superuser + + +async def test_a_blank_password_still_produces_a_usable_account(users_app): + """The button does not need a password; the row still needs a hash.""" + users_app.state.users.settings.demo_mode = True + await ensure_demo_user(users_app) + + row = await _demo_row(users_app) + assert row is not None and row.hashed_password + + +async def test_a_generated_password_is_not_re_rolled_on_every_boot(users_app): + """Re-hashing a secret nobody can use would write an audit entry per + worker per restart, for no gain.""" + users_app.state.users.settings.demo_mode = True + await ensure_demo_user(users_app) + first = (await _demo_row(users_app)).hashed_password + + await ensure_demo_user(users_app) + + assert (await _demo_row(users_app)).hashed_password == first + + +async def test_a_configured_password_is_reapplied(users_app): + """Changing demo_password in the admin UI has to take effect.""" + users_app.state.users.settings.demo_mode = True + await ensure_demo_user(users_app) + first = (await _demo_row(users_app)).hashed_password + + users_app.state.users.settings.demo_password = "a-published-demo-password" + await ensure_demo_user(users_app) + + assert (await _demo_row(users_app)).hashed_password != first + + +async def test_demo_off_clears_the_cached_id(users_app): + users_app.state.users.settings.demo_mode = True + await ensure_demo_user(users_app) + users_app.state.users.settings.demo_mode = False + + assert await ensure_demo_user(users_app) is None + assert users_app.state.users.demo_user_id is None + + +async def test_demoting_the_role_drops_the_admin_grant(users_app): + """Flipping demo_role back is how an operator revokes a too-open demo.""" + async with users_app.state.sm.db.session_factory() as session: + account = DemoAccount( + email=DEMO_EMAIL, password="x", full_name="Demo", role=ADMIN_ROLE_NAME, read_only=True + ) + user = await reconcile_demo_user(session, account) + assert user.is_superuser + + await reconcile_demo_user( + session, + DemoAccount( + email=DEMO_EMAIL, + password="x", + full_name="Demo", + role=USER_ROLE_NAME, + read_only=True, + ), + ) + + async with users_app.state.sm.db.session_factory() as session: + roles = ( + ( + await session.execute( + select(Role.name) + .join(UserRole, UserRole.role_id == Role.id) + .where(UserRole.user_id == user.id) + ) + ) + .scalars() + .all() + ) + refreshed = await session.get(User, user.id) + assert roles == [USER_ROLE_NAME] + assert refreshed is not None and not refreshed.is_superuser + + +# ── endpoint + login page ─────────────────────────────────────────────────── + + +async def test_the_endpoint_is_absent_when_demo_mode_is_off(anon_client): + res = await anon_client.post("/api/users/auth/demo") + assert res.status_code == 404 + + +async def test_the_login_page_advertises_no_demo_by_default(anon_client): + res = await anon_client.get("/users/login", headers={"X-Inertia": "true"}) + assert res.status_code == 200 + assert res.json()["props"]["demo_signin"] == {"enabled": False, "read_only": False} + + +async def test_the_login_page_advertises_the_demo(demo_client): + res = await demo_client.get("/users/login", headers={"X-Inertia": "true"}) + assert res.json()["props"]["demo_signin"] == {"enabled": True, "read_only": True} + + +async def test_the_login_page_never_leaks_the_demo_password(demo_app, demo_client): + demo_app.state.users.settings.demo_password = "sup3r-s3cret-demo" + await ensure_demo_user(demo_app) + + res = await demo_client.get("/users/login", headers={"X-Inertia": "true"}) + assert "sup3r-s3cret-demo" not in res.text + + +async def test_a_disabled_demo_account_is_not_a_way_in(demo_app, demo_client): + """Disabling the row in the admin UI has to actually stop the button.""" + from datetime import UTC, datetime + + async with demo_app.state.sm.db.session_factory() as session: + row = await session.get(User, demo_app.state.users.demo_user_id) + row.is_active = False + row.disabled_at = datetime.now(UTC) + await session.commit() + + assert (await demo_client.post("/api/users/auth/demo")).status_code == 404 + + +async def test_one_click_signs_the_visitor_in(demo_client): + res = await demo_client.post("/api/users/auth/demo") + assert res.status_code == 204 + + me = await demo_client.get("/api/users/me") + assert me.status_code == 200 + assert me.json()["email"] == DEMO_EMAIL + + +async def test_signing_in_turns_the_banner_on(demo_client): + """The shared ``demo`` prop is what every shell reads to say "this is a + demo". Before the button it is inactive; after it, it is not.""" + before = (await demo_client.get("/users/login", headers={"X-Inertia": "true"})).json() + assert before["props"]["demo"] == {"active": False, "readOnly": False} + + await demo_client.post("/api/users/auth/demo") + + after = (await demo_client.get("/users/login", headers={"X-Inertia": "true"})).json() + assert after["props"]["demo"] == {"active": True, "readOnly": True} + + +async def test_the_offer_and_the_session_are_separate_props(demo_client): + """Page props win the Inertia merge, so a page prop named ``demo`` would + shadow the banner's shared prop on this page alone — a collision that + breaks quietly and only here.""" + props = (await demo_client.get("/users/login", headers={"X-Inertia": "true"})).json()["props"] + + assert set(props["demo"]) == {"active", "readOnly"} + assert set(props["demo_signin"]) == {"enabled", "read_only"} diff --git a/modules/users/tests/test_demo_guard.py b/modules/users/tests/test_demo_guard.py new file mode 100644 index 00000000..03eb3c9e --- /dev/null +++ b/modules/users/tests/test_demo_guard.py @@ -0,0 +1,117 @@ +"""``DemoReadOnlyMiddleware`` — what a shared demo session may and may not do. + +Split from ``test_demo_account`` (which covers configuring and seeding the +account) because this is the security boundary: every test here is a way the +guard could fail open. +""" + +from __future__ import annotations + +import httpx +from _demo_support import DEMO_EMAIL +from users.demo import SESSION_DEMO_KEY +from users.demo_guard import DEMO_READ_ONLY_DETAIL + +# ── read-only guard ───────────────────────────────────────────────────────── + + +async def test_a_demo_session_cannot_write(demo_client): + await demo_client.post("/api/users/auth/demo") + + res = await demo_client.patch("/api/users/me", json={"full_name": "Owned"}) + assert res.status_code == 403 + assert res.json()["detail"] == DEMO_READ_ONLY_DETAIL + + +async def test_a_demo_session_can_still_read(demo_client): + await demo_client.post("/api/users/auth/demo") + assert (await demo_client.get("/api/users/me")).status_code == 200 + + +async def test_a_demo_session_can_still_sign_out(demo_client): + await demo_client.post("/api/users/auth/demo") + assert (await demo_client.post("/api/users/auth/logout")).status_code == 204 + + +async def test_an_inertia_write_gets_a_hard_redirect_not_a_raw_403(demo_client): + """Inertia renders a non-Inertia body as a modal — on a showcase instance + that reads as a crash. The protocol's own answer is 409 + Location.""" + await demo_client.post("/api/users/auth/demo") + + res = await demo_client.patch( + "/api/users/me", + json={"full_name": "Owned"}, + headers={"X-Inertia": "true", "Referer": "http://testserver/users/me"}, + ) + assert res.status_code == 409 + assert res.headers["X-Inertia-Location"] == "http://testserver/users/me" + + +async def test_writes_are_allowed_when_read_only_is_off(demo_app, demo_client): + demo_app.state.users.settings.demo_read_only = False + await demo_client.post("/api/users/auth/demo") + + res = await demo_client.patch("/api/users/me", json={"full_name": "Explorer"}) + assert res.status_code == 200 + + +async def test_the_guard_leaves_other_accounts_alone(demo_app): + """A real administrator on a demo instance is doing real work. + + Goes through an admin route rather than ``/api/users/me``: that one + authenticates off the fastapi-users cookie, which a forged session cookie + does not carry, so a 401 there would prove nothing about the guard. + """ + from _users_app_builders import _make_admin_user + from simple_module_test import forge_session_cookie + + admin = await _make_admin_user(demo_app) + cookie = forge_session_cookie( + str(demo_app.state.sm.settings.secret_key), {"user_id": str(admin.id)} + ) + transport = httpx.ASGITransport(app=demo_app) + async with httpx.AsyncClient( + transport=transport, base_url="http://testserver", cookies={"session": cookie} + ) as client: + res = await client.patch( + f"/api/users/admin/{demo_app.state.users.demo_user_id}", + json={"email": DEMO_EMAIL, "full_name": "Renamed By A Real Admin"}, + ) + assert res.status_code == 200 + + +async def test_a_demo_session_cannot_reach_the_admin_write_routes(demo_app): + """The showcase account may browse /admin/*, never mutate through it.""" + from simple_module_test import forge_session_cookie + + cookie = forge_session_cookie( + str(demo_app.state.sm.settings.secret_key), + {"user_id": str(demo_app.state.users.demo_user_id), SESSION_DEMO_KEY: True}, + ) + transport = httpx.ASGITransport(app=demo_app) + async with httpx.AsyncClient( + transport=transport, base_url="http://testserver", cookies={"session": cookie} + ) as client: + res = await client.delete(f"/api/users/admin/{demo_app.state.users.demo_user_id}") + assert res.status_code == 403 + assert res.json()["detail"] == DEMO_READ_ONLY_DETAIL + + +async def test_an_unstamped_session_on_the_demo_account_still_blocks(demo_app): + """The stamp is sufficient, not necessary. + + Matching on the user id too is what catches a visitor who signed in with a + published ``demo_password`` through the ordinary form — that session never + passes through the demo endpoint, so it carries no stamp. + """ + from simple_module_test import forge_session_cookie + + payload = {"user_id": str(demo_app.state.users.demo_user_id)} + assert SESSION_DEMO_KEY not in payload + cookie = forge_session_cookie(str(demo_app.state.sm.settings.secret_key), payload) + transport = httpx.ASGITransport(app=demo_app) + async with httpx.AsyncClient( + transport=transport, base_url="http://testserver", cookies={"session": cookie} + ) as client: + res = await client.patch("/api/users/me", json={"full_name": "Owned"}) + assert res.status_code == 403 diff --git a/modules/users/tests/test_users_shared_props.py b/modules/users/tests/test_users_shared_props.py index 4ebeb5df..97ebc4b3 100644 --- a/modules/users/tests/test_users_shared_props.py +++ b/modules/users/tests/test_users_shared_props.py @@ -1,9 +1,14 @@ -"""The ``signup`` shared prop drives whether a "Sign up" link is rendered. +"""The shared props the users module contributes to every Inertia payload. -``/users/register`` raises 404 when ``allow_signup`` is off, so the public shell -has to know the answer before it draws the link. Getting the default wrong in -either direction is a visible bug: too eager and every visitor hits a 404, too -shy and a genuinely open instance hides its own signup. +``signup`` drives whether a "Sign up" link is rendered: ``/users/register`` +raises 404 when ``allow_signup`` is off, so the public shell has to know the +answer before it draws the link. Getting the default wrong in either direction +is a visible bug — too eager and every visitor hits a 404, too shy and a +genuinely open instance hides its own signup. + +``demo`` drives the standing demo banner, and is per *session*: an operator +signed in normally on a showcase instance must not be told their work is +throwaway. """ from __future__ import annotations @@ -11,19 +16,23 @@ from types import SimpleNamespace import pytest +from users.demo import SESSION_DEMO_KEY from users.shared_props import users_shared_props -def _request(users_state: object) -> SimpleNamespace: +def _request(users_state: object, session: dict | None = None) -> SimpleNamespace: """A stand-in carrying only what the provider is allowed to touch.""" - return SimpleNamespace(app=SimpleNamespace(state=SimpleNamespace(users=users_state))) + return SimpleNamespace( + app=SimpleNamespace(state=SimpleNamespace(users=users_state)), + scope={"session": session or {}}, + ) class TestSignupSharedProp: @pytest.mark.parametrize("allow", [True, False]) def test_reflects_the_setting(self, allow: bool) -> None: request = _request(SimpleNamespace(settings=SimpleNamespace(allow_signup=allow))) - assert users_shared_props(request) == {"signup": {"allowed": allow}} + assert users_shared_props(request)["signup"] == {"allowed": allow} def test_coerces_to_a_real_bool(self) -> None: """The value is serialised straight to JSON, so it must not leak a @@ -33,13 +42,55 @@ def test_coerces_to_a_real_bool(self) -> None: def test_defaults_closed_when_settings_are_missing(self) -> None: request = _request(SimpleNamespace()) - assert users_shared_props(request) == {"signup": {"allowed": False}} + assert users_shared_props(request)["signup"] == {"allowed": False} def test_defaults_closed_when_module_state_is_absent(self) -> None: """Never raises: the provider runs on every request and a failure here would cost the whole page, not just the link.""" - request = SimpleNamespace(app=SimpleNamespace(state=SimpleNamespace())) - assert users_shared_props(request) == {"signup": {"allowed": False}} + request = SimpleNamespace(app=SimpleNamespace(state=SimpleNamespace()), scope={}) + assert users_shared_props(request)["signup"] == {"allowed": False} + + +class TestDemoSharedProp: + def test_inactive_when_demo_mode_is_off(self) -> None: + request = _request( + SimpleNamespace(settings=SimpleNamespace(demo_mode=False), demo_user_id=None), + session={SESSION_DEMO_KEY: True}, + ) + assert users_shared_props(request)["demo"] == {"active": False, "readOnly": False} + + def test_inactive_for_an_ordinary_session_on_a_demo_instance(self) -> None: + """The operator's own session is not a demo session.""" + request = _request( + SimpleNamespace( + settings=SimpleNamespace(demo_mode=True, demo_read_only=True), + demo_user_id="11111111-1111-1111-1111-111111111111", + ), + session={"user_id": "22222222-2222-2222-2222-222222222222"}, + ) + assert users_shared_props(request)["demo"] == {"active": False, "readOnly": False} + + def test_active_for_the_demo_session(self) -> None: + request = _request( + SimpleNamespace( + settings=SimpleNamespace(demo_mode=True, demo_read_only=True), + demo_user_id=None, + ), + session={SESSION_DEMO_KEY: True}, + ) + assert users_shared_props(request)["demo"] == {"active": True, "readOnly": True} + + def test_read_only_tracks_the_setting(self) -> None: + """The banner says "changes are not saved" — it must not say it when + they are.""" + request = _request( + SimpleNamespace( + settings=SimpleNamespace(demo_mode=True, demo_read_only=False), + demo_user_id=None, + ), + session={SESSION_DEMO_KEY: True}, + ) + assert users_shared_props(request)["demo"] == {"active": True, "readOnly": False} class TestProviderIsRegistered: diff --git a/modules/users/users/auth_local/components/DemoSignIn.tsx b/modules/users/users/auth_local/components/DemoSignIn.tsx new file mode 100644 index 00000000..e5d692d0 --- /dev/null +++ b/modules/users/users/auth_local/components/DemoSignIn.tsx @@ -0,0 +1,50 @@ +import { keys, useT } from '@simple-module-py/i18n'; +import { Button } from '@simple-module-py/ui/components/ui/button'; + +interface DemoSignInProps { + /** Whether the demo session will be refused every write. */ + readOnly: boolean; + pending: boolean; + error: string | null; + onStart: () => void; +} + +/** + * "Explore the demo" — the one-click entry to a showcase instance. + * + * Deliberately below the credentials form and behind its own divider: it is an + * alternative to signing in, not an OAuth provider, and putting it in that row + * would read as "sign in with Demo". + * + * No email or password crosses the wire. The button posts to + * `/api/users/auth/demo`, which looks the shared account up server-side — the + * dev quick-fill buttons above it paste real credentials into the form and are + * development-only for exactly that reason. + */ +export function DemoSignIn({ readOnly, pending, error, onStart }: DemoSignInProps) { + const { t } = useT(); + + return ( +
+

+ {t(keys.users.login.demo_divider)} +

+ +

+ {readOnly + ? t(keys.users.login.demo_hint_read_only) + : t(keys.users.login.demo_hint_writable)} +

+ {error &&

{error}

} +
+ ); +} diff --git a/modules/users/users/auth_local/demo_api.py b/modules/users/users/auth_local/demo_api.py new file mode 100644 index 00000000..dddac91b --- /dev/null +++ b/modules/users/users/auth_local/demo_api.py @@ -0,0 +1,79 @@ +"""``POST /api/users/auth/demo`` — sign in as the shared demo account. + +Its own module rather than a branch inside ``api.login``: there is no password +in the request, so none of the rate-limit-on-failure, verification or +"remember me" machinery applies. What it shares with the password path is the +session bridging, and that is three lines. + +The account's password is never sent to the browser. A visitor clicks the +button, the server looks the account up itself, and the session it mints is +stamped :data:`users.demo.SESSION_DEMO_KEY` so +:class:`users.demo_guard.DemoReadOnlyMiddleware` can recognise it. +""" + +from __future__ import annotations + +import logging + +from fastapi import APIRouter, Depends, HTTPException, Request, Response, status +from fastapi_users import exceptions as fu_exceptions +from simple_module_core.redirect_safety import SESSION_NEXT_KEY + +from users.auth_local.rate_limit import enforce_auth_throughput_limit +from users.constants import SESSION_USER_ID_KEY +from users.demo import SESSION_DEMO_KEY, resolve_demo_account +from users.deps import auth_backend, get_user_manager +from users.manager import UserManager + +logger = logging.getLogger("users.demo") + +router = APIRouter() + + +@router.post( + "/auth/demo", + status_code=204, + # Every call mints an access-token row and a session cookie, and the + # endpoint takes no credential — without a budget it is a free row + # generator for anyone who finds the showcase instance. + dependencies=[Depends(enforce_auth_throughput_limit)], +) +async def demo_login( + request: Request, + response: Response, + user_manager: UserManager = Depends(get_user_manager), + strategy=Depends(auth_backend.get_strategy), +) -> Response: + """Sign the caller in as the configured demo account. + + 404 rather than 403 when demo mode is off, matching ``/users/register``: + a disabled feature should look absent, not forbidden, so probing the + endpoint says nothing about how the instance is configured. + """ + state = request.app.state.users + account = resolve_demo_account(state.settings) + if account is None: + raise HTTPException(status_code=status.HTTP_404_NOT_FOUND) + + # By email, not by the cached ``state.demo_user_id``: the id is a boot-time + # convenience, so a worker whose reconcile failed would otherwise serve a + # button that 500s. Through the manager rather than a session of our own + # so the row ``on_after_login`` writes ``last_login_at`` to is this one. + try: + demo_user = await user_manager.get_by_email(account.email) + except fu_exceptions.UserNotExists: + logger.warning("users.demo.missing_account", extra={"email": account.email}) + raise HTTPException(status_code=status.HTTP_404_NOT_FOUND) from None + if not demo_user.is_active or demo_user.disabled_at is not None: + logger.warning("users.demo.inactive_account", extra={"email": account.email}) + raise HTTPException(status_code=status.HTTP_404_NOT_FOUND) + + await user_manager.on_after_login(demo_user, request, response) + login_response = await auth_backend.login(strategy, demo_user) + request.session[SESSION_USER_ID_KEY] = str(demo_user.id) + request.session[SESSION_DEMO_KEY] = True + # A demo visitor bounced off a deep link should land there, same as any + # other sign-in; clearing it here keeps the next plain visit to the login + # page from inheriting a stale destination. + request.session.pop(SESSION_NEXT_KEY, None) + return login_response diff --git a/modules/users/users/auth_local/views.py b/modules/users/users/auth_local/views.py index 1c8fcf45..fe9d691c 100644 --- a/modules/users/users/auth_local/views.py +++ b/modules/users/users/auth_local/views.py @@ -16,6 +16,7 @@ from users.auth_local.token_preview import decode_verify_token, preview_reset from users.bootstrap import resolve_bootstrap_credentials from users.contracts.schemas import UserRead +from users.demo import resolve_demo_account from users.mailer import mailer_delivers from users.manager import UserManager, get_user_manager from users.models import User @@ -71,6 +72,21 @@ async def login_page(request: Request, inertia: InertiaDep) -> InertiaResponse: _PAGE_LOGIN, { "allow_signup": users_settings.allow_signup, + # The demo card, when a showcase account is configured. Carries no + # password: the button posts to /api/users/auth/demo and the + # server looks the account up itself. ``read_only`` is here so the + # card can say what the visitor will and will not be able to do + # before they click, rather than after their first refused save. + # + # Not ``demo`` — that name belongs to the shared prop describing + # the *current* session (``users.shared_props``). Page props win + # the merge, so reusing it would have this key quietly shadow the + # banner's on this one page. + "demo_signin": ( + {"enabled": True, "read_only": demo_account.read_only} + if (demo_account := resolve_demo_account(users_settings)) + else {"enabled": False, "read_only": False} + ), "dev_accounts": dev_accounts, # Where AuthMiddleware bounced them from, when it bounced them. # Read, not popped: a reload of the login page must not silently diff --git a/modules/users/users/demo.py b/modules/users/users/demo.py new file mode 100644 index 00000000..dc8ff7d7 --- /dev/null +++ b/modules/users/users/demo.py @@ -0,0 +1,190 @@ +"""Shared demo account — the one-click sign-in behind a public showcase instance. + +Three pieces live here because they have to agree on one question ("is a demo +account configured, and who is it?"): + +* :func:`resolve_demo_account` — reads the answer out of settings. +* :func:`ensure_demo_user` — reconciles the account row against that answer and + caches its id on ``app.state.users``. +* :data:`SESSION_DEMO_KEY` — what a demo sign-in stamps on the session so the + read-only guard can recognise it without a database read. + +Deliberately *not* the dev quick-login buttons (``users.auth_local.views``): +those are development-only and paste real credentials into the form, which is +exactly what a published demo must not do. Here the password never leaves the +server — :mod:`users.auth_local.demo_api` mints the session directly. +""" + +from __future__ import annotations + +import logging +import secrets +import uuid +from dataclasses import dataclass + +from fastapi import FastAPI +from fastapi_users.password import PasswordHelper +from sqlalchemy import func, select +from sqlalchemy.ext.asyncio import AsyncSession + +from users.constants import ( + ADMIN_ROLE_DESCRIPTION, + ADMIN_ROLE_ID, + ADMIN_ROLE_NAME, + USER_ROLE_DESCRIPTION, + USER_ROLE_ID, + USER_ROLE_NAME, +) +from users.models import Role, User, UserRole +from users.settings import UsersSettings + +logger = logging.getLogger("users.demo") + +# Stamped on the session by the demo sign-in endpoint. The guard also matches +# on the user id, so this is not the only line of defence — it is what keeps +# the hot path off the database. +SESSION_DEMO_KEY = "is_demo" + +_EVT_CREATED = "users.demo.created" +_EVT_RECONCILED = "users.demo.reconciled" +_EVT_DISABLED = "users.demo.disabled" + +_ROLE_SEEDS = { + ADMIN_ROLE_NAME: (ADMIN_ROLE_ID, ADMIN_ROLE_DESCRIPTION), + USER_ROLE_NAME: (USER_ROLE_ID, USER_ROLE_DESCRIPTION), +} + + +@dataclass(frozen=True) +class DemoAccount: + """The demo account an operator has asked for, normalised.""" + + email: str + password: str + full_name: str + role: str + read_only: bool + + +def resolve_demo_account(settings: UsersSettings | None) -> DemoAccount | None: + """The configured demo account, or ``None`` when the feature is off. + + A blank ``demo_email`` reads as off rather than as an error: the field is + editable in the admin UI, and a half-finished edit should leave the sign-in + page unchanged instead of failing the next boot. + """ + if settings is None or not getattr(settings, "demo_mode", False): + return None + email = (settings.demo_email or "").strip() + if not email: + logger.warning("%s — demo_mode is on but demo_email is blank", _EVT_DISABLED) + return None + return DemoAccount( + email=email, + password=settings.demo_password or "", + full_name=(settings.demo_full_name or "").strip() or "Demo User", + role=settings.demo_role if settings.demo_role in _ROLE_SEEDS else USER_ROLE_NAME, + read_only=bool(settings.demo_read_only), + ) + + +async def _role_row(db: AsyncSession, name: str) -> Role: + """The Role row for ``name``, created from its seed constants if absent. + + Same shape as ``users.bootstrap``: the seed migration normally inserts + both, so this only fires for tests built with ``create_all`` and for + databases downgraded past the seed revision. + """ + role = (await db.execute(select(Role).where(Role.name == name))).scalar_one_or_none() + if role is not None: + return role + role_id, description = _ROLE_SEEDS[name] + role = (await db.execute(select(Role).where(Role.id == role_id))).scalar_one_or_none() + if role is None: + role = Role(id=role_id, name=name, description=description) + db.add(role) + await db.flush() + return role + + +async def _sync_role(db: AsyncSession, user: User, role_name: str) -> None: + """Give the demo user exactly the configured role, dropping the other one. + + Dropping matters: flipping ``demo_role`` from ``admin`` back to ``user`` is + how an operator revokes a demo that turned out to be too open, and a switch + that only ever added rows would leave the admin grant in place. + """ + wanted = await _role_row(db, role_name) + links = (await db.execute(select(UserRole).where(UserRole.user_id == user.id))).scalars().all() + managed = {rid for rid, _ in _ROLE_SEEDS.values()} + if not any(link.role_id == wanted.id for link in links): + db.add(UserRole(user_id=user.id, role_id=wanted.id)) + for link in links: + if link.role_id != wanted.id and link.role_id in managed: + await db.delete(link) + + +async def reconcile_demo_user(db: AsyncSession, account: DemoAccount) -> User: + """Create or update the demo account row so it matches ``account``. + + Idempotent, and run on every boot *and* every settings reload — an operator + who turns demo mode on in the admin UI of a long-running install must not + have to restart to get the account. + + A configured ``demo_password`` is (re)applied every time, so changing it in + the admin UI takes effect. A blank one is hashed from a fresh random secret + on create only — that is what makes "reachable only via the button" true, + and re-rolling it every boot would write an audit entry per worker per + restart for a value nobody can use. + """ + hasher = PasswordHelper() + user = ( + await db.execute(select(User).where(func.lower(User.email) == account.email.lower())) + ).scalar_one_or_none() + created = user is None + if user is None: + user = User(email=account.email) + db.add(user) + + if account.password: + user.hashed_password = hasher.hash(account.password) + elif created: + user.hashed_password = hasher.hash(secrets.token_urlsafe(32)) + user.full_name = account.full_name + user.is_active = True + user.is_verified = True + user.is_superuser = account.role == ADMIN_ROLE_NAME + user.disabled_at = None + await db.flush() + await _sync_role(db, user, account.role) + await db.commit() + await db.refresh(user) + logger.info( + _EVT_CREATED if created else _EVT_RECONCILED, + extra={"email": account.email, "id": str(user.id), "role": account.role}, + ) + return user + + +async def ensure_demo_user(app: FastAPI) -> uuid.UUID | None: + """Reconcile the demo account and cache its id on ``app.state.users``. + + Returns the id (also stored as ``state.demo_user_id``) or ``None`` when no + demo account is configured. Never raises: a demo instance failing to boot + because the showcase account could not be written is a worse outcome than + booting without the button. + """ + state = app.state.users + account = resolve_demo_account(state.settings) + if account is None: + state.demo_user_id = None + return None + try: + async with app.state.sm.db.session_factory() as session: + user = await reconcile_demo_user(session, account) + except Exception: + logger.exception("users.demo.failed", extra={"email": account.email}) + state.demo_user_id = None + return None + state.demo_user_id = user.id + return user.id diff --git a/modules/users/users/demo_guard.py b/modules/users/users/demo_guard.py new file mode 100644 index 00000000..bcd569da --- /dev/null +++ b/modules/users/users/demo_guard.py @@ -0,0 +1,107 @@ +"""Read-only enforcement for the shared demo account. + +A demo account is a *published* credential: the sign-in page hands it to +whoever asks. Without this guard, hosting a showcase instance means publishing +write access to the settings editor, the user table and maintenance mode — so +``demo_read_only`` defaults to on and this is what implements it. + +Why a middleware and not a dependency: the point is to cover every route in +every installed module, including ones written before demo mode existed. A +dependency covers only the routes that remember to declare it. + +Why it reads the session rather than ``request.state.user``: module middleware +sorts by ``depends_on``, and ``users`` depends on ``Auth``, so this executes +*before* ``AuthMiddleware`` has resolved anyone. The session cookie is already +decoded by then (``Session`` sits outside every module's middleware), and it +carries both the stamp the demo endpoint writes and the user id — which is +what catches a visitor who signed in with a published demo password through +the ordinary form instead of the button. +""" + +from __future__ import annotations + +from starlette.requests import Request +from starlette.responses import JSONResponse, Response +from starlette.types import ASGIApp, Receive, Scope, Send + +from users.constants import SESSION_USER_ID_KEY +from users.demo import SESSION_DEMO_KEY + +#: The error body the frontend matches on to show "this is a read-only demo". +DEMO_READ_ONLY_DETAIL = "DEMO_READ_ONLY" + +_SAFE_METHODS = frozenset({"GET", "HEAD", "OPTIONS", "TRACE"}) + +#: Writes a demo visitor must keep: leaving is not a mutation of the instance, +#: and a demo you cannot sign out of is a demo you cannot show twice. +_ALLOWED_PATHS = frozenset( + { + "/users/logout", + "/api/users/auth/logout", + "/api/users/auth/demo", + } +) + + +def is_demo_session(scope: Scope, demo_user_id) -> bool: + """Whether this request is carrying the demo account's session.""" + session = scope.get("session") or {} + if session.get(SESSION_DEMO_KEY): + return True + if demo_user_id is None: + return False + return str(session.get(SESSION_USER_ID_KEY) or "") == str(demo_user_id) + + +def _refusal(scope: Scope) -> Response: + """The response a blocked write gets. + + Inertia rejects a non-Inertia response by throwing up a modal with the raw + body in it, which on a showcase instance reads as a crash. The protocol's + own escape hatch is a 409 carrying ``X-Inertia-Location``: the client does + a hard visit to that URL instead. Sending it back where it came from means + the page simply re-renders unchanged, and the standing demo banner is what + explains why. Plain ``fetch`` callers — which is how most of this app + writes — get the 403 and can show the reason inline. + """ + headers = Request(scope).headers + if headers.get("x-inertia"): + return Response( + status_code=409, + headers={"X-Inertia-Location": headers.get("referer") or "/"}, + ) + return JSONResponse({"detail": DEMO_READ_ONLY_DETAIL}, status_code=403) + + +class DemoReadOnlyMiddleware: + """Refuse unsafe HTTP methods from the shared demo session.""" + + def __init__(self, app: ASGIApp) -> None: + self.app = app + + async def __call__(self, scope: Scope, receive: Receive, send: Send) -> None: + if scope["type"] != "http" or scope.get("method", "GET") in _SAFE_METHODS: + await self.app(scope, receive, send) + return + + state = getattr(scope["app"].state, "users", None) + settings = getattr(state, "settings", None) + # ``demo_mode`` and ``demo_read_only`` are both live-editable, so they + # are read per request rather than captured at construction. + if ( + settings is None + or not getattr(settings, "demo_mode", False) + or not getattr(settings, "demo_read_only", True) + ): + await self.app(scope, receive, send) + return + + if scope["path"] in _ALLOWED_PATHS: + await self.app(scope, receive, send) + return + + if not is_demo_session(scope, getattr(state, "demo_user_id", None)): + await self.app(scope, receive, send) + return + + await _refusal(scope)(scope, receive, send) diff --git a/modules/users/users/locales/en.json b/modules/users/users/locales/en.json index 69857e3a..17cb930c 100644 --- a/modules/users/users/locales/en.json +++ b/modules/users/users/locales/en.json @@ -230,7 +230,13 @@ "waiting_body": "Confirm your email address before signing in. Nothing else is reachable until it's done.", "waiting_back": "Use a different account", "no_account_invite_only": "No account? Ask an admin to invite you.", - "resend_failed": "We could not send it just now. Try again in a minute." + "resend_failed": "We could not send it just now. Try again in a minute.", + "demo_divider": "Just looking?", + "demo_submit": "Explore the demo", + "demo_submitting": "Opening the demo…", + "demo_hint_read_only": "Signs you in to a shared, read-only account. Nothing you do is saved.", + "demo_hint_writable": "Signs you in to a shared account. Anyone else exploring sees the same data.", + "demo_error": "The demo account is unavailable right now. Please try again shortly." }, "metadata_card": { "title": "Account", diff --git a/modules/users/users/module.py b/modules/users/users/module.py index bbc99ae7..ebdc4ae2 100644 --- a/modules/users/users/module.py +++ b/modules/users/users/module.py @@ -103,9 +103,26 @@ async def _rebuild_oauth_clients(event: settings_reloaded) -> None: state = app.state.users state.oauth_clients = build_client_map(state.settings) state.oauth_providers = provider_buttons(state.oauth_clients) + # Same reason: turning demo mode on from the settings UI has to + # seed the account and refresh the cached id, or the button + # appears on a sign-in page that 404s until the next restart. + from users.demo import ensure_demo_user + + await ensure_demo_user(app) bus.subscribe(settings_reloaded, _rebuild_oauth_clients) + def register_middleware(self, app: FastAPI) -> None: + """Refuse writes from the shared demo session. + + A no-op until ``demo_mode`` and ``demo_read_only`` are both on — it + re-reads them per request — so an install that never hosts a demo pays + one frozenset lookup on unsafe methods and nothing on reads. + """ + from users.demo_guard import DemoReadOnlyMiddleware + + app.add_middleware(DemoReadOnlyMiddleware) + def register_permissions(self, registry: PermissionRegistry) -> None: registry.add_group( "Users", @@ -172,6 +189,7 @@ def locale_dirs(self) -> dict[str, Path]: def register_routes(self, api_router: APIRouter, view_router: APIRouter) -> None: from users.admin.api import admin_router from users.auth_local import api as auth_local_api + from users.auth_local.demo_api import router as demo_router from users.auth_local.token_api import router as token_router from users.auth_local.views import router as auth_views from users.contracts.schemas import UserCreate, UserRead @@ -179,6 +197,7 @@ def register_routes(self, api_router: APIRouter, view_router: APIRouter) -> None from users.oauth.api import register_oauth_routes api_router.include_router(auth_local_api.router) + api_router.include_router(demo_router) api_router.include_router(token_router) api_router.include_router(admin_router) # Throughput-wrap the stock fastapi-users routers; ``require_signup_enabled`` @@ -220,11 +239,14 @@ async def on_startup(self, app: FastAPI) -> None: from users.auth_local.rate_limit import LoginRateLimiter, ThroughputLimiter from users.backend import reconfigure_cookie_transport from users.bootstrap import bootstrap_admin_from_env + from users.demo import ensure_demo_user from users.deps import auth_backend from users.mailer import build_mailer, default_app_name from users.oauth.providers import build_client_map, provider_buttons from users.roles_cache import refresh_roles_cache from users.session_version_cache import configure_session_version_cache + from users.settings import DEFAULT_LOGIN_REDIRECT_URL + from users.startup import apply_login_redirect_fallback, register_mailer_health_check state = app.state.users s = state.settings @@ -237,25 +259,7 @@ def _app_name() -> str: return name or default_app_name() state.mailer = build_mailer(s, _app_name) - - # Registered here rather than in register_health_checks because the - # check needs the app to re-read DB-hydrated settings on every run. - # The owner is passed explicitly since the boot-time set_owner window - # has long closed by startup. - from simple_module_core.health import HealthCheck - - from users.health import CHECK_MAILER, build_mailer_check - - app.state.sm.health_registry.add( - HealthCheck( - name=CHECK_MAILER, - check=build_mailer_check(app), - module=self.meta.name, - # On demand only: this authenticates against the mail provider, - # which must not happen on a readiness-probe timer. - probe=False, - ) - ) + register_mailer_health_check(app, self.meta.name) state.rate_limiter = LoginRateLimiter( max_failures=s.login_rate_limit_failures, window_seconds=s.login_rate_limit_window_seconds, @@ -268,24 +272,7 @@ def _app_name() -> str: state.oauth_clients = build_client_map(s) state.oauth_providers = provider_buttons(state.oauth_clients) - # Auto-fall-back when the default ``/dashboard/`` target is - # unreachable because the Dashboard module isn't installed (e.g. - # ``--preset minimal`` or apps like smpy_gis that omit it). - # Pick the first sibling module that exposes view routes instead - # of hard-coding ``/`` which may itself 404 (#173). Operator-set - # overrides are always preserved. - if s.login_redirect_url == "/dashboard/" and not any( - m.meta.name == "Dashboard" for m in app.state.sm.modules - ): - first_view = next( - ( - m.meta.view_prefix - for m in app.state.sm.modules - if m.meta.view_prefix and m.meta.name != self.meta.name - ), - None, - ) - s.login_redirect_url = f"{first_view}/" if first_view else "/" + apply_login_redirect_fallback(app, s, self.meta.name, DEFAULT_LOGIN_REDIRECT_URL) reconfigure_cookie_transport(auth_backend, s) # The revocation cache's staleness window is an operator choice: the @@ -297,3 +284,7 @@ def _app_name() -> str: bootstrap_admin_from_env(app), refresh_roles_cache(app), ) + # After the env bootstrap, not beside it: that one only runs while the + # users table is empty, and the demo account has to be reconciled on + # every boot so a change to demo_role or demo_password takes effect. + await ensure_demo_user(app) diff --git a/modules/users/users/pages/Login.tsx b/modules/users/users/pages/Login.tsx index b90e1613..f42920eb 100644 --- a/modules/users/users/pages/Login.tsx +++ b/modules/users/users/pages/Login.tsx @@ -4,6 +4,7 @@ import { Button } from '@simple-module-py/ui/components/ui/button'; import { AuthCardShell } from '@simple-module-py/ui/layouts/AuthCardShell'; import { AuthSplitAside } from '@simple-module-py/ui/layouts/AuthSplitAside'; import { useState } from 'react'; +import { DemoSignIn } from '../auth_local/components/DemoSignIn'; import { LoginForm, type OAuthProvider } from '../auth_local/components/LoginForm'; import { WaitingOnYou } from '../auth_local/components/WaitingOnYou'; @@ -13,8 +14,14 @@ interface DevAccount { password: string; } +interface DemoSignInProps { + enabled: boolean; + read_only: boolean; +} + interface Props { allow_signup: boolean; + demo_signin: DemoSignInProps; dev_accounts: DevAccount[]; login_redirect_url: string; oauth_providers: OAuthProvider[]; @@ -22,8 +29,14 @@ interface Props { } function Login() { - const { allow_signup, dev_accounts, login_redirect_url, oauth_providers, remember_me_days } = - usePage<{ props: Props }>().props as unknown as Props; + const { + allow_signup, + demo_signin, + dev_accounts, + login_redirect_url, + oauth_providers, + remember_me_days, + } = usePage<{ props: Props }>().props as unknown as Props; const { t } = useT(); const [email, setEmail] = useState(''); @@ -34,6 +47,8 @@ function Login() { const [resent, setResent] = useState(false); const [resendFailed, setResendFailed] = useState(false); const [loading, setLoading] = useState(false); + const [demoPending, setDemoPending] = useState(false); + const [demoError, setDemoError] = useState(null); // Server-decided, deliberately. The post-login destination used to be read // from `?next=` here, which let any crafted login link bounce the user to an @@ -74,6 +89,31 @@ function Login() { .finally(() => setLoading(false)); }; + // No credentials in the body: the server resolves the shared demo account + // itself, so the page never holds a password it could leak into a bug + // report, a screenshot or the browser's autofill store. + const startDemo = () => { + setDemoError(null); + setError(null); + setDemoPending(true); + fetch('/api/users/auth/demo', { method: 'POST' }) + .then((res) => { + if (res.status === 204) { + router.visit(nextUrl); + return; + } + // 404 is demo mode having been switched off since this page was + // rendered; 429 is the shared throughput budget. Neither is worth its + // own copy — both mean "not right now". + setDemoError(t(keys.users.login.demo_error)); + setDemoPending(false); + }) + .catch(() => { + setDemoError(t(keys.users.common.error_try_again)); + setDemoPending(false); + }); + }; + const handleSubmit = (e: React.FormEvent) => { e.preventDefault(); submitLogin(email, password); @@ -133,6 +173,15 @@ function Login() { /> )} + {demo_signin?.enabled && !needsVerification && ( + + )} + {dev_accounts && dev_accounts.length > 0 && !needsVerification && (

diff --git a/modules/users/users/settings.py b/modules/users/users/settings.py index 25825d44..85efa4d1 100644 --- a/modules/users/users/settings.py +++ b/modules/users/users/settings.py @@ -10,6 +10,7 @@ from __future__ import annotations +import logging import os from pydantic import Field, field_validator, model_validator @@ -19,13 +20,19 @@ from simple_module_core.redirect_safety import non_empty_redirect from simple_module_core.settings_base import DbBackedSettings +from users.constants import ADMIN_ROLE_NAME from users.session_version_cache import SESSION_VERSION_TTL_SECONDS +logger = logging.getLogger("users.settings") + _PLACEHOLDER_RESET_SECRET = "dev-reset-token-secret-change-me" _PLACEHOLDER_VERIFY_SECRET = "dev-verify-token-secret-change-me" DEFAULT_LOGIN_REDIRECT_URL = "/dashboard/" +# Groups the demo fields together in the module-settings editor. +DEMO_SETTINGS_GROUP = "Demo account" + class UsersSettings(DbBackedSettings): """Local user management configuration.""" @@ -112,6 +119,33 @@ def _non_empty_redirect(cls, value: str) -> str: auth_rate_limit_attempts: int = 10 auth_rate_limit_window_seconds: int = 300 + # ── Demo account ──────────────────────────────────────────────────── + # For public showcase instances: one click on the sign-in card signs the + # visitor in as a shared, pre-seeded account. The password is never sent + # to the browser (unlike the dev quick-fill buttons, which are + # development-only and paste real credentials into the form). + demo_mode: bool = Field(default=False, json_schema_extra={"group": DEMO_SETTINGS_GROUP}) + demo_email: str = Field( + default="demo@example.com", json_schema_extra={"group": DEMO_SETTINGS_GROUP} + ) + # Blank means "no password anyone can type": the account is seeded with a + # random one and is reachable only through the demo button. Set it only if + # you also want the credentials published (e.g. for API demos). + demo_password: str = Field(default="", json_schema_extra={"group": DEMO_SETTINGS_GROUP}) + demo_full_name: str = Field( + default="Demo User", json_schema_extra={"group": DEMO_SETTINGS_GROUP} + ) + # Which role the demo account carries. ``admin`` is what shows off the + # admin surface — pair it with ``demo_read_only`` (see the validator below) + # or the first visitor can rewrite the instance's settings. + demo_role: str = Field( + default="user", pattern="^(user|admin)$", json_schema_extra={"group": DEMO_SETTINGS_GROUP} + ) + # Refuse every unsafe HTTP method from a demo session. On by default: + # a demo account is a published credential, so the safe posture is the one + # you get without reading the docs. + demo_read_only: bool = Field(default=True, json_schema_extra={"group": DEMO_SETTINGS_GROUP}) + # Bootstrap (env-var auto-create users on first boot) bootstrap_email: str = "" bootstrap_password: str = "" @@ -148,6 +182,26 @@ def _non_empty_redirect(cls, value: str) -> str: default="common", json_schema_extra={"group": "Microsoft OAuth"} ) + @model_validator(mode="after") + def _warn_on_writable_admin_demo(self) -> UsersSettings: + """A writable admin demo is a public superuser — say so, loudly. + + Not an error: an operator who resets the demo database on a timer has + a legitimate reason to want it, and refusing would make a settings + edit in the admin UI fail with no way to opt in. But the combination + hands anyone who finds the URL the settings editor, the user table and + maintenance mode, so it does not get to happen quietly. + """ + if self.demo_mode and self.demo_role == ADMIN_ROLE_NAME and not self.demo_read_only: + logger.warning( + "users.demo.writable_admin — demo_mode is on with demo_role=%r and " + "demo_read_only=False: anyone who can reach the sign-in page gets a " + "writable administrator session. Set demo_read_only=True unless this " + "instance's database is disposable.", + self.demo_role, + ) + return self + @model_validator(mode="after") def _forbid_placeholder_token_secrets_in_production(self) -> UsersSettings: """Fail boot if the reset/verify token secrets are still placeholders. diff --git a/modules/users/users/shared_props.py b/modules/users/users/shared_props.py index 8af5056f..6993a976 100644 --- a/modules/users/users/shared_props.py +++ b/modules/users/users/shared_props.py @@ -13,17 +13,33 @@ from typing import TYPE_CHECKING +from users.demo_guard import is_demo_session + if TYPE_CHECKING: from starlette.requests import Request def users_shared_props(request: Request) -> dict: - """Whether local self-signup is currently accepted. + """Whether local self-signup is accepted, and whether this is a demo session. Runs on every request, so it only reads already-hydrated state. Defaults to closed: if settings are missing, the safe answer is "no signup link" rather than a link that 404s. + + The ``demo`` block is what lets every shell show a standing banner. It is + per-*session*, not per-install: an operator signed in to their own account + on a demo instance is doing real work and should not be told otherwise, + and it is the demo visitor who needs to know their saves will bounce. """ state = getattr(request.app.state, "users", None) settings = getattr(state, "settings", None) - return {"signup": {"allowed": bool(getattr(settings, "allow_signup", False))}} + demo_active = bool(getattr(settings, "demo_mode", False)) and is_demo_session( + getattr(request, "scope", {}), getattr(state, "demo_user_id", None) + ) + return { + "signup": {"allowed": bool(getattr(settings, "allow_signup", False))}, + "demo": { + "active": demo_active, + "readOnly": demo_active and bool(getattr(settings, "demo_read_only", True)), + }, + } diff --git a/modules/users/users/startup.py b/modules/users/users/startup.py new file mode 100644 index 00000000..fd716daa --- /dev/null +++ b/modules/users/users/startup.py @@ -0,0 +1,67 @@ +"""Two self-contained decisions ``UsersModule.on_startup`` makes. + +Neither is about *sequencing* the boot — which is what the hook itself is for +— so they live here and it reads as a list of steps. Pulled out when the hook +grew past the 300-line file cap and the alternative was to compress the +comments that explain why each one exists. +""" + +from __future__ import annotations + +from typing import TYPE_CHECKING + +if TYPE_CHECKING: + from fastapi import FastAPI + + from users.settings import UsersSettings + +_FALLBACK_REDIRECT = "/" + + +def register_mailer_health_check(app: FastAPI, module_name: str) -> None: + """Add the mailer check to the health registry. + + Registered at startup rather than in ``register_health_checks`` because the + check needs the app to re-read DB-hydrated settings on every run. The owner + is passed explicitly since the boot-time ``set_owner`` window has long + closed by then. + """ + from simple_module_core.health import HealthCheck + + from users.health import CHECK_MAILER, build_mailer_check + + app.state.sm.health_registry.add( + HealthCheck( + name=CHECK_MAILER, + check=build_mailer_check(app), + module=module_name, + # On demand only: this authenticates against the mail provider, + # which must not happen on a readiness-probe timer. + probe=False, + ) + ) + + +def apply_login_redirect_fallback( + app: FastAPI, settings: UsersSettings, module_name: str, default_url: str +) -> None: + """Retarget the default post-login URL when Dashboard isn't installed. + + ``/dashboard/`` is unreachable under ``smpy new --preset minimal`` and in + apps like ``smpy_gis`` that omit the module. Picks the first sibling that + exposes view routes rather than hard-coding ``/``, which may itself 404 + (#173). An operator-set override is never touched. + """ + if settings.login_redirect_url != default_url: + return + if any(m.meta.name == "Dashboard" for m in app.state.sm.modules): + return + first_view = next( + ( + m.meta.view_prefix + for m in app.state.sm.modules + if m.meta.view_prefix and m.meta.name != module_name + ), + None, + ) + settings.login_redirect_url = f"{first_view}/" if first_view else _FALLBACK_REDIRECT diff --git a/modules/users/users/state.py b/modules/users/users/state.py index 5d31ee54..02158230 100644 --- a/modules/users/users/state.py +++ b/modules/users/users/state.py @@ -16,6 +16,8 @@ from typing import TYPE_CHECKING if TYPE_CHECKING: + import uuid + from users.auth_local.rate_limit import LoginRateLimiter, ThroughputLimiter from users.mailer import Mailer from users.oauth.providers import OAuthProvider @@ -34,3 +36,7 @@ class UsersState: roles_cache: list[RoleSummary] = field(default_factory=list) oauth_providers: list[dict[str, str]] = field(default_factory=list) oauth_clients: dict[str, OAuthProvider] = field(default_factory=dict) + # Id of the shared demo account, or None when demo mode is off. Cached at + # boot (and on every settings reload) so ``DemoReadOnlyMiddleware`` can + # recognise a demo session without a database read per request. + demo_user_id: uuid.UUID | None = None diff --git a/packages/i18n/src/generated-resources.ts b/packages/i18n/src/generated-resources.ts index 1d630273..0cb3c0e9 100644 --- a/packages/i18n/src/generated-resources.ts +++ b/packages/i18n/src/generated-resources.ts @@ -694,6 +694,8 @@ export default { 'ui.command_palette.placeholder': '', 'ui.command_palette.title': '', 'ui.command_palette.trigger': '', + 'ui.demo.banner_read_only': '', + 'ui.demo.banner_writable': '', 'ui.errors.generic_description': '', 'ui.errors.generic_title': '', 'ui.errors.go_home_button': '', @@ -914,6 +916,12 @@ export default { 'users.login.aside_check_sso': '', 'users.login.aside_heading': '', 'users.login.continue_with': '', + 'users.login.demo_divider': '', + 'users.login.demo_error': '', + 'users.login.demo_hint_read_only': '', + 'users.login.demo_hint_writable': '', + 'users.login.demo_submit': '', + 'users.login.demo_submitting': '', 'users.login.dev_divider': '', 'users.login.divider_or': '', 'users.login.error_invalid_credentials': '', diff --git a/packages/i18n/src/keys.generated.ts b/packages/i18n/src/keys.generated.ts index 06e38949..9c745975 100644 --- a/packages/i18n/src/keys.generated.ts +++ b/packages/i18n/src/keys.generated.ts @@ -904,6 +904,10 @@ export const keys = { title: 'ui.command_palette.title', trigger: 'ui.command_palette.trigger', }, + demo: { + banner_read_only: 'ui.demo.banner_read_only', + banner_writable: 'ui.demo.banner_writable', + }, errors: { generic_description: 'ui.errors.generic_description', generic_title: 'ui.errors.generic_title', @@ -1173,6 +1177,12 @@ export const keys = { aside_check_sso: 'users.login.aside_check_sso', aside_heading: 'users.login.aside_heading', continue_with: 'users.login.continue_with', + demo_divider: 'users.login.demo_divider', + demo_error: 'users.login.demo_error', + demo_hint_read_only: 'users.login.demo_hint_read_only', + demo_hint_writable: 'users.login.demo_hint_writable', + demo_submit: 'users.login.demo_submit', + demo_submitting: 'users.login.demo_submitting', dev_divider: 'users.login.dev_divider', divider_or: 'users.login.divider_or', error_invalid_credentials: 'users.login.error_invalid_credentials', diff --git a/packages/ui/locales/en.json b/packages/ui/locales/en.json index 48c8b655..70430cb8 100644 --- a/packages/ui/locales/en.json +++ b/packages/ui/locales/en.json @@ -65,5 +65,9 @@ "sign_in": "Sign in", "open_menu": "Open menu", "close_menu": "Close menu" + }, + "demo": { + "banner_read_only": "Demo mode — you are signed in to a shared account and changes are not saved.", + "banner_writable": "Demo mode — you are signed in to a shared account that everyone exploring this site uses." } } diff --git a/packages/ui/locales/es.json b/packages/ui/locales/es.json index 037ca359..be44a9e8 100644 --- a/packages/ui/locales/es.json +++ b/packages/ui/locales/es.json @@ -65,5 +65,9 @@ "sign_in": "Iniciar sesión", "open_menu": "Abrir menú", "close_menu": "Cerrar menú" + }, + "demo": { + "banner_read_only": "Modo demostración: has iniciado sesión en una cuenta compartida y los cambios no se guardan.", + "banner_writable": "Modo demostración: has iniciado sesión en una cuenta compartida que usan todas las personas que exploran este sitio." } } diff --git a/packages/ui/src/components/DemoBanner.tsx b/packages/ui/src/components/DemoBanner.tsx new file mode 100644 index 00000000..2814ab2e --- /dev/null +++ b/packages/ui/src/components/DemoBanner.tsx @@ -0,0 +1,33 @@ +import { usePage } from '@inertiajs/react'; +import { keys, useT } from '@simple-module-py/i18n'; +import type React from 'react'; +import type { SharedProps } from '../types'; + +/** + * Standing "you are in a demo" bar, driven by the `demo` shared prop the users + * module contributes. + * + * Per-session, not per-install: an operator signed in to their own account on + * the same instance is doing real work and is not shown this. It is also what + * makes the read-only guard legible — without it, a refused save on a showcase + * instance is indistinguishable from a bug. + * + * Renders nothing when the viewer is not on a demo session, so layouts mount it + * unconditionally next to `BrandingBanner`. + */ +export function DemoBanner(): React.ReactElement | null { + const { demo } = usePage<{ props: SharedProps }>().props as unknown as SharedProps; + const { t } = useT(); + if (!demo?.active) return null; + + return ( +

+ {demo.readOnly ? t(keys.ui.demo.banner_read_only) : t(keys.ui.demo.banner_writable)} +
+ ); +} diff --git a/packages/ui/src/layouts/SidebarLayout.tsx b/packages/ui/src/layouts/SidebarLayout.tsx index d5a045e3..000c7e72 100644 --- a/packages/ui/src/layouts/SidebarLayout.tsx +++ b/packages/ui/src/layouts/SidebarLayout.tsx @@ -15,6 +15,7 @@ import { BrandingBanner } from '../components/BrandingBanner'; import { BrandingFooter } from '../components/BrandingFooter'; import { BrandingHead } from '../components/BrandingHead'; import { BrandingMark } from '../components/BrandingMark'; +import { DemoBanner } from '../components/DemoBanner'; import { LocaleSwitcher, useHasMultipleLocales } from '../components/LocaleSwitcher'; import { NavIcon } from '../components/NavIcon'; import { PageHeadingProvider, usePageSection } from '../components/page-heading'; @@ -117,6 +118,7 @@ function SidebarShell({ children, menuKey, theme, headerSlot, footerNavSlot }: S + {/* --app-chrome-h names the height of the bar above the content — the topbar on lg, the mobile bar below it, both mutually exclusive. Both bars size themselves off this one variable (h-[var(--app-chrome-h)]) diff --git a/packages/ui/src/types.ts b/packages/ui/src/types.ts index e1e1079b..96901e97 100644 --- a/packages/ui/src/types.ts +++ b/packages/ui/src/types.ts @@ -62,6 +62,11 @@ export interface SharedProps { // auth provider is installed (e.g. a Keycloak-only deployment), which reads // the same as "closed" — the host isn't the one taking signups. signup?: { allowed: boolean }; + // Also from the users module. `active` is true only for the shared demo + // account's own session — an operator signed in normally on a demo instance + // sees nothing. `readOnly` mirrors the `demo_read_only` setting that makes + // `DemoReadOnlyMiddleware` refuse the session's writes. + demo?: { active: boolean; readOnly: boolean }; // Injected by the branding module's shared-props provider (optional: the // module may not be installed). branding?: BrandingShared; From 8c93d997f35f427436445f9d94e88a87feb8159c Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 19 Sep 2026 14:40:36 +0000 Subject: [PATCH 2/3] =?UTF-8?q?feat(users):=20two=20demo=20accounts=20?= =?UTF-8?q?=E2=80=94=20admin=20and=20standard=20user?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reworks demo mode from one shared account into two, because the two halves of the app look nothing alike. The admin surface — users, roles, settings, modules, audit history — is what the framework is *for*, and a visitor who only ever sees the end-user app never meets it; one who only sees /admin/* never meets the app you build for people. The sign-in card now offers both, admin first, and a visitor can switch between them without signing out. Settings replace the single-account fields: `demo_admin_email` / `demo_user_email` and their optional passwords, with `demo_mode` and `demo_read_only` still covering the pair. `demo_role` and `demo_full_name` are gone — the role is now implied by which account, and the display names are seeded constants rather than another knob. Blanking an email switches that account off, closing its route as well as hiding its button: the offer and the endpoint come from the same resolution, so hiding a button can never leave a live way in behind it. `POST /api/users/auth/demo` becomes `POST /api/users/auth/demo/{role}`. The role is a path segment rather than a body field so each account gets its own throughput budget — the limiter keys on path + client IP, so a bot hammering one cannot lock a visitor out of the other — and so an unknown role 404s in routing rather than in the handler. Both sign-in routes are exempt from the read-only guard, since comparing the two surfaces is the point of having two and must not require a sign-out. `ensure_demo_users` seeds and reconciles both; a failure on one no longer withdraws the other. `_sync_role` still drops the role it is replacing, so repointing an address from the admin account to the user one actually demotes the row. The guard matches any of the cached ids, and the banner shared prop is unchanged — per session, so an operator signed in to their own account on a demo instance sees nothing and writes freely. Verified end to end against a booted app: both buttons sign in as the right identity, both raise the banner, both are refused writes with DEMO_READ_ONLY, an unknown role 404s, and blanking `demo_admin_email` closes the admin route and drops its button. Claude-Session: https://claude.ai/code/session_015SeBCuMuwvpfANx9FHv4qn --- docs/modules/users.md | 121 ++++---- modules/users/README.md | 34 ++- modules/users/tests/_demo_support.py | 11 +- modules/users/tests/test_demo_account.py | 262 +++++++----------- modules/users/tests/test_demo_guard.py | 133 +++++---- modules/users/tests/test_demo_login.py | 137 +++++++++ .../users/tests/test_users_shared_props.py | 47 ++-- .../auth_local/components/DemoSignIn.tsx | 89 ++++-- modules/users/users/auth_local/demo_api.py | 49 ++-- modules/users/users/auth_local/views.py | 24 +- modules/users/users/demo.py | 131 +++++---- modules/users/users/demo_guard.py | 50 ++-- modules/users/users/locales/en.json | 11 +- modules/users/users/module.py | 18 +- modules/users/users/pages/Login.tsx | 33 +-- modules/users/users/settings.py | 52 ++-- modules/users/users/shared_props.py | 2 +- modules/users/users/state.py | 4 +- packages/i18n/src/generated-resources.ts | 5 +- packages/i18n/src/keys.generated.ts | 5 +- 20 files changed, 725 insertions(+), 493 deletions(-) create mode 100644 modules/users/tests/test_demo_login.py diff --git a/docs/modules/users.md b/docs/modules/users.md index c95dd9f8..2934735f 100644 --- a/docs/modules/users.md +++ b/docs/modules/users.md @@ -33,7 +33,7 @@ The module is built on [`fastapi-users`](https://fastapi-users.github.io/) for p | `POST /api/users/auth/request-verify-token` | `RequestVerifyToken` | rate-limited | | `POST /api/users/auth/verify` | `VerifyRequest` | | | `POST /api/users/auth/accept-invite` | `AcceptInviteRequest` | sets password + signs the user in | -| `POST /api/users/auth/demo` | — | one-click sign-in as the shared demo account; `404` unless `demo_mode` is on; rate-limited — see [Demo mode](#demo-mode-hosting-a-showcase-instance) | +| `POST /api/users/auth/demo/{role}` | — | one-click sign-in as a shared demo account (`role` ∈ `admin`, `user`); `404` unless that account is configured; rate-limited per role — see [Demo mode](#demo-mode-hosting-a-showcase-instance) | | `POST /api/users/auth/token` | `TokenRequest` (email + password) | bearer login for mobile / API clients → `{access_token, refresh_token, token_type, expires_in}`; `401` for external/SSO users | | `POST /api/users/auth/token/refresh` | `RefreshRequest` (refresh_token) | rotates a refresh token into a new pair (old one revoked) | | `DELETE /api/users/auth/token` | `RefreshRequest` (refresh_token) | revokes a refresh token (idempotent) | @@ -197,10 +197,9 @@ Everything else is DB-backed (initial values are pydantic defaults; edit under U | `auth_rate_limit_window_seconds` | `300` | | `bootstrap_email`, `bootstrap_password`, `bootstrap_user_email`, `bootstrap_user_password` | `""` — see [Bootstrap](#bootstrap-the-first-admin) | | `demo_mode` | `False` — see [Demo mode](#demo-mode-hosting-a-showcase-instance) | -| `demo_email` | `"demo@example.com"` | -| `demo_password` | `""` (a random one is generated, so the account is reachable only through the button) | -| `demo_full_name` | `"Demo User"` | -| `demo_role` | `"user"` (or `"admin"`) | +| `demo_admin_email` | `"demo-admin@example.com"` — blank to stop offering the admin demo | +| `demo_user_email` | `"demo-user@example.com"` — blank to stop offering the standard-user demo | +| `demo_admin_password` / `demo_user_password` | `""` (a random one is generated, so the account is reachable only through its button) | | `demo_read_only` | `True` | | `oauth_google_client_id` / `oauth_google_client_secret` | `""` — Google OAuth | | `oauth_github_client_id` / `oauth_github_client_secret` | `""` — GitHub OAuth | @@ -261,69 +260,91 @@ Two paths to seed the first admin: ## Demo mode (hosting a showcase instance) -Turn `demo_mode` on and the sign-in card grows an **Explore the demo** button. -One click signs the visitor in as a shared, pre-seeded account — no email, no -password, no signup. +Turn `demo_mode` on and the sign-in card grows a button per demo account. +One click signs the visitor straight in — no email, no password, no signup. ``` -users.demo_mode = true -users.demo_email = demo@example.com -users.demo_role = admin # what actually shows off the framework -users.demo_read_only = true # default; leave it on +users.demo_mode = true +users.demo_admin_email = demo-admin@example.com +users.demo_user_email = demo-user@example.com +users.demo_read_only = true # default; leave it on ``` Set these under Users at `/admin/settings/`, or seed them in the settings store -before first boot. Changes apply live — the `SettingsReloaded` handler -re-seeds the account and refreshes the cached id, so there is no restart. +before first boot. Changes apply live — the `SettingsReloaded` handler re-seeds +the accounts and refreshes the cached ids, so there is no restart. + +### Two accounts, not one + +An **administrator** and an ordinary **standard user**, because the two halves +of the app look nothing alike. The admin surface — users, roles, settings, +modules, audit history — is what the framework is *for*, and a visitor who +only ever sees the end-user app never meets it. One who only ever sees +`/admin/*` never meets the app you actually build for people. Offering both, +and letting someone switch between them without signing out, is what makes a +showcase instance answer "what is this?" in one visit. + +Each is independently switchable: blank an email and that button disappears +*and* its route starts answering 404 — the offer and the endpoint come from +the same resolution, so hiding a button never leaves a live way in behind it. +Blank both and `demo_mode` has nothing to turn on (logged, since you plainly +meant something by switching it on). ### What it does -- **Seeds the account** on every boot and every settings reload - (`users.demo.ensure_demo_user`). Idempotent, and it *reconciles*: changing - `demo_role` from `admin` back to `user` demotes the existing row and drops - the admin grant rather than leaving it in place. -- **Never sends the password to the browser.** The button posts to - `POST /api/users/auth/demo` with an empty body and the server resolves the - account itself. This is the difference between demo mode and the dev - quick-login buttons, which paste real credentials into the form and are - therefore development-only. Leave `demo_password` blank and the account is - seeded with a random secret nobody — including you — can type; set it only - if you also intend to publish the credentials (for an API demo, say). +- **Seeds the accounts** on every boot and every settings reload + (`users.demo.ensure_demo_users`). Idempotent, and it *reconciles*: the admin + row carries `is_superuser` and the `admin` role, the user row carries + neither, and repointing an email from one to the other demotes the row + rather than leaving the old grant in place. One account failing to seed does + not withdraw the other. +- **Never sends a password to the browser.** Each button posts to + `POST /api/users/auth/demo/{role}` with an empty body and the server + resolves the account itself. This is the difference between demo mode and + the dev quick-login buttons, which paste real credentials into the form and + are therefore development-only. Leave a `demo_*_password` blank and that + account is seeded with a random secret nobody — including you — can type; + set one only if you also intend to publish the credentials (for an API + demo, say). - **Refuses writes** while `demo_read_only` is on. - `DemoReadOnlyMiddleware` rejects every unsafe HTTP method from a demo - session with `403 {"detail": "DEMO_READ_ONLY"}`, except signing out. Inertia - requests get the protocol's `409` + `X-Inertia-Location` instead, so a - blocked save re-renders the page rather than throwing up an error modal. + `DemoReadOnlyMiddleware` rejects every unsafe HTTP method from either demo + session with `403 {"detail": "DEMO_READ_ONLY"}`, except signing out and the + demo sign-in routes themselves — switching between the two accounts is the + point of having two, so it must not require a sign-out. Inertia requests get + the protocol's `409` + `X-Inertia-Location` instead, so a blocked save + re-renders the page rather than throwing up an error modal. - **Says so.** `DemoBanner` renders a standing bar in every app shell, driven by the `demo` shared prop. It is **per session**, not per install — you, signed in to your own account on the same instance, do not see it, and your writes are not touched. -### `demo_role = "admin"` and `demo_read_only = false` +### `demo_read_only = false` -This combination hands anyone who can reach your sign-in page a writable -superuser: the settings editor (including the SMTP password and OAuth client -secrets), the user table, and maintenance mode. It is allowed — an instance -whose database is rebuilt on a timer has a legitimate reason to want a -writable demo — but it logs `users.demo.writable_admin` at WARNING on every -settings load. Do not run it against a database you care about. +With an admin demo configured, this hands anyone who can reach your sign-in +page a writable superuser: the settings editor (including the SMTP password +and OAuth client secrets), the user table, and maintenance mode. It is allowed +— an instance whose database is rebuilt on a timer has a legitimate reason to +want a writable demo — but it logs `users.demo.writable_admin` at WARNING on +every settings load. Do not run it against a database you care about. Blanking +`demo_admin_email` is the way to have a writable demo without that exposure. ### Limits worth knowing -- The account is **shared**. Two visitors exploring at once see each other's - state, and with `demo_read_only = false` they can overwrite each other. - Periodically resetting the database is the only real answer. -- `demo_read_only` matches on the **session** — the stamp the demo endpoint - writes, or a session whose `user_id` is the demo account's. A bearer token - is not covered directly, but minting one is itself a `POST`, so a read-only - demo cannot get hold of one. -- The endpoint shares the `auth_rate_limit_*` budget with the other - credential-adjacent routes. Each click mints a session row, so a demo - instance under real traffic may want that raised. -- With `demo_role = "admin"`, the demo account **satisfies the first-run setup - gate** — an administrator exists, so `/setup` never appears. Seed your own - admin first (`smpy users create-admin`, or the `SM_USERS_BOOTSTRAP_*` vars), - because a read-only demo session cannot complete the wizard. +- The accounts are **shared**. Two visitors exploring the same one at once see + each other's state, and with `demo_read_only = false` they can overwrite + each other. Periodically resetting the database is the only real answer. +- `demo_read_only` matches on the **session** — the stamp a demo endpoint + writes, or a session whose `user_id` is one of the demo accounts'. A bearer + token is not covered directly, but minting one is itself a `POST`, so a + read-only demo cannot get hold of one. +- Each role's endpoint has its own `auth_rate_limit_*` budget (the limiter + keys on path + client IP), so a bot hammering one cannot lock a visitor out + of the other. Each click mints a session row, so a demo instance under real + traffic may want the budget raised. +- The demo **admin satisfies the first-run setup gate** — an administrator + exists, so `/setup` never appears. Seed your own admin first + (`smpy users create-admin`, or the `SM_USERS_BOOTSTRAP_*` vars), because a + read-only demo session cannot complete the wizard. ## Mailer backends diff --git a/modules/users/README.md b/modules/users/README.md index 3b1499f8..fb6d7e3b 100644 --- a/modules/users/README.md +++ b/modules/users/README.md @@ -19,7 +19,7 @@ Pre-wired into any app scaffolded with `smpy new`. - `smpy users create-admin` CLI for ad-hoc admin creation. - Inertia pages for login/register/invite-accept/admin-invite. - Console mailer (logs to stdout) or SMTP mailer (`SM_USERS_MAILER=smtp`). -- Demo mode — a one-click, read-only shared account on the sign-in card, for public showcase instances. +- Demo mode — one-click, read-only shared **admin** and **standard user** accounts on the sign-in card, for public showcase instances. ## Usage @@ -57,21 +57,27 @@ async def profile(user: CurrentUser): For hosting a public showcase instance. Set under **/admin/settings/ → Users**: ``` -demo_mode = true -demo_email = demo@example.com -demo_role = admin # "user" by default -demo_read_only = true # default — keep it on +demo_mode = true +demo_admin_email = demo-admin@example.com +demo_user_email = demo-user@example.com +demo_read_only = true # default — keep it on ``` -The sign-in card grows an **Explore the demo** button that posts to -`POST /api/users/auth/demo`; the account's password never reaches the browser. -The account is seeded (and reconciled) at boot and on every settings reload, -and while `demo_read_only` is on, `DemoReadOnlyMiddleware` refuses every unsafe -HTTP method from the demo session — everyone else on the instance is unaffected. -A standing banner tells the visitor they are in a demo. - -Full detail, including the `demo_role = "admin"` + `demo_read_only = false` -warning, is in [docs/modules/users.md](../../docs/modules/users.md#demo-mode-hosting-a-showcase-instance). +The sign-in card grows one button per configured account — **Explore as an +administrator** and **Explore as a standard user** — each posting to +`POST /api/users/auth/demo/{role}`. Neither account's password reaches the +browser. Two accounts because the admin surface is what the framework is for +and the end-user app is what you build with it; a visitor can switch between +them without signing out. Blank either email to offer only the other; that +closes its route too, not just its button. + +Both accounts are seeded (and reconciled) at boot and on every settings +reload, and while `demo_read_only` is on, `DemoReadOnlyMiddleware` refuses +every unsafe HTTP method from either demo session — everyone else on the +instance is unaffected. A standing banner tells the visitor they are in a demo. + +Full detail, including the `demo_read_only = false` warning, is in +[docs/modules/users.md](../../docs/modules/users.md#demo-mode-hosting-a-showcase-instance). ## Social sign-in (Google, GitHub, Microsoft, OIDC) diff --git a/modules/users/tests/_demo_support.py b/modules/users/tests/_demo_support.py index d3c08789..fe411e68 100644 --- a/modules/users/tests/_demo_support.py +++ b/modules/users/tests/_demo_support.py @@ -9,24 +9,25 @@ import httpx import pytest from _users_app_builders import _build_users_app -from users.demo import ensure_demo_user +from users.demo import ensure_demo_users -DEMO_EMAIL = "demo@example.com" +DEMO_ADMIN_EMAIL = "demo-admin@example.com" +DEMO_USER_EMAIL = "demo-user@example.com" @pytest.fixture async def demo_app(monkeypatch): - """A users app with demo mode on and the demo account seeded.""" + """A users app with demo mode on and both demo accounts seeded.""" application, ctx = await _build_users_app(monkeypatch, allow_signup=False) application.state.users.settings.demo_mode = True - await ensure_demo_user(application) + await ensure_demo_users(application) yield application await ctx.__aexit__(None, None, None) @pytest.fixture async def demo_client(demo_app): - """Anonymous client against ``demo_app`` — it signs itself in via the button.""" + """Anonymous client against ``demo_app`` — it signs itself in via a button.""" transport = httpx.ASGITransport(app=demo_app) async with httpx.AsyncClient(transport=transport, base_url="http://testserver") as client: yield client diff --git a/modules/users/tests/test_demo_account.py b/modules/users/tests/test_demo_account.py index b468a4cf..4ab414df 100644 --- a/modules/users/tests/test_demo_account.py +++ b/modules/users/tests/test_demo_account.py @@ -1,21 +1,22 @@ -"""Demo account: how it is configured, seeded, and entered. +"""Demo accounts: how they are configured, seeded, and entered. -Two of the ways the feature can fail open live here — a demo button that -appears when no account is configured, and an endpoint that signs someone in -when the feature is off. The third, a demo session that is allowed to write, -is ``test_demo_guard``. +Two of the ways the feature can fail open live here — buttons that appear when +no account is configured, and an endpoint that signs someone in when the +feature is off. The third, a demo session that is allowed to write, is +``test_demo_guard``. """ from __future__ import annotations -from _demo_support import DEMO_EMAIL +from _demo_support import DEMO_ADMIN_EMAIL, DEMO_USER_EMAIL from sqlalchemy import select from users.constants import ADMIN_ROLE_NAME, USER_ROLE_NAME from users.demo import ( DemoAccount, - ensure_demo_user, + ensure_demo_users, reconcile_demo_user, resolve_demo_account, + resolve_demo_accounts, ) from users.models import Role, User, UserRole from users.settings import UsersSettings @@ -29,210 +30,153 @@ def _settings(**overrides) -> UsersSettings: return UsersSettings(**base, **overrides) -# ── resolve_demo_account ──────────────────────────────────────────────────── +# ── resolve_demo_accounts ─────────────────────────────────────────────────── def test_demo_is_off_by_default(): - assert resolve_demo_account(_settings()) is None + assert resolve_demo_accounts(_settings()) == () -def test_blank_email_reads_as_off_not_as_an_error(): - """A half-finished edit in the admin UI must not break the sign-in page.""" - assert resolve_demo_account(_settings(demo_mode=True, demo_email=" ")) is None - - -def test_an_unknown_role_falls_back_to_the_standard_one(): - """Never silently upgrade: an unrecognised value must not mean admin. - - ``demo_role`` carries a pydantic pattern, so this can only come from a - value written straight into the settings table — which is exactly the case - the fallback is there for, and why it is tested through ``model_construct`` - rather than the validated constructor. - """ - raw = UsersSettings.model_construct( - demo_mode=True, - demo_email=DEMO_EMAIL, - demo_password="", - demo_full_name="Demo User", - demo_role="superuser", - demo_read_only=True, - ) - account = resolve_demo_account(raw) - assert account is not None - assert account.role == USER_ROLE_NAME - - -def test_read_only_is_the_default_posture(): - account = resolve_demo_account(_settings(demo_mode=True)) - assert account is not None and account.read_only is True - - -# ── seeding ───────────────────────────────────────────────────────────────── - - -async def _demo_row(app) -> User | None: - async with app.state.sm.db.session_factory() as session: - return ( - await session.execute(select(User).where(User.email == DEMO_EMAIL)) - ).scalar_one_or_none() - - -async def test_ensure_demo_user_seeds_the_account(users_app): - users_app.state.users.settings.demo_mode = True - user_id = await ensure_demo_user(users_app) - - assert user_id is not None - assert users_app.state.users.demo_user_id == user_id - row = await _demo_row(users_app) - assert row is not None - assert row.is_active and row.is_verified and not row.is_superuser +def test_both_accounts_are_offered_admin_first(): + """Admin first: it is the surface someone evaluating the framework came + to see, and button order is the only thing that says so.""" + accounts = resolve_demo_accounts(_settings(demo_mode=True)) + assert [a.role for a in accounts] == [ADMIN_ROLE_NAME, USER_ROLE_NAME] + assert [a.email for a in accounts] == [DEMO_ADMIN_EMAIL, DEMO_USER_EMAIL] -async def test_a_blank_password_still_produces_a_usable_account(users_app): - """The button does not need a password; the row still needs a hash.""" - users_app.state.users.settings.demo_mode = True - await ensure_demo_user(users_app) - row = await _demo_row(users_app) - assert row is not None and row.hashed_password +def test_blanking_one_email_offers_only_the_other(): + accounts = resolve_demo_accounts(_settings(demo_mode=True, demo_admin_email=" ")) + assert [a.role for a in accounts] == [USER_ROLE_NAME] -async def test_a_generated_password_is_not_re_rolled_on_every_boot(users_app): - """Re-hashing a secret nobody can use would write an audit entry per - worker per restart, for no gain.""" - users_app.state.users.settings.demo_mode = True - await ensure_demo_user(users_app) - first = (await _demo_row(users_app)).hashed_password - await ensure_demo_user(users_app) +def test_blanking_both_emails_reads_as_off_not_as_an_error(): + """A half-finished edit in the admin UI must not break the sign-in page.""" + assert ( + resolve_demo_accounts(_settings(demo_mode=True, demo_admin_email="", demo_user_email="")) + == () + ) - assert (await _demo_row(users_app)).hashed_password == first +def test_resolve_by_role_finds_each_account(): + settings = _settings(demo_mode=True) -async def test_a_configured_password_is_reapplied(users_app): - """Changing demo_password in the admin UI has to take effect.""" - users_app.state.users.settings.demo_mode = True - await ensure_demo_user(users_app) - first = (await _demo_row(users_app)).hashed_password + assert resolve_demo_account(settings, ADMIN_ROLE_NAME).email == DEMO_ADMIN_EMAIL + assert resolve_demo_account(settings, USER_ROLE_NAME).email == DEMO_USER_EMAIL + assert resolve_demo_account(settings, "superuser") is None - users_app.state.users.settings.demo_password = "a-published-demo-password" - await ensure_demo_user(users_app) - assert (await _demo_row(users_app)).hashed_password != first +def test_read_only_is_the_default_posture(): + assert _settings(demo_mode=True).demo_read_only is True -async def test_demo_off_clears_the_cached_id(users_app): - users_app.state.users.settings.demo_mode = True - await ensure_demo_user(users_app) - users_app.state.users.settings.demo_mode = False - - assert await ensure_demo_user(users_app) is None - assert users_app.state.users.demo_user_id is None +# ── seeding ───────────────────────────────────────────────────────────────── -async def test_demoting_the_role_drops_the_admin_grant(users_app): - """Flipping demo_role back is how an operator revokes a too-open demo.""" - async with users_app.state.sm.db.session_factory() as session: - account = DemoAccount( - email=DEMO_EMAIL, password="x", full_name="Demo", role=ADMIN_ROLE_NAME, read_only=True - ) - user = await reconcile_demo_user(session, account) - assert user.is_superuser +async def _row(app, email: str) -> User | None: + async with app.state.sm.db.session_factory() as session: + return (await session.execute(select(User).where(User.email == email))).scalar_one_or_none() - await reconcile_demo_user( - session, - DemoAccount( - email=DEMO_EMAIL, - password="x", - full_name="Demo", - role=USER_ROLE_NAME, - read_only=True, - ), - ) - async with users_app.state.sm.db.session_factory() as session: - roles = ( +async def _role_names(app, user_id) -> list[str]: + async with app.state.sm.db.session_factory() as session: + return list( ( await session.execute( select(Role.name) .join(UserRole, UserRole.role_id == Role.id) - .where(UserRole.user_id == user.id) + .where(UserRole.user_id == user_id) ) ) .scalars() .all() ) - refreshed = await session.get(User, user.id) - assert roles == [USER_ROLE_NAME] - assert refreshed is not None and not refreshed.is_superuser - -# ── endpoint + login page ─────────────────────────────────────────────────── +async def test_ensure_demo_users_seeds_both_accounts(users_app): + users_app.state.users.settings.demo_mode = True -async def test_the_endpoint_is_absent_when_demo_mode_is_off(anon_client): - res = await anon_client.post("/api/users/auth/demo") - assert res.status_code == 404 + ids = await ensure_demo_users(users_app) + assert len(ids) == 2 + assert users_app.state.users.demo_user_ids == ids + admin = await _row(users_app, DEMO_ADMIN_EMAIL) + user = await _row(users_app, DEMO_USER_EMAIL) + assert admin is not None and admin.is_superuser + assert user is not None and not user.is_superuser + assert all(row.is_active and row.is_verified for row in (admin, user)) -async def test_the_login_page_advertises_no_demo_by_default(anon_client): - res = await anon_client.get("/users/login", headers={"X-Inertia": "true"}) - assert res.status_code == 200 - assert res.json()["props"]["demo_signin"] == {"enabled": False, "read_only": False} +async def test_each_account_gets_its_own_role(users_app): + users_app.state.users.settings.demo_mode = True + await ensure_demo_users(users_app) -async def test_the_login_page_advertises_the_demo(demo_client): - res = await demo_client.get("/users/login", headers={"X-Inertia": "true"}) - assert res.json()["props"]["demo_signin"] == {"enabled": True, "read_only": True} + admin = await _row(users_app, DEMO_ADMIN_EMAIL) + user = await _row(users_app, DEMO_USER_EMAIL) + assert await _role_names(users_app, admin.id) == [ADMIN_ROLE_NAME] + assert await _role_names(users_app, user.id) == [USER_ROLE_NAME] -async def test_the_login_page_never_leaks_the_demo_password(demo_app, demo_client): - demo_app.state.users.settings.demo_password = "sup3r-s3cret-demo" - await ensure_demo_user(demo_app) +async def test_a_blank_password_still_produces_usable_accounts(users_app): + """The buttons do not need passwords; the rows still need hashes.""" + users_app.state.users.settings.demo_mode = True + await ensure_demo_users(users_app) - res = await demo_client.get("/users/login", headers={"X-Inertia": "true"}) - assert "sup3r-s3cret-demo" not in res.text + assert (await _row(users_app, DEMO_ADMIN_EMAIL)).hashed_password + assert (await _row(users_app, DEMO_USER_EMAIL)).hashed_password -async def test_a_disabled_demo_account_is_not_a_way_in(demo_app, demo_client): - """Disabling the row in the admin UI has to actually stop the button.""" - from datetime import UTC, datetime +async def test_a_generated_password_is_not_re_rolled_on_every_boot(users_app): + """Re-hashing a secret nobody can use would write an audit entry per + worker per restart, for no gain.""" + users_app.state.users.settings.demo_mode = True + await ensure_demo_users(users_app) + first = (await _row(users_app, DEMO_ADMIN_EMAIL)).hashed_password - async with demo_app.state.sm.db.session_factory() as session: - row = await session.get(User, demo_app.state.users.demo_user_id) - row.is_active = False - row.disabled_at = datetime.now(UTC) - await session.commit() + await ensure_demo_users(users_app) - assert (await demo_client.post("/api/users/auth/demo")).status_code == 404 + assert (await _row(users_app, DEMO_ADMIN_EMAIL)).hashed_password == first -async def test_one_click_signs_the_visitor_in(demo_client): - res = await demo_client.post("/api/users/auth/demo") - assert res.status_code == 204 +async def test_a_configured_password_is_reapplied(users_app): + """Changing a demo password in the admin UI has to take effect.""" + users_app.state.users.settings.demo_mode = True + await ensure_demo_users(users_app) + first = (await _row(users_app, DEMO_USER_EMAIL)).hashed_password - me = await demo_client.get("/api/users/me") - assert me.status_code == 200 - assert me.json()["email"] == DEMO_EMAIL + users_app.state.users.settings.demo_user_password = "a-published-demo-password" + await ensure_demo_users(users_app) + assert (await _row(users_app, DEMO_USER_EMAIL)).hashed_password != first -async def test_signing_in_turns_the_banner_on(demo_client): - """The shared ``demo`` prop is what every shell reads to say "this is a - demo". Before the button it is inactive; after it, it is not.""" - before = (await demo_client.get("/users/login", headers={"X-Inertia": "true"})).json() - assert before["props"]["demo"] == {"active": False, "readOnly": False} - await demo_client.post("/api/users/auth/demo") +async def test_demo_off_clears_the_cached_ids(users_app): + users_app.state.users.settings.demo_mode = True + await ensure_demo_users(users_app) + users_app.state.users.settings.demo_mode = False - after = (await demo_client.get("/users/login", headers={"X-Inertia": "true"})).json() - assert after["props"]["demo"] == {"active": True, "readOnly": True} + assert await ensure_demo_users(users_app) == () + assert users_app.state.users.demo_user_ids == () -async def test_the_offer_and_the_session_are_separate_props(demo_client): - """Page props win the Inertia merge, so a page prop named ``demo`` would - shadow the banner's shared prop on this page alone — a collision that - breaks quietly and only here.""" - props = (await demo_client.get("/users/login", headers={"X-Inertia": "true"})).json()["props"] +async def test_repointing_an_email_demotes_the_account_that_held_it(users_app): + """An address that served as the demo admin and is then configured as the + demo user must lose the admin grant, or the demotion is cosmetic.""" + shared = "demo@example.com" + async with users_app.state.sm.db.session_factory() as session: + user = await reconcile_demo_user( + session, + DemoAccount(role=ADMIN_ROLE_NAME, email=shared, password="x", full_name="Demo"), + ) + assert user.is_superuser + await reconcile_demo_user( + session, + DemoAccount(role=USER_ROLE_NAME, email=shared, password="x", full_name="Demo"), + ) - assert set(props["demo"]) == {"active", "readOnly"} - assert set(props["demo_signin"]) == {"enabled", "read_only"} + assert await _role_names(users_app, user.id) == [USER_ROLE_NAME] + async with users_app.state.sm.db.session_factory() as session: + refreshed = await session.get(User, user.id) + assert not refreshed.is_superuser diff --git a/modules/users/tests/test_demo_guard.py b/modules/users/tests/test_demo_guard.py index 03eb3c9e..e88e812b 100644 --- a/modules/users/tests/test_demo_guard.py +++ b/modules/users/tests/test_demo_guard.py @@ -1,117 +1,132 @@ """``DemoReadOnlyMiddleware`` — what a shared demo session may and may not do. -Split from ``test_demo_account`` (which covers configuring and seeding the -account) because this is the security boundary: every test here is a way the -guard could fail open. +Split from the configuration and entry suites because this is the security +boundary: every test here is a way the guard could fail open. """ from __future__ import annotations import httpx -from _demo_support import DEMO_EMAIL +from users.constants import ADMIN_ROLE_NAME, SESSION_USER_ID_KEY, USER_ROLE_NAME from users.demo import SESSION_DEMO_KEY from users.demo_guard import DEMO_READ_ONLY_DETAIL -# ── read-only guard ───────────────────────────────────────────────────────── +def _demo_route(role: str) -> str: + return f"/api/users/auth/demo/{role}" -async def test_a_demo_session_cannot_write(demo_client): - await demo_client.post("/api/users/auth/demo") + +def _client(app, session_payload: dict) -> httpx.AsyncClient: + """A client carrying a forged session cookie for ``session_payload``.""" + from simple_module_test import forge_session_cookie + + cookie = forge_session_cookie(str(app.state.sm.settings.secret_key), session_payload) + return httpx.AsyncClient( + transport=httpx.ASGITransport(app=app), + base_url="http://testserver", + cookies={"session": cookie}, + ) + + +# ── both demo sessions are read-only ──────────────────────────────────────── + + +async def test_the_admin_demo_cannot_write(demo_client): + await demo_client.post(_demo_route(ADMIN_ROLE_NAME)) + + res = await demo_client.patch("/api/users/me", json={"full_name": "Owned"}) + assert res.status_code == 403 + assert res.json()["detail"] == DEMO_READ_ONLY_DETAIL + + +async def test_the_user_demo_cannot_write(demo_client): + await demo_client.post(_demo_route(USER_ROLE_NAME)) res = await demo_client.patch("/api/users/me", json={"full_name": "Owned"}) assert res.status_code == 403 assert res.json()["detail"] == DEMO_READ_ONLY_DETAIL +async def test_the_admin_demo_cannot_reach_the_admin_write_routes(demo_app): + """The showcase admin may browse /admin/*, never mutate through it — that + is the whole reason an admin demo is safe to publish.""" + target = demo_app.state.users.demo_user_ids[0] + async with _client( + demo_app, {SESSION_USER_ID_KEY: str(target), SESSION_DEMO_KEY: True} + ) as client: + res = await client.delete(f"/api/users/admin/{target}") + + assert res.status_code == 403 + assert res.json()["detail"] == DEMO_READ_ONLY_DETAIL + + async def test_a_demo_session_can_still_read(demo_client): - await demo_client.post("/api/users/auth/demo") + await demo_client.post(_demo_route(ADMIN_ROLE_NAME)) assert (await demo_client.get("/api/users/me")).status_code == 200 async def test_a_demo_session_can_still_sign_out(demo_client): - await demo_client.post("/api/users/auth/demo") + await demo_client.post(_demo_route(ADMIN_ROLE_NAME)) assert (await demo_client.post("/api/users/auth/logout")).status_code == 204 +async def test_an_unstamped_session_on_a_demo_account_still_blocks(demo_app): + """The stamp is sufficient, not necessary. + + Matching on the user ids too is what catches a visitor who signed in with + a published demo password through the ordinary form — that session never + passes through a demo endpoint, so it carries no stamp. + """ + payload = {SESSION_USER_ID_KEY: str(demo_app.state.users.demo_user_ids[1])} + assert SESSION_DEMO_KEY not in payload + + async with _client(demo_app, payload) as client: + res = await client.patch("/api/users/me", json={"full_name": "Owned"}) + + assert res.status_code == 403 + + async def test_an_inertia_write_gets_a_hard_redirect_not_a_raw_403(demo_client): """Inertia renders a non-Inertia body as a modal — on a showcase instance that reads as a crash. The protocol's own answer is 409 + Location.""" - await demo_client.post("/api/users/auth/demo") + await demo_client.post(_demo_route(ADMIN_ROLE_NAME)) res = await demo_client.patch( "/api/users/me", json={"full_name": "Owned"}, headers={"X-Inertia": "true", "Referer": "http://testserver/users/me"}, ) + assert res.status_code == 409 assert res.headers["X-Inertia-Location"] == "http://testserver/users/me" +# ── it leaves everyone else alone ─────────────────────────────────────────── + + async def test_writes_are_allowed_when_read_only_is_off(demo_app, demo_client): demo_app.state.users.settings.demo_read_only = False - await demo_client.post("/api/users/auth/demo") + await demo_client.post(_demo_route(USER_ROLE_NAME)) res = await demo_client.patch("/api/users/me", json={"full_name": "Explorer"}) assert res.status_code == 200 -async def test_the_guard_leaves_other_accounts_alone(demo_app): +async def test_the_guard_leaves_real_accounts_alone(demo_app): """A real administrator on a demo instance is doing real work. Goes through an admin route rather than ``/api/users/me``: that one authenticates off the fastapi-users cookie, which a forged session cookie does not carry, so a 401 there would prove nothing about the guard. """ + from _demo_support import DEMO_USER_EMAIL from _users_app_builders import _make_admin_user - from simple_module_test import forge_session_cookie admin = await _make_admin_user(demo_app) - cookie = forge_session_cookie( - str(demo_app.state.sm.settings.secret_key), {"user_id": str(admin.id)} - ) - transport = httpx.ASGITransport(app=demo_app) - async with httpx.AsyncClient( - transport=transport, base_url="http://testserver", cookies={"session": cookie} - ) as client: + async with _client(demo_app, {SESSION_USER_ID_KEY: str(admin.id)}) as client: res = await client.patch( - f"/api/users/admin/{demo_app.state.users.demo_user_id}", - json={"email": DEMO_EMAIL, "full_name": "Renamed By A Real Admin"}, + f"/api/users/admin/{demo_app.state.users.demo_user_ids[1]}", + json={"email": DEMO_USER_EMAIL, "full_name": "Renamed By A Real Admin"}, ) - assert res.status_code == 200 - - -async def test_a_demo_session_cannot_reach_the_admin_write_routes(demo_app): - """The showcase account may browse /admin/*, never mutate through it.""" - from simple_module_test import forge_session_cookie - - cookie = forge_session_cookie( - str(demo_app.state.sm.settings.secret_key), - {"user_id": str(demo_app.state.users.demo_user_id), SESSION_DEMO_KEY: True}, - ) - transport = httpx.ASGITransport(app=demo_app) - async with httpx.AsyncClient( - transport=transport, base_url="http://testserver", cookies={"session": cookie} - ) as client: - res = await client.delete(f"/api/users/admin/{demo_app.state.users.demo_user_id}") - assert res.status_code == 403 - assert res.json()["detail"] == DEMO_READ_ONLY_DETAIL - - -async def test_an_unstamped_session_on_the_demo_account_still_blocks(demo_app): - """The stamp is sufficient, not necessary. - - Matching on the user id too is what catches a visitor who signed in with a - published ``demo_password`` through the ordinary form — that session never - passes through the demo endpoint, so it carries no stamp. - """ - from simple_module_test import forge_session_cookie - payload = {"user_id": str(demo_app.state.users.demo_user_id)} - assert SESSION_DEMO_KEY not in payload - cookie = forge_session_cookie(str(demo_app.state.sm.settings.secret_key), payload) - transport = httpx.ASGITransport(app=demo_app) - async with httpx.AsyncClient( - transport=transport, base_url="http://testserver", cookies={"session": cookie} - ) as client: - res = await client.patch("/api/users/me", json={"full_name": "Owned"}) - assert res.status_code == 403 + assert res.status_code == 200 diff --git a/modules/users/tests/test_demo_login.py b/modules/users/tests/test_demo_login.py new file mode 100644 index 00000000..872d2bf0 --- /dev/null +++ b/modules/users/tests/test_demo_login.py @@ -0,0 +1,137 @@ +"""The one-click demo sign-ins, and what the login page advertises. + +Split from ``test_demo_account`` (configuration and seeding) and +``test_demo_guard`` (the read-only boundary): this is the entry itself — +the route, what it will and will not sign you in as, and the props the +sign-in card is drawn from. +""" + +from __future__ import annotations + +from _demo_support import DEMO_ADMIN_EMAIL, DEMO_USER_EMAIL +from users.constants import ADMIN_ROLE_NAME, USER_ROLE_NAME +from users.demo import ensure_demo_users +from users.models import User + +_LOGIN_PAGE = "/users/login" +_INERTIA = {"X-Inertia": "true"} + + +def _demo_route(role: str) -> str: + return f"/api/users/auth/demo/{role}" + + +async def _props(client) -> dict: + return (await client.get(_LOGIN_PAGE, headers=_INERTIA)).json()["props"] + + +# ── the endpoint is absent unless configured ──────────────────────────────── + + +async def test_both_routes_are_absent_when_demo_mode_is_off(anon_client): + for role in (ADMIN_ROLE_NAME, USER_ROLE_NAME): + assert (await anon_client.post(_demo_route(role))).status_code == 404 + + +async def test_an_unknown_role_is_not_a_way_in(demo_client): + assert (await demo_client.post(_demo_route("superuser"))).status_code == 404 + + +async def test_a_blanked_account_stops_answering(demo_app, demo_client): + """Blanking one email must close its route, not just hide its button.""" + demo_app.state.users.settings.demo_admin_email = "" + + assert (await demo_client.post(_demo_route(ADMIN_ROLE_NAME))).status_code == 404 + assert (await demo_client.post(_demo_route(USER_ROLE_NAME))).status_code == 204 + + +async def test_a_disabled_row_is_not_a_way_in(demo_app, demo_client): + """Disabling the account in the admin UI has to stop the button.""" + from datetime import UTC, datetime + + async with demo_app.state.sm.db.session_factory() as session: + row = await session.get(User, demo_app.state.users.demo_user_ids[0]) + row.is_active = False + row.disabled_at = datetime.now(UTC) + await session.commit() + + assert (await demo_client.post(_demo_route(ADMIN_ROLE_NAME))).status_code == 404 + + +# ── signing in ────────────────────────────────────────────────────────────── + + +async def test_one_click_signs_the_visitor_in_as_the_admin(demo_client): + assert (await demo_client.post(_demo_route(ADMIN_ROLE_NAME))).status_code == 204 + + me = await demo_client.get("/api/users/me") + assert me.status_code == 200 + assert me.json()["email"] == DEMO_ADMIN_EMAIL + + +async def test_one_click_signs_the_visitor_in_as_the_standard_user(demo_client): + assert (await demo_client.post(_demo_route(USER_ROLE_NAME))).status_code == 204 + + assert (await demo_client.get("/api/users/me")).json()["email"] == DEMO_USER_EMAIL + + +async def test_the_two_demos_are_different_identities(demo_client): + """The whole point of two buttons: the second must not land you back in + the first one's session.""" + await demo_client.post(_demo_route(ADMIN_ROLE_NAME)) + first = (await demo_client.get("/api/users/me")).json()["id"] + + await demo_client.post(_demo_route(USER_ROLE_NAME)) + second = (await demo_client.get("/api/users/me")).json()["id"] + + assert first != second + + +async def test_switching_demos_needs_no_sign_out(demo_client): + """Comparing the two surfaces is why there are two, so the sign-in routes + stay reachable from inside a read-only demo session.""" + await demo_client.post(_demo_route(USER_ROLE_NAME)) + + assert (await demo_client.post(_demo_route(ADMIN_ROLE_NAME))).status_code == 204 + assert (await demo_client.get("/api/users/me")).json()["email"] == DEMO_ADMIN_EMAIL + + +# ── what the sign-in card is drawn from ───────────────────────────────────── + + +async def test_the_login_page_offers_nothing_by_default(anon_client): + assert (await _props(anon_client))["demo_signin"] == {"accounts": [], "read_only": True} + + +async def test_the_login_page_offers_both_accounts(demo_client): + assert (await _props(demo_client))["demo_signin"] == { + "accounts": [{"role": ADMIN_ROLE_NAME}, {"role": USER_ROLE_NAME}], + "read_only": True, + } + + +async def test_the_login_page_never_leaks_a_demo_password(demo_app, demo_client): + demo_app.state.users.settings.demo_admin_password = "sup3r-s3cret-demo" + await ensure_demo_users(demo_app) + + assert "sup3r-s3cret-demo" not in (await demo_client.get(_LOGIN_PAGE, headers=_INERTIA)).text + + +async def test_signing_in_turns_the_banner_on(demo_client): + """The shared ``demo`` prop is what every shell reads to say "this is a + demo". Before a button it is inactive; after one, it is not.""" + assert (await _props(demo_client))["demo"] == {"active": False, "readOnly": False} + + await demo_client.post(_demo_route(USER_ROLE_NAME)) + + assert (await _props(demo_client))["demo"] == {"active": True, "readOnly": True} + + +async def test_the_offer_and_the_session_are_separate_props(demo_client): + """Page props win the Inertia merge, so a page prop named ``demo`` would + shadow the banner's shared prop on this page alone — a collision that + breaks quietly and only here.""" + props = await _props(demo_client) + + assert set(props["demo"]) == {"active", "readOnly"} + assert set(props["demo_signin"]) == {"accounts", "read_only"} diff --git a/modules/users/tests/test_users_shared_props.py b/modules/users/tests/test_users_shared_props.py index 97ebc4b3..e2da0abb 100644 --- a/modules/users/tests/test_users_shared_props.py +++ b/modules/users/tests/test_users_shared_props.py @@ -8,7 +8,7 @@ ``demo`` drives the standing demo banner, and is per *session*: an operator signed in normally on a showcase instance must not be told their work is -throwaway. +throwaway, whichever of the two demo accounts is also in use. """ from __future__ import annotations @@ -52,44 +52,45 @@ def test_defaults_closed_when_module_state_is_absent(self) -> None: class TestDemoSharedProp: - def test_inactive_when_demo_mode_is_off(self) -> None: - request = _request( - SimpleNamespace(settings=SimpleNamespace(demo_mode=False), demo_user_id=None), - session={SESSION_DEMO_KEY: True}, + ADMIN_ID = "11111111-1111-1111-1111-111111111111" + USER_ID = "22222222-2222-2222-2222-222222222222" + OUTSIDER_ID = "33333333-3333-3333-3333-333333333333" + + def _state(self, *, demo_mode=True, read_only=True, ids=()): + return SimpleNamespace( + settings=SimpleNamespace(demo_mode=demo_mode, demo_read_only=read_only), + demo_user_ids=ids, ) + + def test_inactive_when_demo_mode_is_off(self) -> None: + request = _request(self._state(demo_mode=False), session={SESSION_DEMO_KEY: True}) assert users_shared_props(request)["demo"] == {"active": False, "readOnly": False} def test_inactive_for_an_ordinary_session_on_a_demo_instance(self) -> None: """The operator's own session is not a demo session.""" request = _request( - SimpleNamespace( - settings=SimpleNamespace(demo_mode=True, demo_read_only=True), - demo_user_id="11111111-1111-1111-1111-111111111111", - ), - session={"user_id": "22222222-2222-2222-2222-222222222222"}, + self._state(ids=(self.ADMIN_ID, self.USER_ID)), + session={"user_id": self.OUTSIDER_ID}, ) assert users_shared_props(request)["demo"] == {"active": False, "readOnly": False} - def test_active_for_the_demo_session(self) -> None: + @pytest.mark.parametrize("which", ["ADMIN_ID", "USER_ID"]) + def test_active_for_either_demo_account(self, which: str) -> None: + """Both buttons lead to a banner — an admin demo is not less of a demo.""" request = _request( - SimpleNamespace( - settings=SimpleNamespace(demo_mode=True, demo_read_only=True), - demo_user_id=None, - ), - session={SESSION_DEMO_KEY: True}, + self._state(ids=(self.ADMIN_ID, self.USER_ID)), + session={"user_id": getattr(self, which)}, ) assert users_shared_props(request)["demo"] == {"active": True, "readOnly": True} + def test_active_for_a_stamped_session(self) -> None: + request = _request(self._state(), session={SESSION_DEMO_KEY: True}) + assert users_shared_props(request)["demo"] == {"active": True, "readOnly": True} + def test_read_only_tracks_the_setting(self) -> None: """The banner says "changes are not saved" — it must not say it when they are.""" - request = _request( - SimpleNamespace( - settings=SimpleNamespace(demo_mode=True, demo_read_only=False), - demo_user_id=None, - ), - session={SESSION_DEMO_KEY: True}, - ) + request = _request(self._state(read_only=False), session={SESSION_DEMO_KEY: True}) assert users_shared_props(request)["demo"] == {"active": True, "readOnly": False} diff --git a/modules/users/users/auth_local/components/DemoSignIn.tsx b/modules/users/users/auth_local/components/DemoSignIn.tsx index e5d692d0..cfa8eef7 100644 --- a/modules/users/users/auth_local/components/DemoSignIn.tsx +++ b/modules/users/users/auth_local/components/DemoSignIn.tsx @@ -1,44 +1,91 @@ import { keys, useT } from '@simple-module-py/i18n'; import { Button } from '@simple-module-py/ui/components/ui/button'; +export interface DemoAccount { + /** `admin` or `user` — also the last segment of the sign-in route. */ + role: string; +} + interface DemoSignInProps { + accounts: DemoAccount[]; /** Whether the demo session will be refused every write. */ readOnly: boolean; - pending: boolean; + /** Role currently being signed in, or `null`. */ + pendingRole: string | null; error: string | null; - onStart: () => void; + onStart: (role: string) => void; } /** - * "Explore the demo" — the one-click entry to a showcase instance. + * Copy per role. `as const` is load-bearing: `t()` accepts only keys present + * in the generated union, so a widened `Record` would not + * typecheck — which is the catalog guarantee working, not an obstacle. + */ +const ROLE_COPY = { + admin: { label: keys.users.login.demo_admin, hint: keys.users.login.demo_admin_hint }, + user: { label: keys.users.login.demo_user, hint: keys.users.login.demo_user_hint }, +} as const; + +type RoleCopy = (typeof ROLE_COPY)[keyof typeof ROLE_COPY]; + +function copyFor(role: string): RoleCopy | undefined { + return ROLE_COPY[role as keyof typeof ROLE_COPY]; +} + +/** + * The demo entry — one button per configured account. + * + * Deliberately below the credentials form and behind its own divider: these + * are alternatives to signing in, not OAuth providers, and putting them in + * that row would read as "sign in with Demo". * - * Deliberately below the credentials form and behind its own divider: it is an - * alternative to signing in, not an OAuth provider, and putting it in that row - * would read as "sign in with Demo". + * No email or password crosses the wire. Each button posts to + * `/api/users/auth/demo/{role}`, which looks the shared account up + * server-side — the dev quick-fill buttons below paste real credentials into + * the form and are development-only for exactly that reason. * - * No email or password crosses the wire. The button posts to - * `/api/users/auth/demo`, which looks the shared account up server-side — the - * dev quick-fill buttons above it paste real credentials into the form and are - * development-only for exactly that reason. + * A role with no label key still renders, captioned by its own name: a demo + * account the server offers must never be unreachable because this map is + * behind. */ -export function DemoSignIn({ readOnly, pending, error, onStart }: DemoSignInProps) { +export function DemoSignIn({ accounts, readOnly, pendingRole, error, onStart }: DemoSignInProps) { const { t } = useT(); + const busy = pendingRole !== null; return (

{t(keys.users.login.demo_divider)}

- +
+ {accounts.map((account) => { + const copy = copyFor(account.role); + return ( + + ); + })} +

{readOnly ? t(keys.users.login.demo_hint_read_only) diff --git a/modules/users/users/auth_local/demo_api.py b/modules/users/users/auth_local/demo_api.py index dddac91b..ca2b9bf9 100644 --- a/modules/users/users/auth_local/demo_api.py +++ b/modules/users/users/auth_local/demo_api.py @@ -1,14 +1,16 @@ -"""``POST /api/users/auth/demo`` — sign in as the shared demo account. +"""``POST /api/users/auth/demo/{role}`` — sign in as a shared demo account. Its own module rather than a branch inside ``api.login``: there is no password in the request, so none of the rate-limit-on-failure, verification or "remember me" machinery applies. What it shares with the password path is the session bridging, and that is three lines. -The account's password is never sent to the browser. A visitor clicks the -button, the server looks the account up itself, and the session it mints is -stamped :data:`users.demo.SESSION_DEMO_KEY` so -:class:`users.demo_guard.DemoReadOnlyMiddleware` can recognise it. +``role`` in the path rather than a body field so each account gets its own +throughput budget (the limiter keys on path + client IP) and so switching +between the two is a plain link's worth of work. No password reaches the +browser either way: a visitor clicks a button, the server looks the account up +itself, and the session it mints is stamped :data:`users.demo.SESSION_DEMO_KEY` +so :class:`users.demo_guard.DemoReadOnlyMiddleware` can recognise it. """ from __future__ import annotations @@ -20,7 +22,7 @@ from simple_module_core.redirect_safety import SESSION_NEXT_KEY from users.auth_local.rate_limit import enforce_auth_throughput_limit -from users.constants import SESSION_USER_ID_KEY +from users.constants import ADMIN_ROLE_NAME, SESSION_USER_ID_KEY, USER_ROLE_NAME from users.demo import SESSION_DEMO_KEY, resolve_demo_account from users.deps import auth_backend, get_user_manager from users.manager import UserManager @@ -29,43 +31,56 @@ router = APIRouter() +# Spelled as a literal rather than derived from the role constants: this is a +# URL segment, and a rename of the internal role name must not silently change +# the public route. ``{role}` is constrained here so an unknown value 404s in +# routing rather than reaching the handler. +_DEMO_ROLE_PATH = "/auth/demo/{role}" +_KNOWN_ROLES = frozenset({ADMIN_ROLE_NAME, USER_ROLE_NAME}) + @router.post( - "/auth/demo", + _DEMO_ROLE_PATH, status_code=204, # Every call mints an access-token row and a session cookie, and the # endpoint takes no credential — without a budget it is a free row - # generator for anyone who finds the showcase instance. + # generator for anyone who finds the showcase instance. Keyed on path, so + # the two accounts get independent budgets and a bot hammering one cannot + # lock a visitor out of the other. dependencies=[Depends(enforce_auth_throughput_limit)], ) async def demo_login( + role: str, request: Request, response: Response, user_manager: UserManager = Depends(get_user_manager), strategy=Depends(auth_backend.get_strategy), ) -> Response: - """Sign the caller in as the configured demo account. + """Sign the caller in as the demo account for ``role``. - 404 rather than 403 when demo mode is off, matching ``/users/register``: - a disabled feature should look absent, not forbidden, so probing the - endpoint says nothing about how the instance is configured. + 404 rather than 403 for every failure here — demo mode off, this account + not configured, an unknown role — matching ``/users/register``: a disabled + feature should look absent, not forbidden, so probing says nothing about + how the instance is configured. """ - state = request.app.state.users - account = resolve_demo_account(state.settings) + if role not in _KNOWN_ROLES: + raise HTTPException(status_code=status.HTTP_404_NOT_FOUND) + + account = resolve_demo_account(request.app.state.users.settings, role) if account is None: raise HTTPException(status_code=status.HTTP_404_NOT_FOUND) - # By email, not by the cached ``state.demo_user_id``: the id is a boot-time + # By email, not by a cached id: the ids on app state are a boot-time # convenience, so a worker whose reconcile failed would otherwise serve a # button that 500s. Through the manager rather than a session of our own # so the row ``on_after_login`` writes ``last_login_at`` to is this one. try: demo_user = await user_manager.get_by_email(account.email) except fu_exceptions.UserNotExists: - logger.warning("users.demo.missing_account", extra={"email": account.email}) + logger.warning("users.demo.missing_account", extra={"email": account.email, "role": role}) raise HTTPException(status_code=status.HTTP_404_NOT_FOUND) from None if not demo_user.is_active or demo_user.disabled_at is not None: - logger.warning("users.demo.inactive_account", extra={"email": account.email}) + logger.warning("users.demo.inactive_account", extra={"email": account.email, "role": role}) raise HTTPException(status_code=status.HTTP_404_NOT_FOUND) await user_manager.on_after_login(demo_user, request, response) diff --git a/modules/users/users/auth_local/views.py b/modules/users/users/auth_local/views.py index fe9d691c..d36e9c7f 100644 --- a/modules/users/users/auth_local/views.py +++ b/modules/users/users/auth_local/views.py @@ -16,7 +16,7 @@ from users.auth_local.token_preview import decode_verify_token, preview_reset from users.bootstrap import resolve_bootstrap_credentials from users.contracts.schemas import UserRead -from users.demo import resolve_demo_account +from users.demo import resolve_demo_accounts from users.mailer import mailer_delivers from users.manager import UserManager, get_user_manager from users.models import User @@ -72,21 +72,23 @@ async def login_page(request: Request, inertia: InertiaDep) -> InertiaResponse: _PAGE_LOGIN, { "allow_signup": users_settings.allow_signup, - # The demo card, when a showcase account is configured. Carries no - # password: the button posts to /api/users/auth/demo and the - # server looks the account up itself. ``read_only`` is here so the - # card can say what the visitor will and will not be able to do - # before they click, rather than after their first refused save. + # One entry per configured demo account, in button order. Carries + # no passwords: each button posts to /api/users/auth/demo/{role} + # and the server looks the account up itself. ``read_only`` is + # here so the card can say what the visitor will and will not be + # able to do before they click, rather than after their first + # refused save. # # Not ``demo`` — that name belongs to the shared prop describing # the *current* session (``users.shared_props``). Page props win # the merge, so reusing it would have this key quietly shadow the # banner's on this one page. - "demo_signin": ( - {"enabled": True, "read_only": demo_account.read_only} - if (demo_account := resolve_demo_account(users_settings)) - else {"enabled": False, "read_only": False} - ), + "demo_signin": { + "accounts": [ + {"role": account.role} for account in resolve_demo_accounts(users_settings) + ], + "read_only": users_settings.demo_read_only, + }, "dev_accounts": dev_accounts, # Where AuthMiddleware bounced them from, when it bounced them. # Read, not popped: a reload of the login page must not silently diff --git a/modules/users/users/demo.py b/modules/users/users/demo.py index dc8ff7d7..5c8e9998 100644 --- a/modules/users/users/demo.py +++ b/modules/users/users/demo.py @@ -1,17 +1,22 @@ -"""Shared demo account — the one-click sign-in behind a public showcase instance. +"""Shared demo accounts — the one-click sign-ins behind a public showcase instance. -Three pieces live here because they have to agree on one question ("is a demo -account configured, and who is it?"): +Three pieces live here because they have to agree on one question ("which demo +accounts are configured, and who are they?"): -* :func:`resolve_demo_account` — reads the answer out of settings. -* :func:`ensure_demo_user` — reconciles the account row against that answer and - caches its id on ``app.state.users``. +* :func:`resolve_demo_accounts` — reads the answer out of settings. +* :func:`ensure_demo_users` — reconciles the rows against that answer and + caches their ids on ``app.state.users``. * :data:`SESSION_DEMO_KEY` — what a demo sign-in stamps on the session so the read-only guard can recognise it without a database read. +Two accounts, an administrator and an ordinary user, because the two halves of +the app look nothing alike: a visitor who only ever sees ``/admin/*`` never +meets the app an end user uses, and one who never sees it misses what the +framework is for. Each is independently switchable by blanking its email. + Deliberately *not* the dev quick-login buttons (``users.auth_local.views``): those are development-only and paste real credentials into the form, which is -exactly what a published demo must not do. Here the password never leaves the +exactly what a published demo must not do. Here the passwords never leave the server — :mod:`users.auth_local.demo_api` mints the session directly. """ @@ -41,7 +46,7 @@ logger = logging.getLogger("users.demo") # Stamped on the session by the demo sign-in endpoint. The guard also matches -# on the user id, so this is not the only line of defence — it is what keeps +# on the user ids, so this is not the only line of defence — it is what keeps # the hot path off the database. SESSION_DEMO_KEY = "is_demo" @@ -54,38 +59,54 @@ USER_ROLE_NAME: (USER_ROLE_ID, USER_ROLE_DESCRIPTION), } +# (role, email field, password field, seeded display name), in the order the +# buttons should appear on the sign-in card. Admin first: it is the surface +# someone evaluating the framework came to see. +_ACCOUNT_SPECS: tuple[tuple[str, str, str, str], ...] = ( + (ADMIN_ROLE_NAME, "demo_admin_email", "demo_admin_password", "Demo Administrator"), + (USER_ROLE_NAME, "demo_user_email", "demo_user_password", "Demo User"), +) + @dataclass(frozen=True) class DemoAccount: - """The demo account an operator has asked for, normalised.""" + """One demo account an operator has asked for, normalised.""" + role: str email: str password: str full_name: str - role: str - read_only: bool -def resolve_demo_account(settings: UsersSettings | None) -> DemoAccount | None: - """The configured demo account, or ``None`` when the feature is off. +def resolve_demo_accounts(settings: UsersSettings | None) -> tuple[DemoAccount, ...]: + """The configured demo accounts, empty when the feature is off. - A blank ``demo_email`` reads as off rather than as an error: the field is - editable in the admin UI, and a half-finished edit should leave the sign-in - page unchanged instead of failing the next boot. + A blank email reads as "don't offer this one" rather than as an error: the + fields are editable in the admin UI, and a half-finished edit should leave + the sign-in page with one button rather than failing the next boot. Blank + both and ``demo_mode`` has nothing to turn on, which is worth a line in + the log because the operator plainly meant something by switching it on. """ if settings is None or not getattr(settings, "demo_mode", False): - return None - email = (settings.demo_email or "").strip() - if not email: - logger.warning("%s — demo_mode is on but demo_email is blank", _EVT_DISABLED) - return None - return DemoAccount( - email=email, - password=settings.demo_password or "", - full_name=(settings.demo_full_name or "").strip() or "Demo User", - role=settings.demo_role if settings.demo_role in _ROLE_SEEDS else USER_ROLE_NAME, - read_only=bool(settings.demo_read_only), + return () + accounts = tuple( + DemoAccount( + role=role, + email=email, + password=getattr(settings, password_field, "") or "", + full_name=full_name, + ) + for role, email_field, password_field, full_name in _ACCOUNT_SPECS + if (email := (getattr(settings, email_field, "") or "").strip()) ) + if not accounts: + logger.warning("%s — demo_mode is on but no demo email is set", _EVT_DISABLED) + return accounts + + +def resolve_demo_account(settings: UsersSettings | None, role: str) -> DemoAccount | None: + """The configured demo account for ``role``, or ``None``.""" + return next((a for a in resolve_demo_accounts(settings) if a.role == role), None) async def _role_row(db: AsyncSession, name: str) -> Role: @@ -110,9 +131,9 @@ async def _role_row(db: AsyncSession, name: str) -> Role: async def _sync_role(db: AsyncSession, user: User, role_name: str) -> None: """Give the demo user exactly the configured role, dropping the other one. - Dropping matters: flipping ``demo_role`` from ``admin`` back to ``user`` is - how an operator revokes a demo that turned out to be too open, and a switch - that only ever added rows would leave the admin grant in place. + Dropping matters: an operator who repoints ``demo_user_email`` at an + address that previously served as the demo *admin* is demoting it, and a + sync that only ever added rows would leave the admin grant in place. """ wanted = await _role_row(db, role_name) links = (await db.execute(select(UserRole).where(UserRole.user_id == user.id))).scalars().all() @@ -125,15 +146,15 @@ async def _sync_role(db: AsyncSession, user: User, role_name: str) -> None: async def reconcile_demo_user(db: AsyncSession, account: DemoAccount) -> User: - """Create or update the demo account row so it matches ``account``. + """Create or update one demo account's row so it matches ``account``. Idempotent, and run on every boot *and* every settings reload — an operator who turns demo mode on in the admin UI of a long-running install must not - have to restart to get the account. + have to restart to get the accounts. - A configured ``demo_password`` is (re)applied every time, so changing it in - the admin UI takes effect. A blank one is hashed from a fresh random secret - on create only — that is what makes "reachable only via the button" true, + A configured password is (re)applied every time, so changing it in the + admin UI takes effect. A blank one is hashed from a fresh random secret on + create only — that is what makes "reachable only through the button" true, and re-rolling it every boot would write an audit entry per worker per restart for a value nobody can use. """ @@ -166,25 +187,29 @@ async def reconcile_demo_user(db: AsyncSession, account: DemoAccount) -> User: return user -async def ensure_demo_user(app: FastAPI) -> uuid.UUID | None: - """Reconcile the demo account and cache its id on ``app.state.users``. +async def ensure_demo_users(app: FastAPI) -> tuple[uuid.UUID, ...]: + """Reconcile every configured demo account; cache the ids on app state. - Returns the id (also stored as ``state.demo_user_id``) or ``None`` when no + Returns the ids (also stored as ``state.demo_user_ids``), empty when no demo account is configured. Never raises: a demo instance failing to boot - because the showcase account could not be written is a worse outcome than - booting without the button. + because a showcase account could not be written is a worse outcome than + booting without the buttons. + + One account failing does not cost the other — they are independent + offers, and an admin demo that cannot be seeded is no reason to withdraw + a working user demo. """ state = app.state.users - account = resolve_demo_account(state.settings) - if account is None: - state.demo_user_id = None - return None - try: - async with app.state.sm.db.session_factory() as session: - user = await reconcile_demo_user(session, account) - except Exception: - logger.exception("users.demo.failed", extra={"email": account.email}) - state.demo_user_id = None - return None - state.demo_user_id = user.id - return user.id + ids: list[uuid.UUID] = [] + for account in resolve_demo_accounts(state.settings): + try: + async with app.state.sm.db.session_factory() as session: + user = await reconcile_demo_user(session, account) + except Exception: + logger.exception( + "users.demo.failed", extra={"email": account.email, "role": account.role} + ) + continue + ids.append(user.id) + state.demo_user_ids = tuple(ids) + return state.demo_user_ids diff --git a/modules/users/users/demo_guard.py b/modules/users/users/demo_guard.py index bcd569da..12a12c90 100644 --- a/modules/users/users/demo_guard.py +++ b/modules/users/users/demo_guard.py @@ -1,9 +1,12 @@ -"""Read-only enforcement for the shared demo account. +"""Read-only enforcement for the shared demo accounts. A demo account is a *published* credential: the sign-in page hands it to -whoever asks. Without this guard, hosting a showcase instance means publishing -write access to the settings editor, the user table and maintenance mode — so -``demo_read_only`` defaults to on and this is what implements it. +whoever asks, and the admin one carries the whole admin surface. Without this +guard, hosting a showcase instance means publishing write access to the +settings editor, the user table and maintenance mode — so ``demo_read_only`` +defaults to on and this is what implements it. It covers both accounts: one +toggle, because "the demo is read-only" is a property of the instance rather +than of whichever button the visitor pressed. Why a middleware and not a dependency: the point is to cover every route in every installed module, including ones written before demo mode existed. A @@ -13,9 +16,9 @@ sorts by ``depends_on``, and ``users`` depends on ``Auth``, so this executes *before* ``AuthMiddleware`` has resolved anyone. The session cookie is already decoded by then (``Session`` sits outside every module's middleware), and it -carries both the stamp the demo endpoint writes and the user id — which is +carries both the stamp the demo endpoints write and the user id — which is what catches a visitor who signed in with a published demo password through -the ordinary form instead of the button. +the ordinary form instead of a button. """ from __future__ import annotations @@ -33,24 +36,24 @@ _SAFE_METHODS = frozenset({"GET", "HEAD", "OPTIONS", "TRACE"}) #: Writes a demo visitor must keep: leaving is not a mutation of the instance, -#: and a demo you cannot sign out of is a demo you cannot show twice. -_ALLOWED_PATHS = frozenset( - { - "/users/logout", - "/api/users/auth/logout", - "/api/users/auth/demo", - } -) - - -def is_demo_session(scope: Scope, demo_user_id) -> bool: - """Whether this request is carrying the demo account's session.""" +#: and a demo you cannot sign out of is a demo you cannot show twice. The +#: sign-in routes are here so switching between the two demo accounts works +#: without signing out first — that comparison is the point of having two. +_ALLOWED_PATHS = frozenset({"/users/logout", "/api/users/auth/logout"}) + +#: Prefix form of the same, for the per-role demo sign-in routes. +_ALLOWED_PREFIXES = ("/api/users/auth/demo/",) + + +def is_demo_session(scope: Scope, demo_user_ids) -> bool: + """Whether this request carries one of the demo accounts' sessions.""" session = scope.get("session") or {} if session.get(SESSION_DEMO_KEY): return True - if demo_user_id is None: + if not demo_user_ids: return False - return str(session.get(SESSION_USER_ID_KEY) or "") == str(demo_user_id) + signed_in = str(session.get(SESSION_USER_ID_KEY) or "") + return bool(signed_in) and any(signed_in == str(known) for known in demo_user_ids) def _refusal(scope: Scope) -> Response: @@ -74,7 +77,7 @@ def _refusal(scope: Scope) -> Response: class DemoReadOnlyMiddleware: - """Refuse unsafe HTTP methods from the shared demo session.""" + """Refuse unsafe HTTP methods from either shared demo session.""" def __init__(self, app: ASGIApp) -> None: self.app = app @@ -96,11 +99,12 @@ async def __call__(self, scope: Scope, receive: Receive, send: Send) -> None: await self.app(scope, receive, send) return - if scope["path"] in _ALLOWED_PATHS: + path = scope["path"] + if path in _ALLOWED_PATHS or path.startswith(_ALLOWED_PREFIXES): await self.app(scope, receive, send) return - if not is_demo_session(scope, getattr(state, "demo_user_id", None)): + if not is_demo_session(scope, getattr(state, "demo_user_ids", ())): await self.app(scope, receive, send) return diff --git a/modules/users/users/locales/en.json b/modules/users/users/locales/en.json index 17cb930c..0f92aece 100644 --- a/modules/users/users/locales/en.json +++ b/modules/users/users/locales/en.json @@ -232,11 +232,14 @@ "no_account_invite_only": "No account? Ask an admin to invite you.", "resend_failed": "We could not send it just now. Try again in a minute.", "demo_divider": "Just looking?", - "demo_submit": "Explore the demo", "demo_submitting": "Opening the demo…", - "demo_hint_read_only": "Signs you in to a shared, read-only account. Nothing you do is saved.", - "demo_hint_writable": "Signs you in to a shared account. Anyone else exploring sees the same data.", - "demo_error": "The demo account is unavailable right now. Please try again shortly." + "demo_hint_read_only": "Shared demo accounts. Nothing you change is saved.", + "demo_hint_writable": "Shared demo accounts — anyone else exploring sees the same data.", + "demo_error": "That demo account is unavailable right now. Please try again shortly.", + "demo_admin": "Explore as an administrator", + "demo_admin_hint": "Users, roles, settings, modules — the whole admin surface.", + "demo_user": "Explore as a standard user", + "demo_user_hint": "The app as the people you build it for see it." }, "metadata_card": { "title": "Account", diff --git a/modules/users/users/module.py b/modules/users/users/module.py index ebdc4ae2..5d3df1cc 100644 --- a/modules/users/users/module.py +++ b/modules/users/users/module.py @@ -104,16 +104,16 @@ async def _rebuild_oauth_clients(event: settings_reloaded) -> None: state.oauth_clients = build_client_map(state.settings) state.oauth_providers = provider_buttons(state.oauth_clients) # Same reason: turning demo mode on from the settings UI has to - # seed the account and refresh the cached id, or the button - # appears on a sign-in page that 404s until the next restart. - from users.demo import ensure_demo_user + # seed the accounts and refresh the cached ids, or the buttons + # appear on a sign-in page that 404s until the next restart. + from users.demo import ensure_demo_users - await ensure_demo_user(app) + await ensure_demo_users(app) bus.subscribe(settings_reloaded, _rebuild_oauth_clients) def register_middleware(self, app: FastAPI) -> None: - """Refuse writes from the shared demo session. + """Refuse writes from either shared demo session. A no-op until ``demo_mode`` and ``demo_read_only`` are both on — it re-reads them per request — so an install that never hosts a demo pays @@ -239,7 +239,7 @@ async def on_startup(self, app: FastAPI) -> None: from users.auth_local.rate_limit import LoginRateLimiter, ThroughputLimiter from users.backend import reconfigure_cookie_transport from users.bootstrap import bootstrap_admin_from_env - from users.demo import ensure_demo_user + from users.demo import ensure_demo_users from users.deps import auth_backend from users.mailer import build_mailer, default_app_name from users.oauth.providers import build_client_map, provider_buttons @@ -285,6 +285,6 @@ def _app_name() -> str: refresh_roles_cache(app), ) # After the env bootstrap, not beside it: that one only runs while the - # users table is empty, and the demo account has to be reconciled on - # every boot so a change to demo_role or demo_password takes effect. - await ensure_demo_user(app) + # users table is empty, and the demo accounts have to be reconciled on + # every boot so a change to either email or password takes effect. + await ensure_demo_users(app) diff --git a/modules/users/users/pages/Login.tsx b/modules/users/users/pages/Login.tsx index f42920eb..82a70e32 100644 --- a/modules/users/users/pages/Login.tsx +++ b/modules/users/users/pages/Login.tsx @@ -4,7 +4,7 @@ import { Button } from '@simple-module-py/ui/components/ui/button'; import { AuthCardShell } from '@simple-module-py/ui/layouts/AuthCardShell'; import { AuthSplitAside } from '@simple-module-py/ui/layouts/AuthSplitAside'; import { useState } from 'react'; -import { DemoSignIn } from '../auth_local/components/DemoSignIn'; +import { type DemoAccount, DemoSignIn } from '../auth_local/components/DemoSignIn'; import { LoginForm, type OAuthProvider } from '../auth_local/components/LoginForm'; import { WaitingOnYou } from '../auth_local/components/WaitingOnYou'; @@ -15,7 +15,7 @@ interface DevAccount { } interface DemoSignInProps { - enabled: boolean; + accounts: DemoAccount[]; read_only: boolean; } @@ -47,7 +47,7 @@ function Login() { const [resent, setResent] = useState(false); const [resendFailed, setResendFailed] = useState(false); const [loading, setLoading] = useState(false); - const [demoPending, setDemoPending] = useState(false); + const [demoPendingRole, setDemoPendingRole] = useState(null); const [demoError, setDemoError] = useState(null); // Server-decided, deliberately. The post-login destination used to be read @@ -89,28 +89,28 @@ function Login() { .finally(() => setLoading(false)); }; - // No credentials in the body: the server resolves the shared demo account - // itself, so the page never holds a password it could leak into a bug + // No credentials in the body: the role in the path is all the server + // needs, so the page never holds a password it could leak into a bug // report, a screenshot or the browser's autofill store. - const startDemo = () => { + const startDemo = (role: string) => { setDemoError(null); setError(null); - setDemoPending(true); - fetch('/api/users/auth/demo', { method: 'POST' }) + setDemoPendingRole(role); + fetch(`/api/users/auth/demo/${role}`, { method: 'POST' }) .then((res) => { if (res.status === 204) { router.visit(nextUrl); return; } - // 404 is demo mode having been switched off since this page was - // rendered; 429 is the shared throughput budget. Neither is worth its - // own copy — both mean "not right now". + // 404 is that account having been switched off since this page was + // rendered; 429 is its throughput budget. Neither is worth its own + // copy — both mean "not right now". setDemoError(t(keys.users.login.demo_error)); - setDemoPending(false); + setDemoPendingRole(null); }) .catch(() => { setDemoError(t(keys.users.common.error_try_again)); - setDemoPending(false); + setDemoPendingRole(null); }); }; @@ -173,10 +173,11 @@ function Login() { /> )} - {demo_signin?.enabled && !needsVerification && ( + {demo_signin?.accounts?.length > 0 && !needsVerification && ( @@ -194,7 +195,7 @@ function Login() { type="button" variant="outline" size="sm" - disabled={loading} + disabled={loading || demoPendingRole !== null} onClick={() => { setEmail(acct.email); setPassword(acct.password); diff --git a/modules/users/users/settings.py b/modules/users/users/settings.py index 85efa4d1..2d853f51 100644 --- a/modules/users/users/settings.py +++ b/modules/users/users/settings.py @@ -20,7 +20,6 @@ from simple_module_core.redirect_safety import non_empty_redirect from simple_module_core.settings_base import DbBackedSettings -from users.constants import ADMIN_ROLE_NAME from users.session_version_cache import SESSION_VERSION_TTL_SECONDS logger = logging.getLogger("users.settings") @@ -119,29 +118,34 @@ def _non_empty_redirect(cls, value: str) -> str: auth_rate_limit_attempts: int = 10 auth_rate_limit_window_seconds: int = 300 - # ── Demo account ──────────────────────────────────────────────────── - # For public showcase instances: one click on the sign-in card signs the - # visitor in as a shared, pre-seeded account. The password is never sent - # to the browser (unlike the dev quick-fill buttons, which are - # development-only and paste real credentials into the form). + # ── Demo accounts ─────────────────────────────────────────────────── + # For public showcase instances: the sign-in card grows one button per + # configured account, and a click signs the visitor straight in. Two + # accounts rather than one because the two halves of the app look nothing + # alike — the admin surface is what the framework is *for*, and a + # visitor who only ever sees it never meets the app an end user uses. + # + # No password is ever sent to the browser, which is what separates this + # from the dev quick-fill buttons: those paste real credentials into the + # form and stay development-only for exactly that reason. demo_mode: bool = Field(default=False, json_schema_extra={"group": DEMO_SETTINGS_GROUP}) - demo_email: str = Field( - default="demo@example.com", json_schema_extra={"group": DEMO_SETTINGS_GROUP} - ) - # Blank means "no password anyone can type": the account is seeded with a - # random one and is reachable only through the demo button. Set it only if - # you also want the credentials published (e.g. for API demos). - demo_password: str = Field(default="", json_schema_extra={"group": DEMO_SETTINGS_GROUP}) - demo_full_name: str = Field( - default="Demo User", json_schema_extra={"group": DEMO_SETTINGS_GROUP} + + # Blank either email to offer only the other account. Blank both and + # ``demo_mode`` has nothing to turn on. + demo_admin_email: str = Field( + default="demo-admin@example.com", json_schema_extra={"group": DEMO_SETTINGS_GROUP} ) - # Which role the demo account carries. ``admin`` is what shows off the - # admin surface — pair it with ``demo_read_only`` (see the validator below) - # or the first visitor can rewrite the instance's settings. - demo_role: str = Field( - default="user", pattern="^(user|admin)$", json_schema_extra={"group": DEMO_SETTINGS_GROUP} + demo_user_email: str = Field( + default="demo-user@example.com", json_schema_extra={"group": DEMO_SETTINGS_GROUP} ) - # Refuse every unsafe HTTP method from a demo session. On by default: + + # Blank means "no password anyone can type": the account is seeded with a + # random one and is reachable only through its button. Set one only if you + # also intend to publish the credentials (for an API demo, say). + demo_admin_password: str = Field(default="", json_schema_extra={"group": DEMO_SETTINGS_GROUP}) + demo_user_password: str = Field(default="", json_schema_extra={"group": DEMO_SETTINGS_GROUP}) + + # Refuse every unsafe HTTP method from either demo session. On by default: # a demo account is a published credential, so the safe posture is the one # you get without reading the docs. demo_read_only: bool = Field(default=True, json_schema_extra={"group": DEMO_SETTINGS_GROUP}) @@ -192,13 +196,13 @@ def _warn_on_writable_admin_demo(self) -> UsersSettings: hands anyone who finds the URL the settings editor, the user table and maintenance mode, so it does not get to happen quietly. """ - if self.demo_mode and self.demo_role == ADMIN_ROLE_NAME and not self.demo_read_only: + if self.demo_mode and self.demo_admin_email.strip() and not self.demo_read_only: logger.warning( - "users.demo.writable_admin — demo_mode is on with demo_role=%r and " + "users.demo.writable_admin — demo_mode is on with demo_admin_email=%r and " "demo_read_only=False: anyone who can reach the sign-in page gets a " "writable administrator session. Set demo_read_only=True unless this " "instance's database is disposable.", - self.demo_role, + self.demo_admin_email, ) return self diff --git a/modules/users/users/shared_props.py b/modules/users/users/shared_props.py index 6993a976..d58c3c94 100644 --- a/modules/users/users/shared_props.py +++ b/modules/users/users/shared_props.py @@ -34,7 +34,7 @@ def users_shared_props(request: Request) -> dict: state = getattr(request.app.state, "users", None) settings = getattr(state, "settings", None) demo_active = bool(getattr(settings, "demo_mode", False)) and is_demo_session( - getattr(request, "scope", {}), getattr(state, "demo_user_id", None) + getattr(request, "scope", {}), getattr(state, "demo_user_ids", ()) ) return { "signup": {"allowed": bool(getattr(settings, "allow_signup", False))}, diff --git a/modules/users/users/state.py b/modules/users/users/state.py index 02158230..da4b949a 100644 --- a/modules/users/users/state.py +++ b/modules/users/users/state.py @@ -36,7 +36,7 @@ class UsersState: roles_cache: list[RoleSummary] = field(default_factory=list) oauth_providers: list[dict[str, str]] = field(default_factory=list) oauth_clients: dict[str, OAuthProvider] = field(default_factory=dict) - # Id of the shared demo account, or None when demo mode is off. Cached at + # Ids of the seeded demo accounts, empty when demo mode is off. Cached at # boot (and on every settings reload) so ``DemoReadOnlyMiddleware`` can # recognise a demo session without a database read per request. - demo_user_id: uuid.UUID | None = None + demo_user_ids: tuple[uuid.UUID, ...] = () diff --git a/packages/i18n/src/generated-resources.ts b/packages/i18n/src/generated-resources.ts index 0cb3c0e9..9e041912 100644 --- a/packages/i18n/src/generated-resources.ts +++ b/packages/i18n/src/generated-resources.ts @@ -916,12 +916,15 @@ export default { 'users.login.aside_check_sso': '', 'users.login.aside_heading': '', 'users.login.continue_with': '', + 'users.login.demo_admin': '', + 'users.login.demo_admin_hint': '', 'users.login.demo_divider': '', 'users.login.demo_error': '', 'users.login.demo_hint_read_only': '', 'users.login.demo_hint_writable': '', - 'users.login.demo_submit': '', 'users.login.demo_submitting': '', + 'users.login.demo_user': '', + 'users.login.demo_user_hint': '', 'users.login.dev_divider': '', 'users.login.divider_or': '', 'users.login.error_invalid_credentials': '', diff --git a/packages/i18n/src/keys.generated.ts b/packages/i18n/src/keys.generated.ts index 9c745975..12d5b801 100644 --- a/packages/i18n/src/keys.generated.ts +++ b/packages/i18n/src/keys.generated.ts @@ -1177,12 +1177,15 @@ export const keys = { aside_check_sso: 'users.login.aside_check_sso', aside_heading: 'users.login.aside_heading', continue_with: 'users.login.continue_with', + demo_admin: 'users.login.demo_admin', + demo_admin_hint: 'users.login.demo_admin_hint', demo_divider: 'users.login.demo_divider', demo_error: 'users.login.demo_error', demo_hint_read_only: 'users.login.demo_hint_read_only', demo_hint_writable: 'users.login.demo_hint_writable', - demo_submit: 'users.login.demo_submit', demo_submitting: 'users.login.demo_submitting', + demo_user: 'users.login.demo_user', + demo_user_hint: 'users.login.demo_user_hint', dev_divider: 'users.login.dev_divider', divider_or: 'users.login.divider_or', error_invalid_credentials: 'users.login.error_invalid_credentials', From c91b6e35a3e81321bb89011e5f97779a8b4238b1 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 20 Sep 2026 09:00:54 +0000 Subject: [PATCH 3/3] fix(users): three ways the demo guard failed on its own state MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Found by a review pass over the demo-accounts diff; all three reproduce against a booted app. **A stale demo stamp locked a visitor out of signing in.** Nothing ever cleared `is_demo`: `UsersAuthProvider._forget` drops user_id, user_ctx, session_version and the remember window, but left the marker behind. So once a demo session lapsed — expiry, "sign out everywhere" on the demo account, or a nightly reset that recreates the rows with new ids — the browser was anonymous but still stamped, and its `POST /api/users/auth/login` came back `403 DEMO_READ_ONLY`. No way back in short of clearing cookies. The stamp now lives in `users.constants` so `manager` and `provider` can reach it without importing the seeding module; `on_after_login` pops it (the demo endpoint re-stamps after the hook, so every *other* sign-in path clears it) and `_forget` pops it too. `/api/users/auth/login` joins the guard's allowlist — it is how you stop being the demo, it costs a real password, and the rate limiter still applies. `/auth/token` deliberately stays blocked. **Turning `demo_mode` off promoted every live demo session to a writable superuser.** The guard short-circuited on `demo_mode`, so the operator action that means "end the showcase" did the opposite for everyone already inside it. The guard (and the banner prop, which has to agree with it) now key on `demo_read_only` plus the session's own stamp. `demo_mode` governs whether the instance still *offers* the demo, not what an already-minted session is. Ending the sessions themselves is disabling the rows, which is now documented. **Reconcile re-enabled a demo account an admin had disabled.** `reconcile_demo_user` wrote `is_active=True; disabled_at=None` unconditionally, so the kill switch for an account being abused was reverted on the next boot or the next unrelated Users settings save. Both fields are now set on create only, matching the generated password. Regression tests for each in `test_demo_guard.py`; `test_users_shared_props.py`'s `test_inactive_when_demo_mode_is_off` encoded the old rule and is replaced by two for the new one. Claude-Session: https://claude.ai/code/session_015SeBCuMuwvpfANx9FHv4qn --- docs/modules/users.md | 12 +++++ modules/users/tests/test_demo_guard.py | 46 +++++++++++++++++++ .../users/tests/test_users_shared_props.py | 11 ++++- modules/users/users/constants.py | 7 +++ modules/users/users/demo.py | 30 +++++++++--- modules/users/users/demo_guard.py | 27 +++++++---- modules/users/users/manager.py | 8 ++++ modules/users/users/provider.py | 7 ++- modules/users/users/shared_props.py | 6 ++- 9 files changed, 136 insertions(+), 18 deletions(-) diff --git a/docs/modules/users.md b/docs/modules/users.md index 2934735f..1b3eb519 100644 --- a/docs/modules/users.md +++ b/docs/modules/users.md @@ -337,6 +337,18 @@ every settings load. Do not run it against a database you care about. Blanking writes, or a session whose `user_id` is one of the demo accounts'. A bearer token is not covered directly, but minting one is itself a `POST`, so a read-only demo cannot get hold of one. +- The guard deliberately does **not** consult `demo_mode`. Switching demo mode + off withdraws the *offer* — it does not retract the sessions already handed + out, and promoting those live cookies to writable superusers on the way out + would be the opposite of what "turn the demo off" means. A stamped session + stays read-only (and keeps its banner) until it signs out or lapses. To end + the sessions themselves, disable the demo rows in the user editor; a + reconcile will not re-enable them. +- Signing in with real credentials (`POST /api/users/auth/login`) is allowed + from a demo session — it is how you *stop* being the demo, it takes a real + password, and the rate limiter still applies. Every non-demo sign-in clears + the stamp, so a browser that once held a demo session is never locked out of + the ordinary login. - Each role's endpoint has its own `auth_rate_limit_*` budget (the limiter keys on path + client IP), so a bot hammering one cannot lock a visitor out of the other. Each click mints a session row, so a demo instance under real diff --git a/modules/users/tests/test_demo_guard.py b/modules/users/tests/test_demo_guard.py index e88e812b..734c07c8 100644 --- a/modules/users/tests/test_demo_guard.py +++ b/modules/users/tests/test_demo_guard.py @@ -130,3 +130,49 @@ async def test_the_guard_leaves_real_accounts_alone(demo_app): ) assert res.status_code == 200 + + +# ── ways out of a demo session ────────────────────────────────────────────── + + +async def test_a_demo_session_can_sign_in_as_a_real_user_and_gets_its_writes_back( + demo_app, demo_client +): + """Signing in as yourself is how you stop being the demo. + + Two failures met here. Refusing the sign-in POST stranded anyone whose + browser still carried the stamp — including a lapsed demo session, which + ``_forget`` leaves anonymous but still marked — with no reachable way back + in short of clearing cookies. And a stamp that survived the sign-in would + hand the guard to the real account that replaced it. + """ + from _users_app_builders import _make_admin_user + + admin = await _make_admin_user(demo_app) + await demo_client.post(_demo_route(ADMIN_ROLE_NAME)) + + signed_in = await demo_client.post( + "/api/users/auth/login", + data={"username": admin.email, "password": "AdminPass1!", "remember": "false"}, + ) + assert signed_in.status_code == 204 + + res = await demo_client.patch("/api/users/me", json={"full_name": "A Real Admin"}) + assert res.status_code == 200 + + +async def test_demo_sessions_stay_read_only_after_demo_mode_is_switched_off(demo_app, demo_client): + """Withdrawing the offer must not promote the sessions already handed out. + + ``demo_mode = False`` stops the buttons; it does not expire the cookies, so + a guard that keyed on it would hand every live demo visitor a writable + superuser on the way out. + """ + await demo_client.post(_demo_route(ADMIN_ROLE_NAME)) + demo_app.state.users.settings.demo_mode = False + demo_app.state.users.demo_user_ids = () + + res = await demo_client.patch("/api/users/me", json={"full_name": "Owned"}) + + assert res.status_code == 403 + assert res.json()["detail"] == DEMO_READ_ONLY_DETAIL diff --git a/modules/users/tests/test_users_shared_props.py b/modules/users/tests/test_users_shared_props.py index e2da0abb..b0acc585 100644 --- a/modules/users/tests/test_users_shared_props.py +++ b/modules/users/tests/test_users_shared_props.py @@ -62,10 +62,17 @@ def _state(self, *, demo_mode=True, read_only=True, ids=()): demo_user_ids=ids, ) - def test_inactive_when_demo_mode_is_off(self) -> None: - request = _request(self._state(demo_mode=False), session={SESSION_DEMO_KEY: True}) + def test_inactive_for_a_session_that_was_never_a_demo(self) -> None: + request = _request(self._state(), session={}) assert users_shared_props(request)["demo"] == {"active": False, "readOnly": False} + def test_a_stamped_session_stays_a_demo_after_demo_mode_is_switched_off(self) -> None: + """Judged on the same rule as the guard, which keeps refusing this + session's writes once the offer is withdrawn — a banner that vanished + would leave those refusals looking like a crash.""" + request = _request(self._state(demo_mode=False), session={SESSION_DEMO_KEY: True}) + assert users_shared_props(request)["demo"] == {"active": True, "readOnly": True} + def test_inactive_for_an_ordinary_session_on_a_demo_instance(self) -> None: """The operator's own session is not a demo session.""" request = _request( diff --git a/modules/users/users/constants.py b/modules/users/users/constants.py index 5b85a2ea..58716ad0 100644 --- a/modules/users/users/constants.py +++ b/modules/users/users/constants.py @@ -24,6 +24,13 @@ # pressed "Sign out everywhere", so the auth provider refuses it. Absent means # 0, which is what every session predating the column carries. SESSION_VERSION_KEY = "session_version" +# Stamped by the demo sign-in endpoint so ``DemoReadOnlyMiddleware`` can +# recognise a shared demo session without a database read. Lives here rather +# than in ``users.demo`` because ``manager``/``provider`` have to clear it on +# every *other* sign-in and on every session teardown, and neither of those +# should have to import the seeding module to do it. Re-exported from +# ``users.demo``, which is where the feature's own code reads it from. +SESSION_DEMO_KEY = "is_demo" # request.state flag set by the OAuth callback before find-or-create so the # manager's on_after_register hook can mark *newly provisioned* OAuth users as diff --git a/modules/users/users/demo.py b/modules/users/users/demo.py index 5c8e9998..86f58dcd 100644 --- a/modules/users/users/demo.py +++ b/modules/users/users/demo.py @@ -36,6 +36,7 @@ ADMIN_ROLE_DESCRIPTION, ADMIN_ROLE_ID, ADMIN_ROLE_NAME, + SESSION_DEMO_KEY, USER_ROLE_DESCRIPTION, USER_ROLE_ID, USER_ROLE_NAME, @@ -45,10 +46,20 @@ logger = logging.getLogger("users.demo") -# Stamped on the session by the demo sign-in endpoint. The guard also matches -# on the user ids, so this is not the only line of defence — it is what keeps -# the hot path off the database. -SESSION_DEMO_KEY = "is_demo" +# ``SESSION_DEMO_KEY`` is stamped on the session by the demo sign-in endpoint; +# the guard also matches on the user ids, so it is not the only line of defence +# — it is what keeps the hot path off the database. It is defined in +# ``users.constants`` because ``manager`` and ``provider`` have to clear it on +# every other sign-in and on session teardown, and neither may import this +# module. Re-exported here, which is where the feature's own code reads it. +__all__ = [ + "SESSION_DEMO_KEY", + "DemoAccount", + "ensure_demo_users", + "reconcile_demo_user", + "resolve_demo_account", + "resolve_demo_accounts", +] _EVT_CREATED = "users.demo.created" _EVT_RECONCILED = "users.demo.reconciled" @@ -157,6 +168,12 @@ async def reconcile_demo_user(db: AsyncSession, account: DemoAccount) -> User: create only — that is what makes "reachable only through the button" true, and re-rolling it every boot would write an audit entry per worker per restart for a value nobody can use. + + ``is_active``/``disabled_at`` are set on *create* only, for the same reason + the generated password is: disabling the row in the admin UI is the kill + switch for a demo account being abused, and a reconcile that re-enabled it + would undo that on the next boot or the next unrelated Users settings save. + Re-enabling is an admin action, not a config one. """ hasher = PasswordHelper() user = ( @@ -172,10 +189,11 @@ async def reconcile_demo_user(db: AsyncSession, account: DemoAccount) -> User: elif created: user.hashed_password = hasher.hash(secrets.token_urlsafe(32)) user.full_name = account.full_name - user.is_active = True user.is_verified = True user.is_superuser = account.role == ADMIN_ROLE_NAME - user.disabled_at = None + if created: + user.is_active = True + user.disabled_at = None await db.flush() await _sync_role(db, user, account.role) await db.commit() diff --git a/modules/users/users/demo_guard.py b/modules/users/users/demo_guard.py index 12a12c90..fdc6f957 100644 --- a/modules/users/users/demo_guard.py +++ b/modules/users/users/demo_guard.py @@ -39,7 +39,14 @@ #: and a demo you cannot sign out of is a demo you cannot show twice. The #: sign-in routes are here so switching between the two demo accounts works #: without signing out first — that comparison is the point of having two. -_ALLOWED_PATHS = frozenset({"/users/logout", "/api/users/auth/logout"}) +#: +#: ``/api/users/auth/login`` is here for the same reason in reverse: signing in +#: as yourself is how you *stop* being the demo, and it takes real credentials +#: (still rate-limited) rather than mutating anything. Refusing it stranded a +#: visitor whose demo session had lapsed — the stamp outlives the identity — +#: with no reachable way back in. ``/auth/token`` is deliberately *not* here: +#: minting a bearer token is what a read-only demo must not be able to do. +_ALLOWED_PATHS = frozenset({"/users/logout", "/api/users/auth/logout", "/api/users/auth/login"}) #: Prefix form of the same, for the per-role demo sign-in routes. _ALLOWED_PREFIXES = ("/api/users/auth/demo/",) @@ -89,13 +96,17 @@ async def __call__(self, scope: Scope, receive: Receive, send: Send) -> None: state = getattr(scope["app"].state, "users", None) settings = getattr(state, "settings", None) - # ``demo_mode`` and ``demo_read_only`` are both live-editable, so they - # are read per request rather than captured at construction. - if ( - settings is None - or not getattr(settings, "demo_mode", False) - or not getattr(settings, "demo_read_only", True) - ): + # ``demo_read_only`` is live-editable, so it is read per request rather + # than captured at construction. + # + # ``demo_mode`` is deliberately *not* consulted. It governs whether the + # instance still *offers* the demo, not what an already-minted demo + # session is: an operator ending the showcase by switching it off would + # otherwise promote every live demo cookie from read-only to a writable + # superuser, which is the opposite of what they asked for. Sessions are + # recognised by the stamp they carry, so an install that never hosted a + # demo still pays nothing beyond one dict lookup on unsafe methods. + if settings is None or not getattr(settings, "demo_read_only", True): await self.app(scope, receive, send) return diff --git a/modules/users/users/manager.py b/modules/users/users/manager.py index 3539d2db..53794faf 100644 --- a/modules/users/users/manager.py +++ b/modules/users/users/manager.py @@ -14,6 +14,7 @@ from users.constants import ( OAUTH_REGISTRATION_REQUEST_FLAG, + SESSION_DEMO_KEY, SESSION_USER_ID_KEY, SESSION_VERSION_KEY, ) @@ -153,6 +154,13 @@ async def on_after_login( # wrappers — re-assigning the same value here is a harmless no-op. if request is not None: 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 diff --git a/modules/users/users/provider.py b/modules/users/users/provider.py index 77a8e53b..146c83b3 100644 --- a/modules/users/users/provider.py +++ b/modules/users/users/provider.py @@ -23,7 +23,7 @@ ) from starlette.requests import Request -from users.constants import SESSION_USER_ID_KEY, SESSION_VERSION_KEY +from users.constants import SESSION_DEMO_KEY, SESSION_USER_ID_KEY, SESSION_VERSION_KEY # Re-exported: the cache lives in its own module (see its docstring), but # ``users.provider`` is where callers reach for it. @@ -64,6 +64,11 @@ def _forget(session) -> None: # "Keep me signed in" was a choice about *this* sign-in. Leaving it behind # would hand a 30-day cookie to the anonymous session that replaces it. session.pop(SESSION_REMEMBER_KEY, None) + # Including the demo stamp. A forgotten session is anonymous, and an + # anonymous session still marked as a demo is refused every unsafe method + # — including the sign-in POST — which leaves the visitor with no way back + # in short of clearing cookies. + session.pop(SESSION_DEMO_KEY, None) class UsersAuthProvider: diff --git a/modules/users/users/shared_props.py b/modules/users/users/shared_props.py index d58c3c94..e238a251 100644 --- a/modules/users/users/shared_props.py +++ b/modules/users/users/shared_props.py @@ -30,10 +30,14 @@ def users_shared_props(request: Request) -> dict: per-*session*, not per-install: an operator signed in to their own account on a demo instance is doing real work and should not be told otherwise, and it is the demo visitor who needs to know their saves will bounce. + + Judged on the same rule as ``DemoReadOnlyMiddleware`` — the session's own + stamp, not ``demo_mode``. The two must agree, or switching demo mode off + leaves the visitor's saves being refused by a page that no longer says why. """ state = getattr(request.app.state, "users", None) settings = getattr(state, "settings", None) - demo_active = bool(getattr(settings, "demo_mode", False)) and is_demo_session( + demo_active = is_demo_session( getattr(request, "scope", {}), getattr(state, "demo_user_ids", ()) ) return {