pal-e-docs

Review: Add westside-basketball Keycloak client and seed admin user

review-1608-2026-06-24-r3 Review

review ready
Project

Verdict: APPROVED

Third review (re-review after full rewrite). The rewrite resolves all critical issues from reviews 1 and 2. The scope is now correctly targeted at pal-e-services (which has the mrparkers/keycloak v5.0 provider), with a single file target (terraform/k3s.tfvars), and the client creation gap is closed -- this ticket IS the client creation ticket.

Prior Review Issues -- Resolution Status

  • [x] [BODY] Wrong repo (pal-e-platform) -- FIXED. Now correctly targets ldraney/pal-e-services.
  • [x] [BODY] Invalid file targets (no keycloak provider in pal-e-platform) -- FIXED. Targets terraform/k3s.tfvars in pal-e-services, which has the mrparkers/keycloak provider in providers.tf and for_each resources in keycloak.tf.
  • [x] [BODY] Client creation gap (nobody creates the client) -- FIXED. This ticket now explicitly creates both the keycloak_clients entry and keycloak_users entry. The title and scope match.
  • [ ] [SCOPE] story:WS-AUTH not in project-westside-basketball user-stories -- PERSISTS but acceptable (see Traceability).
  • [ ] [SCOPE] arch-keycloak-oidc note missing -- PERSISTS but acceptable (see Traceability).

Template Completeness

  • [x] Type -- Feature
  • [x] Lineage -- prerequisite for #54, story WS-AUTH
  • [x] Repo -- ldraney/pal-e-services
  • [x] User Story -- Rails app needs OIDC client and seed users
  • [x] Context -- excellent: references existing realm, existing provider, landscaping-assistant pattern, explicitly states what exists and what is missing
  • [x] File Targets -- present with both "should modify" and "should NOT touch" sections
  • [x] Feature Flag -- none (correct for infra)
  • [x] Acceptance Criteria -- 6 AC (within limits)
  • [x] Test Expectations -- 4 items including tofu plan command
  • [x] Constraints -- 4 constraints, all actionable
  • [x] Checklist -- present
  • [x] Related -- references #54, #56, landscaping-assistant pattern
All template sections present and well-populated.

