fix(auth): preserve API-key user across RequireAuth re-dispatch

backend/server/api/middlewares.go

@@ -33,6 +33,7 @@ import (
 	"github.com/apache/incubator-devlake/core/errors"
 	"github.com/apache/incubator-devlake/core/models/common"
 	"github.com/apache/incubator-devlake/helpers/apikeyhelper"
+	"github.com/apache/incubator-devlake/server/api/shared"
 	"github.com/gin-gonic/gin"
 )
 
@@ -272,7 +273,7 @@ func CheckAuthorizationHeader(c *gin.Context, logger log.Logger, db dal.Dal, api
 
 	logger.Info("redirect path: %s to: %s", c.Request.URL.Path, path)
 	c.Request.URL.Path = path
-	c.Set(common.USER, &common.User{
+	shared.SetUserOnRequest(c, &common.User{
 		Name:  apiKey.Creator.Creator,
 		Email: apiKey.Creator.CreatorEmail,
 	})

backend/server/api/middlewares_authchain_test.go

@@ -0,0 +1,161 @@
+/*
+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 (
+	stdctx "context"
+	"net/http"
+	"net/http/httptest"
+	"strings"
+	"testing"
+
+	corectx "github.com/apache/incubator-devlake/core/context"
+	"github.com/apache/incubator-devlake/core/dal"
+	"github.com/apache/incubator-devlake/core/errors"
+	contextimpl "github.com/apache/incubator-devlake/impls/context"
+	"github.com/apache/incubator-devlake/impls/logruslog"
+	"github.com/apache/incubator-devlake/server/api/auth"
+	"github.com/gin-gonic/gin"
+	"github.com/spf13/viper"
+)
+
+// authChainStubDal is a minimal dal.Dal sufficient to drive the auth chain
+// without a real database. Only the three methods exercised by this test are
+// implemented; any other call would (intentionally) panic via the embedded
+// nil dal.Dal, surfacing an unexpected dependency.
+type authChainStubDal struct {
+	dal.Dal
+	firstErr errors.Error // returned by First; nil means "found"
+}
+
+// All backs the auth service's revocation-cache boot load. Returning no rows
+// keeps the service from depending on a real DB.
+func (s *authChainStubDal) All(dst interface{}, _ ...dal.Clause) errors.Error { return nil }
+
+// First backs apikeyhelper.GetApiKey. On success it leaves dst as the zero
+// ApiKey, whose empty AllowedPath regex matches every path — enough to model
+// "a valid, correctly-scoped key" without persisting one.
+func (s *authChainStubDal) First(dst interface{}, _ ...dal.Clause) errors.Error { return s.firstErr }
+
+func (s *authChainStubDal) IsErrorNotFound(err error) bool { return err != nil }
+
+// newAuthChainEnv builds a router whose middleware chain mirrors production
+// with AUTH_ENABLED=true and OIDC_ENABLED=false (the hardened-image default):
+//
+//	RestAuthentication -> OIDCAuthentication -> RequireAuth
+//
+// firstErr controls how the stubbed API-key lookup resolves.
+func newAuthChainEnv(t *testing.T, firstErr errors.Error) *gin.Engine {
+	t.Helper()
+	// apikeyhelper reads ENCRYPTION_SECRET from the global config (AutomaticEnv).
+	t.Setenv("ENCRYPTION_SECRET", strings.Repeat("a", 32))
+
+	cfg := viper.New()
+	cfg.Set("ENCRYPTION_SECRET", strings.Repeat("a", 32))
+	cfg.Set("AUTH_ENABLED", true)
+	cfg.Set("OIDC_ENABLED", false)
+
+	db := &authChainStubDal{firstErr: firstErr}
+	basicRes := contextimpl.NewDefaultBasicRes(cfg, logruslog.Global, db)
+
+	ctx, cancel := stdctx.WithCancel(stdctx.Background())
+	t.Cleanup(cancel)
+	svc, err := auth.NewService(ctx, basicRes)
+	if err != nil {
+		t.Fatalf("auth.NewService: %v", err)
+	}
+
+	gin.SetMode(gin.TestMode)
+	router := gin.New()
+	router.Use(RestAuthentication(router, basicRes)) // API-key auth for /rest, then re-dispatches
+	router.Use(svc.OIDCAuthentication())             // session cookie -> sets user (no-op here)
+	router.Use(svc.RequireAuth())                    // terminal gate: no user -> 401
+
+	// Open-API handler, reached only after RestAuthentication rewrites the
+	// /rest-prefixed path and re-dispatches through the chain.
+	router.POST("/plugins/webhook/connections/:id/deployments", func(c *gin.Context) {
+		c.JSON(http.StatusOK, gin.H{"success": true})
+	})
+	// A normal protected route used to prove RequireAuth is actually active.
+	router.GET("/plugins/webhook/connections", func(c *gin.Context) {
+		c.JSON(http.StatusOK, gin.H{"success": true})
+	})
+	return router
+}
+
+// TestAuthChainHoldsForApiKeyWithOIDCEnabled is the regression test for the
+// open-API 401 bug. With AUTH_ENABLED on and the OIDC RequireAuth gate live, a
+// valid API key against a /rest endpoint must survive the internal
+// HandleContext re-dispatch (which wipes gin Keys) and reach the handler.
+func TestAuthChainHoldsForApiKeyWithOIDCEnabled(t *testing.T) {
+	const restPath = "/rest/plugins/webhook/connections/1/deployments"
+
+	t.Run("valid key reaches open-api handler (200)", func(t *testing.T) {
+		router := newAuthChainEnv(t, nil)
+		resp := httptest.NewRecorder()
+		req := httptest.NewRequest(http.MethodPost, restPath, strings.NewReader(`{}`))
+		req.Header.Set("Authorization", "Bearer valid-key")
+		router.ServeHTTP(resp, req)
+
+		if resp.Code != http.StatusOK {
+			t.Fatalf("status = %d, want %d (RequireAuth wiped the API-key user)", resp.Code, http.StatusOK)
+		}
+	})
+
+	t.Run("missing token (401)", func(t *testing.T) {
+		router := newAuthChainEnv(t, nil)
+		resp := httptest.NewRecorder()
+		req := httptest.NewRequest(http.MethodPost, restPath, strings.NewReader(`{}`))
+		router.ServeHTTP(resp, req)
+
+		if resp.Code != http.StatusUnauthorized {
+			t.Fatalf("status = %d, want %d", resp.Code, http.StatusUnauthorized)
+		}
+		if !strings.Contains(resp.Body.String(), "token is missing") {
+			t.Errorf("body = %q, want it to mention 'token is missing'", resp.Body.String())
+		}
+	})
+
+	t.Run("invalid key (403)", func(t *testing.T) {
+		router := newAuthChainEnv(t, errors.NotFound.New("not found"))
+		resp := httptest.NewRecorder()
+		req := httptest.NewRequest(http.MethodPost, restPath, strings.NewReader(`{}`))
+		req.Header.Set("Authorization", "Bearer wrong-key")
+		router.ServeHTTP(resp, req)
+
+		if resp.Code != http.StatusForbidden {
+			t.Fatalf("status = %d, want %d", resp.Code, http.StatusForbidden)
+		}
+		if !strings.Contains(resp.Body.String(), "api key is invalid") {
+			t.Errorf("body = %q, want it to mention 'api key is invalid'", resp.Body.String())
+		}
+	})
+
+	t.Run("protected route without user is gated (401)", func(t *testing.T) {
+		router := newAuthChainEnv(t, nil)
+		resp := httptest.NewRecorder()
+		req := httptest.NewRequest(http.MethodGet, "/plugins/webhook/connections", nil)
+		router.ServeHTTP(resp, req)
+
+		if resp.Code != http.StatusUnauthorized {
+			t.Fatalf("status = %d, want %d (RequireAuth not active?)", resp.Code, http.StatusUnauthorized)
+		}
+	})
+}
+
+var _ corectx.BasicRes = (*contextimpl.DefaultBasicRes)(nil)

backend/server/api/shared/gin_utils.go

@@ -18,15 +18,40 @@ limitations under the License.
 package shared
 
 import (
+	"context"
+
 	"github.com/apache/incubator-devlake/core/models/common"
 	"github.com/gin-gonic/gin"
 )
 
+// userContextKey is a dedicated, unexported type for storing the
+// authenticated user on the request's context.Context. A dedicated type
+// avoids collisions with other packages and satisfies go vet (which flags
+// basic types such as plain strings used as context keys).
+type userContextKey struct{}
+
+// SetUserOnRequest stores the authenticated user on the request's
+// context.Context so the identity survives gin's Engine.HandleContext
+// re-dispatch, which calls Context.reset() and wipes gin's Keys (where
+// c.Set stores values). Callers that re-dispatch (e.g. the /rest open-API
+// path rewrite) must use this so the terminal RequireAuth gate can still
+// see the user. It also mirrors the value into gin Keys via c.Set for the
+// common, non-re-dispatched case.
+func SetUserOnRequest(c *gin.Context, user *common.User) {
+	c.Set(common.USER, user)
+	c.Request = c.Request.WithContext(context.WithValue(c.Request.Context(), userContextKey{}, user))
+}
+
 func GetUser(c *gin.Context) (*common.User, bool) {
-	userObj, exist := c.Get(common.USER)
-	if !exist {
-		return nil, false
+	if userObj, exist := c.Get(common.USER); exist {
+		if user, ok := userObj.(*common.User); ok {
+			return user, true
+		}
+	}
+	// Fall back to the request context, which survives Engine.HandleContext
+	// re-dispatch (unlike gin Keys, which Context.reset() clears).
+	if user, ok := c.Request.Context().Value(userContextKey{}).(*common.User); ok {
+		return user, true
 	}
-	user := userObj.(*common.User)
-	return user, true
+	return nil, false
 }

backend/server/api/shared/gin_utils_test.go

@@ -0,0 +1,97 @@
+/*
+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 shared
+
+import (
+	"net/http"
+	"net/http/httptest"
+	"testing"
+
+	"github.com/apache/incubator-devlake/core/models/common"
+	"github.com/gin-gonic/gin"
+)
+
+// TestGetUserReadsGinKeys covers the common, non-re-dispatched case where the
+// user is set directly on the gin context.
+func TestGetUserReadsGinKeys(t *testing.T) {
+	gin.SetMode(gin.TestMode)
+	c, _ := gin.CreateTestContext(httptest.NewRecorder())
+	c.Request = httptest.NewRequest(http.MethodGet, "/", nil)
+	c.Set(common.USER, &common.User{Name: "alice"})
+
+	user, ok := GetUser(c)
+	if !ok || user == nil || user.Name != "alice" {
+		t.Fatalf("GetUser() = %+v, %v; want alice, true", user, ok)
+	}
+}
+
+// TestSetUserOnRequestSurvivesHandleContext is the regression test for the
+// open-API key 401 bug: gin's Engine.HandleContext re-dispatch calls
+// Context.reset(), which sets c.Keys = nil and therefore drops anything set
+// via c.Set. The authenticated user must instead ride on the request's
+// context.Context so the terminal RequireAuth gate can still see it after the
+// /rest path-rewrite re-dispatch.
+func TestSetUserOnRequestSurvivesHandleContext(t *testing.T) {
+	gin.SetMode(gin.TestMode)
+
+	var (
+		ginKeysVisible  bool
+		requestVisible  bool
+		reachedTerminal bool
+	)
+
+	router := gin.New()
+	router.GET("/target", func(c *gin.Context) {
+		reachedTerminal = true
+		// gin Keys are wiped by reset() during HandleContext re-dispatch.
+		if _, ok := c.Get(common.USER); ok {
+			ginKeysVisible = true
+		}
+		// The user must survive on the request context.
+		if user, ok := GetUser(c); ok && user != nil && user.Name == "bob" {
+			requestVisible = true
+		}
+		c.Status(http.StatusOK)
+	})
+
+	// Entry handler mimics RestAuthentication: authenticate, stash the user,
+	// rewrite the path, and re-dispatch through the engine.
+	router.GET("/rest/target", func(c *gin.Context) {
+		SetUserOnRequest(c, &common.User{Name: "bob"})
+		c.Request.URL.Path = "/target"
+		router.HandleContext(c)
+		c.Abort()
+	})
+
+	resp := httptest.NewRecorder()
+	req := httptest.NewRequest(http.MethodGet, "/rest/target", nil)
+	router.ServeHTTP(resp, req)
+
+	if !reachedTerminal {
+		t.Fatal("re-dispatched request never reached the terminal handler")
+	}
+	if ginKeysVisible {
+		t.Error("gin Keys unexpectedly survived HandleContext; test no longer exercises the reset() bug")
+	}
+	if !requestVisible {
+		t.Error("user did not survive HandleContext re-dispatch via request context (regression)")
+	}
+	if resp.Code != http.StatusOK {
+		t.Errorf("status = %d, want %d", resp.Code, http.StatusOK)
+	}
+}