From 28f0c4f131b02ab67bd9c254f9853168ec6a5b65 Mon Sep 17 00:00:00 2001 From: Jean-Philippe Lang Date: Sun, 15 Apr 2012 14:31:54 +0000 Subject: [PATCH] Adds the ability for users to delete their own account (#10664). Can be disabled in application settings. git-svn-id: svn+ssh://rubyforge.org/var/svn/redmine/trunk@9417 e93f8b46-1217-0410-a6f0-8f06a7374b81 --- app/controllers/account_controller.rb | 8 ----- app/controllers/application_controller.rb | 9 +++++ app/controllers/my_controller.rb | 18 ++++++++++ app/models/user.rb | 6 ++++ app/views/my/_sidebar.html.erb | 3 ++ app/views/my/destroy.html.erb | 11 ++++++ app/views/settings/_authentication.html.erb | 2 ++ config/locales/en.yml | 4 +++ config/locales/fr.yml | 4 +++ config/routes.rb | 2 ++ config/settings.yml | 2 ++ test/functional/my_controller_test.rb | 39 +++++++++++++++++++++ test/integration/routing/my_test.rb | 6 ++++ test/unit/user_test.rb | 27 ++++++++++++++ 14 files changed, 133 insertions(+), 8 deletions(-) create mode 100644 app/views/my/destroy.html.erb diff --git a/app/controllers/account_controller.rb b/app/controllers/account_controller.rb index 3874d2d89..926e04499 100644 --- a/app/controllers/account_controller.rb +++ b/app/controllers/account_controller.rb @@ -131,14 +131,6 @@ class AccountController < ApplicationController private - def logout_user - if User.current.logged? - cookies.delete :autologin - Token.delete_all(["user_id = ? AND action = ?", User.current.id, 'autologin']) - self.logged_user = nil - end - end - def authenticate_user if Setting.openid? && using_open_id? open_id_authenticate(params[:openid_url]) diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 5ac72cc70..0ecc04fcb 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -126,6 +126,15 @@ class ApplicationController < ActionController::Base end end + # Logs out current user + def logout_user + if User.current.logged? + cookies.delete :autologin + Token.delete_all(["user_id = ? AND action = ?", User.current.id, 'autologin']) + self.logged_user = nil + end + end + # check if login is globally required to access the application def check_if_login_required # no check needed if user is already logged in diff --git a/app/controllers/my_controller.rb b/app/controllers/my_controller.rb index cdf0182de..b3c975b78 100644 --- a/app/controllers/my_controller.rb +++ b/app/controllers/my_controller.rb @@ -65,6 +65,24 @@ class MyController < ApplicationController end end + # Destroys user's account + def destroy + @user = User.current + unless @user.own_account_deletable? + redirect_to :action => 'account' + return + end + + if request.post? && params[:confirm] + @user.destroy + if @user.destroyed? + logout_user + flash[:notice] = l(:notice_account_deleted) + end + redirect_to home_path + end + end + # Manage user's password def password @user = User.current diff --git a/app/models/user.rb b/app/models/user.rb index d1fa2822a..b377dda67 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -482,6 +482,12 @@ class User < Principal allowed_to?(action, nil, options.reverse_merge(:global => true), &block) end + # Returns true if the user is allowed to delete his own account + def own_account_deletable? + Setting.unsubscribe? && + (!admin? || User.active.first(:conditions => ["admin = ? AND id <> ?", true, id]).present?) + end + safe_attributes 'login', 'firstname', 'lastname', diff --git a/app/views/my/_sidebar.html.erb b/app/views/my/_sidebar.html.erb index 407fe990f..c89e6f3b4 100644 --- a/app/views/my/_sidebar.html.erb +++ b/app/views/my/_sidebar.html.erb @@ -3,6 +3,9 @@

<%=l(:field_login)%>: <%= link_to_user(@user, :format => :username) %>
<%=l(:field_created_on)%>: <%= format_time(@user.created_on) %>

+<% if @user.own_account_deletable? %> +

<%= link_to(l(:button_delete_my_account), {:action => 'destroy'}, :class => 'icon icon-del') %>

+<% end %>

<%= l(:label_feeds_access_key) %>

diff --git a/app/views/my/destroy.html.erb b/app/views/my/destroy.html.erb new file mode 100644 index 000000000..5d6eaa004 --- /dev/null +++ b/app/views/my/destroy.html.erb @@ -0,0 +1,11 @@ +

<%=l(:label_confirmation)%>

+
+

<%= simple_format l(:text_account_destroy_confirmation)%>

+

+ <% form_tag({}) do %> + + <%= submit_tag l(:button_delete_my_account) %> | + <%= link_to l(:button_cancel), :action => 'account' %> + <% end %> +

+
diff --git a/app/views/settings/_authentication.html.erb b/app/views/settings/_authentication.html.erb index bec373805..14396e274 100644 --- a/app/views/settings/_authentication.html.erb +++ b/app/views/settings/_authentication.html.erb @@ -10,6 +10,8 @@ [l(:label_registration_manual_activation), "2"], [l(:label_registration_automatic_activation), "3"]] %>

