From 2d20811f406ab4f9b6ac27a935f27c721833f882 Mon Sep 17 00:00:00 2001 From: Marius Balteanu Date: Mon, 8 Jul 2024 21:28:24 +0000 Subject: [PATCH] Improve watcher list permissions check to explicitly require @view_issue_watchers@ permission (#40946). MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit Patch by Felix Schäfer (@felix). git-svn-id: https://svn.redmine.org/redmine/trunk@22913 e93f8b46-1217-0410-a6f0-8f06a7374b81 --- app/controllers/watchers_controller.rb | 2 +- app/views/watchers/_watchers.html.erb | 4 ++++ .../lib/acts_as_watchable.rb | 10 ++++++++++ test/functional/issues_controller_test.rb | 11 +++++++++++ test/functional/messages_controller_test.rb | 11 +++++++++++ test/functional/watchers_controller_test.rb | 19 +++++++++++++++++++ test/functional/wiki_controller_test.rb | 11 +++++++++++ 7 files changed, 67 insertions(+), 1 deletion(-) diff --git a/app/controllers/watchers_controller.rb b/app/controllers/watchers_controller.rb index 4113464bc..b4efa10e9 100644 --- a/app/controllers/watchers_controller.rb +++ b/app/controllers/watchers_controller.rb @@ -156,7 +156,7 @@ class WatchersController < ApplicationController users = scope.sorted.like(params[:q]).to_a if @watchables && @watchables.size == 1 watchable_object = @watchables.first - users -= watchable_object.watcher_users + users -= watchable_object.visible_watcher_users if watchable_object.respond_to?(:visible?) users.reject! {|user| user.is_a?(User) && !watchable_object.visible?(user)} diff --git a/app/views/watchers/_watchers.html.erb b/app/views/watchers/_watchers.html.erb index b53e09d50..25d852c87 100644 --- a/app/views/watchers/_watchers.html.erb +++ b/app/views/watchers/_watchers.html.erb @@ -8,6 +8,10 @@ <% end %> +<% if User.current.allowed_to?(:"view_#{watched_klass_name}_watchers", watched.project) %>

<%= l(:"label_#{watched_klass_name}_watchers") %> (<%= watched.watcher_users.size %>)

<%= watchers_list(watched) %> +<% else %> +

<%= l(:"label_#{watched_klass_name}_watchers") %>

+<% end %> diff --git a/lib/plugins/acts_as_watchable/lib/acts_as_watchable.rb b/lib/plugins/acts_as_watchable/lib/acts_as_watchable.rb index 200702cb9..56612d692 100644 --- a/lib/plugins/acts_as_watchable/lib/acts_as_watchable.rb +++ b/lib/plugins/acts_as_watchable/lib/acts_as_watchable.rb @@ -60,6 +60,16 @@ module Redmine users end + # array of watchers that the given user is allowed to see + def visible_watcher_users(user = User.current) + if user.allowed_to?(:"view_#{self.class.name.underscore}_watchers", project) + watcher_users + else + # without permission, the user can only see themselves (if they're a watcher) + watcher_users & [user] + end + end + # Adds user as a watcher def add_watcher(user) if persisted? diff --git a/test/functional/issues_controller_test.rb b/test/functional/issues_controller_test.rb index cfd6219ba..44ad19f22 100644 --- a/test/functional/issues_controller_test.rb +++ b/test/functional/issues_controller_test.rb @@ -2801,6 +2801,17 @@ class IssuesControllerTest < Redmine::ControllerTest end end + def test_show_should_not_display_watchers_without_permission + @request.session[:user_id] = 2 + Role.find(1).remove_permission! :view_issue_watchers + issue = Issue.find(1) + issue.add_watcher User.find(2) + issue.add_watcher Group.find(10) + get(:show, :params => {:id => 1}) + assert_select 'div#watchers ul', 0 + assert_select 'h3', {text: /Watchers \(\d*\)/, count: 0} + end + def test_show_should_display_watchers_with_gravatars @request.session[:user_id] = 2 issue = Issue.find(1) diff --git a/test/functional/messages_controller_test.rb b/test/functional/messages_controller_test.rb index 12a6d5f29..db91082ff 100644 --- a/test/functional/messages_controller_test.rb +++ b/test/functional/messages_controller_test.rb @@ -114,6 +114,17 @@ class MessagesControllerTest < Redmine::ControllerTest end end + def test_show_should_not_display_watchers_without_permission + @request.session[:user_id] = 2 + Role.find(1).remove_permission! :view_message_watchers + message = Message.find(1) + message.add_watcher User.find(2) + message.add_watcher Group.find(10) + get(:show, :params => {:board_id => 1, :id => 1}) + assert_select 'div#watchers ul', 0 + assert_select 'h3', {text: /Watchers \(\d*\)/, count: 0} + end + def test_get_new @request.session[:user_id] = 2 get(:new, :params => {:board_id => 1}) diff --git a/test/functional/watchers_controller_test.rb b/test/functional/watchers_controller_test.rb index d8719a991..65aad4e1f 100644 --- a/test/functional/watchers_controller_test.rb +++ b/test/functional/watchers_controller_test.rb @@ -211,6 +211,25 @@ class WatchersControllerTest < Redmine::ControllerTest ) end + def test_new_without_view_watchers_permission + @request.session[:user_id] = 2 + Role.find(1).remove_permission! :view_issue_watchers + get :new, :params => {:object_type => 'issue', :object_id => '2'}, :xhr => true + assert_response :success + assert_match %r{name=\\\"watcher\[user_ids\]\[\]\\\" value=\\\"2\\\"}, response.body + # User should not be able to reverse engineer that User 3 is watching the issue already + assert_match %r{name=\\\"watcher\[user_ids\]\[\]\\\" value=\\\"3\\\"}, response.body + end + + def test_new_dont_show_self_when_watching_without_view_watchers_permission + @request.session[:user_id] = 2 + Role.find(1).remove_permission! :view_issue_watchers + Issue.find(2).add_watcher(User.find(2)) + get :new, :params => {:object_type => 'issue', :object_id => '2'}, :xhr => true + assert_response :success + assert_no_match %r{name=\\\"watcher\[user_ids\]\[\]\\\" value=\\\"2\\\"}, response.body + end + def test_create_as_html @request.session[:user_id] = 2 assert_difference('Watcher.count') do diff --git a/test/functional/wiki_controller_test.rb b/test/functional/wiki_controller_test.rb index be62dcc8e..fdfaaf805 100644 --- a/test/functional/wiki_controller_test.rb +++ b/test/functional/wiki_controller_test.rb @@ -150,6 +150,17 @@ class WikiControllerTest < Redmine::ControllerTest end end + def test_show_should_not_display_watchers_without_permission + @request.session[:user_id] = 2 + Role.find(1).remove_permission! :view_wiki_page_watchers + page = Project.find(1).wiki.find_page('Another_page') + page.add_watcher User.find(2) + page.add_watcher Group.find(10) + get(:show, :params => {:project_id => 1, :id => 'Another_page'}) + assert_select 'div#watchers ul', 0 + assert_select 'h3', {text: /Watchers \(\d*\)/, count: 0} + end + def test_show_should_display_section_edit_links with_settings :text_formatting => 'textile' do @request.session[:user_id] = 2 -- 2.39.5