Admins never inhabit a seat + count chips bypass tn() ("1 members", "1 rules") #426
Labels
No labels
No milestone
No project
No assignees
1 participant
Notifications
Due date
No due date set.
Dependencies
No dependencies set.
Reference: Cordy/Cairn#426
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "%!s()"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Two findings from the #346 live verification pass (2026-09-05), parked deliberately — cut as decided, not yet to build.
1. Seat accounting: admins are invisible to seat usage
Observed: bao Overview shows Seats in use 0 of 60 while two admins are signed in / active.
Root cause (
internal/license/provider.go):For
u.Admin == true(including everyadminGroupsmember),Admit()is never called — andAdmit()is the only writer ofSeenUsers, which feeds both the Seats tile (SeatsUsed) and theseatsUsedfigure reported at licence check-in. Admins therefore neither consume nor even register a seat.Two fused concerns to separate:
seatsUsed.Fix shape: a record-always / cap-only-for-non-admins path in the Manager (e.g.
Admit(username, isAdmin)or a separateRecord), wired inseatProvider.Authenticate; tests for admin-records-but-never-blocked, at-cap admin sign-in, and existing-user idempotency. NoteAdmit's internalm.adminsbypass only covers staticauth.admins, not group admins — the wrapper'su.Adminis the correct authority; the internal map check becomes redundant once the split exists.Decision to bless before building: do admins consume a paid seat, or only get counted? (Recording is unambiguous; whether they occupy one of the licensed N when at cap is a business call — though with always-pass admission, at-cap admins would overshoot the cap in the count rather than be denied, which is honest reporting.)
2. Count chips bypass tn(): singular/plural bugs
Two confirmed instances, found minutes apart:
Both interpolate a raw count into a pluralised string instead of going through
tn(). Fix ×4 locales for both, plus a sweep of index.html for siblings — two found casually implies more (any{n} <plural-noun>chip/row built withtf()instead oftn()).Verification
Item 3 (from the #346 broken-secret live test): the Admin client row shows config presence, not live health.
With a deliberately wrong
auth.groups.keycloakClientSecret, the Users & access page correctly fail-closes the Administrators list (gone entirely, no static-name degradation) and drops the member-count chip — but the Admin client row keeps saying "Configured · group membership can be resolved" with a green On chip while every resolve is failing. The failure is signalled only by omission.Fix shape: the
/admin/usersendpoint already knows the resolve failed (that is what suppresses the list) — surface it. When configured-but-failing: row copy along the lines of "Configured · the directory refused the credentials — group membership cannot be resolved" with an amber chip, distinct from both "Not configured" and healthy "On". i18n ×4. A dogfound-state test: endpoint returns configured=true + resolveError=true → UI renders the amber row and no Administrators section.Decision blessed (Nikola, 2026-09-05): admins consume a paid seat, and remain always able to sign in. At cap, an admin sign-in records honestly past the cap rather than being denied — enforcement never locks out operators, reporting never lies.
Research note: industry practice on admin seats genuinely varies by product/market; the consistent best practices are (a) a clearly documented seat definition, (b) honest usage reporting, (c) enforcement that cannot lock out administrators. This decision satisfies all three. A line stating the definition ("every distinct signed-in account occupies a seat, administrators included; administrators are never denied sign-in by the cap") goes into licensing-faq.md.
Implementation starting, TDD with a witnessed red phase: failing tests for
Admit(username, isAdmin)(admin recorded; admin at cap admitted and recorded over cap; idempotency; non-admin at cap still refused) pushed and observed failing on the runner before the implementation commit. Item 2 (tn() sweep) rides the same release. Item 3 (admin-client health row) is UI: mockup first, built only after approval.Shipped and live-verified — v0.6.128 (PR #427; both dogfoods rolled 02:09 UTC).
All three items built strictly TDD with the red phase witnessed on the runner before each implementation (run logs show e.g.
SeatsUsed = 0, want 1andSeatsUsed = 50, want 51failing, then green in the same job):Admit(username, isAdmin): record always, cap only for non-admins; admission untouched (break-glass preserved; honest overshoot at cap). Seat definition documented in licensing-faq.md. Live proof minutes after rollout: bao Overview went from 0 of 60 to "Seats in use 1 of 60 · 59 unassigned" on the first admin sign-in.tpuSummary,tlgSummary,audShownOf,licSeatsFoot) — revisit only if one ever shows up ugly in practice.adminClientFailingendpoint field; amber "Failing" chip with "check the admin client secret and that the IdP is reachable"; explanatory note where the Administrators section would be. Endpoint behaviour covered by three tests (failing / healthy / unconfigured), fail-closed list retained.Closing.