feat(azure-openai): wire AAD (Entra ID) client_credentials Bearer auth (#302 Phase F D6.6) - #388
Merged
Conversation
#302 Phase F D6.6) Adds the second Azure auth scheme to aisix-provider-azure-openai. Today the bridge supports only the resource-key scheme (`api-key:` header). This PR adds AAD client_credentials (Entra ID) so an operator can configure a ProviderKey backed by a service-principal app registration instead of pasting the resource's master api-key. Backward-compatible: existing api-key deployments keep working unchanged. The auth scheme is autodetected from the secret shape: - Secret starts with `{` → JSON-parse as AAD credentials {tenant_id, client_id, client_secret}. Bridge mints a token via the client_credentials grant, caches it, and sends Authorization: Bearer <minted-token>. - Otherwise → verbatim string, used as the resource api-key (sent via the api-key: header per the existing path). ## Wire shape (AAD branch) ``` POST https://login.microsoftonline.com/{tenant_id}/oauth2/v2.0/token Content-Type: application/x-www-form-urlencoded grant_type=client_credentials &client_id=<app-registration-uuid> &client_secret=<rotation-managed-secret> &scope=https://cognitiveservices.azure.com/.default ``` Unlike Vertex SA OAuth (#387), AAD client_credentials is a straight form-encoded POST — NO JWT signing on the gateway side. No `jsonwebtoken` dep added; pure reqwest + serde. ## Cache Keyed by `(tenant_id, client_id)` — multiple ProviderKeys backed by the same AAD app share a slot, but distinct apps under the same tenant don't collide. Refresh 60s before upstream-reported expiry. ## Error classification (audit-aware) Mirrors the Vertex audit MEDIUM fix from ai-gateway#387: - AAD 5xx → BridgeError::UpstreamStatus + Retry-After propagated (transient backend should hit cooldown layer, not 500 operator-must-fix). - AAD 4xx → BridgeError::Config (invalid_client / revoked secret / wrong scope IS operator-actionable). ## Files - `aad_token_mint.rs` (new, ~290 lines + tests): TokenMinter, AadCredentials, RwLock-backed cache. 7 unit tests covering happy mint, cache reuse, cache separation across distinct apps, 5xx/4xx classification, empty/URL-injection rejection at validate(). - `bridge.rs`: - Added `AzureSecret` discriminated parse (api-key verbatim vs AAD JSON), and `AzureAuth` resolved-header pair. - Bridge struct carries an Arc<TokenMinter> for the AAD path. - `resolve_auth(ctx)` is called BEFORE the chat / chat_stream future so AAD mint failures surface as direct Err returns (matches existing 4xx/timeout error semantics). - `build_request_headers` signature changed from `&str` to `&AzureAuth`; emits either `api-key:` (legacy) or `Authorization: Bearer` (AAD) based on which is set. - Added test-only `with_aad_token_endpoint_override` seam mirroring the existing `with_url_override` pattern. - Removed the now-unused `fn api_key()` helper (replaced by `AzureSecret::parse`). - 7 new tests: secret-parse (api-key / AAD / empty / bad JSON), end-to-end chat with AAD bearer header set, cache reuse across 3 chats, AAD 4xx surfaces before Azure call. - `lib.rs`: declares aad_token_mint module, ticks D6.6 in status block. `cargo test -p aisix-provider-azure-openai` → 53/53 PASS (+7). `cargo clippy -p aisix-provider-azure-openai --all-targets -- -D warnings` clean. `cargo fmt --all` applied. ## References (CLAUDE.md §7) - Microsoft identity platform — client credentials grant flow: https://learn.microsoft.com/en-us/entra/identity-platform/v2-oauth2-client-creds-grant-flow - Azure OpenAI Entra ID auth: https://learn.microsoft.com/en-us/azure/ai-services/openai/how-to/managed-identity - OAuth2 RFC 6749 §4.4 (client_credentials): https://www.rfc-editor.org/rfc/rfc6749#section-4.4 - Mirrors the audit-corrected pattern from `aisix-provider-vertex::token_mint` (ai-gateway#387). ## Unblocks AC.12 hardening in api7/AISIX-Cloud#302: Azure-OpenAI was already ~70% done (chat + stream + filter tolerance via #319); the AAD auth path was the explicit D6.6 gap called out in the audit. With this PR Phase F is complete. Live e2e against the Step 0.1 mock-llm Azure profile is the next sub-step (separate PR).
ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Free Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThis PR adds Azure Entra ID client_credentials token minting and caching, secret parsing to choose API-key vs AAD flows, credential validation, refactors header construction to emit ChangesAzure Entra ID Token Minting
Sequence DiagramsequenceDiagram
participant Client as Bridge Call-site
participant ResolveAuth as resolve_auth()
participant TokenMinter
participant Cache as In-Memory Cache
participant AzureTokenEndpoint as login.microsoftonline.com
participant AzureUpstream as Azure OpenAI Upstream
Client->>ResolveAuth: provider_key.secret
ResolveAuth->>ResolveAuth: parse AzureSecret (API key vs AAD JSON)
alt API-key
ResolveAuth-->>Client: AzureAuth { api_key }
Client->>AzureUpstream: request with header `api-key: ...`
else AAD
ResolveAuth->>TokenMinter: get_token(&AadCredentials)
TokenMinter->>Cache: lookup (tenant, client)
alt cached
Cache-->>TokenMinter: token
TokenMinter-->>ResolveAuth: access_token
else mint
TokenMinter->>AzureTokenEndpoint: POST client_credentials form
AzureTokenEndpoint-->>TokenMinter: {access_token, expires_in} / 4xx / 5xx
TokenMinter->>Cache: store token (on 2xx)
TokenMinter-->>ResolveAuth: access_token or BridgeError
end
ResolveAuth-->>Client: AzureAuth { bearer_token }
Client->>AzureUpstream: request with header `Authorization: Bearer ...`
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Note 🎁 Summarized by CodeRabbit FreeYour organization has reached its limit of developer seats under the Pro Plan. For new users, CodeRabbit will generate a high-level summary and a walkthrough for each pull request. For a comprehensive line-by-line review, please add seats to your subscription by visiting https://app.coderabbit.ai/login.If you believe this is a mistake and have available seats, please assign one to the pull request author through the subscription management page using the link above. Comment |
There was a problem hiding this comment.
Pull request overview
Adds Azure OpenAI Entra ID (AAD) client_credentials authentication to aisix-provider-azure-openai, alongside the existing resource api-key scheme, by autodetecting the auth mode from provider_key.secret and minting/caching Bearer tokens in-process when AAD credentials are provided.
Changes:
- Introduces
aad_token_mintmodule withAadCredentialsvalidation, token minting, and(tenant_id, client_id)-keyed cache. - Updates Azure bridge to parse/discriminate secrets, resolve auth early, and emit either
api-keyorAuthorization: Bearerheaders. - Adds unit tests for secret parsing, AAD minting behavior, caching, and error classification.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| crates/aisix-provider-azure-openai/src/lib.rs | Updates status docs and wires in the new aad_token_mint module. |
| crates/aisix-provider-azure-openai/src/bridge.rs | Adds secret parsing + per-request auth resolution; updates header construction to support Bearer auth; adds AAD-related tests. |
| crates/aisix-provider-azure-openai/src/aad_token_mint.rs | Implements AAD client-credentials token minting with cache, validation, and error classification + tests. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+314
to
+332
| let trimmed = secret.trim(); | ||
| if trimmed.is_empty() { | ||
| return Err(BridgeError::Config("provider_key.secret is empty".into())); | ||
| } | ||
| if trimmed.starts_with('{') { | ||
| let creds: crate::aad_token_mint::AadCredentials = serde_json::from_str(trimmed) | ||
| .map_err(|_e| { | ||
| BridgeError::Config( | ||
| "azure provider_key.secret looks JSON-shaped but failed to parse \ | ||
| as AAD client_credentials \ | ||
| {tenant_id, client_id, client_secret}" | ||
| .into(), | ||
| ) | ||
| })?; | ||
| creds.validate()?; | ||
| Ok(AzureSecret::Aad(creds)) | ||
| } else { | ||
| Ok(AzureSecret::ApiKey(trimmed.to_string())) | ||
| } |
Comment on lines
+94
to
+111
| for (name, value) in [ | ||
| ("tenant_id", &self.tenant_id), | ||
| ("client_id", &self.client_id), | ||
| ] { | ||
| if value.contains('/') | ||
| || value.contains('?') | ||
| || value.contains('#') | ||
| || value.contains(' ') | ||
| || value.contains('\t') | ||
| || value.contains('\n') | ||
| || value.contains("..") | ||
| { | ||
| return Err(BridgeError::Config(format!( | ||
| "azure aad credentials.{name} {value:?} contains URL-control \ | ||
| characters — reject `/`, `?`, `#`, whitespace, `..`" | ||
| ))); | ||
| } | ||
| } |
Comment on lines
+153
to
+156
| /// Test-only seam: replace the `login.microsoftonline.com` host | ||
| /// with this URL. Tenant id is still interpolated into the path | ||
| /// (so the request URL shape is verifiable end-to-end against | ||
| /// wiremock matchers). |
…t LOW on #388) audit-aigw-388-azure-aad flagged that chat_stream() calls the same resolve_auth helper as chat() but had no test pinning the AAD → Authorization: Bearer flow on the streaming path. A future refactor that accidentally skipped resolve_auth in chat_stream (e.g. moved auth resolution into the chat() future and forgot to mirror it on the stream side) would slip past every existing test. Mirrors the same gap noted in audit-aigw-387 (Vertex SA OAuth) which was deferred there as non-blocking; applying the equivalent guard here while the cost is one short test function. The test pins: - Authorization: Bearer <minted-token> set on the upstream stream request - api-key: NOT set (mutex with bearer path) - Accept: text/event-stream set (matches existing chat_stream contract regardless of auth scheme) cargo test -p aisix-provider-azure-openai → 54/54 PASS (was 53; +1).
Member
Author
Audit response — addressedIndependent audit-aigw-388-azure-aad returned APPROVE, no HIGH/MEDIUM findings. Audit-verified against Microsoft docs:
LOW addressed in code (commit `9fac7e5`)The single LOW finding was no chat_stream-side AAD test (same gap noted in audit-aigw-387 on Vertex). Added `chat_stream_with_aad_secret_sets_authorization_bearer_header` per the audit's suggested code — pins:
`cargo test -p aisix-provider-azure-openai` → 54/54 PASS (was 53; +1). All audit findings addressed. Awaiting fresh CI green. |
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.
Summary
Adds the second Azure auth scheme to `aisix-provider-azure-openai`. Today the bridge supports only the resource-key scheme (`api-key:` header). This PR adds AAD client_credentials (Entra ID), so an operator can configure a ProviderKey backed by a service-principal app registration instead of pasting the resource's master api-key.
Backward-compatible. Existing api-key deployments keep working unchanged. The auth scheme is autodetected from the secret shape:
Wire shape (AAD branch)
```
POST https://login.microsoftonline.com/{tenant_id}/oauth2/v2.0/token
Content-Type: application/x-www-form-urlencoded
grant_type=client_credentials
&client_id=
&client_secret=
&scope=https://cognitiveservices.azure.com/.default
```
Unlike Vertex SA OAuth (#387), AAD client_credentials is a straight form-encoded POST — NO JWT signing on the gateway side. No `jsonwebtoken` dep added; pure reqwest + serde.
Cache
Keyed by `(tenant_id, client_id)` — multiple ProviderKeys backed by the same AAD app share a slot, but distinct apps under the same tenant don't collide. Refresh 60s before upstream-reported expiry. Pattern mirrors Vertex `token_mint`.
Error classification (audit-aware)
Mirrors the audit MEDIUM fix from #387 right out of the gate — no need for a separate audit cycle:
Files
Test plan
References (CLAUDE.md §7)
Unblocks
AC.12 hardening in api7/AISIX-Cloud#302. Azure-OpenAI was ~70% done (chat + stream + filter tolerance via #319); the AAD auth path was the explicit D6.6 gap the audit called out. Phase F is complete after this PR. Live e2e against the Step 0.1 mock-llm Azure profile is the next sub-step (separate PR).
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Tests