Resolve "Unify permission for member access request approval"
What does this MR do and why?
This MR is the first step towards implementing the change in #374599.
Currently, the permissions used for approving, declining and withdrawing access requests are very generic and used in multiple different places apart from being used for the member access requests area. This makes it really hard to modify for the use case in #374599.
In order for us to be able to make changes in this area for #374599, it is a prerequisite that we isolate these permissions and use them only related to the access requests area so as to keep the affected radius as small as possible. And hence this MR introduces new permissions related to approving, declining and withdrawing access requests.
backend:
On theBefore the change:
-
For access request approvals: We have the
update_group_member
orupdate_project_member
permission being checked when a project maintainer or above/group owner is about to approve the member access request of another user in a group/project. -
For access request removals:
- Removal of access requests are of 2 broad types:
- A project maintainer or group owner can destroy access requests coming into the group/projects that they admin. This is based on the
destroy_group_member
/destroy_project_member
policy. - Once a normal user has requested access to a project/group, they can "withdraw" this access request themselves. This is also based on the
destroy_group_member
/destroy_project_member
policy.
- A project maintainer or group owner can destroy access requests coming into the group/projects that they admin. This is based on the
- Removal of access requests are of 2 broad types:
After the change:
-
For access request approvals: We have the
admin_member_access_requests
permission being checked when a project maintainer or above/group owner is about to approve the member access request of another user in a group/project. However, this new policy does not need to work at theMember
level as before, it could work at themember.source
level. This is because the characteristics of aMember
record does not dictate whether an owner can approve or reject it. The owner just needs to have enough privilege at themember.source
level to be able to perform this action, so this new policy is a better fit at theGroupPolicy
/ProjectPolicy
level thanGroupMemberPolicy
/ProjectMemberPolicy
level. -
For access request removals:
- Removal of access requests are of 2 broad types:
- A project maintainer or group owner can destroy access requests coming into the group/projects that they admin. This is based on the same
admin_member_access_requests
policy as described above. There isn't a need to split up the permission here toupdate
vsdestroy
.admin_<entity>
is enough to encapsulate both scenarios. - Once a normal user has requested access to a project/group, they can "withdraw" this access request themselves. This is now based on the
withdraw_member_access_request
policy, and this policy only works for access requests of self, so it is well isolated from the other permissions.
- A project maintainer or group owner can destroy access requests coming into the group/projects that they admin. This is based on the same
- Removal of access requests are of 2 broad types:
frontend
On theScreenshots or screen recordings
Screenshots are required for UI changes, and strongly recommended for all other merge requests.
How to set up and validate locally
- One on tab, login as GitLab admin or a group/project owner of a public project/group.
- In another tab, login as a normal user and navigate to the page of the public project/group selected above.
- Normal user should be able to Request access and Withdraw access.
- On the other tab, once the normal user has requested access, the admin should be able to the see this request in the
Access requests
tab of theMembers
page, and should be able to accept and reject this access request.
MR acceptance checklist
This checklist encourages us to confirm any changes have been analyzed to reduce risks in quality, performance, reliability, security, and maintainability.
-
I have evaluated the MR acceptance checklist for this MR.