-
Notifications
You must be signed in to change notification settings - Fork 631
MCP Authentication #12966
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
MCP Authentication #12966
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR adds MCP (Model Context Protocol) authentication support to the agentgateway, enabling JWT-based authentication for MCP backends with support for Auth0 and Keycloak identity providers.
Key changes:
- Added MCP authentication policy configuration with JWKS validation
- Implemented Auth0 mock server for e2e testing
- Extended JWKS store controller to handle MCP authentication JWKS sources
Reviewed changes
Copilot reviewed 26 out of 27 changed files in this pull request and generated 14 comments.
Show a summary per file
| File | Description |
|---|---|
api/v1alpha1/agentgateway_policy_types.go |
Added MCPAuthentication type with issuer, audiences, JWKS, and IDP configuration |
api/v1alpha1/zz_generated.deepcopy.go |
Generated deep copy methods for new authentication types |
pkg/agentgateway/plugins/backend_policies.go |
Implemented translateBackendMCPAuthentication to convert policy to agentgateway format |
internal/kgateway/agentjwksstore/jwks_store_controller.go |
Extended to fetch JWKS from both AgentgatewayPolicy and AgentgatewayBackend MCP auth configs |
install/helm/kgateway-crds/templates/*.yaml |
Updated CRDs with MCP authentication schema |
test/e2e/features/agentgateway/mcp/* |
Added e2e tests for MCP authentication with Auth0 mock |
hack/dummy-auth0/* |
New Python-based Auth0 mock server for testing OAuth flows |
go.mod |
Uses local replace directive for agentgateway dependency |
Makefile |
Added build targets for dummy-auth0 Docker image |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
5098478 to
4eedaac
Compare
| // McpIDP specifies the identity provider to use for authentication | ||
| // +kubebuilder:validation:Enum=Auth0;Keycloak | ||
| // +optional | ||
| McpIDP *McpIDP `json:"idp,omitempty"` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why do we need to know the IDP type? i.e. why isn't the issuer enough?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We use the idp to determine the path type: https://github.com/agentgateway/agentgateway/blob/a479200e2076bbaaa7321b2d439b5c19c9d44030/crates/agentgateway/src/types/agent.rs#L1331
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not related to the PR, is the above a constraint in our implementation, or is it coming from a spec? The spec i was able to find (https://modelcontextprotocol.io/specification/2025-03-26/basic/authorization#third-party-authorization-flow) is pretty minimal when it comes to third-party auth providers.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Our implementation- the idp and issuers are both determining the path in the line I sent earlier.
| } | ||
|
|
||
| // enqueue Backend MCP authentication JWKS (if present) | ||
| if p.Spec.Backend != nil && p.Spec.Backend.MCP != nil && p.Spec.Backend.MCP.Authentication != nil { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should look more closely at intoducing a jwks backend for AgentgatewayPolicy (see notes in #13014 for examples), as it would unify tls/traffic config UX and handling for fetching of remote jwks.
| // TODO: implement | ||
| // ResourceMetadata defines the metadata to use for MCP resources. | ||
| // +optional | ||
| ResourceMetadata map[string]string `json:"resourceMetadata"` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should not be map[string]string but rather map[string]JsonValue. I think the appropriate way to represent this is map[string]apiextensionsv1.JSON in kubebuilder but not 100% sure it works inside a map. You can see similar in FieldDefault
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks like ResourceMetadata map[string]apiextensionsv1.JSON worked with kubebuilder 🎉
| // McpIDP specifies the identity provider to use for authentication | ||
| // +kubebuilder:validation:Enum=Auth0;Keycloak | ||
| // +optional | ||
| McpIDP *McpIDP `json:"idp,omitempty"` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: provider or identityProvider I think may be more fitting
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll use provider since it's already under the mcp.authentication fields. I think identityProvider is a little too long
| @@ -0,0 +1,18 @@ | |||
| FROM python:3.11-slim | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: would be great to not have so many different mocks. can we just merge this in with the Golang mock IDP?
Have a separate image, especially a python one, is going to bloat our CI times
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ideally, yes- I think @shashankram is also looking at adding a provider for e2e tests on the envoy side. The current golang mock idp we're using for the jwt tests is pretty jwt specific (it just provides the different jwks to fetch, doesn't have the paths we need to test the mcp auth flow integration). I think we should discuss offline and consolitdate into having one e2e idp provider that ideally works for all these scenarios.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I did a minimal thing just to support remote jwks e2e tests; we could extend the fake idp, or even have a real idp with fake data (like using zitadel + preset accounts and jwks; not sure re: jwts, those may need to be generated).
|
|
||
| idp := api.BackendPolicySpec_McpAuthentication_AUTH0 | ||
| if authnPolicy.McpIDP != nil && *authnPolicy.McpIDP == v1alpha1.Keycloak { | ||
| idp = api.BackendPolicySpec_McpAuthentication_KEYCLOAK |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This needs a proto change, but we need a way to represent "no provider".
Provider means "this provider is no implementing the proper RFCs and needs workarounds" so ideally a user doesn't need one at all. And we should not default to keycloak
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It currently defaults to auth0, but agree- it would be ideal to not require a provider. Will create a follow up for this to make the dataplane change along with the mode configuration to support non-optional jwt modes
| extraResourceMetadata = make(map[string]*structpb.Value) | ||
| } | ||
| var parsed any | ||
| if err := json.Unmarshal([]byte(v), &parsed); err != nil { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if we do my commet on the API we don't need this or the json-in-string from users
Signed-off-by: npolshakova <[email protected]>
Signed-off-by: npolshakova <[email protected]>
Signed-off-by: npolshakova <[email protected]>
Signed-off-by: npolshakova <[email protected]>
Signed-off-by: npolshakova <[email protected]>
Signed-off-by: npolshakova <[email protected]>
Signed-off-by: npolshakova <[email protected]>
Signed-off-by: npolshakova <[email protected]>
Signed-off-by: npolshakova <[email protected]>
Signed-off-by: npolshakova <[email protected]>
Signed-off-by: npolshakova <[email protected]>
Signed-off-by: npolshakova <[email protected]>
a043893 to
c78ffd4
Compare
Signed-off-by: npolshakova <[email protected]>
Signed-off-by: npolshakova <[email protected]>
Signed-off-by: npolshakova <[email protected]> Signed-off-by: John Howard <[email protected]>
Description
Motivation:
Allows users to configure MCP authentication for MCP backends.
What changed:
Requires these PRs to go in first :)
Fixes #12469
Change Type
Changelog
Additional Notes
For testing MCP Authentication with keycloak, apply the
remote-authn-keycloak.yaml,common.yamlandkeycloak.yamlmanifests along with thecurl_pod.yamlfrom the e2e test setup.curlpod you should be able to get a token from keycloak underaccess_tokenand save it asTOKEN:You should no longer see the 401 error!
You can also test this with the MCP inspector:
npx modelcontextprotocol/inspector#0.16.2http://localhost:8080/) without the token using Streamable HTTPTOKENunder the API Token Authentication field, then click Connecttoolstab and test thefetchtool with a URL of your choiceExample without the token:

Example with token:

Follow ups: