summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorJean-Philippe Lang <jp_lang@yahoo.fr>2014-11-11 13:08:52 +0000
committerJean-Philippe Lang <jp_lang@yahoo.fr>2014-11-11 13:08:52 +0000
commitbdd3ccf8e52c69d2b6e16e7230a1b8f9a6c69e60 (patch)
tree1571b147765d42bccab602cdd9a79499829de612
parent140ca9532c1c12b7ff710c076c6985dce18500e4 (diff)
downloadredmine-bdd3ccf8e52c69d2b6e16e7230a1b8f9a6c69e60.tar.gz
redmine-bdd3ccf8e52c69d2b6e16e7230a1b8f9a6c69e60.zip
Adds a role setting for controlling visibility of users: all or members of visible projects (#11724).
git-svn-id: http://svn.redmine.org/redmine/trunk@13584 e93f8b46-1217-0410-a6f0-8f06a7374b81
-rw-r--r--app/controllers/users_controller.rb12
-rw-r--r--app/controllers/watchers_controller.rb16
-rw-r--r--app/models/issue_query.rb8
-rw-r--r--app/models/principal.rb28
-rw-r--r--app/models/role.rb8
-rw-r--r--app/models/time_entry_query.rb6
-rw-r--r--app/models/user.rb6
-rw-r--r--app/views/roles/_form.html.erb26
-rw-r--r--config/locales/en.yml3
-rw-r--r--config/locales/fr.yml3
-rw-r--r--db/migrate/20141109112308_add_roles_users_visibility.rb9
-rw-r--r--lib/redmine/default_data/loader.rb1
-rw-r--r--test/fixtures/roles.yml5
-rw-r--r--test/functional/users_controller_test.rb15
-rw-r--r--test/functional/watchers_controller_test.rb15
-rw-r--r--test/unit/principal_test.rb23
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)