From 81a6fc3d97687b3122f357529729218f9a8019c5 Mon Sep 17 00:00:00 2001 From: Ryan Lue Date: Fri, 31 Aug 2018 17:27:57 +0200 Subject: [PATCH] Stop converting binary user attributes to text during LDAP setup (fixes #2140) --- lib/ldap/user.rb | 56 ++++++++---------- spec/lib/ldap/user_spec.rb | 48 +++++++++++++++ spec/support/tcr.rb | 5 ++ .../skips_binary_attributes_2140_.marshal | Bin 0 -> 4141 bytes 4 files changed, 79 insertions(+), 30 deletions(-) create mode 100644 spec/support/tcr.rb create mode 100644 test/data/tcr_cassettes/lib/ldap/user/skips_binary_attributes_2140_.marshal diff --git a/lib/ldap/user.rb b/lib/ldap/user.rb index 4b228fb27..0d39bb00a 100644 --- a/lib/ldap/user.rb +++ b/lib/ldap/user.rb @@ -120,38 +120,34 @@ class Ldap # #=> {:dn=>"dn (e. g. CN=Administrator,CN=Users,DC=domain,DC=tld)", ...} # # @return [Hash{Symbol=>String}] The available User attributes as key and the name and an example as value. - def attributes(filter: nil, base_dn: nil) - - filter ||= filter() - - attributes = {} - known_attributes = BLACKLISTED.dup - lookup_counter = 1 - - @ldap.search(filter, base: base_dn) do |entry| - new_attributes = entry.attribute_names - known_attributes - - if new_attributes.blank? - lookup_counter += 1 - # check max 50 entries with - # the same attributes in a row - break if lookup_counter == 50 - next - end - - new_attributes.each do |attribute| - value = entry[attribute] - next if value.blank? - next if value[0].blank? - - example_value = value[0].force_encoding('UTF-8').utf8_encode(fallback: :read_as_sanitized_binary) - attributes[attribute] = "#{attribute} (e. g. #{example_value})" - end - - known_attributes.concat(new_attributes) + def attributes(custom_filter: nil, base_dn: nil) + @attributes ||= begin + attributes = {}.with_indifferent_access lookup_counter = 0 + + # collect sample attributes + @ldap.search(custom_filter || filter, base: base_dn) do |entry| + pre_merge_count = attributes.count + + attributes.reverse_merge!(entry.to_h + .except(*BLACKLISTED) + .transform_values(&:first) + .compact) + + # check max 50 entries with the same attributes in a row + lookup_counter = (pre_merge_count < attributes.count ? 0 : lookup_counter.next) + break if lookup_counter >= 50 + end + + # format sample values for presentation + attributes.each do |name, value| + attributes[name] = if value.encoding == Encoding.find('ascii-8bit') + "#{name} (binary data)" + else + "#{name} (e.g., #{value.utf8_encode})" + end + end end - attributes end # The active filter of the instance. If none give on initialization an automatic lookup is performed. diff --git a/spec/lib/ldap/user_spec.rb b/spec/lib/ldap/user_spec.rb index 8bf5657ec..ad567050e 100644 --- a/spec/lib/ldap/user_spec.rb +++ b/spec/lib/ldap/user_spec.rb @@ -2,6 +2,7 @@ require 'rails_helper' # rails autoloading issue require 'ldap' require 'ldap/user' +require 'tcr/net/ldap' RSpec.describe Ldap::User do @@ -195,4 +196,51 @@ RSpec.describe Ldap::User do end end end + + # Each of these test cases depends on + # sample TCP transmission data recorded with TCR, + # stored in test/data/tcr_cassettes. + context 'on mocked LDAP connections' do + around do |example| + cassette_name = example.description.gsub(/[^0-9A-Za-z.\-]+/, '_') + + begin + original_tcr_format = TCR.configuration.format + TCR.configuration.format = 'marshal' + TCR.use_cassette("lib/ldap/user/#{cassette_name}") { example.run } + ensure + TCR.configuration.format = original_tcr_format + end + end + + describe '#attributes' do + let(:subject) { described_class.new(config, ldap: ldap) } + let(:ldap) { Ldap.new(config) } + let(:config) do + { 'host_url' => 'ldap://localhost', + 'options' => { 'dc=example,dc=org' => 'dc=example,dc=org' }, + 'option' => 'dc=example,dc=org', + 'base_dn' => 'dc=example,dc=org', + 'bind_user' => 'cn=admin,dc=example,dc=org', + 'bind_pw' => 'admin' }.with_indifferent_access + end + + # see https://github.com/zammad/zammad/issues/2140 + # + # This method grabs sample values of user attributes on the LDAP server. + # It used to coerce ALL values to Unicode strings, including binary attributes + # (e.g., usersmimecertificate / msexchmailboxsecuritydescriptor), + # which led to valid Unicode gibberish (e.g., "\u0001\u0001\u0004...") + # + # When saving these values to the database, + # ActiveRecord::Store would convert them to binary (ASCII-8BIT) strings, + # which would then break #to_json with an Encoding::UndefinedConversion error. + it 'skips binary attributes (#2140)' do + Setting.set('ldap_config', subject.attributes) + + expect { Setting.get('ldap_config').to_json } + .not_to raise_error + end + end + end end diff --git a/spec/support/tcr.rb b/spec/support/tcr.rb new file mode 100644 index 000000000..b744d365e --- /dev/null +++ b/spec/support/tcr.rb @@ -0,0 +1,5 @@ +TCR.configure do |config| + config.cassette_library_dir = 'test/data/tcr_cassettes' + config.hook_tcp_ports = [389] # LDAP + config.format = 'yaml' +end diff --git a/test/data/tcr_cassettes/lib/ldap/user/skips_binary_attributes_2140_.marshal b/test/data/tcr_cassettes/lib/ldap/user/skips_binary_attributes_2140_.marshal new file mode 100644 index 0000000000000000000000000000000000000000..6bcc3fd825c99b95dd88e5a606ffbb6bb8410cb3 GIT binary patch literal 4141 zcmeHJ-EZ4e6t|s|wE3!5no_obG)mbEt$VMXWF1qHvNUwrnl`E{K6n^I*0wx$>bbD&HX9Aw@tBd-8+ysCnV!j9xH2T8 zq+{3MwE-n}>M_eTZ3`9$v=5o?H62t;3`l#1-uQ?)9l@0WRm*651y-)=mg&}Af2Jo^ zqV4C;NzeTR>A8d{Fxt|0Adt57HFIC(3}dexIBmZT$pqC^AjUTS+2QnNszZ zN@P_|%5)1>rgIcnp(v6@c@&eA3Q@T#>O-v0pt7RC>-g>(rfMqHl~?I)u8wpiEpwNu znoMq?vLdUnFjS|itc;C)=BYyJ)0T&R%-s&7qseqyRtQmVO^+G_pT$ri+P6gIEz}twJKJy@Vco*jR_k+W`DTr#>!%SgAyO-IZ5ZcwWOfkHI4T0{AMF)jzN3ajf7MQu`6S>-YydcMuH3`A{}gb(B&{zv;22XnDk4gw0Ce`j z4@)o#LI6#dInNecy{?zbcGL0@hS>KjKQ6-E7b3=Mw8dXwt!WzDkL;?oZa24!%1mLS1hX@z_tmt~G;J~Yd^$LM{yhp_WBAqc_^eE7AF z4`LC&f|0Rr5Z)9zEGdEil27h<`Beu@J$S$Bfd5MeoWE=T