gitlabhq · dimaj · Jan 6, 2013
diff --git a/README.md b/README.md
@@ -11,6 +11,8 @@ Use the LDAP strategy as a middleware in your application:
         :method => :plain,
         :base => 'dc=intridea, dc=com',
         :uid => 'sAMAccountName',
+        # Or, alternatively:
+        #:filter => '(&(uid=%{username})(memberOf=cn=myapp-users,ou=groups,dc=example,dc=com))'
         :name_proc => Proc.new {|name| name.gsub(/@.*$/,'')}
         :bind_dn => 'default_bind_dn'
         :password => 'password'
@@ -29,6 +31,9 @@ Allowed values of :method are: :plain, :ssl, :tls.
 :uid is the LDAP attribute name for the user name in the login form. 
   typically AD would be 'sAMAccountName' or 'UserPrincipalName', while OpenLDAP is 'uid'.
 
+:filter is the LDAP filter used to search the user entry. It can be used in place of :uid for more flexibility.
+  `%{username}` will be replaced by the user name processed by :name_proc.
+
 :name_proc allows you to match the user name entered with the format of the :uid attributes. 
   For example, value of 'sAMAccountName' in AD contains only the windows user name. If your user prefers using 
   email to login, a name_proc as above will trim the email string down to just the windows login name. 

diff --git a/lib/omniauth-ldap/adaptor.rb b/lib/omniauth-ldap/adaptor.rb
@@ -14,9 +14,10 @@ class ConfigurationError < StandardError; end
       class AuthenticationError < StandardError; end
       class ConnectionError < StandardError; end
 
-      VALID_ADAPTER_CONFIGURATION_KEYS = [:host, :port, :method, :bind_dn, :password, :try_sasl, :sasl_mechanisms, :uid, :base, :allow_anonymous]
+      VALID_ADAPTER_CONFIGURATION_KEYS = [:host, :port, :method, :bind_dn, :password, :try_sasl, :sasl_mechanisms, :uid, :base, :allow_anonymous, :filter]
 
-      MUST_HAVE_KEYS = [:host, :port, :method, :uid, :base]
+      # A list of needed keys. Possible alternatives are specified using sub-lists.
+      MUST_HAVE_KEYS = [:host, :port, :method, [:uid, :filter], :base]
 
       METHOD = {
         :ssl => :simple_tls,
@@ -25,11 +26,15 @@ class ConnectionError < StandardError; end
       }
 
       attr_accessor :bind_dn, :password
-      attr_reader :connection, :uid, :base, :auth
+      attr_reader :connection, :uid, :base, :auth, :filter
       def self.validate(configuration={})
         message = []
-        MUST_HAVE_KEYS.each do |name|
-           message << name if configuration[name].nil?
+        MUST_HAVE_KEYS.each do |names|
+          names = [names].flatten
+          missing_keys = names.select{|name| configuration[name].nil?}
+          if missing_keys == names
+            message << names.join(' or ')
+          end
         end
         raise ArgumentError.new(message.join(",") +" MUST be provided") unless message.empty?
       end

diff --git a/lib/omniauth/strategies/ldap.rb b/lib/omniauth/strategies/ldap.rb
@@ -45,7 +45,7 @@ def callback_phase
             raise MissingCredentialsError.new("Missing login credentials")
           end
 
-          @ldap_user_info = @adaptor.bind_as(:filter => Net::LDAP::Filter.eq(@adaptor.uid, @options[:name_proc].call(request['username'])),:size => 1, :password => request['password'])
+          @ldap_user_info = @adaptor.bind_as(:filter => filter(@adaptor), :size => 1, :password => request['password'])
           return fail!(:invalid_credentials) if !@ldap_user_info
 
           @user_info = self.class.map_user(@@config, @ldap_user_info)
@@ -55,6 +55,14 @@ def callback_phase
         end
       end
 
