Enable ability for administrators to delete users (#7296).

User's personal data (eg. preferences, tokens, private queries...) are deleted, public data (eg. issues, wiki edits, attachments...) are reassigned to the anonymous user.

git-svn-id: svn+ssh://rubyforge.org/var/svn/redmine/trunk@4729 e93f8b46-1217-0410-a6f0-8f06a7374b81

diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb
index 2a213f5e85f2ad04d8d5f732ce09d37b7c3b0a13..c7f9dcf1b94f2a39d62d8d92774f56d8306969be 100644 (file)
--- a/app/controllers/users_controller.rb
+++ b/app/controllers/users_controller.rb
@@ -19,8 +19,8 @@ class UsersController < ApplicationController
   layout 'admin'
   
   before_filter :require_admin, :except => :show
-  before_filter :find_user, :only => [:show, :edit, :update, :edit_membership, :destroy_membership]
-  accept_key_auth :index, :show, :create, :update
+  before_filter :find_user, :only => [:show, :edit, :update, :destroy, :edit_membership, :destroy_membership]
+  accept_key_auth :index, :show, :create, :update, :destroy
 
   helper :sort
   include SortHelper
@@ -177,6 +177,15 @@ class UsersController < ApplicationController
     redirect_to :controller => 'users', :action => 'edit', :id => @user
   end
 
+  verify :method => :delete, :only => :destroy, :render => {:nothing => true, :status => :method_not_allowed }
+  def destroy
+    @user.destroy
+    respond_to do |format|
+      format.html { redirect_to(users_url) }
+      format.api  { head :ok }
+    end
+  end
+
   def edit_membership
     @membership = Member.edit_membership(params[:membership_id], params[:membership], @user)
     @membership.save if request.post?

diff --git a/app/views/users/edit.rhtml b/app/views/users/edit.rhtml
index 0d9cb0133e6753760d6be3ae37c4e22e55398a9f..0c5883e8b8390e792eea881728e7d1226dcd1ed3 100644 (file)
--- a/app/views/users/edit.rhtml
+++ b/app/views/users/edit.rhtml
@@ -1,6 +1,7 @@
 <div class="contextual">
 <%= link_to l(:label_profile), user_path(@user), :class => 'icon icon-user' %>
 <%= change_status_link(@user) %>
+<%= link_to(l(:button_delete), @user, :confirm => l(:text_are_you_sure), :method => :delete, :class => 'icon icon-del') if User.current != @user %>
 </div>
 
 <h2><%= link_to l(:label_user_plural), :controller => 'users', :action => 'index' %> &#187; <%=h @user.login %></h2>

diff --git a/app/views/users/index.rhtml b/app/views/users/index.rhtml
index 69ad7374728be1972f19ab9128dad54436a98290..07308265ee2e1db847013bc1c01ea4e31d7e7843 100644 (file)
--- a/app/views/users/index.rhtml
+++ b/app/views/users/index.rhtml
@@ -37,7 +37,10 @@
        <td align="center"><%= checked_image user.admin? %></td>
        <td class="created_on" align="center"><%= format_time(user.created_on) %></td>
        <td class="last_login_on" align="center"><%= format_time(user.last_login_on) unless user.last_login_on.nil? %></td>
-    <td><small><%= change_status_link(user) %></small></td>
+    <td class="buttons">
+       <%= change_status_link(user) %>
+       <%= link_to(l(:button_delete), user, :confirm => l(:text_are_you_sure), :method => :delete, :class => 'icon icon-del') unless User.current == user %>
+    </td>
   </tr>
 <% end -%>
   </tbody>

diff --git a/config/routes.rb b/config/routes.rb
index 3b37b515089ff54d4b6fcd36bffc1a57540bdce2..383d9cf89e3dc5b21d555d41242a28c589da4962 100644 (file)
--- a/config/routes.rb
+++ b/config/routes.rb
@@ -122,8 +122,7 @@ ActionController::Routing::Routes.draw do |map|
   map.resources :users, :member => {
     :edit_membership => :post,
     :destroy_membership => :post
-  },
-  :except => [:destroy]
+  }
 
   # For nice "roadmap" in the url for the index action
   map.connect 'projects/:project_id/roadmap', :controller => 'versions', :action => 'index'

