Fix blanking out of FileIds and backwards compatability issue with v3 (#5950)

4 files changed, 41 insertions, 21 deletions

diff --git a/api/post.go b/api/post.go
index 27efcd44c..bfc68a0d0 100644
--- a/api/post.go
+++ b/api/post.go
@@ -84,7 +84,7 @@ func updatePost(c *Context, w http.ResponseWriter, r *http.Request) {
 
 	post.UserId = c.Session.UserId
 
-	rpost, err := app.UpdatePost(post)
+	rpost, err := app.UpdatePost(post, true)
 	if err != nil {
 		c.Err = err
 		return
diff --git a/api4/post.go b/api4/post.go
index 67cd325d9..5cbfeae92 100644
--- a/api4/post.go
+++ b/api4/post.go
@@ -238,9 +238,14 @@ func updatePost(c *Context, w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
-	post.UserId = c.Session.UserId
+	if !app.SessionHasPermissionToPost(c.Session, c.Params.PostId, model.PERMISSION_EDIT_OTHERS_POSTS) {
+		c.SetPermissionError(model.PERMISSION_EDIT_OTHERS_POSTS)
+		return
+	}
+
+	post.Id = c.Params.PostId
 
-	rpost, err := app.UpdatePost(post)
+	rpost, err := app.UpdatePost(post, false)
 	if err != nil {
 		c.Err = err
 		return
@@ -262,6 +267,11 @@ func patchPost(c *Context, w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
+	if !app.SessionHasPermissionToChannelByPost(c.Session, c.Params.PostId, model.PERMISSION_EDIT_POST) {
+		c.SetPermissionError(model.PERMISSION_EDIT_POST)
+		return
+	}
+
 	if !app.SessionHasPermissionToPost(c.Session, c.Params.PostId, model.PERMISSION_EDIT_OTHERS_POSTS) {
 		c.SetPermissionError(model.PERMISSION_EDIT_OTHERS_POSTS)
 		return
diff --git a/api4/post_test.go b/api4/post_test.go
index 562136ca9..e5c72ae9e 100644
--- a/api4/post_test.go
+++ b/api4/post_test.go
@@ -167,6 +167,15 @@ func TestUpdatePost(t *testing.T) {
 	Client.Logout()
 	_, resp = Client.UpdatePost(rpost.Id, rpost)
 	CheckUnauthorizedStatus(t, resp)
+
+	th.LoginBasic2()
+	_, resp = Client.UpdatePost(rpost.Id, rpost)
+	CheckForbiddenStatus(t, resp)
+
+	Client.Logout()
+
+	_, resp = th.SystemAdminClient.UpdatePost(rpost.Id, rpost)
+	CheckNoError(t, resp)
 }
 
 func TestPatchPost(t *testing.T) {
@@ -262,6 +271,10 @@ func TestPatchPost(t *testing.T) {
 	_, resp = Client.PatchPost(post.Id, patch)
 	CheckUnauthorizedStatus(t, resp)
 
+	th.LoginBasic2()
+	_, resp = Client.PatchPost(post.Id, patch)
+	CheckForbiddenStatus(t, resp)
+
 	th.LoginTeamAdmin()
 	_, resp = Client.PatchPost(post.Id, patch)
 	CheckNoError(t, resp)
diff --git a/app/post.go b/app/post.go
index 7589a19df..7f38a9bd2 100644
--- a/app/post.go
+++ b/app/post.go
@@ -247,11 +247,10 @@ func SendEphemeralPost(teamId, userId string, post *model.Post) *model.Post {
 	return post
 }
 
-func UpdatePost(post *model.Post) (*model.Post, *model.AppError) {
+func UpdatePost(post *model.Post, safeUpdate bool) (*model.Post, *model.AppError) {
 	if utils.IsLicensed {
 		if *utils.Cfg.ServiceSettings.AllowEditPost == model.ALLOW_EDIT_POST_NEVER {
-			err := model.NewLocAppError("updatePost", "api.post.update_post.permissions_denied.app_error", nil, "")
-			err.StatusCode = http.StatusForbidden
+			err := model.NewAppError("UpdatePost", "api.post.update_post.permissions_denied.app_error", nil, "", http.StatusForbidden)
 			return nil, err
 		}
 	}
@@ -263,33 +262,28 @@ func UpdatePost(post *model.Post) (*model.Post, *model.AppError) {
 		oldPost = result.Data.(*model.PostList).Posts[post.Id]
 
 		if oldPost == nil {
-			err := model.NewLocAppError("updatePost", "api.post.update_post.find.app_error", nil, "id="+post.Id)
-			err.StatusCode = http.StatusBadRequest
+			err := model.NewAppError("UpdatePost", "api.post.update_post.find.app_error", nil, "id="+post.Id, http.StatusBadRequest)
 			return nil, err
 		}
 
 		if oldPost.UserId != post.UserId {
-			err := model.NewLocAppError("updatePost", "api.post.update_post.permissions.app_error", nil, "oldUserId="+oldPost.UserId)
-			err.StatusCode = http.StatusBadRequest
+			err := model.NewAppError("UpdatePost", "api.post.update_post.permissions.app_error", nil, "oldUserId="+oldPost.UserId, http.StatusBadRequest)
 			return nil, err
 		}
 
 		if oldPost.DeleteAt != 0 {
-			err := model.NewLocAppError("updatePost", "api.post.update_post.permissions_details.app_error", map[string]interface{}{"PostId": post.Id}, "")
-			err.StatusCode = http.StatusBadRequest
+			err := model.NewAppError("UpdatePost", "api.post.update_post.permissions_details.app_error", map[string]interface{}{"PostId": post.Id}, "", http.StatusBadRequest)
 			return nil, err
 		}
 
 		if oldPost.IsSystemMessage() {
-			err := model.NewLocAppError("updatePost", "api.post.update_post.system_message.app_error", nil, "id="+post.Id)
-			err.StatusCode = http.StatusBadRequest
+			err := model.NewAppError("UpdatePost", "api.post.update_post.system_message.app_error", nil, "id="+post.Id, http.StatusBadRequest)
 			return nil, err
 		}
 
 		if utils.IsLicensed {
 			if *utils.Cfg.ServiceSettings.AllowEditPost == model.ALLOW_EDIT_POST_TIME_LIMIT && model.GetMillis() > oldPost.CreateAt+int64(*utils.Cfg.ServiceSettings.PostEditTimeLimit*1000) {
-				err := model.NewLocAppError("updatePost", "api.post.update_post.permissions_time_limit.app_error", map[string]interface{}{"timeLimit": *utils.Cfg.ServiceSettings.PostEditTimeLimit}, "")
-				err.StatusCode = http.StatusBadRequest
+				err := model.NewAppError("UpdatePost", "api.post.update_post.permissions_time_limit.app_error", map[string]interface{}{"timeLimit": *utils.Cfg.ServiceSettings.PostEditTimeLimit}, "", http.StatusBadRequest)
 				return nil, err
 			}
 		}
@@ -299,12 +293,15 @@ func UpdatePost(post *model.Post) (*model.Post, *model.AppError) {
 	*newPost = *oldPost
 
 	newPost.Message = post.Message
-	newPost.Props = post.Props
 	newPost.EditAt = model.GetMillis()
 	newPost.Hashtags, _ = model.ParseHashtags(post.Message)
-	newPost.IsPinned = post.IsPinned
-	newPost.HasReactions = post.HasReactions
-	newPost.FileIds = post.FileIds
+
+	if !safeUpdate {
+		newPost.IsPinned = post.IsPinned
+		newPost.HasReactions = post.HasReactions
+		newPost.FileIds = post.FileIds
+		newPost.Props = post.Props
+	}
 
 	if result := <-Srv.Store.Post().Update(newPost, oldPost); result.Err != nil {
 		return nil, result.Err
@@ -327,7 +324,7 @@ func PatchPost(postId string, patch *model.PostPatch) (*model.Post, *model.AppEr
 
 	post.Patch(patch)
 
-	updatedPost, err := UpdatePost(post)
+	updatedPost, err := UpdatePost(post, false)
 	if err != nil {
 		return nil, err
 	}