Remove needs_persistence property from token providers

Since the sql token storage mechanism was removed in Rocky, we no
longer need hooks in the token Manager to determine if a token needs
to be retrieved from or written to a backend somewhere. Instead, token
providers will need to handle storage requirements if they need them.
This will result in a cleaner token provider interface.

Change-Id: Icc095987d41e9c08de2f34dc657b08b98bd944e4

keystone/tests/unit/token/test_fernet_provider.py

@@ -54,9 +54,6 @@ class TestFernetTokenProvider(unit.TestCase):
     def test_supports_bind_authentication_returns_false(self):
         self.assertFalse(self.provider._supports_bind_authentication)
 
-    def test_needs_persistence_returns_false(self):
-        self.assertFalse(self.provider.needs_persistence())
-
     def test_invalid_v3_token_raises_token_not_found(self):
         token_id = uuid.uuid4().hex
         e = self.assertRaises(

keystone/token/provider.py

@@ -15,11 +15,9 @@
 """Token provider interface."""
 
 import datetime
-import sys
 
 from oslo_log import log
 from oslo_utils import timeutils
-import six
 
 from keystone.common import cache
 from keystone.common import manager
@@ -62,7 +60,6 @@ class Manager(manager.Manager):
 
     V3 = V3
     VERSIONS = VERSIONS
-    _persistence_manager = None
 
     def __init__(self):
         super(Manager, self).__init__(CONF.token.provider)
@@ -99,35 +96,6 @@ class Manager(manager.Manager):
                 notifications.register_event_callback(event, resource_type,
                                                       callback_fns)
 
-    @property
-    def _needs_persistence(self):
-        return self.driver.needs_persistence()
-
-    @property
-    def _persistence(self):
-        # NOTE(morganfainberg): This should not be handled via __init__ to
-        # avoid dependency injection oddities circular dependencies (where
-        # the provider manager requires the token persistence manager, which
-        # requires the token provider manager).
-        if self._persistence_manager is None:
-            self._persistence_manager = self._token_persistence_manager
-        return self._persistence_manager
-
-    def _create_token(self, token_id, token_data):
-        try:
-            if isinstance(token_data['expires'], six.string_types):
-                token_data['expires'] = timeutils.normalize_time(
-                    timeutils.parse_isotime(token_data['expires']))
-            self._persistence.create_token(token_id, token_data)
-        except Exception:
-            exc_info = sys.exc_info()
-            # an identical token may have been created already.
-            # if so, return the token_data as it is also identical
-            try:
-                self._persistence.get_token(token_id)
-            except exception.TokenNotFound:
-                six.reraise(*exc_info)
-
     def check_revocation_v3(self, token):
         try:
             token_data = token['token']
@@ -144,15 +112,6 @@ class Manager(manager.Manager):
             raise exception.TokenNotFound(_('No token in the request'))
 
         try:
-            # NOTE(lbragstad): Only go to persistent storage if we have a token
-            # to fetch from the backend (the driver persists the token).
-            # Otherwise the information about the token must be in the token
-            # id.
-            if self._needs_persistence:
-                token_ref = self._persistence.get_token(token_id)
-                # 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, window_seconds=window_seconds)
             return token_ref
@@ -207,18 +166,6 @@ class Manager(manager.Manager):
             app_cred_id=app_cred_id, include_catalog=include_catalog,
             parent_audit_id=parent_audit_id)
 
-        if self._needs_persistence:
-            data = dict(key=token_id,
-                        id=token_id,
-                        expires=token_data['token']['expires_at'],
-                        user=token_data['token']['user'],
-                        tenant=token_data['token'].get('project'),
-                        is_domain=is_domain,
-                        token_data=token_data,
-                        trust_id=trust['id'] if trust else None,
-                        token_version=self.V3)
-            self._create_token(token_id, data)
-
         if CONF.token.cache_on_issue:
             # NOTE(amakarov): here and above TOKENS_REGION is to be passed
             # to serve as required positional "self" argument. It's ignored,
@@ -255,9 +202,6 @@ class Manager(manager.Manager):
         else:
             PROVIDERS.revoke_api.revoke_by_audit_id(token_ref.audit_id)
 
-        if CONF.token.revoke_by_id and self._needs_persistence:
-            self._persistence.delete_token(token_id=token_id)
-
         # FIXME(morganfainberg): Does this cache actually need to be
         # invalidated? We maintain a cached revocation list, which should be
         # consulted before accepting a token as valid.  For now we will

keystone/token/providers/base.py

@@ -23,16 +23,6 @@ from keystone import exception
 class Provider(object):
     """Interface description for a Token provider."""
 
-    @abc.abstractmethod
-    def needs_persistence(self):
-        """Determine if the token should be persisted.
-
-        If the token provider requires that the token be persisted to a
-        backend this should return True, otherwise return False.
-
-        """
-        raise exception.NotImplemented()  # pragma: no cover
-
     @abc.abstractmethod
     def get_token_version(self, token_data):
         """Return the version of the given token data.

keystone/token/providers/common.py

@@ -641,78 +641,45 @@ class BaseProvider(provider_api.ProviderAPIMixin, base.Provider):
         return token_ref
 
     def validate_token(self, token_id):
-        if self.needs_persistence():
-            token_ref = token_id
-            token_data = token_ref.get('token_data')
-            user_id = token_ref['user_id']
-            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')
-            system = token_data['token'].get('system', {}).get('all')
-            if system:
-                system = 'all'
-            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 = PROVIDERS.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:
-            try:
-                (user_id, methods, audit_ids, system, 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)
+        try:
+            (user_id, methods, audit_ids, system, 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)
 
-            bind = None
-            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
+        bind = None
+        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 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 = PROVIDERS.trust_api.get_trust(trust_id)
+        if trust_id:
+            trust_ref = PROVIDERS.trust_api.get_trust(trust_id)
 
-            access_token = None
-            if access_token_id:
-                access_token = PROVIDERS.oauth_api.get_access_token(
-                    access_token_id
-                )
+        access_token = None
+        if access_token_id:
+            access_token = PROVIDERS.oauth_api.get_access_token(
+                access_token_id
+            )
 
         return self.v3_token_data_helper.get_token_data(
             user_id,

keystone/token/providers/fernet/core.py

@@ -44,10 +44,6 @@ class Provider(common.BaseProvider):
 
         self.token_formatter = tf.TokenFormatter()
 
-    def needs_persistence(self):
-        """Should the token be written to a backend."""
-        return False
-
     def issue_token(self, *args, **kwargs):
         token_id, token_data = super(Provider, self).issue_token(
             *args, **kwargs)

releasenotes/notes/token-provider-refactor-a3a64146807daf36.yaml

@@ -0,0 +1,7 @@
+---
+upgrade:
+  - |
+    The token provider API has removed the ``needs_persistence`` property from
+    the abstract interface. Token providers are expected to handle persistence
+    requirement if needed. This will require out-of-tree token providers to
+    remove the unused property and handle token storage.