-
Notifications
You must be signed in to change notification settings - Fork 3.3k
Identity use pbyte #11173
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
Identity use pbyte #11173
Changes from 3 commits
eeba9f4
1d2268f
d14e483
3ae7c73
cfb1d84
202ad08
705f796
21661bf
a6814d7
06b953b
25fb58d
8473cad
d981976
a3b02b9
a12402e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -7,9 +7,10 @@ | |
| from .._exceptions import CredentialUnavailableError | ||
| from .._constants import AZURE_VSCODE_CLIENT_ID | ||
| from .._internal.aad_client import AadClient | ||
| if sys.platform.startswith('win'): | ||
|
|
||
| if sys.platform.startswith("win"): | ||
| from .win_vscode_adapter import get_credentials | ||
| elif sys.platform.startswith('darwin'): | ||
| elif sys.platform.startswith("darwin"): | ||
| from .macos_vscode_adapter import get_credentials | ||
| else: | ||
| from .linux_vscode_adapter import get_credentials | ||
|
|
@@ -24,6 +25,7 @@ class VSCodeCredential(object): | |
| """Authenticates by redeeming a refresh token previously saved by VS Code | ||
|
|
||
| """ | ||
|
|
||
| def __init__(self, **kwargs): | ||
| self._client = kwargs.pop("_client", None) or AadClient("organizations", AZURE_VSCODE_CLIENT_ID, **kwargs) | ||
|
|
||
|
|
@@ -45,9 +47,8 @@ def get_token(self, *scopes, **kwargs): | |
|
|
||
| refresh_token = get_credentials() | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Does this get the refresh token from the protected store every time? Perhaps we should only retrieve from the store once?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think the question is will this change? e.g. if user logs out on VS code, what's our expected behavior? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think the behavior should be that once we authenticate we continue to use that account even if the VS code user changes during the execution of the process, any other behavior is too hard to reason about. That being said this is also an issue in the .NET implementation now, so if we can't fix it today, we can create an issue to ensure we fix it by GA.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Updated.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. To be clear, this update doesn't implement the behavior Scott described. The credential can still change refresh token and thus identity. To behave as described, it would need to read the refresh token exactly once. |
||
| if not refresh_token: | ||
| raise CredentialUnavailableError( | ||
| message="No Azure user is logged in to Visual Studio Code." | ||
| ) | ||
| token = self._client.get_cached_access_token(scopes) \ | ||
| or self._client.obtain_token_by_refresh_token(refresh_token, scopes, **kwargs) | ||
| raise CredentialUnavailableError(message="No Azure user is logged in to Visual Studio Code.") | ||
| token = self._client.get_cached_access_token(scopes) or self._client.obtain_token_by_refresh_token( | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. when calling
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sorry, you mean cache refresh token or access token? access token is already cached in get_cached_access_token |
||
| refresh_token, scopes, **kwargs | ||
| ) | ||
| return token | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This can be more helpful: what does
VSCodeCredentialdo? How do I use it? Also, "#10840" can link to something--I suggest #10472 as a better target.