From 0d0734ac9845ef32c55ebf4c3185ba85065c5940 Mon Sep 17 00:00:00 2001 From: David Lu Date: Fri, 27 May 2016 08:35:55 -0700 Subject: Added duplicated trigger validation (#3124) --- api/command.go | 20 ++++++++++++++++++++ api/command_test.go | 41 ++++++++++++++++++++++++++++------------- api/webhook.go | 17 +++++++++++++++++ api/webhook_test.go | 11 +++++++++++ i18n/en.json | 8 ++++++++ utils/utils.go | 21 +++++++++++++++++++++ utils/utils_test.go | 30 ++++++++++++++++++++++++++++++ 7 files changed, 135 insertions(+), 13 deletions(-) create mode 100644 utils/utils.go create mode 100644 utils/utils_test.go diff --git a/api/command.go b/api/command.go index 72249a48c..e1c576bba 100644 --- a/api/command.go +++ b/api/command.go @@ -288,6 +288,26 @@ func createCommand(c *Context, w http.ResponseWriter, r *http.Request) { cmd.CreatorId = c.Session.UserId cmd.TeamId = c.TeamId + if result := <-Srv.Store.Command().GetByTeam(c.TeamId); result.Err != nil { + c.Err = result.Err + return + } else { + teamCmds := result.Data.([]*model.Command) + for _, existingCommand := range teamCmds { + if cmd.Trigger == existingCommand.Trigger { + c.Err = model.NewLocAppError("createCommand", "api.command.duplicate_trigger.app_error", nil, "") + return + } + } + for _, builtInProvider := range commandProviders { + builtInCommand := *builtInProvider.GetCommand(c) + if cmd.Trigger == builtInCommand.Trigger { + c.Err = model.NewLocAppError("createCommand", "api.command.duplicate_trigger.app_error", nil, "") + return + } + } + } + if result := <-Srv.Store.Command().Save(cmd); result.Err != nil { c.Err = result.Err return diff --git a/api/command_test.go b/api/command_test.go index c6500c6cf..9c0b34085 100644 --- a/api/command_test.go +++ b/api/command_test.go @@ -45,16 +45,28 @@ func TestCreateCommand(t *testing.T) { }() *utils.Cfg.ServiceSettings.EnableCommands = true - cmd := &model.Command{URL: "http://nowhere.com", Method: model.COMMAND_METHOD_POST, Trigger: "trigger"} + cmd1 := &model.Command{ + CreatorId: user.Id, + TeamId: team.Id, + URL: "http://nowhere.com", + Method: model.COMMAND_METHOD_POST, + Trigger: "trigger"} - if _, err := Client.CreateCommand(cmd); err == nil { + if _, err := Client.CreateCommand(cmd1); err == nil { t.Fatal("should have failed because not admin") } Client = th.SystemAdminClient + cmd2 := &model.Command{ + CreatorId: user.Id, + TeamId: team.Id, + URL: "http://nowhere.com", + Method: model.COMMAND_METHOD_POST, + Trigger: "trigger"} + var rcmd *model.Command - if result, err := Client.CreateCommand(cmd); err != nil { + if result, err := Client.CreateCommand(cmd2); err != nil { t.Fatal(err) } else { rcmd = result.Data.(*model.Command) @@ -68,16 +80,19 @@ func TestCreateCommand(t *testing.T) { t.Fatal("team ids didn't match") } - cmd = &model.Command{CreatorId: "123", TeamId: "456", URL: "http://nowhere.com", Method: model.COMMAND_METHOD_POST, Trigger: "trigger"} - if result, err := Client.CreateCommand(cmd); err != nil { - t.Fatal(err) - } else { - if result.Data.(*model.Command).CreatorId != user.Id { - t.Fatal("bad user id wasn't overwritten") - } - if result.Data.(*model.Command).TeamId != team.Id { - t.Fatal("bad team id wasn't overwritten") - } + cmd3 := &model.Command{ + CreatorId: "123", + TeamId: "456", + URL: "http://nowhere.com", + Method: model.COMMAND_METHOD_POST, + Trigger: "trigger"} + if _, err := Client.CreateCommand(cmd3); err == nil { + t.Fatal("trigger cannot be duplicated") + } + + cmd4 := cmd3 + if _, err := Client.CreateCommand(cmd4); err == nil { + t.Fatal("command cannot be duplicated") } } diff --git a/api/webhook.go b/api/webhook.go index 11456d69e..676fd2cbc 100644 --- a/api/webhook.go +++ b/api/webhook.go @@ -214,6 +214,23 @@ func createOutgoingHook(c *Context, w http.ResponseWriter, r *http.Request) { return } + if result := <-Srv.Store.Webhook().GetOutgoingByTeam(c.TeamId); result.Err != nil { + c.Err = result.Err + return + } else { + allHooks := result.Data.([]*model.OutgoingWebhook) + + for _, existingOutHook := range allHooks { + urlIntersect := utils.StringArrayIntersection(existingOutHook.CallbackURLs, hook.CallbackURLs) + triggerIntersect := utils.StringArrayIntersection(existingOutHook.TriggerWords, hook.TriggerWords) + + if existingOutHook.ChannelId == hook.ChannelId && len(urlIntersect) != 0 && len(triggerIntersect) != 0 { + c.Err = model.NewLocAppError("createOutgoingHook", "api.webhook.create_outgoing.intersect.app_error", nil, "") + return + } + } + } + if result := <-Srv.Store.Webhook().SaveOutgoing(hook); result.Err != nil { c.Err = result.Err return diff --git a/api/webhook_test.go b/api/webhook_test.go index 1b13bb5d4..80ee8ad7d 100644 --- a/api/webhook_test.go +++ b/api/webhook_test.go @@ -262,6 +262,17 @@ func TestCreateOutgoingHook(t *testing.T) { t.Fatal("team ids didn't match") } + hook = &model.OutgoingWebhook{ChannelId: channel1.Id, TriggerWords: []string{"cats", "dogs"}, CallbackURLs: []string{"http://nowhere.com", "http://cats.com"}} + hook1 := &model.OutgoingWebhook{ChannelId: channel1.Id, TriggerWords: []string{"cats"}, CallbackURLs: []string{"http://nowhere.com"}} + + if _, err := Client.CreateOutgoingWebhook(hook); err != nil { + t.Fatal("multiple trigger words and urls failed") + } + + if _, err := Client.CreateOutgoingWebhook(hook1); err == nil { + t.Fatal("should have failed - duplicate trigger words and urls") + } + hook = &model.OutgoingWebhook{ChannelId: "junk", CallbackURLs: []string{"http://nowhere.com"}} if _, err := Client.CreateOutgoingWebhook(hook); err == nil { t.Fatal("should have failed - bad channel id") diff --git a/i18n/en.json b/i18n/en.json index b0b120b4e..35a9a3102 100644 --- a/i18n/en.json +++ b/i18n/en.json @@ -299,6 +299,10 @@ "id": "api.command.admin_only.app_error", "translation": "Integrations have been limited to admins only." }, + { + "id": "api.command.duplicate_trigger.app_error", + "translation": "This trigger word is already in use. Please choose another word." + }, { "id": "api.command.delete.app_error", "translation": "Inappropriate permissions to delete command" @@ -1855,6 +1859,10 @@ "id": "api.webhook.create_outgoing.disabled.app_error", "translation": "Outgoing webhooks have been disabled by the system admin." }, + { + "id": "api.webhook.create_outgoing.intersect.app_error", + "translation": "Outgoing webhooks from the same channel cannot have the same trigger words/callback URLs." + }, { "id": "api.webhook.create_outgoing.not_open.app_error", "translation": "Outgoing webhooks can only be created for public channels." diff --git a/utils/utils.go b/utils/utils.go new file mode 100644 index 000000000..f826c65a0 --- /dev/null +++ b/utils/utils.go @@ -0,0 +1,21 @@ +// Copyright (c) 2016 Mattermost, Inc. All Rights Reserved. +// See License.txt for license information. + +package utils + +func StringArrayIntersection(arr1, arr2 []string) []string { + arrMap := map[string]bool{} + result := []string{} + + for _, value := range arr1 { + arrMap[value] = true + } + + for _, value := range arr2 { + if arrMap[value] { + result = append(result, value) + } + } + + return result +} diff --git a/utils/utils_test.go b/utils/utils_test.go new file mode 100644 index 000000000..41e995e63 --- /dev/null +++ b/utils/utils_test.go @@ -0,0 +1,30 @@ +// Copyright (c) 2016 Mattermost, Inc. All Rights Reserved. +// See License.txt for license information. + +package utils + +import ( + "testing" +) + +func TestStringArrayIntersection(t *testing.T) { + a := []string{ + "abc", + "def", + "ghi", + } + b := []string{ + "jkl", + } + c := []string{ + "def", + } + + if len(StringArrayIntersection(a, b)) != 0 { + t.Fatal("should be 0") + } + + if len(StringArrayIntersection(a, c)) != 1 { + t.Fatal("should be 1") + } +} -- cgit v1.2.3-1-g7c22