Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Slack webhook channel name cannot be empty or just contain an hashtag #4786

Merged
Merged
Show file tree
Hide file tree
Changes from 9 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions modules/auth/repo_form.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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"`
Expand Down
1 change: 1 addition & 0 deletions options/locale/locale_en-US.ini
Original file line number Diff line number Diff line change
Expand Up @@ -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 <a target="_blank" rel="noopener noreferrer" href="%s">webhooks guide</a>.
settings.webhook_deletion = Remove Webhook
settings.webhook_deletion_desc = Removing a webhook deletes its settings and delivery history. Continue?
Expand Down
15 changes: 12 additions & 3 deletions routers/api/v1/utils/hook.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,17 @@
package utils

import (
api "code.gitea.io/sdk/gitea"

"encoding/json"
"net/http"
"strings"

api "code.gitea.io/sdk/gitea"

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove empty line between internal package.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

otherwise L-GTM

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove line13

"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"
lafriks marked this conversation as resolved.
Show resolved Hide resolved

"github.com/Unknwon/com"
)

Expand Down Expand Up @@ -119,8 +122,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"],
Expand Down
16 changes: 14 additions & 2 deletions routers/repo/webhook.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down
19 changes: 19 additions & 0 deletions routers/utils/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
17 changes: 17 additions & 0 deletions routers/utils/utils_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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))
}
}