Add view_issues permission (#3187).

A migration adds this permission to all existing roles to preserve current behaviour. This permission controls access to issues, roadmap and changelog. git-svn-id: svn+ssh://rubyforge.org/var/svn/redmine/trunk@3039 e93f8b46-1217-0410-a6f0-8f06a7374b81

Add view_issues permission (#3187).
A migration adds this permission to all existing roles to preserve current behaviour. This permission controls access to issues, roadmap and changelog. git-svn-id: svn+ssh://rubyforge.org/var/svn/redmine/trunk@3039 e93f8b46-1217-0410-a6f0-8f06a7374b81
dfd02040 · Jean-Philippe Lang · 326ed79b · dfd02040 · dfd02040 · dfd02040
Commit dfd02040 authored Nov 14, 2009 by Jean-Philippe Lang
10 changed files
--- a/app/controllers/search_controller.rb
+++ b/app/controllers/search_controller.rb
@@ -43,7 +43,7 @@ class SearchController < ApplicationController
    begin; offset = params[:offset].to_time if params[:offset]; rescue; end
    
    # quick jump to an issue
-    if @question.match(/^#?(\d+)$/) && Issue.find_by_id($1, :include => :project, :conditions => Project.visible_by(User.current))
+    if @question.match(/^#?(\d+)$/) && Issue.visible.find_by_id($1)
      redirect_to :controller => "issues", :action => "show", :id => $1
      return
    end

--- a/app/helpers/application_helper.rb
+++ b/app/helpers/application_helper.rb
@@ -480,7 +480,7 @@ module ApplicationHelper
          oid = oid.to_i
          case prefix
          when nil
-            if issue = Issue.find_by_id(oid, :include => [:project, :status], :conditions => Project.visible_by(User.current))
+            if issue = Issue.visible.find_by_id(oid, :include => :status)
              link = link_to("##{oid}", {:only_path => only_path, :controller => 'issues', :action => 'show', :id => oid},
                                        :class => (issue.closed? ? 'issue closed' : 'issue'),
                                        :title => "#{truncate(issue.subject, :length => 100)} (#{issue.status.name})")

--- a/app/views/projects/index.rhtml
+++ b/app/views/projects/index.rhtml
 <div class="contextual">
    <%= link_to(l(:label_project_new), {:controller => 'projects', :action => 'add'}, :class => 'icon icon-add') + ' |' if User.current.allowed_to?(:add_project, nil, :global => true) %>
-    <%= link_to l(:label_issue_view_all), { :controller => 'issues' } %> |
+    <%= link_to(l(:label_issue_view_all), { :controller => 'issues' }) + ' |' if User.current.allowed_to?(:view_issues, nil, :global => true) %>
    <%= link_to l(:label_overall_activity), { :controller => 'projects', :action => 'activity' }%>
 </div>


--- a/app/views/repositories/revision.rhtml
+++ b/app/views/repositories/revision.rhtml
@@ -26,10 +26,10 @@

 <%= textilizable @changeset.comments %>

-<% if @changeset.issues.any? %>
+<% if @changeset.issues.visible.any? %>
 <h3><%= l(:label_related_issues) %></h3>
 <ul>
-<% @changeset.issues.each do |issue| %>
+<% @changeset.issues.visible.each do |issue| %>
  <li><%= link_to_issue issue %>: <%=h issue.subject %></li>
 <% end %>
 </ul>

--- a/db/migrate/20091114105931_add_view_issues_permission.rb
+++ b/db/migrate/20091114105931_add_view_issues_permission.rb
+class AddViewIssuesPermission < ActiveRecord::Migration
+  def self.up
+    Role.find(:all).each do |r|
+      r.add_permission!(:view_issues)
+    end
+  end
+
+  def self.down
+    Role.find(:all).each do |r|
+      r.remove_permission!(:view_issues)
+    end
+  end
+end
--- a/lib/redmine.rb
+++ b/lib/redmine.rb
@@ -41,7 +41,7 @@ Redmine::AccessControl.map do |map|
                                  :issues => [:index, :changes, :show, :context_menu],
                                  :versions => [:show, :status_by],
                                  :queries => :index,
-                                  :reports => :issue_report}, :public => true                    
+                                  :reports => :issue_report}
    map.permission :add_issues, {:issues => :new}
    map.permission :edit_issues, {:issues => [:edit, :reply, :bulk_edit]}
    map.permission :manage_issue_relations, {:issue_relations => [:new, :destroy]}

--- a/lib/redmine/default_data/loader.rb
+++ b/lib/redmine/default_data/loader.rb
@@ -49,6 +49,7 @@ module Redmine
                                      :position => 2, 
                                      :permissions => [:manage_versions, 
                                                      :manage_categories,
+                                                      :view_issues,
                                                      :add_issues,
                                                      :edit_issues,
                                                      :manage_issue_relations,
@@ -74,7 +75,8 @@ module Redmine
            
            reporter = Role.create! :name => l(:default_role_reporter),
                                    :position => 3,
-                                    :permissions => [:add_issues,
+                                    :permissions => [:view_issues,
+                                                    :add_issues,
                                                    :add_issue_notes,
                                                    :save_queries,
                                                    :view_gantt,
@@ -91,7 +93,8 @@ module Redmine
                                                    :browse_repository,
                                                    :view_changesets]
                        
-            Role.non_member.update_attribute :permissions, [:add_issues,
+            Role.non_member.update_attribute :permissions, [:view_issues,
+                                                            :add_issues,
                                                            :add_issue_notes,
                                                            :save_queries,
                                                            :view_gantt,
@@ -106,7 +109,8 @@ module Redmine
                                                            :browse_repository,
                                                            :view_changesets]
          
-            Role.anonymous.update_attribute :permissions, [:view_gantt,
+            Role.anonymous.update_attribute :permissions, [:view_issues,
+                                                           :view_gantt,
                                                           :view_calendar,
                                                           :view_time_entries,
                                                           :view_documents,

--- a/test/fixtures/roles.yml
+++ b/test/fixtures/roles.yml
@@ -10,6 +10,7 @@ roles_001:
    - :manage_members
    - :manage_versions
    - :manage_categories
+    - :view_issues
    - :add_issues
    - :edit_issues
    - :manage_issue_relations
@@ -60,6 +61,7 @@ roles_002:
    - :manage_members
    - :manage_versions
    - :manage_categories
+    - :view_issues
    - :add_issues
    - :edit_issues
    - :manage_issue_relations
@@ -102,6 +104,7 @@ roles_003:
    - :manage_members
    - :manage_versions
    - :manage_categories
+    - :view_issues
    - :add_issues
    - :edit_issues
    - :manage_issue_relations
@@ -135,6 +138,7 @@ roles_004:
  builtin: 1
  permissions: |
    --- 
+    - :view_issues
    - :add_issues
    - :edit_issues
    - :manage_issue_relations
@@ -164,6 +168,7 @@ roles_005:
  builtin: 2
  permissions: |
    --- 
+    - :view_issues
    - :add_issue_notes
    - :view_gantt
    - :view_calendar

--- a/test/functional/issues_controller_test.rb
+++ b/test/functional/issues_controller_test.rb
@@ -358,6 +358,26 @@ class IssuesControllerTest < ActionController::TestCase
                                            :content => /Notes/ } }
  end
  
+  def test_show_should_deny_anonymous_access_without_permission
+    Role.anonymous.remove_permission!(:view_issues)
+    get :show, :id => 1
+    assert_response :redirect
+  end
+  
+  def test_show_should_deny_non_member_access_without_permission
+    Role.non_member.remove_permission!(:view_issues)
+    @request.session[:user_id] = 9
+    get :show, :id => 1
+    assert_response 403
+  end
+  
+  def test_show_should_deny_member_access_without_permission
+    Role.find(1).remove_permission!(:view_issues)
+    @request.session[:user_id] = 2
+    get :show, :id => 1
+    assert_response 403
+  end
+  
  def test_show_should_not_disclose_relations_to_invisible_issues
    Setting.cross_project_issue_relations = '1'
    IssueRelation.create!(:issue_from => Issue.find(1), :issue_to => Issue.find(2), :relation_type => 'relates')

--- a/test/unit/issue_test.rb
+++ b/test/unit/issue_test.rb
@@ -18,7 +18,7 @@
 require File.dirname(__FILE__) + '/../test_helper'

 class IssueTest < ActiveSupport::TestCase
-  fixtures :projects, :users, :members, :member_roles,
+  fixtures :projects, :users, :members, :member_roles, :roles,
           :trackers, :projects_trackers,
           :versions,
           :issue_statuses, :issue_categories, :issue_relations, :workflows, 
@@ -64,6 +64,47 @@ class IssueTest < ActiveSupport::TestCase
    assert_equal 'PostgreSQL', issue.custom_value_for(field).value
  end
  
+  def test_visible_scope_for_anonymous
+    # Anonymous user should see issues of public projects only
+    issues = Issue.visible(User.anonymous).all
+    assert issues.any?
+    assert_nil issues.detect {|issue| !issue.project.is_public?}
+    # Anonymous user should not see issues without permission
+    Role.anonymous.remove_permission!(:view_issues)
+    issues = Issue.visible(User.anonymous).all
+    assert issues.empty?
+  end
+  
+  def test_visible_scope_for_user
+    user = User.find(9)
+    assert user.projects.empty?
+    # Non member user should see issues of public projects only
+    issues = Issue.visible(user).all
+    assert issues.any?
+    assert_nil issues.detect {|issue| !issue.project.is_public?}
+    # Non member user should not see issues without permission
+    Role.non_member.remove_permission!(:view_issues)
+    user.reload
+    issues = Issue.visible(user).all
+    assert issues.empty?
+    # User should see issues of projects for which he has view_issues permissions only
+    Member.create!(:principal => user, :project_id => 2, :role_ids => [1])
+    user.reload
+    issues = Issue.visible(user).all
+    assert issues.any?
+    assert_nil issues.detect {|issue| issue.project_id != 2}
+  end
+  
+  def test_visible_scope_for_admin
+    user = User.find(1)
+    user.members.each(&:destroy)
+    assert user.projects.empty?
+    issues = Issue.visible(user).all
+    assert issues.any?
+    # Admin should see issues on private projects that he does not belong to
+    assert issues.detect {|issue| !issue.project.is_public?}
+  end
+  
  def test_errors_full_messages_should_include_custom_fields_errors
    field = IssueCustomField.find_by_name('Database')