diff options
| -rw-r--r-- | app/controllers/media_proxy_controller.rb | 5 | ||||
| -rw-r--r-- | db/migrate/20200622213645_media_attachment_ids_to_timestamp_ids.rb | 17 | ||||
| -rw-r--r-- | db/schema.rb | 26 | ||||
| -rw-r--r-- | spec/controllers/media_controller_spec.rb | 3 | ||||
| -rw-r--r-- | spec/controllers/media_proxy_controller_spec.rb | 42 |
5 files changed, 77 insertions, 16 deletions
diff --git a/app/controllers/media_proxy_controller.rb b/app/controllers/media_proxy_controller.rb index a8261ec2b..0b1d09de9 100644 --- a/app/controllers/media_proxy_controller.rb +++ b/app/controllers/media_proxy_controller.rb @@ -2,6 +2,7 @@ class MediaProxyController < ApplicationController include RoutingHelper + include Authorization skip_before_action :store_current_location skip_before_action :require_functional! @@ -10,12 +11,14 @@ class MediaProxyController < ApplicationController rescue_from ActiveRecord::RecordInvalid, with: :not_found rescue_from Mastodon::UnexpectedResponseError, with: :not_found + rescue_from Mastodon::NotPermittedError, with: :not_found rescue_from HTTP::TimeoutError, HTTP::ConnectionError, OpenSSL::SSL::SSLError, with: :internal_server_error def show RedisLock.acquire(lock_options) do |lock| if lock.acquired? - @media_attachment = MediaAttachment.remote.find(params[:id]) + @media_attachment = MediaAttachment.remote.attached.find(params[:id]) + authorize @media_attachment.status, :show? redownload! if @media_attachment.needs_redownload? && !reject_media? else raise Mastodon::RaceConditionError diff --git a/db/migrate/20200622213645_media_attachment_ids_to_timestamp_ids.rb b/db/migrate/20200622213645_media_attachment_ids_to_timestamp_ids.rb new file mode 100644 index 000000000..5c6865b92 --- /dev/null +++ b/db/migrate/20200622213645_media_attachment_ids_to_timestamp_ids.rb @@ -0,0 +1,17 @@ +class MediaAttachmentIdsToTimestampIds < ActiveRecord::Migration[5.1] + def up + # Set up the media_attachments.id column to use our timestamp-based IDs. + safety_assured do + execute("ALTER TABLE media_attachments ALTER COLUMN id SET DEFAULT timestamp_id('media_attachments')") + end + + # Make sure we have a sequence to use. + Mastodon::Snowflake.ensure_id_sequences_exist + end + + def down + execute("LOCK media_attachments") + execute("SELECT setval('media_attachments_id_seq', (SELECT MAX(id) FROM media_attachments))") + execute("ALTER TABLE media_attachments ALTER COLUMN id SET DEFAULT nextval('media_attachments_id_seq')") + end +end diff --git a/db/schema.rb b/db/schema.rb index 712ba1d2d..cf31b6d23 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -77,6 +77,16 @@ ActiveRecord::Schema.define(version: 2020_06_28_133322) do t.index ["target_account_id"], name: "index_account_moderation_notes_on_target_account_id" end + create_table "account_notes", force: :cascade do |t| + t.bigint "account_id" + t.bigint "target_account_id" + t.text "comment", null: false + t.datetime "created_at", null: false + t.datetime "updated_at", null: false + t.index ["account_id", "target_account_id"], name: "index_account_notes_on_account_id_and_target_account_id", unique: true + t.index ["target_account_id"], name: "index_account_notes_on_target_account_id" + end + create_table "account_pins", force: :cascade do |t| t.bigint "account_id" t.bigint "target_account_id" @@ -471,7 +481,7 @@ ActiveRecord::Schema.define(version: 2020_06_28_133322) do t.index ["user_id", "timeline"], name: "index_markers_on_user_id_and_timeline", unique: true end - create_table "media_attachments", force: :cascade do |t| + create_table "media_attachments", id: :bigint, default: -> { "timestamp_id('media_attachments'::text)" }, force: :cascade do |t| t.bigint "status_id" t.string "file_file_name" t.string "file_content_type" @@ -833,16 +843,6 @@ ActiveRecord::Schema.define(version: 2020_06_28_133322) do t.index ["user_id"], name: "index_user_invite_requests_on_user_id" end - create_table "account_notes", force: :cascade do |t| - t.bigint "account_id" - t.bigint "target_account_id" - t.text "comment", null: false - t.datetime "created_at", null: false - t.datetime "updated_at", null: false - t.index ["account_id", "target_account_id"], name: "index_account_notes_on_account_id_and_target_account_id", unique: true - t.index ["target_account_id"], name: "index_account_notes_on_target_account_id" - end - create_table "users", force: :cascade do |t| t.string "email", default: "", null: false t.datetime "created_at", null: false @@ -918,6 +918,8 @@ ActiveRecord::Schema.define(version: 2020_06_28_133322) do add_foreign_key "account_migrations", "accounts", on_delete: :cascade add_foreign_key "account_moderation_notes", "accounts" add_foreign_key "account_moderation_notes", "accounts", column: "target_account_id" + add_foreign_key "account_notes", "accounts", column: "target_account_id", on_delete: :cascade + add_foreign_key "account_notes", "accounts", on_delete: :cascade add_foreign_key "account_pins", "accounts", column: "target_account_id", on_delete: :cascade add_foreign_key "account_pins", "accounts", on_delete: :cascade add_foreign_key "account_stats", "accounts", on_delete: :cascade @@ -999,8 +1001,6 @@ ActiveRecord::Schema.define(version: 2020_06_28_133322) do add_foreign_key "statuses_tags", "tags", name: "fk_3081861e21", on_delete: :cascade add_foreign_key "tombstones", "accounts", on_delete: :cascade add_foreign_key "user_invite_requests", "users", on_delete: :cascade - add_foreign_key "account_notes", "accounts", column: "target_account_id", on_delete: :cascade - add_foreign_key "account_notes", "accounts", on_delete: :cascade add_foreign_key "users", "accounts", name: "fk_50500f500d", on_delete: :cascade add_foreign_key "users", "invites", on_delete: :nullify add_foreign_key "users", "oauth_applications", column: "created_by_application_id", on_delete: :nullify diff --git a/spec/controllers/media_controller_spec.rb b/spec/controllers/media_controller_spec.rb index ac44a76f2..2925aed59 100644 --- a/spec/controllers/media_controller_spec.rb +++ b/spec/controllers/media_controller_spec.rb @@ -28,9 +28,8 @@ describe MediaController do end it 'raises when not permitted to view' do - status = Fabricate(:status) + status = Fabricate(:status, visibility: :direct) media_attachment = Fabricate(:media_attachment, status: status) - allow_any_instance_of(MediaController).to receive(:authorize).and_raise(ActiveRecord::RecordNotFound) get :show, params: { id: media_attachment.to_param } expect(response).to have_http_status(404) diff --git a/spec/controllers/media_proxy_controller_spec.rb b/spec/controllers/media_proxy_controller_spec.rb new file mode 100644 index 000000000..32510cf43 --- /dev/null +++ b/spec/controllers/media_proxy_controller_spec.rb @@ -0,0 +1,42 @@ +# frozen_string_literal: true + +require 'rails_helper' + +describe MediaProxyController do + render_views + + before do + stub_request(:get, 'http://example.com/attachment.png').to_return(request_fixture('avatar.txt')) + end + + describe '#show' do + it 'redirects when attached to a status' do + status = Fabricate(:status) + media_attachment = Fabricate(:media_attachment, status: status, remote_url: 'http://example.com/attachment.png') + get :show, params: { id: media_attachment.id } + + expect(response).to have_http_status(302) + end + + it 'responds with missing when there is not an attached status' do + media_attachment = Fabricate(:media_attachment, status: nil, remote_url: 'http://example.com/attachment.png') + get :show, params: { id: media_attachment.id } + + expect(response).to have_http_status(404) + end + + it 'raises when id cant be found' do + get :show, params: { id: 'missing' } + + expect(response).to have_http_status(404) + end + + it 'raises when not permitted to view' do + status = Fabricate(:status, visibility: :direct) + media_attachment = Fabricate(:media_attachment, status: status, remote_url: 'http://example.com/attachment.png') + get :show, params: { id: media_attachment.id } + + expect(response).to have_http_status(404) + end + end +end |