Skip to content
Merged
Show file tree
Hide file tree
Changes from 71 commits
Commits
Show all changes
82 commits
Select commit Hold shift + click to select a range
0002805
identity_win_vscode_credential
xiangyan99 Apr 14, 2020
b1e79de
remove used import
xiangyan99 Apr 14, 2020
13fd676
update
xiangyan99 Apr 15, 2020
d356ba6
update
xiangyan99 Apr 15, 2020
41b0394
update
xiangyan99 Apr 15, 2020
526470d
update
xiangyan99 Apr 15, 2020
dc671f2
update
xiangyan99 Apr 15, 2020
5a47894
update shared requirements
xiangyan99 Apr 15, 2020
c797482
add async credential and tests
xiangyan99 Apr 15, 2020
25de404
update dev_requirements
xiangyan99 Apr 15, 2020
f48fdd4
add try in import
xiangyan99 Apr 15, 2020
a77e336
not raise
xiangyan99 Apr 15, 2020
9f5ad5d
update
xiangyan99 Apr 15, 2020
d155f80
mac os support
xiangyan99 Apr 15, 2020
015f1f7
update
xiangyan99 Apr 15, 2020
bfa2ea1
update msal version
xiangyan99 Apr 15, 2020
16873bd
roll back msal change
xiangyan99 Apr 15, 2020
58844b9
remove dependency on pywin32
xiangyan99 Apr 16, 2020
05694a9
update
xiangyan99 Apr 16, 2020
671d5ea
update
xiangyan99 Apr 16, 2020
921b7a2
update
xiangyan99 Apr 16, 2020
3911b88
add tests
xiangyan99 Apr 16, 2020
f313ba4
add pygobject dependency
xiangyan99 Apr 16, 2020
283a055
linux support
xiangyan99 Apr 16, 2020
9974361
update
xiangyan99 Apr 16, 2020
4e7b9c4
updates
xiangyan99 Apr 16, 2020
bbe7a20
updates
xiangyan99 Apr 16, 2020
94b152b
pylint fix
xiangyan99 Apr 16, 2020
657617c
updates
xiangyan99 Apr 17, 2020
426dea7
updates
xiangyan99 Apr 17, 2020
f9a2abf
updates
xiangyan99 Apr 17, 2020
593a62f
update tests
xiangyan99 Apr 17, 2020
0805e74
format
xiangyan99 Apr 17, 2020
eaf10cb
add type checking
xiangyan99 Apr 17, 2020
e1dab12
refactor code
xiangyan99 Apr 17, 2020
9c33cfd
remove unused import
xiangyan99 Apr 17, 2020
d072e20
updates
xiangyan99 Apr 17, 2020
6024a9a
remove pygobject dependency
xiangyan99 Apr 17, 2020
a63e157
updates
xiangyan99 Apr 18, 2020
a8d45c0
typo
xiangyan99 Apr 18, 2020
2fca38a
update tests
xiangyan99 Apr 18, 2020
3866a93
update mac tests
xiangyan99 Apr 18, 2020
ebb5df2
add linux tests
xiangyan99 Apr 18, 2020
595cdea
updates
xiangyan99 Apr 18, 2020
246ab2d
updates
xiangyan99 Apr 18, 2020
af482e6
clean up
xiangyan99 Apr 18, 2020
57953ba
mock patch not work well on async 3.8+
xiangyan99 Apr 18, 2020
022f376
refactor code
xiangyan99 Apr 21, 2020
7e15ddd
pylint
xiangyan99 Apr 21, 2020
975f6ed
pylint
xiangyan99 Apr 21, 2020
767906f
add tests for win apis
xiangyan99 Apr 22, 2020
1e10649
use __module__
xiangyan99 Apr 22, 2020
cf54345
update
xiangyan99 Apr 22, 2020
e43a74f
update
xiangyan99 Apr 22, 2020
4fe1be3
add tests
xiangyan99 Apr 22, 2020
0fd89e6
typo
xiangyan99 Apr 22, 2020
5524ea9
update
xiangyan99 Apr 22, 2020
ff9d532
update
xiangyan99 Apr 22, 2020
e6b83ca
update
xiangyan99 Apr 22, 2020
9232a3b
Update sdk/identity/azure-identity/tests/test_vscode_credential.py
xiangyan99 Apr 23, 2020
9ea845e
updates
xiangyan99 Apr 23, 2020
77f8838
update
xiangyan99 Apr 23, 2020
0f69230
update
xiangyan99 Apr 23, 2020
ec1f0a3
update
xiangyan99 Apr 27, 2020
4fadaf3
update
xiangyan99 Apr 27, 2020
435fdce
update
xiangyan99 Apr 27, 2020
cb74ce2
updates
xiangyan99 Apr 28, 2020
a402f3b
update
xiangyan99 Apr 28, 2020
853dd3e
update
xiangyan99 Apr 28, 2020
54d1b48
update
xiangyan99 Apr 29, 2020
75624b8
disable on 2.7
xiangyan99 Apr 29, 2020
ed1db0c
raise on Python 2.7 Linux
xiangyan99 Apr 29, 2020
6fc8708
fix typo
xiangyan99 Apr 29, 2020
7d36ad5
update
xiangyan99 Apr 29, 2020
6f8ae61
update
xiangyan99 Apr 30, 2020
42270b8
check error
xiangyan99 Apr 30, 2020
5c8bcec
update
xiangyan99 Apr 30, 2020
77cc4d8
add vs code credential to default
xiangyan99 Apr 30, 2020
10a9e43
Merge branch 'master' into identity_vcode_credential
xiangyan99 Apr 30, 2020
ff8e1c7
update
xiangyan99 Apr 30, 2020
8efee5c
update
xiangyan99 Apr 30, 2020
a5dc253
updates
xiangyan99 Apr 30, 2020
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion sdk/identity/azure-identity/azure/identity/_constants.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,8 @@


