From 88d3d88f22d768abe2f87b8e7d86e42236a27a1d Mon Sep 17 00:00:00 2001 From: Christophe Kamphaus Date: Sun, 5 Jul 2026 20:35:26 +0200 Subject: [PATCH 1/2] Add test for webhook api key with activated auth --- .../server/api/middlewares_rest_auth_test.go | 107 ++++++++++++++++++ 1 file changed, 107 insertions(+) create mode 100644 backend/server/api/middlewares_rest_auth_test.go diff --git a/backend/server/api/middlewares_rest_auth_test.go b/backend/server/api/middlewares_rest_auth_test.go new file mode 100644 index 00000000000..41d421b6529 --- /dev/null +++ b/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()) + } +} From 4ab90c4c2d8a2a57ee3d3d1f4e87711b0a91749d Mon Sep 17 00:00:00 2001 From: Christophe Kamphaus Date: Sun, 5 Jul 2026 20:56:06 +0200 Subject: [PATCH 2/2] Fix webhook authorization --- backend/server/api/middlewares.go | 9 ++++++-- backend/server/api/shared/gin_utils.go | 29 ++++++++++++++++++++++---- 2 files changed, 32 insertions(+), 6 deletions(-) diff --git a/backend/server/api/middlewares.go b/backend/server/api/middlewares.go index a988c41288a..e1609b1ec6a 100644 --- a/backend/server/api/middlewares.go +++ b/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 } diff --git a/backend/server/api/shared/gin_utils.go b/backend/server/api/shared/gin_utils.go index 892dc489ae6..c15d0a73019 100644 --- a/backend/server/api/shared/gin_utils.go +++ b/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 }