This is an automated email from the ASF dual-hosted git repository.
klesh pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/devlake.git
The following commit(s) were added to refs/heads/main by this push:
new 20179fa52 Fix webhook authorization when AUTH_ENABLED=true (#9008)
20179fa52 is described below
commit 20179fa520d03df65dd58d7295421a3d832fe0d5
Author: Christophe Kamphaus <[email protected]>
AuthorDate: Mon Jul 27 08:14:21 2026 +0200
Fix webhook authorization when AUTH_ENABLED=true (#9008)
* test(webhook): add test for webhook api key with activated auth
* fix(webhook): fix webhook authorization
---
backend/server/api/middlewares.go | 9 +-
backend/server/api/middlewares_rest_auth_test.go | 107 +++++++++++++++++++++++
backend/server/api/shared/gin_utils.go | 29 +++++-
3 files changed, 139 insertions(+), 6 deletions(-)
diff --git a/backend/server/api/middlewares.go
b/backend/server/api/middlewares.go
index a988c4128..e1609b1ec 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/middlewares_rest_auth_test.go
b/backend/server/api/middlewares_rest_auth_test.go
new file mode 100644
index 000000000..41d421b65
--- /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())
+ }
+}
diff --git a/backend/server/api/shared/gin_utils.go
b/backend/server/api/shared/gin_utils.go
index 892dc489a..c15d0a730 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
}
