Repository navigation
FIX: Make auth mode an explicit choice instead of an inferred one - #3010
Open
hannahwestra25 wants to merge 11 commits into
Open
hannahwestra25 wants to merge 11 commits into
hannahwestra25 wants to merge 11 commits into
Conversation
Richard Lundeen (richlundeen)
approved these changes
Oct 7, 2026
Selecting identity-based authentication was signalled by deleting the api_key, but "no api_key" is ambiguous: every auth resolver interprets it as "read the key from the environment variable", so an explicit identity choice was silently downgraded to api-key auth whenever the env var was set. Thread an explicit auth_mode through resolve_openai_auth, OpenAITarget, AzureMLChatTarget and PromptShieldTarget. auth_mode defaults to "api_key", so the existing callable -> explicit key -> env var -> Entra fallback chain is unchanged; only an explicit auth_mode="identity" short-circuits to a token provider. Identity still refuses to mint tokens for unrecognized hosts and now raises a clear ValueError instead. AuthMode moves to pyrit/common/auth_mode.py so pyrit.auth can reference it without depending on the target layer; it is re-exported from pyrit.prompt_target.common.prompt_target for backward compatibility. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
auth_mode is also a constructor parameter, so the registry accepted it inside params as a second, competing channel. A request with auth_mode="api_key" and params["auth_mode"]="identity" selected identity, silently ignoring a supplied api_key and bypassing the service's supported_auth_modes check; the opposite conflict was silently resolved in favor of the top-level value. Reject conflicting values and forward the request-level auth_mode for both modes so it is authoritative in either direction. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
The configuration guide described Entra auth only as the implicit fallback. Document the explicit mode, the resolution order it bypasses, and the targets that accept it. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
PR microsoft#2846 landed the AzureBlobStorageTarget half of this bug using a get_auth_mode_parameters classmethod. Adopt that hook as the single mechanism for carrying auth intent into target construction and drop the _accepts_auth_mode signature introspection, which only existed because AzureBlobStorageTarget lacked the parameter. Every target advertising identity support now overrides the hook, guarded by a registry-wide contract test so a future identity target cannot silently fall back to inferring auth from a missing key. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
hannahwestra25
force-pushed
the
hannahwestra25-explicit-identity-auth
branch
from
October 7, 2026 21:25
c312db1 to
9e51bbd
Compare
AUTH_MODES is typed tuple[AuthMode, ...], so the membership check already narrows auth_mode to AuthMode and the cast tripped ty's redundant-cast rule. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Richard Lundeen (richlundeen)
approved these changes
Oct 8, 2026
…icit Per review feedback, drop the "no key found and the endpoint looks like Azure, so mint a token" fallback from the OpenAI resolver and the two inlined copies in AzureMLChatTarget and PromptShieldTarget. That fallback is what made an explicit auth_mode="identity" indistinguishable from "no key configured", so the two paths could not be told apart. api_key mode now requires a key or an explicit token provider and fails with an error that names the env var, api_key, and auth_mode="identity". identity mode ignores keys entirely. Also threads auth_mode through OpenAITextEmbedding, which is a fourth consumer of resolve_openai_auth and would otherwise have lost its only ergonomic path to identity auth. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Three follow-ups to the explicit auth_mode change: 1. Reject identity + explicit api_key instead of silently discarding it. An api_key value may be a key string or a caller-supplied token provider callable. Dropping a provider replaced the caller's chosen identity with a bare DefaultAzureCredential at a hardcoded scope, which could succeed under a different principal. All three resolvers (openai_auth, AzureMLChatTarget, PromptShieldTarget) now raise ValueError when both are supplied. 2. Correct docstrings that still described the removed implicit Entra fallback, including the canonical AuthMode definition. 3. Deprecate rather than silently keep the AzureBlobStorageTarget DefaultAzureCredential fallback. Its auth_mode is tri-state and the backend API forwards auth_mode="api_key" by default, so the fallback stays functional but now emits a deprecation notice pointing at auth_mode="identity", scheduled for removal in 1.4.0. Also reworded the non-Azure error message, which previously suggested an auth mode that is not valid for those endpoints. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
.env_example promised that an unset key falls back to Entra ID automatically. This PR removes that fallback for the OpenAI targets, AzureMLChatTarget, PromptShieldTarget and OpenAITextEmbedding, so a user following the file (Azure endpoint, no key, az login) now gets a ValueError instead of a working target. This is the first configuration file new users copy, so it is the highest-traffic place the removed contract was documented. Point it at auth_mode="identity" and at the configuration guide so the two do not drift. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
The explicit auth_mode fix did not require deleting the implicit Entra fallback, and deleting it in three of the four sites while giving AzureBlobStorageTarget a 1.4.0 deprecation window was inconsistent with the release policy for functionality that was never deprecated. Restore the fallback in resolve_openai_auth, AzureMLChatTarget and PromptShieldTarget behind print_deprecation_message(removed_in="1.4.0"), matching the AzureBlobStorageTarget wording. The identity path is unchanged: it still returns before the callable check, the explicit key and the environment read, so an explicit auth_mode="identity" is never downgraded to an ambient key. This also restores keyless and empty-string-key configurations, which the integration pipeline and .env files hit in practice. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
github-merge-queue
Bot
removed this pull request from the merge queue due to failed status checks
Oct 9, 2026
github-merge-queue
Bot
removed this pull request from the merge queue due to failed status checks
Oct 9, 2026
This branch has not been deployed
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.
Description
Selecting identity auth didn't force it.
target_servicesignalled identity by deletingapi_key, but "no key" is ambiguous — every resolver read it as "use the env var", so an explicit choice was silently downgraded to whatever key sat in.env.Fix: thread an explicit
auth_modeintoresolve_openai_auth,AzureMLChatTarget,PromptShieldTargetandtarget_service.auth_mode="identity"skips the key and env var, and rejects non-Azure endpoints.AuthModemoves topyrit/common/auth_mode.pyto break an import cycle, re-exported from its old home.Not breaking: the implicit Entra fallback still works, now behind a
DeprecationWarning(removed in 1.4.0) at all four sites. Builds on #2846.Tests and Documentation
Regression tests on all four resolver paths plus
target_service; identity asserted warning-free.doc/code/setup/1_configurationand.env_exampleupdated; jupytext in sync. CI green.