openstack/keystone
Code Issues Proposed changes

Honor group_members_are_ids for user_enabled_emulation

Browse Source 
Applied when group config is to be honored
(i.e. set user_enabled_emulation_use_group_config).
Conditionals follow usage of group_members_are_ids.

Added new test for the case with ids.
It fails without fix.
The original test expanded to ensure the change did not
break its internals either.
It passes without fix as well.

Additionally some TODOs are added for observed potential issues.

Change-Id: I7874a70e6109219baee80309c3a27f8af9905a6d
Closes-Bug: #1839133
Signed-off-by: Radosław Piliszek <radoslaw.piliszek@gmail.com>
This commit is contained in:
Radosław Piliszek 2019-08-06 13:25:17 +02:00
parent 73045dfbca
commit c7fae97d87
3 changed files with 72 additions and 4 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

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

@ -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,13 @@ 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 +1837,33 @@ 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)

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

@ -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,44 @@ 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')

5
releasenotes/notes/bug-1839133-24570c9fbacb530d.yaml Normal file
View File

@ -0,0 +1,5 @@
---
fixes:
- |
[`bug 1839133 <https://bugs.launchpad.net/keystone/+bug/1839133>`_]
Makes user_enabled_emulation_use_group_config honor group_members_are_ids.