AZURE_CLI_CLIENT_ID = "04b07795-8ddb-461a-bbee-02f9e1bf7b46"

AZURE_VSCODE_CLIENT_ID = "aebc6443-996d-45c2-90f0-388ff96faa56"
VSCODE_CREDENTIALS_SECTION = "VS Code Azure"

class KnownAuthorities:
AZURE_CHINA = "login.chinacloudapi.cn"
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
# ------------------------------------
# Copyright (c) Microsoft Corporation.
# Licensed under the MIT License.
# ------------------------------------
import os
import json
import ctypes as ct
from .._constants import VSCODE_CREDENTIALS_SECTION


def _c_str(string):
return ct.c_char_p(string.encode('utf-8'))
Copy link
Member

Choose a reason for hiding this comment

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

It seems this is used for service name and account name - service name is probably okay, but is there any change that an account name might not be utf8?

Copy link
Member Author

Choose a reason for hiding this comment

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

This is a good question and I don't have an answer. I opened #11135 to track it.

Copy link
Member

Choose a reason for hiding this comment

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

I assume that would be utf8

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes. I discussed with Anna offline. We assume this is utf8. We will try if we can find a way to test it.



try:
_libsecret = ct.cdll.LoadLibrary('libsecret-1.so.0')
Copy link
Member

Choose a reason for hiding this comment

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

Do we want a message if the lib is not available, to do something like sudo apt-get install libsecret-1-0 or something?

Copy link
Member Author

Choose a reason for hiding this comment

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

In fact, this credential is usually used in DefaultCredential which is a chain of credentials. If one does not work, it will try next one. So if libsecret-1-0 is not installed, we prefer just skip this one and go to next credential rather than failing.

Copy link
Member Author

Choose a reason for hiding this comment

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

In other words, if libsecret-1-0 is not installed, the feature is disabled silently.

Copy link
Member

Choose a reason for hiding this comment

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

Re-thinking this, I guess if they have vscode the lib is available, or maybe vscode embed it to save their credentials?

Copy link
Member Author

Choose a reason for hiding this comment

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

Azure connectors (e.g. azure-account, azure-storage, etc.) are vs code extensions. User can use VS code w/o them. Given this case, user uses vs code w/o Azure extensions. (and libsecret-1-0 is not installed). We should just not do anything, right?

Copy link
Member

Choose a reason for hiding this comment

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

I got my answer:

$ dpkg -I /mnt/c/Users/lmazuel/Downloads/code_1.44.2-1587059832_amd64.deb
 new Debian package, version 2.0.
 size 62335842 bytes: control archive=2325 bytes.
     705 bytes,    14 lines      control
    2909 bytes,    73 lines   *  postinst             #!/usr/bin/env
     189 bytes,     5 lines   *  postrm               #!/bin/bash
     274 bytes,     6 lines   *  prerm                #!/usr/bin/env
 Package: code
 Version: 1.44.2-1587059832
 Section: devel
 Depends: libnotify4, libnss3 (>= 2:3.26), gnupg, apt, libxkbfile1, libsecret-1-0, libgtk-3-0 (>= 3.10.0), libxss1
 Priority: optional
 Architecture: amd64
 Maintainer: Microsoft Corporation <[email protected]>
 Homepage: https://code.visualstudio.com/
 Installed-Size: 259118
 Provides: visual-studio-code
 Conflicts: visual-studio-code
 Replaces: visual-studio-code
 Description: Code editing. Redefined.
  Visual Studio Code is a new choice of tool that combines the simplicity of a code editor with what developers need for the core edit-build-debug cycle. See https://code.visualstudio.com/docs/setup/linux for installation instructions and FAQ.

