diff --git a/images/assets/patches/0078-Return-authentication-challenge-for-missing-identity.patch b/images/assets/patches/0078-Return-authentication-challenge-for-missing-identity.patch index af45f771..480d554e 100644 --- a/images/assets/patches/0078-Return-authentication-challenge-for-missing-identity.patch +++ b/images/assets/patches/0078-Return-authentication-challenge-for-missing-identity.patch @@ -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 @@ -10,7 +10,21 @@ 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 = [] @@ -18,7 +32,7 @@ index 8c9535d..8d3690e 100644 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 @@ -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 @@ -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 ( @@ -53,15 +67,16 @@ 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) @@ -69,7 +84,7 @@ index 5abe50f..fbccce4 100644 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) @@ -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) @@ -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( diff --git a/images/assets/patches/CLAUDE.md b/images/assets/patches/CLAUDE.md index fd426b17..190f7282 100644 --- a/images/assets/patches/CLAUDE.md +++ b/images/assets/patches/CLAUDE.md @@ -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 diff --git a/pulp_service/pulp_service/tests/functional/test_domain_dual_write.py b/pulp_service/pulp_service/tests/functional/test_domain_dual_write.py index 6fe5e5ee..885e67be 100644 --- a/pulp_service/pulp_service/tests/functional/test_domain_dual_write.py +++ b/pulp_service/pulp_service/tests/functional/test_domain_dual_write.py @@ -18,6 +18,7 @@ LEGACY_DEFAULT_IDENTITY_CONTENT_GUARD_NAME, VPN_VERIFIED_HEADER_NAME, ) +from pulp_service.exceptions import AuthenticationError # These tests exercise the RBAC dual-write in the post_create_domain signal # (pulp_service/app/signals.py) via the self-service create-domain endpoint @@ -306,9 +307,9 @@ def test_composite_allows_identity_or_verified_vpn(): ) ) - with pytest.raises(PermissionError): + with pytest.raises(AuthenticationError): composite_guard.permit(SimpleNamespace(headers={})) - with pytest.raises(PermissionError): + with pytest.raises(AuthenticationError): composite_guard.permit(SimpleNamespace(headers={VPN_VERIFIED_HEADER_NAME: "ZmFsc2U="})) finally: domain.delete() diff --git a/pulp_service/pulp_service/tests/unit/test_authentication_error_content_guard.py b/pulp_service/pulp_service/tests/unit/test_authentication_error_content_guard.py index 03494ade..07f1dfd4 100644 --- a/pulp_service/pulp_service/tests/unit/test_authentication_error_content_guard.py +++ b/pulp_service/pulp_service/tests/unit/test_authentication_error_content_guard.py @@ -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 @@ -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) @@ -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")