From 615001d746000cc48a4e94c33af8aabc168418a8 Mon Sep 17 00:00:00 2001 From: Norwin Date: Sun, 27 Jun 2021 17:21:19 +0200 Subject: [PATCH 1/4] review comments: break-word for long file names (#16272) * review comments: break-word for long file names fixes #16248 Co-authored-by: zeripath --- templates/repo/issue/view_content/comments.tmpl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/templates/repo/issue/view_content/comments.tmpl b/templates/repo/issue/view_content/comments.tmpl index de31430ce0599..dcc0401c99962 100644 --- a/templates/repo/issue/view_content/comments.tmpl +++ b/templates/repo/issue/view_content/comments.tmpl @@ -462,7 +462,7 @@ {{ range $filename, $lines := .Review.CodeComments}} {{range $line, $comms := $lines}}
-
+
{{$invalid := (index $comms 0).Invalidated}} {{$resolved := (index $comms 0).IsResolved}} {{$resolveDoer := (index $comms 0).ResolveDoer}} From 2a98ec1c3cd2f8396f3b5148fc8c796802f9c236 Mon Sep 17 00:00:00 2001 From: Jimmy Praet Date: Sun, 27 Jun 2021 19:35:31 +0200 Subject: [PATCH 2/4] Add jpraet to MAINTAINERS (#16274) --- MAINTAINERS | 1 + 1 file changed, 1 insertion(+) diff --git a/MAINTAINERS b/MAINTAINERS index 0c94df87b9530..f43ba5ab99065 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -43,3 +43,4 @@ Kyle Dumont (@kdumontnu) Patrick Schratz (@pat-s) Janis Estelmann (@KN4CK3R) Steven Kriegler (@justusbunsi) +Jimmy Praet (@jpraet) From 0b27b93728fd3cf2ecc82ac6a2b5859270543ef2 Mon Sep 17 00:00:00 2001 From: 6543 <6543@obermui.de> Date: Sun, 27 Jun 2021 20:47:35 +0200 Subject: [PATCH 3/4] Make allowed Visiblity modes configurable for Users (#16271) Now that #16069 is merged, some sites may wish to enforce that users are all public, limited or private, and/or disallow users from becoming private. This PR adds functionality and settings to constrain a user's ability to change their visibility. Co-authored-by: zeripath --- custom/conf/app.example.ini | 3 + .../doc/advanced/config-cheat-sheet.en-us.md | 1 + models/user.go | 64 ++++++++++++------- models/user_test.go | 22 +++++++ modules/setting/service.go | 34 +++++++++- routers/web/admin/users.go | 2 + routers/web/admin/users_test.go | 4 -- routers/web/user/setting/profile.go | 1 + templates/admin/user/edit.tmpl | 30 ++++----- templates/admin/user/new.tmpl | 18 ++++-- templates/user/settings/profile.tmpl | 30 ++++----- 11 files changed, 146 insertions(+), 63 deletions(-) diff --git a/custom/conf/app.example.ini b/custom/conf/app.example.ini index e7fe9206ed960..33ff7a62c56b3 100644 --- a/custom/conf/app.example.ini +++ b/custom/conf/app.example.ini @@ -656,6 +656,9 @@ PATH = ;; Public is for users visible for everyone ;DEFAULT_USER_VISIBILITY = public ;; +;; Set whitch visibibilty modes a user can have +;ALLOWED_USER_VISIBILITY_MODES = public,limited,private +;; ;; Either "public", "limited" or "private", default is "public" ;; Limited is for organizations visible only to signed users ;; Private is for organizations visible only to members of the organization diff --git a/docs/content/doc/advanced/config-cheat-sheet.en-us.md b/docs/content/doc/advanced/config-cheat-sheet.en-us.md index 21359dcab1445..d1d47bc89301d 100644 --- a/docs/content/doc/advanced/config-cheat-sheet.en-us.md +++ b/docs/content/doc/advanced/config-cheat-sheet.en-us.md @@ -513,6 +513,7 @@ relation to port exhaustion. - `AUTO_WATCH_NEW_REPOS`: **true**: Enable this to let all organisation users watch new repos when they are created - `AUTO_WATCH_ON_CHANGES`: **false**: Enable this to make users watch a repository after their first commit to it - `DEFAULT_USER_VISIBILITY`: **public**: Set default visibility mode for users, either "public", "limited" or "private". +- `ALLOWED_USER_VISIBILITY_MODES`: **public,limited,private**: Set whitch visibibilty modes a user can have - `DEFAULT_ORG_VISIBILITY`: **public**: Set default visibility mode for organisations, either "public", "limited" or "private". - `DEFAULT_ORG_MEMBER_VISIBLE`: **false** True will make the membership of the users visible when added to the organisation. - `ALLOW_ONLY_INTERNAL_REGISTRATION`: **false** Set to true to force registration only via gitea. diff --git a/models/user.go b/models/user.go index 221c840a7f7f3..47d24aefd6aa4 100644 --- a/models/user.go +++ b/models/user.go @@ -863,26 +863,36 @@ func CreateUser(u *User, overwriteDefault ...*CreateUserOverwriteOptions) (err e return err } + // set system defaults + u.KeepEmailPrivate = setting.Service.DefaultKeepEmailPrivate + u.Visibility = setting.Service.DefaultUserVisibilityMode + u.AllowCreateOrganization = setting.Service.DefaultAllowCreateOrganization && !setting.Admin.DisableRegularOrgCreation + u.EmailNotificationsPreference = setting.Admin.DefaultEmailNotification + u.MaxRepoCreation = -1 + u.Theme = setting.UI.DefaultTheme + + // overwrite defaults if set + if len(overwriteDefault) != 0 && overwriteDefault[0] != nil { + u.Visibility = overwriteDefault[0].Visibility + } + sess := x.NewSession() defer sess.Close() if err = sess.Begin(); err != nil { return err } - isExist, err := isUserExist(sess, 0, u.Name) - if err != nil { - return err - } else if isExist { - return ErrUserAlreadyExist{u.Name} - } + // validate data - if err = deleteUserRedirect(sess, u.Name); err != nil { + if err := validateUser(u); err != nil { return err } - u.Email = strings.ToLower(u.Email) - if err = ValidateEmail(u.Email); err != nil { + isExist, err := isUserExist(sess, 0, u.Name) + if err != nil { return err + } else if isExist { + return ErrUserAlreadyExist{u.Name} } isExist, err = isEmailUsed(sess, u.Email) @@ -892,6 +902,8 @@ func CreateUser(u *User, overwriteDefault ...*CreateUserOverwriteOptions) (err e return ErrEmailAlreadyUsed{u.Email} } + // prepare for database + u.LowerName = strings.ToLower(u.Name) u.AvatarEmail = u.Email if u.Rands, err = GetUserSalt(); err != nil { @@ -901,16 +913,10 @@ func CreateUser(u *User, overwriteDefault ...*CreateUserOverwriteOptions) (err e return err } - // set system defaults - u.KeepEmailPrivate = setting.Service.DefaultKeepEmailPrivate - u.Visibility = setting.Service.DefaultUserVisibilityMode - u.AllowCreateOrganization = setting.Service.DefaultAllowCreateOrganization && !setting.Admin.DisableRegularOrgCreation - u.EmailNotificationsPreference = setting.Admin.DefaultEmailNotification - u.MaxRepoCreation = -1 - u.Theme = setting.UI.DefaultTheme - // overwrite defaults if set - if len(overwriteDefault) != 0 && overwriteDefault[0] != nil { - u.Visibility = overwriteDefault[0].Visibility + // save changes to database + + if err = deleteUserRedirect(sess, u.Name); err != nil { + return err } if _, err = sess.Insert(u); err != nil { @@ -1056,12 +1062,22 @@ func checkDupEmail(e Engine, u *User) error { return nil } -func updateUser(e Engine, u *User) (err error) { +// validateUser check if user is valide to insert / update into database +func validateUser(u *User) error { + if !setting.Service.AllowedUserVisibilityModesSlice.IsAllowedVisibility(u.Visibility) { + return fmt.Errorf("visibility Mode not allowed: %s", u.Visibility.String()) + } + u.Email = strings.ToLower(u.Email) - if err = ValidateEmail(u.Email); err != nil { + return ValidateEmail(u.Email) +} + +func updateUser(e Engine, u *User) error { + if err := validateUser(u); err != nil { return err } - _, err = e.ID(u.ID).AllCols().Update(u) + + _, err := e.ID(u.ID).AllCols().Update(u) return err } @@ -1076,6 +1092,10 @@ func UpdateUserCols(u *User, cols ...string) error { } func updateUserCols(e Engine, u *User, cols ...string) error { + if err := validateUser(u); err != nil { + return err + } + _, err := e.ID(u.ID).Cols(cols...).Update(u) return err } diff --git a/models/user_test.go b/models/user_test.go index 39a1b3c989c05..34c465c586498 100644 --- a/models/user_test.go +++ b/models/user_test.go @@ -11,6 +11,7 @@ import ( "testing" "code.gitea.io/gitea/modules/setting" + "code.gitea.io/gitea/modules/structs" "code.gitea.io/gitea/modules/util" "github.com/stretchr/testify/assert" @@ -189,6 +190,7 @@ func TestDeleteUser(t *testing.T) { func TestEmailNotificationPreferences(t *testing.T) { assert.NoError(t, PrepareTestDatabase()) + for _, test := range []struct { expected string userID int64 @@ -467,3 +469,23 @@ ssh-dss AAAAB3NzaC1kc3MAAACBAOChCC7lf6Uo9n7BmZ6M8St19PZf4Tn59NriyboW2x/DZuYAz3ib } } } + +func TestUpdateUser(t *testing.T) { + assert.NoError(t, PrepareTestDatabase()) + user := AssertExistsAndLoadBean(t, &User{ID: 2}).(*User) + + user.KeepActivityPrivate = true + assert.NoError(t, UpdateUser(user)) + user = AssertExistsAndLoadBean(t, &User{ID: 2}).(*User) + assert.True(t, user.KeepActivityPrivate) + + setting.Service.AllowedUserVisibilityModesSlice = []bool{true, false, false} + user.KeepActivityPrivate = false + user.Visibility = structs.VisibleTypePrivate + assert.Error(t, UpdateUser(user)) + user = AssertExistsAndLoadBean(t, &User{ID: 2}).(*User) + assert.True(t, user.KeepActivityPrivate) + + user.Email = "no mail@mail.org" + assert.Error(t, UpdateUser(user)) +} diff --git a/modules/setting/service.go b/modules/setting/service.go index 3f689212f373c..dbabfb8400ad0 100644 --- a/modules/setting/service.go +++ b/modules/setting/service.go @@ -14,9 +14,11 @@ import ( ) // Service settings -var Service struct { +var Service = struct { DefaultUserVisibility string DefaultUserVisibilityMode structs.VisibleType + AllowedUserVisibilityModes []string + AllowedUserVisibilityModesSlice AllowedVisibility `ini:"-"` DefaultOrgVisibility string DefaultOrgVisibilityMode structs.VisibleType ActiveCodeLives int @@ -71,6 +73,29 @@ var Service struct { RequireSigninView bool `ini:"REQUIRE_SIGNIN_VIEW"` DisableUsersPage bool `ini:"DISABLE_USERS_PAGE"` } `ini:"service.explore"` +}{ + AllowedUserVisibilityModesSlice: []bool{true, true, true}, +} + +// AllowedVisibility store in a 3 item bool array what is allowed +type AllowedVisibility []bool + +// IsAllowedVisibility check if a AllowedVisibility allow a specific VisibleType +func (a AllowedVisibility) IsAllowedVisibility(t structs.VisibleType) bool { + if int(t) >= len(a) { + return false + } + return a[t] +} + +// ToVisibleTypeSlice convert a AllowedVisibility into a VisibleType slice +func (a AllowedVisibility) ToVisibleTypeSlice() (result []structs.VisibleType) { + for i, v := range a { + if v { + result = append(result, structs.VisibleType(i)) + } + } + return } func newService() { @@ -122,6 +147,13 @@ func newService() { Service.AutoWatchOnChanges = sec.Key("AUTO_WATCH_ON_CHANGES").MustBool(false) Service.DefaultUserVisibility = sec.Key("DEFAULT_USER_VISIBILITY").In("public", structs.ExtractKeysFromMapString(structs.VisibilityModes)) Service.DefaultUserVisibilityMode = structs.VisibilityModes[Service.DefaultUserVisibility] + Service.AllowedUserVisibilityModes = sec.Key("ALLOWED_USER_VISIBILITY_MODES").Strings(",") + if len(Service.AllowedUserVisibilityModes) != 0 { + Service.AllowedUserVisibilityModesSlice = []bool{false, false, false} + for _, sMode := range Service.AllowedUserVisibilityModes { + Service.AllowedUserVisibilityModesSlice[structs.VisibilityModes[sMode]] = true + } + } Service.DefaultOrgVisibility = sec.Key("DEFAULT_ORG_VISIBILITY").In("public", structs.ExtractKeysFromMapString(structs.VisibilityModes)) Service.DefaultOrgVisibilityMode = structs.VisibilityModes[Service.DefaultOrgVisibility] Service.DefaultOrgMemberVisible = sec.Key("DEFAULT_ORG_MEMBER_VISIBLE").MustBool() diff --git a/routers/web/admin/users.go b/routers/web/admin/users.go index dc2a97e5261d1..e1903ab1dfafc 100644 --- a/routers/web/admin/users.go +++ b/routers/web/admin/users.go @@ -52,6 +52,7 @@ func NewUser(ctx *context.Context) { ctx.Data["PageIsAdmin"] = true ctx.Data["PageIsAdminUsers"] = true ctx.Data["DefaultUserVisibilityMode"] = setting.Service.DefaultUserVisibilityMode + ctx.Data["AllowedUserVisibilityModes"] = setting.Service.AllowedUserVisibilityModesSlice.ToVisibleTypeSlice() ctx.Data["login_type"] = "0-0" @@ -211,6 +212,7 @@ func EditUser(ctx *context.Context) { ctx.Data["PageIsAdminUsers"] = true ctx.Data["DisableRegularOrgCreation"] = setting.Admin.DisableRegularOrgCreation ctx.Data["DisableMigrations"] = setting.Repository.DisableMigrations + ctx.Data["AllowedUserVisibilityModes"] = setting.Service.AllowedUserVisibilityModesSlice.ToVisibleTypeSlice() prepareUserInfo(ctx) if ctx.Written() { diff --git a/routers/web/admin/users_test.go b/routers/web/admin/users_test.go index 17c5a309b4d93..5ce20d8fa777c 100644 --- a/routers/web/admin/users_test.go +++ b/routers/web/admin/users_test.go @@ -56,7 +56,6 @@ func TestNewUserPost_MustChangePassword(t *testing.T) { } func TestNewUserPost_MustChangePasswordFalse(t *testing.T) { - models.PrepareTestEnv(t) ctx := test.MockContext(t, "admin/users/new") @@ -94,7 +93,6 @@ func TestNewUserPost_MustChangePasswordFalse(t *testing.T) { } func TestNewUserPost_InvalidEmail(t *testing.T) { - models.PrepareTestEnv(t) ctx := test.MockContext(t, "admin/users/new") @@ -125,7 +123,6 @@ func TestNewUserPost_InvalidEmail(t *testing.T) { } func TestNewUserPost_VisiblityDefaultPublic(t *testing.T) { - models.PrepareTestEnv(t) ctx := test.MockContext(t, "admin/users/new") @@ -164,7 +161,6 @@ func TestNewUserPost_VisiblityDefaultPublic(t *testing.T) { } func TestNewUserPost_VisibilityPrivate(t *testing.T) { - models.PrepareTestEnv(t) ctx := test.MockContext(t, "admin/users/new") diff --git a/routers/web/user/setting/profile.go b/routers/web/user/setting/profile.go index 463c4ec2038c8..682f9205784e3 100644 --- a/routers/web/user/setting/profile.go +++ b/routers/web/user/setting/profile.go @@ -38,6 +38,7 @@ const ( func Profile(ctx *context.Context) { ctx.Data["Title"] = ctx.Tr("settings") ctx.Data["PageIsSettingsProfile"] = true + ctx.Data["AllowedUserVisibilityModes"] = setting.Service.AllowedUserVisibilityModesSlice.ToVisibleTypeSlice() ctx.HTML(http.StatusOK, tplSettingsProfile) } diff --git a/templates/admin/user/edit.tmpl b/templates/admin/user/edit.tmpl index dba24d9837df0..5e5bc75c9695c 100644 --- a/templates/admin/user/edit.tmpl +++ b/templates/admin/user/edit.tmpl @@ -32,25 +32,25 @@
- {{if .User.Visibility.IsPublic}} - - {{end}} - {{if .User.Visibility.IsLimited}} - - {{end}} - {{if .User.Visibility.IsPrivate}} - - {{end}} + {{if .User.Visibility.IsPublic}}{{end}} + {{if .User.Visibility.IsLimited}}{{end}} + {{if .User.Visibility.IsPrivate}}{{end}}
- {{if .User.Visibility.IsPublic}}{{.i18n.Tr "settings.visibility.public"}}{{end}} - {{if .User.Visibility.IsLimited}}{{.i18n.Tr "settings.visibility.limited"}}{{end}} - {{if .User.Visibility.IsPrivate}}{{.i18n.Tr "settings.visibility.private"}}{{end}} + {{if .User.Visibility.IsPublic}}{{.i18n.Tr "settings.visibility.public"}}{{end}} + {{if .User.Visibility.IsLimited}}{{.i18n.Tr "settings.visibility.limited"}}{{end}} + {{if .User.Visibility.IsPrivate}}{{.i18n.Tr "settings.visibility.private"}}{{end}}
{{svg "octicon-triangle-down" 14 "dropdown icon"}}
-
{{.i18n.Tr "settings.visibility.public"}}
-
{{.i18n.Tr "settings.visibility.limited"}}
-
{{.i18n.Tr "settings.visibility.private"}}
+ {{range $mode := .AllowedUserVisibilityModes}} + {{if $mode.IsPublic}} +
{{$.i18n.Tr "settings.visibility.public"}}
+ {{else if $mode.IsLimited}} +
{{$.i18n.Tr "settings.visibility.limited"}}
+ {{else if $mode.IsPrivate}} +
{{$.i18n.Tr "settings.visibility.private"}}
+ {{end}} + {{end}}
diff --git a/templates/admin/user/new.tmpl b/templates/admin/user/new.tmpl index 2e391725353a7..a433c5a7cc865 100644 --- a/templates/admin/user/new.tmpl +++ b/templates/admin/user/new.tmpl @@ -30,15 +30,21 @@
- {{if .DefaultUserVisibilityMode.IsPublic}}{{.i18n.Tr "settings.visibility.public"}}{{end}} - {{if .DefaultUserVisibilityMode.IsLimited}}{{.i18n.Tr "settings.visibility.limited"}}{{end}} - {{if .DefaultUserVisibilityMode.IsPrivate}}{{.i18n.Tr "settings.visibility.private"}}{{end}} + {{if .DefaultUserVisibilityMode.IsPublic}}{{.i18n.Tr "settings.visibility.public"}}{{end}} + {{if .DefaultUserVisibilityMode.IsLimited}}{{.i18n.Tr "settings.visibility.limited"}}{{end}} + {{if .DefaultUserVisibilityMode.IsPrivate}}{{.i18n.Tr "settings.visibility.private"}}{{end}}
{{svg "octicon-triangle-down" 14 "dropdown icon"}}
-
{{.i18n.Tr "settings.visibility.public"}}
-
{{.i18n.Tr "settings.visibility.limited"}}
-
{{.i18n.Tr "settings.visibility.private"}}
+ {{range $mode := .AllowedUserVisibilityModes}} + {{if $mode.IsPublic}} +
{{$.i18n.Tr "settings.visibility.public"}}
+ {{else if $mode.IsLimited}} +
{{$.i18n.Tr "settings.visibility.limited"}}
+ {{else if $mode.IsPrivate}} +
{{$.i18n.Tr "settings.visibility.private"}}
+ {{end}} + {{end}}
diff --git a/templates/user/settings/profile.tmpl b/templates/user/settings/profile.tmpl index 4b860049d8323..1f1585a78773b 100644 --- a/templates/user/settings/profile.tmpl +++ b/templates/user/settings/profile.tmpl @@ -71,25 +71,25 @@
- {{if .SignedUser.Visibility.IsPublic}} - - {{end}} - {{if .SignedUser.Visibility.IsLimited}} - - {{end}} - {{if .SignedUser.Visibility.IsPrivate}} - - {{end}} + {{if .SignedUser.Visibility.IsPublic}}{{end}} + {{if .SignedUser.Visibility.IsLimited}}{{end}} + {{if .SignedUser.Visibility.IsPrivate}}{{end}}
- {{if .SignedUser.Visibility.IsPublic}}{{.i18n.Tr "settings.visibility.public"}}{{end}} - {{if .SignedUser.Visibility.IsLimited}}{{.i18n.Tr "settings.visibility.limited"}}{{end}} - {{if .SignedUser.Visibility.IsPrivate}}{{.i18n.Tr "settings.visibility.private"}}{{end}} + {{if .SignedUser.Visibility.IsPublic}}{{.i18n.Tr "settings.visibility.public"}}{{end}} + {{if .SignedUser.Visibility.IsLimited}}{{.i18n.Tr "settings.visibility.limited"}}{{end}} + {{if .SignedUser.Visibility.IsPrivate}}{{.i18n.Tr "settings.visibility.private"}}{{end}}
{{svg "octicon-triangle-down" 14 "dropdown icon"}}
-
{{.i18n.Tr "settings.visibility.public"}}
-
{{.i18n.Tr "settings.visibility.limited"}}
-
{{.i18n.Tr "settings.visibility.private"}}
+ {{range $mode := .AllowedUserVisibilityModes}} + {{if $mode.IsPublic}} +
{{$.i18n.Tr "settings.visibility.public"}}
+ {{else if $mode.IsLimited}} +
{{$.i18n.Tr "settings.visibility.limited"}}
+ {{else if $mode.IsPrivate}} +
{{$.i18n.Tr "settings.visibility.private"}}
+ {{end}} + {{end}}
From 9b1b4b543358c212a3da2b480d361d0c1375b279 Mon Sep 17 00:00:00 2001 From: KN4CK3R Date: Sun, 27 Jun 2021 21:21:09 +0200 Subject: [PATCH 4/4] Refactor Webhook + Add X-Hub-Signature (#16176) This PR removes multiple unneeded fields from the `HookTask` struct and adds the two headers `X-Hub-Signature` and `X-Hub-Signature-256`. ## :warning: BREAKING :warning: * The `Secret` field is no longer passed as part of the payload. * "Breaking" change (or fix?): The webhook history shows the real called url and not the url registered in the webhook (`deliver.go`@129). Close #16115 Fixes #7788 Fixes #11755 Co-authored-by: zeripath --- models/migrations/migrations.go | 2 + models/migrations/v187.go | 46 +++++++++++++++ models/webhook.go | 52 ++++++++--------- models/webhook_test.go | 14 ----- modules/structs/hook.go | 55 ------------------ routers/api/v1/utils/hook.go | 2 +- routers/web/repo/webhook.go | 2 +- services/webhook/deliver.go | 61 +++++++++++++------- services/webhook/dingtalk.go | 3 - services/webhook/discord.go | 3 - services/webhook/feishu.go | 3 - services/webhook/matrix.go | 9 +-- services/webhook/matrix_test.go | 4 +- services/webhook/msteams.go | 3 - services/webhook/slack.go | 3 - services/webhook/telegram.go | 3 - services/webhook/webhook.go | 40 +++---------- templates/repo/settings/webhook/history.tmpl | 4 +- 18 files changed, 130 insertions(+), 179 deletions(-) create mode 100644 models/migrations/v187.go diff --git a/models/migrations/migrations.go b/models/migrations/migrations.go index 978ba6368f958..0e8b0d0e3db5f 100644 --- a/models/migrations/migrations.go +++ b/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 diff --git a/models/migrations/v187.go b/models/migrations/v187.go new file mode 100644 index 0000000000000..627423717a308 --- /dev/null +++ b/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() +} diff --git a/models/webhook.go b/models/webhook.go index 24510cc6f757b..29cfcf6ed4f60 100644 --- a/models/webhook.go +++ b/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:"-"` diff --git a/models/webhook_test.go b/models/webhook_test.go index 88b2d40a39203..cab44a120e3e3 100644 --- a/models/webhook_test.go +++ b/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(), diff --git a/modules/structs/hook.go b/modules/structs/hook.go index 693820b57d3e9..e4ec99df40d77 100644 --- a/modules/structs/hook.go +++ b/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,7 +123,6 @@ var ( // CreatePayload FIXME type CreatePayload struct { - Secret string `json:"secret"` Sha string `json:"sha"` Ref string `json:"ref"` RefType string `json:"ref_type"` @@ -132,11 +130,6 @@ type CreatePayload struct { 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,7 +174,6 @@ 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"` @@ -189,11 +181,6 @@ type DeletePayload struct { 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 diff --git a/routers/api/v1/utils/hook.go b/routers/api/v1/utils/hook.go index c7af40dcf056e..5f2be65a29d99 100644 --- a/routers/api/v1/utils/hook.go +++ b/routers/api/v1/utils/hook.go @@ -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"] diff --git a/routers/web/repo/webhook.go b/routers/web/repo/webhook.go index fe16d249eb0c9..ab254dbe46cc1 100644 --- a/routers/web/repo/webhook.go +++ b/routers/web/repo/webhook.go @@ -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 diff --git a/services/webhook/deliver.go b/services/webhook/deliver.go index a417a9e846d49..8243fde1bb743 100644 --- a/services/webhook/deliver.go +++ b/services/webhook/deliver.go @@ -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" @@ -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 } @@ -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 } @@ -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, ",") @@ -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 { diff --git a/services/webhook/dingtalk.go b/services/webhook/dingtalk.go index d781b8c87dbf6..49e161ea57b5e 100644 --- a/services/webhook/dingtalk.go +++ b/services/webhook/dingtalk.go @@ -25,9 +25,6 @@ var ( _ PayloadConvertor = &DingtalkPayload{} ) -// SetSecret sets the dingtalk secret -func (d *DingtalkPayload) SetSecret(_ string) {} - // JSONPayload Marshals the DingtalkPayload to json func (d *DingtalkPayload) JSONPayload() ([]byte, error) { json := jsoniter.ConfigCompatibleWithStandardLibrary diff --git a/services/webhook/discord.go b/services/webhook/discord.go index 378d9ff725db6..ea3879f1980d0 100644 --- a/services/webhook/discord.go +++ b/services/webhook/discord.go @@ -97,9 +97,6 @@ var ( redColor = color("ff3232") ) -// SetSecret sets the discord secret -func (d *DiscordPayload) SetSecret(_ string) {} - // JSONPayload Marshals the DiscordPayload to json func (d *DiscordPayload) JSONPayload() ([]byte, error) { json := jsoniter.ConfigCompatibleWithStandardLibrary diff --git a/services/webhook/feishu.go b/services/webhook/feishu.go index 5c80efb820db7..b280e67759ce5 100644 --- a/services/webhook/feishu.go +++ b/services/webhook/feishu.go @@ -35,9 +35,6 @@ func newFeishuTextPayload(text string) *FeishuPayload { } } -// SetSecret sets the Feishu secret -func (f *FeishuPayload) SetSecret(_ string) {} - // JSONPayload Marshals the FeishuPayload to json func (f *FeishuPayload) JSONPayload() ([]byte, error) { json := jsoniter.ConfigCompatibleWithStandardLibrary diff --git a/services/webhook/matrix.go b/services/webhook/matrix.go index 1658dd4b44e97..6fca67ca84f97 100644 --- a/services/webhook/matrix.go +++ b/services/webhook/matrix.go @@ -76,9 +76,6 @@ type MatrixPayloadSafe struct { Commits []*api.PayloadCommit `json:"io.gitea.commits,omitempty"` } -// SetSecret sets the Matrix secret -func (m *MatrixPayloadUnsafe) SetSecret(_ string) {} - // JSONPayload Marshals the MatrixPayloadUnsafe to json func (m *MatrixPayloadUnsafe) JSONPayload() ([]byte, error) { json := jsoniter.ConfigCompatibleWithStandardLibrary @@ -263,7 +260,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 { @@ -288,9 +285,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 } diff --git a/services/webhook/matrix_test.go b/services/webhook/matrix_test.go index 7b10e21cfa4a3..451dff69495f4 100644 --- a/services/webhook/matrix_test.go +++ b/services/webhook/matrix_test.go @@ -184,6 +184,8 @@ func TestMatrixJSONPayload(t *testing.T) { } func TestMatrixHookRequest(t *testing.T) { + w := &models.Webhook{} + h := &models.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", @@ -245,7 +247,7 @@ func TestMatrixHookRequest(t *testing.T) { ] }` - req, err := getMatrixHookRequest(h) + req, err := getMatrixHookRequest(w, h) require.NoError(t, err) require.NotNil(t, req) diff --git a/services/webhook/msteams.go b/services/webhook/msteams.go index 51bb28a36181d..035dbc1c4cf23 100644 --- a/services/webhook/msteams.go +++ b/services/webhook/msteams.go @@ -55,9 +55,6 @@ type ( } ) -// SetSecret sets the MSTeams secret -func (m *MSTeamsPayload) SetSecret(_ string) {} - // JSONPayload Marshals the MSTeamsPayload to json func (m *MSTeamsPayload) JSONPayload() ([]byte, error) { json := jsoniter.ConfigCompatibleWithStandardLibrary diff --git a/services/webhook/slack.go b/services/webhook/slack.go index 1de9c9c37c70c..f522ca35f2f0c 100644 --- a/services/webhook/slack.go +++ b/services/webhook/slack.go @@ -56,9 +56,6 @@ type SlackAttachment struct { Text string `json:"text"` } -// SetSecret sets the slack secret -func (s *SlackPayload) SetSecret(_ string) {} - // JSONPayload Marshals the SlackPayload to json func (s *SlackPayload) JSONPayload() ([]byte, error) { json := jsoniter.ConfigCompatibleWithStandardLibrary diff --git a/services/webhook/telegram.go b/services/webhook/telegram.go index f71352141dee2..4c4230759d318 100644 --- a/services/webhook/telegram.go +++ b/services/webhook/telegram.go @@ -45,9 +45,6 @@ var ( _ PayloadConvertor = &TelegramPayload{} ) -// SetSecret sets the telegram secret -func (t *TelegramPayload) SetSecret(_ string) {} - // JSONPayload Marshals the TelegramPayload to json func (t *TelegramPayload) JSONPayload() ([]byte, error) { t.ParseMode = "HTML" diff --git a/services/webhook/webhook.go b/services/webhook/webhook.go index cc79ec15d1d7a..d094a7754bac8 100644 --- a/services/webhook/webhook.go +++ b/services/webhook/webhook.go @@ -5,9 +5,6 @@ package webhook import ( - "crypto/hmac" - "crypto/sha256" - "encoding/hex" "fmt" "strings" @@ -21,12 +18,12 @@ import ( ) type webhook struct { - name models.HookTaskType + name models.HookType payloadCreator func(p api.Payloader, event models.HookEventType, meta string) (api.Payloader, error) } var ( - webhooks = map[models.HookTaskType]*webhook{ + webhooks = map[models.HookType]*webhook{ models.SLACK: { name: models.SLACK, payloadCreator: GetSlackPayload, @@ -60,7 +57,7 @@ var ( // RegisterWebhook registers a webhook func RegisterWebhook(name string, webhook *webhook) { - webhooks[models.HookTaskType(name)] = webhook + webhooks[models.HookType(name)] = webhook } // IsValidHookTaskType returns true if a webhook registered @@ -68,7 +65,7 @@ func IsValidHookTaskType(name string) bool { if name == models.GITEA || name == models.GOGS { return true } - _, ok := webhooks[models.HookTaskType(name)] + _, ok := webhooks[models.HookType(name)] return ok } @@ -161,35 +158,14 @@ func prepareWebhook(w *models.Webhook, repo *models.Repository, event models.Hoo return fmt.Errorf("create payload for %s[%s]: %v", w.Type, event, err) } } else { - p.SetSecret(w.Secret) payloader = p } - var signature string - if len(w.Secret) > 0 { - data, err := payloader.JSONPayload() - if err != nil { - log.Error("prepareWebhooks.JSONPayload: %v", err) - } - sig := hmac.New(sha256.New, []byte(w.Secret)) - _, err = sig.Write(data) - if err != nil { - log.Error("prepareWebhooks.sigWrite: %v", err) - } - signature = hex.EncodeToString(sig.Sum(nil)) - } - if err = models.CreateHookTask(&models.HookTask{ - RepoID: repo.ID, - HookID: w.ID, - Typ: w.Type, - URL: w.URL, - Signature: signature, - Payloader: payloader, - HTTPMethod: w.HTTPMethod, - ContentType: w.ContentType, - EventType: event, - IsSSL: w.IsSSL, + RepoID: repo.ID, + HookID: w.ID, + Payloader: payloader, + EventType: event, }); err != nil { return fmt.Errorf("CreateHookTask: %v", err) } diff --git a/templates/repo/settings/webhook/history.tmpl b/templates/repo/settings/webhook/history.tmpl index cf4884531db7a..d2fe68738f5f4 100644 --- a/templates/repo/settings/webhook/history.tmpl +++ b/templates/repo/settings/webhook/history.tmpl @@ -44,8 +44,8 @@
{{if .RequestInfo}}
{{$.i18n.Tr "repo.settings.webhook.headers"}}
- 
Request URL: {{.URL}}
-Request method: {{if .HTTPMethod}}{{.HTTPMethod}}{{else}}POST{{end}}
+								
Request URL: {{.RequestInfo.URL}}
+Request method: {{if .RequestInfo.HTTPMethod}}{{.RequestInfo.HTTPMethod}}{{else}}POST{{end}}
 {{ range $key, $val := .RequestInfo.Headers }}{{$key}}: {{$val}}
 {{end}}

 								
{{$.i18n.Tr "repo.settings.webhook.payload"}}