4 files changed, 86 insertions, 3 deletions

diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb
index 1bd436e5..b06d11fd 100644
--- a/app/controllers/users_controller.rb
+++ b/app/controllers/users_controller.rb
@@ -50,12 +50,26 @@ class UsersController < ApplicationController
     end
 
     permitted = user_params
+    desired   = permitted.key?(:roles) ? permitted.delete(:roles) : nil
+    refusals  = []
+    saved     = false
 
-    if @user.update(permitted)
+    User.transaction do
+      saved = @user.update(permitted)
+      raise ActiveRecord::Rollback unless saved
+
+      refusals = desired ? @user.update_roles!(desired, :actor => current_user) : []
+      raise ActiveRecord::Rollback if refusals.any?
+    end
+
+    if !saved
+      render :edit
+    elsif refusals.any?
+      flash.now[:error] = refusals.map { |r| t("flash.users.#{r}", :login => @user.login) }.to_sentence
+      render :edit
+    else
       flash[:notice] = t("flash.users.updated", :login => @user.login)
       redirect_to user_path(@user)
-    else
-      render :edit
     end
   end
 
diff --git a/app/helpers/node_actions_helper.rb b/app/helpers/node_actions_helper.rb
index 7dd55bdb..4cf990b8 100644
--- a/app/helpers/node_actions_helper.rb
+++ b/app/helpers/node_actions_helper.rb
@@ -24,6 +24,8 @@ module NodeActionsHelper
     "user_reactivate"    => "user-check",
     "redaktion_grant"    => "users-plus",
     "redaktion_revoke"   => "users-minus",
+    "admin_grant"        => "shield-plus",
+    "admin_revoke"       => "shield-minus",
     "event_create"       => "calendar-plus",
     "event_update"       => "calendar-event",
     "event_destroy"      => "calendar-x"
@@ -371,6 +373,16 @@ module NodeActionsHelper
        :target => user_participant_ref(action)).html_safe
   end
 
+  def summarize_admin_grant action
+    t("node_actions.admin_grant", :actor => actor_ref(action),
+       :target => user_participant_ref(action)).html_safe
+  end
+
+  def summarize_admin_revoke action
+    t("node_actions.admin_revoke", :actor => actor_ref(action),
+       :target => user_participant_ref(action)).html_safe
+  end
+
   def summarize_user_create action
     t("node_actions.user_create", :actor => actor_ref(action),
        :target => user_participant_ref(action)).html_safe
diff --git a/app/models/node_action.rb b/app/models/node_action.rb
index 0167762b..bfa469b1 100644
--- a/app/models/node_action.rb
+++ b/app/models/node_action.rb
@@ -105,6 +105,12 @@ class NodeAction < ApplicationRecord
   # otp_disable is self-service; otp_reset and all three account
   # verbs are an administrator acting on someone else, so actor and
   # participant differ:
+  #   "redaktion_grant" / "redaktion_revoke" / "admin_grant" /
+  #   "admin_revoke" -- role changes. Both pairs come from
+  #   User#grant_* / #revoke_*, so the roles form reaches them through
+  #   update_roles! rather than writing the attribute: witnessing is the
+  #   reason the form does not touch roles directly. Alumni changes record
+  #   as user_deactivate / user_reactivate, not as a role verb.
   #   "target_login" -- flat string, the affected account's login
   #
   # "event_create" / "event_update" / "event_destroy" (calendar
diff --git a/app/models/user.rb b/app/models/user.rb
index adfdc564..786f8d14 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -132,6 +132,7 @@ class User < ApplicationRecord
 
   def deactivate!(actor:)
     return false if alumni?
+    return false if actor == self
     transaction do
       update_column(:roles, (roles | ["alumni"]).sort)
       NodeAction.record!(:participants => [self], :user => actor,
@@ -174,6 +175,56 @@ class User < ApplicationRecord
     :revoked
   end
 
+  def grant_admin!(actor:)
+    return :already if is_admin?
+    return :no_second_factor unless otp_enrolled?
+
+    transaction do
+      update_column(:roles, (roles | ["admin"]).sort)
+      NodeAction.record!(:participants => [self], :user => actor,
+                          :action => "admin_grant", :target_login => login)
+    end
+    :granted
+  end
+
+  def revoke_admin!(actor:)
+    return :already unless is_admin?
+    return :self unless actor != self
+
+    transaction do
+      update_column(:roles, (roles - ["admin"]).sort)
+      NodeAction.record!(:participants => [self], :user => actor,
+                          :action => "admin_revoke", :target_login => login)
+    end
+    :revoked
+  end
+
+  def update_roles!(desired, actor:)
+    desired = Array(desired).map(&:to_s) & ROLES
+    refusals = []
+
+    transaction do
+      refusals << :admin_not_self         if is_admin?  && !desired.include?("admin")     && actor == self
+      refusals << :redaktion_not_self     if redaktion? && !desired.include?("redaktion") && actor == self
+      refusals << :cannot_deactivate_self if !alumni?   &&  desired.include?("alumni")    && actor == self
+      refusals << :admin_needs_otp        if !is_admin?  && desired.include?("admin")     && !otp_enrolled?
+      refusals << :redaktion_needs_otp    if !redaktion? && desired.include?("redaktion") && !otp_enrolled?
+
+      raise ActiveRecord::Rollback if refusals.any?
+
+      revoke_admin!(:actor => actor)     if is_admin?   && !desired.include?("admin")
+      revoke_redaktion!(:actor => actor) if redaktion?  && !desired.include?("redaktion")
+      reactivate!(:actor => actor)       if alumni?     && !desired.include?("alumni")
+
+      grant_admin!(:actor => actor)     if !is_admin?  && desired.include?("admin")
+      grant_redaktion!(:actor => actor) if !redaktion? && desired.include?("redaktion")
+
+      deactivate!(:actor => actor) if !alumni? && desired.include?("alumni")
+    end
+
+    refusals
+  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