Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor Webhook + Add X-Hub-Signature #16176

Merged
merged 10 commits into from
Jun 27, 2021
2 changes: 2 additions & 0 deletions models/migrations/migrations.go
Original file line number Diff line number Diff line change
Expand Up @@ -317,6 +317,8 @@ var migrations = []Migration{
NewMigration("Add issue resource index table", addIssueResourceIndexTable),
// v183 -> v184
NewMigration("Create PushMirror table", createPushMirrorTable),
// v184 -> v185
NewMigration("Drop unneeded webhook related columns", dropWebhookColumns),
}

// GetCurrentDBVersion returns the current db version
Expand Down
46 changes: 46 additions & 0 deletions models/migrations/v184.go
Original file line number Diff line number Diff line change
@@ -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 models.HookType `xorm:"VARCHAR(16) index"`
URL string `xorm:"TEXT"`
Signature string `xorm:"TEXT"`
HTTPMethod string `xorm:"http_method"`
ContentType models.HookContentType
KN4CK3R marked this conversation as resolved.
Show resolved Hide resolved
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 {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

don't know why this columns should be dropped? webhook maybe changed after hook_task finished. So these columns will keep the original information when requesting.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Webhook:
Signature and IsSSL are never used

HookTask:
Typ see #16176 (comment)
URL moved to HookRequest
Signature calculated on delivery. Is saved indirect in the Header map in HookRequest
HTTPMethod moved to HookRequest
ContentType of the Webhook is used on delivery. If the Webhook gets changed before sending the HookTask will use the current/correct settings. The original value is not available after a change but it's not visible on the UI anyway.
IsSSL is never used

return err
}

return sess.Commit()
}
52 changes: 23 additions & 29 deletions models/webhook.go
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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"`
Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -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.
Expand All @@ -651,15 +651,9 @@ type HookTask struct {
RepoID int64 `xorm:"INDEX"`
HookID int64
UUID string
Typ HookTaskType `xorm:"VARCHAR(16) index"`
lunny marked this conversation as resolved.
Show resolved Hide resolved
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:"-"`
Expand Down
14 changes: 0 additions & 14 deletions models/webhook_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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(),
Expand All @@ -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,
}
Expand All @@ -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(),
Expand All @@ -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(),
Expand All @@ -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,
}
Expand All @@ -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(),
Expand Down
2 changes: 1 addition & 1 deletion routers/api/v1/utils/hook.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"]
Expand Down
2 changes: 1 addition & 1 deletion routers/web/repo/webhook.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
61 changes: 41 additions & 20 deletions services/webhook/deliver.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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
}
Expand All @@ -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
}
Expand All @@ -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, ",")
Expand All @@ -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 {
Expand Down
6 changes: 3 additions & 3 deletions services/webhook/matrix.go
Original file line number Diff line number Diff line change
Expand Up @@ -263,7 +263,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 {
Expand All @@ -288,9 +288,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
}
Expand Down
Loading