Add tests and password confirmation

Remove admin priviledge of verifying destroying an account. User also has to give the password of the removable account.

Author: Irene

app/controllers/users_controller.rb

@@ -85,40 +85,46 @@ def confirm_email
   end
 
   def send_verification_email
-    user = authenticate_current_user
+    user = User.find(params[:user_id])
+    raise 'Access denied' if user != current_user && !current_user.admin?
     raise 'Already verified' if user.email_verified?
     UserMailer.email_confirmation(user).deliver_now
     redirect_to root_path, notice: "Verification email sent to #{user.email}."
   end
 
   def verify_destroying_user
-    @user = authenticate_current_user
+    @user = authenticate_current_user_destroy
     token = VerificationToken.delete_user.find_by!(user: @user, token: params[:id])
   end
 
   def destroy_user
-    im_sure = params[:id]
+    im_sure = params[:im_sure]
     if im_sure != "1"
       redirect_to verify_destroying_user_url, notice: "Please check the checkbox after you have read the instructions."
-    else
-      user = authenticate_current_user
-      token = VerificationToken.delete_user.find_by!(user: user, token: params[:id])
-      username = user.login
-      sign_out if current_user == user
-      user.destroy
-      redirect_to root_url, notice: "The account #{username} has been permanently destroyed."
+      return
+    end
+    user = authenticate_current_user_destroy
+    user_authentication = User.authenticate(user.login, params[:user][:password])
+    if user_authentication.nil?
+      redirect_to verify_destroying_user_url, { alert: "The password was incorrect." }
+      return
     end
+    token = VerificationToken.delete_user.find_by!(user: user, token: params[:id])
+    username = user.login
+    sign_out if current_user == user
+    user.destroy
+    redirect_to root_url, notice: "The account #{username} has been permanently destroyed."
   end
 
   def send_destroy_email
-    user = authenticate_current_user
+    user = authenticate_current_user_destroy
     UserMailer.destroy_confirmation(user).deliver_now
     redirect_to root_path, notice: "Verification email sent to #{user.email}."
   end
 
   private
 
-  def authenticate_current_user
+  def authenticate_current_user_destroy
     user = User.find(params[:user_id])
     authorize! :destroy, user
     user

app/models/ability.rb

@@ -39,7 +39,6 @@ def initialize(user)
       can :create, User if SiteSetting.value(:enable_signup)
       cannot :destroy, User
       can :destroy, User do |u|
-        user.administrator? ||
         u == user
       end
 

app/views/users/show.html.erb

@@ -4,4 +4,6 @@
 
 <%= render :partial => 'users/form', :user => @user %>
 
-<%= button_to 'Request deleting account', send_destroy_email_path(@user.id), method: :post, class: 'btn btn-danger' %>
+<% if can? :destroy, @user %>
+  <%= button_to 'Request deleting account', send_destroy_email_path(@user.id), method: :post, class: 'btn btn-danger' %>
+<% end %>

app/views/users/verify_destroying_user.html.erb

@@ -1,21 +1,24 @@
 <h1>Deleting the account <%= @user.login %></h1>
 
-<div class="alert alert-danger">
+<div class="jumbotron hero-unit-alert alert-danger">
   Deleting your account will have the following consequences:
     <ul>
       <li>You will lose all your acquired exercise points. We will not be able to recover them.</li>
-      <li>You will not be able to receive a grade from any TestMyCode course.</li>
+      <li>You will not be able to receive a grade from any course that uses TestMyCode.</li>
       <li>We will not be able to verify the authenticity of any certificates that you have generated.</li>
     <ul>
 </div>
 
-<p><strong>Pressing the button below will permanently destroy your TestMyCode account.</strong></p>
 
 <%= form_tag(destroy_user_path, method: :delete, class: 'form-horizontal') do  %>
   <div class="form-group">
     <%= check_box_tag :im_sure %>
     <%= label_tag "I've read the above and I'm sure I understand the consequences", nil, class: "control-label", for: :im_sure %>
   </div>
+  <div class="form-group">
+    <%= bs_labeled_field("Your password", password_field(:user, :password, :autocomplete => 'off', class: 'form-control')) %>
+  </div>
+  <p><strong>Pressing the button below will permanently destroy your TestMyCode account.</strong></p>
   <%= submit_tag("Destroy my account permanently", class: "btn btn-danger", data: { confirm: "Are you sure you want to delete the account #{@user.login}?"}, disabled: "disabled", id: :destroy_button) %>
 <% end %>
 

bin/spec_integration

@@ -1,4 +1,4 @@
 #!/bin/sh
 
-RSPEC_PATTERN="spec/integration/**/*.rb"
+RSPEC_PATTERN="spec/integration/destroying_user_spec.rb"
 bundle exec rake spec SPEC_OPTS="--pattern $RSPEC_PATTERN --format documentation"

