openstack/keystone
Code Issues Proposed changes

Merge "Refactor some ldap code to implement TODOs"

Browse Source
This commit is contained in:
Zuul 2020-03-04 16:24:59 +00:00 committed by Gerrit Code Review
commit a03b041747
2 changed files with 21 additions and 26 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

43
keystone/identity/backends/ldap/common.py
View File

@ -1807,16 +1807,19 @@ 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):
def _id_to_member_attribute_value(self, object_id):
"""Convert id to value expected by member_attribute."""
if self.group_members_are_ids:
dn = object_id
else:
dn = self._id_to_dn(object_id)
return object_id
return self._id_to_dn(object_id)
def _is_id_enabled(self, object_id, conn):
member_attr_val = self._id_to_member_attribute_value(object_id)
return self._is_member_enabled(member_attr_val, conn)
def _is_member_enabled(self, member_attr_val, conn):
query = '(%s=%s)' % (self.member_attribute,
ldap.filter.escape_filter_chars(dn))
ldap.filter.escape_filter_chars(member_attr_val))
try:
enabled_value = conn.search_s(self.enabled_emulation_dn,
ldap.SCOPE_BASE,
@ -1827,34 +1830,26 @@ 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)
member_attr_val = self._id_to_member_attribute_value(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):
if not self._is_member_enabled(member_attr_val, conn):
modlist = [(ldap.MOD_ADD,
self.member_attribute,
[dn])]
[member_attr_val])]
try:
conn.modify_s(self.enabled_emulation_dn, modlist)
except ldap.NO_SUCH_OBJECT:
attr_list = [('objectClass', [self.group_objectclass]),
(self.member_attribute,
[dn]),
[member_attr_val]),
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)
member_attr_val = self._id_to_member_attribute_value(object_id)
modlist = [(ldap.MOD_DELETE,
self.member_attribute,
[dn])]
[member_attr_val])]
with self.get_connection() as conn:
try:
conn.modify_s(self.enabled_emulation_dn, modlist)
@ -1879,7 +1874,7 @@ class EnabledEmuMixIn(BaseLdap):
ref = super(EnabledEmuMixIn, self).get(object_id, ldap_filter)
if ('enabled' not in self.attribute_ignore and
self.enabled_emulation):
ref['enabled'] = self._get_enabled(object_id, conn)
ref['enabled'] = self._is_id_enabled(object_id, conn)
return ref
def get_all(self, ldap_filter=None, hints=None):
@ -1891,7 +1886,7 @@ class EnabledEmuMixIn(BaseLdap):
if x[0] != self.enabled_emulation_dn]
with self.get_connection() as conn:
for obj_ref in obj_list:
obj_ref['enabled'] = self._get_enabled(
obj_ref['enabled'] = self._is_id_enabled(
obj_ref['id'], conn)
return obj_list
else:

4
keystone/tests/unit/test_backend_ldap.py
View File

@ -2224,7 +2224,7 @@ class LDAPIdentityEnabledEmulation(LDAPIdentity, unit.TestCase):
# Override the tree_dn, it's used to build the enabled member filter
mixin_impl.tree_dn = sample_dn
# The filter that _get_enabled is going to build contains the
# The filter, which _is_id_enabled is going to build, contains the
# tree_dn, which better be escaped in this case.
exp_filter = '(%s=%s=%s,%s)' % (
mixin_impl.member_attribute, mixin_impl.id_attr, object_id,
@ -2233,7 +2233,7 @@ class LDAPIdentityEnabledEmulation(LDAPIdentity, unit.TestCase):
with mixin_impl.get_connection() as conn:
m = self.useFixture(
fixtures.MockPatchObject(conn, 'search_s')).mock
mixin_impl._get_enabled(object_id, conn)
mixin_impl._is_id_enabled(object_id, conn)
# The 3rd argument is the DN.
self.assertEqual(exp_filter, m.call_args[0][2])