Commit c674c9ea authored by Diego Louzán's avatar Diego Louzán Committed by Imre Farkas

chore: rename User#full_private_access? to User#can_read_all_resources?

Technical debt triggered from admin mode feature
parent f25dfc80
...@@ -45,7 +45,7 @@ class GroupsFinder < UnionFinder ...@@ -45,7 +45,7 @@ class GroupsFinder < UnionFinder
def all_groups def all_groups
return [owned_groups] if params[:owned] return [owned_groups] if params[:owned]
return [groups_with_min_access_level] if min_access_level? return [groups_with_min_access_level] if min_access_level?
return [Group.all] if current_user&.full_private_access? && all_available? return [Group.all] if current_user&.can_read_all_resources? && all_available?
groups = [] groups = []
groups << Gitlab::ObjectHierarchy.new(groups_for_ancestors, groups_for_descendants).all_objects if current_user groups << Gitlab::ObjectHierarchy.new(groups_for_ancestors, groups_for_descendants).all_objects if current_user
......
...@@ -127,7 +127,7 @@ class IssuesFinder < IssuableFinder ...@@ -127,7 +127,7 @@ class IssuesFinder < IssuableFinder
return @user_can_see_all_confidential_issues if defined?(@user_can_see_all_confidential_issues) return @user_can_see_all_confidential_issues if defined?(@user_can_see_all_confidential_issues)
return @user_can_see_all_confidential_issues = false if current_user.blank? return @user_can_see_all_confidential_issues = false if current_user.blank?
return @user_can_see_all_confidential_issues = true if current_user.full_private_access? return @user_can_see_all_confidential_issues = true if current_user.can_read_all_resources?
@user_can_see_all_confidential_issues = @user_can_see_all_confidential_issues =
if project? && project if project? && project
......
...@@ -242,7 +242,7 @@ class Issue < ApplicationRecord ...@@ -242,7 +242,7 @@ class Issue < ApplicationRecord
return false unless readable_by?(user) return false unless readable_by?(user)
user.full_private_access? || user.can_read_all_resources? ||
::Gitlab::ExternalAuthorization.access_allowed?( ::Gitlab::ExternalAuthorization.access_allowed?(
user, project.external_authorization_classification_label) user, project.external_authorization_classification_label)
end end
......
...@@ -186,7 +186,7 @@ class ProjectFeature < ApplicationRecord ...@@ -186,7 +186,7 @@ class ProjectFeature < ApplicationRecord
def team_access?(user, feature) def team_access?(user, feature)
return unless user return unless user
return true if user.full_private_access? return true if user.can_read_all_resources?
project.team.member?(user, ProjectFeature.required_minimum_access_level(feature)) project.team.member?(user, ProjectFeature.required_minimum_access_level(feature))
end end
......
...@@ -1473,9 +1473,7 @@ class User < ApplicationRecord ...@@ -1473,9 +1473,7 @@ class User < ApplicationRecord
self.admin = (new_level == 'admin') self.admin = (new_level == 'admin')
end end
# Does the user have access to all private groups & projects? def can_read_all_resources?
# Overridden in EE to also check auditor?
def full_private_access?
can?(:read_all_resources) can?(:read_all_resources)
end end
......
...@@ -40,6 +40,7 @@ class BasePolicy < DeclarativePolicy::Base ...@@ -40,6 +40,7 @@ class BasePolicy < DeclarativePolicy::Base
prevent :read_cross_project prevent :read_cross_project
end end
# Policy extended in EE to also enable auditors
rule { admin }.enable :read_all_resources rule { admin }.enable :read_all_resources
rule { default }.enable :read_cross_project rule { default }.enable :read_cross_project
......
---
title: Rename User#full_private_access? to User#can_read_all_resources?
merge_request: 21668
author: Diego Louzán
type: other
...@@ -26,7 +26,7 @@ module EE ...@@ -26,7 +26,7 @@ module EE
def elastic_projects def elastic_projects
strong_memoize(:elastic_projects) do strong_memoize(:elastic_projects) do
if current_user&.full_private_access? if current_user&.can_read_all_resources?
:any :any
elsif current_user elsif current_user
current_user.authorized_projects.pluck(:id) # rubocop: disable CodeReuse/ActiveRecord current_user.authorized_projects.pluck(:id) # rubocop: disable CodeReuse/ActiveRecord
......
...@@ -181,7 +181,7 @@ module Elastic ...@@ -181,7 +181,7 @@ module Elastic
def pick_projects_by_visibility(visibility, user, features) def pick_projects_by_visibility(visibility, user, features)
condition = { term: { visibility_level: visibility } } condition = { term: { visibility_level: visibility } }
limit_by_feature(condition, features, include_members_only: user&.full_private_access?) limit_by_feature(condition, features, include_members_only: user&.can_read_all_resources?)
end end
# If a project feature(s) is specified, access is dependent on its visibility # If a project feature(s) is specified, access is dependent on its visibility
......
...@@ -21,7 +21,7 @@ module Elastic ...@@ -21,7 +21,7 @@ module Elastic
private private
def confidentiality_filter(query_hash, current_user) def confidentiality_filter(query_hash, current_user)
return query_hash if current_user && current_user.full_private_access? return query_hash if current_user && current_user.can_read_all_resources?
filter = filter =
if current_user if current_user
......
...@@ -29,7 +29,7 @@ module Elastic ...@@ -29,7 +29,7 @@ module Elastic
private private
def confidentiality_filter(query_hash, current_user) def confidentiality_filter(query_hash, current_user)
return query_hash if current_user&.full_private_access? return query_hash if current_user&.can_read_all_resources?
filter = { filter = {
bool: { bool: {
......
...@@ -25,7 +25,7 @@ module Elastic ...@@ -25,7 +25,7 @@ module Elastic
def filter(query_hash, options) def filter(query_hash, options)
user = options[:current_user] user = options[:current_user]
return query_hash if user&.full_private_access? return query_hash if user&.can_read_all_resources?
filter_conditions = filter_conditions =
filter_personal_snippets(user, options) + filter_personal_snippets(user, options) +
......
...@@ -52,7 +52,7 @@ describe Gitlab::Elastic::SnippetSearchResults, :elastic, :sidekiq_might_not_nee ...@@ -52,7 +52,7 @@ describe Gitlab::Elastic::SnippetSearchResults, :elastic, :sidekiq_might_not_nee
end end
end end
context 'when user has full_private_access', :do_not_mock_admin_mode do context 'when user has read_all_resources', :do_not_mock_admin_mode do
include_context 'custom session' include_context 'custom session'
let(:user) { create(:admin) } let(:user) { create(:admin) }
......
...@@ -228,11 +228,11 @@ describe User do ...@@ -228,11 +228,11 @@ describe User do
end end
end end
describe '#full_private_access?' do describe '#can_read_all_resources?' do
it 'returns true for auditor user' do it 'returns true for auditor user' do
user = build(:user, :auditor) user = build(:user, :auditor)
expect(user.full_private_access?).to be_truthy expect(user.can_read_all_resources?).to be_truthy
end end
end end
......
...@@ -31,7 +31,7 @@ module API ...@@ -31,7 +31,7 @@ module API
find_params = params.slice(:all_available, :custom_attributes, :owned, :min_access_level) find_params = params.slice(:all_available, :custom_attributes, :owned, :min_access_level)
find_params[:parent] = find_group!(parent_id) if parent_id find_params[:parent] = find_group!(parent_id) if parent_id
find_params[:all_available] = find_params[:all_available] =
find_params.fetch(:all_available, current_user&.full_private_access?) find_params.fetch(:all_available, current_user&.can_read_all_resources?)
groups = GroupsFinder.new(current_user, find_params).execute groups = GroupsFinder.new(current_user, find_params).execute
groups = groups.search(params[:search]) if params[:search].present? groups = groups.search(params[:search]) if params[:search].present?
......
...@@ -213,9 +213,9 @@ module API ...@@ -213,9 +213,9 @@ module API
unauthorized! unless Devise.secure_compare(secret_token, input) unauthorized! unless Devise.secure_compare(secret_token, input)
end end
def authenticated_with_full_private_access! def authenticated_with_can_read_all_resources!
authenticate! authenticate!
forbidden! unless current_user.full_private_access? forbidden! unless current_user.can_read_all_resources?
end end
def authenticated_as_admin! def authenticated_as_admin!
......
...@@ -6,7 +6,7 @@ module API ...@@ -6,7 +6,7 @@ module API
prepend_if_ee('::EE::API::Helpers::ProjectSnapshotsHelpers') # rubocop: disable Cop/InjectEnterpriseEditionModule prepend_if_ee('::EE::API::Helpers::ProjectSnapshotsHelpers') # rubocop: disable Cop/InjectEnterpriseEditionModule
def authorize_read_git_snapshot! def authorize_read_git_snapshot!
authenticated_with_full_private_access! authenticated_with_can_read_all_resources!
end end
def send_git_snapshot(repository) def send_git_snapshot(repository)
......
...@@ -24,7 +24,7 @@ module API ...@@ -24,7 +24,7 @@ module API
requires :fingerprint, type: String, desc: 'Search for a SSH fingerprint' requires :fingerprint, type: String, desc: 'Search for a SSH fingerprint'
end end
get do get do
authenticated_with_full_private_access! authenticated_with_can_read_all_resources!
finder_params = params.merge(key_type: 'ssh') finder_params = params.merge(key_type: 'ssh')
......
...@@ -4,7 +4,7 @@ module API ...@@ -4,7 +4,7 @@ module API
class Pages < Grape::API class Pages < Grape::API
before do before do
require_pages_config_enabled! require_pages_config_enabled!
authenticated_with_full_private_access! authenticated_with_can_read_all_resources!
end end
params do params do
......
...@@ -37,7 +37,7 @@ module API ...@@ -37,7 +37,7 @@ module API
resource :pages do resource :pages do
before do before do
require_pages_config_enabled! require_pages_config_enabled!
authenticated_with_full_private_access! authenticated_with_can_read_all_resources!
end end
desc "Get all pages domains" do desc "Get all pages domains" do
......
...@@ -29,7 +29,7 @@ module Gitlab ...@@ -29,7 +29,7 @@ module Gitlab
def levels_for_user(user = nil) def levels_for_user(user = nil)
return [PUBLIC] unless user return [PUBLIC] unless user
if user.full_private_access? if user.can_read_all_resources?
[PRIVATE, INTERNAL, PUBLIC] [PRIVATE, INTERNAL, PUBLIC]
elsif user.external? elsif user.external?
[PUBLIC] [PUBLIC]
......
...@@ -2894,11 +2894,11 @@ describe User, :do_not_mock_admin_mode do ...@@ -2894,11 +2894,11 @@ describe User, :do_not_mock_admin_mode do
end end
end end
describe '#full_private_access?' do describe '#can_read_all_resources?' do
it 'returns false for regular user' do it 'returns false for regular user' do
user = build(:user) user = build(:user)
expect(user.full_private_access?).to be_falsy expect(user.can_read_all_resources?).to be_falsy
end end
context 'for admin user' do context 'for admin user' do
...@@ -2908,7 +2908,7 @@ describe User, :do_not_mock_admin_mode do ...@@ -2908,7 +2908,7 @@ describe User, :do_not_mock_admin_mode do
context 'when admin mode is disabled' do context 'when admin mode is disabled' do
it 'returns false' do it 'returns false' do
expect(user.full_private_access?).to be_falsy expect(user.can_read_all_resources?).to be_falsy
end end
end end
...@@ -2919,7 +2919,7 @@ describe User, :do_not_mock_admin_mode do ...@@ -2919,7 +2919,7 @@ describe User, :do_not_mock_admin_mode do
end end
it 'returns true' do it 'returns true' do
expect(user.full_private_access?).to be_truthy expect(user.can_read_all_resources?).to be_truthy
end end
end end
end end
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment