diff --git a/modules/ldap_groups/app/services/ldap_groups/synchronize_filter_service.rb b/modules/ldap_groups/app/services/ldap_groups/synchronize_filter_service.rb index 698858902e..0b0ded8222 100644 --- a/modules/ldap_groups/app/services/ldap_groups/synchronize_filter_service.rb +++ b/modules/ldap_groups/app/services/ldap_groups/synchronize_filter_service.rb @@ -69,7 +69,8 @@ module LdapGroups ## # Create or update the synchronized group item def create_or_update_sync_group(dn) - LdapGroups::SynchronizedGroup.find_or_initialize_by(dn: dn).tap do |sync| + group = LdapGroups::SynchronizedGroup + .find_or_initialize_by(dn: dn).tap do |sync| # Always set the filter and auth source, in case multiple filters match the same group # they are simply being re-assigned to the latest one sync.filter_id = filter.id @@ -78,6 +79,8 @@ module LdapGroups # Tell the group to synchronize users if the filter has requested it to sync.sync_users = filter.sync_users end + + group.tap { group.save! if group.persisted? } end ## diff --git a/modules/ldap_groups/spec/factories/synchronized_filter_factory.rb b/modules/ldap_groups/spec/factories/synchronized_filter_factory.rb index fe8d76ba70..660cb0f9d0 100644 --- a/modules/ldap_groups/spec/factories/synchronized_filter_factory.rb +++ b/modules/ldap_groups/spec/factories/synchronized_filter_factory.rb @@ -5,5 +5,6 @@ FactoryBot.define do group_name_attribute { 'cn' } base_dn { 'dc=example,dc=com' } auth_source factory: :ldap_auth_source + sync_users { true } end end diff --git a/modules/ldap_groups/spec/factories/synchronized_group_factory.rb b/modules/ldap_groups/spec/factories/synchronized_group_factory.rb index cd773296f4..bab7846d7e 100644 --- a/modules/ldap_groups/spec/factories/synchronized_group_factory.rb +++ b/modules/ldap_groups/spec/factories/synchronized_group_factory.rb @@ -3,5 +3,6 @@ FactoryBot.define do dn { 'cn=foo,ou=groups,dc=example,dc=com' } group factory: :group auth_source factory: :ldap_auth_source + sync_users { true } end end diff --git a/modules/ldap_groups/spec/services/synchronize_filter_spec.rb b/modules/ldap_groups/spec/services/synchronize_filter_spec.rb index 719f3440eb..4c3176f2e7 100644 --- a/modules/ldap_groups/spec/services/synchronize_filter_spec.rb +++ b/modules/ldap_groups/spec/services/synchronize_filter_spec.rb @@ -29,11 +29,11 @@ describe LdapGroups::SynchronizeFilterService, with_ee: %i[ldap_groups] do let(:synced_foo) do FactoryBot.create :ldap_synchronized_group, dn: 'cn=foo,ou=groups,dc=example,dc=com', group: group_foo, - auth_source: auth_source + auth_source: auth_source end let(:synced_bar) do FactoryBot.create :ldap_synchronized_group, dn: 'cn=bar,ou=groups,dc=example,dc=com', group: group_bar, - auth_source: auth_source + auth_source: auth_source end let(:filter_foo_bar) { FactoryBot.create :ldap_synchronized_filter, auth_source: auth_source } @@ -83,6 +83,33 @@ describe LdapGroups::SynchronizeFilterService, with_ee: %i[ldap_groups] do end end + describe 'when one group already exists with different settings' do + let(:synced_foo) do + FactoryBot.create :ldap_synchronized_group, + dn: 'cn=foo,ou=groups,dc=example,dc=com', + group: group_foo, + sync_users: false, + auth_source: auth_source + end + let(:filter_foo_bar) do + FactoryBot.create :ldap_synchronized_filter, + sync_users: true, + auth_source: auth_source + end + + before do + synced_foo + end + + it 'the group receives the value of the filter' do + expect(synced_foo.sync_users).to eq false + expect { subject }.not_to raise_error + + synced_foo.reload + expect(synced_foo.sync_users).to eq true + end + end + describe 'when it has a group that no longer exists in ldap' do let!(:group_doesnotexist) { FactoryBot.create :group, lastname: 'doesnotexist' } let!(:synced_doesnotexist) do