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

[ACR] Always use access token for acr login #6211

Merged
merged 1 commit into from
Apr 26, 2018

Conversation

djyou
Copy link
Member

@djyou djyou commented Apr 25, 2018

Fixes #6131


This checklist is used to make sure that common guidelines for a pull request are followed.

  • The PR has modified HISTORY.rst describing any customer-facing, functional changes. Note that this does not include changes only to help content. (see Modifying change log).

  • I adhere to the Command Guidelines.

@promptws
Copy link

View a preview at https://prompt.ws/r/Azure/azure-cli/6211
This is an experimental preview for @microsoft users.

@djyou djyou changed the title [ACR] Always use access token for acr login [DO NOT MERGE][ACR] Always use access token for acr login Apr 25, 2018
@yugangw-msft
Copy link
Contributor

@djyou, this idea looks good to me. Please note, the token returned from get_raw_token will guarantee be valid for 5 minutes, so if it is long enough, then nothing to worry about.

@djyou djyou changed the title [DO NOT MERGE][ACR] Always use access token for acr login [ACR] Always use access token for acr login Apr 25, 2018
@derekbekoe derekbekoe requested a review from yugangw-msft April 25, 2018 23:36
@djyou
Copy link
Member Author

djyou commented Apr 25, 2018

@yugangw-msft Thank you for the review and offline discussions today.
@sajayantony From the discussions, it appeared that supporting refresh token with SP+Cert is possible but require some work on our side, but supporting refresh token with MSI doesn't seem technically possible.
/cc @ankurkhemani

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

Successfully merging this pull request may close these issues.

4 participants