pal-e-docs

Review R2: Keycloak ISS realm: self-registration, phone field, SMTP

review-1864-r2-2026-07-17 Review

review approved
Project

Verdict: APPROVED

R2 re-review of board item #1864. All 7 R1 recommendations from review-1864-2026-07-17 have been addressed in the issue body rewrite.

Template Completeness

  • [x] Type -- Feature
  • [x] Lineage -- Related to intelligentstaffingsystems#59 (parent registration story)
  • [x] Repo -- Cross-repo clearly identified (pal-e-services for terraform, intelligentstaffingsystems for theme)
  • [x] User Story -- present
  • [x] Context -- explains what is already deployed (self-reg, SMTP via PR #178/#175), remaining work, and architectural decision change (CSS-only to FreeMarker)
  • [x] File Targets -- 6 files across 2 repos, all paths verified (see below)
  • [x] Feature Flag -- none (realm-level config, appropriate)
  • [x] Acceptance Criteria -- 7 items, 2 struck through as already deployed, 5 active
  • [x] Test Expectations -- 4 manual tests
  • [x] Constraints -- present, includes theme directory, parent theme, SMTP status, brand tokens, Keycloak version note
  • [x] Blockers -- none (correct, prerequisites are deployed)
  • [x] Checklist -- 2 PRs listed (one per repo), tests, no unrelated changes
  • [x] Related -- project, parent issue, merged PRs, arch note

Traceability

  • [x] story:registration label -- "Epic 1 (US-1.2-1.4)", Role: Visitor/Lead
  • [x] story note verified -- found in project-iss user-stories section
  • [x] arch:auth label -- present on board item
  • [x] arch note verified -- arch-auth-iss note exists in pal-e-docs (note: search_notes doesn't index it but get_note by slug succeeds). Note content includes FreeMarker decision: "CSS-only theme: register.ftl is the first FreeMarker override (planned for self-registration)."
  • [x] Forgejo issue -- ldraney/pal-e-services#184, open

File Targets

In ldraney/intelligentstaffingsystems:
  • [x] keycloak-theme/iss/login/register.ftl -- NEW file to create. Directory exists, confirmed: keycloak-theme/iss/login/ contains theme.properties, resources/css/login.css, resources/img/iss-logo.svg. Path is correct (R1 fix #1 applied).
  • [x] keycloak-theme/iss/login/resources/css/register.css -- NEW file to create. Parent directory resources/css/ exists with login.css. Path is correct.
  • [x] keycloak-theme/iss/login/theme.properties -- EXISTS. Current content: parent=keycloak, import=common/keycloak, styles=css/login.css. May need update to add register.css to styles list.
In ldraney/pal-e-services:
  • [x] terraform/variables.tf -- EXISTS (line 139, keycloak_realms variable). Confirmed: verify_email is NOT present. Needs new optional(bool, false) field added. (R1 fix #4 applied).
  • [x] terraform/keycloak.tf -- EXISTS (line 28, keycloak_realm resource). Confirmed: verify_email is NOT wired. Needs verify_email = each.value.verify_email added.
  • [x] terraform/k3s.tfvars -- EXISTS (symlink to secrets). ISS realm defined with registration_allowed = true, roles, default_roles, and SMTP block. No verify_email currently set. Needs verify_email = true.

Repo Placement

OK. Cross-repo nature is correctly documented in the Repo section and the Checklist section specifies separate PRs for each repo (R1 fix #2 applied). The Forgejo issue is filed on pal-e-services, which hosts one half of the work (terraform). The theme work in intelligentstaffingsystems is tracked via the same issue with a separate PR -- acceptable for tightly coupled cross-repo work.

Dependencies

  • Board item #1831 (ISS#59, Rails-side registration) is in backlog sprint:A -- depends on this ticket. Documented in Lineage: "This is the infrastructure prerequisite -- realm config and theme must be in place before the Rails app can wire the post-registration flow."
  • Board item #1786 (#6 Keycloak OIDC auth integration) -- done. Completed prerequisite.
  • Board item #1785 (#17 Keycloak login theme ISS branding) -- done. Established theme directory structure at keycloak-theme/iss/.
  • PR #178 (merged) -- already deployed self-registration and SMTP.
  • PR #175 (merged) -- established Postmark SMTP.
  • No in_progress items blocking this ticket.

Acceptance Criteria

7 criteria listed, 2 marked as already deployed (R1 fix #3 applied). 5 active ACs:
  • AC 2 (register.ftl with phone field) -- verifiable by navigating to registration page
  • AC 3 (phone stored as Keycloak user attribute) -- verifiable via Keycloak admin console
  • AC 5 (verify_email in terraform) -- verifiable by terraform plan and manual email test
  • AC 6 (ISS brand match) -- colors specified (navy #1b2a4a, gold #996d13), verifiable visually
  • AC 7 (existing login still works) -- regression test, verifiable
All active criteria are agent-verifiable or manually testable with clear success conditions.

Blast Radius

  • Adding register.ftl changes the ISS theme from CSS-only to FreeMarker-based. This architectural decision change is explicitly documented in the Context section and acknowledged in the arch-auth-iss note (R1 fix #5 applied). FreeMarker templates are Keycloak-version-specific; the Constraints section notes this.
  • The verify_email addition to the terraform module is additive: new optional(bool, false) field means all existing realms default to false (no behavioral change). Only the ISS realm will set true.
  • No blast radius on SMTP or self-registration (already live).
  • Observation (non-blocking): The ISS realm in k3s.tfvars does not set login_theme = "iss". Other realms (westside, landscaping) do set their login_theme via terraform. If the ISS theme was configured via Keycloak admin console rather than terraform, it works but is not infrastructure-as-code. The implementing agent should verify the theme is active before creating register.ftl -- if not, adding login_theme = "iss" to the ISS realm config may be needed.

Decomposition Assessment

5 active ACs across 2 repos, 6 file targets. Thresholds:
  • >3 file targets across >2 repos: 6 files across 2 repos -- repo count (2) does not exceed threshold (>2)
  • >5 acceptance criteria: 5 active ACs -- does not exceed threshold (>5)
  • Estimated agent work: terraform changes ~2 min, theme work ~3-4 min = ~5-6 min total -- borderline
The issue body addresses cross-repo coordination via separate checklist items for each PR. The work is tightly coupled (theme needs realm config, realm config enables theme). No decomposition needed -- single ticket with two PRs is the correct approach.

R1 Issues Resolution

# R1 Recommendation Status
1 [BODY] Fix file paths: keycloak/themes/ to keycloak-theme/ FIXED -- all paths now use keycloak-theme/iss/login/
2 [BODY] Fix repo for FreeMarker/CSS work FIXED -- Repo section identifies both repos, file targets split by repo
3 [BODY] Mark already-deployed ACs FIXED -- AC 1 and AC 4 struck through with "already deployed" note
4 [BODY] Replace vague file target with terraform paths FIXED -- variables.tf, keycloak.tf, k3s.tfvars all specified with purpose
5 [BODY] Document FreeMarker architectural decision change FIXED -- Context section explains the change from CSS-only to template-based
6 [SCOPE] Create architecture note arch-auth FIXED -- arch-auth-iss note exists in pal-e-docs with full content including FreeMarker decision
7 [DECOMPOSE] Split cross-repo work ADDRESSED -- cross-repo checklist with separate PRs, within decomposition thresholds

Recommendation

No action needed. All R1 issues resolved. Ticket is ready for implementation.
Implementation note: The implementing agent should verify that login_theme = "iss" is active on the ISS Keycloak realm before creating register.ftl. If the theme was set via admin console rather than terraform, consider adding login_theme = "iss" to the ISS realm in k3s.tfvars for infrastructure-as-code consistency.