Normalize values, reusing DN normalization code

I first attempted to extract logic from the code that normalizes DNs, but I was unsuccessful. This is a hack but it works.

2 files changed, 74 insertions, 28 deletions

diff --git a/spec/lib/gitlab/ldap/dn_spec.rb b/spec/lib/gitlab/ldap/dn_spec.rb
index 725a1324109..709eadd7e38 100644
--- a/spec/lib/gitlab/ldap/dn_spec.rb
+++ b/spec/lib/gitlab/ldap/dn_spec.rb
@@ -3,6 +3,78 @@ require 'spec_helper'
 describe Gitlab::LDAP::DN do
   using RSpec::Parameterized::TableSyntax
 
+  describe '#normalize_value' do
+    subject { described_class.normalize_value(given) }
+
+    it_behaves_like 'normalizes a DN attribute value'
+
+    context 'when the given DN is malformed' do
+      context 'when ending with a comma' do
+        let(:given) { 'John Smith,' }
+
+        it 'raises MalformedDnError' do
+          expect { subject }.to raise_error(Gitlab::LDAP::MalformedDnError, 'DN string ended unexpectedly')
+        end
+      end
+
+      context 'when given a BER encoded attribute value with a space in it' do
+        let(:given) { '#aa aa' }
+
+        it 'raises MalformedDnError' do
+          expect { subject }.to raise_error(Gitlab::LDAP::MalformedDnError, "Expected the end of an attribute value, but got \"a\"")
+        end
+      end
+
+      context 'when given a BER encoded attribute value with a non-hex character in it' do
+        let(:given) { '#aaXaaa' }
+
+        it 'raises MalformedDnError' do
+          expect { subject }.to raise_error(Gitlab::LDAP::MalformedDnError, "Expected the first character of a hex pair, but got \"X\"")
+        end
+      end
+
+      context 'when given a BER encoded attribute value with a non-hex character in it' do
+        let(:given) { '#aaaYaa' }
+
+        it 'raises MalformedDnError' do
+          expect { subject }.to raise_error(Gitlab::LDAP::MalformedDnError, "Expected the second character of a hex pair, but got \"Y\"")
+        end
+      end
+
+      context 'when given a hex pair with a non-hex character in it, inside double quotes' do
+        let(:given) { '"Sebasti\\cX\\a1n"' }
+
+        it 'raises MalformedDnError' do
+          expect { subject }.to raise_error(Gitlab::LDAP::MalformedDnError, "Expected the second character of a hex pair inside a double quoted value, but got \"X\"")
+        end
+      end
+
+      context 'with an open (as opposed to closed) double quote' do
+        let(:given) { '"James' }
+
+        it 'raises MalformedDnError' do
+          expect { subject }.to raise_error(Gitlab::LDAP::MalformedDnError, 'DN string ended unexpectedly')
+        end
+      end
+
+      context 'with an invalid escaped hex code' do
+        let(:given) { 'J\ames' }
+
+        it 'raises MalformedDnError' do
+          expect { subject }.to raise_error(Gitlab::LDAP::MalformedDnError, 'Invalid escaped hex code "\am"')
+        end
+      end
+
+      context 'with a value ending with the escape character' do
+        let(:given) { 'foo\\' }
+
+        it 'raises MalformedDnError' do
+          expect { subject }.to raise_error(Gitlab::LDAP::MalformedDnError, 'DN string ended unexpectedly')
+        end
+      end
+    end
+  end
+
   describe '#to_normalized_s' do
     subject { described_class.new(given).to_normalized_s }
 
diff --git a/spec/lib/gitlab/ldap/person_spec.rb b/spec/lib/gitlab/ldap/person_spec.rb
index 02904f1e351..743b3fbde2b 100644
--- a/spec/lib/gitlab/ldap/person_spec.rb
+++ b/spec/lib/gitlab/ldap/person_spec.rb
@@ -1,7 +1,6 @@
 require 'spec_helper'
 
 describe Gitlab::LDAP::Person do
-  using RSpec::Parameterized::TableSyntax
   include LdapHelpers
 
   let(:entry) { ldap_user_entry('john.doe') }
@@ -17,38 +16,13 @@ describe Gitlab::LDAP::Person do
     )
   end
 
-  shared_examples_for 'normalizes the UID' do
-    where(:test_description, :given, :expected) do
-      'strips extraneous whitespace'                                        | ' John C. Smith   '                     | 'john c. smith'
-      'strips extraneous whitespace without changing escaped characters'    | '  Sebasti\\c3\\a1n\\ C.\\20Smith\\   ' | 'sebasti\\c3\\a1n\\ c.\\20smith\\ '
-      'downcases the whole string'                                          | 'John Smith'                            | 'john smith'
-      'does not strip the escaped leading space in an attribute value'      | '   \\ John Smith '                     | '\\ john smith'
-      'does not strip the escaped trailing space in an attribute value'     | '    John Smith\\   '                   | 'john smith\\ '
-      'does not strip the escaped leading newline in an attribute value'    | '     \\\nJohn Smith  '                 | '\\\njohn smith'
-      'does not strip the escaped trailing newline in an attribute value'   | '    John Smith\\\n  '                  | 'john smith\\\n'
-      'does not strip the unescaped leading newline in an attribute value'  | '   \nJohn Smith '                      | '\njohn smith'
-      'does not strip the unescaped trailing newline in an attribute value' | '  John Smith\n   '                     | 'john smith\n'
-      'does not strip non whitespace'                                       | 'John Smith'                            | 'john smith'
-      'does not treat escaped equal signs as attribute delimiters'          | ' foo  \\=  bar'                        | 'foo  \\=  bar'
-      'does not treat escaped hex equal signs as attribute delimiters'      | ' foo  \\3D  bar'                       | 'foo  \\3d  bar'
-      'does not treat escaped commas as attribute delimiters'               | ' Smith\\, John C.'                     | 'smith\\, john c.'
-      'does not treat escaped hex commas as attribute delimiters'           | ' Smith\\2C John C.'                    | 'smith\\2c john c.'
-    end
-
-    with_them do
-      it 'normalizes the UID' do
-        assert_generic_test(test_description, subject, expected)
-      end
-    end
-  end
-
   describe '.normalize_uid' do
     subject { described_class.normalize_uid(given) }
 
-    it_behaves_like 'normalizes the UID'
+    it_behaves_like 'normalizes a DN attribute value'
 
     context 'with an exception during normalization' do
-      let(:given) { described_class } # just something that will cause an exception
+      let(:given) { 'John "Smith,' } # just something that will cause an exception
 
       it 'returns the given UID unmodified' do
         expect(subject).to eq(given)