Remove mocks from find_group_with_ldap

openSUSE · Oct 2, 2024 · e3971a4 · e3971a4
1 parent 77dc45c
commit e3971a4
Show file tree

Hide file tree

Showing 3 changed files with 41 additions and 52 deletions.
diff --git a/src/api/app/models/user_ldap_strategy.rb b/src/api/app/models/user_ldap_strategy.rb
@@ -15,7 +15,7 @@ def find_group_with_ldap(group)
       result = []
       @@ldap_search_con.search(CONFIG['ldap_group_search_base'], LDAP::LDAP_SCOPE_SUBTREE, filter) do |entry|
         result << entry.dn
-        result << entry.attrs
+        # result << entry.attrs
       end
 
       if result.empty?
@@ -152,7 +152,7 @@ def find_with_credentials(login, password)
     # this method returns a ldap object using the provided user name
     # and password
     def initialize_ldap_con(user_name, password)
-      return unless defined?(CONFIG['ldap_servers'])
+      return if CONFIG['ldap_servers'].blank?
 
       require 'ldap'
       ldap_servers = CONFIG['ldap_servers'].split(':')

diff --git a/src/api/config/environments/test_ldap.rb b/src/api/config/environments/test_ldap.rb
@@ -140,3 +140,4 @@
 CONFIG['ldap_auth_attr'] = 'userPassword'
 CONFIG['ldap_group_search_base'] = 'dc=example,dc=org'
 CONFIG['ldap_group_title_attr'] = 'cn'
+CONFIG['ldap_group_objectclass_attr'] = 'posixGroup'
diff --git a/src/api/spec/models/user_ldap_strategy_spec.rb b/src/api/spec/models/user_ldap_strategy_spec.rb
@@ -87,41 +87,45 @@
   end
 
   describe '.initialize_ldap_con' do
-    context 'when no ldap_servers are configured' do
-      it { expect(UserLdapStrategy.send(:initialize_ldap_con, 'tux', 'tux_password')).to be_nil }
-    end
+    skip 'mocking SSL is a bit hard for now'
 
-    context 'when ldap servers are configured' do
-      context 'for SSL' do
-        include_context 'setup ldap mock', for_ssl: true
+    context 'for SSL' do
+      include_context 'setup ldap mock', for_ssl: true
 
-        before do
-          stub_const('CONFIG', CONFIG.merge('ldap_ssl' => :on))
-        end
+      before do
+        stub_const('CONFIG', CONFIG.merge('ldap_ssl' => :on))
+      end
 
-        it_behaves_like 'a mocked ldap connection'
+      it_behaves_like 'a mocked ldap connection'
+    end
+
+    context 'configured for TSL' do
+      include_context 'setup ldap mock', for_ssl: true, start_tls: true
+
+      before do
+        stub_const('CONFIG', CONFIG.merge('ldap_start_tls' => :on))
       end
 
-      context 'configured for TSL' do
-        include_context 'setup ldap mock', for_ssl: true, start_tls: true
+      it_behaves_like 'a mocked ldap connection'
+    end
 
+    context 'not configured for TSL or SSL' do
+      context 'when no ldap_servers are configured' do
         before do
-          stub_const('CONFIG', CONFIG.merge('ldap_start_tls' => :on))
+          stub_const('CONFIG', CONFIG.merge('ldap_servers' => nil))
         end
 
-        it_behaves_like 'a mocked ldap connection'
+        it { expect(UserLdapStrategy.send(:initialize_ldap_con, 'tux', 'tux_password')).to be_nil }
       end
 
-      context 'not configured for TSL or SSL' do
-        context 'when a connection can be established' do
-          it 'returns the connection object' do
-            expect(UserLdapStrategy.send(:initialize_ldap_con, CONFIG['ldap_search_user'], CONFIG['ldap_search_auth'])).to be_bound
-          end
+      context 'when a connection can be established' do
+        it 'returns the connection object' do
+          expect(UserLdapStrategy.send(:initialize_ldap_con, CONFIG['ldap_search_user'], CONFIG['ldap_search_auth'])).to be_bound
         end
