Add status destroy authorization to policy (#3453)

* Add status destroy authorization to policy

* Create explicit unreblog status authorization

diff --git a/app/controllers/admin/reported_statuses_controller.rb b/app/controllers/admin/reported_statuses_controller.rb
index 0e7a8943709560edcea632eecb99d525efc91907..32434d30f707dde787a5159e1934a91eadbce235 100644 (file)
--- 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 592540f45772dfac22c5d911c1ccb0bb5d87f607..7386d715825525a647e9c70c6bbf6bda3a1121cd 100644 (file)
--- 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 41d63fcbcbca047b8cae40dfa408984e9ea8439f..2ded61850a3a4385ef6211eacf25a115140f61b8 100644 (file)
--- 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 bd9afaf2eff101726ef6b2da8cc4265717e515c5..584a109ad476d590f07b212771d4e048ef785723 100644 (file)
--- 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 8e85efb8e39785689c0eb9c5ad4af0979f274333..bacb8fd9e3fe9e62d4c80367022cc8ce0ac833af 100644 (file)
--- 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 f589f690df49a10b81347b4ad669efdd963a78cb..3ea7aec59bca6f8ef4c8660ad39181dc907f7196 100644 (file)
--- 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
+        <entry xmlns="http://www.w3.org/2005/Atom" xmlns:activity="http://activitystrea.ms/spec/1.0/">
+          <author>
+            <email>carol@localdomain.com</email>
+            <name>carol</name>
+            <uri>https://webdomain.com/users/carol</uri>
+          </author>
+
+          <id>#{remote_status.id}</id>
+          <activity:verb>http://activitystrea.ms/schema/1.0/delete</activity:verb>
+        </entry>
+      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)