feat: teams, users and impersonation - #63
Merged
Merged
Conversation
Contributor
Test results672 tests 672 ✅ 48s ⏱️ Results for commit 1cad447. ♻️ This comment has been updated with latest results. |
Two problems that look like one, and conflating them would put a directory-wide credential in a browser. Reading who someone is is a token question. The 'No tenant to show' bug is now explained rather than suspected: PrincipalResolver reads an 'organization' claim, and the app registration has groupMembershipClaims: null and optionalClaims: null. Entra was never going to emit it. Mapping an Entra group to a Tenant fixes it with no Graph, no secret and no second registration. Changing the directory needs Graph application permissions, and those must not go on the Apus app registration: it is a SPA, a public client, which cannot hold a secret or use client credentials at all -- permissions there would be unusable and would invite the assumption that the browser holds them. So a second confidential registration used only by the api module, server-side. Workload identity federation would avoid a secret entirely, but it is unavailable here and this was checked: the cluster's OIDC issuer is https://api.k8s.onelite.feather:6443, an internal name Entra cannot reach. The permissions are directory-wide and Entra offers no narrower variant, so the narrowing lives in Apus's code -- refuse any group no Tenant claims, refuse any user outside one, refuse password resets against privileged directory roles -- and the tests are written from the attacker's side. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019Bff5mpWkUnZA77jys8DiR
Subsystems C and D. The Graph permissions behind this are directory-wide -- Entra offers no 'these groups only' variant of Group.ReadWrite.All -- so the whole design is about where the narrowing lives. DirectoryGuard is that narrowing, and it is closed by default: an instance that has not been told which groups are managed refuses everything, which is also its state at startup and if the tenant index ever fails to load. It refuses any group no Tenant claims, any user outside one, any password reset aimed at a privileged directory account, and any reset of the caller's own password. Its tests are written from the attacker's side. Tenant.spec.identity.groupId ties a tenant to an Entra group, which fixes the 'No tenant to show' bug the tenant app has had since SSO was set up: PrincipalResolver read an 'organization' claim, and the app registration has groupMembershipClaims: null and optionalClaims: null. The claim was not mis-mapped, it never existed. Groups are the fallback; an explicit organisation claim still wins where a broker emits one. Impersonation only ever narrows. The effective principal never holds platform-admin and never holds a role its caller does not, so there is no combination of headers that lets anyone do something they could not already do as themselves -- only in a different tenant. Applied in one filter rather than per controller, so controllers written later cannot forget it. The directory being down is not a tenant page failing: reads report the panel unavailable with a reason, and a count is null rather than 0, because a zero meaning 'we could not ask' is a lie an administrator would act on. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019Bff5mpWkUnZA77jys8DiR
The console side of subsystems C and D, and the last three of the five things the goal asked for. Tenant list gains a teams/people column, fetched per row after the table is already on screen. Deliberately not part of the tenant list itself: those counts come from the identity provider, and blocking the table on somebody else's service would make every tenant unreadable whenever it has a bad minute. A tenant whose counts fail keeps a dash -- never '0 / 0', which would say it has nobody in it. Tenant detail gains teams and people with create, invite and reset. Two rules run through the component and both are about not lying: unavailable is not empty, and an unknown team size is not zero. The temporary password is shown once, in a box that says it will not be shown again. Impersonation is two request headers and a banner. The banner lives in the layout rather than on the page that starts the session, because that is the point -- once it is running every page answers as somebody else, and a banner you stop seeing when you navigate is a banner you stop seeing exactly when you have forgotten. Sending the headers grants nothing: the API strips the platform role and refuses a tenant the caller may not act in. The chart wires the directory credential from a Secret via secretKeyRef, off by default, and its comment says plainly that the registration must not be the one the browser uses. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019Bff5mpWkUnZA77jys8DiR
The steps that cannot be automated from inside Apus, because granting an application permission is a directory administrator's decision and Apus deliberately holds no permission that would let it grant itself another. Step 1 stands on its own and is worth doing regardless: setting groupMembershipClaims is what fixes the 'No tenant to show' bug, and it needs none of the Graph permissions the rest of the runbook asks for. Step 2 states plainly what the permissions allow -- rename any group in the organisation, reset any account's password -- and what DirectoryGuard promises in return, so the reader can decline the trade with the facts in front of them. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019Bff5mpWkUnZA77jys8DiR
It reads the Authentication the security filter puts on the request. Left to the default ordering it could run first, find none, and refuse every impersonated request -- a failure that reads like a permission problem and sends somebody looking at roles instead of at filter order. The order is a literal because an annotation value must be a constant expression and ServerFilterPhase.SECURITY.after() is a method call. A test asserts the two agree, so a Micronaut release that renumbers the phases fails the build rather than silently reordering this filter. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019Bff5mpWkUnZA77jys8DiR
'Zuweisungen sehen' -- the assignment itself, not the two lists side by side. Fetched one team at a time when somebody opens it, rather than up front: that would be a request per team on every page load, for a list most people only scan, and the identity provider throttles bursts like that. The endpoint checks the team belongs to this tenant instead of trusting the id in the path. Without that the group guard would still pass -- the tenant's own group is managed -- while the id pointed anywhere in the directory, which would turn this into a way to read every group's membership in the organisation. There is a test aimed at exactly that. The UI says 'Loading' rather than 'Nobody is in this team yet' while the answer is still out; an empty list shown mid-request reads as a fact about the team. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019Bff5mpWkUnZA77jys8DiR
The endpoints moved under /api/tenants/{name}/directory so one prefix carries
the whole capability, and the assignments endpoint was added. The spec travels
with the code; an endpoint table that names paths nobody serves is worse than
none.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019Bff5mpWkUnZA77jys8DiR
Graph reports members@odata.count for the group being listed, not for each nested group in the result, so a team's size is normally absent from the list response -- meaning the honest label would have appeared on every single row and told nobody anything. The label now says nothing about size until the team is opened, at which point the loaded membership is an exact count. What it still never does is print a zero it did not measure. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019Bff5mpWkUnZA77jys8DiR
Microsoft split password resets out of User.ReadWrite.All into User-PasswordProfile.ReadWrite.All. The runbook asked for the broad one, which would have granted the ability to rewrite every attribute of every account in the directory and still returned 403 on the one operation it was for. All four permission ids are now read back from the Graph service principal in this tenant rather than written from memory, and the runbook says how to check them. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019Bff5mpWkUnZA77jys8DiR
TheMeinerLP
force-pushed
the
feat/teams-and-users
branch
from
August 17, 2026 07:17
97126e2 to
1cad447
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Teams and people per tenant, who is in which team, invitations, password resets, and viewing the
platform as a tenant sees it. Subsystems C and D — the last three of the five things the goal
asked for.
Design:
docs/superpowers/specs/2026-08-16-teams-and-users-design.mdRunbook:
docs/runbooks/directory-and-impersonation-setup.mdMerge #62 first. This branch sits on top of it, so until #62 lands the diff below also shows
#62's commits; GitHub cleans that up by itself once it merges.
The base is
mainrather thanfeat/per-tenant-app-instanceon purpose:build-pr.ymltriggerson
pull_request: branches: [main], so a stacked base means no CI at all — a tidier diff is notworth an unverified branch.
Two problems that look like one
Conflating them would have put a directory-wide credential in a browser.
Reading who somebody is is a token question. No Graph, no secret, no second registration.
Changing the directory needs Graph application permissions, which only a confidential client
can use.
The first one fixes a bug that has been open since SSO was set up
The tenant application has shown "No tenant (platform-level account)" for every user. The cause
is now confirmed rather than suspected:
PrincipalResolverreads a claim namedorganization,and the
Apusapp registration hasgroupMembershipClaims: nullandoptionalClaims: null. Theclaim was not mis-mapped — Entra was never going to emit it.
Tenant.spec.identity.groupIdties a tenant to an Entra group,TenantGroupIndexmaps a token'sgroupsclaim back to a tenant, and an explicitorganizationclaim still wins where a brokeremits one. Setting
groupMembershipClaimsis step 1 of the runbook and needs none of thepermissions below — it is worth doing even if the rest is declined.
Where the narrowing lives
Group.ReadWrite.All,User.ReadWrite.AllandUser.Invite.Allare directory-wide. Entra has no"these groups only" variant of any of them. The credential this API holds could rename any group
in the organisation and reset any account's password.
So
DirectoryGuardis the only thing between a controller bug and the whole directory, and it isclosed by default — an instance that has not been told which groups are managed refuses
everything, which is also its state at startup and if the tenant index ever fails to load. It is
pure: no network call, because a guard that has to reach out to decide fails open exactly when
the network does.
It refuses a group no
Tenantclaims, a user outside one, a password reset aimed at a privilegeddirectory account, and a reset of the caller's own password. Its 13 tests are written from the
attacker's side —
refusesResettingThePasswordOfAGlobalAdministrator, notresetWorks.Impersonation only ever narrows
The effective principal never holds
platform-adminand never holds a role its caller does not.There is no combination of headers that lets anyone do something they could not already do as
themselves; the only thing that changes is which tenant they do it in. That is what makes the
feature's blast radius equal to its caller's, and why the policy strips the platform role rather
than checking for its absence.
Applied in one server filter rather than per controller, so a controller written later cannot
forget it — every one of them already resolves through
PrincipalResolver, and the filterreplaces the request's authentication. A refused impersonation fails the request rather than
quietly serving the real principal: being answered as yourself while believing you are a tenant
is worse than an error.
The banner lives in the console layout, not on the page that starts the session. Once it is
running every page answers as somebody else, and a banner you stop seeing when you navigate is a
banner you stop seeing exactly when you have forgotten.
Not lying when the directory is down
Graph is somebody else's service and it will be throttling at some point. A tenant whose storage
and renders are fine must stay readable.
something the server already said
null, never0— a zero meaning "we could not ask" is a fact an administratorwould act on
still out
Assignments, and one trap in them
Who is in which team is fetched per team, when somebody opens it — not up front, which would be a
request per team on every page load for a list most people only scan.
The endpoint checks the team belongs to this tenant rather than trusting the id in the path.
Without that the group guard would still pass, because the tenant's own group is managed, while
the id pointed at any group in the organisation — turning a members endpoint into a way to read
every group's membership. There is a test aimed at exactly that.
Verification
:api:test:operator:testspotlessCheckpnpm lint/typecheckpnpm testsecretKeyRef, never inlinedWhat still needs a human
Everything in the runbook, and none of it is automatable from inside Apus — granting an
application permission is a directory administrator's decision, and Apus deliberately holds no
permission that would let it grant itself another. The runbook states what the permissions allow
in plain words before asking for them, so the trade can be declined with the facts in view.
🤖 Generated with Claude Code
https://claude.ai/code/session_019Bff5mpWkUnZA77jys8DiR