Inconsistency in group/project member access to parent group

added devopsmanage groupauthentication and authorization [DEPRECATED] namespace sectiondev typefeature typemaintenance + 2 deleted labels

added to epic &6689

I think the simplest approach is to limit access according to membership, and allow access to only flow down in the hierarchy. In other words, we shouldn't give access to parent Namespaces.

@mushakov, @gweaver, what do you think?

cc @acroitor, @jprovaznik

@ifarkas That would be the ideal solution but it is a breaking change to existing customers. I think we would need to wait until a major release to make this change. I also wonder what the user experience would be like since you'd have clickable items in the breadcrumbs (as an example, there are probably more) that would now be broken. We'd have to do a full sweep to see what else breaks and how we resolve it .

I think the simplest approach is to limit access according to membership, and allow access to only flow down in the hierarchy. In other words, we shouldn't give access to parent Namespaces.

Yes, it would be best option. It has some implications though, off hand:

users wouldn't be able to assign some labels, milestones, epics... (these from ancestor groups if they don't have access to it) to issues/MRs and other resources.
if someone else (who can see these labels, milestones, epics...) assigns them to an issue, some other users may not see them -> we will have to present these as something like "set but not visible to you". I think this is something we will need for sharing resourcess across multiple groups anyway (discussed in depth in #296668).
for larger organizations which use sub-groups and sub-projects and give users access only to selective projects but at the same time they use epics or labels for a whole department, I can imagine this may cause a lot of pain, as users couldn't suddenly see these resources. I wonder if we can figure out an alternative solution for such use-case?

Even with these downsides I would prefer to rather make this change but its impact is quite big so it depends if it's acceptable by PM.

I was going to mention the above implications as well. I would like it from engineering perspective, makes things much clearer, and perhaps easier to understand for the users, but I am not sure we can actually do this, given the footprint of the breaking change.

Even if we limit access of the user according to membership, we still need to give a bit of extra, right ? E.g. in a GroupA -> GroupB -> ProjectC hierarchy, user with membership only in ProjectC would still need to be able to at least read paths of the GroupB and GroupA to see the breadcrums, or do we show no breadcrumbs? If user can see the breadcrumbs, do we let the user also see the tree structure of GroupA -> GroupB -> ProjectC, for instance let user navigate to the GroupA page and show the structure of the groups and projects they are part of?

Is this issue to discuss how we fix the specific case of Inconsistency in group / project member access to parent group or permissions in general? IMHO the fix with the smallest footprint for the Inconsistency in group / project member access to parent group would be to give User:Bob member of Subgroup read permission in Group.

users wouldn't be able to assign some labels, milestones, epics... (these from ancestor groups if they don't have access to it) to issues/MRs and other resources.

@jprovaznik, there's a caveat to that: if there's no project in the group hierarchy, the user does not have access to eg. epics in the ancestors groups.

user with membership only in ProjectC would still need to be able to at least read paths of the GroupB and GroupA to see the breadcrums

@acroitor, that's correct and the behaviour is consistent even in today. Project and group members see the full path and the breadcrumbs are also displayed correctly. The difference comes from:

let user navigate to the GroupA page and show the structure of the groups and projects they are part of

This is only possible when there's a project in the hierarhcy. Otherwise members are not able to navigate to the groups page.

Is this issue to discuss how we fix the specific case of Inconsistency in group / project member access to parent group or permissions in general?

My intent was to discuss this specific exception and I plan to create a separate issue for discussion group-level resource access in general.

there's a caveat to that: if there's no project in the group hierarchy, the user does not have access to eg. epics in the ancestors groups.

@ifarkas yes, and it's very confusing - I think it's tracked here #36658 (closed)

@ifarkas @jprovaznik @acroitor From my vantage point, it's hard for me to decouple this proposal from the "group-level resource access in general" given what I discussed here -- #338819 (comment 667830288). If we solve that problem for allowing parents to "selectively disclose resources to children such that they are viewable within the child's context", then preventing a member of a child namespace (gitlab.com/parent/child) from clicking into a parent namespace (gitlab.com/parent) seems completely logical, but as @mushakov mentioned, it would, unfortunately, need to be treated as a breaking change.

