From be2cdea85177ac016e56dcce6cbe6015d754adf5 Mon Sep 17 00:00:00 2001
From: erdgeist <erdgeist@erdgeist.org>
Date: Mon, 3 Aug 2026 23:00:28 +0200
Subject: Require elevation before editing another account

---
 app/controllers/users_controller.rb       | 18 ++++++++++
 test/controllers/users_controller_test.rb | 55 +++++++++++++++++++++++++++++++
 2 files changed, 73 insertions(+)

diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb
index 7932e28b..1bd436e5 100644
--- a/app/controllers/users_controller.rb
+++ b/app/controllers/users_controller.rb
@@ -10,6 +10,7 @@ class UsersController < ApplicationController
   before_action :require_admin,     :only => [:new, :create, :reset_otp, :deactivate, :reactivate]
   before_action :require_elevation, :only => [:new, :create, :reset_otp, :deactivate, :reactivate]
   before_action :verify_status,   :except => [:index, :grant_redaktion, :revoke_redaktion]
+  before_action :require_elevation_for_other_accounts, :only => [:edit, :update]
 
   layout 'admin'
 
@@ -43,6 +44,11 @@ class UsersController < ApplicationController
   end
 
   def update
+    if roles_change_needs_elevation?
+      session[:elevation_return_to] = request.fullpath
+      return redirect_to new_elevation_path
+    end
+
     permitted = user_params
 
     if @user.update(permitted)
@@ -128,4 +134,16 @@ class UsersController < ApplicationController
     def deny_user_access
       deny_role_access(:admin_required)
     end
+
+    def require_elevation_for_other_accounts
+      return if @user == current_user
+      require_elevation
+    end
+
+    def roles_change_needs_elevation?
+      return false unless params[:user].respond_to?(:key?) && params[:user].key?(:roles)
+      submitted = Array(params[:user][:roles]).reject(&:blank?).sort
+      return false if submitted == @user.roles.sort
+      !(current_user.is_admin? && elevated?)
+    end
 end
diff --git a/test/controllers/users_controller_test.rb b/test/controllers/users_controller_test.rb
index a1812cb7..69c535f5 100644
--- a/test/controllers/users_controller_test.rb
+++ b/test/controllers/users_controller_test.rb
@@ -105,6 +105,7 @@ class UsersControllerTest < ActionController::TestCase
   
   test "get edit of another user being logged in as admin user" do
     login_as :aaron
+    elevate_session!
     get :edit, params: { :id => User.find_by_login("quentin").id }
     assert_response :success
   end
@@ -129,6 +130,7 @@ class UsersControllerTest < ActionController::TestCase
   test "updating an user when being login in as admin user" do
     user = User.find_by_login("quentin")
     login_as :aaron
+    elevate_session!
     put :update, params: { :id => user.id, :user => {:login => "random"} }
     assert_redirected_to user_path(user)
     assert_equal "random", user.reload.login
@@ -302,4 +304,57 @@ class UsersControllerTest < ActionController::TestCase
     assert_equal User.find_by(:login => "newcomer").id,
                  entry.action_participants.first.subject_id
   end
+
+  test "editing another account without elevation prompts for a code" do
+    login_as :aaron
+    get :edit, params: { :locale => "de", :id => users(:quentin).id }
+    assert_redirected_to new_elevation_path
+  end
+
+  test "editing your own account needs no elevation" do
+    login_as :aaron
+    get :edit, params: { :locale => "de", :id => users(:aaron).id }
+    assert_response :success
+  end
+
+  test "a role change submitted without elevation is refused, not silently dropped" do
+    login_as :aaron
+    target = users(:quentin)
+    assert_empty target.roles
+
+    put :update, params: { :locale => "de", :id => target.id,
+                           :user => { :roles => ["", "redaktion"] } }
+
+    assert_redirected_to new_elevation_path
+    assert_empty target.reload.roles
+    assert_nil flash[:notice]
+  end
+
+  test "a role change with elevation goes through" do
+    login_as :aaron
+    elevate_session!
+    target = users(:redella)
+    target.update_column(:otp_secret, ROTP::Base32.random)
+
+    put :update, params: { :locale => "de", :id => target.id,
+                           :user => { :roles => ["", "redaktion", "admin"] } }
+
+    assert_redirected_to user_path(target)
+    assert_equal ["admin", "redaktion"], target.reload.roles.sort
+  end
+
+  test "editing your own account without elevation does not prompt" do
+    login_as :aaron
+    put :update, params: { :locale => "de", :id => users(:aaron).id,
+                           :user => { :roles => ["", "admin", "redaktion"] } }
+    assert_redirected_to user_path(users(:aaron))
+  end
+
+  test "changing your own roles without elevation is refused" do
+    login_as :aaron
+    put :update, params: { :locale => "de", :id => users(:aaron).id,
+                           :user => { :roles => [""] } }
+    assert_redirected_to new_elevation_path
+    assert_equal ["admin", "redaktion"], users(:aaron).reload.roles.sort
+  end
 end
-- 
cgit v1.3