diff --git a/keystone/tests/unit/token/test_fernet_provider.py b/keystone/tests/unit/token/test_fernet_provider.py index b2661b6801..a3e6d870c1 100644 --- a/keystone/tests/unit/token/test_fernet_provider.py +++ b/keystone/tests/unit/token/test_fernet_provider.py @@ -352,21 +352,73 @@ class TestPayloads(unit.TestCase): actual_time_float) self.assertEqual(expected_time_str, actual_time_str) + def test_convert_or_decode_uuid_bytes(self): + payload_cls = token_formatters.BasePayload + + expected_hex_uuid = uuid.uuid4().hex + uuid_obj = uuid.UUID(expected_hex_uuid) + expected_uuid_in_bytes = uuid_obj.bytes + + actual_hex_uuid = payload_cls._convert_or_decode( + is_stored_as_bytes=True, + value=expected_uuid_in_bytes + ) + + self.assertEqual(expected_hex_uuid, actual_hex_uuid) + + def test_convert_or_decode_binary_type(self): + payload_cls = token_formatters.BasePayload + + expected_hex_uuid = uuid.uuid4().hex + + actual_hex_uuid = payload_cls._convert_or_decode( + is_stored_as_bytes=False, + value=expected_hex_uuid.encode('utf-8') + ) + + self.assertEqual(expected_hex_uuid, actual_hex_uuid) + + def test_convert_or_decode_text_type(self): + payload_cls = token_formatters.BasePayload + + expected_hex_uuid = uuid.uuid4().hex + + actual_hex_uuid = payload_cls._convert_or_decode( + is_stored_as_bytes=False, + value=expected_hex_uuid + ) + + self.assertEqual(expected_hex_uuid, actual_hex_uuid) + 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_group_ids=None, exp_identity_provider_id=None, exp_protocol_id=None, - exp_access_token_id=None, exp_app_cred_id=None): + exp_access_token_id=None, exp_app_cred_id=None, + encode_ids=False): + def _encode_id(value): + if value is not None and six.text_type(value) and encode_ids: + return value.encode('utf-8') + return value 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) exp_audit_ids = [provider.random_urlsafe_str()] 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_group_ids, exp_identity_provider_id, exp_protocol_id, - exp_access_token_id, exp_app_cred_id) + _encode_id(exp_user_id), + exp_methods, + _encode_id(exp_system), + _encode_id(exp_project_id), + exp_domain_id, + exp_expires_at, + exp_audit_ids, + exp_trust_id, + _encode_id(exp_federated_group_ids), + _encode_id(exp_identity_provider_id), + exp_protocol_id, + _encode_id(exp_access_token_id), + _encode_id(exp_app_cred_id)) (user_id, methods, system, project_id, domain_id, expires_at, audit_ids, @@ -429,6 +481,12 @@ class TestPayloads(unit.TestCase): exp_user_id='0123456789abcdef', exp_project_id='0123456789abcdef') + def test_project_scoped_payload_with_binary_encoded_ids(self): + self._test_payload(token_formatters.ProjectScopedPayload, + exp_user_id='someNonUuidUserId', + exp_project_id='someNonUuidProjectId', + encode_ids=True) + def test_domain_scoped_payload_with_non_uuid_user_id(self): self._test_payload(token_formatters.DomainScopedPayload, exp_user_id='nonUuidUserId', diff --git a/keystone/token/token_formatters.py b/keystone/token/token_formatters.py index cfd22010a8..182755cc67 100644 --- a/keystone/token/token_formatters.py +++ b/keystone/token/token_formatters.py @@ -313,9 +313,11 @@ class BasePayload(object): """ try: return (True, cls.convert_uuid_hex_to_bytes(value)) - except ValueError: - # this might not be a UUID, depending on the situation (i.e. - # federation) + except (ValueError, TypeError): + # ValueError: this might not be a UUID, depending on the + # situation (i.e. federation) + # TypeError: the provided value may be binary encoded + # in which case just return the value (i.e. Python 3) return (False, value) @classmethod @@ -344,6 +346,22 @@ class BasePayload(object): # restore the padding (==) at the end of the string return base64.urlsafe_b64decode(s + '==') + @classmethod + def _convert_or_decode(cls, is_stored_as_bytes, value): + """Convert a value to text type, translating uuid -> hex if required. + + :param is_stored_as_bytes: whether value is already bytes + :type is_stored_as_bytes: six.boolean + :param value: value to attempt to convert to bytes + :type value: six.text_type or six.binary_type + :rtype: six.text_type + """ + if is_stored_as_bytes: + return cls.convert_uuid_bytes_to_hex(value) + elif isinstance(value, six.binary_type): + return value.decode('utf-8') + return value + class UnscopedPayload(BasePayload): version = 0 @@ -363,8 +381,7 @@ class UnscopedPayload(BasePayload): @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) + user_id = cls._convert_or_decode(is_stored_as_bytes, user_id) methods = auth_plugins.convert_integer_to_method_list(payload[1]) expires_at_str = cls._convert_float_to_time_string(payload[2]) audit_ids = list(map(cls.base64_encode, payload[3])) @@ -409,8 +426,7 @@ class DomainScopedPayload(BasePayload): @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) + user_id = cls._convert_or_decode(is_stored_as_bytes, user_id) methods = auth_plugins.convert_integer_to_method_list(payload[1]) try: domain_id = cls.convert_uuid_bytes_to_hex(payload[2]) @@ -457,12 +473,10 @@ class ProjectScopedPayload(BasePayload): @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) + user_id = cls._convert_or_decode(is_stored_as_bytes, 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) + project_id = cls._convert_or_decode(is_stored_as_bytes, project_id) expires_at_str = cls._convert_float_to_time_string(payload[3]) audit_ids = list(map(cls.base64_encode, payload[4])) system = None @@ -501,12 +515,10 @@ class TrustScopedPayload(BasePayload): @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) + user_id = cls._convert_or_decode(is_stored_as_bytes, 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) + project_id = cls._convert_or_decode(is_stored_as_bytes, project_id) expires_at_str = cls._convert_float_to_time_string(payload[3]) audit_ids = list(map(cls.base64_encode, payload[4])) trust_id = cls.convert_uuid_bytes_to_hex(payload[5]) @@ -533,8 +545,7 @@ class FederatedUnscopedPayload(BasePayload): @classmethod def unpack_group_id(cls, group_id_in_bytes): (is_stored_as_bytes, group_id) = group_id_in_bytes - if is_stored_as_bytes: - group_id = cls.convert_uuid_bytes_to_hex(group_id) + group_id = cls._convert_or_decode(is_stored_as_bytes, group_id) return {'id': group_id} @classmethod @@ -556,24 +567,11 @@ class FederatedUnscopedPayload(BasePayload): @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) - else: - # NOTE(cmurphy): The user ID of shadowed federated users is no - # longer a UUID but a sha256 hash string, and so it should not be - # converted to a byte string since it is not a UUID format. - # However. on python3 msgpack returns the serialized input as a - # byte string anyway. Similar to other msgpack'd values in the - # payload, we need to explicitly decode it to a string value. - if six.PY3 and isinstance(user_id, six.binary_type): - user_id = user_id.decode('utf-8') + user_id = cls._convert_or_decode(is_stored_as_bytes, user_id) methods = auth_plugins.convert_integer_to_method_list(payload[1]) group_ids = list(map(cls.unpack_group_id, payload[2])) (is_stored_as_bytes, idp_id) = payload[3] - if is_stored_as_bytes: - idp_id = cls.convert_uuid_bytes_to_hex(idp_id) - else: - idp_id = idp_id.decode('utf-8') + idp_id = cls._convert_or_decode(is_stored_as_bytes, idp_id) protocol_id = payload[4] if isinstance(protocol_id, six.binary_type): protocol_id = protocol_id.decode('utf-8') @@ -614,38 +612,10 @@ class FederatedScopedPayload(FederatedUnscopedPayload): @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) - else: - # NOTE(cmurphy): The user ID of shadowed federated users is no - # longer a UUID but a sha256 hash string, and so it should not be - # converted to a byte string since it is not a UUID format. - # However. on python3 msgpack returns the serialized input as a - # byte string anyway. Similar to other msgpack'd values in the - # payload, we need to explicitly decode it to a string value. - if six.PY3 and isinstance(user_id, six.binary_type): - user_id = user_id.decode('utf-8') + user_id = cls._convert_or_decode(is_stored_as_bytes, user_id) methods = auth_plugins.convert_integer_to_method_list(payload[1]) (is_stored_as_bytes, scope_id) = payload[2] - if is_stored_as_bytes: - scope_id = cls.convert_uuid_bytes_to_hex(scope_id) - else: - # NOTE(lbragstad): We assembled the token payload scope as a tuple - # (False, domain_id) for cases like (False, 'default'), since the - # default domain ID isn't converted to a byte string when it's not - # in UUID format. Despite the boolean indicator in the tuple that - # denotes if the value is stored as a byte string or not, msgpack - # apparently returns the serialized input as byte strings anyway. - # For example, this means what we though we were passing in as - # (False, 'default') during token creation actually comes out as - # (False, b'default') in token validation through msgpack, which - # clearly isn't correct according to our boolean indicator. This - # causes comparison issues due to different string types (e.g., - # b'default' != 'default') with python 3. See bug 1813085 for - # details. We use this pattern for other strings in the payload - # like idp_id and protocol_id for the same reason. - if six.PY3 and isinstance(scope_id, six.binary_type): - scope_id = scope_id.decode('utf-8') + scope_id = cls._convert_or_decode(is_stored_as_bytes, scope_id) project_id = ( scope_id if cls.version == FederatedProjectScopedPayload.version else None) @@ -654,11 +624,7 @@ class FederatedScopedPayload(FederatedUnscopedPayload): if cls.version == FederatedDomainScopedPayload.version else None) group_ids = list(map(cls.unpack_group_id, payload[3])) (is_stored_as_bytes, idp_id) = payload[4] - if is_stored_as_bytes: - idp_id = cls.convert_uuid_bytes_to_hex(idp_id) - else: - if six.PY3 and isinstance(idp_id, six.binary_type): - idp_id = idp_id.decode('utf-8') + idp_id = cls._convert_or_decode(is_stored_as_bytes, idp_id) protocol_id = payload[5] if six.PY3 and isinstance(protocol_id, six.binary_type): protocol_id = protocol_id.decode('utf-8') @@ -703,15 +669,13 @@ class OauthScopedPayload(BasePayload): @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) + user_id = cls._convert_or_decode(is_stored_as_bytes, 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) + project_id = cls._convert_or_decode(is_stored_as_bytes, project_id) (is_stored_as_bytes, access_token_id) = payload[3] - if is_stored_as_bytes: - access_token_id = cls.convert_uuid_bytes_to_hex(access_token_id) + access_token_id = cls._convert_or_decode(is_stored_as_bytes, + access_token_id) expires_at_str = cls._convert_float_to_time_string(payload[4]) audit_ids = list(map(cls.base64_encode, payload[5])) system = None @@ -746,8 +710,7 @@ class SystemScopedPayload(BasePayload): @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) + user_id = cls._convert_or_decode(is_stored_as_bytes, user_id) methods = auth_plugins.convert_integer_to_method_list(payload[1]) system = payload[2] expires_at_str = cls._convert_float_to_time_string(payload[3]) @@ -787,12 +750,10 @@ class ApplicationCredentialScopedPayload(BasePayload): @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) + user_id = cls._convert_or_decode(is_stored_as_bytes, 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) + project_id = cls._convert_or_decode(is_stored_as_bytes, project_id) expires_at_str = cls._convert_float_to_time_string(payload[3]) audit_ids = list(map(cls.base64_encode, payload[4])) system = None @@ -803,8 +764,7 @@ class ApplicationCredentialScopedPayload(BasePayload): protocol_id = 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) + app_cred_id = cls._convert_or_decode(is_stored_as_bytes, app_cred_id) return (user_id, methods, system, project_id, domain_id, expires_at_str, audit_ids, trust_id, federated_group_ids, identity_provider_id, protocol_id, access_token_id, diff --git a/releasenotes/notes/bug-1832265-cb76ccf505c2d9d1.yaml b/releasenotes/notes/bug-1832265-cb76ccf505c2d9d1.yaml new file mode 100644 index 0000000000..9dcf16aa7b --- /dev/null +++ b/releasenotes/notes/bug-1832265-cb76ccf505c2d9d1.yaml @@ -0,0 +1,7 @@ +--- +fixes: + - | + [`bug 1832265 `_] + Binary msgpack payload types are now consistently and correctly decoded + when running Keystone under Python 3, avoiding any TypeErrors when + attempting to convert binary encoded strings into UUID's.