Merge branch 'public_profiles' into 'master'

Make all group public

https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/1219#

Internal issue - https://dev.gitlab.org/gitlab/gitlabhq/issues/1361

See merge request !1247

13 files changed, 93 insertions, 78 deletions

diff --git a/CHANGELOG b/CHANGELOG
index ddfd384f8c8..84f9688c3a8 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -32,7 +32,7 @@ v 8.0.0 (unreleased)
   - Fix 500 error when submit project snippet without body
   - Improve search page usability
   - Bring more UI consistency in way how projects, snippets and groups lists are rendered
-  - Make all profiles public
+  - Make all profiles and group public
   - Fixed login failure when extern_uid changes (Joel Koglin)
   - Don't notify users without access to the project when they are (accidentally) mentioned in a note.
   - Retrieving oauth token with LDAP credentials
diff --git a/app/assets/stylesheets/generic/sidebar.scss b/app/assets/stylesheets/generic/sidebar.scss
index 41ffd358576..3d055f0e66f 100644
--- a/app/assets/stylesheets/generic/sidebar.scss
+++ b/app/assets/stylesheets/generic/sidebar.scss
@@ -152,7 +152,6 @@
     }
 
     .collapse-nav a {
-      left: 0px;
       width: $sidebar_collapsed_width;
     }
 
@@ -171,6 +170,7 @@
   width: $sidebar_width;
   position: fixed;
   bottom: 0;
+  left: 0;
   font-size: 13px;
   background: transparent;
   height: 40px;
diff --git a/app/controllers/groups_controller.rb b/app/controllers/groups_controller.rb
index 486c6b2819c..524218290c6 100644
--- a/app/controllers/groups_controller.rb
+++ b/app/controllers/groups_controller.rb
@@ -4,7 +4,7 @@ class GroupsController < Groups::ApplicationController
   before_action :group, except: [:new, :create]
 
   # Authorize
-  before_action :authorize_read_group!, except: [:new, :create]
+  before_action :authorize_read_group!, except: [:show, :new, :create]
   before_action :authorize_admin_group!, only: [:edit, :update, :destroy, :projects]
   before_action :authorize_create_group!, only: [:new, :create]
 
diff --git a/app/controllers/namespaces_controller.rb b/app/controllers/namespaces_controller.rb
index 83eec1bf4a2..282012c60a1 100644
--- a/app/controllers/namespaces_controller.rb
+++ b/app/controllers/namespaces_controller.rb
@@ -14,7 +14,7 @@ class NamespacesController < ApplicationController
 
     if user
       redirect_to user_path(user)
-    elsif group && can?(current_user, :read_group, group)
+    elsif group
       redirect_to group_path(group)
     elsif current_user.nil?
       authenticate_user!
diff --git a/app/views/groups/show.html.haml b/app/views/groups/show.html.haml
index 0577f4ec142..a9ba9d2ba10 100644
--- a/app/views/groups/show.html.haml
+++ b/app/views/groups/show.html.haml
@@ -1,3 +1,6 @@
+- unless can?(current_user, :read_group, @group)
+  - @disable_search_panel = true
+
 = content_for :meta_tags do
   - if current_user
     = auto_discovery_link_tag(:atom, group_url(@group, format: :atom, private_token: current_user.private_token), title: "#{@group.name} activity")
@@ -16,22 +19,25 @@
 
   = render 'shared/show_aside'
 
-  .row
-    %section.activities.col-md-7
-      .hidden-xs
-        - if current_user
-          = render "events/event_last_push", event: @last_push
-
+  - if can?(current_user, :read_group, @group)
+    .row
+      %section.activities.col-md-7
+        .hidden-xs
           - if current_user
+            = render "events/event_last_push", event: @last_push
+
             %ul.nav.nav-pills.event_filter.pull-right
               %li
                 = link_to group_path(@group, { format: :atom, private_token: current_user.private_token }), title: "Feed", class: 'rss-btn' do
                   %i.fa.fa-rss
 
-          = render 'shared/event_filter'
-          %hr
+            = render 'shared/event_filter'
+            %hr
 
