From d42e95520302f3203706993304e024e82f947728 Mon Sep 17 00:00:00 2001 From: Lance Bragstad Date: Thu, 15 Feb 2018 15:30:45 +0000 Subject: [PATCH] Address FIXMEs for listing revoked tokens Now that support for sql token storage and uuid tokens has been removed, it doesn't make sense to still expose an API for listing revoked tokens. Maintaining this behavior would require keystone to persist non-persistent tokens, which defeats the purpose. This change makes the API return either a 410 Gone or a 403 Forbidden depending on configuration for backwards compatibility. Logic to list revoked tokens was also removed from the token provider API since it's no longer called by any controllers. Change-Id: Ic7bcba148f0a062b144e6dfbe9693f2125008458 --- keystone/auth/controllers.py | 34 ++++++----------------------- keystone/tests/unit/test_v3_auth.py | 13 +++++++++++ keystone/token/provider.py | 11 ---------- 3 files changed, 20 insertions(+), 38 deletions(-) diff --git a/keystone/auth/controllers.py b/keystone/auth/controllers.py index 0375153fc5..1a6735df34 100644 --- a/keystone/auth/controllers.py +++ b/keystone/auth/controllers.py @@ -12,9 +12,7 @@ # License for the specific language governing permissions and limitations # under the License. -from keystoneclient.common import cms from oslo_log import log -from oslo_serialization import jsonutils import six from keystone.auth import core @@ -22,7 +20,6 @@ from keystone.auth import schema from keystone.common import authorization from keystone.common import controller from keystone.common import provider_api -from keystone.common import utils from keystone.common import wsgi import keystone.conf from keystone import exception @@ -296,33 +293,16 @@ class Auth(controller.V3Controller): return render_token_data_response(token.id, token_reference) - @controller.protected() def revocation_list(self, request): if not CONF.token.revoke_by_id: raise exception.Gone() - - audit_id_only = 'audit_id_only' in request.params - - tokens = PROVIDERS.token_provider_api.list_revoked_tokens() - - for t in tokens: - expires = t['expires'] - if not (expires and isinstance(expires, six.text_type)): - t['expires'] = utils.isotime(expires) - if audit_id_only: - t.pop('id', None) - data = {'revoked': tokens} - - if audit_id_only: - # No need to obfuscate if no token IDs. - return data - - json_data = jsonutils.dumps(data) - signed_text = cms.cms_sign_text(json_data, - CONF.signing.certfile, - CONF.signing.keyfile) - - return {'signed': signed_text} + # NOTE(lbragstad): This API is deprecated and isn't supported. Keystone + # also doesn't store tokens, so returning a list of revoked tokens + # would require keystone to write invalid tokens to disk, which defeats + # the purpose. Return a 403 instead of removing the API all together. + # The alternative would be to return a signed response of just an empty + # list. + raise exception.Forbidden() def _combine_lists_uniquely(self, a, b): # it's most likely that only one of these will be filled so avoid diff --git a/keystone/tests/unit/test_v3_auth.py b/keystone/tests/unit/test_v3_auth.py index 6adc435744..714e696759 100644 --- a/keystone/tests/unit/test_v3_auth.py +++ b/keystone/tests/unit/test_v3_auth.py @@ -3437,6 +3437,19 @@ class TestTokenRevokeApi(TestTokenRevokeById): self.head('/auth/tokens/OS-PKI/revoked', expected_status=http_client.GONE) + def test_revoke_by_id_true_returns_forbidden(self): + self.config_fixture.config( + group='token', + revoke_by_id=True) + self.get( + '/auth/tokens/OS-PKI/revoked', + expected_status=http_client.FORBIDDEN + ) + self.head( + '/auth/tokens/OS-PKI/revoked', + expected_status=http_client.FORBIDDEN + ) + def test_list_delete_project_shows_in_event_list(self): self.role_data_fixtures() events = self.get('/OS-REVOKE/events').json_body['events'] diff --git a/keystone/token/provider.py b/keystone/token/provider.py index 5de31e9fd8..50ada78b17 100644 --- a/keystone/token/provider.py +++ b/keystone/token/provider.py @@ -291,14 +291,3 @@ class Manager(manager.Manager): # consulted before accepting a token as valid. For now we will # do the explicit individual token invalidation. self.invalidate_individual_token_cache(token_id) - - def list_revoked_tokens(self): - # FIXME(lbragstad): In the future, the token providers are going to be - # responsible for handling persistence if they require it (e.g. token - # providers not doing some sort of authenticated encryption strategy). - # When that happens, we could still expose this API by checking an - # interface on the provider can calling it if available. For now, this - # will return a valid response, but it will just be an empty list. See - # http://paste.openstack.org/raw/670196/ for and example using - # keystoneclient.common.cms to verify the response. - return []