Commit 265b1a3b authored by Winnie Hellmann's avatar Winnie Hellmann Committed by Fatih Acet

Show confirmation modal before deleting account

parent 2cf5dca8
...@@ -14,6 +14,9 @@ If you need to compose a headers object, use the spread operator: ...@@ -14,6 +14,9 @@ If you need to compose a headers object, use the spread operator:
someOtherHeader: '12345', someOtherHeader: '12345',
} }
``` ```
see also http://guides.rubyonrails.org/security.html#cross-site-request-forgery-csrf
and https://github.com/rails/jquery-rails/blob/v4.3.1/vendor/assets/javascripts/jquery_ujs.js#L59-L62
*/ */
const csrf = { const csrf = {
...@@ -53,4 +56,3 @@ if ($.rails) { ...@@ -53,4 +56,3 @@ if ($.rails) {
} }
export default csrf; export default csrf;
<script>
import popupDialog from '../../../vue_shared/components/popup_dialog.vue';
import { __, s__, sprintf } from '../../../locale';
import csrf from '../../../lib/utils/csrf';
export default {
props: {
actionUrl: {
type: String,
required: true,
},
confirmWithPassword: {
type: Boolean,
required: true,
},
username: {
type: String,
required: true,
},
},
data() {
return {
enteredPassword: '',
enteredUsername: '',
isOpen: false,
};
},
components: {
popupDialog,
},
computed: {
csrfToken() {
return csrf.token;
},
inputLabel() {
let confirmationValue;
if (this.confirmWithPassword) {
confirmationValue = __('password');
} else {
confirmationValue = __('username');
}
confirmationValue = `<code>${confirmationValue}</code>`;
return sprintf(
s__('Profiles|Type your %{confirmationValue} to confirm:'),
{ confirmationValue },
false,
);
},
text() {
return sprintf(
s__(`Profiles|
You are about to permanently delete %{yourAccount}, and all of the issues, merge requests, and groups linked to your account.
Once you confirm %{deleteAccount}, it cannot be undone or recovered.`),
{
yourAccount: `<strong>${s__('Profiles|your account')}</strong>`,
deleteAccount: `<strong>${s__('Profiles|Delete Account')}</strong>`,
},
false,
);
},
},
methods: {
canSubmit() {
if (this.confirmWithPassword) {
return this.enteredPassword !== '';
}
return this.enteredUsername === this.username;
},
onSubmit(status) {
if (status) {
if (!this.canSubmit()) {
return;
}
this.$refs.form.submit();
}
this.toggleOpen(false);
},
toggleOpen(isOpen) {
this.isOpen = isOpen;
},
},
};
</script>
<template>
<div>
<popup-dialog
v-if="isOpen"
:title="s__('Profiles|Delete your account?')"
:text="text"
:kind="`danger ${!canSubmit() && 'disabled'}`"
:primary-button-label="s__('Profiles|Delete account')"
@toggle="toggleOpen"
@submit="onSubmit">
<template slot="body" scope="props">
<p v-html="props.text"></p>
<form
ref="form"
:action="actionUrl"
method="post">
<input
type="hidden"
name="_method"
value="delete" />
<input
type="hidden"
name="authenticity_token"
:value="csrfToken" />
<p id="input-label" v-html="inputLabel"></p>
<input
v-if="confirmWithPassword"
name="password"
class="form-control"
type="password"
v-model="enteredPassword"
aria-labelledby="input-label" />
<input
v-else
name="username"
class="form-control"
type="text"
v-model="enteredUsername"
aria-labelledby="input-label" />
</form>
</template>
</popup-dialog>
<button
type="button"
class="btn btn-danger"
@click="toggleOpen(true)">
{{ s__('Profiles|Delete account') }}
</button>
</div>
</template>
import Vue from 'vue';
import deleteAccountModal from './components/delete_account_modal.vue';
const deleteAccountModalEl = document.getElementById('delete-account-modal');
// eslint-disable-next-line no-new
new Vue({
el: deleteAccountModalEl,
components: {
deleteAccountModal,
},
render(createElement) {
return createElement('delete-account-modal', {
props: {
actionUrl: deleteAccountModalEl.dataset.actionUrl,
confirmWithPassword: !!deleteAccountModalEl.dataset.confirmWithPassword,
username: deleteAccountModalEl.dataset.username,
},
});
},
});
...@@ -62,7 +62,7 @@ export default { ...@@ -62,7 +62,7 @@ export default {
:primary-button-label="__('Discard changes')" :primary-button-label="__('Discard changes')"
kind="warning" kind="warning"
:title="__('Are you sure?')" :title="__('Are you sure?')"
:body="__('Are you sure you want to discard your changes?')" :text="__('Are you sure you want to discard your changes?')"
@toggle="toggleDialogOpen" @toggle="toggleDialogOpen"
@submit="dialogSubmitted" @submit="dialogSubmitted"
/> />
......
...@@ -7,7 +7,7 @@ export default { ...@@ -7,7 +7,7 @@ export default {
type: String, type: String,
required: true, required: true,
}, },
body: { text: {
type: String, type: String,
required: true, required: true,
}, },
...@@ -63,7 +63,9 @@ export default { ...@@ -63,7 +63,9 @@ export default {
<h4 class="modal-title">{{this.title}}</h4> <h4 class="modal-title">{{this.title}}</h4>
</div> </div>
<div class="modal-body"> <div class="modal-body">
<p>{{this.body}}</p> <slot name="body" :text="text">
<p>{{text}}</p>
</slot>
</div> </div>
<div class="modal-footer"> <div class="modal-footer">
<button <button
......
.modal-header {
padding: #{3 * $grid-size} #{2 * $grid-size};
.page-title {
margin-top: 0;
}
}
.modal-body { .modal-body {
position: relative; position: relative;
padding: 15px; padding: #{3 * $grid-size} #{2 * $grid-size};
.form-actions { .form-actions {
margin: -$gl-padding + 1; margin: #{2 * $grid-size} #{-2 * $grid-size} #{-2 * $grid-size};
margin-top: 15px;
} }
.text-danger { .text-danger {
......
/* /*
* Layout * Layout
*/ */
$grid-size: 8px;
$gutter_collapsed_width: 62px; $gutter_collapsed_width: 62px;
$gutter_width: 290px; $gutter_width: 290px;
$gutter_inner_width: 250px; $gutter_inner_width: 250px;
......
...@@ -25,18 +25,33 @@ class RegistrationsController < Devise::RegistrationsController ...@@ -25,18 +25,33 @@ class RegistrationsController < Devise::RegistrationsController
end end
def destroy def destroy
current_user.delete_async(deleted_by: current_user) if destroy_confirmation_valid?
current_user.delete_async(deleted_by: current_user)
respond_to do |format| session.try(:destroy)
format.html do redirect_to new_user_session_path, status: 303, notice: s_('Profiles|Account scheduled for removal.')
session.try(:destroy) else
redirect_to new_user_session_path, status: 302, notice: "Account scheduled for removal." redirect_to profile_account_path, status: 303, alert: destroy_confirmation_failure_message
end
end end
end end
protected protected
def destroy_confirmation_valid?
if current_user.confirm_deletion_with_password?
current_user.valid_password?(params[:password])
else
current_user.username == params[:username]
end
end
def destroy_confirmation_failure_message
if current_user.confirm_deletion_with_password?
s_('Profiles|Invalid password')
else
s_('Profiles|Invalid username')
end
end
def build_resource(hash = nil) def build_resource(hash = nil)
super super
end end
......
...@@ -654,6 +654,10 @@ class User < ActiveRecord::Base ...@@ -654,6 +654,10 @@ class User < ActiveRecord::Base
Ability.allowed?(self, action, subject) Ability.allowed?(self, action, subject)
end end
def confirm_deletion_with_password?
!password_automatically_set? && allow_password_authentication?
end
def first_name def first_name
name.split.first unless name.blank? name.split.first unless name.blank?
end end
......
...@@ -97,21 +97,29 @@ ...@@ -97,21 +97,29 @@
.row.prepend-top-default .row.prepend-top-default
.col-lg-4.profile-settings-sidebar .col-lg-4.profile-settings-sidebar
%h4.prepend-top-0.danger-title %h4.prepend-top-0.danger-title
Remove account = s_('Profiles|Delete account')
.col-lg-8 .col-lg-8
- if @user.can_be_removed? && can?(current_user, :destroy_user, @user) - if @user.can_be_removed? && can?(current_user, :destroy_user, @user)
%p %p
Deleting an account has the following effects: = s_('Profiles|Deleting an account has the following effects:')
= render 'users/deletion_guidance', user: current_user = render 'users/deletion_guidance', user: current_user
= link_to 'Delete account', user_registration_path, data: { confirm: "REMOVE #{current_user.name}? Are you sure?" }, method: :delete, class: "btn btn-remove"
#delete-account-modal{ data: { action_url: user_registration_path,
confirm_with_password: ('true' if current_user.confirm_deletion_with_password?),
username: current_user.username } }
%button.btn.btn-danger.disabled
= s_('Profiles|Delete account')
- else - else
- if @user.solo_owned_groups.present? - if @user.solo_owned_groups.present?
%p %p
Your account is currently an owner in these groups: = s_('Profiles|Your account is currently an owner in these groups:')
%strong= @user.solo_owned_groups.map(&:name).join(', ') %strong= @user.solo_owned_groups.map(&:name).join(', ')
%p %p
You must transfer ownership or delete these groups before you can delete your account. = s_('Profiles|You must transfer ownership or delete these groups before you can delete your account.')
- else - else
%p %p
You don't have access to delete this user. = s_("Profiles|You don't have access to delete this user.")
.append-bottom-default .append-bottom-default
- content_for :page_specific_javascripts do
= webpack_bundle_tag('account')
---
title: Show confirmation modal before deleting account
merge_request: 14360
author:
type: changed
...@@ -26,6 +26,7 @@ var config = { ...@@ -26,6 +26,7 @@ var config = {
}, },
context: path.join(ROOT_PATH, 'app/assets/javascripts'), context: path.join(ROOT_PATH, 'app/assets/javascripts'),
entry: { entry: {
account: './profile/account/index.js',
balsamiq_viewer: './blob/balsamiq_viewer.js', balsamiq_viewer: './blob/balsamiq_viewer.js',
blob: './blob_edit/blob_bundle.js', blob: './blob_edit/blob_bundle.js',
boards: './boards/boards_bundle.js', boards: './boards/boards_bundle.js',
......
...@@ -76,12 +76,68 @@ describe RegistrationsController do ...@@ -76,12 +76,68 @@ describe RegistrationsController do
sign_in(user) sign_in(user)
end end
it 'schedules the user for destruction' do def expect_failure(message)
expect(DeleteUserWorker).to receive(:perform_async).with(user.id, user.id, {}) expect(flash[:alert]).to eq(message)
expect(response.status).to eq(303)
expect(response).to redirect_to profile_account_path
end
def expect_password_failure
expect_failure('Invalid password')
end
def expect_username_failure
expect_failure('Invalid username')
end
def expect_success
expect(flash[:notice]).to eq 'Account scheduled for removal.'
expect(response.status).to eq(303)
expect(response).to redirect_to new_user_session_path
end
post(:destroy) context 'user requires password confirmation' do
it 'fails if password confirmation is not provided' do
post :destroy
expect(response.status).to eq(302) expect_password_failure
end
it 'fails if password confirmation is wrong' do
post :destroy, password: 'wrong password'
expect_password_failure
end
it 'succeeds if password is confirmed' do
post :destroy, password: '12345678'
expect_success
end
end
context 'user does not require password confirmation' do
before do
stub_application_setting(password_authentication_enabled: false)
end
it 'fails if username confirmation is not provided' do
post :destroy
expect_username_failure
end
it 'fails if username confirmation is wrong' do
post :destroy, username: 'wrong username'
expect_username_failure
end
it 'succeeds if username is confirmed' do
post :destroy, username: user.username
expect_success
end
end end
end end
end end
...@@ -28,8 +28,7 @@ feature 'Issue Detail', :js do ...@@ -28,8 +28,7 @@ feature 'Issue Detail', :js do
fill_in 'issue-title', with: 'issue title' fill_in 'issue-title', with: 'issue title'
click_button 'Save' click_button 'Save'
visit profile_account_path Users::DestroyService.new(user).execute(user)
click_link 'Delete account'
visit project_issue_path(project, issue) visit project_issue_path(project, issue)
end end
......
...@@ -12,11 +12,47 @@ describe 'Profile account page' do ...@@ -12,11 +12,47 @@ describe 'Profile account page' do
visit profile_account_path visit profile_account_path
end end
it { expect(page).to have_content('Remove account') } it { expect(page).to have_content('Delete account') }
it 'deletes the account' do it 'does not immediately delete the account' do
expect { click_link 'Delete account' }.to change { User.where(id: user.id).count }.by(-1) click_button 'Delete account'
expect(current_path).to eq(new_user_session_path)
expect(User.exists?(user.id)).to be_truthy
end
it 'deletes user', :js do
click_button 'Delete account'
fill_in 'password', with: '12345678'
page.within '.popup-dialog' do
click_button 'Delete account'
end
expect(page).to have_content('Account scheduled for removal')
expect(User.exists?(user.id)).to be_falsy
end
it 'shows invalid password flash message', :js do
click_button 'Delete account'
fill_in 'password', with: 'testing123'
page.within '.popup-dialog' do
click_button 'Delete account'
end
expect(page).to have_content('Invalid password')
end
it 'does not show delete button when user owns a group' do
group = create(:group)
group.add_owner(user)
visit profile_account_path
expect(page).not_to have_button('Delete account')
expect(page).to have_content("Your account is currently an owner in these groups: #{group.name}")
end end
end end
......
import Vue from 'vue';
import deleteAccountModal from '~/profile/account/components/delete_account_modal.vue';
import mountComponent from '../../../helpers/vue_mount_component_helper';
describe('DeleteAccountModal component', () => {
const actionUrl = `${gl.TEST_HOST}/delete/user`;
const username = 'hasnoname';
let Component;
let vm;
beforeEach(() => {
Component = Vue.extend(deleteAccountModal);
});
afterEach(() => {
vm.$destroy();
});
const findElements = () => {
const confirmation = vm.confirmWithPassword ? 'password' : 'username';
return {
form: vm.$refs.form,
input: vm.$el.querySelector(`[name="${confirmation}"]`),
submitButton: vm.$el.querySelector('.btn-danger'),
};
};
describe('with password confirmation', () => {
beforeEach((done) => {
vm = mountComponent(Component, {
actionUrl,
confirmWithPassword: true,
username,
});
vm.isOpen = true;
Vue.nextTick()
.then(done)
.catch(done.fail);
});
it('does not accept empty password', (done) => {
const { form, input, submitButton } = findElements();
spyOn(form, 'submit');
input.value = '';
input.dispatchEvent(new Event('input'));
Vue.nextTick()
.then(() => {
expect(vm.enteredPassword).toBe(input.value);
expect(submitButton).toHaveClass('disabled');
submitButton.click();
expect(form.submit).not.toHaveBeenCalled();
})
.then(done)
.catch(done.fail);
});
it('submits form with password', (done) => {
const { form, input, submitButton } = findElements();
spyOn(form, 'submit');
input.value = 'anything';
input.dispatchEvent(new Event('input'));
Vue.nextTick()
.then(() => {
expect(vm.enteredPassword).toBe(input.value);
expect(submitButton).not.toHaveClass('disabled');
submitButton.click();
expect(form.submit).toHaveBeenCalled();
})
.then(done)
.catch(done.fail);
});
});
describe('with username confirmation', () => {
beforeEach((done) => {
vm = mountComponent(Component, {
actionUrl,
confirmWithPassword: false,
username,
});
vm.isOpen = true;
Vue.nextTick()
.then(done)
.catch(done.fail);
});
it('does not accept wrong username', (done) => {
const { form, input, submitButton } = findElements();
spyOn(form, 'submit');
input.value = 'this is wrong';
input.dispatchEvent(new Event('input'));
Vue.nextTick()
.then(() => {
expect(vm.enteredUsername).toBe(input.value);
expect(submitButton).toHaveClass('disabled');
submitButton.click();
expect(form.submit).not.toHaveBeenCalled();
})
.then(done)
.catch(done.fail);
});
it('submits form with correct username', (done) => {
const { form, input, submitButton } = findElements();
spyOn(form, 'submit');
input.value = username;
input.dispatchEvent(new Event('input'));
Vue.nextTick()
.then(() => {
expect(vm.enteredUsername).toBe(input.value);
expect(submitButton).not.toHaveClass('disabled');
submitButton.click();
expect(form.submit).toHaveBeenCalled();
})
.then(done)
.catch(done.fail);
});
});
});
...@@ -2282,4 +2282,49 @@ describe User do ...@@ -2282,4 +2282,49 @@ describe User do
end end
end end
end end
describe '#confirm_deletion_with_password?' do
where(
password_automatically_set: [true, false],
ldap_user: [true, false],
password_authentication_disabled: [true, false]
)
with_them do
let!(:user) { create(:user, password_automatically_set: password_automatically_set) }
let!(:identity) { create(:identity, user: user) if ldap_user }
# Only confirm deletion with password if all inputs are false
let(:expected) { !(password_automatically_set || ldap_user || password_authentication_disabled) }
before do
stub_application_setting(password_authentication_enabled: !password_authentication_disabled)
end
it 'returns false unless all inputs are true' do
expect(user.confirm_deletion_with_password?).to eq(expected)
end
end
end
describe '#delete_async' do
let(:user) { create(:user) }
let(:deleted_by) { create(:user) }
it 'blocks the user then schedules them for deletion if a hard delete is specified' do
expect(DeleteUserWorker).to receive(:perform_async).with(deleted_by.id, user.id, hard_delete: true)
user.delete_async(deleted_by: deleted_by, params: { hard_delete: true })
expect(user).to be_blocked
end
it 'schedules user for deletion without blocking them' do
expect(DeleteUserWorker).to receive(:perform_async).with(deleted_by.id, user.id, {})
user.delete_async(deleted_by: deleted_by)
expect(user).not_to be_blocked
end
end
end end
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment