Skip to content

Commit 77d2917

Browse files
const-cloudinaryconst-cloudinary
authored
Fix response signature verification
1 parent d0d7733 commit 77d2917

File tree

7 files changed

+169
-28
lines changed

7 files changed

+169
-28
lines changed

‎lib/cloudinary/cloudinary_controller.rb‎

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2,10 +2,12 @@ module Cloudinary::CloudinaryController
22
protected
33

44
def valid_cloudinary_response?
5-
received_signature = request.query_parameters[:signature]
6-
calculated_signature = Cloudinary::Utils.api_sign_request(
7-
request.query_parameters.select{|key, value| [:public_id, :version].include?(key.to_sym)},
8-
Cloudinary.config.api_secret)
9-
return received_signature == calculated_signature
5+
params = request.query_parameters.select { |key, value| [:public_id, :version, :signature].include?(key.to_sym) }.transform_keys(&:to_sym)
6+
7+
Cloudinary::Utils.verify_api_response_signature(
8+
params[:public_id],
9+
params[:version],
10+
params[:signature]
11+
)
1012
end
1113
end

‎lib/cloudinary/preloaded_file.rb‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,8 +9,7 @@ def initialize(file_info)
99

1010
def valid?
1111
public_id = @resource_type == "raw" ? self.filename : self.public_id
12-
expected_signature = Cloudinary::Utils.api_sign_request({:public_id=>public_id, :version=>version}, Cloudinary.config.api_secret)
13-
@signature == expected_signature
12+
Cloudinary::Utils.verify_api_response_signature(public_id, version, @signature)
1413
end
1514

1615
def identifier

‎lib/cloudinary/uploader.rb‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -360,7 +360,8 @@ def self.call_api(action, options)
360360
api_key = options[:api_key] || Cloudinary.config.api_key || raise(CloudinaryException, "Must supply api_key")
361361
api_secret = options[:api_secret] || Cloudinary.config.api_secret || raise(CloudinaryException, "Must supply api_secret")
362362
signature_algorithm = options[:signature_algorithm]
363-
params[:signature] = Cloudinary::Utils.api_sign_request(params.reject { |k, v| non_signable.include?(k) }, api_secret, signature_algorithm)
363+
signature_version = options[:signature_version]
364+
params[:signature] = Cloudinary::Utils.api_sign_request(params.reject { |k, v| non_signable.include?(k) }, api_secret, signature_algorithm, signature_version)
364365
params[:api_key] = api_key
365366
end
366367

‎lib/cloudinary/utils.rb‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -504,13 +504,14 @@ def self.api_string_to_sign(params_to_sign, signature_version = 2)
504504
#
505505
# @param [Hash] params_to_sign Parameters to include in the signature
506506
# @param [String] api_secret API secret for signing
507-
# @param [Symbol] signature_algorithm Hash algorithm to use (:sha1 or :sha256)
508-
# @param [Integer] signature_version Version of signature algorithm to use:
507+
# @param [Symbol|nil] signature_algorithm Hash algorithm to use (:sha1 or :sha256)
508+
# @param [Integer|nil] signature_version Version of signature algorithm to use:
509509
# - Version 1: Original behavior without parameter encoding
510510
# - Version 2+ (default): Includes parameter encoding to prevent parameter smuggling
511511
# @return [String] Hexadecimal signature
512512
# @private
513-
def self.api_sign_request(params_to_sign, api_secret, signature_algorithm = nil, signature_version = 2)
513+
def self.api_sign_request(params_to_sign, api_secret, signature_algorithm = nil, signature_version = nil)
514+
signature_version ||= Cloudinary.config.signature_version || 2
514515
to_sign = api_string_to_sign(params_to_sign, signature_version)
515516
hash("#{to_sign}#{api_secret}", signature_algorithm, :hexdigest)
516517
end
@@ -783,8 +784,9 @@ def self.sign_request(params, options={})
783784
api_key = options[:api_key] || Cloudinary.config.api_key || raise(CloudinaryException, "Must supply api_key")
784785
api_secret = options[:api_secret] || Cloudinary.config.api_secret || raise(CloudinaryException, "Must supply api_secret")
785786
signature_algorithm = options[:signature_algorithm]
787+
signature_version = options[:signature_version]
786788
params = params.reject{|k, v| self.safe_blank?(v)}
787-
params[:signature] = api_sign_request(params, api_secret, signature_algorithm)
789+
params[:signature] = api_sign_request(params, api_secret, signature_algorithm, signature_version)
788790
params[:api_key] = api_key
789791
params
790792
end

