Merge pull request #131 from ruby-ldap/instrument-messages

Parse Net::LDAP::PDU when calling Net::LDAP::Connection#read, instrument PDU parsing

Author: mtodd

lib/net/ldap/connection.rb

@@ -89,9 +89,7 @@ def setup_encryption(args)
     when :start_tls
       request = [Net::LDAP::StartTlsOid.to_ber_contextspecific(0)].to_ber_appsequence(Net::LDAP::PDU::ExtendedRequest)
       write(request)
-      be = read
-      raise Net::LDAP::LdapError, "no start_tls result" if be.nil?
-      pdu = Net::LDAP::PDU.new(be)
+      pdu = read
       raise Net::LDAP::LdapError, "no start_tls result" if pdu.nil?
       if pdu.result_code.zero?
         @conn = self.class.wrap_with_ssl(@conn)
@@ -117,13 +115,25 @@ def close
   #
   # - syntax: the BER syntax to use to parse the read data with
   #
-  # Returns basic BER objects.
+  # Returns parsed Net::LDAP::PDU object.
   def read(syntax = Net::LDAP::AsnSyntax)
-    instrument "read.net_ldap_connection", :syntax => syntax do |payload|
-      @conn.read_ber(syntax) do |id, content_length|
-        payload[:object_type_id] = id
-        payload[:content_length] = content_length
+    ber_object =
+      instrument "read.net_ldap_connection", :syntax => syntax do |payload|
+        @conn.read_ber(syntax) do |id, content_length|
+          payload[:object_type_id] = id
+          payload[:content_length] = content_length
+        end
       end
+
+    return unless ber_object
+
+    instrument "parse_pdu.net_ldap_connection" do |payload|
+      pdu = payload[:pdu]  = Net::LDAP::PDU.new(ber_object)
+
+      payload[:message_id] = pdu.message_id
+      payload[:app_tag]    = pdu.app_tag
+
+      pdu
     end
   end
   private :read
@@ -181,7 +191,8 @@ def bind_simple(auth)
       psw.to_ber_contextspecific(0)].to_ber_appsequence(0)
     write(request)
 
-    (be = read and pdu = Net::LDAP::PDU.new(be)) or raise Net::LDAP::LdapError, "no bind result"
+    pdu = read
+    raise Net::LDAP::LdapError, "no bind result" unless pdu
 
     pdu
   end
@@ -218,7 +229,9 @@ def bind_sasl(auth)
       request = [LdapVersion.to_ber, "".to_ber, sasl].to_ber_appsequence(0)
       write(request)
 
-      (be = read and pdu = Net::LDAP::PDU.new(be)) or raise Net::LDAP::LdapError, "no bind result"
+      pdu = read
+      raise Net::LDAP::LdapError, "no bind result" unless pdu
+
       return pdu unless pdu.result_code == 14 # saslBindInProgress
       raise Net::LDAP::LdapError, "sasl-challenge overflow" if ((n += 1) > MaxSaslChallenges)
 
@@ -395,7 +408,7 @@ def search(args = {})
         result_pdu = nil
         controls = []
 
-        while (be = read) && (pdu = Net::LDAP::PDU.new(be))
+        while pdu = read
           case pdu.app_tag
           when Net::LDAP::PDU::SearchReturnedData
             n_results += 1
@@ -500,7 +513,11 @@ def modify(args)
       ops.to_ber_sequence ].to_ber_appsequence(6)
     write(request)
 
-    (be = read) && (pdu = Net::LDAP::PDU.new(be)) && (pdu.app_tag == Net::LDAP::PDU::ModifyResponse) or raise Net::LDAP::LdapError, "response missing or invalid"
+    pdu = read
+
+    if !pdu || pdu.app_tag != Net::LDAP::PDU::ModifyResponse
+      raise Net::LDAP::LdapError, "response missing or invalid"
+    end
 
     pdu
   end
@@ -522,10 +539,11 @@ def add(args)
     request = [add_dn.to_ber, add_attrs.to_ber_sequence].to_ber_appsequence(8)
     write(request)
 
-    (be = read) &&
-      (pdu = Net::LDAP::PDU.new(be)) &&
-      (pdu.app_tag == Net::LDAP::PDU::AddResponse) or
+    pdu = read
+
+    if !pdu || pdu.app_tag != Net::LDAP::PDU::AddResponse
       raise Net::LDAP::LdapError, "response missing or invalid"
+    end
 
     pdu
   end
@@ -544,9 +562,11 @@ def rename(args)
 
     write(request.to_ber_appsequence(12))
 
