From 7602dc0b19ebc3e4aaad3a5a9cd27dc897ab516e Mon Sep 17 00:00:00 2001 From: Ashish Bhate Date: Wed, 22 Jul 2020 12:02:23 +0000 Subject: [PATCH] [MM-25647]: ability to permanently delete users (#14944) Summary: Config option to allow permanent user deletion ServiceSettings.EnableAPIUserDeletion Expose permanent user deletion through API Local mode for delete user for use in mmctl Ticket Link: Server part of https://mattermost.atlassian.net/browse/MM-25647 --- api4/user.go | 11 ++++++- api4/user_local.go | 33 +++++++++++++++++++++ api4/user_test.go | 72 +++++++++++++++++++++++++++++++++++----------- i18n/en.json | 4 +++ model/client4.go | 10 +++++++ model/config.go | 5 ++++ 6 files changed, 117 insertions(+), 18 deletions(-) diff --git a/api4/user.go b/api4/user.go index ecbdcf3a2a..3326115551 100644 --- a/api4/user.go +++ b/api4/user.go @@ -1193,7 +1193,16 @@ func deleteUser(c *Context, w http.ResponseWriter, r *http.Request) { } auditRec.AddMeta("user", user) - if _, err = c.App.UpdateActive(user, false); err != nil { + if c.Params.Permanent { + if *c.App.Config().ServiceSettings.EnableAPIUserDeletion { + err = c.App.PermanentDeleteUser(user) + } else { + err = model.NewAppError("deleteUser", "api.user.delete_user.not_enabled.app_error", nil, "userId="+c.Params.UserId, http.StatusUnauthorized) + } + } else { + _, err = c.App.UpdateActive(user, false) + } + if err != nil { c.Err = err return } diff --git a/api4/user_local.go b/api4/user_local.go index 2f54209f33..e94057dd2f 100644 --- a/api4/user_local.go +++ b/api4/user_local.go @@ -19,6 +19,7 @@ func (api *API) InitUserLocal() { api.BaseRoutes.User.Handle("", api.ApiLocal(getUser)).Methods("GET") api.BaseRoutes.User.Handle("", api.ApiLocal(updateUser)).Methods("PUT") + api.BaseRoutes.User.Handle("", api.ApiLocal(localDeleteUser)).Methods("DELETE") api.BaseRoutes.User.Handle("/roles", api.ApiLocal(updateUserRoles)).Methods("PUT") api.BaseRoutes.User.Handle("/mfa", api.ApiLocal(updateUserMfa)).Methods("PUT") api.BaseRoutes.User.Handle("/active", api.ApiLocal(updateUserActive)).Methods("PUT") @@ -32,6 +33,38 @@ func (api *API) InitUserLocal() { api.BaseRoutes.User.Handle("/tokens", api.ApiLocal(createUserAccessToken)).Methods("POST") } +func localDeleteUser(c *Context, w http.ResponseWriter, r *http.Request) { + c.RequireUserId() + if c.Err != nil { + return + } + + userId := c.Params.UserId + + auditRec := c.MakeAuditRecord("localDeleteUser", audit.Fail) + defer c.LogAuditRec(auditRec) + + user, err := c.App.GetUser(userId) + if err != nil { + c.Err = err + return + } + auditRec.AddMeta("user", user) + + if c.Params.Permanent { + err = c.App.PermanentDeleteUser(user) + } else { + _, err = c.App.UpdateActive(user, false) + } + if err != nil { + c.Err = err + return + } + + auditRec.Success() + ReturnStatusOK(w) +} + func localPermanentDeleteAllUsers(c *Context, w http.ResponseWriter, r *http.Request) { auditRec := c.MakeAuditRecord("localPermanentDeleteAllUsers", audit.Fail) defer c.LogAuditRec(auditRec) diff --git a/api4/user_test.go b/api4/user_test.go index d97e5e1dc4..d3547c55a2 100644 --- a/api4/user_test.go +++ b/api4/user_test.go @@ -1857,34 +1857,32 @@ func TestDeleteUser(t *testing.T) { th := Setup(t).InitBasic() defer th.TearDown() - user := th.BasicUser th.LoginBasic() - - testUser := th.SystemAdminUser - _, resp := th.Client.DeleteUser(testUser.Id) + _, resp := th.Client.DeleteUser(th.SystemAdminUser.Id) CheckForbiddenStatus(t, resp) th.Client.Logout() - - _, resp = th.Client.DeleteUser(user.Id) + _, resp = th.Client.DeleteUser(th.BasicUser.Id) CheckUnauthorizedStatus(t, resp) - th.Client.Login(testUser.Email, testUser.Password) + th.TestForSystemAdminAndLocal(t, func(t *testing.T, c *model.Client4) { + _, resp = c.DeleteUser(model.NewId()) + CheckNotFoundStatus(t, resp) - user.Id = model.NewId() - _, resp = th.Client.DeleteUser(user.Id) - CheckNotFoundStatus(t, resp) + _, resp = c.DeleteUser("junk") + CheckBadRequestStatus(t, resp) - user.Id = "junk" - _, resp = th.Client.DeleteUser(user.Id) - CheckBadRequestStatus(t, resp) - - _, resp = th.Client.DeleteUser(testUser.Id) - CheckNoError(t, resp) + userToDelete := th.CreateUser() + _, resp = c.DeleteUser(userToDelete.Id) + CheckNoError(t, resp) + }) selfDeleteUser := th.CreateUser() - th.Client.Login(selfDeleteUser.Email, selfDeleteUser.Password) + th.LoginBasic() + _, resp = th.Client.DeleteUser(selfDeleteUser.Id) + CheckForbiddenStatus(t, resp) + th.Client.Login(selfDeleteUser.Email, selfDeleteUser.Password) th.App.UpdateConfig(func(c *model.Config) { *c.TeamSettings.EnableUserDeactivation = false }) @@ -1898,6 +1896,46 @@ func TestDeleteUser(t *testing.T) { CheckNoError(t, resp) } +func TestPermanentDeleteUser(t *testing.T) { + th := Setup(t).InitBasic() + defer th.TearDown() + + enableAPIUserDeletion := *th.App.Config().ServiceSettings.EnableAPIUserDeletion + defer func() { + th.App.UpdateConfig(func(cfg *model.Config) { cfg.ServiceSettings.EnableAPIUserDeletion = &enableAPIUserDeletion }) + }() + + th.App.UpdateConfig(func(cfg *model.Config) { *cfg.ServiceSettings.EnableAPIUserDeletion = false }) + + userToDelete := th.CreateUser() + + t.Run("Permanent deletion not available through API if EnableAPIUserDeletion is not set", func(t *testing.T) { + _, resp := th.SystemAdminClient.PermanentDeleteUser(userToDelete.Id) + CheckUnauthorizedStatus(t, resp) + }) + + t.Run("Permanent deletion available through local mode even if EnableAPIUserDeletion is not set", func(t *testing.T) { + ok, resp := th.LocalClient.PermanentDeleteUser(userToDelete.Id) + CheckNoError(t, resp) + assert.True(t, ok) + }) + + th.App.UpdateConfig(func(cfg *model.Config) { *cfg.ServiceSettings.EnableAPIUserDeletion = true }) + th.TestForSystemAdminAndLocal(t, func(t *testing.T, c *model.Client4) { + userToDelete = th.CreateUser() + ok, resp := c.PermanentDeleteUser(userToDelete.Id) + CheckNoError(t, resp) + assert.True(t, ok) + + _, err := th.App.GetTeam(userToDelete.Id) + assert.NotNil(t, err) + + ok, resp = c.PermanentDeleteUser("junk") + CheckBadRequestStatus(t, resp) + require.False(t, ok, "should have returned false") + }, "Permanent deletion with EnableAPIUserDeletion set") +} + func TestPermanentDeleteAllUsers(t *testing.T) { th := Setup(t).InitBasic() defer th.TearDown() diff --git a/i18n/en.json b/i18n/en.json index 65383b5423..f31029db83 100644 --- a/i18n/en.json +++ b/i18n/en.json @@ -2622,6 +2622,10 @@ "id": "api.user.delete_team.not_enabled.app_error", "translation": "Permanent team deletion feature is not enabled. Please contact your System Administrator." }, + { + "id": "api.user.delete_user.not_enabled.app_error", + "translation": "Permanent user deletion feature is not enabled. Please contact your System Administrator." + }, { "id": "api.user.demote_user_to_guest.already_guest.app_error", "translation": "Unable to convert the user to guest because is already a guest." diff --git a/model/client4.go b/model/client4.go index d523410650..3319383bcc 100644 --- a/model/client4.go +++ b/model/client4.go @@ -1240,6 +1240,16 @@ func (c *Client4) DeleteUser(userId string) (bool, *Response) { return CheckStatusOK(r), BuildResponse(r) } +// PermanentDeleteUser deletes a user in the system based on the provided user id string. +func (c *Client4) PermanentDeleteUser(userId string) (bool, *Response) { + r, err := c.DoApiDelete(c.GetUserRoute(userId) + "?permanent=" + c.boolString(true)) + if err != nil { + return false, BuildErrorResponse(r, err) + } + defer closeBody(r) + return CheckStatusOK(r), BuildResponse(r) +} + // ConvertUserToBot converts a user to a bot user. func (c *Client4) ConvertUserToBot(userId string) (*Bot, *Response) { r, err := c.DoApiPost(c.GetUserRoute(userId)+"/convert_to_bot", "") diff --git a/model/config.go b/model/config.go index ff1821bc84..68ed3ecaec 100644 --- a/model/config.go +++ b/model/config.go @@ -327,6 +327,7 @@ type ServiceSettings struct { DEPRECATED_DO_NOT_USE_ImageProxyURL *string `json:"ImageProxyURL" mapstructure:"ImageProxyURL"` // This field is deprecated and must not be used. DEPRECATED_DO_NOT_USE_ImageProxyOptions *string `json:"ImageProxyOptions" mapstructure:"ImageProxyOptions"` // This field is deprecated and must not be used. EnableAPITeamDeletion *bool + EnableAPIUserDeletion *bool ExperimentalEnableHardenedMode *bool DisableLegacyMFA *bool `restricted:"true"` ExperimentalStrictCSRFEnforcement *bool `restricted:"true"` @@ -699,6 +700,10 @@ func (s *ServiceSettings) SetDefaults(isUpdate bool) { s.EnableAPITeamDeletion = NewBool(false) } + if s.EnableAPIUserDeletion == nil { + s.EnableAPIUserDeletion = NewBool(false) + } + if s.ExperimentalEnableHardenedMode == nil { s.ExperimentalEnableHardenedMode = NewBool(false) }