-      .content_list
-      = spinner
-    %aside.side.col-md-5
-      = render "projects", projects: @projects
+        .content_list
+        = spinner
+      %aside.side.col-md-5
+        = render "projects", projects: @projects
+  - else
+    %p
+      This group does not have public projects
diff --git a/app/views/layouts/header/_default.html.haml b/app/views/layouts/header/_default.html.haml
index 3892f71c0e3..c31b1cbe9a8 100644
--- a/app/views/layouts/header/_default.html.haml
+++ b/app/views/layouts/header/_default.html.haml
@@ -7,8 +7,9 @@
 
       .navbar-collapse.collapse
         %ul.nav.navbar-nav.pull-right
-          %li.hidden-sm.hidden-xs
-            = render 'layouts/search'
+          - unless @disable_search_panel
+            %li.hidden-sm.hidden-xs
+              = render 'layouts/search'
           %li.visible-sm.visible-xs
             = link_to search_path, title: 'Search', data: {toggle: 'tooltip', placement: 'bottom'} do
               = icon('search')
diff --git a/app/views/layouts/nav/_group.html.haml b/app/views/layouts/nav/_group.html.haml
index f57ec7e13f2..eb35af22b93 100644
--- a/app/views/layouts/nav/_group.html.haml
+++ b/app/views/layouts/nav/_group.html.haml
@@ -12,34 +12,35 @@
       = icon('dashboard fw')
       %span
         Group
-  - if current_user
-    = nav_link(controller: [:group, :milestones]) do
-      = link_to group_milestones_path(@group), title: 'Milestones', data: {placement: 'right'} do
-        = icon('clock-o fw')
+  - if can?(current_user, :read_group, @group)
+    - if current_user
+      = nav_link(controller: [:group, :milestones]) do
+        = link_to group_milestones_path(@group), title: 'Milestones', data: {placement: 'right'} do
+          = icon('clock-o fw')
+          %span
+            Milestones
+    = nav_link(path: 'groups#issues') do
+      = link_to issues_group_path(@group), title: 'Issues', data: {placement: 'right'} do
+        = icon('exclamation-circle fw')
         %span
-          Milestones
-  = nav_link(path: 'groups#issues') do
-    = link_to issues_group_path(@group), title: 'Issues', data: {placement: 'right'} do
-      = icon('exclamation-circle fw')
-      %span
-        Issues
-        - if current_user
-          %span.count= Issue.opened.of_group(@group).count
-  = nav_link(path: 'groups#merge_requests') do
-    = link_to merge_requests_group_path(@group), title: 'Merge Requests', data: {placement: 'right'} do
-      = icon('tasks fw')
-      %span
-        Merge Requests
-        - if current_user
-          %span.count= MergeRequest.opened.of_group(@group).count
-  = nav_link(controller: [:group_members]) do
-    = link_to group_group_members_path(@group), title: 'Members', data: {placement: 'right'} do
-      = icon('users fw')
-      %span
-        Members
-  - if can?(current_user, :admin_group, @group)
-    = nav_link(html_options: { class: "separate-item" }) do
-      = link_to edit_group_path(@group), title: 'Settings', data: {placement: 'right'} do
-        = icon ('cogs fw')
+          Issues
+          - if current_user
+            %span.count= Issue.opened.of_group(@group).count
+    = nav_link(path: 'groups#merge_requests') do
+      = link_to merge_requests_group_path(@group), title: 'Merge Requests', data: {placement: 'right'} do
+        = icon('tasks fw')
+        %span
+          Merge Requests
+          - if current_user
+            %span.count= MergeRequest.opened.of_group(@group).count
+    = nav_link(controller: [:group_members]) do
+      = link_to group_group_members_path(@group), title: 'Members', data: {placement: 'right'} do
+        = icon('users fw')
         %span
-          Settings
+          Members
+    - if can?(current_user, :admin_group, @group)
+      = nav_link(html_options: { class: "separate-item" }) do
+        = link_to edit_group_path(@group), title: 'Settings', data: {placement: 'right'} do
+          = icon ('cogs fw')
+          %span
+            Settings
diff --git a/features/explore/groups.feature b/features/explore/groups.feature
index c11634bd74a..a42e59c98f2 100644
--- a/features/explore/groups.feature
+++ b/features/explore/groups.feature
@@ -3,20 +3,6 @@ Feature: Explore Groups
   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
diff --git a/features/groups.feature b/features/groups.feature
index d5272fdddcf..db37fa3b375 100644
--- a/features/groups.feature
+++ b/features/groups.feature
@@ -159,3 +159,14 @@ Feature: Groups
     When I visit group "Owned" projects page
     Then I should see group "Owned" projects list
     And I should see "archived" label
