Skip to content

feat(auth): add Microsoft Entra ID support with On-Behalf-Of token exchange#953

Merged
manusa merged 1 commit intocontainers:mainfrom
nader-ziada:entraid
Apr 14, 2026
Merged

feat(auth): add Microsoft Entra ID support with On-Behalf-Of token exchange#953
manusa merged 1 commit intocontainers:mainfrom
nader-ziada:entraid

Conversation

@nader-ziada
Copy link
Copy Markdown
Collaborator

Add support for Microsoft Entra ID (Azure AD) as an OIDC provider

Changes:

  • Add entra-obo token exchange strategy for On-Behalf-Of flow
  • Implement well-known endpoint fallback for providers without oauth-authorization-server (falls back to openid-configuration)
  • Add a new config option cluster_auth_mode that has two values: passthrough and kubeconfig
    • Add auto-detection: passthrough when require_oauth=true, else kubeconfig
    • Passthrough mode now automatically exchanges tokens if configured

@Cali0707 Cali0707 requested review from Cali0707, manusa and matzew and removed request for matzew March 23, 2026 18:00
@nader-ziada nader-ziada force-pushed the entraid branch 5 times, most recently from 5f76f86 to c419416 Compare April 9, 2026 14:19
@nader-ziada nader-ziada marked this pull request as ready for review April 9, 2026 14:19
Copy link
Copy Markdown
Collaborator

@Cali0707 Cali0707 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for all the hard work getting entra id to work @nader-ziada !

I did a first pass looking through the PR, left some comments/questions

Comment thread pkg/tokenexchange/assertion.go
Comment thread pkg/http/wellknown.go Outdated
Comment thread pkg/kubernetes/token_exchange.go Outdated
Comment thread pkg/tokenexchange/config.go
Comment thread pkg/tokenexchange/assertion.go Outdated
Comment thread pkg/kubernetes/manager.go Outdated
…change

Add support for Microsoft Entra ID (Azure AD) as an OIDC provider

add JWT client assertion support for Entra ID OBO flow

Implement RFC 7523 JWT Client Assertion for Microsoft Entra ID
On-Behalf-Of token exchange, enabling certificate-based authentication
as an alternative to client secrets.

Signed-off-by: Nader Ziada <nziada@redhat.com>
@matzew
Copy link
Copy Markdown
Collaborator

matzew commented Apr 14, 2026

Positives:

  • Well-structured config validation in ValidateClusterAuthMode() that catches contradictory settings early (passthrough without OAuth, kubeconfig with token exchange, token exchange without OAuth) — all rejected at startup.
  • Good test coverage: TokenExchangeRoutingSuite covers auto-detect, explicit modes, and token behavior. Assertion tests verify SHA-256 headers, lifetime defaults, and caching. TestMetadataGenerationFallback covers the core 404-fallback path.
  • The trust_proxy_headers default-off approach is the right security posture.
  • ResolveClusterAuthMode() with auto-detect is clean and predictable — explicit config wins, otherwise derived from require_oauth.
  • oauth.State with atomic.Pointer[Snapshot] gives lock-free concurrent reads with snapshot diffing to detect what changed on SIGHUP reload.
  • The 503-line ENTRA_ID_SETUP.md is thorough — covers three deployment patterns (basic, ServiceAccount, OBO), troubleshooting, architecture diagram, and the two-app-registration scenario.
  • All 6 review comments from @Cali0707 have been addressed in the force-pushed commit (SHA-256 migration, trust_proxy_headers, STS deduplication, HTTPClient mutex, error messages, centralized auth mode).

Minor observations (non-blocking):

  1. OIDC config cache in fetchOpenIDConfiguration uses RLock/check/RUnlock then fetch then Lock/write/Unlock. Two concurrent cache misses will both fetch and write. Functionally harmless (same data, last write wins) but worth being aware of under load.
  2. stsConfig parameter threading through ExchangeTokenInContext — the "pass nil to build fresh" pattern pushes caching responsibility to callers. Works fine with the mutex + URL check in tokenExchangingProvider, just an unusual API surface.
  3. 2500+ lines across 35 files is a larger PR — just noting for awareness.

Copy link
Copy Markdown
Member

@manusa manusa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @nader-ziada, thanks for the great work on this. The security posture throughout (fail-closed on token exchange failure, opt-in trust_proxy_headers, path-traversal guards) is spot on.

One optional cleanup if you feel like it: IsRequireOAuth() looks like dead code now. After moving Derived() over to ResolveClusterAuthMode(), I couldn't find any remaining callers.

  • Interface: pkg/api/config.go:24
  • Impl: pkg/config/config.go:394
$ grep -rn 'IsRequireOAuth()'
pkg/api/config.go:24:    IsRequireOAuth() bool
pkg/config/config.go:394:func (c *StaticConfig) IsRequireOAuth() bool {

The RequireOAuth field and require_oauth TOML key are obviously still needed, only the method. Feel free to drop it in this PR or leave it for a follow-up, whichever you prefer.

Thanks again!

@manusa manusa requested a review from Cali0707 April 14, 2026 12:01
Copy link
Copy Markdown
Collaborator

@Cali0707 Cali0707 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks @nader-ziada !!

@manusa manusa merged commit ef5b9a8 into containers:main Apr 14, 2026
11 checks passed
manusa added a commit to marcnuri-forks/kubernetes-mcp-server that referenced this pull request Apr 14, 2026
The IsRequireOAuth() method and AuthProvider interface became dead
code in containers#953 when Derived() moved to ResolveClusterAuthMode(). The
RequireOAuth field and require_oauth TOML key are still needed —
only the method was unreferenced.

Refs containers#953

Signed-off-by: Marc Nuri <marc@marcnuri.com>
nader-ziada pushed a commit that referenced this pull request Apr 14, 2026
* refactor(config): drop unused IsRequireOAuth method

The IsRequireOAuth() method and AuthProvider interface became dead
code in #953 when Derived() moved to ResolveClusterAuthMode(). The
RequireOAuth field and require_oauth TOML key are still needed —
only the method was unreferenced.

Refs #953

Signed-off-by: Marc Nuri <marc@marcnuri.com>

* test(auth): add Entra ID / OBO follow-up coverage

- Strengthen the OBO happy path: assert the exchanged token — not the
  original user token — reaches the backend, replacing the removed log
  substring assertion with an observable behavior check.
- Add TestAuthorizationOidcTokenExchangeSSE that drives the same flow
  over mcp.SSEClientTransport. SSE relies on AuthorizationMiddleware
  injecting the validated bearer into the request context;
  StreamableHTTP propagates headers directly, so the context-injection
  path was previously untested (#1043).
- Add TrustProxyHeadersSuite covering client.address and url.scheme
  gating in RequestMiddleware. Assertions read OTel span attributes
  from an in-memory recorder installed via TestMain (#1044).

Closes #1043
Closes #1044

Signed-off-by: Marc Nuri <marc@marcnuri.com>

---------

Signed-off-by: Marc Nuri <marc@marcnuri.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants