#370 - OIDC identity: usernames, not emails — import fails on email owner, emails must never be exposed, and the stray back-to-form button - crueber/walhub

crueber commented

2026-09-12 12:08:35 +00:00

Owner

What's wrong (three linked defects from the same root)

The import form's owner dropdown offers and uses the user's email address (crueber@gmail.com) as the repo owner. Three consequences, all visible in the screenshot:

The import fails: bad target: invalid repo id "crueber@gmail.com/walhub" — the target owner/name parser (git.ParseRepoId via ParseRequest, internal/repoimport/service.go) rejects @ and . in the owner segment, so the import can never succeed for an OIDC user.
The email is exposed as a repo owner — repo paths, listings, and the created repo's owner field would render crueber@gmail.com to any visitor. Emails must never be exposed to users other than the logged-in user. The principal name is the email today (see root cause), so every surface that renders Principal.Name leaks it: repo paths, /explore, issue authorship, the commit/author displays, notifications.
A "back to form" button renders on the form itself (screenshot, bottom-left, next to "start import") — a dead affordance that makes no sense in-place.

Root cause (code evidence)

The principal name IS the raw email in OIDC mode: principalFromEmail returns auth.Principal{Name: email, …} (internal/server/auth.go:230-277 — note :277, Name: email, lowercased but otherwise the full address). Every OIDC login, session mint, and wgt_ token derives identity the same way (:200-228).
The owner dropdown in the import form lists the raw owners/principals — you and your orgs only (web/src/pages/Import.jsx, the getOwners() select in the current tree) — so for an OIDC user the dropdown contains their email, and that value flows into owner of the import request.
The backend then correctly rejects it: the owner segment must be a valid ID part (no @), and the identity model has no username concept at all — there is no users/<username> object, no display-name-vs-login separation for users. #234 added an owner profile (display name etc.) but the profile is keyed by the principal (the email), not by a username.
The "back to form" button is rendered by the import page's error/confirm state — it exists for the case where the import ran and the user lands on an outcome view, but it also renders (or persists) in the inline-error state where the user is already on the form. Wrong conditional, cosmetic but confusing on top of the failure.

What's needed

A. Usernames as the identity key (the systemic fix — this is the real ticket):

