From 2763040162d6ec432234c21d4c6a837c559c3ac2 Mon Sep 17 00:00:00 2001 From: Michael Baker Date: Tue, 29 Nov 2011 22:01:34 +0000 Subject: [PATCH] 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