From e96e9cae62a229f03b30573a6d06a97765c2a766 Mon Sep 17 00:00:00 2001 From: ThibG Date: Tue, 7 Jul 2020 02:01:13 +0200 Subject: Add test for removing endorsed accounts on account deletion/suspension (#14241) --- spec/services/suspend_account_service_spec.rb | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) (limited to 'spec') diff --git a/spec/services/suspend_account_service_spec.rb b/spec/services/suspend_account_service_spec.rb index eebbbc12a..32726d763 100644 --- a/spec/services/suspend_account_service_spec.rb +++ b/spec/services/suspend_account_service_spec.rb @@ -20,6 +20,7 @@ RSpec.describe SuspendAccountService, type: :service do let!(:passive_relationship) { Fabricate(:follow, target_account: account) } let!(:remote_alice) { Fabricate(:account, inbox_url: 'https://alice.com/inbox', protocol: :activitypub) } let!(:remote_bob) { Fabricate(:account, inbox_url: 'https://bob.com/inbox', protocol: :activitypub) } + let!(:endorsment) { Fabricate(:account_pin, account: passive_relationship.account, target_account: account) } it 'deletes associated records' do is_expected.to change { @@ -30,8 +31,9 @@ RSpec.describe SuspendAccountService, type: :service do account.favourites, account.active_relationships, account.passive_relationships, + AccountPin.where(target_account: account), ].map(&:count) - }.from([1, 1, 1, 1, 1, 1]).to([0, 0, 0, 0, 0, 0]) + }.from([1, 1, 1, 1, 1, 1, 1]).to([0, 0, 0, 0, 0, 0, 0]) end it 'sends a delete actor activity to all known inboxes' do -- cgit From 6e25574ce599cbc37b7215ded03c7d07208af6bb Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Tue, 7 Jul 2020 15:26:51 +0200 Subject: Fix media attachments enumeration (#14254) * Fix media attachment enumeration * Switch media_attachments id to snowflake ids Co-authored-by: Thibaut Girka --- app/controllers/media_proxy_controller.rb | 5 ++- ...213645_media_attachment_ids_to_timestamp_ids.rb | 17 +++++++++ db/schema.rb | 26 +++++++------- spec/controllers/media_controller_spec.rb | 3 +- spec/controllers/media_proxy_controller_spec.rb | 42 ++++++++++++++++++++++ 5 files changed, 77 insertions(+), 16 deletions(-) create mode 100644 db/migrate/20200622213645_media_attachment_ids_to_timestamp_ids.rb create mode 100644 spec/controllers/media_proxy_controller_spec.rb (limited to 'spec') 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 -- cgit