From 464dd4266bdc433805010b5dca428f4cb75c2a81 Mon Sep 17 00:00:00 2001 From: erdgeist Date: Fri, 31 Jul 2026 15:55:43 +0200 Subject: Group user accounts by role Replaces the two-way admin/user split with four groups ordered by capability: administration, Redaktion, editors, alumni. alumni takes precedence over capability in role_group, so a retired admin appears at the bottom rather than the top. Forms now offer the three roles as checkboxes rather than a single admin checkbox, with a trailing hidden blank so an empty set can be posted, and user_params permits roles only for admins. Three create buttons prefill the common combinations. --- app/controllers/users_controller.rb | 27 ++++++++---- app/helpers/users_helper.rb | 9 ---- app/models/user.rb | 51 ++++++++++++++++++++++- app/views/users/_user.html.erb | 13 ++++-- app/views/users/edit.html.erb | 14 ++++++- app/views/users/index.html.erb | 55 ++++++++++++++----------- app/views/users/new.html.erb | 14 ++++++- app/views/users/show.html.erb | 6 ++- config/locales/de.yml | 29 +++++++++++-- config/locales/en.yml | 29 +++++++++++-- db/migrate/20260731124136_add_roles_to_users.rb | 13 ++++++ lib/authenticated_system.rb | 7 +++- lib/tasks/development_init.rake | 2 +- public/stylesheets/admin.css | 19 +++++++++ test/controllers/users_controller_test.rb | 27 ++++++++---- test/fixtures/users.yml | 3 +- 16 files changed, 243 insertions(+), 75 deletions(-) delete mode 100644 app/helpers/users_helper.rb create mode 100644 db/migrate/20260731124136_add_roles_to_users.rb diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index cb71db23..95dff220 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -9,14 +9,20 @@ class UsersController < ApplicationController layout 'admin' + ROLE_PRESETS = { + "editor" => [], + "redaktion" => ["redaktion"], + "admin" => ["admin", "redaktion"] + }.freeze + + GROUP_ORDER = [:admin, :redaktion, :editor, :alumni].freeze + def index - @users = User.order("login ASC").all.group_by do |user| - user.admin? ? :admin : :user - end + @users = User.order("login ASC").all.group_by(&:role_group) end def new - @user = User.new(admin: params[:admin].present?) + @user = User.new(:roles => ROLE_PRESETS.fetch(params[:preset], [])) end def create @@ -35,8 +41,7 @@ class UsersController < ApplicationController def update permitted = user_params - permitted.delete(:admin) unless current_user.is_admin? - + if @user.update(permitted) flash[:notice] = t("flash.users.updated", :login => @user.login) redirect_to user_path(@user) @@ -63,9 +68,13 @@ class UsersController < ApplicationController private def user_params - allowed = [:login, :email, :password, :password_confirmation] - allowed << :admin if current_user.admin? - params.fetch(:user, {}).permit(allowed) + permitted = params.fetch(:user, {}) + .permit(:login, :email, :password, :password_confirmation, + :roles => []) + # Checkbox arrays post a leading blank from the hidden field. + permitted[:roles] = Array(permitted[:roles]).reject(&:blank?) if permitted.key?(:roles) + permitted.delete(:roles) unless current_user.is_admin? + permitted end def find_user diff --git a/app/helpers/users_helper.rb b/app/helpers/users_helper.rb deleted file mode 100644 index ff031387..00000000 --- a/app/helpers/users_helper.rb +++ /dev/null @@ -1,9 +0,0 @@ -module UsersHelper - def user_list_by_admin_status - if current_user && current_user.admin - render :partial => 'admin_user_item', :collection => @users - else - render :partial => 'user_item', :collection => @users - end - end -end diff --git a/app/models/user.rb b/app/models/user.rb index e1eff059..2e9da86c 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -9,6 +9,8 @@ class User < ApplicationRecord include Authentication include Authentication::ByPassword + ROLES = %w[redaktion admin alumni].freeze + # Validations validates_presence_of :login validates_length_of :login, :within => 1..40 @@ -22,6 +24,8 @@ class User < ApplicationRecord validates_format_of :email, :with => Authentication.email_regex, :message => Authentication.bad_email_message + validate :roles_are_known + # Authenticates a user by their login name and unencrypted password. Returns the user or nil. def self.authenticate(login, password) return if login.blank? || password.blank? @@ -60,7 +64,45 @@ class User < ApplicationRecord end def is_admin? - !!admin + roles.include?("admin") + end + + # Compatibility shims for the users form, which posts user[admin] as a + # checkbox. Goes away when that form learns about roles. + def admin + is_admin? + end + + def admin? + is_admin? + end + + def admin=(value) + if ActiveModel::Type::Boolean.new.cast(value) + self.roles = (roles | ["admin"]) + else + self.roles = (roles - ["admin"]) + end + end + + def redaktion? + roles.include?("redaktion") + end + + def alumni? + roles.include?("alumni") + end + + def role_group + return :alumni if alumni? + return :admin if is_admin? + return :redaktion if redaktion? + :editor + end + + # Human-readable role names, for the list and the forms. + def role_labels + roles.map { |r| I18n.t("users.roles.#{r}", :default => r) } end # otp_secret present == enrolled. otp_pending_secret holds the secret @@ -133,4 +175,11 @@ class User < ApplicationRecord end true end + + private + + def roles_are_known + unknown = roles.to_a - ROLES + errors.add(:roles, :unknown, :list => unknown.join(", ")) if unknown.any? + end end diff --git a/app/views/users/_user.html.erb b/app/views/users/_user.html.erb index 9c6466a2..04884be8 100644 --- a/app/views/users/_user.html.erb +++ b/app/views/users/_user.html.erb @@ -1,11 +1,16 @@ <% users.each do |user| %> <%= user.login %> - <%= link_to "show", user_path(user) %> - <% if current_user.admin? || current_user == user %> - - <%= link_to "edit", edit_user_path(user) %> + + <% if user.roles.any? %> + <%= user.role_labels.join(", ") %> + <% else %> + <%= t(".no_roles") %> + <% end %> + <%= link_to t("admin.common.show"), user_path(user) %> + <% if current_user.admin? || current_user == user %> + <%= link_to t("admin.common.edit"), edit_user_path(user) %> <%= button_to user_path(user), method: :delete, form: { data: { confirm: t(".confirm_destroy", :login => user.login) }, class: 'button_to destructive' } do %> diff --git a/app/views/users/edit.html.erb b/app/views/users/edit.html.erb index 9df93815..ad2e0891 100644 --- a/app/views/users/edit.html.erb +++ b/app/views/users/edit.html.erb @@ -21,8 +21,18 @@
<%= f.password_field :password_confirmation, :autocomplete => "new-password" %>
<% if current_user.admin? %> -
<%= t("users.labels.admin") %>
-
<%= f.check_box :admin %>
+
<%= t("users.labels.roles") %>
+
+ <% User::ROLES.each do |role| %> + + <% end %> + <%= hidden_field_tag "user[roles][]", "" %> +
<% end %>
diff --git a/app/views/users/index.html.erb b/app/views/users/index.html.erb index 0a003cb1..854811a2 100644 --- a/app/views/users/index.html.erb +++ b/app/views/users/index.html.erb @@ -1,27 +1,32 @@ -

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

