From 49fdffda8ff7bf5df7bfc3d3866893cecf49659d Mon Sep 17 00:00:00 2001 From: Harshil Sharma <18575143+harshilsharma63@users.noreply.github.com> Date: Mon, 25 May 2026 15:54:15 +0530 Subject: [PATCH] Edit attachment permission (#36227) (#36727) Automatic Merge --- e2e-tests/cypress/tests/support/api/role.js | 4 +- server/channels/api4/data_retention_test.go | 21 +- server/channels/api4/post.go | 10 + server/channels/api4/post_test.go | 241 ++++++++++++++++++ server/channels/api4/post_utils.go | 12 + server/channels/api4/post_utils_test.go | 94 +++++++ server/channels/app/app_test.go | 1 + server/channels/app/permissions_migrations.go | 10 + server/channels/testlib/store.go | 1 + server/channels/utils/utils.go | 19 ++ server/public/model/migration.go | 1 + server/public/model/permission.go | 9 + server/public/model/role.go | 2 + .../guest_permissions_tree.test.tsx | 1 + .../guest_permissions_tree.tsx | 1 + .../guest_permissions_tree/index.tsx | 1 + .../permissions_tree.test.tsx.snap | 7 + .../permissions_tree.test.tsx | 11 + .../permissions_tree/permissions_tree.tsx | 1 + .../strings/groups.tsx | 10 + .../strings/permissions.tsx | 10 + .../advanced_text_editor/use_upload_files.tsx | 25 +- .../file_preview/file_preview.test.tsx | 58 +++++ .../components/file_preview/file_preview.tsx | 33 ++- webapp/channels/src/i18n/en.json | 5 + .../src/constants/permissions.ts | 1 + .../channels/src/sass/components/_files.scss | 9 + webapp/channels/src/utils/constants.tsx | 3 + 28 files changed, 570 insertions(+), 31 deletions(-) create mode 100644 server/channels/api4/post_utils_test.go diff --git a/e2e-tests/cypress/tests/support/api/role.js b/e2e-tests/cypress/tests/support/api/role.js index 8398c39c5fe..53469692d1c 100644 --- a/e2e-tests/cypress/tests/support/api/role.js +++ b/e2e-tests/cypress/tests/support/api/role.js @@ -10,8 +10,8 @@ import xor from 'lodash.xor'; export const defaultRolesPermissions = { channel_admin: 'use_channel_mentions remove_reaction manage_public_channel_members use_group_mentions manage_channel_roles manage_private_channel_members add_reaction read_public_channel_groups create_post read_private_channel_groups add_bookmark_public_channel edit_bookmark_public_channel delete_bookmark_public_channel order_bookmark_public_channel add_bookmark_private_channel edit_bookmark_private_channel delete_bookmark_private_channel order_bookmark_private_channel manage_public_channel_auto_translation manage_private_channel_auto_translation', - channel_guest: 'upload_file edit_post create_post use_channel_mentions read_channel read_channel_content add_reaction remove_reaction', - channel_user: 'manage_private_channel_members read_public_channel_groups delete_post read_private_channel_groups use_group_mentions manage_private_channel_properties delete_public_channel add_reaction manage_public_channel_properties edit_post upload_file use_channel_mentions get_public_link read_channel read_channel_content delete_private_channel manage_public_channel_members create_post remove_reaction add_bookmark_public_channel edit_bookmark_public_channel delete_bookmark_public_channel order_bookmark_public_channel add_bookmark_private_channel edit_bookmark_private_channel delete_bookmark_private_channel order_bookmark_private_channel', + channel_guest: 'upload_file edit_post create_post use_channel_mentions read_channel read_channel_content add_reaction remove_reaction edit_file_attachment', + channel_user: 'manage_private_channel_members read_public_channel_groups delete_post read_private_channel_groups use_group_mentions manage_private_channel_properties delete_public_channel add_reaction manage_public_channel_properties edit_post upload_file use_channel_mentions get_public_link read_channel read_channel_content delete_private_channel manage_public_channel_members create_post remove_reaction add_bookmark_public_channel edit_bookmark_public_channel delete_bookmark_public_channel order_bookmark_public_channel add_bookmark_private_channel edit_bookmark_private_channel delete_bookmark_private_channel order_bookmark_private_channel edit_file_attachment', custom_group_user: '', playbook_admin: 'playbook_private_manage_properties playbook_public_make_private playbook_public_manage_members playbook_public_manage_roles playbook_public_manage_properties playbook_private_manage_members playbook_private_manage_roles', playbook_member: 'playbook_public_view playbook_public_manage_members playbook_public_manage_properties playbook_private_view playbook_private_manage_members playbook_private_manage_properties run_create', diff --git a/server/channels/api4/data_retention_test.go b/server/channels/api4/data_retention_test.go index adf9945c89f..c22cac200b2 100644 --- a/server/channels/api4/data_retention_test.go +++ b/server/channels/api4/data_retention_test.go @@ -8,6 +8,7 @@ import ( "net/http" "testing" + "github.com/mattermost/mattermost/server/v8/channels/utils" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" @@ -1043,28 +1044,10 @@ func TestAddChannelsToPolicy(t *testing.T) { validChannelIDs := []string{model.NewId(), model.NewId()} invalidChannelIDs := []string{"invalid_channel_id"} - // Custom function to compare slices regardless of order - unorderedSlicesEqual := func(a, b []string) bool { - if len(a) != len(b) { - return false - } - counts := make(map[string]int) - for _, item := range a { - counts[item]++ - } - for _, item := range b { - counts[item]-- - if counts[item] < 0 { - return false - } - } - return true - } - // Custom matcher for unordered slice comparison unorderedSliceMatcher := func(expected []string) func(actual []string) bool { return func(actual []string) bool { - return unorderedSlicesEqual(expected, actual) + return utils.SliceEqualUnordered(expected, actual) } } diff --git a/server/channels/api4/post.go b/server/channels/api4/post.go index e8f23bebfab..aa2ae07b005 100644 --- a/server/channels/api4/post.go +++ b/server/channels/api4/post.go @@ -1090,6 +1090,11 @@ func updatePost(c *Context, w http.ResponseWriter, r *http.Request) { return } + checkEditFileAttachmentPermission(c, post.FileIds, originalPost) + if c.Err != nil { + return + } + if c.AppContext.Session().UserId != originalPost.UserId { // We don't need to check the member here, since we already checked it above if ok, _ := c.App.SessionHasPermissionToChannel(c.AppContext, *c.AppContext.Session(), originalPost.ChannelId, model.PermissionEditOthersPosts); !ok { @@ -1164,6 +1169,11 @@ func patchPost(c *Context, w http.ResponseWriter, r *http.Request) { if c.Err != nil { return } + + checkEditFileAttachmentPermission(c, *post.FileIds, originalPost) + if c.Err != nil { + return + } } patchedPost, isMemberForPReviews, err := c.App.PatchPost(c.AppContext, c.Params.PostId, c.App.PostPatchWithProxyRemovedFromImageURLs(&post), nil) diff --git a/server/channels/api4/post_test.go b/server/channels/api4/post_test.go index e4e83af4a78..f2ecd38a4de 100644 --- a/server/channels/api4/post_test.go +++ b/server/channels/api4/post_test.go @@ -1961,6 +1961,124 @@ func TestUpdatePost(t *testing.T) { require.Equal(t, int64(0), postFileInfos[0].DeleteAt) }) + t.Run("should prevent adding files when edit_file_attachment permission is revoked", func(t *testing.T) { + th.LoginBasic(t) + + fileResp, _, err := client.UploadFile(context.Background(), data, channel.Id, "test.png") + require.NoError(t, err) + fileId := fileResp.FileInfos[0].Id + + postWithoutFiles, _, appErr := th.App.CreatePost(th.Context, &model.Post{ + UserId: th.BasicUser.Id, + ChannelId: channel.Id, + Message: "Post without files", + }, channel, model.CreatePostFlags{SetOnline: true}) + require.Nil(t, appErr) + + th.RemovePermissionFromRole(t, model.PermissionEditFileAttachment.Id, model.ChannelUserRoleId) + defer th.AddPermissionToRole(t, model.PermissionEditFileAttachment.Id, model.ChannelUserRoleId) + + updatePost := &model.Post{ + Id: postWithoutFiles.Id, + ChannelId: channel.Id, + Message: "Updated post with file", + FileIds: model.StringArray{fileId}, + } + _, resp, err := client.UpdatePost(context.Background(), postWithoutFiles.Id, updatePost) + require.Error(t, err) + require.Equal(t, http.StatusForbidden, resp.StatusCode) + require.Equal(t, "You do not have the appropriate permissions.", err.Error()) + CheckForbiddenStatus(t, resp) + }) + + t.Run("should prevent removing files when edit_file_attachment permission is revoked", func(t *testing.T) { + th.LoginBasic(t) + + fileResp, _, err := client.UploadFile(context.Background(), data, channel.Id, "test.png") + require.NoError(t, err) + fileId := fileResp.FileInfos[0].Id + + postWithFiles, _, appErr := th.App.CreatePost(th.Context, &model.Post{ + UserId: th.BasicUser.Id, + ChannelId: channel.Id, + Message: "Post with files", + FileIds: model.StringArray{fileId}, + }, channel, model.CreatePostFlags{SetOnline: true}) + require.Nil(t, appErr) + + th.RemovePermissionFromRole(t, model.PermissionEditFileAttachment.Id, model.ChannelUserRoleId) + defer th.AddPermissionToRole(t, model.PermissionEditFileAttachment.Id, model.ChannelUserRoleId) + + updatePost := &model.Post{ + Id: postWithFiles.Id, + ChannelId: channel.Id, + Message: "Updated post without file", + FileIds: model.StringArray{}, + } + _, resp, err := client.UpdatePost(context.Background(), postWithFiles.Id, updatePost) + require.Error(t, err) + require.Equal(t, http.StatusForbidden, resp.StatusCode) + require.Equal(t, "You do not have the appropriate permissions.", err.Error()) + CheckForbiddenStatus(t, resp) + }) + + t.Run("should allow updating post with unchanged files when edit_file_attachment permission is revoked", func(t *testing.T) { + th.LoginBasic(t) + + fileResp, _, err := client.UploadFile(context.Background(), data, channel.Id, "test.png") + require.NoError(t, err) + fileId := fileResp.FileInfos[0].Id + + postWithFiles, _, appErr := th.App.CreatePost(th.Context, &model.Post{ + UserId: th.BasicUser.Id, + ChannelId: channel.Id, + Message: "Post with files", + FileIds: model.StringArray{fileId}, + }, channel, model.CreatePostFlags{SetOnline: true}) + require.Nil(t, appErr) + + th.RemovePermissionFromRole(t, model.PermissionEditFileAttachment.Id, model.ChannelUserRoleId) + defer th.AddPermissionToRole(t, model.PermissionEditFileAttachment.Id, model.ChannelUserRoleId) + + updatePost := &model.Post{ + Id: postWithFiles.Id, + ChannelId: channel.Id, + Message: "Updated message only", + FileIds: model.StringArray{fileId}, + } + updatedPost, resp, err := client.UpdatePost(context.Background(), postWithFiles.Id, updatePost) + require.NoError(t, err) + CheckOKStatus(t, resp) + require.NotNil(t, updatedPost) + assert.Equal(t, "Updated message only", updatedPost.Message) + }) + + t.Run("should allow changing files when edit_file_attachment permission is present", func(t *testing.T) { + th.LoginBasic(t) + + fileResp, _, err := client.UploadFile(context.Background(), data, channel.Id, "test.png") + require.NoError(t, err) + fileId := fileResp.FileInfos[0].Id + + postWithoutFiles, _, appErr := th.App.CreatePost(th.Context, &model.Post{ + UserId: th.BasicUser.Id, + ChannelId: channel.Id, + Message: "Post without files", + }, channel, model.CreatePostFlags{SetOnline: true}) + require.Nil(t, appErr) + + updatePost := &model.Post{ + Id: postWithoutFiles.Id, + ChannelId: channel.Id, + Message: "Updated post with file", + FileIds: model.StringArray{fileId}, + } + updatedPost, resp, err := client.UpdatePost(context.Background(), postWithoutFiles.Id, updatePost) + require.NoError(t, err) + CheckOKStatus(t, resp) + require.NotNil(t, updatedPost) + }) + t.Run("should be able to add and remove files simultaneously", func(t *testing.T) { th.LoginBasic(t) // create new file @@ -2218,6 +2336,129 @@ func TestPatchPost(t *testing.T) { CheckForbiddenStatus(t, resp) }) + t.Run("should prevent patching file ids when edit_file_attachment permission is revoked", func(t *testing.T) { + th.LoginBasic(t) + + fileResp, _, err := client.UploadFile(context.Background(), data, channel.Id, "test.png") + require.NoError(t, err) + fileId := fileResp.FileInfos[0].Id + + postToEdit, _, err := client.CreatePost(context.Background(), &model.Post{ + ChannelId: channel.Id, + Message: "original message", + }) + require.NoError(t, err) + + th.RemovePermissionFromRole(t, model.PermissionEditFileAttachment.Id, model.ChannelUserRoleId) + defer th.AddPermissionToRole(t, model.PermissionEditFileAttachment.Id, model.ChannelUserRoleId) + + patch := &model.PostPatch{ + FileIds: &model.StringArray{fileId}, + } + _, resp, err := client.PatchPost(context.Background(), postToEdit.Id, patch) + require.Error(t, err) + CheckForbiddenStatus(t, resp) + }) + + t.Run("should prevent removing files via patch when edit_file_attachment permission is revoked", func(t *testing.T) { + th.LoginBasic(t) + + fileResp, _, err := client.UploadFile(context.Background(), data, channel.Id, "test.png") + require.NoError(t, err) + fileId := fileResp.FileInfos[0].Id + + postToEdit, _, err := client.CreatePost(context.Background(), &model.Post{ + ChannelId: channel.Id, + Message: "post with file", + FileIds: model.StringArray{fileId}, + }) + require.NoError(t, err) + + th.RemovePermissionFromRole(t, model.PermissionEditFileAttachment.Id, model.ChannelUserRoleId) + defer th.AddPermissionToRole(t, model.PermissionEditFileAttachment.Id, model.ChannelUserRoleId) + + emptyFileIds := model.StringArray{} + patch := &model.PostPatch{ + FileIds: &emptyFileIds, + } + _, resp, err := client.PatchPost(context.Background(), postToEdit.Id, patch) + require.Error(t, err) + CheckForbiddenStatus(t, resp) + }) + + t.Run("should allow patching message without file change when edit_file_attachment permission is revoked", func(t *testing.T) { + th.LoginBasic(t) + + fileResp, _, err := client.UploadFile(context.Background(), data, channel.Id, "test.png") + require.NoError(t, err) + fileId := fileResp.FileInfos[0].Id + + postToEdit, _, err := client.CreatePost(context.Background(), &model.Post{ + ChannelId: channel.Id, + Message: "original message", + FileIds: model.StringArray{fileId}, + }) + require.NoError(t, err) + + th.RemovePermissionFromRole(t, model.PermissionEditFileAttachment.Id, model.ChannelUserRoleId) + defer th.AddPermissionToRole(t, model.PermissionEditFileAttachment.Id, model.ChannelUserRoleId) + + patch := &model.PostPatch{ + Message: model.NewPointer("updated message only"), + } + patchedPost, _, err := client.PatchPost(context.Background(), postToEdit.Id, patch) + require.NoError(t, err) + assert.Equal(t, "updated message only", patchedPost.Message) + }) + + t.Run("should allow patching with same file ids when edit_file_attachment permission is revoked", func(t *testing.T) { + th.LoginBasic(t) + + fileResp, _, err := client.UploadFile(context.Background(), data, channel.Id, "test.png") + require.NoError(t, err) + fileId := fileResp.FileInfos[0].Id + + postToEdit, _, err := client.CreatePost(context.Background(), &model.Post{ + ChannelId: channel.Id, + Message: "original message", + FileIds: model.StringArray{fileId}, + }) + require.NoError(t, err) + + th.RemovePermissionFromRole(t, model.PermissionEditFileAttachment.Id, model.ChannelUserRoleId) + defer th.AddPermissionToRole(t, model.PermissionEditFileAttachment.Id, model.ChannelUserRoleId) + + sameFileIds := model.StringArray{fileId} + patch := &model.PostPatch{ + Message: model.NewPointer("updated message"), + FileIds: &sameFileIds, + } + patchedPost, _, err := client.PatchPost(context.Background(), postToEdit.Id, patch) + require.NoError(t, err) + assert.Equal(t, "updated message", patchedPost.Message) + }) + + t.Run("should allow patching files when edit_file_attachment permission is present", func(t *testing.T) { + th.LoginBasic(t) + + fileResp, _, err := client.UploadFile(context.Background(), data, channel.Id, "test.png") + require.NoError(t, err) + fileId := fileResp.FileInfos[0].Id + + postToEdit, _, err := client.CreatePost(context.Background(), &model.Post{ + ChannelId: channel.Id, + Message: "original message", + }) + require.NoError(t, err) + + patch := &model.PostPatch{ + FileIds: &model.StringArray{fileId}, + } + patchedPost, _, err := client.PatchPost(context.Background(), postToEdit.Id, patch) + require.NoError(t, err) + require.NotNil(t, patchedPost) + }) + t.Run("time limit expired", func(t *testing.T) { th.App.UpdateConfig(func(cfg *model.Config) { *cfg.ServiceSettings.PostEditTimeLimit = 1 diff --git a/server/channels/api4/post_utils.go b/server/channels/api4/post_utils.go index 9fc40da80be..b7332f3f940 100644 --- a/server/channels/api4/post_utils.go +++ b/server/channels/api4/post_utils.go @@ -6,6 +6,7 @@ package api4 import ( "github.com/mattermost/mattermost/server/public/model" "github.com/mattermost/mattermost/server/v8/channels/app" + "github.com/mattermost/mattermost/server/v8/channels/utils" ) func userCreatePostPermissionCheckWithContext(c *Context, channelId string) { @@ -77,3 +78,14 @@ func checkUploadFilePermissionForNewFiles(c *Context, newFileIds []string, origi } } } + +// checkEditFileAttachmentPermission checks edit_file_attachment permission +// when file IDs are being changed (files added or removed) during post edit. +func checkEditFileAttachmentPermission(c *Context, newFileIds []string, originalPost *model.Post) { + if utils.SliceEqualUnordered(newFileIds, originalPost.FileIds) { + return + } + if ok, _ := c.App.SessionHasPermissionToChannel(c.AppContext, *c.AppContext.Session(), originalPost.ChannelId, model.PermissionEditFileAttachment); !ok { + c.SetPermissionError(model.PermissionEditFileAttachment) + } +} diff --git a/server/channels/api4/post_utils_test.go b/server/channels/api4/post_utils_test.go new file mode 100644 index 00000000000..2f0f1bc39f0 --- /dev/null +++ b/server/channels/api4/post_utils_test.go @@ -0,0 +1,94 @@ +// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. +// See LICENSE.txt for license information. + +package api4 + +import ( + "testing" + + "github.com/mattermost/mattermost/server/v8/channels/utils" + "github.com/stretchr/testify/assert" +) + +func TestSameFileIDs(t *testing.T) { + tests := []struct { + name string + a []string + b []string + expected bool + }{ + { + name: "both empty", + a: []string{}, + b: []string{}, + expected: true, + }, + { + name: "both nil", + a: nil, + b: nil, + expected: true, + }, + { + name: "same files same order", + a: []string{"file1", "file2", "file3"}, + b: []string{"file1", "file2", "file3"}, + expected: true, + }, + { + name: "same files different order", + a: []string{"file3", "file1", "file2"}, + b: []string{"file1", "file2", "file3"}, + expected: true, + }, + { + name: "one file added", + a: []string{"file1", "file2", "file3"}, + b: []string{"file1", "file2"}, + expected: false, + }, + { + name: "one file removed", + a: []string{"file1"}, + b: []string{"file1", "file2"}, + expected: false, + }, + { + name: "different files same length", + a: []string{"file1", "file2"}, + b: []string{"file1", "file3"}, + expected: false, + }, + { + name: "duplicate IDs in a", + a: []string{"file1", "file1"}, + b: []string{"file1", "file2"}, + expected: false, + }, + { + name: "duplicate IDs same in both", + a: []string{"file1", "file1"}, + b: []string{"file1", "file1"}, + expected: true, + }, + { + name: "empty vs non-empty", + a: []string{}, + b: []string{"file1"}, + expected: false, + }, + { + name: "nil vs non-empty", + a: nil, + b: []string{"file1"}, + expected: false, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + result := utils.SliceEqualUnordered(tc.a, tc.b) + assert.Equal(t, tc.expected, result) + }) + } +} diff --git a/server/channels/app/app_test.go b/server/channels/app/app_test.go index 85de4883a8b..822f4a3414c 100644 --- a/server/channels/app/app_test.go +++ b/server/channels/app/app_test.go @@ -125,6 +125,7 @@ func TestDoAdvancedPermissionsMigration(t *testing.T) { model.PermissionManagePrivateChannelMembers.Id, model.PermissionDeletePost.Id, model.PermissionEditPost.Id, + model.PermissionEditFileAttachment.Id, model.PermissionAddBookmarkPublicChannel.Id, model.PermissionEditBookmarkPublicChannel.Id, model.PermissionDeleteBookmarkPublicChannel.Id, diff --git a/server/channels/app/permissions_migrations.go b/server/channels/app/permissions_migrations.go index 028ec0762b3..cfa898a8e27 100644 --- a/server/channels/app/permissions_migrations.go +++ b/server/channels/app/permissions_migrations.go @@ -1293,6 +1293,15 @@ func (a *App) getRestoreManageOAuthPermissionMigration() (permissionsMap, error) }, nil } +func (a *App) getAddEditFileAttachmentPermissionMigration() (permissionsMap, error) { + return permissionsMap{ + permissionTransformation{ + On: permissionExists(model.PermissionEditPost.Id), + Add: []string{model.PermissionEditFileAttachment.Id}, + }, + }, nil +} + // DoPermissionsMigrations execute all the permissions migrations need by the current version. func (a *App) DoPermissionsMigrations() error { return a.Srv().doPermissionsMigrations() @@ -1353,6 +1362,7 @@ func (s *Server) doPermissionsMigrations() error { {Key: model.MigrationKeyAddSharedChannelManagerPermissions, Migration: a.getAddSharedChannelManagerPermissionsMigration}, {Key: model.MigrationKeyAddSecureConnectionManagerPermissions, Migration: a.getAddSecureConnectionManagerPermissionsMigration}, {Key: model.MigrationKeyRestoreManageOAuthPermission, Migration: a.getRestoreManageOAuthPermissionMigration}, + {Key: model.MigrationKeyAddEditFileAttachmentPermission, Migration: a.getAddEditFileAttachmentPermissionMigration}, } roles, err := s.Store().Role().GetAll() diff --git a/server/channels/testlib/store.go b/server/channels/testlib/store.go index 9edf15ce825..5a5f2e6273f 100644 --- a/server/channels/testlib/store.go +++ b/server/channels/testlib/store.go @@ -96,6 +96,7 @@ func GetMockStoreForSetupFunctions() *mocks.Store { systemStore.On("GetByName", model.MigrationKeyAddChannelAccessRulesPermission).Return(&model.System{Name: model.MigrationKeyAddChannelAccessRulesPermission, Value: "true"}, nil) systemStore.On("GetByName", model.MigrationKeyAddChannelAutoTranslationPermissions).Return(&model.System{Name: model.MigrationKeyAddChannelAutoTranslationPermissions, Value: "true"}, nil) systemStore.On("GetByName", model.MigrationKeyRestoreManageOAuthPermission).Return(&model.System{Name: model.MigrationKeyRestoreManageOAuthPermission, Value: "true"}, nil) + systemStore.On("GetByName", model.MigrationKeyAddEditFileAttachmentPermission).Return(&model.System{Name: model.MigrationKeyAddEditFileAttachmentPermission, Value: "true"}, nil) systemStore.On("InsertIfExists", mock.AnythingOfType("*model.System")).Return(&model.System{}, nil).Once() systemStore.On("Save", mock.AnythingOfType("*model.System")).Return(nil) diff --git a/server/channels/utils/utils.go b/server/channels/utils/utils.go index f9748c2e93e..5be9a754411 100644 --- a/server/channels/utils/utils.go +++ b/server/channels/utils/utils.go @@ -277,3 +277,22 @@ func RoundOffToZeroesResolution(n float64, minResolution int) int64 { significantDigits := int64(n) / tens return significantDigits * tens } + +// SliceEqualUnordered returns true if both slices contain the same set of elements, +// regardless of order. +func SliceEqualUnordered[K comparable](a, b []K) bool { + if len(a) != len(b) { + return false + } + set := make(map[K]int, len(a)) + for _, id := range a { + set[id]++ + } + for _, id := range b { + set[id]-- + if set[id] < 0 { + return false + } + } + return true +} diff --git a/server/public/model/migration.go b/server/public/model/migration.go index 816f366657f..77f14c23f87 100644 --- a/server/public/model/migration.go +++ b/server/public/model/migration.go @@ -61,4 +61,5 @@ const ( MigrationKeyAddSharedChannelManagerPermissions = "shared_channel_manager_permissions" MigrationKeyAddSecureConnectionManagerPermissions = "secure_connection_manager_permissions" MigrationKeyRestoreManageOAuthPermission = "restore_manage_oauth_permission" + MigrationKeyAddEditFileAttachmentPermission = "add_edit_file_attachment_permission" ) diff --git a/server/public/model/permission.go b/server/public/model/permission.go index 1bbfee97e53..f90ba878255 100644 --- a/server/public/model/permission.go +++ b/server/public/model/permission.go @@ -177,6 +177,7 @@ var PermissionManageLicenseInformation *Permission var PermissionManagePublicChannelBanner *Permission var PermissionManagePrivateChannelBanner *Permission var PermissionManageChannelAccessRules *Permission +var PermissionEditFileAttachment *Permission var PermissionSysconsoleReadAbout *Permission var PermissionSysconsoleWriteAbout *Permission @@ -1356,6 +1357,13 @@ func initializePermissions() { PermissionScopeChannel, } + PermissionEditFileAttachment = &Permission{ + "edit_file_attachment", + "", + "", + PermissionScopeChannel, + } + PermissionReadOtherUsersTeams = &Permission{ "read_other_users_teams", "authentication.permissions.read_other_users_teams.name", @@ -2597,6 +2605,7 @@ func initializePermissions() { PermissionManagePublicChannelBanner, PermissionManagePrivateChannelBanner, PermissionManageChannelAccessRules, + PermissionEditFileAttachment, } GroupScopedPermissions := []*Permission{ diff --git a/server/public/model/role.go b/server/public/model/role.go index 3f8d48565f5..1d5f0fc3934 100644 --- a/server/public/model/role.go +++ b/server/public/model/role.go @@ -876,6 +876,7 @@ func MakeDefaultRoles() map[string]*Role { PermissionEditPost.Id, PermissionCreatePost.Id, PermissionUseChannelMentions.Id, + PermissionEditFileAttachment.Id, }, SchemeManaged: true, BuiltIn: true, @@ -902,6 +903,7 @@ func MakeDefaultRoles() map[string]*Role { PermissionManagePrivateChannelMembers.Id, PermissionDeletePost.Id, PermissionEditPost.Id, + PermissionEditFileAttachment.Id, PermissionAddBookmarkPublicChannel.Id, PermissionEditBookmarkPublicChannel.Id, PermissionDeleteBookmarkPublicChannel.Id, diff --git a/webapp/channels/src/components/admin_console/permission_schemes_settings/guest_permissions_tree/guest_permissions_tree.test.tsx b/webapp/channels/src/components/admin_console/permission_schemes_settings/guest_permissions_tree/guest_permissions_tree.test.tsx index 3b75fc3dc1b..e127b4e5001 100644 --- a/webapp/channels/src/components/admin_console/permission_schemes_settings/guest_permissions_tree/guest_permissions_tree.test.tsx +++ b/webapp/channels/src/components/admin_console/permission_schemes_settings/guest_permissions_tree/guest_permissions_tree.test.tsx @@ -71,6 +71,7 @@ describe('components/admin_console/permission_schemes_settings/guest_permissions expect(permissionIds).toContain('guest_create_post'); expect(permissionIds).toContain('guest_reactions'); expect(permissionIds).toContain('guest_use_channel_mentions'); + expect(permissionIds).toContain('guest_edit_file_attachment'); if (includeGroupMentions) { expect(permissionIds).toContain('guest_use_group_mentions'); diff --git a/webapp/channels/src/components/admin_console/permission_schemes_settings/guest_permissions_tree/guest_permissions_tree.tsx b/webapp/channels/src/components/admin_console/permission_schemes_settings/guest_permissions_tree/guest_permissions_tree.tsx index da50c651c99..d7cff0943d9 100644 --- a/webapp/channels/src/components/admin_console/permission_schemes_settings/guest_permissions_tree/guest_permissions_tree.tsx +++ b/webapp/channels/src/components/admin_console/permission_schemes_settings/guest_permissions_tree/guest_permissions_tree.tsx @@ -32,6 +32,7 @@ const GuestPermissionsTree = ({license, onToggle, readOnly, scope, selectRow, pa const defaultPermissions = [ Permissions.CREATE_PRIVATE_CHANNEL, Permissions.EDIT_POST, + Permissions.EDIT_FILE_ATTACHMENT, Permissions.DELETE_POST, { id: 'guest_' + Permissions.CREATE_POST, diff --git a/webapp/channels/src/components/admin_console/permission_schemes_settings/guest_permissions_tree/index.tsx b/webapp/channels/src/components/admin_console/permission_schemes_settings/guest_permissions_tree/index.tsx index 04b7cbae203..6ad5ce10239 100644 --- a/webapp/channels/src/components/admin_console/permission_schemes_settings/guest_permissions_tree/index.tsx +++ b/webapp/channels/src/components/admin_console/permission_schemes_settings/guest_permissions_tree/index.tsx @@ -18,6 +18,7 @@ export const GUEST_INCLUDED_PERMISSIONS = [ Permissions.REMOVE_REACTION, Permissions.READ_CHANNEL, Permissions.UPLOAD_FILE, + Permissions.EDIT_FILE_ATTACHMENT, Permissions.USE_CHANNEL_MENTIONS, Permissions.USE_GROUP_MENTIONS, Permissions.CREATE_POST, diff --git a/webapp/channels/src/components/admin_console/permission_schemes_settings/permissions_tree/__snapshots__/permissions_tree.test.tsx.snap b/webapp/channels/src/components/admin_console/permission_schemes_settings/permissions_tree/__snapshots__/permissions_tree.test.tsx.snap index dfd509ad3f4..3be1d59394c 100644 --- a/webapp/channels/src/components/admin_console/permission_schemes_settings/permissions_tree/__snapshots__/permissions_tree.test.tsx.snap +++ b/webapp/channels/src/components/admin_console/permission_schemes_settings/permissions_tree/__snapshots__/permissions_tree.test.tsx.snap @@ -128,6 +128,7 @@ exports[`components/admin_console/permission_schemes_settings/permission_tree sh "edit_others_posts", ], }, + "edit_file_attachment", Object { "id": "delete_posts", "permissions": Array [ @@ -322,6 +323,7 @@ exports[`components/admin_console/permission_schemes_settings/permission_tree sh "edit_others_posts", ], }, + "edit_file_attachment", Object { "id": "delete_posts", "permissions": Array [ @@ -532,6 +534,7 @@ exports[`components/admin_console/permission_schemes_settings/permission_tree sh "edit_others_posts", ], }, + "edit_file_attachment", Object { "id": "delete_posts", "permissions": Array [ @@ -753,6 +756,7 @@ exports[`components/admin_console/permission_schemes_settings/permission_tree sh "edit_others_posts", ], }, + "edit_file_attachment", Object { "id": "delete_posts", "permissions": Array [ @@ -974,6 +978,7 @@ exports[`components/admin_console/permission_schemes_settings/permission_tree sh "edit_others_posts", ], }, + "edit_file_attachment", Object { "id": "delete_posts", "permissions": Array [ @@ -1195,6 +1200,7 @@ exports[`components/admin_console/permission_schemes_settings/permission_tree sh "edit_others_posts", ], }, + "edit_file_attachment", Object { "id": "delete_posts", "permissions": Array [ @@ -1423,6 +1429,7 @@ exports[`components/admin_console/permission_schemes_settings/permission_tree sh "edit_others_posts", ], }, + "edit_file_attachment", Object { "id": "delete_posts", "permissions": Array [ diff --git a/webapp/channels/src/components/admin_console/permission_schemes_settings/permissions_tree/permissions_tree.test.tsx b/webapp/channels/src/components/admin_console/permission_schemes_settings/permissions_tree/permissions_tree.test.tsx index e78f260f7d0..d4f480e5da7 100644 --- a/webapp/channels/src/components/admin_console/permission_schemes_settings/permissions_tree/permissions_tree.test.tsx +++ b/webapp/channels/src/components/admin_console/permission_schemes_settings/permissions_tree/permissions_tree.test.tsx @@ -130,6 +130,17 @@ describe('components/admin_console/permission_schemes_settings/permission_tree', } }); + test('should include edit_file_attachment in the posts permission group', () => { + const wrapper = shallow( + , + ); + + const groups = wrapper.find(PermissionGroup).first().prop('permissions') as Array; + const postsGroup = groups[6]; + expect(postsGroup.id).toBe('posts'); + expect(postsGroup.permissions).toContain('edit_file_attachment'); + }); + test('should hide disabbled integration options', () => { const wrapper = shallow( { Permissions.EDIT_OTHERS_POSTS, ], }, + Permissions.EDIT_FILE_ATTACHMENT, { id: 'delete_posts', permissions: [ diff --git a/webapp/channels/src/components/admin_console/permission_schemes_settings/strings/groups.tsx b/webapp/channels/src/components/admin_console/permission_schemes_settings/strings/groups.tsx index 48d91cf8dd4..d726950bdb5 100644 --- a/webapp/channels/src/components/admin_console/permission_schemes_settings/strings/groups.tsx +++ b/webapp/channels/src/components/admin_console/permission_schemes_settings/strings/groups.tsx @@ -165,6 +165,16 @@ export const groupRolesStrings: Record defaultMessage: 'Add and delete reactions on posts.', }, }), + guest_edit_file_attachment: defineMessages({ + name: { + id: 'admin.permissions.group.guest_edit_file_attachment.name', + defaultMessage: 'Edit Attachments', + }, + description: { + id: 'admin.permissions.group.guest_edit_file_attachment.description', + defaultMessage: 'Allow users to add or remove file attachments when editing posts.', + }, + }), guest_create_post: defineMessages({ name: { id: 'admin.permissions.group.guest_create_post.name', diff --git a/webapp/channels/src/components/admin_console/permission_schemes_settings/strings/permissions.tsx b/webapp/channels/src/components/admin_console/permission_schemes_settings/strings/permissions.tsx index e7dea308eb4..f0c35d4a7c6 100644 --- a/webapp/channels/src/components/admin_console/permission_schemes_settings/strings/permissions.tsx +++ b/webapp/channels/src/components/admin_console/permission_schemes_settings/strings/permissions.tsx @@ -495,6 +495,16 @@ export const permissionRolesStrings: Record { @@ -36,12 +41,25 @@ const useUploadFiles = ( setServerError: (err: (ServerError & { submittedMessage?: string }) | null) => void, isPostBeingEdited?: boolean, ): [React.ReactNode, React.ReactNode] => { + const intl = useIntl(); const locale = useSelector(getCurrentLocale); + const canEditAttachments = useSelector((state: GlobalState) => { + const channel = getChannel(state, channelId); + return channel ? haveIChannelPermission(state, channel.team_id, channel.id, Permissions.EDIT_FILE_ATTACHMENT) : true; + }); + const editAttachmentsDisabled = isPostBeingEdited && !canEditAttachments; const [uploadsProgressPercent, setUploadsProgressPercent] = useState<{ [clientID: string]: FilePreviewInfo }>({}); const fileUploadRef = useRef(null); + const removeTooltip = useMemo(() => { + return editAttachmentsDisabled ? intl.formatMessage({ + id: 'file_preview.no_edit_permission', + defaultMessage: 'Post attachments cannot be edited', + }) : undefined; + }, [editAttachmentsDisabled, intl]); + const handleFileUploadChange = useCallback(() => { focusTextbox(); }, [focusTextbox]); @@ -146,9 +164,10 @@ const useUploadFiles = ( attachmentPreview = ( ); } @@ -160,7 +179,7 @@ const useUploadFiles = ( postType = isThreadView ? 'thread' : 'comment'; } - const fileUploadJSX = isDisabled ? null : ( + const fileUploadJSX = (isDisabled || editAttachmentsDisabled) ? null : ( { expect(screen.getByAltText('file preview')).toHaveAttribute('src', getFileUrl(fileId)); }); + + test('should render disabled remove button with tooltip when onRemove is absent and disabledRemoveTooltip is provided', async () => { + const props = { + ...baseProps, + onRemove: undefined, + uploadsInProgress: [], + disabledRemoveTooltip: 'You do not have permission to edit file attachments', + }; + + const {container} = renderWithContext( + , + ); + + const disabledRemove = container.querySelector('.file-preview__remove--disabled'); + expect(disabledRemove).toBeInTheDocument(); + expect(container.querySelector('a.file-preview__remove')).not.toBeInTheDocument(); + if (!disabledRemove) { + throw new Error('Disabled remove button not found'); + } + + const user = userEvent.setup(); + await user.hover(disabledRemove); + expect(await screen.findByText('You do not have permission to edit file attachments')).toBeInTheDocument(); + + expect(container.querySelector('a.file-preview__remove')).not.toBeInTheDocument(); + }); + + test('should render normal remove button when onRemove is defined', () => { + const props = { + ...baseProps, + onRemove: jest.fn(), + uploadsInProgress: [], + disabledRemoveTooltip: 'tooltip text', + }; + + const {container} = renderWithContext( + , + ); + + expect(container.querySelector('a.file-preview__remove')).toBeInTheDocument(); + expect(container.querySelector('.file-preview__remove--disabled')).not.toBeInTheDocument(); + }); + + test('should not render any remove button when onRemove is absent and no disabledRemoveTooltip', () => { + const props = { + ...baseProps, + onRemove: undefined, + uploadsInProgress: [], + disabledRemoveTooltip: undefined, + }; + + const {container} = renderWithContext( + , + ); + + expect(container.querySelector('a.file-preview__remove')).not.toBeInTheDocument(); + expect(container.querySelector('.file-preview__remove--disabled')).not.toBeInTheDocument(); + }); }); diff --git a/webapp/channels/src/components/file_preview/file_preview.tsx b/webapp/channels/src/components/file_preview/file_preview.tsx index 85eba9c429f..0ca9c5069ba 100644 --- a/webapp/channels/src/components/file_preview/file_preview.tsx +++ b/webapp/channels/src/components/file_preview/file_preview.tsx @@ -9,6 +9,7 @@ import type {FileInfo} from '@mattermost/types/files'; import {getFileThumbnailUrl, getFileUrl} from 'mattermost-redux/utils/file_utils'; import FilenameOverlay from 'components/file_attachment/filename_overlay'; +import WithTooltip from 'components/with_tooltip'; import Constants, {FileTypes} from 'utils/constants'; import * as Utils from 'utils/utils'; @@ -28,7 +29,8 @@ type Props = { fileInfos: FilePreviewInfo[]; uploadsInProgress?: string[]; uploadsProgressPercent?: {[clientID: string]: FilePreviewInfo}; -} + disabledRemoveTooltip?: string; +}; export default class FilePreview extends React.PureComponent { static defaultProps = { @@ -90,9 +92,7 @@ export default class FilePreview extends React.PureComponent { key={info.id} className={className} > -
- {previewImage} -
+
{previewImage}
@@ -101,12 +101,18 @@ export default class FilePreview extends React.PureComponent { compactDisplay={false} canDownload={false} /> - {info.extension && {info.extension.toUpperCase()}} - {Utils.fileSizeToString(info.size)} + {info.extension && ( + + {info.extension.toUpperCase()} + + )} + + {Utils.fileSizeToString(info.size)} +
- {Boolean(this.props.onRemove) && ( + {this.props.onRemove && ( { )} + + {!this.props.onRemove && this.props.disabledRemoveTooltip && ( + + + + + + )}
, diff --git a/webapp/channels/src/i18n/en.json b/webapp/channels/src/i18n/en.json index ecf1ada49c3..855eb34f472 100644 --- a/webapp/channels/src/i18n/en.json +++ b/webapp/channels/src/i18n/en.json @@ -1904,6 +1904,8 @@ "admin.permissions.group.guest_create_private_channel.name": "Create Channels", "admin.permissions.group.guest_delete_post.description": "Author's own posts can be deleted.", "admin.permissions.group.guest_delete_post.name": "Delete Own Posts", + "admin.permissions.group.guest_edit_file_attachment.description": "Allow users to add or remove file attachments when editing posts.", + "admin.permissions.group.guest_edit_file_attachment.name": "Edit Attachments", "admin.permissions.group.guest_edit_post.description": "{editTimeLimitButton} after posting, allow users to edit their own posts.", "admin.permissions.group.guest_edit_post.name": "Edit Own Posts", "admin.permissions.group.guest_reactions.description": "Add and delete reactions on posts.", @@ -1998,6 +2000,8 @@ "admin.permissions.permission.delete_public_channel.name": "Archive Channels", "admin.permissions.permission.edit_custom_group.description": "Rename custom groups.", "admin.permissions.permission.edit_custom_group.name": "Edit", + "admin.permissions.permission.edit_file_attachment.description": "Allow users to add or remove file attachments when editing posts.", + "admin.permissions.permission.edit_file_attachment.name": "Edit Attachments", "admin.permissions.permission.edit_other_users.description": "Edit other users", "admin.permissions.permission.edit_other_users.name": "Edit other users", "admin.permissions.permission.edit_others_posts.description": "Allow users to edit others' posts.", @@ -4521,6 +4525,7 @@ "file_preview_modal_main_nav.file": "{count, number} of {total, number}", "file_preview_modal_main_nav.nextAriaLabel": "Next file", "file_preview_modal_main_nav.prevAriaLabel": "Previous file", + "file_preview.no_edit_permission": "Post attachments cannot be edited", "file_search_result_item.copy_link": "Copy link", "file_search_result_item.download": "Download", "file_search_result_item.more_actions": "More Actions", diff --git a/webapp/channels/src/packages/mattermost-redux/src/constants/permissions.ts b/webapp/channels/src/packages/mattermost-redux/src/constants/permissions.ts index af3ae93b283..378afdc8b61 100644 --- a/webapp/channels/src/packages/mattermost-redux/src/constants/permissions.ts +++ b/webapp/channels/src/packages/mattermost-redux/src/constants/permissions.ts @@ -46,6 +46,7 @@ const values = { REMOVE_OTHERS_REACTIONS: 'remove_others_reactions', PERMANENT_DELETE_USER: 'permanent_delete_user', UPLOAD_FILE: 'upload_file', + EDIT_FILE_ATTACHMENT: 'edit_file_attachment', GET_PUBLIC_LINK: 'get_public_link', MANAGE_WEBHOOKS: 'manage_webhooks', MANAGE_OTHERS_WEBHOOKS: 'manage_others_webhooks', diff --git a/webapp/channels/src/sass/components/_files.scss b/webapp/channels/src/sass/components/_files.scss index 3bac076ceb7..a1c629b13ae 100644 --- a/webapp/channels/src/sass/components/_files.scss +++ b/webapp/channels/src/sass/components/_files.scss @@ -69,6 +69,15 @@ background: rgba(var(--button-bg-rgb), 0.08); color: functions.v(button-bg); } + + &--disabled { + cursor: not-allowed; + opacity: 0.5; + + &:hover { + background: none; + } + } } } diff --git a/webapp/channels/src/utils/constants.tsx b/webapp/channels/src/utils/constants.tsx index 461d7e8d31e..d80003f764c 100644 --- a/webapp/channels/src/utils/constants.tsx +++ b/webapp/channels/src/utils/constants.tsx @@ -1162,6 +1162,7 @@ export const PermissionsScope = { [Permissions.REMOVE_OTHERS_REACTIONS]: 'channel_scope', [Permissions.PERMANENT_DELETE_USER]: 'system_scope', [Permissions.UPLOAD_FILE]: 'channel_scope', + [Permissions.EDIT_FILE_ATTACHMENT]: 'channel_scope', [Permissions.GET_PUBLIC_LINK]: 'system_scope', [Permissions.MANAGE_INCOMING_WEBHOOKS]: 'team_scope', [Permissions.MANAGE_OWN_INCOMING_WEBHOOKS]: 'team_scope', @@ -1261,6 +1262,7 @@ export const DefaultRolePermissions = { Permissions.MANAGE_PRIVATE_CHANNEL_MEMBERS, Permissions.DELETE_POST, Permissions.EDIT_POST, + Permissions.EDIT_FILE_ATTACHMENT, Permissions.USE_CHANNEL_MENTIONS, Permissions.USE_GROUP_MENTIONS, Permissions.CREATE_CUSTOM_GROUP, @@ -1355,6 +1357,7 @@ export const DefaultRolePermissions = { ], guests: [ Permissions.EDIT_POST, + Permissions.EDIT_FILE_ATTACHMENT, Permissions.ADD_REACTION, Permissions.REMOVE_REACTION, Permissions.USE_CHANNEL_MENTIONS,