From 4a618908e836ecb94f70e99f2198ee7b3ba3b2ec Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Wed, 14 Jun 2017 18:01:27 +0200 Subject: [PATCH] Account deletion (#3728) * Add form for account deletion * If avatar or header are gone from source, remove them * Add option to have SuspendAccountService remove user record, add tests * Exclude suspended accounts from search --- .../settings/deletes_controller.rb | 27 +++++++ app/javascript/styles/admin.scss | 15 ++++ app/javascript/styles/forms.scss | 3 + app/models/account.rb | 3 + app/models/form/delete_confirmation.rb | 7 ++ app/services/suspend_account_service.rb | 7 +- app/services/update_remote_profile_service.rb | 15 +++- app/views/auth/registrations/edit.html.haml | 5 ++ app/views/settings/deletes/show.html.haml | 16 +++++ app/workers/admin/suspension_worker.rb | 4 +- config/locales/en.yml | 11 +++ config/locales/simple_form.ru.yml | 2 +- config/navigation.rb | 2 +- config/routes.rb | 1 + .../settings/deletes_controller_spec.rb | 72 +++++++++++++++++++ 15 files changed, 183 insertions(+), 7 deletions(-) create mode 100644 app/controllers/settings/deletes_controller.rb create mode 100644 app/models/form/delete_confirmation.rb create mode 100644 app/views/settings/deletes/show.html.haml create mode 100644 spec/controllers/settings/deletes_controller_spec.rb diff --git a/app/controllers/settings/deletes_controller.rb b/app/controllers/settings/deletes_controller.rb new file mode 100644 index 000000000..55c18345b --- /dev/null +++ b/app/controllers/settings/deletes_controller.rb @@ -0,0 +1,27 @@ +# frozen_string_literal: true + +class Settings::DeletesController < ApplicationController + layout 'admin' + + before_action :authenticate_user! + + def show + @confirmation = Form::DeleteConfirmation.new + end + + def destroy + if current_user.valid_password?(delete_params[:password]) + Admin::SuspensionWorker.perform_async(current_user.account_id, true) + sign_out + redirect_to new_user_session_path, notice: I18n.t('deletes.success_msg') + else + redirect_to settings_delete_path, alert: I18n.t('deletes.bad_password_msg') + end + end + + private + + def delete_params + params.permit(:password) + end +end diff --git a/app/javascript/styles/admin.scss b/app/javascript/styles/admin.scss index 541e57f32..c2bfc10a0 100644 --- a/app/javascript/styles/admin.scss +++ b/app/javascript/styles/admin.scss @@ -96,6 +96,13 @@ margin-bottom: 40px; } + h6 { + font-size: 16px; + color: $ui-primary-color; + line-height: 28px; + font-weight: 400; + } + & > p { font-size: 14px; line-height: 18px; @@ -114,6 +121,14 @@ background: transparent; border-bottom: 1px solid $ui-base-color; } + + .muted-hint { + color: lighten($ui-base-color, 27%); + + a { + color: $ui-primary-color; + } + } } .simple_form { diff --git a/app/javascript/styles/forms.scss b/app/javascript/styles/forms.scss index 6af846b0a..059c4a7d8 100644 --- a/app/javascript/styles/forms.scss +++ b/app/javascript/styles/forms.scss @@ -303,7 +303,10 @@ code { font-weight: 500; } } +} +.simple_form, +.table-form { .warning { max-width: 400px; box-sizing: border-box; diff --git a/app/models/account.rb b/app/models/account.rb index 5c0f70ff8..72bad51a2 100644 --- a/app/models/account.rb +++ b/app/models/account.rb @@ -177,6 +177,7 @@ class Account < ApplicationRecord account_id IN (SELECT * FROM first_degree) AND target_account_id NOT IN (SELECT * FROM first_degree) AND target_account_id NOT IN (:excluded_account_ids) + AND accounts.suspended = false GROUP BY target_account_id, accounts.id ORDER BY count(account_id) DESC OFFSET :offset @@ -199,6 +200,7 @@ class Account < ApplicationRecord ts_rank_cd(#{textsearch}, #{query}, 32) AS rank FROM accounts WHERE #{query} @@ #{textsearch} + AND accounts.suspended = false ORDER BY rank DESC LIMIT ? SQL @@ -216,6 +218,7 @@ class Account < ApplicationRecord FROM accounts LEFT OUTER JOIN follows AS f ON (accounts.id = f.account_id AND f.target_account_id = ?) OR (accounts.id = f.target_account_id AND f.account_id = ?) WHERE #{query} @@ #{textsearch} + AND accounts.suspended = false GROUP BY accounts.id ORDER BY rank DESC LIMIT ? diff --git a/app/models/form/delete_confirmation.rb b/app/models/form/delete_confirmation.rb new file mode 100644 index 000000000..0884a09b8 --- /dev/null +++ b/app/models/form/delete_confirmation.rb @@ -0,0 +1,7 @@ +# frozen_string_literal: true + +class Form::DeleteConfirmation + include ActiveModel::Model + + attr_accessor :password +end diff --git a/app/services/suspend_account_service.rb b/app/services/suspend_account_service.rb index 4a4f23b80..842500259 100644 --- a/app/services/suspend_account_service.rb +++ b/app/services/suspend_account_service.rb @@ -1,9 +1,10 @@ # frozen_string_literal: true class SuspendAccountService < BaseService - def call(account) + def call(account, remove_user = false) @account = account + purge_user if remove_user purge_content purge_profile unsubscribe_push_subscribers @@ -11,6 +12,10 @@ class SuspendAccountService < BaseService private + def purge_user + @account.user.destroy + end + def purge_content @account.statuses.reorder(nil).find_each do |status| # This federates out deletes to previous followers diff --git a/app/services/update_remote_profile_service.rb b/app/services/update_remote_profile_service.rb index c65f35867..49a907682 100644 --- a/app/services/update_remote_profile_service.rb +++ b/app/services/update_remote_profile_service.rb @@ -27,8 +27,19 @@ class UpdateRemoteProfileService < BaseService account.locked = remote_profile.locked? if !account.suspended? && !DomainBlock.find_by(domain: account.domain)&.reject_media? - account.avatar_remote_url = remote_profile.avatar if remote_profile.avatar.present? - account.header_remote_url = remote_profile.header if remote_profile.header.present? + if remote_profile.avatar.present? + account.avatar_remote_url = remote_profile.avatar + else + account.avatar_remote_url = '' + account.avatar.destroy + end + + if remote_profile.header.present? + account.header_remote_url = remote_profile.header + else + account.header_remote_url = '' + account.header.destroy + end end end end diff --git a/app/views/auth/registrations/edit.html.haml b/app/views/auth/registrations/edit.html.haml index 8ab188f0c..cbaa75ae0 100644 --- a/app/views/auth/registrations/edit.html.haml +++ b/app/views/auth/registrations/edit.html.haml @@ -11,3 +11,8 @@ .actions = f.button :button, t('generic.save_changes'), type: :submit + +%hr/ + +%h6= t('auth.delete_account') +%p.muted-hint= t('auth.delete_account_html', path: settings_delete_path) diff --git a/app/views/settings/deletes/show.html.haml b/app/views/settings/deletes/show.html.haml new file mode 100644 index 000000000..d49a7bd0c --- /dev/null +++ b/app/views/settings/deletes/show.html.haml @@ -0,0 +1,16 @@ +- content_for :page_title do + = t('settings.delete') + += simple_form_for @confirmation, url: settings_delete_path, method: :delete do |f| + .warning + %strong + = fa_icon('warning') + = t('deletes.warning_title') + = t('deletes.warning_html') + + %p.hint= t('deletes.description_html') + + = f.input :password, autocomplete: 'off', placeholder: t('simple_form.labels.defaults.current_password'), input_html: { 'aria-label' => t('simple_form.labels.defaults.current_password') }, hint: t('deletes.confirm_password') + + .actions + = f.button :button, t('deletes.proceed'), type: :submit, class: 'negative' diff --git a/app/workers/admin/suspension_worker.rb b/app/workers/admin/suspension_worker.rb index 7ef2b35ec..6338b1130 100644 --- a/app/workers/admin/suspension_worker.rb +++ b/app/workers/admin/suspension_worker.rb @@ -5,7 +5,7 @@ class Admin::SuspensionWorker sidekiq_options queue: 'pull' - def perform(account_id) - SuspendAccountService.new.call(Account.find(account_id)) + def perform(account_id, remove_user = false) + SuspendAccountService.new.call(Account.find(account_id), remove_user) end end diff --git a/config/locales/en.yml b/config/locales/en.yml index 71def8fe2..0d33aae3f 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -201,6 +201,8 @@ en: invalid_url: The provided URL is invalid auth: change_password: Credentials + delete_account: Delete account + delete_account_html: If you wish to delete your account, you can proceed here. You will be asked for confirmation. didnt_get_confirmation: Didn't receive confirmation instructions? forgot_password: Forgot your password? login: Log in @@ -228,6 +230,14 @@ en: x_minutes: "%{count}m" x_months: "%{count}mo" x_seconds: "%{count}s" + deletes: + bad_password_msg: Nice try, hackers! Incorrect password + confirm_password: Enter your current password to verify your identity + description_html: This will permanently, irreversibly remove content from your account and deactivate it. Your username will remain reserved to prevent future impersonations. + proceed: Delete account + success_msg: Your account was successfully deleted + warning_html: Only deletion of content from this particular instance is guaranteed. Content that has been widely shared is likely to leave traces. Offline servers and servers that have unsubscribed from your updates will not update their databases. + warning_title: Disseminated content availability errors: '403': You don't have permission to view this page. '404': The page you were looking for doesn't exist. @@ -313,6 +323,7 @@ en: settings: authorized_apps: Authorized apps back: Back to Mastodon + delete: Account deletion edit_profile: Edit profile export: Data export followers: Authorized followers diff --git a/config/locales/simple_form.ru.yml b/config/locales/simple_form.ru.yml index a2e2eaf9e..c4e6ad8a8 100644 --- a/config/locales/simple_form.ru.yml +++ b/config/locales/simple_form.ru.yml @@ -41,8 +41,8 @@ ru: password: Пароль setting_auto_play_gif: Автоматически проигрывать анимированные GIF setting_boost_modal: Показывать диалог подтверждения перед продвижением - setting_delete_modal: Показывать диалог подтверждения перед удалением setting_default_privacy: Видимость постов + setting_delete_modal: Показывать диалог подтверждения перед удалением severity: Строгость type: Тип импорта username: Имя пользователя diff --git a/config/navigation.rb b/config/navigation.rb index 85ed62118..d7508f019 100644 --- a/config/navigation.rb +++ b/config/navigation.rb @@ -7,7 +7,7 @@ SimpleNavigation::Configuration.run do |navigation| primary.item :settings, safe_join([fa_icon('cog fw'), t('settings.settings')]), settings_profile_url do |settings| settings.item :profile, safe_join([fa_icon('user fw'), t('settings.edit_profile')]), settings_profile_url settings.item :preferences, safe_join([fa_icon('sliders fw'), t('settings.preferences')]), settings_preferences_url - settings.item :password, safe_join([fa_icon('cog fw'), t('auth.change_password')]), edit_user_registration_url + settings.item :password, safe_join([fa_icon('cog fw'), t('auth.change_password')]), edit_user_registration_url, highlights_on: %r{/auth/edit|/settings/delete} settings.item :two_factor_authentication, safe_join([fa_icon('mobile fw'), t('settings.two_factor_authentication')]), settings_two_factor_authentication_url, highlights_on: %r{/settings/two_factor_authentication} settings.item :import, safe_join([fa_icon('cloud-upload fw'), t('settings.import')]), settings_import_url settings.item :export, safe_join([fa_icon('cloud-download fw'), t('settings.export')]), settings_export_url diff --git a/config/routes.rb b/config/routes.rb index 601330d39..963fedcb4 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -66,6 +66,7 @@ Rails.application.routes.draw do end resource :follower_domains, only: [:show, :update] + resource :delete, only: [:show, :destroy] end resources :media, only: [:show] diff --git a/spec/controllers/settings/deletes_controller_spec.rb b/spec/controllers/settings/deletes_controller_spec.rb new file mode 100644 index 000000000..c9e163261 --- /dev/null +++ b/spec/controllers/settings/deletes_controller_spec.rb @@ -0,0 +1,72 @@ +require 'rails_helper' + +describe Settings::DeletesController do + render_views + + describe 'GET #show' do + context 'when signed in' do + let(:user) { Fabricate(:user) } + + before do + sign_in user, scope: :user + end + + it 'renders confirmation page' do + get :show + expect(response).to have_http_status(:success) + end + end + + context 'when not signed in' do + it 'redirects' do + get :show + expect(response).to redirect_to '/auth/sign_in' + end + end + end + + describe 'DELETE #destroy' do + context 'when signed in' do + let(:user) { Fabricate(:user, password: 'petsmoldoggos') } + + before do + sign_in user, scope: :user + end + + context 'with correct password' do + before do + delete :destroy, params: { password: 'petsmoldoggos' } + end + + it 'redirects to sign in page' do + expect(response).to redirect_to '/auth/sign_in' + end + + it 'removes user record' do + expect(User.find_by(id: user.id)).to be_nil + end + + it 'marks account as suspended' do + expect(user.account.reload).to be_suspended + end + end + + context 'with incorrect password' do + before do + delete :destroy, params: { password: 'blaze420' } + end + + it 'redirects back to confirmation page' 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 + end +end