Skip to content

fix: return 401 with WWW-Authenticate for unauthenticated content req… - #1649

Merged
git-hyagi merged 2 commits into
pulp:mainfrom
git-hyagi:content-guard-403-issue
Oct 28, 2026
Merged

git-hyagi merged 2 commits into
pulp:mainfrom
git-hyagi:content-guard-403-issue

Conversation

@git-hyagi

@git-hyagi git-hyagi commented Oct 27, 2026 •

Copy link
Copy Markdown
Collaborator

…uests

HeaderContentGuard.permit() was raising PermissionError (mapped to 403) when the x-rh-identity header was missing, preventing browsers from showing the authentication popup. Only FeatureContentGuard had the AuthenticationError path that triggers 401 + WWW-Authenticate.

Extends patch 0078 to also handle HeaderContentGuard, covering regular distributions that use the domain default content guard. Also derives the WWW-Authenticate realm from CONTENT_ORIGIN instead of hardcoding it.

Summary by Sourcery

Return an authentication challenge for unauthenticated content requests while retaining forbidden responses for authorization failures.

Bug Fixes:

  • Return HTTP 401 responses with a Basic WWW-Authenticate challenge when content requests lack the x-rh-identity header.
  • Derive the authentication challenge realm from CONTENT_ORIGIN while preserving 403 responses for ordinary authorization denials.

Enhancements:

  • Apply consistent authentication-error handling to both header-based and feature-based content guards, including composite guards.

Documentation:

  • Update patch documentation to describe authentication handling for both content guard types and the CONTENT_ORIGIN-derived realm.

Tests:

  • Add coverage for missing identity headers, non-identity header denials, composite guard behavior, and derived authentication realms.

…uests

HeaderContentGuard.permit() was raising PermissionError (mapped to 403)
when the x-rh-identity header was missing, preventing browsers from
showing the authentication popup. Only FeatureContentGuard had the
AuthenticationError path that triggers 401 + WWW-Authenticate.

Extends patch 0078 to also handle HeaderContentGuard, covering regular
distributions that use the domain default content guard. Also derives
the WWW-Authenticate realm from CONTENT_ORIGIN instead of hardcoding it.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@sourcery-ai

sourcery-ai Bot commented Oct 27, 2026 •

Copy link
Copy Markdown
Contributor

Reviewer's Guide

Extends the missing-identity authentication flow to HeaderContentGuard so unauthenticated content requests return 401 with a Basic WWW-Authenticate challenge, while ordinary authorization failures remain 403; the challenge realm is now derived from CONTENT_ORIGIN and covered by focused tests.

Sequence diagram for unauthenticated content authentication challenge

sequenceDiagram
    participant Client
    participant ContentApp
    participant ContentGuard

    Client->>ContentApp: GET content request
    ContentApp->>ContentGuard: permit()
    ContentGuard-->>ContentApp: AuthenticationError
    ContentApp-->>Client: 401 Unauthorized
    ContentApp-->>Client: WWW-Authenticate: Basic realm=CONTENT_ORIGIN
Loading

Flow diagram for content authorization outcomes

flowchart TD
    A[Content request] --> B{ x-rh-identity present? }
    B -->|No| C[AuthenticationError]
    C --> D[401 Unauthorized + Basic challenge]
    B -->|Yes, not authorized| E[PermissionError]
    E --> F[403 Forbidden]
    B -->|Yes, authorized| G[Serve content]
Loading

File-Level Changes

Change Details Files
Route missing identity headers through the authentication-challenge path for header-based content guards.
  • Raise AuthenticationError for a missing x-rh-identity header while retaining PermissionError behavior for other headers.
  • Preserve authentication-required outcomes through composite guard evaluation and map them to HTTP 401.
  • Cover both identity-specific and non-identity header behavior with unit tests.
images/assets/patches/0078-Return-authentication-challenge-for-missing-identity.patch
pulp_service/pulp_service/tests/unit/test_authentication_error_content_guard.py
Derive the Basic authentication challenge realm from the configured content origin.
  • Build the WWW-Authenticate realm from CONTENT_ORIGIN rather than a hardcoded host.
  • Verify the derived realm and existing 403 behavior in handler tests.
images/assets/patches/0078-Return-authentication-challenge-for-missing-identity.patch
pulp_service/pulp_service/tests/unit/test_authentication_error_content_guard.py
Update patch documentation to describe the expanded authentication handling.
  • Document HeaderContentGuard and FeatureContentGuard authentication errors, composite propagation, and CONTENT_ORIGIN-based challenge realms.
images/assets/patches/CLAUDE.md

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

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey - I've reviewed your changes and they look great!

Sourcery assessment

Needs a human reviewer. Requests missing the x-rh-identity header now receive a 401 with a Basic authentication challenge whose realm comes from CONTENT_ORIGIN, rather than the previous denial response. Reverting restores the old response, but cannot retract challenges already sent; this does not grant access or expose protected content.


Sourcery is free for open source - if you like our reviews please consider sharing them ✨

When both headers are missing, the identity guard now raises
AuthenticationError which takes priority over PermissionError
in the composite guard's denial logic (per patch 0078).

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@git-hyagi
git-hyagi merged commit 83c773d into pulp:main Oct 28, 2026
6 checks passed
@git-hyagi
git-hyagi deleted the content-guard-403-issue branch October 28, 2026 11:00
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