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_accessmethod and usingsuper do ... yield if block_given? endAt 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.typeto 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 endThen we just implement the
access_allowed?methods for each type. I think we should also renamecheck_accessas 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.