From a7bcb0397e29ce785f9bf02555600545035a30c9 Mon Sep 17 00:00:00 2001 From: littlejian <17816869670@163.com> Date: Wed, 15 Sep 2021 09:56:24 +0800 Subject: [PATCH] fix: gittar remove skipAuth (#1856) * fix: remove skipAuth * polish the code and add unit test --- modules/gittar/auth/auth.go | 79 +++++++++++++-------------- modules/gittar/auth/auth_test.go | 88 +++++++++++++++++++++++++++++++ modules/gittar/initialize.go | 3 ++ modules/gittar/models/web_hook.go | 6 +-- 4 files changed, 132 insertions(+), 44 deletions(-) create mode 100644 modules/gittar/auth/auth_test.go diff --git a/modules/gittar/auth/auth.go b/modules/gittar/auth/auth.go index 4a5bae480ad..0a1a8ae01ea 100644 --- a/modules/gittar/auth/auth.go +++ b/modules/gittar/auth/auth.go @@ -196,50 +196,43 @@ func doAuth(c *webcontext.Context, repo *models.Repo, repoName string) { var gitRepository = &gitmodule.Repository{} var err error var userInfo *ucauth.UserInfo - //skip authentication - host := c.Host() - for _, skipUrl := range conf.SkipAuthUrls() { - if skipUrl != "" && strings.HasSuffix(host, skipUrl) { - logrus.Debugf("skip authenticate host: %s", host) - gitRepository, err = openRepository(c, repo) - if err != nil { - c.AbortWithStatus(500, err) - return - } - c.Set("repository", gitRepository) - userIdStr := c.GetHeader(httputil.UserHeader) - if userIdStr == "" { - c.AbortWithStatus(500, errors.New("the userID is empty")) - return - } + if !isGitProtocolRequest(c) { + gitRepository, err = openRepository(c, repo) + if err != nil { + c.AbortWithStatus(500, err) + return + } + c.Set("repository", gitRepository) - userInfoDto, err := uc.FindUserById(userIdStr) - if err != nil { - c.AbortWithStatus(500, err) - return - } - logrus.Infof("repo: %s userId: %v, username: %s", repoName, userIdStr, userInfoDto.Username) - //校验通过缓存5分钟结果 - //校验失败每次都会请求 - _, validateError := ValidaUserRepoWithCache(c, userIdStr, repo) - if validateError != nil { - logrus.Infof("openapi auth fail repo:%s user:%s", repoName, userInfoDto.Username) - c.AbortWithStatus(403, validateError) - return - } - c.Set("repository", gitRepository) - //c.Set("lock", repoLock.Lock) + userIdStr := c.GetHeader(httputil.UserHeader) + if userIdStr == "" { + c.AbortWithStatus(500, errors.New("the userID is empty")) + return + } + userInfoDto, err := uc.FindUserById(userIdStr) + if err != nil { + c.AbortWithStatus(500, err) + return + } + logrus.Infof("repo: %s userId: %v, username: %s", repoName, userIdStr, userInfoDto.Username) - c.Set("user", &models.User{ - Name: userInfoDto.Username, - NickName: userInfoDto.NickName, - Email: userInfoDto.Email, - Id: userIdStr, - }) - c.Next() + // if success, caches the results for 5 minutes + _, validateError := ValidaUserRepoWithCache(c, userIdStr, repo) + if validateError != nil { + logrus.Infof("openapi auth fail repo:%s user:%s", repoName, userInfoDto.Username) + c.AbortWithStatus(403, validateError) return } + + c.Set("user", &models.User{ + Name: userInfoDto.Username, + NickName: userInfoDto.NickName, + Email: userInfoDto.Email, + Id: userIdStr, + }) + c.Next() + return } userInfo, err = GetUserInfoByTokenOrBasicAuth(c, repo.ProjectID) @@ -463,3 +456,11 @@ func getOrgIDV3(c *webcontext.Context, orgName string) (int64, error) { } return orgID, nil } + +// isGitProtocolRequest if request like git clone,git push... return true +func isGitProtocolRequest(c *webcontext.Context) bool { + if c.GetHeader("Git-Protocol") != "" { + return true + } + return false +} diff --git a/modules/gittar/auth/auth_test.go b/modules/gittar/auth/auth_test.go new file mode 100644 index 00000000000..f6e14e565b6 --- /dev/null +++ b/modules/gittar/auth/auth_test.go @@ -0,0 +1,88 @@ +// Copyright (c) 2021 Terminus, Inc. +// +// Licensed 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 auth + +import ( + "net/http" + "net/http/httptest" + "testing" + + "bou.ke/monkey" + "github.com/labstack/echo" + + "github.com/erda-project/erda/modules/gittar/models" + "github.com/erda-project/erda/modules/gittar/pkg/gitmodule" + "github.com/erda-project/erda/modules/gittar/webcontext" +) + +func TestIsGitProtocolRequest(t *testing.T) { + e := echo.New() + req1 := httptest.NewRequest(http.MethodGet, "/", nil) + req2 := httptest.NewRequest(http.MethodGet, "/", nil) + req1.Header.Add("Git-Protocol", "version") + res := httptest.NewRecorder() + ctx1 := &webcontext.Context{ + EchoContext: e.NewContext(req1, res), + Repository: nil, + User: nil, + Service: nil, + DBClient: nil, + Bundle: nil, + UCAuth: nil, + EtcdClient: nil, + } + ctx2 := &webcontext.Context{ + EchoContext: e.NewContext(req2, res), + Repository: nil, + User: nil, + Service: nil, + DBClient: nil, + Bundle: nil, + UCAuth: nil, + EtcdClient: nil, + } + + if isGitProtocolRequest(ctx1) != true { + t.Error("fail") + } + if isGitProtocolRequest(ctx2) == true { + t.Error("fail") + } +} + +func TestDoAuthWithHttpProtocolWithoutUserID(t *testing.T) { + e := echo.New() + req := httptest.NewRequest(http.MethodGet, "/", nil) + res := httptest.NewRecorder() + ctx := &webcontext.Context{ + EchoContext: e.NewContext(req, res), + Repository: nil, + User: nil, + Service: nil, + DBClient: nil, + Bundle: nil, + UCAuth: nil, + EtcdClient: nil, + } + monkey.Patch(openRepository, func(ctx *webcontext.Context, repo *models.Repo) (*gitmodule.Repository, error) { + return nil, nil + }) + defer monkey.UnpatchAll() + + doAuth(ctx, nil, "") + if ctx.EchoContext.Response().Status != 500 { + t.Error("fail") + } +} diff --git a/modules/gittar/initialize.go b/modules/gittar/initialize.go index e18fe69b2ac..4989abb14e3 100644 --- a/modules/gittar/initialize.go +++ b/modules/gittar/initialize.go @@ -139,6 +139,9 @@ func (p *provider) Initialize() error { // cron task to git gc all repository go gc.ScheduledExecuteClean() + // start hook task consumer + models.Init(dbClient) + return e.Start(":" + conf.ListenPort()) } diff --git a/modules/gittar/models/web_hook.go b/modules/gittar/models/web_hook.go index 0cda1221d56..40adcecc4a8 100644 --- a/modules/gittar/models/web_hook.go +++ b/modules/gittar/models/web_hook.go @@ -120,11 +120,7 @@ func (svc *Service) CreateHookTask(task *WebHookTask) error { } -func init() { - db, err := OpenDB() - if err != nil { - panic(err) - } +func Init(db *DBClient) { for i := 0; i < 5; i += 1 { go StartHookTaskConsumer(db) }