Add specs and fix code

6 files changed, 65 insertions, 18 deletions

diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb
index 69b66e161cf..642f5eea1de 100644
--- a/app/controllers/users_controller.rb
+++ b/app/controllers/users_controller.rb
@@ -1,6 +1,6 @@
 class UsersController < ApplicationController
   skip_before_action :authenticate_user!
-  #TO-DO Remove this "set_user" before action. It is not good to use before filters for loading database records.
+  #TODO felipe_artur: Remove this "set_user" before action. It is not good to use before filters for loading database records.
   before_action :set_user, except: [:show]
   before_action :authorize_read_user, only: [:show]
 
diff --git a/app/models/ability.rb b/app/models/ability.rb
index d3e724b84ec..2914ca16b2d 100644
--- a/app/models/ability.rb
+++ b/app/models/ability.rb
@@ -1,4 +1,6 @@
 class Ability
+  @public_restricted = nil
+
   class << self
     def allowed(user, subject)
       return anonymous_abilities(user, subject) if user.nil?
@@ -18,7 +20,7 @@ class Ability
       when Namespace then namespace_abilities(user, subject)
       when GroupMember then group_member_abilities(user, subject)
       when ProjectMember then project_member_abilities(user, subject)
-      when User then user_abilities()
+      when User then user_abilities
       else []
       end.concat(global_abilities(user))
     end
@@ -37,7 +39,7 @@ class Ability
       when subject.is_a?(Group) || subject.respond_to?(:group)
         anonymous_group_abilities(subject)
       when subject.is_a?(User)
-        anonymous_user_abilities()
+        anonymous_user_abilities
       else
         []
       end
@@ -71,8 +73,7 @@ class Ability
         rules << :read_issue unless subject.is_a?(Issue) && subject.confidential?
 
         # Allow anonymous users to read project members if public is not a restricted level
-        restricted_public_level = current_application_settings.restricted_visibility_levels.include?(Gitlab::VisibilityLevel::PUBLIC)
-        rules << :read_project_member unless restricted_public_level
+        rules << :read_project_member unless restricted_public_level?
 
         rules - project_disabled_features_rules(project)
       else
@@ -100,8 +101,7 @@ class Ability
         rules << [:read_group] if group.public?
 
         # Allow anonymous users to read project members if public is not a restricted level
-        restricted_public_level = current_application_settings.restricted_visibility_levels.include?(Gitlab::VisibilityLevel::PUBLIC)
-        rules << [:read_group_members] unless restricted_public_level
+        rules << [:read_group_members] unless restricted_public_level?
       end
 
       rules
@@ -123,9 +123,8 @@ class Ability
       end
     end
 
-    def anonymous_user_abilities()
-      restricted_by_public = current_application_settings.restricted_visibility_levels.include?(Gitlab::VisibilityLevel::PUBLIC)
-      [:read_user] unless restricted_by_public
+    def anonymous_user_abilities
+      [:read_user] unless restricted_public_level?
     end
 
     def global_abilities(user)
@@ -303,7 +302,6 @@ class Ability
 
     def group_abilities(user, group)
       rules = []
-
       rules << [:read_group, :read_group_members] if can_read_group?(user, group)
 
       # Only group masters and group owners can create new projects
@@ -475,7 +473,7 @@ class Ability
       rules
     end
 
-    def user_abilities()
+    def user_abilities
       [:read_user]
     end
 
@@ -493,6 +491,11 @@ class Ability
 
     private
 
+    def restricted_public_level?
+      @public_restricted ||= current_application_settings.restricted_visibility_levels.include?(Gitlab::VisibilityLevel::PUBLIC)
+      @public_restricted
+    end
+
     def named_abilities(name)
       [
         :"read_#{name}",
diff --git a/app/views/layouts/nav/_group.html.haml b/app/views/layouts/nav/_group.html.haml
index 55940741dc0..927f61c89fa 100644
--- a/app/views/layouts/nav/_group.html.haml
+++ b/app/views/layouts/nav/_group.html.haml
@@ -36,11 +36,14 @@
         Merge Requests
         - merge_requests = MergeRequestsFinder.new(current_user, group_id: @group.id, state: 'opened').execute
         %span.count= number_with_delimiter(merge_requests.count)
-  = 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, :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
+
   - 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 86b46e8c75e..d651de0fbe0 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) && can?(current_user, :read_project_members, @project)
     = 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
new file mode 100644
index 00000000000..3a4dd2bf1fa
--- /dev/null
+++ b/spec/controllers/groups/group_members_controller_spec.rb
@@ -0,0 +1,19 @@
+require 'spec_helper'
+
+describe Groups::GroupMembersController do
+  let(:user)  { create(:user) }
+  let(:group) { create(:group) }
+
+
+  context "When public visibility level is restricted" do
+    before do
+      group.add_owner(user)
+      stub_application_setting(restricted_visibility_levels: [Gitlab::VisibilityLevel::PUBLIC])
+    end
+
+    it 'does not show group members' do
+      get :index, group_id: group.path
+      expect(response.status).to eq(404)
+    end
+  end
+end
diff --git a/spec/controllers/users_controller_spec.rb b/spec/controllers/users_controller_spec.rb
index 7337ff58be1..f6235c29a17 100644
--- a/spec/controllers/users_controller_spec.rb
+++ b/spec/controllers/users_controller_spec.rb
@@ -38,6 +38,28 @@ describe UsersController do
         end
       end
     end
+
+    context 'When public visibility level is restricted' do
+      before do
+        stub_application_setting(restricted_visibility_levels: [Gitlab::VisibilityLevel::PUBLIC])
+      end
+
+      context 'when logged out' do
+        it 'renders 404' do
+          get :show, username: user.username
+          expect(response.status).to eq(404)
+        end
+      end
+
+      context 'when logged in' do
+        before { sign_in(user) }
+
+        it 'renders 404' do
+          get :show, username: user.username
+          expect(response.status).to eq(200)
+        end
+      end
+    end
   end
 
   describe 'GET #calendar' do