diff options
-rw-r--r-- | app/controllers/users_controller.rb | 12 | ||||
-rw-r--r-- | app/controllers/watchers_controller.rb | 16 | ||||
-rw-r--r-- | app/models/issue_query.rb | 8 | ||||
-rw-r--r-- | app/models/principal.rb | 28 | ||||
-rw-r--r-- | app/models/role.rb | 8 | ||||
-rw-r--r-- | app/models/time_entry_query.rb | 6 | ||||
-rw-r--r-- | app/models/user.rb | 6 | ||||
-rw-r--r-- | app/views/roles/_form.html.erb | 26 | ||||
-rw-r--r-- | config/locales/en.yml | 3 | ||||
-rw-r--r-- | config/locales/fr.yml | 3 | ||||
-rw-r--r-- | db/migrate/20141109112308_add_roles_users_visibility.rb | 9 | ||||
-rw-r--r-- | lib/redmine/default_data/loader.rb | 1 | ||||
-rw-r--r-- | test/fixtures/roles.yml | 5 | ||||
-rw-r--r-- | test/functional/users_controller_test.rb | 15 | ||||
-rw-r--r-- | test/functional/watchers_controller_test.rb | 15 | ||||
-rw-r--r-- | test/unit/principal_test.rb | 23 |
16 files changed, 145 insertions, 39 deletions
diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index d14914af4..d62bea449 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -60,19 +60,17 @@ class UsersController < ApplicationController end def show + unless @user.visible? + render_404 + return + end + # show projects based on current user visibility @memberships = @user.memberships.where(Project.visible_condition(User.current)).to_a events = Redmine::Activity::Fetcher.new(User.current, :author => @user).events(nil, nil, :limit => 10) @events_by_day = events.group_by(&:event_date) - unless User.current.admin? - if !@user.active? || (@user != User.current && @memberships.empty? && events.empty?) - render_404 - return - end - end - respond_to do |format| format.html { render :layout => 'base' } format.api diff --git a/app/controllers/watchers_controller.rb b/app/controllers/watchers_controller.rb index ade977b41..060c09f6c 100644 --- a/app/controllers/watchers_controller.rb +++ b/app/controllers/watchers_controller.rb @@ -40,8 +40,9 @@ class WatchersController < ApplicationController else user_ids << params[:user_id] end - user_ids.flatten.compact.uniq.each do |user_id| - Watcher.create(:watchable => @watched, :user_id => user_id) + users = User.active.visible.where(:id => user_ids.flatten.compact.uniq) + users.each do |user| + Watcher.create(:watchable => @watched, :user => user) end respond_to do |format| format.html { redirect_to_referer_or {render :text => 'Watcher added.', :layout => true}} @@ -53,7 +54,7 @@ class WatchersController < ApplicationController def append if params[:watcher].is_a?(Hash) user_ids = params[:watcher][:user_ids] || [params[:watcher][:user_id]] - @users = User.active.where(:id => user_ids).to_a + @users = User.active.visible.where(:id => user_ids).to_a end if @users.blank? render :nothing => true @@ -61,7 +62,7 @@ class WatchersController < ApplicationController end def destroy - @watched.set_watcher(User.find(params[:user_id]), false) + @watched.set_watcher(User.visible.find(params[:user_id]), false) respond_to do |format| format.html { redirect_to :back } format.js @@ -115,12 +116,13 @@ class WatchersController < ApplicationController end def users_for_new_watcher - users = [] + scope = nil if params[:q].blank? && @project.present? - users = @project.users.sorted + scope = @project.users else - users = User.active.sorted.like(params[:q]).limit(100) + scope = User.all.limit(100) end + users = scope.active.visible.sorted.like(params[:q]).to_a if @watched users -= @watched.watcher_users end diff --git a/app/models/issue_query.rb b/app/models/issue_query.rb index 15c49c5d6..8140163d8 100644 --- a/app/models/issue_query.rb +++ b/app/models/issue_query.rb @@ -131,17 +131,17 @@ class IssueQuery < Query issue_custom_fields = [] if project - principals += project.principals.sort + principals += project.principals.visible unless project.leaf? subprojects = project.descendants.visible.to_a - principals += Principal.member_of(subprojects) + principals += Principal.member_of(subprojects).visible end versions = project.shared_versions.to_a categories = project.issue_categories.to_a issue_custom_fields = project.all_issue_custom_fields else if all_projects.any? - principals += Principal.member_of(all_projects) + principals += Principal.member_of(all_projects).visible end versions = Version.visible.where(:sharing => 'system').to_a issue_custom_fields = IssueCustomField.where(:is_for_all => true) @@ -185,7 +185,7 @@ class IssueQuery < Query :type => :list_optional, :values => assigned_to_values ) unless assigned_to_values.empty? - group_values = Group.givable.collect {|g| [g.name, g.id.to_s] } + group_values = Group.givable.visible.collect {|g| [g.name, g.id.to_s] } add_available_filter("member_of_group", :type => :list_optional, :values => group_values ) unless group_values.empty? diff --git a/app/models/principal.rb b/app/models/principal.rb index e6e6ea78e..d98531dca 100644 --- a/app/models/principal.rb +++ b/app/models/principal.rb @@ -38,6 +38,30 @@ class Principal < ActiveRecord::Base # Groups and active users scope :active, lambda { where(:status => STATUS_ACTIVE) } + scope :visible, lambda {|*args| + user = args.first || User.current + + if user.admin? + all + else + view_all_active = false + if user.memberships.to_a.any? + view_all_active = user.memberships.any? {|m| m.roles.any? {|r| r.users_visibility == 'all'}} + else + view_all_active = user.builtin_role.users_visibility == 'all' + end + + if view_all_active + active + else + # self and members of visible projects + active.where("#{table_name}.id = ? OR #{table_name}.id IN (SELECT user_id FROM #{Member.table_name} WHERE project_id IN (?))", + user.id, user.visible_project_ids + ) + end + end + } + scope :like, lambda {|q| q = q.to_s if q.blank? @@ -84,6 +108,10 @@ class Principal < ActiveRecord::Base to_s end + def visible?(user=User.current) + Principal.visible(user).where(:id => id).first == self + end + # Return true if the principal is a member of project def member_of?(project) projects.to_a.include?(project) diff --git a/app/models/role.rb b/app/models/role.rb index 31dbf75a1..c6a6b4979 100644 --- a/app/models/role.rb +++ b/app/models/role.rb @@ -39,6 +39,11 @@ class Role < ActiveRecord::Base ['own', :label_issues_visibility_own] ] + USERS_VISIBILITY_OPTIONS = [ + ['all', :label_users_visibility_all], + ['members_of_visible_projects', :label_users_visibility_members_of_visible_projects] + ] + scope :sorted, lambda { order(:builtin, :position) } scope :givable, lambda { order(:position).where(:builtin => 0) } scope :builtin, lambda { |*args| @@ -67,6 +72,9 @@ class Role < ActiveRecord::Base validates_inclusion_of :issues_visibility, :in => ISSUES_VISIBILITY_OPTIONS.collect(&:first), :if => lambda {|role| role.respond_to?(:issues_visibility)} + validates_inclusion_of :users_visibility, + :in => USERS_VISIBILITY_OPTIONS.collect(&:first), + :if => lambda {|role| role.respond_to?(:users_visibility)} # Copies attributes from another role, arg can be an id or a Role def copy_from(arg, options={}) diff --git a/app/models/time_entry_query.rb b/app/models/time_entry_query.rb index 05c39f55e..13ef60c20 100644 --- a/app/models/time_entry_query.rb +++ b/app/models/time_entry_query.rb @@ -40,20 +40,20 @@ class TimeEntryQuery < Query principals = [] if project - principals += project.principals.sort + principals += project.principals.visible.sort unless project.leaf? subprojects = project.descendants.visible.to_a if subprojects.any? add_available_filter "subproject_id", :type => :list_subprojects, :values => subprojects.collect{|s| [s.name, s.id.to_s] } - principals += Principal.member_of(subprojects) + principals += Principal.member_of(subprojects).visible end end else if all_projects.any? # members of visible projects - principals += Principal.member_of(all_projects) + principals += Principal.member_of(all_projects).visible # project filter project_values = [] if User.current.logged? && User.current.memberships.any? diff --git a/app/models/user.rb b/app/models/user.rb index a0a703700..b5bb6ae1e 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -148,6 +148,7 @@ class User < Principal @notified_projects_ids = nil @notified_projects_ids_changed = false @builtin_role = nil + @visible_project_ids = nil base_reload(*args) end @@ -528,6 +529,11 @@ class User < Principal @projects_by_role = hash end + # Returns the ids of visible projects + def visible_project_ids + @visible_project_ids ||= Project.visible(self).pluck(:id) + end + # Returns true if user is arg or belongs to arg def is_or_belongs_to?(arg) if arg.is_a?(User) diff --git a/app/views/roles/_form.html.erb b/app/views/roles/_form.html.erb index e3c32ee84..1d4ce3a2a 100644 --- a/app/views/roles/_form.html.erb +++ b/app/views/roles/_form.html.erb @@ -1,18 +1,22 @@ <%= error_messages_for 'role' %> -<% unless @role.anonymous? %> <div class="box tabular"> -<% 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 for="copy_workflow_from"><%= l(:label_copy_workflow_from) %></label> -<%= select_tag(:copy_workflow_from, content_tag("option") + options_from_collection_for_select(@roles, :id, :name, params[:copy_workflow_from] || @copy_from.try(:id))) %></p> -<% end %> + <% unless @role.builtin? %> + <p><%= f.text_field :name, :required => true %></p> + <p><%= f.check_box :assignable %></p> + <% end %> + + <% unless @role.anonymous? %> + <p><%= f.select :issues_visibility, Role::ISSUES_VISIBILITY_OPTIONS.collect {|v| [l(v.last), v.first]} %></p> + <% end %> + + <p><%= f.select :users_visibility, Role::USERS_VISIBILITY_OPTIONS.collect {|v| [l(v.last), v.first]} %></p> + + <% if @role.new_record? && @roles.any? %> + <p><label for="copy_workflow_from"><%= l(:label_copy_workflow_from) %></label> + <%= select_tag(:copy_workflow_from, content_tag("option") + options_from_collection_for_select(@roles, :id, :name, params[:copy_workflow_from] || @copy_from.try(:id))) %></p> + <% end %> </div> -<% end %> <h3><%= l(:label_permissions) %></h3> <div class="box tabular" id="permissions"> diff --git a/config/locales/en.yml b/config/locales/en.yml index f0372bccf..c94fb0b57 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -338,6 +338,7 @@ en: field_generate_password: Generate password field_must_change_passwd: Must change password at next logon field_default_status: Default status + field_users_visibility: Users visibility setting_app_title: Application title setting_app_subtitle: Application subtitle @@ -920,6 +921,8 @@ en: label_latest_compatible_version: Latest compatible version label_unknown_plugin: Unknown plugin label_add_projects: Add projects + label_users_visibility_all: All active users + label_users_visibility_members_of_visible_projects: Members of visible projects button_login: Login button_submit: Submit diff --git a/config/locales/fr.yml b/config/locales/fr.yml index 25693d080..3281f5510 100644 --- a/config/locales/fr.yml +++ b/config/locales/fr.yml @@ -358,6 +358,7 @@ fr: field_generate_password: Générer un mot de passe field_must_change_passwd: Doit changer de mot de passe à la prochaine connexion field_default_status: Statut par défaut + field_users_visibility: Visibilité des utilisateurs setting_app_title: Titre de l'application setting_app_subtitle: Sous-titre de l'application @@ -940,6 +941,8 @@ fr: label_latest_compatible_version: Dernière version compatible label_unknown_plugin: Plugin inconnu label_add_projects: Ajouter des projets + label_users_visibility_all: Tous les utilisateurs actifs + label_users_visibility_members_of_visible_projects: Membres des projets visibles button_login: Connexion button_submit: Soumettre diff --git a/db/migrate/20141109112308_add_roles_users_visibility.rb b/db/migrate/20141109112308_add_roles_users_visibility.rb new file mode 100644 index 000000000..05c4b6cff --- /dev/null +++ b/db/migrate/20141109112308_add_roles_users_visibility.rb @@ -0,0 +1,9 @@ +class AddRolesUsersVisibility < ActiveRecord::Migration + def self.up + add_column :roles, :users_visibility, :string, :limit => 30, :default => 'all', :null => false + end + + def self.down + remove_column :roles, :users_visibility + end +end diff --git a/lib/redmine/default_data/loader.rb b/lib/redmine/default_data/loader.rb index 9358795d5..8cd1129c8 100644 --- a/lib/redmine/default_data/loader.rb +++ b/lib/redmine/default_data/loader.rb @@ -42,6 +42,7 @@ module Redmine # Roles manager = Role.create! :name => l(:default_role_manager), :issues_visibility => 'all', + :users_visibility => 'all', :position => 1 manager.permissions = manager.setable_permissions.collect {|p| p.name} manager.save! diff --git a/test/fixtures/roles.yml b/test/fixtures/roles.yml index 2f1e57804..241c5afe6 100644 --- a/test/fixtures/roles.yml +++ b/test/fixtures/roles.yml @@ -4,6 +4,7 @@ roles_001: id: 1 builtin: 0 issues_visibility: all + users_visibility: all permissions: | --- - :add_project @@ -67,6 +68,7 @@ roles_002: id: 2 builtin: 0 issues_visibility: default + users_visibility: all permissions: | --- - :edit_project @@ -114,6 +116,7 @@ roles_003: id: 3 builtin: 0 issues_visibility: default + users_visibility: all permissions: | --- - :edit_project @@ -155,6 +158,7 @@ roles_004: id: 4 builtin: 1 issues_visibility: default + users_visibility: all permissions: | --- - :view_issues @@ -184,6 +188,7 @@ roles_005: id: 5 builtin: 2 issues_visibility: default + users_visibility: all permissions: | --- - :view_issues diff --git a/test/functional/users_controller_test.rb b/test/functional/users_controller_test.rb index 78b6689fc..d9a46bd60 100644 --- a/test/functional/users_controller_test.rb +++ b/test/functional/users_controller_test.rb @@ -106,12 +106,6 @@ class UsersControllerTest < ActionController::TestCase assert_response 404 end - def test_show_should_not_reveal_users_with_no_visible_activity_or_project - @request.session[:user_id] = nil - get :show, :id => 9 - assert_response 404 - end - def test_show_inactive_by_admin @request.session[:user_id] = 1 get :show, :id => 5 @@ -119,6 +113,15 @@ class UsersControllerTest < ActionController::TestCase assert_not_nil assigns(:user) end + def test_show_user_who_is_not_visible_should_return_404 + Role.anonymous.update! :users_visibility => 'members_of_visible_projects' + user = User.generate! + + @request.session[:user_id] = nil + get :show, :id => user.id + assert_response 404 + end + def test_show_displays_memberships_based_on_project_visibility @request.session[:user_id] = 1 get :show, :id => 2 diff --git a/test/functional/watchers_controller_test.rb b/test/functional/watchers_controller_test.rb index dc72a9b6b..628ad0550 100644 --- a/test/functional/watchers_controller_test.rb +++ b/test/functional/watchers_controller_test.rb @@ -227,6 +227,21 @@ class WatchersControllerTest < ActionController::TestCase assert Issue.find(2).watched_by?(user) end + def test_autocomplete_for_user_should_return_visible_users + Role.update_all :users_visibility => 'members_of_visible_projects' + + hidden = User.generate!(:lastname => 'autocomplete') + visible = User.generate!(:lastname => 'autocomplete') + User.add_to_project(visible, Project.find(1)) + + @request.session[:user_id] = 2 + xhr :get, :autocomplete_for_user, :q => 'autocomp', :project_id => 'ecookbook' + assert_response :success + + assert_include visible, assigns(:users) + assert_not_include hidden, assigns(:users) + end + def test_append @request.session[:user_id] = 2 assert_no_difference 'Watcher.count' do diff --git a/test/unit/principal_test.rb b/test/unit/principal_test.rb index 3b599c373..4e2dd4f21 100644 --- a/test/unit/principal_test.rb +++ b/test/unit/principal_test.rb @@ -20,7 +20,7 @@ require File.expand_path('../../test_helper', __FILE__) class PrincipalTest < ActiveSupport::TestCase - fixtures :users, :projects, :members, :member_roles + fixtures :users, :projects, :members, :member_roles, :roles def test_active_scope_should_return_groups_and_active_users result = Principal.active.to_a @@ -30,6 +30,27 @@ class PrincipalTest < ActiveSupport::TestCase assert_nil result.detect {|p| p.is_a?(AnonymousUser)} end + def test_visible_scope_for_admin_should_return_all_principals + admin = User.generate! {|u| u.admin = true} + assert_equal Principal.count, Principal.visible(admin).count + end + + def test_visible_scope_for_user_with_members_of_visible_projects_visibility_should_return_active_principals + Role.non_member.update! :users_visibility => 'all' + user = User.generate! + + expected = Principal.active + assert_equal expected.map(&:id).sort, Principal.visible(user).pluck(:id).sort + end + + def test_visible_scope_for_user_with_members_of_visible_projects_visibility_should_return_members_of_visible_projects_and_self + Role.non_member.update! :users_visibility => 'members_of_visible_projects' + user = User.generate! + + expected = Project.visible(user).map(&:member_principals).flatten.map(&:principal).uniq << user + assert_equal expected.map(&:id).sort, Principal.visible(user).pluck(:id).sort + end + def test_member_of_scope_should_return_the_union_of_all_members projects = Project.find([1]) assert_equal [3, 2], Principal.member_of(projects).sort.map(&:id) |