From b05f0a48584ea45cc89a8efaafd8e54642b8497c Mon Sep 17 00:00:00 2001
From: Felipe Artur <felipefac@gmail.com>
Date: Thu, 24 Mar 2016 12:55:04 -0300
Subject: [PATCH 01/11] Restrict user profiles based on restricted visibility
 levels

---
 app/controllers/users_controller.rb | 4 ++++
 app/models/user.rb                  | 4 ++++
 2 files changed, 8 insertions(+)

diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb
index 8e7956da48f..49ddcfed7b1 100644
--- a/app/controllers/users_controller.rb
+++ b/app/controllers/users_controller.rb
@@ -1,6 +1,7 @@
 class UsersController < ApplicationController
   skip_before_action :authenticate_user!
   before_action :set_user
+  before_filter :authorize_read_user, only: [:show]
 
   def show
     respond_to do |format|
@@ -74,6 +75,9 @@ class UsersController < ApplicationController
   end
 
   private
+  def authorize_read_user
+    render_404 unless @user.public?
+  end
 
   def set_user
     @user = User.find_by_username!(params[:username])
diff --git a/app/models/user.rb b/app/models/user.rb
index 031315debd7..e2b602d598b 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -835,6 +835,10 @@ class User < ActiveRecord::Base
     notification_settings.find_or_initialize_by(source: source)
   end
 
+  def public?
+    current_application_settings.restricted_visibility_levels.include?(Gitlab::VisibilityLevel::PUBLIC)
+  end
+
   private
 
   def projects_union

From 57519565f167cb771ffed504feefe7b0eb37c027 Mon Sep 17 00:00:00 2001
From: Felipe Artur <felipefac@gmail.com>
Date: Tue, 29 Mar 2016 12:24:42 -0300
Subject: [PATCH 02/11] Move verification to abilities

---
 .../groups/group_members_controller.rb        |  7 ++++
 .../projects/project_members_controller.rb    |  7 ++++
 app/controllers/users_controller.rb           |  8 +++--
 app/models/ability.rb                         | 33 ++++++++++++++++---
 app/models/user.rb                            |  4 ---
 5 files changed, 47 insertions(+), 12 deletions(-)

diff --git a/app/controllers/groups/group_members_controller.rb b/app/controllers/groups/group_members_controller.rb
index d5ef33888c6..50398d5d3b4 100644
--- a/app/controllers/groups/group_members_controller.rb
+++ b/app/controllers/groups/group_members_controller.rb
@@ -1,6 +1,7 @@
 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]
@@ -79,4 +80,10 @@ 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/controllers/projects/project_members_controller.rb b/app/controllers/projects/project_members_controller.rb
index e457db2f0b7..7badbb47d0c 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_read_project_members, only: :index
 
   def index
     @project_members = @project.project_members
@@ -112,4 +113,10 @@ class Projects::ProjectMembersController < Projects::ApplicationController
   def member_params
     params.require(:project_member).permit(:user_id, :access_level)
   end
+
+  private
+
+  def authorize_read_project_members
+    can?(current_user, :read_project_members, @project)
+  end
 end
diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb
index 49ddcfed7b1..69b66e161cf 100644
--- a/app/controllers/users_controller.rb
+++ b/app/controllers/users_controller.rb
@@ -1,7 +1,8 @@
 class UsersController < ApplicationController
   skip_before_action :authenticate_user!
-  before_action :set_user
-  before_filter :authorize_read_user, only: [:show]
+  #TO-DO 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]
 
   def show
     respond_to do |format|
@@ -76,7 +77,8 @@ class UsersController < ApplicationController
 
   private
   def authorize_read_user
-    render_404 unless @user.public?
+    set_user
+    render_404 unless can?(current_user, :read_user, @user)
   end
 
   def set_user
diff --git a/app/models/ability.rb b/app/models/ability.rb
index c0bf6def7c5..d3e724b84ec 100644
--- a/app/models/ability.rb
+++ b/app/models/ability.rb
@@ -18,6 +18,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()
       else []
       end.concat(global_abilities(user))
     end