-<%= link_to new_user_path(admin: true), class: 'action_button' do %> - <%= icon("plus", library: "tabler", "aria-hidden": true) %> <%= t(".create_admin") %> -<% end %> - - - - - - - - <%= render :partial => "user", :locals => {:users => @users[:admin] ||= []} %> -
<%= t("users.labels.login") %>
+

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

+ +

+ <% UsersController::ROLE_PRESETS.each_key do |preset| %> + <%= link_to new_user_path(:preset => preset), class: 'action_button' do %> + <%= icon("plus", library: "tabler", "aria-hidden": true) %> + <%= t(".create_#{preset}") %> + <% end %> + <% end %> +

-

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

-<%= link_to new_user_path, class: 'action_button' do %> - <%= icon("plus", library: "tabler", "aria-hidden": true) %> <%= t(".create_user") %> +<% UsersController::GROUP_ORDER.each do |group| %> + <% members = @users[group] || [] %> +

"> + <%= t(".group_#{group}") %> + <%= members.size %> +

+ <% if members.any? %> + "> + + + + + + + + <%= render :partial => "user", :locals => { :users => members } %> +
<%= t("users.labels.login") %><%= t("users.labels.roles") %>
+ <% else %> +

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

+ <% end %> <% end %> - - - - - - - - <%= render :partial => "user", :locals => {:users => @users[:user] ||= []} %> -
<%= t("users.labels.login") %>
diff --git a/app/views/users/new.html.erb b/app/views/users/new.html.erb index 776e6e96..8d99fd92 100644 --- a/app/views/users/new.html.erb +++ b/app/views/users/new.html.erb @@ -21,8 +21,18 @@
<%= t("users.labels.confirm") %>
<%= f.password_field :password_confirmation %>
-
<%= t("users.labels.admin") %>
-
<%= f.check_box :admin %>
+
<%= t("users.labels.roles") %>
+
+ <% User::ROLES.each do |role| %> + + <% end %> + <%= hidden_field_tag "user[roles][]", "" %> +
<%= f.submit t("admin.common.create") %>
diff --git a/app/views/users/show.html.erb b/app/views/users/show.html.erb index 44976800..a320e53e 100644 --- a/app/views/users/show.html.erb +++ b/app/views/users/show.html.erb @@ -19,7 +19,9 @@
<%= t("users.labels.email") %>
<%= @user.email %>
-
<%= t("users.labels.admin") %>
-
<%= @user.admin ? t("admin.common.yes") : t("admin.common.no") %>
+
<%= t("users.labels.roles") %>
+
+ <%= @user.roles.any? ? @user.role_labels.join(", ") : t("users.no_roles") %> +
diff --git a/config/locales/de.yml b/config/locales/de.yml index cf93e05b..e15a08d2 100644 --- a/config/locales/de.yml +++ b/config/locales/de.yml @@ -145,6 +145,10 @@ de: restore_target_invalid: "Das Wiederherstellungsziel muss ein lebender Node sein" destroy_outside_trash: "Nodes können nur aus dem Papierkorb gelöscht werden" attach_in_trash: "An einen Node im Papierkorb können keine Assets angehängt werden" + user: + attributes: + roles: + unknown: "enthält unbekannte Rollen: %{list}" related_asset: attributes: headline: @@ -276,11 +280,28 @@ de: admin: "Admin?" user: confirm_destroy: "Benutzer %{login} wirklich löschen?" + no_roles: "—" index: - admins: "Admins" - create_admin: "Admin-Benutzer anlegen" - users: "Benutzer" - create_user: "Benutzer anlegen" + title: "Benutzerkonten" + create_editor: "Editor-Konto anlegen" + create_redaktion: "Redaktions-Konto anlegen" + create_admin: "Admin-Konto anlegen" + group_admin: "Administration" + group_redaktion: "Redaktion" + group_editor: "Editors" + group_alumni: "Ehemalige" + group_empty: "— keine —" + labels: + roles: "Rollen" + roles: + admin: "Administration" + redaktion: "Redaktion" + alumni: "Ehemalig" + role_hints: + admin: "Benutzerverwaltung und Navigation, jeweils nach erneuter Bestätigung." + redaktion: "Darf in den geschützten Bereichen veröffentlichen." + alumni: "Kein Login mehr möglich; Zuschreibungen bleiben erhalten." + no_roles: "keine besonderen Rollen" new: title: "Neuen Benutzer anlegen" diff --git a/config/locales/en.yml b/config/locales/en.yml index c3f515b5..f8b94a00 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -92,6 +92,10 @@ en: restore_target_invalid: "Restore target must be a living node" destroy_outside_trash: "Nodes are only destroyed from the Trash" attach_in_trash: "Cannot attach assets to a node in the Trash" + user: + attributes: + roles: + unknown: "contains unknown roles: %{list}" related_asset: attributes: headline: @@ -224,11 +228,28 @@ en: admin: "admin?" user: confirm_destroy: "Do you really want to destroy user %{login}?" + no_roles: "—" index: - admins: "Admins" - create_admin: "Create admin user" - users: "Users" - create_user: "Create user" + title: "User accounts" + create_editor: "Create editor account" + create_redaktion: "Create Redaktion account" + create_admin: "Create admin account" + group_admin: "Administration" + group_redaktion: "Redaktion" + group_editor: "Editors" + group_alumni: "Alumni" + group_empty: "— none —" + labels: + roles: "Roles" + roles: + admin: "Administration" + redaktion: "Redaktion" + alumni: "Alumnus" + role_hints: + admin: "User management and navigation, each behind a fresh confirmation." + redaktion: "May publish in the protected sections." + alumni: "Can no longer log in; attributions are preserved." + no_roles: "no special roles" new: title: "Create new user" diff --git a/db/migrate/20260731124136_add_roles_to_users.rb b/db/migrate/20260731124136_add_roles_to_users.rb new file mode 100644 index 00000000..ea003e60 --- /dev/null +++ b/db/migrate/20260731124136_add_roles_to_users.rb @@ -0,0 +1,13 @@ +class AddRolesToUsers < ActiveRecord::Migration[8.1] + def up + add_column :users, :roles, :string, :array => true, :default => [], :null => false + execute "UPDATE users SET roles = ARRAY['admin','redaktion'] WHERE admin = true" + remove_column :users, :admin + end + + def down + add_column :users, :admin, :boolean + execute "UPDATE users SET admin = true WHERE 'admin' = ANY(roles)" + remove_column :users, :roles + end +end diff --git a/lib/authenticated_system.rb b/lib/authenticated_system.rb index 2ec15a77..4e70c28d 100644 --- a/lib/authenticated_system.rb +++ b/lib/authenticated_system.rb @@ -102,7 +102,12 @@ module AuthenticatedSystem def login_from_session return unless session[:user_id] if session[:logged_in_at].to_i > SESSION_MAX_AGE.ago.to_i - self.current_user = User.find_by(:id => session[:user_id]) + user = User.find_by(:id => session[:user_id]) + if user.nil? || user.alumni? + session[:user_id] = nil + else + self.current_user = user + end else session[:user_id] = nil end diff --git a/lib/tasks/development_init.rake b/lib/tasks/development_init.rake index 94f323c3..91797d9b 100644 --- a/lib/tasks/development_init.rake +++ b/lib/tasks/development_init.rake @@ -19,7 +19,7 @@ namespace :cccms do :email => 'admin@cccms.de', :password => 'foobar', :password_confirmation => 'foobar', - :admin => true + :roles => ["admin", "redaktion"] ) end diff --git a/public/stylesheets/admin.css b/public/stylesheets/admin.css index c0496c6e..9991fad0 100644 --- a/public/stylesheets/admin.css +++ b/public/stylesheets/admin.css @@ -796,6 +796,25 @@ table.revisions_table tr:hover { margin: 0; } +.user_group_heading { + margin-top: 1.5rem; +} + +/* Retired accounts: present for attribution, not for action. */ +.user_group_alumni, +.user_table_alumni { + color: var(--text-muted); +} + +.role_choice { + display: block; + margin-bottom: 0.35rem; +} + +.role_choice .field_hint { + margin-left: 0.5rem; +} + /* ============================================================ Translation compare view (page_translations#show) ============================================================ */ diff --git a/test/controllers/users_controller_test.rb b/test/controllers/users_controller_test.rb index 67f7c053..1c5d16fc 100644 --- a/test/controllers/users_controller_test.rb +++ b/test/controllers/users_controller_test.rb @@ -9,12 +9,12 @@ class UsersControllerTest < ActionController::TestCase assert_select "a", { :count => 0, :text => "Destroy" } end - test "get index as admin user renders admin partial" do + test "get index as admin shows every group with per-row actions" do login_as :aaron get :index assert_response :success assert_select "button[type=submit]", I18n.t("admin.common.destroy") - assert_select "a", "show" + assert_select "a", I18n.t("admin.common.show") end test "get new when logged in as admin" do @@ -60,7 +60,7 @@ class UsersControllerTest < ActionController::TestCase :email => "foo@bar.com", :password => "xxxzzz", :password_confirmation => "xxxzzz", - :admin => true + :roles => ["admin", "redaktion"] } } end @@ -174,19 +174,17 @@ class UsersControllerTest < ActionController::TestCase test "admin user can promote regular users to admins" do login_as :aaron user = users(:quentin) - put :update, params: { :id => user.id, :user => {:admin => true} } + put :update, params: { :id => user.id, :user => {:roles => ["admin", "redaktion"]} } - user.reload - assert_equal true, user.is_admin? + assert_equal true, user.reload.is_admin? end test "regular users cannot promote themselves to admins" do login_as :quentin user = users(:quentin) - put :update, params: { :id => user.id, :user => {:admin => true} } + put :update, params: { :id => user.id, :user => {:roles => ["admin", "redaktion"]} } - user.reload - assert_equal false, user.is_admin? + assert_equal false, user.reload.is_admin? end test "reset_otp is admin-only and witnessed" do @@ -202,4 +200,15 @@ class UsersControllerTest < ActionController::TestCase assert_not user.reload.otp_enrolled? assert_equal "otp_reset", NodeAction.last.action end + + test "index groups a retired admin under alumni, not administration" do + login_as :aaron + user = users(:quentin) + user.update_column(:roles, ["admin", "alumni"]) + + get :index + + assert_response :success + assert_select "h2", :text => /#{I18n.t("users.index.group_alumni")}/ + end end diff --git a/test/fixtures/users.yml b/test/fixtures/users.yml index 7276bcb4..f8d32d3c 100644 --- a/test/fixtures/users.yml +++ b/test/fixtures/users.yml @@ -13,5 +13,4 @@ aaron: salt: 5be6f9cdd04fd7ab3c91cd32a5334ba2339b8005 crypted_password: 740a48caf7dd5ff11318d812d57c0a0928cfbc12 # 'monkey' created_at: 2024-01-02 00:00:00 - admin: true - + roles: ["admin", "redaktion"] -- cgit v1.3