From abd7ee1fc2ecc15b50944db30c59bedc26ec41b6 Mon Sep 17 00:00:00 2001 From: erdgeist Date: Sat, 1 Aug 2026 04:14:00 +0200 Subject: Let Redaktion grant and revoke its own role Any holder may add or remove another account, witnessed as redaktion_grant/revoke so the vouching is legible. Not behind elevation: onboarding must not wait for a keyholder, and a compromised Redaktion account can already publish. --- app/controllers/users_controller.rb | 30 ++++++++++++++++++++++++++---- app/helpers/node_actions_helper.rb | 14 +++++++++++++- app/models/user.rb | 31 +++++++++++++++++++++++++++++++ app/views/users/_user.html.erb | 17 +++++++++++++++++ app/views/users/index.html.erb | 1 + 5 files changed, 88 insertions(+), 5 deletions(-) (limited to 'app') diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index 583ebac0..9b9d64e5 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -5,17 +5,17 @@ class UsersController < ApplicationController # Private 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 :find_user, :only => [:show, :edit, :update, :reset_otp, :deactivate, :reactivate, :grant_redaktion, :revoke_redaktion] + before_action :require_redaktion, :only => [:index] + 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] + before_action :verify_status, :except => [:index, :grant_redaktion, :revoke_redaktion] layout 'admin' ROLE_PRESETS = { "editor" => [], "redaktion" => ["redaktion"], - "admin" => ["admin", "redaktion"] }.freeze GROUP_ORDER = [:admin, :redaktion, :editor, :alumni].freeze @@ -74,6 +74,28 @@ class UsersController < ApplicationController redirect_to users_path end + def grant_redaktion + return deny_role_access(:redaktion_required) unless current_user.redaktion? + + case @user.grant_redaktion!(:actor => current_user) + when :granted then flash[:notice] = t("flash.users.redaktion_granted", :login => @user.login) + when :no_second_factor then flash[:error] = t("flash.users.redaktion_needs_otp", :login => @user.login) + end + + redirect_to users_path + end + + def revoke_redaktion + return deny_role_access(:redaktion_required) unless current_user.redaktion? + + case @user.revoke_redaktion!(:actor => current_user) + when :revoked then flash[:notice] = t("flash.users.redaktion_revoked", :login => @user.login) + when :self then flash[:error] = t("flash.users.redaktion_not_self") + end + + redirect_to users_path + end + def reset_otp @user.disable_otp!(:actor => current_user) flash[:notice] = t("flash.users.otp_reset", :login => @user.login) diff --git a/app/helpers/node_actions_helper.rb b/app/helpers/node_actions_helper.rb index f57ef84f..53f6ecd0 100644 --- a/app/helpers/node_actions_helper.rb +++ b/app/helpers/node_actions_helper.rb @@ -20,7 +20,9 @@ module NodeActionsHelper "otp_disable" => "shield-off", "otp_reset" => "shield-x", "user_deactivate" => "user-off", - "user_reactivate" => "user-check" + "user_reactivate" => "user-check", + "redaktion_grant" => "user-plus", + "redaktion_revoke" => "user-minus" }.freeze def verb_icon action @@ -283,4 +285,14 @@ module NodeActionsHelper t("node_actions.user_reactivate", :actor => actor_ref(action), :target => user_participant_ref(action)).html_safe end + + def summarize_redaktion_grant action + t("node_actions.redaktion_grant", :actor => actor_ref(action), + :target => user_participant_ref(action)).html_safe + end + + def summarize_redaktion_revoke action + t("node_actions.redaktion_revoke", :actor => actor_ref(action), + :target => user_participant_ref(action)).html_safe + end end diff --git a/app/models/user.rb b/app/models/user.rb index bf0f40ee..c3035a02 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -25,6 +25,7 @@ class User < ApplicationRecord :message => Authentication.bad_email_message validate :roles_are_known + validate :admin_needs_second_factor # Authenticates a user by their login name and unencrypted password. Returns the user or nil. def self.authenticate(login, password) @@ -136,6 +137,30 @@ class User < ApplicationRecord true end + def grant_redaktion!(actor:) + return :already if redaktion? + return :no_second_factor unless otp_enrolled? + + transaction do + update_column(:roles, (roles | ["redaktion"]).sort) + NodeAction.record!(:participants => [self], :user => actor, + :action => "redaktion_grant", :target_login => login) + end + :granted + end + + def revoke_redaktion!(actor:) + return :already unless redaktion? + return :self unless actor != self + + transaction do + update_column(:roles, (roles - ["redaktion"]).sort) + NodeAction.record!(:participants => [self], :user => actor, + :action => "redaktion_revoke", :target_login => login) + end + :revoked + 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 @@ -213,4 +238,10 @@ class User < ApplicationRecord unknown = roles.to_a - ROLES errors.add(:roles, :unknown, :list => unknown.join(", ")) if unknown.any? end + + def admin_needs_second_factor + return unless roles.include?("admin") + return if otp_secret.present? + errors.add(:roles, :admin_needs_otp) + end end diff --git a/app/views/users/_user.html.erb b/app/views/users/_user.html.erb index ff9d4e37..ba82375d 100644 --- a/app/views/users/_user.html.erb +++ b/app/views/users/_user.html.erb @@ -26,5 +26,22 @@ <% end %> <% end %> + + <% if current_user.redaktion? && !user.alumni? %> + <% if user.redaktion? %> + <% unless user == current_user %> + <%= button_to t(".revoke_redaktion"), revoke_redaktion_user_path(user), method: :put, + form: { data: { confirm: t(".confirm_revoke_redaktion", :login => user.login) }, + class: 'button_to destructive' } %> + <% end %> + <% elsif user.otp_enrolled? %> + <%= button_to t(".grant_redaktion"), grant_redaktion_user_path(user), method: :put, + form: { data: { confirm: t(".confirm_grant_redaktion", :login => user.login) }, + class: 'button_to state_changing' } %> + <% else %> + <%= t(".needs_otp") %> + <% end %> + <% end %> + <% end %> diff --git a/app/views/users/index.html.erb b/app/views/users/index.html.erb index 854811a2..2936bbea 100644 --- a/app/views/users/index.html.erb +++ b/app/views/users/index.html.erb @@ -8,6 +8,7 @@ <% end %> <% end %>

+

<%= t(".admin_hint") %>

<% UsersController::GROUP_ORDER.each do |group| %> <% members = @users[group] || [] %> -- cgit v1.3