crueber/walhub

Watch

Fork 0

Auth UX: allow anonymous read in OIDC mode (public browsing), navbar Login button + identity dropdown (profile/keys/invitations/setup/logout) #371

New issue

Closed

opened 2026-09-12 12:12:18 +00:00 by crueber · 3 comments

crueber commented

2026-09-12 12:12:18 +00:00

Owner

What's wrong

Two coupled problems around auth mode and the navbar:

OIDC mode forbids anonymous read by validation, blocking public browsing. internal/config/validate.go:104-107 hard-errors: server.auth.anonymous_read must be false when auth.mode = "oidc". The setup page states it too ("must be false in oidc mode"). But with #345 (public repos viewable without an account) landing, anonymous read of public repos is exactly what OIDC mode should support — an instance with OIDC login and public repos needs anonymous_read = true semantics so visitors can browse without authenticating.
The navbar has no identity surface at all. Keys, setup, and (implicitly) login are flat links in the primary nav (web/src/App.jsx:60-61), there is no Login button for signed-out users, and no logged-in indicator (avatar/username with a menu) for authenticated ones.

Desired behavior

A. Anonymous read rework (per #345's semantics):

When auth.mode = "oidc" and the instance has public repos, anonymous_read = true must be allowed — visitors browse public repos without logging in.
The validation rule flips from a blanket prohibition to: allow anonymous_read = true in OIDC mode (the default recommendation), keeping false available as the "everything requires login" hard-lock option. The current behavior is desirable only in that locked configuration.
Update the setup-page copy ("must be false in oidc mode") to describe the real trade-off, and amend docs/go/06_server_http.md/the auth docs where the prohibition is stated. This partially intersects #344 (silent OIDC misconfig) — the validation pass should be amended in one coherent change with #344's startup hardening.

B. Navbar identity surface (upper right, in the main navbar):

Signed out (with OIDC browser login enabled): a "Login" button that sends the user through the OIDC pathway (/_auth/login?next=<current path>).
Signed in: an identity control in the upper right showing the user's avatar, or their username when no avatar exists — opening a dropdown menu containing:
- Your profile (the owner profile page, #234),
- Keys (/keys — moved out of the primary nav),
- Invitations (/invitations or wherever the identity invites surface lives — moved in),
- Setup (/setup — moved in; visible only when the user can actually use it, i.e. host admin / setup-capable),
- Log out.
When auth.mode = "none": Setup also stays where it is today (visible in the primary nav) so the zero-config first-run flow stays obvious — the screenshot's warning ("anyone who can reach this port can read and write every repository") is exactly the state where discoverability matters.
Signed-out in none mode: no Login button (there's nothing to log in to), setup visible as today.

Implementation notes (code evidence)

The primary nav is web/src/App.jsx:57-62 (site-nav: explore/import/API/keys/setup); the right cluster is the ml-auto div (:63+) with NotificationTray + theme toggle — the identity control slots in there, left of the tray.
The dropdown pattern to copy: RefPicker/ChooserMenu-style popover with outside-click close + Esc + focus return (the #255/#311 family — ChooserMenu in CommentComposer.jsx is the newest reference).
Auth state for the navbar: me() (GET /api/v1/me — {principal, write, anonymous}) is already fetched app-wide via useData ("me" cache key) by several pages; lift it to the shell. Avatar: no avatar storage exists yet (#349 candidate) — render the username (or initials chip) until avatars land; the component should take an optional avatar URL so adding avatars later is a prop change.
"Setup only when you can use it": the server knows (/api/v1/setup/test admin-gates; me().admin/write flags exist) — expose setup availability in me() or the discovery document rather than guessing client-side. The identity/user object from #370 (username, email-self-only) feeds the avatar/username display.
Login button enablement ties to #344: when browser login is disabled (misconfigured trio), the Login button must not be shown or must render the diagnostic state — the discovery auth block should advertise login availability (also requested in #344).
Log out: verify the server exposes a logout/session-invalidation route (/_auth/* family); if none exists, that's a small backend addition in this ticket (session cookie clear + redirect home).
Validation change is spec-relevant: docs/go/06_server_http.md §8.6 and the auth docs state the oidc/anonymous_read relationship — amend in the same change (law 12 duty), noting the #345 interaction.

Acceptance criteria

auth.mode = "oidc" with anonymous_read = true passes config validation (setup + /api/v1/setup/test); the setup-page copy describes the real behavior instead of "must be false".
With anonymous read on and OIDC enabled: a signed-out visitor browses public repos; the navbar shows a Login button that runs the OIDC flow and returns to the originating page (next).
Signed-in: the upper-right identity control shows avatar-or-username with a dropdown containing profile, keys, invitations, setup (admin-only), and log out; log out invalidates the session and returns home.
Keys/invitations/setup are removed from the primary nav for signed-in OIDC users; setup remains visible in the nav when auth.mode = "none" (both nav and menu), and appears in the dropdown only for users who can use it.
Signed-out in none mode: no Login button; nav unchanged from today except the identity control's absence.
The validation flip and its docs amendment land together (06_server_http §8.6 + auth docs); tests cover the validation matrix (oidc × anonymous_read × login-enabled) and the navbar render matrix (mode × auth state × admin).
Dropdown matches the app's popover patterns (outside-click, Esc, focus return); light/dark themes; 390px mobile width per the #273-#278 sweep.

## What's wrong Two coupled problems around auth mode and the navbar: 1. **OIDC mode forbids anonymous read by validation, blocking public browsing.** `internal/config/validate.go:104-107` hard-errors: `server.auth.anonymous_read must be false when auth.mode = "oidc"`. The setup page states it too ("must be false in oidc mode"). But with #345 (public repos viewable without an account) landing, **anonymous read of public repos is exactly what OIDC mode should support** — an instance with OIDC login and public repos needs `anonymous_read = true` semantics so visitors can browse without authenticating. 2. **The navbar has no identity surface at all.** Keys, setup, and (implicitly) login are flat links in the primary nav (`web/src/App.jsx:60-61`), there is no Login button for signed-out users, and no logged-in indicator (avatar/username with a menu) for authenticated ones. ## Desired behavior **A. Anonymous read rework (per #345's semantics):** - When `auth.mode = "oidc"` **and** the instance has public repos, `anonymous_read = true` must be **allowed** — visitors browse public repos without logging in. - The validation rule flips from a blanket prohibition to: allow `anonymous_read = true` in OIDC mode (the default recommendation), keeping `false` available as the "everything requires login" hard-lock option. The current behavior is desirable only in that locked configuration. - Update the setup-page copy ("must be false in oidc mode") to describe the real trade-off, and amend `docs/go/06_server_http.md`/the auth docs where the prohibition is stated. This partially intersects #344 (silent OIDC misconfig) — the validation pass should be amended in one coherent change with #344's startup hardening. **B. Navbar identity surface (upper right, in the main navbar):** - **Signed out (with OIDC browser login enabled):** a **"Login"** button that sends the user through the OIDC pathway (`/_auth/login?next=<current path>`). - **Signed in:** an identity control in the upper right showing **the user's avatar, or their username when no avatar exists** — opening a dropdown menu containing: - **Your profile** (the owner profile page, #234), - **Keys** (`/keys` — moved out of the primary nav), - **Invitations** (`/invitations` or wherever the identity invites surface lives — moved in), - **Setup** (`/setup` — moved in; **visible only when the user can actually use it**, i.e. host admin / setup-capable), - **Log out**. - **When `auth.mode = "none"`:** Setup **also stays where it is today** (visible in the primary nav) so the zero-config first-run flow stays obvious — the screenshot's warning ("anyone who can reach this port can read and write every repository") is exactly the state where discoverability matters. - Signed-out in `none` mode: no Login button (there's nothing to log in to), setup visible as today. ## Implementation notes (code evidence) - The primary nav is `web/src/App.jsx:57-62` (`site-nav`: explore/import/API/keys/setup); the right cluster is the `ml-auto` div (:63+) with `NotificationTray` + theme toggle — the identity control slots in there, left of the tray. - The dropdown pattern to copy: `RefPicker`/`ChooserMenu`-style popover with outside-click close + Esc + focus return (the #255/#311 family — `ChooserMenu` in `CommentComposer.jsx` is the newest reference). - Auth state for the navbar: `me()` (`GET /api/v1/me` — `{principal, write, anonymous}`) is already fetched app-wide via `useData` ("me" cache key) by several pages; lift it to the shell. Avatar: no avatar storage exists yet (#349 candidate) — render the username (or initials chip) until avatars land; the component should take an optional avatar URL so adding avatars later is a prop change. - "Setup only when you can use it": the server knows (`/api/v1/setup/test` admin-gates; `me().admin`/write flags exist) — expose setup availability in `me()` or the discovery document rather than guessing client-side. The identity/user object from #370 (username, email-self-only) feeds the avatar/username display. - Login button enablement ties to #344: when browser login is *disabled* (misconfigured trio), the Login button must not be shown or must render the diagnostic state — the discovery auth block should advertise login availability (also requested in #344). - Log out: verify the server exposes a logout/session-invalidation route (`/_auth/*` family); if none exists, that's a small backend addition in this ticket (session cookie clear + redirect home). - **Validation change is spec-relevant**: `docs/go/06_server_http.md` §8.6 and the auth docs state the oidc/anonymous_read relationship — amend in the same change (law 12 duty), noting the #345 interaction. ## Acceptance criteria - [ ] `auth.mode = "oidc"` with `anonymous_read = true` passes config validation (setup + `/api/v1/setup/test`); the setup-page copy describes the real behavior instead of "must be false". - [ ] With anonymous read on and OIDC enabled: a signed-out visitor browses public repos; the navbar shows a **Login** button that runs the OIDC flow and returns to the originating page (`next`). - [ ] Signed-in: the upper-right identity control shows avatar-or-username with a dropdown containing profile, keys, invitations, setup (admin-only), and log out; log out invalidates the session and returns home. - [ ] Keys/invitations/setup are removed from the primary nav for signed-in OIDC users; setup remains visible in the nav when `auth.mode = "none"` (both nav and menu), and appears in the dropdown only for users who can use it. - [ ] Signed-out in `none` mode: no Login button; nav unchanged from today except the identity control's absence. - [ ] The validation flip and its docs amendment land together (06_server_http §8.6 + auth docs); tests cover the validation matrix (oidc × anonymous_read × login-enabled) and the navbar render matrix (mode × auth state × admin). - [ ] Dropdown matches the app's popover patterns (outside-click, Esc, focus return); light/dark themes; 390px mobile width per the #273-#278 sweep.

crueber added the

issues

oidc

labels

2026-09-12 12:12:18 +00:00

crueber added this to the v1 milestone

2026-09-12 12:12:18 +00:00

crueber referenced this issue from a commit

2026-09-12 13:39:20 +00:00

Fix #371: OIDC anonymous read + navbar identity (11 §5/§8, 06 §8.6/§14, 07 §8/§14, 12 §14)

crueber referenced this issue from a pull request that will close it,

2026-09-12 13:39:25 +00:00

Fix #371: OIDC anonymous read + navbar identity #375

crueber commented

2026-09-12 13:39:28 +00:00

Author

Owner

Fix is up: #375 (branch fix/issue-371, from origin/main @1ca4c5b). Validation flip + navbar Login/identity menu + me.admin/discovery.mode signals with docs amended in the same change. Test summary in the PR description. Not merging per instructions.

Fix is up: https://git.packden.us/crueber/walhub/pulls/375 (branch fix/issue-371, from origin/main @1ca4c5b). Validation flip + navbar Login/identity menu + me.admin/discovery.mode signals with docs amended in the same change. Test summary in the PR description. Not merging per instructions.

crueber commented

2026-09-12 13:42:52 +00:00

Author

Owner

Review of PR #375 (fix/issue-371, commit 2cd882f) — verified in scratch worktree (built web via local vite+esbuild, then removed it; main worktree untouched, still clean).

All 7 acceptance criteria hold; no code changes pushed (nothing to fix).

(1) Validation flip — exact. internal/config/validate.go:103-108: blanket anon refusal deleted, allowlist + oauth pair + #344 trio + secret-length rules byte-untouched. web/src/lib/setup.js:213-214 (copy) + :457-460 (validator mirror) agree; /api/v1/setup/test agrees via config.Validate (internal/server/setup_api.go:425,544). Old 'must be false' copy gone everywhere except docs/MASTER_RUST_SPEC.md:2103, which is the Rust reference text and correctly stays.
(2) Signals — zero new trips, no leak. me.admin rides GET /api/v1/me (internal/api/discovery.go:153-167, AuthRead-gated, no-store); discovery mode rides the one AuthOpen GET (authModeOf :61-66, nil/empty defaults to 'none'). App.jsx:48-49 reuses the shared 'me'/'discovery' useData keys Repos/Owners/Apidocs already fetch (single-flight per key). Admin leak checked: auth.Anonymous() is Admin:false (internal/server/auth/principal.go:22); auth.None() is Admin:true but isSignedIn() excludes mode 'none' so the menu never renders there — and setup is open in none mode anyway. me() exposes no email (shape is principal/write/anonymous/admin only).
(3) Navbar matrix — matches spec. navModel (web/src/lib/identity.js:88-110): signed-out+oidc+flow → Login with ?next=current (encodeURIComponent); flow dead (browser_login false / empty login_url) → hidden; signed-in non-anon, mode!=none → IdentityMenu with profile //username (owner page), keys, invitations, setup iff me.admin, logout → /_auth/logout?next=/; none mode → legacy nav, no Login, no menu. App.jsx:72-88 moves keys/invitations/setup out of primary nav only when signed in; setup stays in nav for signed-out + none mode.
(4) Logout — verified, no code change needed. GET /_auth/logout wired at internal/server/router.go:184; handler auth_oidc.go:321-330 clears walgit_session (MaxAge:-1, same path/flags) and 302s to sanitizeNext (non-/-prefixed or //-targets → /). Plain-anchor logout is correct.
(5) Popover contract — holds. IdentityMenu.jsx: outside-click with root-contains guard + onCleanup removal (:63-66), Esc closes + refocuses toggle (:53-58, :547), arrows walk items, Tab dismisses, menu/menuitem roles + aria-haspopup/expanded. #278 bounds: menu right-0 + max-w-[calc(100vw-2rem)] + max-h-96 scroll; toggle max-w-32 truncate so 390px header never overflows; site-nav keeps overflow-x-auto and identity sits in the ml-auto shrink-0 cluster left of the tray (App.jsx:65,69,96-99) — #273 no-regress.
(6) Username — no email leak. Menu renders me.principal only; OIDC maps Name via usernameFor(email) with Email kept separate (internal/server/auth.go:293), so principal is the #370 username.
(7) #344/#345 no-regress. Trio/pair/allowlist intact; new TestValidateOIDCAnonTrioMatrix pins trio failures never blame anonymous_read; hard-lock (anon=false) still boots and still 401s anon on non-repo surfaces while public repos stay browsable per #345.
(8) Gate results (scratch worktree, -race): internal/config ok, cover 95.7%; internal/api ok, cover 95.2%; internal/server ok (first run failed only on 'ui shell missing' — fresh worktree had no web/dist; after local vite+esbuild build, green). node --test web/test/unit/*.test.js: 743/745 — the 2 fails are smoke.test.js hitting the foreign :8080 listener (I did not touch it; one later run with it unreachable skipped cleanly, and identity-nav + setup-form alone are 69/69). gofmt clean, go vet clean, go build ./... green, vite+esbuild green, no Go/npm dep changes (go.mod/package.json diff empty). Docs amended together per law 12: 11_config_cli §5 rule 2 + key table + Decisions, 06_server_http §14 (logout contract pinned), 07_api §8+§14, 12_web_ui entry.

Notes (non-blocking): Apidocs.jsx uses useData('discovery') without .catch while App.jsx uses .catch — shared key, App (the shell) always mounts first so its fn wins the race in practice. Browser proof skipped per review instructions (tests + reasoning only).

MERGE RECOMMENDATION: ready to merge.

Review of PR #375 (fix/issue-371, commit 2cd882f) — verified in scratch worktree (built web via local vite+esbuild, then removed it; main worktree untouched, still clean). All 7 acceptance criteria hold; no code changes pushed (nothing to fix). (1) Validation flip — exact. internal/config/validate.go:103-108: blanket anon refusal deleted, allowlist + oauth pair + #344 trio + secret-length rules byte-untouched. web/src/lib/setup.js:213-214 (copy) + :457-460 (validator mirror) agree; /api/v1/setup/test agrees via config.Validate (internal/server/setup_api.go:425,544). Old 'must be false' copy gone everywhere except docs/MASTER_RUST_SPEC.md:2103, which is the Rust reference text and correctly stays. (2) Signals — zero new trips, no leak. me.admin rides GET /api/v1/me (internal/api/discovery.go:153-167, AuthRead-gated, no-store); discovery mode rides the one AuthOpen GET (authModeOf :61-66, nil/empty defaults to 'none'). App.jsx:48-49 reuses the shared 'me'/'discovery' useData keys Repos/Owners/Apidocs already fetch (single-flight per key). Admin leak checked: auth.Anonymous() is Admin:false (internal/server/auth/principal.go:22); auth.None() is Admin:true but isSignedIn() excludes mode 'none' so the menu never renders there — and setup is open in none mode anyway. me() exposes no email (shape is principal/write/anonymous/admin only). (3) Navbar matrix — matches spec. navModel (web/src/lib/identity.js:88-110): signed-out+oidc+flow → Login with ?next=current (encodeURIComponent); flow dead (browser_login false / empty login_url) → hidden; signed-in non-anon, mode!=none → IdentityMenu with profile //username (owner page), keys, invitations, setup iff me.admin, logout → /_auth/logout?next=/; none mode → legacy nav, no Login, no menu. App.jsx:72-88 moves keys/invitations/setup out of primary nav only when signed in; setup stays in nav for signed-out + none mode. (4) Logout — verified, no code change needed. GET /_auth/logout wired at internal/server/router.go:184; handler auth_oidc.go:321-330 clears walgit_session (MaxAge:-1, same path/flags) and 302s to sanitizeNext (non-/-prefixed or //-targets → /). Plain-anchor logout is correct. (5) Popover contract — holds. IdentityMenu.jsx: outside-click with root-contains guard + onCleanup removal (:63-66), Esc closes + refocuses toggle (:53-58, :547), arrows walk items, Tab dismisses, menu/menuitem roles + aria-haspopup/expanded. #278 bounds: menu right-0 + max-w-[calc(100vw-2rem)] + max-h-96 scroll; toggle max-w-32 truncate so 390px header never overflows; site-nav keeps overflow-x-auto and identity sits in the ml-auto shrink-0 cluster left of the tray (App.jsx:65,69,96-99) — #273 no-regress. (6) Username — no email leak. Menu renders me.principal only; OIDC maps Name via usernameFor(email) with Email kept separate (internal/server/auth.go:293), so principal is the #370 username. (7) #344/#345 no-regress. Trio/pair/allowlist intact; new TestValidateOIDCAnonTrioMatrix pins trio failures never blame anonymous_read; hard-lock (anon=false) still boots and still 401s anon on non-repo surfaces while public repos stay browsable per #345. (8) Gate results (scratch worktree, -race): internal/config ok, cover 95.7%; internal/api ok, cover 95.2%; internal/server ok (first run failed only on 'ui shell missing' — fresh worktree had no web/dist; after local vite+esbuild build, green). node --test web/test/unit/*.test.js: 743/745 — the 2 fails are smoke.test.js hitting the foreign :8080 listener (I did not touch it; one later run with it unreachable skipped cleanly, and identity-nav + setup-form alone are 69/69). gofmt clean, go vet clean, go build ./... green, vite+esbuild green, no Go/npm dep changes (go.mod/package.json diff empty). Docs amended together per law 12: 11_config_cli §5 rule 2 + key table + Decisions, 06_server_http §14 (logout contract pinned), 07_api §8+§14, 12_web_ui entry. Notes (non-blocking): Apidocs.jsx uses useData('discovery') without .catch while App.jsx uses .catch — shared key, App (the shell) always mounts first so its fn wins the race in practice. Browser proof skipped per review instructions (tests + reasoning only). MERGE RECOMMENDATION: ready to merge.

crueber closed this issue

2026-09-12 13:43:06 +00:00

crueber commented

2026-09-12 13:43:06 +00:00

Author

Owner

Fixed by PR #375 (review clean — all 8 dimensions pass, logout verified, no leaks, #344/#345 intact), merged. Closing.

crueber referenced this issue from a commit

2026-09-12 13:43:07 +00:00

Merge pull request 'Fix #371: OIDC anonymous read + navbar identity' (#375) from fix/issue-371 into main

crueber referenced this issue

2026-09-12 13:48:50 +00:00