11 files changed, 155 insertions, 3 deletions

diff --git a/app/controllers/concerns/role_required.rb b/app/controllers/concerns/role_required.rb
index b841b8cc..22ce625e 100644
--- a/app/controllers/concerns/role_required.rb
+++ b/app/controllers/concerns/role_required.rb
@@ -20,4 +20,13 @@ module RoleRequired
       flash[:error] = t("flash.common.#{key}")
       redirect_to admin_path
     end
+
+    # require_admin must precede this in the filter chain, so a non-admin is
+    # denied by role before elevation is ever considered.
+    def require_elevation
+      return if elevated?
+
+      session[:elevation_return_to] = request.fullpath
+      redirect_to new_elevation_path
+    end
 end
diff --git a/app/controllers/elevations_controller.rb b/app/controllers/elevations_controller.rb
new file mode 100644
index 00000000..804b8f95
--- /dev/null
+++ b/app/controllers/elevations_controller.rb
@@ -0,0 +1,44 @@
+# Step-up authentication for janitorial work. Mirrors the two-step login's
+# attempt cap: verify_otp! guards replay but not rate, and an attacker with a
+# stolen cookie would otherwise have unlimited tries at a six-digit code.
+class ElevationsController < ApplicationController
+  include RoleRequired
+
+  layout 'admin'
+
+  before_action :login_required
+  before_action :require_admin
+
+  MAX_ATTEMPTS = 5
+
+  def new
+    redirect_to admin_path if elevated?
+  end
+
+  def create
+    return render(:new) unless current_user.otp_enrolled?
+
+    session[:elevation_attempts] = session[:elevation_attempts].to_i + 1
+    if session[:elevation_attempts] > MAX_ATTEMPTS
+      logout_killing_session!
+      flash[:error] = t("flash.otp.too_many_attempts")
+      return redirect_to(login_path)
+    end
+
+    if current_user.verify_otp!(params[:code])
+      session.delete(:elevation_attempts)
+      elevate!
+      redirect_to safe_return_to(session.delete(:elevation_return_to),
+                                 :default => users_path)
+    else
+      flash.now[:error] = t("flash.otp.code_mismatch")
+      render :new
+    end
+  end
+
+  def destroy
+    drop_elevation!
+    flash[:notice] = t("flash.elevation.dropped")
+    redirect_to admin_path
+  end
+end
diff --git a/app/controllers/otp_challenges_controller.rb b/app/controllers/otp_challenges_controller.rb
index eeaeac20..87586241 100644
--- a/app/controllers/otp_challenges_controller.rb
+++ b/app/controllers/otp_challenges_controller.rb
@@ -28,6 +28,9 @@ class OtpChallengesController < ApplicationController
       reset_session
       self.current_user = user
       session[:logged_in_at] = Time.now.to_i
+      # an admin who logs in and goes straight to user management
+      # is already elevated
+      elevate! if user.is_admin?
       flash[:notice] = t("flash.common.logged_in")
       redirect_to safe_return_to(return_to, :default => admin_path)
     else
diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb
index 052b2928..583ebac0 100644
--- a/app/controllers/users_controller.rb
+++ b/app/controllers/users_controller.rb
@@ -7,6 +7,7 @@ class UsersController < ApplicationController
   before_action :login_required
   before_action :find_user,     :only => [:show, :edit, :update, :reset_otp, :deactivate, :reactivate]
   before_action :require_admin, :only => [:index, :new, :create, :reset_otp, :deactivate, :reactivate]
+  before_action :require_elevation, :only => [:new, :create, :reset_otp, :deactivate, :reactivate]
   before_action :verify_status, :except => [:index]
 
   layout 'admin'
@@ -87,7 +88,7 @@ class UsersController < ApplicationController
                                 :roles => [])
       # Checkbox arrays post a leading blank from the hidden field.
       permitted[:roles] = Array(permitted[:roles]).reject(&:blank?) if permitted.key?(:roles)
-      permitted.delete(:roles) unless current_user.is_admin?
+      permitted.delete(:roles) unless current_user.is_admin? && elevated?
       permitted
     end
 
