Skip to content

feat: enable RBAC by defaulting to PulpServiceAccessPolicy - #1467

Draft
CryptoRodeo wants to merge 1 commit into
pulp:mainfrom
CryptoRodeo:feat/enable-rbac-permission-class
Draft

feat: enable RBAC by defaulting to PulpServiceAccessPolicy#1467
CryptoRodeo wants to merge 1 commit into
pulp:mainfrom
CryptoRodeo:feat/enable-rbac-permission-class

Conversation

@CryptoRodeo

@CryptoRodeo CryptoRodeo commented Sep 11, 2026

Copy link
Copy Markdown
Contributor

Switch the default permission class from DomainBasedPermission to PulpServiceAccessPolicy, in both the dev container and prod (clowdapp).

This fixes the bug that surfaced when RBAC was first turned on: after pushing content, a user couldn't view it. RBAC scopes content reads to repository membership, so orphan (not-in-a-repo) content was hidden. It now works because domain members hold core.view_content (via service.domain_admin), and the content access policy gates list on that permission instead of scoping by repository.

Add functional tests covering the fix: create a domain, upload orphan content, and confirm the owning org can view it (generic and typed file endpoints) while an unrelated org cannot.

Summary by Sourcery

Enable PulpServiceAccessPolicy by default and preserve domain-member access to orphan content across REST content endpoints.

New Features:

  • Enable RBAC authorization by default for REST APIs in development and production deployments.
  • Allow domain members to view orphan content immediately after upload through generic and typed file content endpoints.
  • Add functional coverage for owner visibility, unrelated-organization isolation, and RBAC domain access behavior.

Bug Fixes:

  • Fix content read-after-upload failures caused by repository-based scoping hiding orphan content from domain members.

Enhancements:

  • Ensure service roles are rebuilt after plugin migrations and populated transactionally with the complete plugin permission set.
  • Update domain and lightwell permission tests for the RBAC authorization model and supported self-service domain creation flow.

Deployment:

  • Switch the production ClowdApp default REST permission class to PulpServiceAccessPolicy.

Documentation:

  • Document RBAC as the default REST authorization backend and orphan-content visibility behavior.

Tests:

  • Add reusable functional fixtures for self-service domain creation and reliable domain cleanup.
  • Update existing authentication, domain, group, content-guard, and lightwell tests to validate RBAC behavior.

Chores:

  • Remove the obsolete lightwell content-listing permission test.

@sourcery-ai

sourcery-ai Bot commented Sep 11, 2026

Copy link
Copy Markdown
Contributor

Reviewer's Guide

The PR switches development and production REST APIs to PulpServiceAccessPolicy so domain-scoped RBAC permissions allow owners to immediately view orphan content, while adding functional tests that validate owner visibility through both content endpoints and deny unrelated organizations access.

Sequence diagram for RBAC orphan content visibility

sequenceDiagram
    actor Owner as OwningOrg
    actor Other as UnrelatedOrg
    participant API as ContentAPI
    participant Policy as PulpServiceAccessPolicy
    participant RBAC as DomainRBAC
    participant Content as OrphanContent

    Owner->>API: Upload content
    API->>RBAC: Check domain membership
    RBAC-->>Policy: core.view_content via service.domain_admin
    Owner->>API: GET content endpoint
    API->>Policy: Check core.view_content
    Policy->>Content: List orphan content
    Content-->>Owner: Content visible
    Other->>API: GET content endpoint
    API->>Policy: Check core.view_content
    Policy-->>Other: Access denied
Loading

File-Level Changes

Change Details Files
Make RBAC the default REST authorization mechanism across development and production deployments.
  • Replace DomainBasedPermission with PulpServiceAccessPolicy in REST framework settings.
  • Document the default permission-class change and its orphan-content behavior.
