diff --git a/app/authorization.go b/app/authorization.go index 2abf390722..05d32c5a81 100644 --- a/app/authorization.go +++ b/app/authorization.go @@ -80,7 +80,7 @@ func (a *App) SessionHasPermissionToChannel(session model.Session, channelId str return true } - if channel.TeamId != "" { + if appErr == nil && channel.TeamId != "" { return a.SessionHasPermissionToTeam(session, channel.TeamId, permission) } diff --git a/app/authorization_test.go b/app/authorization_test.go index 1adad930f4..c153110d64 100644 --- a/app/authorization_test.go +++ b/app/authorization_test.go @@ -4,6 +4,9 @@ package app import ( + "fmt" + "github.com/mattermost/mattermost-server/v5/plugin/plugintest/mock" + "github.com/mattermost/mattermost-server/v5/store/storetest/mocks" "testing" "github.com/stretchr/testify/assert" @@ -60,6 +63,44 @@ func TestHasPermissionToTeam(t *testing.T) { assert.True(t, th.App.HasPermissionToTeam(th.SystemAdminUser.Id, th.BasicTeam.Id, model.PERMISSION_LIST_TEAM_CHANNELS)) } +func TestSessionHasPermissionToChannel(t *testing.T) { + th := Setup(t).InitBasic() + defer th.TearDown() + + session := model.Session{ + UserId: th.BasicUser.Id, + } + + t.Run("basic user can access basic channel", func(t *testing.T) { + assert.True(t, th.App.SessionHasPermissionToChannel(session, th.BasicChannel.Id, model.PERMISSION_ADD_REACTION)) + }) + + t.Run("does not panic if fetching channel causes an error", func(t *testing.T) { + // Regression test for MM-29812 + // Mock the channel store so getting the channel returns with an error, as per the bug report. + mockStore := mocks.Store{} + mockChannelStore := mocks.ChannelStore{} + mockChannelStore.On("Get", mock.Anything, mock.Anything).Return(nil, fmt.Errorf("arbitrary error")) + mockChannelStore.On("GetAllChannelMembersForUser", mock.Anything, mock.Anything, mock.Anything).Return(th.App.Srv().Store.Channel().GetAllChannelMembersForUser(th.BasicUser.Id, false, false)) + mockChannelStore.On("ClearCaches").Return() + mockStore.On("Channel").Return(&mockChannelStore) + mockStore.On("FileInfo").Return(th.App.Srv().Store.FileInfo()) + mockStore.On("License").Return(th.App.Srv().Store.License()) + mockStore.On("Post").Return(th.App.Srv().Store.Post()) + mockStore.On("Role").Return(th.App.Srv().Store.Role()) + mockStore.On("System").Return(th.App.Srv().Store.System()) + mockStore.On("Team").Return(th.App.Srv().Store.Team()) + mockStore.On("User").Return(th.App.Srv().Store.User()) + mockStore.On("Webhook").Return(th.App.Srv().Store.Webhook()) + mockStore.On("Close").Return(nil) + th.App.Srv().Store = &mockStore + + // If there's an error returned from the GetChannel call the code should continue to cascade and since there + // are no session level permissions in this test case, the permission should be denied. + assert.False(t, th.App.SessionHasPermissionToChannel(session, th.BasicUser.Id, model.PERMISSION_ADD_REACTION)) + }) +} + func TestHasPermissionToCategory(t *testing.T) { th := Setup(t).InitBasic() defer th.TearDown()