From 2763040162d6ec432234c21d4c6a837c559c3ac2 Mon Sep 17 00:00:00 2001 From: Michael Baker Date: Tue, 29 Nov 2011 22:01:34 +0000 Subject: [PATCH 1/2] Return PDU instead of result code --- lib/net/ldap.rb | 57 ++++++++++++++++++++--------------- lib/net/ldap/pdu.rb | 8 +++++ spec/unit/ldap/search_spec.rb | 7 ++--- spec/unit/ldap_spec.rb | 32 +++++++++++++++++++- 4 files changed, 74 insertions(+), 30 deletions(-) diff --git a/lib/net/ldap.rb b/lib/net/ldap.rb index 35b3ddf..fe31ce8 100644 --- a/lib/net/ldap.rb +++ b/lib/net/ldap.rb @@ -629,11 +629,10 @@ class Net::LDAP yield entry if block_given? } else - @result = 0 begin conn = Net::LDAP::Connection.new(:host => @host, :port => @port, :encryption => @encryption) - if (@result = conn.bind(args[:auth] || @auth)) == 0 + if (@result = conn.bind(args[:auth] || @auth)).result_code == 0 @result = conn.search(args) { |entry| result_set << entry if result_set yield entry if block_given? @@ -645,9 +644,9 @@ class Net::LDAP end if return_result_set - @result == 0 ? result_set : nil + (!@result.nil? && @result.result_code == 0) ? result_set : nil else - @result == 0 + @result end end @@ -721,7 +720,7 @@ class Net::LDAP end end - @result == 0 + @result end # #bind_as is for testing authentication credentials. @@ -816,14 +815,14 @@ class Net::LDAP begin conn = Connection.new(:host => @host, :port => @port, :encryption => @encryption) - if (@result = conn.bind(args[:auth] || @auth)) == 0 + if (@result = conn.bind(args[:auth] || @auth)).result_code == 0 @result = conn.add(args) end ensure conn.close if conn end end - @result == 0 + @result end # Modifies the attribute values of a particular entry on the LDAP @@ -914,14 +913,15 @@ class Net::LDAP begin conn = Connection.new(:host => @host, :port => @port, :encryption => @encryption) - if (@result = conn.bind(args[:auth] || @auth)) == 0 + if (@result = conn.bind(args[:auth] || @auth)).result_code == 0 @result = conn.modify(args) end ensure conn.close if conn end end - @result == 0 + + @result end # Add a value to an attribute. Takes the full DN of the entry to modify, @@ -985,14 +985,14 @@ class Net::LDAP begin conn = Connection.new(:host => @host, :port => @port, :encryption => @encryption) - if (@result = conn.bind(args[:auth] || @auth)) == 0 + if (@result = conn.bind(args[:auth] || @auth)).result_code == 0 @result = conn.rename(args) end ensure conn.close if conn end end - @result == 0 + @result end alias_method :modify_rdn, :rename @@ -1013,14 +1013,14 @@ class Net::LDAP begin conn = Connection.new(:host => @host, :port => @port, :encryption => @encryption) - if (@result = conn.bind(args[:auth] || @auth)) == 0 + if (@result = conn.bind(args[:auth] || @auth)).result_code == 0 @result = conn.delete(args) end ensure conn.close end end - @result == 0 + @result end # Delete an entry from the LDAP directory along with all subordinate entries. @@ -1250,7 +1250,7 @@ class Net::LDAP::Connection #:nodoc: (be = @conn.read_ber(Net::LDAP::AsnSyntax) and pdu = Net::LDAP::PDU.new(be)) or raise Net::LDAP::LdapError, "no bind result" - pdu.result_code + pdu end #-- @@ -1288,7 +1288,7 @@ class Net::LDAP::Connection #:nodoc: @conn.write request_pkt (be = @conn.read_ber(Net::LDAP::AsnSyntax) and pdu = Net::LDAP::PDU.new(be)) or raise Net::LDAP::LdapError, "no bind result" - return pdu.result_code unless pdu.result_code == 14 # saslBindInProgress + return pdu unless pdu.result_code == 14 # saslBindInProgress raise Net::LDAP::LdapError, "sasl-challenge overflow" if ((n += 1) > MaxSaslChallenges) cred = chall.call(pdu.result_server_sasl_creds) @@ -1374,7 +1374,7 @@ class Net::LDAP::Connection #:nodoc: # to do a root-DSE record search and not do a paged search if the LDAP # doesn't support it. Yuck. rfc2696_cookie = [126, ""] - result_code = 0 + result_pdu = nil n_results = 0 loop { @@ -1413,7 +1413,7 @@ class Net::LDAP::Connection #:nodoc: pkt = [next_msgid.to_ber, request, controls].to_ber_sequence @conn.write pkt - result_code = 0 + result_pdu = nil controls = [] while (be = @conn.read_ber(Net::LDAP::AsnSyntax)) && (pdu = Net::LDAP::PDU.new(be)) @@ -1430,7 +1430,7 @@ class Net::LDAP::Connection #:nodoc: end end when 5 # search-result - result_code = pdu.result_code + result_pdu = pdu controls = pdu.result_controls break else @@ -1449,7 +1449,7 @@ class Net::LDAP::Connection #:nodoc: # of type OCTET STRING, covered in the default syntax supported by # read_ber, so I guess we're ok. more_pages = false - if result_code == 0 and controls + if result_pdu.result_code == 0 and controls controls.each do |c| if c.oid == Net::LDAP::LDAPControls::PAGED_RESULTS # just in case some bogus server sends us more than 1 of these. @@ -1468,7 +1468,7 @@ class Net::LDAP::Connection #:nodoc: break unless more_pages } # loop - result_code + result_pdu || OpenStruct.new(:status => :failure, :result_code => 1, :message => "Invalid search") end MODIFY_OPERATIONS = { #:nodoc: @@ -1508,7 +1508,8 @@ class Net::LDAP::Connection #:nodoc: @conn.write pkt (be = @conn.read_ber(Net::LDAP::AsnSyntax)) && (pdu = Net::LDAP::PDU.new(be)) && (pdu.app_tag == 7) or raise Net::LDAP::LdapError, "response missing or invalid" - pdu.result_code + + pdu end #-- @@ -1529,8 +1530,12 @@ class Net::LDAP::Connection #:nodoc: pkt = [next_msgid.to_ber, request].to_ber_sequence @conn.write pkt - (be = @conn.read_ber(Net::LDAP::AsnSyntax)) && (pdu = Net::LDAP::PDU.new(be)) && (pdu.app_tag == 9) or raise Net::LDAP::LdapError, "response missing or invalid" - pdu.result_code + (be = @conn.read_ber(Net::LDAP::AsnSyntax)) && + (pdu = Net::LDAP::PDU.new(be)) && + (pdu.app_tag == 9) or + raise Net::LDAP::LdapError, "response missing or invalid" + + pdu end #-- @@ -1551,7 +1556,8 @@ class Net::LDAP::Connection #:nodoc: (be = @conn.read_ber(Net::LDAP::AsnSyntax)) && (pdu = Net::LDAP::PDU.new( be )) && (pdu.app_tag == 13) or raise Net::LDAP::LdapError.new( "response missing or invalid" ) - pdu.result_code + + pdu end #-- @@ -1565,6 +1571,7 @@ class Net::LDAP::Connection #:nodoc: @conn.write pkt (be = @conn.read_ber(Net::LDAP::AsnSyntax)) && (pdu = Net::LDAP::PDU.new(be)) && (pdu.app_tag == 11) or raise Net::LDAP::LdapError, "response missing or invalid" - pdu.result_code + + pdu end end # class Connection diff --git a/lib/net/ldap/pdu.rb b/lib/net/ldap/pdu.rb index bdde92c..37c3c08 100644 --- a/lib/net/ldap/pdu.rb +++ b/lib/net/ldap/pdu.rb @@ -112,6 +112,10 @@ class Net::LDAP::PDU @ldap_result || {} end + def error_message + result[:errorMessage] || "" + end + ## # This returns an LDAP result code taken from the PDU, but it will be nil # if there wasn't a result code. That can easily happen depending on the @@ -120,6 +124,10 @@ class Net::LDAP::PDU @ldap_result and @ldap_result[code] end + def status + result_code == 0 ? :success : :failure + end + ## # Return serverSaslCreds, which are only present in BindResponse packets. #-- diff --git a/spec/unit/ldap/search_spec.rb b/spec/unit/ldap/search_spec.rb index 61cb4fd..8f9446c 100644 --- a/spec/unit/ldap/search_spec.rb +++ b/spec/unit/ldap/search_spec.rb @@ -3,8 +3,7 @@ describe Net::LDAP, "search method" do class FakeConnection def search(args) - error_code = 1 - return error_code + OpenStruct.new(:result_code => 1, :message => "error") end end @@ -22,8 +21,8 @@ describe Net::LDAP, "search method" do context "when :return_result => false" do it "should return false upon error" do - success = @connection.search(:return_result => false) - success.should == false + result = @connection.search(:return_result => false) + result.result_code.should == 1 end end diff --git a/spec/unit/ldap_spec.rb b/spec/unit/ldap_spec.rb index 1edb5c9..0cf91ae 100644 --- a/spec/unit/ldap_spec.rb +++ b/spec/unit/ldap_spec.rb @@ -45,4 +45,34 @@ describe Net::LDAP::Connection do end end end -end \ No newline at end of file + + context "populate error messages" do + before do + @tcp_socket = flexmock(:connection) + @tcp_socket.should_receive(:write) + flexmock(TCPSocket).should_receive(:new).and_return(@tcp_socket) + end + + subject { Net::LDAP::Connection.new(:server => 'test.mocked.com', :port => 636) } + + it "should get back error messages if operation fails" do + ber = Net::BER::BerIdentifiedArray.new([53, "", "The provided password value was rejected by a password validator: The provided password did not contain enough characters from the character set 'ABCDEFGHIJKLMNOPQRSTUVWXYZ'. The minimum number of characters from that set that must be present in user passwords is 1"]) + ber.ber_identifier = 7 + @tcp_socket.should_receive(:read_ber).and_return([2, ber]) + + result = subject.modify(:dn => "1", :operations => [[:replace, "mail", "something@sothsdkf.com"]]) + result.status.should == :failure + result.error_message.should == "The provided password value was rejected by a password validator: The provided password did not contain enough characters from the character set 'ABCDEFGHIJKLMNOPQRSTUVWXYZ'. The minimum number of characters from that set that must be present in user passwords is 1" + end + + it "shouldn't get back error messages if operation succeeds" do + ber = Net::BER::BerIdentifiedArray.new([0, "", ""]) + ber.ber_identifier = 7 + @tcp_socket.should_receive(:read_ber).and_return([2, ber]) + + result = subject.modify(:dn => "1", :operations => [[:replace, "mail", "something@sothsdkf.com"]]) + result.status.should == :success + result.error_message.should == "" + end + end +end From 40f0e1857ee619250051d7d37f48a5c03424a29f Mon Sep 17 00:00:00 2001 From: Michael Baker Date: Wed, 30 Nov 2011 20:03:02 +0000 Subject: [PATCH 2/2] Add success and failure methods. --- lib/net/ldap/pdu.rb | 8 ++++++++ spec/unit/ldap_spec.rb | 16 ++++++++-------- 2 files changed, 16 insertions(+), 8 deletions(-) diff --git a/lib/net/ldap/pdu.rb b/lib/net/ldap/pdu.rb index 37c3c08..b771fab 100644 --- a/lib/net/ldap/pdu.rb +++ b/lib/net/ldap/pdu.rb @@ -128,6 +128,14 @@ class Net::LDAP::PDU result_code == 0 ? :success : :failure end + def success? + status == :success + end + + def failure? + !success? + end + ## # Return serverSaslCreds, which are only present in BindResponse packets. #-- diff --git a/spec/unit/ldap_spec.rb b/spec/unit/ldap_spec.rb index 0cf91ae..272d4ee 100644 --- a/spec/unit/ldap_spec.rb +++ b/spec/unit/ldap_spec.rb @@ -7,11 +7,11 @@ describe Net::LDAP::Connection do flexmock(TCPSocket). should_receive(:new).and_raise(Errno::ECONNREFUSED) end - + it "should raise LdapError" do lambda { Net::LDAP::Connection.new( - :server => 'test.mocked.com', + :server => 'test.mocked.com', :port => 636) }.should raise_error(Net::LDAP::LdapError) end @@ -21,11 +21,11 @@ describe Net::LDAP::Connection do flexmock(TCPSocket). should_receive(:new).and_raise(SocketError) end - + it "should raise LdapError" do lambda { Net::LDAP::Connection.new( - :server => 'test.mocked.com', + :server => 'test.mocked.com', :port => 636) }.should raise_error(Net::LDAP::LdapError) end @@ -35,11 +35,11 @@ describe Net::LDAP::Connection do flexmock(TCPSocket). should_receive(:new).and_raise(NameError) end - + it "should rethrow the exception" do lambda { Net::LDAP::Connection.new( - :server => 'test.mocked.com', + :server => 'test.mocked.com', :port => 636) }.should raise_error(NameError) end @@ -61,7 +61,7 @@ describe Net::LDAP::Connection do @tcp_socket.should_receive(:read_ber).and_return([2, ber]) result = subject.modify(:dn => "1", :operations => [[:replace, "mail", "something@sothsdkf.com"]]) - result.status.should == :failure + result.should be_failure result.error_message.should == "The provided password value was rejected by a password validator: The provided password did not contain enough characters from the character set 'ABCDEFGHIJKLMNOPQRSTUVWXYZ'. The minimum number of characters from that set that must be present in user passwords is 1" end @@ -71,7 +71,7 @@ describe Net::LDAP::Connection do @tcp_socket.should_receive(:read_ber).and_return([2, ber]) result = subject.modify(:dn => "1", :operations => [[:replace, "mail", "something@sothsdkf.com"]]) - result.status.should == :success + result.should be_success result.error_message.should == "" end end