+      def filter adaptor
+        if adaptor.filter and !adaptor.filter.empty?
+          Net::LDAP::Filter.construct(adaptor.filter % {username: @options[:name_proc].call(request['username'])})
+        else
+          Net::LDAP::Filter.eq(adaptor.uid, @options[:name_proc].call(request['username']))
+        end
+      end
+
       uid {
         @user_info["uid"]
       }

diff --git a/spec/omniauth/strategies/ldap_spec.rb b/spec/omniauth/strategies/ldap_spec.rb
@@ -50,62 +50,62 @@ def session
   end
 
   describe 'post /auth/ldap/callback' do
-    before(:each) do
-      @adaptor = mock(OmniAuth::LDAP::Adaptor, {:uid => 'ping'})
-      OmniAuth::LDAP::Adaptor.stub(:new).and_return(@adaptor)
-    end
-
-    context 'failure' do
-      before(:each) do
-        @adaptor.stub(:bind_as).and_return(false)
-      end
-
-      it 'should raise MissingCredentialsError' do
-        post('/auth/ldap/callback', {})
-        last_response.should be_redirect
-        last_response.headers['Location'].should =~ %r{ldap_error}
-      end
-
-      it 'should redirect to error page' do
-        post('/auth/ldap/callback', {:username => 'ping', :password => 'password'})
-        last_response.should be_redirect
-        last_response.headers['Location'].should =~ %r{invalid_credentials}
-      end
-
-      it 'should redirect to error page when there is exception' do
-        @adaptor.stub(:bind_as).and_throw(Exception.new('connection_error'))
-        post('/auth/ldap/callback', {:username => 'ping', :password => 'password'})
-        last_response.should be_redirect
-        last_response.headers['Location'].should =~ %r{ldap_error}
-      end
-    end
-
-    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 => ['ping@intridea.com'], :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'})
-      end
-
-      it 'should raise MissingCredentialsError' do
-        should_not raise_error OmniAuth::Strategies::LDAP::MissingCredentialsError
-      end
-      it 'should map user info' do
-        auth_hash.uid.should == 'cn=ping, dc=intridea, dc=com'
-        auth_hash.info.email.should == 'ping@intridea.com'
-        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'
+    {:filter => '(ping=%{username})', :uid => 'ping'}.each_pair do |key, value|
+      context "when using :#{key}" do
+        before(:each) do
+          mocked_methods = {:filter => nil, :uid => nil}
+          mocked_methods[key] = value
+          @adaptor = mock(OmniAuth::LDAP::Adaptor, mocked_methods)
+          OmniAuth::LDAP::Adaptor.stub(:new).and_return(@adaptor)
+        end
+        context 'failure' do
+          before(:each) do
+            @adaptor.stub(:bind_as).and_return(false)
+          end
+          it 'should raise MissingCredentialsError' do
+            lambda{post('/auth/ldap/callback', {})}.should raise_error OmniAuth::Strategies::LDAP::MissingCredentialsError
+          end
+          it 'should redirect to error page' do        
+            post('/auth/ldap/callback', {:username => 'ping', :password => 'password'})
+            last_response.should be_redirect
+            last_response.headers['Location'].should =~ %r{invalid_credentials}
+          end
+          it 'should redirect to error page when there is exception' do        
+            @adaptor.stub(:bind_as).and_throw(Exception.new('connection_error'))
+            post('/auth/ldap/callback', {:username => 'ping', :password => 'password'})
+            last_response.should be_redirect
+            last_response.headers['Location'].should =~ %r{ldap_error}
+          end
+        end
+        
+        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 => ['ping@intridea.com'], :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'})
+          end
+          
+          it 'should raise MissingCredentialsError' do
+            should_not raise_error OmniAuth::Strategies::LDAP::MissingCredentialsError
+          end
+          it 'should map user info' do
+            auth_hash.uid.should == 'cn=ping, dc=intridea, dc=com'
+            auth_hash.info.email.should == 'ping@intridea.com'
+            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
+        end
       end
     end
   end