diff --git a/app/views/elevations/new.html.erb b/app/views/elevations/new.html.erb
new file mode 100644
index 00000000..1091ff36
--- /dev/null
+++ b/app/views/elevations/new.html.erb
@@ -0,0 +1,23 @@
+<h1><%= t(".title") %></h1>
+
+<div id="admin_layout">
+  <% if current_user.otp_enrolled? %>
+    <p class="field_hint">
+      <%= t(".hint", :minutes => AuthenticatedSystem::ELEVATION_MAX_AGE.in_minutes.to_i) %>
+    </p>
+    <%= form_tag elevation_path do %>
+      <div class="layout_row_label"><%= t(".code") %></div>
+      <div class="layout_row_content">
+        <%= text_field_tag :code, nil, :autocomplete => "one-time-code",
+              :inputmode => "numeric", :autofocus => true %>
+      </div>
+      <div class="layout_row_label"></div>
+      <div class="layout_row_content"><%= submit_tag t(".elevate") %></div>
+    <% end %>
+  <% else %>
+    <%# An admin without an enrolled factor cannot elevate at all. Say so and
+        point at enrolment rather than showing a field that cannot work. %>
+    <p><%= t(".not_enrolled") %></p>
+    <%= link_to t(".enrol"), otp_enrollment_path, :class => "action_button" %>
+  <% end %>
+</div>
diff --git a/config/locales/de.yml b/config/locales/de.yml
index 04eb738e..7edffe2d 100644
--- a/config/locales/de.yml
+++ b/config/locales/de.yml
@@ -628,6 +628,8 @@ de:
       none_to_remove: "Es existiert keine %{lang}-Übersetzung zum Entfernen."
       last_translation: "Die einzige verbliebene Übersetzung kann nicht entfernt werden."
       removed: "%{lang}-Übersetzung aus dem Entwurf entfernt. Veröffentlichen, um das dauerhaft zu machen."
+    elevation:
+      dropped: "Administrative Rechte abgelegt."
 
   assets:
     index:
@@ -721,6 +723,15 @@ de:
     title_fields:
       falls_back: "Menu-Titel. Leere Titel werden aus deutsch übernommen"
 
+  elevations:
+    new:
+      title: "Administrative Rechte anfordern"
+      hint: "Für Benutzerverwaltung ist ein aktueller zweiter Faktor nötig. Die Rechte gelten dann %{minutes} Minuten."
+      code: "Aktueller Code"
+      elevate: "Rechte anfordern"
+      not_enrolled: "Für administrative Arbeiten ist ein zweiter Faktor erforderlich. Bitte zuerst einen einrichten."
+      enrol: "Zweiten Faktor einrichten"
+
   layouts:
     application:
       light_mode_aria: "Zwischen dunklem und hellem Modus wechseln"
diff --git a/config/locales/en.yml b/config/locales/en.yml
index 2f7ac72f..9a8041bf 100644
--- a/config/locales/en.yml
+++ b/config/locales/en.yml
@@ -587,6 +587,8 @@ en:
       none_to_remove: "No %{lang} translation exists to remove."
       last_translation: "Can't remove the only remaining translation."
       removed: "%{lang} translation removed from the draft. Publish to make this permanent."
+    elevation:
+      dropped: "Administrative rights dropped."
 
   assets:
     index:
@@ -664,6 +666,15 @@ en:
       title: "Compare"
       default_marker: "default"
 
+  elevations:
+    new:
+      title: "Request administrative rights"
+      hint: "User management needs a current second factor. The rights then last %{minutes} minutes."
+      code: "Current code"
+      elevate: "Request rights"
+      not_enrolled: "Administrative work requires a second factor. Please set one up first."
+      enrol: "Set up a second factor"
+
   layouts:
     application:
       light_mode_aria: "Switch between dark and light mode"
diff --git a/config/routes.rb b/config/routes.rb
index 1898dbbb..4c37e70f 100644
--- a/config/routes.rb
+++ b/config/routes.rb
@@ -106,6 +106,7 @@ Cccms::Application.routes.draw do
     end
     resource :otp_enrollment, :only => [:show, :create, :update, :destroy]
     resource :otp_challenge, :only => [:new, :create]
+    resource :elevation, :only => [:new, :create, :destroy]
 
     resources :menu_items, :except => :show do
       member do
diff --git a/lib/authenticated_system.rb b/lib/authenticated_system.rb
index 4e70c28d..9a351dde 100644
--- a/lib/authenticated_system.rb
+++ b/lib/authenticated_system.rb
@@ -1,5 +1,6 @@
 module AuthenticatedSystem
   SESSION_MAX_AGE = 7.days
+  ELEVATION_MAX_AGE = 30.minutes
 
   protected
     # Returns true or false if the user is logged in.
@@ -20,6 +21,26 @@ module AuthenticatedSystem
       @current_user = new_user || false
     end
 
