bida/bastodon
6
0
Fork 0
Code Issues 7 Pull requests Releases Wiki Activity

Extract authorization policy for viewing statuses (#3150)

Browse source
This commit is contained in:
Jack Jennings 2017-05-29 09:22:22 -07:00 committed by Eugen Rochko
parent 9a81be0d37
commit 3a2003ba86
16 changed files with 155 additions and 80 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

1
Gemfile
View file

@ -38,6 +38,7 @@ gem 'nokogiri', '~> 1.7'
gem 'oj', '~> 3.0'
gem 'ostatus2', '~> 2.0'
gem 'ox', '~> 2.5'
gem 'pundit', '~> 1.1'
gem 'rabl', '~> 0.13'
gem 'rack-attack', '~> 5.0'
gem 'rack-cors', '~> 0.4', require: 'rack/cors'

3
Gemfile.lock
View file

@ -283,6 +283,8 @@ GEM
pry (>= 0.10.4)
public_suffix (2.0.5)
puma (3.8.2)
pundit (1.1.0)
activesupport (>= 3.0.0)
rabl (0.13.1)
activesupport (>= 2.3.14)
rack (2.0.3)
@ -519,6 +521,7 @@ DEPENDENCIES
pkg-config (~> 1.2)
pry-rails (~> 0.3)
puma (~> 3.8)
pundit (~> 1.1)
rabl (~> 0.13)
rack-attack (~> 5.0)
rack-cors (~> 0.4)

4
app/controllers/api/activitypub/activities_controller.rb
View file

@ -1,6 +1,8 @@
# frozen_string_literal: true
class Api::Activitypub::ActivitiesController < ApiController
include Authorization
# before_action :set_follow, only: [:show_follow]
before_action :set_status, only: [:show_status]
@ -8,7 +10,7 @@ class Api::Activitypub::ActivitiesController < ApiController
# Show a status in AS2 format, as either an Announce (reblog) or a Create (post) activity.
def show_status
return forbidden unless @status.permitted?
authorize @status, :show?
if @status.reblog?
render :show_status_announce

4
app/controllers/api/activitypub/notes_controller.rb
View file

@ -1,12 +1,14 @@
# frozen_string_literal: true
class Api::Activitypub::NotesController < ApiController
include Authorization
before_action :set_status
respond_to :activitystreams2
def show
forbidden unless @status.permitted?
authorize @status, :show?
end
private

7
app/controllers/api/v1/statuses_controller.rb
View file

@ -1,6 +1,8 @@
# frozen_string_literal: true
class Api::V1::StatusesController < ApiController
include Authorization
before_action :authorize_if_got_token, except: [:create, :destroy, :reblog, :unreblog, :favourite, :unfavourite, :mute, :unmute]
before_action -> { doorkeeper_authorize! :write }, only: [:create, :destroy, :reblog, :unreblog, :favourite, :unfavourite, :mute, :unmute]
before_action :require_user!, except: [:show, :context, :card, :reblogged_by, :favourited_by]
@ -130,7 +132,10 @@ class Api::V1::StatusesController < ApiController
def set_status
@status = Status.find(params[:id])
raise ActiveRecord::RecordNotFound unless @status.permitted?(current_account)
authorize @status, :show?
rescue Mastodon::NotPermittedError
# Reraise in order to get a 404 instead of a 403 error code
raise ActiveRecord::RecordNotFound
end
def set_conversation

22
app/controllers/concerns/authorization.rb Normal file
View file

@ -0,0 +1,22 @@
# frozen_string_literal: true
module Authorization
extend ActiveSupport::Concern
include Pundit
def pundit_user
current_account
end
def authorize(*)
super
rescue Pundit::NotAuthorizedError
raise Mastodon::NotPermittedError
end
def authorize_with(user, record, query)
Pundit.authorize(user, record, query)
rescue Pundit::NotAuthorizedError
raise Mastodon::NotPermittedError
end
end

7
app/controllers/media_controller.rb
View file

@ -1,6 +1,8 @@
# frozen_string_literal: true
class MediaController < ApplicationController
include Authorization
before_action :verify_permitted_status
def show
@ -14,6 +16,9 @@ class MediaController < ApplicationController
end
def verify_permitted_status
raise ActiveRecord::RecordNotFound unless media_attachment.status.permitted?(current_account)
authorize media_attachment.status, :show?
rescue Mastodon::NotPermittedError
# Reraise in order to get a 404 instead of a 403 error code
raise ActiveRecord::RecordNotFound
end
end

7
app/controllers/statuses_controller.rb
View file

@ -1,6 +1,8 @@
# frozen_string_literal: true
class StatusesController < ApplicationController
include Authorization
layout 'public'
before_action :set_account
@ -30,7 +32,10 @@ class StatusesController < ApplicationController
@stream_entry = @status.stream_entry
@type = @stream_entry.activity_type.downcase
raise ActiveRecord::RecordNotFound unless @status.permitted?(current_account)
authorize @status, :show?
rescue Mastodon::NotPermittedError
# Reraise in order to get a 404
raise ActiveRecord::RecordNotFound
end
def check_account_suspension

8
app/controllers/stream_entries_controller.rb
View file

@ -1,6 +1,8 @@
# frozen_string_literal: true
class StreamEntriesController < ApplicationController
include Authorization
layout 'public'
before_action :set_account
@ -42,7 +44,11 @@ class StreamEntriesController < ApplicationController
@stream_entry = @account.stream_entries.where(activity_type: 'Status').find(params[:id])
@type = @stream_entry.activity_type.downcase
raise ActiveRecord::RecordNotFound if @stream_entry.activity.nil? || (@stream_entry.hidden? && !@stream_entry.activity.permitted?(current_account))
raise ActiveRecord::RecordNotFound if @stream_entry.activity.nil?
authorize @stream_entry.activity, :show? if @stream_entry.hidden?
rescue Mastodon::NotPermittedError
# Reraise in order to get a 404
raise ActiveRecord::RecordNotFound
end
def check_account_suspension

12
app/models/status.rb
View file

@ -113,16 +113,6 @@ class Status < ApplicationRecord
private_visibility? || direct_visibility?
end
def permitted?(other_account = nil)
if direct_visibility?
account.id == other_account&.id || mentions.where(account: other_account).exists?
elsif private_visibility?
account.id == other_account&.id || other_account&.following?(account) || mentions.where(account: other_account).exists?
else
other_account.nil? || !account.blocking?(other_account)
end
end
def ancestors(account = nil)
ids = Rails.cache.fetch("ancestors:#{id}") { (Status.find_by_sql(['WITH RECURSIVE search_tree(id, in_reply_to_id, path) AS (SELECT id, in_reply_to_id, ARRAY[id] FROM statuses WHERE id = ? UNION ALL SELECT statuses.id, statuses.in_reply_to_id, path || statuses.id FROM search_tree JOIN statuses ON statuses.id = search_tree.in_reply_to_id WHERE NOT statuses.id = ANY(path)) SELECT id FROM search_tree ORDER BY path DESC', id]) - [self]).pluck(:id) }
find_statuses_from_tree_path(ids, account)
@ -301,7 +291,7 @@ class Status < ApplicationRecord
should_filter ||= account&.domain_blocking?(status.account.domain)
should_filter ||= account&.muting?(status.account_id)
should_filter ||= (status.account.silenced? && !account&.following?(status.account_id))
should_filter ||= !status.permitted?(account)
should_filter ||= !StatusPolicy.new(account, status).show?
should_filter
end
end

20
app/policies/status_policy.rb Normal file
View file

@ -0,0 +1,20 @@
# frozen_string_literal: true
class StatusPolicy
attr_reader :account, :status
def initialize(account, status)
@account = account
@status = status
end
def show?
if status.direct_visibility?
status.account.id == account&.id || status.mentions.where(account: account).exists?
elsif status.private_visibility?
status.account.id == account&.id || account&.following?(status.account) || status.mentions.where(account: account).exists?
else
account.nil? || !status.account.blocking?(account)
end
end
end

4
app/services/favourite_service.rb
View file

@ -1,12 +1,14 @@
# frozen_string_literal: true
class FavouriteService < BaseService
include Authorization
# Favourite a status and notify remote user
# @param [Account] account
# @param [Status] status
# @return [Favourite]
def call(account, status)
raise Mastodon::NotPermittedError unless status.permitted?(account)
authorize_with account, status, :show?
favourite = Favourite.create!(account: account, status: status)

4
app/services/reblog_service.rb
View file

@ -1,6 +1,7 @@
# frozen_string_literal: true
class ReblogService < BaseService
include Authorization
include StreamEntryRenderer
# Reblog a status and notify its remote author
@ -10,7 +11,8 @@ class ReblogService < BaseService
def call(account, reblogged_status)
reblogged_status = reblogged_status.reblog if reblogged_status.reblog?
raise Mastodon::NotPermittedError if reblogged_status.direct_visibility? || reblogged_status.private_visibility? || !reblogged_status.permitted?(account)
authorize_with account, reblogged_status, :show?
raise Mastodon::NotPermittedError if reblogged_status.direct_visibility? || reblogged_status.private_visibility?
reblog = account.statuses.create!(reblog: reblogged_status, text: '')

2
spec/controllers/media_controller_spec.rb
View file

@ -30,7 +30,7 @@ describe MediaController do
it 'raises when not permitted to view' do
status = Fabricate(:status)
media_attachment = Fabricate(:media_attachment, status: status)
allow_any_instance_of(Status).to receive(:permitted?).and_return(false)
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(:missing)

60
spec/models/status_spec.rb
View file

@ -119,66 +119,6 @@ RSpec.describe Status, type: :model do
end
end
describe '#permitted?' do
it 'returns true when direct and account is viewer' do
subject.visibility = :direct
expect(subject.permitted?(subject.account)).to be true
end
it 'returns true when direct and viewer is mentioned' do
subject.visibility = :direct
subject.mentions = [Fabricate(:mention, account: alice)]
expect(subject.permitted?(alice)).to be true
end
it 'returns false when direct and viewer is not mentioned' do
viewer = Fabricate(:account)
subject.visibility = :direct
expect(subject.permitted?(viewer)).to be false
end
it 'returns true when private and account is viewer' do
subject.visibility = :direct
expect(subject.permitted?(subject.account)).to be true
end
it 'returns true when private and account is following viewer' do
follow = Fabricate(:follow)
subject.visibility = :private
subject.account = follow.target_account
expect(subject.permitted?(follow.account)).to be true
end
it 'returns true when private and viewer is mentioned' do
subject.visibility = :private
subject.mentions = [Fabricate(:mention, account: alice)]
expect(subject.permitted?(alice)).to be true
end
it 'returns false when private and viewer is not mentioned or followed' do
viewer = Fabricate(:account)
subject.visibility = :private
expect(subject.permitted?(viewer)).to be false
end
it 'returns true when no viewer' do
expect(subject.permitted?).to be true
end
it 'returns false when viewer is blocked' do
block = Fabricate(:block)
subject.visibility = :private
subject.account = block.target_account
expect(subject.permitted?(block.account)).to be false
end
end
describe '#ancestors' do
let!(:alice) { Fabricate(:account, username: 'alice') }
let!(:bob) { Fabricate(:account, username: 'bob', domain: 'example.com') }

70
spec/policies/status_policy_spec.rb Normal file
View file

@ -0,0 +1,70 @@
require 'rails_helper'
require 'pundit/rspec'
RSpec.describe StatusPolicy, type: :model do
subject { described_class }
let(:alice) { Fabricate(:account, username: 'alice') }
let(:status) { Fabricate(:status, account: alice) }
permissions :show? do
it 'grants access when direct and account is viewer' do
status.visibility = :direct
expect(subject).to permit(status.account, status)
end
it 'grants access when direct and viewer is mentioned' do
status.visibility = :direct
status.mentions = [Fabricate(:mention, account: alice)]
expect(subject).to permit(alice, status)
end
it 'denies access when direct and viewer is not mentioned' do
viewer = Fabricate(:account)
status.visibility = :direct
expect(subject).to_not permit(viewer, status)
end
it 'grants access when private and account is viewer' do
status.visibility = :direct
expect(subject).to permit(status.account, status)
end
it 'grants access when private and account is following viewer' do
follow = Fabricate(:follow)
status.visibility = :private
status.account = follow.target_account
expect(subject).to permit(follow.account, status)
end
it 'grants access when private and viewer is mentioned' do
status.visibility = :private
status.mentions = [Fabricate(:mention, account: alice)]
expect(subject).to permit(alice, status)
end
it 'denies access when private and viewer is not mentioned or followed' do
viewer = Fabricate(:account)
status.visibility = :private
expect(subject).to_not permit(viewer, status)
end
it 'grants access when no viewer' do
expect(subject).to permit(nil, status)
end
it 'denies access when viewer is blocked' do
block = Fabricate(:block)
status.visibility = :private
status.account = block.target_account
expect(subject).to_not permit(block.account, status)
end
end
end