From 09c8cf9de68c5d6f1250d6717b00f3b7e2008d3f Mon Sep 17 00:00:00 2001
From: Felipe Artur <felipefac@gmail.com>
Date: Thu, 7 Apr 2016 16:36:26 -0300
Subject: [PATCH] Remove group members check

---
 app/controllers/groups/group_members_controller.rb  |  7 -------
 app/models/ability.rb                               |  8 +++-----
 app/views/layouts/nav/_group.html.haml              | 13 +++++--------
 app/views/layouts/nav/_project.html.haml            |  2 +-
 .../groups/group_members_controller_spec.rb         |  8 +++++---
 5 files changed, 14 insertions(+), 24 deletions(-)

diff --git a/app/controllers/groups/group_members_controller.rb b/app/controllers/groups/group_members_controller.rb
index 9fc72635806..d5ef33888c6 100644
--- a/app/controllers/groups/group_members_controller.rb
+++ b/app/controllers/groups/group_members_controller.rb
@@ -1,7 +1,6 @@
 class Groups::GroupMembersController < Groups::ApplicationController
   # Authorize
   before_action :authorize_admin_group_member!, except: [:index, :leave]
-  before_action :authorize_read_group_members!, only: [:index]
 
   def index
     @project = @group.projects.find(params[:project_id]) if params[:project_id]
@@ -80,10 +79,4 @@ class Groups::GroupMembersController < Groups::ApplicationController
   def member_params
     params.require(:group_member).permit(:access_level, :user_id)
   end
-
-  private
-
-  def authorize_read_group_members!
-    render_404 unless can?(current_user, :read_group_members, @group)
-  end
 end
diff --git a/app/models/ability.rb b/app/models/ability.rb
index e327d4eef28..a4bde72d991 100644
--- a/app/models/ability.rb
+++ b/app/models/ability.rb
@@ -57,6 +57,7 @@ class Ability
           :read_label,
           :read_milestone,
           :read_project_snippet,
+          :read_project_member,
           :read_merge_request,
           :read_note,
           :read_commit_status,
@@ -91,10 +92,7 @@ class Ability
                 subject.group
               end
 
-      if group.public?
-        rules << :read_group
-        rules << :read_group_members unless restricted_public_level?
-      end
+      rules << :read_group if group.public?
 
       rules
     end
@@ -293,7 +291,7 @@ class Ability
 
     def group_abilities(user, group)
       rules = []
-      rules << [:read_group, :read_group_members] if can_read_group?(user, group)
+      rules << :read_group if can_read_group?(user, group)
 
       # Only group masters and group owners can create new projects
       if group.has_master?(user) || group.has_owner?(user) || user.admin?
diff --git a/app/views/layouts/nav/_group.html.haml b/app/views/layouts/nav/_group.html.haml
index 927f61c89fa..55940741dc0 100644
--- a/app/views/layouts/nav/_group.html.haml
+++ b/app/views/layouts/nav/_group.html.haml
@@ -36,14 +36,11 @@
         Merge Requests
         - merge_requests = MergeRequestsFinder.new(current_user, group_id: @group.id, state: 'opened').execute
         %span.count= number_with_delimiter(merge_requests.count)
-
-  - if can?(current_user, :read_group_members, @group)
-    = nav_link(controller: [:group_members]) do
-      = link_to group_group_members_path(@group), title: 'Members' do
-        = icon('users fw')
-        %span
-          Members
-
+  = nav_link(controller: [:group_members]) do
+    = link_to group_group_members_path(@group), title: 'Members' 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' do
diff --git a/app/views/layouts/nav/_project.html.haml b/app/views/layouts/nav/_project.html.haml
index 2c9e2159486..86b46e8c75e 100644
--- a/app/views/layouts/nav/_project.html.haml
+++ b/app/views/layouts/nav/_project.html.haml
@@ -77,7 +77,7 @@
           Merge Requests
           %span.count.merge_counter= number_with_delimiter(@project.merge_requests.opened.count)
 
-  - if project_nav_tab?(:settings)
+  - if project_nav_tab? :settings
     = nav_link(controller: [:project_members, :teams]) do
       = link_to namespace_project_project_members_path(@project.namespace, @project), title: 'Members', class: 'team-tab tab' do
         = icon('users fw')
diff --git a/spec/controllers/groups/group_members_controller_spec.rb b/spec/controllers/groups/group_members_controller_spec.rb
index cf8f2a28052..a5986598715 100644
--- a/spec/controllers/groups/group_members_controller_spec.rb
+++ b/spec/controllers/groups/group_members_controller_spec.rb
@@ -4,15 +4,17 @@ describe Groups::GroupMembersController do
   let(:user)  { create(:user) }
   let(:group) { create(:group) }
 
-  context "when public visibility level is restricted" do
+  context "index" do
     before do
       group.add_owner(user)
       stub_application_setting(restricted_visibility_levels: [Gitlab::VisibilityLevel::PUBLIC])
     end
 
-    it 'does not show group members' do
+    it 'renders index with group members' do
       get :index, group_id: group.path
-      expect(response.status).to eq(404)
+
+      expect(response.status).to eq(200)
+      expect(response).to render_template(:index)
     end
   end
 end