diff --git a/keystone/contrib/revoke/core.py b/keystone/contrib/revoke/core.py index 2f1c415ede..8f3cb06c0c 100644 --- a/keystone/contrib/revoke/core.py +++ b/keystone/contrib/revoke/core.py @@ -128,10 +128,21 @@ class Manager(manager.Manager): def revoke_by_user(self, user_id): return self.revoke(model.RevokeEvent(user_id=user_id)) - def revoke_by_expiration(self, user_id, expires_at): + def revoke_by_expiration(self, user_id, expires_at, + domain_id=None, project_id=None): + + if domain_id is not None and project_id is not None: + msg = _('The call to keystone.contrib.revoke.Manager ' + 'revoke_by_expiration() must not have both domain_id and ' + 'project_id. This is a bug in the keystone server. The ' + 'current request is aborted.') + raise exception.UnexpectedError(exception=msg) + self.revoke( model.RevokeEvent(user_id=user_id, - expires_at=expires_at)) + expires_at=expires_at, + domain_id=domain_id, + project_id=project_id)) def revoke_by_grant(self, role_id, user_id=None, domain_id=None, project_id=None): diff --git a/keystone/contrib/revoke/model.py b/keystone/contrib/revoke/model.py index 84cab54ad0..3ab25b384a 100644 --- a/keystone/contrib/revoke/model.py +++ b/keystone/contrib/revoke/model.py @@ -27,6 +27,10 @@ _NAMES = ['trust_id', # Additional arguments for creating a RevokeEvent _EVENT_ARGS = ['issued_before', 'revoked_at'] +# Names of attributes in the RevocationEvent, including "virtual" attributes. +# Virtual attributes are those added based on other values. +_EVENT_NAMES = _NAMES + ['domain_scope_id'] + # Values that will be in the token data but not in the event. # These will compared with event values that have different names. # For example: both trustor_id and trustee_id are compared against user_id @@ -56,6 +60,15 @@ class RevokeEvent(object): for k in REVOKE_KEYS: v = kwargs.get(k, None) setattr(self, k, v) + + if self.domain_id and self.expires_at: + # This is revoking a domain-scoped token. + self.domain_scope_id = self.domain_id + self.domain_id = None + else: + # This is revoking all tokens for a domain. + self.domain_scope_id = None + if self.revoked_at is None: self.revoked_at = timeutils.utcnow() if self.issued_before is None: @@ -65,7 +78,9 @@ class RevokeEvent(object): keys = ['user_id', 'role_id', 'domain_id', - 'project_id'] + 'domain_scope_id', + 'project_id', + ] event = dict((key, self.__dict__[key]) for key in keys if self.__dict__[key] is not None) if self.trust_id is not None: @@ -87,7 +102,7 @@ class RevokeEvent(object): def attr_keys(event): - return map(event.key_for_name, _NAMES) + return map(event.key_for_name, _EVENT_NAMES) class RevokeTree(object): @@ -137,7 +152,7 @@ class RevokeTree(object): """ stack = [] revoke_map = self.revoke_map - for name in _NAMES: + for name in _EVENT_NAMES: key = event.key_for_name(name) nxt = revoke_map.get(key) if nxt is None: @@ -176,11 +191,13 @@ class RevokeTree(object): alternatives = { 'user_id': ['user_id', 'trustor_id', 'trustee_id'], 'domain_id': ['identity_domain_id', 'assignment_domain_id'], + # For a domain-scoped token, the domain is in assignment_domain_id. + 'domain_scope_id': ['assignment_domain_id', ], } # Contains current forest (collection of trees) to be checked. partial_matches = [self.revoke_map] # We iterate over every layer of our revoke tree (except the last one). - for name in _NAMES: + for name in _EVENT_NAMES: # bundle is the set of partial matches for the next level down # the tree bundle = [] diff --git a/keystone/tests/test_revoke.py b/keystone/tests/test_revoke.py index 7fbc45a734..b353de77e2 100644 --- a/keystone/tests/test_revoke.py +++ b/keystone/tests/test_revoke.py @@ -15,6 +15,7 @@ import datetime import uuid import mock +from testtools import matchers from keystone.common import dependency from keystone import config @@ -85,6 +86,10 @@ def _matches(event, token_values): else: return False + if event.domain_scope_id is not None: + if event.domain_scope_id != token_values['assignment_domain_id']: + return False + # If any one check does not match, the while token does # not match the event. The numerous return False indicate # that the token is still valid and short-circuits the @@ -166,6 +171,17 @@ class RevokeTests(object): # should no longer throw an exception self.revoke_api.check_token(token_values) + def test_revoke_by_expiration_project_and_domain_fails(self): + user_id = _new_id() + expires_at = timeutils.isotime(_future_time(), subsecond=True) + domain_id = _new_id() + project_id = _new_id() + self.assertThat( + lambda: self.revoke_api.revoke_by_expiration( + user_id, expires_at, domain_id=domain_id, + project_id=project_id), + matchers.raises(exception.UnexpectedError)) + class SqlRevokeTests(test_backend_sql.SqlTests, RevokeTests): def config_overrides(self): @@ -255,10 +271,13 @@ class RevokeTreeTests(tests.TestCase): return self.tree.add_event( model.RevokeEvent(user_id=user_id)) - def _revoke_by_expiration(self, user_id, expires_at): + def _revoke_by_expiration(self, user_id, expires_at, project_id=None, + domain_id=None): event = self.tree.add_event( model.RevokeEvent(user_id=user_id, - expires_at=expires_at)) + expires_at=expires_at, + project_id=project_id, + domain_id=domain_id)) self.events.append(event) return event @@ -340,6 +359,40 @@ class RevokeTreeTests(tests.TestCase): self.removeEvent(event) self._assertTokenNotRevoked(token_data_1) + def test_by_user_project(self): + # When a user has a project-scoped token and the project-scoped token + # is revoked then the token is revoked. + + user_id = _new_id() + project_id = _new_id() + + future_time = _future_time() + + token_data = _sample_blank_token() + token_data['user_id'] = user_id + token_data['project_id'] = project_id + token_data['expires_at'] = future_time + + self._revoke_by_expiration(user_id, future_time, project_id=project_id) + self._assertTokenRevoked(token_data) + + def test_by_user_domain(self): + # When a user has a domain-scoped token and the domain-scoped token + # is revoked then the token is revoked. + + user_id = _new_id() + domain_id = _new_id() + + future_time = _future_time() + + token_data = _sample_blank_token() + token_data['user_id'] = user_id + token_data['assignment_domain_id'] = domain_id + token_data['expires_at'] = future_time + + self._revoke_by_expiration(user_id, future_time, domain_id=domain_id) + self._assertTokenRevoked(token_data) + def removeEvent(self, event): self.events.remove(event) self.tree.remove_event(event) diff --git a/keystone/tests/test_v3_auth.py b/keystone/tests/test_v3_auth.py index d46ba6f875..a14f5a778c 100644 --- a/keystone/tests/test_v3_auth.py +++ b/keystone/tests/test_v3_auth.py @@ -1268,9 +1268,6 @@ class TestTokenRevokeById(test_v3.RestfulTestCase): # the scoped token can be revoked, and the unscoped token remains # valid. - # FIXME(blk-u): This isn't working correctly. The unscoped token should - # remain valid. See bug 1347318. - unscoped_token = self.get_requested_token( self.build_authentication_request( user_id=self.user1['id'], @@ -1301,12 +1298,12 @@ class TestTokenRevokeById(test_v3.RestfulTestCase): # The unscoped token should still be valid. self.head('/auth/tokens', headers={'X-Subject-Token': unscoped_token}, - expected_status=404) # FIXME(blk-u): This should be 200! + expected_status=200) # The domain-scoped token should still be valid. self.head('/auth/tokens', headers={'X-Subject-Token': domain_scoped_token}, - expected_status=404) # FIXME(blk-u): This should be 200! + expected_status=200) # revoke the domain-scoped token. self.delete('/auth/tokens', @@ -1321,7 +1318,7 @@ class TestTokenRevokeById(test_v3.RestfulTestCase): # The unscoped token should still be valid. self.head('/auth/tokens', headers={'X-Subject-Token': unscoped_token}, - expected_status=404) # FIXME(blk-u): This should be 200! + expected_status=200) def test_revoke_token_from_token_v2(self): # Test that a scoped token can be requested from an unscoped token, @@ -1350,7 +1347,7 @@ class TestTokenRevokeById(test_v3.RestfulTestCase): # The unscoped token should still be valid. self.head('/auth/tokens', headers={'X-Subject-Token': unscoped_token}, - expected_status=404) # FIXME(blk-u): This should be 200! + expected_status=200) @dependency.requires('revoke_api') @@ -1391,17 +1388,24 @@ class TestTokenRevokeApi(TestTokenRevokeById): expected_response = {'events': [{'domain_id': domain_id}]} self.assertEqual(expected_response, events_response) - def assertValidRevokedTokenResponse(self, events_response, user_id): + def assertValidRevokedTokenResponse(self, events_response, user_id, + project_id=None): events = events_response['events'] self.assertEqual(1, len(events)) self.assertEqual(user_id, events[0]['user_id']) + if project_id: + self.assertEqual(project_id, events[0]['project_id']) self.assertIsNotNone(events[0]['expires_at']) self.assertIsNotNone(events[0]['issued_before']) self.assertIsNotNone(events_response['links']) del (events_response['events'][0]['expires_at']) del (events_response['events'][0]['issued_before']) del (events_response['links']) - expected_response = {'events': [{'user_id': user_id}]} + + expected_event_data = {'user_id': user_id} + if project_id: + expected_event_data['project_id'] = project_id + expected_response = {'events': [expected_event_data]} self.assertEqual(expected_response, events_response) def test_revoke_token(self): @@ -1412,7 +1416,8 @@ class TestTokenRevokeApi(TestTokenRevokeById): self.head('/auth/tokens', headers=headers, expected_status=404) events_response = self.get('/OS-REVOKE/events', expected_status=200).json_body - self.assertValidRevokedTokenResponse(events_response, self.user['id']) + self.assertValidRevokedTokenResponse(events_response, self.user['id'], + project_id=self.project['id']) def test_revoke_v2_token(self): token = self.get_v2_token() @@ -1501,10 +1506,11 @@ class TestTokenRevokeApi(TestTokenRevokeById): self.assertUserAndExpiryInList(events, token2['user']['id'], token2['expires_at']) - self.assertValidRevokedTokenResponse(events_response, self.user['id']) + self.assertValidRevokedTokenResponse(events_response, self.user['id'], + project_id=self.project['id']) self.head('/auth/tokens', headers=headers, expected_status=404) - self.head('/auth/tokens', headers=headers2, expected_status=404) - self.head('/auth/tokens', headers=headers3, expected_status=404) + self.head('/auth/tokens', headers=headers2, expected_status=200) + self.head('/auth/tokens', headers=headers3, expected_status=200) self.head('/auth/tokens', headers=headers_unrevoked, expected_status=200) diff --git a/keystone/token/providers/common.py b/keystone/token/providers/common.py index d25418b3f4..be09e5c525 100644 --- a/keystone/token/providers/common.py +++ b/keystone/token/providers/common.py @@ -520,10 +520,18 @@ class BaseProvider(provider.Provider): if version == provider.V3: user_id = token['user']['id'] expires_at = token['expires'] + + token_data = token['token_data']['token'] + project_id = token_data.get('project', {}).get('id') + domain_id = token_data.get('domain', {}).get('id') elif version == provider.V2: user_id = token['user_id'] expires_at = token['expires'] - self.revoke_api.revoke_by_expiration(user_id, expires_at) + project_id = (token.get('tenant') or {}).get('id') + domain_id = None # A V2 token can't be scoped to a domain. + self.revoke_api.revoke_by_expiration(user_id, expires_at, + project_id=project_id, + domain_id=domain_id) if CONF.token.revoke_by_id: self.token_api.delete_token(token_id=token_id)