Fixed: search engine may reveal private projects (#1613).

git-svn-id: http://redmine.rubyforge.org/svn/trunk@1649 e93f8b46-1217-0410-a6f0-8f06a7374b81

4 files changed, 147 insertions, 10 deletions

diff --git a/app/models/journal.rb b/app/models/journal.rb
index 67a3eee3b..8583f63de 100644
--- a/app/models/journal.rb
+++ b/app/models/journal.rb
@@ -28,7 +28,8 @@ class Journal < ActiveRecord::Base
   acts_as_searchable :columns => 'notes',
                      :include => {:issue => :project},
                      :project_key => "#{Issue.table_name}.project_id",
-                     :date_column => "#{Issue.table_name}.created_on"
+                     :date_column => "#{Issue.table_name}.created_on",
+                     :permission => :view_issues
   
   acts_as_event :title => Proc.new {|o| status = ((s = o.new_status) ? " (#{s})" : nil); "#{o.issue.tracker} ##{o.issue.id}#{status}: #{o.issue.subject}" },
                 :description => :notes,
diff --git a/app/models/project.rb b/app/models/project.rb
index a5ba246b1..67e6c0e39 100644
--- a/app/models/project.rb
+++ b/app/models/project.rb
@@ -112,16 +112,18 @@ class Project < ActiveRecord::Base
     end
     if user.admin?
       # no restriction
-    elsif user.logged?
-      statements << "#{Project.table_name}.is_public = #{connection.quoted_true}" if Role.non_member.allowed_to?(permission)
-      allowed_project_ids = user.memberships.select {|m| m.role.allowed_to?(permission)}.collect {|m| m.project_id}
-      statements << "#{Project.table_name}.id IN (#{allowed_project_ids.join(',')})" if allowed_project_ids.any?
-    elsif Role.anonymous.allowed_to?(permission)
-      # anonymous user allowed on public project
-      statements << "#{Project.table_name}.is_public = #{connection.quoted_true}" 
     else
-      # anonymous user is not authorized
       statements << "1=0"
+      if user.logged?
+        statements << "#{Project.table_name}.is_public = #{connection.quoted_true}" if Role.non_member.allowed_to?(permission)
+        allowed_project_ids = user.memberships.select {|m| m.role.allowed_to?(permission)}.collect {|m| m.project_id}
+        statements << "#{Project.table_name}.id IN (#{allowed_project_ids.join(',')})" if allowed_project_ids.any?
+      elsif Role.anonymous.allowed_to?(permission)
+        # anonymous user allowed on public project
+        statements << "#{Project.table_name}.is_public = #{connection.quoted_true}" 
+      else
+        # anonymous user is not authorized
+      end
     end
     statements.empty? ? base_statement : "((#{base_statement}) AND (#{statements.join(' OR ')}))"
   end
diff --git a/test/unit/search_test.rb b/test/unit/search_test.rb
new file mode 100644
index 000000000..e2cc8e766
--- /dev/null
+++ b/test/unit/search_test.rb
@@ -0,0 +1,134 @@
+# redMine - project management software
+# Copyright (C) 2006-2008  Jean-Philippe Lang
+#
+# 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.
+
+require File.dirname(__FILE__) + '/../test_helper'
+
+class SearchTest < Test::Unit::TestCase
+  fixtures :users,
+           :members,
+           :projects,
+           :roles,
+           :enabled_modules,
+           :issues,
+           :trackers,
+           :journals,
+           :journal_details,
+           :repositories,
+           :changesets
+
+  def setup
+    @project = Project.find(1)
+    @issue_keyword = '%Unable to print recipes%'
+    @issue = Issue.find(1)
+    @changeset_keyword = '%very first commit%'
+    @changeset = Changeset.find(100)
+  end
+  
+  def test_search_by_anonymous
+    User.current = nil
+    
+    r = Issue.search(@issue_keyword)
+    assert r.include?(@issue)
+    r = Changeset.search(@changeset_keyword)
+    assert r.include?(@changeset)
+    
+    # Removes the :view_changesets permission from Anonymous role
+    remove_permission Role.anonymous, :view_changesets
+    
+    r = Issue.search(@issue_keyword)
+    assert r.include?(@issue)
+    r = Changeset.search(@changeset_keyword)
+    assert !r.include?(@changeset)
+    
+    # Make the project private
+    @project.update_attribute :is_public, false
+    r = Issue.search(@issue_keyword)
+    assert !r.include?(@issue)
+    r = Changeset.search(@changeset_keyword)
+    assert !r.include?(@changeset)
+  end
+  
+  def test_search_by_user
+    User.current = User.find_by_login('rhill')
+    assert User.current.memberships.empty?
+    
+    r = Issue.search(@issue_keyword)
+    assert r.include?(@issue)
+    r = Changeset.search(@changeset_keyword)
+    assert r.include?(@changeset)
+    
+    # Removes the :view_changesets permission from Non member role
+    remove_permission Role.non_member, :view_changesets
+    
+    r = Issue.search(@issue_keyword)
+    assert r.include?(@issue)
+    r = Changeset.search(@changeset_keyword)
+    assert !r.include?(@changeset)
+    
+    # Make the project private
+    @project.update_attribute :is_public, false
+    r = Issue.search(@issue_keyword)
+    assert !r.include?(@issue)
+    r = Changeset.search(@changeset_keyword)
+    assert !r.include?(@changeset)
+  end
+  
+  def test_search_by_allowed_member
+    User.current = User.find_by_login('jsmith')
+    assert User.current.projects.include?(@project)
+    
+    r = Issue.search(@issue_keyword)
+    assert r.include?(@issue)
+    r = Changeset.search(@changeset_keyword)
+    assert r.include?(@changeset)
+
+    # Make the project private
+    @project.update_attribute :is_public, false
+    r = Issue.search(@issue_keyword)
+    assert r.include?(@issue)
+    r = Changeset.search(@changeset_keyword)
+    assert r.include?(@changeset)
+  end
+
+  def test_search_by_unallowed_member
+    # Removes the :view_changesets permission from user's and non member role
+    remove_permission Role.find(1), :view_changesets
+    remove_permission Role.non_member, :view_changesets
+    
+    User.current = User.find_by_login('jsmith')
+    assert User.current.projects.include?(@project)
+    
+    r = Issue.search(@issue_keyword)
+    assert r.include?(@issue)
+    r = Changeset.search(@changeset_keyword)
+    assert !r.include?(@changeset)
+
+    # Make the project private
+    @project.update_attribute :is_public, false
+    r = Issue.search(@issue_keyword)
+    assert r.include?(@issue)
+    r = Changeset.search(@changeset_keyword)
+    assert !r.include?(@changeset)
+  end
+  
+  private
+  
+  def remove_permission(role, permission)
+    role.permissions = role.permissions - [ permission ]
+    role.save
+  end
+end
diff --git a/vendor/plugins/acts_as_searchable/lib/acts_as_searchable.rb b/vendor/plugins/acts_as_searchable/lib/acts_as_searchable.rb
index 1eb64c30f..2c773d70a 100644
--- a/vendor/plugins/acts_as_searchable/lib/acts_as_searchable.rb
+++ b/vendor/plugins/acts_as_searchable/lib/acts_as_searchable.rb
@@ -67,7 +67,7 @@ module Redmine
         module ClassMethods
           # Search the model for the given tokens
           # projects argument can be either nil (will search all projects), a project or an array of projects
-          def search(tokens, projects, options={})
+          def search(tokens, projects=nil, options={})
             tokens = [] << tokens unless tokens.is_a?(Array)
             projects = [] << projects unless projects.nil? || projects.is_a?(Array)