diff options
author | Horatiu Eugen Vlad <horatiu@vlad.eu> | 2018-02-23 15:10:39 +0300 |
---|---|---|
committer | Horatiu Eugen Vlad <horatiu@vlad.eu> | 2018-02-28 18:53:02 +0300 |
commit | 1ad5df49b1925f1865e99c3fd8576a762aea9cae (patch) | |
tree | b9cee2aabea4c4584883245ada7e8e91e1a01295 /spec/lib/gitlab/auth/ldap | |
parent | 77097c9196da7c43d1102249da1d40446176f803 (diff) |
Moved o_auth/saml/ldap modules under gitlab/auth
Diffstat (limited to 'spec/lib/gitlab/auth/ldap')
-rw-r--r-- | spec/lib/gitlab/auth/ldap/access_spec.rb | 166 | ||||
-rw-r--r-- | spec/lib/gitlab/auth/ldap/adapter_spec.rb | 144 | ||||
-rw-r--r-- | spec/lib/gitlab/auth/ldap/auth_hash_spec.rb | 110 | ||||
-rw-r--r-- | spec/lib/gitlab/auth/ldap/authentication_spec.rb | 58 | ||||
-rw-r--r-- | spec/lib/gitlab/auth/ldap/config_spec.rb | 373 | ||||
-rw-r--r-- | spec/lib/gitlab/auth/ldap/dn_spec.rb | 224 | ||||
-rw-r--r-- | spec/lib/gitlab/auth/ldap/person_spec.rb | 160 | ||||
-rw-r--r-- | spec/lib/gitlab/auth/ldap/user_spec.rb | 241 |
8 files changed, 1476 insertions, 0 deletions
diff --git a/spec/lib/gitlab/auth/ldap/access_spec.rb b/spec/lib/gitlab/auth/ldap/access_spec.rb new file mode 100644 index 00000000000..9b3916bf9e3 --- /dev/null +++ b/spec/lib/gitlab/auth/ldap/access_spec.rb @@ -0,0 +1,166 @@ +require 'spec_helper' + +describe Gitlab::Auth::LDAP::Access do + let(:access) { described_class.new user } + let(:user) { create(:omniauth_user) } + + describe '.allowed?' do + it 'updates the users `last_credential_check_at' do + expect(access).to receive(:allowed?) { true } + expect(described_class).to receive(:open).and_yield(access) + + expect { described_class.allowed?(user) } + .to change { user.last_credential_check_at } + end + end + + describe '#allowed?' do + subject { access.allowed? } + + context 'when the user cannot be found' do + before do + allow(Gitlab::Auth::LDAP::Person).to receive(:find_by_dn).and_return(nil) + end + + it { is_expected.to be_falsey } + + it 'blocks user in GitLab' do + expect(access).to receive(:block_user).with(user, 'does not exist anymore') + + access.allowed? + end + end + + context 'when the user is found' do + before do + allow(Gitlab::Auth::LDAP::Person).to receive(:find_by_dn).and_return(:ldap_user) + end + + context 'and the user is disabled via active directory' do + before do + allow(Gitlab::Auth::LDAP::Person).to receive(:disabled_via_active_directory?).and_return(true) + end + + it { is_expected.to be_falsey } + + it 'blocks user in GitLab' do + expect(access).to receive(:block_user).with(user, 'is disabled in Active Directory') + + access.allowed? + end + end + + context 'and has no disabled flag in active diretory' do + before do + allow(Gitlab::Auth::LDAP::Person).to receive(:disabled_via_active_directory?).and_return(false) + end + + it { is_expected.to be_truthy } + + context 'when auto-created users are blocked' do + before do + user.block + end + + it 'does not unblock user in GitLab' do + expect(access).not_to receive(:unblock_user) + + access.allowed? + + expect(user).to be_blocked + expect(user).not_to be_ldap_blocked # this block is handled by omniauth not by our internal logic + end + end + + context 'when auto-created users are not blocked' do + before do + user.ldap_block + end + + it 'unblocks user in GitLab' do + expect(access).to receive(:unblock_user).with(user, 'is not disabled anymore') + + access.allowed? + end + end + end + + context 'without ActiveDirectory enabled' do + before do + allow(Gitlab::Auth::LDAP::Config).to receive(:enabled?).and_return(true) + allow_any_instance_of(Gitlab::Auth::LDAP::Config).to receive(:active_directory).and_return(false) + end + + it { is_expected.to be_truthy } + + context 'when user cannot be found' do + before do + allow(Gitlab::Auth::LDAP::Person).to receive(:find_by_dn).and_return(nil) + end + + it { is_expected.to be_falsey } + + it 'blocks user in GitLab' do + expect(access).to receive(:block_user).with(user, 'does not exist anymore') + + access.allowed? + end + end + + context 'when user was previously ldap_blocked' do + before do + user.ldap_block + end + + it 'unblocks the user if it exists' do + expect(access).to receive(:unblock_user).with(user, 'is available again') + + access.allowed? + end + end + end + end + end + + describe '#block_user' do + before do + user.activate + allow(Gitlab::AppLogger).to receive(:info) + + access.block_user user, 'reason' + end + + it 'blocks the user' do + expect(user).to be_blocked + expect(user).to be_ldap_blocked + end + + it 'logs the reason' do + expect(Gitlab::AppLogger).to have_received(:info).with( + "LDAP account \"123456\" reason, " \ + "blocking Gitlab user \"#{user.name}\" (#{user.email})" + ) + end + end + + describe '#unblock_user' do + before do + user.ldap_block + allow(Gitlab::AppLogger).to receive(:info) + + access.unblock_user user, 'reason' + end + + it 'activates the user' do + expect(user).not_to be_blocked + expect(user).not_to be_ldap_blocked + end + + it 'logs the reason' do + Gitlab::AppLogger.info( + "LDAP account \"123456\" reason, " \ + "unblocking Gitlab user \"#{user.name}\" (#{user.email})" + ) + end + end +end diff --git a/spec/lib/gitlab/auth/ldap/adapter_spec.rb b/spec/lib/gitlab/auth/ldap/adapter_spec.rb new file mode 100644 index 00000000000..10c60d792bd --- /dev/null +++ b/spec/lib/gitlab/auth/ldap/adapter_spec.rb @@ -0,0 +1,144 @@ +require 'spec_helper' + +describe Gitlab::Auth::LDAP::Adapter do + include LdapHelpers + + let(:ldap) { double(:ldap) } + let(:adapter) { ldap_adapter('ldapmain', ldap) } + + describe '#users' do + before do + stub_ldap_config(base: 'dc=example,dc=com') + end + + it 'searches with the proper options when searching by uid' do + # Requires this expectation style to match the filter + 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(ldap_attributes) + end.and_return({}) + + adapter.users('uid', 'johndoe') + end + + it 'searches with the proper options when searching by dn' do + expect(adapter).to receive(:ldap_search).with( + base: 'uid=johndoe,ou=users,dc=example,dc=com', + scope: Net::LDAP::SearchScope_BaseObject, + attributes: ldap_attributes, + filter: nil + ).and_return({}) + + adapter.users('dn', 'uid=johndoe,ou=users,dc=example,dc=com') + end + + it 'searches with the proper options when searching with a limit' do + expect(adapter) + .to receive(:ldap_search).with(hash_including(size: 100)).and_return({}) + + adapter.users('uid', 'johndoe', 100) + end + + it 'returns an LDAP::Person if search returns a result' do + entry = ldap_user_entry('johndoe') + allow(adapter).to receive(:ldap_search).and_return([entry]) + + results = adapter.users('uid', 'johndoe') + + expect(results.size).to eq(1) + expect(results.first.uid).to eq('johndoe') + end + + it 'returns empty array if search entry does not respond to uid' do + entry = Net::LDAP::Entry.new + entry['dn'] = user_dn('johndoe') + allow(adapter).to receive(:ldap_search).and_return([entry]) + + results = adapter.users('uid', 'johndoe') + + expect(results).to be_empty + end + + 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: ldap_attributes) + ).and_return({}) + + adapter.users('sAMAccountName', 'johndoe') + end + end + + describe '#dn_matches_filter?' do + subject { adapter.dn_matches_filter?(:dn, :filter) } + + context "when the search result is non-empty" do + before do + allow(adapter).to receive(:ldap_search).and_return([:foo]) + end + + it { is_expected.to be_truthy } + end + + context "when the search result is empty" do + before do + allow(adapter).to receive(:ldap_search).and_return([]) + end + + it { is_expected.to be_falsey } + end + end + + describe '#ldap_search' do + subject { adapter.ldap_search(base: :dn, filter: :filter) } + + context "when the search is successful" do + context "and the result is non-empty" do + before do + allow(ldap).to receive(:search).and_return([:foo]) + end + + it { is_expected.to eq [:foo] } + end + + context "and the result is empty" do + before do + allow(ldap).to receive(:search).and_return([]) + end + + it { is_expected.to eq [] } + end + end + + context "when the search encounters an error" do + before do + allow(ldap).to receive_messages( + search: nil, + get_operation_result: double(code: 1, message: 'some error') + ) + end + + it { is_expected.to eq [] } + end + + context "when the search raises an LDAP exception" do + before do + allow(ldap).to receive(:search) { raise Net::LDAP::Error, "some error" } + allow(Rails.logger).to receive(:warn) + end + + it { is_expected.to eq [] } + + it 'logs the error' do + subject + expect(Rails.logger).to have_received(:warn).with( + "LDAP search raised exception Net::LDAP::Error: some error") + end + end + end + + def ldap_attributes + Gitlab::Auth::LDAP::Person.ldap_attributes(Gitlab::Auth::LDAP::Config.new('ldapmain')) + end +end diff --git a/spec/lib/gitlab/auth/ldap/auth_hash_spec.rb b/spec/lib/gitlab/auth/ldap/auth_hash_spec.rb new file mode 100644 index 00000000000..05541972f87 --- /dev/null +++ b/spec/lib/gitlab/auth/ldap/auth_hash_spec.rb @@ -0,0 +1,110 @@ +require 'spec_helper' + +describe Gitlab::Auth::LDAP::AuthHash do + include LdapHelpers + + let(:auth_hash) do + described_class.new( + OmniAuth::AuthHash.new( + uid: given_uid, + provider: 'ldapmain', + info: info, + extra: { + raw_info: raw_info + } + ) + ) + end + + let(:info) do + { + name: 'Smith, J.', + email: 'johnsmith@example.com', + nickname: '123456' + } + end + + let(:raw_info) do + { + uid: ['123456'], + email: ['johnsmith@example.com'], + cn: ['Smith, J.'], + fullName: ['John Smith'] + } + end + + context "without overridden attributes" do + let(:given_uid) { 'uid=John Smith,ou=People,dc=example,dc=com' } + + it "has the correct username" do + expect(auth_hash.username).to eq("123456") + end + + it "has the correct name" do + expect(auth_hash.name).to eq("Smith, J.") + end + end + + context "with overridden attributes" do + let(:given_uid) { 'uid=John Smith,ou=People,dc=example,dc=com' } + + let(:attributes) do + { + 'username' => %w(mail email), + 'name' => 'fullName' + } + end + + before do + allow_any_instance_of(Gitlab::Auth::LDAP::Config).to receive(:attributes).and_return(attributes) + end + + it "has the correct username" do + expect(auth_hash.username).to eq("johnsmith@example.com") + end + + it "has the correct name" do + expect(auth_hash.name).to eq("John Smith") + end + end + + describe '#uid' do + context 'when there is extraneous (but valid) whitespace' do + let(:given_uid) { 'uid =john smith , ou = people, dc= example,dc =com' } + + it 'removes the extraneous whitespace' do + expect(auth_hash.uid).to eq('uid=john smith,ou=people,dc=example,dc=com') + end + end + + context 'when there are upper case characters' do + let(:given_uid) { 'UID=John Smith,ou=People,dc=example,dc=com' } + + it 'downcases' do + expect(auth_hash.uid).to eq('uid=john smith,ou=people,dc=example,dc=com') + end + end + end + + describe '#username' do + context 'if lowercase_usernames setting is' do + let(:given_uid) { 'uid=John Smith,ou=People,dc=example,dc=com' } + + before do + raw_info[:uid] = ['JOHN'] + end + + it 'enabled the username attribute is lower cased' do + stub_ldap_config(lowercase_usernames: true) + + expect(auth_hash.username).to eq 'john' + end + + it 'disabled the username attribute is not lower cased' do + stub_ldap_config(lowercase_usernames: false) + + expect(auth_hash.username).to eq 'JOHN' + end + end + end +end diff --git a/spec/lib/gitlab/auth/ldap/authentication_spec.rb b/spec/lib/gitlab/auth/ldap/authentication_spec.rb new file mode 100644 index 00000000000..111572d043b --- /dev/null +++ b/spec/lib/gitlab/auth/ldap/authentication_spec.rb @@ -0,0 +1,58 @@ +require 'spec_helper' + +describe Gitlab::Auth::LDAP::Authentication do + let(:dn) { 'uid=John Smith, ou=People, dc=example, dc=com' } + let(:user) { create(:omniauth_user, extern_uid: Gitlab::Auth::LDAP::Person.normalize_dn(dn)) } + let(:login) { 'john' } + let(:password) { 'password' } + + describe 'login' do + before do + allow(Gitlab::Auth::LDAP::Config).to receive(:enabled?).and_return(true) + end + + it "finds the user if authentication is successful" do + expect(user).not_to be_nil + + # try only to fake the LDAP call + adapter = double('adapter', dn: dn).as_null_object + allow_any_instance_of(described_class) + .to receive(:adapter).and_return(adapter) + + expect(described_class.login(login, password)).to be_truthy + end + + it "is false if the user does not exist" do + # try only to fake the LDAP call + adapter = double('adapter', dn: dn).as_null_object + allow_any_instance_of(described_class) + .to receive(:adapter).and_return(adapter) + + expect(described_class.login(login, password)).to be_falsey + end + + it "is false if authentication fails" do + expect(user).not_to be_nil + + # try only to fake the LDAP call + adapter = double('adapter', bind_as: nil).as_null_object + allow_any_instance_of(described_class) + .to receive(:adapter).and_return(adapter) + + expect(described_class.login(login, password)).to be_falsey + end + + it "fails if ldap is disabled" do + allow(Gitlab::Auth::LDAP::Config).to receive(:enabled?).and_return(false) + expect(described_class.login(login, password)).to be_falsey + end + + it "fails if no login is supplied" do + expect(described_class.login('', password)).to be_falsey + end + + it "fails if no password is supplied" do + expect(described_class.login(login, '')).to be_falsey + end + end +end diff --git a/spec/lib/gitlab/auth/ldap/config_spec.rb b/spec/lib/gitlab/auth/ldap/config_spec.rb new file mode 100644 index 00000000000..82587e2ba55 --- /dev/null +++ b/spec/lib/gitlab/auth/ldap/config_spec.rb @@ -0,0 +1,373 @@ +require 'spec_helper' + +describe Gitlab::Auth::LDAP::Config do + include LdapHelpers + + let(:config) { described_class.new('ldapmain') } + + describe '.servers' do + it 'returns empty array if no server information is available' do + allow(Gitlab.config).to receive(:ldap).and_return('enabled' => false) + + expect(described_class.servers).to eq [] + end + end + + describe '#initialize' do + it 'requires a provider' do + expect { described_class.new }.to raise_error ArgumentError + end + + it 'works' do + expect(config).to be_a described_class + end + + it 'raises an error if a unknown provider is used' do + expect { described_class.new 'unknown' }.to raise_error(RuntimeError) + end + end + + describe '#adapter_options' do + it 'constructs basic options' do + stub_ldap_config( + options: { + 'host' => 'ldap.example.com', + 'port' => 386, + 'encryption' => 'plain' + } + ) + + expect(config.adapter_options).to eq( + host: 'ldap.example.com', + port: 386, + encryption: nil + ) + end + + it 'includes authentication options when auth is configured' do + stub_ldap_config( + options: { + 'host' => 'ldap.example.com', + 'port' => 686, + 'encryption' => 'simple_tls', + 'verify_certificates' => true, + 'bind_dn' => 'uid=admin,dc=example,dc=com', + 'password' => 'super_secret' + } + ) + + expect(config.adapter_options).to include({ + auth: { + method: :simple, + username: 'uid=admin,dc=example,dc=com', + password: 'super_secret' + } + }) + end + + it 'sets encryption method to simple_tls when configured as simple_tls' do + stub_ldap_config( + options: { + 'host' => 'ldap.example.com', + 'port' => 686, + 'encryption' => 'simple_tls' + } + ) + + expect(config.adapter_options[:encryption]).to include({ method: :simple_tls }) + end + + it 'sets encryption method to start_tls when configured as start_tls' do + stub_ldap_config( + options: { + 'host' => 'ldap.example.com', + 'port' => 686, + 'encryption' => 'start_tls' + } + ) + + expect(config.adapter_options[:encryption]).to include({ method: :start_tls }) + end + + context 'when verify_certificates is enabled' do + it 'sets tls_options to OpenSSL defaults' do + stub_ldap_config( + options: { + 'host' => 'ldap.example.com', + 'port' => 686, + 'encryption' => 'simple_tls', + 'verify_certificates' => true + } + ) + + expect(config.adapter_options[:encryption]).to include({ tls_options: OpenSSL::SSL::SSLContext::DEFAULT_PARAMS }) + end + end + + context 'when verify_certificates is disabled' do + it 'sets verify_mode to OpenSSL VERIFY_NONE' do + stub_ldap_config( + options: { + 'host' => 'ldap.example.com', + 'port' => 686, + 'encryption' => 'simple_tls', + 'verify_certificates' => false + } + ) + + expect(config.adapter_options[:encryption]).to include({ + tls_options: { + verify_mode: OpenSSL::SSL::VERIFY_NONE + } + }) + end + end + + context 'when ca_file is specified' do + it 'passes it through in tls_options' do + stub_ldap_config( + options: { + 'host' => 'ldap.example.com', + 'port' => 686, + 'encryption' => 'simple_tls', + 'ca_file' => '/etc/ca.pem' + } + ) + + expect(config.adapter_options[:encryption][:tls_options]).to include({ ca_file: '/etc/ca.pem' }) + end + end + + context 'when ca_file is a blank string' do + it 'does not add the ca_file key to tls_options' do + stub_ldap_config( + options: { + 'host' => 'ldap.example.com', + 'port' => 686, + 'encryption' => 'simple_tls', + 'ca_file' => ' ' + } + ) + + expect(config.adapter_options[:encryption][:tls_options]).not_to have_key(:ca_file) + end + end + + context 'when ssl_version is specified' do + it 'passes it through in tls_options' do + stub_ldap_config( + options: { + 'host' => 'ldap.example.com', + 'port' => 686, + 'encryption' => 'simple_tls', + 'ssl_version' => 'TLSv1_2' + } + ) + + expect(config.adapter_options[:encryption][:tls_options]).to include({ ssl_version: 'TLSv1_2' }) + end + end + + context 'when ssl_version is a blank string' do + it 'does not add the ssl_version key to tls_options' do + stub_ldap_config( + options: { + 'host' => 'ldap.example.com', + 'port' => 686, + 'encryption' => 'simple_tls', + 'ssl_version' => ' ' + } + ) + + expect(config.adapter_options[:encryption][:tls_options]).not_to have_key(:ssl_version) + end + end + end + + describe '#omniauth_options' do + it 'constructs basic options' do + stub_ldap_config( + options: { + 'host' => 'ldap.example.com', + 'port' => 386, + 'base' => 'ou=users,dc=example,dc=com', + 'encryption' => 'plain', + 'uid' => 'uid' + } + ) + + expect(config.omniauth_options).to include( + host: 'ldap.example.com', + port: 386, + base: 'ou=users,dc=example,dc=com', + encryption: 'plain', + filter: '(uid=%{username})' + ) + expect(config.omniauth_options.keys).not_to include(:bind_dn, :password) + end + + it 'includes authentication options when auth is configured' do + stub_ldap_config( + options: { + 'uid' => 'sAMAccountName', + 'user_filter' => '(memberOf=cn=group1,ou=groups,dc=example,dc=com)', + 'bind_dn' => 'uid=admin,dc=example,dc=com', + 'password' => 'super_secret' + } + ) + + expect(config.omniauth_options).to include( + filter: '(&(sAMAccountName=%{username})(memberOf=cn=group1,ou=groups,dc=example,dc=com))', + bind_dn: 'uid=admin,dc=example,dc=com', + password: 'super_secret' + ) + end + + context 'when verify_certificates is enabled' do + it 'specifies disable_verify_certificates as false' do + stub_ldap_config( + options: { + 'host' => 'ldap.example.com', + 'port' => 686, + 'encryption' => 'simple_tls', + 'verify_certificates' => true + } + ) + + expect(config.omniauth_options).to include({ disable_verify_certificates: false }) + end + end + + context 'when verify_certificates is disabled' do + it 'specifies disable_verify_certificates as true' do + stub_ldap_config( + options: { + 'host' => 'ldap.example.com', + 'port' => 686, + 'encryption' => 'simple_tls', + 'verify_certificates' => false + } + ) + + expect(config.omniauth_options).to include({ disable_verify_certificates: true }) + end + end + + context 'when ca_file is present' do + it 'passes it through' do + stub_ldap_config( + options: { + 'host' => 'ldap.example.com', + 'port' => 686, + 'encryption' => 'simple_tls', + 'verify_certificates' => true, + 'ca_file' => '/etc/ca.pem' + } + ) + + expect(config.omniauth_options).to include({ ca_file: '/etc/ca.pem' }) + end + end + + context 'when ca_file is blank' do + it 'does not include the ca_file option' do + stub_ldap_config( + options: { + 'host' => 'ldap.example.com', + 'port' => 686, + 'encryption' => 'simple_tls', + 'verify_certificates' => true, + 'ca_file' => ' ' + } + ) + + expect(config.omniauth_options).not_to have_key(:ca_file) + end + end + + context 'when ssl_version is present' do + it 'passes it through' do + stub_ldap_config( + options: { + 'host' => 'ldap.example.com', + 'port' => 686, + 'encryption' => 'simple_tls', + 'verify_certificates' => true, + 'ssl_version' => 'TLSv1_2' + } + ) + + expect(config.omniauth_options).to include({ ssl_version: 'TLSv1_2' }) + end + end + + context 'when ssl_version is blank' do + it 'does not include the ssl_version option' do + stub_ldap_config( + options: { + 'host' => 'ldap.example.com', + 'port' => 686, + 'encryption' => 'simple_tls', + 'verify_certificates' => true, + 'ssl_version' => ' ' + } + ) + + expect(config.omniauth_options).not_to have_key(:ssl_version) + end + end + end + + describe '#has_auth?' do + it 'is true when password is set' do + stub_ldap_config( + options: { + 'bind_dn' => 'uid=admin,dc=example,dc=com', + 'password' => 'super_secret' + } + ) + + expect(config.has_auth?).to be_truthy + end + + it 'is true when bind_dn is set and password is empty' do + stub_ldap_config( + options: { + 'bind_dn' => 'uid=admin,dc=example,dc=com', + 'password' => '' + } + ) + + expect(config.has_auth?).to be_truthy + end + + it 'is false when password and bind_dn are not set' do + stub_ldap_config(options: { 'bind_dn' => nil, 'password' => nil }) + + expect(config.has_auth?).to be_falsey + end + end + + describe '#attributes' do + it 'uses default attributes when no custom attributes are configured' do + expect(config.attributes).to eq(config.default_attributes) + end + + it 'merges the configuration attributes with default attributes' do + stub_ldap_config( + options: { + 'attributes' => { + 'username' => %w(sAMAccountName), + 'email' => %w(userPrincipalName) + } + } + ) + + expect(config.attributes).to include({ + 'username' => %w(sAMAccountName), + 'email' => %w(userPrincipalName), + 'name' => 'cn' + }) + end + end +end diff --git a/spec/lib/gitlab/auth/ldap/dn_spec.rb b/spec/lib/gitlab/auth/ldap/dn_spec.rb new file mode 100644 index 00000000000..f2983a02602 --- /dev/null +++ b/spec/lib/gitlab/auth/ldap/dn_spec.rb @@ -0,0 +1,224 @@ +require 'spec_helper' + +describe Gitlab::Auth::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 MalformedError' do + expect { subject }.to raise_error(Gitlab::Auth::LDAP::DN::MalformedError, '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 MalformedError' do + expect { subject }.to raise_error(Gitlab::Auth::LDAP::DN::MalformedError, "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 MalformedError' do + expect { subject }.to raise_error(Gitlab::Auth::LDAP::DN::MalformedError, "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 MalformedError' do + expect { subject }.to raise_error(Gitlab::Auth::LDAP::DN::MalformedError, "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 MalformedError' do + expect { subject }.to raise_error(Gitlab::Auth::LDAP::DN::MalformedError, "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 MalformedError' do + expect { subject }.to raise_error(Gitlab::Auth::LDAP::DN::MalformedError, 'DN string ended unexpectedly') + end + end + + context 'with an invalid escaped hex code' do + let(:given) { 'J\ames' } + + it 'raises MalformedError' do + expect { subject }.to raise_error(Gitlab::Auth::LDAP::DN::MalformedError, 'Invalid escaped hex code "\am"') + end + end + + context 'with a value ending with the escape character' do + let(:given) { 'foo\\' } + + it 'raises MalformedError' do + expect { subject }.to raise_error(Gitlab::Auth::LDAP::DN::MalformedError, 'DN string ended unexpectedly') + end + end + end + end + + describe '#to_normalized_s' do + subject { described_class.new(given).to_normalized_s } + + it_behaves_like 'normalizes a DN' + + context 'when we do not support the given DN format' do + context 'multivalued RDNs' do + context 'without extraneous whitespace' do + let(:given) { 'uid=john smith+telephonenumber=+1 555-555-5555,ou=people,dc=example,dc=com' } + + it 'raises UnsupportedError' do + expect { subject }.to raise_error(Gitlab::Auth::LDAP::DN::UnsupportedError) + end + end + + context 'with extraneous whitespace' do + context 'around the phone number plus sign' do + let(:given) { 'uid = John Smith + telephoneNumber = + 1 555-555-5555 , ou = People,dc=example,dc=com' } + + it 'raises UnsupportedError' do + expect { subject }.to raise_error(Gitlab::Auth::LDAP::DN::UnsupportedError) + end + end + + context 'not around the phone number plus sign' do + let(:given) { 'uid = John Smith + telephoneNumber = +1 555-555-5555 , ou = People,dc=example,dc=com' } + + it 'raises UnsupportedError' do + expect { subject }.to raise_error(Gitlab::Auth::LDAP::DN::UnsupportedError) + end + end + end + end + end + + context 'when the given DN is malformed' do + context 'when ending with a comma' do + let(:given) { 'uid=John Smith,' } + + it 'raises MalformedError' do + expect { subject }.to raise_error(Gitlab::Auth::LDAP::DN::MalformedError, 'DN string ended unexpectedly') + end + end + + context 'when given a BER encoded attribute value with a space in it' do + let(:given) { '0.9.2342.19200300.100.1.25=#aa aa' } + + it 'raises MalformedError' do + expect { subject }.to raise_error(Gitlab::Auth::LDAP::DN::MalformedError, "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) { '0.9.2342.19200300.100.1.25=#aaXaaa' } + + it 'raises MalformedError' do + expect { subject }.to raise_error(Gitlab::Auth::LDAP::DN::MalformedError, "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) { '0.9.2342.19200300.100.1.25=#aaaYaa' } + + it 'raises MalformedError' do + expect { subject }.to raise_error(Gitlab::Auth::LDAP::DN::MalformedError, "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) { 'uid="Sebasti\\cX\\a1n"' } + + it 'raises MalformedError' do + expect { subject }.to raise_error(Gitlab::Auth::LDAP::DN::MalformedError, "Expected the second character of a hex pair inside a double quoted value, but got \"X\"") + end + end + + context 'without a name value pair' do + let(:given) { 'John' } + + it 'raises MalformedError' do + expect { subject }.to raise_error(Gitlab::Auth::LDAP::DN::MalformedError, 'DN string ended unexpectedly') + end + end + + context 'with an open (as opposed to closed) double quote' do + let(:given) { 'cn="James' } + + it 'raises MalformedError' do + expect { subject }.to raise_error(Gitlab::Auth::LDAP::DN::MalformedError, 'DN string ended unexpectedly') + end + end + + context 'with an invalid escaped hex code' do + let(:given) { 'cn=J\ames' } + + it 'raises MalformedError' do + expect { subject }.to raise_error(Gitlab::Auth::LDAP::DN::MalformedError, 'Invalid escaped hex code "\am"') + end + end + + context 'with a value ending with the escape character' do + let(:given) { 'cn=\\' } + + it 'raises MalformedError' do + expect { subject }.to raise_error(Gitlab::Auth::LDAP::DN::MalformedError, 'DN string ended unexpectedly') + end + end + + context 'with an invalid OID attribute type name' do + let(:given) { '1.2.d=Value' } + + it 'raises MalformedError' do + expect { subject }.to raise_error(Gitlab::Auth::LDAP::DN::MalformedError, 'Unrecognized RDN OID attribute type name character "d"') + end + end + + context 'with a period in a non-OID attribute type name' do + let(:given) { 'd1.2=Value' } + + it 'raises MalformedError' do + expect { subject }.to raise_error(Gitlab::Auth::LDAP::DN::MalformedError, 'Unrecognized RDN attribute type name character "."') + end + end + + context 'when starting with non-space, non-alphanumeric character' do + let(:given) { ' -uid=John Smith' } + + it 'raises MalformedError' do + expect { subject }.to raise_error(Gitlab::Auth::LDAP::DN::MalformedError, 'Unrecognized first character of an RDN attribute type name "-"') + end + end + + context 'when given a UID with an escaped equal sign' do + let(:given) { 'uid\\=john' } + + it 'raises MalformedError' do + expect { subject }.to raise_error(Gitlab::Auth::LDAP::DN::MalformedError, 'Unrecognized RDN attribute type name character "\\"') + end + 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 + end +end diff --git a/spec/lib/gitlab/auth/ldap/person_spec.rb b/spec/lib/gitlab/auth/ldap/person_spec.rb new file mode 100644 index 00000000000..1527fe60fb9 --- /dev/null +++ b/spec/lib/gitlab/auth/ldap/person_spec.rb @@ -0,0 +1,160 @@ +require 'spec_helper' + +describe Gitlab::Auth::LDAP::Person do + include LdapHelpers + + let(:entry) { ldap_user_entry('john.doe') } + + before do + stub_ldap_config( + options: { + 'uid' => 'uid', + 'attributes' => { + '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) } + + 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) } + + it_behaves_like 'normalizes a DN attribute value' + + context 'with an exception during normalization' do + let(:given) { 'John "Smith,' } # just something that will cause an exception + + it 'returns the given UID unmodified' do + expect(subject).to eq(given) + end + 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::Auth::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 '#name' do + it 'uses the configured name attribute and handles values as an array' do + name = 'John Doe' + entry['cn'] = [name] + person = described_class.new(entry, 'ldapmain') + + expect(person.name).to eq(name) + end + end + + describe '#email' do + it 'returns the value of mail, if present' do + mail = 'john@example.com' + entry['mail'] = mail + person = described_class.new(entry, 'ldapmain') + + expect(person.email).to eq([mail]) + end + + it 'returns the value of userPrincipalName, if mail and email are not present' do + user_principal_name = 'john.doe@example.com' + entry['userPrincipalName'] = user_principal_name + person = described_class.new(entry, 'ldapmain') + + expect(person.email).to eq([user_principal_name]) + 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 + + context 'if lowercase_usernames setting is' do + let(:username_attribute) { 'uid' } + + before do + entry[username_attribute] = 'JOHN' + @person = described_class.new(entry, 'ldapmain') + end + + it 'enabled the username attribute is lower cased' do + stub_ldap_config(lowercase_usernames: true) + + expect(@person.username).to eq 'john' + end + + it 'disabled the username attribute is not lower cased' do + stub_ldap_config(lowercase_usernames: false) + + expect(@person.username).to eq 'JOHN' + 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 + end +end diff --git a/spec/lib/gitlab/auth/ldap/user_spec.rb b/spec/lib/gitlab/auth/ldap/user_spec.rb new file mode 100644 index 00000000000..cab2169593a --- /dev/null +++ b/spec/lib/gitlab/auth/ldap/user_spec.rb @@ -0,0 +1,241 @@ +require 'spec_helper' + +describe Gitlab::Auth::LDAP::User do + let(:ldap_user) { described_class.new(auth_hash) } + let(:gl_user) { ldap_user.gl_user } + let(:info) do + { + name: 'John', + email: 'john@example.com', + nickname: 'john' + } + end + let(:auth_hash) do + OmniAuth::AuthHash.new(uid: 'uid=John Smith,ou=People,dc=example,dc=com', provider: 'ldapmain', info: info) + end + let(:ldap_user_upper_case) { described_class.new(auth_hash_upper_case) } + let(:info_upper_case) do + { + name: 'John', + email: 'John@Example.com', # Email address has upper case chars + nickname: 'john' + } + end + let(:auth_hash_upper_case) do + OmniAuth::AuthHash.new(uid: 'uid=John Smith,ou=People,dc=example,dc=com', provider: 'ldapmain', info: info_upper_case) + end + + describe '#changed?' do + it "marks existing ldap user as changed" do + create(:omniauth_user, extern_uid: 'uid=John Smith,ou=People,dc=example,dc=com', provider: 'ldapmain') + expect(ldap_user.changed?).to be_truthy + end + + it "marks existing non-ldap user if the email matches as changed" do + create(:user, email: 'john@example.com') + expect(ldap_user.changed?).to be_truthy + end + + it "does not mark existing ldap user as changed" do + create(:omniauth_user, email: 'john@example.com', extern_uid: 'uid=john smith,ou=people,dc=example,dc=com', provider: 'ldapmain') + expect(ldap_user.changed?).to be_falsey + end + end + + describe '.find_by_uid_and_provider' do + let(:dn) { 'CN=John Åström, CN=Users, DC=Example, DC=com' } + + it 'retrieves the correct user' do + special_info = { + name: 'John Åström', + email: 'john@example.com', + nickname: 'jastrom' + } + special_hash = OmniAuth::AuthHash.new(uid: dn, provider: 'ldapmain', info: special_info) + special_chars_user = described_class.new(special_hash) + user = special_chars_user.save + + expect(described_class.find_by_uid_and_provider(dn, 'ldapmain')).to eq user + end + end + + describe 'find or create' do + it "finds the user if already existing" do + create(:omniauth_user, extern_uid: 'uid=john smith,ou=people,dc=example,dc=com', provider: 'ldapmain') + + expect { ldap_user.save }.not_to change { User.count } + end + + it "connects to existing non-ldap user if the email matches" do + existing_user = create(:omniauth_user, email: 'john@example.com', provider: "twitter") + expect { ldap_user.save }.not_to change { User.count } + + existing_user.reload + expect(existing_user.ldap_identity.extern_uid).to eql 'uid=john smith,ou=people,dc=example,dc=com' + expect(existing_user.ldap_identity.provider).to eql 'ldapmain' + end + + it 'connects to existing ldap user if the extern_uid changes' do + existing_user = create(:omniauth_user, email: 'john@example.com', extern_uid: 'old-uid', provider: 'ldapmain') + expect { ldap_user.save }.not_to change { User.count } + + existing_user.reload + expect(existing_user.ldap_identity.extern_uid).to eql 'uid=john smith,ou=people,dc=example,dc=com' + expect(existing_user.ldap_identity.provider).to eql 'ldapmain' + expect(existing_user.id).to eql ldap_user.gl_user.id + end + + it 'connects to existing ldap user if the extern_uid changes and email address has upper case characters' do + existing_user = create(:omniauth_user, email: 'john@example.com', extern_uid: 'old-uid', provider: 'ldapmain') + expect { ldap_user_upper_case.save }.not_to change { User.count } + + existing_user.reload + expect(existing_user.ldap_identity.extern_uid).to eql 'uid=john smith,ou=people,dc=example,dc=com' + expect(existing_user.ldap_identity.provider).to eql 'ldapmain' + expect(existing_user.id).to eql ldap_user.gl_user.id + end + + it 'maintains an identity per provider' do + existing_user = create(:omniauth_user, email: 'john@example.com', provider: 'twitter') + expect(existing_user.identities.count).to be(1) + + ldap_user.save + expect(ldap_user.gl_user.identities.count).to be(2) + + # Expect that find_by provider only returns a single instance of an identity and not an Enumerable + expect(ldap_user.gl_user.identities.find_by(provider: 'twitter')).to be_instance_of Identity + expect(ldap_user.gl_user.identities.find_by(provider: auth_hash.provider)).to be_instance_of Identity + end + + it "creates a new user if not found" do + expect { ldap_user.save }.to change { User.count }.by(1) + end + + context 'when signup is disabled' do + before do + stub_application_setting signup_enabled: false + end + + it 'creates the user' do + ldap_user.save + + expect(gl_user).to be_persisted + end + end + + context 'when user confirmation email is enabled' do + before do + stub_application_setting send_user_confirmation_email: true + end + + it 'creates and confirms the user anyway' do + ldap_user.save + + expect(gl_user).to be_persisted + expect(gl_user).to be_confirmed + end + end + end + + describe 'updating email' do + context "when LDAP sets an email" do + it "has a real email" do + expect(ldap_user.gl_user.email).to eq(info[:email]) + end + + it "has email set as synced" do + expect(ldap_user.gl_user.user_synced_attributes_metadata.email_synced).to be_truthy + end + + it "has email set as read-only" do + expect(ldap_user.gl_user.read_only_attribute?(:email)).to be_truthy + end + + it "has synced attributes provider set to ldapmain" do + expect(ldap_user.gl_user.user_synced_attributes_metadata.provider).to eql 'ldapmain' + end + end + + context "when LDAP doesn't set an email" do + before do + info.delete(:email) + end + + it "has a temp email" do + expect(ldap_user.gl_user.temp_oauth_email?).to be_truthy + end + + it "has email set as not synced" do + expect(ldap_user.gl_user.user_synced_attributes_metadata.email_synced).to be_falsey + end + + it "does not have email set as read-only" do + expect(ldap_user.gl_user.read_only_attribute?(:email)).to be_falsey + end + end + end + + describe 'blocking' do + def configure_block(value) + allow_any_instance_of(Gitlab::Auth::LDAP::Config) + .to receive(:block_auto_created_users).and_return(value) + end + + context 'signup' do + context 'dont block on create' do + before do + configure_block(false) + end + + it do + ldap_user.save + expect(gl_user).to be_valid + expect(gl_user).not_to be_blocked + end + end + + context 'block on create' do + before do + configure_block(true) + end + + it do + ldap_user.save + expect(gl_user).to be_valid + expect(gl_user).to be_blocked + end + end + end + + context 'sign-in' do + before do + ldap_user.save + ldap_user.gl_user.activate + end + + context 'dont block on create' do + before do + configure_block(false) + end + + it do + ldap_user.save + expect(gl_user).to be_valid + expect(gl_user).not_to be_blocked + end + end + + context 'block on create' do + before do + configure_block(true) + end + + it do + ldap_user.save + expect(gl_user).to be_valid + expect(gl_user).not_to be_blocked + end + end + end + end +end |