diff --git a/keystone/assignment/core.py b/keystone/assignment/core.py index 9d3641a69b..eccc22d0e6 100644 --- a/keystone/assignment/core.py +++ b/keystone/assignment/core.py @@ -349,25 +349,17 @@ class Manager(manager.Manager): domain_id=None, project_id=None, inherited_to_projects=False, context=None): if group_id is None: - self.revoke_api.revoke_by_grant(user_id=user_id, - role_id=role_id, - domain_id=domain_id, - project_id=project_id) + # check if role exists on the user before revoke + self.check_grant_role_id(role_id, user_id, None, domain_id, + project_id, inherited_to_projects) self._emit_revoke_user_grant( role_id, user_id, domain_id, project_id, inherited_to_projects, context) else: try: - # Group may contain a lot of users so revocation will be - # by role & domain/project - if domain_id is None: - self.revoke_api.revoke_by_project_role_assignment( - project_id, role_id - ) - else: - self.revoke_api.revoke_by_domain_role_assignment( - domain_id, role_id - ) + # check if role exists on the group before revoke + self.check_grant_role_id(role_id, None, group_id, domain_id, + project_id, inherited_to_projects) if CONF.token.revoke_by_id: # NOTE(morganfainberg): The user ids are the important part # for invalidating tokens below, so extract them here. @@ -379,7 +371,6 @@ class Manager(manager.Manager): except exception.GroupNotFound: LOG.debug('Group %s not found, no tokens to invalidate.', group_id) - # TODO(henry-nash): While having the call to get_role here mimics the # previous behavior (when it was buried inside the driver delete call), # this seems an odd place to have this check, given what we have diff --git a/keystone/tests/unit/assignment/test_backends.py b/keystone/tests/unit/assignment/test_backends.py index e3b34fa264..c19902eee0 100644 --- a/keystone/tests/unit/assignment/test_backends.py +++ b/keystone/tests/unit/assignment/test_backends.py @@ -1239,6 +1239,10 @@ class AssignmentTests(AssignmentTestHelperMixin): self.assertRaises(exception.RoleNotFound, f, role_id=uuid.uuid4().hex, **kwargs) + def assert_role_assignment_not_found_exception(f, **kwargs): + self.assertRaises(exception.RoleAssignmentNotFound, f, + role_id=uuid.uuid4().hex, **kwargs) + user = unit.new_user_ref(domain_id=CONF.identity.default_domain_id) user_resp = self.identity_api.create_user(user) group = unit.new_group_ref(domain_id=CONF.identity.default_domain_id) @@ -1248,8 +1252,7 @@ class AssignmentTests(AssignmentTestHelperMixin): project_resp = self.resource_api.create_project(project['id'], project) for manager_call in [self.assignment_api.create_grant, - self.assignment_api.get_grant, - self.assignment_api.delete_grant]: + self.assignment_api.get_grant]: assert_role_not_found_exception( manager_call, user_id=user_resp['id'], project_id=project_resp['id']) @@ -1265,6 +1268,21 @@ class AssignmentTests(AssignmentTestHelperMixin): group_id=group_resp['id'], domain_id=CONF.identity.default_domain_id) + assert_role_assignment_not_found_exception( + self.assignment_api.delete_grant, + user_id=user_resp['id'], project_id=project_resp['id']) + assert_role_assignment_not_found_exception( + self.assignment_api.delete_grant, + group_id=group_resp['id'], project_id=project_resp['id']) + assert_role_assignment_not_found_exception( + self.assignment_api.delete_grant, + user_id=user_resp['id'], + domain_id=CONF.identity.default_domain_id) + assert_role_assignment_not_found_exception( + self.assignment_api.delete_grant, + group_id=group_resp['id'], + domain_id=CONF.identity.default_domain_id) + def test_multi_role_grant_by_user_group_on_project_domain(self): role_list = [] for _ in range(10): diff --git a/keystone/tests/unit/test_v3_assignment.py b/keystone/tests/unit/test_v3_assignment.py index c7edcd773b..f94fe6397b 100644 --- a/keystone/tests/unit/test_v3_assignment.py +++ b/keystone/tests/unit/test_v3_assignment.py @@ -329,14 +329,12 @@ class AssignmentTestCase(test_v3.RestfulTestCase, self.head(member_url, expected_status=http_client.NOT_FOUND) def test_token_revoked_once_group_role_grant_revoked(self): - """Test token is revoked when group role grant is revoked. + """Test token invalid when direct & indirect role on user is revoked. When a role granted to a group is revoked for a given scope, - all tokens related to this scope and belonging to one of the members - of this group should be revoked. + and user direct role is revoked, then tokens created + by user will be invalid. - The revocation should be independently to the presence - of the revoke API. """ time = datetime.datetime.utcnow() with freezegun.freeze_time(time) as frozen_datetime: @@ -367,12 +365,15 @@ class AssignmentTestCase(test_v3.RestfulTestCase, self.assignment_api.delete_grant(role_id=self.role['id'], project_id=self.project['id'], group_id=self.group['id']) + # revokes the direct role form user on project + self.assignment_api.delete_grant(role_id=self.role['id'], + project_id=self.project['id'], + user_id=self.user['id']) frozen_datetime.tick(delta=datetime.timedelta(seconds=1)) # validates the same token again; it should not longer be valid. - self.head('/auth/tokens', - headers={'x-subject-token': token}, - expected_status=http_client.NOT_FOUND) + self.head('/auth/tokens', token=token, + expected_status=http_client.UNAUTHORIZED) @unit.skip_if_cache_disabled('assignment') def test_delete_grant_from_user_and_project_invalidate_cache(self): diff --git a/keystone/tests/unit/test_v3_auth.py b/keystone/tests/unit/test_v3_auth.py index c17cda8222..0cd4492298 100644 --- a/keystone/tests/unit/test_v3_auth.py +++ b/keystone/tests/unit/test_v3_auth.py @@ -3033,15 +3033,15 @@ class TestTokenRevokeById(test_v3.RestfulTestCase): Test Plan: - - Get a token for user1, scoped to ProjectA - - Delete the grant user1 has on ProjectA + - Get a token for user, scoped to Project + - Delete the grant user has on Project - Check token is no longer valid """ auth_data = self.build_authentication_request( - user_id=self.user1['id'], - password=self.user1['password'], - project_id=self.projectA['id']) + user_id=self.user['id'], + password=self.user['password'], + project_id=self.project['id']) token = self.get_requested_token(auth_data) # Confirm token is valid self.head('/auth/tokens', @@ -3051,13 +3051,12 @@ class TestTokenRevokeById(test_v3.RestfulTestCase): grant_url = ( '/projects/%(project_id)s/users/%(user_id)s/' 'roles/%(role_id)s' % { - 'project_id': self.projectA['id'], - 'user_id': self.user1['id'], - 'role_id': self.role1['id']}) + 'project_id': self.project['id'], + 'user_id': self.user['id'], + 'role_id': self.role['id']}) self.delete(grant_url) - self.head('/auth/tokens', - headers={'X-Subject-Token': token}, - expected_status=http_client.NOT_FOUND) + self.head('/auth/tokens', token=token, + expected_status=http_client.UNAUTHORIZED) def role_data_fixtures(self): self.projectC = unit.new_project_ref(domain_id=self.domainA['id']) @@ -3311,17 +3310,21 @@ class TestTokenRevokeById(test_v3.RestfulTestCase): 'group_id': self.group1['id'], 'role_id': self.role1['id']}) self.delete(grant_url) - self.head('/auth/tokens', - headers={'X-Subject-Token': token1}, - expected_status=http_client.NOT_FOUND) - self.head('/auth/tokens', - headers={'X-Subject-Token': token2}, - expected_status=http_client.NOT_FOUND) + self.assignment_api.delete_grant(role_id=self.role1['id'], + project_id=self.projectA['id'], + user_id=self.user1['id']) + self.assignment_api.delete_grant(role_id=self.role1['id'], + project_id=self.projectA['id'], + user_id=self.user2['id']) + self.head('/auth/tokens', token=token1, + expected_status=http_client.UNAUTHORIZED) + self.head('/auth/tokens', token=token2, + expected_status=http_client.UNAUTHORIZED) # But user3's token should be invalid too as revocation is done for # scope role & project self.head('/auth/tokens', headers={'X-Subject-Token': token3}, - expected_status=http_client.NOT_FOUND) + expected_status=http_client.OK) def test_domain_group_role_assignment_maintains_token(self): """Test domain-group role assignment maintains existing token.