pal-e-docs

Review: Update Keycloak redirect URIs to westsidekingsandqueens.com

review-1608-2026-06-24 Review

review needs-refinement
Project

Verdict: NEEDS_REFINEMENT

Template Completeness

  • [x] Type -- Feature
  • [x] Lineage -- references #54, DNS dependency, story WS-S7
  • [x] Repo -- cross-repo: pal-e-platform + westside-basketball
  • [x] User Story -- As Marcus logging into westsidekingsandqueens.com...
  • [x] Context -- solid background referencing landscaping-assistant precedent
  • [x] File Targets -- 3 targets listed (see verification below)
  • [x] Feature Flag -- none (appropriate for config change)
  • [x] Acceptance Criteria -- 6 criteria listed
  • [x] Test Expectations -- manual test steps
  • [x] Constraints -- references landscaping-assistant patterns
  • [x] Checklist -- present
  • [x] Related -- references landscaping-assistant issues and dependencies
All required feature template sections are present.

Traceability

  • [ ] story:WS-AUTH label -- MISMATCH: board label says story:WS-AUTH but issue body says WS-S7. Neither WS-AUTH nor a matching auth story exists in the project-westside-basketball user-stories section. WS-S7 is "As an admin, I want to send branded email announcements" which does not match this ticket's scope. The closest story would be about auth/login flow but none exists yet. [SCOPE] Create a proper user story (e.g. WS-S34 or similar) for auth redirect/login flow on the project-westside-basketball user-stories section, then update the board label to match.
  • [ ] arch:keycloak-oidc label -- arch note MISSING. Search for arch-keycloak-oidc in pal-e-docs returned no results. [SCOPE] Create architecture note arch-keycloak-oidc for the Keycloak OIDC component.
  • [x] Forgejo issue -- ldraney/westside-basketball#57, state: open

File Targets

  • [ ] pal-e-platform Keycloak Terraform client -- ISSUE: No Keycloak client resource for westside-basketball exists in Terraform yet. The Keycloak module at pal-e-platform/terraform/modules/keycloak/ only defines infrastructure (namespace, deployment, service, themes), not client/realm resources. Per pal-e-platform's consolidation.md, old clients (westside-app, westside-spa) were destroyed and no client for westside-ror/westside-basketball exists. This is issue #54's job, not this ticket's.
  • [ ] westside-basketball/config/initializers/omniauth.rb -- ISSUE: File does not exist. No OmniAuth configuration exists anywhere in westside-basketball. No keycloak references found in the codebase at all. This file would need to be created, not modified. This is also issue #54's responsibility.
  • [ ] westside-basketball deployment config (KEYCLOAK_ISSUER) -- ISSUE: Neither k8s/dev.yaml nor docker-compose.yml contain any Keycloak env vars. These would need to be added, which is again issue #54's scope.
  • [x] Landscaping-assistant reference pattern -- VERIFIED: landscaping-assistant/config/initializers/omniauth.rb exists and shows the target pattern (dynamic redirect_uri using KEYCLOAK_URL, KEYCLOAK_REALM, KEYCLOAK_CLIENT_ID, KEYCLOAK_CLIENT_SECRET, APP_URL env vars).

Repo Placement

The issue correctly identifies cross-repo scope (pal-e-platform for Terraform, westside-basketball for Rails config). However, the Forgejo issue is filed only on westside-basketball. Since most file targets actually belong to prerequisites (#54 Keycloak auth setup), the scope of THIS ticket may reduce to just Terraform redirect URI additions + verifying dynamic redirect_uri works with the public domain -- but only after #54 lands.

Dependencies

  • depends:56 (board label) -- "Set up westsidekingsandqueens.com via Hetzner edge proxy" -- board item 1607, currently in backlog. This itself depends on godaddy-tofu.
  • Undocumented dependency on #54 -- "Add Keycloak auth with admin/parent/player roles" -- board item 1605, currently in backlog. The issue body mentions this in Related but NOT in the depends label. This ticket cannot work without #54 landing first because no Keycloak client, no OmniAuth config, and no Keycloak env vars exist yet.
  • Dependency chain: #54 (Keycloak auth) must land first, then #56 (DNS/proxy), then this ticket (#57). All three are in backlog.

Acceptance Criteria

6 acceptance criteria listed. They are generally testable but several are only verifiable after #54 and #56 land:
  • AC1 (redirect URIs in Keycloak client) -- testable via Terraform plan, but requires #54 to create the client first
  • AC2 (dynamic redirect_uri) -- testable in code review, but requires omniauth.rb to exist (#54)
  • AC3 (end-to-end login flow) -- requires both #54 and #56 to be done
  • AC4 (issuer validation) -- requires #54
  • AC5 (auth.palinks.app accessible publicly) -- this is infrastructure, may already exist or may need separate work
  • AC6 (Tailscale fallback) -- requires #54
6 AC exceeds the 5-criterion decomposition threshold. However, this is borderline -- the criteria are tightly coupled and the actual scope (once #54 and #56 exist) is small: add a redirect URI to Terraform + verify the flow. Decomposition is NOT recommended; instead, the scope should be narrowed to only the redirect URI changes that are incremental to #54.

Blast Radius

The landscaping-assistant precedent (issues #259 and #246) shows this exact pattern has been solved before. The blast radius is limited to:
  • Keycloak client config for westside-basketball only (other clients unaffected)
  • OmniAuth initializer for westside-basketball only
  • No other services consume this Keycloak client
Low blast radius once dependencies are in place.

Decomposition Assessment

6 AC (exceeds 5 threshold), cross-repo (2 repos), but the file targets are largely invalid because they belong to prerequisite #54. Once #54 lands, the actual scope of this ticket shrinks to 1-2 files (Terraform redirect URI addition + verification). No decomposition needed -- the ticket needs scope narrowing, not splitting.

Recommendations

  • [LABEL] Add depends:54 to the board item labels. Issue #54 (Keycloak auth) is an undocumented hard dependency -- no Keycloak client, no OmniAuth config, and no Keycloak env vars exist without it.
  • [BODY] Narrow file targets: remove omniauth.rb creation and KEYCLOAK_ISSUER env var setup (those belong to #54). This ticket's file targets should be: (1) pal-e-platform Terraform -- add westsidekingsandqueens.com redirect URI to the westside-basketball Keycloak client (created by #54), (2) verify/adjust OmniAuth dynamic redirect_uri if #54 didn't already handle it.
  • [BODY] Reduce acceptance criteria to only what is incremental to #54: redirect URI addition in Terraform, end-to-end login via public URL, Tailscale fallback still works. Remove criteria that #54 should satisfy (dynamic redirect_uri, issuer validation).
  • [SCOPE] Create user story entry on project-westside-basketball user-stories section for auth/login flow. The board label story:WS-AUTH does not match any existing story. The issue body references WS-S7 which is about email announcements, not auth.
  • [SCOPE] Create architecture note arch-keycloak-oidc for the Keycloak OIDC component.
  • [LABEL] Fix story label from story:WS-AUTH to whatever new story ID is created for auth/login flow.