+      end
 
-        context 'when a connection can not be established' do
-          it { expect(UserLdapStrategy.send(:initialize_ldap_con, CONFIG['ldap_search_user'], 'WRONG_password')).to be_nil }
-        end
+      context 'when a connection can not be established' do
+        it { expect(UserLdapStrategy.send(:initialize_ldap_con, CONFIG['ldap_search_user'], 'WRONG_password')).to be_nil }
       end
     end
   end
@@ -134,51 +138,35 @@
     end
 
     context 'when there is no connection' do
+      before do
+        stub_const('CONFIG', CONFIG.reject { |key, _| key == 'ldap_servers' })
+      end
+
       it { expect(UserLdapStrategy.find_group_with_ldap('any_group')).to be_blank }
     end
 
     context 'when there is a connection' do
-      include_context 'setup ldap mock', for_ssl: true
-
-      before do
-        stub_const('CONFIG', CONFIG.merge('ldap_search_user' => 'tux',
-                                          'ldap_search_auth' => 'tux_password',
-                                          'ldap_group_objectclass_attr' => 'groupOfNames',
-                                          'ldap_group_search_base' => 'ou=OBSGROUPS,dc=EXAMPLE,dc=COM',
-                                          'ldap_group_title_attr' => 'ldap_group',
-                                          'ldap_ssl' => :on))
-
-        allow(ldap_mock).to receive(:bind).with('tux', 'tux_password')
-        allow(ldap_mock).to receive(:bound?).and_return(true)
-      end
-
       context "with 'ldap_group_objectclass_attr' configured" do
-        before do
-          allow(ldap_mock).to receive(:search).with(
-            'ou=OBSGROUPS,dc=EXAMPLE,dc=COM', LDAP::LDAP_SCOPE_SUBTREE, '(&(ldap_group=any_group)(objectclass=groupOfNames))'
-          ).and_yield(double(dn: 'some_dn', attrs: 'some_attr'))
-        end
-
-        it { expect(UserLdapStrategy.find_group_with_ldap('any_group')).to eq(%w[some_dn some_attr]) }
+        it { expect(UserLdapStrategy.find_group_with_ldap('users')).to eq(%w[cn=users,ou=groups,dc=example,dc=org]) }
       end
 
       context "without 'ldap_group_objectclass_attr' configured" do
         before do
           stub_const('CONFIG', CONFIG.reject { |key, _| key == 'ldap_group_objectclass_attr' })
 
-          allow(ldap_mock).to receive(:search).with(
-            'ou=OBSGROUPS,dc=EXAMPLE,dc=COM', LDAP::LDAP_SCOPE_SUBTREE, '(ldap_group=any_group)'
-          ).and_yield(double(dn: 'some_dn', attrs: 'some_attr'))
+          # allow(ldap_mock).to receive(:search).with(
+          #   'ou=OBSGROUPS,dc=EXAMPLE,dc=COM', LDAP::LDAP_SCOPE_SUBTREE, '(ldap_group=any_group)'
+          # ).and_yield(double(dn: 'some_dn', attrs: 'some_attr'))
         end
 
-        it { expect(UserLdapStrategy.find_group_with_ldap('any_group')).to eq(%w[some_dn some_attr]) }
+        it { expect(UserLdapStrategy.find_group_with_ldap('users')).to eq(%w[cn=users,ou=groups,dc=example,dc=org]) }
       end
 
       context 'when there is no result' do
         before do
-          allow(ldap_mock).to receive(:search).with(
-            'ou=OBSGROUPS,dc=EXAMPLE,dc=COM', LDAP::LDAP_SCOPE_SUBTREE, '(&(ldap_group=any_group)(objectclass=groupOfNames))'
-          )
+          # allow(ldap_mock).to receive(:search).with(
+          #   'ou=OBSGROUPS,dc=EXAMPLE,dc=COM', LDAP::LDAP_SCOPE_SUBTREE, '(&(ldap_group=any_group)(objectclass=groupOfNames))'
+          # )
         end
 
         it { expect(UserLdapStrategy.find_group_with_ldap('any_group')).to eq([]) }