From 988b0493fea7a850130b83d0e81675bda8dd9d8e Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Sun, 3 May 2020 16:30:36 +0200 Subject: Add more tests for ActivityPub controllers (#13585) --- app/controllers/accounts_controller.rb | 14 +++++++------- .../activitypub/collections_controller.rb | 17 ++++++++++------- app/controllers/activitypub/outboxes_controller.rb | 6 +++--- app/controllers/activitypub/replies_controller.rb | 21 +++++++++++++++------ app/controllers/api/v1/polls/votes_controller.rb | 2 +- app/controllers/api/v1/polls_controller.rb | 2 +- .../api/v1/push/subscriptions_controller.rb | 11 ++++++----- app/controllers/api/v1/statuses/mutes_controller.rb | 3 +-- app/controllers/api/v1/statuses_controller.rb | 2 +- app/controllers/media_controller.rb | 2 +- app/controllers/remote_interaction_controller.rb | 2 +- app/controllers/statuses_controller.rb | 2 +- app/models/status.rb | 2 +- 13 files changed, 49 insertions(+), 37 deletions(-) (limited to 'app') diff --git a/app/controllers/accounts_controller.rb b/app/controllers/accounts_controller.rb index 124393d62..b35b2279e 100644 --- a/app/controllers/accounts_controller.rb +++ b/app/controllers/accounts_controller.rb @@ -27,7 +27,7 @@ class AccountsController < ApplicationController end @pinned_statuses = cache_collection(@account.pinned_statuses, Status) if show_pinned_statuses? - @statuses = filtered_status_page(params) + @statuses = filtered_status_page @statuses = cache_collection(@statuses, Status) @rss_url = rss_url @@ -140,12 +140,12 @@ class AccountsController < ApplicationController request.path.split('.').first.ends_with?(Addressable::URI.parse("/tagged/#{params[:tag]}").normalize) end - def filtered_status_page(params) - if params[:min_id].present? - filtered_statuses.paginate_by_min_id(PAGE_SIZE, params[:min_id]).reverse - else - filtered_statuses.paginate_by_max_id(PAGE_SIZE, params[:max_id], params[:since_id]).to_a - end + def filtered_status_page + filtered_statuses.paginate_by_id(PAGE_SIZE, params_slice(:max_id, :min_id, :since_id)) + end + + def params_slice(*keys) + params.slice(*keys).permit(*keys) end def restrict_fields_to diff --git a/app/controllers/activitypub/collections_controller.rb b/app/controllers/activitypub/collections_controller.rb index 910fefb1c..c1e7aa550 100644 --- a/app/controllers/activitypub/collections_controller.rb +++ b/app/controllers/activitypub/collections_controller.rb @@ -24,20 +24,23 @@ class ActivityPub::CollectionsController < ActivityPub::BaseController def set_size case params[:id] when 'featured' - @account.pinned_statuses.count + @size = @account.pinned_statuses.count else - raise ActiveRecord::RecordNotFound + not_found end end def scope_for_collection case params[:id] when 'featured' - return Status.none if @account.blocking?(signed_request_account) - - @account.pinned_statuses - else - raise ActiveRecord::RecordNotFound + # Because in public fetch mode we cache the response, there would be no + # benefit from performing the check below, since a blocked account or domain + # would likely be served the cache from the reverse proxy anyway + if authorized_fetch_mode? && !signed_request_account.nil? && (@account.blocking?(signed_request_account) || (!signed_request_account.domain.nil? && @account.domain_blocking?(signed_request_account.domain))) + Status.none + else + @account.pinned_statuses + end end end diff --git a/app/controllers/activitypub/outboxes_controller.rb b/app/controllers/activitypub/outboxes_controller.rb index 891756b7e..e25a4bc07 100644 --- a/app/controllers/activitypub/outboxes_controller.rb +++ b/app/controllers/activitypub/outboxes_controller.rb @@ -11,7 +11,7 @@ class ActivityPub::OutboxesController < ActivityPub::BaseController before_action :set_cache_headers def show - expires_in(page_requested? ? 0 : 3.minutes, public: public_fetch_mode?) + expires_in(page_requested? ? 0 : 3.minutes, public: public_fetch_mode? && !(signed_request_account.present? && page_requested?)) render json: outbox_presenter, serializer: ActivityPub::OutboxSerializer, adapter: ActivityPub::Adapter, content_type: 'application/activity+json' end @@ -50,12 +50,12 @@ class ActivityPub::OutboxesController < ActivityPub::BaseController return unless page_requested? @statuses = @account.statuses.permitted_for(@account, signed_request_account) - @statuses = params[:min_id].present? ? @statuses.paginate_by_min_id(LIMIT, params[:min_id]).reverse : @statuses.paginate_by_max_id(LIMIT, params[:max_id]) + @statuses = @statuses.paginate_by_id(LIMIT, params_slice(:max_id, :min_id, :since_id)) @statuses = cache_collection(@statuses, Status) end def page_requested? - params[:page] == 'true' + truthy_param?(:page) end def page_params diff --git a/app/controllers/activitypub/replies_controller.rb b/app/controllers/activitypub/replies_controller.rb index c62061555..43bf4e657 100644 --- a/app/controllers/activitypub/replies_controller.rb +++ b/app/controllers/activitypub/replies_controller.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true class ActivityPub::RepliesController < ActivityPub::BaseController - include SignatureAuthentication + include SignatureVerification include Authorization include AccountOwnedConcern @@ -19,15 +19,19 @@ class ActivityPub::RepliesController < ActivityPub::BaseController private + def pundit_user + signed_request_account + end + def set_status @status = @account.statuses.find(params[:status_id]) authorize @status, :show? rescue Mastodon::NotPermittedError - raise ActiveRecord::RecordNotFound + not_found end def set_replies - @replies = page_params[:only_other_accounts] ? Status.where.not(account_id: @account.id) : @account.statuses + @replies = only_other_accounts? ? Status.where.not(account_id: @account.id) : @account.statuses @replies = @replies.where(in_reply_to_id: @status.id, visibility: [:public, :unlisted]) @replies = @replies.paginate_by_min_id(DESCENDANTS_LIMIT, params[:min_id]) end @@ -38,7 +42,7 @@ class ActivityPub::RepliesController < ActivityPub::BaseController type: :unordered, part_of: account_status_replies_url(@account, @status), next: next_page, - items: @replies.map { |status| status.local ? status : status.uri } + items: @replies.map { |status| status.local? ? status : status.uri } ) return page if page_requested? @@ -51,16 +55,21 @@ class ActivityPub::RepliesController < ActivityPub::BaseController end def page_requested? - params[:page] == 'true' + truthy_param?(:page) + end + + def only_other_accounts? + truthy_param?(:only_other_accounts) end def next_page only_other_accounts = !(@replies&.last&.account_id == @account.id && @replies.size == DESCENDANTS_LIMIT) + account_status_replies_url( @account, @status, page: true, - min_id: only_other_accounts && !page_params[:only_other_accounts] ? nil : @replies&.last&.id, + min_id: only_other_accounts && !only_other_accounts? ? nil : @replies&.last&.id, only_other_accounts: only_other_accounts ) end diff --git a/app/controllers/api/v1/polls/votes_controller.rb b/app/controllers/api/v1/polls/votes_controller.rb index e1d26106a..513b937ef 100644 --- a/app/controllers/api/v1/polls/votes_controller.rb +++ b/app/controllers/api/v1/polls/votes_controller.rb @@ -18,7 +18,7 @@ class Api::V1::Polls::VotesController < Api::BaseController @poll = Poll.attached.find(params[:poll_id]) authorize @poll.status, :show? rescue Mastodon::NotPermittedError - raise ActiveRecord::RecordNotFound + not_found end def vote_params diff --git a/app/controllers/api/v1/polls_controller.rb b/app/controllers/api/v1/polls_controller.rb index 744baf7bb..6435e9f0d 100644 --- a/app/controllers/api/v1/polls_controller.rb +++ b/app/controllers/api/v1/polls_controller.rb @@ -17,7 +17,7 @@ class Api::V1::PollsController < Api::BaseController @poll = Poll.attached.find(params[:id]) authorize @poll.status, :show? rescue Mastodon::NotPermittedError - raise ActiveRecord::RecordNotFound + not_found end def refresh_poll diff --git a/app/controllers/api/v1/push/subscriptions_controller.rb b/app/controllers/api/v1/push/subscriptions_controller.rb index 1cbc92b93..d34b333eb 100644 --- a/app/controllers/api/v1/push/subscriptions_controller.rb +++ b/app/controllers/api/v1/push/subscriptions_controller.rb @@ -4,6 +4,7 @@ class Api::V1::Push::SubscriptionsController < Api::BaseController before_action -> { doorkeeper_authorize! :push } before_action :require_user! before_action :set_web_push_subscription + before_action :check_web_push_subscription, only: [:show, :update] def create @web_subscription&.destroy! @@ -21,16 +22,11 @@ class Api::V1::Push::SubscriptionsController < Api::BaseController end def show - raise ActiveRecord::RecordNotFound if @web_subscription.nil? - render json: @web_subscription, serializer: REST::WebPushSubscriptionSerializer end def update - raise ActiveRecord::RecordNotFound if @web_subscription.nil? - @web_subscription.update!(data: data_params) - render json: @web_subscription, serializer: REST::WebPushSubscriptionSerializer end @@ -45,12 +41,17 @@ class Api::V1::Push::SubscriptionsController < Api::BaseController @web_subscription = ::Web::PushSubscription.find_by(access_token_id: doorkeeper_token.id) end + def check_web_push_subscription + not_found if @web_subscription.nil? + end + def subscription_params params.require(:subscription).permit(:endpoint, keys: [:auth, :p256dh]) end def data_params return {} if params[:data].blank? + params.require(:data).permit(alerts: [:follow, :follow_request, :favourite, :reblog, :mention, :poll]) end end diff --git a/app/controllers/api/v1/statuses/mutes_controller.rb b/app/controllers/api/v1/statuses/mutes_controller.rb index 43c7a525a..87071a2b9 100644 --- a/app/controllers/api/v1/statuses/mutes_controller.rb +++ b/app/controllers/api/v1/statuses/mutes_controller.rb @@ -28,8 +28,7 @@ class Api::V1::Statuses::MutesController < Api::BaseController @status = Status.find(params[:status_id]) authorize @status, :show? rescue Mastodon::NotPermittedError - # Reraise in order to get a 404 instead of a 403 error code - raise ActiveRecord::RecordNotFound + not_found end def set_conversation diff --git a/app/controllers/api/v1/statuses_controller.rb b/app/controllers/api/v1/statuses_controller.rb index 93a253cbb..8d6cb84b6 100644 --- a/app/controllers/api/v1/statuses_controller.rb +++ b/app/controllers/api/v1/statuses_controller.rb @@ -67,7 +67,7 @@ class Api::V1::StatusesController < Api::BaseController @status = Status.find(params[:id]) authorize @status, :show? rescue Mastodon::NotPermittedError - raise ActiveRecord::RecordNotFound + not_found end def set_thread diff --git a/app/controllers/media_controller.rb b/app/controllers/media_controller.rb index 05cf09c28..1d166d6e7 100644 --- a/app/controllers/media_controller.rb +++ b/app/controllers/media_controller.rb @@ -33,7 +33,7 @@ class MediaController < ApplicationController def verify_permitted_status! authorize @media_attachment.status, :show? rescue Mastodon::NotPermittedError - raise ActiveRecord::RecordNotFound + not_found end def check_playable diff --git a/app/controllers/remote_interaction_controller.rb b/app/controllers/remote_interaction_controller.rb index 4073e7ac3..3b9202a5c 100644 --- a/app/controllers/remote_interaction_controller.rb +++ b/app/controllers/remote_interaction_controller.rb @@ -41,7 +41,7 @@ class RemoteInteractionController < ApplicationController @status = Status.find(params[:id]) authorize @status, :show? rescue Mastodon::NotPermittedError - raise ActiveRecord::RecordNotFound + not_found end def set_body_classes diff --git a/app/controllers/statuses_controller.rb b/app/controllers/statuses_controller.rb index 4fa128303..d362b97dc 100644 --- a/app/controllers/statuses_controller.rb +++ b/app/controllers/statuses_controller.rb @@ -46,7 +46,7 @@ class StatusesController < ApplicationController end def embed - return not_found if @status.hidden? + return not_found if @status.hidden? || @status.reblog? expires_in 180, public: true response.headers['X-Frame-Options'] = 'ALLOWALL' diff --git a/app/models/status.rb b/app/models/status.rb index fef4e2596..30f86e664 100644 --- a/app/models/status.rb +++ b/app/models/status.rb @@ -354,7 +354,7 @@ class Status < ApplicationRecord if account.nil? where(visibility: visibility) - elsif target_account.blocking?(account) # get rid of blocked peeps + elsif target_account.blocking?(account) || (account.domain.present? && target_account.domain_blocking?(account.domain)) # get rid of blocked peeps none elsif account.id == target_account.id # author can see own stuff all -- cgit