deploy/clowdapp.yaml
dev-container/settings.py
CHANGES/enable-rbac-permission-class.feature
Add functional coverage for read-after-upload visibility and cross-organization isolation of orphan content.
  • Create a domain through the self-service endpoint and upload orphan file content into it.
  • Verify the owning organization can list the content through generic and typed file endpoints.
  • Verify an unrelated organization receives no content or a forbidden response.
  • Clean up the created domain after each test fixture.
pulp_service/pulp_service/tests/functional/test_content_view_after_upload.py

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

@CryptoRodeo
CryptoRodeo force-pushed the feat/enable-rbac-permission-class branch 2 times, most recently from f2b7880 to 6970ede Compare September 11, 2026 19:18
@CryptoRodeo

Copy link
Copy Markdown
Contributor Author

Had to update the other tests so CI passes, they used domain based permissions instead of RBAC. This makes the diff larger, but we won't have to dig through Konflux logs to see what failed and if the orphaned content tests passed.

Switch the default permission class from DomainBasedPermission to
PulpServiceAccessPolicy, in both the dev container and prod (clowdapp).

This fixes the bug that surfaced when RBAC was first turned on: after
pushing content, a user couldn't view it. RBAC scopes content reads to
repository membership, so orphan (not-in-a-repo) content was hidden. It
now works because domain members hold core.view_content (via
service.domain_admin), and the content access policy gates list on that
permission instead of scoping by repository. Both the generic /content/
and the typed /content/file/files/ endpoints get the override, since the
typed viewset otherwise keeps repository-based queryset scoping.

Fix an ordering bug in the service role seeding that the flip exposed.
service.domain_admin / service.domain_viewer were populated only on the
service app's post_migrate, on the assumption every plugin's permissions
already existed by then. They do not: post_migrate is emitted in app
order, and plugins that emit after service (file, certguard) had not yet
created their permissions, so on a fresh single migrate the role was
seeded without file.add_filerepository -- domain owners then got 403
creating a repository. Seed on every plugin's post_migrate again (each
rebuild wrapped in a transaction so no partial role is ever observed),
so the roles hold the complete permission set once the last plugin has
migrated.

Add functional tests covering the fix: create a domain, upload orphan
content, and confirm the owning org can view it (generic and typed file
endpoints) while an unrelated org cannot.

Convert the existing functional suite to RBAC so CI stays green with the
new default. The DomainBasedPermission-era tests assumed the old backend
and started returning 403s once RBAC was on; they now drive the
self-service create-domain flow and assert role-based access. A shared
conftest seeds template-domain-s3 and provides a create-domain fixture
with robust admin-auth teardown. The lightwell content-listing test is
removed (superseded by the RBAC content access policy).

Co-Authored-By: Claude Opus 4.8 <[email protected]>
@CryptoRodeo
CryptoRodeo force-pushed the feat/enable-rbac-permission-class branch from 6970ede to 18dc380 Compare September 11, 2026 21:15
@CryptoRodeo

CryptoRodeo commented Sep 11, 2026

Copy link
Copy Markdown
Contributor Author

orphaned content tests have passed (pulp_service/pulp_service/tests/functional/test_content_view_after_upload.py):

usr/local/lib/pulp/lib64/python3.11/site-packages/pulp_service/tests/functional/test_content_view_after_upload.py::test_owner_org_views_orphan_content_via_generic_endpoint PASSED [ 39%]
usr/local/lib/pulp/lib64/python3.11/site-packages/pulp_service/tests/functional/test_content_view_after_upload.py::test_owner_org_views_orphan_content_via_typed_file_endpoint PASSED [ 40%]
usr/local/lib/pulp/lib64/python3.11/site-packages/pulp_service/tests/functional/test_content_view_after_upload.py::test_unrelated_org_cannot_view_orphan_content PASSED [ 41%]

@CryptoRodeo

Copy link
Copy Markdown
Contributor Author

@dkliban The tests for viewing orphaned content passed. I wonder what other edge cases we should test, if any others exist? AFAIK it's standard to link content to a repository and the only edge case is orphaned content 🤔

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant