From 379580046191e931c0761b19310c0affb2054af3 Mon Sep 17 00:00:00 2001 From: Mathias Date: Tue, 12 May 2026 14:44:38 +0200 Subject: [PATCH] fix(auth): require Bearer on /mcp regardless of DefaultToken MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Previously BearerMiddleware allowed requests with no Authorization header to pass through whenever GITEA_MCP_DEFAULT_TOKEN was set. The intent was "fall back to the service PAT for upstream Gitea calls," but the side effect was that anyone could hit /mcp anonymously and the server would happily proxy requests as the service account. Drop that path. Auth on /mcp now requires either: - a valid Dex-issued JWT, or - a Bearer matching GITEA_MCP_STATIC_TOKEN. The Gitea service PAT (GITEA_MCP_DEFAULT_TOKEN) is no longer wired into BearerMiddleware at all — it stays an upstream-client concern, used by gitea.NewClient for outbound API calls only. This decouples "can this caller invoke a tool" from "what credentials does the tool use against Gitea". Tests updated: drop the NoAuthHeader_WithDefault permissive case, add NoAuthHeader_RejectsEvenWhenStaticConfigured to lock in the new behavior. Closes part of mathias/infra#2. Co-Authored-By: Claude Opus 4.7 (1M context) --- cmd/gitea-mcp/main.go | 2 +- internal/auth/bearer.go | 27 ++++++++------- internal/auth/bearer_test.go | 65 ++++++++++++++++-------------------- 3 files changed, 43 insertions(+), 51 deletions(-) diff --git a/cmd/gitea-mcp/main.go b/cmd/gitea-mcp/main.go index 13ef665..647a0d9 100644 --- a/cmd/gitea-mcp/main.go +++ b/cmd/gitea-mcp/main.go @@ -68,7 +68,7 @@ func main() { mux := http.NewServeMux() mux.Handle("/mcp", mcp.OriginAllowlist(cfg.OriginAllowlist)( - auth.BearerMiddleware(jwtValidator, cfg.StaticToken, cfg.DefaultToken, + auth.BearerMiddleware(jwtValidator, cfg.StaticToken, auth.CallerMiddleware(mcpSrv), ), )) diff --git a/internal/auth/bearer.go b/internal/auth/bearer.go index 415a157..55d5901 100644 --- a/internal/auth/bearer.go +++ b/internal/auth/bearer.go @@ -6,24 +6,23 @@ import ( "strings" ) -// BearerMiddleware authenticates requests via one of three paths (in order): +// BearerMiddleware authenticates requests via the Authorization header. // -// 1. Bearer token is a valid JWT issued by the configured Dex OIDC server. -// 2. Bearer token matches staticToken (constant-time compare). -// 3. No Authorization header and defaultToken is set — allow through; the -// Gitea client will use its service PAT for upstream calls. +// A request is allowed when: // -// Any other case returns 401. -func BearerMiddleware(jwtValidator *JWTValidator, staticToken, defaultToken string, next http.Handler) http.Handler { +// 1. The Bearer token is a valid JWT issued by the configured Dex OIDC server, or +// 2. The Bearer token matches staticToken (constant-time compare). +// +// Any other case — including missing or empty Authorization header — returns 401. +// +// The Gitea service PAT is intentionally NOT used to authenticate the caller: +// it is only used by the Gitea client for upstream API calls. Decoupling the +// two prevents the MCP endpoint from being reachable anonymously when a service +// PAT happens to be configured. +func BearerMiddleware(jwtValidator *JWTValidator, staticToken string, next http.Handler) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { bearer, hasBearer := strings.CutPrefix(r.Header.Get("Authorization"), "Bearer ") - hasBearer = hasBearer && bearer != "" - - if !hasBearer { - if defaultToken != "" { - next.ServeHTTP(w, r) - return - } + if !hasBearer || bearer == "" { http.Error(w, "unauthorized", http.StatusUnauthorized) return } diff --git a/internal/auth/bearer_test.go b/internal/auth/bearer_test.go index 426a2fb..ddc1d92 100644 --- a/internal/auth/bearer_test.go +++ b/internal/auth/bearer_test.go @@ -10,17 +10,17 @@ import ( "github.com/stretchr/testify/require" ) -// helper: BearerMiddleware with no JWT validator and no static token -func noJWTMiddleware(defaultToken string, next http.Handler) http.Handler { - return auth.BearerMiddleware(nil, "", defaultToken, next) +func okHandler(called *bool) http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + if called != nil { + *called = true + } + w.WriteHeader(http.StatusOK) + }) } -func TestBearerMiddleware_NoAuthHeader_NoDefault(t *testing.T) { - srv := httptest.NewServer(noJWTMiddleware("", - http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { - w.WriteHeader(http.StatusOK) - }), - )) +func TestBearerMiddleware_NoAuthHeader(t *testing.T) { + srv := httptest.NewServer(auth.BearerMiddleware(nil, "", okHandler(nil))) defer srv.Close() resp, err := http.Post(srv.URL+"/mcp", "application/json", nil) @@ -29,32 +29,33 @@ func TestBearerMiddleware_NoAuthHeader_NoDefault(t *testing.T) { assert.Equal(t, http.StatusUnauthorized, resp.StatusCode) } -func TestBearerMiddleware_NoAuthHeader_WithDefault(t *testing.T) { - called := false - srv := httptest.NewServer(noJWTMiddleware("default-pat", - http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { - called = true - w.WriteHeader(http.StatusOK) - }), - )) +func TestBearerMiddleware_NoAuthHeader_RejectsEvenWhenStaticConfigured(t *testing.T) { + // A configured staticToken must not allow unauthenticated callers through. + srv := httptest.NewServer(auth.BearerMiddleware(nil, "any-static", okHandler(nil))) defer srv.Close() resp, err := http.Post(srv.URL+"/mcp", "application/json", nil) require.NoError(t, err) defer func() { _ = resp.Body.Close() }() - assert.Equal(t, http.StatusOK, resp.StatusCode) - assert.True(t, called) + assert.Equal(t, http.StatusUnauthorized, resp.StatusCode) +} + +func TestBearerMiddleware_EmptyBearer(t *testing.T) { + srv := httptest.NewServer(auth.BearerMiddleware(nil, "static", okHandler(nil))) + defer srv.Close() + + req, _ := http.NewRequest(http.MethodPost, srv.URL+"/mcp", nil) + req.Header.Set("Authorization", "Bearer ") + resp, err := http.DefaultClient.Do(req) + require.NoError(t, err) + defer func() { _ = resp.Body.Close() }() + assert.Equal(t, http.StatusUnauthorized, resp.StatusCode) } func TestBearerMiddleware_StaticToken_Valid(t *testing.T) { const staticToken = "my-static-token" called := false - srv := httptest.NewServer(auth.BearerMiddleware(nil, staticToken, "", - http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { - called = true - w.WriteHeader(http.StatusOK) - }), - )) + srv := httptest.NewServer(auth.BearerMiddleware(nil, staticToken, okHandler(&called))) defer srv.Close() req, _ := http.NewRequest(http.MethodPost, srv.URL+"/mcp", nil) @@ -67,11 +68,7 @@ func TestBearerMiddleware_StaticToken_Valid(t *testing.T) { } func TestBearerMiddleware_StaticToken_Invalid(t *testing.T) { - srv := httptest.NewServer(auth.BearerMiddleware(nil, "correct-token", "", - http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { - w.WriteHeader(http.StatusOK) - }), - )) + srv := httptest.NewServer(auth.BearerMiddleware(nil, "correct-token", okHandler(nil))) defer srv.Close() req, _ := http.NewRequest(http.MethodPost, srv.URL+"/mcp", nil) @@ -82,12 +79,8 @@ func TestBearerMiddleware_StaticToken_Invalid(t *testing.T) { assert.Equal(t, http.StatusUnauthorized, resp.StatusCode) } -func TestBearerMiddleware_UnknownBearer_NoJWT(t *testing.T) { - srv := httptest.NewServer(noJWTMiddleware("", - http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { - w.WriteHeader(http.StatusOK) - }), - )) +func TestBearerMiddleware_UnknownBearer_NoStatic_NoJWT(t *testing.T) { + srv := httptest.NewServer(auth.BearerMiddleware(nil, "", okHandler(nil))) defer srv.Close() req, _ := http.NewRequest(http.MethodPost, srv.URL+"/mcp", nil)