Improve watcher list permissions check to explicitly require @view_issue_watchers...

Patch by Felix Schäfer (@felix).

git-svn-id: https://svn.redmine.org/redmine/trunk@22913 e93f8b46-1217-0410-a6f0-8f06a7374b81

diff --git a/app/controllers/watchers_controller.rb b/app/controllers/watchers_controller.rb
index 4113464bc2aeeac78ec4b15825ccfd3eba7f002a..b4efa10e93c7968292629813b207cd76505620ee 100644 (file)
--- 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 b53e09d500d3012cb0ae6d423405c5b05028a8ee..25d852c876771497cc4cd00fc561dff18aa74b50 100644 (file)
--- a/app/views/watchers/_watchers.html.erb
+++ b/app/views/watchers/_watchers.html.erb
@@ -8,6 +8,10 @@
 </div>
 <% end %>
 
+<% if User.current.allowed_to?(:"view_#{watched_klass_name}_watchers", watched.project) %>
 <h3><%= l(:"label_#{watched_klass_name}_watchers") %> (<%= watched.watcher_users.size %>)</h3>
 
 <%= watchers_list(watched) %>
+<% else %>
+<h3><%= l(:"label_#{watched_klass_name}_watchers") %></h3>
+<% 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 200702cb9e956c84b61d75bc5b2609bfe61f8226..56612d69205e16bc1f66737c61a959683f1f401c 100644 (file)
--- 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 cfd6219ba561e24af57c9466bf282974a0b037db..44ad19f224ec42abc5ca6ad1a4835623f4940825 100644 (file)
--- 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 12a6d5f29a6caf27c998dfe021efa80e8639522a..db91082ff6a732dc8413bb6503751c68b87daa63 100644 (file)
--- 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 d8719a991afae3a2ea65cbad19aab1ff121fa2c2..65aad4e1f38fc56e40f02758bf82ab7fb1b5b090 100644 (file)
--- 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 be62dcc8e3e1b69d037e17f7c4f88740ba1794fb..fdfaaf8050627026d472e12ec34784c0596a15f3 100644 (file)
--- 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