diff --git a/CHANGES/1624.bugfix b/CHANGES/1624.bugfix new file mode 100644 index 000000000..239ceba94 --- /dev/null +++ b/CHANGES/1624.bugfix @@ -0,0 +1,2 @@ +Permitted users with the `pull_new_distributions` permission to pull data via pull-through +distributions. diff --git a/pulp_container/app/access_policy.py b/pulp_container/app/access_policy.py index aa814bffd..84ac524b3 100644 --- a/pulp_container/app/access_policy.py +++ b/pulp_container/app/access_policy.py @@ -31,6 +31,10 @@ def get_policy_statements(self, request, view): access_policy_obj = AccessPolicyModel.objects.get( viewset_name="distributions/container/container" ) + elif isinstance(view.get_object(), models.ContainerPullThroughDistribution): + access_policy_obj = AccessPolicyModel.objects.get( + viewset_name="distributions/container/pull-through" + ) else: access_policy_obj = AccessPolicyModel.objects.get( viewset_name="pulp_container/namespaces" diff --git a/pulp_container/app/authorization.py b/pulp_container/app/authorization.py index e3791dba7..5ecea6d81 100644 --- a/pulp_container/app/authorization.py +++ b/pulp_container/app/authorization.py @@ -11,12 +11,17 @@ from django.conf import settings from django.http import HttpRequest +from django.db.models import F, Value from rest_framework.request import Request from cryptography.hazmat.backends import default_backend from cryptography.hazmat.primitives import serialization -from pulp_container.app.models import ContainerDistribution, ContainerNamespace +from pulp_container.app.models import ( + ContainerDistribution, + ContainerNamespace, + ContainerPullThroughDistribution, +) from pulp_container.app.access_policy import RegistryAccessPolicy TOKEN_EXPIRATION_TIME = settings.get("TOKEN_EXPIRATION_TIME", 300) @@ -41,6 +46,15 @@ def __getattr__(self, *args, **kwargs): FakeViewWithSerializer = partial(FakeView, get_serializer=get_serializer) +def get_pull_through_distribution(path): + return ( + ContainerPullThroughDistribution.objects.annotate(path=Value(path)) + .filter(path__startswith=F("base_path")) + .order_by("-base_path") + .first() + ) + + class AuthorizationService: """ A class responsible for generating and managing a Bearer token. @@ -207,14 +221,26 @@ def has_pull_permissions(self, path): try: namespace = ContainerNamespace.objects.get(name=namespace_name) except ContainerNamespace.DoesNotExist: - # Check if user is allowed to create a new namespace + # Check if the user is allowed to create a new namespace return self.has_permission(None, "POST", "create", {"name": namespace_name}) - # Check if user is allowed to view distributions in the namespace + + if pt_distribution := get_pull_through_distribution(path): + # Check if the user is allowed to create a new distribution + return self.has_permission(pt_distribution, "GET", "pull_new_distribution", {}) + else: + # Check if the user is allowed to view distributions in the namespace + return self.has_permission( + namespace, "GET", "view_distribution", {"name": namespace_name} + ) + + if pt_distribution := get_pull_through_distribution(path): + # Check if the user is allowed to pull content via a pull-through distribution return self.has_permission( - namespace, "GET", "view_distribution", {"name": namespace_name} + pt_distribution, "GET", "pull_new_distribution", {"base_path": path} ) - - return self.has_permission(distribution, "GET", "pull", {"base_path": path}) + else: + # Check if the user has general pull permissions + return self.has_permission(distribution, "GET", "pull", {"base_path": path}) def has_push_permissions(self, path): """ diff --git a/pulp_container/app/migrations/0041_add_pull_through_pull_permissions.py b/pulp_container/app/migrations/0041_add_pull_through_pull_permissions.py new file mode 100644 index 000000000..bb972ba8e --- /dev/null +++ b/pulp_container/app/migrations/0041_add_pull_through_pull_permissions.py @@ -0,0 +1,17 @@ +# Generated by Django 4.2.11 on 2024-07-04 21:50 + +from django.db import migrations + + +class Migration(migrations.Migration): + + dependencies = [ + ('container', '0040_add_remote_repo_filter'), + ] + + operations = [ + migrations.AlterModelOptions( + name='containerpullthroughdistribution', + options={'default_related_name': '%(app_label)s_%(model_name)s', 'permissions': [('manage_roles_containerpullthroughdistribution', 'Can manage role assignments on pull-through cache distribution'), ('pull_new_containerdistribution', 'Can pull new content via the pull-through cache distribution')]}, + ), + ] diff --git a/pulp_container/app/models.py b/pulp_container/app/models.py index 76efabb15..85ada04a3 100644 --- a/pulp_container/app/models.py +++ b/pulp_container/app/models.py @@ -711,6 +711,10 @@ class Meta: "manage_roles_containerpullthroughdistribution", "Can manage role assignments on pull-through cache distribution", ), + ( + "pull_new_containerdistribution", + "Can pull new content via the pull-through cache distribution", + ), ] diff --git a/pulp_container/app/viewsets.py b/pulp_container/app/viewsets.py index e42be79f9..b9ca95e94 100644 --- a/pulp_container/app/viewsets.py +++ b/pulp_container/app/viewsets.py @@ -1453,6 +1453,23 @@ class ContainerPullThroughDistributionViewSet(DistributionViewSet, RolesMixin): "has_model_or_obj_perms:container.manage_roles_containerpullthroughdistribution" ], }, + { + "action": ["pull_new_distribution"], + "principal": "*", + "effect": "allow", + "condition_expression": [ + "not is_private", + ], + }, + { + "action": ["pull_new_distribution"], + "principal": "authenticated", + "effect": "allow", + "condition_expression": [ + "has_model_or_obj_perms:container.pull_new_containerdistribution or " + "has_model_or_obj_perms:container.namespace_add_containerdistribution", + ], + }, ], "creation_hooks": [ { @@ -1472,12 +1489,15 @@ class ContainerPullThroughDistributionViewSet(DistributionViewSet, RolesMixin): "container.delete_containerpullthroughdistribution", "container.change_containerpullthroughdistribution", "container.manage_roles_containerpullthroughdistribution", + "container.pull_new_containerdistribution", ], "container.containerpullthroughdistribution_collaborator": [ "container.view_containerpullthroughdistribution", + "container.pull_new_containerdistribution", ], "container.containerpullthroughdistribution_consumer": [ "container.view_containerpullthroughdistribution", + "container.pull_new_containerdistribution", ], } diff --git a/pulp_container/tests/functional/api/test_pull_through_cache.py b/pulp_container/tests/functional/api/test_pull_through_cache.py index 5dae32bfe..9c86be36c 100644 --- a/pulp_container/tests/functional/api/test_pull_through_cache.py +++ b/pulp_container/tests/functional/api/test_pull_through_cache.py @@ -143,21 +143,37 @@ def test_pull_from_private_distribution( if settings.TOKEN_AUTH_DISABLED: pytest.skip("RBAC cannot be tested when token authentication is disabled") - image = f"{PULP_HELLO_WORLD_REPO}:latest" - local_image_path = f"{pull_through_distribution(private=True).base_path}/{image}" + pull_through_distribution = pull_through_distribution(private=True) + + image1 = f"{PULP_HELLO_WORLD_REPO}:latest" + image2 = f"{PULP_FIXTURE_1}:manifest_a" + local_image_path1 = f"{pull_through_distribution.base_path}/{image1}" + local_image_path2 = f"{pull_through_distribution.base_path}/{image2}" with anonymous_user, pytest.raises(CalledProcessError): - local_registry.pull(local_image_path) + local_registry.pull(local_image_path1) - local_registry.pull(local_image_path) + local_registry.pull(local_image_path1) - add_pull_through_entities_to_cleanup(local_image_path.split(":")[0]) + add_pull_through_entities_to_cleanup(local_image_path1.split(":")[0]) with anonymous_user, pytest.raises(CalledProcessError): - local_registry.pull(local_image_path) + local_registry.pull(local_image_path1) with gen_user(model_roles=["container.containernamespace_collaborator"]): - local_registry.pull(local_image_path) + local_registry.pull(local_image_path1) + + with gen_user(model_roles=["container.containerpullthroughdistribution_consumer"]): + local_registry.pull(local_image_path1) + local_registry.pull(local_image_path2) + + add_pull_through_entities_to_cleanup(local_image_path2.split(":")[0]) + + with ( + gen_user(model_roles=["container.containerdistribution_collaborator"]), + pytest.raises(CalledProcessError), + ): + local_registry.pull(local_image_path2) def test_conflicting_names_and_paths(