diff --git a/actiontext/app/assets/javascripts/actiontext.js b/actiontext/app/assets/javascripts/actiontext.js index 792a2c1fc310..f65f24402895 100644 --- a/actiontext/app/assets/javascripts/actiontext.js +++ b/actiontext/app/assets/javascripts/actiontext.js @@ -506,14 +506,16 @@ var activestorage = {exports: {}}; } } class BlobRecord { - constructor(file, checksum, url) { + constructor(file, checksum, url, directUploadToken, attachmentName) { this.file = file; this.attributes = { filename: file.name, content_type: file.type || "application/octet-stream", byte_size: file.size, - checksum: checksum + checksum: checksum, }; + this.directUploadToken = directUploadToken; + this.attachmentName = attachmentName; this.xhr = new XMLHttpRequest; this.xhr.open("POST", url, true); this.xhr.responseType = "json"; @@ -541,7 +543,9 @@ var activestorage = {exports: {}}; create(callback) { this.callback = callback; this.xhr.send(JSON.stringify({ - blob: this.attributes + blob: this.attributes, + direct_upload_token: this.directUploadToken, + attachment_name: this.attachmentName })); } requestDidLoad(event) { @@ -599,10 +603,12 @@ var activestorage = {exports: {}}; } let id = 0; class DirectUpload { - constructor(file, url, delegate) { + constructor(file, url, directUploadToken, attachmentName, delegate) { this.id = ++id; this.file = file; this.url = url; + this.directUploadToken = directUploadToken; + this.attachmentName = attachmentName; this.delegate = delegate; } create(callback) { @@ -611,7 +617,7 @@ var activestorage = {exports: {}}; callback(error); return; } - const blob = new BlobRecord(this.file, checksum, this.url); + const blob = new BlobRecord(this.file, checksum, this.url, this.directUploadToken, this.attachmentName); notify(this.delegate, "directUploadWillCreateBlobWithXHR", blob.xhr); blob.create((error => { if (error) { @@ -640,7 +646,7 @@ var activestorage = {exports: {}}; constructor(input, file) { this.input = input; this.file = file; - this.directUpload = new DirectUpload(this.file, this.url, this); + this.directUpload = new DirectUpload(this.file, this.url, this.directUploadToken, this.attachmentName, this); this.dispatch("initialize"); } start(callback) { @@ -671,6 +677,12 @@ var activestorage = {exports: {}}; get url() { return this.input.getAttribute("data-direct-upload-url"); } + get directUploadToken() { + return this.input.getAttribute("data-direct-upload-token"); + } + get attachmentName() { + return this.input.getAttribute("data-direct-upload-attachment-name"); + } dispatch(name, detail = {}) { detail.file = this.file; detail.id = this.directUpload.id; @@ -830,7 +842,7 @@ class AttachmentUpload { constructor(attachment, element) { this.attachment = attachment; this.element = element; - this.directUpload = new activestorage.exports.DirectUpload(attachment.file, this.directUploadUrl, this); + this.directUpload = new activestorage.exports.DirectUpload(attachment.file, this.directUploadUrl, this.directUploadToken, this.directUploadAttachmentName, this); } start() { @@ -865,6 +877,14 @@ class AttachmentUpload { return this.element.dataset.directUploadUrl } + get directUploadToken() { + return this.element.dataset.directUploadToken + } + + get directUploadAttachmentName() { + return this.element.dataset.directUploadAttachmentName + } + get blobUrlTemplate() { return this.element.dataset.blobUrlTemplate } diff --git a/actiontext/app/helpers/action_text/tag_helper.rb b/actiontext/app/helpers/action_text/tag_helper.rb index 2f216737a5c0..8307038f5d43 100644 --- a/actiontext/app/helpers/action_text/tag_helper.rb +++ b/actiontext/app/helpers/action_text/tag_helper.rb @@ -32,6 +32,14 @@ def rich_text_area_tag(name, value = nil, options = {}) options[:data][:direct_upload_url] ||= main_app.rails_direct_uploads_url options[:data][:blob_url_template] ||= main_app.rails_service_blob_url(":signed_id", ":filename") + class_with_attachment = "ActionText::RichText#embeds" + options[:data][:direct_upload_attachment_name] ||= class_with_attachment + options[:data][:direct_upload_token] = ActiveStorage::DirectUploadToken.generate_direct_upload_token( + class_with_attachment, + ActiveStorage::Blob.service.name, + session + ) + editor_tag = content_tag("trix-editor", "", options) input_tag = hidden_field_tag(name, value.try(:to_trix_html) || value, id: options[:input], form: form) diff --git a/actiontext/test/template/form_helper_test.rb b/actiontext/test/template/form_helper_test.rb index c87d53fe3c95..004916ce949b 100644 --- a/actiontext/test/template/form_helper_test.rb +++ b/actiontext/test/template/form_helper_test.rb @@ -1,6 +1,7 @@ # frozen_string_literal: true require "test_helper" +require "minitest/mock" class ActionText::FormHelperTest < ActionView::TestCase tests ActionText::TagHelper @@ -28,158 +29,184 @@ def form_with(*, **) test "rich text area tag" do message = Message.new - form_with model: message, scope: :message do |form| - rich_text_area_tag :content, message.content, { input: "trix_input_1" } - end + with_stub_token do + form_with model: message, scope: :message do |form| + rich_text_area_tag :content, message.content, { input: "trix_input_1" } + end - assert_dom_equal \ - '
' \ - '' \ - '' \ - "" \ - "
", - output_buffer + assert_dom_equal \ + '
' \ + '' \ + '' \ + "" \ + "
", + output_buffer + end end test "form with rich text area" do - form_with model: Message.new, scope: :message do |form| - form.rich_text_area :content - end + with_stub_token do + form_with model: Message.new, scope: :message do |form| + form.rich_text_area :content + end - assert_dom_equal \ - '
' \ - '' \ - '' \ - "" \ - "
", - output_buffer + assert_dom_equal \ + '
' \ + '' \ + '' \ + "" \ + "
", + output_buffer + end end test "form with rich text area having class" do - form_with model: Message.new, scope: :message do |form| - form.rich_text_area :content, class: "custom-class" - end + with_stub_token do + form_with model: Message.new, scope: :message do |form| + form.rich_text_area :content, class: "custom-class" + end - assert_dom_equal \ - '
' \ - '' \ - '' \ - "" \ - "
", - output_buffer + assert_dom_equal \ + '
' \ + '' \ + '' \ + "" \ + "
", + output_buffer + end end test "form with rich text area for non-attribute" do - form_with model: Message.new, scope: :message do |form| - form.rich_text_area :not_an_attribute - end + with_stub_token do + form_with model: Message.new, scope: :message do |form| + form.rich_text_area :not_an_attribute + end - assert_dom_equal \ - '
' \ - '' \ - '' \ - "" \ - "
", - output_buffer + assert_dom_equal \ + '
' \ + '' \ + '' \ + "" \ + "
", + output_buffer + end end test "modelless form with rich text area" do - form_with url: "/messages", scope: :message do |form| - form.rich_text_area :content, { input: "trix_input_2" } - end + with_stub_token do + form_with url: "/messages", scope: :message do |form| + form.rich_text_area :content, { input: "trix_input_2" } + end - assert_dom_equal \ - '
' \ - '' \ - '' \ - "" \ - "
", - output_buffer + assert_dom_equal \ + '
' \ + '' \ + '' \ + "" \ + "
", + output_buffer + end end test "form with rich text area having placeholder without locale" do - form_with model: Message.new, scope: :message do |form| - form.rich_text_area :content, placeholder: true - end + with_stub_token do + form_with model: Message.new, scope: :message do |form| + form.rich_text_area :content, placeholder: true + end - assert_dom_equal \ - '
' \ - '' \ - '' \ - "" \ - "
", - output_buffer + assert_dom_equal \ + '
' \ + '' \ + '' \ + "" \ + "
", + output_buffer + end end test "form with rich text area having placeholder with locale" do I18n.with_locale :placeholder do form_with model: Message.new, scope: :message do |form| - form.rich_text_area :title, placeholder: true + with_stub_token do + form.rich_text_area :title, placeholder: true + end end - end - assert_dom_equal \ - '
' \ - '' \ - '' \ - "" \ - "
", - output_buffer + assert_dom_equal \ + '
' \ + '' \ + '' \ + "" \ + "
", + output_buffer + end end test "form with rich text area with value" do - form_with model: Message.new, scope: :message do |form| - form.rich_text_area :title, value: "

hello world

" - end + with_stub_token do + form_with model: Message.new, scope: :message do |form| + form.rich_text_area :title, value: "

hello world

" + end - assert_dom_equal \ - '
' \ - '' \ - '' \ - "" \ - "
", - output_buffer + assert_dom_equal \ + '
' \ + '' \ + '' \ + "" \ + "
", + output_buffer + end end test "form with rich text area with form attribute" do - form_with model: Message.new, scope: :message do |form| - form.rich_text_area :title, form: "other_form" - end + with_stub_token do + form_with model: Message.new, scope: :message do |form| + form.rich_text_area :title, form: "other_form" + end - assert_dom_equal \ - '
' \ - '' \ - '' \ - "" \ - "
", - output_buffer + assert_dom_equal \ + '
' \ + '' \ + '' \ + "" \ + "
", + output_buffer + end end test "form with rich text area with data[direct_upload_url]" do - form_with model: Message.new, scope: :message do |form| - form.rich_text_area :content, data: { direct_upload_url: "http://test.host/direct_uploads" } - end + with_stub_token do + form_with model: Message.new, scope: :message do |form| + form.rich_text_area :content, data: { direct_upload_url: "http://test.host/direct_uploads" } + end - assert_dom_equal \ - '
' \ - '' \ - '' \ - "" \ - "
", - output_buffer + assert_dom_equal \ + '
' \ + '' \ + '' \ + "" \ + "
", + output_buffer + end end test "form with rich text area with data[blob_url_template]" do - form_with model: Message.new, scope: :message do |form| - form.rich_text_area :content, data: { blob_url_template: "http://test.host/blobs/:signed_id/:filename" } + with_stub_token do + form_with model: Message.new, scope: :message do |form| + form.rich_text_area :content, data: { blob_url_template: "http://test.host/blobs/:signed_id/:filename" } + end + + assert_dom_equal \ + '
' \ + '' \ + '' \ + "" \ + "
", + output_buffer end + end - assert_dom_equal \ - '
' \ - '' \ - '' \ - "" \ - "
", - output_buffer + def with_stub_token(&block) + ActiveStorage::DirectUploadToken.stub(:generate_direct_upload_token, "token", &block) end end diff --git a/actionview/CHANGELOG.md b/actionview/CHANGELOG.md index 21af906bcc2b..baaec274b0c4 100644 --- a/actionview/CHANGELOG.md +++ b/actionview/CHANGELOG.md @@ -1,3 +1,27 @@ +* Add support for `button_to ..., authenticity_token: false` + + ```ruby + button_to "Create", Post.new, authenticity_token: false + # =>
+ + button_to "Create", Post.new, authenticity_token: true + # =>
+ + button_to "Create", Post.new, authenticity_token: "secret" + # =>
+ ``` + + *Sean Doyle* + +* Support rendering `
` elements _without_ `[action]` attributes by: + + * `form_with url: false` or `form_with ..., html: { action: false }` + * `form_for ..., url: false` or `form_for ..., html: { action: false }` + * `form_tag false` or `form_tag ..., action: false` + * `button_to "...", false` or `button_to(false) { ... }` + + *Sean Doyle* + * Add `:day_format` option to `date_select` date_select("article", "written_on", day_format: ->(day) { day.ordinalize }) diff --git a/actionview/lib/action_view/helpers/form_helper.rb b/actionview/lib/action_view/helpers/form_helper.rb index ab7e88f92c56..f33a48343dac 100644 --- a/actionview/lib/action_view/helpers/form_helper.rb +++ b/actionview/lib/action_view/helpers/form_helper.rb @@ -282,6 +282,12 @@ module FormHelper # ... # <% end %> # + # You can omit the action attribute by passing url: false: + # + # <%= form_for(@post, url: false) do |f| %> + # ... + # <% end %> + # # You can also set the answer format, like this: # # <%= form_for(@post, format: :json) do |f| %> @@ -449,7 +455,7 @@ def form_for(record, options = {}, &block) output = capture(builder, &block) html_options[:multipart] ||= builder.multipart? - html_options = html_options_for_form(options[:url] || {}, html_options) + html_options = html_options_for_form(options.fetch(:url, {}), html_options) form_tag_with_body(html_options, output) end @@ -465,10 +471,12 @@ def apply_form_for_options!(record, object, options) # :nodoc: method: method ) - options[:url] ||= if options.key?(:format) - polymorphic_path(record, format: options.delete(:format)) - else - polymorphic_path(record, {}) + if options[:url] != false + options[:url] ||= if options.key?(:format) + polymorphic_path(record, format: options.delete(:format)) + else + polymorphic_path(record, {}) + end end end private :apply_form_for_options! @@ -488,6 +496,15 @@ def apply_form_for_options!(record, object, options) # :nodoc: # #
# + # # With an intentionally empty URL: + # <%= form_with url: false do |form| %> + # <%= form.text_field :title %> + # <% end %> + # # => + #
+ # + #
+ # # # Adding a scope prefixes the input field names: # <%= form_with scope: :post, url: posts_path do |form| %> # <%= form.text_field :title %> @@ -744,7 +761,9 @@ def form_with(model: nil, scope: nil, url: nil, format: nil, **options, &block) options[:skip_default_ids] = !form_with_generates_ids if model - url ||= polymorphic_path(model, format: format) + if url != false + url ||= polymorphic_path(model, format: format) + end model = model.last if model.is_a?(Array) scope ||= model_name_from_record_or_class(model).param_key @@ -1220,7 +1239,7 @@ def hidden_field(object_name, method, options = {}) # file_field(:attachment, :file, class: 'file_input') # # => def file_field(object_name, method, options = {}) - Tags::FileField.new(object_name, method, self, convert_direct_upload_option_to_url(options.dup)).render + Tags::FileField.new(object_name, method, self, convert_direct_upload_option_to_url(method, options.dup)).render end # Returns a textarea opening and closing tag set tailored for accessing a specified attribute (identified by +method+) @@ -1559,7 +1578,11 @@ def html_options_for_form_with(url_for_options = nil, model = nil, html: {}, loc # The following URL is unescaped, this is just a hash of options, and it is the # responsibility of the caller to escape all the values. - html_options[:action] = url_for(url_for_options || {}) + if url_for_options == false || html_options[:action] == false + html_options.delete(:action) + else + html_options[:action] = url_for(url_for_options || {}) + end html_options[:"accept-charset"] = "UTF-8" html_options[:"data-remote"] = true unless local diff --git a/actionview/lib/action_view/helpers/form_tag_helper.rb b/actionview/lib/action_view/helpers/form_tag_helper.rb index 3d5c2e174a3b..db24b1994563 100644 --- a/actionview/lib/action_view/helpers/form_tag_helper.rb +++ b/actionview/lib/action_view/helpers/form_tag_helper.rb @@ -62,6 +62,9 @@ module FormTagHelper # # <%= form_tag('/posts', remote: true) %> # # =>
+ + # form_tag(false, method: :get) + # # => # # form_tag('http://far.away.com/form', authenticity_token: false) # # form without authenticity token @@ -316,7 +319,7 @@ def hidden_field_tag(name, value = nil, options = {}) # file_field_tag 'file', accept: 'text/html', class: 'upload', value: 'index.html' # # => def file_field_tag(name, options = {}) - text_field_tag(name, nil, convert_direct_upload_option_to_url(options.merge(type: :file))) + text_field_tag(name, nil, convert_direct_upload_option_to_url(name, options.merge(type: :file))) end # Creates a password field, a masked text field that will hide the users input behind a mask character. @@ -875,7 +878,11 @@ def html_options_for_form(url_for_options, options) html_options["enctype"] = "multipart/form-data" if html_options.delete("multipart") # The following URL is unescaped, this is just a hash of options, and it is the # responsibility of the caller to escape all the values. - html_options["action"] = url_for(url_for_options) + if url_for_options == false || html_options["action"] == false + html_options.delete("action") + else + html_options["action"] = url_for(url_for_options) + end html_options["accept-charset"] = "UTF-8" html_options["data-remote"] = true if html_options.delete("remote") @@ -954,9 +961,23 @@ def set_default_disable_with(value, tag_options) tag_options.delete("data-disable-with") end - def convert_direct_upload_option_to_url(options) + def convert_direct_upload_option_to_url(name, options) if options.delete(:direct_upload) && respond_to?(:rails_direct_uploads_url) options["data-direct-upload-url"] = rails_direct_uploads_url + + if options[:object] && options[:object].class.respond_to?(:reflect_on_attachment) + attachment_reflection = options[:object].class.reflect_on_attachment(name) + + class_with_attachment = "#{options[:object].class.name.underscore}##{name}" + options["data-direct-upload-attachment-name"] = class_with_attachment + + service_name = attachment_reflection.options[:service_name] || ActiveStorage::Blob.service.name + options["data-direct-upload-token"] = ActiveStorage::DirectUploadToken.generate_direct_upload_token( + class_with_attachment, + service_name, + session + ) + end end options end diff --git a/actionview/lib/action_view/helpers/url_helper.rb b/actionview/lib/action_view/helpers/url_helper.rb index 36751c8add3a..d8f355d7b71e 100644 --- a/actionview/lib/action_view/helpers/url_helper.rb +++ b/actionview/lib/action_view/helpers/url_helper.rb @@ -238,7 +238,15 @@ def link_to(name = nil, options = nil, html_options = nil, &block) # HTTP verb via the +:method+ option within +html_options+. # # ==== Options - # The +options+ hash accepts the same options as +url_for+. + # The +options+ hash accepts the same options as +url_for+. To generate a + # element without an [action] attribute, pass + # false: + # + # <%= button_to "New", false %> + # # => " + # # + # # + # #
" # # Most values in +html_options+ are passed through to the button element, # but there are a few special options: @@ -324,14 +332,20 @@ def link_to(name = nil, options = nil, html_options = nil, &block) # # def button_to(name = nil, options = nil, html_options = nil, &block) html_options, options = options, name if block_given? - options ||= {} html_options ||= {} html_options = html_options.stringify_keys - url = options.is_a?(String) ? options : url_for(options) + url = + case options + when FalseClass then nil + else url_for(options) + end + remote = html_options.delete("remote") params = html_options.delete("params") + authenticity_token = html_options.delete("authenticity_token") + method = html_options.delete("method").to_s method_tag = BUTTON_TAG_METHOD_VERBS.include?(method) ? method_tag(method) : "".html_safe @@ -344,7 +358,7 @@ def button_to(name = nil, options = nil, html_options = nil, &block) request_token_tag = if form_method == "post" request_method = method.empty? ? "post" : method - token_tag(nil, form_options: { action: url, method: request_method }) + token_tag(authenticity_token, form_options: { action: url, method: request_method }) else "" end @@ -768,7 +782,12 @@ def method_not_get_method?(method) def token_tag(token = nil, form_options: {}) if token != false && defined?(protect_against_forgery?) && protect_against_forgery? - token ||= form_authenticity_token(form_options: form_options) + token = + if token == true || token.nil? + form_authenticity_token(form_options: form_options.merge(authenticity_token: token)) + else + token + end tag(:input, type: "hidden", name: request_forgery_protection_token.to_s, value: token, autocomplete: "off") else "" diff --git a/actionview/test/template/form_helper/form_with_test.rb b/actionview/test/template/form_helper/form_with_test.rb index f019e5312787..011640cef7c4 100644 --- a/actionview/test/template/form_helper/form_with_test.rb +++ b/actionview/test/template/form_helper/form_with_test.rb @@ -53,7 +53,7 @@ def form_text(action = "http://www.example.com", local: false, **options) method = method.to_s == "get" ? "get" : "post" - txt = +%{
}, + button_to("Hello", "http://www.example.com", authenticity_token: "token") + ) + ensure + self.request_forgery = false + end + + def test_button_to_with_authenticity_token_true + self.request_forgery = true + + assert_dom_equal( + %{
}, + button_to("Hello", "http://www.example.com", authenticity_token: true) + ) + ensure + self.request_forgery = false + end + + def test_button_to_with_authenticity_token_false + self.request_forgery = true + + assert_dom_equal( + %{
}, + button_to("Hello", "http://www.example.com", authenticity_token: false) + ) + ensure + self.request_forgery = false + end + def test_button_to_with_straight_url assert_dom_equal %{
}, button_to("Hello", "http://www.example.com") end @@ -161,6 +194,20 @@ def test_button_to_with_path ) end + def test_button_to_with_false_url + assert_dom_equal( + %{
}, + button_to("Hello", false) + ) + end + + def test_button_to_with_false_url_and_block + assert_dom_equal( + %{
}, + button_to(false) { "Hello" } + ) + end + def test_button_to_with_straight_url_and_request_forgery self.request_forgery = true diff --git a/activestorage/CHANGELOG.md b/activestorage/CHANGELOG.md index 1cb9ccbe5d03..a5601279d259 100644 --- a/activestorage/CHANGELOG.md +++ b/activestorage/CHANGELOG.md @@ -1,3 +1,7 @@ +* Support direct uploads to multiple services. + + *Dmitry Tsepelev* + * Invalid default content types are deprecated Blobs created with content_type `image/jpg`, `image/pjpeg`, `image/bmp`, `text/javascript` will now produce diff --git a/activestorage/app/assets/javascripts/activestorage.esm.js b/activestorage/app/assets/javascripts/activestorage.esm.js index 87ac2553f9b9..fe9f4938bd91 100644 --- a/activestorage/app/assets/javascripts/activestorage.esm.js +++ b/activestorage/app/assets/javascripts/activestorage.esm.js @@ -508,7 +508,7 @@ function toArray(value) { } class BlobRecord { - constructor(file, checksum, url) { + constructor(file, checksum, url, directUploadToken, attachmentName) { this.file = file; this.attributes = { filename: file.name, @@ -516,6 +516,8 @@ class BlobRecord { byte_size: file.size, checksum: checksum }; + this.directUploadToken = directUploadToken; + this.attachmentName = attachmentName; this.xhr = new XMLHttpRequest; this.xhr.open("POST", url, true); this.xhr.responseType = "json"; @@ -543,7 +545,9 @@ class BlobRecord { create(callback) { this.callback = callback; this.xhr.send(JSON.stringify({ - blob: this.attributes + blob: this.attributes, + direct_upload_token: this.directUploadToken, + attachment_name: this.attachmentName })); } requestDidLoad(event) { @@ -604,10 +608,12 @@ class BlobUpload { let id = 0; class DirectUpload { - constructor(file, url, delegate) { + constructor(file, url, serviceName, attachmentName, delegate) { this.id = ++id; this.file = file; this.url = url; + this.serviceName = serviceName; + this.attachmentName = attachmentName; this.delegate = delegate; } create(callback) { @@ -616,7 +622,7 @@ class DirectUpload { callback(error); return; } - const blob = new BlobRecord(this.file, checksum, this.url); + const blob = new BlobRecord(this.file, checksum, this.url, this.serviceName, this.attachmentName); notify(this.delegate, "directUploadWillCreateBlobWithXHR", blob.xhr); blob.create((error => { if (error) { @@ -647,7 +653,7 @@ class DirectUploadController { constructor(input, file) { this.input = input; this.file = file; - this.directUpload = new DirectUpload(this.file, this.url, this); + this.directUpload = new DirectUpload(this.file, this.url, this.directUploadToken, this.attachmentName, this); this.dispatch("initialize"); } start(callback) { @@ -678,6 +684,12 @@ class DirectUploadController { get url() { return this.input.getAttribute("data-direct-upload-url"); } + get directUploadToken() { + return this.input.getAttribute("data-direct-upload-token"); + } + get attachmentName() { + return this.input.getAttribute("data-direct-upload-attachment-name"); + } dispatch(name, detail = {}) { detail.file = this.file; detail.id = this.directUpload.id; diff --git a/activestorage/app/assets/javascripts/activestorage.js b/activestorage/app/assets/javascripts/activestorage.js index 646f21949d1d..0953dc83c869 100644 --- a/activestorage/app/assets/javascripts/activestorage.js +++ b/activestorage/app/assets/javascripts/activestorage.js @@ -503,7 +503,7 @@ } } class BlobRecord { - constructor(file, checksum, url) { + constructor(file, checksum, url, directUploadToken, attachmentName) { this.file = file; this.attributes = { filename: file.name, @@ -511,6 +511,8 @@ byte_size: file.size, checksum: checksum }; + this.directUploadToken = directUploadToken; + this.attachmentName = attachmentName; this.xhr = new XMLHttpRequest; this.xhr.open("POST", url, true); this.xhr.responseType = "json"; @@ -538,7 +540,9 @@ create(callback) { this.callback = callback; this.xhr.send(JSON.stringify({ - blob: this.attributes + blob: this.attributes, + direct_upload_token: this.directUploadToken, + attachment_name: this.attachmentName })); } requestDidLoad(event) { @@ -596,10 +600,12 @@ } let id = 0; class DirectUpload { - constructor(file, url, delegate) { + constructor(file, url, serviceName, attachmentName, delegate) { this.id = ++id; this.file = file; this.url = url; + this.serviceName = serviceName; + this.attachmentName = attachmentName; this.delegate = delegate; } create(callback) { @@ -608,7 +614,7 @@ callback(error); return; } - const blob = new BlobRecord(this.file, checksum, this.url); + const blob = new BlobRecord(this.file, checksum, this.url, this.serviceName, this.attachmentName); notify(this.delegate, "directUploadWillCreateBlobWithXHR", blob.xhr); blob.create((error => { if (error) { @@ -637,7 +643,7 @@ constructor(input, file) { this.input = input; this.file = file; - this.directUpload = new DirectUpload(this.file, this.url, this); + this.directUpload = new DirectUpload(this.file, this.url, this.directUploadToken, this.attachmentName, this); this.dispatch("initialize"); } start(callback) { @@ -668,6 +674,12 @@ get url() { return this.input.getAttribute("data-direct-upload-url"); } + get directUploadToken() { + return this.input.getAttribute("data-direct-upload-token"); + } + get attachmentName() { + return this.input.getAttribute("data-direct-upload-attachment-name"); + } dispatch(name, detail = {}) { detail.file = this.file; detail.id = this.directUpload.id; diff --git a/activestorage/app/controllers/active_storage/direct_uploads_controller.rb b/activestorage/app/controllers/active_storage/direct_uploads_controller.rb index 99634597f3f9..bfd622d8f7eb 100644 --- a/activestorage/app/controllers/active_storage/direct_uploads_controller.rb +++ b/activestorage/app/controllers/active_storage/direct_uploads_controller.rb @@ -4,8 +4,10 @@ # When the client-side upload is completed, the signed_blob_id can be submitted as part of the form to reference # the blob that was created up front. class ActiveStorage::DirectUploadsController < ActiveStorage::BaseController + include ActiveStorage::DirectUploadToken + def create - blob = ActiveStorage::Blob.create_before_direct_upload!(**blob_args) + blob = ActiveStorage::Blob.create_before_direct_upload!(**blob_args.merge(service_name: verified_service_name)) render json: direct_upload_json(blob) end @@ -14,6 +16,10 @@ def blob_args params.require(:blob).permit(:filename, :byte_size, :checksum, :content_type, metadata: {}).to_h.symbolize_keys end + def verified_service_name + ActiveStorage::DirectUploadToken.verify_direct_upload_token(params[:direct_upload_token], params[:attachment_name], session) + end + def direct_upload_json(blob) blob.as_json(root: false, methods: :signed_id).merge(direct_upload: { url: blob.service_url_for_direct_upload, diff --git a/activestorage/app/javascript/activestorage/blob_record.js b/activestorage/app/javascript/activestorage/blob_record.js index 997c123870ae..cf06d97cf17f 100644 --- a/activestorage/app/javascript/activestorage/blob_record.js +++ b/activestorage/app/javascript/activestorage/blob_record.js @@ -1,16 +1,19 @@ import { getMetaValue } from "./helpers" export class BlobRecord { - constructor(file, checksum, url) { + constructor(file, checksum, url, directUploadToken, attachmentName) { this.file = file this.attributes = { filename: file.name, content_type: file.type || "application/octet-stream", byte_size: file.size, - checksum: checksum + checksum: checksum, } + this.directUploadToken = directUploadToken + this.attachmentName = attachmentName + this.xhr = new XMLHttpRequest this.xhr.open("POST", url, true) this.xhr.responseType = "json" @@ -43,7 +46,11 @@ export class BlobRecord { create(callback) { this.callback = callback - this.xhr.send(JSON.stringify({ blob: this.attributes })) + this.xhr.send(JSON.stringify({ + blob: this.attributes, + direct_upload_token: this.directUploadToken, + attachment_name: this.attachmentName + })) } requestDidLoad(event) { diff --git a/activestorage/app/javascript/activestorage/direct_upload.js b/activestorage/app/javascript/activestorage/direct_upload.js index c2eedf289b80..e5c279d173d6 100644 --- a/activestorage/app/javascript/activestorage/direct_upload.js +++ b/activestorage/app/javascript/activestorage/direct_upload.js @@ -5,10 +5,12 @@ import { BlobUpload } from "./blob_upload" let id = 0 export class DirectUpload { - constructor(file, url, delegate) { + constructor(file, url, serviceName, attachmentName, delegate) { this.id = ++id this.file = file this.url = url + this.serviceName = serviceName + this.attachmentName = attachmentName this.delegate = delegate } @@ -19,7 +21,7 @@ export class DirectUpload { return } - const blob = new BlobRecord(this.file, checksum, this.url) + const blob = new BlobRecord(this.file, checksum, this.url, this.serviceName, this.attachmentName) notify(this.delegate, "directUploadWillCreateBlobWithXHR", blob.xhr) blob.create(error => { diff --git a/activestorage/app/javascript/activestorage/direct_upload_controller.js b/activestorage/app/javascript/activestorage/direct_upload_controller.js index 987050889a75..40bb1b98c51b 100644 --- a/activestorage/app/javascript/activestorage/direct_upload_controller.js +++ b/activestorage/app/javascript/activestorage/direct_upload_controller.js @@ -5,7 +5,7 @@ export class DirectUploadController { constructor(input, file) { this.input = input this.file = file - this.directUpload = new DirectUpload(this.file, this.url, this) + this.directUpload = new DirectUpload(this.file, this.url, this.directUploadToken, this.attachmentName, this) this.dispatch("initialize") } @@ -41,6 +41,14 @@ export class DirectUploadController { return this.input.getAttribute("data-direct-upload-url") } + get directUploadToken() { + return this.input.getAttribute("data-direct-upload-token") + } + + get attachmentName() { + return this.input.getAttribute("data-direct-upload-attachment-name") + } + dispatch(name, detail = {}) { detail.file = this.file detail.id = this.directUpload.id diff --git a/activestorage/lib/active_storage.rb b/activestorage/lib/active_storage.rb index 03acb297e261..6e5c99ac6f99 100644 --- a/activestorage/lib/active_storage.rb +++ b/activestorage/lib/active_storage.rb @@ -41,6 +41,7 @@ module ActiveStorage autoload :Service autoload :Previewer autoload :Analyzer + autoload :DirectUploadToken mattr_accessor :logger mattr_accessor :verifier diff --git a/activestorage/lib/active_storage/direct_upload_token.rb b/activestorage/lib/active_storage/direct_upload_token.rb new file mode 100644 index 000000000000..9b5aa1fecc19 --- /dev/null +++ b/activestorage/lib/active_storage/direct_upload_token.rb @@ -0,0 +1,59 @@ +# frozen_string_literal: true + +module ActiveStorage + module DirectUploadToken + extend self + + SEPARATOR = "." + DIRECT_UPLOAD_TOKEN_LENGTH = 32 + + def generate_direct_upload_token(attachment_name, service_name, session) + token = direct_upload_token(session, attachment_name) + encode_direct_upload_token([service_name, token].join(SEPARATOR)) + end + + def verify_direct_upload_token(token, attachment_name, session) + raise ActiveStorage::InvalidDirectUploadTokenError if token.nil? + + service_name, *token_components = decode_token(token).split(SEPARATOR) + decoded_token = token_components.join(SEPARATOR) + + return service_name if valid_direct_upload_token?(decoded_token, attachment_name, session) + + raise ActiveStorage::InvalidDirectUploadTokenError + end + + private + def direct_upload_token(session, attachment_name) # :doc: + direct_upload_token_hmac(session, "direct_upload##{attachment_name}") + end + + def valid_direct_upload_token?(token, attachment_name, session) # :doc: + correct_token = direct_upload_token(session, attachment_name) + ActiveSupport::SecurityUtils.fixed_length_secure_compare(token, correct_token) + rescue ArgumentError + raise ActiveStorage::InvalidDirectUploadTokenError + end + + def direct_upload_token_hmac(session, identifier) # :doc: + OpenSSL::HMAC.digest( + OpenSSL::Digest::SHA256.new, + real_direct_upload_token(session), + identifier + ) + end + + def real_direct_upload_token(session) # :doc: + session[:_direct_upload_token] ||= SecureRandom.urlsafe_base64(DIRECT_UPLOAD_TOKEN_LENGTH, padding: false) + encode_direct_upload_token(session[:_direct_upload_token]) + end + + def decode_token(encoded_token) # :nodoc: + Base64.urlsafe_decode64(encoded_token) + end + + def encode_direct_upload_token(raw_token) # :nodoc: + Base64.urlsafe_encode64(raw_token) + end + end +end diff --git a/activestorage/lib/active_storage/errors.rb b/activestorage/lib/active_storage/errors.rb index df544a12bc2c..816e5f31e09c 100644 --- a/activestorage/lib/active_storage/errors.rb +++ b/activestorage/lib/active_storage/errors.rb @@ -26,4 +26,7 @@ class FileNotFoundError < Error; end # Raised when a Previewer is unable to generate a preview image. class PreviewError < Error; end + + # Raised when direct upload fails because of the invalid token + class InvalidDirectUploadTokenError < Error; end end diff --git a/activestorage/test/controllers/direct_uploads_controller_test.rb b/activestorage/test/controllers/direct_uploads_controller_test.rb index 8c49121584bc..34adb1508720 100644 --- a/activestorage/test/controllers/direct_uploads_controller_test.rb +++ b/activestorage/test/controllers/direct_uploads_controller_test.rb @@ -2,6 +2,7 @@ require "test_helper" require "database/setup" +require "minitest/mock" if SERVICE_CONFIGURATIONS[:s3] && SERVICE_CONFIGURATIONS[:s3][:access_key_id].present? class ActiveStorage::S3DirectUploadsControllerTest < ActionDispatch::IntegrationTest @@ -24,8 +25,10 @@ class ActiveStorage::S3DirectUploadsControllerTest < ActionDispatch::Integration "library_ID": "12345" } - post rails_direct_uploads_url, params: { blob: { - filename: "hello.txt", byte_size: 6, checksum: checksum, content_type: "text/plain", metadata: metadata } } + ActiveStorage::DirectUploadToken.stub(:verify_direct_upload_token, "local") do + post rails_direct_uploads_url, params: { blob: { + filename: "hello.txt", byte_size: 6, checksum: checksum, content_type: "text/plain", metadata: metadata } } + end response.parsed_body.tap do |details| assert_equal ActiveStorage::Blob.find(details["id"]), ActiveStorage::Blob.find_signed!(details["signed_id"]) @@ -67,8 +70,10 @@ class ActiveStorage::GCSDirectUploadsControllerTest < ActionDispatch::Integratio "library_ID": "12345" } - post rails_direct_uploads_url, params: { blob: { - filename: "hello.txt", byte_size: 6, checksum: checksum, content_type: "text/plain", metadata: metadata } } + ActiveStorage::DirectUploadToken.stub(:verify_direct_upload_token, "local") do + post rails_direct_uploads_url, params: { blob: { + filename: "hello.txt", byte_size: 6, checksum: checksum, content_type: "text/plain", metadata: metadata } } + end @response.parsed_body.tap do |details| assert_equal ActiveStorage::Blob.find(details["id"]), ActiveStorage::Blob.find_signed!(details["signed_id"]) @@ -109,8 +114,10 @@ class ActiveStorage::AzureStorageDirectUploadsControllerTest < ActionDispatch::I "library_ID": "12345" } - post rails_direct_uploads_url, params: { blob: { - filename: "hello.txt", byte_size: 6, checksum: checksum, content_type: "text/plain", metadata: metadata } } + ActiveStorage::DirectUploadToken.stub(:verify_direct_upload_token, "local") do + post rails_direct_uploads_url, params: { blob: { + filename: "hello.txt", byte_size: 6, checksum: checksum, content_type: "text/plain", metadata: metadata } } + end @response.parsed_body.tap do |details| assert_equal ActiveStorage::Blob.find(details["id"]), ActiveStorage::Blob.find_signed!(details["signed_id"]) @@ -139,8 +146,10 @@ class ActiveStorage::DiskDirectUploadsControllerTest < ActionDispatch::Integrati "library_ID": "12345" } - post rails_direct_uploads_url, params: { blob: { - filename: "hello.txt", byte_size: 6, checksum: checksum, content_type: "text/plain", metadata: metadata } } + with_valid_service_name do + post rails_direct_uploads_url, params: { blob: { + filename: "hello.txt", byte_size: 6, checksum: checksum, content_type: "text/plain", metadata: metadata } } + end @response.parsed_body.tap do |details| assert_equal ActiveStorage::Blob.find(details["id"]), ActiveStorage::Blob.find_signed!(details["signed_id"]) @@ -164,9 +173,11 @@ class ActiveStorage::DiskDirectUploadsControllerTest < ActionDispatch::Integrati "library_ID": "12345" } - set_include_root_in_json(true) do - post rails_direct_uploads_url, params: { blob: { - filename: "hello.txt", byte_size: 6, checksum: checksum, content_type: "text/plain", metadata: metadata } } + with_valid_service_name do + set_include_root_in_json(true) do + post rails_direct_uploads_url, params: { blob: { + filename: "hello.txt", byte_size: 6, checksum: checksum, content_type: "text/plain", metadata: metadata } } + end end @response.parsed_body.tap do |details| @@ -175,6 +186,33 @@ class ActiveStorage::DiskDirectUploadsControllerTest < ActionDispatch::Integrati end end + test "handling direct upload with custom service name" do + checksum = OpenSSL::Digest::MD5.base64digest("Hello") + metadata = { + "foo": "bar", + "my_key_1": "my_value_1", + "my_key_2": "my_value_2", + "platform": "my_platform", + "library_ID": "12345" + } + + with_valid_service_name do + post rails_direct_uploads_url, params: { blob: { + filename: "hello.txt", byte_size: 6, checksum: checksum, content_type: "text/plain", metadata: metadata } } + end + + @response.parsed_body.tap do |details| + 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"] + assert_equal metadata, details["metadata"].transform_keys(&:to_sym) + assert_equal "text/plain", details["content_type"] + assert_match(/rails\/active_storage\/disk/, details["direct_upload"]["url"]) + assert_equal({ "Content-Type" => "text/plain" }, details["direct_upload"]["headers"]) + end + end + private def set_include_root_in_json(value) original = ActiveRecord::Base.include_root_in_json @@ -183,4 +221,8 @@ def set_include_root_in_json(value) ensure ActiveRecord::Base.include_root_in_json = original end + + def with_valid_service_name(&block) + ActiveStorage::DirectUploadToken.stub(:verify_direct_upload_token, "local", &block) + end end diff --git a/activestorage/test/direct_upload_token_test.rb b/activestorage/test/direct_upload_token_test.rb new file mode 100644 index 000000000000..ee0526c828d7 --- /dev/null +++ b/activestorage/test/direct_upload_token_test.rb @@ -0,0 +1,54 @@ +# frozen_string_literal: true + +require "test_helper" + +class DirectUploadTokenTest < ActionController::TestCase + setup do + @session = {} + @service_name = "local" + @avatar_attachment_name = "user#avatar" + end + + def test_validates_correct_token + token = ActiveStorage::DirectUploadToken.generate_direct_upload_token(@avatar_attachment_name, @service_name, @session) + verified_service_name = ActiveStorage::DirectUploadToken.verify_direct_upload_token(token, @avatar_attachment_name, @session) + + assert_equal verified_service_name, @service_name + end + + def test_not_validates_nil_token + token = nil + + assert_raises(ActiveStorage::InvalidDirectUploadTokenError) do + ActiveStorage::DirectUploadToken.verify_direct_upload_token(token, @avatar_attachment_name, @session) + end + end + + def test_not_validates_token_when_session_is_empty + token = ActiveStorage::DirectUploadToken.generate_direct_upload_token(@avatar_attachment_name, @service_name, {}) + + assert_raises(ActiveStorage::InvalidDirectUploadTokenError) do + ActiveStorage::DirectUploadToken.verify_direct_upload_token(token, @avatar_attachment_name, @session) + end + end + + def test_not_validates_token_from_different_attachment + background_attachment_name = "user#background" + token = ActiveStorage::DirectUploadToken.generate_direct_upload_token(background_attachment_name, @service_name, @session) + + assert_raises(ActiveStorage::InvalidDirectUploadTokenError) do + ActiveStorage::DirectUploadToken.verify_direct_upload_token(token, @avatar_attachment_name, @session) + end + end + + def test_not_validates_token_from_different_session + token = ActiveStorage::DirectUploadToken.generate_direct_upload_token(@avatar_attachment_name, @service_name, @session) + + another_session = {} + ActiveStorage::DirectUploadToken.generate_direct_upload_token(@avatar_attachment_name, @service_name, another_session) + + assert_raises(ActiveStorage::InvalidDirectUploadTokenError) do + ActiveStorage::DirectUploadToken.verify_direct_upload_token(token, @avatar_attachment_name, another_session) + end + end +end diff --git a/guides/Rakefile b/guides/Rakefile index 383cb5898e45..694976fe8472 100644 --- a/guides/Rakefile +++ b/guides/Rakefile @@ -26,7 +26,7 @@ namespace :guides do # Validate guides ------------------------------------------------------------------------- desc 'Validate guides, use ONLY=foo to process just "foo.html"' - task validate: :encoding do + task :validate do ruby "w3c_validator.rb" end diff --git a/guides/source/action_controller_overview.md b/guides/source/action_controller_overview.md index 735962c8c3c0..55937c2e3607 100644 --- a/guides/source/action_controller_overview.md +++ b/guides/source/action_controller_overview.md @@ -140,6 +140,8 @@ To send a hash, you include the key name inside the brackets: When this form is submitted, the value of `params[:client]` will be `{ "name" => "Acme", "phone" => "12345", "address" => { "postcode" => "12345", "city" => "Carrot City" } }`. Note the nested hash in `params[:client][:address]`. +NOTE: for `` The parameter name must have an array indicator. + The `params` object acts like a Hash, but lets you use symbols and strings interchangeably as keys. ### JSON parameters diff --git a/guides/source/active_storage_overview.md b/guides/source/active_storage_overview.md index f11b6e2f2a74..57c76656cc58 100644 --- a/guides/source/active_storage_overview.md +++ b/guides/source/active_storage_overview.md @@ -1148,9 +1148,12 @@ input.addEventListener('change', (event) => { const uploadFile = (file) => { // your form needs the file_field direct_upload: true, which - // provides data-direct-upload-url + // provides data-direct-upload-url, data-direct-upload-token + // and data-direct-upload-attachment-name const url = input.dataset.directUploadUrl - const upload = new DirectUpload(file, url) + const token = input.dataset.directUploadToken + const attachmentName = input.dataset.directUploadAttachmentName + const upload = new DirectUpload(file, url, token, attachmentName) upload.create((error, blob) => { if (error) { @@ -1169,7 +1172,7 @@ const uploadFile = (file) => { } ``` -If you need to track the progress of the file upload, you can pass a third +If you need to track the progress of the file upload, you can pass a fifth parameter to the `DirectUpload` constructor. During the upload, DirectUpload will call the object's `directUploadWillStoreFileWithXHR` method. You can then bind your own progress handler on the XHR. @@ -1178,8 +1181,8 @@ bind your own progress handler on the XHR. import { DirectUpload } from "@rails/activestorage" class Uploader { - constructor(file, url) { - this.upload = new DirectUpload(this.file, this.url, this) + constructor(file, url, token, attachmentName) { + this.upload = new DirectUpload(file, url, token, attachmentName, this) } upload(file) {