@@ -35,6 +36,8 @@ class Ability
         anonymous_project_abilities(subject)
       when subject.is_a?(Group) || subject.respond_to?(:group)
         anonymous_group_abilities(subject)
+      when subject.is_a?(User)
+        anonymous_user_abilities()
       else
         []
       end
@@ -67,6 +70,10 @@ class Ability
         # Allow to read issues by anonymous user if issue is not confidential
         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 - project_disabled_features_rules(project)
       else
         []
@@ -81,17 +88,23 @@ class Ability
     end
 
     def anonymous_group_abilities(subject)
+      rules = []
+
       group = if subject.is_a?(Group)
                 subject
               else
                 subject.group
               end
 
-      if group && group.public?
-        [:read_group]
-      else
-        []
+      if group
+        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
       end
+
+      rules
     end
 
     def anonymous_personal_snippet_abilities(snippet)
@@ -110,6 +123,11 @@ 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
+    end
+
     def global_abilities(user)
       rules = []
       rules << :create_group if user.can_create_group
@@ -164,6 +182,7 @@ class Ability
         :download_code,
         :fork_project,
         :read_commit_status,
+        :read_project_members
       ]
     end
 
@@ -285,7 +304,7 @@ class Ability
     def group_abilities(user, group)
       rules = []
 
-      rules << :read_group if can_read_group?(user, group)
+      rules << [:read_group, :read_group_members] 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?
@@ -456,6 +475,10 @@ class Ability
       rules
     end
 
+    def user_abilities()
+      [:read_user]
+    end
+
     def abilities
       @abilities ||= begin
         abilities = Six.new
diff --git a/app/models/user.rb b/app/models/user.rb
index e2b602d598b..031315debd7 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -835,10 +835,6 @@ class User < ActiveRecord::Base
     notification_settings.find_or_initialize_by(source: source)
   end
 
-  def public?
-    current_application_settings.restricted_visibility_levels.include?(Gitlab::VisibilityLevel::PUBLIC)
-  end
-
   private
 
   def projects_union

From 668d6ffa437aa5c920e987beb5de4e8dacbfd00c Mon Sep 17 00:00:00 2001
From: Felipe Artur <felipefac@gmail.com>
Date: Wed, 30 Mar 2016 17:14:21 -0300
Subject: [PATCH 03/11] Add specs and fix code

---
 app/controllers/users_controller.rb           |  2 +-
 app/models/ability.rb                         | 25 +++++++++++--------
 app/views/layouts/nav/_group.html.haml        | 13 ++++++----
 app/views/layouts/nav/_project.html.haml      |  2 +-
 .../groups/group_members_controller_spec.rb   | 19 ++++++++++++++
 spec/controllers/users_controller_spec.rb     | 22 ++++++++++++++++
 6 files changed, 65 insertions(+), 18 deletions(-)
 create mode 100644 spec/controllers/groups/group_members_controller_spec.rb

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

From e8a77c0aee3eaf99793b3678a0eb97194244b339 Mon Sep 17 00:00:00 2001
From: Felipe Artur <felipefac@gmail.com>
Date: Thu, 31 Mar 2016 11:36:40 -0300
Subject: [PATCH 04/11] Fix code

---
 app/controllers/groups/group_members_controller.rb       | 4 ++--
 app/controllers/users_controller.rb                      | 6 +++---
 app/models/ability.rb                                    | 4 ----
 spec/controllers/groups/group_members_controller_spec.rb | 3 +--
 spec/controllers/users_controller_spec.rb                | 3 ++-
 5 files changed, 8 insertions(+), 12 deletions(-)