diff --git a/test/functional/users_controller_test.rb b/test/functional/users_controller_test.rb
index 8aa311ebb8770b4614a48c7cb8b89b1aec548452..6837deab4ff1745827b2157eed8cbd8aa3103257 100644 (file)
--- a/test/functional/users_controller_test.rb
+++ b/test/functional/users_controller_test.rb
@@ -264,6 +264,30 @@ class UsersControllerTest < ActionController::TestCase
     assert u.check_password?('newpass')
   end
   
+  def test_destroy
+    assert_difference 'User.count', -1 do
+      delete :destroy, :id => 2
+    end
+    assert_redirected_to '/users'
+    assert_nil User.find_by_id(2)
+  end
+
+  def test_destroy_should_not_accept_get_requests
+    assert_no_difference 'User.count' do
+      get :destroy, :id => 2
+    end
+    assert_response 405
+  end
+
+  def test_destroy_should_be_denied_for_non_admin_users
+    @request.session[:user_id] = 3
+    
+    assert_no_difference 'User.count' do
+      get :destroy, :id => 2
+    end
+    assert_response 403
+  end
+  
   def test_edit_membership
     post :edit_membership, :id => 2, :membership_id => 1,
                            :membership => { :role_ids => [2]}

diff --git a/test/integration/api_test/users_test.rb b/test/integration/api_test/users_test.rb
index e1eb7a237c2a8c6b5876a99c40cf7719638daf63..00dc4cc95c72288ab61f0edf29a41a22c3d6f21c 100644 (file)
--- a/test/integration/api_test/users_test.rb
+++ b/test/integration/api_test/users_test.rb
@@ -245,26 +245,36 @@ class ApiTest::UsersTest < ActionController::IntegrationTest
         end
       end
     end
+  end
     
-    context "DELETE /users/2" do
-      context ".xml" do
-        should "not be allowed" do
-          assert_no_difference('User.count') do
-            delete '/users/2.xml'
-          end
-          
-          assert_response :method_not_allowed
+  context "DELETE /users/2" do
+    context ".xml" do
+      should_allow_api_authentication(:delete,
+        '/users/2.xml',
+        {},
+        {:success_code => :ok})
+      
+      should "delete user" do
+        assert_difference('User.count', -1) do
+          delete '/users/2.xml', {}, :authorization => credentials('admin')
         end
+        
+        assert_response :ok
       end
-      
-      context ".json" do
-        should "not be allowed" do
-          assert_no_difference('User.count') do
-            delete '/users/2.json'
-          end
+    end
+    
+    context ".json" do
+      should_allow_api_authentication(:delete,
+        '/users/2.xml',
+        {},
+        {:success_code => :ok})
           
-          assert_response :method_not_allowed
+      should "delete user" do
+        assert_difference('User.count', -1) do
+          delete '/users/2.json', {}, :authorization => credentials('admin')
         end
+        
+        assert_response :ok
       end
     end
   end

diff --git a/test/integration/routing_test.rb b/test/integration/routing_test.rb
index 4b18e317afde7d9de910ec1ef5e04f3454d8ef38..1198081fcd3c4df90f58e8a45cb2ee717a0e3170 100644 (file)
--- a/test/integration/routing_test.rb
+++ b/test/integration/routing_test.rb
@@ -302,6 +302,9 @@ class RoutingTest < ActionController::IntegrationTest
 
     should_route :put, "/users/444", :controller => 'users', :action => 'update', :id => '444'
     should_route :put, "/users/444.xml", :controller => 'users', :action => 'update', :id => '444', :format => 'xml'
+
+    should_route :delete, "/users/44", :controller => 'users', :action => 'destroy', :id => '44'
+    should_route :delete, "/users/44.xml", :controller => 'users', :action => 'destroy', :id => '44', :format => 'xml'
   end
 
   # TODO: should they all be scoped under /projects/:project_id ?