feat: support Microsoft Entra ID as an OIDC provider
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) <noreply@anthropic.com>
This commit is contained in:
+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
|
||||
|
||||
Reference in New Issue
Block a user