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

Use access_token grant instead of refresh_token #2153

Merged
merged 3 commits into from
Jul 14, 2020
Merged

Conversation

bwateratmsft
Copy link
Collaborator

@bwateratmsft bwateratmsft commented Jul 13, 2020

This substantially mitigates #1959 or possibly eliminates the issue entirely. Changing the ADAL token <=> ACR token exchange's grant_type to access_token, and not passing in the ADAL refresh_token, dramatically reduces the size of the obtained ACR refresh_token. My token's size was pretty consistently about 950 bytes, compared to > 2600 before.

It may be functionally impossible to get a token using this different mechanism that is too long for Docker CLI. We won't update people's existing Docker configs (at least not now...), but we hopefully will never need to suggest disabling Wincred again.

The disadvantage to this grant type is that if the ADAL access_token is near expiry, they (ACR) will not be able to use the ADAL refresh_token to get a new one--however, that shouldn't be an issue for us since we get a fresh ADAL token from the Azure Account extension every time.

@bwateratmsft bwateratmsft added this to the 1.4.0 milestone Jul 13, 2020
@bwateratmsft bwateratmsft requested a review from karolz-ms July 13, 2020 20:27
@bwateratmsft bwateratmsft requested a review from a team as a code owner July 13, 2020 20:27
Copy link
Contributor

@karolz-ms karolz-ms left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please address the comment before merging, otherwise LGTM

src/commands/context/aci/createAciContext.ts Outdated Show resolved Hide resolved
@bwateratmsft bwateratmsft merged commit a6150dd into master Jul 14, 2020
@bwateratmsft bwateratmsft deleted the bmw/token branch July 14, 2020 00:31
Dmarch28 pushed a commit to Dmarch28/vscode-docker that referenced this pull request Mar 4, 2021
@microsoft microsoft locked and limited conversation to collaborators Oct 27, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants