Disallow anonymous users to pull new data via pull-through distributions

closes pulp#1657

CHANGES/1657.bugfix

@@ -0,0 +1,2 @@
+Disallowed anonymous users to pull new content via a pull-through caching distribution. Content that
+is already cached/downloaded can be still pulled.

pulp_container/app/registry_api.py

@@ -306,7 +306,7 @@ def get_pull_through_drv(self, path):
             .order_by("-base_path")
             .first()
         )
-        if not pull_through_cache_distribution:
+        if not pull_through_cache_distribution or not self.request.user.is_authenticated:
             raise RepositoryNotFound(name=path)
 
         try:
@@ -1028,7 +1028,11 @@ 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()
-                if distribution.remote and distribution.pull_through_distribution_id:
+                if (
+                    distribution.remote
+                    and distribution.pull_through_distribution_id
+                    and request.user.is_authenticated
+                ):
                     remote = distribution.remote.cast()
                     # issue a head request first to ensure that the content exists on the remote
                     # source; we want to prevent immediate "not found" error responses from
@@ -1066,6 +1070,7 @@ def handle_safe_method(self, request, path, pk):
         else:
             try:
                 manifest = models.Manifest.objects.get(digest=pk, pk__in=repository_version.content)
+                return redirects.issue_manifest_redirect(manifest)
             except models.Manifest.DoesNotExist:
                 repository = repository.cast()
                 # the manifest might be a part of listed manifests currently being uploaded
@@ -1074,17 +1079,22 @@ def handle_safe_method(self, request, path, pk):
                     manifest = repository.pending_manifests.get(digest=pk)
                     manifest.touch()
                 except models.Manifest.DoesNotExist:
-                    pass
+                    manifest = None
 
                 distribution = distribution.cast()
-                if distribution.remote and distribution.pull_through_distribution_id:
+                if (
+                    distribution.remote
+                    and distribution.pull_through_distribution_id
+                    and request.user.is_authenticated
+                ):
                     remote = distribution.remote.cast()
                     self.fetch_manifest(remote, pk)
                     return redirects.redirect_to_content_app("manifests", pk)
+                elif manifest:
+                    return redirects.issue_manifest_redirect(manifest)
                 else:
                     raise ManifestNotFound(reference=pk)
 
-            return redirects.issue_manifest_redirect(manifest)
 
     def get_content_units_to_add(self, manifest, tag):
         add_content_units = [str(tag.pk), str(manifest.pk)]

pulp_container/app/token_verification.py

