Skip to content

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.

On the backend:

Before the change:

  • For access request approvals: We have the update_group_member or update_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.

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 the Member level as before, it could work at the member.source level. This is because the characteristics of a Member record does not dictate whether an owner can approve or reject it. The owner just needs to have enough privilege at the member.source level to be able to perform this action, so this new policy is a better fit at the GroupPolicy/ProjectPolicy level than GroupMemberPolicy/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 to update vs destroy. 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.

On the frontend

  • Screenshot_2022-10-25_at_11.06.33_AM
  • Screenshot_2022-10-25_at_11.14.19_AM
  • Screenshot_2022-10-25_at_11.14.31_AM

Screenshots 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 the Members 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.

Edited by Manoj M J [On PTO]

Merge request reports