Traceability

  • [x] story:WS-AUTH label present
  • [ ] story note MISSING -- story:WS-AUTH does not exist in project-westside-basketball user-stories section (which uses WS-S{N} convention). However, WS-AUTH is a coherent story grouping used across 3 related items (#1605, #1607, #1608) covering the full auth stack (DNS, Keycloak client, Rails integration). This is foundational infrastructure work. [SCOPE] Create user story entry for WS-AUTH on project-westside-basketball, or map these items to an existing story (closest match: WS-S3 "manage Keycloak realms").
  • [x] arch:keycloak-oidc label present
  • [ ] arch note MISSING -- arch-keycloak-oidc does not exist in pal-e-docs. [SCOPE] Create architecture note arch-keycloak-oidc documenting the Keycloak OIDC component (realms, clients, providers, tfvars pattern, for_each resource design).
  • [x] Forgejo issue -- ldraney/westside-basketball#57, open
The two [SCOPE] items (story note, arch note) are documentation gaps that do not block implementation. The work itself is well-scoped and the labels provide correct traceability grouping. These notes should be created as part of sprint prep but do not require another review cycle.

File Targets

  • [x] terraform/k3s.tfvars (symlink to ~/secrets/pal-e-services/k3s.tfvars) -- VERIFIED. File exists. keycloak_clients map starts at line 73, currently has 3 entries (pal-e-app, pal-enterprises, landscaping-assistant). No westside-basketball client entry exists. keycloak_users map starts at line 249, currently has 4 entries (all in landscaping realm). No westside-basketball/* user entry exists.
  • [x] terraform/keycloak.tf -- VERIFIED should NOT touch. keycloak_openid_client resource (line 137) uses for_each = var.keycloak_clients. keycloak_user resource (line 214) uses for_each = var.keycloak_users. Both handle new entries automatically.
  • [x] terraform/variables.tf -- VERIFIED should NOT touch. keycloak_clients variable type supports all fields mentioned in AC (realm_key, public_client, pkce_code_challenge_method, include_realm_roles_mapper, use_refresh_tokens, valid_redirect_uris, web_origins, post_logout_redirect_uris). keycloak_users variable type supports realm_key, username, first_name, last_name, email, initial_password, roles.
  • [x] terraform/providers.tf -- VERIFIED. mrparkers/keycloak v5.0 provider is configured.
  • [x] westside-basketball realm -- VERIFIED exists in k3s.tfvars (line 30) with roles [admin, coach, player], login_theme "westside", and SMTP configured.
  • [x] landscaping-assistant pattern -- VERIFIED as reference. Confidential client (public_client = false), PKCE S256, include_realm_roles_mapper = true, use_refresh_tokens = true, redirect URIs for both Tailscale and public domain.

Repo Placement

Issue filed on ldraney/westside-basketball, work targets ldraney/pal-e-services. This is a deliberate cross-repo placement: the issue is filed on the consuming project's board (westside-basketball) because it tracks auth infrastructure for that project, but the PR will be opened on pal-e-services where the file lives. The issue body explicitly states ### Repo: ldraney/pal-e-services and the checklist says "PR opened on pal-e-services". This is correct and clear.

Dependencies

  • [x] depends:56 -- Set up westsidekingsandqueens.com via Hetzner edge proxy (board item #1607, currently next_up, sprint:3). Public redirect URIs require the domain to resolve. Correctly declared. This item must complete before #57 can be fully validated.
  • [x] Downstream: Board item #1605 (issue #54, "Add Keycloak auth with admin/coach/player roles") has depends:57 label. Correctly declares that Rails auth depends on this client existing. Dependency chain is clean: #56 (DNS) -> #57 (Keycloak client) -> #54 (Rails auth).
  • [x] Sprint ordering: #56 is sprint:3, #57 is sprint:4, #54 is sprint:4. Sprint boundaries respect the dependency chain.
Note: #57 can be partially worked (Tailscale redirect URIs) before #56 completes. The public domain URIs are additive and can be configured now even if the domain doesn't resolve yet -- tofu apply will succeed regardless. Full validation (AC2 end-to-end login via public URL) requires #56.

Acceptance Criteria

6 AC, all testable by an agent:
  • AC1-4: Structural checks on tfvars entries -- verifiable via grep/read after edit
  • AC5: User entry with correct realm, role, credentials -- verifiable via grep/read
  • AC6: tofu plan shows only new resources -- verifiable via command output
Test expectations include tofu plan (2-3 new resources), tofu apply, and Keycloak admin UI verification. The run command is provided: cd ~/pal-e-services/terraform && tofu plan -var-file=k3s.tfvars. All criteria are machine-verifiable.

Blast Radius

Low. The change is purely additive -- adding new map entries to keycloak_clients and keycloak_users. Existing clients (pal-e-app, pal-enterprises, landscaping-assistant) are not affected. Existing users (landscaping realm) are not affected. The for_each pattern means Terraform will create new resources without modifying existing ones. tofu plan AC confirms no drift on existing resources.

Decomposition Assessment

1 file target, 1 repo, 6 AC (at the limit but all are structural checks on the same file), estimated agent work well under 5 minutes. No decomposition needed.

Recommendation

No blocking issues. The two [SCOPE] items below are documentation housekeeping that should be addressed but do not block implementation:
  • [SCOPE] Create user story entry for WS-AUTH on project-westside-basketball user-stories section (or map to existing WS-S3).
  • [SCOPE] Create architecture note arch-keycloak-oidc documenting the Keycloak OIDC component.
These are the same [SCOPE] items from reviews 1 and 2. They are documentation gaps, not implementation blockers. The ticket scope itself is now solid.