Skip to content

fix(auth): forward synthesized HF principal to PDP check - #768

Merged
anastasia-nesterenko merged 3 commits into
mainfrom
anesterenko/auth-fix-passing-token-in-header
Jul 17, 2026
Merged

fix(auth): forward synthesized HF principal to PDP check#768
anastasia-nesterenko merged 3 commits into
mainfrom
anesterenko/auth-fix-passing-token-in-header

Conversation

@anastasia-nesterenko

@anastasia-nesterenko anastasia-nesterenko commented Jul 17, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Pass preprocessed principal headers into the PDP authorization check.
  • Preserve existing behavior for requests that use their original headers.
  • Add regression coverage verifying that HF-compatible Bearer authentication authorizes as the synthesized service principal.

Problem

The HF-compatible authentication flow synthesizes principal headers from service Bearer tokens without mutating request.headers. The PDP authorization path rebuilt its headers from the original request, discarding the synthesized service principal.

Testing

  • uv run --frozen pytest packages/nmp_common/tests/auth/test_middleware.py -q
  • 27 tests passed

Summary by CodeRabbit

  • Bug Fixes
    • Improved authorization handling for HF-compatible requests by supporting preprocessed headers in the authorization flow.
    • Service principals synthesized from Bearer tokens (e.g., service:<name>) are now passed correctly to authorization checks.
  • Tests
    • Added unit coverage to confirm authorization succeeds and that the derived service principal ID is correctly provided to the authorization client.

Signed-off-by: anastasia-nesterenko <anesterenko@nvidia.com>
@coderabbitai

coderabbitai Bot commented Jul 17, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 1004885d-5dd5-479d-8da2-9f5e68f658a6

📥 Commits

Reviewing files that changed from the base of the PR and between 6450b7d and d98915d.

📒 Files selected for processing (1)
  • packages/nmp_common/src/nmp/common/auth/middleware.py
🚧 Files skipped from review as they are similar to previous changes (1)
  • packages/nmp_common/src/nmp/common/auth/middleware.py

📝 Walkthrough

Walkthrough

Authorization middleware now accepts preprocessed headers for PDP checks, preserving synthesized HF X-NMP-* values without mutating requests. A unit test verifies Bearer-token service-principal authorization and the principal sent to AuthClient.

Changes

Authorization header processing

Layer / File(s) Summary
Forward headers through authorization
packages/nmp_common/src/nmp/common/auth/middleware.py, packages/nmp_common/tests/auth/test_middleware.py
_handle_principal_headers_request forwards preprocessed headers to _handle_auth_check, which uses them for principal and scope extraction; the test verifies the synthesized service:models principal and HTTP 200 authorization response.

Suggested reviewers: benmccown, ironcommit

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title is concise and accurately summarizes the main auth-path fix.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch anesterenko/auth-fix-passing-token-in-header

Comment @coderabbitai help to get the list of available commands.

@anastasia-nesterenko anastasia-nesterenko changed the title fix(auth): Pass headers to auth check fix(auth): forward synthesized HF principal to PDP check Jul 17, 2026
@github-actions

github-actions Bot commented Jul 17, 2026

Copy link
Copy Markdown
Contributor
Suite Lines Covered Line Rate Branch Rate
Unit Tests 25557/32774 78.0% 62.6%
Integration Tests 14749/31423 46.9% 19.3%

Comment thread packages/nmp_common/src/nmp/common/auth/middleware.py Outdated
Signed-off-by: anastasia-nesterenko <anesterenko@nvidia.com>
@anastasia-nesterenko
anastasia-nesterenko added this pull request to the merge queue Jul 17, 2026
Merged via the queue into main with commit 57a4700 Jul 17, 2026
58 checks passed
@anastasia-nesterenko
anastasia-nesterenko deleted the anesterenko/auth-fix-passing-token-in-header branch July 17, 2026 20:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants