Ensure v2 tokens are correctly invalidated when using BelongsTo

Due to the optional paramater of the tenant on several v2 token
validation methods, we need to ensure that calling token validation
with different permutations of parameters does not lead to an incorrect
cache value being returned.  This is done by lifting the 'BelongsTo'
checks out of the token backend and into the Manager, in a layer above
where the token caching takes place.

Fixes bug 1226225

Change-Id: Ifa3162923ad41aac6a9e5d5b4996bc43dc9b11fb

keystone/tests/test_backend.py

@@ -2763,6 +2763,96 @@ class TokenTests(object):
             self.assertIn('expires', t)
 
 
+class TokenCacheInvalidation(object):
+    def _create_test_data(self):
+        self.user = {'id': uuid.uuid4().hex, 'name': uuid.uuid4().hex,
+                     'password': uuid.uuid4().hex,
+                     'domain_id': DEFAULT_DOMAIN_ID, 'enabled': True}
+        self.tenant = {'id': uuid.uuid4().hex, 'name': uuid.uuid4().hex,
+                       'domain_id': DEFAULT_DOMAIN_ID, 'enabled': True}
+
+        # Create an equivalent of a scoped token
+        token_dict = {'user': self.user, 'tenant': self.tenant,
+                      'metadata': {}, 'id': 'placeholder'}
+        id, data = self.token_provider_api.issue_v2_token(token_dict)
+        self.scoped_token_id = id
+
+        # ..and an un-scoped one
+        token_dict = {'user': self.user, 'tenant': None,
+                      'metadata': {}, 'id': 'placeholder'}
+        id, data = self.token_provider_api.issue_v2_token(token_dict)
+        self.unscoped_token_id = id
+
+        # Validate them, in the various ways possible - this will load the
+        # responses into the token cache.
+        self._check_scoped_tokens_are_valid()
+        self._check_unscoped_tokens_are_valid()
+
+    def _check_unscoped_tokens_are_invalid(self):
+        self.assertRaises(
+            exception.Unauthorized,
+            self.token_provider_api.validate_token,
+            self.unscoped_token_id)
+        self.assertRaises(
+            exception.Unauthorized,
+            self.token_provider_api.validate_v2_token,
+            self.unscoped_token_id)
+
+    def _check_scoped_tokens_are_invalid(self):
+        self.assertRaises(
+            exception.Unauthorized,
+            self.token_provider_api.validate_token,
+            self.scoped_token_id)
+        self.assertRaises(
+            exception.Unauthorized,
+            self.token_provider_api.validate_token,
+            self.scoped_token_id,
+            self.tenant['id'])
+        self.assertRaises(
+            exception.Unauthorized,
+            self.token_provider_api.validate_v2_token,
+            self.scoped_token_id)
+        self.assertRaises(
+            exception.Unauthorized,
+            self.token_provider_api.validate_v2_token,
+            self.scoped_token_id,
+            self.tenant['id'])
+
+    def _check_scoped_tokens_are_valid(self):
+        self.token_provider_api.validate_token(self.scoped_token_id)
+        self.token_provider_api.validate_token(
+            self.scoped_token_id, belongs_to=self.tenant['id'])
+        self.token_provider_api.validate_v2_token(self.scoped_token_id)
+        self.token_provider_api.validate_v2_token(
+            self.scoped_token_id, belongs_to=self.tenant['id'])
+
+    def _check_unscoped_tokens_are_valid(self):
+        self.token_provider_api.validate_token(self.unscoped_token_id)
+        self.token_provider_api.validate_v2_token(self.unscoped_token_id)
+
+    def test_delete_unscoped_token(self):
+        self.token_api.delete_token(self.unscoped_token_id)
+        self._check_unscoped_tokens_are_invalid()
+
+    def test_delete_scoped_token_by_id(self):
+        self.token_api.delete_token(self.scoped_token_id)
+        self._check_scoped_tokens_are_invalid()
+        self._check_unscoped_tokens_are_valid()
+
+    def test_delete_scoped_token_by_user(self):
+        self.token_api.delete_tokens(self.user['id'])
+        # Since we are deleting all tokens for this user, they should all
+        # now be invalid.
+        self._check_scoped_tokens_are_invalid()
+        self._check_unscoped_tokens_are_invalid()
+
+    def test_delete_scoped_token_by_user_and_tenant(self):
+        self.token_api.delete_tokens(self.user['id'],
+                                     tenant_id=self.tenant['id'])
+        self._check_scoped_tokens_are_invalid()
+        self._check_unscoped_tokens_are_valid()
+
+
 class TrustTests(object):
     def create_sample_trust(self, new_id):
         self.trustor = self.user_foo

