Commit aefc96ca authored by Michael Kozono's avatar Michael Kozono

Rely on LDAP providers giving DNs, not UIDs

parent 010cd3de
...@@ -4,7 +4,7 @@ module Gitlab ...@@ -4,7 +4,7 @@ module Gitlab
module LDAP module LDAP
class AuthHash < Gitlab::OAuth::AuthHash class AuthHash < Gitlab::OAuth::AuthHash
def uid def uid
Gitlab::LDAP::Person.normalize_uid_or_dn(super) Gitlab::LDAP::Person.normalize_dn(super)
end end
private private
......
...@@ -36,28 +36,6 @@ module Gitlab ...@@ -36,28 +36,6 @@ module Gitlab
] ]
end end
# Returns the UID or DN in a normalized form
def self.normalize_uid_or_dn(uid_or_dn)
if dn?(uid_or_dn)
normalize_dn(uid_or_dn)
else
normalize_uid(uid_or_dn)
end
rescue StandardError => e
Rails.logger.info("Returning original DN \"#{uid_or_dn}\" due to error during normalization attempt: #{e.message}")
Rails.logger.info(e.backtrace.join("\n"))
uid_or_dn
end
# Returns true if the string looks like a DN rather than a UID.
#
# An empty string is technically a valid DN (null DN), although we should
# never need to worry about that.
def self.dn?(uid_or_dn)
uid_or_dn.blank? || !!uid_or_dn.match(/(?<!\\)=/)
end
# Returns the UID in a normalized form. # Returns the UID in a normalized form.
# #
# 1. Excess spaces are stripped # 1. Excess spaces are stripped
......
...@@ -77,21 +77,6 @@ describe Gitlab::LDAP::Person do ...@@ -77,21 +77,6 @@ describe Gitlab::LDAP::Person do
end end
end end
describe '.normalize_uid_or_dn' do
subject { described_class.normalize_uid_or_dn(given) }
it_behaves_like 'normalizes the DN'
it_behaves_like 'normalizes the UID'
context 'with an exception during normalization' do
let(:given) { described_class } # just something that will cause an exception
it 'returns the given object unmodified' do
expect(subject).to eq(given)
end
end
end
describe '.normalize_uid' do describe '.normalize_uid' do
subject { described_class.normalize_uid(given) } subject { described_class.normalize_uid(given) }
...@@ -120,26 +105,6 @@ describe Gitlab::LDAP::Person do ...@@ -120,26 +105,6 @@ describe Gitlab::LDAP::Person do
end end
end end
describe '.dn?' do
where(:test_description, :given, :expected) do
'given a DN with a single RDN' | 'uid=John C. Smith' | true
'given a DN with multiple RDNs' | 'uid=John C. Smith,ou=People,dc=example,dc=com' | true
'given a DN with a single RDN with excess spaces' | ' uid=John C. Smith ' | true
'given a DN with multiple RDNs with excess spaces' | ' uid=John C. Smith,ou=People,dc=example,dc=com ' | true
'given a DN with an escaped equal sign' | 'uid=John C. Smith,ou=People\\=' | true
'given a DN with an equal sign in escaped hex' | 'uid=John C. Smith,ou=People\\3D' | true
'given a UID' | 'John C. Smith' | false
'given a UID with excess spaces' | ' John C. Smith ' | false
'given a UID with an escaped equal sign' | 'John C. \\= Smith' | false
end
with_them do
it 'returns the expected boolean' do
assert_generic_test(test_description, described_class.dn?(given), expected)
end
end
end
describe '#name' do describe '#name' do
it 'uses the configured name attribute and handles values as an array' do it 'uses the configured name attribute and handles values as an array' do
name = 'John Doe' name = 'John Doe'
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment