Fix revoking a scoped token from an unscoped token
When a scoped token that was created from an unscoped token was revoked, the original token wound up being revoked. This is because the scope wasn't included in the revocation event. By including the scope in the revocation event only the scoped token is revoked. Change-Id: I5652663ab7e1176d3b1efc5d218a8a020498067e Closes-Bug: #1347318
This commit is contained in:
parent
097b9aa039
commit
da00da732d
@ -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):
|
||||
|
@ -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 = []
|
||||
|
@ -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)
|
||||
|
@ -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)
|
||||
|
||||
|
@ -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)
|
||||
|
Loading…
Reference in New Issue
Block a user