openstack/keystone
Code Issues Proposed changes

One validate method to rule them all...

Browse Source 
Regardless of persistence requirements or format, let's perform
token validation one way.

This simplifies the validation path of the token provider API.

Change-Id: Idb5de4459fd8bf83973ed74fccc275a64873c88c
This commit is contained in:
Lance Bragstad 2016-09-30 22:31:30 +00:00
parent f84dd99965
commit 71134fbe1c
15 changed files with 147 additions and 159 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
keystone/auth/controllers.py
View File

@ -541,7 +541,7 @@ class Auth(controller.V3Controller):
@controller.protected()
def check_token(self, request):
token_id = request.context_dict.get('subject_token_id')
token_data = self.token_provider_api.validate_v3_token(
token_data = self.token_provider_api.validate_token(
token_id)
# NOTE(morganfainberg): The code in
# ``keystone.common.wsgi.render_response`` will remove the content
@ -557,7 +557,7 @@ class Auth(controller.V3Controller):
def validate_token(self, request):
token_id = request.context_dict.get('subject_token_id')
include_catalog = 'nocatalog' not in request.params
token_data = self.token_provider_api.validate_v3_token(
token_data = self.token_provider_api.validate_token(
token_id)
if not include_catalog and 'catalog' in token_data['token']:
del token_data['token']['catalog']

2
keystone/auth/plugins/mapped.py
View File

@ -35,7 +35,7 @@ class Mapped(base.AuthMethodHandler):
def _get_token_ref(self, auth_payload):
token_id = auth_payload['id']
response = self.token_provider_api.validate_v3_token(token_id)
response = self.token_provider_api.validate_token(token_id)
return token_model.KeystoneToken(token_id=token_id,
token_data=response)

2
keystone/auth/plugins/token.py
View File

@ -35,7 +35,7 @@ class Token(base.AuthMethodHandler):
def _get_token_ref(self, auth_payload):
token_id = auth_payload['id']
response = self.token_provider_api.validate_v3_token(token_id)
response = self.token_provider_api.validate_token(token_id)
return token_model.KeystoneToken(token_id=token_id,
token_data=response)

2
keystone/common/controller.py
View File

@ -133,7 +133,7 @@ def protected(callback=None):
if request.context_dict.get('subject_token_id') is not None:
token_ref = token_model.KeystoneToken(
token_id=request.context_dict['subject_token_id'],
token_data=self.token_provider_api.validate_v3_token(
token_data=self.token_provider_api.validate_token(
request.context_dict['subject_token_id']))
policy_dict.setdefault('target', {})
policy_dict['target'].setdefault(self.member_name, {})

2
keystone/federation/controllers.py
View File

@ -364,7 +364,7 @@ class Auth(auth_controllers.Auth):
sp_url = service_provider['sp_url']
token_id = auth['identity']['token']['id']
token_data = self.token_provider_api.validate_v3_token(token_id)
token_data = self.token_provider_api.validate_token(token_id)
token_ref = token_model.KeystoneToken(token_id, token_data)
if not token_ref.project_scoped:

2
keystone/middleware/auth.py
View File

@ -46,7 +46,7 @@ class AuthContextMiddleware(auth_token.BaseAuthProtocol):
def fetch_token(self, token):
try:
return self.token_provider_api.validate_v3_token(token)
return self.token_provider_api.validate_token(token)
except exception.TokenNotFound:
raise auth_token.InvalidToken(_('Could not find token'))

10
keystone/tests/unit/test_auth.py
View File

@ -594,10 +594,10 @@ class AuthWithToken(object):
self.token_provider_api.revoke_token(token_id, revoke_chain=True)
self.assertRaises(exception.TokenNotFound,
self.token_provider_api.validate_v3_token,
self.token_provider_api.validate_token,
token_id=token_id)
self.assertRaises(exception.TokenNotFound,
self.token_provider_api.validate_v3_token,
self.token_provider_api.validate_token,
token_id=token_2_id)
def test_revoke_by_audit_chain_id_chained_token(self):
@ -620,10 +620,10 @@ class AuthWithToken(object):
self.token_provider_api.revoke_token(token_2_id, revoke_chain=True)
self.assertRaises(exception.TokenNotFound,
self.token_provider_api.validate_v3_token,
self.token_provider_api.validate_token,
token_id=token_id)
self.assertRaises(exception.TokenNotFound,
self.token_provider_api.validate_v3_token,
self.token_provider_api.validate_token,
token_id=token_2_id)
def _mock_audit_info(self, parent_audit_id):
@ -922,7 +922,7 @@ class AuthWithTrust(object):
def _create_auth_request(self, token_id):
token_ref = token_model.KeystoneToken(
token_id=token_id,
token_data=self.token_provider_api.validate_v3_token(token_id))
token_data=self.token_provider_api.validate_token(token_id))
auth_context = authorization.token_to_auth_context(token_ref)
# NOTE(gyee): if public_endpoint and admin_endpoint are not set, which
# is the default, the base url will be constructed from the environment

2
keystone/tests/unit/test_token_provider.py
View File

@ -794,7 +794,7 @@ class TestTokenProvider(unit.TestCase):
def test_validate_v3_token_with_no_token_raises_token_not_found(self):
self.assertRaises(
exception.TokenNotFound,
self.token_provider_api.validate_v3_token,
self.token_provider_api.validate_token,
None)

8
keystone/tests/unit/token/test_backends.py
View File

@ -489,20 +489,20 @@ class TokenCacheInvalidation(object):
def _check_unscoped_tokens_are_invalid(self):
self.assertRaises(
exception.TokenNotFound,
self.token_provider_api.validate_v3_token,
self.token_provider_api.validate_token,
self.unscoped_token_id)
def _check_scoped_tokens_are_invalid(self):
self.assertRaises(
exception.TokenNotFound,
self.token_provider_api.validate_v3_token,
self.token_provider_api.validate_token,
self.scoped_token_id)
def _check_scoped_tokens_are_valid(self):
self.token_provider_api.validate_v3_token(self.scoped_token_id)
self.token_provider_api.validate_token(self.scoped_token_id)
def _check_unscoped_tokens_are_valid(self):
self.token_provider_api.validate_v3_token(self.unscoped_token_id)
self.token_provider_api.validate_token(self.unscoped_token_id)
def test_delete_unscoped_token(self):
self.token_provider_api._persistence.delete_token(

16
keystone/tests/unit/token/test_fernet_provider.py
View File

@ -55,14 +55,10 @@ class TestFernetTokenProvider(unit.TestCase):
self.assertFalse(self.provider.needs_persistence())
def test_invalid_v3_token_raises_token_not_found(self):
# NOTE(lbragstad): Here we use the validate_non_persistent_token()
# methods because the validate_v3_token() method is strictly for
# validating UUID formatted tokens. It is written to assume cached
# tokens from a backend, where validate_non_persistent_token() is not.
token_id = uuid.uuid4().hex
e = self.assertRaises(
exception.TokenNotFound,
self.provider.validate_non_persistent_token,
self.provider.validate_token,
token_id)
self.assertIn(token_id, u'%s' % e)
@ -70,7 +66,7 @@ class TestFernetTokenProvider(unit.TestCase):
token_id = uuid.uuid4().hex
e = self.assertRaises(
exception.TokenNotFound,
self.provider.validate_non_persistent_token,
self.provider.validate_token,
token_id)
self.assertIn(token_id, u'%s' % e)
@ -107,7 +103,7 @@ class TestValidate(unit.TestCase):
token_id, token_data_ = self.token_provider_api.issue_v3_token(
user_ref['id'], method_names)
token_data = self.token_provider_api.validate_v3_token(token_id)
token_data = self.token_provider_api.validate_token(token_id)
token = token_data['token']
self.assertIsInstance(token['audit_ids'], list)
self.assertIsInstance(token['expires_at'], str)
@ -149,7 +145,7 @@ class TestValidate(unit.TestCase):
token_id, token_data_ = self.token_provider_api.issue_v3_token(
user_ref['id'], method_names, auth_context=auth_context)
token_data = self.token_provider_api.validate_v3_token(token_id)
token_data = self.token_provider_api.validate_token(token_id)
token = token_data['token']
exp_user_info = {
'id': user_ref['id'],
@ -207,7 +203,7 @@ class TestValidate(unit.TestCase):
user_ref['id'], method_names, project_id=project_ref['id'],
trust=trust_ref)
token_data = self.token_provider_api.validate_v3_token(token_id)
token_data = self.token_provider_api.validate_token(token_id)
token = token_data['token']
exp_trust_info = {
'id': trust_ref['id'],
@ -223,7 +219,7 @@ class TestValidate(unit.TestCase):
# A uuid string isn't a valid Fernet token.
token_id = uuid.uuid4().hex
self.assertRaises(exception.TokenNotFound,
self.token_provider_api.validate_v3_token, token_id)
self.token_provider_api.validate_token, token_id)
class TestTokenFormatter(unit.TestCase):

8
keystone/token/controllers.py
View File

@ -176,7 +176,7 @@ class Auth(controller.V2Controller):
size=CONF.max_token_size)
try:
v3_token_data = self.token_provider_api.validate_v3_token(
v3_token_data = self.token_provider_api.validate_token(
old_token
)
# NOTE(lbragstad): Even though we are not using the v2.0 token
@ -438,7 +438,7 @@ class Auth(controller.V2Controller):
the content body.
"""
v3_token_response = self.token_provider_api.validate_v3_token(token_id)
v3_token_response = self.token_provider_api.validate_token(token_id)
v2_helper = providers.common.V2TokenDataHelper()
token = v2_helper.v3_to_v2_token(v3_token_response, token_id)
belongs_to = request.params.get('belongsTo')
@ -457,7 +457,7 @@ class Auth(controller.V2Controller):
"""
# TODO(ayoung) validate against revocation API
v3_token_response = self.token_provider_api.validate_v3_token(token_id)
v3_token_response = self.token_provider_api.validate_token(token_id)
v2_helper = providers.common.V2TokenDataHelper()
token = v2_helper.v3_to_v2_token(v3_token_response, token_id)
belongs_to = request.params.get('belongsTo')
@ -496,7 +496,7 @@ class Auth(controller.V2Controller):
"""Return a list of endpoints available to the token."""
self.assert_admin(request)
token_data = self.token_provider_api.validate_v3_token(token_id)
token_data = self.token_provider_api.validate_token(token_id)
token_ref = token_model.KeystoneToken(token_id, token_data)
catalog_ref = None

52
keystone/token/provider.py
View File

@ -230,7 +230,7 @@ class Manager(manager.Manager):
else:
return self.check_revocation_v3(token)
def validate_v3_token(self, token_id):
def validate_token(self, token_id):
if not token_id:
raise exception.TokenNotFound(_('No token in the request'))
@ -239,14 +239,15 @@ class Manager(manager.Manager):
# to fetch from the backend (the driver persists the token).
# Otherwise the information about the token must be in the token
# id.
if not self._needs_persistence:
token_ref = self.validate_non_persistent_token(token_id)
else:
if self._needs_persistence:
unique_id = utils.generate_unique_id(token_id)
# NOTE(morganfainberg): Ensure we never use the long-form
# token_id (PKI) as part of the cache_key.
token_ref = self._persistence.get_token(unique_id)
token_ref = self._validate_v3_token(token_ref)
# Overload the token_id variable to be a token reference
# instead.
token_id = token_ref
token_ref = self._validate_token(token_id)
self._is_valid_token(token_ref)
return token_ref
except exception.Unauthorized as e:
@ -254,12 +255,8 @@ class Manager(manager.Manager):
raise exception.TokenNotFound(token_id=token_id)
@MEMOIZE_TOKENS
def validate_non_persistent_token(self, token_id):
return self.driver.validate_non_persistent_token(token_id)
@MEMOIZE_TOKENS
def _validate_v3_token(self, token_id):
return self.driver.validate_v3_token(token_id)
def _validate_token(self, token_id):
return self.driver.validate_token(token_id)
def _is_valid_token(self, token):
"""Verify the token is valid format and has not expired."""
@ -313,12 +310,10 @@ class Manager(manager.Manager):
# on issuing a token using v2.0 API.
if CONF.token.cache_on_issue:
if self._needs_persistence:
validate_response = self.driver.validate_v3_token(token_ref)
validate_response = self.driver.validate_token(token_ref)
else:
validate_response = self.driver.validate_non_persistent_token(
token_id
)
self._validate_v3_token.set(
validate_response = self.driver.validate_token(token_id)
self._validate_token.set(
validate_response,
TOKENS_REGION,
token_id
@ -369,9 +364,7 @@ class Manager(manager.Manager):
# NOTE(amakarov): here and above TOKENS_REGION is to be passed
# to serve as required positional "self" argument. It's ignored,
# so I've put it here for convenience - any placeholder is fine.
self._validate_v3_token.set(token_data, TOKENS_REGION, token_id)
self.validate_non_persistent_token.set(
token_data, TOKENS_REGION, token_id)
self._validate_token.set(token_data, TOKENS_REGION, token_id)
return token_id, token_data
@ -385,16 +378,12 @@ class Manager(manager.Manager):
# consulted before accepting a token as valid. For now we will
# do the explicit individual token invalidation.
self._validate_v3_token.invalidate(self, token_id)
# This method isn't actually called in the case of non-persistent
# tokens, but we include the invalidation in case this ever changes
# in the future.
self.validate_non_persistent_token.invalidate(self, token_id)
self._validate_token.invalidate(self, token_id)
def revoke_token(self, token_id, revoke_chain=False):
token_ref = token_model.KeystoneToken(
token_id=token_id,
token_data=self.validate_v3_token(token_id))
token_data=self.validate_token(token_id))
project_id = token_ref.project_id if token_ref.project_scoped else None
domain_id = token_ref.domain_id if token_ref.domain_scoped else None
@ -551,18 +540,7 @@ class Provider(object):
raise exception.NotImplemented() # pragma: no cover
@abc.abstractmethod
def validate_non_persistent_token(self, token_id):
"""Validate a given non-persistent token id and return the token_data.
:param token_id: the token id
:type token_id: string
:returns: token data
:raises keystone.exception.TokenNotFound: When the token is invalid
"""
raise exception.NotImplemented() # pragma: no cover
@abc.abstractmethod
def validate_v3_token(self, token_ref):
def validate_token(self, token_ref):
"""Validate the given V3 token and return the token_data.
:param token_ref: the token reference

179
keystone/token/providers/common.py
View File

@ -728,50 +728,8 @@ class BaseProvider(provider.Provider):
raise exception.Unauthorized()
return token_ref
def validate_non_persistent_token(self, token_id):
try:
(user_id, methods, audit_ids, domain_id, project_id, trust_id,
federated_info, access_token_id, issued_at, expires_at) = (
self.token_formatter.validate_token(token_id))
except exception.ValidationError as e:
raise exception.TokenNotFound(e)
token_dict = None
trust_ref = None
if federated_info:
# NOTE(lbragstad): We need to rebuild information about the
# federated token as well as the federated token roles. This is
# because when we validate a non-persistent token, we don't have a
# token reference to pull the federated token information out of.
# As a result, we have to extract it from the token itself and
# rebuild the federated context. These private methods currently
# live in the keystone.token.providers.fernet.Provider() class.
token_dict = self._rebuild_federated_info(federated_info, user_id)
if project_id or domain_id:
self._rebuild_federated_token_roles(token_dict, federated_info,
user_id, project_id,
domain_id)
if trust_id:
trust_ref = self.trust_api.get_trust(trust_id)
access_token = None
if access_token_id:
access_token = self.oauth_api.get_access_token(access_token_id)
return self.v3_token_data_helper.get_token_data(
user_id,
method_names=methods,
domain_id=domain_id,
project_id=project_id,
issued_at=issued_at,
expires=expires_at,
trust=trust_ref,
token=token_dict,
access_token=access_token,
audit_info=audit_ids)
def validate_v3_token(self, token_ref):
user_id = token_ref['user_id']
def validate_token(self, token_id):
user_id = None # id of the user of the token
methods = None # list of methods used to obtain a token
bind = None # dictionary of bind methods
issued_at = None # time at which the token was issued
@ -782,56 +740,97 @@ class BaseProvider(provider.Provider):
access_token = None # dictionary containing OAUTH1 information
trust_ref = None # dictionary containing trust scope
token_dict = None # existing token information
token_data = token_ref.get('token_data')
if not token_data or 'token' not in token_data:
# NOTE(lbragstad): v2.0 tokens have an `access` dictionary instead
# of a `token` one. At this point we can safely assume we are
# validating a token that was created using the v2.0 API.
methods = ['password', 'token']
bind = token_ref.get('bind')
# I have no idea why issued_at and expires_at come from two
# different places...
issued_at = token_ref['token_data']['access']['token']['issued_at']
expires_at = token_ref['expires']
audit_ids = token_ref['token_data']['access']['token'].get(
'audit_ids'
)
project_id = None
project_ref = token_ref.get('tenant')
if project_ref:
project_id = project_ref['id']
trust_id = token_ref.get('trust_id')
if trust_id:
trust_ref = self.trust_api.get_trust(trust_id)
if self.needs_persistence():
token_ref = token_id
token_data = token_ref.get('token_data')
user_id = token_ref['user_id']
if not token_data or 'token' not in token_data:
# NOTE(lbragstad): v2.0 tokens have an `access` dictionary
# instead of a `token` one. At this point we can safely assume
# we are validating a token that was created using the v2.0
# API.
methods = ['password', 'token']
bind = token_ref.get('bind')
# I have no idea why issued_at and expires_at come from two
# different places...
issued_at = (
token_ref['token_data']['access']['token']['issued_at']
)
expires_at = token_ref['expires']
audit_ids = token_ref['token_data']['access']['token'].get(
'audit_ids'
)
project_id = None
project_ref = token_ref.get('tenant')
if project_ref:
project_id = project_ref['id']
trust_id = token_ref.get('trust_id')
if trust_id:
trust_ref = self.trust_api.get_trust(trust_id)
else:
# NOTE(lbragstad): Otherwise assume we are validating a token
# that was created using the v3 token API.
methods = token_data['token']['methods']
bind = token_data['token'].get('bind')
issued_at = token_data['token']['issued_at']
expires_at = token_data['token']['expires_at']
audit_ids = token_data['token'].get('audit_ids')
domain_id = token_data['token'].get('domain', {}).get('id')
project_id = token_data['token'].get('project', {}).get('id')
access_token = None
if token_data['token'].get('OS-OAUTH1'):
access_token = {
'id': token_data['token'].get('OS-OAUTH1', {}).get(
'access_token_id'
),
'consumer_id': token_data['token'].get(
'OS-OAUTH1', {}
).get('consumer_id')
}
trust_ref = None
trust_id = token_ref.get('trust_id')
if trust_id:
trust_ref = self.trust_api.get_trust(trust_id)
token_dict = None
if token_data['token']['user'].get(
federation_constants.FEDERATION):
token_dict = {'user': token_ref['user']}
else:
# NOTE(lbragstad): Otherwise assume we are validating a token that
# was created using the v3 token API.
methods = token_data['token']['methods']
bind = token_data['token'].get('bind')
issued_at = token_data['token']['issued_at']
expires_at = token_data['token']['expires_at']
audit_ids = token_data['token'].get('audit_ids')
domain_id = token_data['token'].get('domain', {}).get('id')
project_id = token_data['token'].get('project', {}).get('id')
access_token = None
if token_data['token'].get('OS-OAUTH1'):
access_token = {
'id': token_data['token'].get('OS-OAUTH1', {}).get(
'access_token_id'
),
'consumer_id': token_data['token'].get(
'OS-OAUTH1', {}
).get('consumer_id')
}
try:
(user_id, methods, audit_ids, domain_id, project_id, trust_id,
federated_info, access_token_id, issued_at, expires_at) = (
self.token_formatter.validate_token(token_id))
except exception.ValidationError as e:
raise exception.TokenNotFound(e)
token_dict = None
trust_ref = None
trust_id = token_ref.get('trust_id')
if federated_info:
# NOTE(lbragstad): We need to rebuild information about the
# federated token as well as the federated token roles. This is
# because when we validate a non-persistent token, we don't
# have a token reference to pull the federated token
# information out of. As a result, we have to extract it from
# the token itself and rebuild the federated context. These
# private methods currently live in the
# keystone.token.providers.fernet.Provider() class.
token_dict = self._rebuild_federated_info(
federated_info, user_id
)
if project_id or domain_id:
self._rebuild_federated_token_roles(
token_dict,
federated_info,
user_id,
project_id,
domain_id
)
if trust_id:
trust_ref = self.trust_api.get_trust(trust_id)
token_dict = None
if token_data['token']['user'].get(
federation_constants.FEDERATION):
token_dict = {'user': token_ref['user']}
access_token = None
if access_token_id:
access_token = self.oauth_api.get_access_token(access_token_id)
return self.v3_token_data_helper.get_token_data(
user_id,

2
keystone/v2_crud/user_crud.py
View File

@ -53,7 +53,7 @@ class UserController(identity.controllers.User):
token_id = request.context_dict.get('token_id')
original_password = user.get('original_password')
token_data = self.token_provider_api.validate_v3_token(token_id)
token_data = self.token_provider_api.validate_token(token_id)
token_ref = token_model.KeystoneToken(token_id=token_id,
token_data=token_data)

15
releasenotes/notes/remove-all-but-one-validate-token-method-7fd7377e62a1bc62.yaml Normal file
View File

@ -0,0 +1,15 @@
---
upgrade:
- The ``validate_v3_token()`` and
``validate_non_persistent_token()`` methods have been
removed from the token provider interface. The token
provider API now uses a single validation method
called ``validate_token()``. Having any validation method
defined except ``validate_token()`` will fail since the
interface no longer includes legacy methods. Please take
this into consideration and plan accordingly if you're
maintaining a custom token provider.
critical:
- If writing a custom token provider, see the upgrade
section about the removal of the ``validate_v3_token()``
and ``validate_non_persistent()`` token methods.