Fix problem where the LDAP server returns alternate field

Gemfile.lock

@@ -1,7 +1,7 @@
 PATH
   remote: .
   specs:
-    gitlab_omniauth-ldap (1.0.2)
+    gitlab_omniauth-ldap (1.0.3)
       net-ldap (~> 0.3.1)
       omniauth (~> 1.0)
       pyu-ruby-sasl (~> 0.0.3.1)
@@ -12,11 +12,11 @@ GEM
   specs:
     coderay (1.0.8)
     diff-lcs (1.1.3)
-    hashie (1.2.0)
+    hashie (2.0.5)
     method_source (0.8.1)
     net-ldap (0.3.1)
-    omniauth (1.1.1)
-      hashie (~> 1.2)
+    omniauth (1.1.4)
+      hashie (>= 1.2, < 3)
       rack
     pry (0.9.10)
       coderay (~> 1.0.5)

lib/omniauth/strategies/ldap.rb

@@ -70,9 +70,11 @@ def self.map_user(mapper, object)
         mapper.each do |key, value|
           case value
           when String
-            user[key] = object[value.downcase.to_sym].first if object[value.downcase.to_sym]
+            user[key] = [object[value.downcase.to_sym]].flatten.first
           when Array
-            value.each {|v| (user[key] = object[v.downcase.to_sym].first; break;) if object[v.downcase.to_sym]}
+            user[key] = value.map { |v| 
+              [object[v.downcase.to_sym]].flatten.first 
+            }.compact.first
           when Hash
             value.map do |key1, value1|
               pattern = key1.dup

spec/omniauth/strategies/ldap_spec.rb

@@ -82,30 +82,79 @@ def session
 
     context 'success' do
       let(:auth_hash){ last_request.env['omniauth.auth'] }
-      before(:each) do
-        @adaptor.stub(:bind_as).and_return({:dn => ['cn=ping, dc=intridea, dc=com'], :mail => ['[email protected]'], :givenname => ['Ping'], :sn => ['Yu'],
-                                           :telephonenumber => ['555-555-5555'], :mobile => ['444-444-4444'], :uid => ['ping'], :title => ['dev'], :address =>[ 'k street'],
-                                           :l => ['Washington'], :st => ['DC'], :co => ["U.S.A"], :postofficebox => ['20001'], :wwwhomepage => ['www.intridea.com'],
-                                           :jpegphoto => ['http://www.intridea.com/ping.jpg'], :description => ['omniauth-ldap']})
-        post('/auth/ldap/callback', {:username => 'ping', :password => 'password'})
+      let(:search_result) do
+        {
+          :dn => ['cn=ping, dc=intridea, dc=com'], 
+          :mail => ['[email protected]'], 
+          :givenname => ['Ping'], 
+          :sn => ['Yu'],
+          :telephonenumber => ['555-555-5555'], 
+          :mobile => ['444-444-4444'], 
+          :uid => ['ping'], 
+          :title => ['dev'], 
+          :address =>[ 'k street'],
+          :l => ['Washington'], 
+          :st => ['DC'], 
+          :co => ["U.S.A"], 
+          :postofficebox => ['20001'], 
+          :wwwhomepage => ['www.intridea.com'],
+          :jpegphoto => ['http://www.intridea.com/ping.jpg'], 
+          :description => ['omniauth-ldap']
+        }
       end
 
-      it 'should raise MissingCredentialsError' do
-        should_not raise_error OmniAuth::Strategies::LDAP::MissingCredentialsError
+      let(:search_result_alt_fields) do
+        search_result.dup.tap do |hsh|
+          hsh[:userprincipalname] = hsh[:mail]
+          hsh.delete :mail
+        end
+      end
+
+      let(:verify_block) { 
+        Proc.new do
+          auth_hash.uid.should == 'cn=ping, dc=intridea, dc=com'
+          auth_hash.info.email.should == '[email protected]'
+          auth_hash.info.first_name.should == 'Ping'
+          auth_hash.info.last_name.should == 'Yu'
+          auth_hash.info.phone.should == '555-555-5555'
+          auth_hash.info.mobile.should == '444-444-4444'
+          auth_hash.info.nickname.should == 'ping'
+          auth_hash.info.title.should == 'dev'
+          auth_hash.info.location.should == 'k street, Washington, DC, U.S.A 20001'
+          auth_hash.info.url.should == 'www.intridea.com'
+          auth_hash.info.image.should == 'http://www.intridea.com/ping.jpg'
+          auth_hash.info.description.should == 'omniauth-ldap'
+        end
+      }
+
+      context 'results with first option' do
+        before(:each) do
+          @adaptor.stub(:bind_as).and_return(search_result)
+          post('/auth/ldap/callback', {:username => 'ping', :password => 'password'})
+        end
+
+        it 'should raise MissingCredentialsError' do
+          should_not raise_error OmniAuth::Strategies::LDAP::MissingCredentialsError
+        end
+
+        it 'should map user info' do
+          verify_block.call
+        end
       end
-      it 'should map user info' do
-        auth_hash.uid.should == 'cn=ping, dc=intridea, dc=com'
-        auth_hash.info.email.should == '[email protected]'
-        auth_hash.info.first_name.should == 'Ping'
-        auth_hash.info.last_name.should == 'Yu'
-        auth_hash.info.phone.should == '555-555-5555'
-        auth_hash.info.mobile.should == '444-444-4444'
-        auth_hash.info.nickname.should == 'ping'
-        auth_hash.info.title.should == 'dev'
-        auth_hash.info.location.should == 'k street, Washington, DC, U.S.A 20001'
-        auth_hash.info.url.should == 'www.intridea.com'
-        auth_hash.info.image.should == 'http://www.intridea.com/ping.jpg'
-        auth_hash.info.description.should == 'omniauth-ldap'
+
+      context 'results with alternate fields' do
+        before(:each) do
+          @adaptor.stub(:bind_as).and_return(search_result_alt_fields)
+          post('/auth/ldap/callback', {:username => 'ping', :password => 'password'})
+        end
+
+        it 'should raise MissingCredentialsError' do
+          should_not raise_error OmniAuth::Strategies::LDAP::MissingCredentialsError
+        end
+
+        it 'should map user info' do
+          verify_block.call
+        end
       end
     end
   end