[Profile] az login: Add --certificate for authenticating with service principal certificate#30091
[Profile] az login: Add --certificate for authenticating with service principal certificate#30091
az login: Add --certificate for authenticating with service principal certificate#30091Conversation
️✔️AzureCLI-FullTest
|
|
| rule | cmd_name | rule_message | suggest_message |
|---|---|---|---|
| login | cmd login added parameter certificate |
|
az login refinement |
| # Service principal | ||
| c.argument('service_principal', action='store_true', | ||
| help='Log in with a service principal.') | ||
| c.argument('certificate', help='A PEM file with key and public certificate.') |
There was a problem hiding this comment.
I am hesitating on whether --certificate should have an alias -c. Using full name is definitely preferred.
There was a problem hiding this comment.
We can add a parameter in the future, let's keep --certificate for now.
| PASSWORD_CERTIFICATE_WARNING = ( | ||
| "Using --password to pass service principal certificate is deprecated and will be removed in a " | ||
| "future release. Use --certificate instead.") |
There was a problem hiding this comment.
Please kindly rephase the warning message. @dcaro @dbradish-microsoft
There was a problem hiding this comment.
| PASSWORD_CERTIFICATE_WARNING = ( | |
| "Using --password to pass service principal certificate is deprecated and will be removed in a " | |
| "future release. Use --certificate instead.") | |
| PASSWORD_CERTIFICATE_WARNING = ( | |
| "Passing the service principal certificate with `--password` is deprecated and will be removed in a future release. Please use `--certificate` instead.") |
There was a problem hiding this comment.
Updated. Shall we be explicit on "a future release"?
There was a problem hiding this comment.
Since the next breaking change release is very close, shall we give customers more time to see this warning message?
There was a problem hiding this comment.
Yes, let's give sufficient time for customers to notice the warning message
| # Service principal | ||
| c.argument('service_principal', action='store_true', | ||
| help='Log in with a service principal.') | ||
| c.argument('certificate', help='A PEM file with key and public certificate.') |
There was a problem hiding this comment.
We can add a parameter in the future, let's keep --certificate for now.
| PASSWORD_CERTIFICATE_WARNING = ( | ||
| "Using --password to pass service principal certificate is deprecated and will be removed in a " | ||
| "future release. Use --certificate instead.") |
There was a problem hiding this comment.
| PASSWORD_CERTIFICATE_WARNING = ( | |
| "Using --password to pass service principal certificate is deprecated and will be removed in a " | |
| "future release. Use --certificate instead.") | |
| PASSWORD_CERTIFICATE_WARNING = ( | |
| "Passing the service principal certificate with `--password` is deprecated and will be removed in a future release. Please use `--certificate` instead.") |
|
|
||
| if username: | ||
| if not (password or client_assertion): | ||
| if not (password or client_assertion or certificate): |
There was a problem hiding this comment.
Shall we also add certificate check in
azure-cli/src/azure-cli/azure/cli/command_modules/profile/custom.py
Lines 123 to 126 in 6c32c4d
There was a problem hiding this comment.
No, as missing all 3 types of credentials will result in prompting for secrets.
There was a problem hiding this comment.
I mean, if user pass in --certificate together with --identity/--use-device-code, we should raise error as well, right?
There was a problem hiding this comment.
--certificate will be discarded in that case. As you can see, client_assertion is not checked either. We can do that in a separate PR.
|
|
||
| if service_principal: | ||
| from azure.cli.core.auth.identity import ServicePrincipalAuth | ||
| password = ServicePrincipalAuth.build_credential(password, client_assertion, use_cert_sn_issuer) |
There was a problem hiding this comment.
Passing keyword arguments as positional arguments is fragile and may break unexpectedly.
There was a problem hiding this comment.
Agree, I am not a big fan of positional arguments in general.
|
That should be also updates for this |
6 participants
Related command
az loginDescription
Fix #28839
Require #30090
--certificatefor authenticating with service principal certificate--passwordto pass service principal certificateTesting Guide
History Notes
[Profile]
az login: Passing the service principal certificate with--passwordis deprecated and will be removed in version 2.67.0. Please use--certificateinstead.