From b068d71b59c092820b1e78dd87a3fb00b40802eb Mon Sep 17 00:00:00 2001 From: Lance Bragstad Date: Mon, 7 Aug 2017 20:29:08 +0000 Subject: [PATCH] Except forbidden when clearing default project IDs The identity backend registers a callback that listens for when a project is deleted. When it receives a notification, it uses the project ID send in the notification and removes all references to it from the identity backend, where users might have it referenced in their `default_project_id` attribute. The original fix for this did not account for LDAP backends being read-only. This caused an issue where DELETE /v3/projects/{project_id} actually caused an HTTP 403 Forbidden exception because the LDAP backend wasn't writeable, despite that project actually being deleted. This change makes the identity API manager handle the exception and tests it specifically for LDAP, or read-only, backends. Change-Id: I16f4fcb289dad2fe752f3188476329c95cf777c9 Closes-Bug: 1705081 --- keystone/identity/core.py | 10 +++++++++- keystone/tests/unit/test_backend_ldap.py | 20 ++++++++++++++++++++ 2 files changed, 29 insertions(+), 1 deletion(-) diff --git a/keystone/identity/core.py b/keystone/identity/core.py index b783836d85..afa397ba2f 100644 --- a/keystone/identity/core.py +++ b/keystone/identity/core.py @@ -530,7 +530,15 @@ class Manager(manager.Manager): """ project_id = payload['resource_info'] - self.driver.unset_default_project_id(project_id) + try: + self.driver.unset_default_project_id(project_id) + except exception.Forbidden: + # NOTE(lbragstad): If the driver throws a Forbidden, it's because + # the driver doesn't support writes. This is the case with the + # in-tree LDAP implementation since it is read-only. This also + # ensures consistency for out-of-tree backends that might be + # read-only. + pass # Domain ID normalization methods def _set_domain_id_and_mapping(self, ref, domain_id, driver, diff --git a/keystone/tests/unit/test_backend_ldap.py b/keystone/tests/unit/test_backend_ldap.py index 56eb1ee9bf..a553504c7d 100644 --- a/keystone/tests/unit/test_backend_ldap.py +++ b/keystone/tests/unit/test_backend_ldap.py @@ -31,6 +31,7 @@ from keystone.common import driver_hints import keystone.conf from keystone import exception from keystone import identity +from keystone.identity.backends import ldap as ldap_identity from keystone.identity.backends.ldap import common as common_ldap from keystone.identity.mapping_backends import mapping as map from keystone.tests import unit @@ -1723,6 +1724,25 @@ class LDAPIdentity(BaseLDAPIdentity, unit.TestCase): self.assertEqual('crap', user_ref['id']) self.assertEqual('Foo Bar', user_ref['name']) + def test_identity_manager_catches_forbidden_when_deleting_a_project(self): + # The identity API registers a callback that listens for notifications + # that a project has been deleted. When it receives one, it uses the ID + # and attempts to clear any users who have `default_project_id` + # attributes associated to that project. Since the LDAP backend is + # read-only, clearing the `default_project_id` requires a write which + # isn't possible. + project = unit.new_project_ref( + domain_id=CONF.identity.default_domain_id + ) + project = self.resource_api.create_project(project['id'], project) + with mock.patch.object( + ldap_identity.Identity, '_disallow_write' + ) as mocked: + mocked.side_effect = exception.Forbidden() + self.resource_api.delete_project(project['id']) + + mocked.assert_called_once() + class LDAPLimitTests(unit.TestCase, identity_tests.LimitTests): def setUp(self):