From 36ac3a43b1ce794d0e252f984e459d28e4edb0b7 Mon Sep 17 00:00:00 2001 From: Harshil Sharma <18575143+harshilsharma63@users.noreply.github.com> Date: Mon, 25 May 2026 15:54:05 +0530 Subject: [PATCH] Edit attachment permission (#36227) (#36723) Automatic Merge --- e2e-tests/cypress/tests/support/api/role.js | 4 +- server/channels/api4/data_retention_test.go | 21 +- server/channels/api4/post.go | 11 + 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.snap | 42 +++ .../guest_permissions_tree.tsx | 1 + .../guest_permissions_tree/index.tsx | 1 + .../permissions_tree.test.tsx.snap | 7 + .../permissions_tree.test.tsx | 10 + .../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 | 51 ++++ .../components/file_preview/file_preview.tsx | 31 ++- 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, 602 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 4180581d36..be5376144e 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', - 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 8a81368667..5e53af2d72 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" @@ -1057,28 +1058,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 a49572cf99..5320872a3f 100644 --- a/server/channels/api4/post.go +++ b/server/channels/api4/post.go @@ -1065,6 +1065,12 @@ func updatePost(c *Context, w http.ResponseWriter, r *http.Request) { return } + // Check edit_file_attachment permission if file IDs are being changed (files added or removed) + 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 { @@ -1139,6 +1145,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 d8b1eb764e..6c42ddbb07 100644 --- a/server/channels/api4/post_test.go +++ b/server/channels/api4/post_test.go @@ -1900,6 +1900,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() + + 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(model.PermissionEditFileAttachment.Id, model.ChannelUserRoleId) + defer th.AddPermissionToRole(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() + + 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(model.PermissionEditFileAttachment.Id, model.ChannelUserRoleId) + defer th.AddPermissionToRole(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() + + 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(model.PermissionEditFileAttachment.Id, model.ChannelUserRoleId) + defer th.AddPermissionToRole(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() + + 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() // create new file @@ -2159,6 +2277,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() + + 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(model.PermissionEditFileAttachment.Id, model.ChannelUserRoleId) + defer th.AddPermissionToRole(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() + + 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(model.PermissionEditFileAttachment.Id, model.ChannelUserRoleId) + defer th.AddPermissionToRole(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() + + 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(model.PermissionEditFileAttachment.Id, model.ChannelUserRoleId) + defer th.AddPermissionToRole(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() + + 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(model.PermissionEditFileAttachment.Id, model.ChannelUserRoleId) + defer th.AddPermissionToRole(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() + + 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 edc5ee720d..83980f43cf 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) { @@ -69,3 +70,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 0000000000..2f0f1bc39f --- /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 0dc1b2705b..429fd5d346 100644 --- a/server/channels/app/app_test.go +++ b/server/channels/app/app_test.go @@ -129,6 +129,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 0c11af5ef1..80e3cf5fa1 100644 --- a/server/channels/app/permissions_migrations.go +++ b/server/channels/app/permissions_migrations.go @@ -1206,6 +1206,15 @@ func (a *App) getRestrictAcessToChannelConversionToPublic() (permissionsMap, err }, 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() @@ -1260,6 +1269,7 @@ func (s *Server) doPermissionsMigrations() error { {Key: model.MigrationRemoveGetAnalyticsPermission, Migration: a.removeGetAnalyticsPermissionMigration}, {Key: model.MigrationAddSysconsoleMobileSecurityPermission, Migration: a.addSysConsoleMobileSecurityPermission}, {Key: model.MigrationKeyAddChannelBannerPermissions, Migration: a.getAddChannelBannerPermissionMigration}, + {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 bfcb5588f6..11f61a9182 100644 --- a/server/channels/testlib/store.go +++ b/server/channels/testlib/store.go @@ -87,6 +87,7 @@ func GetMockStoreForSetupFunctions() *mocks.Store { systemStore.On("GetByName", "elasticsearch_fix_channel_index_migration").Return(&model.System{Name: "elasticsearch_fix_channel_index_migration", Value: "true"}, nil) systemStore.On("GetByName", model.MigrationAddSysconsoleMobileSecurityPermission).Return(&model.System{Name: model.MigrationAddSysconsoleMobileSecurityPermission, Value: "true"}, nil) systemStore.On("GetByName", model.MigrationKeyAddChannelBannerPermissions).Return(&model.System{Name: model.MigrationKeyAddChannelBannerPermissions, 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 b66b30a052..8666677a8c 100644 --- a/server/channels/utils/utils.go +++ b/server/channels/utils/utils.go @@ -264,3 +264,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 b4756f4e44..61595f81fa 100644 --- a/server/public/model/migration.go +++ b/server/public/model/migration.go @@ -55,4 +55,5 @@ const ( MigrationRemoveGetAnalyticsPermission = "remove_get_analytics_permission" MigrationAddSysconsoleMobileSecurityPermission = "add_sysconsole_mobile_security_permission" MigrationKeyAddChannelBannerPermissions = "add_channel_banner_permissions" + MigrationKeyAddEditFileAttachmentPermission = "add_edit_file_attachment_permission" ) diff --git a/server/public/model/permission.go b/server/public/model/permission.go index 863ecc4c57..0d176ff35d 100644 --- a/server/public/model/permission.go +++ b/server/public/model/permission.go @@ -169,6 +169,7 @@ var PermissionReadLicenseInformation *Permission var PermissionManageLicenseInformation *Permission var PermissionManagePublicChannelBanner *Permission var PermissionManagePrivateChannelBanner *Permission +var PermissionEditFileAttachment *Permission var PermissionSysconsoleReadAbout *Permission var PermissionSysconsoleWriteAbout *Permission @@ -1299,6 +1300,13 @@ func initializePermissions() { PermissionScopeChannel, } + PermissionEditFileAttachment = &Permission{ + "edit_file_attachment", + "", + "", + PermissionScopeChannel, + } + PermissionReadOtherUsersTeams = &Permission{ "read_other_users_teams", "authentication.permissions.read_other_users_teams.name", @@ -2538,6 +2546,7 @@ func initializePermissions() { PermissionOrderBookmarkPrivateChannel, PermissionManagePublicChannelBanner, PermissionManagePrivateChannelBanner, + PermissionEditFileAttachment, } GroupScopedPermissions := []*Permission{ diff --git a/server/public/model/role.go b/server/public/model/role.go index 8ae0dc5e04..0cf05daffb 100644 --- a/server/public/model/role.go +++ b/server/public/model/role.go @@ -866,6 +866,7 @@ func MakeDefaultRoles() map[string]*Role { PermissionEditPost.Id, PermissionCreatePost.Id, PermissionUseChannelMentions.Id, + PermissionEditFileAttachment.Id, }, SchemeManaged: true, BuiltIn: true, @@ -892,6 +893,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/__snapshots__/guest_permissions_tree.test.tsx.snap b/webapp/channels/src/components/admin_console/permission_schemes_settings/guest_permissions_tree/__snapshots__/guest_permissions_tree.test.tsx.snap index 48916e3485..e34f1bcbcd 100644 --- a/webapp/channels/src/components/admin_console/permission_schemes_settings/guest_permissions_tree/__snapshots__/guest_permissions_tree.test.tsx.snap +++ b/webapp/channels/src/components/admin_console/permission_schemes_settings/guest_permissions_tree/__snapshots__/guest_permissions_tree.test.tsx.snap @@ -58,6 +58,13 @@ exports[`components/admin_console/permission_schemes_settings/permission_tree sh "edit_post", ], }, + Object { + "combined": true, + "id": "guest_edit_file_attachment", + "permissions": Array [ + "edit_file_attachment", + ], + }, Object { "combined": true, "id": "guest_delete_post", @@ -183,6 +190,13 @@ exports[`components/admin_console/permission_schemes_settings/permission_tree sh "edit_post", ], }, + Object { + "combined": true, + "id": "guest_edit_file_attachment", + "permissions": Array [ + "edit_file_attachment", + ], + }, Object { "combined": true, "id": "guest_delete_post", @@ -301,6 +315,13 @@ exports[`components/admin_console/permission_schemes_settings/permission_tree sh "edit_post", ], }, + Object { + "combined": true, + "id": "guest_edit_file_attachment", + "permissions": Array [ + "edit_file_attachment", + ], + }, Object { "combined": true, "id": "guest_delete_post", @@ -426,6 +447,13 @@ exports[`components/admin_console/permission_schemes_settings/permission_tree sh "edit_post", ], }, + Object { + "combined": true, + "id": "guest_edit_file_attachment", + "permissions": Array [ + "edit_file_attachment", + ], + }, Object { "combined": true, "id": "guest_delete_post", @@ -551,6 +579,13 @@ exports[`components/admin_console/permission_schemes_settings/permission_tree sh "edit_post", ], }, + Object { + "combined": true, + "id": "guest_edit_file_attachment", + "permissions": Array [ + "edit_file_attachment", + ], + }, Object { "combined": true, "id": "guest_delete_post", @@ -692,6 +727,13 @@ exports[`components/admin_console/permission_schemes_settings/permission_tree sh "edit_post", ], }, + Object { + "combined": true, + "id": "guest_edit_file_attachment", + "permissions": Array [ + "edit_file_attachment", + ], + }, Object { "combined": true, "id": "guest_delete_post", 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 eeb9a0de16..e5703b7eb0 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 @@ -30,6 +30,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 04b7cbae20..6ad5ce1023 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 8f50adbcef..9e27b924f9 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 [ @@ -513,6 +515,7 @@ exports[`components/admin_console/permission_schemes_settings/permission_tree sh "edit_others_posts", ], }, + "edit_file_attachment", Object { "id": "delete_posts", "permissions": Array [ @@ -715,6 +718,7 @@ exports[`components/admin_console/permission_schemes_settings/permission_tree sh "edit_others_posts", ], }, + "edit_file_attachment", Object { "id": "delete_posts", "permissions": Array [ @@ -917,6 +921,7 @@ exports[`components/admin_console/permission_schemes_settings/permission_tree sh "edit_others_posts", ], }, + "edit_file_attachment", Object { "id": "delete_posts", "permissions": Array [ @@ -1119,6 +1124,7 @@ exports[`components/admin_console/permission_schemes_settings/permission_tree sh "edit_others_posts", ], }, + "edit_file_attachment", Object { "id": "delete_posts", "permissions": Array [ @@ -1328,6 +1334,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 f13b22de91..04f7be2c21 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,16 @@ 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; + expect(groups[6].id).toBe('posts'); + expect(groups[6].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 f3291975c6..61934a914b 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 @@ -125,6 +125,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 2ce60d406a..a90ae6aba5 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 @@ -405,6 +405,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 : ( { @@ -141,4 +143,53 @@ describe('FilePreview', () => { expect(wrapper.find('img').find({src: getFileUrl(fileId)}).exists()).toBe(true); expect(wrapper.find('div').find('.file-icon.generic').exists()).toBe(false); }); + + test('should render disabled remove button with tooltip when onRemove is absent and disabledRemoveTooltip is provided', () => { + const props = { + ...baseProps, + onRemove: undefined, + uploadsInProgress: [], + disabledRemoveTooltip: 'You do not have permission to edit file attachments', + }; + + const wrapper = shallow( + , + ); + + expect(wrapper.find('.file-preview__remove--disabled').exists()).toBe(true); + expect(wrapper.find('a.file-preview__remove').exists()).toBe(false); + expect(wrapper.find(WithTooltip).prop('title')).toBe('You do not have permission to edit file attachments'); + }); + + test('should render normal remove button when onRemove is defined', () => { + const props = { + ...baseProps, + onRemove: jest.fn(), + uploadsInProgress: [], + disabledRemoveTooltip: 'tooltip text', + }; + + const wrapper = shallow( + , + ); + + expect(wrapper.find('a.file-preview__remove').exists()).toBe(true); + expect(wrapper.find('.file-preview__remove--disabled').exists()).toBe(false); + }); + + test('should not render any remove button when onRemove is absent and no disabledRemoveTooltip', () => { + const props = { + ...baseProps, + onRemove: undefined, + uploadsInProgress: [], + disabledRemoveTooltip: undefined, + }; + + const wrapper = shallow( + , + ); + + expect(wrapper.find('a.file-preview__remove').exists()).toBe(false); + expect(wrapper.find('.file-preview__remove--disabled').exists()).toBe(false); + }); }); diff --git a/webapp/channels/src/components/file_preview/file_preview.tsx b/webapp/channels/src/components/file_preview/file_preview.tsx index 85eba9c429..0166fc8459 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 f2d89434c1..ffad609401 100644 --- a/webapp/channels/src/i18n/en.json +++ b/webapp/channels/src/i18n/en.json @@ -1857,6 +1857,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.", @@ -1941,6 +1943,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.", @@ -4229,6 +4233,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 8c0b5c1403..b615c21a3c 100644 --- a/webapp/channels/src/packages/mattermost-redux/src/constants/permissions.ts +++ b/webapp/channels/src/packages/mattermost-redux/src/constants/permissions.ts @@ -42,6 +42,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 3bac076ceb..a1c629b13a 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 bec02710e0..8e2322bc9a 100644 --- a/webapp/channels/src/utils/constants.tsx +++ b/webapp/channels/src/utils/constants.tsx @@ -1254,6 +1254,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_OTHERS_INCOMING_WEBHOOKS]: 'team_scope', @@ -1350,6 +1351,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, @@ -1439,6 +1441,7 @@ export const DefaultRolePermissions = { ], guests: [ Permissions.EDIT_POST, + Permissions.EDIT_FILE_ATTACHMENT, Permissions.ADD_REACTION, Permissions.REMOVE_REACTION, Permissions.USE_CHANNEL_MENTIONS,