From c2d08b754050c26086d742025d718e28de730278 Mon Sep 17 00:00:00 2001 From: Ben Schumacher Date: Tue, 20 May 2025 11:15:25 +0200 Subject: [PATCH] [MM-63772] Add LDAP setting to re-add removed members (#30787) --- server/channels/api4/ldap.go | 15 ++++---- server/channels/api4/ldap_test.go | 16 ++++---- server/channels/app/group.go | 12 +++--- server/channels/app/ldap.go | 6 +-- server/channels/app/syncables.go | 12 +++--- .../channels/store/retrylayer/retrylayer.go | 8 ++-- server/channels/store/sqlstore/group_store.go | 8 ++-- server/channels/store/store.go | 8 ++-- .../channels/store/storetest/group_store.go | 4 +- .../store/storetest/mocks/GroupStore.go | 24 ++++++------ .../channels/store/timerlayer/timerlayer.go | 8 ++-- server/cmd/mmctl/client/client.go | 2 +- server/cmd/mmctl/commands/ldap.go | 37 +++++++++++++------ server/cmd/mmctl/commands/ldap_test.go | 18 +++++---- server/cmd/mmctl/docs/mmctl_ldap_sync.rst | 3 +- server/cmd/mmctl/mocks/client_mock.go | 2 +- server/einterfaces/ldap.go | 2 +- server/einterfaces/mocks/LdapInterface.go | 18 ++++----- server/public/model/client4.go | 19 +++++++--- server/public/model/config.go | 7 +++- .../admin_console/admin_definition.tsx | 13 +++++++ webapp/channels/src/i18n/en.json | 2 + 22 files changed, 143 insertions(+), 101 deletions(-) diff --git a/server/channels/api4/ldap.go b/server/channels/api4/ldap.go index 8d2d03b860..e9865c7c2b 100644 --- a/server/channels/api4/ldap.go +++ b/server/channels/api4/ldap.go @@ -49,10 +49,14 @@ func syncLdap(c *Context, w http.ResponseWriter, r *http.Request) { return } - type LdapSyncOptions struct { - IncludeRemovedMembers bool `json:"include_removed_members"` + if !c.App.SessionHasPermissionTo(*c.AppContext.Session(), model.PermissionCreateLdapSyncJob) { + c.SetPermissionError(model.PermissionCreateLdapSyncJob) + return + } + + var opts struct { + IncludeRemovedMembers *bool `json:"include_removed_members"` } - var opts LdapSyncOptions err := json.NewDecoder(r.Body).Decode(&opts) if err != nil { c.Logger.LogM(mlog.MlvlLDAPInfo, "Error decoding LDAP sync options", mlog.Err(err)) @@ -61,11 +65,6 @@ func syncLdap(c *Context, w http.ResponseWriter, r *http.Request) { auditRec := c.MakeAuditRecord("syncLdap", audit.Fail) defer c.LogAuditRec(auditRec) - if !c.App.SessionHasPermissionTo(*c.AppContext.Session(), model.PermissionCreateLdapSyncJob) { - c.SetPermissionError(model.PermissionCreateLdapSyncJob) - return - } - c.App.SyncLdap(c.AppContext, opts.IncludeRemovedMembers) auditRec.Success() diff --git a/server/channels/api4/ldap_test.go b/server/channels/api4/ldap_test.go index 3d26038205..415082ffb9 100644 --- a/server/channels/api4/ldap_test.go +++ b/server/channels/api4/ldap_test.go @@ -147,29 +147,29 @@ func TestSyncLdap(t *testing.T) { "StartSynchronizeJob", mock.AnythingOfType("*request.Context"), mock.AnythingOfType("bool"), - mock.AnythingOfType("bool"), + mock.AnythingOfType("*bool"), ).Return(nil, nil) ready := make(chan bool) - includeRemovedMembers := false + reAddRemovedMembers := false mockCall.RunFn = func(args mock.Arguments) { - includeRemovedMembers = args[2].(bool) + reAddRemovedMembers = *args[2].(*bool) ready <- true } th.App.Channels().Ldap = ldapMock th.TestForSystemAdminAndLocal(t, func(t *testing.T, client *model.Client4) { - _, err := client.SyncLdap(context.Background(), false) + _, err := client.SyncLdap(context.Background(), model.NewPointer(false)) <-ready require.NoError(t, err) - require.False(t, includeRemovedMembers) + require.False(t, reAddRemovedMembers) - _, err = client.SyncLdap(context.Background(), true) + _, err = client.SyncLdap(context.Background(), model.NewPointer(true)) <-ready require.NoError(t, err) - require.True(t, includeRemovedMembers) + require.True(t, reAddRemovedMembers) }) - resp, err := th.Client.SyncLdap(context.Background(), false) + resp, err := th.Client.SyncLdap(context.Background(), model.NewPointer(false)) require.Error(t, err) CheckForbiddenStatus(t, resp) } diff --git a/server/channels/app/group.go b/server/channels/app/group.go index 075851f747..35d7ae9b7a 100644 --- a/server/channels/app/group.go +++ b/server/channels/app/group.go @@ -563,10 +563,10 @@ func (a *App) DeleteGroupSyncable(groupID string, syncableID string, syncableTyp // 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 +// If reAddRemovedMembers 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) +func (a *App) TeamMembersToAdd(since int64, teamID *string, reAddRemovedMembers bool) ([]*model.UserTeamIDPair, *model.AppError) { + userTeams, err := a.Srv().Store().Group().TeamMembersToAdd(since, teamID, reAddRemovedMembers) if err != nil { return nil, model.NewAppError("TeamMembersToAdd", "app.select_error", nil, "", http.StatusInternalServerError).Wrap(err) } @@ -578,10 +578,10 @@ func (a *App) TeamMembersToAdd(since int64, teamID *string, includeRemovedMember // 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 +// If reAddRemovedMembers 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) +func (a *App) ChannelMembersToAdd(since int64, channelID *string, reAddRemovedMembers bool) ([]*model.UserChannelIDPair, *model.AppError) { + userChannels, err := a.Srv().Store().Group().ChannelMembersToAdd(since, channelID, reAddRemovedMembers) if err != nil { return nil, model.NewAppError("ChannelMembersToAdd", "app.select_error", nil, "", http.StatusInternalServerError).Wrap(err) } diff --git a/server/channels/app/ldap.go b/server/channels/app/ldap.go index e2a26ed104..644a9cf853 100644 --- a/server/channels/app/ldap.go +++ b/server/channels/app/ldap.go @@ -15,9 +15,9 @@ import ( ) // SyncLdap starts an LDAP sync job. -// If includeRemovedMembers is true, then members who left or were removed from a team/channel will +// If reAddRemovedMembers 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(c request.CTX, includeRemovedMembers bool) { +func (a *App) SyncLdap(c request.CTX, reAddRemovedMembers *bool) { a.Srv().Go(func() { if license := a.Srv().License(); license != nil && *license.Features.LDAP { if !*a.Config().LdapSettings.EnableSync { @@ -30,7 +30,7 @@ func (a *App) SyncLdap(c request.CTX, includeRemovedMembers bool) { c.Logger().Error("Not executing ldap sync because ldap is not available") return } - if _, appErr := ldapI.StartSynchronizeJob(c, false, includeRemovedMembers); appErr != nil { + if _, appErr := ldapI.StartSynchronizeJob(c, false, reAddRemovedMembers); appErr != nil { c.Logger().Error("Failed to start LDAP sync job") } } diff --git a/server/channels/app/syncables.go b/server/channels/app/syncables.go index bbb37aafa8..e1ba027d48 100644 --- a/server/channels/app/syncables.go +++ b/server/channels/app/syncables.go @@ -17,7 +17,7 @@ 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. -// If includeRemovedMembers is true, then channel members who left or were removed from the channel will +// If params.ReAddRemovedMembers 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(rctx request.CTX, params model.CreateDefaultMembershipParams) error { channelMembers, appErr := a.ChannelMembersToAdd(params.Since, params.ScopedChannelID, params.ReAddRemovedMembers) @@ -86,7 +86,7 @@ func (a *App) createDefaultChannelMemberships(rctx request.CTX, params model.Cre // 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. -// If includeRemovedMembers is true, then team members who left or were removed from the team will +// If params.ReAddRemovedMembers 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(rctx request.CTX, params model.CreateDefaultMembershipParams) error { teamMembers, appErr := a.TeamMembersToAdd(params.Since, params.ScopedTeamID, params.ReAddRemovedMembers) @@ -123,7 +123,7 @@ func (a *App) createDefaultTeamMemberships(rctx request.CTX, params model.Create // 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. -// If includeRemovedMembers is true, then members who left or were removed from a team/channel will +// If params.AddRemovedMembers 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(rctx request.CTX, params model.CreateDefaultMembershipParams) error { err := a.createDefaultTeamMemberships(rctx, params) @@ -283,17 +283,17 @@ func (a *App) SyncRolesAndMembership(rctx request.CTX, syncableID string, syncab } var since int64 - includeRemovedMembers := true + reAddRemovedMembers := true if group.Source == model.GroupSourceLdap { lastJob, _ := a.Srv().Store().Job().GetNewestJobByStatusAndType(model.JobStatusSuccess, model.JobTypeLdapSync) if lastJob != nil { since = lastJob.StartAt } - includeRemovedMembers = false + reAddRemovedMembers = *a.Config().LdapSettings.ReAddRemovedMembers } - params := model.CreateDefaultMembershipParams{Since: since, ReAddRemovedMembers: includeRemovedMembers} + params := model.CreateDefaultMembershipParams{Since: since, ReAddRemovedMembers: reAddRemovedMembers} switch syncableType { case model.GroupSyncableTypeTeam: diff --git a/server/channels/store/retrylayer/retrylayer.go b/server/channels/store/retrylayer/retrylayer.go index 7241564c9a..40616eaf76 100644 --- a/server/channels/store/retrylayer/retrylayer.go +++ b/server/channels/store/retrylayer/retrylayer.go @@ -5186,11 +5186,11 @@ func (s *RetryLayerGroupStore) ChannelMembersMinusGroupMembers(channelID string, } -func (s *RetryLayerGroupStore) ChannelMembersToAdd(since int64, channelID *string, includeRemovedMembers bool) ([]*model.UserChannelIDPair, error) { +func (s *RetryLayerGroupStore) ChannelMembersToAdd(since int64, channelID *string, reAddRemovedMembers bool) ([]*model.UserChannelIDPair, error) { tries := 0 for { - result, err := s.GroupStore.ChannelMembersToAdd(since, channelID, includeRemovedMembers) + result, err := s.GroupStore.ChannelMembersToAdd(since, channelID, reAddRemovedMembers) if err == nil { return result, nil } @@ -6152,11 +6152,11 @@ func (s *RetryLayerGroupStore) TeamMembersMinusGroupMembers(teamID string, group } -func (s *RetryLayerGroupStore) TeamMembersToAdd(since int64, teamID *string, includeRemovedMembers bool) ([]*model.UserTeamIDPair, error) { +func (s *RetryLayerGroupStore) TeamMembersToAdd(since int64, teamID *string, reAddRemovedMembers bool) ([]*model.UserTeamIDPair, error) { tries := 0 for { - result, err := s.GroupStore.TeamMembersToAdd(since, teamID, includeRemovedMembers) + result, err := s.GroupStore.TeamMembersToAdd(since, teamID, reAddRemovedMembers) if err == nil { return result, nil } diff --git a/server/channels/store/sqlstore/group_store.go b/server/channels/store/sqlstore/group_store.go index 0f4d66b4a0..38a08f1e3f 100644 --- a/server/channels/store/sqlstore/group_store.go +++ b/server/channels/store/sqlstore/group_store.go @@ -963,7 +963,7 @@ func (s *SqlGroupStore) DeleteGroupSyncable(groupID string, syncableID string, s return groupSyncable, nil } -func (s *SqlGroupStore) TeamMembersToAdd(since int64, teamID *string, includeRemovedMembers bool) ([]*model.UserTeamIDPair, error) { +func (s *SqlGroupStore) TeamMembersToAdd(since int64, teamID *string, reAddRemovedMembers bool) ([]*model.UserTeamIDPair, error) { builder := s.getQueryBuilder().Select("GroupMembers.UserId UserID", "GroupTeams.TeamId TeamID"). From("GroupMembers"). Join("GroupTeams ON GroupTeams.GroupId = GroupMembers.GroupId"). @@ -977,7 +977,7 @@ func (s *SqlGroupStore) TeamMembersToAdd(since int64, teamID *string, includeRem "Teams.DeleteAt": 0, }) - if !includeRemovedMembers { + if !reAddRemovedMembers { builder = builder. JoinClause("LEFT OUTER JOIN TeamMembers ON TeamMembers.TeamId = GroupTeams.TeamId AND TeamMembers.UserId = GroupMembers.UserId"). Where(sq.Eq{"TeamMembers.UserId": nil}). @@ -999,7 +999,7 @@ func (s *SqlGroupStore) TeamMembersToAdd(since int64, teamID *string, includeRem return teamMembers, nil } -func (s *SqlGroupStore) ChannelMembersToAdd(since int64, channelID *string, includeRemovedMembers bool) ([]*model.UserChannelIDPair, error) { +func (s *SqlGroupStore) ChannelMembersToAdd(since int64, channelID *string, reAddRemovedMembers bool) ([]*model.UserChannelIDPair, error) { builder := s.getQueryBuilder().Select("GroupMembers.UserId UserID", "GroupChannels.ChannelId ChannelID"). From("GroupMembers"). Join("GroupChannels ON GroupChannels.GroupId = GroupMembers.GroupId"). @@ -1013,7 +1013,7 @@ func (s *SqlGroupStore) ChannelMembersToAdd(since int64, channelID *string, incl "Channels.DeleteAt": 0, }) - if !includeRemovedMembers { + if !reAddRemovedMembers { builder = builder. JoinClause("LEFT OUTER JOIN ChannelMemberHistory ON ChannelMemberHistory.ChannelId = GroupChannels.ChannelId AND ChannelMemberHistory.UserId = GroupMembers.UserId"). Where(sq.Eq{ diff --git a/server/channels/store/store.go b/server/channels/store/store.go index 775713ec3d..6c7b397664 100644 --- a/server/channels/store/store.go +++ b/server/channels/store/store.go @@ -915,17 +915,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. - // If includeRemovedMembers is true, then team members who left or were removed from the team will + // If reAddRemovedMembers 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) + TeamMembersToAdd(since int64, teamID *string, reAddRemovedMembers 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. - // If includeRemovedMembers is true, then channel members who left or were removed from the channel will + // If reAddRemovedMembers 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) + ChannelMembersToAdd(since int64, channelID *string, reAddRemovedMembers 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/server/channels/store/storetest/group_store.go b/server/channels/store/storetest/group_store.go index 0af5151215..0f3d7a3c20 100644 --- a/server/channels/store/storetest/group_store.go +++ b/server/channels/store/storetest/group_store.go @@ -2087,7 +2087,7 @@ func testTeamMembersToAdd(t *testing.T, rctx request.CTX, ss store.Store) { require.NoError(t, err) require.Empty(t, teamMembers) - // If includeRemovedMembers is set to true, removed members should be added back in + // If reAddRemovedMembers 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) @@ -2352,7 +2352,7 @@ func testChannelMembersToAdd(t *testing.T, rctx request.CTX, ss store.Store) { require.NoError(t, err) require.Len(t, channelMembers, 1) - // If includeRemovedMembers is set to true, removed members should be added back in + // If reAddRemovedMembers 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) diff --git a/server/channels/store/storetest/mocks/GroupStore.go b/server/channels/store/storetest/mocks/GroupStore.go index d7cf25ffe6..12df20207f 100644 --- a/server/channels/store/storetest/mocks/GroupStore.go +++ b/server/channels/store/storetest/mocks/GroupStore.go @@ -74,9 +74,9 @@ func (_m *GroupStore) ChannelMembersMinusGroupMembers(channelID string, groupIDs return r0, r1 } -// 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) +// ChannelMembersToAdd provides a mock function with given fields: since, channelID, reAddRemovedMembers +func (_m *GroupStore) ChannelMembersToAdd(since int64, channelID *string, reAddRemovedMembers bool) ([]*model.UserChannelIDPair, error) { + ret := _m.Called(since, channelID, reAddRemovedMembers) if len(ret) == 0 { panic("no return value specified for ChannelMembersToAdd") @@ -85,10 +85,10 @@ func (_m *GroupStore) ChannelMembersToAdd(since int64, channelID *string, includ var r0 []*model.UserChannelIDPair var r1 error if rf, ok := ret.Get(0).(func(int64, *string, bool) ([]*model.UserChannelIDPair, error)); ok { - return rf(since, channelID, includeRemovedMembers) + return rf(since, channelID, reAddRemovedMembers) } if rf, ok := ret.Get(0).(func(int64, *string, bool) []*model.UserChannelIDPair); ok { - r0 = rf(since, channelID, includeRemovedMembers) + r0 = rf(since, channelID, reAddRemovedMembers) } else { if ret.Get(0) != nil { r0 = ret.Get(0).([]*model.UserChannelIDPair) @@ -96,7 +96,7 @@ func (_m *GroupStore) ChannelMembersToAdd(since int64, channelID *string, includ } if rf, ok := ret.Get(1).(func(int64, *string, bool) error); ok { - r1 = rf(since, channelID, includeRemovedMembers) + r1 = rf(since, channelID, reAddRemovedMembers) } else { r1 = ret.Error(1) } @@ -1414,9 +1414,9 @@ func (_m *GroupStore) TeamMembersMinusGroupMembers(teamID string, groupIDs []str return r0, r1 } -// 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) +// TeamMembersToAdd provides a mock function with given fields: since, teamID, reAddRemovedMembers +func (_m *GroupStore) TeamMembersToAdd(since int64, teamID *string, reAddRemovedMembers bool) ([]*model.UserTeamIDPair, error) { + ret := _m.Called(since, teamID, reAddRemovedMembers) if len(ret) == 0 { panic("no return value specified for TeamMembersToAdd") @@ -1425,10 +1425,10 @@ func (_m *GroupStore) TeamMembersToAdd(since int64, teamID *string, includeRemov var r0 []*model.UserTeamIDPair var r1 error if rf, ok := ret.Get(0).(func(int64, *string, bool) ([]*model.UserTeamIDPair, error)); ok { - return rf(since, teamID, includeRemovedMembers) + return rf(since, teamID, reAddRemovedMembers) } if rf, ok := ret.Get(0).(func(int64, *string, bool) []*model.UserTeamIDPair); ok { - r0 = rf(since, teamID, includeRemovedMembers) + r0 = rf(since, teamID, reAddRemovedMembers) } else { if ret.Get(0) != nil { r0 = ret.Get(0).([]*model.UserTeamIDPair) @@ -1436,7 +1436,7 @@ func (_m *GroupStore) TeamMembersToAdd(since int64, teamID *string, includeRemov } if rf, ok := ret.Get(1).(func(int64, *string, bool) error); ok { - r1 = rf(since, teamID, includeRemovedMembers) + r1 = rf(since, teamID, reAddRemovedMembers) } else { r1 = ret.Error(1) } diff --git a/server/channels/store/timerlayer/timerlayer.go b/server/channels/store/timerlayer/timerlayer.go index 7367116ff4..ab59be6405 100644 --- a/server/channels/store/timerlayer/timerlayer.go +++ b/server/channels/store/timerlayer/timerlayer.go @@ -4199,10 +4199,10 @@ func (s *TimerLayerGroupStore) ChannelMembersMinusGroupMembers(channelID string, return result, err } -func (s *TimerLayerGroupStore) ChannelMembersToAdd(since int64, channelID *string, includeRemovedMembers bool) ([]*model.UserChannelIDPair, error) { +func (s *TimerLayerGroupStore) ChannelMembersToAdd(since int64, channelID *string, reAddRemovedMembers bool) ([]*model.UserChannelIDPair, error) { start := time.Now() - result, err := s.GroupStore.ChannelMembersToAdd(since, channelID, includeRemovedMembers) + result, err := s.GroupStore.ChannelMembersToAdd(since, channelID, reAddRemovedMembers) elapsed := float64(time.Since(start)) / float64(time.Second) if s.Root.Metrics != nil { @@ -4935,10 +4935,10 @@ func (s *TimerLayerGroupStore) TeamMembersMinusGroupMembers(teamID string, group return result, err } -func (s *TimerLayerGroupStore) TeamMembersToAdd(since int64, teamID *string, includeRemovedMembers bool) ([]*model.UserTeamIDPair, error) { +func (s *TimerLayerGroupStore) TeamMembersToAdd(since int64, teamID *string, reAddRemovedMembers bool) ([]*model.UserTeamIDPair, error) { start := time.Now() - result, err := s.GroupStore.TeamMembersToAdd(since, teamID, includeRemovedMembers) + result, err := s.GroupStore.TeamMembersToAdd(since, teamID, reAddRemovedMembers) elapsed := float64(time.Since(start)) / float64(time.Second) if s.Root.Metrics != nil { diff --git a/server/cmd/mmctl/client/client.go b/server/cmd/mmctl/client/client.go index 2b52b7b951..6f5d17fc56 100644 --- a/server/cmd/mmctl/client/client.go +++ b/server/cmd/mmctl/client/client.go @@ -97,7 +97,7 @@ type Client interface { PatchConfig(context.Context, *model.Config) (*model.Config, *model.Response, error) ReloadConfig(ctx context.Context) (*model.Response, error) MigrateConfig(ctx context.Context, from, to string) (*model.Response, error) - SyncLdap(ctx context.Context, includeRemovedMembers bool) (*model.Response, error) + SyncLdap(ctx context.Context, reAddRemovedMembers *bool) (*model.Response, error) MigrateIdLdap(ctx context.Context, toAttribute string) (*model.Response, error) GetUsers(ctx context.Context, page, perPage int, etag string) ([]*model.User, *model.Response, error) UpdateUserActive(ctx context.Context, userID string, activate bool) (*model.Response, error) diff --git a/server/cmd/mmctl/commands/ldap.go b/server/cmd/mmctl/commands/ldap.go index 69175a6c1a..a88217ba6e 100644 --- a/server/cmd/mmctl/commands/ldap.go +++ b/server/cmd/mmctl/commands/ldap.go @@ -20,12 +20,22 @@ var LdapCmd = &cobra.Command{ Short: "LDAP related utilities", } -var LdapSyncCmd = &cobra.Command{ - Use: "sync", - Short: "Synchronize now", - Long: "Synchronize all LDAP users and groups now.", - Example: " ldap sync", - RunE: withClient(ldapSyncCmdF), +func newLDAPSyncCmd() *cobra.Command { + cmd := &cobra.Command{ + Use: "sync", + Short: "Synchronize now", + Long: "Synchronize all LDAP users and groups now.", + Example: " ldap sync", + RunE: withClient(ldapSyncCmdF), + } + + cmd.Flags().Bool("include-removed-members", false, "Include members who left or were removed from a group-synced team/channel") + err := cmd.Flags().MarkDeprecated("include-removed-members", "This flag is deprecated and will be removed in a future version. Use LdapSettings.ReAddRemovedMembers instead.") + if err != nil { + panic(err) + } + + return cmd } var LdapIDMigrate = &cobra.Command{ @@ -68,7 +78,7 @@ var LdapJobShowCmd = &cobra.Command{ } func init() { - LdapSyncCmd.Flags().Bool("include-removed-members", false, "Include members who left or were removed from a group-synced team/channel") + ldapSyncCmd := newLDAPSyncCmd() LdapJobListCmd.Flags().Int("page", 0, "Page number to fetch for the list of import jobs") LdapJobListCmd.Flags().Int("per-page", 200, "Number of import jobs to be fetched") @@ -80,7 +90,7 @@ func init() { ) LdapCmd.AddCommand( - LdapSyncCmd, + ldapSyncCmd, LdapIDMigrate, LdapJobCmd, ) @@ -90,9 +100,14 @@ func init() { func ldapSyncCmdF(c client.Client, cmd *cobra.Command, args []string) error { printer.SetSingle(true) - includeRemovedMembers, _ := cmd.Flags().GetBool("include-removed-members") - - resp, err := c.SyncLdap(context.TODO(), includeRemovedMembers) + var resp *model.Response + var err error + if cmd.Flags().Changed("include-removed-members") { + reAddRemovedMembers, _ := cmd.Flags().GetBool("include-removed-members") + resp, err = c.SyncLdap(context.TODO(), &reAddRemovedMembers) + } else { + resp, err = c.SyncLdap(context.TODO(), nil) + } if err != nil { return err } diff --git a/server/cmd/mmctl/commands/ldap_test.go b/server/cmd/mmctl/commands/ldap_test.go index b44c99af37..5e3345f6f9 100644 --- a/server/cmd/mmctl/commands/ldap_test.go +++ b/server/cmd/mmctl/commands/ldap_test.go @@ -22,7 +22,7 @@ func (s *MmctlUnitTestSuite) TestLdapSyncCmd() { s.client. EXPECT(). - SyncLdap(context.TODO(), false). + SyncLdap(context.TODO(), nil). Return(&model.Response{StatusCode: http.StatusOK}, nil). Times(1) @@ -39,7 +39,7 @@ func (s *MmctlUnitTestSuite) TestLdapSyncCmd() { s.client. EXPECT(). - SyncLdap(context.TODO(), false). + SyncLdap(context.TODO(), nil). Return(&model.Response{StatusCode: http.StatusBadRequest}, nil). Times(1) @@ -56,7 +56,7 @@ func (s *MmctlUnitTestSuite) TestLdapSyncCmd() { s.client. EXPECT(). - SyncLdap(context.TODO(), false). + SyncLdap(context.TODO(), nil). Return(&model.Response{StatusCode: http.StatusBadRequest}, mockError). Times(1) @@ -67,18 +67,20 @@ func (s *MmctlUnitTestSuite) TestLdapSyncCmd() { s.Require().Len(printer.GetErrorLines(), 0) }) - s.Run("Sync with includeRemoveMembers", func() { + s.Run("Sync with deprecated includeRemoveMembers", func() { printer.Clean() - cmd := &cobra.Command{} - cmd.Flags().Bool("include-removed-members", true, "") + + cmd := newLDAPSyncCmd() + err := cmd.ParseFlags([]string{"--include-removed-members"}) + s.Require().Nil(err) s.client. EXPECT(). - SyncLdap(context.TODO(), true). + SyncLdap(context.TODO(), model.NewPointer(true)). Return(&model.Response{StatusCode: http.StatusOK}, nil). Times(1) - err := ldapSyncCmdF(s.client, cmd, []string{}) + err = ldapSyncCmdF(s.client, cmd, []string{}) s.Require().Nil(err) }) } diff --git a/server/cmd/mmctl/docs/mmctl_ldap_sync.rst b/server/cmd/mmctl/docs/mmctl_ldap_sync.rst index 1d9e6b598f..3cc5b4ebe1 100644 --- a/server/cmd/mmctl/docs/mmctl_ldap_sync.rst +++ b/server/cmd/mmctl/docs/mmctl_ldap_sync.rst @@ -27,8 +27,7 @@ Options :: - -h, --help help for sync - --include-removed-members Include members who left or were removed from a group-synced team/channel + -h, --help help for sync Options inherited from parent commands ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ diff --git a/server/cmd/mmctl/mocks/client_mock.go b/server/cmd/mmctl/mocks/client_mock.go index aabd3f95f9..adb61db957 100644 --- a/server/cmd/mmctl/mocks/client_mock.go +++ b/server/cmd/mmctl/mocks/client_mock.go @@ -2074,7 +2074,7 @@ func (mr *MockClientMockRecorder) SoftDeleteTeam(arg0, arg1 interface{}) *gomock } // SyncLdap mocks base method. -func (m *MockClient) SyncLdap(arg0 context.Context, arg1 bool) (*model.Response, error) { +func (m *MockClient) SyncLdap(arg0 context.Context, arg1 *bool) (*model.Response, error) { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "SyncLdap", arg0, arg1) ret0, _ := ret[0].(*model.Response) diff --git a/server/einterfaces/ldap.go b/server/einterfaces/ldap.go index c615a70eb6..893101725f 100644 --- a/server/einterfaces/ldap.go +++ b/server/einterfaces/ldap.go @@ -15,7 +15,7 @@ type LdapInterface interface { GetUserAttributes(rctx request.CTX, id string, attributes []string) (map[string]string, *model.AppError) CheckProviderAttributes(c request.CTX, LS *model.LdapSettings, ouser *model.User, patch *model.UserPatch) string SwitchToLdap(c request.CTX, userID, ldapID, ldapPassword string) *model.AppError - StartSynchronizeJob(c request.CTX, waitForJobToFinish bool, includeRemovedMembers bool) (*model.Job, *model.AppError) + StartSynchronizeJob(c request.CTX, waitForJobToFinish bool, reAddRemovedMembers *bool) (*model.Job, *model.AppError) GetAllLdapUsers(c request.CTX) ([]*model.User, *model.AppError) MigrateIDAttribute(c request.CTX, toAttribute string) error GetGroup(rctx request.CTX, groupUID string) (*model.Group, *model.AppError) diff --git a/server/einterfaces/mocks/LdapInterface.go b/server/einterfaces/mocks/LdapInterface.go index 0266ae5aca..7b63d40648 100644 --- a/server/einterfaces/mocks/LdapInterface.go +++ b/server/einterfaces/mocks/LdapInterface.go @@ -309,9 +309,9 @@ func (_m *LdapInterface) MigrateIDAttribute(c request.CTX, toAttribute string) e return r0 } -// StartSynchronizeJob provides a mock function with given fields: c, waitForJobToFinish, includeRemovedMembers -func (_m *LdapInterface) StartSynchronizeJob(c request.CTX, waitForJobToFinish bool, includeRemovedMembers bool) (*model.Job, *model.AppError) { - ret := _m.Called(c, waitForJobToFinish, includeRemovedMembers) +// StartSynchronizeJob provides a mock function with given fields: c, waitForJobToFinish, reAddRemovedMembers +func (_m *LdapInterface) StartSynchronizeJob(c request.CTX, waitForJobToFinish bool, reAddRemovedMembers *bool) (*model.Job, *model.AppError) { + ret := _m.Called(c, waitForJobToFinish, reAddRemovedMembers) if len(ret) == 0 { panic("no return value specified for StartSynchronizeJob") @@ -319,19 +319,19 @@ func (_m *LdapInterface) StartSynchronizeJob(c request.CTX, waitForJobToFinish b var r0 *model.Job var r1 *model.AppError - if rf, ok := ret.Get(0).(func(request.CTX, bool, bool) (*model.Job, *model.AppError)); ok { - return rf(c, waitForJobToFinish, includeRemovedMembers) + if rf, ok := ret.Get(0).(func(request.CTX, bool, *bool) (*model.Job, *model.AppError)); ok { + return rf(c, waitForJobToFinish, reAddRemovedMembers) } - if rf, ok := ret.Get(0).(func(request.CTX, bool, bool) *model.Job); ok { - r0 = rf(c, waitForJobToFinish, includeRemovedMembers) + if rf, ok := ret.Get(0).(func(request.CTX, bool, *bool) *model.Job); ok { + r0 = rf(c, waitForJobToFinish, reAddRemovedMembers) } else { if ret.Get(0) != nil { r0 = ret.Get(0).(*model.Job) } } - if rf, ok := ret.Get(1).(func(request.CTX, bool, bool) *model.AppError); ok { - r1 = rf(c, waitForJobToFinish, includeRemovedMembers) + if rf, ok := ret.Get(1).(func(request.CTX, bool, *bool) *model.AppError); ok { + r1 = rf(c, waitForJobToFinish, reAddRemovedMembers) } else { if ret.Get(1) != nil { r1 = ret.Get(1).(*model.AppError) diff --git a/server/public/model/client4.go b/server/public/model/client4.go index 8787475967..9d8fbc2643 100644 --- a/server/public/model/client4.go +++ b/server/public/model/client4.go @@ -5759,16 +5759,23 @@ func (c *Client4) GetClusterStatus(ctx context.Context) ([]*ClusterInfo, *Respon // LDAP Section -// SyncLdap will force a sync with the configured LDAP server. -// If includeRemovedMembers is true, then group members who left or were removed from a +// SyncLdap starts a run of the LDAP sync job. +// +// If reAddRemovedMembers 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(ctx context.Context, includeRemovedMembers bool) (*Response, error) { - reqBody, err := json.Marshal(map[string]any{ - "include_removed_members": includeRemovedMembers, - }) +// +// The ReAddRemovedMembers option is deprecated. Use LdapSettings.ReAddRemovedMembers instead. +func (c *Client4) SyncLdap(ctx context.Context, reAddRemovedMembers *bool) (*Response, error) { + data := map[string]any{} + if reAddRemovedMembers != nil { + data["include_removed_members"] = *reAddRemovedMembers + } + + reqBody, err := json.Marshal(data) if err != nil { return nil, NewAppError("SyncLdap", "api.marshal_error", nil, "", http.StatusInternalServerError).Wrap(err) } + r, err := c.DoAPIPostBytes(ctx, c.ldapRoute()+"/sync", reqBody) if err != nil { return BuildResponse(r), err diff --git a/server/public/model/config.go b/server/public/model/config.go index 6627e7205d..8e201ceb5f 100644 --- a/server/public/model/config.go +++ b/server/public/model/config.go @@ -2480,7 +2480,8 @@ type LdapSettings struct { PictureAttribute *string `access:"authentication_ldap"` // Synchronization - SyncIntervalMinutes *int `access:"authentication_ldap"` + SyncIntervalMinutes *int `access:"authentication_ldap"` + ReAddRemovedMembers *bool `access:"authentication_ldap"` // Advanced SkipCertificateVerification *bool `access:"authentication_ldap"` @@ -2613,6 +2614,10 @@ func (s *LdapSettings) SetDefaults() { s.SyncIntervalMinutes = NewPointer(60) } + if s.ReAddRemovedMembers == nil { + s.ReAddRemovedMembers = NewPointer(false) + } + if s.SkipCertificateVerification == nil { s.SkipCertificateVerification = NewPointer(false) } diff --git a/webapp/channels/src/components/admin_console/admin_definition.tsx b/webapp/channels/src/components/admin_console/admin_definition.tsx index 3da1fa0406..a122fe873b 100644 --- a/webapp/channels/src/components/admin_console/admin_definition.tsx +++ b/webapp/channels/src/components/admin_console/admin_definition.tsx @@ -4177,6 +4177,19 @@ const AdminDefinition: AdminDefinitionType = { ), ), }, + { + type: 'bool', + key: 'LdapSettings.ReAddRemovedMembers', + label: defineMessage({id: 'admin.ldap.reAddRemovedMembersTitle', defaultMessage: 'Re-add removed members on sync:'}), + help_text: defineMessage({id: 'admin.ldap.reAddRemovedMembersDesc', defaultMessage: 'When enabled, members who were previously removed from group-synced teams or channels will be re-added during LDAP synchronization if they are still a member of the LDAP group.'}), + isDisabled: it.any( + it.not(it.userHasWritePermissionOnResource(RESOURCE_KEYS.AUTHENTICATION.LDAP)), + it.all( + it.stateIsFalse('LdapSettings.Enable'), + it.stateIsFalse('LdapSettings.EnableSync'), + ), + ), + }, { type: 'number', key: 'LdapSettings.MaxPageSize', diff --git a/webapp/channels/src/i18n/en.json b/webapp/channels/src/i18n/en.json index b2fff491da..82d84441ca 100644 --- a/webapp/channels/src/i18n/en.json +++ b/webapp/channels/src/i18n/en.json @@ -1471,6 +1471,8 @@ "admin.ldap.queryDesc": "The timeout value for queries to the AD/LDAP server. Increase if you are getting timeout errors caused by a slow AD/LDAP server.", "admin.ldap.queryEx": "E.g.: \"60\"", "admin.ldap.queryTitle": "Query Timeout (seconds):", + "admin.ldap.reAddRemovedMembersDesc": "When enabled, members who were previously removed from group-synced teams or channels will be re-added during LDAP synchronization if they are still a member of the LDAP group.", + "admin.ldap.reAddRemovedMembersTitle": "Re-add removed members on sync:", "admin.ldap.remove.privKey": "Remove TLS Certificate Private Key", "admin.ldap.remove.sp_certificate": "Remove Service Provider Certificate", "admin.ldap.removing.certificate": "Removing Certificate...",