Skip to content

Commit

Permalink
TODO: Add a permission to pull from pull-through distributions
Browse files Browse the repository at this point in the history
closes #1624
  • Loading branch information
lubosmj committed Jul 4, 2024
1 parent 78ca84c commit 80f115e
Show file tree
Hide file tree
Showing 7 changed files with 88 additions and 12 deletions.
2 changes: 2 additions & 0 deletions CHANGES/1624.bugfix
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
Permitted users with the `pull_new_distributions` permission to pull data via pull-through
distributions.
4 changes: 4 additions & 0 deletions pulp_container/app/access_policy.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
39 changes: 34 additions & 5 deletions pulp_container/app/authorization.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -209,12 +214,36 @@ def has_pull_permissions(self, path):
except ContainerNamespace.DoesNotExist:
# Check if 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

pull_through_cache_distribution = (
ContainerPullThroughDistribution.objects.annotate(path=Value(path))
.filter(path__startswith=F("base_path"))
.order_by("-base_path")
.first()
)
if pull_through_cache_distribution:
# Check if user is allowed to pull new content via a pull-through distribution
return self.has_permission(
pull_through_cache_distribution, "GET", "pull_new_distribution", {}
)
else:
# Check if user is allowed to view distributions in the namespace
return self.has_permission(
namespace, "GET", "view_distribution", {"name": namespace_name}
)

pull_through_cache_distribution = (
ContainerPullThroughDistribution.objects.annotate(path=Value(path))
.filter(path__startswith=F("base_path"))
.order_by("-base_path")
.first()
)
if pull_through_cache_distribution:
return self.has_permission(
namespace, "GET", "view_distribution", {"name": namespace_name}
pull_through_cache_distribution, "GET", "pull_new_distribution", {"base_path": path}
)

return self.has_permission(distribution, "GET", "pull", {"base_path": path})
else:
return self.has_permission(distribution, "GET", "pull", {"base_path": path})

def has_push_permissions(self, path):
"""
Expand Down
Original file line number Diff line number Diff line change
@@ -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')]},
),
]
4 changes: 4 additions & 0 deletions pulp_container/app/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -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",
),
]


Expand Down
12 changes: 12 additions & 0 deletions pulp_container/app/viewsets.py
Original file line number Diff line number Diff line change
Expand Up @@ -1453,6 +1453,15 @@ 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 or "
"has_namespace_or_obj_perms:container.pull_containerdistribution",
],
},
],
"creation_hooks": [
{
Expand All @@ -1472,12 +1481,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",
],
}

Expand Down
22 changes: 15 additions & 7 deletions pulp_container/tests/functional/api/test_pull_through_cache.py
Original file line number Diff line number Diff line change
Expand Up @@ -143,21 +143,29 @@ 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)


def test_conflicting_names_and_paths(
Expand Down

0 comments on commit 80f115e

Please sign in to comment.