commit: 33f669a5f851b4095fb6189147ae0fe6f8343d44
parent: 3576fa0d591db69a1727153a1130ff5bebf37167
Author: Jack Jennings <jack@standard-library.com>
Date: Tue, 30 May 2017 13:56:31 -0700
Add status destroy authorization to policy (#3453)
* Add status destroy authorization to policy
* Create explicit unreblog status authorization
Diffstat:
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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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)
