Skip to content

Commit a981291

Browse files
willypuzzlehanzei
andauthored
MM-42819 mmctl: informative errors on permanent deletions (mattermost#30230)
Co-authored-by: Ben Schumacher <ben.schumacher@mattermost.com>
1 parent c9837e1 commit a981291

File tree

7 files changed

+137
-5
lines changed

7 files changed

+137
-5
lines changed

server/channels/api4/channel.go

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1401,7 +1401,13 @@ func deleteChannel(c *Context, w http.ResponseWriter, r *http.Request) {
14011401
if *c.App.Config().ServiceSettings.EnableAPIChannelDeletion {
14021402
err = c.App.PermanentDeleteChannel(c.AppContext, channel)
14031403
} else {
1404-
err = model.NewAppError("deleteChannel", "api.user.delete_channel.not_enabled.app_error", nil, "channelId="+c.Params.ChannelId, http.StatusUnauthorized)
1404+
user, usrErr := c.App.GetUser(c.AppContext.Session().UserId)
1405+
if usrErr == nil && user != nil && user.IsSystemAdmin() {
1406+
// More verbose error message for system admins
1407+
err = model.NewAppError("deleteChannel", "api.user.delete_channel.not_enabled.for_admin.app_error", nil, "channelId="+c.Params.ChannelId, http.StatusUnauthorized)
1408+
} else {
1409+
err = model.NewAppError("deleteChannel", "api.user.delete_channel.not_enabled.app_error", nil, "channelId="+c.Params.ChannelId, http.StatusUnauthorized)
1410+
}
14051411
}
14061412
} else {
14071413
err = c.App.DeleteChannel(c.AppContext, channel, c.AppContext.Session().UserId)

server/channels/api4/team.go

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -521,7 +521,13 @@ func deleteTeam(c *Context, w http.ResponseWriter, r *http.Request) {
521521
if *c.App.Config().ServiceSettings.EnableAPITeamDeletion {
522522
err = c.App.PermanentDeleteTeamId(c.AppContext, c.Params.TeamId)
523523
} else {
524-
err = model.NewAppError("deleteTeam", "api.user.delete_team.not_enabled.app_error", nil, "teamId="+c.Params.TeamId, http.StatusUnauthorized)
524+
user, usrErr := c.App.GetUser(c.AppContext.Session().UserId)
525+
if usrErr == nil && user != nil && user.IsSystemAdmin() {
526+
// More verbose error message for system admins
527+
err = model.NewAppError("deleteTeam", "api.user.delete_team.not_enabled.for_admin.app_error", nil, "teamId="+c.Params.TeamId, http.StatusUnauthorized)
528+
} else {
529+
err = model.NewAppError("deleteTeam", "api.user.delete_team.not_enabled.app_error", nil, "teamId="+c.Params.TeamId, http.StatusUnauthorized)
530+
}
525531
}
526532
} else {
527533
err = c.App.SoftDeleteTeam(c.Params.TeamId)

server/channels/api4/user.go

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1534,7 +1534,13 @@ func deleteUser(c *Context, w http.ResponseWriter, r *http.Request) {
15341534
if *c.App.Config().ServiceSettings.EnableAPIUserDeletion {
15351535
err = c.App.PermanentDeleteUser(c.AppContext, user)
15361536
} else {
1537-
err = model.NewAppError("deleteUser", "api.user.delete_user.not_enabled.app_error", nil, "userId="+c.Params.UserId, http.StatusUnauthorized)
1537+
loggedUser, usrErr := c.App.GetUser(c.AppContext.Session().UserId)
1538+
if usrErr == nil && loggedUser != nil && loggedUser.IsSystemAdmin() {
1539+
// More verbose error message for system admins
1540+
err = model.NewAppError("deleteUser", "api.user.delete_user.not_enabled.for_admin.app_error", nil, "userId="+c.Params.UserId, http.StatusUnauthorized)
1541+
} else {
1542+
err = model.NewAppError("deleteUser", "api.user.delete_user.not_enabled.app_error", nil, "userId="+c.Params.UserId, http.StatusUnauthorized)
1543+
}
15381544
}
15391545
} else {
15401546
_, err = c.App.UpdateActive(c.AppContext, user, false)

server/cmd/mmctl/commands/channel_e2e_test.go

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -360,6 +360,63 @@ func (s *MmctlE2ETestSuite) TestDeleteChannelsCmd() {
360360
s.CheckErrorID(err, "app.channel.get.existing.app_error")
361361
})
362362

363+
s.Run("Delete channel with disabled config as system admin", func() {
364+
channel, appErr := s.th.App.CreateChannel(s.th.Context, &model.Channel{Type: model.ChannelTypeOpen, Name: "channel_name_you_cannot_delete", CreatorId: user.Id}, true)
365+
s.Require().Nil(appErr)
366+
367+
previousVal := s.th.App.Config().ServiceSettings.EnableAPIChannelDeletion
368+
s.th.App.UpdateConfig(func(cfg *model.Config) { *cfg.ServiceSettings.EnableAPIChannelDeletion = false })
369+
defer func() {
370+
s.th.App.UpdateConfig(func(cfg *model.Config) { *cfg.ServiceSettings.EnableAPIChannelDeletion = *previousVal })
371+
}()
372+
373+
cmd := &cobra.Command{}
374+
cmd.Flags().Bool("confirm", true, "")
375+
args := []string{team.Id + ":" + channel.Id}
376+
377+
printer.Clean()
378+
err := deleteChannelsCmdF(s.th.SystemAdminClient, cmd, args)
379+
380+
var expected error
381+
expected = multierror.Append(expected, errors.New("unable to delete channel '\""+channel.Name+"\"' error: Permanent channel deletion feature is not enabled. ServiceSettings.EnableAPIChannelDeletion must be set to true to use this command. See https://mattermost.com/pl/environment-configuration-settings for more information"))
382+
383+
s.Require().NotNil(err)
384+
s.Require().EqualError(err, expected.Error())
385+
386+
channel, err = s.th.App.GetChannel(s.th.Context, channel.Id)
387+
388+
s.Require().Nil(err)
389+
s.Require().NotNil(channel)
390+
})
391+
392+
s.Run("Delete channel with disabled config as local client", func() {
393+
channel, appErr := s.th.App.CreateChannel(s.th.Context, &model.Channel{Type: model.ChannelTypeOpen, Name: "channel_name", CreatorId: user.Id}, true)
394+
s.Require().Nil(appErr)
395+
396+
previousVal := s.th.App.Config().ServiceSettings.EnableAPIChannelDeletion
397+
s.th.App.UpdateConfig(func(cfg *model.Config) { *cfg.ServiceSettings.EnableAPIChannelDeletion = false })
398+
defer func() {
399+
s.th.App.UpdateConfig(func(cfg *model.Config) { *cfg.ServiceSettings.EnableAPIChannelDeletion = *previousVal })
400+
}()
401+
402+
cmd := &cobra.Command{}
403+
cmd.Flags().Bool("confirm", true, "")
404+
args := []string{team.Id + ":" + channel.Id}
405+
406+
printer.Clean()
407+
err := deleteChannelsCmdF(s.th.LocalClient, cmd, args)
408+
409+
s.Require().Nil(err)
410+
s.Require().Len(printer.GetLines(), 1)
411+
s.Require().Equal(channel, printer.GetLines()[0])
412+
s.Require().Len(printer.GetErrorLines(), 0)
413+
414+
// expect the channel deleted
415+
_, err = s.th.App.GetChannel(s.th.Context, channel.Id)
416+
s.Require().NotNil(err)
417+
s.CheckErrorID(err, "app.channel.get.existing.app_error")
418+
})
419+
363420
s.Run("Delete channel without permissions", func() {
364421
cmd := &cobra.Command{}
365422
cmd.Flags().Bool("confirm", true, "")

server/cmd/mmctl/commands/team_e2e_test.go

Lines changed: 46 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -125,6 +125,51 @@ func (s *MmctlE2ETestSuite) TestDeleteTeamsCmdF() {
125125
config, _, _ = c.GetConfig(context.TODO())
126126
config.ServiceSettings.EnableAPITeamDeletion = &enableConfig
127127
_, _, _ = c.UpdateConfig(context.TODO(), config)
128+
129+
_, err = s.th.App.GetTeam(teamName)
130+
s.Require().NotNil(err)
131+
})
132+
133+
s.Run("Delete team with disabled config as local client", func() {
134+
printer.Clean()
135+
136+
teamName := "teamname" + model.NewRandomString(10)
137+
teamDisplayname := "Mock Display Name"
138+
cmd := &cobra.Command{}
139+
cmd.Flags().String("name", teamName, "")
140+
cmd.Flags().String("display-name", teamDisplayname, "")
141+
err := createTeamCmdF(s.th.LocalClient, cmd, []string{})
142+
s.Require().Nil(err)
143+
144+
cmd = &cobra.Command{}
145+
args := []string{teamName}
146+
cmd.Flags().String("display-name", "newDisplayName", "Team Display Name")
147+
cmd.Flags().Bool("confirm", true, "")
148+
149+
c := s.th.LocalClient
150+
151+
enableConfig := false
152+
config, _, _ := c.GetConfig(context.TODO())
153+
holdConfig := config.ServiceSettings.EnableAPITeamDeletion
154+
// Set EnableAPITeamDeletion
155+
config.ServiceSettings.EnableAPITeamDeletion = &enableConfig
156+
_, _, _ = c.UpdateConfig(context.TODO(), config)
157+
158+
// Deletion should succeed for local client now
159+
err = deleteTeamsCmdF(c, cmd, args)
160+
s.Require().Nil(err)
161+
team := printer.GetLines()[0].(*model.Team)
162+
s.Equal(teamName, team.Name)
163+
s.Len(printer.GetErrorLines(), 0)
164+
165+
// Reset config
166+
config, _, _ = c.GetConfig(context.TODO())
167+
config.ServiceSettings.EnableAPITeamDeletion = holdConfig
168+
_, _, _ = c.UpdateConfig(context.TODO(), config)
169+
170+
// expect team is deleted
171+
_, err = s.th.App.GetTeam(teamName)
172+
s.Require().NotNil(err)
128173
})
129174

130175
s.Run("Permission denied error for system admin when deleting a valid team", func() {
@@ -140,7 +185,7 @@ func (s *MmctlE2ETestSuite) TestDeleteTeamsCmdF() {
140185
s.Require().Error(err)
141186
s.Len(printer.GetLines(), 0)
142187
s.Len(printer.GetErrorLines(), 1)
143-
s.Equal("Unable to delete team '"+s.th.BasicTeam.Name+"' error: Permanent team deletion feature is not enabled. Please contact your System Administrator.", printer.GetErrorLines()[0])
188+
s.Equal("Unable to delete team '"+s.th.BasicTeam.Name+"' error: Permanent team deletion feature is not enabled. ServiceSettings.EnableAPITeamDeletion must be set to true to use this command. See https://mattermost.com/pl/environment-configuration-settings for more information.", printer.GetErrorLines()[0])
144189

145190
// verify team still exists
146191
team, _ := s.th.App.GetTeam(s.th.BasicTeam.Id)

server/cmd/mmctl/commands/user_e2e_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -764,7 +764,7 @@ func (s *MmctlE2ETestSuite) TestDeleteUsersCmd() {
764764

765765
var expectedErr *multierror.Error
766766
expectedErr = multierror.Append(expectedErr, fmt.Errorf("unable to delete user %s error: %w", newUser.Username,
767-
fmt.Errorf("Permanent user deletion feature is not enabled. Please contact your System Administrator.")))
767+
fmt.Errorf("Permanent user deletion feature is not enabled. ServiceSettings.EnableAPIUserDeletion must be set to true to use this command. See https://mattermost.com/pl/environment-configuration-settings for more information.")))
768768

769769
err := deleteUsersCmdF(s.th.SystemAdminClient, cmd, []string{newUser.Email})
770770
s.Require().NotNil(err)

server/i18n/en.json

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4182,14 +4182,26 @@
41824182
"id": "api.user.delete_channel.not_enabled.app_error",
41834183
"translation": "Permanent channel deletion feature is not enabled. Please contact your System Administrator."
41844184
},
4185+
{
4186+
"id": "api.user.delete_channel.not_enabled.for_admin.app_error",
4187+
"translation": "Permanent channel deletion feature is not enabled. ServiceSettings.EnableAPIChannelDeletion must be set to true to use this command. See https://mattermost.com/pl/environment-configuration-settings for more information"
4188+
},
41854189
{
41864190
"id": "api.user.delete_team.not_enabled.app_error",
41874191
"translation": "Permanent team deletion feature is not enabled. Please contact your System Administrator."
41884192
},
4193+
{
4194+
"id": "api.user.delete_team.not_enabled.for_admin.app_error",
4195+
"translation": "Permanent team deletion feature is not enabled. ServiceSettings.EnableAPITeamDeletion must be set to true to use this command. See https://mattermost.com/pl/environment-configuration-settings for more information."
4196+
},
41894197
{
41904198
"id": "api.user.delete_user.not_enabled.app_error",
41914199
"translation": "Permanent user deletion feature is not enabled. Please contact your System Administrator."
41924200
},
4201+
{
4202+
"id": "api.user.delete_user.not_enabled.for_admin.app_error",
4203+
"translation": "Permanent user deletion feature is not enabled. ServiceSettings.EnableAPIUserDeletion must be set to true to use this command. See https://mattermost.com/pl/environment-configuration-settings for more information."
4204+
},
41934205
{
41944206
"id": "api.user.demote_user_to_guest.already_guest.app_error",
41954207
"translation": "Unable to convert the user to guest because is already a guest."

0 commit comments

Comments
 (0)