feat: plugin gateway route handlers with auto-namespacing#126
Conversation
- Add routes contribution type to plugin schema (contributes.gateway.routes)
- Routes are auto-namespaced under /plugins/{plugin-name}/ to prevent conflicts
- Generator validates route path conflicts at generate time
- Add gateway public_url config field for OAuth callbacks
- Extend gateway SDK with RegisterRoute/MatchRoute + middleware Abort API
- Add toJSON template function to middleware/route rendering
mcp-oauth plugin updates:
- Remove redundant token_file option (derive from token_dir + provider name)
- Add OAuth callback handler at /plugins/mcp-oauth/callback
- Middleware returns 401 + authorize_url when no token exists
- Token files now per-provider: {token_dir}/{provider}.json
Automated Review SummaryAgents: security, correctness, architecture core/plugins/mcp-oauth/handlers/callback.go:100 core/plugins/mcp-oauth/handlers/callback.go:79 core/plugins/mcp-oauth/handlers/callback.go:73 core/plugins/mcp-oauth/middlewares/oauth.go:469 core/plugins/mcp-oauth/middlewares/oauth.go:43 core/plugins/mcp-oauth/handlers/callback.go:343 core/plugins/mcp-oauth/handlers/callback.go:28 internal/generate/v1/generator.go:963 internal/generate/v1/routes.go:101 core/sdk/gateway/middleware.go:593 core/plugins/mcp-oauth/handlers/callback.go:20 |
Automated Review SummaryAgents: security, correctness, edge_cases, architecture core/plugins/mcp-oauth/handlers/callback.go:96 core/plugins/mcp-oauth/handlers/callback.go:80 core/plugins/mcp-oauth/handlers/callback.go:57 core/plugins/mcp-oauth/middlewares/oauth.go:116 core/plugins/mcp-oauth/handlers/callback.go:87 core/plugins/mcp-oauth/middlewares/oauth.go:429 core/plugins/mcp-oauth/middlewares/oauth.go:492 core/plugins/mcp-oauth/handlers/callback.go:31 core/plugins/mcp-oauth/handlers/callback.go:252 internal/generate/v1/routes.go:46 internal/generate/v1/routes.go:108 core/sdk/gateway/middleware.go:593 core/plugins/mcp-oauth/handlers/callback.go:268 |
![dorey-agent[bot]](https://avatars.githubusercontent.com/in/3504508?s=60&v=4){kind=small}
| } | ||
|
|
||
| func handleOAuthCallback(w http.ResponseWriter, r *http.Request) { | ||
| code := r.URL.Query().Get("code") |
There was a problem hiding this comment.
⚪ [unknown] [security] CSRF: state used as provider name instead of a nonce
The state parameter is validated only by checking whether it matches a known provider name. It carries no CSRF protection. RFC 6749 §10.12 requires state to be an unguessable random value tied to the session that initiated the authorization request. As-is, any party that obtains a valid authorization code (e.g., via referrer leakage, log exposure, or interception) can craft:
GET /plugins/mcp-oauth/callback?code=STOLEN&state=notion
and the handler will exchange the code and write the resulting token without any verification that this callback corresponds to a flow this gateway started.
Fix: generate a random nonce on the authorize redirect, store it (e.g., in a short-lived file or in-memory map keyed by nonce), and verify on callback. Encode both nonce and provider name in state (e.g., notion:hex-nonce).
| if v, ok := cfg["client_id"].(string); ok { | ||
| p.ClientID = v | ||
| } | ||
| if v, ok := cfg["scopes"].(string); ok { |
There was a problem hiding this comment.
⚪ [unknown] [correctness] Non-deterministic provider selection
Go map iteration order is randomized at runtime. This loop:
for name := range providers {
defaultProvider = name
break
}picks an arbitrary provider when multiple are configured. The middleware then only handles that one provider — silently ignoring all others. Given that the plugin schema explicitly supports a providers map (multi-provider), this is a correctness gap: a user who configures notion and github will have one of them work and the other silently fail on every process restart, non-deterministically.
For the current single-provider-per-middleware use case, either:
- Validate at generate time that exactly one provider is configured per installation, and error clearly if not, or
- Support multiple providers by matching on request domain → provider mapping.
| // Exchange authorization code for token | ||
| redirectURI := r.URL.Query().Get("redirect_uri") | ||
| if redirectURI == "" { | ||
| // Reconstruct from request |
There was a problem hiding this comment.
⚪ [unknown] [correctness] redirect_uri reconstructed from r.TLS — always wrong behind a proxy
When the gateway runs behind a TLS-terminating load balancer or reverse proxy (the normal production setup), r.TLS is always nil even for HTTPS requests. This produces http://... redirect URIs sent to the OAuth token endpoint, which will:
- Not match the
https://URI registered with the provider → token exchange fails, or - Accidentally expose the code exchange over HTTP if the provider blindly accepts it.
gateway.public_url was added in this very PR for exactly this reason. It should be used here:
redirectURI = gatewayPublicURL + r.URL.Path // baked in via {{ .gateway.public_url }}The query-param redirect_uri override path is also suspicious — the OAuth provider never sends redirect_uri back in the callback; this dead branch should be removed.
| @@ -73,6 +116,18 @@ func init() { | |||
| }) | |||
There was a problem hiding this comment.
⚪ [unknown] [correctness] buildAuthorizeURL omits redirect_uri
redirect_uri is missing from the constructed authorize URL. Many providers (GitHub, Google, Microsoft) require it and will reject the request or use an ambiguous default. More importantly, without an explicit redirect_uri in the authorize request, the token endpoint can't verify the redirect matches — weakening the auth code binding.
The public_url from config should be used to construct the full callback URL:
params.Set("redirect_uri", publicURL+"/plugins/mcp-oauth/callback")This is doubly important given the CSRF issue above — redirect_uri binding is the other defense that limits where auth codes can be redirected.
| oauthCallbackTokenDir = "{{ .options.token_dir }}" | ||
| providersJSON := `{{ toJSON .options.providers }}` | ||
| var providers map[string]map[string]any | ||
| if err := json.Unmarshal([]byte(providersJSON), &providers); err == nil { |
There was a problem hiding this comment.
⚪ [unknown] [edge case] Silent JSON parse failure leaves provider map empty
if err := json.Unmarshal([]byte(providersJSON), &providers); err == nil {
// populate oauthCallbackProviders
}If the template renders invalid JSON (e.g., a provider option contains backtick characters, or toJSON fails silently), json.Unmarshal errors and oauthCallbackProviders stays empty. The init() still runs, gateway.RegisterRoute registers the handler, but every callback returns 400 unknown provider with no diagnostic about why the config failed to parse.
This same pattern exists in middlewares/oauth.go. Both should log a fatal/panic or return an early error on parse failure, since there is no valid runtime state if provider config is unreadable.
| } | ||
|
|
||
| data, err := json.MarshalIndent(stored, "", " ") | ||
| if err != nil { |
There was a problem hiding this comment.
⚪ [unknown] [security] client_secret written to the shared token file
if provider.ClientSecret != "" {
stored["client_secret"] = provider.ClientSecret
}The OAuth client_secret is written into the token JSON file on the shared oauth-tokens volume, which is mounted into both the gateway and agent containers. The secret is already available at runtime via the baked-in template; there's no functional reason to persist it in the token file. The middleware reads client_secret from the storedToken struct for refresh, but it could read from the in-memory config instead.
Storing secrets in token files expands the blast radius: a compromised agent container, an accidental log dump of the token file, or volume backup exposure all leak the secret alongside the token.
| type: object | ||
| required: true | ||
| description: "Map of provider name to MCP config" | ||
| description: "Map of provider name to MCP config (each needs mcp_url, client_id, client_secret)" |
There was a problem hiding this comment.
⚪ [unknown] [regression] Removing token_file is a breaking change without a migration path
The token_file option is removed and the token path is now derived as {token_dir}/{defaultProvider}.json. Existing deployments that:
- Explicitly set
token_file, or - Have a token stored at the old default
/data/oauth-tokens/token.json
will silently fail after upgrading — the middleware will treat the missing file as "not authenticated" and start returning 401s, triggering the OAuth flow from scratch.
At minimum, the changelog and plugin README should document this migration requirement. Ideally, generate-time validation should error if an unknown option (including the removed token_file) is provided, rather than silently ignoring it.
| b, err := json.Marshal(v) | ||
| if err != nil { | ||
| return "", fmt.Errorf("toJSON: %w", err) | ||
| } |
There was a problem hiding this comment.
⚪ [unknown] [architecture] toJSON template function duplicated
The toJSON function is defined identically in both middleware_copy.go:renderMiddleware and routes.go:renderRouteHandler. Extract it to a shared templateFuncMap() helper in this package to keep them in sync. Otherwise a future change (e.g., adding HTML escaping) needs to be made in two places and one will inevitably be missed.
- CSRF: add nonce-based state parameter (single-use, validated on callback) - Path traversal: state is validated against known provider map (no raw file path construction) - redirect_uri: derived from gateway public_url (baked at generate time), never from request headers - redirect_uri included in authorize URL for proper OAuth round-trip - Non-deterministic provider: use sorted keys for deterministic default selection - client_secret: no longer persisted to token files (stays in gateway binary only) - Error logging: JSON parse failures now logged instead of silently swallowed - Error messages: internal details (endpoint URLs, provider errors) no longer leaked to HTTP responses - HTML escaping: provider name escaped in success page - public_url passed to route handler templates via new parameter
- Remove cross-package function calls (GenerateOAuthNonce, OAuthCallbackURL) between handlers/ and middlewares/ — they compile as separate packages in CI - Use deterministic HMAC-based CSRF state derived from providers config (both packages derive the same key independently) - Remove ClientSecret field from storedToken struct (no longer persisted) - Remove unused crypto/rand import
- Gateway SDK: DiscoverOAuthMetadata() + RegisterOAuthClient() utilities
- Middleware auto-detects mode: client_id present → static, absent → dynamic
- Dynamic mode: discovers .well-known/oauth-authorization-server, registers client
- Registration response cached to {token_dir}/{provider}.reg.json for reuse
- Updated README with dual-mode examples (dynamic for Notion, static for custom)
- For Notion/MCP providers: just provide mcp_url, everything else auto-discovered
- Gateway main.go: route handler dispatch on health server (port 8080) - Compose: auto-expose gateway port 8080 when plugin routes are registered - Default public_url to http://localhost:8080 when not configured - local-coding example: add Notion MCP with mcp-oauth plugin (dynamic mode) - README: document Notion OAuth setup flow (zero-config, just mcp_url)
Plugin now generates gateway service entries from providers.mcp_url automatically — users don't need to duplicate the URL in gateway.services. Uses Go template range over providers map to emit service entries at generate time.
Each provider gets its own middleware scoped to its mcp_url domain. Request to mcp.notion.com → notion's authorize URL. Request to mcp.datadog.com → datadog's authorize URL. Previously used a single middleware with a hardcoded default provider, which returned the wrong authorize URL for multi-provider configs.
1 participant
What
Plugins can now register HTTP route handlers on the gateway, auto-namespaced under
/plugins/{plugin-name}/. The mcp-oauth plugin uses this to implement a full OAuth authorization code flow — including dynamic client registration (RFC 7591) for providers like Notion that support it.Why
Previously, plugins could only contribute middleware (request interceptors). There was no way for a plugin to serve direct HTTP endpoints (needed for OAuth callbacks, webhooks). Users had to manually create token files for OAuth providers — terrible UX.
For MCP providers like Notion that use dynamic registration, users shouldn't need to manually create OAuth apps and copy credentials.
How
Gateway SDK:
RegisterRoute(RouteDef)/MatchRoute(path)— route handler registryMiddlewareContext.Abort(status, body)— middleware can short-circuit with a responseDiscoverOAuthMetadata()— fetches.well-known/oauth-authorization-serverRegisterOAuthClient()— performs RFC 7591 dynamic client registrationPlugin schema:
contributes.gateway.routes[]withpathandhandlerfields/plugins/{plugin-name}/at generate timeConfig:
gateway.public_urlfield — used for OAuth callbacksmcp-oauth plugin:
mcp_url): auto-discovers OAuth endpoints + registers clientclient_idprovided): uses given credentials directly/plugins/mcp-oauth/callback(code exchange + token write)Testing
go test ./internal/... ./core/sdk/...New tests: route namespacing, conflict detection, template rendering, SDK route registry, middleware abort API, public_url passthrough.