From e1e9a2e30ec1c2fa3422af1ee63c62e7563a49a0 Mon Sep 17 00:00:00 2001 From: Mathias Date: Mon, 14 Sep 2026 20:59:04 +0200 Subject: [PATCH] feat(auth): every accepted request names the path that granted it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes #60. Closes #58 (fixed in mcp-chassis v0.7.0, consumed here). Three auth paths reach this server and none of them said so on success. PassthroughMiddleware logged nothing at all, success or failure. While verifying #59 end to end there was no way to confirm from the logs whether calls were using the new PAT pass-through — the caller's own Gitea PAT, the caller's own identity — or falling through to the shared JWT/static path. The only available reasoning was inference from which failures were absent. INFO mcp auth accepted auth_path=passthrough caller=alice token_fp=... INFO mcp auth accepted auth_path=jwt caller=mathiasbq token_fp=... INFO mcp auth accepted auth_path=static token_fp=... The first line matters more than the other two. Pass-through is the path where a request acts as a SPECIFIC Gitea user; an audit trail that cannot say which identity performed a write is not an audit trail. chassisauth.Fingerprint, not a local equivalent. The chassis emits token_fp on its own accepted and rejected lines, and two different fingerprints for the same token cannot be correlated — which is the only reason the field exists. Exported from the chassis in v0.7.1 for exactly this. A token that is not a valid PAT still falls through SILENTLY. The chassis owns the verdict, and logging here as well would double-count every static-token request, since all of them reach this middleware first. Asserted, because "logs nothing on the failure path" is indistinguishable from "was never called". Also lands #58 by consuming mcp-chassis v0.7.0: a JWT presented to a server with no JWT auth configured now reports jwt_auth_not_configured rather than static_token_mismatch. That log line is what sent a debugging session into re-running the OAuth flow while the credentials were valid the whole time. Verified: task check EXIT=0. Co-Authored-By: Claude Opus 5 --- go.mod | 2 +- go.sum | 4 ++ internal/auth/passthrough.go | 26 +++++++ internal/auth/passthrough_logging_test.go | 83 +++++++++++++++++++++++ 4 files changed, 114 insertions(+), 1 deletion(-) create mode 100644 internal/auth/passthrough_logging_test.go diff --git a/go.mod b/go.mod index b02224e..778f557 100644 --- a/go.mod +++ b/go.mod @@ -3,7 +3,7 @@ module git.d-ma.be/mathias/gitea-mcp go 1.26.2 require ( - git.d-ma.be/mathias/mcp-chassis v0.6.0 + git.d-ma.be/mathias/mcp-chassis v0.7.1 github.com/hashicorp/golang-lru/v2 v2.0.7 github.com/stretchr/testify v1.11.1 ) diff --git a/go.sum b/go.sum index d2520b5..510ec11 100644 --- a/go.sum +++ b/go.sum @@ -2,6 +2,10 @@ git.d-ma.be/mathias/mcp-chassis v0.5.0 h1:0w3dt4t4r8OtZBHIOoZkR6p6L3L6YDiqhMh9bT git.d-ma.be/mathias/mcp-chassis v0.5.0/go.mod h1:Ks7EK2UnGAN0H3rJjKUxUagX8/ZBdtLrOlcUbv0RwH8= git.d-ma.be/mathias/mcp-chassis v0.6.0 h1:/MuxT9IyzZbN26+COlI2FK0moTyXMLG/qXc/cm0i0Vg= git.d-ma.be/mathias/mcp-chassis v0.6.0/go.mod h1:Ks7EK2UnGAN0H3rJjKUxUagX8/ZBdtLrOlcUbv0RwH8= +git.d-ma.be/mathias/mcp-chassis v0.7.0 h1:ct31v1F1PMhMCva/V0ErEbgMzwnT+27lYKbug2A/quE= +git.d-ma.be/mathias/mcp-chassis v0.7.0/go.mod h1:Ks7EK2UnGAN0H3rJjKUxUagX8/ZBdtLrOlcUbv0RwH8= +git.d-ma.be/mathias/mcp-chassis v0.7.1 h1:4h8TOBHTA0L/no4IPcr4tLcQtmKUlhE2/cXyPdNFGmM= +git.d-ma.be/mathias/mcp-chassis v0.7.1/go.mod h1:Ks7EK2UnGAN0H3rJjKUxUagX8/ZBdtLrOlcUbv0RwH8= github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c= github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= diff --git a/internal/auth/passthrough.go b/internal/auth/passthrough.go index ba6efc0..43c4bf1 100644 --- a/internal/auth/passthrough.go +++ b/internal/auth/passthrough.go @@ -2,9 +2,12 @@ package auth import ( "context" + "log/slog" "net/http" "strings" + chassisauth "git.d-ma.be/mathias/mcp-chassis/auth" + "git.d-ma.be/mathias/gitea-mcp/internal/gitea" ) @@ -25,11 +28,34 @@ func PassthroughMiddleware(validator TokenValidator, onValid, fallback http.Hand token, hasBearer := strings.CutPrefix(authz, "Bearer ") if hasBearer && token != "" { if login, ok := validator.ValidateToken(r.Context(), token); ok { + // gitea-mcp#60. This middleware logged nothing at all, so there + // was no way to tell a PAT pass-through request from one that + // fell through to the shared JWT/static path — only inference + // from which failures were absent. + // + // It matters more here than on the other two paths: this is + // where a request acts as a SPECIFIC Gitea user. An audit trail + // that cannot say which identity performed a write is not an + // audit trail. + // + // chassisauth.Fingerprint, not a local equivalent: the chassis + // emits the same field on its own accepted/rejected lines, and + // two different fingerprints for one token cannot be correlated. + slog.Info("mcp auth accepted", + "auth_path", "passthrough", + "caller", login, + "token_fp", chassisauth.Fingerprint(token), + "path", r.URL.Path, + ) ctx := withCaller(r.Context(), login) ctx = gitea.WithToken(ctx, token) onValid.ServeHTTP(w, r.WithContext(ctx)) return } + // A token that is not a valid PAT falls through SILENTLY. The + // chassis owns the verdict; logging here too would double-count + // every static-token request, since all of them reach this + // middleware first. } fallback.ServeHTTP(w, r) }) diff --git a/internal/auth/passthrough_logging_test.go b/internal/auth/passthrough_logging_test.go new file mode 100644 index 0000000..af9fe8d --- /dev/null +++ b/internal/auth/passthrough_logging_test.go @@ -0,0 +1,83 @@ +package auth + +import ( + "bytes" + "context" + "encoding/json" + "log/slog" + "net/http" + "net/http/httptest" + "strings" + "testing" + + "github.com/stretchr/testify/require" +) + +type stubValidator struct{ login string } + +func (s stubValidator) ValidateToken(context.Context, string) (string, bool) { + return s.login, s.login != "" +} + +// gitea-mcp#60: PassthroughMiddleware logged nothing at all, success or failure. +// +// While verifying #59 end to end there was no way to confirm from the server +// logs whether calls were using the new PAT pass-through (caller's own Gitea +// PAT, own identity) or falling through to the shared JWT/static path. The only +// way to reason about it was inference. The chassis names its two paths as of +// v0.7.0; this is the third. +// +// This matters more than the other two: pass-through is the path where the +// request acts as a SPECIFIC Gitea user. An audit trail that cannot say which +// identity performed a write is not an audit trail. +func TestPassthroughMiddleware_LogsTheAcceptedPath(t *testing.T) { + var buf bytes.Buffer + prev := slog.Default() + slog.SetDefault(slog.New(slog.NewJSONHandler(&buf, &slog.HandlerOptions{Level: slog.LevelInfo}))) + t.Cleanup(func() { slog.SetDefault(prev) }) + + reached := false + h := PassthroughMiddleware(stubValidator{login: "alice"}, + http.HandlerFunc(func(http.ResponseWriter, *http.Request) { reached = true }), + http.HandlerFunc(func(http.ResponseWriter, *http.Request) { t.Fatal("must not fall back") })) + + req := httptest.NewRequest(http.MethodGet, "/mcp", nil) + req.Header.Set("Authorization", "Bearer a-real-gitea-pat") + h.ServeHTTP(httptest.NewRecorder(), req) + require.True(t, reached) + + var line map[string]any + require.NoError(t, json.Unmarshal([]byte(strings.TrimSpace(buf.String())), &line)) + require.Equal(t, "mcp auth accepted", line["msg"]) + require.Equal(t, "passthrough", line["auth_path"]) + require.Equal(t, "alice", line["caller"]) + require.NotEmpty(t, line["token_fp"], "correlatable without being a credential") + + for k, v := range line { + require.NotContains(t, []any{v}, "a-real-gitea-pat", + "the PAT must never appear in any field, including %q", k) + } +} + +// A token that is not a valid PAT must fall through silently to the chassis, +// which owns the rejection log. Logging here as well would double-count every +// static-token request — every one of them reaches this middleware first. +func TestPassthroughMiddleware_FallsThroughWithoutLogging(t *testing.T) { + var buf bytes.Buffer + prev := slog.Default() + slog.SetDefault(slog.New(slog.NewJSONHandler(&buf, &slog.HandlerOptions{Level: slog.LevelInfo}))) + t.Cleanup(func() { slog.SetDefault(prev) }) + + fellBack := false + h := PassthroughMiddleware(stubValidator{}, + http.HandlerFunc(func(http.ResponseWriter, *http.Request) { t.Fatal("must not accept") }), + http.HandlerFunc(func(http.ResponseWriter, *http.Request) { fellBack = true })) + + req := httptest.NewRequest(http.MethodGet, "/mcp", nil) + req.Header.Set("Authorization", "Bearer not-a-pat") + h.ServeHTTP(httptest.NewRecorder(), req) + + require.True(t, fellBack) + require.Empty(t, strings.TrimSpace(buf.String()), + "the chassis owns the verdict; logging here too would double-count every static-token call") +}