webhook: overhaul route handlers (#6002)

* Overual route handlers and fixes #5366

* Merge routes for repo and org

* Inject OrgRepoContext

* DRY validateWebhook

* DRY c.HasError

* Add tests

* Update CHANGELOG

Author: unknwon

CHANGELOG.md

@@ -40,6 +40,7 @@ All notable changes to Gogs are documented in this file.
 - [Security] Potential ability to delete files outside a repository.
 - [Security] Potential ability to set primary email on others' behalf from their verified emails.
 - [Security] Potential XSS attack via `.ipynb`. [#5170](https://github.com/gogs/gogs/issues/5170)
+- [Security] Potential SSRF attack via webhooks. [#5366](https://github.com/gogs/gogs/issues/5366)
 - [Security] Potential CSRF attack in admin panel. [#5367](https://github.com/gogs/gogs/issues/5367)
 - [Security] Potential RCE on mirror repositories. [#5767](https://github.com/gogs/gogs/issues/5767)
 - [Security] Potential XSS attack with raw markdown API. [#5907](https://github.com/gogs/gogs/pull/5907)

conf/locale/locale_en-US.ini

@@ -807,8 +807,10 @@ settings.invite_collaborator = Invite Collaborator
 settings.invite_placeholder = Enter mail adress to invite
 settings.invite_collaborator_warn = Be aware that this will send out invitation mails!
 settings.org_not_allowed_to_be_collaborator = Organization is not allowed to be added as a collaborator.
-settings.add_webhook = Add Webhook
-settings.hooks_desc = Webhooks are much like basic HTTP POST event triggers. Whenever something occurs in GIN, we will handle the notification to the target host you specify. Learn more in this <a target="_blank" href="%s">Webhooks Guide</a>.
+settings.hooks_desc = Webhooks are much like basic HTTP POST event triggers. Whenever something occurs in Gogs, we will handle the notification to the target host you specify.
+settings.webhooks.add_new = Add a new webhook:
+settings.webhooks.choose_a_type = Choose a type...
+settings.add_webhook = Add webhook
 settings.webhook_deletion = Delete Webhook
 settings.webhook_deletion_desc = Delete this webhook will remove its information and all delivery history. Do you want to continue?
 settings.webhook_deletion_success = Webhook has been deleted successfully!
@@ -822,6 +824,8 @@ settings.webhook.response = Response
 settings.webhook.headers = Headers
 settings.webhook.payload = Payload
 settings.webhook.body = Body
+settings.webhook.err_cannot_parse_payload_url = Cannot parse payload URL: %v
+settings.webhook.err_cannot_use_local_addresses = Non admins are not allowed to use local addresses.
 settings.githooks_desc = Git Hooks are powered by Git itself, you can edit files of supported hooks in the list below to perform custom operations.
 settings.githook_edit_desc = If the hook is inactive, sample content will be presented. Leaving content to an empty value will disable this hook.
 settings.githook_name = Hook Name

internal/assets/conf/conf_gen.go

@@ -364,6 +364,23 @@ func runWeb(c *cli.Context) error {
 	reqRepoAdmin := context.RequireRepoAdmin()
 	reqRepoWriter := context.RequireRepoWriter()
 
+	webhookRoutes := func() {
+		m.Group("", func() {
+			m.Get("", repo.Webhooks)
+			m.Post("/delete", repo.DeleteWebhook)
+			m.Get("/:type/new", repo.WebhooksNew)
+			m.Post("/gogs/new", bindIgnErr(form.NewWebhook{}), repo.WebhooksNewPost)
+			m.Post("/slack/new", bindIgnErr(form.NewSlackHook{}), repo.WebhooksSlackNewPost)
+			m.Post("/discord/new", bindIgnErr(form.NewDiscordHook{}), repo.WebhooksDiscordNewPost)
+			m.Post("/dingtalk/new", bindIgnErr(form.NewDingtalkHook{}), repo.WebhooksDingtalkNewPost)
+			m.Get("/:id", repo.WebhooksEdit)
+			m.Post("/gogs/:id", bindIgnErr(form.NewWebhook{}), repo.WebhooksEditPost)
+			m.Post("/slack/:id", bindIgnErr(form.NewSlackHook{}), repo.WebhooksSlackEditPost)
+			m.Post("/discord/:id", bindIgnErr(form.NewDiscordHook{}), repo.WebhooksDiscordEditPost)
+			m.Post("/dingtalk/:id", bindIgnErr(form.NewDingtalkHook{}), repo.WebhooksDingtalkEditPost)
+		}, repo.InjectOrgRepoContext())
+	}
+
 	// ***** START: Organization *****
 	m.Group("/org", func() {
 		m.Group("", func() {
@@ -404,20 +421,7 @@ func runWeb(c *cli.Context) error {
 				m.Post("/avatar", binding.MultipartForm(form.Avatar{}), org.SettingsAvatar)
 				m.Post("/avatar/delete", org.SettingsDeleteAvatar)
 
-				m.Group("/hooks", func() {
-					m.Get("", org.Webhooks)
-					m.Post("/delete", org.DeleteWebhook)
-					m.Get("/:type/new", repo.WebhooksNew)
-					m.Post("/gogs/new", bindIgnErr(form.NewWebhook{}), repo.WebHooksNewPost)
-					m.Post("/slack/new", bindIgnErr(form.NewSlackHook{}), repo.SlackHooksNewPost)
-					m.Post("/discord/new", bindIgnErr(form.NewDiscordHook{}), repo.DiscordHooksNewPost)
-					m.Post("/dingtalk/new", bindIgnErr(form.NewDingtalkHook{}), repo.DingtalkHooksNewPost)
-					m.Get("/:id", repo.WebHooksEdit)
-					m.Post("/gogs/:id", bindIgnErr(form.NewWebhook{}), repo.WebHooksEditPost)
-					m.Post("/slack/:id", bindIgnErr(form.NewSlackHook{}), repo.SlackHooksEditPost)
-					m.Post("/discord/:id", bindIgnErr(form.NewDiscordHook{}), repo.DiscordHooksEditPost)
-					m.Post("/dingtalk/:id", bindIgnErr(form.NewDingtalkHook{}), repo.DingtalkHooksEditPost)
-				})
+				m.Group("/hooks", webhookRoutes)
 
 				m.Route("/delete", "GET,POST", org.SettingsDelete)
 			})
@@ -464,20 +468,9 @@ func runWeb(c *cli.Context) error {
 			})
 
 			m.Group("/hooks", func() {
-				m.Get("", repo.Webhooks)
-				m.Post("/delete", repo.DeleteWebhook)
-				m.Get("/:type/new", repo.WebhooksNew)
-				m.Post("/gogs/new", bindIgnErr(form.NewWebhook{}), repo.WebHooksNewPost)
-				m.Post("/slack/new", bindIgnErr(form.NewSlackHook{}), repo.SlackHooksNewPost)
-				m.Post("/discord/new", bindIgnErr(form.NewDiscordHook{}), repo.DiscordHooksNewPost)
-				m.Post("/dingtalk/new", bindIgnErr(form.NewDingtalkHook{}), repo.DingtalkHooksNewPost)
-				m.Post("/gogs/:id", bindIgnErr(form.NewWebhook{}), repo.WebHooksEditPost)
-				m.Post("/slack/:id", bindIgnErr(form.NewSlackHook{}), repo.SlackHooksEditPost)
-				m.Post("/discord/:id", bindIgnErr(form.NewDiscordHook{}), repo.DiscordHooksEditPost)
-				m.Post("/dingtalk/:id", bindIgnErr(form.NewDingtalkHook{}), repo.DingtalkHooksEditPost)
+				webhookRoutes()
 
 				m.Group("/:id", func() {
-					m.Get("", repo.WebHooksEdit)
 					m.Post("/test", repo.TestWebhook)
 					m.Post("/redelivery", repo.RedeliveryWebhook)
 				})

internal/assets/public/public_gen.go

@@ -60,7 +60,7 @@ var File *ini.File
 //
 // NOTE: The order of loading configuration sections matters as one may depend on another.
 //
-// ⚠️ WARNING: Do not print anything in this function other than wanrings.
+// ⚠️ WARNING: Do not print anything in this function other than warnings.
 func Init(customConf string) error {
 	var err error
 	File, err = ini.LoadSources(ini.LoadOptions{

internal/assets/templates/templates_gen.go

@@ -136,10 +136,10 @@ func (w *Webhook) AfterSet(colName string, _ xorm.Cell) {
 	}
 }
 
-func (w *Webhook) GetSlackHook() *SlackMeta {
+func (w *Webhook) SlackMeta() *SlackMeta {
 	s := &SlackMeta{}
 	if err := jsoniter.Unmarshal([]byte(w.Meta), s); err != nil {
-		log.Error("GetSlackHook [%d]: %v", w.ID, err)
+		log.Error("Failed to get Slack meta [webhook_id: %d]: %v", w.ID, err)
 	}
 	return s
 }

internal/cmd/web.go

@@ -0,0 +1,33 @@
+// 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 mock
+
+import (
+	"gopkg.in/macaron.v1"
+)
+
+var _ macaron.Locale = (*Locale)(nil)
+
+// Locale is a mock that implements macaron.Locale.
+type Locale struct {
+	lang string
+	tr   func(string, ...interface{}) string
+}
+
+// NewLocale creates a new mock for macaron.Locale.
+func NewLocale(lang string, tr func(string, ...interface{}) string) *Locale {
+	return &Locale{
+		lang: lang,
+		tr:   tr,
+	}
+}
+
+func (l *Locale) Language() string {
+	return l.lang
+}
+
+func (l *Locale) Tr(format string, args ...interface{}) string {
+	return l.tr(format, args...)
+}

internal/conf/conf.go

@@ -75,7 +75,7 @@ func ToHook(repoLink string, w *db.Webhook) *api.Hook {
 		"content_type": w.ContentType.Name(),
 	}
 	if w.HookTaskType == db.SLACK {
-		s := w.GetSlackHook()
+		s := w.SlackMeta()
 		config["channel"] = s.Channel
 		config["username"] = s.Username
 		config["icon_url"] = s.IconURL

internal/db/webhook.go

@@ -17,9 +17,8 @@ import (
 )
 
 const (
-	SETTINGS_OPTIONS  = "org/settings/options"
-	SETTINGS_DELETE   = "org/settings/delete"
-	SETTINGS_WEBHOOKS = "org/settings/webhooks"
+	SETTINGS_OPTIONS = "org/settings/options"
+	SETTINGS_DELETE  = "org/settings/delete"
 )
 
 func Settings(c *context.Context) {
@@ -136,32 +135,3 @@ func SettingsDelete(c *context.Context) {
 
 	c.Success(SETTINGS_DELETE)
 }
-
-func Webhooks(c *context.Context) {
-	c.Title("org.settings")
-	c.Data["PageIsSettingsHooks"] = true
-	c.Data["BaseLink"] = c.Org.OrgLink
-	c.Data["Description"] = c.Tr("org.settings.hooks_desc")
-	c.Data["Types"] = conf.Webhook.Types
-
-	ws, err := db.GetWebhooksByOrgID(c.Org.Organization.ID)
-	if err != nil {
-		c.Error(err, "get webhooks by organization ID")
-		return
-	}
-
-	c.Data["Webhooks"] = ws
-	c.Success(SETTINGS_WEBHOOKS)
-}
-
-func DeleteWebhook(c *context.Context) {
-	if err := db.DeleteWebhookOfOrgByID(c.Org.Organization.ID, c.QueryInt64("id")); err != nil {
-		c.Flash.Error("DeleteWebhookByOrgID: " + err.Error())
-	} else {
-		c.Flash.Success(c.Tr("repo.settings.webhook_deletion_success"))
-	}
-
-	c.JSONSuccess( map[string]interface{}{
-		"redirect": c.Org.OrgLink + "/settings/hooks",
-	})
-}