diff --git a/api4/group.go b/api4/group.go index 6315288f83..35dffb6125 100644 --- a/api4/group.go +++ b/api4/group.go @@ -247,7 +247,7 @@ func linkGroupSyncable(c *Context, w http.ResponseWriter, r *http.Request) { } c.App.Srv().Go(func() { - c.App.SyncRolesAndMembership(syncableID, syncableType) + c.App.SyncRolesAndMembership(syncableID, syncableType, false) }) w.WriteHeader(http.StatusCreated) @@ -408,7 +408,7 @@ func patchGroupSyncable(c *Context, w http.ResponseWriter, r *http.Request) { auditRec.AddMeta("new_syncable_type", groupSyncable.Type) c.App.Srv().Go(func() { - c.App.SyncRolesAndMembership(syncableID, syncableType) + c.App.SyncRolesAndMembership(syncableID, syncableType, false) }) b, marshalErr := json.Marshal(groupSyncable) @@ -462,7 +462,7 @@ func unlinkGroupSyncable(c *Context, w http.ResponseWriter, r *http.Request) { } c.App.Srv().Go(func() { - c.App.SyncRolesAndMembership(syncableID, syncableType) + c.App.SyncRolesAndMembership(syncableID, syncableType, false) }) auditRec.Success() diff --git a/api4/ldap.go b/api4/ldap.go index 3013bcb54a..7b89fc35e3 100644 --- a/api4/ldap.go +++ b/api4/ldap.go @@ -47,6 +47,12 @@ func syncLdap(c *Context, w http.ResponseWriter, r *http.Request) { return } + type LdapSyncOptions struct { + IncludeRemovedMembers bool `json:"include_removed_members"` + } + var opts LdapSyncOptions + json.NewDecoder(r.Body).Decode(&opts) + auditRec := c.MakeAuditRecord("syncLdap", audit.Fail) defer c.LogAuditRec(auditRec) @@ -55,7 +61,7 @@ func syncLdap(c *Context, w http.ResponseWriter, r *http.Request) { return } - c.App.SyncLdap() + c.App.SyncLdap(opts.IncludeRemovedMembers) auditRec.Success() ReturnStatusOK(w) diff --git a/api4/ldap_test.go b/api4/ldap_test.go index ef092bab40..73b54d893e 100644 --- a/api4/ldap_test.go +++ b/api4/ldap_test.go @@ -8,7 +8,9 @@ import ( "github.com/stretchr/testify/require" + "github.com/mattermost/mattermost-server/v5/einterfaces/mocks" "github.com/mattermost/mattermost-server/v5/model" + "github.com/mattermost/mattermost-server/v5/plugin/plugintest/mock" ) var spPrivateKey = `-----BEGIN PRIVATE KEY----- @@ -134,13 +136,37 @@ func TestSyncLdap(t *testing.T) { }) th.App.Srv().SetLicense(model.NewTestLicense("ldap_groups")) - - th.TestForSystemAdminAndLocal(t, func(t *testing.T, client *model.Client4) { - _, resp := client.SyncLdap() - CheckNoError(t, resp) + th.App.UpdateConfig(func(cfg *model.Config) { + *cfg.LdapSettings.EnableSync = true }) - _, resp := th.Client.SyncLdap() + ldapMock := &mocks.LdapInterface{} + mockCall := ldapMock.On( + "StartSynchronizeJob", + mock.AnythingOfType("bool"), + mock.AnythingOfType("bool"), + ).Return(nil, nil) + ready := make(chan bool) + includeRemovedMembers := false + mockCall.RunFn = func(args mock.Arguments) { + includeRemovedMembers = args[1].(bool) + ready <- true + } + th.App.Srv().Ldap = ldapMock + + th.TestForSystemAdminAndLocal(t, func(t *testing.T, client *model.Client4) { + _, resp := client.SyncLdap(false) + <-ready + CheckNoError(t, resp) + require.Equal(t, false, includeRemovedMembers) + + _, resp = client.SyncLdap(true) + <-ready + CheckNoError(t, resp) + require.Equal(t, true, includeRemovedMembers) + }) + + _, resp := th.Client.SyncLdap(false) CheckForbiddenStatus(t, resp) } diff --git a/app/app_iface.go b/app/app_iface.go index d1c6f9145a..b7b82616b3 100644 --- a/app/app_iface.go +++ b/app/app_iface.go @@ -60,6 +60,13 @@ type AppIface interface { // The result can be used, for example, to determine the set of users who would be removed from a channel if the // channel were group-constrained with the given groups. ChannelMembersMinusGroupMembers(channelID string, groupIDs []string, page, perPage int) ([]*model.UserWithGroups, int64, *model.AppError) + // ChannelMembersToAdd returns a slice of UserChannelIDPair that need newly created memberships + // based on the groups configurations. The returned list can be optionally scoped to a single given channel. + // + // Typically since will be the last successful group sync time. + // If includeRemovedMembers is true, then channel members who left or were removed from the channel will + // be included; otherwise, they will be excluded. + ChannelMembersToAdd(since int64, channelID *string, includeRemovedMembers bool) ([]*model.UserChannelIDPair, *model.AppError) // CheckProviderAttributes returns the empty string if the patch can be applied without // overriding attributes set by the user's login provider; otherwise, the name of the offending // field is returned. @@ -79,7 +86,9 @@ type AppIface interface { CreateDefaultChannels(teamID string) ([]*model.Channel, *model.AppError) // CreateDefaultMemberships adds users to teams and channels based on their group memberships and how those groups // are configured to sync with teams and channels for group members on or after the given timestamp. - CreateDefaultMemberships(since int64) error + // If includeRemovedMembers is true, then members who left or were removed from a team/channel will + // be re-added; otherwise, they will not be re-added. + CreateDefaultMemberships(since int64, includeRemovedMembers bool) error // CreateGuest creates a guest and sets several fields of the returned User struct to // their zero values. CreateGuest(user *model.User) (*model.User, *model.AppError) @@ -305,12 +314,16 @@ type AppIface interface { // status to away if needed. Used by the WS to set status to away if an 'online' device disconnects // while an 'away' device is still connected SetStatusLastActivityAt(userID string, activityAt int64) + // SyncLdap starts an LDAP sync job. + // If includeRemovedMembers is true, then members who left or were removed from a team/channel will + // be re-added; otherwise, they will not be re-added. + SyncLdap(includeRemovedMembers bool) // SyncPlugins synchronizes the plugins installed locally // with the plugin bundles available in the file store. SyncPlugins() *model.AppError // SyncRolesAndMembership updates the SchemeAdmin status and membership of all of the members of the given // syncable. - SyncRolesAndMembership(syncableID string, syncableType model.GroupSyncableType) + SyncRolesAndMembership(syncableID string, syncableType model.GroupSyncableType, includeRemovedMembers bool) // SyncSyncableRoles updates the SchemeAdmin field value of the given syncable's members based on the configuration of // the member's group memberships and the configuration of those groups to the syncable. This method should only // be invoked on group-synced (aka group-constrained) syncables. @@ -321,6 +334,13 @@ type AppIface interface { // The result can be used, for example, to determine the set of users who would be removed from a team if the team // were group-constrained with the given groups. TeamMembersMinusGroupMembers(teamID string, groupIDs []string, page, perPage int) ([]*model.UserWithGroups, int64, *model.AppError) + // TeamMembersToAdd returns a slice of UserTeamIDPair that need newly created memberships + // based on the groups configurations. The returned list can be optionally scoped to a single given team. + // + // Typically since will be the last successful group sync time. + // If includeRemovedMembers is true, then team members who left or were removed from the team will + // be included; otherwise, they will be excluded. + TeamMembersToAdd(since int64, teamID *string, includeRemovedMembers bool) ([]*model.UserTeamIDPair, *model.AppError) // This function migrates the default built in roles from code/config to the database. DoAdvancedPermissionsMigration() // This function zip's up all the files in fileDatas array and then saves it to the directory specified with the specified zip file name @@ -408,7 +428,6 @@ type AppIface interface { BulkImport(fileReader io.Reader, dryRun bool, workers int) (*model.AppError, int) BulkImportWithPath(fileReader io.Reader, dryRun bool, workers int, importPath string) (*model.AppError, int) CancelJob(jobId string) *model.AppError - ChannelMembersToAdd(since int64, channelID *string) ([]*model.UserChannelIDPair, *model.AppError) ChannelMembersToRemove(teamID *string) ([]*model.ChannelMember, *model.AppError) CheckAndSendUserLimitWarningEmails() *model.AppError CheckCanInviteToSharedChannel(channelId string) error @@ -1014,10 +1033,8 @@ type AppIface interface { SwitchEmailToOAuth(w http.ResponseWriter, r *http.Request, email, password, code, service string) (string, *model.AppError) SwitchLdapToEmail(ldapPassword, code, email, newPassword string) (string, *model.AppError) SwitchOAuthToEmail(email, password, requesterId string) (string, *model.AppError) - SyncLdap() SyncPluginsActiveState() T(translationID string, args ...interface{}) string - TeamMembersToAdd(since int64, teamID *string) ([]*model.UserTeamIDPair, *model.AppError) TeamMembersToRemove(teamID *string) ([]*model.TeamMember, *model.AppError) TelemetryId() string TestElasticsearch(cfg *model.Config) *model.AppError diff --git a/app/group.go b/app/group.go index 14e46c4a7b..f7393500b9 100644 --- a/app/group.go +++ b/app/group.go @@ -399,8 +399,14 @@ func (a *App) DeleteGroupSyncable(groupID string, syncableID string, syncableTyp return gs, nil } -func (a *App) TeamMembersToAdd(since int64, teamID *string) ([]*model.UserTeamIDPair, *model.AppError) { - userTeams, err := a.Srv().Store.Group().TeamMembersToAdd(since, teamID) +// TeamMembersToAdd returns a slice of UserTeamIDPair that need newly created memberships +// based on the groups configurations. The returned list can be optionally scoped to a single given team. +// +// Typically since will be the last successful group sync time. +// If includeRemovedMembers is true, then team members who left or were removed from the team will +// be included; otherwise, they will be excluded. +func (a *App) TeamMembersToAdd(since int64, teamID *string, includeRemovedMembers bool) ([]*model.UserTeamIDPair, *model.AppError) { + userTeams, err := a.Srv().Store.Group().TeamMembersToAdd(since, teamID, includeRemovedMembers) if err != nil { return nil, model.NewAppError("TeamMembersToAdd", "app.select_error", nil, err.Error(), http.StatusInternalServerError) } @@ -408,8 +414,14 @@ func (a *App) TeamMembersToAdd(since int64, teamID *string) ([]*model.UserTeamID return userTeams, nil } -func (a *App) ChannelMembersToAdd(since int64, channelID *string) ([]*model.UserChannelIDPair, *model.AppError) { - userChannels, err := a.Srv().Store.Group().ChannelMembersToAdd(since, channelID) +// ChannelMembersToAdd returns a slice of UserChannelIDPair that need newly created memberships +// based on the groups configurations. The returned list can be optionally scoped to a single given channel. +// +// Typically since will be the last successful group sync time. +// If includeRemovedMembers is true, then channel members who left or were removed from the channel will +// be included; otherwise, they will be excluded. +func (a *App) ChannelMembersToAdd(since int64, channelID *string, includeRemovedMembers bool) ([]*model.UserChannelIDPair, *model.AppError) { + userChannels, err := a.Srv().Store.Group().ChannelMembersToAdd(since, channelID, includeRemovedMembers) if err != nil { return nil, model.NewAppError("ChannelMembersToAdd", "app.select_error", nil, err.Error(), http.StatusInternalServerError) } diff --git a/app/ldap.go b/app/ldap.go index ec32a5758f..2fcb75b224 100644 --- a/app/ldap.go +++ b/app/ldap.go @@ -13,12 +13,15 @@ import ( "github.com/mattermost/mattermost-server/v5/shared/mlog" ) -func (a *App) SyncLdap() { +// SyncLdap starts an LDAP sync job. +// If includeRemovedMembers is true, then members who left or were removed from a team/channel will +// be re-added; otherwise, they will not be re-added. +func (a *App) SyncLdap(includeRemovedMembers bool) { a.Srv().Go(func() { if license := a.Srv().License(); license != nil && *license.Features.LDAP && *a.Config().LdapSettings.EnableSync { if ldapI := a.Ldap(); ldapI != nil { - ldapI.StartSynchronizeJob(false) + ldapI.StartSynchronizeJob(false, includeRemovedMembers) } else { mlog.Error("Not executing ldap sync because ldap is not available") } diff --git a/app/opentracing/opentracing_layer.go b/app/opentracing/opentracing_layer.go index ffcc729a7b..bb753821d3 100644 --- a/app/opentracing/opentracing_layer.go +++ b/app/opentracing/opentracing_layer.go @@ -1054,7 +1054,7 @@ func (a *OpenTracingAppLayer) ChannelMembersMinusGroupMembers(channelID string, return resultVar0, resultVar1, resultVar2 } -func (a *OpenTracingAppLayer) ChannelMembersToAdd(since int64, channelID *string) ([]*model.UserChannelIDPair, *model.AppError) { +func (a *OpenTracingAppLayer) ChannelMembersToAdd(since int64, channelID *string, includeRemovedMembers bool) ([]*model.UserChannelIDPair, *model.AppError) { origCtx := a.ctx span, newCtx := tracing.StartSpanWithParentByContext(a.ctx, "app.ChannelMembersToAdd") @@ -1066,7 +1066,7 @@ func (a *OpenTracingAppLayer) ChannelMembersToAdd(since int64, channelID *string }() defer span.Finish() - resultVar0, resultVar1 := a.app.ChannelMembersToAdd(since, channelID) + resultVar0, resultVar1 := a.app.ChannelMembersToAdd(since, channelID, includeRemovedMembers) if resultVar1 != nil { span.LogFields(spanlog.Error(resultVar1)) @@ -1878,7 +1878,7 @@ func (a *OpenTracingAppLayer) CreateDefaultChannels(teamID string) ([]*model.Cha return resultVar0, resultVar1 } -func (a *OpenTracingAppLayer) CreateDefaultMemberships(since int64) error { +func (a *OpenTracingAppLayer) CreateDefaultMemberships(since int64, includeRemovedMembers bool) error { origCtx := a.ctx span, newCtx := tracing.StartSpanWithParentByContext(a.ctx, "app.CreateDefaultMemberships") @@ -1890,7 +1890,7 @@ func (a *OpenTracingAppLayer) CreateDefaultMemberships(since int64) error { }() defer span.Finish() - resultVar0 := a.app.CreateDefaultMemberships(since) + resultVar0 := a.app.CreateDefaultMemberships(since, includeRemovedMembers) if resultVar0 != nil { span.LogFields(spanlog.Error(resultVar0)) @@ -15239,7 +15239,7 @@ func (a *OpenTracingAppLayer) SwitchOAuthToEmail(email string, password string, return resultVar0, resultVar1 } -func (a *OpenTracingAppLayer) SyncLdap() { +func (a *OpenTracingAppLayer) SyncLdap(includeRemovedMembers bool) { origCtx := a.ctx span, newCtx := tracing.StartSpanWithParentByContext(a.ctx, "app.SyncLdap") @@ -15251,7 +15251,7 @@ func (a *OpenTracingAppLayer) SyncLdap() { }() defer span.Finish() - a.app.SyncLdap() + a.app.SyncLdap(includeRemovedMembers) } func (a *OpenTracingAppLayer) SyncPlugins() *model.AppError { @@ -15291,7 +15291,7 @@ func (a *OpenTracingAppLayer) SyncPluginsActiveState() { a.app.SyncPluginsActiveState() } -func (a *OpenTracingAppLayer) SyncRolesAndMembership(syncableID string, syncableType model.GroupSyncableType) { +func (a *OpenTracingAppLayer) SyncRolesAndMembership(syncableID string, syncableType model.GroupSyncableType, includeRemovedMembers bool) { origCtx := a.ctx span, newCtx := tracing.StartSpanWithParentByContext(a.ctx, "app.SyncRolesAndMembership") @@ -15303,7 +15303,7 @@ func (a *OpenTracingAppLayer) SyncRolesAndMembership(syncableID string, syncable }() defer span.Finish() - a.app.SyncRolesAndMembership(syncableID, syncableType) + a.app.SyncRolesAndMembership(syncableID, syncableType, includeRemovedMembers) } func (a *OpenTracingAppLayer) SyncSyncableRoles(syncableID string, syncableType model.GroupSyncableType) *model.AppError { @@ -15350,7 +15350,7 @@ func (a *OpenTracingAppLayer) TeamMembersMinusGroupMembers(teamID string, groupI return resultVar0, resultVar1, resultVar2 } -func (a *OpenTracingAppLayer) TeamMembersToAdd(since int64, teamID *string) ([]*model.UserTeamIDPair, *model.AppError) { +func (a *OpenTracingAppLayer) TeamMembersToAdd(since int64, teamID *string, includeRemovedMembers bool) ([]*model.UserTeamIDPair, *model.AppError) { origCtx := a.ctx span, newCtx := tracing.StartSpanWithParentByContext(a.ctx, "app.TeamMembersToAdd") @@ -15362,7 +15362,7 @@ func (a *OpenTracingAppLayer) TeamMembersToAdd(since int64, teamID *string) ([]* }() defer span.Finish() - resultVar0, resultVar1 := a.app.TeamMembersToAdd(since, teamID) + resultVar0, resultVar1 := a.app.TeamMembersToAdd(since, teamID, includeRemovedMembers) if resultVar1 != nil { span.LogFields(spanlog.Error(resultVar1)) diff --git a/app/syncables.go b/app/syncables.go index 2cda472959..b2fa0c81d0 100644 --- a/app/syncables.go +++ b/app/syncables.go @@ -15,8 +15,10 @@ import ( // createDefaultChannelMemberships adds users to channels based on their group memberships and how those groups are // configured to sync with channels for group members on or after the given timestamp. If a channelID is given // only that channel's members are created. If channelID is nil all channel memberships are created. -func (a *App) createDefaultChannelMemberships(since int64, channelID *string) error { - channelMembers, appErr := a.ChannelMembersToAdd(since, channelID) +// If includeRemovedMembers is true, then channel members who left or were removed from the channel will +// be re-added; otherwise, they will not be re-added. +func (a *App) createDefaultChannelMemberships(since int64, channelID *string, includeRemovedMembers bool) error { + channelMembers, appErr := a.ChannelMembersToAdd(since, channelID, includeRemovedMembers) if appErr != nil { return appErr } @@ -78,8 +80,10 @@ func (a *App) createDefaultChannelMemberships(since int64, channelID *string) er // createDefaultTeamMemberships adds users to teams based on their group memberships and how those groups are // configured to sync with teams for group members on or after the given timestamp. If a teamID is given // only that team's members are created. If teamID is nil all team memberships are created. -func (a *App) createDefaultTeamMemberships(since int64, teamID *string) error { - teamMembers, appErr := a.TeamMembersToAdd(since, teamID) +// If includeRemovedMembers is true, then team members who left or were removed from the team will +// be re-added; otherwise, they will not be re-added. +func (a *App) createDefaultTeamMemberships(since int64, teamID *string, includeRemovedMembers bool) error { + teamMembers, appErr := a.TeamMembersToAdd(since, teamID, includeRemovedMembers) if appErr != nil { return appErr } @@ -108,13 +112,15 @@ func (a *App) createDefaultTeamMemberships(since int64, teamID *string) error { // CreateDefaultMemberships adds users to teams and channels based on their group memberships and how those groups // are configured to sync with teams and channels for group members on or after the given timestamp. -func (a *App) CreateDefaultMemberships(since int64) error { - err := a.createDefaultTeamMemberships(since, nil) +// If includeRemovedMembers is true, then members who left or were removed from a team/channel will +// be re-added; otherwise, they will not be re-added. +func (a *App) CreateDefaultMemberships(since int64, includeRemovedMembers bool) error { + err := a.createDefaultTeamMemberships(since, nil, includeRemovedMembers) if err != nil { return err } - err = a.createDefaultChannelMemberships(since, nil) + err = a.createDefaultChannelMemberships(since, nil, includeRemovedMembers) if err != nil { return err } @@ -226,7 +232,7 @@ func (a *App) SyncSyncableRoles(syncableID string, syncableType model.GroupSynca // SyncRolesAndMembership updates the SchemeAdmin status and membership of all of the members of the given // syncable. -func (a *App) SyncRolesAndMembership(syncableID string, syncableType model.GroupSyncableType) { +func (a *App) SyncRolesAndMembership(syncableID string, syncableType model.GroupSyncableType, includeRemovedMembers bool) { a.SyncSyncableRoles(syncableID, syncableType) lastJob, _ := a.Srv().Store.Job().GetNewestJobByStatusAndType(model.JOB_STATUS_SUCCESS, model.JOB_TYPE_LDAP_SYNC) @@ -237,11 +243,11 @@ func (a *App) SyncRolesAndMembership(syncableID string, syncableType model.Group switch syncableType { case model.GroupSyncableTypeTeam: - a.createDefaultTeamMemberships(since, &syncableID) + a.createDefaultTeamMemberships(since, &syncableID, includeRemovedMembers) a.deleteGroupConstrainedTeamMemberships(&syncableID) a.ClearTeamMembersCache(syncableID) case model.GroupSyncableTypeChannel: - a.createDefaultChannelMemberships(since, &syncableID) + a.createDefaultChannelMemberships(since, &syncableID, includeRemovedMembers) a.deleteGroupConstrainedChannelMemberships(&syncableID) a.ClearChannelMembersCache(syncableID) } diff --git a/app/syncables_test.go b/app/syncables_test.go index fc9be6bdeb..ff723bc367 100644 --- a/app/syncables_test.go +++ b/app/syncables_test.go @@ -104,7 +104,7 @@ func TestCreateDefaultMemberships(t *testing.T) { t.Errorf("test groupmember not created: %s", err.Error()) } - pErr := th.App.CreateDefaultMemberships(0) + pErr := th.App.CreateDefaultMemberships(0, false) if pErr != nil { t.Errorf("faild to populate syncables: %s", pErr.Error()) } @@ -174,7 +174,7 @@ func TestCreateDefaultMemberships(t *testing.T) { } // Sync everything after syncable was created (proving that team updates trigger re-sync) - pErr = th.App.CreateDefaultMemberships(scientistGroupMember.CreateAt + 1) + pErr = th.App.CreateDefaultMemberships(scientistGroupMember.CreateAt+1, false) if pErr != nil { t.Errorf("faild to populate syncables: %s", pErr.Error()) } @@ -217,7 +217,7 @@ func TestCreateDefaultMemberships(t *testing.T) { } // Sync everything after syncable was created (proving that channel updates trigger re-sync) - pErr = th.App.CreateDefaultMemberships(scientistGroupMember.CreateAt + 1) + pErr = th.App.CreateDefaultMemberships(scientistGroupMember.CreateAt+1, false) if pErr != nil { t.Errorf("faild to populate syncables: %s", pErr.Error()) } @@ -242,7 +242,7 @@ func TestCreateDefaultMemberships(t *testing.T) { } // Even re-syncing from the beginning doesn't re-add to channel or team - pErr = th.App.CreateDefaultMemberships(0) + pErr = th.App.CreateDefaultMemberships(0, false) if pErr != nil { t.Errorf("faild to populate syncables: %s", pErr.Error()) } @@ -283,7 +283,7 @@ func TestCreateDefaultMemberships(t *testing.T) { t.Errorf("error updating group syncable: %s", err.Error()) } - pErr = th.App.CreateDefaultMemberships(0) + pErr = th.App.CreateDefaultMemberships(0, false) if pErr != nil { t.Errorf("faild to populate syncables: %s", pErr.Error()) } @@ -304,7 +304,7 @@ func TestCreateDefaultMemberships(t *testing.T) { t.Errorf("error permanently deleting channelmemberhistory: %s", nErr.Error()) } - pErr = th.App.CreateDefaultMemberships(scienceChannelGroupSyncable.UpdateAt) + pErr = th.App.CreateDefaultMemberships(scienceChannelGroupSyncable.UpdateAt, false) if pErr != nil { t.Errorf("failed to populate syncables: %s", pErr.Error()) } @@ -320,7 +320,7 @@ func TestCreateDefaultMemberships(t *testing.T) { t.Errorf("error permanently deleting channelmemberhistory: %s", nErr.Error()) } - pErr = th.App.CreateDefaultMemberships(scienceChannelGroupSyncable.UpdateAt) + pErr = th.App.CreateDefaultMemberships(scienceChannelGroupSyncable.UpdateAt, false) if pErr != nil { t.Errorf("failed to populate syncables: %s", pErr.Error()) } @@ -360,7 +360,7 @@ func TestCreateDefaultMemberships(t *testing.T) { _, err = th.App.UpsertGroupSyncable(model.NewGroupChannel(scienceGroup.Id, restrictedChannel.Id, true)) require.Nil(t, err) - pErr = th.App.CreateDefaultMemberships(0) + pErr = th.App.CreateDefaultMemberships(0, false) require.NoError(t, pErr) // Ensure only the restricted user was added to both the team and channel diff --git a/cmd/mattermost/commands/ldap.go b/cmd/mattermost/commands/ldap.go index 3f80de269d..5c42b7992c 100644 --- a/cmd/mattermost/commands/ldap.go +++ b/cmd/mattermost/commands/ldap.go @@ -33,6 +33,7 @@ var LdapIdMigrate = &cobra.Command{ } func init() { + LdapSyncCmd.Flags().Bool("include-removed-members", false, "Include members who left or were removed from a group-synced team/channel") LdapCmd.AddCommand( LdapSyncCmd, LdapIdMigrate, @@ -47,8 +48,10 @@ func ldapSyncCmdF(command *cobra.Command, args []string) error { } defer a.Srv().Shutdown() + includeRemovedMembers, _ := command.Flags().GetBool("include-removed-members") + if ldapI := a.Ldap(); ldapI != nil { - job, err := ldapI.StartSynchronizeJob(true) + job, err := ldapI.StartSynchronizeJob(true, includeRemovedMembers) if err != nil || job.Status == model.JOB_STATUS_ERROR || job.Status == model.JOB_STATUS_CANCELED { CommandPrintErrorln("ERROR: AD/LDAP Synchronization please check the server logs") } else { diff --git a/einterfaces/ldap.go b/einterfaces/ldap.go index 88c4a6359a..305b015020 100644 --- a/einterfaces/ldap.go +++ b/einterfaces/ldap.go @@ -15,7 +15,7 @@ type LdapInterface interface { CheckPasswordAuthData(authData string, password string) *model.AppError CheckProviderAttributes(LS *model.LdapSettings, ouser *model.User, patch *model.UserPatch) string SwitchToLdap(userID, ldapID, ldapPassword string) *model.AppError - StartSynchronizeJob(waitForJobToFinish bool) (*model.Job, *model.AppError) + StartSynchronizeJob(waitForJobToFinish bool, includeRemovedMembers bool) (*model.Job, *model.AppError) RunTest() *model.AppError GetAllLdapUsers() ([]*model.User, *model.AppError) MigrateIDAttribute(toAttribute string) error diff --git a/einterfaces/mocks/LdapInterface.go b/einterfaces/mocks/LdapInterface.go index 29e71883d1..031da77e4f 100644 --- a/einterfaces/mocks/LdapInterface.go +++ b/einterfaces/mocks/LdapInterface.go @@ -312,13 +312,13 @@ func (_m *LdapInterface) RunTest() *model.AppError { return r0 } -// StartSynchronizeJob provides a mock function with given fields: waitForJobToFinish -func (_m *LdapInterface) StartSynchronizeJob(waitForJobToFinish bool) (*model.Job, *model.AppError) { - ret := _m.Called(waitForJobToFinish) +// StartSynchronizeJob provides a mock function with given fields: waitForJobToFinish, includeRemovedMembers +func (_m *LdapInterface) StartSynchronizeJob(waitForJobToFinish bool, includeRemovedMembers bool) (*model.Job, *model.AppError) { + ret := _m.Called(waitForJobToFinish, includeRemovedMembers) var r0 *model.Job - if rf, ok := ret.Get(0).(func(bool) *model.Job); ok { - r0 = rf(waitForJobToFinish) + if rf, ok := ret.Get(0).(func(bool, bool) *model.Job); ok { + r0 = rf(waitForJobToFinish, includeRemovedMembers) } else { if ret.Get(0) != nil { r0 = ret.Get(0).(*model.Job) @@ -326,8 +326,8 @@ func (_m *LdapInterface) StartSynchronizeJob(waitForJobToFinish bool) (*model.Jo } var r1 *model.AppError - if rf, ok := ret.Get(1).(func(bool) *model.AppError); ok { - r1 = rf(waitForJobToFinish) + if rf, ok := ret.Get(1).(func(bool, bool) *model.AppError); ok { + r1 = rf(waitForJobToFinish, includeRemovedMembers) } else { if ret.Get(1) != nil { r1 = ret.Get(1).(*model.AppError) diff --git a/model/client4.go b/model/client4.go index 8f523b1f6c..f49ca3c3e2 100644 --- a/model/client4.go +++ b/model/client4.go @@ -4033,8 +4033,13 @@ func (c *Client4) GetClusterStatus() ([]*ClusterInfo, *Response) { // LDAP Section // SyncLdap will force a sync with the configured LDAP server. -func (c *Client4) SyncLdap() (bool, *Response) { - r, err := c.DoApiPost(c.GetLdapRoute()+"/sync", "") +// If includeRemovedMembers is true, then group members who left or were removed from a +// synced team/channel will be re-joined; otherwise, they will be excluded. +func (c *Client4) SyncLdap(includeRemovedMembers bool) (bool, *Response) { + reqBody, _ := json.Marshal(map[string]interface{}{ + "include_removed_members": includeRemovedMembers, + }) + r, err := c.doApiPostBytes(c.GetLdapRoute()+"/sync", reqBody) if err != nil { return false, BuildErrorResponse(r, err) } diff --git a/store/opentracinglayer/opentracinglayer.go b/store/opentracinglayer/opentracinglayer.go index 3f747fe3b4..59e6ed5a45 100644 --- a/store/opentracinglayer/opentracinglayer.go +++ b/store/opentracinglayer/opentracinglayer.go @@ -3367,7 +3367,7 @@ func (s *OpenTracingLayerGroupStore) ChannelMembersMinusGroupMembers(channelID s return result, err } -func (s *OpenTracingLayerGroupStore) ChannelMembersToAdd(since int64, channelID *string) ([]*model.UserChannelIDPair, error) { +func (s *OpenTracingLayerGroupStore) ChannelMembersToAdd(since int64, channelID *string, includeRemovedMembers bool) ([]*model.UserChannelIDPair, error) { origCtx := s.Root.Store.Context() span, newCtx := tracing.StartSpanWithParentByContext(s.Root.Store.Context(), "GroupStore.ChannelMembersToAdd") s.Root.Store.SetContext(newCtx) @@ -3376,7 +3376,7 @@ func (s *OpenTracingLayerGroupStore) ChannelMembersToAdd(since int64, channelID }() defer span.Finish() - result, err := s.GroupStore.ChannelMembersToAdd(since, channelID) + result, err := s.GroupStore.ChannelMembersToAdd(since, channelID, includeRemovedMembers) if err != nil { span.LogFields(spanlog.Error(err)) ext.Error.Set(span, true) @@ -4033,7 +4033,7 @@ func (s *OpenTracingLayerGroupStore) TeamMembersMinusGroupMembers(teamID string, return result, err } -func (s *OpenTracingLayerGroupStore) TeamMembersToAdd(since int64, teamID *string) ([]*model.UserTeamIDPair, error) { +func (s *OpenTracingLayerGroupStore) TeamMembersToAdd(since int64, teamID *string, includeRemovedMembers bool) ([]*model.UserTeamIDPair, error) { origCtx := s.Root.Store.Context() span, newCtx := tracing.StartSpanWithParentByContext(s.Root.Store.Context(), "GroupStore.TeamMembersToAdd") s.Root.Store.SetContext(newCtx) @@ -4042,7 +4042,7 @@ func (s *OpenTracingLayerGroupStore) TeamMembersToAdd(since int64, teamID *strin }() defer span.Finish() - result, err := s.GroupStore.TeamMembersToAdd(since, teamID) + result, err := s.GroupStore.TeamMembersToAdd(since, teamID, includeRemovedMembers) if err != nil { span.LogFields(spanlog.Error(err)) ext.Error.Set(span, true) diff --git a/store/retrylayer/retrylayer.go b/store/retrylayer/retrylayer.go index 1a92a9766f..c50709d187 100644 --- a/store/retrylayer/retrylayer.go +++ b/store/retrylayer/retrylayer.go @@ -3606,11 +3606,11 @@ func (s *RetryLayerGroupStore) ChannelMembersMinusGroupMembers(channelID string, } -func (s *RetryLayerGroupStore) ChannelMembersToAdd(since int64, channelID *string) ([]*model.UserChannelIDPair, error) { +func (s *RetryLayerGroupStore) ChannelMembersToAdd(since int64, channelID *string, includeRemovedMembers bool) ([]*model.UserChannelIDPair, error) { tries := 0 for { - result, err := s.GroupStore.ChannelMembersToAdd(since, channelID) + result, err := s.GroupStore.ChannelMembersToAdd(since, channelID, includeRemovedMembers) if err == nil { return result, nil } @@ -4346,11 +4346,11 @@ func (s *RetryLayerGroupStore) TeamMembersMinusGroupMembers(teamID string, group } -func (s *RetryLayerGroupStore) TeamMembersToAdd(since int64, teamID *string) ([]*model.UserTeamIDPair, error) { +func (s *RetryLayerGroupStore) TeamMembersToAdd(since int64, teamID *string, includeRemovedMembers bool) ([]*model.UserTeamIDPair, error) { tries := 0 for { - result, err := s.GroupStore.TeamMembersToAdd(since, teamID) + result, err := s.GroupStore.TeamMembersToAdd(since, teamID, includeRemovedMembers) if err == nil { return result, nil } diff --git a/store/sqlstore/group_store.go b/store/sqlstore/group_store.go index 519c9bafbc..ad432270cb 100644 --- a/store/sqlstore/group_store.go +++ b/store/sqlstore/group_store.go @@ -706,23 +706,29 @@ func (s *SqlGroupStore) DeleteGroupSyncable(groupID string, syncableID string, s return groupSyncable, nil } -func (s *SqlGroupStore) TeamMembersToAdd(since int64, teamID *string) ([]*model.UserTeamIDPair, error) { +func (s *SqlGroupStore) TeamMembersToAdd(since int64, teamID *string, includeRemovedMembers bool) ([]*model.UserTeamIDPair, error) { builder := s.getQueryBuilder().Select("GroupMembers.UserId", "GroupTeams.TeamId"). From("GroupMembers"). Join("GroupTeams ON GroupTeams.GroupId = GroupMembers.GroupId"). Join("UserGroups ON UserGroups.Id = GroupMembers.GroupId"). Join("Teams ON Teams.Id = GroupTeams.TeamId"). - JoinClause("LEFT OUTER JOIN TeamMembers ON TeamMembers.TeamId = GroupTeams.TeamId AND TeamMembers.UserId = GroupMembers.UserId"). Where(sq.Eq{ - "TeamMembers.UserId": nil, "UserGroups.DeleteAt": 0, "GroupTeams.DeleteAt": 0, "GroupTeams.AutoAdd": true, "GroupMembers.DeleteAt": 0, "Teams.DeleteAt": 0, - }). - Where("(GroupMembers.CreateAt >= ? OR GroupTeams.UpdateAt >= ?)", since, since) + }) + if !includeRemovedMembers { + builder = builder. + JoinClause("LEFT OUTER JOIN TeamMembers ON TeamMembers.TeamId = GroupTeams.TeamId AND TeamMembers.UserId = GroupMembers.UserId"). + Where(sq.Eq{"TeamMembers.UserId": nil}). + Where(sq.Or{ + sq.GtOrEq{"GroupMembers.CreateAt": since}, + sq.GtOrEq{"GroupTeams.UpdateAt": since}, + }) + } if teamID != nil { builder = builder.Where(sq.Eq{"Teams.Id": *teamID}) } @@ -742,24 +748,32 @@ func (s *SqlGroupStore) TeamMembersToAdd(since int64, teamID *string) ([]*model. return teamMembers, nil } -func (s *SqlGroupStore) ChannelMembersToAdd(since int64, channelID *string) ([]*model.UserChannelIDPair, error) { +func (s *SqlGroupStore) ChannelMembersToAdd(since int64, channelID *string, includeRemovedMembers bool) ([]*model.UserChannelIDPair, error) { builder := s.getQueryBuilder().Select("GroupMembers.UserId", "GroupChannels.ChannelId"). From("GroupMembers"). Join("GroupChannels ON GroupChannels.GroupId = GroupMembers.GroupId"). Join("UserGroups ON UserGroups.Id = GroupMembers.GroupId"). Join("Channels ON Channels.Id = GroupChannels.ChannelId"). - JoinClause("LEFT OUTER JOIN ChannelMemberHistory ON ChannelMemberHistory.ChannelId = GroupChannels.ChannelId AND ChannelMemberHistory.UserId = GroupMembers.UserId"). Where(sq.Eq{ - "ChannelMemberHistory.UserId": nil, - "ChannelMemberHistory.LeaveTime": nil, - "UserGroups.DeleteAt": 0, - "GroupChannels.DeleteAt": 0, - "GroupChannels.AutoAdd": true, - "GroupMembers.DeleteAt": 0, - "Channels.DeleteAt": 0, - }). - Where("(GroupMembers.CreateAt >= ? OR GroupChannels.UpdateAt >= ?)", since, since) + "UserGroups.DeleteAt": 0, + "GroupChannels.DeleteAt": 0, + "GroupChannels.AutoAdd": true, + "GroupMembers.DeleteAt": 0, + "Channels.DeleteAt": 0, + }) + if !includeRemovedMembers { + builder = builder. + JoinClause("LEFT OUTER JOIN ChannelMemberHistory ON ChannelMemberHistory.ChannelId = GroupChannels.ChannelId AND ChannelMemberHistory.UserId = GroupMembers.UserId"). + Where(sq.Eq{ + "ChannelMemberHistory.UserId": nil, + "ChannelMemberHistory.LeaveTime": nil, + }). + Where(sq.Or{ + sq.GtOrEq{"GroupMembers.CreateAt": since}, + sq.GtOrEq{"GroupChannels.UpdateAt": since}, + }) + } if channelID != nil { builder = builder.Where(sq.Eq{"Channels.Id": *channelID}) } diff --git a/store/store.go b/store/store.go index a974de95d8..eabbf4679e 100644 --- a/store/store.go +++ b/store/store.go @@ -768,13 +768,17 @@ type GroupStore interface { // based on the groups configurations. The returned list can be optionally scoped to a single given team. // // Typically since will be the last successful group sync time. - TeamMembersToAdd(since int64, teamID *string) ([]*model.UserTeamIDPair, error) + // If includeRemovedMembers is true, then team members who left or were removed from the team will + // be included; otherwise, they will be excluded. + TeamMembersToAdd(since int64, teamID *string, includeRemovedMembers bool) ([]*model.UserTeamIDPair, error) // ChannelMembersToAdd returns a slice of UserChannelIDPair that need newly created memberships // based on the groups configurations. The returned list can be optionally scoped to a single given channel. // // Typically since will be the last successful group sync time. - ChannelMembersToAdd(since int64, channelID *string) ([]*model.UserChannelIDPair, error) + // If includeRemovedMembers is true, then channel members who left or were removed from the channel will + // be included; otherwise, they will be excluded. + ChannelMembersToAdd(since int64, channelID *string, includeRemovedMembers bool) ([]*model.UserChannelIDPair, error) // TeamMembersToRemove returns all team members that should be removed based on group constraints. TeamMembersToRemove(teamID *string) ([]*model.TeamMember, error) diff --git a/store/storetest/group_store.go b/store/storetest/group_store.go index d4c759e2f2..8b269026b0 100644 --- a/store/storetest/group_store.go +++ b/store/storetest/group_store.go @@ -1367,14 +1367,14 @@ func testTeamMembersToAdd(t *testing.T, ss store.Store) { require.NoError(t, err) // Time before syncable was created - teamMembers, err := ss.Group().TeamMembersToAdd(syncable.CreateAt-1, nil) + teamMembers, err := ss.Group().TeamMembersToAdd(syncable.CreateAt-1, nil, false) require.NoError(t, err) require.Len(t, teamMembers, 1) require.Equal(t, user.Id, teamMembers[0].UserID) require.Equal(t, team.Id, teamMembers[0].TeamID) // Time after syncable was created - teamMembers, err = ss.Group().TeamMembersToAdd(syncable.CreateAt+1, nil) + teamMembers, err = ss.Group().TeamMembersToAdd(syncable.CreateAt+1, nil, false) require.NoError(t, err) require.Empty(t, teamMembers) @@ -1383,7 +1383,7 @@ func testTeamMembersToAdd(t *testing.T, ss store.Store) { require.NoError(t, err) _, err = ss.Group().UpsertMember(group.Id, user.Id) require.NoError(t, err) - teamMembers, err = ss.Group().TeamMembersToAdd(syncable.CreateAt+1, nil) + teamMembers, err = ss.Group().TeamMembersToAdd(syncable.CreateAt+1, nil, false) require.NoError(t, err) require.Len(t, teamMembers, 1) @@ -1393,14 +1393,14 @@ func testTeamMembersToAdd(t *testing.T, ss store.Store) { require.NoError(t, err) // Time before syncable was updated - teamMembers, err = ss.Group().TeamMembersToAdd(syncable.UpdateAt-1, nil) + teamMembers, err = ss.Group().TeamMembersToAdd(syncable.UpdateAt-1, nil, false) require.NoError(t, err) require.Len(t, teamMembers, 1) require.Equal(t, user.Id, teamMembers[0].UserID) require.Equal(t, team.Id, teamMembers[0].TeamID) // Time after syncable was updated - teamMembers, err = ss.Group().TeamMembersToAdd(syncable.UpdateAt+1, nil) + teamMembers, err = ss.Group().TeamMembersToAdd(syncable.UpdateAt+1, nil, false) require.NoError(t, err) require.Empty(t, teamMembers) @@ -1408,21 +1408,21 @@ func testTeamMembersToAdd(t *testing.T, ss store.Store) { syncable.AutoAdd = false _, err = ss.Group().UpdateGroupSyncable(syncable) require.NoError(t, err) - teamMembers, err = ss.Group().TeamMembersToAdd(0, nil) + teamMembers, err = ss.Group().TeamMembersToAdd(0, nil, false) require.NoError(t, err) require.Empty(t, teamMembers) // reset state of syncable and verify _, err = ss.Group().UpdateGroupSyncable(&pristineSyncable) require.NoError(t, err) - teamMembers, err = ss.Group().TeamMembersToAdd(0, nil) + teamMembers, err = ss.Group().TeamMembersToAdd(0, nil, false) require.NoError(t, err) require.Len(t, teamMembers, 1) // No result if Group deleted _, err = ss.Group().Delete(group.Id) require.NoError(t, err) - teamMembers, err = ss.Group().TeamMembersToAdd(0, nil) + teamMembers, err = ss.Group().TeamMembersToAdd(0, nil, false) require.NoError(t, err) require.Empty(t, teamMembers) @@ -1430,7 +1430,7 @@ func testTeamMembersToAdd(t *testing.T, ss store.Store) { group.DeleteAt = 0 _, err = ss.Group().Update(group) require.NoError(t, err) - teamMembers, err = ss.Group().TeamMembersToAdd(0, nil) + teamMembers, err = ss.Group().TeamMembersToAdd(0, nil, false) require.NoError(t, err) require.Len(t, teamMembers, 1) @@ -1438,7 +1438,7 @@ func testTeamMembersToAdd(t *testing.T, ss store.Store) { team.DeleteAt = model.GetMillis() team, nErr = ss.Team().Update(team) require.NoError(t, nErr) - teamMembers, err = ss.Group().TeamMembersToAdd(0, nil) + teamMembers, err = ss.Group().TeamMembersToAdd(0, nil, false) require.NoError(t, err) require.Empty(t, teamMembers) @@ -1446,35 +1446,35 @@ func testTeamMembersToAdd(t *testing.T, ss store.Store) { team.DeleteAt = 0 team, nErr = ss.Team().Update(team) require.NoError(t, nErr) - teamMembers, err = ss.Group().TeamMembersToAdd(0, nil) + teamMembers, err = ss.Group().TeamMembersToAdd(0, nil, false) require.NoError(t, err) require.Len(t, teamMembers, 1) // No result if GroupTeam deleted _, err = ss.Group().DeleteGroupSyncable(group.Id, team.Id, model.GroupSyncableTypeTeam) require.NoError(t, err) - teamMembers, err = ss.Group().TeamMembersToAdd(0, nil) + teamMembers, err = ss.Group().TeamMembersToAdd(0, nil, false) require.NoError(t, err) require.Empty(t, teamMembers) // reset GroupTeam and verify _, err = ss.Group().UpdateGroupSyncable(&pristineSyncable) require.NoError(t, err) - teamMembers, err = ss.Group().TeamMembersToAdd(0, nil) + teamMembers, err = ss.Group().TeamMembersToAdd(0, nil, false) require.NoError(t, err) require.Len(t, teamMembers, 1) // No result if GroupMember deleted _, err = ss.Group().DeleteMember(group.Id, user.Id) require.NoError(t, err) - teamMembers, err = ss.Group().TeamMembersToAdd(0, nil) + teamMembers, err = ss.Group().TeamMembersToAdd(0, nil, false) require.NoError(t, err) require.Empty(t, teamMembers) // restore group member and verify _, err = ss.Group().UpsertMember(group.Id, user.Id) require.NoError(t, err) - teamMembers, err = ss.Group().TeamMembersToAdd(0, nil) + teamMembers, err = ss.Group().TeamMembersToAdd(0, nil, false) require.NoError(t, err) require.Len(t, teamMembers, 1) @@ -1484,9 +1484,25 @@ func testTeamMembersToAdd(t *testing.T, ss store.Store) { UserId: user.Id, }, 999) require.NoError(t, nErr) - teamMembers, err = ss.Group().TeamMembersToAdd(0, nil) + teamMembers, err = ss.Group().TeamMembersToAdd(0, nil, false) require.NoError(t, err) require.Empty(t, teamMembers) + + // Leaving Team should still not return result + _, nErr = ss.Team().UpdateMember(&model.TeamMember{ + TeamId: team.Id, + UserId: user.Id, + DeleteAt: model.GetMillis(), + }) + require.NoError(t, nErr) + teamMembers, err = ss.Group().TeamMembersToAdd(0, nil, false) + require.NoError(t, err) + require.Empty(t, teamMembers) + + // If includeRemovedMembers is set to true, removed members should be added back in + teamMembers, err = ss.Group().TeamMembersToAdd(0, nil, true) + require.NoError(t, err) + require.Len(t, teamMembers, 1) } func testTeamMembersToAddSingleTeam(t *testing.T, ss store.Store) { @@ -1566,15 +1582,15 @@ func testTeamMembersToAddSingleTeam(t *testing.T, ss store.Store) { _, err = ss.Group().CreateGroupSyncable(model.NewGroupTeam(group2.Id, team2.Id, true)) require.NoError(t, err) - teamMembers, err := ss.Group().TeamMembersToAdd(0, nil) + teamMembers, err := ss.Group().TeamMembersToAdd(0, nil, false) require.NoError(t, err) require.Len(t, teamMembers, 3) - teamMembers, err = ss.Group().TeamMembersToAdd(0, &team1.Id) + teamMembers, err = ss.Group().TeamMembersToAdd(0, &team1.Id, false) require.NoError(t, err) require.Len(t, teamMembers, 2) - teamMembers, err = ss.Group().TeamMembersToAdd(0, &team2.Id) + teamMembers, err = ss.Group().TeamMembersToAdd(0, &team2.Id, false) require.NoError(t, err) require.Len(t, teamMembers, 1) } @@ -1616,14 +1632,14 @@ func testChannelMembersToAdd(t *testing.T, ss store.Store) { require.NoError(t, err) // Time before syncable was created - channelMembers, err := ss.Group().ChannelMembersToAdd(syncable.CreateAt-1, nil) + channelMembers, err := ss.Group().ChannelMembersToAdd(syncable.CreateAt-1, nil, false) require.NoError(t, err) require.Len(t, channelMembers, 1) require.Equal(t, user.Id, channelMembers[0].UserID) require.Equal(t, channel.Id, channelMembers[0].ChannelID) // Time after syncable was created - channelMembers, err = ss.Group().ChannelMembersToAdd(syncable.CreateAt+1, nil) + channelMembers, err = ss.Group().ChannelMembersToAdd(syncable.CreateAt+1, nil, false) require.NoError(t, err) require.Empty(t, channelMembers) @@ -1632,7 +1648,7 @@ func testChannelMembersToAdd(t *testing.T, ss store.Store) { require.NoError(t, err) _, err = ss.Group().UpsertMember(group.Id, user.Id) require.NoError(t, err) - channelMembers, err = ss.Group().ChannelMembersToAdd(syncable.CreateAt+1, nil) + channelMembers, err = ss.Group().ChannelMembersToAdd(syncable.CreateAt+1, nil, false) require.NoError(t, err) require.Len(t, channelMembers, 1) @@ -1642,14 +1658,14 @@ func testChannelMembersToAdd(t *testing.T, ss store.Store) { require.NoError(t, err) // Time before syncable was updated - channelMembers, err = ss.Group().ChannelMembersToAdd(syncable.UpdateAt-1, nil) + channelMembers, err = ss.Group().ChannelMembersToAdd(syncable.UpdateAt-1, nil, false) require.NoError(t, err) require.Len(t, channelMembers, 1) require.Equal(t, user.Id, channelMembers[0].UserID) require.Equal(t, channel.Id, channelMembers[0].ChannelID) // Time after syncable was updated - channelMembers, err = ss.Group().ChannelMembersToAdd(syncable.UpdateAt+1, nil) + channelMembers, err = ss.Group().ChannelMembersToAdd(syncable.UpdateAt+1, nil, false) require.NoError(t, err) require.Empty(t, channelMembers) @@ -1657,21 +1673,21 @@ func testChannelMembersToAdd(t *testing.T, ss store.Store) { syncable.AutoAdd = false _, err = ss.Group().UpdateGroupSyncable(syncable) require.NoError(t, err) - channelMembers, err = ss.Group().ChannelMembersToAdd(0, nil) + channelMembers, err = ss.Group().ChannelMembersToAdd(0, nil, false) require.NoError(t, err) require.Empty(t, channelMembers) // reset state of syncable and verify _, err = ss.Group().UpdateGroupSyncable(&pristineSyncable) require.NoError(t, err) - channelMembers, err = ss.Group().ChannelMembersToAdd(0, nil) + channelMembers, err = ss.Group().ChannelMembersToAdd(0, nil, false) require.NoError(t, err) require.Len(t, channelMembers, 1) // No result if Group deleted _, err = ss.Group().Delete(group.Id) require.NoError(t, err) - channelMembers, err = ss.Group().ChannelMembersToAdd(0, nil) + channelMembers, err = ss.Group().ChannelMembersToAdd(0, nil, false) require.NoError(t, err) require.Empty(t, channelMembers) @@ -1679,14 +1695,14 @@ func testChannelMembersToAdd(t *testing.T, ss store.Store) { group.DeleteAt = 0 _, err = ss.Group().Update(group) require.NoError(t, err) - channelMembers, err = ss.Group().ChannelMembersToAdd(0, nil) + channelMembers, err = ss.Group().ChannelMembersToAdd(0, nil, false) require.NoError(t, err) require.Len(t, channelMembers, 1) // No result if Channel deleted nErr = ss.Channel().Delete(channel.Id, model.GetMillis()) require.NoError(t, nErr) - channelMembers, err = ss.Group().ChannelMembersToAdd(0, nil) + channelMembers, err = ss.Group().ChannelMembersToAdd(0, nil, false) require.NoError(t, err) require.Empty(t, channelMembers) @@ -1694,56 +1710,63 @@ func testChannelMembersToAdd(t *testing.T, ss store.Store) { channel.DeleteAt = 0 _, nErr = ss.Channel().Update(channel) require.NoError(t, nErr) - channelMembers, err = ss.Group().ChannelMembersToAdd(0, nil) + channelMembers, err = ss.Group().ChannelMembersToAdd(0, nil, false) require.NoError(t, err) require.Len(t, channelMembers, 1) // No result if GroupChannel deleted _, err = ss.Group().DeleteGroupSyncable(group.Id, channel.Id, model.GroupSyncableTypeChannel) require.NoError(t, err) - channelMembers, err = ss.Group().ChannelMembersToAdd(0, nil) + channelMembers, err = ss.Group().ChannelMembersToAdd(0, nil, false) require.NoError(t, err) require.Empty(t, channelMembers) // reset GroupChannel and verify _, err = ss.Group().UpdateGroupSyncable(&pristineSyncable) require.NoError(t, err) - channelMembers, err = ss.Group().ChannelMembersToAdd(0, nil) + channelMembers, err = ss.Group().ChannelMembersToAdd(0, nil, false) require.NoError(t, err) require.Len(t, channelMembers, 1) // No result if GroupMember deleted _, err = ss.Group().DeleteMember(group.Id, user.Id) require.NoError(t, err) - channelMembers, err = ss.Group().ChannelMembersToAdd(0, nil) + channelMembers, err = ss.Group().ChannelMembersToAdd(0, nil, false) require.NoError(t, err) require.Empty(t, channelMembers) // restore group member and verify _, err = ss.Group().UpsertMember(group.Id, user.Id) require.NoError(t, err) - channelMembers, err = ss.Group().ChannelMembersToAdd(0, nil) + channelMembers, err = ss.Group().ChannelMembersToAdd(0, nil, false) require.NoError(t, err) require.Len(t, channelMembers, 1) // Adding Channel (ChannelMemberHistory) should stop returning result nErr = ss.ChannelMemberHistory().LogJoinEvent(user.Id, channel.Id, model.GetMillis()) require.NoError(t, nErr) - channelMembers, err = ss.Group().ChannelMembersToAdd(0, nil) + channelMembers, err = ss.Group().ChannelMembersToAdd(0, nil, false) require.NoError(t, err) require.Empty(t, channelMembers) // Leaving Channel (ChannelMemberHistory) should still not return result nErr = ss.ChannelMemberHistory().LogLeaveEvent(user.Id, channel.Id, model.GetMillis()) require.NoError(t, nErr) - channelMembers, err = ss.Group().ChannelMembersToAdd(0, nil) + channelMembers, err = ss.Group().ChannelMembersToAdd(0, nil, false) require.NoError(t, err) require.Empty(t, channelMembers) // Purging ChannelMemberHistory re-returns the result _, nErr = ss.ChannelMemberHistory().PermanentDeleteBatch(model.GetMillis()+1, 100) require.NoError(t, nErr) - channelMembers, err = ss.Group().ChannelMembersToAdd(0, nil) + channelMembers, err = ss.Group().ChannelMembersToAdd(0, nil, false) + require.NoError(t, err) + require.Len(t, channelMembers, 1) + + // If includeRemovedMembers is set to true, removed members should be added back in + nErr = ss.ChannelMemberHistory().LogLeaveEvent(user.Id, channel.Id, model.GetMillis()) + require.NoError(t, nErr) + channelMembers, err = ss.Group().ChannelMembersToAdd(0, nil, true) require.NoError(t, err) require.Len(t, channelMembers, 1) } @@ -1815,15 +1838,15 @@ func testChannelMembersToAddSingleChannel(t *testing.T, ss store.Store) { _, err = ss.Group().CreateGroupSyncable(model.NewGroupChannel(group2.Id, channel2.Id, true)) require.NoError(t, err) - channelMembers, err := ss.Group().ChannelMembersToAdd(0, nil) + channelMembers, err := ss.Group().ChannelMembersToAdd(0, nil, false) require.NoError(t, err) require.GreaterOrEqual(t, len(channelMembers), 3) - channelMembers, err = ss.Group().ChannelMembersToAdd(0, &channel1.Id) + channelMembers, err = ss.Group().ChannelMembersToAdd(0, &channel1.Id, false) require.NoError(t, err) require.Len(t, channelMembers, 2) - channelMembers, err = ss.Group().ChannelMembersToAdd(0, &channel2.Id) + channelMembers, err = ss.Group().ChannelMembersToAdd(0, &channel2.Id, false) require.NoError(t, err) require.Len(t, channelMembers, 1) } diff --git a/store/storetest/mocks/GroupStore.go b/store/storetest/mocks/GroupStore.go index 8c17caa44a..a50b558c15 100644 --- a/store/storetest/mocks/GroupStore.go +++ b/store/storetest/mocks/GroupStore.go @@ -60,13 +60,13 @@ func (_m *GroupStore) ChannelMembersMinusGroupMembers(channelID string, groupIDs return r0, r1 } -// ChannelMembersToAdd provides a mock function with given fields: since, channelID -func (_m *GroupStore) ChannelMembersToAdd(since int64, channelID *string) ([]*model.UserChannelIDPair, error) { - ret := _m.Called(since, channelID) +// ChannelMembersToAdd provides a mock function with given fields: since, channelID, includeRemovedMembers +func (_m *GroupStore) ChannelMembersToAdd(since int64, channelID *string, includeRemovedMembers bool) ([]*model.UserChannelIDPair, error) { + ret := _m.Called(since, channelID, includeRemovedMembers) var r0 []*model.UserChannelIDPair - if rf, ok := ret.Get(0).(func(int64, *string) []*model.UserChannelIDPair); ok { - r0 = rf(since, channelID) + if rf, ok := ret.Get(0).(func(int64, *string, bool) []*model.UserChannelIDPair); ok { + r0 = rf(since, channelID, includeRemovedMembers) } else { if ret.Get(0) != nil { r0 = ret.Get(0).([]*model.UserChannelIDPair) @@ -74,8 +74,8 @@ func (_m *GroupStore) ChannelMembersToAdd(since int64, channelID *string) ([]*mo } var r1 error - if rf, ok := ret.Get(1).(func(int64, *string) error); ok { - r1 = rf(since, channelID) + if rf, ok := ret.Get(1).(func(int64, *string, bool) error); ok { + r1 = rf(since, channelID, includeRemovedMembers) } else { r1 = ret.Error(1) } @@ -880,13 +880,13 @@ func (_m *GroupStore) TeamMembersMinusGroupMembers(teamID string, groupIDs []str return r0, r1 } -// TeamMembersToAdd provides a mock function with given fields: since, teamID -func (_m *GroupStore) TeamMembersToAdd(since int64, teamID *string) ([]*model.UserTeamIDPair, error) { - ret := _m.Called(since, teamID) +// TeamMembersToAdd provides a mock function with given fields: since, teamID, includeRemovedMembers +func (_m *GroupStore) TeamMembersToAdd(since int64, teamID *string, includeRemovedMembers bool) ([]*model.UserTeamIDPair, error) { + ret := _m.Called(since, teamID, includeRemovedMembers) var r0 []*model.UserTeamIDPair - if rf, ok := ret.Get(0).(func(int64, *string) []*model.UserTeamIDPair); ok { - r0 = rf(since, teamID) + if rf, ok := ret.Get(0).(func(int64, *string, bool) []*model.UserTeamIDPair); ok { + r0 = rf(since, teamID, includeRemovedMembers) } else { if ret.Get(0) != nil { r0 = ret.Get(0).([]*model.UserTeamIDPair) @@ -894,8 +894,8 @@ func (_m *GroupStore) TeamMembersToAdd(since int64, teamID *string) ([]*model.Us } var r1 error - if rf, ok := ret.Get(1).(func(int64, *string) error); ok { - r1 = rf(since, teamID) + if rf, ok := ret.Get(1).(func(int64, *string, bool) error); ok { + r1 = rf(since, teamID, includeRemovedMembers) } else { r1 = ret.Error(1) } diff --git a/store/timerlayer/timerlayer.go b/store/timerlayer/timerlayer.go index 41cb39caff..f186655f49 100644 --- a/store/timerlayer/timerlayer.go +++ b/store/timerlayer/timerlayer.go @@ -3083,10 +3083,10 @@ func (s *TimerLayerGroupStore) ChannelMembersMinusGroupMembers(channelID string, return result, err } -func (s *TimerLayerGroupStore) ChannelMembersToAdd(since int64, channelID *string) ([]*model.UserChannelIDPair, error) { +func (s *TimerLayerGroupStore) ChannelMembersToAdd(since int64, channelID *string, includeRemovedMembers bool) ([]*model.UserChannelIDPair, error) { start := timemodule.Now() - result, err := s.GroupStore.ChannelMembersToAdd(since, channelID) + result, err := s.GroupStore.ChannelMembersToAdd(since, channelID, includeRemovedMembers) elapsed := float64(timemodule.Since(start)) / float64(timemodule.Second) if s.Root.Metrics != nil { @@ -3675,10 +3675,10 @@ func (s *TimerLayerGroupStore) TeamMembersMinusGroupMembers(teamID string, group return result, err } -func (s *TimerLayerGroupStore) TeamMembersToAdd(since int64, teamID *string) ([]*model.UserTeamIDPair, error) { +func (s *TimerLayerGroupStore) TeamMembersToAdd(since int64, teamID *string, includeRemovedMembers bool) ([]*model.UserTeamIDPair, error) { start := timemodule.Now() - result, err := s.GroupStore.TeamMembersToAdd(since, teamID) + result, err := s.GroupStore.TeamMembersToAdd(since, teamID, includeRemovedMembers) elapsed := float64(timemodule.Since(start)) / float64(timemodule.Second) if s.Root.Metrics != nil {