Add per-request identity (P2) and OIDC authentication (P3) - #109
Merged
Merged
Conversation
P2 - the sharpest end of hosting this app: ME was a module-level global, resolved once at process startup from `gh api user`. Hosted, that is one identity shared by every concurrent visitor on ThreadingHTTPServer, so a save could be silently mis-attributed in git to whoever else's request last touched the global. Replaced with threading.local() (_LOCAL) and a current_login() reader; do_GET/do_POST resolve and set it once per request via _set_current_login(). Laptop/CLI path unchanged: unset AUTH_ENABLED means current_login() always resolves to _STARTUP_ME, the same identity main() has always resolved once at startup. P3 - OIDC Authorization Code + PKCE, deliberately mirroring ops-tools' own login gate pattern (operator's instruction): same discovery-doc/ token/userinfo flow, same httpOnly+Secure+SameSite signed session cookie over an in-memory store, same welcome/not-authorized pre-auth pages. Entirely off unless FKB_OIDC_CLIENT_ID is set. The identity bridge: OIDC gives an email; this app's authorization (contributors.json, agent-owners.json) is keyed on a GitHub username. FKB_IDENTITY_MAP (host-side JSON file, outside git) maps casefolded email -> GitHub login. An authenticated user who is unmapped, or whose mapped login isn't a real contributor, gets "Not authorized" - never a silent fallback to an admin/default identity. Also: corrected is_admin()'s docstring, which claimed hiding the UI "does not hide the data" - true on a laptop (everyone has the full checkout), false for a hosted visitor whose only access IS the app. Fixed auto_commit_message()'s precedence so hosted PR titles carry the real per-request reviewer instead of the shared container-wide git identity, restoring the "several reviewers" readability the function's own docstring describes. Added /healthz - reachable with no session, minimum surface, same pattern as ops-tools' own. New scripts/check_auth_identity.py: unset-vs-hosted behavior over real HTTP, plus session/cookie-tamper/thread-isolation checks in isolation (a live identity gateway isn't available to a CI check).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
MEmodule global ->threading.local()+current_login(). Fixes the mis-attribution risk of ThreadingHTTPServer sharing one identity across concurrent hosted visitors. Laptop/CLI path (AUTH_ENABLEDoff) is unaffected -current_login()resolves to the same_STARTUP_MEMEused to be.FKB_OIDC_CLIENT_IDis set.FKB_IDENTITY_MAP(host-side, outside git) bridges OIDC email -> GitHub login for this app's existingcontributors.json/agent-owners.jsonauthorization. Unmapped or non-contributor logins get "Not authorized," never a silent fallback.is_admin()'s docstring (was laptop-only-true), fixedauto_commit_message()'s identity precedence for hosted PR titles, added/healthz.Test plan
python3 scripts/check_auth_identity.py- unset-vs-hosted-gate over real HTTP, plus session sign/tamper and thread-isolation checks (two concurrent "requests" never see each other's identity)check_base_path.py,check_write_boundary.py,check_ui_js.py,check_admin_paths.py,review_status.py --check- all still pass