Merge "Honor group_members_are_ids for user_enabled_emulation" into stable/stein

3 files changed, 76 insertions, 4 deletions

diff --git a/keystone/identity/backends/ldap/common.py b/keystone/identity/backends/ldap/common.py
index b9becea74..f79c123d9 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 aa7a50747..d2b8c7db9 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 000000000..b6ed1556d
--- /dev/null
+++ b/releasenotes/notes/bug-1839133-24570c9fbacb530d.yaml
@@ -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.