Skip to content

Commit

Permalink
[MM-62798][MM-63193] Restrict channel permissions on archived channel…
Browse files Browse the repository at this point in the history
…s when viewing archived channels is disabled (mattermost#30314)

* [MM-62798][MM-63193] Restrict channel permissions on archived channels when viewing archived channels is disabled

* PR feedback

* PR feedback
  • Loading branch information
devinbinnie authored Feb 27, 2025
1 parent aa1f50c commit 3411863
Show file tree
Hide file tree
Showing 2 changed files with 126 additions and 17 deletions.
53 changes: 36 additions & 17 deletions server/channels/app/authorization.go
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,19 @@ func (a *App) SessionHasPermissionToChannel(c request.CTX, session model.Session
return false
}

channel, appErr := a.GetChannel(c, channelID)
if appErr != nil && appErr.StatusCode == http.StatusNotFound {
return false
}

if session.IsUnrestricted() || a.RolesGrantPermission(session.GetUserRoles(), model.PermissionManageSystem.Id) {
return true
}

if a.isChannelArchivedAndHidden(channel) {
return false
}

ids, err := a.Srv().Store().Channel().GetAllChannelMembersForUser(c, session.UserId, true, true)
var channelRoles []string
if err == nil {
Expand All @@ -93,15 +106,6 @@ func (a *App) SessionHasPermissionToChannel(c request.CTX, session model.Session
}
}

channel, appErr := a.GetChannel(c, channelID)
if appErr != nil && appErr.StatusCode == http.StatusNotFound {
return false
}

if session.IsUnrestricted() {
return true
}

if appErr == nil && channel.TeamId != "" {
return a.SessionHasPermissionToTeam(session, channel.TeamId, permission)
}
Expand All @@ -115,22 +119,32 @@ func (a *App) SessionHasPermissionToChannels(c request.CTX, session model.Sessio
return true
}

if session.IsUnrestricted() || a.RolesGrantPermission(session.GetUserRoles(), model.PermissionManageSystem.Id) {
return true
}

for _, channelID := range channelIDs {
if channelID == "" {
return false
}
}

// if System Roles (i.e. Admin, TeamAdmin) allow permissions
// if so, no reason to check team
if a.SessionHasPermissionTo(session, permission) {
// make sure all channels exist, otherwise return false.
for _, channelID := range channelIDs {
_, appErr := a.GetChannel(c, channelID)
if appErr != nil && appErr.StatusCode == http.StatusNotFound {
channel, appErr := a.GetChannel(c, channelID)
if appErr != nil {
return false
}

// if any channel is archived and the user doesn't have permission to view archived channels, return false
if a.isChannelArchivedAndHidden(channel) {
return false
}
}
}

// if System Roles (i.e. Admin, TeamAdmin) allow permissions
// if so, no reason to check team
if a.SessionHasPermissionTo(session, permission) {
return true
}

Expand All @@ -148,6 +162,7 @@ func (a *App) SessionHasPermissionToChannels(c request.CTX, session model.Sessio
}
return false
}

return true
}

Expand Down Expand Up @@ -389,7 +404,7 @@ func (a *App) SessionHasPermissionToReadChannel(c request.CTX, session model.Ses
}

func (a *App) HasPermissionToReadChannel(c request.CTX, userID string, channel *model.Channel) bool {
if !*a.Config().TeamSettings.ExperimentalViewArchivedChannels && channel.DeleteAt != 0 {
if a.isChannelArchivedAndHidden(channel) {
return false
}
if a.HasPermissionToChannel(c, userID, channel.Id, model.PermissionReadChannelContent) {
Expand All @@ -404,7 +419,7 @@ func (a *App) HasPermissionToReadChannel(c request.CTX, userID string, channel *
}

func (a *App) HasPermissionToChannelMemberCount(c request.CTX, userID string, channel *model.Channel) bool {
if !*a.Config().TeamSettings.ExperimentalViewArchivedChannels && channel.DeleteAt != 0 {
if a.isChannelArchivedAndHidden(channel) {
return false
}
if a.HasPermissionToChannel(c, userID, channel.Id, model.PermissionReadChannelContent) {
Expand All @@ -417,3 +432,7 @@ func (a *App) HasPermissionToChannelMemberCount(c request.CTX, userID string, ch

return false
}

func (a *App) isChannelArchivedAndHidden(channel *model.Channel) bool {
return !*a.Config().TeamSettings.ExperimentalViewArchivedChannels && channel.DeleteAt != 0
}
90 changes: 90 additions & 0 deletions server/channels/app/authorization_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,24 @@ func TestSessionHasPermissionToChannel(t *testing.T) {
assert.True(t, th.App.SessionHasPermissionToChannel(th.Context, session, th.BasicChannel.Id, model.PermissionAddReaction))
})

t.Run("basic user cannot access archived channel if setting is off", func(t *testing.T) {
th.App.UpdateConfig(func(cfg *model.Config) {
*cfg.TeamSettings.ExperimentalViewArchivedChannels = false
})
err := th.App.DeleteChannel(th.Context, th.BasicChannel, th.SystemAdminUser.Id)
require.Nil(t, err)
assert.False(t, th.App.SessionHasPermissionToChannel(th.Context, session, th.BasicChannel.Id, model.PermissionReadChannel))
})

t.Run("basic user can access archived channel if setting is on", func(t *testing.T) {
th.App.UpdateConfig(func(cfg *model.Config) {
*cfg.TeamSettings.ExperimentalViewArchivedChannels = true
})
err := th.App.DeleteChannel(th.Context, th.BasicChannel, th.SystemAdminUser.Id)
require.Nil(t, err)
assert.True(t, th.App.SessionHasPermissionToChannel(th.Context, session, th.BasicChannel.Id, model.PermissionReadChannel))
})

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.
Expand Down Expand Up @@ -203,6 +221,78 @@ func TestSessionHasPermissionToChannels(t *testing.T) {
assert.False(t, th.App.SessionHasPermissionToChannels(th.Context, session, allChannels, model.PermissionReadChannel))
})

t.Run("basic user can access archived channel if setting is on", func(t *testing.T) {
session := model.Session{
UserId: th.BasicUser.Id,
}
th.App.UpdateConfig(func(cfg *model.Config) {
*cfg.TeamSettings.ExperimentalViewArchivedChannels = true
})

newChannel := th.CreateChannel(th.Context, th.BasicTeam)
_, appErr := th.App.AddUserToChannel(th.Context, th.BasicUser, newChannel, false)
assert.Nil(t, appErr)

err := th.App.DeleteChannel(th.Context, newChannel, th.SystemAdminUser.Id)
require.Nil(t, err)
assert.True(t, th.App.SessionHasPermissionToChannels(th.Context, session, []string{newChannel.Id}, model.PermissionReadChannel))
})

t.Run("basic user cannot access archived channel if setting is off", func(t *testing.T) {
session := model.Session{
UserId: th.BasicUser.Id,
}
th.App.UpdateConfig(func(cfg *model.Config) {
*cfg.TeamSettings.ExperimentalViewArchivedChannels = false
})

newChannel := th.CreateChannel(th.Context, th.BasicTeam)
_, appErr := th.App.AddUserToChannel(th.Context, th.BasicUser, newChannel, false)
assert.Nil(t, appErr)

err := th.App.DeleteChannel(th.Context, newChannel, th.SystemAdminUser.Id)
require.Nil(t, err)
assert.False(t, th.App.SessionHasPermissionToChannels(th.Context, session, []string{newChannel.Id}, model.PermissionReadChannel))
})

t.Run("basic user cannot access mixed archived and non-archived channels if setting is off", func(t *testing.T) {
session := model.Session{
UserId: th.BasicUser.Id,
}
th.App.UpdateConfig(func(cfg *model.Config) {
*cfg.TeamSettings.ExperimentalViewArchivedChannels = false
})

archivedChannel := th.CreateChannel(th.Context, th.BasicTeam)
_, appErr := th.App.AddUserToChannel(th.Context, th.BasicUser, archivedChannel, false)
assert.Nil(t, appErr)

err := th.App.DeleteChannel(th.Context, archivedChannel, th.SystemAdminUser.Id)
require.Nil(t, err)

mixedChannels := []string{th.BasicChannel.Id, archivedChannel.Id}
assert.False(t, th.App.SessionHasPermissionToChannels(th.Context, session, mixedChannels, model.PermissionReadChannel))
})

t.Run("basic user can access mixed archived and non-archived channels if setting is on", func(t *testing.T) {
session := model.Session{
UserId: th.BasicUser.Id,
}
th.App.UpdateConfig(func(cfg *model.Config) {
*cfg.TeamSettings.ExperimentalViewArchivedChannels = true
})

archivedChannel := th.CreateChannel(th.Context, th.BasicTeam)
_, appErr := th.App.AddUserToChannel(th.Context, th.BasicUser, archivedChannel, false)
assert.Nil(t, appErr)

err := th.App.DeleteChannel(th.Context, archivedChannel, th.SystemAdminUser.Id)
require.Nil(t, err)

mixedChannels := []string{th.BasicChannel.Id, archivedChannel.Id}
assert.True(t, th.App.SessionHasPermissionToChannels(th.Context, session, mixedChannels, model.PermissionReadChannel))
})

t.Run("System Admins can access basic channels", func(t *testing.T) {
session := model.Session{
UserId: th.SystemAdminUser.Id,
Expand Down

0 comments on commit 3411863

Please sign in to comment.