From 344c5a7528d521635a49b9275547b6c6579a6e41 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Alejandro=20Garc=C3=ADa=20Montoro?=
 <alejandro.garciamontoro@gmail.com>
Date: Wed, 28 Feb 2024 15:00:41 +0100
Subject: [PATCH] MM-56781: Use correct error in doInviteRemote (#26304)

* Use correct error in doInviteRemote

A Sentry crash report identified this nil pointer dereference:
https://mattermost-mr.sentry.io/issues/4930233067/events/eae438652c7b4335be2bbe19c977f680

Interestingly, the stack trace shows the actual crash happening in the
function deferred above this line, which makes sense, given that it's
accessing the returned value, which is now nil as well. However, the
origin of the crash is here, since it's using the previous appErr, which
at this point is ensured to be nil, instead of the error returned by
InviteRemoteToChannel. This makes the returned value that should have
been generated by responsef to no longer exist.

* Test inviting a remote to a channel shared with us

* Improve error when InviteRemoteToChannel fails

* Clarify that channel has remoteID

Co-authored-by: Doug Lauder <wiggin77@warpmail.net>

* gofmt

---------

Co-authored-by: Doug Lauder <wiggin77@warpmail.net>
---
 .../app/slashcommands/command_share.go        |  2 +-
 .../app/slashcommands/command_share_test.go   | 38 +++++++++++++++++++
 server/i18n/en.json                           |  4 ++
 3 files changed, 43 insertions(+), 1 deletion(-)

diff --git a/server/channels/app/slashcommands/command_share.go b/server/channels/app/slashcommands/command_share.go
index 91affe307a..3b3d2a4348 100644
--- a/server/channels/app/slashcommands/command_share.go
+++ b/server/channels/app/slashcommands/command_share.go
@@ -254,7 +254,7 @@ func (sp *ShareProvider) doInviteRemote(a *app.App, c request.CTX, args *model.C
 	}
 
 	if err = a.InviteRemoteToChannel(args.ChannelId, remoteID, args.UserId, true); err != nil {
-		return responsef(appErr.Error())
+		return responsef(args.T("api.command_share.invite_remote_to_channel.error", map[string]any{"Error": err.Error()}))
 	}
 
 	return responsef("##### " + args.T("api.command_share.invitation_sent", map[string]any{"Name": rc.DisplayName, "SiteURL": rc.SiteURL}))
diff --git a/server/channels/app/slashcommands/command_share_test.go b/server/channels/app/slashcommands/command_share_test.go
index c42f7338f9..cbcee62761 100644
--- a/server/channels/app/slashcommands/command_share_test.go
+++ b/server/channels/app/slashcommands/command_share_test.go
@@ -90,4 +90,42 @@ func TestShareProviderDoCommand(t *testing.T) {
 		})
 		require.Len(t, channelConvertedMessages, 2) // one msg for share creation, one for unshare.
 	})
+
+	t.Run("invite remote to channel shared with us", func(t *testing.T) {
+		th := setupForSharedChannels(t).initBasic()
+		defer th.tearDown()
+
+		th.addPermissionToRole(model.PermissionManageSharedChannels.Id, th.BasicUser.Roles)
+
+		mockSyncService := app.NewMockSharedChannelService(th.Server.GetSharedChannelSyncService())
+		th.Server.SetSharedChannelSyncService(mockSyncService)
+
+		testCluster := &testlib.FakeClusterInterface{}
+		th.Server.Platform().SetCluster(testCluster)
+
+		remoteCluster, err := th.App.AddRemoteCluster(&model.RemoteCluster{
+			RemoteId:   model.NewId(),
+			Name:       "remote",
+			SiteURL:    "example.com",
+			Token:      model.NewId(),
+			Topics:     "topic",
+			CreateAt:   model.GetMillis(),
+			LastPingAt: model.GetMillis(),
+			CreatorId:  model.NewId(),
+		})
+		require.Nil(t, err)
+
+		commandProvider := ShareProvider{}
+		channel := th.CreateChannel(th.BasicTeam, WithShared(true)) // will create with generated remoteID
+		args := &model.CommandArgs{
+			T:         func(s string, args ...any) string { return s },
+			ChannelId: channel.Id,
+			UserId:    th.BasicUser.Id,
+			TeamId:    th.BasicTeam.Id,
+			Command:   "/share-channel invite --connectionID " + remoteCluster.RemoteId,
+		}
+
+		response := commandProvider.DoCommand(th.App, th.Context, args, "")
+		require.Contains(t, response.Text, args.T("api.command_share.invite_remote_to_channel.error"))
+	})
 }
diff --git a/server/i18n/en.json b/server/i18n/en.json
index be04380150..4514fdeeec 100644
--- a/server/i18n/en.json
+++ b/server/i18n/en.json
@@ -1449,6 +1449,10 @@
     "id": "api.command_share.invite_remote.help",
     "translation": "Invites an external Mattermost instance to the current shared channel"
   },
+  {
+    "id": "api.command_share.invite_remote_to_channel.error",
+    "translation": "Cannot invite remote to channel: {{.Error}}"
+  },
   {
     "id": "api.command_share.missing_action",
     "translation": "Missing action. Available actions: {{.Actions}}"