pal-e-docs

Review: Keycloak realm, client, users, and login theme

review-1693-2026-07-02 Review

review approved
Project

Verdict: APPROVED

Re-review of board item #1693. All 7 issues from the previous NEEDS_REFINEMENT review have been verified fixed. One new documentation issue found in arch notes (non-blocking). Ticket is ready for implementation.

Previous Review Fixes (all verified)

  • [x] Theme file paths corrected from keycloak-theme/ to keycloak/themes/kalshi-assistant/
  • [x] ConfigMap reference removed (themes are directly mounted, no ConfigMap pattern exists)
  • [x] AC #6 replaced: "OmniAuth callback works" (unachievable) changed to "Keycloak client configured with correct redirect URI for future Rails OmniAuth integration (verified in Keycloak admin console)"
  • [x] Dependencies added to Lineage: depends on #7 (service onboarding), soft dep on #1 (Rails scaffold)
  • [x] Cross-Repo Note section added explaining PRs target pal-e-services and pal-e-platform, not kalshi-assistant
  • [x] Repo description cleaned: "theme ConfigMap" changed to "theme files"
  • [x] Architecture note arch-keycloak-kalshi-assistant created
  • [x] Architecture note arch-auth created

Template Completeness

Validated against template-issue-feature.
  • [x] Type — Feature
  • [x] Lineage — depends on #7, notes #1
  • [x] Repo — ldraney/pal-e-services + ldraney/pal-e-platform
  • [x] Cross-Repo Note — bonus section, clarifies PR targeting
  • [x] User Story — login with Keycloak SSO
  • [x] Context — follows landscaping-assistant model, Auth Code + PKCE
  • [x] File Targets — 3 files across 2 repos
  • [x] Feature Flag — none
  • [x] Acceptance Criteria — 6 items
  • [x] Test Expectations — tofu plan + manual test
  • [x] Constraints — landscaping pattern, registration, PKCE, no direct grants
  • [x] Checklist — PR, tofu plan, theme, no unrelated changes
  • [x] Related — project, story, SOP references

Traceability

  • [x] story:app-experience label — App Experience
  • [x] story note verified — "app-experience" found in project-kalshi-assistant user-stories section
  • [x] arch:keycloak label — Keycloak configuration
  • [x] arch note verified — arch-keycloak-kalshi-assistant exists in pal-e-docs (note: slug uses -kalshi-assistant suffix because arch-keycloak was taken)
  • [x] arch:auth label — Authentication architecture
  • [x] arch note verified — arch-auth exists in pal-e-docs
  • [x] Forgejo issue — ldraney/kalshi-assistant#10, open

File Targets

  • [x] terraform/k3s.tfvars (pal-e-services) — verified: file exists (symlink to secrets), keycloak_realms at line 40, keycloak_clients at line 92, keycloak_users at line 292. Landscaping-assistant pattern at lines 139-166 confirmed as reference.
  • [x] keycloak/themes/kalshi-assistant/login/theme.properties (pal-e-platform) — to create. Matches convention: keycloak/themes/landscaping/login/theme.properties exists.
  • [x] keycloak/themes/kalshi-assistant/login/resources/css/login.css (pal-e-platform) — to create. Matches convention: keycloak/themes/landscaping/login/resources/css/login.css exists.

Repo Placement

OK. Issue filed on ldraney/kalshi-assistant (the project repo), but work targets ldraney/pal-e-services (Terraform config) and ldraney/pal-e-platform (theme files). Cross-Repo Note explicitly documents this. PRs should target those repos. This is acceptable for project-level infrastructure work.

Dependencies

  • #7 (Service onboarding — namespace, Harbor, ArgoCD, Tailscale funnel) — board item 1694, sprint:1, backlog. Hard dependency: namespace and ArgoCD app must exist first. Documented in Lineage.
  • #1 (Rails app scaffold with Kalshi API client) — board item 1686, sprint:1, backlog. Soft dependency: needed for full OAuth flow testing only. Documented in Lineage.

Acceptance Criteria

6 criteria, all testable:
  • AC 1-4: verifiable via tofu plan (realm, client, redirect URIs, seeded user)
  • AC 5: manual verification (login page branding)
  • AC 6: verifiable in Keycloak admin console (redirect URI configuration)
All criteria are achievable within this ticket's scope (AC #6 was correctly scoped down from the previous "OmniAuth callback works" version).

Blast Radius

Low. All changes are additive:
  • k3s.tfvars: adding new realm/client/user entries alongside existing ones. No modifications to landscaping, westside-basketball, or platform realms.
  • Theme files: new directory under keycloak/themes/. No changes to existing themes.

Decomposition Assessment

3 file targets across 2 repos (does not exceed >3 across >2 threshold). 6 AC (borderline at >5 threshold, but work is cohesive: one Terraform block + two theme files). Estimated agent time ~3-4 minutes. No decomposition needed.

New Finding: Arch Notes Contradict Issue Spec

Both arch-keycloak-kalshi-assistant and arch-auth describe ROPC/direct grant with a public client, but the issue spec and the verified landscaping-assistant pattern (k3s.tfvars lines 139-166) both specify Auth Code + PKCE with a confidential client and direct_access_grants_enabled = false.
Specific contradictions:
  • arch-keycloak-kalshi-assistant: diagram shows IOS -->|"direct grant"| CLIENT, components table says "Direct grant enabled. Public client", key decisions say "Direct grant for mobile — Public client with ROPC"
  • arch-auth: diagram shows App->>KC: Direct grant (username + password), components table says "Direct grant flow (no browser redirect)", key decisions say "Direct grant over redirect flow"
The issue spec is correct (Auth Code + PKCE, confidential, no ROPC). The arch notes should be updated to match. This is non-blocking because the Forgejo issue is the implementation authority, and it is correct.

Recommendation

  • [SCOPE] Update arch-keycloak-kalshi-assistant: change "direct grant / public client / ROPC" references to "Auth Code + PKCE / confidential client / standard flow". Fix diagram, components table, and key decisions section.
  • [SCOPE] Update arch-auth: change "direct grant (username + password)" to "Auth Code + PKCE via ASWebAuthenticationSession". Fix diagram, components table, and key decisions section.