keystone/tests/test_backend_kvs.py

@@ -15,12 +15,15 @@
 # under the License.
 import uuid
 
+from keystone import config
 from keystone import exception
 from keystone import identity
 from keystone import tests
 from keystone.tests import default_fixtures
 from keystone.tests import test_backend
 
+CONF = config.CONF
+
 
 class KvsIdentity(tests.TestCase, test_backend.IdentityTests):
     def setUp(self):
@@ -115,3 +118,13 @@ class KvsCatalog(tests.TestCase, test_backend.CatalogTests):
     def test_get_catalog(self):
         catalog_ref = self.catalog_api.get_catalog('foo', 'bar')
         self.assertDictEqual(catalog_ref, self.catalog_foobar)
+
+
+class KvsTokenCacheInvalidation(tests.TestCase,
+                                test_backend.TokenCacheInvalidation):
+    def setUp(self):
+        super(KvsTokenCacheInvalidation, self).setUp()
+        CONF.identity.driver = 'keystone.identity.backends.kvs.Identity'
+        CONF.token.driver = 'keystone.token.backends.kvs.Token'
+        self.load_backends()
+        self._create_test_data()

keystone/tests/test_backend_memcache.py

@@ -21,6 +21,7 @@ import uuid
 import memcache
 
 from keystone.common import utils
+from keystone import config
 from keystone import exception
 from keystone.openstack.common import jsonutils
 from keystone.openstack.common import timeutils
@@ -30,6 +31,8 @@ from keystone.tests import test_utils
 from keystone import token
 from keystone.token.backends import memcache as token_memcache
 
+CONF = config.CONF
+
 
 class MemcacheClient(object):
     """Replicates a tiny subset of memcached client interface."""
@@ -214,3 +217,17 @@ class MemcacheToken(tests.TestCase, test_backend.TokenTests):
             data_in = _create_token(expire_time_expired)
             self.assertRaises(exception.TokenNotFound,
                               self.token_api.get_token, data_in['id'])
+
+
+class MemcacheTokenCacheInvalidation(tests.TestCase,
+                                     test_backend.TokenCacheInvalidation):
+    def setUp(self):
+        super(MemcacheTokenCacheInvalidation, self).setUp()
+        CONF.token.driver = 'keystone.token.backends.memcache.Token'
+        self.load_backends()
+        fake_client = MemcacheClient()
+        self.token_man = token.Manager()
+        self.token_man.driver = token_memcache.Token(client=fake_client)
+        self.token_api = self.token_man
+        self.token_provider_api.driver.token_api = self.token_api
+        self._create_test_data()

keystone/tests/test_backend_sql.py

@@ -416,3 +416,9 @@ class SqlPolicy(SqlTests, test_backend.PolicyTests):
 
 class SqlInheritance(SqlTests, test_backend.InheritanceTests):
     pass
+
+
+class SqlTokenCacheInvalidation(SqlTests, test_backend.TokenCacheInvalidation):
+    def setUp(self):
+        super(SqlTokenCacheInvalidation, self).setUp()
+        self._create_test_data()

keystone/token/core.py

@@ -164,7 +164,7 @@ class Manager(manager.Manager):
         self.driver.delete_tokens(user_id, tenant_id, trust_id, consumer_id)
         for token_id in token_list:
             unique_id = self.unique_id(token_id)
