Skip to content

Update the group permission check in packages finder helper

David Fernandez requested to merge 326653-fix-package-finder-helpers into master

🏰 Context

Following !57600 (merged), the maven package finder started to use the package finder helper.

This introduces an additional check for the target group: users need to have read_package on it.

As we will see this leads to a ~bug.

Let's take the following setup:

Group -> Subgroup -> Project -> package

All the above objects are private.

Before !57600 (merged), users with at least the reporter permission on Subgroup could pull the package targeting the Group.

How is that possible? This is because:

  • the maven api endpoints needed two permissions:
    • read_group on the target group (Group). Granted by this rule. Basically, if you can access any of the included projects, you get read_group. reporters of Subgroup have access to Project = read_group for Group is granted for them.
    • read_package on the project linked to the package. This is the usual permission for pulling packages. Users need to have at least the reporter permission on the project hosting the package. reporters of Subgroup have such permission = read_package for Project is granted for them.

=> reporters of Subgroup can pull package.

Now with !57600 (merged), we get the same checks as the above but in addition, users need:

  • read_package on the target group (Group). This is granted only to reporters of the Group. reporters of Subgroup will not get such permission.

=> reporters of Subgroup can't pull package.

This is issue #326653 (closed)

🤔 What does this MR do?

  • Check for read_group on the group in the package finder helper instead of read_package.
    • This allows reporters of subgroups to see the packages in sub projects.
  • Update the related spec with the above scenario
  • The only other "client" of the package finder helper is the nuget API.
    • It's safe to use read_group as the endpoints themselves will require read_package on the target group.
    • Update the related spec with the above scenario and make sure that the request is rejected because the read_package permission is required on the target group.
    • This inconsistency in permission checks should be fixed. I opened #326710 (closed) for that.

🖼 Screenshots (strongly suggested)

n / a

📐 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 David Fernandez

Merge request reports