From 9b1b4b543358c212a3da2b480d361d0c1375b279 Mon Sep 17 00:00:00 2001 From: KN4CK3R Date: Sun, 27 Jun 2021 21:21:09 +0200 Subject: [PATCH] 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`. ## :warning: BREAKING :warning: * 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 --- models/migrations/migrations.go | 2 + models/migrations/v187.go | 46 +++++++++++++++ models/webhook.go | 52 ++++++++--------- models/webhook_test.go | 14 ----- modules/structs/hook.go | 55 ------------------ routers/api/v1/utils/hook.go | 2 +- routers/web/repo/webhook.go | 2 +- services/webhook/deliver.go | 61 +++++++++++++------- services/webhook/dingtalk.go | 3 - services/webhook/discord.go | 3 - services/webhook/feishu.go | 3 - services/webhook/matrix.go | 9 +-- services/webhook/matrix_test.go | 4 +- services/webhook/msteams.go | 3 - services/webhook/slack.go | 3 - services/webhook/telegram.go | 3 - services/webhook/webhook.go | 40 +++---------- templates/repo/settings/webhook/history.tmpl | 4 +- 18 files changed, 130 insertions(+), 179 deletions(-) create mode 100644 models/migrations/v187.go diff --git a/models/migrations/migrations.go b/models/migrations/migrations.go index 978ba6368..0e8b0d0e3 100644 --- a/models/migrations/migrations.go +++ b/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 diff --git a/models/migrations/v187.go b/models/migrations/v187.go new file mode 100644 index 000000000..627423717 --- /dev/null +++ b/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() +} diff --git a/models/webhook.go b/models/webhook.go index 24510cc6f..29cfcf6ed 100644 --- a/models/webhook.go +++ b/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"` - Type HookTaskType `xorm:"VARCHAR(16) 'type'"` - Meta string `xorm:"TEXT"` // store hook-specific attributes - LastStatus HookStatus // Last delivery status + IsActive bool `xorm:"INDEX"` + Type HookType `xorm:"VARCHAR(16) 'type'"` + Meta string `xorm:"TEXT"` // store hook-specific attributes + 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:"-"` diff --git a/models/webhook_test.go b/models/webhook_test.go index 88b2d40a3..cab44a120 100644 --- a/models/webhook_test.go +++ b/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(), diff --git a/modules/structs/hook.go b/modules/structs/hook.go index 693820b57..e4ec99df4 100644 --- a/modules/structs/hook.go +++ b/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 diff --git a/routers/api/v1/utils/hook.go b/routers/api/v1/utils/hook.go index c7af40dcf..5f2be65a2 100644 --- a/routers/api/v1/utils/hook.go +++ b/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"] diff --git a/routers/web/repo/webhook.go b/routers/web/repo/webhook.go index fe16d249e..ab254dbe4 100644 --- a/routers/web/repo/webhook.go +++ b/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 diff --git a/services/webhook/deliver.go b/services/webhook/deliver.go index a417a9e84..8243fde1b 100644 --- a/services/webhook/deliver.go +++ b/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 { diff --git a/services/webhook/dingtalk.go b/services/webhook/dingtalk.go index d781b8c87..49e161ea5 100644 --- a/services/webhook/dingtalk.go +++ b/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 diff --git a/services/webhook/discord.go b/services/webhook/discord.go index 378d9ff72..ea3879f19 100644 --- a/services/webhook/discord.go +++ b/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 diff --git a/services/webhook/feishu.go b/services/webhook/feishu.go index 5c80efb82..b280e6775 100644 --- a/services/webhook/feishu.go +++ b/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 diff --git a/services/webhook/matrix.go b/services/webhook/matrix.go index 1658dd4b4..6fca67ca8 100644 --- a/services/webhook/matrix.go +++ b/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 } diff --git a/services/webhook/matrix_test.go b/services/webhook/matrix_test.go index 7b10e21cf..451dff694 100644 --- a/services/webhook/matrix_test.go +++ b/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) diff --git a/services/webhook/msteams.go b/services/webhook/msteams.go index 51bb28a36..035dbc1c4 100644 --- a/services/webhook/msteams.go +++ b/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 diff --git a/services/webhook/slack.go b/services/webhook/slack.go index 1de9c9c37..f522ca35f 100644 --- a/services/webhook/slack.go +++ b/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 diff --git a/services/webhook/telegram.go b/services/webhook/telegram.go index f71352141..4c4230759 100644 --- a/services/webhook/telegram.go +++ b/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" diff --git a/services/webhook/webhook.go b/services/webhook/webhook.go index cc79ec15d..d094a7754 100644 --- a/services/webhook/webhook.go +++ b/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, - HookID: w.ID, - Typ: w.Type, - URL: w.URL, - Signature: signature, - Payloader: payloader, - HTTPMethod: w.HTTPMethod, - ContentType: w.ContentType, - EventType: event, - IsSSL: w.IsSSL, + RepoID: repo.ID, + HookID: w.ID, + Payloader: payloader, + EventType: event, }); err != nil { return fmt.Errorf("CreateHookTask: %v", err) } diff --git a/templates/repo/settings/webhook/history.tmpl b/templates/repo/settings/webhook/history.tmpl index cf4884531..d2fe68738 100644 --- a/templates/repo/settings/webhook/history.tmpl +++ b/templates/repo/settings/webhook/history.tmpl @@ -44,8 +44,8 @@
{{if .RequestInfo}}
{{$.i18n.Tr "repo.settings.webhook.headers"}}
- 
Request URL: {{.URL}}
-Request method: {{if .HTTPMethod}}{{.HTTPMethod}}{{else}}POST{{end}}
+								
Request URL: {{.RequestInfo.URL}}
+Request method: {{if .RequestInfo.HTTPMethod}}{{.RequestInfo.HTTPMethod}}{{else}}POST{{end}}
 {{ range $key, $val := .RequestInfo.Headers }}{{$key}}: {{$val}}
 {{end}}

 								
{{$.i18n.Tr "repo.settings.webhook.payload"}}