From 2f69213e3f32e2e4222f6335e790e2c778069014 Mon Sep 17 00:00:00 2001
From: Jason Hollingsworth <jhworth.developer@gmail.com>
Date: Thu, 13 Feb 2014 14:45:51 -0600
Subject: [PATCH] Allow access to groups with public projects.

Fixed Group avatars to only display when user has read
permissions to at least one project in the group.
---
 app/controllers/dashboard_controller.rb       |   1 +
 app/controllers/groups_controller.rb          |  25 +++-
 app/controllers/public/projects_controller.rb |   2 +-
 app/controllers/users_controller.rb           |   6 +-
 app/helpers/groups_helper.rb                  |   8 ++
 app/helpers/search_helper.rb                  |  15 +--
 app/models/ability.rb                         |  16 ++-
 app/models/group.rb                           |   6 +
 app/models/namespace.rb                       |   8 ++
 app/models/project.rb                         |  14 ++-
 app/services/filtering_service.rb             |   8 +-
 app/services/search/global_service.rb         |   6 +-
 app/views/groups/issues.html.haml             |   4 +-
 app/views/groups/members.html.haml            |  14 ++-
 app/views/groups/merge_requests.html.haml     |   4 +-
 app/views/groups/show.html.haml               |  20 +--
 app/views/layouts/nav/_group.html.haml        |   6 +-
 app/views/layouts/public_group.html.haml      |  10 ++
 app/views/layouts/public_projects.html.haml   |   2 +-
 app/views/shared/_filter.html.haml            |  23 ++--
 app/views/users/show.html.haml                |   4 +-
 app/views/users_groups/_users_group.html.haml |  40 +++---
 features/public/public_groups.feature         | 118 ++++++++++++++++++
 features/steps/public/groups_feature.rb       |  93 ++++++++++++++
 .../security/{ => group}/group_access_spec.rb |   6 +
 .../group/internal_group_access_spec.rb       |  87 +++++++++++++
 .../security/group/mixed_group_access_spec.rb |  88 +++++++++++++
 .../group/public_group_access_spec.rb         |  87 +++++++++++++
 28 files changed, 633 insertions(+), 88 deletions(-)
 create mode 100644 app/views/layouts/public_group.html.haml
 create mode 100644 features/public/public_groups.feature
 create mode 100644 features/steps/public/groups_feature.rb
 rename spec/features/security/{ => group}/group_access_spec.rb (91%)
 create mode 100644 spec/features/security/group/internal_group_access_spec.rb
 create mode 100644 spec/features/security/group/mixed_group_access_spec.rb
 create mode 100644 spec/features/security/group/public_group_access_spec.rb

diff --git a/app/controllers/dashboard_controller.rb b/app/controllers/dashboard_controller.rb
index b95233c0e91..8f204d09af1 100644
--- a/app/controllers/dashboard_controller.rb
+++ b/app/controllers/dashboard_controller.rb
@@ -77,5 +77,6 @@ class DashboardController < ApplicationController
   def default_filter
     params[:scope] = 'assigned-to-me' if params[:scope].blank?
     params[:state] = 'opened' if params[:state].blank?
+    params[:authorized_only] = true
   end
 end
diff --git a/app/controllers/groups_controller.rb b/app/controllers/groups_controller.rb
index b927dd2f20a..f6f7e3b3ecd 100644
--- a/app/controllers/groups_controller.rb
+++ b/app/controllers/groups_controller.rb
@@ -1,4 +1,5 @@
 class GroupsController < ApplicationController
+  skip_before_filter :authenticate_user!, only: [:show, :issues, :members, :merge_requests]
   respond_to :html
   before_filter :group, except: [:new, :create]
 
@@ -36,7 +37,7 @@ class GroupsController < ApplicationController
     @events = Event.in_projects(project_ids)
     @events = event_filter.apply_filter(@events)
     @events = @events.limit(20).offset(params[:offset] || 0)
-    @last_push = current_user.recent_push
+    @last_push = current_user.recent_push if current_user
 
     respond_to do |format|
       format.html
@@ -98,17 +99,21 @@ class GroupsController < ApplicationController
   end
 
   def projects
-    @projects ||= current_user.authorized_projects.where(namespace_id: group.id).sorted_by_activity
+    @projects ||= group.projects_accessible_to(current_user).sorted_by_activity
   end
 
   def project_ids
-    projects.map(&:id)
+    projects.pluck(:id)
   end
 
   # Dont allow unauthorized access to group
   def authorize_read_group!
     unless @group and (projects.present? or can?(current_user, :read_group, @group))
-      return render_404
+      if current_user.nil?
+        return authenticate_user!
+      else
+        return render_404
+      end
     end
   end
 
@@ -131,13 +136,21 @@ class GroupsController < ApplicationController
   def determine_layout
     if [:new, :create].include?(action_name.to_sym)
       'navless'
-    else
+    elsif current_user
       'group'
+    else
+      'public_group'
     end
   end
 
   def default_filter
-    params[:scope] = 'assigned-to-me' if params[:scope].blank?
+    if params[:scope].blank?
+      if current_user
+        params[:scope] = 'assigned-to-me'
+      else
+        params[:scope] = 'all'
+      end
+    end
     params[:state] = 'opened' if params[:state].blank?
     params[:group_id] = @group.id
   end
diff --git a/app/controllers/public/projects_controller.rb b/app/controllers/public/projects_controller.rb
index d7297161c22..d6238f79547 100644
--- a/app/controllers/public/projects_controller.rb
+++ b/app/controllers/public/projects_controller.rb
@@ -6,7 +6,7 @@ class Public::ProjectsController < ApplicationController
   layout 'public'
 
   def index
