From 24a9188fd979699694b42c1c065a5fd7bded48ff Mon Sep 17 00:00:00 2001 From: Lubos Mjachky Date: Wed, 3 Jul 2024 20:44:58 +0200 Subject: [PATCH] Disallow anonymous users to pull from private pull-through distributions closes #1623 --- CHANGES/1623.bugfix | 1 + pulp_container/app/registry_api.py | 3 +- .../functional/api/test_pull_through_cache.py | 40 +++++++++++++++++-- pulp_container/tests/functional/conftest.py | 14 ++++--- 4 files changed, 47 insertions(+), 11 deletions(-) create mode 100644 CHANGES/1623.bugfix diff --git a/CHANGES/1623.bugfix b/CHANGES/1623.bugfix new file mode 100644 index 000000000..a27049eb7 --- /dev/null +++ b/CHANGES/1623.bugfix @@ -0,0 +1 @@ +Disallowed anonymous users to pull images from private pull-through distributions. diff --git a/pulp_container/app/registry_api.py b/pulp_container/app/registry_api.py index b50777522..e1d21e604 100644 --- a/pulp_container/app/registry_api.py +++ b/pulp_container/app/registry_api.py @@ -339,13 +339,12 @@ def get_pull_through_drv(self, path): **remote_data, ) - # TODO: Propagate the user's permissions and private flag from the pull-through - # distribution to this distribution distribution, _ = models.ContainerDistribution.objects.get_or_create( name=path, base_path=path, remote=remote, repository=repository, + private=pull_through_cache_distribution.private, ) except IntegrityError: # some entities needed to be created, but their keys already exist in the database 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 6de18859a..0cb0379a2 100644 --- a/pulp_container/tests/functional/api/test_pull_through_cache.py +++ b/pulp_container/tests/functional/api/test_pull_through_cache.py @@ -2,6 +2,8 @@ import subprocess import pytest +from django.conf import settings + from subprocess import CalledProcessError from uuid import uuid4 @@ -44,7 +46,6 @@ def pull_and_verify( ): def _pull_and_verify(images, pull_through_distribution): tags_to_verify = [] - pull_through_distribution = pull_through_distribution() for version, image_path in enumerate(images, start=1): remote_image_path = f"{REGISTRY_V2}/{image_path}" local_image_path = f"{pull_through_distribution.base_path}/{image_path}" @@ -102,12 +103,12 @@ def _pull_and_verify(images, pull_through_distribution): def test_manifest_list_pull(delete_orphans_pre, pull_through_distribution, pull_and_verify): images = [f"{PULP_HELLO_WORLD_REPO}:latest", f"{PULP_HELLO_WORLD_REPO}:linux"] - pull_and_verify(images, pull_through_distribution) + pull_and_verify(images, pull_through_distribution()) def test_manifest_pull(delete_orphans_pre, pull_through_distribution, pull_and_verify): images = [f"{PULP_FIXTURE_1}:manifest_a", f"{PULP_FIXTURE_1}:manifest_b"] - pull_and_verify(images, pull_through_distribution) + pull_and_verify(images, pull_through_distribution()) def test_anonymous_pull_by_digest( @@ -117,8 +118,11 @@ def test_anonymous_pull_by_digest( local_registry, pull_through_distribution, ): + if settings.TOKEN_AUTH_DISABLED: + pytest.skip("RBAC cannot be tested when token authentication is disabled") + image = f"{PULP_HELLO_WORLD_REPO}@{PULP_HELLO_WORLD_LINUX_AMD64_DIGEST}" - local_image_path = f"{pull_through_distribution.base_path}/{image}" + local_image_path = f"{pull_through_distribution().base_path}/{image}" with anonymous_user, pytest.raises(CalledProcessError): local_registry.pull(local_image_path) @@ -131,6 +135,34 @@ def test_anonymous_pull_by_digest( local_registry.pull(local_image_path) +def test_pull_from_private_distribution( + delete_orphans_pre, + add_pull_through_entities_to_cleanup, + anonymous_user, + local_registry, + pull_through_distribution, + gen_user, +): + 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}" + + 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, pytest.raises(CalledProcessError): + local_registry.pull(local_image_path) + + with gen_user(model_roles=["container.containernamespace_collaborator"]): + local_registry.pull(local_image_path) + + def test_conflicting_names_and_paths( container_remote_api, container_remote_factory, diff --git a/pulp_container/tests/functional/conftest.py b/pulp_container/tests/functional/conftest.py index cdc55085e..4152eedeb 100644 --- a/pulp_container/tests/functional/conftest.py +++ b/pulp_container/tests/functional/conftest.py @@ -440,7 +440,7 @@ def pull_through_distribution( container_pull_through_remote_api, container_pull_through_distribution_api, ): - def _pull_through_distribution(includes=None, excludes=None): + def _pull_through_distribution(includes=None, excludes=None, private=False): remote = gen_object_with_cleanup( container_pull_through_remote_api, { @@ -450,10 +450,14 @@ def _pull_through_distribution(includes=None, excludes=None): "excludes": excludes, }, ) - distribution = gen_object_with_cleanup( - container_pull_through_distribution_api, - {"name": str(uuid4()), "base_path": str(uuid4()), "remote": remote.pulp_href}, - ) + + data = { + "name": str(uuid4()), + "base_path": str(uuid4()), + "remote": remote.pulp_href, + "private": private, + } + distribution = gen_object_with_cleanup(container_pull_through_distribution_api, data) return distribution return _pull_through_distribution