diff --git a/keystone/identity/core.py b/keystone/identity/core.py index 82e8864822..594503255d 100644 --- a/keystone/identity/core.py +++ b/keystone/identity/core.py @@ -1125,6 +1125,32 @@ class Manager(manager.Manager): if new_ref['domain_id'] != orig_ref['domain_id']: raise exception.ValidationError(_('Cannot change Domain ID')) + def _update_user_with_federated_objects(self, user, driver, entity_id): + # If the user did not pass a federated object along inside the user + # object then we simply update the user as normal and add the + # currently associated federated objects to user to be added to the + # dictionary. + if not user.get('federated'): + if 'federated' in user: + del user['federated'] + user = driver.update_user(entity_id, user) + fed_objects = self.shadow_users_api.get_federated_objects( + user['id']) + if fed_objects: + user['federated'] = fed_objects + return user + # Otherwise, we validate, remove the previous user's federated objects, + # and update the user along with their updated federated objects. + else: + user_ref = user.copy() + self._validate_federated_objects(user_ref['federated']) + self.shadow_users_api.delete_federated_object(entity_id) + del user['federated'] + user = driver.update_user(entity_id, user) + self._create_federated_objects(user, user_ref['federated']) + user['federated'] = user_ref['federated'] + return user + @domains_configured @exception_translated('user') def update_user(self, user_id, user_ref, initiator=None): @@ -1153,7 +1179,7 @@ class Manager(manager.Manager): self.get_user_by_name.invalidate(self, old_user_ref['name'], old_user_ref['domain_id']) - ref = driver.update_user(entity_id, user) + ref = self._update_user_with_federated_objects(user, driver, entity_id) notifications.Audit.updated(self._USER, user_id, initiator) diff --git a/keystone/identity/shadow_backends/base.py b/keystone/identity/shadow_backends/base.py index 12e7f545f1..134f86fec5 100644 --- a/keystone/identity/shadow_backends/base.py +++ b/keystone/identity/shadow_backends/base.py @@ -70,6 +70,13 @@ class ShadowUsersDriverBase(object, metaclass=abc.ABCMeta): """ raise exception.NotImplemented() + def delete_federated_object(self, user_id): + """Delete a user's federated objects. + + :param user_id: Unique identifier of the user + """ + raise exception.NotImplemented() + @abc.abstractmethod def get_federated_objects(self, user_id): """Get all federated objects for a user. diff --git a/keystone/identity/shadow_backends/sql.py b/keystone/identity/shadow_backends/sql.py index d45d325a58..628eb9b942 100644 --- a/keystone/identity/shadow_backends/sql.py +++ b/keystone/identity/shadow_backends/sql.py @@ -60,6 +60,12 @@ class ShadowUsers(base.ShadowUsersDriverBase): fed_ref = model.FederatedUser.from_dict(fed_dict) session.add(fed_ref) + def delete_federated_object(self, user_id): + with sql.session_for_write() as session: + q = session.query(model.FederatedUser) + q = q.filter(model.FederatedUser.user_id == user_id) + q.delete(False) + def get_federated_objects(self, user_id): with sql.session_for_read() as session: query = session.query(model.FederatedUser) diff --git a/keystone/tests/unit/test_shadow_users.py b/keystone/tests/unit/test_shadow_users.py index 2cbfe4629d..ef9514c115 100644 --- a/keystone/tests/unit/test_shadow_users.py +++ b/keystone/tests/unit/test_shadow_users.py @@ -149,3 +149,57 @@ class TestUserWithFederatedUser(ShadowUsersTests): self.protocol['id'], unique_id) self.assertIsNotNone(fed_user) + + def test_update_user_with_invalid_idp_and_protocol_fails(self): + baduser = unit.new_user_ref(domain_id=self.domain_id) + baduser['federated'] = [ + { + 'idp_id': 'fakeidp', + 'protocols': [ + { + 'protocol_id': 'nonexistent', + 'unique_id': 'unknown' + } + ] + } + ] + # Check validation works by throwing a federated object with + # invalid idp_id, protocol_id inside the user passed to create_user. + self.assertRaises(exception.ValidationError, + self.identity_api.create_user, + baduser) + + baduser['federated'][0]['idp_id'] = self.idp['id'] + self.assertRaises(exception.ValidationError, + self.identity_api.create_user, + baduser) + + def test_update_user_with_federated_attributes(self): + user = self.shadow_users_api.create_federated_user( + self.domain_id, self.federated_user) + user = self.identity_api.get_user(user['id']) + + # Test that update user can return a federated object with the user as + # a response if the user has any + user = self.identity_api.update_user(user['id'], user) + self.assertFederatedDictsEqual(self.federated_user, + user['federated'][0]) + + # Test that update user can replace a users federated objects if added + # in the request and that its response is that new federated objects + new_fed = [ + { + 'idp_id': self.idp['id'], + 'protocols': [ + { + 'protocol_id': self.protocol['id'], + 'unique_id': uuid.uuid4().hex + } + ] + } + ] + user['federated'] = new_fed + user = self.identity_api.update_user(user['id'], user) + self.assertTrue('federated' in user) + self.assertTrue(len(user['federated']), 1) + self.assertEqual(user['federated'][0], new_fed[0]) diff --git a/keystone/tests/unit/test_v3_identity.py b/keystone/tests/unit/test_v3_identity.py index 8441f49dbe..71e0ec4d00 100644 --- a/keystone/tests/unit/test_v3_identity.py +++ b/keystone/tests/unit/test_v3_identity.py @@ -1222,3 +1222,55 @@ class UserFederatedAttributesTests(test_v3.RestfulTestCase): ref['federated'][0]['idp_id'] = idp['id'] self.post('/users', body={'user': ref}, token=self.get_admin_token(), expected_status=http.client.BAD_REQUEST) + + def test_update_user_with_federated_attributes(self): + """Call ``PATCH /users/{user_id}``.""" + user = self.fed_user.copy() + del user['id'] + user['name'] = 'James Doe' + idp, protocol = self._create_federated_attributes() + user['federated'] = [ + { + 'idp_id': idp['id'], + 'protocols': [ + { + 'protocol_id': protocol['id'], + 'unique_id': 'jdoe' + } + ] + } + ] + r = self.patch('/users/%(user_id)s' % { + 'user_id': self.fed_user['id']}, + body={'user': user}) + resp_user = r.result['user'] + self.assertEqual(user['name'], resp_user['name']) + self.assertEqual(user['federated'], resp_user['federated']) + self.assertValidUserResponse(r, user) + + def test_update_user_fails_when_given_invalid_idp_and_protocols(self): + """Call ``PATCH /users/{user_id}``.""" + user = self.fed_user.copy() + del user['id'] + idp, protocol = self._create_federated_attributes() + user['federated'] = [ + { + 'idp_id': 'fakeidp', + 'protocols': [ + { + 'protocol_id': 'fakeprotocol_id', + 'unique_id': uuid.uuid4().hex + } + ] + } + ] + + self.patch('/users/%(user_id)s' % { + 'user_id': self.fed_user['id']}, + body={'user': user}, + expected_status=http.client.BAD_REQUEST) + user['federated'][0]['idp_id'] = idp['id'] + self.patch('/users/%(user_id)s' % { + 'user_id': self.fed_user['id']}, + body={'user': user}, + expected_status=http.client.BAD_REQUEST)