From ba64d8ff56f1f2eb87dac92850dc2f398ad1fefb Mon Sep 17 00:00:00 2001 From: Klaus Zanders Date: Mon, 4 May 2026 10:20:24 +0200 Subject: [PATCH] Ensure that only roles with fitting permissions can be used as the role for the project creator --- .../settings/new_project_settings_form.rb | 5 ++- app/models/project_role.rb | 27 +++++++++++- config/locales/en.yml | 3 ++ .../wizard_from_template_flow_spec.rb | 2 +- spec/models/project_role_spec.rb | 44 ++++++++++++++++--- .../projects/copy_service_integration_spec.rb | 4 +- 6 files changed, 74 insertions(+), 11 deletions(-) diff --git a/app/forms/settings/new_project_settings_form.rb b/app/forms/settings/new_project_settings_form.rb index 0e97ca72108..d6d6f2673bb 100644 --- a/app/forms/settings/new_project_settings_form.rb +++ b/app/forms/settings/new_project_settings_form.rb @@ -51,10 +51,11 @@ module Settings f.select_list( name: :new_project_user_role_id, label: I18n.t(:setting_new_project_user_role_id), + caption: I18n.t(:setting_new_project_user_role_id_caption), input_width: :medium, - include_blank: I18n.t(:actionview_instancetag_blank_option) + include_blank: false ) do |select| - ProjectRole.givable.each do |role| + ProjectRole.assignable_to_project_creator.each do |role| select.option( value: role.id.to_s, label: role.name, diff --git a/app/models/project_role.rb b/app/models/project_role.rb index 430c78e2e10..1b59726a104 100644 --- a/app/models/project_role.rb +++ b/app/models/project_role.rb @@ -29,6 +29,12 @@ # ++ class ProjectRole < Role + # Permissions a role must grant in order to be assignable as the default + # role for a non-admin user who creates a project. Without these, the + # creator cannot complete project setup (filling out the PIR, adding + # members, etc.). + PERMISSIONS_FOR_PROJECT_CREATOR = %i[edit_project_attributes manage_members].freeze + has_many :custom_fields_roles, foreign_key: "role_id", dependent: :restrict_with_error, @@ -39,6 +45,20 @@ class ProjectRole < Role .where(type: "ProjectRole") end + # Roles eligible to be granted to a non-admin user upon project creation. + # Restricted to givable roles that include all PERMISSIONS_FOR_PROJECT_CREATOR. + def self.assignable_to_project_creator + permissions = PERMISSIONS_FOR_PROJECT_CREATOR.map(&:to_s) + + role_ids = RolePermission + .where(permission: permissions) + .group(:role_id) + .having("COUNT(DISTINCT permission) = ?", permissions.size) + .select(:role_id) + + givable.where(id: role_ids) + end + # Return the builtin 'non member' role. If the role doesn't exist, # it will be created on the fly. def self.non_member @@ -66,9 +86,12 @@ class ProjectRole < Role end def self.in_new_project - givable + assignable_to_project_creator .except(:order) - .order(Arel.sql("COALESCE(#{Setting.new_project_user_role_id.to_i} = id, false) DESC, position")) + .reorder(Arel.sql( + "COALESCE(#{Setting.new_project_user_role_id.to_i} = #{quoted_table_name}.id, false) DESC, " \ + "#{quoted_table_name}.position" + )) .first end end diff --git a/config/locales/en.yml b/config/locales/en.yml index a4982f47c17..4862bd155e8 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -5349,6 +5349,9 @@ en: setting_mail_handler_body_delimiter_regex: "Truncate emails matching this regex" setting_mail_handler_ignore_filenames: "Ignored mail attachments" setting_new_project_user_role_id: "Role given to a non-admin user who creates a project" + setting_new_project_user_role_id_caption: > + Only roles that include the permissions to edit project attributes and to manage members are listed, + so that the creator can complete the project setup. setting_new_project_send_confirmation_email: "Send notification to author when creating a new project" setting_new_project_notification_text: "Notification text" setting_password_active_rules: "Password requirements" diff --git a/spec/features/projects/creation_wizard/wizard_from_template_flow_spec.rb b/spec/features/projects/creation_wizard/wizard_from_template_flow_spec.rb index a047373aca6..4c76de33a20 100644 --- a/spec/features/projects/creation_wizard/wizard_from_template_flow_spec.rb +++ b/spec/features/projects/creation_wizard/wizard_from_template_flow_spec.rb @@ -44,7 +44,7 @@ RSpec.describe "Project creation wizard from a template", # Role assigned to users in newly created projects - only wizard permissions, NO add_work_packages shared_let(:new_project_role) do create(:project_role, - permissions: %i[view_work_packages view_project_attributes edit_project_attributes]) + permissions: %i[view_work_packages view_project_attributes edit_project_attributes manage_members]) end # Role for the assignee user - assigned via the user custom field role assignment diff --git a/spec/models/project_role_spec.rb b/spec/models/project_role_spec.rb index b51e84336b4..25a33653a2d 100644 --- a/spec/models/project_role_spec.rb +++ b/spec/models/project_role_spec.rb @@ -42,21 +42,45 @@ RSpec.describe ProjectRole do it { expect(described_class.givable).to contain_exactly project_role } end - describe ".in_new_project" do + describe ".assignable_to_project_creator" do let!(:ungivable_role) { create(:non_member) } + let!(:role_with_all_perms) do + create(:project_role, permissions: ProjectRole::PERMISSIONS_FOR_PROJECT_CREATOR + %i[view_work_packages]) + end + let!(:role_missing_one_perm) do + create(:project_role, permissions: [ProjectRole::PERMISSIONS_FOR_PROJECT_CREATOR.first]) + end + let!(:role_with_unrelated_perms) do + create(:project_role, permissions: %i[view_work_packages]) + end + + it "includes only givable roles that grant all required permissions" do + expect(described_class.assignable_to_project_creator) + .to contain_exactly(role_with_all_perms) + end + end + + describe ".in_new_project" do + let(:required_permissions) { ProjectRole::PERMISSIONS_FOR_PROJECT_CREATOR } + let!(:ungivable_role) { create(:non_member) } + let!(:role_without_required_permissions) do + create(:project_role, permissions: %i[view_work_packages]).tap do |r| + r.update_column(:position, 0) + end + end let!(:second_role) do - create(:project_role).tap do |r| + create(:project_role, permissions: required_permissions).tap do |r| r.update_column(:position, 100) end end let!(:first_role) do - create(:project_role).tap do |r| + create(:project_role, permissions: required_permissions).tap do |r| r.update_column(:position, 1) end end context "without a specified role" do - it "returns the first role (by position)" do + it "returns the first qualifying role (by position)" do expect(described_class.in_new_project) .to eql first_role end @@ -82,11 +106,21 @@ RSpec.describe ProjectRole do .and_return("-1") end - it "returns the first role (by position)" do + it "returns the first qualifying role (by position)" do expect(described_class.in_new_project) .to eql first_role end end + + context "when no role has the required permissions" do + before do + described_class.where.not(id: role_without_required_permissions.id).destroy_all + end + + it "returns nil" do + expect(described_class.in_new_project).to be_nil + end + end end describe ".anonymous" do diff --git a/spec/services/projects/copy_service_integration_spec.rb b/spec/services/projects/copy_service_integration_spec.rb index 8aa8166b174..6b127707064 100644 --- a/spec/services/projects/copy_service_integration_spec.rb +++ b/spec/services/projects/copy_service_integration_spec.rb @@ -91,7 +91,9 @@ RSpec.describe( edit_project_attributes select_project_custom_fields]) end - shared_let(:new_project_role) { create(:project_role, permissions: %i[]) } + shared_let(:new_project_role) do + create(:project_role, permissions: ProjectRole::PERMISSIONS_FOR_PROJECT_CREATOR) + end before do allow(Setting)