From bf6014644401dd3fdf9031670b3a00ccd866f49f Mon Sep 17 00:00:00 2001 From: S7evinK Date: Fri, 31 Jul 2020 00:04:19 +0200 Subject: [PATCH] Don't use legacy method to send Matrix Webhook (#12348) * Don't use legacy send for messages * Add migrations to ensure Matrix webhooks use PUT * Set HTTP method to PUT as default * Fix sql condition.. Signed-off-by: Till Faelligen * Rename getTxnID -> getMatrixTxnID * Use local variable instead of constant value Co-authored-by: techknowlogick --- models/migrations/migrations.go | 2 ++ models/migrations/v144.go | 25 +++++++++++++++++++++++++ modules/webhook/deliver.go | 17 ++++++++++------- modules/webhook/matrix.go | 21 ++++++++++++++++++++- modules/webhook/matrix_test.go | 29 +++++++++++++++++++++++++++++ routers/repo/webhook.go | 1 + 6 files changed, 87 insertions(+), 8 deletions(-) create mode 100644 models/migrations/v144.go diff --git a/models/migrations/migrations.go b/models/migrations/migrations.go index d205794e1f3..7e1cf2f50a5 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 00000000000..beb089dde68 --- /dev/null +++ b/models/migrations/v144.go @@ -0,0 +1,25 @@ +// 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/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": 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) + } + return err +} diff --git a/modules/webhook/deliver.go b/modules/webhook/deliver.go index 7b0c6517336..c29fcb6fa9e 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 68c65623f72..d6309000a86 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 := getMatrixTxnID(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 } + +// 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 { + 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 4e8b878ad46..3d1c660126c 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 := getMatrixTxnID(tt.args.payload) + if (err != nil) != tt.wantErr { + t.Errorf("getMatrixTxnID() error = %v, wantErr %v", err, tt.wantErr) + return + } + assert.Equal(t, tt.want, got) + }) + } +} diff --git a/routers/repo/webhook.go b/routers/repo/webhook.go index 7ac403b4625..bec401021cc 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,