diff --git a/lib/omniauth/strategies/ldap.rb b/lib/omniauth/strategies/ldap.rb index 45fb1d1..2ba577a 100644 --- a/lib/omniauth/strategies/ldap.rb +++ b/lib/omniauth/strategies/ldap.rb @@ -3,7 +3,6 @@ module OmniAuth module Strategies class LDAP - class MissingCredentialsError < StandardError; end include OmniAuth::Strategy @@config = { 'name' => 'cn', @@ -38,7 +37,7 @@ def request_phase def callback_phase @adaptor = OmniAuth::LDAP::Adaptor.new @options - raise MissingCredentialsError.new("Missing login credentials") if request['username'].nil? || request['password'].nil? + return fail!(:missing_credentials) if missing_credentials? begin @ldap_user_info = @adaptor.bind_as(:filter => Net::LDAP::Filter.eq(@adaptor.uid, @options[:name_proc].call(request['username'])),:size => 1, :password => request['password']) return fail!(:invalid_credentials) if !@ldap_user_info @@ -81,6 +80,12 @@ def self.map_user(mapper, object) end user end + + protected + + def missing_credentials? + request['username'].nil? or request['username'].empty? or request['password'].nil? or request['password'].empty? + end # missing_credentials? end end end diff --git a/spec/omniauth-ldap/adaptor_spec.rb b/spec/omniauth-ldap/adaptor_spec.rb index b205c57..e6a304f 100644 --- a/spec/omniauth-ldap/adaptor_spec.rb +++ b/spec/omniauth-ldap/adaptor_spec.rb @@ -2,13 +2,13 @@ describe "OmniAuth::LDAP::Adaptor" do describe 'initialize' do - it 'should throw exception when must have field is not set' do #[:host, :port, :method, :bind_dn] - lambda { OmniAuth::LDAP::Adaptor.new({host: "192.168.1.145", method: 'plain'})}.should raise_error(ArgumentError) + lambda { OmniAuth::LDAP::Adaptor.new({host: "192.168.1.145", method: 'plain'})}.should raise_error(ArgumentError) end + it 'should throw exception when method is not supported' do - lambda { OmniAuth::LDAP::Adaptor.new({host: "192.168.1.145", method: 'myplain', uid: 'uid', port: 389, base: 'dc=com'})}.should raise_error(OmniAuth::LDAP::Adaptor::ConfigurationError) + lambda { OmniAuth::LDAP::Adaptor.new({host: "192.168.1.145", method: 'myplain', uid: 'uid', port: 389, base: 'dc=com'})}.should raise_error(OmniAuth::LDAP::Adaptor::ConfigurationError) end it 'should setup ldap connection with anonymous' do @@ -17,54 +17,59 @@ adaptor.connection.host.should == '192.168.1.145' adaptor.connection.port.should == 389 adaptor.connection.base.should == 'dc=intridea, dc=com' - adaptor.connection.instance_variable_get('@auth').should == {:method => :anonymous, :username => nil, :password => nil} + adaptor.connection.instance_variable_get('@auth').should == {:method => :anonymous, :username => nil, :password => nil} end + it 'should setup ldap connection with simple' do adaptor = OmniAuth::LDAP::Adaptor.new({host: "192.168.1.145", method: 'plain', base: 'dc=intridea, dc=com', port: 389, uid: 'sAMAccountName', bind_dn: 'bind_dn', password: 'password'}) adaptor.connection.should_not == nil adaptor.connection.host.should == '192.168.1.145' adaptor.connection.port.should == 389 adaptor.connection.base.should == 'dc=intridea, dc=com' - adaptor.connection.instance_variable_get('@auth').should == {:method => :simple, :username => 'bind_dn', :password => 'password'} - end + adaptor.connection.instance_variable_get('@auth').should == {:method => :simple, :username => 'bind_dn', :password => 'password'} + end + it 'should setup ldap connection with sasl-md5' do adaptor = OmniAuth::LDAP::Adaptor.new({host: "192.168.1.145", method: 'plain', base: 'dc=intridea, dc=com', port: 389, uid: 'sAMAccountName', try_sasl: true, sasl_mechanisms: ["DIGEST-MD5"], bind_dn: 'bind_dn', password: 'password'}) adaptor.connection.should_not == nil adaptor.connection.host.should == '192.168.1.145' adaptor.connection.port.should == 389 adaptor.connection.base.should == 'dc=intridea, dc=com' - adaptor.connection.instance_variable_get('@auth')[:method].should == :sasl - adaptor.connection.instance_variable_get('@auth')[:mechanism].should == 'DIGEST-MD5' - adaptor.connection.instance_variable_get('@auth')[:initial_credential].should == '' - adaptor.connection.instance_variable_get('@auth')[:challenge_response].should_not be_nil + adaptor.connection.instance_variable_get('@auth')[:method].should == :sasl + adaptor.connection.instance_variable_get('@auth')[:mechanism].should == 'DIGEST-MD5' + adaptor.connection.instance_variable_get('@auth')[:initial_credential].should == '' + adaptor.connection.instance_variable_get('@auth')[:challenge_response].should_not be_nil end + it 'should setup ldap connection with sasl-gss' do adaptor = OmniAuth::LDAP::Adaptor.new({host: "192.168.1.145", method: 'plain', base: 'dc=intridea, dc=com', port: 389, uid: 'sAMAccountName', try_sasl: true, sasl_mechanisms: ["GSS-SPNEGO"], bind_dn: 'bind_dn', password: 'password'}) adaptor.connection.should_not == nil adaptor.connection.host.should == '192.168.1.145' adaptor.connection.port.should == 389 adaptor.connection.base.should == 'dc=intridea, dc=com' - adaptor.connection.instance_variable_get('@auth')[:method].should == :sasl - adaptor.connection.instance_variable_get('@auth')[:mechanism].should == 'GSS-SPNEGO' - adaptor.connection.instance_variable_get('@auth')[:initial_credential].should =~ /^NTLMSSP/ - adaptor.connection.instance_variable_get('@auth')[:challenge_response].should_not be_nil + adaptor.connection.instance_variable_get('@auth')[:method].should == :sasl + adaptor.connection.instance_variable_get('@auth')[:mechanism].should == 'GSS-SPNEGO' + adaptor.connection.instance_variable_get('@auth')[:initial_credential].should =~ /^NTLMSSP/ + adaptor.connection.instance_variable_get('@auth')[:challenge_response].should_not be_nil end end - + describe 'bind_as' do let(:args) { {:filter => Net::LDAP::Filter.eq('sAMAccountName', 'username'), :password => 'password', :size => 1} } let(:rs) { Struct.new(:dn).new('new dn') } + it 'should bind simple' do adaptor = OmniAuth::LDAP::Adaptor.new({host: "192.168.1.126", method: 'plain', base: 'dc=score, dc=local', port: 389, uid: 'sAMAccountName', bind_dn: 'bind_dn', password: 'password'}) adaptor.connection.should_receive(:open).and_yield(adaptor.connection) - adaptor.connection.should_receive(:search).with(args).and_return([rs]) + adaptor.connection.should_receive(:search).with(args).and_return([rs]) adaptor.connection.should_receive(:bind).with({:username => 'new dn', :password => args[:password], :method => :simple}).and_return(true) adaptor.bind_as(args).should == rs end + it 'should bind sasl' do adaptor = OmniAuth::LDAP::Adaptor.new({host: "192.168.1.145", method: 'plain', base: 'dc=intridea, dc=com', port: 389, uid: 'sAMAccountName', try_sasl: true, sasl_mechanisms: ["GSS-SPNEGO"], bind_dn: 'bind_dn', password: 'password'}) adaptor.connection.should_receive(:open).and_yield(adaptor.connection) - adaptor.connection.should_receive(:search).with(args).and_return([rs]) + adaptor.connection.should_receive(:search).with(args).and_return([rs]) adaptor.connection.should_receive(:bind).and_return(true) adaptor.bind_as(args).should == rs end diff --git a/spec/omniauth/strategies/ldap_spec.rb b/spec/omniauth/strategies/ldap_spec.rb index 2a39f79..caea7dd 100644 --- a/spec/omniauth/strategies/ldap_spec.rb +++ b/spec/omniauth/strategies/ldap_spec.rb @@ -1,6 +1,6 @@ require 'spec_helper' describe "OmniAuth::Strategies::LDAP" do - # :title => "My LDAP", + # :title => "My LDAP", # :host => '10.101.10.1', # :port => 389, # :method => :plain, @@ -8,17 +8,18 @@ # :uid => 'sAMAccountName', # :name_proc => Proc.new {|name| name.gsub(/@.*$/,'')} # :bind_dn => 'default_bind_dn' - # :password => 'password' - class MyLdapProvider < OmniAuth::Strategies::LDAP; end - def app + # :password => 'password' + class MyLdapProvider < OmniAuth::Strategies::LDAP; end + + let(:app) do Rack::Builder.new { use OmniAuth::Test::PhonySession - use MyLdapProvider, :name => 'ldap', :title => 'MyLdap Form', :host => '192.168.1.145', :base => 'dc=score, dc=local', :name_proc => Proc.new {|name| name.gsub(/@.*$/,'')} + use MyLdapProvider, :name => 'ldap', :title => 'MyLdap Form', :host => '192.168.1.145', :base => 'dc=score, dc=local', :name_proc => Proc.new {|name| name.gsub(/@.*$/,'')} run lambda { |env| [404, {'Content-Type' => 'text/plain'}, [env.key?('omniauth.auth').to_s]] } }.to_app end - def session + let(:session) do last_request.env['rack.session'] end @@ -44,7 +45,6 @@ def session it 'should have a label of the form title' do last_response.body.scan('MyLdap Form').size.should > 1 end - end describe 'post /auth/ldap/callback' do @@ -52,40 +52,101 @@ def session @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 - lambda{post('/auth/ldap/callback', {})}.should raise_error OmniAuth::Strategies::LDAP::MissingCredentialsError + before(:each) do + @adaptor.stub(:bind_as).and_return(false) 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} + + context "when username is not preset" do + it 'should redirect to error page' do + post('/auth/ldap/callback', {}) + + last_response.should be_redirect + last_response.headers['Location'].should =~ %r{missing_credentials} + end 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} + + context "when username is empty" do + it 'should redirect to error page' do + post('/auth/ldap/callback', {:username => ""}) + + last_response.should be_redirect + last_response.headers['Location'].should =~ %r{missing_credentials} + end + end + + context "when username is present" do + context "and password is not preset" do + it 'should redirect to error page' do + post('/auth/ldap/callback', {:username => "ping"}) + + last_response.should be_redirect + last_response.headers['Location'].should =~ %r{missing_credentials} + end + end + + context "and password is empty" do + it 'should redirect to error page' do + post('/auth/ldap/callback', {:username => "ping", :password => ""}) + + last_response.should be_redirect + last_response.headers['Location'].should =~ %r{missing_credentials} + end + end + end + + context "when username and password are present" do + context "and bind on LDAP server failed" do + 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 + end + + context "and communication with LDAP server caused an exception" do + before :each do + @adaptor.stub(:bind_as).and_throw(Exception.new('connection_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{ldap_error} + end + end 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']}) + @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 + + it 'should not redirect to error page' do + last_response.should_not be_redirect end - it 'should map user info' do + + it 'should map user info to Auth Hash' 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'