diff options
| -rw-r--r-- | CHANGELOG.md | 1 | ||||
| -rw-r--r-- | internal/cmd/hook.go | 7 | ||||
| -rw-r--r-- | internal/cmd/web.go | 3 | ||||
| -rw-r--r-- | internal/route/lfs/route.go | 2 | ||||
| -rw-r--r-- | internal/route/repo/pull.go | 49 | ||||
| -rw-r--r-- | internal/route/repo/tasks.go | 74 |
6 files changed, 82 insertions, 54 deletions
diff --git a/CHANGELOG.md b/CHANGELOG.md index 0313c1ec..9da981dd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -51,6 +51,7 @@ All notable changes to Gogs are documented in this file. - Enable Federated Avatar Lookup could cause server to crash. [#5848](https://github.com/gogs/gogs/issues/5848) - Private repositories are hidden in the organization's view. [#5869](https://github.com/gogs/gogs/issues/5869) - Server error when changing email address in user settings page. [#5899](https://github.com/gogs/gogs/issues/5899) +- Webhooks are not fired after push when `[service] REQUIRE_SIGNIN_VIEW = true`. ### Removed diff --git a/internal/cmd/hook.go b/internal/cmd/hook.go index 0e9136b0..9b4e2278 100644 --- a/internal/cmd/hook.go +++ b/internal/cmd/hook.go @@ -238,9 +238,10 @@ func runHookPostReceive(c *cli.Context) error { reqURL := fmt.Sprintf("%s%s/%s/tasks/trigger?%s", conf.Server.LocalRootURL, options.RepoUserName, options.RepoName, q.Encode()) log.Trace("Trigger task: %s", reqURL) - resp, err := httplib.Head(reqURL).SetTLSClientConfig(&tls.Config{ - InsecureSkipVerify: true, - }).Response() + resp, err := httplib.Get(reqURL). + SetTLSClientConfig(&tls.Config{ + InsecureSkipVerify: true, + }).Response() if err == nil { _ = resp.Body.Close() if resp.StatusCode/100 != 2 { diff --git a/internal/cmd/web.go b/internal/cmd/web.go index 99b8d195..43b9dfc5 100644 --- a/internal/cmd/web.go +++ b/internal/cmd/web.go @@ -614,7 +614,6 @@ func runWeb(c *cli.Context) error { m.Get("", repo.Home) m.Get("/stars", repo.Stars) m.Get("/watchers", repo.Watchers) - m.Head("/tasks/trigger", repo.TriggerTask) // TODO: Without session and CSRF }, ignSignIn, context.RepoAssignment(), context.RepoRef()) // ***** END: Repository ***** @@ -654,6 +653,8 @@ func runWeb(c *cli.Context) error { // *************************** m.Group("/:username/:reponame", func() { + m.Get("/tasks/trigger", repo.TriggerTask) + m.Group("/info/lfs", func() { lfs.RegisterRoutes(m.Router) }) diff --git a/internal/route/lfs/route.go b/internal/route/lfs/route.go index a7003708..08045ac5 100644 --- a/internal/route/lfs/route.go +++ b/internal/route/lfs/route.go @@ -85,7 +85,7 @@ func authenticate() macaron.Handler { // Once we found the token, we're supposed to find its related user, // thus any error is unexpected. internalServerError(c.Resp) - log.Error("Failed to get user: %v", err) + log.Error("Failed to get user [id: %d]: %v", token.UserID, err) return } } diff --git a/internal/route/repo/pull.go b/internal/route/repo/pull.go index 7e064f40..f9a5b341 100644 --- a/internal/route/repo/pull.go +++ b/internal/route/repo/pull.go @@ -19,7 +19,6 @@ import ( "gogs.io/gogs/internal/db" "gogs.io/gogs/internal/form" "gogs.io/gogs/internal/gitutil" - "gogs.io/gogs/internal/tool" ) const ( @@ -744,51 +743,3 @@ func CompareAndPullRequestPost(c *context.Context, f form.NewIssue) { log.Trace("Pull request created: %d/%d", repo.ID, pullIssue.ID) c.Redirect(c.Repo.RepoLink + "/pulls/" + com.ToStr(pullIssue.Index)) } - -func parseOwnerAndRepo(c *context.Context) (*db.User, *db.Repository) { - owner, err := db.GetUserByName(c.Params(":username")) - if err != nil { - c.NotFoundOrError(err, "get user by name") - return nil, nil - } - - repo, err := db.GetRepositoryByName(owner.ID, c.Params(":reponame")) - if err != nil { - c.NotFoundOrError(err, "get repository by name") - return nil, nil - } - - return owner, repo -} - -func TriggerTask(c *context.Context) { - pusherID := c.QueryInt64("pusher") - branch := c.Query("branch") - secret := c.Query("secret") - if len(branch) == 0 || len(secret) == 0 || pusherID <= 0 { - c.NotFound() - log.Trace("TriggerTask: branch or secret is empty, or pusher ID is not valid") - return - } - owner, repo := parseOwnerAndRepo(c) - if c.Written() { - return - } - if secret != tool.MD5(owner.Salt) { - c.NotFound() - log.Trace("TriggerTask [%s/%s]: invalid secret", owner.Name, repo.Name) - return - } - - pusher, err := db.GetUserByID(pusherID) - if err != nil { - c.NotFoundOrError(err, "get user by ID") - return - } - - log.Trace("TriggerTask '%s/%s' by '%s'", repo.Name, branch, pusher.Name) - - go db.HookQueue.Add(repo.ID) - go db.AddTestPullRequestTask(pusher, repo.ID, branch, true) - c.Status(202) -} diff --git a/internal/route/repo/tasks.go b/internal/route/repo/tasks.go new file mode 100644 index 00000000..e06d3ee7 --- /dev/null +++ b/internal/route/repo/tasks.go @@ -0,0 +1,74 @@ +// Copyright 2020 The Gogs Authors. All rights reserved. +// Use of this source code is governed by a MIT-style +// license that can be found in the LICENSE file. + +package repo + +import ( + "net/http" + + "gopkg.in/macaron.v1" + log "unknwon.dev/clog/v2" + + "gogs.io/gogs/internal/db" + "gogs.io/gogs/internal/tool" +) + +func TriggerTask(c *macaron.Context) { + branch := c.Query("branch") + pusherID := c.QueryInt64("pusher") + secret := c.Query("secret") + if branch == "" || pusherID <= 0 || secret == "" { + c.Error(http.StatusBadRequest, "Incomplete branch, pusher or secret") + return + } + + username := c.Params(":username") + reponame := c.Params(":reponame") + + owner, err := db.Users.GetByUsername(username) + if err != nil { + if db.IsErrUserNotExist(err) { + c.Error(http.StatusBadRequest, "Owner does not exist") + } else { + c.Status(http.StatusInternalServerError) + log.Error("Failed to get user [name: %s]: %v", username, err) + } + return + } + + // 🚨 SECURITY: No need to check existence of the repository if the client + // can't even get the valid secret. Mostly likely not a legitimate request. + if secret != tool.MD5(owner.Salt) { + c.Error(http.StatusBadRequest, "Invalid secret") + return + } + + repo, err := db.Repos.GetByName(owner.ID, reponame) + if err != nil { + if db.IsErrRepoNotExist(err) { + c.Error(http.StatusBadRequest, "Repository does not exist") + } else { + c.Status(http.StatusInternalServerError) + log.Error("Failed to get repository [owner_id: %d, name: %s]: %v", owner.ID, reponame, err) + } + return + } + + pusher, err := db.Users.GetByID(pusherID) + if err != nil { + if db.IsErrUserNotExist(err) { + c.Error(http.StatusBadRequest, "Pusher does not exist") + } else { + c.Status(http.StatusInternalServerError) + log.Error("Failed to get user [id: %d]: %v", pusherID, err) + } + return + } + + log.Trace("TriggerTask: %s/%s@%s by %q", owner.Name, repo.Name, branch, pusher.Name) + + go db.HookQueue.Add(repo.ID) + go db.AddTestPullRequestTask(pusher, repo.ID, branch, true) + c.Status(http.StatusAccepted) +} |