Skip to content

auth: additive roles — a principal keeps every mapped group's access - #40

Merged
selic merged 1 commit into
mainfrom
feat/additive-roles
Aug 14, 2026
Merged

auth: additive roles — a principal keeps every mapped group's access#40
selic merged 1 commit into
mainfrom
feat/additive-roles

Conversation

@selic

@selic selic commented Aug 14, 2026

Copy link
Copy Markdown
Member

Closes #28.

The problem

resolveOidcRole returned one role: an explicit users.role_id, else the
highest group mapping. That was harmless while roles nested by tier
(viewer ⊂ editor ⊂ admin) — the winner contained the loser. It stops being
harmless the moment roles are incomparable, which is exactly what named tool sets
(#27) introduce: a person in Techs and Billing gets the higher-tier role and
silently loses the other's surface.

The model

A Principal carries every role it holds, and the envelope is their union.
PolicyService.allowsAny evaluates each role on its own grant, default tier and
overrides, so adding a group can only ever widen access.

A per-role deny override closes that role's path, not the others' —
subtracting for everyone remains the global kill switch's job. Both halves are
pinned by tests.

Schema v5

Splitting "the admin chose this" from "this came from a group" needed storage:

  • users.role_source marks an explicit override, which still replaces
    group-derived roles (that is what an override is for);
  • user_login_roles remembers what the last login's group claims mapped to,
    because the cookie and gateway-JWT paths see no group claims and previously
    leaned on the login path writing role_id.

Upgrade safety: existing non-null role_id rows migrate to
role_source='admin' deliberately — on a live deployment they were either set
from the Users tab or written back unchanged by the login path, so nobody's
access moves on upgrade. db/migrate-v5.test.ts builds a v4-shaped database by
hand and asserts exactly that, plus idempotency on a re-run.

Also in this batch (today's diagnosis session)

  • GET /api/me/sessions — my connected clients and whether any holds the
    notification stream, rendered next to Apply now on /me. With no stream
    the gateway cannot push list_changed at all, which is precisely when the
    button is the only cure; better shown than explained.
  • The /me hint now spells out the whole model: a switch bites immediately, the
    list is fetched at connect time, change first then Apply — and if a client
    still disagrees it is caching above the protocol (new chat, or reconnect the
    connector).
  • [mcp] logging of tools/list size and tools/call decisions — names and
    verdicts only, never arguments. "Did the request even reach the gateway?"
    cost an afternoon today; now it is one grep.
  • gw_status reported the MODE in its version field, so it claimed to be
    running version "integrated". Both fields now, with a test.
  • /api/users reports loginRoles and effectiveRoles; /me shows every role.

Tests

domain/additive-roles.test.ts — two deliberately incomparable roles (techs:
tickets, billing: invoices): the union keeps both surfaces, takes the most
permissive ceiling per upstream, a none grant in one role doesn't drag the
union down, a per-role deny is escapable through the other role while the kill
switch is not, isAdmin is true if any role is, and the session binding key
covers the whole role set order-independently.

db/migrate-v5.test.ts — the upgrade path above.

db/repo.test.tsresolveOidcRoles union, unmapped groups ignored, no-groups
fallback to stored login roles, replace-all semantics, override replaces, and
clearing the override falls back to the remembered groups.

npm run build clean, npm test 280 passed, suite run twice.

🤖 Generated with Claude Code

Closes #28.

resolveOidcRole returned ONE role: an explicit users.role_id, else the highest
group mapping. Harmless while roles nested by tier (viewer subset of editor
subset of admin) because the winner contained the loser. It stops being harmless
the moment roles are incomparable, which is exactly what named tool sets (#27)
introduce: someone in Techs and Billing would get the higher-tier role and
silently lose the other's surface.

A Principal now carries every role it holds and the envelope is their UNION:
PolicyService.allowsAny evaluates each role on its own grant, default tier and
overrides, so being added to a group can only widen access. A per-role deny
closes that role's path, not the others' — subtracting for everyone stays the
global kill switch's job, which the tests pin.

Splitting "the admin chose this" from "this came from a group" needed schema v5:
users.role_source marks an explicit override (which still REPLACES groups, that
being its purpose) and user_login_roles remembers what the last login's group
claims mapped to, since the cookie and gateway-JWT paths see no group claims and
previously leaned on the login path writing role_id. Existing non-null role_id
rows migrate to role_source='admin' deliberately: on a live deployment they were
either set from the Users tab or written back unchanged by the login path, so
nobody's access moves on upgrade — db/migrate-v5.test.ts builds a v4 database by
hand and asserts exactly that, plus idempotency.

Also in this batch, all from today's diagnosis session:

- GET /api/me/sessions — my connected clients and whether ANY holds the
  notification stream, rendered next to Apply now on /me. Without a stream the
  gateway cannot push list_changed at all, which is precisely when the button is
  the only cure; better shown than explained.
- The /me hint spells out the whole model: a switch bites immediately, the list
  is fetched at connect time, change first then Apply, and if a client still
  disagrees it caches above the protocol — new chat or reconnect the connector.
- [mcp] logging of tools/list size and tools/call decisions (names and verdicts
  only, never arguments). "Did the request even reach the gateway?" cost an
  afternoon today; now it is one grep.
- gw_status reported the MODE in its `version` field, so it claimed to be running
  version "integrated". Both fields now, with a test.
- /api/users reports loginRoles and effectiveRoles; /me shows every role.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@selic
selic merged commit a2c1493 into main Aug 14, 2026
1 check passed
@selic
selic deleted the feat/additive-roles branch August 14, 2026 22:10
selic pushed a commit that referenced this pull request Aug 14, 2026
Minor (#40): additive roles (#28) — a principal keeps every mapped group's
access instead of only the highest-tier role, with schema v5 separating an
admin's explicit override from group-derived roles so no existing access moves
on upgrade. Plus GET /api/me/sessions behind the Apply now button, [mcp] call
logging (names and verdicts only), and gw_status reporting version and mode
separately.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Additive roles: honour every mapped Entra group, not just the highest

1 participant