From 6a1c3ca773310263a8a1bf48a370e84e6127b471 Mon Sep 17 00:00:00 2001 From: Jiashuo Li <4003950+jiasli@users.noreply.github.com> Date: Fri, 8 Nov 2024 18:23:45 +0800 Subject: [PATCH] [Profile] BREAKING CHANGE: `az login`: `--password` no longer accepts service principal certificate (#30092) --- .../azure/cli/core/auth/identity.py | 22 ++++------------- .../cli/core/auth/tests/test_identity.py | 16 +++---------- .../azure/cli/core/auth/util.py | 24 +++++++++++++------ .../cli/command_modules/profile/_help.py | 4 ++-- .../cli/command_modules/profile/custom.py | 2 +- 5 files changed, 28 insertions(+), 40 deletions(-) diff --git a/src/azure-cli-core/azure/cli/core/auth/identity.py b/src/azure-cli-core/azure/cli/core/auth/identity.py index d9f4e5a2027..69f853a4a36 100644 --- a/src/azure-cli-core/azure/cli/core/auth/identity.py +++ b/src/azure-cli-core/azure/cli/core/auth/identity.py @@ -37,10 +37,6 @@ "Select the account you want to log in with. " "For more information on login with Azure CLI, see https://go.microsoft.com/fwlink/?linkid=2271136") -PASSWORD_CERTIFICATE_WARNING = ( - "Passing the service principal certificate with `--password` is deprecated and will be removed " - "by version 2.74. Please use `--certificate` instead.") - logger = get_logger(__name__) @@ -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) result = cca.acquire_token_for_client(scopes) check_result(result) @@ -307,7 +303,7 @@ def build_from_credential(cls, tenant_id, client_id, credential): return ServicePrincipalAuth(entry) @classmethod - def build_credential(cls, secret_or_certificate=None, + def build_credential(cls, client_secret=None, certificate=None, use_cert_sn_issuer=None, client_assertion=None): """Build credential from user input. The credential looks like below, but only one key can exist. @@ -318,20 +314,12 @@ def build_credential(cls, secret_or_certificate=None, } """ entry = {} - if certificate: + if client_secret: + entry[_CLIENT_SECRET] = client_secret + elif certificate: entry[_CERTIFICATE] = os.path.expanduser(certificate) if use_cert_sn_issuer: entry[_USE_CERT_SN_ISSUER] = use_cert_sn_issuer - elif secret_or_certificate: - # TODO: Make secret_or_certificate secret only - user_expanded = os.path.expanduser(secret_or_certificate) - if os.path.isfile(user_expanded): - logger.warning(PASSWORD_CERTIFICATE_WARNING) - entry[_CERTIFICATE] = user_expanded - if use_cert_sn_issuer: - entry[_USE_CERT_SN_ISSUER] = use_cert_sn_issuer - else: - entry[_CLIENT_SECRET] = secret_or_certificate elif client_assertion: entry[_CLIENT_ASSERTION] = client_assertion return entry diff --git a/src/azure-cli-core/azure/cli/core/auth/tests/test_identity.py b/src/azure-cli-core/azure/cli/core/auth/tests/test_identity.py index 677de98ef29..993039faca3 100644 --- a/src/azure-cli-core/azure/cli/core/auth/tests/test_identity.py +++ b/src/azure-cli-core/azure/cli/core/auth/tests/test_identity.py @@ -264,20 +264,10 @@ def test_service_principal_auth_client_assertion(self): assert client_credential == {'client_assertion': 'test_jwt'} def test_build_credential(self): - # secret - cred = ServicePrincipalAuth.build_credential(secret_or_certificate="test_secret") + # client_secret + cred = ServicePrincipalAuth.build_credential(client_secret="test_secret") assert cred == {"client_secret": "test_secret"} - # secret with '~', which is preserved as-is - cred = ServicePrincipalAuth.build_credential(secret_or_certificate="~test_secret") - assert cred == {"client_secret": "~test_secret"} - - # certificate as password (deprecated) - current_dir = os.path.dirname(os.path.realpath(__file__)) - test_cert_file = os.path.join(current_dir, 'sp_cert.pem') - cred = ServicePrincipalAuth.build_credential(secret_or_certificate=test_cert_file) - assert cred == {'certificate': test_cert_file} - # certificate current_dir = os.path.dirname(os.path.realpath(__file__)) test_cert_file = os.path.join(current_dir, 'sp_cert.pem') @@ -297,7 +287,7 @@ def test_build_credential(self): cred = ServicePrincipalAuth.build_credential(certificate=test_cert_file, use_cert_sn_issuer=True) assert cred == {'certificate': test_cert_file, 'use_cert_sn_issuer': True} - # client assertion + # client_assertion cred = ServicePrincipalAuth.build_credential(client_assertion="test_jwt") assert cred == {"client_assertion": "test_jwt"} diff --git a/src/azure-cli-core/azure/cli/core/auth/util.py b/src/azure-cli-core/azure/cli/core/auth/util.py index 61e91b27ca1..a89500ef9ce 100644 --- a/src/azure-cli-core/azure/cli/core/auth/util.py +++ b/src/azure-cli-core/azure/cli/core/auth/util.py @@ -14,6 +14,12 @@ AccessToken = namedtuple("AccessToken", ["token", "expires_on"]) +PASSWORD_CERTIFICATE_WARNING = ( + "The error may be caused by passing a service principal certificate with --password. " + "Please note that --password no longer accepts a service principal certificate. " + "To pass a service principal certificate, use --certificate instead.") + + def aad_error_handler(error, **kwargs): """ Handle the error from AAD server returned by ADAL or MSAL. """ @@ -30,17 +36,21 @@ def aad_error_handler(error, **kwargs): "below, please mention the hostname '%s'", socket.gethostname()) error_description = error.get('error_description') + error_codes = error.get('error_codes') # Build recommendation message - login_command = _generate_login_command(**kwargs) - login_message = ( - # Cloud Shell uses IMDS-like interface for implicit login. If getting token/cert failed, - # 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: + recommendation = PASSWORD_CERTIFICATE_WARNING + else: + login_command = _generate_login_command(**kwargs) + recommendation = ( + # Cloud Shell uses IMDS-like interface for implicit login. If getting token/cert failed, + # 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) from azure.cli.core.azclierror import AuthenticationError - raise AuthenticationError(error_description, msal_error=error, recommendation=login_message) + raise AuthenticationError(error_description, msal_error=error, recommendation=recommendation) def _generate_login_command(scopes=None, claims=None): diff --git a/src/azure-cli/azure/cli/command_modules/profile/_help.py b/src/azure-cli/azure/cli/command_modules/profile/_help.py index 0b71bac93b6..d3f5c3b9abe 100644 --- a/src/azure-cli/azure/cli/command_modules/profile/_help.py +++ b/src/azure-cli/azure/cli/command_modules/profile/_help.py @@ -21,8 +21,8 @@ For more details, see https://go.microsoft.com/fwlink/?linkid=2276314 - [WARNING] Passing the service principal certificate with `--password` is deprecated and will be removed - by version 2.74. Please use `--certificate` instead. + [WARNING] `--password` no longer accepts a service principal certificate. + Use `--certificate` to pass a service principal certificate. To log in with a service principal, specify --service-principal. diff --git a/src/azure-cli/azure/cli/command_modules/profile/custom.py b/src/azure-cli/azure/cli/command_modules/profile/custom.py index 07db5ac2d04..89416732a73 100644 --- a/src/azure-cli/azure/cli/command_modules/profile/custom.py +++ b/src/azure-cli/azure/cli/command_modules/profile/custom.py @@ -159,7 +159,7 @@ def login(cmd, username=None, password=None, tenant=None, scopes=None, allow_no_ if service_principal: from azure.cli.core.auth.identity import ServicePrincipalAuth password = ServicePrincipalAuth.build_credential( - secret_or_certificate=password, + client_secret=password, certificate=certificate, use_cert_sn_issuer=use_cert_sn_issuer, client_assertion=client_assertion)