From bbea0f74f356fe249a25bf5fe1f2a096a9ea4ab7 Mon Sep 17 00:00:00 2001 From: Josh Knapp Date: Wed, 12 Aug 2026 10:51:29 -0700 Subject: [PATCH 1/2] feat: support Microsoft Entra ID as an OIDC provider MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three defects stood between this codebase and a working Entra deployment. All three fail silently, which is why they are grouped: each one masks the next, and fixing any two still leaves a broken or dangerous install. 1. JWKS discovery. The key set URL was hardcoded to `${issuer}/jwks/`, which is Authentik's convention, not a standard. Entra serves keys at `/{tenant}/discovery/v2.0/keys`, so every Entra-issued MCP token failed verification on a 404 — authentication was impossible, not merely misconfigured. We now read `jwks_uri` from the issuer's discovery document and fall back to the old path, so Authentik is untouched. Discovery failure arms a 60s retry rather than pinning the wrong URL for the life of the container. 2. Identity. Entra's `sub` is pairwise — derived from the token recipient — so the Web UI and MCP app registrations emit different `sub` values for the same human. Keyed on (iss, sub), that person got two rows: sign into the Web UI, connect Claude Code, land in an empty account. Both paths upsert, so nothing errored. Identity now keys on `oid`, which Microsoft documents as constant across applications in a tenant, via one resolver both surfaces share. Rows created before the 0005 migration adopt their `oid` on next sign-in. 3. Groups overage. Past 200 groups Entra omits `groups` entirely and substitutes a `_claim_names` pointer. `normalizeGroupsClaim` read that as "zero groups" and the sync deleted every membership the user had, revoking access to every shared project on both surfaces with no error raised. Both surfaces now refuse such a token instead — the Web UI fails the sign-in, MCP returns 401 — leaving memberships intact and naming the operator fix. An absent claim with no overage marker still clears memberships, which is unchanged and deliberate. Verified against a real pgvector instance: 66 tests pass, and reverting either new behaviour fails exactly the tests that cover it. Co-Authored-By: Claude Opus 5 (1M context) --- apps/web/auth.ts | 45 +++-- apps/web/drizzle/0005_user_oid.sql | 29 ++++ apps/web/lib/auth/identity.test.ts | 165 +++++++++++++++++++ apps/web/lib/auth/identity.ts | 125 ++++++++++++++ apps/web/lib/auth/jwt.test.ts | 226 ++++++++++++++++++++++++++ apps/web/lib/auth/jwt.ts | 133 +++++++++++++-- apps/web/lib/auth/sync-groups.test.ts | 126 ++++++++++++++ apps/web/lib/auth/sync-groups.ts | 85 ++++++++-- apps/web/lib/db/schema.ts | 20 +++ apps/web/lib/mcp/context.ts | 48 ++---- 10 files changed, 920 insertions(+), 82 deletions(-) create mode 100644 apps/web/drizzle/0005_user_oid.sql create mode 100644 apps/web/lib/auth/identity.test.ts create mode 100644 apps/web/lib/auth/identity.ts create mode 100644 apps/web/lib/auth/jwt.test.ts create mode 100644 apps/web/lib/auth/sync-groups.test.ts diff --git a/apps/web/auth.ts b/apps/web/auth.ts index 6dbb286..0dfce13 100644 --- a/apps/web/auth.ts +++ b/apps/web/auth.ts @@ -1,7 +1,6 @@ import NextAuth from "next-auth"; import { env } from "@/lib/env"; -import { db } from "@/lib/db/client"; -import { users } from "@/lib/db/schema"; +import { oidClaim, resolveUserId } from "@/lib/auth/identity"; import { syncUserGroupsFromClaim } from "@/lib/auth/sync-groups"; /** @@ -41,27 +40,18 @@ export const { auth, handlers, signIn, signOut } = NextAuth({ const iss = (profile.iss as string | undefined) ?? env().OIDC_ISSUER; if (!sub) throw new Error("OIDC profile missing `sub` claim"); - const row = await db - .insert(users) - .values({ - oidcSub: sub, - oidcIss: iss, - email: profile.email ?? null, - name: profile.name ?? null, - picture: (profile.picture as string | undefined) ?? null, - }) - .onConflictDoUpdate({ - target: [users.oidcIss, users.oidcSub], - set: { - email: profile.email ?? null, - name: profile.name ?? null, - picture: (profile.picture as string | undefined) ?? null, - lastSeenAt: new Date(), - }, - }) - .returning({ id: users.id }); + // Shared with the MCP path (lib/mcp/context.ts). On EntraID the `oid` + // claim is what keeps the two surfaces resolving to one account — + // `sub` differs per app registration there. See lib/auth/identity.ts. + const userId = await resolveUserId({ + iss, + sub, + oid: oidClaim(profile), + email: profile.email ?? null, + name: profile.name ?? null, + picture: (profile.picture as string | undefined) ?? null, + }); - const userId = row[0]?.id; token.userId = userId; token.sub = sub; token.iss = iss; @@ -71,10 +61,15 @@ export const { auth, handlers, signIn, signOut } = NextAuth({ // wipes the user's existing memberships, which is the conservative // choice (don't keep stale grants alive if the IdP stopped // asserting them). + // + // The whole profile goes in, not just `profile.groups`: an absent + // claim means one thing on its own and something else entirely next + // to EntraID's overage markers, and only the second case must abort. + // A GroupsOverageError thrown here fails the sign-in, which is the + // intent — it leaves the user's existing memberships untouched + // instead of silently deleting them. if (userId) { - // `profile.groups` is untyped at the next-auth boundary — coerce. - const claimGroups = (profile as { groups?: unknown }).groups; - await syncUserGroupsFromClaim(userId, iss, claimGroups); + await syncUserGroupsFromClaim(userId, iss, profile); } } return token; diff --git a/apps/web/drizzle/0005_user_oid.sql b/apps/web/drizzle/0005_user_oid.sql new file mode 100644 index 0000000..ab9d29c --- /dev/null +++ b/apps/web/drizzle/0005_user_oid.sql @@ -0,0 +1,29 @@ +-- EntraID identity: key users on `oid` when the IdP emits it. +-- +-- EntraID's `sub` is a PAIRWISE identifier — derived from the token recipient, +-- so the Web UI app registration and the MCP app registration hand out +-- different `sub` values for the same person. Both `auth.ts` and +-- `lib/mcp/context.ts` upsert on (oidc_iss, oidc_sub), so on EntraID one human +-- resolves to two rows: they sign into the Web UI, connect an MCP client, and +-- land in an empty account with their memories nowhere to be seen. Nothing +-- errors, which is what makes it dangerous. +-- +-- `oid` is the user's directory object id, which Microsoft documents as +-- constant for a user across every application in a tenant. Recording it gives +-- us a key that holds across both surfaces. +-- +-- Nullable on purpose: Authentik, Keycloak and Okta emit no `oid`, and there +-- `sub` is already application-independent. Those deployments keep using +-- (oidc_iss, oidc_sub) and are untouched by this migration. + +ALTER TABLE "users" ADD COLUMN "oidc_oid" text; + +-- Partial index. Every non-EntraID row holds NULL here; a plain unique index +-- would treat those as colliding and permit exactly one such user. +CREATE UNIQUE INDEX "users_iss_oid_uq" + ON "users" ("oidc_iss", "oidc_oid") + WHERE "oidc_oid" IS NOT NULL; + +-- No backfill. `oid` is only knowable from a token, so existing rows adopt +-- theirs on the owner's next sign-in (see `adoptLegacyRow` in +-- lib/auth/identity.ts). Backfilling would mean guessing. diff --git a/apps/web/lib/auth/identity.test.ts b/apps/web/lib/auth/identity.test.ts new file mode 100644 index 0000000..8cbb57d --- /dev/null +++ b/apps/web/lib/auth/identity.test.ts @@ -0,0 +1,165 @@ +import { afterAll, beforeEach, describe, expect, test } from "vitest"; + +/** + * Identity resolution across the two surfaces. + * + * The bug these guard against is silent: on EntraID the Web UI and the MCP + * endpoint see different `sub` values for the same person, both code paths + * UPSERT rather than fail, and the result is two accounts — the user signs in, + * connects an MCP client, and finds their memories gone. Nothing errors, so + * only a test that asserts "same person ⇒ same row id" catches it. + */ +const { db, pg } = await import("@/lib/db/client"); +const { users } = await import("@/lib/db/schema"); +const { resolveUserId, oidClaim } = await import("@/lib/auth/identity"); +const { eq } = await import("drizzle-orm"); + +/** `noUncheckedIndexedAccess` is on; narrow once rather than at every use. */ +function first(rows: T[]): T { + const row = rows[0]; + if (!row) throw new Error("expected at least one row"); + return row; +} + +const ISS = "https://login.microsoftonline.com/test-tenant/v2.0"; + +/** Distinct `sub` values, as EntraID's pairwise identifiers would be. */ +const WEB_SUB = "pairwise-sub-for-web-registration"; +const MCP_SUB = "pairwise-sub-for-mcp-registration"; +const OID = "00000000-1111-2222-3333-444444444444"; + +function claims(overrides: Record = {}) { + return { + iss: ISS, + sub: WEB_SUB, + oid: OID, + email: "person@example.com", + name: "Person", + picture: null, + ...overrides, + } as Parameters[0]; +} + +beforeEach(async () => { + await db.delete(users).where(eq(users.oidcIss, ISS)); +}); + +afterAll(async () => { + await db.delete(users).where(eq(users.oidcIss, ISS)); + await pg.end(); +}); + +describe("oidClaim", () => { + test("reads a string oid", () => { + expect(oidClaim({ oid: OID })).toBe(OID); + }); + + test("is null when absent, blank, or not a string", () => { + expect(oidClaim({})).toBeNull(); + expect(oidClaim({ oid: " " })).toBeNull(); + expect(oidClaim({ oid: 42 })).toBeNull(); + expect(oidClaim(null)).toBeNull(); + }); +}); + +describe("resolveUserId with an oid (EntraID)", () => { + test("both surfaces resolve to ONE row despite different subs", async () => { + const fromWeb = await resolveUserId(claims({ sub: WEB_SUB })); + const fromMcp = await resolveUserId(claims({ sub: MCP_SUB })); + + expect(fromMcp).toBe(fromWeb); + + const rows = await db.select().from(users).where(eq(users.oidcIss, ISS)); + expect(rows).toHaveLength(1); + }); + + test("does not rewrite oidc_sub once the row exists", async () => { + await resolveUserId(claims({ sub: WEB_SUB })); + await resolveUserId(claims({ sub: MCP_SUB })); + + const rows = await db.select().from(users).where(eq(users.oidcIss, ISS)); + // Whichever arrived first stays put; flip-flopping it on every request + // could collide with the (iss, sub) unique index. + expect(first(rows).oidcSub).toBe(WEB_SUB); + expect(first(rows).oidcOid).toBe(OID); + }); + + test("adopts a pre-migration row instead of stranding it", async () => { + // A deployment that signed this person in before 0005 ran: correct sub, + // no oid recorded. + const legacy = first( + await db + .insert(users) + .values({ oidcIss: ISS, oidcSub: WEB_SUB, email: "old@example.com" }) + .returning({ id: users.id }), + ); + + const resolved = await resolveUserId(claims({ sub: WEB_SUB })); + + expect(resolved).toBe(legacy.id); + const rows = await db.select().from(users).where(eq(users.oidcIss, ISS)); + expect(rows).toHaveLength(1); + expect(first(rows).oidcOid).toBe(OID); + }); + + test("refreshes profile fields on an existing row", async () => { + await resolveUserId(claims({ name: "Old Name" })); + await resolveUserId(claims({ sub: MCP_SUB, name: "New Name" })); + + const rows = await db.select().from(users).where(eq(users.oidcIss, ISS)); + expect(first(rows).name).toBe("New Name"); + }); + + test("concurrent first-contact from both surfaces yields one row", async () => { + const [a, b] = await Promise.all([ + resolveUserId(claims({ sub: WEB_SUB })), + resolveUserId(claims({ sub: MCP_SUB })), + ]); + + expect(a).toBe(b); + const rows = await db.select().from(users).where(eq(users.oidcIss, ISS)); + expect(rows).toHaveLength(1); + }); + + test("different people in one tenant stay separate", async () => { + const one = await resolveUserId(claims()); + const two = await resolveUserId( + claims({ sub: "other-sub", oid: "99999999-1111-2222-3333-444444444444" }), + ); + + expect(two).not.toBe(one); + const rows = await db.select().from(users).where(eq(users.oidcIss, ISS)); + expect(rows).toHaveLength(2); + }); +}); + +describe("resolveUserId without an oid (Authentik and friends)", () => { + test("keys on (iss, sub) exactly as before", async () => { + const initial = await resolveUserId(claims({ oid: null })); + const again = await resolveUserId(claims({ oid: null, name: "Renamed" })); + + expect(again).toBe(initial); + const rows = await db.select().from(users).where(eq(users.oidcIss, ISS)); + expect(rows).toHaveLength(1); + expect(first(rows).oidcOid).toBeNull(); + expect(first(rows).name).toBe("Renamed"); + }); + + test("distinct subs are distinct people", async () => { + await resolveUserId(claims({ oid: null, sub: "a" })); + await resolveUserId(claims({ oid: null, sub: "b" })); + + const rows = await db.select().from(users).where(eq(users.oidcIss, ISS)); + expect(rows).toHaveLength(2); + }); + + test("several oid-less users coexist — the unique index is partial", async () => { + // A non-partial unique index on (iss, oid) would allow exactly one NULL + // pair and reject everyone after the first. + for (const sub of ["u1", "u2", "u3"]) { + await resolveUserId(claims({ oid: null, sub })); + } + const rows = await db.select().from(users).where(eq(users.oidcIss, ISS)); + expect(rows).toHaveLength(3); + }); +}); diff --git a/apps/web/lib/auth/identity.ts b/apps/web/lib/auth/identity.ts new file mode 100644 index 0000000..eacf6a1 --- /dev/null +++ b/apps/web/lib/auth/identity.ts @@ -0,0 +1,125 @@ +import { and, eq, isNull } from "drizzle-orm"; +import { db } from "@/lib/db/client"; +import { users } from "@/lib/db/schema"; + +/** + * Resolving OIDC claims to the internal `users.id`. + * + * Both surfaces come through here — the Web UI (`auth.ts`) and the MCP + * endpoint (`lib/mcp/context.ts`) — and that is the point. They each used to + * carry their own copy of this upsert, which is how the two drifted into + * disagreeing about who a user is. + * + * ## Why `oid` exists here + * + * Authentik's `sub` is `user.uid`, identical across every provider, so + * (iss, sub) identifies a person. EntraID's `sub` is PAIRWISE: Microsoft + * derives it from the token recipient, so the Web UI app registration and the + * MCP app registration emit different `sub` values for the same human. Keyed + * on `sub`, that person gets two rows — they sign into the Web UI, connect + * Claude Code, and find an empty account. Both paths upsert, so nothing + * errors; the split is completely silent. + * + * `oid` is the directory object id, which Microsoft documents as constant for + * a user across every application in a tenant. When it's present it wins. + * When it's absent (Authentik, Keycloak, Okta) behaviour is exactly as before. + */ + +/** Extract a usable EntraID `oid` claim, or null on IdPs that don't emit one. */ +export function oidClaim(claims: unknown): string | null { + const raw = (claims as { oid?: unknown } | null | undefined)?.oid; + if (typeof raw !== "string") return null; + const trimmed = raw.trim(); + return trimmed.length > 0 ? trimmed : null; +} + +export interface IdentityInput { + iss: string; + sub: string; + /** EntraID object id, or null. */ + oid: string | null; + email: string | null; + name: string | null; + picture: string | null; +} + +/** Postgres unique-violation. */ +function isUniqueViolation(err: unknown): boolean { + return (err as { code?: unknown } | null)?.code === "23505"; +} + +/** + * Resolve (creating if needed) the `users` row for a set of verified claims. + * + * Resolution order when `oid` is present: + * + * 1. a row already keyed on this `oid` — the steady state + * 2. a pre-`oid` row for the same (iss, sub), which gets its `oid` + * backfilled in place. This is how a deployment that ran before the + * 0005 migration keeps its accounts instead of stranding them. + * 3. insert + * + * Without `oid` this collapses to the original (iss, sub) upsert. + */ +export async function resolveUserId(input: IdentityInput): Promise { + const { iss, sub, oid, email, name, picture } = input; + const profile = { email, name, picture, lastSeenAt: new Date() }; + + if (oid) { + // 1. Steady state. + // + // Deliberately does NOT touch `oidc_sub`. The stored value is whichever + // app registration this person first arrived through; rewriting it on + // every request would flip it back and forth between the Web and MCP + // values and could collide with the (iss, sub) unique index. + const byOid = await db + .update(users) + .set(profile) + .where(and(eq(users.oidcIss, iss), eq(users.oidcOid, oid))) + .returning({ id: users.id }); + if (byOid[0]) return byOid[0].id; + + // 2. Adopt a row created before `oid` was recorded. + const adopted = await db + .update(users) + .set({ ...profile, oidcOid: oid }) + .where( + and(eq(users.oidcIss, iss), eq(users.oidcSub, sub), isNull(users.oidcOid)), + ) + .returning({ id: users.id }); + if (adopted[0]) return adopted[0].id; + } + + // 3. Insert. The conflict target stays (iss, sub) because that is the index + // every row has; a concurrent writer racing us on `oid` instead is caught + // below. + try { + const inserted = await db + .insert(users) + .values({ oidcIss: iss, oidcSub: sub, oidcOid: oid, email, name, picture }) + .onConflictDoUpdate({ + target: [users.oidcIss, users.oidcSub], + set: profile, + }) + .returning({ id: users.id }); + if (inserted[0]) return inserted[0].id; + } catch (err) { + // Two requests for the same person arriving together through DIFFERENT + // app registrations: same `oid`, different `sub`, so the (iss, sub) + // conflict target doesn't fire and the partial (iss, oid) index rejects + // the loser. Fall through and read the winner's row. + if (!isUniqueViolation(err)) throw err; + } + + const existing = await db + .select({ id: users.id }) + .from(users) + .where( + oid + ? and(eq(users.oidcIss, iss), eq(users.oidcOid, oid)) + : and(eq(users.oidcIss, iss), eq(users.oidcSub, sub)), + ) + .limit(1); + if (!existing[0]) throw new Error("user upsert failed and not found on re-read"); + return existing[0].id; +} diff --git a/apps/web/lib/auth/jwt.test.ts b/apps/web/lib/auth/jwt.test.ts new file mode 100644 index 0000000..5105e21 --- /dev/null +++ b/apps/web/lib/auth/jwt.test.ts @@ -0,0 +1,226 @@ +import { createServer, type Server } from "node:http"; +import { AddressInfo } from "node:net"; +import { SignJWT, exportJWK, generateKeyPair } from "jose"; +import type { JWK, KeyLike } from "jose"; +import { afterEach, beforeEach, describe, expect, test, vi } from "vitest"; + +/** + * The JWKS location is discovered, not assumed. + * + * `${issuer}/jwks/` used to be hardcoded here. That is an Authentik + * convention — EntraID serves its keys at + * `https://login.microsoftonline.com/{tenant}/discovery/v2.0/keys`, so on + * EntraID the hardcoded path 404s and NO MCP token can ever verify. These + * tests pin the three behaviours that make the discovery path safe to ship: + * discovery is honoured, failure degrades to the old path rather than to a + * broken deployment, and it happens once rather than per request. + * + * A real loopback HTTP server is used rather than a `fetch` mock because jose + * fetches the key set through `node:http` directly, not through global + * `fetch` — a mocked `fetch` would silently never be consulted for the JWKS + * request, and the assertion about *which* URL was used would prove nothing. + */ + +const TENANT = "11111111-2222-3333-4444-555555555555"; +const AUDIENCE = "99999999-8888-7777-6666-555555555555"; + +/** Paths the fake IdP was asked for, in order. */ +let requested: string[] = []; +/** Response the fake IdP gives for the discovery document. */ +let discoveryResponse: { status: number; body: string }; +let server: Server; +let origin: string; +let privateKey: KeyLike; +let publicJwk: JWK; + +/** Authentik-shaped issuer: application-scoped path, trailing slash. */ +function authentikIssuer(): string { + return `${origin}/application/o/shared-memory-mcp/`; +} + +/** EntraID-shaped issuer: tenant-scoped, no trailing slash. */ +function entraIssuer(): string { + return `${origin}/${TENANT}/v2.0`; +} + +/** Where an EntraID discovery document points for keys. */ +function entraKeysPath(): string { + return `/${TENANT}/discovery/v2.0/keys`; +} + +beforeEach(async () => { + const pair = await generateKeyPair("RS256"); + privateKey = pair.privateKey; + publicJwk = { ...(await exportJWK(pair.publicKey)), kid: "test-key", alg: "RS256", use: "sig" }; + requested = []; + + server = createServer((req, res) => { + requested.push(req.url ?? ""); + if (req.url?.endsWith("/.well-known/openid-configuration")) { + res.writeHead(discoveryResponse.status, { "content-type": "application/json" }); + res.end(discoveryResponse.body); + return; + } + // Every other path is treated as a key set endpoint. Which path the + // request actually arrived on is the thing under test. + res.writeHead(200, { "content-type": "application/json" }); + res.end(JSON.stringify({ keys: [publicJwk] })); + }); + + await new Promise((resolve) => server.listen(0, "127.0.0.1", resolve)); + origin = `http://127.0.0.1:${(server.address() as AddressInfo).port}`; +}); + +afterEach(async () => { + // The key set is memoised on globalThis, which survives vi.resetModules(). + // Without clearing it, the second test in this file would silently reuse + // the first test's resolution and assert nothing. + delete (globalThis as Record).__sharedMemoryJwks; + delete (globalThis as Record).__sharedMemoryJwksRetryAt; + delete process.env.OIDC_ISSUER_MCP; + delete process.env.OIDC_AUDIENCE; + await new Promise((resolve) => server.close(() => resolve())); +}); + +/** + * Import `authenticateBearer` fresh so it observes the env vars this test set + * (lib/env.ts caches its parse in a module singleton). + */ +async function loadAuthenticateBearer() { + vi.resetModules(); + const mod = await import("@/lib/auth/jwt"); + return mod.authenticateBearer; +} + +async function signAccessToken(issuer: string): Promise { + return new SignJWT({ groups: ["memory-users"] }) + .setProtectedHeader({ alg: "RS256", kid: "test-key" }) + .setIssuer(issuer) + .setAudience(AUDIENCE) + .setSubject("user-object-id") + .setIssuedAt() + .setExpirationTime("5m") + .sign(privateKey); +} + +function discoveryPathsSeen(): string[] { + return requested.filter((p) => p.endsWith("/.well-known/openid-configuration")); +} + +describe("MCP JWKS resolution", () => { + test("uses the jwks_uri from discovery, so EntraID's key endpoint is reached", async () => { + process.env.OIDC_ISSUER_MCP = entraIssuer(); + process.env.OIDC_AUDIENCE = AUDIENCE; + discoveryResponse = { + status: 200, + // Shape of https://login.microsoftonline.com/{tenant}/v2.0/.well-known/openid-configuration + body: JSON.stringify({ + issuer: entraIssuer(), + jwks_uri: `${origin}${entraKeysPath()}`, + token_endpoint: `${origin}/${TENANT}/oauth2/v2.0/token`, + }), + }; + + const authenticateBearer = await loadAuthenticateBearer(); + const claims = await authenticateBearer(`Bearer ${await signAccessToken(entraIssuer())}`); + + expect(claims.sub).toBe("user-object-id"); + expect(requested).toContain(entraKeysPath()); + // The Authentik convention must NOT have been tried. + expect(requested).not.toContain(`/${TENANT}/v2.0/jwks/`); + }); + + test("tolerates a trailing slash on the issuer while still honouring discovery", async () => { + // acceptedIssuers() takes both spellings; discovery must not regress that. + process.env.OIDC_ISSUER_MCP = `${entraIssuer()}/`; + process.env.OIDC_AUDIENCE = AUDIENCE; + discoveryResponse = { + status: 200, + body: JSON.stringify({ jwks_uri: `${origin}${entraKeysPath()}` }), + }; + + const authenticateBearer = await loadAuthenticateBearer(); + // Token carries the un-slashed spelling; the env var carries the slashed one. + const claims = await authenticateBearer(`Bearer ${await signAccessToken(entraIssuer())}`); + + expect(claims.sub).toBe("user-object-id"); + expect(requested).toContain(entraKeysPath()); + }); + + test("falls back to ${issuer}/jwks/ when discovery is unreachable", async () => { + process.env.OIDC_ISSUER_MCP = authentikIssuer(); + process.env.OIDC_AUDIENCE = AUDIENCE; + discoveryResponse = { status: 500, body: "upstream exploded" }; + + const authenticateBearer = await loadAuthenticateBearer(); + const claims = await authenticateBearer(`Bearer ${await signAccessToken(authentikIssuer())}`); + + // Existing Authentik deployments keep working with no discovery document. + expect(claims.sub).toBe("user-object-id"); + expect(requested).toContain("/application/o/shared-memory-mcp/jwks/"); + }); + + test("falls back to ${issuer}/jwks/ when discovery omits jwks_uri", async () => { + process.env.OIDC_ISSUER_MCP = authentikIssuer(); + process.env.OIDC_AUDIENCE = AUDIENCE; + // 200 OK, valid JSON, no usable key set pointer — the malformed case that + // a naive `doc.jwks_uri` read would turn into `new URL(undefined)`. + discoveryResponse = { status: 200, body: JSON.stringify({ issuer: authentikIssuer() }) }; + + const authenticateBearer = await loadAuthenticateBearer(); + const claims = await authenticateBearer(`Bearer ${await signAccessToken(authentikIssuer())}`); + + expect(claims.sub).toBe("user-object-id"); + expect(requested).toContain("/application/o/shared-memory-mcp/jwks/"); + }); + + test("falls back when discovery returns non-JSON", async () => { + process.env.OIDC_ISSUER_MCP = authentikIssuer(); + process.env.OIDC_AUDIENCE = AUDIENCE; + discoveryResponse = { status: 200, body: "login page" }; + + const authenticateBearer = await loadAuthenticateBearer(); + const claims = await authenticateBearer(`Bearer ${await signAccessToken(authentikIssuer())}`); + + expect(claims.sub).toBe("user-object-id"); + expect(requested).toContain("/application/o/shared-memory-mcp/jwks/"); + }); + + test("discovers once across many verifications, not once per request", async () => { + // The MCP endpoint verifies a token on essentially every request. A + // discovery fetch per request would add a round-trip to every tool call. + process.env.OIDC_ISSUER_MCP = entraIssuer(); + process.env.OIDC_AUDIENCE = AUDIENCE; + discoveryResponse = { + status: 200, + body: JSON.stringify({ jwks_uri: `${origin}${entraKeysPath()}` }), + }; + + const authenticateBearer = await loadAuthenticateBearer(); + for (let i = 0; i < 3; i++) { + await authenticateBearer(`Bearer ${await signAccessToken(entraIssuer())}`); + } + + expect(discoveryPathsSeen()).toHaveLength(1); + }); + + test("concurrent cold requests share a single discovery fetch", async () => { + // Caching the settled value rather than the in-flight promise would let + // every request that arrives before the first one resolves start its own + // discovery fetch — a thundering herd at process start. + process.env.OIDC_ISSUER_MCP = entraIssuer(); + process.env.OIDC_AUDIENCE = AUDIENCE; + discoveryResponse = { + status: 200, + body: JSON.stringify({ jwks_uri: `${origin}${entraKeysPath()}` }), + }; + + const authenticateBearer = await loadAuthenticateBearer(); + const token = await signAccessToken(entraIssuer()); + await Promise.all( + Array.from({ length: 5 }, () => authenticateBearer(`Bearer ${token}`)), + ); + + expect(discoveryPathsSeen()).toHaveLength(1); + }); +}); diff --git a/apps/web/lib/auth/jwt.ts b/apps/web/lib/auth/jwt.ts index 3b04423..f404c34 100644 --- a/apps/web/lib/auth/jwt.ts +++ b/apps/web/lib/auth/jwt.ts @@ -2,13 +2,14 @@ import { createRemoteJWKSet, jwtVerify, errors as joseErrors } from "jose"; import type { JWTPayload } from "jose"; import { env } from "@/lib/env"; import { CLI_TOKEN_KID, tokenKid, verifyCliToken } from "./cli-token"; +import { detectGroupsOverage } from "./sync-groups"; /** * Authenticates a bearer token presented to the MCP endpoint. Two token * kinds are accepted, dispatched by the JWT `kid` header: * - * - Authentik-issued OIDC access tokens (any kid) — verified against - * Authentik's JWKS over the network. + * - IdP-issued OIDC access tokens (any kid) — verified against the + * issuer's JWKS over the network, located via OIDC discovery. * - CLI tokens minted at /connect (kid="cli-v1") — verified locally * with the HMAC CLI_TOKEN_SECRET. * @@ -18,8 +19,25 @@ import { CLI_TOKEN_KID, tokenKid, verifyCliToken } from "./cli-token"; * This is distinct from the NextAuth session cookie path used by the Web UI. */ +type JwkSet = ReturnType; + type GlobalWithJwks = typeof globalThis & { - __sharedMemoryJwks?: ReturnType; + /** + * Resolved key set, cached as a *promise* rather than a value. + * + * Resolution now involves a network round-trip (OIDC discovery), and the + * MCP endpoint verifies a token on essentially every request. Caching the + * settled value would leave a window in which N concurrent cold requests + * each start their own discovery fetch; caching the in-flight promise means + * the first caller does the work and everyone else awaits the same result. + */ + __sharedMemoryJwks?: Promise; + /** + * Epoch ms after which discovery should be re-attempted, set only when we + * had to fall back (see `jwks()`). Undefined means the cached set came from + * a successful discovery and is good indefinitely. + */ + __sharedMemoryJwksRetryAt?: number; }; const g = globalThis as GlobalWithJwks; @@ -27,6 +45,10 @@ const g = globalThis as GlobalWithJwks; * Issuer of MCP access tokens. The MCP endpoint is a separate application in * the IdP from the Web UI, and Authentik stamps each token with its own * application slug, so this is NOT interchangeable with OIDC_ISSUER. + * + * Not every IdP works that way: EntraID has one issuer per tenant regardless + * of how many app registrations you create, so OIDC_ISSUER_MCP is left unset + * there and this falls through to OIDC_ISSUER. */ export function mcpIssuer(): string { return env().OIDC_ISSUER_MCP ?? env().OIDC_ISSUER; @@ -50,16 +72,88 @@ function acceptedIssuers(): [string, string] { return [bare, `${bare}/`]; } -function jwks() { - if (g.__sharedMemoryJwks) return g.__sharedMemoryJwks; - // Authentik discovery is at `${issuer}/.well-known/openid-configuration`; - // the JWKS URI is normally `${issuer}/jwks/` or `${issuer}/.well-known/jwks.json`. - // Authentik canonically serves `${issuer}/jwks/`. - const url = new URL(`${mcpIssuer().replace(/\/$/, "")}/jwks/`); - g.__sharedMemoryJwks = createRemoteJWKSet(url, { - cacheMaxAge: 10 * 60 * 1000, // 10 min - cooldownDuration: 30 * 1000, - }); +const JWKS_OPTIONS = { + cacheMaxAge: 10 * 60 * 1000, // 10 min + cooldownDuration: 30 * 1000, +} as const; + +/** How long to keep serving a fallback key set before retrying discovery. */ +const DISCOVERY_RETRY_COOLDOWN_MS = 60 * 1000; + +/** Discovery can hang; every MCP request waits on it, so bound it. */ +const DISCOVERY_TIMEOUT_MS = 5 * 1000; + +/** + * The pre-discovery convention: `${issuer}/jwks/`. + * + * This is Authentik's canonical JWKS path and was hardcoded here. It stays as + * the fallback so that a deployment whose discovery document is unreachable + * behaves exactly as it did before this change. + */ +function fallbackJwksUri(): string { + return `${mcpIssuer().replace(/\/$/, "")}/jwks/`; +} + +/** + * Read `jwks_uri` out of the MCP issuer's OIDC discovery document. + * + * `${issuer}/jwks/` is an Authentik convention, not a standard — RFC 8414 + * says the key set lives wherever `jwks_uri` points, and providers disagree + * wildly. EntraID serves keys at + * `https://login.microsoftonline.com/{tenant}/discovery/v2.0/keys`, nowhere + * near `${issuer}/jwks/`, so with the path hardcoded every EntraID-issued MCP + * token fails verification with a 404 on the key set — authentication is + * simply impossible, not merely misconfigured. Ask the issuer where its keys + * are instead of guessing. + * + * Returns null (never throws) on any failure, so the caller can fall back. + */ +async function discoverJwksUri(): Promise { + const url = `${mcpIssuer().replace(/\/$/, "")}/.well-known/openid-configuration`; + try { + const res = await fetch(url, { + headers: { accept: "application/json" }, + signal: AbortSignal.timeout(DISCOVERY_TIMEOUT_MS), + }); + if (!res.ok) return null; + const doc: unknown = await res.json(); + const uri = (doc as { jwks_uri?: unknown } | null)?.jwks_uri; + if (typeof uri !== "string" || uri.trim().length === 0) return null; + // A malformed jwks_uri must not blow up the request path. + new URL(uri); + return uri; + } catch { + return null; + } +} + +/** + * The key set MCP access tokens are verified against, resolved once per + * process. + * + * Async because discovery is a network call. The cached promise is installed + * synchronously — before the first `await` inside the IIFE runs — so + * concurrent callers always join the existing resolution rather than racing + * to start their own. + * + * When discovery fails we serve the legacy fallback but arm a retry: a single + * blip at process start would otherwise pin the wrong URL for the lifetime of + * the container, which on EntraID means MCP auth stays broken until someone + * restarts it. The cooldown keeps a persistently-unreachable discovery + * endpoint from being hit on every request. + */ +function jwks(): Promise { + const retryAt = g.__sharedMemoryJwksRetryAt; + const dueForRetry = retryAt !== undefined && Date.now() >= retryAt; + if (g.__sharedMemoryJwks && !dueForRetry) return g.__sharedMemoryJwks; + + g.__sharedMemoryJwksRetryAt = undefined; + g.__sharedMemoryJwks = (async () => { + const discovered = await discoverJwksUri(); + if (discovered) return createRemoteJWKSet(new URL(discovered), JWKS_OPTIONS); + g.__sharedMemoryJwksRetryAt = Date.now() + DISCOVERY_RETRY_COOLDOWN_MS; + return createRemoteJWKSet(new URL(fallbackJwksUri()), JWKS_OPTIONS); + })(); return g.__sharedMemoryJwks; } @@ -143,7 +237,7 @@ export async function authenticateBearer(authHeader: string | null): Promise { + const rows = await db + .select({ name: groups.name }) + .from(userGroups) + .innerJoin(groups, eq(userGroups.groupId, groups.id)) + .where(eq(userGroups.userId, userId)); + return rows.map((r) => r.name).sort(); +} + +beforeEach(async () => { + await db.delete(users).where(eq(users.oidcIss, ISS)); + await db.delete(groups).where(eq(groups.oidcIss, ISS)); + const rows = await db + .insert(users) + .values({ oidcIss: ISS, oidcSub: "sync-test-sub" }) + .returning({ id: users.id }); + const row = rows[0]; + if (!row) throw new Error("failed to seed test user"); + userId = row.id; +}); + +afterAll(async () => { + await db.delete(users).where(eq(users.oidcIss, ISS)); + await db.delete(groups).where(eq(groups.oidcIss, ISS)); + await pg.end(); +}); + +describe("detectGroupsOverage", () => { + test("spots the JWT overage pointer", () => { + expect( + detectGroupsOverage({ + _claim_names: { groups: "src1" }, + _claim_sources: { src1: { endpoint: "https://graph.windows.net/x" } }, + }), + ).toBe(true); + }); + + test("spots the implicit-flow indicator", () => { + expect(detectGroupsOverage({ hasgroups: true })).toBe(true); + }); + + test("is false for ordinary claims", () => { + expect(detectGroupsOverage({ groups: ["a"] })).toBe(false); + expect(detectGroupsOverage({})).toBe(false); + expect(detectGroupsOverage(null)).toBe(false); + // A _claim_names for some OTHER claim is not a groups overage. + expect(detectGroupsOverage({ _claim_names: { roles: "src1" } })).toBe(false); + expect(detectGroupsOverage({ hasgroups: false })).toBe(false); + }); +}); + +describe("syncUserGroupsFromClaim", () => { + test("stores the claim's names", async () => { + await syncUserGroupsFromClaim(userId, ISS, { groups: ["eng", "ops"] }); + expect(await memberships()).toEqual(["eng", "ops"]); + }); + + test("an empty claim really does clear memberships", async () => { + await syncUserGroupsFromClaim(userId, ISS, { groups: ["eng"] }); + await syncUserGroupsFromClaim(userId, ISS, { groups: [] }); + expect(await memberships()).toEqual([]); + }); + + test("an absent claim clears memberships", async () => { + // Unchanged behaviour: the IdP has stopped asserting groups, so we stop + // honouring them rather than keeping stale grants alive. + await syncUserGroupsFromClaim(userId, ISS, { groups: ["eng"] }); + await syncUserGroupsFromClaim(userId, ISS, {}); + expect(await memberships()).toEqual([]); + }); + + test("overage throws instead of clearing", async () => { + await syncUserGroupsFromClaim(userId, ISS, { groups: ["eng", "ops"] }); + + await expect( + syncUserGroupsFromClaim(userId, ISS, { + _claim_names: { groups: "src1" }, + _claim_sources: { src1: { endpoint: "https://graph.windows.net/x" } }, + }), + ).rejects.toBeInstanceOf(GroupsOverageError); + + // The whole point: the snapshot survives, so the operator can fix the IdP + // and the user comes back with their access intact. + expect(await memberships()).toEqual(["eng", "ops"]); + }); + + test("the overage error names the fix", async () => { + let err: Error | null = null; + try { + await syncUserGroupsFromClaim(userId, ISS, { hasgroups: true }); + } catch (e) { + err = e as Error; + } + + expect(err).toBeInstanceOf(GroupsOverageError); + expect(err?.message).toContain("groupMembershipClaims"); + expect(err?.message).toContain("ApplicationGroup"); + }); + + test("non-string entries are dropped, names are de-duplicated", async () => { + await syncUserGroupsFromClaim(userId, ISS, { + groups: ["eng", 7, null, " eng ", "ops"], + }); + expect(await memberships()).toEqual(["eng", "ops"]); + }); +}); diff --git a/apps/web/lib/auth/sync-groups.ts b/apps/web/lib/auth/sync-groups.ts index e3e342c..ec4d906 100644 --- a/apps/web/lib/auth/sync-groups.ts +++ b/apps/web/lib/auth/sync-groups.ts @@ -2,34 +2,99 @@ import { and, eq, notInArray, sql } from "drizzle-orm"; import { db } from "@/lib/db/client"; import { groups, userGroups } from "@/lib/db/schema"; +/** + * Raised when the IdP signals that it holds group memberships it declined to + * enumerate (EntraID's "groups overage"). Callers must abort — see + * `detectGroupsOverage` for why this cannot be treated as "no groups". + */ +export class GroupsOverageError extends Error { + constructor() { + super( + "OIDC groups overage: the identity provider signalled group membership " + + "it did not enumerate, so the user's groups cannot be determined. On " + + "EntraID, set the app registration's `groupMembershipClaims` to " + + '"ApplicationGroup" (portal: "Groups assigned to the application") and ' + + "assign the groups you share projects with. See docs/oidc-entra-id.md " + + "§10b.", + ); + this.name = "GroupsOverageError"; + } +} + +/** + * Does this token say "there are groups, but I'm not listing them"? + * + * EntraID stops emitting `groups` past 200 entries in a JWT (150 in SAML, 5 in + * implicit flow) and substitutes a pointer: + * + * "_claim_names": { "groups": "src1" }, + * "_claim_sources": { "src1": { "endpoint": "https://graph.windows.net/…" } } + * + * or, for implicit flow, `"hasgroups": true`. + * + * This is NOT a truncated list — it is no list at all, and it is materially + * different from "this user belongs to zero groups". Conflating the two is + * what made this dangerous: the absent-claim branch below deletes every one of + * the user's memberships, so a user crossing the 200-group line would silently + * lose access to every shared project on both surfaces, with no error raised + * anywhere. + * + * We refuse instead. Group state gates `readableProjectIds` / `canWriteProject`, + * and granting or revoking access on state we know we don't have is guesswork + * either way. Failing loudly destroys nothing and names its own fix. + */ +export function detectGroupsOverage(claims: unknown): boolean { + const c = claims as + | { _claim_names?: unknown; hasgroups?: unknown } + | null + | undefined; + if (!c || typeof c !== "object") return false; + if (c.hasgroups === true) return true; + const names = c._claim_names; + return ( + typeof names === "object" && + names !== null && + "groups" in (names as Record) + ); +} + /** * Sync a user's group memberships from the OIDC `groups` claim on sign-in. * + * Takes the whole claims object, not just the claim value, because deciding + * what an absent `groups` means requires seeing the overage markers that sit + * beside it. + * * Claim shape: `string[]`. Authentik emits group *names* directly here; - * Keycloak and Okta likewise (with the right mappers configured). EntraID, - * when correctly configured per README, emits names too — but the default - * "groups" optional-claim variant emits object-id GUIDs instead, and if the - * user is in too many groups EntraID switches to a "groups overage" - * indicator (no group list at all). We take the conservative path: + * Keycloak and Okta likewise (with the right mappers configured). EntraID + * emits object-id GUIDs by default — `cloud_displayname` gets you names, but + * only under `groupMembershipClaims: "ApplicationGroup"`, and only for + * directly assigned groups. See docs/oidc-entra-id.md §10a. * * - whatever strings appear in the claim are treated as names verbatim * and stored as-is. If your IdP emits GUIDs, the UI will show GUIDs; * fix it at the IdP layer (we don't attempt resolution). * - if the claim is missing/empty, the user is treated as having zero - * groups and all existing memberships are deleted. - * - groups overage (where EntraID emits `_claim_names.groups` instead of - * `groups`) is not handled in v1 — the user appears as having no - * groups. Documented limit; revisit if it bites someone. + * groups and all existing memberships are deleted. That is the + * conservative reading: don't keep stale grants alive once the IdP has + * stopped asserting them. + * - if the IdP signals an overage, we throw rather than apply either + * reading. See `detectGroupsOverage`. * * The whole operation runs in a single transaction so the membership * snapshot is atomic (no window where a user partially has new memberships * and still has stale ones). + * + * @throws {GroupsOverageError} when the claims carry an overage indicator. */ export async function syncUserGroupsFromClaim( userId: string, oidcIss: string, - rawClaim: unknown, + claims: unknown, ): Promise { + if (detectGroupsOverage(claims)) throw new GroupsOverageError(); + + const rawClaim = (claims as { groups?: unknown } | null | undefined)?.groups; const names = normalizeGroupsClaim(rawClaim); await db.transaction(async (tx) => { diff --git a/apps/web/lib/db/schema.ts b/apps/web/lib/db/schema.ts index 4db9a24..8c298cd 100644 --- a/apps/web/lib/db/schema.ts +++ b/apps/web/lib/db/schema.ts @@ -53,6 +53,21 @@ export const users = pgTable( oidcSub: text("oidc_sub").notNull(), // OIDC `iss` so we can disambiguate if we ever federate. oidcIss: text("oidc_iss").notNull(), + /** + * EntraID `oid` — the user's directory object id. + * + * Null on IdPs that don't emit it (Authentik, Keycloak, Okta), where + * `sub` is already stable across applications and remains the key. + * + * EntraID's `sub` is PAIRWISE: it is derived from the token recipient, + * so the Web UI app registration and the MCP app registration produce + * different `sub` values for the same human. Keying on `sub` there + * silently creates two accounts for one person — sign in on the web, + * connect an MCP client, find an empty account. `oid` is the identifier + * Microsoft documents as constant for a user across every application in + * a tenant, so it takes precedence whenever it's present. + */ + oidcOid: text("oidc_oid"), email: text("email"), name: text("name"), picture: text("picture"), @@ -61,6 +76,11 @@ export const users = pgTable( }, (t) => ({ uniqueIss: uniqueIndex("users_iss_sub_uq").on(t.oidcIss, t.oidcSub), + // Partial: rows from IdPs that emit no `oid` all hold NULL here, and a + // plain unique index would collapse them into a single allowed row. + uniqueOid: uniqueIndex("users_iss_oid_uq") + .on(t.oidcIss, t.oidcOid) + .where(sql`${t.oidcOid} IS NOT NULL`), }), ); diff --git a/apps/web/lib/mcp/context.ts b/apps/web/lib/mcp/context.ts index e0eb6fc..1e7e933 100644 --- a/apps/web/lib/mcp/context.ts +++ b/apps/web/lib/mcp/context.ts @@ -1,6 +1,7 @@ import { db } from "@/lib/db/client"; -import { users, groups, userGroups } from "@/lib/db/schema"; -import { and, eq } from "drizzle-orm"; +import { groups, userGroups } from "@/lib/db/schema"; +import { eq } from "drizzle-orm"; +import { oidClaim, resolveUserId } from "@/lib/auth/identity"; import type { AuthenticatedClaims } from "@/lib/auth/jwt"; /** @@ -55,37 +56,18 @@ export async function userContextFromClaims( const name = (claims.name as string | undefined) ?? null; const picture = (claims.picture as string | undefined) ?? null; - const row = await db - .insert(users) - .values({ - oidcSub: claims.sub, - oidcIss: claims.iss, - email, - name, - picture, - }) - .onConflictDoUpdate({ - target: [users.oidcIss, users.oidcSub], - set: { - email, - name, - picture, - lastSeenAt: new Date(), - }, - }) - .returning({ id: users.id }); - - let userId = row[0]?.id; - if (!userId) { - // Race against another upsert — fall back to a select. - const existing = await db - .select({ id: users.id }) - .from(users) - .where(and(eq(users.oidcIss, claims.iss), eq(users.oidcSub, claims.sub))) - .limit(1); - if (!existing[0]) throw new Error("user upsert failed and not found on re-read"); - userId = existing[0].id; - } + // Shared with the Web UI sign-in path (auth.ts). Keeping one resolver is + // what stops the two surfaces disagreeing about who a user is — on EntraID + // they see different `sub` values for the same person and would otherwise + // each create their own account. See lib/auth/identity.ts. + const userId = await resolveUserId({ + iss: claims.iss, + sub: claims.sub, + oid: oidClaim(claims), + email, + name, + picture, + }); // OIDC bearer tokens carry a `groups` claim (when the IdP is configured to // emit it). CLI tokens never do — they go through verifyCliToken which From 4d6694620a5270bfcbd344a00945ebe295e0e519 Mon Sep 17 00:00:00 2001 From: Josh Knapp Date: Wed, 12 Aug 2026 10:51:39 -0700 Subject: [PATCH 2/2] docs: add the Entra ID provider walkthrough The README's OIDC section is written against Authentik and stays that way; Entra differs enough that inlining it would have doubled a file that is already 32k. The new doc parallels the README's A/B structure so the two are diffable, and leads with the traps, since every one of them surfaces as an opaque 401 rather than as anything resembling its cause: the access token version, the tenant-specific authority, `aud` being the client-ID GUID while the requested scope is an `api://` URI, redirect-URI platform types, and group GUIDs. Sections 7 and 10b document the identity and overage behaviour shipped in the previous commit, including the one upgrade-ordering caveat: an existing Entra deployment should sign a user into the Web UI once before reconnecting their MCP client, or the pre-migration row is stranded. Co-Authored-By: Claude Opus 5 (1M context) --- README.md | 7 + docs/oidc-entra-id.md | 688 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 695 insertions(+) create mode 100644 docs/oidc-entra-id.md diff --git a/README.md b/README.md index 1cfc83f..77ba3ef 100644 --- a/README.md +++ b/README.md @@ -249,6 +249,13 @@ shape is the same on any OIDC provider; the UI labels differ: | Redirect URI list | Provider's "Redirect URIs / Origins" | App's "Redirect URIs" | Client's "Valid Redirect URIs" | | Audience claim | Scope mapping or property mapping | "Expose an API" + scope | Client scope with audience mapper | +> **Using Microsoft Entra ID?** The differences are large enough that Entra +> gets its own walkthrough: **[docs/oidc-entra-id.md](docs/oidc-entra-id.md)**. +> It follows the same A/B structure as the steps below, and covers the +> Entra-specific traps — access token version, tenant-specific authority, +> `aud` vs. scope URI, redirect-URI platform type, group GUIDs and overage — +> which otherwise surface only as opaque 401s. + ### A. Web UI provider **Admin → Applications → Providers → Create → OAuth2/OpenID Provider** diff --git a/docs/oidc-entra-id.md b/docs/oidc-entra-id.md new file mode 100644 index 0000000..1676e59 --- /dev/null +++ b/docs/oidc-entra-id.md @@ -0,0 +1,688 @@ +# OIDC provider setup: Microsoft Entra ID + +Companion to the **OIDC provider setup** section in [`README.md`](../README.md), +which walks through Authentik. The structure here deliberately mirrors it — +app registration A (Web UI), app registration B (MCP resource server), env var +mapping, verification — so the two are diffable. Where Entra genuinely differs +from Authentik, the difference is called out rather than smoothed over. + +Everything below assumes a **single-tenant** deployment (`signInAudience` = +"Accounts in this organizational directory only"). Multitenant is possible but +the `iss` verification in `apps/web/lib/auth/jwt.ts` compares against a fixed +string, so it would need code changes — see [Multitenant](#13-multitenant-is-not-supported) +at the end. + +--- + +## 0. Prerequisite: your build must have JWKS discovery + +**Check this first. Nothing else in this document works without it.** + +Until recently `apps/web/lib/auth/jwt.ts` hardcoded the JWKS location as +`${issuer}/jwks/`. That is an *Authentik* convention, not a standard — RFC 8414 +says the key set lives wherever the discovery document's `jwks_uri` points, and +Entra puts it somewhere else entirely: + +| IdP | JWKS URL | +|---|---| +| Authentik | `https://auth.example.com/application/o//jwks/` | +| Entra ID | `https://login.microsoftonline.com//discovery/v2.0/keys` | + +With the path hardcoded, every Entra-issued MCP access token fails verification +because the key set fetch 404s. There is no configuration that works around it; +MCP authentication is simply impossible. + +The current code resolves `jwks_uri` from +`${OIDC_ISSUER_MCP or OIDC_ISSUER}/.well-known/openid-configuration`, caches the +result for the process lifetime, and falls back to `${issuer}/jwks/` only if +discovery is unreachable (so existing Authentik deployments are untouched). + +Confirm your deployment has it before debugging anything else: + +```bash +# Should return the Entra keys endpoint, not a 404. +curl -s "https://login.microsoftonline.com//v2.0/.well-known/openid-configuration" \ + | jq -r .jwks_uri +# → https://login.microsoftonline.com//discovery/v2.0/keys +``` + +If MCP calls 401 with `error_description="verification failed"` and your app +logs show a fetch to `.../v2.0/jwks/`, you are on an older build. + +--- + +## 1. Concepts, Authentik → Entra + +| Concept here | Authentik | Entra ID | +|---|---|---| +| OAuth2 client | Provider + Application | App registration | +| Issuer | Per-application (`.../application/o//`) | **Per-tenant only** — one issuer for the whole directory | +| Audience claim | Scope mapping returning `{"aud": …}` | "Expose an API" scope on the resource app; `aud` is set automatically | +| Redirect URI matching | Regex allowed (any port) | **Exact string match**, with one loopback exception | +| Dynamic client registration | Not implemented | Not implemented | + +The **issuer** row is the one that reshapes the setup. On Authentik, the Web UI +and MCP endpoint are separate applications with separate issuers, which is why +`OIDC_ISSUER_MCP` exists. Entra has exactly one issuer per tenant no matter how +many app registrations you create, so **`OIDC_ISSUER_MCP` is left unset on +Entra** and `mcpIssuer()` falls through to `OIDC_ISSUER`. + +You still create **two app registrations**, for the same reason as on Authentik: +one confidential client for the browser sign-in, one resource server that owns +the audience the MCP endpoint validates. (A third participant — the *public +PKCE client* Claude Code uses — is covered in §4; you can fold it into +registration B or split it out.) + +--- + +## 2. Find your tenant ID and use it explicitly + +Everywhere below, `` is your directory (tenant) GUID, from +**Entra admin center → Overview → Tenant ID**. + +**Do not use the `common` or `organizations` authority.** Their discovery +documents return a *templated* issuer — the literal string, verified live: + +```bash +curl -s https://login.microsoftonline.com/common/v2.0/.well-known/openid-configuration | jq -r .issuer +# → https://login.microsoftonline.com/{tenantid}/v2.0 +``` + +That `{tenantid}` is not a formatting artifact; it is what the endpoint really +returns. `jwt.ts` compares `iss` by exact string (via `acceptedIssuers()`), so +against a templated issuer **no token can ever match** and every MCP call fails +with `claim invalid: iss`. + +Microsoft's documented pattern for multitenant apps is to substitute the token's +`tid` claim into the placeholder and then compare — this app does not do that +(see [Multitenant](#13-multitenant-is-not-supported)). For single-tenant, the fix +is simply to use the tenant-specific authority, whose discovery document +returns a concrete issuer: + +```bash +curl -s "https://login.microsoftonline.com//v2.0/.well-known/openid-configuration" | jq -r .issuer +# → https://login.microsoftonline.com//v2.0 (no trailing slash) +``` + +A verified domain (`contoso.onmicrosoft.com`) also works as the authority +segment — Entra resolves it server-side and returns the GUID form in both +`issuer` and `jwks_uri`. Since the *returned* issuer is what tokens carry, +`OIDC_ISSUER` must be the GUID form regardless of which you typed. + +--- + +## 3. App registration A — Web UI (confidential client) + +**Entra admin center → Entra ID → App registrations → New registration** + +- **Name:** `shared-memory-web` +- **Supported account types:** Accounts in this organizational directory only +- **Redirect URI:** platform **Web**, value: + ``` + https://memory.example.com/api/auth/callback/oidc + ``` + (replace with your `PUBLIC_URL`; the `/oidc` suffix comes from the provider + id in `apps/web/auth.ts` and is not configurable without a code change) + +Register, then collect: + +- **Overview → Application (client) ID** → `.env` as `OIDC_CLIENT_ID_WEB` +- **Certificates & secrets → New client secret** → `.env` as + `OIDC_CLIENT_SECRET_WEB` (copy the *Value*, not the Secret ID; it is shown + once) + +**API permissions:** `openid`, `profile`, `email` are Microsoft Graph delegated +permissions and are present by default via `User.Read`. Add `profile` +explicitly if it is missing — it gates the `oid` and `tid` claims, which +matter for §7. + +No "Expose an API" configuration is needed on this registration. Auth.js only +consumes the ID token here. + +--- + +## 4. App registration B — MCP resource server + +This registration is what `OIDC_AUDIENCE` refers to. It owns the API scope that +Claude Code requests, and the MCP endpoint validates that tokens were minted +for it. + +**App registrations → New registration** + +- **Name:** `shared-memory-mcp` +- **Supported account types:** same as A + +### 4a. Set the access token version — the single most common failure + +**Manage → Manifest**, find and set: + +```json +"api": { + "requestedAccessTokenVersion": 2 +} +``` + +There is no checkbox for this; it is a manifest edit. + +> **Note on the property name.** Older guides (and older versions of this +> project's notes) call this `accessTokenAcceptedVersion` at the top level of +> the manifest. That is the **retired Azure AD Graph** manifest format — +> Microsoft removed it from the portal's manifest editor on 2025-01-07, so you +> will not find that property. The current Microsoft Graph app manifest nests +> it as `api.requestedAccessTokenVersion`. The semantics are identical: +> `null` or `1` → v1.0 tokens, `2` → v2.0 tokens. + +Leave it at the default `null` and Entra issues **v1.0** access tokens, whose +issuer is: + +``` +https://sts.windows.net// ← note the trailing slash +``` + +not `https://login.microsoftonline.com//v2.0`. Verification then +fails with `claim invalid: iss`, and — because everything else in the OAuth +handshake succeeded — it looks like a mysterious 401 rather than a +configuration error. + +The setting lives on the **resource** app and wins over whichever endpoint the +client used: with `requestedAccessTokenVersion: 2`, a client hitting the v1.0 +endpoint still receives a v2.0 access token. + +### 4b. Expose an API + +**Manage → Expose an API** + +1. **Application ID URI** → *Add* → accept the default `api://`. +2. **Add a scope**: + - **Scope name:** `access_as_user` + - **Who can consent:** **Admins and users** (see §9) + - Fill in the admin/user consent display strings; they appear on the consent + prompt. + +The resulting full scope string is `api:///access_as_user`. + +### 4c. The public PKCE client (Claude Code) + +Claude Code is a public client using PKCE. Add a platform to registration B +(or to a third registration if you prefer them separated — then that +registration's client ID is `OIDC_CLIENT_ID_MCP`, and it needs +`api:///access_as_user` under **API permissions**): + +**Manage → Authentication → Add a platform → Mobile and desktop applications** + +> **The platform type is not cosmetic.** A redirect URI registered under the +> **Web** platform classifies the app as a *confidential* client, and the +> token exchange then demands a `client_secret` or `client_assertion` — +> Claude Code has neither, so the flow dies with +> `AADSTS7000218: The request body must contain the following parameter: +> 'client_assertion' or 'client_secret'`. The portal will happily accept +> `http://localhost:33418/callback` as a Web redirect URI, which is what makes +> this trap easy to fall into. **Mobile and desktop applications** +> (`publicClient` in the manifest) is the correct platform. SPA is not an +> option either — Entra rejects SPA redirect URIs for non-SPA flows. + +Under **Custom redirect URIs**, register: + +``` +http://localhost/callback +https://memory.example.com/auth/cli-callback +``` + +The first covers the loopback listener from README → *B. OAuth flow*; the +second is the manual-paste fallback from *C*. "Mobile and desktop +applications" permits arbitrary `https://` URIs alongside the loopback one, so +both live on the same platform. + +**Note the missing port.** Entra ignores the port component when matching +`http://localhost` redirect URIs, so the single registration +`http://localhost/callback` matches `http://localhost:33418/callback`, +`http://localhost:9999/callback`, and any other port. This is Entra's +equivalent of the Authentik regex (`^http://(127\.0\.0\.1|localhost):\d+/.*$`) +the README mentions — users can pick any `--callback-port` without +re-registering. + +Three constraints on that convenience: + +- **The path is still matched exactly.** Registering bare `http://localhost` + does *not* match `http://localhost:33418/callback`. The `/callback` suffix + must be there, and paths are case-sensitive. +- **Do not register several localhost URIs differing only by port.** Entra + picks one arbitrarily when matching. +- **Port-agnostic matching is documented for `localhost` only**, not for + `127.0.0.1` — and the portal text box refuses the `http://127.0.0.1` form + anyway (it requires a manifest edit). Use `localhost`. `[::1]` is not + supported at all. + +You do **not** need to enable **Allow public client flows** +(`allowPublicClient`). That toggle is a *fallback* for flows where Entra can't +infer the client type from a redirect URI — device code, ROPC, Windows +Integrated Auth. Authorization code + PKCE with a registered +mobile-and-desktop redirect URI is inferred correctly without it. (Entra's own +`reply-url` doc says otherwise in one sentence; the manifest reference and the +AADSTS7000218 troubleshooting article agree it is a fallback. Leave it off +unless you hit a problem — Microsoft warns that flipping a confidential client +to public has security implications.) + +**Application (client) ID** of whichever registration Claude Code +authenticates as → `.env` as `OIDC_CLIENT_ID_MCP`. + +--- + +## 5. `OIDC_AUDIENCE` vs. `OIDC_AUDIENCE_SCOPE` + +On Authentik these two look redundant — the scope mapping is named +`aud-shared-memory` and it emits `aud: shared-memory`, so the values track each +other. On Entra they are **necessarily different strings**, and swapping them is +the easiest mistake to make here. + +| Var | What it is | Entra value | +|---|---|---| +| `OIDC_AUDIENCE` | The `aud` claim `jwt.ts` requires on the token | `` — a bare GUID | +| `OIDC_AUDIENCE_SCOPE` | The scope string the *client* asks for, advertised in `/.well-known/oauth-protected-resource` | `api:///access_as_user` | + +Why they differ: the client requests a scope by its full URI +(*Application ID URI* + `/` + scope name), but Entra does not put that URI in the +token. For **v2.0** access tokens it splits the request into `aud` (the API's +**client-ID GUID**) and `scp` (the **short** scope name, `access_as_user`). +Three distinct strings for what feels like one concept. + +> **Do not trust this document — decode a real token.** Microsoft's own +> [access-tokens](https://learn.microsoft.com/en-us/entra/identity-platform/access-tokens) +> page says web APIs "must only accept tokens containing one of their AppId +> URIs as the `aud` claim", which contradicts the authoritative +> [access token claims reference](https://learn.microsoft.com/en-us/entra/identity-platform/access-token-claims-reference) +> ("In v2.0 tokens, this value is always the client ID of the API"). The +> claims reference is correct for v2.0, but given that Microsoft's docs +> disagree with each other, verify empirically — see §8. + +`OIDC_AUDIENCE_SCOPE` **must** be set explicitly on Entra. Left unset, the code +defaults to `aud-${OIDC_AUDIENCE}`, which is an Authentik naming convention and +means nothing to Entra — the client would request a nonexistent scope and the +authorize request fails outright. + +--- + +## 6. `offline_access` + +Set `OIDC_OFFLINE_ACCESS=true` from the start. Unlike Authentik — where you must +first attach an `offline_access` scope mapping to the provider — Entra treats +`offline_access` as one of its well-defined platform scopes (`openid`, `email`, +`profile`, `offline_access`). Nothing to create, and it is **implicitly +granted**: if any delegated permission is consented, `offline_access` is too. + +Two caveats: + +- It must still be *requested* at runtime, which is exactly what + `OIDC_OFFLINE_ACCESS=true` achieves — the flag adds it to `scopes_supported` + in `/.well-known/oauth-protected-resource`, and MCP clients only request + scopes they see advertised there. +- A refresh token comes back only on authorization-code-style flows. That is + what Claude Code uses, so this is satisfied; implicit flow would not be. + +The `.env` comment on this var warns that advertising a scope the IdP doesn't +offer risks `invalid_scope`. On Entra that risk doesn't apply. + +--- + +## 7. Identity: `sub` splits accounts across app registrations + +**Handled as of migration `0005_user_oid.sql`. Read this anyway — it explains +why `oid` is in your database, and what happens if you deploy the migration +late.** + +The app keys the `users` row on `(oidc_iss, oidc_sub)` — see the upsert in +`apps/web/lib/mcp/context.ts` and the one in `apps/web/auth.ts`. On Authentik +that is safe, because Authentik's `sub` is `user.uid`, a user-level value that +is identical across providers. + +Entra's `sub` is a **pairwise identifier**. Microsoft documents it as *"based on +a combination of the token recipient, tenant, and user"* — so the value is +scoped to the app registration in the `aud` position of that particular token: + +- Web UI sign-in → ID token with `aud` = registration **A** → `sub` = *X* +- MCP access token → `aud` = registration **B** → `sub` = *Y* + +*X ≠ Y*, by design, for privacy. `iss` is identical for both (one tenant, one +issuer), so `(iss, sub)` yields **two different keys for the same human**. Both +code paths *upsert* rather than fail, so nothing looks broken: the person signs +into the Web UI, sees their memories, connects Claude Code, and finds an empty +account. Writes land in the second row. + +There is no configuration fix. `sub` is in Entra's restricted claim set (no +claims-mapping policy can alter it), `subject_types_supported` advertises only +`pairwise`, and Microsoft has stated that `sector_identifier_uri` is not used to +generate it. + +**How it's handled.** Identity is keyed on `oid` — the directory object id, +which Microsoft documents as constant for a user across every application in a +tenant (*"all apps get the same `oid` and `tid` claims for a user acting in a +tenant"*). It is emitted by default in v2.0 ID *and* access tokens as long as +the `profile` scope is requested, which it is. + +`apps/web/lib/auth/identity.ts` holds the single resolver both surfaces call. +Resolution order when `oid` is present: + +1. an existing row keyed on `(oidc_iss, oidc_oid)` — the steady state +2. a pre-migration row matching `(oidc_iss, oidc_sub)` with no `oid` yet, which + gets its `oid` backfilled in place +3. insert + +IdPs that emit no `oid` (Authentik, Keycloak, Okta) skip straight to the +original `(iss, sub)` behaviour, unchanged. + +> **One upgrade-ordering caveat.** Step 2 adopts a legacy row by matching +> `sub`, and the only `sub` that can match is the one that created it — the +> **Web UI** one, since MCP auth against Entra was impossible before the JWKS +> fix in §0. So if you already had Entra users signing into the Web UI, have +> them **sign into the Web UI once** after deploying this migration, before +> connecting an MCP client. Connecting MCP first creates a fresh row keyed on +> `oid` and leaves the original stranded, with the memories in it invisible. +> Deployments that have never run Entra are unaffected. + +The single-registration layout (making registration A the resource server too) +also works and needs no migration, but you lose audience separation between the +Web UI and MCP. + +--- + +## 8. Verification + +Run these in order; each one isolates a different failure. + +**1. The issuer is concrete, not templated.** + +```bash +curl -s "https://login.microsoftonline.com//v2.0/.well-known/openid-configuration" \ + | jq '{issuer, jwks_uri}' +``` +`issuer` must be a GUID URL, not `{tenantid}`. Copy it verbatim into +`OIDC_ISSUER`. + +**2. Our metadata advertises the right scopes.** + +```bash +curl -s https://memory.example.com/.well-known/oauth-protected-resource | jq +``` +`scopes_supported` must contain `api:///access_as_user` (not +`aud-…`), plus `offline_access` if you enabled it. `authorization_servers[0]` +must be the tenant-specific v2.0 issuer. + +**3. Decode a real access token.** This is the only step that proves the +`aud`/`iss`/version questions. Get a token (from Claude Code's stored +credentials, or by running the flow manually) and inspect the payload: + +```bash +TOKEN='eyJ...' +echo "$TOKEN" | cut -d. -f2 | tr '_-' '/+' | base64 -d 2>/dev/null | jq '{ver, iss, aud, sub, oid, tid, scp, groups}' +``` + +Expected: + +| Field | Expected value | If wrong | +|---|---|---| +| `ver` | `"2.0"` | `api.requestedAccessTokenVersion` is not `2` (§4a) | +| `iss` | `https://login.microsoftonline.com//v2.0` | v1 token, or `common` authority (§2, §4a) | +| `aud` | ``, a bare GUID | set `OIDC_AUDIENCE` to whatever is actually here (§5) | +| `scp` | `access_as_user` | the scope wasn't requested or consented (§9) | +| `groups` | array of GUIDs, or absent | see §10 | + +**4. Confirm the 401 reason** when something is still wrong — the MCP endpoint +names the failing claim: + +```bash +curl -s -i -H "Authorization: Bearer $TOKEN" https://memory.example.com/api/mcp | head -20 +``` +Look at `WWW-Authenticate`: `error_description="claim invalid: iss"` → +§2/§4a. `"claim invalid: aud"` → §5. `"verification failed"` → JWKS could not +be fetched, §0. + +--- + +## 9. Consent for the API scope + +A custom scope is not inherently admin-only. Two levers decide it: + +- **The scope's own setting.** "Who can consent?" on the scope — **Admins and + users** lets users self-consent; **Admins only** always requires an admin. + Select "Admins and users" (§4b). Microsoft's docs don't state which radio the + portal preselects, so set it deliberately rather than assuming. +- **The tenant's user-consent policy.** The default is *"users are allowed to + consent to applications for permissions that don't require administrator + consent"*, but many tenants tighten this to "verified publishers only" or + disable user consent entirely, in which case an admin must consent regardless + of the scope setting. + +Admin consent becomes **mandatory** if: the scope is "Admins only"; the tenant +policy restricts user consent; or — the one that catches people — the enterprise +application is set to **require user assignment**, which forces admin consent +even when tenant policy would otherwise permit self-consent. + +**To grant it:** App registrations → *the client app* (the one Claude Code uses, +not the API) → **API permissions** → **Grant admin consent for \**. The +button is disabled if you aren't an admin or no permissions are configured. + +Alternatively, suppress the prompt entirely with **pre-authorization**: on +registration B, **Expose an API → Authorized client applications → Add a client +application**, select the MCP client ID and tick `access_as_user`. Consent is +then implicit. Reasonable here, since you control both registrations. + +If you prefer the URL form of admin consent, note it needs the `/v2.0/` segment +and must not use `common`: + +``` +https://login.microsoftonline.com//v2.0/adminconsent + ?client_id= + &scope=api:///access_as_user + &redirect_uri=https://memory.example.com/auth/cli-callback + &state=12345 +``` + +--- + +## 10. Groups + +Group memberships gate access to shared projects (`readableProjectIds` / +`canWriteProject` in `apps/web/lib/mcp/tools.ts` and +`apps/web/lib/memory-mutations.ts`). Entra's groups claim needs care on two +independent axes: **what the values look like**, and **what happens when the +claim goes missing**. + +### 10a. By default you get GUIDs, not names + +Entra emits `groups` as a **JSON array of group object-ID GUIDs**. Not display +names. `apps/web/lib/auth/sync-groups.ts` stores whatever strings arrive +verbatim and makes no attempt to resolve them, so the Web UI will list +memberships like `8f4c…-b21a` and your project ACLs must be written against +those GUIDs. + +There *is* a supported way to get display names for cloud-only groups — +contrary to the older note in `sync-groups.ts`, which says names are available +only for AD-synced groups. That was true of the `sam_account_name` family +(those attributes genuinely exist only on groups synced from on-premises AD via +Entra Connect 1.2.70+), but Entra also has `cloud_displayname`: + +**App registrations → \ → Token configuration → Add groups claim**, select +**Groups assigned to the application**, then tick the cloud-only display name +option. In the manifest: + +```json +"groupMembershipClaims": "ApplicationGroup", +"optionalClaims": { + "accessToken": [ + { "name": "groups", + "additionalProperties": ["cloud_displayname"] } + ], + "idToken": [ + { "name": "groups", + "additionalProperties": ["cloud_displayname"] } + ] +} +``` + +Both collections matter: `idToken` feeds the Web UI sign-in path (`auth.ts`), +`accessToken` feeds the MCP path (`jwt.ts`). Configure only one and the two +surfaces disagree about your group names. + +Constraints, all of them load-bearing: + +- `cloud_displayname` **only works with `groupMembershipClaims: + "ApplicationGroup"`**. Microsoft's stated reason is that group display names + aren't unique, so they only emit them for groups explicitly assigned to the + application. +- Only **directly assigned** groups appear. **Nested groups are excluded.** +- Assign the groups under **Enterprise applications → \ → Users and + groups**, or they simply won't be emitted. +- Microsoft's published `cloud_displayname` examples cover `idToken` and + `saml2Token`; we found no official example pairing it with `accessToken`. + It is a documented-valid collection, but **decode a real access token (§8) + and confirm `groups` contains names before relying on it** rather than + assuming symmetry. + +A claims-mapping policy cannot fix this instead: `groups` is a restricted +claim, so its data source can't be changed and no transformation applies. + +If none of this appeals, Microsoft's own recommendation is to use **app roles** +rather than groups for authorization — but this app reads `groups`, so that +would need a code change. + +### 10b. Groups overage — now refused rather than obeyed + +**This was the sharpest edge in this document. It is now a hard failure with a +readable message, which is a much better outcome than what it used to do.** + +Past a limit, Entra stops emitting `groups` altogether and substitutes an +overage indicator: + +| Token | Limit | What you get past it | +|---|---|---| +| JWT (access + ID) | **200** groups | `groups` absent; `_claim_names` / `_claim_sources` present | +| SAML | 150 groups | same | +| Implicit flow | **5** groups | `"hasgroups": true` | + +The indicator looks like this — note it is *not* a truncated list, it is no +list at all: + +```json +{ + "_claim_names": { "groups": "src1" }, + "_claim_sources": { "src1": { "endpoint": "https://graph.windows.net/…" } } +} +``` + +(That endpoint is an **Azure AD Graph** URL, not Microsoft Graph. Don't follow +it; Microsoft says to construct +`https://graph.microsoft.com/v1.0/users/{id}/getMemberObjects` yourself. +Limits are inclusive of nested groups.) + +**What this used to do.** An absent `groups` claim and a claim saying "zero +groups" were indistinguishable to `normalizeGroupsClaim`, which returned `[]` +for both — and the `names.length === 0` branch **deletes every one of that +user's `user_groups` rows**. So a user crossing 200 groups signed into the Web +UI once and silently lost access to every shared project, on both surfaces, +with no error anywhere. (The MCP path never deleted anything, but it then read +the snapshot the Web sign-in had just emptied.) + +**What happens now.** `detectGroupsOverage` looks for `_claim_names.groups` and +`hasgroups`, and both surfaces refuse the token rather than acting on group +state they know they don't have: + +- **Web sign-in** throws `GroupsOverageError`, which fails the sign-in. Existing + memberships are left completely untouched. +- **MCP** returns 401 with + `error_description="groups overage: IdP did not enumerate group membership …"`. + +The user is blocked until an admin fixes the claim configuration — and then +signs in and finds their access exactly as it was. Nothing to restore, because +nothing was destroyed. Granting access from a stale snapshot, or revoking it on +a claim the IdP never made, are both guesses; refusing is the only honest +answer available. + +An absent `groups` claim with **no** overage marker still clears memberships. +That is unchanged and deliberate: the IdP has genuinely stopped asserting the +groups, so we stop honouring them. + +#### Getting a blocked user back in + +1. App registration → **Token configuration** (or the manifest) → set + `groupMembershipClaims` to **`ApplicationGroup`** — the portal labels this + **"Groups assigned to the application"**. It emits only the groups + explicitly assigned to *this* application, which for a memory server is a + handful, so the 200-group ceiling stops being reachable. Microsoft + recommends it for exactly this reason, and it is the same setting + `cloud_displayname` requires — §10a and §10b have one shared fix. +2. Enterprise applications → your app → **Users and groups** → assign the + groups you actually share projects with. `ApplicationGroup` emits **directly + assigned groups only**; nested and transitive membership is excluded, so + assign the real groups rather than a parent. +3. Confirm the `groups` optional claim is configured for the **access token**, + not only the ID token — the MCP path reads the access token. +4. The user signs in again. Their memberships were never deleted, so their + access returns as it was. + +Leaving `groupMembershipClaims` at `All` or `SecurityGroup` in a large tenant is +what makes this bite in the first place. + +If a group genuinely must exceed the limit, the other way out is **app roles**, +which are app-scoped and never overage — but they arrive in a `roles` claim and +this codebase reads `groups`, so that is a code change, not a config change. + +--- + +## 11. Connecting Claude Code + +Everything in README → **Connecting Claude Code** applies unchanged, with one +Entra-specific confirmation: **the pre-registered client-id path is +mandatory.** + +Entra does not implement RFC 7591 Dynamic Client Registration. Its discovery +document publishes no `registration_endpoint`, it serves no RFC 8414 +authorization-server metadata at all (only OIDC discovery), and it does not +advertise `client_id_metadata_document_supported` — so neither DCR nor the CIMD +mechanism that superseded it in the MCP spec is available. Microsoft states this +plainly in its own MCP guidance ("Microsoft Entra ID doesn't currently support +client registration") and has said it is not on the near-term roadmap. + +Practically, this means: + +- Use the plugin (`plugin/.mcp.json` ships a pre-registered `clientId`), or +- Pass `--client-id ` explicitly on `claude mcp add`. + +A client that expects to self-register will fail. This is the same situation as +Authentik, so the README's guidance needs no adjustment. + +--- + +## 12. Env var reference card + +Straight from Entra's UI labels to `.env` keys. `` is app registration A +(Web UI, §3); `` is app registration B (MCP resource server, §4). + +| `.env` key | Where it comes from in Entra | Example | +|---|---|---| +| `OIDC_ISSUER` | `issuer` from the **tenant-specific** discovery document (§2). Not the authority you typed — the value the endpoint returns. | `https://login.microsoftonline.com//v2.0` | +| `OIDC_ISSUER_MCP` | **Leave unset.** Entra has one issuer per tenant; there is no per-application issuer to point at. `mcpIssuer()` falls back to `OIDC_ISSUER`. | *(unset)* | +| `OIDC_CLIENT_ID_WEB` | `` → **Overview → Application (client) ID** | `1111…-aaaa` | +| `OIDC_CLIENT_SECRET_WEB` | `` → **Certificates & secrets → Client secrets → Value** (not Secret ID; shown once) | `abc8Q~…` | +| `OIDC_CLIENT_ID_MCP` | Client ID of the **public PKCE** registration Claude Code authenticates as (§4c) | `3333…-cccc` | +| `OIDC_AUDIENCE` | `` → **Overview → Application (client) ID**. The bare GUID, *not* the `api://` URI. Confirm by decoding a token (§8). | `2222…-bbbb` | +| `OIDC_AUDIENCE_SCOPE` | `` → **Expose an API** → the scope's full string: Application ID URI + `/` + scope name. Must be set explicitly; the `aud-…` default is Authentik-only. | `api://2222…-bbbb/access_as_user` | +| `OIDC_OFFLINE_ACCESS` | Nothing to configure in Entra — set it to `true` (§6). | `true` | + +`PUBLIC_URL` and the non-OIDC vars are unchanged from the README. + +--- + +## 13. Multitenant is not supported + +`acceptedIssuers()` in `apps/web/lib/auth/jwt.ts` compares `iss` against a +fixed pair of strings (with and without a trailing slash). Multitenant Entra +apps require substituting each token's `tid` claim into the `{tenantid}` +placeholder before comparing, and separately validating the signing key's own +issuer. Neither is implemented. + +Beyond `iss`, multitenant would also need the identity keying in §7 resolved, +since Microsoft is explicit that `oid` and `sub` differ per tenant by design and +that a guest user authenticating in another tenant *"should be treated as if +they're a brand new user to the service."* + +Single-tenant is the supported configuration.