Skip to content

Commit 887d255

Browse files
authored
fix: allow authenticated users to read PyPI views and public domains (#1231)
* fix: allow authenticated users to read PyPI views and public domains DomainBasedPermission.has_permission() only allowed anonymous users to bypass domain access checks for safe requests on PyPI views and public domains. Authenticated users hitting the same endpoints were routed to _has_domain_access(), which returned 403 when their org had no DomainOrg entry for the target domain. Move the PyPI (isinstance PyPIMixin) and public-domain checks before the authentication gate so both anonymous and authenticated users get read access. This restores the behavior that existed when patch 0038 wired AllowUnauthPull onto PyPIMixin.permission_classes. Also remove the now-dead AllowUnauthPull class — nothing references it since patch 0038 was deleted in 5096c48. Assisted-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * refactor: move PyPIMixin import to module level, add unit tests Move the lazy PyPIMixin import from inside has_permission() to the module header. The circular dependency that originally motivated the lazy import no longer exists since patch 0038 was deleted. Add unit tests for DomainBasedPermission.has_permission() covering: - Anonymous and authenticated GET/HEAD on PyPI views (allowed) - Anonymous and authenticated GET on public domains (allowed) - Anonymous GET on private domains (denied) - Anonymous POST on PyPI views and public domains (denied) - Authenticated POST on PyPI views requires domain access (denied) - Superuser bypass (allowed) Assisted-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * style: apply ruff format to test file Assisted-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 0ac03af commit 887d255

2 files changed

Lines changed: 185 additions & 20 deletions

File tree

pulp_service/pulp_service/app/authorization.py

Lines changed: 10 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,8 @@
1111
from pulpcore.plugin.models import Domain
1212
from pulpcore.plugin.util import extract_pk, get_domain_pk
1313

14+
from pulp_python.app.pypi.views import PyPIMixin
15+
1416
from pulp_service.app.models import DomainOrg
1517

1618
_logger = logging.getLogger(__name__)
@@ -48,16 +50,15 @@ def has_permission(self, request, view):
4850

4951
user = request.user
5052

51-
# Anonymous users can make safe requests on public domains and PyPI views
53+
# Allow safe requests on PyPI views and public domains for all users
54+
if request.method in SAFE_METHODS:
55+
if isinstance(view, PyPIMixin):
56+
return True
57+
domain = getattr(request, "pulp_domain", None)
58+
if domain and "public-" in domain.name:
59+
return True
60+
5261
if not user.is_authenticated:
53-
if request.method in SAFE_METHODS:
54-
from pulp_python.app.pypi.views import PyPIMixin
55-
56-
if isinstance(view, PyPIMixin):
57-
return True
58-
domain = getattr(request, "pulp_domain", None)
59-
if domain and "public-" in domain.name:
60-
return True
6162
return False
6263

6364
# Check if user is creating a domain or creating a resource within a domain
@@ -156,14 +157,3 @@ def scope_queryset(self, view, qs):
156157
query |= Q(domain_orgs__org_id=org_id)
157158

158159
return qs.filter(query).distinct()
159-
160-
161-
class AllowUnauthPull(BasePermission):
162-
"""
163-
A Permission Class that grants permission to make GET/HEAD/OPTIONS requests without authN/authZ
164-
"""
165-
166-
def has_permission(self, request, view):
167-
if request.method in SAFE_METHODS:
168-
return True
169-
return None
Lines changed: 175 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,175 @@
1+
"""
2+
Unit tests for DomainBasedPermission.has_permission().
3+
4+
These tests mock Django ORM and Pulp internals so they can run without
5+
a live Pulp stack. They cover the safe-method bypass for PyPI views
6+
and public domains, for both anonymous and authenticated users.
7+
"""
8+
9+
from types import SimpleNamespace
10+
from unittest.mock import MagicMock, patch
11+
12+
from pulp_service.app.authorization import DomainBasedPermission
13+
14+
15+
def _make_request(method="GET", user=None, domain=None, view_name="pypi-metadata"):
16+
"""Build a mock DRF request."""
17+
request = MagicMock()
18+
request.method = method
19+
request.user = user or _make_anonymous_user()
20+
request.pulp_domain = domain
21+
request.resolver_match = MagicMock()
22+
request.resolver_match.view_name = view_name
23+
request.META = {"REQUEST_METHOD": method, "PATH_INFO": "/api/pypi/test/main/simple/"}
24+
return request
25+
26+
27+
def _make_anonymous_user():
28+
user = MagicMock()
29+
user.is_authenticated = False
30+
user.is_superuser = False
31+
return user
32+
33+
34+
def _make_authenticated_user():
35+
user = MagicMock()
36+
user.is_authenticated = True
37+
user.is_superuser = False
38+
user.groups.values_list.return_value = []
39+
return user
40+
41+
42+
def _make_domain(name):
43+
domain = SimpleNamespace(name=name, pk=42)
44+
return domain
45+
46+
47+
def _make_pypi_view():
48+
"""Create a mock view that is an instance of PyPIMixin."""
49+
from pulp_python.app.pypi.views import PyPIMixin
50+
51+
class FakePyPIView(PyPIMixin):
52+
pass
53+
54+
view = FakePyPIView()
55+
return view
56+
57+
58+
def _make_regular_view():
59+
"""Create a mock view that is NOT a PyPIMixin."""
60+
return MagicMock()
61+
62+
63+
class TestSafeMethodBypass:
64+
"""Verify that safe methods on PyPI views and public domains are allowed for all users."""
65+
66+
def test_anonymous_get_pypi_view_allowed(self):
67+
permission = DomainBasedPermission()
68+
request = _make_request(method="GET", user=_make_anonymous_user())
69+
view = _make_pypi_view()
70+
71+
assert permission.has_permission(request, view) is True
72+
73+
def test_authenticated_get_pypi_view_allowed(self):
74+
permission = DomainBasedPermission()
75+
request = _make_request(method="GET", user=_make_authenticated_user())
76+
view = _make_pypi_view()
77+
78+
assert permission.has_permission(request, view) is True
79+
80+
def test_anonymous_head_pypi_view_allowed(self):
81+
permission = DomainBasedPermission()
82+
request = _make_request(method="HEAD", user=_make_anonymous_user())
83+
view = _make_pypi_view()
84+
85+
assert permission.has_permission(request, view) is True
86+
87+
def test_authenticated_head_pypi_view_allowed(self):
88+
permission = DomainBasedPermission()
89+
request = _make_request(method="HEAD", user=_make_authenticated_user())
90+
view = _make_pypi_view()
91+
92+
assert permission.has_permission(request, view) is True
93+
94+
def test_anonymous_get_public_domain_allowed(self):
95+
permission = DomainBasedPermission()
96+
domain = _make_domain("public-trusted-libraries")
97+
request = _make_request(method="GET", user=_make_anonymous_user(), domain=domain)
98+
view = _make_regular_view()
99+
100+
assert permission.has_permission(request, view) is True
101+
102+
def test_authenticated_get_public_domain_allowed(self):
103+
permission = DomainBasedPermission()
104+
domain = _make_domain("public-trusted-libraries")
105+
request = _make_request(method="GET", user=_make_authenticated_user(), domain=domain)
106+
view = _make_regular_view()
107+
108+
assert permission.has_permission(request, view) is True
109+
110+
111+
class TestSafeMethodDenied:
112+
"""Verify that safe methods are denied when they should be."""
113+
114+
def test_anonymous_get_private_domain_non_pypi_denied(self):
115+
permission = DomainBasedPermission()
116+
domain = _make_domain("my-private-domain")
117+
request = _make_request(method="GET", user=_make_anonymous_user(), domain=domain)
118+
view = _make_regular_view()
119+
120+
assert permission.has_permission(request, view) is False
121+
122+
def test_anonymous_get_no_domain_non_pypi_denied(self):
123+
permission = DomainBasedPermission()
124+
request = _make_request(method="GET", user=_make_anonymous_user(), domain=None)
125+
view = _make_regular_view()
126+
127+
assert permission.has_permission(request, view) is False
128+
129+
130+
class TestUnsafeMethodsDenied:
131+
"""Verify that unsafe methods still require domain access."""
132+
133+
def test_anonymous_post_pypi_view_denied(self):
134+
permission = DomainBasedPermission()
135+
request = _make_request(method="POST", user=_make_anonymous_user())
136+
view = _make_pypi_view()
137+
138+
assert permission.has_permission(request, view) is False
139+
140+
def test_anonymous_post_public_domain_denied(self):
141+
permission = DomainBasedPermission()
142+
domain = _make_domain("public-trusted-libraries")
143+
request = _make_request(method="POST", user=_make_anonymous_user(), domain=domain)
144+
view = _make_regular_view()
145+
146+
assert permission.has_permission(request, view) is False
147+
148+
@patch("pulp_service.app.authorization.get_domain_pk", return_value=42)
149+
@patch("pulp_service.app.authorization.DomainOrg.objects")
150+
def test_authenticated_post_pypi_view_checks_domain_access(self, mock_domain_org, mock_get_domain_pk):
151+
permission = DomainBasedPermission()
152+
request = _make_request(
153+
method="POST",
154+
user=_make_authenticated_user(),
155+
view_name="simple-detail",
156+
)
157+
request.META["HTTP_X_RH_IDENTITY"] = ""
158+
view = _make_pypi_view()
159+
160+
mock_domain_org.filter.return_value.exists.return_value = False
161+
162+
assert permission.has_permission(request, view) is False
163+
164+
165+
class TestSuperuserBypass:
166+
"""Verify that superusers bypass all checks."""
167+
168+
def test_superuser_always_allowed(self):
169+
permission = DomainBasedPermission()
170+
user = MagicMock()
171+
user.is_superuser = True
172+
request = _make_request(method="DELETE", user=user)
173+
view = _make_regular_view()
174+
175+
assert permission.has_permission(request, view) is True

0 commit comments

Comments
 (0)