-            self._invalidate_individual_token_cache(unique_id, tenant_id)
+            self._invalidate_individual_token_cache(unique_id)
         self.invalidate_revocation_list()
 
     @cache.on_arguments(should_cache_fn=SHOULD_CACHE,
@@ -178,7 +178,7 @@ class Manager(manager.Manager):
         # determining cache-keys.
         self.list_revoked_tokens.invalidate(self)
 
-    def _invalidate_individual_token_cache(self, token_id, belongs_to=None):
+    def _invalidate_individual_token_cache(self, token_id):
         # NOTE(morganfainberg): invalidate takes the exact same arguments as
         # the normal method, this means we need to pass "self" in (which gets
         # stripped off).
@@ -188,8 +188,7 @@ class Manager(manager.Manager):
         # consulted before accepting a token as valid.  For now we will
         # do the explicit individual token invalidation.
         self._get_token.invalidate(self, token_id)
-        self.token_provider_api.invalidate_individual_token_cache(token_id,
-                                                                  belongs_to)
+        self.token_provider_api.invalidate_individual_token_cache(token_id)
 
 
 class Driver(object):
@@ -265,6 +264,11 @@ class Driver(object):
         :raises: keystone.exception.TokenNotFound
 
         """
+        # TODO(henry-nash): The SQL driver already has a more efficient
+        # implementation of this, although this is missing from the other
+        # backends.  These should be completed and then this should become
+        # a virtual method.  This is raised as bug #1227507.
+
         token_list = self.list_tokens(user_id,
                                       tenant_id=tenant_id,
                                       trust_id=trust_id,

keystone/token/provider.py

@@ -106,7 +106,8 @@ class Manager(manager.Manager):
         unique_id = self.token_api.unique_id(token_id)
         # NOTE(morganfainberg): Ensure we never use the long-form token_id
         # (PKI) as part of the cache_key.
-        token = self._validate_token(unique_id, belongs_to)
+        token = self._validate_token(unique_id)
+        self._token_belongs_to(token, belongs_to)
         self._is_valid_token(token)
         return token
 
@@ -114,7 +115,8 @@ class Manager(manager.Manager):
         unique_id = self.token_api.unique_id(token_id)
         # NOTE(morganfainberg): Ensure we never use the long-form token_id
         # (PKI) as part of the cache_key.
-        token = self._validate_v2_token(unique_id, belongs_to)
+        token = self._validate_v2_token(unique_id)
+        self._token_belongs_to(token, belongs_to)
         self._is_valid_token(token)
         return token
 
@@ -153,13 +155,13 @@ class Manager(manager.Manager):
 
     @cache.on_arguments(should_cache_fn=SHOULD_CACHE,
                         expiration_time=CONF.token.cache_time)
-    def _validate_token(self, token_id, belongs_to=None):
-        return self.driver.validate_token(token_id, belongs_to)
+    def _validate_token(self, token_id):
+        return self.driver.validate_token(token_id)
 
     @cache.on_arguments(should_cache_fn=SHOULD_CACHE,
                         expiration_time=CONF.token.cache_time)
-    def _validate_v2_token(self, token_id, belongs_to=None):
-        return self.driver.validate_v2_token(token_id, belongs_to)
+    def _validate_v2_token(self, token_id):
+        return self.driver.validate_v2_token(token_id)
 
     @cache.on_arguments(should_cache_fn=SHOULD_CACHE,
                         expiration_time=CONF.token.cache_time)
@@ -190,7 +192,20 @@ class Manager(manager.Manager):
         # Token is expired, we have a malformed token, or something went wrong.
         raise exception.Unauthorized(_('Failed to validate token'))
 
-    def invalidate_individual_token_cache(self, token_id, belongs_to=None):
+    def _token_belongs_to(self, token, belongs_to):
+        """Check if the token belongs to the right tenant.
+
+        This is only used on v2 tokens.  The structural validity of the token
+        will have already been checked before this method is called.
+
+        """
+        if belongs_to:
+            token_data = token['access']['token']
+            if ('tenant' not in token_data or
+                    token_data['tenant']['id'] != belongs_to):
+                raise exception.Unauthorized()
+
+    def invalidate_individual_token_cache(self, token_id):
         # NOTE(morganfainberg): invalidate takes the exact same arguments as
         # the normal method, this means we need to pass "self" in (which gets
         # stripped off).
@@ -199,10 +214,10 @@ class Manager(manager.Manager):
         # invalidated? We maintain a cached revocation list, which should be
         # 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)
+
+        self._validate_token.invalidate(self, token_id)
         self._validate_v2_token.invalidate(self, token_id)
-        self._validate_v2_token.invalidate(self, token_id, belongs_to)
-        self._validate_token.invalidate(self, token_id, belongs_to)
+        self._validate_v3_token.invalidate(self, token_id)
 
 
 class Provider(object):
@@ -268,29 +283,25 @@ class Provider(object):
         """
         raise exception.NotImplemented()
 
-    def validate_token(self, token_id, belongs_to=None):
+    def validate_token(self, token_id):
         """Detect token version and validate token and return the token data.
 
         Must raise Unauthorized exception if unable to validate token.
 
         :param token_id: identity of the token
         :type token_id: string
-        :param belongs_to: optional (V2) identity of the scoped project
-        :type belongs_to: string
         :returns: token_data
         :raises: keystone.exception.Unauthorized
         """
         raise exception.NotImplemented()
 
-    def validate_v2_token(self, token_id, belongs_to=None):
+    def validate_v2_token(self, token_id):
         """Validate the given V2 token and return the token data.
 
         Must raise Unauthorized exception if unable to validate token.
 
         :param token_id: identity of the token
         :type token_id: string
-        :param belongs_to: optional identity of the scoped project to validate
-        :type belongs_to: string
         :returns: token data
         :raises: keystone.exception.Unauthorized
 

keystone/token/providers/uuid.py

@@ -453,23 +453,18 @@ class Provider(token.provider.Provider):
 
         return (token_id, token_data)
 
-    def _verify_token(self, token_id, belongs_to=None):
+    def _verify_token(self, token_id):
         """Verify the given token and return the token_ref."""
         try:
             token_ref = self.token_api.get_token(token_id)
-            return self._verify_token_ref(token_ref, belongs_to)
+            return self._verify_token_ref(token_ref)
         except exception.TokenNotFound:
                 raise exception.Unauthorized()
 
-    def _verify_token_ref(self, token_ref, belongs_to=None):
+    def _verify_token_ref(self, token_ref):
         """Verify and return the given token_ref."""
         if not token_ref:
             raise exception.Unauthorized()
-        if belongs_to:
-            if not (token_ref['tenant'] and
-                    token_ref['tenant']['id'] == belongs_to):
-                raise exception.Unauthorized()
-
         return token_ref
 
     def revoke_token(self, token_id):
@@ -516,8 +511,8 @@ class Provider(token.provider.Provider):
                 if project_ref['domain_id'] != DEFAULT_DOMAIN_ID:
                     raise exception.Unauthorized(msg)
 
-    def validate_v2_token(self, token_id, belongs_to=None):
-        token_ref = self._verify_token(token_id, belongs_to)
+    def validate_v2_token(self, token_id):
+        token_ref = self._verify_token(token_id)
         return self._validate_v2_token_ref(token_ref)
 
     def _validate_v2_token_ref(self, token_ref):
@@ -591,8 +586,8 @@ class Provider(token.provider.Provider):
                 expires=token_ref['expires'])
         return token_data
 
-    def validate_token(self, token_id, belongs_to=None):
-        token_ref = self._verify_token(token_id, belongs_to=belongs_to)
+    def validate_token(self, token_id):
+        token_ref = self._verify_token(token_id)
         version = self.get_token_version(token_ref)
         if version == token.provider.V3:
             return self._validate_v3_token_ref(token_ref)