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

CLAUDE.md

@@ -14,11 +14,11 @@ Self-hosted MCP manager/gateway: one streamable-HTTP `/mcp` endpoint federating
 - `config.ts` — flags/env + `mspstack.config.json` (`ConfigError`); parses `MCP_TOKENS_<ROLE>` lists (duplicate labels are a `ConfigError` — labels are /me identities), OIDC (`OIDC_ISSUER`/`ENTRA_TENANT_ID` + required `OIDC_AUDIENCE`), `BAO_*`
 - `db/backup.ts` — online snapshots (`VACUUM INTO`, never a file copy of a live db), retention, optional Azure Blob shipping (lazy SDK, DefaultAzureCredential); scheduler unref'd, off for `:memory:`; `GET/POST /api/backups`; restore is manual (docs/backups.md)
 - `mcp/self-tools.ts` — admin-only `gw_*` toolset (issue #2): status, list servers/tools (values REDACTED), bulk enable by tier/group, tier overrides, grants by role name, preset install, refresh, backup-now. Hidden from non-admins AND re-checked per call with the unknown-tool wording; `gw` namespace reserved in config.ts; `GATEWAY_SELF_TOOLS=off` disables. Target resolution shared with both HTTP APIs via `domain/tool-targets.ts`
-- `db/` — `node:sqlite` schema (roles/upstreams/grants/tool_overrides/tool_settings/users/group_mappings, seeded viewer/editor/admin) + typed `Repo`
+- `db/` — `node:sqlite` schema (roles/upstreams/grants/tool_overrides/tool_settings/users/group_mappings/user_login_roles, seeded viewer/editor/admin) + typed `Repo`; migrations are `PRAGMA user_version` blocks and `migrate()` is exported so a hand-built legacy DB can be tested (`db/migrate-v5.test.ts`)
 - `domain/catalog.ts` — namespacing (`${namespace}_${tool}`, no double-prefix), routing map (no string-splitting), annotation-derived tiers (port of mcp-itglue `tierOf`), `derivedGroupOf(tool)` — the category for the group switches: `_meta.group`/`_meta.toolset` first (how the family servers tag their toolsets), else a bracketed description prefix (`[Identity > …]`, CIPP); an explicit `group_label` always wins
 - `domain/presets.ts` — one-click upstream presets: builtin family configs (itglue/cwpsa/planner/cipp — full specs incl. BYOK headers, per-user mode, userConnect, personalCredentials, the `auth` mint block) + optional `mspstack.presets.json` (file overrides builtin ids); `{{param}}` templating rendered server-side and validated via `parseUpstreamSpec`; recommended grants by role NAME resolved at install (`GET /api/presets`, `POST /api/presets/:id/install` with `dryRun`). Spec's `personalCredentials` metadata drives the /me guided credential forms (`credentialFields` in `/api/me/access`)
 - `domain/policy.ts` — `PolicyService`: toolEnabled ∧ (override(allow) ∨ (tier ≤ maxTier ∧ ¬deny)); maxTier = per-upstream grant ?? role default. Same function gates tools/list AND tools/call. `allowsFor(principal, entry)` = envelope ∧ personal prefs (deny rows in `user_prefs`; "enable" deletes the row — narrowing can never widen). Spec `userDefault: "off"` inverts the personal layer for that upstream: nothing is live until an explicit opt-in row exists (per tool or server-wide `''`), still capped by the envelope — for servers with hundreds of tools. `/api/me/access` derives its `enabled` flags from `allowsFor` so the page can't disagree with the boundary
-- `auth/` — `static-tokens.ts` (timing-safe bearer match), `oidc.ts` (jose JWKS resource-server verifier for inbound *access* tokens), `login.ts` (interactive login: openid-client cookie+PKCE confidential-client flow consuming an *id-token*; signed identity-only session cookie, HMAC + freshness; `safeReturnTo`), `authz-server.ts` (OAuth AS facade: RFC 8414 metadata, RFC 7591 DCR for public clients, single-use hashed 60s codes + PKCE S256, HS256 gateway JWTs keyed by `GATEWAY_JWT_SECRET` (default derived from `SESSION_SECRET`), rotating refresh tokens — 30d sliding, family-revoked on replay, client-bound consume that can't burn a live token — register rate limit; clients managed via `/api/oauth-clients` + Users tab), `prm.ts` (RFC 9728 doc + WWW-Authenticate; lists the gateway itself as AS when login is configured, else the raw IdP), `directory.ts` (app-only Graph search of Entra users/groups via the login app's own creds — powers the admin UI group-mapping typeahead at `/api/directory/search`; null for non-Entra issuers → UI degrades to paste-an-id), `principal.ts` (session binding key). Four inbound auth paths in `createAuthResolver`: static token, gateway-issued JWT (routed by unverified `iss == PUBLIC_URL`, then fully verified), OIDC bearer, and the cookie session — the cookie/JWT carry only identity and the role is re-resolved every request (persisted at callback via `setUserRole`), so a session id never carries privilege. `loginUpsert()` is shared by the bearer + callback paths so they can't drift. `/oauth/authorize` brokers user auth to Entra by piggybacking the interactive login: the pending request rides in the signed transient cookie and `/auth/callback` mints the code.
+- `auth/` — `static-tokens.ts` (timing-safe bearer match), `oidc.ts` (jose JWKS resource-server verifier for inbound *access* tokens), `login.ts` (interactive login: openid-client cookie+PKCE confidential-client flow consuming an *id-token*; signed identity-only session cookie, HMAC + freshness; `safeReturnTo`), `authz-server.ts` (OAuth AS facade: RFC 8414 metadata, RFC 7591 DCR for public clients, single-use hashed 60s codes + PKCE S256, HS256 gateway JWTs keyed by `GATEWAY_JWT_SECRET` (default derived from `SESSION_SECRET`), rotating refresh tokens — 30d sliding, family-revoked on replay, client-bound consume that can't burn a live token — register rate limit; clients managed via `/api/oauth-clients` + Users tab), `prm.ts` (RFC 9728 doc + WWW-Authenticate; lists the gateway itself as AS when login is configured, else the raw IdP), `directory.ts` (app-only Graph search of Entra users/groups via the login app's own creds — powers the admin UI group-mapping typeahead at `/api/directory/search`; null for non-Entra issuers → UI degrades to paste-an-id), `principal.ts` (session binding key; a Principal carries EVERY role it holds and the envelope is their union — `resolveOidcRoles` returns all mapped groups' roles, an explicit `users.role_id` override replaces them, and `user_login_roles` remembers the group-derived set for the cookie/JWT paths that see no group claims). Four inbound auth paths in `createAuthResolver`: static token, gateway-issued JWT (routed by unverified `iss == PUBLIC_URL`, then fully verified), OIDC bearer, and the cookie session — the cookie/JWT carry only identity and the roles are re-resolved every request (group-derived set persisted at callback via `setLoginRoles`), so a session id never carries privilege. `loginUpsert()` is shared by the bearer + callback paths so they can't drift. `/oauth/authorize` brokers user auth to Entra by piggybacking the interactive login: the pending request rides in the signed transient cookie and `/auth/callback` mints the code.
 - `secrets/` — `SecretStore` interface (scheme-tagged: `bao` | `kv`), `openbao.ts` (KV v2, AppRole or token, 5-min cache), `keyvault.ts` (Azure Key Vault, `DefaultAzureCredential`, lazy SDK import, same 5-min cache; `put(path, field)` writes `path-field`), `memory.ts` (tests). Refs: `bao:path#field` / `kv:secret-name`; env refs: `${VAR}` — all resolved only at upstream connect time. One store at a time (`BAO_ADDR` xor `KEY_VAULT_URI`)
 - `upstream/connection.ts` — one pooled SDK `Client` per upstream; header/env injection; backoff reconnect (1s→60s) + `onRecovered`; retry-once on dropped transport AND on server-side session expiry (upstream 404 "unknown session" → transparent re-initialize + retry, per MCP spec). Optional spec `auth` block (`oauth2-client-credentials`): the gateway mints the upstream's bearer itself (secret via `${VAR}`/`bao:`/`kv:` ref), caches it, and rebuilds the connection when it nears expiry (60s skew) — for third-party servers that want a finished token, e.g. CIPP behind Easy Auth. Neither secret nor token is ever logged
 - `upstream/manager.ts` also pools **per-principal links** for `sessionMode:"per-user"` upstreams: spec clone with the caller's credential REFS layered over headers/env (still resolved via the secret store at connect — anti-passthrough intact); catalog discovery stays on the shared link; personal pool flushed on upstream upsert/remove. `requirePersonalCredentials` refuses the shared fallback

packages/gateway/public/me.html

@@ -164,7 +164,9 @@ <h2>Sign in</h2>
   try { data = await api("/access"); }
   catch (e) { el.innerHTML = `<section class="panel"><p class="muted">${esc(e.message)}</p></section>`; return; }
 
-  $("#whoami").textContent = data.principal ? `${data.principal.label} · ${data.principal.role}` : "";
+  $("#whoami").textContent = data.principal
+    ? `${data.principal.label} · ${(data.principal.roles ?? [data.principal.role]).join(" + ")}`
+    : "";
 
   if (!data.servers.length) {
     el.innerHTML = `<section class="panel"><h2>My access</h2>
@@ -176,8 +178,15 @@ <h2>Sign in</h2>
 
   el.innerHTML = `<section class="panel"><h2>My access</h2>
     <p class="muted">Turn servers, whole tiers, or individual tools off for your own sessions. You can only narrow what your role already grants — turning something on just removes your personal off-switch.</p>
-    <p class="muted" style="border-left:2px solid var(--accent); padding-left:10px">Changes take effect immediately: a tool you switched off is refused even if your client still lists it. The <em>list</em> your client shows is fetched when it connects — if a change doesn't appear there, use <b>Apply now</b> and your clients will reconnect and pick up the current set.
-      <button class="btn" id="reload-sessions" style="margin-left:8px">Apply now</button></p>
+    <section style="border-left:2px solid var(--accent); padding:2px 0 2px 12px; margin-bottom:14px">
+      <p class="muted" style="margin:0 0 6px"><b style="color:var(--text)">A switch here bites immediately.</b> A tool you turned off is refused on the next call, even while your client still shows it in its list — enforcement and display are two different things.</p>
+      <p class="muted" style="margin:0 0 6px"><b style="color:var(--text)">The list is fetched when a client connects.</b> Change something, then press <b>Apply now</b>: your live sessions are dropped, your clients reconnect and read the current set. Order matters — change first, apply second.</p>
+      <p class="muted" style="margin:0 0 8px">If a tool still doesn't appear, that client is caching the list above the protocol. Then: start a new chat, or turn the connector off and on in your client's settings. Nothing is wrong with your access — check this page, it is the source of truth.</p>
+      <div class="row" style="margin:0; align-items:center">
+        <button class="btn" id="reload-sessions">Apply now</button>
+        <span class="muted" id="session-state"></span>
+      </div>
+    </section>
     ${data.servers.map(s => {
       const counts = tierCounts(s.tools);
       const optIn = s.userDefault === "off";
@@ -285,8 +294,9 @@ <h3>
         ? `Reconnecting ${r.closed} session${r.closed === 1 ? "" : "s"} — your client will re-read its tools`
         : "No live sessions to reconnect — your client will read the current set when it connects");
     } catch (e) { toast(e.message, true); }
-    finally { b.disabled = false; }
+    finally { b.disabled = false; renderSessionState(); }
   };
+  renderSessionState();
 
   el.onchange = async (ev) => {
     const i = ev.target;
@@ -300,6 +310,26 @@ <h3>
   };
 }
 
+/**
+ * Connected clients, and whether any of them holds the notification stream.
+ * Without a stream the gateway cannot push tools/list_changed at all, which is
+ * precisely when Apply now is not optional — so say it here instead of leaving
+ * people to guess why a change didn't show up.
+ */
+async function renderSessionState() {
+  const el = $("#session-state");
+  if (!el) return;
+  try {
+    const r = await api("/sessions");
+    const n = r.sessions.length;
+    el.textContent = n === 0
+      ? "no clients connected right now — the next one to connect reads the current set"
+      : `${n} client${n === 1 ? "" : "s"} connected · ` + (r.notificationStream
+          ? "live notifications available"
+          : "no notification channel — press Apply now after a change");
+  } catch { el.textContent = ""; }
+}
+
 // ── My credentials ──
 async function renderCredentials() {
   const el = $("#credentials");

packages/gateway/src/auth/login.test.ts

@@ -202,8 +202,11 @@ describe("interactive login callback → cookie session", () => {
     const session = cookieFrom(cb, "mspstack_session");
     expect(session).toBeTruthy();
 
-    // The role was persisted at callback time.
-    expect(repo.userBySubject(ISS, "oid-editor")?.roleId).toBe(editorRoleId);
+    // The group-derived roles were persisted at callback time — as login roles,
+    // NOT as an explicit override, so an admin's own choice stays theirs (#28).
+    const user = repo.userBySubject(ISS, "oid-editor")!;
+    expect(user.roleId).toBeNull();
+    expect(repo.loginRoles(user.id).map((r) => r.id)).toEqual([editorRoleId]);
 
     // The session cookie now authenticates /api/me (no bearer token at all).
     const access = await fetch(`${base}/api/me/access`, { headers: { Cookie: session! } });

packages/gateway/src/auth/principal.ts

@@ -6,18 +6,53 @@
 
 import { createHash } from "node:crypto";
 
+export interface PrincipalRole {
+  id: number;
+  name: string;
+  isAdmin: boolean;
+}
+
 export interface Principal {
   kind: "static" | "oidc" | "dev";
   /** Stable identity: static → token label; oidc → `${iss}|${sub}`; dev → "dev". */
   subject: string;
   /** Human-readable, for logs (never a secret). */
   label: string;
+  /**
+   * Every role held. The envelope is their UNION (issue #28): a user in two
+   * mapped groups keeps both surfaces, because "we gave you billing access"
+   * must never take ticket access away. Subtraction lives outside roles — the
+   * global kill switch and the user's own /me prefs.
+   */
+  roles: PrincipalRole[];
+  /** Primary role — most privileged, first in `roles` — for display and messages. */
   roleId: number;
   roleName: string;
   isAdmin: boolean;
 }
 
-export const principalKey = (p: Principal): string => `${p.kind}:${p.subject}:${p.roleId}`;
+/** Build a Principal's role fields from a "most privileged first" role list. */
+export function withRoles<T extends { kind: Principal["kind"]; subject: string; label: string }>(
+  base: T,
+  roles: PrincipalRole[]
+): T & Pick<Principal, "roles" | "roleId" | "roleName" | "isAdmin"> {
+  const primary = roles[0]!;
+  return {
+    ...base,
+    roles,
+    roleId: primary.id,
+    roleName: roles.length > 1 ? roles.map((r) => r.name).join("+") : primary.name,
+    isAdmin: roles.some((r) => r.isAdmin),
+  };
+}
+
+/** Role ids, sorted — the identity half of a session binding key. */
+export const roleIdsOf = (p: Principal): number[] => p.roles.map((r) => r.id).sort((a, b) => a - b);
+
+// A role change mid-session yields a different key, so the existing
+// principal-mismatch 403 makes the client reconnect — unchanged behaviour.
+export const principalKey = (p: Principal): string =>
+  `${p.kind}:${p.subject}:${roleIdsOf(p).join(",")}`;
 
 /**
  * Identity key for personal state (prefs, registered credentials) —

packages/gateway/src/db/index.ts

@@ -16,7 +16,8 @@ export function openDatabase(path: string): DatabaseSync {
   return db;
 }
 
-function migrate(db: DatabaseSync): void {
+/** Exported so migrations can be tested against a hand-built legacy database. */
+export function migrate(db: DatabaseSync): void {
   const version = (db.prepare("PRAGMA user_version").get() as { user_version: number })
     .user_version;
 
@@ -173,4 +174,37 @@ function migrate(db: DatabaseSync): void {
 
     db.exec("PRAGMA user_version = 4");
   }
+
+  if (version < 5) {
+    // Additive roles (issue #28). Until now the login path wrote the
+    // group-derived role into users.role_id, which made "the admin chose this
+    // role" and "this came from a group" the same row — and forced a single
+    // winner. Split them:
+    //
+    //   users.role_id + role_source='admin' → an explicit override, REPLACES groups
+    //   user_login_roles                    → every role the last login's groups mapped to
+    //
+    // Existing non-null role_id rows are marked 'admin' deliberately: on a live
+    // deployment they were either set from the Users tab or written back
+    // unchanged by the login path, so treating them as explicit overrides keeps
+    // everyone's access byte-for-byte. The union only starts to matter for users
+    // with no override, which is the state a fresh group mapping produces.
+    db.exec(`
+      CREATE TABLE IF NOT EXISTS user_login_roles (
+        user_id INTEGER NOT NULL REFERENCES users(id) ON DELETE CASCADE,
+        role_id INTEGER NOT NULL REFERENCES roles(id) ON DELETE CASCADE,
+        PRIMARY KEY (user_id, role_id)
+      );
+    `);
+    const columns = db.prepare("PRAGMA table_info(users)").all() as Array<{ name: string }>;
+    if (!columns.some((c) => c.name === "role_source")) {
+      db.exec(`
+        ALTER TABLE users ADD COLUMN role_source TEXT
+          CHECK (role_source IS NULL OR role_source IN ('admin','login'));
+        UPDATE users SET role_source = 'admin' WHERE role_id IS NOT NULL;
+      `);
+    }
+
+    db.exec("PRAGMA user_version = 5");
+  }
 }