From b6e81357bd6fb80f8ba94c513f89a210beb05313 Mon Sep 17 00:00:00 2001 From: oliverpool <3864879+oliverpool@users.noreply.github.com> Date: Thu, 3 Nov 2022 19:23:20 +0100 Subject: [PATCH] Add Webhook authorization header (#20926) _This is a different approach to #20267, I took the liberty of adapting some parts, see below_ ## Context In some cases, a weebhook endpoint requires some kind of authentication. The usual way is by sending a static `Authorization` header, with a given token. For instance: - Matrix expects a `Bearer ` (already implemented, by storing the header cleartext in the metadata - which is buggy on retry #19872) - TeamCity #18667 - Gitea instances #20267 - SourceHut https://man.sr.ht/graphql.md#authentication-strategies (this is my actual personal need :) ## Proposed solution Add a dedicated encrypt column to the webhook table (instead of storing it as meta as proposed in #20267), so that it gets available for all present and future hook types (especially the custom ones #19307). This would also solve the buggy matrix retry #19872. As a first step, I would recommend focusing on the backend logic and improve the frontend at a later stage. For now the UI is a simple `Authorization` field (which could be later customized with `Bearer` and `Basic` switches): ![2022-08-23-142911](https://user-images.githubusercontent.com/3864879/186162483-5b721504-eef5-4932-812e-eb96a68494cc.png) The header name is hard-coded, since I couldn't fine any usecase justifying otherwise. ## Questions - What do you think of this approach? @justusbunsi @Gusted @silverwind - ~~How are the migrations generated? Do I have to manually create a new file, or is there a command for that?~~ - ~~I started adding it to the API: should I complete it or should I drop it? (I don't know how much the API is actually used)~~ ## Done as well: - add a migration for the existing matrix webhooks and remove the `Authorization` logic there _Closes #19872_ Co-authored-by: Lunny Xiao Co-authored-by: Gusted Co-authored-by: delvh --- docs/content/doc/features/webhooks.en-us.md | 4 + .../expected_webhook.yml | 9 + .../hook_task.yml | 8 + .../webhook.yml | 10 + models/migrations/migrations.go | 2 + models/migrations/v1_19/v233.go | 183 +++++++++++++++++ models/migrations/v1_19/v233_test.go | 88 ++++++++ models/webhook/webhook.go | 28 +++ modules/convert/convert.go | 26 ++- modules/structs/hook.go | 29 +-- options/locale/locale_en-US.ini | 3 +- routers/api/v1/org/hook.go | 14 +- routers/api/v1/repo/hook.go | 13 +- routers/api/v1/utils/hook.go | 52 ++++- routers/web/repo/webhook.go | 12 +- services/forms/repo_form.go | 2 +- services/webhook/deliver.go | 17 +- services/webhook/deliver_test.go | 41 ++++ services/webhook/main_test.go | 4 + services/webhook/matrix.go | 131 ++++-------- services/webhook/matrix_test.go | 190 ++++++------------ templates/repo/settings/webhook/matrix.tmpl | 4 - templates/repo/settings/webhook/settings.tmpl | 9 + templates/swagger/v1_json.tmpl | 12 ++ tests/integration/api_repo_hook_test.go | 47 +++++ 25 files changed, 673 insertions(+), 265 deletions(-) create mode 100644 models/migrations/fixtures/Test_addHeaderAuthorizationEncryptedColWebhook/expected_webhook.yml create mode 100644 models/migrations/fixtures/Test_addHeaderAuthorizationEncryptedColWebhook/hook_task.yml create mode 100644 models/migrations/fixtures/Test_addHeaderAuthorizationEncryptedColWebhook/webhook.yml create mode 100644 models/migrations/v1_19/v233.go create mode 100644 models/migrations/v1_19/v233_test.go create mode 100644 tests/integration/api_repo_hook_test.go diff --git a/docs/content/doc/features/webhooks.en-us.md b/docs/content/doc/features/webhooks.en-us.md index 2dba7b7f8..ecbe13c3a 100644 --- a/docs/content/doc/features/webhooks.en-us.md +++ b/docs/content/doc/features/webhooks.en-us.md @@ -188,3 +188,7 @@ if (json_last_error() !== JSON_ERROR_NONE) { ``` There is a Test Delivery button in the webhook settings that allows to test the configuration as well as a list of the most Recent Deliveries. + +### Authorization header + +**With 1.19**, Gitea hooks can be configured to send an [authorization header](https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Authorization) to the webhook target. diff --git a/models/migrations/fixtures/Test_addHeaderAuthorizationEncryptedColWebhook/expected_webhook.yml b/models/migrations/fixtures/Test_addHeaderAuthorizationEncryptedColWebhook/expected_webhook.yml new file mode 100644 index 000000000..f6239998c --- /dev/null +++ b/models/migrations/fixtures/Test_addHeaderAuthorizationEncryptedColWebhook/expected_webhook.yml @@ -0,0 +1,9 @@ +# for matrix, the access_token has been moved to "header_authorization" +- + id: 1 + meta: '{"homeserver_url":"https://matrix.example.com","room_id":"roomID","message_type":1}' + header_authorization: "Bearer s3cr3t" +- + id: 2 + meta: '' + header_authorization: "" diff --git a/models/migrations/fixtures/Test_addHeaderAuthorizationEncryptedColWebhook/hook_task.yml b/models/migrations/fixtures/Test_addHeaderAuthorizationEncryptedColWebhook/hook_task.yml new file mode 100644 index 000000000..8f61d6e70 --- /dev/null +++ b/models/migrations/fixtures/Test_addHeaderAuthorizationEncryptedColWebhook/hook_task.yml @@ -0,0 +1,8 @@ +# unsafe payload +- id: 1 + hook_id: 1 + payload_content: '{"homeserver_url":"https://matrix.example.com","room_id":"roomID","access_token":"s3cr3t","message_type":1}' +# safe payload +- id: 2 + hook_id: 2 + payload_content: '{"homeserver_url":"https://matrix.example.com","room_id":"roomID","message_type":1}' diff --git a/models/migrations/fixtures/Test_addHeaderAuthorizationEncryptedColWebhook/webhook.yml b/models/migrations/fixtures/Test_addHeaderAuthorizationEncryptedColWebhook/webhook.yml new file mode 100644 index 000000000..ec6f9bffa --- /dev/null +++ b/models/migrations/fixtures/Test_addHeaderAuthorizationEncryptedColWebhook/webhook.yml @@ -0,0 +1,10 @@ +# matrix webhook +- id: 1 + type: matrix + meta: '{"homeserver_url":"https://matrix.example.com","room_id":"roomID","access_token":"s3cr3t","message_type":1}' + header_authorization_encrypted: '' +# gitea webhook +- id: 2 + type: gitea + meta: '' + header_authorization_encrypted: '' diff --git a/models/migrations/migrations.go b/models/migrations/migrations.go index f7f48bee0..5f5ec8fdd 100644 --- a/models/migrations/migrations.go +++ b/models/migrations/migrations.go @@ -435,6 +435,8 @@ var migrations = []Migration{ NewMigration("Add index for hook_task", v1_19.AddIndexForHookTask), // v232 -> v233 NewMigration("Alter package_version.metadata_json to LONGTEXT", v1_19.AlterPackageVersionMetadataToLongText), + // v233 -> v234 + NewMigration("Add header_authorization_encrypted column to webhook table", v1_19.AddHeaderAuthorizationEncryptedColWebhook), } // GetCurrentDBVersion returns the current db version diff --git a/models/migrations/v1_19/v233.go b/models/migrations/v1_19/v233.go new file mode 100644 index 000000000..6443d58fb --- /dev/null +++ b/models/migrations/v1_19/v233.go @@ -0,0 +1,183 @@ +// Copyright 2022 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 v1_19 //nolint + +import ( + "fmt" + + "code.gitea.io/gitea/models/webhook" + "code.gitea.io/gitea/modules/json" + "code.gitea.io/gitea/modules/secret" + "code.gitea.io/gitea/modules/setting" + api "code.gitea.io/gitea/modules/structs" + + "xorm.io/builder" + "xorm.io/xorm" +) + +func batchProcess[T any](x *xorm.Engine, buf []T, query func(limit, start int) *xorm.Session, process func(*xorm.Session, T) error) error { + size := cap(buf) + start := 0 + for { + err := query(size, start).Find(&buf) + if err != nil { + return err + } + if len(buf) == 0 { + return nil + } + + err = func() error { + sess := x.NewSession() + defer sess.Close() + if err := sess.Begin(); err != nil { + return fmt.Errorf("unable to allow start session. Error: %w", err) + } + for _, record := range buf { + if err := process(sess, record); err != nil { + return err + } + } + return sess.Commit() + }() + if err != nil { + return err + } + + if len(buf) < size { + return nil + } + start += size + buf = buf[:0] + } +} + +func AddHeaderAuthorizationEncryptedColWebhook(x *xorm.Engine) error { + // Add the column to the table + type Webhook struct { + ID int64 `xorm:"pk autoincr"` + Type webhook.HookType `xorm:"VARCHAR(16) 'type'"` + Meta string `xorm:"TEXT"` // store hook-specific attributes + + // HeaderAuthorizationEncrypted should be accessed using HeaderAuthorization() and SetHeaderAuthorization() + HeaderAuthorizationEncrypted string `xorm:"TEXT"` + } + err := x.Sync(new(Webhook)) + if err != nil { + return err + } + + // Migrate the matrix webhooks + + type MatrixMeta struct { + HomeserverURL string `json:"homeserver_url"` + Room string `json:"room_id"` + MessageType int `json:"message_type"` + } + type MatrixMetaWithAccessToken struct { + MatrixMeta + AccessToken string `json:"access_token"` + } + + err = batchProcess(x, + make([]*Webhook, 0, 50), + func(limit, start int) *xorm.Session { + return x.Where("type=?", "matrix").OrderBy("id").Limit(limit, start) + }, + func(sess *xorm.Session, hook *Webhook) error { + // retrieve token from meta + var withToken MatrixMetaWithAccessToken + err := json.Unmarshal([]byte(hook.Meta), &withToken) + if err != nil { + return fmt.Errorf("unable to unmarshal matrix meta for webhook[id=%d]: %w", hook.ID, err) + } + if withToken.AccessToken == "" { + return nil + } + + // encrypt token + authorization := "Bearer " + withToken.AccessToken + hook.HeaderAuthorizationEncrypted, err = secret.EncryptSecret(setting.SecretKey, authorization) + if err != nil { + return fmt.Errorf("unable to encrypt access token for webhook[id=%d]: %w", hook.ID, err) + } + + // remove token from meta + withoutToken, err := json.Marshal(withToken.MatrixMeta) + if err != nil { + return fmt.Errorf("unable to marshal matrix meta for webhook[id=%d]: %w", hook.ID, err) + } + hook.Meta = string(withoutToken) + + // save in database + count, err := sess.ID(hook.ID).Cols("meta", "header_authorization_encrypted").Update(hook) + if count != 1 || err != nil { + return fmt.Errorf("unable to update header_authorization_encrypted for webhook[id=%d]: %d,%w", hook.ID, count, err) + } + return nil + }) + if err != nil { + return err + } + + // Remove access_token from HookTask + + type HookTask struct { + ID int64 `xorm:"pk autoincr"` + HookID int64 + PayloadContent string `xorm:"LONGTEXT"` + } + + type MatrixPayloadSafe struct { + Body string `json:"body"` + MsgType string `json:"msgtype"` + Format string `json:"format"` + FormattedBody string `json:"formatted_body"` + Commits []*api.PayloadCommit `json:"io.gitea.commits,omitempty"` + } + type MatrixPayloadUnsafe struct { + MatrixPayloadSafe + AccessToken string `json:"access_token"` + } + + err = batchProcess(x, + make([]*HookTask, 0, 50), + func(limit, start int) *xorm.Session { + return x.Where(builder.And( + builder.In("hook_id", builder.Select("id").From("webhook").Where(builder.Eq{"type": "matrix"})), + builder.Like{"payload_content", "access_token"}, + )).OrderBy("id").Limit(limit, 0) // ignore the provided "start", since other payload were already converted and don't contain 'payload_content' anymore + }, + func(sess *xorm.Session, hookTask *HookTask) error { + // retrieve token from payload_content + var withToken MatrixPayloadUnsafe + err := json.Unmarshal([]byte(hookTask.PayloadContent), &withToken) + if err != nil { + return fmt.Errorf("unable to unmarshal payload_content for hook_task[id=%d]: %w", hookTask.ID, err) + } + if withToken.AccessToken == "" { + return nil + } + + // remove token from payload_content + withoutToken, err := json.Marshal(withToken.MatrixPayloadSafe) + if err != nil { + return fmt.Errorf("unable to marshal payload_content for hook_task[id=%d]: %w", hookTask.ID, err) + } + hookTask.PayloadContent = string(withoutToken) + + // save in database + count, err := sess.ID(hookTask.ID).Cols("payload_content").Update(hookTask) + if count != 1 || err != nil { + return fmt.Errorf("unable to update payload_content for hook_task[id=%d]: %d,%w", hookTask.ID, count, err) + } + return nil + }) + if err != nil { + return err + } + + return nil +} diff --git a/models/migrations/v1_19/v233_test.go b/models/migrations/v1_19/v233_test.go new file mode 100644 index 000000000..f0a44df8c --- /dev/null +++ b/models/migrations/v1_19/v233_test.go @@ -0,0 +1,88 @@ +// Copyright 2022 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 v1_19 //nolint + +import ( + "testing" + + "code.gitea.io/gitea/models/migrations/base" + "code.gitea.io/gitea/models/webhook" + "code.gitea.io/gitea/modules/json" + "code.gitea.io/gitea/modules/secret" + "code.gitea.io/gitea/modules/setting" + + "github.com/stretchr/testify/assert" +) + +func Test_addHeaderAuthorizationEncryptedColWebhook(t *testing.T) { + // Create Webhook table + type Webhook struct { + ID int64 `xorm:"pk autoincr"` + Type webhook.HookType `xorm:"VARCHAR(16) 'type'"` + Meta string `xorm:"TEXT"` // store hook-specific attributes + + // HeaderAuthorizationEncrypted should be accessed using HeaderAuthorization() and SetHeaderAuthorization() + HeaderAuthorizationEncrypted string `xorm:"TEXT"` + } + + type ExpectedWebhook struct { + ID int64 `xorm:"pk autoincr"` + Meta string + HeaderAuthorization string + } + + type HookTask struct { + ID int64 `xorm:"pk autoincr"` + HookID int64 + PayloadContent string `xorm:"LONGTEXT"` + } + + // Prepare and load the testing database + x, deferable := base.PrepareTestEnv(t, 0, new(Webhook), new(ExpectedWebhook), new(HookTask)) + defer deferable() + if x == nil || t.Failed() { + return + } + + if err := AddHeaderAuthorizationEncryptedColWebhook(x); err != nil { + assert.NoError(t, err) + return + } + + expected := []ExpectedWebhook{} + if err := x.Table("expected_webhook").Asc("id").Find(&expected); !assert.NoError(t, err) { + return + } + + got := []Webhook{} + if err := x.Table("webhook").Select("id, meta, header_authorization_encrypted").Asc("id").Find(&got); !assert.NoError(t, err) { + return + } + + for i, e := range expected { + assert.Equal(t, e.Meta, got[i].Meta) + + if e.HeaderAuthorization == "" { + assert.Equal(t, "", got[i].HeaderAuthorizationEncrypted) + } else { + cipherhex := got[i].HeaderAuthorizationEncrypted + cleartext, err := secret.DecryptSecret(setting.SecretKey, cipherhex) + assert.NoError(t, err) + assert.Equal(t, e.HeaderAuthorization, cleartext) + } + } + + // ensure that no hook_task has some remaining "access_token" + hookTasks := []HookTask{} + if err := x.Table("hook_task").Select("id, payload_content").Asc("id").Find(&hookTasks); !assert.NoError(t, err) { + return + } + for _, h := range hookTasks { + var m map[string]interface{} + err := json.Unmarshal([]byte(h.PayloadContent), &m) + assert.NoError(t, err) + assert.Nil(t, m["access_token"]) + } +} diff --git a/models/webhook/webhook.go b/models/webhook/webhook.go index aebe0d6e7..6426b9520 100644 --- a/models/webhook/webhook.go +++ b/models/webhook/webhook.go @@ -13,6 +13,8 @@ import ( "code.gitea.io/gitea/models/db" "code.gitea.io/gitea/modules/json" "code.gitea.io/gitea/modules/log" + "code.gitea.io/gitea/modules/secret" + "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/timeutil" "code.gitea.io/gitea/modules/util" @@ -195,6 +197,9 @@ type Webhook struct { Meta string `xorm:"TEXT"` // store hook-specific attributes LastStatus HookStatus // Last delivery status + // HeaderAuthorizationEncrypted should be accessed using HeaderAuthorization() and SetHeaderAuthorization() + HeaderAuthorizationEncrypted string `xorm:"TEXT"` + CreatedUnix timeutil.TimeStamp `xorm:"INDEX created"` UpdatedUnix timeutil.TimeStamp `xorm:"INDEX updated"` } @@ -401,6 +406,29 @@ func (w *Webhook) EventsArray() []string { return events } +// HeaderAuthorization returns the decrypted Authorization header. +// Not on the reference (*w), to be accessible on WebhooksNew. +func (w Webhook) HeaderAuthorization() (string, error) { + if w.HeaderAuthorizationEncrypted == "" { + return "", nil + } + return secret.DecryptSecret(setting.SecretKey, w.HeaderAuthorizationEncrypted) +} + +// SetHeaderAuthorization encrypts and sets the Authorization header. +func (w *Webhook) SetHeaderAuthorization(cleartext string) error { + if cleartext == "" { + w.HeaderAuthorizationEncrypted = "" + return nil + } + ciphertext, err := secret.EncryptSecret(setting.SecretKey, cleartext) + if err != nil { + return err + } + w.HeaderAuthorizationEncrypted = ciphertext + return nil +} + // CreateWebhook creates a new web hook. func CreateWebhook(ctx context.Context, w *Webhook) error { w.Type = strings.TrimSpace(w.Type) diff --git a/modules/convert/convert.go b/modules/convert/convert.go index 8c92bbb37..78eb62d42 100644 --- a/modules/convert/convert.go +++ b/modules/convert/convert.go @@ -243,7 +243,7 @@ func ToGPGKeyEmail(email *user_model.EmailAddress) *api.GPGKeyEmail { } // ToHook convert models.Webhook to api.Hook -func ToHook(repoLink string, w *webhook.Webhook) *api.Hook { +func ToHook(repoLink string, w *webhook.Webhook) (*api.Hook, error) { config := map[string]string{ "url": w.URL, "content_type": w.ContentType.Name(), @@ -256,16 +256,22 @@ func ToHook(repoLink string, w *webhook.Webhook) *api.Hook { config["color"] = s.Color } - return &api.Hook{ - ID: w.ID, - Type: w.Type, - URL: fmt.Sprintf("%s/settings/hooks/%d", repoLink, w.ID), - Active: w.IsActive, - Config: config, - Events: w.EventsArray(), - Updated: w.UpdatedUnix.AsTime(), - Created: w.CreatedUnix.AsTime(), + authorizationHeader, err := w.HeaderAuthorization() + if err != nil { + return nil, err } + + return &api.Hook{ + ID: w.ID, + Type: w.Type, + URL: fmt.Sprintf("%s/settings/hooks/%d", repoLink, w.ID), + Active: w.IsActive, + Config: config, + Events: w.EventsArray(), + AuthorizationHeader: authorizationHeader, + Updated: w.UpdatedUnix.AsTime(), + Created: w.CreatedUnix.AsTime(), + }, nil } // ToGitHook convert git.Hook to api.GitHook diff --git a/modules/structs/hook.go b/modules/structs/hook.go index 8321a15a8..f0600a192 100644 --- a/modules/structs/hook.go +++ b/modules/structs/hook.go @@ -18,12 +18,13 @@ var ErrInvalidReceiveHook = errors.New("Invalid JSON payload received over webho // Hook a hook is a web hook when one repository changed type Hook struct { - ID int64 `json:"id"` - Type string `json:"type"` - URL string `json:"-"` - Config map[string]string `json:"config"` - Events []string `json:"events"` - Active bool `json:"active"` + ID int64 `json:"id"` + Type string `json:"type"` + URL string `json:"-"` + Config map[string]string `json:"config"` + Events []string `json:"events"` + AuthorizationHeader string `json:"authorization_header"` + Active bool `json:"active"` // swagger:strfmt date-time Updated time.Time `json:"updated_at"` // swagger:strfmt date-time @@ -43,19 +44,21 @@ type CreateHookOption struct { // enum: dingtalk,discord,gitea,gogs,msteams,slack,telegram,feishu,wechatwork,packagist Type string `json:"type" binding:"Required"` // required: true - Config CreateHookOptionConfig `json:"config" binding:"Required"` - Events []string `json:"events"` - BranchFilter string `json:"branch_filter" binding:"GlobPattern"` + Config CreateHookOptionConfig `json:"config" binding:"Required"` + Events []string `json:"events"` + BranchFilter string `json:"branch_filter" binding:"GlobPattern"` + AuthorizationHeader string `json:"authorization_header"` // default: false Active bool `json:"active"` } // EditHookOption options when modify one hook type EditHookOption struct { - Config map[string]string `json:"config"` - Events []string `json:"events"` - BranchFilter string `json:"branch_filter" binding:"GlobPattern"` - Active *bool `json:"active"` + Config map[string]string `json:"config"` + Events []string `json:"events"` + BranchFilter string `json:"branch_filter" binding:"GlobPattern"` + AuthorizationHeader string `json:"authorization_header"` + Active *bool `json:"active"` } // Payloader payload is some part of one hook diff --git a/options/locale/locale_en-US.ini b/options/locale/locale_en-US.ini index 8ffb7a5b2..8cda25055 100644 --- a/options/locale/locale_en-US.ini +++ b/options/locale/locale_en-US.ini @@ -2012,6 +2012,8 @@ settings.event_package = Package settings.event_package_desc = Package created or deleted in a repository. settings.branch_filter = Branch filter settings.branch_filter_desc = Branch whitelist for push, branch creation and branch deletion events, specified as glob pattern. If empty or *, events for all branches are reported. See github.com/gobwas/glob documentation for syntax. Examples: master, {master,release*}. +settings.authorization_header = Authorization Header +settings.authorization_header_desc = Will be included as authorization header for requests when present. Examples: %s. settings.active = Active settings.active_helper = Information about triggered events will be sent to this webhook URL. settings.add_hook_success = The webhook has been added. @@ -2126,7 +2128,6 @@ settings.bot_token = Bot Token settings.chat_id = Chat ID settings.matrix.homeserver_url = Homeserver URL settings.matrix.room_id = Room ID -settings.matrix.access_token = Access Token settings.matrix.message_type = Message Type settings.archive.button = Archive Repo settings.archive.header = Archive This Repo diff --git a/routers/api/v1/org/hook.go b/routers/api/v1/org/hook.go index 2ddb2b2d2..f8ea5a876 100644 --- a/routers/api/v1/org/hook.go +++ b/routers/api/v1/org/hook.go @@ -59,7 +59,11 @@ func ListHooks(ctx *context.APIContext) { hooks := make([]*api.Hook, len(orgHooks)) for i, hook := range orgHooks { - hooks[i] = convert.ToHook(ctx.Org.Organization.AsUser().HomeLink(), hook) + hooks[i], err = convert.ToHook(ctx.Org.Organization.AsUser().HomeLink(), hook) + if err != nil { + ctx.InternalServerError(err) + return + } } ctx.SetTotalCountHeader(count) @@ -95,7 +99,13 @@ func GetHook(ctx *context.APIContext) { if err != nil { return } - ctx.JSON(http.StatusOK, convert.ToHook(org.AsUser().HomeLink(), hook)) + + apiHook, err := convert.ToHook(org.AsUser().HomeLink(), hook) + if err != nil { + ctx.InternalServerError(err) + return + } + ctx.JSON(http.StatusOK, apiHook) } // CreateHook create a hook for an organization diff --git a/routers/api/v1/repo/hook.go b/routers/api/v1/repo/hook.go index 5956fe9da..57fff7d32 100644 --- a/routers/api/v1/repo/hook.go +++ b/routers/api/v1/repo/hook.go @@ -69,7 +69,11 @@ func ListHooks(ctx *context.APIContext) { apiHooks := make([]*api.Hook, len(hooks)) for i := range hooks { - apiHooks[i] = convert.ToHook(ctx.Repo.RepoLink, hooks[i]) + apiHooks[i], err = convert.ToHook(ctx.Repo.RepoLink, hooks[i]) + if err != nil { + ctx.InternalServerError(err) + return + } } ctx.SetTotalCountHeader(count) @@ -112,7 +116,12 @@ func GetHook(ctx *context.APIContext) { if err != nil { return } - ctx.JSON(http.StatusOK, convert.ToHook(repo.RepoLink, hook)) + apiHook, err := convert.ToHook(repo.RepoLink, hook) + if err != nil { + ctx.InternalServerError(err) + return + } + ctx.JSON(http.StatusOK, apiHook) } // TestHook tests a hook diff --git a/routers/api/v1/utils/hook.go b/routers/api/v1/utils/hook.go index 7e4dfca9a..aa922f4f5 100644 --- a/routers/api/v1/utils/hook.go +++ b/routers/api/v1/utils/hook.go @@ -72,18 +72,39 @@ func CheckCreateHookOption(ctx *context.APIContext, form *api.CreateHookOption) func AddOrgHook(ctx *context.APIContext, form *api.CreateHookOption) { org := ctx.Org.Organization hook, ok := addHook(ctx, form, org.ID, 0) - if ok { - ctx.JSON(http.StatusCreated, convert.ToHook(org.AsUser().HomeLink(), hook)) + if !ok { + return } + apiHook, ok := toAPIHook(ctx, org.AsUser().HomeLink(), hook) + if !ok { + return + } + ctx.JSON(http.StatusCreated, apiHook) } // AddRepoHook add a hook to a repo. Writes to `ctx` accordingly func AddRepoHook(ctx *context.APIContext, form *api.CreateHookOption) { repo := ctx.Repo hook, ok := addHook(ctx, form, 0, repo.Repository.ID) - if ok { - ctx.JSON(http.StatusCreated, convert.ToHook(repo.RepoLink, hook)) + if !ok { + return } + apiHook, ok := toAPIHook(ctx, repo.RepoLink, hook) + if !ok { + return + } + ctx.JSON(http.StatusCreated, apiHook) +} + +// toAPIHook converts the hook to its API representation. +// If there is an error, write to `ctx` accordingly. Return (hook, ok) +func toAPIHook(ctx *context.APIContext, repoLink string, hook *webhook.Webhook) (*api.Hook, bool) { + apiHook, err := convert.ToHook(repoLink, hook) + if err != nil { + ctx.Error(http.StatusInternalServerError, "ToHook", err) + return nil, false + } + return apiHook, true } func issuesHook(events []string, event string) bool { @@ -135,6 +156,11 @@ func addHook(ctx *context.APIContext, form *api.CreateHookOption, orgID, repoID IsActive: form.Active, Type: form.Type, } + err := w.SetHeaderAuthorization(form.AuthorizationHeader) + if err != nil { + ctx.Error(http.StatusInternalServerError, "SetHeaderAuthorization", err) + return nil, false + } if w.Type == webhook.SLACK { channel, ok := form.Config["channel"] if !ok { @@ -185,7 +211,11 @@ func EditOrgHook(ctx *context.APIContext, form *api.EditHookOption, hookID int64 if err != nil { return } - ctx.JSON(http.StatusOK, convert.ToHook(org.AsUser().HomeLink(), updated)) + apiHook, ok := toAPIHook(ctx, org.AsUser().HomeLink(), updated) + if !ok { + return + } + ctx.JSON(http.StatusOK, apiHook) } // EditRepoHook edit webhook `w` according to `form`. Writes to `ctx` accordingly @@ -202,7 +232,11 @@ func EditRepoHook(ctx *context.APIContext, form *api.EditHookOption, hookID int6 if err != nil { return } - ctx.JSON(http.StatusOK, convert.ToHook(repo.RepoLink, updated)) + apiHook, ok := toAPIHook(ctx, repo.RepoLink, updated) + if !ok { + return + } + ctx.JSON(http.StatusOK, apiHook) } // editHook edit the webhook `w` according to `form`. If an error occurs, write @@ -254,6 +288,12 @@ func editHook(ctx *context.APIContext, form *api.EditHookOption, w *webhook.Webh w.Release = util.IsStringInSlice(string(webhook.HookEventRelease), form.Events, true) w.BranchFilter = form.BranchFilter + err := w.SetHeaderAuthorization(form.AuthorizationHeader) + if err != nil { + ctx.Error(http.StatusInternalServerError, "SetHeaderAuthorization", err) + return false + } + // Issues w.Issues = issuesHook(form.Events, "issues_only") w.IssueAssign = issuesHook(form.Events, string(webhook.HookEventIssueAssign)) diff --git a/routers/web/repo/webhook.go b/routers/web/repo/webhook.go index ee980333b..5496496e8 100644 --- a/routers/web/repo/webhook.go +++ b/routers/web/repo/webhook.go @@ -239,6 +239,11 @@ func createWebhook(ctx *context.Context, params webhookParams) { OrgID: orCtx.OrgID, IsSystemWebhook: orCtx.IsSystemWebhook, } + err = w.SetHeaderAuthorization(params.WebhookForm.AuthorizationHeader) + if err != nil { + ctx.ServerError("SetHeaderAuthorization", err) + return + } if err := w.UpdateEvent(); err != nil { ctx.ServerError("UpdateEvent", err) return @@ -285,6 +290,12 @@ func editWebhook(ctx *context.Context, params webhookParams) { w.HTTPMethod = params.HTTPMethod w.Meta = string(meta) + err = w.SetHeaderAuthorization(params.WebhookForm.AuthorizationHeader) + if err != nil { + ctx.ServerError("SetHeaderAuthorization", err) + return + } + if err := w.UpdateEvent(); err != nil { ctx.ServerError("UpdateEvent", err) return @@ -445,7 +456,6 @@ func matrixHookParams(ctx *context.Context) webhookParams { Meta: &webhook_service.MatrixMeta{ HomeserverURL: form.HomeserverURL, Room: form.RoomID, - AccessToken: form.AccessToken, MessageType: form.MessageType, }, } diff --git a/services/forms/repo_form.go b/services/forms/repo_form.go index c1e9cb319..64f47aadd 100644 --- a/services/forms/repo_form.go +++ b/services/forms/repo_form.go @@ -247,6 +247,7 @@ type WebhookForm struct { Package bool Active bool BranchFilter string `binding:"GlobPattern"` + AuthorizationHeader string } // PushOnly if the hook will be triggered when push @@ -359,7 +360,6 @@ func (f *NewTelegramHookForm) Validate(req *http.Request, errs binding.Errors) b type NewMatrixHookForm struct { HomeserverURL string `binding:"Required;ValidUrl"` RoomID string `binding:"Required"` - AccessToken string `binding:"Required"` MessageType int WebhookForm } diff --git a/services/webhook/deliver.go b/services/webhook/deliver.go index 74a69c297..85717e097 100644 --- a/services/webhook/deliver.go +++ b/services/webhook/deliver.go @@ -90,7 +90,12 @@ func Deliver(ctx context.Context, t *webhook_model.HookTask) error { case http.MethodPut: switch w.Type { case webhook_model.MATRIX: - req, err = getMatrixHookRequest(w, t) + txnID, err := getMatrixTxnID([]byte(t.PayloadContent)) + if err != nil { + return err + } + url := fmt.Sprintf("%s/%s", w.URL, url.PathEscape(txnID)) + req, err = http.NewRequest("PUT", url, strings.NewReader(t.PayloadContent)) if err != nil { return err } @@ -130,6 +135,16 @@ func Deliver(ctx context.Context, t *webhook_model.HookTask) error { req.Header["X-GitHub-Event"] = []string{event} req.Header["X-GitHub-Event-Type"] = []string{eventType} + // Add Authorization Header + authorization, err := w.HeaderAuthorization() + if err != nil { + log.Error("Webhook could not get Authorization header [%d]: %v", w.ID, err) + return err + } + if authorization != "" { + req.Header["Authorization"] = []string{authorization} + } + // Record delivery information. t.RequestInfo = &webhook_model.HookRequest{ URL: req.URL.String(), diff --git a/services/webhook/deliver_test.go b/services/webhook/deliver_test.go index 8d1d587c3..83ca7d617 100644 --- a/services/webhook/deliver_test.go +++ b/services/webhook/deliver_test.go @@ -5,10 +5,16 @@ package webhook import ( + "context" "net/http" + "net/http/httptest" "net/url" "testing" + "time" + "code.gitea.io/gitea/models/db" + "code.gitea.io/gitea/models/unittest" + webhook_model "code.gitea.io/gitea/models/webhook" "code.gitea.io/gitea/modules/setting" "github.com/stretchr/testify/assert" @@ -38,3 +44,38 @@ func TestWebhookProxy(t *testing.T) { } } } + +func TestWebhookDeliverAuthorizationHeader(t *testing.T) { + assert.NoError(t, unittest.PrepareTestDatabase()) + + done := make(chan struct{}, 1) + s := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + assert.Equal(t, "/webhook", r.URL.Path) + assert.Equal(t, "Bearer s3cr3t-t0ken", r.Header.Get("Authorization")) + w.WriteHeader(200) + done <- struct{}{} + })) + t.Cleanup(s.Close) + + hook := &webhook_model.Webhook{ + RepoID: 3, + URL: s.URL + "/webhook", + ContentType: webhook_model.ContentTypeJSON, + IsActive: true, + Type: webhook_model.GITEA, + } + err := hook.SetHeaderAuthorization("Bearer s3cr3t-t0ken") + assert.NoError(t, err) + assert.NoError(t, webhook_model.CreateWebhook(db.DefaultContext, hook)) + + hookTask := &webhook_model.HookTask{HookID: hook.ID, EventType: webhook_model.HookEventPush} + + assert.NoError(t, Deliver(context.Background(), hookTask)) + select { + case <-done: + case <-time.After(5 * time.Second): + t.Fatal("waited to long for request to happen") + } + + assert.True(t, hookTask.IsSucceed) +} diff --git a/services/webhook/main_test.go b/services/webhook/main_test.go index 1dc2e1bd8..5ddb6cf1f 100644 --- a/services/webhook/main_test.go +++ b/services/webhook/main_test.go @@ -9,6 +9,7 @@ import ( "testing" "code.gitea.io/gitea/models/unittest" + "code.gitea.io/gitea/modules/hostmatcher" "code.gitea.io/gitea/modules/setting" _ "code.gitea.io/gitea/models" @@ -17,6 +18,9 @@ import ( func TestMain(m *testing.M) { setting.LoadForTest() setting.NewQueueService() + + // for tests, allow only loopback IPs + setting.Webhook.AllowedHostList = hostmatcher.MatchBuiltinLoopback unittest.MainTest(m, &unittest.TestOptions{ GiteaRootPath: filepath.Join("..", ".."), SetUp: Init, diff --git a/services/webhook/matrix.go b/services/webhook/matrix.go index 6bbae7042..7ff8b1e63 100644 --- a/services/webhook/matrix.go +++ b/services/webhook/matrix.go @@ -6,10 +6,10 @@ package webhook import ( "crypto/sha1" + "encoding/hex" "errors" "fmt" "html" - "net/http" "net/url" "regexp" "strings" @@ -29,7 +29,6 @@ const matrixPayloadSizeLimit = 1024 * 64 type MatrixMeta struct { HomeserverURL string `json:"homeserver_url"` Room string `json:"room_id"` - AccessToken string `json:"access_token"` MessageType int `json:"message_type"` } @@ -47,27 +46,10 @@ func GetMatrixHook(w *webhook_model.Webhook) *MatrixMeta { return s } -// MatrixPayloadUnsafe contains the (unsafe) payload for a Matrix room -type MatrixPayloadUnsafe struct { - MatrixPayloadSafe - AccessToken string `json:"access_token"` -} +var _ PayloadConvertor = &MatrixPayload{} -var _ PayloadConvertor = &MatrixPayloadUnsafe{} - -// safePayload "converts" a unsafe payload to a safe payload -func (m *MatrixPayloadUnsafe) safePayload() *MatrixPayloadSafe { - return &MatrixPayloadSafe{ - Body: m.Body, - MsgType: m.MsgType, - Format: m.Format, - FormattedBody: m.FormattedBody, - Commits: m.Commits, - } -} - -// MatrixPayloadSafe contains (safe) payload for a Matrix room -type MatrixPayloadSafe struct { +// MatrixPayload contains payload for a Matrix room +type MatrixPayload struct { Body string `json:"body"` MsgType string `json:"msgtype"` Format string `json:"format"` @@ -75,8 +57,8 @@ type MatrixPayloadSafe struct { Commits []*api.PayloadCommit `json:"io.gitea.commits,omitempty"` } -// JSONPayload Marshals the MatrixPayloadUnsafe to json -func (m *MatrixPayloadUnsafe) JSONPayload() ([]byte, error) { +// JSONPayload Marshals the MatrixPayload to json +func (m *MatrixPayload) JSONPayload() ([]byte, error) { data, err := json.MarshalIndent(m, "", " ") if err != nil { return []byte{}, err @@ -103,62 +85,62 @@ func MatrixLinkToRef(repoURL, ref string) string { } // Create implements PayloadConvertor Create method -func (m *MatrixPayloadUnsafe) Create(p *api.CreatePayload) (api.Payloader, error) { +func (m *MatrixPayload) Create(p *api.CreatePayload) (api.Payloader, error) { repoLink := MatrixLinkFormatter(p.Repo.HTMLURL, p.Repo.FullName) refLink := MatrixLinkToRef(p.Repo.HTMLURL, p.Ref) text := fmt.Sprintf("[%s:%s] %s created by %s", repoLink, refLink, p.RefType, p.Sender.UserName) - return getMatrixPayloadUnsafe(text, nil, m.AccessToken, m.MsgType), nil + return getMatrixPayload(text, nil, m.MsgType), nil } // Delete composes Matrix payload for delete a branch or tag. -func (m *MatrixPayloadUnsafe) Delete(p *api.DeletePayload) (api.Payloader, error) { +func (m *MatrixPayload) Delete(p *api.DeletePayload) (api.Payloader, error) { refName := git.RefEndName(p.Ref) repoLink := MatrixLinkFormatter(p.Repo.HTMLURL, p.Repo.FullName) text := fmt.Sprintf("[%s:%s] %s deleted by %s", repoLink, refName, p.RefType, p.Sender.UserName) - return getMatrixPayloadUnsafe(text, nil, m.AccessToken, m.MsgType), nil + return getMatrixPayload(text, nil, m.MsgType), nil } // Fork composes Matrix payload for forked by a repository. -func (m *MatrixPayloadUnsafe) Fork(p *api.ForkPayload) (api.Payloader, error) { +func (m *MatrixPayload) Fork(p *api.ForkPayload) (api.Payloader, error) { baseLink := MatrixLinkFormatter(p.Forkee.HTMLURL, p.Forkee.FullName) forkLink := MatrixLinkFormatter(p.Repo.HTMLURL, p.Repo.FullName) text := fmt.Sprintf("%s is forked to %s", baseLink, forkLink) - return getMatrixPayloadUnsafe(text, nil, m.AccessToken, m.MsgType), nil + return getMatrixPayload(text, nil, m.MsgType), nil } // Issue implements PayloadConvertor Issue method -func (m *MatrixPayloadUnsafe) Issue(p *api.IssuePayload) (api.Payloader, error) { +func (m *MatrixPayload) Issue(p *api.IssuePayload) (api.Payloader, error) { text, _, _, _ := getIssuesPayloadInfo(p, MatrixLinkFormatter, true) - return getMatrixPayloadUnsafe(text, nil, m.AccessToken, m.MsgType), nil + return getMatrixPayload(text, nil, m.MsgType), nil } // IssueComment implements PayloadConvertor IssueComment method -func (m *MatrixPayloadUnsafe) IssueComment(p *api.IssueCommentPayload) (api.Payloader, error) { +func (m *MatrixPayload) IssueComment(p *api.IssueCommentPayload) (api.Payloader, error) { text, _, _ := getIssueCommentPayloadInfo(p, MatrixLinkFormatter, true) - return getMatrixPayloadUnsafe(text, nil, m.AccessToken, m.MsgType), nil + return getMatrixPayload(text, nil, m.MsgType), nil } // Wiki implements PayloadConvertor Wiki method -func (m *MatrixPayloadUnsafe) Wiki(p *api.WikiPayload) (api.Payloader, error) { +func (m *MatrixPayload) Wiki(p *api.WikiPayload) (api.Payloader, error) { text, _, _ := getWikiPayloadInfo(p, MatrixLinkFormatter, true) - return getMatrixPayloadUnsafe(text, nil, m.AccessToken, m.MsgType), nil + return getMatrixPayload(text, nil, m.MsgType), nil } // Release implements PayloadConvertor Release method -func (m *MatrixPayloadUnsafe) Release(p *api.ReleasePayload) (api.Payloader, error) { +func (m *MatrixPayload) Release(p *api.ReleasePayload) (api.Payloader, error) { text, _ := getReleasePayloadInfo(p, MatrixLinkFormatter, true) - return getMatrixPayloadUnsafe(text, nil, m.AccessToken, m.MsgType), nil + return getMatrixPayload(text, nil, m.MsgType), nil } // Push implements PayloadConvertor Push method -func (m *MatrixPayloadUnsafe) Push(p *api.PushPayload) (api.Payloader, error) { +func (m *MatrixPayload) Push(p *api.PushPayload) (api.Payloader, error) { var commitDesc string if p.TotalCommits == 1 { @@ -181,18 +163,18 @@ func (m *MatrixPayloadUnsafe) Push(p *api.PushPayload) (api.Payloader, error) { } - return getMatrixPayloadUnsafe(text, p.Commits, m.AccessToken, m.MsgType), nil + return getMatrixPayload(text, p.Commits, m.MsgType), nil } // PullRequest implements PayloadConvertor PullRequest method -func (m *MatrixPayloadUnsafe) PullRequest(p *api.PullRequestPayload) (api.Payloader, error) { +func (m *MatrixPayload) PullRequest(p *api.PullRequestPayload) (api.Payloader, error) { text, _, _, _ := getPullRequestPayloadInfo(p, MatrixLinkFormatter, true) - return getMatrixPayloadUnsafe(text, nil, m.AccessToken, m.MsgType), nil + return getMatrixPayload(text, nil, m.MsgType), nil } // Review implements PayloadConvertor Review method -func (m *MatrixPayloadUnsafe) Review(p *api.PullRequestPayload, event webhook_model.HookEventType) (api.Payloader, error) { +func (m *MatrixPayload) Review(p *api.PullRequestPayload, event webhook_model.HookEventType) (api.Payloader, error) { senderLink := MatrixLinkFormatter(setting.AppURL+url.PathEscape(p.Sender.UserName), p.Sender.UserName) title := fmt.Sprintf("#%d %s", p.Index, p.PullRequest.Title) titleLink := MatrixLinkFormatter(p.PullRequest.URL, title) @@ -209,11 +191,11 @@ func (m *MatrixPayloadUnsafe) Review(p *api.PullRequestPayload, event webhook_mo text = fmt.Sprintf("[%s] Pull request review %s: %s by %s", repoLink, action, titleLink, senderLink) } - return getMatrixPayloadUnsafe(text, nil, m.AccessToken, m.MsgType), nil + return getMatrixPayload(text, nil, m.MsgType), nil } // Repository implements PayloadConvertor Repository method -func (m *MatrixPayloadUnsafe) Repository(p *api.RepositoryPayload) (api.Payloader, error) { +func (m *MatrixPayload) Repository(p *api.RepositoryPayload) (api.Payloader, error) { senderLink := MatrixLinkFormatter(setting.AppURL+p.Sender.UserName, p.Sender.UserName) repoLink := MatrixLinkFormatter(p.Repository.HTMLURL, p.Repository.FullName) var text string @@ -225,27 +207,25 @@ func (m *MatrixPayloadUnsafe) Repository(p *api.RepositoryPayload) (api.Payloade text = fmt.Sprintf("[%s] Repository deleted by %s", repoLink, senderLink) } - return getMatrixPayloadUnsafe(text, nil, m.AccessToken, m.MsgType), nil + return getMatrixPayload(text, nil, m.MsgType), nil } -// GetMatrixPayload converts a Matrix webhook into a MatrixPayloadUnsafe +// GetMatrixPayload converts a Matrix webhook into a MatrixPayload func GetMatrixPayload(p api.Payloader, event webhook_model.HookEventType, meta string) (api.Payloader, error) { - s := new(MatrixPayloadUnsafe) + s := new(MatrixPayload) matrix := &MatrixMeta{} if err := json.Unmarshal([]byte(meta), &matrix); err != nil { return s, errors.New("GetMatrixPayload meta json:" + err.Error()) } - s.AccessToken = matrix.AccessToken s.MsgType = messageTypeText[matrix.MessageType] return convertPayloader(s, p, event) } -func getMatrixPayloadUnsafe(text string, commits []*api.PayloadCommit, accessToken, msgType string) *MatrixPayloadUnsafe { - p := MatrixPayloadUnsafe{} - p.AccessToken = accessToken +func getMatrixPayload(text string, commits []*api.PayloadCommit, msgType string) *MatrixPayload { + p := MatrixPayload{} p.FormattedBody = text p.Body = getMessageBody(text) p.Format = "org.matrix.custom.html" @@ -262,52 +242,17 @@ func getMessageBody(htmlText string) string { return htmlText } -// getMatrixHookRequest creates a new request which contains an Authorization header. -// The access_token is removed from t.PayloadContent -func getMatrixHookRequest(w *webhook_model.Webhook, t *webhook_model.HookTask) (*http.Request, error) { - payloadunsafe := MatrixPayloadUnsafe{} - if err := json.Unmarshal([]byte(t.PayloadContent), &payloadunsafe); err != nil { - log.Error("Matrix Hook delivery failed: %v", err) - return nil, err - } - - payloadsafe := payloadunsafe.safePayload() - - var payload []byte - var err error - if payload, err = json.MarshalIndent(payloadsafe, "", " "); err != nil { - return nil, err - } - if len(payload) >= matrixPayloadSizeLimit { - return nil, fmt.Errorf("getMatrixHookRequest: payload size %d > %d", len(payload), matrixPayloadSizeLimit) - } - t.PayloadContent = string(payload) - - txnID, err := getMatrixTxnID(payload) - if err != nil { - return nil, fmt.Errorf("getMatrixHookRequest: unable to hash payload: %+v", err) - } - - url := fmt.Sprintf("%s/%s", w.URL, url.PathEscape(txnID)) - - req, err := http.NewRequest(w.HTTPMethod, url, strings.NewReader(string(payload))) - if err != nil { - return nil, err - } - - req.Header.Set("Content-Type", "application/json") - req.Header.Add("Authorization", "Bearer "+payloadunsafe.AccessToken) - - return req, nil -} - -// getMatrixTxnID creates a txnID based on the payload to ensure idempotency +// getMatrixTxnID computes the transaction ID to ensure idempotency func getMatrixTxnID(payload []byte) (string, error) { + if len(payload) >= matrixPayloadSizeLimit { + return "", fmt.Errorf("getMatrixTxnID: payload size %d > %d", len(payload), matrixPayloadSizeLimit) + } + h := sha1.New() _, err := h.Write(payload) if err != nil { return "", err } - return fmt.Sprintf("%x", h.Sum(nil)), nil + return hex.EncodeToString(h.Sum(nil)), nil } diff --git a/services/webhook/matrix_test.go b/services/webhook/matrix_test.go index 624986ee9..bbcdef356 100644 --- a/services/webhook/matrix_test.go +++ b/services/webhook/matrix_test.go @@ -18,275 +18,203 @@ func TestMatrixPayload(t *testing.T) { t.Run("Create", func(t *testing.T) { p := createTestPayload() - d := new(MatrixPayloadUnsafe) + d := new(MatrixPayload) pl, err := d.Create(p) require.NoError(t, err) require.NotNil(t, pl) - require.IsType(t, &MatrixPayloadUnsafe{}, pl) + require.IsType(t, &MatrixPayload{}, pl) - assert.Equal(t, "[[test/repo](http://localhost:3000/test/repo):[test](http://localhost:3000/test/repo/src/branch/test)] branch created by user1", pl.(*MatrixPayloadUnsafe).Body) - assert.Equal(t, `[test/repo:test] branch created by user1`, pl.(*MatrixPayloadUnsafe).FormattedBody) + assert.Equal(t, "[[test/repo](http://localhost:3000/test/repo):[test](http://localhost:3000/test/repo/src/branch/test)] branch created by user1", pl.(*MatrixPayload).Body) + assert.Equal(t, `[test/repo:test] branch created by user1`, pl.(*MatrixPayload).FormattedBody) }) t.Run("Delete", func(t *testing.T) { p := deleteTestPayload() - d := new(MatrixPayloadUnsafe) + d := new(MatrixPayload) pl, err := d.Delete(p) require.NoError(t, err) require.NotNil(t, pl) - require.IsType(t, &MatrixPayloadUnsafe{}, pl) + require.IsType(t, &MatrixPayload{}, pl) - assert.Equal(t, "[[test/repo](http://localhost:3000/test/repo):test] branch deleted by user1", pl.(*MatrixPayloadUnsafe).Body) - assert.Equal(t, `[test/repo:test] branch deleted by user1`, pl.(*MatrixPayloadUnsafe).FormattedBody) + assert.Equal(t, "[[test/repo](http://localhost:3000/test/repo):test] branch deleted by user1", pl.(*MatrixPayload).Body) + assert.Equal(t, `[test/repo:test] branch deleted by user1`, pl.(*MatrixPayload).FormattedBody) }) t.Run("Fork", func(t *testing.T) { p := forkTestPayload() - d := new(MatrixPayloadUnsafe) + d := new(MatrixPayload) pl, err := d.Fork(p) require.NoError(t, err) require.NotNil(t, pl) - require.IsType(t, &MatrixPayloadUnsafe{}, pl) + require.IsType(t, &MatrixPayload{}, pl) - assert.Equal(t, "[test/repo2](http://localhost:3000/test/repo2) is forked to [test/repo](http://localhost:3000/test/repo)", pl.(*MatrixPayloadUnsafe).Body) - assert.Equal(t, `test/repo2 is forked to test/repo`, pl.(*MatrixPayloadUnsafe).FormattedBody) + assert.Equal(t, "[test/repo2](http://localhost:3000/test/repo2) is forked to [test/repo](http://localhost:3000/test/repo)", pl.(*MatrixPayload).Body) + assert.Equal(t, `test/repo2 is forked to test/repo`, pl.(*MatrixPayload).FormattedBody) }) t.Run("Push", func(t *testing.T) { p := pushTestPayload() - d := new(MatrixPayloadUnsafe) + d := new(MatrixPayload) pl, err := d.Push(p) require.NoError(t, err) require.NotNil(t, pl) - require.IsType(t, &MatrixPayloadUnsafe{}, pl) + require.IsType(t, &MatrixPayload{}, pl) - assert.Equal(t, "[[test/repo](http://localhost:3000/test/repo)] user1 pushed 2 commits to [test](http://localhost:3000/test/repo/src/branch/test):\n[2020558](http://localhost:3000/test/repo/commit/2020558fe2e34debb818a514715839cabd25e778): commit message - user1\n[2020558](http://localhost:3000/test/repo/commit/2020558fe2e34debb818a514715839cabd25e778): commit message - user1", pl.(*MatrixPayloadUnsafe).Body) - assert.Equal(t, `[test/repo] user1 pushed 2 commits to test:
2020558: commit message - user1
2020558: commit message - user1`, pl.(*MatrixPayloadUnsafe).FormattedBody) + assert.Equal(t, "[[test/repo](http://localhost:3000/test/repo)] user1 pushed 2 commits to [test](http://localhost:3000/test/repo/src/branch/test):\n[2020558](http://localhost:3000/test/repo/commit/2020558fe2e34debb818a514715839cabd25e778): commit message - user1\n[2020558](http://localhost:3000/test/repo/commit/2020558fe2e34debb818a514715839cabd25e778): commit message - user1", pl.(*MatrixPayload).Body) + assert.Equal(t, `[test/repo] user1 pushed 2 commits to test:
2020558: commit message - user1
2020558: commit message - user1`, pl.(*MatrixPayload).FormattedBody) }) t.Run("Issue", func(t *testing.T) { p := issueTestPayload() - d := new(MatrixPayloadUnsafe) + d := new(MatrixPayload) p.Action = api.HookIssueOpened pl, err := d.Issue(p) require.NoError(t, err) require.NotNil(t, pl) - require.IsType(t, &MatrixPayloadUnsafe{}, pl) + require.IsType(t, &MatrixPayload{}, pl) - assert.Equal(t, "[[test/repo](http://localhost:3000/test/repo)] Issue opened: [#2 crash](http://localhost:3000/test/repo/issues/2) by [user1](https://try.gitea.io/user1)", pl.(*MatrixPayloadUnsafe).Body) - assert.Equal(t, `[test/repo] Issue opened: #2 crash by user1`, pl.(*MatrixPayloadUnsafe).FormattedBody) + assert.Equal(t, "[[test/repo](http://localhost:3000/test/repo)] Issue opened: [#2 crash](http://localhost:3000/test/repo/issues/2) by [user1](https://try.gitea.io/user1)", pl.(*MatrixPayload).Body) + assert.Equal(t, `[test/repo] Issue opened: #2 crash by user1`, pl.(*MatrixPayload).FormattedBody) p.Action = api.HookIssueClosed pl, err = d.Issue(p) require.NoError(t, err) require.NotNil(t, pl) - require.IsType(t, &MatrixPayloadUnsafe{}, pl) + require.IsType(t, &MatrixPayload{}, pl) - assert.Equal(t, "[[test/repo](http://localhost:3000/test/repo)] Issue closed: [#2 crash](http://localhost:3000/test/repo/issues/2) by [user1](https://try.gitea.io/user1)", pl.(*MatrixPayloadUnsafe).Body) - assert.Equal(t, `[test/repo] Issue closed: #2 crash by user1`, pl.(*MatrixPayloadUnsafe).FormattedBody) + assert.Equal(t, "[[test/repo](http://localhost:3000/test/repo)] Issue closed: [#2 crash](http://localhost:3000/test/repo/issues/2) by [user1](https://try.gitea.io/user1)", pl.(*MatrixPayload).Body) + assert.Equal(t, `[test/repo] Issue closed: #2 crash by user1`, pl.(*MatrixPayload).FormattedBody) }) t.Run("IssueComment", func(t *testing.T) { p := issueCommentTestPayload() - d := new(MatrixPayloadUnsafe) + d := new(MatrixPayload) pl, err := d.IssueComment(p) require.NoError(t, err) require.NotNil(t, pl) - require.IsType(t, &MatrixPayloadUnsafe{}, pl) + require.IsType(t, &MatrixPayload{}, pl) - assert.Equal(t, "[[test/repo](http://localhost:3000/test/repo)] New comment on issue [#2 crash](http://localhost:3000/test/repo/issues/2) by [user1](https://try.gitea.io/user1)", pl.(*MatrixPayloadUnsafe).Body) - assert.Equal(t, `[test/repo] New comment on issue #2 crash by user1`, pl.(*MatrixPayloadUnsafe).FormattedBody) + assert.Equal(t, "[[test/repo](http://localhost:3000/test/repo)] New comment on issue [#2 crash](http://localhost:3000/test/repo/issues/2) by [user1](https://try.gitea.io/user1)", pl.(*MatrixPayload).Body) + assert.Equal(t, `[test/repo] New comment on issue #2 crash by user1`, pl.(*MatrixPayload).FormattedBody) }) t.Run("PullRequest", func(t *testing.T) { p := pullRequestTestPayload() - d := new(MatrixPayloadUnsafe) + d := new(MatrixPayload) pl, err := d.PullRequest(p) require.NoError(t, err) require.NotNil(t, pl) - require.IsType(t, &MatrixPayloadUnsafe{}, pl) + require.IsType(t, &MatrixPayload{}, pl) - assert.Equal(t, "[[test/repo](http://localhost:3000/test/repo)] Pull request opened: [#12 Fix bug](http://localhost:3000/test/repo/pulls/12) by [user1](https://try.gitea.io/user1)", pl.(*MatrixPayloadUnsafe).Body) - assert.Equal(t, `[test/repo] Pull request opened: #12 Fix bug by user1`, pl.(*MatrixPayloadUnsafe).FormattedBody) + assert.Equal(t, "[[test/repo](http://localhost:3000/test/repo)] Pull request opened: [#12 Fix bug](http://localhost:3000/test/repo/pulls/12) by [user1](https://try.gitea.io/user1)", pl.(*MatrixPayload).Body) + assert.Equal(t, `[test/repo] Pull request opened: #12 Fix bug by user1`, pl.(*MatrixPayload).FormattedBody) }) t.Run("PullRequestComment", func(t *testing.T) { p := pullRequestCommentTestPayload() - d := new(MatrixPayloadUnsafe) + d := new(MatrixPayload) pl, err := d.IssueComment(p) require.NoError(t, err) require.NotNil(t, pl) - require.IsType(t, &MatrixPayloadUnsafe{}, pl) + require.IsType(t, &MatrixPayload{}, pl) - assert.Equal(t, "[[test/repo](http://localhost:3000/test/repo)] New comment on pull request [#12 Fix bug](http://localhost:3000/test/repo/pulls/12) by [user1](https://try.gitea.io/user1)", pl.(*MatrixPayloadUnsafe).Body) - assert.Equal(t, `[test/repo] New comment on pull request #12 Fix bug by user1`, pl.(*MatrixPayloadUnsafe).FormattedBody) + assert.Equal(t, "[[test/repo](http://localhost:3000/test/repo)] New comment on pull request [#12 Fix bug](http://localhost:3000/test/repo/pulls/12) by [user1](https://try.gitea.io/user1)", pl.(*MatrixPayload).Body) + assert.Equal(t, `[test/repo] New comment on pull request #12 Fix bug by user1`, pl.(*MatrixPayload).FormattedBody) }) t.Run("Review", func(t *testing.T) { p := pullRequestTestPayload() p.Action = api.HookIssueReviewed - d := new(MatrixPayloadUnsafe) + d := new(MatrixPayload) pl, err := d.Review(p, webhook_model.HookEventPullRequestReviewApproved) require.NoError(t, err) require.NotNil(t, pl) - require.IsType(t, &MatrixPayloadUnsafe{}, pl) + require.IsType(t, &MatrixPayload{}, pl) - assert.Equal(t, "[[test/repo](http://localhost:3000/test/repo)] Pull request review approved: [#12 Fix bug](http://localhost:3000/test/repo/pulls/12) by [user1](https://try.gitea.io/user1)", pl.(*MatrixPayloadUnsafe).Body) - assert.Equal(t, `[test/repo] Pull request review approved: #12 Fix bug by user1`, pl.(*MatrixPayloadUnsafe).FormattedBody) + assert.Equal(t, "[[test/repo](http://localhost:3000/test/repo)] Pull request review approved: [#12 Fix bug](http://localhost:3000/test/repo/pulls/12) by [user1](https://try.gitea.io/user1)", pl.(*MatrixPayload).Body) + assert.Equal(t, `[test/repo] Pull request review approved: #12 Fix bug by user1`, pl.(*MatrixPayload).FormattedBody) }) t.Run("Repository", func(t *testing.T) { p := repositoryTestPayload() - d := new(MatrixPayloadUnsafe) + d := new(MatrixPayload) pl, err := d.Repository(p) require.NoError(t, err) require.NotNil(t, pl) - require.IsType(t, &MatrixPayloadUnsafe{}, pl) + require.IsType(t, &MatrixPayload{}, pl) - assert.Equal(t, `[[test/repo](http://localhost:3000/test/repo)] Repository created by [user1](https://try.gitea.io/user1)`, pl.(*MatrixPayloadUnsafe).Body) - assert.Equal(t, `[test/repo] Repository created by user1`, pl.(*MatrixPayloadUnsafe).FormattedBody) + assert.Equal(t, `[[test/repo](http://localhost:3000/test/repo)] Repository created by [user1](https://try.gitea.io/user1)`, pl.(*MatrixPayload).Body) + assert.Equal(t, `[test/repo] Repository created by user1`, pl.(*MatrixPayload).FormattedBody) }) t.Run("Wiki", func(t *testing.T) { p := wikiTestPayload() - d := new(MatrixPayloadUnsafe) + d := new(MatrixPayload) p.Action = api.HookWikiCreated pl, err := d.Wiki(p) require.NoError(t, err) require.NotNil(t, pl) - require.IsType(t, &MatrixPayloadUnsafe{}, pl) + require.IsType(t, &MatrixPayload{}, pl) - assert.Equal(t, "[[test/repo](http://localhost:3000/test/repo)] New wiki page '[index](http://localhost:3000/test/repo/wiki/index)' (Wiki change comment) by [user1](https://try.gitea.io/user1)", pl.(*MatrixPayloadUnsafe).Body) - assert.Equal(t, `[test/repo] New wiki page 'index' (Wiki change comment) by user1`, pl.(*MatrixPayloadUnsafe).FormattedBody) + assert.Equal(t, "[[test/repo](http://localhost:3000/test/repo)] New wiki page '[index](http://localhost:3000/test/repo/wiki/index)' (Wiki change comment) by [user1](https://try.gitea.io/user1)", pl.(*MatrixPayload).Body) + assert.Equal(t, `[test/repo] New wiki page 'index' (Wiki change comment) by user1`, pl.(*MatrixPayload).FormattedBody) p.Action = api.HookWikiEdited pl, err = d.Wiki(p) require.NoError(t, err) require.NotNil(t, pl) - require.IsType(t, &MatrixPayloadUnsafe{}, pl) + require.IsType(t, &MatrixPayload{}, pl) - assert.Equal(t, "[[test/repo](http://localhost:3000/test/repo)] Wiki page '[index](http://localhost:3000/test/repo/wiki/index)' edited (Wiki change comment) by [user1](https://try.gitea.io/user1)", pl.(*MatrixPayloadUnsafe).Body) - assert.Equal(t, `[test/repo] Wiki page 'index' edited (Wiki change comment) by user1`, pl.(*MatrixPayloadUnsafe).FormattedBody) + assert.Equal(t, "[[test/repo](http://localhost:3000/test/repo)] Wiki page '[index](http://localhost:3000/test/repo/wiki/index)' edited (Wiki change comment) by [user1](https://try.gitea.io/user1)", pl.(*MatrixPayload).Body) + assert.Equal(t, `[test/repo] Wiki page 'index' edited (Wiki change comment) by user1`, pl.(*MatrixPayload).FormattedBody) p.Action = api.HookWikiDeleted pl, err = d.Wiki(p) require.NoError(t, err) require.NotNil(t, pl) - require.IsType(t, &MatrixPayloadUnsafe{}, pl) + require.IsType(t, &MatrixPayload{}, pl) - assert.Equal(t, "[[test/repo](http://localhost:3000/test/repo)] Wiki page '[index](http://localhost:3000/test/repo/wiki/index)' deleted by [user1](https://try.gitea.io/user1)", pl.(*MatrixPayloadUnsafe).Body) - assert.Equal(t, `[test/repo] Wiki page 'index' deleted by user1`, pl.(*MatrixPayloadUnsafe).FormattedBody) + assert.Equal(t, "[[test/repo](http://localhost:3000/test/repo)] Wiki page '[index](http://localhost:3000/test/repo/wiki/index)' deleted by [user1](https://try.gitea.io/user1)", pl.(*MatrixPayload).Body) + assert.Equal(t, `[test/repo] Wiki page 'index' deleted by user1`, pl.(*MatrixPayload).FormattedBody) }) t.Run("Release", func(t *testing.T) { p := pullReleaseTestPayload() - d := new(MatrixPayloadUnsafe) + d := new(MatrixPayload) pl, err := d.Release(p) require.NoError(t, err) require.NotNil(t, pl) - require.IsType(t, &MatrixPayloadUnsafe{}, pl) + require.IsType(t, &MatrixPayload{}, pl) - assert.Equal(t, "[[test/repo](http://localhost:3000/test/repo)] Release created: [v1.0](http://localhost:3000/test/repo/releases/tag/v1.0) by [user1](https://try.gitea.io/user1)", pl.(*MatrixPayloadUnsafe).Body) - assert.Equal(t, `[test/repo] Release created: v1.0 by user1`, pl.(*MatrixPayloadUnsafe).FormattedBody) + assert.Equal(t, "[[test/repo](http://localhost:3000/test/repo)] Release created: [v1.0](http://localhost:3000/test/repo/releases/tag/v1.0) by [user1](https://try.gitea.io/user1)", pl.(*MatrixPayload).Body) + assert.Equal(t, `[test/repo] Release created: v1.0 by user1`, pl.(*MatrixPayload).FormattedBody) }) } func TestMatrixJSONPayload(t *testing.T) { p := pushTestPayload() - pl, err := new(MatrixPayloadUnsafe).Push(p) + pl, err := new(MatrixPayload).Push(p) require.NoError(t, err) require.NotNil(t, pl) - require.IsType(t, &MatrixPayloadUnsafe{}, pl) + require.IsType(t, &MatrixPayload{}, pl) json, err := pl.JSONPayload() require.NoError(t, err) assert.NotEmpty(t, json) } -func TestMatrixHookRequest(t *testing.T) { - w := &webhook_model.Webhook{} - - h := &webhook_model.HookTask{ - PayloadContent: `{ - "body": "[[user1/test](http://localhost:3000/user1/test)] user1 pushed 1 commit to [master](http://localhost:3000/user1/test/src/branch/master):\n[5175ef2](http://localhost:3000/user1/test/commit/5175ef26201c58b035a3404b3fe02b4e8d436eee): Merge pull request 'Change readme.md' (#2) from add-matrix-webhook into master\n\nReviewed-on: http://localhost:3000/user1/test/pulls/2\n - user1", - "msgtype": "m.notice", - "format": "org.matrix.custom.html", - "formatted_body": "[\u003ca href=\"http://localhost:3000/user1/test\"\u003euser1/test\u003c/a\u003e] user1 pushed 1 commit to \u003ca href=\"http://localhost:3000/user1/test/src/branch/master\"\u003emaster\u003c/a\u003e:\u003cbr\u003e\u003ca href=\"http://localhost:3000/user1/test/commit/5175ef26201c58b035a3404b3fe02b4e8d436eee\"\u003e5175ef2\u003c/a\u003e: Merge pull request 'Change readme.md' (#2) from add-matrix-webhook into master\n\nReviewed-on: http://localhost:3000/user1/test/pulls/2\n - user1", - "io.gitea.commits": [ - { - "id": "5175ef26201c58b035a3404b3fe02b4e8d436eee", - "message": "Merge pull request 'Change readme.md' (#2) from add-matrix-webhook into master\n\nReviewed-on: http://localhost:3000/user1/test/pulls/2\n", - "url": "http://localhost:3000/user1/test/commit/5175ef26201c58b035a3404b3fe02b4e8d436eee", - "author": { - "name": "user1", - "email": "user@mail.com", - "username": "" - }, - "committer": { - "name": "user1", - "email": "user@mail.com", - "username": "" - }, - "verification": null, - "timestamp": "0001-01-01T00:00:00Z", - "added": null, - "removed": null, - "modified": null - } - ], - "access_token": "dummy_access_token" -}`, - } - - wantPayloadContent := `{ - "body": "[[user1/test](http://localhost:3000/user1/test)] user1 pushed 1 commit to [master](http://localhost:3000/user1/test/src/branch/master):\n[5175ef2](http://localhost:3000/user1/test/commit/5175ef26201c58b035a3404b3fe02b4e8d436eee): Merge pull request 'Change readme.md' (#2) from add-matrix-webhook into master\n\nReviewed-on: http://localhost:3000/user1/test/pulls/2\n - user1", - "msgtype": "m.notice", - "format": "org.matrix.custom.html", - "formatted_body": "[\u003ca href=\"http://localhost:3000/user1/test\"\u003euser1/test\u003c/a\u003e] user1 pushed 1 commit to \u003ca href=\"http://localhost:3000/user1/test/src/branch/master\"\u003emaster\u003c/a\u003e:\u003cbr\u003e\u003ca href=\"http://localhost:3000/user1/test/commit/5175ef26201c58b035a3404b3fe02b4e8d436eee\"\u003e5175ef2\u003c/a\u003e: Merge pull request 'Change readme.md' (#2) from add-matrix-webhook into master\n\nReviewed-on: http://localhost:3000/user1/test/pulls/2\n - user1", - "io.gitea.commits": [ - { - "id": "5175ef26201c58b035a3404b3fe02b4e8d436eee", - "message": "Merge pull request 'Change readme.md' (#2) from add-matrix-webhook into master\n\nReviewed-on: http://localhost:3000/user1/test/pulls/2\n", - "url": "http://localhost:3000/user1/test/commit/5175ef26201c58b035a3404b3fe02b4e8d436eee", - "author": { - "name": "user1", - "email": "user@mail.com", - "username": "" - }, - "committer": { - "name": "user1", - "email": "user@mail.com", - "username": "" - }, - "verification": null, - "timestamp": "0001-01-01T00:00:00Z", - "added": null, - "removed": null, - "modified": null - } - ] -}` - - req, err := getMatrixHookRequest(w, h) - require.NoError(t, err) - require.NotNil(t, req) - - 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 diff --git a/templates/repo/settings/webhook/matrix.tmpl b/templates/repo/settings/webhook/matrix.tmpl index 8edab870c..d3ab5588b 100644 --- a/templates/repo/settings/webhook/matrix.tmpl +++ b/templates/repo/settings/webhook/matrix.tmpl @@ -10,10 +10,6 @@ -
- - -
diff --git a/templates/repo/settings/webhook/settings.tmpl b/templates/repo/settings/webhook/settings.tmpl index 1543bef6c..42bc144b6 100644 --- a/templates/repo/settings/webhook/settings.tmpl +++ b/templates/repo/settings/webhook/settings.tmpl @@ -248,6 +248,15 @@ {{.locale.Tr "repo.settings.branch_filter_desc" | Str2html}}
+ +
+ + + {{if ne .HookType "matrix"}}{{/* Matrix doesn't make the authorization optional but it is implied by the help string, should be changed.*/}} + {{.locale.Tr "repo.settings.authorization_header_desc" "Bearer token123456, Basic YWxhZGRpbjpvcGVuc2VzYW1l" | Str2html}} + {{end}} +
+
diff --git a/templates/swagger/v1_json.tmpl b/templates/swagger/v1_json.tmpl index 94fb67ab4..229e21906 100644 --- a/templates/swagger/v1_json.tmpl +++ b/templates/swagger/v1_json.tmpl @@ -14448,6 +14448,10 @@ "default": false, "x-go-name": "Active" }, + "authorization_header": { + "type": "string", + "x-go-name": "AuthorizationHeader" + }, "branch_filter": { "type": "string", "x-go-name": "BranchFilter" @@ -15437,6 +15441,10 @@ "type": "boolean", "x-go-name": "Active" }, + "authorization_header": { + "type": "string", + "x-go-name": "AuthorizationHeader" + }, "branch_filter": { "type": "string", "x-go-name": "BranchFilter" @@ -16544,6 +16552,10 @@ "type": "boolean", "x-go-name": "Active" }, + "authorization_header": { + "type": "string", + "x-go-name": "AuthorizationHeader" + }, "config": { "type": "object", "additionalProperties": { diff --git a/tests/integration/api_repo_hook_test.go b/tests/integration/api_repo_hook_test.go new file mode 100644 index 000000000..e503834e1 --- /dev/null +++ b/tests/integration/api_repo_hook_test.go @@ -0,0 +1,47 @@ +// Copyright 2022 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 integration + +import ( + "fmt" + "net/http" + "testing" + + repo_model "code.gitea.io/gitea/models/repo" + "code.gitea.io/gitea/models/unittest" + user_model "code.gitea.io/gitea/models/user" + api "code.gitea.io/gitea/modules/structs" + "code.gitea.io/gitea/tests" + + "github.com/stretchr/testify/assert" +) + +func TestAPICreateHook(t *testing.T) { + defer tests.PrepareTestEnv(t)() + + repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 37}) + owner := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: repo.OwnerID}) + + // user1 is an admin user + session := loginUser(t, "user1") + token := getTokenForLoggedInUser(t, session) + completeURL := func(lastSegment string) string { + return fmt.Sprintf("/api/v1/repos/%s/%s/%s?token=%s", owner.Name, repo.Name, lastSegment, token) + } + req := NewRequestWithJSON(t, "POST", completeURL("hooks"), api.CreateHookOption{ + Type: "gitea", + Config: api.CreateHookOptionConfig{ + "content_type": "json", + "url": "http://example.com/", + }, + AuthorizationHeader: "Bearer s3cr3t", + }) + resp := MakeRequest(t, req, http.StatusCreated) + + var apiHook *api.Hook + DecodeJSON(t, resp, &apiHook) + assert.Equal(t, "http://example.com/", apiHook.Config["url"]) + assert.Equal(t, "Bearer s3cr3t", apiHook.AuthorizationHeader) +}