Update the group permission check in packages finder helper
🏰 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_groupon the target group (Group). Granted by this rule. Basically, if you can access any of the included projects, you getread_group.reporters ofSubgrouphave access toProject=read_groupforGroupis granted for them. -
read_packageon the project linked to the package. This is the usual permission for pulling packages. Users need to have at least thereporterpermission on the project hosting the package.reporters ofSubgrouphave such permission =read_packageforProjectis 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_packageon the target group (Group). This is granted only to reporters of theGroup.reporters ofSubgroupwill not get such permission.
=> reporters of Subgroup can't pull package.
This is issue #326653 (closed)
🤔 What does this MR do?
- Check for
read_groupon the group in the package finder helper instead ofread_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_groupas the endpoints themselves will requireread_packageon the target group. - Update the related spec with the above scenario and make sure that the request is rejected because the
read_packagepermission is required on the target group. - This inconsistency in permission checks should be fixed. I opened #326710 (closed) for that.
- It's safe to use
🖼 Screenshots (strongly suggested)
n / a
📐 Does this MR meet the acceptance criteria?
Conformity
-
📋 Does this MR need a changelog?-
I have included a changelog entry. -
I have not included a changelog entry because _____.
-
- [-] Documentation (if required)
-
Code review guidelines -
Merge request performance guidelines -
Style guides - [-] Database guides
- [-] Separation of EE specific content
Availability and Testing
- [-] Review and add/update tests for this feature/bug. Consider all test levels. See the Test Planning Process.
- [-] Tested in all supported browsers
- [-] Informed Infrastructure department of a default or new setting change, if applicable per definition of done
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