[Profile] BREAKING CHANGE: az login: --password no longer accepts service principal certificate
#30092
Conversation
This file contains 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
️✔️AzureCLI-FullTest
|
️✔️AzureCLI-BreakingChangeTest
|
|
az login refinement |
jiasli
modified the milestones:
October 2024 (2024-11-05),
November 2024 (2024-11-19)❗ Breaking change release ❗
jiasli
force-pushed
the
sp-cert2
branch
3 times, most recently
from
400c6c9 to
c9328ce
Compare
jiasli
force-pushed
the
sp-cert2
branch
2 times, most recently
from
58e01df to
521ca74
Compare
jiasli
commented
Nov 1, 2024
| @@ -196,7 +192,7 @@ def login_with_service_principal(self, client_id, credential, scopes): | |||
| """ | |||
| sp_auth = ServicePrincipalAuth.build_from_credential(self.tenant_id, client_id, credential) | |||
| client_credential = sp_auth.get_msal_client_credential() | |||
| cca = ConfidentialClientApplication(client_id, client_credential, **self._msal_app_kwargs) | |||
| cca = ConfidentialClientApplication(client_id, client_credential=client_credential, **self._msal_app_kwargs) | |||
There was a problem hiding this comment.
ConfidentialClientApplication's client_credential is a keyword argument. #29439 passes a positional argument, breaking tests such as azure.cli.core.auth.tests.test_identity.TestIdentity.test_login_with_service_principal_certificate.
Also see Azure/azure-cli-dev-tools#318
jiasli
commented
Nov 7, 2024
| @@ -318,20 +318,13 @@ def build_credential(cls, secret_or_certificate=None, | |||
| } | |||
| """ | |||
| entry = {} | |||
| if certificate: | |||
| if client_secret: | |||
| logger.warning(PASSWORD_CERTIFICATE_WARNING) | |||
There was a problem hiding this comment.
Because the os.path.isfile() detection logic is removed, it is no longer possible to know if client_secret is a certificate, so the warning is now unconditionally shown when --password is used.
|
The warnings are kept but rephrased:
They will be removed after several releases. We followed this pattern during Microsoft Graph migration: |
evelyn-ys
previously approved these changes
Nov 8, 2024
evelyn-ys
approved these changes
Nov 8, 2024
jiasli
commented
Nov 8, 2024
Comment on lines
-271
to
-273
| # secret with '~', which is preserved as-is | ||
| cred = ServicePrincipalAuth.build_credential(secret_or_certificate="~test_secret") | ||
| assert cred == {"client_secret": "~test_secret"} |
There was a problem hiding this comment.
The secret is no longer expanded, so this check is meaningless.
bebound
approved these changes
Nov 8, 2024
bebound
reviewed
Nov 8, 2024
| # we let the user explicitly log in to AAD with MSAL. | ||
| "Please explicitly log in with:\n{}" if error.get('error') == 'broker_error' | ||
| else "Interactive authentication is needed. Please run:\n{}").format(login_command) | ||
| if error_codes and 7000215 in error_codes: |
There was a problem hiding this comment.
What's the full message of 7000215?
There was a problem hiding this comment.
It is in the PR description:
AADSTS7000215: Invalid client secret provided. Ensure the secret being sent in the request is the client secret value, not the client secret ID, for a secret added to app 'dd5baa90-0c41-4cab-a458-e8c33d3249bb'.
yonzhan
approved these changes
Nov 8, 2024
snguyen64
pushed a commit
to snguyen64/azure-cli
that referenced
this pull request
Nov 8, 2024
… service principal certificate (Azure#30092)
Merged
jiasli
added a commit
that referenced
this pull request
Dec 24, 2024
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.
Related command
Description
Fix #28839
Follow up work on #30091
--passwordno longer accepts service principal certificate.Testing Guide
History Notes
[Profile] BREAKING CHANGE:
az login:--passwordno longer accepts a service principal certificate. Use--certificateto pass a service principal certificate