From 62f6601c598d59781137109c0eee5c5ea1792e13 Mon Sep 17 00:00:00 2001
From: Felipe Artur <felipefac@gmail.com>
Date: Fri, 15 Apr 2016 12:04:07 -0300
Subject: [PATCH] Show project members only for members

---
 .../projects/project_members_controller.rb    |  7 ++++-
 app/helpers/projects_helper.rb                |  4 +++
 app/models/ability.rb                         | 12 ++++++++-
 app/views/layouts/nav/_project.html.haml      |  2 +-
 .../project_members_controller_spec.rb        | 27 +++++++++++++++++++
 5 files changed, 49 insertions(+), 3 deletions(-)

diff --git a/app/controllers/projects/project_members_controller.rb b/app/controllers/projects/project_members_controller.rb
index e457db2f0b7..f8c9ff657df 100644
--- a/app/controllers/projects/project_members_controller.rb
+++ b/app/controllers/projects/project_members_controller.rb
@@ -1,6 +1,7 @@
 class Projects::ProjectMembersController < Projects::ApplicationController
   # Authorize
-  before_action :authorize_admin_project_member!, except: :leave
+  before_action :authorize_admin_project_member!, except: [:leave, :index]
+  before_action :authorize_read_members_list!, only: [:index]
 
   def index
     @project_members = @project.project_members
@@ -112,4 +113,8 @@ class Projects::ProjectMembersController < Projects::ApplicationController
   def member_params
     params.require(:project_member).permit(:user_id, :access_level)
   end
+
+  def authorize_read_members_list!
+    render_403 unless can?(current_user, :read_members_list , @project)
+  end
 end
diff --git a/app/helpers/projects_helper.rb b/app/helpers/projects_helper.rb
index 7e00aacceaa..fc3662bc097 100644
--- a/app/helpers/projects_helper.rb
+++ b/app/helpers/projects_helper.rb
@@ -144,6 +144,10 @@ module ProjectsHelper
       nav_tabs << :settings
     end
 
+    if can?(current_user, :read_members_list, project)
+      nav_tabs << :team
+    end
+
     if can?(current_user, :read_issue, project)
       nav_tabs << :issues
     end
diff --git a/app/models/ability.rb b/app/models/ability.rb
index 6103a2947e2..a037aee6d51 100644
--- a/app/models/ability.rb
+++ b/app/models/ability.rb
@@ -154,9 +154,17 @@ class Ability
       end
     end
 
+    def project_member_rules(team, user)
+      all_members_rules = []
+
+      #Rules only for members which does not include public behavior
+      all_members_rules << :read_members_list if team.members.include?(user)
+      all_members_rules
+    end
+
     def project_team_rules(team, user)
       # Rules based on role in project
-      if team.master?(user)
+      filtered_rules = if team.master?(user)
         project_master_rules
       elsif team.developer?(user)
         project_dev_rules
@@ -165,6 +173,8 @@ class Ability
       elsif team.guest?(user)
         project_guest_rules
       end
+
+      Array(filtered_rules) + project_member_rules(team, user)
     end
 
     def public_project_rules
diff --git a/app/views/layouts/nav/_project.html.haml b/app/views/layouts/nav/_project.html.haml
index 86b46e8c75e..a15b7758c4b 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? :team
     = 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/projects/project_members_controller_spec.rb b/spec/controllers/projects/project_members_controller_spec.rb
index d47e4ab9a4f..c52c586cc9b 100644
--- a/spec/controllers/projects/project_members_controller_spec.rb
+++ b/spec/controllers/projects/project_members_controller_spec.rb
@@ -46,4 +46,31 @@ describe Projects::ProjectMembersController do
       end
     end
   end
+
+  describe 'index' do
+    let(:project) { create(:project, :internal) }
+
+    context 'when user is member' do
+      let(:member) { create(:user) }
+
+      before do
+        project.team << [member, :guest]
+        sign_in(member)
+        get :index, namespace_id: project.namespace.to_param, project_id: project.to_param
+      end
+
+       it { expect(response.status).to eq(200) }
+    end
+
+    context 'when user is not member' do
+      let(:not_member) { create(:user) }
+
+      before do
+        sign_in(not_member)
+        get :index, namespace_id: project.namespace.to_param, project_id: project.to_param
+      end
+
+      it { expect(response.status).to eq(403) }
+    end
+  end
 end