Skip to content

FIX: Make auth mode an explicit choice instead of an inferred one - #3010

Open
hannahwestra25 wants to merge 11 commits into
microsoft:mainfrom
hannahwestra25:hannahwestra25-explicit-identity-auth
Open

hannahwestra25 wants to merge 11 commits into
microsoft:mainfrom
hannahwestra25:hannahwestra25-explicit-identity-auth

Conversation

@hannahwestra25

@hannahwestra25 hannahwestra25 commented Oct 6, 2026 •

Copy link
Copy Markdown
Contributor

Description

Selecting identity auth didn't force it. target_service signalled identity by deleting api_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.

resolve_openai_auth(endpoint="https://x.openai.azure.com/openai/v1", api_key=None,
                    api_key_environment_variable="OPENAI_CHAT_API_KEY")
# main -> 'sk-SECRET-FROM-DOTENV'; expected an Entra token provider

Fix: thread an explicit auth_mode into resolve_openai_auth, AzureMLChatTarget, PromptShieldTarget and target_service. auth_mode="identity" skips the key and env var, and rejects non-Azure endpoints. AuthMode moves to pyrit/common/auth_mode.py to 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_configuration and .env_example updated; jupytext in sync. CI green.

@richlundeen Richard Lundeen (richlundeen) left a comment •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Take a look at my comments. None are blockers but recommendations; happy to chat about any further!

Comment thread pyrit/backend/services/target_service.py Outdated
Comment thread pyrit/backend/services/target_service.py Outdated
Comment thread pyrit/auth/openai_auth.py Outdated
Copilot AI added 4 commits October 7, 2026 16:35
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
hannahwestra25 force-pushed the hannahwestra25-explicit-identity-auth branch from c312db1 to 9e51bbd Compare October 7, 2026 21:25
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>
hannahwestra25 and others added 2 commits October 8, 2026 14:56
…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>
@hannahwestra25 hannahwestra25 changed the title FIX: Make identity auth an explicit choice instead of an inferred one FIX: Make auth mode an explicit choice instead of an inferred one Oct 8, 2026
Copilot AI and others added 4 commits October 9, 2026 11:51
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>
@hannahwestra25
hannahwestra25 added this pull request to the merge queue Oct 9, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Oct 9, 2026
@hannahwestra25
hannahwestra25 added this pull request to the merge queue Oct 9, 2026
@hannahwestra25
hannahwestra25 removed this pull request from the merge queue due to a manual request Oct 9, 2026
@hannahwestra25
hannahwestra25 added this pull request to the merge queue Oct 9, 2026
@github-merge-queue
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

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants