Repository navigation
fix(auth): propagate REST submit token to async workers - #367
Conversation
Signed-off-by: Ajay Thorve <athorve@nvidia.com>
|
Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually. Contributors can view more details about this message here. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Enterprise Run ID: 📒 Files selected for processing (1)
📜 Recent review details⏰ Context from checks skipped due to timeout. (3)
🧰 Additional context used📓 Path-based instructions (3)**/*.py📄 CodeRabbit inference engine (CONTRIBUTING.md)
Files:
src/aiq_agent/**/*.py📄 CodeRabbit inference engine (AGENTS.md)
Files:
{src/aiq_agent/auth/**,frontends/aiq_api/src/aiq_api/auth/**}⚙️ CodeRabbit configuration file
Files:
🔇 Additional comments (1)
Walkthrough
ChangesAuthenticated token forwarding
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant AuthMiddleware
participant AsyncSubmitEndpoint
participant submit_agent_job
Client->>AuthMiddleware: Send bearer header or idToken cookie
AuthMiddleware-->>AsyncSubmitEndpoint: Provide validated token
AsyncSubmitEndpoint->>submit_agent_job: Pass auth_token
submit_agent_job-->>Client: Return HTTP 200
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/aiq_agent/auth/utils.py`:
- Around line 192-193: Update the exception handler around token retrieval in
the auth utility to log only a static failure message. Remove the exception
object from the logger.debug call so credential-bearing details from
auth-context failures are never emitted.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Enterprise
Run ID: ef799947-e622-40c8-87ee-f6a65399f3d9
📒 Files selected for processing (3)
frontends/aiq_api/tests/test_job_submit_data_sources.pysrc/aiq_agent/auth/utils.pytests/aiq_agent/auth/test_auth_utils.py
📜 Review details
⏰ Context from checks skipped due to timeout. (3)
- GitHub Check: Lint and Hooks
- GitHub Check: Pytest and Coverage
- GitHub Check: Script Validation
🧰 Additional context used
📓 Path-based instructions (4)
**/*.py
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Run ruff check and ruff format validation for Python code changes
**/*.py: Python code must be linted and formatted with Ruff using line length 120, target Python 3.11, rule sets E,F,W,I,PL,UP, and isort force-single-line configuration
Never commit secrets, tokens, or environment-specific hostnames; use environment variables and SecretStr instead, resolving API keys at runtime
Never print or log secret values, including in tool output or error messages
Missing-secret paths must degrade gracefully (stub/skip), not crash or leak
Do not hand-reformat unrelated code when making changes; match the existing import and formatting style
Files:
frontends/aiq_api/tests/test_job_submit_data_sources.pytests/aiq_agent/auth/test_auth_utils.pysrc/aiq_agent/auth/utils.py
**/*test*.py
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Run pytest for all behavior changes in Python code
Files:
frontends/aiq_api/tests/test_job_submit_data_sources.pytests/aiq_agent/auth/test_auth_utils.py
src/aiq_agent/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
src/aiq_agent/**/*.py: Respect authenticated data sources by honoring requires_auth, per-user token pass-through, and backend token validators; apply owner guardrails before loading protected report or artifact context into an agent
Do not weaken or bypass AuthMiddleware, validators, or auth gating without a prior design discussion
Files:
src/aiq_agent/auth/utils.py
{src/aiq_agent/auth/**,frontends/aiq_api/src/aiq_api/auth/**}
⚙️ CodeRabbit configuration file
{src/aiq_agent/auth/**,frontends/aiq_api/src/aiq_api/auth/**}: Review authentication changes for issuer/audience validation, token parsing, error hygiene, logging safety,
and compatibility with local and deployed modes. Do not accept changes that expose tokens, weaken validation,
or blur trusted server-side identity with client-supplied fields.
Files:
src/aiq_agent/auth/utils.py
🧠 Learnings (1)
📚 Learning: 2026-06-14T17:49:00.640Z
Learnt from: torkian
Repo: NVIDIA-AI-Blueprints/aiq PR: 273
File: frontends/aiq_api/tests/test_sse_reconnect_cursor.py:384-401
Timestamp: 2026-06-14T17:49:00.640Z
Learning: When using `unittest.mock.patch` for code that imports dependencies inside functions/generators (e.g., inside `aiq_api.routes.jobs`), don’t patch via an attribute that doesn’t exist on the consuming module. If the generator does `from ..jobs.event_store import EventStore` inside the generator body, then `aiq_api.routes.jobs` will not have an `EventStore` attribute; patch the source class/method in its defining module instead (e.g., `aiq_api.jobs.event_store.EventStore.get_events_async`). Patching `aiq_api.routes.jobs.EventStore...` would raise `AttributeError` because that symbol is not present at module scope.
Applied to files:
frontends/aiq_api/tests/test_job_submit_data_sources.py
🔇 Additional comments (3)
src/aiq_agent/auth/utils.py (1)
24-30: LGTM!Also applies to: 159-162
tests/aiq_agent/auth/test_auth_utils.py (1)
56-72: LGTM!frontends/aiq_api/tests/test_job_submit_data_sources.py (1)
168-234: LGTM!
Signed-off-by: Ajay Thorve <athorve@nvidia.com>
|
/merge |
32f17be
into
NVIDIA-AI-Blueprints:release/2.2
Overview
Fix authenticated token propagation for jobs submitted through
POST /v1/jobs/async/submit.AuthMiddlewarevalidated Bearer andidTokencredentials, but the custom async-submit route calledget_auth_token()outside a NATSessionManagerrequest context. The submitter therefore passedNoneto Dask even though the request had an authenticated principal. Authenticated tools in the worker then ran without the submitting user's identity.This change makes
get_auth_token()consult the request token already validated and stored byAuthMiddlewarebefore falling back to NAT request metadata. Registered token fetchers remain first so each Dask task's job-scoped token stays authoritative. Regression tests exercise the real async-submit route with both Bearer andidTokencookie credentials.DCO sign-off for the squash commit
Signed-off-by: Ajay Thorve athorve@nvidia.com
Signed-off-by: Ajay Thorve AjayThorve@users.noreply.github.com
Validation
git commit -sor an equivalent sign-off.Where should reviewers start?
Start with
src/aiq_agent/auth/utils.py, then reviewtest_submit_job_forwards_authenticated_request_tokenfor the Bearer and cookie regression coverage at the actual REST submission boundary.Related Issues
Summary by CodeRabbit
Summary by CodeRabbit
Bug Fixes
Tests