-    @projects = Project.public_or_internal_only(current_user)
+    @projects = Project.publicish(current_user)
     @projects = @projects.search(params[:search]) if params[:search].present?
     @projects = @projects.sort(@sort = params[:sort])
     @projects = @projects.includes(:namespace).page(params[:page]).per(20)
diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb
index e86601a439e..9461174b950 100644
--- a/app/controllers/users_controller.rb
+++ b/app/controllers/users_controller.rb
@@ -1,15 +1,15 @@
 class UsersController < ApplicationController
-
   skip_before_filter :authenticate_user!, only: [:show]
   layout :determine_layout
 
   def show
     @user = User.find_by_username!(params[:username])
-    @projects = @user.authorized_projects.includes(:namespace).select {|project| can?(current_user, :read_project, project)}
+    @projects = @user.authorized_projects.accessible_to(current_user)
     if !current_user && @projects.empty?
       return authenticate_user!
     end
-    @events = @user.recent_events.where(project_id: @projects.map(&:id)).limit(20)
+    @groups = @user.groups.accessible_to(current_user)
+    @events = @user.recent_events.where(project_id: @projects.pluck(:id)).limit(20)
     @title = @user.name
   end
 
diff --git a/app/helpers/groups_helper.rb b/app/helpers/groups_helper.rb
index 5865396b698..cfc9a572cac 100644
--- a/app/helpers/groups_helper.rb
+++ b/app/helpers/groups_helper.rb
@@ -6,6 +6,14 @@ module GroupsHelper
   def leave_group_message(group)
     "Are you sure you want to leave \"#{group}\" group?"
   end
+  
+  def should_user_see_group_roles?(user, group)
+    if user
+      user.is_admin? || group.members.exists?(user_id: user.id)
+    else
+      false
+    end
+  end
 
   def group_head_title
     title = @group.name
diff --git a/app/helpers/search_helper.rb b/app/helpers/search_helper.rb
index 470a495f036..d7f3da7e537 100644
--- a/app/helpers/search_helper.rb
+++ b/app/helpers/search_helper.rb
@@ -4,8 +4,7 @@ module SearchHelper
 
     resources_results = [
       groups_autocomplete(term),
-      projects_autocomplete(term),
-      public_projects_autocomplete(term),
+      projects_autocomplete(term)
     ].flatten
 
     generic_results = project_autocomplete + default_autocomplete + help_autocomplete
@@ -82,17 +81,7 @@ module SearchHelper
 
   # Autocomplete results for the current user's projects
   def projects_autocomplete(term, limit = 5)
-    current_user.authorized_projects.search_by_title(term).non_archived.limit(limit).map do |p|
-      {
-        label: "project: #{search_result_sanitize(p.name_with_namespace)}",
-        url: project_path(p)
-      }
-    end
-  end
-
-  # Autocomplete results for the current user's projects
-  def public_projects_autocomplete(term, limit = 5)
-    Project.public_or_internal_only(current_user).search_by_title(term).non_archived.limit(limit).map do |p|
+    Project.accessible_to(current_user).search_by_title(term).non_archived.limit(limit).map do |p|
       {
         label: "project: #{search_result_sanitize(p.name_with_namespace)}",
         url: project_path(p)
diff --git a/app/models/ability.rb b/app/models/ability.rb
index ba0ce527f64..89f8f320da9 100644
--- a/app/models/ability.rb
+++ b/app/models/ability.rb
@@ -43,7 +43,19 @@ class Ability
           :download_code
         ]
       else
-        []
+        group = if subject.kind_of?(Group)
+                  subject
+                elsif subject.respond_to?(:group)
+                  subject.group
+                else
+                  nil
+                end
+        
+        if group && group.has_projects_accessible_to?(nil)
+          [:read_group]
+        else
+          []
+        end
       end
     end
 
@@ -172,7 +184,7 @@ class Ability
     def group_abilities user, group
       rules = []
 
-      if group.users.include?(user) || user.admin?
+      if user.admin? || group.users.include?(user) || group.has_projects_accessible_to?(user)
         rules << :read_group
       end
 
diff --git a/app/models/group.rb b/app/models/group.rb
index 8de0c78c158..0d4d5f4e836 100644
--- a/app/models/group.rb
+++ b/app/models/group.rb
@@ -25,6 +25,12 @@ class Group < Namespace
   validates :avatar, file_size: { maximum: 100.kilobytes.to_i }
 
   mount_uploader :avatar, AttachmentUploader
+  
+  def self.accessible_to(user)
+    accessible_ids = Project.accessible_to(user).pluck(:namespace_id)
+    accessible_ids += user.groups.pluck(:id) if user
+    where(id: accessible_ids)
+  end
 
   def human_name
     name
diff --git a/app/models/namespace.rb b/app/models/namespace.rb
index 0bc5e1862eb..468c93bd426 100644
--- a/app/models/namespace.rb
+++ b/app/models/namespace.rb
@@ -47,6 +47,14 @@ class Namespace < ActiveRecord::Base
   def self.global_id
     'GLN'
   end
+  
+  def projects_accessible_to(user)
+    projects.accessible_to(user)
+  end
+  
+  def has_projects_accessible_to?(user)
+    projects_accessible_to(user).present?
+  end
 
   def to_param
     path
diff --git a/app/models/project.rb b/app/models/project.rb
index d9da2c377c8..316575c94f5 100644
--- a/app/models/project.rb
+++ b/app/models/project.rb
@@ -114,8 +114,6 @@ class Project < ActiveRecord::Base
   scope :sorted_by_activity, -> { reorder("projects.last_activity_at DESC") }
   scope :personal, ->(user) { where(namespace_id: user.namespace_id) }
   scope :joined, ->(user) { where("namespace_id != ?", user.namespace_id) }
