0001-require-explicit-confirmation-before-deleting-a-user.patch

Jens Krämer, 2020-12-09 04:12

Download (7.65 KB)

View differences:

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(
876
-