diff --git a/e2e-tests/cypress/tests/integration/channels/messaging/message_deletion_spec.js b/e2e-tests/cypress/tests/integration/channels/messaging/message_deletion_spec.js index 1a55846c58..bbfc20a807 100644 --- a/e2e-tests/cypress/tests/integration/channels/messaging/message_deletion_spec.js +++ b/e2e-tests/cypress/tests/integration/channels/messaging/message_deletion_spec.js @@ -1,6 +1,8 @@ // Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. // See LICENSE.txt for license information. +import {getAdminAccount} from 'tests/support/env'; + // *************************************************************** // - [#] indicates a test step (e.g. # Go to a page) // - [*] indicates an assertion (e.g. * Check the title) @@ -8,10 +10,16 @@ // *************************************************************** // Stage: @prod -// Group: @channels @messaging +// Group: @channels @deleting_messages describe('Message deletion', () => { before(() => { + cy.apiUpdateConfig({ + ServiceSettings: { + EnableAPIPostDeletion: true, + }, + }); + // # Login as test user and visit off-topic cy.apiInitSetup({loginAfter: true}).then(({offTopicUrl}) => { cy.visit(offTopicUrl); @@ -70,4 +78,17 @@ describe('Message deletion', () => { }); }); }); + + it('Permanently delete a post and ensure it\'s reflected in the UI', () => { + // # Post message in center. + cy.postMessage('test message deletion'); + + cy.getLastPostId().then((parentMessageId) => { + const admin = getAdminAccount(); + + cy.apiDeletePost(parentMessageId, admin, true); + + cy.get(`#post_${parentMessageId}`).should('contain', '(message deleted)'); + }); + }); }); diff --git a/e2e-tests/cypress/tests/support/api_commands.ts b/e2e-tests/cypress/tests/support/api_commands.ts index 8b5b59f7d3..00c233b1be 100644 --- a/e2e-tests/cypress/tests/support/api_commands.ts +++ b/e2e-tests/cypress/tests/support/api_commands.ts @@ -74,11 +74,11 @@ function apiCreatePost(channelId: string, message: string, rootId: string, props Cypress.Commands.add('apiCreatePost', apiCreatePost); -function apiDeletePost(postId: string, user: User = getAdminAccount()): Cypress.Chainable<{status: number}> { +function apiDeletePost(postId: string, user: User = getAdminAccount(), permanent = false): Cypress.Chainable<{status: number}> { return cy.externalRequest({ user, method: 'delete', - path: `posts/${postId}`, + path: `posts/${postId}?permanent=${permanent}`, }).then((response) => { // * Validate that request was successful expect(response.status).to.equal(200); diff --git a/e2e-tests/playwright/support/server/default_config.ts b/e2e-tests/playwright/support/server/default_config.ts index 5bd4fe4058..52e1139a89 100644 --- a/e2e-tests/playwright/support/server/default_config.ts +++ b/e2e-tests/playwright/support/server/default_config.ts @@ -166,6 +166,7 @@ const defaultServerConfig: AdminConfig = { EnableAPITeamDeletion: false, EnableAPITriggerAdminNotifications: false, EnableAPIUserDeletion: false, + EnableAPIPostDeletion: false, ExperimentalEnableHardenedMode: false, ExperimentalStrictCSRFEnforcement: false, EnableEmailInvitations: false, diff --git a/server/channels/api4/post.go b/server/channels/api4/post.go index 559fec1c4d..e33f1d904e 100644 --- a/server/channels/api4/post.go +++ b/server/channels/api4/post.go @@ -626,13 +626,28 @@ func deletePost(c *Context, w http.ResponseWriter, _ *http.Request) { return } + permanent := c.Params.Permanent + auditRec := c.MakeAuditRecord("deletePost", audit.Fail) defer c.LogAuditRecWithLevel(auditRec, app.LevelContent) audit.AddEventParameter(auditRec, "post_id", c.Params.PostId) + audit.AddEventParameter(auditRec, "permanent", permanent) - post, err := c.App.GetSinglePost(c.AppContext, c.Params.PostId, false) - if err != nil { - c.SetPermissionError(model.PermissionDeletePost) + includeDeleted := permanent + + if permanent && !*c.App.Config().ServiceSettings.EnableAPIPostDeletion { + c.Err = model.NewAppError("deletePost", "api.post.delete_post.not_enabled.app_error", nil, "postId="+c.Params.PostId, http.StatusNotImplemented) + return + } + + if permanent && !c.App.SessionHasPermissionTo(*c.AppContext.Session(), model.PermissionManageSystem) { + c.SetPermissionError(model.PermissionManageSystem) + return + } + + post, appErr := c.App.GetSinglePost(c.AppContext, c.Params.PostId, includeDeleted) + if appErr != nil { + c.Err = appErr return } auditRec.AddEventPriorState(post) @@ -650,8 +665,14 @@ func deletePost(c *Context, w http.ResponseWriter, _ *http.Request) { } } - if _, err := c.App.DeletePost(c.AppContext, c.Params.PostId, c.AppContext.Session().UserId); err != nil { - c.Err = err + if permanent { + appErr = c.App.PermanentDeletePost(c.AppContext, c.Params.PostId, c.AppContext.Session().UserId) + } else { + _, appErr = c.App.DeletePost(c.AppContext, c.Params.PostId, c.AppContext.Session().UserId) + } + + if appErr != nil { + c.Err = appErr return } diff --git a/server/channels/api4/post_local.go b/server/channels/api4/post_local.go index 89967d17aa..15562f82dc 100644 --- a/server/channels/api4/post_local.go +++ b/server/channels/api4/post_local.go @@ -3,10 +3,53 @@ package api4 -import "net/http" +import ( + "net/http" + + "github.com/mattermost/mattermost/server/v8/channels/app" + "github.com/mattermost/mattermost/server/v8/channels/audit" +) func (api *API) InitPostLocal() { api.BaseRoutes.Post.Handle("", api.APILocal(getPost)).Methods(http.MethodGet) - api.BaseRoutes.PostsForChannel.Handle("", api.APILocal(getPostsForChannel)).Methods(http.MethodGet) + api.BaseRoutes.Post.Handle("", api.APILocal(localDeletePost)).Methods(http.MethodDelete) +} + +func localDeletePost(c *Context, w http.ResponseWriter, r *http.Request) { + c.RequirePostId() + if c.Err != nil { + return + } + + permanent := c.Params.Permanent + + auditRec := c.MakeAuditRecord("localDeletePost", audit.Fail) + defer c.LogAuditRecWithLevel(auditRec, app.LevelContent) + audit.AddEventParameter(auditRec, "post_id", c.Params.PostId) + audit.AddEventParameter(auditRec, "permanent", permanent) + + includeDeleted := permanent + + post, appErr := c.App.GetSinglePost(c.AppContext, c.Params.PostId, includeDeleted) + if appErr != nil { + c.Err = appErr + return + } + auditRec.AddEventPriorState(post) + auditRec.AddEventObjectType("post") + + if permanent { + appErr = c.App.PermanentDeletePost(c.AppContext, c.Params.PostId, c.AppContext.Session().UserId) + } else { + _, appErr = c.App.DeletePost(c.AppContext, c.Params.PostId, c.AppContext.Session().UserId) + } + + if appErr != nil { + c.Err = appErr + return + } + + auditRec.Success() + ReturnStatusOK(w) } diff --git a/server/channels/api4/post_test.go b/server/channels/api4/post_test.go index f70c25fa59..61f2c66729 100644 --- a/server/channels/api4/post_test.go +++ b/server/channels/api4/post_test.go @@ -2817,39 +2817,124 @@ func TestDeletePost(t *testing.T) { defer th.TearDown() client := th.Client - resp, err := client.DeletePost(context.Background(), "") - require.Error(t, err) - CheckNotFoundStatus(t, resp) + t.Run("Post not found", func(t *testing.T) { + resp, err := client.DeletePost(context.Background(), "") + require.Error(t, err) + CheckNotFoundStatus(t, resp) + }) - resp, err = client.DeletePost(context.Background(), "junk") - require.Error(t, err) - CheckBadRequestStatus(t, resp) + t.Run("Post doesn't exist", func(t *testing.T) { + resp, err := client.DeletePost(context.Background(), "junk") + require.Error(t, err) + CheckBadRequestStatus(t, resp) + }) - resp, err = client.DeletePost(context.Background(), th.BasicPost.Id) - require.Error(t, err) - CheckForbiddenStatus(t, resp) + t.Run("No permissions to delete a post", func(t *testing.T) { + resp, err := client.DeletePost(context.Background(), th.BasicPost.Id) + require.Error(t, err) + CheckForbiddenStatus(t, resp) + }) - client.Login(context.Background(), th.TeamAdminUser.Email, th.TeamAdminUser.Password) - _, err = client.DeletePost(context.Background(), th.BasicPost.Id) - require.NoError(t, err) + t.Run("Try to delete a post across different user roles", func(t *testing.T) { + client.Login(context.Background(), th.TeamAdminUser.Email, th.TeamAdminUser.Password) + _, cErr := client.DeletePost(context.Background(), th.BasicPost.Id) + require.NoError(t, cErr) - post := th.CreatePost() - user := th.CreateUser() + post := th.CreatePost() + post2 := th.CreatePost() + user := th.CreateUser() - client.Logout(context.Background()) - client.Login(context.Background(), user.Email, user.Password) + client.Logout(context.Background()) + client.Login(context.Background(), user.Email, user.Password) - resp, err = client.DeletePost(context.Background(), post.Id) - require.Error(t, err) - CheckForbiddenStatus(t, resp) + resp, err := client.DeletePost(context.Background(), post.Id) + require.Error(t, err) + CheckForbiddenStatus(t, resp) - client.Logout(context.Background()) - resp, err = client.DeletePost(context.Background(), model.NewId()) - require.Error(t, err) - CheckUnauthorizedStatus(t, resp) + client.Logout(context.Background()) + resp, err = client.DeletePost(context.Background(), model.NewId()) + require.Error(t, err) + CheckUnauthorizedStatus(t, resp) - _, err = th.SystemAdminClient.DeletePost(context.Background(), post.Id) - require.NoError(t, err) + _, err = th.SystemAdminClient.DeletePost(context.Background(), post.Id) + require.NoError(t, err) + + _, err = th.LocalClient.DeletePost(context.Background(), post2.Id) + require.NoError(t, err) + }) +} + +func TestPermanentDeletePost(t *testing.T) { + th := Setup(t).InitBasic() + defer th.TearDown() + client := th.Client + + enableAPIPostDeletion := *th.App.Config().ServiceSettings.EnableAPIPostDeletion + defer func() { + th.App.UpdateConfig(func(cfg *model.Config) { cfg.ServiceSettings.EnableAPIPostDeletion = &enableAPIPostDeletion }) + }() + + th.App.UpdateConfig(func(cfg *model.Config) { *cfg.ServiceSettings.EnableAPIPostDeletion = false }) + + t.Run("Post not found", func(t *testing.T) { + resp, err := client.PermanentDeletePost(context.Background(), "") + require.Error(t, err) + CheckNotFoundStatus(t, resp) + }) + + t.Run("Post doesn't exist", func(t *testing.T) { + resp, err := client.PermanentDeletePost(context.Background(), "junk") + require.Error(t, err) + CheckBadRequestStatus(t, resp) + }) + + t.Run("Permanent deletion not available through API if EnableAPIPostDeletion is not set", func(t *testing.T) { + resp, err := th.SystemAdminClient.PermanentDeletePost(context.Background(), th.BasicPost.Id) + require.Error(t, err) + CheckNotImplementedStatus(t, resp) + }) + + t.Run("Permanent deletion available through local mode even if EnableAPIPostDeletion is not set", func(t *testing.T) { + post := th.CreatePost() + _, err := th.LocalClient.PermanentDeletePost(context.Background(), post.Id) + require.NoError(t, err) + }) + + t.Run("No permissions to permanently delete a post", func(t *testing.T) { + th.App.UpdateConfig(func(cfg *model.Config) { *cfg.ServiceSettings.EnableAPIPostDeletion = true }) + resp, err := client.PermanentDeletePost(context.Background(), th.BasicPost.Id) + require.Error(t, err) + CheckForbiddenStatus(t, resp) + }) + + t.Run("Try to permanently delete a post across different user roles", func(t *testing.T) { + client.Login(context.Background(), th.TeamAdminUser.Email, th.TeamAdminUser.Password) + resp, err := client.PermanentDeletePost(context.Background(), th.BasicPost.Id) + require.Error(t, err) + CheckForbiddenStatus(t, resp) + + post := th.CreatePost() + post2 := th.CreatePost() + user := th.CreateUser() + + client.Logout(context.Background()) + client.Login(context.Background(), user.Email, user.Password) + + resp, err = client.PermanentDeletePost(context.Background(), post.Id) + require.Error(t, err) + CheckForbiddenStatus(t, resp) + + client.Logout(context.Background()) + resp, err = client.PermanentDeletePost(context.Background(), post.Id) + require.Error(t, err) + CheckUnauthorizedStatus(t, resp) + + _, err = th.SystemAdminClient.PermanentDeletePost(context.Background(), post.Id) + require.NoError(t, err) + + _, err = th.LocalClient.PermanentDeletePost(context.Background(), post2.Id) + require.NoError(t, err) + }) } func TestDeletePostEvent(t *testing.T) { diff --git a/server/channels/app/app_iface.go b/server/channels/app/app_iface.go index 4dce7026d0..84ec484497 100644 --- a/server/channels/app/app_iface.go +++ b/server/channels/app/app_iface.go @@ -491,6 +491,7 @@ type AppIface interface { CheckUserPostflightAuthenticationCriteria(rctx request.CTX, user *model.User) *model.AppError CheckUserPreflightAuthenticationCriteria(rctx request.CTX, user *model.User, mfaToken string) *model.AppError CheckWebConn(userID, connectionID string) *platform.CheckConnResult + CleanUpAfterPostDeletion(c request.CTX, post *model.Post, deleteByID string) *model.AppError CleanupReportChunks(format string, prefix string, numberOfChunks int) *model.AppError ClearChannelMembersCache(c request.CTX, channelID string) error ClearLatestVersionCache(rctx request.CTX) @@ -579,7 +580,7 @@ type AppIface interface { DeleteOAuthApp(rctx request.CTX, appID string) *model.AppError DeleteOutgoingWebhook(hookID string) *model.AppError DeletePluginKey(pluginID string, key string) *model.AppError - DeletePost(c request.CTX, postID, deleteByID string) (*model.Post, *model.AppError) + DeletePost(rctx request.CTX, postID, deleteByID string) (*model.Post, *model.AppError) DeletePreferences(c request.CTX, userID string, preferences model.Preferences) *model.AppError DeleteReactionForPost(c request.CTX, reaction *model.Reaction) *model.AppError DeleteRemoteCluster(remoteClusterId string) (bool, *model.AppError) @@ -986,9 +987,11 @@ type AppIface interface { PatchUser(c request.CTX, userID string, patch *model.UserPatch, asAdmin bool) (*model.User, *model.AppError) PermanentDeleteAllUsers(c request.CTX) *model.AppError PermanentDeleteChannel(c request.CTX, channel *model.Channel) *model.AppError + PermanentDeleteFilesByPost(rctx request.CTX, postID string) *model.AppError + PermanentDeletePost(rctx request.CTX, postID, deleteByID string) *model.AppError PermanentDeleteTeam(c request.CTX, team *model.Team) *model.AppError PermanentDeleteTeamId(c request.CTX, teamID string) *model.AppError - PermanentDeleteUser(c request.CTX, user *model.User) *model.AppError + PermanentDeleteUser(rctx request.CTX, user *model.User) *model.AppError PostActionCookieSecret() []byte PostAddToChannelMessage(c request.CTX, user *model.User, addedUser *model.User, channel *model.Channel, postRootId string) *model.AppError PostPatchWithProxyRemovedFromImageURLs(patch *model.PostPatch) *model.PostPatch @@ -1022,6 +1025,8 @@ type AppIface interface { RemoveDirectory(path string) *model.AppError RemoveExportFile(path string) *model.AppError RemoveFile(path string) *model.AppError + RemoveFileFromFileStore(rctx request.CTX, path string) + RemoveFilesFromFileStore(rctx request.CTX, fileInfos []*model.FileInfo) RemoveLdapPrivateCertificate() *model.AppError RemoveLdapPublicCertificate() *model.AppError RemoveNotifications(c request.CTX, post *model.Post, channel *model.Channel) error diff --git a/server/channels/app/file.go b/server/channels/app/file.go index 59ce3cd927..0f9d894d0d 100644 --- a/server/channels/app/file.go +++ b/server/channels/app/file.go @@ -1594,3 +1594,65 @@ func getFileExtFromMimeType(mimeType string) string { } return "jpg" } + +func (a *App) PermanentDeleteFilesByPost(rctx request.CTX, postID string) *model.AppError { + fileInfos, err := a.Srv().Store().FileInfo().GetForPost(postID, false, true, true) + if err != nil { + return model.NewAppError("PermanentDeleteFilesByPost", "app.file_info.get_by_post_id.app_error", nil, "", http.StatusInternalServerError).Wrap(err) + } + if len(fileInfos) == 0 { + rctx.Logger().Debug("No files found for post", mlog.String("post_id", postID)) + return nil + } + + a.RemoveFilesFromFileStore(rctx, fileInfos) + + err = a.Srv().Store().FileInfo().PermanentDeleteForPost(rctx, postID) + if err != nil { + return model.NewAppError("PermanentDeleteFilesByPost", "app.file_info.permanent_delete_for_post.app_error", nil, "", http.StatusInternalServerError).Wrap(err) + } + + a.Srv().Store().FileInfo().InvalidateFileInfosForPostCache(postID, true) + a.Srv().Store().FileInfo().InvalidateFileInfosForPostCache(postID, false) + + return nil +} + +func (a *App) RemoveFilesFromFileStore(rctx request.CTX, fileInfos []*model.FileInfo) { + for _, info := range fileInfos { + a.RemoveFileFromFileStore(rctx, info.Path) + if info.PreviewPath != "" { + a.RemoveFileFromFileStore(rctx, info.PreviewPath) + } + if info.ThumbnailPath != "" { + a.RemoveFileFromFileStore(rctx, info.ThumbnailPath) + } + } +} + +func (a *App) RemoveFileFromFileStore(rctx request.CTX, path string) { + res, appErr := a.FileExists(path) + if appErr != nil { + rctx.Logger().Warn( + "Error checking existence of file", + mlog.String("path", path), + mlog.Err(appErr), + ) + return + } + + if !res { + rctx.Logger().Warn("File not found", mlog.String("path", path)) + return + } + + appErr = a.RemoveFile(path) + if appErr != nil { + rctx.Logger().Warn( + "Unable to remove file", + mlog.String("path", path), + mlog.Err(appErr), + ) + return + } +} diff --git a/server/channels/app/file_test.go b/server/channels/app/file_test.go index e03ce41750..1877c708c1 100644 --- a/server/channels/app/file_test.go +++ b/server/channels/app/file_test.go @@ -730,3 +730,59 @@ func TestSetFileSearchableContent(t *testing.T) { require.Nil(t, appErr) assert.Equal(t, 1, len(result.Order)) } + +func TestPermanentDeleteFilesByPost(t *testing.T) { + th := Setup(t).InitBasic() + defer th.TearDown() + + t.Run("should delete files for post", func(t *testing.T) { + // Create a post with a file attachment. + teamID := th.BasicTeam.Id + channelID := th.BasicChannel.Id + userID := th.BasicUser.Id + filename := "test" + data := []byte("abcd") + + info1, err := th.App.DoUploadFile(th.Context, time.Date(2007, 2, 4, 1, 2, 3, 4, time.Local), teamID, channelID, userID, filename, data, true) + require.Nil(t, err) + + post := &model.Post{ + Message: "asd", + ChannelId: channelID, + PendingPostId: model.NewId() + ":" + fmt.Sprint(model.GetMillis()), + UserId: userID, + CreateAt: 0, + FileIds: []string{info1.Id}, + } + + post, err = th.App.CreatePost(th.Context, post, th.BasicChannel, false, true) + assert.Nil(t, err) + + err = th.App.PermanentDeleteFilesByPost(th.Context, post.Id) + require.Nil(t, err) + + _, err = th.App.GetFileInfo(th.Context, info1.Id) + require.NotNil(t, err) + }) + + t.Run("should not delete files for post that doesn't exist", func(t *testing.T) { + err := th.App.PermanentDeleteFilesByPost(th.Context, "postId1") + assert.Nil(t, err) + }) + + t.Run("should handle empty file list", func(t *testing.T) { + post := &model.Post{ + Message: "asd", + ChannelId: th.BasicChannel.Id, + PendingPostId: model.NewId() + ":" + fmt.Sprint(model.GetMillis()), + UserId: th.BasicUser.Id, + CreateAt: 0, + } + + post, err := th.App.CreatePost(th.Context, post, th.BasicChannel, false, true) + assert.Nil(t, err) + + err = th.App.PermanentDeleteFilesByPost(th.Context, post.Id) + assert.Nil(t, err) + }) +} diff --git a/server/channels/app/opentracing/opentracing_layer.go b/server/channels/app/opentracing/opentracing_layer.go index 8655c1bc4b..05b678f74d 100644 --- a/server/channels/app/opentracing/opentracing_layer.go +++ b/server/channels/app/opentracing/opentracing_layer.go @@ -1455,6 +1455,28 @@ func (a *OpenTracingAppLayer) CheckWebConn(userID string, connectionID string) * return resultVar0 } +func (a *OpenTracingAppLayer) CleanUpAfterPostDeletion(c request.CTX, post *model.Post, deleteByID string) *model.AppError { + origCtx := a.ctx + span, newCtx := tracing.StartSpanWithParentByContext(a.ctx, "app.CleanUpAfterPostDeletion") + + a.ctx = newCtx + a.app.Srv().Store().SetContext(newCtx) + defer func() { + a.app.Srv().Store().SetContext(origCtx) + a.ctx = origCtx + }() + + defer span.Finish() + resultVar0 := a.app.CleanUpAfterPostDeletion(c, post, deleteByID) + + if resultVar0 != nil { + span.LogFields(spanlog.Error(resultVar0)) + ext.Error.Set(span, true) + } + + return resultVar0 +} + func (a *OpenTracingAppLayer) CleanupReportChunks(format string, prefix string, numberOfChunks int) *model.AppError { origCtx := a.ctx span, newCtx := tracing.StartSpanWithParentByContext(a.ctx, "app.CleanupReportChunks") @@ -3588,7 +3610,7 @@ func (a *OpenTracingAppLayer) DeletePluginKey(pluginID string, key string) *mode return resultVar0 } -func (a *OpenTracingAppLayer) DeletePost(c request.CTX, postID string, deleteByID string) (*model.Post, *model.AppError) { +func (a *OpenTracingAppLayer) DeletePost(rctx request.CTX, postID string, deleteByID string) (*model.Post, *model.AppError) { origCtx := a.ctx span, newCtx := tracing.StartSpanWithParentByContext(a.ctx, "app.DeletePost") @@ -3600,7 +3622,7 @@ func (a *OpenTracingAppLayer) DeletePost(c request.CTX, postID string, deleteByI }() defer span.Finish() - resultVar0, resultVar1 := a.app.DeletePost(c, postID, deleteByID) + resultVar0, resultVar1 := a.app.DeletePost(rctx, postID, deleteByID) if resultVar1 != nil { span.LogFields(spanlog.Error(resultVar1)) @@ -13618,6 +13640,50 @@ func (a *OpenTracingAppLayer) PermanentDeleteChannel(c request.CTX, channel *mod return resultVar0 } +func (a *OpenTracingAppLayer) PermanentDeleteFilesByPost(rctx request.CTX, postID string) *model.AppError { + origCtx := a.ctx + span, newCtx := tracing.StartSpanWithParentByContext(a.ctx, "app.PermanentDeleteFilesByPost") + + a.ctx = newCtx + a.app.Srv().Store().SetContext(newCtx) + defer func() { + a.app.Srv().Store().SetContext(origCtx) + a.ctx = origCtx + }() + + defer span.Finish() + resultVar0 := a.app.PermanentDeleteFilesByPost(rctx, postID) + + if resultVar0 != nil { + span.LogFields(spanlog.Error(resultVar0)) + ext.Error.Set(span, true) + } + + return resultVar0 +} + +func (a *OpenTracingAppLayer) PermanentDeletePost(rctx request.CTX, postID string, deleteByID string) *model.AppError { + origCtx := a.ctx + span, newCtx := tracing.StartSpanWithParentByContext(a.ctx, "app.PermanentDeletePost") + + a.ctx = newCtx + a.app.Srv().Store().SetContext(newCtx) + defer func() { + a.app.Srv().Store().SetContext(origCtx) + a.ctx = origCtx + }() + + defer span.Finish() + resultVar0 := a.app.PermanentDeletePost(rctx, postID, deleteByID) + + if resultVar0 != nil { + span.LogFields(spanlog.Error(resultVar0)) + ext.Error.Set(span, true) + } + + return resultVar0 +} + func (a *OpenTracingAppLayer) PermanentDeleteTeam(c request.CTX, team *model.Team) *model.AppError { origCtx := a.ctx span, newCtx := tracing.StartSpanWithParentByContext(a.ctx, "app.PermanentDeleteTeam") @@ -13662,7 +13728,7 @@ func (a *OpenTracingAppLayer) PermanentDeleteTeamId(c request.CTX, teamID string return resultVar0 } -func (a *OpenTracingAppLayer) PermanentDeleteUser(c request.CTX, user *model.User) *model.AppError { +func (a *OpenTracingAppLayer) PermanentDeleteUser(rctx request.CTX, user *model.User) *model.AppError { origCtx := a.ctx span, newCtx := tracing.StartSpanWithParentByContext(a.ctx, "app.PermanentDeleteUser") @@ -13674,7 +13740,7 @@ func (a *OpenTracingAppLayer) PermanentDeleteUser(c request.CTX, user *model.Use }() defer span.Finish() - resultVar0 := a.app.PermanentDeleteUser(c, user) + resultVar0 := a.app.PermanentDeleteUser(rctx, user) if resultVar0 != nil { span.LogFields(spanlog.Error(resultVar0)) @@ -14454,6 +14520,36 @@ func (a *OpenTracingAppLayer) RemoveFile(path string) *model.AppError { return resultVar0 } +func (a *OpenTracingAppLayer) RemoveFileFromFileStore(rctx request.CTX, path string) { + origCtx := a.ctx + span, newCtx := tracing.StartSpanWithParentByContext(a.ctx, "app.RemoveFileFromFileStore") + + a.ctx = newCtx + a.app.Srv().Store().SetContext(newCtx) + defer func() { + a.app.Srv().Store().SetContext(origCtx) + a.ctx = origCtx + }() + + defer span.Finish() + a.app.RemoveFileFromFileStore(rctx, path) +} + +func (a *OpenTracingAppLayer) RemoveFilesFromFileStore(rctx request.CTX, fileInfos []*model.FileInfo) { + origCtx := a.ctx + span, newCtx := tracing.StartSpanWithParentByContext(a.ctx, "app.RemoveFilesFromFileStore") + + a.ctx = newCtx + a.app.Srv().Store().SetContext(newCtx) + defer func() { + a.app.Srv().Store().SetContext(origCtx) + a.ctx = origCtx + }() + + defer span.Finish() + a.app.RemoveFilesFromFileStore(rctx, fileInfos) +} + func (a *OpenTracingAppLayer) RemoveLdapPrivateCertificate() *model.AppError { origCtx := a.ctx span, newCtx := tracing.StartSpanWithParentByContext(a.ctx, "app.RemoveLdapPrivateCertificate") diff --git a/server/channels/app/post.go b/server/channels/app/post.go index 4a68f6c5d6..4a254ad988 100644 --- a/server/channels/app/post.go +++ b/server/channels/app/post.go @@ -1361,23 +1361,22 @@ func (a *App) GetPostsForChannelAroundLastUnread(c request.CTX, channelID, userI return postList, nil } -func (a *App) DeletePost(c request.CTX, postID, deleteByID string) (*model.Post, *model.AppError) { - post, err := a.Srv().Store().Post().GetSingle(sqlstore.RequestContextWithMaster(c), postID, false) +func (a *App) DeletePost(rctx request.CTX, postID, deleteByID string) (*model.Post, *model.AppError) { + post, err := a.Srv().Store().Post().GetSingle(sqlstore.RequestContextWithMaster(rctx), postID, false) if err != nil { return nil, model.NewAppError("DeletePost", "app.post.get.app_error", nil, "", http.StatusBadRequest).Wrap(err) } - channel, appErr := a.GetChannel(c, post.ChannelId) + channel, appErr := a.GetChannel(rctx, post.ChannelId) if appErr != nil { return nil, appErr } if channel.DeleteAt != 0 { - appErr := model.NewAppError("DeletePost", "api.post.delete_post.can_not_delete_post_in_deleted.error", nil, "", http.StatusBadRequest) - return nil, appErr + return nil, model.NewAppError("DeletePost", "api.post.delete_post.can_not_delete_post_in_deleted.error", nil, "", http.StatusBadRequest) } - err = a.Srv().Store().Post().Delete(c, postID, model.GetMillis(), deleteByID) + err = a.Srv().Store().Post().Delete(rctx, postID, model.GetMillis(), deleteByID) if err != nil { var nfErr *store.ErrNotFound switch { @@ -1388,60 +1387,18 @@ func (a *App) DeletePost(c request.CTX, postID, deleteByID string) (*model.Post, } } - if post.RootId == "" { - if appErr := a.DeletePersistentNotification(c, post); appErr != nil { - return nil, appErr - } - } - - postJSON, err := json.Marshal(post) - if err != nil { - return nil, model.NewAppError("DeletePost", "api.marshal_error", nil, "", http.StatusInternalServerError).Wrap(err) - } - - userMessage := model.NewWebSocketEvent(model.WebsocketEventPostDeleted, "", post.ChannelId, "", nil, "") - userMessage.Add("post", string(postJSON)) - userMessage.GetBroadcast().ContainsSanitizedData = true - a.Publish(userMessage) - - adminMessage := model.NewWebSocketEvent(model.WebsocketEventPostDeleted, "", post.ChannelId, "", nil, "") - adminMessage.Add("post", string(postJSON)) - adminMessage.Add("delete_by", deleteByID) - adminMessage.GetBroadcast().ContainsSensitiveData = true - a.Publish(adminMessage) - if len(post.FileIds) > 0 { a.Srv().Go(func() { - a.deletePostFiles(c, post.Id) + a.deletePostFiles(rctx, post.Id) }) a.Srv().Store().FileInfo().InvalidateFileInfosForPostCache(postID, true) a.Srv().Store().FileInfo().InvalidateFileInfosForPostCache(postID, false) } - a.Srv().Go(func() { - a.deleteFlaggedPosts(c, post.Id) - }) - pluginPost := post.ForPlugin() - pluginContext := pluginContext(c) - a.Srv().Go(func() { - a.ch.RunMultiHook(func(hooks plugin.Hooks) bool { - hooks.MessageHasBeenDeleted(pluginContext, pluginPost) - return true - }, plugin.MessageHasBeenDeletedID) - }) - - a.Srv().Go(func() { - if err = a.RemoveNotifications(c, post, channel); err != nil { - c.Logger().Error("DeletePost failed to delete notification", mlog.Err(err)) - } - }) - - // delete drafts associated with the post when deleting the post - a.Srv().Go(func() { - a.deleteDraftsAssociatedWithPost(c, channel, post) - }) - - a.invalidateCacheForChannelPosts(post.ChannelId) + appErr = a.CleanUpAfterPostDeletion(rctx, post, deleteByID) + if appErr != nil { + return nil, appErr + } return post, nil } @@ -2646,3 +2603,86 @@ func (a *App) MoveThread(c request.CTX, postID string, sourceChannelID, channelI c.Logger().Info(msg) return nil } + +func (a *App) PermanentDeletePost(rctx request.CTX, postID, deleteByID string) *model.AppError { + post, err := a.Srv().Store().Post().GetSingle(sqlstore.RequestContextWithMaster(rctx), postID, true) + if err != nil { + return model.NewAppError("DeletePost", "app.post.get.app_error", nil, "", http.StatusBadRequest).Wrap(err) + } + + if len(post.FileIds) > 0 { + appErr := a.PermanentDeleteFilesByPost(rctx, post.Id) + if appErr != nil { + return appErr + } + } + + err = a.Srv().Store().Post().PermanentDelete(rctx, post.Id) + if err != nil { + return model.NewAppError("PermanentDeletePost", "app.post.permanent_delete_post.error", nil, "", http.StatusInternalServerError).Wrap(err) + } + + appErr := a.CleanUpAfterPostDeletion(rctx, post, deleteByID) + if appErr != nil { + return appErr + } + + return nil +} + +func (a *App) CleanUpAfterPostDeletion(c request.CTX, post *model.Post, deleteByID string) *model.AppError { + channel, appErr := a.GetChannel(c, post.ChannelId) + if appErr != nil { + return appErr + } + + if post.RootId == "" { + if appErr := a.DeletePersistentNotification(c, post); appErr != nil { + return appErr + } + } + + postJSON, err := json.Marshal(post) + if err != nil { + return model.NewAppError("DeletePost", "api.marshal_error", nil, "", http.StatusInternalServerError).Wrap(err) + } + + userMessage := model.NewWebSocketEvent(model.WebsocketEventPostDeleted, "", post.ChannelId, "", nil, "") + userMessage.Add("post", string(postJSON)) + userMessage.GetBroadcast().ContainsSanitizedData = true + a.Publish(userMessage) + + adminMessage := model.NewWebSocketEvent(model.WebsocketEventPostDeleted, "", post.ChannelId, "", nil, "") + adminMessage.Add("post", string(postJSON)) + adminMessage.Add("delete_by", deleteByID) + adminMessage.GetBroadcast().ContainsSensitiveData = true + a.Publish(adminMessage) + + a.Srv().Go(func() { + a.deleteFlaggedPosts(c, post.Id) + }) + + pluginPost := post.ForPlugin() + pluginContext := pluginContext(c) + a.Srv().Go(func() { + a.ch.RunMultiHook(func(hooks plugin.Hooks) bool { + hooks.MessageHasBeenDeleted(pluginContext, pluginPost) + return true + }, plugin.MessageHasBeenDeletedID) + }) + + a.Srv().Go(func() { + if err = a.RemoveNotifications(c, post, channel); err != nil { + c.Logger().Error("DeletePost failed to delete notification", mlog.Err(err)) + } + }) + + // delete drafts associated with the post when deleting the post + a.Srv().Go(func() { + a.deleteDraftsAssociatedWithPost(c, channel, post) + }) + + a.invalidateCacheForChannelPosts(post.ChannelId) + + return nil +} diff --git a/server/channels/app/post_test.go b/server/channels/app/post_test.go index e02d86e50e..5fdd6cd8f0 100644 --- a/server/channels/app/post_test.go +++ b/server/channels/app/post_test.go @@ -3569,3 +3569,94 @@ func TestValidateMoveOrCopy(t *testing.T) { require.Equal(t, "the thread is 2 posts long, but this command is configured to only move threads of up to 1 posts", e.Error()) }) } + +func TestPermanentDeletePost(t *testing.T) { + th := Setup(t).InitBasic() + defer th.TearDown() + + t.Run("should permanently delete a post and its file attachment", func(t *testing.T) { + // Create a post with a file attachment. + teamID := th.BasicTeam.Id + channelID := th.BasicChannel.Id + userID := th.BasicUser.Id + filename := "test" + data := []byte("abcd") + + info1, err := th.App.DoUploadFile(th.Context, time.Date(2007, 2, 4, 1, 2, 3, 4, time.Local), teamID, channelID, userID, filename, data, true) + assert.Nil(t, err) + + post := &model.Post{ + Message: "asd", + ChannelId: channelID, + PendingPostId: model.NewId() + ":" + fmt.Sprint(model.GetMillis()), + UserId: userID, + CreateAt: 0, + FileIds: []string{info1.Id}, + } + + post, err = th.App.CreatePost(th.Context, post, th.BasicChannel, false, true) + assert.Nil(t, err) + + // Delete the post. + err = th.App.PermanentDeletePost(th.Context, post.Id, userID) + assert.Nil(t, err) + + // Wait for the cleanup routine to finish. + time.Sleep(time.Millisecond * 100) + + // Check that the post can no longer be reached. + _, err = th.App.GetSinglePost(th.Context, post.Id, true) + assert.NotNil(t, err) + + // Check that the file can no longer be reached. + _, err = th.App.GetFileInfo(th.Context, info1.Id) + assert.NotNil(t, err) + }) + + t.Run("should permanently delete a post that is soft deleted", func(t *testing.T) { + // Create a post with a file attachment. + teamID := th.BasicTeam.Id + channelID := th.BasicChannel.Id + userID := th.BasicUser.Id + filename := "test" + data := []byte("abcd") + + info1, err := th.App.DoUploadFile(th.Context, time.Date(2007, 2, 4, 1, 2, 3, 4, time.Local), teamID, channelID, userID, filename, data, true) + require.Nil(t, err) + + post := &model.Post{ + Message: "asd", + ChannelId: channelID, + PendingPostId: model.NewId() + ":" + fmt.Sprint(model.GetMillis()), + UserId: userID, + CreateAt: 0, + FileIds: []string{info1.Id}, + } + + post, err = th.App.CreatePost(th.Context, post, th.BasicChannel, false, true) + assert.Nil(t, err) + + infos, sErr := th.App.Srv().Store().FileInfo().GetForPost(post.Id, true, true, false) + require.NoError(t, sErr) + assert.Len(t, infos, 1) + + // Soft delete the post. + _, err = th.App.DeletePost(th.Context, post.Id, userID) + assert.Nil(t, err) + + // Wait for the cleanup routine to finish. + time.Sleep(time.Millisecond * 100) + + // Delete the post. + err = th.App.PermanentDeletePost(th.Context, post.Id, userID) + assert.Nil(t, err) + + // Check that the post can no longer be reached. + _, err = th.App.GetSinglePost(th.Context, post.Id, true) + assert.NotNil(t, err) + + infos, sErr = th.App.Srv().Store().FileInfo().GetForPost(post.Id, true, true, false) + require.NoError(t, sErr) + assert.Len(t, infos, 0) + }) +} diff --git a/server/channels/app/user.go b/server/channels/app/user.go index c17155e700..22e3aced81 100644 --- a/server/channels/app/user.go +++ b/server/channels/app/user.go @@ -1759,13 +1759,13 @@ func (a *App) UpdateUserRolesWithUser(c request.CTX, user *model.User, newRoles return ruser, nil } -func (a *App) PermanentDeleteUser(c request.CTX, user *model.User) *model.AppError { - c.Logger().Warn("Attempting to permanently delete account", mlog.String("user_id", user.Id), mlog.String("user_email", user.Email)) +func (a *App) PermanentDeleteUser(rctx request.CTX, user *model.User) *model.AppError { + rctx.Logger().Warn("Attempting to permanently delete account", mlog.String("user_id", user.Id), mlog.String("user_email", user.Email)) if user.IsInRole(model.SystemAdminRoleId) { - c.Logger().Warn("You are deleting a user that is a system administrator. You may need to set another account as the system administrator using the command line tools.", mlog.String("user_email", user.Email)) + rctx.Logger().Warn("You are deleting a user that is a system administrator. You may need to set another account as the system administrator using the command line tools.", mlog.String("user_email", user.Email)) } - if _, err := a.UpdateActive(c, user, false); err != nil { + if _, err := a.UpdateActive(rctx, user, false); err != nil { return err } @@ -1797,7 +1797,7 @@ func (a *App) PermanentDeleteUser(c request.CTX, user *model.User) *model.AppErr return model.NewAppError("PermanentDeleteUser", "app.preference.permanent_delete_by_user.app_error", nil, "", http.StatusInternalServerError).Wrap(err) } - if err := a.Srv().Store().Channel().PermanentDeleteMembersByUser(c, user.Id); err != nil { + if err := a.Srv().Store().Channel().PermanentDeleteMembersByUser(rctx, user.Id); err != nil { return model.NewAppError("PermanentDeleteUser", "app.channel.permanent_delete_members_by_user.app_error", nil, "", http.StatusInternalServerError).Wrap(err) } @@ -1805,7 +1805,7 @@ func (a *App) PermanentDeleteUser(c request.CTX, user *model.User) *model.AppErr return model.NewAppError("PermanentDeleteUser", "app.group.permanent_delete_members_by_user.app_error", nil, "", http.StatusInternalServerError).Wrap(err) } - if err := a.Srv().Store().Post().PermanentDeleteByUser(c, user.Id); err != nil { + if err := a.Srv().Store().Post().PermanentDeleteByUser(rctx, user.Id); err != nil { return model.NewAppError("PermanentDeleteUser", "app.post.permanent_delete_by_user.app_error", nil, "", http.StatusInternalServerError).Wrap(err) } @@ -1825,35 +1825,10 @@ func (a *App) PermanentDeleteUser(c request.CTX, user *model.User) *model.AppErr infos, err := a.Srv().Store().FileInfo().GetForUser(user.Id) if err != nil { - c.Logger().Warn("Error getting file list for user from FileInfoStore", mlog.Err(err)) + rctx.Logger().Warn("Error getting file list for user from FileInfoStore", mlog.Err(err)) } - for _, info := range infos { - res, err := a.FileExists(info.Path) - if err != nil { - c.Logger().Warn( - "Error checking existence of file", - mlog.String("path", info.Path), - mlog.Err(err), - ) - continue - } - - if !res { - c.Logger().Warn("File not found", mlog.String("path", info.Path)) - continue - } - - err = a.RemoveFile(info.Path) - - if err != nil { - c.Logger().Warn( - "Unable to remove file", - mlog.String("path", info.Path), - mlog.Err(err), - ) - } - } + a.RemoveFilesFromFileStore(rctx, infos) // delete directory containing user's profile image profileImageDirectory := getProfileImageDirectory(user.Id) @@ -1864,7 +1839,7 @@ func (a *App) PermanentDeleteUser(c request.CTX, user *model.User) *model.AppErr if errProfileImageExists != nil { fileHandlingErrorsFound = true - c.Logger().Warn( + rctx.Logger().Warn( "Error checking existence of profile image.", mlog.String("path", profileImagePath), mlog.Err(errProfileImageExists), @@ -1876,7 +1851,7 @@ func (a *App) PermanentDeleteUser(c request.CTX, user *model.User) *model.AppErr if errRemoveDirectory != nil { fileHandlingErrorsFound = true - c.Logger().Warn( + rctx.Logger().Warn( "Unable to remove profile image directory", mlog.String("path", profileImageDirectory), mlog.Err(errRemoveDirectory), @@ -1884,11 +1859,11 @@ func (a *App) PermanentDeleteUser(c request.CTX, user *model.User) *model.AppErr } } - if _, err := a.Srv().Store().FileInfo().PermanentDeleteByUser(c, user.Id); err != nil { + if _, err := a.Srv().Store().FileInfo().PermanentDeleteByUser(rctx, user.Id); err != nil { return model.NewAppError("PermanentDeleteUser", "app.file_info.permanent_delete_by_user.app_error", nil, "", http.StatusInternalServerError).Wrap(err) } - if err := a.Srv().Store().User().PermanentDelete(c, user.Id); err != nil { + if err := a.Srv().Store().User().PermanentDelete(rctx, user.Id); err != nil { return model.NewAppError("PermanentDeleteUser", "app.user.permanent_delete.app_error", nil, "", http.StatusInternalServerError).Wrap(err) } @@ -1896,7 +1871,7 @@ func (a *App) PermanentDeleteUser(c request.CTX, user *model.User) *model.AppErr return model.NewAppError("PermanentDeleteUser", "app.audit.permanent_delete_by_user.app_error", nil, "", http.StatusInternalServerError).Wrap(err) } - if err := a.Srv().Store().Team().RemoveAllMembersByUser(c, user.Id); err != nil { + if err := a.Srv().Store().Team().RemoveAllMembersByUser(rctx, user.Id); err != nil { return model.NewAppError("PermanentDeleteUser", "app.team.remove_member.app_error", nil, "", http.StatusInternalServerError).Wrap(err) } @@ -1906,7 +1881,7 @@ func (a *App) PermanentDeleteUser(c request.CTX, user *model.User) *model.AppErr return model.NewAppError("PermanentDeleteUser", "app.file_info.permanent_delete_by_user.app_error", nil, "Couldn't delete profile image of the user.", http.StatusAccepted) } - c.Logger().Warn("Permanently deleted account", mlog.String("user_email", user.Email), mlog.String("user_id", user.Id)) + rctx.Logger().Warn("Permanently deleted account", mlog.String("user_email", user.Email), mlog.String("user_id", user.Id)) return nil } diff --git a/server/channels/store/opentracinglayer/opentracinglayer.go b/server/channels/store/opentracinglayer/opentracinglayer.go index 9b1f31dec0..17fed03d06 100644 --- a/server/channels/store/opentracinglayer/opentracinglayer.go +++ b/server/channels/store/opentracinglayer/opentracinglayer.go @@ -4060,6 +4060,24 @@ func (s *OpenTracingLayerFileInfoStore) PermanentDeleteByUser(ctx request.CTX, u return result, err } +func (s *OpenTracingLayerFileInfoStore) PermanentDeleteForPost(rctx request.CTX, postID string) error { + origCtx := s.Root.Store.Context() + span, newCtx := tracing.StartSpanWithParentByContext(s.Root.Store.Context(), "FileInfoStore.PermanentDeleteForPost") + s.Root.Store.SetContext(newCtx) + defer func() { + s.Root.Store.SetContext(origCtx) + }() + + defer span.Finish() + err := s.FileInfoStore.PermanentDeleteForPost(rctx, postID) + if err != nil { + span.LogFields(spanlog.Error(err)) + ext.Error.Set(span, true) + } + + return err +} + func (s *OpenTracingLayerFileInfoStore) Save(ctx request.CTX, info *model.FileInfo) (*model.FileInfo, error) { origCtx := s.Root.Store.Context() span, newCtx := tracing.StartSpanWithParentByContext(s.Root.Store.Context(), "FileInfoStore.Save") @@ -6874,6 +6892,24 @@ func (s *OpenTracingLayerPostStore) OverwriteMultiple(posts []*model.Post) ([]*m return result, resultVar1, err } +func (s *OpenTracingLayerPostStore) PermanentDelete(rctx request.CTX, postID string) error { + origCtx := s.Root.Store.Context() + span, newCtx := tracing.StartSpanWithParentByContext(s.Root.Store.Context(), "PostStore.PermanentDelete") + s.Root.Store.SetContext(newCtx) + defer func() { + s.Root.Store.SetContext(origCtx) + }() + + defer span.Finish() + err := s.PostStore.PermanentDelete(rctx, postID) + if err != nil { + span.LogFields(spanlog.Error(err)) + ext.Error.Set(span, true) + } + + return err +} + func (s *OpenTracingLayerPostStore) PermanentDeleteBatch(endTime int64, limit int64) (int64, error) { origCtx := s.Root.Store.Context() span, newCtx := tracing.StartSpanWithParentByContext(s.Root.Store.Context(), "PostStore.PermanentDeleteBatch") diff --git a/server/channels/store/retrylayer/retrylayer.go b/server/channels/store/retrylayer/retrylayer.go index 470bb123ac..6c9c42e213 100644 --- a/server/channels/store/retrylayer/retrylayer.go +++ b/server/channels/store/retrylayer/retrylayer.go @@ -4559,6 +4559,27 @@ func (s *RetryLayerFileInfoStore) PermanentDeleteByUser(ctx request.CTX, userID } +func (s *RetryLayerFileInfoStore) PermanentDeleteForPost(rctx request.CTX, postID string) error { + + tries := 0 + for { + err := s.FileInfoStore.PermanentDeleteForPost(rctx, postID) + if err == nil { + return nil + } + if !isRepeatableError(err) { + return err + } + tries++ + if tries >= 3 { + err = errors.Wrap(err, "giving up after 3 consecutive repeatable transaction failures") + return err + } + timepkg.Sleep(100 * timepkg.Millisecond) + } + +} + func (s *RetryLayerFileInfoStore) Save(ctx request.CTX, info *model.FileInfo) (*model.FileInfo, error) { tries := 0 @@ -7796,6 +7817,27 @@ func (s *RetryLayerPostStore) OverwriteMultiple(posts []*model.Post) ([]*model.P } +func (s *RetryLayerPostStore) PermanentDelete(rctx request.CTX, postID string) error { + + tries := 0 + for { + err := s.PostStore.PermanentDelete(rctx, postID) + if err == nil { + return nil + } + if !isRepeatableError(err) { + return err + } + tries++ + if tries >= 3 { + err = errors.Wrap(err, "giving up after 3 consecutive repeatable transaction failures") + return err + } + timepkg.Sleep(100 * timepkg.Millisecond) + } + +} + func (s *RetryLayerPostStore) PermanentDeleteBatch(endTime int64, limit int64) (int64, error) { tries := 0 diff --git a/server/channels/store/searchlayer/file_info_layer.go b/server/channels/store/searchlayer/file_info_layer.go index d6ccd89029..1811475d1f 100644 --- a/server/channels/store/searchlayer/file_info_layer.go +++ b/server/channels/store/searchlayer/file_info_layer.go @@ -4,6 +4,8 @@ package searchlayer import ( + "fmt" + "github.com/mattermost/mattermost/server/public/model" "github.com/mattermost/mattermost/server/public/shared/mlog" "github.com/mattermost/mattermost/server/public/shared/request" @@ -74,6 +76,7 @@ func (s SearchFileInfoStore) deleteFileIndexForUser(rctx request.CTX, userID str } } +//nolint:unused // Temporarily unused until the post_id is indexed with the file func (s SearchFileInfoStore) deleteFileIndexForPost(rctx request.CTX, postID string) { for _, engine := range s.rootStore.searchEngine.GetActiveEngines() { if engine.IsIndexingEnabled() { @@ -133,14 +136,36 @@ func (s SearchFileInfoStore) AttachToPost(rctx request.CTX, fileId, postId, chan return err } -func (s SearchFileInfoStore) DeleteForPost(rctx request.CTX, postId string) (string, error) { - result, err := s.FileInfoStore.DeleteForPost(rctx, postId) +func (s SearchFileInfoStore) DeleteForPost(rctx request.CTX, postID string) (string, error) { + // temporary workaround because deleteFileIndexForPost is not working due to the post_id not being indexed with the file + files, err := s.FileInfoStore.GetForPost(postID, false, true, true) + if err != nil { + return "", fmt.Errorf("failed to get files for post %s: %w", postID, err) + } + result, err := s.FileInfoStore.DeleteForPost(rctx, postID) if err == nil { - s.deleteFileIndexForPost(rctx, postId) + for _, file := range files { + s.deleteFileIndex(rctx, file.Id) + } } return result, err } +func (s SearchFileInfoStore) PermanentDeleteForPost(rctx request.CTX, postID string) error { + // temporary workaround because deleteFileIndexForPost is not working due to the post_id not being indexed with the file + files, err := s.FileInfoStore.GetForPost(postID, false, true, true) + if err != nil { + return err + } + err = s.FileInfoStore.PermanentDeleteForPost(rctx, postID) + if err == nil { + for _, file := range files { + s.deleteFileIndex(rctx, file.Id) + } + } + return err +} + func (s SearchFileInfoStore) PermanentDelete(rctx request.CTX, fileId string) error { err := s.FileInfoStore.PermanentDelete(rctx, fileId) if err == nil { diff --git a/server/channels/store/searchlayer/post_layer.go b/server/channels/store/searchlayer/post_layer.go index ea3563729b..5e694f5fbf 100644 --- a/server/channels/store/searchlayer/post_layer.go +++ b/server/channels/store/searchlayer/post_layer.go @@ -4,8 +4,6 @@ package searchlayer import ( - "context" - "github.com/pkg/errors" "github.com/mattermost/mattermost/server/public/model" @@ -107,19 +105,29 @@ func (s SearchPostStore) Save(rctx request.CTX, post *model.Post) (*model.Post, func (s SearchPostStore) Delete(rctx request.CTX, postId string, date int64, deletedByID string) error { err := s.PostStore.Delete(rctx, postId, date, deletedByID) - - if err == nil { - opts := model.GetPostsOptions{ - SkipFetchThreads: true, - } - postList, err2 := s.PostStore.Get(context.Background(), postId, opts, "", map[string]bool{}) - if postList != nil && len(postList.Order) > 0 { - if err2 != nil { - s.deletePostIndex(rctx, postList.Posts[postList.Order[0]]) - } - } + if err != nil { + return err } - return err + post, err := s.PostStore.GetSingle(rctx, postId, true) + if err != nil { + return err + } + s.deletePostIndex(rctx, post) + return nil +} + +func (s SearchPostStore) PermanentDelete(rctx request.CTX, postID string) error { + // Get full post struct for later + post, err := s.PostStore.GetSingle(rctx, postID, true) + if err != nil { + return err + } + err = s.PostStore.PermanentDelete(rctx, postID) + if err != nil { + return err + } + s.deletePostIndex(rctx, post) + return nil } func (s SearchPostStore) PermanentDeleteByUser(rctx request.CTX, userID string) error { diff --git a/server/channels/store/searchtest/file_info_layer.go b/server/channels/store/searchtest/file_info_layer.go index ccee5750f6..bcc482fc08 100644 --- a/server/channels/store/searchtest/file_info_layer.go +++ b/server/channels/store/searchtest/file_info_layer.go @@ -183,6 +183,11 @@ var searchFileInfoStoreTests = []searchTest{ Fn: testFileInfoSearchEmailsWithoutQuotes, Tags: []string{EngineElasticSearch}, }, + { + Name: "Should be removed from search index when deleted", + Fn: testSearchFileDeletedPost, + Tags: []string{EngineAll}, + }, { Name: "Should not search files not attached to a post", Fn: testFileInfoSearchNoResultForPostlessFileInfos, @@ -1656,6 +1661,42 @@ func testFileInfoSearchEmailsWithoutQuotes(t *testing.T, th *SearchTestHelper) { th.checkFileInfoInSearchResults(t, p1.Id, results.FileInfos) } +func testSearchFileDeletedPost(t *testing.T, th *SearchTestHelper) { + t.Run("Should not return file info for soft deleted post", func(t *testing.T) { + post, err := th.createPost(th.User.Id, th.ChannelBasic.Id, "deletedmessage", "", model.PostTypeDefault, 0, false) + require.NoError(t, err) + + _, err = th.createFileInfo(th.User.Id, post.Id, post.ChannelId, "deletedmessage", "deletedmessage", "jpg", "image/jpeg", 0, 0) + require.NoError(t, err) + + _, err = th.Store.FileInfo().DeleteForPost(th.Context, post.Id) + require.NoError(t, err) + + params := &model.SearchParams{Terms: "deletedmessage"} + results, err := th.Store.FileInfo().Search(th.Context, []*model.SearchParams{params}, th.User.Id, th.Team.Id, 0, 20) + require.NoError(t, err) + + require.Len(t, results.FileInfos, 0) + }) + + t.Run("Should not return file info for hard deleted post", func(t *testing.T) { + post, err := th.createPost(th.User.Id, th.ChannelBasic.Id, "deletedmessage", "", model.PostTypeDefault, 0, false) + require.NoError(t, err) + + _, err = th.createFileInfo(th.User.Id, post.Id, post.ChannelId, "deletedmessage", "deletedmessage", "jpg", "image/jpeg", 0, 0) + require.NoError(t, err) + + err = th.Store.FileInfo().PermanentDeleteForPost(th.Context, post.Id) + require.NoError(t, err) + + params := &model.SearchParams{Terms: "deletedmessage"} + results, err := th.Store.FileInfo().Search(th.Context, []*model.SearchParams{params}, th.User.Id, th.Team.Id, 0, 20) + require.NoError(t, err) + + require.Len(t, results.FileInfos, 0) + }) +} + func testFileInfoSearchNoResultForPostlessFileInfos(t *testing.T, th *SearchTestHelper) { _, err := th.createFileInfo(th.User.Id, "", th.ChannelBasic.Id, "message test@test.com", "message test@test.com", "jpg", "image/jpeg", 0, 0) require.NoError(t, err) diff --git a/server/channels/store/searchtest/post_layer.go b/server/channels/store/searchtest/post_layer.go index b728e70a3e..e8ba847c37 100644 --- a/server/channels/store/searchtest/post_layer.go +++ b/server/channels/store/searchtest/post_layer.go @@ -275,6 +275,11 @@ var searchPostStoreTests = []searchTest{ Fn: testSearchAcrossTeams, Tags: []string{EngineAll}, }, + { + Name: "Should be removed from search index when deleted", + Fn: testSearchPostDeleted, + Tags: []string{EngineAll}, + }, } func TestSearchPostStore(t *testing.T, s store.Store, testEngine *SearchTestEngine) { @@ -1955,3 +1960,31 @@ func testSearchAcrossTeams(t *testing.T, th *SearchTestHelper) { require.Len(t, results.Posts, 2) } + +func testSearchPostDeleted(t *testing.T, th *SearchTestHelper) { + t.Run("Search for soft deleted post", func(t *testing.T) { + p1, err := th.createPost(th.User.Id, th.ChannelBasic.Id, "message to delete", "", model.PostTypeDefault, 0, false) + require.NoError(t, err) + + err = th.Store.Post().Delete(th.Context, p1.Id, p1.UpdateAt, th.User.Id) + require.NoError(t, err) + + params := &model.SearchParams{Terms: "message to delete"} + results, err := th.Store.Post().SearchPostsForUser(th.Context, []*model.SearchParams{params}, th.User.Id, th.Team.Id, 0, 20) + require.NoError(t, err) + require.Len(t, results.Posts, 0) + }) + + t.Run("Search for hard deleted post", func(t *testing.T) { + p2, err := th.createPost(th.User.Id, th.ChannelBasic.Id, "message to delete", "", model.PostTypeDefault, 0, false) + require.NoError(t, err) + + err = th.Store.Post().PermanentDelete(th.Context, p2.Id) + require.NoError(t, err) + + params := &model.SearchParams{Terms: "message to delete"} + results, err := th.Store.Post().SearchPostsForUser(th.Context, []*model.SearchParams{params}, th.User.Id, th.Team.Id, 0, 20) + require.NoError(t, err) + require.Len(t, results.Posts, 0) + }) +} diff --git a/server/channels/store/sqlstore/file_info_store.go b/server/channels/store/sqlstore/file_info_store.go index ad4d8933c3..021e11154f 100644 --- a/server/channels/store/sqlstore/file_info_store.go +++ b/server/channels/store/sqlstore/file_info_store.go @@ -455,6 +455,13 @@ func (fs SqlFileInfoStore) DeleteForPost(rctx request.CTX, postId string) (strin return postId, nil } +func (fs SqlFileInfoStore) PermanentDeleteForPost(rctx request.CTX, postID string) error { + if _, err := fs.GetMasterX().Exec(`DELETE FROM FileInfo WHERE PostId = ?`, postID); err != nil { + return errors.Wrapf(err, "failed to delete FileInfo with PostId=%s", postID) + } + return nil +} + func (fs SqlFileInfoStore) PermanentDelete(rctx request.CTX, fileId string) error { if _, err := fs.GetMasterX().Exec(`DELETE FROM FileInfo WHERE Id = ?`, fileId); err != nil { return errors.Wrapf(err, "failed to delete FileInfo with id=%s", fileId) diff --git a/server/channels/store/sqlstore/post_store.go b/server/channels/store/sqlstore/post_store.go index 9b85a7334c..a3766aefd4 100644 --- a/server/channels/store/sqlstore/post_store.go +++ b/server/channels/store/sqlstore/post_store.go @@ -955,6 +955,10 @@ func (s *SqlPostStore) Delete(rctx request.CTX, postID string, time int64, delet return nil } +func (s *SqlPostStore) PermanentDelete(rctx request.CTX, postID string) (err error) { + return s.permanentDelete([]string{postID}) +} + func (s *SqlPostStore) permanentDelete(postIds []string) (err error) { transaction, err := s.GetMasterX().Beginx() if err != nil { diff --git a/server/channels/store/store.go b/server/channels/store/store.go index 11a281876b..2681c73030 100644 --- a/server/channels/store/store.go +++ b/server/channels/store/store.go @@ -361,6 +361,7 @@ type PostStore interface { Get(ctx context.Context, id string, opts model.GetPostsOptions, userID string, sanitizeOptions map[string]bool) (*model.PostList, error) GetSingle(rctx request.CTX, id string, inclDeleted bool) (*model.Post, error) Delete(rctx request.CTX, postID string, timestamp int64, deleteByID string) error + PermanentDelete(rctx request.CTX, postID string) error PermanentDeleteByUser(rctx request.CTX, userID string) error PermanentDeleteByChannel(rctx request.CTX, channelID string) error GetPosts(options model.GetPostsOptions, allowFromCache bool, sanitizeOptions map[string]bool) (*model.PostList, error) @@ -720,6 +721,7 @@ type FileInfoStore interface { InvalidateFileInfosForPostCache(postID string, deleted bool) AttachToPost(c request.CTX, fileID string, postID string, channelID, creatorID string) error DeleteForPost(c request.CTX, postID string) (string, error) + PermanentDeleteForPost(rctx request.CTX, postID string) error PermanentDelete(c request.CTX, fileID string) error PermanentDeleteBatch(ctx request.CTX, endTime int64, limit int64) (int64, error) PermanentDeleteByUser(ctx request.CTX, userID string) (int64, error) diff --git a/server/channels/store/storetest/file_info_store.go b/server/channels/store/storetest/file_info_store.go index 4bb3ac1568..b9b705a1d5 100644 --- a/server/channels/store/storetest/file_info_store.go +++ b/server/channels/store/storetest/file_info_store.go @@ -37,6 +37,7 @@ func TestFileInfoStore(t *testing.T, rctx request.CTX, ss store.Store, s SqlStor t.Run("CountAll", func(t *testing.T) { testFileInfoStoreCountAll(t, rctx, ss) }) t.Run("GetStorageUsage", func(t *testing.T) { testFileInfoGetStorageUsage(t, rctx, ss) }) t.Run("GetUptoNSizeFileTime", func(t *testing.T) { testGetUptoNSizeFileTime(t, rctx, ss, s) }) + t.Run("FileInfoPermanentDeleteForPost", func(t *testing.T) { testPermanentDeleteForPost(t, rctx, ss) }) } func testFileInfoSaveGet(t *testing.T, rctx request.CTX, ss store.Store) { @@ -941,3 +942,28 @@ func testGetUptoNSizeFileTime(t *testing.T, rctx request.CTX, ss store.Store, s require.NoError(t, err) assert.Equal(t, f2.CreateAt, createAt) } + +func testPermanentDeleteForPost(t *testing.T, rctx request.CTX, ss store.Store) { + postId := model.NewId() + + _, err := ss.FileInfo().Save(rctx, &model.FileInfo{ + PostId: postId, + CreatorId: model.NewId(), + Size: 10, + Path: "file1.txt", + CreateAt: utils.MillisFromTime(time.Now()), + }) + require.NoError(t, err) + + err = ss.FileInfo().PermanentDeleteForPost(rctx, postId) + require.NoError(t, err) + + postInfos, err := ss.FileInfo().GetForPost( + postId, + true, + true, + false, + ) + require.NoError(t, err) + assert.Len(t, postInfos, 0) +} diff --git a/server/channels/store/storetest/mocks/FileInfoStore.go b/server/channels/store/storetest/mocks/FileInfoStore.go index 71231ea3ce..94de49ed5d 100644 --- a/server/channels/store/storetest/mocks/FileInfoStore.go +++ b/server/channels/store/storetest/mocks/FileInfoStore.go @@ -469,6 +469,24 @@ func (_m *FileInfoStore) PermanentDeleteByUser(ctx request.CTX, userID string) ( return r0, r1 } +// PermanentDeleteForPost provides a mock function with given fields: rctx, postID +func (_m *FileInfoStore) PermanentDeleteForPost(rctx request.CTX, postID string) error { + ret := _m.Called(rctx, postID) + + if len(ret) == 0 { + panic("no return value specified for PermanentDeleteForPost") + } + + var r0 error + if rf, ok := ret.Get(0).(func(request.CTX, string) error); ok { + r0 = rf(rctx, postID) + } else { + r0 = ret.Error(0) + } + + return r0 +} + // Save provides a mock function with given fields: ctx, info func (_m *FileInfoStore) Save(ctx request.CTX, info *model.FileInfo) (*model.FileInfo, error) { ret := _m.Called(ctx, info) diff --git a/server/channels/store/storetest/mocks/PostStore.go b/server/channels/store/storetest/mocks/PostStore.go index df5ab5523f..eeee213fa9 100644 --- a/server/channels/store/storetest/mocks/PostStore.go +++ b/server/channels/store/storetest/mocks/PostStore.go @@ -1046,6 +1046,24 @@ func (_m *PostStore) OverwriteMultiple(posts []*model.Post) ([]*model.Post, int, return r0, r1, r2 } +// PermanentDelete provides a mock function with given fields: rctx, postID +func (_m *PostStore) PermanentDelete(rctx request.CTX, postID string) error { + ret := _m.Called(rctx, postID) + + if len(ret) == 0 { + panic("no return value specified for PermanentDelete") + } + + var r0 error + if rf, ok := ret.Get(0).(func(request.CTX, string) error); ok { + r0 = rf(rctx, postID) + } else { + r0 = ret.Error(0) + } + + return r0 +} + // PermanentDeleteBatch provides a mock function with given fields: endTime, limit func (_m *PostStore) PermanentDeleteBatch(endTime int64, limit int64) (int64, error) { ret := _m.Called(endTime, limit) diff --git a/server/channels/store/timerlayer/timerlayer.go b/server/channels/store/timerlayer/timerlayer.go index 8ab2c0f696..b91949f031 100644 --- a/server/channels/store/timerlayer/timerlayer.go +++ b/server/channels/store/timerlayer/timerlayer.go @@ -3707,6 +3707,22 @@ func (s *TimerLayerFileInfoStore) PermanentDeleteByUser(ctx request.CTX, userID return result, err } +func (s *TimerLayerFileInfoStore) PermanentDeleteForPost(rctx request.CTX, postID string) error { + start := time.Now() + + err := s.FileInfoStore.PermanentDeleteForPost(rctx, postID) + + elapsed := float64(time.Since(start)) / float64(time.Second) + if s.Root.Metrics != nil { + success := "false" + if err == nil { + success = "true" + } + s.Root.Metrics.ObserveStoreMethodDuration("FileInfoStore.PermanentDeleteForPost", success, elapsed) + } + return err +} + func (s *TimerLayerFileInfoStore) Save(ctx request.CTX, info *model.FileInfo) (*model.FileInfo, error) { start := time.Now() @@ -6217,6 +6233,22 @@ func (s *TimerLayerPostStore) OverwriteMultiple(posts []*model.Post) ([]*model.P return result, resultVar1, err } +func (s *TimerLayerPostStore) PermanentDelete(rctx request.CTX, postID string) error { + start := time.Now() + + err := s.PostStore.PermanentDelete(rctx, postID) + + elapsed := float64(time.Since(start)) / float64(time.Second) + if s.Root.Metrics != nil { + success := "false" + if err == nil { + success = "true" + } + s.Root.Metrics.ObserveStoreMethodDuration("PostStore.PermanentDelete", success, elapsed) + } + return err +} + func (s *TimerLayerPostStore) PermanentDeleteBatch(endTime int64, limit int64) (int64, error) { start := time.Now() diff --git a/server/cmd/mmctl/client/client.go b/server/cmd/mmctl/client/client.go index fb7372d492..9bc1fa922d 100644 --- a/server/cmd/mmctl/client/client.go +++ b/server/cmd/mmctl/client/client.go @@ -159,4 +159,6 @@ type Client interface { GetPreferenceByCategoryAndName(ctx context.Context, userId, category, preferenceName string) (*model.Preference, *model.Response, error) UpdatePreferences(ctx context.Context, userId string, preferences model.Preferences) (*model.Response, error) DeletePreferences(ctx context.Context, userId string, preferences model.Preferences) (*model.Response, error) + PermanentDeletePost(ctx context.Context, postID string) (*model.Response, error) + DeletePost(ctx context.Context, postId string) (*model.Response, error) } diff --git a/server/cmd/mmctl/commands/post.go b/server/cmd/mmctl/commands/post.go index 74c9c18e5c..f3029e8b00 100644 --- a/server/cmd/mmctl/commands/post.go +++ b/server/cmd/mmctl/commands/post.go @@ -41,6 +41,22 @@ var PostListCmd = &cobra.Command{ RunE: withClient(postListCmdF), } +var PostDeleteCmd = &cobra.Command{ + Use: "delete [posts]", + Short: "Mark posts as deleted or permanently delete posts with the --permanent flag", + Long: `This command will mark the post as deleted and remove it from the user's clients, but it does not permanently delete the post from the database. Please use the --permanent flag to permanently delete a post and its attachments from your database.`, + Example: ` # Mark Post as deleted + $ mmctl post delete udjmt396tjghi8wnsk3a1qs1sw + + # Permanently delete a post and it's file contents from the database and filestore + $ mmctl post delete udjmt396tjghi8wnsk3a1qs1sw --permanent + + # Permanently delete multiple posts and their file contents from the database and filestore + $ mmctl post delete udjmt396tjghi8wnsk3a1qs1sw 7jgcjt7tyjyyu83qz81wo84w6o --permanent`, + Args: cobra.MinimumNArgs(1), + RunE: withClient(deletePostsCmdF), +} + const ( ISO8601Layout = "2006-01-02T15:04:05-07:00" PostTimeFormat = "2006-01-02 15:04:05-07:00" @@ -55,9 +71,13 @@ func init() { PostListCmd.Flags().BoolP("follow", "f", false, "Output appended data as new messages are posted to the channel") PostListCmd.Flags().StringP("since", "s", "", "List messages posted after a certain time (ISO 8601)") + PostDeleteCmd.Flags().Bool("confirm", false, "Confirm you really want to delete the post and a DB backup has been performed") + PostDeleteCmd.Flags().Bool("permanent", false, "Permanently delete the post and its contents from the database") + PostCmd.AddCommand( PostCreateCmd, PostListCmd, + PostDeleteCmd, ) RootCmd.AddCommand(PostCmd) @@ -217,3 +237,42 @@ func postListCmdF(c client.Client, cmd *cobra.Command, args []string) error { } return multiErr.ErrorOrNil() } + +func deletePostsCmdF(c client.Client, cmd *cobra.Command, args []string) error { + permanent, err := cmd.Flags().GetBool("permanent") + if err != nil { + return err + } + + confirmFlag, _ := cmd.Flags().GetBool("confirm") + if !confirmFlag && permanent { + if err = getConfirmation("Are you sure you want to delete the posts specified?", true); err != nil { + return err + } + } + + var result *multierror.Error + var deleteFunc func(ctx context.Context, postID string) (*model.Response, error) + + if permanent { + deleteFunc = c.PermanentDeletePost + } else { + deleteFunc = c.DeletePost + } + + for _, postID := range args { + isValidId := model.IsValidId(postID) + if !isValidId { + printer.PrintError(fmt.Sprintf("Invalid postID: %s", postID)) + result = multierror.Append(result, err) + continue + } + if _, err := deleteFunc(context.TODO(), postID); err != nil { + printer.PrintError(fmt.Sprintf("Error deleting post: %s. Error: %s", postID, err.Error())) + result = multierror.Append(result, err) + continue + } + printer.Print(fmt.Sprintf("%s successfully deleted", postID)) + } + return result.ErrorOrNil() +} diff --git a/server/cmd/mmctl/commands/post_test.go b/server/cmd/mmctl/commands/post_test.go index f272395782..2c10b8c452 100644 --- a/server/cmd/mmctl/commands/post_test.go +++ b/server/cmd/mmctl/commands/post_test.go @@ -5,6 +5,7 @@ package commands import ( "context" + "net/http" "time" "github.com/mattermost/mattermost/server/public/model" @@ -257,3 +258,122 @@ func (s *MmctlUnitTestSuite) TestPostListCmdF() { s.Len(printer.GetErrorLines(), 0) }) } + +func (s *MmctlUnitTestSuite) TestDeletePostsCmdF() { + postID1 := "ux9bxc1b8bf1zdoj1tfu14836e" + postID2 := "ux9bxc1b8bf1zdoj1tfu14836f" + + s.Run("invalid post id", func() { + id := "invalid-id" + + cmd := &cobra.Command{} + cmd.Flags().Bool("confirm", true, "") + cmd.Flags().Bool("permanent", false, "") + + err := deletePostsCmdF(s.client, cmd, []string{id}) + s.Require().Nil(err) + s.Require().Equal("Invalid postID: invalid-id", printer.GetErrorLines()[0]) + }) + + s.Run("successfully permanently delete one post", func() { + printer.Clean() + s.client. + EXPECT(). + PermanentDeletePost(context.TODO(), postID1). + Return(&model.Response{StatusCode: http.StatusOK}, nil). + Times(1) + cmd := &cobra.Command{} + cmd.Flags().Bool("confirm", true, "") + cmd.Flags().Bool("permanent", true, "") + + err := deletePostsCmdF(s.client, cmd, []string{postID1}) + s.Require().Nil(err) + s.Require().Equal(postID1+" successfully deleted", printer.GetLines()[0]) + }) + + s.Run("successfully soft delete one post", func() { + printer.Clean() + s.client. + EXPECT(). + DeletePost(context.TODO(), postID1). + Return(&model.Response{StatusCode: http.StatusOK}, nil). + Times(1) + cmd := &cobra.Command{} + cmd.Flags().Bool("confirm", true, "") + cmd.Flags().Bool("permanent", false, "") + + err := deletePostsCmdF(s.client, cmd, []string{postID1}) + s.Require().Nil(err) + s.Require().Equal(postID1+" successfully deleted", printer.GetLines()[0]) + }) + + s.Run("successfully delete multiple posts", func() { + printer.Clean() + s.client. + EXPECT(). + PermanentDeletePost(context.TODO(), postID1). + Return(&model.Response{StatusCode: http.StatusOK}, nil). + Times(1) + s.client. + EXPECT(). + PermanentDeletePost(context.TODO(), postID2). + Return(&model.Response{StatusCode: http.StatusOK}, nil). + Times(1) + cmd := &cobra.Command{} + cmd.Flags().Bool("confirm", true, "") + cmd.Flags().Bool("permanent", true, "") + + err := deletePostsCmdF(s.client, cmd, []string{postID1, postID2}) + s.Require().Nil(err) + s.Require().Equal(postID1+" successfully deleted", printer.GetLines()[0]) + s.Require().Equal(postID2+" successfully deleted", printer.GetLines()[1]) + }) + + s.Run("PermanentDeletePost api request returns an error", func() { + printer.Clean() + + mockError := errors.New("an error occurred on deleting a post") + + s.client. + EXPECT(). + PermanentDeletePost(context.TODO(), postID1). + Return(&model.Response{StatusCode: http.StatusBadRequest}, mockError). + Times(1) + + cmd := &cobra.Command{} + cmd.Flags().Bool("confirm", true, "") + cmd.Flags().Bool("permanent", true, "") + + err := deletePostsCmdF(s.client, cmd, []string{postID1}) + s.Require().ErrorContains(err, "an error occurred on deleting a post") + s.Require().Len(printer.GetErrorLines(), 1) + s.Require().Equal("Error deleting post: "+postID1+". Error: an error occurred on deleting a post", + printer.GetErrorLines()[0]) + }) + + s.Run("Delete multiple posts but one fails with an error", func() { + printer.Clean() + mockError := errors.New("an error occurred on deleting a post") + s.client. + EXPECT(). + PermanentDeletePost(context.TODO(), postID1). + Return(&model.Response{StatusCode: http.StatusOK}, nil). + Times(1) + s.client. + EXPECT(). + PermanentDeletePost(context.TODO(), postID2). + Return(&model.Response{StatusCode: http.StatusBadRequest}, mockError). + Times(1) + cmd := &cobra.Command{} + cmd.Flags().Bool("confirm", true, "") + cmd.Flags().Bool("permanent", true, "") + + err := deletePostsCmdF(s.client, cmd, []string{postID1, postID2}) + s.Require().ErrorContains(err, "an error occurred on deleting a post") + s.Require().Len(printer.GetLines(), 1) + s.Require().Len(printer.GetErrorLines(), 1) + s.Require().Equal(postID1+" successfully deleted", printer.GetLines()[0]) + s.Require().Equal("Error deleting post: "+postID2+". Error: an error occurred on deleting a post", + printer.GetErrorLines()[0]) + }) +} diff --git a/server/cmd/mmctl/docs/mmctl_post.rst b/server/cmd/mmctl/docs/mmctl_post.rst index af35928122..e06356c028 100644 --- a/server/cmd/mmctl/docs/mmctl_post.rst +++ b/server/cmd/mmctl/docs/mmctl_post.rst @@ -38,5 +38,6 @@ SEE ALSO * `mmctl `_ - Remote client for the Open Source, self-hosted Slack-alternative * `mmctl post create `_ - Create a post +* `mmctl post delete `_ - Mark posts as deleted or permanently delete posts with the --permanent flag * `mmctl post list `_ - List posts for a channel diff --git a/server/cmd/mmctl/docs/mmctl_post_delete.rst b/server/cmd/mmctl/docs/mmctl_post_delete.rst new file mode 100644 index 0000000000..db77abb03f --- /dev/null +++ b/server/cmd/mmctl/docs/mmctl_post_delete.rst @@ -0,0 +1,60 @@ +.. _mmctl_post_delete: + +mmctl post delete +----------------- + +Mark posts as deleted or permanently delete posts with the --permanent flag + +Synopsis +~~~~~~~~ + + +This command will mark the post as deleted and remove it from the user's clients, but it does not permanently delete the post from the database. Please use the --permanent flag to permanently delete a post and its attachments from your database. + +:: + + mmctl post delete [posts] [flags] + +Examples +~~~~~~~~ + +:: + + # Mark Post as deleted + $ mmctl post delete udjmt396tjghi8wnsk3a1qs1sw + + # Permanently delete a post and it's file contents from the database and filestore + $ mmctl post delete udjmt396tjghi8wnsk3a1qs1sw --permanent + + # Permanently delete multiple posts and their file contents from the database and filestore + $ mmctl post delete udjmt396tjghi8wnsk3a1qs1sw 7jgcjt7tyjyyu83qz81wo84w6o --permanent + +Options +~~~~~~~ + +:: + + --confirm Confirm you really want to delete the post and a DB backup has been performed + -h, --help help for delete + --permanent Permanently delete the post and its contents from the database + +Options inherited from parent commands +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +:: + + --config string path to the configuration file (default "$XDG_CONFIG_HOME/mmctl/config") + --disable-pager disables paged output + --insecure-sha1-intermediate allows to use insecure TLS protocols, such as SHA-1 + --insecure-tls-version allows to use TLS versions 1.0 and 1.1 + --json the output format will be in json format + --local allows communicating with the server through a unix socket + --quiet prevent mmctl to generate output for the commands + --strict will only run commands if the mmctl version matches the server one + --suppress-warnings disables printing warning messages + +SEE ALSO +~~~~~~~~ + +* `mmctl post `_ - Management of posts + diff --git a/server/cmd/mmctl/mocks/client_mock.go b/server/cmd/mmctl/mocks/client_mock.go index 6fe8d254cf..cd23d7ce59 100644 --- a/server/cmd/mmctl/mocks/client_mock.go +++ b/server/cmd/mmctl/mocks/client_mock.go @@ -417,6 +417,21 @@ func (mr *MockClientMockRecorder) DeleteOutgoingWebhook(arg0, arg1 interface{}) return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DeleteOutgoingWebhook", reflect.TypeOf((*MockClient)(nil).DeleteOutgoingWebhook), arg0, arg1) } +// DeletePost mocks base method. +func (m *MockClient) DeletePost(arg0 context.Context, arg1 string) (*model.Response, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "DeletePost", arg0, arg1) + ret0, _ := ret[0].(*model.Response) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// DeletePost indicates an expected call of DeletePost. +func (mr *MockClientMockRecorder) DeletePost(arg0, arg1 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DeletePost", reflect.TypeOf((*MockClient)(nil).DeletePost), arg0, arg1) +} + // DeletePreferences mocks base method. func (m *MockClient) DeletePreferences(arg0 context.Context, arg1 string, arg2 model.Preferences) (*model.Response, error) { m.ctrl.T.Helper() @@ -1750,6 +1765,21 @@ func (mr *MockClientMockRecorder) PermanentDeleteChannel(arg0, arg1 interface{}) return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "PermanentDeleteChannel", reflect.TypeOf((*MockClient)(nil).PermanentDeleteChannel), arg0, arg1) } +// PermanentDeletePost mocks base method. +func (m *MockClient) PermanentDeletePost(arg0 context.Context, arg1 string) (*model.Response, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "PermanentDeletePost", arg0, arg1) + ret0, _ := ret[0].(*model.Response) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// PermanentDeletePost indicates an expected call of PermanentDeletePost. +func (mr *MockClientMockRecorder) PermanentDeletePost(arg0, arg1 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "PermanentDeletePost", reflect.TypeOf((*MockClient)(nil).PermanentDeletePost), arg0, arg1) +} + // PermanentDeleteTeam mocks base method. func (m *MockClient) PermanentDeleteTeam(arg0 context.Context, arg1 string) (*model.Response, error) { m.ctrl.T.Helper() diff --git a/server/i18n/en.json b/server/i18n/en.json index 10b243a862..49dabc28f6 100644 --- a/server/i18n/en.json +++ b/server/i18n/en.json @@ -2516,6 +2516,10 @@ "id": "api.post.delete_post.can_not_delete_post_in_deleted.error", "translation": "Can not delete a post in a deleted channel." }, + { + "id": "api.post.delete_post.not_enabled.app_error", + "translation": "Cannot delete post, ServiceSettings.EnableAPIPostDeletion is not enabled." + }, { "id": "api.post.disabled_all", "translation": "@all has been disabled because the channel has more than {{.Users}} users." @@ -5082,6 +5086,10 @@ "id": "app.file_info.get.gif.app_error", "translation": "Could not decode gif." }, + { + "id": "app.file_info.get_by_post_id.app_error", + "translation": "Failed to find files for post." + }, { "id": "app.file_info.get_for_post.app_error", "translation": "Unable to get the file info for the post." @@ -5094,6 +5102,10 @@ "id": "app.file_info.permanent_delete_by_user.app_error", "translation": "Unable to delete attachments of the user." }, + { + "id": "app.file_info.permanent_delete_for_post.app_error", + "translation": "Failed to permanently delete file for post." + }, { "id": "app.file_info.save.app_error", "translation": "Unable to save the file info." @@ -6226,6 +6238,10 @@ "id": "app.post.permanent_delete_by_user.app_error", "translation": "Unable to select the posts to delete for the user." }, + { + "id": "app.post.permanent_delete_post.error", + "translation": "Failed to permanently delete post." + }, { "id": "app.post.save.app_error", "translation": "Unable to save the Post." diff --git a/server/platform/services/telemetry/telemetry.go b/server/platform/services/telemetry/telemetry.go index 70beddd333..1baa5e86c1 100644 --- a/server/platform/services/telemetry/telemetry.go +++ b/server/platform/services/telemetry/telemetry.go @@ -519,6 +519,7 @@ func (ts *TelemetryService) trackConfig() { "enable_api_team_deletion": *cfg.ServiceSettings.EnableAPITeamDeletion, "enable_api_trigger_admin_notification": *cfg.ServiceSettings.EnableAPITriggerAdminNotifications, "enable_api_user_deletion": *cfg.ServiceSettings.EnableAPIUserDeletion, + "enable_api_post_deletion": *cfg.ServiceSettings.EnableAPIPostDeletion, "enable_api_channel_deletion": *cfg.ServiceSettings.EnableAPIChannelDeletion, "experimental_enable_hardened_mode": *cfg.ServiceSettings.ExperimentalEnableHardenedMode, "experimental_strict_csrf_enforcement": *cfg.ServiceSettings.ExperimentalStrictCSRFEnforcement, diff --git a/server/public/model/client4.go b/server/public/model/client4.go index 37b1240792..15815f8d4b 100644 --- a/server/public/model/client4.go +++ b/server/public/model/client4.go @@ -4021,6 +4021,16 @@ func (c *Client4) DeletePost(ctx context.Context, postId string) (*Response, err return BuildResponse(r), nil } +// PermanentDeletePost permanently deletes a post and its files from the provided post id string. +func (c *Client4) PermanentDeletePost(ctx context.Context, postId string) (*Response, error) { + r, err := c.DoAPIDelete(ctx, c.postRoute(postId)+"?permanent="+c.boolString(true)) + if err != nil { + return BuildResponse(r), err + } + defer closeBody(r) + return BuildResponse(r), nil +} + // GetPostThread gets a post with all the other posts in the same thread. func (c *Client4) GetPostThread(ctx context.Context, postId string, etag string, collapsedThreads bool) (*PostList, *Response, error) { url := c.postRoute(postId) + "/thread" diff --git a/server/public/model/config.go b/server/public/model/config.go index dba1eee5e3..65fe958ab1 100644 --- a/server/public/model/config.go +++ b/server/public/model/config.go @@ -387,6 +387,7 @@ type ServiceSettings struct { EnableAPITeamDeletion *bool EnableAPITriggerAdminNotifications *bool EnableAPIUserDeletion *bool + EnableAPIPostDeletion *bool EnableDesktopLandingPage *bool ExperimentalEnableHardenedMode *bool `access:"experimental_features"` ExperimentalStrictCSRFEnforcement *bool `access:"experimental_features,write_restrictable,cloud_restrictable"` @@ -806,6 +807,10 @@ func (s *ServiceSettings) SetDefaults(isUpdate bool) { s.EnableAPIUserDeletion = NewPointer(false) } + if s.EnableAPIPostDeletion == nil { + s.EnableAPIPostDeletion = NewBool(false) + } + if s.EnableAPIChannelDeletion == nil { s.EnableAPIChannelDeletion = NewPointer(false) } diff --git a/webapp/channels/src/packages/mattermost-redux/src/reducers/entities/posts.test.ts b/webapp/channels/src/packages/mattermost-redux/src/reducers/entities/posts.test.ts index fe4293dcbc..40d263a26c 100644 --- a/webapp/channels/src/packages/mattermost-redux/src/reducers/entities/posts.test.ts +++ b/webapp/channels/src/packages/mattermost-redux/src/reducers/entities/posts.test.ts @@ -311,7 +311,7 @@ describe('posts', () => { expect(nextState).not.toBe(state); expect(nextState.post1).not.toBe(state.post1); expect(nextState).toEqual({ - post1: {id: 'post1', file_ids: [], has_reactions: false, state: Posts.POST_DELETED}, + post1: {id: 'post1', message: '', file_ids: [], has_reactions: false, state: Posts.POST_DELETED}, }); }); @@ -386,7 +386,7 @@ describe('posts', () => { expect(nextState.comment2).toBe(state.comment2); expect(nextState).toEqual({ post1: {id: 'post1'}, - comment1: {id: 'comment1', root_id: 'post1', file_ids: [], has_reactions: false, state: Posts.POST_DELETED}, + comment1: {id: 'comment1', message: '', root_id: 'post1', file_ids: [], has_reactions: false, state: Posts.POST_DELETED}, comment2: {id: 'comment2', root_id: 'post1'}, }); }); diff --git a/webapp/channels/src/packages/mattermost-redux/src/reducers/entities/posts.ts b/webapp/channels/src/packages/mattermost-redux/src/reducers/entities/posts.ts index cba1279a11..f6d8adf566 100644 --- a/webapp/channels/src/packages/mattermost-redux/src/reducers/entities/posts.ts +++ b/webapp/channels/src/packages/mattermost-redux/src/reducers/entities/posts.ts @@ -194,6 +194,7 @@ export function handlePosts(state: IDMappedObjects = {}, action: AnyAction [post.id]: { ...state[post.id], state: Posts.POST_DELETED, + message: '', file_ids: [], has_reactions: false, }, diff --git a/webapp/platform/types/src/config.ts b/webapp/platform/types/src/config.ts index 67ebe0f6bf..94ce938438 100644 --- a/webapp/platform/types/src/config.ts +++ b/webapp/platform/types/src/config.ts @@ -395,6 +395,7 @@ export type ServiceSettings = { UniqueEmojiReactionLimitPerPost: number; RefreshPostStatsRunTime: string; MaximumPayloadSizeBytes: number; + EnableAPIPostDeletion: boolean; MaximumURLLength: number; };