it's hard for me to decouple this proposal from the "group-level resource access in general"

@gweaver, I think the level of coupling depends on the direction we take in each question. For example, considering the parent group access inconsistency, a solution could be to revoke the generic group access, but grant access to a few types of resources of the parent (like epics, milestones, labels). This way, we can avoid the implications @jprovaznik and @acroitor mentioned.

On the other hand, if we decide that all resources need to flow down, it might not make sense to decouple the generic group access permission from the read inheritable resources of the parent group permission. A group (in some sense) is the collection of its resources.

However, if this is the case, I am confused by the concept of membership inheritance. It seems users have the same access down the hierarchy as upward. But then why are "inherited" members only listed in descendants?

For example, considering the parent group access inconsistency, a solution could be to revoke the generic group access, but grant access to a few types of resources of the parent (like epics, milestones, labels).

@ifarkas this is a possible solution, and perhaps most feasible, though as you pointed out:

On the other hand, if we decide that all resources need to flow down, it might not make sense to decouple the generic group access permission from the read inheritable resources of the parent group permission. A group (in some sense) is the collection of its resources.

Yes, if we distinguish between inheritable and not-inheritable resources, what would be left in not-inheritable resources? If this set is "big enough", I think this might be a good solution.

However, if this is the case, I am confused by the concept of membership inheritance. It seems users have the same access down the hierarchy as upward. But then why are "inherited" members only listed in descendants?

only nit: It's not exactly true users have same access up/down. Users have only "guest" access upward, not matter what access they have downward - IOW they still can't see confidential issues/epics for example or manage resources in upward groups. This can be viewed as providing "necessary minimum" access. Not that I would agree this, just playing devil's advocate, I would still prefer to provide more restricted/selective access upward if necessary.

/cc @gweaver

@jprovaznik @ifarkas The bit that confuses me about access back up the hierarchy is that you can't modify an epic right? But, you can modify the contents of the epic by adding/removing issues to/from the epic, which is effectively modifying the epic?

E.g. as a child, I don't have access to my parents bank account, but I can debit and credit the account.

But, you can modify the contents of the epic by adding/removing issues to/from the epic

@alexpooley not yet, but soon - this is being discussed in #208425 (closed).

I understand this may be questionable what permissions should be required when assigning an issue to an epic. Personally I agree with consensus there that it's less confusing to require just read_epic. For assigning e.g. milestone we don't require milestone admin permission either just to be able to use it. But I understand your point.

I guess we can break down permissions in our app into several classes:

CRUD permissions on the object itself.
Permissions on object's attributes(in bulk), I don't think we have per attribute permissions granularity and not sure we want it either But we do have examples where we have different permissions between some attributes for same user.
Permissions on object's relationships. Can an object be related to another object in general and by a given user in particular. This seems similar to permissions on attributes, but I think it is different because the permissions on the relationship also needs to be accounted for. It is a bi-directional permissions relation. Taking the milestone <-> issue example above(may not be the best one for this example ), some user may have permission to relate an issue to a milestone, however milestone permission also dictates if that specific milestone can be set.

Would be great to step back and think of how we can integrate these into a cohesive permission framework/engine as right now I don't think there is an easy way to get an overview of the permissions we have.

Does it make sense to think of a more explicit way to express permissions, with explicit defaults, that can be changed vs implicit behaviours(e.g. if one has read access they can assign object to another object and this cannot be changed). At the same time the more granularity and control, the more complex and complicated it becomes for the end user and also scaling a very granular permissions system can be a big problem.

Users have only "guest" access upward

@jprovaznik, not sure about that... For example, a guest has access to all projects within that group, which is not true in this case. This is still the most confusing part for me. I am not sure we can ultimately pinpoint a membership level.

I also tried to find where this behaviour is documented. I only found inheritance described in https://docs.gitlab.com/ee/user/group/subgroups/#membership Are you aware of any documentation on providing access to ancestors?

As a side note, a comment from #338819 (comment 667131809)

we have epics flow down the hierarchy

Personally, I find this model more intuitive. This is also more in line with revoking read_group and granting a read_inheritable_resources which they can use in the scope of their actual membership.

