feat(users): external (SSO) users — null-password provisioning, credential guards & admin UI - #219
Merged
Merged
Conversation
Marks IdP-provisioned users external with a genuinely null password, guards all password-credential paths, surfaces the marking in the admin UI, and keeps normal role assignment. Independent of PR #192. Claude-Session: https://claude.ai/code/session_012kKthZeQPYEUWRL4jygquY
Users created via OAuth/OIDC login (Microsoft/Entra, Google, GitHub, generic OIDC) are now first-class rows in users_user, marked external with a truly NULL password instead of a random one — so they sign in only through their IdP and admins manage their roles like any other user. Backend - models/user.py: hashed_password is now nullable; add is_external (server_default false). Migration 92965b00f105 makes the column nullable + adds the marker via batch_alter_table (SQLite rebuild recreates the lower(email) functional index). Forks off the users head (873ca2015033), not the keycloak branch, keeping the users migration line independent of the optional keycloak provider. - oauth/api.py: flag the request before find-or-create so the manager can mark only *newly provisioned* OAuth users. Logins that link to an existing password account are untouched (on_after_register won't fire) — link-by-email behaviour is preserved. - manager.py: on_after_register nulls the password + sets is_external for OAuth-provisioned users; authenticate() and forgot_password() refuse external users (no local password); generate_reset_password_token raises ExternalUserNoPasswordError instead of hashing None. - admin reset-password-link returns 409 for external users. - expose is_external in UserRead / UserListItem + admin list query. Frontend - list + detail surface an "External · SSO" / SSO badge; the detail page hides the password-reset action for external users and explains why. Tests - test_external_users.py: provisioning (new vs linked), credential guards (login, reset-link, forgot-password no-op), role assignment, and admin-list visibility. Full users suite: 303 passed. No default role is assigned; scope covers any OAuth/OIDC-provisioned user. The standalone oidc module (PR #192) remains separate for the bearer/stateless use case. Claude-Session: https://claude.ai/code/session_012kKthZeQPYEUWRL4jygquY
Code-review (round 1) finding: the bearer-token login path (POST /api/users/auth/token) verified the password directly instead of going through the guarded UserManager.authenticate, so an external user (hashed_password is None) hit verify_and_update(pw, None) -> TypeError -> 500, which also leaked account type via timing/error (missing user got a clean 401 after a dummy hash). Treat null-password users like a missing user: run the dummy hash and return 401. Also align forgot_password's no-op guard to (is_external or hashed_password is None) to match generate_reset_password_token, add a regression test for the bearer path, and note the downgrade constraint. Claude-Session: https://claude.ai/code/session_012kKthZeQPYEUWRL4jygquY
ty 0.0.52 newly flags previously-valid suppressions. Remove four now-unused '# ty: ignore[invalid-assignment]' directives (framework/core/tests/*, users/backend.py) and add '# ty: ignore[unsupported-base]' to keycloak's SQLModel table class, matching the users User model. Comment-only; no behavior change. Unblocks 'make lint' / CI typecheck, which fails repo-wide otherwise. Claude-Session: https://claude.ai/code/session_012kKthZeQPYEUWRL4jygquY
Deploying simple-module-python with |
| Latest commit: | aee833e |
| Status: | ✅ Deploy successful! |
| Preview URL: | https://c9a29539.simple-module-python.pages.dev |
| Branch Preview URL: | https://feat-users-external-sso.simple-module-python.pages.dev |
Uh oh!
There was an error while loading. Please reload this page.
2 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
users_userrows marked external (is_external=True) with a truly NULLhashed_password— they sign in only via their IdP, and admins assign roles to them like any other user.authenticate), bearer-token login (/auth/token), forgot-password, and admin reset-password-link.What's in the branch
b13fe7e): nullablehashed_password+is_externalcolumn (migration92965b00f105, parented on the users head for module independence);on_after_registermarks new OAuth users external and nulls the password; guards inauthenticate/forgot_password/generate_reset_password_token; admin reset-link → 409;is_externalexposed inUserRead/UserListItem+ list query; admin-UI badges. Link-by-email preserved (existing password accounts that OAuth-link stay local); no default role.fd39c91): the bearer-token login path verified the password directly instead of via the guardedUserManager.authenticate, so an external user (hashed_password is None) hitverify_and_update(pw, None)→TypeError→ 500, leaking account type via timing/error. Now treats null-password users like a missing user (dummy hash + clean 401). Adds a regression test; alignsforgot_password's guard; notes the downgrade constraint.aee833e): resolve pre-existingty 0.0.52diagnostics (4 now-unused# ty: ignoredirectives +ignore[unsupported-base]on keycloak's SQLModel table). Comment-only, no behavior change;ty 0.0.52reddensmake lint/ CI typecheck repo-wide otherwise.Verification
test_external_users.py(new-OAuth→external, link-existing→stays-local, session login rejected, bearer login rejected (401 not 500), admin reset-link 409, forgot-password no-op, admin role assignment, admin-list exposesis_external).Test plan
/users/adminand confirms the SSO badge appears only on external users/auth/token)