refactor: consolidate MCP OAuth handlers and add Area51 DCR support (#4679)

DavidS-ovm · actions-user · commit 43f43338d292 · 2026-04-16T10:53:56.000Z
Deduplicate OAuth metadata (RFC 8414), Dynamic Client Registration (RFC 7591), and Protected Resource Metadata (RFC 9728) handlers from customermcp into shared go/auth package. Add Area51-specific OAuth metadata and DCR endpoints so MCP clients auto-discover the client_id without hardcoded auth blocks in .cursor/mcp.json. Redirect URI filtering in DCR restricts echoed URIs to localhost-only per RFC 8252, preventing open-redirect via registration responses.  --- > [!NOTE] > **Medium Risk** > Adds new OAuth metadata and Dynamic Client Registration endpoints for Area51 and refactors MCP discovery responses, which affects OAuth client discovery flows and could break MCP authentication if misconfigured. > > **Overview** > Enables MCP clients (e.g. Cursor) to **auto-discover OAuth configuration and `client_id`** for the Area51 MCP endpoint by adding RFC 8414 Authorization Server Metadata and RFC 7591 Dynamic Client Registration routes, and wiring them into `api-server` startup and routing. > > Consolidates MCP OAuth/PRM handler implementations into shared `go/auth` helpers (`NewMCPOAuthMetadataHandler`, `NewMCPDCRHandler`, `NewMCPPRMHandler`) and updates both Area51 and customer MCP code to use them; PRM responses no longer include `client_id` and DCR responses now *filter redirect URIs to localhost only*. > > Updates local dev proxying and config: nginx now forwards `/.well-known/oauth-authorization-server/area51/oauth` to the api-server, devcontainer envs add `API_SERVER_MCP_CLIENT_ID`, and `.cursor/mcp.json` removes hardcoded Area51 OAuth `CLIENT_ID` blocks. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit bcb15bc9d0cddc8a8b43923abf2d2d3b1a898421. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup>  GitOrigin-RevId: 86ea4467c7f28631dfde437f2fe22c3c58cc78ca
diff --git a/go/auth/mcpoauth.go b/go/auth/mcpoauth.go
@@ -0,0 +1,145 @@
+package auth
+
+import (
+	"encoding/json"
+	"fmt"
+	"net/http"
+	"net/url"
+)
+
+// NewMCPOAuthMetadataHandler returns an HTTP handler that serves OAuth 2.0
+// Authorization Server Metadata (RFC 8414) for an MCP endpoint.
+//
+// Instead of proxying Auth0's metadata at runtime, it constructs a static
+// document that points authorization_endpoint and token_endpoint to Auth0
+// while advertising our own registration_endpoint for Dynamic Client
+// Registration (RFC 7591). This lets MCP clients like Cursor discover
+// the client_id automatically without any user configuration.
+//
+// scopes should include both the standard OIDC scopes and any
+// application-specific scopes (e.g. "admin:read", "changes:read").
+func NewMCPOAuthMetadataHandler(auth0Domain, issuerURL, registrationEndpointURL string, scopes []string) http.Handler {
+	metadata := map[string]any{
+		"issuer":                 issuerURL,
+		"authorization_endpoint": fmt.Sprintf("https://%s/authorize", auth0Domain),
+		"token_endpoint":         fmt.Sprintf("https://%s/oauth/token", auth0Domain),
+		"registration_endpoint":  registrationEndpointURL,
+
+		"jwks_uri":            fmt.Sprintf("https://%s/.well-known/jwks.json", auth0Domain),
+		"userinfo_endpoint":   fmt.Sprintf("https://%s/userinfo", auth0Domain),
+		"revocation_endpoint": fmt.Sprintf("https://%s/oauth/revoke", auth0Domain),
+
+		"response_types_supported":              []string{"code"},
+		"grant_types_supported":                 []string{"authorization_code", "refresh_token"},
+		"code_challenge_methods_supported":      []string{"S256"},
+		"token_endpoint_auth_methods_supported": []string{"none"},
+		"scopes_supported":                      scopes,
+	}
+
+	body, _ := json.Marshal(metadata) //nolint:errchkjson // static map of strings/slices, cannot fail
+
+	return http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
+		w.Header().Set("Content-Type", "application/json")
+		_, _ = w.Write(body)
+	})
+}
+
+// NewMCPDCRHandler returns an HTTP handler that implements a minimal OAuth 2.0
+// Dynamic Client Registration (RFC 7591) endpoint. It always returns the
+// same pre-configured Auth0 client_id since all MCP clients share a single
+// public OAuth application.
+//
+// Per RFC 7591 Section 3.2, the response echoes back the registered client
+// metadata including redirect_uris from the request.
+func NewMCPDCRHandler(clientID string) http.Handler {
+	type dcrRequest struct {
+		RedirectURIs []string `json:"redirect_uris"`
+		ClientName   string   `json:"client_name"`
+	}
+
+	type dcrResponse struct {
+		ClientID                string   `json:"client_id"`
+		RedirectURIs            []string `json:"redirect_uris"`
+		ClientName              string   `json:"client_name,omitempty"`
+		TokenEndpointAuthMethod string   `json:"token_endpoint_auth_method"`
+	}
+
+	return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		if r.Method != http.MethodPost {
+			http.Error(w, "method not allowed", http.StatusMethodNotAllowed)
+			return
+		}
+
+		limited := http.MaxBytesReader(w, r.Body, 64<<10)
+		var req dcrRequest
+		if err := json.NewDecoder(limited).Decode(&req); err != nil {
+			req = dcrRequest{}
+		}
+		_ = limited.Close()
+
+		// Don't echo back arbitrary redirect_uris; Auth0 enforces the
+		// registered set during token exchange, but echoing unchecked URIs
+		// could mislead clients that trust the DCR response blindly.
+		// Instead, return only localhost URIs which are the standard
+		// callback for native/public OAuth clients per RFC 8252.
+		safeURIs := make([]string, 0, len(req.RedirectURIs))
+		for _, uri := range req.RedirectURIs {
+			if IsLocalhostRedirect(uri) {
+				safeURIs = append(safeURIs, uri)
+			}
+		}
+
+		resp := dcrResponse{
+			ClientID:                clientID,
+			RedirectURIs:            safeURIs,
+			ClientName:              req.ClientName,
+			TokenEndpointAuthMethod: "none",
+		}
+
+		w.Header().Set("Content-Type", "application/json")
+		w.WriteHeader(http.StatusCreated)
+		_ = json.NewEncoder(w).Encode(resp)
+	})
+}
+
+// NewMCPPRMHandler returns an http.Handler that serves the OAuth 2.0 Protected
+// Resource Metadata (RFC 9728) JSON document for an MCP endpoint. No
+// authentication is required.
+//
+// authorizationServerURL is the issuer URL of the OAuth metadata endpoint (not
+// the raw Auth0 domain). MCP clients use this to discover the authorization
+// and token endpoints, as well as the Dynamic Client Registration endpoint.
+func NewMCPPRMHandler(authorizationServerURL, resourceURL string, scopes []string) http.Handler {
+	type prmResponse struct {
+		Resource               string   `json:"resource"`
+		AuthorizationServers   []string `json:"authorization_servers"`
+		ScopesSupported        []string `json:"scopes_supported"`
+		BearerMethodsSupported []string `json:"bearer_methods_supported"`
+	}
+
+	resp := prmResponse{
+		Resource:               resourceURL,
+		AuthorizationServers:   []string{authorizationServerURL},
+		ScopesSupported:        scopes,
+		BearerMethodsSupported: []string{"header"},
+	}
+
+	body, _ := json.Marshal(resp) //nolint:errchkjson // static struct of strings, cannot fail
+
+	return http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
+		w.Header().Set("Content-Type", "application/json")
+		w.WriteHeader(http.StatusOK)
+		_, _ = w.Write(body)
+	})
+}
+
+// IsLocalhostRedirect returns true if the URI is a loopback redirect, which is
+// the standard callback for native/public OAuth clients (RFC 8252 Section 7.3).
+func IsLocalhostRedirect(raw string) bool {
+	u, err := url.Parse(raw)
+	if err != nil {
+		return false
+	}
+	host := u.Hostname()
+	return host == "127.0.0.1" || host == "::1" || host == "localhost"
+}
diff --git a/go/auth/mcpoauth_test.go b/go/auth/mcpoauth_test.go
@@ -0,0 +1,198 @@
+package auth
+
+import (
+	"encoding/json"
+	"net/http"
+	"net/http/httptest"
+	"strings"
+	"testing"
+)
+
+func TestNewMCPOAuthMetadataHandler(t *testing.T) {
+	scopes := []string{"openid", "profile", "email", "offline_access", "admin:read"}
+	handler := NewMCPOAuthMetadataHandler(
+		"auth.example.com",
+		"https://api.example.com/area51/oauth",
+		"https://api.example.com/area51/oauth/register",
+		scopes,
+	)
+
+	req := httptest.NewRequestWithContext(t.Context(), http.MethodGet, "/.well-known/oauth-authorization-server/area51/oauth", nil)
+	rec := httptest.NewRecorder()
+	handler.ServeHTTP(rec, req)
+
+	if rec.Code != http.StatusOK {
+		t.Fatalf("expected 200, got %d", rec.Code)
+	}
+
+	var body map[string]any
+	if err := json.NewDecoder(rec.Body).Decode(&body); err != nil {
+		t.Fatalf("failed to decode metadata: %v", err)
+	}
+
+	if body["issuer"] != "https://api.example.com/area51/oauth" {
+		t.Errorf("unexpected issuer: %v", body["issuer"])
+	}
+	if body["authorization_endpoint"] != "https://auth.example.com/authorize" {
+		t.Errorf("unexpected authorization_endpoint: %v", body["authorization_endpoint"])
+	}
+	if body["token_endpoint"] != "https://auth.example.com/oauth/token" {
+		t.Errorf("unexpected token_endpoint: %v", body["token_endpoint"])
+	}
+	if body["registration_endpoint"] != "https://api.example.com/area51/oauth/register" {
+		t.Errorf("unexpected registration_endpoint: %v", body["registration_endpoint"])
+	}
+	if body["jwks_uri"] != "https://auth.example.com/.well-known/jwks.json" {
+		t.Errorf("unexpected jwks_uri: %v", body["jwks_uri"])
+	}
+
+	scopesAny, ok := body["scopes_supported"].([]any)
+	if !ok {
+		t.Fatalf("scopes_supported is not an array: %T", body["scopes_supported"])
+	}
+	if len(scopesAny) != len(scopes) {
+		t.Errorf("expected %d scopes, got %d", len(scopes), len(scopesAny))
+	}
+}
+
+func TestNewMCPDCRHandler(t *testing.T) {
+	handler := NewMCPDCRHandler("test-client-id")
+
+	reqBody := `{"redirect_uris":["http://127.0.0.1/callback"],"client_name":"Test Client"}`
+	req := httptest.NewRequestWithContext(t.Context(), http.MethodPost, "/area51/oauth/register", strings.NewReader(reqBody))
+	req.Header.Set("Content-Type", "application/json")
+	rec := httptest.NewRecorder()
+	handler.ServeHTTP(rec, req)
+
+	if rec.Code != http.StatusCreated {
+		t.Fatalf("expected 201, got %d", rec.Code)
+	}
+
+	var body struct {
+		ClientID                string   `json:"client_id"`
+		RedirectURIs            []string `json:"redirect_uris"`
+		ClientName              string   `json:"client_name"`
+		TokenEndpointAuthMethod string   `json:"token_endpoint_auth_method"`
+	}
+	if err := json.NewDecoder(rec.Body).Decode(&body); err != nil {
+		t.Fatalf("failed to decode DCR response: %v", err)
+	}
+
+	if body.ClientID != "test-client-id" {
+		t.Errorf("unexpected client_id: %q", body.ClientID)
+	}
+	if body.TokenEndpointAuthMethod != "none" {
+		t.Errorf("unexpected token_endpoint_auth_method: %q", body.TokenEndpointAuthMethod)
+	}
+	if len(body.RedirectURIs) != 1 || body.RedirectURIs[0] != "http://127.0.0.1/callback" {
+		t.Errorf("unexpected redirect_uris: %v", body.RedirectURIs)
+	}
+	if body.ClientName != "Test Client" {
+		t.Errorf("unexpected client_name: %q", body.ClientName)
+	}
+}
+
+func TestNewMCPDCRHandler_MethodNotAllowed(t *testing.T) {
+	handler := NewMCPDCRHandler("test-client-id")
+
+	req := httptest.NewRequestWithContext(t.Context(), http.MethodGet, "/area51/oauth/register", nil)
+	rec := httptest.NewRecorder()
+	handler.ServeHTTP(rec, req)
+
+	if rec.Code != http.StatusMethodNotAllowed {
+		t.Fatalf("expected 405, got %d", rec.Code)
+	}
+}
+
+func TestNewMCPDCRHandler_FiltersNonLocalhostRedirects(t *testing.T) {
+	handler := NewMCPDCRHandler("test-client-id")
+
+	reqBody := `{"redirect_uris":["http://127.0.0.1/callback","https://evil.com/callback","http://localhost:3000/callback"]}`
+	req := httptest.NewRequestWithContext(t.Context(), http.MethodPost, "/register", strings.NewReader(reqBody))
+	req.Header.Set("Content-Type", "application/json")
+	rec := httptest.NewRecorder()
+	handler.ServeHTTP(rec, req)
+
+	if rec.Code != http.StatusCreated {
+		t.Fatalf("expected 201, got %d", rec.Code)
+	}
+
+	var body struct {
+		RedirectURIs []string `json:"redirect_uris"`
+	}
+	if err := json.NewDecoder(rec.Body).Decode(&body); err != nil {
+		t.Fatalf("failed to decode: %v", err)
+	}
+
+	if len(body.RedirectURIs) != 2 {
+		t.Fatalf("expected 2 safe redirect URIs, got %d: %v", len(body.RedirectURIs), body.RedirectURIs)
+	}
+}
+
+func TestNewMCPPRMHandler(t *testing.T) {
+	handler := NewMCPPRMHandler(
+		"https://api.example.com/area51/oauth",
+		"https://api.example.com/area51/mcp",
+		[]string{"admin:read"},
+	)
+
+	req := httptest.NewRequestWithContext(t.Context(), http.MethodGet, "/.well-known/oauth-protected-resource/area51/mcp", nil)
+	rec := httptest.NewRecorder()
+	handler.ServeHTTP(rec, req)
+
+	if rec.Code != http.StatusOK {
+		t.Fatalf("expected 200, got %d", rec.Code)
+	}
+
+	if ct := rec.Header().Get("Content-Type"); ct != "application/json" {
+		t.Errorf("expected Content-Type application/json, got %q", ct)
+	}
+
+	var body struct {
+		Resource               string   `json:"resource"`
+		AuthorizationServers   []string `json:"authorization_servers"`
+		ScopesSupported        []string `json:"scopes_supported"`
+		BearerMethodsSupported []string `json:"bearer_methods_supported"`
+		ClientID               string   `json:"client_id"`
+	}
+	if err := json.NewDecoder(rec.Body).Decode(&body); err != nil {
+		t.Fatalf("failed to decode PRM response: %v", err)
+	}
+
+	if body.Resource != "https://api.example.com/area51/mcp" {
+		t.Errorf("unexpected resource: %q", body.Resource)
+	}
+	if len(body.AuthorizationServers) != 1 || body.AuthorizationServers[0] != "https://api.example.com/area51/oauth" {
+		t.Errorf("unexpected authorization_servers: %v", body.AuthorizationServers)
+	}
+	if len(body.ScopesSupported) != 1 || body.ScopesSupported[0] != "admin:read" {
+		t.Errorf("unexpected scopes_supported: %v", body.ScopesSupported)
+	}
+	if len(body.BearerMethodsSupported) != 1 || body.BearerMethodsSupported[0] != "header" {
+		t.Errorf("unexpected bearer_methods_supported: %v", body.BearerMethodsSupported)
+	}
+	if body.ClientID != "" {
+		t.Errorf("expected no client_id in PRM, got %q", body.ClientID)
+	}
+}
+
+func TestIsLocalhostRedirect(t *testing.T) {
+	tests := []struct {
+		uri  string
+		want bool
+	}{
+		{"http://127.0.0.1/callback", true},
+		{"http://localhost:3000/callback", true},
+		{"http://[::1]:8080/callback", true},
+		{"https://evil.com/callback", false},
+		{"https://example.com", false},
+		{"not-a-url", false},
+	}
+
+	for _, tt := range tests {
+		got := IsLocalhostRedirect(tt.uri)
+		if got != tt.want {
+			t.Errorf("IsLocalhostRedirect(%q) = %v, want %v", tt.uri, got, tt.want)
+		}
+	}
+}