This means libsecret will be installed if you have vscode

_libsecret.secret_schema_new.argtypes = \
[ct.c_char_p, ct.c_uint, ct.c_char_p, ct.c_uint, ct.c_char_p, ct.c_uint, ct.c_void_p]
_libsecret.secret_password_lookup_sync.argtypes = \
[ct.c_void_p, ct.c_void_p, ct.c_void_p, ct.c_char_p, ct.c_char_p, ct.c_char_p, ct.c_char_p, ct.c_void_p]
except OSError:
_libsecret = None


def _get_user_settings_path():
app_data_folder = os.environ['HOME']
return os.path.join(app_data_folder, ".config", "Code", "User", "settings.json")


def _get_user_settings():
path = _get_user_settings_path()
try:
with open(path) as file:
data = json.load(file)
environment_name = data.get("azure.cloud", "Azure")
return environment_name
except IOError:
return "Azure"


def _get_refresh_token(service_name, account_name):
# _libsecret.secret_password_lookup_sync raises segment fault on Python 2.7
# temporarily disable it on 2.7
import sys
if sys.version_info[0] < 3:
return None

if not _libsecret:
return None
schema = _libsecret.secret_schema_new(_c_str("org.freedesktop.Secret.Generic"), 2,
_c_str("service"), 0, _c_str("account"), 0, None)
p_str = _libsecret.secret_password_lookup_sync(schema, None, None, _c_str("service"), _c_str(service_name),
Copy link
Member

Choose a reason for hiding this comment

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

You're passing None for the third parameter, you're not checking the error. From the testbase of libsecret

static void
test_lookup_no_name (Test *test,
                     gconstpointer used)
{
	GError *error = NULL;
	gchar *password;

	/* should return null, because nothing with mock schema and 5 */
	password = secret_password_lookup_sync (&MOCK_SCHEMA, NULL, &error,
	                                        "number", 5,
	                                        NULL);
	g_assert_no_error (error);
	g_assert (password == NULL);

	/* should return an item, because we have a prime schema with 5, and flags not to match name */
	password = secret_password_lookup_sync (&NO_NAME_SCHEMA, NULL, &error,
	                                        "number", 5,
	                                        NULL);

	g_assert_no_error (error);
	g_assert_cmpstr (password, ==, "555");

	secret_password_free (password);
}

Copy link
Member

Choose a reason for hiding this comment

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

I'm keen to see the coredump on 2.7 ;)

Copy link
Member Author

Choose a reason for hiding this comment

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

Added.

_c_str("account"), _c_str(account_name), None)
return ct.c_char_p(p_str).value.decode('utf-8')
Copy link
Member

Choose a reason for hiding this comment

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

Is this an actual password that could be utf8, or an ascii token?

Copy link
Member Author

@xiangyan99 xiangyan99 Apr 30, 2020

Choose a reason for hiding this comment

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

We tried some real ones and they were utf8.



def get_credentials():
try:
environment_name = _get_user_settings()
credentials = _get_refresh_token(VSCODE_CREDENTIALS_SECTION, environment_name)
return credentials
except Exception: #pylint: disable=broad-except
return None
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
# ------------------------------------
# Copyright (c) Microsoft Corporation.
# Licensed under the MIT License.
# ------------------------------------
import os
import json
from msal_extensions.osx import Keychain, KeychainError
from .._constants import VSCODE_CREDENTIALS_SECTION


def _get_user_settings_path():
app_data_folder = os.environ['USER']
return os.path.join(app_data_folder, "Library", "Application Support", "Code", "User", "settings.json")


def _get_user_settings():
path = _get_user_settings_path()
try:
with open(path) as file:
data = json.load(file)
environment_name = data.get("azure.cloud", "Azure")
return environment_name
except IOError:
return "Azure"


