Fix webhook authorization when AUTH_ENABLED=true (apache#9008)

* test(webhook): add test for webhook api key with activated auth

* fix(webhook): fix webhook authorization

Author: kamphaus

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,9 +273,13 @@ 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{
+	user := &common.User{
 		Name:  apiKey.Creator.Creator,
 		Email: apiKey.Creator.CreatorEmail,
-	})
+	}
+	c.Set(common.USER, user)
+	// Also store in the request context so the user survives gin's HandleContext
+	// resetting c.Keys when rerouting from /rest/... to /plugins/...
+	c.Request = shared.SetRestAuthUser(c.Request, user)
 	return true
 }

backend/server/api/middlewares_rest_auth_test.go

@@ -0,0 +1,107 @@
+/*
+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 (
+	"net/http"
+	"net/http/httptest"
+	"testing"
+
+	"github.com/apache/incubator-devlake/core/config"
+	coremodels "github.com/apache/incubator-devlake/core/models"
+	"github.com/apache/incubator-devlake/core/models/common"
+	"github.com/apache/incubator-devlake/helpers/apikeyhelper"
+	contextimpl "github.com/apache/incubator-devlake/impls/context"
+	"github.com/apache/incubator-devlake/impls/logruslog"
+	mockdal "github.com/apache/incubator-devlake/mocks/core/dal"
+	"github.com/apache/incubator-devlake/server/api/shared"
+	"github.com/gin-gonic/gin"
+	"github.com/stretchr/testify/mock"
+)
+
+// requireUserGate simulates RequireAuth with AUTH_ENABLED=true: it rejects any
+// request whose gin context does not carry an authenticated user. This is the
+// exact check that caused 401s for valid REST API key requests.
+func requireUserGate() gin.HandlerFunc {
+	return func(c *gin.Context) {
+		if _, ok := shared.GetUser(c); !ok {
+			c.AbortWithStatusJSON(http.StatusUnauthorized, gin.H{
+				"success": false,
+				"message": "unauthorized",
+			})
+			return
+		}
+		c.Next()
+	}
+}
+
+// TestRestAuthKeyReachesHandlerWhenAuthEnabled sends a valid /rest/... Bearer
+// token request through a router that also has a RequireAuth-style gate. The
+// request must reach the downstream handler (200) rather than being rejected
+// by the auth gate (401).
+//
+// Without the fix, gin's HandleContext resets c.Keys, wiping the user that
+// RestAuthentication stored before rerouting, so the auth gate sees no user
+// and returns 401.
+func TestRestAuthKeyReachesHandlerWhenAuthEnabled(t *testing.T) {
+	gin.SetMode(gin.TestMode)
+
+	// apikeyhelper reads ENCRYPTION_SECRET from the global viper config.
+	const encryptionSecret = "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" // 32 bytes
+	config.GetConfig().Set("ENCRYPTION_SECRET", encryptionSecret)
+
+	basicRes := contextimpl.NewDefaultBasicRes(config.GetConfig(), logruslog.Global, nil)
+	helper := apikeyhelper.NewApiKeyHelper(basicRes, logruslog.Global)
+
+	const plaintext = "test-api-key-plaintext"
+	hashedKey, hashErr := helper.DigestToken(plaintext)
+	if hashErr != nil {
+		t.Fatalf("DigestToken: %v", hashErr)
+	}
+
+	// Mock DAL: when First is called, populate the destination with a valid key
+	// whose AllowedPath covers the webhook endpoint under test.
+	db := &mockdal.Dal{}
+	db.On("First", mock.Anything, mock.Anything).
+		Run(func(args mock.Arguments) {
+			dst := args.Get(0).(*coremodels.ApiKey)
+			dst.ApiKey = hashedKey
+			dst.AllowedPath = `/plugins/webhook/connections/1/.*`
+			dst.Creator = common.Creator{Creator: "test-user"}
+		}).
+		Return(nil)
+
+	basicRes = contextimpl.NewDefaultBasicRes(config.GetConfig(), logruslog.Global, db)
+
+	router := gin.New()
+	router.Use(RestAuthentication(router, basicRes))
+	router.Use(requireUserGate())
+	router.POST("/plugins/webhook/connections/:id/deployments", func(c *gin.Context) {
+		c.Status(http.StatusOK)
+	})
+
+	req := httptest.NewRequest(http.MethodPost, "/rest/plugins/webhook/connections/1/deployments", nil)
+	req.Header.Set("Authorization", "Bearer "+plaintext)
+	resp := httptest.NewRecorder()
+	router.ServeHTTP(resp, req)
+
+	if resp.Code != http.StatusOK {
+		t.Fatalf("expected 200 with valid REST API key when auth gate is active, got %d: %s",
+			resp.Code, resp.Body.String())
+	}
+}

backend/server/api/shared/gin_utils.go

@@ -18,15 +18,36 @@ limitations under the License.
 package shared
 
 import (
+	"context"
+	"net/http"
+
 	"github.com/apache/incubator-devlake/core/models/common"
 	"github.com/gin-gonic/gin"
 )
 
+// restAuthKey is an unexported type used as a request-context key so it cannot
+// collide with keys set by other packages.
+type restAuthKey struct{}
+
+// SetRestAuthUser stores the authenticated user in the HTTP request context.
+// This is necessary because gin's HandleContext calls c.reset(), which clears
+// c.Keys but leaves c.Request (and its context) intact. RestAuthentication
+// calls this before rerouting so the user survives the reset.
+func SetRestAuthUser(r *http.Request, user *common.User) *http.Request {
+	return r.WithContext(context.WithValue(r.Context(), restAuthKey{}, user))
+}
+
 func GetUser(c *gin.Context) (*common.User, bool) {
 	userObj, exist := c.Get(common.USER)
-	if !exist {
-		return nil, false
+	if exist {
+		if user, ok := userObj.(*common.User); ok {
+			return user, true
+		}
+	}
+	// Fallback: RestAuthentication stores the user here before calling
+	// HandleContext, which resets c.Keys but preserves c.Request.
+	if user, ok := c.Request.Context().Value(restAuthKey{}).(*common.User); ok && user != nil {
+		return user, true
 	}
-	user := userObj.(*common.User)
-	return user, true
+	return nil, false
 }