feat: pluggable auth — AuthProvider contract + Keycloak OIDC module + bearer tokens - #184
Merged
Merged
Conversation
Introduces the AuthProvider contract that allows swapping the users module for a Keycloak OIDC module. Covers bearer token support for mobile clients, JWT validation via JWKS, role mapping, and the SM020/SM021 conflict diagnostics. Builds on the principal-resolver chain from the #163 spec.
16-task TDD plan covering AuthProvider protocol, provider-agnostic middleware, UsersAuthProvider, Keycloak OIDC module (JWKS, token exchange, user cache), bearer/refresh token endpoints, SM020/SM021 diagnostics, and integration tests.
Add POST /api/users/auth/token (email+password login), POST /api/users/auth/token/refresh (rotate tokens), and DELETE /api/users/auth/token (revoke) for non-browser clients. New model: RefreshToken with expiry and revocation tracking. New settings: bearer_token_lifetime_seconds, refresh_token_lifetime_seconds. Also fix keycloak module depends_on to include Settings (was failing tests). Migration not generated — needs full app context.
Add 6 integration tests verifying both auth providers satisfy the AuthProvider protocol, SM020/SM021 diagnostics fire correctly, and module metadata is consistent. Fix 4 regressions surfaced by the full suite: - keycloak depends_on assertion (["Auth"] -> ["Auth", "Settings"]) - SM020 fires in test_app_state_has_sm_services (exclude Keycloak) - typer 0.26 vendored click breaks Exit catch in CLI test - PydanticUndefined default_factory fields crash settings serializer - Remove keycloak/tests/__init__.py that shadowed root tests package
JWKSCache now requires issuer and audience at construction time and always verifies them. Previously these were optional with empty-string defaults, which silently disabled the checks — a token from any issuer or audience would be accepted.
UsersAuthProvider.resolve_user() now checks for Authorization: Bearer headers and validates the access token against the users_access_token table before falling back to session-cookie resolution. This enables mobile/API clients to authenticate using tokens from POST /api/users/auth/token.
BUG-001: POST /api/users/auth/logout now clears the Starlette session in addition to deleting the sm_auth cookie. The stock fastapi-users logout at /auth-inner/logout only cleared the cookie, leaving the session user_id intact so the user remained authenticated. BUG-002: token_login now runs a dummy bcrypt verification when the user is not found, preventing timing-based email enumeration (existing user took ~50ms for bcrypt, missing user was instant).
The login_page view returned a coroutine instead of a response, causing a 500 error. inertia.render() is async and must be awaited.
Deploying simple-module-python with |
| Latest commit: | 806996e |
| Status: | ✅ Deploy successful! |
| Preview URL: | https://599a5dad.simple-module-python.pages.dev |
| Branch Preview URL: | https://worktree-pluggable-auth-keyc.simple-module-python.pages.dev |
…gDict false positive - package-lock.json: add @simple-module/keycloak workspace entry - test_jwks.py: mark refetch test to allow unconsumed mock responses (pytest-httpx strict mode in CI) - pyproject.toml: ignore ty invalid-assignment rule (SQLModel ConfigDict vs SQLModelConfig false positive across all modules)
- Regenerate i18n keys with all module locales (dashboard, bg_tasks, etc. were missing from the previous gen-pages run) - Rewrite JWKS refetch test to use unittest.mock.patch instead of pytest-httpx (the sequential mock response ordering was unreliable across pytest-httpx versions)
- test_jwks.py: remove unused httpx and AsyncMock imports - pr.yml: add SM_MODULES_ENABLED to E2E smoke job to exclude Keycloak (SM020 prevents both users and keycloak from running simultaneously; the E2E smoke tests use the users module)
Uh oh!
There was an error while loading. Please reload this page.
antosubash added a commit
that referenced
this pull request
May 28, 2026
PR #184 introduced SM_MODULES_ENABLED to exclude Keycloak from the E2E smoke job. The allowlist must now also include AuditLog so the module's routes (/audit_log, /api/audit_log) are mounted — otherwise tests/e2e/test_audit_log_ui.py hits 404.
antosubash added a commit
that referenced
this pull request
May 28, 2026
* docs: add audit log module design spec Covers data model, capture mechanism (SQLAlchemy before_flush callback), module structure, REST API, admin Browse page, and testing strategy. * docs: add audit log module implementation plan 13-task TDD plan covering framework changes (AuditRecord, callback wiring), module scaffold, model, service, API, Inertia Browse page, tests, and migration. * feat(db): add AuditRecord dataclass and collect_audit_records diff logic * feat(db): wire audit callback into DatabaseState and before_flush listener * feat(audit_log): scaffold module package with constants and host wiring * feat(audit_log): add AuditEntry model and DTO schemas * feat(audit_log): add capture callback * feat(audit_log): add service layer and deps * feat(audit_log): add API and Inertia view endpoints * feat(audit_log): add AuditLogModule with lifecycle hooks * feat(audit_log): add i18n locale strings * feat(audit_log): add Browse.tsx page with filters, pagination, and change diffs * test(audit_log): add integration tests for capture, API filtering, and recursion guard Also fix AuditEntryRead.id type from str to uuid.UUID to match the AuditEntry model (surfaced by the new tests). * migration(audit_log): add audit_log_audit_entry table * chore: regenerate i18n and package-lock with audit_log module * fix(audit_log): use datetime-local inputs and preserve page_size in navigation * fix(test): ensure test_filter_by_action is non-vacuous * fix(audit_log): deterministic pagination, error-resilient capture, entity_id filter, non-null created_at * fix(db): correctly classify soft-deleted entities and exclude SoftDeleteMixin fields from audit diffs * fix(audit_log): gracefully handle invalid query params in view endpoint View endpoint now accepts page/page_size as raw strings and sanitizes them (clamp to valid range, fall back to defaults on parse failure) instead of relying on FastAPI Query(ge=, le=) constraints that produce raw JSON 422 errors unfriendly for Inertia page visits. API endpoint retains strict validation — callers get proper 422s. * fix(db): two-phase audit capture resolves DB-assigned integer PKs BUG-002: Entities with DB-assigned integer PKs (e.g. id: int | None = Field(default=None, primary_key=True)) were recorded in the audit log with entity_id="" because _entity_pk_str() ran in before_flush while the PK was still None. UUID PKs were unaffected because default_factory populates them Python-side. The fix splits audit capture into two phases: Phase 1 (before_flush): snapshot_changes() reads attribute history (which is wiped after flush) and stores per-entity diffs alongside the live object reference in session.info — not yet resolved entity_ids. Phase 2 (after_flush_postexec): _after_flush_audit pops the pending snapshots, calls finalize_records() to resolve entity_id from the now-populated PK, and dispatches to the audit_callback. The added AuditEntry rows land in session.new and are flushed when commit runs autoflush. collect_audit_records remains a public single-phase wrapper for tests and any caller whose PKs are already populated. * chore(audit_log): format files, fix frozenset typing, add e2e specs - ruff format applied (migration, service.py, e2e spec) - ruff check fixed unused __init__.py imports - _excluded_fields return type matches actual frozenset usage - Add 3 e2e tests for audit_log UI (renders, integer-PK regression, filter) * chore: add QA report and verification screenshots * style(audit_log): align Browse page layout with other module pages * fix(audit_log): satisfy CI checks for the layout pass - Extract FilterBar into its own component so Browse.tsx stays under the 300-line cap - Wire htmlFor/id on every filter label so biome's noLabelWithoutControl is satisfied - Split test fixtures into _audit_models.py so test_audit.py drops back under 300 lines * fix(audit_log): inline useT in ChangesList to satisfy TS strict typing ChangesList previously received the t function via props with a loose TFn alias. react-i18next's useT returns a strictly-typed t that wasn't assignable to the alias. Switching to a local useT() call inside the component drops the TFn alias entirely. * fix(ci): unblock PR checks - Ignore ty's invalid-assignment rule globally — every SQLModel contracts/schemas.py with ``model_config = ConfigDict(...)`` trips it because ty cannot see that SQLModelConfig is compatible with pydantic's ConfigDict. Same pattern already used for invalid-argument-type. Run on main locally surfaces the same noise. - Catch ``typer.Exit`` (not ``click.exceptions.Exit``) in test_missing_pyproject_exits_nonzero — modern typer (>=0.20) vendors click under ``typer._click`` so the two classes diverged. - Extract ``_MODULE_USERS`` constant in audit_log/module.py — the hardcoded-strings check rejects module-name literals in depends_on. Matches the pattern in dashboard/module.py. - Fill in audit_log/README.md with Install + Usage sections — the READMEs check requires both. - Drop unused ``# ty: ignore[invalid-assignment]`` comment now that the rule is globally ignored. * ci(e2e): include AuditLog in SM_MODULES_ENABLED allowlist PR #184 introduced SM_MODULES_ENABLED to exclude Keycloak from the E2E smoke job. The allowlist must now also include AuditLog so the module's routes (/audit_log, /api/audit_log) are mounted — otherwise tests/e2e/test_audit_log_ui.py hits 404.
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
Make the framework's authentication layer swappable. Framework users install either
simple-module-users(local credentials + OAuth) or a newsimple-module-keycloak(Keycloak OIDC) — both implement the sameAuthProviderprotocol so every other module works unchanged.auth/contracts/provider.py— the contract both providers implementusers/toauth/— delegates to the active provider, returns 401 JSON for API paths and 302 redirect for browser viewsPOST /api/users/auth/token(login),POST .../refresh(rotation),DELETE .../token(revoke) for mobile/API clientskeycloakmodule — OIDC Authorization Code flow, JWKS JWT validation with key-rotation retry, role mapping, lightweight user cache tableScreenshots
Users Module (Login + Dashboard)
Keycloak Module (OIDC Flow)
QA Summary
awaiton inertia.render, API logout not clearing session, timing side-channel on token login, bearer tokens not resolved by providerKey Design Decisions
UserContextremains the single identity type — every downstream consumer (menus, permissions, audit, Inertia props) is unaffected by the provider swapUserContextin the Starlette session cookie — independent of Keycloak token lifetimeTest Plan
make lintpasses (ruff, ty, biome, tsc, file-size, metadata)make testpasses (1277 Python + 16 JS)