-  scope :public_only, -> { where(visibility_level: PUBLIC) }
-  scope :public_or_internal_only, ->(user) { where("visibility_level IN (:levels)", levels: user ? [ INTERNAL, PUBLIC ] : [ PUBLIC ]) }
 
   scope :non_archived, -> { where(archived: false) }
 
@@ -125,6 +123,18 @@ class Project < ActiveRecord::Base
     def abandoned
       where('projects.last_activity_at < ?', 6.months.ago)
     end
+    
+    def publicish(user)
+      visibility_levels = [Project::PUBLIC]
+      visibility_levels += [Project::INTERNAL] if user
+      where(visibility_level: visibility_levels)
+    end
+    
+    def accessible_to(user)
+      accessible_ids = publicish(user).pluck(:id)
+      accessible_ids += user.authorized_projects.pluck(:id) if user
+      where(id: accessible_ids)
+    end
 
     def with_push
       includes(:events).where('events.action = ?', Event::PUSHED)
diff --git a/app/services/filtering_service.rb b/app/services/filtering_service.rb
index 52537f7ba4f..cbbb04ccba9 100644
--- a/app/services/filtering_service.rb
+++ b/app/services/filtering_service.rb
@@ -41,16 +41,16 @@ class FilteringService
   def init_collection
     table_name = klass.table_name
 
-    return klass.of_projects(Project.public_only) unless current_user
-
     if project
-      if current_user.can?(:read_project, project)
+      if project.public? || (current_user && current_user.can?(:read_project, project))
         project.send(table_name)
       else
         []
       end
-    else
+    elsif current_user && params[:authorized_only].presence
       klass.of_projects(current_user.authorized_projects)
+    else
+      klass.of_projects(Project.accessible_to(current_user))
     end
   end
 
diff --git a/app/services/search/global_service.rb b/app/services/search/global_service.rb
index c1130401578..09c7cb25dd5 100644
--- a/app/services/search/global_service.rb
+++ b/app/services/search/global_service.rb
@@ -11,12 +11,8 @@ module Search
       query = Shellwords.shellescape(query) if query.present?
       return result unless query.present?
 
-      authorized_projects_ids = []
-      authorized_projects_ids += current_user.authorized_projects.pluck(:id) if current_user
-      authorized_projects_ids += Project.public_or_internal_only(current_user).pluck(:id)
-
       group = Group.find_by(id: params[:group_id]) if params[:group_id].present?
-      projects = Project.where(id: authorized_projects_ids)
+      projects = Project.accessible_to(current_user)
       projects = projects.where(namespace_id: group.id) if group
       projects = projects.search(query)
       project_ids = projects.pluck(:id)
diff --git a/app/views/groups/issues.html.haml b/app/views/groups/issues.html.haml
index e24df310910..4b11d91dc98 100644
--- a/app/views/groups/issues.html.haml
+++ b/app/views/groups/issues.html.haml
@@ -5,7 +5,9 @@
 %p.light
   Only issues from
   %strong #{@group.name}
-  group are listed here. To see all issues you should visit #{link_to 'dashboard', issues_dashboard_path} page.
+  group are listed here.
+  - if current_user
+    To see all issues you should visit #{link_to 'dashboard', issues_dashboard_path} page.
 %hr
 
 .row
diff --git a/app/views/groups/members.html.haml b/app/views/groups/members.html.haml
index 38069feb37b..0c972622f88 100644
--- a/app/views/groups/members.html.haml
+++ b/app/views/groups/members.html.haml
@@ -1,9 +1,11 @@
+- show_roles = should_user_see_group_roles?(current_user, @group)
 %h3.page-title
   Group members
-%p.light
-  Members of group have access to all group projects.
-  Read more about permissions
-  %strong= link_to "here", help_permissions_path, class: "vlink"
+- if show_roles
+  %p.light
+    Members of group have access to all group projects.
+    Read more about permissions
+    %strong= link_to "here", help_permissions_path, class: "vlink"
 
 %hr
 
@@ -13,7 +15,7 @@
       = search_field_tag :search, params[:search], { placeholder: 'Find member by name', class: 'form-control search-text-input input-mn-300' }
     = submit_tag 'Search', class: 'btn'
 
-  - if current_user.can? :manage_group, @group
+  - if current_user && current_user.can?(:manage_group, @group)
     .pull-right
       = link_to '#', class: 'btn btn-new js-toggle-visibility-link' do
         Add members
