Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

{Role} Keep --sdk-auth, for now #19872

Merged
merged 2 commits into from
Oct 15, 2021
Merged

{Role} Keep --sdk-auth, for now #19872

merged 2 commits into from
Oct 15, 2021

Conversation

jiasli
Copy link
Member

@jiasli jiasli commented Oct 13, 2021

Require #19853

Context

Even though we have announced the deprecation of --sdk-auth in #19414 since Python SDK has deprecated the usage of the output JSON file, there are still other places when this JSON file is used, like

  1. GitHub Actions (https://github.com/Azure/github/issues/104#issuecomment-941920571): https://github.com/Azure/login/blob/master/README.md#configure-deployment-credentials:
  2. https://github.com/MicrosoftDocs/azure-devops-docs/blob/master/docs/pipelines/targets/azure-stack.md#create-or-validate-your-spn
  3. https://github.com/MicrosoftDocs/azure-docs/blob/master/articles/azure-netapp-files/azacsnap-installation.md#azure-netapp-files-with-virtual-machine
  4. https://github.com/MicrosoftDocs/azure-stack-docs/blob/master/azure-stack/user/ci-cd-github-action-webapp.md#get-service-principal

Change

Thus the deprecation is delayed and --sdk-auth is brought back for now.

Security concerns

az account show --sdk-auth compromises the security of MSAL's encrypted service principal credential store, as it can "spit out" / echo the original service principal secrets. This is not a secure behavior - consider you can see your password after you login to Windows or https://outlook.live.com/.

Testing guide

  • az ad sp create-for-rbac --sdk-auth
  • az account show --sdk-auth

References

@yonzhan
Copy link
Collaborator

yonzhan commented Oct 13, 2021

Bring back --sdk-auth

@yonzhan yonzhan added this to the Oct 2021 (2021-11-02) milestone Oct 13, 2021
@jiasli jiasli added the MSAL label Oct 13, 2021
@jiasli jiasli requested a review from yonzhan October 15, 2021 06:14
@jiasli jiasli changed the title {Role} Bring back --sdk-auth, for now {Role} Keep --sdk-auth, for now Oct 15, 2021
@jiasli
Copy link
Member Author

jiasli commented Feb 7, 2022

The output of --sdk-auth is also used by Application Gateway Ingress Controller (AGIC):

https://docs.microsoft.com/en-us/azure/application-gateway/ingress-controller-install-existing#using-a-service-principal

@galiacheng
Copy link

Thank you @jiasli Glad to see --sdk-auth back! Java EE on Azure team is using sdk-auth format service principal to install AGIC for Azure WebLogic on AKS offer.

@jiasli
Copy link
Member Author

jiasli commented Feb 7, 2022

Migration

With --sdk-auth option vs without it, the JSON keys are different (e.g. --sdk-auth gives clientId while without it, you get appId).

For tools that consume the output of --sdk-auth, you may translate the JSON output of az ad sp create-for-rbac to match az ad sp create-for-rbac --sdk-auth:

  • appId -> clientId
  • password -> clientSecret
  • tenant -> tenantId
  • add subscriptionId

Without --sdk-auth:

> az ad sp create-for-rbac

{
  "appId": "21ec2946-231c-480f-86c7-824b215326a4",
  "displayName": "azure-cli-2022-02-07-07-07-00",
  "password": "...",
  "tenant": "54826b22-38d6-4fb2-bad9-b7b93a3e9c5a"
}

With --sdk-auth:

> az ad sp create-for-rbac --sdk-auth
{
  "clientId": "<appId>",
  "clientSecret": "<password>",
  "subscriptionId": "...",
  "tenantId": "<tenant>",
  "activeDirectoryEndpointUrl": "https://login.microsoftonline.com",
  "resourceManagerEndpointUrl": "https://management.azure.com/",
  "activeDirectoryGraphResourceId": "https://graph.windows.net/",
  "sqlManagementEndpointUrl": "https://management.core.windows.net:8443/",
  "galleryEndpointUrl": "https://gallery.azure.com/",
  "managementEndpointUrl": "https://management.core.windows.net/"
}

Originally posted by @erik-ha-msft in Azure/aks-set-context#40 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants