diff --git a/keystone/identity/backends/ldap/common.py b/keystone/identity/backends/ldap/common.py index b9becea749..f79c123d90 100644 --- a/keystone/identity/backends/ldap/common.py +++ b/keystone/identity/backends/ldap/common.py @@ -1781,6 +1781,7 @@ class EnabledEmuMixIn(BaseLdap): DEFAULT_GROUP_OBJECTCLASS = 'groupOfNames' DEFAULT_MEMBER_ATTRIBUTE = 'member' + DEFAULT_GROUP_MEMBERS_ARE_IDS = False def __init__(self, conf): super(EnabledEmuMixIn, self).__init__(conf) @@ -1797,9 +1798,11 @@ class EnabledEmuMixIn(BaseLdap): if not self.use_group_config: self.member_attribute = self.DEFAULT_MEMBER_ATTRIBUTE self.group_objectclass = self.DEFAULT_GROUP_OBJECTCLASS + self.group_members_are_ids = self.DEFAULT_GROUP_MEMBERS_ARE_IDS else: self.member_attribute = conf.ldap.group_member_attribute self.group_objectclass = conf.ldap.group_objectclass + self.group_members_are_ids = conf.ldap.group_members_are_ids if not self.enabled_emulation_dn: naming_attr_name = 'cn' @@ -1815,8 +1818,14 @@ class EnabledEmuMixIn(BaseLdap): naming_rdn[1]) self.enabled_emulation_naming_attr = naming_attr + # TODO(yoctozepto): methods below use _id_to_dn which requests another + # LDAP connection - optimize it + def _get_enabled(self, object_id, conn): - dn = self._id_to_dn(object_id) + if self.group_members_are_ids: + dn = object_id + else: + dn = self._id_to_dn(object_id) query = '(%s=%s)' % (self.member_attribute, ldap.filter.escape_filter_chars(dn)) try: @@ -1829,24 +1838,34 @@ class EnabledEmuMixIn(BaseLdap): return bool(enabled_value) def _add_enabled(self, object_id): + if self.group_members_are_ids: + dn = object_id + else: + dn = self._id_to_dn(object_id) with self.get_connection() as conn: + # TODO(yoctozepto): _get_enabled potentially calls + # _id_to_dn 2nd time - optimize it if not self._get_enabled(object_id, conn): modlist = [(ldap.MOD_ADD, self.member_attribute, - [self._id_to_dn(object_id)])] + [dn])] try: conn.modify_s(self.enabled_emulation_dn, modlist) except ldap.NO_SUCH_OBJECT: attr_list = [('objectClass', [self.group_objectclass]), (self.member_attribute, - [self._id_to_dn(object_id)]), + [dn]), self.enabled_emulation_naming_attr] conn.add_s(self.enabled_emulation_dn, attr_list) def _remove_enabled(self, object_id): + if self.group_members_are_ids: + dn = object_id + else: + dn = self._id_to_dn(object_id) modlist = [(ldap.MOD_DELETE, self.member_attribute, - [self._id_to_dn(object_id)])] + [dn])] with self.get_connection() as conn: try: conn.modify_s(self.enabled_emulation_dn, modlist) diff --git a/keystone/tests/unit/test_backend_ldap.py b/keystone/tests/unit/test_backend_ldap.py index aa7a50747f..d2b8c7db9b 100644 --- a/keystone/tests/unit/test_backend_ldap.py +++ b/keystone/tests/unit/test_backend_ldap.py @@ -2046,9 +2046,17 @@ class LDAPIdentityEnabledEmulation(LDAPIdentity, unit.TestCase): "Enabled emulation conflicts with enabled mask") def test_user_enabled_use_group_config(self): + # Establish enabled-emulation group name to later query its members + group_name = 'enabled_users' + driver = PROVIDERS.identity_api._select_identity_driver( + CONF.identity.default_domain_id) + group_dn = 'cn=%s,%s' % (group_name, driver.group.tree_dn) + self.config_fixture.config( group='ldap', user_enabled_emulation_use_group_config=True, + user_enabled_emulation_dn=group_dn, + group_name_attribute='cn', group_member_attribute='uniqueMember', group_objectclass='groupOfUniqueNames') self.ldapdb.clear() @@ -2064,6 +2072,46 @@ class LDAPIdentityEnabledEmulation(LDAPIdentity, unit.TestCase): user_ref = PROVIDERS.identity_api.get_user(user_ref['id']) self.assertIs(True, user_ref['enabled']) + # Ensure state matches the group config + group_ref = PROVIDERS.identity_api.get_group_by_name( + group_name, CONF.identity.default_domain_id) + PROVIDERS.identity_api.check_user_in_group( + user_ref['id'], group_ref['id']) + + def test_user_enabled_use_group_config_with_ids(self): + # Establish enabled-emulation group name to later query its members + group_name = 'enabled_users' + driver = PROVIDERS.identity_api._select_identity_driver( + CONF.identity.default_domain_id) + group_dn = 'cn=%s,%s' % (group_name, driver.group.tree_dn) + + self.config_fixture.config( + group='ldap', + user_enabled_emulation_use_group_config=True, + user_enabled_emulation_dn=group_dn, + group_name_attribute='cn', + group_member_attribute='memberUid', + group_members_are_ids=True, + group_objectclass='posixGroup') + self.ldapdb.clear() + self.load_backends() + + # Create a user and ensure they are enabled. + user1 = unit.new_user_ref(enabled=True, + domain_id=CONF.identity.default_domain_id) + user_ref = PROVIDERS.identity_api.create_user(user1) + self.assertIs(True, user_ref['enabled']) + + # Get a user and ensure they are enabled. + user_ref = PROVIDERS.identity_api.get_user(user_ref['id']) + self.assertIs(True, user_ref['enabled']) + + # Ensure state matches the group config + group_ref = PROVIDERS.identity_api.get_group_by_name( + group_name, CONF.identity.default_domain_id) + PROVIDERS.identity_api.check_user_in_group( + user_ref['id'], group_ref['id']) + def test_user_enabled_invert(self): self.config_fixture.config(group='ldap', user_enabled_invert=True, user_enabled_default='False') diff --git a/releasenotes/notes/bug-1839133-24570c9fbacb530d.yaml b/releasenotes/notes/bug-1839133-24570c9fbacb530d.yaml new file mode 100644 index 0000000000..b6ed1556de --- /dev/null +++ b/releasenotes/notes/bug-1839133-24570c9fbacb530d.yaml @@ -0,0 +1,5 @@ +--- +fixes: + - | + [`bug 1839133 `_] + Makes user_enabled_emulation_use_group_config honor group_members_are_ids.