From 8bcacace28df52fd972c54e6850aa3b93f5c8bdf Mon Sep 17 00:00:00 2001 From: erdgeist Date: Sat, 1 Aug 2026 00:27:34 +0200 Subject: Declare role requirements per controller RoleRequired supplies require_redaktion and require_admin for surfaces that are not nodes and so cannot be reached by Node#restricted?. Navigation is content rather than plumbing, so menu_items requires redaktion. User management is janitorial and requires admin: index, new, create, reset_otp, deactivate, reactivate. verify_status now also covers show, without which any logged-in user could read any account by walking a small id space. Editing your own account stays open. The dashboard hides the Users and Navigation buttons from those who cannot use them; everything else stays visible to everyone. Both denials share one message and land on the dashboard. Adds redella (redaktion) and alufa (redaktion + alumni) fixtures. --- app/controllers/concerns/role_required.rb | 23 ++++++++++++++++ app/controllers/menu_items_controller.rb | 2 ++ app/controllers/users_controller.rb | 12 +++----- app/views/admin/index.html.erb | 12 +++++--- config/locales/de.yml | 2 ++ config/locales/en.yml | 2 ++ test/controllers/menu_items_controller_test.rb | 13 +++++++-- test/controllers/users_controller_test.rb | 38 +++++++++++++++++--------- test/fixtures/users.yml | 18 ++++++++++++ 9 files changed, 94 insertions(+), 28 deletions(-) create mode 100644 app/controllers/concerns/role_required.rb diff --git a/app/controllers/concerns/role_required.rb b/app/controllers/concerns/role_required.rb new file mode 100644 index 00000000..b841b8cc --- /dev/null +++ b/app/controllers/concerns/role_required.rb @@ -0,0 +1,23 @@ +# Controller-level role gates, for surfaces that are not nodes and so cannot +# be reached by Node#restricted?. The node gates live in the models, since +# those verbs are callable from rake tasks; these are HTTP-only. +module RoleRequired + extend ActiveSupport::Concern + + private + + def require_redaktion + return if current_user&.redaktion? + deny_role_access(:redaktion_required) + end + + def require_admin + return if current_user&.is_admin? + deny_role_access(:admin_required) + end + + def deny_role_access(key) + flash[:error] = t("flash.common.#{key}") + redirect_to admin_path + end +end diff --git a/app/controllers/menu_items_controller.rb b/app/controllers/menu_items_controller.rb index f169e0ca..63935f13 100644 --- a/app/controllers/menu_items_controller.rb +++ b/app/controllers/menu_items_controller.rb @@ -1,9 +1,11 @@ class MenuItemsController < ApplicationController include PinnedToDefaultLocale + include RoleRequired # Private before_action :login_required + before_action :require_redaktion layout 'admin' diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index 7bf23f17..052b2928 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -1,11 +1,13 @@ class UsersController < ApplicationController include PinnedToDefaultLocale + include RoleRequired # Private before_action :login_required before_action :find_user, :only => [:show, :edit, :update, :reset_otp, :deactivate, :reactivate] - before_action :verify_status, :except => [:index, :show] + before_action :require_admin, :only => [:index, :new, :create, :reset_otp, :deactivate, :reactivate] + before_action :verify_status, :except => [:index] layout 'admin' @@ -54,8 +56,6 @@ class UsersController < ApplicationController end def deactivate - return deny_user_access unless current_user.is_admin? - if @user == current_user flash[:error] = t("flash.users.cannot_deactivate_self") elsif @user.deactivate!(:actor => current_user) @@ -66,8 +66,6 @@ class UsersController < ApplicationController end def reactivate - return deny_user_access unless current_user.is_admin? - if @user.reactivate!(:actor => current_user) flash[:notice] = t("flash.users.reactivated", :login => @user.login) end @@ -76,7 +74,6 @@ class UsersController < ApplicationController end def reset_otp - return deny_user_access unless current_user.admin? @user.disable_otp!(:actor => current_user) flash[:notice] = t("flash.users.otp_reset", :login => @user.login) redirect_to edit_user_path(@user) @@ -106,7 +103,6 @@ class UsersController < ApplicationController end def deny_user_access - flash[:notice] = t("flash.common.admin_required") - redirect_to users_path + deny_role_access(:admin_required) end end diff --git a/app/views/admin/index.html.erb b/app/views/admin/index.html.erb index 984858e5..e3591c4e 100644 --- a/app/views/admin/index.html.erb +++ b/app/views/admin/index.html.erb @@ -71,11 +71,15 @@ <%= link_to assets_path, class: "action_button" do %> <%= icon("folder", library: "tabler", "aria-hidden": true) %> <%= t("assets.index.title") %> <% end %> - <%= link_to users_path, class: "action_button" do %> - <%= icon("users", library: "tabler", "aria-hidden": true) %> <%= t("users.index.users") %> + <% if current_user.is_admin? %> + <%= link_to users_path, class: "action_button" do %> + <%= icon("users", library: "tabler", "aria-hidden": true) %> <%= t("users.index.title") %> + <% end %> <% end %> - <%= link_to menu_items_path, class: "action_button" do %> - <%= icon("menu-2", library: "tabler", "aria-hidden": true) %> <%= t(".navigation") %> + <% if current_user.redaktion? %> + <%= link_to menu_items_path, class: "action_button" do %> + <%= icon("menu-2", library: "tabler", "aria-hidden": true) %> <%= t(".navigation") %> + <% end %> <% end %> <% trash_count = Node.trash.children.count %> <%= link_to trashed_nodes_path, class: "action_button" do %> diff --git a/config/locales/de.yml b/config/locales/de.yml index 74f6e8ae..04eb738e 100644 --- a/config/locales/de.yml +++ b/config/locales/de.yml @@ -574,6 +574,8 @@ de: headline_ineligible: "Dieser Asset-Typ kann kein Aufmacher sein." locked_by_other: "Die Seite ist gerade von jemand anderem gesperrt." autosave_failed: "Autosave fehlgeschlagen" + redaktion_required: "Diese Seite ist der Redaktion vorbehalten." + admin_required: "Diese Seite ist der Administration vorbehalten." sessions: otp_setup_now: "Dein Konto erfordert einen zweiten Faktor — richte ihn jetzt ein." logged_out: "Du wurdest abgemeldet." diff --git a/config/locales/en.yml b/config/locales/en.yml index c23f2c33..2f7ac72f 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -533,6 +533,8 @@ en: headline_ineligible: "This asset type cannot be a headline." locked_by_other: "The page is locked by another editor." autosave_failed: "Autosave failed" + redaktion_required: "This page is reserved for Redaktion." + admin_required: "This page is reserved for administrators." sessions: otp_setup_now: "Your account requires a second factor -- set it up now." logged_out: "You have been logged out." diff --git a/test/controllers/menu_items_controller_test.rb b/test/controllers/menu_items_controller_test.rb index 15a7b30b..d09198a3 100644 --- a/test/controllers/menu_items_controller_test.rb +++ b/test/controllers/menu_items_controller_test.rb @@ -9,7 +9,7 @@ class MenuItemsControllerTest < ActionController::TestCase end test "updating stores a title per locale" do - login_as :quentin + login_as :aaron item = create_menu_item patch :update, params: { :id => item.id, @@ -20,7 +20,7 @@ class MenuItemsControllerTest < ActionController::TestCase end test "blanking a non-default title falls back to the default locale" do - login_as :quentin + login_as :aaron item = create_menu_item patch :update, params: { :id => item.id, :menu_item => { :titles => { "de" => "Transparenz", "en" => "Transparency" } } } @@ -33,7 +33,7 @@ class MenuItemsControllerTest < ActionController::TestCase end test "a blank default title is rejected" do - login_as :quentin + login_as :aaron item = create_menu_item patch :update, params: { :id => item.id, :menu_item => { :titles => { "de" => "" } } } @@ -41,4 +41,11 @@ class MenuItemsControllerTest < ActionController::TestCase assert_response :success # re-rendered :edit, not a redirect assert_not_equal "", item.reload.translations.find_by(:locale => "de").title end + + test "an editor without redaktion cannot reach the menu" do + login_as :quentin + get :index + assert_redirected_to admin_path + assert_equal I18n.t("flash.common.redaktion_required"), flash[:error] + end end diff --git a/test/controllers/users_controller_test.rb b/test/controllers/users_controller_test.rb index 14133029..fe099928 100644 --- a/test/controllers/users_controller_test.rb +++ b/test/controllers/users_controller_test.rb @@ -2,11 +2,11 @@ require 'test_helper' class UsersControllerTest < ActionController::TestCase - test "get index as regular user renders stripped partial" do + test "an editor without admin cannot reach the user list" do login_as :quentin get :index - assert_response :success - assert_select "a", { :count => 0, :text => "Destroy" } + assert_redirected_to admin_path + assert_equal I18n.t("flash.common.admin_required"), flash[:error] end test "get index as admin shows every group with per-row actions" do @@ -27,10 +27,10 @@ class UsersControllerTest < ActionController::TestCase login_as :quentin get :new assert_response :redirect - assert_redirected_to users_path + assert_redirected_to admin_path assert_equal( I18n.t("flash.common.admin_required"), - flash[:notice] + flash[:error] ) end @@ -82,20 +82,20 @@ class UsersControllerTest < ActionController::TestCase } end - assert_redirected_to users_path + assert_redirected_to admin_path assert_equal( I18n.t("flash.common.admin_required"), - flash[:notice] + flash[:error] ) end test "get edit of another user being logged in as regular user wont work" do login_as :quentin get :edit, params: { :id => User.find_by_login("aaron").id } - assert_redirected_to users_path + assert_redirected_to admin_path assert_equal( I18n.t("flash.common.admin_required"), - flash[:notice] + flash[:error] ) end @@ -115,10 +115,10 @@ class UsersControllerTest < ActionController::TestCase user = User.find_by_login("aaron") login_as :quentin put :update, params: { :id => user.id, :user => {:login => "random"} } - assert_redirected_to users_path + assert_redirected_to admin_path assert_equal( I18n.t("flash.common.admin_required"), - flash[:notice] + flash[:error] ) end @@ -140,7 +140,7 @@ class UsersControllerTest < ActionController::TestCase test "showing a user" do login_as :quentin - get :show, params: { :id => User.find_by_login("aaron").id } + get :show, params: { :id => users(:quentin).id } assert_response :success end @@ -148,7 +148,7 @@ class UsersControllerTest < ActionController::TestCase login_as :quentin put :deactivate, params: { :id => users(:quentin).id } - assert_redirected_to users_path + assert_redirected_to admin_path assert_not users(:quentin).reload.alumni? end @@ -227,4 +227,16 @@ class UsersControllerTest < ActionController::TestCase assert_response :success assert_select "h2", :text => /#{I18n.t("users.index.group_alumni")}/ end + + test "an editor without admin cannot create accounts" do + login_as :quentin + get :new + assert_redirected_to admin_path + end + + test "an editor cannot read another account by id" do + login_as :quentin + get :show, params: { :id => users(:aaron).id } + assert_redirected_to admin_path + end end diff --git a/test/fixtures/users.yml b/test/fixtures/users.yml index f8d32d3c..2c433067 100644 --- a/test/fixtures/users.yml +++ b/test/fixtures/users.yml @@ -14,3 +14,21 @@ aaron: crypted_password: 740a48caf7dd5ff11318d812d57c0a0928cfbc12 # 'monkey' created_at: 2024-01-02 00:00:00 roles: ["admin", "redaktion"] + +redella: + id: 3 + login: redella + email: redella@example.com + salt: cf993996a70d31f924aff17a5f997722cb6ec2dd + crypted_password: 11c672158b0eb6e8c91c438b3eb844902308b138 # 'monkey' + created_at: 2024-01-03 00:00:00 + roles: ["redaktion"] + +alufa: + id: 4 + login: alufa + email: alufa@example.com + salt: cf993996a70d31f924aff17a5f997722cb6ec2dd + crypted_password: 11c672158b0eb6e8c91c438b3eb844902308b138 # 'monkey' + created_at: 2024-01-04 00:00:00 + roles: ["redaktion", "alumni"] -- cgit v1.3