From 9713c6b270ab5436c3ceda5e70640eaef15fc7f3 Mon Sep 17 00:00:00 2001 From: Marius Balteanu Date: Mon, 20 Jun 2022 05:45:47 +0000 Subject: [PATCH] Merged r21645-r21646 and r21657 to 5.0-stable (#37255). git-svn-id: https://svn.redmine.org/redmine/branches/5.0-stable@21658 e93f8b46-1217-0410-a6f0-8f06a7374b81 --- app/helpers/application_helper.rb | 1 + app/models/issue_custom_field.rb | 2 +- app/models/query.rb | 6 +- app/models/time_entry.rb | 6 +- test/functional/timelog_controller_test.rb | 96 ++++++++++++++++++++++ test/unit/query_test.rb | 5 +- 6 files changed, 110 insertions(+), 6 deletions(-) diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb index 4ddbf650a..1a8402020 100644 --- a/app/helpers/application_helper.rb +++ b/app/helpers/application_helper.rb @@ -295,6 +295,7 @@ module ApplicationHelper object.filename end when 'CustomValue', 'CustomFieldValue' + return "" unless object.customized&.visible? if object.custom_field f = object.custom_field.format.formatted_custom_value(self, object, html) if f.nil? || f.is_a?(String) diff --git a/app/models/issue_custom_field.rb b/app/models/issue_custom_field.rb index 18ec6bb43..de5c9ceb2 100644 --- a/app/models/issue_custom_field.rb +++ b/app/models/issue_custom_field.rb @@ -39,7 +39,7 @@ class IssueCustomField < CustomField project_condition = "EXISTS (SELECT 1 FROM #{CustomField.table_name} ifa WHERE ifa.is_for_all = #{self.class.connection.quoted_true} AND ifa.id = #{id_column})" + " OR #{Issue.table_name}.project_id IN (SELECT project_id FROM #{table_name_prefix}custom_fields_projects#{table_name_suffix} WHERE custom_field_id = #{id_column})" - "((#{sql}) AND (#{tracker_condition}) AND (#{project_condition}))" + "((#{sql}) AND (#{tracker_condition}) AND (#{project_condition}) AND (#{Issue.visible_condition(user)}))" end def validate_custom_field diff --git a/app/models/query.rb b/app/models/query.rb index 0186cc379..7695bc165 100644 --- a/app/models/query.rb +++ b/app/models/query.rb @@ -113,7 +113,8 @@ class QueryAssociationColumn < QueryColumn end def value_object(object) - if assoc = object.send(@association) + assoc = object.send(@association) + if assoc && assoc.visible? assoc.send @attribute end end @@ -184,7 +185,8 @@ class QueryAssociationCustomFieldColumn < QueryCustomFieldColumn end def value_object(object) - if assoc = object.send(@association) + assoc = object.send(@association) + if assoc && assoc.visible? super(assoc) end end diff --git a/app/models/time_entry.rb b/app/models/time_entry.rb index 88e1796f3..34e4aab7b 100644 --- a/app/models/time_entry.rb +++ b/app/models/time_entry.rb @@ -63,7 +63,11 @@ class TimeEntry < ActiveRecord::Base where(TimeEntry.visible_condition(args.shift || User.current, *args)) end) scope :left_join_issue, (lambda do - joins("LEFT OUTER JOIN #{Issue.table_name} ON #{Issue.table_name}.id = #{TimeEntry.table_name}.issue_id") + joins( + "LEFT OUTER JOIN #{Issue.table_name}" \ + " ON #{Issue.table_name}.id = #{TimeEntry.table_name}.issue_id" \ + " AND (#{Issue.visible_condition(User.current)})" + ) end) scope :on_issue, (lambda do |issue| joins(:issue). diff --git a/test/functional/timelog_controller_test.rb b/test/functional/timelog_controller_test.rb index 51fc9c310..321ed4d21 100644 --- a/test/functional/timelog_controller_test.rb +++ b/test/functional/timelog_controller_test.rb @@ -1466,6 +1466,102 @@ class TimelogControllerTest < Redmine::ControllerTest assert_select 'td.issue_cf_2', :text => 'filter_on_issue_custom_field' end + def test_index_should_not_disclose_issue_data + category = IssueCategory.find 2 + issue = + Issue.generate!( + :project_id => 1, :tracker_id => 1, + :custom_field_values => {2 => 'filter_on_issue_custom_field'} + ) + entry = TimeEntry.generate!(:issue => issue, :hours => 2.5) + session[:user_id] = 3 + issue.update_columns is_private: true, category_id: category.id + assert_not issue.visible?(User.find(3)) + # since the issue is not visible, its custom fields and associated ojects should not be visible either + + get :index, :params => { + :c => %w(issue issue.cf_2 issue.category) + } + assert_response :success + assert_select 'td.issue', :text => /#{issue.subject}/, :count => 0 + assert_select 'td.issue-category', :text => /#{category.name}/, :count => 0 + assert_select 'td.issue_cf_2', :text => 'filter_on_issue_custom_field', :count => 0 + end + + def test_index_should_not_filter_by_invisible_issue_data + issue = + Issue.generate!( + :project_id => 1, :tracker_id => 1, + :custom_field_values => {2 => 'filter_on_issue_custom_field'} + ) + entry = TimeEntry.generate!(:issue => issue, :hours => 2.5) + session[:user_id] = 3 + issue.update_columns is_private: true + assert_not issue.visible?(User.find(3)) + # since the issue is not visible, its custom fields and associated ojects should not be filterable + + get :index, :params => { + :f => ['issue.tracker_id'], + :op => {'issue.tracker_id' => '='}, + :v => {'issue.tracker_id' => ["1"]}, + } + assert_response :success + assert_not_include entry.id.to_s, css_select('input[name="ids[]"]').map {|e| e.attr(:value)} + + get :index, :params => { + :f => ['issue.cf_2'], + :op => {'issue.cf_2' => '='}, + :v => {'issue.cf_2' => ['filter_on_issue_custom_field']}, + } + assert_response :success + assert_equal [], css_select('input[name="ids[]"]').map {|e| e.attr(:value)} + end + + def test_indext_should_not_sort_by_invisible_issue_data + category1 = IssueCategory.find 1 + category2 = IssueCategory.find 2 + issue1 = + Issue.generate!( + :project_id => 1, :tracker_id => 1, + :custom_field_values => {2 => 'filter_on_issue_custom_field'} + ) + issue2 = + Issue.generate!( + :project_id => 1, :tracker_id => 1, + :custom_field_values => {2 => 'xxx_this_will_be_last'} + ) + entry1 = TimeEntry.generate!(:issue => issue1, :hours => 2.5, :spent_on => '2022-06-13') + entry2 = TimeEntry.generate!(:issue => issue2, :hours => 2.5, :spent_on => '2022-06-12') + session[:user_id] = 3 + issue1.update_columns is_private: true, category_id: category1.id + issue2.update_columns is_private: true, category_id: category2.id + assert_not issue1.visible?(User.find(3)) + assert_not issue2.visible?(User.find(3)) + # since the issues are not visible, their custom fields and associated ojects should not be sortable + + # issue.cf_2:desc would be entry2, entry1 + # spent_on:desc is entry1, entry2 + get :index, :params => { + :sort => "issue.cf_2:desc,spent_on:desc", + :f => ['spent_on'], + :op => {'spent_on' => '><'}, + :v => {'spent_on' => ['2022-06-12', '2022-06-13']}, + } + assert_response :success + assert_equal [entry1.id.to_s, entry2.id.to_s], css_select('input[name="ids[]"]').map {|e| e.attr(:value)} + + # issue.category:desc would be entry2, entry1 + # spent_on:desc is entry1, entry2 + get :index, :params => { + :sort => "issue.category:desc,spent_on:desc", + :f => ['spent_on'], + :op => {'spent_on' => '><'}, + :v => {'spent_on' => ['2022-06-12', '2022-06-13']}, + } + assert_response :success + assert_equal [entry1.id.to_s, entry2.id.to_s], css_select('input[name="ids[]"]').map {|e| e.attr(:value)} + end + def test_index_with_time_entry_custom_field_column field = TimeEntryCustomField.generate!(:field_format => 'string') entry = TimeEntry.generate!(:hours => 2.5, :custom_field_values => {field.id => 'CF Value'}) diff --git a/test/unit/query_test.rb b/test/unit/query_test.rb index d6f7922a1..dbf95519f 100644 --- a/test/unit/query_test.rb +++ b/test/unit/query_test.rb @@ -1129,14 +1129,15 @@ class QueryTest < ActiveSupport::TestCase def test_filter_on_custom_field_should_ignore_projects_with_field_disabled field = IssueCustomField.generate!( - :trackers => Tracker.all, :project_ids => [1, 3, 4], + :trackers => Tracker.all, :project_ids => [1, 3, 5], :is_for_all => false, :is_filter => true ) Issue.generate!(:project_id => 3, :tracker_id => 2, :custom_field_values => {field.id.to_s => 'Foo'}) - Issue.generate!(:project_id => 4, :tracker_id => 2, + Issue.generate!(:project_id => 5, :tracker_id => 2, :custom_field_values => {field.id.to_s => 'Foo'}) + User.current = User.find(1) query = IssueQuery.new(:name => '_', :project => Project.find(1)) query.filters = {"cf_#{field.id}" => {:operator => '=', :values => ['Foo']}} assert_equal 2, find_issues_with_query(query).size -- 2.39.5