refactor(keycloak): follow the module layout (moves only)

CHANGELOG.md

@@ -16,12 +16,13 @@ All notable changes to this project are documented in this file. The format is b
 
 ### Changed
 
-- Scaffold new modules with `core/` and `web/` areas and mirrored nested Python tests; existing modules migrate individually. See `docs/module-layout.md`.
+- Scaffold new modules with `core/` and `web/` areas and mirrored nested Python tests; existing modules migrate individually. See `docs/module-layout.md`. Moved modules log under their new paths (for example `mail.delivery.sender`), so logging config that names an old logger needs the new name.
 - `mail` follows the module layout: `mail.core.{constants,settings,state,data_compatibility}` and `mail.delivery.{sender,runtime}`. `mail.settings` and `mail.constants` still import, with a `DeprecationWarning`, until 0.0.46.
 - `site_lock` follows the module layout: `site_lock.core.{constants,settings,state}` and `site_lock.web.{middleware,page,rate_limit}` (the unlock template moves with the page). `site_lock.settings` still imports, with a `DeprecationWarning`, until 0.0.46.
 - `sharing` follows the module layout: `sharing.core.{constants,settings,services,service,policy,audience,tenant_cleanup,user_cleanup}`, `sharing.web.{deps,present,endpoints}`, and the page components and types move to `ui/`. `sharing.policy`, `sharing.constants` and `sharing.endpoints.api` still import, with a `DeprecationWarning`, until 0.0.46; new code imports `sharing.core.policy`.
 - `dashboard` follows the module layout: `dashboard.core.constants`, `dashboard.web.{endpoints,stats,doctor}`, and the page components move from `pages/components/` to `ui/components/`. Nothing outside the module imported the old paths, so no aliases remain.
 - `feature_flags` follows the module layout: `feature_flags.core.{constants,service,invalidation,tenant_cleanup}`, `feature_flags.web.{deps,endpoints}`, and the page components and types move to `ui/`. `feature_flags.constants` still imports, with a `DeprecationWarning`, until 0.0.46.
+- `keycloak` follows the module layout: `keycloak.core.{constants,settings,state}`, `keycloak.identity.{provider,oidc,renewal,directory,persistence,tokens,jwks}` and `keycloak.web.endpoints`. `keycloak.{constants,settings,provider,jwks,tokens}` still import, with a `DeprecationWarning`, until 0.0.46.
 
 ### Fixed
 

docs/framework/middleware.md

@@ -96,7 +96,7 @@ Behind a TLS-terminating proxy this is **recommended**: without it the app belie
 
 It used to be load-bearing for the UI too, because Inertia's page url was absolute and its cross-scheme `pushState` threw a `SecurityError` on every page (GH #223). The page url is root-relative now, so that failure mode is gone whether or not this is set.
 
-It is still load-bearing for anything else that reads `request.url.scheme` or calls `request.url_for(...)` to build an absolute url — OAuth/OIDC's `callback_url` (`modules/users/users/oauth/api.py`, `modules/keycloak/keycloak/endpoints/api.py`) and the locale cookie's `secure` flag (`simple_module_hosting/i18n_routes.py`) among them. Left unset behind a TLS-terminating proxy, an OAuth redirect_uri is built as `http://…` and most providers reject it as a mismatch against the `https://…` one registered in their console.
+It is still load-bearing for anything else that reads `request.url.scheme` or calls `request.url_for(...)` to build an absolute url — OAuth/OIDC's `callback_url` (`modules/users/users/oauth/api.py`, `modules/keycloak/keycloak/web/endpoints/api.py`) and the locale cookie's `secure` flag (`simple_module_hosting/i18n_routes.py`) among them. Left unset behind a TLS-terminating proxy, an OAuth redirect_uri is built as `http://…` and most providers reject it as a mismatch against the `https://…` one registered in their console.
 
 Set it to a comma-separated list of proxy IPs/CIDRs, or `*` to trust any peer — correct when the container is only reachable through one proxy, wrong when anything else can connect.
 

