diff --git a/app/models/post.rb b/app/models/post.rb index 76db8ddd769..c386ae11e93 100644 --- a/app/models/post.rb +++ b/app/models/post.rb @@ -304,7 +304,11 @@ class Post < ActiveRecord::Base each_upload_url do |url| uri = URI.parse(url) if FileHelper.is_supported_media?(File.basename(uri.path)) - raw = raw.sub(Discourse.store.s3_upload_host, "#{Discourse.base_url}/#{Upload::SECURE_MEDIA_ROUTE}") + raw = raw.sub( + url, Rails.application.routes.url_for( + controller: "uploads", action: "show_secure", path: uri.path[1..-1], host: Discourse.current_hostname + ) + ) end end end diff --git a/app/models/upload.rb b/app/models/upload.rb index df8f6dfa504..24eb7d9fd30 100644 --- a/app/models/upload.rb +++ b/app/models/upload.rb @@ -9,7 +9,6 @@ class Upload < ActiveRecord::Base SHA1_LENGTH = 40 SEEDED_ID_THRESHOLD = 0 URL_REGEX ||= /(\/original\/\dX[\/\.\w]*\/(\h+)[\.\w]*)/ - SECURE_MEDIA_ROUTE = "secure-media-uploads" belongs_to :user belongs_to :access_control_post, class_name: 'Post' @@ -155,16 +154,28 @@ class Upload < ActiveRecord::Base def self.secure_media_url?(url) # we do not want to exclude topic links that for whatever reason # have secure-media-uploads in the URL e.g. /t/secure-media-uploads-are-cool/223452 - url.include?(SECURE_MEDIA_ROUTE) && !url.include?("/t/") && FileHelper.is_supported_media?(url) + route = UrlHelper.rails_route_from_url(url) + route[:action] == "show_secure" && route[:controller] == "uploads" && FileHelper.is_supported_media?(url) + rescue ActionController::RoutingError + false end def self.signed_url_from_secure_media_url(url) - secure_upload_s3_path = url.sub(Discourse.base_url, "").sub("/#{SECURE_MEDIA_ROUTE}/", "") + route = UrlHelper.rails_route_from_url(url) + url = Rails.application.routes.url_for(route.merge(only_path: true)) + secure_upload_s3_path = url[url.index(route[:path])..-1] Discourse.store.signed_url_for_path(secure_upload_s3_path) end def self.secure_media_url_from_upload_url(url) - url.sub(SiteSetting.Upload.absolute_base_url, "/#{SECURE_MEDIA_ROUTE}") + return url if !url.include?(SiteSetting.Upload.absolute_base_url) + uri = URI.parse(url) + Rails.application.routes.url_for( + controller: "uploads", + action: "show_secure", + path: uri.path[1..-1], + only_path: true + ) end def self.short_path(sha1:, extension:) diff --git a/lib/url_helper.rb b/lib/url_helper.rb index 893af239df9..38696f84be9 100644 --- a/lib/url_helper.rb +++ b/lib/url_helper.rb @@ -67,6 +67,11 @@ class UrlHelper UrlHelper.encode_component(CGI.unescapeHTML(UrlHelper.unencode(uri))) end + def self.rails_route_from_url(url) + path = URI.parse(url).path + Rails.application.routes.recognize_path(path) + end + def self.s3_presigned_url?(url) url[/x-amz-(algorithm|credential)/i].present? end diff --git a/spec/models/upload_spec.rb b/spec/models/upload_spec.rb index 9049a6d1f1e..23d79fc4692 100644 --- a/spec/models/upload_spec.rb +++ b/spec/models/upload_spec.rb @@ -432,6 +432,72 @@ describe Upload do expect(user.user_profile.card_background_upload_id).to eq(nil) expect(user.user_profile.profile_background_upload_id).to eq(nil) end + end + context ".signed_url_from_secure_media_url" do + before do + # must be done so signed_url_for_path exists + enable_secure_media + end + + it "correctly gives back a signed url from a path only" do + secure_url = "/secure-media-uploads/original/1X/c5a2c4ba0fa390f5aac5c2c1a12416791ebdd9e9.png" + signed_url = Upload.signed_url_from_secure_media_url(secure_url) + expect(signed_url).not_to include("secure-media-uploads") + expect(UrlHelper.s3_presigned_url?(signed_url)).to eq(true) + end + + it "correctly gives back a signed url from a full url" do + secure_url = "http://localhost:3000/secure-media-uploads/original/1X/c5a2c4ba0fa390f5aac5c2c1a12416791ebdd9e9.png" + signed_url = Upload.signed_url_from_secure_media_url(secure_url) + expect(signed_url).not_to include(Discourse.base_url) + expect(UrlHelper.s3_presigned_url?(signed_url)).to eq(true) + end + end + + context ".secure_media_url_from_upload_url" do + before do + # must be done so signed_url_for_path exists + enable_secure_media + end + + it "gets the secure media url from an S3 upload url" do + upload = Fabricate(:upload_s3, secure: true) + url = upload.url + secure_url = Upload.secure_media_url_from_upload_url(url) + expect(secure_url).not_to include(SiteSetting.Upload.absolute_base_url) + end + end + + context ".secure_media_url?" do + it "works for a secure media url with or without schema + host" do + url = "//localhost:3000/secure-media-uploads/original/2X/f/f62055931bb702c7fd8f552fb901f977e0289a18.png" + expect(Upload.secure_media_url?(url)).to eq(true) + url = "/secure-media-uploads/original/2X/f/f62055931bb702c7fd8f552fb901f977e0289a18.png" + expect(Upload.secure_media_url?(url)).to eq(true) + url = "http://localhost:3000/secure-media-uploads/original/2X/f/f62055931bb702c7fd8f552fb901f977e0289a18.png" + expect(Upload.secure_media_url?(url)).to eq(true) + end + + it "does not get false positives on a topic url" do + url = "/t/secure-media-uploads-are-cool/42839" + expect(Upload.secure_media_url?(url)).to eq(false) + end + + it "returns true only for secure media URL for actual media (images/video/audio)" do + url = "/secure-media-uploads/original/2X/f/f62055931bb702c7fd8f552fb901f977e0289a18.mp4" + expect(Upload.secure_media_url?(url)).to eq(true) + url = "/secure-media-uploads/original/2X/f/f62055931bb702c7fd8f552fb901f977e0289a18.png" + expect(Upload.secure_media_url?(url)).to eq(true) + url = "/secure-media-uploads/original/2X/f/f62055931bb702c7fd8f552fb901f977e0289a18.mp3" + expect(Upload.secure_media_url?(url)).to eq(true) + url = "/secure-media-uploads/original/2X/f/f62055931bb702c7fd8f552fb901f977e0289a18.pdf" + expect(Upload.secure_media_url?(url)).to eq(false) + end + + it "does not work for regular upload urls" do + url = "/uploads/default/test_0/original/1X/e1864389d8252958586c76d747b069e9f68827e3.png" + expect(Upload.secure_media_url?(url)).to eq(false) + end end end