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

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

@@ -790,8 +790,10 @@ settings.collaborator_deletion_desc = This user will no longer have collaboratio
 settings.remove_collaborator_success = Collaborator has been removed.
 settings.search_user_placeholder = Search user...
 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 Gogs, 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!
@@ -805,6 +807,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/cmd/web.go

@@ -356,6 +356,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() {
@@ -396,20 +413,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)
 			})
@@ -454,20 +458,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/conf/conf.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/db/webhook.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/mock/locale.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/route/api/v1/convert/convert.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/route/org/setting.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",
-	})
-}

internal/route/repo/webhook.go

@@ -7,13 +7,13 @@ package repo
 import (
 	"fmt"
 	"net/http"
+	"net/url"
 	"strings"
 
-	jsoniter "github.com/json-iterator/go"
-	"github.com/unknwon/com"
-
-	git "github.com/gogs/git-module"
+	"github.com/gogs/git-module"
 	api "github.com/gogs/go-gogs-client"
+	jsoniter "github.com/json-iterator/go"
+	"gopkg.in/macaron.v1"
 
 	"gogs.io/gogs/internal/conf"
 	"gogs.io/gogs/internal/context"
@@ -23,89 +23,163 @@ import (
 )
 
 const (
-	WEBHOOKS        = "repo/settings/webhook/base"
-	WEBHOOK_NEW     = "repo/settings/webhook/new"
-	ORG_WEBHOOK_NEW = "org/settings/webhook_new"
+	tmplRepoSettingsWebhooks   = "repo/settings/webhook/base"
+	tmplRepoSettingsWebhookNew = "repo/settings/webhook/new"
+	tmplOrgSettingsWebhooks    = "org/settings/webhooks"
+	tmplOrgSettingsWebhookNew  = "org/settings/webhook_new"
 )
 
-func Webhooks(c *context.Context) {
-	c.Data["Title"] = c.Tr("repo.settings.hooks")
-	c.Data["PageIsSettingsHooks"] = true
-	c.Data["BaseLink"] = c.Repo.RepoLink
-	c.Data["Description"] = c.Tr("repo.settings.hooks_desc", "https://github.com/gogs/docs-api/blob/master/Repositories/Webhooks.md")
-	c.Data["Types"] = conf.Webhook.Types
-
-	ws, err := db.GetWebhooksByRepoID(c.Repo.Repository.ID)
-	if err != nil {
-		c.Error(err, "get webhooks by repository ID")
-		return
+func InjectOrgRepoContext() macaron.Handler {
+	return func(c *context.Context) {
+		orCtx, err := getOrgRepoContext(c)
+		if err != nil {
+			c.Error(err, "get organization or repository context")
+			return
+		}
+		c.Map(orCtx)
 	}
-	c.Data["Webhooks"] = ws
-
-	c.Success(WEBHOOKS)
 }
 
-type OrgRepoCtx struct {
-	OrgID       int64
-	RepoID      int64
-	Link        string
-	NewTemplate string
+type orgRepoContext struct {
+	OrgID    int64
+	RepoID   int64
+	Link     string
+	TmplList string
+	TmplNew  string
 }
 
-// getOrgRepoCtx determines whether this is a repo context or organization context.
-func getOrgRepoCtx(c *context.Context) (*OrgRepoCtx, error) {
+// getOrgRepoContext determines whether this is a repo context or organization context.
+func getOrgRepoContext(c *context.Context) (*orgRepoContext, error) {
 	if len(c.Repo.RepoLink) > 0 {
-		c.Data["PageIsRepositoryContext"] = true
-		return &OrgRepoCtx{
-			RepoID:      c.Repo.Repository.ID,
-			Link:        c.Repo.RepoLink,
-			NewTemplate: WEBHOOK_NEW,
+		c.PageIs("RepositoryContext")
+		return &orgRepoContext{
+			RepoID:   c.Repo.Repository.ID,
+			Link:     c.Repo.RepoLink,
+			TmplList: tmplRepoSettingsWebhooks,
+			TmplNew:  tmplRepoSettingsWebhookNew,
 		}, nil
 	}
 
 	if len(c.Org.OrgLink) > 0 {
-		c.Data["PageIsOrganizationContext"] = true
-		return &OrgRepoCtx{
-			OrgID:       c.Org.Organization.ID,
-			Link:        c.Org.OrgLink,
-			NewTemplate: ORG_WEBHOOK_NEW,
+		c.PageIs("OrganizationContext")
+		return &orgRepoContext{
+			OrgID:    c.Org.Organization.ID,
+			Link:     c.Org.OrgLink,
+			TmplList: tmplOrgSettingsWebhooks,
+			TmplNew:  tmplOrgSettingsWebhookNew,
 		}, nil
 	}
 
-	return nil, errors.New("Unable to set OrgRepo context")
+	return nil, errors.New("unable to determine context")
 }
 
-func checkHookType(c *context.Context) string {
+func Webhooks(c *context.Context, orCtx *orgRepoContext) {
+	c.Title("repo.settings.hooks")
+	c.PageIs("SettingsHooks")
+	c.Data["Types"] = conf.Webhook.Types
+
+	var err error
+	var ws []*db.Webhook
+	if orCtx.RepoID > 0 {
+		c.Data["Description"] = c.Tr("repo.settings.hooks_desc")
+		ws, err = db.GetWebhooksByRepoID(orCtx.RepoID)
+	} else {
+		c.Data["Description"] = c.Tr("org.settings.hooks_desc")
+		ws, err = db.GetWebhooksByOrgID(orCtx.OrgID)
+	}
+	if err != nil {
+		c.Error(err, "get webhooks")
+		return
+	}
+	c.Data["Webhooks"] = ws
+
+	c.Success(orCtx.TmplList)
+}
+
+func WebhooksNew(c *context.Context, orCtx *orgRepoContext) {
+	c.Title("repo.settings.add_webhook")
+	c.PageIs("SettingsHooks")
+	c.PageIs("SettingsHooksNew")
+
+	allowed := false
 	hookType := strings.ToLower(c.Params(":type"))
-	if !com.IsSliceContainsStr(conf.Webhook.Types, hookType) {
+	for _, typ := range conf.Webhook.Types {
+		if hookType == typ {
+			allowed = true
+			c.Data["HookType"] = typ
+			break
+		}
+	}
+	if !allowed {
 		c.NotFound()
-		return ""
+		return
 	}
-	return hookType
+
+	c.Success(orCtx.TmplNew)
 }
 
-func WebhooksNew(c *context.Context) {
-	c.Data["Title"] = c.Tr("repo.settings.add_webhook")
-	c.Data["PageIsSettingsHooks"] = true
-	c.Data["PageIsSettingsHooksNew"] = true
-	c.Data["Webhook"] = db.Webhook{HookEvent: &db.HookEvent{}}
+var localHostnames = []string{
+	"localhost",
+	"127.0.0.1",
+	"::1",
+	"0:0:0:0:0:0:0:1",
+}
 
-	orCtx, err := getOrgRepoCtx(c)
-	if err != nil {
-		c.Error(err, "get organization repository context")
+// isLocalHostname returns true if given hostname is a known local address.
+func isLocalHostname(hostname string) bool {
+	for _, local := range localHostnames {
+		if hostname == local {
+			return true
+		}
+	}
+	return false
+}
+
+func validateWebhook(actor *db.User, l macaron.Locale, w *db.Webhook) (field string, msg string, ok bool) {
+	if !actor.IsAdmin {
+		// 🚨 SECURITY: Local addresses must not be allowed by non-admins to prevent SSRF,
+		// see https://github.com/gogs/gogs/issues/5366 for details.
+		payloadURL, err := url.Parse(w.URL)
+		if err != nil {
+			return "PayloadURL", l.Tr("repo.settings.webhook.err_cannot_parse_payload_url", err), false
+		}
+
+		if isLocalHostname(payloadURL.Hostname()) {
+			return "PayloadURL", l.Tr("repo.settings.webhook.err_cannot_use_local_addresses"), false
+		}
+	}
+
+	return "", "", true
+}
+
+func validateAndCreateWebhook(c *context.Context, orCtx *orgRepoContext, w *db.Webhook) {
+	c.Data["Webhook"] = w
+
+	if c.HasError() {
+		c.Success(orCtx.TmplNew)
 		return
 	}
 
-	c.Data["HookType"] = checkHookType(c)
-	if c.Written() {
+	field, msg, ok := validateWebhook(c.User, c.Locale, w)
+	if !ok {
+		c.FormErr(field)
+		c.RenderWithErr(msg, orCtx.TmplNew, nil)
+		return
+	}
+
+	if err := w.UpdateEvent(); err != nil {
+		c.Error(err, "update event")
+		return
+	} else if err := db.CreateWebhook(w); err != nil {
+		c.Error(err, "create webhook")
 		return
 	}
-	c.Data["BaseLink"] = orCtx.Link
 
-	c.Success(orCtx.NewTemplate)
+	c.Flash.Success(c.Tr("repo.settings.add_hook_success"))
+	c.Redirect(orCtx.Link + "/settings/hooks")
 }
 
-func ParseHookEvent(f form.Webhook) *db.HookEvent {
+func toHookEvent(f form.Webhook) *db.HookEvent {
 	return &db.HookEvent{
 		PushOnly:       f.PushOnly(),
 		SendEverything: f.SendEverything(),
@@ -123,25 +197,12 @@ func ParseHookEvent(f form.Webhook) *db.HookEvent {
 	}
 }
 
-func WebHooksNewPost(c *context.Context, f form.NewWebhook) {
-	c.Data["Title"] = c.Tr("repo.settings.add_webhook")
-	c.Data["PageIsSettingsHooks"] = true
-	c.Data["PageIsSettingsHooksNew"] = true
-	c.Data["Webhook"] = db.Webhook{HookEvent: &db.HookEvent{}}
+func WebhooksNewPost(c *context.Context, orCtx *orgRepoContext, f form.NewWebhook) {
+	c.Title("repo.settings.add_webhook")
+	c.PageIs("SettingsHooks")
+	c.PageIs("SettingsHooksNew")
 	c.Data["HookType"] = "gogs"
 
-	orCtx, err := getOrgRepoCtx(c)
-	if err != nil {
-		c.Error(err, "get organization repository context")
-		return
-	}
-	c.Data["BaseLink"] = orCtx.Link
-
-	if c.HasError() {
-		c.Success(orCtx.NewTemplate)
-		return
-	}
-
 	contentType := db.JSON
 	if db.HookContentType(f.ContentType) == db.FORM {
 		contentType = db.FORM
@@ -149,49 +210,32 @@ func WebHooksNewPost(c *context.Context, f form.NewWebhook) {
 
 	w := &db.Webhook{
 		RepoID:       orCtx.RepoID,
+		OrgID:        orCtx.OrgID,
 		URL:          f.PayloadURL,
 		ContentType:  contentType,
 		Secret:       f.Secret,
-		HookEvent:    ParseHookEvent(f.Webhook),
+		HookEvent:    toHookEvent(f.Webhook),
 		IsActive:     f.Active,
 		HookTaskType: db.GOGS,
-		OrgID:        orCtx.OrgID,
 	}
-	if err := w.UpdateEvent(); err != nil {
-		c.Error(err, "update event")
-		return
-	} else if err := db.CreateWebhook(w); err != nil {
-		c.Error(err, "create webhook")
-		return
-	}
-
-	c.Flash.Success(c.Tr("repo.settings.add_hook_success"))
-	c.Redirect(orCtx.Link + "/settings/hooks")
+	validateAndCreateWebhook(c, orCtx, w)
 }
 
-func SlackHooksNewPost(c *context.Context, f form.NewSlackHook) {
-	c.Data["Title"] = c.Tr("repo.settings")
-	c.Data["PageIsSettingsHooks"] = true
-	c.Data["PageIsSettingsHooksNew"] = true
-	c.Data["Webhook"] = db.Webhook{HookEvent: &db.HookEvent{}}
-
-	orCtx, err := getOrgRepoCtx(c)
-	if err != nil {
-		c.Error(err, "get organization repository context")
-		return
-	}
-
-	if c.HasError() {
-		c.Success(orCtx.NewTemplate)
-		return
-	}
+func WebhooksSlackNewPost(c *context.Context, orCtx *orgRepoContext, f form.NewSlackHook) {
+	c.Title("repo.settings.add_webhook")
+	c.PageIs("SettingsHooks")
+	c.PageIs("SettingsHooksNew")
+	c.Data["HookType"] = "slack"
 
-	meta, err := jsoniter.Marshal(&db.SlackMeta{
+	meta := &db.SlackMeta{
 		Channel:  f.Channel,
 		Username: f.Username,
 		IconURL:  f.IconURL,
 		Color:    f.Color,
-	})
+	}
+	c.Data["SlackMeta"] = meta
+
+	p, err := jsoniter.Marshal(meta)
 	if err != nil {
 		c.Error(err, "marshal JSON")
 		return
@@ -201,47 +245,29 @@ func SlackHooksNewPost(c *context.Context, f form.NewSlackHook) {
 		RepoID:       orCtx.RepoID,
 		URL:          f.PayloadURL,
 		ContentType:  db.JSON,
-		HookEvent:    ParseHookEvent(f.Webhook),
+		HookEvent:    toHookEvent(f.Webhook),
 		IsActive:     f.Active,
 		HookTaskType: db.SLACK,
-		Meta:         string(meta),
+		Meta:         string(p),
 		OrgID:        orCtx.OrgID,
 	}
-	if err := w.UpdateEvent(); err != nil {
-		c.Error(err, "update event")
-		return
-	} else if err := db.CreateWebhook(w); err != nil {
-		c.Error(err, "create webhook")
-		return
-	}
-
-	c.Flash.Success(c.Tr("repo.settings.add_hook_success"))
-	c.Redirect(orCtx.Link + "/settings/hooks")
+	validateAndCreateWebhook(c, orCtx, w)
 }
 
-// FIXME: merge logic to Slack
-func DiscordHooksNewPost(c *context.Context, f form.NewDiscordHook) {
-	c.Data["Title"] = c.Tr("repo.settings")
-	c.Data["PageIsSettingsHooks"] = true
-	c.Data["PageIsSettingsHooksNew"] = true
-	c.Data["Webhook"] = db.Webhook{HookEvent: &db.HookEvent{}}
+func WebhooksDiscordNewPost(c *context.Context, orCtx *orgRepoContext, f form.NewDiscordHook) {
+	c.Title("repo.settings.add_webhook")
+	c.PageIs("SettingsHooks")
+	c.PageIs("SettingsHooksNew")
+	c.Data["HookType"] = "discord"
 
-	orCtx, err := getOrgRepoCtx(c)
-	if err != nil {
-		c.Error(err, "get organization repository context")
-		return
-	}
-
-	if c.HasError() {
-		c.Success(orCtx.NewTemplate)
-		return
-	}
-
-	meta, err := jsoniter.Marshal(&db.SlackMeta{
+	meta := &db.SlackMeta{
 		Username: f.Username,
 		IconURL:  f.IconURL,
 		Color:    f.Color,
-	})
+	}
+	c.Data["SlackMeta"] = meta
+
+	p, err := jsoniter.Marshal(meta)
 	if err != nil {
 		c.Error(err, "marshal JSON")
 		return
@@ -251,72 +277,37 @@ func DiscordHooksNewPost(c *context.Context, f form.NewDiscordHook) {
 		RepoID:       orCtx.RepoID,
 		URL:          f.PayloadURL,
 		ContentType:  db.JSON,
-		HookEvent:    ParseHookEvent(f.Webhook),
+		HookEvent:    toHookEvent(f.Webhook),
 		IsActive:     f.Active,
 		HookTaskType: db.DISCORD,
-		Meta:         string(meta),
+		Meta:         string(p),
 		OrgID:        orCtx.OrgID,
 	}
-	if err := w.UpdateEvent(); err != nil {
-		c.Error(err, "update event")
-		return
-	} else if err := db.CreateWebhook(w); err != nil {
-		c.Error(err, "create webhook")
-		return
-	}
-
-	c.Flash.Success(c.Tr("repo.settings.add_hook_success"))
-	c.Redirect(orCtx.Link + "/settings/hooks")
+	validateAndCreateWebhook(c, orCtx, w)
 }
 
-func DingtalkHooksNewPost(c *context.Context, f form.NewDingtalkHook) {
-	c.Data["Title"] = c.Tr("repo.settings")
-	c.Data["PageIsSettingsHooks"] = true
-	c.Data["PageIsSettingsHooksNew"] = true
-	c.Data["Webhook"] = db.Webhook{HookEvent: &db.HookEvent{}}
-
-	orCtx, err := getOrgRepoCtx(c)
-	if err != nil {
-		c.Error(err, "get organization repository context")
-		return
-	}
-
-	if c.HasError() {
-		c.Success(orCtx.NewTemplate)
-		return
-	}
+func WebhooksDingtalkNewPost(c *context.Context, orCtx *orgRepoContext, f form.NewDingtalkHook) {
+	c.Title("repo.settings.add_webhook")
+	c.PageIs("SettingsHooks")
+	c.PageIs("SettingsHooksNew")
+	c.Data["HookType"] = "dingtalk"
 
 	w := &db.Webhook{
 		RepoID:       orCtx.RepoID,
 		URL:          f.PayloadURL,
 		ContentType:  db.JSON,
-		HookEvent:    ParseHookEvent(f.Webhook),
+		HookEvent:    toHookEvent(f.Webhook),
 		IsActive:     f.Active,
 		HookTaskType: db.DINGTALK,
 		OrgID:        orCtx.OrgID,
 	}
-	if err := w.UpdateEvent(); err != nil {
-		c.Error(err, "update event")
-		return
-	} else if err := db.CreateWebhook(w); err != nil {
-		c.Error(err, "create webhook")
-		return
-	}
-
-	c.Flash.Success(c.Tr("repo.settings.add_hook_success"))
-	c.Redirect(orCtx.Link + "/settings/hooks")
+	validateAndCreateWebhook(c, orCtx, w)
 }
 
-func checkWebhook(c *context.Context) (*OrgRepoCtx, *db.Webhook) {
-	c.Data["RequireHighlightJS"] = true
-
-	orCtx, err := getOrgRepoCtx(c)
-	if err != nil {
-		c.Error(err, "get organization repository context")
-		return nil, nil
-	}
-	c.Data["BaseLink"] = orCtx.Link
+func loadWebhook(c *context.Context, orCtx *orgRepoContext) *db.Webhook {
+	c.RequireHighlightJS()
 
+	var err error
 	var w *db.Webhook
 	if orCtx.RepoID > 0 {
 		w, err = db.GetWebhookOfRepoByID(c.Repo.Repository.ID, c.ParamsInt64(":id"))
@@ -325,70 +316,61 @@ func checkWebhook(c *context.Context) (*OrgRepoCtx, *db.Webhook) {
 	}
 	if err != nil {
 		c.NotFoundOrError(err, "get webhook")
-		return nil, nil
+		return nil
 	}
+	c.Data["Webhook"] = w
 
 	switch w.HookTaskType {
 	case db.SLACK:
-		c.Data["SlackHook"] = w.GetSlackHook()
+		c.Data["SlackMeta"] = w.SlackMeta()
 		c.Data["HookType"] = "slack"
 	case db.DISCORD:
-		c.Data["SlackHook"] = w.GetSlackHook()
+		c.Data["SlackMeta"] = w.SlackMeta()
 		c.Data["HookType"] = "discord"
 	case db.DINGTALK:
 		c.Data["HookType"] = "dingtalk"
 	default:
 		c.Data["HookType"] = "gogs"
 	}
+	c.Data["FormURL"] = fmt.Sprintf("%s/settings/hooks/%s/%d", orCtx.Link, c.Data["HookType"], w.ID)
+	c.Data["DeleteURL"] = fmt.Sprintf("%s/settings/hooks/delete", orCtx.Link)
 
 	c.Data["History"], err = w.History(1)
 	if err != nil {
 		c.Error(err, "get history")
-		return nil, nil
+		return nil
 	}
-	return orCtx, w
+	return w
 }
 
-func WebHooksEdit(c *context.Context) {
-	c.Data["Title"] = c.Tr("repo.settings.update_webhook")
-	c.Data["PageIsSettingsHooks"] = true
-	c.Data["PageIsSettingsHooksEdit"] = true
+func WebhooksEdit(c *context.Context, orCtx *orgRepoContext) {
+	c.Title("repo.settings.update_webhook")
+	c.PageIs("SettingsHooks")
+	c.PageIs("SettingsHooksEdit")
 
-	orCtx, w := checkWebhook(c)
+	loadWebhook(c, orCtx)
 	if c.Written() {
 		return
 	}
-	c.Data["Webhook"] = w
 
-	c.Success(orCtx.NewTemplate)
+	c.Success(orCtx.TmplNew)
 }
 
-func WebHooksEditPost(c *context.Context, f form.NewWebhook) {
-	c.Data["Title"] = c.Tr("repo.settings.update_webhook")
-	c.Data["PageIsSettingsHooks"] = true
-	c.Data["PageIsSettingsHooksEdit"] = true
-
-	orCtx, w := checkWebhook(c)
-	if c.Written() {
-		return
-	}
+func validateAndUpdateWebhook(c *context.Context, orCtx *orgRepoContext, w *db.Webhook) {
 	c.Data["Webhook"] = w
 
 	if c.HasError() {
-		c.Success(orCtx.NewTemplate)
+		c.Success(orCtx.TmplNew)
 		return
 	}
 
-	contentType := db.JSON
-	if db.HookContentType(f.ContentType) == db.FORM {
-		contentType = db.FORM
+	field, msg, ok := validateWebhook(c.User, c.Locale, w)
+	if !ok {
+		c.FormErr(field)
+		c.RenderWithErr(msg, orCtx.TmplNew, nil)
+		return
 	}
 
-	w.URL = f.PayloadURL
-	w.ContentType = contentType
-	w.Secret = f.Secret
-	w.HookEvent = ParseHookEvent(f.Webhook)
-	w.IsActive = f.Active
 	if err := w.UpdateEvent(); err != nil {
 		c.Error(err, "update event")
 		return
@@ -401,19 +383,36 @@ func WebHooksEditPost(c *context.Context, f form.NewWebhook) {
 	c.Redirect(fmt.Sprintf("%s/settings/hooks/%d", orCtx.Link, w.ID))
 }
 
-func SlackHooksEditPost(c *context.Context, f form.NewSlackHook) {
-	c.Data["Title"] = c.Tr("repo.settings")
-	c.Data["PageIsSettingsHooks"] = true
-	c.Data["PageIsSettingsHooksEdit"] = true
+func WebhooksEditPost(c *context.Context, orCtx *orgRepoContext, f form.NewWebhook) {
+	c.Title("repo.settings.update_webhook")
+	c.PageIs("SettingsHooks")
+	c.PageIs("SettingsHooksEdit")
 
-	orCtx, w := checkWebhook(c)
+	w := loadWebhook(c, orCtx)
 	if c.Written() {
 		return
 	}
-	c.Data["Webhook"] = w
 
-	if c.HasError() {
-		c.Success(orCtx.NewTemplate)
+	contentType := db.JSON
+	if db.HookContentType(f.ContentType) == db.FORM {
+		contentType = db.FORM
+	}
+
+	w.URL = f.PayloadURL
+	w.ContentType = contentType
+	w.Secret = f.Secret
+	w.HookEvent = toHookEvent(f.Webhook)
+	w.IsActive = f.Active
+	validateAndUpdateWebhook(c, orCtx, w)
+}
+
+func WebhooksSlackEditPost(c *context.Context, orCtx *orgRepoContext, f form.NewSlackHook) {
+	c.Title("repo.settings.update_webhook")
+	c.PageIs("SettingsHooks")
+	c.PageIs("SettingsHooksEdit")
+
+	w := loadWebhook(c, orCtx)
+	if c.Written() {
 		return
 	}
 
@@ -430,36 +429,20 @@ func SlackHooksEditPost(c *context.Context, f form.NewSlackHook) {
 
 	w.URL = f.PayloadURL
 	w.Meta = string(meta)
-	w.HookEvent = ParseHookEvent(f.Webhook)
+	w.HookEvent = toHookEvent(f.Webhook)
 	w.IsActive = f.Active
-	if err := w.UpdateEvent(); err != nil {
-		c.Error(err, "update event")
-		return
-	} else if err := db.UpdateWebhook(w); err != nil {
-		c.Error(err, "update webhook")
-		return
-	}
-
-	c.Flash.Success(c.Tr("repo.settings.update_hook_success"))
-	c.Redirect(fmt.Sprintf("%s/settings/hooks/%d", orCtx.Link, w.ID))
+	validateAndUpdateWebhook(c, orCtx, w)
 }
 
-// FIXME: merge logic to Slack
-func DiscordHooksEditPost(c *context.Context, f form.NewDiscordHook) {
-	c.Data["Title"] = c.Tr("repo.settings")
-	c.Data["PageIsSettingsHooks"] = true
-	c.Data["PageIsSettingsHooksEdit"] = true
+func WebhooksDiscordEditPost(c *context.Context, orCtx *orgRepoContext, f form.NewDiscordHook) {
+	c.Title("repo.settings.update_webhook")
+	c.PageIs("SettingsHooks")
+	c.PageIs("SettingsHooksEdit")
 
-	orCtx, w := checkWebhook(c)
+	w := loadWebhook(c, orCtx)
 	if c.Written() {
 		return
 	}
-	c.Data["Webhook"] = w
-
-	if c.HasError() {
-		c.Success(orCtx.NewTemplate)
-		return
-	}
 
 	meta, err := jsoniter.Marshal(&db.SlackMeta{
 		Username: f.Username,
@@ -473,53 +456,28 @@ func DiscordHooksEditPost(c *context.Context, f form.NewDiscordHook) {
 
 	w.URL = f.PayloadURL
 	w.Meta = string(meta)
-	w.HookEvent = ParseHookEvent(f.Webhook)
+	w.HookEvent = toHookEvent(f.Webhook)
 	w.IsActive = f.Active
-	if err := w.UpdateEvent(); err != nil {
-		c.Error(err, "update event")
-		return
-	} else if err := db.UpdateWebhook(w); err != nil {
-		c.Error(err, "update webhook")
-		return
-	}
-
-	c.Flash.Success(c.Tr("repo.settings.update_hook_success"))
-	c.Redirect(fmt.Sprintf("%s/settings/hooks/%d", orCtx.Link, w.ID))
+	validateAndUpdateWebhook(c, orCtx, w)
 }
 
-func DingtalkHooksEditPost(c *context.Context, f form.NewDingtalkHook) {
-	c.Data["Title"] = c.Tr("repo.settings")
-	c.Data["PageIsSettingsHooks"] = true
-	c.Data["PageIsSettingsHooksEdit"] = true
+func WebhooksDingtalkEditPost(c *context.Context, orCtx *orgRepoContext, f form.NewDingtalkHook) {
+	c.Title("repo.settings.update_webhook")
+	c.PageIs("SettingsHooks")
+	c.PageIs("SettingsHooksEdit")
 
-	orCtx, w := checkWebhook(c)
+	w := loadWebhook(c, orCtx)
 	if c.Written() {
 		return
 	}
-	c.Data["Webhook"] = w
-
-	if c.HasError() {
-		c.Success(orCtx.NewTemplate)
-		return
-	}
 
 	w.URL = f.PayloadURL
-	w.HookEvent = ParseHookEvent(f.Webhook)
+	w.HookEvent = toHookEvent(f.Webhook)
 	w.IsActive = f.Active
-	if err := w.UpdateEvent(); err != nil {
-		c.Error(err, "update event")
-		return
-	} else if err := db.UpdateWebhook(w); err != nil {
-		c.Error(err, "update webhook")
-		return
-	}
-
-	c.Flash.Success(c.Tr("repo.settings.update_hook_success"))
-	c.Redirect(fmt.Sprintf("%s/settings/hooks/%d", orCtx.Link, w.ID))
+	validateAndUpdateWebhook(c, orCtx, w)
 }
 
 func TestWebhook(c *context.Context) {
-
 	var (
 		commitID          string
 		commitMessage     string
@@ -634,14 +592,20 @@ func RedeliveryWebhook(c *context.Context) {
 	c.Status(http.StatusOK)
 }
 
-func DeleteWebhook(c *context.Context) {
-	if err := db.DeleteWebhookOfRepoByID(c.Repo.Repository.ID, c.QueryInt64("id")); err != nil {
-		c.Flash.Error("DeleteWebhookByRepoID: " + err.Error())
+func DeleteWebhook(c *context.Context, orCtx *orgRepoContext) {
+	var err error
+	if orCtx.RepoID > 0 {
+		err = db.DeleteWebhookOfRepoByID(orCtx.RepoID, c.QueryInt64("id"))
 	} else {
-		c.Flash.Success(c.Tr("repo.settings.webhook_deletion_success"))
+		err = db.DeleteWebhookOfOrgByID(orCtx.OrgID, c.QueryInt64("id"))
+	}
+	if err != nil {
+		c.Error(err, "delete webhook")
+		return
 	}
+	c.Flash.Success(c.Tr("repo.settings.webhook_deletion_success"))
 
 	c.JSONSuccess(map[string]interface{}{
-		"redirect": c.Repo.RepoLink + "/settings/hooks",
+		"redirect": orCtx.Link + "/settings/hooks",
 	})
 }

internal/route/repo/webhook_test.go

@@ -0,0 +1,72 @@
+// 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 (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+
+	"gogs.io/gogs/internal/db"
+	"gogs.io/gogs/internal/mock"
+)
+
+func Test_isLocalHostname(t *testing.T) {
+	tests := []struct {
+		hostname string
+		want     bool
+	}{
+		{hostname: "localhost", want: true},
+		{hostname: "127.0.0.1", want: true},
+		{hostname: "::1", want: true},
+		{hostname: "0:0:0:0:0:0:0:1", want: true},
+
+		{hostname: "gogs.io", want: false},
+	}
+	for _, test := range tests {
+		t.Run("", func(t *testing.T) {
+			assert.Equal(t, test.want, isLocalHostname(test.hostname))
+		})
+	}
+}
+
+func Test_validateWebhook(t *testing.T) {
+	l := mock.NewLocale("en", func(s string, _ ...interface{}) string {
+		return s
+	})
+
+	tests := []struct {
+		name     string
+		actor    *db.User
+		webhook  *db.Webhook
+		expField string
+		expMsg   string
+		expOK    bool
+	}{
+		{
+			name:    "admin bypass local address check",
+			actor:   &db.User{IsAdmin: true},
+			webhook: &db.Webhook{URL: "http://localhost:3306"},
+			expOK:   true,
+		},
+
+		{
+			name:     "local address not allowed",
+			actor:    &db.User{},
+			webhook:  &db.Webhook{URL: "http://localhost:3306"},
+			expField: "PayloadURL",
+			expMsg:   "repo.settings.webhook.err_cannot_use_local_addresses",
+			expOK:    false,
+		},
+	}
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			field, msg, ok := validateWebhook(test.actor, l, test.webhook)
+			assert.Equal(t, test.expOK, ok)
+			assert.Equal(t, test.expMsg, msg)
+			assert.Equal(t, test.expField, field)
+		})
+	}
+}

public/less/_form.less

@@ -1,50 +1,50 @@
 .form {
-	.help {
-		color: #999999;
-	  padding-top: .6em;
-	  padding-bottom: .6em;
-	  display: inline-block;
-	  word-break: break-word;
-	}
+  .help {
+    color: #999999;
+    padding-top: 0.6em;
+    padding-bottom: 0.6em;
+    display: inline-block;
+    word-break: break-word;
+  }
 }
 .ui.attached.header {
-	background: #f0f0f0;
-	.right {
-		margin-top: -5px;
-		.button {
-			padding: 8px 10px;
-			font-weight: normal;
-		}
-	}
+  background: #f0f0f0;
+  .right {
+    margin-top: -5px;
+    .button {
+      padding: 8px 10px;
+      font-weight: normal;
+    }
+  }
 }
 
 @create-page-form-input-padding: 250px !important;
 #create-page-form {
-	form {
-		margin: auto;
-		width: 800px!important;
-		.ui.message {
-			text-align: center;
-		}
-		.header {
-			padding-left: @create-page-form-input-padding+30px;
-		}
-		.inline.field > label {
-			text-align: right;
-			width: @create-page-form-input-padding;
-			word-wrap: break-word;
-		}
-		.help {
-			margin-left: @create-page-form-input-padding+15px;
-		}
-		.optional .title {
-			margin-left: @create-page-form-input-padding;
-		}
-		input,
-		textarea {
-			width: 50% !important;
-		}
-	}
+  form {
+    margin: auto;
+    width: 800px !important;
+    .ui.message {
+      text-align: center;
+    }
+    .header {
+      padding-left: @create-page-form-input-padding+30px;
+    }
+    .inline.field > label {
+      text-align: right;
+      width: @create-page-form-input-padding;
+      word-wrap: break-word;
+    }
+    .help {
+      margin-left: @create-page-form-input-padding+15px;
+    }
+    .optional .title {
+      margin-left: @create-page-form-input-padding;
+    }
+    input,
+    textarea {
+      width: 50% !important;
+    }
+  }
 }
 
 .user.activate,
@@ -52,72 +52,85 @@
 .user.reset.password,
 .user.signin,
 .user.signup {
-	@input-padding: 200px !important;
-	#create-page-form;
-	form {
-		width: 700px !important;
-		.header {
-			padding-left: @input-padding+30px;
-		}
-		.inline.field > label {
-			width: @input-padding;
-		}
-	}
+  @input-padding: 200px !important;
+  #create-page-form;
+  form {
+    width: 700px !important;
+    .header {
+      padding-left: @input-padding+30px;
+    }
+    .inline.field > label {
+      width: @input-padding;
+    }
+  }
 }
 
 .user.signin.two-factor {
-	form {
-		width: 300px !important;
-		.header {
-			padding-left: inherit !important;
-		}
-	}
+  form {
+    width: 300px !important;
+    .header {
+      padding-left: inherit !important;
+    }
+  }
 }
 
 .repository {
-	&.new.repo,
-	&.new.migrate,
-	&.new.fork {
-		#create-page-form;
-		form {
-			.dropdown {
-				.dropdown.icon {
-					margin-top: -7px!important;
-				}
-				.text {
-					margin-right: 0!important;
-					i {
-						margin-right: 0!important;
-					}
-				}
-			}
-		}
-	}
+  &.new.repo,
+  &.new.migrate,
+  &.new.fork {
+    #create-page-form;
+    form {
+      .dropdown {
+        .dropdown.icon {
+          margin-top: -7px !important;
+        }
+        .text {
+          margin-right: 0 !important;
+          i {
+            margin-right: 0 !important;
+          }
+        }
+      }
+    }
+  }
 
-	&.new.repo {
-		.ui.form {
-			.selection.dropdown:not(.owner) {
-				width: 50%!important;
-			}
-			#auto-init {
-				margin-left: @create-page-form-input-padding+15px;
-			}
-		}
-	}
+  &.new.repo {
+    .ui.form {
+      .selection.dropdown:not(.owner) {
+        width: 50% !important;
+      }
+      #auto-init {
+        margin-left: @create-page-form-input-padding+15px;
+      }
+    }
+  }
 }
 
 .new.webhook {
-	form {
-		.help {
-			margin-left: 25px;
-		}
-	}
+  form {
+    .text.desc {
+      margin-top: 5px;
+    }
+    .help {
+      margin-left: 25px;
+    }
+    .events {
+      .column {
+        padding-bottom: 0;
+      }
+      .help {
+        font-size: 13px;
+        margin-left: 26px;
+        padding-top: 0;
+      }
+    }
+  }
 }
 
 .new.webhook {
-	.events.fields {
-		.column {
-			padding-left: 40px;
-		}
-	}
+  .events.fields {
+    .column {
+      padding-left: 40px;
+    }
+  }
 }

public/less/_repository.less

@@ -1436,22 +1436,6 @@
         margin-top: -4px;
       }
     }
-
-    &.webhook {
-      .text.desc {
-        margin-top: 5px;
-      }
-      .events {
-        .column {
-          padding-bottom: 0;
-        }
-        .help {
-          font-size: 13px;
-          margin-left: 26px;
-          padding-top: 0;
-        }
-      }
-    }
   }
 }
 // End of .repository
@@ -1605,11 +1589,11 @@
     }
   }
   .hook.list {
-    > .item:not(:first-child) {
-      border-top: 1px solid #eaeaea;
+    > .item:not(:last-child) {
+      border-bottom: 1px solid #eaeaea;
     }
     .item {
-      padding: 10px 20px;
+      padding: 10px 0;
       .octicon,
       .fa {
         width: 20px;
@@ -1622,7 +1606,7 @@
   }
   .hook.history.list {
     .item {
-      padding-left: 13px;
+      padding: 10px 20px;
       .meta {
         .ui.right {
           margin-top: 5px;

templates/repo/settings/githooks.tmpl

@@ -9,11 +9,9 @@
 				<h4 class="ui top attached header">
 					{{.i18n.Tr "repo.settings.githooks"}}
 				</h4>
-				<div class="ui attached table segment">
+				<div class="ui attached segment">
+					<p>{{.i18n.Tr "repo.settings.githooks_desc"}}</p>
 					<div class="ui hook list">
-						<div class="item">
-							{{.i18n.Tr "repo.settings.githooks_desc" | Str2HTML}}
-						</div>
 						{{range .Hooks}}
 							<div class="item">
 								<span class="text {{if not .IsSample}}green{{else}}grey{{end}}"><i class="octicon octicon-primitive-dot"></i></span>

templates/repo/settings/webhook/dingtalk.tmpl

@@ -1,6 +1,6 @@
 {{if eq .HookType "dingtalk"}}
 	<p>{{.i18n.Tr "repo.settings.add_dingtalk_hook_desc" "https://open-doc.dingtalk.com/" | Str2HTML}}</p>
-	<form class="ui form" action="{{.BaseLink}}/settings/hooks/dingtalk/{{if .PageIsSettingsHooksNew}}new{{else}}{{.Webhook.ID}}{{end}}" method="post">
+	<form class="ui form" action="{{if .PageIsSettingsHooksNew}}{{$.Link}}{{else}}{{.FormURL}}{{end}}" method="post">
 		{{.CSRFTokenHTML}}
 		<div class="required field {{if .Err_PayloadURL}}error{{end}}">
 			<label for="payload_url">{{.i18n.Tr "repo.settings.payload_url"}}</label>

templates/repo/settings/webhook/discord.tmpl

@@ -1,6 +1,6 @@
 {{if eq .HookType "discord"}}
 	<p>{{.i18n.Tr "repo.settings.add_discord_hook_desc" "https://discordapp.com/" | Str2HTML}}</p>
-	<form class="ui form" action="{{.BaseLink}}/settings/hooks/discord/{{if .PageIsSettingsHooksNew}}new{{else}}{{.Webhook.ID}}{{end}}" method="post">
+	<form class="ui form" action="{{if .PageIsSettingsHooksNew}}{{$.Link}}{{else}}{{.FormURL}}{{end}}" method="post">
 		{{.CSRFTokenHTML}}
 		<div class="required field {{if .Err_PayloadURL}}error{{end}}">
 			<label for="payload_url">{{.i18n.Tr "repo.settings.payload_url"}}</label>
@@ -9,15 +9,15 @@
 
 		<div class="field">
 			<label for="username">{{.i18n.Tr "repo.settings.slack_username"}}</label>
-			<input id="username" name="username" value="{{.SlackHook.Username}}" placeholder="e.g. Gogs">
+			<input id="username" name="username" value="{{.SlackMeta.Username}}" placeholder="e.g. Gogs">
 		</div>
 		<div class="field">
 			<label for="icon_url">{{.i18n.Tr "repo.settings.slack_icon_url"}}</label>
-			<input id="icon_url" name="icon_url" value="{{.SlackHook.IconURL}}" placeholder="e.g. https://example.com/img/favicon.png">
+			<input id="icon_url" name="icon_url" value="{{.SlackMeta.IconURL}}" placeholder="e.g. https://example.com/img/favicon.png">
 		</div>
 		<div class="field">
 			<label for="color">{{.i18n.Tr "repo.settings.slack_color"}}</label>
-			<input id="color" name="color" value="{{.SlackHook.Color}}" placeholder="e.g. #dd4b39">
+			<input id="color" name="color" value="{{.SlackMeta.Color}}" placeholder="e.g. #dd4b39">
 		</div>
 		{{template "repo/settings/webhook/settings" .}}
 	</form>

templates/repo/settings/webhook/gogs.tmpl

@@ -1,6 +1,6 @@
 {{if eq .HookType "gogs"}}
-	<p>{{.i18n.Tr "repo.settings.add_webhook_desc" "https://gogs.io/docs/features/webhook.html" | Str2HTML}}</p>
-	<form class="ui form" action="{{.BaseLink}}/settings/hooks/gogs/{{if .PageIsSettingsHooksNew}}new{{else}}{{.Webhook.ID}}{{end}}" method="post">
+	<p>{{.i18n.Tr "repo.settings.add_webhook_desc" "https://gogs.io/docs/features/webhook.html" | Safe}}</p>
+	<form class="ui form" action="{{if .PageIsSettingsHooksNew}}{{$.Link}}{{else}}{{.FormURL}}{{end}}" method="post">
 		{{.CSRFTokenHTML}}
 		<div class="required field {{if .Err_PayloadURL}}error{{end}}">
 			<label for="payload_url">{{.i18n.Tr "repo.settings.payload_url"}}</label>

templates/repo/settings/webhook/list.tmpl

@@ -2,40 +2,10 @@
 	{{template "base/alert" .}}
 	<h4 class="ui top attached header">
 		{{.i18n.Tr "repo.settings.hooks"}}
-		<div class="ui right">
-			<div class="ui types jump dropdown">
-				{{if .Types}}
-					<div class="ui blue tiny button">{{.i18n.Tr "repo.settings.add_webhook"}}</div>
-					<div class="menu">
-						{{range .Types}}
-							{{if eq . "gogs"}}
-								<a class="item logo" href="{{$.BaseLink}}/settings/hooks/gogs/new">
-									<img class="img-12" src="{{AppSubURL}}/img/favicon.png">Gogs
-								</a>
-							{{else if eq . "slack"}}
-								<a class="item logo" href="{{$.BaseLink}}/settings/hooks/slack/new">
-									<img class="img-12" src="{{AppSubURL}}/img/slack.png">Slack
-								</a>
-							{{else if eq . "discord"}}
-								<a class="item logo" href="{{$.BaseLink}}/settings/hooks/discord/new">
-									<img class="img-12" src="{{AppSubURL}}/img/discord.png">Discord
-								</a>
-							{{else if eq . "dingtalk"}}
-								<a class="item logo" href="{{$.BaseLink}}/settings/hooks/dingtalk/new">
-									<img class="img-12" src="{{AppSubURL}}/img/dingtalk.png">Dingtalk
-								</a>
-							{{end}}
-						{{end}}
-					</div>
-				{{end}}
-			</div>
-		</div>
 	</h4>
-	<div class="ui attached table segment">
+	<div class="ui attached segment">
+		<p>{{.Description | Safe}}</p>
 		<div class="ui hook list">
-			<div class="item">
-				{{.Description | Str2HTML}}
-			</div>
 			{{range .Webhooks}}
 				<div class="item">
 					{{if eq .LastStatus 1}}
@@ -45,15 +15,43 @@
 					{{else}}
 						<span class="text grey"><i class="octicon octicon-primitive-dot"></i></span>
 					{{end}}
-					<a href="{{$.BaseLink}}/settings/hooks/{{.ID}}">{{.URL}}</a>
+					<a href="{{$.Link}}/{{.ID}}">{{.URL}}</a>
 					<div class="ui right">
-						<span class="text blue"><a href="{{$.BaseLink}}/settings/hooks/{{.ID}}"><i class="fa fa-pencil"></i></a></span>
-						<span class="text red"><a class="delete-button" data-url="{{$.Link}}/delete" data-id="{{.ID}}"><i class="fa fa-times"></i></a></span>
+						<a class="text blue" href="{{$.Link}}/{{.ID}}"><i class="fa fa-pencil"></i></a>
+						<a class="text red delete-button" data-url="{{$.Link}}/delete" data-id="{{.ID}}"><i class="fa fa-trash-o"></i></a>
 					</div>
 				</div>
 			{{end}}
 		</div>
 	</div>
+	<div class="ui bottom attached segment">
+		<span><b>{{.i18n.Tr "repo.settings.webhooks.add_new"}}</b></span>
+		<div class="ui selection jump dropdown">
+			<i class="dropdown icon"></i>
+			<div class="default text">{{.i18n.Tr "repo.settings.webhooks.choose_a_type"}}</div>
+			<div class="menu">
+				{{range .Types}}
+					{{if eq . "gogs"}}
+						<a class="item logo" href="{{$.Link}}/gogs/new">
+							<img class="img-12" src="{{AppSubURL}}/img/favicon.png">Gogs
+						</a>
+					{{else if eq . "slack"}}
+						<a class="item logo" href="{{$.Link}}/slack/new">
+							<img class="img-12" src="{{AppSubURL}}/img/slack.png">Slack
+						</a>
+					{{else if eq . "discord"}}
+						<a class="item logo" href="{{$.Link}}/discord/new">
+							<img class="img-12" src="{{AppSubURL}}/img/discord.png">Discord
+						</a>
+					{{else if eq . "dingtalk"}}
+						<a class="item logo" href="{{$.Link}}/dingtalk/new">
+							<img class="img-12" src="{{AppSubURL}}/img/dingtalk.png">Dingtalk
+						</a>
+					{{end}}
+				{{end}}
+			</div>
+		</div>
+	</div>
 </div>
 
 {{template "repo/settings/webhook/delete_modal" .}}

templates/repo/settings/webhook/settings.tmpl

@@ -119,7 +119,7 @@
 		<button class="ui green button">{{.i18n.Tr "repo.settings.add_webhook"}}</button>
 	{{else}}
 		<button class="ui green button">{{.i18n.Tr "repo.settings.update_webhook"}}</button>
-		<a class="ui red delete-button button" data-url="{{.BaseLink}}/settings/hooks/delete" data-id="{{.Webhook.ID}}">{{.i18n.Tr "repo.settings.delete_webhook"}}</a>
+		<a class="ui red delete-button button" data-url="{{.DeleteURL}}" data-id="{{.Webhook.ID}}">{{.i18n.Tr "repo.settings.delete_webhook"}}</a>
 	{{end}}
 </div>
 

templates/repo/settings/webhook/slack.tmpl

@@ -1,6 +1,6 @@
 {{if eq .HookType "slack"}}
-	<p>{{.i18n.Tr "repo.settings.add_slack_hook_desc" "https://slack.com" | Str2HTML}}</p>
-	<form class="ui form" action="{{.BaseLink}}/settings/hooks/slack/{{if .PageIsSettingsHooksNew}}new{{else}}{{.Webhook.ID}}{{end}}" method="post">
+	<p>{{.i18n.Tr "repo.settings.add_slack_hook_desc" "https://slack.com" | Safe}}</p>
+	<form class="ui form" action="{{if .PageIsSettingsHooksNew}}{{$.Link}}{{else}}{{.FormURL}}{{end}}" method="post">
 		{{.CSRFTokenHTML}}
 		<div class="required field {{if .Err_PayloadURL}}error{{end}}">
 			<label for="payload_url">{{.i18n.Tr "repo.settings.payload_url"}}</label>
@@ -8,20 +8,20 @@
 		</div>
 		<div class="required field {{if .Err_Channel}}error{{end}}">
 			<label for="channel">{{.i18n.Tr "repo.settings.slack_channel"}}</label>
-			<input id="channel" name="channel" value="{{.SlackHook.Channel}}" placeholder="e.g. #general" required>
+			<input id="channel" name="channel" value="{{.SlackMeta.Channel}}" placeholder="e.g. #general" required>
 		</div>
 
 		<div class="field">
 			<label for="username">{{.i18n.Tr "repo.settings.slack_username"}}</label>
-			<input id="username" name="username" value="{{.SlackHook.Username}}" placeholder="e.g. Gogs">
+			<input id="username" name="username" value="{{.SlackMeta.Username}}" placeholder="e.g. Gogs">
 		</div>
 		<div class="field">
 			<label for="icon_url">{{.i18n.Tr "repo.settings.slack_icon_url"}}</label>
-			<input id="icon_url" name="icon_url" value="{{.SlackHook.IconURL}}" placeholder="e.g. https://example.com/img/favicon.png">
+			<input id="icon_url" name="icon_url" value="{{.SlackMeta.IconURL}}" placeholder="e.g. https://example.com/img/favicon.png">
 		</div>
 		<div class="field">
 			<label for="color">{{.i18n.Tr "repo.settings.slack_color"}}</label>
-			<input id="color" name="color" value="{{.SlackHook.Color}}" placeholder="e.g. #dd4b39, good, warning, danger">
+			<input id="color" name="color" value="{{.SlackMeta.Color}}" placeholder="e.g. #dd4b39, good, warning, danger">
 		</div>
 		{{template "repo/settings/webhook/settings" .}}
 	</form>