Merge remote-tracking branch 'origin/release/13.1' into dev

app/components/projects/row_component.html.erb

@@ -70,7 +70,7 @@ See COPYRIGHT and LICENSE files for more details.
     <% end %>
   </td>
 </tr>
-<% if project.description.present? %>
+<% if User.current.allowed_in_project?(:view_project, project) && project.description.present? %>
   <tr class="project-description <%= project_css_classes %> <%= row_css_level_classes %> <%= params[:expand] == 'all' ? '-expanded' : '' %>"
       data-project-target="descriptionRow"
       data-project-id="<%= project.id %>">

app/components/projects/row_component.rb

@@ -52,6 +52,8 @@ def column_value(column)
     end
 
     def custom_field_column(column)
+      return nil unless user_can_view_project?
+
       cf = custom_field(column)
       custom_value = project.formatted_custom_value_for(cf)
 
@@ -92,6 +94,8 @@ def name
     end
 
     def project_status
+      return nil unless user_can_view_project?
+
       content = ''.html_safe
 
       status_code = project.status_code
@@ -106,6 +110,8 @@ def project_status
     end
 
     def status_explanation
+      return nil unless user_can_view_project?
+
       if project.status_explanation
         content_tag :div, helpers.format_text(project.status_explanation), class: 'wiki'
       end
@@ -165,5 +171,9 @@ def additional_css_class(column)
         "format-#{cf.field_format}#{formattable}"
       end
     end
+
+    def user_can_view_project?
+      User.current.allowed_in_project?(:view_project, project)
+    end
   end
 end

app/policies/work_package_policy.rb

@@ -31,12 +31,10 @@ class WorkPackagePolicy < BasePolicy
 
   def cache(work_package)
     @cache ||= Hash.new do |wp_hash, wp|
-      wp_hash[wp] = Hash.new do |project_hash, project|
-        project_hash[project] = allowed_hash(wp)
-      end
+      wp_hash[wp] = allowed_hash(wp)
     end
 
-    @cache[work_package][work_package.project]
+    @cache[work_package]
   end
 
   def allowed_hash(work_package)

lib/api/caching/cached_representer.rb

@@ -190,7 +190,7 @@ def json_key_representer_parts
           cacheable << json_key_parts_of_represented
           cacheable << json_key_dependencies
 
-          OpenProject::Cache::CacheKey.expand(cacheable.flatten.compact)
+          OpenProject::Cache::CacheKey.expand(OpenProject::Cache::CacheKey.key(cacheable.flatten.compact))
         end
 
         def json_key_part_represented

lib/api/v3/work_packages/schema/work_package_schema_representer.rb

@@ -62,21 +62,21 @@ def schema(property, *args)
               opts, = args
               opts[:attribute_group] = attribute_group property
 
-              super property, **opts
+              super(property, **opts)
             end
 
             def schema_with_allowed_link(property, *args)
               opts, = args
               opts[:attribute_group] = attribute_group property
 
-              super property, **opts
+              super(property, **opts)
             end
 
             def schema_with_allowed_collection(property, *args)
               opts, = args
               opts[:attribute_group] = attribute_group property
 
-              super property, **opts
+              super(property, **opts)
             end
           end
 
@@ -168,9 +168,9 @@ def initialize(schema, self_link:, **context)
                  type: 'Duration',
                  required: false,
                  show_if: ->(*) {
-                   current_user.allowed_in_project?(:view_time_entries, represented.project) ||
-                   current_user.allowed_in_work_package?(:view_own_time_entries, represented)
-                 }
+                            current_user.allowed_in_project?(:view_time_entries, represented.project) ||
+                            current_user.allowed_in_any_work_package?(:view_own_time_entries, in_project: represented.project)
+                          }
 
           schema :percentage_done,
                  type: 'Integer',

lib/api/v3/work_packages/work_package_representer.rb

@@ -573,7 +573,16 @@ def current_user_update_allowed?
         def view_time_entries_allowed?
           @view_time_entries_allowed ||=
             current_user.allowed_in_project?(:view_time_entries, represented.project) ||
