Group role revocation invalidates all user tokens
Keystone invalidates every token for a user after revoking one group role within one project. This patch replaces 'invalidate user's everything' logic with revocation by grant via notifications for delete_grant assignment operation. Change-Id: I7af205757232f19724b1bf124399d4e206aa2003 Closes-Bug: #1402760 Closes-Bug: #1401926
This commit is contained in:
parent
d16ffa00b1
commit
2cf743d6de
|
@ -426,6 +426,11 @@ class Manager(manager.Manager):
|
|||
def _emit_invalidate_user_token_persistence(self, user_id):
|
||||
self.identity_api.emit_invalidate_user_token_persistence(user_id)
|
||||
|
||||
def _emit_invalidate_grant_token_persistence(self, user_id, project_id):
|
||||
self.identity_api.emit_invalidate_grant_token_persistence(
|
||||
{'user_id': user_id, 'project_id': project_id}
|
||||
)
|
||||
|
||||
@notifications.role_assignment('created')
|
||||
def create_grant(self, role_id, user_id=None, group_id=None,
|
||||
domain_id=None, project_id=None,
|
||||
|
@ -463,6 +468,10 @@ class Manager(manager.Manager):
|
|||
return self.role_api.list_roles_from_ids(grant_ids)
|
||||
|
||||
@notifications.role_assignment('deleted')
|
||||
def _emit_revoke_user_grant(self, role_id, user_id, domain_id, project_id,
|
||||
inherited_to_projects, context):
|
||||
self._emit_invalidate_grant_token_persistence(user_id, project_id)
|
||||
|
||||
def delete_grant(self, role_id, user_id=None, group_id=None,
|
||||
domain_id=None, project_id=None,
|
||||
inherited_to_projects=False, context=None):
|
||||
|
@ -471,15 +480,29 @@ class Manager(manager.Manager):
|
|||
role_id=role_id,
|
||||
domain_id=domain_id,
|
||||
project_id=project_id)
|
||||
self._emit_revoke_user_grant(
|
||||
role_id, user_id, domain_id, project_id,
|
||||
inherited_to_projects, context)
|
||||
else:
|
||||
try:
|
||||
# NOTE(morganfainberg): The user ids are the important part
|
||||
# for invalidating tokens below, so extract them here.
|
||||
for user in self.identity_api.list_users_in_group(group_id):
|
||||
if user['id'] != user_id:
|
||||
self.revoke_api.revoke_by_grant(
|
||||
user_id=user['id'], role_id=role_id,
|
||||
domain_id=domain_id, project_id=project_id)
|
||||
# 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
|
||||
)
|
||||
if CONF.token.revoke_by_id:
|
||||
# NOTE(morganfainberg): The user ids are the important part
|
||||
# for invalidating tokens below, so extract them here.
|
||||
for user in self.identity_api.list_users_in_group(
|
||||
group_id):
|
||||
self._emit_revoke_user_grant(
|
||||
role_id, user['id'], domain_id, project_id,
|
||||
inherited_to_projects, context)
|
||||
except exception.GroupNotFound:
|
||||
LOG.debug('Group %s not found, no tokens to invalidate.',
|
||||
group_id)
|
||||
|
@ -496,8 +519,6 @@ class Manager(manager.Manager):
|
|||
self.resource_api.get_project(project_id)
|
||||
self.driver.delete_grant(role_id, user_id, group_id, domain_id,
|
||||
project_id, inherited_to_projects)
|
||||
if user_id is not None:
|
||||
self._emit_invalidate_user_token_persistence(user_id)
|
||||
|
||||
def delete_tokens_for_role_assignments(self, role_id):
|
||||
assignments = self.list_role_assignments_for_role(role_id=role_id)
|
||||
|
|
|
@ -111,11 +111,12 @@ class Manager(manager.Manager):
|
|||
self.revoke(
|
||||
model.RevokeEvent(access_token_id=payload['resource_info']))
|
||||
|
||||
def _group_callback(self, service, resource_type, operation, payload):
|
||||
user_ids = (u['id'] for u in self.identity_api.list_users_in_group(
|
||||
payload['resource_info']))
|
||||
for uid in user_ids:
|
||||
self.revoke(model.RevokeEvent(user_id=uid))
|
||||
def _role_assignment_callback(self, service, resource_type, operation,
|
||||
payload):
|
||||
info = payload['resource_info']
|
||||
self.revoke_by_grant(role_id=info['role_id'], user_id=info['user_id'],
|
||||
domain_id=info.get('domain_id'),
|
||||
project_id=info.get('project_id'))
|
||||
|
||||
def _register_listeners(self):
|
||||
callbacks = {
|
||||
|
@ -126,6 +127,7 @@ class Manager(manager.Manager):
|
|||
['role', self._role_callback],
|
||||
['user', self._user_callback],
|
||||
['project', self._project_callback],
|
||||
['role_assignment', self._role_assignment_callback]
|
||||
],
|
||||
notifications.ACTIONS.disabled: [
|
||||
['user', self._user_callback],
|
||||
|
|
|
@ -971,6 +971,19 @@ class Manager(manager.Manager):
|
|||
"""
|
||||
pass
|
||||
|
||||
@notifications.internal(
|
||||
notifications.INVALIDATE_USER_PROJECT_TOKEN_PERSISTENCE)
|
||||
def emit_invalidate_grant_token_persistence(self, user_project):
|
||||
"""Emit a notification to the callback system to revoke grant tokens.
|
||||
|
||||
This method and associated callback listener removes the need for
|
||||
making a direct call to another manager to delete and revoke tokens.
|
||||
|
||||
:param user_project: {'user_id': user_id, 'project_id': project_id}
|
||||
:type user_project: dict
|
||||
"""
|
||||
pass
|
||||
|
||||
@manager.response_truncated
|
||||
@domains_configured
|
||||
@exception_translated('user')
|
||||
|
|
|
@ -817,10 +817,10 @@ class CadfNotificationsWrapperTestCase(test_v3.RestfulTestCase):
|
|||
self.assertEqual(project, event.project)
|
||||
if domain:
|
||||
self.assertEqual(domain, event.domain)
|
||||
if user:
|
||||
self.assertEqual(user, event.user)
|
||||
if group:
|
||||
self.assertEqual(group, event.group)
|
||||
elif user:
|
||||
self.assertEqual(user, event.user)
|
||||
self.assertEqual(role_id, event.role)
|
||||
self.assertEqual(inherit, event.inherited_to_projects)
|
||||
|
||||
|
@ -867,7 +867,7 @@ class CadfNotificationsWrapperTestCase(test_v3.RestfulTestCase):
|
|||
event_type = '%s.%s.%s' % (notifications.SERVICE,
|
||||
self.ROLE_ASSIGNMENT, DELETED_OPERATION)
|
||||
self._assert_last_note(action, self.user_id, event_type)
|
||||
self._assert_event(role, project, domain, user, group)
|
||||
self._assert_event(role, project, domain, user, None)
|
||||
|
||||
def test_user_project_grant(self):
|
||||
url = ('/projects/%s/users/%s/roles/%s' %
|
||||
|
@ -879,10 +879,12 @@ class CadfNotificationsWrapperTestCase(test_v3.RestfulTestCase):
|
|||
def test_group_domain_grant(self):
|
||||
group_ref = self.new_group_ref(domain_id=self.domain_id)
|
||||
group = self.identity_api.create_group(group_ref)
|
||||
self.identity_api.add_user_to_group(self.user_id, group['id'])
|
||||
url = ('/domains/%s/groups/%s/roles/%s' %
|
||||
(self.domain_id, group['id'], self.role_id))
|
||||
self._test_role_assignment(url, self.role_id,
|
||||
domain=self.domain_id,
|
||||
user=self.user_id,
|
||||
group=group['id'])
|
||||
|
||||
def test_add_role_to_user_and_project(self):
|
||||
|
|
|
@ -35,7 +35,6 @@ from keystone.tests import unit as tests
|
|||
from keystone.tests.unit import ksfixtures
|
||||
from keystone.tests.unit import test_v3
|
||||
|
||||
|
||||
CONF = cfg.CONF
|
||||
|
||||
|
||||
|
@ -1053,7 +1052,7 @@ class TestTokenRevokeById(test_v3.RestfulTestCase):
|
|||
- Delete the grant group1 has on ProjectA
|
||||
- Check tokens for user1 & user2 are no longer valid,
|
||||
since user1 and user2 are members of group1
|
||||
- Check token for user3 is still valid
|
||||
- Check token for user3 is invalid too
|
||||
|
||||
"""
|
||||
auth_data = self.build_authentication_request(
|
||||
|
@ -1096,10 +1095,11 @@ class TestTokenRevokeById(test_v3.RestfulTestCase):
|
|||
self.head('/auth/tokens',
|
||||
headers={'X-Subject-Token': token2},
|
||||
expected_status=404)
|
||||
# But user3's token should still be valid
|
||||
# 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=200)
|
||||
expected_status=404)
|
||||
|
||||
def test_domain_group_role_assignment_maintains_token(self):
|
||||
"""Test domain-group role assignment maintains existing token.
|
||||
|
@ -1186,6 +1186,14 @@ class TestTokenRevokeById(test_v3.RestfulTestCase):
|
|||
|
||||
def test_removing_role_assignment_does_not_affect_other_users(self):
|
||||
"""Revoking a role from one user should not affect other users."""
|
||||
|
||||
# This group grant is not needed for the test
|
||||
self.delete(
|
||||
'/projects/%(project_id)s/groups/%(group_id)s/roles/%(role_id)s' %
|
||||
{'project_id': self.projectA['id'],
|
||||
'group_id': self.group1['id'],
|
||||
'role_id': self.role1['id']})
|
||||
|
||||
user1_token = self.get_requested_token(
|
||||
self.build_authentication_request(
|
||||
user_id=self.user1['id'],
|
||||
|
@ -1204,12 +1212,6 @@ class TestTokenRevokeById(test_v3.RestfulTestCase):
|
|||
'project_id': self.projectA['id'],
|
||||
'user_id': self.user1['id'],
|
||||
'role_id': self.role1['id']})
|
||||
self.delete(
|
||||
'/projects/%(project_id)s/groups/%(group_id)s/roles/%(role_id)s' %
|
||||
{'project_id': self.projectA['id'],
|
||||
'group_id': self.group1['id'],
|
||||
'role_id': self.role1['id']})
|
||||
|
||||
# authorization for the first user should now fail
|
||||
self.head('/auth/tokens',
|
||||
headers={'X-Subject-Token': user1_token},
|
||||
|
@ -1368,6 +1370,58 @@ class TestTokenRevokeById(test_v3.RestfulTestCase):
|
|||
expected_status=200)
|
||||
|
||||
|
||||
class TestTokenRevokeByAssignment(TestTokenRevokeById):
|
||||
|
||||
def config_overrides(self):
|
||||
super(TestTokenRevokeById, self).config_overrides()
|
||||
self.config_fixture.config(
|
||||
group='revoke',
|
||||
driver='kvs')
|
||||
self.config_fixture.config(
|
||||
group='token',
|
||||
provider='uuid',
|
||||
revoke_by_id=True)
|
||||
|
||||
def test_removing_role_assignment_keeps_other_project_token_groups(self):
|
||||
"""Test assignment isolation.
|
||||
|
||||
Revoking a group role from one project should not invalidate all group
|
||||
users' tokens
|
||||
"""
|
||||
self.assignment_api.create_grant(self.role1['id'],
|
||||
group_id=self.group1['id'],
|
||||
project_id=self.projectB['id'])
|
||||
|
||||
project_token = self.get_requested_token(
|
||||
self.build_authentication_request(
|
||||
user_id=self.user1['id'],
|
||||
password=self.user1['password'],
|
||||
project_id=self.projectB['id']))
|
||||
|
||||
other_project_token = self.get_requested_token(
|
||||
self.build_authentication_request(
|
||||
user_id=self.user1['id'],
|
||||
password=self.user1['password'],
|
||||
project_id=self.projectA['id']))
|
||||
|
||||
self.assignment_api.delete_grant(self.role1['id'],
|
||||
group_id=self.group1['id'],
|
||||
project_id=self.projectB['id'])
|
||||
|
||||
# authorization for the projectA should still succeed
|
||||
self.head('/auth/tokens',
|
||||
headers={'X-Subject-Token': other_project_token},
|
||||
expected_status=200)
|
||||
# while token for the projectB should not
|
||||
self.head('/auth/tokens',
|
||||
headers={'X-Subject-Token': project_token},
|
||||
expected_status=404)
|
||||
revoked_tokens = [
|
||||
t['id'] for t in self.token_provider_api.list_revoked_tokens()]
|
||||
# token is in token revocation list
|
||||
self.assertIn(project_token, revoked_tokens)
|
||||
|
||||
|
||||
class TestTokenRevokeApi(TestTokenRevokeById):
|
||||
EXTENSION_NAME = 'revoke'
|
||||
EXTENSION_TO_ADD = 'revoke_extension'
|
||||
|
|
Loading…
Reference in New Issue