From 1df98c7566cd8127caaadbee9816435b615705a2 Mon Sep 17 00:00:00 2001 From: Gerrod Ubben Date: Thu, 24 Oct 2024 16:02:31 -0400 Subject: [PATCH] Ensure users can not add cross-domain content through modify endpoint fixes: #5934 (cherry picked from commit b898e2e275da0e5bf5ef5607dc23411a6308a82c) --- CHANGES/5934.bugfix | 1 + .../tests/functional/api/test_domains.py | 76 ++++++++++++++++++- pulpcore/app/models/repository.py | 2 + pulpcore/app/serializers/base.py | 2 +- pulpcore/app/util.py | 21 ++++- 5 files changed, 97 insertions(+), 5 deletions(-) create mode 100644 CHANGES/5934.bugfix diff --git a/CHANGES/5934.bugfix b/CHANGES/5934.bugfix new file mode 100644 index 0000000000..3c71e491de --- /dev/null +++ b/CHANGES/5934.bugfix @@ -0,0 +1 @@ +Fixed repository modify allowing content from separate domains. diff --git a/pulp_file/tests/functional/api/test_domains.py b/pulp_file/tests/functional/api/test_domains.py index cee0347d2f..d068339648 100644 --- a/pulp_file/tests/functional/api/test_domains.py +++ b/pulp_file/tests/functional/api/test_domains.py @@ -51,7 +51,7 @@ def test_object_creation( assert e.value.status == 400 # What key should this error be under? non-field-errors seems wrong assert json.loads(e.value.body) == { - "non_field_errors": [f"Objects must all be apart of the {domain_name} domain."] + "non_field_errors": [f"Objects must all be a part of the {domain_name} domain."] } with pytest.raises(ApiException) as e: @@ -59,7 +59,7 @@ def test_object_creation( file_bindings.RepositoriesFileApi.sync(repo.pulp_href, sync_body) assert e.value.status == 400 assert json.loads(e.value.body) == { - "non_field_errors": [f"Objects must all be apart of the {domain_name} domain."] + "non_field_errors": [f"Objects must all be a part of the {domain_name} domain."] } @@ -289,3 +289,75 @@ def test_domain_rbac(pulpcore_bindings, file_bindings, gen_user, gen_object_with {"name": str(uuid.uuid4())}, pulp_domain=domain.name ) assert e.value.status == 403 + + +@pytest.mark.parallel +def test_no_cross_pollination( + pulpcore_bindings, + file_bindings, + basic_manifest_path, + file_repository_factory, + file_remote_factory, + file_publication_factory, + file_distribution_factory, + gen_object_with_cleanup, + monitor_task, +): + """Tests that you can't use objects across domains.""" + d_remote = file_remote_factory(manifest_path=basic_manifest_path, policy="immediate") + d_repo = file_repository_factory(remote=d_remote.pulp_href) + monitor_task(file_bindings.RepositoriesFileApi.sync(d_repo.pulp_href, {}).task) + d_repo = file_bindings.RepositoriesFileApi.read(d_repo.pulp_href) + assert d_repo.latest_version_href[-2] == "1" + + body = { + "name": str(uuid.uuid4()), + "storage_class": "pulpcore.app.models.storage.FileSystem", + "storage_settings": {"MEDIA_ROOT": "/var/lib/pulp/media/"}, + } + domain = gen_object_with_cleanup(pulpcore_bindings.DomainsApi, body) + # Fail repository create + with pytest.raises(file_bindings.ApiException) as e: + file_repository_factory(remote=d_remote.pulp_href, pulp_domain=domain.name) + assert e.value.status == 400 + assert json.loads(e.value.body) == { + "non_field_errors": [f"Objects must all be a part of the {domain.name} domain."] + } + + # Fail repository sync + repo = file_repository_factory(pulp_domain=domain.name) + with pytest.raises(file_bindings.ApiException) as e: + file_bindings.RepositoriesFileApi.sync(repo.pulp_href, {"remote": d_remote.pulp_href}) + assert e.value.status == 400 + assert json.loads(e.value.body) == { + "non_field_errors": [f"Objects must all be a part of the {domain.name} domain."] + } + + # Fail publication + with pytest.raises(file_bindings.ApiException) as e: + file_publication_factory(repository=d_repo.pulp_href, pulp_domain=domain.name) + assert e.value.status == 400 + assert json.loads(e.value.body) == { + "non_field_errors": [f"Objects must all be a part of the {domain.name} domain."] + } + + # Fail distribute + with pytest.raises(file_bindings.ApiException) as e: + file_distribution_factory(repository=d_repo.pulp_href, pulp_domain=domain.name) + assert e.value.status == 400 + assert json.loads(e.value.body) == { + "non_field_errors": [f"Objects must all be a part of the {domain.name} domain."] + } + + # Fail repo modify + content = file_bindings.ContentFilesApi.list(repository_version=d_repo.latest_version_href) + content_hrefs = [c.pulp_href for c in content.results] + body = {"add_content_units": content_hrefs} + with pytest.raises(file_bindings.ApiException) as e: + file_bindings.RepositoriesFileApi.modify(repo.pulp_href, body) + assert e.value.status == 400 + error = json.loads(e.value.body) + assert "add_content_units" in error + assert error["add_content_units"][0].startswith( + f"Content units are not a part of the current domain {domain.name}: [" + ) diff --git a/pulpcore/app/models/repository.py b/pulpcore/app/models/repository.py index a5c0634aa1..5fbff679cb 100644 --- a/pulpcore/app/models/repository.py +++ b/pulpcore/app/models/repository.py @@ -984,6 +984,7 @@ def add_content(self, content): if self.complete: raise ResourceImmutableError(self) + assert not content.exclude(pulp_domain_id=get_domain_pk()).exists() repo_content = [] to_add = set(content.values_list("pk", flat=True)) - set( self.content.values_list("pk", flat=True) @@ -1025,6 +1026,7 @@ def remove_content(self, content): if not content or not content.count(): return + assert not content.exclude(pulp_domain_id=get_domain_pk()).exists() # Normalize representation if content has already been added in this version. # Undo addition by deleting the RepositoryContent. diff --git a/pulpcore/app/serializers/base.py b/pulpcore/app/serializers/base.py index 3b00a8a3e7..446576afbb 100644 --- a/pulpcore/app/serializers/base.py +++ b/pulpcore/app/serializers/base.py @@ -338,7 +338,7 @@ def check_cross_domains(self, data): ): if current_domain.pulp_id != domain_id: raise serializers.ValidationError( - _("Objects must all be apart of the {} domain.").format( + _("Objects must all be a part of the {} domain.").format( current_domain.name ) ) diff --git a/pulpcore/app/util.py b/pulpcore/app/util.py index dacaedce90..5d558b95c0 100644 --- a/pulpcore/app/util.py +++ b/pulpcore/app/util.py @@ -202,7 +202,11 @@ def extract_pk(uri, only_prn=False): raise ValidationError("URI does not contain an unqualified resource PK") -def raise_for_unknown_content_units(existing_content_units, content_units_pks_hrefs): +def raise_for_unknown_content_units( + existing_content_units, + content_units_pks_hrefs, + domain=None, +): """Verify if all the specified content units were found in the database. Args: @@ -210,9 +214,22 @@ def raise_for_unknown_content_units(existing_content_units, content_units_pks_hr specified_content_units. content_units_pks_hrefs (dict): An original dictionary of pk-href pairs that are used for the verification. + domain (pulpcore.plugin.models.Domain): A domain to use for verifying that all the + content units are within the same domain. defaults to current domain. Raises: - ValidationError: If some of the referenced content units are not present in the database + ValidationError: If some of the referenced content units are not present in the database or + content units are from multiple domains """ + domain = domain or get_domain() + bad_domain_pks = existing_content_units.exclude(pulp_domain=domain).values_list("pk", flat=True) + if bad_domain_pks: + bad_hrefs = [content_units_pks_hrefs[str(pk)] for pk in bad_domain_pks] + raise ValidationError( + _("Content units are not a part of the current domain {}: {}").format( + domain.name, bad_hrefs + ) + ) + existing_content_units_pks = existing_content_units.values_list("pk", flat=True) existing_content_units_pks = set(map(str, existing_content_units_pks))