fix: require secret for forwarded user auth (#8880)

Reject spoofed X-Forwarded-User headers unless a configured shared secret matches the forwarded secret header.

Co-authored-by: Klesh Wong <kleshwong@gmail.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Author: 3 people

backend/server/api/api.go

@@ -107,8 +107,8 @@ func CreateApiServer() *gin.Engine {
 
 	// Auth chain order matters: REST API key first (its own short-circuit),
 	// then the push API key gate, then OIDC session, then oauth2-proxy header
-	// (only sets USER if not yet set), then the terminal 401 gate, finally
-	// CSRF on unsafe methods.
+	// (only sets USER if not yet set and the forwarded secret matches), then the terminal 401 gate,
+	// finally CSRF on unsafe methods.
 	router.Use(RestAuthentication(router, basicRes))
 	router.Use(RequirePushAuthentication(basicRes))
 	router.Use(auth.OIDCAuthentication())

backend/server/api/middlewares.go

@@ -18,6 +18,7 @@ limitations under the License.
 package api
 
 import (
+	"crypto/subtle"
 	"encoding/base64"
 	"fmt"
 	"net/http"
@@ -35,12 +36,28 @@ import (
 	"github.com/gin-gonic/gin"
 )
 
-func getOAuthUserInfo(c *gin.Context) (*common.User, error) {
+const (
+	forwardedUserHeader       = "X-Forwarded-User"
+	forwardedEmailHeader      = "X-Forwarded-Email"
+	forwardedUserSecretHeader = "X-Forwarded-User-Secret"
+)
+
+func getOAuthUserInfo(c *gin.Context, forwardedUserSecret string) (*common.User, error) {
 	if c == nil {
 		return nil, errors.Default.New("request is nil")
 	}
-	user := c.GetHeader("X-Forwarded-User")
-	email := c.GetHeader("X-Forwarded-Email")
+	user := strings.TrimSpace(c.GetHeader(forwardedUserHeader))
+	if user == "" {
+		return nil, nil
+	}
+	if forwardedUserSecret == "" {
+		return nil, errors.Default.New("ignoring forwarded user headers because FORWARDED_USER_SECRET is not configured")
+	}
+	providedSecret := strings.TrimSpace(c.GetHeader(forwardedUserSecretHeader))
+	if subtle.ConstantTimeCompare([]byte(providedSecret), []byte(forwardedUserSecret)) != 1 {
+		return nil, errors.Default.New("ignoring forwarded user headers because X-Forwarded-User-Secret did not match")
+	}
+	email := strings.TrimSpace(c.GetHeader(forwardedEmailHeader))
 	return &common.User{
 		Name:  user,
 		Email: email,
@@ -75,12 +92,13 @@ func getBasicAuthUserInfo(c *gin.Context, basicRes context.BasicRes) (*common.Us
 
 func OAuth2ProxyAuthentication(basicRes context.BasicRes) gin.HandlerFunc {
 	logger := basicRes.GetLogger()
+	forwardedUserSecret := strings.TrimSpace(basicRes.GetConfigReader().GetString("FORWARDED_USER_SECRET"))
 	return func(c *gin.Context) {
 		_, exist := c.Get(common.USER)
 		if !exist {
-			user, err := getOAuthUserInfo(c)
+			user, err := getOAuthUserInfo(c, forwardedUserSecret)
 			if err != nil {
-				logger.Error(err, "getOAuthUserInfo")
+				logger.Warn(err, "rejected forwarded user headers")
 			}
 			if user == nil || user.Name == "" {
 				// fetch with basic auth header

backend/server/api/middlewares_forwardsecret_test.go

@@ -0,0 +1,147 @@
+/*
+Licensed to the Apache Software Foundation (ASF) under one or more
+contributor license agreements.  See the NOTICE file distributed with
+this work for additional information regarding copyright ownership.
+The ASF licenses this file to You under the Apache License, Version 2.0
+(the "License"); you may not use this file except in compliance with
+the License.  You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package api
+
+import (
+	"encoding/json"
+	"net/http"
+	"net/http/httptest"
+	"testing"
+
+	"github.com/gin-gonic/gin"
+	"github.com/spf13/viper"
+
+	"github.com/apache/incubator-devlake/core/config"
+	corecontext "github.com/apache/incubator-devlake/core/context"
+	"github.com/apache/incubator-devlake/core/dal"
+	"github.com/apache/incubator-devlake/core/log"
+	"github.com/apache/incubator-devlake/impls/logruslog"
+	"github.com/apache/incubator-devlake/server/api/shared"
+)
+
+type proxyAuthTestBasicRes struct {
+	cfg    config.ConfigReader
+	logger log.Logger
+}
+
+func (b *proxyAuthTestBasicRes) GetConfigReader() config.ConfigReader { return b.cfg }
+func (b *proxyAuthTestBasicRes) GetConfig(name string) string         { return b.cfg.GetString(name) }
+func (b *proxyAuthTestBasicRes) GetLogger() log.Logger                { return b.logger }
+func (b *proxyAuthTestBasicRes) NestedLogger(name string) corecontext.BasicRes {
+	return &proxyAuthTestBasicRes{cfg: b.cfg, logger: b.logger.Nested(name)}
+}
+func (b *proxyAuthTestBasicRes) ReplaceLogger(logger log.Logger) corecontext.BasicRes {
+	return &proxyAuthTestBasicRes{cfg: b.cfg, logger: logger}
+}
+func (b *proxyAuthTestBasicRes) GetDal() dal.Dal { return nil }
+
+type proxyAuthResponse struct {
+	Authenticated bool   `json:"authenticated"`
+	Name          string `json:"name"`
+	Email         string `json:"email"`
+}
+
+func newProxyAuthRouter(secret string) *gin.Engine {
+	gin.SetMode(gin.TestMode)
+	cfg := viper.New()
+	cfg.Set("FORWARDED_USER_SECRET", secret)
+	basicRes := &proxyAuthTestBasicRes{
+		cfg:    cfg,
+		logger: logruslog.Global,
+	}
+	r := gin.New()
+	r.Use(OAuth2ProxyAuthentication(basicRes))
+	r.GET("/me", func(c *gin.Context) {
+		user, ok := shared.GetUser(c)
+		if !ok {
+			c.JSON(http.StatusOK, proxyAuthResponse{})
+			return
+		}
+		c.JSON(http.StatusOK, proxyAuthResponse{
+			Authenticated: true,
+			Name:          user.Name,
+			Email:         user.Email,
+		})
+	})
+	return r
+}
+
+func performProxyAuthRequest(t *testing.T, router *gin.Engine, headers map[string]string) proxyAuthResponse {
+	t.Helper()
+	req := httptest.NewRequest(http.MethodGet, "/me", nil)
+	for key, value := range headers {
+		req.Header.Set(key, value)
+	}
+	recorder := httptest.NewRecorder()
+	router.ServeHTTP(recorder, req)
+	if recorder.Code != http.StatusOK {
+		t.Fatalf("expected 200, got %d: %s", recorder.Code, recorder.Body.String())
+	}
+	var body proxyAuthResponse
+	if err := json.Unmarshal(recorder.Body.Bytes(), &body); err != nil {
+		t.Fatalf("decode response: %v", err)
+	}
+	return body
+}
+
+func TestOAuth2ProxyAuthenticationRejectsUntrustedForwardedHeaders(t *testing.T) {
+	router := newProxyAuthRouter("shared-secret")
+	cases := map[string]map[string]string{
+		"missing secret header": {
+			forwardedUserHeader:  "admin@example.com",
+			forwardedEmailHeader: "admin@example.com",
+		},
+		"mismatched secret header": {
+			forwardedUserHeader:       "admin@example.com",
+			forwardedEmailHeader:      "admin@example.com",
+			forwardedUserSecretHeader: "wrong-secret",
+		},
+	}
+	for name, headers := range cases {
+		t.Run(name, func(t *testing.T) {
+			body := performProxyAuthRequest(t, router, headers)
+			if body.Authenticated {
+				t.Fatalf("expected forwarded headers to be rejected, got %+v", body)
+			}
+		})
+	}
+}
+
+func TestOAuth2ProxyAuthenticationRequiresConfiguredSecret(t *testing.T) {
+	router := newProxyAuthRouter("")
+	body := performProxyAuthRequest(t, router, map[string]string{
+		forwardedUserHeader:       "admin@example.com",
+		forwardedEmailHeader:      "admin@example.com",
+		forwardedUserSecretHeader: "shared-secret",
+	})
+	if body.Authenticated {
+		t.Fatalf("expected forwarded headers to be ignored without FORWARDED_USER_SECRET, got %+v", body)
+	}
+}
+
+func TestOAuth2ProxyAuthenticationAcceptsTrustedForwardedHeaders(t *testing.T) {
+	router := newProxyAuthRouter("shared-secret")
+	body := performProxyAuthRequest(t, router, map[string]string{
+		forwardedUserHeader:       "admin@example.com",
+		forwardedEmailHeader:      "admin@example.com",
+		forwardedUserSecretHeader: "shared-secret",
+	})
+	if !body.Authenticated || body.Name != "admin@example.com" || body.Email != "admin@example.com" {
+		t.Fatalf("expected trusted forwarded user, got %+v", body)
+	}
+}

env.example

@@ -103,6 +103,11 @@ AUTH_ENABLED=true
 # OIDC user login. Requires AUTH_ENABLED=true.
 OIDC_ENABLED=false
 
+# Shared secret required before DevLake trusts X-Forwarded-User and
+# X-Forwarded-Email from an upstream proxy. Configure the proxy to send the
+# same value as X-Forwarded-User-Secret on each forwarded request.
+FORWARDED_USER_SECRET=
+
 # Comma-separated provider identifiers. Each name <NAME> binds to the env
 # vars OIDC_<NAME>_ISSUER_URL, OIDC_<NAME>_CLIENT_ID, etc. Add a name and a
 # matching block of vars to onboard another IdP.