diff --git a/app/components/members/user_filter_component.rb b/app/components/members/user_filter_component.rb index 386c4abd69f7..894018f6830a 100644 --- a/app/components/members/user_filter_component.rb +++ b/app/components/members/user_filter_component.rb @@ -52,8 +52,10 @@ def extra_user_status_options end def status_members_query(status) - params = { project_id: project.id, - status: } + params = { + project_id: project.id, + status: + } self.class.filter(params) end @@ -62,8 +64,19 @@ def filter_path project_members_path(project) end - def self.base_query - Queries::Members::MemberQuery + class << self + def base_query + Queries::Members::MemberQuery + end + + protected + + def apply_filters(params, query) + super(params, query) + query.where(:only_project_member, '=', 't') + + query + end end end end diff --git a/app/contracts/base_contract.rb b/app/contracts/base_contract.rb index cd7f9cb728ac..1e0f321fba76 100644 --- a/app/contracts/base_contract.rb +++ b/app/contracts/base_contract.rb @@ -239,7 +239,7 @@ def reduce_by_writable_conditions(attributes) attributes end - def reduce_by_writable_permissions(attributes) # rubocop:disable Metrics/PerceivedComplexity, Metrics/AbcSize + def reduce_by_writable_permissions(attributes) attribute_permissions = collect_ancestor_attributes(:attribute_permissions) attributes.reject do |attribute| @@ -251,15 +251,34 @@ def reduce_by_writable_permissions(attributes) # rubocop:disable Metrics/Perceiv next unless permissions - # This will break once a model that does not respond to project is used. - # This is intended to be worked on then with the additional knowledge. - next if model.project.present? && permissions.any? { |perm| user.allowed_in_project?(perm, model.project) } - next if model.project.blank? && permissions.any? { |perm| user.allowed_in_any_project?(perm) } + next if permissions.any? do |perm| + user.allowed_based_on_permission_context?( + perm, + project: project_for_permission_check, + entity: entity_for_permission_check + ) + end true end end + def project_for_permission_check + if model.is_a?(Project) + model + else + model.respond_to?(:project) ? model.project : nil + end + end + + def entity_for_permission_check + if model.is_a?(Project) + nil + else + model + end + end + def with_merged_former_errors former_errors = errors.dup diff --git a/app/contracts/queries/update_contract.rb b/app/contracts/queries/update_contract.rb index 269ad18e6769..b9b685e55ad2 100644 --- a/app/contracts/queries/update_contract.rb +++ b/app/contracts/queries/update_contract.rb @@ -57,11 +57,7 @@ def user_allowed_to_change_public end def user_allowed_to_edit_work_packages? - if model.project - user.allowed_in_project?(:edit_work_packages, model.project) - else - user.allowed_in_any_project?(:edit_work_packages) - end + user.allowed_in_any_work_package?(:edit_work_packages, in_project: model.project) end def user_allowed_to_save_queries? diff --git a/app/contracts/relations/base_contract.rb b/app/contracts/relations/base_contract.rb index 86af8e7ea099..01b99134813e 100644 --- a/app/contracts/relations/base_contract.rb +++ b/app/contracts/relations/base_contract.rb @@ -78,7 +78,7 @@ def visible_work_packages end def manage_relations? - user.allowed_in_project?(:manage_work_package_relations, model.from.project) + user.allowed_in_work_package?(:manage_work_package_relations, model.from) end end end diff --git a/app/contracts/relations/delete_contract.rb b/app/contracts/relations/delete_contract.rb index 394c0cbd8b8b..a0a10c26465a 100644 --- a/app/contracts/relations/delete_contract.rb +++ b/app/contracts/relations/delete_contract.rb @@ -28,6 +28,6 @@ module Relations class DeleteContract < ::DeleteContract - delete_permission -> { user.allowed_in_project?(:manage_work_package_relations, model.from.project) } + delete_permission -> { user.allowed_in_work_package?(:manage_work_package_relations, model.from) } end end diff --git a/app/controllers/work_packages_controller.rb b/app/controllers/work_packages_controller.rb index 1f9ccf85399a..0635b21b775b 100644 --- a/app/controllers/work_packages_controller.rb +++ b/app/controllers/work_packages_controller.rb @@ -135,11 +135,7 @@ def protect_from_unauthorized_export end def user_allowed_to_export? - if @project - User.current.allowed_in_project?(:export_work_packages, @project) - else - User.current.allowed_in_any_project?(:export_work_packages) - end + User.current.allowed_in_any_work_package?(:export_work_packages, in_project: @project) end def supported_list_formats diff --git a/app/models/authorization/scopes/allowed_to.rb b/app/models/authorization/scopes/allowed_to.rb deleted file mode 100644 index 9142038314bd..000000000000 --- a/app/models/authorization/scopes/allowed_to.rb +++ /dev/null @@ -1,142 +0,0 @@ -# -- copyright -# OpenProject is an open source project management software. -# Copyright (C) 2010-2023 the OpenProject GmbH -# -# This program is free software; you can redistribute it and/or -# modify it under the terms of the GNU General Public License version 3. -# -# OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows: -# Copyright (C) 2006-2013 Jean-Philippe Lang -# Copyright (C) 2010-2013 the ChiliProject Team -# -# This program is free software; you can redistribute it and/or -# modify it under the terms of the GNU General Public License -# as published by the Free Software Foundation; either version 2 -# of the License, or (at your option) any later version. -# -# This program is distributed in the hope that it will be useful, -# but WITHOUT ANY WARRANTY; without even the implied warranty of -# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the -# GNU General Public License for more details. -# -# You should have received a copy of the GNU General Public License -# along with this program; if not, write to the Free Software -# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. -# -# See COPYRIGHT and LICENSE files for more details. -# ++ - -module Authorization::Scopes - module AllowedTo - extend ActiveSupport::Concern - - class_methods do - # Returns an ActiveRecord::Relation to find all entries for which - # +user+ has the given +permission+. - def allowed_to(user, permission) - permissions = allowed_to_permissions(permission) - - return none if user.locked? - return none if permissions.empty? - - if user.admin? && permissions.all?(&:grant_to_admin?) - allowed_to_admin(permissions) - elsif user.anonymous? - allowed_to_anonymous(user, permissions) - else - allowed_to_member(user, permissions) - end - end - - private - - def allowed_to_admin(permissions) - where(id: allowed_to_admin_relation(permissions)) - end - - def allowed_to_anonymous(user, permissions) - where(id: allowed_to_non_member_relation(user, permissions)) - end - - def allowed_to_member(user, permissions) - where(arel_table[:id].in(allowed_to_member_relation(user, permissions) - .select(arel_table[:id]) - .arel - .union(:all, allowed_to_non_member_relation(user, permissions).select(:id).arel))) - end - - def allowed_to_admin_relation(permissions) - joins(allowed_to_enabled_module_join(permissions)) - .where(Project.arel_table[:active].eq(true)) - end - - def allowed_to_member_relation(user, permissions) - Member - .joins(allowed_to_member_in_active_project_join(user)) - .joins(allowed_to_enabled_module_join(permissions)) - .joins(:roles) - .joins(allowed_to_role_permission_join(permissions)) - end - - def allowed_to_non_member_relation(_user, _permission) - raise NotImplementedError - end - - def allowed_to_enabled_module_join(permissions) # rubocop:disable Metrics/AbcSize - project_module = permissions.filter_map(&:project_module).uniq - enabled_module_table = EnabledModule.arel_table - projects_table = Project.arel_table - - if project_module.any? - arel_table.join(enabled_module_table, Arel::Nodes::InnerJoin) - .on(projects_table[:id].eq(enabled_module_table[:project_id]) - .and(enabled_module_table[:name].in(project_module)) - .and(projects_table[:active].eq(true))) - .join_sources - end - end - - def allowed_to_role_permission_join(permissions) # rubocop:disable Metrics/AbcSize - return if permissions.all?(&:public?) - - role_permissions_table = RolePermission.arel_table - enabled_modules_table = EnabledModule.arel_table - roles_table = Role.arel_table - - condition = permissions.inject(Arel::Nodes::False.new) do |or_condition, permission| - permission_condition = role_permissions_table[:permission].eq(permission.name) - - if permission.project_module.present? - permission_condition = permission_condition.and(enabled_modules_table[:name].eq(permission.project_module)) - end - - or_condition.or(permission_condition) - end - - arel_table - .join(role_permissions_table, Arel::Nodes::InnerJoin) - .on(roles_table[:id].eq(role_permissions_table[:role_id]) - .and(condition)) - .join_sources - end - - def allowed_to_members_condition(_user) - raise NotImplementedError - end - - def allowed_to_member_in_active_project_join(user) - Project.arel_table - .join(Project.arel_table) - .on(Project.arel_table[:active].eq(true) - .and(allowed_to_members_condition(user))) - .join_sources - end - - def allowed_to_permissions(permission) - Authorization.contextual_permissions(permission, - to_s.underscore.to_sym, - raise_on_unknown: true) - end - end - end -end diff --git a/app/models/journable/historic_active_record_relation.rb b/app/models/journable/historic_active_record_relation.rb index 4eaa4ee9317c..e10d32788341 100644 --- a/app/models/journable/historic_active_record_relation.rb +++ b/app/models/journable/historic_active_record_relation.rb @@ -114,6 +114,7 @@ def build_arel(aliases = nil) # Based on the previous modifications, build the algebra object. arel = relation.call_original_build_arel(aliases) + arel = modify_where_clauses(arel) arel = modify_order_clauses(arel) modify_joins(arel) end @@ -364,6 +365,108 @@ def modify_joins(arel) arel end + def modify_where_clauses(arel) + arel.instance_variable_get(:@ast).instance_variable_get(:@cores).each do |core| + core.instance_variable_get(:@wheres).each do |where_clause| + modify_conditions(where_clause) + end + end + + arel + end + + def modify_conditions(node) + if node.kind_of? Arel::TreeManager + # We have another sub-tree, investigate its core, which is a SelectCore + node.instance_variable_get(:@ast).instance_variable_get(:@cores).each do |core| + modify_conditions(core) + end + elsif node.kind_of? Arel::Nodes::SelectStatement + # A sub-select statement, we need to investigate its core, which is also a SelectCore + modify_conditions(node.instance_variable_get(:@cores).first) + + # all the orders need to be modified as well + node.instance_variable_get(:@orders).each do |order_clause| + if order_clause.kind_of? Arel::Nodes::SqlLiteral + gsub_table_names_in_sql_string!(order_clause) + elsif order_clause.expr.relation == model.arel_table + if order_clause.expr.name == "id" + order_clause.expr.name = "journable_id" + order_clause.expr.relation = Journal.arel_table + else + order_clause.expr.relation = model.journal_class.arel_table + end + end + end + elsif node.kind_of? Arel::Nodes::SelectCore + # We have another SelectCore, which is the main part of the select statement. + # Sources are the select table (left) and all joins (right) + source = node.instance_variable_get(:@source) + + # when we are selecting from the model's table, we need to select from the journalized table instead + if source.left == model.arel_table + source.left = model.journal_class.arel_table + + # check if we are also joining the journals table, if not we need to add it + if source.right.none? { |join_source| join_source.kind_of?(Arel::Nodes::Join) && join_source.left == Journal.arel_table } + source.right << Arel::Nodes::StringJoin.new(Arel.sql(journals_join_statement_with_timestamps)) + end + end + + # Check if we are joining the model's table, and if so, it will later be replaced by the journalized table, but as the + # journalized table does not contain the model's ID we need to add another join to the journals table as well + if source.right.any? { |join_source| join_source.kind_of?(Arel::Nodes::Join) && join_source.left == model.arel_table } + source.right.unshift Arel::Nodes::StringJoin.new(Arel.sql(journals_join_statement_with_timestamps_and_members)) + end + + # go through all other joins and modify them as well + source.right.each do |src| + modify_conditions(src) + end + + # all the fields in the select statement need to be modified as well + projections = node.instance_variable_get(:@projections) + projections.each do |projection| + modify_conditions(projection) + end + + # the where's need to be modified as well + node.instance_variable_get(:@wheres).each do |wheres| + modify_conditions(wheres) + end + elsif node.kind_of?(Arel::Nodes::On) + # In an ON node we must not traverse down left and right directly (like other NodeExpressions) but go through the + # expr, which is a NodeExpression itself + [node.expr.left, node.expr.right].each { |child| modify_conditions(child) } + elsif node.kind_of?(Arel::Attributes::Attribute) + # We find an attribute, figure out if it is the model's table + if node.relation == model.arel_table + if node.name == "id" + # ID needs to be pulled from the Journal table + node.relation = Journal.arel_table + node.name = "journable_id" + else + # all other attributes can be pulled from the journalized table + node.relation = model.journal_class.arel_table + end + end + elsif node.kind_of?(Arel::Nodes::Join) + # We found another join, left is the table name, right is the ON condition, if it points to the model's table + # replace it with the journalized table + if node.left == model.arel_table + node.left = model.journal_class.arel_table + end + # Go thorugh the ON condition and figure out if we need to rename attributes in there + modify_conditions(node.right) + elsif node.kind_of? Arel::Nodes::NodeExpression + # Generic case, go through left and right + modify_conditions(node.left) if node.respond_to?(:left) && node.left + modify_conditions(node.right) if node.respond_to?(:right) && node.right + end + + node + end + # Replace table names in sql strings, e.g. # # "work_package.id" => "journals.journable_id" @@ -400,6 +503,38 @@ def timestamp_case_when_statements .join(" ") end + def journals_join_statement_with_timestamps_and_members + journals_join_statement + <<~SQL.squish + AND "journals"."journable_type" = "members"."entity_type" + AND "journals"."journable_id" = "members"."entity_id" + SQL + end + + def journals_join_statement_with_timestamps + statement = <<~SQL.squish + INNER JOIN "journals" ON + "journals"."data_type" = '#{model.journal_class.name}' AND + "journals"."data_id" = "#{model.journal_class.table_name}"."id" + SQL + + additional_conditions = timestamp + .map do |timestamp| + comparison_time = case timestamp + when Timestamp + timestamp.to_time + when DateTime + timestamp.in_time_zone + else + raise NotImplementedError, "Unknown timestamp type: #{timestamp.class}" + end + "\"journals\".\"validity_period\" @> timestamp with time zone '#{comparison_time}'" + end + + return statement if additional_conditions.blank? + + "#{statement} AND (#{additional_conditions.join(' OR ')})" + end + class NotImplementedError < StandardError; end end diff --git a/app/models/project.rb b/app/models/project.rb index 3876f50c21f3..e433196e2b96 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -157,6 +157,9 @@ class Project < ApplicationRecord } scope :public_projects, -> { where(public: true) } scope :visible, ->(user = User.current) { where(id: Project.visible_by(user)) } + scope :with_visible_work_packages, ->(user = User.current) do + where(id: WorkPackage.visible(user).select(:project_id)).or(allowed_to(user, :view_work_packages)) + end scope :newest, -> { order(created_at: :desc) } scope :active, -> { where(active: true) } scope :archived, -> { where(active: false) } @@ -176,7 +179,8 @@ class Project < ApplicationRecord } def visible?(user = User.current) - active? and (public? or user.admin? or user.member_of?(self)) + active? and (public? or user.admin? or user.member_of?(self) or user.allowed_in_any_work_package?(:view_work_packages, + in_project: self)) end def archived? @@ -202,7 +206,7 @@ def self.selectable_projects # to everybody having at least one role in a project regardless of the # role's permissions. def self.visible_by(user = User.current) - allowed_to(user, :view_project) + allowed_to(user, :view_project).or(where(id: WorkPackage.visible(user).select(:project_id))) end # Returns a :conditions SQL string that can be used to find the issues associated with this project. diff --git a/app/models/projects/scopes/allowed_to.rb b/app/models/projects/scopes/allowed_to.rb index 00d7b28c7529..a832621b7c90 100644 --- a/app/models/projects/scopes/allowed_to.rb +++ b/app/models/projects/scopes/allowed_to.rb @@ -29,9 +29,25 @@ module Projects::Scopes module AllowedTo extend ActiveSupport::Concern - include Authorization::Scopes::AllowedTo class_methods do + # Returns an ActiveRecord::Relation to find all entries for which + # +user+ has the given +permission+. + def allowed_to(user, permission) + permissions = allowed_to_permissions(permission) + + return none if user.locked? + return none if permissions.empty? + + if user.admin? && permissions.all?(&:grant_to_admin?) + allowed_to_admin(permissions) + elsif user.anonymous? + allowed_to_anonymous(user, permissions) + else + allowed_to_member(user, permissions) + end + end + private def allowed_to_non_member_relation(user, permissions) @@ -40,15 +56,6 @@ def allowed_to_non_member_relation(user, permissions) .joins(allowed_to_role_permission_join(permissions)) end - def allowed_to_members_condition(user) - members_table = Member.arel_table - - members_table[:project_id].eq(arel_table[:id]) - .and(members_table[:user_id].eq(user.id)) - .and(members_table[:entity_type].eq(nil)) - .and(members_table[:entity_id].eq(nil)) - end - def allowed_to_builtin_roles_in_active_project_join(user) condition = allowed_to_built_roles_in_active_project_condition(user) @@ -80,11 +87,99 @@ def allowed_to_built_roles_in_active_project_condition(user) def allowed_to_no_member_exists_condition(user) Member .select(1) - .where(allowed_to_members_condition(user)) + .where(member_conditions(user)) + .where(Member.arel_table[:project_id].eq(arel_table[:id])) .arel .exists .not end + + def allowed_to_admin(permissions) + where(id: allowed_to_admin_relation(permissions)) + end + + def allowed_to_anonymous(user, permissions) + where(id: allowed_to_non_member_relation(user, permissions)) + end + + def allowed_to_member(user, permissions) + allowed_via_membership = allowed_to_member_relation(user, permissions).select(arel_table[:id]).arel + allowed_via_non_member = allowed_to_non_member_relation(user, permissions).select(:id).arel + + where(arel_table[:id].in(Arel::Nodes::UnionAll.new(allowed_via_membership, allowed_via_non_member))) + end + + def allowed_to_admin_relation(permissions) + joins(allowed_to_enabled_module_join(permissions)) + .where(Project.arel_table[:active].eq(true)) + end + + def allowed_to_member_relation(user, permissions) + Member + .where(member_conditions(user)) + .joins(allowed_to_member_in_active_project_join) + .joins(allowed_to_enabled_module_join(permissions)) + .joins(:roles) + .joins(allowed_to_role_permission_join(permissions)) + end + + def allowed_to_enabled_module_join(permissions) # rubocop:disable Metrics/AbcSize + project_module = permissions.filter_map(&:project_module).uniq + enabled_module_table = EnabledModule.arel_table + projects_table = Project.arel_table + + if project_module.any? + arel_table.join(enabled_module_table, Arel::Nodes::InnerJoin) + .on(projects_table[:id].eq(enabled_module_table[:project_id]) + .and(enabled_module_table[:name].in(project_module)) + .and(projects_table[:active].eq(true))) + .join_sources + end + end + + def allowed_to_role_permission_join(permissions) # rubocop:disable Metrics/AbcSize + return if permissions.all?(&:public?) + + role_permissions_table = RolePermission.arel_table + enabled_modules_table = EnabledModule.arel_table + roles_table = Role.arel_table + + condition = permissions.inject(Arel::Nodes::False.new) do |or_condition, permission| + permission_condition = role_permissions_table[:permission].eq(permission.name) + + if permission.project_module.present? + permission_condition = permission_condition.and(enabled_modules_table[:name].eq(permission.project_module)) + end + + or_condition.or(permission_condition) + end + + arel_table + .join(role_permissions_table, Arel::Nodes::InnerJoin) + .on(roles_table[:id].eq(role_permissions_table[:role_id]) + .and(condition)) + .join_sources + end + + def allowed_to_member_in_active_project_join + Member.arel_table + .join(Project.arel_table) + .on(Project.arel_table[:active].eq(true) + .and(Member.arel_table[:project_id].eq(arel_table[:id]))) + .join_sources + end + + def member_conditions(user) + Member.arel_table[:user_id].eq(user.id) + .and(Member.arel_table[:entity_id].eq(nil)) + .and(Member.arel_table[:entity_type].eq(nil)) + end + + def allowed_to_permissions(permission) + Authorization.contextual_permissions(permission, + to_s.underscore.to_sym, + raise_on_unknown: true) + end end end end diff --git a/app/models/queries/members.rb b/app/models/queries/members.rb index 47ddbce0410f..b3ece3032a89 100644 --- a/app/models/queries/members.rb +++ b/app/models/queries/members.rb @@ -42,6 +42,7 @@ module Queries::Members filter Filters::EntityIdFilter filter Filters::EntityTypeFilter filter Filters::AlsoProjectMemberFilter + filter Filters::OnlyProjectMemberFilter order Orders::DefaultOrder order Orders::NameOrder diff --git a/app/models/queries/members/filters/only_project_member_filter.rb b/app/models/queries/members/filters/only_project_member_filter.rb new file mode 100644 index 000000000000..f84d7090a841 --- /dev/null +++ b/app/models/queries/members/filters/only_project_member_filter.rb @@ -0,0 +1,47 @@ +#-- copyright +# OpenProject is an open source project management software. +# Copyright (C) 2012-2023 the OpenProject GmbH +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License version 3. +# +# OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows: +# Copyright (C) 2006-2013 Jean-Philippe Lang +# Copyright (C) 2010-2013 the ChiliProject Team +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License +# as published by the Free Software Foundation; either version 2 +# of the License, or (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program; if not, write to the Free Software +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. +# +# See COPYRIGHT and LICENSE files for more details. +#++ + +class Queries::Members::Filters::OnlyProjectMemberFilter < Queries::Members::Filters::MemberFilter + include Queries::Filters::Shared::BooleanFilter + + def where + if allowed_values.first.intersect?(values) + "members.entity_type IS NULL AND members.entity_id IS NULL" + else + "1=1" + end + end + + def available_operators + [::Queries::Operators::BooleanEquals] + end + + def type_strategy + @type_strategy ||= ::Queries::Filters::Strategies::BooleanListStrict.new self + end +end diff --git a/app/models/users/permission_checks.rb b/app/models/users/permission_checks.rb index fe0615a98ca6..6225258817fc 100644 --- a/app/models/users/permission_checks.rb +++ b/app/models/users/permission_checks.rb @@ -56,6 +56,15 @@ def allowed(action, project) def allowed_members(action, project) Authorization.users(action, project).where.not(members: { id: nil }) end + + def allowed_members_on_work_package(action, work_package) + project_members = allowed_members(action, work_package.project) + .where(members: { entity: nil }) + work_package_members = allowed_members(action, work_package.project) + .where(members: { entity: work_package }) + + project_members.or(work_package_members) + end end def reload(*args) @@ -100,20 +109,26 @@ def all_permissions_for(context) def allowed_based_on_permission_context?(permission, project: nil, entity: nil) # rubocop:disable Metrics/PerceivedComplexity, Metrics/AbcSize permissions = Authorization.permissions_for(permission, raise_on_unknown: true) + entity_blank_or_not_project_scoped = (entity.blank? || !entity.respond_to?(:project) || (entity.respond_to?(:project) && entity.project.blank?)) + entity_is_work_package_or_list = ((entity.is_a?(WorkPackage) && entity.persisted?) || (entity.is_a?(Enumerable) && entity.all?(WorkPackage))) + entity_is_project_scoped_and_project_is_present = (entity.respond_to?(:project) && entity.project.present?) + permissions.any? do |perm| if perm.global? allowed_globally?(perm) - elsif perm.work_package? && (entity.is_a?(WorkPackage) || (entity.is_a?(Array) && entity.all? { |e| e.is_a?(WorkPackage) })) + elsif perm.work_package? && entity_is_work_package_or_list allowed_in_work_package?(perm, entity) elsif perm.work_package? && entity.blank? && project.blank? allowed_in_any_work_package?(perm) - elsif perm.work_package? && project && entity.blank? + elsif perm.work_package? && entity && entity.new_record? && entity.respond_to?(:project) + allowed_in_any_work_package?(perm, in_project: entity.project) + elsif perm.work_package? && project && (entity.blank? || entity.new_record?) allowed_in_any_work_package?(perm, in_project: project) elsif perm.project? && project allowed_in_project?(perm, project) - elsif perm.project? && entity && entity.respond_to?(:project) + elsif perm.project? && project.nil? && entity.present? && entity_is_project_scoped_and_project_is_present allowed_in_project?(perm, entity.project) - elsif perm.project? && entity.blank? && project.blank? + elsif perm.project? && entity_blank_or_not_project_scoped && project.blank? allowed_in_any_project?(perm) else false diff --git a/app/models/work_package.rb b/app/models/work_package.rb index c8dfd94a88bf..625cf3aa7ba4 100644 --- a/app/models/work_package.rb +++ b/app/models/work_package.rb @@ -79,9 +79,7 @@ class WorkPackage < ApplicationRecord order(updated_at: :desc) } - scope :visible, ->(*args) { - where(project_id: Project.allowed_to(args.first || User.current, :view_work_packages)) - } + scope :visible, ->(user = User.current) { allowed_to(user, :view_work_packages) } scope :in_status, ->(*args) do where(status_id: (args.first.respond_to?(:id) ? args.first.id : args.first)) @@ -222,7 +220,7 @@ def self.use_field_for_done_ratio? # Returns true if usr or current user is allowed to view the work_package def visible?(usr = User.current) - usr.allowed_in_project?(:view_work_packages, project) + usr.allowed_in_work_package?(:view_work_packages, self) end # RELATIONS @@ -361,7 +359,7 @@ def update_done_ratio_from_status # see Acts::Journalized::Permissions#journal_editable_by def journal_editable_by?(journal, user) user.allowed_in_project?(:edit_work_package_notes, project) || - (user.allowed_in_project?(:edit_own_work_package_notes, project) && journal.user_id == user.id) + (user.allowed_in_work_package?(:edit_own_work_package_notes, self) && journal.user_id == user.id) end # Returns a scope for the projects diff --git a/app/models/work_packages/scopes/allowed_to.rb b/app/models/work_packages/scopes/allowed_to.rb index 4f130d346e6c..5374a2a82546 100644 --- a/app/models/work_packages/scopes/allowed_to.rb +++ b/app/models/work_packages/scopes/allowed_to.rb @@ -29,42 +29,108 @@ module WorkPackages::Scopes module AllowedTo extend ActiveSupport::Concern - include Authorization::Scopes::AllowedTo class_methods do - private + # Returns an ActiveRecord::Relation to find all work packages for which + # +user+ has the given +permission+ either directly on the work package + # or by the linked project + def allowed_to(user, permission) # rubocop:disable Metrics/AbcSize + permissions = Authorization.contextual_permissions(permission, :work_package, raise_on_unknown: true) + + return none if user.locked? + return none if permissions.empty? + + if user.admin? && permissions.all?(&:grant_to_admin?) + where(id: allowed_to_admin_relation(permissions)) + elsif user.anonymous? + where(project_id: Project.allowed_to(user, permissions)) + else + allowed_via_wp_membership = allowed_to_member_relation(user, permissions).select(arel_table[:id]).arel + allowed_via_project_membership = unscoped.where(project_id: Project.unscoped.allowed_to(user, permissions).select(:id)) + .select(arel_table[:id]) + .arel - def allowed_to_anonymous(user, permissions) - where(project_id: Project.allowed_to(user, permissions).select(:id)) + where(arel_table[:id].in(Arel::Nodes::UnionAll.new(allowed_via_wp_membership, allowed_via_project_membership))) + end end - alias_method :allowed_to_non_member_relation, :allowed_to_anonymous + private def allowed_to_admin_relation(permissions) - joins(:project) - .joins(allowed_to_enabled_module_join(permissions)) + unscoped + .joins(:project) + .joins(allowed_to_enabled_module_join(permissions)) .where(Project.arel_table[:active].eq(true)) end - def allowed_to_member_relation(user, permission) - super + def allowed_to_member_relation(user, permissions) + Member .joins(allowed_to_member_in_work_package_join) + .joins(active_project_join) + .joins(allowed_to_enabled_module_join(permissions)) + .joins(member_roles: :role) + .joins(allowed_to_role_permission_join(permissions)) + .where(member_conditions(user)) + .select(arel_table[:id]) end - def allowed_to_members_condition(user) - members_table = Member.arel_table + def allowed_to_enabled_module_join(permissions) # rubocop:disable Metrics/AbcSize + project_module = permissions.filter_map(&:project_module).uniq + enabled_module_table = EnabledModule.arel_table + projects_table = Project.arel_table - members_table[:project_id].eq(arel_table[:project_id]) - .and(members_table[:user_id].eq(user.id)) - .and(members_table[:entity_type].eq(model_name.name)) + if project_module.any? + arel_table.join(enabled_module_table, Arel::Nodes::InnerJoin) + .on(projects_table[:id].eq(enabled_module_table[:project_id]) + .and(enabled_module_table[:name].in(project_module)) + .and(projects_table[:active].eq(true))) + .join_sources + end + end + + def allowed_to_role_permission_join(permissions) # rubocop:disable Metrics/AbcSize + return if permissions.all?(&:public?) + + role_permissions_table = RolePermission.arel_table + enabled_modules_table = EnabledModule.arel_table + roles_table = Role.arel_table + + condition = permissions.inject(Arel::Nodes::False.new) do |or_condition, permission| + permission_condition = role_permissions_table[:permission].eq(permission.name) + + if permission.project_module.present? + permission_condition = permission_condition.and(enabled_modules_table[:name].eq(permission.project_module)) + end + + or_condition.or(permission_condition) + end + + arel_table + .join(role_permissions_table, Arel::Nodes::InnerJoin) + .on(roles_table[:id].eq(role_permissions_table[:role_id]) + .and(condition)) + .join_sources + end + + def active_project_join + projects_table = Project.arel_table + arel_table + .join(projects_table) + .on(projects_table[:active].eq(true) + .and(projects_table[:id].eq(arel_table[:project_id]))) + .join_sources end def allowed_to_member_in_work_package_join members_table = Member.arel_table - arel_table.join(arel_table) - .on(members_table[:entity_id].eq(arel_table[:id]).and(members_table[:entity_type].eq(model_name.name))) - .join_sources + .on(members_table[:entity_id].eq(arel_table[:id])) + .join_sources + end + + def member_conditions(user) + Member.arel_table[:user_id].eq(user.id) + .and(Member.arel_table[:entity_type].eq(model_name.name)) end end end diff --git a/app/policies/query_policy.rb b/app/policies/query_policy.rb index 2003b3ea4af1..c2441d62dd06 100644 --- a/app/policies/query_policy.rb +++ b/app/policies/query_policy.rb @@ -90,50 +90,38 @@ def reorder_work_packages?(query) end def view_work_packages_allowed?(query) - @view_work_packages_cache ||= Hash.new do |hash, project| - hash[project] = allowed_in_project_or_any_project?(:view_work_packages, project) + if query.project + user.allowed_in_any_work_package?(:view_work_packages, in_project: query.project) + else + user.allowed_in_any_work_package?(:view_work_packages) end - - @view_work_packages_cache[query.project] end def edit_work_packages_allowed?(query) - @edit_work_packages_cache ||= Hash.new do |hash, project| - hash[project] = allowed_in_project_or_any_project?(:edit_work_packages, project) - end - - @edit_work_packages_cache[query.project] + user.allowed_in_any_work_package?(:edit_work_packages, in_project: query.project) end def save_queries_allowed?(query) - @save_queries_cache ||= Hash.new do |hash, project| - hash[project] = allowed_in_project_or_any_project?(:save_queries, project) + if query.project + user.allowed_in_project?(:save_queries, query.project) + else + user.allowed_in_any_project?(:save_queries) end - - @save_queries_cache[query.project] end def manage_public_queries_allowed?(query) - @manage_public_queries_cache ||= Hash.new do |hash, project| - hash[project] = allowed_in_project_or_any_project?(:manage_public_queries, project) + if query.project + user.allowed_in_project?(:manage_public_queries, query.project) + else + user.allowed_in_any_project?(:manage_public_queries) end - - @manage_public_queries_cache[query.project] end def share_via_ical_allowed?(query) - @share_via_ical_cache ||= Hash.new do |hash, project| - hash[project] = allowed_in_project_or_any_project?(:share_calendars, project) - end - - @share_via_ical_cache[query.project] - end - - def allowed_in_project_or_any_project?(permission, project) - if project - user.allowed_in_project?(permission, project) + if query.project + user.allowed_in_project?(:share_calendars, query.project) else - user.allowed_in_any_project?(permission) + user.allowed_in_any_project?(:share_calendars) end end end diff --git a/app/policies/work_package_policy.rb b/app/policies/work_package_policy.rb index b7eccc59b6ef..3e5bcaf919f5 100644 --- a/app/policies/work_package_policy.rb +++ b/app/policies/work_package_policy.rb @@ -56,19 +56,11 @@ def allowed_hash(work_package) end def edit_allowed?(work_package) - @edit_cache ||= Hash.new do |hash, project| - hash[project] = work_package.persisted? && user.allowed_in_project?(:edit_work_packages, project) - end - - @edit_cache[work_package.project] + work_package.persisted? && user.allowed_in_work_package?(:edit_work_packages, work_package) end def move_allowed?(work_package) - @move_cache ||= Hash.new do |hash, project| - hash[project] = user.allowed_in_project?(:move_work_packages, project) - end - - @move_cache[work_package.project] + user.allowed_in_project?(:move_work_packages, work_package.project) end def copy_allowed?(work_package) @@ -76,19 +68,11 @@ def copy_allowed?(work_package) end def delete_allowed?(work_package) - @delete_cache ||= Hash.new do |hash, project| - hash[project] = user.allowed_in_project?(:delete_work_packages, project) - end - - @delete_cache[work_package.project] + user.allowed_in_project?(:delete_work_packages, work_package.project) end def add_allowed?(work_package) - @add_cache ||= Hash.new do |hash, project| - hash[project] = user.allowed_in_project?(:add_work_packages, project) - end - - @add_cache[work_package.project] + user.allowed_in_project?(:add_work_packages, work_package.project) end def type_active_in_project?(work_package) @@ -102,26 +86,14 @@ def type_active_in_project?(work_package) end def manage_subtasks_allowed?(work_package) - @manage_subtasks_cache ||= Hash.new do |hash, project| - hash[project] = user.allowed_in_project?(:manage_subtasks, project) - end - - @manage_subtasks_cache[work_package.project] + user.allowed_in_project?(:manage_subtasks, work_package.project) end def comment_allowed?(work_package) - @comment_cache ||= Hash.new do |hash, project| - hash[project] = user.allowed_in_project?(:add_work_package_notes, project) || edit_allowed?(work_package) - end - - @comment_cache[work_package.project] + user.allowed_in_work_package?(:add_work_package_notes, work_package) || edit_allowed?(work_package) end def assign_version_allowed?(work_package) - @assign_version_cache ||= Hash.new do |hash, project| - hash[project] = user.allowed_in_project?(:assign_versions, project) - end - - @assign_version_cache[work_package.project] + user.allowed_in_project?(:assign_versions, work_package.project) end end diff --git a/app/services/authorization/user_permissible_service.rb b/app/services/authorization/user_permissible_service.rb index e158da8531d8..127b023731a9 100644 --- a/app/services/authorization/user_permissible_service.rb +++ b/app/services/authorization/user_permissible_service.rb @@ -53,7 +53,7 @@ def allowed_in_any_entity?(permission, entity_class, in_project: nil) return true if admin_and_all_granted_to_admin?(perms) # entity_class.allowed_to will also check whether the user has the permission via a membership in the project. - # TODO: ^--- thats' actually not the case weirdly... + # ^-- still a problem in some cases allowed_scope = entity_class.allowed_to(user, perms) if in_project @@ -87,6 +87,7 @@ def allowed_in_single_project?(permissions, project) def allowed_in_single_entity?(permissions, entity) return false if entity.nil? + return false if entity.project.nil? return false unless entity.project.active? || entity.project.being_archived? permissions_filtered_for_project = permissions_by_enabled_project_modules(entity.project, permissions) diff --git a/lib/api/root_api.rb b/lib/api/root_api.rb index f2a8d63d33a8..312fd51a2e56 100644 --- a/lib/api/root_api.rb +++ b/lib/api/root_api.rb @@ -193,6 +193,40 @@ def authorize_in_any_project(permission_or_permissions, user: current_user, &blo authorize_by_with_raise(authorized, &block) end + # Checks that the current user has the given permission on any work package or project or raise {API::Errors::Unauthorized}. + # + # @param permission_or_permissions [String, [String], Hash] the permission name, an array of permissions or a hash + # with controller and action keys. When an array of permissions is given, the user needs to have at least one of + # those permissions, not all. + # + # @raise [API::Errors::Unauthorized] when permission is not met + def authorize_in_any_work_package(permission_or_permissions, user: current_user, in_project: nil, &block) + permissions = Array.wrap(permission_or_permissions) + authorized = permissions.any? do |permission| + user.allowed_in_any_work_package?(permission, in_project:) + end + + authorize_by_with_raise(authorized, &block) + end + + # Checks that the current user has the given permission on the given work package or raise {API::Errors::Unauthorized}. + # + # @param permission_or_permissions [String, [String], Hash] the permission name, an array of permissions or a hash + # with controller and action keys. When an array of permissions is given, the user needs to have at least one of + # those permissions, not all. + # + # @param work_package [Project] the work package the permission needs to be checked on + # + # @raise [API::Errors::Unauthorized] when permission is not met + def authorize_in_work_package(permission_or_permissions, work_package:, user: current_user, &block) + permissions = Array.wrap(permission_or_permissions) + authorized = permissions.any? do |permission| + user.allowed_in_work_package?(permission, work_package) + end + + authorize_by_with_raise(authorized, &block) + end + # Checks that the current user has the given permission globally or raise {API::Errors::Unauthorized}. # # @param permission_or_permissions [String, [String], Hash] the permission name, an array of permissions or a hash diff --git a/lib/api/v3/activities/activities_by_work_package_api.rb b/lib/api/v3/activities/activities_by_work_package_api.rb index 92840037737f..5dd5f182e5ee 100644 --- a/lib/api/v3/activities/activities_by_work_package_api.rb +++ b/lib/api/v3/activities/activities_by_work_package_api.rb @@ -50,7 +50,7 @@ class ActivitiesByWorkPackageAPI < ::API::OpenProjectAPI requires :comment, type: Hash end post do - authorize_in_project({ controller: :journals, action: :new }, project: @work_package.project) do + authorize_in_work_package({ controller: :journals, action: :new }, work_package: @work_package) do raise ::API::Errors::NotFound.new end diff --git a/lib/api/v3/attachments/attachments_by_container_api.rb b/lib/api/v3/attachments/attachments_by_container_api.rb index 7afd6a54a6fe..c7464272deba 100644 --- a/lib/api/v3/attachments/attachments_by_container_api.rb +++ b/lib/api/v3/attachments/attachments_by_container_api.rb @@ -51,7 +51,13 @@ def post_request? # Additionally to what would be checked by the contract, # we need to restrict permissions in some use cases of the mounts of this endpoint. def restrict_permissions(permissions) - authorize_in_project(permissions, project: container.project) unless permissions.empty? + return if permissions.empty? + + if container.is_a?(WorkPackage) + authorize_in_work_package(permissions, work_package: container) + else + authorize_in_project(permissions, project: container.project) + end end end diff --git a/lib/api/v3/custom_actions/custom_actions_api.rb b/lib/api/v3/custom_actions/custom_actions_api.rb index e5b56232d4f6..8bff2f26dbca 100644 --- a/lib/api/v3/custom_actions/custom_actions_api.rb +++ b/lib/api/v3/custom_actions/custom_actions_api.rb @@ -41,7 +41,7 @@ def custom_action helpers ::API::V3::WorkPackages::WorkPackagesSharedHelpers after_validation do - authorize_in_any_project(:edit_work_packages) + authorize_in_any_work_package(:edit_work_packages) end get do diff --git a/lib/api/v3/custom_options/custom_options_api.rb b/lib/api/v3/custom_options/custom_options_api.rb index dfedcf7973a1..f77cae7738e0 100644 --- a/lib/api/v3/custom_options/custom_options_api.rb +++ b/lib/api/v3/custom_options/custom_options_api.rb @@ -44,7 +44,11 @@ def authorize_custom_option_visibility(custom_option) when ProjectCustomField authorize_in_any_project(%i[view_project]) { raise API::Errors::NotFound } when TimeEntryCustomField - authorize_in_any_project(%i[log_time log_own_time]) { raise API::Errors::NotFound } + authorize_in_any_work_package(:log_own_time) do + authorize_in_any_project(:log_time) do + raise API::Errors::NotFound + end + end when UserCustomField, GroupCustomField true else @@ -54,7 +58,7 @@ def authorize_custom_option_visibility(custom_option) def authorized_work_package_option(custom_option) allowed = Project - .allowed_to(current_user, :view_work_packages) + .with_visible_work_packages(current_user) .joins(:work_package_custom_fields) .exists?(custom_fields: { id: custom_option.custom_field_id }) diff --git a/lib/api/v3/priorities/priorities_api.rb b/lib/api/v3/priorities/priorities_api.rb index 7145cf4686c8..1300397ecc4d 100644 --- a/lib/api/v3/priorities/priorities_api.rb +++ b/lib/api/v3/priorities/priorities_api.rb @@ -35,7 +35,7 @@ module Priorities class PrioritiesAPI < ::API::OpenProjectAPI resources :priorities do after_validation do - authorize_in_any_project(:view_work_packages) + authorize_in_any_work_package(:view_work_packages) @priorities = IssuePriority.all end diff --git a/lib/api/v3/projects/available_assignees_api.rb b/lib/api/v3/projects/available_assignees_api.rb index fbe4bb3fceb2..305911ccc8a7 100644 --- a/lib/api/v3/projects/available_assignees_api.rb +++ b/lib/api/v3/projects/available_assignees_api.rb @@ -34,7 +34,7 @@ module Projects class AvailableAssigneesAPI < ::API::OpenProjectAPI resource :available_assignees do after_validation do - authorize_in_any_project(:view_work_packages) + authorize_in_any_work_package(:view_work_packages) end get &::API::V3::Utilities::Endpoints::Index.new(model: Principal, diff --git a/lib/api/v3/projects/available_responsibles_api.rb b/lib/api/v3/projects/available_responsibles_api.rb index 3db9015b41d0..e5e12b7869b8 100644 --- a/lib/api/v3/projects/available_responsibles_api.rb +++ b/lib/api/v3/projects/available_responsibles_api.rb @@ -34,7 +34,7 @@ module Projects class AvailableResponsiblesAPI < ::API::OpenProjectAPI resource :available_responsibles do after_validation do - authorize_in_any_project(:view_work_packages) + authorize_in_any_work_package(:view_work_packages) end get &::API::V3::Utilities::Endpoints::Index.new(model: Principal, diff --git a/lib/api/v3/queries/columns/query_columns_api.rb b/lib/api/v3/queries/columns/query_columns_api.rb index 3d73db684a8c..6b4cfaecd788 100644 --- a/lib/api/v3/queries/columns/query_columns_api.rb +++ b/lib/api/v3/queries/columns/query_columns_api.rb @@ -39,7 +39,7 @@ def convert_to_ar(attribute) end after_validation do - authorize_in_any_project(:view_work_packages) + authorize_in_any_work_package(:view_work_packages) end route_param :id, type: String, regexp: /\A\w+\z/, desc: 'Column ID' do diff --git a/lib/api/v3/queries/filters/query_filters_api.rb b/lib/api/v3/queries/filters/query_filters_api.rb index 172779b15023..4cde3952a82a 100644 --- a/lib/api/v3/queries/filters/query_filters_api.rb +++ b/lib/api/v3/queries/filters/query_filters_api.rb @@ -40,7 +40,7 @@ def convert_to_ar(attribute) end after_validation do - authorize_in_any_project(:view_work_packages) + authorize_in_any_work_package(:view_work_packages) end route_param :id, type: String, regexp: /\A\w+\z/, desc: 'Filter ID' do diff --git a/lib/api/v3/queries/group_bys/query_group_bys_api.rb b/lib/api/v3/queries/group_bys/query_group_bys_api.rb index 1e9b73180f0f..4ac2ed833bbb 100644 --- a/lib/api/v3/queries/group_bys/query_group_bys_api.rb +++ b/lib/api/v3/queries/group_bys/query_group_bys_api.rb @@ -39,7 +39,7 @@ def convert_to_ar(attribute) end after_validation do - authorize_in_any_project(:view_work_packages) + authorize_in_any_work_package(:view_work_packages) end route_param :id, type: String, regexp: /\A\w+\z/, desc: 'Group by ID' do diff --git a/lib/api/v3/queries/operators/query_operators_api.rb b/lib/api/v3/queries/operators/query_operators_api.rb index 0d3d315b3774..f8214a4e6a11 100644 --- a/lib/api/v3/queries/operators/query_operators_api.rb +++ b/lib/api/v3/queries/operators/query_operators_api.rb @@ -33,7 +33,7 @@ module Operators class QueryOperatorsAPI < ::API::OpenProjectAPI resource :operators do after_validation do - authorize_in_any_project(:view_work_packages) + authorize_in_any_work_package(:view_work_packages) end route_param :id, type: String, regexp: /\A\w+\z/, desc: 'Operator ID' do diff --git a/lib/api/v3/queries/queries_api.rb b/lib/api/v3/queries/queries_api.rb index 9f7cbf8da4ff..df0e5022fa31 100644 --- a/lib/api/v3/queries/queries_api.rb +++ b/lib/api/v3/queries/queries_api.rb @@ -57,7 +57,9 @@ def allowed_to?(action) end get do - authorize_in_any_project(%i(view_work_packages manage_public_queries)) + authorize_in_any_work_package(:view_work_packages) do + authorize_in_any_project(:manage_public_queries) + end queries_scope = Query.all.includes(QueryRepresenter.to_eager_load) @@ -68,11 +70,11 @@ def allowed_to?(action) namespace 'available_projects' do after_validation do - authorize_in_any_project(:view_work_packages) + authorize_in_any_work_package(:view_work_packages) end get do - available_projects = Project.allowed_to(current_user, :view_work_packages) + available_projects = Project.with_visible_work_packages self_link = api_v3_paths.query_available_projects ::API::V3::Projects::ProjectCollectionRepresenter.new(available_projects, @@ -89,7 +91,7 @@ def allowed_to?(action) get do @query = Query.new_default(user: current_user) - authorize_by_policy(:show) + authorize_in_any_work_package(:view_work_packages) query_representer_response(@query, params, params.delete(:valid_subset)) end diff --git a/lib/api/v3/queries/queries_by_project_api.rb b/lib/api/v3/queries/queries_by_project_api.rb index 4bd735cac559..15e6141c25fc 100644 --- a/lib/api/v3/queries/queries_by_project_api.rb +++ b/lib/api/v3/queries/queries_by_project_api.rb @@ -34,7 +34,7 @@ class QueriesByProjectAPI < ::API::OpenProjectAPI helpers ::API::V3::Queries::Helpers::QueryRepresenterResponse after_validation do - authorize_in_project(:view_work_packages, project: @project) + authorize_in_any_work_package(:view_work_packages, in_project: @project) end mount API::V3::Queries::Schemas::QueryProjectFilterInstanceSchemaAPI diff --git a/lib/api/v3/queries/schemas/query_filter_instance_schema_api.rb b/lib/api/v3/queries/schemas/query_filter_instance_schema_api.rb index e25f12a886ea..2c7859d19072 100644 --- a/lib/api/v3/queries/schemas/query_filter_instance_schema_api.rb +++ b/lib/api/v3/queries/schemas/query_filter_instance_schema_api.rb @@ -43,7 +43,7 @@ def single_representer end after_validation do - authorize_in_any_project(:view_work_packages) + authorize_in_any_work_package(:view_work_packages) end get do diff --git a/lib/api/v3/queries/schemas/query_schema_api.rb b/lib/api/v3/queries/schemas/query_schema_api.rb index aa01a21edb01..3d46604e0963 100644 --- a/lib/api/v3/queries/schemas/query_schema_api.rb +++ b/lib/api/v3/queries/schemas/query_schema_api.rb @@ -39,7 +39,7 @@ def representer end after_validation do - authorize_in_any_project(:view_work_packages) + authorize_in_any_work_package(:view_work_packages) end get do diff --git a/lib/api/v3/queries/sort_bys/query_sort_bys_api.rb b/lib/api/v3/queries/sort_bys/query_sort_bys_api.rb index cf1c49f407ec..e8a0031da33a 100644 --- a/lib/api/v3/queries/sort_bys/query_sort_bys_api.rb +++ b/lib/api/v3/queries/sort_bys/query_sort_bys_api.rb @@ -52,7 +52,7 @@ def find_column(attribute) end after_validation do - authorize_in_any_project(:view_work_packages) + authorize_in_any_work_package(:view_work_packages) end namespace ':id-:direction' do diff --git a/lib/api/v3/relations/relation_representer.rb b/lib/api/v3/relations/relation_representer.rb index e476cdbdcdf9..6fd90caba81d 100644 --- a/lib/api/v3/relations/relation_representer.rb +++ b/lib/api/v3/relations/relation_representer.rb @@ -105,7 +105,7 @@ def reverse_type=(reverse_type) end def manage_relations? - current_user.allowed_in_project?(:manage_work_package_relations, represented.from.project) + current_user.allowed_in_work_package?(:manage_work_package_relations, represented.from) end self.to_eager_load = [:to, diff --git a/lib/api/v3/repositories/revisions_by_work_package_api.rb b/lib/api/v3/repositories/revisions_by_work_package_api.rb index 8bdfded6d9c6..270e9ef4eb63 100644 --- a/lib/api/v3/repositories/revisions_by_work_package_api.rb +++ b/lib/api/v3/repositories/revisions_by_work_package_api.rb @@ -34,7 +34,7 @@ module Repositories class RevisionsByWorkPackageAPI < ::API::OpenProjectAPI resources :revisions do after_validation do - authorize_in_project(:view_work_packages, project: work_package.project) + authorize_in_work_package(:view_work_packages, work_package:) end get do diff --git a/lib/api/v3/statuses/statuses_api.rb b/lib/api/v3/statuses/statuses_api.rb index 1f83c79cde3b..721f5c80ff9d 100644 --- a/lib/api/v3/statuses/statuses_api.rb +++ b/lib/api/v3/statuses/statuses_api.rb @@ -35,7 +35,7 @@ module Statuses class StatusesAPI < ::API::OpenProjectAPI resources :statuses do after_validation do - authorize_in_any_project(:view_work_packages) + authorize_in_any_work_package(:view_work_packages) end get do diff --git a/lib/api/v3/versions/schemas/version_schema_api.rb b/lib/api/v3/versions/schemas/version_schema_api.rb index 891eb36d21d8..6f8615e5fad6 100644 --- a/lib/api/v3/versions/schemas/version_schema_api.rb +++ b/lib/api/v3/versions/schemas/version_schema_api.rb @@ -33,7 +33,9 @@ module Schemas class VersionSchemaAPI < ::API::OpenProjectAPI resources :schema do before do - authorize_in_any_project(%i[manage_versions view_work_packages]) + authorize_in_any_work_package(:view_work_packages) do + authorize_in_any_project(:manage_versions) + end end get &::API::V3::Utilities::Endpoints::Schema.new(model: Version).mount diff --git a/lib/api/v3/work_packages/available_projects_on_edit_api.rb b/lib/api/v3/work_packages/available_projects_on_edit_api.rb index c564342bae4d..5bae8f90aa34 100644 --- a/lib/api/v3/work_packages/available_projects_on_edit_api.rb +++ b/lib/api/v3/work_packages/available_projects_on_edit_api.rb @@ -32,7 +32,7 @@ module WorkPackages class AvailableProjectsOnEditAPI < ::API::OpenProjectAPI resource :available_projects do after_validation do - authorize_in_project(:edit_work_packages, project: @work_package.project) + authorize_in_work_package(:edit_work_packages, work_package: @work_package) checked_permissions = Projects::ProjectCollectionRepresenter.checked_permissions current_user.preload_projects_allowed_to(checked_permissions) diff --git a/lib/api/v3/work_packages/create_project_form_representer.rb b/lib/api/v3/work_packages/create_project_form_representer.rb index f0f159739499..1b308839b77e 100644 --- a/lib/api/v3/work_packages/create_project_form_representer.rb +++ b/lib/api/v3/work_packages/create_project_form_representer.rb @@ -52,7 +52,7 @@ class CreateProjectFormRepresenter < FormRepresenter end link :commit do - if current_user.allowed_in_project?(:edit_work_packages, represented.project) && + if current_user.allowed_in_work_package?(:edit_work_packages, represented) && @errors.empty? { href: api_v3_paths.work_packages_by_project(represented.project_id), diff --git a/lib/api/v3/work_packages/schema/work_package_schema_representer.rb b/lib/api/v3/work_packages/schema/work_package_schema_representer.rb index 4d60ef31cbea..071dc445cad6 100644 --- a/lib/api/v3/work_packages/schema/work_package_schema_representer.rb +++ b/lib/api/v3/work_packages/schema/work_package_schema_representer.rb @@ -39,8 +39,7 @@ class WorkPackageSchemaRepresenter < ::API::Decorators::SchemaRepresenter include API::Caching::CachedRepresenter cached_representer key_parts: %i[project type], dependencies: -> { - User.current.roles_for_project(represented.project).map(&:permissions).flatten.uniq.sort + - [Setting.work_package_done_ratio] + all_permissions_granted_to_user_under_project + [Setting.work_package_done_ratio] } custom_field_injector type: :schema_representer @@ -170,7 +169,7 @@ def initialize(schema, self_link:, **context) required: false, show_if: ->(*) { current_user.allowed_in_project?(:view_time_entries, represented.project) || - current_user.allowed_in_project?(:view_own_time_entries, represented.project) + current_user.allowed_in_work_package?(:view_own_time_entries, represented) } schema :percentage_done, @@ -368,6 +367,16 @@ def form_config_attribute_cache_key(group) .flatten .compact end + + def all_permissions_granted_to_user_under_project + Role + .joins(:members) + .where(members: { project_id: represented.project, principal: User.current }) + .map(&:permissions) + .flatten + .uniq + .sort + end end end end diff --git a/lib/api/v3/work_packages/schema/work_package_schemas_api.rb b/lib/api/v3/work_packages/schema/work_package_schemas_api.rb index 205d2c01445f..e37f24eb72f4 100644 --- a/lib/api/v3/work_packages/schema/work_package_schemas_api.rb +++ b/lib/api/v3/work_packages/schema/work_package_schemas_api.rb @@ -73,7 +73,7 @@ def schemas_path_with_filters_params end get do - authorize_in_any_project(:view_work_packages) + authorize_in_any_work_package(:view_work_packages) project_type_pairs = parse_filter_for_project_type_pairs @@ -104,7 +104,7 @@ def schemas_path_with_filters_params raise404 end - authorize_in_project(:view_work_packages, project: @project) do + authorize_in_any_work_package(:view_work_packages, in_project: @project) do raise404 end end @@ -124,7 +124,7 @@ def schemas_path_with_filters_params namespace 'sums' do get do - authorize_in_any_project(:view_work_packages) do + authorize_in_any_work_package(:view_work_packages) do raise404 end diff --git a/lib/api/v3/work_packages/update_form_representer.rb b/lib/api/v3/work_packages/update_form_representer.rb index 764e10960eec..1421bbd9c8c3 100644 --- a/lib/api/v3/work_packages/update_form_representer.rb +++ b/lib/api/v3/work_packages/update_form_representer.rb @@ -52,7 +52,7 @@ class UpdateFormRepresenter < FormRepresenter end link :commit do - if current_user.allowed_in_project?(:edit_work_packages, represented.project) && + if current_user.allowed_in_work_package?(:edit_work_packages, represented) && @errors.empty? { href: api_v3_paths.work_package(represented.id), diff --git a/lib/api/v3/work_packages/watchers_api.rb b/lib/api/v3/work_packages/watchers_api.rb index 6d5dbc4da85b..71c9117203e2 100644 --- a/lib/api/v3/work_packages/watchers_api.rb +++ b/lib/api/v3/work_packages/watchers_api.rb @@ -72,7 +72,7 @@ def watchers_collection user_id = representer.represented.user_id.to_i if current_user.id == user_id - authorize_in_project(:view_work_packages, project: @work_package.project) + authorize_in_work_package(:view_work_packages, work_package: @work_package) else authorize_in_project(:add_work_package_watchers, project: @work_package.project) end @@ -96,7 +96,7 @@ def watchers_collection delete do if current_user.id == params[:user_id] - authorize_in_project(:view_work_packages, project: @work_package.project) + authorize_in_work_package(:view_work_packages, work_package: @work_package) else authorize_in_project(:delete_work_package_watchers, project: @work_package.project) end diff --git a/lib/api/v3/work_packages/work_package_collection_representer.rb b/lib/api/v3/work_packages/work_package_collection_representer.rb index 95242fed8c30..f60b35636a5f 100644 --- a/lib/api/v3/work_packages/work_package_collection_representer.rb +++ b/lib/api/v3/work_packages/work_package_collection_representer.rb @@ -135,8 +135,7 @@ def initialize(models, end links :representations do - if (project && current_user.allowed_in_project?(:export_work_packages, project)) || - (!project && current_user.allowed_in_any_project?(:export_work_packages)) + if current_user.allowed_in_any_work_package?(:export_work_packages, in_project: project) representation_formats end end @@ -186,11 +185,7 @@ def current_user_allowed_to_add_work_packages? end def current_user_allowed_to_edit_work_packages? - if project - current_user.allowed_in_project?(:edit_work_packages, project) - else - current_user.allowed_in_any_project?(:edit_work_packages) - end + current_user.allowed_in_any_work_package?(:edit_work_packages, in_project: project) end def schemas diff --git a/lib/api/v3/work_packages/work_package_representer.rb b/lib/api/v3/work_packages/work_package_representer.rb index 608e7809a6e8..44c7cb1d92aa 100644 --- a/lib/api/v3/work_packages/work_package_representer.rb +++ b/lib/api/v3/work_packages/work_package_representer.rb @@ -248,7 +248,7 @@ def self_v3_path(*) end link :addRelation, - cache_if: -> { current_user.allowed_in_project?(:manage_work_package_relations, represented.project) } do + cache_if: -> { current_user.allowed_in_work_package?(:manage_work_package_relations, represented) } do { href: api_v3_paths.work_package_relations(represented.id), method: :post, @@ -277,7 +277,7 @@ def self_v3_path(*) end link :addComment, - cache_if: -> { current_user.allowed_in_project?(:add_work_package_notes, represented.project) } do + cache_if: -> { current_user.allowed_in_work_package?(:add_work_package_notes, represented) } do { href: api_v3_paths.work_package_activities(represented.id), method: :post, @@ -566,20 +566,20 @@ def current_user_watcher? def current_user_update_allowed? @current_user_update_allowed ||= - current_user.allowed_in_project?(:edit_work_packages, represented.project) || + current_user.allowed_in_work_package?(:edit_work_packages, represented) || current_user.allowed_in_project?(:assign_versions, represented.project) end def view_time_entries_allowed? @view_time_entries_allowed ||= current_user.allowed_in_project?(:view_time_entries, represented.project) || - current_user.allowed_in_project?(:view_own_time_entries, represented.project) + current_user.allowed_in_work_package?(:view_own_time_entries, represented) end def log_time_allowed? @log_time_allowed ||= current_user.allowed_in_project?(:log_time, represented.project) || - current_user.allowed_in_project?(:log_own_time, represented.project) + current_user.allowed_in_work_package?(:log_own_time, represented) end def view_budgets_allowed? @@ -588,7 +588,7 @@ def view_budgets_allowed? def export_work_packages_allowed? @export_work_packages_allowed ||= - current_user.allowed_in_project?(:export_work_packages, represented.project) + current_user.allowed_in_work_package?(:export_work_packages, represented) end def add_work_packages_allowed? diff --git a/lib/api/v3/work_packages/work_packages_api.rb b/lib/api/v3/work_packages/work_packages_api.rb index 1fb24d9ea9e6..3001c6d9bf1b 100644 --- a/lib/api/v3/work_packages/work_packages_api.rb +++ b/lib/api/v3/work_packages/work_packages_api.rb @@ -42,7 +42,7 @@ class WorkPackagesAPI < ::API::OpenProjectAPI mount ::API::V3::WorkPackages::Schema::WorkPackageSchemasAPI get do - authorize_in_any_project(:view_work_packages) + authorize_in_any_work_package(:view_work_packages) call = raise_invalid_query_on_service_failure do WorkPackageCollectionFromQueryParamsService @@ -71,7 +71,7 @@ class WorkPackagesAPI < ::API::OpenProjectAPI after_validation do @work_package = WorkPackage.find(declared_params[:id]) - authorize_in_project(:view_work_packages, project: @work_package.project) do + authorize_in_work_package(:view_work_packages, work_package: @work_package) do raise API::Errors::NotFound.new model: :work_package end end diff --git a/lib/api/v3/work_packages/work_packages_by_project_api.rb b/lib/api/v3/work_packages/work_packages_by_project_api.rb index c87dcdd774fa..f723abeda069 100644 --- a/lib/api/v3/work_packages/work_packages_by_project_api.rb +++ b/lib/api/v3/work_packages/work_packages_by_project_api.rb @@ -34,7 +34,7 @@ class WorkPackagesByProjectAPI < ::API::OpenProjectAPI helpers ::API::V3::WorkPackages::WorkPackagesSharedHelpers get do - authorize_in_project(:view_work_packages, project: @project) + authorize_in_any_work_package(:view_work_packages, in_project: @project) service = raise_invalid_query_on_service_failure do WorkPackageCollectionFromQueryParamsService diff --git a/lib_static/plugins/acts_as_attachable/lib/acts_as_attachable.rb b/lib_static/plugins/acts_as_attachable/lib/acts_as_attachable.rb index 01f0a5f1bad7..42d4ab38ef1d 100644 --- a/lib_static/plugins/acts_as_attachable/lib/acts_as_attachable.rb +++ b/lib_static/plugins/acts_as_attachable/lib/acts_as_attachable.rb @@ -210,7 +210,7 @@ def allowed_to_on_attachment?(user, permissions) permission_project_context = (project if respond_to?(:project)) Array(permissions).any? do |permission| - user.allowed_based_on_permission_context?(permission, project: permission_project_context) + user.allowed_based_on_permission_context?(permission, project: permission_project_context, entity: self) end end diff --git a/lib_static/plugins/acts_as_watchable/lib/acts_as_watchable.rb b/lib_static/plugins/acts_as_watchable/lib/acts_as_watchable.rb index 949a75abb4d4..8c2fea219da3 100644 --- a/lib_static/plugins/acts_as_watchable/lib/acts_as_watchable.rb +++ b/lib_static/plugins/acts_as_watchable/lib/acts_as_watchable.rb @@ -97,7 +97,9 @@ def self.prepended(base) end def possible_watcher?(user) - user.allowed_in_project?(self.class.acts_as_watchable_permission, project) + user.allowed_based_on_permission_context?(self.class.acts_as_watchable_permission, + project:, + entity: self) end # Returns all users that could potentially be watchers. @@ -112,7 +114,7 @@ def possible_watcher_users allowed_scope = if project.public? User.allowed(self.class.acts_as_watchable_permission, project) else - User.allowed_members(self.class.acts_as_watchable_permission, project) + User.allowed_members_on_work_package(self.class.acts_as_watchable_permission, self) end active_scope.where(id: allowed_scope) diff --git a/modules/backlogs/app/controllers/rb_impediments_controller.rb b/modules/backlogs/app/controllers/rb_impediments_controller.rb index ff2976837746..9daa7053cb76 100644 --- a/modules/backlogs/app/controllers/rb_impediments_controller.rb +++ b/modules/backlogs/app/controllers/rb_impediments_controller.rb @@ -72,8 +72,8 @@ def impediment_params(instance) # We block block_ids only when user is not allowed to create or update the # instance passed. unless instance && ((instance.new_record? && User.current.allowed_in_project?(:add_work_packages, - @project)) || User.current.allowed_in_project?( - :edit_work_packages, @project + @project)) || User.current.allowed_in_any_work_package?( + :edit_work_packages, in_project: @project )) hash.delete(:block_ids) end diff --git a/modules/backlogs/app/controllers/work_package_boxes_controller.rb b/modules/backlogs/app/controllers/work_package_boxes_controller.rb index c084ebd7d6c4..070c089d477f 100644 --- a/modules/backlogs/app/controllers/work_package_boxes_controller.rb +++ b/modules/backlogs/app/controllers/work_package_boxes_controller.rb @@ -39,7 +39,7 @@ def show r.other_work_package(@work_package) && r.other_work_package(@work_package).visible? end @allowed_statuses = WorkPackages::UpdateContract.new(work_package, User.current).assignable_statuses - @edit_allowed = User.current.allowed_in_project?(:edit_work_packages, @project) + @edit_allowed = User.current.allowed_in_work_package?(:edit_work_packages, @work_package) @priorities = IssuePriority.all @time_entry = TimeEntry.new diff --git a/modules/backlogs/spec/views/rb_taskboards/show_spec.rb b/modules/backlogs/spec/views/rb_taskboards/show_spec.rb index 798652e057fa..b298108f3814 100644 --- a/modules/backlogs/spec/views/rb_taskboards/show_spec.rb +++ b/modules/backlogs/spec/views/rb_taskboards/show_spec.rb @@ -37,11 +37,7 @@ end let(:role_forbidden) { create(:project_role) } # We need to create these as some view helpers access the database - let(:statuses) do - [create(:status), - create(:status), - create(:status)] - end + let(:statuses) { create_list(:status, 3) } let(:type_task) { create(:type_task) } let(:type_feature) { create(:type_feature) } diff --git a/modules/costs/app/contracts/time_entries/create_contract.rb b/modules/costs/app/contracts/time_entries/create_contract.rb index fcaad7d944b4..fe74503f3e2d 100644 --- a/modules/costs/app/contracts/time_entries/create_contract.rb +++ b/modules/costs/app/contracts/time_entries/create_contract.rb @@ -45,7 +45,7 @@ def allowed_to_log_for_others? end def allowed_to_log_to_himself? - model.user == user && user.allowed_in_project?(:log_own_time, model.project) + model.user == user && user.allowed_in_work_package?(:log_own_time, model.work_package) end end end diff --git a/modules/costs/app/contracts/time_entries/delete_contract.rb b/modules/costs/app/contracts/time_entries/delete_contract.rb index 868e28fba9a9..46835ab205c3 100644 --- a/modules/costs/app/contracts/time_entries/delete_contract.rb +++ b/modules/costs/app/contracts/time_entries/delete_contract.rb @@ -30,8 +30,8 @@ module TimeEntries class DeleteContract < ::DeleteContract delete_permission -> { edit_all = user.allowed_in_project?(:edit_time_entries, model.project) - edit_own = user.allowed_in_project?(:edit_own_time_entries, model.project) - edit_ongoing = model.ongoing && user.allowed_in_project?(:log_own_time, model.project) + edit_own = user.allowed_in_work_package?(:edit_own_time_entries, model.work_package) + edit_ongoing = model.ongoing && user.allowed_in_work_package?(:log_own_time, model.work_package) if model.user == user edit_own || edit_all || edit_ongoing diff --git a/modules/costs/app/contracts/time_entries/update_contract.rb b/modules/costs/app/contracts/time_entries/update_contract.rb index 97e885f3a3b4..5e447ad1ff1c 100644 --- a/modules/costs/app/contracts/time_entries/update_contract.rb +++ b/modules/costs/app/contracts/time_entries/update_contract.rb @@ -45,24 +45,26 @@ def validate_user_allowed_to_update def user_allowed_to_update? if model.ongoing || model.ongoing_was with_unchanged_project_id do - user_allowed_to_modify_ongoing?(model.project) - end && user_allowed_to_modify_ongoing?(model.project) + user_allowed_to_modify_ongoing?(model) + end && user_allowed_to_modify_ongoing?(model) else with_unchanged_project_id do - user_allowed_to_update_in?(model.project) - end && user_allowed_to_update_in?(model.project) + user_allowed_to_update_in?(model) + end && user_allowed_to_update_in?(model) end end private - def user_allowed_to_update_in?(project) - user.allowed_in_project?(:edit_time_entries, project) || - (model.user == user && user.allowed_in_project?(:edit_own_time_entries, project)) + def user_allowed_to_update_in?(time_entry) + user.allowed_in_project?(:edit_time_entries, time_entry.project) || + (model.user == user && user.allowed_in_work_package?(:edit_own_time_entries, time_entry.work_package)) end - def user_allowed_to_modify_ongoing?(project) - model.user == user && (user.allowed_in_project?(:log_time, project) || user.allowed_in_project?(:log_own_time, project)) + def user_allowed_to_modify_ongoing?(time_entry) + model.user == user && (user.allowed_in_project?(:log_time, + time_entry.project) || user.allowed_in_work_package?(:log_own_time, + time_entry.work_package)) end end end diff --git a/modules/costs/app/models/cost_scopes.rb b/modules/costs/app/models/cost_scopes.rb index 6379a7f8ec53..fa1ba1d581e4 100644 --- a/modules/costs/app/models/cost_scopes.rb +++ b/modules/costs/app/models/cost_scopes.rb @@ -38,6 +38,8 @@ def with_visible_entries_on(scope, user: User.current, project: nil) table = arel_table view_allowed = Project.allowed_to(user, view_allowed_entries_permission).select(:id) + # TODO: We either should move all _own_ permissions to be work package scoped and then change this + # query, or make this method less generic view_own_allowed = Project.allowed_to(user, view_allowed_own_entries_permission).select(:id) visible_scope = scope.where view_or_view_own(table, view_allowed, view_own_allowed, user) diff --git a/modules/costs/app/models/projects/scopes/visible_with_activated_time_activity.rb b/modules/costs/app/models/projects/scopes/visible_with_activated_time_activity.rb index 99b1672ca9cb..bc57345f3934 100644 --- a/modules/costs/app/models/projects/scopes/visible_with_activated_time_activity.rb +++ b/modules/costs/app/models/projects/scopes/visible_with_activated_time_activity.rb @@ -40,7 +40,7 @@ def visible_with_activated_time_activity(activity) def allowed_scope where(id: allowed_to(User.current, :view_time_entries).select(:id)) - .or(where(id: Project.allowed_to(User.current, :view_own_time_entries).select(:id))) + .or(where(id: WorkPackage.allowed_to(User.current, :view_own_time_entries).select(:project_id))) end end end diff --git a/modules/costs/app/models/time_entries/scopes/ongoing.rb b/modules/costs/app/models/time_entries/scopes/ongoing.rb index cbdc65a6325d..cd6f422bcc37 100644 --- a/modules/costs/app/models/time_entries/scopes/ongoing.rb +++ b/modules/costs/app/models/time_entries/scopes/ongoing.rb @@ -37,7 +37,7 @@ def ongoing def visible_ongoing(user = User.current) TimeEntry - .where(project_id: Project.allowed_to(user, :log_own_time), user:, ongoing: true) + .where(work_package_id: WorkPackage.allowed_to(user, :log_own_time), user:, ongoing: true) end def not_ongoing diff --git a/modules/costs/app/models/time_entries/scopes/visible.rb b/modules/costs/app/models/time_entries/scopes/visible.rb index 2e1ef4a8aa79..cc4d39d51b97 100644 --- a/modules/costs/app/models/time_entries/scopes/visible.rb +++ b/modules/costs/app/models/time_entries/scopes/visible.rb @@ -36,7 +36,7 @@ def visible(user = User.current) .where(project_id: Project.allowed_to(user, :view_time_entries)) own_scope = TimeEntry - .where(project_id: Project.allowed_to(user, :view_own_time_entries)) + .where(work_package_id: WorkPackage.allowed_to(user, :view_own_time_entries)) .where(user:) all_scope.or(own_scope) diff --git a/modules/costs/app/models/time_entry.rb b/modules/costs/app/models/time_entry.rb index 94484de0a668..c745e6df00cc 100644 --- a/modules/costs/app/models/time_entry.rb +++ b/modules/costs/app/models/time_entry.rb @@ -86,7 +86,7 @@ def hours=(value) # Returns true if the time entry can be edited by usr, otherwise false def editable_by?(usr) - (usr == user && usr.allowed_in_project?(:edit_own_time_entries, project)) || + (usr == user && usr.allowed_in_work_package?(:edit_own_time_entries, work_package)) || usr.allowed_in_project?(:edit_time_entries, project) end @@ -96,7 +96,7 @@ def current_rate def visible_by?(usr) usr.allowed_in_project?(:view_time_entries, project) || - (user_id == usr.id && usr.allowed_in_project?(:view_own_time_entries, project)) + (user_id == usr.id && usr.allowed_in_work_package?(:view_own_time_entries, work_package)) end def costs_visible_by?(usr) diff --git a/modules/costs/lib/api/v3/time_entries/available_projects_api.rb b/modules/costs/lib/api/v3/time_entries/available_projects_api.rb index 3e33a1e50c32..1e314023def8 100644 --- a/modules/costs/lib/api/v3/time_entries/available_projects_api.rb +++ b/modules/costs/lib/api/v3/time_entries/available_projects_api.rb @@ -31,7 +31,9 @@ module V3 module TimeEntries class AvailableProjectsAPI < ::API::OpenProjectAPI after_validation do - authorize_in_any_project(%i[log_time edit_time_entries edit_own_time_entries]) + authorize_in_any_work_package(:edit_own_time_entries) do + authorize_in_any_project(%i[log_time edit_time_entries]) + end end resources :available_projects do @@ -39,10 +41,10 @@ class AvailableProjectsAPI < ::API::OpenProjectAPI .new(model: Project, scope: -> { Project - .where(id: Project.allowed_to(User.current, :log_own_time)) + .where(id: WorkPackage.allowed_to(User.current, :log_own_time).select(:project_id)) .or(Project.where(id: Project.allowed_to(User.current, :log_time))) .or(Project.where(id: Project.allowed_to(User.current, :edit_time_entries))) - .or(Project.where(id: Project.allowed_to(User.current, :edit_own_time_entries))) + .or(Project.where(id: WorkPackage.allowed_to(User.current, :edit_own_time_entries).select(:project_id))) }) .mount end diff --git a/modules/costs/lib/api/v3/time_entries/available_work_packages_on_create_api.rb b/modules/costs/lib/api/v3/time_entries/available_work_packages_on_create_api.rb index 840c289bc7b2..d9a658510daa 100644 --- a/modules/costs/lib/api/v3/time_entries/available_work_packages_on_create_api.rb +++ b/modules/costs/lib/api/v3/time_entries/available_work_packages_on_create_api.rb @@ -31,14 +31,16 @@ module V3 module TimeEntries class AvailableWorkPackagesOnCreateAPI < ::API::OpenProjectAPI after_validation do - authorize_in_any_project(%i[log_time log_own_time]) + authorize_in_any_work_package(:log_own_time) do + authorize_in_any_project(:log_time) + end end helpers AvailableWorkPackagesHelper helpers do def allowed_scope - WorkPackage.where(project_id: Project.allowed_to(User.current, :log_own_time)) + WorkPackage.where(id: WorkPackage.allowed_to(User.current, :log_own_time)) .or(WorkPackage.where(project_id: Project.allowed_to(User.current, :log_time))) end end diff --git a/modules/costs/lib/api/v3/time_entries/available_work_packages_on_edit_api.rb b/modules/costs/lib/api/v3/time_entries/available_work_packages_on_edit_api.rb index 2bea06d5f69b..04c9d69a43d4 100644 --- a/modules/costs/lib/api/v3/time_entries/available_work_packages_on_edit_api.rb +++ b/modules/costs/lib/api/v3/time_entries/available_work_packages_on_edit_api.rb @@ -31,8 +31,9 @@ module V3 module TimeEntries class AvailableWorkPackagesOnEditAPI < ::API::OpenProjectAPI after_validation do - authorize_in_projects(%i[log_time log_own_time edit_time_entries edit_own_time_entries], - projects: @time_entry.project) + authorize_in_work_package(%i[log_own_time edit_own_time_entries], work_package: @time_entry.work_package) do + authorize_in_project(%i[log_time edit_time_entries], project: @time_entry.project) + end end helpers AvailableWorkPackagesHelper @@ -40,7 +41,7 @@ class AvailableWorkPackagesOnEditAPI < ::API::OpenProjectAPI helpers do def allowed_scope edit_scope = WorkPackage.where(project_id: Project.allowed_to(User.current, :edit_time_entries)) - edit_own_scope = WorkPackage.where(project_id: Project.allowed_to(User.current, :edit_own_time_entries)) + edit_own_scope = WorkPackage.where(id: WorkPackage.allowed_to(User.current, :edit_own_time_entries)) ongoing_scope = WorkPackage.where(id: TimeEntry.visible_ongoing.select(:work_package_id)) edit_scope diff --git a/modules/costs/lib/api/v3/time_entries/create_form_api.rb b/modules/costs/lib/api/v3/time_entries/create_form_api.rb index d77db6301d9d..3e94098be796 100644 --- a/modules/costs/lib/api/v3/time_entries/create_form_api.rb +++ b/modules/costs/lib/api/v3/time_entries/create_form_api.rb @@ -32,7 +32,9 @@ module TimeEntries class CreateFormAPI < ::API::OpenProjectAPI resource :form do after_validation do - authorize_in_any_project(%i[log_time log_own_time]) + authorize_in_any_work_package(:log_own_time) do + authorize_in_any_project(:log_time) + end end post &::API::V3::Utilities::Endpoints::CreateForm diff --git a/modules/costs/lib/api/v3/time_entries/schemas/time_entry_schema_api.rb b/modules/costs/lib/api/v3/time_entries/schemas/time_entry_schema_api.rb index 125f80e5891a..9d581f8bfa58 100644 --- a/modules/costs/lib/api/v3/time_entries/schemas/time_entry_schema_api.rb +++ b/modules/costs/lib/api/v3/time_entries/schemas/time_entry_schema_api.rb @@ -33,11 +33,9 @@ module Schemas class TimeEntrySchemaAPI < ::API::OpenProjectAPI resources :schema do after_validation do - authorize_in_any_project(%i[log_time - log_own_time - view_time_entries - edit_time_entries - edit_own_time_entries]) + authorize_in_any_work_package(%i[log_own_time edit_own_time_entries]) do + authorize_in_any_project(%i[log_time view_time_entries edit_time_entries]) + end end get &::API::V3::Utilities::Endpoints::Schema diff --git a/modules/costs/lib/api/v3/time_entries/time_entries_activity_api.rb b/modules/costs/lib/api/v3/time_entries/time_entries_activity_api.rb index c8be590a8f20..1d1edbb5fe51 100644 --- a/modules/costs/lib/api/v3/time_entries/time_entries_activity_api.rb +++ b/modules/costs/lib/api/v3/time_entries/time_entries_activity_api.rb @@ -33,12 +33,13 @@ class TimeEntriesActivityAPI < ::API::OpenProjectAPI resources :activities do route_param :id, type: Integer, desc: 'Time entry activity ID' do after_validation do - authorize_in_any_project(%i(log_time - view_time_entries - edit_time_entries - edit_own_time_entries - manage_project_activities)) do - raise API::Errors::NotFound.new + authorize_in_any_work_package(:edit_own_time_entries) do + authorize_in_any_project(%i(log_time + view_time_entries + edit_time_entries + manage_project_activities)) do + raise API::Errors::NotFound.new + end end @activity = TimeEntryActivity diff --git a/modules/costs/lib/costs/patches/projects_controller_patch.rb b/modules/costs/lib/costs/patches/projects_controller_patch.rb index f036e01afdb5..44de9ac31158 100644 --- a/modules/costs/lib/costs/patches/projects_controller_patch.rb +++ b/modules/costs/lib/costs/patches/projects_controller_patch.rb @@ -37,7 +37,7 @@ def self.included(base) # :nodoc: module InstanceMethods def own_total_hours - if User.current.allowed_in_project?(:view_own_time_entries, @project) + if User.current.allowed_in_any_work_package?(:view_own_time_entries, in_project: @project) cond = @project.project_condition(Setting.display_subprojects_work_packages?) @total_hours = TimeEntry.not_ongoing.visible.includes(:project).where(cond).sum(:hours).to_f end diff --git a/modules/costs/spec/factories/time_entry_factory.rb b/modules/costs/spec/factories/time_entry_factory.rb index 876e15fee698..a3096b765ecd 100644 --- a/modules/costs/spec/factories/time_entry_factory.rb +++ b/modules/costs/spec/factories/time_entry_factory.rb @@ -28,12 +28,15 @@ FactoryBot.define do factory :time_entry do - project user work_package spent_on { Date.today } activity factory: :time_entry_activity hours { 1.0 } logged_by { user } + + after(:build) do |time_entry| + time_entry.project ||= time_entry.work_package.project + end end end diff --git a/modules/costs/spec/lib/api/v3/time_entries/time_entry_representer_rendering_spec.rb b/modules/costs/spec/lib/api/v3/time_entries/time_entry_representer_rendering_spec.rb index e33795edc3a7..cd9889401e63 100644 --- a/modules/costs/spec/lib/api/v3/time_entries/time_entry_representer_rendering_spec.rb +++ b/modules/costs/spec/lib/api/v3/time_entries/time_entry_representer_rendering_spec.rb @@ -40,10 +40,11 @@ hours:, activity:, project:, + work_package:, user:) end let(:project) { build_stubbed(:project) } - let(:work_package) { time_entry.work_package } + let(:work_package) { build_stubbed(:work_package, project:) } let(:activity) { build_stubbed(:time_entry_activity) } let(:user) { build_stubbed(:user) } let(:current_user) { user } @@ -61,6 +62,7 @@ mock_permissions_for(current_user) do |mock| mock.allow_in_project *permissions, project: end + allow(time_entry) .to receive(:available_custom_fields) .and_return([]) diff --git a/modules/costs/spec/models/projects/scopes/visible_with_activated_time_activity_spec.rb b/modules/costs/spec/models/projects/scopes/visible_with_activated_time_activity_spec.rb index 451cfdab153f..835e8b6603ca 100644 --- a/modules/costs/spec/models/projects/scopes/visible_with_activated_time_activity_spec.rb +++ b/modules/costs/spec/models/projects/scopes/visible_with_activated_time_activity_spec.rb @@ -31,7 +31,9 @@ RSpec.describe Projects::Scopes::VisibleWithActivatedTimeActivity do let!(:activity) { create(:time_entry_activity) } let!(:project) { create(:project) } + let!(:work_package) { create(:work_package, project:) } let!(:other_project) { create(:project) } + let!(:other_work_package) { create(:work_package, project: other_project) } let(:project_permissions) { [:view_time_entries] } let(:other_project_permissions) { [:view_time_entries] } let(:current_user) do @@ -59,7 +61,7 @@ context 'and being active' do it 'returns all projects' do expect(subject) - .to match_array [project, other_project] + .to contain_exactly(project, other_project) end end @@ -80,7 +82,7 @@ it 'returns all projects' do expect(subject) - .to match_array [project, other_project] + .to contain_exactly(project, other_project) end end @@ -104,7 +106,7 @@ context 'and being active' do it 'returns the project the activity is activated in' do expect(subject) - .to match_array [project] + .to contain_exactly(project) end end @@ -115,7 +117,7 @@ it 'returns only the projects the activity is activated in' do expect(subject) - .to match_array [project] + .to contain_exactly(project) end end end diff --git a/modules/costs/spec/models/queries/time_entries/time_entry_query_integration_spec.rb b/modules/costs/spec/models/queries/time_entries/time_entry_query_integration_spec.rb index d5b3a9801149..b6d9e522e213 100644 --- a/modules/costs/spec/models/queries/time_entries/time_entry_query_integration_spec.rb +++ b/modules/costs/spec/models/queries/time_entries/time_entry_query_integration_spec.rb @@ -38,10 +38,12 @@ context 'when using ongoing filter' do let(:project) { create(:project, enabled_module_names: %w[costs]) } let(:user) { create(:user, member_with_permissions: { project => %i[log_own_time] }) } + let(:work_package) { create(:work_package, project:) } let(:other_user) { create(:user, member_with_permissions: { project => %i[log_own_time] }) } + let(:other_work_package) { create(:work_package, project:) } - let!(:user_timer) { create(:time_entry, user:, project:, ongoing: true) } - let!(:other_user_timer) { create(:time_entry, user: other_user, project:, ongoing: true) } + let!(:user_timer) { create(:time_entry, user:, work_package:, ongoing: true) } + let!(:other_user_timer) { create(:time_entry, user: other_user, work_package: other_work_package, ongoing: true) } describe '#results' do subject { instance.results } diff --git a/modules/costs/spec/requests/api/time_entries/available_projects_resource_spec.rb b/modules/costs/spec/requests/api/time_entries/available_projects_resource_spec.rb index 752e51328680..abde372baca1 100644 --- a/modules/costs/spec/requests/api/time_entries/available_projects_resource_spec.rb +++ b/modules/costs/spec/requests/api/time_entries/available_projects_resource_spec.rb @@ -45,6 +45,7 @@ roles: [create(:project_role, permissions: [:log_own_time])], project: p, user: current_user) + create(:work_package, project: p, author: current_user) end end let(:project_with_edit_permission) do @@ -61,6 +62,7 @@ roles: [create(:project_role, permissions: [:edit_own_time_entries])], project: p, user: current_user) + create(:work_package, project: p, author: current_user) end end let(:project_with_view_permission) do @@ -85,7 +87,7 @@ subject(:response) { last_response } - describe 'GET api/v3/memberships/available_projects' do + describe 'GET api/v3/time_entries/available_projects' do let(:projects) do [project_with_log_permission, project_with_edit_permission, @@ -98,6 +100,7 @@ before do projects + login_as(current_user) get path diff --git a/modules/github_integration/lib/api/v3/github_pull_requests/github_pull_requests_by_work_package_api.rb b/modules/github_integration/lib/api/v3/github_pull_requests/github_pull_requests_by_work_package_api.rb index 725da18adc7e..7839e45b81a4 100644 --- a/modules/github_integration/lib/api/v3/github_pull_requests/github_pull_requests_by_work_package_api.rb +++ b/modules/github_integration/lib/api/v3/github_pull_requests/github_pull_requests_by_work_package_api.rb @@ -31,7 +31,7 @@ module V3 module GithubPullRequests class GithubPullRequestsByWorkPackageAPI < ::API::OpenProjectAPI after_validation do - authorize_in_project(:show_github_content, project: @work_package.project) + authorize_in_work_package(:show_github_content, work_package: @work_package) @github_pull_requests = @work_package.github_pull_requests end diff --git a/modules/github_integration/lib/open_project/github_integration/notification_handler/helper.rb b/modules/github_integration/lib/open_project/github_integration/notification_handler/helper.rb index 9254a68a1241..df00d4379a60 100644 --- a/modules/github_integration/lib/open_project/github_integration/notification_handler/helper.rb +++ b/modules/github_integration/lib/open_project/github_integration/notification_handler/helper.rb @@ -63,7 +63,7 @@ def find_visible_work_packages(ids, user) WorkPackage .includes(:project) .where(id: ids) - .select { |wp| user.allowed_in_project?(:add_work_package_notes, wp.project) } + .select { |wp| user.allowed_in_work_package?(:add_work_package_notes, wp) } end # Returns a list of `WorkPackage`s that were referenced in the `text` and are visible to the given `user`. diff --git a/modules/github_integration/lib/open_project/github_integration/patches/api/work_package_representer.rb b/modules/github_integration/lib/open_project/github_integration/patches/api/work_package_representer.rb index c8b48ef51493..1b2930659395 100644 --- a/modules/github_integration/lib/open_project/github_integration/patches/api/work_package_representer.rb +++ b/modules/github_integration/lib/open_project/github_integration/patches/api/work_package_representer.rb @@ -7,7 +7,7 @@ module WorkPackageRepresenter def extension ->(*) do link :github, - cache_if: -> { current_user.allowed_in_project?(:show_github_content, represented.project) } do + cache_if: -> { current_user.allowed_in_work_package?(:show_github_content, represented) } do { href: "#{work_package_path(id: represented.id)}/tabs/github", title: "github" diff --git a/modules/meeting/spec/features/meetings_copy_spec.rb b/modules/meeting/spec/features/meetings_copy_spec.rb index b1b871df2a23..a20cc1e115c5 100644 --- a/modules/meeting/spec/features/meetings_copy_spec.rb +++ b/modules/meeting/spec/features/meetings_copy_spec.rb @@ -44,7 +44,7 @@ member_with_permissions: { project => permissions }) end - shared_let(:start_time) { Time.current.beginning_of_week.at_noon } + shared_let(:start_time) { Time.current.next_day.at_noon } shared_let(:duration) { 1.5 } shared_let(:agenda_text) { "We will talk" } shared_let(:meeting) do @@ -62,7 +62,7 @@ shared_let(:twelve_hour_format) { "%I:%M %p" } shared_let(:copied_meeting_time_heading) do - date = start_time.next_week.strftime("%m/%d/%Y") + date = (start_time + 1.week).strftime("%m/%d/%Y") start_of_meeting = start_time.strftime(twelve_hour_format) end_of_meeting = (start_time + meeting.duration.hours).strftime(twelve_hour_format) @@ -73,7 +73,7 @@ login_as user end - it 'copying a meeting' do + it 'copying a meeting' do visit project_meetings_path(project) click_link meeting.title @@ -90,7 +90,7 @@ expect(page) .to have_field 'Duration', with: meeting.duration expect(page) - .to have_field 'Start date', with: start_time.next_week.strftime("%Y-%m-%d") + .to have_field 'Start date', with: (start_time + 1.week).strftime("%Y-%m-%d") expect(page) .to have_field 'Time', with: start_time.strftime("%H:%M") diff --git a/modules/reporting/app/views/cost_reports/index.html.erb b/modules/reporting/app/views/cost_reports/index.html.erb index 22ea7be6ab91..6a9e4f342910 100644 --- a/modules/reporting/app/views/cost_reports/index.html.erb +++ b/modules/reporting/app/views/cost_reports/index.html.erb @@ -43,7 +43,7 @@ See COPYRIGHT and LICENSE files for more details.