-              current_user.allowed_in_work_package?(:view_own_time_entries, represented)
+            view_own_time_entries_allowed?
+        end
+
+        def view_own_time_entries_allowed?
+          @view_own_time_entries_allowed ||= if represented.new_record?
+                                               current_user.allowed_in_any_work_package?(:view_own_time_entries,
+                                                                                         in_project: represented.project)
+                                             else
+                                               current_user.allowed_in_work_package?(:view_own_time_entries, represented)
+                                             end
         end
 
         def log_time_allowed?

modules/costs/app/models/cost_scopes.rb

@@ -37,11 +37,9 @@ def with_visible_costs_on(scope, user: User.current, project: nil)
   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)
+    visible_scope = scope.where(
+      view_or_view_own(table, view_allowed_entries_permission, view_allowed_own_entries_permission, user)
+    )
 
     if project
       visible_scope.where(project_id: project.id)
@@ -50,14 +48,26 @@ def with_visible_entries_on(scope, user: User.current, project: nil)
     end
   end
 
-  def view_or_view_own(table, view_allowed, view_own_allowed, user)
-    table[:project_id]
-      .in(view_allowed.arel)
-      .or(
+  def view_or_view_own(table, allowed_permission, allowed_own_permission, user) # rubocop:disable Metrics/AbcSize
+    project_allowed_scope = table[:project_id].in(Project.allowed_to(user, allowed_permission).select(:id).arel)
+
+    # We allow some of the `_own_` permissions on the WorkPackage, but others only on the project,
+    # so we need to figure out the correct scope to use
+    wp_scoped_permission = Authorization.permissions_for(allowed_own_permission).any?(&:work_package?)
+
+    if wp_scoped_permission
+      project_allowed_scope.or(
+        table[:work_package_id]
+        .in(WorkPackage.allowed_to(user, allowed_own_permission).select(:id).arel)
+        .and(table[:user_id].eq(user.id))
+      )
+    else
+      project_allowed_scope.or(
         table[:project_id]
-          .in(view_own_allowed.arel)
-          .and(table[:user_id].eq(user.id))
+        .in(Project.allowed_to(user, allowed_own_permission).select(:id).arel)
+        .and(table[:user_id].eq(user.id))
       )
+    end
   end
 
   def with_visible_rates_on(scope, user: User.current)

modules/costs/app/models/time_entries/time_entry_scopes.rb

@@ -12,11 +12,7 @@ def view_allowed_own_entries_permission
 
     def with_visible_rates_on(scope, user: User.current)
       table = arel_table
-
-      view_allowed = Project.allowed_to(user, :view_hourly_rates).select(:id)
-      view_own_allowed = Project.allowed_to(user, :view_own_hourly_rate).select(:id)
-
-      scope.where view_or_view_own(table, view_allowed, view_own_allowed, user)
+      scope.where(view_or_view_own(table, :view_hourly_rates, :view_own_hourly_rate, user))
     end
   end
 end

modules/costs/app/models/time_entry.rb

@@ -52,7 +52,6 @@ class TimeEntry < ApplicationRecord
   include Entry::SplashedDates
 
   scopes :of_user_and_day,
-         :visible,
          :ongoing
 
   # TODO: move into service

modules/costs/spec/features/view_own_rates_spec.rb

@@ -28,20 +28,20 @@
 
 require File.expand_path(File.dirname(__FILE__) + '/../spec_helper.rb')
 
-RSpec.describe 'Only see your own rates', js: true do
+RSpec.describe 'Only see your own rates', :js do
   let(:project) { work_package.project }
   let(:user) do
     create(:user,
            member_with_roles: { project => role })
   end
   let(:role) do
     create(:project_role, permissions: %i[view_own_hourly_rate
-                                  view_work_packages
-                                  view_work_packages
-                                  view_own_time_entries
-                                  view_own_cost_entries
-                                  view_cost_rates
-                                  log_costs])
+                                          view_work_packages
+                                          view_work_packages
+                                          view_own_time_entries
+                                          view_own_cost_entries
+                                          view_cost_rates
+                                          log_costs])
   end
   let(:work_package) { create(:work_package) }
   let(:wp_page) { Pages::FullWorkPackage.new(work_package) }

