Merge pull request #963 from dmlond/DDS-859-mail_bugfix

DDS-859 mail bugfix

Author: dmann

app/models/ldap_identity_provider.rb

@@ -36,7 +36,7 @@ def ldap_search(ldap_filter)
           username: entry.uid.first,
           first_name: entry.givenName.first,
           last_name: entry.sn.first,
-          email: entry.attribute_names.include?(:uid) ? entry.mail.first : nil,
+          email: entry.attribute_names.include?(:mail) ? entry.mail.first : nil,
           display_name: entry.displayName.first
         )
       end

spec/models/ldap_identity_provider_spec.rb

@@ -56,21 +56,57 @@
       end
 
       context 'greater than 3 characters' do
-        let(:ldap_returns) { [test_user] }
-        include_context 'mocked ldap', returns: :ldap_returns
-        subject { auth_provider.identity_provider.affiliates(
-          test_user[:last_name]
-        ) }
+        context 'missing uid' do
+          let(:test_user) { FactoryGirl.attributes_for(:user).reject{|k| k == :username } }
+          let(:ldap_returns) { [test_user] }
+          include_context 'mocked ldap', returns: :ldap_returns
+          subject { auth_provider.identity_provider.affiliates(
+            test_user[:last_name]
+          ) }
 
-        it {
-          is_expected.to be_a Array
-          expect(subject.length).to be > 0
-          subject.each do |response|
-            expect(response).to be_a User
-            expect(response).not_to be_persisted
-            expect(response.display_name).to eq test_user[:display_name]
-          end
-        }
+          it {
+            is_expected.to be_a Array
+            expect(subject.length).to eq 0
+          }
+        end
+
+        context 'missing mail' do
+          let(:test_user) { FactoryGirl.attributes_for(:user).reject{|k| k == :email } }
+          let(:ldap_returns) { [test_user] }
+          include_context 'mocked ldap', returns: :ldap_returns
+          subject { auth_provider.identity_provider.affiliates(
+            test_user[:last_name]
+          ) }
+
+          it {
+            is_expected.to be_a Array
+            expect(subject.length).to be > 0
+            subject.each do |response|
+              expect(response).to be_a User
+              expect(response).not_to be_persisted
+              expect(response.display_name).to eq test_user[:display_name]
+              expect(response.email).not_to be
+            end
+          }
+        end
+
+        context 'complete record' do
+          let(:ldap_returns) { [test_user] }
+          include_context 'mocked ldap', returns: :ldap_returns
+          subject { auth_provider.identity_provider.affiliates(
+            test_user[:last_name]
+          ) }
+
+          it {
+            is_expected.to be_a Array
+            expect(subject.length).to be > 0
+            subject.each do |response|
+              expect(response).to be_a User
+              expect(response).not_to be_persisted
+              expect(response.display_name).to eq test_user[:display_name]
+            end
+          }
+        end
       end
     end
   end

spec/support/ldap_shared_examples.rb

@@ -3,10 +3,10 @@
   let(:expected_entries) {
     entry_users.map { |test_user|
       expected_entry = Net::LDAP::Entry.new
-      expected_entry[:uid] = test_user[:username]
+      expected_entry[:uid] = test_user[:username] if test_user[:username]
       expected_entry[:givenName] = test_user[:first_name]
       expected_entry[:sn] = test_user[:last_name]
-      expected_entry[:mail] = test_user[:email]
+      expected_entry[:mail] = test_user[:email] if test_user[:email]
       expected_entry[:displayName] = test_user[:display_name]
       expected_entry
     }