From 7e7ac5340a281ed767066af0b5f4dd45a3d7076f Mon Sep 17 00:00:00 2001 From: Jean-Philippe Lang Date: Sun, 28 Sep 2014 14:51:08 +0000 Subject: [PATCH] Adds buit-in groups to give specific permissions to anonymous and non members users per project (#17976). git-svn-id: http://svn.redmine.org/redmine/trunk@13417 e93f8b46-1217-0410-a6f0-8f06a7374b81 --- app/controllers/groups_controller.rb | 8 ++- app/helpers/groups_helper.rb | 11 ++-- app/helpers/users_helper.rb | 2 +- app/models/group.rb | 35 +++++++++++- app/models/group_anonymous.rb | 30 ++++++++++ app/models/group_builtin.rb | 56 +++++++++++++++++++ app/models/group_non_member.rb | 30 ++++++++++ app/models/issue_query.rb | 7 ++- app/models/principal.rb | 3 + app/models/project.rb | 26 ++++++--- app/models/user.rb | 34 ++++++----- app/views/groups/_form.html.erb | 4 +- app/views/groups/_general.html.erb | 2 +- app/views/groups/edit.html.erb | 2 +- app/views/groups/index.api.rsb | 1 + app/views/groups/index.html.erb | 6 +- app/views/groups/show.api.rsb | 3 +- app/views/users/_groups.html.erb | 2 +- config/locales/en.yml | 2 + config/locales/fr.yml | 2 + .../20140920094058_insert_builtin_groups.rb | 21 +++++++ extra/svn/Redmine.pm | 20 +++++-- public/stylesheets/application.css | 5 +- .../redmine_pm/repository_subversion_test.rb | 15 +++++ test/extra/redmine_pm/test_case.rb | 1 + test/fixtures/users.yml | 13 ++++- test/integration/api_test/groups_test.rb | 34 ++++++++++- test/integration/issues_test.rb | 21 +++++++ test/unit/group_test.rb | 16 ++++++ test/unit/issue_test.rb | 31 ++++++++++ test/unit/principal_test.rb | 16 ++---- test/unit/query_test.rb | 2 +- test/unit/user_test.rb | 38 +++++++++++-- 33 files changed, 430 insertions(+), 69 deletions(-) create mode 100644 app/models/group_anonymous.rb create mode 100644 app/models/group_builtin.rb create mode 100644 app/models/group_non_member.rb create mode 100644 db/migrate/20140920094058_insert_builtin_groups.rb diff --git a/app/controllers/groups_controller.rb b/app/controllers/groups_controller.rb index 0cd4055e5..555fc61df 100644 --- a/app/controllers/groups_controller.rb +++ b/app/controllers/groups_controller.rb @@ -25,12 +25,16 @@ class GroupsController < ApplicationController helper :custom_fields def index - @groups = Group.sorted.all respond_to do |format| format.html { + @groups = Group.sorted.all @user_count_by_group_id = user_count_by_group_id } - format.api + format.api { + scope = Group.sorted + scope = scope.givable unless params[:builtin] == '1' + @groups = scope.all + } end end diff --git a/app/helpers/groups_helper.rb b/app/helpers/groups_helper.rb index 5629aea46..4777328b0 100644 --- a/app/helpers/groups_helper.rb +++ b/app/helpers/groups_helper.rb @@ -18,11 +18,12 @@ # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. module GroupsHelper - def group_settings_tabs - tabs = [{:name => 'general', :partial => 'groups/general', :label => :label_general}, - {:name => 'users', :partial => 'groups/users', :label => :label_user_plural}, - {:name => 'memberships', :partial => 'groups/memberships', :label => :label_project_plural} - ] + def group_settings_tabs(group) + tabs = [] + tabs << {:name => 'general', :partial => 'groups/general', :label => :label_general} + tabs << {:name => 'users', :partial => 'groups/users', :label => :label_user_plural} if group.givable? + tabs << {:name => 'memberships', :partial => 'groups/memberships', :label => :label_project_plural} + tabs end def render_principals_for_new_group_users(group) diff --git a/app/helpers/users_helper.rb b/app/helpers/users_helper.rb index edc2e341d..415b5fc09 100644 --- a/app/helpers/users_helper.rb +++ b/app/helpers/users_helper.rb @@ -46,7 +46,7 @@ module UsersHelper tabs = [{:name => 'general', :partial => 'users/general', :label => :label_general}, {:name => 'memberships', :partial => 'users/memberships', :label => :label_project_plural} ] - if Group.all.any? + if Group.givable.any? tabs.insert 1, {:name => 'groups', :partial => 'users/groups', :label => :label_group_plural} end tabs diff --git a/app/models/group.rb b/app/models/group.rb index 9824bd998..7b82d1c1f 100644 --- a/app/models/group.rb +++ b/app/models/group.rb @@ -31,17 +31,18 @@ class Group < Principal before_destroy :remove_references_before_destroy - scope :sorted, lambda { order("#{table_name}.lastname ASC") } + scope :sorted, lambda { order("#{table_name}.type, #{table_name}.lastname ASC") } scope :named, lambda {|arg| where("LOWER(#{table_name}.lastname) = LOWER(?)", arg.to_s.strip)} + scope :givable, lambda {where(:type => 'Group')} safe_attributes 'name', 'user_ids', 'custom_field_values', 'custom_fields', - :if => lambda {|group, user| user.admin?} + :if => lambda {|group, user| user.admin? && !group.builtin?} def to_s - lastname.to_s + name.to_s end def name @@ -52,6 +53,20 @@ class Group < Principal self.lastname = arg end + def builtin_type + nil + end + + # Return true if the group is a builtin group + def builtin? + false + end + + # Returns true if the group can be given to a user + def givable? + !builtin? + end + def user_added(user) members.each do |member| next if member.project.nil? @@ -80,6 +95,18 @@ class Group < Principal super(attr_name, *args) end + def self.builtin_id(arg) + (arg.anonymous? ? GroupAnonymous : GroupNonMember).instance_id + end + + def self.anonymous + GroupAnonymous.load_instance + end + + def self.non_member + GroupNonMember.load_instance + end + private # Removes references that are not handled by associations @@ -89,3 +116,5 @@ class Group < Principal Issue.where(['assigned_to_id = ?', id]).update_all('assigned_to_id = NULL') end end + +require_dependency "group_builtin" diff --git a/app/models/group_anonymous.rb b/app/models/group_anonymous.rb new file mode 100644 index 000000000..c3c821bb2 --- /dev/null +++ b/app/models/group_anonymous.rb @@ -0,0 +1,30 @@ +# Redmine - project management software +# Copyright (C) 2006-2014 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 +# as published by the Free Software Foundation; either version 2 +# of the License, or (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program; if not, write to the Free Software +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + +class GroupAnonymous < GroupBuiltin + def name + l(:label_group_anonymous) + end + + def builtin_type + "anonymous" + end + + def self.instance_id + @@instance_id ||= load_instance.id + end +end diff --git a/app/models/group_builtin.rb b/app/models/group_builtin.rb new file mode 100644 index 000000000..71ecc06ab --- /dev/null +++ b/app/models/group_builtin.rb @@ -0,0 +1,56 @@ +# Redmine - project management software +# Copyright (C) 2006-2014 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 +# as published by the Free Software Foundation; either version 2 +# of the License, or (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program; if not, write to the Free Software +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + +class GroupBuiltin < Group + validate :validate_uniqueness, :on => :create + + def validate_uniqueness + errors.add :base, 'The builtin group already exists.' if self.class.exists? + end + + def builtin? + true + end + + def destroy + false + end + + def user_added(user) + raise 'Cannot add users to a builtin group' + end + + class << self + def load_instance + return nil if self == GroupBuiltin + instance = first(:order => 'id') || create_instance + end + + def create_instance + raise 'The builtin group already exists.' if exists? + instance = new + instance.lastname = name + instance.save :validate => false + raise 'Unable to create builtin group.' if instance.new_record? + instance + end + private :create_instance + end +end + +require_dependency "group_anonymous" +require_dependency "group_non_member" diff --git a/app/models/group_non_member.rb b/app/models/group_non_member.rb new file mode 100644 index 000000000..8b3dfd4aa --- /dev/null +++ b/app/models/group_non_member.rb @@ -0,0 +1,30 @@ +# Redmine - project management software +# Copyright (C) 2006-2014 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 +# as published by the Free Software Foundation; either version 2 +# of the License, or (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program; if not, write to the Free Software +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + +class GroupNonMember < GroupBuiltin + def name + l(:label_group_non_member) + end + + def builtin_type + "non_member" + end + + def self.instance_id + @@instance_id ||= load_instance.id + end +end diff --git a/app/models/issue_query.rb b/app/models/issue_query.rb index bf90f56d1..c9115100c 100644 --- a/app/models/issue_query.rb +++ b/app/models/issue_query.rb @@ -147,6 +147,7 @@ class IssueQuery < Query end principals.uniq! principals.sort! + principals.reject! {|p| p.is_a?(GroupBuiltin)} users = principals.select {|p| p.is_a?(User)} add_available_filter "status_id", @@ -183,7 +184,7 @@ class IssueQuery < Query :type => :list_optional, :values => assigned_to_values ) unless assigned_to_values.empty? - group_values = Group.all.collect {|g| [g.name, g.id.to_s] } + group_values = Group.givable.collect {|g| [g.name, g.id.to_s] } add_available_filter("member_of_group", :type => :list_optional, :values => group_values ) unless group_values.empty? @@ -404,10 +405,10 @@ class IssueQuery < Query def sql_for_member_of_group_field(field, operator, value) if operator == '*' # Any group - groups = Group.all + groups = Group.givable operator = '=' # Override the operator since we want to find by assigned_to elsif operator == "!*" - groups = Group.all + groups = Group.givable operator = '!' # Override the operator since we want to find by assigned_to else groups = Group.where(:id => value).all diff --git a/app/models/principal.rb b/app/models/principal.rb index 451349df2..1bbbbe88e 100644 --- a/app/models/principal.rb +++ b/app/models/principal.rb @@ -114,3 +114,6 @@ class Principal < ActiveRecord::Base true end end + +require_dependency "user" +require_dependency "group" diff --git a/app/models/project.rb b/app/models/project.rb index 96de85517..6a428b209 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -32,7 +32,7 @@ class Project < ActiveRecord::Base has_many :memberships, :class_name => 'Member' has_many :member_principals, :class_name => 'Member', :include => :principal, - :conditions => "#{Principal.table_name}.type='Group' OR (#{Principal.table_name}.type='User' AND #{Principal.table_name}.status=#{Principal::STATUS_ACTIVE})" + :conditions => "#{Principal.table_name}.status=#{Principal::STATUS_ACTIVE}" has_many :enabled_modules, :dependent => :delete_all has_and_belongs_to_many :trackers, :order => "#{Tracker.table_name}.position" @@ -191,11 +191,9 @@ class Project < ActiveRecord::Base statement_by_role[role] = "#{Project.table_name}.is_public = #{connection.quoted_true}" end end - if user.logged? - user.projects_by_role.each do |role, projects| - if role.allowed_to?(permission) && projects.any? - statement_by_role[role] = "#{Project.table_name}.id IN (#{projects.collect(&:id).join(',')})" - end + user.projects_by_role.each do |role, projects| + if role.allowed_to?(permission) && projects.any? + statement_by_role[role] = "#{Project.table_name}.id IN (#{projects.collect(&:id).join(',')})" end end if statement_by_role.empty? @@ -213,6 +211,12 @@ class Project < ActiveRecord::Base end end + def override_roles(role) + @override_members ||= memberships.where(:user_id => [GroupAnonymous.instance_id, GroupNonMember.instance_id]).all + member = @override_members.detect {|m| role.anonymous? ^ (m.user_id == GroupNonMember.instance_id)} + member ? member.roles : [role] + end + def principals @principals ||= Principal.active.joins(:members).where("#{Member.table_name}.project_id = ?", id).uniq end @@ -305,6 +309,7 @@ class Project < ActiveRecord::Base @actions_allowed = nil @start_date = nil @due_date = nil + @override_members = nil base_reload(*args) end @@ -498,8 +503,13 @@ class Project < ActiveRecord::Base # Users/groups issues can be assigned to def assignable_users - assignable = Setting.issue_group_assignment? ? member_principals : members - assignable.select {|m| m.roles.detect {|role| role.assignable?}}.collect {|m| m.principal}.sort + types = ['User'] + types << 'Group' if Setting.issue_group_assignment? + + member_principals. + select {|m| types.include?(m.principal.type) && m.roles.detect(&:assignable?)}. + map(&:principal). + sort end # Returns the mail addresses of users that should be always notified on project events diff --git a/app/models/user.rb b/app/models/user.rb index c79c674a9..d8590f47c 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -474,15 +474,15 @@ class User < Principal # Return user's roles for project def roles_for_project(project) - roles = [] # No role on archived projects - return roles if project.nil? || project.archived? + return [] if project.nil? || project.archived? if membership = membership(project) - roles = membership.roles + membership.roles.dup + elsif project.is_public? + project.override_roles(builtin_role) else - roles << builtin_role + [] end - roles end # Return true if the user is a member of project @@ -494,20 +494,28 @@ class User < Principal def projects_by_role return @projects_by_role if @projects_by_role - @projects_by_role = Hash.new([]) - memberships.each do |membership| - if membership.project - membership.roles.each do |role| - @projects_by_role[role] = [] unless @projects_by_role.key?(role) - @projects_by_role[role] << membership.project + hash = Hash.new([]) + + members = Member.joins(:project). + where("#{Project.table_name}.status <> 9"). + where("#{Member.table_name}.user_id = ? OR (#{Project.table_name}.is_public = ? AND #{Member.table_name}.user_id = ?)", self.id, true, Group.builtin_id(self)). + preload(:project, :roles) + + members.reject! {|member| member.user_id != id && project_ids.include?(member.project_id)} + members.each do |member| + if member.project + member.roles.each do |role| + hash[role] = [] unless hash.key?(role) + hash[role] << member.project end end end - @projects_by_role.each do |role, projects| + + hash.each do |role, projects| projects.uniq! end - @projects_by_role + @projects_by_role = hash end # Returns true if user is arg or belongs to arg diff --git a/app/views/groups/_form.html.erb b/app/views/groups/_form.html.erb index 7dc224061..9d5b087e1 100644 --- a/app/views/groups/_form.html.erb +++ b/app/views/groups/_form.html.erb @@ -1,7 +1,9 @@ <%= error_messages_for @group %>
-

<%= f.text_field :name, :required => true, :size => 60 %>

+

<%= f.text_field :name, :required => true, :size => 60, + :disabled => !@group.safe_attribute?('name') %>

+ <% @group.custom_field_values.each do |value| %>

<%= custom_field_tag_with_label :group, value %>

<% end %> diff --git a/app/views/groups/_general.html.erb b/app/views/groups/_general.html.erb index 8c720c3ee..c48f54cd4 100644 --- a/app/views/groups/_general.html.erb +++ b/app/views/groups/_general.html.erb @@ -1,4 +1,4 @@ -<%= labelled_form_for @group do |f| %> +<%= labelled_form_for @group, :url => group_path(@group) do |f| %> <%= render :partial => 'form', :locals => { :f => f } %> <%= submit_tag l(:button_save) %> <% end %> diff --git a/app/views/groups/edit.html.erb b/app/views/groups/edit.html.erb index 43be52f30..b2f37bde1 100644 --- a/app/views/groups/edit.html.erb +++ b/app/views/groups/edit.html.erb @@ -1,3 +1,3 @@ <%= title [l(:label_group_plural), groups_path], @group.name %> -<%= render_tabs group_settings_tabs %> +<%= render_tabs group_settings_tabs(@group) %> diff --git a/app/views/groups/index.api.rsb b/app/views/groups/index.api.rsb index 6a4efeaf7..8ebad1baf 100644 --- a/app/views/groups/index.api.rsb +++ b/app/views/groups/index.api.rsb @@ -3,6 +3,7 @@ api.array :groups do api.group do api.id group.id api.name group.lastname + api.builtin group.builtin_type if group.builtin_type render_api_custom_values group.visible_custom_field_values, api end diff --git a/app/views/groups/index.html.erb b/app/views/groups/index.html.erb index 22c89eff4..a600ca48d 100644 --- a/app/views/groups/index.html.erb +++ b/app/views/groups/index.html.erb @@ -12,10 +12,10 @@ <% @groups.each do |group| %> - + "> <%= link_to h(group), edit_group_path(group) %> - <%= @user_count_by_group_id[group.id] || 0 %> - <%= delete_link group %> + <%= (@user_count_by_group_id[group.id] || 0) unless group.builtin? %> + <%= delete_link group unless group.builtin? %> <% end %> diff --git a/app/views/groups/show.api.rsb b/app/views/groups/show.api.rsb index 626474410..15211f2cf 100644 --- a/app/views/groups/show.api.rsb +++ b/app/views/groups/show.api.rsb @@ -1,6 +1,7 @@ api.group do api.id @group.id api.name @group.lastname + api.builtin @group.builtin_type if @group.builtin_type render_api_custom_values @group.visible_custom_field_values, api @@ -8,7 +9,7 @@ api.group do @group.users.each do |user| api.user :id => user.id, :name => user.name end - end if include_in_api_response?('users') + end if include_in_api_response?('users') && !@group.builtin? api.array :memberships do @group.memberships.each do |membership| diff --git a/app/views/users/_groups.html.erb b/app/views/users/_groups.html.erb index 1203d3b59..1f54a8944 100644 --- a/app/views/users/_groups.html.erb +++ b/app/views/users/_groups.html.erb @@ -1,6 +1,6 @@ <%= form_for(:user, :url => { :action => 'update' }, :html => {:method => :put}) do %>
-<% Group.all.sort.each do |group| %> +<% Group.givable.sort.each do |group| %>
<% end %> <%= hidden_field_tag 'user[group_ids][]', '' %> diff --git a/config/locales/en.yml b/config/locales/en.yml index fdc2d9bc8..a8de0ef9c 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -851,6 +851,8 @@ en: label_group: Group label_group_plural: Groups label_group_new: New group + label_group_anonymous: Anonymous users + label_group_non_member: Non member users label_time_entry_plural: Spent time label_version_sharing_none: Not shared label_version_sharing_descendants: With subprojects diff --git a/config/locales/fr.yml b/config/locales/fr.yml index ff225e7ff..067a87dea 100644 --- a/config/locales/fr.yml +++ b/config/locales/fr.yml @@ -871,6 +871,8 @@ fr: label_group: Groupe label_group_plural: Groupes label_group_new: Nouveau groupe + label_group_anonymous: Utilisateurs anonymes + label_group_non_member: Utilisateurs non membres label_time_entry_plural: Temps passé label_version_sharing_none: Non partagé label_version_sharing_descendants: Avec les sous-projets diff --git a/db/migrate/20140920094058_insert_builtin_groups.rb b/db/migrate/20140920094058_insert_builtin_groups.rb new file mode 100644 index 000000000..ec505af2f --- /dev/null +++ b/db/migrate/20140920094058_insert_builtin_groups.rb @@ -0,0 +1,21 @@ +class InsertBuiltinGroups < ActiveRecord::Migration + def up + Group.reset_column_information + + unless GroupAnonymous.any? + g = GroupAnonymous.new(:lastname => 'Anonymous users') + g.status = 1 + g.save :validate => false + end + unless GroupNonMember.any? + g = GroupNonMember.new(:lastname => 'Non member users') + g.status = 1 + g.save :validate => false + end + end + + def down + GroupAnonymous.delete_all + GroupNonMember.delete_all + end +end diff --git a/extra/svn/Redmine.pm b/extra/svn/Redmine.pm index ac7387122..ddad660b3 100644 --- a/extra/svn/Redmine.pm +++ b/extra/svn/Redmine.pm @@ -248,7 +248,12 @@ sub RedmineDSN { AND ( roles.id IN (SELECT member_roles.role_id FROM members, member_roles WHERE members.user_id = users.id AND members.project_id = projects.id AND members.id = member_roles.member_id) OR - (roles.builtin=1 AND cast(projects.is_public as CHAR) IN ('t', '1')) + (cast(projects.is_public as CHAR) IN ('t', '1') + AND (roles.builtin=1 + OR roles.id IN (SELECT member_roles.role_id FROM members, member_roles, users g + WHERE members.user_id = g.id AND members.project_id = projects.id AND members.id = member_roles.member_id + AND g.type = 'GroupNonMember')) + ) ) AND roles.permissions IS NOT NULL"; $self->{RedmineQuery} = trim($query); @@ -328,7 +333,7 @@ sub access_handler { my $project_id = get_project_identifier($r); $r->set_handlers(PerlAuthenHandler => [\&OK]) - if is_public_project($project_id, $r) && anonymous_role_allows_browse_repository($r); + if is_public_project($project_id, $r) && anonymous_allowed_to_browse_repository($project_id, $r); return OK } @@ -400,15 +405,20 @@ sub is_public_project { $ret; } -sub anonymous_role_allows_browse_repository { +sub anonymous_allowed_to_browse_repository { + my $project_id = shift; my $r = shift; my $dbh = connect_database($r); my $sth = $dbh->prepare( - "SELECT permissions FROM roles WHERE builtin = 2;" + "SELECT permissions FROM roles WHERE permissions like '%browse_repository%' + AND (roles.builtin = 2 + OR roles.id IN (SELECT member_roles.role_id FROM projects, members, member_roles, users + WHERE members.user_id = users.id AND members.project_id = projects.id AND members.id = member_roles.member_id + AND projects.identifier = ? AND users.type = 'GroupAnonymous'));" ); - $sth->execute(); + $sth->execute($project_id); my $ret = 0; if (my @row = $sth->fetchrow_array) { if ($row[0] =~ /:browse_repository/) { diff --git a/public/stylesheets/application.css b/public/stylesheets/application.css index f068cd980..19507fced 100644 --- a/public/stylesheets/application.css +++ b/public/stylesheets/application.css @@ -241,6 +241,8 @@ table p {margin:0;} .odd {background-color:#f6f7f8;} .even {background-color: #fff;} +tr.builtin td.name {font-style:italic;} + a.sort { padding-right: 16px; background-position: 100% 50%; background-repeat: no-repeat; } a.sort.asc { background-image: url(../images/sort_asc.png); } a.sort.desc { background-image: url(../images/sort_desc.png); } @@ -609,7 +611,8 @@ select.bool_cf {width:auto !important;} #users_for_watcher {height: 200px; overflow:auto;} #users_for_watcher label {display: block;} -table.members td.group { padding-left: 20px; background: url(../images/group.png) no-repeat 0% 50%; } +table.members td.name {padding-left: 20px;} +table.members td.group, table.members td.groupnonmember, table.members td.groupanonymous {background: url(../images/group.png) no-repeat 0% 1px;} input#principal_search, input#user_search {width:90%} diff --git a/test/extra/redmine_pm/repository_subversion_test.rb b/test/extra/redmine_pm/repository_subversion_test.rb index 81cf2b840..b40210467 100644 --- a/test/extra/redmine_pm/repository_subversion_test.rb +++ b/test/extra/redmine_pm/repository_subversion_test.rb @@ -27,6 +27,12 @@ class RedminePmTest::RepositorySubversionTest < RedminePmTest::TestCase assert_success "ls", svn_url end + def test_anonymous_read_on_public_repo_with_anonymous_group_permission_should_succeed + Role.anonymous.remove_permission! :browse_repository + Member.create!(:project_id => 1, :principal => Group.anonymous, :role_ids => [2]) + assert_success "ls", svn_url + end + def test_anonymous_read_on_public_repo_without_permission_should_fail Role.anonymous.remove_permission! :browse_repository assert_failure "ls", svn_url @@ -55,6 +61,15 @@ class RedminePmTest::RepositorySubversionTest < RedminePmTest::TestCase end end + def test_non_member_read_on_public_repo_with_non_member_group_permission_should_succeed + Role.anonymous.remove_permission! :browse_repository + Role.non_member.remove_permission! :browse_repository + Member.create!(:project_id => 1, :principal => Group.non_member, :role_ids => [2]) + with_credentials "miscuser8", "foo" do + assert_success "ls", svn_url + end + end + def test_non_member_read_on_public_repo_without_permission_should_fail Role.anonymous.remove_permission! :browse_repository Role.non_member.remove_permission! :browse_repository diff --git a/test/extra/redmine_pm/test_case.rb b/test/extra/redmine_pm/test_case.rb index f1a6b92e8..d9d25bd65 100644 --- a/test/extra/redmine_pm/test_case.rb +++ b/test/extra/redmine_pm/test_case.rb @@ -65,6 +65,7 @@ module RedminePmTest @command = args.join(' ') @status = nil IO.popen("#{command} 2>&1") do |io| + io.set_encoding("ASCII-8BIT") if io.respond_to?(:set_encoding) @response = io.read end @status = $?.exitstatus diff --git a/test/fixtures/users.yml b/test/fixtures/users.yml index 8136b41d9..9adab5edd 100644 --- a/test/fixtures/users.yml +++ b/test/fixtures/users.yml @@ -161,9 +161,20 @@ groups_010: id: 10 lastname: A Team type: Group + status: 1 groups_011: id: 11 lastname: B Team type: Group + status: 1 +groups_non_member: + id: 12 + lastname: Non member users + type: GroupNonMember + status: 1 +groups_anonymous: + id: 13 + lastname: Anonymous users + type: GroupAnonymous + status: 1 - diff --git a/test/integration/api_test/groups_test.rb b/test/integration/api_test/groups_test.rb index 00752065e..550eb50e5 100644 --- a/test/integration/api_test/groups_test.rb +++ b/test/integration/api_test/groups_test.rb @@ -29,12 +29,13 @@ class Redmine::ApiTest::GroupsTest < Redmine::ApiTest::Base assert_response 401 end - test "GET /groups.xml should return groups" do + test "GET /groups.xml should return givable groups" do get '/groups.xml', {}, credentials('admin') assert_response :success assert_equal 'application/xml', response.content_type assert_select 'groups' do + assert_select 'group', Group.givable.count assert_select 'group' do assert_select 'name', :text => 'A Team' assert_select 'id', :text => '10' @@ -42,6 +43,24 @@ class Redmine::ApiTest::GroupsTest < Redmine::ApiTest::Base end end + test "GET /groups.xml?builtin=1 should return all groups" do + get '/groups.xml?builtin=1', {}, credentials('admin') + assert_response :success + assert_equal 'application/xml', response.content_type + + assert_select 'groups' do + assert_select 'group', Group.givable.count + 2 + assert_select 'group' do + assert_select 'builtin', :text => 'non_member' + assert_select 'id', :text => '12' + end + assert_select 'group' do + assert_select 'builtin', :text => 'anonymous' + assert_select 'id', :text => '13' + end + end + end + test "GET /groups.json should require authentication" do get '/groups.json' assert_response 401 @@ -60,7 +79,7 @@ class Redmine::ApiTest::GroupsTest < Redmine::ApiTest::Base assert_equal({'id' => 10, 'name' => 'A Team'}, group) end - test "GET /groups/:id.xml should return the group with its users" do + test "GET /groups/:id.xml should return the group" do get '/groups/10.xml', {}, credentials('admin') assert_response :success assert_equal 'application/xml', response.content_type @@ -71,6 +90,17 @@ class Redmine::ApiTest::GroupsTest < Redmine::ApiTest::Base end end + test "GET /groups/:id.xml should return the builtin group" do + get '/groups/12.xml', {}, credentials('admin') + assert_response :success + assert_equal 'application/xml', response.content_type + + assert_select 'group' do + assert_select 'builtin', :text => 'non_member' + assert_select 'id', :text => '12' + end + end + test "GET /groups/:id.xml should include users if requested" do get '/groups/10.xml?include=users', {}, credentials('admin') assert_response :success diff --git a/test/integration/issues_test.rb b/test/integration/issues_test.rb index 196f16f5b..21faaa93f 100644 --- a/test/integration/issues_test.rb +++ b/test/integration/issues_test.rb @@ -65,6 +65,27 @@ class IssuesTest < ActionController::IntegrationTest assert_equal 1, issue.status.id end + def test_create_issue_by_anonymous_without_permission_should_fail + Role.anonymous.remove_permission! :add_issues + + assert_no_difference 'Issue.count' do + post 'projects/1/issues', :tracker_id => "1", :issue => {:subject => "new test issue"} + end + assert_response 302 + end + + def test_create_issue_by_anonymous_with_custom_permission_should_succeed + Role.anonymous.remove_permission! :add_issues + Member.create!(:project_id => 1, :principal => Group.anonymous, :role_ids => [3]) + + assert_difference 'Issue.count' do + post 'projects/1/issues', :tracker_id => "1", :issue => {:subject => "new test issue"} + end + assert_response 302 + issue = Issue.order("id DESC").first + assert_equal User.anonymous, issue.author + end + # add then remove 2 attachments to an issue def test_issue_attachments log_user('jsmith', 'jsmith') diff --git a/test/unit/group_test.rb b/test/unit/group_test.rb index 188e9468a..d3b5d20ec 100644 --- a/test/unit/group_test.rb +++ b/test/unit/group_test.rb @@ -133,4 +133,20 @@ class GroupTest < ActiveSupport::TestCase assert_equal nil, Issue.find(1).assigned_to_id end + + def test_builtin_id_with_anonymous_user_should_return_anonymous_group + assert_equal 13, Group.builtin_id(User.anonymous) + end + + def test_builtin_id_with_anonymous_role_should_return_anonymous_group + assert_equal 13, Group.builtin_id(Role.anonymous) + end + + def test_builtin_id_with_user_should_return_non_member_group + assert_equal 12, Group.builtin_id(User.find(1)) + end + + def test_builtin_id_with_non_member_role_should_return_non_member_group + assert_equal 12, Group.builtin_id(Role.non_member) + end end diff --git a/test/unit/issue_test.rb b/test/unit/issue_test.rb index c4cf77c79..3454c71f5 100644 --- a/test/unit/issue_test.rb +++ b/test/unit/issue_test.rb @@ -219,6 +219,16 @@ class IssueTest < ActiveSupport::TestCase assert_visibility_match User.anonymous, issues end + def test_visible_scope_for_anonymous_without_view_issues_permissions_and_membership + Role.anonymous.remove_permission!(:view_issues) + Member.create!(:project_id => 1, :principal => Group.anonymous, :role_ids => [2]) + + issues = Issue.visible(User.anonymous).all + assert issues.any? + assert_equal [1], issues.map(&:project_id).uniq.sort + assert_visibility_match User.anonymous, issues + end + def test_anonymous_should_not_see_private_issues_with_issues_visibility_set_to_default assert Role.anonymous.update_attribute(:issues_visibility, 'default') issue = Issue.generate!(:author => User.anonymous, :assigned_to => User.anonymous, :is_private => true) @@ -265,6 +275,17 @@ class IssueTest < ActiveSupport::TestCase assert_visibility_match user, issues end + def test_visible_scope_for_non_member_without_view_issues_permissions_and_membership + Role.non_member.remove_permission!(:view_issues) + Member.create!(:project_id => 1, :principal => Group.non_member, :role_ids => [2]) + user = User.find(9) + + issues = Issue.visible(user).all + assert issues.any? + assert_equal [1], issues.map(&:project_id).uniq.sort + assert_visibility_match user, issues + end + def test_visible_scope_for_member user = User.find(9) # User should see issues of projects for which user has view_issues permissions only @@ -1724,6 +1745,16 @@ class IssueTest < ActiveSupport::TestCase end end + def test_assignable_users_should_not_include_builtin_groups + Member.create!(:project_id => 1, :principal => Group.non_member, :role_ids => [1]) + Member.create!(:project_id => 1, :principal => Group.anonymous, :role_ids => [1]) + issue = Issue.new(:project => Project.find(1)) + + with_settings :issue_group_assignment => '1' do + assert_nil issue.assignable_users.detect {|u| u.is_a?(GroupBuiltin)} + end + end + def test_create_should_send_email_notification ActionMailer::Base.deliveries.clear issue = Issue.new(:project_id => 1, :tracker_id => 1, diff --git a/test/unit/principal_test.rb b/test/unit/principal_test.rb index 2ddcc8b1c..b37f781a3 100644 --- a/test/unit/principal_test.rb +++ b/test/unit/principal_test.rb @@ -55,17 +55,11 @@ class PrincipalTest < ActiveSupport::TestCase end def test_sorted_scope_should_sort_users_before_groups - scope = Principal.where("type <> ?", 'AnonymousUser') - expected_order = scope.all.sort do |a, b| - if a.is_a?(User) && b.is_a?(Group) - -1 - elsif a.is_a?(Group) && b.is_a?(User) - 1 - else - a.name.downcase <=> b.name.downcase - end - end - assert_equal expected_order.map(&:name).map(&:downcase), + scope = Principal.where(:type => ['User', 'Group']) + users = scope.select {|p| p.is_a?(User)}.sort + groups = scope.select {|p| p.is_a?(Group)}.sort + + assert_equal (users + groups).map(&:name).map(&:downcase), scope.sorted.map(&:name).map(&:downcase) end diff --git a/test/unit/query_test.rb b/test/unit/query_test.rb index 084e91a82..102efd4de 100644 --- a/test/unit/query_test.rb +++ b/test/unit/query_test.rb @@ -1240,7 +1240,7 @@ class QueryTest < ActiveSupport::TestCase assert query.available_filters.keys.include?("member_of_group") assert_equal :list_optional, query.available_filters["member_of_group"][:type] assert query.available_filters["member_of_group"][:values].present? - assert_equal Group.all.sort.map {|g| [g.name, g.id.to_s]}, + assert_equal Group.givable.sort.map {|g| [g.name, g.id.to_s]}, query.available_filters["member_of_group"][:values].sort end diff --git a/test/unit/user_test.rb b/test/unit/user_test.rb index 69753960a..187ec7c8b 100644 --- a/test/unit/user_test.rb +++ b/test/unit/user_test.rb @@ -838,14 +838,42 @@ class UserTest < ActiveSupport::TestCase assert_nil membership end - def test_roles_for_project - # user with a role + def test_roles_for_project_with_member_on_public_project_should_return_roles_and_non_member roles = @jsmith.roles_for_project(Project.find(1)) assert_kind_of Role, roles.first - assert_equal "Manager", roles.first.name + assert_equal ["Manager"], roles.map(&:name) + end + + def test_roles_for_project_with_member_on_private_project_should_return_roles + Project.find(1).update_attribute :is_public, false + + roles = @jsmith.roles_for_project(Project.find(1)) + assert_kind_of Role, roles.first + assert_equal ["Manager"], roles.map(&:name) + end + + def test_roles_for_project_with_non_member_with_public_project_should_return_non_member + roles = User.find(8).roles_for_project(Project.find(1)) + assert_equal ["Non member"], roles.map(&:name) + end + + def test_roles_for_project_with_non_member_with_public_project_should_return_no_roles + Project.find(1).update_attribute :is_public, false + + roles = User.find(8).roles_for_project(Project.find(1)) + assert_equal [], roles.map(&:name) + end + + def test_roles_for_project_with_anonymous_with_public_project_should_return_anonymous + roles = User.anonymous.roles_for_project(Project.find(1)) + assert_equal ["Anonymous"], roles.map(&:name) + end - # user with no role - assert_nil @dlopper.roles_for_project(Project.find(2)).detect {|role| role.member?} + def test_roles_for_project_with_anonymous_with_public_project_should_return_no_roles + Project.find(1).update_attribute :is_public, false + + roles = User.anonymous.roles_for_project(Project.find(1)) + assert_equal [], roles.map(&:name) end def test_projects_by_role_for_user_with_role -- 2.39.5