feat: authentication and role-based repo access - #9
Merged
Conversation
Equalize argon2 cost for unknown-username logins against a precomputed dummy hash so the store-lookup miss no longer returns faster than a known-user/wrong-password attempt (username enumeration via timing). Wrap the remaining unguarded store calls (create_session in login; delete_session in logout; upsert_user/delete_user_sessions in change-password) so any store error surfaces as 503, never a 500, matching the plan's fail-closed constraint.
_StubAuth._client() now raises immediately instead of falling through to AuthLayer's real db.get_client(), which the /chat handler's non-admin role-scope lookup was silently reaching over the network -- ~21s stalls against an unreachable-but-not-refusing TIMEPLUS_HOST. The fail-closed except-Exception -> frozenset() path is now exercised deterministically with zero I/O, matching the rest of test_server.py.
…-required 403 handling in apiFetch
Documents the auth/roles feature (login model, seeded admin + forced change, name@ref role access lists, user/role API, break-glass DB reset, TPK_SESSION_TTL) and the dedicated tpk DB user / required TIMEPLUS_PASSWORD, and removes every stale TPK_ADMIN_TOKEN/X-Admin-Token reference now that /api is always admin bearer-token gated. Also fixes a real bug hit while running the live E2E gate against a stack built from this branch: unquoted backticks in a heredoc comment in tpk-entrypoint.sh were interpreted as shell command substitution, producing spurious "command not found" errors on every container start (harmless to the generated users.d override, but noisy).
Post-final-review fixes on the auth & roles branch: - App.tsx mount effect now catches /auth/me network failures and still sets checked=true, falling through to the login gate instead of rendering a permanent blank page. - App.tsx logout() always clears the local session in a finally block, even if the /auth/logout request throws. - server.py's chat handler runs the non-admin role lookup via run_in_threadpool so a slow/unreachable store no longer blocks the event loop for every concurrent request; fail-closed frozenset() behavior on error is preserved. - Users.tsx add-user form no longer defaults to the admin role; role selection is now required (least privilege).
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
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.
Closes #8. Closes #6 (eager
tpk servebootstrap seeds theadminaccount on startup).Summary
Adds authentication and role-based repo access to
tpk serve:src/tpk/auth.py):kg_users/kg_roles/kg_sessionsmutable streams, argon2id password hashing, seededadmin/changemeaccount with forced password change, session tokens with configurable TTL (TPK_SESSION_TTL, default 24h)./authrouter:login,logout,me,change-password. Every non-/auth, non-/healthzroute requires a valid bearer session; amust_change_passworduser gets403 password_change_requireduntil they change it. Login is timing-safe against username enumeration (dummy argon2 verify on unknown usernames)./api/*route (corpus management,/api/users*,/api/roles*) now requires an authenticatedadminbearer token — replaces the old optionalTPK_ADMIN_TOKEN/X-Admin-Tokenheader scheme entirely. Last-admin protection (can't demote/disable/delete the only remaining admin). Role deletion blocked while still assigned to a user.entry_keys(name@ref, e.g.docs@main). AROLE_SCOPEcontextvar threads the caller's role scope throughKnowledgeGraph's query paths and the chat agent's tools, so a non-admin user's chat/search is provably restricted to their role's entries —adminis reserved and always unscoped.must_change_passwordusers, and adds a Users tab (admin-only) for user/role CRUD, including checkbox-basedentry_keysselection when building a role.tpkDB user:docker composenow provisions atpkDB user and password-locks the built-indefaultuser with the sameTIMEPLUS_PASSWORD(now required), instead of leavingdefaultopen. Bothtpkandagentservices connect astpk.E2E evidence (live stack built from this branch)
Ran
docker compose build && docker compose up -dfrom this branch with fresh, uniquely-named compose volumes (no reused state from prior stacks).POST /auth/login {admin, changeme}→ 200,must_change_password: true./api/reposwith that token →403 password_change_required;/auth/change-passwordclears it;/api/reposthen → 200./api/reposand/chat→ 401. Non-admin/api/repos→ 403.docs(semantic) andproton-enterprise(code-only) via the management API using the admin token, polling/api/jobstook.docs-only(entry_keys: ["docs@main"]) and usercust; confirmed role-scoped chat answers docs questions but never citesproton-enterprise, while an admin asking the same question does cite it.curl -u default: http://localhost:8123 --data "SELECT 1"→AUTHENTICATION_FAILED(403);-u tpk:$TIMEPLUS_PASSWORD→1(200).must_change_passworduser; Users tab CRUD (create/delete role with checkbox-builtentry_keys, create/delete user) all verified end-to-end; non-admin session shows only the Chat nav tab (no Manage/Users).TIMEPLUS_HOST=localhost TIMEPLUS_USER=tpk TIMEPLUS_PASSWORD=... uv run pytest -q→ 134 passed (proves the credentialed path against the password-locked stack, not just the open dev default).web:npm run buildandnpm run check:sanitizeboth pass.Deviations from the spec
defaultuser to loopback networks; this branch instead password-locksdefaultwith the sameTIMEPLUS_PASSWORDas the newtpkuser. Reason: reliably scopingdefaultto loopback-only via the timeplusd YAML config-merge semantics proved fragile across the base image's XML/YAML merge behavior (network restrictions didn't merge/override predictably), whereas an explicit password ondefaultis simple and equally effective for this compose topology (no service other thantpk/agentneedsdefaultat all).chmod/@removefixes. The base image ships/etc/timeplusd-server/users.d/asdr-x------, read-only even to its own owner (UID 101) — the entrypoint needschmod u+won that directory at build time so it can write the renderedtpk-users.yamloverride at container start. Separately, timeplusd's YAML config merge unions sibling keys rather than replacing them, so overridingdefault's plaintextpassword: ''required an explicit'@remove': '1'on thepasswordkey (the YAML equivalent of ClickHouse/timeplusd's XML@removemerge attribute) before settingpassword_sha256_hex, or the server refused to start with "more than one field of 'password', 'password_sha256_hex' are used."Also fixed during this task's live E2E gate: unquoted backticks in a heredoc comment inside
tpk-entrypoint.shwere being interpreted as shell command substitution (`password: ''`/`default`), producing spurious "command not found" errors in container logs on every startup (functionally harmless — the generated YAML's real fields were unaffected — but noisy); escaped them.Post-final-review fixes
Follow-up fixes from the final whole-branch review (commit
46691b8):/auth/meunhandled rejection. The mount effect now wraps the/auth/mecall intry/catch/finally; a network-level fetch rejection (e.g. server restarting on page load) falls through to the login gate instead of leaving the app renderingnullforever.logout()error handling.logout()now clears the local session (setToken(null),setMe(null)) in afinallyblock, so a network failure on/auth/logoutno longer strands the user in the authenticated view.chathandler's non-adminauth_mod.get_role(...)call now runs viarun_in_threadpoolinstead of synchronously on the event loop, so a slow/unreachable store no longer blocks every concurrent request for a TCP connect timeout. The existing fail-closedfrozenset()behavior on error is unchanged.roletoadmin; role selection is now required (empty option disabled, submit button disabled until a role is picked).Verified:
cd web && npm run build && npm run check:sanitizepass;TIMEPLUS_HOST=localhost uv run pytest -q→ 134 passed (dev store'sdefaultuser has no password, so the plain unauthenticated path applies).