def _get_refresh_token(service_name, account_name):
key_chain = Keychain()
try:
return key_chain.get_generic_password(service_name, account_name)
except KeychainError:
return None


def get_credentials():
try:
environment_name = _get_user_settings()
credentials = _get_refresh_token(VSCODE_CREDENTIALS_SECTION, environment_name)
return credentials
except Exception: #pylint: disable=broad-except
return None
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
# ------------------------------------
# Copyright (c) Microsoft Corporation.
# Licensed under the MIT License.
# ------------------------------------
import sys
from typing import TYPE_CHECKING
from .._exceptions import CredentialUnavailableError
from .._constants import AZURE_VSCODE_CLIENT_ID
from .._internal.aad_client import AadClient
if sys.platform.startswith('win'):
from .win_vscode_adapter import get_credentials
elif sys.platform.startswith('darwin'):
from .macos_vscode_adapter import get_credentials
else:
from .linux_vscode_adapter import get_credentials

if TYPE_CHECKING:
# pylint:disable=unused-import,ungrouped-imports
from typing import Any, Iterable, Optional
from azure.core.credentials import AccessToken


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)

def get_token(self, *scopes, **kwargs):
# type: (*str, **Any) -> AccessToken
"""Request an access token for `scopes`.

.. note:: This method is called by Azure SDK clients. It isn't intended for use in application code.

When this method is called, the credential will try to get the refresh token saved by VS Code. If a refresh
token can be found, it will redeem the refresh token for an access token and return the access token.

:param str scopes: desired scopes for the access token. This method requires at least one scope.
:rtype: :class:`azure.core.credentials.AccessToken`
:raises ~azure.identity.CredentialUnavailableError: fail to get refresh token.
"""
if not scopes:
raise ValueError("'get_token' requires at least one scope")

refresh_token = get_credentials()
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)
return token
Original file line number Diff line number Diff line change
@@ -0,0 +1,84 @@
# ------------------------------------
# Copyright (c) Microsoft Corporation.
# Licensed under the MIT License.
# ------------------------------------
import os
import json
import ctypes as ct
from .._constants import VSCODE_CREDENTIALS_SECTION
try:
import ctypes.wintypes as wt
except (IOError, ValueError):
pass


SUPPORTED_CREDKEYS = set((
'Type', 'TargetName', 'Persist',
'UserName', 'Comment', 'CredentialBlob'))


class _CREDENTIAL(ct.Structure):
_fields_ = [
("Flags", wt.DWORD),
("Type", wt.DWORD),
("TargetName", ct.c_wchar_p),
("Comment", ct.c_wchar_p),
("LastWritten", wt.FILETIME),
("CredentialBlobSize", wt.DWORD),
("CredentialBlob", wt.LPBYTE),
("Persist", wt.DWORD),
("AttributeCount", wt.DWORD),
("Attributes", ct.c_void_p),
("TargetAlias", ct.c_wchar_p),
("UserName", ct.c_wchar_p)]


_PCREDENTIAL = ct.POINTER(_CREDENTIAL)

_advapi = ct.WinDLL('advapi32')
_advapi.CredReadW.argtypes = [wt.LPCWSTR, wt.DWORD, wt.DWORD, ct.POINTER(_PCREDENTIAL)]
_advapi.CredReadW.restype = wt.BOOL
_advapi.CredFree.argtypes = [_PCREDENTIAL]


def _read_credential(service_name, account_name):
target = "{}/{}".format(service_name, account_name)
cred_ptr = _PCREDENTIAL()
if _advapi.CredReadW(target, 1, 0, ct.byref(cred_ptr)):
cred_blob = cred_ptr.contents.CredentialBlob
cred_blob_size = cred_ptr.contents.CredentialBlobSize
password_as_list = [int.from_bytes(cred_blob[pos:pos + 1], 'little')
for pos in range(0, cred_blob_size)]
cred = ''.join(map(chr, password_as_list))
_advapi.CredFree(cred_ptr)
return cred
return None


def _get_user_settings_path():
app_data_folder = os.environ['APPDATA']
return os.path.join(app_data_folder, "Code", "User", "settings.json")


def _get_user_settings():
path = _get_user_settings_path()
try:
with open(path) as file:
data = json.load(file)
environment_name = data.get("azure.cloud", "Azure")
return environment_name
except IOError:
return "Azure"


def _get_refresh_token(service_name, account_name):
return _read_credential(service_name, account_name)


def get_credentials():
try:
environment_name = _get_user_settings()
Copy link
Member

Choose a reason for hiding this comment

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

