From e6b272e5c9c227cfbbe375a893f567c5967d669c Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Fri, 11 Sep 2020 15:16:29 +0200 Subject: Change REST API to return empty data for suspended accounts (#14765) --- app/controllers/activitypub/outboxes_controller.rb | 3 +-- app/controllers/api/v1/accounts/featured_tags_controller.rb | 2 +- app/controllers/api/v1/accounts/follower_accounts_controller.rb | 2 +- app/controllers/api/v1/accounts/following_accounts_controller.rb | 2 +- app/controllers/api/v1/accounts/identity_proofs_controller.rb | 2 +- app/controllers/api/v1/accounts/lists_controller.rb | 2 +- app/controllers/api/v1/accounts/relationships_controller.rb | 2 +- app/controllers/api/v1/accounts/statuses_controller.rb | 2 +- app/controllers/api/v1/accounts_controller.rb | 5 ----- app/controllers/api/v1/blocks_controller.rb | 2 ++ app/controllers/api/v1/endorsements_controller.rb | 2 +- app/controllers/api/v1/follow_requests_controller.rb | 2 +- app/controllers/api/v1/lists/accounts_controller.rb | 4 ++-- app/controllers/api/v1/mutes_controller.rb | 2 ++ app/controllers/api/v1/notifications_controller.rb | 4 ++-- app/controllers/api/v1/statuses/favourited_by_accounts_controller.rb | 1 + app/controllers/api/v1/statuses/reblogged_by_accounts_controller.rb | 2 +- 17 files changed, 20 insertions(+), 21 deletions(-) (limited to 'app/controllers') diff --git a/app/controllers/activitypub/outboxes_controller.rb b/app/controllers/activitypub/outboxes_controller.rb index e066860bf..5fd735ad6 100644 --- a/app/controllers/activitypub/outboxes_controller.rb +++ b/app/controllers/activitypub/outboxes_controller.rb @@ -57,9 +57,8 @@ class ActivityPub::OutboxesController < ActivityPub::BaseController def set_statuses return unless page_requested? - @statuses = @account.statuses.permitted_for(@account, signed_request_account) @statuses = cache_collection_paginated_by_id( - @statuses, + @account.statuses.permitted_for(@account, signed_request_account), Status, LIMIT, params_slice(:max_id, :min_id, :since_id) diff --git a/app/controllers/api/v1/accounts/featured_tags_controller.rb b/app/controllers/api/v1/accounts/featured_tags_controller.rb index d6277261d..014d71956 100644 --- a/app/controllers/api/v1/accounts/featured_tags_controller.rb +++ b/app/controllers/api/v1/accounts/featured_tags_controller.rb @@ -17,6 +17,6 @@ class Api::V1::Accounts::FeaturedTagsController < Api::BaseController end def set_featured_tags - @featured_tags = @account.featured_tags + @featured_tags = @account.suspended? ? @account.featured_tags : [] end end diff --git a/app/controllers/api/v1/accounts/follower_accounts_controller.rb b/app/controllers/api/v1/accounts/follower_accounts_controller.rb index 2277067c9..a665863eb 100644 --- a/app/controllers/api/v1/accounts/follower_accounts_controller.rb +++ b/app/controllers/api/v1/accounts/follower_accounts_controller.rb @@ -25,7 +25,7 @@ class Api::V1::Accounts::FollowerAccountsController < Api::BaseController end def hide_results? - (@account.hides_followers? && current_account&.id != @account.id) || (current_account && @account.blocking?(current_account)) + @account.suspended? || (@account.hides_followers? && current_account&.id != @account.id) || (current_account && @account.blocking?(current_account)) end def default_accounts diff --git a/app/controllers/api/v1/accounts/following_accounts_controller.rb b/app/controllers/api/v1/accounts/following_accounts_controller.rb index 93d4bd3a4..7d885a212 100644 --- a/app/controllers/api/v1/accounts/following_accounts_controller.rb +++ b/app/controllers/api/v1/accounts/following_accounts_controller.rb @@ -25,7 +25,7 @@ class Api::V1::Accounts::FollowingAccountsController < Api::BaseController end def hide_results? - (@account.hides_following? && current_account&.id != @account.id) || (current_account && @account.blocking?(current_account)) + @account.suspended? || (@account.hides_following? && current_account&.id != @account.id) || (current_account && @account.blocking?(current_account)) end def default_accounts diff --git a/app/controllers/api/v1/accounts/identity_proofs_controller.rb b/app/controllers/api/v1/accounts/identity_proofs_controller.rb index 8dad6fee9..4b5f6902c 100644 --- a/app/controllers/api/v1/accounts/identity_proofs_controller.rb +++ b/app/controllers/api/v1/accounts/identity_proofs_controller.rb @@ -5,7 +5,7 @@ class Api::V1::Accounts::IdentityProofsController < Api::BaseController before_action :set_account def index - @proofs = @account.identity_proofs.active + @proofs = @account.suspended? ? [] : @account.identity_proofs.active render json: @proofs, each_serializer: REST::IdentityProofSerializer end diff --git a/app/controllers/api/v1/accounts/lists_controller.rb b/app/controllers/api/v1/accounts/lists_controller.rb index ccb751f8f..c92f1f8a0 100644 --- a/app/controllers/api/v1/accounts/lists_controller.rb +++ b/app/controllers/api/v1/accounts/lists_controller.rb @@ -6,7 +6,7 @@ class Api::V1::Accounts::ListsController < Api::BaseController before_action :set_account def index - @lists = @account.lists.where(account: current_account) + @lists = @account.suspended? ? [] : @account.lists.where(account: current_account) render json: @lists, each_serializer: REST::ListSerializer end diff --git a/app/controllers/api/v1/accounts/relationships_controller.rb b/app/controllers/api/v1/accounts/relationships_controller.rb index 1d3992a28..503f85c97 100644 --- a/app/controllers/api/v1/accounts/relationships_controller.rb +++ b/app/controllers/api/v1/accounts/relationships_controller.rb @@ -5,7 +5,7 @@ class Api::V1::Accounts::RelationshipsController < Api::BaseController before_action :require_user! def index - accounts = Account.where(id: account_ids).select('id') + accounts = Account.without_suspended.where(id: account_ids).select('id') # .where doesn't guarantee that our results are in the same order # we requested them, so return the "right" order to the requestor. @accounts = accounts.index_by(&:id).values_at(*account_ids).compact diff --git a/app/controllers/api/v1/accounts/statuses_controller.rb b/app/controllers/api/v1/accounts/statuses_controller.rb index 85a9133e3..92ccb8061 100644 --- a/app/controllers/api/v1/accounts/statuses_controller.rb +++ b/app/controllers/api/v1/accounts/statuses_controller.rb @@ -18,7 +18,7 @@ class Api::V1::Accounts::StatusesController < Api::BaseController end def load_statuses - cached_account_statuses + @account.suspended? ? [] : cached_account_statuses end def cached_account_statuses diff --git a/app/controllers/api/v1/accounts_controller.rb b/app/controllers/api/v1/accounts_controller.rb index 0080faf33..61dcb87c2 100644 --- a/app/controllers/api/v1/accounts_controller.rb +++ b/app/controllers/api/v1/accounts_controller.rb @@ -9,7 +9,6 @@ class Api::V1::AccountsController < Api::BaseController before_action :require_user!, except: [:show, :create] before_action :set_account, except: [:create] - before_action :check_account_suspension, only: [:show] before_action :check_enabled_registrations, only: [:create] skip_before_action :require_authenticated_user!, only: :create @@ -73,10 +72,6 @@ class Api::V1::AccountsController < Api::BaseController AccountRelationshipsPresenter.new([@account.id], current_user.account_id, options) end - def check_account_suspension - gone if @account.suspended? - end - def account_params params.permit(:username, :email, :password, :agreement, :locale, :reason) end diff --git a/app/controllers/api/v1/blocks_controller.rb b/app/controllers/api/v1/blocks_controller.rb index a2baeef90..586cdfca9 100644 --- a/app/controllers/api/v1/blocks_controller.rb +++ b/app/controllers/api/v1/blocks_controller.rb @@ -18,6 +18,8 @@ class Api::V1::BlocksController < Api::BaseController def paginated_blocks @paginated_blocks ||= Block.eager_load(target_account: :account_stat) + .joins(:target_account) + .merge(Account.without_suspended) .where(account: current_account) .paginate_by_max_id( limit_param(DEFAULT_ACCOUNTS_LIMIT), diff --git a/app/controllers/api/v1/endorsements_controller.rb b/app/controllers/api/v1/endorsements_controller.rb index c87dbc4ce..9e80f468a 100644 --- a/app/controllers/api/v1/endorsements_controller.rb +++ b/app/controllers/api/v1/endorsements_controller.rb @@ -25,7 +25,7 @@ class Api::V1::EndorsementsController < Api::BaseController end def endorsed_accounts - current_account.endorsed_accounts.includes(:account_stat) + current_account.endorsed_accounts.includes(:account_stat).without_suspended end def insert_pagination_headers diff --git a/app/controllers/api/v1/follow_requests_controller.rb b/app/controllers/api/v1/follow_requests_controller.rb index 0ee6e531f..0420b7bef 100644 --- a/app/controllers/api/v1/follow_requests_controller.rb +++ b/app/controllers/api/v1/follow_requests_controller.rb @@ -37,7 +37,7 @@ class Api::V1::FollowRequestsController < Api::BaseController end def default_accounts - Account.includes(:follow_requests, :account_stat).references(:follow_requests) + Account.without_suspended.includes(:follow_requests, :account_stat).references(:follow_requests) end def paginated_follow_requests diff --git a/app/controllers/api/v1/lists/accounts_controller.rb b/app/controllers/api/v1/lists/accounts_controller.rb index 23078263e..b66ea9bfe 100644 --- a/app/controllers/api/v1/lists/accounts_controller.rb +++ b/app/controllers/api/v1/lists/accounts_controller.rb @@ -37,9 +37,9 @@ class Api::V1::Lists::AccountsController < Api::BaseController def load_accounts if unlimited? - @list.accounts.includes(:account_stat).all + @list.accounts.without_suspended.includes(:account_stat).all else - @list.accounts.includes(:account_stat).paginate_by_max_id(limit_param(DEFAULT_ACCOUNTS_LIMIT), params[:max_id], params[:since_id]) + @list.accounts.without_suspended.includes(:account_stat).paginate_by_max_id(limit_param(DEFAULT_ACCOUNTS_LIMIT), params[:max_id], params[:since_id]) end end diff --git a/app/controllers/api/v1/mutes_controller.rb b/app/controllers/api/v1/mutes_controller.rb index 65439fe9b..805d0dee2 100644 --- a/app/controllers/api/v1/mutes_controller.rb +++ b/app/controllers/api/v1/mutes_controller.rb @@ -18,6 +18,8 @@ class Api::V1::MutesController < Api::BaseController def paginated_mutes @paginated_mutes ||= Mute.eager_load(:target_account) + .joins(:target_account) + .merge(Account.without_suspended) .where(account: current_account) .paginate_by_max_id( limit_param(DEFAULT_ACCOUNTS_LIMIT), diff --git a/app/controllers/api/v1/notifications_controller.rb b/app/controllers/api/v1/notifications_controller.rb index 9d03cb879..522c35ba5 100644 --- a/app/controllers/api/v1/notifications_controller.rb +++ b/app/controllers/api/v1/notifications_controller.rb @@ -14,7 +14,7 @@ class Api::V1::NotificationsController < Api::BaseController end def show - @notification = current_account.notifications.find(params[:id]) + @notification = current_account.notifications.without_suspended.find(params[:id]) render json: @notification, serializer: REST::NotificationSerializer end @@ -40,7 +40,7 @@ class Api::V1::NotificationsController < Api::BaseController end def browserable_account_notifications - current_account.notifications.browserable(exclude_types, from_account) + current_account.notifications.without_suspended.browserable(exclude_types, from_account) end def target_statuses_from_notifications 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 8229786d6..2b614a837 100644 --- a/app/controllers/api/v1/statuses/favourited_by_accounts_controller.rb +++ b/app/controllers/api/v1/statuses/favourited_by_accounts_controller.rb @@ -22,6 +22,7 @@ class Api::V1::Statuses::FavouritedByAccountsController < Api::BaseController def default_accounts Account + .without_suspended .includes(:favourites, :account_stat) .references(:favourites) .where(favourites: { status_id: @status.id }) 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 6c9e49d90..24db30fcc 100644 --- a/app/controllers/api/v1/statuses/reblogged_by_accounts_controller.rb +++ b/app/controllers/api/v1/statuses/reblogged_by_accounts_controller.rb @@ -21,7 +21,7 @@ class Api::V1::Statuses::RebloggedByAccountsController < Api::BaseController end def default_accounts - Account.includes(:statuses, :account_stat).references(:statuses) + Account.without_suspended.includes(:statuses, :account_stat).references(:statuses) end def paginated_statuses -- cgit From 4e4b3a0c8e69a724e229f028896ce774ef26df3b Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Fri, 11 Sep 2020 20:56:35 +0200 Subject: Refactor settings controllers (#14767) - Disallow suspended accounts from revoking sessions and apps - Allow suspended accounts to access exports --- .../concerns/export_controller_concern.rb | 5 ---- .../oauth/authorized_applications_controller.rb | 5 ++++ app/controllers/settings/aliases_controller.rb | 4 ++-- .../settings/applications_controller.rb | 3 --- app/controllers/settings/base_controller.rb | 7 ++++++ app/controllers/settings/deletes_controller.rb | 7 ++---- .../exports/blocked_accounts_controller.rb | 2 +- .../settings/exports/blocked_domains_controller.rb | 2 +- .../exports/following_accounts_controller.rb | 2 +- .../settings/exports/lists_controller.rb | 2 +- .../settings/exports/muted_accounts_controller.rb | 2 +- app/controllers/settings/exports_controller.rb | 11 --------- .../settings/featured_tags_controller.rb | 3 --- .../settings/identity_proofs_controller.rb | 3 --- app/controllers/settings/imports_controller.rb | 3 --- .../settings/migration/redirects_controller.rb | 9 +------ app/controllers/settings/migrations_controller.rb | 9 +------ app/controllers/settings/pictures_controller.rb | 1 - app/controllers/settings/preferences_controller.rb | 4 ---- app/controllers/settings/profiles_controller.rb | 3 --- app/controllers/settings/sessions_controller.rb | 6 ++--- .../confirmations_controller.rb | 5 +--- .../otp_authentication_controller.rb | 5 +--- .../recovery_codes_controller.rb | 5 +--- .../webauthn_credentials_controller.rb | 3 +-- ...two_factor_authentication_methods_controller.rb | 5 +--- app/views/auth/registrations/_sessions.html.haml | 2 +- app/views/auth/registrations/edit.html.haml | 21 ++++++++-------- .../oauth/authorized_applications/index.html.haml | 2 +- config/navigation.rb | 2 +- .../settings/deletes_controller_spec.rb | 28 +++++++++++----------- 31 files changed, 59 insertions(+), 112 deletions(-) (limited to 'app/controllers') diff --git a/app/controllers/concerns/export_controller_concern.rb b/app/controllers/concerns/export_controller_concern.rb index bfe990c82..24cfc7a01 100644 --- a/app/controllers/concerns/export_controller_concern.rb +++ b/app/controllers/concerns/export_controller_concern.rb @@ -5,7 +5,6 @@ module ExportControllerConcern included do before_action :authenticate_user! - before_action :require_not_suspended! before_action :load_export skip_before_action :require_functional! @@ -30,8 +29,4 @@ module ExportControllerConcern def export_filename "#{controller_name}.csv" end - - def require_not_suspended! - forbidden if current_account.suspended? - end end diff --git a/app/controllers/oauth/authorized_applications_controller.rb b/app/controllers/oauth/authorized_applications_controller.rb index fb8389034..45151cdd7 100644 --- a/app/controllers/oauth/authorized_applications_controller.rb +++ b/app/controllers/oauth/authorized_applications_controller.rb @@ -5,6 +5,7 @@ class Oauth::AuthorizedApplicationsController < Doorkeeper::AuthorizedApplicatio before_action :store_current_location before_action :authenticate_resource_owner! + before_action :require_not_suspended!, only: :destroy before_action :set_body_classes skip_before_action :require_functional! @@ -25,4 +26,8 @@ class Oauth::AuthorizedApplicationsController < Doorkeeper::AuthorizedApplicatio def store_current_location store_location_for(:user, request.url) end + + def require_not_suspended! + forbidden if current_account.suspended? + end end diff --git a/app/controllers/settings/aliases_controller.rb b/app/controllers/settings/aliases_controller.rb index b7c9a409d..a421b8ede 100644 --- a/app/controllers/settings/aliases_controller.rb +++ b/app/controllers/settings/aliases_controller.rb @@ -1,9 +1,9 @@ # frozen_string_literal: true class Settings::AliasesController < Settings::BaseController - layout 'admin' + skip_before_action :require_functional! - before_action :authenticate_user! + before_action :require_not_suspended! before_action :set_aliases, except: :destroy before_action :set_alias, only: :destroy diff --git a/app/controllers/settings/applications_controller.rb b/app/controllers/settings/applications_controller.rb index ed3f82a8e..d3ac268d8 100644 --- a/app/controllers/settings/applications_controller.rb +++ b/app/controllers/settings/applications_controller.rb @@ -1,9 +1,6 @@ # frozen_string_literal: true class Settings::ApplicationsController < Settings::BaseController - layout 'admin' - - before_action :authenticate_user! before_action :set_application, only: [:show, :update, :destroy, :regenerate] before_action :prepare_scopes, only: [:create, :update] diff --git a/app/controllers/settings/base_controller.rb b/app/controllers/settings/base_controller.rb index 3c404cfff..8311538a5 100644 --- a/app/controllers/settings/base_controller.rb +++ b/app/controllers/settings/base_controller.rb @@ -1,6 +1,9 @@ # frozen_string_literal: true class Settings::BaseController < ApplicationController + layout 'admin' + + before_action :authenticate_user! before_action :set_body_classes before_action :set_cache_headers @@ -13,4 +16,8 @@ class Settings::BaseController < ApplicationController def set_cache_headers response.headers['Cache-Control'] = 'no-cache, no-store, max-age=0, must-revalidate' end + + def require_not_suspended! + forbidden if current_account.suspended? + end end diff --git a/app/controllers/settings/deletes_controller.rb b/app/controllers/settings/deletes_controller.rb index 15a59c999..7d4844e60 100644 --- a/app/controllers/settings/deletes_controller.rb +++ b/app/controllers/settings/deletes_controller.rb @@ -1,13 +1,10 @@ # frozen_string_literal: true class Settings::DeletesController < Settings::BaseController - layout 'admin' + skip_before_action :require_functional! - before_action :check_enabled_deletion - before_action :authenticate_user! before_action :require_not_suspended! - - skip_before_action :require_functional! + before_action :check_enabled_deletion def show @confirmation = Form::DeleteConfirmation.new diff --git a/app/controllers/settings/exports/blocked_accounts_controller.rb b/app/controllers/settings/exports/blocked_accounts_controller.rb index 2092104e0..2190caa36 100644 --- a/app/controllers/settings/exports/blocked_accounts_controller.rb +++ b/app/controllers/settings/exports/blocked_accounts_controller.rb @@ -2,7 +2,7 @@ module Settings module Exports - class BlockedAccountsController < ApplicationController + class BlockedAccountsController < BaseController include ExportControllerConcern def index diff --git a/app/controllers/settings/exports/blocked_domains_controller.rb b/app/controllers/settings/exports/blocked_domains_controller.rb index 6676ce340..bee4b2431 100644 --- a/app/controllers/settings/exports/blocked_domains_controller.rb +++ b/app/controllers/settings/exports/blocked_domains_controller.rb @@ -2,7 +2,7 @@ module Settings module Exports - class BlockedDomainsController < ApplicationController + class BlockedDomainsController < BaseController include ExportControllerConcern def index diff --git a/app/controllers/settings/exports/following_accounts_controller.rb b/app/controllers/settings/exports/following_accounts_controller.rb index 74281ddca..acefcb15d 100644 --- a/app/controllers/settings/exports/following_accounts_controller.rb +++ b/app/controllers/settings/exports/following_accounts_controller.rb @@ -2,7 +2,7 @@ module Settings module Exports - class FollowingAccountsController < ApplicationController + class FollowingAccountsController < BaseController include ExportControllerConcern def index diff --git a/app/controllers/settings/exports/lists_controller.rb b/app/controllers/settings/exports/lists_controller.rb index cf5a9de44..bc65f56a0 100644 --- a/app/controllers/settings/exports/lists_controller.rb +++ b/app/controllers/settings/exports/lists_controller.rb @@ -2,7 +2,7 @@ module Settings module Exports - class ListsController < ApplicationController + class ListsController < BaseController include ExportControllerConcern def index diff --git a/app/controllers/settings/exports/muted_accounts_controller.rb b/app/controllers/settings/exports/muted_accounts_controller.rb index e511619ca..50b7bf1f7 100644 --- a/app/controllers/settings/exports/muted_accounts_controller.rb +++ b/app/controllers/settings/exports/muted_accounts_controller.rb @@ -2,7 +2,7 @@ module Settings module Exports - class MutedAccountsController < ApplicationController + class MutedAccountsController < BaseController include ExportControllerConcern def index diff --git a/app/controllers/settings/exports_controller.rb b/app/controllers/settings/exports_controller.rb index 0e93d07a9..30138d29e 100644 --- a/app/controllers/settings/exports_controller.rb +++ b/app/controllers/settings/exports_controller.rb @@ -3,11 +3,6 @@ class Settings::ExportsController < Settings::BaseController include Authorization - layout 'admin' - - before_action :authenticate_user! - before_action :require_not_suspended! - skip_before_action :require_functional! def show @@ -16,8 +11,6 @@ class Settings::ExportsController < Settings::BaseController end def create - raise Mastodon::NotPermittedError unless user_signed_in? - backup = nil RedisLock.acquire(lock_options) do |lock| @@ -37,8 +30,4 @@ class Settings::ExportsController < Settings::BaseController def lock_options { redis: Redis.current, key: "backup:#{current_user.id}" } end - - def require_not_suspended! - forbidden if current_account.suspended? - end end diff --git a/app/controllers/settings/featured_tags_controller.rb b/app/controllers/settings/featured_tags_controller.rb index e9861da56..e805527d0 100644 --- a/app/controllers/settings/featured_tags_controller.rb +++ b/app/controllers/settings/featured_tags_controller.rb @@ -1,9 +1,6 @@ # frozen_string_literal: true class Settings::FeaturedTagsController < Settings::BaseController - layout 'admin' - - before_action :authenticate_user! before_action :set_featured_tags, only: :index before_action :set_featured_tag, except: [:index, :create] before_action :set_recently_used_tags, only: :index diff --git a/app/controllers/settings/identity_proofs_controller.rb b/app/controllers/settings/identity_proofs_controller.rb index 3a90b7c4d..bf2899da6 100644 --- a/app/controllers/settings/identity_proofs_controller.rb +++ b/app/controllers/settings/identity_proofs_controller.rb @@ -1,9 +1,6 @@ # frozen_string_literal: true class Settings::IdentityProofsController < Settings::BaseController - layout 'admin' - - before_action :authenticate_user! before_action :check_required_params, only: :new def index diff --git a/app/controllers/settings/imports_controller.rb b/app/controllers/settings/imports_controller.rb index 7b8c4ae23..d4516526e 100644 --- a/app/controllers/settings/imports_controller.rb +++ b/app/controllers/settings/imports_controller.rb @@ -1,9 +1,6 @@ # frozen_string_literal: true class Settings::ImportsController < Settings::BaseController - layout 'admin' - - before_action :authenticate_user! before_action :set_account def show diff --git a/app/controllers/settings/migration/redirects_controller.rb b/app/controllers/settings/migration/redirects_controller.rb index 97193ade0..6d469f384 100644 --- a/app/controllers/settings/migration/redirects_controller.rb +++ b/app/controllers/settings/migration/redirects_controller.rb @@ -1,13 +1,10 @@ # frozen_string_literal: true class Settings::Migration::RedirectsController < Settings::BaseController - layout 'admin' + skip_before_action :require_functional! - before_action :authenticate_user! before_action :require_not_suspended! - skip_before_action :require_functional! - def new @redirect = Form::Redirect.new end @@ -38,8 +35,4 @@ class Settings::Migration::RedirectsController < Settings::BaseController def resource_params params.require(:form_redirect).permit(:acct, :current_password, :current_username) end - - def require_not_suspended! - forbidden if current_account.suspended? - end end diff --git a/app/controllers/settings/migrations_controller.rb b/app/controllers/settings/migrations_controller.rb index 68304bb51..62603aba8 100644 --- a/app/controllers/settings/migrations_controller.rb +++ b/app/controllers/settings/migrations_controller.rb @@ -1,15 +1,12 @@ # frozen_string_literal: true class Settings::MigrationsController < Settings::BaseController - layout 'admin' + skip_before_action :require_functional! - before_action :authenticate_user! before_action :require_not_suspended! before_action :set_migrations before_action :set_cooldown - skip_before_action :require_functional! - def show @migration = current_account.migrations.build end @@ -44,8 +41,4 @@ class Settings::MigrationsController < Settings::BaseController def on_cooldown? @cooldown.present? end - - def require_not_suspended! - forbidden if current_account.suspended? - end end diff --git a/app/controllers/settings/pictures_controller.rb b/app/controllers/settings/pictures_controller.rb index df2a6eed3..28df65f8f 100644 --- a/app/controllers/settings/pictures_controller.rb +++ b/app/controllers/settings/pictures_controller.rb @@ -2,7 +2,6 @@ module Settings class PicturesController < BaseController - before_action :authenticate_user! before_action :set_account before_action :set_picture diff --git a/app/controllers/settings/preferences_controller.rb b/app/controllers/settings/preferences_controller.rb index bac9b329d..be4dc904d 100644 --- a/app/controllers/settings/preferences_controller.rb +++ b/app/controllers/settings/preferences_controller.rb @@ -1,10 +1,6 @@ # frozen_string_literal: true class Settings::PreferencesController < Settings::BaseController - layout 'admin' - - before_action :authenticate_user! - def show; end def update diff --git a/app/controllers/settings/profiles_controller.rb b/app/controllers/settings/profiles_controller.rb index 19a7ce157..0c15447a6 100644 --- a/app/controllers/settings/profiles_controller.rb +++ b/app/controllers/settings/profiles_controller.rb @@ -1,9 +1,6 @@ # frozen_string_literal: true class Settings::ProfilesController < Settings::BaseController - layout 'admin' - - before_action :authenticate_user! before_action :set_account def show diff --git a/app/controllers/settings/sessions_controller.rb b/app/controllers/settings/sessions_controller.rb index df5ace803..ee2fc5dc8 100644 --- a/app/controllers/settings/sessions_controller.rb +++ b/app/controllers/settings/sessions_controller.rb @@ -1,11 +1,11 @@ # frozen_string_literal: true class Settings::SessionsController < Settings::BaseController - before_action :authenticate_user! - before_action :set_session, only: :destroy - skip_before_action :require_functional! + before_action :require_not_suspended! + before_action :set_session, only: :destroy + def destroy @session.destroy! flash[:notice] = I18n.t('sessions.revoke_success') diff --git a/app/controllers/settings/two_factor_authentication/confirmations_controller.rb b/app/controllers/settings/two_factor_authentication/confirmations_controller.rb index 9f23011a7..1a0afe58b 100644 --- a/app/controllers/settings/two_factor_authentication/confirmations_controller.rb +++ b/app/controllers/settings/two_factor_authentication/confirmations_controller.rb @@ -5,14 +5,11 @@ module Settings class ConfirmationsController < BaseController include ChallengableConcern - layout 'admin' + skip_before_action :require_functional! - before_action :authenticate_user! before_action :require_challenge! before_action :ensure_otp_secret - skip_before_action :require_functional! - def new prepare_two_factor_form end diff --git a/app/controllers/settings/two_factor_authentication/otp_authentication_controller.rb b/app/controllers/settings/two_factor_authentication/otp_authentication_controller.rb index 6836f7ef6..cbba842a9 100644 --- a/app/controllers/settings/two_factor_authentication/otp_authentication_controller.rb +++ b/app/controllers/settings/two_factor_authentication/otp_authentication_controller.rb @@ -5,14 +5,11 @@ module Settings class OtpAuthenticationController < BaseController include ChallengableConcern - layout 'admin' + skip_before_action :require_functional! - before_action :authenticate_user! before_action :verify_otp_not_enabled, only: [:show] before_action :require_challenge!, only: [:create] - skip_before_action :require_functional! - def show @confirmation = Form::TwoFactorConfirmation.new end diff --git a/app/controllers/settings/two_factor_authentication/recovery_codes_controller.rb b/app/controllers/settings/two_factor_authentication/recovery_codes_controller.rb index 0c4f5bff7..6ec53224d 100644 --- a/app/controllers/settings/two_factor_authentication/recovery_codes_controller.rb +++ b/app/controllers/settings/two_factor_authentication/recovery_codes_controller.rb @@ -5,13 +5,10 @@ module Settings class RecoveryCodesController < BaseController include ChallengableConcern - layout 'admin' + skip_before_action :require_functional! - before_action :authenticate_user! before_action :require_challenge!, on: :create - skip_before_action :require_functional! - def create @recovery_codes = current_user.generate_otp_backup_codes! current_user.save! diff --git a/app/controllers/settings/two_factor_authentication/webauthn_credentials_controller.rb b/app/controllers/settings/two_factor_authentication/webauthn_credentials_controller.rb index a19c604f3..1c557092b 100644 --- a/app/controllers/settings/two_factor_authentication/webauthn_credentials_controller.rb +++ b/app/controllers/settings/two_factor_authentication/webauthn_credentials_controller.rb @@ -3,9 +3,8 @@ module Settings module TwoFactorAuthentication class WebauthnCredentialsController < BaseController - layout 'admin' + skip_before_action :require_functional! - before_action :authenticate_user! before_action :require_otp_enabled before_action :require_webauthn_enabled, only: [:index, :destroy] diff --git a/app/controllers/settings/two_factor_authentication_methods_controller.rb b/app/controllers/settings/two_factor_authentication_methods_controller.rb index 224d3a45c..205933ea8 100644 --- a/app/controllers/settings/two_factor_authentication_methods_controller.rb +++ b/app/controllers/settings/two_factor_authentication_methods_controller.rb @@ -4,14 +4,11 @@ module Settings class TwoFactorAuthenticationMethodsController < BaseController include ChallengableConcern - layout 'admin' + skip_before_action :require_functional! - before_action :authenticate_user! before_action :require_challenge!, only: :disable before_action :require_otp_enabled - skip_before_action :require_functional! - def index; end def disable diff --git a/app/views/auth/registrations/_sessions.html.haml b/app/views/auth/registrations/_sessions.html.haml index 395e36a9f..d3a04c00e 100644 --- a/app/views/auth/registrations/_sessions.html.haml +++ b/app/views/auth/registrations/_sessions.html.haml @@ -27,5 +27,5 @@ - else %time.time-ago{ datetime: session.updated_at.iso8601, title: l(session.updated_at) }= l(session.updated_at) %td - - if current_session.session_id != session.session_id + - if current_session.session_id != session.session_id && !current_account.suspended? = table_link_to 'times', t('sessions.revoke'), settings_session_path(session), method: :delete diff --git a/app/views/auth/registrations/edit.html.haml b/app/views/auth/registrations/edit.html.haml index 4a46b27a9..a3445b421 100644 --- a/app/views/auth/registrations/edit.html.haml +++ b/app/views/auth/registrations/edit.html.haml @@ -30,18 +30,19 @@ = render 'sessions' -%hr.spacer/ +- unless current_account.suspended? + %hr.spacer/ -%h3= t('auth.migrate_account') -%p.muted-hint= t('auth.migrate_account_html', path: settings_migration_path) + %h3= t('auth.migrate_account') + %p.muted-hint= t('auth.migrate_account_html', path: settings_migration_path) -%hr.spacer/ + %hr.spacer/ -%h3= t('migrations.incoming_migrations') -%p.muted-hint= t('migrations.incoming_migrations_html', path: settings_aliases_path) + %h3= t('migrations.incoming_migrations') + %p.muted-hint= t('migrations.incoming_migrations_html', path: settings_aliases_path) -- if open_deletion? && !current_account.suspended? - %hr.spacer/ + - if open_deletion? + %hr.spacer/ - %h3= t('auth.delete_account') - %p.muted-hint= t('auth.delete_account_html', path: settings_delete_path) + %h3= t('auth.delete_account') + %p.muted-hint= t('auth.delete_account_html', path: settings_delete_path) diff --git a/app/views/oauth/authorized_applications/index.html.haml b/app/views/oauth/authorized_applications/index.html.haml index 7b77108a9..fbb733db4 100644 --- a/app/views/oauth/authorized_applications/index.html.haml +++ b/app/views/oauth/authorized_applications/index.html.haml @@ -20,5 +20,5 @@ %th!= application.scopes.map { |scope| t(scope, scope: [:doorkeeper, :scopes]) }.join(', ') %td= l application.created_at %td - - unless application.superapp? + - unless application.superapp? || current_account.suspended? = table_link_to 'times', t('doorkeeper.authorized_applications.buttons.revoke'), oauth_authorized_application_path(application), method: :delete, data: { confirm: t('doorkeeper.authorized_applications.confirmations.revoke') } diff --git a/config/navigation.rb b/config/navigation.rb index ece41d4bf..c113a3c3e 100644 --- a/config/navigation.rb +++ b/config/navigation.rb @@ -21,7 +21,7 @@ SimpleNavigation::Configuration.run do |navigation| n.item :security, safe_join([fa_icon('lock fw'), t('settings.account')]), edit_user_registration_url do |s| s.item :password, safe_join([fa_icon('lock fw'), t('settings.account_settings')]), edit_user_registration_url, highlights_on: %r{/auth/edit|/settings/delete|/settings/migration|/settings/aliases} - s.item :two_factor_authentication, safe_join([fa_icon('mobile fw'), t('settings.two_factor_authentication')]), settings_two_factor_authentication_methods_url, highlights_on: %r{/settings/two_factor_authentication|/settings/security_keys} + s.item :two_factor_authentication, safe_join([fa_icon('mobile fw'), t('settings.two_factor_authentication')]), settings_two_factor_authentication_methods_url, highlights_on: %r{/settings/two_factor_authentication|/settings/otp_authentication|/settings/security_keys} s.item :authorized_apps, safe_join([fa_icon('list fw'), t('settings.authorized_apps')]), oauth_authorized_applications_url end diff --git a/spec/controllers/settings/deletes_controller_spec.rb b/spec/controllers/settings/deletes_controller_spec.rb index 996872efd..8d5c4774f 100644 --- a/spec/controllers/settings/deletes_controller_spec.rb +++ b/spec/controllers/settings/deletes_controller_spec.rb @@ -77,26 +77,26 @@ describe Settings::DeletesController do expect(response).to redirect_to settings_delete_path end end - end - context 'when not signed in' do - it 'redirects' do - delete :destroy - expect(response).to redirect_to '/auth/sign_in' - end - end + context 'when account deletions are disabled' do + around do |example| + open_deletion = Setting.open_deletion + example.run + Setting.open_deletion = open_deletion + end - context do - around do |example| - open_deletion = Setting.open_deletion - example.run - Setting.open_deletion = open_deletion + it 'redirects' do + Setting.open_deletion = false + delete :destroy + expect(response).to redirect_to root_path + end end + end + context 'when not signed in' do it 'redirects' do - Setting.open_deletion = false delete :destroy - expect(response).to redirect_to root_path + expect(response).to redirect_to '/auth/sign_in' end end end -- cgit From cd4ec7cd74c0975c7ff9aa832ed7e1bb10966439 Mon Sep 17 00:00:00 2001 From: ThibG Date: Mon, 14 Sep 2020 13:04:29 +0200 Subject: Do not serve account actors at all in limited federation mode (#14800) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Do not serve account actors at all in limited federation mode When an account is fetched without a signature from an allowed instance, return an error. This isn't really an improvement in security, as the only information that was previously returned was required protocol-level info, and the only personal bit was the existence of the account. The existence of the account can still be checked by issuing a webfinger query, as those are accepted without signatures. However, this change makes it so that unallowed instances won't create account records on their end when they find a reference to an unknown account. The previous behavior of rendering a limited list of fields, instead of not rendering the actor at all, was in order to prevent situations in which two instances in Authorized Fetch mode or Limited Federation mode would fail to reach each other because resolving an account would require a signed query… from an account which can only be fetched with a signed query itself. However, this should now be fine as fetching accounts is done by signing on behalf of the special instance actor, which does not require any kind of valid signature to be fetched. * Fix tests --- app/controllers/accounts_controller.rb | 11 ++--------- spec/controllers/accounts_controller_spec.rb | 20 ++------------------ 2 files changed, 4 insertions(+), 27 deletions(-) (limited to 'app/controllers') diff --git a/app/controllers/accounts_controller.rb b/app/controllers/accounts_controller.rb index d97d88fd9..6d711afd0 100644 --- a/app/controllers/accounts_controller.rb +++ b/app/controllers/accounts_controller.rb @@ -7,6 +7,7 @@ class AccountsController < ApplicationController include AccountControllerConcern include SignatureAuthentication + before_action :require_signature!, if: -> { request.format == :json && authorized_fetch_mode? } before_action :set_cache_headers before_action :set_body_classes @@ -48,7 +49,7 @@ class AccountsController < ApplicationController format.json do expires_in 3.minutes, public: !(authorized_fetch_mode? && signed_request_account.present?) - render_with_cache json: @account, content_type: 'application/activity+json', serializer: ActivityPub::ActorSerializer, adapter: ActivityPub::Adapter, fields: restrict_fields_to + render_with_cache json: @account, content_type: 'application/activity+json', serializer: ActivityPub::ActorSerializer, adapter: ActivityPub::Adapter end end end @@ -153,12 +154,4 @@ class AccountsController < ApplicationController def params_slice(*keys) params.slice(*keys).permit(*keys) end - - def restrict_fields_to - if signed_request_account.present? || public_fetch_mode? - # Return all fields - else - %i(id type preferred_username inbox public_key endpoints) - end - end end diff --git a/spec/controllers/accounts_controller_spec.rb b/spec/controllers/accounts_controller_spec.rb index 93bf2c83f..b04f4650b 100644 --- a/spec/controllers/accounts_controller_spec.rb +++ b/spec/controllers/accounts_controller_spec.rb @@ -348,24 +348,8 @@ RSpec.describe AccountsController, type: :controller do context 'in authorized fetch mode' do let(:authorized_fetch_mode) { true } - it 'returns http success' do - expect(response).to have_http_status(200) - end - - it 'returns application/activity+json' do - expect(response.content_type).to eq 'application/activity+json' - end - - it_behaves_like 'cachable response' - - it 'returns Vary header with Signature' do - expect(response.headers['Vary']).to include 'Signature' - end - - it 'renders bare minimum account' do - json = body_as_json - expect(json).to include(:id, :type, :preferredUsername, :inbox, :publicKey) - expect(json).to_not include(:name, :summary) + it 'returns http unauthorized' do + expect(response).to have_http_status(401) end end end -- cgit From ed099d8bdc5b3d9e7df7ce5358441887e6bb7e48 Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Tue, 15 Sep 2020 14:37:58 +0200 Subject: Change account suspensions to be reversible by default (#14726) --- app/controllers/admin/accounts_controller.rb | 31 ++-- app/controllers/api/base_controller.rb | 4 +- .../api/v1/admin/accounts_controller.rb | 9 +- app/controllers/settings/deletes_controller.rb | 2 +- app/lib/activitypub/activity/delete.rb | 2 +- app/mailers/notification_mailer.rb | 16 +- app/mailers/user_mailer.rb | 28 ++-- app/models/account.rb | 9 +- app/models/account_deletion_request.rb | 20 +++ app/models/admin/account_action.rb | 2 +- app/models/concerns/account_associations.rb | 3 + app/models/form/account_batch.rb | 2 +- app/models/invite.rb | 2 +- app/models/user.rb | 4 +- app/policies/account_policy.rb | 4 + app/services/after_unallow_domain_service.rb | 2 +- app/services/block_domain_service.rb | 2 +- app/services/delete_account_service.rb | 180 ++++++++++++++++++++ app/services/suspend_account_service.rb | 183 ++++----------------- app/services/unsuspend_account_service.rb | 52 ++++++ app/views/admin/accounts/show.html.haml | 114 ++++++------- app/workers/account_deletion_worker.rb | 13 ++ app/workers/admin/account_deletion_worker.rb | 13 ++ app/workers/admin/suspension_worker.rb | 6 +- app/workers/admin/unsuspension_worker.rb | 13 ++ app/workers/scheduler/user_cleanup_scheduler.rb | 13 ++ config/locales/en.yml | 31 +++- config/locales/simple_form.en.yml | 8 +- config/routes.rb | 4 +- ...00908193330_create_account_deletion_requests.rb | 8 + db/schema.rb | 10 +- lib/mastodon/accounts_cli.rb | 4 +- lib/mastodon/domains_cli.rb | 2 +- .../auth/registrations_controller_spec.rb | 3 +- .../concerns/export_controller_concern_spec.rb | 1 + .../account_deletion_request_fabricator.rb | 3 + spec/models/account_deletion_request_spec.rb | 4 + spec/models/invite_spec.rb | 2 +- spec/services/delete_account_service_spec.rb | 84 ++++++++++ spec/services/suspend_account_service_spec.rb | 84 ---------- 40 files changed, 612 insertions(+), 365 deletions(-) create mode 100644 app/models/account_deletion_request.rb create mode 100644 app/services/delete_account_service.rb create mode 100644 app/services/unsuspend_account_service.rb create mode 100644 app/workers/account_deletion_worker.rb create mode 100644 app/workers/admin/account_deletion_worker.rb create mode 100644 app/workers/admin/unsuspension_worker.rb create mode 100644 db/migrate/20200908193330_create_account_deletion_requests.rb create mode 100644 spec/fabricators/account_deletion_request_fabricator.rb create mode 100644 spec/models/account_deletion_request_spec.rb create mode 100644 spec/services/delete_account_service_spec.rb delete mode 100644 spec/services/suspend_account_service_spec.rb (limited to 'app/controllers') diff --git a/app/controllers/admin/accounts_controller.rb b/app/controllers/admin/accounts_controller.rb index 7b1783542..b9b75727d 100644 --- a/app/controllers/admin/accounts_controller.rb +++ b/app/controllers/admin/accounts_controller.rb @@ -2,7 +2,7 @@ module Admin class AccountsController < BaseController - before_action :set_account, only: [:show, :redownload, :remove_avatar, :remove_header, :enable, :unsilence, :unsuspend, :memorialize, :approve, :reject] + before_action :set_account, except: [:index] before_action :require_remote_account!, only: [:redownload] before_action :require_local_account!, only: [:enable, :memorialize, :approve, :reject] @@ -14,49 +14,58 @@ module Admin def show authorize @account, :show? + @deletion_request = @account.deletion_request @account_moderation_note = current_account.account_moderation_notes.new(target_account: @account) @moderation_notes = @account.targeted_moderation_notes.latest @warnings = @account.targeted_account_warnings.latest.custom + @domain_block = DomainBlock.rule_for(@account.domain) end def memorialize authorize @account, :memorialize? @account.memorialize! log_action :memorialize, @account - redirect_to admin_account_path(@account.id) + redirect_to admin_account_path(@account.id), notice: I18n.t('admin.accounts.memorialized_msg', username: @account.acct) end def enable authorize @account.user, :enable? @account.user.enable! log_action :enable, @account.user - redirect_to admin_account_path(@account.id) + redirect_to admin_account_path(@account.id), notice: I18n.t('admin.accounts.enabled_msg', username: @account.acct) end def approve authorize @account.user, :approve? @account.user.approve! - redirect_to admin_pending_accounts_path + redirect_to admin_pending_accounts_path, notice: I18n.t('admin.accounts.approved_msg', username: @account.acct) end def reject authorize @account.user, :reject? - SuspendAccountService.new.call(@account, reserve_email: false, reserve_username: false) - redirect_to admin_pending_accounts_path + DeleteAccountService.new.call(@account, reserve_email: false, reserve_username: false) + redirect_to admin_pending_accounts_path, notice: I18n.t('admin.accounts.rejected_msg', username: @account.acct) + end + + def destroy + authorize @account, :destroy? + Admin::AccountDeletionWorker.perform_async(@account.id) + redirect_to admin_account_path(@account.id), notice: I18n.t('admin.accounts.destroyed_msg', username: @account.acct) end def unsilence authorize @account, :unsilence? @account.unsilence! log_action :unsilence, @account - redirect_to admin_account_path(@account.id) + redirect_to admin_account_path(@account.id), notice: I18n.t('admin.accounts.unsilenced_msg', username: @account.acct) end def unsuspend authorize @account, :unsuspend? @account.unsuspend! + Admin::UnsuspensionWorker.perform_async(@account.id) log_action :unsuspend, @account - redirect_to admin_account_path(@account.id) + redirect_to admin_account_path(@account.id), notice: I18n.t('admin.accounts.unsuspended_msg', username: @account.acct) end def redownload @@ -65,7 +74,7 @@ module Admin @account.update!(last_webfingered_at: nil) ResolveAccountService.new.call(@account) - redirect_to admin_account_path(@account.id) + redirect_to admin_account_path(@account.id), notice: I18n.t('admin.accounts.redownloaded_msg', username: @account.acct) end def remove_avatar @@ -76,7 +85,7 @@ module Admin log_action :remove_avatar, @account.user - redirect_to admin_account_path(@account.id) + redirect_to admin_account_path(@account.id), notice: I18n.t('admin.accounts.removed_avatar_msg', username: @account.acct) end def remove_header @@ -87,7 +96,7 @@ module Admin log_action :remove_header, @account.user - redirect_to admin_account_path(@account.id) + redirect_to admin_account_path(@account.id), notice: I18n.t('admin.accounts.removed_header_msg', username: @account.acct) end private diff --git a/app/controllers/api/base_controller.rb b/app/controllers/api/base_controller.rb index 467225547..e962c4e97 100644 --- a/app/controllers/api/base_controller.rb +++ b/app/controllers/api/base_controller.rb @@ -96,12 +96,12 @@ class Api::BaseController < ApplicationController def require_user! if !current_user render json: { error: 'This method requires an authenticated user' }, status: 422 - elsif current_user.disabled? - render json: { error: 'Your login is currently disabled' }, status: 403 elsif !current_user.confirmed? render json: { error: 'Your login is missing a confirmed e-mail address' }, status: 403 elsif !current_user.approved? render json: { error: 'Your login is currently pending approval' }, status: 403 + elsif !current_user.functional? + render json: { error: 'Your login is currently disabled' }, status: 403 else set_user_activity end diff --git a/app/controllers/api/v1/admin/accounts_controller.rb b/app/controllers/api/v1/admin/accounts_controller.rb index 24c7fbef1..3af572f25 100644 --- a/app/controllers/api/v1/admin/accounts_controller.rb +++ b/app/controllers/api/v1/admin/accounts_controller.rb @@ -58,7 +58,13 @@ class Api::V1::Admin::AccountsController < Api::BaseController def reject authorize @account.user, :reject? - SuspendAccountService.new.call(@account, reserve_email: false, reserve_username: false) + DeleteAccountService.new.call(@account, reserve_email: false, reserve_username: false) + render json: @account, serializer: REST::Admin::AccountSerializer + end + + def destroy + authorize @account, :destroy? + Admin::AccountDeletionWorker.perform_async(@account.id) render json: @account, serializer: REST::Admin::AccountSerializer end @@ -72,6 +78,7 @@ class Api::V1::Admin::AccountsController < Api::BaseController def unsuspend authorize @account, :unsuspend? @account.unsuspend! + Admin::UnsuspensionWorker.perform_async(@account.id) log_action :unsuspend, @account render json: @account, serializer: REST::Admin::AccountSerializer end diff --git a/app/controllers/settings/deletes_controller.rb b/app/controllers/settings/deletes_controller.rb index 7d4844e60..f96c83b80 100644 --- a/app/controllers/settings/deletes_controller.rb +++ b/app/controllers/settings/deletes_controller.rb @@ -43,7 +43,7 @@ class Settings::DeletesController < Settings::BaseController def destroy_account! current_account.suspend! - Admin::SuspensionWorker.perform_async(current_user.account_id, true) + AccountDeletionWorker.perform_async(current_user.account_id) sign_out end end diff --git a/app/lib/activitypub/activity/delete.rb b/app/lib/activitypub/activity/delete.rb index dc9ff580c..09b9e5e0e 100644 --- a/app/lib/activitypub/activity/delete.rb +++ b/app/lib/activitypub/activity/delete.rb @@ -13,7 +13,7 @@ class ActivityPub::Activity::Delete < ActivityPub::Activity def delete_person lock_or_return("delete_in_progress:#{@account.id}") do - SuspendAccountService.new.call(@account, reserve_username: false) + DeleteAccountService.new.call(@account, reserve_username: false) end end diff --git a/app/mailers/notification_mailer.rb b/app/mailers/notification_mailer.rb index 9d8a7886c..54db892cc 100644 --- a/app/mailers/notification_mailer.rb +++ b/app/mailers/notification_mailer.rb @@ -10,7 +10,7 @@ class NotificationMailer < ApplicationMailer @me = recipient @status = notification.target_status - return if @me.user.disabled? || @status.nil? + return unless @me.user.functional? && @status.present? locale_for_account(@me) do thread_by_conversation(@status.conversation) @@ -22,7 +22,7 @@ class NotificationMailer < ApplicationMailer @me = recipient @account = notification.from_account - return if @me.user.disabled? + return unless @me.user.functional? locale_for_account(@me) do mail to: @me.user.email, subject: I18n.t('notification_mailer.follow.subject', name: @account.acct) @@ -34,7 +34,7 @@ class NotificationMailer < ApplicationMailer @account = notification.from_account @status = notification.target_status - return if @me.user.disabled? || @status.nil? + return unless @me.user.functional? && @status.present? locale_for_account(@me) do thread_by_conversation(@status.conversation) @@ -47,7 +47,7 @@ class NotificationMailer < ApplicationMailer @account = notification.from_account @status = notification.target_status - return if @me.user.disabled? || @status.nil? + return unless @me.user.functional? && @status.present? locale_for_account(@me) do thread_by_conversation(@status.conversation) @@ -59,7 +59,7 @@ class NotificationMailer < ApplicationMailer @me = recipient @account = notification.from_account - return if @me.user.disabled? + return unless @me.user.functional? locale_for_account(@me) do mail to: @me.user.email, subject: I18n.t('notification_mailer.follow_request.subject', name: @account.acct) @@ -67,7 +67,7 @@ class NotificationMailer < ApplicationMailer end def digest(recipient, **opts) - return if recipient.user.disabled? + return unless recipient.user.functional? @me = recipient @since = opts[:since] || [@me.user.last_emailed_at, (@me.user.current_sign_in_at + 1.day)].compact.max @@ -88,8 +88,10 @@ class NotificationMailer < ApplicationMailer def thread_by_conversation(conversation) return if conversation.nil? + msg_id = "" + headers['In-Reply-To'] = msg_id - headers['References'] = msg_id + headers['References'] = msg_id end end diff --git a/app/mailers/user_mailer.rb b/app/mailers/user_mailer.rb index b55768551..95996ba3f 100644 --- a/app/mailers/user_mailer.rb +++ b/app/mailers/user_mailer.rb @@ -15,7 +15,7 @@ class UserMailer < Devise::Mailer @token = token @instance = Rails.configuration.x.local_domain - return if @resource.disabled? + return unless @resource.active_for_authentication? I18n.with_locale(@resource.locale || I18n.default_locale) do mail to: @resource.unconfirmed_email.presence || @resource.email, @@ -29,7 +29,7 @@ class UserMailer < Devise::Mailer @token = token @instance = Rails.configuration.x.local_domain - return if @resource.disabled? + return unless @resource.active_for_authentication? I18n.with_locale(@resource.locale || I18n.default_locale) do mail to: @resource.email, subject: I18n.t('devise.mailer.reset_password_instructions.subject') @@ -40,7 +40,7 @@ class UserMailer < Devise::Mailer @resource = user @instance = Rails.configuration.x.local_domain - return if @resource.disabled? + return unless @resource.active_for_authentication? I18n.with_locale(@resource.locale || I18n.default_locale) do mail to: @resource.email, subject: I18n.t('devise.mailer.password_change.subject') @@ -51,7 +51,7 @@ class UserMailer < Devise::Mailer @resource = user @instance = Rails.configuration.x.local_domain - return if @resource.disabled? + return unless @resource.active_for_authentication? I18n.with_locale(@resource.locale || I18n.default_locale) do mail to: @resource.email, subject: I18n.t('devise.mailer.email_changed.subject') @@ -62,7 +62,7 @@ class UserMailer < Devise::Mailer @resource = user @instance = Rails.configuration.x.local_domain - return if @resource.disabled? + return unless @resource.active_for_authentication? I18n.with_locale(@resource.locale || I18n.default_locale) do mail to: @resource.email, subject: I18n.t('devise.mailer.two_factor_enabled.subject') @@ -73,7 +73,7 @@ class UserMailer < Devise::Mailer @resource = user @instance = Rails.configuration.x.local_domain - return if @resource.disabled? + return unless @resource.active_for_authentication? I18n.with_locale(@resource.locale || I18n.default_locale) do mail to: @resource.email, subject: I18n.t('devise.mailer.two_factor_disabled.subject') @@ -84,7 +84,7 @@ class UserMailer < Devise::Mailer @resource = user @instance = Rails.configuration.x.local_domain - return if @resource.disabled? + return unless @resource.active_for_authentication? I18n.with_locale(@resource.locale || I18n.default_locale) do mail to: @resource.email, subject: I18n.t('devise.mailer.two_factor_recovery_codes_changed.subject') @@ -95,7 +95,7 @@ class UserMailer < Devise::Mailer @resource = user @instance = Rails.configuration.x.local_domain - return if @resource.disabled? + return unless @resource.active_for_authentication? I18n.with_locale(@resource.locale || I18n.default_locale) do mail to: @resource.email, subject: I18n.t('devise.mailer.webauthn_enabled.subject') @@ -106,7 +106,7 @@ class UserMailer < Devise::Mailer @resource = user @instance = Rails.configuration.x.local_domain - return if @resource.disabled? + return unless @resource.active_for_authentication? I18n.with_locale(@resource.locale || I18n.default_locale) do mail to: @resource.email, subject: I18n.t('devise.mailer.webauthn_disabled.subject') @@ -118,7 +118,7 @@ class UserMailer < Devise::Mailer @instance = Rails.configuration.x.local_domain @webauthn_credential = webauthn_credential - return if @resource.disabled? + return unless @resource.active_for_authentication? I18n.with_locale(@resource.locale || I18n.default_locale) do mail to: @resource.email, subject: I18n.t('devise.mailer.webauthn_credential.added.subject') @@ -130,7 +130,7 @@ class UserMailer < Devise::Mailer @instance = Rails.configuration.x.local_domain @webauthn_credential = webauthn_credential - return if @resource.disabled? + return unless @resource.active_for_authentication? I18n.with_locale(@resource.locale || I18n.default_locale) do mail to: @resource.email, subject: I18n.t('devise.mailer.webauthn_credential.deleted.subject') @@ -141,7 +141,7 @@ class UserMailer < Devise::Mailer @resource = user @instance = Rails.configuration.x.local_domain - return if @resource.disabled? + return unless @resource.active_for_authentication? I18n.with_locale(@resource.locale || I18n.default_locale) do mail to: @resource.email, subject: I18n.t('user_mailer.welcome.subject') @@ -153,7 +153,7 @@ class UserMailer < Devise::Mailer @instance = Rails.configuration.x.local_domain @backup = backup - return if @resource.disabled? + return unless @resource.active_for_authentication? I18n.with_locale(@resource.locale || I18n.default_locale) do mail to: @resource.email, subject: I18n.t('user_mailer.backup_ready.subject') @@ -181,7 +181,7 @@ class UserMailer < Devise::Mailer @detection = Browser.new(user_agent) @timestamp = timestamp.to_time.utc - return if @resource.disabled? + return unless @resource.active_for_authentication? I18n.with_locale(@resource.locale || I18n.default_locale) do mail to: @resource.email, diff --git a/app/models/account.rb b/app/models/account.rb index 6b7ebda9e..5acc8d621 100644 --- a/app/models/account.rb +++ b/app/models/account.rb @@ -222,23 +222,20 @@ class Account < ApplicationRecord def suspend!(date = Time.now.utc) transaction do - user&.disable! if local? + create_deletion_request! update!(suspended_at: date) end end def unsuspend! transaction do - user&.enable! if local? + deletion_request&.destroy! update!(suspended_at: nil) end end def memorialize! - transaction do - user&.disable! if local? - update!(memorial: true) - end + update!(memorial: true) end def sign? diff --git a/app/models/account_deletion_request.rb b/app/models/account_deletion_request.rb new file mode 100644 index 000000000..7d0c346cc --- /dev/null +++ b/app/models/account_deletion_request.rb @@ -0,0 +1,20 @@ +# frozen_string_literal: true + +# == Schema Information +# +# Table name: account_deletion_requests +# +# id :bigint(8) not null, primary key +# account_id :bigint(8) +# created_at :datetime not null +# updated_at :datetime not null +# +class AccountDeletionRequest < ApplicationRecord + DELAY_TO_DELETION = 30.days.freeze + + belongs_to :account + + def due_at + created_at + DELAY_TO_DELETION + end +end diff --git a/app/models/admin/account_action.rb b/app/models/admin/account_action.rb index 9edd152f5..c4ac09520 100644 --- a/app/models/admin/account_action.rb +++ b/app/models/admin/account_action.rb @@ -134,7 +134,7 @@ class Admin::AccountAction end def process_email! - UserMailer.warning(target_account.user, warning, status_ids).deliver_now! if warnable? + UserMailer.warning(target_account.user, warning, status_ids).deliver_later! if warnable? end def warnable? diff --git a/app/models/concerns/account_associations.rb b/app/models/concerns/account_associations.rb index cca3a17fa..98849f8fc 100644 --- a/app/models/concerns/account_associations.rb +++ b/app/models/concerns/account_associations.rb @@ -60,5 +60,8 @@ module AccountAssociations # Hashtags has_and_belongs_to_many :tags has_many :featured_tags, -> { includes(:tag) }, dependent: :destroy, inverse_of: :account + + # Account deletion requests + has_one :deletion_request, class_name: 'AccountDeletionRequest', inverse_of: :account, dependent: :destroy end end diff --git a/app/models/form/account_batch.rb b/app/models/form/account_batch.rb index 0b285fde9..7b9e40f68 100644 --- a/app/models/form/account_batch.rb +++ b/app/models/form/account_batch.rb @@ -69,6 +69,6 @@ class Form::AccountBatch records = accounts.includes(:user) records.each { |account| authorize(account.user, :reject?) } - .each { |account| SuspendAccountService.new.call(account, reserve_email: false, reserve_username: false) } + .each { |account| DeleteAccountService.new.call(account, reserve_email: false, reserve_username: false) } end end diff --git a/app/models/invite.rb b/app/models/invite.rb index 29d25eae8..7ea4e2f98 100644 --- a/app/models/invite.rb +++ b/app/models/invite.rb @@ -28,7 +28,7 @@ class Invite < ApplicationRecord before_validation :set_code def valid_for_use? - (max_uses.nil? || uses < max_uses) && !expired? && !(user.nil? || user.disabled?) + (max_uses.nil? || uses < max_uses) && !expired? && user&.functional? end private diff --git a/app/models/user.rb b/app/models/user.rb index dbee08988..6b21d6ed6 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -168,7 +168,7 @@ class User < ApplicationRecord end def active_for_authentication? - true + !account.memorial? end def suspicious_sign_in?(ip) @@ -176,7 +176,7 @@ class User < ApplicationRecord end def functional? - confirmed? && approved? && !disabled? && !account.suspended? && account.moved_to_account_id.nil? + confirmed? && approved? && !disabled? && !account.suspended? && !account.memorial? && account.moved_to_account_id.nil? end def unconfirmed_or_pending? diff --git a/app/policies/account_policy.rb b/app/policies/account_policy.rb index 9c145979d..1b105e92a 100644 --- a/app/policies/account_policy.rb +++ b/app/policies/account_policy.rb @@ -17,6 +17,10 @@ class AccountPolicy < ApplicationPolicy staff? && !record.user&.staff? end + def destroy? + record.suspended? && record.deletion_request.present? && admin? + end + def unsuspend? staff? end diff --git a/app/services/after_unallow_domain_service.rb b/app/services/after_unallow_domain_service.rb index ccd0b8ae9..d3008a105 100644 --- a/app/services/after_unallow_domain_service.rb +++ b/app/services/after_unallow_domain_service.rb @@ -3,7 +3,7 @@ class AfterUnallowDomainService < BaseService def call(domain) Account.where(domain: domain).find_each do |account| - SuspendAccountService.new.call(account, reserve_username: false) + DeleteAccountService.new.call(account, reserve_username: false) end end end diff --git a/app/services/block_domain_service.rb b/app/services/block_domain_service.rb index dc23ef8d8..1cf3382b3 100644 --- a/app/services/block_domain_service.rb +++ b/app/services/block_domain_service.rb @@ -36,7 +36,7 @@ class BlockDomainService < BaseService def suspend_accounts! blocked_domain_accounts.without_suspended.in_batches.update_all(suspended_at: @domain_block.created_at) blocked_domain_accounts.where(suspended_at: @domain_block.created_at).reorder(nil).find_each do |account| - SuspendAccountService.new.call(account, reserve_username: true, suspended_at: @domain_block.created_at) + DeleteAccountService.new.call(account, reserve_username: true, suspended_at: @domain_block.created_at) end end diff --git a/app/services/delete_account_service.rb b/app/services/delete_account_service.rb new file mode 100644 index 000000000..15bdd13e3 --- /dev/null +++ b/app/services/delete_account_service.rb @@ -0,0 +1,180 @@ +# frozen_string_literal: true + +class DeleteAccountService < BaseService + include Payloadable + + ASSOCIATIONS_ON_SUSPEND = %w( + account_pins + active_relationships + block_relationships + blocked_by_relationships + conversation_mutes + conversations + custom_filters + domain_blocks + favourites + follow_requests + list_accounts + mute_relationships + muted_by_relationships + notifications + owned_lists + passive_relationships + report_notes + scheduled_statuses + status_pins + ).freeze + + ASSOCIATIONS_ON_DESTROY = %w( + reports + targeted_moderation_notes + targeted_reports + ).freeze + + # Suspend or remove an account and remove as much of its data + # as possible. If it's a local account and it has not been confirmed + # or never been approved, then side effects are skipped and both + # the user and account records are removed fully. Otherwise, + # it is controlled by options. + # @param [Account] + # @param [Hash] options + # @option [Boolean] :reserve_email Keep user record. Only applicable for local accounts + # @option [Boolean] :reserve_username Keep account record + # @option [Boolean] :skip_side_effects Side effects are ActivityPub and streaming API payloads + # @option [Time] :suspended_at Only applicable when :reserve_username is true + def call(account, **options) + @account = account + @options = { reserve_username: true, reserve_email: true }.merge(options) + + if @account.local? && @account.user_unconfirmed_or_pending? + @options[:reserve_email] = false + @options[:reserve_username] = false + @options[:skip_side_effects] = true + end + + reject_follows! + purge_user! + purge_profile! + purge_content! + fulfill_deletion_request! + end + + private + + def reject_follows! + return if @account.local? || !@account.activitypub? + + ActivityPub::DeliveryWorker.push_bulk(Follow.where(account: @account)) do |follow| + [build_reject_json(follow), follow.target_account_id, follow.account.inbox_url] + end + end + + def purge_user! + return if !@account.local? || @account.user.nil? + + if @options[:reserve_email] + @account.user.disable! + @account.user.invites.where(uses: 0).destroy_all + else + @account.user.destroy + end + end + + def purge_content! + distribute_delete_actor! if @account.local? && !@options[:skip_side_effects] + + @account.statuses.reorder(nil).find_in_batches do |statuses| + statuses.reject! { |status| reported_status_ids.include?(status.id) } if @options[:reserve_username] + BatchedRemoveStatusService.new.call(statuses, skip_side_effects: @options[:skip_side_effects]) + end + + @account.media_attachments.reorder(nil).find_each do |media_attachment| + next if @options[:reserve_username] && reported_status_ids.include?(media_attachment.status_id) + + media_attachment.destroy + end + + @account.polls.reorder(nil).find_each do |poll| + next if @options[:reserve_username] && reported_status_ids.include?(poll.status_id) + + poll.destroy + end + + associations_for_destruction.each do |association_name| + destroy_all(@account.public_send(association_name)) + end + + @account.destroy unless @options[:reserve_username] + end + + def purge_profile! + # If the account is going to be destroyed + # there is no point wasting time updating + # its values first + + return unless @options[:reserve_username] + + @account.silenced_at = nil + @account.suspended_at = @options[:suspended_at] || Time.now.utc + @account.locked = false + @account.memorial = false + @account.discoverable = false + @account.display_name = '' + @account.note = '' + @account.fields = [] + @account.statuses_count = 0 + @account.followers_count = 0 + @account.following_count = 0 + @account.moved_to_account = nil + @account.trust_level = :untrusted + @account.avatar.destroy + @account.header.destroy + @account.save! + end + + def fulfill_deletion_request! + @account.deletion_request&.destroy + end + + def destroy_all(association) + association.in_batches.destroy_all + end + + def distribute_delete_actor! + ActivityPub::DeliveryWorker.push_bulk(delivery_inboxes) do |inbox_url| + [delete_actor_json, @account.id, inbox_url] + end + + ActivityPub::LowPriorityDeliveryWorker.push_bulk(low_priority_delivery_inboxes) do |inbox_url| + [delete_actor_json, @account.id, inbox_url] + end + end + + def delete_actor_json + @delete_actor_json ||= Oj.dump(serialize_payload(@account, ActivityPub::DeleteActorSerializer, signer: @account)) + end + + def build_reject_json(follow) + Oj.dump(serialize_payload(follow, ActivityPub::RejectFollowSerializer)) + end + + def delivery_inboxes + @delivery_inboxes ||= @account.followers.inboxes + Relay.enabled.pluck(:inbox_url) + end + + def low_priority_delivery_inboxes + Account.inboxes - delivery_inboxes + end + + def reported_status_ids + @reported_status_ids ||= Report.where(target_account: @account).unresolved.pluck(:status_ids).flatten.uniq + end + + def associations_for_destruction + if @options[:reserve_username] + ASSOCIATIONS_ON_SUSPEND + else + ASSOCIATIONS_ON_SUSPEND + ASSOCIATIONS_ON_DESTROY + end + end +end diff --git a/app/services/suspend_account_service.rb b/app/services/suspend_account_service.rb index ecc893931..5a079c3ac 100644 --- a/app/services/suspend_account_service.rb +++ b/app/services/suspend_account_service.rb @@ -1,175 +1,52 @@ # frozen_string_literal: true class SuspendAccountService < BaseService - include Payloadable - - ASSOCIATIONS_ON_SUSPEND = %w( - account_pins - active_relationships - block_relationships - blocked_by_relationships - conversation_mutes - conversations - custom_filters - domain_blocks - favourites - follow_requests - list_accounts - mute_relationships - muted_by_relationships - notifications - owned_lists - passive_relationships - report_notes - scheduled_statuses - status_pins - ).freeze - - ASSOCIATIONS_ON_DESTROY = %w( - reports - targeted_moderation_notes - targeted_reports - ).freeze - - # Suspend or remove an account and remove as much of its data - # as possible. If it's a local account and it has not been confirmed - # or never been approved, then side effects are skipped and both - # the user and account records are removed fully. Otherwise, - # it is controlled by options. - # @param [Account] - # @param [Hash] options - # @option [Boolean] :reserve_email Keep user record. Only applicable for local accounts - # @option [Boolean] :reserve_username Keep account record - # @option [Boolean] :skip_side_effects Side effects are ActivityPub and streaming API payloads - # @option [Time] :suspended_at Only applicable when :reserve_username is true - def call(account, **options) + def call(account) @account = account - @options = { reserve_username: true, reserve_email: true }.merge(options) - - if @account.local? && @account.user_unconfirmed_or_pending? - @options[:reserve_email] = false - @options[:reserve_username] = false - @options[:skip_side_effects] = true - end - reject_follows! - purge_user! - purge_profile! - purge_content! + suspend! + unmerge_from_home_timelines! + unmerge_from_list_timelines! + privatize_media_attachments! end private - def reject_follows! - return if @account.local? || !@account.activitypub? - - ActivityPub::DeliveryWorker.push_bulk(Follow.where(account: @account)) do |follow| - [build_reject_json(follow), follow.target_account_id, follow.account.inbox_url] - end + def suspend! + @account.suspend! unless @account.suspended? end - def purge_user! - return if !@account.local? || @account.user.nil? - - if @options[:reserve_email] - @account.user.disable! - @account.user.invites.where(uses: 0).destroy_all - else - @account.user.destroy + def unmerge_from_home_timelines! + @account.followers_for_local_distribution.find_each do |follower| + FeedManager.instance.unmerge_from_timeline(@account, follower) end end - def purge_content! - distribute_delete_actor! if @account.local? && !@options[:skip_side_effects] - - @account.statuses.reorder(nil).find_in_batches do |statuses| - statuses.reject! { |status| reported_status_ids.include?(status.id) } if @options[:reserve_username] - BatchedRemoveStatusService.new.call(statuses, skip_side_effects: @options[:skip_side_effects]) + def unmerge_from_list_timelines! + @account.lists_for_local_distribution.find_each do |list| + FeedManager.instance.unmerge_from_list(@account, list) end - - @account.media_attachments.reorder(nil).find_each do |media_attachment| - next if @options[:reserve_username] && reported_status_ids.include?(media_attachment.status_id) - - media_attachment.destroy - end - - @account.polls.reorder(nil).find_each do |poll| - next if @options[:reserve_username] && reported_status_ids.include?(poll.status_id) - - poll.destroy - end - - associations_for_destruction.each do |association_name| - destroy_all(@account.public_send(association_name)) - end - - @account.destroy unless @options[:reserve_username] end - def purge_profile! - # If the account is going to be destroyed - # there is no point wasting time updating - # its values first - - return unless @options[:reserve_username] + def privatize_media_attachments! + attachment_names = MediaAttachment.attachment_definitions.keys - @account.silenced_at = nil - @account.suspended_at = @options[:suspended_at] || Time.now.utc - @account.locked = false - @account.memorial = false - @account.discoverable = false - @account.display_name = '' - @account.note = '' - @account.fields = [] - @account.statuses_count = 0 - @account.followers_count = 0 - @account.following_count = 0 - @account.moved_to_account = nil - @account.trust_level = :untrusted - @account.avatar.destroy - @account.header.destroy - @account.save! - end - - def destroy_all(association) - association.in_batches.destroy_all - end - - def distribute_delete_actor! - ActivityPub::DeliveryWorker.push_bulk(delivery_inboxes) do |inbox_url| - [delete_actor_json, @account.id, inbox_url] - end - - ActivityPub::LowPriorityDeliveryWorker.push_bulk(low_priority_delivery_inboxes) do |inbox_url| - [delete_actor_json, @account.id, inbox_url] - end - end - - def delete_actor_json - @delete_actor_json ||= Oj.dump(serialize_payload(@account, ActivityPub::DeleteActorSerializer, signer: @account)) - end - - def build_reject_json(follow) - Oj.dump(serialize_payload(follow, ActivityPub::RejectFollowSerializer)) - end - - def delivery_inboxes - @delivery_inboxes ||= @account.followers.inboxes + Relay.enabled.pluck(:inbox_url) - end - - def low_priority_delivery_inboxes - Account.inboxes - delivery_inboxes - end - - def reported_status_ids - @reported_status_ids ||= Report.where(target_account: @account).unresolved.pluck(:status_ids).flatten.uniq - end + @account.media_attachments.find_each do |media_attachment| + attachment_names.each do |attachment_name| + attachment = media_attachment.public_send(attachment_name) + styles = [:original] | attachment.styles.keys - def associations_for_destruction - if @options[:reserve_username] - ASSOCIATIONS_ON_SUSPEND - else - ASSOCIATIONS_ON_SUSPEND + ASSOCIATIONS_ON_DESTROY + styles.each do |style| + case Paperclip::Attachment.default_options[:storage] + when :s3 + attachment.s3_object(style).acl.put(:private) + when :fog + # Not supported + when :filesystem + FileUtils.chmod(0o600 & ~File.umask, attachment.path(style)) + end + end + end end end end diff --git a/app/services/unsuspend_account_service.rb b/app/services/unsuspend_account_service.rb new file mode 100644 index 000000000..3e731ddd9 --- /dev/null +++ b/app/services/unsuspend_account_service.rb @@ -0,0 +1,52 @@ +# frozen_string_literal: true + +class UnsuspendAccountService < BaseService + def call(account) + @account = account + + unsuspend! + merge_into_home_timelines! + merge_into_list_timelines! + publish_media_attachments! + end + + private + + def unsuspend! + @account.unsuspend! if @account.suspended? + end + + def merge_into_home_timelines! + @account.followers_for_local_distribution.find_each do |follower| + FeedManager.instance.merge_into_timeline(@account, follower) + end + end + + def merge_into_list_timelines! + @account.lists_for_local_distribution.find_each do |list| + FeedManager.instance.merge_into_list(@account, list) + end + end + + def publish_media_attachments! + attachment_names = MediaAttachment.attachment_definitions.keys + + @account.media_attachments.find_each do |media_attachment| + attachment_names.each do |attachment_name| + attachment = media_attachment.public_send(attachment_name) + styles = [:original] | attachment.styles.keys + + styles.each do |style| + case Paperclip::Attachment.default_options[:storage] + when :s3 + attachment.s3_object(style).acl.put(Paperclip::Attachment.default_options[:s3_permissions]) + when :fog + # Not supported + when :filesystem + FileUtils.chmod(0o666 & ~File.umask, attachment.path(style)) + end + end + end + end + end +end diff --git a/app/views/admin/accounts/show.html.haml b/app/views/admin/accounts/show.html.haml index e6461aad0..2c48692b7 100644 --- a/app/views/admin/accounts/show.html.haml +++ b/app/views/admin/accounts/show.html.haml @@ -56,19 +56,21 @@ = link_to admin_action_logs_path(target_account_id: @account.id) do .dashboard__counters__text - if @account.local? && @account.user.nil? - %span.neutral= t('admin.accounts.deleted') + = t('admin.accounts.deleted') + - elsif @account.memorial? + = t('admin.accounts.memorialized') - elsif @account.suspended? - %span.red= t('admin.accounts.suspended') + = t('admin.accounts.suspended') - elsif @account.silenced? - %span.red= t('admin.accounts.silenced') + = t('admin.accounts.silenced') - elsif @account.local? && @account.user&.disabled? - %span.red= t('admin.accounts.disabled') + = t('admin.accounts.disabled') - elsif @account.local? && !@account.user&.confirmed? - %span.neutral= t('admin.accounts.confirming') + = t('admin.accounts.confirming') - elsif @account.local? && !@account.user_approved? - %span.neutral= t('admin.accounts.pending') + = t('admin.accounts.pending') - else - %span.neutral= t('admin.accounts.no_limits_imposed') + = t('admin.accounts.no_limits_imposed') .dashboard__counters__label= t 'admin.accounts.login_status' - unless @account.local? && @account.user.nil? @@ -122,19 +124,6 @@ = t('admin.accounts.confirming') %td= table_link_to 'refresh', t('admin.accounts.resend_confirmation.send'), resend_admin_account_confirmation_path(@account.id), method: :post if can?(:confirm, @account.user) - %tr - %th= t('admin.accounts.login_status') - %td - - if @account.user&.disabled? - = t('admin.accounts.disabled') - - else - = t('admin.accounts.enabled') - %td - - if @account.user&.disabled? - = table_link_to 'unlock', t('admin.accounts.enable'), enable_admin_account_path(@account.id), method: :post if can?(:enable, @account.user) - - elsif @account.user_approved? - = table_link_to 'lock', t('admin.accounts.disable'), new_admin_account_action_path(@account.id, type: 'disable') if can?(:disable, @account.user) - %tr %th= t('simple_form.labels.defaults.locale') %td= @account.user_locale @@ -172,49 +161,62 @@ %td = @account.inbox_url = fa_icon DeliveryFailureTracker.available?(@account.inbox_url) ? 'check' : 'times' + %td + = table_link_to 'search', @domain_block.present? ? t('admin.domain_blocks.view') : t('admin.accounts.view_domain'), admin_instance_path(@account.domain) %tr %th= t('admin.accounts.shared_inbox_url') %td = @account.shared_inbox_url = fa_icon DeliveryFailureTracker.available?(@account.shared_inbox_url) ? 'check': 'times' + %td + - if @domain_block.nil? + = table_link_to 'ban', t('admin.domain_blocks.add_new'), new_admin_domain_block_path(_domain: @account.domain) + + - if @account.suspended? + %hr.spacer/ + + %p.muted-hint= @deletion_request.present? ? t('admin.accounts.suspension_reversible_hint_html', date: content_tag(:strong, l(@deletion_request.due_at.to_date))) : t('admin.accounts.suspension_irreversible') + + = link_to t('admin.accounts.undo_suspension'), unsuspend_admin_account_path(@account.id), method: :post, class: 'button' if can?(:unsuspend, @account) - %div.action-buttons - %div - - if @account.local? && @account.user_approved? - = link_to t('admin.accounts.warn'), new_admin_account_action_path(@account.id, type: 'none'), class: 'button' if can?(:warn, @account) - - if @account.silenced? - = link_to t('admin.accounts.undo_silenced'), unsilence_admin_account_path(@account.id), method: :post, class: 'button' if can?(:unsilence, @account) - - elsif !@account.local? || @account.user_approved? - = link_to t('admin.accounts.silence'), new_admin_account_action_path(@account.id, type: 'silence'), class: 'button button--destructive' if can?(:silence, @account) - - - if @account.local? - - if @account.user_pending? - = link_to t('admin.accounts.approve'), approve_admin_account_path(@account.id), method: :post, data: { confirm: t('admin.accounts.are_you_sure') }, class: 'button' if can?(:approve, @account.user) - = link_to t('admin.accounts.reject'), reject_admin_account_path(@account.id), method: :post, data: { confirm: t('admin.accounts.are_you_sure') }, class: 'button button--destructive' if can?(:reject, @account.user) - - - unless @account.user_confirmed? - = link_to t('admin.accounts.confirm'), admin_account_confirmation_path(@account.id), method: :post, class: 'button' if can?(:confirm, @account.user) - - - if @account.suspended? - = link_to t('admin.accounts.undo_suspension'), unsuspend_admin_account_path(@account.id), method: :post, class: 'button' if can?(:unsuspend, @account) - - elsif !@account.local? || @account.user_approved? - = link_to t('admin.accounts.perform_full_suspension'), new_admin_account_action_path(@account.id, type: 'suspend'), class: 'button button--destructive' if can?(:suspend, @account) - - - unless @account.local? - - if DomainBlock.rule_for(@account.domain) - = link_to t('admin.domain_blocks.view'), admin_instance_path(@account.domain), class: 'button' + - if @deletion_request.present? + = link_to t('admin.accounts.delete'), admin_account_path(@account.id), method: :destroy, class: 'button button--destructive', data: { confirm: t('admin.accounts.are_you_sure') } if can?(:destroy, @account) + - else + %div.action-buttons + %div + - if @account.local? && @account.user_approved? + = link_to t('admin.accounts.warn'), new_admin_account_action_path(@account.id, type: 'none'), class: 'button' if can?(:warn, @account) + + - if @account.user_disabled? + = link_to t('admin.accounts.enable'), enable_admin_account_path(@account.id), method: :post, class: 'button' if can?(:enable, @account.user) + - else + = link_to t('admin.accounts.disable'), new_admin_account_action_path(@account.id, type: 'disable'), class: 'button' if can?(:disable, @account.user) + + - if @account.silenced? + = link_to t('admin.accounts.undo_silenced'), unsilence_admin_account_path(@account.id), method: :post, class: 'button' if can?(:unsilence, @account) + - elsif !@account.local? || @account.user_approved? + = link_to t('admin.accounts.silence'), new_admin_account_action_path(@account.id, type: 'silence'), class: 'button' if can?(:silence, @account) + + - if @account.local? + - if @account.user_pending? + = link_to t('admin.accounts.approve'), approve_admin_account_path(@account.id), method: :post, data: { confirm: t('admin.accounts.are_you_sure') }, class: 'button' if can?(:approve, @account.user) + = link_to t('admin.accounts.reject'), reject_admin_account_path(@account.id), method: :post, data: { confirm: t('admin.accounts.are_you_sure') }, class: 'button button--destructive' if can?(:reject, @account.user) + + - unless @account.user_confirmed? + = link_to t('admin.accounts.confirm'), admin_account_confirmation_path(@account.id), method: :post, class: 'button' if can?(:confirm, @account.user) + + - if !@account.local? || @account.user_approved? + = link_to t('admin.accounts.perform_full_suspension'), new_admin_account_action_path(@account.id, type: 'suspend'), class: 'button' if can?(:suspend, @account) + + %div + - if @account.local? + = link_to t('admin.accounts.reset_password'), admin_account_reset_path(@account.id), method: :create, class: 'button' if can?(:reset_password, @account.user) + - if @account.user&.otp_required_for_login? + = link_to t('admin.accounts.disable_two_factor_authentication'), admin_user_two_factor_authentication_path(@account.user.id), method: :delete, class: 'button' if can?(:disable_2fa, @account.user) + - if !@account.memorial? && @account.user_approved? + = link_to t('admin.accounts.memorialize'), memorialize_admin_account_path(@account.id), method: :post, data: { confirm: t('admin.accounts.are_you_sure') }, class: 'button button--destructive' if can?(:memorialize, @account) - else - = link_to t('admin.domain_blocks.add_new'), new_admin_domain_block_path(_domain: @account.domain), class: 'button button--destructive' - - %div - - if @account.local? - = link_to t('admin.accounts.reset_password'), admin_account_reset_path(@account.id), method: :create, class: 'button' if can?(:reset_password, @account.user) - - if @account.user&.otp_required_for_login? - = link_to t('admin.accounts.disable_two_factor_authentication'), admin_user_two_factor_authentication_path(@account.user.id), method: :delete, class: 'button' if can?(:disable_2fa, @account.user) - - if !@account.memorial? && @account.user_approved? - = link_to t('admin.accounts.memorialize'), memorialize_admin_account_path(@account.id), method: :post, data: { confirm: t('admin.accounts.are_you_sure') }, class: 'button button--destructive' if can?(:memorialize, @account) - - else - = link_to t('admin.accounts.redownload'), redownload_admin_account_path(@account.id), method: :post, class: 'button' if can?(:redownload, @account) + = link_to t('admin.accounts.redownload'), redownload_admin_account_path(@account.id), method: :post, class: 'button' if can?(:redownload, @account) %hr.spacer/ diff --git a/app/workers/account_deletion_worker.rb b/app/workers/account_deletion_worker.rb new file mode 100644 index 000000000..0f6be71e1 --- /dev/null +++ b/app/workers/account_deletion_worker.rb @@ -0,0 +1,13 @@ +# frozen_string_literal: true + +class AccountDeletionWorker + include Sidekiq::Worker + + sidekiq_options queue: 'pull' + + def perform(account_id) + DeleteAccountService.new.call(Account.find(account_id), reserve_username: true, reserve_email: false) + rescue ActiveRecord::RecordNotFound + true + end +end diff --git a/app/workers/admin/account_deletion_worker.rb b/app/workers/admin/account_deletion_worker.rb new file mode 100644 index 000000000..82f269ad6 --- /dev/null +++ b/app/workers/admin/account_deletion_worker.rb @@ -0,0 +1,13 @@ +# frozen_string_literal: true + +class Admin::AccountDeletionWorker + include Sidekiq::Worker + + sidekiq_options queue: 'pull' + + def perform(account_id) + DeleteAccountService.new.call(Account.find(account_id), reserve_username: true, reserve_email: true) + rescue ActiveRecord::RecordNotFound + true + end +end diff --git a/app/workers/admin/suspension_worker.rb b/app/workers/admin/suspension_worker.rb index 83c815efd..35c570336 100644 --- a/app/workers/admin/suspension_worker.rb +++ b/app/workers/admin/suspension_worker.rb @@ -5,7 +5,9 @@ class Admin::SuspensionWorker sidekiq_options queue: 'pull' - def perform(account_id, remove_user = false) - SuspendAccountService.new.call(Account.find(account_id), reserve_username: true, reserve_email: !remove_user) + def perform(account_id) + SuspendAccountService.new.call(Account.find(account_id)) + rescue ActiveRecord::RecordNotFound + true end end diff --git a/app/workers/admin/unsuspension_worker.rb b/app/workers/admin/unsuspension_worker.rb new file mode 100644 index 000000000..7cb2349b1 --- /dev/null +++ b/app/workers/admin/unsuspension_worker.rb @@ -0,0 +1,13 @@ +# frozen_string_literal: true + +class Admin::UnsuspensionWorker + include Sidekiq::Worker + + sidekiq_options queue: 'pull' + + def perform(account_id) + UnsuspendAccountService.new.call(Account.find(account_id)) + rescue ActiveRecord::RecordNotFound + true + end +end diff --git a/app/workers/scheduler/user_cleanup_scheduler.rb b/app/workers/scheduler/user_cleanup_scheduler.rb index 6113edde1..8571b59e1 100644 --- a/app/workers/scheduler/user_cleanup_scheduler.rb +++ b/app/workers/scheduler/user_cleanup_scheduler.rb @@ -6,9 +6,22 @@ class Scheduler::UserCleanupScheduler sidekiq_options lock: :until_executed, retry: 0 def perform + clean_unconfirmed_accounts! + clean_suspended_accounts! + end + + private + + def clean_unconfirmed_accounts! User.where('confirmed_at is NULL AND confirmation_sent_at <= ?', 2.days.ago).reorder(nil).find_in_batches do |batch| Account.where(id: batch.map(&:account_id)).delete_all User.where(id: batch.map(&:id)).delete_all end end + + def clean_suspended_accounts! + AccountDeletionRequest.where('created_at <= ?', AccountDeletionRequest::DELAY_TO_DELETION.ago).reorder(nil).find_each do |deletion_request| + Admin::AccountDeletionWorker.perform_async(deletion_request.account_id) + end + end end diff --git a/config/locales/en.yml b/config/locales/en.yml index ab96074fd..427b2c3fc 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -98,6 +98,7 @@ en: add_email_domain_block: Block e-mail domain approve: Approve approve_all: Approve all + approved_msg: Successfully approved %{username}'s sign-up application are_you_sure: Are you sure? avatar: Avatar by_domain: Domain @@ -111,18 +112,21 @@ en: confirm: Confirm confirmed: Confirmed confirming: Confirming + delete: Delete data deleted: Deleted demote: Demote - disable: Disable + destroyed_msg: "%{username}'s data is now queued to be deleted imminently" + disable: Freeze disable_two_factor_authentication: Disable 2FA - disabled: Disabled + disabled: Frozen display_name: Display name domain: Domain edit: Edit email: Email email_status: Email status - enable: Enable + enable: Unfreeze enabled: Enabled + enabled_msg: Successfully unfroze %{username}'s account followers: Followers follows: Follows header: Header @@ -138,6 +142,8 @@ en: login_status: Login status media_attachments: Media attachments memorialize: Turn into memoriam + memorialized: Memorialized + memorialized_msg: Successfully turned %{username} into a memorial account moderation: active: Active all: All @@ -158,10 +164,14 @@ en: public: Public push_subscription_expires: PuSH subscription expires redownload: Refresh profile + redownloaded_msg: Successfully refreshed %{username}'s profile from origin reject: Reject reject_all: Reject all + rejected_msg: Successfully rejected %{username}'s sign-up application remove_avatar: Remove avatar remove_header: Remove header + removed_avatar_msg: Successfully removed %{username}'s avatar image + removed_header_msg: Successfully removed %{username}'s header image resend_confirmation: already_confirmed: This user is already confirmed send: Resend confirmation email @@ -182,18 +192,23 @@ en: show: created_reports: Made reports targeted_reports: Reported by others - silence: Silence - silenced: Silenced + silence: Limit + silenced: Limited statuses: Statuses subscribe: Subscribe suspended: Suspended + suspension_irreversible: The data of this account has been irreversibly deleted. You can unsuspend the account to make it usable but it will not recover any data it previously had. + suspension_reversible_hint_html: The account has been suspended, and the data will be fully removed on %{date}. Until then, the account can be restored without any ill effects. If you wish to remove all of the account's data immediately, you can do so below. time_in_queue: Waiting in queue %{time} title: Accounts unconfirmed_email: Unconfirmed email undo_silenced: Undo silence undo_suspension: Undo suspension + unsilenced_msg: Successfully unlimited %{username}'s account unsubscribe: Unsubscribe + unsuspended_msg: Successfully unsuspended %{username}'s account username: Username + view_domain: View summary for domain warn: Warn web: Web whitelisted: Allowed for federation @@ -1304,9 +1319,9 @@ en: title: Sign in attempt warning: explanation: - disable: While your account is frozen, your account data remains intact, but you cannot perform any actions until it is unlocked. - silence: While your account is limited, only people who are already following you will see your toots on this server, and you may be excluded from various public listings. However, others may still manually follow you. - suspend: Your account has been suspended, and all of your toots and your uploaded media files have been irreversibly removed from this server, and servers where you had followers. + disable: You can no longer login to your account or use it in any other way, but your profile and other data remains intact. + silence: You can still use your account but only people who are already following you will see your toots on this server, and you may be excluded from various public listings. However, others may still manually follow you. + suspend: You can no longer use your account, and your profile and other data are no longer accessible. You can still login to request a backup of your data until the data is fully removed, but we will retain some data to prevent you from evading the suspension. get_in_touch: You can reply to this e-mail to get in touch with the staff of %{instance}. review_server_policies: Review server policies statuses: 'Specifically, for:' diff --git a/config/locales/simple_form.en.yml b/config/locales/simple_form.en.yml index 4ab0d1871..910e77ec2 100644 --- a/config/locales/simple_form.en.yml +++ b/config/locales/simple_form.en.yml @@ -90,10 +90,10 @@ en: text: Custom warning type: Action types: - disable: Disable login - none: Do nothing - silence: Silence - suspend: Suspend and irreversibly delete account data + disable: Freeze + none: Send a warning + silence: Limit + suspend: Suspend warning_preset_id: Use a warning preset announcement: all_day: All-day event diff --git a/config/routes.rb b/config/routes.rb index c281a86e3..8d9bc317b 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -232,7 +232,7 @@ Rails.application.routes.draw do resources :report_notes, only: [:create, :destroy] - resources :accounts, only: [:index, :show] do + resources :accounts, only: [:index, :show, :destroy] do member do post :enable post :unsilence @@ -466,7 +466,7 @@ Rails.application.routes.draw do end namespace :admin do - resources :accounts, only: [:index, :show] do + resources :accounts, only: [:index, :show, :destroy] do member do post :enable post :unsilence diff --git a/db/migrate/20200908193330_create_account_deletion_requests.rb b/db/migrate/20200908193330_create_account_deletion_requests.rb new file mode 100644 index 000000000..e03183ae4 --- /dev/null +++ b/db/migrate/20200908193330_create_account_deletion_requests.rb @@ -0,0 +1,8 @@ +class CreateAccountDeletionRequests < ActiveRecord::Migration[5.2] + def change + create_table :account_deletion_requests do |t| + t.references :account, foreign_key: { on_delete: :cascade } + t.timestamps + end + end +end diff --git a/db/schema.rb b/db/schema.rb index e37aae962..038e39130 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 2020_06_30_190544) do +ActiveRecord::Schema.define(version: 2020_09_08_193330) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -36,6 +36,13 @@ ActiveRecord::Schema.define(version: 2020_06_30_190544) do t.index ["conversation_id"], name: "index_account_conversations_on_conversation_id" end + create_table "account_deletion_requests", force: :cascade do |t| + t.bigint "account_id" + t.datetime "created_at", null: false + t.datetime "updated_at", null: false + t.index ["account_id"], name: "index_account_deletion_requests_on_account_id" + end + create_table "account_domain_blocks", force: :cascade do |t| t.string "domain" t.datetime "created_at", null: false @@ -926,6 +933,7 @@ ActiveRecord::Schema.define(version: 2020_06_30_190544) do add_foreign_key "account_aliases", "accounts", on_delete: :cascade add_foreign_key "account_conversations", "accounts", on_delete: :cascade add_foreign_key "account_conversations", "conversations", on_delete: :cascade + add_foreign_key "account_deletion_requests", "accounts", on_delete: :cascade add_foreign_key "account_domain_blocks", "accounts", name: "fk_206c6029bd", on_delete: :cascade add_foreign_key "account_identity_proofs", "accounts", on_delete: :cascade add_foreign_key "account_migrations", "accounts", column: "target_account_id", on_delete: :nullify diff --git a/lib/mastodon/accounts_cli.rb b/lib/mastodon/accounts_cli.rb index 8c91c3013..8f9279a3c 100644 --- a/lib/mastodon/accounts_cli.rb +++ b/lib/mastodon/accounts_cli.rb @@ -87,7 +87,7 @@ module Mastodon say('Use --force to reattach it anyway and delete the other user') return elsif account.user.present? - account.user.destroy! + DeleteAccountService.new.call(account, reserve_email: false) end end @@ -192,7 +192,7 @@ module Mastodon end say("Deleting user with #{account.statuses_count} statuses, this might take a while...") - SuspendAccountService.new.call(account, reserve_email: false) + DeleteAccountService.new.call(account, reserve_email: false) say('OK', :green) end diff --git a/lib/mastodon/domains_cli.rb b/lib/mastodon/domains_cli.rb index 558737c27..5433ddd9d 100644 --- a/lib/mastodon/domains_cli.rb +++ b/lib/mastodon/domains_cli.rb @@ -42,7 +42,7 @@ module Mastodon end processed, = parallelize_with_progress(scope) do |account| - SuspendAccountService.new.call(account, reserve_username: false, skip_side_effects: true) unless options[:dry_run] + DeleteAccountService.new.call(account, reserve_username: false, skip_side_effects: true) unless options[:dry_run] end DomainBlock.where(domain: domains).destroy_all unless options[:dry_run] diff --git a/spec/controllers/auth/registrations_controller_spec.rb b/spec/controllers/auth/registrations_controller_spec.rb index c2e9f33a8..bef822763 100644 --- a/spec/controllers/auth/registrations_controller_spec.rb +++ b/spec/controllers/auth/registrations_controller_spec.rb @@ -199,9 +199,10 @@ RSpec.describe Auth::RegistrationsController, type: :controller do end subject do + inviter = Fabricate(:user, confirmed_at: 2.days.ago) Setting.registrations_mode = 'approved' request.headers["Accept-Language"] = accept_language - invite = Fabricate(:invite, max_uses: nil, expires_at: 1.hour.from_now) + invite = Fabricate(:invite, user: inviter, max_uses: nil, expires_at: 1.hour.from_now) post :create, params: { user: { account_attributes: { username: 'test' }, email: 'test@example.com', password: '12345678', password_confirmation: '12345678', 'invite_code': invite.code, agreement: 'true' } } end diff --git a/spec/controllers/concerns/export_controller_concern_spec.rb b/spec/controllers/concerns/export_controller_concern_spec.rb index e5861c801..fce129bee 100644 --- a/spec/controllers/concerns/export_controller_concern_spec.rb +++ b/spec/controllers/concerns/export_controller_concern_spec.rb @@ -5,6 +5,7 @@ require 'rails_helper' describe ApplicationController, type: :controller do controller do include ExportControllerConcern + def index send_export_file end diff --git a/spec/fabricators/account_deletion_request_fabricator.rb b/spec/fabricators/account_deletion_request_fabricator.rb new file mode 100644 index 000000000..08a82ba3c --- /dev/null +++ b/spec/fabricators/account_deletion_request_fabricator.rb @@ -0,0 +1,3 @@ +Fabricator(:account_deletion_request) do + account +end diff --git a/spec/models/account_deletion_request_spec.rb b/spec/models/account_deletion_request_spec.rb new file mode 100644 index 000000000..afaecbe22 --- /dev/null +++ b/spec/models/account_deletion_request_spec.rb @@ -0,0 +1,4 @@ +require 'rails_helper' + +RSpec.describe AccountDeletionRequest, type: :model do +end diff --git a/spec/models/invite_spec.rb b/spec/models/invite_spec.rb index 30abfb86b..b0596c561 100644 --- a/spec/models/invite_spec.rb +++ b/spec/models/invite_spec.rb @@ -29,7 +29,7 @@ RSpec.describe Invite, type: :model do it 'returns false when invite creator has been disabled' do invite = Fabricate(:invite, max_uses: nil, expires_at: nil) - SuspendAccountService.new.call(invite.user.account) + invite.user.account.suspend! expect(invite.valid_for_use?).to be false end end diff --git a/spec/services/delete_account_service_spec.rb b/spec/services/delete_account_service_spec.rb new file mode 100644 index 000000000..d208b25b8 --- /dev/null +++ b/spec/services/delete_account_service_spec.rb @@ -0,0 +1,84 @@ +require 'rails_helper' + +RSpec.describe DeleteAccountService, type: :service do + describe '#call on local account' do + before do + stub_request(:post, "https://alice.com/inbox").to_return(status: 201) + stub_request(:post, "https://bob.com/inbox").to_return(status: 201) + end + + subject do + -> { described_class.new.call(account) } + end + + let!(:account) { Fabricate(:account) } + let!(:status) { Fabricate(:status, account: account) } + let!(:media_attachment) { Fabricate(:media_attachment, account: account) } + let!(:notification) { Fabricate(:notification, account: account) } + let!(:favourite) { Fabricate(:favourite, account: account) } + let!(:active_relationship) { Fabricate(:follow, account: account) } + let!(:passive_relationship) { Fabricate(:follow, target_account: account) } + let!(:remote_alice) { Fabricate(:account, inbox_url: 'https://alice.com/inbox', protocol: :activitypub) } + let!(:remote_bob) { Fabricate(:account, inbox_url: 'https://bob.com/inbox', protocol: :activitypub) } + let!(:endorsment) { Fabricate(:account_pin, account: passive_relationship.account, target_account: account) } + + it 'deletes associated records' do + is_expected.to change { + [ + account.statuses, + account.media_attachments, + account.notifications, + account.favourites, + account.active_relationships, + account.passive_relationships, + AccountPin.where(target_account: account), + ].map(&:count) + }.from([1, 1, 1, 1, 1, 1, 1]).to([0, 0, 0, 0, 0, 0, 0]) + end + + it 'sends a delete actor activity to all known inboxes' do + subject.call + expect(a_request(:post, "https://alice.com/inbox")).to have_been_made.once + expect(a_request(:post, "https://bob.com/inbox")).to have_been_made.once + end + end + + describe '#call on remote account' do + before do + stub_request(:post, "https://alice.com/inbox").to_return(status: 201) + stub_request(:post, "https://bob.com/inbox").to_return(status: 201) + end + + subject do + -> { described_class.new.call(remote_bob) } + end + + let!(:account) { Fabricate(:account) } + let!(:remote_alice) { Fabricate(:account, inbox_url: 'https://alice.com/inbox', protocol: :activitypub) } + let!(:remote_bob) { Fabricate(:account, inbox_url: 'https://bob.com/inbox', protocol: :activitypub) } + let!(:status) { Fabricate(:status, account: remote_bob) } + let!(:media_attachment) { Fabricate(:media_attachment, account: remote_bob) } + let!(:notification) { Fabricate(:notification, account: remote_bob) } + let!(:favourite) { Fabricate(:favourite, account: remote_bob) } + let!(:active_relationship) { Fabricate(:follow, account: remote_bob, target_account: account) } + let!(:passive_relationship) { Fabricate(:follow, target_account: remote_bob) } + + it 'deletes associated records' do + is_expected.to change { + [ + remote_bob.statuses, + remote_bob.media_attachments, + remote_bob.notifications, + remote_bob.favourites, + remote_bob.active_relationships, + remote_bob.passive_relationships, + ].map(&:count) + }.from([1, 1, 1, 1, 1, 1]).to([0, 0, 0, 0, 0, 0]) + end + + it 'sends a reject follow to follwer inboxes' do + subject.call + expect(a_request(:post, remote_bob.inbox_url)).to have_been_made.once + end + end +end diff --git a/spec/services/suspend_account_service_spec.rb b/spec/services/suspend_account_service_spec.rb deleted file mode 100644 index 32726d763..000000000 --- a/spec/services/suspend_account_service_spec.rb +++ /dev/null @@ -1,84 +0,0 @@ -require 'rails_helper' - -RSpec.describe SuspendAccountService, type: :service do - describe '#call on local account' do - before do - stub_request(:post, "https://alice.com/inbox").to_return(status: 201) - stub_request(:post, "https://bob.com/inbox").to_return(status: 201) - end - - subject do - -> { described_class.new.call(account) } - end - - let!(:account) { Fabricate(:account) } - let!(:status) { Fabricate(:status, account: account) } - let!(:media_attachment) { Fabricate(:media_attachment, account: account) } - let!(:notification) { Fabricate(:notification, account: account) } - let!(:favourite) { Fabricate(:favourite, account: account) } - let!(:active_relationship) { Fabricate(:follow, account: account) } - let!(:passive_relationship) { Fabricate(:follow, target_account: account) } - let!(:remote_alice) { Fabricate(:account, inbox_url: 'https://alice.com/inbox', protocol: :activitypub) } - let!(:remote_bob) { Fabricate(:account, inbox_url: 'https://bob.com/inbox', protocol: :activitypub) } - let!(:endorsment) { Fabricate(:account_pin, account: passive_relationship.account, target_account: account) } - - it 'deletes associated records' do - is_expected.to change { - [ - account.statuses, - account.media_attachments, - account.notifications, - account.favourites, - account.active_relationships, - account.passive_relationships, - AccountPin.where(target_account: account), - ].map(&:count) - }.from([1, 1, 1, 1, 1, 1, 1]).to([0, 0, 0, 0, 0, 0, 0]) - end - - it 'sends a delete actor activity to all known inboxes' do - subject.call - expect(a_request(:post, "https://alice.com/inbox")).to have_been_made.once - expect(a_request(:post, "https://bob.com/inbox")).to have_been_made.once - end - end - - describe '#call on remote account' do - before do - stub_request(:post, "https://alice.com/inbox").to_return(status: 201) - stub_request(:post, "https://bob.com/inbox").to_return(status: 201) - end - - subject do - -> { described_class.new.call(remote_bob) } - end - - let!(:account) { Fabricate(:account) } - let!(:remote_alice) { Fabricate(:account, inbox_url: 'https://alice.com/inbox', protocol: :activitypub) } - let!(:remote_bob) { Fabricate(:account, inbox_url: 'https://bob.com/inbox', protocol: :activitypub) } - let!(:status) { Fabricate(:status, account: remote_bob) } - let!(:media_attachment) { Fabricate(:media_attachment, account: remote_bob) } - let!(:notification) { Fabricate(:notification, account: remote_bob) } - let!(:favourite) { Fabricate(:favourite, account: remote_bob) } - let!(:active_relationship) { Fabricate(:follow, account: remote_bob, target_account: account) } - let!(:passive_relationship) { Fabricate(:follow, target_account: remote_bob) } - - it 'deletes associated records' do - is_expected.to change { - [ - remote_bob.statuses, - remote_bob.media_attachments, - remote_bob.notifications, - remote_bob.favourites, - remote_bob.active_relationships, - remote_bob.passive_relationships, - ].map(&:count) - }.from([1, 1, 1, 1, 1, 1]).to([0, 0, 0, 0, 0, 0]) - end - - it 'sends a reject follow to follwer inboxes' do - subject.call - expect(a_request(:post, remote_bob.inbox_url)).to have_been_made.once - end - end -end -- cgit From 974b1b79ce58e6799e5e5bb576e630ca783150de Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Fri, 18 Sep 2020 17:26:45 +0200 Subject: Add option to be notified when a followed user posts (#13546) * Add bell button Fix #4890 * Remove duplicate type from post-deployment migration * Fix legacy class type mappings * Improve query performance with better index * Fix validation * Remove redundant index from notifications --- app/controllers/api/v1/accounts_controller.rb | 5 +- .../api/v1/follow_requests_controller.rb | 2 +- app/javascript/mastodon/actions/accounts.js | 4 +- app/javascript/mastodon/actions/notifications.js | 2 +- .../mastodon/features/account/components/header.js | 12 +++- .../features/account_timeline/components/header.js | 5 ++ .../containers/header_container.js | 12 +++- .../notifications/components/filter_bar.js | 8 +++ .../notifications/components/notification.js | 35 +++++++++ app/javascript/styles/mastodon/components.scss | 4 ++ app/lib/activitypub/activity.rb | 4 +- app/lib/activitypub/activity/follow.rb | 4 +- app/lib/activitypub/activity/like.rb | 2 +- app/models/concerns/account_interactions.rb | 26 ++++--- app/models/follow.rb | 3 +- app/models/follow_request.rb | 3 +- app/models/notification.rb | 44 +++++++----- app/serializers/rest/notification_serializer.rb | 2 +- app/serializers/rest/relationship_serializer.rb | 12 +++- app/services/favourite_service.rb | 2 +- app/services/follow_service.rb | 15 ++-- app/services/import_service.rb | 6 +- app/services/notify_service.rb | 8 ++- app/services/process_mentions_service.rb | 2 +- app/services/reblog_service.rb | 2 +- app/workers/feed_insert_worker.rb | 15 +++- app/workers/local_notification_worker.rb | 4 +- app/workers/poll_expiration_notify_worker.rb | 4 +- app/workers/refollow_worker.rb | 3 +- app/workers/unfollow_follow_worker.rb | 5 +- db/migrate/20200917192924_add_notify_to_follows.rb | 19 +++++ .../20200917193034_add_type_to_notifications.rb | 5 ++ ...200917222316_add_index_notifications_on_type.rb | 7 ++ .../20200917193528_migrate_notifications_type.rb | 22 ++++++ ...move_index_notifications_on_account_activity.rb | 15 ++++ db/schema.rb | 8 ++- .../controllers/api/v1/accounts_controller_spec.rb | 84 +++++++++++++++------- spec/models/concerns/account_interactions_spec.rb | 2 +- spec/models/follow_request_spec.rb | 2 +- spec/services/import_service_spec.rb | 1 + spec/services/notify_service_spec.rb | 6 +- spec/workers/refollow_worker_spec.rb | 4 +- 42 files changed, 324 insertions(+), 106 deletions(-) create mode 100644 db/migrate/20200917192924_add_notify_to_follows.rb create mode 100644 db/migrate/20200917193034_add_type_to_notifications.rb create mode 100644 db/migrate/20200917222316_add_index_notifications_on_type.rb create mode 100644 db/post_migrate/20200917193528_migrate_notifications_type.rb create mode 100644 db/post_migrate/20200917222734_remove_index_notifications_on_account_activity.rb (limited to 'app/controllers') diff --git a/app/controllers/api/v1/accounts_controller.rb b/app/controllers/api/v1/accounts_controller.rb index 61dcb87c2..aef51a647 100644 --- a/app/controllers/api/v1/accounts_controller.rb +++ b/app/controllers/api/v1/accounts_controller.rb @@ -30,9 +30,8 @@ class Api::V1::AccountsController < Api::BaseController end def follow - FollowService.new.call(current_user.account, @account, reblogs: truthy_param?(:reblogs), with_rate_limit: true) - - options = @account.locked? || current_user.account.silenced? ? {} : { following_map: { @account.id => { reblogs: truthy_param?(:reblogs) } }, requested_map: { @account.id => false } } + follow = FollowService.new.call(current_user.account, @account, reblogs: params.key?(:reblogs) ? truthy_param?(:reblogs) : nil, notify: params.key?(:notify) ? truthy_param?(:notify) : nil, with_rate_limit: true) + options = @account.locked? || current_user.account.silenced? ? {} : { following_map: { @account.id => { reblogs: follow.show_reblogs?, notify: follow.notify? } }, requested_map: { @account.id => false } } render json: @account, serializer: REST::RelationshipSerializer, relationships: relationships(options) end diff --git a/app/controllers/api/v1/follow_requests_controller.rb b/app/controllers/api/v1/follow_requests_controller.rb index 0420b7bef..b34c76f29 100644 --- a/app/controllers/api/v1/follow_requests_controller.rb +++ b/app/controllers/api/v1/follow_requests_controller.rb @@ -13,7 +13,7 @@ class Api::V1::FollowRequestsController < Api::BaseController def authorize AuthorizeFollowService.new.call(account, current_account) - NotifyService.new.call(current_account, Follow.find_by(account: account, target_account: current_account)) + NotifyService.new.call(current_account, :follow, Follow.find_by(account: account, target_account: current_account)) render json: account, serializer: REST::RelationshipSerializer, relationships: relationships end diff --git a/app/javascript/mastodon/actions/accounts.js b/app/javascript/mastodon/actions/accounts.js index d28f7dad8..723c04e55 100644 --- a/app/javascript/mastodon/actions/accounts.js +++ b/app/javascript/mastodon/actions/accounts.js @@ -109,14 +109,14 @@ export function fetchAccountFail(id, error) { }; }; -export function followAccount(id, reblogs = true) { +export function followAccount(id, options = { reblogs: true }) { return (dispatch, getState) => { const alreadyFollowing = getState().getIn(['relationships', id, 'following']); const locked = getState().getIn(['accounts', id, 'locked'], false); dispatch(followAccountRequest(id, locked)); - api(getState).post(`/api/v1/accounts/${id}/follow`, { reblogs }).then(response => { + api(getState).post(`/api/v1/accounts/${id}/follow`, options).then(response => { dispatch(followAccountSuccess(response.data, alreadyFollowing)); }).catch(error => { dispatch(followAccountFail(error, locked)); diff --git a/app/javascript/mastodon/actions/notifications.js b/app/javascript/mastodon/actions/notifications.js index a26844f84..099e42f6c 100644 --- a/app/javascript/mastodon/actions/notifications.js +++ b/app/javascript/mastodon/actions/notifications.js @@ -59,7 +59,7 @@ export function updateNotifications(notification, intlMessages, intlLocale) { let filtered = false; - if (notification.type === 'mention') { + if (['mention', 'status'].includes(notification.type)) { const dropRegex = filters[0]; const regex = filters[1]; const searchIndex = searchTextFromRawStatus(notification.status); diff --git a/app/javascript/mastodon/features/account/components/header.js b/app/javascript/mastodon/features/account/components/header.js index 02217b62c..2b97af4e6 100644 --- a/app/javascript/mastodon/features/account/components/header.js +++ b/app/javascript/mastodon/features/account/components/header.js @@ -7,6 +7,7 @@ import ImmutablePureComponent from 'react-immutable-pure-component'; import { autoPlayGif, me, isStaff } from 'mastodon/initial_state'; import classNames from 'classnames'; import Icon from 'mastodon/components/icon'; +import IconButton from 'mastodon/components/icon_button'; import Avatar from 'mastodon/components/avatar'; import { counterRenderer } from 'mastodon/components/common_counter'; import ShortNumber from 'mastodon/components/short_number'; @@ -35,6 +36,8 @@ const messages = defineMessages({ unblockDomain: { id: 'account.unblock_domain', defaultMessage: 'Unblock domain {domain}' }, hideReblogs: { id: 'account.hide_reblogs', defaultMessage: 'Hide boosts from @{name}' }, showReblogs: { id: 'account.show_reblogs', defaultMessage: 'Show boosts from @{name}' }, + enableNotifications: { id: 'account.enable_notifications', defaultMessage: 'Notify me when @{name} posts' }, + disableNotifications: { id: 'account.disable_notifications', defaultMessage: 'Stop notifying me when @{name} posts' }, pins: { id: 'navigation_bar.pins', defaultMessage: 'Pinned toots' }, preferences: { id: 'navigation_bar.preferences', defaultMessage: 'Preferences' }, follow_requests: { id: 'navigation_bar.follow_requests', defaultMessage: 'Follow requests' }, @@ -68,8 +71,9 @@ class Header extends ImmutablePureComponent { onBlock: PropTypes.func.isRequired, onMention: PropTypes.func.isRequired, onDirect: PropTypes.func.isRequired, - onReport: PropTypes.func.isRequired, onReblogToggle: PropTypes.func.isRequired, + onNotifyToggle: PropTypes.func.isRequired, + onReport: PropTypes.func.isRequired, onMute: PropTypes.func.isRequired, onBlockDomain: PropTypes.func.isRequired, onUnblockDomain: PropTypes.func.isRequired, @@ -144,6 +148,7 @@ class Header extends ImmutablePureComponent { let info = []; let actionBtn = ''; + let bellBtn = ''; let lockedIcon = ''; let menu = []; @@ -173,6 +178,10 @@ class Header extends ImmutablePureComponent { actionBtn = +