diff --git a/app/controllers/groups/group_members_controller.rb b/app/controllers/groups/group_members_controller.rb
index 50398d5d3b4..9fc72635806 100644
--- a/app/controllers/groups/group_members_controller.rb
+++ b/app/controllers/groups/group_members_controller.rb
@@ -1,7 +1,7 @@
 class Groups::GroupMembersController < Groups::ApplicationController
   # Authorize
   before_action :authorize_admin_group_member!, except: [:index, :leave]
-  before_action :authorize_read_group_members, only: [:index]
+  before_action :authorize_read_group_members!, only: [:index]
 
   def index
     @project = @group.projects.find(params[:project_id]) if params[:project_id]
@@ -83,7 +83,7 @@ class Groups::GroupMembersController < Groups::ApplicationController
 
   private
 
-  def authorize_read_group_members
+  def authorize_read_group_members!
     render_404 unless can?(current_user, :read_group_members, @group)
   end
 end
diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb
index 642f5eea1de..233dca54b99 100644
--- a/app/controllers/users_controller.rb
+++ b/app/controllers/users_controller.rb
@@ -1,8 +1,7 @@
 class UsersController < ApplicationController
   skip_before_action :authenticate_user!
-  #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]
+  before_action :authorize_read_user!, only: [:show]
 
   def show
     respond_to do |format|
@@ -76,7 +75,8 @@ class UsersController < ApplicationController
   end
 
   private
-  def authorize_read_user
+
+  def authorize_read_user!
     set_user
     render_404 unless can?(current_user, :read_user, @user)
   end
diff --git a/app/models/ability.rb b/app/models/ability.rb
index 2914ca16b2d..874ec360944 100644
--- a/app/models/ability.rb
+++ b/app/models/ability.rb
@@ -1,5 +1,4 @@
 class Ability
-  @public_restricted = nil
 
   class << self
     def allowed(user, subject)
@@ -72,7 +71,6 @@ class Ability
         # Allow to read issues by anonymous user if issue is not confidential
         rules << :read_issue unless subject.is_a?(Issue) && subject.confidential?
 
-        # Allow anonymous users to read project members if public is not a restricted level
         rules << :read_project_member unless restricted_public_level?
 
         rules - project_disabled_features_rules(project)
@@ -100,7 +98,6 @@ class Ability
       if group
         rules << [:read_group] if group.public?
 
-        # Allow anonymous users to read project members if public is not a restricted level
         rules << [:read_group_members] unless restricted_public_level?
       end
 
@@ -493,7 +490,6 @@ class Ability
 
     def restricted_public_level?
       @public_restricted ||= current_application_settings.restricted_visibility_levels.include?(Gitlab::VisibilityLevel::PUBLIC)
-      @public_restricted
     end
 
     def named_abilities(name)
diff --git a/spec/controllers/groups/group_members_controller_spec.rb b/spec/controllers/groups/group_members_controller_spec.rb
index 3a4dd2bf1fa..cf8f2a28052 100644
--- a/spec/controllers/groups/group_members_controller_spec.rb
+++ b/spec/controllers/groups/group_members_controller_spec.rb
@@ -4,8 +4,7 @@ describe Groups::GroupMembersController do
   let(:user)  { create(:user) }
   let(:group) { create(:group) }
 
-
-  context "When public visibility level is restricted" do
+  context "when public visibility level is restricted" do
     before do
       group.add_owner(user)
       stub_application_setting(restricted_visibility_levels: [Gitlab::VisibilityLevel::PUBLIC])
diff --git a/spec/controllers/users_controller_spec.rb b/spec/controllers/users_controller_spec.rb
index f6235c29a17..26acfd3bd96 100644
--- a/spec/controllers/users_controller_spec.rb
+++ b/spec/controllers/users_controller_spec.rb
@@ -54,9 +54,10 @@ describe UsersController do
       context 'when logged in' do
         before { sign_in(user) }
 
-        it 'renders 404' do
+        it 'renders show' do
           get :show, username: user.username
           expect(response.status).to eq(200)
+          expect(response).to render_template('show')
         end
       end
     end

From 147879ae66fd742d13bbb5b72d492788bc48c8d9 Mon Sep 17 00:00:00 2001
From: Felipe Artur <felipefac@gmail.com>
Date: Fri, 1 Apr 2016 16:50:18 -0300
Subject: [PATCH 05/11] Fix specs

---
 app/models/ability.rb                     | 4 ++--
 spec/controllers/users_controller_spec.rb | 4 +++-
 2 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/app/models/ability.rb b/app/models/ability.rb
index 874ec360944..684834aa394 100644
--- a/app/models/ability.rb
+++ b/app/models/ability.rb
@@ -156,6 +156,7 @@ class Ability
           rules -= project_archived_rules
         end
 
+        rules << :read_project_members
         rules - project_disabled_features_rules(project)
       end
     end
@@ -177,8 +178,7 @@ class Ability
       @public_project_rules ||= project_guest_rules + [
         :download_code,
         :fork_project,
-        :read_commit_status,
-        :read_project_members
+        :read_commit_status
       ]
     end
 
diff --git a/spec/controllers/users_controller_spec.rb b/spec/controllers/users_controller_spec.rb
index 26acfd3bd96..7701da9747a 100644
--- a/spec/controllers/users_controller_spec.rb
+++ b/spec/controllers/users_controller_spec.rb
@@ -30,10 +30,12 @@ describe UsersController do
       end
 
       describe 'when logged out' do
+        before { stub_application_setting(restricted_visibility_levels: []) }
+
         it 'renders the show template' do
           get :show, username: user.username
 
-          expect(response).to be_success
+          expect(response.status).to eq(200)
           expect(response).to render_template('show')
         end
       end

From 07b38c3b389b8b0b6a3d6af7a38555c189e71afe Mon Sep 17 00:00:00 2001
From: Felipe Artur <felipefac@gmail.com>
Date: Tue, 5 Apr 2016 18:56:07 -0300
Subject: [PATCH 06/11] Code fixes

---
 .../projects/project_members_controller.rb    |  7 -------
 app/controllers/users_controller.rb           | 21 +++++++++----------
 app/models/ability.rb                         | 10 ++-------
 app/views/layouts/nav/_project.html.haml      |  2 +-
 spec/controllers/users_controller_spec.rb     |  2 +-
 5 files changed, 14 insertions(+), 28 deletions(-)

diff --git a/app/controllers/projects/project_members_controller.rb b/app/controllers/projects/project_members_controller.rb
index 7badbb47d0c..e457db2f0b7 100644
--- a/app/controllers/projects/project_members_controller.rb
+++ b/app/controllers/projects/project_members_controller.rb
@@ -1,7 +1,6 @@
 class Projects::ProjectMembersController < Projects::ApplicationController
   # Authorize
   before_action :authorize_admin_project_member!, except: :leave
-  before_action :authorize_read_project_members, only: :index
 
   def index
     @project_members = @project.project_members
@@ -113,10 +112,4 @@ class Projects::ProjectMembersController < Projects::ApplicationController
   def member_params
     params.require(:project_member).permit(:user_id, :access_level)
   end
-
-  private
-
-  def authorize_read_project_members
-    can?(current_user, :read_project_members, @project)
-  end
 end
diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb
index 233dca54b99..2ae180c8a12 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!
-  before_action :set_user, except: [:show]
+  before_action :user
   before_action :authorize_read_user!, only: [:show]
 
   def show
@@ -77,26 +77,25 @@ class UsersController < ApplicationController
   private
 
   def authorize_read_user!
-    set_user
-    render_404 unless can?(current_user, :read_user, @user)
+    render_404 unless can?(current_user, :read_user, user)
   end
 
-  def set_user
-    @user = User.find_by_username!(params[:username])
+  def user
+    @user ||= User.find_by_username!(params[:username])
   end
 
   def contributed_projects
-    ContributedProjectsFinder.new(@user).execute(current_user)
+    ContributedProjectsFinder.new(user).execute(current_user)
   end
 
   def contributions_calendar
     @contributions_calendar ||= Gitlab::ContributionsCalendar.
