diff options
author | Drew Blessing <drew@gitlab.com> | 2017-11-08 15:32:12 -0600 |
---|---|---|
committer | Drew Blessing <drew@gitlab.com> | 2018-01-04 17:10:40 -0600 |
commit | 7d1fdcdc836921d2f2324265752107519f47a6de (patch) | |
tree | 951e015c99c80817824106505f608d0dae78e5f1 /spec/lib/gitlab | |
parent | 2cbb2d0eceaed0f31c92d4eed8932e98f4f74559 (diff) | |
download | gitlab-ce-7d1fdcdc836921d2f2324265752107519f47a6de.tar.gz |
Modify `LDAP::Person` to return username value based on attributes
`Gitlab::LDAP::Person` did not respect the LDAP attributes username
configuration and would simply return the uid value. There are
cases where users would like to specify a different username field
to allow more friendly GitLab usernames. For example, it's common
in AD to have sAMAccountName be an employee ID like `A12345` while
the local part of the email address is more human-friendly.
Diffstat (limited to 'spec/lib/gitlab')
-rw-r--r-- | spec/lib/gitlab/ldap/adapter_spec.rb | 10 | ||||
-rw-r--r-- | spec/lib/gitlab/ldap/person_spec.rb | 73 | ||||
-rw-r--r-- | spec/lib/gitlab/o_auth/user_spec.rb | 20 |
3 files changed, 98 insertions, 5 deletions
diff --git a/spec/lib/gitlab/ldap/adapter_spec.rb b/spec/lib/gitlab/ldap/adapter_spec.rb index d9ddb4326be..6132abd9b35 100644 --- a/spec/lib/gitlab/ldap/adapter_spec.rb +++ b/spec/lib/gitlab/ldap/adapter_spec.rb @@ -16,7 +16,7 @@ describe Gitlab::LDAP::Adapter do expect(adapter).to receive(:ldap_search) do |arg| expect(arg[:filter].to_s).to eq('(uid=johndoe)') expect(arg[:base]).to eq('dc=example,dc=com') - expect(arg[:attributes]).to match(%w{dn uid cn mail email userPrincipalName}) + expect(arg[:attributes]).to match(ldap_attributes) end.and_return({}) adapter.users('uid', 'johndoe') @@ -26,7 +26,7 @@ describe Gitlab::LDAP::Adapter do expect(adapter).to receive(:ldap_search).with( base: 'uid=johndoe,ou=users,dc=example,dc=com', scope: Net::LDAP::SearchScope_BaseObject, - attributes: %w{dn uid cn mail email userPrincipalName}, + attributes: ldap_attributes, filter: nil ).and_return({}) @@ -63,7 +63,7 @@ describe Gitlab::LDAP::Adapter do it 'uses the right uid attribute when non-default' do stub_ldap_config(uid: 'sAMAccountName') expect(adapter).to receive(:ldap_search).with( - hash_including(attributes: %w{dn sAMAccountName cn mail email userPrincipalName}) + hash_including(attributes: ldap_attributes) ).and_return({}) adapter.users('sAMAccountName', 'johndoe') @@ -137,4 +137,8 @@ describe Gitlab::LDAP::Adapter do end end end + + def ldap_attributes + Gitlab::LDAP::Person.ldap_attributes(Gitlab::LDAP::Config.new('ldapmain')) + end end diff --git a/spec/lib/gitlab/ldap/person_spec.rb b/spec/lib/gitlab/ldap/person_spec.rb index d204050ef66..ff29d9aa5be 100644 --- a/spec/lib/gitlab/ldap/person_spec.rb +++ b/spec/lib/gitlab/ldap/person_spec.rb @@ -8,13 +8,16 @@ describe Gitlab::LDAP::Person do before do stub_ldap_config( options: { + 'uid' => 'uid', 'attributes' => { - 'name' => 'cn', - 'email' => %w(mail email userPrincipalName) + 'name' => 'cn', + 'email' => %w(mail email userPrincipalName), + 'username' => username_attribute } } ) end + let(:username_attribute) { %w(uid sAMAccountName userid) } describe '.normalize_dn' do subject { described_class.normalize_dn(given) } @@ -44,6 +47,34 @@ describe Gitlab::LDAP::Person do end end + describe '.ldap_attributes' do + it 'returns a compact and unique array' do + stub_ldap_config( + options: { + 'uid' => nil, + 'attributes' => { + 'name' => 'cn', + 'email' => 'mail', + 'username' => %w(uid mail memberof) + } + } + ) + config = Gitlab::LDAP::Config.new('ldapmain') + ldap_attributes = described_class.ldap_attributes(config) + + expect(ldap_attributes).to match_array(%w(dn uid cn mail memberof)) + end + end + + describe '.validate_entry' do + it 'raises InvalidEntryError' do + entry['foo'] = 'bar' + + expect { described_class.new(entry, 'ldapmain') } + .to raise_error(Gitlab::LDAP::Person::InvalidEntryError) + end + end + describe '#name' do it 'uses the configured name attribute and handles values as an array' do name = 'John Doe' @@ -72,6 +103,44 @@ describe Gitlab::LDAP::Person do end end + describe '#username' do + context 'with default uid username attribute' do + let(:username_attribute) { 'uid' } + + it 'returns the proper username value' do + attr_value = 'johndoe' + entry[username_attribute] = attr_value + person = described_class.new(entry, 'ldapmain') + + expect(person.username).to eq(attr_value) + end + end + + context 'with a different username attribute' do + let(:username_attribute) { 'sAMAccountName' } + + it 'returns the proper username value' do + attr_value = 'johndoe' + entry[username_attribute] = attr_value + person = described_class.new(entry, 'ldapmain') + + expect(person.username).to eq(attr_value) + end + end + + context 'with a non-standard username attribute' do + let(:username_attribute) { 'mail' } + + it 'returns the proper username value' do + attr_value = 'john.doe@example.com' + entry[username_attribute] = attr_value + person = described_class.new(entry, 'ldapmain') + + expect(person.username).to eq(attr_value) + end + end + end + def assert_generic_test(test_description, got, expected) test_failure_message = "Failed test description: '#{test_description}'\n\n expected: #{expected}\n got: #{got}" expect(got).to eq(expected), test_failure_message diff --git a/spec/lib/gitlab/o_auth/user_spec.rb b/spec/lib/gitlab/o_auth/user_spec.rb index 6334bcd0156..45fff4c5787 100644 --- a/spec/lib/gitlab/o_auth/user_spec.rb +++ b/spec/lib/gitlab/o_auth/user_spec.rb @@ -275,6 +275,26 @@ describe Gitlab::OAuth::User do end end + context 'and a corresponding LDAP person with a non-default username' do + before do + allow(ldap_user).to receive(:uid) { uid } + allow(ldap_user).to receive(:username) { 'johndoe@example.com' } + allow(ldap_user).to receive(:email) { %w(johndoe@example.com john2@example.com) } + allow(ldap_user).to receive(:dn) { dn } + end + + context 'and no account for the LDAP user' do + it 'creates a user favoring the LDAP username and strips email domain' do + allow(Gitlab::LDAP::Person).to receive(:find_by_uid).and_return(ldap_user) + + oauth_user.save + + expect(gl_user).to be_valid + expect(gl_user.username).to eql 'johndoe' + end + end + end + context "and no corresponding LDAP person" do before do allow(Gitlab::LDAP::Person).to receive(:find_by_uid).and_return(nil) |