From a46501f59815a10b94326b973ca4709b96bc3cf0 Mon Sep 17 00:00:00 2001 From: Till Faelligen Date: Tue, 28 Jul 2020 13:24:41 +0200 Subject: [PATCH 1/6] Don't use legacy send for messages --- modules/webhook/deliver.go | 17 ++++++++++------- modules/webhook/matrix.go | 21 ++++++++++++++++++++- modules/webhook/matrix_test.go | 29 +++++++++++++++++++++++++++++ 3 files changed, 59 insertions(+), 8 deletions(-) diff --git a/modules/webhook/deliver.go b/modules/webhook/deliver.go index 7b0c651733628..c29fcb6fa9e47 100644 --- a/modules/webhook/deliver.go +++ b/modules/webhook/deliver.go @@ -77,17 +77,20 @@ func Deliver(t *models.HookTask) error { if err != nil { return err } + case http.MethodPut: + switch t.Type { + case models.MATRIX: + req, err = getMatrixHookRequest(t) + if err != nil { + return err + } + default: + return fmt.Errorf("Invalid http method for webhook: [%d] %v", t.ID, t.HTTPMethod) + } default: return fmt.Errorf("Invalid http method for webhook: [%d] %v", t.ID, t.HTTPMethod) } - if t.Type == models.MATRIX { - req, err = getMatrixHookRequest(t) - if err != nil { - return err - } - } - 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) diff --git a/modules/webhook/matrix.go b/modules/webhook/matrix.go index 68c65623f727a..335ec5af0cc46 100644 --- a/modules/webhook/matrix.go +++ b/modules/webhook/matrix.go @@ -5,6 +5,7 @@ package webhook import ( + "crypto/sha1" "encoding/json" "errors" "fmt" @@ -291,7 +292,14 @@ func getMatrixHookRequest(t *models.HookTask) (*http.Request, error) { } t.PayloadContent = string(payload) - req, err := http.NewRequest("POST", t.URL, strings.NewReader(string(payload))) + txnID, err := getTxnID(payload) + if err != nil { + return nil, fmt.Errorf("getMatrixHookRequest: unable to hash payload: %+v", err) + } + + t.URL = fmt.Sprintf("%s/%s", t.URL, txnID) + + req, err := http.NewRequest(t.HTTPMethod, t.URL, strings.NewReader(string(payload))) if err != nil { return nil, err } @@ -301,3 +309,14 @@ func getMatrixHookRequest(t *models.HookTask) (*http.Request, error) { return req, nil } + +// getTxnID creates a txnID based on the payload to ensure idempotency +func getTxnID(payload []byte) (string, error) { + h := sha1.New() + _, err := h.Write(payload) + if err != nil { + return "", err + } + + return fmt.Sprintf("%x", h.Sum(nil)), nil +} diff --git a/modules/webhook/matrix_test.go b/modules/webhook/matrix_test.go index cfe234f850c43..4f6976d61b09e 100644 --- a/modules/webhook/matrix_test.go +++ b/modules/webhook/matrix_test.go @@ -154,3 +154,32 @@ func TestMatrixHookRequest(t *testing.T) { assert.Equal(t, "Bearer dummy_access_token", req.Header.Get("Authorization")) assert.Equal(t, wantPayloadContent, h.PayloadContent) } + +func Test_getTxnID(t *testing.T) { + type args struct { + payload []byte + } + tests := []struct { + name string + args args + want string + wantErr bool + }{ + { + name: "dummy payload", + args: args{payload: []byte("Hello World")}, + want: "0a4d55a8d778e5022fab701977c5d840bbc486d0", + wantErr: false, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := getTxnID(tt.args.payload) + if (err != nil) != tt.wantErr { + t.Errorf("getTxnID() error = %v, wantErr %v", err, tt.wantErr) + return + } + assert.Equal(t, tt.want, got) + }) + } +} From 7522aab300a005e77f413a78d806cc3693d6616c Mon Sep 17 00:00:00 2001 From: Till Faelligen Date: Tue, 28 Jul 2020 13:25:17 +0200 Subject: [PATCH 2/6] Add migrations to ensure Matrix webhooks use PUT --- models/migrations/migrations.go | 2 ++ models/migrations/v144.go | 27 +++++++++++++++++++++++++++ 2 files changed, 29 insertions(+) create mode 100644 models/migrations/v144.go diff --git a/models/migrations/migrations.go b/models/migrations/migrations.go index d205794e1f346..7e1cf2f50a55f 100644 --- a/models/migrations/migrations.go +++ b/models/migrations/migrations.go @@ -220,6 +220,8 @@ var migrations = []Migration{ NewMigration("Ensure Repository.IsArchived is not null", setIsArchivedToFalse), // v143 -> v144 NewMigration("recalculate Stars number for all user", recalculateStars), + // v144 -> v145 + NewMigration("update Matrix Webhook http method to 'PUT'", updateMatrixWebhookHTTPMethod), } // GetCurrentDBVersion returns the current db version diff --git a/models/migrations/v144.go b/models/migrations/v144.go new file mode 100644 index 0000000000000..6c1383b8d769e --- /dev/null +++ b/models/migrations/v144.go @@ -0,0 +1,27 @@ +// Copyright 2020 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 ( + "code.gitea.io/gitea/models" + "code.gitea.io/gitea/modules/log" + "xorm.io/builder" + "xorm.io/xorm" +) + +func updateMatrixWebhookHTTPMethod(x *xorm.Engine) error { + type Webhook struct { + HTTPMethod string + } + count, err := x.Where( + builder.Neq{ + "http_method": "PUT", + "hook_task_type": models.MATRIX, + }).Cols("http_method").Update(&Webhook{HTTPMethod: "PUT"}) + if err == nil { + log.Debug("Updated %d Matrix webhooks with http_method 'PUT'", count) + } + return err +} From 19ada70190b8c0231eff790c00256d2bf33134b5 Mon Sep 17 00:00:00 2001 From: Till Faelligen Date: Tue, 28 Jul 2020 13:25:44 +0200 Subject: [PATCH 3/6] Set HTTP method to PUT as default --- routers/repo/webhook.go | 1 + 1 file changed, 1 insertion(+) diff --git a/routers/repo/webhook.go b/routers/repo/webhook.go index 7ac403b46259c..bec401021cc5a 100644 --- a/routers/repo/webhook.go +++ b/routers/repo/webhook.go @@ -454,6 +454,7 @@ func MatrixHooksNewPost(ctx *context.Context, form auth.NewMatrixHookForm) { RepoID: orCtx.RepoID, URL: fmt.Sprintf("%s/_matrix/client/r0/rooms/%s/send/m.room.message", form.HomeserverURL, form.RoomID), ContentType: models.ContentTypeJSON, + HTTPMethod: "PUT", HookEvent: ParseHookEvent(form.WebhookForm), IsActive: form.Active, HookTaskType: models.MATRIX, From 630ec9e413f0edfb7cfd5ea91d5d3cfed3cb5a54 Mon Sep 17 00:00:00 2001 From: Till Faelligen Date: Tue, 28 Jul 2020 14:18:33 +0200 Subject: [PATCH 4/6] Fix sql condition.. Signed-off-by: Till Faelligen --- models/migrations/v144.go | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/models/migrations/v144.go b/models/migrations/v144.go index 6c1383b8d769e..aeffb72662431 100644 --- a/models/migrations/v144.go +++ b/models/migrations/v144.go @@ -15,11 +15,9 @@ func updateMatrixWebhookHTTPMethod(x *xorm.Engine) error { type Webhook struct { HTTPMethod string } - count, err := x.Where( - builder.Neq{ - "http_method": "PUT", - "hook_task_type": models.MATRIX, - }).Cols("http_method").Update(&Webhook{HTTPMethod: "PUT"}) + + cond := builder.Eq{"hook_task_type": models.MATRIX}.And(builder.Neq{"http_method": "PUT"}) + count, err := x.Where(cond).Cols("http_method").Update(&Webhook{HTTPMethod: "PUT"}) if err == nil { log.Debug("Updated %d Matrix webhooks with http_method 'PUT'", count) } From 580beddf0154165b9a61530e336f624ca954d8c6 Mon Sep 17 00:00:00 2001 From: Till Faelligen Date: Thu, 30 Jul 2020 07:20:49 +0200 Subject: [PATCH 5/6] Rename getTxnID -> getMatrixTxnID --- modules/webhook/matrix.go | 6 +++--- modules/webhook/matrix_test.go | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/modules/webhook/matrix.go b/modules/webhook/matrix.go index 335ec5af0cc46..d6309000a863b 100644 --- a/modules/webhook/matrix.go +++ b/modules/webhook/matrix.go @@ -292,7 +292,7 @@ func getMatrixHookRequest(t *models.HookTask) (*http.Request, error) { } t.PayloadContent = string(payload) - txnID, err := getTxnID(payload) + txnID, err := getMatrixTxnID(payload) if err != nil { return nil, fmt.Errorf("getMatrixHookRequest: unable to hash payload: %+v", err) } @@ -310,8 +310,8 @@ func getMatrixHookRequest(t *models.HookTask) (*http.Request, error) { return req, nil } -// getTxnID creates a txnID based on the payload to ensure idempotency -func getTxnID(payload []byte) (string, error) { +// getMatrixTxnID creates a txnID based on the payload to ensure idempotency +func getMatrixTxnID(payload []byte) (string, error) { h := sha1.New() _, err := h.Write(payload) if err != nil { diff --git a/modules/webhook/matrix_test.go b/modules/webhook/matrix_test.go index 2b779f1d52c1c..3d1c660126c39 100644 --- a/modules/webhook/matrix_test.go +++ b/modules/webhook/matrix_test.go @@ -174,9 +174,9 @@ func Test_getTxnID(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - got, err := getTxnID(tt.args.payload) + got, err := getMatrixTxnID(tt.args.payload) if (err != nil) != tt.wantErr { - t.Errorf("getTxnID() error = %v, wantErr %v", err, tt.wantErr) + t.Errorf("getMatrixTxnID() error = %v, wantErr %v", err, tt.wantErr) return } assert.Equal(t, tt.want, got) From 206f4ac38da07c61c882e58db1de256980234f95 Mon Sep 17 00:00:00 2001 From: Till Faelligen Date: Thu, 30 Jul 2020 07:32:01 +0200 Subject: [PATCH 6/6] Use local variable instead of constant value --- models/migrations/v144.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/models/migrations/v144.go b/models/migrations/v144.go index aeffb72662431..beb089dde680d 100644 --- a/models/migrations/v144.go +++ b/models/migrations/v144.go @@ -5,18 +5,18 @@ package migrations import ( - "code.gitea.io/gitea/models" "code.gitea.io/gitea/modules/log" "xorm.io/builder" "xorm.io/xorm" ) func updateMatrixWebhookHTTPMethod(x *xorm.Engine) error { + var matrixHookTaskType = 9 // value comes from the models package type Webhook struct { HTTPMethod string } - cond := builder.Eq{"hook_task_type": models.MATRIX}.And(builder.Neq{"http_method": "PUT"}) + cond := builder.Eq{"hook_task_type": matrixHookTaskType}.And(builder.Neq{"http_method": "PUT"}) count, err := x.Where(cond).Cols("http_method").Update(&Webhook{HTTPMethod: "PUT"}) if err == nil { log.Debug("Updated %d Matrix webhooks with http_method 'PUT'", count)