From 3b701cdf70d7331c781a6b1a46dd8247906b9b63 Mon Sep 17 00:00:00 2001 From: wangxiyuan Date: Tue, 17 Apr 2018 19:21:43 +0800 Subject: [PATCH] Invalidate the shadow user cache when deleting a user When deleting a user, the cache for the related shadow user should be invalidated as well. Otherwise the federation authentication will not work well and will raise 404 UserNotFound error. This patch fixes the bug and adds a new function for shadow backend to get the shadow user information. Change-Id: I3882f0dc6e8f8f618bb89ebd699736bc4b352261 Closes-bug: #1760205 --- keystone/identity/core.py | 11 ++++++++++ keystone/identity/shadow_backends/base.py | 13 ++++++++++++ keystone/identity/shadow_backends/sql.py | 7 +++++++ keystone/tests/unit/test_v3_federation.py | 20 +++++++++++++++++++ .../notes/bug-1760205-87dedd6d8812db3f.yaml | 14 +++++++++++++ 5 files changed, 65 insertions(+) create mode 100644 releasenotes/notes/bug-1760205-87dedd6d8812db3f.yaml diff --git a/keystone/identity/core.py b/keystone/identity/core.py index 21ad023cf7..3a3a21f65a 100644 --- a/keystone/identity/core.py +++ b/keystone/identity/core.py @@ -1147,11 +1147,22 @@ class Manager(manager.Manager): self._get_domain_driver_and_entity_id(user_id)) # Get user details to invalidate the cache. user_old = self.get_user(user_id) + + hints = driver_hints.Hints() + hints.add_filter('user_id', user_id) + fed_users = PROVIDERS.shadow_users_api.list_federated_users_info(hints) + driver.delete_user(entity_id) PROVIDERS.assignment_api.delete_user_assignments(user_id) self.get_user.invalidate(self, user_id) self.get_user_by_name.invalidate(self, user_old['name'], user_old['domain_id']) + for fed_user in fed_users: + self.shadow_federated_user.invalidate( + self, fed_user['idp_id'], fed_user['protocol_id'], + fed_user['unique_id'], fed_user['display_name'], + user_old.get('extra', {}).get('email')) + PROVIDERS.credential_api.delete_credentials_for_user(user_id) PROVIDERS.id_mapping_api.delete_id_mapping(user_id) notifications.Audit.deleted(self._USER, user_id, initiator) diff --git a/keystone/identity/shadow_backends/base.py b/keystone/identity/shadow_backends/base.py index 0b7ea172bf..5f505338bd 100644 --- a/keystone/identity/shadow_backends/base.py +++ b/keystone/identity/shadow_backends/base.py @@ -88,3 +88,16 @@ class ShadowUsersDriverBase(object): """ raise exception.NotImplemented() + + @abc.abstractmethod + def list_federated_users_info(self, hints=None): + """Get the shadow users info with the specified filters. + + :param hints: contains the list of filters yet to be satisfied. + Any filters satisfied here will be removed so that + the caller will know if any filters remain. + :returns list: A list of objects that containing the shadow users + reference. + + """ + raise exception.NotImplemented() diff --git a/keystone/identity/shadow_backends/sql.py b/keystone/identity/shadow_backends/sql.py index e1e33667a7..e861acb799 100644 --- a/keystone/identity/shadow_backends/sql.py +++ b/keystone/identity/shadow_backends/sql.py @@ -170,3 +170,10 @@ class ShadowUsers(base.ShadowUsersDriverBase): if not user_ref: raise exception.UserNotFound(user_id=user_id) return user_ref + + def list_federated_users_info(self, hints=None): + with sql.session_for_read() as session: + query = session.query(model.FederatedUser) + fed_user_refs = sql.filter_limit_query(model.FederatedUser, query, + hints) + return [x.to_dict() for x in fed_user_refs] diff --git a/keystone/tests/unit/test_v3_federation.py b/keystone/tests/unit/test_v3_federation.py index f480983274..a890aa3548 100644 --- a/keystone/tests/unit/test_v3_federation.py +++ b/keystone/tests/unit/test_v3_federation.py @@ -1901,6 +1901,26 @@ class FederatedTokenTests(test_v3.RestfulTestCase, FederatedSetupMixin): self.assertIsNotNone(r.headers.get('X-Subject-Token')) self.assertValidMappedUser(r.json['token']) + def test_issue_the_same_unscoped_token_with_user_deleted(self): + r = self._issue_unscoped_token() + token = r.json['token'] + user1 = token['user'] + user_id1 = user1.pop('id') + + # delete the referenced user, and authenticate again. Keystone should + # create another new shadow user. + PROVIDERS.identity_api.delete_user(user_id1) + + r = self._issue_unscoped_token() + token = r.json['token'] + user2 = token['user'] + user_id2 = user2.pop('id') + + # Only the user_id is different. Other properties include + # identity_provider, protocol, groups and domain are the same. + self.assertIsNot(user_id2, user_id1) + self.assertEqual(user1, user2) + def test_issue_unscoped_token_disabled_idp(self): """Check if authentication works with disabled identity providers. diff --git a/releasenotes/notes/bug-1760205-87dedd6d8812db3f.yaml b/releasenotes/notes/bug-1760205-87dedd6d8812db3f.yaml new file mode 100644 index 0000000000..a40fdf37af --- /dev/null +++ b/releasenotes/notes/bug-1760205-87dedd6d8812db3f.yaml @@ -0,0 +1,14 @@ +--- +fixes: + - | + [`bug 1760205 `_] + When deleting a shadow user, the related cache info is not invalidated so + that Keystone will raise 404 UserNotFound error when authenticating with + the previous federation info. This bug has been fixed now. + +other: + - | + A new interface called `list_federated_users_info` is added to shadow + backend. It's used to get the shadow user information internally. If you + are maintaining any out-tree shadow backends, please implement this + function for them as well.