From 4fe82efcb7479ff75762032d7822f7b255250011 Mon Sep 17 00:00:00 2001 From: Andy Anderson Date: Fri, 28 Aug 2026 14:12:44 -0400 Subject: [PATCH 001/223] =?UTF-8?q?=F0=9F=93=96=20docs(gtr):=20correct=20t?= =?UTF-8?q?he=20NOTICE=20status=20=E2=80=94=20the=20generator=20could=20no?= =?UTF-8?q?t=20run,=20not=20just=20drift=20(#5012)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The attribution answer said the notice-drift job "fails until a maintainer commits the authoritative go-licenses output", implying that output existed and only needed committing. It did not. Its first real run against v4 failed BEFORE generating anything, on a defect rather than the placeholder drift it was built to detect: LICENSE lives at the repository root while the Go module is src/, so go-licenses searched upward, stopped at the module root, reported every one of the project's own packages as unlicensed, and exited non-zero. #5010 fixes it by excluding the project's own module, which a third-party notice should not list anyway. So this is two steps, not one, and the GTR now says which. The distinction matters to a reviewer: "output generated, awaiting commit" and "generator broken, no output" describe different degrees of readiness, and a reviewer who follows the link finds the second. This also illustrates something worth stating: an expected-to-fail check is a bad place to hide a real failure. From a check list, "failing as designed" and "failing for a reason nobody predicted" look identical, and the design comment argues for reading it as the former. The only reason it surfaced was going after the generated artifact rather than trusting the failure's stated meaning. Swept the rest of the document for claims that went stale since it was written; found none — the single-maintainer and badge references are already the corrected ones. Signed-off-by: Andy Anderson --- src/docs/general-technical-review.md | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/docs/general-technical-review.md b/src/docs/general-technical-review.md index 39ecc329f..c38424d5f 100644 --- a/src/docs/general-technical-review.md +++ b/src/docs/general-technical-review.md @@ -27,8 +27,10 @@ plainly in the relevant answers below rather than hidden behind a marker: - **`NOTICE` data not yet authoritative** — the attribution mechanism now exists (generator, `notice-drift` CI guard, release attachment), but the committed `NOTICE` is a statically-derived placeholder whose license - fields all read `UNVERIFIED`. No license was inferred; the authoritative - `go-licenses` output requires one maintainer step, tracked in + fields all read `UNVERIFIED`. No license was inferred. The generator could + not run at all until [#5010](https://github.com/kubestellar/hive/pull/5010) + (a repo-layout defect: `LICENSE` at the root, module in `src/`); after that + lands, committing the real output is tracked in [#5007](https://github.com/kubestellar/hive/issues/5007) ([attribution](#what-steps-does-the-project-take-to-ensure-that-all-third-party-code-and-components-have-correct-and-complete-attribution-and-license-notices)). - **No formal compliance certification** — no SOC 2, FedRAMP, or other @@ -441,7 +443,7 @@ The process and its enforcement are in place; the authoritative data is not yet A repo-root `NOTICE` lists every Go module dependency compiled into `hive`, `hive-hub`, and `hive-contributor`, generated by `src/scripts/generate-notice.sh` (pinned `google/go-licenses`) and kept current by the `notice-drift` CI job in `go-security-analysis.yml`, which regenerates on every change to `src/go.mod`, `src/go.sum`, or the script — and weekly, since an upstream dependency can relicense with our files untouched. Tagged releases attach `NOTICE` alongside the per-image SBOMs (`src/docs/releases.md`). -**The committed `NOTICE` is currently a statically-derived placeholder.** It was assembled from `src/go.mod` without running Go tooling, so every license field reads `UNVERIFIED` and transitive modules present only in `go.sum` may be absent. No license identifier was inferred or fabricated — an attribution file asserting a wrong license is a false legal claim, which is worse than a missing one. The `notice-drift` job fails until a maintainer commits the authoritative `go-licenses` output; that single remaining step is tracked in [#5007](https://github.com/kubestellar/hive/issues/5007). Note `go.sum` pinning is integrity and provenance tracking, not license attribution, and never produced an assembled notice by itself. +**The committed `NOTICE` is currently a statically-derived placeholder.** It was assembled from `src/go.mod` without running Go tooling, so every license field reads `UNVERIFIED` and transitive modules present only in `go.sum` may be absent. No license identifier was inferred or fabricated — an attribution file asserting a wrong license is a false legal claim, which is worse than a missing one. The `notice-drift` job currently fails, and its first run against `v4` revealed a generator defect rather than the placeholder drift it was built to detect: `LICENSE` lives at the repository root while the Go module is `src/`, so `go-licenses` searched upward, stopped at the module root, reported every one of the project's own packages as unlicensed, and exited before producing any output. [#5010](https://github.com/kubestellar/hive/pull/5010) fixes that by excluding the project's own module — its code does not belong in a third-party notice regardless. Once that lands, the job produces real `go-licenses` output and a maintainer commits it, tracked in [#5007](https://github.com/kubestellar/hive/issues/5007). Note `go.sum` pinning is integrity and provenance tracking, not license attribution, and never produced an assembled notice by itself. #### Describe how the project ensures alignment with CNCF recommendations for attribution notices. From 71c3a0ede82a1c6a86b2f73ee60782513bf8af64 Mon Sep 17 00:00:00 2001 From: Andy Anderson Date: Fri, 28 Aug 2026 14:15:26 -0400 Subject: [PATCH 002/223] =?UTF-8?q?=F0=9F=90=9B=20show=20OIDC=20display=20?= =?UTF-8?q?names=20instead=20of=20raw=20identity=20keys=20across=20hub=20a?= =?UTF-8?q?nd=20spoke=20UIs=20(#5013)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit OIDC users (ibmid:/google:/microsoft:) rendered as opaque subject keys (ibmid:5500087VJB) in surfaces the earlier Manage Access fix did not cover. Resolve them to stored display names at serve time - presentation only, every key, filter and authorization check stays on the raw identity. Hub: identityLabeler memoized resolver; Usage panel Label/OwnerName; My Hives OwnerName for Group-by-Owner; Timeline/access-log ActorName on served copies; Alert AckByName; access-faces +N tooltip uses display_label. Spoke: /api/role display_name from AuthorizedUserNames; header chip shows display name with initials avatar and no fabricated github.com profile/avatar for provider keys; audit log user_name decorated at serve time with provider-aware avatar. Signed-off-by: Andy Anderson Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/pkg/dashboard/api.go | 21 +- src/pkg/dashboard/audit.go | 11 ++ src/pkg/dashboard/clickable_avatars_test.go | 8 +- .../oidc_header_audit_display_test.go | 154 +++++++++++++++ src/pkg/dashboard/static/index.html | 60 +++++- src/pkg/hub/alerts.go | 17 +- src/pkg/hub/oauth.go | 20 ++ .../hub/oidc_identity_labels_serve_test.go | 185 ++++++++++++++++++ src/pkg/hub/saas.go | 42 +++- src/pkg/hub/timeline.go | 19 ++ src/pkg/hub/usage.go | 26 +++ 11 files changed, 539 insertions(+), 24 deletions(-) create mode 100644 src/pkg/dashboard/oidc_header_audit_display_test.go create mode 100644 src/pkg/hub/oidc_identity_labels_serve_test.go diff --git a/src/pkg/dashboard/api.go b/src/pkg/dashboard/api.go index 074d4f385..81fd1fe29 100644 --- a/src/pkg/dashboard/api.go +++ b/src/pkg/dashboard/api.go @@ -571,13 +571,30 @@ func (s *Server) handleRole(w http.ResponseWriter, r *http.Request) { if role == "" { role = "owner" } - jsonResponse(w, map[string]string{ + resp := map[string]string{ "role": role, "user": user, // The queue label is server-configured, so the dashboard must be told // it rather than hard-coding a name that a hive may have changed. "automerge_label": s.autoMergeLabel(), - }) + } + // display_name is the human name for an opaque OIDC identity key + // ("ibmid:5500…"), delivered by the hub heartbeat (AuthorizedUserNames). + // Purely cosmetic — the header chip shows it; every auth decision stays on + // the raw user key. Absent when unknown; the UI falls back to the key. + if dn := s.authorizedDisplayName(user); dn != "" && dn != user { + resp["display_name"] = dn + } + jsonResponse(w, resp) +} + +// authorizedDisplayName looks up the hub-delivered cosmetic display name for +// an identity key. Nil-safe: /api/role is served before deps are required. +func (s *Server) authorizedDisplayName(user string) string { + if s == nil || s.deps == nil || s.deps.Config == nil || user == "" { + return "" + } + return strings.TrimSpace(s.deps.Config.Dashboard.AuthorizedUserNames[user]) } // autoMergeLabel reports the configured queue label. /api/role is served diff --git a/src/pkg/dashboard/audit.go b/src/pkg/dashboard/audit.go index 9b7af7171..7257b090c 100644 --- a/src/pkg/dashboard/audit.go +++ b/src/pkg/dashboard/audit.go @@ -48,6 +48,10 @@ type AuditEntry struct { Action string `json:"action"` Detail string `json:"detail,omitempty"` Agent string `json:"agent,omitempty"` + // UserName is the hub-delivered display name when User is an opaque OIDC + // identity key. Stamped at SERVE time only (handleAuditLog) — the ring and + // the on-disk log keep the raw key, so history survives name changes. + UserName string `json:"user_name,omitempty"` } type AuditLog struct { @@ -301,6 +305,13 @@ func (s *Server) handleAuditLog(w http.ResponseWriter, r *http.Request) { return } entries := s.audit.Recent(auditMaxEntries) + // Cosmetic: attach display names for opaque OIDC actor keys. Recent() + // returns copies, so the ring itself is never mutated. + for i := range entries { + if dn := s.authorizedDisplayName(entries[i].User); dn != "" && dn != entries[i].User { + entries[i].UserName = dn + } + } jsonResponse(w, map[string]any{"entries": entries}) } diff --git a/src/pkg/dashboard/clickable_avatars_test.go b/src/pkg/dashboard/clickable_avatars_test.go index 3478e4ae0..4069c1c80 100644 --- a/src/pkg/dashboard/clickable_avatars_test.go +++ b/src/pkg/dashboard/clickable_avatars_test.go @@ -39,7 +39,11 @@ func TestAvatarSitesUseSharedHelper(t *testing.T) { // hand-rolls an . TestAccessRowAvatarDelegates below pins the // delegation so this indirection cannot become a bypass. {"access tab allowlist", "accessRowAvatar(u.username, u.display_name || '', ACCESS_LIST_AVATAR_PX)"}, - {"audit log actor", "? linkedAvatar(actor, AUDIT_AVATAR_PX, actor, '', 'margin-right:4px')"}, + // The audit log's GitHub-shaped actors still go through linkedAvatar; + // provider-prefixed OIDC keys route through accessRowAvatar (initials + // tile, no fabricated github.com link) — same split as the access tab. + {"audit log actor", "face = linkedAvatar(actor, AUDIT_AVATAR_PX, actor, '', 'margin-right:4px');"}, + {"audit log OIDC actor", "face = accessRowAvatar(actor, actorLabel, AUDIT_AVATAR_PX);"}, } for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { @@ -163,7 +167,7 @@ func TestRedundantProfileAffordancesRemoved(t *testing.T) { // profile and imply they performed the action. func TestAnonymousAuditActorIsNotLinked(t *testing.T) { html := indexHTML(t) - if !strings.Contains(html, ": avatarImg(ghProfileURL('ghost') + '.png', AUDIT_AVATAR_PX, 'margin-right:4px');") { + if !strings.Contains(html, "face = avatarImg(ghProfileURL('ghost') + '.png', AUDIT_AVATAR_PX, 'margin-right:4px');") { t.Error("the audit log's actorless placeholder face is no longer rendered unlinked") } if strings.Contains(html, "linkedAvatar(e.user||'ghost'") { diff --git a/src/pkg/dashboard/oidc_header_audit_display_test.go b/src/pkg/dashboard/oidc_header_audit_display_test.go new file mode 100644 index 000000000..e8fa8e06c --- /dev/null +++ b/src/pkg/dashboard/oidc_header_audit_display_test.go @@ -0,0 +1,154 @@ +package dashboard + +// Tests for the spoke's remaining OIDC identity DISPLAY surfaces: the top-bar +// account chip (/api/role → header avatar menu) and the Settings → Audit Log +// actor column. Presentation only — the raw identity key keeps driving every +// permission decision, and each case asserts it survives unchanged alongside +// the optional resolved name. + +import ( + "encoding/json" + "net/http" + "net/http/httptest" + "strings" + "testing" +) + +// roleGet performs GET /api/role with the auth-proxy identity headers set. +func roleGet(t *testing.T, s *Server, user, role string) map[string]string { + t.Helper() + req := httptest.NewRequest(http.MethodGet, "/api/role", nil) + req.Header.Set("X-Hive-User", user) + req.Header.Set("X-Hive-Role", role) + rec := httptest.NewRecorder() + s.handleRole(rec, req) + if rec.Code != http.StatusOK { + t.Fatalf("GET /api/role = %d, want 200", rec.Code) + } + var body map[string]string + if err := json.Unmarshal(rec.Body.Bytes(), &body); err != nil { + t.Fatalf("unmarshal: %v", err) + } + return body +} + +// TestRoleCarriesDisplayNameForOIDCIdentity asserts /api/role attaches +// display_name from the hub-delivered AuthorizedUserNames map for an opaque +// OIDC key, while user stays the raw key the UI must keep authorizing with. +func TestRoleCarriesDisplayNameForOIDCIdentity(t *testing.T) { + s, deps := apiServer(t) + deps.Config.Dashboard.AuthorizedUserNames = map[string]string{ + "ibmid:5500087VJB": "Jane Doe", + } + body := roleGet(t, s, "ibmid:5500087VJB", "read") + if body["user"] != "ibmid:5500087VJB" { + t.Fatalf("user = %q, want the untouched raw key", body["user"]) + } + if body["display_name"] != "Jane Doe" { + t.Errorf(`display_name = %q, want "Jane Doe"`, body["display_name"]) + } +} + +// TestRoleOmitsDisplayNameWhenUnknown asserts no display_name is emitted for +// a key the hub has no name for (or when the map was never delivered): absent +// means "render the raw key", never blank/undefined. +func TestRoleOmitsDisplayNameWhenUnknown(t *testing.T) { + s, deps := apiServer(t) + deps.Config.Dashboard.AuthorizedUserNames = map[string]string{ + "ibmid:OTHERUSER": "Someone Else", + } + body := roleGet(t, s, "google:NONAMECLAIM", "read") + if v, ok := body["display_name"]; ok && v != "" { + t.Errorf("display_name = %q for an unknown key, want absent", v) + } + + deps.Config.Dashboard.AuthorizedUserNames = nil + body = roleGet(t, s, "clubanderson", "owner") + if v, ok := body["display_name"]; ok && v != "" { + t.Errorf("display_name = %q with a nil names map, want absent", v) + } + if body["user"] != "clubanderson" { + t.Fatalf("user = %q, want unchanged", body["user"]) + } +} + +// TestAuditLogDecoratesUserNameServeTimeOnly asserts handleAuditLog attaches +// user_name for OIDC actors with a hub-resolved name, leaves it absent +// otherwise, keeps the raw user on the wire, and never mutates the ring (the +// on-disk log and later reads must keep raw keys). +func TestAuditLogDecoratesUserNameServeTimeOnly(t *testing.T) { + s, deps := apiServer(t) + deps.Config.Dashboard.AuthorizedUserNames = map[string]string{ + "ibmid:5500087VJB": "Jane Doe", + } + s.audit.Log("ibmid:5500087VJB", "test-action", "detail", "") + s.audit.Log("clubanderson", "test-action-2", "detail", "") + + req := httptest.NewRequest(http.MethodGet, "/api/audit-log", nil) + req.Header.Set("X-Hive-Role", "owner") + rec := httptest.NewRecorder() + s.handleAuditLog(rec, req) + if rec.Code != http.StatusOK { + t.Fatalf("GET /api/audit-log = %d, want 200", rec.Code) + } + var body struct { + Entries []AuditEntry `json:"entries"` + } + if err := json.Unmarshal(rec.Body.Bytes(), &body); err != nil { + t.Fatalf("unmarshal: %v", err) + } + var oidc, gh *AuditEntry + for i := range body.Entries { + switch body.Entries[i].User { + case "ibmid:5500087VJB": + oidc = &body.Entries[i] + case "clubanderson": + gh = &body.Entries[i] + } + } + if oidc == nil || gh == nil { + t.Fatalf("audit entries missing: %+v", body.Entries) + } + if oidc.UserName != "Jane Doe" { + t.Errorf(`OIDC entry user_name = %q, want "Jane Doe"`, oidc.UserName) + } + if gh.UserName != "" { + t.Errorf("GitHub entry user_name = %q, want empty (key is already the label)", gh.UserName) + } + // Serve-time only: the ring itself must still hold raw, undecorated rows. + for _, e := range s.audit.Recent(10) { + if e.UserName != "" { + t.Errorf("ring entry for %q was mutated with UserName %q — decoration must stay serve-time", e.User, e.UserName) + } + } +} + +// TestHeaderChipAndAuditRowsRenderResolvedNames pins the index.html render +// sites: the header account chip must show display_name (with an initials +// avatar and no fabricated github.com profile/avatar for provider-prefixed +// keys), and the audit table must prefer user_name while keeping the raw key +// in the row tooltip. +func TestHeaderChipAndAuditRowsRenderResolvedNames(t *testing.T) { + html := indexHTML(t) + for _, snippet := range []string{ + // checkGHAuth: no github.com/.png for OIDC keys, and the resolved + // name rides along to showGHAuthState. + "showGHAuthState(true, roleData.user, ghAvatar, roleData.role, roleData.display_name);", + "var ghAvatar = roleData.user.indexOf(':') === -1", + // showGHAuthState: display name in the menu, initials avatar fallback, + // profile entry hidden for provider keys. + "const shownName = displayName || username;", + "img.src = initialsAvatarURI(shownName);", + "if (username && !isProviderKey) {", + "function initialsAvatarURI(name)", + // Audit rows: resolved label + provider-aware avatar, raw key kept as + // the cell tooltip. + "var actorLabel = String(e.user_name || e.user || '');", + "face = accessRowAvatar(actor, actorLabel, AUDIT_AVATAR_PX);", + `title="' + esc(actor) + '">' + face + esc(actorLabel || '—')`, + } { + if !strings.Contains(html, snippet) { + t.Errorf("index.html missing expected snippet: %q", snippet) + } + } +} diff --git a/src/pkg/dashboard/static/index.html b/src/pkg/dashboard/static/index.html index 59e53ad48..ba632c718 100644 --- a/src/pkg/dashboard/static/index.html +++ b/src/pkg/dashboard/static/index.html @@ -12051,12 +12051,21 @@

How Agents Use Knowledge (Priming)

rendered UNLINKED by passing no username, rather than sending the reader to a stranger's profile. */ var actor = String(e.user || ''); - var face = actor - ? linkedAvatar(actor, AUDIT_AVATAR_PX, actor, '', 'margin-right:4px') - : avatarImg(ghProfileURL('ghost') + '.png', AUDIT_AVATAR_PX, 'margin-right:4px'); + /* user_name is the server-resolved display name for an opaque OIDC key + ("ibmid:5500…"); provider-prefixed keys get an unlinked initials tile + via accessRowAvatar instead of a fabricated github.com avatar/link. */ + var actorLabel = String(e.user_name || e.user || ''); + var face; + if (!actor) { + face = avatarImg(ghProfileURL('ghost') + '.png', AUDIT_AVATAR_PX, 'margin-right:4px'); + } else if (actor.indexOf(':') !== -1 || identityKeyProvider(actor) !== 'github') { + face = accessRowAvatar(actor, actorLabel, AUDIT_AVATAR_PX); + } else { + face = linkedAvatar(actor, AUDIT_AVATAR_PX, actor, '', 'margin-right:4px'); + } return '' + '' + timeStr + '' + - '' + face + esc(e.user||'—') + '' + + '' + face + esc(actorLabel || '—') + '' + '' + esc(e.action||'—') + '' + '' + esc(e.agent||'—') + '' + '' + esc(e.detail||'') + '' + @@ -22076,12 +22085,32 @@

Escalation Breaker

setTimeout(() => { btn.textContent = 'Copy'; btn.style.background = 'var(--green)'; }, 2000); } - function showGHAuthState(loggedIn, username, avatarURL, role) { + /* Data-URI SVG avatar showing the person's initial — used for OIDC + identities ("ibmid:5500…") that have no GitHub avatar to fetch. */ + function initialsAvatarURI(name) { + var base = String(name || '?'); + var colon = base.indexOf(':'); + if (colon !== -1 && colon + 1 < base.length) base = base.slice(colon + 1); + var ch = (base.trim().charAt(0) || '?').toUpperCase(); + var svg = '' + + '' + + '' + + ch.replace(/&/g, '&').replace(//g, '>') + ''; + return 'data:image/svg+xml;charset=utf-8,' + encodeURIComponent(svg); + } + + function showGHAuthState(loggedIn, username, avatarURL, role, displayName) { if (username) window._hiveUser = username; if (role) window._hiveRole = role; const banner = document.getElementById('gh-auth-banner'); const topBtn = document.getElementById('oc-gh-login-btn'); const avatarWrap = document.getElementById('oc-gh-avatar-wrap'); + /* OIDC identity keys carry a provider prefix ("ibmid:…", "google:…"). + They have no github.com avatar or profile page, and the menu should + show the person's display name, not the opaque key. Every permission + check elsewhere still runs on the raw key. */ + const isProviderKey = !!(username && String(username).indexOf(':') !== -1); + const shownName = displayName || username; if (loggedIn) { banner.style.display = 'none'; if (topBtn) topBtn.style.display = 'none'; @@ -22090,16 +22119,20 @@

Escalation Breaker

const img = document.getElementById('oc-gh-avatar'); if (avatarURL) { img.src = avatarURL + (avatarURL.includes('?') ? '&' : '?') + 's=56'; - img.alt = username; + img.alt = shownName; + } else if (isProviderKey) { + img.src = initialsAvatarURI(shownName); + img.alt = shownName; } - document.getElementById('oc-gh-menu-user').textContent = username; + document.getElementById('oc-gh-menu-user').textContent = shownName; /* Point the menu's profile entry at the signed-in account, and show it only once there is a login to point at. href is assigned as a property (not interpolated into markup), so encodeURIComponent on - the path segment is the whole escaping story here. */ + the path segment is the whole escaping story here. OIDC keys have + no forge profile page — hide the entry entirely. */ const profEl = document.getElementById('oc-gh-menu-profile'); if (profEl) { - if (username) { + if (username && !isProviderKey) { profEl.href = ghProfileURL(username); profEl.style.display = 'block'; } else { @@ -22219,7 +22252,14 @@

Escalation Breaker

const roleData = await roleResp.json(); if (roleData.user) { if (roleData.automerge_label) window._hiveAutoMergeLabel = roleData.automerge_label; - showGHAuthState(true, roleData.user, 'https://github.com/' + roleData.user + '.png', roleData.role); + /* OIDC identity keys ("ibmid:5500…") are not GitHub logins — don't + fabricate a github.com/.png avatar for them; showGHAuthState + falls back to an initials avatar and the server-resolved + display_name. */ + var ghAvatar = roleData.user.indexOf(':') === -1 + ? 'https://github.com/' + roleData.user + '.png' + : ''; + showGHAuthState(true, roleData.user, ghAvatar, roleData.role, roleData.display_name); return; } } catch (e) {} diff --git a/src/pkg/hub/alerts.go b/src/pkg/hub/alerts.go index 832bc8012..896bdc29d 100644 --- a/src/pkg/hub/alerts.go +++ b/src/pkg/hub/alerts.go @@ -223,6 +223,10 @@ type Alert struct { // unacknowledged alert, which the client would have to special-case. AckBy string `json:"ackBy,omitempty"` AckAt *time.Time `json:"ackAt,omitempty"` + // AckByName is the resolved display label when AckBy is an opaque OIDC + // identity with a stored display name. Serve-time cosmetic only; the ack + // record itself keeps the raw identity. + AckByName string `json:"ackByName,omitempty"` } // AlertSummary is the whole fleet's alert state, shaped for a single render @@ -1208,7 +1212,18 @@ func (s *HubServer) fleetAlerts(entries []MyHiveEntry) AlertSummary { for _, e := range entries { regs = append(regs, e.RegistryEntry) } - return evaluateAlerts(s.alerts, hives, s.urlUnreachableAlerts(regs, now), now) + summary := evaluateAlerts(s.alerts, hives, s.urlUnreachableAlerts(regs, now), now) + // Cosmetic: resolve opaque OIDC ack identities to display names for the + // "— ack by …" note. The stored ack keeps the raw identity. + label := s.identityLabeler() + for i := range summary.Alerts { + if by := summary.Alerts[i].AckBy; by != "" { + if l := label(by); l != by { + summary.Alerts[i].AckByName = l + } + } + } + return summary } // alertAcksFileMu serialises writers of the acks file. Two concurrent ack diff --git a/src/pkg/hub/oauth.go b/src/pkg/hub/oauth.go index e402c8eb4..536fd652c 100644 --- a/src/pkg/hub/oauth.go +++ b/src/pkg/hub/oauth.go @@ -911,6 +911,26 @@ func (s *HubServer) displayIdentity(identity string) (login, avatarURL string) { return login, avatarURL } +// identityLabeler returns a per-request memoized resolver from a canonical (or +// legacy bare) identity to its human display label — displayIdentity's login +// half, cached so a list that repeats the same few owners does one lookup per +// DISTINCT identity instead of one stored-user read per row. Empty in, empty +// out. NOT safe for concurrent use; intended for a single handler invocation. +func (s *HubServer) identityLabeler() func(string) string { + cache := map[string]string{} + return func(identity string) string { + if identity == "" { + return "" + } + if l, ok := cache[identity]; ok { + return l + } + l, _ := s.displayIdentity(identity) + cache[identity] = l + return l + } +} + func (s *HubServer) handleAuthUser(w http.ResponseWriter, r *http.Request) { // Trust a carried username only when its signature verifies; a legacy // unsigned or forged cookie reports unauthenticated, prompting a re-login. diff --git a/src/pkg/hub/oidc_identity_labels_serve_test.go b/src/pkg/hub/oidc_identity_labels_serve_test.go new file mode 100644 index 000000000..aa3b75389 --- /dev/null +++ b/src/pkg/hub/oidc_identity_labels_serve_test.go @@ -0,0 +1,185 @@ +package hub + +// Tests for the serve-time OIDC display-name decoration added for the Usage +// panel, "Group by owner", Activity Timeline, access log and alert-ack +// surfaces. Everything here is presentation only: each case asserts the raw +// identity key is left untouched on the wire and a SEPARATE optional field +// carries the resolved human name — never a rewrite of the key itself. + +import ( + "strings" + "testing" + "time" +) + +// TestIdentityLabelerResolvesAndFallsBack pins the resolver the decorations +// share: a stored OIDC user resolves to their display name, an unknown or +// GitHub-native key resolves to itself, and empty stays empty. +func TestIdentityLabelerResolvesAndFallsBack(t *testing.T) { + withTempSaaSDirs(t) + if err := saveSaaSUser(&SaaSUser{ + GitHubUsername: "ibmid:5500087VJB", + Provider: "ibmid", + DisplayName: "Jane Doe", + }); err != nil { + t.Fatalf("saveSaaSUser: %v", err) + } + s := &HubServer{} + label := s.identityLabeler() + if got := label("ibmid:5500087VJB"); got != "Jane Doe" { + t.Errorf(`label("ibmid:5500087VJB") = %q, want "Jane Doe"`, got) + } + // Memoized second read must agree. + if got := label("ibmid:5500087VJB"); got != "Jane Doe" { + t.Errorf(`memoized label = %q, want "Jane Doe"`, got) + } + if got := label("google:NEVERSEEN"); got != "google:NEVERSEEN" { + t.Errorf("unknown key must resolve to itself, got %q", got) + } + if got := label(""); got != "" { + t.Errorf(`label("") = %q, want ""`, got) + } +} + +// TestDecorateTimelineActorsServeTimeOnly asserts ActorName is stamped on the +// served copies for OIDC actors with a known name, omitted otherwise, and +// that Actor (the raw key history must keep) is never rewritten. +func TestDecorateTimelineActorsServeTimeOnly(t *testing.T) { + withTempSaaSDirs(t) + if err := saveSaaSUser(&SaaSUser{ + GitHubUsername: "ibmid:5500087VJB", + Provider: "ibmid", + DisplayName: "Jane Doe", + }); err != nil { + t.Fatalf("saveSaaSUser: %v", err) + } + s := &HubServer{} + events := []TimelineEvent{ + {TS: "2026-08-28T00:00:00Z", Kind: TimelineAccess, Actor: "ibmid:5500087VJB"}, + {TS: "2026-08-28T00:01:00Z", Kind: TimelineAccess, Actor: "clubanderson"}, + {TS: "2026-08-28T00:02:00Z", Kind: TimelineAccess}, + } + s.decorateTimelineActors(events) + if events[0].Actor != "ibmid:5500087VJB" { + t.Fatalf("raw Actor was rewritten to %q — decoration must be a separate field", events[0].Actor) + } + if events[0].ActorName != "Jane Doe" { + t.Errorf(`ActorName = %q, want "Jane Doe"`, events[0].ActorName) + } + if events[1].ActorName != "" { + t.Errorf("GitHub-native actor got ActorName %q, want empty (key is already the label)", events[1].ActorName) + } + if events[2].ActorName != "" { + t.Errorf("actorless event got ActorName %q, want empty", events[2].ActorName) + } +} + +// TestUsageOwnerBucketsCarryLabel asserts the /api/saas/usage decoration: +// an owner bucket keyed by an opaque OIDC identity carries Label, the raw Key +// is untouched (jump/filter actions depend on it), and buckets whose key is +// already the best label carry no Label at all. +func TestUsageOwnerBucketsCarryLabel(t *testing.T) { + withTempSaaSDirs(t) + if err := saveSaaSUser(&SaaSUser{ + GitHubUsername: "ibmid:5500087VJB", + Provider: "ibmid", + DisplayName: "Jane Doe", + }); err != nil { + t.Fatalf("saveSaaSUser: %v", err) + } + s := &HubServer{} + label := s.identityLabeler() + buckets := []UsageBucket{ + {Key: "ibmid:5500087VJB"}, + {Key: "clubanderson"}, + } + for i := range buckets { + if l := label(buckets[i].Key); l != buckets[i].Key { + buckets[i].Label = l + } + } + if buckets[0].Key != "ibmid:5500087VJB" { + t.Fatalf("raw bucket key was rewritten to %q", buckets[0].Key) + } + if buckets[0].Label != "Jane Doe" { + t.Errorf(`OIDC bucket Label = %q, want "Jane Doe"`, buckets[0].Label) + } + if buckets[1].Label != "" { + t.Errorf("GitHub bucket Label = %q, want empty (omitempty keeps the wire clean)", buckets[1].Label) + } +} + +// TestFleetAlertsDecoratesAckByName asserts an acknowledged alert served to +// the dashboard carries AckByName when the acking admin is an OIDC identity +// with a stored name, while AckBy keeps the raw key. +func TestFleetAlertsDecoratesAckByName(t *testing.T) { + withTempSaaSDirs(t) + if err := saveSaaSUser(&SaaSUser{ + GitHubUsername: "ibmid:5500087VJB", + Provider: "ibmid", + DisplayName: "Jane Doe", + }); err != nil { + t.Fatalf("saveSaaSUser: %v", err) + } + s := &HubServer{alerts: newAlertState()} + // A stale heartbeat trips the offline alert deterministically. + stale := time.Now().Add(-2 * time.Hour).UTC().Format(time.RFC3339) + entries := []MyHiveEntry{{ + RegistryEntry: RegistryEntry{ID: "h1", Name: "org/repo", Org: "org", LastHeartbeat: stale}, + }} + pre := s.fleetAlerts(entries) + if len(pre.Alerts) == 0 { + t.Fatal("expected at least one alert for a stale hive") + } + if !s.alerts.setAck(pre.Alerts[0].HiveID, pre.Alerts[0].Type, "ibmid:5500087VJB", time.Now()) { + t.Fatal("setAck refused — condition firstSeen not recorded?") + } + + summary := s.fleetAlerts(entries) + found := false + for _, a := range summary.Alerts { + if !a.Acknowledged { + continue + } + found = true + if a.AckBy != "ibmid:5500087VJB" { + t.Errorf("AckBy = %q, want the raw identity key", a.AckBy) + } + if a.AckByName != "Jane Doe" { + t.Errorf(`AckByName = %q, want "Jane Doe"`, a.AckByName) + } + } + if !found { + t.Fatal("no acknowledged alert came back from fleetAlerts") + } +} + +// TestHubDashboardRendersResolvedNamesNotRawKeys pins the JS render sites: +// each surface must prefer the server-resolved name field and fall back to +// the raw key — never the reverse, and never the raw key alone. +func TestHubDashboardRendersResolvedNamesNotRawKeys(t *testing.T) { + for _, snippet := range []string{ + // Usage tables: display label, raw key preserved for the jump action. + "var rLabel = String(r.label || r.key || '');", + // Zero-consumption chip tooltip. + "(h.ownerName || h.owner)", + // Activity Timeline + access log actor lines. + "esc(ev.actorName || ev.actor)", + // Alert ack note. + "esc(a.ackByName || a.ackBy)", + // +N overflow tooltip on the co-member faces. + "members[j].display_label || members[j].username", + } { + if !strings.Contains(dashboardHTML, snippet) { + t.Errorf("hub dashboard missing expected render snippet: %q", snippet) + } + } + // Group-by-owner must group on the resolved label… + if !strings.Contains(dashboardHTML, "return (h && (h.ownerName || h.owner)) || ''; }},") { + t.Error("Group by: Owner must label groups with ownerName when resolved") + } + // …while the usage jump keeps operating on the RAW key. + if !strings.Contains(dashboardHTML, "jumpToUsageBucket(decodeURIComponent(\\'' + esc(encodeURIComponent(String(r.key || '')))") { + t.Error("jumpToUsageBucket must keep receiving the raw r.key — the label is display-only") + } +} diff --git a/src/pkg/hub/saas.go b/src/pkg/hub/saas.go index 91c1722ac..212f2eb6d 100644 --- a/src/pkg/hub/saas.go +++ b/src/pkg/hub/saas.go @@ -2970,6 +2970,12 @@ type MyHiveEntry struct { ProvError string `json:"provError,omitempty"` ProvStatus string `json:"provStatus,omitempty"` AutoUpgrade bool `json:"autoUpgrade"` + // OwnerName is the resolved human display label for an opaque OIDC Owner + // identity ("ibmid:5500…" → "Jane Doe"), stamped at serve time from the + // stored user record. Empty when Owner is already the best label (GitHub + // logins). Purely cosmetic: grouping labels and tooltips show it; every + // key, filter and authorization check stays on the raw Owner. + OwnerName string `json:"ownerName,omitempty"` // TrackedChannel is the release channel this hive's image is pinned to // ("stable", "candidate", "edge"), or "" for a plain-branch hive. Overlaid // at read time from the hub-owned SaaSHive record — deliberately NOT from @@ -3666,6 +3672,16 @@ func (s *HubServer) handleMyHives(w http.ResponseWriter, r *http.Request) { // let the header polygon disagree with the rows it summarises. fleetQuadrant := attachQuadrants(result, isAdmin, journeyNow) + // Cosmetic owner labels: resolve opaque OIDC owner identities to their + // stored display names so "Group by owner" headers and tooltips read like + // people. Memoized — a fleet shares a handful of owners. + ownerLabel := s.identityLabeler() + for i := range result { + if l := ownerLabel(result[i].Owner); l != result[i].Owner { + result[i].OwnerName = l + } + } + // Server-side scoping (filter/sort/pagination) happens LAST, after every // set-wide computation above (drift norm, alerts, outage suppression, // quadrant percentiles) has run over the caller's full visible set — the @@ -11396,7 +11412,9 @@ const dashboardHTML = ` still identifiable without opening the hover panel. */ var hiddenNames = []; for (var j = shown.length; j < members.length; j++) { - hiddenNames.push(String(members[j].username || '')); + /* display_label (when present) is the resolved human name for an + opaque OIDC key — same precedence the visible faces use. */ + hiddenNames.push(String(members[j].display_label || members[j].username || '')); } faces += '+' + overflow + ''; @@ -12884,7 +12902,7 @@ const dashboardHTML = ` {key: HIVE_GROUP_NONE, label: 'No grouping', of: function() { return ''; }}, {key: HIVE_GROUP_CLUSTER, label: 'Cluster', of: function(h) { return (h && (h.clusterName || h.clusterId)) || ''; }}, {key: HIVE_GROUP_ORG, label: 'Org', of: function(h) { return (h && h.org) || ''; }}, - {key: HIVE_GROUP_OWNER, label: 'Owner', of: function(h) { return (h && h.owner) || ''; }}, + {key: HIVE_GROUP_OWNER, label: 'Owner', of: function(h) { /* ownerName resolves opaque OIDC ids ("ibmid:…") to display names; grouping by the label keeps headers human while row data stays raw. */ return (h && (h.ownerName || h.owner)) || ''; }}, {key: HIVE_GROUP_ACMM, label: 'ACMM level', of: function(h) { /* acmmLevel is numeric; render as "Level N" so the header reads as a label rather than a bare digit. 0/absent falls through to @@ -13710,7 +13728,7 @@ const dashboardHTML = ` : ''; } - var ackedBy = (acked && a.ackBy) ? '— ack by ' + esc(a.ackBy) + '' : ''; + var ackedBy = (acked && a.ackBy) ? '— ack by ' + esc(a.ackByName || a.ackBy) + '' : ''; /* The row itself is a