docs/framework/user-directory.md

@@ -47,7 +47,7 @@ The active auth provider installs its directory in `register_settings` with
 | Provider | Directory | Knows |
 |---|---|---|
 | `users` | `users.directory.UsersDirectory` | every local account, roles via `users_user_role`, all three stats |
-| `keycloak` | `keycloak.directory.KeycloakDirectory` | users who have signed in at least once (`keycloak_user_cache`); roles `None`; `created_this_month` `None`; `entity_type` `None` |
+| `keycloak` | `keycloak.identity.directory.KeycloakDirectory` | users who have signed in at least once (`keycloak_user_cache`); roles `None`; `created_this_month` `None`; `entity_type` `None` |
 | none | `NullUserDirectory` | nobody; every stat `None`; `match_ids` `None` |
 
 A new provider implements the four methods and `entity_type`, and registers

docs/module-layout.md

@@ -213,7 +213,7 @@ when a later module PR moves files.
 | dashboard | 2 | 0 | follows this layout |
 | feature_flags | 3 (+1 shim) | 0 | follows this layout |
 | file_storage | 39 | 65 | flat |
-| keycloak | 13 | 19 | flat |
+| keycloak | 3 (+5 shims) | 0 | follows this layout |
 | mail | 2 (+2 shims) | 0 | follows this layout |
 | oidc | 16 | 24 | flat |
 | permissions | 12 | 14 | flat |

docs/modules/keycloak.md

@@ -53,7 +53,7 @@ The browser session uses a signed, **unencrypted** cookie, so the refresh token
 
 ## Bearer-token validation
 
-`JWKSCache` (`keycloak/jwks.py`) caches Keycloak's RS256 public signing keys (`/realms/<realm>/protocol/openid-connect/certs`) and validates tokens with PyJWT, requiring `iss`, non-empty `sub`, `aud`, integer `exp` and integer `iat`.
+`JWKSCache` (`keycloak/identity/jwks.py`) caches Keycloak's RS256 public signing keys (`/realms/<realm>/protocol/openid-connect/certs`) and validates tokens with PyJWT, requiring `iss`, non-empty `sub`, `aud`, integer `exp` and integer `iat`.
 The issuer must match and `client_id` must be among the token's `aud` values
 (extra audiences and the `azp` are not checked, see below). A 10-second clock leeway tolerates small clock
 skew, while browser authority still ends at the access token's exact expiry.
@@ -64,7 +64,7 @@ On a valid bearer token the provider upserts a `KeycloakUserCache` row keyed by
 
 ## Browser login flow (OIDC)
 
-`OIDCClient` (`keycloak/oidc.py`) wraps the realm's authorization-code endpoints.
+`OIDCClient` (`keycloak/identity/oidc.py`) wraps the realm's authorization-code endpoints.
 
 | Method + path | What |
 |---|---|
@@ -141,7 +141,7 @@ Keycloak registers no setup steps: its local `users` table is legitimately empty
 | `last_login_at` | `datetime \| None` | written on login; bearer activity writes throttled to once per five minutes |
 
 The cache backs this module's [user directory](/framework/user-directory)
-(`keycloak.directory.KeycloakDirectory`, installed in `register_settings`), so
+(`keycloak.identity.directory.KeycloakDirectory`, installed in `register_settings`), so
 the audit log, dashboard, file storage and the permissions grants screen name
 and count Keycloak users. It knows only users who have signed in at least once;
 roles are `None` (they live in the IdP) and `created_this_month` is `None`.

host/tests/test_inactive_provider_public_paths.py

@@ -9,7 +9,7 @@
 
 from __future__ import annotations
 
-from keycloak.constants import PUBLIC_PATH_PREFIXES as KEYCLOAK_PREFIXES
+from keycloak.core.constants import PUBLIC_PATH_PREFIXES as KEYCLOAK_PREFIXES
 from keycloak.module import KeycloakModule
 from simple_module_core.discovery import select_auth_provider
 from simple_module_core.public_routes import PublicRouteRegistry

