Adds an issues visibility level on roles (#7412).

It can be set so that users only see their own issues (created or assigned).

git-svn-id: svn+ssh://rubyforge.org/var/svn/redmine/trunk@5416 e93f8b46-1217-0410-a6f0-8f06a7374b81

app/controllers/application_controller.rb

@@ -1,5 +1,5 @@
-# redMine - project management software
-# Copyright (C) 2006-2007  Jean-Philippe Lang
+# Redmine - project management software
+# Copyright (C) 2006-2011  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
@@ -221,6 +221,10 @@ class ApplicationController < ActionController::Base
   def find_issues
     @issues = Issue.find_all_by_id(params[:id] || params[:ids])
     raise ActiveRecord::RecordNotFound if @issues.empty?
+    if @issues.detect {|issue| !issue.visible?}
+      deny_access
+      return
+    end
     @projects = @issues.collect(&:project).compact.uniq
     @project = @projects.first if @projects.size == 1
   rescue ActiveRecord::RecordNotFound

app/controllers/issues_controller.rb

@@ -1,5 +1,5 @@
 # Redmine - project management software
-# Copyright (C) 2006-2008  Jean-Philippe Lang
+# Copyright (C) 2006-2011  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
@@ -251,7 +251,13 @@ class IssuesController < ApplicationController
 
 private
   def find_issue
+    # Issue.visible.find(...) can not be used to redirect user to the login form
+    # if the issue actually exists but requires authentication
     @issue = Issue.find(params[:id], :include => [:project, :tracker, :status, :author, :priority, :category])
+    unless @issue.visible?
+      deny_access
+      return
+    end
     @project = @issue.project
   rescue ActiveRecord::RecordNotFound
     render_404

app/models/issue.rb

@@ -88,12 +88,30 @@ class Issue < ActiveRecord::Base
   
   # Returns a SQL conditions string used to find all issues visible by the specified user
   def self.visible_condition(user, options={})
-    Project.allowed_to_condition(user, :view_issues, options)
+    Project.allowed_to_condition(user, :view_issues, options) do |role, user|
+      case role.issues_visibility
+      when 'default'
+        nil
+      when 'own'
+        "(#{table_name}.author_id = #{user.id} OR #{table_name}.assigned_to_id = #{user.id})"
+      else
+        '1=0'
+      end
+    end
   end
 
   # Returns true if usr or current user is allowed to view the issue
   def visible?(usr=nil)
-    (usr || User.current).allowed_to?(:view_issues, self.project)
+    (usr || User.current).allowed_to?(:view_issues, self.project) do |role, user|
+      case role.issues_visibility
+      when 'default'
+        true
+      when 'own'
+        self.author == user || self.assigned_to == user
+      else
+        false
+      end
+    end
   end
   
   def after_initialize

app/models/project.rb

@@ -174,6 +174,13 @@ class Project < ActiveRecord::Base
       if statement_by_role.empty?
         "1=0"
       else
+        if block_given?
+          statement_by_role.each do |role, statement|
+            if s = yield(role, user)
+              statement_by_role[role] = "(#{statement} AND (#{s}))"
+            end
+          end
+        end
         "((#{base_statement}) AND (#{statement_by_role.values.join(' OR ')}))"
       end
     end

app/models/role.rb

@@ -20,6 +20,11 @@ class Role < ActiveRecord::Base
   BUILTIN_NON_MEMBER = 1
   BUILTIN_ANONYMOUS  = 2
   
+  ISSUES_VISIBILITY_OPTIONS = [
+    ['default', :label_issues_visibility_all],
+    ['own', :label_issues_visibility_own]
+  ]
+
   named_scope :givable, { :conditions => "builtin = 0", :order => 'position' }
   named_scope :builtin, lambda { |*args|
     compare = 'not' if args.first == true
@@ -43,6 +48,9 @@ class Role < ActiveRecord::Base
   validates_presence_of :name
   validates_uniqueness_of :name
   validates_length_of :name, :maximum => 30
+  validates_inclusion_of :issues_visibility,
+    :in => ISSUES_VISIBILITY_OPTIONS.collect(&:first),
+    :if => lambda {|role| role.respond_to?(:issues_visibility)}
   
   def permissions
     read_attribute(:permissions) || []

app/models/user.rb

@@ -394,10 +394,10 @@ class User < Principal
   # * a permission Symbol (eg. :edit_project)
   # Context can be:
   # * a project : returns true if user is allowed to do the specified action on this project
-  # * a group of projects : returns true if user is allowed on every project
+  # * an array of projects : returns true if user is allowed on every project
   # * nil with options[:global] set : check if user has at least one role allowed for this action, 
   #   or falls back to Non Member / Anonymous permissions depending if the user is logged
-  def allowed_to?(action, context, options={})
+  def allowed_to?(action, context, options={}, &block)
     if context && context.is_a?(Project)
       # No action allowed on archived projects
       return false unless context.active?
@@ -408,12 +408,15 @@ class User < Principal
       
       roles = roles_for_project(context)
       return false unless roles
-      roles.detect {|role| (context.is_public? || role.member?) && role.allowed_to?(action)}
-      
+      roles.detect {|role|
+        (context.is_public? || role.member?) &&
+        role.allowed_to?(action) &&
+        (block_given? ? yield(role, self) : true)
+      }
     elsif context && context.is_a?(Array)
       # Authorize if user is authorized on every element of the array
       context.map do |project|
-        allowed_to?(action,project,options)
+        allowed_to?(action, project, options, &block)
       end.inject do |memo,allowed|
         memo && allowed
       end
@@ -423,7 +426,11 @@ class User < Principal
       
       # authorize if user has at least one role that has this permission
       roles = memberships.collect {|m| m.roles}.flatten.uniq
-      roles.detect {|r| r.allowed_to?(action)} || (self.logged? ? Role.non_member.allowed_to?(action) : Role.anonymous.allowed_to?(action))
+      roles << (self.logged? ? Role.non_member : Role.anonymous)
+      roles.detect {|role|
+        role.allowed_to?(action) &&
+        (block_given? ? yield(role, self) : true)
+      }
     else
       false
     end
@@ -431,8 +438,8 @@ class User < Principal
 
   # Is the user allowed to do the specified action on any project?
   # See allowed_to? for the actions and valid options.
-  def allowed_to_globally?(action, options)
-    allowed_to?(action, nil, options.reverse_merge(:global => true))
+  def allowed_to_globally?(action, options, &block)
+    allowed_to?(action, nil, options.reverse_merge(:global => true), &block)
   end
 
   safe_attributes 'login',

app/views/roles/_form.rhtml

@@ -1,15 +1,16 @@
 <%= error_messages_for 'role' %> 
 
-<% unless @role.builtin? %>
 <div class="box">
+<% unless @role.builtin? %>
 <p><%= f.text_field :name, :required => true %></p>
 <p><%= f.check_box :assignable %></p>
+<% end %>
+<p><%= f.select :issues_visibility, Role::ISSUES_VISIBILITY_OPTIONS.collect {|v| [l(v.last), v.first]} %></p>
 <% if @role.new_record? && @roles.any? %>
 <p><label><%= l(:label_copy_workflow_from) %></label>
 <%= select_tag(:copy_workflow_from, content_tag("option") + options_from_collection_for_select(@roles, :id, :name)) %></p>
 <% end %>
 </div>
-<% end %>
 
 <h3><%= l(:label_permissions) %></h3>
 <div class="box" id="permissions">

config/locales/en.yml

@@ -304,6 +304,7 @@ en:
   field_text: Text field
   field_visible: Visible
   field_warn_on_leaving_unsaved: "Warn me when leaving a page with unsaved text"
+  field_issues_visibility: Issues visibility
   
   setting_app_title: Application title
   setting_app_subtitle: Application subtitle
@@ -804,6 +805,8 @@ en:
   label_user_search: "Search for user:"
   label_additional_workflow_transitions_for_author: Additional transitions allowed when the user is the author
   label_additional_workflow_transitions_for_assignee: Additional transitions allowed when the user is the assignee
+  label_issues_visibility_all: All issues
+  label_issues_visibility_own: Issues created by or assigned to the user
   
   button_login: Login
   button_submit: Submit

config/locales/fr.yml

@@ -308,6 +308,7 @@ fr:
   field_parent_issue: Tâche parente
   field_visible: Visible
   field_warn_on_leaving_unsaved: "M'avertir lorsque je quitte une page contenant du texte non sauvegardé"
+  field_issues_visibility: Visibilité des demandes
   
   setting_app_title: Titre de l'application
   setting_app_subtitle: Sous-titre de l'application
@@ -791,6 +792,8 @@ fr:
   label_user_search: "Rechercher un utilisateur :"
   label_additional_workflow_transitions_for_author: Autorisations supplémentaires lorsque l'utilisateur a créé la demande
   label_additional_workflow_transitions_for_assignee: Autorisations supplémentaires lorsque la demande est assignée à l'utilisateur
+  label_issues_visibility_all: Toutes les demandes
+  label_issues_visibility_own: Demandes créées par ou assignées à l'utilisateur
   
   button_login: Connexion
   button_submit: Soumettre

db/migrate/20110408103312_add_roles_issues_visibility.rb

@@ -0,0 +1,9 @@
+class AddRolesIssuesVisibility < ActiveRecord::Migration
+  def self.up
+    add_column :roles, :issues_visibility, :string, :limit => 30, :default => 'default', :null => false
+  end
+
+  def self.down
+    remove_column :roles, :issues_visibility
+  end
+end

test/fixtures/roles.yml

@@ -3,6 +3,7 @@ roles_001:
   name: Manager
   id: 1
   builtin: 0
+  issues_visibility: default
   permissions: |
     --- 
     - :add_project
@@ -58,6 +59,7 @@ roles_002:
   name: Developer
   id: 2
   builtin: 0
+  issues_visibility: default
   permissions: |
     --- 
     - :edit_project
@@ -102,6 +104,7 @@ roles_003:
   name: Reporter
   id: 3
   builtin: 0
+  issues_visibility: default
   permissions: |
     --- 
     - :edit_project
@@ -140,6 +143,7 @@ roles_004:
   name: Non member
   id: 4
   builtin: 1
+  issues_visibility: default
   permissions: |
     --- 
     - :view_issues
@@ -170,6 +174,7 @@ roles_005:
   name: Anonymous
   id: 5
   builtin: 2
+  issues_visibility: default
   permissions: |
     --- 
     - :view_issues

test/unit/issue_test.rb

@@ -65,35 +65,76 @@ class IssueTest < ActiveSupport::TestCase
     assert_equal 'PostgreSQL', issue.custom_value_for(field).value
   end
   
+  def assert_visibility_match(user, issues)
+    assert_equal issues.collect(&:id).sort, Issue.all.select {|issue| issue.visible?(user)}.collect(&:id).sort
+  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?}
+    assert_visibility_match User.anonymous, issues
+  end
+  
+  def test_visible_scope_for_anonymous_with_own_issues_visibility
+    Role.anonymous.update_attribute :issues_visibility, 'own'
+    Issue.create!(:project_id => 1, :tracker_id => 1, :author_id => User.anonymous.id, :subject => 'Issue by anonymous')
+    
+    issues = Issue.visible(User.anonymous).all
+    assert issues.any?
+    assert_nil issues.detect {|issue| issue.author != User.anonymous}
+    assert_visibility_match User.anonymous, issues
+  end
+  
+  def test_visible_scope_for_anonymous_without_view_issues_permissions
     # Anonymous user should not see issues without permission
     Role.anonymous.remove_permission!(:view_issues)
     issues = Issue.visible(User.anonymous).all
     assert issues.empty?
+    assert_visibility_match User.anonymous, issues
   end
   
-  def test_visible_scope_for_user
+  def test_visible_scope_for_non_member
     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?}
