Enforce permission checks while pulling from pull-through distributions

closes pulp#1624

CHANGES/1624.bugfix

@@ -0,0 +1,2 @@
+Permitted users with the `pull_new_distribution` permission to pull data via pull-through
+distributions.

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"

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)
@@ -62,14 +67,15 @@ def __init__(self, user, service, scopes):
         self.user = user
         self.service = service
         self.scopes = scopes
-        self.access_policy = RegistryAccessPolicy()
+
+        permission_checker = PermissionChecker(user)
 
         self.actions_permissions = defaultdict(
             lambda: lambda *args: False,
             {
-                "pull": self.has_pull_permissions,
-                "push": self.has_push_permissions,
-                "*": self.has_view_catalog_permissions,
+                "pull": permission_checker.has_pull_permissions,
+                "push": permission_checker.has_push_permissions,
+                "*": permission_checker.has_view_catalog_permissions,
             },
         )
 
@@ -184,6 +190,39 @@ def permit_scope(self, scope):
 
         return [{"type": typ, "name": name, "actions": list(permitted_actions)}]
 
+    @staticmethod
+    def generate_claim_set(issuer, issued_at, subject, audience, access):
+        """
+        Generate the claim set that will be signed and dispatched back to the requesting subject.
+        """
+        token_id = str(uuid.UUID(int=random.getrandbits(128), version=4))
+        expiration = issued_at + TOKEN_EXPIRATION_TIME
+        return {
+            "access": access,
+            "aud": audience,
+            "exp": expiration,
+            "iat": issued_at,
+            "iss": issuer,
+            "jti": token_id,
+            "nbf": issued_at,
+            "sub": subject,
+        }
+
+
+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 PermissionChecker:
+    def __init__(self, user):
+        self.user = user
+        self.access_policy = RegistryAccessPolicy()
+
     def has_permission(self, obj, method, action, data):
         """Check if user has permission to perform action."""
 
@@ -207,13 +246,24 @@ 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
-            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 create a new distribution
+                return self.has_modify_pull_through_cache_permissions(pt_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 new content via a pull-through distribution
+            if self.has_modify_pull_through_cache_permissions(pt_distribution, distribution):
+                return True
+
+        # Check if the user has general pull permissions
         return self.has_permission(distribution, "GET", "pull", {"base_path": path})
 
     def has_push_permissions(self, path):
@@ -241,28 +291,14 @@ def has_view_catalog_permissions(self, path):
         if path != "catalog":
             return False
 
-        # Fake the request
-        request = Request(HttpRequest())
-        request.method = "GET"
-        request.user = self.user
-        # Fake the view
-        view = FakeViewWithSerializer("catalog", lambda: ContainerDistribution())
-        return self.access_policy.has_permission(request, view)
+        return self.has_permission(ContainerDistribution(), "GET", "catalog", {})
 
-    @staticmethod
-    def generate_claim_set(issuer, issued_at, subject, audience, access):
-        """
-        Generate the claim set that will be signed and dispatched back to the requesting subject.
-        """
-        token_id = str(uuid.UUID(int=random.getrandbits(128), version=4))
-        expiration = issued_at + TOKEN_EXPIRATION_TIME
-        return {
-            "access": access,
-            "aud": audience,
-            "exp": expiration,
-            "iat": issued_at,
-            "iss": issuer,
-            "jti": token_id,
-            "nbf": issued_at,
-            "sub": subject,
-        }
+    def has_modify_pull_through_cache_permissions(self, pt_distribution, distribution=None):
+        has_distro_perms = False
+        if distribution:
+            has_distro_perms = self.has_permission(distribution, "GET", "pull_new_content", {})
+        return (
+            has_distro_perms
+            or self.has_permission(pt_distribution, "GET", "pull_new_distribution", {})
+            or self.has_permission(pt_distribution.namespace, "POST", "create_distribution", {})
+        )

pulp_container/app/global_access_conditions.py

@@ -27,10 +27,9 @@ def has_namespace_obj_perms(request, view, action, permission):
         for dist in obj.distributions.all():
             if request.user.has_perm(permission, dist.cast().namespace):
                 return True
-    elif type(obj) is models.ContainerPushRepositoryVersion:
-        for dist in obj.repository.distributions.all():
-            if request.user.has_perm(permission, dist.cast().namespace):
-                return True
+    elif type(obj) is models.ContainerPullThroughDistribution:
+        namespace = obj.namespace
+        return request.user.has_perm(permission, namespace)
     return False
 
 

pulp_container/app/migrations/0041_add_pull_through_pull_permissions.py

@@ -0,0 +1,21 @@
+# Generated by Django 4.2.11 on 2024-07-07 21:38
+
+from django.db import migrations
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('container', '0040_add_remote_repo_filter'),
+    ]
+
+    operations = [
+        migrations.AlterModelOptions(
+            name='containernamespace',
+            options={'permissions': [('namespace_add_containerdistribution', 'Add any distribution to a namespace'), ('namespace_delete_containerdistribution', 'Delete any distribution from a namespace'), ('namespace_view_containerdistribution', 'View any distribution in a namespace'), ('namespace_pull_containerdistribution', 'Pull from any distribution in a namespace'), ('namespace_push_containerdistribution', 'Push to any distribution in a namespace'), ('namespace_change_containerdistribution', 'Change any distribution in a namespace'), ('namespace_view_containerpushrepository', 'View any push repository in a namespace'), ('namespace_modify_content_containerpushrepository', 'Modify content in any push repository in a namespace'), ('namespace_modify_content_containerrepository', 'Modify content in any repository in a namespace'), ('namespace_change_containerpushrepository', 'Update any existing push repository in a namespace'), ('manage_roles_containernamespace', 'Can manage role assignments on container namespace')]},
+        ),
+        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')]},
+        ),
+    ]

