Commit 55f93e15 authored by Drew Blessing's avatar Drew Blessing Committed by Stan Hu

Fix LDAP attributes sync ssh keys boolean value bug

We tell users to set `sync_ssh_keys` to `false` if they don't
intend to use the feature. Strangely, `true` is not how you
enable it, rather you give it an LDAP attribute name. The
default value for `sync_ssh_keys` is `nil` which doesn't break
`Gitlab::LDAP::Person.ldap_attributes`, but `false` does. This
rejects `false` *and* `true` for good measure and adds an EE-only
test to ensure it doesn't regress.

Closes #4662
parent e6c0b419
---
title: Fix failed LDAP logins when sync_ssh_keys is included in config
merge_request:
author:
type: fixed
......@@ -46,7 +46,7 @@ module EE
def ldap_attributes(config)
attributes = super + [
'memberof',
config.sync_ssh_keys
(config.sync_ssh_keys if config.sync_ssh_keys.is_a?(String))
]
attributes.compact.uniq
end
......
......@@ -69,6 +69,50 @@ describe Gitlab::LDAP::Person do
end
end
describe '.ldap_attributes' do
def stub_sync_ssh_keys(value)
stub_ldap_config(
options: {
'uid' => nil,
'attributes' => {
'name' => 'cn',
'email' => 'mail',
'username' => %w(uid mail memberof)
},
'sync_ssh_keys' => value
}
)
end
let(:config) { Gitlab::LDAP::Config.new('ldapmain') }
let(:ldap_attributes) { described_class.ldap_attributes(config) }
let(:expected_attributes) { %w(dn cn uid mail memberof) }
it 'includes a real attribute name' do
stub_sync_ssh_keys('my-ssh-attribute')
expect(ldap_attributes).to match_array(expected_attributes + ['my-ssh-attribute'])
end
it 'excludes integers' do
stub_sync_ssh_keys(0)
expect(ldap_attributes).to match_array(expected_attributes)
end
it 'excludes false values' do
stub_sync_ssh_keys(false)
expect(ldap_attributes).to match_array(expected_attributes)
end
it 'excludes true values' do
stub_sync_ssh_keys(true)
expect(ldap_attributes).to match_array(expected_attributes)
end
end
describe '#kerberos_principal' do
let(:entry) do
ldif = "dn: cn=foo, dc=bar, dc=com\nsAMAccountName: myName\n"
......
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