feat: enable RBAC by making PulpServiceAccessPolicy the default

CHANGES/enable-rbac.feature

@@ -0,0 +1,25 @@
+Transitioned domain and content authorization to the RBAC system by making
+``PulpServiceAccessPolicy`` the default permission class. Domain create and migrate
+endpoints now require an authenticated user, with migration additionally enforcing an
+explicit ``core.change_domain`` object-level check, and domain-create request context is
+reset after each request so a failed create cannot leak into the next.
+
+The switchover also restores access that the previous permission model granted:
+
+* Domain members can view orphan content (uploaded into a domain but not yet in a
+  repository) on both the generic and typed content endpoints, gated by the
+  domain-scoped ``core.view_content`` permission instead of repository scoping.
+* Reads against ``public-*`` domains no longer return 404 for callers with no role on the
+  domain; the public-domain read bypass is honoured in ``scope_queryset`` as well as
+  ``has_permission``.
+* Org members regain access to org-owned domains whose ``rh-org-<org_id>`` group held no
+  roles because of a null ``org_id``. A data migration backfills existing domains, and
+  domain creation now derives ``org_id`` from the creating user's ``rh-org-<org_id>``
+  membership when the request omits ``internal.org_id``.
+* Domains created through the generic pulpcore domains endpoint (not just the self-service
+  ``CreateDomainView``) again get their owner ``DomainOrg`` row and RBAC roles assigned;
+  the former ``DomainBasedPermission`` default used to populate the request context those
+  assignments depend on.
+
+``DomainBasedPermission`` itself is retained (no longer the default, and no longer
+referenced anywhere) only to keep this change focused; its removal is left to a follow-up.

Dockerfile

@@ -113,9 +113,6 @@ RUN ln -s /usr/local/lib/pulp/bin/pulpcore-manager /usr/local/bin/pulpcore-manag
 RUN chmod 2775 /var/lib/pulp/{scripts,media,tmp,assets}
 RUN chown :root /var/lib/pulp/{scripts,media,tmp,assets}
 
-COPY images/assets/patches/0018-Re-root-the-registry-API-at-api-pulp-v2.patch /tmp/
-RUN patch -p1 -d /usr/local/lib/pulp/lib/python${PYTHON_VERSION}/site-packages < /tmp/0018-Re-root-the-registry-API-at-api-pulp-v2.patch
-
 COPY images/assets/patches/0022-Adds-authentication-to-the-mvn-deploy-api.patch /tmp/
 RUN patch -p1 -d /usr/local/lib/pulp/lib/python${PYTHON_VERSION}/site-packages < /tmp/0022-Adds-authentication-to-the-mvn-deploy-api.patch
 

deploy/clowdapp.yaml

@@ -1752,7 +1752,7 @@ parameters:
     value: "['pulp_service.app.authentication.RHServiceAccountCertAuthentication','rest_framework.authentication.BasicAuthentication','rest_framework.authentication.SessionAuthentication','pulp_service.app.authentication.RHTermsBasedRegistryAuthentication']"
   - name: PULP_REST_FRAMEWORK__DEFAULT_PERMISSION_CLASSES
     description: The default permission classes for the REST API
-    value: "['pulp_service.app.authorization.DomainBasedPermission']"
+    value: "['pulp_service.app.access_policy.PulpServiceAccessPolicy']"
   - name: PULP_AUTHENTICATION_JSON_HEADER
     description: The name of the header where Authentication information is found.
     value: "HTTP_X_RH_IDENTITY"

dev-container/settings.py

@@ -46,7 +46,7 @@
 )
 
 REST_FRAMEWORK__DEFAULT_PERMISSION_CLASSES = (
-    "pulp_service.app.authorization.DomainBasedPermission",
+    "pulp_service.app.access_policy.PulpServiceAccessPolicy",
 )
 
 AUTHENTICATION_JSON_HEADER = "HTTP_X_RH_IDENTITY"

images/assets/patches/CLAUDE.md

@@ -32,12 +32,6 @@ The separate `oci-storage-backup-setup` repository is unaffected.
 
 ## Patches
 
-### 0018 — Re-root the registry API at /api/pulp/v2/
-
-- **Package:** pulp_container
-- **Files:** `pulp_container/app/content.py`, `pulp_container/app/redirects.py`, `pulp_container/app/token_verification.py`, `pulp_container/app/urls.py`
-- **Description:** Moves all container registry URL routes from `/v2/` to `/api/pulp/v2/` and the content app prefix from `/pulp/container/` to `/api/pulp-container/`. Replaces `RegistryPermission` with `DomainBasedPermission`.
-
 ### 0022 — Adds authentication to the mvn deploy api
 
 - **Package:** pulp_maven

pulp_service/pulp_service/app/__init__.py

@@ -55,13 +55,20 @@ def _populate_domain_view_access_policies(sender, apps, **kwargs):  # noqa: ARG0
 
 
 def _populate_service_roles(sender, apps, **kwargs):  # noqa: ARG001
-    """Create/update service.domain_admin and service.domain_viewer roles with plugin permissions."""
-    # Only run for the service app's own migration. pulp_service depends on all plugins, so by the
-    # time it migrates every plugin's permissions exist. Running once (instead of once per plugin)
-    # avoids repeated permissions.set() DELETE+INSERT windows that cause 403s during rolling upgrades.
-    if sender.label != "service":
-        return
-
+    """Create/update service.domain_admin and service.domain_viewer roles with plugin permissions.
+
+    Connected to every plugin's post_migrate (see ready()). It must fire on each one:
+    Django creates a plugin's permissions during that plugin's own post_migrate, so a
+    receiver only sees the permissions of plugins whose post_migrate already ran. Running
+    once on pulp_service's own post_migrate silently dropped any plugin whose permissions
+    were created afterwards (e.g. pulp_file), so domain owners got 403s creating file repos
+    despite holding service.domain_admin. Firing on every plugin guarantees the last one
+    assembles the complete set. permissions.set() only DELETE+INSERTs the diff on the
+    role_permissions m2m, so a firing whose set is unchanged writes nothing to it. And since
+    Django's create_permissions is purely additive (it never deletes existing rows), an early
+    firing during an upgrade can only grow the role -- no rolling-upgrade window where a domain
+    owner loses a permission they already held.
+    """
     try:
         Role = apps.get_model("core", "Role")
         Permission = apps.get_model("auth", "Permission")

pulp_service/pulp_service/app/access_policy.py

@@ -4,6 +4,7 @@
 from binascii import Error as Base64DecodeError
 
 import jq
+from django.conf import settings
 from django.db.models import Q
 from django.http import Http404
 from rest_framework.permissions import SAFE_METHODS
@@ -12,6 +13,8 @@
 from pulpcore.plugin.models import Content, Domain
 from pulpcore.plugin.util import get_domain_pk
 
+from pulp_service.app.authorization import set_domain_create_context
+from pulp_service.app.features_service import check_subscription
 from pulp_service.app.models import DomainOrg
 
 _logger = logging.getLogger(__name__)
@@ -33,7 +36,9 @@ class PulpServiceAccessPolicy(AccessPolicyFromSettings):
         1. Superuser bypass
         2. Public domain anonymous read (safe methods on public-* domains)
         3. PyPi content guard delegation
-        4. Fall through to standard RBAC (super().has_permission)
+        4. DOMAIN_ACCESS_POLICIES grants (subscription feature / readonly group, safe methods)
+        5. admin-readonly group read (safe methods)
+        6. Fall through to standard RBAC (super().has_permission)
     """
 
     def has_permission(self, request, view):
@@ -50,8 +55,38 @@ def has_permission(self, request, view):
             if pypi_access is not None:
                 return pypi_access
 
+            # DOMAIN_ACCESS_POLICIES: subscription-feature and readonly-group read grants.
+            if domain:
+                policy = self._get_domain_policies().get(domain.name, {})
+                if policy and self._check_domain_policy(request, domain, request.user, policy):
+                    return True
+
+            # admin-readonly members can read any endpoint the checks above did not decide.
+            # Content guards still win, because a guarded PyPI view returns an explicit
+            # allow/deny above.
+            if self._is_admin_readonly(request.user):
+                return True
+
+        # Generic pulpcore DomainViewSet create (POST domains-list): populate the ContextVars the
+        # post_create_domain signal (signals.py) consumes for the DomainOrg/RBAC dual-write.
+        # CreateDomainView sets these itself; the generic endpoint used to get them from the
+        # DomainBasedPermission default, which no longer exists. Without this, a domain created
+        # here gets no DomainOrg row and no owner roles, so it is invisible to its own org under
+        # RBAC. set_domain_create_context overwrites both vars unconditionally, so a create that
+        # is denied below cannot leak this request's principal into the next create.
+        if self._is_domain_create(request):
+            set_domain_create_context(request)
+
         return super().has_permission(request, view)
 
+    @staticmethod
+    def _is_domain_create(request):
+        """True for a POST to the generic pulpcore DomainViewSet create endpoint (domains-list)."""
+        if request.method in SAFE_METHODS:
+            return False
+        match = getattr(request, "resolver_match", None)
+        return bool(match and match.view_name == "domains-list")
+
     @staticmethod
     def _is_public_domain_read(request):
         """
