feat: support Microsoft Entra ID as an OIDC provider #22
@@ -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**
|
||||
|
||||
+17
-22
@@ -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,
|
||||
// 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,
|
||||
})
|
||||
.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 });
|
||||
});
|
||||
|
||||
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;
|
||||
|
||||
@@ -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.
|
||||
@@ -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<T>(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<string, unknown> = {}) {
|
||||
return {
|
||||
iss: ISS,
|
||||
sub: WEB_SUB,
|
||||
oid: OID,
|
||||
email: "person@example.com",
|
||||
name: "Person",
|
||||
picture: null,
|
||||
...overrides,
|
||||
} as Parameters<typeof resolveUserId>[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);
|
||||
});
|
||||
});
|
||||
@@ -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<string> {
|
||||
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;
|
||||
}
|
||||
@@ -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<void>((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<string, unknown>).__sharedMemoryJwks;
|
||||
delete (globalThis as Record<string, unknown>).__sharedMemoryJwksRetryAt;
|
||||
delete process.env.OIDC_ISSUER_MCP;
|
||||
delete process.env.OIDC_AUDIENCE;
|
||||
await new Promise<void>((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<string> {
|
||||
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: "<html>login page</html>" };
|
||||
|
||||
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);
|
||||
});
|
||||
});
|
||||
+116
-11
@@ -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<typeof createRemoteJWKSet>;
|
||||
|
||||
type GlobalWithJwks = typeof globalThis & {
|
||||
__sharedMemoryJwks?: ReturnType<typeof createRemoteJWKSet>;
|
||||
/**
|
||||
* 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<JwkSet>;
|
||||
/**
|
||||
* 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, {
|
||||
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<string | null> {
|
||||
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<JwkSet> {
|
||||
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<Aut
|
||||
} as AuthenticatedClaims;
|
||||
}
|
||||
|
||||
const { payload } = await jwtVerify(token, jwks(), {
|
||||
const { payload } = await jwtVerify(token, await jwks(), {
|
||||
issuer: acceptedIssuers(),
|
||||
audience: env().OIDC_AUDIENCE,
|
||||
});
|
||||
@@ -153,6 +247,17 @@ export async function authenticateBearer(authHeader: string | null): Promise<Aut
|
||||
buildWwwAuthenticate("invalid_token", "missing sub"),
|
||||
);
|
||||
}
|
||||
// Groups overage: the IdP is telling us it holds memberships it declined
|
||||
// to list. `extractGroupsClaim` would read that as "no claim emitted" and
|
||||
// userContextFromClaims would fall back to the DB snapshot — granting
|
||||
// project access from a stale record while the live state is admittedly
|
||||
// unknown. Refuse; the operator fix is in the description.
|
||||
if (detectGroupsOverage(payload)) {
|
||||
const desc =
|
||||
"groups overage: IdP did not enumerate group membership " +
|
||||
"(set groupMembershipClaims=ApplicationGroup on EntraID)";
|
||||
throw new UnauthorizedError(desc, buildWwwAuthenticate("invalid_token", desc));
|
||||
}
|
||||
// Normalize the issuer for identity purposes.
|
||||
//
|
||||
// The token was just verified against mcpIssuer() — that check is done.
|
||||
|
||||
@@ -0,0 +1,126 @@
|
||||
import { afterAll, beforeEach, describe, expect, test } from "vitest";
|
||||
|
||||
/**
|
||||
* Group sync, and specifically the overage case.
|
||||
*
|
||||
* `syncUserGroupsFromClaim` deletes every membership when it sees no groups.
|
||||
* That is correct for "the IdP says zero groups" and catastrophic for "the
|
||||
* IdP declined to enumerate them" — EntraID past 200 groups. The two look
|
||||
* identical if you only inspect `claims.groups`, which is why the function
|
||||
* takes the whole claims object.
|
||||
*/
|
||||
const { db, pg } = await import("@/lib/db/client");
|
||||
const { users, groups, userGroups } = await import("@/lib/db/schema");
|
||||
const { syncUserGroupsFromClaim, detectGroupsOverage, GroupsOverageError } =
|
||||
await import("@/lib/auth/sync-groups");
|
||||
const { eq } = await import("drizzle-orm");
|
||||
|
||||
const ISS = "https://login.microsoftonline.com/sync-test/v2.0";
|
||||
let userId: string;
|
||||
|
||||
async function memberships(): Promise<string[]> {
|
||||
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"]);
|
||||
});
|
||||
});
|
||||
@@ -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<string, unknown>)
|
||||
);
|
||||
}
|
||||
|
||||
/**
|
||||
* 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<void> {
|
||||
if (detectGroupsOverage(claims)) throw new GroupsOverageError();
|
||||
|
||||
const rawClaim = (claims as { groups?: unknown } | null | undefined)?.groups;
|
||||
const names = normalizeGroupsClaim(rawClaim);
|
||||
|
||||
await db.transaction(async (tx) => {
|
||||
|
||||
@@ -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`),
|
||||
}),
|
||||
);
|
||||
|
||||
|
||||
+12
-30
@@ -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,
|
||||
// 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,
|
||||
})
|
||||
.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;
|
||||
}
|
||||
});
|
||||
|
||||
// OIDC bearer tokens carry a `groups` claim (when the IdP is configured to
|
||||
// emit it). CLI tokens never do — they go through verifyCliToken which
|
||||
|
||||
@@ -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/<slug>/jwks/` |
|
||||
| Entra ID | `https://login.microsoftonline.com/<tenant>/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/<tenant-id>/v2.0/.well-known/openid-configuration" \
|
||||
| jq -r .jwks_uri
|
||||
# → https://login.microsoftonline.com/<tenant-id>/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/<slug>/`) | **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, `<tenant-id>` 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/<tenant-id>/v2.0/.well-known/openid-configuration" | jq -r .issuer
|
||||
# → https://login.microsoftonline.com/<tenant-id>/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/<tenant-id>/ ← note the trailing slash
|
||||
```
|
||||
|
||||
not `https://login.microsoftonline.com/<tenant-id>/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://<client-id-of-B>`.
|
||||
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://<client-id-of-B>/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://<client-id-of-B>/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 | `<client-id-of-B>` — a bare GUID |
|
||||
| `OIDC_AUDIENCE_SCOPE` | The scope string the *client* asks for, advertised in `/.well-known/oauth-protected-resource` | `api://<client-id-of-B>/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/<tenant-id>/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://<client-id-of-B>/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/<tenant-id>/v2.0` | v1 token, or `common` authority (§2, §4a) |
|
||||
| `aud` | `<client-id-of-B>`, 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 \<tenant\>**. 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/<tenant-id>/v2.0/adminconsent
|
||||
?client_id=<OIDC_CLIENT_ID_MCP>
|
||||
&scope=api://<client-id-of-B>/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 → \<B\> → 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 → \<B\> → 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 <OIDC_CLIENT_ID_MCP>` 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. `<A>` is app registration A
|
||||
(Web UI, §3); `<B>` 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/<tenant-id>/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` | `<A>` → **Overview → Application (client) ID** | `1111…-aaaa` |
|
||||
| `OIDC_CLIENT_SECRET_WEB` | `<A>` → **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` | `<B>` → **Overview → Application (client) ID**. The bare GUID, *not* the `api://` URI. Confirm by decoding a token (§8). | `2222…-bbbb` |
|
||||
| `OIDC_AUDIENCE_SCOPE` | `<B>` → **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.
|
||||
Reference in New Issue
Block a user