Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
diff --git a/pulpcore/app/models/publication.py b/pulpcore/app/models/publication.py
index 8c9535d..8d3690e 100644
index 8c9535d31..c66c235c7 100644
--- a/pulpcore/app/models/publication.py
+++ b/pulpcore/app/models/publication.py
@@ -26,6 +26,7 @@ from pulpcore.app.models import AutoAddObjPermsMixin
Expand All @@ -10,15 +10,29 @@ index 8c9535d..8d3690e 100644
from pulpcore.responses import ArtifactResponse

from .base import BaseModel, MasterModel
@@ -665,6 +666,7 @@ class CompositeContentGuard(ContentGuard, AutoAddObjPermsMixin):
@@ -529,6 +530,13 @@ class HeaderContentGuard(ContentGuard, AutoAddObjPermsMixin):
def permit(self, request):
header_content = request.headers.get(self.header_name)
if not header_content:
+ if self.header_name.casefold() == "x-rh-identity":
+ _logger.info(
+ "Authentication required. Header %s not found.",
+ self.header_name,
+ )
+ raise AuthenticationError(_("Authentication required."))
+
_logger.debug(
"Access not allowed. Header {header_name} not found.".format(
header_name=self.header_name
@@ -665,6 +673,7 @@ class CompositeContentGuard(ContentGuard, AutoAddObjPermsMixin):
Permit if ANY content-guard allows (OR permissions).
"""
errors = []
+ authentication_error = None
if not self.guards.all():
# No guards specified? PASS
return
@@ -674,6 +676,13 @@ class CompositeContentGuard(ContentGuard, AutoAddObjPermsMixin):
@@ -674,6 +683,13 @@ class CompositeContentGuard(ContentGuard, AutoAddObjPermsMixin):
try:
detail_guard.permit(request)
return # success on first-pass
Expand All @@ -32,7 +46,7 @@ index 8c9535d..8d3690e 100644
except PermissionError as pe:
guard_error = _("Guard: '{}', HREF: '{}', class: '{}', denial: [{}].").format(
detail_guard.name, get_url(detail_guard), type(detail_guard), str(pe)
@@ -681,6 +690,8 @@ class CompositeContentGuard(ContentGuard, AutoAddObjPermsMixin):
@@ -681,6 +697,8 @@ class CompositeContentGuard(ContentGuard, AutoAddObjPermsMixin):
errors.append(guard_error)

# If we get here - FAIL
Expand All @@ -42,7 +56,7 @@ index 8c9535d..8d3690e 100644
raise PermissionError(msg)

diff --git a/pulpcore/content/handler.py b/pulpcore/content/handler.py
index 5abe50f..fbccce4 100644
index 5abe50f80..13eae096e 100644
--- a/pulpcore/content/handler.py
+++ b/pulpcore/content/handler.py
@@ -20,6 +20,7 @@ from aiohttp.web_exceptions import (
Expand All @@ -53,23 +67,24 @@ index 5abe50f..fbccce4 100644
)
from asgiref.sync import sync_to_async
from django.utils import timezone
@@ -68,6 +69,7 @@ from pulpcore.exceptions import ( # noqa: E402
@@ -68,6 +69,8 @@ from pulpcore.exceptions import ( # noqa: E402
UnsupportedDigestValidationError,
)
from pulpcore.metrics import artifacts_size_counter # noqa: E402
+from pulp_service.exceptions import AuthenticationError # noqa: E402
+from urllib.parse import urlparse # noqa: E402

log = logging.getLogger(__name__)
_current_distribution = ContextVar("current_distribution", default=None)
@@ -269,6 +271,7 @@ class Handler:
@@ -269,6 +272,7 @@ class Handler:
"""
guard_key = "DISTRO#GUARD#PRESENT"
present = await cached.get(guard_key, base_key=base_key)
+ guard = None
if present == b"True" or present is None:
path = request.match_info["path"]
if not (distro := _current_distribution.get()):
@@ -278,11 +281,11 @@ class Handler:
@@ -278,11 +282,11 @@ class Handler:
_current_distribution.set(distro)
try:
guard = await sync_to_async(cls._permit)(request, distro)
Expand All @@ -83,7 +98,7 @@ index 5abe50f..fbccce4 100644
await cached.set(guard_key, str(guard), base_key=base_key)

@AsyncContentCache(
@@ -517,6 +520,15 @@ class Handler:
@@ -517,6 +521,16 @@ class Handler:
return False
try:
guard.cast().permit(request)
Expand All @@ -92,9 +107,10 @@ index 5abe50f..fbccce4 100644
+ 'Path: %(p)s requires authentication for guard: "%(g)s" reason: %(r)s',
+ {"p": request.path, "g": guard.name, "r": str(ae)},
+ )
+ realm = urlparse(settings.CONTENT_ORIGIN).hostname or "pulp-content"
+ raise HTTPUnauthorized(
+ reason=str(ae),
+ headers={"WWW-Authenticate": 'Basic realm="packages.redhat.com"'},
+ headers={"WWW-Authenticate": f'Basic realm="{realm}"'},
+ )
except PermissionError as pe:
log.debug(
Expand Down
2 changes: 1 addition & 1 deletion images/assets/patches/CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ The separate `oci-storage-backup-setup` repository is unaffected.

- **Package:** pulpcore
- **Files:** `pulpcore/app/models/publication.py`, `pulpcore/content/handler.py`
- **Description:** Preserves an authentication-required result while evaluating composite content guards, then maps it to `401 Unauthorized` with a TBR Basic challenge in the content app. Ordinary authorization denials remain `403 Forbidden`. The patch also keeps guard-presence caching safe for both responses and unexpected guard exceptions.
- **Description:** Makes both `HeaderContentGuard` and `FeatureContentGuard` raise `AuthenticationError` (instead of `PermissionError`) when the `x-rh-identity` header is missing, preserves that result through composite content guards, and maps it to `401 Unauthorized` with a `WWW-Authenticate: Basic` challenge (realm derived from `CONTENT_ORIGIN`) in the content app. Ordinary authorization denials remain `403 Forbidden`. The patch also keeps guard-presence caching safe for both responses and unexpected guard exceptions.

### 0077 - Directory membership timestamp experiment

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
from aiohttp.web_exceptions import HTTPForbidden, HTTPUnauthorized
from asgiref.sync import async_to_sync

from pulpcore.app.models import CompositeContentGuard
from pulpcore.app.models import CompositeContentGuard, HeaderContentGuard
from pulpcore.content.handler import Handler

from pulp_service.app.access_policy import PulpServiceAccessPolicy
Expand Down Expand Up @@ -35,6 +35,36 @@ def _distribution(detail_guard):
return distribution


class TestHeaderContentGuardAuthentication:
def _make_guard(self, header_name="x-rh-identity"):
guard = HeaderContentGuard()
guard.header_name = header_name
guard.header_value = ""
guard.jq_filter = '""'
return guard

def test_missing_identity_header_raises_authentication_error(self):
guard = self._make_guard(header_name="x-rh-identity")
request = MagicMock()
request.headers.get.return_value = None

with pytest.raises(AuthenticationError, match="Authentication required"):
guard.permit(request)

def test_missing_non_identity_header_raises_permission_error(self):
guard = self._make_guard(header_name="x-custom-header")
request = MagicMock()
request.headers.get.return_value = None

with pytest.raises(PermissionError):
guard.permit(request)

with pytest.raises(PermissionError) as exc_info:
guard.permit(request)

assert not isinstance(exc_info.value, AuthenticationError)


class TestCompositeContentGuardAuthentication:
def test_authentication_error_is_not_authorization_error(self):
assert not issubclass(AuthenticationError, PermissionError)
Expand Down Expand Up @@ -102,6 +132,16 @@ def test_permit_maps_authentication_error_to_401(self, settings):
assert exc_info.value.status == 401
assert exc_info.value.headers["WWW-Authenticate"] == 'Basic realm="packages.stage.redhat.com"'

def test_permit_derives_realm_from_content_origin(self, settings):
detail_guard = _detail_guard("identity", AuthenticationError("Authentication required."))
request = MagicMock(path="/lightwell/osv/record")
settings.CONTENT_ORIGIN = "https://packages.redhat.com"

with pytest.raises(HTTPUnauthorized) as exc_info:
Handler._permit(request, _distribution(detail_guard))

assert exc_info.value.headers["WWW-Authenticate"] == 'Basic realm="packages.redhat.com"'

def test_permit_keeps_permission_error_as_403(self):
detail_guard = _detail_guard("feature", PermissionError("Access denied."))
request = MagicMock(path="/lightwell/osv/record")
Expand Down
Loading