-      new(contributed_projects, @user)
+      new(contributed_projects, user)
   end
 
   def load_events
     # Get user activity feed for projects common for both users
-    @events = @user.recent_events.
+    @events = user.recent_events.
       merge(projects_for_current_user).
       references(:project).
       with_associations.
@@ -105,16 +104,16 @@ class UsersController < ApplicationController
 
   def load_projects
     @projects =
-      PersonalProjectsFinder.new(@user).execute(current_user)
+      PersonalProjectsFinder.new(user).execute(current_user)
       .page(params[:page])
   end
 
   def load_contributed_projects
-    @contributed_projects = contributed_projects.joined(@user)
+    @contributed_projects = contributed_projects.joined(user)
   end
 
   def load_groups
-    @groups = JoinedGroupsFinder.new(@user).execute(current_user)
+    @groups = JoinedGroupsFinder.new(user).execute(current_user)
   end
 
   def projects_for_current_user
diff --git a/app/models/ability.rb b/app/models/ability.rb
index 684834aa394..7c452c69d14 100644
--- a/app/models/ability.rb
+++ b/app/models/ability.rb
@@ -1,5 +1,4 @@
 class Ability
-
   class << self
     def allowed(user, subject)
       return anonymous_abilities(user, subject) if user.nil?
@@ -58,7 +57,6 @@ class Ability
           :read_label,
           :read_milestone,
           :read_project_snippet,
-          :read_project_member,
           :read_merge_request,
           :read_note,
           :read_commit_status,
@@ -71,8 +69,6 @@ class Ability
         # Allow to read issues by anonymous user if issue is not confidential
         rules << :read_issue unless subject.is_a?(Issue) && subject.confidential?
 
-        rules << :read_project_member unless restricted_public_level?
-
         rules - project_disabled_features_rules(project)
       else
         []
@@ -96,9 +92,8 @@ class Ability
               end
 
       if group
-        rules << [:read_group] if group.public?
-
-        rules << [:read_group_members] unless restricted_public_level?
+        rules << :read_group if group.public?
+        rules << :read_group_members unless restricted_public_level?
       end
 
       rules
@@ -156,7 +151,6 @@ class Ability
           rules -= project_archived_rules
         end
 
-        rules << :read_project_members
         rules - project_disabled_features_rules(project)
       end
     end
diff --git a/app/views/layouts/nav/_project.html.haml b/app/views/layouts/nav/_project.html.haml
index d651de0fbe0..2c9e2159486 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) && can?(current_user, :read_project_members, @project)
+  - 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/users_controller_spec.rb b/spec/controllers/users_controller_spec.rb
index 7701da9747a..948935bc10d 100644
--- a/spec/controllers/users_controller_spec.rb
+++ b/spec/controllers/users_controller_spec.rb
@@ -41,7 +41,7 @@ describe UsersController do
       end
     end
 
-    context 'When public visibility level is restricted' do
+    context 'when public visibility level is restricted' do
       before do
         stub_application_setting(restricted_visibility_levels: [Gitlab::VisibilityLevel::PUBLIC])
       end

From ce96d482d9056e9acdfea02d055c2706653cba92 Mon Sep 17 00:00:00 2001
From: Felipe Artur <felipefac@gmail.com>
Date: Wed, 6 Apr 2016 18:09:24 -0300
Subject: [PATCH 07/11] Insert users check into api

---
 app/models/ability.rb                     |  6 +++---
 lib/api/api_guard.rb                      |  4 ++++
 lib/api/users.rb                          | 10 ++++++++--
 spec/controllers/users_controller_spec.rb |  2 --
 spec/requests/api/users_spec.rb           | 18 ++++++++++++++++++
 5 files changed, 33 insertions(+), 7 deletions(-)