+    # Tied to is_admin? so losing the role closes the window at once, rather
+    # than leaving a timestamp that would count again if the role returned.
+    def elevated?
+      return false unless current_user&.is_admin?
+      session[:elevated_at].to_i > ELEVATION_MAX_AGE.ago.to_i
+    end
+
+    def elevation_expires_at
+      return nil unless elevated?
+      Time.at(session[:elevated_at].to_i) + ELEVATION_MAX_AGE
+    end
+
+    def elevate!
+      session[:elevated_at] = Time.now.to_i
+    end
+
+    def drop_elevation!
+      session.delete(:elevated_at)
+    end
+
     # Check if the user is authorized
     #
     # Override this method in your controllers if you want to restrict access
@@ -91,7 +112,8 @@ module AuthenticatedSystem
     # Inclusion hook to make #current_user and #logged_in?
     # available as ActionView helper methods.
     def self.included(base)
-      base.send :helper_method, :current_user, :logged_in?, :authorized? if base.respond_to? :helper_method
+      base.send :helper_method, :current_user, :logged_in?, :authorized?,
+                :elevated?, :elevation_expires_at if base.respond_to? :helper_method
     end
 
     #
@@ -123,7 +145,8 @@ module AuthenticatedSystem
     def logout_keeping_session!
       @current_user = false     # not logged in, and don't do it for me
       session[:user_id] = nil   # keeps the session but kill our variable
-      # explicitly kill any other session variables you set
+      session.delete(:elevated_at)
+      session.delete(:elevation_attempts)
     end
 
     # The session should only be reset at the tail end of a form POST --
diff --git a/lib/authenticated_test_helper.rb b/lib/authenticated_test_helper.rb
index 8f3a3732..065a5f7d 100644
--- a/lib/authenticated_test_helper.rb
+++ b/lib/authenticated_test_helper.rb
@@ -4,4 +4,8 @@ module AuthenticatedTestHelper
     @request.session[:user_id] = user ? users(user).id : nil
     @request.session[:logged_in_at] = Time.now.to_i
   end
+
+  def elevate_session!
+    session[:elevated_at] = Time.now.to_i
+  end
 end
diff --git a/test/controllers/users_controller_test.rb b/test/controllers/users_controller_test.rb
index fe099928..b6f0970d 100644
--- a/test/controllers/users_controller_test.rb
+++ b/test/controllers/users_controller_test.rb
@@ -19,6 +19,7 @@ class UsersControllerTest < ActionController::TestCase
  
   test "get new when logged in as admin" do
     login_as :aaron
+    elevate_session!
     get :new
     assert_response :success
   end
@@ -36,6 +37,7 @@ class UsersControllerTest < ActionController::TestCase
   
   test "creating new users being logged in as admin" do
     login_as :aaron
+    elevate_session!
     assert_difference "User.count", +1 do
       post :create, params: {
         :user => {
@@ -53,6 +55,7 @@ class UsersControllerTest < ActionController::TestCase
   
   test "creating new admin users being logged in as admin" do
     login_as :aaron
+    elevate_session!
     assert_difference "User.count", +1 do
       post :create, params: {
         :user => {
@@ -154,6 +157,7 @@ class UsersControllerTest < ActionController::TestCase
   
   test "an admin deactivates another user, who can no longer sign in" do
     login_as :aaron
+    elevate_session!
     user = users(:quentin)
 
     put :deactivate, params: { :id => user.id }
@@ -165,6 +169,7 @@ class UsersControllerTest < ActionController::TestCase
 
   test "reactivation restores the other roles untouched" do
     login_as :aaron
+    elevate_session!
     user = users(:quentin)
     user.update_column(:roles, ["alumni", "redaktion"])
 
@@ -189,6 +194,7 @@ class UsersControllerTest < ActionController::TestCase
   
   test "admin user can promote regular users to admins" do
     login_as :aaron
+    elevate_session!
     user = users(:quentin)
     put :update, params: { :id => user.id, :user => {:roles => ["admin", "redaktion"]} }
     
@@ -212,6 +218,7 @@ class UsersControllerTest < ActionController::TestCase
     assert user.reload.otp_enrolled?, "non-admin must be refused"
 
     login_as :aaron
+    elevate_session!
     put :reset_otp, params: { :id => user.id }
     assert_not user.reload.otp_enrolled?
     assert_equal "otp_reset", NodeAction.last.action
@@ -239,4 +246,19 @@ class UsersControllerTest < ActionController::TestCase
     get :show, params: { :id => users(:aaron).id }
     assert_redirected_to admin_path
   end
+
+  test "an admin without an open window is sent to elevation" do
+    login_as :aaron
+    get :new
+    assert_redirected_to new_elevation_path
+  end
+
+  test "an unelevated admin cannot change roles" do
+    login_as :aaron
+    user = users(:quentin)
+
+    put :update, params: { :id => user.id, :user => { :roles => ["admin"] } }
+
+    assert_not user.reload.is_admin?
+  end
 end