-    (be = read) &&
-    (pdu = Net::LDAP::PDU.new( be )) && (pdu.app_tag == Net::LDAP::PDU::ModifyRDNResponse) or
-    raise Net::LDAP::LdapError.new( "response missing or invalid" )
+    pdu = read
+
+    if !pdu || pdu.app_tag != Net::LDAP::PDU::ModifyRDNResponse
+      raise Net::LDAP::LdapError.new "response missing or invalid"
+    end
 
     pdu
   end
@@ -560,7 +580,11 @@ def delete(args)
     request = dn.to_s.to_ber_application_string(10)
     write(request, controls)
 
-    (be = read) && (pdu = Net::LDAP::PDU.new(be)) && (pdu.app_tag == Net::LDAP::PDU::DeleteResponse) or raise Net::LDAP::LdapError, "response missing or invalid"
+    pdu = read
+
+    if !pdu || pdu.app_tag != Net::LDAP::PDU::DeleteResponse
+      raise Net::LDAP::LdapError, "response missing or invalid"
+    end
 
     pdu
   end

test/integration/test_add.rb

@@ -0,0 +1,28 @@
+require_relative '../test_helper'
+
+class TestAddIntegration < LDAPIntegrationTestCase
+  def setup
+    super
+    @ldap.authenticate "cn=admin,dc=rubyldap,dc=com", "passworD1"
+
+    @dn = "uid=added-user1,ou=People,dc=rubyldap,dc=com"
+  end
+
+  def test_add
+    attrs = {
+      objectclass: %w(top inetOrgPerson organizationalPerson person),
+      uid:  "added-user1",
+      cn:   "added-user1",
+      sn:   "added-user1",
+      mail: "added-user1@rubyldap.com"
+    }
+
+    assert @ldap.add(dn: @dn, attributes: attrs), @ldap.get_operation_result.inspect
+
+    assert result = @ldap.search(base: @dn, scope: Net::LDAP::SearchScope_BaseObject).first
+  end
+
+  def teardown
+    @ldap.delete dn: @dn
+  end
+end

test/integration/test_delete.rb

@@ -0,0 +1,29 @@
+require_relative '../test_helper'
+
+class TestDeleteIntegration < LDAPIntegrationTestCase
+  def setup
+    super
+    @ldap.authenticate "cn=admin,dc=rubyldap,dc=com", "passworD1"
+
+    @dn = "uid=delete-user1,ou=People,dc=rubyldap,dc=com"
+
+    attrs = {
+      objectclass: %w(top inetOrgPerson organizationalPerson person),
+      uid:  "delete-user1",
+      cn:   "delete-user1",
+      sn:   "delete-user1",
+      mail: "delete-user1@rubyldap.com"
+    }
+    assert @ldap.add(dn: @dn, attributes: attrs), @ldap.get_operation_result.inspect
+    assert @ldap.search(base: @dn, scope: Net::LDAP::SearchScope_BaseObject)
+  end
+
+  def test_delete
+    assert @ldap.delete(dn: @dn), @ldap.get_operation_result.inspect
+    refute @ldap.search(base: @dn, scope: Net::LDAP::SearchScope_BaseObject)
+
+    result = @ldap.get_operation_result
+    assert_equal 32, result.code
+    assert_equal Net::LDAP::ResultStrings[32], result.message
+  end
+end

test/test_ldap_connection.rb

@@ -144,6 +144,28 @@ def test_read_net_ldap_connection_event
     assert_equal read_result, result
   end
 
+  def test_parse_pdu_net_ldap_connection_event
+    ber = Net::BER::BerIdentifiedArray.new([0, "", ""])
+    ber.ber_identifier = Net::LDAP::PDU::BindResult
+    read_result = [2, ber]
+    @tcp_socket.should_receive(:read_ber).and_return(read_result)
+
+    events = @service.subscribe "parse_pdu.net_ldap_connection"
+
+    result = @connection.bind(method: :anon)
+    assert result.success?, "should be success"
+
+    # a parse_pdu event
+    payload, result = events.pop
+    assert payload.has_key?(:pdu)
+    assert payload.has_key?(:app_tag)
+    assert payload.has_key?(:message_id)
+    assert_equal Net::LDAP::PDU::BindResult, payload[:app_tag]
+    assert_equal 2, payload[:message_id]
+    pdu = payload[:pdu]
+    assert_equal 0, pdu.result_code
+  end
+
   def test_bind_net_ldap_connection_event
     ber = Net::BER::BerIdentifiedArray.new([0, "", ""])
     ber.ber_identifier = Net::LDAP::PDU::BindResult