Skip to content

Allow auditor user to access project private features

Drew Blessing requested to merge dblessing-auditor-project-access into master

What does this MR do?

Related to #14731 (closed)

Previously the auditor user was unable to access a project repository for public/internal projects with the repository visibility set to 'only project members'. This change allows the project policy to respect the :read_all_resources ability for admins and auditors to allow all private features to be accessible.

Policy debug output

[4] pry(main)> policy = ProjectPolicy.new(user, project)
=> #<ProjectPolicy (@auditor : Project/22)>
[5] pry(main)> policy.debug(:download_code)
- [0] prevent when all?(anonymous, ~public_project) ((@auditor : Project/22))
  ProjectFeature Load (1.7ms)  SELECT "project_features".* FROM "project_features" WHERE "project_features"."project_id" = 22 LIMIT 1
- [28] prevent when repository_disabled ((@auditor : Project/22))
  ApplicationSetting Load (3.3ms)  SELECT "application_settings".* FROM "application_settings" ORDER BY "application_settings"."id" DESC LIMIT 1
+ [72] enable when can?(:public_access) ((@auditor : Project/22))
=> #<DeclarativePolicy::Runner::State:0x00007ff1636897a8 @enabled=true, @prevented=false>

Steps to reproduce/test

From the original issue #14731 (closed):

  1. Create public or internal project.
  2. Change the View and edit files in this project setting to Only Project Members.
  3. Impersonate auditor user and navigate to that project.
  4. Issue is that you cannot see the code in there.

Screenshots

Does this MR meet the acceptance criteria?

Conformity

Availability and Testing

Security

If this MR contains changes to processing or storing of credentials or tokens, authorization and authentication methods and other items described in the security review guidelines:

  • [-] Label as security and @ mention @gitlab-com/gl-security/appsec
  • [-] The MR includes necessary changes to maintain consistency between UI, API, email, or other methods
  • [-] Security reports checked/validated by a reviewer from the AppSec team
Edited by Peter Leitzen

Merge request reports

Loading