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