Populate application credential data in token

Without this patch, the token formatter does not have enough data to
construct a token created with an application credential. This means
that if the token cache is disabled or expired, when keystone goes to
create the token it will not find any application credential information
and will not recreate the application_credential_restricted parameter in
the token data. This patch creates a new Payload class for application
credentials so that the application credential ID is properly persisted
in the msgpack'd payload. It also adds more data to the token data
object so that the application credential ID and name as well as its
restricted status is available when the token is queried.

Co-authored-by: Lance Bragstad <lbragstad@gmail.com>

Change-Id: I322a40404d8287748fe8c3a8d6dc1256d935d84a
Closes-bug: #1750415
(cherry picked from commit 796198f19670e3eb899ca3b1db5d2a21a4127a30)

8 files changed, 146 insertions, 35 deletions

diff --git a/keystone/application_credential/controllers.py b/keystone/application_credential/controllers.py
index 194eb7a2d..0cb2df2c8 100644
--- a/keystone/application_credential/controllers.py
+++ b/keystone/application_credential/controllers.py
@@ -86,7 +86,8 @@ class ApplicationCredentialV3(controller.V3Controller):
     def _check_unrestricted(self, token):
         auth_methods = token['methods']
         if 'application_credential' in auth_methods:
-            if token.token_data['token']['application_credential_restricted']:
+            td = token.token_data['token']
+            if td['application_credential']['restricted']:
                 action = _("Using method 'application_credential' is not "
                            "allowed for managing additional application "
                            "credentials.")
diff --git a/keystone/tests/unit/test_v3_auth.py b/keystone/tests/unit/test_v3_auth.py
index d1177b6c0..f83df746d 100644
--- a/keystone/tests/unit/test_v3_auth.py
+++ b/keystone/tests/unit/test_v3_auth.py
@@ -5426,6 +5426,27 @@ class ApplicationCredentialAuth(test_v3.RestfulTestCase):
             app_cred_id=app_cred_ref['id'], secret=app_cred_ref['secret'])
         self.v3_create_token(auth_data, expected_status=http_client.CREATED)
 
+    def test_validate_application_credential_token_populates_restricted(self):
+        self.config_fixture.config(group='token', cache_on_issue=False)
+        app_cred = self._make_app_cred()
+        app_cred_ref = self.app_cred_api.create_application_credential(
+            app_cred)
+        auth_data = self.build_authentication_request(
+            app_cred_id=app_cred_ref['id'], secret=app_cred_ref['secret'])
+        auth_response = self.v3_create_token(
+            auth_data, expected_status=http_client.CREATED)
+        self.assertTrue(
+            auth_response.json['token']['application_credential']['restricted']
+        )
+        token_id = auth_response.headers.get('X-Subject-Token')
+        headers = {'X-Auth-Token': token_id, 'X-Subject-Token': token_id}
+        validate_response = self.get(
+            '/auth/tokens', headers=headers
+        ).json_body
+        self.assertTrue(
+            validate_response['token']['application_credential']['restricted']
+        )
+
     def test_valid_application_credential_with_name_succeeds(self):
         app_cred = self._make_app_cred()
         app_cred_ref = self.app_cred_api.create_application_credential(
diff --git a/keystone/tests/unit/token/test_fernet_provider.py b/keystone/tests/unit/token/test_fernet_provider.py
index da7e82023..bf93acf6c 100644
--- a/keystone/tests/unit/token/test_fernet_provider.py
+++ b/keystone/tests/unit/token/test_fernet_provider.py
@@ -340,7 +340,8 @@ class TestPayloads(unit.TestCase):
     def _test_payload(self, payload_class, exp_user_id=None, exp_methods=None,
                       exp_system=None, exp_project_id=None,
                       exp_domain_id=None, exp_trust_id=None,
-                      exp_federated_info=None, exp_access_token_id=None):
+                      exp_federated_info=None, exp_access_token_id=None,
+                      exp_app_cred_id=None):
         exp_user_id = exp_user_id or uuid.uuid4().hex
         exp_methods = exp_methods or ['password']
         exp_expires_at = utils.isotime(timeutils.utcnow(), subsecond=True)
