From d1ce06b22ec5ec3ecbfb8cbf9e8cee7b307ea83c Mon Sep 17 00:00:00 2001 From: Lanre Adelowo Date: Fri, 24 Aug 2018 20:01:21 +0100 Subject: [PATCH 1/7] validate slack channel name to make sure it cannot be empty and contain just an # --- modules/auth/repo_form.go | 5 +++++ options/locale/locale_en-US.ini | 1 + routers/api/v1/utils/hook.go | 10 +++++++++- routers/repo/webhook.go | 16 ++++++++++++++-- routers/utils/utils.go | 19 +++++++++++++++++++ routers/utils/utils_test.go | 17 +++++++++++++++++ 6 files changed, 65 insertions(+), 3 deletions(-) diff --git a/modules/auth/repo_form.go b/modules/auth/repo_form.go index 3c7940afce756..b80042224218e 100644 --- a/modules/auth/repo_form.go +++ b/modules/auth/repo_form.go @@ -10,6 +10,7 @@ import ( "strings" "code.gitea.io/gitea/models" + "code.gitea.io/gitea/routers/utils" "github.com/Unknwon/com" "github.com/go-macaron/binding" "gopkg.in/macaron.v1" @@ -225,6 +226,10 @@ func (f *NewSlackHookForm) Validate(ctx *macaron.Context, errs binding.Errors) b return validate(errs, ctx.Data, f, ctx.Locale) } +func (f NewSlackHookForm) HasInvalidChannel() bool { + return !utils.IsValidSlackChannel(f.Channel) +} + // NewDiscordHookForm form for creating discord hook type NewDiscordHookForm struct { PayloadURL string `binding:"Required;ValidUrl"` diff --git a/options/locale/locale_en-US.ini b/options/locale/locale_en-US.ini index 4b2b20204f6a4..7d52469ce13c8 100644 --- a/options/locale/locale_en-US.ini +++ b/options/locale/locale_en-US.ini @@ -1043,6 +1043,7 @@ settings.search_user_placeholder = Search user… settings.org_not_allowed_to_be_collaborator = Organizations cannot be added as a collaborator. settings.user_is_org_member = The user is an organization member who cannot be added as a collaborator. settings.add_webhook = Add Webhook +settings.add_webhook.invalid_channel_name = Webhook channel name cannot be empty and cannot contain only a # character. settings.hooks_desc = Webhooks automatically make HTTP POST requests to a server when certain Gitea events trigger. Read more in the webhooks guide. settings.webhook_deletion = Remove Webhook settings.webhook_deletion_desc = Removing a webhook deletes its settings and delivery history. Continue? diff --git a/routers/api/v1/utils/hook.go b/routers/api/v1/utils/hook.go index d0538ec54f640..8eb9c86ebfd0b 100644 --- a/routers/api/v1/utils/hook.go +++ b/routers/api/v1/utils/hook.go @@ -5,6 +5,8 @@ package utils import ( + "strings" + api "code.gitea.io/sdk/gitea" "encoding/json" @@ -13,6 +15,7 @@ import ( "code.gitea.io/gitea/models" "code.gitea.io/gitea/modules/context" "code.gitea.io/gitea/routers/api/v1/convert" + "code.gitea.io/gitea/routers/utils" "github.com/Unknwon/com" ) @@ -119,8 +122,13 @@ func addHook(ctx *context.APIContext, form *api.CreateHookOption, orgID, repoID ctx.Error(422, "", "Missing config option: channel") return nil, false } + + if !utils.IsValidSlackChannel(channel) { + ctx.Error(400, "", "Invalid slack channel name") + } + meta, err := json.Marshal(&models.SlackMeta{ - Channel: channel, + Channel: strings.TrimSpace(channel), Username: form.Config["username"], IconURL: form.Config["icon_url"], Color: form.Config["color"], diff --git a/routers/repo/webhook.go b/routers/repo/webhook.go index 53c1afe660ea9..6c69354c700eb 100644 --- a/routers/repo/webhook.go +++ b/routers/repo/webhook.go @@ -332,8 +332,14 @@ func SlackHooksNewPost(ctx *context.Context, form auth.NewSlackHookForm) { return } + if form.HasInvalidChannel() { + ctx.Flash.Error(ctx.Tr("repo.settings.add_webhook.invalid_channel_name")) + ctx.Redirect(orCtx.Link + "/settings/hooks/slack/new") + return + } + meta, err := json.Marshal(&models.SlackMeta{ - Channel: form.Channel, + Channel: strings.TrimSpace(form.Channel), Username: form.Username, IconURL: form.IconURL, Color: form.Color, @@ -515,8 +521,14 @@ func SlackHooksEditPost(ctx *context.Context, form auth.NewSlackHookForm) { return } + if form.HasInvalidChannel() { + ctx.Flash.Error(ctx.Tr("repo.settings.add_webhook.invalid_channel_name")) + ctx.Redirect(fmt.Sprintf("%s/settings/hooks/%d", orCtx.Link, w.ID)) + return + } + meta, err := json.Marshal(&models.SlackMeta{ - Channel: form.Channel, + Channel: strings.TrimSpace(form.Channel), Username: form.Username, IconURL: form.IconURL, Color: form.Color, diff --git a/routers/utils/utils.go b/routers/utils/utils.go index 6ead7d60e2d15..7c90fd7048ab7 100644 --- a/routers/utils/utils.go +++ b/routers/utils/utils.go @@ -15,3 +15,22 @@ func RemoveUsernameParameterSuffix(name string) string { } return name } + +// IsValidSlackChannel validates a channel name conforms to what slack expects. +// It makes sure a channel name cannot be empty and invalid ( only an # ) +func IsValidSlackChannel(channelName string) bool { + switch len(strings.TrimSpace(channelName)) { + case 0: + return false + case 1: + // Keep default behaviour where a channel name is still + // valid without an # + // But if it contains only an #, it should be regarded as + // invalid + if channelName[0] == '#' { + return false + } + } + + return true +} diff --git a/routers/utils/utils_test.go b/routers/utils/utils_test.go index fb56ac85c2831..d96e1d7d26cf7 100644 --- a/routers/utils/utils_test.go +++ b/routers/utils/utils_test.go @@ -15,3 +15,20 @@ func TestRemoveUsernameParameterSuffix(t *testing.T) { assert.Equal(t, "foobar", RemoveUsernameParameterSuffix("foobar")) assert.Equal(t, "", RemoveUsernameParameterSuffix("")) } + +func TestIsValidSlackChannel(t *testing.T) { + tt := []struct { + channelName string + expected bool + }{ + {"gitea", true}, + {" ", false}, + {"#", false}, + {"gitea ", true}, + {" gitea", true}, + } + + for _, v := range tt { + assert.Equal(t, v.expected, IsValidSlackChannel(v.channelName)) + } +} From e70e41d1616502b5e8980b8d56a122e1aeb52671 Mon Sep 17 00:00:00 2001 From: Lanre Adelowo Date: Fri, 24 Aug 2018 20:54:30 +0100 Subject: [PATCH 2/7] rearrange imports --- routers/api/v1/utils/hook.go | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/routers/api/v1/utils/hook.go b/routers/api/v1/utils/hook.go index 8eb9c86ebfd0b..62eefb43cbde4 100644 --- a/routers/api/v1/utils/hook.go +++ b/routers/api/v1/utils/hook.go @@ -5,13 +5,12 @@ package utils import ( + "encoding/json" + "net/http" "strings" api "code.gitea.io/sdk/gitea" - "encoding/json" - "net/http" - "code.gitea.io/gitea/models" "code.gitea.io/gitea/modules/context" "code.gitea.io/gitea/routers/api/v1/convert" From f767bc3abea7fe3f37e7c9437240ff26379f92a1 Mon Sep 17 00:00:00 2001 From: Lanre Adelowo Date: Fri, 24 Aug 2018 20:57:14 +0100 Subject: [PATCH 3/7] add comment --- modules/auth/repo_form.go | 1 + 1 file changed, 1 insertion(+) diff --git a/modules/auth/repo_form.go b/modules/auth/repo_form.go index b80042224218e..165cdb2920696 100644 --- a/modules/auth/repo_form.go +++ b/modules/auth/repo_form.go @@ -226,6 +226,7 @@ func (f *NewSlackHookForm) Validate(ctx *macaron.Context, errs binding.Errors) b return validate(errs, ctx.Data, f, ctx.Locale) } +// HasInvalidChannel validates the channel name is in the right format func (f NewSlackHookForm) HasInvalidChannel() bool { return !utils.IsValidSlackChannel(f.Channel) } From 9e4d13a527f5ccd235c18e866dd3b13dafda4833 Mon Sep 17 00:00:00 2001 From: Lanre Adelowo Date: Fri, 24 Aug 2018 21:06:43 +0100 Subject: [PATCH 4/7] rearrange imports --- modules/auth/repo_form.go | 1 + 1 file changed, 1 insertion(+) diff --git a/modules/auth/repo_form.go b/modules/auth/repo_form.go index 165cdb2920696..a819a60491b92 100644 --- a/modules/auth/repo_form.go +++ b/modules/auth/repo_form.go @@ -11,6 +11,7 @@ import ( "code.gitea.io/gitea/models" "code.gitea.io/gitea/routers/utils" + "github.com/Unknwon/com" "github.com/go-macaron/binding" "gopkg.in/macaron.v1" From 1c61885246e6fb4356250a1c3bcb8a763ff7fdc5 Mon Sep 17 00:00:00 2001 From: Lanre Adelowo Date: Fri, 24 Aug 2018 21:14:00 +0100 Subject: [PATCH 5/7] add missing return statement --- routers/api/v1/utils/hook.go | 1 + 1 file changed, 1 insertion(+) diff --git a/routers/api/v1/utils/hook.go b/routers/api/v1/utils/hook.go index 62eefb43cbde4..60051062aa4e2 100644 --- a/routers/api/v1/utils/hook.go +++ b/routers/api/v1/utils/hook.go @@ -124,6 +124,7 @@ func addHook(ctx *context.APIContext, form *api.CreateHookOption, orgID, repoID if !utils.IsValidSlackChannel(channel) { ctx.Error(400, "", "Invalid slack channel name") + return nil, false } meta, err := json.Marshal(&models.SlackMeta{ From 968d1f2274dc11ab73f0e670d68a0a56a950f238 Mon Sep 17 00:00:00 2001 From: Lanre Adelowo Date: Sat, 25 Aug 2018 09:13:16 +0100 Subject: [PATCH 6/7] fix imports spacing --- routers/api/v1/utils/hook.go | 1 + 1 file changed, 1 insertion(+) diff --git a/routers/api/v1/utils/hook.go b/routers/api/v1/utils/hook.go index 60051062aa4e2..90c7c270fd79f 100644 --- a/routers/api/v1/utils/hook.go +++ b/routers/api/v1/utils/hook.go @@ -15,6 +15,7 @@ import ( "code.gitea.io/gitea/modules/context" "code.gitea.io/gitea/routers/api/v1/convert" "code.gitea.io/gitea/routers/utils" + "github.com/Unknwon/com" ) From c8a2e274985d87ea797034498f459e67004801f5 Mon Sep 17 00:00:00 2001 From: Lanre Adelowo Date: Fri, 7 Sep 2018 10:24:06 +0100 Subject: [PATCH 7/7] remove blank line --- routers/api/v1/utils/hook.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/routers/api/v1/utils/hook.go b/routers/api/v1/utils/hook.go index 90c7c270fd79f..380dc886a53f6 100644 --- a/routers/api/v1/utils/hook.go +++ b/routers/api/v1/utils/hook.go @@ -9,12 +9,11 @@ import ( "net/http" "strings" - api "code.gitea.io/sdk/gitea" - "code.gitea.io/gitea/models" "code.gitea.io/gitea/modules/context" "code.gitea.io/gitea/routers/api/v1/convert" "code.gitea.io/gitea/routers/utils" + api "code.gitea.io/sdk/gitea" "github.com/Unknwon/com" )