From 5235fa3e3fbfec5990bec3759b4e8530d7279613 Mon Sep 17 00:00:00 2001 From: Anto Subash Date: Wed, 3 Jun 2026 10:27:38 +0200 Subject: [PATCH 01/11] docs(users): spec for Microsoft OAuth + DB-settings-driven providers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Microsoft (Entra ID) provider plus a refactor moving all OAuth providers off env vars onto the DB-backed settings UI with live reload, resolving providers at request time so DB-only credentials actually mount. Spec only — no implementation yet. --- .../specs/2026-06-03-microsoft-oidc-design.md | 249 ++++++++++++++++++ 1 file changed, 249 insertions(+) create mode 100644 docs/superpowers/specs/2026-06-03-microsoft-oidc-design.md diff --git a/docs/superpowers/specs/2026-06-03-microsoft-oidc-design.md b/docs/superpowers/specs/2026-06-03-microsoft-oidc-design.md new file mode 100644 index 00000000..52b8ca15 --- /dev/null +++ b/docs/superpowers/specs/2026-06-03-microsoft-oidc-design.md @@ -0,0 +1,249 @@ +# Microsoft (Entra ID) OAuth — DB-settings-driven, hot-reloadable providers + +**Date:** 2026-06-03 +**Status:** Approved (design) +**Module:** `users` + +## Summary + +Add a first-class `microsoft` OAuth provider (Microsoft Entra ID / Microsoft +accounts) to the `users` module, alongside the existing Google, GitHub, and +generic-OIDC providers. As part of the same change, move **all** OAuth provider +configuration off environment variables onto the DB-backed settings UI +(`/settings/modules → Users`) with **live reload**, and fix a latent +inconsistency in how providers are wired today. + +## Background — why this is more than "add one provider" + +The OAuth plumbing is already provider-agnostic at the transport layer +(`users/oauth/api.py` mounts a `/login` + `/callback` pair per provider; +find-or-create flows through `UserManager.oauth_callback`). Adding a named +provider is normally a two-branch change in `users/oauth/providers.py`. + +The requirement here is that Microsoft be configured through the **admin +settings UI**, not env vars. That collides with a structural detail: + +- **Routes mount at app *construction*.** `UsersModule.register_routes` + (`create_app` step 9) calls `register_oauth_routes(api_router, + UsersSettings())`. That fresh `UsersSettings()` only carries values captured + by `env_str()` at import time — DB settings have **not** been hydrated yet + (hydration runs later, in the lifespan, before `on_startup`). +- **Buttons mount at *startup*.** `on_startup` sets + `state.oauth_providers = enabled_provider_names(s)` from the **hydrated** + settings. + +Consequence: a provider configured **only via the settings UI** today renders a +login button (startup reads hydrated settings) whose route was **never mounted** +(construction read empty env defaults) → the button 404s. The existing +Google/GitHub/OIDC providers only work because their credentials arrive via env +and are captured at import time. `requires_restart=True` cannot paper over this: +a restart re-runs `register_routes` before hydration again, so DB-only +credentials remain invisible at mount time. + +Therefore, to support DB-settings-driven providers at all, the route layer must +resolve providers at **request time** from hydrated settings. We do this for +**all** providers (not just Microsoft) so the system stays consistent and the +latent button-404 bug is fixed everywhere. This mirrors the migration +`background_tasks` already made (its settings stopped reading `SM_BG_TASKS_*` +env vars; values now come from DB hydration). + +## Goals + +1. `microsoft` provider usable end-to-end (login button → IdP → callback → + find-or-create user → session). +2. All OAuth providers configured via the settings UI, with credentials masked. +3. Provider changes take effect **without a process restart** (live reload via + the existing `SettingsReloaded` event). +4. No new DB tables or migrations. + +## Non-goals (YAGNI) + +- No token-refresh / offline-access flow. +- No provider logos/icons on buttons (matches current text buttons). +- No PKCE changes (the Microsoft client already sets `response_mode=query`). +- No change to the find-or-create / cookie / redirect semantics in the callback. + +## Design + +### 1. Settings — `modules/users/users/settings.py` + +Add Microsoft fields as plain `Field` (no `env_str`), grouped for the admin UI: + +```python +_MS_OAUTH = {"group": "Microsoft OAuth"} +oauth_microsoft_client_id: str = Field(default="", json_schema_extra=_MS_OAUTH) +oauth_microsoft_client_secret: str = Field(default="", json_schema_extra=_MS_OAUTH) # auto-masked +oauth_microsoft_tenant: str = Field(default="common", json_schema_extra=_MS_OAUTH) +``` + +- Migrate the existing `oauth_google_*`, `oauth_github_*`, `oauth_oidc_*` fields + off `env_str(...)` → plain `Field(default=...)`, adding `group` metadata + (`"Google OAuth"`, `"GitHub OAuth"`, `"OIDC"`). Secret-bearing fields + (`*_client_secret`) auto-mask via `is_secret_field` (matches `secret`). +- **Leave the two token-secret fields (`reset_password_token_secret`, + `verification_token_secret`) on `env_str`** — they are a deliberate bootstrap + path (the `@model_validator` must be satisfiable before any DB-backed setting + can be seeded). Untouched. +- Rewrite the stale comment that claims OAuth secrets are env-only "because + admins can read the DB settings table" — secrets are masked in the UI + (`••••••••`), the same treatment the SMTP password already gets. +- **No `requires_restart`** on any OAuth field: the route layer becomes live. + +**Tenant default:** `common` (httpx_oauth default — any work/school *or* +personal Microsoft account). Configurable per deployment; operators lock down +to their org by setting the tenant to their tenant GUID or `organizations`. +Documented as a security note. + +### 2. Provider construction — `modules/users/users/oauth/providers.py` + +- Add a `microsoft` branch to `build_clients()`: + + ```python + from httpx_oauth.clients.microsoft import MicrosoftGraphOAuth2 + MicrosoftGraphOAuth2( + settings.oauth_microsoft_client_id, + settings.oauth_microsoft_client_secret, + tenant=settings.oauth_microsoft_tenant or "common", + name="microsoft", + ) + ``` + Gated on `client_id and client_secret` being set. +- **Remove `enabled_provider_names()`.** The login-button list is now derived + from the successfully-built client map, so a provider only gets a button if + its client actually constructed (fixes the case where OIDC discovery fails but + a button still shows). +- Add `build_client_map(settings) -> dict[str, OAuthProvider]` (thin wrapper: + `{p.name: p for p in build_clients(settings)}`) for O(1) request-time lookup. + +`OAuthProvider` (NamedTuple: `name`, `display_name`, `client`) is unchanged. + +### 3. Route dispatcher — `modules/users/users/oauth/api.py` + +Replace the N per-provider routers with **one** provider-agnostic pair, mounted +unconditionally: + +- `GET /auth/{provider}/login` +- `GET /auth/{provider}/callback` (route name `users_oauth_callback`) + +Behaviour: + +- Resolve the client at **request time**: `provider_obj = + request.app.state.users.oauth_clients.get(provider)`; if `None` → `404`. +- Callback URL: `request.url_for("users_oauth_callback", provider=provider)` + (Starlette `url_for` with a path param) — used identically in `/login` (to + pass to the IdP) and `/callback` (token exchange). +- State CSRF: unchanged mechanism — `secrets.token_urlsafe(32)` stashed under + the per-provider session key `oauth_state:{provider}`, compared with + `compare_digest`. +- Everything from `get_access_token` → `get_id_email` → `oauth_callback` → + cookie → 303 redirect to `login_redirect_url` is **unchanged**. + +`register_oauth_routes(api_router)` no longer takes `settings` and always mounts +the single dispatcher. + +### 4. Cache lifecycle — `modules/users/users/module.py` + `state.py` + +- `UsersState`: add `oauth_clients: dict[str, OAuthProvider] = + field(default_factory=dict)` (default empty so request handlers and tests that + skip `on_startup` see an empty map, not `AttributeError`). +- `register_routes`: mount the dispatcher unconditionally; **drop** the + pre-hydration `settings = UsersSettings()` block (it existed only to feed the + old route builder). +- `on_startup`: build the cache and derive the button list from the **hydrated** + settings `s`: + ```python + state.oauth_clients = build_client_map(s) + state.oauth_providers = [ + {"name": p.name, "display_name": p.display_name} + for p in state.oauth_clients.values() + ] + ``` +- Override `register_event_handlers(self, bus, app)`: subscribe to + `SettingsReloaded`; when `event.package == "users"`, rebuild `oauth_clients` + and `oauth_providers` from `app.state.users.settings`. → providers can be + added/removed live with no restart. `SettingsReloaded` is imported from + `settings.contracts.events` (plugin→plugin import; `users` already depends on + the `settings` module via `register_module_settings`). + +### 5. Frontend — no change + +`Login.tsx` already maps `oauth_providers` → outline buttons linking to +`/api/users/auth/{name}/login`. A "Microsoft" button appears automatically once +configured. The login view (`users/auth_local/views.py`) reads +`request.app.state.users.oauth_providers` per request, so a live cache rebuild +is reflected on the next page load. + +## Identity mapping & documented caveat + +`MicrosoftGraphOAuth2.get_id_email` returns `(profile["id"], +profile["userPrincipalName"])` from Graph `/me`. For tenant members, +`userPrincipalName` equals the email. For **guest/external** accounts the UPN is +not a clean email (e.g. `user_ext.com#EXT#@tenant.onmicrosoft.com`), and that +string becomes the local `account_email`. We use the stock client and **document +this limitation** rather than overriding `get_id_email` (consistent with using +the upstream client for Google/GitHub). + +## Data / migrations + +None. `OAuthAccount.oauth_name` is already `str(max_length=100)` — `"microsoft"` +fits with no schema change. Settings overrides persist in the settings module's +existing store table. + +## Testing — `modules/users/tests/test_oauth.py` (+ additions) + +- `build_clients` / `build_client_map` include `microsoft` when configured; + carry the right `client_id`; the tenant is reflected in the client's authorize + URL; provider skipped when the secret is missing. +- Dispatcher: a configured provider redirects (302) from `/login`; an unknown / + unconfigured `{provider}` returns `404`. +- `SettingsReloaded(package="users")` handler rebuilds the cache: a provider not + present at boot appears after reload; a provider whose credentials are cleared + disappears. +- Update / remove tests that referenced `enabled_provider_names`. +- Existing Google/GitHub `build_clients` and `oauth_callback` find-or-create / + email-association tests continue to pass unchanged. + +Network-hitting paths (real token exchange, Graph profile fetch) remain out of +automated coverage, consistent with the existing test file's stated policy; +validated in a manual QA pass against a dev IdP. + +## Docs + +- `modules/users/README.md`: add a "Social / Microsoft sign-in" section — + configure at **/settings/modules → Users → Microsoft OAuth**; redirect URI is + `/api/users/auth/microsoft/callback`; tenant guidance (`common` vs + tenant GUID / `organizations`); note that the secret is masked in the UI. +- Note the env→DB migration for existing Google/GitHub/OIDC deployments: run + `smpy settings import-from-env` once (maps `SM_USERS_OAUTH_*` → fields). +- Document the UPN-isn't-always-email caveat for guest accounts. +- Release notes: call out that OAuth credentials are no longer read from + `SM_USERS_OAUTH_*` env vars at runtime — use the settings UI or + `import-from-env`. + +## Risks + +- **Behaviour change for env-configured deployments.** Dropping `env_str` from + Google/GitHub/OIDC means `SM_USERS_OAUTH_*` env vars are no longer read at + runtime. Mitigation: `smpy settings import-from-env` (the same path + `background_tasks` used) + release-notes callout. +- **OIDC discovery timing.** Discovery moves from construction to + `on_startup`/on-reload. Equivalent boot-time cost; failures still degrade + gracefully (`OpenIDConfigurationError` caught → provider dropped from the + map, so no button and a 404 route rather than a boot failure). +- **`url_for` with path param** must produce the exact callback URL registered + with the IdP; covered by the dispatcher test and manual QA. + +## Files touched + +- `modules/users/users/settings.py` — Microsoft fields; migrate OAuth fields off + `env_str`; `group` metadata; comment rewrite. +- `modules/users/users/oauth/providers.py` — `microsoft` branch; remove + `enabled_provider_names`; add `build_client_map`. +- `modules/users/users/oauth/api.py` — single request-time dispatcher. +- `modules/users/users/oauth/__init__.py` — exports. +- `modules/users/users/state.py` — `oauth_clients` field. +- `modules/users/users/module.py` — unconditional mount; `on_startup` cache + build; `register_event_handlers` reload. +- `modules/users/tests/test_oauth.py` — updated + new tests. +- `modules/users/README.md` — provider setup docs. +- Release notes — env→DB migration callout. From 79f49bf053abf6536f5bbbe5b66ad3ddc4ded21c Mon Sep 17 00:00:00 2001 From: Anto Subash Date: Wed, 3 Jun 2026 10:37:05 +0200 Subject: [PATCH 02/11] docs(users): implementation plan for Microsoft OAuth + DB-settings providers --- .../plans/2026-06-03-microsoft-oidc.md | 768 ++++++++++++++++++ 1 file changed, 768 insertions(+) create mode 100644 docs/superpowers/plans/2026-06-03-microsoft-oidc.md diff --git a/docs/superpowers/plans/2026-06-03-microsoft-oidc.md b/docs/superpowers/plans/2026-06-03-microsoft-oidc.md new file mode 100644 index 00000000..10e4cd2e --- /dev/null +++ b/docs/superpowers/plans/2026-06-03-microsoft-oidc.md @@ -0,0 +1,768 @@ +# Microsoft (Entra ID) OAuth + DB-settings-driven providers — Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Add a first-class `microsoft` OAuth provider and move all OAuth providers off env vars onto the DB-backed settings UI, resolving providers at request time so DB-only credentials actually mount and changes apply without a restart. + +**Architecture:** OAuth routes become a single provider-agnostic dispatcher (`/auth/{provider}/{login,callback}`) mounted unconditionally at construction. Each request resolves its provider from a client cache on `app.state.users.oauth_clients`, built in `on_startup` from hydrated DB settings and rebuilt on the `SettingsReloaded` event. The login-button list is derived from that same cache. + +**Tech Stack:** Python 3.12, FastAPI, `fastapi-users`, `httpx_oauth` (ships `MicrosoftGraphOAuth2`), pydantic-settings, in-process `EventBus` (pyee), pytest + anyio. + +**Spec:** `docs/superpowers/specs/2026-06-03-microsoft-oidc-design.md` + +--- + +## Conventions for every task + +- Work from the worktree root: `/home/anto/Repos/simple_module_python/.claude/worktrees/microsoft-oidc`. +- Run the OAuth suite with: `uv run pytest modules/users/tests/test_oauth.py -v` +- A single test: `uv run pytest modules/users/tests/test_oauth.py::test_name -v` +- Before each commit, format the files you touched: `uv run ruff format ` (line-length is 100; `make lint` runs `ruff format --check` and will fail on unformatted code). +- Async tests use `@pytest.mark.anyio` and sync tests are plain `def` — match the existing `modules/users/tests/test_oauth.py` style. +- Each task ends green. Tests written first will be red until the implementation step in the same task. + +## File map + +| File | Responsibility | Tasks | +|------|----------------|-------| +| `modules/users/users/settings.py` | OAuth provider settings (Microsoft added; all OAuth fields DB-backed + grouped) | 1 | +| `modules/users/users/oauth/providers.py` | Build httpx_oauth clients; `microsoft` branch; `build_client_map`; drop `enabled_provider_names` | 2, 4 | +| `modules/users/users/oauth/__init__.py` | Public exports | 2, 4 | +| `modules/users/users/state.py` | `UsersState.oauth_clients` cache field | 3 | +| `modules/users/users/oauth/api.py` | Single request-time OAuth dispatcher | 4 | +| `modules/users/users/module.py` | Unconditional mount; `on_startup` cache build; `register_event_handlers` reload | 4, 5 | +| `modules/users/tests/test_oauth.py` | Unit + integration tests | 1–5 | +| `modules/users/README.md` | Provider setup + migration docs | 6 | + +--- + +## Task 1: Settings — Microsoft fields, all OAuth fields DB-backed + grouped + +**Files:** +- Modify: `modules/users/users/settings.py` (the OAuth block, currently lines ~87–100) +- Test: `modules/users/tests/test_oauth.py` + +- [ ] **Step 1: Write the failing tests** + +Add to `modules/users/tests/test_oauth.py` (after the existing imports / settings-section tests): + +```python +def test_microsoft_settings_defaults(): + s = UsersSettings() + assert s.oauth_microsoft_client_id == "" + assert s.oauth_microsoft_client_secret == "" + assert s.oauth_microsoft_tenant == "common" + + +def test_oauth_fields_carry_group_metadata_for_settings_ui(): + fields = UsersSettings.model_fields + assert fields["oauth_google_client_id"].json_schema_extra == {"group": "Google OAuth"} + assert fields["oauth_github_client_id"].json_schema_extra == {"group": "GitHub OAuth"} + assert fields["oauth_oidc_discovery_url"].json_schema_extra == {"group": "OIDC"} + assert fields["oauth_microsoft_client_secret"].json_schema_extra == {"group": "Microsoft OAuth"} +``` + +- [ ] **Step 2: Run tests to verify they fail** + +Run: `uv run pytest modules/users/tests/test_oauth.py::test_microsoft_settings_defaults modules/users/tests/test_oauth.py::test_oauth_fields_carry_group_metadata_for_settings_ui -v` +Expected: FAIL — `oauth_microsoft_*` attributes don't exist / `json_schema_extra` is `None`. + +- [ ] **Step 3: Replace the OAuth settings block** + +In `modules/users/users/settings.py`, replace the current block (the comment + `oauth_google_*` / `oauth_github_*` / `oauth_oidc_*` fields, lines ~87–100) with: + +```python + # OAuth / OIDC providers — configured via the admin settings UI + # (/settings/modules → Users). Credentials live in the DB-backed settings + # store and hydrate after boot; secret fields are masked in the UI (the + # same treatment the SMTP password gets). Provider changes apply live via + # the SettingsReloaded event — no restart (see users/module.py). + oauth_google_client_id: str = Field(default="", json_schema_extra={"group": "Google OAuth"}) + oauth_google_client_secret: str = Field(default="", json_schema_extra={"group": "Google OAuth"}) + oauth_github_client_id: str = Field(default="", json_schema_extra={"group": "GitHub OAuth"}) + oauth_github_client_secret: str = Field(default="", json_schema_extra={"group": "GitHub OAuth"}) + # Generic OIDC — any provider that exposes a discovery URL + # (Keycloak, Authentik, Auth0, Zitadel, ...). + oauth_oidc_client_id: str = Field(default="", json_schema_extra={"group": "OIDC"}) + oauth_oidc_client_secret: str = Field(default="", json_schema_extra={"group": "OIDC"}) + oauth_oidc_discovery_url: str = Field(default="", json_schema_extra={"group": "OIDC"}) + oauth_oidc_display_name: str = Field(default="OIDC", json_schema_extra={"group": "OIDC"}) + # Microsoft Entra ID / Microsoft accounts. tenant: "common" (any work/school + # or personal account), "organizations" (work/school only), or a tenant GUID + # to restrict sign-in to a single Entra tenant. + oauth_microsoft_client_id: str = Field( + default="", json_schema_extra={"group": "Microsoft OAuth"} + ) + oauth_microsoft_client_secret: str = Field( + default="", json_schema_extra={"group": "Microsoft OAuth"} + ) + oauth_microsoft_tenant: str = Field( + default="common", json_schema_extra={"group": "Microsoft OAuth"} + ) +``` + +Notes: +- `Field` is already imported (`from pydantic import Field, model_validator`). +- Keep `env_str` imported and untouched — the two token-secret fields still use it (deliberate bootstrap path). Do **not** change `reset_password_token_secret` / `verification_token_secret`. + +- [ ] **Step 4: Run tests to verify they pass** + +Run: `uv run pytest modules/users/tests/test_oauth.py -v` +Expected: PASS (new tests pass; existing `test_build_clients_google_and_github` and `test_enabled_provider_names_*` still pass — they pass kwargs, not env). + +- [ ] **Step 5: Commit** + +```bash +git add modules/users/users/settings.py modules/users/tests/test_oauth.py +git commit -m "feat(users): DB-backed, grouped OAuth settings + Microsoft fields" +``` + +--- + +## Task 2: providers.py — Microsoft client + `build_client_map` + +**Files:** +- Modify: `modules/users/users/oauth/providers.py` +- Modify: `modules/users/users/oauth/__init__.py` +- Test: `modules/users/tests/test_oauth.py` + +- [ ] **Step 1: Write the failing tests** + +In `modules/users/tests/test_oauth.py`, update the `users.oauth` import to add `build_client_map`: + +```python +from users.oauth import build_client_map, build_clients, enabled_provider_names +``` + +Add these tests in the `build_clients` section: + +```python +def test_build_clients_includes_microsoft(): + s = UsersSettings( + oauth_microsoft_client_id="ms-id", + oauth_microsoft_client_secret="ms-secret", + ) + providers = build_clients(s) + assert [p.name for p in providers] == ["microsoft"] + assert providers[0].display_name == "Microsoft" + assert providers[0].client.client_id == "ms-id" + + +def test_build_clients_skips_microsoft_without_secret(): + s = UsersSettings(oauth_microsoft_client_id="ms-id") # no secret + assert [p.name for p in build_clients(s)] == [] + + +@pytest.mark.anyio +async def test_microsoft_authorize_url_carries_tenant(): + s = UsersSettings( + oauth_microsoft_client_id="ms-id", + oauth_microsoft_client_secret="ms-secret", + oauth_microsoft_tenant="my-tenant-guid", + ) + client = build_client_map(s)["microsoft"].client + url = await client.get_authorization_url("http://testserver/cb", "state123") + assert "my-tenant-guid" in url + + +def test_build_client_map_keys_by_name(): + s = UsersSettings( + oauth_google_client_id="g-id", + oauth_google_client_secret="g-secret", + oauth_microsoft_client_id="ms-id", + oauth_microsoft_client_secret="ms-secret", + ) + m = build_client_map(s) + assert set(m) == {"google", "microsoft"} + assert m["microsoft"].name == "microsoft" +``` + +- [ ] **Step 2: Run tests to verify they fail** + +Run: `uv run pytest modules/users/tests/test_oauth.py -k "microsoft or build_client_map" -v` +Expected: FAIL — `build_client_map` import error / no `microsoft` provider. + +- [ ] **Step 3: Add the Microsoft branch + `build_client_map`** + +In `modules/users/users/oauth/providers.py`, inside `build_clients`, insert the Microsoft branch **after** the GitHub branch and **before** the OIDC branch: + +```python + if settings.oauth_microsoft_client_id and settings.oauth_microsoft_client_secret: + from httpx_oauth.clients.microsoft import MicrosoftGraphOAuth2 + + out.append( + OAuthProvider( + "microsoft", + "Microsoft", + MicrosoftGraphOAuth2( + settings.oauth_microsoft_client_id, + settings.oauth_microsoft_client_secret, + tenant=settings.oauth_microsoft_tenant or "common", + name="microsoft", + ), + ) + ) +``` + +At the end of the file, add: + +```python +def build_client_map(settings: UsersSettings) -> dict[str, OAuthProvider]: + """Configured providers keyed by name for O(1) request-time lookup.""" + return {p.name: p for p in build_clients(settings)} +``` + +- [ ] **Step 4: Export `build_client_map`** + +In `modules/users/users/oauth/__init__.py`, update both lines: + +```python +from users.oauth.providers import ( + OAuthProvider, + build_client_map, + build_clients, + enabled_provider_names, +) + +__all__ = ["OAuthProvider", "build_client_map", "build_clients", "enabled_provider_names"] +``` + +- [ ] **Step 5: Run tests to verify they pass** + +Run: `uv run pytest modules/users/tests/test_oauth.py -v` +Expected: PASS (all, including existing). + +- [ ] **Step 6: Commit** + +```bash +git add modules/users/users/oauth/providers.py modules/users/users/oauth/__init__.py modules/users/tests/test_oauth.py +git commit -m "feat(users): Microsoft OAuth client + build_client_map" +``` + +--- + +## Task 3: state.py — `oauth_clients` cache field + +**Files:** +- Modify: `modules/users/users/state.py` +- Test: `modules/users/tests/test_oauth.py` + +- [ ] **Step 1: Write the failing test** + +Add to `modules/users/tests/test_oauth.py`: + +```python +def test_users_state_defaults_empty_oauth_clients(): + from users.state import UsersState + + state = UsersState(settings=UsersSettings()) + assert state.oauth_clients == {} +``` + +- [ ] **Step 2: Run test to verify it fails** + +Run: `uv run pytest modules/users/tests/test_oauth.py::test_users_state_defaults_empty_oauth_clients -v` +Expected: FAIL — `UsersState` has no `oauth_clients`. + +- [ ] **Step 3: Add the field** + +In `modules/users/users/state.py`: + +Add to the `TYPE_CHECKING` block: + +```python + from users.oauth.providers import OAuthProvider +``` + +Add the field at the end of the `UsersState` dataclass (after `oauth_providers`): + +```python + oauth_clients: dict[str, OAuthProvider] = field(default_factory=dict) +``` + +- [ ] **Step 4: Run test to verify it passes** + +Run: `uv run pytest modules/users/tests/test_oauth.py::test_users_state_defaults_empty_oauth_clients -v` +Expected: PASS + +- [ ] **Step 5: Commit** + +```bash +git add modules/users/users/state.py modules/users/tests/test_oauth.py +git commit -m "feat(users): add oauth_clients cache slot to UsersState" +``` + +--- + +## Task 4: Request-time dispatcher + module wiring (lockstep refactor) + +This task changes the route shape, `register_oauth_routes`' signature, `module.py`, and removes `enabled_provider_names` — all interdependent, so the implementation edits land together and tests run once at the end. + +**Files:** +- Rewrite: `modules/users/users/oauth/api.py` +- Modify: `modules/users/users/module.py` (`register_routes`, `on_startup`) +- Modify: `modules/users/users/oauth/providers.py` (remove `enabled_provider_names`) +- Modify: `modules/users/users/oauth/__init__.py` (drop the export) +- Test: `modules/users/tests/test_oauth.py` + +- [ ] **Step 1: Write the failing dispatcher tests** + +In `modules/users/tests/test_oauth.py`, add these tests (they import `OAuthProvider` locally; the top-level import is finalized in Step 6): + +```python +@pytest.mark.anyio +async def test_oauth_login_redirects_for_configured_provider(users_app, anon_client): + from httpx_oauth.clients.microsoft import MicrosoftGraphOAuth2 + from users.oauth import OAuthProvider + + users_app.state.users.oauth_clients["microsoft"] = OAuthProvider( + "microsoft", "Microsoft", MicrosoftGraphOAuth2("ms-id", "ms-secret") + ) + resp = await anon_client.get("/api/users/auth/microsoft/login", follow_redirects=False) + assert resp.status_code == 302 + assert "login.microsoftonline.com" in resp.headers["location"] + + +@pytest.mark.anyio +async def test_oauth_login_404_for_unknown_provider(anon_client): + resp = await anon_client.get("/api/users/auth/nope/login", follow_redirects=False) + assert resp.status_code == 404 + + +@pytest.mark.anyio +async def test_oauth_callback_rejects_bad_state(users_app, anon_client): + from httpx_oauth.clients.microsoft import MicrosoftGraphOAuth2 + from users.oauth import OAuthProvider + + users_app.state.users.oauth_clients["microsoft"] = OAuthProvider( + "microsoft", "Microsoft", MicrosoftGraphOAuth2("ms-id", "ms-secret") + ) + resp = await anon_client.get( + "/api/users/auth/microsoft/callback?code=abc&state=bad", follow_redirects=False + ) + assert resp.status_code == 400 +``` + +- [ ] **Step 2: Run to confirm they fail** + +Run: `uv run pytest modules/users/tests/test_oauth.py -k "oauth_login or bad_state" -v` +Expected: FAIL — `/auth/{provider}/...` dispatcher doesn't exist yet (per-provider routes only mount when configured). + +- [ ] **Step 3: Rewrite `modules/users/users/oauth/api.py`** + +Replace the entire file with: + +```python +"""OAuth/OIDC login routes — a single provider-agnostic dispatcher. + +One pair of routes (``/auth/{provider}/login`` + ``/auth/{provider}/callback``) +serves every provider. The client is resolved per request from +``app.state.users.oauth_clients`` — the cache built in ``UsersModule.on_startup`` +from hydrated DB settings and rebuilt on ``SettingsReloaded``. Routes mount +unconditionally at construction (before settings hydrate), so providers +configured through the settings UI work and take effect without a restart. + +Why a custom handler rather than ``fastapi_users.get_oauth_router``: the stock +``/callback`` returns 204; Inertia needs the browser to land on a real page, so +``/callback`` returns a 303 redirect to ``login_redirect_url`` with the auth +cookie attached. Find-or-create + email-association go through +``UserManager.oauth_callback``. State CSRF uses Starlette's signed session +cookie. +""" + +from __future__ import annotations + +import logging +import secrets +from typing import TYPE_CHECKING + +from fastapi import APIRouter, Depends, HTTPException, Request, status +from fastapi_users import exceptions as fu_exceptions +from starlette.responses import RedirectResponse + +from users.deps import auth_backend, get_user_manager + +if TYPE_CHECKING: + from users.manager import UserManager + from users.oauth.providers import OAuthProvider + +logger = logging.getLogger(__name__) + +_SESSION_STATE_KEY_FMT = "oauth_state:{provider}" +_CALLBACK_ROUTE_NAME = "users_oauth_callback" + + +def _resolve_provider(request: Request, provider: str) -> OAuthProvider: + """Return the configured provider by name, or raise 404.""" + found = request.app.state.users.oauth_clients.get(provider) + if found is None: + raise HTTPException( + status_code=status.HTTP_404_NOT_FOUND, detail="OAUTH_PROVIDER_NOT_FOUND" + ) + return found + + +def register_oauth_routes(api_router: APIRouter) -> None: + """Mount the provider-agnostic OAuth dispatcher under ``/auth``.""" + router = APIRouter(prefix="/auth", tags=["users-auth"]) + + @router.get("/{provider}/login") + async def begin(request: Request, provider: str) -> RedirectResponse: + """Generate a state nonce, stash it in the session, redirect to the IdP.""" + provider_obj = _resolve_provider(request, provider) + state = secrets.token_urlsafe(32) + request.session[_SESSION_STATE_KEY_FMT.format(provider=provider)] = state + callback_url = str(request.url_for(_CALLBACK_ROUTE_NAME, provider=provider)) + authorization_url = await provider_obj.client.get_authorization_url(callback_url, state) + return RedirectResponse(authorization_url, status_code=302) + + @router.get("/{provider}/callback", name=_CALLBACK_ROUTE_NAME) + async def callback( + request: Request, + provider: str, + code: str | None = None, + state: str | None = None, + user_manager: UserManager = Depends(get_user_manager), + strategy=Depends(auth_backend.get_strategy), + ) -> RedirectResponse: + """Verify state, exchange code, find-or-create user, set cookie, redirect.""" + provider_obj = _resolve_provider(request, provider) + state_key = _SESSION_STATE_KEY_FMT.format(provider=provider) + expected_state = request.session.pop(state_key, None) + if not state or not expected_state or not secrets.compare_digest(state, expected_state): + raise HTTPException( + status_code=status.HTTP_400_BAD_REQUEST, detail="OAUTH_INVALID_STATE" + ) + if not code: + raise HTTPException( + status_code=status.HTTP_400_BAD_REQUEST, detail="OAUTH_MISSING_CODE" + ) + + callback_url = str(request.url_for(_CALLBACK_ROUTE_NAME, provider=provider)) + token = await provider_obj.client.get_access_token(code, callback_url) + account_id, account_email = await provider_obj.client.get_id_email(token["access_token"]) + if account_email is None: + raise HTTPException(status_code=status.HTTP_400_BAD_REQUEST, detail="OAUTH_NO_EMAIL") + + try: + user = await user_manager.oauth_callback( + provider, + token["access_token"], + account_id, + account_email, + token.get("expires_at"), + token.get("refresh_token"), + request, + associate_by_email=True, + is_verified_by_default=True, + ) + except fu_exceptions.UserAlreadyExists: + raise HTTPException( + status_code=status.HTTP_400_BAD_REQUEST, + detail="OAUTH_USER_ALREADY_EXISTS", + ) from None + + if not user.is_active: + raise HTTPException( + status_code=status.HTTP_400_BAD_REQUEST, detail="LOGIN_BAD_CREDENTIALS" + ) + + login_response = await auth_backend.login(strategy, user) + await user_manager.on_after_login(user, request, login_response) + + redirect_url = request.app.state.users.settings.login_redirect_url + redirect = RedirectResponse(redirect_url, status_code=303) + for key, value in login_response.headers.items(): + if key.lower() == "set-cookie": + redirect.raw_headers.append((b"set-cookie", value.encode("latin-1"))) + return redirect + + api_router.include_router(router) +``` + +- [ ] **Step 4: Update `register_routes` in `modules/users/users/module.py`** + +Remove the `from users.settings import UsersSettings` import and the `settings = UsersSettings()` block (current lines ~114–121, including the multi-line comment above it). Change the OAuth mount call (current line ~149) from `register_oauth_routes(api_router, settings)` to: + +```python + register_oauth_routes(api_router) +``` + +(The `from users.oauth.api import register_oauth_routes` import stays.) + +- [ ] **Step 5: Update `on_startup` in `modules/users/users/module.py`** + +Change the import (current line ~163) from: + +```python + from users.oauth.providers import enabled_provider_names +``` + +to: + +```python + from users.oauth.providers import build_client_map +``` + +Replace the button-list assignment (current line ~178) `state.oauth_providers = enabled_provider_names(s)` with: + +```python + state.oauth_clients = build_client_map(s) + state.oauth_providers = [ + {"name": p.name, "display_name": p.display_name} + for p in state.oauth_clients.values() + ] +``` + +- [ ] **Step 6: Remove `enabled_provider_names`** + +In `modules/users/users/oauth/providers.py`, delete the entire `enabled_provider_names` function (and its docstring). + +In `modules/users/users/oauth/__init__.py`, drop the symbol: + +```python +from users.oauth.providers import OAuthProvider, build_client_map, build_clients + +__all__ = ["OAuthProvider", "build_client_map", "build_clients"] +``` + +In `modules/users/tests/test_oauth.py`: +- Change the import to `from users.oauth import OAuthProvider, build_client_map, build_clients`. +- Delete the four now-obsolete tests: `test_enabled_provider_names_empty_by_default`, `test_enabled_provider_names_lists_configured_providers`, `test_enabled_provider_names_skips_provider_missing_secret`, `test_enabled_provider_names_oidc_requires_discovery_url`. + +- [ ] **Step 7: Run the full OAuth suite** + +Run: `uv run pytest modules/users/tests/test_oauth.py -v` +Expected: PASS — dispatcher tests pass, no import errors, existing find-or-create tests still pass. + +- [ ] **Step 8: Boot-smoke the app builds (catches wiring errors the unit tests miss)** + +Run: `uv run pytest modules/users/tests/test_views.py modules/users/tests/test_api_auth.py -q` +Expected: PASS — confirms `register_routes` + `on_startup` still build a working app. + +- [ ] **Step 9: Commit** + +```bash +git add modules/users/users/oauth/api.py modules/users/users/module.py modules/users/users/oauth/providers.py modules/users/users/oauth/__init__.py modules/users/tests/test_oauth.py +git commit -m "refactor(users): request-time OAuth dispatcher over hydrated settings" +``` + +--- + +## Task 5: Live reload on `SettingsReloaded` + +**Files:** +- Modify: `modules/users/users/module.py` (add `register_event_handlers`) +- Test: `modules/users/tests/test_oauth.py` + +- [ ] **Step 1: Write the failing tests** + +Add to `modules/users/tests/test_oauth.py`: + +```python +@pytest.mark.anyio +async def test_settings_reload_adds_provider_to_cache(users_app): + from settings.contracts.events import SettingsReloaded + + assert users_app.state.users.oauth_clients == {} + assert users_app.state.users.oauth_providers == [] + + users_app.state.users.settings = users_app.state.users.settings.model_copy( + update={"oauth_microsoft_client_id": "ms-id", "oauth_microsoft_client_secret": "ms-secret"} + ) + await users_app.state.sm.event_bus.publish( + SettingsReloaded(package="users", changed=("oauth_microsoft_client_id",)) + ) + + assert "microsoft" in users_app.state.users.oauth_clients + buttons = users_app.state.users.oauth_providers + assert {"name": "microsoft", "display_name": "Microsoft"} in buttons + + +@pytest.mark.anyio +async def test_settings_reload_removes_cleared_provider(users_app): + from settings.contracts.events import SettingsReloaded + + users_app.state.users.settings = users_app.state.users.settings.model_copy( + update={"oauth_microsoft_client_id": "ms-id", "oauth_microsoft_client_secret": "ms-secret"} + ) + await users_app.state.sm.event_bus.publish( + SettingsReloaded(package="users", changed=("oauth_microsoft_client_id",)) + ) + assert "microsoft" in users_app.state.users.oauth_clients + + users_app.state.users.settings = users_app.state.users.settings.model_copy( + update={"oauth_microsoft_client_id": "", "oauth_microsoft_client_secret": ""} + ) + await users_app.state.sm.event_bus.publish( + SettingsReloaded(package="users", changed=("oauth_microsoft_client_id",)) + ) + assert "microsoft" not in users_app.state.users.oauth_clients + + +@pytest.mark.anyio +async def test_settings_reload_ignores_other_packages(users_app): + from settings.contracts.events import SettingsReloaded + + sentinel = object() + users_app.state.users.oauth_clients["microsoft"] = sentinel + await users_app.state.sm.event_bus.publish( + SettingsReloaded(package="background_tasks", changed=("broker_url",)) + ) + assert users_app.state.users.oauth_clients["microsoft"] is sentinel +``` + +- [ ] **Step 2: Run to confirm they fail** + +Run: `uv run pytest modules/users/tests/test_oauth.py -k settings_reload -v` +Expected: FAIL — publishing the event has no effect (no subscriber yet); the "adds" and "removes" assertions fail. + +- [ ] **Step 3: Add `register_event_handlers` to `UsersModule`** + +In `modules/users/users/module.py`, add this method to the `UsersModule` class (place it after `register_settings`). Keep the `EventBus`/`FastAPI` types behind `TYPE_CHECKING` — `FastAPI` is already imported there; add `EventBus` to that block: + +```python + def register_event_handlers(self, bus: EventBus, app: FastAPI | None = None) -> None: + """Rebuild the OAuth client cache when the users settings reload. + + Routes mount at construction (before DB hydration), so the cache is the + single source of truth at request time. Rebuilding it here lets an admin + add/remove a provider via the settings UI without a restart. + """ + if app is None: + return + + import importlib + + settings_reloaded = importlib.import_module("settings.contracts.events").SettingsReloaded + from users.oauth.providers import build_client_map + + async def _rebuild_oauth_clients(event) -> None: + if event.package != "users": + return + state = app.state.users + state.oauth_clients = build_client_map(state.settings) + state.oauth_providers = [ + {"name": p.name, "display_name": p.display_name} + for p in state.oauth_clients.values() + ] + + bus.subscribe(settings_reloaded, _rebuild_oauth_clients) +``` + +Add the `EventBus` import to the `TYPE_CHECKING` block at the top of the file: + +```python +if TYPE_CHECKING: + from fastapi import FastAPI + from simple_module_core.events import EventBus +``` + +- [ ] **Step 4: Run to confirm they pass** + +Run: `uv run pytest modules/users/tests/test_oauth.py -k settings_reload -v` +Expected: PASS + +- [ ] **Step 5: Run the full OAuth suite** + +Run: `uv run pytest modules/users/tests/test_oauth.py -v` +Expected: PASS + +- [ ] **Step 6: Commit** + +```bash +git add modules/users/users/module.py modules/users/tests/test_oauth.py +git commit -m "feat(users): hot-reload OAuth providers on SettingsReloaded" +``` + +--- + +## Task 6: Docs — README setup + migration note + +**Files:** +- Modify: `modules/users/README.md` + +- [ ] **Step 1: Add a "Social / Microsoft sign-in" section** + +Append to `modules/users/README.md` (before the License line): + +```markdown +## Social sign-in (Google, GitHub, Microsoft, OIDC) + +OAuth providers are configured in the admin UI at **/settings/modules → Users** +(no environment variables). Each provider activates once its client id **and** +secret are set; the secret is masked in the UI. Changes apply live — no restart. + +**Microsoft (Entra ID).** Register an app in the Entra admin center and set the +redirect URI to `/api/users/auth/microsoft/callback`. Configure under +the **Microsoft OAuth** group: + +- `oauth_microsoft_client_id`, `oauth_microsoft_client_secret` +- `oauth_microsoft_tenant` — `common` (any work/school or personal account, + the default), `organizations` (work/school only), or your tenant GUID to + restrict sign-in to one tenant. + +Each provider's callback URL is `/api/users/auth//callback` +(`google`, `github`, `microsoft`, `oidc`). + +> **Note:** for Microsoft *guest/external* accounts the identity email comes +> from the Graph `userPrincipalName`, which may not be a plain email +> (e.g. `user_ext.com#EXT#@tenant.onmicrosoft.com`). For tenant members it is +> the user's email. + +### Migrating from `SM_USERS_OAUTH_*` env vars + +Earlier versions read provider credentials from `SM_USERS_OAUTH_*` environment +variables. These are no longer read at runtime. Migrate existing values into the +settings store once with: + + uv run smpy settings import-from-env +``` + +- [ ] **Step 2: Verify the doc renders / no broken file-size rule** + +Run: `uv run python scripts/check_file_size.py modules/users/README.md || true` +Expected: no error (Markdown isn't capped, but confirm the command is clean). + +- [ ] **Step 3: Commit** + +```bash +git add modules/users/README.md +git commit -m "docs(users): social sign-in setup + env→settings migration" +``` + +--- + +## Task 7: Full verification + +- [ ] **Step 1: Run the whole users suite** + +Run: `uv run pytest modules/users/tests/ -q` +Expected: PASS (no failures, no import errors). + +- [ ] **Step 2: Format, then run lint** + +Run: `uv run ruff format modules/users/` then `make lint` +Expected: PASS — Ruff format/lint, `ty`, Biome, `tsc`, and the 300-line cap all clean. (`api.py`, `settings.py`, `module.py` all stay well under 300 lines.) + +- [ ] **Step 3: Run module diagnostics** + +Run: `make doctor` +Expected: no new errors/warnings for the `users` module — in particular no `SM012` (`register_settings` still sets `app.state.users`) and no `SM019` (users still registers menu items + permissions). + +- [ ] **Step 4: Final commit if anything changed** + +```bash +git add -A +git commit -m "chore(users): lint/doctor clean for Microsoft OAuth work" || true +``` + +--- + +## Self-review notes (author) + +- **Spec coverage:** settings (T1), Microsoft client + map (T2), state cache (T3), dispatcher + button derivation + `enabled_provider_names` removal + module wiring (T4), hot-reload (T5), docs + migration + UPN caveat (T6), lint/doctor (T7). All spec sections map to a task. +- **No migration task** — intentional: `OAuthAccount.oauth_name` is a plain string; settings persist in the existing settings store. (Stated in spec "Data / migrations".) +- **Type/name consistency:** `build_client_map`, `OAuthProvider(name, display_name, client)`, route name `users_oauth_callback`, session key `oauth_state:{provider}`, cache attr `app.state.users.oauth_clients`, event `SettingsReloaded(package, changed)`, bus at `app.state.sm.event_bus` — all used identically across tasks. +- **Frontend:** no change required (Login.tsx already maps `oauth_providers`); not a task. +``` From 512f18c1f7d7a857e8a8fc5a41b385811c6de654 Mon Sep 17 00:00:00 2001 From: Anto Subash Date: Wed, 3 Jun 2026 10:47:18 +0200 Subject: [PATCH 03/11] feat(users): DB-backed, grouped OAuth settings + Microsoft fields --- modules/users/tests/test_oauth.py | 15 +++++++++++ modules/users/users/settings.py | 41 ++++++++++++++++++++----------- 2 files changed, 42 insertions(+), 14 deletions(-) diff --git a/modules/users/tests/test_oauth.py b/modules/users/tests/test_oauth.py index ba97bc0f..ed3d5f43 100644 --- a/modules/users/tests/test_oauth.py +++ b/modules/users/tests/test_oauth.py @@ -32,6 +32,21 @@ # --------------------------------------------------------------------------- +def test_microsoft_settings_defaults(): + s = UsersSettings() + assert s.oauth_microsoft_client_id == "" + assert s.oauth_microsoft_client_secret == "" + assert s.oauth_microsoft_tenant == "common" + + +def test_oauth_fields_carry_group_metadata_for_settings_ui(): + fields = UsersSettings.model_fields + assert fields["oauth_google_client_id"].json_schema_extra == {"group": "Google OAuth"} + assert fields["oauth_github_client_id"].json_schema_extra == {"group": "GitHub OAuth"} + assert fields["oauth_oidc_discovery_url"].json_schema_extra == {"group": "OIDC"} + assert fields["oauth_microsoft_client_secret"].json_schema_extra == {"group": "Microsoft OAuth"} + + def test_enabled_provider_names_empty_by_default(): assert enabled_provider_names(UsersSettings()) == [] diff --git a/modules/users/users/settings.py b/modules/users/users/settings.py index 5d955fd8..83143b73 100644 --- a/modules/users/users/settings.py +++ b/modules/users/users/settings.py @@ -88,20 +88,33 @@ class UsersSettings(BaseSettings): bootstrap_user_email: str = "" bootstrap_user_password: str = "" - # OAuth / OIDC providers. Each provider is enabled by setting both client - # id and secret; missing credentials = provider not registered. Resolved - # at module-import time (env_str) because client secrets shouldn't ride - # in the DB-backed settings table that admins can read via the UI. - oauth_google_client_id: str = env_str("SM_USERS_OAUTH_GOOGLE_CLIENT_ID", "") - oauth_google_client_secret: str = env_str("SM_USERS_OAUTH_GOOGLE_CLIENT_SECRET", "") - oauth_github_client_id: str = env_str("SM_USERS_OAUTH_GITHUB_CLIENT_ID", "") - oauth_github_client_secret: str = env_str("SM_USERS_OAUTH_GITHUB_CLIENT_SECRET", "") - # Generic OIDC — works with any provider that exposes a discovery URL - # (Keycloak, Authentik, Auth0, Zitadel, Entra ID, ...). - oauth_oidc_client_id: str = env_str("SM_USERS_OAUTH_OIDC_CLIENT_ID", "") - oauth_oidc_client_secret: str = env_str("SM_USERS_OAUTH_OIDC_CLIENT_SECRET", "") - oauth_oidc_discovery_url: str = env_str("SM_USERS_OAUTH_OIDC_DISCOVERY_URL", "") - oauth_oidc_display_name: str = env_str("SM_USERS_OAUTH_OIDC_DISPLAY_NAME", "OIDC") + # OAuth / OIDC providers — configured via the admin settings UI + # (/settings/modules → Users). Credentials live in the DB-backed settings + # store and hydrate after boot; secret fields are masked in the UI (the + # same treatment the SMTP password gets). Provider changes apply live via + # the SettingsReloaded event — no restart (see users/module.py). + oauth_google_client_id: str = Field(default="", json_schema_extra={"group": "Google OAuth"}) + oauth_google_client_secret: str = Field(default="", json_schema_extra={"group": "Google OAuth"}) + oauth_github_client_id: str = Field(default="", json_schema_extra={"group": "GitHub OAuth"}) + oauth_github_client_secret: str = Field(default="", json_schema_extra={"group": "GitHub OAuth"}) + # Generic OIDC — any provider that exposes a discovery URL + # (Keycloak, Authentik, Auth0, Zitadel, ...). + oauth_oidc_client_id: str = Field(default="", json_schema_extra={"group": "OIDC"}) + oauth_oidc_client_secret: str = Field(default="", json_schema_extra={"group": "OIDC"}) + oauth_oidc_discovery_url: str = Field(default="", json_schema_extra={"group": "OIDC"}) + oauth_oidc_display_name: str = Field(default="OIDC", json_schema_extra={"group": "OIDC"}) + # Microsoft Entra ID / Microsoft accounts. tenant: "common" (any work/school + # or personal account), "organizations" (work/school only), or a tenant GUID + # to restrict sign-in to a single Entra tenant. + oauth_microsoft_client_id: str = Field( + default="", json_schema_extra={"group": "Microsoft OAuth"} + ) + oauth_microsoft_client_secret: str = Field( + default="", json_schema_extra={"group": "Microsoft OAuth"} + ) + oauth_microsoft_tenant: str = Field( + default="common", json_schema_extra={"group": "Microsoft OAuth"} + ) @model_validator(mode="after") def _forbid_placeholder_token_secrets_in_production(self) -> UsersSettings: From cffd1fb75cbff7c31423c916087ad4aa40cfd5ad Mon Sep 17 00:00:00 2001 From: Anto Subash Date: Wed, 3 Jun 2026 10:51:54 +0200 Subject: [PATCH 04/11] feat(users): Microsoft OAuth client + build_client_map --- modules/users/tests/test_oauth.py | 42 +++++++++++++++++++++++++- modules/users/users/oauth/__init__.py | 9 ++++-- modules/users/users/oauth/providers.py | 21 +++++++++++++ 3 files changed, 69 insertions(+), 3 deletions(-) diff --git a/modules/users/tests/test_oauth.py b/modules/users/tests/test_oauth.py index ed3d5f43..9b159367 100644 --- a/modules/users/tests/test_oauth.py +++ b/modules/users/tests/test_oauth.py @@ -21,7 +21,7 @@ from fastapi_users.password import PasswordHelper from sqlalchemy import select from users.models import OAuthAccount, User -from users.oauth import build_clients, enabled_provider_names +from users.oauth import build_client_map, build_clients, enabled_provider_names from users.settings import UsersSettings _pw = PasswordHelper() @@ -81,6 +81,46 @@ def test_enabled_provider_names_oidc_requires_discovery_url(): # --------------------------------------------------------------------------- +def test_build_clients_includes_microsoft(): + s = UsersSettings( + oauth_microsoft_client_id="ms-id", + oauth_microsoft_client_secret="ms-secret", + ) + providers = build_clients(s) + assert [p.name for p in providers] == ["microsoft"] + assert providers[0].display_name == "Microsoft" + assert providers[0].client.client_id == "ms-id" + + +def test_build_clients_skips_microsoft_without_secret(): + s = UsersSettings(oauth_microsoft_client_id="ms-id") # no secret + assert [p.name for p in build_clients(s)] == [] + + +@pytest.mark.anyio +async def test_microsoft_authorize_url_carries_tenant(): + s = UsersSettings( + oauth_microsoft_client_id="ms-id", + oauth_microsoft_client_secret="ms-secret", + oauth_microsoft_tenant="my-tenant-guid", + ) + client = build_client_map(s)["microsoft"].client + url = await client.get_authorization_url("http://testserver/cb", "state123") + assert "my-tenant-guid" in url + + +def test_build_client_map_keys_by_name(): + s = UsersSettings( + oauth_google_client_id="g-id", + oauth_google_client_secret="g-secret", + oauth_microsoft_client_id="ms-id", + oauth_microsoft_client_secret="ms-secret", + ) + m = build_client_map(s) + assert set(m) == {"google", "microsoft"} + assert m["microsoft"].name == "microsoft" + + def test_build_clients_google_and_github(): s = UsersSettings( oauth_google_client_id="g-id", diff --git a/modules/users/users/oauth/__init__.py b/modules/users/users/oauth/__init__.py index 65115482..e36783a9 100644 --- a/modules/users/users/oauth/__init__.py +++ b/modules/users/users/oauth/__init__.py @@ -1,5 +1,10 @@ """OAuth feature — public surface re-exported for backward compatibility.""" -from users.oauth.providers import OAuthProvider, build_clients, enabled_provider_names +from users.oauth.providers import ( + OAuthProvider, + build_client_map, + build_clients, + enabled_provider_names, +) -__all__ = ["OAuthProvider", "build_clients", "enabled_provider_names"] +__all__ = ["OAuthProvider", "build_client_map", "build_clients", "enabled_provider_names"] diff --git a/modules/users/users/oauth/providers.py b/modules/users/users/oauth/providers.py index f371314f..d05c77f7 100644 --- a/modules/users/users/oauth/providers.py +++ b/modules/users/users/oauth/providers.py @@ -89,6 +89,22 @@ def build_clients(settings: UsersSettings) -> list[OAuthProvider]: ) ) + if settings.oauth_microsoft_client_id and settings.oauth_microsoft_client_secret: + from httpx_oauth.clients.microsoft import MicrosoftGraphOAuth2 + + out.append( + OAuthProvider( + "microsoft", + "Microsoft", + MicrosoftGraphOAuth2( + settings.oauth_microsoft_client_id, + settings.oauth_microsoft_client_secret, + tenant=settings.oauth_microsoft_tenant or "common", + name="microsoft", + ), + ) + ) + if ( settings.oauth_oidc_client_id and settings.oauth_oidc_client_secret @@ -118,3 +134,8 @@ def build_clients(settings: UsersSettings) -> list[OAuthProvider]: ) return out + + +def build_client_map(settings: UsersSettings) -> dict[str, OAuthProvider]: + """Configured providers keyed by name for O(1) request-time lookup.""" + return {p.name: p for p in build_clients(settings)} From fdc5fd026f3150f3feaaaf15932f25a261c3d0cf Mon Sep 17 00:00:00 2001 From: Anto Subash Date: Wed, 3 Jun 2026 10:55:29 +0200 Subject: [PATCH 05/11] feat(users): add oauth_clients cache slot to UsersState --- modules/users/tests/test_oauth.py | 12 ++++++++++++ modules/users/users/state.py | 2 ++ 2 files changed, 14 insertions(+) diff --git a/modules/users/tests/test_oauth.py b/modules/users/tests/test_oauth.py index 9b159367..ed84a30c 100644 --- a/modules/users/tests/test_oauth.py +++ b/modules/users/tests/test_oauth.py @@ -242,3 +242,15 @@ async def test_oauth_callback_links_to_existing_email(users_app, users_db): assert names == ["github"] finally: await session.__aexit__(None, None, None) + + +# --------------------------------------------------------------------------- +# UsersState defaults +# --------------------------------------------------------------------------- + + +def test_users_state_defaults_empty_oauth_clients(): + from users.state import UsersState + + state = UsersState(settings=UsersSettings()) + assert state.oauth_clients == {} diff --git a/modules/users/users/state.py b/modules/users/users/state.py index 28f6d45a..5d31ee54 100644 --- a/modules/users/users/state.py +++ b/modules/users/users/state.py @@ -18,6 +18,7 @@ if TYPE_CHECKING: from users.auth_local.rate_limit import LoginRateLimiter, ThroughputLimiter from users.mailer import Mailer + from users.oauth.providers import OAuthProvider from users.roles_cache import RoleSummary from users.settings import UsersSettings @@ -32,3 +33,4 @@ class UsersState: auth_throughput_limiter: ThroughputLimiter | None = None 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) From af0a0c2396ba79df8634ba3fab368896cfc5a12f Mon Sep 17 00:00:00 2001 From: Anto Subash Date: Wed, 3 Jun 2026 11:00:50 +0200 Subject: [PATCH 06/11] refactor(users): request-time OAuth dispatcher over hydrated settings --- modules/users/tests/test_oauth.py | 71 ++++++++++--------- modules/users/users/module.py | 17 ++--- modules/users/users/oauth/__init__.py | 9 +-- modules/users/users/oauth/api.py | 96 ++++++++++++-------------- modules/users/users/oauth/providers.py | 22 ------ 5 files changed, 90 insertions(+), 125 deletions(-) diff --git a/modules/users/tests/test_oauth.py b/modules/users/tests/test_oauth.py index ed84a30c..f6a7b17a 100644 --- a/modules/users/tests/test_oauth.py +++ b/modules/users/tests/test_oauth.py @@ -5,8 +5,8 @@ network (token exchange, profile fetch). Those are best validated in a manual QA pass against a dev IdP. What this file *does* cover: -- ``enabled_provider_names`` correctly reflects settings. -- ``build_clients`` instantiates the Google + GitHub clients when configured. +- ``build_clients`` / ``build_client_map`` instantiate clients when configured. +- The provider-agnostic dispatcher resolves clients at request time. - ``OAuthAccount`` persists and FK-cascades on user delete. - ``UserManager.oauth_callback`` (the find-or-create core fastapi-users helper the route delegates to) creates a fresh user + linked OAuthAccount, and @@ -21,7 +21,7 @@ from fastapi_users.password import PasswordHelper from sqlalchemy import select from users.models import OAuthAccount, User -from users.oauth import build_client_map, build_clients, enabled_provider_names +from users.oauth import OAuthProvider, build_client_map, build_clients from users.settings import UsersSettings _pw = PasswordHelper() @@ -47,35 +47,6 @@ def test_oauth_fields_carry_group_metadata_for_settings_ui(): assert fields["oauth_microsoft_client_secret"].json_schema_extra == {"group": "Microsoft OAuth"} -def test_enabled_provider_names_empty_by_default(): - assert enabled_provider_names(UsersSettings()) == [] - - -def test_enabled_provider_names_lists_configured_providers(): - s = UsersSettings( - oauth_google_client_id="g-id", - oauth_google_client_secret="g-secret", - oauth_github_client_id="gh-id", - oauth_github_client_secret="gh-secret", - ) - names = [p["name"] for p in enabled_provider_names(s)] - assert names == ["google", "github"] - - -def test_enabled_provider_names_skips_provider_missing_secret(): - s = UsersSettings(oauth_google_client_id="g-id") # no secret - assert enabled_provider_names(s) == [] - - -def test_enabled_provider_names_oidc_requires_discovery_url(): - s = UsersSettings( - oauth_oidc_client_id="x", - oauth_oidc_client_secret="y", - # discovery_url unset → not registered - ) - assert enabled_provider_names(s) == [] - - # --------------------------------------------------------------------------- # build_clients (no-network providers only) # --------------------------------------------------------------------------- @@ -244,6 +215,42 @@ async def test_oauth_callback_links_to_existing_email(users_app, users_db): await session.__aexit__(None, None, None) +# --------------------------------------------------------------------------- +# Provider-agnostic dispatcher (request-time client resolution) +# --------------------------------------------------------------------------- + + +@pytest.mark.anyio +async def test_oauth_login_redirects_for_configured_provider(users_app, anon_client): + from httpx_oauth.clients.microsoft import MicrosoftGraphOAuth2 + + users_app.state.users.oauth_clients["microsoft"] = OAuthProvider( + "microsoft", "Microsoft", MicrosoftGraphOAuth2("ms-id", "ms-secret") + ) + resp = await anon_client.get("/api/users/auth/microsoft/login", follow_redirects=False) + assert resp.status_code == 302 + assert "login.microsoftonline.com" in resp.headers["location"] + + +@pytest.mark.anyio +async def test_oauth_login_404_for_unknown_provider(anon_client): + resp = await anon_client.get("/api/users/auth/nope/login", follow_redirects=False) + assert resp.status_code == 404 + + +@pytest.mark.anyio +async def test_oauth_callback_rejects_bad_state(users_app, anon_client): + from httpx_oauth.clients.microsoft import MicrosoftGraphOAuth2 + + users_app.state.users.oauth_clients["microsoft"] = OAuthProvider( + "microsoft", "Microsoft", MicrosoftGraphOAuth2("ms-id", "ms-secret") + ) + resp = await anon_client.get( + "/api/users/auth/microsoft/callback?code=abc&state=bad", follow_redirects=False + ) + assert resp.status_code == 400 + + # --------------------------------------------------------------------------- # UsersState defaults # --------------------------------------------------------------------------- diff --git a/modules/users/users/module.py b/modules/users/users/module.py index e9049e59..5225ee91 100644 --- a/modules/users/users/module.py +++ b/modules/users/users/module.py @@ -111,14 +111,6 @@ def register_routes(self, api_router: APIRouter, view_router: APIRouter) -> None from users.contracts.schemas import UserCreate, UserRead from users.deps import fastapi_users from users.oauth.api import register_oauth_routes - from users.settings import UsersSettings - - # Consumed only by ``register_oauth_routes`` → ``build_clients`` at - # registration time, which reads class-attribute defaults captured by - # ``env_str()`` at import. Request-time readers of mutable fields - # (e.g. ``login_redirect_url``) must go through - # ``request.app.state.users.settings``, not this instance. - settings = UsersSettings() api_router.include_router(auth_local_api.router) api_router.include_router(token_router) @@ -146,7 +138,7 @@ def register_routes(self, api_router: APIRouter, view_router: APIRouter) -> None Depends(auth_local_api.enforce_auth_throughput_limit), ], ) - register_oauth_routes(api_router, settings) + register_oauth_routes(api_router) view_router.include_router(auth_views) view_router.include_router(admin_views) @@ -160,7 +152,7 @@ async def on_startup(self, app: FastAPI) -> None: from users.bootstrap import bootstrap_admin_from_env from users.deps import auth_backend from users.mailer import build_mailer - from users.oauth.providers import enabled_provider_names + from users.oauth.providers import build_client_map from users.roles_cache import refresh_roles_cache state = app.state.users @@ -175,7 +167,10 @@ async def on_startup(self, app: FastAPI) -> None: max_attempts=s.auth_rate_limit_attempts, window_seconds=s.auth_rate_limit_window_seconds, ) - state.oauth_providers = enabled_provider_names(s) + state.oauth_clients = build_client_map(s) + state.oauth_providers = [ + {"name": p.name, "display_name": p.display_name} for p in state.oauth_clients.values() + ] # Auto-fall-back when the default ``/dashboard/`` target is # unreachable because the Dashboard module isn't installed (e.g. diff --git a/modules/users/users/oauth/__init__.py b/modules/users/users/oauth/__init__.py index e36783a9..8f5e6f63 100644 --- a/modules/users/users/oauth/__init__.py +++ b/modules/users/users/oauth/__init__.py @@ -1,10 +1,5 @@ """OAuth feature — public surface re-exported for backward compatibility.""" -from users.oauth.providers import ( - OAuthProvider, - build_client_map, - build_clients, - enabled_provider_names, -) +from users.oauth.providers import OAuthProvider, build_client_map, build_clients -__all__ = ["OAuthProvider", "build_client_map", "build_clients", "enabled_provider_names"] +__all__ = ["OAuthProvider", "build_client_map", "build_clients"] diff --git a/modules/users/users/oauth/api.py b/modules/users/users/oauth/api.py index 06131a25..ae5d34c3 100644 --- a/modules/users/users/oauth/api.py +++ b/modules/users/users/oauth/api.py @@ -1,16 +1,18 @@ -"""OAuth/OIDC login routes — one pair (``/login``, ``/callback``) per provider. +"""OAuth/OIDC login routes — a single provider-agnostic dispatcher. + +One pair of routes (``/auth/{provider}/login`` + ``/auth/{provider}/callback``) +serves every provider. The client is resolved per request from +``app.state.users.oauth_clients`` — the cache built in ``UsersModule.on_startup`` +from hydrated DB settings and rebuilt on ``SettingsReloaded``. Routes mount +unconditionally at construction (before settings hydrate), so providers +configured through the settings UI work and take effect without a restart. Why a custom handler rather than ``fastapi_users.get_oauth_router``: the stock -router's ``/callback`` returns a 204 No Content with the auth cookie set. That -works for SPA flows that redirect on a successful AJAX response, but Inertia -expects the user's browser to land on a real page. Here ``/callback`` returns -a 303 redirect to ``settings.login_redirect_url`` instead, with the same -cookie attached. - -Find-or-create + email-association logic still goes through -``UserManager.oauth_callback`` — we don't reimplement it, only the transport -around it. State CSRF uses Starlette's signed session cookie (already mounted -by the framework) instead of fastapi-users' separate JWT-state cookie. +``/callback`` returns 204; Inertia needs the browser to land on a real page, so +``/callback`` returns a 303 redirect to ``login_redirect_url`` with the auth +cookie attached. Find-or-create + email-association go through +``UserManager.oauth_callback``. State CSRF uses Starlette's signed session +cookie. """ from __future__ import annotations @@ -24,40 +26,53 @@ from starlette.responses import RedirectResponse from users.deps import auth_backend, get_user_manager -from users.oauth.providers import OAuthProvider, build_clients if TYPE_CHECKING: from users.manager import UserManager - from users.settings import UsersSettings + from users.oauth.providers import OAuthProvider logger = logging.getLogger(__name__) _SESSION_STATE_KEY_FMT = "oauth_state:{provider}" +_CALLBACK_ROUTE_NAME = "users_oauth_callback" + + +def _resolve_provider(request: Request, provider: str) -> OAuthProvider: + """Return the configured provider by name, or raise 404.""" + found = request.app.state.users.oauth_clients.get(provider) + if found is None: + raise HTTPException( + status_code=status.HTTP_404_NOT_FOUND, detail="OAUTH_PROVIDER_NOT_FOUND" + ) + return found -def _build_provider_router(provider: OAuthProvider) -> APIRouter: - """Mount /login + /callback for one provider.""" - router = APIRouter() - state_key = _SESSION_STATE_KEY_FMT.format(provider=provider.name) +def register_oauth_routes(api_router: APIRouter) -> None: + """Mount the provider-agnostic OAuth dispatcher under ``/auth``.""" + router = APIRouter(prefix="/auth", tags=["users-auth"]) - @router.get("/login") - async def begin(request: Request) -> RedirectResponse: + @router.get("/{provider}/login") + async def begin(request: Request, provider: str) -> RedirectResponse: """Generate a state nonce, stash it in the session, redirect to the IdP.""" + provider_obj = _resolve_provider(request, provider) state = secrets.token_urlsafe(32) - request.session[state_key] = state - callback_url = str(request.url_for(f"oauth_{provider.name}_callback")) - authorization_url = await provider.client.get_authorization_url(callback_url, state) + request.session[_SESSION_STATE_KEY_FMT.format(provider=provider)] = state + callback_url = str(request.url_for(_CALLBACK_ROUTE_NAME, provider=provider)) + authorization_url = await provider_obj.client.get_authorization_url(callback_url, state) return RedirectResponse(authorization_url, status_code=302) - @router.get("/callback", name=f"oauth_{provider.name}_callback") + @router.get("/{provider}/callback", name=_CALLBACK_ROUTE_NAME) async def callback( request: Request, + provider: str, code: str | None = None, state: str | None = None, user_manager: UserManager = Depends(get_user_manager), strategy=Depends(auth_backend.get_strategy), ) -> RedirectResponse: """Verify state, exchange code, find-or-create user, set cookie, redirect.""" + provider_obj = _resolve_provider(request, provider) + state_key = _SESSION_STATE_KEY_FMT.format(provider=provider) expected_state = request.session.pop(state_key, None) if not state or not expected_state or not secrets.compare_digest(state, expected_state): raise HTTPException( @@ -68,15 +83,15 @@ async def callback( status_code=status.HTTP_400_BAD_REQUEST, detail="OAUTH_MISSING_CODE" ) - callback_url = str(request.url_for(f"oauth_{provider.name}_callback")) - token = await provider.client.get_access_token(code, callback_url) - account_id, account_email = await provider.client.get_id_email(token["access_token"]) + callback_url = str(request.url_for(_CALLBACK_ROUTE_NAME, provider=provider)) + token = await provider_obj.client.get_access_token(code, callback_url) + account_id, account_email = await provider_obj.client.get_id_email(token["access_token"]) if account_email is None: raise HTTPException(status_code=status.HTTP_400_BAD_REQUEST, detail="OAUTH_NO_EMAIL") try: user = await user_manager.oauth_callback( - provider.name, + provider, token["access_token"], account_id, account_email, @@ -87,9 +102,6 @@ async def callback( is_verified_by_default=True, ) except fu_exceptions.UserAlreadyExists: - # Email exists but associate_by_email=False would forbid linking. - # We always pass True above, so this branch only fires if the - # provider returns ambiguous data. raise HTTPException( status_code=status.HTTP_400_BAD_REQUEST, detail="OAUTH_USER_ALREADY_EXISTS", @@ -100,14 +112,9 @@ async def callback( status_code=status.HTTP_400_BAD_REQUEST, detail="LOGIN_BAD_CREDENTIALS" ) - # Set the auth cookie via the existing backend, then bridge the - # session in on_after_login (sets session["user_id"] for AuthMiddleware). login_response = await auth_backend.login(strategy, user) await user_manager.on_after_login(user, request, login_response) - # Read login_redirect_url lazily: ``UsersModule.on_startup`` may - # mutate it (e.g. dashboard-fallback) after this router is mounted, - # and admins can change it at runtime via the settings UI. redirect_url = request.app.state.users.settings.login_redirect_url redirect = RedirectResponse(redirect_url, status_code=303) for key, value in login_response.headers.items(): @@ -115,21 +122,4 @@ async def callback( redirect.raw_headers.append((b"set-cookie", value.encode("latin-1"))) return redirect - return router - - -def register_oauth_routes(api_router: APIRouter, settings: UsersSettings) -> None: - """Mount /auth//{login,callback} for every configured provider.""" - providers = build_clients(settings) - for provider in providers: - api_router.include_router( - _build_provider_router(provider), - prefix=f"/auth/{provider.name}", - tags=["users-auth"], - ) - if providers: - logger.info( - "Registered %d OAuth provider(s): %s", - len(providers), - ", ".join(p.name for p in providers), - ) + api_router.include_router(router) diff --git a/modules/users/users/oauth/providers.py b/modules/users/users/oauth/providers.py index d05c77f7..31295ee6 100644 --- a/modules/users/users/oauth/providers.py +++ b/modules/users/users/oauth/providers.py @@ -30,28 +30,6 @@ class OAuthProvider(NamedTuple): client: BaseOAuth2 -def enabled_provider_names(settings: UsersSettings) -> list[dict[str, str]]: - """Return ``[{"name": ..., "display_name": ...}]`` for configured providers. - - Cheap settings-only check used by the login page to render social-login - buttons. Does not construct clients or hit the network — that would be - wasteful per page render and would fail-open if discovery is briefly - unreachable. - """ - out: list[dict[str, str]] = [] - if settings.oauth_google_client_id and settings.oauth_google_client_secret: - out.append({"name": "google", "display_name": "Google"}) - if settings.oauth_github_client_id and settings.oauth_github_client_secret: - out.append({"name": "github", "display_name": "GitHub"}) - if ( - settings.oauth_oidc_client_id - and settings.oauth_oidc_client_secret - and settings.oauth_oidc_discovery_url - ): - out.append({"name": "oidc", "display_name": settings.oauth_oidc_display_name or "OIDC"}) - return out - - def build_clients(settings: UsersSettings) -> list[OAuthProvider]: """Return one entry per provider that has both id and secret configured. From 57d179a29c64e1739b3f582211dc114f42deb0cb Mon Sep 17 00:00:00 2001 From: Anto Subash Date: Wed, 3 Jun 2026 11:08:44 +0200 Subject: [PATCH 07/11] feat(users): hot-reload OAuth providers on SettingsReloaded --- modules/users/tests/test_oauth.py | 57 +++++++++++++++++++++++++++++++ modules/users/users/module.py | 28 +++++++++++++++ 2 files changed, 85 insertions(+) diff --git a/modules/users/tests/test_oauth.py b/modules/users/tests/test_oauth.py index f6a7b17a..85b21957 100644 --- a/modules/users/tests/test_oauth.py +++ b/modules/users/tests/test_oauth.py @@ -261,3 +261,60 @@ def test_users_state_defaults_empty_oauth_clients(): state = UsersState(settings=UsersSettings()) assert state.oauth_clients == {} + + +# --------------------------------------------------------------------------- +# Live cache rebuild on SettingsReloaded +# --------------------------------------------------------------------------- + + +@pytest.mark.anyio +async def test_settings_reload_adds_provider_to_cache(users_app): + from settings.contracts.events import SettingsReloaded + + assert users_app.state.users.oauth_clients == {} + assert users_app.state.users.oauth_providers == [] + + users_app.state.users.settings = users_app.state.users.settings.model_copy( + update={"oauth_microsoft_client_id": "ms-id", "oauth_microsoft_client_secret": "ms-secret"} + ) + await users_app.state.sm.event_bus.publish( + SettingsReloaded(package="users", changed=("oauth_microsoft_client_id",)) + ) + + assert "microsoft" in users_app.state.users.oauth_clients + buttons = users_app.state.users.oauth_providers + assert {"name": "microsoft", "display_name": "Microsoft"} in buttons + + +@pytest.mark.anyio +async def test_settings_reload_removes_cleared_provider(users_app): + from settings.contracts.events import SettingsReloaded + + users_app.state.users.settings = users_app.state.users.settings.model_copy( + update={"oauth_microsoft_client_id": "ms-id", "oauth_microsoft_client_secret": "ms-secret"} + ) + await users_app.state.sm.event_bus.publish( + SettingsReloaded(package="users", changed=("oauth_microsoft_client_id",)) + ) + assert "microsoft" in users_app.state.users.oauth_clients + + users_app.state.users.settings = users_app.state.users.settings.model_copy( + update={"oauth_microsoft_client_id": "", "oauth_microsoft_client_secret": ""} + ) + await users_app.state.sm.event_bus.publish( + SettingsReloaded(package="users", changed=("oauth_microsoft_client_id",)) + ) + assert "microsoft" not in users_app.state.users.oauth_clients + + +@pytest.mark.anyio +async def test_settings_reload_ignores_other_packages(users_app): + from settings.contracts.events import SettingsReloaded + + sentinel = object() + users_app.state.users.oauth_clients["microsoft"] = sentinel + await users_app.state.sm.event_bus.publish( + SettingsReloaded(package="background_tasks", changed=("broker_url",)) + ) + assert users_app.state.users.oauth_clients["microsoft"] is sentinel diff --git a/modules/users/users/module.py b/modules/users/users/module.py index 5225ee91..12603512 100644 --- a/modules/users/users/module.py +++ b/modules/users/users/module.py @@ -18,6 +18,7 @@ if TYPE_CHECKING: from fastapi import FastAPI + from simple_module_core.events import EventBus _MODULE_DEPENDENCY_AUTH = "Auth" @@ -61,6 +62,33 @@ def register_settings(self, app: FastAPI) -> None: app.state.auth.auth_provider = UsersAuthProvider() + def register_event_handlers(self, bus: EventBus, app: FastAPI | None = None) -> None: + """Rebuild the OAuth client cache when the users settings reload. + + Routes mount at construction (before DB hydration), so the cache is the + single source of truth at request time. Rebuilding it here lets an admin + add/remove a provider via the settings UI without a restart. + """ + if app is None: + return + + import importlib + + settings_reloaded = importlib.import_module("settings.contracts.events").SettingsReloaded + from users.oauth.providers import build_client_map + + async def _rebuild_oauth_clients(event) -> None: + if event.package != "users": + return + state = app.state.users + state.oauth_clients = build_client_map(state.settings) + state.oauth_providers = [ + {"name": p.name, "display_name": p.display_name} + for p in state.oauth_clients.values() + ] + + bus.subscribe(settings_reloaded, _rebuild_oauth_clients) + def register_permissions(self, registry: PermissionRegistry) -> None: registry.add_group( "Users", From e1ddf2d2f5cddc6bd7fbb4e0e1d53f30d77d27b5 Mon Sep 17 00:00:00 2001 From: Anto Subash Date: Wed, 3 Jun 2026 11:13:15 +0200 Subject: [PATCH 08/11] refactor(users): extract provider_buttons helper; type reload handler --- modules/users/users/module.py | 15 +++++---------- modules/users/users/oauth/__init__.py | 4 ++-- modules/users/users/oauth/providers.py | 5 +++++ 3 files changed, 12 insertions(+), 12 deletions(-) diff --git a/modules/users/users/module.py b/modules/users/users/module.py index 12603512..1ed95e00 100644 --- a/modules/users/users/module.py +++ b/modules/users/users/module.py @@ -75,17 +75,14 @@ def register_event_handlers(self, bus: EventBus, app: FastAPI | None = None) -> import importlib settings_reloaded = importlib.import_module("settings.contracts.events").SettingsReloaded - from users.oauth.providers import build_client_map + from users.oauth.providers import build_client_map, provider_buttons - async def _rebuild_oauth_clients(event) -> None: + async def _rebuild_oauth_clients(event: settings_reloaded) -> None: if event.package != "users": return state = app.state.users state.oauth_clients = build_client_map(state.settings) - state.oauth_providers = [ - {"name": p.name, "display_name": p.display_name} - for p in state.oauth_clients.values() - ] + state.oauth_providers = provider_buttons(state.oauth_clients) bus.subscribe(settings_reloaded, _rebuild_oauth_clients) @@ -180,7 +177,7 @@ async def on_startup(self, app: FastAPI) -> None: from users.bootstrap import bootstrap_admin_from_env from users.deps import auth_backend from users.mailer import build_mailer - from users.oauth.providers import build_client_map + from users.oauth.providers import build_client_map, provider_buttons from users.roles_cache import refresh_roles_cache state = app.state.users @@ -196,9 +193,7 @@ async def on_startup(self, app: FastAPI) -> None: window_seconds=s.auth_rate_limit_window_seconds, ) state.oauth_clients = build_client_map(s) - state.oauth_providers = [ - {"name": p.name, "display_name": p.display_name} for p in state.oauth_clients.values() - ] + 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. diff --git a/modules/users/users/oauth/__init__.py b/modules/users/users/oauth/__init__.py index 8f5e6f63..6e1d3962 100644 --- a/modules/users/users/oauth/__init__.py +++ b/modules/users/users/oauth/__init__.py @@ -1,5 +1,5 @@ """OAuth feature — public surface re-exported for backward compatibility.""" -from users.oauth.providers import OAuthProvider, build_client_map, build_clients +from users.oauth.providers import OAuthProvider, build_client_map, build_clients, provider_buttons -__all__ = ["OAuthProvider", "build_client_map", "build_clients"] +__all__ = ["OAuthProvider", "build_client_map", "build_clients", "provider_buttons"] diff --git a/modules/users/users/oauth/providers.py b/modules/users/users/oauth/providers.py index 31295ee6..e8bb3e47 100644 --- a/modules/users/users/oauth/providers.py +++ b/modules/users/users/oauth/providers.py @@ -117,3 +117,8 @@ def build_clients(settings: UsersSettings) -> list[OAuthProvider]: def build_client_map(settings: UsersSettings) -> dict[str, OAuthProvider]: """Configured providers keyed by name for O(1) request-time lookup.""" return {p.name: p for p in build_clients(settings)} + + +def provider_buttons(clients: dict[str, OAuthProvider]) -> list[dict[str, str]]: + """Login-button descriptors derived from a built client map.""" + return [{"name": p.name, "display_name": p.display_name} for p in clients.values()] From 9bd67695339d9382d43555252e79a9c690bc225d Mon Sep 17 00:00:00 2001 From: Anto Subash Date: Wed, 3 Jun 2026 11:14:32 +0200 Subject: [PATCH 09/11] =?UTF-8?q?docs(users):=20social=20sign-in=20setup?= =?UTF-8?q?=20+=20env=E2=86=92settings=20migration?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- modules/users/README.md | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/modules/users/README.md b/modules/users/README.md index 04fad9b3..8d761ca4 100644 --- a/modules/users/README.md +++ b/modules/users/README.md @@ -50,6 +50,37 @@ async def profile(user: CurrentUser): - `simple_module_core`, `simple_module_db`, `simple_module_hosting`, `simple_module_auth` - `fastapi-users[sqlalchemy]>=15,<16`, `aiosmtplib`, `cachetools`, `typer` +## Social sign-in (Google, GitHub, Microsoft, OIDC) + +OAuth providers are configured in the admin UI at **/settings/modules → Users** +(no environment variables). Each provider activates once its client id **and** +secret are set; the secret is masked in the UI. Changes apply live — no restart. + +**Microsoft (Entra ID).** Register an app in the Entra admin center and set the +redirect URI to `/api/users/auth/microsoft/callback`. Configure under +the **Microsoft OAuth** group: + +- `oauth_microsoft_client_id`, `oauth_microsoft_client_secret` +- `oauth_microsoft_tenant` — `common` (any work/school or personal account, + the default), `organizations` (work/school only), or your tenant GUID to + restrict sign-in to one tenant. + +Each provider's callback URL is `/api/users/auth//callback` +(`google`, `github`, `microsoft`, `oidc`). + +> **Note:** for Microsoft *guest/external* accounts the identity email comes +> from the Graph `userPrincipalName`, which may not be a plain email +> (e.g. `user_ext.com#EXT#@tenant.onmicrosoft.com`). For tenant members it is +> the user's email. + +### Migrating from `SM_USERS_OAUTH_*` env vars + +Earlier versions read provider credentials from `SM_USERS_OAUTH_*` environment +variables. These are no longer read at runtime. Migrate existing values into the +settings store once with: + + uv run smpy settings import-from-env + ## License MIT — see [LICENSE](https://github.com/antosubash/simple_module_python/blob/main/LICENSE). From 64b7a8400d5e22a849572ded5d6d407f09a443f4 Mon Sep 17 00:00:00 2001 From: Anto Subash Date: Wed, 3 Jun 2026 11:21:34 +0200 Subject: [PATCH 10/11] test(users): split OAuth route + reload tests into test_oauth_routes.py (300-line cap) --- modules/users/tests/test_oauth.py | 101 ++-------------------- modules/users/tests/test_oauth_routes.py | 105 +++++++++++++++++++++++ 2 files changed, 110 insertions(+), 96 deletions(-) create mode 100644 modules/users/tests/test_oauth_routes.py diff --git a/modules/users/tests/test_oauth.py b/modules/users/tests/test_oauth.py index 85b21957..46272ed4 100644 --- a/modules/users/tests/test_oauth.py +++ b/modules/users/tests/test_oauth.py @@ -1,4 +1,4 @@ -"""Unit + integration tests for the OAuth/OIDC plumbing. +"""Unit tests for the OAuth/OIDC plumbing. Provider client construction and the /authorize+/callback ASGI flow are not covered here because both depend on real httpx-oauth clients that hit the @@ -6,11 +6,13 @@ QA pass against a dev IdP. What this file *does* cover: - ``build_clients`` / ``build_client_map`` instantiate clients when configured. -- The provider-agnostic dispatcher resolves clients at request time. - ``OAuthAccount`` persists and FK-cascades on user delete. - ``UserManager.oauth_callback`` (the find-or-create core fastapi-users helper the route delegates to) creates a fresh user + linked OAuthAccount, and associates by email when the user already exists. + +HTTP dispatcher and live-reload (``SettingsReloaded``) integration tests live +in ``test_oauth_routes.py``. """ from __future__ import annotations @@ -21,7 +23,7 @@ from fastapi_users.password import PasswordHelper from sqlalchemy import select from users.models import OAuthAccount, User -from users.oauth import OAuthProvider, build_client_map, build_clients +from users.oauth import build_client_map, build_clients from users.settings import UsersSettings _pw = PasswordHelper() @@ -215,42 +217,6 @@ async def test_oauth_callback_links_to_existing_email(users_app, users_db): await session.__aexit__(None, None, None) -# --------------------------------------------------------------------------- -# Provider-agnostic dispatcher (request-time client resolution) -# --------------------------------------------------------------------------- - - -@pytest.mark.anyio -async def test_oauth_login_redirects_for_configured_provider(users_app, anon_client): - from httpx_oauth.clients.microsoft import MicrosoftGraphOAuth2 - - users_app.state.users.oauth_clients["microsoft"] = OAuthProvider( - "microsoft", "Microsoft", MicrosoftGraphOAuth2("ms-id", "ms-secret") - ) - resp = await anon_client.get("/api/users/auth/microsoft/login", follow_redirects=False) - assert resp.status_code == 302 - assert "login.microsoftonline.com" in resp.headers["location"] - - -@pytest.mark.anyio -async def test_oauth_login_404_for_unknown_provider(anon_client): - resp = await anon_client.get("/api/users/auth/nope/login", follow_redirects=False) - assert resp.status_code == 404 - - -@pytest.mark.anyio -async def test_oauth_callback_rejects_bad_state(users_app, anon_client): - from httpx_oauth.clients.microsoft import MicrosoftGraphOAuth2 - - users_app.state.users.oauth_clients["microsoft"] = OAuthProvider( - "microsoft", "Microsoft", MicrosoftGraphOAuth2("ms-id", "ms-secret") - ) - resp = await anon_client.get( - "/api/users/auth/microsoft/callback?code=abc&state=bad", follow_redirects=False - ) - assert resp.status_code == 400 - - # --------------------------------------------------------------------------- # UsersState defaults # --------------------------------------------------------------------------- @@ -261,60 +227,3 @@ def test_users_state_defaults_empty_oauth_clients(): state = UsersState(settings=UsersSettings()) assert state.oauth_clients == {} - - -# --------------------------------------------------------------------------- -# Live cache rebuild on SettingsReloaded -# --------------------------------------------------------------------------- - - -@pytest.mark.anyio -async def test_settings_reload_adds_provider_to_cache(users_app): - from settings.contracts.events import SettingsReloaded - - assert users_app.state.users.oauth_clients == {} - assert users_app.state.users.oauth_providers == [] - - users_app.state.users.settings = users_app.state.users.settings.model_copy( - update={"oauth_microsoft_client_id": "ms-id", "oauth_microsoft_client_secret": "ms-secret"} - ) - await users_app.state.sm.event_bus.publish( - SettingsReloaded(package="users", changed=("oauth_microsoft_client_id",)) - ) - - assert "microsoft" in users_app.state.users.oauth_clients - buttons = users_app.state.users.oauth_providers - assert {"name": "microsoft", "display_name": "Microsoft"} in buttons - - -@pytest.mark.anyio -async def test_settings_reload_removes_cleared_provider(users_app): - from settings.contracts.events import SettingsReloaded - - users_app.state.users.settings = users_app.state.users.settings.model_copy( - update={"oauth_microsoft_client_id": "ms-id", "oauth_microsoft_client_secret": "ms-secret"} - ) - await users_app.state.sm.event_bus.publish( - SettingsReloaded(package="users", changed=("oauth_microsoft_client_id",)) - ) - assert "microsoft" in users_app.state.users.oauth_clients - - users_app.state.users.settings = users_app.state.users.settings.model_copy( - update={"oauth_microsoft_client_id": "", "oauth_microsoft_client_secret": ""} - ) - await users_app.state.sm.event_bus.publish( - SettingsReloaded(package="users", changed=("oauth_microsoft_client_id",)) - ) - assert "microsoft" not in users_app.state.users.oauth_clients - - -@pytest.mark.anyio -async def test_settings_reload_ignores_other_packages(users_app): - from settings.contracts.events import SettingsReloaded - - sentinel = object() - users_app.state.users.oauth_clients["microsoft"] = sentinel - await users_app.state.sm.event_bus.publish( - SettingsReloaded(package="background_tasks", changed=("broker_url",)) - ) - assert users_app.state.users.oauth_clients["microsoft"] is sentinel diff --git a/modules/users/tests/test_oauth_routes.py b/modules/users/tests/test_oauth_routes.py new file mode 100644 index 00000000..ee408e67 --- /dev/null +++ b/modules/users/tests/test_oauth_routes.py @@ -0,0 +1,105 @@ +"""OAuth HTTP dispatcher + live-reload tests. + +Exercises the running app: the provider-agnostic ``/auth/{provider}/{login,callback}`` +dispatcher (resolution, 404, state CSRF) and the ``SettingsReloaded`` hot-reload of the +provider cache. Provider construction and the account model are unit-tested in +``test_oauth.py``. +""" + +from __future__ import annotations + +import pytest + +# --------------------------------------------------------------------------- +# Provider-agnostic dispatcher (request-time client resolution) +# --------------------------------------------------------------------------- + + +@pytest.mark.anyio +async def test_oauth_login_redirects_for_configured_provider(users_app, anon_client): + from httpx_oauth.clients.microsoft import MicrosoftGraphOAuth2 + from users.oauth import OAuthProvider + + users_app.state.users.oauth_clients["microsoft"] = OAuthProvider( + "microsoft", "Microsoft", MicrosoftGraphOAuth2("ms-id", "ms-secret") + ) + resp = await anon_client.get("/api/users/auth/microsoft/login", follow_redirects=False) + assert resp.status_code == 302 + assert "login.microsoftonline.com" in resp.headers["location"] + + +@pytest.mark.anyio +async def test_oauth_login_404_for_unknown_provider(anon_client): + resp = await anon_client.get("/api/users/auth/nope/login", follow_redirects=False) + assert resp.status_code == 404 + + +@pytest.mark.anyio +async def test_oauth_callback_rejects_bad_state(users_app, anon_client): + from httpx_oauth.clients.microsoft import MicrosoftGraphOAuth2 + from users.oauth import OAuthProvider + + users_app.state.users.oauth_clients["microsoft"] = OAuthProvider( + "microsoft", "Microsoft", MicrosoftGraphOAuth2("ms-id", "ms-secret") + ) + resp = await anon_client.get( + "/api/users/auth/microsoft/callback?code=abc&state=bad", follow_redirects=False + ) + assert resp.status_code == 400 + + +# --------------------------------------------------------------------------- +# Live cache rebuild on SettingsReloaded +# --------------------------------------------------------------------------- + + +@pytest.mark.anyio +async def test_settings_reload_adds_provider_to_cache(users_app): + from settings.contracts.events import SettingsReloaded + + assert users_app.state.users.oauth_clients == {} + assert users_app.state.users.oauth_providers == [] + + users_app.state.users.settings = users_app.state.users.settings.model_copy( + update={"oauth_microsoft_client_id": "ms-id", "oauth_microsoft_client_secret": "ms-secret"} + ) + await users_app.state.sm.event_bus.publish( + SettingsReloaded(package="users", changed=("oauth_microsoft_client_id",)) + ) + + assert "microsoft" in users_app.state.users.oauth_clients + buttons = users_app.state.users.oauth_providers + assert {"name": "microsoft", "display_name": "Microsoft"} in buttons + + +@pytest.mark.anyio +async def test_settings_reload_removes_cleared_provider(users_app): + from settings.contracts.events import SettingsReloaded + + users_app.state.users.settings = users_app.state.users.settings.model_copy( + update={"oauth_microsoft_client_id": "ms-id", "oauth_microsoft_client_secret": "ms-secret"} + ) + await users_app.state.sm.event_bus.publish( + SettingsReloaded(package="users", changed=("oauth_microsoft_client_id",)) + ) + assert "microsoft" in users_app.state.users.oauth_clients + + users_app.state.users.settings = users_app.state.users.settings.model_copy( + update={"oauth_microsoft_client_id": "", "oauth_microsoft_client_secret": ""} + ) + await users_app.state.sm.event_bus.publish( + SettingsReloaded(package="users", changed=("oauth_microsoft_client_id",)) + ) + assert "microsoft" not in users_app.state.users.oauth_clients + + +@pytest.mark.anyio +async def test_settings_reload_ignores_other_packages(users_app): + from settings.contracts.events import SettingsReloaded + + sentinel = object() + users_app.state.users.oauth_clients["microsoft"] = sentinel + await users_app.state.sm.event_bus.publish( + SettingsReloaded(package="background_tasks", changed=("broker_url",)) + ) + assert users_app.state.users.oauth_clients["microsoft"] is sentinel From 40427f8e3572fe096b211e31ea0c3e3f2ef06d52 Mon Sep 17 00:00:00 2001 From: Anto Subash Date: Wed, 3 Jun 2026 11:28:13 +0200 Subject: [PATCH 11/11] refactor(users): drop now-unused logger from oauth dispatcher --- modules/users/users/oauth/api.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/modules/users/users/oauth/api.py b/modules/users/users/oauth/api.py index ae5d34c3..88b141fc 100644 --- a/modules/users/users/oauth/api.py +++ b/modules/users/users/oauth/api.py @@ -17,7 +17,6 @@ from __future__ import annotations -import logging import secrets from typing import TYPE_CHECKING @@ -31,8 +30,6 @@ from users.manager import UserManager from users.oauth.providers import OAuthProvider -logger = logging.getLogger(__name__) - _SESSION_STATE_KEY_FMT = "oauth_state:{provider}" _CALLBACK_ROUTE_NAME = "users_oauth_callback"