Skip to content

Public/internal groups non-direct members shouldn't be allowed to read virtual registry

Context

In Maven Virtual Registry: Permissions policy (!157793 - merged), we added a new group-level policy for the virtual registry feature. This policy is the gateway that any request will go through to be authorized.

In the policy, the read_virtual_registry permission is granted to any authenticated user if they have the read_group permission. However, relying on the read_group permission might be too permissive: an authenticated user will have read_virtual_registry on a public group simply because it's public.

What we want is:

  • Either the user is a direct member of the group (in this case, the user has at least the guest access level) or
  • the user is a direct member in one of the included projects.

What does this MR do and why?

  • Update VirtualRegistries::Packages::Policies::GroupPolicy so we don't blindly rely on can?(:read_group) to grant the read_virtual_registry permission. Instead, we grant it if:

    • user is a direct member of the group (in this case, the user has at least the guest access level) or
    • user is an admin of the group or
    • user is an owner of the group's organization or
    • user is a member of a project in the group.
  • Update the related specs.

MR acceptance checklist

Please evaluate this MR against the MR acceptance checklist. It helps you analyze changes to reduce risks in quality, performance, reliability, security, and maintainability.

Screenshots or screen recordings

Screenshots are required for UI changes, and strongly recommended for all other merge requests.

Before After

How to set up and validate locally

Numbered steps to set up and validate the change are strongly suggested.

  1. In rails console:
# create a public group
public_group = FactoryBot.create(:group, :public)

# create a non-memeber user
non_group_member = FactoryBot.create(:user)

# check if the user is allowed to :read_virtual_registry in the group
Ability.allowed?(non_group_member, :read_virtual_registry, VirtualRegistries::Packages::Policies::Group.new(public_group))
=> false

# create a project in the group
project = FactoryBot.create(:project, namespace: public_group)

# add the user as project's member
project.add_guest(non_group_member)

# check if the user is allowed to :read_virtual_registry in the group
Ability.allowed?(non_group_member, :read_virtual_registry, VirtualRegistries::Packages::Policies::Group.new(private_group))
=> true

Related to #475007 (closed)

Edited by Moaz Khalifa

Merge request reports

Loading