modules/costs/spec/models/time_entries/scopes/visible_spec.rb

@@ -28,13 +28,8 @@
 
 require 'spec_helper'
 
-RSpec.describe TimeEntries::Scopes::Visible do
+RSpec.describe TimeEntry, "visible scope" do
   let(:project) { create(:project) }
-  let(:user) do
-    create(:user,
-           member_with_permissions: { project => permissions })
-  end
-  let(:permissions) { [:view_time_entries] }
 
   let(:work_package) do
     create(:work_package,
@@ -68,18 +63,20 @@
     subject { TimeEntry.visible(user) }
 
     context 'for a user having the view_time_entries permission' do
+      let(:user) { create(:user, member_with_permissions: { project => [:view_time_entries] }) }
+
       it 'retrieves all the time entries of projects the user has the permissions in' do
         expect(subject)
-          .to match_array([own_project_time_entry, project_time_entry])
+          .to contain_exactly(own_project_time_entry, project_time_entry)
       end
     end
 
-    context 'for a user having the view_own_time_entries permission' do
-      let(:permissions) { [:view_own_time_entries] }
+    context 'for a user having the view_own_time_entries permission on a work package' do
+      let(:user) { create(:user, member_with_permissions: { work_package => [:view_own_time_entries] }) }
 
       it 'retrieves all the time entries of the user in projects the user has the permissions in' do
         expect(subject)
-          .to match_array([own_project_time_entry])
+          .to contain_exactly(own_project_time_entry)
       end
     end
   end

spec/factories/principal_factory.rb

@@ -77,7 +77,7 @@
           create(:member, principal:, project: object, roles: [role])
         elsif Member.can_be_member_of?(object)
           role = create(:work_package_role, permissions:)
-          create(:member, principal:, entity: object, project: entity.project, roles: [role])
+          create(:member, principal:, entity: object, project: object.project, roles: [role])
         end
       end
 

spec/features/projects/projects_index_spec.rb

@@ -35,7 +35,7 @@
   shared_let(:manager)   { create(:project_role, name: 'Manager') }
   shared_let(:developer) { create(:project_role, name: 'Developer') }
 
-  shared_let(:custom_field) { create(:project_custom_field) }
+  shared_let(:custom_field) { create(:text_project_custom_field) }
   shared_let(:invisible_custom_field) { create(:project_custom_field, visible: false) }
 
   shared_let(:project) do
@@ -60,6 +60,8 @@
   let(:news) { create(:news, project:) }
   let(:projects_page) { Pages::Projects::Index.new }
 
+  include ProjectStatusHelper
+
   def load_and_open_filters(user)
     login_as(user)
     projects_page.visit!
@@ -120,6 +122,41 @@ def expect_projects_in_order(*projects)
       end
     end
 
+    context 'for work package members', with_ee: %i[custom_fields_in_projects_list] do
+      shared_let(:work_package) { create(:work_package, project: development_project) }
+      shared_let(:user) do
+        create(:user,
+               member_with_permissions: { work_package => [:view_work_packages] },
+               login: 'nerd',
+               firstname: 'Alan',
+               lastname: 'Turing')
+      end
+
+      specify 'only public projects or those the user is member in a specific work package' do
+        Setting.enabled_projects_columns += [custom_field.column_name]
+
+        development_project.update(
+          description: 'I am a nice project',
+          status_explanation: 'We are on track',
+          status_code: 'on_track',
+          custom_field_values: { custom_field.id => 'This is a test value' }
+        )
+
+        login_as(user)
+        visit projects_path
+
+        expect(page).to have_text(development_project.name)
+        expect(page).to have_text(public_project.name)
+        expect(page).not_to have_text(project.name)
+
+        # They should not see the description, status or custom fields for the project
+        expect(page).not_to have_text(development_project.description)
+        expect(page).not_to have_text(project_status_name(development_project.status_code))
+        expect(page).not_to have_text(development_project.status_explanation)
+        expect(page).not_to have_text(development_project.custom_value_for(custom_field))
+      end
+    end
+
     context 'for admins' do
       before do
         project.update(created_at: 7.days.ago, description: 'I am a nice project')