Fix up _ldap_res_to_model for ldap identity backend
If a user specifies a user enabled attribute, then conditional
for checking if it's a string is insufficient, and will always
come back as False, since it's referencing the method, and not
actually calling it.
Closes-Bug: #1415271
(cherry picked from commit ad9ecdbab9
)
Change-Id: I4ea48929bc175b40ddb706e7750661a57882ed6b
This commit is contained in:
parent
410ade1e2c
commit
0c7535b233
|
@ -16,6 +16,7 @@ import uuid
|
||||||
|
|
||||||
import ldap
|
import ldap
|
||||||
import ldap.filter
|
import ldap.filter
|
||||||
|
import six
|
||||||
|
|
||||||
from keystone import clean
|
from keystone import clean
|
||||||
from keystone.common import driver_hints
|
from keystone.common import driver_hints
|
||||||
|
@ -220,8 +221,8 @@ class UserApi(common_ldap.EnabledEmuMixIn, common_ldap.BaseLdap):
|
||||||
# This could be a bool or a string. If it's a string,
|
# This could be a bool or a string. If it's a string,
|
||||||
# we need to convert it so we can invert it properly.
|
# we need to convert it so we can invert it properly.
|
||||||
enabled = obj.get('enabled', self.enabled_default)
|
enabled = obj.get('enabled', self.enabled_default)
|
||||||
if type(enabled) is str:
|
if isinstance(enabled, six.string_types):
|
||||||
if enabled.lower == 'true':
|
if enabled.lower() == 'true':
|
||||||
enabled = True
|
enabled = True
|
||||||
else:
|
else:
|
||||||
enabled = False
|
enabled = False
|
||||||
|
|
|
@ -1294,6 +1294,47 @@ class LDAPIdentity(BaseLDAPIdentity, tests.TestCase):
|
||||||
# from the resource default.
|
# from the resource default.
|
||||||
self.assertIs(True, user_ref['enabled'])
|
self.assertIs(True, user_ref['enabled'])
|
||||||
|
|
||||||
|
@mock.patch.object(common_ldap_core.BaseLdap, '_ldap_get')
|
||||||
|
def test_user_enabled_attribute_handles_expired(self, mock_ldap_get):
|
||||||
|
# If using 'passwordisexpired' as enabled attribute, and inverting it,
|
||||||
|
# Then an unauthorized user (expired password) should not be enabled.
|
||||||
|
self.config_fixture.config(group='ldap', user_enabled_invert=True,
|
||||||
|
user_enabled_attribute='passwordisexpired')
|
||||||
|
mock_ldap_get.return_value = (
|
||||||
|
u'uid=123456789,c=us,ou=our_ldap,o=acme.com',
|
||||||
|
{
|
||||||
|
'uid': [123456789],
|
||||||
|
'mail': ['shaun@acme.com'],
|
||||||
|
'passwordisexpired': ['TRUE'],
|
||||||
|
'cn': ['uid=123456789,c=us,ou=our_ldap,o=acme.com']
|
||||||
|
}
|
||||||
|
)
|
||||||
|
|
||||||
|
user_api = identity.backends.ldap.UserApi(CONF)
|
||||||
|
user_ref = user_api.get('123456789')
|
||||||
|
self.assertIs(False, user_ref['enabled'])
|
||||||
|
|
||||||
|
@mock.patch.object(common_ldap_core.BaseLdap, '_ldap_get')
|
||||||
|
def test_user_enabled_attribute_handles_utf8(self, mock_ldap_get):
|
||||||
|
# If using 'passwordisexpired' as enabled attribute, and inverting it,
|
||||||
|
# and the result is utf8 encoded, then the an authorized user should
|
||||||
|
# be enabled.
|
||||||
|
self.config_fixture.config(group='ldap', user_enabled_invert=True,
|
||||||
|
user_enabled_attribute='passwordisexpired')
|
||||||
|
mock_ldap_get.return_value = (
|
||||||
|
u'uid=123456789,c=us,ou=our_ldap,o=acme.com',
|
||||||
|
{
|
||||||
|
'uid': [123456789],
|
||||||
|
'mail': [u'shaun@acme.com'],
|
||||||
|
'passwordisexpired': [u'false'],
|
||||||
|
'cn': [u'uid=123456789,c=us,ou=our_ldap,o=acme.com']
|
||||||
|
}
|
||||||
|
)
|
||||||
|
|
||||||
|
user_api = identity.backends.ldap.UserApi(CONF)
|
||||||
|
user_ref = user_api.get('123456789')
|
||||||
|
self.assertIs(True, user_ref['enabled'])
|
||||||
|
|
||||||
@mock.patch.object(common_ldap_core.KeystoneLDAPHandler, 'simple_bind_s')
|
@mock.patch.object(common_ldap_core.KeystoneLDAPHandler, 'simple_bind_s')
|
||||||
def test_user_api_get_connection_no_user_password(self, mocked_method):
|
def test_user_api_get_connection_no_user_password(self, mocked_method):
|
||||||
"""Don't bind in case the user and password are blank."""
|
"""Don't bind in case the user and password are blank."""
|
||||||
|
@ -1988,6 +2029,26 @@ class LDAPIdentityEnabledEmulation(LDAPIdentity):
|
||||||
self.skipTest(
|
self.skipTest(
|
||||||
"N/A: Covered by test_user_enabled_invert")
|
"N/A: Covered by test_user_enabled_invert")
|
||||||
|
|
||||||
|
@mock.patch.object(common_ldap_core.BaseLdap, '_ldap_get')
|
||||||
|
def test_user_enabled_attribute_handles_utf8(self, mock_ldap_get):
|
||||||
|
# Since user_enabled_emulation is enabled in this test, this test will
|
||||||
|
# fail since it's using user_enabled_invert.
|
||||||
|
self.config_fixture.config(group='ldap', user_enabled_invert=True,
|
||||||
|
user_enabled_attribute='passwordisexpired')
|
||||||
|
mock_ldap_get.return_value = (
|
||||||
|
u'uid=123456789,c=us,ou=our_ldap,o=acme.com',
|
||||||
|
{
|
||||||
|
'uid': [123456789],
|
||||||
|
'mail': [u'shaun@acme.com'],
|
||||||
|
'passwordisexpired': [u'false'],
|
||||||
|
'cn': [u'uid=123456789,c=us,ou=our_ldap,o=acme.com']
|
||||||
|
}
|
||||||
|
)
|
||||||
|
|
||||||
|
user_api = identity.backends.ldap.UserApi(CONF)
|
||||||
|
user_ref = user_api.get('123456789')
|
||||||
|
self.assertIs(False, user_ref['enabled'])
|
||||||
|
|
||||||
|
|
||||||
class LdapIdentitySqlAssignment(BaseLDAPIdentity, tests.SQLDriverOverrides,
|
class LdapIdentitySqlAssignment(BaseLDAPIdentity, tests.SQLDriverOverrides,
|
||||||
tests.TestCase):
|
tests.TestCase):
|
||||||
|
|
Loading…
Reference in New Issue