Better handle auth when not using built-in auth #5954
Conversation
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
wbreza
reviewed
Oct 20, 2025
…-auth-when-using-az-for-auth
vhvb1989
requested review from
JeffreyCA,
hemarina,
rajeshkamal5050,
tg-msft and
weikanglim
as code owners
Collaborator
Azure Dev CLI Install InstructionsInstall scriptsMacOS/Linux
bash: pwsh: WindowsPowerShell install MSI install Standalone Binary
MSI
Documentationlearn.microsoft.com documentationtitle: Azure Developer CLI reference
|
weikanglim
reviewed
Oct 23, 2025
weikanglim
requested changes
Oct 23, 2025
Contributor
weikanglim
left a comment
weikanglim
left a comment
There was a problem hiding this comment.
Current change blocks the functionality of using --check-status functionality to check if azd is delegating to az
microsoft-github-policy-service
bot
added
the
no-recent-activity
…-auth-when-using-az-for-auth
microsoft-github-policy-service
bot
removed
the
no-recent-activity
microsoft-github-policy-service
bot
added
the
no-recent-activity
…-auth-when-using-az-for-auth
microsoft-github-policy-service
bot
removed
the
no-recent-activity
Contributor
There was a problem hiding this comment.
Pull request overview
This PR enhances the authentication system to better handle scenarios where users are not using azd's built-in authentication. It addresses issue #5949 by introducing auth mode detection and allowing users to switch between authentication modes during the login flow.
Key changes:
- Adds
AuthModetype with three modes: built-in, delegated to az CLI, and external token request - Implements
Mode()method to detect the current authentication mode - Implements
SetBuiltInAuthMode()method to switch from az delegation to built-in mode - Updates login flow to warn users and prompt for mode switching when not in built-in mode
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| cli/azd/pkg/auth/manager.go | Adds AuthMode type and two new methods (Mode and SetBuiltInAuthMode) to detect and manage authentication modes |
| cli/azd/cmd/auth_login.go | Updates login action to detect auth mode and prompt users to switch to built-in mode if needed |
|
|
||
| // protecting against unexpected modes. There should be only azDelegated left. | ||
| if currentMode != AzDelegated { | ||
| return fmt.Errorf("Unexpected mode found: %s", currentMode) |
There was a problem hiding this comment.
Error message should start with a lowercase letter to follow Go error message conventions. Error messages in Go should not be capitalized (unless starting with proper nouns or acronyms) as they are often wrapped by other error messages.
Comment on lines
+1421
to
+1439
| func (m *Manager) Mode() (AuthMode, error) { | ||
| // Check external | ||
| if m.UseExternalAuth() { | ||
| return ExternalRequest, nil | ||
| } | ||
|
|
||
| // check az delegation | ||
| cfg, err := m.userConfigManager.Load() | ||
| if err != nil { | ||
| return "", fmt.Errorf("fetching current user: %w", err) | ||
| } | ||
|
|
||
| if shouldUseLegacyAuth(cfg) { | ||
| return AzDelegated, nil | ||
| } | ||
|
|
||
| // default to azd | ||
| return AzdBuiltIn, nil | ||
| } |
There was a problem hiding this comment.
The new Mode() method lacks test coverage. This method has multiple code paths (external auth, az delegation, and built-in) that should be tested to ensure the auth mode detection works correctly in all scenarios. Consider adding tests in manager_test.go to cover: 1) external auth mode detection when UseExternalAuth() returns true, 2) az delegation mode when useAzCliAuthKey is set, and 3) built-in mode as the default case.
Comment on lines
+1441
to
+1475
| func (m *Manager) SetBuiltInAuthMode() error { | ||
| currentMode, err := m.Mode() | ||
| if err != nil { | ||
| return fmt.Errorf("fetching current auth mode: %w", err) | ||
| } | ||
| if currentMode == AzdBuiltIn { | ||
| return nil | ||
| } | ||
|
|
||
| if currentMode == ExternalRequest { | ||
| return fmt.Errorf("cannot change auth mode when external token mode is set. See %s", | ||
| "https://github.com/Azure/azure-dev/blob/main/cli/azd/docs/external-authentication.md") | ||
| } | ||
|
|
||
| // protecting against unexpected modes. There should be only azDelegated left. | ||
| if currentMode != AzDelegated { | ||
| return fmt.Errorf("Unexpected mode found: %s", currentMode) | ||
| } | ||
|
|
||
| // Unset the useAzCliAuthKey flag | ||
| cfg, err := m.userConfigManager.Load() | ||
| if err != nil { | ||
| return fmt.Errorf("reading user config: %w", err) | ||
| } | ||
|
|
||
| if err := cfg.Unset(useAzCliAuthKey); err != nil { | ||
| return fmt.Errorf("unsetting %s: %w", useAzCliAuthKey, err) | ||
| } | ||
|
|
||
| if err := m.userConfigManager.Save(cfg); err != nil { | ||
| return fmt.Errorf("saving user config: %w", err) | ||
| } | ||
|
|
||
| return nil | ||
| } |
There was a problem hiding this comment.
The new SetBuiltInAuthMode() method lacks test coverage. This method has several important code paths that should be tested: 1) no-op when already in built-in mode, 2) error when in external request mode, 3) successful switch from az delegation mode to built-in mode (unsetting the useAzCliAuthKey flag), and 4) proper error handling when loading/saving config fails. Consider adding tests in manager_test.go similar to TestLegacyAzCliCredentialSupport to verify the config changes are correctly applied.
| Message: "Do you want to switch back to azd built-in authentication?", | ||
| DefaultValue: false, | ||
| Help: "Azd supports multiple authentication modes, including Azure CLI authentication and External " + | ||
| "request for Auth. Switching back to azd built-in authentication will try to disable the current mode.", |
There was a problem hiding this comment.
The word "Auth" should be lowercase "auth" for consistency with the rest of the message. The capitalization of "Auth" here appears to be unintentional as it's not a proper noun or acronym in this context.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
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.
fix: #5949