Skip to content

Commit bf60146

Browse files
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 <[email protected]> * Rename getTxnID -> getMatrixTxnID * Use local variable instead of constant value Co-authored-by: techknowlogick <[email protected]>
1 parent f6d5303 commit bf60146

File tree

6 files changed

+87
-8
lines changed

6 files changed

+87
-8
lines changed

models/migrations/migrations.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -220,6 +220,8 @@ var migrations = []Migration{
220220
NewMigration("Ensure Repository.IsArchived is not null", setIsArchivedToFalse),
221221
// v143 -> v144
222222
NewMigration("recalculate Stars number for all user", recalculateStars),
223+
// v144 -> v145
224+
NewMigration("update Matrix Webhook http method to 'PUT'", updateMatrixWebhookHTTPMethod),
223225
}
224226

225227
// GetCurrentDBVersion returns the current db version

models/migrations/v144.go

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
// Copyright 2020 The Gitea Authors. All rights reserved.
2+
// Use of this source code is governed by a MIT-style
3+
// license that can be found in the LICENSE file.
4+
5+
package migrations
6+
7+
import (
8+
"code.gitea.io/gitea/modules/log"
9+
"xorm.io/builder"
10+
"xorm.io/xorm"
11+
)
12+
13+
func updateMatrixWebhookHTTPMethod(x *xorm.Engine) error {
14+
var matrixHookTaskType = 9 // value comes from the models package
15+
type Webhook struct {
16+
HTTPMethod string
17+
}
18+
19+
cond := builder.Eq{"hook_task_type": matrixHookTaskType}.And(builder.Neq{"http_method": "PUT"})
20+
count, err := x.Where(cond).Cols("http_method").Update(&Webhook{HTTPMethod: "PUT"})
21+
if err == nil {
22+
log.Debug("Updated %d Matrix webhooks with http_method 'PUT'", count)
23+
}
24+
return err
25+
}

modules/webhook/deliver.go

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -77,17 +77,20 @@ func Deliver(t *models.HookTask) error {
7777
if err != nil {
7878
return err
7979
}
80+
case http.MethodPut:
81+
switch t.Type {
82+
case models.MATRIX:
83+
req, err = getMatrixHookRequest(t)
84+
if err != nil {
85+
return err
86+
}
87+
default:
88+
return fmt.Errorf("Invalid http method for webhook: [%d] %v", t.ID, t.HTTPMethod)
89+
}
8090
default:
8191
return fmt.Errorf("Invalid http method for webhook: [%d] %v", t.ID, t.HTTPMethod)
8292
}
8393

84-
if t.Type == models.MATRIX {
85-
req, err = getMatrixHookRequest(t)
86-
if err != nil {
87-
return err
88-
}
89-
}
90-
9194
req.Header.Add("X-Gitea-Delivery", t.UUID)
9295
req.Header.Add("X-Gitea-Event", t.EventType.Event())
9396
req.Header.Add("X-Gitea-Signature", t.Signature)

modules/webhook/matrix.go

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
package webhook
66

77
import (
8+
"crypto/sha1"
89
"encoding/json"
910
"errors"
1011
"fmt"
@@ -291,7 +292,14 @@ func getMatrixHookRequest(t *models.HookTask) (*http.Request, error) {
291292
}
292293
t.PayloadContent = string(payload)
293294

294-
req, err := http.NewRequest("POST", t.URL, strings.NewReader(string(payload)))
295+
txnID, err := getMatrixTxnID(payload)
296+
if err != nil {
297+
return nil, fmt.Errorf("getMatrixHookRequest: unable to hash payload: %+v", err)
298+
}
299+
300+
t.URL = fmt.Sprintf("%s/%s", t.URL, txnID)
301+
302+
req, err := http.NewRequest(t.HTTPMethod, t.URL, strings.NewReader(string(payload)))
295303
if err != nil {
296304
return nil, err
297305
}
@@ -301,3 +309,14 @@ func getMatrixHookRequest(t *models.HookTask) (*http.Request, error) {
301309

302310
return req, nil
303311
}
312+
313+
// getMatrixTxnID creates a txnID based on the payload to ensure idempotency
314+
func getMatrixTxnID(payload []byte) (string, error) {
315+
h := sha1.New()
316+
_, err := h.Write(payload)
317+
if err != nil {
318+
return "", err
319+
}
320+
321+
return fmt.Sprintf("%x", h.Sum(nil)), nil
322+
}

modules/webhook/matrix_test.go

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -154,3 +154,32 @@ func TestMatrixHookRequest(t *testing.T) {
154154
assert.Equal(t, "Bearer dummy_access_token", req.Header.Get("Authorization"))
155155
assert.Equal(t, wantPayloadContent, h.PayloadContent)
156156
}
157+
158+
func Test_getTxnID(t *testing.T) {
159+
type args struct {
160+
payload []byte
161+
}
162+
tests := []struct {
163+
name string
164+
args args
165+
want string
166+
wantErr bool
167+
}{
168+
{
169+
name: "dummy payload",
170+
args: args{payload: []byte("Hello World")},
171+
want: "0a4d55a8d778e5022fab701977c5d840bbc486d0",
172+
wantErr: false,
173+
},
174+
}
175+
for _, tt := range tests {
176+
t.Run(tt.name, func(t *testing.T) {
177+
got, err := getMatrixTxnID(tt.args.payload)
178+
if (err != nil) != tt.wantErr {
179+
t.Errorf("getMatrixTxnID() error = %v, wantErr %v", err, tt.wantErr)
180+
return
181+
}
182+
assert.Equal(t, tt.want, got)
183+
})
184+
}
185+
}

routers/repo/webhook.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -454,6 +454,7 @@ func MatrixHooksNewPost(ctx *context.Context, form auth.NewMatrixHookForm) {
454454
RepoID: orCtx.RepoID,
455455
URL: fmt.Sprintf("%s/_matrix/client/r0/rooms/%s/send/m.room.message", form.HomeserverURL, form.RoomID),
456456
ContentType: models.ContentTypeJSON,
457+
HTTPMethod: "PUT",
457458
HookEvent: ParseHookEvent(form.WebhookForm),
458459
IsActive: form.Active,
459460
HookTaskType: models.MATRIX,

0 commit comments

Comments
 (0)