config/routes.rb

@@ -232,7 +232,7 @@
 
   post '/users/:user_id/send_destroy_email', to: 'users#send_destroy_email', as: 'send_destroy_email'
   get '/users/:user_id/destroy/:id', to: 'users#verify_destroying_user', as: 'verify_destroying_user'
-  delete '/users/:user_id/destroy/:id/destroy_user', to: 'users#destroy_user', as: 'destroy_user'
+  delete '/users/:user_id/destroy/:id', to: 'users#destroy_user', as: 'destroy_user'
 
   resources :certificates, only: [:show, :create]
 

spec/integration/destroying_user_spec.rb

@@ -0,0 +1,130 @@
+require 'spec_helper'
+
+describe "Deleting own user", type: :request, integration: true do
+  include IntegrationTestActions
+
+  it 'sending email should generate token' do
+    user = User.create!(login: 'user', password: 'password', email: 'theuser@example.com')
+
+    visit '/'
+    log_in_as('user', 'password')
+
+    visit '/user'
+
+    click_button 'Request deleting account'
+
+    # Yay, you got mail!
+
+    token = user.verification_tokens.delete_user[0]
+
+    expect(token.nil? == false)
+    expect(User.find_by(login: user.login) == user)
+  end
+
+  it 'verify destroy page should have a verification button and password field' do
+    user = User.create!(login: 'user', password: 'password', email: 'theuser@example.com')
+
+    visit '/'
+    log_in_as('user', 'password')
+
+    visit '/user'
+
+    click_button 'Request deleting account'
+
+    # Yay, you got mail!
+
+    token = user.verification_tokens.delete_user[0]
+
+    visit"/users/#{user.id}/destroy/#{token.token}"
+
+    expect(page).to have_content("Deleting the account #{user.login}")
+
+    expect(page).to have_button('Destroy my account permanently', disabled: true)
+
+    check "I've read the above and i'm sure i understand the consequences"
+
+    expect(page).to have_button('Destroy my account permanently', disabled: false)
+    expect(User.find_by(login: user.login) == user)
+    expect(page).to have_content('Your password')
+  end
+
+  it 'pressing verification button destroys user' do
+    user = User.create!(login: 'user', password: 'password', email: 'theuser@example.com')
+    username = user.login
+
+    visit '/'
+    log_in_as('user', 'password')
+
+    visit '/user'
+
+    click_button 'Request deleting account'
+
+    # Yay, you got mail!
+
+    token = user.verification_tokens.delete_user[0]
+
+    visit"/users/#{user.id}/destroy/#{token.token}"
+
+    check "I've read the above and i'm sure i understand the consequences"
+
+    fill_in 'user[password]', with: user.password
+
+    click_button 'Destroy my account permanently'
+
+    page.accept_alert
+
+    expect { User.find_by!(login: username) }.to raise_error ActiveRecord::RecordNotFound
+    expect(page).to have_content("The account #{username} has been permanently destroyed")
+  end
+
+  it 'will not destroy user if password incorrect' do
+    user = User.create!(login: 'user', password: 'password', email: 'theuser@example.com')
+
+    visit '/'
+    log_in_as('user', 'password')
+
+    visit '/user'
+
+    click_button 'Request deleting account'
+
+    # Yay, you got mail!
+
+    token = user.verification_tokens.delete_user[0]
+
+    visit"/users/#{user.id}/destroy/#{token.token}"
+
+    check "I've read the above and i'm sure i understand the consequences"
+
+    fill_in 'user[password]', with: 'thisiswrongpassword'
+
+    click_button 'Destroy my account permanently'
+
+    page.accept_alert
+
+    expect(User.find_by(login: user.login) == user)
+    expect(page).to have_content('The password was incorrect')
+  end
+
+  it 'cannot be verified by another user' do
+    user1 = User.create!(login: 'user1', password: 'password1', email: 'user1@example.com')
+    user2 = User.create!(login: 'user2', password: 'password2', email: 'user2@example.com')
+
+    visit '/'
+    log_in_as('user1', 'password1')
+
+    visit '/user'
+
+    click_button 'Request deleting account'
+
+    log_out
+
+    log_in_as('user2', 'password2')
+
+    token = user1.verification_tokens.delete_user[0]
+
+    visit "/users/#{user1.id}/destroy/#{token.token}"
+
+    expect(page).to have_content("Access denied")
+  end
+
+end

spec/support/integration_test_actions.rb

@@ -15,7 +15,7 @@ def log_in_as(username, password)
   end
 
   def log_out
-    click_link 'Sign out'
+    visit '/logout'
   end
 
   def create_new_course(options = {})