diff --git a/activestorage/app/controllers/concerns/active_storage/set_blob.rb b/activestorage/app/controllers/concerns/active_storage/set_blob.rb index f072954d7870c7ed6d43e5e6977e8ef763a6584a..e20d22be2be0e1dba47082850a70d707ee76300c 100644 --- a/activestorage/app/controllers/concerns/active_storage/set_blob.rb +++ b/activestorage/app/controllers/concerns/active_storage/set_blob.rb @@ -9,7 +9,7 @@ module ActiveStorage::SetBlob #:nodoc: private def set_blob - @blob = ActiveStorage::Blob.find_signed(params[:signed_blob_id] || params[:signed_id]) + @blob = ActiveStorage::Blob.find_signed!(params[:signed_blob_id] || params[:signed_id]) rescue ActiveSupport::MessageVerifier::InvalidSignature head :not_found end diff --git a/activestorage/app/models/active_storage/blob.rb b/activestorage/app/models/active_storage/blob.rb index b22e576b3ccecc73da6ca97353f491b0848098f5..163f633502c1d64a87803b301e3d644389770dff 100644 --- a/activestorage/app/models/active_storage/blob.rb +++ b/activestorage/app/models/active_storage/blob.rb @@ -35,6 +35,7 @@ class ActiveStorage::Blob < ActiveRecord::Base include ActiveStorage::Blob::Representable self.table_name = "active_storage_blobs" + self.signed_id_verifier = ActiveStorage.verifier MINIMUM_TOKEN_LENGTH = 28 @@ -72,8 +73,8 @@ class << self # that was created ahead of the upload itself on form submission. # # The signed ID is also used to create stable URLs for the blob through the BlobsController. - def find_signed(id, record: nil) - find ActiveStorage.verifier.verify(id, purpose: :blob_id) + def find_signed!(id, record: nil) + super(id, purpose: :blob_id) end def build_after_upload(io:, filename:, content_type: nil, metadata: nil, service_name: nil, identify: true, record: nil) #:nodoc: @@ -125,12 +126,16 @@ def create_before_direct_upload!(key: nil, filename:, byte_size:, checksum:, con def generate_unique_secure_token(length: MINIMUM_TOKEN_LENGTH) SecureRandom.base36(length) end + + # Customize signed ID purposes for backwards compatibility. + def combine_signed_id_purposes(purpose) + purpose.to_s + end end # Returns a signed ID for this blob that's suitable for reference on the client-side without fear of tampering. - # It uses the framework-wide verifier on ActiveStorage.verifier, but with a dedicated purpose. def signed_id - ActiveStorage.verifier.generate(id, purpose: :blob_id) + super(purpose: :blob_id) end # Returns the key pointing to the file on the service that's associated with this blob. The key is the diff --git a/activestorage/lib/active_storage/attached/changes/create_one.rb b/activestorage/lib/active_storage/attached/changes/create_one.rb index 498a6b5337349df66fb4746e4f5a2d913994bc15..b61d3b078c8c1d29dbfec693c864982ec82ae4d5 100644 --- a/activestorage/lib/active_storage/attached/changes/create_one.rb +++ b/activestorage/lib/active_storage/attached/changes/create_one.rb @@ -68,7 +68,7 @@ def find_or_build_blob ) ) when String - ActiveStorage::Blob.find_signed(attachable, record: record) + ActiveStorage::Blob.find_signed!(attachable, record: record) else raise ArgumentError, "Could not find or build blob: expected attachable, got #{attachable.inspect}" end diff --git a/activestorage/test/controllers/direct_uploads_controller_test.rb b/activestorage/test/controllers/direct_uploads_controller_test.rb index 17356c059048a9f876574e4cdd3043ed3260270c..734798ff3caa85af10b37cd07d7a919859376a01 100644 --- a/activestorage/test/controllers/direct_uploads_controller_test.rb +++ b/activestorage/test/controllers/direct_uploads_controller_test.rb @@ -21,7 +21,7 @@ class ActiveStorage::S3DirectUploadsControllerTest < ActionDispatch::Integration filename: "hello.txt", byte_size: 6, checksum: checksum, content_type: "text/plain" } } response.parsed_body.tap do |details| - assert_equal ActiveStorage::Blob.find(details["id"]), ActiveStorage::Blob.find_signed(details["signed_id"]) + assert_equal ActiveStorage::Blob.find(details["id"]), ActiveStorage::Blob.find_signed!(details["signed_id"]) assert_equal "hello.txt", details["filename"] assert_equal 6, details["byte_size"] assert_equal checksum, details["checksum"] @@ -56,7 +56,7 @@ class ActiveStorage::GCSDirectUploadsControllerTest < ActionDispatch::Integratio filename: "hello.txt", byte_size: 6, checksum: checksum, content_type: "text/plain" } } @response.parsed_body.tap do |details| - assert_equal ActiveStorage::Blob.find(details["id"]), ActiveStorage::Blob.find_signed(details["signed_id"]) + assert_equal ActiveStorage::Blob.find(details["id"]), ActiveStorage::Blob.find_signed!(details["signed_id"]) assert_equal "hello.txt", details["filename"] assert_equal 6, details["byte_size"] assert_equal checksum, details["checksum"] @@ -90,7 +90,7 @@ class ActiveStorage::AzureStorageDirectUploadsControllerTest < ActionDispatch::I filename: "hello.txt", byte_size: 6, checksum: checksum, content_type: "text/plain" } } @response.parsed_body.tap do |details| - assert_equal ActiveStorage::Blob.find(details["id"]), ActiveStorage::Blob.find_signed(details["signed_id"]) + assert_equal ActiveStorage::Blob.find(details["id"]), ActiveStorage::Blob.find_signed!(details["signed_id"]) assert_equal "hello.txt", details["filename"] assert_equal 6, details["byte_size"] assert_equal checksum, details["checksum"] @@ -112,7 +112,7 @@ class ActiveStorage::DiskDirectUploadsControllerTest < ActionDispatch::Integrati filename: "hello.txt", byte_size: 6, checksum: checksum, content_type: "text/plain" } } @response.parsed_body.tap do |details| - assert_equal ActiveStorage::Blob.find(details["id"]), ActiveStorage::Blob.find_signed(details["signed_id"]) + assert_equal ActiveStorage::Blob.find(details["id"]), ActiveStorage::Blob.find_signed!(details["signed_id"]) assert_equal "hello.txt", details["filename"] assert_equal 6, details["byte_size"] assert_equal checksum, details["checksum"] diff --git a/activestorage/test/models/attached/one_test.rb b/activestorage/test/models/attached/one_test.rb index f3c5ead528a148477cc0159b87a37e15c0e5fb4c..304f32bf1525263cde568066369274855c522fcd 100644 --- a/activestorage/test/models/attached/one_test.rb +++ b/activestorage/test/models/attached/one_test.rb @@ -32,7 +32,7 @@ class ActiveStorage::OneAttachedTest < ActiveSupport::TestCase test "attaching an existing blob from a signed ID passes record" do blob = create_blob(filename: "funky.jpg") arguments = [blob.signed_id, record: @user] - assert_called_with(ActiveStorage::Blob, :find_signed, arguments, returns: blob) do + assert_called_with(ActiveStorage::Blob, :find_signed!, arguments, returns: blob) do @user.avatar.attach blob.signed_id end end diff --git a/activestorage/test/models/attachment_test.rb b/activestorage/test/models/attachment_test.rb index 88e83c32ab39c484037ab04b1f7280aeb4c85be7..ebcb837e86c6bbc8eec7a20aab9267f4287e5402 100644 --- a/activestorage/test/models/attachment_test.rb +++ b/activestorage/test/models/attachment_test.rb @@ -55,6 +55,14 @@ class ActiveStorage::AttachmentTest < ActiveSupport::TestCase @user.avatar.attach(blob) signed_id = @user.avatar.signed_id - assert_equal blob, ActiveStorage::Blob.find_signed(signed_id) + assert_equal blob, ActiveStorage::Blob.find_signed!(signed_id) + end + + test "signed blob ID backwards compatibility" do + blob = create_blob + @user.avatar.attach(blob) + + signed_id_generated_old_way = ActiveStorage.verifier.generate(@user.avatar.id, purpose: :blob_id) + assert_equal blob, ActiveStorage::Blob.find_signed!(signed_id_generated_old_way) end end