From f289d7daf2a94be36835c41f4bfe6c8ce0cfaea0 Mon Sep 17 00:00:00 2001 From: Navya Canumalla Date: Tue, 12 Nov 2019 16:40:34 -0800 Subject: [PATCH 01/22] Update Readme preview note and links --- README.md | 74 +++++++++++++++++++++++++++---------------------------- 1 file changed, 37 insertions(+), 37 deletions(-) diff --git a/README.md b/README.md index 6b0bf225..3de13d22 100644 --- a/README.md +++ b/README.md @@ -1,40 +1,40 @@ # Microsoft Authentication Library (MSAL) for Python Preview -The MSAL library for Python enables your app to access the -[Microsoft Cloud](https://cloud.microsoft.com) -by supporting authentication of users with -[Microsoft Azure Active Directory accounts](https://azure.microsoft.com/en-us/services/active-directory/) -and [Microsoft Accounts](https://account.microsoft.com) using industry standard OAuth2 and OpenID Connect. -Soon MSAL Python will also support [Azure AD B2C](https://azure.microsoft.com/services/active-directory-b2c/). - -Not sure whether this is the SDK you are looking for? There are other Microsoft Identity SDKs + +| `dev` branch | Reference Docs +|---------------|--------------- + [![Build status](https://api.travis-ci.org/AzureAD/microsoft-authentication-library-for-python.svg?branch=dev)](https://travis-ci.org/AzureAD/microsoft-authentication-library-for-python) | [![Documentation Status](https://readthedocs.org/projects/msal-python/badge/?version=latest)](https://msal-python.readthedocs.io/en/latest/?badge=latest) + +The Microsoft Authentication Library for Python enables applications to integrate with the [Microsoft identity platform](https://aka.ms/aaddevv2). It allows you to sign in users or apps with Microsoft identities ([Azure AD](https://azure.microsoft.com/services/active-directory/), [Microsoft Accounts](https://account.microsoft.com) and [Azure AD B2C](https://azure.microsoft.com/services/active-directory-b2c/) accounts) and obtain tokens to call Microsoft APIs such as [Microsoft Graph](https://graph.microsoft.io/) or your own APIs registered with the Microsoft identity platform. It is built using industry standard OAuth2 and OpenID Connect protocols + +Not sure whether this is the SDK you are looking for your app? There are other Microsoft Identity SDKs [here](https://github.com/AzureAD/microsoft-authentication-library-for-python/wiki/Microsoft-Authentication-Client-Libraries). -## Important Note about the MSAL Preview +Quick links: -This library is suitable for use in a production environment. -We provide the same production level support for this library as we do our current production libraries. -During the preview we may make changes to the API, internal cache format, and other mechanisms of this library, -which you will be required to take along with bug fixes or feature improvements. -This may impact your application. -For instance, a change to the cache format may impact your users, such as requiring them to sign in again. -An API change may require you to update your code. -When we provide the General Availability release -we will require you to update to the General Availability version within six months, -as applications written using a preview version of library may no longer work. +| [Getting Started](https://docs.microsoft.com/azure/active-directory/develop/quickstart-v2-python-webapp) | [Docs](https://github.com/AzureAD/microsoft-authentication-library-for-python/wiki) | [Samples](https://aka.ms/aaddevsamplesv2) | [Support](README.md#community-help-and-support) +| --- | --- | --- | --- | ## Installation +You can find MSAL Python on [Pypi](https://pypi.org/project/msal/). 1. If you haven't already, [install and/or upgrade the pip](https://pip.pypa.io/en/stable/installing/) of your Python environment to a recent version. We tested with pip 18.1. 2. As usual, just run `pip install msal`. -## Usage and Samples +## Versions + +This library follows [Semantic Versioning](http://semver.org/). + +You can find the changes for each version under +[Releases](https://github.com/AzureAD/microsoft-authentication-library-for-python/releases). + +## Usage Before using MSAL Python (or any MSAL SDKs, for that matter), you will have to -[register your application with the AAD 2.0 endpoint](https://docs.microsoft.com/en-us/azure/active-directory/develop/quickstart-v2-register-an-app). +[register your application with the Microsoft identity platform](https://docs.microsoft.com/azure/active-directory/develop/quickstart-v2-register-an-app). -Acquiring tokens with MSAL Python need to follow this 3-step pattern. +Acquiring tokens with MSAL Python follows this 3-step pattern. 1. MSAL proposes a clean separation between [public client applications, and confidential client applications](https://tools.ietf.org/html/rfc6749#section-2.1). @@ -89,28 +89,28 @@ Acquiring tokens with MSAL Python need to follow this 3-step pattern. That is the high level pattern. There will be some variations for different flows. They are demonstrated in [samples hosted right in this repo](https://github.com/AzureAD/microsoft-authentication-library-for-python/tree/dev/sample). +Refer the [Wiki](https://github.com/AzureAD/microsoft-authentication-library-for-python/wiki) pages for more details on the MSAL Python functionality and usage. -## Documentation +## Migrating from ADAL -The generic documents on -[Auth Scenarios](https://docs.microsoft.com/en-us/azure/active-directory/develop/authentication-scenarios) -and -[Auth protocols](https://docs.microsoft.com/en-us/azure/active-directory/develop/active-directory-v2-protocols) -are recommended reading. +If your application is using ADAL Python, we recommend you to update to use MSAL Python. No new feature work will be done in ADAL Python. -There is the [API reference of MSAL Python](https://msal-python.rtfd.io) which documents every parameter of each public method. +See the [ADAL to MSAL migration](https://github.com/AzureAD/microsoft-authentication-library-for-python/wiki/Migrate-to-MSAL-Python) guide. -More and more detail about MSAL Python functionality and usage will be documented in the -[Wiki](https://github.com/AzureAD/microsoft-authentication-library-for-python/wiki). +## Roadmap +You can follow the latest updates and plans for MSAL Python in the [Roadmap](https://github.com/AzureAD/microsoft-authentication-library-for-python/wiki/Roadmap) published on our Wiki. +## Samples and Documentation -## Versions - -This library follows [Semantic Versioning](http://semver.org/). +MSAL Python supports multiple [application types and authentication scenarios](https://docs.microsoft.com/azure/active-directory/develop/authentication-flows-app-scenarios). +The generic documents on +[Auth Scenarios](https://docs.microsoft.com/azure/active-directory/develop/authentication-scenarios) +and +[Auth protocols](https://docs.microsoft.com/azure/active-directory/develop/active-directory-v2-protocols) +are recommended reading. -You can find the changes for each version under -[Releases](https://github.com/AzureAD/microsoft-authentication-library-for-python/releases). +We provide a [full suite of sample applications](https://aka.ms/aaddevsamplesv2) and [documentation](https://aka.ms/aaddevv2) to help you get started with learning the Microsoft identity platform. ## Community Help and Support @@ -124,7 +124,7 @@ Here is the latest Q&A on Stack Overflow for MSAL: ## Security Reporting -If you find a security issue with our libraries or services please report it to [secure@microsoft.com](mailto:secure@microsoft.com) with as much detail as possible. Your submission may be eligible for a bounty through the [Microsoft Bounty](http://aka.ms/bugbounty) program. Please do not post security issues to GitHub Issues or any other public site. We will contact you shortly upon receiving the information. We encourage you to get notifications of when security incidents occur by visiting [this page](https://technet.microsoft.com/en-us/security/dd252948) and subscribing to Security Advisory Alerts. +If you find a security issue with our libraries or services please report it to [secure@microsoft.com](mailto:secure@microsoft.com) with as much detail as possible. Your submission may be eligible for a bounty through the [Microsoft Bounty](http://aka.ms/bugbounty) program. Please do not post security issues to GitHub Issues or any other public site. We will contact you shortly upon receiving the information. We encourage you to get notifications of when security incidents occur by visiting [this page](https://technet.microsoft.com/security/dd252948) and subscribing to Security Advisory Alerts. ## Contributing From 58323b08917d7f9b4ee95963999496cfce987297 Mon Sep 17 00:00:00 2001 From: Ray Luo Date: Fri, 22 Nov 2019 15:26:08 -0800 Subject: [PATCH 02/22] Should have normalized authority host to lowercase --- msal/authority.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/msal/authority.py b/msal/authority.py index 667f5ebd..dae97aab 100644 --- a/msal/authority.py +++ b/msal/authority.py @@ -100,6 +100,7 @@ def user_realm_discovery(self, username, response=None): def canonicalize(authority_url): + # Returns (url_parsed_result, hostname_in_lowercase, tenant) authority = urlparse(authority_url) parts = authority.path.split("/") if authority.scheme != "https" or len(parts) < 2 or not parts[1]: @@ -109,7 +110,7 @@ def canonicalize(authority_url): "https://login.microsoftonline.com/ " "or https://.b2clogin.com/.onmicrosoft.com/policy" % authority_url) - return authority, authority.netloc, parts[1] + return authority, authority.hostname, parts[1] def instance_discovery(url, **kwargs): return requests.get( # Note: This URL seemingly returns V1 endpoint only From 6a670be52e396f6e6a1124718d28b5482e06c210 Mon Sep 17 00:00:00 2001 From: Ray Luo Date: Tue, 26 Nov 2019 13:37:48 -0800 Subject: [PATCH 03/22] Disable OBO tests due to a temporary test environment issue --- tests/test_e2e.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/tests/test_e2e.py b/tests/test_e2e.py index 770e4625..7f4c32bb 100644 --- a/tests/test_e2e.py +++ b/tests/test_e2e.py @@ -390,7 +390,7 @@ def test_adfs2019_fed_user(self): password=self.get_lab_user_secret(config["lab"]["labname"]), **config) @unittest.skipUnless( - os.getenv("OBO_CLIENT_SECRET"), + os.getenv("OBO_CLIENT_SECRET") and False, # Temporarily disable this case "Need OBO_CLIENT_SECRET from https://buildautomation.vault.azure.net/secrets/IdentityDivisionDotNetOBOServiceSecret") def test_acquire_token_obo(self): # Some hardcoded, pre-defined settings @@ -407,7 +407,9 @@ def test_acquire_token_obo(self): scopes=[ # The OBO app's scope. Yours might be different. "%s/access_as_user" % obo_client_id], ) - self.assertIsNotNone(pca_result.get("access_token"), "PCA should work") + self.assertIsNotNone( + pca_result.get("access_token"), + "PCA failed to get AT because %s" % json.dumps(pca_result, indent=2)) # 2. Our mid-tier service uses OBO to obtain a token for downstream service cca = msal.ConfidentialClientApplication( From 0200777e2f5dfd9db8e5086a3e562b45d30c47c5 Mon Sep 17 00:00:00 2001 From: Ray Luo Date: Mon, 18 Nov 2019 13:28:22 -0800 Subject: [PATCH 04/22] Change to new lab api --- tests/test_e2e.py | 82 ++++++++++++++++------------------------------- 1 file changed, 28 insertions(+), 54 deletions(-) diff --git a/tests/test_e2e.py b/tests/test_e2e.py index 7f4c32bb..670d36d6 100644 --- a/tests/test_e2e.py +++ b/tests/test_e2e.py @@ -2,11 +2,11 @@ import os import json import time +import unittest import requests import msal -from tests import unittest logger = logging.getLogger(__name__) @@ -263,19 +263,6 @@ def test_device_flow(self): "%s obtained tokens: %s", self.id(), json.dumps(result, indent=4)) -def get_lab_user(mam=False, mfa=False, isFederated=False, federationProvider=None): - # Based on https://microsoft.sharepoint-df.com/teams/MSIDLABSExtended/SitePages/LAB.aspx - user = requests.get("https://api.msidlab.com/api/user", params=dict( # Publicly available - mam=mam, mfa=mfa, isFederated=isFederated, federationProvider=federationProvider, - )).json() - return { # Mapping lab API response to our simplified configuration format - "authority": user["Authority"][0] + user["Users"]["tenantId"], - "client_id": user["AppID"], - "username": user["Users"]["upn"], - "lab": {"labname": user["Users"]["upn"].split('@')[1].split('.')[0]}, # :( - "scope": ["https://graph.microsoft.com/.default"], - } - def get_lab_app( env_client_id="LAB_APP_CLIENT_ID", env_client_secret="LAB_APP_CLIENT_SECRET", @@ -318,10 +305,8 @@ class LabBasedTestCase(E2eTestCase): @classmethod def setUpClass(cls): - cls.session = get_session(get_lab_app(), [ - "https://request.msidlab.com/.default", # Existing user & password API - # "https://user.msidlab.com/.default", # New user API - ]) + # https://docs.msidlab.com/accounts/apiaccess.html#code-snippet + cls.session = get_session(get_lab_app(), ["https://user.msidlab.com/.default"]) @classmethod def tearDownClass(cls): @@ -332,62 +317,48 @@ def get_lab_user_secret(cls, lab_name="msidlab4"): lab_name = lab_name.lower() if lab_name not in cls._secrets: logger.info("Querying lab user password for %s", lab_name) - # Short link only works in browser "https://aka.ms/GetLabUserSecret?Secret=%s" - # So we use the official link written in here - # https://microsoft.sharepoint-df.com/teams/MSIDLABSExtended/SitePages/Programmatically-accessing-LAB-API%27s.aspx - url = ("https://request.msidlab.com/api/GetLabUserSecret?code=KpY5uCcoKo0aW8VOL/CUO3wnu9UF2XbSnLFGk56BDnmQiwD80MQ7HA==&Secret=%s" - % lab_name) + url = "https://msidlab.com/api/LabUserSecret?secret=%s" % lab_name resp = cls.session.get(url) - cls._secrets[lab_name] = resp.json()["Value"] + cls._secrets[lab_name] = resp.json()["value"] return cls._secrets[lab_name] @classmethod - def get_lab_user(cls, query): # Experimental: The query format is in lab team's Aug 9 email - resp = cls.session.get("https://user.msidlab.com/api/user", params=query) + def get_lab_user(cls, **query): # https://docs.msidlab.com/labapi/userapi.html + resp = cls.session.get("https://msidlab.com/api/user", params=query) result = resp.json()[0] return { # Mapping lab API response to our simplified configuration format - "authority": result["lab"]["authority"] + result["lab"]["tenantid"], - "client_id": result["app"]["objectid"], - "username": result["user"]["upn"], - "lab": result["lab"], + "authority": "https://login.microsoftonline.com/{}.onmicrosoft.com".format( + result["labName"]), + "client_id": result["appId"], + "username": result["upn"], + "lab_name": result["labName"], "scope": ["https://graph.microsoft.com/.default"], } - def test_aad_managed_user(self): # Pure cloud or hybrid - config = get_lab_user(isFederated=False) + def test_aad_managed_user(self): # Pure cloud + config = self.get_lab_user(usertype="cloud") self._test_username_password( - password=self.get_lab_user_secret(config["lab"]["labname"]), **config) + password=self.get_lab_user_secret(config["lab_name"]), **config) def test_adfs4_fed_user(self): - config = get_lab_user(isFederated=True, federationProvider="ADFSv4") - self._test_username_password( - password=self.get_lab_user_secret(config["lab"]["labname"]), **config) - - def test_adfs4_managed_user(self): # Conceptually the hybrid - config = get_lab_user(isFederated=False, federationProvider="ADFSv4") + config = self.get_lab_user(usertype="federated", federationProvider="ADFSv4") self._test_username_password( - password=self.get_lab_user_secret(config["lab"]["labname"]), **config) + password=self.get_lab_user_secret(config["lab_name"]), **config) def test_adfs3_fed_user(self): - config = get_lab_user(isFederated=True, federationProvider="ADFSv3") + config = self.get_lab_user(usertype="federated", federationProvider="ADFSv3") self._test_username_password( - password=self.get_lab_user_secret(config["lab"]["labname"]), **config) - - def test_adfs3_managed_user(self): - config = get_lab_user(isFederated=False, federationProvider="ADFSv3") - self._test_username_password( - password=self.get_lab_user_secret(config["lab"]["labname"]), **config) + password=self.get_lab_user_secret(config["lab_name"]), **config) def test_adfs2_fed_user(self): - config = get_lab_user(isFederated=True, federationProvider="ADFSv2") + config = self.get_lab_user(usertype="federated", federationProvider="ADFSv2") self._test_username_password( - password=self.get_lab_user_secret(config["lab"]["labname"]), **config) + password=self.get_lab_user_secret(config["lab_name"]), **config) - @unittest.skip("Old Lab API returns nothing. We will switch to new api later") def test_adfs2019_fed_user(self): - config = get_lab_user(isFederated=True, federationProvider="ADFSv2019") + config = self.get_lab_user(usertype="federated", federationProvider="ADFSv2019") self._test_username_password( - password=self.get_lab_user_secret(config["lab"]["labname"]), **config) + password=self.get_lab_user_secret(config["lab_name"]), **config) @unittest.skipUnless( os.getenv("OBO_CLIENT_SECRET") and False, # Temporarily disable this case @@ -396,14 +367,14 @@ def test_acquire_token_obo(self): # Some hardcoded, pre-defined settings obo_client_id = "23c64cd8-21e4-41dd-9756-ab9e2c23f58c" downstream_scopes = ["https://graph.microsoft.com/User.Read"] - config = get_lab_user(isFederated=False) + config = self.get_lab_user(usertype="cloud") # 1. An app obtains a token representing a user, for our mid-tier service pca = msal.PublicClientApplication( "be9b0186-7dfd-448a-a944-f771029105bf", authority=config.get("authority")) pca_result = pca.acquire_token_by_username_password( config["username"], - self.get_lab_user_secret(config["lab"]["labname"]), + self.get_lab_user_secret(config["lab_name"]), scopes=[ # The OBO app's scope. Yours might be different. "%s/access_as_user" % obo_client_id], ) @@ -483,3 +454,6 @@ def test_b2c_acquire_token_by_ropc(self): scope=["https://msidlabb2c.onmicrosoft.com/msidlabb2capi/read"], ) +if __name__ == "__main__": + unittest.main() + From 37deb9ebcc4cb408e1276937918a3df1fdf79286 Mon Sep 17 00:00:00 2001 From: Ray Luo Date: Tue, 3 Dec 2019 09:54:18 -0800 Subject: [PATCH 05/22] Re-enable OBO tests The new test credential has been setup in Travis-CI.org --- tests/test_e2e.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_e2e.py b/tests/test_e2e.py index 670d36d6..727f3dba 100644 --- a/tests/test_e2e.py +++ b/tests/test_e2e.py @@ -361,7 +361,7 @@ def test_adfs2019_fed_user(self): password=self.get_lab_user_secret(config["lab_name"]), **config) @unittest.skipUnless( - os.getenv("OBO_CLIENT_SECRET") and False, # Temporarily disable this case + os.getenv("OBO_CLIENT_SECRET"), "Need OBO_CLIENT_SECRET from https://buildautomation.vault.azure.net/secrets/IdentityDivisionDotNetOBOServiceSecret") def test_acquire_token_obo(self): # Some hardcoded, pre-defined settings From cd809cd0e4e2a6a1f495f2691efb4e3f42a38aac Mon Sep 17 00:00:00 2001 From: Ray Luo Date: Tue, 3 Dec 2019 15:59:52 -0800 Subject: [PATCH 06/22] Update test_e2e.py --- tests/test_e2e.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_e2e.py b/tests/test_e2e.py index 727f3dba..37f47d77 100644 --- a/tests/test_e2e.py +++ b/tests/test_e2e.py @@ -306,7 +306,7 @@ class LabBasedTestCase(E2eTestCase): @classmethod def setUpClass(cls): # https://docs.msidlab.com/accounts/apiaccess.html#code-snippet - cls.session = get_session(get_lab_app(), ["https://user.msidlab.com/.default"]) + cls.session = get_session(get_lab_app(), ["https://msidlab.com/.default"]) @classmethod def tearDownClass(cls): From 77a5b9d4cf3f7fd74796fb0d5b1a4e7a10a0aa7b Mon Sep 17 00:00:00 2001 From: Abhidnya Date: Tue, 10 Dec 2019 17:39:02 -0800 Subject: [PATCH 07/22] Adding app_name and app_version headers (#136) --- msal/application.py | 25 +++++++++++++++++++------ 1 file changed, 19 insertions(+), 6 deletions(-) diff --git a/msal/application.py b/msal/application.py index 19c49eb1..17fc8dc9 100644 --- a/msal/application.py +++ b/msal/application.py @@ -73,7 +73,7 @@ def __init__( client_credential=None, authority=None, validate_authority=True, token_cache=None, verify=True, proxies=None, timeout=None, - client_claims=None): + client_claims=None, app_name=None, app_version=None): """Create an instance of application. :param client_id: Your app has a client_id after you register it on AAD. @@ -131,6 +131,12 @@ def __init__( It will be passed to the `timeout parameter in the underlying requests library `_ + :param app_name: (optional) + You can provide your application name for Microsoft telemetry purposes. + Default value is None, means it will not be passed to Microsoft. + :param app_version: (optional) + You can provide your application version for Microsoft telemetry purposes. + Default value is None, means it will not be passed to Microsoft. """ self.client_id = client_id self.client_credential = client_credential @@ -138,6 +144,8 @@ def __init__( self.verify = verify self.proxies = proxies self.timeout = timeout + self.app_name = app_name + self.app_version = app_version self.authority = Authority( authority or "https://login.microsoftonline.com/common/", validate_authority, verify=verify, proxies=proxies, timeout=timeout) @@ -149,6 +157,15 @@ def __init__( def _build_client(self, client_credential, authority): client_assertion = None client_assertion_type = None + default_headers = { + "x-client-sku": "MSAL.Python", "x-client-ver": __version__, + "x-client-os": sys.platform, + "x-client-cpu": "x64" if sys.maxsize > 2 ** 32 else "x86", + } + if self.app_name: + default_headers['x-app-name'] = self.app_name + if self.app_version: + default_headers['x-app-ver'] = self.app_version default_body = {"client_info": 1} if isinstance(client_credential, dict): assert ("private_key" in client_credential @@ -174,11 +191,7 @@ def _build_client(self, client_credential, authority): return Client( server_configuration, self.client_id, - default_headers={ - "x-client-sku": "MSAL.Python", "x-client-ver": __version__, - "x-client-os": sys.platform, - "x-client-cpu": "x64" if sys.maxsize > 2 ** 32 else "x86", - }, + default_headers=default_headers, default_body=default_body, client_assertion=client_assertion, client_assertion_type=client_assertion_type, From 53806455666d98fe35e5bd7d37621aac7ba32307 Mon Sep 17 00:00:00 2001 From: Navya Canumalla Date: Mon, 16 Dec 2019 11:17:46 -0800 Subject: [PATCH 08/22] Update Readme to remove Preview --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 3de13d22..6a87fc1d 100644 --- a/README.md +++ b/README.md @@ -1,4 +1,4 @@ -# Microsoft Authentication Library (MSAL) for Python Preview +# Microsoft Authentication Library (MSAL) for Python | `dev` branch | Reference Docs From 5efb0d58d58ea7dbcade3d0d6a0ed2f281dfb73d Mon Sep 17 00:00:00 2001 From: Santiago Gonzalez <35743865+sangonzal@users.noreply.github.com> Date: Tue, 17 Dec 2019 08:54:44 -0800 Subject: [PATCH 09/22] Add correlation id, public api id, and current request header (#103) * Add correlation id, public api id, and current request header * Make get_new_correlation_id private * Update calls to _get_new_correlation_id() * PR feedback. * Fix authority test * PR feedback * More PR feedback * Remove indent * Refactor and fix a bug in the initiate_device_flow() * Refactor acquire_token_silent() to reuse same correlation_id * fixup! Refactor and fix a bug in the initiate_device_flow() * Define consts for all telemetry headers * Replace one last magic string with const --- msal/application.py | 102 +++++++++++++++++++++++++++++++++++++------- msal/authority.py | 5 ++- 2 files changed, 89 insertions(+), 18 deletions(-) diff --git a/msal/application.py b/msal/application.py index 17fc8dc9..20a77525 100644 --- a/msal/application.py +++ b/msal/application.py @@ -6,6 +6,7 @@ import logging import sys import warnings +import uuid import requests @@ -49,6 +50,16 @@ def decorate_scope( decorated = scope_set | reserved_scope return list(decorated) +CLIENT_REQUEST_ID = 'client-request-id' +CLIENT_CURRENT_TELEMETRY = 'x-client-current-telemetry' + +def _get_new_correlation_id(): + return str(uuid.uuid4()) + + +def _build_current_telemetry_request_header(public_api_id, force_refresh=False): + return "1|{},{}|".format(public_api_id, "1" if force_refresh else "0") + def extract_certs(public_cert_content): # Parses raw public certificate file contents and returns a list of strings @@ -68,6 +79,15 @@ def extract_certs(public_cert_content): class ClientApplication(object): + ACQUIRE_TOKEN_SILENT_ID = "84" + ACQUIRE_TOKEN_BY_USERNAME_PASSWORD_ID = "301" + ACQUIRE_TOKEN_ON_BEHALF_OF_ID = "523" + ACQUIRE_TOKEN_BY_DEVICE_FLOW_ID = "622" + ACQUIRE_TOKEN_FOR_CLIENT_ID = "730" + ACQUIRE_TOKEN_BY_AUTHORIZATION_CODE_ID = "832" + GET_ACCOUNTS_ID = "902" + REMOVE_ACCOUNT_ID = "903" + def __init__( self, client_id, client_credential=None, authority=None, validate_authority=True, @@ -303,6 +323,11 @@ def acquire_token_by_authorization_code( data=dict( kwargs.pop("data", {}), scope=decorate_scope(scopes, self.client_id)), + headers={ + CLIENT_REQUEST_ID: _get_new_correlation_id(), + CLIENT_CURRENT_TELEMETRY: _build_current_telemetry_request_header( + self.ACQUIRE_TOKEN_BY_AUTHORIZATION_CODE_ID), + }, **kwargs) def get_accounts(self, username=None): @@ -426,6 +451,7 @@ def acquire_token_silent( """ assert isinstance(scopes, list), "Invalid parameter type" self._validate_ssh_cert_input_data(kwargs.get("data", {})) + correlation_id = _get_new_correlation_id() if authority: warnings.warn("We haven't decided how/if this method will accept authority parameter") # the_authority = Authority( @@ -433,7 +459,9 @@ def acquire_token_silent( # verify=self.verify, proxies=self.proxies, timeout=self.timeout, # ) if authority else self.authority result = self._acquire_token_silent_from_cache_and_possibly_refresh_it( - scopes, account, self.authority, force_refresh=force_refresh, **kwargs) + scopes, account, self.authority, force_refresh=force_refresh, + correlation_id=correlation_id, + **kwargs) if result: return result for alias in self._get_authority_aliases(self.authority.instance): @@ -442,7 +470,9 @@ def acquire_token_silent( validate_authority=False, verify=self.verify, proxies=self.proxies, timeout=self.timeout) result = self._acquire_token_silent_from_cache_and_possibly_refresh_it( - scopes, account, the_authority, force_refresh=force_refresh, **kwargs) + scopes, account, the_authority, force_refresh=force_refresh, + correlation_id=correlation_id, + **kwargs) if result: return result @@ -480,7 +510,7 @@ def _acquire_token_silent_from_cache_and_possibly_refresh_it( } return self._acquire_token_silent_by_finding_rt_belongs_to_me_or_my_family( authority, decorate_scope(scopes, self.client_id), account, - **kwargs) + force_refresh=force_refresh, **kwargs) def _acquire_token_silent_by_finding_rt_belongs_to_me_or_my_family( self, authority, scopes, account, **kwargs): @@ -526,7 +556,8 @@ def _get_app_metadata(self, environment): def _acquire_token_silent_by_finding_specific_refresh_token( self, authority, scopes, query, - rt_remover=None, break_condition=lambda response: False, **kwargs): + rt_remover=None, break_condition=lambda response: False, + force_refresh=False, correlation_id=None, **kwargs): matches = self.token_cache.find( self.token_cache.CredentialType.REFRESH_TOKEN, # target=scopes, # AAD RTs are scope-independent @@ -539,6 +570,11 @@ def _acquire_token_silent_by_finding_specific_refresh_token( entry, rt_getter=lambda token_item: token_item["secret"], on_removing_rt=rt_remover or self.token_cache.remove_rt, scope=scopes, + headers={ + CLIENT_REQUEST_ID: correlation_id or _get_new_correlation_id(), + CLIENT_CURRENT_TELEMETRY: _build_current_telemetry_request_header( + self.ACQUIRE_TOKEN_SILENT_ID, force_refresh=force_refresh), + }, **kwargs) if "error" not in response: return response @@ -564,6 +600,8 @@ def _validate_ssh_cert_input_data(self, data): class PublicClientApplication(ClientApplication): # browser app or mobile app + DEVICE_FLOW_CORRELATION_ID = "_correlation_id" + def __init__(self, client_id, client_credential=None, **kwargs): if client_credential is not None: raise ValueError("Public Client should not possess credentials") @@ -581,9 +619,16 @@ def initiate_device_flow(self, scopes=None, **kwargs): - A successful response would contain "user_code" key, among others - an error response would contain some other readable key/value pairs. """ - return self.client.initiate_device_flow( + correlation_id = _get_new_correlation_id() + flow = self.client.initiate_device_flow( scope=decorate_scope(scopes or [], self.client_id), + headers={ + CLIENT_REQUEST_ID: correlation_id, + # CLIENT_CURRENT_TELEMETRY is not currently required + }, **kwargs) + flow[self.DEVICE_FLOW_CORRELATION_ID] = correlation_id + return flow def acquire_token_by_device_flow(self, flow, **kwargs): """Obtain token by a device flow object, with customizable polling effect. @@ -600,12 +645,18 @@ def acquire_token_by_device_flow(self, flow, **kwargs): - an error response would contain "error" and usually "error_description". """ return self.client.obtain_token_by_device_flow( - flow, - data=dict(kwargs.pop("data", {}), code=flow["device_code"]), - # 2018-10-4 Hack: - # during transition period, - # service seemingly need both device_code and code parameter. - **kwargs) + flow, + data=dict(kwargs.pop("data", {}), code=flow["device_code"]), + # 2018-10-4 Hack: + # during transition period, + # service seemingly need both device_code and code parameter. + headers={ + CLIENT_REQUEST_ID: + flow.get(self.DEVICE_FLOW_CORRELATION_ID) or _get_new_correlation_id(), + CLIENT_CURRENT_TELEMETRY: _build_current_telemetry_request_header( + self.ACQUIRE_TOKEN_BY_DEVICE_FLOW_ID), + }, + **kwargs) def acquire_token_by_username_password( self, username, password, scopes, **kwargs): @@ -625,13 +676,22 @@ def acquire_token_by_username_password( - an error response would contain "error" and usually "error_description". """ scopes = decorate_scope(scopes, self.client_id) + headers = { + CLIENT_REQUEST_ID: _get_new_correlation_id(), + CLIENT_CURRENT_TELEMETRY: _build_current_telemetry_request_header( + self.ACQUIRE_TOKEN_BY_USERNAME_PASSWORD_ID), + } if not self.authority.is_adfs: - user_realm_result = self.authority.user_realm_discovery(username) + user_realm_result = self.authority.user_realm_discovery( + username, correlation_id=headers[CLIENT_REQUEST_ID]) if user_realm_result.get("account_type") == "Federated": return self._acquire_token_by_username_password_federated( - user_realm_result, username, password, scopes=scopes, **kwargs) + user_realm_result, username, password, scopes=scopes, + headers=headers, **kwargs) return self.client.obtain_token_by_username_password( - username, password, scope=scopes, **kwargs) + username, password, scope=scopes, + headers=headers, + **kwargs) def _acquire_token_by_username_password_federated( self, user_realm_result, username, password, scopes=None, **kwargs): @@ -687,8 +747,13 @@ def acquire_token_for_client(self, scopes, **kwargs): """ # TBD: force_refresh behavior return self.client.obtain_token_for_client( - scope=scopes, # This grant flow requires no scope decoration - **kwargs) + scope=scopes, # This grant flow requires no scope decoration + headers={ + CLIENT_REQUEST_ID: _get_new_correlation_id(), + CLIENT_CURRENT_TELEMETRY: _build_current_telemetry_request_header( + self.ACQUIRE_TOKEN_FOR_CLIENT_ID), + }, + **kwargs) def acquire_token_on_behalf_of(self, user_assertion, scopes, **kwargs): """Acquires token using on-behalf-of (OBO) flow. @@ -723,5 +788,10 @@ def acquire_token_on_behalf_of(self, user_assertion, scopes, **kwargs): # so that the calling app could use id_token_claims to implement # their own cache mapping, which is likely needed in web apps. data=dict(kwargs.pop("data", {}), requested_token_use="on_behalf_of"), + headers={ + CLIENT_REQUEST_ID: _get_new_correlation_id(), + CLIENT_CURRENT_TELEMETRY: _build_current_telemetry_request_header( + self.ACQUIRE_TOKEN_ON_BEHALF_OF_ID), + }, **kwargs) diff --git a/msal/authority.py b/msal/authority.py index dae97aab..d8221eca 100644 --- a/msal/authority.py +++ b/msal/authority.py @@ -82,7 +82,7 @@ def __init__(self, authority_url, validate_authority=True, _, _, self.tenant = canonicalize(self.token_endpoint) # Usually a GUID self.is_adfs = self.tenant.lower() == 'adfs' - def user_realm_discovery(self, username, response=None): + def user_realm_discovery(self, username, correlation_id=None, response=None): # It will typically return a dict containing "ver", "account_type", # "federation_protocol", "cloud_audience_urn", # "federation_metadata_url", "federation_active_auth_url", etc. @@ -90,7 +90,8 @@ def user_realm_discovery(self, username, response=None): resp = response or requests.get( "https://{netloc}/common/userrealm/{username}?api-version=1.0".format( netloc=self.instance, username=username), - headers={'Accept':'application/json'}, + headers={'Accept':'application/json', + 'client-request-id': correlation_id}, verify=self.verify, proxies=self.proxies, timeout=self.timeout) if resp.status_code != 404: resp.raise_for_status() From e2958961e8ec16d0af4199f60c36c3f913497e48 Mon Sep 17 00:00:00 2001 From: Ray Luo Date: Wed, 18 Dec 2019 11:14:57 -0800 Subject: [PATCH 10/22] Marking this library as Production/Stable for PyPI FYI: https://pypi.org/classifiers/ --- setup.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/setup.py b/setup.py index 0c26e5da..960d4bca 100644 --- a/setup.py +++ b/setup.py @@ -53,7 +53,7 @@ author_email='nugetaad@microsoft.com', url='https://github.com/AzureAD/microsoft-authentication-library-for-python', classifiers=[ - 'Development Status :: 4 - Beta', + 'Development Status :: 5 - Production/Stable', 'Programming Language :: Python', 'Programming Language :: Python :: 2', 'Programming Language :: Python :: 2.7', From 5cd241ddc1117668bdb6d3e5e1bb88aa5d03d04d Mon Sep 17 00:00:00 2001 From: Ray Luo Date: Fri, 20 Dec 2019 12:12:43 -0800 Subject: [PATCH 11/22] We should not assume error_description always exist, because it won't. --- msal/application.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/msal/application.py b/msal/application.py index 20a77525..ce7b340a 100644 --- a/msal/application.py +++ b/msal/application.py @@ -578,8 +578,10 @@ def _acquire_token_silent_by_finding_specific_refresh_token( **kwargs) if "error" not in response: return response - logger.debug( - "Refresh failed. {error}: {error_description}".format(**response)) + logger.debug("Refresh failed. {error}: {error_description}".format( + error=response.get("error"), + error_description=response.get("error_description"), + )) if break_condition(response): break From fe0f957d6b3437249b7f35ab661883592d5e76bd Mon Sep 17 00:00:00 2001 From: Ray Luo Date: Fri, 20 Dec 2019 14:36:04 -0800 Subject: [PATCH 12/22] Error test case should better use HTTP 400 status code --- tests/test_application.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_application.py b/tests/test_application.py index cc072838..814f7c71 100644 --- a/tests/test_application.py +++ b/tests/test_application.py @@ -74,7 +74,7 @@ def test_unknown_orphan_app_will_attempt_frt_and_not_remove_it(self): logger.debug("%s.cache = %s", self.id(), self.cache.serialize()) def tester(url, data=None, **kwargs): self.assertEqual(self.frt, data.get("refresh_token"), "Should attempt the FRT") - return Mock(status_code=200, json=Mock(return_value={ + return Mock(status_code=400, json=Mock(return_value={ "error": "invalid_grant", "error_description": "Was issued to another client"})) app._acquire_token_silent_by_finding_rt_belongs_to_me_or_my_family( From e433f959933100edad172492ad9608e8fe8d29e2 Mon Sep 17 00:00:00 2001 From: Ray Luo Date: Mon, 13 Jan 2020 20:48:33 -0800 Subject: [PATCH 13/22] Decouple internal logic & always trigger callbacks --- oauth2cli/oauth2.py | 41 +++++++++++++++++++++-------------------- 1 file changed, 21 insertions(+), 20 deletions(-) diff --git a/oauth2cli/oauth2.py b/oauth2cli/oauth2.py index de38ef61..efdd5eb0 100644 --- a/oauth2cli/oauth2.py +++ b/oauth2cli/oauth2.py @@ -10,10 +10,13 @@ import warnings import time import base64 +import sys import requests +string_types = (str,) if sys.version_info[0] >= 3 else (basestring, ) + class BaseClient(object): # This low-level interface works. Yet you'll find its sub-class @@ -163,6 +166,7 @@ def _obtain_token( # The verb "obtain" is influenced by OAUTH2 RFC 6749 raise def obtain_token_by_refresh_token(self, refresh_token, scope=None, **kwargs): + # type: (str, Union[str, list, set, tuple]) -> dict """Obtain an access token via a refresh token. :param refresh_token: The refresh token issued to the client @@ -170,6 +174,7 @@ def obtain_token_by_refresh_token(self, refresh_token, scope=None, **kwargs): granted by the resource ownser, according to https://tools.ietf.org/html/rfc6749#section-6 """ + assert isinstance(refresh_token, string_types) data = kwargs.pop('data', {}) data.update(refresh_token=refresh_token, scope=scope) return self._obtain_token("refresh_token", data=data, **kwargs) @@ -380,14 +385,10 @@ def __init__(self, self.on_removing_rt = on_removing_rt self.on_updating_rt = on_updating_rt - def _obtain_token(self, grant_type, params=None, data=None, - rt_getter=lambda token_item: token_item["refresh_token"], - *args, **kwargs): + def _obtain_token(self, grant_type, params=None, data=None, *args, **kwargs): RT = "refresh_token" _data = data.copy() # to prevent side effect refresh_token = _data.get(RT) - if grant_type == RT and isinstance(refresh_token, dict): - _data[RT] = rt_getter(refresh_token) # Put raw RT in _data resp = super(Client, self)._obtain_token( grant_type, params, _data, *args, **kwargs) if "error" not in resp: @@ -416,31 +417,31 @@ def obtain_token_by_refresh_token(self, token_item, scope=None, on_removing_rt=None, **kwargs): # type: (Union[str, dict], Union[str, list, set, tuple], Callable) -> dict - """This is an "overload" which accepts a refresh token item as a dict, - therefore this method can relay refresh_token item to event listeners. + """This is an overload which will trigger token storage callbacks. :param token_item: - A refresh token item as a dict, came from the cache managed by this lib. + A refresh token (RT) item, in flexible format. It can be a string, + or a whatever data structure containing RT string and its metadata, + in such case the `rt_getter` callable must be able to + extract the RT string out from the token item data structure. + + Either way, this token_item will be passed into other callbacks as-is. - Alternatively, you can still use a refresh token (RT) as a string, - supposedly came from a token cache managed by a different library, - then this library will store the new RT (if Authority Server issued one) - into this lib's cache. This is a way to migrate from other lib to us. :param scope: If omitted, is treated as equal to the scope originally granted by the resource ownser, according to https://tools.ietf.org/html/rfc6749#section-6 - :param rt_getter: A callable used to extract the RT from token_item + :param rt_getter: A callable to translate the token_item to a raw RT string :param on_removing_rt: If absent, fall back to the one defined in initialization """ resp = super(Client, self).obtain_token_by_refresh_token( - token_item, scope=scope, - rt_getter=rt_getter, # Wire up this for _obtain_token() + rt_getter(token_item) + if not isinstance(token_item, string_types) else token_item, + scope=scope, **kwargs) - if isinstance(token_item, dict): - if resp.get('error') == 'invalid_grant': - (on_removing_rt or self.on_removing_rt)(token_item) # Discard old RT - if 'refresh_token' in resp: - self.on_updating_rt(token_item, resp['refresh_token']) + if resp.get('error') == 'invalid_grant': + (on_removing_rt or self.on_removing_rt)(token_item) # Discard old RT + if 'refresh_token' in resp: + self.on_updating_rt(token_item, resp['refresh_token']) return resp def obtain_token_by_assertion( From eac29de9394c5ae10c8f8f4adde283773fb3a811 Mon Sep 17 00:00:00 2001 From: Ray Luo Date: Tue, 14 Jan 2020 12:53:26 -0800 Subject: [PATCH 14/22] Resolve a long-lasting TODO of auth code grant scope --- oauth2cli/oauth2.py | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/oauth2cli/oauth2.py b/oauth2cli/oauth2.py index efdd5eb0..9a947390 100644 --- a/oauth2cli/oauth2.py +++ b/oauth2cli/oauth2.py @@ -333,7 +333,7 @@ def parse_auth_response(params, state=None): return params def obtain_token_by_authorization_code( - self, code, redirect_uri=None, **kwargs): + self, code, redirect_uri=None, scope=None, **kwargs): """Get a token via auhtorization code. a.k.a. Authorization Code Grant. This is typically used by a server-side app (Confidential Client), @@ -344,9 +344,15 @@ def obtain_token_by_authorization_code( :param redirect_uri: Required, if the "redirect_uri" parameter was included in the authorization request, and their values MUST be identical. + :param scope: + It is both unnecessary and harmless to use scope here, per RFC 6749. + We suggest to use the same scope already used in auth request uri, + so that this library can link the obtained tokens with their scope. """ data = kwargs.pop("data", {}) data.update(code=code, redirect_uri=redirect_uri) + if scope: + data["scope"] = scope if not self.client_secret: # client_id is required, if the client is not authenticating itself. # See https://tools.ietf.org/html/rfc6749#section-4.1.3 @@ -400,7 +406,9 @@ def _obtain_token(self, grant_type, params=None, data=None, *args, **kwargs): scope = _resp["scope"].split() # It is conceptually a set, # but we represent it as a list which can be persisted to JSON else: - # TODO: Deal with absent scope in authorization grant + # Note: The scope will generally be absent in authorization grant, + # but our obtain_token_by_authorization_code(...) encourages + # app developer to still explicitly provide a scope here. scope = _data.get("scope") self.on_obtaining_tokens({ "client_id": self.client_id, From 775e8b51fce14c4d4757bcaa2bb5b8a4dc161e2c Mon Sep 17 00:00:00 2001 From: Ray Luo Date: Tue, 14 Jan 2020 13:10:29 -0800 Subject: [PATCH 15/22] Minor refactor based on underlying changes --- msal/application.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/msal/application.py b/msal/application.py index 20a77525..d053877c 100644 --- a/msal/application.py +++ b/msal/application.py @@ -320,9 +320,7 @@ def acquire_token_by_authorization_code( self._validate_ssh_cert_input_data(kwargs.get("data", {})) return self.client.obtain_token_by_authorization_code( code, redirect_uri=redirect_uri, - data=dict( - kwargs.pop("data", {}), - scope=decorate_scope(scopes, self.client_id)), + scope=decorate_scope(scopes, self.client_id), headers={ CLIENT_REQUEST_ID: _get_new_correlation_id(), CLIENT_CURRENT_TELEMETRY: _build_current_telemetry_request_header( From ab48adf9fea49a9d61c08d7b9d8ddf2181ac867b Mon Sep 17 00:00:00 2001 From: Ray Luo Date: Mon, 25 Nov 2019 15:56:48 -0800 Subject: [PATCH 16/22] Tolerate Authorization Server not granting all scopes --- tests/test_e2e.py | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/tests/test_e2e.py b/tests/test_e2e.py index 37f47d77..3b50ac51 100644 --- a/tests/test_e2e.py +++ b/tests/test_e2e.py @@ -52,12 +52,18 @@ def assertCacheWorksForUser(self, result_from_wire, scope, username=None): accounts = self.app.get_accounts(username=username) self.assertNotEqual(0, len(accounts)) account = accounts[0] - # Going to test acquire_token_silent(...) to locate an AT from cache - result_from_cache = self.app.acquire_token_silent(scope, account=account) - self.assertIsNotNone(result_from_cache) - self.assertEqual( - result_from_wire['access_token'], result_from_cache['access_token'], - "We should get a cached AT") + if ("scope" not in result_from_wire # This is the usual case + or # Authority server could reject some scopes + set(scope) <= set(result_from_wire["scope"].split(" ")) + ): + # Going to test acquire_token_silent(...) to locate an AT from cache + result_from_cache = self.app.acquire_token_silent(scope, account=account) + self.assertIsNotNone(result_from_cache) + self.assertIsNone( + result_from_cache.get("refresh_token"), "A cache hit returns no RT") + self.assertEqual( + result_from_wire['access_token'], result_from_cache['access_token'], + "We should get a cached AT") # Going to test acquire_token_silent(...) to obtain an AT by a RT from cache self.app.token_cache._cache["AccessToken"] = {} # A hacky way to clear ATs From 419ea702d8972d9d3ef83a0ac27072540b044e7f Mon Sep 17 00:00:00 2001 From: Ray Luo Date: Mon, 18 Nov 2019 13:41:03 -0800 Subject: [PATCH 17/22] ADFS 2019 on-prem test cases --- tests/test_e2e.py | 45 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 45 insertions(+) diff --git a/tests/test_e2e.py b/tests/test_e2e.py index 3b50ac51..b4d34e5d 100644 --- a/tests/test_e2e.py +++ b/tests/test_e2e.py @@ -308,6 +308,10 @@ def get_session(lab_app, scopes): # BTW, this infrastructure tests the confiden class LabBasedTestCase(E2eTestCase): _secrets = {} + adfs2019_scopes = ["placeholder"] # Need this to satisfy MSAL API surface. + # Internally, MSAL will also append more scopes like "openid" etc.. + # ADFS 2019 will issue tokens for valid scope only, by default "openid". + # https://docs.microsoft.com/en-us/windows-server/identity/ad-fs/overview/ad-fs-faq#what-permitted-scopes-are-supported-by-ad-fs @classmethod def setUpClass(cls): @@ -366,6 +370,47 @@ def test_adfs2019_fed_user(self): self._test_username_password( password=self.get_lab_user_secret(config["lab_name"]), **config) + def test_ropc_adfs2019_onprem(self): + config = self.get_lab_user(usertype="onprem", federationProvider="ADFSv2019") + config["authority"] = "https://fs.%s.com/adfs" % config["lab_name"] + config["client_id"] = "PublicClientId" + config["scope"] = self.adfs2019_scopes + self._test_username_password( + password=self.get_lab_user_secret(config["lab_name"]), **config) + + @unittest.skipIf(os.getenv("TRAVIS"), "Browser automation is not yet implemented") + def test_adfs2019_onprem_acquire_token_by_auth_code(self): + """When prompted, you can manually login using this account: + + # https://msidlab.com/api/user?usertype=onprem&federationprovider=ADFSv2019 + username = "..." # The upn from the link above + password="***" # From https://aka.ms/GetLabUserSecret?Secret=msidlabXYZ + """ + scopes = self.adfs2019_scopes + config = self.get_lab_user(usertype="onprem", federationProvider="ADFSv2019") + (self.app, ac, redirect_uri) = _get_app_and_auth_code( + # Configuration is derived from https://github.com/AzureAD/microsoft-authentication-library-for-dotnet/blob/4.7.0/tests/Microsoft.Identity.Test.Common/TestConstants.cs#L250-L259 + "PublicClientId", + authority="https://fs.%s.com/adfs" % config["lab_name"], + port=8080, + scopes=scopes, + ) + result = self.app.acquire_token_by_authorization_code( + ac, scopes, redirect_uri=redirect_uri) + logger.debug( + "%s: cache = %s, id_token_claims = %s", + self.id(), + json.dumps(self.app.token_cache._cache, indent=4), + json.dumps(result.get("id_token_claims"), indent=4), + ) + self.assertIn( + "access_token", result, + "{error}: {error_description}".format( + # Note: No interpolation here, cause error won't always present + error=result.get("error"), + error_description=result.get("error_description"))) + self.assertCacheWorksForUser(result, scopes, username=None) + @unittest.skipUnless( os.getenv("OBO_CLIENT_SECRET"), "Need OBO_CLIENT_SECRET from https://buildautomation.vault.azure.net/secrets/IdentityDivisionDotNetOBOServiceSecret") From edc1c58cb5585ddd130757d53e8c57106cd9b8cb Mon Sep 17 00:00:00 2001 From: Ray Luo Date: Wed, 15 Jan 2020 10:58:49 -0800 Subject: [PATCH 18/22] Add a new acquire_token_silent_with_error() --- msal/application.py | 52 ++++++++++++++++++++++++++++++++++----- tests/test_application.py | 43 ++++++++++++++++++++++++++++++++ 2 files changed, 89 insertions(+), 6 deletions(-) diff --git a/msal/application.py b/msal/application.py index 593d957d..a25858f8 100644 --- a/msal/application.py +++ b/msal/application.py @@ -431,10 +431,40 @@ def acquire_token_silent( **kwargs): """Acquire an access token for given account, without user interaction. + It behaves same as :func:`~acquire_token_silent_with_error`, + except that this method will combine the cache empty and refresh error + into one return value, `None`. + If your app does not need to care the exact token refresh error during + token cache look-up, then this method is easier to use. + + Internally, this method calls :func:`~acquire_token_silent_with_error`. + + :return: + - A dict containing no "error" key, + and typically contains an "access_token" key, + if cache lookup succeeded. + - None when cache lookup does not yield a token. + """ + result = self.acquire_token_silent_with_error( + scopes, account, authority, force_refresh, **kwargs) + return result if result and "error" not in result else None + + def acquire_token_silent_with_error( + self, + scopes, # type: List[str] + account, # type: Optional[Account] + authority=None, # See get_authorization_request_url() + force_refresh=False, # type: Optional[boolean] + **kwargs): + """Acquire an access token for given account, without user interaction. + It is done either by finding a valid access token from cache, or by finding a valid refresh token from cache and then automatically use it to redeem a new access token. + Unlike :func:`~acquire_token_silent`, + error happened during token refresh would also be returned. + :param list[str] scopes: (Required) Scopes requested to access a protected API (a resource). :param account: @@ -444,8 +474,11 @@ def acquire_token_silent( If True, it will skip Access Token look-up, and try to find a Refresh Token to obtain a new Access Token. :return: - - A dict containing "access_token" key, when cache lookup succeeds. - - None when cache lookup does not yield anything. + - A dict containing no "error" key, + and typically contains an "access_token" key, + if cache lookup succeeded. + - None when there is simply no token in the cache. + - A dict containing an "error" key, when token refresh failed. """ assert isinstance(scopes, list), "Invalid parameter type" self._validate_ssh_cert_input_data(kwargs.get("data", {})) @@ -460,8 +493,9 @@ def acquire_token_silent( scopes, account, self.authority, force_refresh=force_refresh, correlation_id=correlation_id, **kwargs) - if result: + if result and "error" not in result: return result + final_result = result for alias in self._get_authority_aliases(self.authority.instance): the_authority = Authority( "https://" + alias + "/" + self.authority.tenant, @@ -472,7 +506,10 @@ def acquire_token_silent( correlation_id=correlation_id, **kwargs) if result: - return result + if "error" not in result: + return result + final_result = result + return final_result def _acquire_token_silent_from_cache_and_possibly_refresh_it( self, @@ -533,13 +570,13 @@ def _acquire_token_silent_by_finding_rt_belongs_to_me_or_my_family( # https://msazure.visualstudio.com/One/_git/ESTS-Docs/pullrequest/1138595 "client_mismatch" in response.get("error_additional_info", []), **kwargs) - if at: + if at and "error" not in at: return at if app_metadata.get("family_id"): # Meaning this app belongs to this family at = self._acquire_token_silent_by_finding_specific_refresh_token( authority, scopes, dict(query, family_id=app_metadata["family_id"]), **kwargs) - if at: + if at and "error" not in at: return at # Either this app is an orphan, so we will naturally use its own RT; # or all attempts above have failed, so we fall back to non-foci behavior. @@ -562,6 +599,8 @@ def _acquire_token_silent_by_finding_specific_refresh_token( query=query) logger.debug("Found %d RTs matching %s", len(matches), query) client = self._build_client(self.client_credential, authority) + + response = None # A distinguishable value to mean cache is empty for entry in matches: logger.debug("Cache attempts an RT") response = client.obtain_token_by_refresh_token( @@ -582,6 +621,7 @@ def _acquire_token_silent_by_finding_specific_refresh_token( )) if break_condition(response): break + return response # Returns the latest error (if any), or just None def _validate_ssh_cert_input_data(self, data): if data.get("token_type") == "ssh-cert": diff --git a/tests/test_application.py b/tests/test_application.py index 814f7c71..1973df51 100644 --- a/tests/test_application.py +++ b/tests/test_application.py @@ -46,6 +46,49 @@ def test_extract_multiple_tag_enclosed_certs(self): self.assertEqual(["my_cert1", "my_cert2"], extract_certs(pem)) +class TestClientApplicationAcquireTokenSilentErrorBehaviors(unittest.TestCase): + + def setUp(self): + self.authority_url = "https://login.microsoftonline.com/common" + self.authority = msal.authority.Authority(self.authority_url) + self.scopes = ["s1", "s2"] + self.uid = "my_uid" + self.utid = "my_utid" + self.account = {"home_account_id": "{}.{}".format(self.uid, self.utid)} + self.rt = "this is a rt" + self.cache = msal.SerializableTokenCache() + self.client_id = "my_app" + self.cache.add({ # Pre-populate the cache + "client_id": self.client_id, + "scope": self.scopes, + "token_endpoint": "{}/oauth2/v2.0/token".format(self.authority_url), + "response": TokenCacheTestCase.build_response( + access_token="an expired AT to trigger refresh", expires_in=-99, + uid=self.uid, utid=self.utid, refresh_token=self.rt), + }) # The add(...) helper populates correct home_account_id for future searching + self.app = ClientApplication( + self.client_id, authority=self.authority_url, token_cache=self.cache) + + def test_cache_empty_will_be_returned_as_None(self): + self.assertEqual( + None, self.app.acquire_token_silent(['cache_miss'], self.account)) + self.assertEqual( + None, self.app.acquire_token_silent_with_error(['cache_miss'], self.account)) + + def test_acquire_token_silent_with_error_will_return_error(self): + error_response = {"error": "invalid_grant", "error_description": "xyz"} + def tester(url, **kwargs): + return Mock(status_code=400, json=Mock(return_value=error_response)) + self.assertEqual(error_response, self.app.acquire_token_silent_with_error( + self.scopes, self.account, post=tester)) + + def test_acquire_token_silent_will_suppress_error(self): + error_response = {"error": "invalid_grant", "error_description": "xyz"} + def tester(url, **kwargs): + return Mock(status_code=400, json=Mock(return_value=error_response)) + self.assertEqual(None, self.app.acquire_token_silent( + self.scopes, self.account, post=tester)) + class TestClientApplicationAcquireTokenSilentFociBehaviors(unittest.TestCase): def setUp(self): From aeb11dd661c6c999583f13c82d5ac507d28a1662 Mon Sep 17 00:00:00 2001 From: Ray Luo Date: Wed, 15 Jan 2020 10:59:14 -0800 Subject: [PATCH 19/22] Map suberror into classification --- msal/application.py | 8 ++++++++ tests/test_application.py | 24 ++++++++++++++++++++---- 2 files changed, 28 insertions(+), 4 deletions(-) diff --git a/msal/application.py b/msal/application.py index a25858f8..9b72affb 100644 --- a/msal/application.py +++ b/msal/application.py @@ -509,6 +509,14 @@ def acquire_token_silent_with_error( if "error" not in result: return result final_result = result + if final_result and final_result.get("suberror"): + final_result["classification"] = { # Suppress these suberrors, per #57 + "bad_token": "", + "token_expired": "", + "protection_policy_required": "", + "client_mismatch": "", + "device_authentication_failed": "", + }.get(final_result["suberror"], final_result["suberror"]) return final_result def _acquire_token_silent_from_cache_and_possibly_refresh_it( diff --git a/tests/test_application.py b/tests/test_application.py index 1973df51..4d7c2881 100644 --- a/tests/test_application.py +++ b/tests/test_application.py @@ -75,6 +75,13 @@ def test_cache_empty_will_be_returned_as_None(self): self.assertEqual( None, self.app.acquire_token_silent_with_error(['cache_miss'], self.account)) + def test_acquire_token_silent_will_suppress_error(self): + error_response = {"error": "invalid_grant", "suberror": "xyz"} + def tester(url, **kwargs): + return Mock(status_code=400, json=Mock(return_value=error_response)) + self.assertEqual(None, self.app.acquire_token_silent( + self.scopes, self.account, post=tester)) + def test_acquire_token_silent_with_error_will_return_error(self): error_response = {"error": "invalid_grant", "error_description": "xyz"} def tester(url, **kwargs): @@ -82,12 +89,21 @@ def tester(url, **kwargs): self.assertEqual(error_response, self.app.acquire_token_silent_with_error( self.scopes, self.account, post=tester)) - def test_acquire_token_silent_will_suppress_error(self): - error_response = {"error": "invalid_grant", "error_description": "xyz"} + def test_atswe_will_map_some_suberror_to_classification_as_is(self): + error_response = {"error": "invalid_grant", "suberror": "basic_action"} def tester(url, **kwargs): return Mock(status_code=400, json=Mock(return_value=error_response)) - self.assertEqual(None, self.app.acquire_token_silent( - self.scopes, self.account, post=tester)) + result = self.app.acquire_token_silent_with_error( + self.scopes, self.account, post=tester) + self.assertEqual("basic_action", result.get("classification")) + + def test_atswe_will_map_some_suberror_to_classification_to_empty_string(self): + error_response = {"error": "invalid_grant", "suberror": "client_mismatch"} + def tester(url, **kwargs): + return Mock(status_code=400, json=Mock(return_value=error_response)) + result = self.app.acquire_token_silent_with_error( + self.scopes, self.account, post=tester) + self.assertEqual("", result.get("classification")) class TestClientApplicationAcquireTokenSilentFociBehaviors(unittest.TestCase): From 23c2ed5db43f5f4de5af8117ac61c0d1b0ae5c86 Mon Sep 17 00:00:00 2001 From: Ray Luo Date: Fri, 17 Jan 2020 14:09:24 -0800 Subject: [PATCH 20/22] Adjust API docs based on off-line feedback --- msal/application.py | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/msal/application.py b/msal/application.py index 9b72affb..e5b6bbfb 100644 --- a/msal/application.py +++ b/msal/application.py @@ -431,11 +431,14 @@ def acquire_token_silent( **kwargs): """Acquire an access token for given account, without user interaction. - It behaves same as :func:`~acquire_token_silent_with_error`, - except that this method will combine the cache empty and refresh error + It is done either by finding a valid access token from cache, + or by finding a valid refresh token from cache and then automatically + use it to redeem a new access token. + + This method will combine the cache empty and refresh error into one return value, `None`. - If your app does not need to care the exact token refresh error during - token cache look-up, then this method is easier to use. + If your app does not care about the exact token refresh error during + token cache look-up, then this method is easier and recommended. Internally, this method calls :func:`~acquire_token_silent_with_error`. @@ -462,8 +465,10 @@ def acquire_token_silent_with_error( or by finding a valid refresh token from cache and then automatically use it to redeem a new access token. - Unlike :func:`~acquire_token_silent`, - error happened during token refresh would also be returned. + This method will differentiate cache empty from token refresh error. + If your app cares the exact token refresh error during + token cache look-up, then this method is suitable. + Otherwise, the other method :func:`~acquire_token_silent` is recommended. :param list[str] scopes: (Required) Scopes requested to access a protected API (a resource). From cc23e4a6b48e9978ae38ae9b3a48822e05023ca6 Mon Sep 17 00:00:00 2001 From: Ray Luo Date: Thu, 23 Jan 2020 10:05:05 -0800 Subject: [PATCH 21/22] Switch from unknown.host to example.com https://unknown.host happened to be available but not any more. It would cause our test cases to hang indefinitely. This change switches it to a well-known existing example.com Eventually we might mock it out. --- tests/test_authority.py | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/tests/test_authority.py b/tests/test_authority.py index 340b4936..d1e75ef7 100644 --- a/tests/test_authority.py +++ b/tests/test_authority.py @@ -29,12 +29,18 @@ def test_lessknown_host_will_return_a_set_of_v1_endpoints(self): self.assertNotIn('v2.0', a.token_endpoint) def test_unknown_host_wont_pass_instance_discovery(self): - with self.assertRaisesRegexp(ValueError, "invalid_instance"): - Authority('https://unknown.host/tenant_doesnt_matter_in_this_case') - - def test_invalid_host_skipping_validation_meets_connection_error_down_the_road(self): - with self.assertRaises(requests.exceptions.RequestException): - Authority('https://unknown.host/invalid', validate_authority=False) + _assert = getattr(self, "assertRaisesRegex", self.assertRaisesRegexp) # Hack + with _assert(ValueError, "invalid_instance"): + Authority('https://example.com/tenant_doesnt_matter_in_this_case') + + def test_invalid_host_skipping_validation_can_be_turned_off(self): + try: + Authority('https://example.com/invalid', validate_authority=False) + except ValueError as e: + if "invalid_instance" in str(e): # Imprecise but good enough + self.fail("validate_authority=False should turn off validation") + except: # Could be requests...RequestException, json...JSONDecodeError, etc. + pass # Those are expected for this unittest case class TestAuthorityInternalHelperCanonicalize(unittest.TestCase): From a08970748310e578c9992ebd3710167e356cf927 Mon Sep 17 00:00:00 2001 From: Ray Luo Date: Thu, 23 Jan 2020 10:37:03 -0800 Subject: [PATCH 22/22] MSAL Python 1.1.0 Bump version number --- msal/application.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/msal/application.py b/msal/application.py index e5b6bbfb..01f25461 100644 --- a/msal/application.py +++ b/msal/application.py @@ -19,7 +19,7 @@ # The __init__.py will import this. Not the other way around. -__version__ = "1.0.0" +__version__ = "1.1.0" logger = logging.getLogger(__name__)