From a921eb4e39390eeec6c49c706aad3630b1d50282 Mon Sep 17 00:00:00 2001 From: Fire Demon Date: Thu, 20 Aug 2020 05:52:54 -0500 Subject: [Filtering, Bug] Remove duplicate or unnecessary permission checks, add missing filter_options argument --- app/controllers/activitypub/replies_controller.rb | 1 - .../statuses/favourited_by_accounts_controller.rb | 1 - .../statuses/reblogged_by_accounts_controller.rb | 1 - app/controllers/api/v1/statuses_controller.rb | 1 - app/controllers/remote_interaction_controller.rb | 1 - app/controllers/statuses_controller.rb | 1 - app/lib/feed_manager.rb | 3 +- app/lib/status_filter.rb | 1 + app/policies/status_policy.rb | 39 +++++----------------- 9 files changed, 12 insertions(+), 37 deletions(-) diff --git a/app/controllers/activitypub/replies_controller.rb b/app/controllers/activitypub/replies_controller.rb index 1e1b342b3..4d553fc07 100644 --- a/app/controllers/activitypub/replies_controller.rb +++ b/app/controllers/activitypub/replies_controller.rb @@ -26,7 +26,6 @@ class ActivityPub::RepliesController < ActivityPub::BaseController def set_status @status = @account.statuses.find(params[:status_id]) authorize @status, :show? - authorize @status.reblog, :show? if @status.reblog? rescue Mastodon::NotPermittedError not_found end diff --git a/app/controllers/api/v1/statuses/favourited_by_accounts_controller.rb b/app/controllers/api/v1/statuses/favourited_by_accounts_controller.rb index 45dc212bb..8229786d6 100644 --- a/app/controllers/api/v1/statuses/favourited_by_accounts_controller.rb +++ b/app/controllers/api/v1/statuses/favourited_by_accounts_controller.rb @@ -66,7 +66,6 @@ class Api::V1::Statuses::FavouritedByAccountsController < Api::BaseController def set_status @status = Status.find(params[:status_id]) authorize @status, :show? - authorize @status.reblog, :show? if @status.reblog? rescue Mastodon::NotPermittedError not_found end diff --git a/app/controllers/api/v1/statuses/reblogged_by_accounts_controller.rb b/app/controllers/api/v1/statuses/reblogged_by_accounts_controller.rb index cc8c75ea0..6c9e49d90 100644 --- a/app/controllers/api/v1/statuses/reblogged_by_accounts_controller.rb +++ b/app/controllers/api/v1/statuses/reblogged_by_accounts_controller.rb @@ -63,7 +63,6 @@ class Api::V1::Statuses::RebloggedByAccountsController < Api::BaseController def set_status @status = Status.find(params[:status_id]) authorize @status, :show? - authorize @status.reblog, :show? if @status.reblog? rescue Mastodon::NotPermittedError not_found end diff --git a/app/controllers/api/v1/statuses_controller.rb b/app/controllers/api/v1/statuses_controller.rb index cf21ee90c..d4cdb6eae 100644 --- a/app/controllers/api/v1/statuses_controller.rb +++ b/app/controllers/api/v1/statuses_controller.rb @@ -108,7 +108,6 @@ class Api::V1::StatusesController < Api::BaseController def set_status @status = Status.find(params[:id]) authorize @status, :show? - authorize @status.reblog, :show? if @status.reblog? rescue Mastodon::NotPermittedError not_found end diff --git a/app/controllers/remote_interaction_controller.rb b/app/controllers/remote_interaction_controller.rb index 5db70aac4..5ead3aaa0 100644 --- a/app/controllers/remote_interaction_controller.rb +++ b/app/controllers/remote_interaction_controller.rb @@ -41,7 +41,6 @@ class RemoteInteractionController < ApplicationController def set_status @status = Status.find(params[:id]) authorize @status, :show? - authorize @status.reblog, :show? if @status.reblog? rescue Mastodon::NotPermittedError not_found end diff --git a/app/controllers/statuses_controller.rb b/app/controllers/statuses_controller.rb index 15ea0f38d..6f8e74414 100644 --- a/app/controllers/statuses_controller.rb +++ b/app/controllers/statuses_controller.rb @@ -76,7 +76,6 @@ class StatusesController < ApplicationController def set_status @status = @account.statuses.find(params[:id]) authorize @status, :show? - authorize @status.reblog, :show? if @status.reblog? rescue Mastodon::NotPermittedError not_found end diff --git a/app/lib/feed_manager.rb b/app/lib/feed_manager.rb index 12df1ab88..0ae15db9a 100644 --- a/app/lib/feed_manager.rb +++ b/app/lib/feed_manager.rb @@ -202,9 +202,10 @@ class FeedManager statuses = target_account.statuses.published.without_replies.where(visibility: [:public, :unlisted, :private]).includes(:preloadable_poll, reblog: :account).limit(limit) crutches = build_crutches(account.id, statuses) + filter_options = filter_options_for(account.id) statuses.each do |status| - next if filter_from_home?(status, account.id, crutches) + next if filter_from_home?(status, account.id, crutches, filter_options) add_to_feed(:home, account.id, status, aggregate) end diff --git a/app/lib/status_filter.rb b/app/lib/status_filter.rb index 7555243c0..eb31dcad6 100644 --- a/app/lib/status_filter.rb +++ b/app/lib/status_filter.rb @@ -12,6 +12,7 @@ class StatusFilter def filtered? return false if !account.nil? && account.id == status.account_id + blocked_by_policy? || (account_present? && filtered_status?) || (@filter_silenced && silenced_account?) end diff --git a/app/policies/status_policy.rb b/app/policies/status_policy.rb index 082c27f8e..6d1584ee8 100644 --- a/app/policies/status_policy.rb +++ b/app/policies/status_policy.rb @@ -12,7 +12,7 @@ class StatusPolicy < ApplicationPolicy end def show? - return false if local_only? && (current_account.nil? || !current_account.local?) + return false if local_only? && !current_account&.local? return false unless published? || owned? if requires_mention? @@ -72,12 +72,6 @@ class StatusPolicy < ApplicationPolicy author.domain_blocking?(current_account.domain) end - def parent_author_blocking_domain? - return false if current_account.nil? || current_account.domain.nil? || parent_author.nil? - - parent_author.domain_blocking?(current_account.domain) - end - def conversation_author_blocking_domain? return false if current_account.nil? || current_account.domain.nil? || conversation_owner.nil? @@ -96,12 +90,6 @@ class StatusPolicy < ApplicationPolicy @preloaded_relations[:blocked_by] ? @preloaded_relations[:blocked_by][author.id] : author.blocking?(current_account) end - def parent_author_blocking? - return parent_author&.require_auth? if current_account.nil? || parent_author.nil? - - @preloaded_relations[:blocked_by] ? @preloaded_relations[:blocked_by][parent_author.id] : parent_author.blocking?(current_account) - end - def conversation_author_blocking? return public_conversation? if conversation_owner.nil? @@ -109,10 +97,10 @@ class StatusPolicy < ApplicationPolicy end def blocked_by_owners? - return (author_blocking? || author_blocking_domain?) if conversation_owner&.id == author.id && parent_author&.id == author.id - return true if conversation_author_blocking? || parent_author_blocking? || author_blocking? + return author_blocking? || author_blocking_domain? if conversation_owner&.id == author.id + return true if conversation_author_blocking? || author_blocking? - conversation_author_blocking_domain? || parent_author_blocking_domain? || author_blocking_domain? + conversation_author_blocking_domain? || author_blocking_domain? end def following_author? @@ -121,13 +109,6 @@ class StatusPolicy < ApplicationPolicy @preloaded_relations[:following] ? @preloaded_relations[:following][author.id] : current_account.following?(author) end - def following_parent_author? - return false if current_account.nil? - return true if parent_author.nil? - - @preloaded_relations[:following] ? @preloaded_relations[:following][parent_author.id] : current_account.following?(parent_author) - end - def following_conversation_owner? return false if current_account.nil? return public_conversation? if conversation_owner.nil? @@ -136,19 +117,15 @@ class StatusPolicy < ApplicationPolicy end def following_owners? - return following_author? if conversation_owner&.id == author.id && parent_author&.id == author.id + return following_author? if conversation_owner&.id == author.id - following_conversation_owner? && following_parent_author? && following_author? + following_conversation_owner? && following_author? end def author @author ||= record.account end - def parent_author - @parent_author ||= record.in_reply_to_account - end - def conversation_owner @conversation_owner ||= record.conversation&.account end @@ -162,7 +139,9 @@ class StatusPolicy < ApplicationPolicy end def public_conversation? - @public_conversation ||= record.conversation&.public? || false + return @public_conversation if defined?(@public_conversation) + + @public_conversation = record.conversation&.public? || false end def visibility_for_remote_domain -- cgit