@@ -103,10 +138,41 @@ def scope_queryset(self, view, qs):
         if self._is_domain_content_read(view, qs):
             return qs
 
+        request = getattr(view, "request", None)
+        user = getattr(request, "user", None)
+        is_safe_read = request is not None and getattr(request, "method", None) in SAFE_METHODS
+
+        # The cross-cutting SAFE-method read grants that has_permission honours (admin-readonly
+        # and DOMAIN_ACCESS_POLICIES subscription/readonly-group) must be honoured here too:
+        # otherwise the grantee passes has_permission (200) but the default RBAC scoping below
+        # filters the queryset to empty because they hold no per-object role. For domain-scoped
+        # models base.py has already filtered the queryset to the request domain, so returning it
+        # unscoped stays within that domain (no cross-domain leak).
+        if is_safe_read and user is not None:
+            # admin-readonly is a global read group (support/break-glass), so it reads every model.
+            if self._is_admin_readonly(user):
+                return qs
+            # A DOMAIN_ACCESS_POLICIES grant is scoped to a single domain, so it may only return
+            # the queryset unscoped for models base.py domain-scoped (those with a pulp_domain FK).
+            # Global models (Domain, Group, User, Role) are NOT domain-scoped by base.py; returning
+            # them unscoped would leak every tenant's rows. They fall through: Domain is handled by
+            # the qs.model is Domain block below (readonly-group members see only their policy
+            # domain + public-*); the rest get standard per-object RBAC scoping.
+            domain = getattr(request, "pulp_domain", None)
+            if domain and hasattr(qs.model, "pulp_domain"):
+                policy = self._get_domain_policies().get(domain.name, {})
+                if policy and self._check_domain_policy(request, domain, user, policy):
+                    return qs
+
         qs = super().scope_queryset(view, qs)
         if qs.model is Domain:
-            public_domains = Domain.objects.filter(name__startswith="public-")
-            qs = (qs | public_domains).distinct()
+            extra = Domain.objects.filter(name__startswith="public-")
+            # DOMAIN_ACCESS_POLICIES readonly-group members see the policy's domain in listings.
+            for domain_name, policy in self._get_domain_policies().items():
+                group = policy.get("readonly_group")
+                if group and user is not None and user.is_authenticated and user.groups.filter(name=group).exists():
+                    extra = extra | Domain.objects.filter(name=domain_name)
+            qs = (qs | extra).distinct()
 
         return qs
 
@@ -206,3 +272,46 @@ def _get_org_id(decoded_header_content):
             except json.JSONDecodeError:
                 return None
         return None
+
+    @staticmethod
+    def _is_admin_readonly(user):
+        """True if user is an authenticated member of the ADMIN_READONLY_GROUP."""
+        group_name = settings.ADMIN_READONLY_GROUP
+        return bool(group_name and user and user.is_authenticated and user.groups.filter(name=group_name).exists())
+
+    @staticmethod
+    def _get_domain_policies():
+        return getattr(settings, "DOMAIN_ACCESS_POLICIES", {})
+
+    def _check_domain_policy(self, request, domain, user, policy):
+        """
+        Returns True if a DOMAIN_ACCESS_POLICIES entry grants read access, else None.
+
+        ``subscription_endpoints`` are path prefixes: a policy applies when the request path
+        (after stripping the domain routing prefix) starts with one of them, and the caller's
+        org holds the ``subscription_feature``. ``readonly_group`` grants read to any
+        authenticated member of the named group.
+        """
+        subscription_feature = policy.get("subscription_feature")
+        subscription_endpoints = policy.get("subscription_endpoints", [])
+        if subscription_feature and subscription_endpoints:
+            path = request.path_info
+            if domain:
+                api_root = getattr(settings, "API_ROOT", "/api/pulp/")
+                domain_prefix = f"{api_root.rstrip('/')}/{domain.name}/"
+                if path.startswith(domain_prefix):
+                    path = "/" + path[len(domain_prefix) :]
+            if any(path.startswith(endpoint) for endpoint in subscription_endpoints):
+                decoded_header = self._get_decoded_identity_header(request)
+                org_id = self._get_org_id(decoded_header)
+                try:
+                    if org_id and check_subscription(org_id, [subscription_feature]):
+                        return True
+                except Exception:
+                    _logger.exception("Unexpected error checking %s subscription", subscription_feature)
+
+        readonly_group = policy.get("readonly_group", "")
+        if readonly_group and user.is_authenticated and user.groups.filter(name=readonly_group).exists():
+            return True
+
+        return None

