Replace user deletion with deactivation

Deactivation adds the alumni role and leaves the others in place, so
reactivation is lossless and nobody has to remember what an account held.
login_from_session checks alumni? on every request, so a signed-in user is
locked out on their next one without any session invalidation. Guards
prevent deactivating yourself or the last active admin, and both verbs are
witnessed in the action log.

9 files changed, 118 insertions, 26 deletions

diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb
index 95dff220..7bf23f17 100644
--- a/app/controllers/users_controller.rb
+++ b/app/controllers/users_controller.rb
@@ -4,7 +4,7 @@ class UsersController < ApplicationController
   # Private
 
   before_action :login_required
-  before_action :find_user,     :only => [:show, :edit, :update, :destroy, :reset_otp]
+  before_action :find_user,     :only => [:show, :edit, :update, :reset_otp, :deactivate, :reactivate]
   before_action :verify_status, :except => [:index, :show]
 
   layout 'admin'
@@ -53,8 +53,25 @@ class UsersController < ApplicationController
   def show
   end
 
-  def destroy
-    @user.destroy if @user
+  def deactivate
+    return deny_user_access unless current_user.is_admin?
+
+    if @user == current_user
+      flash[:error] = t("flash.users.cannot_deactivate_self")
+    elsif @user.deactivate!(:actor => current_user)
+      flash[:notice] = t("flash.users.deactivated", :login => @user.login)
+    end
+
+    redirect_to users_path
+  end
+
+  def reactivate
+    return deny_user_access unless current_user.is_admin?
+
+    if @user.reactivate!(:actor => current_user)
+      flash[:notice] = t("flash.users.reactivated", :login => @user.login)
+    end
+
     redirect_to users_path
   end
 
diff --git a/app/helpers/node_actions_helper.rb b/app/helpers/node_actions_helper.rb
index 4cbe741c..f57ef84f 100644
--- a/app/helpers/node_actions_helper.rb
+++ b/app/helpers/node_actions_helper.rb
@@ -18,7 +18,9 @@ module NodeActionsHelper
     "asset_destroy"      => "file-x",
     "otp_enroll"         => "shield-lock",
     "otp_disable"        => "shield-off",
-    "otp_reset"          => "shield-x"
+    "otp_reset"          => "shield-x",
+    "user_deactivate"    => "user-off",
+    "user_reactivate"    => "user-check"
   }.freeze
 
   def verb_icon action
@@ -271,4 +273,14 @@ module NodeActionsHelper
     t("node_actions.otp_reset", :actor => actor_ref(action),
        :target => user_participant_ref(action)).html_safe
   end
+
+  def summarize_user_deactivate action
+    t("node_actions.user_deactivate", :actor => actor_ref(action),
+       :target => user_participant_ref(action)).html_safe
+  end
+
+  def summarize_user_reactivate action
+    t("node_actions.user_reactivate", :actor => actor_ref(action),
+       :target => user_participant_ref(action)).html_safe
+  end
 end
diff --git a/app/models/node_action.rb b/app/models/node_action.rb
index fec5a062..d619aac5 100644
--- a/app/models/node_action.rb
+++ b/app/models/node_action.rb
@@ -98,8 +98,9 @@ class NodeAction < ApplicationRecord
   #   "detached_from"         -- array of unique_names, only when any
   #   "headline_removed_from" -- array of unique_names, only when any
   #
-  # "otp_enroll" / "otp_disable" / "otp_reset" (second-factor lifecycle;
-  # node column nil; participants: the affected User -- the table's first
+  # "otp_enroll" / "otp_disable" / "otp_reset" / "user_deactivate" /
+  # "user_reactivate" (second-factor lifecycle; node column nil;
+  # participants: the affected User
   # User-typed subject. otp_disable is self-service; otp_reset is an
   # administrator clearing someone else's factor, where actor and
   # participant differ):
diff --git a/app/models/user.rb b/app/models/user.rb
index 2e9da86c..1728521a 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -105,6 +105,26 @@ class User < ApplicationRecord
     roles.map { |r| I18n.t("users.roles.#{r}", :default => r) }
   end
 
+  def deactivate!(actor:)
+    return false if alumni?
+    transaction do
+      update_column(:roles, (roles | ["alumni"]).sort)
+      NodeAction.record!(:participants => [self], :user => actor,
+                          :action => "user_deactivate", :target_login => login)
+    end
+    true
+  end
+
+  def reactivate!(actor:)
+    return false unless alumni?
+    transaction do
+      update_column(:roles, (roles - ["alumni"]).sort)
+      NodeAction.record!(:participants => [self], :user => actor,
+                          :action => "user_reactivate", :target_login => login)
+    end
+    true
+  end
+
   # otp_secret present == enrolled. otp_pending_secret holds the secret
   # between QR display and first-code confirmation. otp_consumed_timestep
   # makes every accepted code single-use (replay guard within the drift
diff --git a/app/views/users/_user.html.erb b/app/views/users/_user.html.erb
index 04884be8..ff9d4e37 100644
--- a/app/views/users/_user.html.erb
+++ b/app/views/users/_user.html.erb
@@ -9,14 +9,22 @@
     <% end %>
   </td>
   <td><%= link_to t("admin.common.show"), user_path(user) %></td>
-  <% if current_user.admin? || current_user == user %>
-  <td><%= link_to t("admin.common.edit"), edit_user_path(user) %></td>
   <td>
-  <%= button_to user_path(user), method: :delete,
-        form: { data: { confirm: t(".confirm_destroy", :login => user.login) }, class: 'button_to destructive' } do %>
-    <%= icon("trash", library: "tabler", "aria-hidden": true) %> <%= t("admin.common.destroy") %>
-  <% end %>
+    <% if current_user.admin? || current_user == user %>
+      <%= link_to t("admin.common.edit"), edit_user_path(user) %>
+    <% end %>
+  </td>
+  <td>
+    <% if current_user.admin? && current_user != user %>
+      <% if user.alumni? %>
+        <%= button_to t(".reactivate"), reactivate_user_path(user), method: :put,
+              form: { class: 'button_to state_changing' } %>
+      <% else %>
+        <%= button_to t(".deactivate"), deactivate_user_path(user), method: :put,
+              form: { data: { confirm: t(".confirm_deactivate", :login => user.login) },
+                      class: 'button_to destructive' } %>
+      <% end %>
+    <% end %>
   </td>
-  <% end %>
 </tr>
 <% end %>
diff --git a/config/locales/de.yml b/config/locales/de.yml
index e15a08d2..8aae7ca5 100644
--- a/config/locales/de.yml
+++ b/config/locales/de.yml
@@ -219,6 +219,8 @@ de:
     otp_enroll: "%{actor} hat einen zweiten Faktor eingerichtet"
     otp_disable: "%{actor} hat den zweiten Faktor entfernt"
     otp_reset: "%{actor} hat den zweiten Faktor von %{target} zurückgesetzt"
+    user_deactivate: "%{actor} hat %{target} deaktiviert"
+    user_reactivate: "%{actor} hat %{target} reaktiviert"
 
   open_gallery: "Gallerie anzeigen"
   asset_licenses:
@@ -281,6 +283,9 @@ de:
     user:
       confirm_destroy: "Benutzer %{login} wirklich löschen?"
       no_roles: "—"
+      deactivate: "Deaktivieren"
+      reactivate: "Reaktivieren"
+      confirm_deactivate: "%{login} deaktivieren? Die Anmeldung wird sofort verweigert, Zuschreibungen bleiben erhalten."
     index:
       title: "Benutzerkonten"
       create_editor: "Editor-Konto anlegen"
@@ -578,6 +583,9 @@ de:
       created: "Benutzer %{login} angelegt"
       updated: "Benutzer %{login} aktualisiert"
       otp_reset: "Zweiter Faktor von %{login} zurückgesetzt"
+      deactivated: "%{login} ist jetzt alumni und kann sich nicht mehr anmelden."
+      reactivated: "%{login} kann sich wieder anmelden."
+      cannot_deactivate_self: "Das eigene Konto kann nicht deaktiviert werden."
     assets:
       created: "Asset wurde angelegt."
       updated: "Asset wurde aktualisiert."
