From 33f669a5f851b4095fb6189147ae0fe6f8343d44 Mon Sep 17 00:00:00 2001 From: Jack Jennings Date: Tue, 30 May 2017 13:56:31 -0700 Subject: [PATCH] Add status destroy authorization to policy (#3453) * Add status destroy authorization to policy * Create explicit unreblog status authorization --- .../admin/reported_statuses_controller.rb | 3 ++ app/controllers/api/v1/statuses_controller.rb | 5 ++++ app/policies/status_policy.rb | 18 +++++++++-- app/services/process_interaction_service.rb | 7 +++-- spec/policies/status_policy_spec.rb | 20 +++++++++++++ .../process_interaction_service_spec.rb | 30 ++++++++++++++++++- 6 files changed, 78 insertions(+), 5 deletions(-) diff --git a/app/controllers/admin/reported_statuses_controller.rb b/app/controllers/admin/reported_statuses_controller.rb index 0e7a89437..32434d30f 100644 --- a/app/controllers/admin/reported_statuses_controller.rb +++ b/app/controllers/admin/reported_statuses_controller.rb @@ -2,6 +2,8 @@ module Admin class ReportedStatusesController < BaseController + include Authorization + before_action :set_report before_action :set_status @@ -11,6 +13,7 @@ module Admin end def destroy + authorize @status, :destroy? RemovalWorker.perform_async(@status.id) redirect_to admin_report_path(@report) end diff --git a/app/controllers/api/v1/statuses_controller.rb b/app/controllers/api/v1/statuses_controller.rb index 592540f45..7386d7158 100644 --- a/app/controllers/api/v1/statuses_controller.rb +++ b/app/controllers/api/v1/statuses_controller.rb @@ -79,7 +79,10 @@ class Api::V1::StatusesController < ApiController def destroy @status = Status.where(account_id: current_user.account).find(params[:id]) + authorize @status, :destroy? + RemovalWorker.perform_async(@status.id) + render_empty end @@ -93,6 +96,8 @@ class Api::V1::StatusesController < ApiController @status = reblog.reblog @reblogs_map = { @status.id => false } + authorize reblog, :unreblog? + RemovalWorker.perform_async(reblog.id) render :show diff --git a/app/policies/status_policy.rb b/app/policies/status_policy.rb index 41d63fcbc..2ded61850 100644 --- a/app/policies/status_policy.rb +++ b/app/policies/status_policy.rb @@ -10,9 +10,9 @@ class StatusPolicy def show? if direct? - status.account.id == account&.id || status.mentions.where(account: account).exists? + owned? || status.mentions.where(account: account).exists? elsif private? - status.account.id == account&.id || account&.following?(status.account) || status.mentions.where(account: account).exists? + owned? || account&.following?(status.account) || status.mentions.where(account: account).exists? else account.nil? || !status.account.blocking?(account) end @@ -22,12 +22,26 @@ class StatusPolicy !direct? && !private? && show? end + def destroy? + admin? || owned? + end + + alias unreblog? destroy? + private + def admin? + account&.user&.admin? + end + def direct? status.direct_visibility? end + def owned? + status.account.id == account&.id + end + def private? status.private_visibility? end diff --git a/app/services/process_interaction_service.rb b/app/services/process_interaction_service.rb index bd9afaf2e..584a109ad 100644 --- a/app/services/process_interaction_service.rb +++ b/app/services/process_interaction_service.rb @@ -2,6 +2,7 @@ class ProcessInteractionService < BaseService include AuthorExtractor + include Authorization # Record locally the remote interaction with our user # @param [String] envelope Salmon envelope @@ -46,7 +47,7 @@ class ProcessInteractionService < BaseService reflect_unblock!(account, target_account) end end - rescue Goldfinger::Error, HTTP::Error, OStatus2::BadSalmonError + rescue Goldfinger::Error, HTTP::Error, OStatus2::BadSalmonError, Mastodon::NotPermittedError nil end @@ -103,7 +104,9 @@ class ProcessInteractionService < BaseService return if status.nil? - RemovalWorker.perform_async(status.id) if account.id == status.account_id + authorize_with account, status, :destroy? + + RemovalWorker.perform_async(status.id) end def favourite!(xml, from_account) diff --git a/spec/policies/status_policy_spec.rb b/spec/policies/status_policy_spec.rb index 8e85efb8e..bacb8fd9e 100644 --- a/spec/policies/status_policy_spec.rb +++ b/spec/policies/status_policy_spec.rb @@ -4,7 +4,9 @@ require 'pundit/rspec' RSpec.describe StatusPolicy, type: :model do subject { described_class } + let(:admin) { Fabricate(:user, admin: true) } let(:alice) { Fabricate(:account, username: 'alice') } + let(:bob) { Fabricate(:account, username: 'bob') } let(:status) { Fabricate(:status, account: alice) } permissions :show?, :reblog? do @@ -86,4 +88,22 @@ RSpec.describe StatusPolicy, type: :model do expect(subject).to_not permit(viewer, status) end end + + permissions :destroy?, :unreblog? do + it 'grants access when account is deleter' do + expect(subject).to permit(status.account, status) + end + + it 'grants access when account is admin' do + expect(subject).to permit(admin.account, status) + end + + it 'denies access when account is not deleter' do + expect(subject).to_not permit(bob, status) + end + + it 'denies access when no deleter' do + expect(subject).to_not permit(nil, status) + end + end end diff --git a/spec/services/process_interaction_service_spec.rb b/spec/services/process_interaction_service_spec.rb index f589f690d..3ea7aec59 100644 --- a/spec/services/process_interaction_service_spec.rb +++ b/spec/services/process_interaction_service_spec.rb @@ -7,6 +7,35 @@ RSpec.describe ProcessInteractionService do subject { ProcessInteractionService.new } + describe 'status delete slap' do + let(:remote_status) { Fabricate(:status, account: remote_sender) } + let(:envelope) { OStatus2::Salmon.new.pack(payload, sender.keypair) } + let(:payload) { + <<~XML + + + carol@localdomain.com + carol + https://webdomain.com/users/carol + + + #{remote_status.id} + http://activitystrea.ms/schema/1.0/delete + + XML + } + + before do + receiver.update(locked: true) + remote_sender.update(private_key: sender.private_key, public_key: remote_sender.public_key) + end + + it 'deletes a record' do + expect(RemovalWorker).to receive(:perform_async).with(remote_status.id) + subject.call(envelope, receiver) + end + end + describe 'follow request slap' do before do receiver.update(locked: true) @@ -60,7 +89,6 @@ XML end end - describe 'follow request authorization slap' do before do receiver.update(locked: true)