diff --git a/app/controllers/search_controller.rb b/app/controllers/search_controller.rb index 485d2349..68687347 100644 --- 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 diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb index cacfe298..6fca5ea7 100644 --- 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})") diff --git a/app/views/projects/index.rhtml b/app/views/projects/index.rhtml index 3b243579..11e9bcd3 100644 --- a/app/views/projects/index.rhtml +++ b/app/views/projects/index.rhtml @@ -1,6 +1,6 @@
<%= 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' }%>
diff --git a/app/views/repositories/revision.rhtml b/app/views/repositories/revision.rhtml index f992f046..b7bda453 100644 --- 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? %>

<%= l(:label_related_issues) %>

diff --git a/db/migrate/20091114105931_add_view_issues_permission.rb b/db/migrate/20091114105931_add_view_issues_permission.rb new file mode 100644 index 00000000..5092a8c9 --- /dev/null +++ b/db/migrate/20091114105931_add_view_issues_permission.rb @@ -0,0 +1,13 @@ +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 diff --git a/lib/redmine.rb b/lib/redmine.rb index ebec9060..1b88ee10 100644 --- 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]} diff --git a/lib/redmine/default_data/loader.rb b/lib/redmine/default_data/loader.rb index 435215b1..a8846672 100644 --- 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, diff --git a/test/fixtures/roles.yml b/test/fixtures/roles.yml index 08699ff8..22f69032 100644 --- 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 diff --git a/test/functional/issues_controller_test.rb b/test/functional/issues_controller_test.rb index 6786b02e..1cff860b 100644 --- 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') diff --git a/test/unit/issue_test.rb b/test/unit/issue_test.rb index a8010cf4..84ccef60 100644 --- 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')