From 51e5c9b36ce6b46153198f0e727d223f587a8b5a Mon Sep 17 00:00:00 2001 From: Agniva De Sarker Date: Fri, 20 Jan 2023 10:00:14 +0530 Subject: [PATCH] MM-49551: Check if RemoteId is nil for participant (#22103) We were trying to dereference without a nil check first. https://mattermost.atlassian.net/browse/MM-49551 ```release-note NONE ``` --- app/platform/shared_channel_notifier.go | 2 +- app/platform/shared_channel_notifier_test.go | 38 ++++++++++++++++++++ 2 files changed, 39 insertions(+), 1 deletion(-) diff --git a/app/platform/shared_channel_notifier.go b/app/platform/shared_channel_notifier.go index 2e0bdafc9e..4be7a55c0c 100644 --- a/app/platform/shared_channel_notifier.go +++ b/app/platform/shared_channel_notifier.go @@ -111,7 +111,7 @@ func handleInvitation(ps *PlatformService, syncService SharedChannelServiceIFace return err } - if participant == nil { + if participant == nil || participant.RemoteId == nil { return nil } diff --git a/app/platform/shared_channel_notifier_test.go b/app/platform/shared_channel_notifier_test.go index e439b76a71..dd09c74aea 100644 --- a/app/platform/shared_channel_notifier_test.go +++ b/app/platform/shared_channel_notifier_test.go @@ -7,6 +7,8 @@ import ( "testing" "github.com/mattermost/mattermost-server/v6/model" + "github.com/mattermost/mattermost-server/v6/plugin/plugintest/mock" + "github.com/mattermost/mattermost-server/v6/store/storetest/mocks" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -68,4 +70,40 @@ func TestServerSyncSharedChannelHandler(t *testing.T) { require.Len(t, mockService.channelNotifications, 1) assert.Equal(t, channel.Id, mockService.channelNotifications[0]) }) + + t.Run("sync service doesn't panic when no RemoteId", func(t *testing.T) { + th := SetupWithStoreMock(t) + defer th.TearDown() + + mockStore := th.Service.Store.(*mocks.Store) + + mockChannelStore := &mocks.ChannelStore{} + mockChannelStore.On("Get", "channelID", true).Return(&model.Channel{ + Id: "channelID", + Shared: model.NewBool(true), + }, nil) + + mockUserStore := &mocks.UserStore{} + mockUserStore.On("Get", mock.Anything, "creator").Return(&model.User{}, nil) + // Not setting RemoteId here causes the panic. + mockUserStore.On("Get", mock.Anything, "teammate").Return(&model.User{}, nil) + + mockRemoteClusterStore := &mocks.RemoteClusterStore{} + mockRemoteClusterStore.On("Get", mock.Anything).Return(&model.RemoteCluster{}, nil) + + mockStore.On("Channel").Return(mockChannelStore) + mockStore.On("User").Return(mockUserStore) + mockStore.On("RemoteCluster").Return(mockRemoteClusterStore) + + mockService := NewMockSharedChannelService(nil) + mockService.active = true + th.Service.SetSharedChannelService(mockService) + + require.NotPanics(t, func() { + websocketEvent := model.NewWebSocketEvent(model.WebsocketEventDirectAdded, "teamID", "channelID", "userID", nil, "") + websocketEvent = websocketEvent.SetData(map[string]any{"creator_id": "creator", "teammate_id": "teammate"}) + th.Service.SharedChannelSyncHandler(websocketEvent) + assert.Empty(t, mockService.channelNotifications) + }) + }) }