From afca5cc43bc2442bd95524b7ad030d2e7965902c Mon Sep 17 00:00:00 2001 From: Lance Bragstad Date: Mon, 19 Feb 2018 18:23:25 +0000 Subject: [PATCH] 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 Change-Id: I322a40404d8287748fe8c3a8d6dc1256d935d84a Closes-bug: #1750415 (cherry picked from commit 796198f19670e3eb899ca3b1db5d2a21a4127a30) --- .../application_credential/controllers.py | 3 +- keystone/tests/unit/test_v3_auth.py | 21 ++++ .../tests/unit/token/test_fernet_provider.py | 20 +++- keystone/token/providers/common.py | 16 ++- keystone/token/providers/fernet/core.py | 5 +- keystone/token/token_formatters.py | 106 ++++++++++++++---- keystone/trust/controllers.py | 3 +- .../notes/bug-1750415-95ede3a9685b6e0c.yaml | 7 ++ 8 files changed, 146 insertions(+), 35 deletions(-) create mode 100644 releasenotes/notes/bug-1750415-95ede3a9685b6e0c.yaml diff --git a/keystone/application_credential/controllers.py b/keystone/application_credential/controllers.py index 194eb7a2d7..0cb2df2c81 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 d1177b6c0f..f83df746dc 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 da7e820230..bf93acf6c6 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 64f14dd583..07c0b6ef13 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 33964cfc03..078364934f 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 5d7b6d3a37..d7d560be3e 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 c40648cb07..b33cb28e17 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 0000000000..bda17f62e6 --- /dev/null +++ b/releasenotes/notes/bug-1750415-95ede3a9685b6e0c.yaml @@ -0,0 +1,7 @@ +--- +fixes: + - | + [`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.