Skip to content

Commit 7f3417b

Browse files
Fix: raise clear error when signing without api_secret
Add guards to cloudinary_url() and api_sign_request() to raise ValueError("Must supply api_secret") instead of opaque TypeError when attempting to sign without a configured secret. This matches the behavior of other signing paths (sign_request, Search.to_url, etc.) and provides clear guidance to OAuth-only configurations. Changes: - cloudinary_url(): Guard signing block to validate api_secret before use - api_sign_request(): Guard function entry to validate api_secret parameter - Add 7 comprehensive test cases covering all scenarios Fixes spec: cloudinary-url-sign-without-secret.md Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
1 parent bd50a65 commit 7f3417b

2 files changed

Lines changed: 53 additions & 0 deletions

File tree

cloudinary/utils.py

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -642,6 +642,8 @@ def api_sign_request(params_to_sign, api_secret, algorithm=SIGNATURE_SHA1, signa
642642
- Version 2+: Includes parameter encoding to prevent parameter smuggling
643643
:return: Computed signature
644644
"""
645+
if not api_secret:
646+
raise ValueError("Must supply api_secret")
645647
to_sign = api_string_to_sign(params_to_sign, signature_version)
646648
return compute_hex_hash(to_sign + api_secret, algorithm)
647649

@@ -875,6 +877,8 @@ def cloudinary_url(source, **options):
875877

876878
signature = None
877879
if sign_url and (not auth_token or auth_token.pop('set_url_signature', False)):
880+
if not api_secret:
881+
raise ValueError("Must supply api_secret")
878882
to_sign = "/".join(__compact([transformation, source_to_sign]))
879883
if long_url_signature:
880884
# Long signature forces SHA256

test/test_utils.py

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
encode_unicode_url,
2222
base64url_encode,
2323
patch_fetch_format,
24+
cloudinary_url,
2425
cloudinary_scaled_url,
2526
chain_transformations,
2627
generate_transformation_string,
@@ -1591,6 +1592,54 @@ def test_sign_request_with_signature_version(self):
15911592
self.assertEqual(signed_params_v1['signature'], expected_sig_v1)
15921593
self.assertEqual(signed_params_v2['signature'], expected_sig_v2)
15931594

1595+
def test_cloudinary_url_sign_without_secret_raises(self):
1596+
"""Signing without a secret should raise ValueError"""
1597+
cloudinary.config(cloud_name="test123", api_secret=None)
1598+
with self.assertRaises(ValueError) as context:
1599+
cloudinary_url("sample", sign_url=True)
1600+
self.assertEqual(str(context.exception), "Must supply api_secret")
1601+
1602+
def test_cloudinary_url_unsigned_without_secret_works(self):
1603+
"""Unsigned URL should work without a secret"""
1604+
cloudinary.config(cloud_name="test123", api_secret=None)
1605+
url, _ = cloudinary_url("sample")
1606+
self.assertIn("test123", url)
1607+
self.assertNotIn("s--", url)
1608+
1609+
def test_cloudinary_url_sign_with_secret_works(self):
1610+
"""Signing with a secret should work and include signature"""
1611+
cloudinary.config(cloud_name="test123", api_key="key", api_secret="secret")
1612+
url, _ = cloudinary_url("sample", sign_url=True)
1613+
self.assertIn("s--", url)
1614+
self.assertIn("test123", url)
1615+
1616+
def test_cloudinary_url_per_call_secret_override(self):
1617+
"""Per-call api_secret override should sign successfully"""
1618+
cloudinary.config(cloud_name="test123", api_secret=None)
1619+
url, _ = cloudinary_url("sample", sign_url=True, api_secret="override_secret")
1620+
self.assertIn("s--", url)
1621+
self.assertIn("test123", url)
1622+
1623+
def test_api_sign_request_without_secret_raises(self):
1624+
"""api_sign_request with None secret should raise ValueError"""
1625+
params = {"a": "b"}
1626+
with self.assertRaises(ValueError) as context:
1627+
api_sign_request(params, None)
1628+
self.assertEqual(str(context.exception), "Must supply api_secret")
1629+
1630+
def test_api_sign_request_with_empty_string_raises(self):
1631+
"""api_sign_request with empty string secret should raise ValueError"""
1632+
params = {"a": "b"}
1633+
with self.assertRaises(ValueError) as context:
1634+
api_sign_request(params, "")
1635+
self.assertEqual(str(context.exception), "Must supply api_secret")
1636+
1637+
def test_api_sign_request_with_secret_works(self):
1638+
"""api_sign_request with a real secret should work"""
1639+
params = dict(cloud_name=API_SIGN_REQUEST_CLOUD_NAME, timestamp=1568810420, username="user@cloudinary.com")
1640+
signature = api_sign_request(params, API_SIGN_REQUEST_TEST_SECRET)
1641+
self.assertEqual(signature, "14c00ba6d0dfdedbc86b316847d95b9e6cd46d94")
1642+
15941643

15951644
if __name__ == '__main__':
15961645
unittest.main()

0 commit comments

Comments
 (0)