Move verification to abilities

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