Also, maybe because of the lack of clear definition, the code for querying authorized groups / projects inconsistently applies this rule. In Manage, we saw security issues because of that like https://gitlab.com/gitlab-org/gitlab/-/issues/238112.

E.g. as a child, I don't have access to my parents bank account, but I can debit and credit the account.

@alexpooley, I would prefer to drop this concept, but apparently this is a product requirement for many ( / all? ) group-level resources.

But we do have examples where we have different permissions between some attributes for same user.

@acroitor, That's interesting! Can you share some examples?

Can you share some examples?

I am aware of one example, with issues. A non-member can set title, description, issue type and confidential flag when issue is created but not other attributes. More so when the same user tries to edit the issue they just created they'd only be able to change title and description and not confidentiality flag and not the issue type.

@jprovaznik

But, you can modify the contents of the epic by adding/removing issues to/from the epic

@alexpooley not yet, but soon - this is being discussed in #208425 (closed).

Given this scenario:

graph TD;
  GroupA --> GroupB
  GroupB --> Project
  GroupA -.- Epic
  GroupB -.- Alex

Can Alex add/remove issues from Epic ?

Users have only "guest" access upward

not sure about that... For example, a guest has access to all projects within that group, which is not true in this case. This is still the most confusing part for me. I am not sure we can ultimately pinpoint a membership level.