diff --git a/config/locales/en.yml b/config/locales/en.yml
index f8b94a00..e434538f 100644
--- a/config/locales/en.yml
+++ b/config/locales/en.yml
@@ -167,6 +167,8 @@ en:
     otp_enroll: "%{actor} set up a second factor"
     otp_disable: "%{actor} removed their second factor"
     otp_reset: "%{actor} reset the second factor of %{target}"
+    user_deactivate: "%{actor} deactivated %{target}"
+    user_reactivate: "%{actor} reactivated %{target}"
 
   open_gallery: "Open gallery"
   asset_licenses:
@@ -229,6 +231,9 @@ en:
     user:
       confirm_destroy: "Do you really want to destroy user %{login}?"
       no_roles: "—"
+      deactivate: "Deactivate"
+      reactivate: "Reactivate"
+      confirm_deactivate: "Deactivate %{login}? Sign-in is refused immediately; attributions are preserved."
     index:
       title: "User accounts"
       create_editor: "Create editor account"
@@ -541,6 +546,9 @@ en:
       created: "User created %{login}"
       updated: "Updated user %{login}"
       otp_reset: "Second factor reset for %{login}"
+      deactivated: "%{login} is now an alumnus and can no longer sign in."
+      reactivated: "%{login} can sign in again."
+      cannot_deactivate_self: "You cannot deactivate your own account."
     assets:
       created: "Asset was successfully created."
       updated: "Asset was successfully updated."
diff --git a/config/routes.rb b/config/routes.rb
index 4b5d15c9..1898dbbb 100644
--- a/config/routes.rb
+++ b/config/routes.rb
@@ -97,9 +97,11 @@ Cccms::Application.routes.draw do
     match '/login'       => 'sessions#new',     :as => :login,        :via => :get
     match 'search'       => 'search#index',     :as => :search,       :via => :get
 
-    resources :users do
+    resources :users, :except => :destroy do
       member do
         put :reset_otp
+        put :deactivate
+        put :reactivate
       end
     end
     resource :otp_enrollment, :only => [:show, :create, :update, :destroy]
diff --git a/test/controllers/users_controller_test.rb b/test/controllers/users_controller_test.rb
index 1c5d16fc..14133029 100644
--- a/test/controllers/users_controller_test.rb
+++ b/test/controllers/users_controller_test.rb
@@ -13,7 +13,7 @@ class UsersControllerTest < ActionController::TestCase
     login_as :aaron
     get :index
     assert_response :success
-    assert_select "button[type=submit]", I18n.t("admin.common.destroy")
+    assert_select "button[type=submit]", I18n.t("users.user.deactivate")
     assert_select "a", I18n.t("admin.common.show")
   end
  
@@ -146,22 +146,38 @@ class UsersControllerTest < ActionController::TestCase
   
   test "destroying an user being logged in as regular user wont work" do
     login_as :quentin
-    assert_no_difference "User.count" do
-      delete :destroy, params: { :id => User.find_by_login("aaron").id }
-    end
+    put :deactivate, params: { :id => users(:quentin).id }
+
     assert_redirected_to users_path
-    assert_equal(
-      I18n.t("flash.common.admin_required"),
-      flash[:notice]
-    )
+    assert_not users(:quentin).reload.alumni?
   end
   
-  test "destroying an user being logged in as admin user" do
+  test "an admin deactivates another user, who can no longer sign in" do
     login_as :aaron
-    assert_difference "User.count", -1 do
-      delete :destroy, params: { :id => User.find_by_login("quentin").id }
-    end
+    user = users(:quentin)
+
+    put :deactivate, params: { :id => user.id }
+
     assert_redirected_to users_path
+    assert user.reload.alumni?
+    assert_equal ["admin", "alumni"].sort, user.roles.sort if user.is_admin?
+  end
+
+  test "reactivation restores the other roles untouched" do
+    login_as :aaron
+    user = users(:quentin)
+    user.update_column(:roles, ["alumni", "redaktion"])
+
+    put :reactivate, params: { :id => user.id }
+
+    assert_not user.reload.alumni?
+    assert_equal ["redaktion"], user.roles
+  end
+
+  test "an admin cannot deactivate their own account" do
+    login_as :aaron
+    put :deactivate, params: { :id => users(:aaron).id }
+    assert_not users(:aaron).reload.alumni?
   end
 
   test "enrolled user gets a working my account" do