Skip to content

fix(oauth): support external URL override for TLS-terminating proxy d…#364

Open
michaelalinks wants to merge 3 commits intoAzure:mainfrom
michaelalinks:fix/external-url-oauth-metadata
Open

fix(oauth): support external URL override for TLS-terminating proxy d…#364
michaelalinks wants to merge 3 commits intoAzure:mainfrom
michaelalinks:fix/external-url-oauth-metadata

Conversation

@michaelalinks
Copy link
Copy Markdown

@michaelalinks michaelalinks commented Apr 28, 2026

When aks-mcp is deployed behind a TLS-terminating reverse proxy (e.g. Envoy Gateway, AGIC), r.TLS is always nil so OAuth metadata endpoints incorrectly return http:// URLs. This causes MCP clients to reject the metadata.

Add --oauth-external-url flag (and OAUTH_EXTERNAL_URL env var fallback) that, when set, is used as the base URL in both /.well-known/oauth-protected-resource and /.well-known/oauth-authorization-server responses instead of deriving the scheme from r.TLS. The existing request-derived logic is preserved as the fallback when the flag is not set.

Expose the option in the Helm chart as oauth.externalURL.

Please see also the related open issue (running a custom image of these code changes): #365

michaela-links and others added 2 commits April 28, 2026 12:38
…eployments

When aks-mcp is deployed behind a TLS-terminating reverse proxy (e.g. Envoy
Gateway, AGIC), r.TLS is always nil so OAuth metadata endpoints incorrectly
return http:// URLs. This causes MCP clients to reject the metadata.

Add --oauth-external-url flag (and OAUTH_EXTERNAL_URL env var fallback) that,
when set, is used as the base URL in both /.well-known/oauth-protected-resource
and /.well-known/oauth-authorization-server responses instead of deriving the
scheme from r.TLS. The existing request-derived logic is preserved as the
fallback when the flag is not set.

Expose the option in the Helm chart as oauth.externalURL.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@michaelalinks michaelalinks marked this pull request as ready for review April 28, 2026 12:07
@michaelalinks
Copy link
Copy Markdown
Author

@microsoft-github-policy-service agree

The /oauth/callback handler was attempting to exchange the authorization
code with Azure AD server-side, which always failed with AADSTS9002313
because no code_verifier was included (PKCE is enforced on /authorize).

The correct MCP OAuth proxy pattern is:
1. /authorize: store state → client redirect_uri, then forward to Azure
   AD with the server's own callback URL as redirect_uri
2. /oauth/callback: look up the client redirect_uri from state and 302
   back to the client with code+state intact
3. The MCP client exchanges the code directly via /oauth2/v2.0/token
   using its own PKCE code_verifier

Also removes generateSessionToken, writeCallbackSuccessResponse,
writeCallbackErrorResponse, and exchangeCodeForToken which were only
used by the old callback handler, along with the crypto/rand and
encoding/base64 imports that served them.

Updates and extends tests to cover the new relay behaviour including
state consumption, error forwarding, and unknown state rejection.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@michaelalinks
Copy link
Copy Markdown
Author

Notes on the OAuth callback proxy fix

Adding these notes to document three decisions made in the changes to internal/auth/oauth/endpoints.go.

Why crypto/rand, encoding/base64, and related helpers were removed

The following functions formed a chain only used by the old callbackHandler:

  • generateSessionToken — generated a random token using crypto/rand + encoding/base64
  • writeCallbackSuccessResponse — called generateSessionToken and rendered an HTML token page
  • writeCallbackErrorResponse — rendered an HTML error page
  • exchangeCodeForToken — exchanged the authorization code with Azure AD server-side

None of these are called from anywhere else. Since the new callbackHandler only does a 302 redirect, the entire chain is dead code and has been removed along with the imports that only served it.

Why the removed exchangeCodeForToken was already broken

exchangeCodeForToken never passed a code_verifier to Azure AD, but PKCE is enforced on the /authorize endpoint — meaning every authorization code issued requires a verifier to exchange it. Azure AD
returns AADSTS9002313 without it, which is exactly the error reported in #365. The old server-side token exchange was never going to succeed for any client.

Why the proxy approach works for all MCP clients, not just one

The state → client redirect_uri mapping in pendingStates is fully client-agnostic. Whatever redirect_uri an MCP client sends to /authorize (VS Code Copilot, MCP Inspector, or any future OAuth 2.1
client) gets stored and forwarded back after Azure AD redirects to the server callback. The only operational requirement is that each client's redirect URI is added to the --oauth-redirects allowed list in
the Helm chart — no code changes needed to support new clients.

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.

2 participants