@ifarkas yes, it's not that user would have regular guest membership in ancestor groups, what I meant was that in ancestor groups user doesn't have the same level of access as when going down in hierarchy (#340421 (comment 674785207)). Thus I quoted "guest", as guest permissions are similarly limited but I see that was rather misleading.

I also tried to find where this behaviour is documented. I only found inheritance described in https://docs.gitlab.com/ee/user/group/subgroups/#membership Are you aware of any documentation on providing access to ancestors?

It's quite surprising there is no clear mention about this behavior in the membership documentation. It would be cool to add more info about this - apparently this behavior is around for a long time (37c40143 adds rule { has_projects } .enable :read_group but it was only refactoring - so the behavior will be even older)

Can Alex add/remove issues from Epic ?

@alexpooley no - at the moment admin_epic permission on Epic is checked when adding/removing issues from an epic (#208425 (comment 489494307)). Note that when/if this will be changed to check only admin_issue, the answer will be yes ;).

no - at the moment admin_epic permission on Epic is checked when adding/removing issues from an epic

Here's some additional interesting behavior regarding how things currently work.

graph TB

Group_Top --> Group_1 --> Group_2 --> Project
Guest --> Project
Group_Top --> Epic_1
Group_1 --> Epic2

As Guest, I was only added as a direct member with the Guest role to Project, I can navigate to any parent Group and see a list of the non-confidential epics there. I can open any epic and view the details. I can comment on the epic. If I update Guest to Reporter, I can click edit on an issue in Project and see a list of all epics from parents in the dropdown but I get an error when attempting to actually set the epic.

E.g. as a child, I don't have access to my parents bank account, but I can debit and credit the account.

Current behavior: As a child, I can see my parent's bank account and all transactions, but I cannot debit or credit the account.

If you must be Reporter+ in both Parent and Child in order to make an association, it greatly restricts the idea of "least privileged access". On the other end of the spectrum, if fewer people are members of both, then it is more of a burden on the few that are in order to set up the relationships.

Desired behavior:

As a child, I can see my parent's bank account numbers and deposit money from my bank account into one of theirs. I can also withdraw the money I deposited later, but I can't take more money out than I put in.
As a parent, I want to selectively decide which bank accounts my children can deposit money into.
As a child, I want to hide my bank accounts from parents with the role of Guest, because they aren't family members.

@gweaver thanks! that helps a lot.

If I update Guest to Reporter, I can click edit on an issue in Project and see a list of all epics from parents in the dropdown but I get an error when attempting to actually set the epic.

If I update to Developer, can I set the epic on the issue? This is what I was getting at with my previous query.

@alexpooley I believe you only need to be a reporter to do this. In your previous query, I believe Alex can only remove issues from the epic via the issue UI, but I could be wrong?

mentioned in issue #214755 (closed)

changed the description

mentioned in merge request !53110 (closed)

mentioned in epic &6623 (closed)

@dzaporozhets, the historical context of this feature might be beneficial here. Accessing ancestor groups when membership inheritance only flows down the hierarchy is controversial. A few issues we discussed so far:

inconsistently implemented between groups and projects (as described in the description).
difference between having direct or inherited membership vs having access to ancestors is not obvious to users. They are confused why they are not authorized to do some actions when they don't have membership.
we couldn't find this mentioned in the documentation.
confusing what counts as authorized group or projects for a user. Some of our code considers only membership, while others include ancestor access too. This leads to various issues like https://gitlab.com/gitlab-org/gitlab/-/issues/238112.
hard to settle with any access level. Even the lowest GUESTS access level is too broad for the permissions given when accessing ancestors.

Do you have any thoughts on how to reconcile this feature?

@ifarkas

we stop granting it for project members.

Let's go this way. Its more secure and transparent. In the end we consolidate everything under namespace and this is existing behavior for group/subgroup.

the historical context of this feature might be beneficial here

Probably it was some "temporary" hack. Anyway its good opportunity to solve it now.

difference between having direct or inherited membership vs having access to ancestors is not obvious to users.

Agree

added 2 deleted labels and removed groupauthentication and authorization [DEPRECATED] + 1 deleted label

removed typefeature + 1 deleted label

mentioned in epic &6689

mentioned in issue #351378

An issue where this inconsistency lead to a bug with a workaround to have an empty in the group hierarchy: #351378

Another one where has_projects lead to more permissions on parents: #353730 (closed)

This inconsistency causes issues also in possible epic permission check optimization - #327901 (comment 880465390)

Another issue where it caused confusion around permissions and sharing: #343617.

mentioned in issue #341768 (closed)

mentioned in issue #353730 (closed)

mentioned in issue #327901 (closed)

mentioned in issue #267996

mentioned in issue #19834 (closed)

mentioned in issue #358241

mentioned in issue #364453 (closed)

mentioned in issue gitlab-org/manage/general-discussion#17539

changed milestone to %Backlog

mentioned in issue #255981 (closed)

added [deprecated] Accepting merge requests + 1 deleted label

mentioned in issue #367868 (closed)

added backend + 1 deleted label

mentioned in issue #371067 (closed)

mentioned in merge request !95731 (merged)

mentioned in merge request !97035 (closed)

marked this issue as blocking #358241

mentioned in issue #343617

mentioned in issue #370088 (closed)

mentioned in merge request !98694 (merged)

A large GitLab Ultimate customer reports internally that group members can no longer see the pages of applications that had been added to the group, after their upgrade from 14.9.2 to 15.0.5. Is that an unexpected side effect of the related changes here, or also part of the new, intended behaviour?

We've documented Edit group (settings) as owner-only for many years.

I'm tending towards a new issue like Allow read-only access to group settings for non-owners. WDYT, @lohrc?

Update 2022-10-07 (after this): The customer confirmed in the Rails console that their user account has max_member_access_for_group: 40 in both test instances. On %14.9 they can access /-/settings/applications, but in %15.0 they get 404. To me, this means they should never have had access, but due to another (as of yet unknown) problem before %15.0, they accidentally had.

@mksionek Could weigh in here? I'm not sure whether there were any changes that could affect this recently.

this is Auth, they own the applications page
I cannot locate any changes that could affect that :(

cc @ifarkas @adil.farrukh

I cannot locate any changes that could affect that :(

@katrinleinweber, @mksionek, that's correct. There hasn't been any changes to fix this issue.

The fix would be a breaking change, so it can be released in a major version change, but it's not scheduled yet.

@hsutor This might be one to take up for %16.0.

I've updated my above comment with more details from the customer. That prompted me to rather than continue this thread, start Allow read-only access to group settings to non... (#377379)

@lohrc It seems like the smaller (?) problem of this being scoped to what is shown on the application page would be long to ~"group::authentication and authorization" , but if this is about larger inconsistencies between access to parent groups, I think that is ~"group::workspace"

I also just saw that JiHu is taking #381413 (closed) which was noted to possibly be a duplicate of this A lot to sort out here..

Inconsistency in group/project member access to parent group

Designs

Child items ...

Activity

Inconsistency in group/project member access to parent group

Blocks

Relates to

Activity