diff --git a/app/models/ability.rb b/app/models/ability.rb
index 7c452c69d14..e327d4eef28 100644
--- a/app/models/ability.rb
+++ b/app/models/ability.rb
@@ -91,8 +91,8 @@ class Ability
                 subject.group
               end
 
-      if group
-        rules << :read_group if group.public?
+      if group.public?
+        rules << :read_group
         rules << :read_group_members unless restricted_public_level?
       end
 
@@ -483,7 +483,7 @@ class Ability
     private
 
     def restricted_public_level?
-      @public_restricted ||= current_application_settings.restricted_visibility_levels.include?(Gitlab::VisibilityLevel::PUBLIC)
+      current_application_settings.restricted_visibility_levels.include?(Gitlab::VisibilityLevel::PUBLIC)
     end
 
     def named_abilities(name)
diff --git a/lib/api/api_guard.rb b/lib/api/api_guard.rb
index b9994fcefda..6ce5529abfa 100644
--- a/lib/api/api_guard.rb
+++ b/lib/api/api_guard.rb
@@ -79,6 +79,10 @@ module APIGuard
       @current_user
     end
 
+    def public_access_restricted?
+      current_application_settings.restricted_visibility_levels.include?(Gitlab::VisibilityLevel::PUBLIC)
+    end
+
     private
     def find_access_token
       @access_token ||= Doorkeeper.authenticate(doorkeeper_request, Doorkeeper.configuration.access_token_methods)
diff --git a/lib/api/users.rb b/lib/api/users.rb
index 0a14bac07c0..9647a40686e 100644
--- a/lib/api/users.rb
+++ b/lib/api/users.rb
@@ -11,6 +11,10 @@ module API
       #  GET /users?search=Admin
       #  GET /users?username=root
       get do
+        if !current_user && public_access_restricted?
+          render_api_error!("Not authorized.", 403)
+        end
+
         if params[:username].present?
           @users = User.where(username: params[:username])
         else
@@ -36,10 +40,12 @@ module API
       get ":id" do
         @user = User.find(params[:id])
 
-        if current_user.is_admin?
+        if current_user.present? && current_user.is_admin?
           present @user, with: Entities::UserFull
-        else
+        elsif can?(current_user, :read_user, @user)
           present @user, with: Entities::User
+        else
+          render_api_error!("User not found.", 404)
         end
       end
 
diff --git a/spec/controllers/users_controller_spec.rb b/spec/controllers/users_controller_spec.rb
index 948935bc10d..8045c8b940d 100644
--- a/spec/controllers/users_controller_spec.rb
+++ b/spec/controllers/users_controller_spec.rb
@@ -30,8 +30,6 @@ describe UsersController do
       end
 
       describe 'when logged out' do
-        before { stub_application_setting(restricted_visibility_levels: []) }
-
         it 'renders the show template' do
           get :show, username: user.username
 
diff --git a/spec/requests/api/users_spec.rb b/spec/requests/api/users_spec.rb
index 679227bf881..40b24c125b5 100644
--- a/spec/requests/api/users_spec.rb
+++ b/spec/requests/api/users_spec.rb
@@ -20,6 +20,24 @@ describe API::API, api: true  do
     end
 
     context "when authenticated" do
+      #These specs are written just in case API authentication is not required anymore
+      context "when public level is restricted" do
+        before do
+          stub_application_setting(restricted_visibility_levels: [Gitlab::VisibilityLevel::PUBLIC])
+          allow_any_instance_of(API::Helpers).to receive(:authenticate!).and_return(true)
+        end
+
+        it "renders 403" do
+          get api("/users")
+          expect(response.status).to eq(403)
+        end
+
+        it "renders 404" do
+          get api("/users/#{user.id}")
+          expect(response.status).to eq(404)
+        end
+      end
+
       it "should return an array of users" do
         get api("/users", user)
         expect(response.status).to eq(200)

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 08/11] 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

