From 8c29a04549d4a956508ef9a92a043a309978fa34 Mon Sep 17 00:00:00 2001 From: Michael Kozono Date: Thu, 5 Oct 2017 03:47:48 -0700 Subject: Leave bad DNs alone instead of raising errors --- lib/gitlab/ldap/auth_hash.rb | 2 +- lib/gitlab/ldap/person.rb | 11 ++++++++-- spec/lib/gitlab/ldap/dn_spec.rb | 36 +------------------------------- spec/lib/gitlab/ldap/person_spec.rb | 14 +++++++++++++ spec/support/ldap_shared_examples.rb | 40 ++++++++++++++++++++++++++++++++++++ 5 files changed, 65 insertions(+), 38 deletions(-) diff --git a/lib/gitlab/ldap/auth_hash.rb b/lib/gitlab/ldap/auth_hash.rb index b173b879f5f..3123da17fd9 100644 --- a/lib/gitlab/ldap/auth_hash.rb +++ b/lib/gitlab/ldap/auth_hash.rb @@ -4,7 +4,7 @@ module Gitlab module LDAP class AuthHash < Gitlab::OAuth::AuthHash def uid - Gitlab::LDAP::DN.new(super).to_normalized_s + Gitlab::LDAP::Person.normalize_dn(super) end private diff --git a/lib/gitlab/ldap/person.rb b/lib/gitlab/ldap/person.rb index 81aa352e656..38d7a9ba2f5 100644 --- a/lib/gitlab/ldap/person.rb +++ b/lib/gitlab/ldap/person.rb @@ -36,6 +36,14 @@ module Gitlab ] end + def self.normalize_dn(dn) + ::Gitlab::LDAP::DN.new(dn).to_normalized_s + rescue ::Gitlab::LDAP::DN::FormatError => e + Rails.logger.info("Returning original DN \"#{dn}\" due to error during normalization attempt: #{e.message}") + + dn + end + # Returns the UID in a normalized form. # # 1. Excess spaces are stripped @@ -44,7 +52,6 @@ module Gitlab ::Gitlab::LDAP::DN.normalize_value(uid) rescue ::Gitlab::LDAP::DN::FormatError => e Rails.logger.info("Returning original UID \"#{uid}\" due to error during normalization attempt: #{e.message}") - Rails.logger.info(e.backtrace.join("\n")) uid end @@ -72,7 +79,7 @@ module Gitlab end def dn - DN.new(entry.dn).to_normalized_s + self.class.normalize_dn(entry.dn) end private diff --git a/spec/lib/gitlab/ldap/dn_spec.rb b/spec/lib/gitlab/ldap/dn_spec.rb index c300f7160fe..8e21ecdf9ab 100644 --- a/spec/lib/gitlab/ldap/dn_spec.rb +++ b/spec/lib/gitlab/ldap/dn_spec.rb @@ -78,41 +78,7 @@ describe Gitlab::LDAP::DN do describe '#to_normalized_s' do subject { described_class.new(given).to_normalized_s } - where(:test_description, :given, :expected) do - 'strips extraneous whitespace' | 'uid =John Smith , ou = People, dc= example,dc =com' | 'uid=john smith,ou=people,dc=example,dc=com' - 'strips extraneous whitespace for a DN with a single RDN' | 'uid = John Smith' | 'uid=john smith' - 'unescapes non-reserved, non-special Unicode characters' | 'uid = Sebasti\\c3\\a1n\\ C.\\20Smith, ou=People (aka. \\22humans\\") ,dc=example, dc=com' | 'uid=sebastián c. smith,ou=people (aka. \\"humans\\"),dc=example,dc=com' - 'downcases the whole string' | 'UID=John Smith,ou=People,dc=example,dc=com' | 'uid=john smith,ou=people,dc=example,dc=com' - 'for a null DN (empty string), returns empty string and does not error' | '' | '' - 'does not strip an escaped leading space in an attribute value' | 'uid=\\ John Smith,ou=People,dc=example,dc=com' | 'uid=\\ john smith,ou=people,dc=example,dc=com' - 'does not strip an escaped leading space in the last attribute value' | 'uid=\\ John Smith' | 'uid=\\ john smith' - 'does not strip an escaped trailing space in an attribute value' | 'uid=John Smith\\ ,ou=People,dc=example,dc=com' | 'uid=john smith\\ ,ou=people,dc=example,dc=com' - 'strips extraneous spaces after an escaped trailing space' | 'uid=John Smith\\ ,ou=People,dc=example,dc=com' | 'uid=john smith\\ ,ou=people,dc=example,dc=com' - 'strips extraneous spaces after an escaped trailing space at the end of the DN' | 'uid=John Smith,ou=People,dc=example,dc=com\\ ' | 'uid=john smith,ou=people,dc=example,dc=com\\ ' - 'properly preserves escaped trailing space after unescaped trailing spaces' | 'uid=John Smith \\ ,ou=People,dc=example,dc=com' | 'uid=john smith \\ ,ou=people,dc=example,dc=com' - 'preserves multiple inner spaces in an attribute value' | 'uid=John Smith,ou=People,dc=example,dc=com' | 'uid=john smith,ou=people,dc=example,dc=com' - 'preserves inner spaces after an escaped space' | 'uid=John\\ Smith,ou=People,dc=example,dc=com' | 'uid=john smith,ou=people,dc=example,dc=com' - 'hex-escapes an escaped leading newline in an attribute value' | "uid=\\\nJohn Smith,ou=People,dc=example,dc=com" | "uid=\\0ajohn smith,ou=people,dc=example,dc=com" - 'hex-escapes and does not strip an escaped trailing newline in an attribute value' | "uid=John Smith\\\n,ou=People,dc=example,dc=com" | "uid=john smith\\0a,ou=people,dc=example,dc=com" - 'hex-escapes an unescaped leading newline (actually an invalid DN?)' | "uid=\nJohn Smith,ou=People,dc=example,dc=com" | "uid=\\0ajohn smith,ou=people,dc=example,dc=com" - 'strips an unescaped trailing newline (actually an invalid DN?)' | "uid=John Smith\n,ou=People,dc=example,dc=com" | "uid=john smith,ou=people,dc=example,dc=com" - 'does not strip if no extraneous whitespace' | 'uid=John Smith,ou=People,dc=example,dc=com' | 'uid=john smith,ou=people,dc=example,dc=com' - 'does not modify an escaped equal sign in an attribute value' | 'uid= foo \\= bar' | 'uid=foo \\= bar' - 'converts an escaped hex equal sign to an escaped equal sign in an attribute value' | 'uid= foo \\3D bar' | 'uid=foo \\= bar' - 'does not modify an escaped comma in an attribute value' | 'uid= John C. Smith, ou=San Francisco\\, CA' | 'uid=john c. smith,ou=san francisco\\, ca' - 'converts an escaped hex comma to an escaped comma in an attribute value' | 'uid= John C. Smith, ou=San Francisco\\2C CA' | 'uid=john c. smith,ou=san francisco\\, ca' - 'does not modify an escaped hex carriage return character in an attribute value' | 'uid= John C. Smith, ou=San Francisco\\,\\0DCA' | 'uid=john c. smith,ou=san francisco\\,\\0dca' - 'does not modify an escaped hex line feed character in an attribute value' | 'uid= John C. Smith, ou=San Francisco\\,\\0ACA' | 'uid=john c. smith,ou=san francisco\\,\\0aca' - 'does not modify an escaped hex CRLF in an attribute value' | 'uid= John C. Smith, ou=San Francisco\\,\\0D\\0ACA' | 'uid=john c. smith,ou=san francisco\\,\\0d\\0aca' - 'allows attribute type name OIDs' | '0.9.2342.19200300.100.1.25=Example,0.9.2342.19200300.100.1.25=Com' | '0.9.2342.19200300.100.1.25=example,0.9.2342.19200300.100.1.25=com' - 'strips extraneous whitespace from attribute type name OIDs' | '0.9.2342.19200300.100.1.25 = Example, 0.9.2342.19200300.100.1.25 = Com' | '0.9.2342.19200300.100.1.25=example,0.9.2342.19200300.100.1.25=com' - end - - with_them do - it 'normalizes the DN' do - assert_generic_test(test_description, subject, expected) - end - end + it_behaves_like 'normalizes a DN' context 'when we do not support the given DN format' do context 'multivalued RDNs' do diff --git a/spec/lib/gitlab/ldap/person_spec.rb b/spec/lib/gitlab/ldap/person_spec.rb index 743b3fbde2b..d204050ef66 100644 --- a/spec/lib/gitlab/ldap/person_spec.rb +++ b/spec/lib/gitlab/ldap/person_spec.rb @@ -16,6 +16,20 @@ describe Gitlab::LDAP::Person do ) end + describe '.normalize_dn' do + subject { described_class.normalize_dn(given) } + + it_behaves_like 'normalizes a DN' + + context 'with an exception during normalization' do + let(:given) { 'John "Smith,' } # just something that will cause an exception + + it 'returns the given DN unmodified' do + expect(subject).to eq(given) + end + end + end + describe '.normalize_uid' do subject { described_class.normalize_uid(given) } diff --git a/spec/support/ldap_shared_examples.rb b/spec/support/ldap_shared_examples.rb index 3ab8b1d73a1..52c34e78965 100644 --- a/spec/support/ldap_shared_examples.rb +++ b/spec/support/ldap_shared_examples.rb @@ -1,3 +1,43 @@ +shared_examples_for 'normalizes a DN' do + using RSpec::Parameterized::TableSyntax + + where(:test_description, :given, :expected) do + 'strips extraneous whitespace' | 'uid =John Smith , ou = People, dc= example,dc =com' | 'uid=john smith,ou=people,dc=example,dc=com' + 'strips extraneous whitespace for a DN with a single RDN' | 'uid = John Smith' | 'uid=john smith' + 'unescapes non-reserved, non-special Unicode characters' | 'uid = Sebasti\\c3\\a1n\\ C.\\20Smith, ou=People (aka. \\22humans\\") ,dc=example, dc=com' | 'uid=sebastián c. smith,ou=people (aka. \\"humans\\"),dc=example,dc=com' + 'downcases the whole string' | 'UID=John Smith,ou=People,dc=example,dc=com' | 'uid=john smith,ou=people,dc=example,dc=com' + 'for a null DN (empty string), returns empty string and does not error' | '' | '' + 'does not strip an escaped leading space in an attribute value' | 'uid=\\ John Smith,ou=People,dc=example,dc=com' | 'uid=\\ john smith,ou=people,dc=example,dc=com' + 'does not strip an escaped leading space in the last attribute value' | 'uid=\\ John Smith' | 'uid=\\ john smith' + 'does not strip an escaped trailing space in an attribute value' | 'uid=John Smith\\ ,ou=People,dc=example,dc=com' | 'uid=john smith\\ ,ou=people,dc=example,dc=com' + 'strips extraneous spaces after an escaped trailing space' | 'uid=John Smith\\ ,ou=People,dc=example,dc=com' | 'uid=john smith\\ ,ou=people,dc=example,dc=com' + 'strips extraneous spaces after an escaped trailing space at the end of the DN' | 'uid=John Smith,ou=People,dc=example,dc=com\\ ' | 'uid=john smith,ou=people,dc=example,dc=com\\ ' + 'properly preserves escaped trailing space after unescaped trailing spaces' | 'uid=John Smith \\ ,ou=People,dc=example,dc=com' | 'uid=john smith \\ ,ou=people,dc=example,dc=com' + 'preserves multiple inner spaces in an attribute value' | 'uid=John Smith,ou=People,dc=example,dc=com' | 'uid=john smith,ou=people,dc=example,dc=com' + 'preserves inner spaces after an escaped space' | 'uid=John\\ Smith,ou=People,dc=example,dc=com' | 'uid=john smith,ou=people,dc=example,dc=com' + 'hex-escapes an escaped leading newline in an attribute value' | "uid=\\\nJohn Smith,ou=People,dc=example,dc=com" | "uid=\\0ajohn smith,ou=people,dc=example,dc=com" + 'hex-escapes and does not strip an escaped trailing newline in an attribute value' | "uid=John Smith\\\n,ou=People,dc=example,dc=com" | "uid=john smith\\0a,ou=people,dc=example,dc=com" + 'hex-escapes an unescaped leading newline (actually an invalid DN?)' | "uid=\nJohn Smith,ou=People,dc=example,dc=com" | "uid=\\0ajohn smith,ou=people,dc=example,dc=com" + 'strips an unescaped trailing newline (actually an invalid DN?)' | "uid=John Smith\n,ou=People,dc=example,dc=com" | "uid=john smith,ou=people,dc=example,dc=com" + 'does not strip if no extraneous whitespace' | 'uid=John Smith,ou=People,dc=example,dc=com' | 'uid=john smith,ou=people,dc=example,dc=com' + 'does not modify an escaped equal sign in an attribute value' | 'uid= foo \\= bar' | 'uid=foo \\= bar' + 'converts an escaped hex equal sign to an escaped equal sign in an attribute value' | 'uid= foo \\3D bar' | 'uid=foo \\= bar' + 'does not modify an escaped comma in an attribute value' | 'uid= John C. Smith, ou=San Francisco\\, CA' | 'uid=john c. smith,ou=san francisco\\, ca' + 'converts an escaped hex comma to an escaped comma in an attribute value' | 'uid= John C. Smith, ou=San Francisco\\2C CA' | 'uid=john c. smith,ou=san francisco\\, ca' + 'does not modify an escaped hex carriage return character in an attribute value' | 'uid= John C. Smith, ou=San Francisco\\,\\0DCA' | 'uid=john c. smith,ou=san francisco\\,\\0dca' + 'does not modify an escaped hex line feed character in an attribute value' | 'uid= John C. Smith, ou=San Francisco\\,\\0ACA' | 'uid=john c. smith,ou=san francisco\\,\\0aca' + 'does not modify an escaped hex CRLF in an attribute value' | 'uid= John C. Smith, ou=San Francisco\\,\\0D\\0ACA' | 'uid=john c. smith,ou=san francisco\\,\\0d\\0aca' + 'allows attribute type name OIDs' | '0.9.2342.19200300.100.1.25=Example,0.9.2342.19200300.100.1.25=Com' | '0.9.2342.19200300.100.1.25=example,0.9.2342.19200300.100.1.25=com' + 'strips extraneous whitespace from attribute type name OIDs' | '0.9.2342.19200300.100.1.25 = Example, 0.9.2342.19200300.100.1.25 = Com' | '0.9.2342.19200300.100.1.25=example,0.9.2342.19200300.100.1.25=com' + end + + with_them do + it 'normalizes the DN' do + assert_generic_test(test_description, subject, expected) + end + end +end + shared_examples_for 'normalizes a DN attribute value' do using RSpec::Parameterized::TableSyntax -- cgit v1.2.1