+

<%= setting_check_box :unsubscribe %>

+

<%= setting_text_field :password_min_length, :size => 6 %>

<%= setting_check_box :lost_password, :label => :label_password_lost %>

diff --git a/config/locales/en.yml b/config/locales/en.yml index 6180a860e..9536cf9a9 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -173,6 +173,7 @@ en: notice_gantt_chart_truncated: "The chart was truncated because it exceeds the maximum number of items that can be displayed (%{max})" notice_issue_successful_create: "Issue %{id} created." notice_issue_update_conflict: "The issue has been updated by an other user while you were editing it." + notice_account_deleted: "Your account has been permanently deleted." error_can_t_load_default_data: "Default configuration could not be loaded: %{value}" error_scm_not_found: "The entry or revision was not found in the repository." @@ -383,6 +384,7 @@ en: setting_issue_group_assignment: Allow issue assignment to groups setting_default_issue_start_date_to_creation_date: Use current date as start date for new issues setting_commit_cross_project_ref: Allow issues of all the other projects to be referenced and fixed + setting_unsubscribe: Allow users to unsubscribe permission_add_project: Create project permission_add_subprojects: Create subprojects @@ -894,6 +896,7 @@ en: button_show: Show button_edit_section: Edit this section button_export: Export + button_delete_my_account: Delete my account status_active: active status_registered: registered @@ -978,6 +981,7 @@ en: text_issue_conflict_resolution_overwrite: "Apply my changes anyway (previous notes will be kept but some changes may be overwritten)" text_issue_conflict_resolution_add_notes: "Add my notes and discard my other changes" text_issue_conflict_resolution_cancel: "Discard all my changes and redisplay %{link}" + text_account_destroy_confirmation: "Are you sure you want to proceed?\nYour account will be permanently deleted, with no way to reactivate it." default_role_manager: Manager default_role_developer: Developer diff --git a/config/locales/fr.yml b/config/locales/fr.yml index 10f3f9427..28fb54aee 100644 --- a/config/locales/fr.yml +++ b/config/locales/fr.yml @@ -188,6 +188,7 @@ fr: notice_gantt_chart_truncated: "Le diagramme a été tronqué car il excède le nombre maximal d'éléments pouvant être affichés (%{max})" notice_issue_successful_create: "La demande %{id} a été créée." notice_issue_update_conflict: "La demande a été mise à jour par un autre utilisateur pendant que vous la modifiez." + notice_account_deleted: "Votre compte a été définitivement supprimé." error_can_t_load_default_data: "Une erreur s'est produite lors du chargement du paramétrage : %{value}" error_scm_not_found: "L'entrée et/ou la révision demandée n'existe pas dans le dépôt." @@ -379,6 +380,7 @@ fr: setting_issue_group_assignment: Permettre l'assignement des demandes aux groupes setting_default_issue_start_date_to_creation_date: Donner à la date de début d'une nouvelle demande la valeur de la date du jour setting_commit_cross_project_ref: Permettre le référencement et la résolution des demandes de tous les autres projets + setting_unsubscribe: Permettre aux utilisateurs de se désinscrire permission_add_project: Créer un projet permission_add_subprojects: Créer des sous-projets @@ -868,6 +870,7 @@ fr: button_show: Afficher button_edit_section: Modifier cette section button_export: Exporter + button_delete_my_account: Supprimer mon compte status_active: actif status_registered: enregistré @@ -934,6 +937,7 @@ fr: text_issue_conflict_resolution_overwrite: "Appliquer quand même ma mise à jour (les notes précédentes seront conservées mais des changements pourront être écrasés)" text_issue_conflict_resolution_add_notes: "Ajouter mes notes et ignorer mes autres changements" text_issue_conflict_resolution_cancel: "Annuler ma mise à jour et réafficher %{link}" + text_account_destroy_confirmation: "Êtes-vous sûr de vouloir continuer ?\nVotre compte sera définitivement supprimé, sans aucune possibilité de le réactiver." default_role_manager: "Manager " default_role_developer: "Développeur " diff --git a/config/routes.rb b/config/routes.rb index 51bb66cca..de2a7c8ee 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -78,6 +78,8 @@ ActionController::Routing::Routes.draw do |map| map.connect 'my/account', :controller => 'my', :action => 'account', :conditions => {:method => [:get, :post]} + map.connect 'my/account/destroy', :controller => 'my', :action => 'destroy', + :conditions => {:method => [:get, :post]} map.connect 'my/page', :controller => 'my', :action => 'page', :conditions => {:method => :get} # Redirects to my/page diff --git a/config/settings.yml b/config/settings.yml index 28948fffe..66bc78e15 100644 --- a/config/settings.yml +++ b/config/settings.yml @@ -31,6 +31,8 @@ self_registration: default: '2' lost_password: default: 1 +unsubscribe: + default: 1 password_min_length: format: int default: 4 diff --git a/test/functional/my_controller_test.rb b/test/functional/my_controller_test.rb index a89af91a2..644ecb792 100644 --- a/test/functional/my_controller_test.rb +++ b/test/functional/my_controller_test.rb @@ -84,6 +84,45 @@ class MyControllerTest < ActionController::TestCase assert user.groups.empty? end + def test_my_account_should_show_destroy_link + get :account + assert_select 'a[href=/my/account/destroy]' + end + + def test_get_destroy_should_display_the_destroy_confirmation + get :destroy + assert_response :success + assert_template 'destroy' + assert_select 'form[action=/my/account/destroy]' do + assert_select 'input[name=confirm]' + end + end + + def test_post_destroy_without_confirmation_should_not_destroy_account + assert_no_difference 'User.count' do + post :destroy + end + assert_response :success + assert_template 'destroy' + end + + def test_post_destroy_without_confirmation_should_destroy_account + assert_difference 'User.count', -1 do + post :destroy, :confirm => '1' + end + assert_redirected_to '/' + assert_match /deleted/i, flash[:notice] + end + + def test_post_destroy_with_unsubscribe_not_allowed_should_not_destroy_account + User.any_instance.stubs(:own_account_deletable?).returns(false) + + assert_no_difference 'User.count' do + post :destroy, :confirm => '1' + end + assert_redirected_to '/my/account' + end + def test_change_password get :password assert_response :success diff --git a/test/integration/routing/my_test.rb b/test/integration/routing/my_test.rb index 5537a8b72..11c8a2cfb 100644 --- a/test/integration/routing/my_test.rb +++ b/test/integration/routing/my_test.rb @@ -25,6 +25,12 @@ class RoutingMyTest < ActionController::IntegrationTest { :controller => 'my', :action => 'account' } ) end + ["get", "post"].each do |method| + assert_routing( + { :method => method, :path => "/my/account/destroy" }, + { :controller => 'my', :action => 'destroy' } + ) + end assert_routing( { :method => 'get', :path => "/my/page" }, { :controller => 'my', :action => 'page' } diff --git a/test/unit/user_test.rb b/test/unit/user_test.rb index e698207da..607f23770 100644 --- a/test/unit/user_test.rb +++ b/test/unit/user_test.rb @@ -770,7 +770,34 @@ class UserTest < ActiveSupport::TestCase user.auth_source = denied_auth_source assert !user.change_password_allowed?, "User allowed to change password, though auth source does not" end + end + + def test_own_account_deletable_should_be_true_with_unsubscrive_enabled + with_settings :unsubscribe => '1' do + assert_equal true, User.find(2).own_account_deletable? + end + end + + def test_own_account_deletable_should_be_false_with_unsubscrive_disabled + with_settings :unsubscribe => '0' do + assert_equal false, User.find(2).own_account_deletable? + end + end + def test_own_account_deletable_should_be_false_for_a_single_admin + User.delete_all(["admin = ? AND id <> ?", true, 1]) + + with_settings :unsubscribe => '1' do + assert_equal false, User.find(1).own_account_deletable? + end + end + + def test_own_account_deletable_should_be_true_for_an_admin_if_other_admin_exists + User.generate_with_protected(:admin => true) + + with_settings :unsubscribe => '1' do + assert_equal true, User.find(1).own_account_deletable? + end end context "#allowed_to?" do -- 2.39.5