+
+  # Public group
+  @javascript
+  Scenario: Signed out user should see group
+    Given "Mary Jane" is owner of group "Owned"
+    And I am a signed out user
+    And Group "Owned" has a public project "Public-project"
+    When I visit group "Owned" page
+    Then I should see group "Owned"
+    Then I should see project "Public-project"
+   
diff --git a/features/steps/groups.rb b/features/steps/groups.rb
index a5c2eed4ddd..95bc9baf8d8 100644
--- a/features/steps/groups.rb
+++ b/features/steps/groups.rb
@@ -17,6 +17,26 @@ class Spinach::Features::Groups < Spinach::FeatureSteps
     find(:css, 'button.btn-new').click
   end
 
+  step 'I should see group "Owned"' do
+    expect(page).to have_content '@owned'
+  end
+
+  step 'I am a signed out user' do
+    logout
+  end
+
+  step 'Group "Owned" has a public project "Public-project"' do
+    group = Group.find_by(name: "Owned")
+
+    @project = create :empty_project, :public,
+                 group: group,
+                 name: "Public-project"
+  end
+
+  step 'I should see project "Public-project"' do
+    expect(page).to have_content 'Public-project'
+  end
+
   step 'I select "Mike" as "Reporter"' do
     user = User.find_by(name: "Mike")
 
diff --git a/spec/controllers/namespaces_controller_spec.rb b/spec/controllers/namespaces_controller_spec.rb
index 9c8619722cd..77436958711 100644
--- a/spec/controllers/namespaces_controller_spec.rb
+++ b/spec/controllers/namespaces_controller_spec.rb
@@ -46,13 +46,11 @@ describe NamespacesController do
 
       context "when the project doesn't have public projects" do
         context "when not signed in" do
-          it "redirects to the sign in page" do
+          it "does not redirect to the sign in page" do
             get :show, id: group.path
-
-            expect(response).to redirect_to(new_user_session_path)
+            expect(response).not_to redirect_to(new_user_session_path)
           end
         end
-
         context "when signed in" do
           before do
             sign_in(user)
@@ -86,10 +84,10 @@ describe NamespacesController do
           end
 
           context "when the user doesn't have access to the project" do
-            it "responds with status 404" do
+            it "redirects to the group's page" do
               get :show, id: group.path
 
-              expect(response.status).to eq(404)
+              expect(response).to redirect_to(group_path(group))
             end
           end
         end
diff --git a/spec/controllers/uploads_controller_spec.rb b/spec/controllers/uploads_controller_spec.rb
index 0f9780356b1..af5d043cf02 100644
--- a/spec/controllers/uploads_controller_spec.rb
+++ b/spec/controllers/uploads_controller_spec.rb
@@ -156,14 +156,6 @@ describe UploadsController do
       end
 
       context "when the project doesn't have public projects" do
-        context "when not signed in" do
-          it "redirects to the sign in page" do
-            get :show, model: "group", mounted_as: "avatar", id: group.id, filename: "image.png"
-
-            expect(response).to redirect_to(new_user_session_path)
-          end
-        end
-
         context "when signed in" do
           before do
             sign_in(user)
diff --git a/spec/features/security/group_access_spec.rb b/spec/features/security/group_access_spec.rb
index 8ce15388605..4b78e3a61f0 100644
--- a/spec/features/security/group_access_spec.rb
+++ b/spec/features/security/group_access_spec.rb
@@ -68,7 +68,7 @@ describe 'Group access', feature: true do
       it { is_expected.to be_allowed_for group_member(:guest) }
       it { is_expected.to be_allowed_for :admin }
       it { is_expected.to be_allowed_for :user }
-      it { is_expected.to be_denied_for :visitor }
+      it { is_expected.to be_allowed_for :visitor }
     end
 
     context 'with no projects' do
@@ -77,8 +77,8 @@ describe 'Group access', feature: true do
       it { is_expected.to be_allowed_for group_member(:reporter) }
       it { is_expected.to be_allowed_for group_member(:guest) }
       it { is_expected.to be_allowed_for :admin }
-      it { is_expected.to be_denied_for :user }
-      it { is_expected.to be_denied_for :visitor }
+      it { is_expected.to be_allowed_for :user }
+      it { is_expected.to be_allowed_for :visitor }
     end
   end