@@ -75,7 +75,7 @@ def authenticate(self, request):
         If basic authentication could not success, remote webserver authentication is considered.
         """
         if request.headers.get("Authorization") == "Basic Og==":
-            return (AnonymousUser, None)
+            return (AnonymousUser(), None)
 
         try:
             user = super().authenticate(request)

pulp_container/tests/functional/api/test_pull_through_cache.py

@@ -2,12 +2,14 @@
 import subprocess
 import pytest
 
+from subprocess import CalledProcessError
 from uuid import uuid4
 
 from pulp_container.tests.functional.constants import (
     REGISTRY_V2,
     REGISTRY_V2_FEED_URL,
     PULP_HELLO_WORLD_REPO,
+    PULP_HELLO_WORLD_LINUX_AMD64_DIGEST,
     PULP_FIXTURE_1,
 )
 
@@ -30,12 +32,30 @@ def pull_through_distribution(
 
 
 @pytest.fixture
-def pull_and_verify(
+def add_pull_through_entities_to_cleanup(
+    container_repository_api,
+    container_remote_api,
+    container_distribution_api,
     add_to_cleanup,
+):
+    def _add_pull_through_entities_to_cleanup(path):
+        repository = container_repository_api.list(name=path).results[0]
+        add_to_cleanup(container_repository_api, repository.pulp_href)
+        remote = container_remote_api.list(name=path).results[0]
+        add_to_cleanup(container_remote_api, remote.pulp_href)
+        distribution = container_distribution_api.list(name=path).results[0]
+        add_to_cleanup(container_distribution_api, distribution.pulp_href)
+
+    return _add_pull_through_entities_to_cleanup
+
+
+@pytest.fixture
+def pull_and_verify(
+    anonymous_user,
+    add_pull_through_entities_to_cleanup,
     container_pull_through_distribution_api,
     container_distribution_api,
     container_repository_api,
-    container_remote_api,
     container_tag_api,
     registry_client,
     local_registry,
@@ -46,24 +66,25 @@ def _pull_and_verify(images, pull_through_distribution):
             remote_image_path = f"{REGISTRY_V2}/{image_path}"
             local_image_path = f"{pull_through_distribution.base_path}/{image_path}"
 
+            # 0. test if an anonymous user cannot pull new content through the pull-through cache
+            with anonymous_user, pytest.raises(CalledProcessError):
+                local_registry.pull(local_image_path)
+
             # 1. pull remote content through the pull-through distribution
             local_registry.pull(local_image_path)
             local_image = local_registry.inspect(local_image_path)
 
-            # when the client pulls the image, a repository, distribution, and remote is created in
-            # the background; therefore, scheduling the cleanup for these entities is necessary
             path, tag = local_image_path.split(":")
             tags_to_verify.append(tag)
-            repository = container_repository_api.list(name=path).results[0]
-            add_to_cleanup(container_repository_api, repository.pulp_href)
-            remote = container_remote_api.list(name=path).results[0]
-            add_to_cleanup(container_remote_api, remote.pulp_href)
-            distribution = container_distribution_api.list(name=path).results[0]
-            add_to_cleanup(container_distribution_api, distribution.pulp_href)
+
+            # when the client pulls the image, a repository, distribution, and remote is created in
+            # the background; therefore, scheduling the cleanup for these entities is necessary
+            add_pull_through_entities_to_cleanup(path)
 
             pull_through_distribution = container_pull_through_distribution_api.list(
                 name=pull_through_distribution.name
             ).results[0]
+            distribution = container_distribution_api.list(name=path).results[0]
             assert [distribution.pulp_href] == pull_through_distribution.distributions
 
             # 2. verify if the pulled content is the same as on the remote
@@ -89,6 +110,10 @@ def _pull_and_verify(images, pull_through_distribution):
             tags = container_tag_api.list(repository_version=repository.latest_version_href).results
             assert sorted(tags_to_verify) == sorted([tag.name for tag in tags])
 
+            # 6. test if the anonymous user can pull existing content via the pull-through cache
+            with anonymous_user:
+                local_registry.pull(local_image_path)
+
     return _pull_and_verify
 
 
@@ -102,6 +127,27 @@ def test_manifest_pull(delete_orphans_pre, pull_through_distribution, pull_and_v
     pull_and_verify(images, pull_through_distribution)
 
 
+def test_anonymous_pull_by_digest(
+    delete_orphans_pre,
+    add_pull_through_entities_to_cleanup,
+    anonymous_user,
+    local_registry,
+    pull_through_distribution,
+):
+    image = f"{PULP_HELLO_WORLD_REPO}@{PULP_HELLO_WORLD_LINUX_AMD64_DIGEST}"
+    local_image_path = f"{pull_through_distribution.base_path}/{image}"
+
+    with anonymous_user, pytest.raises(CalledProcessError):
+        local_registry.pull(local_image_path)
+
+    local_registry.pull(local_image_path)
+
+    add_pull_through_entities_to_cleanup(local_image_path.split("@")[0])
+
+    with anonymous_user:
+        local_registry.pull(local_image_path)
+
+
 def test_conflicting_names_and_paths(
     container_remote_api,
     container_remote_factory,

pulp_container/tests/functional/conftest.py

@@ -5,7 +5,7 @@
 import requests
 import subprocess
 
-from contextlib import contextmanager
+from contextlib import contextmanager, suppress
 from urllib.parse import urljoin, urlparse
 from uuid import uuid4
 
@@ -196,12 +196,16 @@ def pull(image_path):
                 registry_client.login(
                     "-u", bindings_cfg.username, "-p", bindings_cfg.password, registry_name
                 )
+
+                try:
+                    registry_client.pull("/".join([registry_name, image_path]))
+                finally:
+                    registry_client.logout(registry_name)
             else:
-                registry_client.logout(registry_name)
-            try:
+                with suppress(subprocess.CalledProcessError):
+                    registry_client.logout(registry_name)
+
                 registry_client.pull("/".join([registry_name, image_path]))
-            finally:
-                registry_client.logout(registry_name)
 
         @staticmethod
         def tag_and_push(image_path, local_url, *args):

pulp_container/tests/functional/constants.py

@@ -31,6 +31,9 @@
 
 # a repository having the size of 1.84kB
 PULP_HELLO_WORLD_REPO = "pulp/hello-world"
+PULP_HELLO_WORLD_LINUX_AMD64_DIGEST = (
+    "sha256:239de6dd745ed1a211123322865b4c342c706e7c1e01644a1bbefe8f8846c5ff"
+)
 
 # a repository containing 4 manifest lists and 5 manifests
 PULP_FIXTURE_1 = "pulp/test-fixture-1"