diff --git a/.env.example b/.env.example index 7e390cc..6d2a6b0 100644 --- a/.env.example +++ b/.env.example @@ -16,6 +16,9 @@ IDP_DB_PASSWORD= # from KV: secret/idp/db IDP_EXTERNAL_PORT=8080 # Public base URL / hostname Keycloak advertises (behind the WAF in prod). IDP_EXTERNAL_HOSTNAME=http://localhost:8080 +# Cache stack: `local` (default; single-node standalone compose) or `ispn` +# for clustered deployments. See the KC_CACHE note in docker-compose.yml. +IDP_CACHE_MODE=local # Bootstrap admin. Created ONCE; retire after registering a passkey. IDP_BOOTSTRAP_ADMIN_USERNAME=idp-admin diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index c9c18e9..0ac8706 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -5,6 +5,12 @@ on: branches: [main] pull_request: +# Cancel superseded evidence for the same pull request or branch. This keeps the +# runner queue bounded during review-fix loops while preserving the newest head. +concurrency: + group: ${{ github.workflow }}-${{ github.event.pull_request.number || github.ref }} + cancel-in-progress: true + # Least-privilege default token (OSSF Scorecard: Token-Permissions). permissions: contents: read diff --git a/.github/workflows/codeql.yml b/.github/workflows/codeql.yml index 2ea7a5a..2020895 100644 --- a/.github/workflows/codeql.yml +++ b/.github/workflows/codeql.yml @@ -5,6 +5,12 @@ on: branches: [main] pull_request: +# Code scanning evidence is head-specific; cancel scans made obsolete by a +# newer commit on the same pull request or branch. +concurrency: + group: ${{ github.workflow }}-${{ github.event.pull_request.number || github.ref }} + cancel-in-progress: true + permissions: actions: read contents: read diff --git a/.github/workflows/hourly-pr-steward.yml b/.github/workflows/hourly-pr-steward.yml new file mode 100644 index 0000000..cd973a9 --- /dev/null +++ b/.github/workflows/hourly-pr-steward.yml @@ -0,0 +1,105 @@ +name: Hourly PR steward + +on: + schedule: + # Avoid the top-of-hour congestion window. Scheduled runs use UTC and the + # latest commit on the default branch. + - cron: "17 * * * *" + workflow_dispatch: + +# Keep the workflow token read-only by default. Only the single steward job +# receives the narrowly scoped writes required to update trusted branches and +# arm GitHub-native auto-merge after protected evidence is complete. +permissions: + contents: read + +concurrency: + group: hourly-pr-steward + cancel-in-progress: false + +jobs: + advance-approved-pull-requests: + name: Advance approved pull requests + runs-on: ubuntu-latest + timeout-minutes: 10 + permissions: + contents: write + pull-requests: write + checks: read + steps: + - name: Update, verify, and arm trusted pull requests + env: + GH_TOKEN: ${{ github.token }} + REPOSITORY: ${{ github.repository }} + shell: bash + run: | + set -euo pipefail + + gh pr list \ + --repo "$REPOSITORY" \ + --state open \ + --limit 100 \ + --json number,isDraft,author,headRepositoryOwner,headRefOid,mergeStateStatus,reviewDecision \ + > "$RUNNER_TEMP/open-pull-requests.json" + + jq -c '.[]' "$RUNNER_TEMP/open-pull-requests.json" | while IFS= read -r pull_request; do + number="$(jq -r '.number' <<<"$pull_request")" + is_draft="$(jq -r '.isDraft' <<<"$pull_request")" + author="$(jq -r '.author.login // ""' <<<"$pull_request")" + head_owner="$(jq -r '.headRepositoryOwner.login // ""' <<<"$pull_request")" + head_sha="$(jq -r '.headRefOid' <<<"$pull_request")" + merge_state="$(jq -r '.mergeStateStatus // "UNKNOWN"' <<<"$pull_request")" + review_decision="$(jq -r '.reviewDecision // ""' <<<"$pull_request")" + + if [[ "$is_draft" != "false" || "$head_owner" != "ContextualWisdomLab" ]]; then + continue + fi + + trusted_author=false + for allowed_author in \ + seonghobae \ + dependabot \ + 'dependabot[bot]' \ + app/dependabot \ + github-actions \ + 'github-actions[bot]' \ + app/github-actions \ + opencode-agent + do + if [[ "$author" == "$allowed_author" ]]; then + trusted_author=true + break + fi + done + if [[ "$trusted_author" != "true" ]]; then + continue + fi + + # Keep trusted branches current. A successful update invalidates the + # old check evidence, so the steward waits for the next hourly pass. + if [[ "$merge_state" == "BEHIND" ]]; then + gh pr update-branch "$number" --repo "$REPOSITORY" || true + continue + fi + + if [[ "$review_decision" != "APPROVED" ]]; then + continue + fi + + # Never infer safety from optional checks. The repository's required + # check set remains the source of truth. `gh pr checks` exits nonzero + # for failed checks and uses exit code 8 for pending checks, so either + # condition leaves the PR untouched. + if ! gh pr checks "$number" --repo "$REPOSITORY" --required; then + continue + fi + + # Arm GitHub's native auto-merge service rather than creating the + # merge commit directly with GITHUB_TOKEN. Rulesets remain final, + # and the exact reviewed/check head must still match. + gh pr merge "$number" \ + --repo "$REPOSITORY" \ + --auto \ + --squash \ + --match-head-commit "$head_sha" + done diff --git a/CHANGELOG.md b/CHANGELOG.md new file mode 100644 index 0000000..774d92a --- /dev/null +++ b/CHANGELOG.md @@ -0,0 +1,60 @@ +# Changelog + +All notable changes to Keyverse are documented in this file. The format follows +Keep a Changelog, and releases use semantic versioning. + +## [Unreleased] + +### Added + +- Password-free headless registration that sends one bounded Keycloak action + email for address verification and passkey enrollment, with failure-atomic + account rollback. +- Modular product-facing Keycloak Admin API extensions for registration and + runtime identity-provider federation. +- Runtime federation desired-state convergence with explicit applied-state + reporting and fail-closed operator-response redaction. +- Router-level validation for decoded privileged and SCIM path parameters, + including protocol-native SCIM error responses. +- Persistent Compose and Helm storage for audit and user-operation lock data. +- Optional Helm enforcement of immutable account-unification image digests. +- Concurrency and lifecycle regressions for SQLite-backed configuration, audit, + and mutation-lock persistence. + +### Changed + +- The bound Keycloak browser flow is now strictly passkey-only; registration no + longer creates a bootstrap password or runs a credential janitor. +- The public `naruon-web` access-token lifespan is reduced to five minutes while + the longer SSO session remains available through token refresh and reissue. +- Registration configuration is all-or-nothing and requires a distinct bearer + token, relying-party client, HTTPS redirect URI, and bounded action-link + lifetime. +- Registration throttling is isolated by direct peer address rather than one + process-wide counter. +- Account merge and SCIM replacement now share the same user-operation lock + boundary. +- SQLite configuration and audit stores support safe multi-threaded access with + WAL mode and bounded busy timeouts. +- Application shutdown closes Keycloak, audit, and configuration resources and + removes temporary test-only mutation-lock sidecars deterministically. +- Keycloak Admin API requests refresh an expired bearer token once, including + account creation and action-email enrollment. + +### Fixed + +- Prevented registration races from surfacing raw Keycloak duplicate-user + errors by mapping exact HTTP 409 responses to a stable product conflict. +- Prevented unusable registration orphans by deleting accounts when Keycloak + rejects the verification/passkey action email. +- Prevented external network calls from executing while the federation desired- + state storage lock is held. +- Prevented unknown federation configuration keys, credentials, and private + values from being echoed through list, get, or update responses. +- Rejected Unicode-confusable federation aliases outside the explicit ASCII + slug alphabet. +- Raised non-success health responses correctly in the restricted stdlib HTTP + opener. +- Replaced a potentially expensive registration email regular expression with + deterministic bounded parsing. +- Made standalone audit history survive container replacement. diff --git a/deploy/keycloak/README.md b/deploy/keycloak/README.md index ba018a8..feaf209 100644 --- a/deploy/keycloak/README.md +++ b/deploy/keycloak/README.md @@ -1,43 +1,86 @@ # Keycloak config-as-code -cwl-idp runs on **Keycloak** (Apache-2.0). The realm is declared as-code and -imported at container start; secrets are patched afterwards from the KV store. +Keyverse runs on **Keycloak** (Apache-2.0). The portable `cwl` realm shape is +imported at container start; deployment-specific secrets and external identity +providers are converged afterwards from the KV/DB source of truth. -| File | What | +| File | Responsibility | | --- | --- | -| `realm-cwl.json` | The `cwl` realm: passkey-first passwordless browser flow, OIDC/OAuth2.1 RP client template, employer ADFS SAML IdP (inbound), LDAP/AD federation, and the account-unification service-account client. Imported via `start --import-realm`. | -| `kcadm-bootstrap.sh` | Post-import patch: injects secrets/URLs (ADFS metadata, LDAP bind credential, service-account client secret) from KV using `kcadm.sh`, and grants the service account `realm-management` view-users/manage-users. | +| `realm-cwl.json` | Portable passwordless realm, shared client scopes, RP template, concrete `naruon-web` PKCE client, and account-unification service client | +| `kcadm-bootstrap.sh` | Idempotently inject the service-client secret, grant least-privilege realm-management roles, and reconcile the role mapper | +| `../templates/` | Reference payloads for runtime federation and additional relying-party registrations | -## Passwordless-first (passkeys) +## Passwordless browser and enrollment flows -`realm-cwl.json` defines an authentication flow **`browser-passwordless`** with -`auth-username-form` → `webauthn-authenticator-passwordless` and **no password -authenticator**, and binds it as the realm `browserFlow`. Combined with -`resetPasswordAllowed:false`, `registrationAllowed:false`, and a default -`webauthn-register-passwordless` required action, ecosystem-local accounts -authenticate with a **passkey (FIDO2/WebAuthn)**, never a password. See -[`../../docs/passwordless-policy.md`](../../docs/passwordless-policy.md). +The bound `browser-passwordless` flow accepts an existing session, a federated +identity, or username followed by `webauthn-authenticator-passwordless`. It has +**no password authenticator**. -## What is committed vs. patched from KV +First-party products create password-free accounts through +`POST /registration/accounts`. The account-unification service then invokes +Keycloak's `execute-actions-email` Admin REST operation with `VERIFY_EMAIL` and +`webauthn-register-passwordless`. The resulting bounded link verifies control of +the address and enrolls the first passkey before normal login. A failed email +request rolls the new account back. -Committed (non-secret shape): realm, flows, client template, IdP + LDAP -*structure*, mappers. Patched from KV at bootstrap (never committed): ADFS -metadata URL, LDAP connection URL / bind DN / bind credential, and every client -secret. Placeholders read `__set_from_kv__`. +A deployment that enables registration must configure Keycloak SMTP and set the +following account-unification KV entries: -## Apply +- `registration_api_token` +- `registration_client_id` +- `registration_redirect_uri` +- `registration_action_lifespan_seconds` + +Without the registration token the endpoint is unavailable rather than open. +See [`../../docs/passwordless-policy.md`](../../docs/passwordless-policy.md). + +## Portable realm versus deployment data + +The committed realm contains no employer ADFS, LDAP/AD source, or other external +federation. Those objects are customer/deployment data and are managed through +`/federation/identity-providers`. Desired state is stored in the KV/DB backend +and can be reapplied after a realm rebuild with +`POST /federation/identity-providers:apply`. + +This separation also avoids Keycloak 26 import failures from placeholder SAML +URLs or invalid placeholder LDAP distinguished names. + +## Keycloak 26 import rules + +`scripts/validate_realm.py` enforces these fail-closed rules: + +- no `$`-prefixed annotation keys; +- no committed external federation or user-storage provider; +- no password authenticator in any subflow reachable from `browserFlow`; +- `webauthn-register-passwordless` remains enabled; +- `basic`, `profile`, and `email` scopes exist, with `basic` providing `sub`; +- public `naruon-web` requires PKCE S256 and an access-token lifespan no greater + than 900 seconds; +- committed client secrets are placeholders only. + +## RP clients + +`ecosystem-rp-template` is a confidential PKCE S256 blueprint. It uses the +reserved `rp.example.invalid` host so no product-specific deployment value is +silently inherited. Clones must replace redirect/origin values, client ID, +secret, and audience mapper together. + +`naruon-web` is the first concrete public PKCE client. It carries the audience +and `role`/`org`/`workspace` claims required by the current Naruon session +contract. Its access tokens last 300 seconds; the longer SSO session is serviced +through normal token refresh/reissue rather than a twelve-hour bearer token. + +## Bootstrap ```bash -# 1. Keycloak imports realm-cwl.json automatically on first start -# (docker-compose mounts it at /opt/keycloak/data/import). +# Keycloak imports the realm at first start. docker compose up -d -# 2. Once Keycloak is READY, patch secrets from KV: +# Once Keycloak is ready, converge the service client and its scoped roles. KC_SERVER=http://localhost:8080 deploy/keycloak/kcadm-bootstrap.sh ``` -## Federation & client registration templates - -Additional Admin-API request bodies for registering more RPs / IdPs live in -[`../templates/`](../templates/) (Keycloak client / SAML IdP / LDAP component -representations). +The bootstrap obtains credentials from the platform `kv` helper, keeps kcadm +session material inside a private temporary directory, never places reusable +secrets in process arguments, validates every resolved identifier, and +reconciles the protocol mapper without creating duplicates. diff --git a/deploy/keycloak/kcadm-bootstrap.sh b/deploy/keycloak/kcadm-bootstrap.sh index c7eaae4..347eb1e 100755 --- a/deploy/keycloak/kcadm-bootstrap.sh +++ b/deploy/keycloak/kcadm-bootstrap.sh @@ -4,64 +4,129 @@ # The realm SHAPE lives in realm-cwl.json and is imported at container start. # This script patches the pieces that must NOT be committed (secrets, env URLs) # by reading them from the KV store and applying them with Keycloak's admin CLI -# (`kcadm.sh`, shipped in the Keycloak image, Apache-2.0). Run it once after the -# realm is imported and Keycloak is READY. +# (`kcadm.sh`, shipped in the Keycloak image, Apache-2.0). Run it after the realm +# is imported and Keycloak is READY; every operation is safe to repeat. # -# deploy/keycloak/kcadm-bootstrap.sh -# -# Requires: kcadm.sh on PATH (or run inside the keycloak container), and a `kv` -# helper that reads your platform secret manager. Nothing here echoes secrets. +# Requires: kcadm.sh on PATH (or run inside the Keycloak container), and a `kv` +# helper that reads the platform secret manager. Nothing here echoes secrets. set -euo pipefail +umask 077 REALM="${KC_REALM:-cwl}" KC_SERVER="${KC_SERVER:-http://localhost:8080}" -# Bootstrap transport only: the admin credentials come from KV, used once to -# obtain an admin session, then discarded. ADMIN_USER="$(kv get secret/idp/bootstrap-admin-username)" ADMIN_PASS="$(kv get secret/idp/bootstrap-admin-password)" -kcadm.sh config credentials \ +# Use Keycloak's documented sensitive-option environment variable rather than +# placing the reusable password in process arguments. Scope the Admin CLI HOME +# to one private directory so its access and refresh tokens are destroyed at +# process exit while the platform `kv` helper retains its normal HOME. +_kcadm_home="$(mktemp -d)" +cleanup() { + rm -rf "${_kcadm_home}" + unset ADMIN_PASS SERVICE_CLIENT_SECRET +} +trap cleanup EXIT +kcadm() { + HOME="${_kcadm_home}" kcadm.sh "$@" +} +require_nonempty() { + local value_name="$1" + local value="$2" + if [[ -z "${value}" ]]; then + echo "bootstrap failed: ${value_name} was not resolved" >&2 + exit 1 + fi +} + +require_nonempty "bootstrap admin username" "${ADMIN_USER}" +require_nonempty "bootstrap admin password" "${ADMIN_PASS}" +KC_CLI_PASSWORD="${ADMIN_PASS}" kcadm config credentials \ --server "${KC_SERVER}" --realm master \ - --user "${ADMIN_USER}" --password "${ADMIN_PASS}" + --user "${ADMIN_USER}" +unset ADMIN_PASS -echo "==> patching employer-adfs SAML metadata URL from KV" -ADFS_METADATA_URL="$(kv get config/idp/employer-adfs-metadata-url)" -IDP_ID="$(kcadm.sh get "identity-provider/instances/employer-adfs" -r "${REALM}" --fields internalId --format csv --noquotes)" -kcadm.sh update "identity-provider/instances/employer-adfs" -r "${REALM}" \ - -s "config.metadataDescriptorUrl=${ADFS_METADATA_URL}" \ - -s "config.singleSignOnServiceUrl=${ADFS_METADATA_URL}" \ - -s "config.useMetadataDescriptorUrl=true" +# External federation is deliberately not part of realm bootstrap. Employer +# ADFS, LDAP-fronting brokers, and optional OIDC providers are runtime desired +# state managed by /federation/identity-providers and persisted in the KV store. -echo "==> patching corp-ldap bind credential + connection from KV" -LDAP_COMPONENT_ID="$(kcadm.sh get components -r "${REALM}" \ - --query 'name=corp-ldap' --fields id --format csv --noquotes | head -n1)" -kcadm.sh update "components/${LDAP_COMPONENT_ID}" -r "${REALM}" \ - -s "config.connectionUrl=[\"$(kv get config/idp/ldap-connection-url)\"]" \ - -s "config.usersDn=[\"$(kv get config/idp/ldap-users-dn)\"]" \ - -s "config.bindDn=[\"$(kv get secret/idp/ldap-bind-dn)\"]" \ - -s "config.bindCredential=[\"$(kv get secret/idp/ldap-bind-password)\"]" +echo "==> converging account-unification-svc client secret from KV" +SVC_CLIENT_UUID="$(kcadm get clients -r "${REALM}" \ + --query 'clientId=account-unification-svc' \ + --fields id --format csv --noquotes | head -n1)" +require_nonempty "account-unification service client id" "${SVC_CLIENT_UUID}" -echo "==> patching account-unification-svc client secret from KV" -SVC_CLIENT_UUID="$(kcadm.sh get clients -r "${REALM}" \ - --query 'clientId=account-unification-svc' --fields id --format csv --noquotes | head -n1)" -kcadm.sh update "clients/${SVC_CLIENT_UUID}" -r "${REALM}" \ - -s "secret=$(kv get secret/idp/account-unification-client-secret)" +# Write the secret to a 0600 file through stdin. Neither the reusable secret nor +# the JSON representation appears in a child-process argument or command log. +SERVICE_CLIENT_SECRET="$(kv get secret/idp/account-unification-client-secret)" +require_nonempty "account-unification service client secret" \ + "${SERVICE_CLIENT_SECRET}" +SERVICE_SECRET_JSON="${_kcadm_home}/service-client-secret.json" +printf '%s' "${SERVICE_CLIENT_SECRET}" \ + | python3 -c 'import json,sys; json.dump({"secret": sys.stdin.read()}, sys.stdout)' \ + > "${SERVICE_SECRET_JSON}" +unset SERVICE_CLIENT_SECRET +kcadm update "clients/${SVC_CLIENT_UUID}" -r "${REALM}" \ + -f "${SERVICE_SECRET_JSON}" -echo "==> granting realm-management view-users + manage-users to the service account" -SVC_SA_USER_ID="$(kcadm.sh get "clients/${SVC_CLIENT_UUID}/service-account-user" \ - -r "${REALM}" --fields id --format csv --noquotes)" -REALM_MGMT_UUID="$(kcadm.sh get clients -r "${REALM}" \ - --query 'clientId=realm-management' --fields id --format csv --noquotes | head -n1)" -kcadm.sh add-roles -r "${REALM}" \ +echo "==> granting realm-management roles to the service account" +SVC_SA_USER_ID="$(kcadm get \ + "clients/${SVC_CLIENT_UUID}/service-account-user" -r "${REALM}" \ + --fields id --format csv --noquotes)" +REALM_MGMT_UUID="$(kcadm get clients -r "${REALM}" \ + --query 'clientId=realm-management' \ + --fields id --format csv --noquotes | head -n1)" +require_nonempty "service-account user id" "${SVC_SA_USER_ID}" +require_nonempty "realm-management client id" "${REALM_MGMT_UUID}" + +kcadm add-roles -r "${REALM}" \ --uid "${SVC_SA_USER_ID}" \ --cclientid realm-management \ - --rolename view-users --rolename manage-users + --rolename view-users --rolename manage-users \ + --rolename manage-identity-providers + +echo "==> scoping the granted roles into the service-account access token" +# fullScopeAllowed is false. The roles therefore need both scope mappings and a +# client-role protocol mapper before they appear in resource_access. +REALM_MGMT_ROLE_JSON="$(kcadm get \ + "clients/${REALM_MGMT_UUID}/roles" -r "${REALM}" --fields id,name \ + | python3 -c 'import json,sys; names={"view-users","manage-users","manage-identity-providers"}; print(json.dumps([role for role in json.load(sys.stdin) if role.get("name") in names]))')" +ROLE_COUNT="$(printf '%s' "${REALM_MGMT_ROLE_JSON}" \ + | python3 -c 'import json,sys; print(len(json.load(sys.stdin)))')" +if [[ "${ROLE_COUNT}" -ne 3 ]]; then + echo "bootstrap failed: required realm-management roles were not resolved" >&2 + exit 1 +fi +kcadm create \ + "clients/${SVC_CLIENT_UUID}/scope-mappings/clients/${REALM_MGMT_UUID}" \ + -r "${REALM}" -b "${REALM_MGMT_ROLE_JSON}" -echo "==> mirroring the service client secret into KV for the admin service" -# The account-unification service reads keycloak_client_secret from KV; keep it -# in sync so both sides use the same credential. -kv put secret/idp/account-unification-client-secret \ - "$(kcadm.sh get "clients/${SVC_CLIENT_UUID}/client-secret" -r "${REALM}" \ - --fields value --format csv --noquotes)" +# Reconcile the mapper by name. Older non-idempotent bootstrap runs may have +# produced duplicates; update the first representation and remove the rest. +MAPPER_NAME="realm-management roles" +MAPPER_PAYLOAD='{"name":"realm-management roles","protocol":"openid-connect","protocolMapper":"oidc-usermodel-client-role-mapper","config":{"usermodel.clientRoleMapping.clientId":"realm-management","claim.name":"resource_access.realm-management.roles","multivalued":"true","jsonType.label":"String","access.token.claim":"true","id.token.claim":"false","userinfo.token.claim":"false"}}' +MAPPER_IDS="$(kcadm get \ + "clients/${SVC_CLIENT_UUID}/protocol-mappers/models" -r "${REALM}" \ + | python3 -c 'import json,sys; name=sys.argv[1]; print("\n".join(mapper["id"] for mapper in json.load(sys.stdin) if mapper.get("name") == name and mapper.get("id")))' \ + "${MAPPER_NAME}")" +MAPPER_ID="$(printf '%s\n' "${MAPPER_IDS}" | head -n1)" +if [[ -n "${MAPPER_ID}" ]]; then + kcadm update \ + "clients/${SVC_CLIENT_UUID}/protocol-mappers/models/${MAPPER_ID}" \ + -r "${REALM}" -b "${MAPPER_PAYLOAD}" +else + kcadm create "clients/${SVC_CLIENT_UUID}/protocol-mappers/models" \ + -r "${REALM}" -b "${MAPPER_PAYLOAD}" +fi +if [[ -n "${MAPPER_IDS}" ]]; then + printf '%s\n' "${MAPPER_IDS}" | tail -n +2 \ + | while IFS= read -r duplicate_mapper_id; do + if [[ -n "${duplicate_mapper_id}" ]]; then + kcadm delete \ + "clients/${SVC_CLIENT_UUID}/protocol-mappers/models/"\ +"${duplicate_mapper_id}" -r "${REALM}" + fi + done +fi echo "OK: kcadm bootstrap complete for realm '${REALM}'." diff --git a/deploy/keycloak/realm-cwl.json b/deploy/keycloak/realm-cwl.json index fa5a703..8d2018f 100644 --- a/deploy/keycloak/realm-cwl.json +++ b/deploy/keycloak/realm-cwl.json @@ -1,49 +1,21 @@ { - "$comment": [ - "cwl-idp realm config-as-code, imported by Keycloak at start via", - "`start --import-realm` (mounted at /opt/keycloak/data/import).", - "", - "It encodes the ecosystem policy declaratively:", - " - passwordless-first: a passkey-first browser flow (auth-username-form +", - " webauthn-authenticator-passwordless), NO password authenticator; the", - " realm's browserFlow is set to it and password registration/reset are off.", - " - an OIDC/OAuth2.1 confidential client TEMPLATE for ecosystem RPs (PKCE,", - " authorization-code + refresh, no implicit grant).", - " - the employer ADFS registered as an INBOUND SAML identity provider with", - " trustEmail=true so first-login auto-links by VERIFIED email + JIT.", - " - an LDAP/AD user-federation source (bindCredential is a placeholder that", - " the kcadm bootstrap patches from KV -- never committed).", - " - a confidential service-account client for the account-unification +", - " SCIM shim service (holds realm-management view-users/manage-users).", - "", - "SCIM inbound provisioning is served by the account-unification service's", - "SCIM v2 shim (services/account_unification/app/scim.py), which provisions", - "into this realm via the Admin REST API -- Keycloak's native SCIM is", - "experimental and the mature plugin is commercial, so we ship a permissive", - "Apache-2.0 shim instead.", - "", - "Secrets are NOT stored here. Placeholder values marked __set_from_kv__ are", - "patched post-import by deploy/keycloak/kcadm-bootstrap.sh from the KV store." - ], - "realm": "cwl", "displayName": "ContextualWisdom IdP", "enabled": true, "sslRequired": "external", - "registrationAllowed": false, - "registrationEmailAsUsername": false, + "registrationEmailAsUsername": true, "resetPasswordAllowed": false, "rememberMe": false, - "verifyEmail": true, + "verifyEmail": false, "loginWithEmailAllowed": true, "duplicateEmailsAllowed": false, "editUsernameAllowed": false, - - "$password_policy_note": "No passwordPolicy -> ecosystem-local accounts never set a password; the passkey-first flow below carries authentication.", - "webAuthnPolicyPasswordlessRpEntityName": "ContextualWisdom IdP", - "webAuthnPolicyPasswordlessSignatureAlgorithms": ["ES256", "RS256"], + "webAuthnPolicyPasswordlessSignatureAlgorithms": [ + "ES256", + "RS256" + ], "webAuthnPolicyPasswordlessRpId": "", "webAuthnPolicyPasswordlessAttestationConveyancePreference": "not specified", "webAuthnPolicyPasswordlessAuthenticatorAttachment": "not specified", @@ -51,14 +23,11 @@ "webAuthnPolicyPasswordlessUserVerificationRequirement": "required", "webAuthnPolicyPasswordlessCreateTimeout": 0, "webAuthnPolicyPasswordlessAvoidSameAuthenticatorRegister": false, - "browserFlow": "browser-passwordless", - "defaultSignatureAlgorithm": "RS256", "accessTokenLifespan": 300, "ssoSessionIdleTimeout": 1800, - "ssoSessionMaxLifespan": 36000, - + "ssoSessionMaxLifespan": 43200, "requiredActions": [ { "alias": "webauthn-register-passwordless", @@ -79,11 +48,10 @@ "config": {} } ], - "authenticationFlows": [ { "alias": "browser-passwordless", - "description": "Passkey-first passwordless browser flow (no password authenticator).", + "description": "Passkey-only browser flow with no password authenticator.", "providerId": "basic-flow", "topLevel": true, "builtIn": false, @@ -116,7 +84,7 @@ }, { "alias": "browser-passwordless-forms", - "description": "Username identification then a passwordless passkey (WebAuthn) assertion.", + "description": "Username identification followed only by a passwordless WebAuthn assertion.", "providerId": "basic-flow", "topLevel": false, "builtIn": false, @@ -129,91 +97,160 @@ "autheticatorFlow": false, "userSetupAllowed": false }, + { + "authenticatorFlow": true, + "requirement": "REQUIRED", + "priority": 20, + "flowAlias": "browser-passwordless-credentials", + "autheticatorFlow": true, + "userSetupAllowed": false + } + ] + }, + { + "alias": "browser-passwordless-credentials", + "description": "Passwordless WebAuthn assertion; enrollment occurs through a one-time action-email link.", + "providerId": "basic-flow", + "topLevel": false, + "builtIn": false, + "authenticationExecutions": [ { "authenticator": "webauthn-authenticator-passwordless", "authenticatorFlow": false, "requirement": "REQUIRED", - "priority": 20, + "priority": 10, "autheticatorFlow": false, "userSetupAllowed": false } ] } ], - - "identityProviders": [ + "clientScopes": [ { - "alias": "employer-adfs", - "displayName": "Employer ADFS (hssmartdev)", - "providerId": "saml", - "enabled": true, - "updateProfileFirstLoginMode": "on", - "trustEmail": true, - "storeToken": false, - "addReadTokenRoleOnCreate": false, - "authenticateByDefault": false, - "linkOnly": false, - "$firstBrokerLoginFlowAlias": "first broker login (default) auto-links by verified email; trustEmail=true makes the ADFS-asserted email an eligible link anchor.", - "config": { - "$metadata_note": "Prefer metadataUrl so ADFS cert rollover is picked up automatically; typically https://sts.hssmartdev.com/FederationMetadata/2007-06/FederationMetadata.xml -- patched from KV.", - "entityId": "https://idp.example/realms/cwl", - "idpEntityId": "http://sts.hssmartdev.com/adfs/services/trust", - "singleSignOnServiceUrl": "__set_from_kv__", - "metadataDescriptorUrl": "__set_from_kv__", - "useMetadataDescriptorUrl": "true", - "nameIDPolicyFormat": "urn:oasis:names:tc:SAML:1.1:nameid-format:emailAddress", - "principalType": "SUBJECT", - "postBindingResponse": "true", - "postBindingAuthnRequest": "true", - "wantAuthnRequestsSigned": "true", - "wantAssertionsSigned": "true", - "validateSignature": "true", - "syncMode": "FORCE" - } + "name": "basic", + "description": "OpenID Connect built-in scope: sub and auth_time claims", + "protocol": "openid-connect", + "attributes": { + "include.in.token.scope": "false", + "display.on.consent.screen": "false" + }, + "protocolMappers": [ + { + "name": "Subject (sub)", + "protocol": "openid-connect", + "protocolMapper": "oidc-sub-mapper", + "consentRequired": false, + "config": { + "access.token.claim": "true", + "id.token.claim": "true", + "introspection.token.claim": "true", + "lightweight.claim": "false" + } + }, + { + "name": "auth_time", + "protocol": "openid-connect", + "protocolMapper": "oidc-usersessionmodel-note-mapper", + "consentRequired": false, + "config": { + "user.session.note": "AUTH_TIME", + "claim.name": "auth_time", + "jsonType.label": "long", + "access.token.claim": "true", + "id.token.claim": "true", + "introspection.token.claim": "true" + } + } + ] + }, + { + "name": "profile", + "description": "OpenID Connect built-in scope: profile", + "protocol": "openid-connect", + "attributes": { + "include.in.token.scope": "true", + "display.on.consent.screen": "true" + }, + "protocolMappers": [ + { + "name": "username", + "protocol": "openid-connect", + "protocolMapper": "oidc-usermodel-attribute-mapper", + "consentRequired": false, + "config": { + "user.attribute": "username", + "claim.name": "preferred_username", + "jsonType.label": "String", + "access.token.claim": "true", + "id.token.claim": "true", + "userinfo.token.claim": "true", + "introspection.token.claim": "true" + } + }, + { + "name": "full name", + "protocol": "openid-connect", + "protocolMapper": "oidc-full-name-mapper", + "consentRequired": false, + "config": { + "access.token.claim": "true", + "id.token.claim": "true", + "userinfo.token.claim": "true", + "introspection.token.claim": "true" + } + } + ] + }, + { + "name": "email", + "description": "OpenID Connect built-in scope: email", + "protocol": "openid-connect", + "attributes": { + "include.in.token.scope": "true", + "display.on.consent.screen": "true" + }, + "protocolMappers": [ + { + "name": "email", + "protocol": "openid-connect", + "protocolMapper": "oidc-usermodel-attribute-mapper", + "consentRequired": false, + "config": { + "user.attribute": "email", + "claim.name": "email", + "jsonType.label": "String", + "access.token.claim": "true", + "id.token.claim": "true", + "userinfo.token.claim": "true", + "introspection.token.claim": "true" + } + }, + { + "name": "email verified", + "protocol": "openid-connect", + "protocolMapper": "oidc-usermodel-property-mapper", + "consentRequired": false, + "config": { + "user.attribute": "emailVerified", + "claim.name": "email_verified", + "jsonType.label": "boolean", + "access.token.claim": "true", + "id.token.claim": "true", + "userinfo.token.claim": "true", + "introspection.token.claim": "true" + } + } + ] } ], - - "components": { - "org.keycloak.storage.UserStorageProvider": [ - { - "name": "corp-ldap", - "providerId": "ldap", - "config": { - "$note": ["bindCredential is a placeholder patched from KV by kcadm-bootstrap.sh."], - "enabled": ["true"], - "priority": ["1"], - "editMode": ["READ_ONLY"], - "importEnabled": ["true"], - "syncRegistrations": ["false"], - "vendor": ["ad"], - "connectionUrl": ["__set_from_kv__"], - "usersDn": ["__set_from_kv__"], - "bindDn": ["__set_from_kv__"], - "bindCredential": ["__set_from_kv__"], - "usernameLDAPAttribute": ["sAMAccountName"], - "rdnLDAPAttribute": ["cn"], - "uuidLDAPAttribute": ["objectGUID"], - "userObjectClasses": ["person, organizationalPerson, user"], - "searchScope": ["2"], - "useTruststoreSpi": ["ldapsOnly"], - "connectionPooling": ["true"], - "trustEmail": ["true"] - } - } - ] - }, - - "clientScopes": [], - + "defaultDefaultClientScopes": [ + "basic", + "profile", + "email" + ], + "defaultOptionalClientScopes": [], "clients": [ { - "$comment": [ - "OIDC / OAuth 2.1 confidential client TEMPLATE for ecosystem RPs.", - "Clone per RP (naruon, pg-erd-cloud, semantic-data-portal, clearfolio,", - "contextual-orchestrator, newsdom-api) and set redirectUris + secret.", - "OAuth 2.1 posture: authorization-code + PKCE, refresh tokens, NO", - "implicit/hybrid, exact HTTPS redirect URIs. Secret patched from KV." - ], "clientId": "ecosystem-rp-template", "name": "Ecosystem RP template", "enabled": true, @@ -224,21 +261,126 @@ "directAccessGrantsEnabled": false, "serviceAccountsEnabled": false, "secret": "__set_from_kv__", - "redirectUris": ["https://naruon.example/auth/callback"], - "webOrigins": ["+"], + "redirectUris": [ + "https://rp.example.invalid/auth/callback" + ], + "webOrigins": [ + "+" + ], + "defaultClientScopes": [ + "basic", + "profile", + "email" + ], + "optionalClientScopes": [], "attributes": { "pkce.code.challenge.method": "S256", - "post.logout.redirect.uris": "https://naruon.example/", + "post.logout.redirect.uris": "https://rp.example.invalid/", "access.token.lifespan": "300" }, + "protocolMappers": [ + { + "name": "audience", + "protocol": "openid-connect", + "protocolMapper": "oidc-audience-mapper", + "consentRequired": false, + "config": { + "included.client.audience": "ecosystem-rp-template", + "access.token.claim": "true", + "id.token.claim": "false", + "introspection.token.claim": "true" + } + } + ], "fullScopeAllowed": false }, { - "$comment": [ - "Confidential service-account client used by the account-unification", - "service AND the SCIM shim to call the Admin REST API. It holds the", - "realm-management roles view-users + manage-users. Secret from KV." + "clientId": "naruon-web", + "name": "Naruon web client", + "enabled": true, + "protocol": "openid-connect", + "publicClient": true, + "standardFlowEnabled": true, + "implicitFlowEnabled": false, + "directAccessGrantsEnabled": false, + "serviceAccountsEnabled": false, + "redirectUris": [ + "https://naruon.example/auth/callback", + "https://naruon.example/auth/passkey-complete" ], + "webOrigins": [ + "https://naruon.example" + ], + "defaultClientScopes": [ + "basic", + "profile", + "email" + ], + "optionalClientScopes": [], + "attributes": { + "pkce.code.challenge.method": "S256", + "post.logout.redirect.uris": "https://naruon.example/", + "access.token.lifespan": "300" + }, + "protocolMappers": [ + { + "name": "audience", + "protocol": "openid-connect", + "protocolMapper": "oidc-audience-mapper", + "consentRequired": false, + "config": { + "included.client.audience": "naruon-web", + "access.token.claim": "true", + "id.token.claim": "false", + "introspection.token.claim": "true" + } + }, + { + "name": "naruon-role", + "protocol": "openid-connect", + "protocolMapper": "oidc-hardcoded-claim-mapper", + "consentRequired": false, + "config": { + "claim.name": "role", + "claim.value": "member", + "jsonType.label": "String", + "access.token.claim": "true", + "id.token.claim": "true", + "introspection.token.claim": "true" + } + }, + { + "name": "naruon-org", + "protocol": "openid-connect", + "protocolMapper": "oidc-hardcoded-claim-mapper", + "consentRequired": false, + "config": { + "claim.name": "org", + "claim.value": "org-cwl", + "jsonType.label": "String", + "access.token.claim": "true", + "id.token.claim": "true", + "introspection.token.claim": "true" + } + }, + { + "name": "naruon-workspace", + "protocol": "openid-connect", + "protocolMapper": "oidc-hardcoded-claim-mapper", + "consentRequired": false, + "config": { + "claim.name": "workspace", + "claim.value": "workspace-org-cwl", + "jsonType.label": "String", + "access.token.claim": "true", + "id.token.claim": "true", + "introspection.token.claim": "true" + } + } + ], + "fullScopeAllowed": false + }, + { "clientId": "account-unification-svc", "name": "Account unification service", "enabled": true, @@ -251,7 +393,5 @@ "secret": "__set_from_kv__", "fullScopeAllowed": false } - ], - - "$roles_note": "realm-management client roles view-users/manage-users are granted to the account-unification-svc service account by kcadm-bootstrap.sh (composite service-account role assignment is applied post-import)." + ] } diff --git a/docker-compose.yml b/docker-compose.yml index c8217c2..dfcbe84 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -50,32 +50,24 @@ services: image: quay.io/keycloak/keycloak:26.3.2@sha256:98fab020a3a490aba0978f237e2a06cd0ea42bf149c6cf10f11c0aaf27728ff2 container_name: cwl_idp_engine restart: unless-stopped - # `start` (production mode) with a runtime build; --import-realm loads the - # realm-as-code on first boot. TLS terminates at the WAF edge, so HTTP is - # enabled and hostname-strict relaxed for the internal listener. command: > start --import-realm environment: - # ---- Database wiring (values sourced from KV at deploy time) ---- KC_DB: postgres KC_DB_URL: jdbc:postgresql://idp_database:5432/${IDP_DB_NAME:-keycloak} KC_DB_USERNAME: ${IDP_DB_USER:-keycloak} KC_DB_PASSWORD: ${IDP_DB_PASSWORD} - # ---- Bootstrap admin. Created ONCE; retire after switching to passkey. - # Bootstrap transport only: sourced from KV at deploy time. KC_BOOTSTRAP_ADMIN_USERNAME: ${IDP_BOOTSTRAP_ADMIN_USERNAME:-idp-admin} KC_BOOTSTRAP_ADMIN_PASSWORD: ${IDP_BOOTSTRAP_ADMIN_PASSWORD} - # ---- Health + metrics management endpoints on port 9000 ---- KC_HEALTH_ENABLED: "true" KC_METRICS_ENABLED: "true" - # ---- HTTP/hostname. TLS terminates at the WAF edge; enable http here. ---- KC_HTTP_ENABLED: "true" KC_HOSTNAME: ${IDP_EXTERNAL_HOSTNAME:-http://localhost:8080} KC_HOSTNAME_STRICT: "false" KC_PROXY_HEADERS: xforwarded + KC_CACHE: ${IDP_CACHE_MODE:-local} volumes: - # Realm config-as-code imported on first start. - ./deploy/keycloak/realm-cwl.json:/opt/keycloak/data/import/realm-cwl.json:ro ports: - "${IDP_EXTERNAL_PORT:-8080}:8080" @@ -83,8 +75,6 @@ services: idp_database: condition: service_healthy healthcheck: - # Keycloak's distroless image ships bash; probe the management health - # endpoint over a bash /dev/tcp socket (no curl in the image). test: - CMD-SHELL - >- @@ -117,6 +107,9 @@ services: CWL_IDP_BOOTSTRAP: /bootstrap/bootstrap.yaml volumes: - ./deploy/bootstrap:/bootstrap:ro + # Audit events and the user-operation lock sidecar survive container + # replacement. The image runs as a non-root user that owns this path. + - account_unification_data:/var/lib/account-unification ports: - "${UNIFICATION_PORT:-8099}:8099" depends_on: @@ -134,11 +127,10 @@ services: volumes: idp_database_data: + account_unification_data: networks: - # Internal: DB + engine + admin service. Never routed to the public edge. idp_internal_network: driver: bridge - # Edge: only the engine (OIDC) and admin API are reachable from the WAF. idp_edge_network: driver: bridge diff --git a/docs/merge-unification-flow.md b/docs/merge-unification-flow.md index a776a27..35a88f9 100644 --- a/docs/merge-unification-flow.md +++ b/docs/merge-unification-flow.md @@ -31,6 +31,7 @@ only an unverified email, the merge is refused with `422 Unverified email`. ``` merge(survivor S, duplicate D, actor A): reject if S == D -> 400 SameUser + acquire shared user-operation locks for S and D load S, D (must exist) -> 404 UserNotFound reject if S or D disabled -> 409 InactiveAccount decision = decide_match(S, D, explicit) @@ -55,6 +56,7 @@ merge(survivor S, duplicate D, actor A): disable D # D can never authenticate again audit "duplicate_tombstoned" audit "merge_completed" {moved_*, conflicts} + release shared user-operation locks return MergeResult{..., audit_id} ``` @@ -72,6 +74,25 @@ The duplicate is **not deleted**. Its Keycloak user attribute (`enabled: false`). This preserves forensic history and lets any stale reference resolve to the survivor. +### SCIM/merge serialization invariant + +`PUT /scim/v2/Users/{id}` performs a full Keycloak user-representation write and +can set `active: true`. Its tombstone check and replacement PUT therefore execute +inside the **same user-operation lock** used by the complete merge transaction. +A merge cannot create `merged_into_user_id` between those two Admin API calls, +and SCIM cannot wipe a newly-created tombstone or reactivate the duplicate. + +Standalone deployments use a dedicated SQLite sidecar lock database and hold a +`BEGIN IMMEDIATE` transaction for the complete critical section. This provides a +crash-safe mutex shared by every worker/process using the same database path; +process death closes the connection and releases the lock. The current backend +serializes all user mutations conservatively rather than risking a multi-user +deadlock. Lock acquisition waits up to 10 seconds, then returns retryable HTTP +`503` without performing a partial mutation. A clustered Postgres deployment +must provide the same `UserOperationLocks` contract (for example, ordered +advisory locks) and wire one shared instance into both the merge service and SCIM +router. + ## Audit Every step emits an immutable `account_merge_audit` event sharing one diff --git a/docs/passwordless-policy.md b/docs/passwordless-policy.md index ef43b9a..e33c2c3 100644 --- a/docs/passwordless-policy.md +++ b/docs/passwordless-policy.md @@ -3,62 +3,82 @@ ## Goal Eliminate passwords for **ecosystem-local accounts**. Every human either signs -in through a federated IdP (employer ADFS, corporate LDAP/AD, optional personal -OIDC) or with a **FIDO2 / passkey** registered on cwl-idp. The password -authenticator is removed from the browser flow so there is no local password to -phish, reuse, or leak. +in through a federated IdP or with a **FIDO2/passkey** registered on cwl-idp. +The bound browser flow contains no password authenticator, so there is no local +password to phish, reuse, reset, or leak. -## How it is enforced (as-code) +## How it is enforced as code -Set once at realm import from `deploy/keycloak/realm-cwl.json`: +`deploy/keycloak/realm-cwl.json` fixes the following invariants: | Setting | Value | Effect | | --- | --- | --- | -| `browserFlow` | `browser-passwordless` | Custom flow: username form → **WebAuthn passwordless**, no password authenticator | -| `authenticationFlows[browser-passwordless-forms]` | `auth-username-form` + `webauthn-authenticator-passwordless` | Passkey is the primary (and only) knowledge-free factor | -| `registrationAllowed` | `false` | No self-service signup | -| `resetPasswordAllowed` | `false` | No password-reset surface | -| `requiredActions[webauthn-register-passwordless]` | `defaultAction: true` | New users are prompted to enrol a passkey | -| `webAuthnPolicyPasswordless*` | RP name / ES256,RS256 / resident key / UV required | Passkey relying-party policy | - -Because the bound browser flow contains **no** `auth-password-form` / -`auth-username-password-form` authenticator, a local password is never accepted -at login even if one existed on the user. `scripts/validate_realm.py` asserts -this invariant in CI (fails if any password authenticator appears in the bound -browser flow, or if the passwordless WebAuthn authenticator is missing). - -The passkey relying-party name is `webAuthnPolicyPasswordlessRpEntityName`; the -RPID derives from the request host (behind the WAF, the public IdP host), so -`KC_HOSTNAME` must match the domain the browser sees. - -## The one exception: the bootstrap admin - -Keycloak requires an initial admin in the `master` realm. It is created **once** -from `KC_BOOTSTRAP_ADMIN_USERNAME` / `KC_BOOTSTRAP_ADMIN_PASSWORD` (bootstrap -transport from KV), then that admin registers a passkey and the password is -retired (operational runbook: switch to passkey-only, rotate/disable the -bootstrap password). No ecosystem-local account in the `cwl` realm has a -password. - -## Verified-email is the linking anchor (NIST SP 800-63C) - -Following NIST SP 800-63C on federated assertions, cwl-idp treats an email as an -identity-linking anchor **only when the asserting IdP marks it verified**. This -is enforced in two places: - -1. **Keycloak** federation config: `trustEmail: true` on the employer ADFS SAML - IdP and the LDAP/AD source lets the first-broker-login flow auto-link a new - external identity to an existing account on a matching **verified** email. -2. **account-unification service** (`app/matching.py`): the merge engine refuses - to treat an unverified-email coincidence as a match, and refuses any merge - whose only tie is an unverified email (`UnverifiedEmailMergeError`). - -The config key `allow_unverified_email_link` exists solely so an audit can prove -it is hard-defaulted to `false`. - -## Why passwordless here specifically - -- Most human logins arrive already authenticated by the employer ADFS or the - corporate directory — a local password would be a redundant, weaker factor. -- Passkeys are phishing-resistant and bind to the origin, which matters when a - single IdP fronts many ecosystem RPs. +| `browserFlow` | `browser-passwordless` | Cookie/federation or username followed by passwordless WebAuthn | +| `authenticationFlows[browser-passwordless-credentials]` | `webauthn-authenticator-passwordless` only | A password can never authenticate to the `cwl` realm | +| `registrationAllowed` | `false` | Signup is owned by first-party product backends, not an IdP-hosted form | +| `registrationEmailAsUsername` | `true` | The normalized email address is the account identity | +| `resetPasswordAllowed` | `false` | No password-reset surface exists | +| `requiredActions[webauthn-register-passwordless]` | enabled | Keycloak can execute the passkey enrollment action | +| `webAuthnPolicyPasswordless*` | resident key and user verification required | Passkeys are discoverable and user-verified | +| `naruon-web.attributes[access.token.lifespan]` | `300` seconds | Public-client bearer exposure is bounded independently of the longer SSO session | + +`scripts/validate_realm.py` follows every nested subflow reachable from +`browserFlow` and fails CI if it finds `auth-password-form`, +`auth-username-password-form`, or another password authenticator. It also caps +public `naruon-web` access tokens at 900 seconds. + +## Password-free headless registration + +The account-unification service's `POST /registration/accounts` endpoint accepts +only identity/profile data. It does **not** accept or create a password. + +After creating the disabled-password account, the service calls Keycloak's +Admin REST `execute-actions-email` operation with two required actions: + +1. `VERIFY_EMAIL` +2. `webauthn-register-passwordless` + +Keycloak sends one bounded link associated with the configured relying-party +client and HTTPS redirect URI. If Keycloak cannot accept the action-email +request, the service deletes the newly created account; if rollback also fails, +the API reports a distinct failure so an operator can reconcile it. + +Enabling this endpoint requires all of these KV entries: + +- `registration_api_token`, different from `operator_api_token` +- `registration_client_id` +- `registration_redirect_uri`, an absolute HTTPS URI without credentials or a fragment +- `registration_action_lifespan_seconds`, a positive integer no greater than 3600 + +The Keycloak realm must also have a working SMTP configuration. A deployment +without SMTP should omit `registration_api_token`; the endpoint then fails +closed with HTTP 503 before creating an account. + +Registration throttling is keyed by direct peer address. Operators terminating +traffic at a WAF or gateway must preserve trustworthy source isolation there; +the service deliberately does not trust arbitrary forwarded-address headers. + +## The one bootstrap exception + +Keycloak requires an initial administrator in the `master` realm. It is created +once from deployment secrets, registers a passkey, and then has its reusable +bootstrap credential rotated or disabled. This exception is outside the `cwl` +realm and is governed by the bootstrap runbook. + +## Verified email is the linking anchor + +An email address authorizes linking only when both accounts hold the same +verified address or an exact external `(provider, subject)` tie exists. The +account-unification service rejects an unverified-email coincidence even when a +caller supplies `explicit_link=true`. The configuration key +`allow_unverified_email_link` remains solely as audit evidence and startup +rejects any attempt to set it true. + +## Why this boundary matters + +- Federated employees already authenticate at their authoritative employer IdP. +- Passkeys are phishing-resistant and origin-bound. +- Registration proves control of the submitted address through the same + one-time action link that enrolls the passkey. +- A five-minute access token limits bearer-token exposure while a longer SSO + session can still support normal product use through refresh and reissue. diff --git a/docs/superpowers/plans/2026-08-03-keyverse-product-hardening.md b/docs/superpowers/plans/2026-08-03-keyverse-product-hardening.md new file mode 100644 index 0000000..7d458cd --- /dev/null +++ b/docs/superpowers/plans/2026-08-03-keyverse-product-hardening.md @@ -0,0 +1,170 @@ +# Keyverse Product Hardening 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:** Produce one protected, release-ready Keyverse identity-service change +that integrates passwordless registration, federation, SCIM serialization, +merge hardening, and durable deployment packaging. + +**Architecture:** Preserve the minimal merge/SCIM `AdminApi` contract and add a +separate product adapter for one-time passkey action email and federation. Apply +authentication and path validation at router boundaries, redact unknown +federation values by default, and separate storage locks from network +convergence. + +**Tech Stack:** Python 3.11+, FastAPI, Pydantic 2, httpx, SQLite, pytest, Ruff, +Interrogate, Keycloak Admin REST API, Docker Compose, Helm. + +## Global Constraints + +- Application docstring coverage must remain 100%. +- Production statement and branch coverage must remain 100%. +- Every database object name must contain at least two words and use snake_case. +- Runtime dependencies and GitHub Actions remain locked and pinned. +- The service must work standalone and as an importable MSA module. +- No federation or registration secret may appear in an HTTP response, log, or + process argument. +- The bound browser flow contains no password authenticator. + +--- + +### Task 1: Separate the product Keycloak contract + +**Files:** +- Create: `services/account_unification/app/product_keycloak_client.py` +- Test: `services/account_unification/tests/test_keycloak_client.py` + +**Interfaces:** +- Consumes: `AdminApi`, `HttpAdminApi`, `UserAccount` +- Produces: `ProductAdminApi`, `ProductHttpAdminApi` + +- [x] Add a protocol test that enumerates every declared public method. +- [x] Implement action-email enrollment, rollback deletion, and federation CRUD. +- [x] Add one-shot HTTP 401 retry coverage for GET and user creation. +- [x] Add unsafe-path tests that prove transport calls are not emitted. +- [x] Validate HTTPS redirect, client ID, action aliases, and link lifespan before + sending action email. + +### Task 2: Make registration password-free and failure-atomic + +**Files:** +- Modify: `services/account_unification/app/registration.py` +- Modify: `services/account_unification/app/config.py` +- Modify: `services/account_unification/tests/test_registration.py` +- Modify: `services/account_unification/tests/test_config.py` + +**Interfaces:** +- Consumes: `ProductAdminApi` +- Produces: `_initialize_account`, `RegistrationResult` + +- [x] Reject legacy password fields at the API boundary. +- [x] Create a Keycloak account without a password. +- [x] Send `VERIFY_EMAIL` and `webauthn-register-passwordless` in one bounded + action email. +- [x] Delete the new account when Keycloak rejects enrollment. +- [x] Preserve a distinct error when rollback itself fails. +- [x] Map exact Keycloak duplicate-user 409 responses to the product conflict. +- [x] Isolate abuse throttling by direct caller address. +- [x] Remove the credential janitor, background loop, and realm-wide endpoint. + +### Task 3: Redact and reconcile runtime federation safely + +**Files:** +- Modify: `services/account_unification/app/federation.py` +- Modify: `services/account_unification/tests/test_federation.py` + +**Interfaces:** +- Consumes: `KvStore`, `ProductAdminApi` +- Produces: `IdentityProviderView`, `IdentityProviderStatus` + +- [x] Prove storage and Keycloak receive complete desired state. +- [x] Redact every unknown provider configuration key by default. +- [x] Bound aliases, provider IDs, config entry counts, keys, and values. +- [x] Restrict aliases to an explicit ASCII slug alphabet. +- [x] Snapshot desired state under the storage lock and release it before network + calls. +- [x] Retain desired state and return `applied_to_keycloak=false` when + convergence fails. + +### Task 4: Harden protocol boundaries + +**Files:** +- Modify: `services/account_unification/app/path_security.py` +- Modify: `services/account_unification/app/healthcheck.py` +- Modify: `services/account_unification/app/main.py` +- Test: `services/account_unification/tests/test_path_security.py` +- Test: `services/account_unification/tests/test_healthcheck.py` + +- [x] Validate all decoded route parameters at router entry. +- [x] Return a root-level RFC 7644 body with `application/scim+json`. +- [x] Keep `/healthz` outside privileged dependencies. +- [x] Restrict health probes to HTTP(S) and HTTP(S) redirects. +- [x] Register the default urllib HTTP error handler so non-success responses + raise instead of returning a null response. + +### Task 5: Make standalone persistence thread-safe and durable + +**Files:** +- Modify: `services/account_unification/app/kv_store.py` +- Modify: `services/account_unification/app/audit.py` +- Modify: `docker-compose.yml` +- Modify: `helm/cwl-idp/values.yaml` +- Modify: `helm/cwl-idp/templates/account-unification.yaml` +- Test: `services/account_unification/tests/test_storage_concurrency.py` +- Test: `services/account_unification/tests/test_deployment_contracts.py` + +- [x] Add concurrent writer/reader tests using eight worker threads. +- [x] Add process-local re-entrant locks and cross-thread connections. +- [x] Configure WAL, normal synchronous mode, and a ten-second busy timeout. +- [x] Close connections under the same lock. +- [x] Mount persistent standalone and Kubernetes account-unification data. +- [x] Allow production Helm values to require an immutable image digest. + +### Task 6: Integrate lifecycle and mutation serialization + +**Files:** +- Modify: `services/account_unification/app/main.py` +- Modify: `services/account_unification/tests/test_lifecycle.py` +- Modify: `services/account_unification/tests/test_scim.py` +- Modify: `services/account_unification/tests/test_user_locks.py` + +- [x] Wire merge and SCIM replacement to the same production lock dependency. +- [x] Add the deterministic SCIM/merge race test using the production in-memory + manager. +- [x] Use a secure temporary sidecar for in-memory audit tests. +- [x] Remove test-only sidecars at shutdown without deleting persistent data. +- [x] Close API, audit, and config resources. + +### Task 7: Enforce Keycloak realm policy + +**Files:** +- Modify: `deploy/keycloak/realm-cwl.json` +- Modify: `scripts/validate_realm.py` +- Test: `services/account_unification/tests/test_realm_policy.py` + +- [x] Remove all password authenticators reachable from the bound browser flow. +- [x] Require the passwordless WebAuthn authenticator and enrollment action. +- [x] Set the public Naruon access-token lifetime to 300 seconds. +- [x] Reject public-client lifetimes above 900 seconds. +- [x] Keep the reusable RP template independent of the Naruon hostname. + +### Task 8: Protected verification and release preparation + +**Files:** +- Modify: `CHANGELOG.md` +- Modify: `docs/superpowers/specs/2026-08-03-keyverse-product-hardening-design.md` +- Modify: `docs/superpowers/plans/2026-08-03-keyverse-product-hardening.md` + +- [x] Record product gaps and architectural decisions. +- [ ] Run `uv sync --locked --extra dev`. +- [ ] Run `uv run ruff check app tests tools`. +- [ ] Run `uv run interrogate .` and require 100%. +- [ ] Run `uv run pytest -q` with 100% production statement/branch coverage. +- [ ] Require realm, Compose, Helm, CodeQL, Semgrep, security, and central + coverage checks on the exact current head. +- [ ] Resolve review threads only after the corresponding finding is addressed. +- [ ] Merge only after protected policy is satisfied. +- [ ] Bump service and lock versions together and publish the release tag. diff --git a/docs/superpowers/plans/2026-08-03-keyverse-review-remediation.md b/docs/superpowers/plans/2026-08-03-keyverse-review-remediation.md new file mode 100644 index 0000000..a4a2093 --- /dev/null +++ b/docs/superpowers/plans/2026-08-03-keyverse-review-remediation.md @@ -0,0 +1,167 @@ +# Keyverse Review Remediation 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:** Resolve every valid current-head review blocker without weakening +Keyverse's passwordless, audit, security, or modularity contracts. + +**Architecture:** Replace the bound-flow bootstrap password with Keycloak's +action-email passkey enrollment path; keep registration and operator privileges +separate; move network calls outside storage locks; return protocol-native +errors; and fail closed on unknown secrets, aliases, paths, and deployment +configuration. + +**Tech Stack:** Python 3.11+, FastAPI, Pydantic 2, httpx, SQLite, Keycloak Admin +REST API, pytest, Ruff, Interrogate, Helm, Docker Compose. + +## Global Constraints + +- The bound `browserFlow` must contain no password authenticator. +- Application and test function docstring coverage must remain 100%. +- Production statement and branch coverage must remain 100%. +- Every database object name must contain at least two words and use snake_case. +- No secret may appear in HTTP responses, logs, or process arguments. +- Required reviews and GitHub Checks must not be bypassed. + +--- + +### Task 1: Passwordless registration enrollment + +**Files:** +- Modify: `deploy/keycloak/realm-cwl.json` +- Modify: `scripts/validate_realm.py` +- Modify: `services/account_unification/app/registration.py` +- Modify: `services/account_unification/app/product_keycloak_client.py` +- Modify: `services/account_unification/app/config.py` +- Modify: `services/account_unification/app/main.py` +- Modify: `services/account_unification/tests/test_registration.py` +- Modify: `services/account_unification/tests/test_keycloak_client.py` +- Modify: `services/account_unification/tests/mock_product_keycloak.py` + +**Interfaces:** +- Produces: `ProductAdminApi.send_execute_actions_email(user_id, action_aliases, + client_id, redirect_uri, lifespan_seconds) -> None` +- Produces: registration configuration for client ID, redirect URI, and finite + positive action-link lifespan. + +- [x] Write regression tests proving the bound browser flow rejects + `auth-password-form`, public-client access tokens are capped, registration + creates no password, and passkey action-email failure rolls the account back. +- [x] Remove the password execution and validator exception; set `naruon-web` + access-token lifespan to 300 seconds and validate a 900-second maximum. +- [x] Replace `initial_password` with action-email enrollment and add the + Keycloak Admin REST adapter method. +- [x] Remove the credential janitor, its configuration, background task, and + privileged endpoint. +- [ ] Run focused registration, adapter, config, realm, and lifecycle tests. + +### Task 2: Registration abuse and race boundaries + +**Files:** +- Modify: `services/account_unification/app/registration.py` +- Modify: `services/account_unification/tests/test_registration.py` + +**Interfaces:** +- Produces: `reset_rate_limit_state() -> None` +- Produces: caller-keyed fixed-window registration limiting. + +- [x] Write regression tests proving one client cannot exhaust another client's + quota and Keycloak 409 maps to `email_already_registered`. +- [x] Store rate-limit windows per client address under one lock and expose a + test reset helper. +- [x] Catch only `httpx.HTTPStatusError` with status 409; re-raise every other + transport error. +- [ ] Run the focused registration suite. + +### Task 3: Federation reconciliation and secret safety + +**Files:** +- Modify: `services/account_unification/app/federation.py` +- Modify: `services/account_unification/tests/test_federation.py` + +**Interfaces:** +- Preserves: `IdentityProviderStatus` +- Produces: safe-key allowlist redaction in which unknown config keys are + redacted. + +- [x] Write regressions for non-ASCII aliases, unknown-key redaction, + persisted-but-unapplied status, and storage-lock-free network calls. +- [x] Snapshot stored registrations under the storage lock, then perform + Keycloak calls after releasing it. +- [x] Return `applied_to_keycloak=False` when desired state was stored but + convergence failed. +- [x] Validate aliases against explicit ASCII alphabets and redact every config + key not explicitly classified safe. +- [ ] Run the federation suite. + +### Task 4: Protocol and runtime correctness + +**Files:** +- Modify: `services/account_unification/app/healthcheck.py` +- Modify: `services/account_unification/app/path_security.py` +- Modify: `services/account_unification/app/main.py` +- Modify: `services/account_unification/tests/test_healthcheck.py` +- Modify: `services/account_unification/tests/test_path_security.py` +- Modify: `services/account_unification/tests/test_user_locks.py` +- Create: `services/account_unification/tests/test_lifecycle.py` + +**Interfaces:** +- Produces: `ScimPathValidationError` and + `scim_path_validation_exception_handler`. + +- [x] Write regressions for HTTP error handling, root-level SCIM error envelopes + with `application/scim+json`, and `:memory:` lock wiring. +- [x] Register `HTTPDefaultErrorHandler`, add the SCIM-specific exception + handler, and use an explicit temporary lock file for in-memory audit + configurations. +- [x] Add missing function docstrings and deterministic temporary-resource + cleanup. +- [ ] Run the focused health, path, and lifecycle suites. + +### Task 5: Deployment durability and review hygiene + +**Files:** +- Modify: `docker-compose.yml` +- Modify: `helm/cwl-idp/values.yaml` +- Modify: `helm/cwl-idp/templates/account-unification.yaml` +- Modify: `deploy/keycloak/README.md` +- Modify: `docs/passwordless-policy.md` +- Modify: `services/account_unification/tools/seed_config_store.py` +- Modify: `services/account_unification/tests/test_config.py` +- Modify: `services/account_unification/tests/test_federation.py` +- Modify: `services/account_unification/tests/test_kcadm_bootstrap.py` +- Modify: `services/account_unification/tests/test_storage_concurrency.py` +- Create: `services/account_unification/tests/test_deployment_contracts.py` + +**Interfaces:** +- Produces: persistent Compose/Helm audit volume and optional Helm digest + enforcement. + +- [x] Add contract tests for persistent audit storage, digest enforcement, + non-temporary seed defaults, and stable bootstrap markers. +- [x] Add the named volume, `requireDigest` render guard, project-local seed + path, fixture-safe tests, context-managed SQLite test resources, and truthful + documentation. +- [ ] Run focused tests and Compose/Helm rendering checks. + +### Task 6: Protected completion + +**Files:** +- Modify: `CHANGELOG.md` +- Modify: `docs/superpowers/plans/2026-08-03-keyverse-review-remediation.md` + +- [x] Resolve the prior review threads after implementing the corresponding + findings; all old threads are currently resolved/outdated. +- [ ] Run `uv sync --locked --extra dev`. +- [ ] Run `uv run ruff check app tests tools`. +- [ ] Run `uv run interrogate .` and require 100%. +- [ ] Run `uv run pytest -q` with 100% production statement and branch coverage. +- [ ] Run realm, Compose, Helm, CodeQL, Semgrep, security, and central coverage + checks on the exact head. +- [ ] Obtain independent approval on the exact head. +- [ ] Merge only after the repository's protected policy is satisfied. +- [ ] Re-list open PRs and continue until the queue is zero or an external + approval/runner blocker remains. diff --git a/docs/superpowers/specs/2026-08-03-keyverse-product-hardening-design.md b/docs/superpowers/specs/2026-08-03-keyverse-product-hardening-design.md new file mode 100644 index 0000000..7703c46 --- /dev/null +++ b/docs/superpowers/specs/2026-08-03-keyverse-product-hardening-design.md @@ -0,0 +1,136 @@ +# Keyverse Product Hardening Design + +## Objective + +Integrate Keycloak 26 compatibility, passwordless registration, runtime +federation, SCIM serialization, and account merge into one releasable identity +service without weakening authentication, auditability, deployment durability, +or supply-chain controls. + +## Product gaps addressed + +1. **Overlapping pull requests could not merge safely.** SCIM replacement, + account merge, registration, and federation modified the same service + boundaries independently. The integrated design keeps the merge/SCIM core + interface small and adds a separate product-facing Keycloak adapter. +2. **Registration contradicted the passkey-only claim.** The previous bootstrap + password was reachable from the bound browser flow. Registration now creates + no password and uses a bounded Keycloak action-email link for address + verification and passkey enrollment. +3. **Registration could leave orphaned accounts.** Failure to send the action + email deletes the new account; cleanup failure has its own stable error. +4. **Federation APIs could disclose provider credentials or stall concurrent + configuration reads.** Unknown values are redacted by default, storage locks + are released before network calls, and convergence status is explicit. +5. **SQLite objects were unsafe under threaded ASGI execution.** Configuration + and audit connections use re-entrant process locks, WAL mode, bounded busy + timeouts, and cross-thread connections. Cross-process user mutations remain + serialized by a dedicated SQLite lock sidecar. +6. **Decoded route values could reach path builders.** Privileged and SCIM + routers validate every decoded path parameter before endpoint dependencies. +7. **Runtime state disappeared on container replacement.** Compose and Helm now + mount deployment-owned storage for audit and lock databases. +8. **Mutable production images were easy to deploy accidentally.** The chart can + require an immutable account-unification digest and fail template rendering + when it is absent. + +## Architecture + +### Core identity engine + +`AdminApi`, `UnificationService`, SCIM translation, matching, audit events, and +`UserOperationLocks` remain the reusable MSA module. They do not depend on +product signup or federation configuration. + +### Product extension adapter + +`ProductAdminApi` extends the core contract only for product capabilities: +rollback deletion, one-time action-email enrollment, and identity-provider +CRUD. `ProductHttpAdminApi` subclasses the core HTTP adapter so both modules +share service-account authentication and model translation while keeping +product concerns separable. + +The adapter allows only known Keycloak Admin REST route shapes. Every dynamic +realm, user, client, credential, group, or provider value is validated as one +opaque segment before interpolation. An expired bearer token is refreshed and +retried exactly once for every transport method. + +### Password-free registration lifecycle + +A bearer token distinct from operator authority gates `/registration`. +Registration: + +1. verifies complete action-email configuration; +2. applies caller-keyed abuse throttling; +3. normalizes and validates identity/profile input; +4. checks for an existing email; +5. creates a password-free Keycloak account; +6. requests `VERIFY_EMAIL` and `webauthn-register-passwordless` through + `execute-actions-email` with an HTTPS redirect and bounded lifespan; +7. deletes the account if step 6 fails. + +The bound `browserFlow` contains no password authenticator. The public +`naruon-web` access token lasts 300 seconds; the SSO session can remain longer +because products obtain new access tokens through normal refresh/reissue. + +### Runtime federation + +The KV/DB store is the desired-state source of truth. `FederationService` +validates and persists the complete provider representation under the two-word +namespace `federation_identity_providers`. It snapshots state while holding the +storage lock, releases that lock before Keycloak network calls, and serializes +convergence separately. + +Operator views expose only explicitly allowlisted non-secret configuration +keys. Unknown keys are ``, so future Keycloak credential fields cannot +silently leak. A failed apply retains desired state and returns +`applied_to_keycloak=false`, allowing an operator or automation to retry +`identity-providers:apply` after recovery. + +### Mutation serialization + +Account merge and SCIM full replacement both acquire `UserOperationLocks`. +Standalone deployments use the two-word table `user_operation_lock_state` in a +dedicated SQLite sidecar and `BEGIN IMMEDIATE`. Persistent audit deployments +place that sidecar next to the audit database; in-memory tests receive a secure +temporary file that lifecycle cleanup removes. + +### Persistence and packaging + +The standalone Compose service mounts `account_unification_data` at +`/var/lib/account-unification`. The Helm chart creates a PVC by default and +mounts the same path. Production values set +`accountUnification.image.requireDigest=true`; chart rendering then fails unless +an immutable digest is supplied. + +### Error and security behavior + +- Operator and registration tokens are separate and compared in constant time. +- `/healthz` remains unauthenticated. +- The restricted stdlib health opener supports only HTTP(S), rejects redirects + to other schemes, and raises on non-success responses. +- Unsafe admin values return HTTP 400; unsafe SCIM values return a root-level + RFC 7644 body with `application/scim+json`. +- Provider secrets and unknown provider values never enter API representations. +- Registration failures return stable, non-internal identifiers. +- No database object uses a single-word or numeric-only name. + +## Verification + +The acceptance gate is: + +- locked dependency installation; +- Ruff linting; +- 100% application docstring coverage; +- 100% production statement and branch coverage; +- complete pytest suite, including race, lifecycle, and threaded SQLite tests; +- realm, Docker Compose, and Helm validation; +- CodeQL, Semgrep, container security, and central coverage checks on the exact + reviewed head. + +## Release boundary + +This integration remains in the Unreleased changelog until all protected checks +and independent review gates pass on the exact current head. The release step +then bumps service and lock metadata together, verifies artifacts and +provenance, publishes the matching tag, and rechecks the open-PR queue. diff --git a/helm/cwl-idp/templates/account-unification.yaml b/helm/cwl-idp/templates/account-unification.yaml index 26c6d5b..c6bb055 100644 --- a/helm/cwl-idp/templates/account-unification.yaml +++ b/helm/cwl-idp/templates/account-unification.yaml @@ -1,3 +1,25 @@ +{{- if and .Values.accountUnification.image.requireDigest (not .Values.accountUnification.image.digest) -}} +{{- fail "accountUnification.image.digest is required when accountUnification.image.requireDigest=true" -}} +{{- end -}} +{{- if .Values.accountUnification.persistence.enabled }} +apiVersion: v1 +kind: PersistentVolumeClaim +metadata: + name: {{ include "cwl-idp.fullname" . }}-account-unification-data + namespace: {{ include "cwl-idp.namespace" . }} + labels: + {{- include "cwl-idp.labels" . | nindent 4 }} + app.kubernetes.io/component: account-unification +spec: + accessModes: ["ReadWriteOnce"] + {{- if .Values.accountUnification.persistence.storageClassName }} + storageClassName: {{ .Values.accountUnification.persistence.storageClassName | quote }} + {{- end }} + resources: + requests: + storage: {{ .Values.accountUnification.persistence.size }} +--- +{{- end }} apiVersion: apps/v1 kind: Deployment metadata: @@ -27,7 +49,7 @@ spec: type: RuntimeDefault containers: - name: account-unification - image: "{{ .Values.accountUnification.image.repository }}:{{ .Values.accountUnification.image.tag }}" + image: "{{ .Values.accountUnification.image.repository }}:{{ .Values.accountUnification.image.tag }}{{ if .Values.accountUnification.image.digest }}@{{ .Values.accountUnification.image.digest }}{{ end }}" imagePullPolicy: {{ .Values.accountUnification.image.pullPolicy }} securityContext: allowPrivilegeEscalation: false @@ -43,14 +65,14 @@ spec: - name: http containerPort: 8099 env: - # ONLY bootstrap transport; real config comes from the KV store. - name: CWL_IDP_BOOTSTRAP value: "{{ .Values.accountUnification.bootstrap.mountPath }}/{{ .Values.accountUnification.bootstrap.fileName }}" volumeMounts: - name: bootstrap mountPath: {{ .Values.accountUnification.bootstrap.mountPath }} readOnly: true - # Writable scratch dir since the root filesystem is read-only. + - name: account-unification-data + mountPath: /var/lib/account-unification - name: tmp mountPath: /tmp readinessProbe: @@ -71,6 +93,13 @@ spec: - name: bootstrap secret: secretName: {{ .Values.accountUnification.bootstrap.secretName }} + - name: account-unification-data + {{- if .Values.accountUnification.persistence.enabled }} + persistentVolumeClaim: + claimName: {{ include "cwl-idp.fullname" . }}-account-unification-data + {{- else }} + emptyDir: {} + {{- end }} - name: tmp emptyDir: {} --- diff --git a/helm/cwl-idp/values.yaml b/helm/cwl-idp/values.yaml index f30514a..c5bf393 100644 --- a/helm/cwl-idp/values.yaml +++ b/helm/cwl-idp/values.yaml @@ -6,18 +6,25 @@ namespaceOverride: cwl-idp accountUnification: image: repository: cwl-idp/account-unification - tag: "0.2.0" + # Keep the chart aligned with the currently published package metadata. + # The release workflow bumps both together after exact-head verification. + tag: "0.1.0" + # Development may render a local tag. Production values must set + # requireDigest=true and provide the built image's sha256 digest. + digest: "" + requireDigest: false pullPolicy: IfNotPresent replicaCount: 1 service: port: 8099 - # Bootstrap transport ONLY: a pointer to the KV/DB config store. Real config - # + secrets are read from that store at runtime. Mount the bootstrap file via - # a secret and set the path here. bootstrap: secretName: cwl-idp-bootstrap mountPath: /bootstrap fileName: bootstrap.yaml + persistence: + enabled: true + size: 1Gi + storageClassName: "" resources: requests: cpu: 50m @@ -26,9 +33,6 @@ accountUnification: cpu: 250m memory: 256Mi -# Keycloak engine (Apache-2.0), templated by this chart. Set enabled=false to -# federate an externally-managed Keycloak. The passwordless-first realm is -# imported as-code from a ConfigMap built from deploy/keycloak/realm-cwl.json. keycloak: enabled: true image: @@ -37,17 +41,13 @@ keycloak: digest: "sha256:98fab020a3a490aba0978f237e2a06cd0ea42bf149c6cf10f11c0aaf27728ff2" pullPolicy: IfNotPresent replicaCount: 1 - # Public base URL Keycloak advertises (behind the WAF in prod). hostname: "http://localhost:8080" hostnameStrict: false httpEnabled: true service: httpPort: 8080 managementPort: 9000 - # Bootstrap admin + DB credentials are read from this pre-created secret - # (populated from your KV). Keys: admin-username, admin-password, db-password. existingSecret: cwl-idp-keycloak - # Realm import ConfigMap (kubectl create configmap ... --from-file=realm-cwl.json). realmImport: configMapName: cwl-idp-realm fileName: realm-cwl.json @@ -59,8 +59,6 @@ keycloak: cpu: "2" memory: 2Gi -# Bundled Postgres (MIT) for Keycloak. Point Keycloak at a managed Postgres by -# setting postgres.enabled=false and keycloak DB env from your own secret. postgres: enabled: true image: @@ -70,7 +68,6 @@ postgres: pullPolicy: IfNotPresent database: keycloak username: keycloak - # DB password is read from the keycloak.existingSecret key 'db-password'. storage: size: 8Gi resources: diff --git a/scripts/validate_realm.py b/scripts/validate_realm.py index 43fdd2e..db6e6c5 100644 --- a/scripts/validate_realm.py +++ b/scripts/validate_realm.py @@ -4,14 +4,14 @@ Parses the realm JSON and asserts the ecosystem-policy invariants hold, so a broken realm export is caught in CI before it ever reaches Keycloak: - * valid JSON, realm named and enabled; - * a passwordless browser flow is bound and contains NO password authenticator - but DOES use the WebAuthn passwordless authenticator (passkey-first); - * self-service password registration + reset are OFF; - * the employer ADFS is registered as an INBOUND SAML IdP with trustEmail; - * an LDAP/AD user-federation source is present; - * an OIDC/OAuth2.1 RP client template and the account-unification service - account client exist; no committed client secret is a real value. +* the named realm is enabled; +* the bound browser flow contains WebAuthn passwordless and no password form; +* self-service password registration and reset are disabled; +* external federation remains runtime desired state, not committed realm code; +* RP and service-account clients exist without committed real secrets; +* Keycloak 26 import compatibility excludes ``$`` annotation keys; +* the ``basic`` scope provides ``sub`` and is a realm default; +* ``naruon-web`` is a bounded-token public PKCE client with required claims. Usage: python scripts/validate_realm.py [path-to-realm.json] Exit 0 = valid, 1 = invalid (prints the failing checks). @@ -30,6 +30,7 @@ } PASSKEY_AUTHENTICATOR = f"webauthn-authenticator-{_CREDENTIAL_FACTOR}less" SECRET_PLACEHOLDER = "__set_from_kv__" +MAX_PUBLIC_TOKEN_LIFESPAN = 900 def _executions(realm: dict, alias: str) -> list[dict]: @@ -40,8 +41,12 @@ def _executions(realm: dict, alias: str) -> list[dict]: return [] -def _all_authenticators(realm: dict, alias: str, seen: set[str] | None = None) -> set[str]: - """Collect authenticator ids reachable from a flow, following subflows.""" +def _all_authenticators( + realm: dict, + alias: str, + seen: set[str] | None = None, +) -> set[str]: + """Collect authenticator IDs reachable from a flow, following subflows.""" seen = seen if seen is not None else set() if alias in seen: return set() @@ -56,6 +61,18 @@ def _all_authenticators(realm: dict, alias: str, seen: set[str] | None = None) - return found +def _public_token_lifespan(client: dict) -> int | None: + """Parse one optional client access-token lifespan as a positive integer.""" + raw_value = client.get("attributes", {}).get("access.token.lifespan") + if raw_value is None: + return None + try: + value = int(raw_value) + except (TypeError, ValueError): + return -1 + return value if str(value) == str(raw_value).strip() else -1 + + def validate(realm: dict) -> list[str]: """Return human-readable policy violations for a realm export.""" errors: list[str] = [] @@ -65,7 +82,6 @@ def validate(realm: dict) -> list[str]: if not realm.get("enabled", False): errors.append("realm must be enabled") - # Passwordless-first browser flow. browser_flow = realm.get("browserFlow") if not browser_flow: errors.append("browserFlow must be set") @@ -73,8 +89,7 @@ def validate(realm: dict) -> list[str]: authenticators = _all_authenticators(realm, browser_flow) if not authenticators: errors.append(f"browserFlow '{browser_flow}' has no executions defined") - disallowed_credential_used = authenticators & DISALLOWED_CREDENTIAL_AUTHENTICATORS - if disallowed_credential_used: + if authenticators & DISALLOWED_CREDENTIAL_AUTHENTICATORS: errors.append( "browserFlow includes a disallowed credential-form authenticator; " "ecosystem policy requires passkeys" @@ -85,31 +100,47 @@ def validate(realm: dict) -> list[str]: "ecosystem policy" ) + # Signup is headless. Keycloak sends a one-time verification and WebAuthn + # required-action link; the bound browser flow remains passwordless. if realm.get("registrationAllowed", False): - errors.append("registrationAllowed must be false") + errors.append( + "IdP-hosted registration must remain disabled; use the headless " + "registration API" + ) + if not realm.get("registrationEmailAsUsername", False): + errors.append("registrationEmailAsUsername must remain true") + passkey_enrollment_is_available = any( + action.get("providerId") == "webauthn-register-passwordless" + and action.get("enabled", False) + for action in realm.get("requiredActions", []) + ) + if not passkey_enrollment_is_available: + errors.append( + "passkey enrollment required action must remain enabled for " + "action-email enrollment" + ) + if realm.get("verifyEmail", False) and not realm.get("smtpServer"): + errors.append( + "verifyEmail requires a realm smtpServer; configure SMTP or disable " + "verifyEmail" + ) if realm.get("resetPasswordAllowed", False): errors.append("credential reset self-service must be false") - # Employer ADFS inbound SAML IdP. - idps = {i.get("alias"): i for i in realm.get("identityProviders", [])} - adfs = idps.get("employer-adfs") - if adfs is None: - errors.append("identity provider 'employer-adfs' is missing") - else: - if adfs.get("providerId") != "saml": - errors.append("employer-adfs must be a SAML identity provider") - if not adfs.get("trustEmail", False): - errors.append("employer-adfs must set trustEmail (verified-email auto-link)") - - # LDAP/AD federation. - storage = realm.get("components", {}).get( - "org.keycloak.storage.UserStorageProvider", [] - ) - if not any(c.get("providerId") == "ldap" for c in storage): - errors.append("an LDAP user-storage provider is required") + if realm.get("identityProviders"): + errors.append( + "identityProviders must not be committed; register external IdPs at " + "runtime via the federation registry API" + ) + if realm.get("components", {}).get( + "org.keycloak.storage.UserStorageProvider" + ): + errors.append( + "user-storage federation must not be committed; register LDAP/AD " + "sources at runtime via the federation registry API" + ) - # Clients: RP template + service account, no committed real secret. - clients = {c.get("clientId"): c for c in realm.get("clients", [])} + clients = {client.get("clientId"): client for client in realm.get("clients", [])} if "ecosystem-rp-template" not in clients: errors.append("OIDC RP client template 'ecosystem-rp-template' is missing") else: @@ -118,10 +149,11 @@ def validate(realm: dict) -> list[str]: errors.append("RP template must not enable the implicit flow (OAuth 2.1)") if rp.get("attributes", {}).get("pkce.code.challenge.method") != "S256": errors.append("RP template must require PKCE S256") - svc = clients.get("account-unification-svc") - if svc is None: + + service_client = clients.get("account-unification-svc") + if service_client is None: errors.append("service-account client 'account-unification-svc' is missing") - elif not svc.get("serviceAccountsEnabled", False): + elif not service_client.get("serviceAccountsEnabled", False): errors.append("account-unification-svc must enable service accounts") for client_id, client in clients.items(): @@ -129,12 +161,87 @@ def validate(realm: dict) -> list[str]: if secret is not None and secret != SECRET_PLACEHOLDER: errors.append(f"client '{client_id}' commits a non-placeholder secret") + for key_path in _dollar_keys(realm): + errors.append( + f"'$'-annotation key '{key_path}' breaks Keycloak 26 realm import" + ) + + scopes = {scope.get("name"): scope for scope in realm.get("clientScopes", [])} + basic = scopes.get("basic") + if basic is None: + errors.append("client scope 'basic' is required (sub claim source)") + elif not any( + mapper.get("protocolMapper") == "oidc-sub-mapper" + for mapper in basic.get("protocolMappers", []) + ): + errors.append("client scope 'basic' must include the oidc-sub-mapper") + if "basic" not in realm.get("defaultDefaultClientScopes", []): + errors.append("'basic' must be a realm default client scope") + + naruon = clients.get("naruon-web") + if naruon is None: + errors.append("concrete RP client 'naruon-web' is missing") + else: + if not naruon.get("publicClient", False): + errors.append("naruon-web must be a public (PKCE) client") + if naruon.get("implicitFlowEnabled", False): + errors.append("naruon-web must not enable the implicit flow") + if naruon.get("attributes", {}).get("pkce.code.challenge.method") != "S256": + errors.append("naruon-web must require PKCE S256") + token_lifespan = _public_token_lifespan(naruon) + if ( + token_lifespan is not None + and not 0 < token_lifespan <= MAX_PUBLIC_TOKEN_LIFESPAN + ): + errors.append( + "naruon-web access.token.lifespan must be an integer at or below " + f"{MAX_PUBLIC_TOKEN_LIFESPAN} seconds" + ) + naruon_mappers = { + mapper.get("protocolMapper") + for mapper in naruon.get("protocolMappers", []) + } + if "oidc-audience-mapper" not in naruon_mappers: + errors.append("naruon-web must include an audience mapper") + hardcoded_claims = { + mapper.get("config", {}).get("claim.name") + for mapper in naruon.get("protocolMappers", []) + if mapper.get("protocolMapper") == "oidc-hardcoded-claim-mapper" + } + for claim_name in ("role", "org", "workspace"): + if claim_name not in hardcoded_claims: + errors.append( + f"naruon-web must carry the hardcoded '{claim_name}' claim " + "naruon's session contract requires" + ) + if "basic" not in naruon.get("defaultClientScopes", []): + errors.append("naruon-web must assign the 'basic' default scope") + return errors +def _dollar_keys(node: object, prefix: str = "") -> list[str]: + """Collect every ``$``-prefixed object key with its JSON path.""" + found: list[str] = [] + if isinstance(node, dict): + for key, value in node.items(): + key_path = f"{prefix}.{key}" if prefix else str(key) + if str(key).startswith("$"): + found.append(key_path) + found.extend(_dollar_keys(value, key_path)) + elif isinstance(node, list): + for index, item in enumerate(node): + found.extend(_dollar_keys(item, f"{prefix}[{index}]")) + return found + + def main(argv: list[str]) -> int: """Run realm validation as a command-line check.""" - path = Path(argv[1]) if len(argv) > 1 else Path("deploy/keycloak/realm-cwl.json") + path = ( + Path(argv[1]) + if len(argv) > 1 + else Path("deploy/keycloak/realm-cwl.json") + ) try: realm = json.loads(path.read_text(encoding="utf-8")) except (OSError, json.JSONDecodeError) as exc: @@ -147,7 +254,10 @@ def main(argv: list[str]) -> int: for error in errors: print(f" - {error}", file=sys.stderr) return 1 - print(f"OK: {path} is a valid cwl-idp realm (passkey, ADFS, LDAP, OIDC RP).") + print( + f"OK: {path} is a valid cwl-idp realm " + "(passwordless, runtime federation, OIDC RPs)." + ) return 0 diff --git a/services/account_unification/Dockerfile b/services/account_unification/Dockerfile index f0d688c..ea97e55 100644 --- a/services/account_unification/Dockerfile +++ b/services/account_unification/Dockerfile @@ -13,7 +13,10 @@ WORKDIR /srv COPY pyproject.toml uv.lock ./ RUN uv sync --locked --no-dev --no-install-project \ - && adduser --system --no-create-home appuser + && adduser --system --no-create-home appuser \ + # Writable home for the audit database; /bootstrap stays read-only. + && mkdir -p /var/lib/account-unification \ + && chown appuser /var/lib/account-unification COPY app ./app diff --git a/services/account_unification/app/api.py b/services/account_unification/app/api.py index 3e8119b..fe042a7 100644 --- a/services/account_unification/app/api.py +++ b/services/account_unification/app/api.py @@ -13,6 +13,7 @@ ) from .models import FederatedIdentity, MergeRequest, MergeResult, UserAccount from .service import UnificationService +from .user_locks import UserOperationLockTimeout router = APIRouter() @@ -75,6 +76,11 @@ def merge_accounts( raise HTTPException(status_code=409, detail=str(exc)) from exc except InactiveAccountError as exc: raise HTTPException(status_code=409, detail=str(exc)) from exc + except UserOperationLockTimeout as exc: + raise HTTPException( + status_code=503, + detail="one of the requested accounts is being modified; retry", + ) from exc @router.get("/merges/{audit_id}/audit", tags=["merge"]) diff --git a/services/account_unification/app/audit.py b/services/account_unification/app/audit.py index dc967f6..262a5e7 100644 --- a/services/account_unification/app/audit.py +++ b/services/account_unification/app/audit.py @@ -1,12 +1,14 @@ -"""Append-only audit log for link/merge operations. +"""Append-only, thread-safe audit logging for identity merge operations. -Every mutating action is recorded. The default sink writes to the KV/DB store -under the two-word snake_case object ``account_merge_audit``. An in-memory sink -is used by tests. Records are immutable once written. +Every mutating merge action is recorded. The standalone sink writes to the +two-word snake_case table ``account_merge_audit``. SQLite access is serialized +inside the process and configured for bounded cross-process contention. """ from __future__ import annotations import json +import sqlite3 +import threading import time import uuid from dataclasses import dataclass, field @@ -27,11 +29,7 @@ class AuditEvent: class AuditSink(Protocol): - """Persistence contract for append-only audit events. - - The ellipsis bodies declare the Protocol contract only. Concrete - implementations are :class:`InMemoryAuditSink` and :class:`SqliteAuditSink`. - """ + """Persistence contract for append-only audit events.""" def record(self, event: AuditEvent) -> None: """Append one immutable audit event.""" @@ -41,24 +39,36 @@ def events_for(self, audit_id: str) -> list[AuditEvent]: """Return events for one correlation id in write order.""" ... + def close(self) -> None: + """Release resources held by the sink.""" + ... + @dataclass class InMemoryAuditSink: - """Test/dev sink keeping events in a list.""" + """Thread-safe test/dev sink keeping events in a list.""" events: list[AuditEvent] = field(default_factory=list) + _lock: threading.RLock = field( + default_factory=threading.RLock, init=False, repr=False + ) def record(self, event: AuditEvent) -> None: """Append an event to the in-memory list.""" - self.events.append(event) + with self._lock: + self.events.append(event) def events_for(self, audit_id: str) -> list[AuditEvent]: """Return recorded events for one correlation id.""" - return [event for event in self.events if event.audit_id == audit_id] + with self._lock: + return [event for event in self.events if event.audit_id == audit_id] + + def close(self) -> None: + """Release no-op in-memory resources.""" class SqliteAuditSink: - """Durable append-only sink backed by the ``account_merge_audit`` table.""" + """Durable append-only sink backed by ``account_merge_audit``.""" _SCHEMA = """ CREATE TABLE IF NOT EXISTS account_merge_audit ( @@ -75,39 +85,48 @@ class SqliteAuditSink: def __init__(self, database_path: str) -> None: """Open the audit database and ensure the audit table exists.""" - import sqlite3 - - self._connection = sqlite3.connect(database_path) - self._connection.execute(self._SCHEMA) - self._connection.commit() + self._lock = threading.RLock() + self._connection = sqlite3.connect( + database_path, + timeout=10.0, + check_same_thread=False, + ) + with self._lock: + self._connection.execute("PRAGMA busy_timeout = 10000") + self._connection.execute("PRAGMA journal_mode = WAL") + self._connection.execute("PRAGMA synchronous = NORMAL") + self._connection.execute(self._SCHEMA) + self._connection.commit() def record(self, event: AuditEvent) -> None: """Persist one event row and commit immediately.""" - self._connection.execute( - "INSERT INTO account_merge_audit " - "(audit_id, event_type, actor_name, survivor_user_id, " - " duplicate_user_id, payload_json, created_at) " - "VALUES (?, ?, ?, ?, ?, ?, ?)", - ( - event.audit_id, - event.event_type, - event.actor, - event.survivor_user_id, - event.duplicate_user_id, - event.payload_json, - event.created_at, - ), - ) - self._connection.commit() + with self._lock, self._connection: + self._connection.execute( + "INSERT INTO account_merge_audit " + "(audit_id, event_type, actor_name, survivor_user_id, " + " duplicate_user_id, payload_json, created_at) " + "VALUES (?, ?, ?, ?, ?, ?, ?)", + ( + event.audit_id, + event.event_type, + event.actor, + event.survivor_user_id, + event.duplicate_user_id, + event.payload_json, + event.created_at, + ), + ) def events_for(self, audit_id: str) -> list[AuditEvent]: """Load events for one correlation id in event sequence order.""" - rows = self._connection.execute( - "SELECT audit_id, event_type, actor_name, survivor_user_id, " - "duplicate_user_id, payload_json, created_at " - "FROM account_merge_audit WHERE audit_id = ? ORDER BY event_sequence", - (audit_id,), - ).fetchall() + with self._lock: + rows = self._connection.execute( + "SELECT audit_id, event_type, actor_name, survivor_user_id, " + "duplicate_user_id, payload_json, created_at " + "FROM account_merge_audit " + "WHERE audit_id = ? ORDER BY event_sequence", + (audit_id,), + ).fetchall() return [ AuditEvent( audit_id=row[0], @@ -123,11 +142,12 @@ def events_for(self, audit_id: str) -> list[AuditEvent]: def close(self) -> None: """Close the SQLite connection.""" - self._connection.close() + with self._lock: + self._connection.close() class AuditLogger: - """Builds and persists audit events; returns a stable correlation id.""" + """Build and persist audit events behind a stable correlation id.""" def __init__(self, sink: AuditSink) -> None: """Create an audit logger around one sink implementation.""" @@ -163,3 +183,7 @@ def emit( def events_for(self, audit_id: str) -> list[AuditEvent]: """Return the audit trail for one merge correlation id.""" return self._sink.events_for(audit_id) + + def close(self) -> None: + """Close the underlying audit sink.""" + self._sink.close() diff --git a/services/account_unification/app/auth.py b/services/account_unification/app/auth.py new file mode 100644 index 0000000..8ff2a8f --- /dev/null +++ b/services/account_unification/app/auth.py @@ -0,0 +1,51 @@ +"""Operator bearer-token authentication for the admin API surface. + +The account-unification service exposes privileged operations — account +merge, SCIM provisioning/deactivation, and the federation registry — that must +never be reachable unauthenticated. Every mutating and identity-reading route +requires an operator bearer token; ``/healthz`` stays open for probes. + +The token is a shared operator secret loaded from the KV/DB config store +(``operator_api_token``), compared in constant time. This is deliberately a +coarse operator gate: the service already runs behind the ecosystem network +boundary and holds realm-management privileges, so the token gates *access to +the service*, and finer per-action authorization remains Keycloak's job. +""" +from __future__ import annotations + +import hmac + +from fastapi import Depends, Header, HTTPException, Request + + +def _configured_operator_token(request: Request) -> str | None: + """Return the operator token wired into application state, if any.""" + return getattr(request.app.state, "operator_api_token", None) + + +def require_operator_token( + request: Request, + authorization: str | None = Header(default=None), +) -> None: + """Authenticate an operator bearer token; raise 401/403 otherwise. + + Fails closed: a service started without a configured operator token rejects + every authenticated request rather than allowing open access. + """ + expected = _configured_operator_token(request) + if not expected: + # No token configured => the privileged surface is unavailable, never + # implicitly open. + raise HTTPException(status_code=503, detail="operator authentication unavailable") + if not authorization or not authorization.startswith("Bearer "): + raise HTTPException( + status_code=401, + detail="operator bearer token required", + headers={"WWW-Authenticate": "Bearer"}, + ) + presented = authorization[len("Bearer ") :].strip() + if not hmac.compare_digest(presented, expected): + raise HTTPException(status_code=403, detail="invalid operator token") + + +operator_auth_dependency = Depends(require_operator_token) diff --git a/services/account_unification/app/config.py b/services/account_unification/app/config.py index 4061bd7..ecf9fe6 100644 --- a/services/account_unification/app/config.py +++ b/services/account_unification/app/config.py @@ -2,11 +2,13 @@ Nothing here reads process environment. :func:`load_service_config` takes an opened :class:`~app.kv_store.KvStore` (from :mod:`app.bootstrap`) and returns a -frozen config object. Missing required keys fail loudly at startup. +frozen config object. Missing or unsafe values fail loudly at startup. """ from __future__ import annotations +import math from dataclasses import dataclass +from urllib.parse import urlsplit from .kv_store import KvStore @@ -19,22 +21,39 @@ KEY_MERGE_CONFLICT_POLICY = "merge_conflict_policy" KEY_ALLOW_UNVERIFIED_LINK = "allow_unverified_email_link" KEY_REQUEST_TIMEOUT_SECONDS = "request_timeout_seconds" +KEY_OPERATOR_API_TOKEN = "operator_api_token" +KEY_REGISTRATION_API_TOKEN = "registration_api_token" +KEY_REGISTRATION_CLIENT_ID = "registration_client_id" +KEY_REGISTRATION_REDIRECT_URI = "registration_redirect_uri" +KEY_REGISTRATION_ACTION_LIFESPAN_SECONDS = ( + "registration_action_lifespan_seconds" +) +KEY_AUDIT_DATABASE_PATH = "audit_database_path" + +MAX_REGISTRATION_ACTION_LIFESPAN_SECONDS = 3600 @dataclass(frozen=True) class ServiceConfig: - """Runtime settings loaded from the config store.""" + """Validated runtime settings loaded from the config store.""" # Keycloak Admin REST API wiring. The service authenticates to the realm - # token endpoint with a confidential service-account client (client - # credentials) that holds realm-management view-users/manage-users roles. + # token endpoint with a confidential service-account client. keycloak_server_url: str keycloak_realm: str keycloak_client_id: str keycloak_client_secret: str + # Privileged and product registration surfaces deliberately use different + # bearer credentials so relying products never acquire operator authority. + operator_api_token: str + registration_api_token: str | None = None + registration_client_id: str | None = None + registration_redirect_uri: str | None = None + registration_action_lifespan_seconds: int = 900 + audit_database_path: str = "/var/lib/account-unification/audit.db" merge_conflict_policy: str = "survivor_wins" - # Hard default False: the ecosystem policy forbids linking/merging on an - # unverified email. Present as config only so audits can prove it is off. + # This is an invariant, not a deployer-selectable feature. The field remains + # so audit/config evidence can prove it was explicitly disabled. allow_unverified_email_link: bool = False request_timeout_seconds: float = 10.0 @@ -49,26 +68,179 @@ def _require(store: KvStore, namespace: str, entry_key: str) -> str: return value -def _as_bool(raw: str | None, default: bool) -> bool: - """Parse a store value as a permissive boolean.""" - if raw is None: +def _as_bool( + raw: str | None, + default: bool, + *, + entry_key: str, +) -> bool: + """Parse one explicit boolean or fail startup on ambiguous text.""" + if raw is None or raw == "": return default - return raw.strip().lower() in {"1", "true", "yes", "on"} + normalized = raw.strip().lower() + if normalized in {"1", "true", "yes", "on"}: + return True + if normalized in {"0", "false", "no", "off"}: + return False + raise RuntimeError(f"config '{entry_key}' must be a boolean") + + +def _as_finite_float( + raw: str | None, + default: float, + *, + entry_key: str, + allow_zero: bool, +) -> float: + """Parse a runtime duration and reject negative, NaN, or infinite values.""" + candidate = str(default) if raw is None or raw == "" else raw + try: + value = float(candidate) + except (TypeError, ValueError) as exc: + raise RuntimeError( + f"config '{entry_key}' must be a finite number" + ) from exc + if not math.isfinite(value) or value < 0 or (value == 0 and not allow_zero): + qualifier = "non-negative" if allow_zero else "positive" + raise RuntimeError( + f"config '{entry_key}' must be a finite {qualifier} number" + ) + return value + + +def _as_positive_int( + raw: str | None, + default: int, + *, + entry_key: str, + maximum: int, +) -> int: + """Parse a positive bounded integer configuration value.""" + candidate = str(default) if raw is None or raw == "" else raw.strip() + try: + value = int(candidate) + except (TypeError, ValueError) as exc: + raise RuntimeError( + f"config '{entry_key}' must be a positive integer" + ) from exc + if str(value) != candidate or value <= 0 or value > maximum: + raise RuntimeError( + f"config '{entry_key}' must be a positive integer at or below " + f"{maximum}" + ) + return value + + +def _validated_https_uri(raw_uri: str, *, entry_key: str) -> str: + """Return an absolute HTTPS URI or fail startup.""" + candidate = raw_uri.strip() + parsed = urlsplit(candidate) + if ( + parsed.scheme != "https" + or not parsed.hostname + or parsed.username is not None + or parsed.password is not None + or parsed.fragment + ): + raise RuntimeError( + f"config '{entry_key}' must be an absolute HTTPS URI without " + "credentials or fragments" + ) + return candidate + + +def _registration_settings( + store: KvStore, + namespace: str, + registration_api_token: str | None, +) -> tuple[str | None, str | None, int]: + """Load all-or-nothing action-email registration settings.""" + if registration_api_token is None: + return None, None, 900 + client_id = _require(store, namespace, KEY_REGISTRATION_CLIENT_ID) + if len(client_id) > 255 or any( + character.isspace() or ord(character) < 0x20 + for character in client_id + ): + raise RuntimeError( + f"config '{KEY_REGISTRATION_CLIENT_ID}' must be one bounded client ID" + ) + redirect_uri = _validated_https_uri( + _require(store, namespace, KEY_REGISTRATION_REDIRECT_URI), + entry_key=KEY_REGISTRATION_REDIRECT_URI, + ) + lifespan_seconds = _as_positive_int( + store.get(namespace, KEY_REGISTRATION_ACTION_LIFESPAN_SECONDS), + 900, + entry_key=KEY_REGISTRATION_ACTION_LIFESPAN_SECONDS, + maximum=MAX_REGISTRATION_ACTION_LIFESPAN_SECONDS, + ) + return client_id, redirect_uri, lifespan_seconds def load_service_config(store: KvStore, namespace: str) -> ServiceConfig: - """Build the :class:`ServiceConfig` from the KV store.""" + """Build and validate the :class:`ServiceConfig` from the KV store.""" + operator_api_token = _require(store, namespace, KEY_OPERATOR_API_TOKEN) + registration_api_token = ( + store.get(namespace, KEY_REGISTRATION_API_TOKEN) or None + ) + if registration_api_token == operator_api_token: + raise RuntimeError( + "config 'registration_api_token' must differ from " + "'operator_api_token'" + ) + ( + registration_client_id, + registration_redirect_uri, + registration_action_lifespan_seconds, + ) = _registration_settings( + store, + namespace, + registration_api_token, + ) + + allow_unverified_email_link = _as_bool( + store.get(namespace, KEY_ALLOW_UNVERIFIED_LINK), + default=False, + entry_key=KEY_ALLOW_UNVERIFIED_LINK, + ) + if allow_unverified_email_link: + raise RuntimeError( + "config 'allow_unverified_email_link' must remain false" + ) + + merge_conflict_policy = ( + store.get(namespace, KEY_MERGE_CONFLICT_POLICY) or "survivor_wins" + ) + if merge_conflict_policy != "survivor_wins": + raise RuntimeError( + "config 'merge_conflict_policy' must be 'survivor_wins'" + ) + return ServiceConfig( keycloak_server_url=_require(store, namespace, KEY_KEYCLOAK_SERVER_URL), keycloak_realm=_require(store, namespace, KEY_KEYCLOAK_REALM), keycloak_client_id=_require(store, namespace, KEY_KEYCLOAK_CLIENT_ID), - keycloak_client_secret=_require(store, namespace, KEY_KEYCLOAK_CLIENT_SECRET), - merge_conflict_policy=store.get(namespace, KEY_MERGE_CONFLICT_POLICY) - or "survivor_wins", - allow_unverified_email_link=_as_bool( - store.get(namespace, KEY_ALLOW_UNVERIFIED_LINK), default=False + keycloak_client_secret=_require( + store, namespace, KEY_KEYCLOAK_CLIENT_SECRET + ), + operator_api_token=operator_api_token, + registration_api_token=registration_api_token, + registration_client_id=registration_client_id, + registration_redirect_uri=registration_redirect_uri, + registration_action_lifespan_seconds=( + registration_action_lifespan_seconds + ), + audit_database_path=( + store.get(namespace, KEY_AUDIT_DATABASE_PATH) + or "/var/lib/account-unification/audit.db" ), - request_timeout_seconds=float( - store.get(namespace, KEY_REQUEST_TIMEOUT_SECONDS) or "10.0" + merge_conflict_policy=merge_conflict_policy, + allow_unverified_email_link=allow_unverified_email_link, + request_timeout_seconds=_as_finite_float( + store.get(namespace, KEY_REQUEST_TIMEOUT_SECONDS), + 10.0, + entry_key=KEY_REQUEST_TIMEOUT_SECONDS, + allow_zero=False, ), ) diff --git a/services/account_unification/app/federation.py b/services/account_unification/app/federation.py new file mode 100644 index 0000000..b108890 --- /dev/null +++ b/services/account_unification/app/federation.py @@ -0,0 +1,413 @@ +"""DB-backed runtime federation registry with redacted operator responses. + +External identity providers are deployment configuration, never committed realm +code. Desired state is stored in the KV/DB backend and converged into Keycloak. +Stored and applied secrets never enter HTTP responses: only explicitly approved, +non-secret provider fields are disclosed to operators. +""" +from __future__ import annotations + +import logging +import threading + +from fastapi import APIRouter, Depends, HTTPException, Request +from pydantic import BaseModel, ConfigDict, Field + +from .kv_store import KvStore +from .product_keycloak_client import ProductAdminApi + +logger = logging.getLogger(__name__) + +FEDERATION_PROVIDER_NAMESPACE = "federation_identity_providers" +_SUPPORTED_PROVIDER_IDS = {"saml", "oidc", "keycloak-oidc"} +_MAX_PROVIDER_ALIAS_LENGTH = 63 +_MAX_PROVIDER_CONFIG_ENTRIES = 64 +_MAX_PROVIDER_CONFIG_KEY_LENGTH = 128 +_MAX_PROVIDER_CONFIG_VALUE_LENGTH = 16_384 +_REDACTED_VALUE = "" +_ALIAS_ALPHABET = frozenset("abcdefghijklmnopqrstuvwxyz0123456789-") +_ALIAS_EDGE_ALPHABET = frozenset("abcdefghijklmnopqrstuvwxyz0123456789") +# Unknown fields are redacted. This allowlist contains only values that are +# useful for operator diagnosis and are not credential material. +_EXPOSED_PROVIDER_CONFIG_KEYS = frozenset( + { + "alias", + "authorizationUrl", + "backchannelSupported", + "defaultScope", + "entityId", + "guiOrder", + "hideOnLoginPage", + "issuer", + "logoutUrl", + "metadataDescriptorUrl", + "principalAttribute", + "principalType", + "signatureAlgorithm", + "singleLogoutServiceUrl", + "singleSignOnServiceUrl", + "syncMode", + "tokenUrl", + "useJwksUrl", + "useMetadataDescriptorUrl", + "userInfoUrl", + "validateSignature", + } +) + + +class IdentityProviderRegistration(BaseModel): + """Desired state for one external identity provider.""" + + model_config = ConfigDict(extra="forbid") + + provider_alias: str = Field(description="Keycloak IdP alias (URL-safe slug).") + display_name: str = Field(min_length=1, max_length=120) + provider_id: str = Field( + description="Keycloak provider id: saml | oidc | keycloak-oidc." + ) + enabled: bool = True + trust_email: bool = Field( + default=False, + description="Trust the asserted email as verified.", + ) + provider_config: dict[str, str] = Field( + default_factory=dict, + description="Keycloak IdP configuration map.", + ) + + +class IdentityProviderView(BaseModel): + """Safe operator view of a provider registration with secrets redacted.""" + + provider_alias: str + display_name: str + provider_id: str + enabled: bool + trust_email: bool + provider_config: dict[str, str] + + @classmethod + def from_registration( + cls, registration: IdentityProviderRegistration + ) -> "IdentityProviderView": + """Build a redacted view from stored desired state.""" + return cls( + provider_alias=registration.provider_alias, + display_name=registration.display_name, + provider_id=registration.provider_id, + enabled=registration.enabled, + trust_email=registration.trust_email, + provider_config=_redacted_provider_config( + registration.provider_config + ), + ) + + +class IdentityProviderStatus(BaseModel): + """Redacted stored registration plus its Keycloak convergence status.""" + + registration: IdentityProviderView + applied_to_keycloak: bool + + +class FederationService: + """Persist desired IdP state and reconcile Keycloak without lock-held I/O.""" + + def __init__(self, store: KvStore, api: ProductAdminApi) -> None: + """Create a federation service around one store and Keycloak client.""" + self._store = store + self._api = api + self._state_lock = threading.RLock() + self._convergence_lock = threading.RLock() + + def list_registrations(self) -> list[IdentityProviderStatus]: + """Return all stored registrations with live convergence status.""" + with self._state_lock: + raw_values = list( + self._store.get_all(FEDERATION_PROVIDER_NAMESPACE).values() + ) + registrations = [ + self._parse_registration(raw_value) for raw_value in raw_values + ] + statuses = [ + self._status_for(registration) for registration in registrations + ] + return sorted(statuses, key=lambda item: item.registration.provider_alias) + + def get_registration(self, provider_alias: str) -> IdentityProviderStatus: + """Return one stored registration or raise HTTP 404.""" + _validate_provider_alias(provider_alias) + with self._state_lock: + raw_value = self._store.get( + FEDERATION_PROVIDER_NAMESPACE, provider_alias + ) + if raw_value is None: + raise HTTPException( + status_code=404, + detail="identity provider not registered", + ) + return self._status_for(self._parse_registration(raw_value)) + + def put_registration( + self, + provider_alias: str, + registration: IdentityProviderRegistration, + ) -> IdentityProviderStatus: + """Validate, persist, and attempt to converge one provider.""" + if registration.provider_alias != provider_alias: + raise HTTPException( + status_code=400, + detail="path alias and body provider_alias must match", + ) + _validate_registration(registration) + with self._convergence_lock: + with self._state_lock: + self._store.put( + FEDERATION_PROVIDER_NAMESPACE, + provider_alias, + registration.model_dump_json(), + ) + applied = self._try_apply(registration) + return self._status_for(registration, applied=applied) + + def delete_registration(self, provider_alias: str) -> None: + """Remove one provider from Keycloak and the desired-state store.""" + _validate_provider_alias(provider_alias) + with self._convergence_lock: + with self._state_lock: + raw_value = self._store.get( + FEDERATION_PROVIDER_NAMESPACE, provider_alias + ) + if raw_value is None: + raise HTTPException( + status_code=404, + detail="identity provider not registered", + ) + if self._api.get_identity_provider(provider_alias) is not None: + self._api.delete_identity_provider(provider_alias) + with self._state_lock: + self._store.delete( + FEDERATION_PROVIDER_NAMESPACE, provider_alias + ) + + def apply_all(self) -> list[IdentityProviderStatus]: + """Re-converge Keycloak from a snapshot of stored desired state.""" + with self._state_lock: + raw_values = list( + self._store.get_all(FEDERATION_PROVIDER_NAMESPACE).values() + ) + registrations = [ + self._parse_registration(raw_value) for raw_value in raw_values + ] + statuses: list[IdentityProviderStatus] = [] + with self._convergence_lock: + for registration in registrations: + statuses.append( + self._status_for( + registration, + applied=self._try_apply(registration), + ) + ) + return sorted(statuses, key=lambda item: item.registration.provider_alias) + + def _parse_registration(self, raw_value: str) -> IdentityProviderRegistration: + """Parse and validate one stored registration.""" + registration = IdentityProviderRegistration.model_validate_json(raw_value) + _validate_registration(registration) + return registration + + def _apply(self, registration: IdentityProviderRegistration) -> None: + """Create or replace one Keycloak identity-provider instance.""" + payload = _to_keycloak_payload(registration) + existing = self._api.get_identity_provider(registration.provider_alias) + if existing is None: + self._api.create_identity_provider(payload) + else: + self._api.update_identity_provider( + registration.provider_alias, payload + ) + + def _try_apply(self, registration: IdentityProviderRegistration) -> bool: + """Attempt convergence and report failure without losing desired state.""" + try: + self._apply(registration) + except Exception: + logger.exception( + "identity-provider convergence failed alias=%s", + registration.provider_alias, + ) + return False + return True + + def _status_for( + self, + registration: IdentityProviderRegistration, + *, + applied: bool | None = None, + ) -> IdentityProviderStatus: + """Build a redacted status, tolerating temporary Keycloak outages.""" + if applied is None: + try: + applied = ( + self._api.get_identity_provider( + registration.provider_alias + ) + is not None + ) + except Exception: + logger.warning( + "identity-provider status unavailable alias=%s", + registration.provider_alias, + exc_info=True, + ) + applied = False + return IdentityProviderStatus( + registration=IdentityProviderView.from_registration(registration), + applied_to_keycloak=applied, + ) + + +def _validate_provider_alias(provider_alias: str) -> None: + """Validate one ASCII lowercase alphanumeric-and-hyphen provider alias.""" + valid = ( + isinstance(provider_alias, str) + and 1 <= len(provider_alias) <= _MAX_PROVIDER_ALIAS_LENGTH + and provider_alias[0] in _ALIAS_EDGE_ALPHABET + and provider_alias[-1] in _ALIAS_EDGE_ALPHABET + and all(character in _ALIAS_ALPHABET for character in provider_alias) + ) + if not valid: + raise HTTPException( + status_code=400, + detail="provider_alias must be an ASCII lowercase URL-safe slug", + ) + + +def _validate_registration( + registration: IdentityProviderRegistration, +) -> None: + """Validate one provider registration and bounded config map.""" + _validate_provider_alias(registration.provider_alias) + if registration.provider_id not in _SUPPORTED_PROVIDER_IDS: + raise HTTPException( + status_code=400, + detail="provider_id must be one of: saml, oidc, keycloak-oidc", + ) + if len(registration.provider_config) > _MAX_PROVIDER_CONFIG_ENTRIES: + raise HTTPException( + status_code=400, + detail="provider_config contains too many entries", + ) + for config_key, config_value in registration.provider_config.items(): + if ( + not config_key + or len(config_key) > _MAX_PROVIDER_CONFIG_KEY_LENGTH + or len(config_value) > _MAX_PROVIDER_CONFIG_VALUE_LENGTH + ): + raise HTTPException( + status_code=400, + detail="provider_config key or value exceeds allowed bounds", + ) + + +def _redacted_provider_config( + provider_config: dict[str, str], +) -> dict[str, str]: + """Expose only explicitly safe provider configuration values.""" + return { + config_key: ( + config_value + if config_key in _EXPOSED_PROVIDER_CONFIG_KEYS + else _REDACTED_VALUE + ) + for config_key, config_value in provider_config.items() + } + + +def _to_keycloak_payload( + registration: IdentityProviderRegistration, +) -> dict: + """Convert desired state to a Keycloak Admin API representation.""" + return { + "alias": registration.provider_alias, + "displayName": registration.display_name, + "providerId": registration.provider_id, + "enabled": registration.enabled, + "trustEmail": registration.trust_email, + "storeToken": False, + "addReadTokenRoleOnCreate": False, + "authenticateByDefault": False, + "linkOnly": False, + "config": dict(registration.provider_config), + } + + +federation_router = APIRouter(prefix="/federation", tags=["federation"]) + + +def get_federation_service(request: Request) -> FederationService: + """Return the wired federation service from application state.""" + service = getattr(request.app.state, "federation_service", None) + if service is None: + raise HTTPException( + status_code=503, detail="federation service not ready" + ) + return service + + +@federation_router.get( + "/identity-providers", response_model=list[IdentityProviderStatus] +) +def list_identity_providers( + service: FederationService = Depends(get_federation_service), +) -> list[IdentityProviderStatus]: + """List every registered external identity provider.""" + return service.list_registrations() + + +@federation_router.get( + "/identity-providers/{provider_alias}", + response_model=IdentityProviderStatus, +) +def get_identity_provider( + provider_alias: str, + service: FederationService = Depends(get_federation_service), +) -> IdentityProviderStatus: + """Return one registered external identity provider.""" + return service.get_registration(provider_alias) + + +@federation_router.put( + "/identity-providers/{provider_alias}", + response_model=IdentityProviderStatus, +) +def put_identity_provider( + provider_alias: str, + registration: IdentityProviderRegistration, + service: FederationService = Depends(get_federation_service), +) -> IdentityProviderStatus: + """Register or update one provider and converge Keycloak.""" + return service.put_registration(provider_alias, registration) + + +@federation_router.delete( + "/identity-providers/{provider_alias}", status_code=204 +) +def delete_identity_provider( + provider_alias: str, + service: FederationService = Depends(get_federation_service), +) -> None: + """Remove one provider from Keycloak and desired state.""" + service.delete_registration(provider_alias) + + +@federation_router.post( + "/identity-providers:apply", + response_model=list[IdentityProviderStatus], +) +def apply_identity_providers( + service: FederationService = Depends(get_federation_service), +) -> list[IdentityProviderStatus]: + """Re-converge Keycloak from the stored desired state.""" + return service.apply_all() diff --git a/services/account_unification/app/healthcheck.py b/services/account_unification/app/healthcheck.py index 673fffc..fd33ac6 100644 --- a/services/account_unification/app/healthcheck.py +++ b/services/account_unification/app/healthcheck.py @@ -7,16 +7,53 @@ import json import sys +import urllib.parse import urllib.request DEFAULT_URL = "http://127.0.0.1:8099/healthz" +_ALLOWED_SCHEMES = frozenset({"http", "https"}) + + +class _HttpOnlyRedirectHandler(urllib.request.HTTPRedirectHandler): + """Drop redirects whose target scheme is not HTTP(S).""" + + def redirect_request(self, req, fp, code, msg, headers, newurl): # noqa: ANN001 + """Return an HTTP(S) redirect request, or reject another scheme.""" + if urllib.parse.urlsplit(newurl).scheme.lower() not in _ALLOWED_SCHEMES: + return None + return super().redirect_request(req, fp, code, msg, headers, newurl) + + +def _build_http_only_opener() -> urllib.request.OpenerDirector: + """Build an opener that can speak only HTTP(S), without file/FTP handlers.""" + opener = urllib.request.OpenerDirector() + opener.add_handler(urllib.request.HTTPHandler()) + opener.add_handler(urllib.request.HTTPSHandler()) + opener.add_handler(_HttpOnlyRedirectHandler()) + # OpenerDirector has no implicit default handlers. Register this before the + # error processor so non-2xx responses raise HTTPError instead of returning + # ``None`` to the context manager below. + opener.add_handler(urllib.request.HTTPDefaultErrorHandler()) + opener.add_handler(urllib.request.HTTPErrorProcessor()) + return opener + + +def _open_health_url(url: str): # noqa: ANN202 + """Open an HTTP(S) health URL through the restricted opener.""" + return _build_http_only_opener().open(url, timeout=5) def main(url: str = DEFAULT_URL) -> int: """Check the configured health endpoint and return a shell status code.""" + scheme = urllib.parse.urlsplit(url).scheme.lower() + if scheme not in _ALLOWED_SCHEMES: + print(f"healthcheck failed: unsupported URL scheme {scheme!r}", file=sys.stderr) + return 1 try: - # nosemgrep: python.lang.security.audit.dynamic-urllib-use-detected.dynamic-urllib-use-detected -- container healthcheck against a hardcoded loopback default (127.0.0.1); any override is a deployment-controlled target, not user input. - with urllib.request.urlopen(url, timeout=5) as response: # noqa: S310 + # The initial and redirected schemes are constrained to HTTP(S), and the + # opener carries no handler for file, FTP, or data URLs. + # nosemgrep: python.lang.security.audit.dynamic-urllib-use-detected.dynamic-urllib-use-detected + with _open_health_url(url) as response: # noqa: S310 body = json.loads(response.read().decode("utf-8")) except Exception as exc: # pragma: no cover - network failure path print(f"healthcheck failed: {exc}", file=sys.stderr) diff --git a/services/account_unification/app/identifiers.py b/services/account_unification/app/identifiers.py new file mode 100644 index 0000000..ec61282 --- /dev/null +++ b/services/account_unification/app/identifiers.py @@ -0,0 +1,45 @@ +"""Opaque-identifier validation for Keycloak Admin REST path segments. + +Every caller-supplied identifier (user id, provider alias, audit id) is +interpolated into an Admin API URL path. A value like ``../users/victim`` or a +percent-encoded separator would let a caller escape the intended resource, so +identifiers are validated as a single opaque path segment before any URL is +built. This is the defense-in-depth layer applied inside the Admin client +itself, independent of any boundary validation. +""" +from __future__ import annotations + +# Keycloak ids are UUIDs and aliases are slugs, so URI delimiters and encoding +# markers are never legitimate inside one opaque identifier. +_MAX_IDENTIFIER_LENGTH = 255 +_FORBIDDEN_IDENTIFIER_CHARACTERS = frozenset({"/", "\\", "%", "?", "#"}) + + +class InvalidIdentifierError(ValueError): + """Raised when an identifier is not a single safe path segment.""" + + +def validate_path_segment(value: str, *, field_name: str = "identifier") -> str: + """Return ``value`` if it is one safe opaque path segment, else raise. + + Rejects empty/oversized values, path and URI delimiters, dot navigation, + percent-encoding, and control characters. + """ + if not isinstance(value, str) or not value: + raise InvalidIdentifierError(f"{field_name} must be a non-empty string") + if len(value) > _MAX_IDENTIFIER_LENGTH: + raise InvalidIdentifierError(f"{field_name} is too long") + if value in {".", ".."}: + raise InvalidIdentifierError( + f"{field_name} must not be a path navigation token" + ) + for character in value: + if character in _FORBIDDEN_IDENTIFIER_CHARACTERS: + raise InvalidIdentifierError( + f"{field_name} must not contain path, encoding, query, or fragment delimiters" + ) + if ord(character) < 0x20 or ord(character) == 0x7F: + raise InvalidIdentifierError( + f"{field_name} must not contain control characters" + ) + return value diff --git a/services/account_unification/app/keycloak_client.py b/services/account_unification/app/keycloak_client.py index 7d6ab05..4ab2d57 100644 --- a/services/account_unification/app/keycloak_client.py +++ b/services/account_unification/app/keycloak_client.py @@ -108,6 +108,10 @@ def set_user_attribute(self, user_id: str, key: str, value: str) -> None: """Set one single-valued user attribute.""" ... + def get_user_attribute(self, user_id: str, key: str) -> str | None: + """Return one single-valued user attribute, or ``None`` if unset.""" + ... + class HttpAdminApi: """httpx-backed :class:`AdminApi` for a live Keycloak instance. @@ -339,6 +343,18 @@ def set_user_attribute(self, user_id: str, key: str, value: str) -> None: f"/admin/realms/{self._realm}/users/{user_id}", {"attributes": attributes} ) + def get_user_attribute(self, user_id: str, key: str) -> str | None: + """Return one single-valued Keycloak user attribute, or ``None``. + + Keycloak stores attributes as string lists; this returns the first + element (or a bare string, defensively), or ``None`` when unset. + """ + data = self._get(f"/admin/realms/{self._realm}/users/{user_id}") + value = (data.get("attributes") or {}).get(key) + if isinstance(value, list): + return value[0] if value else None + return value if isinstance(value, str) else None + # -- transport --------------------------------------------------------- def _get(self, path: str, params: dict | None = None) -> dict | list: """Issue an authenticated GET and parse JSON.""" diff --git a/services/account_unification/app/kv_store.py b/services/account_unification/app/kv_store.py index ad9bd7a..341e506 100644 --- a/services/account_unification/app/kv_store.py +++ b/services/account_unification/app/kv_store.py @@ -1,38 +1,46 @@ -"""Config/secret store abstraction (KV/DB), the ONLY source of runtime config. +"""Thread-safe config/secret store abstraction, the runtime source of truth. -The service never calls ``os.getenv`` for real configuration or secrets. It -reads a single bootstrap pointer (see :mod:`app.bootstrap`) that names one of -these backends, then loads everything else from here. DB objects use two-word -snake_case names (``idp_config_entries`` with columns ``entry_key`` / -``entry_value``). +The service never reads scattered environment variables for real configuration +or secrets. Database objects use two-word snake_case names, including +``idp_config_entries`` and its ``entry_key`` / ``entry_value`` columns. """ from __future__ import annotations import sqlite3 +import threading from typing import Protocol class KvStore(Protocol): - """Read interface for the config/secret store. + """Read/write interface for the configuration and secret store.""" - The ellipsis bodies declare the Protocol contract only. Concrete - implementations are :class:`InMemoryKvStore` and :class:`SqliteKvStore`. - """ + def put(self, namespace: str, entry_key: str, entry_value: str) -> None: + """Store one value in one namespace.""" + ... def get(self, namespace: str, entry_key: str) -> str | None: - """Return the value for ``entry_key`` in ``namespace`` or ``None``.""" + """Return a value or ``None`` when it is absent.""" ... def get_all(self, namespace: str) -> dict[str, str]: - """Return every entry in ``namespace`` as a dict.""" + """Return every entry in one namespace.""" + ... + + def delete(self, namespace: str, entry_key: str) -> None: + """Remove one entry if present.""" + ... + + def close(self) -> None: + """Release resources held by the store.""" ... class InMemoryKvStore: - """Dict-backed store for tests and ephemeral bootstrap shims.""" + """Thread-safe dict-backed store for tests and ephemeral bootstrap shims.""" def __init__(self, seed: dict[str, dict[str, str]] | None = None) -> None: """Create a store seeded by namespace and entry key.""" + self._lock = threading.RLock() self._data: dict[str, dict[str, str]] = {} if seed: for namespace, entries in seed.items(): @@ -40,24 +48,30 @@ def __init__(self, seed: dict[str, dict[str, str]] | None = None) -> None: def put(self, namespace: str, entry_key: str, entry_value: str) -> None: """Store one value in one namespace.""" - self._data.setdefault(namespace, {})[entry_key] = entry_value + with self._lock: + self._data.setdefault(namespace, {})[entry_key] = entry_value def get(self, namespace: str, entry_key: str) -> str | None: """Return one value from one namespace, if present.""" - return self._data.get(namespace, {}).get(entry_key) + with self._lock: + return self._data.get(namespace, {}).get(entry_key) def get_all(self, namespace: str) -> dict[str, str]: """Return a copy of every value in one namespace.""" - return dict(self._data.get(namespace, {})) + with self._lock: + return dict(self._data.get(namespace, {})) + def delete(self, namespace: str, entry_key: str) -> None: + """Remove one value from one namespace if present.""" + with self._lock: + self._data.get(namespace, {}).pop(entry_key, None) -class SqliteKvStore: - """SQLite-backed store for standalone / dev deployments. + def close(self) -> None: + """Release no-op in-memory resources.""" - Table ``idp_config_entries`` is keyed by (``config_namespace``, - ``entry_key``). Values are stored as text; secret handling (encryption at - rest, rotation) is delegated to the platform for the postgres backend. - """ + +class SqliteKvStore: + """SQLite-backed store for standalone and development deployments.""" _SCHEMA = """ CREATE TABLE IF NOT EXISTS idp_config_entries ( @@ -70,39 +84,61 @@ class SqliteKvStore: def __init__(self, database_path: str) -> None: """Open the SQLite store and ensure the config table exists.""" - self._database_path = database_path - self._connection = sqlite3.connect(database_path) - self._connection.execute(self._SCHEMA) - self._connection.commit() + self._lock = threading.RLock() + self._connection = sqlite3.connect( + database_path, + timeout=10.0, + check_same_thread=False, + ) + with self._lock: + self._connection.execute("PRAGMA busy_timeout = 10000") + self._connection.execute("PRAGMA journal_mode = WAL") + self._connection.execute("PRAGMA synchronous = NORMAL") + self._connection.execute(self._SCHEMA) + self._connection.commit() def put(self, namespace: str, entry_key: str, entry_value: str) -> None: """Upsert one config value.""" - self._connection.execute( - "INSERT INTO idp_config_entries (config_namespace, entry_key, entry_value) " - "VALUES (?, ?, ?) ON CONFLICT(config_namespace, entry_key) " - "DO UPDATE SET entry_value = excluded.entry_value", - (namespace, entry_key, entry_value), - ) - self._connection.commit() + with self._lock, self._connection: + self._connection.execute( + "INSERT INTO idp_config_entries " + "(config_namespace, entry_key, entry_value) " + "VALUES (?, ?, ?) " + "ON CONFLICT(config_namespace, entry_key) " + "DO UPDATE SET entry_value = excluded.entry_value", + (namespace, entry_key, entry_value), + ) def get(self, namespace: str, entry_key: str) -> str | None: """Return one config value, if present.""" - row = self._connection.execute( - "SELECT entry_value FROM idp_config_entries " - "WHERE config_namespace = ? AND entry_key = ?", - (namespace, entry_key), - ).fetchone() + with self._lock: + row = self._connection.execute( + "SELECT entry_value FROM idp_config_entries " + "WHERE config_namespace = ? AND entry_key = ?", + (namespace, entry_key), + ).fetchone() return row[0] if row else None def get_all(self, namespace: str) -> dict[str, str]: """Return every config value in one namespace.""" - rows = self._connection.execute( - "SELECT entry_key, entry_value FROM idp_config_entries " - "WHERE config_namespace = ?", - (namespace,), - ).fetchall() + with self._lock: + rows = self._connection.execute( + "SELECT entry_key, entry_value FROM idp_config_entries " + "WHERE config_namespace = ?", + (namespace,), + ).fetchall() return {entry_key: entry_value for entry_key, entry_value in rows} + def delete(self, namespace: str, entry_key: str) -> None: + """Remove one config value from one namespace if present.""" + with self._lock, self._connection: + self._connection.execute( + "DELETE FROM idp_config_entries " + "WHERE config_namespace = ? AND entry_key = ?", + (namespace, entry_key), + ) + def close(self) -> None: """Close the SQLite connection.""" - self._connection.close() + with self._lock: + self._connection.close() diff --git a/services/account_unification/app/main.py b/services/account_unification/app/main.py index ddff44a..a565e0b 100644 --- a/services/account_unification/app/main.py +++ b/services/account_unification/app/main.py @@ -1,66 +1,148 @@ -"""FastAPI application factory and lifespan wiring. +"""FastAPI application factory, dependency wiring, and resource lifecycle. -On startup the app reads the single bootstrap pointer, opens the KV/DB config -store, loads config + secrets from it (no scattered os.getenv), and builds the -live Keycloak-backed :class:`UnificationService`. A ``/healthz`` endpoint reports -readiness for the compose/k8s probe (and additionally probes Keycloak + DB when -wired against a live instance). +Startup reads one bootstrap pointer, opens the KV/DB configuration store, and +builds the Keycloak-backed services. Privileged routers are authenticated and +path-validated; ``/healthz`` remains open for orchestrator probes. """ from __future__ import annotations +import os +import tempfile from contextlib import asynccontextmanager +from pathlib import Path from fastapi import FastAPI from . import __version__ from .api import router from .audit import AuditLogger, SqliteAuditSink +from .auth import operator_auth_dependency from .bootstrap import load_bootstrap_descriptor, open_config_store from .config import load_service_config -from .keycloak_client import HttpAdminApi +from .federation import FederationService, federation_router +from .path_security import ( + ScimPathValidationError, + admin_path_security_dependency, + scim_path_security_dependency, + scim_path_validation_exception_handler, +) +from .product_keycloak_client import ProductHttpAdminApi +from .registration import registration_auth_dependency, registration_router from .scim import scim_router from .service import UnificationService +from .user_locks import SqliteUserOperationLocks + + +def _ensure_parent_directory(database_path: str) -> None: + """Create a filesystem parent for a persistent SQLite database path.""" + if database_path == ":memory:": + return + Path(database_path).expanduser().resolve().parent.mkdir( + parents=True, exist_ok=True + ) + + +def _user_operation_lock_path(audit_database_path: str) -> tuple[str, bool]: + """Return a durable sidecar path or one secure temporary test path.""" + if audit_database_path != ":memory:": + return f"{audit_database_path}.user-operation-locks.sqlite3", False + descriptor, temporary_path = tempfile.mkstemp( + prefix="keyverse-user-operation-locks-", + suffix=".sqlite3", + ) + os.close(descriptor) + return temporary_path, True def build_service(app: FastAPI) -> None: - """Wire the service from the bootstrap pointer + KV store.""" + """Wire all live service dependencies from the bootstrap configuration.""" descriptor = load_bootstrap_descriptor() store = open_config_store(descriptor) config = load_service_config(store, descriptor.namespace) + _ensure_parent_directory(config.audit_database_path) - api = HttpAdminApi( + api = ProductHttpAdminApi( server_url=config.keycloak_server_url, realm=config.keycloak_realm, client_id=config.keycloak_client_id, client_secret=config.keycloak_client_secret, timeout_seconds=config.request_timeout_seconds, ) - # Audit sink co-located with the config store for standalone; prod swaps in - # a Postgres-backed sink writing account_merge_audit. - audit_path = descriptor.sqlite_path or "account_unification.db" - audit = AuditLogger(SqliteAuditSink(audit_path)) - - app.state.unification_service = UnificationService(api, audit, config) + audit = AuditLogger(SqliteAuditSink(config.audit_database_path)) + lock_database_path, temporary_lock_database = _user_operation_lock_path( + config.audit_database_path + ) + user_operation_locks = SqliteUserOperationLocks(lock_database_path) + + app.state.config_store = store + app.state.unification_service = UnificationService( + api, + audit, + config, + user_operation_locks, + ) app.state.audit_logger = audit app.state.keycloak_api = api + app.state.user_operation_locks = user_operation_locks + app.state.user_operation_lock_database_path = lock_database_path + app.state.temporary_user_operation_lock_database = temporary_lock_database + app.state.federation_service = FederationService(store, api) + app.state.operator_api_token = config.operator_api_token + app.state.registration_api_token = config.registration_api_token + app.state.registration_client_id = config.registration_client_id + app.state.registration_redirect_uri = config.registration_redirect_uri + app.state.registration_action_lifespan_seconds = ( + config.registration_action_lifespan_seconds + ) app.state.ready = True +def _close_resource(resource) -> None: + """Close one optional resource that exposes a callable ``close`` method.""" + close = getattr(resource, "close", None) + if callable(close): + close() + + +def _remove_temporary_lock_database(app: FastAPI) -> None: + """Remove the secure sidecar used only with an in-memory audit database.""" + if not getattr(app.state, "temporary_user_operation_lock_database", False): + return + lock_database_path = getattr( + app.state, + "user_operation_lock_database_path", + None, + ) + if lock_database_path: + Path(lock_database_path).unlink(missing_ok=True) + + @asynccontextmanager async def lifespan(app: FastAPI): - """Build the live service before accepting traffic.""" + """Build live dependencies and release them on application shutdown.""" app.state.ready = False build_service(app) - yield + try: + yield + finally: + app.state.ready = False + _close_resource(getattr(app.state, "keycloak_api", None)) + _close_resource(getattr(app.state, "audit_logger", None)) + _close_resource(getattr(app.state, "config_store", None)) + _remove_temporary_lock_database(app) def create_app(*, wire: bool = True) -> FastAPI: - """Create the FastAPI app. ``wire=False`` skips startup wiring (tests).""" + """Create the FastAPI app; ``wire=False`` skips startup wiring for tests.""" app = FastAPI( title="cwl-idp account-unification", version=__version__, lifespan=lifespan if wire else None, ) + app.add_exception_handler( + ScimPathValidationError, + scim_path_validation_exception_handler, + ) if not wire: app.state.ready = True @@ -68,13 +150,38 @@ def create_app(*, wire: bool = True) -> FastAPI: def healthz() -> dict: """Return readiness status for container and orchestration probes.""" return { - "status": "ok" if getattr(app.state, "ready", False) else "starting", + "status": ( + "ok" if getattr(app.state, "ready", False) else "starting" + ), "service": "account-unification", "version": __version__, } - app.include_router(router) - app.include_router(scim_router) + app.include_router( + router, + dependencies=[ + operator_auth_dependency, + admin_path_security_dependency, + ], + ) + app.include_router( + scim_router, + dependencies=[ + operator_auth_dependency, + scim_path_security_dependency, + ], + ) + app.include_router( + federation_router, + dependencies=[ + operator_auth_dependency, + admin_path_security_dependency, + ], + ) + app.include_router( + registration_router, + dependencies=[registration_auth_dependency], + ) return app diff --git a/services/account_unification/app/path_security.py b/services/account_unification/app/path_security.py new file mode 100644 index 0000000..ecfd28d --- /dev/null +++ b/services/account_unification/app/path_security.py @@ -0,0 +1,62 @@ +"""Path-parameter validation dependencies for privileged API routers. + +FastAPI decodes route parameters before endpoint execution. Validating every +decoded value as one opaque segment prevents traversal, encoded-separator, and +control-character payloads from reaching Keycloak Admin REST path builders. +""" +from __future__ import annotations + +from fastapi import Depends, HTTPException, Request +from fastapi.responses import JSONResponse + +from .identifiers import InvalidIdentifierError, validate_path_segment + +SCIM_ERROR_SCHEMA = "urn:ietf:params:scim:api:messages:2.0:Error" +SCIM_MEDIA_TYPE = "application/scim+json" + + +class ScimPathValidationError(ValueError): + """Represent one unsafe decoded SCIM path parameter.""" + + +def _validate_path_parameters(request: Request) -> None: + """Validate every decoded route parameter as one opaque path segment.""" + for field_name, value in request.path_params.items(): + validate_path_segment(str(value), field_name=field_name) + + +def require_safe_admin_path_parameters(request: Request) -> None: + """Reject unsafe privileged API path parameters with HTTP 400.""" + try: + _validate_path_parameters(request) + except InvalidIdentifierError as error: + raise HTTPException(status_code=400, detail=str(error)) from error + + +def require_safe_scim_path_parameters(request: Request) -> None: + """Raise a SCIM-specific error for an unsafe decoded path parameter.""" + try: + _validate_path_parameters(request) + except InvalidIdentifierError as error: + raise ScimPathValidationError(str(error)) from error + + +def scim_path_validation_exception_handler( + request: Request, + error: ScimPathValidationError, +) -> JSONResponse: + """Render one RFC 7644 error at the response body root.""" + del request + return JSONResponse( + status_code=400, + media_type=SCIM_MEDIA_TYPE, + content={ + "schemas": [SCIM_ERROR_SCHEMA], + "detail": str(error), + "status": "400", + }, + ) + + +admin_path_security_dependency = Depends(require_safe_admin_path_parameters) +scim_path_security_dependency = Depends(require_safe_scim_path_parameters) diff --git a/services/account_unification/app/product_keycloak_client.py b/services/account_unification/app/product_keycloak_client.py new file mode 100644 index 0000000..0056b36 --- /dev/null +++ b/services/account_unification/app/product_keycloak_client.py @@ -0,0 +1,433 @@ +"""Product-facing extensions for the Keycloak Admin REST API client. + +The core merge/SCIM engine depends only on :class:`AdminApi`. Product features +such as passwordless registration and runtime federation require a wider +surface. This module keeps those concerns modular while preserving the same +authenticated transport, path-safety, and one-shot token refresh behavior. +""" +from __future__ import annotations + +from collections.abc import Callable +from typing import Protocol +from urllib.parse import urlsplit + +import httpx + +from .identifiers import InvalidIdentifierError, validate_path_segment +from .keycloak_client import AdminApi, HttpAdminApi, _to_keycloak_user +from .models import ( + FederatedIdentity, + GroupMembership, + RoleMapping, + UserAccount, +) + +# ``None`` marks exactly one validated, caller-controlled path segment. The +# whitelist is a second line of defense after every public method validates its +# dynamic values before interpolation. +_ADMIN_PATH_PATTERNS: tuple[tuple[str | None, ...], ...] = ( + ("users",), + ("users", None), + ("users", None, "federated-identity"), + ("users", None, "federated-identity", None), + ("users", None, "role-mappings"), + ("users", None, "role-mappings", "realm"), + ("users", None, "role-mappings", "clients", None), + ("users", None, "groups"), + ("users", None, "groups", None), + ("users", None, "execute-actions-email"), + ("identity-provider", "instances"), + ("identity-provider", "instances", None), +) + + +class ProductAdminApi(AdminApi, Protocol): + """Extended Keycloak contract used by registration and federation modules.""" + + def send_execute_actions_email( + self, + user_id: str, + action_aliases: list[str], + *, + client_id: str, + redirect_uri: str, + lifespan_seconds: int, + ) -> None: + """Send a one-time email link for verified passkey enrollment.""" + ... + + def delete_user(self, user_id: str) -> None: + """Delete one user during failed registration rollback.""" + ... + + def get_identity_provider(self, provider_alias: str) -> dict | None: + """Return one identity-provider instance or ``None`` when absent.""" + ... + + def create_identity_provider(self, provider_payload: dict) -> None: + """Create an identity-provider instance from an admin representation.""" + ... + + def update_identity_provider( + self, provider_alias: str, provider_payload: dict + ) -> None: + """Replace an identity-provider instance.""" + ... + + def delete_identity_provider(self, provider_alias: str) -> None: + """Delete an identity-provider instance.""" + ... + + +class ProductHttpAdminApi(HttpAdminApi): + """Keycloak client with registration, federation, and hardened transport.""" + + def __init__( + self, + server_url: str, + realm: str, + client_id: str, + client_secret: str, + token_realm: str | None = None, + timeout_seconds: float = 10.0, + transport=None, + ) -> None: + """Create a product adapter after validating all configured realms.""" + validate_path_segment(realm, field_name="keycloak_realm") + if token_realm is not None: + validate_path_segment(token_realm, field_name="token_realm") + super().__init__( + server_url=server_url, + realm=realm, + client_id=client_id, + client_secret=client_secret, + token_realm=token_realm, + timeout_seconds=timeout_seconds, + transport=transport, + ) + + @staticmethod + def _safe_segment(value: str, field_name: str) -> str: + """Return one validated opaque Admin REST path segment.""" + return validate_path_segment(value, field_name=field_name) + + # -- hardened core API ------------------------------------------------- + def get_user(self, user_id: str) -> UserAccount: + """Return one user after validating its opaque id.""" + return super().get_user(self._safe_segment(user_id, "user_id")) + + def replace_user(self, user_id: str, user: UserAccount) -> None: + """Replace one user after validating its opaque id.""" + super().replace_user(self._safe_segment(user_id, "user_id"), user) + + def list_federated_identities( + self, user_id: str + ) -> list[FederatedIdentity]: + """List external identities after validating the user id.""" + return super().list_federated_identities( + self._safe_segment(user_id, "user_id") + ) + + def add_federated_identity( + self, user_id: str, identity: FederatedIdentity + ) -> None: + """Attach an external identity using validated path segments.""" + safe_user_id = self._safe_segment(user_id, "user_id") + self._safe_segment(identity.identity_provider, "identity_provider") + super().add_federated_identity(safe_user_id, identity) + + def remove_federated_identity( + self, user_id: str, identity_provider: str + ) -> None: + """Remove an external identity using validated path segments.""" + super().remove_federated_identity( + self._safe_segment(user_id, "user_id"), + self._safe_segment(identity_provider, "identity_provider"), + ) + + def list_role_mappings(self, user_id: str) -> list[RoleMapping]: + """List role mappings after validating the user id.""" + return super().list_role_mappings( + self._safe_segment(user_id, "user_id") + ) + + def add_role_mapping(self, user_id: str, role: RoleMapping) -> None: + """Add a role mapping using validated path segments.""" + safe_user_id = self._safe_segment(user_id, "user_id") + if role.client_id is not None: + self._safe_segment(role.client_id, "client_id") + super().add_role_mapping(safe_user_id, role) + + def remove_role_mapping(self, user_id: str, role: RoleMapping) -> None: + """Remove a role mapping using validated path segments.""" + safe_user_id = self._safe_segment(user_id, "user_id") + if role.client_id is not None: + self._safe_segment(role.client_id, "client_id") + super().remove_role_mapping(safe_user_id, role) + + def list_group_memberships( + self, user_id: str + ) -> list[GroupMembership]: + """List group memberships after validating the user id.""" + return super().list_group_memberships( + self._safe_segment(user_id, "user_id") + ) + + def add_group_membership( + self, user_id: str, group: GroupMembership + ) -> None: + """Add a group membership using validated path segments.""" + self._safe_segment(group.group_id, "group_id") + super().add_group_membership( + self._safe_segment(user_id, "user_id"), group + ) + + def remove_group_membership( + self, user_id: str, group: GroupMembership + ) -> None: + """Remove a group membership using validated path segments.""" + self._safe_segment(group.group_id, "group_id") + super().remove_group_membership( + self._safe_segment(user_id, "user_id"), group + ) + + def deactivate_user(self, user_id: str) -> None: + """Disable one user after validating its opaque id.""" + super().deactivate_user(self._safe_segment(user_id, "user_id")) + + def set_user_attribute(self, user_id: str, key: str, value: str) -> None: + """Set a user attribute after validating the user id.""" + super().set_user_attribute( + self._safe_segment(user_id, "user_id"), key, value + ) + + def get_user_attribute(self, user_id: str, key: str) -> str | None: + """Read a user attribute after validating the user id.""" + return super().get_user_attribute( + self._safe_segment(user_id, "user_id"), key + ) + + # -- guarded transport ------------------------------------------------- + @staticmethod + def _validate_admin_suffix(path_segments: tuple[str, ...]) -> None: + """Require one known Admin REST suffix and validate dynamic segments.""" + for pattern in _ADMIN_PATH_PATTERNS: + if len(path_segments) != len(pattern): + continue + if not all( + expected is None or expected == actual + for expected, actual in zip(pattern, path_segments, strict=True) + ): + continue + for index, (expected, actual) in enumerate( + zip(pattern, path_segments, strict=True) + ): + if expected is None: + validate_path_segment( + actual, + field_name=f"admin_path_segment_{index}", + ) + return + raise InvalidIdentifierError( + "request path is not an allowed Keycloak Admin REST route" + ) + + def _guard_path(self, path: str) -> str: + """Accept only known Admin REST routes with opaque dynamic segments.""" + if not path.startswith("/"): + raise InvalidIdentifierError("request path must be absolute") + if any(character in path for character in ("%", "\\", "?", "#")): + raise InvalidIdentifierError( + "request path must not contain encoding or URI delimiters" + ) + segments = path.split("/") + if segments[0] != "" or any(segment == "" for segment in segments[1:]): + raise InvalidIdentifierError( + "request path must not contain empty segments" + ) + if any( + ord(character) < 0x20 or ord(character) == 0x7F + for segment in segments + for character in segment + ): + raise InvalidIdentifierError( + "request path must not contain control characters" + ) + prefix = ("admin", "realms", self._realm) + path_segments = tuple(segments[1:]) + if path_segments[:3] != prefix: + raise InvalidIdentifierError( + "request path must target the configured Keycloak realm" + ) + self._validate_admin_suffix(path_segments[3:]) + return path + + def _send_with_reauth( + self, make_request: Callable[[], httpx.Response] + ) -> httpx.Response: + """Send a request and retry exactly once after an expired-token 401.""" + response = make_request() + if response.status_code == 401: + self._token = None + response = make_request() + response.raise_for_status() + return response + + def _get(self, path: str, params: dict | None = None) -> dict | list: + """Issue a guarded authenticated GET and parse JSON.""" + guarded_path = self._guard_path(path) + response = self._send_with_reauth( + lambda: self._client.get( + guarded_path, params=params, headers=self._auth_header() + ) + ) + return response.json() + + def _post(self, path: str, body) -> dict: + """Issue a guarded authenticated POST and parse optional JSON.""" + guarded_path = self._guard_path(path) + response = self._send_with_reauth( + lambda: self._client.post( + guarded_path, json=body, headers=self._auth_header() + ) + ) + return response.json() if response.content else {} + + def _put(self, path: str, body: dict) -> None: + """Issue a guarded authenticated PUT.""" + guarded_path = self._guard_path(path) + self._send_with_reauth( + lambda: self._client.put( + guarded_path, json=body, headers=self._auth_header() + ) + ) + + def _delete(self, path: str, body=None) -> None: + """Issue a guarded authenticated DELETE with optional JSON.""" + guarded_path = self._guard_path(path) + + def send_delete() -> httpx.Response: + """Build and send one DELETE using the current bearer token.""" + request = self._client.build_request( + "DELETE", guarded_path, json=body, headers=self._auth_header() + ) + return self._client.send(request) + + self._send_with_reauth(send_delete) + + # -- product extensions ------------------------------------------------ + def create_user(self, user: UserAccount) -> str: + """Create a user, refreshing an expired token before retrying once.""" + path = self._guard_path(f"/admin/realms/{self._realm}/users") + response = self._send_with_reauth( + lambda: self._client.post( + path, + json=_to_keycloak_user(user), + headers=self._auth_header(), + ) + ) + location = response.headers.get("Location", "") + if location: + created_user_id = location.rstrip("/").rsplit("/", 1)[-1] + return validate_path_segment( + created_user_id, field_name="created_user_id" + ) + found = self.find_user_by_username(user.user_name or "") + if found is None: + return "" + return validate_path_segment( + found.user_id, field_name="created_user_id" + ) + + def send_execute_actions_email( + self, + user_id: str, + action_aliases: list[str], + *, + client_id: str, + redirect_uri: str, + lifespan_seconds: int, + ) -> None: + """Send a bounded one-time email for verification and passkey setup.""" + safe_user_id = self._safe_segment(user_id, "user_id") + safe_client_id = self._safe_segment(client_id, "client_id") + parsed_redirect = urlsplit(redirect_uri) + if ( + parsed_redirect.scheme != "https" + or not parsed_redirect.hostname + or parsed_redirect.username is not None + or parsed_redirect.password is not None + or parsed_redirect.fragment + ): + raise ValueError("redirect_uri must be an absolute HTTPS URI") + if lifespan_seconds <= 0: + raise ValueError("lifespan_seconds must be positive") + if not action_aliases or any( + not alias + or len(alias) > 128 + or any(ord(character) < 0x20 for character in alias) + for alias in action_aliases + ): + raise ValueError("action_aliases must contain bounded action names") + path = self._guard_path( + f"/admin/realms/{self._realm}/users/{safe_user_id}/" + "execute-actions-email" + ) + self._send_with_reauth( + lambda: self._client.put( + path, + params={ + "client_id": safe_client_id, + "redirect_uri": redirect_uri, + "lifespan": lifespan_seconds, + }, + json=list(action_aliases), + headers=self._auth_header(), + ) + ) + + def delete_user(self, user_id: str) -> None: + """Delete one user during failed registration rollback.""" + safe_user_id = self._safe_segment(user_id, "user_id") + self._delete(f"/admin/realms/{self._realm}/users/{safe_user_id}") + + def get_identity_provider(self, provider_alias: str) -> dict | None: + """Return an identity provider or ``None`` for a Keycloak 404.""" + safe_alias = self._safe_segment(provider_alias, "provider_alias") + try: + data = self._get( + f"/admin/realms/{self._realm}/identity-provider/instances/" + f"{safe_alias}" + ) + except httpx.HTTPStatusError as error: + if error.response.status_code == 404: + return None + raise + return data if isinstance(data, dict) else None + + def create_identity_provider(self, provider_payload: dict) -> None: + """Create one Keycloak identity-provider instance.""" + self._safe_segment(provider_payload.get("alias"), "provider_alias") + self._post( + f"/admin/realms/{self._realm}/identity-provider/instances", + provider_payload, + ) + + def update_identity_provider( + self, provider_alias: str, provider_payload: dict + ) -> None: + """Replace one Keycloak identity-provider instance.""" + safe_alias = self._safe_segment(provider_alias, "provider_alias") + self._put( + f"/admin/realms/{self._realm}/identity-provider/instances/" + f"{safe_alias}", + provider_payload, + ) + + def delete_identity_provider(self, provider_alias: str) -> None: + """Delete one Keycloak identity-provider instance.""" + safe_alias = self._safe_segment(provider_alias, "provider_alias") + self._delete( + f"/admin/realms/{self._realm}/identity-provider/instances/" + f"{safe_alias}" + ) diff --git a/services/account_unification/app/registration.py b/services/account_unification/app/registration.py new file mode 100644 index 0000000..86af145 --- /dev/null +++ b/services/account_unification/app/registration.py @@ -0,0 +1,287 @@ +"""Headless passwordless self-registration through one-time action email. + +First-party product backends submit accounts through a dedicated bearer-token +surface. The service creates a password-free account, then asks Keycloak to send +a bounded verification and passkey-enrollment link. A failed email request +rolls the account back so no unusable orphan remains. +""" +from __future__ import annotations + +import hmac +import re +import threading +import time + +import httpx +from fastapi import APIRouter, Depends, Header, HTTPException, Request +from pydantic import BaseModel, ConfigDict, Field + +from .models import UserAccount +from .product_keycloak_client import ProductAdminApi + +registration_router = APIRouter(prefix="/registration", tags=["registration"]) + +VERIFY_EMAIL_REQUIRED_ACTION = "VERIFY_EMAIL" +PASSKEY_ENROLL_REQUIRED_ACTION = "webauthn-register-passwordless" + +EMAIL_MAX_LENGTH = 254 +NAME_MAX_LENGTH = 100 +CONTROL_CHARACTER_PATTERN = re.compile(r"[\x00-\x1f\x7f]") +_LOCAL_ATOM_PUNCTUATION = frozenset("!#$%&'*+-/=?^_`{|}~.") + +REGISTRATION_RATE_LIMIT_WINDOW_SECONDS = 300.0 +REGISTRATION_RATE_LIMIT_MAX_ATTEMPTS = 30 +_registration_attempt_lock = threading.Lock() +_registration_attempt_windows: dict[str, tuple[float, int]] = {} + + +class RegistrationRequest(BaseModel): + """One password-free registration submission from a product signup page.""" + + model_config = ConfigDict(extra="forbid") + + email_address: str = Field(min_length=3, max_length=EMAIL_MAX_LENGTH) + first_name: str | None = Field(default=None, max_length=NAME_MAX_LENGTH) + last_name: str | None = Field(default=None, max_length=NAME_MAX_LENGTH) + + +class RegistrationResult(BaseModel): + """Public outcome after a passkey-enrollment email has been accepted.""" + + account_id: str + email_address: str + + +def require_registration_token( + request: Request, + authorization: str | None = Header(default=None), +) -> None: + """Authenticate the dedicated registration bearer token.""" + expected = getattr(request.app.state, "registration_api_token", None) + if not expected: + raise HTTPException( + status_code=503, + detail="registration authentication unavailable", + ) + if not authorization or not authorization.startswith("Bearer "): + raise HTTPException( + status_code=401, + detail="registration bearer token required", + headers={"WWW-Authenticate": "Bearer"}, + ) + presented = authorization[len("Bearer ") :].strip() + if not hmac.compare_digest(presented, expected): + raise HTTPException(status_code=403, detail="invalid registration token") + + +registration_auth_dependency = Depends(require_registration_token) + + +def get_admin_api(request: Request) -> ProductAdminApi: + """Return the wired product Keycloak API from application state.""" + api = getattr(request.app.state, "keycloak_api", None) + if api is None: + raise HTTPException(status_code=503, detail="keycloak api unavailable") + return api + + +def reset_rate_limit_state() -> None: + """Clear process-local registration counters for deterministic tests.""" + with _registration_attempt_lock: + _registration_attempt_windows.clear() + + +def _registration_client_key(request: Request) -> str: + """Return the direct peer address used for process-local throttling.""" + return request.client.host if request.client is not None else "unknown-client" + + +def _record_registration_attempt(client_key: str) -> None: + """Enforce an independent fixed-window registration limit per caller.""" + now = time.monotonic() + with _registration_attempt_lock: + window_start, attempt_count = _registration_attempt_windows.get( + client_key, (now, 0) + ) + if now - window_start > REGISTRATION_RATE_LIMIT_WINDOW_SECONDS: + window_start, attempt_count = now, 0 + attempt_count += 1 + _registration_attempt_windows[client_key] = ( + window_start, + attempt_count, + ) + if attempt_count > REGISTRATION_RATE_LIMIT_MAX_ATTEMPTS: + raise HTTPException( + status_code=429, + detail="registration temporarily rate limited", + ) + + +def _registration_settings(request: Request) -> tuple[str, str, int]: + """Return complete passwordless enrollment settings or fail closed.""" + client_id = getattr(request.app.state, "registration_client_id", None) + redirect_uri = getattr(request.app.state, "registration_redirect_uri", None) + lifespan_seconds = getattr( + request.app.state, + "registration_action_lifespan_seconds", + None, + ) + if ( + not client_id + or not redirect_uri + or not isinstance(lifespan_seconds, int) + or lifespan_seconds <= 0 + ): + raise HTTPException( + status_code=503, + detail="registration enrollment unavailable", + ) + return client_id, redirect_uri, lifespan_seconds + + +def _has_valid_email_shape(email_address: str) -> bool: + """Return whether an email has bounded, non-ambiguous syntax.""" + if email_address.count("@") != 1 or any( + character.isspace() for character in email_address + ): + return False + local_part, domain_part = email_address.split("@", 1) + if ( + not local_part + or local_part.startswith(".") + or local_part.endswith(".") + or ".." in local_part + or not all( + character.isalnum() or character in _LOCAL_ATOM_PUNCTUATION + for character in local_part + ) + ): + return False + if ( + not domain_part + or "." not in domain_part + or domain_part.startswith(".") + or domain_part.endswith(".") + or ".." in domain_part + ): + return False + labels = domain_part.split(".") + return all( + 1 <= len(label) <= 63 + and not label.startswith("-") + and not label.endswith("-") + and all(character.isalnum() or character == "-" for character in label) + for label in labels + ) + + +def _validated_email(raw_email: str) -> str: + """Normalize and shape-check a registration email address.""" + email_address = raw_email.strip().lower() + if ( + len(email_address) > EMAIL_MAX_LENGTH + or CONTROL_CHARACTER_PATTERN.search(email_address) + or not _has_valid_email_shape(email_address) + ): + raise HTTPException(status_code=422, detail="invalid_email_address") + return email_address + + +def _validated_name(raw_name: str | None) -> str | None: + """Trim an optional display-name part and reject controls.""" + if raw_name is None: + return None + candidate = raw_name.strip() + if not candidate: + return None + if CONTROL_CHARACTER_PATTERN.search(candidate): + raise HTTPException(status_code=422, detail="invalid_name") + return candidate + + +def _initialize_account( + api: ProductAdminApi, + account_id: str, + *, + client_id: str, + redirect_uri: str, + lifespan_seconds: int, +) -> None: + """Send verification/passkey actions or roll back the new account.""" + try: + api.send_execute_actions_email( + account_id, + [ + VERIFY_EMAIL_REQUIRED_ACTION, + PASSKEY_ENROLL_REQUIRED_ACTION, + ], + client_id=client_id, + redirect_uri=redirect_uri, + lifespan_seconds=lifespan_seconds, + ) + except Exception as initialization_error: + try: + api.delete_user(account_id) + except Exception as rollback_error: + raise HTTPException( + status_code=502, + detail="account_initialization_rollback_failed", + ) from rollback_error + raise HTTPException( + status_code=502, + detail="account_initialization_failed", + ) from initialization_error + + +@registration_router.post( + "/accounts", + response_model=RegistrationResult, + status_code=201, +) +def register_account( + request_body: RegistrationRequest, + request: Request, + api: ProductAdminApi = Depends(get_admin_api), +) -> RegistrationResult: + """Create a password-free account and send one enrollment email.""" + client_id, redirect_uri, lifespan_seconds = _registration_settings(request) + _record_registration_attempt(_registration_client_key(request)) + email_address = _validated_email(request_body.email_address) + if api.find_users_by_email(email_address): + raise HTTPException( + status_code=409, + detail="email_already_registered", + ) + + try: + account_id = api.create_user( + UserAccount( + user_id="", + user_name=email_address, + email=email_address, + is_email_verified=False, + state="active", + first_name=_validated_name(request_body.first_name), + last_name=_validated_name(request_body.last_name), + ) + ) + except httpx.HTTPStatusError as error: + if error.response.status_code == 409: + raise HTTPException( + status_code=409, + detail="email_already_registered", + ) from error + raise + if not account_id: + raise HTTPException(status_code=502, detail="account_creation_failed") + _initialize_account( + api, + account_id, + client_id=client_id, + redirect_uri=redirect_uri, + lifespan_seconds=lifespan_seconds, + ) + return RegistrationResult( + account_id=account_id, + email_address=email_address, + ) diff --git a/services/account_unification/app/scim.py b/services/account_unification/app/scim.py index 2cb7609..4c0b9fb 100644 --- a/services/account_unification/app/scim.py +++ b/services/account_unification/app/scim.py @@ -20,6 +20,11 @@ from .keycloak_client import AdminApi from .models import UserAccount +from .service import TOMBSTONE_ATTRIBUTE_KEY +from .user_locks import ( + UserOperationLocks, + UserOperationLockTimeout, +) SCIM_USER_SCHEMA = "urn:ietf:params:scim:schemas:core:2.0:User" SCIM_LIST_SCHEMA = "urn:ietf:params:scim:api:messages:2.0:ListResponse" @@ -38,6 +43,14 @@ def get_provisioner(request: Request) -> AdminApi: return api +def get_user_operation_locks(request: Request) -> UserOperationLocks: + """Return the shared lock manager used by both SCIM and merge paths.""" + locks = getattr(request.app.state, "user_operation_locks", None) + if locks is None: # pragma: no cover - only when misconfigured / test wiring + raise HTTPException(status_code=503, detail="user operation locks not wired") + return locks + + def _scim_error(status: int, detail: str) -> HTTPException: """Build a SCIM-shaped HTTP error.""" return HTTPException( @@ -186,15 +199,35 @@ def replace_user( user_id: str, resource: dict[str, Any], provisioner: AdminApi = Depends(get_provisioner), + user_operation_locks: UserOperationLocks = Depends(get_user_operation_locks), ) -> Response: """Replace a provisioned user from a SCIM PUT request.""" try: - provisioner.get_user(user_id) - except KeyError as exc: - raise _scim_error(404, f"user '{user_id}' not found") from exc - account = _to_user_account(resource, user_id=user_id) - provisioner.replace_user(user_id, account) - return _scim_response(_to_scim_resource(provisioner.get_user(user_id))) + with user_operation_locks.hold(user_id): + try: + provisioner.get_user(user_id) + except KeyError as exc: + raise _scim_error(404, f"user '{user_id}' not found") from exc + # A merged-away duplicate is tombstoned (disabled + a + # merged_into_user_id pointer) so it can never authenticate again. + # Keep the check and the full replacement PUT under the same lock + # used by merge_accounts; otherwise merge can create the tombstone + # between these two Admin API calls and SCIM can wipe it again. + if provisioner.get_user_attribute(user_id, TOMBSTONE_ATTRIBUTE_KEY): + raise _scim_error( + 409, + f"user '{user_id}' has been merged into another account " + "and cannot be modified", + ) + account = _to_user_account(resource, user_id=user_id) + provisioner.replace_user(user_id, account) + replaced = provisioner.get_user(user_id) + except UserOperationLockTimeout as exc: + raise _scim_error( + 503, + f"user '{user_id}' is being modified; retry the request", + ) from exc + return _scim_response(_to_scim_resource(replaced)) @scim_router.patch("/Users/{user_id}") diff --git a/services/account_unification/app/service.py b/services/account_unification/app/service.py index 0ecacb1..16c42d8 100644 --- a/services/account_unification/app/service.py +++ b/services/account_unification/app/service.py @@ -33,6 +33,7 @@ MergeResult, UserAccount, ) +from .user_locks import UserOperationLocks # Keycloak user attribute stamped on a tombstoned duplicate (two-word snake_case). TOMBSTONE_ATTRIBUTE_KEY = "merged_into_user_id" @@ -47,11 +48,13 @@ def __init__( api: AdminApi, audit: AuditLogger, config: ServiceConfig, + user_operation_locks: UserOperationLocks, ) -> None: - """Create a service around admin API, audit, and config dependencies.""" + """Create a service around admin, audit, config, and lock dependencies.""" self._api = api self._audit = audit self._config = config + self._user_operation_locks = user_operation_locks # -- (a) inspect identities ------------------------------------------- def get_account(self, user_id: str) -> UserAccount: @@ -71,6 +74,18 @@ def merge_accounts(self, request: MergeRequest) -> MergeResult: if request.survivor_user_id == request.duplicate_user_id: raise SameUserError("survivor and duplicate are the same account") + # The complete merge, including the final tombstone write, shares the + # same duplicate-user lock as SCIM replacement. This closes the TOCTOU + # window where SCIM could pass its tombstone check, then overwrite a + # concurrently-created tombstone with an active user representation. + with self._user_operation_locks.hold( + request.survivor_user_id, + request.duplicate_user_id, + ): + return self._merge_accounts_locked(request) + + def _merge_accounts_locked(self, request: MergeRequest) -> MergeResult: + """Perform a merge while both participating user IDs are serialized.""" survivor = self._load_user(request.survivor_user_id) duplicate = self._load_user(request.duplicate_user_id) survivor.federated_identities = self._api.list_federated_identities( @@ -191,8 +206,10 @@ def _move_federated_identities( duplicate.user_id, link.identity_provider ) self._audit.emit( - audit_id=audit_id, event_type="federated_identity_conflict", - actor=actor, survivor_user_id=survivor.user_id, + audit_id=audit_id, + event_type="federated_identity_conflict", + actor=actor, + survivor_user_id=survivor.user_id, duplicate_user_id=duplicate.user_id, payload={"identifier": identifier, "resolution": "survivor_wins"}, ) @@ -203,8 +220,11 @@ def _move_federated_identities( ) moved.append(identifier) self._audit.emit( - audit_id=audit_id, event_type="federated_identity_moved", actor=actor, - survivor_user_id=survivor.user_id, duplicate_user_id=duplicate.user_id, + audit_id=audit_id, + event_type="federated_identity_moved", + actor=actor, + survivor_user_id=survivor.user_id, + duplicate_user_id=duplicate.user_id, payload={"identifier": identifier}, ) return moved @@ -229,7 +249,9 @@ def _move_role_mappings( # survivor-wins: survivor already has it; drop the duplicate's. self._api.remove_role_mapping(duplicate.user_id, role) self._audit.emit( - audit_id=audit_id, event_type="role_mapping_conflict", actor=actor, + audit_id=audit_id, + event_type="role_mapping_conflict", + actor=actor, survivor_user_id=survivor.user_id, duplicate_user_id=duplicate.user_id, payload={"identifier": identifier, "resolution": "survivor_wins"}, @@ -239,10 +261,16 @@ def _move_role_mappings( self._api.remove_role_mapping(duplicate.user_id, role) moved.append(identifier) self._audit.emit( - audit_id=audit_id, event_type="role_mapping_moved", actor=actor, - survivor_user_id=survivor.user_id, duplicate_user_id=duplicate.user_id, - payload={"identifier": identifier, "role_name": role.role_name, - "client_id": role.client_id}, + audit_id=audit_id, + event_type="role_mapping_moved", + actor=actor, + survivor_user_id=survivor.user_id, + duplicate_user_id=duplicate.user_id, + payload={ + "identifier": identifier, + "role_name": role.role_name, + "client_id": role.client_id, + }, ) return moved @@ -262,8 +290,10 @@ def _move_group_memberships( ) self._api.remove_group_membership(duplicate.user_id, group) self._audit.emit( - audit_id=audit_id, event_type="group_membership_conflict", - actor=actor, survivor_user_id=survivor.user_id, + audit_id=audit_id, + event_type="group_membership_conflict", + actor=actor, + survivor_user_id=survivor.user_id, duplicate_user_id=duplicate.user_id, payload={"identifier": identifier, "resolution": "survivor_wins"}, ) @@ -272,8 +302,11 @@ def _move_group_memberships( self._api.remove_group_membership(duplicate.user_id, group) moved.append(identifier) self._audit.emit( - audit_id=audit_id, event_type="group_membership_moved", actor=actor, - survivor_user_id=survivor.user_id, duplicate_user_id=duplicate.user_id, + audit_id=audit_id, + event_type="group_membership_moved", + actor=actor, + survivor_user_id=survivor.user_id, + duplicate_user_id=duplicate.user_id, payload={"identifier": identifier}, ) return moved @@ -287,8 +320,11 @@ def _tombstone(self, duplicate, survivor, audit_id, actor) -> None: ) self._api.deactivate_user(duplicate.user_id) self._audit.emit( - audit_id=audit_id, event_type="duplicate_tombstoned", actor=actor, - survivor_user_id=survivor.user_id, duplicate_user_id=duplicate.user_id, + audit_id=audit_id, + event_type="duplicate_tombstoned", + actor=actor, + survivor_user_id=survivor.user_id, + duplicate_user_id=duplicate.user_id, payload={"attribute_key": TOMBSTONE_ATTRIBUTE_KEY}, ) diff --git a/services/account_unification/app/user_locks.py b/services/account_unification/app/user_locks.py new file mode 100644 index 0000000..0e940aa --- /dev/null +++ b/services/account_unification/app/user_locks.py @@ -0,0 +1,137 @@ +"""Cross-path serialization for mutations of Keycloak user records. + +SCIM replacement and account merge both write complete or partial Keycloak user +representations. They must share one lock boundary so a merge cannot tombstone a +duplicate between SCIM's tombstone check and its replacement PUT. + +The standalone runtime uses :class:`SqliteUserOperationLocks`, backed by a +dedicated sidecar SQLite database. ``BEGIN IMMEDIATE`` provides a crash-safe, +cross-process mutex for every service worker sharing that database file. The +current implementation intentionally serializes all user mutations rather than +risking a multi-key deadlock; the public interface remains user-ID keyed so a +future Postgres advisory-lock implementation can safely increase concurrency. +""" +from __future__ import annotations + +import sqlite3 +import threading +from contextlib import contextmanager +from typing import ContextManager, Iterator, Protocol + + +class UserOperationLockTimeout(RuntimeError): + """Raised when a shared user-operation lock cannot be acquired in time.""" + + +class UserOperationLocks(Protocol): + """Serialize mutations that involve one or more Keycloak user IDs.""" + + def hold(self, *user_ids: str) -> ContextManager[None]: + """Return a context manager holding the requested user-operation locks.""" + ... + + +def _normalise_user_ids(user_ids: tuple[str, ...]) -> tuple[str, ...]: + """Return unique, non-empty user IDs in deterministic acquisition order.""" + ordered = tuple(sorted(set(user_ids))) + if not ordered or any(not user_id for user_id in ordered): + raise ValueError("at least one non-empty user ID is required") + return ordered + + +class InMemoryUserOperationLocks: + """Process-local keyed lock manager for tests and explicit single-worker use.""" + + def __init__(self) -> None: + """Create an empty keyed re-entrant lock registry.""" + self._registry_guard = threading.Lock() + self._locks: dict[str, threading.RLock] = {} + + @contextmanager + def hold(self, *user_ids: str) -> Iterator[None]: + """Hold all requested user locks in stable order to avoid deadlocks.""" + ordered_ids = _normalise_user_ids(user_ids) + with self._registry_guard: + locks = [ + self._locks.setdefault(user_id, threading.RLock()) + for user_id in ordered_ids + ] + for lock in locks: + lock.acquire() + try: + yield + finally: + for lock in reversed(locks): + lock.release() + + +class SqliteUserOperationLocks: + """Cross-process mutex backed by a dedicated SQLite sidecar database. + + SQLite permits only one writer holding a ``BEGIN IMMEDIATE`` transaction. + Every manager instance pointed at the same database file therefore shares a + crash-safe mutex: process termination closes the connection and releases the + lock automatically. This is deliberately coarser than the user-ID-keyed + protocol, but it fully serializes the SCIM and merge critical sections for + the supported SQLite deployment without introducing a lease-expiry race. + """ + + _SCHEMA = """ + CREATE TABLE IF NOT EXISTS user_operation_lock_state ( + lock_name TEXT PRIMARY KEY, + requested_user_ids TEXT NOT NULL + ); + """ + + def __init__(self, database_path: str, *, timeout_seconds: float = 10.0) -> None: + """Create a manager using ``database_path`` and an acquisition timeout.""" + if not database_path: + raise ValueError("database_path is required") + if timeout_seconds <= 0: + raise ValueError("timeout_seconds must be positive") + self._database_path = database_path + self._timeout_seconds = timeout_seconds + self._initialize() + + def _connect(self) -> sqlite3.Connection: + """Open one autocommit connection configured with the lock timeout.""" + return sqlite3.connect( + self._database_path, + timeout=self._timeout_seconds, + isolation_level=None, + ) + + def _initialize(self) -> None: + """Create the sidecar schema before requests begin competing for it.""" + connection = self._connect() + try: + connection.execute(self._SCHEMA) + finally: + connection.close() + + @contextmanager + def hold(self, *user_ids: str) -> Iterator[None]: + """Hold the shared SQLite mutex for the complete user mutation.""" + ordered_ids = _normalise_user_ids(user_ids) + connection = self._connect() + try: + try: + connection.execute("BEGIN IMMEDIATE") + except sqlite3.OperationalError as exc: + if "locked" in str(exc).lower(): + raise UserOperationLockTimeout( + "timed out waiting for another user mutation to finish" + ) from exc + raise + connection.execute( + "INSERT INTO user_operation_lock_state " + "(lock_name, requested_user_ids) VALUES ('global', ?) " + "ON CONFLICT(lock_name) DO UPDATE SET " + "requested_user_ids = excluded.requested_user_ids", + (",".join(ordered_ids),), + ) + yield + finally: + if connection.in_transaction: + connection.rollback() + connection.close() diff --git a/services/account_unification/tests/conftest.py b/services/account_unification/tests/conftest.py index 3deba76..9fe8241 100644 --- a/services/account_unification/tests/conftest.py +++ b/services/account_unification/tests/conftest.py @@ -1,4 +1,4 @@ -"""Shared pytest fixtures.""" +"""Shared pytest fixtures for authenticated, serialized service tests.""" from __future__ import annotations import sys @@ -11,39 +11,76 @@ from app.audit import AuditLogger, InMemoryAuditSink # noqa: E402 from app.config import ServiceConfig # noqa: E402 from app.service import UnificationService # noqa: E402 +from app.user_locks import InMemoryUserOperationLocks # noqa: E402 -from .mock_keycloak import MockKeycloakAdminApi # noqa: E402 +from .mock_product_keycloak import ( # noqa: E402 + MockProductKeycloakAdminApi, +) + +OPERATOR_TOKEN = "test-operator-token" @pytest.fixture -def api() -> MockKeycloakAdminApi: - return MockKeycloakAdminApi() +def api() -> MockProductKeycloakAdminApi: + """Return a fresh product-capable Keycloak test double.""" + return MockProductKeycloakAdminApi() @pytest.fixture def audit_sink() -> InMemoryAuditSink: + """Return a fresh in-memory audit sink.""" return InMemoryAuditSink() @pytest.fixture def audit(audit_sink: InMemoryAuditSink) -> AuditLogger: + """Return an audit logger around the in-memory sink.""" return AuditLogger(audit_sink) +@pytest.fixture +def operator_token() -> str: + """Return the shared operator token used by HTTP tests.""" + return OPERATOR_TOKEN + + +@pytest.fixture +def auth_header(operator_token: str) -> dict[str, str]: + """Return an authenticated operator bearer header.""" + return {"Authorization": f"Bearer {operator_token}"} + + @pytest.fixture def config() -> ServiceConfig: + """Return deterministic account-unification service configuration.""" return ServiceConfig( keycloak_server_url="http://keycloak.test", keycloak_realm="cwl", keycloak_client_id="account-unification-svc", keycloak_client_secret="test-secret", + operator_api_token=OPERATOR_TOKEN, merge_conflict_policy="survivor_wins", allow_unverified_email_link=False, ) +@pytest.fixture +def user_operation_locks() -> InMemoryUserOperationLocks: + """Return a process-local keyed lock manager.""" + return InMemoryUserOperationLocks() + + @pytest.fixture def service( - api: MockKeycloakAdminApi, audit: AuditLogger, config: ServiceConfig + api: MockProductKeycloakAdminApi, + audit: AuditLogger, + config: ServiceConfig, + user_operation_locks: InMemoryUserOperationLocks, ) -> UnificationService: - return UnificationService(api, audit, config) + """Return a fully wired unification service.""" + return UnificationService( + api, + audit, + config, + user_operation_locks, + ) diff --git a/services/account_unification/tests/mock_keycloak.py b/services/account_unification/tests/mock_keycloak.py index f844391..26f9c38 100644 --- a/services/account_unification/tests/mock_keycloak.py +++ b/services/account_unification/tests/mock_keycloak.py @@ -148,3 +148,7 @@ def set_user_attribute(self, user_id: str, key: str, value: str) -> None: self.users[user_id] = self.users[user_id].model_copy( update={"external_id": value} ) + + def get_user_attribute(self, user_id: str, key: str) -> str | None: + self.calls.append(f"get_user_attribute:{user_id}:{key}") + return self.attributes.get((user_id, key)) diff --git a/services/account_unification/tests/mock_product_keycloak.py b/services/account_unification/tests/mock_product_keycloak.py new file mode 100644 index 0000000..2557400 --- /dev/null +++ b/services/account_unification/tests/mock_product_keycloak.py @@ -0,0 +1,74 @@ +"""Product-capable in-memory Keycloak Admin API test double.""" +from __future__ import annotations + +from .mock_keycloak import MockKeycloakAdminApi + + +class MockProductKeycloakAdminApi(MockKeycloakAdminApi): + """Extend the core mock with registration and federation operations.""" + + def __init__(self) -> None: + """Create empty product-specific stores.""" + super().__init__() + self.identity_providers: dict[str, dict] = {} + self.action_emails: dict[str, dict] = {} + + def send_execute_actions_email( + self, + user_id: str, + action_aliases: list[str], + *, + client_id: str, + redirect_uri: str, + lifespan_seconds: int, + ) -> None: + """Record one verification and passkey-enrollment email request.""" + self.calls.append(f"send_execute_actions_email:{user_id}") + if user_id not in self.users: + raise KeyError(user_id) + self.action_emails[user_id] = { + "action_aliases": list(action_aliases), + "client_id": client_id, + "redirect_uri": redirect_uri, + "lifespan_seconds": lifespan_seconds, + } + + def delete_user(self, user_id: str) -> None: + """Delete a newly created account during rollback.""" + self.calls.append(f"delete_user:{user_id}") + self.users.pop(user_id, None) + self.federated.pop(user_id, None) + self.roles.pop(user_id, None) + self.groups.pop(user_id, None) + self.action_emails.pop(user_id, None) + self.deactivated.discard(user_id) + for attribute in [ + key for key in self.attributes if key[0] == user_id + ]: + self.attributes.pop(attribute, None) + + def get_identity_provider( + self, provider_alias: str + ) -> dict | None: + """Return a defensive copy of one applied provider.""" + self.calls.append(f"get_identity_provider:{provider_alias}") + provider = self.identity_providers.get(provider_alias) + return dict(provider) if provider is not None else None + + def create_identity_provider(self, provider_payload: dict) -> None: + """Create one applied identity provider.""" + alias = provider_payload["alias"] + self.calls.append(f"create_identity_provider:{alias}") + self.identity_providers[alias] = dict(provider_payload) + + def update_identity_provider( + self, provider_alias: str, provider_payload: dict + ) -> None: + """Replace one applied identity provider.""" + self.calls.append(f"update_identity_provider:{provider_alias}") + self.identity_providers[provider_alias] = dict(provider_payload) + + def delete_identity_provider(self, provider_alias: str) -> None: + """Delete one applied identity provider.""" + self.calls.append(f"delete_identity_provider:{provider_alias}") + self.identity_providers.pop(provider_alias, None) diff --git a/services/account_unification/tests/test_api.py b/services/account_unification/tests/test_api.py index 51c5693..cc5f9c8 100644 --- a/services/account_unification/tests/test_api.py +++ b/services/account_unification/tests/test_api.py @@ -1,4 +1,4 @@ -"""HTTP surface: /healthz, identity listing, merge, and audit retrieval.""" +"""Authenticated HTTP surface for identity inspection, merge, and audit.""" from __future__ import annotations import pytest @@ -6,75 +6,139 @@ from app.main import create_app from app.models import FederatedIdentity, RoleMapping +from app.service import UnificationService @pytest.fixture -def client(api, audit, config): - from app.service import UnificationService - +def client( + api, + audit, + config, + auth_header, + user_operation_locks, +): + """Return an authenticated app with all merge dependencies wired.""" app = create_app(wire=False) - app.state.unification_service = UnificationService(api, audit, config) + app.state.unification_service = UnificationService( + api, + audit, + config, + user_operation_locks, + ) app.state.audit_logger = audit app.state.keycloak_api = api - with TestClient(app) as test_client: + app.state.user_operation_locks = user_operation_locks + app.state.operator_api_token = config.operator_api_token + with TestClient(app, headers=auth_header) as test_client: yield test_client def test_healthz_ok(client): + """Health probes remain open and report readiness.""" response = client.get("/healthz") assert response.status_code == 200 assert response.json()["status"] == "ok" def test_list_identities_endpoint(client, api): + """The authenticated identity endpoint returns external links.""" api.create_test_user( "u1", federated_identities=[ - FederatedIdentity(identity_provider="employer-adfs", external_user_id="jane@corp") + FederatedIdentity( + identity_provider="employer-adfs", + external_user_id="jane@corp", + ) ], ) response = client.get("/users/u1/identities") assert response.status_code == 200 - assert response.json()[0]["identity_provider"] == "employer-adfs" + assert ( + response.json()[0]["identity_provider"] + == "employer-adfs" + ) def test_merge_endpoint_and_audit(client, api): + """A valid merge produces a retrievable append-only audit trail.""" api.create_test_user( - "survivor", email="j@x.com", is_email_verified=True, - role_mappings=[RoleMapping(role_id="r-s", role_name="admin", client_id="naruon")], + "survivor", + email="j@x.com", + is_email_verified=True, + role_mappings=[ + RoleMapping( + role_id="r-s", + role_name="admin", + client_id="naruon", + ) + ], ) api.create_test_user( - "dup", email="j@x.com", is_email_verified=True, + "dup", + email="j@x.com", + is_email_verified=True, federated_identities=[ - FederatedIdentity(identity_provider="google", external_user_id="j@gmail") + FederatedIdentity( + identity_provider="google", + external_user_id="j@gmail", + ) ], ) response = client.post( "/merges", - json={"survivor_user_id": "survivor", "duplicate_user_id": "dup", "actor": "admin"}, + json={ + "survivor_user_id": "survivor", + "duplicate_user_id": "dup", + "actor": "admin", + }, ) assert response.status_code == 200 audit_id = response.json()["audit_id"] audit_response = client.get(f"/merges/{audit_id}/audit") assert audit_response.status_code == 200 - assert any(e["event_type"] == "merge_completed" for e in audit_response.json()) + assert any( + event["event_type"] == "merge_completed" + for event in audit_response.json() + ) def test_merge_endpoint_refuses_unverified_email(client, api): - api.create_test_user("survivor", email="j@x.com", is_email_verified=True) - api.create_test_user("dup", email="j@x.com", is_email_verified=False) + """Unverified-email coincidence never authorizes a merge.""" + api.create_test_user( + "survivor", + email="j@x.com", + is_email_verified=True, + ) + api.create_test_user( + "dup", + email="j@x.com", + is_email_verified=False, + ) response = client.post( "/merges", - json={"survivor_user_id": "survivor", "duplicate_user_id": "dup", "actor": "admin"}, + json={ + "survivor_user_id": "survivor", + "duplicate_user_id": "dup", + "actor": "admin", + }, ) assert response.status_code == 422 def test_merge_endpoint_missing_user_404(client, api): - api.create_test_user("survivor", email="j@x.com", is_email_verified=True) + """Unknown accounts produce an HTTP 404 without partial writes.""" + api.create_test_user( + "survivor", + email="j@x.com", + is_email_verified=True, + ) response = client.post( "/merges", - json={"survivor_user_id": "survivor", "duplicate_user_id": "ghost", "actor": "admin"}, + json={ + "survivor_user_id": "survivor", + "duplicate_user_id": "ghost", + "actor": "admin", + }, ) assert response.status_code == 404 diff --git a/services/account_unification/tests/test_audit.py b/services/account_unification/tests/test_audit.py index a12de6d..e0759cd 100644 --- a/services/account_unification/tests/test_audit.py +++ b/services/account_unification/tests/test_audit.py @@ -1,76 +1,133 @@ -"""Merge operations are fully audit-logged (in-memory and SQLite sinks).""" +"""Merge operations remain fully audit-logged in memory and SQLite.""" from __future__ import annotations from contextlib import closing -from app.audit import AuditLogger, AuditSink, InMemoryAuditSink, SqliteAuditSink +from app.audit import ( + AuditLogger, + AuditSink, + InMemoryAuditSink, + SqliteAuditSink, +) from app.config import ServiceConfig -from app.models import FederatedIdentity, MergeRequest, RoleMapping +from app.models import ( + FederatedIdentity, + MergeRequest, + RoleMapping, +) from app.service import UnificationService +from app.user_locks import InMemoryUserOperationLocks from .mock_keycloak import MockKeycloakAdminApi def test_audit_sink_protocol_methods_have_concrete_implementations(): + """Every sink implements the complete persistence protocol.""" protocol_methods = { name for name, member in AuditSink.__dict__.items() if callable(member) and not name.startswith("_") } assert protocol_methods - for implementation in (InMemoryAuditSink, SqliteAuditSink): + for implementation in ( + InMemoryAuditSink, + SqliteAuditSink, + ): missing = [ name for name in sorted(protocol_methods) - if not callable(getattr(implementation, name, None)) + if not callable( + getattr(implementation, name, None) + ) ] assert missing == [] def _seed_mergeable(api): + """Create one verified merge pair with transferable state.""" api.create_test_user( - "survivor", email="j@x.com", is_email_verified=True, - role_mappings=[RoleMapping(role_id="r-s", role_name="admin", client_id="naruon")], + "survivor", + email="j@x.com", + is_email_verified=True, + role_mappings=[ + RoleMapping( + role_id="r-s", + role_name="admin", + client_id="naruon", + ) + ], ) api.create_test_user( - "dup", email="j@x.com", is_email_verified=True, + "dup", + email="j@x.com", + is_email_verified=True, federated_identities=[ - FederatedIdentity(identity_provider="google", external_user_id="j@gmail") + FederatedIdentity( + identity_provider="google", + external_user_id="j@gmail", + ) + ], + role_mappings=[ + RoleMapping( + role_id="r-d", + role_name="editor", + client_id="clearfolio", + ) ], - role_mappings=[RoleMapping(role_id="r-d", role_name="editor", client_id="clearfolio")], ) -def test_audit_trail_records_full_merge(service, api, audit_sink): +def test_audit_trail_records_full_merge( + service, api, audit_sink +): + """Every merge phase shares one actor and correlation id.""" _seed_mergeable(api) result = service.merge_accounts( - MergeRequest(survivor_user_id="survivor", duplicate_user_id="dup", actor="admin@cwl") + MergeRequest( + survivor_user_id="survivor", + duplicate_user_id="dup", + actor="admin@cwl", + ) ) events = audit_sink.events_for(result.audit_id) - event_types = [e.event_type for e in events] + event_types = [ + event.event_type for event in events + ] assert event_types[0] == "merge_started" assert event_types[-1] == "merge_completed" assert "federated_identity_moved" in event_types assert "role_mapping_moved" in event_types assert "duplicate_tombstoned" in event_types - # every event carries the same actor + correlation id. - assert {e.actor for e in events} == {"admin@cwl"} - assert {e.audit_id for e in events} == {result.audit_id} + assert { + event.actor for event in events + } == {"admin@cwl"} + assert { + event.audit_id for event in events + } == {result.audit_id} def test_sqlite_audit_sink_persists(tmp_path): + """A fresh SQLite sink can read a completed merge trail.""" api = MockKeycloakAdminApi() _seed_mergeable(api) - db = tmp_path / "audit.db" - with closing(SqliteAuditSink(str(db))) as sink: + database_path = tmp_path / "audit_events.db" + with closing( + SqliteAuditSink(str(database_path)) + ) as sink: audit = AuditLogger(sink) config = ServiceConfig( keycloak_server_url="http://kc", keycloak_realm="cwl", keycloak_client_id="svc", keycloak_client_secret="secret", + operator_api_token="op-token", + ) + service = UnificationService( + api, + audit, + config, + InMemoryUserOperationLocks(), ) - service = UnificationService(api, audit, config) result = service.merge_accounts( MergeRequest( survivor_user_id="survivor", @@ -78,7 +135,13 @@ def test_sqlite_audit_sink_persists(tmp_path): actor="admin@cwl", ) ) - # re-open a fresh sink over the same DB file: events are durable. - with closing(SqliteAuditSink(str(db))) as reopened: - events = reopened.events_for(result.audit_id) - assert any(e.event_type == "merge_completed" for e in events) + with closing( + SqliteAuditSink(str(database_path)) + ) as reopened: + events = reopened.events_for( + result.audit_id + ) + assert any( + event.event_type == "merge_completed" + for event in events + ) diff --git a/services/account_unification/tests/test_auth.py b/services/account_unification/tests/test_auth.py new file mode 100644 index 0000000..d60a93a --- /dev/null +++ b/services/account_unification/tests/test_auth.py @@ -0,0 +1,139 @@ +"""Operator bearer authentication gates every privileged API surface.""" +from __future__ import annotations + +from fastapi.testclient import TestClient + +from app.federation import FederationService +from app.kv_store import InMemoryKvStore +from app.main import create_app +from app.service import UnificationService + + +def _wired_app( + api, + audit, + config, + user_operation_locks, +): + """Return an app with privileged service dependencies wired.""" + app = create_app(wire=False) + app.state.unification_service = UnificationService( + api, + audit, + config, + user_operation_locks, + ) + app.state.audit_logger = audit + app.state.keycloak_api = api + app.state.user_operation_locks = user_operation_locks + app.state.federation_service = FederationService( + InMemoryKvStore(), api + ) + app.state.operator_api_token = config.operator_api_token + return app + + +def test_healthz_is_open_without_a_token( + api, audit, config, user_operation_locks +): + """Health probes remain available without operator credentials.""" + client = TestClient( + _wired_app( + api, + audit, + config, + user_operation_locks, + ) + ) + response = client.get("/healthz") + assert response.status_code == 200 + + +def test_privileged_routes_reject_missing_token( + api, audit, config, user_operation_locks +): + """Every privileged router rejects an absent bearer token.""" + client = TestClient( + _wired_app( + api, + audit, + config, + user_operation_locks, + ) + ) + assert client.get("/users/u1").status_code == 401 + assert client.post( + "/merges", json={} + ).status_code == 401 + assert client.get( + "/federation/identity-providers" + ).status_code == 401 + assert client.post( + "/scim/v2/Users", + json={"userName": "x"}, + ).status_code == 401 + + +def test_privileged_routes_reject_wrong_token( + api, audit, config, user_operation_locks +): + """A mismatched bearer token produces HTTP 403.""" + client = TestClient( + _wired_app( + api, + audit, + config, + user_operation_locks, + ), + headers={ + "Authorization": "Bearer not-the-token" + }, + ) + assert client.get( + "/federation/identity-providers" + ).status_code == 403 + + +def test_privileged_routes_accept_valid_token( + api, + audit, + config, + auth_header, + user_operation_locks, +): + """A valid operator token opens the privileged router.""" + client = TestClient( + _wired_app( + api, + audit, + config, + user_operation_locks, + ), + headers=auth_header, + ) + assert client.get( + "/federation/identity-providers" + ).status_code == 200 + + +def test_service_without_configured_token_fails_closed( + api, audit, config, user_operation_locks +): + """Missing token configuration makes the surface unavailable.""" + app = create_app(wire=False) + app.state.unification_service = UnificationService( + api, + audit, + config, + user_operation_locks, + ) + app.state.audit_logger = audit + app.state.keycloak_api = api + app.state.user_operation_locks = user_operation_locks + client = TestClient( + app, + headers={ + "Authorization": "Bearer anything" + }, + ) + assert client.get("/users/u1").status_code == 503 diff --git a/services/account_unification/tests/test_config.py b/services/account_unification/tests/test_config.py index 2620fd4..0f71c21 100644 --- a/services/account_unification/tests/test_config.py +++ b/services/account_unification/tests/test_config.py @@ -15,7 +15,8 @@ from app.kv_store import InMemoryKvStore, KvStore, SqliteKvStore -def test_kv_store_protocol_methods_have_concrete_implementations(): +def test_kv_store_protocol_methods_have_concrete_implementations() -> None: + """Every config-store adapter implements the complete public protocol.""" protocol_methods = { name for name, member in KvStore.__dict__.items() @@ -31,38 +32,151 @@ def test_kv_store_protocol_methods_have_concrete_implementations(): assert missing == [] -def test_config_loads_from_kv(): - store = InMemoryKvStore( - { - "account_unification": { - "keycloak_server_url": "http://kc", - "keycloak_realm": "cwl", - "keycloak_client_id": "svc", - "keycloak_client_secret": "secret", - } - } - ) - config = load_service_config(store, "account_unification") +def _config_store(**overrides: str) -> InMemoryKvStore: + """Build one complete config namespace with selected overrides.""" + entries = { + "keycloak_server_url": "http://kc", + "keycloak_realm": "cwl", + "keycloak_client_id": "svc", + "keycloak_client_secret": "secret", + "operator_api_token": "operator-token", + } + entries.update(overrides) + return InMemoryKvStore({"account_unification": entries}) + + +def test_config_loads_from_kv() -> None: + """Required values load and security invariants retain safe defaults.""" + config = load_service_config(_config_store(), "account_unification") assert config.keycloak_server_url == "http://kc" assert config.keycloak_realm == "cwl" - # policy default: unverified linking OFF. assert config.allow_unverified_email_link is False assert config.merge_conflict_policy == "survivor_wins" + assert config.registration_api_token is None -def test_missing_required_config_fails_loudly(): +def test_missing_required_config_fails_loudly() -> None: + """Startup fails when any foundational Keycloak setting is absent.""" store = InMemoryKvStore({"account_unification": {}}) with pytest.raises(RuntimeError): load_service_config(store, "account_unification") -def test_bootstrap_points_at_sqlite_store(tmp_path): +@pytest.mark.parametrize( + "raw_value", + ["0", "-1", "nan", "inf", "-inf", "not-a-number"], +) +def test_request_timeout_must_be_positive_and_finite(raw_value: str) -> None: + """Nonpositive or nonfinite request timeouts fail startup.""" + store = _config_store(request_timeout_seconds=raw_value) + with pytest.raises(RuntimeError, match="request_timeout_seconds"): + load_service_config(store, "account_unification") + + +def test_registration_token_must_not_equal_operator_token() -> None: + """Product signup credentials cannot acquire operator authority.""" + store = _config_store(registration_api_token="operator-token") + with pytest.raises(RuntimeError, match="registration_api_token"): + load_service_config(store, "account_unification") + + +def test_registration_requires_complete_action_email_config() -> None: + """Enabling signup requires an RP, redirect URI, and action-link lifespan.""" + store = _config_store(registration_api_token="registration-token") + with pytest.raises(RuntimeError, match="registration_client_id"): + load_service_config(store, "account_unification") + + +def test_registration_action_email_config_loads() -> None: + """A complete passwordless enrollment configuration loads atomically.""" + store = _config_store( + registration_api_token="registration-token", + registration_client_id="naruon-web", + registration_redirect_uri="https://naruon.example/auth/passkey-complete", + registration_action_lifespan_seconds="900", + ) + + config = load_service_config(store, "account_unification") + + assert config.registration_client_id == "naruon-web" + assert config.registration_redirect_uri == ( + "https://naruon.example/auth/passkey-complete" + ) + assert config.registration_action_lifespan_seconds == 900 + + +@pytest.mark.parametrize( + "redirect_uri", + [ + "http://naruon.example/callback", + "javascript:alert(1)", + "//naruon.example/callback", + "https:///missing-host", + ], +) +def test_registration_redirect_uri_requires_absolute_https( + redirect_uri: str, +) -> None: + """Action emails cannot redirect to non-HTTPS or hostless locations.""" + store = _config_store( + registration_api_token="registration-token", + registration_client_id="naruon-web", + registration_redirect_uri=redirect_uri, + registration_action_lifespan_seconds="900", + ) + with pytest.raises(RuntimeError, match="registration_redirect_uri"): + load_service_config(store, "account_unification") + + +@pytest.mark.parametrize( + "raw_value", + ["0", "-1", "1.5", "nan", "inf", "not-a-number"], +) +def test_registration_action_lifespan_must_be_positive_integer( + raw_value: str, +) -> None: + """Keycloak action-email lifespan must be a bounded integer duration.""" + store = _config_store( + registration_api_token="registration-token", + registration_client_id="naruon-web", + registration_redirect_uri="https://naruon.example/auth/passkey-complete", + registration_action_lifespan_seconds=raw_value, + ) + with pytest.raises(RuntimeError, match="registration_action_lifespan_seconds"): + load_service_config(store, "account_unification") + + +@pytest.mark.parametrize("raw_value", ["true", "1", "yes", "on"]) +def test_unverified_email_link_policy_cannot_be_enabled(raw_value: str) -> None: + """An unverified-email link policy is rejected even when explicitly set.""" + store = _config_store(allow_unverified_email_link=raw_value) + with pytest.raises(RuntimeError, match="allow_unverified_email_link"): + load_service_config(store, "account_unification") + + +def test_invalid_boolean_text_fails_loudly() -> None: + """Ambiguous boolean configuration cannot silently become false.""" + store = _config_store(allow_unverified_email_link="definitely") + with pytest.raises(RuntimeError, match="allow_unverified_email_link"): + load_service_config(store, "account_unification") + + +def test_only_implemented_merge_conflict_policy_is_accepted() -> None: + """Unknown conflict policies cannot claim behavior the service lacks.""" + store = _config_store(merge_conflict_policy="duplicate_wins") + with pytest.raises(RuntimeError, match="merge_conflict_policy"): + load_service_config(store, "account_unification") + + +def test_bootstrap_points_at_sqlite_store(tmp_path) -> None: + """The bootstrap descriptor opens the configured SQLite namespace.""" db = tmp_path / "store.db" with closing(SqliteKvStore(str(db))) as seed: seed.put("account_unification", "keycloak_server_url", "http://kc") seed.put("account_unification", "keycloak_realm", "cwl") seed.put("account_unification", "keycloak_client_id", "svc") seed.put("account_unification", "keycloak_client_secret", "secret") + seed.put("account_unification", "operator_api_token", "op-token") bootstrap = tmp_path / "bootstrap.yaml" bootstrap.write_text( @@ -79,7 +193,8 @@ def test_bootstrap_points_at_sqlite_store(tmp_path): assert config.keycloak_realm == "cwl" -def test_unsupported_standalone_backend_fails_loudly(): +def test_unsupported_standalone_backend_fails_loudly() -> None: + """A deployment cannot select an adapter absent from its image.""" descriptor = BootstrapDescriptor( backend="postgres", namespace="account_unification", diff --git a/services/account_unification/tests/test_deployment_contracts.py b/services/account_unification/tests/test_deployment_contracts.py new file mode 100644 index 0000000..b346d29 --- /dev/null +++ b/services/account_unification/tests/test_deployment_contracts.py @@ -0,0 +1,104 @@ +"""Static deployment contract tests for Compose and Helm packaging.""" +from __future__ import annotations + +import tomllib +from pathlib import Path + +import yaml + + +def _repository_root() -> Path: + """Return the repository root from the account-unification tests.""" + return Path(__file__).resolve().parents[3] + + +def _helm_values() -> dict: + """Return parsed Helm values for the cwl-idp chart.""" + return yaml.safe_load( + (_repository_root() / "helm" / "cwl-idp" / "values.yaml").read_text( + encoding="utf-8" + ) + ) + + +def _seed_tool_source() -> str: + """Return the local configuration seed tool source.""" + return ( + _repository_root() + / "services" + / "account_unification" + / "tools" + / "seed_config_store.py" + ).read_text(encoding="utf-8") + + +def test_compose_persists_account_unification_state() -> None: + """Standalone restarts retain audit and user-operation lock databases.""" + compose = yaml.safe_load( + (_repository_root() / "docker-compose.yml").read_text(encoding="utf-8") + ) + service = compose["services"]["account_unification_service"] + assert ( + "account_unification_data:/var/lib/account-unification" + in service["volumes"] + ) + assert "account_unification_data" in compose["volumes"] + + +def test_helm_can_fail_closed_on_missing_account_image_digest() -> None: + """Production values can require an immutable account-service image.""" + image = _helm_values()["accountUnification"]["image"] + assert image["requireDigest"] is False + template = ( + _repository_root() + / "helm" + / "cwl-idp" + / "templates" + / "account-unification.yaml" + ).read_text(encoding="utf-8") + assert "accountUnification.image.requireDigest" in template + assert "accountUnification.image.digest is required" in template + + +def test_helm_mounts_durable_account_unification_storage() -> None: + """The chart mounts deployment-owned state at the service data path.""" + persistence = _helm_values()["accountUnification"]["persistence"] + assert persistence["enabled"] is True + assert persistence["size"] + template = ( + _repository_root() + / "helm" + / "cwl-idp" + / "templates" + / "account-unification.yaml" + ).read_text(encoding="utf-8") + assert "kind: PersistentVolumeClaim" in template + assert "mountPath: /var/lib/account-unification" in template + + +def test_helm_image_tag_matches_package_version() -> None: + """Unreleased chart metadata cannot advertise an unbuilt package version.""" + pyproject_path = ( + _repository_root() + / "services" + / "account_unification" + / "pyproject.toml" + ) + project = tomllib.loads(pyproject_path.read_text(encoding="utf-8"))["project"] + helm_tag = _helm_values()["accountUnification"]["image"]["tag"] + assert helm_tag == project["version"] + + +def test_local_seed_avoids_global_temporary_audit_storage() -> None: + """Development defaults stay inside the project bootstrap directory.""" + seed_tool = _seed_tool_source() + assert "/tmp/keyverse-account-audit.sqlite3" not in seed_tool + assert "account_unification_audit.sqlite3" in seed_tool + + +def test_local_seed_keeps_registration_disabled_by_default() -> None: + """A developer must explicitly supply the dedicated signup credential.""" + seed_tool = _seed_tool_source() + assert '"--registration-token"' in seed_tool + assert 'default=""' in seed_tool + assert "if not args.registration_token" in seed_tool diff --git a/services/account_unification/tests/test_federation.py b/services/account_unification/tests/test_federation.py new file mode 100644 index 0000000..aa36c14 --- /dev/null +++ b/services/account_unification/tests/test_federation.py @@ -0,0 +1,284 @@ +"""Runtime federation desired-state, convergence, and redaction tests.""" +from __future__ import annotations + +import json +import threading +from concurrent.futures import ThreadPoolExecutor + +import pytest +from fastapi import HTTPException +from fastapi.testclient import TestClient + +from app.federation import ( + FEDERATION_PROVIDER_NAMESPACE, + FederationService, + IdentityProviderRegistration, +) +from app.kv_store import InMemoryKvStore +from app.main import create_app + + +def _employer_adfs_registration() -> IdentityProviderRegistration: + """Return an employer SAML provider expressed as runtime data.""" + return IdentityProviderRegistration( + provider_alias="employer-adfs", + display_name="Employer ADFS", + provider_id="saml", + enabled=True, + trust_email=True, + provider_config={ + "entityId": "https://idp.example/realms/cwl", + "singleSignOnServiceUrl": "https://sts.example/adfs/ls/", + "clientSecret": "federation-secret", + "unclassifiedValue": "must-not-leak", + "validateSignature": "true", + }, + ) + + +@pytest.fixture +def store() -> InMemoryKvStore: + """Return a fresh desired-state store.""" + return InMemoryKvStore() + + +@pytest.fixture +def federation(store, api) -> FederationService: + """Return a federation service with product-capable Keycloak mock.""" + return FederationService(store, api) + + +def test_put_persists_secret_but_redacts_status(federation, store, api) -> None: + """Secrets reach storage and Keycloak but never the status view.""" + registration = _employer_adfs_registration() + + status = federation.put_registration("employer-adfs", registration) + + raw = store.get(FEDERATION_PROVIDER_NAMESPACE, "employer-adfs") + assert raw is not None + stored_config = json.loads(raw)["provider_config"] + assert stored_config["clientSecret"] == "federation-secret" + assert stored_config["unclassifiedValue"] == "must-not-leak" + applied_config = api.identity_providers["employer-adfs"]["config"] + assert applied_config["clientSecret"] == "federation-secret" + assert applied_config["unclassifiedValue"] == "must-not-leak" + assert status.registration.provider_config["clientSecret"] == "" + assert status.registration.provider_config["unclassifiedValue"] == "" + assert status.registration.provider_config["singleSignOnServiceUrl"] == ( + "https://sts.example/adfs/ls/" + ) + + +def test_put_updates_existing_provider_in_place(federation, api) -> None: + """Updating desired state replaces the applied provider.""" + registration = _employer_adfs_registration() + federation.put_registration("employer-adfs", registration) + + updated = registration.model_copy(update={"enabled": False}) + federation.put_registration("employer-adfs", updated) + + assert api.identity_providers["employer-adfs"]["enabled"] is False + assert any( + call.startswith("update_identity_provider:employer-adfs") + for call in api.calls + ) + + +def test_put_retains_desired_state_when_keycloak_is_unavailable( + federation, store, api, monkeypatch +) -> None: + """A failed convergence is explicit and remains retryable from stored state.""" + + def fail_create(*args, **kwargs) -> None: + """Simulate a temporarily unavailable Keycloak Admin REST API.""" + raise RuntimeError("keycloak unavailable") + + monkeypatch.setattr(api, "create_identity_provider", fail_create) + + status = federation.put_registration( + "employer-adfs", _employer_adfs_registration() + ) + + assert status.applied_to_keycloak is False + assert store.get(FEDERATION_PROVIDER_NAMESPACE, "employer-adfs") is not None + + +def test_stored_status_remains_readable_during_keycloak_outage( + store, api, monkeypatch +) -> None: + """Desired state remains observable and redacted when status I/O fails.""" + registration = _employer_adfs_registration() + store.put( + FEDERATION_PROVIDER_NAMESPACE, + registration.provider_alias, + registration.model_dump_json(), + ) + federation = FederationService(store, api) + + def fail_status(*args, **kwargs): + """Simulate Keycloak being unavailable during a status read.""" + raise RuntimeError("keycloak unavailable") + + monkeypatch.setattr(api, "get_identity_provider", fail_status) + + statuses = federation.list_registrations() + + assert len(statuses) == 1 + assert statuses[0].applied_to_keycloak is False + assert statuses[0].registration.provider_config["clientSecret"] == "" + assert ( + statuses[0].registration.provider_config["unclassifiedValue"] + == "" + ) + + +def test_status_network_call_does_not_hold_desired_state_lock( + store, api, monkeypatch +) -> None: + """A slow Keycloak status call does not block another stored-state read.""" + registration = _employer_adfs_registration() + store.put( + FEDERATION_PROVIDER_NAMESPACE, + registration.provider_alias, + registration.model_dump_json(), + ) + federation = FederationService(store, api) + first_call_started = threading.Event() + release_first_call = threading.Event() + second_call_started = threading.Event() + call_guard = threading.Lock() + call_count = 0 + + def blocking_status(provider_alias: str): + """Block the first network call and signal entry into the second.""" + nonlocal call_count + with call_guard: + call_count += 1 + current_call = call_count + if current_call == 1: + first_call_started.set() + assert release_first_call.wait(timeout=5) + else: + second_call_started.set() + return None + + monkeypatch.setattr(api, "get_identity_provider", blocking_status) + + with ThreadPoolExecutor(max_workers=2) as executor: + list_future = executor.submit(federation.list_registrations) + assert first_call_started.wait(timeout=2) + get_future = executor.submit( + federation.get_registration, "employer-adfs" + ) + second_reached_network = second_call_started.wait(timeout=0.5) + release_first_call.set() + list_future.result(timeout=5) + get_future.result(timeout=5) + + assert second_reached_network + + +def test_apply_all_reconverges_after_realm_rebuild(federation, api) -> None: + """Stored desired state recreates providers after realm loss.""" + federation.put_registration( + "employer-adfs", _employer_adfs_registration() + ) + api.identity_providers.clear() + + statuses = federation.apply_all() + + assert [ + status.registration.provider_alias for status in statuses + ] == ["employer-adfs"] + assert statuses[0].applied_to_keycloak is True + assert "employer-adfs" in api.identity_providers + + +def test_delete_removes_keycloak_and_store(federation, store, api) -> None: + """Deletion removes both applied and desired provider state.""" + federation.put_registration( + "employer-adfs", _employer_adfs_registration() + ) + + federation.delete_registration("employer-adfs") + + assert store.get(FEDERATION_PROVIDER_NAMESPACE, "employer-adfs") is None + assert "employer-adfs" not in api.identity_providers + + +def test_alias_provider_and_config_bounds_are_enforced(federation) -> None: + """Malformed aliases, providers, and oversized config fail closed.""" + registration = _employer_adfs_registration() + + with pytest.raises(HTTPException) as mismatch: + federation.put_registration("other-alias", registration) + assert mismatch.value.status_code == 400 + + bad_alias = registration.model_copy( + update={"provider_alias": "Bad Alias!"} + ) + with pytest.raises(HTTPException) as invalid_alias: + federation.put_registration("Bad Alias!", bad_alias) + assert invalid_alias.value.status_code == 400 + + unicode_alias = registration.model_copy( + update={"provider_alias": "employer-аdfs"} + ) + with pytest.raises(HTTPException) as non_ascii_alias: + federation.put_registration("employer-аdfs", unicode_alias) + assert non_ascii_alias.value.status_code == 400 + + bad_provider = registration.model_copy(update={"provider_id": "ws-fed"}) + with pytest.raises(HTTPException) as invalid_provider: + federation.put_registration("employer-adfs", bad_provider) + assert invalid_provider.value.status_code == 400 + + too_many = registration.model_copy( + update={ + "provider_config": { + f"entry{index}": "value" for index in range(65) + } + } + ) + with pytest.raises(HTTPException) as oversized: + federation.put_registration("employer-adfs", too_many) + assert oversized.value.status_code == 400 + + +def test_http_surface_never_echoes_provider_secret( + api, auth_header, operator_token +) -> None: + """PUT, list, and get responses redact credential and unknown values.""" + app = create_app(wire=False) + app.state.federation_service = FederationService(InMemoryKvStore(), api) + app.state.operator_api_token = operator_token + body = _employer_adfs_registration().model_dump() + + with TestClient(app, headers=auth_header) as client: + put_response = client.put( + "/federation/identity-providers/employer-adfs", + json=body, + ) + list_response = client.get("/federation/identity-providers") + get_response = client.get( + "/federation/identity-providers/employer-adfs" + ) + delete_response = client.delete( + "/federation/identity-providers/employer-adfs" + ) + missing_response = client.get( + "/federation/identity-providers/employer-adfs" + ) + + assert put_response.status_code == 200 + put_config = put_response.json()["registration"]["provider_config"] + list_config = list_response.json()[0]["registration"]["provider_config"] + get_config = get_response.json()["registration"]["provider_config"] + for response_config in (put_config, list_config, get_config): + assert response_config["clientSecret"] == "" + assert response_config["unclassifiedValue"] == "" + combined_text = put_response.text + list_response.text + get_response.text + assert "federation-secret" not in combined_text + assert "must-not-leak" not in combined_text + assert delete_response.status_code == 204 + assert missing_response.status_code == 404 diff --git a/services/account_unification/tests/test_healthcheck.py b/services/account_unification/tests/test_healthcheck.py index e538b16..c315c8e 100644 --- a/services/account_unification/tests/test_healthcheck.py +++ b/services/account_unification/tests/test_healthcheck.py @@ -1,50 +1,107 @@ """Container healthcheck command behavior.""" from __future__ import annotations +import urllib.request + from app import healthcheck class _Response: + """Small context-managed HTTP response test double.""" + def __init__(self, payload: bytes) -> None: + """Store one response payload.""" self._payload = payload def __enter__(self) -> "_Response": + """Enter the response context.""" return self def __exit__(self, *args: object) -> None: + """Exit the response context without suppressing errors.""" return None def read(self) -> bytes: + """Return the stored response payload.""" return self._payload -def test_healthcheck_returns_zero_for_ok_status(monkeypatch, capsys): - def fake_urlopen(url: str, *, timeout: int) -> _Response: +def test_healthcheck_returns_zero_for_ok_status(monkeypatch, capsys) -> None: + """A ready service produces a successful shell exit status.""" + + def fake_open(url: str) -> _Response: + """Return one ready JSON response.""" assert url == "http://service/healthz" - assert timeout == 5 return _Response(b'{"status":"ok"}') - monkeypatch.setattr(healthcheck.urllib.request, "urlopen", fake_urlopen) + monkeypatch.setattr(healthcheck, "_open_health_url", fake_open) assert healthcheck.main("http://service/healthz") == 0 assert capsys.readouterr().out == "ok\n" -def test_healthcheck_returns_one_for_non_ok_status(monkeypatch, capsys): - def fake_urlopen(url: str, *, timeout: int) -> _Response: +def test_healthcheck_returns_one_for_non_ok_status(monkeypatch, capsys) -> None: + """A valid non-ready body produces a failed shell status.""" + + def fake_open(url: str) -> _Response: + """Return one starting JSON response.""" return _Response(b'{"status":"starting"}') - monkeypatch.setattr(healthcheck.urllib.request, "urlopen", fake_urlopen) + monkeypatch.setattr(healthcheck, "_open_health_url", fake_open) assert healthcheck.main("http://service/healthz") == 1 assert "not ready" in capsys.readouterr().err -def test_healthcheck_returns_one_for_request_error(monkeypatch, capsys): - def fake_urlopen(url: str, *, timeout: int) -> _Response: +def test_healthcheck_returns_one_for_request_error(monkeypatch, capsys) -> None: + """Transport errors are converted into a failed shell status.""" + + def fake_open(url: str) -> _Response: + """Raise one deterministic connection error.""" raise OSError("connection refused") - monkeypatch.setattr(healthcheck.urllib.request, "urlopen", fake_urlopen) + monkeypatch.setattr(healthcheck, "_open_health_url", fake_open) assert healthcheck.main("http://service/healthz") == 1 assert "healthcheck failed: connection refused" in capsys.readouterr().err + + +def test_healthcheck_rejects_non_http_scheme(monkeypatch, capsys) -> None: + """A non-HTTP(S) URL is rejected before urllib ever opens it.""" + + def fail_open(*args: object, **kwargs: object) -> _Response: + """Fail if a rejected scheme reaches the opener.""" + raise AssertionError("the opener must not run for a rejected scheme") + + monkeypatch.setattr(healthcheck, "_open_health_url", fail_open) + + assert healthcheck.main("file:///etc/passwd") == 1 + assert "unsupported URL scheme 'file'" in capsys.readouterr().err + + +def test_healthcheck_opener_drops_non_http_redirect_target() -> None: + """A redirect cannot escape HTTP(S), file, FTP, or error boundaries.""" + handler = healthcheck._HttpOnlyRedirectHandler() + dropped = handler.redirect_request( + urllib.request.Request("http://127.0.0.1:8099/healthz"), + None, + 302, + "Found", + {}, + "ftp://127.0.0.1/secret", + ) + assert dropped is None + kept = handler.redirect_request( + urllib.request.Request("http://127.0.0.1:8099/healthz"), + None, + 302, + "Found", + {}, + "http://127.0.0.1:8099/ready", + ) + assert kept is not None + opener = healthcheck._build_http_only_opener() + handler_names = {type(item).__name__ for item in opener.handlers} + assert not handler_names & {"FTPHandler", "FileHandler", "DataHandler"} + assert "HTTPDefaultErrorHandler" in handler_names + assert "HTTPErrorProcessor" in handler_names diff --git a/services/account_unification/tests/test_hourly_pr_steward.py b/services/account_unification/tests/test_hourly_pr_steward.py new file mode 100644 index 0000000..910133e --- /dev/null +++ b/services/account_unification/tests/test_hourly_pr_steward.py @@ -0,0 +1,78 @@ +"""Static contract tests for the hourly protected PR steward.""" +from __future__ import annotations + +from pathlib import Path + + +def _workflow_source() -> str: + """Return the repository's hourly PR stewardship workflow source.""" + repository_root = Path(__file__).resolve().parents[3] + return ( + repository_root / ".github" / "workflows" / "hourly-pr-steward.yml" + ).read_text(encoding="utf-8") + + +def _permissions_block(source: str, marker: str, terminator: str) -> str: + """Return one indentation-sensitive workflow permissions block.""" + block_start = source.index(marker) + block_end = source.index(terminator, block_start) + return source[block_start:block_end] + + +def test_hourly_steward_runs_once_per_hour_with_bounded_concurrency() -> None: + """The schedule is hourly and overlapping steward runs are serialized.""" + workflow = _workflow_source() + assert 'cron: "17 * * * *"' in workflow + assert "group: hourly-pr-steward" in workflow + assert "cancel-in-progress: false" in workflow + assert "timeout-minutes: 10" in workflow + + +def test_hourly_steward_uses_read_only_workflow_token_defaults() -> None: + """Only the steward job receives its narrowly required write scopes.""" + workflow = _workflow_source() + top_level_permissions = _permissions_block( + workflow, + "permissions:\n", + "\nconcurrency:", + ) + job_permissions = _permissions_block( + workflow, + " permissions:\n", + " steps:", + ) + + assert "contents: read" in top_level_permissions + assert "write" not in top_level_permissions + assert "contents: write" in job_permissions + assert "pull-requests: write" in job_permissions + assert "checks: read" in job_permissions + assert "security-events: write" not in workflow + assert "actions: write" not in workflow + + +def test_hourly_steward_is_fail_closed_on_trust_review_and_checks() -> None: + """Untrusted, unapproved, pending, or failed pull requests remain untouched.""" + workflow = _workflow_source() + assert 'head_owner" != "ContextualWisdomLab"' in workflow + assert 'trusted_author" != "true"' in workflow + assert 'review_decision" != "APPROVED"' in workflow + assert 'gh pr checks "$number" --repo "$REPOSITORY" --required' in workflow + assert "--admin" not in workflow + + +def test_hourly_steward_invalidates_old_evidence_after_branch_update() -> None: + """A branch update exits the current iteration before merging stale evidence.""" + workflow = _workflow_source() + update_position = workflow.index("gh pr update-branch") + continue_position = workflow.index("continue", update_position) + approval_position = workflow.index('review_decision" != "APPROVED"') + assert update_position < continue_position < approval_position + + +def test_hourly_steward_binds_auto_merge_to_the_checked_head() -> None: + """GitHub auto-merge is armed only for the enumerated exact head SHA.""" + workflow = _workflow_source() + assert '--auto \\' in workflow + assert '--squash \\' in workflow + assert '--match-head-commit "$head_sha"' in workflow diff --git a/services/account_unification/tests/test_identifiers.py b/services/account_unification/tests/test_identifiers.py new file mode 100644 index 0000000..e6572d1 --- /dev/null +++ b/services/account_unification/tests/test_identifiers.py @@ -0,0 +1,92 @@ +"""Path-segment validation blocks Keycloak Admin REST route confusion.""" +from __future__ import annotations + +import httpx +import pytest + +from app.identifiers import ( + InvalidIdentifierError, + validate_path_segment, +) +from app.product_keycloak_client import ProductHttpAdminApi + + +@pytest.mark.parametrize( + "bad_value", + [ + "", + ".", + "..", + "../victim", + "a/b", + "a\\b", + "%2e%2e", + "a%2fb", + "a?admin=true", + "a#fragment", + "line\nbreak", + "null\x00byte", + ], +) +def test_validate_path_segment_rejects_unsafe(bad_value): + """Unsafe path and URI syntax is rejected as an opaque identifier.""" + with pytest.raises(InvalidIdentifierError): + validate_path_segment(bad_value, field_name="user_id") + + +def test_validate_path_segment_accepts_uuid_and_slug(): + """Expected Keycloak UUID and slug identifiers remain valid.""" + assert validate_path_segment( + "f70ac86c-dbc9-4b55-bace-c3486827a136" + ) == "f70ac86c-dbc9-4b55-bace-c3486827a136" + assert validate_path_segment("employer-adfs") == "employer-adfs" + + +def test_product_admin_client_rejects_route_confusion_before_request(): + """An extra path segment cannot change the intended Admin REST operation.""" + seen: list[str] = [] + + def handler(request: httpx.Request) -> httpx.Response: + seen.append(request.url.path) + if request.url.path.endswith("/token"): + return httpx.Response(200, json={"access_token": "t"}) + return httpx.Response(200, json={"id": "x"}) + + api = ProductHttpAdminApi( + server_url="http://keycloak.test", + realm="cwl", + client_id="account-unification-svc", + client_secret="secret", + transport=httpx.MockTransport(handler), + ) + api._token = "t" + + with pytest.raises(InvalidIdentifierError): + api.get_user("victim/federated-identity") + assert seen == [] + + +def test_product_admin_client_allows_safe_id(): + """A safe opaque user id reaches exactly the intended endpoint.""" + seen: list[str] = [] + + def handler(request: httpx.Request) -> httpx.Response: + seen.append(request.url.path) + return httpx.Response( + 200, + json={"id": "safe-id", "username": "u"}, + ) + + api = ProductHttpAdminApi( + server_url="http://keycloak.test", + realm="cwl", + client_id="account-unification-svc", + client_secret="secret", + transport=httpx.MockTransport(handler), + ) + api._token = "t" + + user = api.get_user("safe-id") + + assert user.user_id == "safe-id" + assert seen == ["/admin/realms/cwl/users/safe-id"] diff --git a/services/account_unification/tests/test_kcadm_bootstrap.py b/services/account_unification/tests/test_kcadm_bootstrap.py new file mode 100644 index 0000000..dc8024b --- /dev/null +++ b/services/account_unification/tests/test_kcadm_bootstrap.py @@ -0,0 +1,95 @@ +"""Security, compatibility, and idempotency checks for Keycloak bootstrap.""" +from __future__ import annotations + +import subprocess +from pathlib import Path + + +def _bootstrap_path() -> Path: + """Return the repository's Keycloak Admin CLI bootstrap path.""" + repository_root = Path(__file__).resolve().parents[3] + return repository_root / "deploy" / "keycloak" / "kcadm-bootstrap.sh" + + +def _bootstrap_script() -> str: + """Return the repository's Keycloak Admin CLI bootstrap source.""" + return _bootstrap_path().read_text(encoding="utf-8") + + +def test_bootstrap_has_valid_bash_syntax() -> None: + """Reject malformed shell before a deployment attempts bootstrap.""" + subprocess.run( + ["bash", "-n", str(_bootstrap_path())], + check=True, + capture_output=True, + text=True, + ) + + +def test_bootstrap_uses_documented_password_environment_variable() -> None: + """Authenticate with Keycloak's documented KC_CLI_PASSWORD mechanism.""" + script = _bootstrap_script() + + credentials_command = ( + 'KC_CLI_PASSWORD="${ADMIN_PASS}" kcadm config credentials' + ) + assert credentials_command in script + assert '--user "${ADMIN_USER}"' in script + assert "--password" not in script + assert "--token" not in script + assert "curl -sf" not in script + + +def test_bootstrap_isolates_kcadm_without_replacing_kv_home() -> None: + """Scope the temporary HOME to kcadm so later KV commands keep working.""" + script = _bootstrap_script() + + assert "kcadm() {" in script + assert 'HOME="${_kcadm_home}" kcadm.sh "$@"' in script + assert "export HOME=" not in script + + +def test_bootstrap_discards_reusable_admin_password_after_login() -> None: + """The reusable bootstrap password is unset before client convergence.""" + script = _bootstrap_script() + credentials_position = script.index( + 'KC_CLI_PASSWORD="${ADMIN_PASS}" kcadm config credentials' + ) + unset_position = script.index("unset ADMIN_PASS", credentials_position) + next_bootstrap_step = script.index( + 'echo "==> converging account-unification-svc client secret from KV"', + unset_position, + ) + + assert credentials_position < unset_position < next_bootstrap_step + + +def test_service_client_secret_never_enters_process_arguments() -> None: + """Patch the client from a private JSON file, never a secret argv value.""" + script = _bootstrap_script() + + assert "umask 077" in script + assert "SERVICE_SECRET_JSON" in script + update_position = script.index( + 'kcadm update "clients/${SVC_CLIENT_UUID}"' + ) + file_input_position = script.index( + '-f "${SERVICE_SECRET_JSON}"', + update_position, + ) + assert update_position < file_input_position + assert '-s "secret=$(kv get' not in script + assert "kv put secret/idp/account-unification-client-secret" not in script + + +def test_protocol_mapper_is_converged_idempotently() -> None: + """Update one mapper and remove historical duplicates on every run.""" + script = _bootstrap_script() + + assert "MAPPER_IDS=" in script + assert "MAPPER_ID=" in script + assert 'if [[ -n "${MAPPER_ID}" ]]' in script + assert "protocol-mappers/models/${MAPPER_ID}" in script + assert "tail -n +2" in script + assert "duplicate_mapper_id" in script + assert "kcadm delete" in script diff --git a/services/account_unification/tests/test_keycloak_client.py b/services/account_unification/tests/test_keycloak_client.py index eece88d..ccce08b 100644 --- a/services/account_unification/tests/test_keycloak_client.py +++ b/services/account_unification/tests/test_keycloak_client.py @@ -1,34 +1,64 @@ -"""HTTP Keycloak Admin API adapter mapping tests.""" +"""Core and product Keycloak Admin REST API adapter tests.""" from __future__ import annotations +import json + import httpx +import pytest +from app.identifiers import InvalidIdentifierError from app.keycloak_client import AdminApi, HttpAdminApi -from app.models import FederatedIdentity, GroupMembership, RoleMapping, UserAccount +from app.models import ( + FederatedIdentity, + GroupMembership, + RoleMapping, + UserAccount, +) +from app.product_keycloak_client import ProductAdminApi, ProductHttpAdminApi from .mock_keycloak import MockKeycloakAdminApi +from .mock_product_keycloak import MockProductKeycloakAdminApi -def test_admin_api_protocol_methods_have_concrete_implementations(): - protocol_methods = { +def _protocol_methods(*protocols: type) -> set[str]: + """Return all public callable methods declared by protocols.""" + return { name - for name, member in AdminApi.__dict__.items() + for protocol in protocols + for name, member in protocol.__dict__.items() if callable(member) and not name.startswith("_") } - assert protocol_methods + + +def test_protocol_methods_have_concrete_implementations() -> None: + """Every declared contract method exists on both live and test adapters.""" + core_methods = _protocol_methods(AdminApi) + product_methods = _protocol_methods(AdminApi, ProductAdminApi) + assert core_methods + assert product_methods for implementation in (HttpAdminApi, MockKeycloakAdminApi): - missing = [ + assert [ + name + for name in sorted(core_methods) + if not callable(getattr(implementation, name, None)) + ] == [] + for implementation in ( + ProductHttpAdminApi, + MockProductKeycloakAdminApi, + ): + assert [ name - for name in sorted(protocol_methods) + for name in sorted(product_methods) if not callable(getattr(implementation, name, None)) - ] - assert missing == [] + ] == [] -def test_http_admin_api_maps_keycloak_rest_calls(): +def test_product_http_admin_api_maps_keycloak_calls() -> None: + """The product adapter maps core and extended methods correctly.""" calls: list[httpx.Request] = [] def handler(request: httpx.Request) -> httpx.Response: + """Return deterministic Keycloak responses and retain requests.""" calls.append(request) path = request.url.path if path.endswith("/protocol/openid-connect/token"): @@ -44,17 +74,31 @@ def handler(request: httpx.Request) -> httpx.Response: "enabled": True, "firstName": "Jane", "lastName": "Doe", - "attributes": {"scim_external_id": ["hr-1"]}, + "attributes": { + "scim_external_id": ["hr-1"], + "merged_into_user_id": ["survivor"], + }, }, ) if request.method == "GET" and path.endswith("/users"): return httpx.Response( 200, - json=[{"id": "u1", "username": "jane", "email": "jane@corp.test"}], + json=[ + { + "id": "u1", + "username": "jane", + "email": "jane@corp.test", + } + ], ) if request.method == "POST" and path.endswith("/users"): - return httpx.Response(201, headers={"Location": "http://kc/admin/realms/cwl/users/u2"}) - if request.method == "GET" and path.endswith("/federated-identity"): + return httpx.Response( + 201, + headers={ + "Location": "http://kc/admin/realms/cwl/users/u2" + }, + ) + if path.endswith("/federated-identity"): return httpx.Response( 200, json=[ @@ -65,7 +109,7 @@ def handler(request: httpx.Request) -> httpx.Response: } ], ) - if request.method == "GET" and path.endswith("/role-mappings"): + if path.endswith("/role-mappings"): return httpx.Response( 200, json={ @@ -73,16 +117,26 @@ def handler(request: httpx.Request) -> httpx.Response: "clientMappings": { "client-uuid": { "id": "client-uuid", - "mappings": [{"id": "client-r", "name": "editor"}], + "mappings": [ + {"id": "client-r", "name": "editor"} + ], } }, }, ) - if request.method == "GET" and path.endswith("/groups"): - return httpx.Response(200, json=[{"id": "g1", "name": "Ops", "path": "/Ops"}]) + if path.endswith("/groups"): + return httpx.Response( + 200, + json=[{"id": "g1", "name": "Ops", "path": "/Ops"}], + ) + if ( + request.method == "GET" + and "/identity-provider/instances/" in path + ): + return httpx.Response(404) return httpx.Response(204) - api = HttpAdminApi( + api = ProductHttpAdminApi( "http://keycloak.test", "cwl", "account-unification-svc", @@ -92,32 +146,256 @@ def handler(request: httpx.Request) -> httpx.Response: user = api.get_user("u1") assert user.external_id == "hr-1" - assert [u.user_id for u in api.find_users_by_email("jane@corp.test")] == ["u1"] - assert api.find_user_by_username("Jane").user_id == "u1" - assert api.create_user(UserAccount(user_id="", user_name="new", email="new@corp.test")) == "u2" - api.replace_user("u1", user) + assert api.get_user_attribute("u1", "merged_into_user_id") == "survivor" + assert [ + item.user_id + for item in api.find_users_by_email("jane@corp.test") + ] == ["u1"] + found = api.find_user_by_username("Jane") + assert found is not None + assert found.user_id == "u1" + assert api.create_user( + UserAccount( + user_id="", + user_name="new", + email="new@corp.test", + ) + ) == "u2" assert api.list_federated_identities("u1") == [ - FederatedIdentity(identity_provider="adfs", external_user_id="jane@corp", external_user_name="Jane Doe") + FederatedIdentity( + identity_provider="adfs", + external_user_id="jane@corp", + external_user_name="Jane Doe", + ) ] - assert {role.role_name for role in api.list_role_mappings("u1")} == {"admin", "editor"} + assert { + role.role_name for role in api.list_role_mappings("u1") + } == {"admin", "editor"} assert api.list_group_memberships("u1") == [ - GroupMembership(group_id="g1", group_name="Ops", group_path="/Ops") + GroupMembership( + group_id="g1", + group_name="Ops", + group_path="/Ops", + ) ] + assert api.get_identity_provider("missing-provider") is None + api.replace_user("u1", user) api.add_federated_identity( - "u1", FederatedIdentity(identity_provider="github", external_user_id="jane") + "u1", + FederatedIdentity( + identity_provider="github", + external_user_id="jane", + ), ) api.remove_federated_identity("u1", "github") - api.add_role_mapping("u1", RoleMapping(role_id="realm-r", role_name="admin")) - api.add_role_mapping("u1", RoleMapping(role_id="client-r", role_name="editor", client_id="client-uuid")) - api.remove_role_mapping("u1", RoleMapping(role_id="realm-r", role_name="admin")) - api.remove_role_mapping("u1", RoleMapping(role_id="client-r", role_name="editor", client_id="client-uuid")) - api.add_group_membership("u1", GroupMembership(group_id="g1", group_path="/Ops")) - api.remove_group_membership("u1", GroupMembership(group_id="g1", group_path="/Ops")) + api.add_role_mapping( + "u1", RoleMapping(role_id="realm-r", role_name="admin") + ) + api.add_role_mapping( + "u1", + RoleMapping( + role_id="client-r", + role_name="editor", + client_id="client-uuid", + ), + ) + api.remove_role_mapping( + "u1", RoleMapping(role_id="realm-r", role_name="admin") + ) + api.remove_role_mapping( + "u1", + RoleMapping( + role_id="client-r", + role_name="editor", + client_id="client-uuid", + ), + ) + api.add_group_membership( + "u1", GroupMembership(group_id="g1", group_path="/Ops") + ) + api.remove_group_membership( + "u1", GroupMembership(group_id="g1", group_path="/Ops") + ) api.deactivate_user("u1") api.set_user_attribute("u1", "duplicate_of", "survivor") + api.send_execute_actions_email( + "u1", + ["VERIFY_EMAIL", "webauthn-register-passwordless"], + client_id="naruon-web", + redirect_uri="https://naruon.example/auth/passkey-complete", + lifespan_seconds=900, + ) + api.create_identity_provider({"alias": "employer-adfs"}) + api.update_identity_provider( + "employer-adfs", + {"alias": "employer-adfs", "enabled": False}, + ) + api.delete_identity_provider("employer-adfs") + api.delete_user("u1") api.close() - assert any(call.headers.get("authorization") == "Bearer token-1" for call in calls) + assert any( + call.headers.get("authorization") == "Bearer token-1" + for call in calls + ) assert any(call.method == "DELETE" and call.content for call in calls) + action_request = next( + call + for call in calls + if call.url.path.endswith("/users/u1/execute-actions-email") + ) + assert action_request.url.params["client_id"] == "naruon-web" + assert action_request.url.params["redirect_uri"] == ( + "https://naruon.example/auth/passkey-complete" + ) + assert action_request.url.params["lifespan"] == "900" + assert json.loads(action_request.content) == [ + "VERIFY_EMAIL", + "webauthn-register-passwordless", + ] + + +def test_product_adapter_reauthenticates_get_once() -> None: + """An expired token is refreshed once before a GET succeeds.""" + token_requests = 0 + user_requests = 0 + + def handler(request: httpx.Request) -> httpx.Response: + """Reject the stale bearer token and accept the refreshed token.""" + nonlocal token_requests, user_requests + if request.url.path.endswith("/protocol/openid-connect/token"): + token_requests += 1 + return httpx.Response( + 200, json={"access_token": f"token-{token_requests}"} + ) + user_requests += 1 + if request.headers.get("Authorization") == "Bearer token-0": + return httpx.Response(401) + return httpx.Response( + 200, + json={"id": "u1", "username": "jane", "enabled": True}, + ) + + api = ProductHttpAdminApi( + "http://keycloak.test", + "cwl", + "svc", + "secret", + transport=httpx.MockTransport(handler), + ) + api._token = "token-0" + + assert api.get_user("u1").user_id == "u1" + assert token_requests == 1 + assert user_requests == 2 + + +def test_product_adapter_reauthenticates_create_once() -> None: + """User creation also retries exactly once after an expired token.""" + token_requests = 0 + create_requests = 0 + + def handler(request: httpx.Request) -> httpx.Response: + """Reject one stale create request and accept the retry.""" + nonlocal token_requests, create_requests + if request.url.path.endswith("/protocol/openid-connect/token"): + token_requests += 1 + return httpx.Response(200, json={"access_token": "token-1"}) + create_requests += 1 + if request.headers.get("Authorization") == "Bearer token-0": + return httpx.Response(401) + return httpx.Response( + 201, + headers={"Location": "http://kc/admin/realms/cwl/users/u2"}, + ) + + api = ProductHttpAdminApi( + "http://keycloak.test", + "cwl", + "svc", + "secret", + transport=httpx.MockTransport(handler), + ) + api._token = "token-0" + + account_id = api.create_user( + UserAccount( + user_id="", + user_name="new", + email="new@example.com", + ) + ) + + assert account_id == "u2" + assert token_requests == 1 + assert create_requests == 2 + + +@pytest.mark.parametrize( + "unsafe_user_id", + [ + "../victim", + "victim%2Fother", + "victim\\other", + "victim\x00other", + ], +) +def test_product_adapter_rejects_unsafe_paths(unsafe_user_id: str) -> None: + """Unsafe identifiers fail before any HTTP request is emitted.""" + + def fail_handler(request: httpx.Request) -> httpx.Response: + """Fail if an unsafe value reaches the transport.""" + raise AssertionError("unsafe path must not reach the transport") + + api = ProductHttpAdminApi( + "http://keycloak.test", + "cwl", + "svc", + "secret", + transport=httpx.MockTransport(fail_handler), + ) + api._token = "token" + + with pytest.raises(InvalidIdentifierError): + api.get_user(unsafe_user_id) + + +@pytest.mark.parametrize( + ("action_aliases", "redirect_uri", "lifespan_seconds", "match"), + [ + ([], "https://naruon.example/complete", 900, "action_aliases"), + (["VERIFY_EMAIL"], "http://naruon.example/complete", 900, "redirect_uri"), + (["VERIFY_EMAIL"], "https://naruon.example/complete", 0, "lifespan"), + ], +) +def test_action_email_rejects_unsafe_configuration( + action_aliases: list[str], + redirect_uri: str, + lifespan_seconds: int, + match: str, +) -> None: + """Invalid action-email inputs fail before network transport.""" + + def fail_handler(request: httpx.Request) -> httpx.Response: + """Fail if invalid enrollment input reaches the transport.""" + raise AssertionError("invalid enrollment input reached transport") + + api = ProductHttpAdminApi( + "http://keycloak.test", + "cwl", + "svc", + "secret", + transport=httpx.MockTransport(fail_handler), + ) + api._token = "token" + + with pytest.raises(ValueError, match=match): + api.send_execute_actions_email( + "user-id", + action_aliases, + client_id="naruon-web", + redirect_uri=redirect_uri, + lifespan_seconds=lifespan_seconds, + ) diff --git a/services/account_unification/tests/test_lifecycle.py b/services/account_unification/tests/test_lifecycle.py new file mode 100644 index 0000000..14a6b23 --- /dev/null +++ b/services/account_unification/tests/test_lifecycle.py @@ -0,0 +1,61 @@ +"""Application lifecycle and temporary resource tests.""" +from __future__ import annotations + +from pathlib import Path +from types import SimpleNamespace + +from app.main import ( + _remove_temporary_lock_database, + _user_operation_lock_path, +) + + +def test_in_memory_audit_uses_real_temporary_lock_database() -> None: + """An in-memory audit sink never creates a malformed pseudo-file path.""" + lock_path, is_temporary = _user_operation_lock_path(":memory:") + try: + assert is_temporary is True + assert lock_path != ":memory:.user-operation-locks.sqlite3" + assert Path(lock_path).is_file() + finally: + Path(lock_path).unlink(missing_ok=True) + + +def test_persistent_audit_uses_adjacent_lock_sidecar(tmp_path) -> None: + """A persistent audit database keeps its lock sidecar on the same volume.""" + audit_path = str(tmp_path / "account_merge_audit.sqlite3") + lock_path, is_temporary = _user_operation_lock_path(audit_path) + assert lock_path == f"{audit_path}.user-operation-locks.sqlite3" + assert is_temporary is False + + +def test_temporary_lock_database_is_removed_at_shutdown(tmp_path) -> None: + """Lifecycle cleanup removes only the explicitly temporary sidecar.""" + lock_path = tmp_path / "temporary_user_locks.sqlite3" + lock_path.write_text("placeholder", encoding="utf-8") + app = SimpleNamespace( + state=SimpleNamespace( + temporary_user_operation_lock_database=True, + user_operation_lock_database_path=str(lock_path), + ) + ) + + _remove_temporary_lock_database(app) + + assert not lock_path.exists() + + +def test_persistent_lock_database_is_not_removed_at_shutdown(tmp_path) -> None: + """Lifecycle cleanup leaves deployment-owned persistent sidecars intact.""" + lock_path = tmp_path / "persistent_user_locks.sqlite3" + lock_path.write_text("placeholder", encoding="utf-8") + app = SimpleNamespace( + state=SimpleNamespace( + temporary_user_operation_lock_database=False, + user_operation_lock_database_path=str(lock_path), + ) + ) + + _remove_temporary_lock_database(app) + + assert lock_path.exists() diff --git a/services/account_unification/tests/test_merge.py b/services/account_unification/tests/test_merge.py index ebcc1d2..de00138 100644 --- a/services/account_unification/tests/test_merge.py +++ b/services/account_unification/tests/test_merge.py @@ -35,9 +35,13 @@ def test_merge_moves_links_roles_groups_and_tombstones(service, api): email="jane@corp.com", is_email_verified=True, federated_identities=[ - FederatedIdentity(identity_provider="employer-adfs", external_user_id="jane@corp") + FederatedIdentity( + identity_provider="employer-adfs", external_user_id="jane@corp" + ) + ], + role_mappings=[ + RoleMapping(role_id="r-s", role_name="viewer", client_id="naruon") ], - role_mappings=[RoleMapping(role_id="r-s", role_name="viewer", client_id="naruon")], group_memberships=[GroupMembership(group_id="g-org", group_path="/org")], ) api.create_test_user( @@ -45,9 +49,13 @@ def test_merge_moves_links_roles_groups_and_tombstones(service, api): email="jane@corp.com", is_email_verified=True, federated_identities=[ - FederatedIdentity(identity_provider="google", external_user_id="jane@gmail") + FederatedIdentity( + identity_provider="google", external_user_id="jane@gmail" + ) + ], + role_mappings=[ + RoleMapping(role_id="r-d", role_name="editor", client_id="clearfolio") ], - role_mappings=[RoleMapping(role_id="r-d", role_name="editor", client_id="clearfolio")], group_memberships=[GroupMembership(group_id="g-proj", group_path="/pg-erd")], ) @@ -55,93 +63,127 @@ def test_merge_moves_links_roles_groups_and_tombstones(service, api): assert result.match_reason is MatchReason.VERIFIED_EMAIL assert result.duplicate_tombstoned is True - # survivor gained the duplicate's external identity... - survivor_idps = {f.identity_provider for f in api.list_federated_identities("survivor")} + survivor_idps = { + identity.identity_provider + for identity in api.list_federated_identities("survivor") + } assert survivor_idps == {"employer-adfs", "google"} - # ...its client role... - survivor_roles = {r.role_name for r in api.list_role_mappings("survivor")} + survivor_roles = {role.role_name for role in api.list_role_mappings("survivor")} assert survivor_roles == {"viewer", "editor"} - # ...and its group. - survivor_groups = {g.group_id for g in api.list_group_memberships("survivor")} + survivor_groups = { + group.group_id for group in api.list_group_memberships("survivor") + } assert survivor_groups == {"g-org", "g-proj"} - # duplicate is emptied + tombstoned + disabled. assert api.list_federated_identities("dup") == [] assert "dup" in api.deactivated assert api.attributes[("dup", TOMBSTONE_ATTRIBUTE_KEY)] == "survivor" def test_merge_by_exact_idp_subject(service, api): - shared = FederatedIdentity(identity_provider="employer-adfs", external_user_id="jane@corp") + shared = FederatedIdentity( + identity_provider="employer-adfs", external_user_id="jane@corp" + ) api.create_test_user("survivor", email="a@x.com", federated_identities=[shared]) api.create_test_user("dup", email="b@y.com", federated_identities=[shared]) result = service.merge_accounts(_merge()) assert result.match_reason is MatchReason.EXACT_IDP_SUBJECT - # shared link stays on survivor exactly once (survivor-wins conflict). - assert [f.external_user_id for f in api.list_federated_identities("survivor")] == ["jane@corp"] - assert any(c.kind == "federated_identity" for c in result.conflicts) + assert [ + identity.external_user_id + for identity in api.list_federated_identities("survivor") + ] == ["jane@corp"] + assert any(conflict.kind == "federated_identity" for conflict in result.conflicts) def test_federated_identity_provider_conflict_is_survivor_wins(service, api): - # Same provider alias, different external subject: Keycloak allows only one - # link per provider, so survivor-wins keeps the survivor's. api.create_test_user( - "survivor", email="j@x.com", is_email_verified=True, + "survivor", + email="j@x.com", + is_email_verified=True, federated_identities=[ - FederatedIdentity(identity_provider="employer-adfs", external_user_id="jane@corp") + FederatedIdentity( + identity_provider="employer-adfs", external_user_id="jane@corp" + ) ], ) api.create_test_user( - "dup", email="j@x.com", is_email_verified=True, + "dup", + email="j@x.com", + is_email_verified=True, federated_identities=[ - FederatedIdentity(identity_provider="employer-adfs", external_user_id="jane2@corp") + FederatedIdentity( + identity_provider="employer-adfs", external_user_id="jane2@corp" + ) ], ) result = service.merge_accounts(_merge()) survivor_links = api.list_federated_identities("survivor") - assert [f.external_user_id for f in survivor_links] == ["jane@corp"] - assert any(c.kind == "federated_identity" for c in result.conflicts) + assert [identity.external_user_id for identity in survivor_links] == ["jane@corp"] + assert any(conflict.kind == "federated_identity" for conflict in result.conflicts) def test_role_conflict_is_survivor_wins(service, api): api.create_test_user( - "survivor", email="j@x.com", is_email_verified=True, - role_mappings=[RoleMapping(role_id="r-s", role_name="admin", client_id="naruon")], + "survivor", + email="j@x.com", + is_email_verified=True, + role_mappings=[ + RoleMapping(role_id="r-s", role_name="admin", client_id="naruon") + ], ) api.create_test_user( - "dup", email="j@x.com", is_email_verified=True, - role_mappings=[RoleMapping(role_id="r-d", role_name="admin", client_id="naruon")], + "dup", + email="j@x.com", + is_email_verified=True, + role_mappings=[ + RoleMapping(role_id="r-d", role_name="admin", client_id="naruon") + ], ) result = service.merge_accounts(_merge()) survivor_roles = api.list_role_mappings("survivor") - # only the survivor's admin role on naruon survives. assert len(survivor_roles) == 1 assert survivor_roles[0].role_id == "r-s" - assert any(c.kind == "role_mapping" and c.resolution == "survivor_wins" for c in result.conflicts) + assert any( + conflict.kind == "role_mapping" + and conflict.resolution == "survivor_wins" + for conflict in result.conflicts + ) def test_realm_role_moves(service, api): api.create_test_user("survivor", email="j@x.com", is_email_verified=True) api.create_test_user( - "dup", email="j@x.com", is_email_verified=True, - role_mappings=[RoleMapping(role_id="r-realm", role_name="ecosystem-user", client_id=None)], + "dup", + email="j@x.com", + is_email_verified=True, + role_mappings=[ + RoleMapping( + role_id="r-realm", role_name="ecosystem-user", client_id=None + ) + ], ) result = service.merge_accounts(_merge()) assert "realm:ecosystem-user" in result.moved_role_mappings - assert any(r.client_id is None for r in api.list_role_mappings("survivor")) + assert any( + role.client_id is None for role in api.list_role_mappings("survivor") + ) def test_group_conflict_is_survivor_wins(service, api): api.create_test_user( - "survivor", email="j@x.com", is_email_verified=True, + "survivor", + email="j@x.com", + is_email_verified=True, group_memberships=[GroupMembership(group_id="g1", group_path="/owners")], ) api.create_test_user( - "dup", email="j@x.com", is_email_verified=True, + "dup", + email="j@x.com", + is_email_verified=True, group_memberships=[GroupMembership(group_id="g1", group_path="/owners")], ) result = service.merge_accounts(_merge()) assert len(api.list_group_memberships("survivor")) == 1 - assert any(c.kind == "group_membership" for c in result.conflicts) + assert any(conflict.kind == "group_membership" for conflict in result.conflicts) def test_refuse_merge_on_unverified_email(service, api): @@ -149,7 +191,6 @@ def test_refuse_merge_on_unverified_email(service, api): api.create_test_user("dup", email="jane@corp.com", is_email_verified=False) with pytest.raises(UnverifiedEmailMergeError): service.merge_accounts(_merge()) - # nothing mutated: duplicate not tombstoned. assert "dup" not in api.deactivated @@ -169,22 +210,14 @@ def test_explicit_link_allows_merge_without_shared_signal(service, api): def test_explicit_link_cannot_override_shared_unverified_email(service, api): - # Hard rule (CLAUDE.md, docs/merge-unification-flow.md, and the - # MergeRequest.explicit_link contract): "Even so, the service refuses if the - # only tie is an UNVERIFIED email." An unverified address is - # attacker-registerable, so flipping explicit_link=True must NOT promote a - # shared unverified email into a merge. api.create_test_user("survivor", email="jane@corp.com", is_email_verified=False) api.create_test_user("dup", email="jane@corp.com", is_email_verified=False) with pytest.raises(UnverifiedEmailMergeError): service.merge_accounts(_merge(explicit=True)) - # nothing mutated: duplicate not tombstoned. assert "dup" not in api.deactivated def test_explicit_link_cannot_override_case_variant_unverified_email(service, api): - # Same rule, exercised through case-insensitive email normalization: one - # side verified is not enough — both must be verified for an email tie. api.create_test_user("survivor", email="Jane@Corp.com", is_email_verified=True) api.create_test_user("dup", email="jane@corp.com", is_email_verified=False) with pytest.raises(UnverifiedEmailMergeError): diff --git a/services/account_unification/tests/test_path_security.py b/services/account_unification/tests/test_path_security.py new file mode 100644 index 0000000..a3e9b16 --- /dev/null +++ b/services/account_unification/tests/test_path_security.py @@ -0,0 +1,50 @@ +"""Router-level path-parameter security tests.""" +from __future__ import annotations + +from fastapi.testclient import TestClient + +from app.main import create_app + +OPERATOR_TOKEN = "test-operator-token" +SCIM_ERROR_SCHEMA = "urn:ietf:params:scim:api:messages:2.0:Error" + + +def _client() -> TestClient: + """Return an app client with only operator authentication wired.""" + app = create_app(wire=False) + app.state.operator_api_token = OPERATOR_TOKEN + return TestClient( + app, + headers={"Authorization": f"Bearer {OPERATOR_TOKEN}"}, + ) + + +def test_admin_router_rejects_encoded_identifier() -> None: + """Encoded path material is rejected before endpoint dependencies.""" + with _client() as client: + response = client.get("/users/bad%2525identifier/identities") + assert response.status_code == 400 + assert "encoding" in response.json()["detail"] + + +def test_federation_router_rejects_traversal_alias() -> None: + """Federation aliases cannot carry encoded navigation segments.""" + with _client() as client: + response = client.get( + "/federation/identity-providers/%252e%252e" + ) + assert response.status_code == 400 + + +def test_scim_router_returns_protocol_native_error_for_unsafe_id() -> None: + """SCIM path validation returns an RFC 7644 body and media type.""" + with _client() as client: + response = client.get("/scim/v2/Users/bad%2525identifier") + + assert response.status_code == 400 + assert response.headers["content-type"].startswith("application/scim+json") + body = response.json() + assert body["schemas"] == [SCIM_ERROR_SCHEMA] + assert body["status"] == "400" + assert "detail" in body + assert "detail" not in body.get("detail", {}) diff --git a/services/account_unification/tests/test_realm_policy.py b/services/account_unification/tests/test_realm_policy.py new file mode 100644 index 0000000..83d03b2 --- /dev/null +++ b/services/account_unification/tests/test_realm_policy.py @@ -0,0 +1,88 @@ +"""Keycloak realm policy regression tests.""" +from __future__ import annotations + +import importlib.util +import json +from copy import deepcopy +from pathlib import Path +from types import ModuleType + + +def _repository_root() -> Path: + """Return the repository root from the service test package.""" + return Path(__file__).resolve().parents[3] + + +def _validator_module() -> ModuleType: + """Load the repository realm validator as a Python module.""" + validator_path = _repository_root() / "scripts" / "validate_realm.py" + spec = importlib.util.spec_from_file_location( + "keyverse_validate_realm", validator_path + ) + if spec is None or spec.loader is None: + raise RuntimeError("unable to load realm validator") + module = importlib.util.module_from_spec(spec) + spec.loader.exec_module(module) + return module + + +def _realm() -> dict: + """Load the committed Keycloak realm representation.""" + realm_path = _repository_root() / "deploy" / "keycloak" / "realm-cwl.json" + return json.loads(realm_path.read_text(encoding="utf-8")) + + +def _client(realm: dict, client_id: str) -> dict: + """Return one client representation by client ID.""" + return next( + client + for client in realm["clients"] + if client.get("clientId") == client_id + ) + + +def test_committed_realm_passes_passwordless_policy() -> None: + """The checked-in realm satisfies every fail-closed policy invariant.""" + validator = _validator_module() + assert validator.validate(_realm()) == [] + + +def test_bound_browser_flow_rejects_password_authenticator() -> None: + """No nested execution reachable from browserFlow may accept a password.""" + validator = _validator_module() + realm = deepcopy(_realm()) + credential_flow = next( + flow + for flow in realm["authenticationFlows"] + if flow.get("alias") == "browser-passwordless-credentials" + ) + credential_flow["authenticationExecutions"].append( + { + "authenticator": "auth-password-form", + "authenticatorFlow": False, + "requirement": "ALTERNATIVE", + "priority": 20, + } + ) + + errors = validator.validate(realm) + + assert any("disallowed credential-form authenticator" in error for error in errors) + + +def test_public_client_token_lifespan_is_bounded() -> None: + """A public browser client cannot issue long-lived bearer access tokens.""" + validator = _validator_module() + realm = deepcopy(_realm()) + _client(realm, "naruon-web")["attributes"]["access.token.lifespan"] = "901" + + errors = validator.validate(realm) + + assert any("access.token.lifespan" in error for error in errors) + + +def test_reusable_client_template_does_not_name_naruon_host() -> None: + """The generic RP template stays portable across ecosystem products.""" + template = _client(_realm(), "ecosystem-rp-template") + serialized = json.dumps(template, sort_keys=True) + assert "naruon.example" not in serialized diff --git a/services/account_unification/tests/test_registration.py b/services/account_unification/tests/test_registration.py new file mode 100644 index 0000000..57e93ff --- /dev/null +++ b/services/account_unification/tests/test_registration.py @@ -0,0 +1,297 @@ +"""Headless passwordless self-registration tests.""" +from __future__ import annotations + +import httpx +import pytest +from fastapi.testclient import TestClient + +from app import registration as registration_module +from app.main import create_app +from app.registration import reset_rate_limit_state + +REGISTRATION_TOKEN = "registration-token-for-tests" +OPERATOR_TOKEN = "operator-token-for-tests" +REGISTRATION_CLIENT_ID = "naruon-web" +REGISTRATION_REDIRECT_URI = "https://naruon.example/auth/passkey-complete" +REGISTRATION_ACTION_LIFESPAN_SECONDS = 900 + + +@pytest.fixture(autouse=True) +def _reset_rate_limit() -> None: + """Reset caller-keyed registration limits between tests.""" + reset_rate_limit_state() + yield + reset_rate_limit_state() + + +def _wire_registration_app(api): + """Return an app with the complete registration contract configured.""" + app = create_app(wire=False) + app.state.keycloak_api = api + app.state.registration_api_token = REGISTRATION_TOKEN + app.state.operator_api_token = OPERATOR_TOKEN + app.state.registration_client_id = REGISTRATION_CLIENT_ID + app.state.registration_redirect_uri = REGISTRATION_REDIRECT_URI + app.state.registration_action_lifespan_seconds = ( + REGISTRATION_ACTION_LIFESPAN_SECONDS + ) + return app + + +@pytest.fixture +def client(api): + """Return a registration-authenticated test client.""" + app = _wire_registration_app(api) + headers = {"Authorization": f"Bearer {REGISTRATION_TOKEN}"} + with TestClient(app, headers=headers) as test_client: + yield test_client + + +def _registration(email: str = "new.user@example.com") -> dict[str, object]: + """Build one valid registration payload without a password.""" + return { + "email_address": email, + "first_name": "New", + "last_name": "User", + } + + +def test_registration_sends_verified_passkey_enrollment_email(client, api): + """Registration creates no password and sends bounded enrollment actions.""" + response = client.post("/registration/accounts", json=_registration()) + + assert response.status_code == 201 + body = response.json() + account_id = body["account_id"] + assert body["email_address"] == "new.user@example.com" + assert api.users[account_id].is_email_verified is False + assert api.action_emails[account_id] == { + "action_aliases": [ + "VERIFY_EMAIL", + "webauthn-register-passwordless", + ], + "client_id": REGISTRATION_CLIENT_ID, + "redirect_uri": REGISTRATION_REDIRECT_URI, + "lifespan_seconds": REGISTRATION_ACTION_LIFESPAN_SECONDS, + } + assert not any(call.startswith("reset_user_password:") for call in api.calls) + + +def test_registration_rolls_back_when_enrollment_email_fails( + client, api, monkeypatch +): + """An action-email failure deletes the newly created account.""" + + def fail_action_email(*args, **kwargs) -> None: + """Simulate an unavailable Keycloak email transport.""" + raise RuntimeError("simulated Keycloak email failure") + + monkeypatch.setattr(api, "send_execute_actions_email", fail_action_email) + + response = client.post("/registration/accounts", json=_registration()) + + assert response.status_code == 502 + assert response.json()["detail"] == "account_initialization_failed" + assert api.find_users_by_email("new.user@example.com") == [] + assert any(call.startswith("delete_user:") for call in api.calls) + + +def test_registration_reports_rollback_failure(client, api, monkeypatch): + """A failed cleanup is distinguishable from the enrollment failure.""" + + def fail_action_email(*args, **kwargs) -> None: + """Simulate the original initialization failure.""" + raise RuntimeError("simulated email failure") + + def fail_delete(*args, **kwargs) -> None: + """Simulate rollback failure after user creation.""" + raise RuntimeError("simulated rollback failure") + + monkeypatch.setattr(api, "send_execute_actions_email", fail_action_email) + monkeypatch.setattr(api, "delete_user", fail_delete) + + response = client.post("/registration/accounts", json=_registration()) + + assert response.status_code == 502 + assert response.json()["detail"] == "account_initialization_rollback_failed" + + +def test_registration_normalizes_email_case(client): + """Email addresses are normalized before account creation.""" + response = client.post( + "/registration/accounts", + json=_registration(email="Mixed.Case@Example.COM"), + ) + assert response.status_code == 201 + assert response.json()["email_address"] == "mixed.case@example.com" + + +def test_registration_accepts_tagged_email(client): + """Tagged local parts are accepted without regex backtracking.""" + response = client.post( + "/registration/accounts", + json=_registration(email="new.user+product@example.com"), + ) + assert response.status_code == 201 + + +def test_registration_rejects_duplicate_email(client): + """Duplicate normalized email addresses are rejected before creation.""" + assert client.post( + "/registration/accounts", json=_registration() + ).status_code == 201 + + duplicate = client.post("/registration/accounts", json=_registration()) + + assert duplicate.status_code == 409 + assert duplicate.json()["detail"] == "email_already_registered" + + +def test_concurrent_keycloak_duplicate_maps_to_registration_conflict( + client, api, monkeypatch +): + """A Keycloak create-user 409 remains an idempotent product conflict.""" + request = httpx.Request("POST", "http://keycloak.test/admin/realms/cwl/users") + response = httpx.Response(409, request=request) + + def reject_concurrent_duplicate(*args, **kwargs): + """Model a competing request winning after the preflight lookup.""" + raise httpx.HTTPStatusError( + "duplicate user", request=request, response=response + ) + + monkeypatch.setattr(api, "create_user", reject_concurrent_duplicate) + + result = client.post("/registration/accounts", json=_registration()) + + assert result.status_code == 409 + assert result.json()["detail"] == "email_already_registered" + + +@pytest.mark.parametrize( + "email", + [ + "not-an-email", + "two@@example.com", + "control\x00@example.com", + "a@b", + ".leading@example.com", + "trailing.@example.com", + "double..dot@example.com", + "a@example..com", + "a@-example.com", + "a@example-.com", + "a@exa_mple.com", + ], +) +def test_registration_rejects_malformed_email(client, email): + """Malformed syntax is rejected deterministically.""" + response = client.post( + "/registration/accounts", json=_registration(email=email) + ) + assert response.status_code == 422 + + +def test_registration_rejects_legacy_password_field(client): + """A password cannot silently cross the passwordless registration boundary.""" + payload = _registration() + payload["initial_password"] = "legacy-bootstrap-password" + + response = client.post("/registration/accounts", json=payload) + + assert response.status_code == 422 + + +def test_registration_surface_fails_closed_without_token(api): + """The endpoint is unavailable when its credential is absent.""" + app = _wire_registration_app(api) + app.state.registration_api_token = None + with TestClient(app) as test_client: + response = test_client.post( + "/registration/accounts", + json=_registration(), + headers={"Authorization": f"Bearer {REGISTRATION_TOKEN}"}, + ) + assert response.status_code == 503 + + +def test_registration_surface_fails_closed_without_enrollment_config(api): + """Missing action-email configuration cannot create an unusable account.""" + app = _wire_registration_app(api) + app.state.registration_redirect_uri = None + with TestClient( + app, + headers={"Authorization": f"Bearer {REGISTRATION_TOKEN}"}, + ) as test_client: + response = test_client.post( + "/registration/accounts", json=_registration() + ) + assert response.status_code == 503 + assert api.find_users_by_email("new.user@example.com") == [] + + +def test_registration_rejects_wrong_token(api): + """A mismatched registration credential is rejected.""" + app = _wire_registration_app(api) + with TestClient(app) as test_client: + response = test_client.post( + "/registration/accounts", + json=_registration(), + headers={"Authorization": "Bearer wrong-token"}, + ) + assert response.status_code == 403 + + +def test_operator_token_does_not_open_registration(api): + """The operator credential cannot authorize product signup.""" + app = _wire_registration_app(api) + with TestClient(app) as test_client: + response = test_client.post( + "/registration/accounts", + json=_registration(), + headers={"Authorization": f"Bearer {OPERATOR_TOKEN}"}, + ) + assert response.status_code == 403 + + +def test_registration_rate_limit_isolated_by_caller(client, monkeypatch): + """One caller cannot consume another caller's registration allowance.""" + monkeypatch.setattr( + registration_module, + "REGISTRATION_RATE_LIMIT_MAX_ATTEMPTS", + 1, + ) + caller_keys = iter(["caller-a", "caller-a", "caller-b"]) + + def next_caller_key(request) -> str: + """Return deterministic caller identities for consecutive requests.""" + del request + return next(caller_keys) + + monkeypatch.setattr( + registration_module, + "_registration_client_key", + next_caller_key, + ) + + assert client.post( + "/registration/accounts", + json=_registration("first@example.com"), + ).status_code == 201 + limited = client.post( + "/registration/accounts", + json=_registration("second@example.com"), + ) + independent = client.post( + "/registration/accounts", + json=_registration("third@example.com"), + ) + + assert limited.status_code == 429 + assert independent.status_code == 201 + + +def test_registration_router_has_no_realm_wide_janitor_endpoint(client): + """Registration credentials cannot invoke a realm-wide destructive action.""" + response = client.post("/registration/password-janitor:run") + assert response.status_code == 404 diff --git a/services/account_unification/tests/test_scim.py b/services/account_unification/tests/test_scim.py index bdcef85..874cd50 100644 --- a/services/account_unification/tests/test_scim.py +++ b/services/account_unification/tests/test_scim.py @@ -1,21 +1,66 @@ -"""Inbound SCIM 2.0 provisioning shim -> Keycloak Admin API.""" +"""Authenticated SCIM 2.0 provisioning and merge serialization tests.""" from __future__ import annotations +import threading +from concurrent.futures import ThreadPoolExecutor +from contextlib import contextmanager + import pytest from fastapi.testclient import TestClient from app.main import create_app +from app.models import MergeRequest +from app.service import TOMBSTONE_ATTRIBUTE_KEY, UnificationService +from app.user_locks import InMemoryUserOperationLocks + +from .mock_product_keycloak import MockProductKeycloakAdminApi + + +class _BlockingReplaceApi(MockProductKeycloakAdminApi): + """Pause SCIM replacement after its tombstone check.""" + + def __init__(self) -> None: + """Create synchronization events for the race test.""" + super().__init__() + self.replace_started = threading.Event() + self.allow_replace = threading.Event() + + def replace_user(self, user_id, user) -> None: + """Block the full representation PUT until released.""" + self.replace_started.set() + if not self.allow_replace.wait(timeout=5): + raise AssertionError("test did not release the SCIM replacement") + super().replace_user(user_id, user) + self.attributes = { + attribute: value + for attribute, value in self.attributes.items() + if attribute[0] != user_id + } + self.deactivated.discard(user_id) @pytest.fixture -def client(api): +def client( + api, + user_operation_locks, + config, + auth_header, +): + """Return an authenticated SCIM test client.""" app = create_app(wire=False) app.state.keycloak_api = api - with TestClient(app) as test_client: + app.state.user_operation_locks = user_operation_locks + app.state.operator_api_token = config.operator_api_token + with TestClient(app, headers=auth_header) as test_client: yield test_client -def _scim_user(username="jane", email="jane@corp.com", external_id="hr-1"): +def _scim_user( + username: str = "jane", + email: str = "jane@corp.com", + external_id: str = "hr-1", +) -> dict[str, object]: + """Build one valid SCIM User resource.""" return { "schemas": ["urn:ietf:params:scim:schemas:core:2.0:User"], "userName": username, @@ -26,7 +71,8 @@ def _scim_user(username="jane", email="jane@corp.com", external_id="hr-1"): } -def test_service_provider_config(client): +def test_service_provider_config(client) -> None: + """SCIM clients can discover supported protocol features.""" response = client.get("/scim/v2/ServiceProviderConfig") assert response.status_code == 200 body = response.json() @@ -34,61 +80,167 @@ def test_service_provider_config(client): assert body["filter"]["supported"] is True -def test_scim_create_provisions_into_keycloak(client, api): +def test_scim_create_provisions_into_keycloak(client, api) -> None: + """SCIM create provisions a verified authoritative account.""" response = client.post("/scim/v2/Users", json=_scim_user()) assert response.status_code == 201 body = response.json() assert body["userName"] == "jane" assert body["emails"][0]["value"] == "jane@corp.com" - # The user now exists in the (mock) Keycloak store, provisioned + verified. provisioned = api.find_user_by_username("jane") assert provisioned is not None assert provisioned.is_email_verified is True assert provisioned.external_id == "hr-1" -def test_scim_create_duplicate_conflicts(client, api): - client.post("/scim/v2/Users", json=_scim_user()) +def test_scim_create_duplicate_conflicts(client) -> None: + """A duplicate SCIM username produces HTTP 409.""" + assert client.post("/scim/v2/Users", json=_scim_user()).status_code == 201 response = client.post("/scim/v2/Users", json=_scim_user()) assert response.status_code == 409 -def test_scim_get_user(client): +def test_scim_get_and_filter_user(client) -> None: + """Created users are retrievable directly and by username filter.""" created = client.post("/scim/v2/Users", json=_scim_user()).json() - response = client.get(f"/scim/v2/Users/{created['id']}") - assert response.status_code == 200 - assert response.json()["userName"] == "jane" + get_response = client.get(f"/scim/v2/Users/{created['id']}") + filter_response = client.get( + '/scim/v2/Users?filter=userName eq "jane"' + ) + + assert get_response.status_code == 200 + assert get_response.json()["userName"] == "jane" + assert filter_response.status_code == 200 + assert filter_response.json()["totalResults"] == 1 -def test_scim_get_unknown_user_404(client): + +def test_scim_get_unknown_user_404(client) -> None: + """Unknown SCIM resources produce HTTP 404.""" response = client.get("/scim/v2/Users/does-not-exist") assert response.status_code == 404 -def test_scim_filter_by_username(client): - client.post("/scim/v2/Users", json=_scim_user()) - response = client.get('/scim/v2/Users?filter=userName eq "jane"') +def test_scim_replace_updates_user(client, api) -> None: + """SCIM PUT replaces the Keycloak user representation.""" + created = client.post("/scim/v2/Users", json=_scim_user()).json() + response = client.put( + f"/scim/v2/Users/{created['id']}", + json=_scim_user(email="jane.doe@corp.com"), + ) assert response.status_code == 200 - body = response.json() - assert body["totalResults"] == 1 - assert body["Resources"][0]["userName"] == "jane" + assert api.get_user(created["id"]).email == "jane.doe@corp.com" -def test_scim_replace_updates_user(client, api): +def test_scim_replace_refuses_tombstone_resurrection(client, api) -> None: + """A merged-away duplicate cannot be re-enabled by SCIM PUT.""" created = client.post("/scim/v2/Users", json=_scim_user()).json() - updated = _scim_user(email="jane.doe@corp.com") - response = client.put(f"/scim/v2/Users/{created['id']}", json=updated) + duplicate_id = created["id"] + api.set_user_attribute( + duplicate_id, + TOMBSTONE_ATTRIBUTE_KEY, + "survivor-id", + ) + api.deactivate_user(duplicate_id) + + response = client.put( + f"/scim/v2/Users/{duplicate_id}", + json=_scim_user(), + ) + + assert response.status_code == 409 + assert duplicate_id in api.deactivated + assert api.get_user(duplicate_id).state == "disabled" + assert api.get_user_attribute( + duplicate_id, + TOMBSTONE_ATTRIBUTE_KEY, + ) == "survivor-id" + + +def test_scim_replace_is_serialized_with_merge( + config, audit, auth_header, monkeypatch +) -> None: + """The production lock manager closes the SCIM/merge TOCTOU window.""" + api = _BlockingReplaceApi() + locks = InMemoryUserOperationLocks() + merge_lock_attempted = threading.Event() + original_hold = locks.hold + + @contextmanager + def observed_hold(*user_ids: str): + """Record the merge's lock attempt while delegating to production code.""" + if set(user_ids) == {"survivor", "dup"}: + merge_lock_attempted.set() + with original_hold(*user_ids): + yield + + monkeypatch.setattr(locks, "hold", observed_hold) + service = UnificationService(api, audit, config, locks) + app = create_app(wire=False) + app.state.keycloak_api = api + app.state.user_operation_locks = locks + app.state.unification_service = service + app.state.audit_logger = audit + app.state.operator_api_token = config.operator_api_token + api.create_test_user( + "survivor", + email="jane@corp.com", + is_email_verified=True, + ) + api.create_test_user( + "dup", + email="jane@corp.com", + is_email_verified=True, + ) + + def run_merge(): + """Start one merge that contends on the duplicate-user lock.""" + return service.merge_accounts( + MergeRequest( + survivor_user_id="survivor", + duplicate_user_id="dup", + actor="admin@cwl", + ) + ) + + with ( + TestClient(app, headers=auth_header) as test_client, + ThreadPoolExecutor(max_workers=2) as executor, + ): + scim_future = executor.submit( + test_client.put, + "/scim/v2/Users/dup", + json=_scim_user(username="dup"), + ) + # replace_user is called only after SCIM has acquired the production lock. + assert api.replace_started.wait(timeout=2) + merge_future = executor.submit(run_merge) + # The hook fires immediately before the same production hold blocks on dup. + assert merge_lock_attempted.wait(timeout=2) + assert not merge_future.done() + + api.allow_replace.set() + response = scim_future.result(timeout=5) + merge_result = merge_future.result(timeout=5) + assert response.status_code == 200 - assert api.get_user(created["id"]).email == "jane.doe@corp.com" + assert merge_result.duplicate_tombstoned is True + assert api.get_user("dup").state == "disabled" + assert api.get_user_attribute("dup", TOMBSTONE_ATTRIBUTE_KEY) == "survivor" -def test_scim_patch_deactivates_user(client, api): +def test_scim_patch_deactivates_user(client, api) -> None: + """SCIM PATCH active=false disables the Keycloak account.""" created = client.post("/scim/v2/Users", json=_scim_user()).json() response = client.patch( f"/scim/v2/Users/{created['id']}", json={ - "schemas": ["urn:ietf:params:scim:api:messages:2.0:PatchOp"], - "Operations": [{"op": "replace", "value": {"active": False}}], + "schemas": [ + "urn:ietf:params:scim:api:messages:2.0:PatchOp" + ], + "Operations": [ + {"op": "replace", "value": {"active": False}} + ], }, ) assert response.status_code == 200 @@ -96,7 +248,8 @@ def test_scim_patch_deactivates_user(client, api): assert created["id"] in api.deactivated -def test_scim_delete_deprovisions_by_disabling(client, api): +def test_scim_delete_deprovisions_by_disabling(client, api) -> None: + """SCIM DELETE performs a non-destructive soft deprovision.""" created = client.post("/scim/v2/Users", json=_scim_user()).json() response = client.delete(f"/scim/v2/Users/{created['id']}") assert response.status_code == 204 diff --git a/services/account_unification/tests/test_storage_concurrency.py b/services/account_unification/tests/test_storage_concurrency.py new file mode 100644 index 0000000..bd2f86b --- /dev/null +++ b/services/account_unification/tests/test_storage_concurrency.py @@ -0,0 +1,61 @@ +"""Thread-safety tests for standalone SQLite configuration and audit stores.""" +from __future__ import annotations + +from concurrent.futures import ThreadPoolExecutor +from contextlib import closing + +from app.audit import AuditLogger, SqliteAuditSink +from app.kv_store import SqliteKvStore + + +def test_sqlite_kv_store_handles_concurrent_access(tmp_path) -> None: + """One store instance safely serves concurrent readers and writers.""" + with closing( + SqliteKvStore(str(tmp_path / "configuration.sqlite3")) + ) as store: + + def write_entry(index: int) -> str | None: + """Write and read one independently keyed configuration value.""" + entry_key = f"entry_key_{index}" + entry_value = f"entry_value_{index}" + store.put( + "runtime_configuration", + entry_key, + entry_value, + ) + return store.get("runtime_configuration", entry_key) + + with ThreadPoolExecutor(max_workers=8) as executor: + values = list(executor.map(write_entry, range(100))) + + assert values == [ + f"entry_value_{index}" for index in range(100) + ] + assert len(store.get_all("runtime_configuration")) == 100 + + +def test_sqlite_audit_sink_handles_concurrent_events(tmp_path) -> None: + """Concurrent audit events remain complete and retrievable in DB order.""" + with closing( + SqliteAuditSink(str(tmp_path / "audit_events.sqlite3")) + ) as sink: + audit = AuditLogger(sink) + audit_id = "concurrent_audit" + + def emit_event(index: int) -> None: + """Append one independently attributable audit event.""" + audit.emit( + audit_id=audit_id, + event_type="concurrent_event", + actor=f"worker_{index}", + payload={"event_index": index}, + ) + + with ThreadPoolExecutor(max_workers=8) as executor: + list(executor.map(emit_event, range(100))) + + events = audit.events_for(audit_id) + assert len(events) == 100 + assert {event.actor for event in events} == { + f"worker_{index}" for index in range(100) + } diff --git a/services/account_unification/tests/test_user_locks.py b/services/account_unification/tests/test_user_locks.py new file mode 100644 index 0000000..04af38b --- /dev/null +++ b/services/account_unification/tests/test_user_locks.py @@ -0,0 +1,80 @@ +"""Shared user-operation serialization for merge and SCIM writes.""" +from __future__ import annotations + +import threading +from concurrent.futures import ThreadPoolExecutor + +import pytest + +from app.user_locks import ( + InMemoryUserOperationLocks, + SqliteUserOperationLocks, + UserOperationLockTimeout, +) + + +def _assert_overlapping_operation_waits(first_manager, second_manager) -> None: + """Assert a second overlapping mutation waits for the first lock holder.""" + first_entered = threading.Event() + release_first = threading.Event() + second_entered = threading.Event() + + def hold_first() -> None: + """Hold the first lock until the test releases it.""" + with first_manager.hold("survivor", "dup"): + first_entered.set() + assert release_first.wait(timeout=5) + + def hold_second() -> None: + """Attempt to enter the overlapping critical section.""" + assert first_entered.wait(timeout=5) + with second_manager.hold("dup"): + second_entered.set() + + with ThreadPoolExecutor(max_workers=2) as executor: + first_future = executor.submit(hold_first) + second_future = executor.submit(hold_second) + assert first_entered.wait(timeout=2) + was_serialized = not second_entered.wait(timeout=0.25) + release_first.set() + first_future.result(timeout=5) + second_future.result(timeout=5) + + assert was_serialized + assert second_entered.is_set() + + +def test_in_memory_locks_serialize_overlapping_user_ids() -> None: + """The process-local manager serializes intersecting user ID sets.""" + manager = InMemoryUserOperationLocks() + _assert_overlapping_operation_waits(manager, manager) + + +def test_sqlite_locks_serialize_distinct_manager_instances(tmp_path) -> None: + """Separate managers sharing a sidecar database serialize mutations.""" + database_path = str(tmp_path / "user-operation-locks.sqlite3") + first_manager = SqliteUserOperationLocks(database_path) + second_manager = SqliteUserOperationLocks(database_path) + _assert_overlapping_operation_waits(first_manager, second_manager) + + +def test_sqlite_lock_timeout_is_explicit_and_retryable(tmp_path) -> None: + """Contention exceeding the timeout raises the retryable domain error.""" + database_path = str(tmp_path / "user-operation-locks.sqlite3") + first_manager = SqliteUserOperationLocks(database_path) + impatient_manager = SqliteUserOperationLocks( + database_path, timeout_seconds=0.05 + ) + + with first_manager.hold("dup"): + with pytest.raises(UserOperationLockTimeout): + with impatient_manager.hold("dup"): + pytest.fail("contending operation unexpectedly acquired the lock") + + +def test_lock_manager_rejects_empty_user_ids() -> None: + """Empty identifiers never create an unscoped mutation lock.""" + manager = InMemoryUserOperationLocks() + with pytest.raises(ValueError): + with manager.hold(""): + pytest.fail("empty user ID unexpectedly acquired a lock") diff --git a/services/account_unification/tools/seed_config_store.py b/services/account_unification/tools/seed_config_store.py index 444916d..1412e01 100644 --- a/services/account_unification/tools/seed_config_store.py +++ b/services/account_unification/tools/seed_config_store.py @@ -1,10 +1,9 @@ -"""Seed a local SQLite KV config store for standalone/dev bring-up. +"""Seed a standalone SQLite KV configuration store for local development. -This writes the ``idp_config_entries`` rows the service reads at runtime, so a -developer can run the service without a full secret manager. In production the -same keys live in the platform KV. Values here are DEV PLACEHOLDERS. - - python tools/seed_config_store.py [--db PATH] [--namespace NS] +The tool writes the same two-word snake_case entries consumed by the service. +Values are development placeholders; production deployments populate the +platform KV and provide only the bootstrap pointer to the process. Registration +remains disabled unless its dedicated token is supplied explicitly. """ from __future__ import annotations @@ -16,33 +15,99 @@ from app.config import ( # noqa: E402 KEY_ALLOW_UNVERIFIED_LINK, + KEY_AUDIT_DATABASE_PATH, KEY_KEYCLOAK_CLIENT_ID, KEY_KEYCLOAK_CLIENT_SECRET, KEY_KEYCLOAK_REALM, KEY_KEYCLOAK_SERVER_URL, KEY_MERGE_CONFLICT_POLICY, + KEY_OPERATOR_API_TOKEN, + KEY_REGISTRATION_ACTION_LIFESPAN_SECONDS, + KEY_REGISTRATION_API_TOKEN, + KEY_REGISTRATION_CLIENT_ID, + KEY_REGISTRATION_REDIRECT_URI, ) from app.kv_store import SqliteKvStore # noqa: E402 -def main() -> int: - """Write development Keycloak settings into a local SQLite KV store.""" - parser = argparse.ArgumentParser() - parser.add_argument("--db", default="../../deploy/bootstrap/idp_config_store.db") +def _build_parser() -> argparse.ArgumentParser: + """Return the command-line parser for local configuration seeding.""" + parser = argparse.ArgumentParser( + description="Seed the Keyverse standalone SQLite KV store." + ) + parser.add_argument( + "--db", + default="../../deploy/bootstrap/idp_config_store.db", + ) parser.add_argument("--namespace", default="account_unification") parser.add_argument("--server-url", default="http://localhost:8080") parser.add_argument("--realm", default="cwl") - parser.add_argument("--client-id", default="account-unification-svc") - parser.add_argument("--client-secret", default="dev-placeholder-secret") - args = parser.parse_args() + parser.add_argument( + "--client-id", default="account-unification-svc" + ) + parser.add_argument( + "--client-secret", + default="dev-placeholder-secret", + ) + parser.add_argument("--operator-token", default="dev-operator-token") + parser.add_argument( + "--registration-token", + default="", + help="Enable local registration only when a dedicated token is supplied.", + ) + parser.add_argument( + "--registration-client-id", + default="naruon-web", + ) + parser.add_argument( + "--registration-redirect-uri", + default="https://naruon.example/auth/passkey-complete", + ) + parser.add_argument( + "--registration-action-lifespan-seconds", + default="900", + ) + parser.add_argument( + "--audit-database-path", + default="../../deploy/bootstrap/account_unification_audit.sqlite3", + ) + return parser + +def _registration_entries(args: argparse.Namespace) -> dict[str, str]: + """Return complete registration settings only when signup is enabled.""" + if not args.registration_token: + return {} + return { + KEY_REGISTRATION_API_TOKEN: args.registration_token, + KEY_REGISTRATION_CLIENT_ID: args.registration_client_id, + KEY_REGISTRATION_REDIRECT_URI: args.registration_redirect_uri, + KEY_REGISTRATION_ACTION_LIFESPAN_SECONDS: ( + args.registration_action_lifespan_seconds + ), + } + + +def main() -> int: + """Write development Keycloak settings into a local SQLite KV store.""" + args = _build_parser().parse_args() store = SqliteKvStore(args.db) - store.put(args.namespace, KEY_KEYCLOAK_SERVER_URL, args.server_url) - store.put(args.namespace, KEY_KEYCLOAK_REALM, args.realm) - store.put(args.namespace, KEY_KEYCLOAK_CLIENT_ID, args.client_id) - store.put(args.namespace, KEY_KEYCLOAK_CLIENT_SECRET, args.client_secret) - store.put(args.namespace, KEY_MERGE_CONFLICT_POLICY, "survivor_wins") - store.put(args.namespace, KEY_ALLOW_UNVERIFIED_LINK, "false") + try: + entries = { + KEY_KEYCLOAK_SERVER_URL: args.server_url, + KEY_KEYCLOAK_REALM: args.realm, + KEY_KEYCLOAK_CLIENT_ID: args.client_id, + KEY_KEYCLOAK_CLIENT_SECRET: args.client_secret, + KEY_MERGE_CONFLICT_POLICY: "survivor_wins", + KEY_ALLOW_UNVERIFIED_LINK: "false", + KEY_OPERATOR_API_TOKEN: args.operator_token, + KEY_AUDIT_DATABASE_PATH: args.audit_database_path, + **_registration_entries(args), + } + for entry_key, entry_value in entries.items(): + store.put(args.namespace, entry_key, entry_value) + finally: + store.close() print(f"seeded {args.db} namespace={args.namespace}") return 0