feat(users): external (SSO) users — null-password provisioning, credential guards & admin UI

docs/superpowers/specs/2026-06-23-external-sso-users-design.md

@@ -0,0 +1,139 @@
+# External (SSO) users in the `users` module
+
+**Date:** 2026-06-23
+**Status:** Approved (design) — pending implementation plan
+**Branch:** `feat/users-external-sso` (independent of PR #192)
+
+## Problem
+
+When someone signs in through an external IdP (Microsoft/Entra, Google, GitHub,
+generic OIDC), the `users` module already provisions a real local `users_user`
+row just-in-time (via fastapi-users `oauth_callback`, see
+`modules/users/users/oauth/api.py`), links it to a `users_oauth_account`, and
+lets admins assign roles to it through the normal admin path
+(`PUT /api/users/admin/{id}/roles`). Two things are missing for the desired UX:
+
+1. OAuth-provisioned users get a **random unknown password hash**, not a real
+ "no password" — and `users_user.hashed_password` is **not nullable**.
+2. Nothing **marks them as external**, so the admin UI treats them like local
+ password accounts (e.g. offers a password reset that makes no sense).
+
+The standalone `oidc` module (PR #192) is a *different* model — stateless bearer
+-token validation with its own cache table and no local users — and is **out of
+scope here**; it stays as-is for that use case.
+
+## Goal
+
+An external user:
+- is **created locally** on first SSO login (existing behaviour),
+- has a **genuinely null password** (cannot log in with a password, cannot be
+ sent a password reset),
+- is **clearly marked external** in the admin UI, and
+- can be **assigned roles exactly like a normal user** (existing behaviour).
+
+## Non-goals (YAGNI)
+
+- Converting external ↔ local (e.g. an admin setting an initial password to
+ "graduate" an SSO user to a password account).
+- Per-provider external policy (all IdP-provisioned users are uniformly
+ external).
+- Any change to the standalone `oidc` module / PR #192.
+- A default role on creation — external users start with **no roles**; admins
+ assign them.
+
+## Design
+
+### A. Data model + migration
+
+`modules/users/users/models/user.py`:
+- `hashed_password: str` → **`hashed_password: str | None`** (nullable).
+- Add **`is_external: bool`** (NOT NULL, `server_default` false, default False).
+
+One autogenerated Alembic migration under `host/migrations/versions/`:
+- make `users_user.hashed_password` nullable,
+- add `users_user.is_external` with `server_default="false"` so existing rows
+ backfill to non-external; passwords are untouched.
+
+(SQLite note: the project runs both Postgres and SQLite; Alembic batch
+operations already used elsewhere handle SQLite column alters.)
+
+### B. Provisioning — OAuth creates an external user
+
+The OAuth callback path (`modules/users/users/oauth/api.py` →
+`UserManager.oauth_callback`) must, **when it creates a new user**, set
+`hashed_password = None` and `is_external = True`.
+
+- Implementation hook: override `oauth_callback` in
+ `modules/users/users/manager.py` (`UserManager`) — call `super().oauth_callback(...)`
+ but provision new users with a null password + `is_external=True`. The exact
+ mechanism (override `oauth_callback` vs. detect-new-then-patch in the
+ `oauth/api.py` wrapper) is finalised in the implementation plan; the contract
+ is: **only newly-created OAuth users become external.**
+- **Existing-email linking is unchanged** (`associate_by_email=True`): when an
+ IdP login matches an existing *password* user by (verified) email, the OAuth
+ account is linked to that row; the user keeps their password and stays
+ **non-external**.
+
+### C. Credential guards (null-password safety net)
+
+A null `hashed_password` must never reach a hashing/verification call. Guard all
+password-credential paths so external users are rejected with a clear message
+(e.g. *"This account signs in with SSO."*):
+
+- **Password login** (`modules/users/users/auth_local/api.py`, fastapi-users
+ `authenticate`): override `UserManager.authenticate` so a user with
+ `hashed_password is None` fails as invalid credentials, preserving the
+ timing-safe dummy-hash behaviour.
+- **Forgot / reset password** (public fastapi-users routers wired in
+ `modules/users/users/module.py:146`) and **admin reset-link**
+ (`modules/users/users/admin/api.py:235` → `UserManager.generate_reset_password_token`,
+ `manager.py:133`, which calls `password_helper.hash(user.hashed_password)` and
+ would crash on `None`): reject external users **before** the hash call.
+- Any self-service set/change-password route: same rejection. (Exact route list
+ enumerated in the implementation plan.)
+
+### D. Admin UI marking
+
+- Add `is_external` to `UserListItem` (`modules/users/users/contracts/schemas.py:71`)
+ and the user-detail schema, and to the admin queries that build them
+ (`modules/users/users/admin/queries.py` / `admin/service.py`).
+- `/users/admin` list + detail (`modules/users/users/pages/Users/Index.tsx`,
+ `pages/Users/components/DetailsCard.tsx`, `components/DangerZone.tsx`): show an
+ **"External · SSO"** badge and **hide/disable** the "set password" / "send
+ reset link" actions for external users.
+
+### E. Roles — no change
+
+Role assignment is the existing `PUT /api/users/admin/{id}/roles`
+(`admin/api.py:198`). External users start with no roles; admins assign exactly
+as for password users.
+
+### F. Auth provider / login — no change
+
+`UsersAuthProvider.resolve_user` (`modules/users/users/provider.py`) resolves the
+principal from `session["user_id"]` → DB user (roles eager-loaded), honouring
+`is_active` / `disabled_at`. External users resolve identically.
+
+## Testing
+
+- OAuth-created user → `is_external is True`, `hashed_password is None`.
+- Password login for an external user → rejected (clear message), no crash.
+- Forgot-password and admin reset-link for an external user → rejected, no
+ `hash(None)` crash.
+- Admin **can assign roles** to an external user via the normal endpoint.
+- Existing **password user** logging in via OAuth → account linked, password
+ retained, `is_external` stays False.
+- Admin list/detail payloads include `is_external`; UI shows the badge and hides
+ password actions (component test).
+- Migration: existing rows backfill `is_external=False`; `hashed_password`
+ becomes nullable without data loss.
+
+## Risks / edge cases
+
+- **fastapi-users assumes a non-null `hashed_password`** in `authenticate` and
+ reset-token generation — both are explicitly guarded above; tests cover the
+ null path.
+- **Email-based linking trust**: `associate_by_email=True` links by IdP email.
+ Acceptable for Entra (verified emails); a provider that emits unverified
+ emails could allow linking to an existing account. Out of scope to change
+ here, but noted.

framework/core/tests/test_module_base.py

@@ -35,7 +35,7 @@ async def test_custom_fields(self):
  async def test_frozen(self):
  meta = ModuleMeta(name="Frozen")
  with pytest.raises(AttributeError):
- meta.name = "Changed" # type: ignore[misc] # ty: ignore[invalid-assignment]
+ meta.name = "Changed" # type: ignore[misc]
 
 
 class DummyModule(ModuleBase):

framework/core/tests/test_services.py

@@ -11,13 +11,13 @@ async def test_services_is_frozen(self) -> None:
  """Mutation after construction must raise — singletons don't change at runtime."""
  s = _make_services()
  with pytest.raises((AttributeError, TypeError)):
- s.settings = None # type: ignore[misc,assignment] # ty: ignore[invalid-assignment]
+ s.settings = None # type: ignore[misc,assignment]
 
  async def test_services_has_slots(self) -> None:
  """Slotted dataclass prevents silent attribute additions (the original bloat pattern)."""
  s = _make_services()
  with pytest.raises((AttributeError, TypeError)):
- s.rogue_new_attribute = 42 # type: ignore[attr-defined] # ty: ignore[invalid-assignment]
+ s.rogue_new_attribute = 42 # type: ignore[attr-defined]
 
  async def test_services_round_trip_field_access(self) -> None:
  """Every declared field must be readable after construction."""

host/migrations/versions/92965b00f105_users_external_sso_nullable_password_is_.py

@@ -0,0 +1,62 @@
+"""users external sso: nullable password + is_external
+
+Revision ID: 92965b00f105
+Revises: 873ca2015033
+Create Date: 2026-06-23 16:08:20.901942
+"""
+
+from collections.abc import Sequence
+
+import sqlalchemy as sa
+from alembic import op
+
+# revision identifiers, used by Alembic.
+revision: str = "92965b00f105"
+down_revision: str | None = "873ca2015033"
+branch_labels: str | Sequence[str] | None = None
+depends_on: str | Sequence[str] | None = None
+
+# External (SSO) users have no local password, so ``hashed_password`` becomes
+# nullable and a marker column ``is_external`` is added. SQLite cannot ALTER a
+# column's nullability in place, so the change goes through ``batch_alter_table``
+# (a table rebuild on SQLite, a direct ALTER on Postgres). The rebuild can't
+# reflect the expression-based ``lower(email)`` index, so it's dropped and
+# recreated explicitly on SQLite.
+
+
+def upgrade() -> None:
+ bind = op.get_bind()
+ with op.batch_alter_table("users_user") as batch_op:
+ batch_op.add_column(
+ sa.Column(
+ "is_external",
+ sa.Boolean(),
+ server_default=sa.false(),
+ nullable=False,
+ )
+ )
+ batch_op.alter_column(
+ "hashed_password",
+ existing_type=sa.String(length=1024),
+ nullable=True,
+ )
+ if bind.dialect.name == "sqlite":
+ op.execute("DROP INDEX IF EXISTS ix_users_user_email_lower")
+ op.create_index("ix_users_user_email_lower", "users_user", [sa.text("lower(email)")])
+
+
+def downgrade() -> None:
+ # Restoring ``hashed_password NOT NULL`` will fail if any external (SSO)
+ # user exists at downgrade time — they carry a NULL password by design.
+ # Such rows must be deleted or given a password before downgrading.
+ bind = op.get_bind()
+ with op.batch_alter_table("users_user") as batch_op:
+ batch_op.alter_column(
+ "hashed_password",
+ existing_type=sa.String(length=1024),
+ nullable=False,
+ )
+ batch_op.drop_column("is_external")
+ if bind.dialect.name == "sqlite":
+ op.execute("DROP INDEX IF EXISTS ix_users_user_email_lower")
+ op.create_index("ix_users_user_email_lower", "users_user", [sa.text("lower(email)")])

modules/keycloak/keycloak/models.py

@@ -11,7 +11,7 @@
 Base = create_module_base("keycloak")
 
 
-class KeycloakUserCache(Base, table=True):
+class KeycloakUserCache(Base, table=True): # ty: ignore[unsupported-base]
  __tablename__ = "keycloak_user_cache"
 
  id: uuid_mod.UUID = Field(default_factory=uuid_mod.uuid4, primary_key=True)