@@ -349,12 +350,12 @@ class TestPayloads(unit.TestCase):
         payload = payload_class.assemble(
             exp_user_id, exp_methods, exp_system, exp_project_id,
             exp_domain_id, exp_expires_at, exp_audit_ids, exp_trust_id,
-            exp_federated_info, exp_access_token_id)
+            exp_federated_info, exp_access_token_id, exp_app_cred_id)
 
         (user_id, methods, system, project_id,
          domain_id, expires_at, audit_ids,
          trust_id, federated_info,
-         access_token_id) = payload_class.disassemble(payload)
+         access_token_id, app_cred_id) = payload_class.disassemble(payload)
 
         self.assertEqual(exp_user_id, user_id)
         self.assertEqual(exp_methods, methods)
@@ -365,6 +366,7 @@ class TestPayloads(unit.TestCase):
         self.assertEqual(exp_domain_id, domain_id)
         self.assertEqual(exp_trust_id, trust_id)
         self.assertEqual(exp_access_token_id, access_token_id)
+        self.assertEqual(exp_app_cred_id, app_cred_id)
 
         if exp_federated_info:
             self.assertDictEqual(exp_federated_info, federated_info)
@@ -479,6 +481,18 @@ class TestPayloads(unit.TestCase):
                            exp_project_id=uuid.uuid4().hex,
                            exp_access_token_id=uuid.uuid4().hex)
 
+    def test_app_cred_scoped_payload_with_non_uuid_ids(self):
+        self._test_payload(token_formatters.ApplicationCredentialScopedPayload,
+                           exp_user_id='someNonUuidUserId',
+                           exp_project_id='someNonUuidProjectId',
+                           exp_app_cred_id='someNonUuidAppCredId')
+
+    def test_app_cred_scoped_payload_with_16_char_non_uuid_ids(self):
+        self._test_payload(token_formatters.ApplicationCredentialScopedPayload,
+                           exp_user_id='0123456789abcdef',
+                           exp_project_id='0123456789abcdef',
+                           exp_app_cred_id='0123456789abcdef')
+
 
 class TestFernetKeyRotation(unit.TestCase):
     def setUp(self):
diff --git a/keystone/token/providers/common.py b/keystone/token/providers/common.py
index 64f14dd58..07c0b6ef1 100644
--- a/keystone/token/providers/common.py
+++ b/keystone/token/providers/common.py
@@ -488,12 +488,15 @@ class V3TokenDataHelper(provider_api.ProviderAPIMixin, object):
             LOG.error(msg)
             raise exception.UnexpectedError(msg)
 
-    def _populate_app_cred_restrictions(self, token_data, app_cred_id):
+    def _populate_app_cred(self, token_data, app_cred_id):
         if app_cred_id:
             app_cred_api = PROVIDERS.application_credential_api
             app_cred = app_cred_api.get_application_credential(app_cred_id)
             restricted = not app_cred['unrestricted']