host/tests/test_module_layout.py

@@ -32,6 +32,7 @@
 )
 # Add named domain areas here only when they contain several related features.
 DOMAIN_AREAS: dict[str, frozenset[str]] = {
+    "keycloak": frozenset({"identity"}),
     "mail": frozenset({"delivery"}),
 }
 
@@ -84,12 +85,6 @@
         uploads.py visibility.py worker.py worker_support.py
         """.split()
     ),
-    "keycloak": frozenset(
-        """
-        constants.py directory.py endpoints jwks.py oidc.py persistence.py provider.py
-        renewal.py settings.py state.py tokens.py
-        """.split()
-    ),
     "oidc": frozenset(
         """
         client.py constants.py directory.py discovery.py endpoints jwks.py persistence.py
@@ -138,6 +133,11 @@ def _is_layout_shim(path: Path) -> bool:
     return "deprecated_module(" in text and "sys.modules[__name__]" in text
 
 
+def _only_bytecode(path: Path) -> bool:
+    """A directory an older checkout left behind holding only ``__pycache__``."""
+    return all(child.name == "__pycache__" for child in path.iterdir())
+
+
 def _is_shim_package(path: Path) -> bool:
     """An old package path kept only for its aliases: shims plus a docstring __init__."""
     files = [f for f in path.glob("*.py") if f.name != "__init__.py"]
@@ -152,7 +152,12 @@ def test_module_root_layout(module: str) -> None:
         p.name
         for p in package.iterdir()
         if (p.suffix == ".py" and not _is_layout_shim(p))
-        or (p.is_dir() and p.name != "__pycache__" and not _is_shim_package(p))
+        or (
+            p.is_dir()
+            and p.name != "__pycache__"
+            and not _only_bytecode(p)
+            and not _is_shim_package(p)
+        )
     }
     allowed = ALLOWED_ROOT | DOMAIN_AREAS.get(module, frozenset())
     legacy = LEGACY_FLAT.get(module, frozenset())

modules/auth/tests/test_auth_oidc_authority.py

@@ -7,8 +7,8 @@
 import httpx
 import pytest
 from auth.contracts.schemas import UserContext
-from keycloak.provider import KeycloakAuthProvider
-from keycloak.settings import KeycloakSettings
+from keycloak.core.settings import KeycloakSettings
+from keycloak.identity.provider import KeycloakAuthProvider
 from oidc.provider import OidcAuthProvider
 from oidc.settings import OidcSettings
 from simple_module_hosting.session import SessionMiddleware

modules/auth/tests/test_auth_oidc_jwks_cache.py

@@ -9,7 +9,7 @@
 import pytest
 from cryptography.hazmat.primitives.asymmetric import rsa
 from jwt.algorithms import RSAAlgorithm
-from keycloak.jwks import JWKSCache as KeycloakJWKS
+from keycloak.identity.jwks import JWKSCache as KeycloakJWKS
 from oidc.jwks import JWKSCache as OidcJWKS
 
 

modules/auth/tests/test_auth_oidc_jwt_hardening.py

@@ -5,7 +5,7 @@
 import jwt
 import pytest
 from cryptography.hazmat.primitives.asymmetric import rsa
-from keycloak.jwks import JWKSCache as KeycloakJWKS
+from keycloak.identity.jwks import JWKSCache as KeycloakJWKS
 from oidc.jwks import JWKSCache as OidcJWKS
 
 

modules/auth/tests/test_auth_oidc_tokens.py

@@ -7,7 +7,7 @@
 from cryptography.fernet import Fernet
 from cryptography.hazmat.primitives import hashes
 from cryptography.hazmat.primitives.kdf.hkdf import HKDF
-from keycloak import tokens as keycloak_tokens
+from keycloak.identity import tokens as keycloak_tokens
 from oidc import tokens as oidc_tokens
 
 

modules/keycloak/keycloak/constants.py

@@ -1,22 +1,11 @@
-"""Keycloak module constants."""
+"""Moved to :mod:`keycloak.core.constants`; this alias goes in 0.0.46 (module layout)."""
 
 from __future__ import annotations
 
-# Anonymous-allowed prefixes, registered in ``KeycloakModule.register_public_routes``.
-PUBLIC_PATH_PREFIXES = (
-    "/keycloak/login",
-    "/keycloak/logout",
-    # Where the end-session redirect lands: there is no session left to check,
-    # and bouncing it to /keycloak/login would sign the visitor straight back in.
-    "/keycloak/logged-out",
-    "/api/keycloak/auth/",
-)
+import importlib
+import sys
 
-SESSION_REFRESH_TOKEN_KEY = "keycloak_refresh_token"
-SESSION_REFRESH_RETRY_AT_KEY = "keycloak_refresh_retry_at"
-SESSION_ID_TOKEN_KEY = "keycloak_id_token"
-SESSION_USER_KEY = "user_ctx"
-SESSION_STATE_KEY = "keycloak_oidc_state"
-SESSION_NONCE_KEY = "keycloak_oidc_nonce"
-SESSION_VERIFIER_KEY = "keycloak_oidc_verifier"
-SESSION_IDENTITY_KEY = "keycloak_identity"
+from simple_module_core.deprecation import deprecated_module
+
+deprecated_module("keycloak.constants", remove_in="0.0.46", use="keycloak.core.constants")
+sys.modules[__name__] = importlib.import_module("keycloak.core.constants")

modules/keycloak/keycloak/core/__init__.py

@@ -0,0 +1 @@
+"""What the Keycloak provider builds on: constants, settings and app state."""

modules/keycloak/keycloak/core/constants.py

@@ -0,0 +1,22 @@
+"""Keycloak module constants."""
+
+from __future__ import annotations
+
+# Anonymous-allowed prefixes, registered in ``KeycloakModule.register_public_routes``.
+PUBLIC_PATH_PREFIXES = (
+    "/keycloak/login",
+    "/keycloak/logout",
+    # Where the end-session redirect lands: there is no session left to check,
+    # and bouncing it to /keycloak/login would sign the visitor straight back in.
+    "/keycloak/logged-out",
+    "/api/keycloak/auth/",
+)
+
+SESSION_REFRESH_TOKEN_KEY = "keycloak_refresh_token"
+SESSION_REFRESH_RETRY_AT_KEY = "keycloak_refresh_retry_at"
+SESSION_ID_TOKEN_KEY = "keycloak_id_token"
+SESSION_USER_KEY = "user_ctx"
+SESSION_STATE_KEY = "keycloak_oidc_state"
+SESSION_NONCE_KEY = "keycloak_oidc_nonce"
+SESSION_VERIFIER_KEY = "keycloak_oidc_verifier"
+SESSION_IDENTITY_KEY = "keycloak_identity"

modules/keycloak/keycloak/core/settings.py

