From e8260555197b59ac15facd1e398d7c7e8446654b Mon Sep 17 00:00:00 2001
From: Agniva De Sarker <agnivade@yahoo.co.in>
Date: Fri, 3 Jan 2020 20:56:32 +0530
Subject: [PATCH] MM-21336: Improve channel rename restrictions (#13441)

* MM-21336: Improve channel rename restrictions

Non-direct channels should not have __ in them.

* Fix order of i18n extraction

* Incorporate review comments

Use a more relaxed check by actually checking for valid userIds.

* Improve error message
---
 app/channel.go      |  9 +++++++++
 app/channel_test.go | 16 ++++++++++++++--
 i18n/en.json        |  4 ++++
 3 files changed, 27 insertions(+), 2 deletions(-)

diff --git a/app/channel.go b/app/channel.go
index 963fc9b23a..3387301cf7 100644
--- a/app/channel.go
+++ b/app/channel.go
@@ -508,7 +508,16 @@ func (a *App) GetGroupChannel(userIds []string) (*model.Channel, *model.AppError
 	return channel, nil
 }
 
+// UpdateChannel updates a given channel by its Id. It also publishes the CHANNEL_UPDATED event.
 func (a *App) UpdateChannel(channel *model.Channel) (*model.Channel, *model.AppError) {
+	userIds := strings.Split(channel.Name, "__")
+	if channel.Type != model.CHANNEL_DIRECT &&
+		len(userIds) == 2 &&
+		model.IsValidId(userIds[0]) &&
+		model.IsValidId(userIds[1]) {
+		return nil, model.NewAppError("UpdateChannel", "api.channel.update_channel.invalid_character.app_error", nil, "", http.StatusBadRequest)
+	}
+
 	_, err := a.Srv.Store.Channel().Update(channel)
 	if err != nil {
 		return nil, err
diff --git a/app/channel_test.go b/app/channel_test.go
index 328b9256f0..a0b3d46024 100644
--- a/app/channel_test.go
+++ b/app/channel_test.go
@@ -743,6 +743,7 @@ func TestRenameChannel(t *testing.T) {
 		Name                string
 		Channel             *model.Channel
 		ExpectError         bool
+		ChannelName         string
 		ExpectedName        string
 		ExpectedDisplayName string
 	}{
@@ -751,19 +752,30 @@ func TestRenameChannel(t *testing.T) {
 			th.createChannel(th.BasicTeam, model.CHANNEL_OPEN),
 			false,
 			"newchannelname",
+			"newchannelname",
 			"New Display Name",
 		},
+		{
+			"Fail on rename open channel with bad name",
+			th.createChannel(th.BasicTeam, model.CHANNEL_OPEN),
+			true,
+			"6zii9a9g6pruzj451x3esok54h__wr4j4g8zqtnhmkw771pfpynqwo",
+			"",
+			"",
+		},
 		{
 			"Fail on rename direct message channel",
 			th.CreateDmChannel(th.BasicUser2),
 			true,
+			"newchannelname",
 			"",
 			"",
 		},
 		{
-			"Fail on rename direct message channel",
+			"Fail on rename group message channel",
 			th.CreateGroupChannel(th.BasicUser2, th.CreateUser()),
 			true,
+			"newchannelname",
 			"",
 			"",
 		},
@@ -771,7 +783,7 @@ func TestRenameChannel(t *testing.T) {
 
 	for _, tc := range testCases {
 		t.Run(tc.Name, func(t *testing.T) {
-			channel, err := th.App.RenameChannel(tc.Channel, "newchannelname", "New Display Name")
+			channel, err := th.App.RenameChannel(tc.Channel, tc.ChannelName, "New Display Name")
 			if tc.ExpectError {
 				assert.NotNil(t, err)
 			} else {
diff --git a/i18n/en.json b/i18n/en.json
index 0a7045f51f..45659494f9 100644
--- a/i18n/en.json
+++ b/i18n/en.json
@@ -371,6 +371,10 @@
     "id": "api.channel.update_channel.deleted.app_error",
     "translation": "The channel has been archived or deleted"
   },
+  {
+    "id": "api.channel.update_channel.invalid_character.app_error",
+    "translation": "Invalid channel name. User ids are not permitted in channel name for non-direct message channels."
+  },
   {
     "id": "api.channel.update_channel.tried.app_error",
     "translation": "Tried to perform an invalid update of the default channel {{.Channel}}"