Refactor Webhook + Add X-Hub-Signature

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

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()
+}

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:"-"`

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(),

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,19 +123,13 @@ var (
 
 // CreatePayload FIXME
 type CreatePayload struct {
-	Secret  string      `json:"secret"`
 	Sha     string      `json:"sha"`
 	Ref     string      `json:"ref"`
 	RefType string      `json:"ref_type"`
 	Repo    *Repository `json:"repository"`
 	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,19 +174,13 @@ 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"`
 	Repo       *Repository `json:"repository"`
 	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