From be48397945c77748d412baad3493bb5bd1c95e2a Mon Sep 17 00:00:00 2001 From: Lanre Adelowo Date: Mon, 10 Sep 2018 15:31:08 +0100 Subject: [PATCH] Slack webhook channel name cannot be empty or just contain an hashtag (#4786) --- modules/auth/repo_form.go | 7 +++++++ options/locale/locale_en-US.ini | 1 + routers/api/v1/utils/hook.go | 14 +++++++++++--- routers/repo/webhook.go | 16 ++++++++++++++-- routers/utils/utils.go | 19 +++++++++++++++++++ routers/utils/utils_test.go | 17 +++++++++++++++++ 6 files changed, 69 insertions(+), 5 deletions(-) diff --git a/modules/auth/repo_form.go b/modules/auth/repo_form.go index 3c7940afce..a819a60491 100644 --- a/modules/auth/repo_form.go +++ b/modules/auth/repo_form.go @@ -10,6 +10,8 @@ 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 +227,11 @@ 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) +} + // 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 43a5aa0415..b2d9114e95 100644 --- a/options/locale/locale_en-US.ini +++ b/options/locale/locale_en-US.ini @@ -1044,6 +1044,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 d0538ec54f..380dc886a5 100644 --- a/routers/api/v1/utils/hook.go +++ b/routers/api/v1/utils/hook.go @@ -5,14 +5,16 @@ package utils import ( - api "code.gitea.io/sdk/gitea" - "encoding/json" "net/http" + "strings" "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" ) @@ -119,8 +121,14 @@ 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") + return nil, false + } + 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 53c1afe660..6c69354c70 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 6ead7d60e2..7c90fd7048 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 fb56ac85c2..d96e1d7d26 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)) + } +}