‎spec/carriewave_spec.rb‎

Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,3 +32,83 @@ class SanitizedFile; end
3232
end
3333
end
3434
end
35+
36+
RSpec.describe Cloudinary::PreloadedFile do
37+
let(:test_api_secret) { "X7qLTrsES31MzxxkxPPA-pAGGfU" }
38+
39+
before do
40+
Cloudinary.config.update(:api_secret => test_api_secret)
41+
end
42+
43+
describe "folder support" do
44+
it "should allow to use folders in PreloadedFile" do
45+
signature = Cloudinary::Utils.api_sign_request({ :public_id => "folder/file", :version => "1234" }, Cloudinary.config.api_secret)
46+
preloaded = Cloudinary::PreloadedFile.new("image/upload/v1234/folder/file.jpg#" + signature)
47+
expect(preloaded).to be_valid
48+
[
49+
[:filename, 'folder/file.jpg'],
50+
[:version, '1234'],
51+
[:public_id, 'folder/file'],
52+
[:signature, signature],
53+
[:resource_type, 'image'],
54+
[:type, 'upload'],
55+
[:format, 'jpg']
56+
].each do |attr, value|
57+
expect(preloaded.send(attr)).to eq(value)
58+
end
59+
end
60+
end
61+
62+
describe "signature verification" do
63+
let(:public_id) { 'tests/logo.png' }
64+
let(:test_version) { 1234 }
65+
66+
it "should correctly verify signature with proper parameter order" do
67+
# PreloadedFile extracts public_id by removing the format extension
68+
# So if filename is "tests/logo.png", public_id becomes "tests/logo"
69+
filename_with_format = public_id
70+
public_id_without_format = "tests/logo" # public_id without .png extension
71+
72+
# Generate a valid signature using the public_id without extension
73+
# The version parsed from preloaded string will be a string, so we use string here too
74+
version_string = test_version.to_s
75+
expected_signature = Cloudinary::Utils.api_sign_request(
76+
{ :public_id => public_id_without_format, :version => version_string },
77+
test_api_secret,
78+
nil,
79+
1 # verify_api_response_signature uses version 1
80+
)
81+
82+
# Create a preloaded file string
83+
preloaded_string = "image/upload/v#{version_string}/#{filename_with_format}##{expected_signature}"
84+
preloaded_file = Cloudinary::PreloadedFile.new(preloaded_string)
85+
86+
expect(preloaded_file).to be_valid
87+
end
88+
89+
it "should fail verification with incorrect signature" do
90+
wrong_signature = "wrongsignature"
91+
preloaded_string = "image/upload/v#{test_version}/#{public_id}##{wrong_signature}"
92+
preloaded_file = Cloudinary::PreloadedFile.new(preloaded_string)
93+
94+
expect(preloaded_file).not_to be_valid
95+
end
96+
97+
it "should handle raw resource type correctly" do
98+
raw_filename = "document.pdf"
99+
version_string = test_version.to_s
100+
raw_signature = Cloudinary::Utils.api_sign_request(
101+
{ :public_id => raw_filename, :version => version_string },
102+
test_api_secret,
103+
nil,
104+
1
105+
)
106+
107+
preloaded_string = "raw/upload/v#{version_string}/#{raw_filename}##{raw_signature}"
108+
preloaded_file = Cloudinary::PreloadedFile.new(preloaded_string)
109+
110+
expect(preloaded_file).to be_valid
111+
expect(preloaded_file.resource_type).to eq('raw')
112+
end
113+
end
114+
end

