docs(design): multi-user auth + ownership design note (#113) - #167
Closed
lucastononro wants to merge 2 commits into
Closed
docs(design): multi-user auth + ownership design note (#113)#167lucastononro wants to merge 2 commits into
lucastononro wants to merge 2 commits into
Conversation
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Wp66uSq9saSpRq9Bbmjxj4
- ProjectShare: specify CHECK constraint + create-path validation for the user_id/link_token_hash mutual-exclusion invariant (P1) - AuthSession: spell out the hash-the-cookie-once-at-lookup pattern - Bootstrap: recommend env-var pre-seed; restrict interactive bootstrap to localhost / token-mode gate (first-come-first-served window) - scope_to_user: state explicitly that pre-redemption link-browsing sessions are scoped to the single project only, by design
Owner
Author
|
Merged into integration branch |
16 tasks
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.
Summary
Design note only — no code. Multi-user auth/ownership touches every model, all 15 routers, and the frontend, so this PR makes #113 build-ready instead of attempting a blind build:
docs/design/multi-user-auth.mdgrounds the design in the current code (models.py,main.py, routers,services/usage.py,api.ts/AppContext) and in the two in-flight foundations — PR #138 (opt-inAPI_AUTH_TOKENbearer middleware, #88) and PR #153 (Alembic, #121).Proposed approach (short):
users(+auth_sessions,api_tokens,project_shares) withowner_idFKs onProject/Experiment/RegisteredModel/Deployment;Project.owner_idis the authoritative access root, childowner_ids are creator attribution;usage_events.user_idfor per-user cost.AUTH_MODE = none | token | multi_user. Browser = HttpOnly session cookie (fixes SSE natively —EventSourcesends cookies, so Opt-in bearer-token auth for /api/* (API_AUTH_TOKEN) #138's?token=stays only for PAT/CLI clients); CLI = per-user PATs on the same Bearer wire shape as Opt-in bearer-token auth for /api/* (API_AUTH_TOKEN) #138; OIDC SSO later mints the same session. Innone/tokenmodeget_current_userresolves to a fixed default-owner user, so ownership code ships dark with zero behavior change.NOT NULLflip; first real login "claims" the default owner's data.can_writeshare → write; 404 for invisible objects), a reusableget_accessible_projectdependency +scope_to_userquery helper, per-router enforcement checklist, project-granularity share-by-link + instance visibility.Keeping #113 open — this is the design for #113, not the implementation; the issue should track the build phases. Same design posted as a comment on the issue.
Changes
docs/design/multi-user-auth.md(new) — the design note. No code, schema, or router changes; no docs index exists to update.Test plan
n/a — documentation only; reviewer reads the design. Verified the branch contains a single new markdown file (no build impact).
Design for #113
🤖 Generated with Claude Code
Greptile Summary
This PR adds a documentation-only design note (
docs/design/multi-user-auth.md) for multi-user authentication, ownership, and sharing (#113). No code, schema, or router changes are included; the document grounds the proposed design in the current codebase and the two in-flight foundation PRs (#138 bearer middleware, #153 Alembic).users,auth_sessions,api_tokens,project_sharestables plusowner_idFKs onProject/Experiment/RegisteredModel/Deploymentand a nullableusage_events.user_idfor per-user cost attribution.none | token | multi_userladder keeps today's single-tenant behaviour unchanged by default; browser uses HttpOnly session cookie (solves SSE natively), CLI/scripts use per-user PATs on the same Bearer wire shape as PR Opt-in bearer-token auth for /api/* (API_AUTH_TOKEN) #138.Confidence Score: 4/5
Safe to merge as a documentation-only PR; one data model gap should be addressed before the Phase 1 implementation begins.
The document is thorough and all previous review threads are addressed inline. One genuine gap remains:
usage_events.user_idis declared nullable-forever but the FK carries noondeletedirective, so deleting a user with attributed events would fail at the DB level — contradicting the 'unattributed rows' dashboard intent. This needs to be fixed in the design before the Alembic migration is written, otherwise the admin user-delete flow will be broken on the first real usage data.Files Needing Attention: docs/design/multi-user-auth.md — specifically the
UsageEvent.user_idFK definition (§3.5) and theProjectShare.idPK style (§3.4).Important Files Changed
usage_events.user_idFK needsondelete=SET NULLto allow user deletion without orphaning attributed events.Sequence Diagram
sequenceDiagram participant B as Browser participant F as FastAPI participant DB as Database Note over B,DB: Phase 2 — multi_user mode login B->>F: "POST /api/auth/login {email, password}" F->>DB: "SELECT user WHERE email=..." DB-->>F: User row F->>F: argon2id verify password F->>DB: "INSERT auth_sessions (id=sha256(token), expires_at)" F-->>B: "200 Set-Cookie: trainable_session=raw_token HttpOnly SameSite=Lax" Note over B,DB: Subsequent requests (browser) B->>F: GET /api/projects (cookie auto-sent) F->>F: get_current_user: sha256(cookie) lookup F->>DB: SELECT auth_session + slide expiry DB-->>F: session + user F->>F: scope_to_user(stmt, user) F-->>B: 200 filtered project list Note over B,DB: AUTH_MODE=none (Phase 1, dark) B->>F: GET /api/projects (no credentials) F->>F: get_current_user returns default_owner (no DB lookup) F->>F: scope_to_user no-op F-->>B: 200 full project list (identical to today)Reviews (2): Last reviewed commit: "fix(review): address Greptile findings o..." | Re-trigger Greptile