crueber/walhub
Watch
1
0
Fork
You've already forked walhub
 0
Code Issues 2 Pull requests Projects Releases Packages Wiki Activity

Wave A: identity + permissions (docs/features/01) #7

Merged
crueber merged 2 commits from feat/identity into main 2026-09-04 01:15:24 +00:00
Conversation 1 Commits 2 Files changed 57 +9043 -24
crueber commented 2026-09-04 01:10:04 +00:00
Owner
Copy link

Wave A of the collaboration layer (tracks #3): internal/identity end to end per docs/features/01_identity_permissions.md §§2–10.

What lands

  • New package internal/identity: orgs/members/teams CRUD (CAS loops, bounded ≤5 then 409; no locks, no sidecars), access.json role resolution per P6 with version-stamped conditional-GET LRU, invitations (Create-only, delete-on-transition, inbox index), visibility + require_read gate (anonymous-denied reads get a real 401 + WWW-Authenticate: Bearer).
  • Seams: Seam 1 both lanes via server.ChainAPI (feature Handle(w,r) bool fronts the core mux); Seam 3 team:/role: expansion at load time incl. new policy.ActorExpander (protect bypass lists expand on a copy — the source doc is never mutated); Seam 5 access-bootstrap op (per-repo materialize; union opStart so listed extension ops start); Seam 7 walhub access get|put.
  • Core touches, all additive: api Env.Access/GroupExpander + Dispatch read hook (AuthRead repo routes) + dry-run expansion warnings; server ReadGate, api-seam principal injection (invalid tokens now 401 instead of anonymous-treated; anonymous unchanged), git/LFS/bundle read-path gates.
  • Frozen overwritable list gains the identity families in the same change (14 §14.2 + Decisions, DEVIATIONS D-EXT-2, 01_overview).
  • UI/SDK: org settings page (/:org/settings, profile/members/teams/invitations), repo Access tab (4th settings tab, CAS version footer, 409 reload hint), SDK users/orgs/access/invites submodules + typedefs; dark+light via existing Tailwind vocabulary.
  • EVIDENCE E2: authz read path measured — 1 conditional GET + 1 per referenced team, 0 bodies warm, lazy invalidation; O(1) argument (bounded bindings, exact keys, no LIST).

Verification

  • internal/identity: 98.5% statements, -race clean, table-driven httptest per handler.
  • make cover green (api 95.4%, server 95.2%, policy 98.6%); full go test -short ./... green; 129/129 node --test.
  • Live: rebuilt compose stack, curl flows (org/team/access/invite CRUD, stale-PUT 409), token-mode server (anon private API+git → 401+Bearer, authed → through, bad token → 401).
  • Real Chromium (Playwright, canonical host): /, repo settings + Access save interaction, org page, /setup, dark + light — zero console errors.
  • Staged tree verified self-consistent in an isolated worktree (build + backend tests + node tests). Only feature files/hunks committed; pre-existing worktree changes untouched.

Deviations / notes (for review)

  1. SynthesizeDefault omits the owner-admin binding when the owner namespace is not an email (else the materialized object would fail its own validation); org repos are covered by org-owner resolution, host flags still apply.
  2. Invite preview requires authentication (subject-match OR token-match); anonymous link holders log in first — invites are scope-located via the caller's inbox, no global LIST.
  3. AcceptInvite validates invite roles fail-closed (a crafted super role can no longer be absorbed by an existing binding).
  4. Added two additive collection endpoints the UI needs: GET …/members and GET /api/v1/orgs/{org}/invitations (owner). No separate /:org/teams/:slug route — team membership edits inline in the Org page.
  5. role: references over missing repos synthesize silently to empty (the legacy default resolves); malformed references warn. Empty match expansion denies fail-closed.
  6. Shared dirty files (router.go, index.jsx, Repos.jsx) contain only my hunks in this PR; your uncommitted hunks there stay in the worktree — expect a small rebase touch on Repos.jsx (old-file tail vs your rewrite).

Do NOT merge per Wave plan — Wave B/C build on these seams.

Wave A of the collaboration layer (tracks #3): `internal/identity` end to end per `docs/features/01_identity_permissions.md` §§2–10. ## What lands - **New package `internal/identity`**: orgs/members/teams CRUD (CAS loops, bounded ≤5 then 409; no locks, no sidecars), `access.json` role resolution per P6 with version-stamped conditional-GET LRU, invitations (Create-only, delete-on-transition, inbox index), visibility + `require_read` gate (anonymous-denied reads get a real 401 + `WWW-Authenticate: Bearer`). - **Seams**: Seam 1 both lanes via `server.ChainAPI` (feature `Handle(w,r) bool` fronts the core mux); Seam 3 `team:`/`role:` expansion at load time incl. new `policy.ActorExpander` (protect bypass lists expand on a copy — the source doc is never mutated); Seam 5 `access-bootstrap` op (per-repo materialize; union `opStart` so listed extension ops start); Seam 7 `walhub access get|put`. - **Core touches, all additive**: api `Env.Access`/`GroupExpander` + Dispatch read hook (AuthRead repo routes) + dry-run expansion warnings; server `ReadGate`, api-seam principal injection (invalid tokens now 401 instead of anonymous-treated; anonymous unchanged), git/LFS/bundle read-path gates. - **Frozen overwritable list** gains the identity families in the same change (14 §14.2 + Decisions, DEVIATIONS D-EXT-2, 01_overview). - **UI/SDK**: org settings page (`/:org/settings`, profile/members/teams/invitations), repo Access tab (4th settings tab, CAS version footer, 409 reload hint), SDK `users/orgs/access/invites` submodules + typedefs; dark+light via existing Tailwind vocabulary. - **EVIDENCE E2**: authz read path measured — 1 conditional GET + 1 per referenced team, 0 bodies warm, lazy invalidation; O(1) argument (bounded bindings, exact keys, no LIST). ## Verification - `internal/identity`: 98.5% statements, `-race` clean, table-driven httptest per handler. - `make cover` green (api 95.4%, server 95.2%, policy 98.6%); full `go test -short ./...` green; 129/129 `node --test`. - Live: rebuilt compose stack, curl flows (org/team/access/invite CRUD, stale-PUT 409), token-mode server (anon private API+git → 401+Bearer, authed → through, bad token → 401). - Real Chromium (Playwright, canonical host): `/`, repo settings + Access save interaction, org page, `/setup`, dark + light — zero console errors. - Staged tree verified self-consistent in an isolated worktree (build + backend tests + node tests). Only feature files/hunks committed; pre-existing worktree changes untouched. ## Deviations / notes (for review) 1. `SynthesizeDefault` omits the owner-admin binding when the owner namespace is not an email (else the materialized object would fail its own validation); org repos are covered by org-owner resolution, host flags still apply. 2. Invite preview requires authentication (subject-match OR token-match); anonymous link holders log in first — invites are scope-located via the caller's inbox, no global LIST. 3. `AcceptInvite` validates invite roles fail-closed (a crafted `super` role can no longer be absorbed by an existing binding). 4. Added two additive collection endpoints the UI needs: `GET …/members` and `GET /api/v1/orgs/{org}/invitations` (owner). No separate `/:org/teams/:slug` route — team membership edits inline in the Org page. 5. `role:` references over missing repos synthesize silently to empty (the legacy default resolves); malformed references warn. Empty match expansion denies fail-closed. 6. Shared dirty files (`router.go`, `index.jsx`, `Repos.jsx`) contain only my hunks in this PR; your uncommitted hunks there stay in the worktree — expect a small rebase touch on `Repos.jsx` (old-file tail vs your rewrite). Do NOT merge per Wave plan — Wave B/C build on these seams.
Wave A of the collaboration layer (Forgejo crueber/walhub#3).

New package internal/identity (Seam 1 RouteProvider surface on both
lanes via server.ChainAPI; Seam 3 team:/role: expansion incl. the
policy.ActorExpander seam on ProtectEffect; Seam 5 access-bootstrap
op; Seam 7 walhub access get/put): org/member/team CRUD with CAS
loops (bounded 5, then 409), access.json role resolution per P6 with
version-stamped conditional-GET LRU, invitations (Create-only,
delete-on-transition, inbox index), and the require_read gate
(anonymous-denied reads get a real 401 + Bearer).

Core touches (all additive): api Env Access/GroupExpander + Dispatch
read hook + dry-run expansion + union opStart; server ChainAPI,
ReadGate, api-seam principal injection, git/LFS read-path gates;
policy expansion helper. Frozen overwritable list gains the identity
families (14 §14.2 + Decisions, DEVIATIONS D-EXT-2, 01_overview).

UI/SDK: org settings page (/:org/settings), repo Access tab, SDK
users/orgs/access/invites submodules + typedefs; EVIDENCE E2 entry
(authz read path O(1), harness named).

Tests: identity 98.5% (table-driven httptest per handler, -race
clean), api 95.4%, server 95.2%, policy 98.6%; make cover green;
129/129 node tests; real-Chromium drive of /, settings, org page
and Access save in dark+light with zero console errors.
- DeleteTeam strips team bindings via the bounded CAS loop (412s retry,
  then 409) instead of a single-shot PUT that swallowed precondition
  failures and left stale bindings behind.
- Invite LIST collections (org + repo) take ?n= (default 100, max 1000,
  the team-list P5 convention); ListRepoInvites/ListOrgInvites signatures
  gain the page cap.
- DeleteOrg lists teams before deleting anything: a LIST failure aborts
  with the org intact instead of half-deleted with leaked teams.
- GetTeam evicts its version-stamped cache entry on NotFound so a deleted
  team leaves no stale roster behind.
- Tests: transient-412 heal case for DeleteTeam; conflict/abort surfacing
  assertions updated to the honest contract.
crueber commented 2026-09-04 01:15:05 +00:00
Author
Owner
Copy link

PR #7 review (Wave A identity, feat/identity) — round 1

Reviewed 3e622b5 + review-fix commit 6e67618 (pushed) against
docs/features/01_identity_permissions.md, P1–P9, 13_concurrency.md,
14_extensibility.md. Verified: full diff file-by-file; gofmt clean;
go vet clean; go test -race ./internal/identity/... pass;
coverage 98.1% (≥95% gate); api/policy/server -race pass;
go build ./... clean; node --test web/test/unit/*.test.js 129/129 pass.
No new third-party imports (go.mod untouched); core↔feature seam
direction holds (api/server depend on identity only via the
ReadAccess/policy.Expander/ExtraRoutes interfaces).

Fixed and pushed in 6e67618 (all in PR files, your other worktree changes untouched)

  1. internal/identity/orgs.go DeleteTeam — the per-repo binding strip
    was a single-shot PutUpdate that swallowed 412 (!IsPreconditionFailed
    → skip), leaving stale team: bindings behind under concurrent admin
    edits and violating the §3 "sequential CAS per affected repo" rule.
    Now a bounded casUpdate loop: transient 412s heal, persistent contention
    surfaces an honest 409. New TestDeleteTeamHealsTransientConflict proves
    the heal; edge_test.go updated to assert the 409 contract.
  2. Invite LISTs lacked pagination (P5 MUST). ListRepoInvites /
    ListOrgInvites now take the team-list ?n= convention (default 100,
    max 1000); handlers parse it via pageSize().
  3. DeleteOrg half-delete on LIST failure — it deleted org.json /
    members.json first and swallowed the team-list error, leaking teams.
    Now lists teams before deleting anything: failure aborts with the org
    intact (mop_test.go updated).
  4. GetTeam evicts its version-stamped cache entry on NotFound so a
    deleted team leaves no stale roster entry behind (was only a hygiene
    leak — the NotFound path already bypassed the cache — but evicting is
    clearly right).

Open findings (non-blocking; recommend follow-ups, not rework here)

  1. Anonymous signed-link preview → 401 (http_invites.go routeInvites):
    spec §8 says GET /invitations/{id}?token= is "token OR subject match",
    but the handler rejects anonymous callers before the token is checked —
    and findInvite needs the caller's inbox, so a token-only lookup is
    structurally impossible without a global index. Authed flow works
    (recipient logs in → subject match). Suggest a one-line doc note in 01 §7.
  2. Top-level DELETE /invitations/{id} is invitee-decline only; spec §8
    says "invitee (decline) or issuer (cancel)". Issuer cancel exists on the
    scoped org/repo admin endpoints, so nothing is unachievable — same doc-note
    suggestion.
  3. Discovery endpoints[] omits the identity routes (14.12 says additive
    endpoints MUST be listed). discoveryEndpoints() derives from the core
    route table and the identity surface fronts it, so there is no seam to
    contribute entries today. The SDK calls fixed paths (never gates on
    discovery) and all 129 JS tests pass, so this is cosmetic — suggest a
    follow-up (e.g. Env.ExtraEndpoints merged by the discovery handler).

Spot-checks that passed: resolution order P6 verbatim (incl. org-owner→admin,
host flag step 3, anon public-read); require_read 401+Bearer realm="walgit"
on git/LFS/API lanes with the §8.4 in-band exception preserved;
CAS loops bounded at 5 → 409; bucket-only state (LRUs version-stamped,
staleness ≤ 1 request); []-never-null / plain-text errors / RFC3339;
both lanes in Handler.Handle; dark: variants on new UI; E2 evidence
reproduces the claimed shape (1 conditional access GET + 1 per referenced
team, 0 bodies warm — matches the code).

Tests re-run after the fix commit: identity -race pass, 98.1% cover;
api/policy/server -race pass; build clean; JS 129/129.

# PR #7 review (Wave A identity, `feat/identity`) — round 1 Reviewed `3e622b5` + review-fix commit `6e67618` (pushed) against `docs/features/01_identity_permissions.md`, P1–P9, `13_concurrency.md`, `14_extensibility.md`. Verified: full diff file-by-file; `gofmt` clean; `go vet` clean; `go test -race ./internal/identity/...` pass; coverage **98.1%** (≥95% gate); `api`/`policy`/`server -race` pass; `go build ./...` clean; `node --test web/test/unit/*.test.js` 129/129 pass. No new third-party imports (`go.mod` untouched); core↔feature seam direction holds (`api`/`server` depend on identity only via the `ReadAccess`/`policy.Expander`/`ExtraRoutes` interfaces). ## Fixed and pushed in 6e67618 (all in PR files, your other worktree changes untouched) 1. **`internal/identity/orgs.go` DeleteTeam** — the per-repo binding strip was a single-shot `PutUpdate` that **swallowed 412** (`!IsPreconditionFailed` → skip), leaving stale `team:` bindings behind under concurrent admin edits and violating the §3 "sequential CAS per affected repo" rule. Now a bounded `casUpdate` loop: transient 412s heal, persistent contention surfaces an honest 409. New `TestDeleteTeamHealsTransientConflict` proves the heal; `edge_test.go` updated to assert the 409 contract. 2. **Invite LISTs lacked pagination (P5 MUST).** `ListRepoInvites` / `ListOrgInvites` now take the team-list `?n=` convention (default 100, max 1000); handlers parse it via `pageSize()`. 3. **`DeleteOrg` half-delete on LIST failure** — it deleted `org.json` / `members.json` first and swallowed the team-list error, leaking teams. Now lists teams *before* deleting anything: failure aborts with the org intact (`mop_test.go` updated). 4. **`GetTeam` evicts its version-stamped cache entry on NotFound** so a deleted team leaves no stale roster entry behind (was only a hygiene leak — the NotFound path already bypassed the cache — but evicting is clearly right). ## Open findings (non-blocking; recommend follow-ups, not rework here) 5. **Anonymous signed-link preview → 401** (`http_invites.go` `routeInvites`): spec §8 says `GET /invitations/{id}?token=` is "token OR subject match", but the handler rejects anonymous callers before the token is checked — and `findInvite` needs the caller's inbox, so a token-only lookup is structurally impossible without a global index. Authed flow works (recipient logs in → subject match). Suggest a one-line doc note in 01 §7. 6. **Top-level `DELETE /invitations/{id}` is invitee-decline only**; spec §8 says "invitee (decline) or issuer (cancel)". Issuer cancel exists on the scoped org/repo admin endpoints, so nothing is unachievable — same doc-note suggestion. 7. **Discovery `endpoints[]` omits the identity routes** (14.12 says additive endpoints MUST be listed). `discoveryEndpoints()` derives from the core route table and the identity surface fronts it, so there is no seam to contribute entries today. The SDK calls fixed paths (never gates on discovery) and all 129 JS tests pass, so this is cosmetic — suggest a follow-up (e.g. `Env.ExtraEndpoints` merged by the discovery handler). Spot-checks that passed: resolution order P6 verbatim (incl. org-owner→admin, host flag step 3, anon public-read); `require_read` 401+`Bearer realm="walgit"` on git/LFS/API lanes with the §8.4 in-band exception preserved; CAS loops bounded at 5 → 409; bucket-only state (LRUs version-stamped, staleness ≤ 1 request); `[]`-never-null / plain-text errors / RFC3339; both lanes in `Handler.Handle`; dark: variants on new UI; E2 evidence reproduces the claimed shape (1 conditional access GET + 1 per referenced team, 0 bodies warm — matches the code). Tests re-run after the fix commit: identity `-race` pass, 98.1% cover; `api`/`policy`/`server` `-race` pass; build clean; JS 129/129.
Sign in to join this conversation.
Reviewers
No reviewers
Labels
Clear labels   
actions

   
bug
Something is not working

  
cli

   
duplicate
This issue or pull request already exists

  
enhancement
New feature

  
fork

   
forum

   
git storage

   
help wanted
Need some help

  
insights

   
invalid
Something is wrong

  
issues

   
moderation

   
oidc

   
ownership transfer

   
packages

   
pr/merge protection rules

   
projects

   
pull requests

   
question
More information is needed

  
releases

   
sponsorships

   
tags

   
webhooks

   
wiki

   
wontfix
This won't be fixed

No labels
actions
bug
cli
duplicate
enhancement
fork
forum
git storage
help wanted
insights
invalid
issues
moderation
oidc
ownership transfer
packages
pr/merge protection rules
projects
pull requests
question
releases
sponsorships
tags
webhooks
wiki
wontfix
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
crueber/walhub!7
No description provided.