feat(server): support Microsoft Entra ID as a token-exchange issuer

[weiishann]

PR Checklist

• A description of the changes is added to the description of this PR.
• If there is a related issue, make sure it is linked to this PR.
• If you've fixed a bug or added code that should be tested, add tests!
• If you've added or modified a feature, documentation in docs is updated

Description of changes

Adds Microsoft Entra ID as a token-exchange issuer, alongside the existing DWSU static-JWKS trust path. Server side only — the UI sign-in flow is a separate, later change.

Why this was not possible before

JwksOperations.loadJwkProvider consulted the static JWKS file whenever the file existed, which made the OIDC-discovery branch below it unreachable. Every deployment rendered by deploy-uc.sh sets that file, so an Entra-signed token failed signature lookup with a 401. Entra's keys rotate and cannot live in a hand-maintained file, so the file-vs-discovery choice had to become per-issuer.

What changed

• Per-issuer JWKS routing. The static file is authoritative only for the issuers it declares (each key's issuer member). Everything else resolves by OIDC discovery. Both trust sources coexist in one deployment, and DWSU hot-onboarding is untouched — the file path stays .cached(false).
• Three-value Entra configuration. UC_ENTRA_TENANT_ID, UC_CLIENT_ID, UC_CLIENT_SECRET. The issuer and audience are derived in ServerProperties and unioned into the configured lists, so UC_ALLOWED_ISSUERS needs no Entra entry. The OAuth URLs are derived at render time in deploy-uc.sh, because their only consumer (Oauth2CliExchange) reads the rendered file directly and never goes through ServerProperties.
• Discovery hardened for production. 5s timeout, response-status checking, per-issuer caching of the built JwkProvider (24h), rate limiting, and single-flight so concurrent callers share one in-flight fetch. Failures are never cached.
• jwks_uri validation. Absolute-URL and scheme checks, and rejection of loopback, link-local, CGNAT, NAT64, multicast and RFC1918 targets. See the open item below for the known gap.
• Failures classified by provenance, not by exception subtype. A local key file that cannot be read is a server-configuration fault; an unreachable identity provider is 503; a kid absent from a healthy key set is 401. Classification happens in JwksOperations, where the origin of the key set is known, rather than by pattern-matching a third-party exception hierarchy in a handler shared by every route.
• Error responses no longer disclose internals. No stack traces, no raw cause messages, no provider URLs or filesystem paths in any client-facing message. All of that goes to the server log.
• Diagnosable principal failures. A subject token missing a usable email claim says so and points at the Entra app registration's optional claims, instead of failing as User not allowed: <guid>. A startup log records the derived issuer and audience so the effective trust set is auditable.
• Docs. New "Microsoft Entra ID sign-in" section in deploy/README.md, mirrored into deploy/README.zh-CN.md.

Effect on existing deployments

None by default. Every derivation is gated on server.entra.tenant-id; with no tenant configured, unionWithDerived returns the caller's list object unchanged, so there is no new code path. The routing change is safe because IssuerScopedJwkProvider already required every key to carry an issuer member, so any working deployment necessarily has a non-empty knownIssuers().

One intended behavior change: an issuer listed in server.allowed-issuers but not declared in the JWKS file now reaches OIDC discovery instead of returning 401. This only affects operator-configured values. Note the corollary — for such an issuer, removing its key from the JWKS file no longer fails closed; it routes to discovery. With the recommended blank UC_ALLOWED_ISSUERS this cannot arise.

Review history

Two rounds of adversarial review ran against this branch and found roughly thirty defects between them, including several introduced by the fixes for earlier ones. Fixed since the first draft: a rate limiter configured 100× tighter than documented (rateLimited's second argument is a refill period, not a count) which allowed ~10 unauthenticated requests to disable Entra sign-in for ~100 minutes; NetworkException used as a proxy for "remote", so a missing local certs.json returned 503 blaming Microsoft across the entire authenticated API surface; List.copyOf().contains(null) turning a 401 into a 500; three unauthenticated paths returning 500s with stack traces; an SSRF guard defeated by a trailing dot; and secret injection through the deploy renderer's sequential substitution.

Testing

265 tests, javafmtCheck clean. New tests drive real behavior rather than mocks: discovery runs over real HTTP against a local Armeria server, caching is asserted via JWKS-endpoint hit counts, and the redaction guard asserts the response body contains no path or URL. Every test added in the later waves was proved load-bearing by reverting the behavior and confirming it goes red.

CI will not verify this. unit-tests.yml triggers only on main and branch-*, so this PR runs lint and the Python client but never the server suite. All test results above are from local runs.

Open item needing a decision

UrlJwkProvider follows same-protocol redirects, so an allow-listed identity provider whose jwks_uri returns a 302 to an internal https address bypasses the jwks_uri validation, which only checks the initial URL. Closing this requires fetching the JWKS directly instead of delegating to the library.

Pending that, the limit is now documented rather than implied: the code javadoc and both deploy READMEs state that the check bounds the URL handed to the fetch and not every address the fetch can reach, so trusting an issuer reads as what it is — a decision about that issuer's operator.

Known follow-ups

• 24-hour key cache means a signing key withdrawn at the identity provider keeps verifying tokens for up to a day, with no flush short of a restart.
• IssuerScopedJwkProvider filters after selection, so a kid collision between two issuers in one JWKS file permanently rejects the later-listed one.
• Single-flight serializes the failure path, so concurrent requests during an outage queue rather than failing in parallel.
• No negative caching of unknown kids, so a flood can still degrade sign-in (recovery ~6s per token).
• The discovery INFO line is emitted before any signing key is fetched, so it can report success while every key lookup fails.
• helm/ does not support the new keys, so Entra is inert on a Helm deploy.
• docs/server/auth.md still documents a superseded error string, and both READMEs describe a warning that is now suppressed.
• --render-only aborts if the server binary is absent, which is the case it is most useful in.
• A Helm-independent gap: a configured tenant's discovery path cannot be integration-tested, because the authority is fixed at login.microsoftonline.com and cannot be pointed at a test server.

Manual acceptance still required

Exchanging a real token from a real tenant. No test can prove a given app registration actually emits the email claim, which is the assumption the whole provisioning model rests on.

🤖 Generated with Claude Code

[hujincalrin41]

The identity chain here is email → sub, which may not admit a service principal, depending on what the deployment actually sends. Flagging it as a question rather than a defect, because I have not verified it against a live tenant.

The concern: an Entra app-only token (client credentials) is documented as carrying neither email nor preferred_username, and its sub is the service principal's object id. If that holds, verifyPrincipal reaches getUserByEmail(<guid>), and the only way to provision such a caller is a UC user whose email field literally holds the GUID. validateUserEmail is lax enough to accept that, so it would function — but with two consequences that are verifiable in this repository rather than inferred:

1. UserRepository.updateUser mutates only name, active and externalId — email is immutable. Entra exposes three similar-looking GUIDs (app object id, application/client id, enterprise-application object id) and only the last appears as oid/sub, so picking the wrong one is an easy mistake. The only remedy would be deleting and recreating the user, which mints a new UUID and drops every grant, since authorization is keyed by the user UUID.
2. The message this branch adds would not help in that case: with no user behind an app-only token, "add 'email' as an optional claim on the app registration" is not an action the operator can take.

Why this seems worth settling before merge: UC_ENTRA_TENANT_ID + UC_CLIENT_ID + UC_CLIENT_SECRET — the three values this PR introduces — is also the configuration shape of a service principal. The question that decides it is narrow: after those three are configured, is an interactive sign-in still required? If yes, this is a user-account flow and the current chain is fine (given email is configured as an optional claim). If no, it is a service principal and this path likely needs to resolve it.

If it turns out to be needed, one direction: resolve through an ordered chain rather than a single claim — email → preferred_username → upn looked up by email, then oid looked up by externalId, then sub. getUserByExternalId(Session, String) already exists in UserRepository (it backs the uniqueness check on insert); it would need a public overload, no schema change. Keeping the GUID in externalId also makes a mistyped id repairable in place, since that field is updatable.

Test coverage: AuthServicePrincipalErrorsTest covers the email/sub outcomes; there is no case for a token carrying only oid.

[hujincalrin41]

getEntraIssuer() derives exactly one issuer from a single-valued server.entra.tenant-id, which fixes this deployment to one Entra tenant. Worth deciding explicitly whether that is the intended scope.

Two things follow from the single value:

1. A second tenant has to be listed by hand in server.allowed-issuers, which AuthService compares by exact string equality. Workable, but it bypasses the derivation this PR adds and has to be maintained separately.
2. The multi-tenant authorities (common, organizations) cannot work at all today. Their OIDC discovery document returns the issuer as a literal template — https://login.microsoftonline.com/{tenantid}/v2.0 — while the token carries the real tenant GUID. The discovery path compares the document's issuer against the token's issuer for equality, so the check fails before any key is fetched. There is no template-aware comparison in this branch.

If single-tenant is the intended scope, it would help to say so in deploy/README.md next to UC_ENTRA_TENANT_ID, so an operator pointing it at common gets an answer from the docs rather than from a 401.

If multi-tenant is in scope, the comparison needs to treat the tenant path segment as a wildcard: match scheme, host and every other segment exactly, and allow that one segment to be any non-empty value. That keeps single-tenant behaviour bit-for-bit identical (an exact match still matches).

Test coverage: AuthServiceEntraTrustDerivationTest covers the single-tenant derivation; no case exercises a {tenantid} discovery document.

[hujincalrin41]

When the JWKS file exists but cannot be parsed, this catch returns Set.of(), and because loadJwkProvider routes on knownIssuers().contains(issuer), every locally-registered issuer stops being "known" and is sent to OIDC discovery instead.

For a DWSU issuer — a bare identifier, not a URL — that means "https://" + issuer, a DNS failure, and a 503 that reads identity provider unreachable or rate-limited. The operator is pointed at Microsoft for what is a local file problem. This also contradicts the classification principle this PR states in its own description: a local key file that cannot be read is a server-configuration fault; an unreachable identity provider is 503. Here an unreadable local file produces exactly the latter.

Before this change the routing did not depend on parsing: Files.exists(jwksPath) returned the file provider unconditionally, so a corrupt file surfaced as a file-provider failure at key-lookup time.

What makes this more than theoretical is the documented DWSU hot-onboarding procedure — an operator appends a key to this JSON file by hand, with no restart. One syntax error there does not just break the new key; it reroutes every issuer declared in that file to the public internet.

The javadoc above this method also describes the empty return as (fail-closed), which is not what it does at the call site: an empty set opens the discovery path rather than closing anything.

Suggested direction: distinguish "file parsed, issuer not declared in it" from "file could not be parsed". Only the first should fall through to discovery; the second is a server-configuration fault and should be reported as one.

Test coverage: JwksOperationsTest covers routing for a well-formed file; there is no case for a present-but-unparseable file.

[hujincalrin41]

This lock is held across the full discovery fetch below it, and a failed discovery is deliberately never cached, so during an identity-provider outage the callers do not fail in parallel — they queue.

Each waiter re-checks the cache, finds nothing (the peer that just failed wrote nothing), and starts its own 5-second attempt. The Nth concurrent /tokens call therefore returns after roughly 5N seconds, and each one holds a thread from Armeria's shared blocking executor for the whole wait. The pool is shared with the rest of the server, so a sustained outage on one issuer can degrade unrelated routes.

Single-flight itself is the right call for the cold-start case the comment describes; the problem is only that failures serialize as well as successes.

Suggested direction: either give failures a short negative cache (a few seconds is enough to collapse a burst) so the queue fails fast, or have waiters share the in-flight attempt through a future rather than take the lock and retry in turn.

Test coverage: JwksOperationsTest asserts caching via endpoint hit counts on the success path; there is no concurrent-outage case.

[hujincalrin41]

The bucket here is 10 tokens refilling one per 6 seconds, and auth0's Guava cache does not cache failures, so every lookup of a kid that is not already cached consumes a token and performs a real fetch against the identity provider.

/tokens needs no credentials, so ten unauthenticated requests carrying the Entra issuer and a random kid drain the bucket, and for roughly the next minute every legitimate cold lookup is refused by the limiter. It is worst exactly when it hurts most: right after a restart, or right after a key rotation, when nothing is cached yet. The ten requests also each cause a real JWKS fetch at Microsoft.

The refusal surfaces as identity provider unreachable or rate-limited, which points the operator at Microsoft rather than at the local limiter.

Suggested direction: at minimum separate the two conditions in the error text so the local limiter is diagnosable. Beyond that, the limiter is currently the only thing standing between an unauthenticated caller and an outbound fetch per request — a short negative cache keyed on (issuer, kid) would absorb the repeat-miss pattern without spending a token each time.

Test coverage: a test inspects the configured bucket; no case drives repeated unknown-kid lookups to observe the refusal.

[hujincalrin41]

Logging BaseException at DEBUG rests on the premise stated in the comment — that the code which threw it already reported the failure — and several throwers do not.

ModelRepository (around line 260) and AliyunCredentialGenerator (around line 146) both construct new BaseException(ErrorCode.INTERNAL, message, cause) and log nothing. Combined with the removal of stack_trace from the response body, a genuine 500 at the shipped info level now leaves no record on either side: the client gets a sanitised message, the server log gets nothing.

The premise does hold for the paths this PR owns — JwksOperations logs under a cooldown, and re-logging those at ERROR would undo the cooldown — but the handler is shared by every route, so the change applies well beyond them.

Suggested direction: gate the level on the status rather than on the exception type, e.g. server errors at ERROR (or WARN) and client errors at DEBUG. That keeps the cooldown intact for the 4xx classifications this PR adds, while a 500 stays visible.

Test coverage: GlobalExceptionHandlerErrorBodyTest asserts the response body; no case asserts what reaches the log.

[hujincalrin41]

UC_CLIENT_SECRET is documented here as not used by the server, and that is accurate for this branch — but it is worth recording where it is used, because the answer affects who can read it.

Its only consumer is examples/cli/.../Oauth2CliExchange.java, which reads etc/conf/server.properties by relative path (Paths.get("etc/conf/server.properties")). The CLI therefore has to run from the UC installation directory, and anyone who can do that — or simply read that file — holds the client secret. The upstream source acknowledges the placement with its own // TODO: These properties, especially client-secret should probably be server side.

Not something this PR has to solve, but two consequences are worth stating in deploy/README.md next to these variables:

1. the file needs restrictive permissions wherever CLI login is enabled, since the secret is at rest in the server's config directory;
2. browser sign-in is not covered by this arrangement. A server-hosted /login + /callback pair would keep the secret inside the server process and let the UI degrade to a plain link, which is also the shape that avoids shipping a confidential client's secret to any browser later.

Worth confirming the intended sequencing here: is browser sign-in expected to reuse these same four OAuth settings server-side, or to stay a CLI-only flow?

[hujincalrin41]

NUMERIC_HOST is \d+(\.\d+)*, which matches strings that are not IPv4 literals — 1.2.3.4.5, 256.1.1.1 — so InetAddress.getByName treats them as names and performs a real DNS lookup, contrary to the javadoc directly above stating this is what keeps this free of DNS lookups.

The exposure is limited: jwks_uri comes from the discovery document of an issuer that already passed the allow-list, so this is not reachable by an unauthenticated caller. Two things still make it worth tightening. The invariant the comment asserts is relied on by the rest of the guard, and the lookup happens while the per-issuer discovery lock is held, so a slow resolver extends the serialization described in the other comment on this file.

Suggested direction: parse the literal instead of pattern-matching it — reject anything that is not a well-formed IPv4 (four octets, each 0-255) or bracketed IPv6 before calling getByName, so the "literal only" property is enforced rather than approximated.

Test coverage: the SSRF cases cover loopback, link-local, CGNAT, NAT64, multicast and RFC1918 literals; none covers a numeric-looking non-literal.

[hujincalrin41]

Doc follow-up, not part of the diff: docs/server/auth.md lines 130 and 132 still show the pre-change wording for the token-exchange path.

unitycatalog/docs/server/auth.md

Lines 130 to 132 in 2aa698c

	Exception in thread "main" java.lang.RuntimeException: io.unitycatalog.client.ApiException: Error authenticating - {"error_code":"INVALID_ARGUMENT","details":[{"reason":"INVALID_ARGUMENT","metadata":{},"@type":"google.rpc.ErrorInfo"}],"stack_trace":null,"message":"User not allowed: bobbie@rocinante"}
	at io.unitycatalog.cli.UnityCatalogCli.main(UnityCatalogCli.java:168)
	Caused by: io.unitycatalog.client.ApiException: Error authenticating - {"error_code":"INVALID_ARGUMENT","details":[{"reason":"INVALID_ARGUMENT","metadata":{},"@type":"google.rpc.ErrorInfo"}],"stack_trace":null,"message":"User not allowed: bobbie@rocinante"}

Both sample responses read "message":"User not allowed: bobbie@rocinante", which this PR renamed to User not provisioned: ... in AuthService.verifyPrincipal. The same samples also show "stack_trace":null, a field this PR removes from the error body.

AuthDecorator keeps its own User not allowed message, so only the token-exchange samples in this file need updating.

[hujincalrin41]

This file and docs/superpowers/specs/2026-09-19-entra-idp-server-trust-design.md add roughly 1,600 lines of planning and design material to the repository. They are useful as review context, but worth a deliberate decision before merge rather than arriving with the code.

Two questions:

1. Should they ship in the repository at all? This is a public fork; a dated implementation plan is a different kind of artifact from docs/server/*, and it will age out of step with the code it describes. Several passages already describe intermediate states ("Expected: FAIL — both responses currently say User not allowed: ...") that are no longer true of the final diff.
2. If they stay, docs/superpowers/ is a new top-level area under docs/. Worth stating what belongs there, so the next feature knows whether to add to it.

Dropping them from the branch and keeping them in the PR description or a linked issue is the lower-maintenance option, and loses nothing for review.