From 2b8dc58b7ff7fb708687c08a75c99b3fb30efc49 Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Mon, 9 May 2022 07:43:08 +0200 Subject: Change RSS feeds (#18356) * Change RSS feeds - Use date and time for titles instead of ellipsized text - Use full content in body, even when there is a content warning - Use media extensions * Change feed icons and add width and height attributes to custom emojis * Fix custom emoji animate on hover breaking * Fix tests --- app/controllers/accounts_controller.rb | 1 - app/controllers/tags_controller.rb | 1 - 2 files changed, 2 deletions(-) (limited to 'app/controllers') diff --git a/app/controllers/accounts_controller.rb b/app/controllers/accounts_controller.rb index ddd38cbb0..fe7d934dc 100644 --- a/app/controllers/accounts_controller.rb +++ b/app/controllers/accounts_controller.rb @@ -44,7 +44,6 @@ class AccountsController < ApplicationController limit = params[:limit].present? ? [params[:limit].to_i, PAGE_SIZE_MAX].min : PAGE_SIZE @statuses = filtered_statuses.without_reblogs.limit(limit) @statuses = cache_collection(@statuses, Status) - render xml: RSS::AccountSerializer.render(@account, @statuses, params[:tag]) end format.json do diff --git a/app/controllers/tags_controller.rb b/app/controllers/tags_controller.rb index 6616ba107..b82da8f0c 100644 --- a/app/controllers/tags_controller.rb +++ b/app/controllers/tags_controller.rb @@ -26,7 +26,6 @@ class TagsController < ApplicationController format.rss do expires_in 0, public: true - render xml: RSS::TagSerializer.render(@tag, @statuses) end format.json do -- cgit From 6cf57c676550068a59149ca82d63fcb5b5431158 Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Fri, 13 May 2022 00:02:35 +0200 Subject: Refactor how Redis locks are created (#18400) * Refactor how Redis locks are created * Fix autorelease duration on account deletion lock --- app/controllers/media_proxy_controller.rb | 17 ++---- app/controllers/settings/exports_controller.rb | 15 ++---- app/lib/activitypub/activity.rb | 17 +----- app/lib/activitypub/activity/announce.rb | 2 +- app/lib/activitypub/activity/create.rb | 4 +- app/lib/activitypub/activity/delete.rb | 6 +-- app/models/account_migration.rb | 13 ++--- app/models/concerns/lockable.rb | 19 +++++++ app/models/concerns/redisable.rb | 8 +-- .../activitypub/process_account_service.rb | 33 +++++------- .../activitypub/process_status_update_service.rb | 46 ++++++---------- app/services/fetch_link_card_service.rb | 15 ++---- app/services/remove_status_service.rb | 61 ++++++++++------------ app/services/resolve_account_service.rb | 13 ++--- app/services/vote_service.rb | 19 +++---- app/workers/distribution_worker.rb | 9 ++-- 16 files changed, 115 insertions(+), 182 deletions(-) create mode 100644 app/models/concerns/lockable.rb (limited to 'app/controllers') diff --git a/app/controllers/media_proxy_controller.rb b/app/controllers/media_proxy_controller.rb index d2a4cb207..3b228722f 100644 --- a/app/controllers/media_proxy_controller.rb +++ b/app/controllers/media_proxy_controller.rb @@ -4,6 +4,7 @@ class MediaProxyController < ApplicationController include RoutingHelper include Authorization include Redisable + include Lockable skip_before_action :store_current_location skip_before_action :require_functional! @@ -16,14 +17,10 @@ class MediaProxyController < ApplicationController rescue_from HTTP::TimeoutError, HTTP::ConnectionError, OpenSSL::SSL::SSLError, with: :internal_server_error def show - RedisLock.acquire(lock_options) do |lock| - if lock.acquired? - @media_attachment = MediaAttachment.remote.attached.find(params[:id]) - authorize @media_attachment.status, :show? - redownload! if @media_attachment.needs_redownload? && !reject_media? - else - raise Mastodon::RaceConditionError - end + with_lock("media_download:#{params[:id]}") do + @media_attachment = MediaAttachment.remote.attached.find(params[:id]) + authorize @media_attachment.status, :show? + redownload! if @media_attachment.needs_redownload? && !reject_media? end redirect_to full_asset_url(@media_attachment.file.url(version)) @@ -45,10 +42,6 @@ class MediaProxyController < ApplicationController end end - def lock_options - { redis: redis, key: "media_download:#{params[:id]}", autorelease: 15.minutes.seconds } - end - def reject_media? DomainBlock.reject_media?(@media_attachment.account.domain) end diff --git a/app/controllers/settings/exports_controller.rb b/app/controllers/settings/exports_controller.rb index 1638d3412..deaa7940e 100644 --- a/app/controllers/settings/exports_controller.rb +++ b/app/controllers/settings/exports_controller.rb @@ -3,6 +3,7 @@ class Settings::ExportsController < Settings::BaseController include Authorization include Redisable + include Lockable skip_before_action :require_functional! @@ -14,21 +15,13 @@ class Settings::ExportsController < Settings::BaseController def create backup = nil - RedisLock.acquire(lock_options) do |lock| - if lock.acquired? - authorize :backup, :create? - backup = current_user.backups.create! - else - raise Mastodon::RaceConditionError - end + with_lock("backup:#{current_user.id}") do + authorize :backup, :create? + backup = current_user.backups.create! end BackupWorker.perform_async(backup.id) redirect_to settings_export_path end - - def lock_options - { redis: redis, key: "backup:#{current_user.id}" } - end end diff --git a/app/lib/activitypub/activity.rb b/app/lib/activitypub/activity.rb index 3c51a7a51..7ff06ea39 100644 --- a/app/lib/activitypub/activity.rb +++ b/app/lib/activitypub/activity.rb @@ -3,6 +3,7 @@ class ActivityPub::Activity include JsonLdHelper include Redisable + include Lockable SUPPORTED_TYPES = %w(Note Question).freeze CONVERTED_TYPES = %w(Image Audio Video Article Page Event).freeze @@ -157,22 +158,6 @@ class ActivityPub::Activity end end - def lock_or_return(key, expire_after = 2.hours.seconds) - yield if redis.set(key, true, nx: true, ex: expire_after) - ensure - redis.del(key) - end - - def lock_or_fail(key, expire_after = 15.minutes.seconds) - RedisLock.acquire({ redis: redis, key: key, autorelease: expire_after }) do |lock| - if lock.acquired? - yield - else - raise Mastodon::RaceConditionError - end - end - end - def fetch? !@options[:delivery] end diff --git a/app/lib/activitypub/activity/announce.rb b/app/lib/activitypub/activity/announce.rb index 0674b1083..0032f13e6 100644 --- a/app/lib/activitypub/activity/announce.rb +++ b/app/lib/activitypub/activity/announce.rb @@ -4,7 +4,7 @@ class ActivityPub::Activity::Announce < ActivityPub::Activity def perform return reject_payload! if delete_arrived_first?(@json['id']) || !related_to_local_activity? - lock_or_fail("announce:#{@object['id']}") do + with_lock("announce:#{@object['id']}") do original_status = status_from_object return reject_payload! if original_status.nil? || !announceable?(original_status) diff --git a/app/lib/activitypub/activity/create.rb b/app/lib/activitypub/activity/create.rb index 1f32d8cce..73882e134 100644 --- a/app/lib/activitypub/activity/create.rb +++ b/app/lib/activitypub/activity/create.rb @@ -47,7 +47,7 @@ class ActivityPub::Activity::Create < ActivityPub::Activity def create_status return reject_payload! if unsupported_object_type? || invalid_origin?(object_uri) || tombstone_exists? || !related_to_local_activity? - lock_or_fail("create:#{object_uri}") do + with_lock("create:#{object_uri}") do return if delete_arrived_first?(object_uri) || poll_vote? @status = find_existing_status @@ -315,7 +315,7 @@ class ActivityPub::Activity::Create < ActivityPub::Activity poll = replied_to_status.preloadable_poll already_voted = true - lock_or_fail("vote:#{replied_to_status.poll_id}:#{@account.id}") do + with_lock("vote:#{replied_to_status.poll_id}:#{@account.id}") do already_voted = poll.votes.where(account: @account).exists? poll.votes.create!(account: @account, choice: poll.options.index(@object['name']), uri: object_uri) end diff --git a/app/lib/activitypub/activity/delete.rb b/app/lib/activitypub/activity/delete.rb index f5ef863f3..871eb3966 100644 --- a/app/lib/activitypub/activity/delete.rb +++ b/app/lib/activitypub/activity/delete.rb @@ -12,7 +12,7 @@ class ActivityPub::Activity::Delete < ActivityPub::Activity private def delete_person - lock_or_return("delete_in_progress:#{@account.id}") do + with_lock("delete_in_progress:#{@account.id}", autorelease: 2.hours, raise_on_failure: false) do DeleteAccountService.new.call(@account, reserve_username: false, skip_activitypub: true) end end @@ -20,14 +20,14 @@ class ActivityPub::Activity::Delete < ActivityPub::Activity def delete_note return if object_uri.nil? - lock_or_return("delete_status_in_progress:#{object_uri}", 5.minutes.seconds) do + with_lock("delete_status_in_progress:#{object_uri}", raise_on_failure: false) do unless invalid_origin?(object_uri) # This lock ensures a concurrent `ActivityPub::Activity::Create` either # does not create a status at all, or has finished saving it to the # database before we try to load it. # Without the lock, `delete_later!` could be called after `delete_arrived_first?` # and `Status.find` before `Status.create!` - lock_or_fail("create:#{object_uri}") { delete_later!(object_uri) } + with_lock("create:#{object_uri}") { delete_later!(object_uri) } Tombstone.find_or_create_by(uri: object_uri, account: @account) end diff --git a/app/models/account_migration.rb b/app/models/account_migration.rb index ded32c9c6..06291c9f3 100644 --- a/app/models/account_migration.rb +++ b/app/models/account_migration.rb @@ -15,6 +15,7 @@ class AccountMigration < ApplicationRecord include Redisable + include Lockable COOLDOWN_PERIOD = 30.days.freeze @@ -41,12 +42,8 @@ class AccountMigration < ApplicationRecord return false unless errors.empty? - RedisLock.acquire(lock_options) do |lock| - if lock.acquired? - save - else - raise Mastodon::RaceConditionError - end + with_lock("account_migration:#{account.id}") do + save end end @@ -83,8 +80,4 @@ class AccountMigration < ApplicationRecord def validate_migration_cooldown errors.add(:base, I18n.t('migrations.errors.on_cooldown')) if account.migrations.within_cooldown.exists? end - - def lock_options - { redis: redis, key: "account_migration:#{account.id}" } - end end diff --git a/app/models/concerns/lockable.rb b/app/models/concerns/lockable.rb new file mode 100644 index 000000000..55a9714ca --- /dev/null +++ b/app/models/concerns/lockable.rb @@ -0,0 +1,19 @@ +# frozen_string_literal: true + +module Lockable + # @param [String] lock_name + # @param [ActiveSupport::Duration] autorelease Automatically release the lock after this time + # @param [Boolean] raise_on_failure Raise an error if a lock cannot be acquired, or fail silently + # @raise [Mastodon::RaceConditionError] + def with_lock(lock_name, autorelease: 15.minutes, raise_on_failure: true) + with_redis do |redis| + RedisLock.acquire(redis: redis, key: "lock:#{lock_name}", autorelease: autorelease.seconds) do |lock| + if lock.acquired? + yield + elsif raise_on_failure + raise Mastodon::RaceConditionError, "Could not acquire lock for #{lock_name}, try again later" + end + end + end + end +end diff --git a/app/models/concerns/redisable.rb b/app/models/concerns/redisable.rb index 8d76b6b82..0dad3abb2 100644 --- a/app/models/concerns/redisable.rb +++ b/app/models/concerns/redisable.rb @@ -1,11 +1,11 @@ # frozen_string_literal: true module Redisable - extend ActiveSupport::Concern - - private - def redis Thread.current[:redis] ||= RedisConfiguration.pool.checkout end + + def with_redis(&block) + RedisConfiguration.with(&block) + end end diff --git a/app/services/activitypub/process_account_service.rb b/app/services/activitypub/process_account_service.rb index 5649153ee..4449a5427 100644 --- a/app/services/activitypub/process_account_service.rb +++ b/app/services/activitypub/process_account_service.rb @@ -4,6 +4,7 @@ class ActivityPub::ProcessAccountService < BaseService include JsonLdHelper include DomainControlHelper include Redisable + include Lockable # Should be called with confirmed valid JSON # and WebFinger-resolved username and domain @@ -17,22 +18,18 @@ class ActivityPub::ProcessAccountService < BaseService @domain = domain @collections = {} - RedisLock.acquire(lock_options) do |lock| - if lock.acquired? - @account = Account.remote.find_by(uri: @uri) if @options[:only_key] - @account ||= Account.find_remote(@username, @domain) - @old_public_key = @account&.public_key - @old_protocol = @account&.protocol - @suspension_changed = false - - create_account if @account.nil? - update_account - process_tags - - process_duplicate_accounts! if @options[:verified_webfinger] - else - raise Mastodon::RaceConditionError - end + with_lock("process_account:#{@uri}") do + @account = Account.remote.find_by(uri: @uri) if @options[:only_key] + @account ||= Account.find_remote(@username, @domain) + @old_public_key = @account&.public_key + @old_protocol = @account&.protocol + @suspension_changed = false + + create_account if @account.nil? + update_account + process_tags + + process_duplicate_accounts! if @options[:verified_webfinger] end return if @account.nil? @@ -289,10 +286,6 @@ class ActivityPub::ProcessAccountService < BaseService !@old_protocol.nil? && @old_protocol != @account.protocol end - def lock_options - { redis: redis, key: "process_account:#{@uri}", autorelease: 15.minutes.seconds } - end - def process_tags return if @json['tag'].blank? diff --git a/app/services/activitypub/process_status_update_service.rb b/app/services/activitypub/process_status_update_service.rb index fb6e44c6d..addd5fc27 100644 --- a/app/services/activitypub/process_status_update_service.rb +++ b/app/services/activitypub/process_status_update_service.rb @@ -3,6 +3,7 @@ class ActivityPub::ProcessStatusUpdateService < BaseService include JsonLdHelper include Redisable + include Lockable def call(status, json) raise ArgumentError, 'Status has unsaved changes' if status.changed? @@ -33,41 +34,32 @@ class ActivityPub::ProcessStatusUpdateService < BaseService last_edit_date = @status.edited_at.presence || @status.created_at # Only allow processing one create/update per status at a time - RedisLock.acquire(lock_options) do |lock| - if lock.acquired? - Status.transaction do - record_previous_edit! - update_media_attachments! - update_poll! - update_immediate_attributes! - update_metadata! - create_edits! - end + with_lock("create:#{@uri}") do + Status.transaction do + record_previous_edit! + update_media_attachments! + update_poll! + update_immediate_attributes! + update_metadata! + create_edits! + end - queue_poll_notifications! + queue_poll_notifications! - next unless significant_changes? + next unless significant_changes? - reset_preview_card! - broadcast_updates! - else - raise Mastodon::RaceConditionError - end + reset_preview_card! + broadcast_updates! end forward_activity! if significant_changes? && @status_parser.edited_at > last_edit_date end def handle_implicit_update! - RedisLock.acquire(lock_options) do |lock| - if lock.acquired? - update_poll!(allow_significant_changes: false) - else - raise Mastodon::RaceConditionError - end + with_lock("create:#{@uri}") do + update_poll!(allow_significant_changes: false) + queue_poll_notifications! end - - queue_poll_notifications! end def update_media_attachments! @@ -241,10 +233,6 @@ class ActivityPub::ProcessStatusUpdateService < BaseService equals_or_includes_any?(@json['type'], %w(Note Question)) end - def lock_options - { redis: redis, key: "create:#{@uri}", autorelease: 15.minutes.seconds } - end - def record_previous_edit! @previous_edit = @status.build_snapshot(at_time: @status.created_at, rate_limit: false) if @status.edits.empty? end diff --git a/app/services/fetch_link_card_service.rb b/app/services/fetch_link_card_service.rb index 868796a6b..e5b5b730e 100644 --- a/app/services/fetch_link_card_service.rb +++ b/app/services/fetch_link_card_service.rb @@ -2,6 +2,7 @@ class FetchLinkCardService < BaseService include Redisable + include Lockable URL_PATTERN = %r{ (#{Twitter::TwitterText::Regex[:valid_url_preceding_chars]}) # $1 preceding chars @@ -22,13 +23,9 @@ class FetchLinkCardService < BaseService @url = @original_url.to_s - RedisLock.acquire(lock_options) do |lock| - if lock.acquired? - @card = PreviewCard.find_by(url: @url) - process_url if @card.nil? || @card.updated_at <= 2.weeks.ago || @card.missing_image? - else - raise Mastodon::RaceConditionError - end + with_lock("fetch:#{@original_url}") do + @card = PreviewCard.find_by(url: @url) + process_url if @card.nil? || @card.updated_at <= 2.weeks.ago || @card.missing_image? end attach_card if @card&.persisted? @@ -155,8 +152,4 @@ class FetchLinkCardService < BaseService @card.assign_attributes(link_details_extractor.to_preview_card_attributes) @card.save_with_optional_image! unless @card.title.blank? && @card.html.blank? end - - def lock_options - { redis: redis, key: "fetch:#{@original_url}", autorelease: 15.minutes.seconds } - end end diff --git a/app/services/remove_status_service.rb b/app/services/remove_status_service.rb index dbd1f6430..8dc521eed 100644 --- a/app/services/remove_status_service.rb +++ b/app/services/remove_status_service.rb @@ -3,6 +3,7 @@ class RemoveStatusService < BaseService include Redisable include Payloadable + include Lockable # Delete a status # @param [Status] status @@ -17,37 +18,33 @@ class RemoveStatusService < BaseService @account = status.account @options = options - RedisLock.acquire(lock_options) do |lock| - if lock.acquired? - @status.discard - - remove_from_self if @account.local? - remove_from_followers - remove_from_lists - - # There is no reason to send out Undo activities when the - # cause is that the original object has been removed, since - # original object being removed implicitly removes reblogs - # of it. The Delete activity of the original is forwarded - # separately. - remove_from_remote_reach if @account.local? && !@options[:original_removed] - - # Since reblogs don't mention anyone, don't get reblogged, - # favourited and don't contain their own media attachments - # or hashtags, this can be skipped - unless @status.reblog? - remove_from_mentions - remove_reblogs - remove_from_hashtags - remove_from_public - remove_from_media if @status.with_media? - remove_media - end - - @status.destroy! if permanently? - else - raise Mastodon::RaceConditionError + with_lock("distribute:#{@status.id}") do + @status.discard + + remove_from_self if @account.local? + remove_from_followers + remove_from_lists + + # There is no reason to send out Undo activities when the + # cause is that the original object has been removed, since + # original object being removed implicitly removes reblogs + # of it. The Delete activity of the original is forwarded + # separately. + remove_from_remote_reach if @account.local? && !@options[:original_removed] + + # Since reblogs don't mention anyone, don't get reblogged, + # favourited and don't contain their own media attachments + # or hashtags, this can be skipped + unless @status.reblog? + remove_from_mentions + remove_reblogs + remove_from_hashtags + remove_from_public + remove_from_media if @status.with_media? + remove_media end + + @status.destroy! if permanently? end end @@ -144,8 +141,4 @@ class RemoveStatusService < BaseService def permanently? @options[:immediate] || !(@options[:preserve] || @status.reported?) end - - def lock_options - { redis: redis, key: "distribute:#{@status.id}", autorelease: 5.minutes.seconds } - end end diff --git a/app/services/resolve_account_service.rb b/app/services/resolve_account_service.rb index 387e2e09b..b55e45409 100644 --- a/app/services/resolve_account_service.rb +++ b/app/services/resolve_account_service.rb @@ -5,6 +5,7 @@ class ResolveAccountService < BaseService include DomainControlHelper include WebfingerHelper include Redisable + include Lockable # Find or create an account record for a remote user. When creating, # look up the user's webfinger and fetch ActivityPub data @@ -108,12 +109,8 @@ class ResolveAccountService < BaseService def fetch_account! return unless activitypub_ready? - RedisLock.acquire(lock_options) do |lock| - if lock.acquired? - @account = ActivityPub::FetchRemoteAccountService.new.call(actor_url) - else - raise Mastodon::RaceConditionError - end + with_lock("resolve:#{@username}@#{@domain}") do + @account = ActivityPub::FetchRemoteAccountService.new.call(actor_url) end @account @@ -146,8 +143,4 @@ class ResolveAccountService < BaseService @account.suspend!(origin: :remote) AccountDeletionWorker.perform_async(@account.id, { 'reserve_username' => false, 'skip_activitypub' => true }) end - - def lock_options - { redis: redis, key: "resolve:#{@username}@#{@domain}", autorelease: 15.minutes.seconds } - end end diff --git a/app/services/vote_service.rb b/app/services/vote_service.rb index b77812970..ccd04dbfc 100644 --- a/app/services/vote_service.rb +++ b/app/services/vote_service.rb @@ -4,6 +4,7 @@ class VoteService < BaseService include Authorization include Payloadable include Redisable + include Lockable def call(account, poll, choices) authorize_with account, poll, :vote? @@ -15,17 +16,13 @@ class VoteService < BaseService already_voted = true - RedisLock.acquire(lock_options) do |lock| - if lock.acquired? - already_voted = @poll.votes.where(account: @account).exists? + with_lock("vote:#{@poll.id}:#{@account.id}") do + already_voted = @poll.votes.where(account: @account).exists? - ApplicationRecord.transaction do - @choices.each do |choice| - @votes << @poll.votes.create!(account: @account, choice: Integer(choice)) - end + ApplicationRecord.transaction do + @choices.each do |choice| + @votes << @poll.votes.create!(account: @account, choice: Integer(choice)) end - else - raise Mastodon::RaceConditionError end end @@ -76,8 +73,4 @@ class VoteService < BaseService @poll.reload retry end - - def lock_options - { redis: redis, key: "vote:#{@poll.id}:#{@account.id}" } - end end diff --git a/app/workers/distribution_worker.rb b/app/workers/distribution_worker.rb index 474b4daaf..59cdbc7b2 100644 --- a/app/workers/distribution_worker.rb +++ b/app/workers/distribution_worker.rb @@ -3,14 +3,11 @@ class DistributionWorker include Sidekiq::Worker include Redisable + include Lockable def perform(status_id, options = {}) - RedisLock.acquire(redis: redis, key: "distribute:#{status_id}", autorelease: 5.minutes.seconds) do |lock| - if lock.acquired? - FanOutOnWriteService.new.call(Status.find(status_id), **options.symbolize_keys) - else - raise Mastodon::RaceConditionError - end + with_lock("distribute:#{status_id}") do + FanOutOnWriteService.new.call(Status.find(status_id), **options.symbolize_keys) end rescue ActiveRecord::RecordNotFound true -- cgit From 94e98864e39c010635e839fea984f2b4893bef1a Mon Sep 17 00:00:00 2001 From: Levi Bard Date: Mon, 16 May 2022 09:29:01 +0200 Subject: Allow import/export of instance-level domain blocks/allows (#1754) * Allow import/export of instance-level domain blocks/allows. Fixes #15095 * Pacify circleci * Address simple code review feedback * Add headers to exported CSV * Extract common import/export functionality to AdminExportControllerConcern * Add additional fields to instance-blocked domain export * Address review feedback * Split instance domain block/allow import/export into separate pages/controllers * Address code review feedback * Pacify DeepSource * Work around Paperclip::HasAttachmentFile for Rails 6 * Fix deprecated API warning in export tests * Remove after_commit workaround --- .../admin/export_domain_allows_controller.rb | 60 +++++++++++++++++++ .../admin/export_domain_blocks_controller.rb | 68 ++++++++++++++++++++++ .../concerns/admin_export_controller_concern.rb | 39 +++++++++++++ app/models/admin/import.rb | 29 +++++++++ app/models/domain_allow.rb | 4 ++ app/validators/admin_import_validator.rb | 19 ++++++ app/views/admin/export_domain_allows/new.html.haml | 10 ++++ app/views/admin/export_domain_blocks/new.html.haml | 10 ++++ app/views/admin/instances/index.html.haml | 4 ++ config/locales/en.yml | 12 ++++ config/routes.rb | 16 ++++- .../admin/domain_allows_controller_spec.rb | 48 +++++++++++++++ .../admin/export_domain_allows_controller_spec.rb | 42 +++++++++++++ .../admin/export_domain_blocks_controller_spec.rb | 47 +++++++++++++++ spec/fixtures/files/domain_allows.csv | 3 + spec/fixtures/files/domain_blocks.csv | 4 ++ 16 files changed, 414 insertions(+), 1 deletion(-) create mode 100644 app/controllers/admin/export_domain_allows_controller.rb create mode 100644 app/controllers/admin/export_domain_blocks_controller.rb create mode 100644 app/controllers/concerns/admin_export_controller_concern.rb create mode 100644 app/models/admin/import.rb create mode 100644 app/validators/admin_import_validator.rb create mode 100644 app/views/admin/export_domain_allows/new.html.haml create mode 100644 app/views/admin/export_domain_blocks/new.html.haml create mode 100644 spec/controllers/admin/domain_allows_controller_spec.rb create mode 100644 spec/controllers/admin/export_domain_allows_controller_spec.rb create mode 100644 spec/controllers/admin/export_domain_blocks_controller_spec.rb create mode 100644 spec/fixtures/files/domain_allows.csv create mode 100644 spec/fixtures/files/domain_blocks.csv (limited to 'app/controllers') diff --git a/app/controllers/admin/export_domain_allows_controller.rb b/app/controllers/admin/export_domain_allows_controller.rb new file mode 100644 index 000000000..eb2955ac3 --- /dev/null +++ b/app/controllers/admin/export_domain_allows_controller.rb @@ -0,0 +1,60 @@ +# frozen_string_literal: true + +require 'csv' + +module Admin + class ExportDomainAllowsController < BaseController + include AdminExportControllerConcern + + before_action :set_dummy_import!, only: [:new] + + ROWS_PROCESSING_LIMIT = 20_000 + + def new + authorize :domain_allow, :create? + end + + def export + authorize :instance, :index? + send_export_file + end + + def import + authorize :domain_allow, :create? + begin + @import = Admin::Import.new(import_params) + parse_import_data!(export_headers) + + @data.take(ROWS_PROCESSING_LIMIT).each do |row| + domain = row['#domain'].strip + next if DomainAllow.allowed?(domain) + + domain_allow = DomainAllow.new(domain: domain) + log_action :create, domain_allow if domain_allow.save + end + flash[:notice] = I18n.t('admin.domain_allows.created_msg') + rescue ActionController::ParameterMissing + flash[:error] = I18n.t('admin.export_domain_allows.no_file') + end + redirect_to admin_instances_path + end + + private + + def export_filename + 'domain_allows.csv' + end + + def export_headers + %w(#domain) + end + + def export_data + CSV.generate(headers: export_headers, write_headers: true) do |content| + DomainAllow.allowed_domains.each do |instance| + content << [instance.domain] + end + end + end + end +end diff --git a/app/controllers/admin/export_domain_blocks_controller.rb b/app/controllers/admin/export_domain_blocks_controller.rb new file mode 100644 index 000000000..0ad5b92b5 --- /dev/null +++ b/app/controllers/admin/export_domain_blocks_controller.rb @@ -0,0 +1,68 @@ +# frozen_string_literal: true + +require 'csv' + +module Admin + class ExportDomainBlocksController < BaseController + include AdminExportControllerConcern + + before_action :set_dummy_import!, only: [:new] + + ROWS_PROCESSING_LIMIT = 20_000 + + def new + authorize :domain_block, :create? + end + + def export + authorize :instance, :index? + send_export_file + end + + def import + authorize :domain_block, :create? + begin + @import = Admin::Import.new(import_params) + parse_import_data!(export_headers) + + @data.take(ROWS_PROCESSING_LIMIT).each do |row| + domain = row['#domain'].strip + next if DomainBlock.rule_for(domain).present? + + domain_block = DomainBlock.new(domain: domain, + severity: row['#severity'].strip, + reject_media: row['#reject_media'].strip, + reject_reports: row['#reject_reports'].strip, + public_comment: row['#public_comment'].strip, + obfuscate: row['#obfuscate'].strip) + if domain_block.save + DomainBlockWorker.perform_async(domain_block.id) + log_action :create, domain_block + end + end + flash[:notice] = I18n.t('admin.domain_blocks.created_msg') + rescue ActionController::ParameterMissing + flash[:error] = I18n.t('admin.export_domain_blocks.no_file') + end + redirect_to admin_instances_path(limited: '1') + end + + private + + def export_filename + 'domain_blocks.csv' + end + + def export_headers + %w(#domain #severity #reject_media #reject_reports #public_comment #obfuscate) + end + + def export_data + CSV.generate(headers: export_headers, write_headers: true) do |content| + DomainBlock.with_user_facing_limitations.each do |instance| + content << [instance.domain, instance.severity, instance.reject_media, instance.reject_reports, instance.public_comment, instance.obfuscate] + end + end + end + end +end diff --git a/app/controllers/concerns/admin_export_controller_concern.rb b/app/controllers/concerns/admin_export_controller_concern.rb new file mode 100644 index 000000000..013915d02 --- /dev/null +++ b/app/controllers/concerns/admin_export_controller_concern.rb @@ -0,0 +1,39 @@ +# frozen_string_literal: true + +module AdminExportControllerConcern + extend ActiveSupport::Concern + + private + + def send_export_file + respond_to do |format| + format.csv { send_data export_data, filename: export_filename } + end + end + + def export_data + raise 'Override in controller' + end + + def export_filename + raise 'Override in controller' + end + + def set_dummy_import! + @import = Admin::Import.new + end + + def import_params + params.require(:admin_import).permit(:data) + end + + def import_data + Paperclip.io_adapters.for(@import.data).read + end + + def parse_import_data!(default_headers) + data = CSV.parse(import_data, headers: true) + data = CSV.parse(import_data, headers: default_headers) unless data.headers&.first&.strip&.include?(default_headers[0]) + @data = data.reject(&:blank?) + end +end diff --git a/app/models/admin/import.rb b/app/models/admin/import.rb new file mode 100644 index 000000000..c305be237 --- /dev/null +++ b/app/models/admin/import.rb @@ -0,0 +1,29 @@ +# frozen_string_literal: true + +# A non-activerecord helper class for csv upload +class Admin::Import + extend ActiveModel::Callbacks + include ActiveModel::Model + include Paperclip::Glue + + FILE_TYPES = %w(text/plain text/csv application/csv).freeze + + # Paperclip required callbacks + define_model_callbacks :save, only: [:after] + define_model_callbacks :destroy, only: [:before, :after] + + attr_accessor :data_file_name, :data_content_type + + has_attached_file :data + validates_attachment_content_type :data, content_type: FILE_TYPES + validates_attachment_presence :data + validates_with AdminImportValidator, on: :create + + def save + run_callbacks :save + end + + def destroy + run_callbacks :destroy + end +end diff --git a/app/models/domain_allow.rb b/app/models/domain_allow.rb index 4b0a89c18..2e14fce25 100644 --- a/app/models/domain_allow.rb +++ b/app/models/domain_allow.rb @@ -23,6 +23,10 @@ class DomainAllow < ApplicationRecord !rule_for(domain).nil? end + def allowed_domains + select(:domain) + end + def rule_for(domain) return if domain.blank? diff --git a/app/validators/admin_import_validator.rb b/app/validators/admin_import_validator.rb new file mode 100644 index 000000000..338ceb3a7 --- /dev/null +++ b/app/validators/admin_import_validator.rb @@ -0,0 +1,19 @@ +# frozen_string_literal: true + +class AdminImportValidator < ActiveModel::Validator + FIRST_HEADER = '#domain' + + def validate(import) + return if import.type.blank? || import.data.blank? + + # We parse because newlines could be part of individual rows. This + # runs on create so we should be reading the local file here before + # it is uploaded to object storage or moved anywhere... + csv_data = CSV.parse(import.data.queued_for_write[:original].read) + + row_count = csv_data.size + row_count -= 1 if csv_data.first&.first == FIRST_HEADER + + import.errors.add(:data, I18n.t('imports.errors.over_rows_processing_limit', count: Admin::DomainBlocksController::ROWS_PROCESSING_LIMIT)) if row_count > Admin::DomainBlocksController::ROWS_PROCESSING_LIMIT + end +end diff --git a/app/views/admin/export_domain_allows/new.html.haml b/app/views/admin/export_domain_allows/new.html.haml new file mode 100644 index 000000000..dc0cf8c52 --- /dev/null +++ b/app/views/admin/export_domain_allows/new.html.haml @@ -0,0 +1,10 @@ +- content_for :page_title do + = t('.title') + += simple_form_for @import, url: import_admin_export_domain_allows_path, html: { multipart: true } do |f| + .fields-row + .fields-group.fields-row__column.fields-row__column-6 + = f.input :data, wrapper: :with_block_label, hint: t('simple_form.hints.imports.data'), as: :file + + .actions + = f.button :button, t('imports.upload'), type: :submit diff --git a/app/views/admin/export_domain_blocks/new.html.haml b/app/views/admin/export_domain_blocks/new.html.haml new file mode 100644 index 000000000..0291aeed7 --- /dev/null +++ b/app/views/admin/export_domain_blocks/new.html.haml @@ -0,0 +1,10 @@ +- content_for :page_title do + = t('.title') + += simple_form_for @import, url: import_admin_export_domain_blocks_path, html: { multipart: true } do |f| + .fields-row + .fields-group.fields-row__column.fields-row__column-6 + = f.input :data, wrapper: :with_block_label, hint: t('simple_form.hints.imports.data'), as: :file + + .actions + = f.button :button, t('imports.upload'), type: :submit diff --git a/app/views/admin/instances/index.html.haml b/app/views/admin/instances/index.html.haml index ee1b3d0ce..abb2d8c0e 100644 --- a/app/views/admin/instances/index.html.haml +++ b/app/views/admin/instances/index.html.haml @@ -4,8 +4,12 @@ - content_for :heading_actions do - if whitelist_mode? = link_to t('admin.domain_allows.add_new'), new_admin_domain_allow_path, class: 'button', id: 'add-instance-button' + = link_to t('admin.domain_allows.export'), export_admin_export_domain_allows_path(format: :csv), class: 'button' + = link_to t('admin.domain_allows.import'), new_admin_export_domain_allow_path, class: 'button' - else = link_to t('admin.domain_blocks.add_new'), new_admin_domain_block_path, class: 'button', id: 'add-instance-button' + = link_to t('admin.domain_blocks.export'), export_admin_export_domain_blocks_path(format: :csv), class: 'button' + = link_to t('admin.domain_blocks.import'), new_admin_export_domain_block_path, class: 'button' .filters .filter-subset diff --git a/config/locales/en.yml b/config/locales/en.yml index 50e762db7..583683bff 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -421,6 +421,8 @@ en: add_new: Allow federation with domain created_msg: Domain has been successfully allowed for federation destroyed_msg: Domain has been disallowed from federation + export: Export + import: Import undo: Disallow federation with domain domain_blocks: add_new: Add new domain block @@ -429,6 +431,8 @@ en: domain: Domain edit: Edit domain block existing_domain_block_html: You have already imposed stricter limits on %{name}, you need to unblock it first. + export: Export + import: Import new: create: Create block hint: The domain block will not prevent creation of account entries in the database, but will retroactively and automatically apply specific moderation methods on those accounts. @@ -469,6 +473,14 @@ en: resolved_dns_records_hint_html: The domain name resolves to the following MX domains, which are ultimately responsible for accepting e-mail. Blocking an MX domain will block sign-ups from any e-mail address which uses the same MX domain, even if the visible domain name is different. Be careful not to block major e-mail providers. resolved_through_html: Resolved through %{domain} title: Blocked e-mail domains + export_domain_allows: + new: + title: Import domain allows + no_file: No file selected + export_domain_blocks: + new: + title: Import domain blocks + no_file: No file selected follow_recommendations: description_html: "Follow recommendations help new users quickly find interesting content. When a user has not interacted with others enough to form personalized follow recommendations, these accounts are recommended instead. They are re-calculated on a daily basis from a mix of accounts with the highest recent engagements and highest local follower counts for a given language." language: For language diff --git a/config/routes.rb b/config/routes.rb index 574715705..787665192 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -194,7 +194,21 @@ Rails.application.routes.draw do get '/dashboard', to: 'dashboard#index' resources :domain_allows, only: [:new, :create, :show, :destroy] - resources :domain_blocks, only: [:new, :create, :destroy, :update, :edit] + resources :domain_blocks, only: [:new, :create, :show, :destroy, :update, :edit] + + resources :export_domain_allows, only: [:new] do + collection do + get :export, constraints: { format: :csv } + post :import + end + end + + resources :export_domain_blocks, only: [:new] do + collection do + get :export, constraints: { format: :csv } + post :import + end + end resources :email_domain_blocks, only: [:index, :new, :create] do collection do diff --git a/spec/controllers/admin/domain_allows_controller_spec.rb b/spec/controllers/admin/domain_allows_controller_spec.rb new file mode 100644 index 000000000..8bacdd3e4 --- /dev/null +++ b/spec/controllers/admin/domain_allows_controller_spec.rb @@ -0,0 +1,48 @@ +require 'rails_helper' + +RSpec.describe Admin::DomainAllowsController, type: :controller do + render_views + + before do + sign_in Fabricate(:user, admin: true), scope: :user + end + + describe 'GET #new' do + it 'assigns a new domain allow' do + get :new + + expect(assigns(:domain_allow)).to be_instance_of(DomainAllow) + expect(response).to have_http_status(200) + end + end + + describe 'POST #create' do + it 'blocks the domain when succeeded to save' do + post :create, params: { domain_allow: { domain: 'example.com' } } + + expect(flash[:notice]).to eq I18n.t('admin.domain_allows.created_msg') + expect(response).to redirect_to(admin_instances_path) + end + + it 'renders new when failed to save' do + Fabricate(:domain_allow, domain: 'example.com') + + post :create, params: { domain_allow: { domain: 'example.com' } } + + expect(response).to render_template :new + end + end + + describe 'DELETE #destroy' do + it 'disallows the domain' do + service = double(call: true) + allow(UnallowDomainService).to receive(:new).and_return(service) + domain_allow = Fabricate(:domain_allow) + delete :destroy, params: { id: domain_allow.id } + + expect(service).to have_received(:call).with(domain_allow) + expect(flash[:notice]).to eq I18n.t('admin.domain_allows.destroyed_msg') + expect(response).to redirect_to(admin_instances_path) + end + end +end diff --git a/spec/controllers/admin/export_domain_allows_controller_spec.rb b/spec/controllers/admin/export_domain_allows_controller_spec.rb new file mode 100644 index 000000000..f6275c2d6 --- /dev/null +++ b/spec/controllers/admin/export_domain_allows_controller_spec.rb @@ -0,0 +1,42 @@ +require 'rails_helper' + +RSpec.describe Admin::ExportDomainAllowsController, type: :controller do + render_views + + before do + sign_in Fabricate(:user, admin: true), scope: :user + end + + describe 'GET #export' do + it 'renders instances' do + Fabricate(:domain_allow, domain: 'good.domain') + Fabricate(:domain_allow, domain: 'better.domain') + + get :export, params: { format: :csv } + expect(response).to have_http_status(200) + expect(response.body).to eq(IO.read(File.join(file_fixture_path, 'domain_allows.csv'))) + end + end + + describe 'POST #import' do + it 'allows imported domains' do + post :import, params: { admin_import: { data: fixture_file_upload('domain_allows.csv') } } + + expect(response).to redirect_to(admin_instances_path) + + # Header should not be imported + expect(DomainAllow.where(domain: '#domain').present?).to eq(false) + + # Domains should now be added + get :export, params: { format: :csv } + expect(response).to have_http_status(200) + expect(response.body).to eq(IO.read(File.join(file_fixture_path, 'domain_allows.csv'))) + end + + it 'displays error on no file selected' do + post :import, params: { admin_import: {} } + expect(response).to redirect_to(admin_instances_path) + expect(flash[:error]).to eq(I18n.t('admin.export_domain_allows.no_file')) + end + end +end diff --git a/spec/controllers/admin/export_domain_blocks_controller_spec.rb b/spec/controllers/admin/export_domain_blocks_controller_spec.rb new file mode 100644 index 000000000..0cb221972 --- /dev/null +++ b/spec/controllers/admin/export_domain_blocks_controller_spec.rb @@ -0,0 +1,47 @@ +require 'rails_helper' + +RSpec.describe Admin::ExportDomainBlocksController, type: :controller do + render_views + + before do + sign_in Fabricate(:user, admin: true), scope: :user + end + + describe 'GET #export' do + it 'renders instances' do + Fabricate(:domain_block, domain: 'bad.domain', severity: 'silence', public_comment: 'bad') + Fabricate(:domain_block, domain: 'worse.domain', severity: 'suspend', reject_media: true, reject_reports: true, public_comment: 'worse', obfuscate: true) + Fabricate(:domain_block, domain: 'reject.media', severity: 'noop', reject_media: true, public_comment: 'reject media') + Fabricate(:domain_block, domain: 'no.op', severity: 'noop', public_comment: 'noop') + + get :export, params: { format: :csv } + expect(response).to have_http_status(200) + expect(response.body).to eq(IO.read(File.join(file_fixture_path, 'domain_blocks.csv'))) + end + end + + describe 'POST #import' do + it 'blocks imported domains' do + allow(DomainBlockWorker).to receive(:perform_async).and_return(true) + + post :import, params: { admin_import: { data: fixture_file_upload('domain_blocks.csv') } } + + expect(response).to redirect_to(admin_instances_path(limited: '1')) + expect(DomainBlockWorker).to have_received(:perform_async).exactly(3).times + + # Header should not be imported + expect(DomainBlock.where(domain: '#domain').present?).to eq(false) + + # Domains should now be added + get :export, params: { format: :csv } + expect(response).to have_http_status(200) + expect(response.body).to eq(IO.read(File.join(file_fixture_path, 'domain_blocks.csv'))) + end + end + + it 'displays error on no file selected' do + post :import, params: { admin_import: {} } + expect(response).to redirect_to(admin_instances_path(limited: '1')) + expect(flash[:error]).to eq(I18n.t('admin.export_domain_blocks.no_file')) + end +end diff --git a/spec/fixtures/files/domain_allows.csv b/spec/fixtures/files/domain_allows.csv new file mode 100644 index 000000000..4200ac3f5 --- /dev/null +++ b/spec/fixtures/files/domain_allows.csv @@ -0,0 +1,3 @@ +#domain +good.domain +better.domain diff --git a/spec/fixtures/files/domain_blocks.csv b/spec/fixtures/files/domain_blocks.csv new file mode 100644 index 000000000..28ffb9175 --- /dev/null +++ b/spec/fixtures/files/domain_blocks.csv @@ -0,0 +1,4 @@ +#domain,#severity,#reject_media,#reject_reports,#public_comment,#obfuscate +bad.domain,silence,false,false,bad,false +worse.domain,suspend,true,true,worse,true +reject.media,noop,true,false,reject media,false -- cgit From b91196f4b73fff91997b8077619ae25b6d04a59e Mon Sep 17 00:00:00 2001 From: Claire Date: Mon, 16 May 2022 18:26:49 +0200 Subject: Add confirmation page when importing blocked domains (#1773) * Move glitch-soc-specific strings to glitch-soc-specific locale files * Add confirmation page when importing blocked domains --- app/controllers/admin/domain_blocks_controller.rb | 21 ++++++++++ .../admin/export_domain_blocks_controller.rb | 49 ++++++++++++---------- app/javascript/core/admin.js | 6 +++ app/models/form/domain_block_batch.rb | 35 ++++++++++++++++ .../export_domain_blocks/_domain_block.html.haml | 27 ++++++++++++ .../admin/export_domain_blocks/import.html.haml | 21 ++++++++++ config/locales-glitch/en.yml | 20 +++++++++ config/locales/en.yml | 12 ------ config/routes.rb | 7 +++- .../admin/domain_blocks_controller_spec.rb | 21 ++++++++++ .../admin/export_domain_blocks_controller_spec.rb | 16 +------ 11 files changed, 185 insertions(+), 50 deletions(-) create mode 100644 app/models/form/domain_block_batch.rb create mode 100644 app/views/admin/export_domain_blocks/_domain_block.html.haml create mode 100644 app/views/admin/export_domain_blocks/import.html.haml (limited to 'app/controllers') diff --git a/app/controllers/admin/domain_blocks_controller.rb b/app/controllers/admin/domain_blocks_controller.rb index 16defc1ea..48e9781d6 100644 --- a/app/controllers/admin/domain_blocks_controller.rb +++ b/app/controllers/admin/domain_blocks_controller.rb @@ -4,6 +4,17 @@ module Admin class DomainBlocksController < BaseController before_action :set_domain_block, only: [:show, :destroy, :edit, :update] + def batch + @form = Form::DomainBlockBatch.new(form_domain_block_batch_params.merge(current_account: current_account, action: action_from_button)) + @form.save + rescue ActionController::ParameterMissing + flash[:alert] = I18n.t('admin.email_domain_blocks.no_domain_block_selected') + rescue Mastodon::NotPermittedError + flash[:alert] = I18n.t('admin.domain_blocks.created_msg') + else + redirect_to admin_instances_path(limited: '1'), notice: I18n.t('admin.domain_blocks.created_msg') + end + def new authorize :domain_block, :create? @domain_block = DomainBlock.new(domain: params[:_domain]) @@ -76,5 +87,15 @@ module Admin def resource_params params.require(:domain_block).permit(:domain, :severity, :reject_media, :reject_reports, :private_comment, :public_comment, :obfuscate) end + + def form_domain_block_batch_params + params.require(:form_domain_block_batch).permit(domain_blocks_attributes: [:enabled, :domain, :severity, :reject_media, :reject_reports, :private_comment, :public_comment, :obfuscate]) + end + + def action_from_button + if params[:save] + 'save' + end + end end end diff --git a/app/controllers/admin/export_domain_blocks_controller.rb b/app/controllers/admin/export_domain_blocks_controller.rb index 0ad5b92b5..db8863551 100644 --- a/app/controllers/admin/export_domain_blocks_controller.rb +++ b/app/controllers/admin/export_domain_blocks_controller.rb @@ -21,30 +21,33 @@ module Admin def import authorize :domain_block, :create? - begin - @import = Admin::Import.new(import_params) - parse_import_data!(export_headers) - - @data.take(ROWS_PROCESSING_LIMIT).each do |row| - domain = row['#domain'].strip - next if DomainBlock.rule_for(domain).present? - - domain_block = DomainBlock.new(domain: domain, - severity: row['#severity'].strip, - reject_media: row['#reject_media'].strip, - reject_reports: row['#reject_reports'].strip, - public_comment: row['#public_comment'].strip, - obfuscate: row['#obfuscate'].strip) - if domain_block.save - DomainBlockWorker.perform_async(domain_block.id) - log_action :create, domain_block - end - end - flash[:notice] = I18n.t('admin.domain_blocks.created_msg') - rescue ActionController::ParameterMissing - flash[:error] = I18n.t('admin.export_domain_blocks.no_file') + + @import = Admin::Import.new(import_params) + parse_import_data!(export_headers) + + @global_private_comment = I18n.t('admin.export_domain_blocks.import.private_comment_template', source: @import.data_file_name, date: I18n.l(Time.now.utc)) + + @form = Form::DomainBlockBatch.new + @domain_blocks = @data.take(ROWS_PROCESSING_LIMIT).filter_map do |row| + domain = row['#domain'].strip + next if DomainBlock.rule_for(domain).present? + + domain_block = DomainBlock.new(domain: domain, + severity: row['#severity'].strip, + reject_media: row['#reject_media'].strip, + reject_reports: row['#reject_reports'].strip, + private_comment: @global_private_comment, + public_comment: row['#public_comment']&.strip, + obfuscate: row['#obfuscate'].strip) + + domain_block if domain_block.valid? end - redirect_to admin_instances_path(limited: '1') + + @warning_domains = Instance.where(domain: @domain_blocks.map(&:domain)).where('EXISTS (SELECT 1 FROM follows JOIN accounts ON follows.account_id = accounts.id OR follows.target_account_id = accounts.id WHERE accounts.domain = instances.domain)').pluck(:domain) + rescue ActionController::ParameterMissing + flash.now[:alert] = I18n.t('admin.export_domain_blocks.no_file') + set_dummy_import! + render :new end private diff --git a/app/javascript/core/admin.js b/app/javascript/core/admin.js index ef0a8f267..c1b9f07a4 100644 --- a/app/javascript/core/admin.js +++ b/app/javascript/core/admin.js @@ -102,6 +102,12 @@ ready(() => { const registrationMode = document.getElementById('form_admin_settings_registrations_mode'); if (registrationMode) onChangeRegistrationMode(registrationMode); + const checkAllElement = document.querySelector('#batch_checkbox_all'); + if (checkAllElement) { + checkAllElement.checked = [].every.call(document.querySelectorAll(batchCheckboxClassName), (content) => content.checked); + checkAllElement.indeterminate = !checkAllElement.checked && [].some.call(document.querySelectorAll(batchCheckboxClassName), (content) => content.checked); + } + document.querySelector('a#add-instance-button')?.addEventListener('click', (e) => { const domain = document.getElementById('by_domain')?.value; diff --git a/app/models/form/domain_block_batch.rb b/app/models/form/domain_block_batch.rb new file mode 100644 index 000000000..39012df51 --- /dev/null +++ b/app/models/form/domain_block_batch.rb @@ -0,0 +1,35 @@ +# frozen_string_literal: true + +class Form::DomainBlockBatch + include ActiveModel::Model + include Authorization + include AccountableConcern + + attr_accessor :domain_blocks_attributes, :action, :current_account + + def save + case action + when 'save' + save! + end + end + + private + + def domain_blocks + @domain_blocks ||= domain_blocks_attributes.values.filter_map do |attributes| + DomainBlock.new(attributes.without('enabled')) if ActiveModel::Type::Boolean.new.cast(attributes['enabled']) + end + end + + def save! + domain_blocks.each do |domain_block| + authorize(domain_block, :create?) + next if DomainBlock.rule_for(domain_block.domain).present? + + domain_block.save! + DomainBlockWorker.perform_async(domain_block.id) + log_action :create, domain_block + end + end +end diff --git a/app/views/admin/export_domain_blocks/_domain_block.html.haml b/app/views/admin/export_domain_blocks/_domain_block.html.haml new file mode 100644 index 000000000..5d4b6c4d0 --- /dev/null +++ b/app/views/admin/export_domain_blocks/_domain_block.html.haml @@ -0,0 +1,27 @@ +- existing_relationships ||= false + +.batch-table__row{ class: [existing_relationships && 'batch-table__row--attention'] } + %label.batch-table__row__select.batch-table__row__select--aligned.batch-checkbox + = f.check_box :enabled, checked: !existing_relationships + .batch-table__row__content.pending-account + .pending-account__header + %strong + = f.object.domain + = f.hidden_field :domain + = f.hidden_field :severity + = f.hidden_field :reject_media + = f.hidden_field :reject_reports + = f.hidden_field :obfuscate + = f.hidden_field :private_comment + = f.hidden_field :public_comment + + %br/ + + = f.object.policies.map { |policy| t(policy, scope: 'admin.instances.content_policies.policies') }.join(' • ') + - if f.object.public_comment.present? + • + = f.object.public_comment + - if existing_relationships + • + = fa_icon 'warning fw' + = t('admin.export_domain_blocks.import.existing_relationships_warning') diff --git a/app/views/admin/export_domain_blocks/import.html.haml b/app/views/admin/export_domain_blocks/import.html.haml new file mode 100644 index 000000000..01add232d --- /dev/null +++ b/app/views/admin/export_domain_blocks/import.html.haml @@ -0,0 +1,21 @@ +- content_for :page_title do + = t('admin.export_domain_blocks.import.title') + +%p= t('admin.export_domain_blocks.import.description_html') + +- if defined?(@global_private_comment) && @global_private_comment.present? + %p= t('admin.export_domain_blocks.import.private_comment_description_html', comment: @global_private_comment) + += form_for(@form, url: batch_admin_domain_blocks_path) do |f| + .batch-table + .batch-table__toolbar + %label.batch-table__toolbar__select.batch-checkbox-all + = check_box_tag :batch_checkbox_all, nil, false + .batch-table__toolbar__actions + = f.button safe_join([fa_icon('copy'), t('admin.domain_blocks.import')]), name: :save, class: 'table-action-link', type: :submit, data: { confirm: t('admin.reports.are_you_sure') } + .batch-table__body + - if @domain_blocks.empty? + = nothing_here 'nothing-here--under-tabs' + - else + = f.simple_fields_for :domain_blocks, @domain_blocks do |ff| + = render 'domain_block', f: ff, existing_relationships: @warning_domains.include?(ff.object.domain) diff --git a/config/locales-glitch/en.yml b/config/locales-glitch/en.yml index 3b554f4a2..78933f54e 100644 --- a/config/locales-glitch/en.yml +++ b/config/locales-glitch/en.yml @@ -4,6 +4,26 @@ en: custom_emojis: batch_copy_error: 'An error occurred when copying some of the selected emoji: %{message}' batch_error: 'An error occurred: %{message}' + domain_allows: + export: Export + import: Import + domain_blocks: + export: Export + import: Import + export_domain_allows: + new: + title: Import domain allows + no_file: No file selected + export_domain_blocks: + import: + description_html: You are about to import a list of domain blocks. Please review this list very carefully, especially if you have not authored this list yourself. + existing_relationships_warning: Existing follow relationships + private_comment_description_html: 'To help you track where imported blocks come from, imported blocks will be created with the following private comment: %{comment}' + private_comment_template: Imported from %{source} on %{date} + title: Import domain blocks + new: + title: Import domain blocks + no_file: No file selected settings: captcha_enabled: desc_html: This relies on external scripts from hCaptcha, which may be a security and privacy concern. In addition, this can make the registration process significantly less accessible to some (especially disabled) people. For these reasons, please consider alternative measures such as approval-based or invite-based registration.
Users that have been invited through a limited-use invite will not need to solve a CAPTCHA diff --git a/config/locales/en.yml b/config/locales/en.yml index 4b39d549b..b90402cdd 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -421,8 +421,6 @@ en: add_new: Allow federation with domain created_msg: Domain has been successfully allowed for federation destroyed_msg: Domain has been disallowed from federation - export: Export - import: Import undo: Disallow federation with domain domain_blocks: add_new: Add new domain block @@ -431,8 +429,6 @@ en: domain: Domain edit: Edit domain block existing_domain_block_html: You have already imposed stricter limits on %{name}, you need to unblock it first. - export: Export - import: Import new: create: Create block hint: The domain block will not prevent creation of account entries in the database, but will retroactively and automatically apply specific moderation methods on those accounts. @@ -473,14 +469,6 @@ en: resolved_dns_records_hint_html: The domain name resolves to the following MX domains, which are ultimately responsible for accepting e-mail. Blocking an MX domain will block sign-ups from any e-mail address which uses the same MX domain, even if the visible domain name is different. Be careful not to block major e-mail providers. resolved_through_html: Resolved through %{domain} title: Blocked e-mail domains - export_domain_allows: - new: - title: Import domain allows - no_file: No file selected - export_domain_blocks: - new: - title: Import domain blocks - no_file: No file selected follow_recommendations: description_html: "Follow recommendations help new users quickly find interesting content. When a user has not interacted with others enough to form personalized follow recommendations, these accounts are recommended instead. They are re-calculated on a daily basis from a mix of accounts with the highest recent engagements and highest local follower counts for a given language." language: For language diff --git a/config/routes.rb b/config/routes.rb index 787665192..5ab3ec1e1 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -194,7 +194,11 @@ Rails.application.routes.draw do get '/dashboard', to: 'dashboard#index' resources :domain_allows, only: [:new, :create, :show, :destroy] - resources :domain_blocks, only: [:new, :create, :show, :destroy, :update, :edit] + resources :domain_blocks, only: [:new, :create, :show, :destroy, :update, :edit] do + collection do + post :batch + end + end resources :export_domain_allows, only: [:new] do collection do @@ -485,6 +489,7 @@ Rails.application.routes.draw do end resource :domain_blocks, only: [:show, :create, :destroy] + resource :directory, only: [:show] resources :follow_requests, only: [:index] do diff --git a/spec/controllers/admin/domain_blocks_controller_spec.rb b/spec/controllers/admin/domain_blocks_controller_spec.rb index ecc79292b..a35b2fb3b 100644 --- a/spec/controllers/admin/domain_blocks_controller_spec.rb +++ b/spec/controllers/admin/domain_blocks_controller_spec.rb @@ -16,6 +16,27 @@ RSpec.describe Admin::DomainBlocksController, type: :controller do end end + describe 'POST #batch' do + it 'blocks the domains when succeeded to save' do + allow(DomainBlockWorker).to receive(:perform_async).and_return(true) + + post :batch, params: { + save: '', + form_domain_block_batch: { + domain_blocks_attributes: { + '0' => { enabled: '1', domain: 'example.com', severity: 'silence' }, + '1' => { enabled: '0', domain: 'mastodon.social', severity: 'suspend' }, + '2' => { enabled: '1', domain: 'mastodon.online', severity: 'suspend' } + } + } + } + + expect(DomainBlockWorker).to have_received(:perform_async).exactly(2).times + expect(flash[:notice]).to eq I18n.t('admin.domain_blocks.created_msg') + expect(response).to redirect_to(admin_instances_path(limited: '1')) + end + end + describe 'POST #create' do it 'blocks the domain when succeeded to save' do allow(DomainBlockWorker).to receive(:perform_async).and_return(true) diff --git a/spec/controllers/admin/export_domain_blocks_controller_spec.rb b/spec/controllers/admin/export_domain_blocks_controller_spec.rb index 0cb221972..0493df859 100644 --- a/spec/controllers/admin/export_domain_blocks_controller_spec.rb +++ b/spec/controllers/admin/export_domain_blocks_controller_spec.rb @@ -22,26 +22,14 @@ RSpec.describe Admin::ExportDomainBlocksController, type: :controller do describe 'POST #import' do it 'blocks imported domains' do - allow(DomainBlockWorker).to receive(:perform_async).and_return(true) - post :import, params: { admin_import: { data: fixture_file_upload('domain_blocks.csv') } } - expect(response).to redirect_to(admin_instances_path(limited: '1')) - expect(DomainBlockWorker).to have_received(:perform_async).exactly(3).times - - # Header should not be imported - expect(DomainBlock.where(domain: '#domain').present?).to eq(false) - - # Domains should now be added - get :export, params: { format: :csv } - expect(response).to have_http_status(200) - expect(response.body).to eq(IO.read(File.join(file_fixture_path, 'domain_blocks.csv'))) + expect(assigns(:domain_blocks).map(&:domain)).to match_array ['bad.domain', 'worse.domain', 'reject.media'] end end it 'displays error on no file selected' do post :import, params: { admin_import: {} } - expect(response).to redirect_to(admin_instances_path(limited: '1')) - expect(flash[:error]).to eq(I18n.t('admin.export_domain_blocks.no_file')) + expect(flash[:alert]).to eq(I18n.t('admin.export_domain_blocks.no_file')) end end -- cgit From 440eb71310e41d668f00980b73358edd5f8df043 Mon Sep 17 00:00:00 2001 From: Claire Date: Thu, 26 May 2022 15:50:33 +0200 Subject: Change unapproved and unconfirmed account to not be accessible in the REST API (#17530) * Change unapproved and unconfirmed account to not be accessible in the REST API * Change Account#searchable? to reject unconfirmed and unapproved users * Disable search for unapproved and unconfirmed users in Account.search_for * Disable search for unapproved and unconfirmed users in Account.advanced_search_for * Remove unconfirmed and unapproved accounts from Account.searchable scope * Prevent mentions to unapproved/unconfirmed accounts * Fix some old tests for Account.advanced_search_for * Add some Account.advanced_search_for tests for existing behaviors * Add some tests for Account.search_for * Add Account.advanced_search_for tests unconfirmed and unapproved accounts * Add Account.searchable tests * Fix Account.without_unapproved scope potentially messing with previously-applied scopes * Allow lookup of unconfirmed/unapproved accounts through /api/v1/accounts/lookup This is so that the API can still be used to check whether an username is free to use. --- app/controllers/api/v1/accounts_controller.rb | 10 ++ app/models/account.rb | 9 +- app/services/process_mentions_service.rb | 3 + spec/models/account_spec.rb | 178 +++++++++++++++++++++++++- 4 files changed, 194 insertions(+), 6 deletions(-) (limited to 'app/controllers') diff --git a/app/controllers/api/v1/accounts_controller.rb b/app/controllers/api/v1/accounts_controller.rb index 5134bfb94..5537cc9b0 100644 --- a/app/controllers/api/v1/accounts_controller.rb +++ b/app/controllers/api/v1/accounts_controller.rb @@ -9,6 +9,8 @@ class Api::V1::AccountsController < Api::BaseController before_action :require_user!, except: [:show, :create] before_action :set_account, except: [:create] + before_action :check_account_approval, except: [:create] + before_action :check_account_confirmation, except: [:create] before_action :check_enabled_registrations, only: [:create] skip_before_action :require_authenticated_user!, only: :create @@ -74,6 +76,14 @@ class Api::V1::AccountsController < Api::BaseController @account = Account.find(params[:id]) end + def check_account_approval + raise(ActiveRecord::RecordNotFound) if @account.local? && @account.user_pending? + end + + def check_account_confirmation + raise(ActiveRecord::RecordNotFound) if @account.local? && !@account.user_confirmed? + end + def relationships(**options) AccountRelationshipsPresenter.new([@account.id], current_user.account_id, **options) end diff --git a/app/models/account.rb b/app/models/account.rb index 7b460b054..bd94142c4 100644 --- a/app/models/account.rb +++ b/app/models/account.rb @@ -109,7 +109,8 @@ class Account < ApplicationRecord scope :matches_username, ->(value) { where(arel_table[:username].matches("#{value}%")) } scope :matches_display_name, ->(value) { where(arel_table[:display_name].matches("#{value}%")) } scope :matches_domain, ->(value) { where(arel_table[:domain].matches("%#{value}%")) } - scope :searchable, -> { without_suspended.where(moved_to_account_id: nil) } + scope :without_unapproved, -> { left_outer_joins(:user).remote.or(left_outer_joins(:user).merge(User.approved.confirmed)) } + scope :searchable, -> { without_unapproved.without_suspended.where(moved_to_account_id: nil) } scope :discoverable, -> { searchable.without_silenced.where(discoverable: true).left_outer_joins(:account_stat) } scope :followable_by, ->(account) { joins(arel_table.join(Follow.arel_table, Arel::Nodes::OuterJoin).on(arel_table[:id].eq(Follow.arel_table[:target_account_id]).and(Follow.arel_table[:account_id].eq(account.id))).join_sources).where(Follow.arel_table[:id].eq(nil)).joins(arel_table.join(FollowRequest.arel_table, Arel::Nodes::OuterJoin).on(arel_table[:id].eq(FollowRequest.arel_table[:target_account_id]).and(FollowRequest.arel_table[:account_id].eq(account.id))).join_sources).where(FollowRequest.arel_table[:id].eq(nil)) } scope :by_recent_status, -> { order(Arel.sql('(case when account_stats.last_status_at is null then 1 else 0 end) asc, account_stats.last_status_at desc, accounts.id desc')) } @@ -193,7 +194,7 @@ class Account < ApplicationRecord end def searchable? - !(suspended? || moved?) + !(suspended? || moved?) && (!local? || (approved? && confirmed?)) end def possibly_stale? @@ -461,9 +462,11 @@ class Account < ApplicationRecord accounts.*, ts_rank_cd(#{TEXTSEARCH}, to_tsquery('simple', :tsquery), 32) AS rank FROM accounts + LEFT JOIN users ON accounts.id = users.account_id WHERE to_tsquery('simple', :tsquery) @@ #{TEXTSEARCH} AND accounts.suspended_at IS NULL AND accounts.moved_to_account_id IS NULL + AND (accounts.domain IS NOT NULL OR (users.approved = TRUE AND users.confirmed_at IS NOT NULL)) ORDER BY rank DESC LIMIT :limit OFFSET :offset SQL @@ -539,9 +542,11 @@ class Account < ApplicationRecord (count(f.id) + 1) * ts_rank_cd(#{TEXTSEARCH}, to_tsquery('simple', :tsquery), 32) AS rank FROM accounts LEFT OUTER JOIN follows AS f ON (accounts.id = f.account_id AND f.target_account_id = :id) OR (accounts.id = f.target_account_id AND f.account_id = :id) + LEFT JOIN users ON accounts.id = users.account_id WHERE to_tsquery('simple', :tsquery) @@ #{TEXTSEARCH} AND accounts.suspended_at IS NULL AND accounts.moved_to_account_id IS NULL + AND (accounts.domain IS NOT NULL OR (users.approved = TRUE AND users.confirmed_at IS NOT NULL)) GROUP BY accounts.id ORDER BY rank DESC LIMIT :limit OFFSET :offset diff --git a/app/services/process_mentions_service.rb b/app/services/process_mentions_service.rb index 9d239fc65..8c63b611d 100644 --- a/app/services/process_mentions_service.rb +++ b/app/services/process_mentions_service.rb @@ -37,6 +37,9 @@ class ProcessMentionsService < BaseService mentioned_account = Account.find_remote(username, domain) + # Unapproved and unconfirmed accounts should not be mentionable + next if mentioned_account&.local? && !(mentioned_account.user_confirmed? && mentioned_account.user_approved?) + # If the account cannot be found or isn't the right protocol, # first try to resolve it if mention_undeliverable?(mentioned_account) diff --git a/spec/models/account_spec.rb b/spec/models/account_spec.rb index 681134d49..dc0ca3da3 100644 --- a/spec/models/account_spec.rb +++ b/spec/models/account_spec.rb @@ -350,6 +350,45 @@ RSpec.describe Account, type: :model do ) end + it 'does not return suspended users' do + match = Fabricate( + :account, + display_name: 'Display Name', + username: 'username', + domain: 'example.com', + suspended: true + ) + + results = Account.search_for('username') + expect(results).to eq [] + end + + it 'does not return unapproved users' do + match = Fabricate( + :account, + display_name: 'Display Name', + username: 'username' + ) + + match.user.update(approved: false) + + results = Account.search_for('username') + expect(results).to eq [] + end + + it 'does not return unconfirmed users' do + match = Fabricate( + :account, + display_name: 'Display Name', + username: 'username' + ) + + match.user.update(confirmed_at: nil) + + results = Account.search_for('username') + expect(results).to eq [] + end + it 'accepts ?, \, : and space as delimiter' do match = Fabricate( :account, @@ -422,8 +461,114 @@ RSpec.describe Account, type: :model do end describe '.advanced_search_for' do + let(:account) { Fabricate(:account) } + + context 'when limiting search to followed accounts' do + it 'accepts ?, \, : and space as delimiter' do + match = Fabricate( + :account, + display_name: 'A & l & i & c & e', + username: 'username', + domain: 'example.com' + ) + account.follow!(match) + + results = Account.advanced_search_for('A?l\i:c e', account, 10, true) + expect(results).to eq [match] + end + + it 'does not return non-followed accounts' do + match = Fabricate( + :account, + display_name: 'A & l & i & c & e', + username: 'username', + domain: 'example.com' + ) + + results = Account.advanced_search_for('A?l\i:c e', account, 10, true) + expect(results).to eq [] + end + + it 'does not return suspended users' do + match = Fabricate( + :account, + display_name: 'Display Name', + username: 'username', + domain: 'example.com', + suspended: true + ) + + results = Account.advanced_search_for('username', account, 10, true) + expect(results).to eq [] + end + + it 'does not return unapproved users' do + match = Fabricate( + :account, + display_name: 'Display Name', + username: 'username' + ) + + match.user.update(approved: false) + + results = Account.advanced_search_for('username', account, 10, true) + expect(results).to eq [] + end + + it 'does not return unconfirmed users' do + match = Fabricate( + :account, + display_name: 'Display Name', + username: 'username' + ) + + match.user.update(confirmed_at: nil) + + results = Account.advanced_search_for('username', account, 10, true) + expect(results).to eq [] + end + end + + it 'does not return suspended users' do + match = Fabricate( + :account, + display_name: 'Display Name', + username: 'username', + domain: 'example.com', + suspended: true + ) + + results = Account.advanced_search_for('username', account) + expect(results).to eq [] + end + + it 'does not return unapproved users' do + match = Fabricate( + :account, + display_name: 'Display Name', + username: 'username' + ) + + match.user.update(approved: false) + + results = Account.advanced_search_for('username', account) + expect(results).to eq [] + end + + it 'does not return unconfirmed users' do + match = Fabricate( + :account, + display_name: 'Display Name', + username: 'username' + ) + + match.user.update(confirmed_at: nil) + + results = Account.advanced_search_for('username', account) + expect(results).to eq [] + end + it 'accepts ?, \, : and space as delimiter' do - account = Fabricate(:account) match = Fabricate( :account, display_name: 'A & l & i & c & e', @@ -437,18 +582,17 @@ RSpec.describe Account, type: :model do it 'limits by 10 by default' do 11.times { Fabricate(:account, display_name: "Display Name") } - results = Account.search_for("display") + results = Account.advanced_search_for("display", account) expect(results.size).to eq 10 end it 'accepts arbitrary limits' do 2.times { Fabricate(:account, display_name: "Display Name") } - results = Account.search_for("display", 1) + results = Account.advanced_search_for("display", account, 1) expect(results.size).to eq 1 end it 'ranks followed accounts higher' do - account = Fabricate(:account) match = Fabricate(:account, username: "Matching") followed_match = Fabricate(:account, username: "Matcher") Fabricate(:follow, account: account, target_account: followed_match) @@ -775,6 +919,32 @@ RSpec.describe Account, type: :model do expect(Account.suspended).to match_array([account_1]) end end + + describe 'searchable' do + let!(:suspended_local) { Fabricate(:account, suspended: true, username: 'suspended_local') } + let!(:suspended_remote) { Fabricate(:account, suspended: true, domain: 'example.org', username: 'suspended_remote') } + let!(:silenced_local) { Fabricate(:account, silenced: true, username: 'silenced_local') } + let!(:silenced_remote) { Fabricate(:account, silenced: true, domain: 'example.org', username: 'silenced_remote') } + let!(:unconfirmed) { Fabricate(:user, confirmed_at: nil).account } + let!(:unapproved) { Fabricate(:user, approved: false).account } + let!(:unconfirmed_unapproved) { Fabricate(:user, confirmed_at: nil, approved: false).account } + let!(:local_account) { Fabricate(:account, username: 'local_account') } + let!(:remote_account) { Fabricate(:account, domain: 'example.org', username: 'remote_account') } + + before do + # Accounts get automatically-approved depending on settings, so ensure they aren't approved + unapproved.user.update(approved: false) + unconfirmed_unapproved.user.update(approved: false) + end + + it 'returns every usable non-suspended account' do + expect(Account.searchable).to match_array([silenced_local, silenced_remote, local_account, remote_account]) + end + + it 'does not mess with previously-applied scopes' do + expect(Account.where.not(id: remote_account.id).searchable).to match_array([silenced_local, silenced_remote, local_account]) + end + end end context 'when is local' do -- cgit From 96129c2f10a82520648f6ae04e585cf797403617 Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Thu, 26 May 2022 22:03:54 +0200 Subject: Fix confirmation redirect to app without `Location` header (#18523) --- app/controllers/auth/confirmations_controller.rb | 2 +- app/lib/application_extension.rb | 4 ++++ config/initializers/doorkeeper.rb | 7 +++++++ 3 files changed, 12 insertions(+), 1 deletion(-) (limited to 'app/controllers') diff --git a/app/controllers/auth/confirmations_controller.rb b/app/controllers/auth/confirmations_controller.rb index 1475bbcef..010fd3755 100644 --- a/app/controllers/auth/confirmations_controller.rb +++ b/app/controllers/auth/confirmations_controller.rb @@ -40,7 +40,7 @@ class Auth::ConfirmationsController < Devise::ConfirmationsController def after_confirmation_path_for(_resource_name, user) if user.created_by_application && truthy_param?(:redirect_to_app) - user.created_by_application.redirect_uri + user.created_by_application.confirmation_redirect_uri else super end diff --git a/app/lib/application_extension.rb b/app/lib/application_extension.rb index a1fea6430..d61ec0e6e 100644 --- a/app/lib/application_extension.rb +++ b/app/lib/application_extension.rb @@ -12,4 +12,8 @@ module ApplicationExtension def most_recently_used_access_token @most_recently_used_access_token ||= access_tokens.where.not(last_used_at: nil).order(last_used_at: :desc).first end + + def confirmation_redirect_uri + redirect_uri.lines.first.strip + end end diff --git a/config/initializers/doorkeeper.rb b/config/initializers/doorkeeper.rb index f78db8653..84b649f5c 100644 --- a/config/initializers/doorkeeper.rb +++ b/config/initializers/doorkeeper.rb @@ -128,6 +128,13 @@ Doorkeeper.configure do # force_ssl_in_redirect_uri false + # Specify what redirect URI's you want to block during Application creation. + # Any redirect URI is whitelisted by default. + # + # You can use this option in order to forbid URI's with 'javascript' scheme + # for example. + forbid_redirect_uri { |uri| %w[data vbscript javascript].include?(uri.scheme.to_s.downcase) } + # Specify what grant flows are enabled in array of Strings. The valid # strings and the flows they enable are: # -- cgit From 9f81b9f29a14093cefcdbf09058ace089cd8e06b Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Thu, 26 May 2022 22:04:05 +0200 Subject: Fix suspended users being able to access APIs that don't require a user (#18524) --- app/controllers/activitypub/base_controller.rb | 1 + app/controllers/api/base_controller.rb | 5 +++++ 2 files changed, 6 insertions(+) (limited to 'app/controllers') diff --git a/app/controllers/activitypub/base_controller.rb b/app/controllers/activitypub/base_controller.rb index 196d85a32..b8a7e0ab9 100644 --- a/app/controllers/activitypub/base_controller.rb +++ b/app/controllers/activitypub/base_controller.rb @@ -2,6 +2,7 @@ class ActivityPub::BaseController < Api::BaseController skip_before_action :require_authenticated_user! + skip_before_action :require_not_suspended! skip_around_action :set_locale private diff --git a/app/controllers/api/base_controller.rb b/app/controllers/api/base_controller.rb index d96285b44..2e393fbb6 100644 --- a/app/controllers/api/base_controller.rb +++ b/app/controllers/api/base_controller.rb @@ -11,6 +11,7 @@ class Api::BaseController < ApplicationController skip_before_action :require_functional!, unless: :whitelist_mode? before_action :require_authenticated_user!, if: :disallow_unauthenticated_api_access? + before_action :require_not_suspended! before_action :set_cache_headers protect_from_forgery with: :null_session @@ -97,6 +98,10 @@ class Api::BaseController < ApplicationController render json: { error: 'This method requires an authenticated user' }, status: 401 unless current_user end + def require_not_suspended! + render json: { error: 'Your login is currently disabled' }, status: 403 if current_user&.account&.suspended? + end + def require_user! if !current_user render json: { error: 'This method requires an authenticated user' }, status: 422 -- cgit