From dfd02040521b84c64e9aa5d7b70ccfa427ffe841 Mon Sep 17 00:00:00 2001 From: Jean-Philippe Lang Date: Sat, 14 Nov 2009 12:08:47 +0000 Subject: [PATCH] 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 --- app/controllers/search_controller.rb | 2 +- app/helpers/application_helper.rb | 2 +- app/views/projects/index.rhtml | 2 +- app/views/repositories/revision.rhtml | 4 +- ...091114105931_add_view_issues_permission.rb | 13 ++++++ lib/redmine.rb | 2 +- lib/redmine/default_data/loader.rb | 10 +++-- test/fixtures/roles.yml | 5 +++ test/functional/issues_controller_test.rb | 20 +++++++++ test/unit/issue_test.rb | 43 ++++++++++++++++++- 10 files changed, 93 insertions(+), 10 deletions(-) create mode 100644 db/migrate/20091114105931_add_view_issues_permission.rb diff --git a/app/controllers/search_controller.rb b/app/controllers/search_controller.rb index 485d2349d..68687347e 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 cacfe2986..6fca5ea7c 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 3b2435799..11e9bcd33 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 f992f046d..b7bda4530 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 000000000..5092a8c9d --- /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 ebec9060b..1b88ee10f 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 435215b1d..a88466727 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 08699ff83..22f690328 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 6786b02e7..1cff860b4 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 a8010cf48..84ccef601 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') -- 2.39.5