diff options
| author | erdgeist <erdgeist@erdgeist.org> | 2026-08-01 00:27:34 +0200 |
|---|---|---|
| committer | erdgeist <erdgeist@erdgeist.org> | 2026-08-01 00:27:34 +0200 |
| commit | 8bcacace28df52fd972c54e6850aa3b93f5c8bdf (patch) | |
| tree | 05e90dd4e8f31ebb142f66239025da15e873901f | |
| parent | 529f81b28ed77c62acaa63fad957e751798f2440 (diff) | |
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.
| -rw-r--r-- | app/controllers/concerns/role_required.rb | 23 | ||||
| -rw-r--r-- | app/controllers/menu_items_controller.rb | 2 | ||||
| -rw-r--r-- | app/controllers/users_controller.rb | 12 | ||||
| -rw-r--r-- | app/views/admin/index.html.erb | 12 | ||||
| -rw-r--r-- | config/locales/de.yml | 2 | ||||
| -rw-r--r-- | config/locales/en.yml | 2 | ||||
| -rw-r--r-- | test/controllers/menu_items_controller_test.rb | 13 | ||||
| -rw-r--r-- | test/controllers/users_controller_test.rb | 38 | ||||
| -rw-r--r-- | test/fixtures/users.yml | 18 |
9 files changed, 94 insertions, 28 deletions
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 @@ | |||
| 1 | # Controller-level role gates, for surfaces that are not nodes and so cannot | ||
| 2 | # be reached by Node#restricted?. The node gates live in the models, since | ||
| 3 | # those verbs are callable from rake tasks; these are HTTP-only. | ||
| 4 | module RoleRequired | ||
| 5 | extend ActiveSupport::Concern | ||
| 6 | |||
| 7 | private | ||
| 8 | |||
| 9 | def require_redaktion | ||
| 10 | return if current_user&.redaktion? | ||
| 11 | deny_role_access(:redaktion_required) | ||
| 12 | end | ||
| 13 | |||
| 14 | def require_admin | ||
| 15 | return if current_user&.is_admin? | ||
| 16 | deny_role_access(:admin_required) | ||
| 17 | end | ||
| 18 | |||
| 19 | def deny_role_access(key) | ||
| 20 | flash[:error] = t("flash.common.#{key}") | ||
| 21 | redirect_to admin_path | ||
| 22 | end | ||
| 23 | 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 @@ | |||
| 1 | class MenuItemsController < ApplicationController | 1 | class MenuItemsController < ApplicationController |
| 2 | include PinnedToDefaultLocale | 2 | include PinnedToDefaultLocale |
| 3 | include RoleRequired | ||
| 3 | 4 | ||
| 4 | # Private | 5 | # Private |
| 5 | 6 | ||
| 6 | before_action :login_required | 7 | before_action :login_required |
| 8 | before_action :require_redaktion | ||
| 7 | 9 | ||
| 8 | layout 'admin' | 10 | layout 'admin' |
| 9 | 11 | ||
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 @@ | |||
| 1 | class UsersController < ApplicationController | 1 | class UsersController < ApplicationController |
| 2 | include PinnedToDefaultLocale | 2 | include PinnedToDefaultLocale |
| 3 | include RoleRequired | ||
| 3 | 4 | ||
| 4 | # Private | 5 | # Private |
| 5 | 6 | ||
| 6 | before_action :login_required | 7 | before_action :login_required |
| 7 | 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] |
| 8 | before_action :verify_status, :except => [:index, :show] | 9 | before_action :require_admin, :only => [:index, :new, :create, :reset_otp, :deactivate, :reactivate] |
| 10 | before_action :verify_status, :except => [:index] | ||
| 9 | 11 | ||
| 10 | layout 'admin' | 12 | layout 'admin' |
| 11 | 13 | ||
| @@ -54,8 +56,6 @@ class UsersController < ApplicationController | |||
| 54 | end | 56 | end |
| 55 | 57 | ||
| 56 | def deactivate | 58 | def deactivate |
| 57 | return deny_user_access unless current_user.is_admin? | ||
| 58 | |||
| 59 | if @user == current_user | 59 | if @user == current_user |
| 60 | flash[:error] = t("flash.users.cannot_deactivate_self") | 60 | flash[:error] = t("flash.users.cannot_deactivate_self") |
| 61 | elsif @user.deactivate!(:actor => current_user) | 61 | elsif @user.deactivate!(:actor => current_user) |
| @@ -66,8 +66,6 @@ class UsersController < ApplicationController | |||
| 66 | end | 66 | end |
| 67 | 67 | ||
| 68 | def reactivate | 68 | def reactivate |
| 69 | return deny_user_access unless current_user.is_admin? | ||
| 70 | |||
| 71 | if @user.reactivate!(:actor => current_user) | 69 | if @user.reactivate!(:actor => current_user) |
| 72 | flash[:notice] = t("flash.users.reactivated", :login => @user.login) | 70 | flash[:notice] = t("flash.users.reactivated", :login => @user.login) |
| 73 | end | 71 | end |
| @@ -76,7 +74,6 @@ class UsersController < ApplicationController | |||
| 76 | end | 74 | end |
| 77 | 75 | ||
| 78 | def reset_otp | 76 | def reset_otp |
| 79 | return deny_user_access unless current_user.admin? | ||
| 80 | @user.disable_otp!(:actor => current_user) | 77 | @user.disable_otp!(:actor => current_user) |
| 81 | flash[:notice] = t("flash.users.otp_reset", :login => @user.login) | 78 | flash[:notice] = t("flash.users.otp_reset", :login => @user.login) |
| 82 | redirect_to edit_user_path(@user) | 79 | redirect_to edit_user_path(@user) |
| @@ -106,7 +103,6 @@ class UsersController < ApplicationController | |||
| 106 | end | 103 | end |
| 107 | 104 | ||
| 108 | def deny_user_access | 105 | def deny_user_access |
| 109 | flash[:notice] = t("flash.common.admin_required") | 106 | deny_role_access(:admin_required) |
| 110 | redirect_to users_path | ||
| 111 | end | 107 | end |
| 112 | end | 108 | 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 @@ | |||
| 71 | <%= link_to assets_path, class: "action_button" do %> | 71 | <%= link_to assets_path, class: "action_button" do %> |
| 72 | <%= icon("folder", library: "tabler", "aria-hidden": true) %> <%= t("assets.index.title") %> | 72 | <%= icon("folder", library: "tabler", "aria-hidden": true) %> <%= t("assets.index.title") %> |
| 73 | <% end %> | 73 | <% end %> |
| 74 | <%= link_to users_path, class: "action_button" do %> | 74 | <% if current_user.is_admin? %> |
| 75 | <%= icon("users", library: "tabler", "aria-hidden": true) %> <%= t("users.index.users") %> | 75 | <%= link_to users_path, class: "action_button" do %> |
| 76 | <%= icon("users", library: "tabler", "aria-hidden": true) %> <%= t("users.index.title") %> | ||
| 77 | <% end %> | ||
| 76 | <% end %> | 78 | <% end %> |
| 77 | <%= link_to menu_items_path, class: "action_button" do %> | 79 | <% if current_user.redaktion? %> |
| 78 | <%= icon("menu-2", library: "tabler", "aria-hidden": true) %> <%= t(".navigation") %> | 80 | <%= link_to menu_items_path, class: "action_button" do %> |
| 81 | <%= icon("menu-2", library: "tabler", "aria-hidden": true) %> <%= t(".navigation") %> | ||
| 82 | <% end %> | ||
| 79 | <% end %> | 83 | <% end %> |
| 80 | <% trash_count = Node.trash.children.count %> | 84 | <% trash_count = Node.trash.children.count %> |
| 81 | <%= link_to trashed_nodes_path, class: "action_button" do %> | 85 | <%= 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: | |||
| 574 | headline_ineligible: "Dieser Asset-Typ kann kein Aufmacher sein." | 574 | headline_ineligible: "Dieser Asset-Typ kann kein Aufmacher sein." |
| 575 | locked_by_other: "Die Seite ist gerade von jemand anderem gesperrt." | 575 | locked_by_other: "Die Seite ist gerade von jemand anderem gesperrt." |
| 576 | autosave_failed: "Autosave fehlgeschlagen" | 576 | autosave_failed: "Autosave fehlgeschlagen" |
| 577 | redaktion_required: "Diese Seite ist der Redaktion vorbehalten." | ||
| 578 | admin_required: "Diese Seite ist der Administration vorbehalten." | ||
| 577 | sessions: | 579 | sessions: |
| 578 | otp_setup_now: "Dein Konto erfordert einen zweiten Faktor — richte ihn jetzt ein." | 580 | otp_setup_now: "Dein Konto erfordert einen zweiten Faktor — richte ihn jetzt ein." |
| 579 | logged_out: "Du wurdest abgemeldet." | 581 | 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: | |||
| 533 | headline_ineligible: "This asset type cannot be a headline." | 533 | headline_ineligible: "This asset type cannot be a headline." |
| 534 | locked_by_other: "The page is locked by another editor." | 534 | locked_by_other: "The page is locked by another editor." |
| 535 | autosave_failed: "Autosave failed" | 535 | autosave_failed: "Autosave failed" |
| 536 | redaktion_required: "This page is reserved for Redaktion." | ||
| 537 | admin_required: "This page is reserved for administrators." | ||
| 536 | sessions: | 538 | sessions: |
| 537 | otp_setup_now: "Your account requires a second factor -- set it up now." | 539 | otp_setup_now: "Your account requires a second factor -- set it up now." |
| 538 | logged_out: "You have been logged out." | 540 | 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 | |||
| 9 | end | 9 | end |
| 10 | 10 | ||
| 11 | test "updating stores a title per locale" do | 11 | test "updating stores a title per locale" do |
| 12 | login_as :quentin | 12 | login_as :aaron |
| 13 | item = create_menu_item | 13 | item = create_menu_item |
| 14 | 14 | ||
| 15 | patch :update, params: { :id => item.id, | 15 | patch :update, params: { :id => item.id, |
| @@ -20,7 +20,7 @@ class MenuItemsControllerTest < ActionController::TestCase | |||
| 20 | end | 20 | end |
| 21 | 21 | ||
| 22 | test "blanking a non-default title falls back to the default locale" do | 22 | test "blanking a non-default title falls back to the default locale" do |
| 23 | login_as :quentin | 23 | login_as :aaron |
| 24 | item = create_menu_item | 24 | item = create_menu_item |
| 25 | patch :update, params: { :id => item.id, | 25 | patch :update, params: { :id => item.id, |
| 26 | :menu_item => { :titles => { "de" => "Transparenz", "en" => "Transparency" } } } | 26 | :menu_item => { :titles => { "de" => "Transparenz", "en" => "Transparency" } } } |
| @@ -33,7 +33,7 @@ class MenuItemsControllerTest < ActionController::TestCase | |||
| 33 | end | 33 | end |
| 34 | 34 | ||
| 35 | test "a blank default title is rejected" do | 35 | test "a blank default title is rejected" do |
| 36 | login_as :quentin | 36 | login_as :aaron |
| 37 | item = create_menu_item | 37 | item = create_menu_item |
| 38 | patch :update, params: { :id => item.id, | 38 | patch :update, params: { :id => item.id, |
| 39 | :menu_item => { :titles => { "de" => "" } } } | 39 | :menu_item => { :titles => { "de" => "" } } } |
| @@ -41,4 +41,11 @@ class MenuItemsControllerTest < ActionController::TestCase | |||
| 41 | assert_response :success # re-rendered :edit, not a redirect | 41 | assert_response :success # re-rendered :edit, not a redirect |
| 42 | assert_not_equal "", item.reload.translations.find_by(:locale => "de").title | 42 | assert_not_equal "", item.reload.translations.find_by(:locale => "de").title |
| 43 | end | 43 | end |
| 44 | |||
| 45 | test "an editor without redaktion cannot reach the menu" do | ||
| 46 | login_as :quentin | ||
| 47 | get :index | ||
| 48 | assert_redirected_to admin_path | ||
| 49 | assert_equal I18n.t("flash.common.redaktion_required"), flash[:error] | ||
| 50 | end | ||
| 44 | end | 51 | 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' | |||
| 2 | 2 | ||
| 3 | class UsersControllerTest < ActionController::TestCase | 3 | class UsersControllerTest < ActionController::TestCase |
| 4 | 4 | ||
| 5 | test "get index as regular user renders stripped partial" do | 5 | test "an editor without admin cannot reach the user list" do |
| 6 | login_as :quentin | 6 | login_as :quentin |
| 7 | get :index | 7 | get :index |
| 8 | assert_response :success | 8 | assert_redirected_to admin_path |
| 9 | assert_select "a", { :count => 0, :text => "Destroy" } | 9 | assert_equal I18n.t("flash.common.admin_required"), flash[:error] |
| 10 | end | 10 | end |
| 11 | 11 | ||
| 12 | test "get index as admin shows every group with per-row actions" do | 12 | test "get index as admin shows every group with per-row actions" do |
| @@ -27,10 +27,10 @@ class UsersControllerTest < ActionController::TestCase | |||
| 27 | login_as :quentin | 27 | login_as :quentin |
| 28 | get :new | 28 | get :new |
| 29 | assert_response :redirect | 29 | assert_response :redirect |
| 30 | assert_redirected_to users_path | 30 | assert_redirected_to admin_path |
| 31 | assert_equal( | 31 | assert_equal( |
| 32 | I18n.t("flash.common.admin_required"), | 32 | I18n.t("flash.common.admin_required"), |
| 33 | flash[:notice] | 33 | flash[:error] |
| 34 | ) | 34 | ) |
| 35 | end | 35 | end |
| 36 | 36 | ||
| @@ -82,20 +82,20 @@ class UsersControllerTest < ActionController::TestCase | |||
| 82 | } | 82 | } |
| 83 | end | 83 | end |
| 84 | 84 | ||
| 85 | assert_redirected_to users_path | 85 | assert_redirected_to admin_path |
| 86 | assert_equal( | 86 | assert_equal( |
| 87 | I18n.t("flash.common.admin_required"), | 87 | I18n.t("flash.common.admin_required"), |
| 88 | flash[:notice] | 88 | flash[:error] |
| 89 | ) | 89 | ) |
| 90 | end | 90 | end |
| 91 | 91 | ||
| 92 | test "get edit of another user being logged in as regular user wont work" do | 92 | test "get edit of another user being logged in as regular user wont work" do |
| 93 | login_as :quentin | 93 | login_as :quentin |
| 94 | get :edit, params: { :id => User.find_by_login("aaron").id } | 94 | get :edit, params: { :id => User.find_by_login("aaron").id } |
| 95 | assert_redirected_to users_path | 95 | assert_redirected_to admin_path |
| 96 | assert_equal( | 96 | assert_equal( |
| 97 | I18n.t("flash.common.admin_required"), | 97 | I18n.t("flash.common.admin_required"), |
| 98 | flash[:notice] | 98 | flash[:error] |
| 99 | ) | 99 | ) |
| 100 | end | 100 | end |
| 101 | 101 | ||
| @@ -115,10 +115,10 @@ class UsersControllerTest < ActionController::TestCase | |||
| 115 | user = User.find_by_login("aaron") | 115 | user = User.find_by_login("aaron") |
| 116 | login_as :quentin | 116 | login_as :quentin |
| 117 | put :update, params: { :id => user.id, :user => {:login => "random"} } | 117 | put :update, params: { :id => user.id, :user => {:login => "random"} } |
| 118 | assert_redirected_to users_path | 118 | assert_redirected_to admin_path |
| 119 | assert_equal( | 119 | assert_equal( |
| 120 | I18n.t("flash.common.admin_required"), | 120 | I18n.t("flash.common.admin_required"), |
| 121 | flash[:notice] | 121 | flash[:error] |
| 122 | ) | 122 | ) |
| 123 | end | 123 | end |
| 124 | 124 | ||
| @@ -140,7 +140,7 @@ class UsersControllerTest < ActionController::TestCase | |||
| 140 | 140 | ||
| 141 | test "showing a user" do | 141 | test "showing a user" do |
| 142 | login_as :quentin | 142 | login_as :quentin |
| 143 | get :show, params: { :id => User.find_by_login("aaron").id } | 143 | get :show, params: { :id => users(:quentin).id } |
| 144 | assert_response :success | 144 | assert_response :success |
| 145 | end | 145 | end |
| 146 | 146 | ||
| @@ -148,7 +148,7 @@ class UsersControllerTest < ActionController::TestCase | |||
| 148 | login_as :quentin | 148 | login_as :quentin |
| 149 | put :deactivate, params: { :id => users(:quentin).id } | 149 | put :deactivate, params: { :id => users(:quentin).id } |
| 150 | 150 | ||
| 151 | assert_redirected_to users_path | 151 | assert_redirected_to admin_path |
| 152 | assert_not users(:quentin).reload.alumni? | 152 | assert_not users(:quentin).reload.alumni? |
| 153 | end | 153 | end |
| 154 | 154 | ||
| @@ -227,4 +227,16 @@ class UsersControllerTest < ActionController::TestCase | |||
| 227 | assert_response :success | 227 | assert_response :success |
| 228 | assert_select "h2", :text => /#{I18n.t("users.index.group_alumni")}/ | 228 | assert_select "h2", :text => /#{I18n.t("users.index.group_alumni")}/ |
| 229 | end | 229 | end |
| 230 | |||
| 231 | test "an editor without admin cannot create accounts" do | ||
| 232 | login_as :quentin | ||
| 233 | get :new | ||
| 234 | assert_redirected_to admin_path | ||
| 235 | end | ||
| 236 | |||
| 237 | test "an editor cannot read another account by id" do | ||
| 238 | login_as :quentin | ||
| 239 | get :show, params: { :id => users(:aaron).id } | ||
| 240 | assert_redirected_to admin_path | ||
| 241 | end | ||
| 230 | end | 242 | 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: | |||
| 14 | crypted_password: 740a48caf7dd5ff11318d812d57c0a0928cfbc12 # 'monkey' | 14 | crypted_password: 740a48caf7dd5ff11318d812d57c0a0928cfbc12 # 'monkey' |
| 15 | created_at: 2024-01-02 00:00:00 | 15 | created_at: 2024-01-02 00:00:00 |
| 16 | roles: ["admin", "redaktion"] | 16 | roles: ["admin", "redaktion"] |
| 17 | |||
| 18 | redella: | ||
| 19 | id: 3 | ||
| 20 | login: redella | ||
| 21 | email: redella@example.com | ||
| 22 | salt: cf993996a70d31f924aff17a5f997722cb6ec2dd | ||
| 23 | crypted_password: 11c672158b0eb6e8c91c438b3eb844902308b138 # 'monkey' | ||
| 24 | created_at: 2024-01-03 00:00:00 | ||
| 25 | roles: ["redaktion"] | ||
| 26 | |||
| 27 | alufa: | ||
| 28 | id: 4 | ||
| 29 | login: alufa | ||
| 30 | email: alufa@example.com | ||
| 31 | salt: cf993996a70d31f924aff17a5f997722cb6ec2dd | ||
| 32 | crypted_password: 11c672158b0eb6e8c91c438b3eb844902308b138 # 'monkey' | ||
| 33 | created_at: 2024-01-04 00:00:00 | ||
| 34 | roles: ["redaktion", "alumni"] | ||
