From 41e6767410073a7b022a499bcc67b195660df207 Mon Sep 17 00:00:00 2001 From: David Wessman Date: Fri, 4 Oct 2024 07:13:27 +0200 Subject: [PATCH 1/3] download_endpoint: Adds support for expiring URLs - Adds `expires_in` and `verifier_secret` to `download_endpoint` plugin - This allows calling `attachment.download_url(expires_in: 5 * 60)` to generate a URL that expires in 5 minutes. - The URL is signed and verified with ActiveSupport::MessageVerifier. --- CHANGELOG.md | 4 +++ doc/plugins/download_endpoint.md | 20 +++++++++++--- lib/shrine/plugins/download_endpoint.rb | 33 ++++++++++++++++++++--- test/plugin/download_endpoint_test.rb | 35 ++++++++++++++++++++++++- 4 files changed, 83 insertions(+), 9 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3f8988261..4fbdc2e16 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,7 @@ +## Unreleased + +* `download_endpoint` - Add support for expiring URLs + ## 3.6.0 (2024-04-29) * Add Rack 3 support (@tomasc, @janko) diff --git a/doc/plugins/download_endpoint.md b/doc/plugins/download_endpoint.md index 0cb5a8f7b..b506451e9 100644 --- a/doc/plugins/download_endpoint.md +++ b/doc/plugins/download_endpoint.md @@ -7,7 +7,7 @@ downloading uploaded files from specified storages. This can be useful when files from your storage isn't accessible over URL (e.g. database storages) or if you want to authenticate your downloads. -## Global Endpoint +## Global Endpoint You can configure the plugin with the path prefix which the endpoint will be mounted on. @@ -34,6 +34,7 @@ Links to the download endpoint are generated by calling ```rb uploaded_file.download_url #=> "/attachments/eyJpZCI6ImFkdzlyeTM..." ``` + ## Endpoint via Uploader You can also configure the plugin in the uploader directly - just make sure to mount it via your Uploader-class. @@ -52,8 +53,8 @@ Rails.application.routes.draw do end ``` -*Hint: For shrine versions 2.x -> ensure that you don't include the plugin -twice (globally and in your uploader class - see #408)* +_Hint: For shrine versions 2.x -> ensure that you don't include the plugin +twice (globally and in your uploader class - see #408)_ ## Calling from a controller @@ -69,6 +70,7 @@ Rails.application.routes.draw do get "/attachments/*rest", to: "downloads#image" end ``` + ```rb # app/controllers/downloads_controller.rb (Rails) class DownloadsController < ApplicationController @@ -131,6 +133,16 @@ plugin :download_endpoint, download_options: -> (uploaded_file, request) { } ``` +## Expiring download urls + +If you want to have URLs that expire after a certain time, you can use the `:expires_in` and `verifier_secret` options: + +```rb +plugin :download_endpoint, expires_in: 5 * 60, verifier_secret: "secret" +``` + +this will generate URLs that are signed with a signature valid for 5 minutes. + ## Performance considerations Streaming files through the app might impact the request throughput, depending @@ -162,7 +174,7 @@ Shrine.download_endpoint(disposition: "attachment") ## Plugin options | Name | Description | Default | -| :-------- | :---------- | :------ | +| :------------------ | :-------------------------------------------------------------------------------- | :------- | | `:disposition` | Whether browser should render the file `inline` or download it as an `attachment` | `inline` | | `:download_options` | Hash of storage-specific options passed to `Storage#open` | `{}` | | `:host` | URL host that will be added to download URLs | `nil` | diff --git a/lib/shrine/plugins/download_endpoint.rb b/lib/shrine/plugins/download_endpoint.rb index b46f09322..059848a72 100644 --- a/lib/shrine/plugins/download_endpoint.rb +++ b/lib/shrine/plugins/download_endpoint.rb @@ -65,14 +65,20 @@ def initialize(file) @file = file end - def call(host: self.host) - [host, *prefix, path].join("/") + def call(host: self.host, expires_in: nil) + [host, *prefix, path(expires_in: expires_in)].join("/") end protected - def path - file.urlsafe_dump(metadata: %w[filename size mime_type]) + def path(expires_in: nil) + if expires_in || default_expires_in + reference = file.urlsafe_dump(metadata: %w[filename size mime_type]) + expires_at = Time.now + (expires_in || default_expires_in) + verifier.generate(reference, expires_at: expires_at) + else + file.urlsafe_dump(metadata: %w[filename size mime_type]) + end end def host @@ -83,6 +89,20 @@ def prefix options[:prefix] end + def default_expires_in + options[:expires_in] + end + + def verifier_secret + options[:verifier_secret] + end + + def verifier + raise Error, "verifier_secret is required for expiring URLs" unless verifier_secret + + ::ActiveSupport::MessageVerifier.new(verifier_secret) + end + def options file.shrine_class.opts[:download_endpoint] end @@ -185,6 +205,11 @@ def get_uploaded_file(serialized) rescue Shrine::Error # storage not found not_found! rescue JSON::ParserError, ArgumentError => error # invalid serialized component + if @verifier_secret + reference = ::ActiveSupport::MessageVerifier.new(@verifier_secret).verified(serialized) + return reference ? get_uploaded_file(reference) : not_found! + end + raise if error.is_a?(ArgumentError) && error.message != "invalid base64" bad_request!("Invalid serialized file") end diff --git a/test/plugin/download_endpoint_test.rb b/test/plugin/download_endpoint_test.rb index 43f9c52e9..83668a1d9 100644 --- a/test/plugin/download_endpoint_test.rb +++ b/test/plugin/download_endpoint_test.rb @@ -1,5 +1,6 @@ require "test_helper" require "shrine/plugins/download_endpoint" +require "active_support/message_verifier" require "rack/test" require "uri" @@ -25,10 +26,42 @@ def endpoint assert_equal 200, response.status assert_equal @uploaded_file.read, response.body assert_equal @uploaded_file.size.to_s, response.headers["Content-Length"] - assert_equal @uploaded_file.mime_type, response.headers["COntent-Type"] + assert_equal @uploaded_file.mime_type, response.headers["Content-Type"] assert_equal ContentDisposition.inline(@uploaded_file.original_filename), response.headers["Content-Disposition"] end + it "raise error if using expires_in without any verifier_secret" do + io = fakeio("a" * 16*1024 + "b" * 16*1024 + "c" * 4*1024, content_type: "text/plain", filename: "content.txt") + @uploaded_file = @uploader.upload(io) + assert_raises Shrine::Error do + @uploaded_file.download_url(expires_in: 1000) + end + end + + it "returns a file response with expiring url" do + @uploader = uploader { plugin :download_endpoint, verifier_secret: SecureRandom.hex(64) } + @shrine = @uploader.class + io = fakeio("a" * 16*1024 + "b" * 16*1024 + "c" * 4*1024, content_type: "text/plain", filename: "content.txt") + @uploaded_file = @uploader.upload(io) + response = app.get(@uploaded_file.download_url(expires_in: 1000)) + + assert_equal 200, response.status + assert_equal @uploaded_file.read, response.body + assert_equal @uploaded_file.size.to_s, response.headers["Content-Length"] + assert_equal @uploaded_file.mime_type, response.headers["Content-Type"] + assert_equal ContentDisposition.inline(@uploaded_file.original_filename), response.headers["Content-Disposition"] + end + + it "does not return a file if expired" do + @uploader = uploader { plugin :download_endpoint, verifier_secret: SecureRandom.hex(64) } + @shrine = @uploader.class + io = fakeio("a" * 16*1024 + "b" * 16*1024 + "c" * 4*1024, content_type: "text/plain", filename: "content.txt") + @uploaded_file = @uploader.upload(io) + response = app.get(@uploaded_file.download_url(expires_in: -1)) + + assert_equal 404, response.status + end + it "applies :download_options hash" do @shrine.plugin :download_endpoint, download_options: { foo: "bar" } @uploaded_file.storage.expects(:open).with(@uploaded_file.id, foo: "bar").returns(StringIO.new("options")) From 89310a38d0ad3076d26b58332fdfa2a7fcc8bccf Mon Sep 17 00:00:00 2001 From: David Wessman Date: Sat, 5 Oct 2024 19:17:56 +0200 Subject: [PATCH 2/3] Version without ActiveSupport --- doc/plugins/download_endpoint.md | 4 +- lib/shrine/plugins/download_endpoint.rb | 71 +++++++++++++++++-------- test/plugin/download_endpoint_test.rb | 37 +++++++++++-- 3 files changed, 83 insertions(+), 29 deletions(-) diff --git a/doc/plugins/download_endpoint.md b/doc/plugins/download_endpoint.md index b506451e9..e820d77cc 100644 --- a/doc/plugins/download_endpoint.md +++ b/doc/plugins/download_endpoint.md @@ -135,10 +135,10 @@ plugin :download_endpoint, download_options: -> (uploaded_file, request) { ## Expiring download urls -If you want to have URLs that expire after a certain time, you can use the `:expires_in` and `verifier_secret` options: +If you want to have URLs that expire after a certain time, you can use the `:expires_in` and `secret_key` options: ```rb -plugin :download_endpoint, expires_in: 5 * 60, verifier_secret: "secret" +plugin :download_endpoint, expires_in: 5 * 60, secret_key: "secret" ``` this will generate URLs that are signed with a signature valid for 5 minutes. diff --git a/lib/shrine/plugins/download_endpoint.rb b/lib/shrine/plugins/download_endpoint.rb index 059848a72..99a0aad22 100644 --- a/lib/shrine/plugins/download_endpoint.rb +++ b/lib/shrine/plugins/download_endpoint.rb @@ -1,5 +1,8 @@ # frozen_string_literal: true +require 'openssl' +require 'base64' + class Shrine module Plugins # Documentation can be found on https://shrinerb.com/docs/plugins/download_endpoint @@ -66,19 +69,35 @@ def initialize(file) end def call(host: self.host, expires_in: nil) - [host, *prefix, path(expires_in: expires_in)].join("/") + path = file.urlsafe_dump(metadata: %w[filename size mime_type]) + + query = signature_as_query(path: path, expires_in: expires_in) + + path = [host, *prefix, path].join("/") + path += "?#{query}" if query + path end protected - def path(expires_in: nil) - if expires_in || default_expires_in - reference = file.urlsafe_dump(metadata: %w[filename size mime_type]) - expires_at = Time.now + (expires_in || default_expires_in) - verifier.generate(reference, expires_at: expires_at) - else - file.urlsafe_dump(metadata: %w[filename size mime_type]) - end + def signature_as_query(path:, expires_in:) + expires_in = default_expires_in if expires_in.nil? + raise(Error, "secret_key is required for expiring URLs") if !secret_key && expires_in + raise(Error, "expires_in is required for expiring URLs") if secret_key && !expires_in + + return nil unless expires_in + + expires_at = (Time.now + expires_in).to_i + signature = OpenSSL::HMAC.digest( + OpenSSL::Digest::SHA256.new, + secret_key, + "#{path}--#{expires_at}" + ) + + Rack::Utils.build_query( + signature: Base64.urlsafe_encode64(signature), + expires_at: expires_at + ) end def host @@ -93,14 +112,8 @@ def default_expires_in options[:expires_in] end - def verifier_secret - options[:verifier_secret] - end - - def verifier - raise Error, "verifier_secret is required for expiring URLs" unless verifier_secret - - ::ActiveSupport::MessageVerifier.new(verifier_secret) + def secret_key + options[:secret_key] end def options @@ -149,6 +162,9 @@ def inspect def handle_request(request) _, serialized, * = request.path_info.split("/") + signature, expires_at = request.params.values_at("signature", "expires_at") + + check_signature!(serialized, signature, expires_at) if @secret_key uploaded_file = get_uploaded_file(serialized) @@ -205,15 +221,26 @@ def get_uploaded_file(serialized) rescue Shrine::Error # storage not found not_found! rescue JSON::ParserError, ArgumentError => error # invalid serialized component - if @verifier_secret - reference = ::ActiveSupport::MessageVerifier.new(@verifier_secret).verified(serialized) - return reference ? get_uploaded_file(reference) : not_found! - end - raise if error.is_a?(ArgumentError) && error.message != "invalid base64" bad_request!("Invalid serialized file") end + def check_signature!(serialized, signature, expires_at) + if expires_at && expires_at.to_i < Time.now.to_i + error!(400, "URL has expired") + end + + calculated_signature = OpenSSL::HMAC.digest( + OpenSSL::Digest::SHA256.new, + @secret_key, + "#{serialized}--#{expires_at}" + ) + + if !Rack::Utils.secure_compare(signature, Base64.urlsafe_encode64(calculated_signature)) + error!(403, "Signature does not match") + end + end + def not_found! error!(404, "File Not Found") end diff --git a/test/plugin/download_endpoint_test.rb b/test/plugin/download_endpoint_test.rb index 83668a1d9..d2dd192fa 100644 --- a/test/plugin/download_endpoint_test.rb +++ b/test/plugin/download_endpoint_test.rb @@ -1,6 +1,5 @@ require "test_helper" require "shrine/plugins/download_endpoint" -require "active_support/message_verifier" require "rack/test" require "uri" @@ -30,7 +29,7 @@ def endpoint assert_equal ContentDisposition.inline(@uploaded_file.original_filename), response.headers["Content-Disposition"] end - it "raise error if using expires_in without any verifier_secret" do + it "raise error if using expires_in without any secret_key" do io = fakeio("a" * 16*1024 + "b" * 16*1024 + "c" * 4*1024, content_type: "text/plain", filename: "content.txt") @uploaded_file = @uploader.upload(io) assert_raises Shrine::Error do @@ -39,7 +38,7 @@ def endpoint end it "returns a file response with expiring url" do - @uploader = uploader { plugin :download_endpoint, verifier_secret: SecureRandom.hex(64) } + @uploader = uploader { plugin :download_endpoint, secret_key: SecureRandom.hex(64) } @shrine = @uploader.class io = fakeio("a" * 16*1024 + "b" * 16*1024 + "c" * 4*1024, content_type: "text/plain", filename: "content.txt") @uploaded_file = @uploader.upload(io) @@ -53,13 +52,13 @@ def endpoint end it "does not return a file if expired" do - @uploader = uploader { plugin :download_endpoint, verifier_secret: SecureRandom.hex(64) } + @uploader = uploader { plugin :download_endpoint, secret_key: SecureRandom.hex(64) } @shrine = @uploader.class io = fakeio("a" * 16*1024 + "b" * 16*1024 + "c" * 4*1024, content_type: "text/plain", filename: "content.txt") @uploaded_file = @uploader.upload(io) response = app.get(@uploaded_file.download_url(expires_in: -1)) - assert_equal 404, response.status + assert_equal 400, response.status end it "applies :download_options hash" do @@ -171,6 +170,34 @@ def endpoint url2 = @uploaded_file.url assert_equal url1, url2 end + it "returns same download_url regardless of metadata order" do + @uploaded_file.data["metadata"] = { "filename" => "a", "mime_type" => "b", "size" => "c" } + url1 = @uploaded_file.download_url + @uploaded_file.data["metadata"] = { "mime_type" => "b", "size" => "c", "filename" => "a" } + url2 = @uploaded_file.download_url + assert_equal url1, url2 + end + + it "returns signature and expires_at when configured" do + secret = SecureRandom.hex(64) + @uploader = uploader { plugin :download_endpoint, secret_key: secret } + @shrine = @uploader.class + @uploaded_file = @uploader.upload(fakeio) + + url = @uploaded_file.download_url(expires_in: 1000) + + uri = URI.parse(url) + path = uri.path.split("/").last + query = URI.decode_www_form(uri.query).to_h + signature, expires_at = query.values_at("signature", "expires_at") + + calculated_signature = OpenSSL::HMAC.digest( + OpenSSL::Digest::SHA256.new, + secret, + "#{path}--#{expires_at}" + ) + assert_equal Base64.urlsafe_decode64(signature), calculated_signature + end it "returns 400 on invalid serialized file" do response = app.get("/dontwork") From 753d170dd0d36e4a139d938e2908ce4f8ced0ae3 Mon Sep 17 00:00:00 2001 From: David Wessman Date: Tue, 15 Oct 2024 13:25:56 +0200 Subject: [PATCH 3/3] Fixes review feedback --- test/plugin/download_endpoint_test.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/plugin/download_endpoint_test.rb b/test/plugin/download_endpoint_test.rb index d2dd192fa..b78156bc1 100644 --- a/test/plugin/download_endpoint_test.rb +++ b/test/plugin/download_endpoint_test.rb @@ -32,7 +32,7 @@ def endpoint it "raise error if using expires_in without any secret_key" do io = fakeio("a" * 16*1024 + "b" * 16*1024 + "c" * 4*1024, content_type: "text/plain", filename: "content.txt") @uploaded_file = @uploader.upload(io) - assert_raises Shrine::Error do + assert_raises Shrine::Error, "secret_key is required for expiring URLs" do @uploaded_file.download_url(expires_in: 1000) end end @@ -179,7 +179,7 @@ def endpoint end it "returns signature and expires_at when configured" do - secret = SecureRandom.hex(64) + secret = "A" * 64 @uploader = uploader { plugin :download_endpoint, secret_key: secret } @shrine = @uploader.class @uploaded_file = @uploader.upload(fakeio)