pal-e-docs

Review: streamlit-keycloak in-app integration (#961)

review-961-2026-04-11 Review

review ready
Project

Verdict: READY

Board item #961 — Forgejo issue forgejo_admin/westside-streamlit#11 — Child B of parent #7 Keycloak decomposition.

Template Completeness

  • [x] Type (Feature)
  • [x] Lineage with parent:7
  • [x] Repo
  • [x] User Story (two-voice: Marcus + operator)
  • [x] Context — explicitly justifies streamlit-keycloak over oauth2-proxy
  • [x] File Targets (requirements.txt + app.py)
  • [x] Acceptance Criteria (8 items, all testable)
  • [x] Test Expectations
  • [x] Constraints
  • [x] Checklist
  • [x] Related (parent, siblings, blockers)

Traceability

  • [x] story:reachable — verified: story-westside-streamlit-reachable note exists
  • [x] arch:keycloak-oidc label present
  • [ ] arch note MISSING — arch-keycloak-oidc not found in pal-e-docs. [SCOPE] Create architecture note arch-keycloak-oidc (shared across Children A/B/C, non-blocking for this ticket).
  • [x] parent:7 label links to umbrella
  • [x] Forgejo issue #11 — open

File Targets

  • [x] requirements.txt — verified via Forgejo raw API. Currently pins streamlit==1.56.0, psycopg[binary]==3.3.3, pandas==3.0.2. Appending streamlit-keycloak is clean.
  • [x] app.py — verified. Imports os/streamlit/psycopg/pandas, defines q() using psycopg.connect(DB), and the FIRST q() call (counts = q(...)) fires immediately after st.title. Correct insertion point for the auth wrapper is above st.set_page_config / st.title so that no SQL runs for non-admin users. Defense-in-depth guarantee is achievable with the proposed wrapper.
Targets are specific enough for an agent to act on without guessing.

Repo Placement

OK — all changes in forgejo_admin/westside-streamlit. Child A (Keycloak client) is terraform, Child C (Secret+env) is pal-e-deployments kustomize. Clean three-repo separation with an explicit env var contract gluing them together.

Dependencies

  • [pending] #10 Child A Keycloak client resource — needed for full E2E, NOT needed for local dev
  • [pending] #12 Child C deployment Secret + env wiring — needed for full E2E, NOT needed for local dev
  • [pending] #6 Tailscale funnel — hard-blocks public exposure, does not block code development
Cross-child env contract is explicit (KC_SERVER_URL, KC_REALM, KC_CLIENT_ID, KC_CLIENT_SECRET) with example values. Child B code CAN be developed and committed against a local .env before A/C land — ticket explicitly documents this path in AC#8. Ordering is understood and tracked.

Acceptance Criteria

All 8 criteria are testable by a fresh-context agent. Critical items verified: AC#2 pins the "auth before any q()" defense-in-depth rule; AC#6 requires JWT claims in st.session_state for downstream role-based filtering; AC#7 requires explicit failure mode on missing env vars; AC#8 mandates local-dev docs in the PR body. Test Expectations adds query-log inspection to prove non-admin flow executes ZERO SELECT statements — belt and suspenders.

Blast Radius

Narrow. Single repo, two files, purely additive. No sibling Streamlit apps exist in the platform. Rollback = revert PR. One minor cross-doc inconsistency: story-westside-streamlit-reachable mentions a westside-ops role in its AC, while this ticket checks the admin realm role. Not blocking — ticket's choice is defensible — but flagged as a scope recommendation.

Decomposition Assessment

Three-thing limit: 2 discrete changes (dep add, app.py wrapper). Five-minute rule: estimated < 5 min agent work — one-line requirements.txt append plus ~30 lines of auth wrapper at the top of app.py. 1 repo, 2 files, 8 AC. Passes comfortably. No further splitting warranted; this ticket is itself the product of parent #7's decomposition.

Specific Router Checks

  • [x] Picks streamlit-keycloak (in-app) NOT oauth2-proxy (sidecar) — justified with three reasons: JWT claim visibility in st.session_state, future role-based data filtering, Python-native alignment.
  • [x] Admin-role check runs BEFORE any q() SQL call — AC#2 + Test Expectations enforce; file target analysis confirms insertion point is above the first q().
  • [x] Env var contract (KC_SERVER_URL, KC_REALM, KC_CLIENT_ID, KC_CLIENT_SECRET) defined clearly for A and C to implement against — example values provided.
  • [x] Denied-page UX specified — st.error("Access denied — admin role required") + st.stop(), friendly not raw 403.
  • [x] session_state usage correct — AC#6 requires JWT claims land there for downstream filtering.
  • [x] Ticket code DEVELOPABLE before A/C land — explicit local .env path.

Recommendation

  • [SCOPE] Create architecture note arch-keycloak-oidc (shared across #10/#11/#12; non-blocking for this ticket but should land before umbrella #7 closes).
  • [SCOPE] Reconcile role name between story-westside-streamlit-reachable (westside-ops) and this ticket (admin). Suggest updating the story to match the ticket's admin realm role.
Both recommendations are non-blocking. Scope is solid, file targets verified, traceability triangle effectively complete (story verified, arch labeled, Forgejo issue open), defense-in-depth correct, cross-child contract explicit, decomposition passes. READY for todo → next_up.