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

@@ -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}

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,

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,
 		},
 	}
 

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()

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
 			}

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.

config/default.json

@@ -45,7 +45,7 @@
         "RestrictCustomEmojiCreation": "all",
         "RestrictPostDelete": "all",
         "AllowEditPost": "always",
-        "PostEditTimeLimit": 300,
+        "PostEditTimeLimit": -1,
         "ExperimentalEnableAuthenticationTransfer": true,
         "TimeBetweenUserTypingUpdatesMilliseconds": 5000,
         "EnablePostSearch": true,

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 {

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,

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
 }