Introduce a username for every principal: derived at first OIDC login (email local-part, uniquified on collision — e.g. crueber, crueber2), stored on a user object (users/<username>/user.json mapping username ↔ email), and immutable thereafter.
principalFromEmail resolves email → username and returns Principal{Name: <username>} — the email never becomes a principal name, owner segment, or path component anywhere.
The email remains visible only to the logged-in user themselves (their own profile/settings page). Audit every surface that renders Principal.Name or owner strings (repo paths, explore listings, issue authorship, timeline events, notifications, invite subjects) — after the fix they all show usernames, and emails appear nowhere user-facing except the user's own settings.
Migration: existing repos/objects created under email-principals (e.g. anything stored as crueber@gmail.com/...) need a rename/mapping decision — at minimum a store-level alias so old keys resolve to the username namespace; enumerate affected keys in the PR.
The owner dropdown (#346 direction) then lists usernames + org names only — no emails anywhere in the options.

B. Import/create owner plumbing: with usernames in place, the owner dropdown options and the submitted owner are valid ID parts, and the bad target failure disappears. Until A lands, the form should at minimum filter out any option that isn't a valid ID part — but A is the fix, not the filter.

C. Remove "back to form" from the inline-error state: it should only render when the user is NOT already on the form (i.e. on the outcome/progress view after a started import). One conditional fix in Import.jsx.

Acceptance criteria

An OIDC user's principal name is a username (no @), assigned at first login, collision-uniquified, immutable, and stable across sessions.
The import form's owner dropdown contains only usernames/org names; an import under the user's username succeeds end-to-end (the bad target error is gone).
No email address is renderable to any user other than the owner of that email: audit repo paths, explore/owner listings, issue/PR authorship, timeline events, notifications, and invite subjects — test asserting emails absent from all of them.
The user's own email is visible only on their own profile/settings view.
Existing email-keyed state (repos created under email owners, if any) resolves via a documented migration/alias; the PR enumerates affected store keys.
"Back to form" renders only on non-form views (outcome/progress), never on the form itself.
Headless tests for username derivation/collision rules and the email-leak audit; UI verification of the import happy path and the corrected button state.

## What's wrong (three linked defects from the same root) The import form's owner dropdown offers and uses the user's **email address** (`crueber@gmail.com`) as the repo owner. Three consequences, all visible in the screenshot: 1. **The import fails**: `bad target: invalid repo id "crueber@gmail.com/walhub"` — the target `owner/name` parser (`git.ParseRepoId` via `ParseRequest`, `internal/repoimport/service.go`) rejects `@` and `.` in the owner segment, so the import can never succeed for an OIDC user. 2. **The email is exposed as a repo owner** — repo paths, listings, and the created repo's owner field would render `crueber@gmail.com` to any visitor. **Emails must never be exposed to users other than the logged-in user.** The principal name *is* the email today (see root cause), so every surface that renders `Principal.Name` leaks it: repo paths, `/explore`, issue authorship, the commit/author displays, notifications. 3. **A "back to form" button renders on the form itself** (screenshot, bottom-left, next to "start import") — a dead affordance that makes no sense in-place. ## Root cause (code evidence) - **The principal name IS the raw email in OIDC mode**: `principalFromEmail` returns `auth.Principal{Name: email, …}` (`internal/server/auth.go:230-277` — note :277, `Name: email`, lowercased but otherwise the full address). Every OIDC login, session mint, and `wgt_` token derives identity the same way (`:200-228`). - The owner dropdown in the import form lists the raw owners/principals — `you and your orgs only` (`web/src/pages/Import.jsx`, the `getOwners()` select in the current tree) — so for an OIDC user the dropdown contains their email, and that value flows into `owner` of the import request. - The backend then correctly rejects it: the owner segment must be a valid ID part (no `@`), and the identity model has **no username concept at all** — there is no `users/<username>` object, no display-name-vs-login separation for users. #234 added an owner *profile* (display name etc.) but the profile is keyed by the principal (the email), not by a username. - The **"back to form" button** is rendered by the import page's error/confirm state — it exists for the case where the import ran and the user lands on an outcome view, but it also renders (or persists) in the inline-error state where the user is already on the form. Wrong conditional, cosmetic but confusing on top of the failure. ## What's needed **A. Usernames as the identity key (the systemic fix — this is the real ticket):** 1. Introduce a **username** for every principal: derived at first OIDC login (email local-part, uniquified on collision — e.g. `crueber`, `crueber2`), stored on a user object (`users/<username>/user.json` mapping username ↔ email), and **immutable** thereafter. 2. `principalFromEmail` resolves email → username and returns `Principal{Name: <username>}` — the email never becomes a principal name, owner segment, or path component anywhere. 3. The email remains visible **only to the logged-in user themselves** (their own profile/settings page). Audit every surface that renders `Principal.Name` or owner strings (repo paths, explore listings, issue authorship, timeline events, notifications, invite subjects) — after the fix they all show usernames, and emails appear nowhere user-facing except the user's own settings. 4. **Migration**: existing repos/objects created under email-principals (e.g. anything stored as `crueber@gmail.com/...`) need a rename/mapping decision — at minimum a store-level alias so old keys resolve to the username namespace; enumerate affected keys in the PR. 5. The owner dropdown (#346 direction) then lists usernames + org names only — no emails anywhere in the options. **B. Import/create owner plumbing:** with usernames in place, the owner dropdown options and the submitted `owner` are valid ID parts, and the `bad target` failure disappears. Until A lands, the form should at minimum filter out any option that isn't a valid ID part — but A is the fix, not the filter. **C. Remove "back to form" from the inline-error state:** it should only render when the user is NOT already on the form (i.e. on the outcome/progress view after a started import). One conditional fix in `Import.jsx`. ## Acceptance criteria - [ ] An OIDC user's principal name is a username (no `@`), assigned at first login, collision-uniquified, immutable, and stable across sessions. - [ ] The import form's owner dropdown contains only usernames/org names; an import under the user's username succeeds end-to-end (the `bad target` error is gone). - [ ] No email address is renderable to any user other than the owner of that email: audit repo paths, explore/owner listings, issue/PR authorship, timeline events, notifications, and invite subjects — test asserting emails absent from all of them. - [ ] The user's own email is visible only on their own profile/settings view. - [ ] Existing email-keyed state (repos created under email owners, if any) resolves via a documented migration/alias; the PR enumerates affected store keys. - [ ] "Back to form" renders only on non-form views (outcome/progress), never on the form itself. - [ ] Headless tests for username derivation/collision rules and the email-leak audit; UI verification of the import happy path and the corrected button state.

crueber added the

issues

oidc

labels

2026-09-12 12:08:35 +00:00

crueber added this to the v1 milestone

2026-09-12 12:08:35 +00:00

crueber referenced this issue

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

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

crueber referenced this issue from a commit

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

Fix #370: OIDC usernames as identity key (01 §identity + 06 §8.4 + import UI)

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

2026-09-12 13:14:12 +00:00

Fix #370: OIDC usernames identity #373

crueber commented

2026-09-12 13:14:15 +00:00

Author

Owner

Fixed by PR #373 (#373): OIDC principals are usernames (derived at first login, collision-uniquified, immutable, email kept on Principal.Email for alias matching only). Owner dropdown holds usernames+orgs (username import admitted, email owner 400s), leak audit tests assert no cross-user email rendering (own email on own profile only), migration is a read-side alias with affected keys enumerated in docs/features/01 §Decisions, and back-to-form lives on the outcome view only.

Fixed by PR #373 (https://git.packden.us/crueber/walhub/pulls/373): OIDC principals are usernames (derived at first login, collision-uniquified, immutable, email kept on Principal.Email for alias matching only). Owner dropdown holds usernames+orgs (username import admitted, email owner 400s), leak audit tests assert no cross-user email rendering (own email on own profile only), migration is a read-side alias with affected keys enumerated in docs/features/01 §Decisions, and back-to-form lives on the outcome view only.

crueber referenced this issue from a commit

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

Fix #370 review: re-read CAS winner on username collision (same-email concurrent first-login must not split identity)

crueber commented

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

Author

Owner

Review: PR #373 (fix/issue-370) -- one real race found + fixed, rest verified

Scratch worktree /tmp/pr373 @ 4724e5b (includes my fix commit below). No browser (per instructions -- tests + reasoning; no UI behavior in this PR depends on a live render beyond the Import.jsx conditional, which is statically verified). Main worktree untouched (still clean).

internal/identity/users.go ResolveUsername: on PutCreate 412 the loser did continue to the NEXT suffix without re-reading the winner. Two sessions racing first-login for one email claimed TWO usernames (race + race2, two user.json for one email) -- the loser session then lives under the wrong username permanently (alias points at the winner, but the loser session + user.json persist). Proven with a barrier probe: 2/50 splits pre-fix. The file's own Concurrency section already documented "read the winner (same email -> adopt...)" -- the code just didn't do it.

Fix pushed to origin/fix/issue-370 (4724e5b): on 412, re-GET the candidate in the same pass -- same email -> adopt + repair alias; foreign email -> next suffix; store error on re-read -> fail closed. Plus committed regression test TestResolveUsernameConcurrentFirstLogin (50 barrier iterations, shared memory store). Post-fix: 5/5 -race runs clean, full identity + server/auth suites green.

Verified OK (adversarial pass)

Derivation (server/auth/username.go): no @ ever possible -- local-part only, every non-[a-z0-9._-] byte folded to -, and usernameFor re-validates via ValidUsername with pure-base fallback. Multi-@, unicode, leading dots, 64-char truncation + suffix room, ../user fallback, anonymous/anon -> -user suffix all sane.
Stability/sessions: wire keeps the verified email (emailOf), username re-resolves per request; pre-change sessions (wire=email) resolve to the username post-change. Distinct-email collision (crueber@gmail.com vs crueber@other.com -> crueber/crueber2) + repeat stability probed green.
Leak audit: Principal.Name is now the username by construction; deny messages, synthesized access, invite issuer (p.Name), org-hook titles, new-state rosters all username-only -- TestEmailLeakAudit + auth370 surface tests pin this. GET /users/{u} fills Email only on matchPrincipal self-match. No .Email renders to non-owners found in identity/server.
No #346 hole: ValidPrincipal accepting both spellings does NOT reintroduce email owners -- owner gate is CheckCreateOwner (self-match on username; email-as-owner falls to deny) + git.ParseRepoId (no @). isUserNamespace excludes synthetics/orgs (exact-key org.json probe)/unclaimed (user.json probe); errors fail closed. Writeless-self pin holds.
#346/#347 no-regress: Resolve/isOrgOwnerFor/inTeamFor/MemberOrgsFor via pure matchPrincipal (no extra round trips, law 6 holds); CheckPush forwards {Name, Email} with flags stripped.
Import: allowedOwners + isValidOwnerPart filter emails client-side; server 400s bad targets regardless. Back-to-form moved into outcome view only; inline-error copy verified gone.
Coverage/gates: identity 95.6%, auth 100%, server 98.3%, repoimport 95.7% (all >=95); -race clean; go build, gofmt, go vet clean; node 721/723 (2 smoke.test.js failures pre-existing on main -- missing concepts/push.gif asset); TestUIAssetConcepts fails identically on main. No new deps.

Residuals (not blocking -- pre-existing data shapes, follow-ups)

Legacy email-spelled bindings render verbatim (serveCollaborators, team/org-owner expansion, GetAccess, rosters, stored issue/PR authors) -- criterion 3 holds for new state only. Follow-up: rewrite or read-side masking.
Profile split: ProfileKey(email) vs ProfileKey(username) -- pre-migration display names stay under the email key; post-migration reads the username key. Consider lazy profile migration on first login.
Placeholder re-affirm: samePrincipal(doc.CreatedBy, p.Name) (api/placeholder.go:445, summary.go:248) -- legacy email-CreatedBy unborn placeholders 409 for the same user post-migration; also missing from the 01 Decisions affected-keys enumeration.
leak_test.go step 4 (collaborators to other user) is a no-op pin (_ = w) -- make it a real assertion when (1) is addressed.

MERGE RECOMMENDATION: ready to merge

The one structural defect (CAS split) is fixed on the branch with a regression test; all gates hold; residuals are documented pre-existing-data limitations, not regressions.

## Review: PR #373 (fix/issue-370) -- one real race found + fixed, rest verified Scratch worktree `/tmp/pr373` @ `4724e5b` (includes my fix commit below). No browser (per instructions -- tests + reasoning; no UI behavior in this PR depends on a live render beyond the Import.jsx conditional, which is statically verified). Main worktree untouched (still clean). ### FOUND + FIXED: same-email concurrent first-login split identity (CAS race) `internal/identity/users.go` `ResolveUsername`: on `PutCreate` 412 the loser did `continue` to the NEXT suffix without re-reading the winner. Two sessions racing first-login for one email claimed TWO usernames (`race` + `race2`, two `user.json` for one email) -- the loser session then lives under the wrong username permanently (alias points at the winner, but the loser session + `user.json` persist). Proven with a barrier probe: 2/50 splits pre-fix. The file's own Concurrency section already documented "read the winner (same email -> adopt...)" -- the code just didn't do it. Fix pushed to `origin/fix/issue-370` (`4724e5b`): on 412, re-GET the candidate in the same pass -- same email -> adopt + repair alias; foreign email -> next suffix; store error on re-read -> fail closed. Plus committed regression test `TestResolveUsernameConcurrentFirstLogin` (50 barrier iterations, shared memory store). Post-fix: 5/5 `-race` runs clean, full `identity` + `server/auth` suites green. ### Verified OK (adversarial pass) 1. Derivation (`server/auth/username.go`): no `@` ever possible -- local-part only, every non-`[a-z0-9._-]` byte folded to `-`, and `usernameFor` re-validates via `ValidUsername` with pure-base fallback. Multi-`@`, unicode, leading dots, 64-char truncation + suffix room, `..`/`user` fallback, `anonymous`/`anon` -> `-user` suffix all sane. 2. Stability/sessions: wire keeps the verified email (`emailOf`), username re-resolves per request; pre-change sessions (wire=email) resolve to the username post-change. Distinct-email collision (`crueber@gmail.com` vs `crueber@other.com` -> `crueber`/`crueber2`) + repeat stability probed green. 3. Leak audit: `Principal.Name` is now the username by construction; deny messages, synthesized access, invite issuer (`p.Name`), org-hook titles, new-state rosters all username-only -- `TestEmailLeakAudit` + `auth370` surface tests pin this. `GET /users/{u}` fills `Email` only on `matchPrincipal` self-match. No `.Email` renders to non-owners found in `identity`/`server`. 4. No #346 hole: `ValidPrincipal` accepting both spellings does NOT reintroduce email owners -- owner gate is `CheckCreateOwner` (self-match on username; email-as-owner falls to deny) + `git.ParseRepoId` (no `@`). `isUserNamespace` excludes synthetics/orgs (exact-key `org.json` probe)/unclaimed (`user.json` probe); errors fail closed. Writeless-self pin holds. 5. #346/#347 no-regress: `Resolve`/`isOrgOwnerFor`/`inTeamFor`/`MemberOrgsFor` via pure `matchPrincipal` (no extra round trips, law 6 holds); `CheckPush` forwards `{Name, Email}` with flags stripped. 6. Import: `allowedOwners` + `isValidOwnerPart` filter emails client-side; server 400s bad targets regardless. Back-to-form moved into outcome view only; inline-error copy verified gone. 7. Coverage/gates: identity 95.6%, auth 100%, server 98.3%, repoimport 95.7% (all >=95); `-race` clean; `go build`, `gofmt`, `go vet` clean; node 721/723 (2 `smoke.test.js` failures pre-existing on main -- missing `concepts/push.gif` asset); `TestUIAssetConcepts` fails identically on main. No new deps. ### Residuals (not blocking -- pre-existing data shapes, follow-ups) 1. Legacy email-spelled bindings render verbatim (`serveCollaborators`, team/org-owner expansion, `GetAccess`, rosters, stored issue/PR authors) -- criterion 3 holds for new state only. Follow-up: rewrite or read-side masking. 2. Profile split: `ProfileKey(email)` vs `ProfileKey(username)` -- pre-migration display names stay under the email key; post-migration reads the username key. Consider lazy profile migration on first login. 3. Placeholder re-affirm: `samePrincipal(doc.CreatedBy, p.Name)` (`api/placeholder.go:445`, `summary.go:248`) -- legacy email-`CreatedBy` unborn placeholders 409 for the same user post-migration; also missing from the 01 Decisions affected-keys enumeration. 4. `leak_test.go` step 4 (collaborators to other user) is a no-op pin (`_ = w`) -- make it a real assertion when (1) is addressed. ### MERGE RECOMMENDATION: ready to merge The one structural defect (CAS split) is fixed on the branch with a regression test; all gates hold; residuals are documented pre-existing-data limitations, not regressions.

crueber closed this issue

2026-09-12 13:21:00 +00:00

crueber referenced this issue from a commit

2026-09-12 13:21:00 +00:00

Merge pull request 'Fix #370: OIDC usernames identity' (#373) from fix/issue-370 into main

crueber commented

2026-09-12 13:21:01 +00:00

Author

Owner

Fixed by PR #373 (review caught + fixed a concurrent-first-login split-brain; derivation, leak audit, migration, import, back-to-form verified), merged. Closing.

crueber referenced this issue

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

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

crueber referenced this issue

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

Auto-generated user avatars: dicebear-go (constellation style, electric preset, seed = email) generated in background on login #376

crueber referenced this issue

2026-09-12 15:23:54 +00:00

Auto-generated user avatars: dicebear-go (constellation style, electric preset, seed = email) generated in background on login #376

crueber referenced this issue

2026-09-12 22:56:29 +00:00

Owner profile: organizations membership list with explicit empty state (backend membership rail missing) #423