-            token_data['application_credential_restricted'] = restricted
+            token_data['application_credential'] = {}
+            token_data['application_credential']['id'] = app_cred['id']
+            token_data['application_credential']['name'] = app_cred['name']
+            token_data['application_credential']['restricted'] = restricted
 
     def get_token_data(self, user_id, method_names, system=None,
                        domain_id=None, project_id=None, expires=None,
@@ -528,7 +531,7 @@ class V3TokenDataHelper(provider_api.ProviderAPIMixin, object):
         self._populate_token_dates(token_data, expires=expires,
                                    issued_at=issued_at)
         self._populate_oauth_section(token_data, access_token)
-        self._populate_app_cred_restrictions(token_data, app_cred_id)
+        self._populate_app_cred(token_data, app_cred_id)
         return {'token': token_data}
 
 
@@ -669,6 +672,8 @@ class BaseProvider(provider_api.ProviderAPIMixin, base.Provider):
             trust_id = token_ref.get('trust_id')
             if trust_id:
                 trust_ref = PROVIDERS.trust_api.get_trust(trust_id)
+            app_cred_id = token_data['token'].get(
+                'application_credential', {}).get('id')
             token_dict = None
             if token_data['token']['user'].get(
                     federation_constants.FEDERATION):
@@ -677,7 +682,7 @@ class BaseProvider(provider_api.ProviderAPIMixin, base.Provider):
             try:
                 (user_id, methods, audit_ids, system, domain_id,
                     project_id, trust_id, federated_info, access_token_id,
-                    issued_at, expires_at) = (
+                    app_cred_id, issued_at, expires_at) = (
                         self.token_formatter.validate_token(token_id))
             except exception.ValidationError as e:
                 raise exception.TokenNotFound(e)
@@ -726,4 +731,5 @@ class BaseProvider(provider_api.ProviderAPIMixin, base.Provider):
             token=token_dict,
             bind=bind,
             access_token=access_token,
-            audit_info=audit_ids)
+            audit_info=audit_ids,
+            app_cred_id=app_cred_id)
diff --git a/keystone/token/providers/fernet/core.py b/keystone/token/providers/fernet/core.py
index 33964cfc0..078364934 100644
--- a/keystone/token/providers/fernet/core.py
+++ b/keystone/token/providers/fernet/core.py
@@ -170,6 +170,8 @@ class Provider(common.BaseProvider):
         access_token_id = token_data['token'].get('OS-OAUTH1', {}).get(
             'access_token_id')
         federated_info = self._build_federated_info(token_data)
+        app_cred_id = token_data['token'].get('application_credential',
+                                              {}).get('id')
 
         return self.token_formatter.create_token(
             user_id,
@@ -181,7 +183,8 @@ class Provider(common.BaseProvider):
             project_id=project_id,
             trust_id=trust_id,
             federated_info=federated_info,
-            access_token_id=access_token_id
+            access_token_id=access_token_id,
+            app_cred_id=app_cred_id
         )
 
     @property
diff --git a/keystone/token/token_formatters.py b/keystone/token/token_formatters.py
index 5d7b6d3a3..d7d560be3 100644
--- a/keystone/token/token_formatters.py
+++ b/keystone/token/token_formatters.py
@@ -137,20 +137,22 @@ class TokenFormatter(object):
 
     def create_token(self, user_id, expires_at, audit_ids, methods=None,
                      system=None, domain_id=None, project_id=None,
-                     trust_id=None, federated_info=None, access_token_id=None):
+                     trust_id=None, federated_info=None, access_token_id=None,
+                     app_cred_id=None):
         """Given a set of payload attributes, generate a Fernet token."""
         for payload_class in PAYLOAD_CLASSES:
             if payload_class.create_arguments_apply(
                     project_id=project_id, domain_id=domain_id,
                     system=system, trust_id=trust_id,
                     federated_info=federated_info,
-                    access_token_id=access_token_id):
+                    access_token_id=access_token_id,
+                    app_cred_id=app_cred_id):
                 break
 
         version = payload_class.version
         payload = payload_class.assemble(
             user_id, methods, system, project_id, domain_id, expires_at,
-            audit_ids, trust_id, federated_info, access_token_id
+            audit_ids, trust_id, federated_info, access_token_id, app_cred_id
         )
 
         versioned_payload = (version,) + payload
@@ -184,7 +186,8 @@ class TokenFormatter(object):
             if version == payload_class.version:
                 (user_id, methods, system, project_id, domain_id,
                  expires_at, audit_ids, trust_id, federated_info,
-                 access_token_id) = payload_class.disassemble(payload)
+                 access_token_id,
+                 app_cred_id) = payload_class.disassemble(payload)
                 break
         else:
             # If the token_format is not recognized, raise ValidationError.
@@ -200,8 +203,8 @@ class TokenFormatter(object):
         expires_at = ks_utils.isotime(at=expires_at, subsecond=True)
 
         return (user_id, methods, audit_ids, system, domain_id, project_id,
-                trust_id, federated_info, access_token_id, issued_at,
-                expires_at)
+                trust_id, federated_info, access_token_id, app_cred_id,
+                issued_at, expires_at)
 
 
 class BasePayload(object):
