From 0aa7ecd5e89f054ae927b246f2aec4bd6348d42b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jes=C3=BAs=20Espino?= Date: Fri, 9 Feb 2018 16:31:01 +0100 Subject: AllowEditPost and PostEditTimeLimit migration (#8208) * AllowEditPost and PostEditTimeLimit migration * Not set EDIT_POST permission to sysadmin_role if ALLOW_EDIT_POST is configured to NEVER * Remove a bit of code duplication --- api/post_test.go | 13 +++++++++---- api4/post_test.go | 7 ------- app/app_test.go | 8 ++++---- app/apptestlib.go | 1 + app/post.go | 9 +-------- app/post_test.go | 46 ++++++++++++++++++++++++++++++++++++++++++++++ config/default.json | 2 +- model/config.go | 2 +- model/role.go | 1 - utils/authorization.go | 23 +++++++++++++++++++++++ 10 files changed, 86 insertions(+), 26 deletions(-) diff --git a/api/post_test.go b/api/post_test.go index fe0c2a6b7..b88c733db 100644 --- a/api/post_test.go +++ b/api/post_test.go @@ -395,7 +395,13 @@ func TestUpdatePost(t *testing.T) { Client := th.BasicClient channel1 := th.BasicChannel - th.App.UpdateConfig(func(cfg *model.Config) { *cfg.ServiceSettings.AllowEditPost = model.ALLOW_EDIT_POST_ALWAYS }) + // Check the appropriate permissions are enforced. + defaultRolePermissions := th.SaveDefaultRolePermissions() + defer func() { + th.RestoreDefaultRolePermissions(defaultRolePermissions) + }() + + th.AddPermissionToRole(model.PERMISSION_EDIT_POST.Id, model.CHANNEL_USER_ROLE_ID) post1 := &model.Post{ChannelId: channel1.Id, Message: "zz" + model.NewId() + "a"} rpost1, err := Client.CreatePost(post1) @@ -474,8 +480,7 @@ func TestUpdatePost(t *testing.T) { utils.SetLicense(&model.License{Features: &model.Features{}}) utils.License().Features.SetDefaults() - th.App.UpdateConfig(func(cfg *model.Config) { *cfg.ServiceSettings.AllowEditPost = model.ALLOW_EDIT_POST_NEVER }) - + th.RemovePermissionFromRole(model.PERMISSION_EDIT_POST.Id, model.CHANNEL_USER_ROLE_ID) post4 := &model.Post{ChannelId: channel1.Id, Message: "zz" + model.NewId() + "a", RootId: rpost1.Data.(*model.Post).Id} rpost4, err := Client.CreatePost(post4) if err != nil { @@ -487,7 +492,7 @@ func TestUpdatePost(t *testing.T) { t.Fatal("shouldn't have been able to update a message when not allowed") } - th.App.UpdateConfig(func(cfg *model.Config) { *cfg.ServiceSettings.AllowEditPost = model.ALLOW_EDIT_POST_TIME_LIMIT }) + th.AddPermissionToRole(model.PERMISSION_EDIT_POST.Id, model.CHANNEL_USER_ROLE_ID) th.App.UpdateConfig(func(cfg *model.Config) { *cfg.ServiceSettings.PostEditTimeLimit = 1 }) //seconds post5 := &model.Post{ChannelId: channel1.Id, Message: "zz" + model.NewId() + "a", RootId: rpost1.Data.(*model.Post).Id} diff --git a/api4/post_test.go b/api4/post_test.go index de627d8bc..52326c4b0 100644 --- a/api4/post_test.go +++ b/api4/post_test.go @@ -489,16 +489,13 @@ func TestUpdatePost(t *testing.T) { isLicensed := utils.IsLicensed() license := utils.License() - allowEditPost := *th.App.Config().ServiceSettings.AllowEditPost defer func() { utils.SetIsLicensed(isLicensed) utils.SetLicense(license) - th.App.UpdateConfig(func(cfg *model.Config) { *cfg.ServiceSettings.AllowEditPost = allowEditPost }) }() utils.SetIsLicensed(true) utils.SetLicense(&model.License{Features: &model.Features{}}) utils.License().Features.SetDefaults() - th.App.UpdateConfig(func(cfg *model.Config) { *cfg.ServiceSettings.AllowEditPost = model.ALLOW_EDIT_POST_ALWAYS }) post := &model.Post{ChannelId: channel.Id, Message: "zz" + model.NewId() + "a"} rpost, resp := Client.CreatePost(post) @@ -571,18 +568,14 @@ func TestPatchPost(t *testing.T) { isLicensed := utils.IsLicensed() license := utils.License() - allowEditPost := *th.App.Config().ServiceSettings.AllowEditPost defer func() { utils.SetIsLicensed(isLicensed) utils.SetLicense(license) - th.App.UpdateConfig(func(cfg *model.Config) { *cfg.ServiceSettings.AllowEditPost = allowEditPost }) }() utils.SetIsLicensed(true) utils.SetLicense(&model.License{Features: &model.Features{}}) utils.License().Features.SetDefaults() - th.App.UpdateConfig(func(cfg *model.Config) { *cfg.ServiceSettings.AllowEditPost = model.ALLOW_EDIT_POST_ALWAYS }) - post := &model.Post{ ChannelId: channel.Id, IsPinned: true, diff --git a/app/app_test.go b/app/app_test.go index 09e002791..bbc012364 100644 --- a/app/app_test.go +++ b/app/app_test.go @@ -109,10 +109,10 @@ func TestDoAdvancedPermissionsMigration(t *testing.T) { model.PERMISSION_UPLOAD_FILE.Id, model.PERMISSION_GET_PUBLIC_LINK.Id, model.PERMISSION_CREATE_POST.Id, - model.PERMISSION_EDIT_POST.Id, model.PERMISSION_USE_SLASH_COMMANDS.Id, model.PERMISSION_MANAGE_PRIVATE_CHANNEL_MEMBERS.Id, model.PERMISSION_DELETE_POST.Id, + model.PERMISSION_EDIT_POST.Id, }, "channel_admin": []string{ model.PERMISSION_MANAGE_CHANNEL_ROLES.Id, @@ -203,7 +203,6 @@ func TestDoAdvancedPermissionsMigration(t *testing.T) { model.PERMISSION_UPLOAD_FILE.Id, model.PERMISSION_GET_PUBLIC_LINK.Id, model.PERMISSION_CREATE_POST.Id, - model.PERMISSION_EDIT_POST.Id, model.PERMISSION_USE_SLASH_COMMANDS.Id, model.PERMISSION_EDIT_OTHERS_POSTS.Id, model.PERMISSION_REMOVE_USER_FROM_TEAM.Id, @@ -214,6 +213,7 @@ func TestDoAdvancedPermissionsMigration(t *testing.T) { model.PERMISSION_MANAGE_SLASH_COMMANDS.Id, model.PERMISSION_MANAGE_OTHERS_SLASH_COMMANDS.Id, model.PERMISSION_MANAGE_WEBHOOKS.Id, + model.PERMISSION_EDIT_POST.Id, }, } @@ -274,10 +274,10 @@ func TestDoAdvancedPermissionsMigration(t *testing.T) { model.PERMISSION_UPLOAD_FILE.Id, model.PERMISSION_GET_PUBLIC_LINK.Id, model.PERMISSION_CREATE_POST.Id, - model.PERMISSION_EDIT_POST.Id, model.PERMISSION_USE_SLASH_COMMANDS.Id, model.PERMISSION_MANAGE_PRIVATE_CHANNEL_MEMBERS.Id, model.PERMISSION_DELETE_POST.Id, + model.PERMISSION_EDIT_POST.Id, }, "channel_admin": []string{ model.PERMISSION_MANAGE_CHANNEL_ROLES.Id, @@ -368,7 +368,6 @@ func TestDoAdvancedPermissionsMigration(t *testing.T) { model.PERMISSION_UPLOAD_FILE.Id, model.PERMISSION_GET_PUBLIC_LINK.Id, model.PERMISSION_CREATE_POST.Id, - model.PERMISSION_EDIT_POST.Id, model.PERMISSION_USE_SLASH_COMMANDS.Id, model.PERMISSION_EDIT_OTHERS_POSTS.Id, model.PERMISSION_REMOVE_USER_FROM_TEAM.Id, @@ -379,6 +378,7 @@ func TestDoAdvancedPermissionsMigration(t *testing.T) { model.PERMISSION_MANAGE_SLASH_COMMANDS.Id, model.PERMISSION_MANAGE_OTHERS_SLASH_COMMANDS.Id, model.PERMISSION_MANAGE_WEBHOOKS.Id, + model.PERMISSION_EDIT_POST.Id, }, } diff --git a/app/apptestlib.go b/app/apptestlib.go index 9aef50ce6..7a2d7157d 100644 --- a/app/apptestlib.go +++ b/app/apptestlib.go @@ -232,6 +232,7 @@ func (me *TestHelper) CreatePost(channel *model.Channel) *model.Post { UserId: me.BasicUser.Id, ChannelId: channel.Id, Message: "message_" + id, + CreateAt: model.GetMillis() - 10000, } utils.DisableDebugLogForTest() diff --git a/app/post.go b/app/post.go index 01abb21cf..843319082 100644 --- a/app/post.go +++ b/app/post.go @@ -332,13 +332,6 @@ func (a *App) UpdatePost(post *model.Post, safeUpdate bool) (*model.Post, *model } else { oldPost = result.Data.(*model.PostList).Posts[post.Id] - if a.License() != nil { - if *a.Config().ServiceSettings.AllowEditPost == model.ALLOW_EDIT_POST_NEVER && post.Message != oldPost.Message { - err := model.NewAppError("UpdatePost", "api.post.update_post.permissions_denied.app_error", nil, "", http.StatusForbidden) - return nil, err - } - } - if oldPost == nil { err := model.NewAppError("UpdatePost", "api.post.update_post.find.app_error", nil, "id="+post.Id, http.StatusBadRequest) return nil, err @@ -355,7 +348,7 @@ func (a *App) UpdatePost(post *model.Post, safeUpdate bool) (*model.Post, *model } if a.License() != nil { - if *a.Config().ServiceSettings.AllowEditPost == model.ALLOW_EDIT_POST_TIME_LIMIT && model.GetMillis() > oldPost.CreateAt+int64(*a.Config().ServiceSettings.PostEditTimeLimit*1000) && post.Message != oldPost.Message { + if *a.Config().ServiceSettings.PostEditTimeLimit != -1 && model.GetMillis() > oldPost.CreateAt+int64(*a.Config().ServiceSettings.PostEditTimeLimit*1000) && post.Message != oldPost.Message { err := model.NewAppError("UpdatePost", "api.post.update_post.permissions_time_limit.app_error", map[string]interface{}{"timeLimit": *a.Config().ServiceSettings.PostEditTimeLimit}, "", http.StatusBadRequest) return nil, err } diff --git a/app/post_test.go b/app/post_test.go index 3f3783265..049d3ff92 100644 --- a/app/post_test.go +++ b/app/post_test.go @@ -15,6 +15,7 @@ import ( "github.com/stretchr/testify/require" "github.com/mattermost/mattermost-server/model" + "github.com/mattermost/mattermost-server/utils" ) func TestUpdatePostEditAt(t *testing.T) { @@ -43,6 +44,51 @@ func TestUpdatePostEditAt(t *testing.T) { } } +func TestUpdatePostTimeLimit(t *testing.T) { + th := Setup().InitBasic() + defer th.TearDown() + + post := &model.Post{} + *post = *th.BasicPost + + isLicensed := utils.IsLicensed() + license := utils.License() + defer func() { + utils.SetIsLicensed(isLicensed) + utils.SetLicense(license) + }() + utils.SetIsLicensed(true) + utils.SetLicense(&model.License{Features: &model.Features{}}) + utils.License().Features.SetDefaults() + + th.App.UpdateConfig(func(cfg *model.Config) { + *cfg.ServiceSettings.PostEditTimeLimit = -1 + }) + if _, err := th.App.UpdatePost(post, true); err != nil { + t.Fatal(err) + } + + th.App.UpdateConfig(func(cfg *model.Config) { + *cfg.ServiceSettings.PostEditTimeLimit = 1000000000 + }) + post.Message = model.NewId() + if _, err := th.App.UpdatePost(post, true); err != nil { + t.Fatal("should allow you to edit the post") + } + + th.App.UpdateConfig(func(cfg *model.Config) { + *cfg.ServiceSettings.PostEditTimeLimit = 1 + }) + post.Message = model.NewId() + if _, err := th.App.UpdatePost(post, true); err == nil { + t.Fatal("should fail on update old post") + } + + th.App.UpdateConfig(func(cfg *model.Config) { + *cfg.ServiceSettings.PostEditTimeLimit = -1 + }) +} + func TestPostReplyToPostWhereRootPosterLeftChannel(t *testing.T) { // This test ensures that when replying to a root post made by a user who has since left the channel, the reply // post completes successfully. This is a regression test for PLT-6523. diff --git a/config/default.json b/config/default.json index 934635cb9..0fa8f235f 100644 --- a/config/default.json +++ b/config/default.json @@ -45,7 +45,7 @@ "RestrictCustomEmojiCreation": "all", "RestrictPostDelete": "all", "AllowEditPost": "always", - "PostEditTimeLimit": 300, + "PostEditTimeLimit": -1, "ExperimentalEnableAuthenticationTransfer": true, "TimeBetweenUserTypingUpdatesMilliseconds": 5000, "EnablePostSearch": true, diff --git a/model/config.go b/model/config.go index 9010eaeae..8fb11d403 100644 --- a/model/config.go +++ b/model/config.go @@ -419,7 +419,7 @@ func (s *ServiceSettings) SetDefaults() { } if s.PostEditTimeLimit == nil { - s.PostEditTimeLimit = NewInt(300) + s.PostEditTimeLimit = NewInt(-1) } if s.EnablePreviewFeatures == nil { diff --git a/model/role.go b/model/role.go index 281304801..5f7352f12 100644 --- a/model/role.go +++ b/model/role.go @@ -182,7 +182,6 @@ func MakeDefaultRoles() map[string]*Role { PERMISSION_UPLOAD_FILE.Id, PERMISSION_GET_PUBLIC_LINK.Id, PERMISSION_CREATE_POST.Id, - PERMISSION_EDIT_POST.Id, PERMISSION_USE_SLASH_COMMANDS.Id, }, SchemeManaged: true, diff --git a/utils/authorization.go b/utils/authorization.go index b18ece141..b17e94587 100644 --- a/utils/authorization.go +++ b/utils/authorization.go @@ -267,5 +267,28 @@ func SetRolePermissionsFromConfig(roles map[string]*model.Role, cfg *model.Confi ) } + if IsLicensed() { + switch *cfg.ServiceSettings.AllowEditPost { + case model.ALLOW_EDIT_POST_ALWAYS, model.ALLOW_EDIT_POST_TIME_LIMIT: + roles[model.CHANNEL_USER_ROLE_ID].Permissions = append( + roles[model.CHANNEL_USER_ROLE_ID].Permissions, + model.PERMISSION_EDIT_POST.Id, + ) + roles[model.SYSTEM_ADMIN_ROLE_ID].Permissions = append( + roles[model.SYSTEM_ADMIN_ROLE_ID].Permissions, + model.PERMISSION_EDIT_POST.Id, + ) + } + } else { + roles[model.CHANNEL_USER_ROLE_ID].Permissions = append( + roles[model.CHANNEL_USER_ROLE_ID].Permissions, + model.PERMISSION_EDIT_POST.Id, + ) + roles[model.SYSTEM_ADMIN_ROLE_ID].Permissions = append( + roles[model.SYSTEM_ADMIN_ROLE_ID].Permissions, + model.PERMISSION_EDIT_POST.Id, + ) + } + return roles } -- cgit v1.2.3-1-g7c22