Skip to content

fix: backfill and derive org_id for DomainOrg under RBAC - #1487

Draft
CryptoRodeo wants to merge 5 commits into
pulp:mainfrom
CryptoRodeo:fix/backfill-domainorg-org-id
Draft

CryptoRodeo wants to merge 5 commits into
pulp:mainfrom
CryptoRodeo:fix/backfill-domainorg-org-id

Conversation

@CryptoRodeo

@CryptoRodeo CryptoRodeo commented Sep 16, 2026

Copy link
Copy Markdown
Contributor

Issue

Under RBAC, members of an org that owns a domain were locked out — 404 on reads, 400 on uploads — when the domain's rh-org-<org_id> group held no roles. This happened for DomainOrg rows with a null org_id: their create request carried no internal.org_id, so the org group's role grant was skipped.

Fix

  • Migration 0022 backfills existing domains, deriving the org from the team group's members and granting rh-org-<org_id> the missing roles.
  • Domain creation now derives org_id from the creating user's rh-org-<org_id> membership when the request omits internal.org_id (only when they belong to exactly one such group), so no new null-org_id domains are produced.

Testing

Reproducing and verifying the lock-out requires RBAC active (PulpServiceAccessPolicy); that default is enabled in the shared base commit. Functional tests cover the historical backfill, create-without-internal.org_id, and org-member access; a unit test covers the derive helper.

Companion PR: #1486 (orphan-content fix / RBAC enablement).

🤖 Generated with Claude Code

Summary by Sourcery

Restore organization-member access to DomainOrg resources under RBAC by backfilling missing organization ownership and deriving organization IDs for new domains.

Bug Fixes:

  • Backfill missing DomainOrg organization IDs and restore organization-group roles so existing domains are accessible to their organization members under RBAC.
  • Derive and persist the organization ID during domain creation when identity metadata omits internal.org_id, preventing new role-less DomainOrg records.

Enhancements:

  • Enable database-backed RBAC as the default permission model and align domain creation and migration authorization with RBAC.
  • Ensure service roles are rebuilt transactionally as plugin permissions become available during migrations.
  • Update domain and RBAC functional coverage for organization-member access, historical repairs, self-service creation, and isolation.

Deployment:

  • Update deployed and development REST API defaults to use PulpServiceAccessPolicy.

Documentation:

  • Add a reproducible simulation procedure and results documenting the authenticated organization-member fix and distinguishing remaining PyPI cache-miss behavior.

Tests:

  • Add regression coverage for historical null-org backfills, creation without internal organization metadata, organization-member repository/content access, and organization-ID derivation.

Chores:

  • Remove obsolete settings-based content access policy and legacy DomainBasedPermission test coverage.

@sourcery-ai

sourcery-ai Bot commented Sep 16, 2026

Copy link
Copy Markdown
Contributor

Reviewer's Guide

This PR enables RBAC by default, completes service-role initialization, and fixes organization ownership propagation for domains both historically and at creation time. It restores org-member access by backfilling or deriving org_id and granting the owning org its domain roles, while extending content policies and functional coverage to preserve orphan-content visibility and validate the updated authorization semantics.

Sequence diagram for deriving org_id during domain creation

sequenceDiagram
    participant User
    participant DomainAPI
    participant post_create_domain
    participant UserGroups
    participant DomainOrg
    participant GroupRole

    User->>DomainAPI: Create domain without internal.org_id
    DomainAPI->>post_create_domain: post_create_domain
    post_create_domain->>UserGroups: _derive_org_id_from_user
    UserGroups-->>post_create_domain: Exactly one rh-org-<org_id> membership
    post_create_domain->>DomainOrg: Save org_id
    post_create_domain->>GroupRole: _assign_domain_roles
    GroupRole-->>User: Org members gain domain access
Loading

Flow diagram for historical DomainOrg org_id backfill

flowchart TD
    A[Migration 0022] --> B[Load DomainOrg records]
    B --> C{org_id already set?}
    C -->|Yes| D[Use stored org_id]
    C -->|No| E[_derive_org_id]
    E --> F{Exactly one org across team members?}
    F -->|Yes| G[Save derived org_id]
    F -->|No| H[Skip and log warning]
    D --> I[Create or get rh-org-<org_id> group]
    G --> I
    I --> J[_assign_pair]
    J --> K[Grant core.domain_owner and service.domain_admin]
Loading

File-Level Changes

