Introduce OIDC token verification if authorization-url is specified#176
Introduce OIDC token verification if authorization-url is specified#176manusa merged 5 commits intocontainers:mainfrom
Conversation
| return s.k.GetAPIServerHost() | ||
| } | ||
|
|
||
| func (s *Server) GetOIDCProvider() *oidc.Provider { |
There was a problem hiding this comment.
OIDCProvider is exported. Maybe we don't need this function.
There was a problem hiding this comment.
we can clean up everything in a follow-up PR once usage is clarified
| } | ||
|
|
||
| // Scopes are likely to be used for authorization. | ||
| scopes := claims.GetScopes() |
There was a problem hiding this comment.
This is the foundational work for scoped based authorization. This can also be used for logging.
There was a problem hiding this comment.
This will probably need clarification and an enumeration of different scenarios in a specific issue or user story
There was a problem hiding this comment.
Agree. Once we have a clear picture about the role of the scopes, we'll need this ^^
ardaguclu
force-pushed
the
oidc-verification-2
branch
from
da2486a to
e96cda1
Compare
|
Just a summary of what the flow looks like if
|
| // validateJWTToken validates basic JWT claims without signature verification | ||
| func validateJWTToken(token, audience string) error { | ||
| func (c *JWTClaims) ContainsAudience(audience string) bool { | ||
| switch aud := c.Audience.(type) { |
There was a problem hiding this comment.
audience is relatively arbitrary fields. So that we have to support different data structures.
|
@manusa this PR is ready for review. Regardless of the ways we decide to support in terms of auth, the changes in this PR will likely be relevant to keep us OAuth compliant. |
| var oidcProvider *oidc.Provider | ||
| if m.StaticConfig.AuthorizationURL != "" { | ||
| provider, err := oidc.NewProvider(context.TODO(), m.StaticConfig.AuthorizationURL) | ||
| if err != nil { | ||
| return fmt.Errorf("unable to setup OIDC provider: %w", err) | ||
| } | ||
| oidcProvider = provider | ||
| } |
There was a problem hiding this comment.
It seems that oidc.Provider is basically passed along to the mcp.Server instance and in turn is then passed on to the http AuthorizationMiddleware function.
Eventually there's also a call to VerifyToken which in turn performs a call to the Kubernetes VerifyToken function.
There seems to be a lot of coupling here that might be possible to extract.
Since the http package is providing auth, IMO mcp should have nothing else to do in this area.
Anyway, this should be done after merging this PR and only if it's possible.
There was a problem hiding this comment.
yes, that is correct that there are a couple of tight couplings. I'm open to improve this.
Maybe we can directly pass oidcprovider instead of carrying via McpServer. That would be better. VerifyToken needs to access kubernetes which is managed in McpServer, this one might be difficult.
There was a problem hiding this comment.
With respect to VerifyToken, I'm open to suggestions.
There was a problem hiding this comment.
I appreciate it. Thank you.
2 participants
This PR is revamped version of #172.
If Authorization URL is set, that means MCP Server is asked to validate the tokens from the given issuer url.
This PR adds support this. Currently, the expectation is to get one token regardless of the authorization url is specified or not. And this token is used against API Server after validated.