diff --git a/api4/user.go b/api4/user.go index 0892624ed0..d6a8d67b33 100644 --- a/api4/user.go +++ b/api4/user.go @@ -1051,6 +1051,11 @@ func updateUserActive(c *Context, w http.ResponseWriter, r *http.Request) { return } + if active && user.IsGuest() && !*c.App.Config().GuestAccountsSettings.Enable { + c.Err = model.NewAppError("updateUserActive", "api.user.update_active.cannot_enable_guest_when_guest_feature_is_disabled.app_error", nil, "userId="+c.Params.UserId, http.StatusUnauthorized) + return + } + if _, err = c.App.UpdateActive(user, active); err != nil { c.Err = err } diff --git a/api4/user_test.go b/api4/user_test.go index 06984e53e6..5f31f43992 100644 --- a/api4/user_test.go +++ b/api4/user_test.go @@ -1718,6 +1718,49 @@ func TestUpdateUserActive(t *testing.T) { assertWebsocketEventUserUpdatedWithEmail(t, webSocketClient, "") assertWebsocketEventUserUpdatedWithEmail(t, adminWebSocketClient, user.Email) }) + + t.Run("activate guest should fail when guests feature is disable", func(t *testing.T) { + th := Setup().InitBasic() + defer th.TearDown() + + id := model.NewId() + guest := &model.User{ + Email: "success+" + id + "@simulator.amazonses.com", + Username: "un_" + id, + Nickname: "nn_" + id, + Password: "Password1", + EmailVerified: true, + } + user, err := th.App.CreateGuest(guest) + require.Nil(t, err) + th.App.UpdateActive(user, false) + + th.App.UpdateConfig(func(cfg *model.Config) { *cfg.GuestAccountsSettings.Enable = false }) + defer th.App.UpdateConfig(func(cfg *model.Config) { *cfg.GuestAccountsSettings.Enable = true }) + _, resp := th.SystemAdminClient.UpdateUserActive(user.Id, true) + CheckUnauthorizedStatus(t, resp) + }) + + t.Run("activate guest should work when guests feature is enabled", func(t *testing.T) { + th := Setup().InitBasic() + defer th.TearDown() + + id := model.NewId() + guest := &model.User{ + Email: "success+" + id + "@simulator.amazonses.com", + Username: "un_" + id, + Nickname: "nn_" + id, + Password: "Password1", + EmailVerified: true, + } + user, err := th.App.CreateGuest(guest) + require.Nil(t, err) + th.App.UpdateActive(user, false) + + th.App.UpdateConfig(func(cfg *model.Config) { *cfg.GuestAccountsSettings.Enable = true }) + _, resp := th.SystemAdminClient.UpdateUserActive(user.Id, true) + CheckNoError(t, resp) + }) } func TestGetUsers(t *testing.T) { diff --git a/app/server.go b/app/server.go index daad2ed0d8..319e8cecdc 100644 --- a/app/server.go +++ b/app/server.go @@ -261,6 +261,21 @@ func NewServer(options ...Option) (*Server, error) { s.StartElasticsearch() } + s.AddConfigListener(func(oldConfig *model.Config, newConfig *model.Config) { + if *oldConfig.GuestAccountsSettings.Enable && !*newConfig.GuestAccountsSettings.Enable { + if appErr := s.FakeApp().DeactivateGuests(); appErr != nil { + mlog.Error("Unable to deactivate guest accounts", mlog.Err(appErr)) + } + } + }) + + // Disable active guest accounts on first run if guest accounts are disabled + if !*s.Config().GuestAccountsSettings.Enable { + if appErr := s.FakeApp().DeactivateGuests(); appErr != nil { + mlog.Error("Unable to deactivate guest accounts", mlog.Err(appErr)) + } + } + s.initJobs() if s.runjobs { diff --git a/app/user.go b/app/user.go index 878ce66a31..f491a0d841 100644 --- a/app/user.go +++ b/app/user.go @@ -943,28 +943,28 @@ func (a *App) UpdatePasswordAsUser(userId, currentPassword, newPassword string) return a.UpdatePasswordSendEmail(user, newPassword, T("api.user.update_password.menu")) } -func (a *App) userDeactivated(user *model.User) *model.AppError { - if err := a.RevokeAllSessions(user.Id); err != nil { +func (a *App) userDeactivated(userId string) *model.AppError { + if err := a.RevokeAllSessions(userId); err != nil { return err } - a.SetStatusOffline(user.Id, false) + a.SetStatusOffline(userId, false) if *a.Config().ServiceSettings.DisableBotsWhenOwnerIsDeactivated { - a.disableUserBots(user.Id) + a.disableUserBots(userId) } return nil } -func (a *App) invalidateUserChannelMembersCaches(user *model.User) *model.AppError { - teamsForUser, err := a.GetTeamsForUser(user.Id) +func (a *App) invalidateUserChannelMembersCaches(userId string) *model.AppError { + teamsForUser, err := a.GetTeamsForUser(userId) if err != nil { return err } for _, team := range teamsForUser { - channelsForUser, err := a.GetChannelsForUser(team.Id, user.Id, false) + channelsForUser, err := a.GetChannelsForUser(team.Id, userId, false) if err != nil { return err } @@ -992,12 +992,12 @@ func (a *App) UpdateActive(user *model.User, active bool) (*model.User, *model.A ruser := userUpdate.New if !active { - if err := a.userDeactivated(ruser); err != nil { + if err := a.userDeactivated(ruser.Id); err != nil { return nil, err } } - a.invalidateUserChannelMembersCaches(user) + a.invalidateUserChannelMembersCaches(user.Id) a.InvalidateCacheForUser(user.Id) a.sendUpdatedUserEvent(*ruser) @@ -1005,6 +1005,27 @@ func (a *App) UpdateActive(user *model.User, active bool) (*model.User, *model.A return ruser, nil } +func (a *App) DeactivateGuests() *model.AppError { + userIds, err := a.Srv.Store.User().DeactivateGuests() + if err != nil { + return err + } + + for _, userId := range userIds { + if err := a.userDeactivated(userId); err != nil { + return err + } + } + + a.Srv.Store.Channel().ClearCaches() + a.Srv.Store.User().ClearCaches() + + message := model.NewWebSocketEvent(model.WEBSOCKET_EVENT_GUESTS_DEACTIVATED, "", "", "", nil) + a.Publish(message) + + return nil +} + func (a *App) GetSanitizeOptions(asAdmin bool) map[string]bool { options := a.Config().GetSanitizeOptions() if asAdmin { diff --git a/app/user_test.go b/app/user_test.go index f0316bfcb9..d8cf99bd80 100644 --- a/app/user_test.go +++ b/app/user_test.go @@ -1164,3 +1164,27 @@ func TestDemoteUserToGuest(t *testing.T) { assert.Len(t, *channelMembers, 3) }) } + +func TestDeactivateGuests(t *testing.T) { + th := Setup(t).InitBasic() + defer th.TearDown() + + guest1 := th.CreateGuest() + guest2 := th.CreateGuest() + user := th.CreateUser() + + err := th.App.DeactivateGuests() + require.Nil(t, err) + + guest1, err = th.App.GetUser(guest1.Id) + assert.Nil(t, err) + assert.NotEqual(t, int64(0), guest1.DeleteAt) + + guest2, err = th.App.GetUser(guest2.Id) + assert.Nil(t, err) + assert.NotEqual(t, int64(0), guest2.DeleteAt) + + user, err = th.App.GetUser(user.Id) + assert.Nil(t, err) + assert.Equal(t, int64(0), user.DeleteAt) +} diff --git a/i18n/en.json b/i18n/en.json index 6631c537a2..33d9be2de8 100644 --- a/i18n/en.json +++ b/i18n/en.json @@ -2630,6 +2630,10 @@ "id": "api.user.send_welcome_email_and_forget.failed.error", "translation": "Failed to send welcome email successfully" }, + { + "id": "api.user.update_active.cannot_enable_guest_when_guest_feature_is_disabled.app_error", + "translation": "You cannot activate a guest account because Guest Access feature is not enabled." + }, { "id": "api.user.update_active.not_enable.app_error", "translation": "You cannot deactivate yourself because this feature is not enabled. Please contact your System Administrator." @@ -7158,6 +7162,14 @@ "id": "store.sql_user.update.username_taken.app_error", "translation": "This username is already taken. Please choose another." }, + { + "id": "store.sql_user.update_active_for_multiple_users.getting_changed_users.app_error", + "translation": "Unable to get the list of deactivate guests ids" + }, + { + "id": "store.sql_user.update_active_for_multiple_users.updating.app_error", + "translation": "Unable to deactivate guests" + }, { "id": "store.sql_user.update_auth_data.app_error", "translation": "Unable to update the auth data" diff --git a/model/websocket_message.go b/model/websocket_message.go index 132b89a084..90988e9755 100644 --- a/model/websocket_message.go +++ b/model/websocket_message.go @@ -51,6 +51,7 @@ const ( WEBSOCKET_EVENT_LICENSE_CHANGED = "license_changed" WEBSOCKET_EVENT_CONFIG_CHANGED = "config_changed" WEBSOCKET_EVENT_OPEN_DIALOG = "open_dialog" + WEBSOCKET_EVENT_GUESTS_DEACTIVATED = "guests_deactivated" ) type WebSocketMessage interface { diff --git a/store/sqlstore/user_store.go b/store/sqlstore/user_store.go index 84f5911a07..aa8c44ef8b 100644 --- a/store/sqlstore/user_store.go +++ b/store/sqlstore/user_store.go @@ -141,6 +141,40 @@ func (us SqlUserStore) Save(user *model.User) (*model.User, *model.AppError) { return user, nil } +func (us SqlUserStore) DeactivateGuests() ([]string, *model.AppError) { + curTime := model.GetMillis() + updateQuery := us.getQueryBuilder().Update("Users"). + Set("UpdateAt", curTime). + Set("DeleteAt", curTime). + Where(sq.Eq{"Roles": "system_guest"}). + Where(sq.Eq{"DeleteAt": 0}) + + queryString, args, err := updateQuery.ToSql() + if err != nil { + return nil, model.NewAppError("SqlUserStore.UpdateActiveForMultipleUsers", "store.sql_user.app_error", nil, err.Error(), http.StatusInternalServerError) + } + + _, err = us.GetMaster().Exec(queryString, args...) + if err != nil { + return nil, model.NewAppError("SqlUserStore.UpdateActiveForMultipleUsers", "store.sql_user.update_active_for_multiple_users.updating.app_error", nil, err.Error(), http.StatusInternalServerError) + } + + selectQuery := us.getQueryBuilder().Select("Id").From("Users").Where(sq.Eq{"DeleteAt": curTime}) + + queryString, args, err = selectQuery.ToSql() + if err != nil { + return nil, model.NewAppError("SqlUserStore.UpdateActiveForMultipleUsers", "store.sql_user.app_error", nil, err.Error(), http.StatusInternalServerError) + } + + userIds := []string{} + _, err = us.GetMaster().Select(&userIds, queryString, args...) + if err != nil { + return nil, model.NewAppError("SqlUserStore.UpdateActiveForMultipleUsers", "store.sql_user.update_active_for_multiple_users.getting_changed_users.app_error", nil, err.Error(), http.StatusInternalServerError) + } + + return userIds, nil +} + func (us SqlUserStore) Update(user *model.User, trustedUpdateData bool) (*model.UserUpdate, *model.AppError) { user.PreUpdate() diff --git a/store/store.go b/store/store.go index 330a95496e..fa811ae9b4 100644 --- a/store/store.go +++ b/store/store.go @@ -296,6 +296,7 @@ type UserStore interface { GetChannelGroupUsers(channelID string) ([]*model.User, *model.AppError) PromoteGuestToUser(userID string) *model.AppError DemoteUserToGuest(userID string) *model.AppError + DeactivateGuests() ([]string, *model.AppError) } type BotStore interface { diff --git a/store/storetest/mocks/UserStore.go b/store/storetest/mocks/UserStore.go index 43931cd758..69e7b5f003 100644 --- a/store/storetest/mocks/UserStore.go +++ b/store/storetest/mocks/UserStore.go @@ -128,6 +128,31 @@ func (_m *UserStore) Count(options model.UserCountOptions) (int64, *model.AppErr return r0, r1 } +// DeactivateGuests provides a mock function with given fields: +func (_m *UserStore) DeactivateGuests() ([]string, *model.AppError) { + ret := _m.Called() + + var r0 []string + if rf, ok := ret.Get(0).(func() []string); ok { + r0 = rf() + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).([]string) + } + } + + var r1 *model.AppError + if rf, ok := ret.Get(1).(func() *model.AppError); ok { + r1 = rf() + } else { + if ret.Get(1) != nil { + r1 = ret.Get(1).(*model.AppError) + } + } + + return r0, r1 +} + // DemoteUserToGuest provides a mock function with given fields: userID func (_m *UserStore) DemoteUserToGuest(userID string) *model.AppError { ret := _m.Called(userID) diff --git a/store/storetest/user_store.go b/store/storetest/user_store.go index bd67187bf2..d02646f7bc 100644 --- a/store/storetest/user_store.go +++ b/store/storetest/user_store.go @@ -80,6 +80,7 @@ func TestUserStore(t *testing.T, ss store.Store, s SqlSupplier) { t.Run("GetChannelGroupUsers", func(t *testing.T) { testUserStoreGetChannelGroupUsers(t, ss) }) t.Run("PromoteGuestToUser", func(t *testing.T) { testUserStorePromoteGuestToUser(t, ss) }) t.Run("DemoteUserToGuest", func(t *testing.T) { testUserStoreDemoteUserToGuest(t, ss) }) + t.Run("DeactivateGuests", func(t *testing.T) { testDeactivateGuests(t, ss) }) t.Run("ResetLastPictureUpdate", func(t *testing.T) { testUserStoreResetLastPictureUpdate(t, ss) }) } @@ -4206,6 +4207,7 @@ func testUserStorePromoteGuestToUser(t *testing.T, ss store.Store) { Roles: "system_user", }) require.Nil(t, err) + defer func() { require.Nil(t, ss.User().PermanentDelete(user.Id)) }() teamId := model.NewId() _, err = ss.Team().SaveMember(&model.TeamMember{TeamId: teamId, UserId: user.Id, SchemeGuest: true, SchemeUser: false}, 999) @@ -4251,6 +4253,7 @@ func testUserStorePromoteGuestToUser(t *testing.T, ss store.Store) { Roles: "system_user system_admin", }) require.Nil(t, err) + defer func() { require.Nil(t, ss.User().PermanentDelete(user.Id)) }() teamId := model.NewId() _, err = ss.Team().SaveMember(&model.TeamMember{TeamId: teamId, UserId: user.Id, SchemeGuest: true, SchemeUser: false}, 999) @@ -4295,6 +4298,7 @@ func testUserStorePromoteGuestToUser(t *testing.T, ss store.Store) { Roles: "system_guest", }) require.Nil(t, err) + defer func() { require.Nil(t, ss.User().PermanentDelete(user.Id)) }() err = ss.User().PromoteGuestToUser(user.Id) assert.Nil(t, err) @@ -4315,6 +4319,7 @@ func testUserStorePromoteGuestToUser(t *testing.T, ss store.Store) { Roles: "system_guest", }) require.Nil(t, err) + defer func() { require.Nil(t, ss.User().PermanentDelete(user.Id)) }() teamId := model.NewId() _, err = ss.Team().SaveMember(&model.TeamMember{TeamId: teamId, UserId: user.Id, SchemeGuest: true, SchemeUser: false}, 999) @@ -4344,6 +4349,7 @@ func testUserStorePromoteGuestToUser(t *testing.T, ss store.Store) { Roles: "system_guest", }) require.Nil(t, err) + defer func() { require.Nil(t, ss.User().PermanentDelete(user.Id)) }() teamId := model.NewId() _, err = ss.Team().SaveMember(&model.TeamMember{TeamId: teamId, UserId: user.Id, SchemeGuest: true, SchemeUser: false}, 999) @@ -4388,6 +4394,7 @@ func testUserStorePromoteGuestToUser(t *testing.T, ss store.Store) { Roles: "system_guest custom_role", }) require.Nil(t, err) + defer func() { require.Nil(t, ss.User().PermanentDelete(user.Id)) }() teamId := model.NewId() _, err = ss.Team().SaveMember(&model.TeamMember{TeamId: teamId, UserId: user.Id, SchemeGuest: true, SchemeUser: false}, 999) @@ -4432,6 +4439,7 @@ func testUserStorePromoteGuestToUser(t *testing.T, ss store.Store) { Roles: "system_guest", }) require.Nil(t, err) + defer func() { require.Nil(t, ss.User().PermanentDelete(user1.Id)) }() teamId1 := model.NewId() _, err = ss.Team().SaveMember(&model.TeamMember{TeamId: teamId1, UserId: user1.Id, SchemeGuest: true, SchemeUser: false}, 999) @@ -4459,6 +4467,7 @@ func testUserStorePromoteGuestToUser(t *testing.T, ss store.Store) { Roles: "system_guest", }) require.Nil(t, err) + defer func() { require.Nil(t, ss.User().PermanentDelete(user2.Id)) }() teamId2 := model.NewId() _, err = ss.Team().SaveMember(&model.TeamMember{TeamId: teamId2, UserId: user2.Id, SchemeGuest: true, SchemeUser: false}, 999) @@ -4513,6 +4522,7 @@ func testUserStoreDemoteUserToGuest(t *testing.T, ss store.Store) { Roles: "system_guest", }) require.Nil(t, err) + defer func() { require.Nil(t, ss.User().PermanentDelete(user.Id)) }() teamId := model.NewId() _, err = ss.Team().SaveMember(&model.TeamMember{TeamId: teamId, UserId: user.Id, SchemeGuest: false, SchemeUser: true}, 999) @@ -4558,6 +4568,7 @@ func testUserStoreDemoteUserToGuest(t *testing.T, ss store.Store) { Roles: "system_user system_admin", }) require.Nil(t, err) + defer func() { require.Nil(t, ss.User().PermanentDelete(user.Id)) }() teamId := model.NewId() _, err = ss.Team().SaveMember(&model.TeamMember{TeamId: teamId, UserId: user.Id, SchemeGuest: true, SchemeUser: false}, 999) @@ -4602,6 +4613,7 @@ func testUserStoreDemoteUserToGuest(t *testing.T, ss store.Store) { Roles: "system_user", }) require.Nil(t, err) + defer func() { require.Nil(t, ss.User().PermanentDelete(user.Id)) }() err = ss.User().DemoteUserToGuest(user.Id) assert.Nil(t, err) @@ -4622,6 +4634,7 @@ func testUserStoreDemoteUserToGuest(t *testing.T, ss store.Store) { Roles: "system_user", }) require.Nil(t, err) + defer func() { require.Nil(t, ss.User().PermanentDelete(user.Id)) }() teamId := model.NewId() _, err = ss.Team().SaveMember(&model.TeamMember{TeamId: teamId, UserId: user.Id, SchemeGuest: false, SchemeUser: true}, 999) @@ -4651,6 +4664,7 @@ func testUserStoreDemoteUserToGuest(t *testing.T, ss store.Store) { Roles: "system_user", }) require.Nil(t, err) + defer func() { require.Nil(t, ss.User().PermanentDelete(user.Id)) }() teamId := model.NewId() _, err = ss.Team().SaveMember(&model.TeamMember{TeamId: teamId, UserId: user.Id, SchemeGuest: false, SchemeUser: true}, 999) @@ -4695,6 +4709,7 @@ func testUserStoreDemoteUserToGuest(t *testing.T, ss store.Store) { Roles: "system_user custom_role", }) require.Nil(t, err) + defer func() { require.Nil(t, ss.User().PermanentDelete(user.Id)) }() teamId := model.NewId() _, err = ss.Team().SaveMember(&model.TeamMember{TeamId: teamId, UserId: user.Id, SchemeGuest: false, SchemeUser: true}, 999) @@ -4739,6 +4754,7 @@ func testUserStoreDemoteUserToGuest(t *testing.T, ss store.Store) { Roles: "system_user", }) require.Nil(t, err) + defer func() { require.Nil(t, ss.User().PermanentDelete(user1.Id)) }() teamId1 := model.NewId() _, err = ss.Team().SaveMember(&model.TeamMember{TeamId: teamId1, UserId: user1.Id, SchemeGuest: false, SchemeUser: true}, 999) @@ -4766,6 +4782,7 @@ func testUserStoreDemoteUserToGuest(t *testing.T, ss store.Store) { Roles: "system_user", }) require.Nil(t, err) + defer func() { require.Nil(t, ss.User().PermanentDelete(user2.Id)) }() teamId2 := model.NewId() _, err = ss.Team().SaveMember(&model.TeamMember{TeamId: teamId2, UserId: user2.Id, SchemeGuest: false, SchemeUser: true}, 999) @@ -4806,6 +4823,84 @@ func testUserStoreDemoteUserToGuest(t *testing.T, ss store.Store) { }) } +func testDeactivateGuests(t *testing.T, ss store.Store) { + // create users + t.Run("Must disable all guests and no regular user or already deactivated users", func(t *testing.T) { + guest1Random := model.NewId() + guest1, err := ss.User().Save(&model.User{ + Email: guest1Random + "@test.com", + Username: "un_" + guest1Random, + Nickname: "nn_" + guest1Random, + FirstName: "f_" + guest1Random, + LastName: "l_" + guest1Random, + Password: "Password1", + Roles: "system_guest", + }) + require.Nil(t, err) + defer func() { require.Nil(t, ss.User().PermanentDelete(guest1.Id)) }() + + guest2Random := model.NewId() + guest2, err := ss.User().Save(&model.User{ + Email: guest2Random + "@test.com", + Username: "un_" + guest2Random, + Nickname: "nn_" + guest2Random, + FirstName: "f_" + guest2Random, + LastName: "l_" + guest2Random, + Password: "Password1", + Roles: "system_guest", + }) + require.Nil(t, err) + defer func() { require.Nil(t, ss.User().PermanentDelete(guest2.Id)) }() + + guest3Random := model.NewId() + guest3, err := ss.User().Save(&model.User{ + Email: guest3Random + "@test.com", + Username: "un_" + guest3Random, + Nickname: "nn_" + guest3Random, + FirstName: "f_" + guest3Random, + LastName: "l_" + guest3Random, + Password: "Password1", + Roles: "system_guest", + DeleteAt: 10, + }) + require.Nil(t, err) + defer func() { require.Nil(t, ss.User().PermanentDelete(guest3.Id)) }() + + regularUserRandom := model.NewId() + regularUser, err := ss.User().Save(&model.User{ + Email: regularUserRandom + "@test.com", + Username: "un_" + regularUserRandom, + Nickname: "nn_" + regularUserRandom, + FirstName: "f_" + regularUserRandom, + LastName: "l_" + regularUserRandom, + Password: "Password1", + Roles: "system_user", + }) + require.Nil(t, err) + defer func() { require.Nil(t, ss.User().PermanentDelete(regularUser.Id)) }() + + ids, err := ss.User().DeactivateGuests() + require.Nil(t, err) + assert.ElementsMatch(t, []string{guest1.Id, guest2.Id}, ids) + + u, err := ss.User().Get(guest1.Id) + require.Nil(t, err) + assert.NotEqual(t, u.DeleteAt, int64(0)) + + u, err = ss.User().Get(guest2.Id) + require.Nil(t, err) + assert.NotEqual(t, u.DeleteAt, int64(0)) + + u, err = ss.User().Get(guest3.Id) + require.Nil(t, err) + assert.Equal(t, u.DeleteAt, int64(10)) + + u, err = ss.User().Get(regularUser.Id) + require.Nil(t, err) + assert.Equal(t, u.DeleteAt, int64(0)) + }) +} + func testUserStoreResetLastPictureUpdate(t *testing.T, ss store.Store) { u1 := &model.User{} u1.Email = MakeEmail()