diff --git a/CLAUDE.md b/CLAUDE.md index 401358f..cb90d14 100644 --- a/CLAUDE.md +++ b/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_` 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 diff --git a/packages/gateway/public/me.html b/packages/gateway/public/me.html index aff3b12..5dc374f 100644 --- a/packages/gateway/public/me.html +++ b/packages/gateway/public/me.html @@ -164,7 +164,9 @@

Sign in

try { data = await api("/access"); } catch (e) { el.innerHTML = `

${esc(e.message)}

`; 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 = `

My access

@@ -176,8 +178,15 @@

Sign in

el.innerHTML = `

My access

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.

-

Changes take effect immediately: a tool you switched off is refused even if your client still lists it. The list your client shows is fetched when it connects — if a change doesn't appear there, use Apply now and your clients will reconnect and pick up the current set. -

+
+

A switch here bites immediately. 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.

+

The list is fetched when a client connects. Change something, then press Apply now: your live sessions are dropped, your clients reconnect and read the current set. Order matters — change first, apply second.

+

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.

+
+ + +
+
${data.servers.map(s => { const counts = tierCounts(s.tools); const optIn = s.userDefault === "off"; @@ -285,8 +294,9 @@

? `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 @@

}; } +/** + * 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"); diff --git a/packages/gateway/src/auth/login.test.ts b/packages/gateway/src/auth/login.test.ts index e55079f..38539a0 100644 --- a/packages/gateway/src/auth/login.test.ts +++ b/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! } }); diff --git a/packages/gateway/src/auth/principal.ts b/packages/gateway/src/auth/principal.ts index 531b2ad..d3273a7 100644 --- a/packages/gateway/src/auth/principal.ts +++ b/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( + base: T, + roles: PrincipalRole[] +): T & Pick { + 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) — diff --git a/packages/gateway/src/db/index.ts b/packages/gateway/src/db/index.ts index 8d8e327..7de51de 100644 --- a/packages/gateway/src/db/index.ts +++ b/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"); + } } diff --git a/packages/gateway/src/db/migrate-v5.test.ts b/packages/gateway/src/db/migrate-v5.test.ts new file mode 100644 index 0000000..a7b49fd --- /dev/null +++ b/packages/gateway/src/db/migrate-v5.test.ts @@ -0,0 +1,99 @@ +/** + * Migration v4 → v5 (additive roles). The interesting case is an EXISTING + * deployment: every user's role_id was written by the login path, and the new + * code reads role_id as "an admin chose this — ignore groups". Get that wrong + * and a live gateway either widens access silently or drops people's roles, so + * build a v4-shaped database by hand and migrate it. + */ + +import { describe, expect, it } from "vitest"; +import { DatabaseSync } from "node:sqlite"; +import { migrate } from "./index.js"; +import { Repo } from "./repo.js"; + +/** The v1 shape of the tables this migration touches, stamped as v4. */ +function legacyDatabase(): DatabaseSync { + const db = new DatabaseSync(":memory:"); + db.exec(` + CREATE TABLE roles ( + id INTEGER PRIMARY KEY, + name TEXT NOT NULL UNIQUE, + default_max_tier TEXT NOT NULL DEFAULT 'none', + is_admin INTEGER NOT NULL DEFAULT 0, + protected INTEGER NOT NULL DEFAULT 0 + ); + CREATE TABLE users ( + id INTEGER PRIMARY KEY, + iss TEXT NOT NULL, + sub TEXT NOT NULL, + email TEXT, + display_name TEXT, + role_id INTEGER REFERENCES roles(id) ON DELETE SET NULL, + last_login_at TEXT, + UNIQUE (iss, sub) + ); + CREATE TABLE group_mappings ( + id INTEGER PRIMARY KEY, + iss TEXT NOT NULL, + claim_value TEXT NOT NULL, + role_id INTEGER NOT NULL REFERENCES roles(id) ON DELETE CASCADE, + UNIQUE (iss, claim_value) + ); + INSERT INTO roles (id, name, default_max_tier, is_admin, protected) VALUES + (1, 'viewer', 'read', 0, 1), (2, 'editor', 'write', 0, 1), + (3, 'admin', 'destructive', 1, 1), (4, 'managers-ro', 'read', 0, 0); + -- one user with a role (the state prod is in) and one without + INSERT INTO users (id, iss, sub, email, role_id) VALUES + (1, 'https://idp', 'has-role', 'daniel@test', 4), + (2, 'https://idp', 'no-role', 'newcomer@test', NULL); + INSERT INTO group_mappings (iss, claim_value, role_id) VALUES ('https://idp', 'g-core', 2); + PRAGMA user_version = 4; + `); + return db; +} + +const roleSource = (db: DatabaseSync, sub: string): string | null => + (db.prepare("SELECT role_source FROM users WHERE sub = ?").get(sub) as { role_source: string | null }) + .role_source; + +describe("migration v4 → v5", () => { + it("keeps an existing role as an explicit override, so nobody's access moves", () => { + const db = legacyDatabase(); + migrate(db); + const repo = new Repo(db); + + // The pre-existing role still resolves and now REPLACES group mappings — + // today's behaviour exactly, even though this user is in a mapped group. + expect(repo.resolveOidcRoles("https://idp", "has-role", ["g-core"]).map((r) => r.name)).toEqual([ + "managers-ro", + ]); + expect(roleSource(db, "has-role")).toBe("admin"); + + // A user without one now picks up every mapped group (the new behaviour). + expect(repo.resolveOidcRoles("https://idp", "no-role", ["g-core"]).map((r) => r.name)).toEqual([ + "editor", + ]); + expect(roleSource(db, "no-role")).toBeNull(); + }); + + it("creates the login-roles table, and re-running changes nothing", () => { + const db = legacyDatabase(); + migrate(db); + const repo = new Repo(db); + repo.setLoginRoles(2, [1, 2]); + expect(repo.loginRoles(2).map((r) => r.name)).toEqual(["editor", "viewer"]); + + // Re-entering the same migration must not throw (duplicate column/table) + // nor disturb stored rows — the ADD COLUMN is guarded by a column check. + db.exec("PRAGMA user_version = 4"); + expect(() => migrate(db)).not.toThrow(); + expect(repo.loginRoles(2)).toHaveLength(2); + expect(roleSource(db, "no-role")).toBeNull(); + }); + + it("reaches v5 on a fresh database too", () => { + const db = new DatabaseSync(":memory:"); + migrate(db); + expect((db.prepare("PRAGMA user_version").get() as { user_version: number }).user_version).toBe(5); + }); +}); diff --git a/packages/gateway/src/db/repo.test.ts b/packages/gateway/src/db/repo.test.ts index 4f88f1d..b29fda4 100644 --- a/packages/gateway/src/db/repo.test.ts +++ b/packages/gateway/src/db/repo.test.ts @@ -99,21 +99,39 @@ describe("Repo", () => { expect(again.email).toBe("a@b.c"); // COALESCE keeps the earlier email }); - it("resolveOidcRole: explicit user role beats group mappings; highest tier mapping wins", () => { + it("resolveOidcRoles: every mapped group counts; an explicit override replaces them", () => { const repo = fresh(); const viewer = repo.roleByName("viewer")!; const editor = repo.roleByName("editor")!; const admin = repo.roleByName("admin")!; const user = repo.upsertUserOnLogin({ iss: "https://idp", sub: "u1" }); + const names = (groups: string[]) => repo.resolveOidcRoles("https://idp", "u1", groups).map((r) => r.name); - expect(repo.resolveOidcRole("https://idp", "u1", ["g1"])).toBeNull(); + expect(names(["g1"])).toEqual([]); repo.setGroupMapping("https://idp", "g1", viewer.id); repo.setGroupMapping("https://idp", "g2", editor.id); - expect(repo.resolveOidcRole("https://idp", "u1", ["g1", "g2"])?.name).toBe("editor"); - + // the UNION, most privileged first — not just the winner (issue #28) + expect(names(["g1", "g2"])).toEqual(["editor", "viewer"]); + expect(names(["g1"])).toEqual(["viewer"]); + // an unmapped group contributes nothing rather than failing the lookup + expect(names(["g1", "nope"])).toEqual(["viewer"]); + + // no groups in hand (cookie / gateway-JWT paths) → what the last login stored + expect(names([])).toEqual([]); + repo.setLoginRoles(user.id, [viewer.id, editor.id]); + expect(names([])).toEqual(["editor", "viewer"]); + // replace-all: losing a group removes its role instead of accumulating + repo.setLoginRoles(user.id, [viewer.id]); + expect(names([])).toEqual(["viewer"]); + + // an explicit override replaces group-derived roles entirely repo.setUserRole(user.id, admin.id); - expect(repo.resolveOidcRole("https://idp", "u1", ["g1"])?.name).toBe("admin"); + expect(names(["g1", "g2"])).toEqual(["admin"]); + expect(repo.userBySubject("https://idp", "u1")?.roleId).toBe(admin.id); + // …and clearing it falls back to the roles the last login remembered + repo.setUserRole(user.id, null); + expect(names([])).toEqual(["viewer"]); }); it("stores and reads back OAuth clients", () => { diff --git a/packages/gateway/src/db/repo.ts b/packages/gateway/src/db/repo.ts index 406e423..fbcb0d2 100644 --- a/packages/gateway/src/db/repo.ts +++ b/packages/gateway/src/db/repo.ts @@ -102,6 +102,10 @@ export interface UserCredentialRow { updatedAt: string; } +/** Ordering helper: "most privileged first" for role lists. */ +const TIER_RANK_SQL = `CASE r.default_max_tier + WHEN 'destructive' THEN 3 WHEN 'write' THEN 2 WHEN 'read' THEN 1 ELSE 0 END`; + export class Repo { constructor(private readonly db: DatabaseSync) {} @@ -423,11 +427,48 @@ export class Repo { ).map(mapUser); } + /** Admin/bootstrap override — REPLACES whatever the user's groups map to. */ setUserRole(userId: number, roleId: number | null): boolean { - const result = this.db.prepare("UPDATE users SET role_id = ? WHERE id = ?").run(roleId, userId); + const result = this.db + .prepare("UPDATE users SET role_id = ?, role_source = ? WHERE id = ?") + .run(roleId, roleId === null ? null : "admin", userId); return result.changes > 0; } + /** + * Remember every role the login's group claims mapped to. Group claims only + * arrive with a fresh IdP token, so the cookie/JWT paths (which see no groups) + * read these rows back instead of re-deriving. Replace-all in one transaction: + * losing a group must remove its role, not accumulate forever. + */ + setLoginRoles(userId: number, roleIds: number[]): void { + this.db.exec("BEGIN"); + try { + this.db.prepare("DELETE FROM user_login_roles WHERE user_id = ?").run(userId); + const insert = this.db.prepare( + "INSERT OR IGNORE INTO user_login_roles (user_id, role_id) VALUES (?, ?)" + ); + for (const roleId of roleIds) insert.run(userId, roleId); + this.db.exec("COMMIT"); + } catch (err) { + this.db.exec("ROLLBACK"); + throw err; + } + } + + loginRoles(userId: number): RoleRow[] { + return ( + this.db + .prepare( + `SELECT r.id, r.name, r.default_max_tier, r.is_admin, r.protected + FROM user_login_roles ulr JOIN roles r ON r.id = ulr.role_id + WHERE ulr.user_id = ? + ORDER BY r.is_admin DESC, ${TIER_RANK_SQL} DESC, r.id` + ) + .all(userId) as Array> + ).map(mapRole); + } + // ── group mappings ── listGroupMappings(): GroupMappingRow[] { @@ -454,25 +495,41 @@ export class Repo { return this.db.prepare("DELETE FROM group_mappings WHERE id = ?").run(id).changes > 0; } - /** Resolve the role for an OIDC login: user override > group mapping (highest tier wins). */ - resolveOidcRole(iss: string, sub: string, groups: string[]): RoleRow | null { + /** + * Every role an OIDC principal holds — the envelope is their UNION, so being + * added to a group can only widen access (issue #28). Order is + * "most privileged first" so callers can take [0] as the primary for display. + * + * - an explicit `users.role_id` override REPLACES group-derived roles (that is + * the point of an override); + * - otherwise every mapped group counts, not just the highest; + * - with no groups in hand (cookie / gateway-JWT paths) the roles remembered + * at the last real login are used. + */ + resolveOidcRoles(iss: string, sub: string, groups: string[]): RoleRow[] { const user = this.userBySubject(iss, sub); - if (user?.roleId != null) return this.roleById(user.roleId); - if (groups.length === 0) return null; + if (user?.roleId != null) { + const role = this.roleById(user.roleId); + return role ? [role] : []; + } + if (groups.length === 0) return user ? this.loginRoles(user.id) : []; + return this.rolesForGroups(iss, groups); + } + + /** Roles mapped from group claims, most privileged first. Override-blind. */ + rolesForGroups(iss: string, groups: string[]): RoleRow[] { + if (groups.length === 0) return []; const placeholders = groups.map(() => "?").join(","); - const row = this.db - .prepare( - `SELECT r.id, r.name, r.default_max_tier, r.is_admin, r.protected - FROM group_mappings gm JOIN roles r ON r.id = gm.role_id - WHERE gm.iss = ? AND gm.claim_value IN (${placeholders}) - ORDER BY r.is_admin DESC, - CASE r.default_max_tier - WHEN 'destructive' THEN 3 WHEN 'write' THEN 2 WHEN 'read' THEN 1 ELSE 0 - END DESC - LIMIT 1` - ) - .get(iss, ...groups) as Record | undefined; - return row ? mapRole(row) : null; + return ( + this.db + .prepare( + `SELECT DISTINCT r.id, r.name, r.default_max_tier, r.is_admin, r.protected + FROM group_mappings gm JOIN roles r ON r.id = gm.role_id + WHERE gm.iss = ? AND gm.claim_value IN (${placeholders}) + ORDER BY r.is_admin DESC, ${TIER_RANK_SQL} DESC, r.id` + ) + .all(iss, ...groups) as Array> + ).map(mapRole); } // ── user prefs (personal narrowing — slice 3) ── diff --git a/packages/gateway/src/domain/additive-roles.test.ts b/packages/gateway/src/domain/additive-roles.test.ts new file mode 100644 index 0000000..e76ae4c --- /dev/null +++ b/packages/gateway/src/domain/additive-roles.test.ts @@ -0,0 +1,120 @@ +/** + * Additive roles (issue #28): a principal holding several roles gets their + * UNION. Being added to a group can only ever widen access — the surprise this + * prevents is "we gave you billing access" silently removing ticket access. + */ + +import { describe, expect, it } from "vitest"; +import type { Tool } from "@modelcontextprotocol/sdk/types.js"; +import { openDatabase } from "../db/index.js"; +import { Repo } from "../db/repo.js"; +import type { Principal } from "../auth/principal.js"; +import { principalKey, withRoles } from "../auth/principal.js"; +import type { CatalogEntry } from "./catalog.js"; +import { PolicyService } from "./policy.js"; + +const tool = (name: string): Tool => ({ name, inputSchema: { type: "object" } }); +const entry = ( + upstreamToolName: string, + tier: "read" | "write" | "destructive", + upstreamId: string +): CatalogEntry => ({ + upstreamId, + namespace: upstreamId, + upstreamToolName, + exposedName: `${upstreamId}_${upstreamToolName}`, + tier, + tool: tool(upstreamToolName), +}); + +/** + * Two deliberately INCOMPARABLE roles, the case "highest tier wins" got wrong: + * techs may write tickets and cannot touch billing; billing may write invoices + * and cannot touch tickets. Neither contains the other. + */ +function setup() { + const repo = new Repo(openDatabase(":memory:")); + const policy = new PolicyService(repo); + const techs = repo.createRole("techs", "none"); + const billing = repo.createRole("billing", "none"); + repo.setGrant(techs.id, "tickets", "write"); + repo.setGrant(billing.id, "invoices", "write"); + + const asRoles = (...roles: Array<{ id: number; name: string }>): Principal => + withRoles( + { kind: "oidc", subject: "https://idp|u1", label: "alice" }, + roles.map((r) => ({ id: r.id, name: r.name, isAdmin: false })) + ); + + return { repo, policy, techs, billing, asRoles }; +} + +const ticket = entry("update_ticket", "write", "tickets"); +const invoice = entry("update_invoice", "write", "invoices"); + +describe("role union", () => { + it("keeps both surfaces where a single winner would drop one", () => { + const { policy, techs, billing, asRoles } = setup(); + + const techOnly = asRoles(techs); + expect(policy.allowsFor(techOnly, ticket)).toBe(true); + expect(policy.allowsFor(techOnly, invoice)).toBe(false); + + // Adding the billing group must ADD invoices without removing tickets. + const both = asRoles(techs, billing); + expect(policy.allowsFor(both, ticket)).toBe(true); + expect(policy.allowsFor(both, invoice)).toBe(true); + expect(policy.envelopeFor(both, [ticket, invoice]).map((e) => e.upstreamId)).toEqual([ + "tickets", + "invoices", + ]); + }); + + it("takes the most permissive ceiling per upstream, not the first role's", () => { + const { repo, policy, techs, billing, asRoles } = setup(); + // techs are capped at read on invoices, billing may write them + repo.setGrant(techs.id, "invoices", "read"); + const both = asRoles(techs, billing); + expect(policy.allowsFor(both, entry("get_invoice", "read", "invoices"))).toBe(true); + expect(policy.allowsFor(both, invoice)).toBe(true); + + // …and a role granted "none" doesn't drag the union down + repo.setGrant(techs.id, "invoices", "none"); + expect(policy.allowsFor(both, invoice)).toBe(true); + }); + + it("a per-role deny closes that role's path only — the kill switch closes all", () => { + const { repo, policy, techs, billing, asRoles } = setup(); + repo.setGrant(billing.id, "tickets", "write"); + repo.setOverride(techs.id, "tickets", "update_ticket", "deny"); + const both = asRoles(techs, billing); + + // still reachable through billing: subtracting for everyone is not a role's job + expect(policy.allows(techs.id, ticket)).toBe(false); + expect(policy.allowsFor(both, ticket)).toBe(true); + + // the global kill switch is the absolute one + repo.upsertToolSetting({ upstreamId: "tickets", toolName: "update_ticket", enabled: false }); + expect(policy.allowsFor(both, ticket)).toBe(false); + }); + + it("an admin role anywhere in the list makes the principal an admin", () => { + const { repo, techs } = setup(); + const adminRole = repo.roleByName("admin")!; + const principal = withRoles({ kind: "oidc", subject: "https://idp|u1", label: "alice" }, [ + { id: techs.id, name: "techs", isAdmin: false }, + { id: adminRole.id, name: "admin", isAdmin: true }, + ]); + expect(principal.isAdmin).toBe(true); + // primary stays the first entry (callers order most-privileged-first) + expect(principal.roleId).toBe(techs.id); + expect(principal.roleName).toBe("techs+admin"); + }); + + it("the session binding key covers the whole role set, order-independently", () => { + const { techs, billing, asRoles } = setup(); + expect(principalKey(asRoles(techs, billing))).toBe(principalKey(asRoles(billing, techs))); + // losing a role changes the key → the existing 403 forces a reconnect + expect(principalKey(asRoles(techs))).not.toBe(principalKey(asRoles(techs, billing))); + }); +}); diff --git a/packages/gateway/src/domain/policy-narrowing.test.ts b/packages/gateway/src/domain/policy-narrowing.test.ts index d177665..3279411 100644 --- a/packages/gateway/src/domain/policy-narrowing.test.ts +++ b/packages/gateway/src/domain/policy-narrowing.test.ts @@ -3,7 +3,7 @@ import type { Tool } from "@modelcontextprotocol/sdk/types.js"; import { openDatabase } from "../db/index.js"; import { Repo } from "../db/repo.js"; import type { Principal } from "../auth/principal.js"; -import { prefsIdentity, principalSlug } from "../auth/principal.js"; +import { prefsIdentity, principalSlug, withRoles } from "../auth/principal.js"; import type { CatalogEntry } from "./catalog.js"; import { PolicyService } from "./policy.js"; @@ -21,14 +21,14 @@ function setup() { const repo = new Repo(openDatabase(":memory:")); const policy = new PolicyService(repo); const editorRole = repo.roleByName("editor")!; - const editor: Principal = { - kind: "oidc", - subject: "https://login.example|ab428be9-1111-2222-3333-444455556666", - label: "alice", - roleId: editorRole.id, - roleName: "editor", - isAdmin: false, - }; + const editor: Principal = withRoles( + { + kind: "oidc", + subject: "https://login.example|ab428be9-1111-2222-3333-444455556666", + label: "alice", + }, + [{ id: editorRole.id, name: "editor", isAdmin: false }] + ); return { repo, policy, editor }; } @@ -144,7 +144,9 @@ describe("personal narrowing (allowsFor = envelope ∧ prefs)", () => { const { repo, policy, editor } = setup(); repo.setUserPref(prefsIdentity(editor), "up1", "get_doc", false); const viewerRole = repo.roleByName("viewer")!; - const sameUserNewRole: Principal = { ...editor, roleId: viewerRole.id, roleName: "viewer" }; + const sameUserNewRole: Principal = withRoles(editor, [ + { id: viewerRole.id, name: "viewer", isAdmin: false }, + ]); expect(policy.allowsFor(sameUserNewRole, entry("get_doc", "read"))).toBe(false); }); }); @@ -185,28 +187,19 @@ describe("denialReason (only self-inflicted denials are nameable)", () => { describe("principalSlug", () => { it("passes Entra OIDs through recognizably", () => { - const p: Principal = { - kind: "oidc", - subject: "https://login.example|AB428BE9-1111-2222-3333-444455556666", - label: "x", - roleId: 1, - roleName: "viewer", - isAdmin: false, - }; + const p: Principal = withRoles({ kind: "oidc", subject: "https://login.example|AB428BE9-1111-2222-3333-444455556666", label: "x" }, [ + { id: 1, name: "viewer", isAdmin: false }, + ]); expect(principalSlug(p)).toBe("ab428be9-1111-2222-3333-444455556666"); }); it("hashes non-KV-safe subjects deterministically", () => { - const p: Principal = { - kind: "oidc", - subject: "user@example.com", - label: "x", - roleId: 1, - roleName: "viewer", - isAdmin: false, - }; + const p: Principal = withRoles({ kind: "oidc", subject: "user@example.com", label: "x" }, [ + { id: 1, name: "viewer", isAdmin: false }, + ]); const slug = principalSlug(p); expect(slug).toMatch(/^[0-9a-f]{16}$/); - expect(principalSlug({ ...p, roleId: 99 })).toBe(slug); // role-independent + // role-independent + expect(principalSlug(withRoles(p, [{ id: 99, name: "admin", isAdmin: true }]))).toBe(slug); }); }); diff --git a/packages/gateway/src/domain/policy.ts b/packages/gateway/src/domain/policy.ts index d40d569..cde03ff 100644 --- a/packages/gateway/src/domain/policy.ts +++ b/packages/gateway/src/domain/policy.ts @@ -65,6 +65,22 @@ export class PolicyService { return [...entries].filter((entry) => this.allows(roleId, entry)); } + /** + * The envelope of a principal holding several roles: the UNION (issue #28). + * Each role is evaluated on its own — its grant, its default tier, its own + * per-tool overrides — so a `deny` override closes that role's path and not + * the others'. Subtracting for everyone is what the kill switch is for. + */ + allowsAny(roleIds: readonly number[], entry: CatalogEntry): boolean { + return roleIds.some((roleId) => this.allows(roleId, entry)); + } + + /** Envelope of every role the principal holds (no personal prefs applied). */ + envelopeFor(principal: Principal, entries: Iterable): CatalogEntry[] { + const roleIds = principal.roles.map((r) => r.id); + return [...entries].filter((entry) => this.allowsAny(roleIds, entry)); + } + /** * Personal narrowing (slice 3): effective = admin envelope ∧ user prefs. * Prefs are deny-only rows (an upstream-wide '' row or a per-tool row), so @@ -72,7 +88,7 @@ export class PolicyService { * Same function gates tools/list and tools/call, like the envelope itself. */ allowsFor(principal: Principal, entry: CatalogEntry): boolean { - if (!this.allows(principal.roleId, entry)) return false; + if (!this.allowsAny(principal.roles.map((r) => r.id), entry)) return false; const who = prefsIdentity(principal); const serverPref = this.repo.userPrefFor(who, entry.upstreamId, ""); const toolPref = this.repo.userPrefFor(who, entry.upstreamId, entry.upstreamToolName); @@ -100,7 +116,7 @@ export class PolicyService { * else stays `"envelope"` and keeps the no-oracle wording. */ denialReason(principal: Principal, entry: CatalogEntry): "allowed" | "envelope" | "personal" | "optIn" { - if (!this.allows(principal.roleId, entry)) return "envelope"; + if (!this.allowsAny(principal.roles.map((r) => r.id), entry)) return "envelope"; if (this.allowsFor(principal, entry)) return "allowed"; return this.repo.getUpstream(entry.upstreamId)?.spec.userDefault === "off" ? "optIn" : "personal"; } diff --git a/packages/gateway/src/http/admin-api.ts b/packages/gateway/src/http/admin-api.ts index a6802bb..2ada367 100644 --- a/packages/gateway/src/http/admin-api.ts +++ b/packages/gateway/src/http/admin-api.ts @@ -506,7 +506,20 @@ export function createAdminRouter(deps: AppDeps, admin: AdminDeps): Router { router.get( "/users", h((_req, res) => { - res.json(repo.listUsers()); + // roleId is the ADMIN override; loginRoles are what the user's groups + // mapped to at their last login. Effective access is the union of + // whichever applies (#28), so show both rather than one "role" column. + res.json( + repo.listUsers().map((user) => { + const loginRoles = repo.loginRoles(user.id).map((r) => r.name); + const override = user.roleId != null ? repo.roleById(user.roleId)?.name ?? null : null; + return { + ...user, + loginRoles, + effectiveRoles: override ? [override] : loginRoles, + }; + }) + ); }) ); diff --git a/packages/gateway/src/http/app.test.ts b/packages/gateway/src/http/app.test.ts index b6600a2..d15af3e 100644 --- a/packages/gateway/src/http/app.test.ts +++ b/packages/gateway/src/http/app.test.ts @@ -583,6 +583,21 @@ describe("session reload", () => { expect(await listFederated("tok-admin", theirs)).toContain("fake_read_thing"); }); + it("shows me my own clients and whether they can be notified at all", async () => { + const sid = await initSession("tok-viewer"); + await initSession("tok-admin"); // someone else's, must not appear + + const mine = (await ( + await fetch(`${base}/api/me/sessions`, { headers: { Authorization: "Bearer tok-viewer" } }) + ).json()) as { sessions: Array<{ sessionId: string; streamOpen: boolean }>; notificationStream: boolean }; + + expect(mine.sessions.map((s) => s.sessionId)).toContain(sid); + expect(mine.sessions.every((s) => s.streamOpen === false)).toBe(true); + // No GET stream anywhere → the gateway cannot push list_changed at all, + // which is what makes "Apply now" the only cure. Say so, don't imply it. + expect(mine.notificationStream).toBe(false); + }); + it("is admin-only on /api/sessions, and targets one session or one principal", async () => { const sid = await initSession("tok-viewer"); diff --git a/packages/gateway/src/http/app.ts b/packages/gateway/src/http/app.ts index 456b5ab..629fbd4 100644 --- a/packages/gateway/src/http/app.ts +++ b/packages/gateway/src/http/app.ts @@ -34,7 +34,7 @@ import type { UpstreamManager } from "../upstream/manager.js"; import type { SecretStore } from "../secrets/store.js"; import type { OidcVerifier, OidcIdentity } from "../auth/oidc.js"; import { authenticateStaticToken, bearerToken } from "../auth/static-tokens.js"; -import { prefsIdentity, principalKey, type Principal } from "../auth/principal.js"; +import { prefsIdentity, principalKey, withRoles, type Principal } from "../auth/principal.js"; import { SESSION_COOKIE, TRANSIENT_COOKIE, @@ -170,7 +170,7 @@ export function loginUpsert( repo: Repo, config: GatewayConfig, identity: OidcIdentity -): { user: UserRow; role: RoleRow | null } { +): { user: UserRow; roles: RoleRow[] } { const user = repo.upsertUserOnLogin({ iss: identity.iss, sub: identity.sub, @@ -189,8 +189,16 @@ export function loginUpsert( } } } - const role = repo.resolveOidcRole(identity.iss, identity.sub, identity.groups); - return { user, role }; + // Group claims only ride on a real IdP token, so remember what they mapped to: + // the cookie and gateway-JWT paths see no groups and read these back. Stored + // even when an explicit override currently wins, so clearing the override + // later falls back to today's groups rather than to nothing. + repo.setLoginRoles( + user.id, + repo.rolesForGroups(identity.iss, identity.groups).map((r) => r.id) + ); + const roles = repo.resolveOidcRoles(identity.iss, identity.sub, identity.groups); + return { user, roles }; } /** Resolve the principal for a request. Exported for tests and the admin API. */ @@ -222,14 +230,9 @@ export function createAuthResolver(deps: AppDeps) { } return { ok: true, - principal: { - kind: "static", - subject: staticEntry.label, - label: staticEntry.label, - roleId: role.id, - roleName: role.name, - isAdmin: role.isAdmin, - }, + principal: withRoles({ kind: "static", subject: staticEntry.label, label: staticEntry.label }, [ + { id: role.id, name: role.name, isAdmin: role.isAdmin }, + ]), }; } @@ -252,19 +255,19 @@ export function createAuthResolver(deps: AppDeps) { } catch (err) { return unauthorized(`Invalid token: ${err instanceof Error ? err.message : String(err)}`); } - const role = repo.resolveOidcRole(identity.iss, identity.sub, []); - if (!role) { + const roles = repo.resolveOidcRoles(identity.iss, identity.sub, []); + if (roles.length === 0) { return { ok: false, status: 403, code: -32003, message: NO_ROLE_MESSAGE }; } const user = repo.userBySubject(identity.iss, identity.sub); - const principal: Principal = { - kind: "oidc", - subject: `${identity.iss}|${identity.sub}`, - label: user?.email ?? user?.displayName ?? identity.sub, - roleId: role.id, - roleName: role.name, - isAdmin: role.isAdmin, - }; + const principal: Principal = withRoles( + { + kind: "oidc", + subject: `${identity.iss}|${identity.sub}`, + label: user?.email ?? user?.displayName ?? identity.sub, + }, + roles + ); cache.set(cacheKey, { principal, expiresAt: Date.now() + CACHE_TTL_MS }); return { ok: true, principal }; } @@ -282,18 +285,14 @@ export function createAuthResolver(deps: AppDeps) { } catch (err) { return unauthorized(`Invalid token: ${err instanceof Error ? err.message : String(err)}`); } - const { role } = loginUpsert(repo, config, identity); - if (!role) { + const { roles } = loginUpsert(repo, config, identity); + if (roles.length === 0) { return { ok: false, status: 403, code: -32003, message: NO_ROLE_MESSAGE }; } - const principal: Principal = { - kind: "oidc", - subject: `${identity.iss}|${identity.sub}`, - label: identity.email ?? identity.sub, - roleId: role.id, - roleName: role.name, - isAdmin: role.isAdmin, - }; + const principal: Principal = withRoles( + { kind: "oidc", subject: `${identity.iss}|${identity.sub}`, label: identity.email ?? identity.sub }, + roles + ); cache.set(cacheKey, { principal, expiresAt: Date.now() + CACHE_TTL_MS }); return { ok: true, principal }; } @@ -307,21 +306,21 @@ export function createAuthResolver(deps: AppDeps) { const cookies = parseCookies(req.headers.cookie); const session = readSessionClaims(cookies[SESSION_COOKIE], config.login.sessionSecret); if (session) { - const role = repo.resolveOidcRole(session.iss, session.sub, []); - if (!role) { + const roles = repo.resolveOidcRoles(session.iss, session.sub, []); + if (roles.length === 0) { return { ok: false, status: 403, code: -32003, message: NO_ROLE_MESSAGE }; } const user = repo.userBySubject(session.iss, session.sub); return { ok: true, - principal: { - kind: "oidc", - subject: `${session.iss}|${session.sub}`, - label: user?.email ?? user?.displayName ?? session.sub, - roleId: role.id, - roleName: role.name, - isAdmin: role.isAdmin, - }, + principal: withRoles( + { + kind: "oidc", + subject: `${session.iss}|${session.sub}`, + label: user?.email ?? user?.displayName ?? session.sub, + }, + roles + ), }; } } @@ -332,14 +331,9 @@ export function createAuthResolver(deps: AppDeps) { if (admin) { return { ok: true, - principal: { - kind: "dev", - subject: "dev", - label: "dev-unauthenticated", - roleId: admin.id, - roleName: admin.name, - isAdmin: true, - }, + principal: withRoles({ kind: "dev", subject: "dev", label: "dev-unauthenticated" }, [ + { id: admin.id, name: admin.name, isAdmin: true }, + ]), }; } } @@ -594,6 +588,7 @@ export function createApp(deps: AppDeps): express.Express { resolveAuth, onPolicyChanged: broadcastVisibility, reloadSessions, + sessionSummaries, }) ); @@ -665,10 +660,10 @@ export function createApp(deps: AppDeps): express.Express { res.status(401).send("Sign-in failed. Please try again."); return; } - const { user, role } = loginUpsert(deps.repo, config, identity); - // Persist the resolved role so the cookie path resolves it from the - // stored user row (approach b) — no privilege ever rides in the cookie. - if (role) deps.repo.setUserRole(user.id, role.id); + // loginUpsert persists the group-derived roles (user_login_roles), which + // is what the cookie path reads back — no privilege ever rides in the + // cookie, and an explicit admin override still wins over them. + loginUpsert(deps.repo, config, identity); // OAuth AS facade: this login was brokering user authentication for an // MCP client — mint a single-use code bound to the pending request and // bounce to the client's registered redirect_uri (validated at @@ -883,14 +878,9 @@ export function createApp(deps: AppDeps): express.Express { const sessionPrincipal = (req: Request): Principal | null => { const claims = readSessionClaims(parseCookies(req.headers.cookie)[SESSION_COOKIE], login.sessionSecret); if (!claims) return null; - return { - kind: "oidc", - subject: `${claims.iss}|${claims.sub}`, - label: claims.sub, - roleId: 0, - roleName: "", - isAdmin: false, - }; + return withRoles({ kind: "oidc", subject: `${claims.iss}|${claims.sub}`, label: claims.sub }, [ + { id: 0, name: "", isAdmin: false }, + ]); }; // NOTE: registered before the parametric route below — otherwise diff --git a/packages/gateway/src/http/me-api.ts b/packages/gateway/src/http/me-api.ts index 0b3bd66..1bce23f 100644 --- a/packages/gateway/src/http/me-api.ts +++ b/packages/gateway/src/http/me-api.ts @@ -26,6 +26,8 @@ interface MeDeps { onPolicyChanged: () => void; /** Close matching live MCP sessions; returns how many were dropped. */ reloadSessions: (match: (session: { principal: Principal }, sessionId: string) => boolean) => number; + /** Live sessions (all principals) — filtered to the caller here. */ + sessionSummaries: () => Array>; } export function createMeRouter(deps: AppDeps, me: MeDeps): Router { @@ -76,7 +78,9 @@ export function createMeRouter(deps: AppDeps, me: MeDeps): Router { string, Array<{ name: string; exposedName: string; tier: string; group: string; enabled: boolean }> >(); - for (const entry of policy.visibleEntries(principal.roleId, manager.catalogEntries())) { + // Union of every role the caller holds (issue #28) — a person in two + // mapped groups sees both surfaces here, not just the higher one. + for (const entry of policy.envelopeFor(principal, manager.catalogEntries())) { const list = byUpstream.get(entry.upstreamId) ?? []; list.push({ name: entry.upstreamToolName, @@ -90,7 +94,11 @@ export function createMeRouter(deps: AppDeps, me: MeDeps): Router { byUpstream.set(entry.upstreamId, list); } res.json({ - principal: { label: principal.label, role: principal.roleName }, + principal: { + label: principal.label, + role: principal.roleName, + roles: principal.roles.map((r) => r.name), + }, servers: [...byUpstream.entries()].map(([upstreamId, tools]) => { const spec = repo.getUpstream(upstreamId)?.spec; const optIn = spec?.userDefault === "off"; @@ -135,7 +143,7 @@ export function createMeRouter(deps: AppDeps, me: MeDeps): Router { // Prefs only make sense inside the envelope; reject junk targets so the // table can't fill with garbage (and enabling can never widen anyway — // "enable" just deletes the personal deny row). - const envelope = policy.visibleEntries(principal.roleId, manager.catalogEntries()); + const envelope = policy.envelopeFor(principal, manager.catalogEntries()); const upstreamKnown = envelope.some((e) => e.upstreamId === body.upstreamId); const toolKnown = body.toolName === "" || @@ -190,6 +198,24 @@ export function createMeRouter(deps: AppDeps, me: MeDeps): Router { * notification, and a stale list is what makes a pref change look ignored — * enforcement was never stale, only the display. Own sessions only. */ + /** + * My live MCP sessions. `streamOpen: false` means the gateway has no channel + * to push tools/list_changed to that client, which is exactly when "Apply + * now" is the only way to make it re-read — worth showing rather than + * explaining. + */ + router.get( + "/sessions", + h((req, res) => { + const who = prefsIdentity(req.principal!); + const mine = me.sessionSummaries().filter((s) => s.principal === who); + res.json({ + sessions: mine.map(({ sessionId, streamOpen, toolCount }) => ({ sessionId, streamOpen, toolCount })), + notificationStream: mine.some((s) => s.streamOpen === true), + }); + }) + ); + router.post( "/sessions/reload", h((req, res) => { diff --git a/packages/gateway/src/mcp/gateway-server.ts b/packages/gateway/src/mcp/gateway-server.ts index a925412..6e7ecd4 100644 --- a/packages/gateway/src/mcp/gateway-server.ts +++ b/packages/gateway/src/mcp/gateway-server.ts @@ -41,16 +41,20 @@ export function createGatewayServer( // Envelope ∧ personal prefs — the same allowsFor gates list AND call, so // a user's own narrowing is enforced at the boundary, not just hidden in UX. - server.setRequestHandler(ListToolsRequestSchema, async () => ({ - tools: [ + server.setRequestHandler(ListToolsRequestSchema, async () => { + const tools = [ // Self-management is offered to admins only; for everyone else these // names simply don't exist (the call handler re-checks anyway). ...(selfTools && principal.isAdmin ? SELF_TOOLS : []), ...policy .visibleEntriesFor(principal, manager.catalogEntries()) .map((entry) => ({ ...entry.tool, name: entry.exposedName })), - ], - })); + ]; + // Logged because "did the client ever re-read the list?" was otherwise + // unanswerable, and that question cost a whole afternoon once. + console.error(`[mcp] ${principal.label} tools/list -> ${tools.length} tool(s)`); + return { tools }; + }); server.setRequestHandler(CallToolRequestSchema, async (request) => { // The `gw` namespace is reserved (config.ts refuses it for upstreams), so @@ -71,6 +75,10 @@ export function createGatewayServer( // their own switch. Every other case keeps the no-oracle wording: unknown, // globally disabled and role-denied tools stay indistinguishable. const reason = entry ? policy.denialReason(principal, entry) : "envelope"; + // Names and decisions only — never arguments, which carry customer data. + console.error( + `[mcp] ${principal.label} tools/call ${request.params.name} -> denied (${entry ? reason : "unknown"})` + ); const where = meUrl ? ` (${meUrl})` : ""; const text = reason === "personal" @@ -81,6 +89,7 @@ export function createGatewayServer( return { isError: true, content: [{ type: "text" as const, text }] }; } const args = request.params.arguments ?? {}; + console.error(`[mcp] ${principal.label} tools/call ${request.params.name} -> allowed`); // sessionMode:"per-user" — route the call over the caller's own // connection, with their registered credential refs layered onto the diff --git a/packages/gateway/src/mcp/self-tools.test.ts b/packages/gateway/src/mcp/self-tools.test.ts index 9b29240..4262ec3 100644 --- a/packages/gateway/src/mcp/self-tools.test.ts +++ b/packages/gateway/src/mcp/self-tools.test.ts @@ -111,6 +111,10 @@ describe("read-only tools", () => { const status = parsed(await callSelfTool(deps, admin, "gw_status", {})); expect(status.toolCount).toBe(3); expect((status.upstreams as Array<{ id: string }>).map((u) => u.id)).toEqual(["fake"]); + // version is the release, mode is standalone/integrated — the field used to + // carry the mode, so gw_status claimed to be running version "integrated" + expect(status.version).toMatch(/^\d+\.\d+\.\d+/); + expect(status.mode).toBe("standalone"); }); it("gw_list_servers redacts literal credential values but shows refs", async () => { diff --git a/packages/gateway/src/mcp/self-tools.ts b/packages/gateway/src/mcp/self-tools.ts index 4a0d032..f49d167 100644 --- a/packages/gateway/src/mcp/self-tools.ts +++ b/packages/gateway/src/mcp/self-tools.ts @@ -28,6 +28,7 @@ import { isMaxTier } from "../domain/policy.js"; import { renderPreset } from "../domain/presets.js"; import { effectiveGroupOf, effectiveTierOf, resolveToolTargets } from "../domain/tool-targets.js"; import { runBackup } from "../db/backup.js"; +import { SERVER_VERSION } from "../version.js"; import type { BackupUploader } from "../db/backup.js"; /** Reserved namespace: `config.ts` refuses it for upstreams. */ @@ -266,7 +267,10 @@ export async function callSelfTool( switch (name) { case `${SELF_NAMESPACE}_status`: return json({ - version: config.mode === "integrated" ? "integrated" : "standalone", + // The field said "version" and carried the MODE — so gw_status claimed + // the gateway was running version "integrated". Report both. + version: SERVER_VERSION, + mode: config.mode, toolCount: entries().length, upstreams: manager.summaries(), secretStore: config.bao ? "openbao" : config.keyVault ? "keyvault" : null,