Skip to content

Commit

Permalink
[Profile] BREAKING CHANGE: az login: --password no longer accepts…
Browse files Browse the repository at this point in the history
… service principal certificate (#30092)
  • Loading branch information
jiasli authored Nov 8, 2024
1 parent 707a930 commit 6a1c3ca
Show file tree
Hide file tree
Showing 5 changed files with 28 additions and 40 deletions.
22 changes: 5 additions & 17 deletions src/azure-cli-core/azure/cli/core/auth/identity.py
Original file line number Diff line number Diff line change
Expand Up @@ -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__)


Expand Down Expand Up @@ -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)

Expand Down Expand Up @@ -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.
Expand All @@ -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
Expand Down
16 changes: 3 additions & 13 deletions src/azure-cli-core/azure/cli/core/auth/tests/test_identity.py
Original file line number Diff line number Diff line change
Expand Up @@ -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')
Expand All @@ -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"}

Expand Down
24 changes: 17 additions & 7 deletions src/azure-cli-core/azure/cli/core/auth/util.py
Original file line number Diff line number Diff line change
Expand Up @@ -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. """

Expand All @@ -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):
Expand Down
4 changes: 2 additions & 2 deletions src/azure-cli/azure/cli/command_modules/profile/_help.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
2 changes: 1 addition & 1 deletion src/azure-cli/azure/cli/command_modules/profile/custom.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down

0 comments on commit 6a1c3ca

Please sign in to comment.