From 2945e8a2b0ce9306bb049e65eb2410038e0fa18c Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jes=C3=BAs=20Espino?= <jespinog@gmail.com>
Date: Tue, 2 Oct 2018 19:33:52 +0200
Subject: MM-10699: Disallow renaming direct and group message channels (#9518)

* MM-10699: Disallow renaming direct and group message channels

* Replacing errors.New with errors.Wrapf
---
 app/channel.go                     |  8 +++++++
 app/channel_test.go                | 47 ++++++++++++++++++++++++++++++++------
 cmd/mattermost/commands/channel.go |  6 ++---
 i18n/en.json                       |  8 +++++++
 4 files changed, 59 insertions(+), 10 deletions(-)

diff --git a/app/channel.go b/app/channel.go
index 30fc2c1b6..54b589175 100644
--- a/app/channel.go
+++ b/app/channel.go
@@ -163,6 +163,14 @@ func (a *App) CreateChannelWithUser(channel *model.Channel, userId string) (*mod
 
 // RenameChannel is used to rename the channel Name and the DisplayName fields
 func (a *App) RenameChannel(channel *model.Channel, newChannelName string, newDisplayName string) (*model.Channel, *model.AppError) {
+	if channel.Type == model.CHANNEL_DIRECT {
+		return nil, model.NewAppError("RenameChannel", "api.channel.rename_channel.cant_rename_direct_messages.app_error", nil, "", http.StatusBadRequest)
+	}
+
+	if channel.Type == model.CHANNEL_GROUP {
+		return nil, model.NewAppError("RenameChannel", "api.channel.rename_channel.cant_rename_group_messages.app_error", nil, "", http.StatusBadRequest)
+	}
+
 	channel.Name = newChannelName
 	if newDisplayName != "" {
 		channel.DisplayName = newDisplayName
diff --git a/app/channel_test.go b/app/channel_test.go
index 4dc8dce37..0501b9406 100644
--- a/app/channel_test.go
+++ b/app/channel_test.go
@@ -640,12 +640,45 @@ func TestRenameChannel(t *testing.T) {
 	th := Setup().InitBasic()
 	defer th.TearDown()
 
-	channel := th.createChannel(th.BasicTeam, model.CHANNEL_OPEN)
-
-	channel, err := th.App.RenameChannel(channel, "newchannelname", "New Display Name")
-	if err != nil {
-		t.Fatal("Failed to update channel name. Error: " + err.Error())
+	testCases := []struct {
+		Name                string
+		Channel             *model.Channel
+		ExpectError         bool
+		ExpectedName        string
+		ExpectedDisplayName string
+	}{
+		{
+			"Rename open channel",
+			th.createChannel(th.BasicTeam, model.CHANNEL_OPEN),
+			false,
+			"newchannelname",
+			"New Display Name",
+		},
+		{
+			"Fail on rename direct message channel",
+			th.CreateDmChannel(th.BasicUser2),
+			true,
+			"",
+			"",
+		},
+		{
+			"Fail on rename direct message channel",
+			th.CreateGroupChannel(th.BasicUser2, th.CreateUser()),
+			true,
+			"",
+			"",
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.Name, func(t *testing.T) {
+			channel, err := th.App.RenameChannel(tc.Channel, "newchannelname", "New Display Name")
+			if tc.ExpectError {
+				assert.NotNil(t, err)
+			} else {
+				assert.Equal(t, tc.ExpectedName, channel.Name)
+				assert.Equal(t, tc.ExpectedDisplayName, channel.DisplayName)
+			}
+		})
 	}
-	assert.Equal(t, "newchannelname", channel.Name)
-	assert.Equal(t, "New Display Name", channel.DisplayName)
 }
diff --git a/cmd/mattermost/commands/channel.go b/cmd/mattermost/commands/channel.go
index 270ed6586..7fb8fea8c 100644
--- a/cmd/mattermost/commands/channel.go
+++ b/cmd/mattermost/commands/channel.go
@@ -4,11 +4,11 @@
 package commands
 
 import (
-	"errors"
 	"fmt"
 
 	"github.com/mattermost/mattermost-server/app"
 	"github.com/mattermost/mattermost-server/model"
+	"github.com/pkg/errors"
 	"github.com/spf13/cobra"
 )
 
@@ -518,7 +518,7 @@ func modifyChannelCmdF(command *cobra.Command, args []string) error {
 
 	user := getUserFromUserArg(a, username)
 	if _, err := a.UpdateChannelPrivacy(channel, user); err != nil {
-		return errors.New("Failed to update channel ('" + args[0] + "') privacy - " + err.Error())
+		return errors.Wrapf(err, "Failed to update channel ('%s') privacy", args[0])
 	}
 
 	return nil
@@ -549,7 +549,7 @@ func renameChannelCmdF(command *cobra.Command, args []string) error {
 
 	_, errch := a.RenameChannel(channel, newChannelName, newDisplayName)
 	if errch != nil {
-		return errors.New("Error in updating channel from " + channel.Name + " to " + newChannelName + err.Error())
+		return errors.Wrapf(errch, "Error in updating channel from %s to %s", channel.Name, newChannelName)
 	}
 
 	return nil
diff --git a/i18n/en.json b/i18n/en.json
index 469e6976d..69d36cbdb 100644
--- a/i18n/en.json
+++ b/i18n/en.json
@@ -239,6 +239,14 @@
     "id": "api.channel.remove_member.removed",
     "translation": "%v removed from the channel."
   },
+  {
+    "id": "api.channel.rename_channel.cant_rename_direct_messages.app_error",
+    "translation": "You cannot rename a direct message channel"
+  },
+  {
+    "id": "api.channel.rename_channel.cant_rename_group_messages.app_error",
+    "translation": "You cannot rename a group message channel"
+  },
   {
     "id": "api.channel.update_channel.deleted.app_error",
     "translation": "The channel has been archived or deleted"
-- 
cgit v1.2.3-1-g7c22