pulp_container/app/models.py

@@ -332,6 +332,10 @@ class Meta:
                 "namespace_modify_content_containerpushrepository",
                 "Modify content in any push repository in a namespace",
             ),
+            (
+                "namespace_modify_content_containerrepository",
+                "Modify content in any repository in a namespace",
+            ),
             (
                 "namespace_change_containerpushrepository",
                 "Update any existing push repository in a namespace",
@@ -711,6 +715,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",
+            ),
         ]
 
 

pulp_container/app/registry_api.py

@@ -51,7 +51,7 @@
 from rest_framework.status import HTTP_401_UNAUTHORIZED
 
 from pulp_container.app import models, serializers
-from pulp_container.app.authorization import AuthorizationService
+from pulp_container.app.authorization import AuthorizationService, PermissionChecker
 from pulp_container.app.cache import (
     find_base_path_cached,
     FlatpakIndexStaticCache,
@@ -291,7 +291,9 @@ def get_drv_pull(self, path):
         Get distribution, repository and repository_version for pull access.
         """
         try:
-            distribution = models.ContainerDistribution.objects.get(base_path=path)
+            distribution = models.ContainerDistribution.objects.prefetch_related(
+                "pull_through_distribution"
+            ).get(base_path=path)
         except models.ContainerDistribution.DoesNotExist:
             # get a pull-through cache distribution whose base_path is a substring of the path
             return self.get_pull_through_drv(path)
@@ -345,6 +347,7 @@ def get_pull_through_drv(self, path):
                     remote=remote,
                     repository=repository,
                     private=pull_through_cache_distribution.private,
+                    namespace=pull_through_cache_distribution.namespace,
                 )
         except IntegrityError:
             # some entities needed to be created, but their keys already exist in the database
@@ -1036,10 +1039,13 @@ def handle_safe_method(self, request, path, pk):
                 tag = models.Tag.objects.get(name=pk, pk__in=repository_version.content)
             except models.Tag.DoesNotExist:
                 distribution = distribution.cast()
+                permission_checker = PermissionChecker(request.user)
                 if (
                     distribution.remote
-                    and distribution.pull_through_distribution_id
-                    and request.user.is_authenticated
+                    and distribution.pull_through_distribution
+                    and permission_checker.has_modify_pull_through_cache_permissions(
+                        distribution.pull_through_distribution, distribution
+                    )
                 ):
                     remote = distribution.remote.cast()
                     # issue a head request first to ensure that the content exists on the remote
@@ -1090,10 +1096,13 @@ def handle_safe_method(self, request, path, pk):
                     manifest = None
 
                 distribution = distribution.cast()
+                permission_checker = PermissionChecker(request.user)
                 if (
                     distribution.remote
-                    and distribution.pull_through_distribution_id
-                    and request.user.is_authenticated
+                    and distribution.pull_through_distribution
+                    and permission_checker.has_modify_pull_through_cache_permissions(
+                        distribution.pull_through_distribution, distribution
+                    )
                 ):
                     remote = distribution.remote.cast()
                     self.fetch_manifest(remote, pk)
@@ -1109,7 +1118,7 @@ def get_content_units_to_add(self, manifest, tag):
             models.MEDIA_TYPE.MANIFEST_LIST,
             models.MEDIA_TYPE.INDEX_OCI,
         ):
-            for listed_manifest in manifest.listed_manifests:
+            for listed_manifest in manifest.listed_manifests.all():
                 add_content_units.append(listed_manifest.pk)
                 add_content_units.append(listed_manifest.config_blob_id)
                 add_content_units.extend(listed_manifest.blobs.values_list("pk", flat=True))
@@ -1149,6 +1158,11 @@ def fetch_manifest(self, remote, pk):
             digest = response.headers.get("docker-content-digest")
             return models.Manifest.objects.filter(digest=digest).first()
 
+    @staticmethod
+    def has_modify_permissions(user, distribution):
+        permission_checker = PermissionChecker(user)
+        return permission_checker.has_permission(distribution, "GET", "pull_new_content", {})
+
     def put(self, request, path, pk=None):
         """
         Responds with the actual manifest

pulp_container/app/viewsets.py

@@ -1298,6 +1298,14 @@ class ContainerDistributionViewSet(DistributionViewSet, RolesMixin):
                     "has_model_or_obj_perms:container.manage_roles_containerdistribution"
                 ],
             },
