diff options
| author | erdgeist <erdgeist@erdgeist.org> | 2026-08-01 01:10:35 +0200 |
|---|---|---|
| committer | erdgeist <erdgeist@erdgeist.org> | 2026-08-01 01:10:35 +0200 |
| commit | f6c1f0f08f031778a491465d35ac694bfcdc12b0 (patch) | |
| tree | 99aec0f419b1122821124fdb7ae8bdf4bc5831db /app/controllers | |
| parent | df90138fb55f7d3652d3d69d58325d7329920f51 (diff) | |
Require a fresh second factor for user management
Administrative actions are gated behind a 30-minute elevation window:
creating and retiring accounts, editing roles, clearing a second factor.
Reading the list is not gated, and content work is untouched.
elevated? is tied to is_admin?, so losing the role closes the window at
once. The window opens when the second factor verifies at login, so an
admin heading straight for user management is already elevated, and closes
on logout with the other session state. Five wrong codes end the session,
mirroring the login challenge.
users#update carries no elevation filter, since self-service reaches it;
the role field is gated in user_params instead and fails closed.
Diffstat (limited to 'app/controllers')
| -rw-r--r-- | app/controllers/concerns/role_required.rb | 9 | ||||
| -rw-r--r-- | app/controllers/elevations_controller.rb | 44 | ||||
| -rw-r--r-- | app/controllers/otp_challenges_controller.rb | 3 | ||||
| -rw-r--r-- | app/controllers/users_controller.rb | 3 |
4 files changed, 58 insertions, 1 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 | |||
| 20 | flash[:error] = t("flash.common.#{key}") | 20 | flash[:error] = t("flash.common.#{key}") |
| 21 | redirect_to admin_path | 21 | redirect_to admin_path |
| 22 | end | 22 | end |
| 23 | |||
| 24 | # require_admin must precede this in the filter chain, so a non-admin is | ||
| 25 | # denied by role before elevation is ever considered. | ||
| 26 | def require_elevation | ||
| 27 | return if elevated? | ||
| 28 | |||
| 29 | session[:elevation_return_to] = request.fullpath | ||
| 30 | redirect_to new_elevation_path | ||
| 31 | end | ||
| 23 | end | 32 | 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 @@ | |||
| 1 | # Step-up authentication for janitorial work. Mirrors the two-step login's | ||
| 2 | # attempt cap: verify_otp! guards replay but not rate, and an attacker with a | ||
| 3 | # stolen cookie would otherwise have unlimited tries at a six-digit code. | ||
| 4 | class ElevationsController < ApplicationController | ||
| 5 | include RoleRequired | ||
| 6 | |||
| 7 | layout 'admin' | ||
| 8 | |||
| 9 | before_action :login_required | ||
| 10 | before_action :require_admin | ||
| 11 | |||
| 12 | MAX_ATTEMPTS = 5 | ||
| 13 | |||
| 14 | def new | ||
| 15 | redirect_to admin_path if elevated? | ||
| 16 | end | ||
| 17 | |||
| 18 | def create | ||
| 19 | return render(:new) unless current_user.otp_enrolled? | ||
| 20 | |||
| 21 | session[:elevation_attempts] = session[:elevation_attempts].to_i + 1 | ||
| 22 | if session[:elevation_attempts] > MAX_ATTEMPTS | ||
| 23 | logout_killing_session! | ||
| 24 | flash[:error] = t("flash.otp.too_many_attempts") | ||
| 25 | return redirect_to(login_path) | ||
| 26 | end | ||
| 27 | |||
| 28 | if current_user.verify_otp!(params[:code]) | ||
| 29 | session.delete(:elevation_attempts) | ||
| 30 | elevate! | ||
| 31 | redirect_to safe_return_to(session.delete(:elevation_return_to), | ||
| 32 | :default => users_path) | ||
| 33 | else | ||
| 34 | flash.now[:error] = t("flash.otp.code_mismatch") | ||
| 35 | render :new | ||
| 36 | end | ||
| 37 | end | ||
| 38 | |||
| 39 | def destroy | ||
| 40 | drop_elevation! | ||
| 41 | flash[:notice] = t("flash.elevation.dropped") | ||
| 42 | redirect_to admin_path | ||
| 43 | end | ||
| 44 | 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 | |||
| 28 | reset_session | 28 | reset_session |
| 29 | self.current_user = user | 29 | self.current_user = user |
| 30 | session[:logged_in_at] = Time.now.to_i | 30 | session[:logged_in_at] = Time.now.to_i |
| 31 | # an admin who logs in and goes straight to user management | ||
| 32 | # is already elevated | ||
| 33 | elevate! if user.is_admin? | ||
| 31 | flash[:notice] = t("flash.common.logged_in") | 34 | flash[:notice] = t("flash.common.logged_in") |
| 32 | redirect_to safe_return_to(return_to, :default => admin_path) | 35 | redirect_to safe_return_to(return_to, :default => admin_path) |
| 33 | else | 36 | 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 | |||
| 7 | before_action :login_required | 7 | before_action :login_required |
| 8 | before_action :find_user, :only => [:show, :edit, :update, :reset_otp, :deactivate, :reactivate] | 8 | before_action :find_user, :only => [:show, :edit, :update, :reset_otp, :deactivate, :reactivate] |
| 9 | before_action :require_admin, :only => [:index, :new, :create, :reset_otp, :deactivate, :reactivate] | 9 | before_action :require_admin, :only => [:index, :new, :create, :reset_otp, :deactivate, :reactivate] |
| 10 | before_action :require_elevation, :only => [:new, :create, :reset_otp, :deactivate, :reactivate] | ||
| 10 | before_action :verify_status, :except => [:index] | 11 | before_action :verify_status, :except => [:index] |
| 11 | 12 | ||
| 12 | layout 'admin' | 13 | layout 'admin' |
| @@ -87,7 +88,7 @@ class UsersController < ApplicationController | |||
| 87 | :roles => []) | 88 | :roles => []) |
| 88 | # Checkbox arrays post a leading blank from the hidden field. | 89 | # Checkbox arrays post a leading blank from the hidden field. |
| 89 | permitted[:roles] = Array(permitted[:roles]).reject(&:blank?) if permitted.key?(:roles) | 90 | permitted[:roles] = Array(permitted[:roles]).reject(&:blank?) if permitted.key?(:roles) |
| 90 | permitted.delete(:roles) unless current_user.is_admin? | 91 | permitted.delete(:roles) unless current_user.is_admin? && elevated? |
| 91 | permitted | 92 | permitted |
| 92 | end | 93 | end |
| 93 | 94 | ||