@@ -0,0 +1,82 @@
+"""Keycloak module settings -- DB-backed via ``register_module_settings``."""
+
+from __future__ import annotations
+
+from pydantic import Field, field_validator, model_validator
+from pydantic_settings import SettingsConfigDict
+from simple_module_core.dotenv import env_bool, env_str
+from simple_module_core.environments import NON_PROD_ENVIRONMENTS
+from simple_module_core.redirect_safety import non_empty_redirect
+from simple_module_core.settings_base import DbBackedSettings
+from simple_module_core.stored_settings import field_error
+
+DEFAULT_LOGIN_REDIRECT_URL = "/dashboard/"
+
+
+class KeycloakSettings(DbBackedSettings):
+    """Keycloak OIDC configuration."""
+
+    # ``DbBackedSettings`` (not ``BaseSettings``) so the DB is genuinely the
+    # only source: omitting ``env_prefix`` would leave pydantic-settings
+    # reading each field from its bare name — GH #283.
+    model_config = SettingsConfigDict(extra="ignore")
+
+    server_url: str = env_str("SM_KEYCLOAK_SERVER_URL", "")
+    realm: str = env_str("SM_KEYCLOAK_REALM", "")
+    client_id: str = env_str("SM_KEYCLOAK_CLIENT_ID", "")
+    client_secret: str = env_str("SM_KEYCLOAK_CLIENT_SECRET", "")
+
+    # Off by default: the JWT ``tenant_id`` claim is only as trustworthy as the
+    # realm mapper that produces it. A mapper reading an attribute the user can
+    # edit lets them name their own tenant. Turn on only for a realm whose
+    # protocol mapper users cannot influence; the ``tenants`` module's
+    # resolver ignores the claim either way.
+    trust_tenant_claim: bool = env_bool("SM_KEYCLOAK_TRUST_TENANT_CLAIM", False)
+
+    roles_claim_path: str = "realm_access.roles"
+    admin_role: str = "admin"
+    login_redirect_url: str = DEFAULT_LOGIN_REDIRECT_URL
+    jwks_cache_ttl_seconds: int = 3600
+
+    role_mapping: dict[str, str] = Field(
+        default_factory=lambda: {"admin": "admin", "user": "user"},
+    )
+
+    @field_validator("login_redirect_url")
+    @classmethod
+    def _non_empty_redirect(cls, value: str) -> str:
+        """Blank is never a usable navigation target.
+
+        The callback puts this straight into a ``Location`` header, and an
+        admin can clear it in the generic module-settings editor. Normalising
+        on the class covers hydration and ``apply_changes_and_reload`` alike.
+        Users' provider has its own copy of this field and does the same.
+        """
+        return non_empty_redirect(value, default=DEFAULT_LOGIN_REDIRECT_URL)
+
+    @model_validator(mode="after")
+    def _check_required_in_production(self) -> KeycloakSettings:
+        import os
+
+        env = os.environ.get("SM_ENVIRONMENT", "development")
+        if env in NON_PROD_ENVIRONMENTS:
+            return self
+        required = (
+            ("server_url", "SM_KEYCLOAK_SERVER_URL"),
+            ("realm", "SM_KEYCLOAK_REALM"),
+            ("client_id", "SM_KEYCLOAK_CLIENT_ID"),
+            ("client_secret", "SM_KEYCLOAK_CLIENT_SECRET"),
+        )
+        missing = [(field, var) for field, var in required if not getattr(self, field)]
+        if missing:
+            # Located on the first empty field so the form shows it there.
+            msg = f"Keycloak settings required in production: {', '.join(v for _, v in missing)}"
+            raise field_error(
+                type(self),
+                missing[0][0],
+                "keycloak_settings_required",
+                msg,
+                value="",
+                related=[field for field, _ in required],
+            )
+        return self

modules/keycloak/keycloak/state.py → modules/keycloak/keycloak/core/state.py

@@ -6,8 +6,8 @@
 from typing import TYPE_CHECKING
 
 if TYPE_CHECKING:
-    from keycloak.jwks import JWKSCache
-    from keycloak.settings import KeycloakSettings
+    from keycloak.core.settings import KeycloakSettings
+    from keycloak.identity.jwks import JWKSCache
 
 
 @dataclass

modules/keycloak/keycloak/identity/__init__.py

@@ -0,0 +1,5 @@
+"""The auth provider.
+
+OIDC client, token renewal and encryption, JWKS, the user directory and its
+cache persistence.
+"""

modules/keycloak/keycloak/identity/jwks.py

@@ -0,0 +1,5 @@
+"""Provider entry point for the shared JWKS validator."""
+
+from auth.oidc_common.jwks import JWKSCache
+
+__all__ = ["JWKSCache"]