feat: expose AI-Q as an API with Auth Middleware - #173
cdgamarose-nv merged 19 commits into
Conversation
Greptile SummaryThis PR adds raw ASGI Confidence Score: 5/5Safe to merge — all previously flagged P0/P1 issues have been resolved; only P2 style/performance suggestions remain. The three issues from the prior review cycle (missing TokenValidator inheritance, raw claims return, JWKS thread safety) are all addressed. The remaining findings are P2: holding a lock during network I/O in JWTValidator (latency concern, not a correctness bug), overly broad exception handling in CLI auth init, and a redundant ImportError in an except clause. None of these block correct operation. frontends/aiq_api/src/aiq_api/auth/jwt_validator.py — lock-during-I/O pattern worth addressing before high-load deployment; frontends/cli/cli.py — silent auth failure swallowing.
|
| Filename | Overview |
|---|---|
| frontends/aiq_api/src/aiq_api/auth/middleware.py | New raw ASGI auth middleware; path-filter, token-validation, and ContextVar propagation all look correct. |
| frontends/aiq_api/src/aiq_api/auth/jwt_validator.py | JWTValidator now correctly extends TokenValidator and validate() returns the full contract dict. However, _get_signing_key holds _jwks_lock for the entire duration of network I/O, serializing concurrent JWT validations under load. |
| frontends/aiq_api/src/aiq_api/auth/base.py | Clean abstract base with well-documented user-dict contract; no issues. |
| frontends/aiq_api/src/aiq_api/auth/errors.py | Simple AuthError exception type; no issues. |
| src/aiq_agent/agents/chat_researcher/register.py | skip_clarifier logic correctly reads from middleware ContextVar; minor redundancy in except clause. |
| src/aiq_agent/agents/chat_researcher/agent.py | AuthError handling added in both shallow and deep research nodes; skip_clarifier flag correctly threaded through the turn-reset dict. |
| src/aiq_agent/agents/chat_researcher/models/state.py | skip_clarifier field added with correct default (False); no issues. |
| src/aiq_agent/auth/utils.py | Authorization header fallback added correctly; Bearer eyJ heuristic is acceptable for JWT detection. |
| frontends/aiq_api/tests/test_auth.py | Comprehensive test suite covering middleware path filtering, auth flow, JWT validator, and ContextVar reset; no issues. |
Sequence Diagram
sequenceDiagram
participant Client
participant Middleware as AuthMiddleware
participant Validator as JWTValidator
participant JWKS as JWKS Endpoint
participant App as ASGI App
Client->>Middleware: HTTP request
Middleware->>Middleware: check _is_external()
alt Internal host
Middleware->>App: pass through (internal user)
else External host
Middleware->>Middleware: check _path_allowed()
alt Path blocked
Middleware-->>Client: 404
else Exempt path
Middleware->>App: anonymous user, no token needed
else Auth disabled
Middleware->>App: anonymous user
else Auth enabled
Middleware->>Middleware: _extract_token()
alt No token found
Middleware-->>Client: 401
else Token present
Middleware->>Validator: can_handle(token)
Validator-->>Middleware: true
Middleware->>Validator: validate(token)
Validator->>JWKS: fetch signing key (under lock)
JWKS-->>Validator: public key
Validator-->>Middleware: user dict or None
alt Invalid
Middleware-->>Client: 401
else Valid
Middleware->>App: user stored in scope + ContextVar
end
end
end
end
Reviews (3): Last reviewed commit: "revert links" | Re-trigger Greptile
|
Tip: Greploops — Automatically fix all review issues by running Use the Greptile plugin for Claude Code to query reviews, search comments, and manage custom context directly from your terminal. |
|
Tested end-to-end with Keycloak as an external OIDC provider to validate the ISV integration path. Setup:
Unit tests: 29/29 pass ( Integration tests against live Keycloak (RS256 JWTs):
Full server test (
Verified:
ISV replicability: Confirmed. An ISV needs only a Follow-up Items (not in scope for this PR)The PR includes application-level auth documentation (README + inline docstrings). The following deployment-level work is needed separately to fully enable auth in production:
|
…s#173) * update nat version and compatibility fixes * middleware for auth for api access * add unit tests for aiq api auth * fix issues with missing dep packages * direct import from module * add nvidia-nat-core dependency * make auth error user facing * remove duplicate status field * edit pyproject.toml * remove log which prints token * fix bugs in validator * fix ruff check * fix ruff check * fix ruff version * add aiq api as known first party * lint fixes for new ruff * fix dead links * revert links
No description provided.