@@ -30,7 +32,7 @@
       (#{@members.total_count})
   %ul.well-list
     - @members.each do |member|
-      = render 'users_groups/users_group', member: member, show_controls: true
+      = render 'users_groups/users_group', member: member, show_roles: show_roles, show_controls: true
 = paginate @members, theme: 'gitlab'
 
 :coffeescript
diff --git a/app/views/groups/merge_requests.html.haml b/app/views/groups/merge_requests.html.haml
index eaf85bbdbc8..209130ec444 100644
--- a/app/views/groups/merge_requests.html.haml
+++ b/app/views/groups/merge_requests.html.haml
@@ -5,7 +5,9 @@
 %p.light
   Only merge requests from
   %strong #{@group.name}
-  group are listed here. To see all merge requests you should visit #{link_to 'dashboard', merge_requests_dashboard_path} page.
+  group are listed here.
+  - if current_user
+    To see all merge requests you should visit #{link_to 'dashboard', merge_requests_dashboard_path} page.
 %hr
 .row
   .col-md-3
diff --git a/app/views/groups/show.html.haml b/app/views/groups/show.html.haml
index 6256c047929..bddb1c5954a 100644
--- a/app/views/groups/show.html.haml
+++ b/app/views/groups/show.html.haml
@@ -1,9 +1,10 @@
 .dashboard
   .activities.col-md-8.hidden-sm
-    = render "events/event_last_push", event: @last_push
-    = link_to dashboard_path, class: 'btn btn-tiny' do
-      &larr; To dashboard
-    &nbsp;
+    - if current_user
+      = render "events/event_last_push", event: @last_push
+      = link_to dashboard_path, class: 'btn btn-tiny' do
+        &larr; To dashboard
+      &nbsp;
     %span.cgray You will only see events from projects in this group
     %hr
     = render 'shared/event_filter'
@@ -21,11 +22,12 @@
         - if @group.description.present?
           %p= @group.description
     = render "projects", projects: @projects
-    .prepend-top-20
-      = link_to group_path(@group, { format: :atom, private_token: current_user.private_token }), title: "Feed" do
-        %strong
-          %i.icon-rss
-          News Feed
+    - if current_user
+      .prepend-top-20
+        = link_to group_path(@group, { format: :atom, private_token: current_user.private_token }), title: "Feed" do
+          %strong
+            %i.icon-rss
+            News Feed
 
     %hr
     = render 'shared/promo'
diff --git a/app/views/layouts/nav/_group.html.haml b/app/views/layouts/nav/_group.html.haml
index a8bb3b91559..36b102dc25a 100644
--- a/app/views/layouts/nav/_group.html.haml
+++ b/app/views/layouts/nav/_group.html.haml
@@ -5,11 +5,13 @@
   = nav_link(path: 'groups#issues') do
     = link_to issues_group_path(@group) do
       Issues
-      %span.count= current_user.assigned_issues.opened.of_group(@group).count
+      - if current_user
+        %span.count= current_user.assigned_issues.opened.of_group(@group).count
   = nav_link(path: 'groups#merge_requests') do
     = link_to merge_requests_group_path(@group) do
       Merge Requests
-      %span.count= current_user.cared_merge_requests.opened.of_group(@group).count
+      - if current_user
+        %span.count= current_user.cared_merge_requests.opened.of_group(@group).count
   = nav_link(path: 'groups#members') do
     = link_to "Members", members_group_path(@group)
 
diff --git a/app/views/layouts/public_group.html.haml b/app/views/layouts/public_group.html.haml
new file mode 100644
index 00000000000..a289b784725
--- /dev/null
+++ b/app/views/layouts/public_group.html.haml
@@ -0,0 +1,10 @@
+!!! 5
+%html{ lang: "en"}
+  = render "layouts/head", title: group_head_title
+  %body{class: "#{app_theme} application", :'data-page' => body_data_page}
+    = render "layouts/broadcast"
+    = render "layouts/public_head_panel", title: "group: #{@group.name}"
+    %nav.main-nav.navbar-collapse.collapse
+      .container= render 'layouts/nav/group'
+    .container
+      .content= yield
diff --git a/app/views/layouts/public_projects.html.haml b/app/views/layouts/public_projects.html.haml
index 5fcf9f99e75..cf4ca9c7a84 100644
--- a/app/views/layouts/public_projects.html.haml
+++ b/app/views/layouts/public_projects.html.haml
@@ -3,7 +3,7 @@
   = render "layouts/head", title: @project.name_with_namespace
   %body{class: "#{app_theme} application", :'data-page' => body_data_page}
     = render "layouts/broadcast"
-    = render "layouts/public_head_panel", title: @project.name_with_namespace
+    = render "layouts/public_head_panel", title: project_title(@project)
     %nav.main-nav
       .container= render 'layouts/nav/project'
     .container
diff --git a/app/views/shared/_filter.html.haml b/app/views/shared/_filter.html.haml
index 6063b4a0732..0becf531cc3 100644
--- a/app/views/shared/_filter.html.haml
+++ b/app/views/shared/_filter.html.haml
@@ -1,16 +1,17 @@
 .side-filters.hidden-xs.hidden-sm
   = form_tag filter_path(entity), method: 'get' do
-    %fieldset.scope-filter
-      %ul.nav.nav-pills.nav-stacked
-        %li{class: ("active" if params[:scope] == 'assigned-to-me')}
-          = link_to filter_path(entity, scope: 'assigned-to-me') do
-            Assigned to me
-        %li{class: ("active" if params[:scope] == 'authored')}
-          = link_to filter_path(entity, scope: 'authored') do
-            Created by me
-        %li{class: ("active" if params[:scope] == 'all')}
-          = link_to filter_path(entity, scope: 'all') do
-            Everyone's
+    - if current_user
+      %fieldset.scope-filter
+        %ul.nav.nav-pills.nav-stacked
+          %li{class: ("active" if params[:scope] == 'assigned-to-me')}
+            = link_to filter_path(entity, scope: 'assigned-to-me') do
+              Assigned to me
+          %li{class: ("active" if params[:scope] == 'authored')}
+            = link_to filter_path(entity, scope: 'authored') do
+              Created by me
+          %li{class: ("active" if params[:scope] == 'all')}
+            = link_to filter_path(entity, scope: 'all') do
+              Everyone's
 
     %fieldset.status-filter
       %legend State
diff --git a/app/views/users/show.html.haml b/app/views/users/show.html.haml
index 98210af1e3d..edcaf3acf98 100644
--- a/app/views/users/show.html.haml
+++ b/app/views/users/show.html.haml
@@ -14,10 +14,10 @@
       %small member since #{@user.created_at.stamp("Nov 12, 2031")}
     .clearfix
     %h4 Groups:
-    = render 'groups', groups: @user.groups 
+    = render 'groups', groups: @groups
     %hr
     %h4 User Activity:
     = render @events
   .col-md-4
     = render 'profile', user: @user
-    = render 'projects', user: @user
+    = render 'projects'
diff --git a/app/views/users_groups/_users_group.html.haml b/app/views/users_groups/_users_group.html.haml
index b66b486fbc5..1784dab35b4 100644
--- a/app/views/users_groups/_users_group.html.haml
+++ b/app/views/users_groups/_users_group.html.haml
@@ -1,5 +1,6 @@
 - user = member.user
 - return unless user
+- show_roles = true if show_roles.nil?
 %li{class: "#{dom_class(member)} js-toggle-container", id: dom_id(member)}
   = image_tag avatar_icon(user.email, 16), class: "avatar s16"
   %strong= user.name
@@ -7,22 +8,23 @@
   - if user == current_user
     %span.label.label-success It's you
 
-  %span.pull-right
-    %strong= member.human_access
-    - if show_controls
-      - if can?(current_user, :modify, member)
-        = link_to '#', class: "btn-tiny btn js-toggle-button", title: 'Edit access level' do
-          %i.icon-edit
-      - if can?(current_user, :destroy, member)
-        - if current_user == member.user
-          = link_to leave_profile_group_path(@group), data: { confirm: leave_group_message(@group.name)}, method: :delete, class: "btn-tiny btn btn-remove", title: 'Remove user from group' do
-            %i.icon-minus.icon-white
-        - else
-          = link_to group_users_group_path(@group, member), data: { confirm: remove_user_from_group_message(@group, user) }, method: :delete, remote: true, class: "btn-tiny btn btn-remove", title: 'Remove user from group' do
-            %i.icon-minus.icon-white
-
-  .edit-member.hide.js-toggle-content
-    = form_for [@group, member], remote: true do |f|
-      .alert.prepend-top-20
-        = f.select :group_access, options_for_select(UsersGroup.group_access_roles, member.group_access)
-        = f.submit 'Save', class: 'btn btn-save btn-small'
+  - if show_roles
+    %span.pull-right
+      %strong= member.human_access
+      - if show_controls
+        - if can?(current_user, :modify, member)
+          = link_to '#', class: "btn-tiny btn js-toggle-button", title: 'Edit access level' do
+            %i.icon-edit
+        - if can?(current_user, :destroy, member)
+          - if current_user == member.user
+            = link_to leave_profile_group_path(@group), data: { confirm: leave_group_message(@group.name)}, method: :delete, class: "btn-tiny btn btn-remove", title: 'Remove user from group' do
+              %i.icon-minus.icon-white
+          - else
+            = link_to group_users_group_path(@group, member), data: { confirm: remove_user_from_group_message(@group, user) }, method: :delete, remote: true, class: "btn-tiny btn btn-remove", title: 'Remove user from group' do
+              %i.icon-minus.icon-white
+  
+    .edit-member.hide.js-toggle-content
+      = form_for [@group, member], remote: true do |f|
+        .alert.prepend-top-20
+          = f.select :group_access, options_for_select(UsersGroup.group_access_roles, member.group_access)
+          = f.submit 'Save', class: 'btn btn-save btn-small'
diff --git a/features/public/public_groups.feature b/features/public/public_groups.feature
new file mode 100644
index 00000000000..7f1ec718e35
--- /dev/null
+++ b/features/public/public_groups.feature
@@ -0,0 +1,118 @@
+Feature: Public Projects Feature
+  Background:
+    Given group "TestGroup" has private project "Enterprise"
+
+  Scenario: I should not see group with private projects as visitor
+    When I visit group "TestGroup" page
+    Then I should be redirected to sign in page
+
+  Scenario: I should not see group with private projects group as user
+    When I sign in as a user
+    And I visit group "TestGroup" page
+    Then page status code should be 404
+
+  Scenario: I should not see group with private and internal projects as visitor
+    Given group "TestGroup" has internal project "Internal"
+    When I visit group "TestGroup" page
+    Then I should be redirected to sign in page
+
+  Scenario: I should see group with private and internal projects as user
+    Given group "TestGroup" has internal project "Internal"
+    When I sign in as a user
+    And I visit group "TestGroup" page
+    Then I should see project "Internal" items
+    And I should not see project "Enterprise" items
+
+  Scenario: I should see group issues for internal project as user
+    Given group "TestGroup" has internal project "Internal"
+    When I sign in as a user
+    And I visit group "TestGroup" issues page
+    And I change filter to Everyone's
+    Then I should see project "Internal" items
+    And I should not see project "Enterprise" items
+
+  Scenario: I should see group merge requests for internal project as user
+    Given group "TestGroup" has internal project "Internal"
+    When I sign in as a user
+    And I visit group "TestGroup" merge requests page
+    And I change filter to Everyone's
+    Then I should see project "Internal" items
+    And I should not see project "Enterprise" items
+
+  Scenario: I should see group's members as user
+    Given group "TestGroup" has internal project "Internal"
+    And "John Doe" is owner of group "TestGroup"
+    When I sign in as a user
+    And I visit group "TestGroup" members page
+    Then I should see group member "John Doe"
+    And I should not see member roles
+
+  Scenario: I should see group with private, internal and public projects as visitor
+    Given group "TestGroup" has internal project "Internal"
+    Given group "TestGroup" has public project "Community"
+    When I visit group "TestGroup" page
+    Then I should see project "Community" items
+    And I should not see project "Internal" items
+    And I should not see project "Enterprise" items
+
+  Scenario: I should see group issues for public project as visitor
+    Given group "TestGroup" has internal project "Internal"
+    Given group "TestGroup" has public project "Community"
+    When I visit group "TestGroup" issues page
+    Then I should see project "Community" items
+    And I should not see project "Internal" items
+    And I should not see project "Enterprise" items
+
+  Scenario: I should see group merge requests for public project as visitor
+    Given group "TestGroup" has internal project "Internal"
+    Given group "TestGroup" has public project "Community"
+    When I visit group "TestGroup" merge requests page
+    Then I should see project "Community" items
+    And I should not see project "Internal" items
+    And I should not see project "Enterprise" items
+
+  Scenario: I should see group's members as visitor
+    Given group "TestGroup" has internal project "Internal"
+    Given group "TestGroup" has public project "Community"
+    And "John Doe" is owner of group "TestGroup"
+    When I visit group "TestGroup" members page
+    Then I should see group member "John Doe"
+    And I should not see member roles
+
+  Scenario: I should see group with private, internal and public projects as user
+    Given group "TestGroup" has internal project "Internal"
+    Given group "TestGroup" has public project "Community"
+    When I sign in as a user
+    And I visit group "TestGroup" page
+    Then I should see project "Community" items
+    And I should see project "Internal" items
+    And I should not see project "Enterprise" items
+
+  Scenario: I should see group issues for internal and public projects as user
+    Given group "TestGroup" has internal project "Internal"
+    Given group "TestGroup" has public project "Community"
+    When I sign in as a user
+    And I visit group "TestGroup" issues page
+    And I change filter to Everyone's
+    Then I should see project "Community" items
+    And I should see project "Internal" items
+    And I should not see project "Enterprise" items
+
+  Scenario: I should see group merge requests for internal and public projects as user
+    Given group "TestGroup" has internal project "Internal"
+    Given group "TestGroup" has public project "Community"
+    When I sign in as a user
+    And I visit group "TestGroup" merge requests page
+    And I change filter to Everyone's
+    Then I should see project "Community" items
+    And I should see project "Internal" items
+    And I should not see project "Enterprise" items
+
+  Scenario: I should see group's members as user
+    Given group "TestGroup" has internal project "Internal"
+    Given group "TestGroup" has public project "Community"
+    And "John Doe" is owner of group "TestGroup"
+    When I sign in as a user
+    And I visit group "TestGroup" members page
+    Then I should see group member "John Doe"
+    And I should not see member roles
diff --git a/features/steps/public/groups_feature.rb b/features/steps/public/groups_feature.rb
new file mode 100644
index 00000000000..015deca5427
--- /dev/null
+++ b/features/steps/public/groups_feature.rb
@@ -0,0 +1,93 @@
+class Spinach::Features::PublicProjectsFeature < Spinach::FeatureSteps
+  include SharedAuthentication
+  include SharedPaths
+  include SharedGroup
+  include SharedProject
+
+  step 'group "TestGroup" has private project "Enterprise"' do
+    group_has_project("TestGroup", "Enterprise", Gitlab::VisibilityLevel::PRIVATE)
+  end
+
+  step 'group "TestGroup" has internal project "Internal"' do
+    group_has_project("TestGroup", "Internal", Gitlab::VisibilityLevel::INTERNAL)
+  end
+
+  step 'group "TestGroup" has public project "Community"' do
+    group_has_project("TestGroup", "Community", Gitlab::VisibilityLevel::PUBLIC)
+  end
+  
+  step '"John Doe" is owner of group "TestGroup"' do
+    group = Group.find_by(name: "TestGroup") || create(:group, name: "TestGroup")
+    user = create(:user, name: "John Doe")
+    group.add_user(user, Gitlab::Access::OWNER)
+  end
+
+  step 'I visit group "TestGroup" page' do
+    visit group_path(Group.find_by(name: "TestGroup"))
+  end
+
+  step 'I visit group "TestGroup" issues page' do
+    visit issues_group_path(Group.find_by(name: "TestGroup"))
+  end
+
+  step 'I visit group "TestGroup" merge requests page' do
+    visit merge_requests_group_path(Group.find_by(name: "TestGroup"))
+  end
+
+  step 'I visit group "TestGroup" members page' do
+    visit members_group_path(Group.find_by(name: "TestGroup"))
+  end
+  
+  step 'I should not see project "Enterprise" items' do
+    page.should_not have_content "Enterprise"
+  end
+  
+  step 'I should see project "Internal" items' do
+    page.should have_content "Internal"
+  end
+  
+  step 'I should not see project "Internal" items' do
+    page.should_not have_content "Internal"
+  end
+  
+  step 'I should see project "Community" items' do
+    page.should have_content "Community"
+  end
+  
+  step 'I change filter to Everyone\'s' do
+    click_link "Everyone's"
+  end
+  
+  step 'I should see group member "John Doe"' do
+    page.should have_content "John Doe"
+  end
+  
+  step 'I should not see member roles' do
+    page.body.should_not match(%r{owner|developer|reporter|guest}i)
+  end
+
+  protected
+
+  def group_has_project(groupname, projectname, visibility_level)
+    group = Group.find_by(name: groupname) || create(:group, name: groupname)
+    project = create(:project,
+      namespace: group,
+      name: projectname,
+      path: "#{groupname}-#{projectname}",
+      visibility_level: visibility_level
+    )
+    create(:issue,
+      title: "#{projectname} feature",
+      project: project
+    )
+    create(:merge_request,
+      title: "#{projectname} feature implemented",
+      source_project: project,
+      target_project: project
+    )
+    create(:closed_issue_event,
+      project: project
+    )
+  end
+end
+
diff --git a/spec/features/security/group_access_spec.rb b/spec/features/security/group/group_access_spec.rb
similarity index 91%
rename from spec/features/security/group_access_spec.rb
rename to spec/features/security/group/group_access_spec.rb
index dea957962a8..7ef372c9199 100644
--- a/spec/features/security/group_access_spec.rb
+++ b/spec/features/security/group/group_access_spec.rb
@@ -14,6 +14,7 @@ describe "Group access" do
     let(:master)   { create(:user) }
     let(:reporter) { create(:user) }
     let(:guest)    { create(:user) }
+    let(:nonmember)  { create(:user) }
 
     before do
       group.add_user(owner, Gitlab::Access::OWNER)
@@ -21,6 +22,11 @@ describe "Group access" do
       group.add_user(reporter, Gitlab::Access::REPORTER)
       group.add_user(guest, Gitlab::Access::GUEST)
     end
+    
+    describe "Group should not have accessible projects" do
+      it { group.has_projects_accessible_to?(nil).should be_false }
+      it { group.has_projects_accessible_to?(nonmember).should be_false }
+    end
 
     describe "GET /groups/:path" do
       subject { group_path(group) }
diff --git a/spec/features/security/group/internal_group_access_spec.rb b/spec/features/security/group/internal_group_access_spec.rb
new file mode 100644
index 00000000000..26b05b667a9
--- /dev/null
+++ b/spec/features/security/group/internal_group_access_spec.rb
@@ -0,0 +1,87 @@
+require 'spec_helper'
+
+describe "Group with internal project access" do
+  describe "Group" do
+    let(:group) { create(:group) }
+
+    let(:owner)   { create(:owner) }
+    let(:master)   { create(:user) }
+    let(:reporter) { create(:user) }
+    let(:guest)    { create(:user) }
+    let(:nonmember)  { create(:user) }
+
+    before do
+      group.add_user(owner, Gitlab::Access::OWNER)
+      group.add_user(master, Gitlab::Access::MASTER)
+      group.add_user(reporter, Gitlab::Access::REPORTER)
+      group.add_user(guest, Gitlab::Access::GUEST)
+      
+      create(:project, group: group, visibility_level: Gitlab::VisibilityLevel::INTERNAL)
+    end
+    
+    describe "Group should have accessible projects for users" do
+      it { group.has_projects_accessible_to?(nil).should be_false }
+      it { group.has_projects_accessible_to?(nonmember).should be_true }
+    end
+
+    describe "GET /groups/:path" do
+      subject { group_path(group) }
+
+      it { should be_allowed_for owner }
+      it { should be_allowed_for master }
+      it { should be_allowed_for reporter }
+      it { should be_allowed_for :admin }
+      it { should be_allowed_for guest }
+      it { should be_allowed_for :user }
+      it { should be_denied_for :visitor }
+    end
+
+    describe "GET /groups/:path/issues" do
+      subject { issues_group_path(group) }
+
+      it { should be_allowed_for owner }
+      it { should be_allowed_for master }
+      it { should be_allowed_for reporter }
+      it { should be_allowed_for :admin }
+      it { should be_allowed_for guest }
+      it { should be_allowed_for :user }
+      it { should be_denied_for :visitor }
+    end
+
+    describe "GET /groups/:path/merge_requests" do
+      subject { merge_requests_group_path(group) }
+
+      it { should be_allowed_for owner }
+      it { should be_allowed_for master }
+      it { should be_allowed_for reporter }
+      it { should be_allowed_for :admin }
+      it { should be_allowed_for guest }
+      it { should be_allowed_for :user }
+      it { should be_denied_for :visitor }
+    end
+
+    describe "GET /groups/:path/members" do
+      subject { members_group_path(group) }
+
+      it { should be_allowed_for owner }
+      it { should be_allowed_for master }
+      it { should be_allowed_for reporter }
+      it { should be_allowed_for :admin }
+      it { should be_allowed_for guest }
+      it { should be_allowed_for :user }
+      it { should be_denied_for :visitor }
+    end
+
+    describe "GET /groups/:path/edit" do
+      subject { edit_group_path(group) }
+
+      it { should be_allowed_for owner }
+      it { should be_denied_for master }
+      it { should be_denied_for reporter }
+      it { should be_allowed_for :admin }
+      it { should be_denied_for guest }
+      it { should be_denied_for :user }
+      it { should be_denied_for :visitor }
+    end
+  end
+end
diff --git a/spec/features/security/group/mixed_group_access_spec.rb b/spec/features/security/group/mixed_group_access_spec.rb
new file mode 100644
index 00000000000..9cae49157a4
--- /dev/null
+++ b/spec/features/security/group/mixed_group_access_spec.rb
@@ -0,0 +1,88 @@
+require 'spec_helper'
+
+describe "Group access" do
+  describe "Group" do
+    let(:group) { create(:group) }
+
+    let(:owner)   { create(:owner) }
+    let(:master)   { create(:user) }
+    let(:reporter) { create(:user) }
+    let(:guest)    { create(:user) }
+    let(:nonmember)  { create(:user) }
+
+    before do
+      group.add_user(owner, Gitlab::Access::OWNER)
+      group.add_user(master, Gitlab::Access::MASTER)
+      group.add_user(reporter, Gitlab::Access::REPORTER)
+      group.add_user(guest, Gitlab::Access::GUEST)
+
+      create(:project, path: "internal_project", group: group, visibility_level: Gitlab::VisibilityLevel::INTERNAL)
+      create(:project, path: "public_project", group: group, visibility_level: Gitlab::VisibilityLevel::PUBLIC)
+    end
+      
+    describe "Group should have accessible projects" do
+      it { group.has_projects_accessible_to?(nil).should be_true }
+      it { group.has_projects_accessible_to?(nonmember).should be_true }
+    end
+
+    describe "GET /groups/:path" do
+      subject { group_path(group) }
+
+      it { should be_allowed_for owner }
+      it { should be_allowed_for master }
+      it { should be_allowed_for reporter }
+      it { should be_allowed_for :admin }
+      it { should be_allowed_for guest }
+      it { should be_allowed_for :user }
+      it { should be_allowed_for :visitor }
+    end
+
+    describe "GET /groups/:path/issues" do
+      subject { issues_group_path(group) }
+
+      it { should be_allowed_for owner }
+      it { should be_allowed_for master }
+      it { should be_allowed_for reporter }
+      it { should be_allowed_for :admin }
+      it { should be_allowed_for guest }
+      it { should be_allowed_for :user }
+      it { should be_allowed_for :visitor }
+    end
+
+    describe "GET /groups/:path/merge_requests" do
+      subject { merge_requests_group_path(group) }
+
+      it { should be_allowed_for owner }
+      it { should be_allowed_for master }
+      it { should be_allowed_for reporter }
+      it { should be_allowed_for :admin }
+      it { should be_allowed_for guest }
+      it { should be_allowed_for :user }
+      it { should be_allowed_for :visitor }
+    end
+
+    describe "GET /groups/:path/members" do
+      subject { members_group_path(group) }
+
+      it { should be_allowed_for owner }
+      it { should be_allowed_for master }
+      it { should be_allowed_for reporter }
+      it { should be_allowed_for :admin }
+      it { should be_allowed_for guest }
+      it { should be_allowed_for :user }
+      it { should be_allowed_for :visitor }
+    end
+
+    describe "GET /groups/:path/edit" do
+      subject { edit_group_path(group) }
+
+      it { should be_allowed_for owner }
+      it { should be_denied_for master }
+      it { should be_denied_for reporter }
+      it { should be_allowed_for :admin }
+      it { should be_denied_for guest }
+      it { should be_denied_for :user }
+      it { should be_denied_for :visitor }
+    end
+  end
+end
diff --git a/spec/features/security/group/public_group_access_spec.rb b/spec/features/security/group/public_group_access_spec.rb
new file mode 100644
index 00000000000..d64be437b7a
--- /dev/null
+++ b/spec/features/security/group/public_group_access_spec.rb
@@ -0,0 +1,87 @@
+require 'spec_helper'
+
+describe "Group with public project access" do
+  describe "Group" do
+    let(:group) { create(:group) }
+
+    let(:owner)   { create(:owner) }
+    let(:master)   { create(:user) }
+    let(:reporter) { create(:user) }
+    let(:guest)    { create(:user) }
+    let(:nonmember)  { create(:user) }
+
+    before do
+      group.add_user(owner, Gitlab::Access::OWNER)
+      group.add_user(master, Gitlab::Access::MASTER)
+      group.add_user(reporter, Gitlab::Access::REPORTER)
+      group.add_user(guest, Gitlab::Access::GUEST)
+      
+      create(:project, group: group, visibility_level: Gitlab::VisibilityLevel::PUBLIC)
+    end
+
+    describe "Group should have accessible projects" do
+      it { group.has_projects_accessible_to?(nil).should be_true }
+      it { group.has_projects_accessible_to?(nonmember).should be_true }
+    end
+
+    describe "GET /groups/:path" do
+      subject { group_path(group) }
+
+      it { should be_allowed_for owner }
+      it { should be_allowed_for master }
+      it { should be_allowed_for reporter }
+      it { should be_allowed_for :admin }
+      it { should be_allowed_for guest }
+      it { should be_allowed_for :user }
+      it { should be_allowed_for :visitor }
+    end
+
+    describe "GET /groups/:path/issues" do
+      subject { issues_group_path(group) }
+
+      it { should be_allowed_for owner }
+      it { should be_allowed_for master }
+      it { should be_allowed_for reporter }
+      it { should be_allowed_for :admin }
+      it { should be_allowed_for guest }
+      it { should be_allowed_for :user }
+      it { should be_allowed_for :visitor }
+    end
+
+    describe "GET /groups/:path/merge_requests" do
+      subject { merge_requests_group_path(group) }
+
+      it { should be_allowed_for owner }
+      it { should be_allowed_for master }
+      it { should be_allowed_for reporter }
+      it { should be_allowed_for :admin }
+      it { should be_allowed_for guest }
+      it { should be_allowed_for :user }
+      it { should be_allowed_for :visitor }
+    end
+
+    describe "GET /groups/:path/members" do
+      subject { members_group_path(group) }
+
+      it { should be_allowed_for owner }
+      it { should be_allowed_for master }
+      it { should be_allowed_for reporter }
+      it { should be_allowed_for :admin }
+      it { should be_allowed_for guest }
+      it { should be_allowed_for :user }
+      it { should be_allowed_for :visitor }
+    end
+
+    describe "GET /groups/:path/edit" do
+      subject { edit_group_path(group) }
+
+      it { should be_allowed_for owner }
+      it { should be_denied_for master }
+      it { should be_denied_for reporter }
+      it { should be_allowed_for :admin }
+      it { should be_denied_for guest }
+      it { should be_denied_for :user }
+      it { should be_denied_for :visitor }
+    end
+  end
+end
-- 
GitLab