From 25ec22d281619db25556ed1c8271ae3ee5b45914 Mon Sep 17 00:00:00 2001 From: Nathan Kinder Date: Wed, 2 Jul 2014 18:36:40 -0700 Subject: [PATCH] Allow LDAP lock attributes to be used as enable attributes Some LDAP servers support disabling accounts via a boolean "lock" attribute. For these servers, a value in LDAP of "True" means that the account is locked, while a value of "False" means the account is active. When the "user_enabled_mask" and "user_enabled_emulation" options are not in use, Keystone currently expects a boolean "enabled" attribute where "True" means the account is enabled and "False" means the account is disabled. To support LDAP account lock attributes, we need a way to tell Keystone that the boolean values from LDAP are inverted. This adds a new "user_enabled_invert" setting that allows the enabled boolean logic to be inverted in the resource (LDAP), while leaving the logic as-is in the model (Keystone user object). The existing default behavior remains as-is. DocImpact Change-Id: I2a89d4b98c854e68e1bb10f53b8b29d92f945f60 Closes-bug: #1337029 --- etc/keystone.conf.sample | 9 ++ keystone/common/config.py | 8 ++ keystone/identity/backends/ldap.py | 21 +++- keystone/tests/test_backend_ldap.py | 166 +++++++++++++++++++++++++--- 4 files changed, 188 insertions(+), 16 deletions(-) diff --git a/etc/keystone.conf.sample b/etc/keystone.conf.sample index bb4271dcc3..6ad6c418f4 100644 --- a/etc/keystone.conf.sample +++ b/etc/keystone.conf.sample @@ -905,6 +905,15 @@ # LDAP attribute mapped to user enabled flag. (string value) #user_enabled_attribute=enabled +# Invert the meaning of the boolean enabled values. Some LDAP +# servers use a boolean lock attribute where "true" means an +# account is disabled. Setting "user_enabled_invert = true" +# will allow these lock attributes to be used. This setting +# will have no effect if "user_enabled_mask" or +# "user_enabled_emulation" settings are in use. (boolean +# value) +#user_enabled_invert=false + # Bitmask integer to indicate the bit that the enabled value # is stored in if the LDAP server represents "enabled" as a # bit on an integer rather than a boolean. A value of "0" diff --git a/keystone/common/config.py b/keystone/common/config.py index 91c1e9475c..d0cb27e764 100644 --- a/keystone/common/config.py +++ b/keystone/common/config.py @@ -516,6 +516,14 @@ FILE_OPTIONS = { help='LDAP attribute mapped to password.'), cfg.StrOpt('user_enabled_attribute', default='enabled', help='LDAP attribute mapped to user enabled flag.'), + cfg.BoolOpt('user_enabled_invert', default=False, + help='Invert the meaning of the boolean enabled values. ' + 'Some LDAP servers use a boolean lock attribute ' + 'where "true" means an account is disabled. Setting ' + '"user_enabled_invert = true" will allow these lock ' + 'attributes to be used. This setting will have no ' + 'effect if "user_enabled_mask" or ' + '"user_enabled_emulation" settings are in use.'), cfg.IntOpt('user_enabled_mask', default=0, help='Bitmask integer to indicate the bit that the enabled ' 'value is stored in if the LDAP server represents ' diff --git a/keystone/identity/backends/ldap.py b/keystone/identity/backends/ldap.py index f8aa9245c5..920150a925 100644 --- a/keystone/identity/backends/ldap.py +++ b/keystone/identity/backends/ldap.py @@ -102,6 +102,13 @@ class Identity(identity.Driver): if self.user.enabled_mask: self.user.mask_enabled_attribute(user) + elif self.user.enabled_invert and not self.user.enabled_emulation: + # We need to invert the enabled value for the old model object + # to prevent the LDAP update code from thinking that the enabled + # values are already equal. + user['enabled'] = not user['enabled'] + old_obj['enabled'] = not old_obj['enabled'] + self.user.update(user_id, user, old_obj) return self.user.get_filtered(user_id) @@ -203,6 +210,8 @@ class UserApi(common_ldap.EnabledEmuMixIn, common_ldap.BaseLdap): super(UserApi, self).__init__(conf) self.enabled_mask = conf.ldap.user_enabled_mask self.enabled_default = conf.ldap.user_enabled_default + self.enabled_invert = conf.ldap.user_enabled_invert + self.enabled_emulation = conf.ldap.user_enabled_emulation def _ldap_res_to_model(self, res): obj = super(UserApi, self)._ldap_res_to_model(res) @@ -210,6 +219,9 @@ class UserApi(common_ldap.EnabledEmuMixIn, common_ldap.BaseLdap): enabled = int(obj.get('enabled', self.enabled_default)) obj['enabled'] = ((enabled & self.enabled_mask) != self.enabled_mask) + elif self.enabled_invert and not self.enabled_emulation: + enabled = obj.get('enabled', self.enabled_default) + obj['enabled'] = not enabled obj['dn'] = res[0] return obj @@ -227,8 +239,15 @@ class UserApi(common_ldap.EnabledEmuMixIn, common_ldap.BaseLdap): if self.enabled_mask: orig_enabled = values['enabled'] self.mask_enabled_attribute(values) + elif self.enabled_invert and not self.enabled_emulation: + orig_enabled = values['enabled'] + if orig_enabled is not None: + values['enabled'] = not orig_enabled + else: + values['enabled'] = self.enabled_default values = super(UserApi, self).create(values) - if self.enabled_mask: + if self.enabled_mask or (self.enabled_invert and + not self.enabled_emulation): values['enabled'] = orig_enabled return values diff --git a/keystone/tests/test_backend_ldap.py b/keystone/tests/test_backend_ldap.py index 86fec3dc61..445ccfc41b 100644 --- a/keystone/tests/test_backend_ldap.py +++ b/keystone/tests/test_backend_ldap.py @@ -80,6 +80,20 @@ class BaseLDAPIdentity(test_backend.IdentityTests): config_files.append(tests.dirs.tests_conf('backend_ldap.conf')) return config_files + def get_user_enabled_vals(self, user): + user_dn = ( + self.identity_api.driver.user._id_to_dn_string(user['id'])) + enabled_attr_name = CONF.ldap.user_enabled_attribute + + ldap_ = self.identity_api.driver.user.get_connection() + res = ldap_.search_s(user_dn, + ldap.SCOPE_BASE, + u'(sn=%s)' % user['name']) + if enabled_attr_name in res[0][1]: + return res[0][1][enabled_attr_name] + else: + return None + def test_build_tree(self): """Regression test for building the tree names """ @@ -1083,18 +1097,6 @@ class LDAPIdentity(BaseLDAPIdentity, tests.TestCase): self.load_backends() self.load_fixtures(default_fixtures) - ldap_ = self.identity_api.driver.user.get_connection() - - def get_enabled_vals(user): - user_dn = ( - self.identity_api.driver.user._id_to_dn_string(user['id'])) - enabled_attr_name = CONF.ldap.user_enabled_attribute - - res = ldap_.search_s(user_dn, - ldap.SCOPE_BASE, - u'(sn=fäké1)') - return res[0][1][enabled_attr_name] - user = {'name': u'fäké1', 'enabled': True, 'domain_id': CONF.identity.default_domain_id} @@ -1105,7 +1107,7 @@ class LDAPIdentity(BaseLDAPIdentity, tests.TestCase): self.assertIs(user_ref['enabled'], True) self.assertNotIn('enabled_nomask', user_ref) - enabled_vals = get_enabled_vals(user_ref) + enabled_vals = self.get_user_enabled_vals(user_ref) self.assertEqual([512], enabled_vals) user_ref = self.identity_api.get_user(user_ref['id']) @@ -1117,7 +1119,7 @@ class LDAPIdentity(BaseLDAPIdentity, tests.TestCase): self.assertIs(user_ref['enabled'], False) self.assertNotIn('enabled_nomask', user_ref) - enabled_vals = get_enabled_vals(user_ref) + enabled_vals = self.get_user_enabled_vals(user_ref) self.assertEqual([514], enabled_vals) user_ref = self.identity_api.get_user(user_ref['id']) @@ -1129,13 +1131,90 @@ class LDAPIdentity(BaseLDAPIdentity, tests.TestCase): self.assertIs(user_ref['enabled'], True) self.assertNotIn('enabled_nomask', user_ref) - enabled_vals = get_enabled_vals(user_ref) + enabled_vals = self.get_user_enabled_vals(user_ref) self.assertEqual([512], enabled_vals) user_ref = self.identity_api.get_user(user_ref['id']) self.assertIs(user_ref['enabled'], True) self.assertNotIn('enabled_nomask', user_ref) + def test_user_enabled_invert(self): + self.config_fixture.config(group='ldap', user_enabled_invert=True, + user_enabled_default=False) + self.clear_database() + self.load_backends() + self.load_fixtures(default_fixtures) + + user1 = {'name': u'fäké1', 'enabled': True, + 'domain_id': CONF.identity.default_domain_id} + + user2 = {'name': u'fäké2', 'enabled': False, + 'domain_id': CONF.identity.default_domain_id} + + user3 = {'name': u'fäké3', + 'domain_id': CONF.identity.default_domain_id} + + # Ensure that the LDAP attribute is False for a newly created + # enabled user. + user_ref = self.identity_api.create_user(user1) + self.assertIs(True, user_ref['enabled']) + enabled_vals = self.get_user_enabled_vals(user_ref) + self.assertEqual([False], enabled_vals) + user_ref = self.identity_api.get_user(user_ref['id']) + self.assertIs(True, user_ref['enabled']) + + # Ensure that the LDAP attribute is True for a disabled user. + user1['enabled'] = False + user_ref = self.identity_api.update_user(user_ref['id'], user1) + self.assertIs(False, user_ref['enabled']) + enabled_vals = self.get_user_enabled_vals(user_ref) + self.assertEqual([True], enabled_vals) + + # Enable the user and ensure that the LDAP attribute is True again. + user1['enabled'] = True + user_ref = self.identity_api.update_user(user_ref['id'], user1) + self.assertIs(True, user_ref['enabled']) + enabled_vals = self.get_user_enabled_vals(user_ref) + self.assertEqual([False], enabled_vals) + + # Ensure that the LDAP attribute is True for a newly created + # disabled user. + user_ref = self.identity_api.create_user(user2) + self.assertIs(False, user_ref['enabled']) + enabled_vals = self.get_user_enabled_vals(user_ref) + self.assertEqual([True], enabled_vals) + user_ref = self.identity_api.get_user(user_ref['id']) + self.assertIs(False, user_ref['enabled']) + + # Ensure that the LDAP attribute is inverted for a newly created + # user when the user_enabled_default setting is used. + user_ref = self.identity_api.create_user(user3) + self.assertIs(True, user_ref['enabled']) + enabled_vals = self.get_user_enabled_vals(user_ref) + self.assertEqual([False], enabled_vals) + user_ref = self.identity_api.get_user(user_ref['id']) + self.assertIs(True, user_ref['enabled']) + + @mock.patch.object(common_ldap_core.BaseLdap, '_ldap_get') + def test_user_enabled_invert_no_enabled_value(self, mock_ldap_get): + self.config_fixture.config(group='ldap', user_enabled_invert=True, + user_enabled_default=False) + # Mock the search results to return an entry with + # no enabled value. + mock_ldap_get.return_value = ( + 'cn=junk,dc=example,dc=com', + { + 'sn': [uuid.uuid4().hex], + 'email': [uuid.uuid4().hex] + } + ) + + user_api = identity.backends.ldap.UserApi(CONF) + user_ref = user_api.get('junk') + # Ensure that the model enabled attribute is inverted + # from the resource default. + self.assertIs(not CONF.ldap.user_enabled_default, user_ref['enabled']) + @mock.patch.object(common_ldap_core.KeystoneLDAPHandler, 'simple_bind_s') def test_user_api_get_connection_no_user_password(self, mocked_method): """Don't bind in case the user and password are blank.""" @@ -1664,6 +1743,63 @@ class LDAPIdentityEnabledEmulation(LDAPIdentity): self.skipTest( "Enabled emulation conflicts with enabled mask") + def test_user_enabled_invert(self): + self.config_fixture.config(group='ldap', user_enabled_invert=True, + user_enabled_default=False) + self.clear_database() + self.load_backends() + self.load_fixtures(default_fixtures) + + user1 = {'name': u'fäké1', 'enabled': True, + 'domain_id': CONF.identity.default_domain_id} + + user2 = {'name': u'fäké2', 'enabled': False, + 'domain_id': CONF.identity.default_domain_id} + + user3 = {'name': u'fäké3', + 'domain_id': CONF.identity.default_domain_id} + + # Ensure that the enabled LDAP attribute is not set for a + # newly created enabled user. + user_ref = self.identity_api.create_user(user1) + self.assertIs(True, user_ref['enabled']) + self.assertIsNone(self.get_user_enabled_vals(user_ref)) + user_ref = self.identity_api.get_user(user_ref['id']) + self.assertIs(True, user_ref['enabled']) + + # Ensure that an enabled LDAP attribute is not set for a disabled user. + user1['enabled'] = False + user_ref = self.identity_api.update_user(user_ref['id'], user1) + self.assertIs(False, user_ref['enabled']) + self.assertIsNone(self.get_user_enabled_vals(user_ref)) + + # Enable the user and ensure that the LDAP enabled + # attribute is not set. + user1['enabled'] = True + user_ref = self.identity_api.update_user(user_ref['id'], user1) + self.assertIs(True, user_ref['enabled']) + self.assertIsNone(self.get_user_enabled_vals(user_ref)) + + # Ensure that the LDAP enabled attribute is not set for a + # newly created disabled user. + user_ref = self.identity_api.create_user(user2) + self.assertIs(False, user_ref['enabled']) + self.assertIsNone(self.get_user_enabled_vals(user_ref)) + user_ref = self.identity_api.get_user(user_ref['id']) + self.assertIs(False, user_ref['enabled']) + + # Ensure that the LDAP enabled attribute is not set for a newly created + # user when the user_enabled_default setting is used. + user_ref = self.identity_api.create_user(user3) + self.assertIs(True, user_ref['enabled']) + self.assertIsNone(self.get_user_enabled_vals(user_ref)) + user_ref = self.identity_api.get_user(user_ref['id']) + self.assertIs(True, user_ref['enabled']) + + def test_user_enabled_invert_no_enabled_value(self): + self.skipTest( + "N/A: Covered by test_user_enabled_invert") + class LdapIdentitySqlAssignment(BaseLDAPIdentity, tests.SQLDriverOverrides, tests.TestCase):