Avoid conflicting scopes and commandIds in quiet logins from Accounts (fix #137601) #137613
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Contributor
Author
|
Repro available in my branch of authenticationprovider-sample which adds two quiet logins:
The two quiet logins are now accessible, but the screenshot reveals another issue, which is that the menu item prompts for the two different-scopes requests are identical. I will open a separate issue for this. |
Member
|
Couldn't we join them with a comma instead? JSON stringify is fine...but I don't want the usage to give off some impression that we deserialize the scopes later since we don't do that. |
Contributor
Author
|
Based on this section of the OAuth2 a scope could contain a comma. But space, doublequote and backslash aren't allowed in a scope. So I've pushed a change to use space as the separator, and to fix a path my original change hadn't accounted for. |
guibber
pushed a commit
to guibber/vscode
that referenced
this pull request
Nov 30, 2021
…fix microsoft#137601) (microsoft#137613) * Avoid conflicting scopes and commandIds in quiet logins from Accounts (fix microsoft#137601) * revert from scopesJSON to scopesList but use space as separator * define SCOPESLIST_SEPARATOR and use it consistently * simplify diff
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR fixes #137601
Internal structure now uses JSON-stringify form of scopes array in place of simple join.
Variable name changed to correspond.
commandId for generated login command now uses a naming scheme that will avoid clashes.
Two interfaces that aren't being used elsewhere are no longer exported.
/assign @TylerLeonhardt