POC - Remove the has_projects condition from GroupPolicy to assess impact
Problem
See also the description in the main epic: Inconsistency in group/project member access to... (&9424)
Members of project can currently access their ancestor groups and have limited permissions in them. Members of groups cannot do that when there is no project underneath the group. As soon as there is a project, members gain access to their ancestor groups due to inheritance into the project, which then grants them the rights to access their ancestor groups. This has led to customers creating dummy projects in groups just so that group members would also get access to their ancestor groups.
As a solution, we have decided to remove the ability to access ancestor groups in this manner from the project level, meaning that neither project nor group members should be able to gain access to their ancestor groups.
There's historic discussion around this issue in &9424
Proposal
Create a POC in which the has_projects
condition is removed from GroupPolicy. This should help us in assessing the impact of that removal to identify which dependent objects would be affected. Based on that, we can determine a strategy per object to fix access.
Availability and Testing
- Please run the manual
e2e:package-and-test
job on the POC MR to run the full E2E test suite in order to help assess impact. Feel free to reach out to@vburton
with any questions on test failures and for a QA review.
Further details
Here's some background information copied over from another similar issue.
When a user is a member of a project, they are also a member of the project's parent group. This rule causes excessive computational overhead, is inconsistenly applied, and requires an inefficient recursive calculation to implement properly. Below I provide background on how this membership rule fits in to our overall group membership system, and why I believe we should remove the rule completely.
Very broadly, group membership involves the following factors:
- Direct group membership
- Inherited group membership
- Shared group membership
- Project membership parent group (The one this issue is about)
- Direct Project membership
- Inherited project memberships
- Shared group project membership
More specifically the group membership calculation works in this way:
- Group membership:
- Direct group memberships.
from members table where source_type = 'Group'
- Groups that parent projects from
2
below. - Groups shared into 1.1 and 1.2.
- Group descendants of 1.1, 1.2, and 1.3.
- Direct group memberships.
- Project membership (lib/gitlab/project_authorizations.rb):
- Direct project members.
from members table where source_type = 'Project'
- Projects that belong to Group membership (
1
). - Projects shared in to Group membership (
1
).from project_group_links
- Direct project members.
In flowchart form this looks like:
flowchart LR
subgraph 1[1 - Group Membership]
1.1[1.1 - Direct group membership]
1.2[1.2 - Project's parent group]
1.3[1.3 - Shared groups]
1.4[1.4 - Descendants]
1.1 --> 1.3
1.2 --> 1.3
1.1 --> 1.4
1.2 --> 1.4
1.3 --> 1.4
end
subgraph 2[2 - Project membership]
2.1[2.1 - Direct project membership]
2.2[2.2 - Group's projects]
2.3[2.3 - Group shared projects]
end
2 --> 1.2
1 --> 2.2
1 --> 2.3
Note the recursive relationship between Group Membership
and Project Membership
. In practice we don't honour this relationship. Instead we calculate Project Membership
first, and then calculate Group Membership
second. Therefore 2.2
and 2.3
of the Project Membership
calculation does not include 1.2
from Group Membership
. This looks more like:
flowchart LR
subgraph 2[2 - Project membership]
2.1[2.1 - Direct project membership]
2.2[2.2 - Group's projects]
2.3[2.3 - Group shared projects]
end
subgraph 1[1 - Group Membership]
subgraph 0[0 - Core group membership]
1.1[1.1 - Direct group membership]
1.3[1.3 - Shared groups]
1.4[1.4 - Descendants]
end
1.2[1.2 - Project's parent group]
1.1 --> 1.3
1.2 --> 1.3
1.1 --> 1.4
1.2 --> 1.4
1.3 --> 1.4
end
0 --> 2.2
0 --> 2.3
2 --> 1.2
There are a number of problems with these calculations. The most notable is the relationship between Group Membership
and Project Membership
.
- The relationship is recursive, but we only step through once. This results in inconsistent product rules.
- We make many redundant calculations because the project membership and group membership calculations overlap greatly.
Removing this rule would simplify group membership greatly:
flowchart LR
subgraph 1[1 - Group Membership]
1.1[1.1 - Direct group membership]
1.3[1.3 - Shared groups]
1.4[1.4 - Descendants]
end
1.1 --> 1.3
1.1 --> 1.4
1.3 --> 1.4