Refactor Webhook + Add X-Hub-Signature (#16176)

This PR removes multiple unneeded fields from the `HookTask` struct and adds the two headers `X-Hub-Signature` and `X-Hub-Signature-256`.

## ⚠️ BREAKING ⚠️ 

* The `Secret` field is no longer passed as part of the payload.
* "Breaking" change (or fix?): The webhook history shows the real called url and not the url registered in the webhook (`deliver.go`@129).

Close #16115
Fixes #7788
Fixes #11755

Co-authored-by: zeripath <art27@cantab.net>

models/migrations/migrations.go

@@ -323,6 +323,8 @@ var migrations = []Migration{
 	NewMigration("Add new table repo_archiver", addRepoArchiver),
 	// v186 -> v187
 	NewMigration("Create protected tag table", createProtectedTagTable),
+	// v187 -> v188
+	NewMigration("Drop unneeded webhook related columns", dropWebhookColumns),
 }
 
 // GetCurrentDBVersion returns the current db version

models/migrations/v187.go

@@ -0,0 +1,46 @@
+// Copyright 2021 The Gitea 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 migrations
+
+import (
+	"xorm.io/xorm"
+)
+
+func dropWebhookColumns(x *xorm.Engine) error {
+	// Make sure the columns exist before dropping them
+	type Webhook struct {
+		Signature string `xorm:"TEXT"`
+		IsSSL     bool   `xorm:"is_ssl"`
+	}
+	if err := x.Sync2(new(Webhook)); err != nil {
+		return err
+	}
+
+	type HookTask struct {
+		Typ         string `xorm:"VARCHAR(16) index"`
+		URL         string `xorm:"TEXT"`
+		Signature   string `xorm:"TEXT"`
+		HTTPMethod  string `xorm:"http_method"`
+		ContentType int
+		IsSSL       bool
+	}
+	if err := x.Sync2(new(HookTask)); err != nil {
+		return err
+	}
+
+	sess := x.NewSession()
+	defer sess.Close()
+	if err := sess.Begin(); err != nil {
+		return err
+	}
+	if err := dropTableColumns(sess, "webhook", "signature", "is_ssl"); err != nil {
+		return err
+	}
+	if err := dropTableColumns(sess, "hook_task", "typ", "url", "signature", "http_method", "content_type", "is_ssl"); err != nil {
+		return err
+	}
+
+	return sess.Commit()
+}

models/webhook.go

@@ -109,6 +109,22 @@ type HookEvent struct {
 	HookEvents `json:"events"`
 }
 
+// HookType is the type of a webhook
+type HookType = string
+
+// Types of webhooks
+const (
+	GITEA    HookType = "gitea"
+	GOGS     HookType = "gogs"
+	SLACK    HookType = "slack"
+	DISCORD  HookType = "discord"
+	DINGTALK HookType = "dingtalk"
+	TELEGRAM HookType = "telegram"
+	MSTEAMS  HookType = "msteams"
+	FEISHU   HookType = "feishu"
+	MATRIX   HookType = "matrix"
+)
+
 // HookStatus is the status of a web hook
 type HookStatus int
 
@@ -126,17 +142,15 @@ type Webhook struct {
 	OrgID           int64 `xorm:"INDEX"`
 	IsSystemWebhook bool
 	URL             string `xorm:"url TEXT"`
-	Signature       string `xorm:"TEXT"`
 	HTTPMethod      string `xorm:"http_method"`
 	ContentType     HookContentType
 	Secret          string `xorm:"TEXT"`
 	Events          string `xorm:"TEXT"`
 	*HookEvent      `xorm:"-"`
-	IsSSL           bool         `xorm:"is_ssl"`
+	IsActive        bool       `xorm:"INDEX"`
-	IsActive        bool         `xorm:"INDEX"`
+	Type            HookType   `xorm:"VARCHAR(16) 'type'"`
-	Type            HookTaskType `xorm:"VARCHAR(16) 'type'"`
+	Meta            string     `xorm:"TEXT"` // store hook-specific attributes
-	Meta            string       `xorm:"TEXT"` // store hook-specific attributes
+	LastStatus      HookStatus // Last delivery status
-	LastStatus      HookStatus   // Last delivery status
 
 	CreatedUnix timeutil.TimeStamp `xorm:"INDEX created"`
 	UpdatedUnix timeutil.TimeStamp `xorm:"INDEX updated"`
@@ -558,22 +572,6 @@ func copyDefaultWebhooksToRepo(e Engine, repoID int64) error {
 //  \___|_  / \____/ \____/|__|_ \ |____|  (____  /____  >__|_ \
 //        \/                    \/              \/     \/     \/
 
-// HookTaskType is the type of an hook task
-type HookTaskType = string
-
-// Types of hook tasks
-const (
-	GITEA    HookTaskType = "gitea"
-	GOGS     HookTaskType = "gogs"
-	SLACK    HookTaskType = "slack"
-	DISCORD  HookTaskType = "discord"
-	DINGTALK HookTaskType = "dingtalk"
-	TELEGRAM HookTaskType = "telegram"
-	MSTEAMS  HookTaskType = "msteams"
-	FEISHU   HookTaskType = "feishu"
-	MATRIX   HookTaskType = "matrix"
-)
-
 // HookEventType is the type of an hook event
 type HookEventType string
 
@@ -635,7 +633,9 @@ func (h HookEventType) Event() string {
 
 // HookRequest represents hook task request information.
 type HookRequest struct {
-	Headers map[string]string `json:"headers"`
+	URL        string            `json:"url"`
+	HTTPMethod string            `json:"http_method"`
+	Headers    map[string]string `json:"headers"`
 }
 
 // HookResponse represents hook task response information.
@@ -651,15 +651,9 @@ type HookTask struct {
 	RepoID          int64 `xorm:"INDEX"`
 	HookID          int64
 	UUID            string
-	Typ             HookTaskType `xorm:"VARCHAR(16) index"`
-	URL             string       `xorm:"TEXT"`
-	Signature       string       `xorm:"TEXT"`
 	api.Payloader   `xorm:"-"`
 	PayloadContent  string `xorm:"TEXT"`
-	HTTPMethod      string `xorm:"http_method"`
-	ContentType     HookContentType
 	EventType       HookEventType
-	IsSSL           bool
 	IsDelivered     bool
 	Delivered       int64
 	DeliveredString string `xorm:"-"`

models/webhook_test.go

@@ -207,8 +207,6 @@ func TestCreateHookTask(t *testing.T) {
 	hookTask := &HookTask{
 		RepoID:    3,
 		HookID:    3,
-		Typ:       GITEA,
-		URL:       "http://www.example.com/unit_test",
 		Payloader: &api.PushPayload{},
 	}
 	AssertNotExistsBean(t, hookTask)
@@ -233,8 +231,6 @@ func TestCleanupHookTaskTable_PerWebhook_DeletesDelivered(t *testing.T) {
 	hookTask := &HookTask{
 		RepoID:      3,
 		HookID:      3,
-		Typ:         GITEA,
-		URL:         "http://www.example.com/unit_test",
 		Payloader:   &api.PushPayload{},
 		IsDelivered: true,
 		Delivered:   time.Now().UnixNano(),
@@ -252,8 +248,6 @@ func TestCleanupHookTaskTable_PerWebhook_LeavesUndelivered(t *testing.T) {
 	hookTask := &HookTask{
 		RepoID:      2,
 		HookID:      4,
-		Typ:         GITEA,
-		URL:         "http://www.example.com/unit_test",
 		Payloader:   &api.PushPayload{},
 		IsDelivered: false,
 	}
@@ -270,8 +264,6 @@ func TestCleanupHookTaskTable_PerWebhook_LeavesMostRecentTask(t *testing.T) {
 	hookTask := &HookTask{
 		RepoID:      2,
 		HookID:      4,
-		Typ:         GITEA,
-		URL:         "http://www.example.com/unit_test",
 		Payloader:   &api.PushPayload{},
 		IsDelivered: true,
 		Delivered:   time.Now().UnixNano(),
@@ -289,8 +281,6 @@ func TestCleanupHookTaskTable_OlderThan_DeletesDelivered(t *testing.T) {
 	hookTask := &HookTask{
 		RepoID:      3,
 		HookID:      3,
-		Typ:         GITEA,
-		URL:         "http://www.example.com/unit_test",
 		Payloader:   &api.PushPayload{},
 		IsDelivered: true,
 		Delivered:   time.Now().AddDate(0, 0, -8).UnixNano(),
@@ -308,8 +298,6 @@ func TestCleanupHookTaskTable_OlderThan_LeavesUndelivered(t *testing.T) {
 	hookTask := &HookTask{
 		RepoID:      2,
 		HookID:      4,
-		Typ:         GITEA,
-		URL:         "http://www.example.com/unit_test",
 		Payloader:   &api.PushPayload{},
 		IsDelivered: false,
 	}
@@ -326,8 +314,6 @@ func TestCleanupHookTaskTable_OlderThan_LeavesTaskEarlierThanAgeToDelete(t *test
 	hookTask := &HookTask{
 		RepoID:      2,
 		HookID:      4,
-		Typ:         GITEA,
-		URL:         "http://www.example.com/unit_test",
 		Payloader:   &api.PushPayload{},
 		IsDelivered: true,
 		Delivered:   time.Now().AddDate(0, 0, -6).UnixNano(),

modules/structs/hook.go

@@ -62,7 +62,6 @@ type EditHookOption struct {
 
 // Payloader payload is some part of one hook
 type Payloader interface {
-	SetSecret(string)
 	JSONPayload() ([]byte, error)
 }
 
@@ -124,7 +123,6 @@ var (
 
 // CreatePayload FIXME
 type CreatePayload struct {
-	Secret  string      `json:"secret"`
 	Sha     string      `json:"sha"`
 	Ref     string      `json:"ref"`
 	RefType string      `json:"ref_type"`
@@ -132,11 +130,6 @@ type CreatePayload struct {
 	Sender  *User       `json:"sender"`
 }
 
-// SetSecret modifies the secret of the CreatePayload
-func (p *CreatePayload) SetSecret(secret string) {
-	p.Secret = secret
-}
-
 // JSONPayload return payload information
 func (p *CreatePayload) JSONPayload() ([]byte, error) {
 	json := jsoniter.ConfigCompatibleWithStandardLibrary
@@ -181,7 +174,6 @@ const (
 
 // DeletePayload represents delete payload
 type DeletePayload struct {
-	Secret     string      `json:"secret"`
 	Ref        string      `json:"ref"`
 	RefType    string      `json:"ref_type"`
 	PusherType PusherType  `json:"pusher_type"`
@@ -189,11 +181,6 @@ type DeletePayload struct {
 	Sender     *User       `json:"sender"`
 }
 
-// SetSecret modifies the secret of the DeletePayload
-func (p *DeletePayload) SetSecret(secret string) {
-	p.Secret = secret
-}
-
 // JSONPayload implements Payload
 func (p *DeletePayload) JSONPayload() ([]byte, error) {
 	json := jsoniter.ConfigCompatibleWithStandardLibrary
@@ -209,17 +196,11 @@ func (p *DeletePayload) JSONPayload() ([]byte, error) {
 
 // ForkPayload represents fork payload
 type ForkPayload struct {
-	Secret string      `json:"secret"`
 	Forkee *Repository `json:"forkee"`
 	Repo   *Repository `json:"repository"`
 	Sender *User       `json:"sender"`
 }
 
-// SetSecret modifies the secret of the ForkPayload
-func (p *ForkPayload) SetSecret(secret string) {
-	p.Secret = secret
-}
-
 // JSONPayload implements Payload
 func (p *ForkPayload) JSONPayload() ([]byte, error) {
 	json := jsoniter.ConfigCompatibleWithStandardLibrary
@@ -238,7 +219,6 @@ const (
 
 // IssueCommentPayload represents a payload information of issue comment event.
 type IssueCommentPayload struct {
-	Secret     string                 `json:"secret"`
 	Action     HookIssueCommentAction `json:"action"`
 	Issue      *Issue                 `json:"issue"`
 	Comment    *Comment               `json:"comment"`
@@ -248,11 +228,6 @@ type IssueCommentPayload struct {
 	IsPull     bool                   `json:"is_pull"`
 }
 
-// SetSecret modifies the secret of the IssueCommentPayload
-func (p *IssueCommentPayload) SetSecret(secret string) {
-	p.Secret = secret
-}
-
 // JSONPayload implements Payload
 func (p *IssueCommentPayload) JSONPayload() ([]byte, error) {
 	json := jsoniter.ConfigCompatibleWithStandardLibrary
@@ -278,18 +253,12 @@ const (
 
 // ReleasePayload represents a payload information of release event.
 type ReleasePayload struct {
-	Secret     string            `json:"secret"`
 	Action     HookReleaseAction `json:"action"`
 	Release    *Release          `json:"release"`
 	Repository *Repository       `json:"repository"`
 	Sender     *User             `json:"sender"`
 }
 
-// SetSecret modifies the secret of the ReleasePayload
-func (p *ReleasePayload) SetSecret(secret string) {
-	p.Secret = secret
-}
-
 // JSONPayload implements Payload
 func (p *ReleasePayload) JSONPayload() ([]byte, error) {
 	json := jsoniter.ConfigCompatibleWithStandardLibrary
@@ -305,7 +274,6 @@ func (p *ReleasePayload) JSONPayload() ([]byte, error) {
 
 // PushPayload represents a payload information of push event.
 type PushPayload struct {
-	Secret     string           `json:"secret"`
 	Ref        string           `json:"ref"`
 	Before     string           `json:"before"`
 	After      string           `json:"after"`
@@ -317,11 +285,6 @@ type PushPayload struct {
 	Sender     *User            `json:"sender"`
 }
 
-// SetSecret modifies the secret of the PushPayload
-func (p *PushPayload) SetSecret(secret string) {
-	p.Secret = secret
-}
-
 // JSONPayload FIXME
 func (p *PushPayload) JSONPayload() ([]byte, error) {
 	json := jsoniter.ConfigCompatibleWithStandardLibrary
@@ -389,7 +352,6 @@ const (
 
 // IssuePayload represents the payload information that is sent along with an issue event.
 type IssuePayload struct {
-	Secret     string          `json:"secret"`
 	Action     HookIssueAction `json:"action"`
 	Index      int64           `json:"number"`
 	Changes    *ChangesPayload `json:"changes,omitempty"`
@@ -398,11 +360,6 @@ type IssuePayload struct {
 	Sender     *User           `json:"sender"`
 }
 
-// SetSecret modifies the secret of the IssuePayload.
-func (p *IssuePayload) SetSecret(secret string) {
-	p.Secret = secret
-}
-
 // JSONPayload encodes the IssuePayload to JSON, with an indentation of two spaces.
 func (p *IssuePayload) JSONPayload() ([]byte, error) {
 	json := jsoniter.ConfigCompatibleWithStandardLibrary
@@ -430,7 +387,6 @@ type ChangesPayload struct {
 
 // PullRequestPayload represents a payload information of pull request event.
 type PullRequestPayload struct {
-	Secret      string          `json:"secret"`
 	Action      HookIssueAction `json:"action"`
 	Index       int64           `json:"number"`
 	Changes     *ChangesPayload `json:"changes,omitempty"`
@@ -440,11 +396,6 @@ type PullRequestPayload struct {
 	Review      *ReviewPayload  `json:"review"`
 }
 
-// SetSecret modifies the secret of the PullRequestPayload.
-func (p *PullRequestPayload) SetSecret(secret string) {
-	p.Secret = secret
-}
-
 // JSONPayload FIXME
 func (p *PullRequestPayload) JSONPayload() ([]byte, error) {
 	json := jsoniter.ConfigCompatibleWithStandardLibrary
@@ -476,18 +427,12 @@ const (
 
 // RepositoryPayload payload for repository webhooks
 type RepositoryPayload struct {
-	Secret       string         `json:"secret"`
 	Action       HookRepoAction `json:"action"`
 	Repository   *Repository    `json:"repository"`
 	Organization *User          `json:"organization"`
 	Sender       *User          `json:"sender"`
 }
 
-// SetSecret modifies the secret of the RepositoryPayload
-func (p *RepositoryPayload) SetSecret(secret string) {
-	p.Secret = secret
-}
-
 // JSONPayload JSON representation of the payload
 func (p *RepositoryPayload) JSONPayload() ([]byte, error) {
 	json := jsoniter.ConfigCompatibleWithStandardLibrary

routers/api/v1/utils/hook.go

@@ -133,7 +133,7 @@ func addHook(ctx *context.APIContext, form *api.CreateHookOption, orgID, repoID
 			BranchFilter: form.BranchFilter,
 		},
 		IsActive: form.Active,
-		Type:     models.HookTaskType(form.Type),
+		Type:     models.HookType(form.Type),
 	}
 	if w.Type == models.SLACK {
 		channel, ok := form.Config["channel"]

routers/web/repo/webhook.go

@@ -239,7 +239,7 @@ func GogsHooksNewPost(ctx *context.Context) {
 }
 
 // newGogsWebhookPost response for creating gogs hook
-func newGogsWebhookPost(ctx *context.Context, form forms.NewGogshookForm, kind models.HookTaskType) {
+func newGogsWebhookPost(ctx *context.Context, form forms.NewGogshookForm, kind models.HookType) {
 	ctx.Data["Title"] = ctx.Tr("repo.settings.add_webhook")
 	ctx.Data["PageIsSettingsHooks"] = true
 	ctx.Data["PageIsSettingsHooksNew"] = true

services/webhook/deliver.go

@@ -6,8 +6,13 @@ package webhook
 
 import (
 	"context"
+	"crypto/hmac"
+	"crypto/sha1"
+	"crypto/sha256"
 	"crypto/tls"
+	"encoding/hex"
 	"fmt"
+	"io"
 	"io/ioutil"
 	"net"
 	"net/http"
@@ -26,27 +31,32 @@ import (
 
 // Deliver deliver hook task
 func Deliver(t *models.HookTask) error {
+	w, err := models.GetWebhookByID(t.HookID)
+	if err != nil {
+		return err
+	}
+
 	defer func() {
 		err := recover()
 		if err == nil {
 			return
 		}
 		// There was a panic whilst delivering a hook...
-		log.Error("PANIC whilst trying to deliver webhook[%d] for repo[%d] to %s Panic: %v\nStacktrace: %s", t.ID, t.RepoID, t.URL, err, log.Stack(2))
+		log.Error("PANIC whilst trying to deliver webhook[%d] for repo[%d] to %s Panic: %v\nStacktrace: %s", t.ID, t.RepoID, w.URL, err, log.Stack(2))
 	}()
+
 	t.IsDelivered = true
 
 	var req *http.Request
-	var err error
 
-	switch t.HTTPMethod {
+	switch w.HTTPMethod {
 	case "":
 		log.Info("HTTP Method for webhook %d empty, setting to POST as default", t.ID)
 		fallthrough
 	case http.MethodPost:
-		switch t.ContentType {
+		switch w.ContentType {
 		case models.ContentTypeJSON:
-			req, err = http.NewRequest("POST", t.URL, strings.NewReader(t.PayloadContent))
+			req, err = http.NewRequest("POST", w.URL, strings.NewReader(t.PayloadContent))
 			if err != nil {
 				return err
 			}
@@ -57,16 +67,15 @@ func Deliver(t *models.HookTask) error {
 				"payload": []string{t.PayloadContent},
 			}
 
-			req, err = http.NewRequest("POST", t.URL, strings.NewReader(forms.Encode()))
+			req, err = http.NewRequest("POST", w.URL, strings.NewReader(forms.Encode()))
 			if err != nil {
-
 				return err
 			}
 
 			req.Header.Set("Content-Type", "application/x-www-form-urlencoded")
 		}
 	case http.MethodGet:
-		u, err := url.Parse(t.URL)
+		u, err := url.Parse(w.URL)
 		if err != nil {
 			return err
 		}
@@ -78,31 +87,48 @@ func Deliver(t *models.HookTask) error {
 			return err
 		}
 	case http.MethodPut:
-		switch t.Typ {
+		switch w.Type {
 		case models.MATRIX:
-			req, err = getMatrixHookRequest(t)
+			req, err = getMatrixHookRequest(w, t)
 			if err != nil {
 				return err
 			}
 		default:
-			return fmt.Errorf("Invalid http method for webhook: [%d] %v", t.ID, t.HTTPMethod)
+			return fmt.Errorf("Invalid http method for webhook: [%d] %v", t.ID, w.HTTPMethod)
 		}
 	default:
-		return fmt.Errorf("Invalid http method for webhook: [%d] %v", t.ID, t.HTTPMethod)
+		return fmt.Errorf("Invalid http method for webhook: [%d] %v", t.ID, w.HTTPMethod)
+	}
+
+	var signatureSHA1 string
+	var signatureSHA256 string
+	if len(w.Secret) > 0 {
+		sig1 := hmac.New(sha1.New, []byte(w.Secret))
+		sig256 := hmac.New(sha256.New, []byte(w.Secret))
+		_, err = io.MultiWriter(sig1, sig256).Write([]byte(t.PayloadContent))
+		if err != nil {
+			log.Error("prepareWebhooks.sigWrite: %v", err)
+		}
+		signatureSHA1 = hex.EncodeToString(sig1.Sum(nil))
+		signatureSHA256 = hex.EncodeToString(sig256.Sum(nil))
 	}
 
 	req.Header.Add("X-Gitea-Delivery", t.UUID)
 	req.Header.Add("X-Gitea-Event", t.EventType.Event())
-	req.Header.Add("X-Gitea-Signature", t.Signature)
+	req.Header.Add("X-Gitea-Signature", signatureSHA256)
 	req.Header.Add("X-Gogs-Delivery", t.UUID)
 	req.Header.Add("X-Gogs-Event", t.EventType.Event())
-	req.Header.Add("X-Gogs-Signature", t.Signature)
+	req.Header.Add("X-Gogs-Signature", signatureSHA256)
+	req.Header.Add("X-Hub-Signature", "sha1="+signatureSHA1)
+	req.Header.Add("X-Hub-Signature-256", "sha256="+signatureSHA256)
 	req.Header["X-GitHub-Delivery"] = []string{t.UUID}
 	req.Header["X-GitHub-Event"] = []string{t.EventType.Event()}
 
 	// Record delivery information.
 	t.RequestInfo = &models.HookRequest{
-		Headers: map[string]string{},
+		URL:        req.URL.String(),
+		HTTPMethod: req.Method,
+		Headers:    map[string]string{},
 	}
 	for k, vals := range req.Header {
 		t.RequestInfo.Headers[k] = strings.Join(vals, ",")
@@ -125,11 +151,6 @@ func Deliver(t *models.HookTask) error {
 		}
 
 		// Update webhook last delivery status.
-		w, err := models.GetWebhookByID(t.HookID)
-		if err != nil {
-			log.Error("GetWebhookByID: %v", err)
-			return
-		}
 		if t.IsSucceed {
 			w.LastStatus = models.HookStatusSucceed
 		} else {

services/webhook/dingtalk.go

@@ -25,9 +25,6 @@ var (
 	_ PayloadConvertor = &DingtalkPayload{}
 )
 
-// SetSecret sets the dingtalk secret
-func (d *DingtalkPayload) SetSecret(_ string) {}
-
 // JSONPayload Marshals the DingtalkPayload to json
 func (d *DingtalkPayload) JSONPayload() ([]byte, error) {
 	json := jsoniter.ConfigCompatibleWithStandardLibrary

services/webhook/discord.go

@@ -97,9 +97,6 @@ var (
 	redColor         = color("ff3232")
 )
 
-// SetSecret sets the discord secret
-func (d *DiscordPayload) SetSecret(_ string) {}
-
 // JSONPayload Marshals the DiscordPayload to json
 func (d *DiscordPayload) JSONPayload() ([]byte, error) {
 	json := jsoniter.ConfigCompatibleWithStandardLibrary

services/webhook/feishu.go

@@ -35,9 +35,6 @@ func newFeishuTextPayload(text string) *FeishuPayload {
 	}
 }
 
-// SetSecret sets the Feishu secret
-func (f *FeishuPayload) SetSecret(_ string) {}
-
 // JSONPayload Marshals the FeishuPayload to json
 func (f *FeishuPayload) JSONPayload() ([]byte, error) {
 	json := jsoniter.ConfigCompatibleWithStandardLibrary

services/webhook/matrix.go

@@ -76,9 +76,6 @@ type MatrixPayloadSafe struct {
 	Commits       []*api.PayloadCommit `json:"io.gitea.commits,omitempty"`
 }
 
-// SetSecret sets the Matrix secret
-func (m *MatrixPayloadUnsafe) SetSecret(_ string) {}
-
 // JSONPayload Marshals the MatrixPayloadUnsafe to json
 func (m *MatrixPayloadUnsafe) JSONPayload() ([]byte, error) {
 	json := jsoniter.ConfigCompatibleWithStandardLibrary
@@ -263,7 +260,7 @@ func getMessageBody(htmlText string) string {
 
 // getMatrixHookRequest creates a new request which contains an Authorization header.
 // The access_token is removed from t.PayloadContent
-func getMatrixHookRequest(t *models.HookTask) (*http.Request, error) {
+func getMatrixHookRequest(w *models.Webhook, t *models.HookTask) (*http.Request, error) {
 	payloadunsafe := MatrixPayloadUnsafe{}
 	json := jsoniter.ConfigCompatibleWithStandardLibrary
 	if err := json.Unmarshal([]byte(t.PayloadContent), &payloadunsafe); err != nil {
@@ -288,9 +285,9 @@ func getMatrixHookRequest(t *models.HookTask) (*http.Request, error) {
 		return nil, fmt.Errorf("getMatrixHookRequest: unable to hash payload: %+v", err)
 	}
 
-	t.URL = fmt.Sprintf("%s/%s", t.URL, txnID)
+	url := fmt.Sprintf("%s/%s", w.URL, txnID)
 
-	req, err := http.NewRequest(t.HTTPMethod, t.URL, strings.NewReader(string(payload)))
+	req, err := http.NewRequest(w.HTTPMethod, url, strings.NewReader(string(payload)))
 	if err != nil {
 		return nil, err
 	}

services/webhook/matrix_test.go

@@ -184,6 +184,8 @@ func TestMatrixJSONPayload(t *testing.T) {
 }
 
 func TestMatrixHookRequest(t *testing.T) {
+	w := &models.Webhook{}
+
 	h := &models.HookTask{
 		PayloadContent: `{
   "body": "[[user1/test](http://localhost:3000/user1/test)] user1 pushed 1 commit to [master](http://localhost:3000/user1/test/src/branch/master):\n[5175ef2](http://localhost:3000/user1/test/commit/5175ef26201c58b035a3404b3fe02b4e8d436eee): Merge pull request 'Change readme.md' (#2) from add-matrix-webhook into master\n\nReviewed-on: http://localhost:3000/user1/test/pulls/2\n - user1",
@@ -245,7 +247,7 @@ func TestMatrixHookRequest(t *testing.T) {
   ]
 }`
 
-	req, err := getMatrixHookRequest(h)
+	req, err := getMatrixHookRequest(w, h)
 	require.NoError(t, err)
 	require.NotNil(t, req)
 

services/webhook/msteams.go

@@ -55,9 +55,6 @@ type (
 	}
 )
 
-// SetSecret sets the MSTeams secret
-func (m *MSTeamsPayload) SetSecret(_ string) {}
-
 // JSONPayload Marshals the MSTeamsPayload to json
 func (m *MSTeamsPayload) JSONPayload() ([]byte, error) {
 	json := jsoniter.ConfigCompatibleWithStandardLibrary

services/webhook/slack.go

@@ -56,9 +56,6 @@ type SlackAttachment struct {
 	Text      string `json:"text"`
 }
 
-// SetSecret sets the slack secret
-func (s *SlackPayload) SetSecret(_ string) {}
-
 // JSONPayload Marshals the SlackPayload to json
 func (s *SlackPayload) JSONPayload() ([]byte, error) {
 	json := jsoniter.ConfigCompatibleWithStandardLibrary

services/webhook/telegram.go

@@ -45,9 +45,6 @@ var (
 	_ PayloadConvertor = &TelegramPayload{}
 )
 
-// SetSecret sets the telegram secret
-func (t *TelegramPayload) SetSecret(_ string) {}
-
 // JSONPayload Marshals the TelegramPayload to json
 func (t *TelegramPayload) JSONPayload() ([]byte, error) {
 	t.ParseMode = "HTML"

services/webhook/webhook.go

@@ -5,9 +5,6 @@
 package webhook
 
 import (
-	"crypto/hmac"
-	"crypto/sha256"
-	"encoding/hex"
 	"fmt"
 	"strings"
 
@@ -21,12 +18,12 @@ import (
 )
 
 type webhook struct {
-	name           models.HookTaskType
+	name           models.HookType
 	payloadCreator func(p api.Payloader, event models.HookEventType, meta string) (api.Payloader, error)
 }
 
 var (
-	webhooks = map[models.HookTaskType]*webhook{
+	webhooks = map[models.HookType]*webhook{
 		models.SLACK: {
 			name:           models.SLACK,
 			payloadCreator: GetSlackPayload,
@@ -60,7 +57,7 @@ var (
 
 // RegisterWebhook registers a webhook
 func RegisterWebhook(name string, webhook *webhook) {
-	webhooks[models.HookTaskType(name)] = webhook
+	webhooks[models.HookType(name)] = webhook
 }
 
 // IsValidHookTaskType returns true if a webhook registered
@@ -68,7 +65,7 @@ func IsValidHookTaskType(name string) bool {
 	if name == models.GITEA || name == models.GOGS {
 		return true
 	}
-	_, ok := webhooks[models.HookTaskType(name)]
+	_, ok := webhooks[models.HookType(name)]
 	return ok
 }
 
@@ -161,35 +158,14 @@ func prepareWebhook(w *models.Webhook, repo *models.Repository, event models.Hoo
 			return fmt.Errorf("create payload for %s[%s]: %v", w.Type, event, err)
 		}
 	} else {
-		p.SetSecret(w.Secret)
 		payloader = p
 	}
 
-	var signature string
-	if len(w.Secret) > 0 {
-		data, err := payloader.JSONPayload()
-		if err != nil {
-			log.Error("prepareWebhooks.JSONPayload: %v", err)
-		}
-		sig := hmac.New(sha256.New, []byte(w.Secret))
-		_, err = sig.Write(data)
-		if err != nil {
-			log.Error("prepareWebhooks.sigWrite: %v", err)
-		}
-		signature = hex.EncodeToString(sig.Sum(nil))
-	}
-
 	if err = models.CreateHookTask(&models.HookTask{
-		RepoID:      repo.ID,
+		RepoID:    repo.ID,
-		HookID:      w.ID,
+		HookID:    w.ID,
-		Typ:         w.Type,
+		Payloader: payloader,
-		URL:         w.URL,
+		EventType: event,
-		Signature:   signature,
-		Payloader:   payloader,
-		HTTPMethod:  w.HTTPMethod,
-		ContentType: w.ContentType,
-		EventType:   event,
-		IsSSL:       w.IsSSL,
 	}); err != nil {
 		return fmt.Errorf("CreateHookTask: %v", err)
 	}

templates/repo/settings/webhook/history.tmpl

@@ -44,8 +44,8 @@
 						<div class="ui bottom attached tab segment active" data-tab="request-{{.ID}}">
 							{{if .RequestInfo}}
 								<h5>{{$.i18n.Tr "repo.settings.webhook.headers"}}</h5>
-								<pre class="webhook-info"><strong>Request URL:</strong> {{.URL}}
+								<pre class="webhook-info"><strong>Request URL:</strong> {{.RequestInfo.URL}}
-<strong>Request method:</strong> {{if .HTTPMethod}}{{.HTTPMethod}}{{else}}POST{{end}}
+<strong>Request method:</strong> {{if .RequestInfo.HTTPMethod}}{{.RequestInfo.HTTPMethod}}{{else}}POST{{end}}
 {{ range $key, $val := .RequestInfo.Headers }}<strong>{{$key}}:</strong> {{$val}}
 {{end}}</pre>
 								<h5>{{$.i18n.Tr "repo.settings.webhook.payload"}}</h5>