From 6cbf835542d62e6e5db4b4aef7141b1731cad9dc Mon Sep 17 00:00:00 2001 From: Brant Knudson Date: Thu, 31 Jul 2014 17:47:00 -0500 Subject: [PATCH] Fix revocation event handling with MySQL When MySQL is used to store revocation events, events are returned from the database with the timestamps truncated to the second. This causes a revocation event for a token (which has the issued_at timestamp to the microsecond) to not match the revocation event and therefore the token is not considered to be revoked. The fix is to have the revocation events and token timestamps both always be truncated to the second. This will cause all tokens for a user that are issued within a second to be revoked when any of those tokens are revoked, which shouldn't be a problem. Conflicts: keystone/tests/test_v3_os_revoke.py Change-Id: Ibd82b4ce910206dfd504c396614ae2ebed025e9b Closes-Bug: #1347961 (cherry picked from commit 7aee6304f653475a4130dc3e5be602e91481f108) --- keystone/contrib/revoke/model.py | 28 ++++++++++++++++++++++------ keystone/tests/test_revoke.py | 6 +++--- keystone/tests/test_v3_auth.py | 12 +++++------- keystone/tests/test_v3_os_revoke.py | 3 +-- 4 files changed, 31 insertions(+), 18 deletions(-) diff --git a/keystone/contrib/revoke/model.py b/keystone/contrib/revoke/model.py index 3ab25b384a..0e1cae33af 100644 --- a/keystone/contrib/revoke/model.py +++ b/keystone/contrib/revoke/model.py @@ -69,6 +69,11 @@ class RevokeEvent(object): # This is revoking all tokens for a domain. self.domain_scope_id = None + if self.expires_at is not None: + # Trim off the expiration time because MySQL timestamps are only + # accurate to the second. + self.expires_at = self.expires_at.replace(microsecond=0) + if self.revoked_at is None: self.revoked_at = timeutils.utcnow() if self.issued_before is None: @@ -90,8 +95,7 @@ class RevokeEvent(object): if self.consumer_id is not None: event['OS-OAUTH1:access_token_id'] = self.access_token_id if self.expires_at is not None: - event['expires_at'] = timeutils.isotime(self.expires_at, - subsecond=True) + event['expires_at'] = timeutils.isotime(self.expires_at) if self.issued_before is not None: event['issued_before'] = timeutils.isotime(self.issued_before, subsecond=True) @@ -242,9 +246,15 @@ class RevokeTree(object): def build_token_values_v2(access, default_domain_id): token_data = access['token'] + + token_expires_at = timeutils.parse_isotime(token_data['expires']) + + # Trim off the microseconds because the revocation event only has + # expirations accurate to the second. + token_expires_at = token_expires_at.replace(microsecond=0) + token_values = { - 'expires_at': timeutils.normalize_time( - timeutils.parse_isotime(token_data['expires'])), + 'expires_at': timeutils.normalize_time(token_expires_at), 'issued_at': timeutils.normalize_time( timeutils.parse_isotime(token_data['issued_at']))} @@ -282,9 +292,15 @@ def build_token_values_v2(access, default_domain_id): def build_token_values(token_data): + + token_expires_at = timeutils.parse_isotime(token_data['expires_at']) + + # Trim off the microseconds because the revocation event only has + # expirations accurate to the second. + token_expires_at = token_expires_at.replace(microsecond=0) + token_values = { - 'expires_at': timeutils.normalize_time( - timeutils.parse_isotime(token_data['expires_at'])), + 'expires_at': timeutils.normalize_time(token_expires_at), 'issued_at': timeutils.normalize_time( timeutils.parse_isotime(token_data['issued_at']))} diff --git a/keystone/tests/test_revoke.py b/keystone/tests/test_revoke.py index 1aebf94506..83a42d6517 100644 --- a/keystone/tests/test_revoke.py +++ b/keystone/tests/test_revoke.py @@ -350,7 +350,7 @@ class RevokeTreeTests(tests.TestCase): event = self._revoke_by_expiration(user_id, future_time) token_data_1 = _sample_blank_token() token_data_1['user_id'] = user_id - token_data_1['expires_at'] = future_time + token_data_1['expires_at'] = future_time.replace(microsecond=0) self._assertTokenRevoked(token_data_1) token_data_2 = _sample_blank_token() @@ -375,7 +375,7 @@ class RevokeTreeTests(tests.TestCase): token_data = _sample_blank_token() token_data['user_id'] = user_id token_data['project_id'] = project_id - token_data['expires_at'] = future_time + token_data['expires_at'] = future_time.replace(microsecond=0) self._revoke_by_expiration(user_id, future_time, project_id=project_id) self._assertTokenRevoked(token_data) @@ -392,7 +392,7 @@ class RevokeTreeTests(tests.TestCase): token_data = _sample_blank_token() token_data['user_id'] = user_id token_data['assignment_domain_id'] = domain_id - token_data['expires_at'] = future_time + token_data['expires_at'] = future_time.replace(microsecond=0) self._revoke_by_expiration(user_id, future_time, domain_id=domain_id) self._assertTokenRevoked(token_data) diff --git a/keystone/tests/test_v3_auth.py b/keystone/tests/test_v3_auth.py index c76292c6cb..a7e372ff47 100644 --- a/keystone/tests/test_v3_auth.py +++ b/keystone/tests/test_v3_auth.py @@ -1441,6 +1441,11 @@ class TestTokenRevokeApi(TestTokenRevokeById): def assertUserAndExpiryInList(self, events, user_id, expires_at): found = False for e in events: + + # Timestamps in the event list are accurate to second. + expires_at = timeutils.parse_isotime(expires_at) + expires_at = timeutils.isotime(expires_at) + if e['user_id'] == user_id and e['expires_at'] == expires_at: found = True self.assertTrue(found, @@ -1464,14 +1469,9 @@ class TestTokenRevokeApi(TestTokenRevokeById): response.json_body['token'] headers3 = {'X-Subject-Token': response.headers['X-Subject-Token']} - scoped_token = self.get_scoped_token() - headers_unrevoked = {'X-Subject-Token': scoped_token} - self.head('/auth/tokens', headers=headers, expected_status=200) 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) self.delete('/auth/tokens', headers=headers, expected_status=204) # NOTE(ayoung): not deleting token3, as it should be deleted @@ -1488,8 +1488,6 @@ class TestTokenRevokeApi(TestTokenRevokeById): self.head('/auth/tokens', headers=headers, 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) def test_list_with_filter(self): diff --git a/keystone/tests/test_v3_os_revoke.py b/keystone/tests/test_v3_os_revoke.py index 155b433b82..f455b43eab 100644 --- a/keystone/tests/test_v3_os_revoke.py +++ b/keystone/tests/test_v3_os_revoke.py @@ -62,8 +62,7 @@ class OSRevokeTests(test_v3.RestfulTestCase): expires_at = token.default_expire_time() sample = self._blank_event() sample['user_id'] = unicode(user_id) - sample['expires_at'] = unicode(timeutils.isotime(expires_at, - subsecond=True)) + sample['expires_at'] = unicode(timeutils.isotime(expires_at)) before_time = timeutils.utcnow() self.revoke_api.revoke_by_expiration(user_id, expires_at) resp = self.get('/OS-REVOKE/events')