pulp_service/pulp_service/app/authorization.py

@@ -24,6 +24,28 @@
 group_var = ContextVar("group")
 
 
+def set_domain_create_context(request):
+    """
+    Populate the ContextVars the post_create_domain signal (signals.py) consumes for
+    the RBAC/DomainOrg dual-write.
+
+    Called for both domain-create paths: CreateDomainView (self-service) and the generic
+    pulpcore DomainViewSet create (from PulpServiceAccessPolicy.has_permission). The former
+    DomainBasedPermission default used to set these for the domain_create action.
+    """
+    header = request.META.get("HTTP_X_RH_IDENTITY")
+    org_id = None
+    if header:
+        try:
+            org_id = org_id_json_path.input_value(json.loads(b64decode(header))).first()
+        except (Base64DecodeError, json.JSONDecodeError):
+            org_id = None
+    # Set unconditionally: a basic-auth create with no X-RH-IDENTITY must clear org_id_var, not
+    # inherit a stale value left in this worker's context by an earlier request.
+    org_id_var.set(org_id)
+    user_id_var.set(request.user.pk)
+
+
 class IsAdminOrAdminReadOnly(BasePermission):
     """
     Full access for superusers/staff; GET/HEAD/OPTIONS-only for members of the
@@ -48,17 +70,16 @@ def has_permission(self, request, view):
 class DomainBasedPermission(BasePermission):
     """
     A Permission Class that grants permission to users who's org_id matches the requested Domain's org_id.
+
+    Retained only for the container registry auth path: the ``0018-Re-root-the-registry-API``
+    patch derives ``RegistryPermission`` from this class. The general REST API now defaults to
+    ``PulpServiceAccessPolicy``. TODO: repoint patch 0018 at the RBAC policy and remove this.
     """
 
     def _is_admin_readonly(self, user):
         """True if user is an authenticated member of the ADMIN_READONLY_GROUP."""
         group_name = settings.ADMIN_READONLY_GROUP
-        return bool(
-            group_name
-            and user
-            and user.is_authenticated
-            and user.groups.filter(name=group_name).exists()
-        )
+        return bool(group_name and user and user.is_authenticated and user.groups.filter(name=group_name).exists())
 
     def _has_domain_access(self, domain_pk, org_id, user):
         """

pulp_service/pulp_service/app/content_view_viewsets.py

@@ -29,7 +29,6 @@
 from pulp_rpm.app.models import Modulemd, Package, PackageEnvironment, PackageGroup, UpdateRecord
 from pulp_rpm.app.models.advisory import UpdateReference
 
-from pulp_service.app.authorization import DomainBasedPermission
 from pulp_service.app.content_view_models import ContentView, ContentViewSearchScope
 from pulp_service.app.content_view_serializers import (
     ContentViewErrataSerializer,
@@ -112,7 +111,6 @@ class ContentViewViewSet(
     serializer_class = ContentViewSerializer
     filterset_class = ContentViewFilter
     ordering = "-pulp_created"
-    permission_classes = [DomainBasedPermission]
     queryset_filtering_required_permission = "service.view_contentview"
 
     DEFAULT_ACCESS_POLICY = {
@@ -242,7 +240,6 @@ class ContentViewSearchViewSet(NamedModelViewSet):
     queryset = ContentViewSearchScope.objects.none()
     parent_viewset = ContentViewViewSet
     parent_lookup_kwargs = {"content_view_pk": "content_view__pk"}
-    permission_classes = [DomainBasedPermission]
 
     DEFAULT_ACCESS_POLICY = {
         "statements": [

pulp_service/pulp_service/app/signals.py

@@ -203,8 +203,8 @@ def post_create_domain(sender, **kwargs):  # noqa: ARG001
                     group = explicit_group
                     if explicit_group:
                         do = DomainOrg.objects.create(org_id=org_id, group=explicit_group)
-                    # Skip the auto-assigned rh-org-<org_id> groups: those are per-org and
-                    # already covered by the org_id match in DomainBasedPermission. Only an
+                    # Skip the auto-assigned rh-org-<org_id> groups as the DomainOrg's group:
+                    # those are per-org and get their own role grant below (org_group). Only an
                     # explicit "team" group should scope domain visibility to a group.
                     # Query through the pulpcore Group proxy (not user.groups, which yields
                     # base auth.Group instances) so assign_role classifies it as a Group.