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 @@
+# 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"]