+            {
+                "action": ["pull_new_content"],
+                "principal": "authenticated",
+                "effect": "allow",
+                "condition": [
+                    "has_namespace_obj_perms:container.namespace_modify_content_containerrepository"
+                ],
+            },
         ],
         "creation_hooks": [
             {
@@ -1453,6 +1461,14 @@ class ContainerPullThroughDistributionViewSet(DistributionViewSet, RolesMixin):
                     "has_model_or_obj_perms:container.manage_roles_containerpullthroughdistribution"
                 ],
             },
+            {
+                "action": ["pull_new_distribution"],
+                "principal": "authenticated",
+                "effect": "allow",
+                "condition_expression": [
+                    "has_model_or_obj_perms:container.pull_new_containerdistribution"
+                ],
+            },
         ],
         "creation_hooks": [
             {
@@ -1472,12 +1488,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",
         ],
     }
 
@@ -1544,7 +1563,9 @@ class ContainerNamespaceViewSet(
                 "action": ["view_distribution"],
                 "principal": "authenticated",
                 "effect": "allow",
-                "condition": "has_model_or_obj_perms:container.namespace_view_containerdistribution",  # noqa: E501
+                "condition": [
+                    "has_model_or_obj_perms:container.namespace_view_containerdistribution"
+                ],
             },
             {
                 "action": ["list_roles", "add_role", "remove_role"],
@@ -1579,6 +1600,7 @@ class ContainerNamespaceViewSet(
             "container.namespace_change_containerdistribution",
             "container.namespace_view_containerpushrepository",
             "container.namespace_modify_content_containerpushrepository",
+            "container.namespace_modify_content_containerrepository",
             "container.namespace_change_containerpushrepository",
             "container.manage_roles_containernamespace",
         ],
@@ -1592,6 +1614,7 @@ class ContainerNamespaceViewSet(
             "container.namespace_change_containerdistribution",
             "container.namespace_view_containerpushrepository",
             "container.namespace_modify_content_containerpushrepository",
+            "container.namespace_modify_content_containerrepository",
             "container.namespace_change_containerpushrepository",
         ],
         "container.containernamespace_consumer": [