Repository navigation
Forward-merge release/2.2 into develop - #369
Merged
Merged
Conversation
#### Overview Fix authenticated token propagation for jobs submitted through `POST /v1/jobs/async/submit`. `AuthMiddleware` validated Bearer and `idToken` credentials, but the custom async-submit route called `get_auth_token()` outside a NAT `SessionManager` request context. The submitter therefore passed `None` to 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 by `AuthMiddleware` before 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 and `idToken` cookie 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 ```text uv run --frozen pytest -q tests/aiq_agent/auth/test_auth_utils.py frontends/aiq_api/tests/test_job_submit_data_sources.py 43 passed, 1 warning in 1.40s uv run --frozen ruff check . All checks passed! uv run --frozen ruff format --check . 371 files already formatted uv run --frozen pytest 1803 passed, 13 skipped, 24 warnings in 60.54s ``` - [x] I ran the relevant local checks or explained why they are not applicable. - [x] I added or updated tests for behavior changes. - [ ] I updated documentation for user-facing or contributor-facing changes. Existing documentation already specifies the intended async-worker token propagation contract. - [x] I confirmed this PR does not include secrets, credentials, or internal-only data. - [x] I certify this contribution under the Developer Certificate of Origin (DCO) and signed my commits with `git commit -s` or an equivalent sign-off. - [x] I replaced the DCO sign-off placeholder with my GitHub commit identity and kept the required angle brackets around the email address. #### Where should reviewers start? Start with `src/aiq_agent/auth/utils.py`, then review `test_submit_job_forwards_authenticated_request_token` for the Bearer and cookie regression coverage at the actual REST submission boundary. #### Related Issues - N/A — QA-reported release issue. ## Summary by CodeRabbit ## Summary by CodeRabbit - **Bug Fixes** - Improved authentication token handling for asynchronous job submissions. - Tokens validated by request authentication are now correctly used when submitting jobs. - Established clearer token-precedence behavior across registered token sources, validated request tokens, and fallback credentials. - **Tests** - Added coverage for bearer-token and cookie-based authentication scenarios for async job submission. - Added validation that token precedence is honored during token extraction. Authors: - Ajay Thorve (https://github.com/AjayThorve) Approvers: - Kyle Zheng (https://github.com/KyleZheng1284) URL: #367
Author
|
SUCCESS - forward-merge complete. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Forward-merge triggered by push to release/2.2 that creates a PR to keep develop up-to-date. If this PR is unable to be immediately merged due to conflicts, it will remain open for the team to manually merge. See forward-merger docs for more info.