+    assert_visibility_match user, issues
+  end
+  
+  def test_visible_scope_for_non_member_with_own_issues_visibility
+    Role.non_member.update_attribute :issues_visibility, 'own'
+    Issue.create!(:project_id => 1, :tracker_id => 1, :author_id => 9, :subject => 'Issue by non member')
+    user = User.find(9)
+    
+    issues = Issue.visible(user).all
+    assert issues.any?
+    assert_nil issues.detect {|issue| issue.author != user}
+    assert_visibility_match user, issues
+  end
+  
+  def test_visible_scope_for_non_member_without_view_issues_permissions
     # Non member user should not see issues without permission
     Role.non_member.remove_permission!(:view_issues)
-    user.reload
+    user = User.find(9)
+    assert user.projects.empty?
     issues = Issue.visible(user).all
     assert issues.empty?
+    assert_visibility_match user, issues
+  end
+  
+  def test_visible_scope_for_member
+    user = User.find(9)
     # User should see issues of projects for which he has view_issues permissions only
+    Role.non_member.remove_permission!(:view_issues)
     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}
+    assert_visibility_match user, issues
   end
   
   def test_visible_scope_for_admin
@@ -104,6 +145,7 @@ class IssueTest < ActiveSupport::TestCase
     assert issues.any?
     # Admin should see issues on private projects that he does not belong to
     assert issues.detect {|issue| !issue.project.is_public?}
+    assert_visibility_match user, issues
   end
   
   def test_visible_scope_with_project