Looking at the .NET code, the environment name apparently indicates which cloud the user authenticated in. If that's the case, the credential needs it to decide where to send the token request.

Using this information in the credential is awkward (though still possible) because the client's authority is set at construction. The client's token cache shares this behavior though, so I'm thinking we'll need to use (and document) a workaround initially: a user can set AZURE_AUTHORITY_HOST or pass authority=KnownAuthorities... to match the cloud set in VS Code.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes. In our current design, it is on user to make sure AZURE_AUTHORITY_HOST or authority=KnownAuthorities... match the setting in vscode.

credentials = _get_refresh_token(VSCODE_CREDENTIALS_SECTION, environment_name)
return credentials
except Exception: #pylint: disable=broad-except
return None
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
# ------------------------------------
# Copyright (c) Microsoft Corporation.
# Licensed under the MIT License.
# ------------------------------------
from typing import TYPE_CHECKING
from ..._exceptions import CredentialUnavailableError
from .._credentials.base import AsyncCredentialBase
from ..._constants import AZURE_VSCODE_CLIENT_ID
from .._internal.aad_client import AadClient
from ..._credentials.vscode_credential import get_credentials
if TYPE_CHECKING:
# pylint:disable=unused-import,ungrouped-imports
from typing import Any, Iterable, Optional
from azure.core.credentials import AccessToken

class VSCodeCredential(AsyncCredentialBase):
"""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)

async def __aenter__(self):
if self._client:
await self._client.__aenter__()
return self

async def close(self):
"""Close the credential's transport session."""

if self._client:
await self._client.__aexit__()

async def get_token(self, *scopes, **kwargs):
# type: (*str, **Any) -> AccessToken
"""Request an access token for `scopes`.

.. note:: This method is called by Azure SDK clients. It isn't intended for use in application code.

When this method is called, the credential will try to get the refresh token saved by VS Code. If a refresh
token can be found, it will redeem the refresh token for an access token and return the access token.

:param str scopes: desired scopes for the access token. This method requires at least one scope.
:rtype: :class:`azure.core.credentials.AccessToken`
:raises ~azure.identity.CredentialUnavailableError: fail to get refresh token.
"""
if not scopes:
raise ValueError("'get_token' requires at least one scope")

refresh_token = get_credentials()
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)
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't this go before trying to get a refresh token?

Copy link
Member Author

@xiangyan99 xiangyan99 Apr 29, 2020

Choose a reason for hiding this comment

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

If we fail to get a refresh token (which means the user may have logged out), why we should still return a valid token if there is one stored?

Copy link
Member

Choose a reason for hiding this comment

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

The point is perhaps moot because get_token is usually called by an auth policy which caches access tokens itself. There will typically be a period of minutes--during which authorization will succeed--between a user signing out and the next get_token call.

Still, you ask a good question. Here's a couple things to think about in answering it:

  1. Access tokens aren't connected to refresh tokens. They're valid for their (short) lifetimes even after the refresh token used to acquire them expires or is revoked.
  2. In this scenario the user has signed out but left a program running. The program has tried to send a service request. Which is more surprising to the user: the request succeeding, or failing?

There's another interesting case here. The user can change the signed in identity between two calls to get_token. That suggests an argument for having get_token always redeem the refresh token, if any, for a new access token, although I don't think users should expect changing identities midstream to pass without error. Redeeming the refresh token every time also raises another issue: when we redeem a refresh token, AAD may return us a new one and invalidate the one we sent. We don't contribute this new refresh token back to VS Code. That means our credential can invalidate the Azure Account extension's refresh token but itself continue to succeed and I guess we then recurse on "what happens if the user signs out of the extension?" 😸

I'm trying to get all the food for thought here down on this page, sorry for all the words. Personally I now lean toward:

  1. forget caching in this credential; leave it up to the auth policy
  2. if the user signs out or changes the signed in identity, we make no promises: undefined behavior ahead
  3. iterate following feedback

Anyone else have thoughts?

Copy link
Member Author

@xiangyan99 xiangyan99 Apr 30, 2020

Choose a reason for hiding this comment

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

In this scenario the user has signed out but left a program running. The program has tried to send a service request. Which is more surprising to the user: the request succeeding, or failing?

I think the choices are failing immediately or failing in a few minutes. I lean toward failing immediately because

  1. It is deterministic
  2. It is easier to diagnose.

if not token:
token = await self._client.obtain_token_by_refresh_token(refresh_token, scopes, **kwargs)
return token
Loading