From 7f42af363be92c6e4a46cfb3ff1a279933fc9299 Mon Sep 17 00:00:00 2001 From: YaHong Du Date: Tue, 24 Sep 2013 14:40:39 +0800 Subject: [PATCH] Add user to project if project ID is changed When an user updates project ID, it is necessary to make sure the new tenant is available. Remove the member role from the old tenant. Add the member role to the new tenant. Fixes bug 1201251 Change-Id: I0c2c8971039f567180fd910e959e4a1f0574034d --- keystone/identity/controllers.py | 5 +- keystone/tests/test_content_types.py | 245 +++++++++++++++++++++++++++ 2 files changed, 248 insertions(+), 2 deletions(-) diff --git a/keystone/identity/controllers.py b/keystone/identity/controllers.py index 73c5f02019..a0b0ee9bd0 100644 --- a/keystone/identity/controllers.py +++ b/keystone/identity/controllers.py @@ -252,8 +252,9 @@ def update_user(self, context, user_id, user): if user_ref['tenantId'] != old_user_ref.get('tenantId'): if old_user_ref.get('tenantId'): try: - self.assignment_api.remove_user_from_project( - old_user_ref['tenantId'], user_id) + member_role_id = config.CONF.member_role_id + self.assignment_api.remove_role_from_user_and_project( + user_id, old_user_ref['tenantId'], member_role_id) except exception.NotFound: # NOTE(morganfainberg): This is not a critical error it # just means that the user cannot be removed from the diff --git a/keystone/tests/test_content_types.py b/keystone/tests/test_content_types.py index 989597ceaa..86952d0521 100644 --- a/keystone/tests/test_content_types.py +++ b/keystone/tests/test_content_types.py @@ -22,10 +22,13 @@ from keystone.common import extension from keystone.common import serializer +from keystone import config from keystone.openstack.common import jsonutils from keystone import tests from keystone.tests import default_fixtures +CONF = config.CONF + class RestfulTestCase(tests.TestCase): """Performs restful tests against the WSGI app over HTTP. @@ -597,14 +600,241 @@ def test_invalid_parameter_error_response(self): expected_status=400) self.assertValidErrorResponse(res) + def _get_user_id(self, r): + """Helper method to return user ID from a response. + + This needs to be overridden by child classes + based on their content type. + + """ + raise NotImplementedError() + + def _get_role_id(self, r): + """Helper method to return a role ID from a response. + + This needs to be overridden by child classes + based on their content type. + + """ + raise NotImplementedError() + + def _get_role_name(self, r): + """Helper method to return role NAME from a response. + + This needs to be overridden by child classes + based on their content type. + + """ + raise NotImplementedError() + + def _get_project_id(self, r): + """Helper method to return project ID from a response. + + This needs to be overridden by child classes + based on their content type. + + """ + raise NotImplementedError() + + def assertNoRoles(self, r): + """Helper method to assert No Roles + + This needs to be overridden by child classes + based on their content type. + + """ + raise NotImplementedError() + + def test_update_user_tenant(self): + token = self.get_scoped_token() + + # Create a new user + r = self.admin_request( + method='POST', + path='/v2.0/users', + body={ + 'user': { + 'name': uuid.uuid4().hex, + 'password': uuid.uuid4().hex, + 'tenantId': self.tenant_bar['id'], + 'enabled': True, + }, + }, + token=token, + expected_status=200) + + user_id = self._get_user_id(r.result) + + # Check if member_role is in tenant_bar + r = self.admin_request( + path='/v2.0/tenants/%(project_id)s/users/%(user_id)s/roles' % { + 'project_id': self.tenant_bar['id'], + 'user_id': user_id + }, + token=token, + expected_status=200) + self.assertEqual(self._get_role_name(r.result), CONF.member_role_name) + + # Create a new tenant + r = self.admin_request( + method='POST', + path='/v2.0/tenants', + body={ + 'tenant': { + 'name': 'test_update_user', + 'description': 'A description ...', + 'enabled': True, + }, + }, + token=token, + expected_status=200) + + project_id = self._get_project_id(r.result) + + # Update user's tenant + r = self.admin_request( + method='PUT', + path='/v2.0/users/%(user_id)s' % { + 'user_id': user_id, + }, + body={ + 'user': { + 'tenantId': project_id, + }, + }, + token=token, + expected_status=200) + + # 'member_role' should be in new_tenant + r = self.admin_request( + path='/v2.0/tenants/%(project_id)s/users/%(user_id)s/roles' % { + 'project_id': project_id, + 'user_id': user_id + }, + token=token, + expected_status=200) + self.assertEqual(self._get_role_name(r.result), '_member_') + + # 'member_role' should not be in tenant_bar any more + r = self.admin_request( + path='/v2.0/tenants/%(project_id)s/users/%(user_id)s/roles' % { + 'project_id': self.tenant_bar['id'], + 'user_id': user_id + }, + token=token, + expected_status=200) + self.assertNoRoles(r.result) + + def test_update_user_with_invalid_tenant(self): + token = self.get_scoped_token() + + # Create a new user + r = self.admin_request( + method='POST', + path='/v2.0/users', + body={ + 'user': { + 'name': 'test_invalid_tenant', + 'password': uuid.uuid4().hex, + 'tenantId': self.tenant_bar['id'], + 'enabled': True, + }, + }, + token=token, + expected_status=200) + user_id = self._get_user_id(r.result) + + # Update user with an invalid tenant + r = self.admin_request( + method='PUT', + path='/v2.0/users/%(user_id)s' % { + 'user_id': user_id, + }, + body={ + 'user': { + 'tenantId': 'abcde12345heha', + }, + }, + token=token, + expected_status=404) + + def test_update_user_with_old_tenant(self): + token = self.get_scoped_token() + + # Create a new user + r = self.admin_request( + method='POST', + path='/v2.0/users', + body={ + 'user': { + 'name': uuid.uuid4().hex, + 'password': uuid.uuid4().hex, + 'tenantId': self.tenant_bar['id'], + 'enabled': True, + }, + }, + token=token, + expected_status=200) + + user_id = self._get_user_id(r.result) + + # Check if member_role is in tenant_bar + r = self.admin_request( + path='/v2.0/tenants/%(project_id)s/users/%(user_id)s/roles' % { + 'project_id': self.tenant_bar['id'], + 'user_id': user_id + }, + token=token, + expected_status=200) + self.assertEqual(self._get_role_name(r.result), CONF.member_role_name) + + # Update user's tenant with old tenant id + r = self.admin_request( + method='PUT', + path='/v2.0/users/%(user_id)s' % { + 'user_id': user_id, + }, + body={ + 'user': { + 'tenantId': self.tenant_bar['id'], + }, + }, + token=token, + expected_status=200) + + # 'member_role' should still be in tenant_bar + r = self.admin_request( + path='/v2.0/tenants/%(project_id)s/users/%(user_id)s/roles' % { + 'project_id': self.tenant_bar['id'], + 'user_id': user_id + }, + token=token, + expected_status=200) + self.assertEqual(self._get_role_name(r.result), '_member_') + class JsonTestCase(RestfulTestCase, CoreApiTests): content_type = 'json' + def _get_user_id(self, r): + return r['user']['id'] + + def _get_role_name(self, r): + return r['roles'][0]['name'] + + def _get_role_id(self, r): + return r['roles'][0]['id'] + + def _get_project_id(self, r): + return r['tenant']['id'] + def _get_token_id(self, r): """Applicable only to JSON.""" return r.result['access']['token']['id'] + def assertNoRoles(self, r): + self.assertEqual(r['roles'], []) + def assertValidErrorResponse(self, r): self.assertIsNotNone(r.result.get('error')) self.assertValidError(r.result['error']) @@ -846,6 +1076,21 @@ class XmlTestCase(RestfulTestCase, CoreApiTests): xmlns = 'http://docs.openstack.org/identity/api/v2.0' content_type = 'xml' + def _get_user_id(self, r): + return r.get('id') + + def _get_role_name(self, r): + return r[0].get('name') + + def _get_role_id(self, r): + return r[0].get('id') + + def _get_project_id(self, r): + return r.get('id') + + def assertNoRoles(self, r): + self.assertEqual(len(r), 0) + def _get_token_id(self, r): return r.result.find(self._tag('token')).get('id')