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

@@ -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
 

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)

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

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"

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

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"

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

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

app/models/principal.rb

@@ -114,3 +114,6 @@ class Principal < ActiveRecord::Base
     true
   end
 end
+
+require_dependency "user"
+require_dependency "group"

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

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

app/views/groups/_form.html.erb

@@ -1,7 +1,9 @@
 <%= error_messages_for @group %>
 
 <div class="box tabular">
-  <p><%= f.text_field :name, :required => true, :size => 60 %></p>
+  <p><%= f.text_field :name, :required => true, :size => 60,
+           :disabled => !@group.safe_attribute?('name')  %></p>
+
   <% @group.custom_field_values.each do |value| %>
     <p><%= custom_field_tag_with_label :group, value %></p>
   <% end %>

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 %>

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) %>

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

app/views/groups/index.html.erb

@@ -12,10 +12,10 @@
   </tr></thead>
   <tbody>
 <% @groups.each do |group| %>
-  <tr id="group-<%= group.id %>" class="<%= cycle 'odd', 'even' %>">
+  <tr id="group-<%= group.id %>" class="<%= cycle 'odd', 'even' %> <%= "builtin" if group.builtin? %>">
     <td class="name"><%= link_to h(group), edit_group_path(group) %></td>
-    <td class="user_count"><%= @user_count_by_group_id[group.id] || 0 %></td>
-    <td class="buttons"><%= delete_link group %></td>
+    <td class="user_count"><%= (@user_count_by_group_id[group.id] || 0) unless group.builtin? %></td>
+    <td class="buttons"><%= delete_link group unless group.builtin? %></td>
   </tr>
 <% end %>
   </tbody>

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|

app/views/users/_groups.html.erb

@@ -1,6 +1,6 @@
 <%= form_for(:user, :url => { :action => 'update' }, :html => {:method => :put}) do %>
 <div class="box">
-<% Group.all.sort.each do |group| %>
+<% Group.givable.sort.each do |group| %>
 <label><%= check_box_tag 'user[group_ids][]', group.id, @user.groups.include?(group), :id => nil %> <%=h group %></label><br />
 <% end %>
 <%= hidden_field_tag 'user[group_ids][]', '' %>

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

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

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

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/) {

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%}
 

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

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

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
 

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

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')

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

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,

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
 

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
 

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
 
-    # user with no role
-    assert_nil @dlopper.roles_for_project(Project.find(2)).detect {|role| role.member?}
+  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
+
+  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