Support private groups in merge request approval rules
What does this MR do?
A user who has permissions to override merge request approvers may not have the same permissions to view the groups as the person who added the group as an approver. To prevent the leakage we don't show the name of the groups (e.g. a hidden_group
). But we need to preserve the ability to override the approval rules.
Adds an option called remove_hidden_groups
, which when true, means hidden private groups that the user doesn't have permission to view but are already assigned as an approval group will be removed from the approval rule. If it is not provided, existing hidden groups would still be appended in the params, resulting in those groups unchanged.
What are the relevant issue numbers?
Closes #10356 (closed)
How to test?
- Pick a user who will not be invited to the private group. Let's call her
sam
. - Log in as
root
. - Create a private group. Invite yourself and some other users, but do not invite
sam
. - In a project, create an approval rule with the hidden group as one of it's members. To create an approval rule, go to Settings -> General -> Merge request approvals and click Add approvers.
- Invite
sam
as a maintainer of the project. - Log in as
sam
. - When managing merge request approvals,
sam
should not be able to see the hidden group information (although she can see the approval rule). -
sam
should be able to create an MR and the approval rule members persist. - If
root
adds an approval rule to the MR,sam
should be able to see that the rule has hidden groups and she can remove the groups (but she cannot see the name or number of hidden groups).
Screenshots
Title | Screenshot | |
---|---|---|
Creating MR with hidden groups works | ||
Editing project approval rule with hidden groups | ||
Editing MR approval rule with hidden groups | ||
Known bug: Avatars missing when editing MR approval rule with hidden groups. Here's the followup issue #10696 (closed) |
Does this MR meet the acceptance criteria?
-
Changelog entry added, if necessary -
Documentation created/updated via this MR -
Documentation reviewed by technical writer or follow-up review issue created -
Tests added for this feature/bug -
Tested in all supported browsers -
Conforms to the code review guidelines -
Conforms to the merge request performance guidelines -
Conforms to the style guides -
Conforms to the database guides -
EE specific content should be in the top level /ee
folder -
For a paid feature, have we considered GitLab.com plans, how it works for groups, and is there a design for promoting it to users who aren't on the correct plan?
Edited by 🤖 GitLab Bot 🤖