fix(brain-mcp): static Bearer short-circuits before OAuth challenge
Reorders BearerAuth so a valid BRAIN_MCP_TOKEN match wins instantly and never emits WWW-Authenticate. Adds RFC 9728 resource_metadata challenge header on 401 (only when MCP_RESOURCE_URL is configured) so claude.ai's OAuth-discovery path still works. Why: claude CLI on koala/flamingo with `.mcp.json` `Authorization: Bearer $BRAIN_MCP_TOKEN` was being kicked into RFC 7591 dynamic client registration against Dex (static-only) and dying. Cause was the auth middleware running JWT validation first and emitting an OAuth challenge on the fall-through 401 even when the caller had a valid static token. Inverting the precedence and gating the challenge on resourceMetadataURL keeps the LAN/Tailscale CLI path silent and only invites OAuth discovery on actually-unauthenticated requests. Regression guards in the test file: - valid static Bearer 200 has no WWW-Authenticate - 401 with resourceMetadataURL set carries the challenge - 401 with empty resourceMetadataURL emits no challenge Closes hyperguild#9 in code. Live verification (claude CLI on koala listing brain tools) blocked on ingestion image rebuild + redeploy.
This commit is contained in:
@@ -8,6 +8,7 @@ import (
|
|||||||
"net/http"
|
"net/http"
|
||||||
"os"
|
"os"
|
||||||
"strconv"
|
"strconv"
|
||||||
|
"strings"
|
||||||
"time"
|
"time"
|
||||||
|
|
||||||
"github.com/mathiasbq/hyperguild/ingestion/internal/api"
|
"github.com/mathiasbq/hyperguild/ingestion/internal/api"
|
||||||
@@ -112,13 +113,20 @@ func main() {
|
|||||||
logger.Info("jwt auth enabled", "issuer", dexURL)
|
logger.Info("jwt auth enabled", "issuer", dexURL)
|
||||||
}
|
}
|
||||||
|
|
||||||
mux.Handle("/mcp", mcp.BearerAuth(mcpToken, jwtValidator, mcpSrv))
|
// Resource-metadata URL is only emitted on 401 when Dex OAuth is
|
||||||
|
// configured. Static-Bearer-only deployments leave this empty so
|
||||||
|
// clients never see an OAuth challenge.
|
||||||
|
var resourceMetadataURL string
|
||||||
if dexURL := os.Getenv("DEX_ISSUER_URL"); dexURL != "" {
|
if dexURL := os.Getenv("DEX_ISSUER_URL"); dexURL != "" {
|
||||||
resourceURL := os.Getenv("MCP_RESOURCE_URL")
|
resourceURL := os.Getenv("MCP_RESOURCE_URL")
|
||||||
mux.HandleFunc("GET /.well-known/oauth-protected-resource",
|
mux.HandleFunc("GET /.well-known/oauth-protected-resource",
|
||||||
auth.ProtectedResourceHandler(resourceURL, os.Getenv("DEX_ISSUER_URL")))
|
auth.ProtectedResourceHandler(resourceURL, dexURL))
|
||||||
|
if resourceURL != "" {
|
||||||
|
resourceMetadataURL = strings.TrimRight(resourceURL, "/") + "/.well-known/oauth-protected-resource"
|
||||||
}
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
mux.Handle("/mcp", mcp.BearerAuth(mcpToken, jwtValidator, resourceMetadataURL, mcpSrv))
|
||||||
|
|
||||||
addr := ":" + port
|
addr := ":" + port
|
||||||
watchIntervalLog := "disabled"
|
watchIntervalLog := "disabled"
|
||||||
|
|||||||
@@ -8,29 +8,58 @@ import (
|
|||||||
"github.com/mathiasbq/hyperguild/ingestion/internal/auth"
|
"github.com/mathiasbq/hyperguild/ingestion/internal/auth"
|
||||||
)
|
)
|
||||||
|
|
||||||
// BearerAuth returns a middleware that enforces authentication on every request.
|
// BearerAuth gates an HTTP handler behind dual-mode authentication.
|
||||||
// It tries a valid Dex JWT first (when v is non-nil), then falls back to the
|
//
|
||||||
// static token. Rejects if token is empty and no valid JWT is presented.
|
// Auth precedence:
|
||||||
func BearerAuth(token string, v *auth.Validator, next http.Handler) http.Handler {
|
//
|
||||||
|
// 1. Static Bearer match (constant-time compare against staticToken).
|
||||||
|
// Wins immediately and never emits a WWW-Authenticate header. This is
|
||||||
|
// the path used by internal Tailscale/LAN CLI callers that supply
|
||||||
|
// `Authorization: Bearer $BRAIN_MCP_TOKEN` via `.mcp.json`. Returning
|
||||||
|
// 200 without a WWW-Authenticate prevents the MCP client from
|
||||||
|
// speculatively flipping into OAuth-discovery mode.
|
||||||
|
// 2. Dex JWT validation (when validator is non-nil). Used by claude.ai
|
||||||
|
// custom MCP connectors that finished the OAuth handshake.
|
||||||
|
// 3. Otherwise 401. When resourceMetadataURL is non-empty, a
|
||||||
|
// `WWW-Authenticate: Bearer resource_metadata="…"` header is emitted
|
||||||
|
// per RFC 9728 §6.2 so claude.ai's OAuth discovery flow can find the
|
||||||
|
// server's protected-resource metadata document.
|
||||||
|
//
|
||||||
|
// The order matters: a valid static Bearer must short-circuit BEFORE any
|
||||||
|
// JWT path runs, because a non-empty WWW-Authenticate emitted on the
|
||||||
|
// fall-through 401 confuses static-Bearer-only clients into discarding
|
||||||
|
// their header and starting an OAuth handshake instead.
|
||||||
|
func BearerAuth(staticToken string, validator *auth.Validator, resourceMetadataURL string, next http.Handler) http.Handler {
|
||||||
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||||
rawToken, ok := strings.CutPrefix(r.Header.Get("Authorization"), "Bearer ")
|
rawToken, ok := strings.CutPrefix(r.Header.Get("Authorization"), "Bearer ")
|
||||||
if !ok {
|
if !ok {
|
||||||
http.Error(w, "unauthorized", http.StatusUnauthorized)
|
unauthorized(w, resourceMetadataURL)
|
||||||
return
|
return
|
||||||
}
|
}
|
||||||
|
|
||||||
if v != nil {
|
// 1. Static Bearer wins first — never emits a challenge.
|
||||||
if _, err := v.Validate(r.Context(), rawToken); err == nil {
|
if staticToken != "" && subtle.ConstantTimeCompare([]byte(rawToken), []byte(staticToken)) == 1 {
|
||||||
|
next.ServeHTTP(w, r)
|
||||||
|
return
|
||||||
|
}
|
||||||
|
|
||||||
|
// 2. Then Dex JWT, if configured.
|
||||||
|
if validator != nil {
|
||||||
|
if _, err := validator.Validate(r.Context(), rawToken); err == nil {
|
||||||
next.ServeHTTP(w, r)
|
next.ServeHTTP(w, r)
|
||||||
return
|
return
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
if token != "" && subtle.ConstantTimeCompare([]byte(rawToken), []byte(token)) == 1 {
|
// 3. Reject with an OAuth resource-metadata challenge if configured.
|
||||||
next.ServeHTTP(w, r)
|
unauthorized(w, resourceMetadataURL)
|
||||||
return
|
|
||||||
}
|
|
||||||
|
|
||||||
http.Error(w, "unauthorized", http.StatusUnauthorized)
|
|
||||||
})
|
})
|
||||||
}
|
}
|
||||||
|
|
||||||
|
func unauthorized(w http.ResponseWriter, resourceMetadataURL string) {
|
||||||
|
if resourceMetadataURL != "" {
|
||||||
|
w.Header().Set("WWW-Authenticate",
|
||||||
|
`Bearer realm="brain", resource_metadata="`+resourceMetadataURL+`"`)
|
||||||
|
}
|
||||||
|
http.Error(w, "unauthorized", http.StatusUnauthorized)
|
||||||
|
}
|
||||||
|
|||||||
@@ -19,6 +19,8 @@ import (
|
|||||||
"github.com/stretchr/testify/require"
|
"github.com/stretchr/testify/require"
|
||||||
)
|
)
|
||||||
|
|
||||||
|
const testResourceMetadataURL = "https://brain-mcp.d-ma.be/.well-known/oauth-protected-resource"
|
||||||
|
|
||||||
func okHandler() http.Handler {
|
func okHandler() http.Handler {
|
||||||
return http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
|
return http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
|
||||||
w.WriteHeader(http.StatusOK)
|
w.WriteHeader(http.StatusOK)
|
||||||
@@ -26,7 +28,7 @@ func okHandler() http.Handler {
|
|||||||
}
|
}
|
||||||
|
|
||||||
func TestBearerAuth_MissingHeader(t *testing.T) {
|
func TestBearerAuth_MissingHeader(t *testing.T) {
|
||||||
handler := mcp.BearerAuth("secret", nil, okHandler())
|
handler := mcp.BearerAuth("secret", nil, "", okHandler())
|
||||||
req := httptest.NewRequest(http.MethodPost, "/mcp", nil)
|
req := httptest.NewRequest(http.MethodPost, "/mcp", nil)
|
||||||
rr := httptest.NewRecorder()
|
rr := httptest.NewRecorder()
|
||||||
handler.ServeHTTP(rr, req)
|
handler.ServeHTTP(rr, req)
|
||||||
@@ -34,7 +36,7 @@ func TestBearerAuth_MissingHeader(t *testing.T) {
|
|||||||
}
|
}
|
||||||
|
|
||||||
func TestBearerAuth_WrongToken(t *testing.T) {
|
func TestBearerAuth_WrongToken(t *testing.T) {
|
||||||
handler := mcp.BearerAuth("secret", nil, okHandler())
|
handler := mcp.BearerAuth("secret", nil, "", okHandler())
|
||||||
req := httptest.NewRequest(http.MethodPost, "/mcp", nil)
|
req := httptest.NewRequest(http.MethodPost, "/mcp", nil)
|
||||||
req.Header.Set("Authorization", "Bearer wrong")
|
req.Header.Set("Authorization", "Bearer wrong")
|
||||||
rr := httptest.NewRecorder()
|
rr := httptest.NewRecorder()
|
||||||
@@ -44,7 +46,7 @@ func TestBearerAuth_WrongToken(t *testing.T) {
|
|||||||
|
|
||||||
func TestBearerAuth_CorrectToken(t *testing.T) {
|
func TestBearerAuth_CorrectToken(t *testing.T) {
|
||||||
called := false
|
called := false
|
||||||
handler := mcp.BearerAuth("secret", nil, http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
|
handler := mcp.BearerAuth("secret", nil, "", http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
|
||||||
called = true
|
called = true
|
||||||
w.WriteHeader(http.StatusOK)
|
w.WriteHeader(http.StatusOK)
|
||||||
}))
|
}))
|
||||||
@@ -57,13 +59,52 @@ func TestBearerAuth_CorrectToken(t *testing.T) {
|
|||||||
}
|
}
|
||||||
|
|
||||||
func TestBearerAuth_EmptyConfiguredToken(t *testing.T) {
|
func TestBearerAuth_EmptyConfiguredToken(t *testing.T) {
|
||||||
handler := mcp.BearerAuth("", nil, okHandler())
|
handler := mcp.BearerAuth("", nil, "", okHandler())
|
||||||
req := httptest.NewRequest(http.MethodPost, "/mcp", nil)
|
req := httptest.NewRequest(http.MethodPost, "/mcp", nil)
|
||||||
rr := httptest.NewRecorder()
|
rr := httptest.NewRecorder()
|
||||||
handler.ServeHTTP(rr, req)
|
handler.ServeHTTP(rr, req)
|
||||||
assert.Equal(t, http.StatusUnauthorized, rr.Code)
|
assert.Equal(t, http.StatusUnauthorized, rr.Code)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Issue #9: a valid static Bearer must never emit a WWW-Authenticate header,
|
||||||
|
// even when a resource-metadata URL is configured. The presence of that
|
||||||
|
// header on a 200 response would flip MCP CLI clients into OAuth-discovery
|
||||||
|
// mode and break static-Bearer auth from `.mcp.json` on Tailscale/LAN.
|
||||||
|
func TestBearerAuth_ValidStaticBearer_NoWWWAuthenticate(t *testing.T) {
|
||||||
|
handler := mcp.BearerAuth("secret", nil, testResourceMetadataURL, okHandler())
|
||||||
|
req := httptest.NewRequest(http.MethodPost, "/mcp", nil)
|
||||||
|
req.Header.Set("Authorization", "Bearer secret")
|
||||||
|
rr := httptest.NewRecorder()
|
||||||
|
handler.ServeHTTP(rr, req)
|
||||||
|
assert.Equal(t, http.StatusOK, rr.Code)
|
||||||
|
assert.Empty(t, rr.Header().Get("WWW-Authenticate"), "static-Bearer 200 must not advertise OAuth")
|
||||||
|
}
|
||||||
|
|
||||||
|
// Issue #9: a 401 with resource-metadata configured must emit a
|
||||||
|
// WWW-Authenticate header so claude.ai discovers the protected-resource
|
||||||
|
// metadata document and continues the OAuth dance.
|
||||||
|
func TestBearerAuth_Unauthorized_EmitsResourceMetadataChallenge(t *testing.T) {
|
||||||
|
handler := mcp.BearerAuth("secret", nil, testResourceMetadataURL, okHandler())
|
||||||
|
req := httptest.NewRequest(http.MethodPost, "/mcp", nil)
|
||||||
|
rr := httptest.NewRecorder()
|
||||||
|
handler.ServeHTTP(rr, req)
|
||||||
|
assert.Equal(t, http.StatusUnauthorized, rr.Code)
|
||||||
|
got := rr.Header().Get("WWW-Authenticate")
|
||||||
|
assert.Contains(t, got, `Bearer realm="brain"`)
|
||||||
|
assert.Contains(t, got, `resource_metadata="`+testResourceMetadataURL+`"`)
|
||||||
|
}
|
||||||
|
|
||||||
|
// Static-Bearer-only deployment: no resource-metadata URL, no challenge
|
||||||
|
// header on 401 — matches pre-#9 behaviour for tests without Dex wired.
|
||||||
|
func TestBearerAuth_Unauthorized_NoChallengeWhenResourceUnset(t *testing.T) {
|
||||||
|
handler := mcp.BearerAuth("secret", nil, "", okHandler())
|
||||||
|
req := httptest.NewRequest(http.MethodPost, "/mcp", nil)
|
||||||
|
rr := httptest.NewRecorder()
|
||||||
|
handler.ServeHTTP(rr, req)
|
||||||
|
assert.Equal(t, http.StatusUnauthorized, rr.Code)
|
||||||
|
assert.Empty(t, rr.Header().Get("WWW-Authenticate"))
|
||||||
|
}
|
||||||
|
|
||||||
// JWT auth tests
|
// JWT auth tests
|
||||||
|
|
||||||
func buildOIDCServer(t *testing.T) (*httptest.Server, jwk.Key) {
|
func buildOIDCServer(t *testing.T) (*httptest.Server, jwk.Key) {
|
||||||
@@ -116,7 +157,7 @@ func TestBearerAuth_ValidJWT(t *testing.T) {
|
|||||||
require.NoError(t, err)
|
require.NoError(t, err)
|
||||||
|
|
||||||
called := false
|
called := false
|
||||||
handler := mcp.BearerAuth("static-secret", v, http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
|
handler := mcp.BearerAuth("static-secret", v, "", http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
|
||||||
called = true
|
called = true
|
||||||
w.WriteHeader(http.StatusOK)
|
w.WriteHeader(http.StatusOK)
|
||||||
}))
|
}))
|
||||||
@@ -135,7 +176,7 @@ func TestBearerAuth_InvalidJWT_FallsBackToStaticToken(t *testing.T) {
|
|||||||
v, err := auth.NewValidator(oidcSrv.URL, "brain")
|
v, err := auth.NewValidator(oidcSrv.URL, "brain")
|
||||||
require.NoError(t, err)
|
require.NoError(t, err)
|
||||||
|
|
||||||
handler := mcp.BearerAuth("static-secret", v, okHandler())
|
handler := mcp.BearerAuth("static-secret", v, "", okHandler())
|
||||||
req := httptest.NewRequest(http.MethodPost, "/mcp", nil)
|
req := httptest.NewRequest(http.MethodPost, "/mcp", nil)
|
||||||
req.Header.Set("Authorization", "Bearer static-secret")
|
req.Header.Set("Authorization", "Bearer static-secret")
|
||||||
rr := httptest.NewRecorder()
|
rr := httptest.NewRecorder()
|
||||||
@@ -148,7 +189,7 @@ func TestBearerAuth_InvalidJWT_WrongStaticToken(t *testing.T) {
|
|||||||
v, err := auth.NewValidator(oidcSrv.URL, "brain")
|
v, err := auth.NewValidator(oidcSrv.URL, "brain")
|
||||||
require.NoError(t, err)
|
require.NoError(t, err)
|
||||||
|
|
||||||
handler := mcp.BearerAuth("static-secret", v, okHandler())
|
handler := mcp.BearerAuth("static-secret", v, "", okHandler())
|
||||||
// Expired JWT — JWT fails, static token doesn't match either
|
// Expired JWT — JWT fails, static token doesn't match either
|
||||||
token := signJWT(t, priv, oidcSrv.URL, "brain", time.Now().Add(-time.Hour))
|
token := signJWT(t, priv, oidcSrv.URL, "brain", time.Now().Add(-time.Hour))
|
||||||
req := httptest.NewRequest(http.MethodPost, "/mcp", nil)
|
req := httptest.NewRequest(http.MethodPost, "/mcp", nil)
|
||||||
|
|||||||
Reference in New Issue
Block a user