Refactor ProtectedRef logic to remove the inherited nature of `#check_access`
The following discussion from !156664 (merged) should be addressed:
-
@jwoodwardgl started a discussion: (+3 comments) After reading through these comments I have re-read the code and realised that it's currently quite confusing due to us overriding the
check_access
method and usingsuper do ... yield if block_given? end
At the moment we don't have do not have clear separation between the access level types. We could make the lines less blurry by removing all of the overrides and using the
access_level.type
to call the correct method e.g.def check_access(current_user, current_project = project) return false if current_user.nil? || no_access? return current_user.admin? if admin_access? # role_access_allowed? # user_access_allowed? # group_access_allowed? # deploy_key_access_allowed? send(:"#{type}_access_allowed?", current_user, current_project) # rubocop:disable GitlabSecurity/PublicSend -- metaprogramming end
Then we just implement the
access_allowed?
methods for each type. I think we should also renamecheck_access
as it's really a boolean method. I think it would make more sense to name itaccess_allowed?(current_user, current_project = project)
.This feels like multiple MRs so will look into splitting these changes.