feat(auth): document caller header precedence + warn on conflict (#10)

CallerMiddleware silently preferred X-Auth-Request-User over X-Forwarded-User
with no explanation and no signal when both were set. Documented the precedence
(X-Auth-Request-User is the verified OIDC identity oauth2-proxy sets, so it is
authoritative; X-Forwarded-User is a fallback), and it now takes a *slog.Logger
and warns when both headers are present and disagree, so a proxy
misconfiguration is visible instead of silently resolved. Table-driven tests
cover precedence (both/single/none) and the conflict-warning path.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

cmd/gitea-mcp/main.go

@@ -56,7 +56,7 @@ func main() {
 	mux := http.NewServeMux()
 	mux.Handle("/mcp", mcp.OriginAllowlist(cfg.OriginAllowlist)(
 		chassisauth.BearerMiddleware(cfg.StaticToken, jwtValidator, "gitea", resourceMetadataURL,
-			auth.CallerMiddleware(mcpSrv),
+			auth.CallerMiddleware(logger, mcpSrv),
 		),
 	))
 	mux.Handle("/healthz", newHealthzHandler(cfg.DexIssuerURL != "", jwtValidator != nil, jwtInitErr))

internal/auth/caller.go

@@ -2,17 +2,40 @@ package auth
 
 import (
 	"context"
+	"log/slog"
 	"net/http"
 )
 
 type ctxKey struct{}
 
-func CallerMiddleware(next http.Handler) http.Handler {
+// CallerMiddleware extracts the authenticated username from the reverse-proxy
+// identity headers and stashes it in the request context for Caller().
+//
+// Header precedence: X-Auth-Request-User takes priority over X-Forwarded-User.
+// X-Auth-Request-User is the header oauth2-proxy sets from the *verified* OIDC
+// identity, so it is authoritative. X-Forwarded-User is a weaker, proxy-set
+// convention some setups populate instead; it is used only as a fallback when
+// X-Auth-Request-User is absent. If a proxy sets BOTH and they disagree, the
+// verified X-Auth-Request-User still wins and we log a warning so the
+// misconfiguration is visible rather than silently resolved (#10).
+//
+// logger may be nil, in which case the conflict warning is skipped.
+func CallerMiddleware(logger *slog.Logger, next http.Handler) http.Handler {
 	return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
-		user := r.Header.Get("X-Auth-Request-User")
+		authUser := r.Header.Get("X-Auth-Request-User")
+		fwdUser := r.Header.Get("X-Forwarded-User")
+
+		user := authUser
 		if user == "" {
-			user = r.Header.Get("X-Forwarded-User")
+			user = fwdUser
 		}
+
+		if logger != nil && authUser != "" && fwdUser != "" && authUser != fwdUser {
+			logger.Warn("conflicting caller identity headers; using X-Auth-Request-User",
+				"x_auth_request_user", authUser,
+				"x_forwarded_user", fwdUser)
+		}
+
 		ctx := context.WithValue(r.Context(), ctxKey{}, user)
 		next.ServeHTTP(w, r.WithContext(ctx))
 	})

internal/auth/caller_test.go

@@ -1,7 +1,9 @@
 package auth_test
 
 import (
+	"bytes"
 	"context"
+	"log/slog"
 	"net/http"
 	"net/http/httptest"
 	"testing"
@@ -10,17 +12,69 @@ import (
 	"github.com/stretchr/testify/assert"
 )
 
-func TestCallerFromContext(t *testing.T) {
-	called := false
-	h := auth.CallerMiddleware(http.HandlerFunc(func(_ http.ResponseWriter, r *http.Request) {
-		called = true
-		assert.Equal(t, "mathiasbq", auth.Caller(r.Context()))
-	}))
+func discardLogger() *slog.Logger {
+	return slog.New(slog.NewTextHandler(bytes.NewBuffer(nil), nil))
+}
+
+// Header precedence: X-Auth-Request-User (verified OIDC identity) wins over
+// X-Forwarded-User, and X-Forwarded-User is only a fallback when the former is
+// absent.
+func TestCallerHeaderPrecedence(t *testing.T) {
+	tests := []struct {
+		name       string
+		authReq    string
+		forwarded  string
+		wantCaller string
+	}{
+		{"auth-request only", "mathiasbq", "", "mathiasbq"},
+		{"forwarded fallback", "", "fwduser", "fwduser"},
+		{"both present, same", "same", "same", "same"},
+		{"both present, differ → auth-request wins", "authuser", "fwduser", "authuser"},
+		{"neither", "", "", ""},
+	}
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			var got string
+			h := auth.CallerMiddleware(discardLogger(), http.HandlerFunc(func(_ http.ResponseWriter, r *http.Request) {
+				got = auth.Caller(r.Context())
+			}))
+			req := httptest.NewRequest(http.MethodPost, "/", nil)
+			if tc.authReq != "" {
+				req.Header.Set("X-Auth-Request-User", tc.authReq)
+			}
+			if tc.forwarded != "" {
+				req.Header.Set("X-Forwarded-User", tc.forwarded)
+			}
+			h.ServeHTTP(httptest.NewRecorder(), req)
+			assert.Equal(t, tc.wantCaller, got)
+		})
+	}
+}
+
+// When both headers are present and disagree, a warning is logged so the proxy
+// misconfiguration is visible rather than silent.
+func TestCallerConflictingHeadersLogsWarning(t *testing.T) {
+	var buf bytes.Buffer
+	logger := slog.New(slog.NewJSONHandler(&buf, &slog.HandlerOptions{Level: slog.LevelWarn}))
+
+	h := auth.CallerMiddleware(logger, http.HandlerFunc(func(_ http.ResponseWriter, _ *http.Request) {}))
 	req := httptest.NewRequest(http.MethodPost, "/", nil)
-	req.Header.Set("X-Auth-Request-User", "mathiasbq")
-	rr := httptest.NewRecorder()
-	h.ServeHTTP(rr, req)
-	assert.True(t, called)
+	req.Header.Set("X-Auth-Request-User", "authuser")
+	req.Header.Set("X-Forwarded-User", "fwduser")
+	h.ServeHTTP(httptest.NewRecorder(), req)
+
+	logged := buf.String()
+	assert.Contains(t, logged, "conflicting")
+	assert.Contains(t, logged, "authuser")
+	assert.Contains(t, logged, "fwduser")
+
+	// No warning when they agree.
+	buf.Reset()
+	req2 := httptest.NewRequest(http.MethodPost, "/", nil)
+	req2.Header.Set("X-Auth-Request-User", "same")
+	req2.Header.Set("X-Forwarded-User", "same")
+	h.ServeHTTP(httptest.NewRecorder(), req2)
+	assert.Empty(t, buf.String(), "no warning expected when headers agree")
 }
 
 func TestCallerEmptyWhenHeaderMissing(t *testing.T) {

internal/tools/pr_create_test.go

@@ -30,7 +30,7 @@ const prFixture = `{
 
 func callerContext(user string) context.Context {
 	var capturedCtx context.Context
-	h := auth.CallerMiddleware(http.HandlerFunc(func(_ http.ResponseWriter, r *http.Request) {
+	h := auth.CallerMiddleware(nil, http.HandlerFunc(func(_ http.ResponseWriter, r *http.Request) {
 		capturedCtx = r.Context()
 	}))
 	req := httptest.NewRequest("POST", "/", nil)