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_group
on the target group (Group
). Granted by this rule. Basically, if you can access any of the included projects, you getread_group
.reporter
s ofSubgroup
have access toProject
=read_group
forGroup
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 thereporter
permission on the project hosting the package.reporter
s ofSubgroup
have such permission =read_package
forProject
is granted for them.
-
=> reporter
s 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 theGroup
.reporter
s ofSubgroup
will not get such permission.
=> reporter
s 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 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_group
as the endpoints themselves will requireread_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.
- 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