Skip to content

Commit bffecf4

Browse files
Fix verify_api_response_signature backward compatibility
1 parent 797c9dd commit bffecf4

File tree

2 files changed

+85
-8
lines changed

2 files changed

+85
-8
lines changed

cloudinary/utils.py

Lines changed: 46 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -627,15 +627,50 @@ def sign_request(params, options):
627627
return params
628628

629629

630-
def api_sign_request(params_to_sign, api_secret, algorithm=SIGNATURE_SHA1):
631-
params = [_encode_param(k + "=" + (
632-
",".join(v) if isinstance(v, list) else
633-
str(v).lower() if isinstance(v, bool) else
634-
str(v)
635-
)) for k, v in params_to_sign.items() if v]
636-
to_sign = "&".join(sorted(params))
630+
def api_sign_request(params_to_sign, api_secret, algorithm=SIGNATURE_SHA1, signature_version=2):
631+
"""
632+
Signs API request parameters using the specified algorithm and signature version.
633+
634+
:param params_to_sign: Parameters to include in the signature
635+
:param api_secret: API secret key
636+
:param algorithm: Signature algorithm (default: SHA1)
637+
:param signature_version: Signature version (default: 2)
638+
- Version 1: Original behavior without parameter encoding
639+
- Version 2+: Includes parameter encoding to prevent parameter smuggling
640+
:return: Computed signature
641+
"""
642+
to_sign = api_string_to_sign(params_to_sign, signature_version)
637643
return compute_hex_hash(to_sign + api_secret, algorithm)
638644

645+
646+
def api_string_to_sign(params_to_sign, signature_version=2):
647+
"""
648+
Generates a string to be signed for API requests.
649+
650+
:param params_to_sign: Parameters to include in the signature
651+
:param signature_version: Version of signature algorithm to use:
652+
- Version 1: Original behavior without parameter encoding
653+
- Version 2+ (default): Includes parameter encoding to prevent parameter smuggling
654+
:return: String to be signed
655+
"""
656+
params = []
657+
for k, v in params_to_sign.items():
658+
if v:
659+
if isinstance(v, list):
660+
value = ",".join(v)
661+
elif isinstance(v, bool):
662+
value = str(v).lower()
663+
else:
664+
value = str(v)
665+
666+
param_string = k + "=" + value
667+
if signature_version >= 2:
668+
param_string = _encode_param(param_string)
669+
params.append(param_string)
670+
671+
return "&".join(sorted(params))
672+
673+
639674
def _encode_param(value):
640675
"""
641676
Encodes a parameter for safe inclusion in URL query strings.
@@ -648,6 +683,7 @@ def _encode_param(value):
648683
"""
649684
return str(value).replace("&", "%26")
650685

686+
651687
def breakpoint_settings_mapper(breakpoint_settings):
652688
breakpoint_settings = copy.deepcopy(breakpoint_settings)
653689
transformation = breakpoint_settings.get("transformation")
@@ -1587,10 +1623,12 @@ def verify_api_response_signature(public_id, version, signature, algorithm=None)
15871623
parameters_to_sign = {'public_id': public_id,
15881624
'version': version}
15891625

1626+
# Use signature version 1 for backward compatibility
15901627
return signature == api_sign_request(
15911628
parameters_to_sign,
15921629
cloudinary.config().api_secret,
1593-
algorithm or cloudinary.config().signature_algorithm
1630+
algorithm or cloudinary.config().signature_algorithm,
1631+
signature_version=1
15941632
)
15951633

15961634

test/test_utils.py

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1489,6 +1489,45 @@ def test_api_sign_request_prevents_parameter_smuggling(self):
14891489
expected_smuggled_signature = "7b4e3a539ff1fa6e6700c41b3a2ee77586a025f9"
14901490
self.assertEqual(expected_smuggled_signature, signature_smuggled)
14911491

1492+
def test_api_sign_request_signature_versions(self):
1493+
"""Should use signature version 1 (without parameter encoding) for backward compatibility"""
1494+
public_id_with_ampersand = 'tests/logo&version=2'
1495+
test_version = 1
1496+
1497+
expected_signature_v1 = api_sign_request(
1498+
{'public_id': public_id_with_ampersand, 'version': test_version},
1499+
API_SIGN_REQUEST_TEST_SECRET,
1500+
cloudinary.utils.SIGNATURE_SHA1,
1501+
signature_version=1
1502+
)
1503+
1504+
expected_signature_v2 = api_sign_request(
1505+
{'public_id': public_id_with_ampersand, 'version': test_version},
1506+
API_SIGN_REQUEST_TEST_SECRET,
1507+
cloudinary.utils.SIGNATURE_SHA1,
1508+
signature_version=2
1509+
)
1510+
1511+
self.assertNotEqual(expected_signature_v1, expected_signature_v2)
1512+
1513+
# verify_api_response_signature should use version 1 for backward compatibility
1514+
with patch('cloudinary.config', return_value=cloudinary.config(api_secret=API_SIGN_REQUEST_TEST_SECRET)):
1515+
self.assertTrue(
1516+
verify_api_response_signature(
1517+
public_id_with_ampersand,
1518+
test_version,
1519+
expected_signature_v1
1520+
)
1521+
)
1522+
1523+
self.assertFalse(
1524+
verify_api_response_signature(
1525+
public_id_with_ampersand,
1526+
test_version,
1527+
expected_signature_v2
1528+
)
1529+
)
1530+
14921531

14931532
if __name__ == '__main__':
14941533
unittest.main()

0 commit comments

Comments
 (0)