‎spec/uploader_spec.rb‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -764,4 +764,31 @@
764764
expect(pdf_result["url"]).to include("w_111")
765765
expect(pdf_result["url"]).to end_with(".pdf")
766766
end
767+
768+
describe "signature_version parameter support" do
769+
it "should use signature_version from config when not specified" do
770+
original_signature_version = Cloudinary.config.signature_version
771+
Cloudinary.config.signature_version = 1
772+
773+
begin
774+
# Test that configuration signature_version affects signing
775+
upload_result = Cloudinary::Uploader.upload(TEST_IMG, :tags => [TEST_TAG, TIMESTAMP_TAG])
776+
public_id = upload_result["public_id"]
777+
version = upload_result["version"]
778+
779+
# Verify the signature was created with version 1
780+
expected_signature_v1 = Cloudinary::Utils.api_sign_request(
781+
{ :public_id => public_id, :version => version },
782+
Cloudinary.config.api_secret,
783+
nil,
784+
1
785+
)
786+
787+
expect(upload_result["signature"]).to eq(expected_signature_v1)
788+
ensure
789+
# Reset config to original value
790+
Cloudinary.config.signature_version = original_signature_version
791+
end
792+
end
793+
end
767794
end

‎spec/utils_spec.rb‎

Lines changed: 46 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -907,22 +907,7 @@
907907
.and empty_options
908908
end
909909

910-
it "should allow to use folders in PreloadedFile" do
911-
signature = Cloudinary::Utils.api_sign_request({ :public_id => "folder/file", :version => "1234" }, Cloudinary.config.api_secret)
912-
preloaded = Cloudinary::PreloadedFile.new("image/upload/v1234/folder/file.jpg#" + signature)
913-
expect(preloaded).to be_valid
914-
[
915-
[:filename, 'folder/file.jpg'],
916-
[:version, '1234'],
917-
[:public_id, 'folder/file'],
918-
[:signature, signature],
919-
[:resource_type, 'image'],
920-
[:type, 'upload'],
921-
[:format, 'jpg']
922-
].each do |attr, value|
923-
expect(preloaded.send(attr)).to eq(value)
924-
end
925-
end
910+
926911

927912
it "should escape public_ids" do
928913
[
@@ -1484,6 +1469,51 @@
14841469
expect(parameters["signature"]).not_to be_nil
14851470
end
14861471

1472+
describe "Response signature verification fixes" do
1473+
let(:public_id) { 'tests/logo.png' }
1474+
let(:test_version) { 1234 }
1475+
let(:test_api_secret) { SIGNATURE_VERIFICATION_API_SECRET }
1476+
1477+
before do
1478+
Cloudinary.config.update(:api_secret => test_api_secret)
1479+
end
1480+
1481+
describe "api_sign_request signature_version parameter support" do
1482+
it "should support signature_version parameter in api_sign_request" do
1483+
params = { :public_id => public_id, :version => test_version }
1484+
1485+
signature_v1 = Cloudinary::Utils.api_sign_request(params, test_api_secret, nil, 1)
1486+
signature_v2 = Cloudinary::Utils.api_sign_request(params, test_api_secret, nil, 2)
1487+
1488+
expect(signature_v1).to be_a(String)
1489+
expect(signature_v2).to be_a(String)
1490+
expect(signature_v1).to eq(signature_v2) # No & in values, so should be the same
1491+
end
1492+
1493+
it "should use default signature_version from config" do
1494+
Cloudinary.config.signature_version = 2
1495+
params = { :public_id => public_id, :version => test_version }
1496+
1497+
signature_with_nil = Cloudinary::Utils.api_sign_request(params, test_api_secret, nil, nil)
1498+
signature_with_v2 = Cloudinary::Utils.api_sign_request(params, test_api_secret, nil, 2)
1499+
1500+
expect(signature_with_nil).to eq(signature_with_v2)
1501+
end
1502+
1503+
it "should default to version 2 when no config is set" do
1504+
Cloudinary.config.signature_version = nil
1505+
params = { :public_id => public_id, :version => test_version }
1506+
1507+
signature_with_nil = Cloudinary::Utils.api_sign_request(params, test_api_secret, nil, nil)
1508+
signature_with_v2 = Cloudinary::Utils.api_sign_request(params, test_api_secret, nil, 2)
1509+
1510+
expect(signature_with_nil).to eq(signature_with_v2)
1511+
end
1512+
end
1513+
1514+
1515+
end
1516+
14871517
it "should download multi" do
14881518
multi_test_tag = "multi_test_tag_#{UNIQUE_TEST_ID}"
14891519
url1 = "https://res.cloudinary.com/demo/image/upload/sample"

0 commit comments

Comments
 (0)