Change Details Files
Enable RBAC as the default authorization path and make service roles complete and transactionally updated.
  • Switch deployment and development REST permission settings to PulpServiceAccessPolicy.
  • Rebuild domain admin/viewer roles after each plugin migration so late-created plugin permissions are included.
  • Wrap role permission replacement in a transaction to avoid partial role state during upgrades.
  • Update access-policy configuration and related authorization expectations for domain-scoped content access.
deploy/clowdapp.yaml
dev-container/settings.py
pulp_service/pulp_service/app/__init__.py
pulp_service/pulp_service/app/settings.py
CHANGES/enable-rbac-permission-class.feature
Repair missing organization ownership and RBAC grants for existing and newly created domains.
  • Add migration 0022 to derive org_id from team-group members only when a single organization is unambiguous.
  • Backfill DomainOrg.org_id and idempotently grant org-group domain-owner and service-admin roles.
  • Derive org_id at create time from the creator's sole rh-org group when internal.org_id is absent, while refusing ambiguous or empty memberships.
  • Add unit and functional coverage for forward creation, historical backfill, org-member access, and cross-org isolation.
pulp_service/pulp_service/app/migrations/0022_backfill_org_group_roles.py
pulp_service/pulp_service/app/signals.py
pulp_service/pulp_service/tests/unit/test_derive_org_id.py
pulp_service/pulp_service/tests/functional/test_create_without_internal_org_id.py
pulp_service/pulp_service/tests/functional/test_org_member_historical_backfill.py
pulp_service/pulp_service/tests/functional/test_org_member_rbac_access.py
CHANGES/2120.bugfix
Preserve immediate visibility of orphan content for domain members under RBAC.
  • Apply the domain-level content list policy to both generic and typed file content endpoints.
  • Allow domain members with core.view_content to list all content in their domain without repository queryset scoping.
  • Add functional coverage for owner access to orphan content and denial/filtering for unrelated organizations.
pulp_service/pulp_service/app/settings.py
pulp_service/pulp_service/tests/functional/test_content_view_after_upload.py
Migrate functional tests to the RBAC-enabled self-service domain-creation flow and update authorization assertions.
  • Introduce fixtures for template storage domains, self-service creation, and reliable admin cleanup of domain-scoped resources.
  • Replace non-admin generic domain creation with POST /api/pulp/create-domain/ across authentication, domain, group, and content-guard tests.
  • Update expected RBAC behavior from domain-level 403s to filtered 200 responses or 404s where appropriate.
  • Replace deleted Lightwell permission coverage with explicit RBAC role assignment and scoped read-only assertions.
pulp_service/pulp_service/tests/functional/conftest.py
pulp_service/pulp_service/tests/functional/test_authentication.py
pulp_service/pulp_service/tests/functional/test_domain_based_permissions.py
pulp_service/pulp_service/tests/functional/test_domain_dual_write.py
pulp_service/pulp_service/tests/functional/test_group_based_permissions.py
pulp_service/pulp_service/tests/functional/test_content_guard_permission.py
pulp_service/pulp_service/tests/functional/test_lightwell_readonly_group_permission.py
pulp_service/pulp_service/tests/functional/test_lightwell_content_listing_permission.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 fix/backfill-domainorg-org-id branch 3 times, most recently from e59ff36 to 0ef7f79 Compare September 16, 2026 15:31
@CryptoRodeo CryptoRodeo changed the title fix: backfill and derive org_id for DomainOrg under RBAC (PULP-2120) fix: backfill and derive org_id for DomainOrg under RBAC Sep 16, 2026
@CryptoRodeo

CryptoRodeo commented Sep 16, 2026

Copy link
Copy Markdown
Contributor Author

Reviewer guide — where to focus

This branch is two commits: a shared RBAC-enablement commit (identical to the first commit of companion PR #1486) and the actual fix. Please concentrate here:

The fix (review closely):

  • pulp_service/pulp_service/app/migrations/0022_backfill_org_group_roles.py — data migration backfilling rh-org-<org_id> roles for existing domains (derives the org from the team group's members).
  • pulp_service/pulp_service/app/signals.py_derive_org_id_from_user and its call site: derive org_id at create time from the creating user's rh-org-<org_id> membership when the request omits internal.org_id (only when they belong to exactly one such group).
  • Tests: test_org_member_historical_backfill.py, test_create_without_internal_org_id.py, test_org_member_rbac_access.py, test_derive_org_id.py.

Skim only — the shared RBAC-enablement commit ("test: enable RBAC as the default backend…"):

…ssion fixes

The domain-permission fixes only manifest when RBAC
(PulpServiceAccessPolicy) is the active authorization backend rather than
DomainBasedPermission. Switch the default so the functional suite runs
under RBAC, converting the existing tests to the self-service
create-domain flow accordingly.

Also seed service.domain_admin/service.domain_viewer on every plugin's
post_migrate (each rebuild wrapped in a transaction) so the roles hold the
complete permission set once the last plugin has migrated; previously they
were seeded only on the service app's post_migrate, before later plugins
(file, certguard) had created their permissions.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@CryptoRodeo

CryptoRodeo commented Sep 16, 2026

Copy link
Copy Markdown
Contributor Author

@dkliban potential fix for backfilling the org_id values

Switch PulpServiceAccessPolicy to AccessPolicyFromDB and move the
domain-create ContextVar setup into CreateDomainView/MigrateDomainView
via set_domain_create_context() (PULP-2120), dropping the now-inert
settings.ACCESS_POLICIES block and DomainBasedPermission from those
views. This puts the branch on 2120's DB-backed RBAC, under which the
org_id gap below is fixed.

Org members were locked out (404 reads, 400 uploads) of domains whose
rh-org-<org_id> group held no roles -- caused by DomainOrg rows with a
null org_id, whose creation skipped the org group's role grant. The
upload 400 is downstream of the same scope-out: the push client's
idempotency pre-check reads a role-scoped content list, is scoped out to
empty, and re-uploads a dependency already in the index, which
pulp_python's duplicate guard rejects.

Migration 0022 repairs existing rows, deriving the org from the team
group's members and granting rh-org-<org_id> the missing roles. Domain
creation now also derives org_id from the creating user's rh-org-<org_id>
membership when the request omits internal.org_id, so no new null rows
are produced.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@CryptoRodeo
CryptoRodeo force-pushed the fix/backfill-domainorg-org-id branch from 0130a63 to 8d3b059 Compare September 16, 2026 18:49
@CryptoRodeo

CryptoRodeo commented Sep 16, 2026

Copy link
Copy Markdown
Contributor Author

@dkliban I added a simulation instruction doc (simulate the scenarios that the Calunga team was experiencing) and a verdict doc:

Verdict (TL;DR):

  • ✅ Fixes Layer 1 (RBAC scope-out): authenticated org-member reads 404→200, and the failed Konflux upload 400 (blinded dedup pre-check → re-upload → duplicate-guard 400), both repaired by migration 0022.
  • ❌ Doesn't fix (by design, documented): anonymous scoped reads and the simple/ GET 404s (fromager cache-misses, not RBAC).

The anonymous-read gap ("Fix A") is closed by #1486 (feat/rbac-orphaned-content-access), it adds a public-* read bypass in scope_queryset so role-less callers can read a public domain's content. This PR + #1486 together cover Layer 1 + Layer 2a; the simple/ 404s remain a separate (non-RBAC) fromager/data concern (not sure we actually need to do anything here. The issue might be on Calunga's end).

See simulation-results-public-trusted-libraries.md for the verdict, simulation-procedure-public-trusted-libraries.md to run it: 8d3b059

A re-runnable simulation (pub_trusted_sim.py) plus procedure and results
docs for the production public-trusted-libraries 404/400 incident, run
against PULP-2120 RBAC with this branch's org_id fix. Confirms the fix
restores Layer 1 reads (404 -> 200) and the failed Konflux upload (the
400 duplicate-guard chain), and notes what it does not fix (anonymous
scoped reads; the fromager simple/ cache-miss 404s, which aren't RBAC).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Signed-off-by: Bryan ramos <bramos@redhat.com>
@CryptoRodeo
CryptoRodeo force-pushed the fix/backfill-domainorg-org-id branch from 8d3b059 to d3fd57e Compare September 16, 2026 19:34
CryptoRodeo and others added 2 commits September 16, 2026 16:55
53e3876 moved this branch to AccessPolicyFromDB and dropped
settings.ACCESS_POLICIES, but two tests still asserted the
settings-based design and failed. Assert AccessPolicyFromDB and
drop the obsolete content-override settings test.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Read-only `pulpcore-manager domainorg_backfill_report` command that previews
which missing-org_id DomainOrg rows migration 0022 will backfill vs skip (and
why), so operators can review and remediate the skip-set before enabling RBAC.
Derivation is extracted into a shared `domainorg_backfill` helper that 0022 now
imports, keeping the report and the migration in lock-step.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Signed-off-by: Bryan ramos <bramos@redhat.com>
@CryptoRodeo

Copy link
Copy Markdown
Contributor Author

Added a command that will let us see which domain orgs get backfilled and which were skipped. This should make it easier to see what was skipped and why: 8a16b81

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