From 7d54e721da0ccd21f0150bbb6ab60b51970033c2 Mon Sep 17 00:00:00 2001
From: Felipe Artur <felipefac@gmail.com>
Date: Thu, 7 Apr 2016 17:44:46 -0300
Subject: [PATCH 09/11] Insert instructions in admin page and permissions
 document

---
 app/views/admin/application_settings/_form.html.haml | 4 +++-
 doc/permissions/permissions.md                       | 7 +++++++
 2 files changed, 10 insertions(+), 1 deletion(-)

diff --git a/app/views/admin/application_settings/_form.html.haml b/app/views/admin/application_settings/_form.html.haml
index 555aea554f0..37b07c348d4 100644
--- a/app/views/admin/application_settings/_form.html.haml
+++ b/app/views/admin/application_settings/_form.html.haml
@@ -26,7 +26,9 @@
         .btn-group{ data: data_attrs }
           - restricted_level_checkboxes('restricted-visibility-help').each do |level|
             = level
-        %span.help-block#restricted-visibility-help Selected levels cannot be used by non-admin users for projects or snippets
+        %span.help-block#restricted-visibility-help
+          Selected levels cannot be used by non-admin users for projects or snippets.
+          If public level is restricted user profiles are not accessible to not logged users.
     .form-group
       = f.label :import_sources, class: 'control-label col-sm-2'
       .col-sm-10
diff --git a/doc/permissions/permissions.md b/doc/permissions/permissions.md
index 6219693b8a8..f8cfd2898f0 100644
--- a/doc/permissions/permissions.md
+++ b/doc/permissions/permissions.md
@@ -93,3 +93,10 @@ An administrator can flag a user as external [through the API](../api/users.md)
 or by checking the checkbox on the admin panel. As an administrator, navigate
 to **Admin > Users** to create a new user or edit an existing one. There, you
 will find the option to flag the user as external.
+
+## Restricted visibility levels
+
+Visibility levels can be restricted in admin settings page by administrator, when
+restricting a visibility level groups, projects and snippets are not allowed to be
+created with that visibility setting. If the public visibility level is restricted
+user profiles are accessible to not logged users.

From 820c08cefd78e593e94012061be29000d523ffd0 Mon Sep 17 00:00:00 2001
From: Felipe Artur <felipefac@gmail.com>
Date: Tue, 12 Apr 2016 12:04:33 -0300
Subject: [PATCH 10/11] Fix documentation and improve permissions code

---
 app/models/ability.rb                                | 1 +
 app/views/admin/application_settings/_form.html.haml | 2 +-
 doc/permissions/permissions.md                       | 7 -------
 doc/public_access/public_access.md                   | 3 +++
 lib/api/api_guard.rb                                 | 4 ----
 lib/api/users.rb                                     | 2 +-
 6 files changed, 6 insertions(+), 13 deletions(-)

diff --git a/app/models/ability.rb b/app/models/ability.rb
index a4bde72d991..6103a2947e2 100644
--- a/app/models/ability.rb
+++ b/app/models/ability.rb
@@ -120,6 +120,7 @@ class Ability
     def global_abilities(user)
       rules = []
       rules << :create_group if user.can_create_group
+      rules << :read_users_list
       rules
     end
 
diff --git a/app/views/admin/application_settings/_form.html.haml b/app/views/admin/application_settings/_form.html.haml
index 37b07c348d4..aadd2c54f20 100644
--- a/app/views/admin/application_settings/_form.html.haml
+++ b/app/views/admin/application_settings/_form.html.haml
@@ -28,7 +28,7 @@
             = level
         %span.help-block#restricted-visibility-help
           Selected levels cannot be used by non-admin users for projects or snippets.
-          If public level is restricted user profiles are not accessible to not logged users.
+          If the public level is restricted, user profiles are only visible to logged in users.
     .form-group
       = f.label :import_sources, class: 'control-label col-sm-2'
       .col-sm-10
diff --git a/doc/permissions/permissions.md b/doc/permissions/permissions.md
index f8cfd2898f0..6219693b8a8 100644
--- a/doc/permissions/permissions.md
+++ b/doc/permissions/permissions.md
@@ -93,10 +93,3 @@ An administrator can flag a user as external [through the API](../api/users.md)
 or by checking the checkbox on the admin panel. As an administrator, navigate
 to **Admin > Users** to create a new user or edit an existing one. There, you
 will find the option to flag the user as external.
-
-## Restricted visibility levels
-
-Visibility levels can be restricted in admin settings page by administrator, when
-restricting a visibility level groups, projects and snippets are not allowed to be
-created with that visibility setting. If the public visibility level is restricted
-user profiles are accessible to not logged users.
diff --git a/doc/public_access/public_access.md b/doc/public_access/public_access.md
index 20aa90f0d69..17bb75ececd 100644
--- a/doc/public_access/public_access.md
+++ b/doc/public_access/public_access.md
@@ -58,6 +58,9 @@ you are logged in or not.
 When visiting the public page of a user, you can only see the projects which
 you are privileged to.
 
+If the public level is restricted, user profiles are only visible to logged in users.
+
+
 ## Restricting the use of public or internal projects
 
 In the Admin area under **Settings** (`/admin/application_settings`), you can
diff --git a/lib/api/api_guard.rb b/lib/api/api_guard.rb
index 6ce5529abfa..b9994fcefda 100644
--- a/lib/api/api_guard.rb
+++ b/lib/api/api_guard.rb
@@ -79,10 +79,6 @@ module APIGuard
       @current_user
     end
 
-    def public_access_restricted?
-      current_application_settings.restricted_visibility_levels.include?(Gitlab::VisibilityLevel::PUBLIC)
-    end
-
     private
     def find_access_token
       @access_token ||= Doorkeeper.authenticate(doorkeeper_request, Doorkeeper.configuration.access_token_methods)
diff --git a/lib/api/users.rb b/lib/api/users.rb
index 9647a40686e..315268fc0ca 100644
--- a/lib/api/users.rb
+++ b/lib/api/users.rb
@@ -11,7 +11,7 @@ module API
       #  GET /users?search=Admin
       #  GET /users?username=root
       get do
-        if !current_user && public_access_restricted?
+        unless can?(current_user, :read_users_list, nil)
           render_api_error!("Not authorized.", 403)
         end
 

From 2366768d3b28ea70c91fc49c471e66152650d442 Mon Sep 17 00:00:00 2001
From: Felipe Artur <felipefac@gmail.com>
Date: Wed, 13 Apr 2016 15:37:17 -0300
Subject: [PATCH 11/11] Add changelog entry

---
 CHANGELOG        | 1 +
 lib/api/users.rb | 2 +-
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/CHANGELOG b/CHANGELOG
index 571c31e63a2..3cb89582d53 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -12,6 +12,7 @@ v 8.7.0 (unreleased)
   - Project switcher uses new dropdown styling
   - Load award emoji images separately unless opening the full picker. Saves several hundred KBs of data for most pages. (Connor Shea)
   - Do not include award_emojis in issue and merge_request comment_count !3610 (Lucas Charles)
+  - Restrict user profiles when public visibility level is restricted.
   - All images in discussions and wikis now link to their source files !3464 (Connor Shea).
   - Return status code 303 after a branch DELETE operation to avoid project deletion (Stan Hu)
   - Add setting for customizing the list of trusted proxies !3524
diff --git a/lib/api/users.rb b/lib/api/users.rb
index 315268fc0ca..ea6fa2dc8a8 100644
--- a/lib/api/users.rb
+++ b/lib/api/users.rb
@@ -40,7 +40,7 @@ module API
       get ":id" do
         @user = User.find(params[:id])
 
-        if current_user.present? && current_user.is_admin?
+        if current_user && current_user.is_admin?
           present @user, with: Entities::UserFull
         elsif can?(current_user, :read_user, @user)
           present @user, with: Entities::User