From 9e4fe654ed3f2fa4040ccbcd0ccc003f56f9bce2 Mon Sep 17 00:00:00 2001 From: Dolph Mathews Date: Sun, 25 Mar 2012 12:03:26 -0500 Subject: user-role-crud 404 (bug 963056) user-role-add user-role-remove Change-Id: I1b3cd019d0d110b01ed175822cdd6c9ddb486412 --- keystone/identity/backends/kvs.py | 4 ++++ keystone/identity/backends/sql.py | 4 ++++ keystone/identity/core.py | 14 ++++++++++++- tests/test_keystoneclient.py | 43 +++++++++++++++++++++++++++++++++++++++ 4 files changed, 64 insertions(+), 1 deletion(-) diff --git a/keystone/identity/backends/kvs.py b/keystone/identity/backends/kvs.py index 95286a9f..55bd8f08 100644 --- a/keystone/identity/backends/kvs.py +++ b/keystone/identity/backends/kvs.py @@ -144,6 +144,10 @@ class Identity(kvs.Base, identity.Driver): if not metadata_ref: metadata_ref = {} roles = set(metadata_ref.get('roles', [])) + if role_id not in roles: + msg = 'Cannot remove role that has not been granted, %s' % role_id + raise exception.RoleNotFound(message=msg) + roles.remove(role_id) metadata_ref['roles'] = list(roles) self.update_metadata(user_id, tenant_id, metadata_ref) diff --git a/keystone/identity/backends/sql.py b/keystone/identity/backends/sql.py index 00dedcba..443ddd3b 100644 --- a/keystone/identity/backends/sql.py +++ b/keystone/identity/backends/sql.py @@ -286,6 +286,10 @@ class Identity(sql.Base, identity.Driver): is_new = True metadata_ref = {} roles = set(metadata_ref.get('roles', [])) + if role_id not in roles: + msg = 'Cannot remove role that has not been granted, %s' % role_id + raise exception.RoleNotFound(message=msg) + roles.remove(role_id) metadata_ref['roles'] = list(roles) if not is_new: diff --git a/keystone/identity/core.py b/keystone/identity/core.py index c2a1041d..ee225264 100644 --- a/keystone/identity/core.py +++ b/keystone/identity/core.py @@ -497,6 +497,12 @@ class RoleController(wsgi.Application): if tenant_id is None: raise exception.NotImplemented(message='User roles not supported: ' 'tenant_id required') + if self.identity_api.get_user(context, user_id) is None: + raise exception.UserNotFound(user_id=user_id) + if self.identity_api.get_tenant(context, tenant_id) is None: + raise exception.TenantNotFound(tenant_id=tenant_id) + if self.identity_api.get_role(context, role_id) is None: + raise exception.RoleNotFound(role_id=role_id) # This still has the weird legacy semantics that adding a role to # a user also adds them to a tenant @@ -517,9 +523,15 @@ class RoleController(wsgi.Application): if tenant_id is None: raise exception.NotImplemented(message='User roles not supported: ' 'tenant_id required') + if self.identity_api.get_user(context, user_id) is None: + raise exception.UserNotFound(user_id=user_id) + if self.identity_api.get_tenant(context, tenant_id) is None: + raise exception.TenantNotFound(tenant_id=tenant_id) + if self.identity_api.get_role(context, role_id) is None: + raise exception.RoleNotFound(role_id=role_id) # This still has the weird legacy semantics that adding a role to - # a user also adds them to a tenant + # a user also adds them to a tenant, so we must follow up on that self.identity_api.remove_role_from_user_and_tenant( context, user_id, tenant_id, role_id) roles = self.identity_api.get_roles_for_user_and_tenant( diff --git a/tests/test_keystoneclient.py b/tests/test_keystoneclient.py index 95a06ecb..aa2a5f6d 100644 --- a/tests/test_keystoneclient.py +++ b/tests/test_keystoneclient.py @@ -670,6 +670,49 @@ class KcMasterTestCase(CompatTestCase, KeystoneClientTests): user_refs = client.tenants.list_users(tenant=self.tenant_baz['id']) self.assert_(self.user_foo['id'] not in [x.id for x in user_refs]) + def test_user_role_add_404(self): + from keystoneclient import exceptions as client_exceptions + client = self.get_client(admin=True) + self.assertRaises(client_exceptions.NotFound, + client.roles.add_user_role, + tenant=uuid.uuid4().hex, + user=self.user_foo['id'], + role=self.role_useless['id']) + self.assertRaises(client_exceptions.NotFound, + client.roles.add_user_role, + tenant=self.tenant_baz['id'], + user=uuid.uuid4().hex, + role=self.role_useless['id']) + self.assertRaises(client_exceptions.NotFound, + client.roles.add_user_role, + tenant=self.tenant_baz['id'], + user=self.user_foo['id'], + role=uuid.uuid4().hex) + + def test_user_role_remove_404(self): + from keystoneclient import exceptions as client_exceptions + client = self.get_client(admin=True) + self.assertRaises(client_exceptions.NotFound, + client.roles.remove_user_role, + tenant=uuid.uuid4().hex, + user=self.user_foo['id'], + role=self.role_useless['id']) + self.assertRaises(client_exceptions.NotFound, + client.roles.remove_user_role, + tenant=self.tenant_baz['id'], + user=uuid.uuid4().hex, + role=self.role_useless['id']) + self.assertRaises(client_exceptions.NotFound, + client.roles.remove_user_role, + tenant=self.tenant_baz['id'], + user=self.user_foo['id'], + role=uuid.uuid4().hex) + self.assertRaises(client_exceptions.NotFound, + client.roles.remove_user_role, + tenant=self.tenant_baz['id'], + user=self.user_foo['id'], + role=self.role_useless['id']) + def test_tenant_list_marker(self): client = self.get_client() -- cgit