@@ -222,7 +225,7 @@ class BasePayload(object):
     @classmethod
     def assemble(cls, user_id, methods, system, project_id, domain_id,
                  expires_at, audit_ids, trust_id, federated_info,
-                 access_token_id):
+                 access_token_id, app_cred_id):
         """Assemble the payload of a token.
 
         :param user_id: identifier of the user in the token request
@@ -237,6 +240,7 @@ class BasePayload(object):
                                provider ID, protocol ID, and federated domain
                                ID
         :param access_token_id: ID of the secret in OAuth1 authentication
+        :param app_cred_id: ID of the application credential in effect
         :returns: the payload of a token
 
         """
@@ -250,7 +254,7 @@ class BasePayload(object):
 
             (user_id, methods, system, project_id, domain_id,
              expires_at_str, audit_ids, trust_id, federated_info,
-             access_token_id)
+             access_token_id, app_cred_id)
 
         * ``methods`` are the auth methods.
         * federated_info is a dict contains the group IDs, the identity
@@ -362,7 +366,7 @@ class UnscopedPayload(BasePayload):
     @classmethod
     def assemble(cls, user_id, methods, system, project_id, domain_id,
                  expires_at, audit_ids, trust_id, federated_info,
-                 access_token_id):
+                 access_token_id, app_cred_id):
         b_user_id = cls.attempt_convert_uuid_hex_to_bytes(user_id)
         methods = auth_plugins.convert_method_list_to_integer(methods)
         expires_at_int = cls._convert_time_string_to_float(expires_at)
@@ -384,9 +388,10 @@ class UnscopedPayload(BasePayload):
         trust_id = None
         federated_info = None
         access_token_id = None
+        app_cred_id = None
         return (user_id, methods, system, project_id, domain_id,
                 expires_at_str, audit_ids, trust_id, federated_info,
-                access_token_id)
+                access_token_id, app_cred_id)
 
 
 class DomainScopedPayload(BasePayload):
@@ -399,7 +404,7 @@ class DomainScopedPayload(BasePayload):
     @classmethod
     def assemble(cls, user_id, methods, system, project_id, domain_id,
                  expires_at, audit_ids, trust_id, federated_info,
-                 access_token_id):
+                 access_token_id, app_cred_id):
         b_user_id = cls.attempt_convert_uuid_hex_to_bytes(user_id)
         methods = auth_plugins.convert_method_list_to_integer(methods)
         try:
@@ -438,9 +443,10 @@ class DomainScopedPayload(BasePayload):
         trust_id = None
         federated_info = None
         access_token_id = None
+        app_cred_id = None
         return (user_id, methods, system, project_id, domain_id,
                 expires_at_str, audit_ids, trust_id, federated_info,
-                access_token_id)
+                access_token_id, app_cred_id)
 
 
 class ProjectScopedPayload(BasePayload):
@@ -453,7 +459,7 @@ class ProjectScopedPayload(BasePayload):
     @classmethod
     def assemble(cls, user_id, methods, system, project_id, domain_id,
                  expires_at, audit_ids, trust_id, federated_info,
-                 access_token_id):
+                 access_token_id, app_cred_id):
         b_user_id = cls.attempt_convert_uuid_hex_to_bytes(user_id)
         methods = auth_plugins.convert_method_list_to_integer(methods)
         b_project_id = cls.attempt_convert_uuid_hex_to_bytes(project_id)
@@ -478,9 +484,10 @@ class ProjectScopedPayload(BasePayload):
         trust_id = None
         federated_info = None
         access_token_id = None
+        app_cred_id = None
         return (user_id, methods, system, project_id, domain_id,
                 expires_at_str, audit_ids, trust_id, federated_info,
-                access_token_id)
+                access_token_id, app_cred_id)
 
 
 class TrustScopedPayload(BasePayload):
@@ -493,7 +500,7 @@ class TrustScopedPayload(BasePayload):
     @classmethod
     def assemble(cls, user_id, methods, system, project_id, domain_id,
                  expires_at, audit_ids, trust_id, federated_info,
-                 access_token_id):
+                 access_token_id, app_cred_id):
         b_user_id = cls.attempt_convert_uuid_hex_to_bytes(user_id)
         methods = auth_plugins.convert_method_list_to_integer(methods)
         b_project_id = cls.attempt_convert_uuid_hex_to_bytes(project_id)
@@ -521,9 +528,10 @@ class TrustScopedPayload(BasePayload):
         domain_id = None
         federated_info = None
         access_token_id = None
+        app_cred_id = None
         return (user_id, methods, system, project_id, domain_id,
                 expires_at_str, audit_ids, trust_id, federated_info,
-                access_token_id)
+                access_token_id, app_cred_id)
 
 
 class FederatedUnscopedPayload(BasePayload):
@@ -547,7 +555,7 @@ class FederatedUnscopedPayload(BasePayload):
     @classmethod
     def assemble(cls, user_id, methods, system, project_id, domain_id,
                  expires_at, audit_ids, trust_id, federated_info,
-                 access_token_id):
+                 access_token_id, app_cred_id):
         b_user_id = cls.attempt_convert_uuid_hex_to_bytes(user_id)
         methods = auth_plugins.convert_method_list_to_integer(methods)
         b_group_ids = list(map(cls.pack_group_id,
@@ -586,9 +594,10 @@ class FederatedUnscopedPayload(BasePayload):
         domain_id = None
         trust_id = None
         access_token_id = None
+        app_cred_id = None
         return (user_id, methods, system, project_id, domain_id,
                 expires_at_str, audit_ids, trust_id, federated_info,
-                access_token_id)
+                access_token_id, app_cred_id)
 
 
 class FederatedScopedPayload(FederatedUnscopedPayload):
@@ -597,7 +606,7 @@ class FederatedScopedPayload(FederatedUnscopedPayload):
     @classmethod
     def assemble(cls, user_id, methods, system, project_id, domain_id,
                  expires_at, audit_ids, trust_id, federated_info,
-                 access_token_id):
+                 access_token_id, app_cred_id):
         b_user_id = cls.attempt_convert_uuid_hex_to_bytes(user_id)
         methods = auth_plugins.convert_method_list_to_integer(methods)
         b_scope_id = cls.attempt_convert_uuid_hex_to_bytes(
@@ -641,9 +650,10 @@ class FederatedScopedPayload(FederatedUnscopedPayload):
         system = None
         trust_id = None
         access_token_id = None
+        app_cred_id = None
         return (user_id, methods, system, project_id, domain_id,
                 expires_at_str, audit_ids, trust_id, federated_info,
-                access_token_id)
+                access_token_id, app_cred_id)
 
 
 class FederatedProjectScopedPayload(FederatedScopedPayload):
@@ -672,7 +682,7 @@ class OauthScopedPayload(BasePayload):
     @classmethod
     def assemble(cls, user_id, methods, system, project_id, domain_id,
                  expires_at, audit_ids, trust_id, federated_info,
-                 access_token_id):
+                 access_token_id, app_cred_id):
         b_user_id = cls.attempt_convert_uuid_hex_to_bytes(user_id)
         methods = auth_plugins.convert_method_list_to_integer(methods)
         b_project_id = cls.attempt_convert_uuid_hex_to_bytes(project_id)
@@ -702,10 +712,11 @@ class OauthScopedPayload(BasePayload):
         domain_id = None
         trust_id = None
         federated_info = None
+        app_cred_id = None
 
         return (user_id, methods, system, project_id, domain_id,
                 expires_at_str, audit_ids, trust_id, federated_info,
-                access_token_id)
+                access_token_id, app_cred_id)
 
 
 class SystemScopedPayload(BasePayload):
@@ -718,7 +729,7 @@ class SystemScopedPayload(BasePayload):
     @classmethod
     def assemble(cls, user_id, methods, system, project_id, domain_id,
                  expires_at, audit_ids, trust_id, federated_info,
-                 access_token_id):
+                 access_token_id, app_cred_id):
         b_user_id = cls.attempt_convert_uuid_hex_to_bytes(user_id)
         methods = auth_plugins.convert_method_list_to_integer(methods)
         expires_at_int = cls._convert_time_string_to_float(expires_at)
@@ -740,9 +751,55 @@ class SystemScopedPayload(BasePayload):
         trust_id = None
         federated_info = None
         access_token_id = None
