Feature #34417 » 0001-require-explicit-confirmation-before-deleting-a-user.patch
| app/controllers/users_controller.rb | ||
|---|---|---|
| 184 | 184 |
end |
| 185 | 185 | |
| 186 | 186 |
def destroy |
| 187 |
@user.destroy |
|
| 188 |
respond_to do |format| |
|
| 189 |
format.html {redirect_back_or_default(users_path)}
|
|
| 190 |
format.api {render_api_ok}
|
|
| 187 |
if api_request? || params[:lock] || params[:confirm] == @user.login |
|
| 188 |
if params[:lock] |
|
| 189 |
@user.update_attribute :status, User::STATUS_LOCKED |
|
| 190 |
else |
|
| 191 |
@user.destroy |
|
| 192 |
end |
|
| 193 |
respond_to do |format| |
|
| 194 |
format.html {redirect_back_or_default(users_path)}
|
|
| 195 |
format.api {render_api_ok}
|
|
| 196 |
end |
|
| 191 | 197 |
end |
| 192 | 198 |
end |
| 193 | 199 | |
| app/views/users/destroy.html.erb | ||
|---|---|---|
| 1 |
<%= title l(:label_confirmation) %> |
|
| 2 | ||
| 3 |
<%= form_tag user_path(@user), method: :delete do %> |
|
| 4 |
<div class="warning"> |
|
| 5 |
<p><strong><%= @user.name %> (<%= @user.login %>)</strong></p> |
|
| 6 | ||
| 7 |
<p><%= l :text_user_destroy_confirmation, login: @user.login %></p> |
|
| 8 | ||
| 9 |
<p> |
|
| 10 |
<label for="confirm"><%= l :field_login %></label> |
|
| 11 |
<%= text_field_tag 'confirm' %> |
|
| 12 |
</p> |
|
| 13 |
</div> |
|
| 14 | ||
| 15 |
<p> |
|
| 16 |
<%= submit_tag l(:button_delete) %> |
|
| 17 |
<%= submit_tag l(:button_lock), name: 'lock' unless @user.locked? %> |
|
| 18 |
<%= link_to l(:button_cancel), users_path %> |
|
| 19 |
</p> |
|
| 20 |
<% end %> |
|
| 21 | ||
| app/views/users/index.html.erb | ||
|---|---|---|
| 53 | 53 |
<td class="last_login_on"><%= format_time(user.last_login_on) unless user.last_login_on.nil? %></td> |
| 54 | 54 |
<td class="buttons"> |
| 55 | 55 |
<%= change_status_link(user) %> |
| 56 |
<%= delete_link user_path(user, :back_url => request.original_fullpath) unless User.current == user %> |
|
| 56 |
<%= delete_link user_path(user, :back_url => request.original_fullpath), :data => {} unless User.current == user %>
|
|
| 57 | 57 |
</td> |
| 58 | 58 |
</tr> |
| 59 | 59 |
<% end -%> |
| config/locales/de.yml | ||
|---|---|---|
| 1369 | 1369 |
error_invalid_size_parameter: Invalid size parameter |
| 1370 | 1370 |
error_attachment_not_found: Attachment %{name} not found
|
| 1371 | 1371 |
field_twofa_scheme: Two-factor authentication scheme |
| 1372 | ||
| 1373 |
text_user_destroy_confirmation: "Wollen Sie diesen Benutzer inklusive aller Referenzen darauf wirklich löschen? Dies kann nicht rückgängig gemacht werden. Oftmals ist es besser, einen Benutzer lediglich zu sperren. Geben Sie bitte zur Bestätigung den Login des Benutzers (%{login}) ein."
|
|
| config/locales/en.yml | ||
|---|---|---|
| 1344 | 1344 |
twofa_text_backup_codes_hint: Use these codes instead of a one-time password should you not have access to your second factor. Each code can only be used once. It is recommended to print and store them in a safe place. |
| 1345 | 1345 |
twofa_text_backup_codes_created_at: Backup codes generated %{datetime}.
|
| 1346 | 1346 |
twofa_backup_codes_already_shown: Backup codes cannot be shown again, please <a data-method="post" href="%{bc_path}">generate new backup codes</a> if required.
|
| 1347 | ||
| 1348 |
text_user_destroy_confirmation: "Are you sure you want to delete this user and remove all references to them? This cannot be undone. Often, locking a user instead of deleting them is the better solution. To confirm, please enter their login (%{login}) below."
|
|
| test/functional/users_controller_test.rb | ||
|---|---|---|
| 770 | 770 |
# if user is already locked, destroying should not send a second mail |
| 771 | 771 |
# (for active admins see furtherbelow) |
| 772 | 772 |
ActionMailer::Base.deliveries.clear |
| 773 |
delete :destroy, :params => {:id => 1}
|
|
| 773 |
delete :destroy, :params => {:id => 1, :confirm => User.find(1).login}
|
|
| 774 | 774 |
assert_nil ActionMailer::Base.deliveries.last |
| 775 | 775 | |
| 776 | 776 |
end |
| ... | ... | |
| 834 | 834 | |
| 835 | 835 |
def test_destroy |
| 836 | 836 |
assert_difference 'User.count', -1 do |
| 837 |
delete :destroy, :params => {:id => 2}
|
|
| 837 |
delete :destroy, :params => {:id => 2, :confirm => User.find(2).login}
|
|
| 838 | 838 |
end |
| 839 | 839 |
assert_redirected_to '/users' |
| 840 | 840 |
assert_nil User.find_by_id(2) |
| 841 | 841 |
end |
| 842 | 842 | |
| 843 |
def test_destroy_with_lock_param_should_lock_instead |
|
| 844 |
assert_no_difference 'User.count' do |
|
| 845 |
delete :destroy, :params => {:id => 2, :lock => 'lock'}
|
|
| 846 |
end |
|
| 847 |
assert_redirected_to '/users' |
|
| 848 |
assert User.find_by_id(2).locked? |
|
| 849 |
end |
|
| 850 | ||
| 851 |
def test_destroy_should_require_confirmation |
|
| 852 |
assert_no_difference 'User.count' do |
|
| 853 |
delete :destroy, :params => {:id => 2}
|
|
| 854 |
end |
|
| 855 |
assert_response :success |
|
| 856 |
assert_select '.warning', :text => /Are you sure you want to delete this user/ |
|
| 857 |
end |
|
| 858 | ||
| 859 |
def test_destroy_should_require_correct_confirmation |
|
| 860 |
assert_no_difference 'User.count' do |
|
| 861 |
delete :destroy, :params => {:id => 2, :confirm => 'wrong'}
|
|
| 862 |
end |
|
| 863 |
assert_response :success |
|
| 864 |
assert_select '.warning', :text => /Are you sure you want to delete this user/ |
|
| 865 |
end |
|
| 866 | ||
| 843 | 867 |
def test_destroy_should_be_denied_for_non_admin_users |
| 844 | 868 |
@request.session[:user_id] = 3 |
| 845 | 869 | |
| 846 | 870 |
assert_no_difference 'User.count' do |
| 847 |
get :destroy, :params => {:id => 2}
|
|
| 871 |
delete :destroy, :params => {:id => 2, :confirm => User.find(2).login}
|
|
| 848 | 872 |
end |
| 849 | 873 |
assert_response 403 |
| 850 | 874 |
end |
| ... | ... | |
| 852 | 876 |
def test_destroy_should_be_denied_for_anonymous |
| 853 | 877 |
assert User.find(6).anonymous? |
| 854 | 878 |
assert_no_difference 'User.count' do |
| 855 |
put :destroy, :params => {:id => 6}
|
|
| 879 |
delete :destroy, :params => {:id => 6, :confirm => User.find(6).login}
|
|
| 856 | 880 |
end |
| 857 | 881 |
assert_response 404 |
| 858 | 882 |
end |
| 859 | 883 | |
| 860 | 884 |
def test_destroy_should_redirect_to_back_url_param |
| 861 | 885 |
assert_difference 'User.count', -1 do |
| 862 |
delete :destroy, :params => {:id => 2, :back_url => '/users?name=foo'}
|
|
| 886 |
delete :destroy, :params => {:id => 2,
|
|
| 887 |
:confirm => User.find(2).login, |
|
| 888 |
:back_url => '/users?name=foo'} |
|
| 863 | 889 |
end |
| 864 | 890 |
assert_redirected_to '/users?name=foo' |
| 865 | 891 |
end |
| ... | ... | |
| 869 | 895 |
user.admin = true |
| 870 | 896 |
user.save! |
| 871 | 897 |
ActionMailer::Base.deliveries.clear |
| 872 |
delete :destroy, :params => {:id => user.id}
|
|
| 898 |
delete :destroy, :params => {:id => user.id, :confirm => user.login}
|
|
| 873 | 899 | |
| 874 | 900 |
assert_not_nil (mail = ActionMailer::Base.deliveries.last) |
| 875 | 901 |
assert_mail_body_match( |
