[MM-57988] Fix move thread logic to not block channel admins (#27061)

* mm-57988: Allowing for channel admins to move thread

* Fix the MoveThread team admin unit test that was introduced

* Renaming the hasPermittedRole function to hasPermittedWranglerRole

---------

Co-authored-by: Mattermost Build <build@mattermost.com>
Этот коммит содержится в:
Matheus
2024-06-18 10:41:49 -07:00
коммит произвёл GitHub
родитель 0da473b9f8
Коммит cbd5d95bbb
3 изменённых файлов: 97 добавлений и 21 удалений

Просмотреть файл

@@ -1160,15 +1160,20 @@ func moveThread(c *Context, w http.ResponseWriter, r *http.Request) {
return return
} }
// If there are no configured PermittedWranglerRoles, skip the check posts, _, err := c.App.GetPostsByIds([]string{c.Params.PostId})
userHasRole := len(c.App.Config().WranglerSettings.PermittedWranglerRoles) == 0 if err != nil {
for _, role := range c.App.Config().WranglerSettings.PermittedWranglerRoles { c.Err = err
if user.IsInRole(role) { return
userHasRole = true
break
}
} }
channelMember, err := c.App.GetChannelMember(c.AppContext, posts[0].ChannelId, user.Id)
if err != nil {
c.Err = err
return
}
userHasRole := hasPermittedWranglerRole(c, user, channelMember)
// Sysadmins are always permitted // Sysadmins are always permitted
if !userHasRole && !user.IsSystemAdmin() { if !userHasRole && !user.IsSystemAdmin() {
c.Err = model.NewAppError("moveThread", "api.post.move_thread.no_permission", nil, "", http.StatusForbidden) c.Err = model.NewAppError("moveThread", "api.post.move_thread.no_permission", nil, "", http.StatusForbidden)
@@ -1267,3 +1272,19 @@ func getPostInfo(c *Context, w http.ResponseWriter, r *http.Request) {
w.Write(js) w.Write(js)
} }
func hasPermittedWranglerRole(c *Context, user *model.User, channelMember *model.ChannelMember) bool {
// If there are no configured PermittedWranglerRoles, skip the check
if len(c.App.Config().WranglerSettings.PermittedWranglerRoles) == 0 {
return true
}
userRoles := user.Roles + " " + channelMember.Roles
for _, role := range c.App.Config().WranglerSettings.PermittedWranglerRoles {
if model.IsInRole(userRoles, role) {
return true
}
}
return false
}

Просмотреть файл

@@ -761,16 +761,20 @@ func TestMoveThread(t *testing.T) {
basicUser2 := th.BasicUser2 basicUser2 := th.BasicUser2
basicUser3 := th.CreateUser() basicUser3 := th.CreateUser()
// Create a new public channel to move the post to // Helper function to create a new public channel to move the post to
publicChannel, resp, err := client.CreateChannel(ctx, &model.Channel{ createPublicChannel := func(teamId, name, displayName string) *model.Channel {
TeamId: th.BasicTeam.Id, channel, resp, err := client.CreateChannel(ctx, &model.Channel{
Name: "test-public-channel", TeamId: teamId,
DisplayName: "Test Public Channel", Name: name,
Type: model.ChannelTypeOpen, DisplayName: displayName,
}) Type: model.ChannelTypeOpen,
require.NoError(t, err) })
require.NotNil(t, resp) require.NoError(t, err)
require.NotNil(t, publicChannel) require.NotNil(t, resp)
require.NotNil(t, channel)
return channel
}
// Create a new private channel to move the post to // Create a new private channel to move the post to
privateChannel, resp, err := client.CreateChannel(ctx, &model.Channel{ privateChannel, resp, err := client.CreateChannel(ctx, &model.Channel{
@@ -795,6 +799,9 @@ func TestMoveThread(t *testing.T) {
require.NotNil(t, resp) require.NotNil(t, resp)
require.NotNil(t, gmChannel) require.NotNil(t, gmChannel)
t.Run("Move to public channel", func(t *testing.T) { t.Run("Move to public channel", func(t *testing.T) {
// Create a public channel
publicChannel := createPublicChannel(th.BasicTeam.Id, "test-public-channel", "Test Public Channel")
// Create a new post to move // Create a new post to move
post := &model.Post{ post := &model.Post{
ChannelId: th.BasicChannel.Id, ChannelId: th.BasicChannel.Id,
@@ -971,6 +978,53 @@ func TestMoveThread(t *testing.T) {
require.Equal(t, newPost.Message, posts.Posts[posts.Order[2]].Message) require.Equal(t, newPost.Message, posts.Posts[posts.Order[2]].Message)
require.Equal(t, rootPost.Message, posts.Posts[posts.Order[3]].Message) require.Equal(t, rootPost.Message, posts.Posts[posts.Order[3]].Message)
}) })
t.Run("Move thread when permitted role is channel admin", func(t *testing.T) {
// Create public channel
publicChannel := createPublicChannel(th.BasicTeam.Id, "test-public-channel-admin", "Test Public Channel Admin")
// Set permitted role as channel admin
enabled := true
th.App.UpdateConfig(func(cfg *model.Config) {
cfg.WranglerSettings = model.WranglerSettings{MoveThreadToAnotherTeamEnable: &enabled,
PermittedWranglerRoles: []string{model.PermissionsChannelAdmin}}
})
defer th.App.UpdateConfig(func(cfg *model.Config) {
cfg.WranglerSettings = model.WranglerSettings{}
})
// Login as channel admin and add to channel
th.LoginTeamAdmin()
th.AddUserToChannel(th.TeamAdminUser, publicChannel)
defer th.LoginBasic()
// Create a new post to move
post := &model.Post{
ChannelId: th.BasicChannel.Id,
Message: "test post",
}
newPost, resp, err := client.CreatePost(ctx, post)
require.NoError(t, err)
require.NotNil(t, resp)
require.NotNil(t, newPost)
// Move the post to the public channel
moveThreadParams := &model.MoveThreadParams{
ChannelId: publicChannel.Id,
}
resp, err = client.MoveThread(ctx, newPost.Id, moveThreadParams)
require.NoError(t, err)
require.Equal(t, http.StatusOK, resp.StatusCode)
// Check that the post was moved to the public channel
posts, resp, err := client.GetPostsForChannel(ctx, publicChannel.Id, 0, 100, "", true, false)
require.NoError(t, err)
require.NotNil(t, resp)
require.NotNil(t, posts)
// There should be 2 posts, the system join message for the user who moved it joining the channel, and the post we moved
require.Equal(t, 2, len(posts.Posts))
require.Equal(t, newPost.Message, posts.Posts[posts.Order[0]].Message)
})
} }
func TestCreatePostPublic(t *testing.T) { func TestCreatePostPublic(t *testing.T) {

Просмотреть файл

@@ -11,7 +11,7 @@ import {getCurrentUser} from 'mattermost-redux/selectors/entities/common';
import {getConfig} from 'mattermost-redux/selectors/entities/general'; import {getConfig} from 'mattermost-redux/selectors/entities/general';
import {getPost} from 'mattermost-redux/selectors/entities/posts'; import {getPost} from 'mattermost-redux/selectors/entities/posts';
import {moveThreadsEnabled} from 'mattermost-redux/selectors/entities/preferences'; import {moveThreadsEnabled} from 'mattermost-redux/selectors/entities/preferences';
import {getCurrentUserId} from 'mattermost-redux/selectors/entities/users'; import {getCurrentUserId, getCurrentUserRoles} from 'mattermost-redux/selectors/entities/users';
import {arePreviewsCollapsed} from 'selectors/preferences'; import {arePreviewsCollapsed} from 'selectors/preferences';
import {getGlobalItem} from 'selectors/storage'; import {getGlobalItem} from 'selectors/storage';
@@ -64,10 +64,11 @@ export function makeCanWrangler() {
'makeCanWrangler', 'makeCanWrangler',
getConfig, getConfig,
getCurrentUser, getCurrentUser,
getCurrentUserRoles,
moveThreadsEnabled, moveThreadsEnabled,
(_state: GlobalState, channelType: Channel['type']) => channelType, (_state: GlobalState, channelType: Channel['type']) => channelType,
(_state: GlobalState, _channelType: Channel['type'], replyCount: number) => replyCount, (_state: GlobalState, _channelType: Channel['type'], replyCount: number) => replyCount,
(config: Partial<ClientConfig>, user: UserProfile, enabled: boolean, channelType: Channel['type'], replyCount: number) => { (config: Partial<ClientConfig>, user: UserProfile, userRoles: string, enabled: boolean, channelType: Channel['type'], replyCount: number) => {
if (!enabled) { if (!enabled) {
return false; return false;
} }
@@ -90,8 +91,8 @@ export function makeCanWrangler() {
allowedEmailDomains = WranglerAllowedEmailDomain?.split(',') || []; allowedEmailDomains = WranglerAllowedEmailDomain?.split(',') || [];
} }
if (permittedUsers.length > 0 && !user.roles.includes('system_admin')) { if (permittedUsers.length > 0 && !userRoles.includes('system_admin')) {
const roles = user.roles.split(' '); const roles = userRoles.split(' ');
const hasRole = roles.some((role) => permittedUsers.includes(role)); const hasRole = roles.some((role) => permittedUsers.includes(role));
if (!hasRole) { if (!hasRole) {
return false; return false;