Added duplicated trigger validation (#3124)

7 files changed, 135 insertions, 13 deletions

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
@@ -300,6 +300,10 @@
     "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"
   },
@@ -1856,6 +1860,10 @@
     "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")
+	}
+}