+        app_cred_id = None
         return (user_id, methods, system, project_id, domain_id,
                 expires_at_str, audit_ids, trust_id, federated_info,
-                access_token_id)
+                access_token_id, app_cred_id)
+
+
+class ApplicationCredentialScopedPayload(BasePayload):
+    version = 9
+
+    @classmethod
+    def create_arguments_apply(cls, **kwargs):
+        return kwargs['app_cred_id']
+
+    @classmethod
+    def assemble(cls, user_id, methods, system, project_id, domain_id,
+                 expires_at, audit_ids, trust_id, federated_info,
+                 access_token_id, app_cred_id):
+        b_user_id = cls.attempt_convert_uuid_hex_to_bytes(user_id)
+        methods = auth_plugins.convert_method_list_to_integer(methods)
+        b_project_id = cls.attempt_convert_uuid_hex_to_bytes(project_id)
+        expires_at_int = cls._convert_time_string_to_float(expires_at)
+        b_audit_ids = list(map(cls.random_urlsafe_str_to_bytes,
+                           audit_ids))
+        b_app_cred_id = cls.attempt_convert_uuid_hex_to_bytes(app_cred_id)
+        return (b_user_id, methods, b_project_id, expires_at_int, b_audit_ids,
+                b_app_cred_id)
+
+    @classmethod
+    def disassemble(cls, payload):
+        (is_stored_as_bytes, user_id) = payload[0]
+        if is_stored_as_bytes:
+            user_id = cls.convert_uuid_bytes_to_hex(user_id)
+        methods = auth_plugins.convert_integer_to_method_list(payload[1])
+        (is_stored_as_bytes, project_id) = payload[2]
+        if is_stored_as_bytes:
+            project_id = cls.convert_uuid_bytes_to_hex(project_id)
+        expires_at_str = cls._convert_float_to_time_string(payload[3])
+        audit_ids = list(map(cls.base64_encode, payload[4]))
+        system = None
+        domain_id = None
+        trust_id = None
+        federated_info = None
+        access_token_id = None
+        (is_stored_as_bytes, app_cred_id) = payload[5]
+        if is_stored_as_bytes:
+            app_cred_id = cls.convert_uuid_bytes_to_hex(app_cred_id)
+        return (user_id, methods, system, project_id, domain_id,
+                expires_at_str, audit_ids, trust_id, federated_info,
+                access_token_id, app_cred_id)
 
 
 # For now, the order of the classes in the following list is important. This
@@ -758,6 +815,7 @@ PAYLOAD_CLASSES = [
     FederatedProjectScopedPayload,
     FederatedDomainScopedPayload,
     FederatedUnscopedPayload,
+    ApplicationCredentialScopedPayload,
     ProjectScopedPayload,
     DomainScopedPayload,
     SystemScopedPayload,
diff --git a/keystone/trust/controllers.py b/keystone/trust/controllers.py
index c40648cb0..b33cb28e1 100644
--- a/keystone/trust/controllers.py
+++ b/keystone/trust/controllers.py
@@ -103,7 +103,8 @@ class TrustV3(controller.V3Controller):
     def _check_unrestricted(self, token):
         auth_methods = token['methods']
         if 'application_credential' in auth_methods:
-            if token.token_data['token']['application_credential_restricted']:
+            td = token.token_data['token']
+            if td['application_credential']['restricted']:
                 action = _("Using method 'application_credential' is not "
                            "allowed for managing trusts.")
                 raise exception.ForbiddenAction(action=action)
diff --git a/releasenotes/notes/bug-1750415-95ede3a9685b6e0c.yaml b/releasenotes/notes/bug-1750415-95ede3a9685b6e0c.yaml
new file mode 100644
index 000000000..bda17f62e
--- /dev/null
+++ b/releasenotes/notes/bug-1750415-95ede3a9685b6e0c.yaml
@@ -0,0 +1,7 @@
+---
+fixes:
+  - |
+    [`bug 1750415 <https://bugs.launchpad.net/keystone/+bug/1750415>`_]
+    Fixes an implementation fault in application credentials where the
+    application credential reference was not populated in the token data,
+    causing problems with the token validation when caching was disabled.