Merge "Unable to delete tenant if contains roles in LDAP backend (bug 1057407)"

etc/keystone.conf.sample

@@ -107,6 +107,7 @@
 # password = None
 # suffix = cn=example,cn=com
 # use_dumb_member = False
+# allow_subtree_delete = False
 
 # user_tree_dn = ou=Users,dc=example,dc=com
 # user_objectclass = inetOrgPerson

keystone/common/ldap/core.py

@@ -25,6 +25,7 @@ LOG = logging.getLogger(__name__)
 
 
 LDAP_VALUES = {'TRUE': True, 'FALSE': False}
+CONTROL_TREEDELETE = '1.2.840.113556.1.4.805'
 
 
 def py2ldap(val):
@@ -103,6 +104,8 @@ class BaseLdap(object):
 
             self.structural_classes = self.DEFAULT_STRUCTURAL_CLASSES
         self.use_dumb_member = getattr(conf.ldap, 'use_dumb_member') or True
+        self.subtree_delete_enabled = getattr(conf.ldap,
+                                              'allow_subtree_delete')
 
     def get_connection(self, user=None, password=None):
         if self.LDAP_URL.startswith('fake://'):
@@ -309,6 +312,14 @@ class BaseLdap(object):
         conn = self.get_connection()
         conn.delete_s(self._id_to_dn(id))
 
+    def deleteTree(self, id):
+        conn = self.get_connection()
+        tree_delete_control = ldap.controls.LDAPControl(CONTROL_TREEDELETE,
+                                                        0,
+                                                        None)
+        conn.delete_ext_s(self._id_to_dn(id),
+                          serverctrls=[tree_delete_control])
+
 
 class LdapWrapper(object):
     def __init__(self, url):
@@ -362,3 +373,7 @@ class LdapWrapper(object):
     def delete_s(self, dn):
         LOG.debug("LDAP delete: dn=%s", dn)
         return self.conn.delete_s(dn)
+
+    def delete_ext_s(self, dn, serverctrls):
+        LOG.debug("LDAP delete_ext: dn=%s, serverctrls=%s", dn, serverctrls)
+        return self.conn.delete_ext_s(dn, serverctrls)

keystone/common/ldap/fakeldap.py

@@ -212,6 +212,20 @@ class FakeLdap(object):
             raise ldap.NO_SUCH_OBJECT
         self.db.sync()
 
+    def delete_ext_s(self, dn, serverctrls):
+        """Remove the ldap object at specified dn."""
+        if server_fail:
+            raise ldap.SERVER_DOWN
+
+        key = '%s%s' % (self.__prefix, dn)
+        LOG.debug('FakeLdap delete item: dn=%s', dn)
+        try:
+            del self.db[key]
+        except KeyError:
+            LOG.error('FakeLdap delete item failed: dn=%s not found.', dn)
+            raise ldap.NO_SUCH_OBJECT
+        self.db.sync()
+
     def modify_s(self, dn, attrs):
         """Modify the object at dn using the attribute list.
 

keystone/config.py

@@ -164,6 +164,7 @@ register_str('password', group='ldap', default='freeipa4all')
 register_str('suffix', group='ldap', default='cn=example,cn=com')
 register_bool('use_dumb_member', group='ldap', default=False)
 register_str('user_name_attribute', group='ldap', default='sn')
+register_bool('allow_subtree_delete', group='ldap', default=False)
 
 register_str('user_tree_dn', group='ldap', default=None)
 register_str('user_objectclass', group='ldap', default='inetOrgPerson')

keystone/identity/backends/ldap/core.py

@@ -572,7 +572,11 @@ class TenantApi(common_ldap.BaseLdap, ApiShimMixin):
         return list(res)
 
     def delete(self, id):
-        super(TenantApi, self).delete(id)
+        if self.subtree_delete_enabled:
+            super(TenantApi, self).deleteTree(id)
+        else:
+            self.role_api.roles_delete_subtree_by_tenant(id)
+            super(TenantApi, self).delete(id)
 
     def update(self, id, values):
         try:
@@ -894,6 +898,20 @@ class RoleApi(common_ldap.BaseLdap, ApiShimMixin):
                     tenant_id=tenant_id))
         return res
 
+    def roles_delete_subtree_by_tenant(self, tenant_id):
+        conn = self.get_connection()
+        query = '(objectClass=%s)' % self.object_class
+        tenant_dn = self.tenant_api._id_to_dn(tenant_id)
+        try:
+            roles = conn.search_s(tenant_dn, ldap.SCOPE_ONELEVEL, query)
+            for role_dn, _ in roles:
+                try:
+                    conn.delete_s(role_dn)
+                except Exception as inst:
+                    raise inst
+        except ldap.NO_SUCH_OBJECT:
+            pass
+
     def rolegrant_get_by_ids(self, user_id, role_id, tenant_id):
         conn = self.get_connection()
         user_dn = self.user_api._id_to_dn(user_id)

tests/test_backend.py

@@ -626,6 +626,16 @@ class IdentityTests(object):
         for test_tenant in default_fixtures.TENANTS:
             self.assertTrue(x for x in tenants if x['id'] == test_tenant['id'])
 
+    def test_delete_tenant_with_role_assignments(self):
+        tenant = {'id': 'fake1', 'name': 'fake1'}
+        self.identity_api.create_tenant('fake1', tenant)
+        self.identity_api.add_role_to_user_and_tenant(
+            self.user_foo['id'], tenant['id'], 'useless')
+        self.identity_api.delete_tenant(tenant['id'])
+        self.assertRaises(exception.NotFound,
+                          self.identity_api.get_tenant,
+                          tenant['id'])
+
 
 class TokenTests(object):
     def test_token_crud(self):