From e110a63a6fca5b1de12beb4a629cad74d82a8cf4 Mon Sep 17 00:00:00 2001 From: Ray Luo Date: Fri, 15 Jan 2021 00:37:02 -0800 Subject: [PATCH 01/19] Be compatible with PyJWT 1 & 2 --- oauth2cli/assertion.py | 12 +++++++++++- oauth2cli/oauth2.py | 8 +++++--- setup.py | 4 ++-- 3 files changed, 18 insertions(+), 6 deletions(-) diff --git a/oauth2cli/assertion.py b/oauth2cli/assertion.py index f8d3f16f..0cf58799 100644 --- a/oauth2cli/assertion.py +++ b/oauth2cli/assertion.py @@ -9,6 +9,15 @@ logger = logging.getLogger(__name__) + +def _str2bytes(raw): + # A conversion based on duck-typing rather than six.text_type + try: # Assuming it is a string + return raw.encode(encoding="utf-8") + except: # Otherwise we treat it as bytes and return it as-is + return raw + + class AssertionCreator(object): def create_normal_assertion( self, audience, issuer, subject, expires_at=None, expires_in=600, @@ -103,8 +112,9 @@ def create_normal_assertion( payload['nbf'] = not_before payload.update(additional_claims or {}) try: - return jwt.encode( + str_or_bytes = jwt.encode( # PyJWT 1 returns bytes, PyJWT 2 returns str payload, self.key, algorithm=self.algorithm, headers=self.headers) + return _str2bytes(str_or_bytes) # We normalize them into bytes except: if self.algorithm.startswith("RS") or self.algorithm.starswith("ES"): logger.exception( diff --git a/oauth2cli/oauth2.py b/oauth2cli/oauth2.py index 07432364..fd764082 100644 --- a/oauth2cli/oauth2.py +++ b/oauth2cli/oauth2.py @@ -99,8 +99,8 @@ def __init__( client_secret (str): Triggers HTTP AUTH for Confidential Client client_assertion (bytes, callable): The client assertion to authenticate this client, per RFC 7521. - It can be a raw SAML2 assertion (this method will encode it for you), - or a raw JWT assertion. + It can be a raw SAML2 assertion (we will base64 encode it for you), + or a raw JWT assertion in bytes (which we will relay to http layer). It can also be a callable (recommended), so that we will do lazy creation of an assertion. client_assertion_type (str): @@ -198,7 +198,9 @@ def _obtain_token( # The verb "obtain" is influenced by OAUTH2 RFC 6749 self.default_body["client_assertion_type"], lambda a: a) _data["client_assertion"] = encoder( self.client_assertion() # Do lazy on-the-fly computation - if callable(self.client_assertion) else self.client_assertion) + if callable(self.client_assertion) else self.client_assertion + ) # The type is bytes, which is preferrable. See also: + # https://github.com/psf/requests/issues/4503#issuecomment-455001070 _data.update(self.default_body) # It may contain authen parameters _data.update(data or {}) # So the content in data param prevails diff --git a/setup.py b/setup.py index ff96ce61..a849b87b 100644 --- a/setup.py +++ b/setup.py @@ -30,7 +30,7 @@ ], packages=['oauth2cli'], install_requires=[ - 'requests>=2.0.0', - 'PyJWT>=1.0.0', + 'requests>=2.0.0,<3', + 'PyJWT>=1.0.0,<3', ] ) From 259ecb1cb8ad6775427262dd5f4d4f5b107bd46b Mon Sep 17 00:00:00 2001 From: Ray Luo Date: Tue, 19 Jan 2021 23:54:52 -0800 Subject: [PATCH 02/19] The ssh-cert scope needs to be updated Switch to the new SSH cert scope --- tests/test_e2e.py | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/tests/test_e2e.py b/tests/test_e2e.py index adfe5d42..ab93eff2 100644 --- a/tests/test_e2e.py +++ b/tests/test_e2e.py @@ -153,13 +153,13 @@ def test_username_password(self): self.skipUnlessWithConfig(["client_id", "username", "password", "scope"]) self._test_username_password(**self.config) - def _get_app_and_auth_code(self, **kwargs): + def _get_app_and_auth_code(self, scopes=None, **kwargs): return _get_app_and_auth_code( self.config["client_id"], client_secret=self.config.get("client_secret"), authority=self.config.get("authority"), port=self.config.get("listen_port", 44331), - scopes=self.config["scope"], + scopes=scopes or self.config["scope"], **kwargs) def _test_auth_code(self, auth_kwargs, token_kwargs): @@ -202,11 +202,15 @@ def test_ssh_cert(self): "sshcrt": "true", } - (self.app, ac, redirect_uri) = self._get_app_and_auth_code() + scopes = [ # Only this scope would result in an SSH-Cert + "https://pas.windows.net/CheckMyAccess/Linux/user_impersonation"] + (self.app, ac, redirect_uri) = self._get_app_and_auth_code(scopes=scopes) result = self.app.acquire_token_by_authorization_code( - ac, self.config["scope"], redirect_uri=redirect_uri, data=data1, + ac, scopes, redirect_uri=redirect_uri, data=data1, params=ssh_test_slice) + self.assertIsNotNone(result.get("access_token"), "Encountered {}: {}".format( + result.get("error"), result.get("error_description"))) self.assertEqual("ssh-cert", result["token_type"]) logger.debug("%s.cache = %s", self.id(), json.dumps(self.app.token_cache._cache, indent=4)) @@ -214,7 +218,7 @@ def test_ssh_cert(self): # acquire_token_silent() needs to be passed the same key to work account = self.app.get_accounts()[0] result_from_cache = self.app.acquire_token_silent( - self.config["scope"], account=account, data=data1) + scopes, account=account, data=data1) self.assertIsNotNone(result_from_cache) self.assertEqual( result['access_token'], result_from_cache['access_token'], @@ -222,7 +226,7 @@ def test_ssh_cert(self): # refresh_token grant can fetch an ssh-cert bound to a different key refreshed_ssh_cert = self.app.acquire_token_silent( - self.config["scope"], account=account, params=ssh_test_slice, + scopes, account=account, params=ssh_test_slice, data={"token_type": "ssh-cert", "key_id": "key2", "req_cnf": JWK2}) self.assertIsNotNone(refreshed_ssh_cert) self.assertEqual(refreshed_ssh_cert["token_type"], "ssh-cert") From 5490182134ffb441b38852dbb68bea717e9423ae Mon Sep 17 00:00:00 2001 From: Ray Luo Date: Mon, 25 Jan 2021 12:08:40 -0800 Subject: [PATCH 03/19] Show correlation_id when unittesting with -v param --- msal/application.py | 4 +++- tests/test_e2e.py | 3 ++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/msal/application.py b/msal/application.py index fcd0d072..01584ac3 100644 --- a/msal/application.py +++ b/msal/application.py @@ -56,7 +56,9 @@ def decorate_scope( CLIENT_CURRENT_TELEMETRY = 'x-client-current-telemetry' def _get_new_correlation_id(): - return str(uuid.uuid4()) + correlation_id = str(uuid.uuid4()) + logger.debug("Generates correlation_id: %s", correlation_id) + return correlation_id def _build_current_telemetry_request_header(public_api_id, force_refresh=False): diff --git a/tests/test_e2e.py b/tests/test_e2e.py index ab93eff2..3e6dc038 100644 --- a/tests/test_e2e.py +++ b/tests/test_e2e.py @@ -3,6 +3,7 @@ import json import time import unittest +import sys import requests @@ -11,7 +12,7 @@ from msal.oauth2cli import AuthCodeReceiver logger = logging.getLogger(__name__) -logging.basicConfig(level=logging.INFO) +logging.basicConfig(level=logging.DEBUG if "-v" in sys.argv else logging.INFO) def _get_app_and_auth_code( From 0b430c707189da4e1f2f128bb2035a3d0fa9d61e Mon Sep 17 00:00:00 2001 From: Ray Luo Date: Wed, 27 Jan 2021 20:02:34 -0800 Subject: [PATCH 04/19] Refactor SSH Cert test cases to represent test requirements, and officially support SSH Cert for SP --- msal/application.py | 1 + tests/test_e2e.py | 162 +++++++++++++++++++++++--------------------- 2 files changed, 85 insertions(+), 78 deletions(-) diff --git a/msal/application.py b/msal/application.py index 01584ac3..6ee6908c 100644 --- a/msal/application.py +++ b/msal/application.py @@ -1235,6 +1235,7 @@ def acquire_token_for_client(self, scopes, claims_challenge=None, **kwargs): - an error response would contain "error" and usually "error_description". """ # TBD: force_refresh behavior + self._validate_ssh_cert_input_data(kwargs.get("data", {})) return self.client.obtain_token_for_client( scope=scopes, # This grant flow requires no scope decoration headers={ diff --git a/tests/test_e2e.py b/tests/test_e2e.py index 3e6dc038..20624e63 100644 --- a/tests/test_e2e.py +++ b/tests/test_e2e.py @@ -50,7 +50,8 @@ def assertLoosely(self, response, assertion=None, error_description=response.get("error_description"))) assertion() - def assertCacheWorksForUser(self, result_from_wire, scope, username=None): + def assertCacheWorksForUser( + self, result_from_wire, scope, username=None, data=None): # You can filter by predefined username, or let end user to choose one accounts = self.app.get_accounts(username=username) self.assertNotEqual(0, len(accounts)) @@ -60,7 +61,8 @@ def assertCacheWorksForUser(self, result_from_wire, scope, username=None): 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) + result_from_cache = self.app.acquire_token_silent( + scope, account=account, data=data or {}) self.assertIsNotNone(result_from_cache) self.assertIsNone( result_from_cache.get("refresh_token"), "A cache hit returns no RT") @@ -70,7 +72,8 @@ def assertCacheWorksForUser(self, result_from_wire, scope, username=None): # 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 - result_from_cache = self.app.acquire_token_silent(scope, account=account) + result_from_cache = self.app.acquire_token_silent( + scope, account=account, data=data or {}) self.assertIsNotNone(result_from_cache, "We should get a result from acquire_token_silent(...) call") self.assertIsNotNone( @@ -132,6 +135,84 @@ def _test_device_flow( logger.info( "%s obtained tokens: %s", self.id(), json.dumps(result, indent=4)) + def _test_acquire_token_interactive( + self, client_id=None, authority=None, scope=None, port=None, + username_uri="", # But you would want to provide one + data=None, # Needed by ssh-cert feature + **ignored): + assert client_id and authority and scope + self.app = msal.PublicClientApplication( + client_id, authority=authority, http_client=MinimalHttpClient()) + result = self.app.acquire_token_interactive( + scope, + timeout=120, + port=port, + welcome_template= # This is an undocumented feature for testing + """

{id}

    +
  1. Get a username from the upn shown at here
    2. +
  2. Get its password from https://aka.ms/GetLabUserSecret?Secret=msidlabXYZ + (replace the lab name with the labName from the link above).
    3. +
  3. Sign In or Abort
    4. +
""".format(id=self.id(), username_uri=username_uri), + data=data or {}, + ) + 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, scope, username=None, data=data or {}) + return result # For further testing + + +class SshCertTestCase(E2eTestCase): + _JWK1 = """{"kty":"RSA", "n":"2tNr73xwcj6lH7bqRZrFzgSLj7OeLfbn8216uOMDHuaZ6TEUBDN8Uz0ve8jAlKsP9CQFCSVoSNovdE-fs7c15MxEGHjDcNKLWonznximj8pDGZQjVdfK-7mG6P6z-lgVcLuYu5JcWU_PeEqIKg5llOaz-qeQ4LEDS4T1D2qWRGpAra4rJX1-kmrWmX_XIamq30C9EIO0gGuT4rc2hJBWQ-4-FnE1NXmy125wfT3NdotAJGq5lMIfhjfglDbJCwhc8Oe17ORjO3FsB5CLuBRpYmP7Nzn66lRY3Fe11Xz8AEBl3anKFSJcTvlMnFtu3EpD-eiaHfTgRBU7CztGQqVbiQ", "e":"AQAB"}""" + _JWK2 = """{"kty":"RSA", "n":"72u07mew8rw-ssw3tUs9clKstGO2lvD7ZNxJU7OPNKz5PGYx3gjkhUmtNah4I4FP0DuF1ogb_qSS5eD86w10Wb1ftjWcoY8zjNO9V3ph-Q2tMQWdDW5kLdeU3-EDzc0HQeou9E0udqmfQoPbuXFQcOkdcbh3eeYejs8sWn3TQprXRwGh_TRYi-CAurXXLxQ8rp-pltUVRIr1B63fXmXhMeCAGwCPEFX9FRRs-YHUszUJl9F9-E0nmdOitiAkKfCC9LhwB9_xKtjmHUM9VaEC9jWOcdvXZutwEoW2XPMOg0Ky-s197F9rfpgHle2gBrXsbvVMvS0D-wXg6vsq6BAHzQ", "e":"AQAB"}""" + DATA1 = {"token_type": "ssh-cert", "key_id": "key1", "req_cnf": _JWK1} + DATA2 = {"token_type": "ssh-cert", "key_id": "key2", "req_cnf": _JWK2} + _SCOPE_USER = ["https://pas.windows.net/CheckMyAccess/Linux/user_impersonation"] + _SCOPE_SP = ["https://pas.windows.net/CheckMyAccess/Linux/.default"] + SCOPE = _SCOPE_SP # Historically there was a separation, at 2021 it is unified + + def test_ssh_cert_for_service_principal(self): + # Any SP can obtain an ssh-cert. Here we use the lab app. + result = get_lab_app().acquire_token_for_client(self.SCOPE, data=self.DATA1) + self.assertIsNotNone(result.get("access_token"), "Encountered {}: {}".format( + result.get("error"), result.get("error_description"))) + self.assertEqual("ssh-cert", result["token_type"]) + + @unittest.skipIf(os.getenv("TRAVIS"), "Browser automation is not yet implemented") + def test_ssh_cert_for_user(self): + result = self._test_acquire_token_interactive( + client_id="04b07795-8ddb-461a-bbee-02f9e1bf7b46", # Azure CLI is one + # of the only 2 clients that are PreAuthz to use ssh cert feature + authority="https://login.microsoftonline.com/common", + scope=self.SCOPE, + data=self.DATA1, + username_uri="https://msidlab.com/api/user?usertype=cloud", + ) # It already tests reading AT from cache, and using RT to refresh + # acquire_token_silent() would work because we pass in the same key + self.assertIsNotNone(result.get("access_token"), "Encountered {}: {}".format( + result.get("error"), result.get("error_description"))) + self.assertEqual("ssh-cert", result["token_type"]) + logger.debug("%s.cache = %s", + self.id(), json.dumps(self.app.token_cache._cache, indent=4)) + + # refresh_token grant can fetch an ssh-cert bound to a different key + account = self.app.get_accounts()[0] + refreshed_ssh_cert = self.app.acquire_token_silent( + self.SCOPE, account=account, data=self.DATA2) + self.assertIsNotNone(refreshed_ssh_cert) + self.assertEqual(refreshed_ssh_cert["token_type"], "ssh-cert") + self.assertNotEqual(result["access_token"], refreshed_ssh_cert['access_token']) + THIS_FOLDER = os.path.dirname(__file__) CONFIG = os.path.join(THIS_FOLDER, "config.json") @@ -191,48 +272,6 @@ def test_auth_code_with_mismatching_nonce(self): self.app.acquire_token_by_authorization_code( ac, self.config["scope"], redirect_uri=redirect_uri, nonce="bar") - def test_ssh_cert(self): - self.skipUnlessWithConfig(["client_id", "scope"]) - - JWK1 = """{"kty":"RSA", "n":"2tNr73xwcj6lH7bqRZrFzgSLj7OeLfbn8216uOMDHuaZ6TEUBDN8Uz0ve8jAlKsP9CQFCSVoSNovdE-fs7c15MxEGHjDcNKLWonznximj8pDGZQjVdfK-7mG6P6z-lgVcLuYu5JcWU_PeEqIKg5llOaz-qeQ4LEDS4T1D2qWRGpAra4rJX1-kmrWmX_XIamq30C9EIO0gGuT4rc2hJBWQ-4-FnE1NXmy125wfT3NdotAJGq5lMIfhjfglDbJCwhc8Oe17ORjO3FsB5CLuBRpYmP7Nzn66lRY3Fe11Xz8AEBl3anKFSJcTvlMnFtu3EpD-eiaHfTgRBU7CztGQqVbiQ", "e":"AQAB"}""" - JWK2 = """{"kty":"RSA", "n":"72u07mew8rw-ssw3tUs9clKstGO2lvD7ZNxJU7OPNKz5PGYx3gjkhUmtNah4I4FP0DuF1ogb_qSS5eD86w10Wb1ftjWcoY8zjNO9V3ph-Q2tMQWdDW5kLdeU3-EDzc0HQeou9E0udqmfQoPbuXFQcOkdcbh3eeYejs8sWn3TQprXRwGh_TRYi-CAurXXLxQ8rp-pltUVRIr1B63fXmXhMeCAGwCPEFX9FRRs-YHUszUJl9F9-E0nmdOitiAkKfCC9LhwB9_xKtjmHUM9VaEC9jWOcdvXZutwEoW2XPMOg0Ky-s197F9rfpgHle2gBrXsbvVMvS0D-wXg6vsq6BAHzQ", "e":"AQAB"}""" - data1 = {"token_type": "ssh-cert", "key_id": "key1", "req_cnf": JWK1} - ssh_test_slice = { - "dc": "prod-wst-test1", - "slice": "test", - "sshcrt": "true", - } - - scopes = [ # Only this scope would result in an SSH-Cert - "https://pas.windows.net/CheckMyAccess/Linux/user_impersonation"] - (self.app, ac, redirect_uri) = self._get_app_and_auth_code(scopes=scopes) - - result = self.app.acquire_token_by_authorization_code( - ac, scopes, redirect_uri=redirect_uri, data=data1, - params=ssh_test_slice) - self.assertIsNotNone(result.get("access_token"), "Encountered {}: {}".format( - result.get("error"), result.get("error_description"))) - self.assertEqual("ssh-cert", result["token_type"]) - logger.debug("%s.cache = %s", - self.id(), json.dumps(self.app.token_cache._cache, indent=4)) - - # acquire_token_silent() needs to be passed the same key to work - account = self.app.get_accounts()[0] - result_from_cache = self.app.acquire_token_silent( - scopes, account=account, data=data1) - self.assertIsNotNone(result_from_cache) - self.assertEqual( - result['access_token'], result_from_cache['access_token'], - "We should get the cached SSH-cert") - - # refresh_token grant can fetch an ssh-cert bound to a different key - refreshed_ssh_cert = self.app.acquire_token_silent( - scopes, account=account, params=ssh_test_slice, - data={"token_type": "ssh-cert", "key_id": "key2", "req_cnf": JWK2}) - self.assertIsNotNone(refreshed_ssh_cert) - self.assertEqual(refreshed_ssh_cert["token_type"], "ssh-cert") - self.assertNotEqual(result["access_token"], refreshed_ssh_cert['access_token']) - def test_client_secret(self): self.skipUnlessWithConfig(["client_id", "client_secret"]) self.app = msal.ConfidentialClientApplication( @@ -446,39 +485,6 @@ def _test_acquire_token_by_auth_code_flow( error_description=result.get("error_description"))) self.assertCacheWorksForUser(result, scope, username=None) - def _test_acquire_token_interactive( - self, client_id=None, authority=None, scope=None, port=None, - username_uri="", # But you would want to provide one - **ignored): - assert client_id and authority and scope - self.app = msal.PublicClientApplication( - client_id, authority=authority, http_client=MinimalHttpClient()) - result = self.app.acquire_token_interactive( - scope, - timeout=60, - port=port, - welcome_template= # This is an undocumented feature for testing - """

{id}

    -
  1. Get a username from the upn shown at here
    2. -
  2. Get its password from https://aka.ms/GetLabUserSecret?Secret=msidlabXYZ - (replace the lab name with the labName from the link above).
    3. -
  3. Sign In or Abort
    4. -
""".format(id=self.id(), username_uri=username_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, scope, username=None) - def _test_acquire_token_obo(self, config_pca, config_cca): # 1. An app obtains a token representing a user, for our mid-tier service pca = msal.PublicClientApplication( From 96614ec61b9a34fc1cf55c381c0a5e43348fc9fc Mon Sep 17 00:00:00 2001 From: Ray Luo Date: Fri, 29 Jan 2021 16:22:40 -0800 Subject: [PATCH 05/19] Precise DeprecationWarning for auth code API --- msal/application.py | 56 ++++++++++++++++++++++++++------------------- 1 file changed, 32 insertions(+), 24 deletions(-) diff --git a/msal/application.py b/msal/application.py index 6ee6908c..1a5f1fec 100644 --- a/msal/application.py +++ b/msal/application.py @@ -441,16 +441,20 @@ def get_authorization_request_url( {"authorization_endpoint": the_authority.authorization_endpoint}, self.client_id, http_client=self.http_client) - return client.build_auth_request_uri( - response_type=response_type, - redirect_uri=redirect_uri, state=state, login_hint=login_hint, - prompt=prompt, - scope=decorate_scope(scopes, self.client_id), - nonce=nonce, - domain_hint=domain_hint, - claims=_merge_claims_challenge_and_capabilities( - self._client_capabilities, claims_challenge), - ) + warnings.warn( + "Change your get_authorization_request_url() " + "to initiate_auth_code_flow()", DeprecationWarning) + with warnings.catch_warnings(record=True): + return client.build_auth_request_uri( + response_type=response_type, + redirect_uri=redirect_uri, state=state, login_hint=login_hint, + prompt=prompt, + scope=decorate_scope(scopes, self.client_id), + nonce=nonce, + domain_hint=domain_hint, + claims=_merge_claims_challenge_and_capabilities( + self._client_capabilities, claims_challenge), + ) def acquire_token_by_auth_code_flow( self, auth_code_flow, auth_response, scopes=None, **kwargs): @@ -572,20 +576,24 @@ def acquire_token_by_authorization_code( # really empty. assert isinstance(scopes, list), "Invalid parameter type" self._validate_ssh_cert_input_data(kwargs.get("data", {})) - return self.client.obtain_token_by_authorization_code( - code, redirect_uri=redirect_uri, - 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), - }, - data=dict( - kwargs.pop("data", {}), - claims=_merge_claims_challenge_and_capabilities( - self._client_capabilities, claims_challenge)), - nonce=nonce, - **kwargs) + warnings.warn( + "Change your acquire_token_by_authorization_code() " + "to acquire_token_by_auth_code_flow()", DeprecationWarning) + with warnings.catch_warnings(record=True): + return self.client.obtain_token_by_authorization_code( + code, redirect_uri=redirect_uri, + 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), + }, + data=dict( + kwargs.pop("data", {}), + claims=_merge_claims_challenge_and_capabilities( + self._client_capabilities, claims_challenge)), + nonce=nonce, + **kwargs) def get_accounts(self, username=None): """Get a list of accounts which previously signed in, i.e. exists in cache. From 523ed9bf5b18ff195a4da1ed9153bd44e659afe4 Mon Sep 17 00:00:00 2001 From: Ray Luo Date: Fri, 29 Jan 2021 18:16:20 -0800 Subject: [PATCH 06/19] Trying github actions Enables Python 2.7 and 3.7 only, for now --- .github/workflows/python-package.yml | 38 ++++++++++++++++++++++++++++ 1 file changed, 38 insertions(+) create mode 100644 .github/workflows/python-package.yml diff --git a/.github/workflows/python-package.yml b/.github/workflows/python-package.yml new file mode 100644 index 00000000..8e7d43e1 --- /dev/null +++ b/.github/workflows/python-package.yml @@ -0,0 +1,38 @@ +# This workflow will install Python dependencies, run tests and lint with a variety of Python versions +# For more information see: https://help.github.com/actions/language-and-framework-guides/using-python-with-github-actions + +name: Python package + +on: + push: + pull_request: + branches: [ dev ] + +jobs: + build: + + runs-on: ubuntu-latest + strategy: + matrix: + python-version: [2.7, 3.7] + + steps: + - uses: actions/checkout@v2 + - name: Set up Python ${{ matrix.python-version }} + uses: actions/setup-python@v2 + with: + python-version: ${{ matrix.python-version }} + - name: Install dependencies + run: | + python -m pip install --upgrade pip + python -m pip install flake8 pytest + if [ -f requirements.txt ]; then pip install -r requirements.txt; fi + - name: Lint with flake8 + run: | + # stop the build if there are Python syntax errors or undefined names + flake8 . --count --select=E9,F63,F7,F82 --show-source --statistics + # exit-zero treats all errors as warnings. The GitHub editor is 127 chars wide + flake8 . --count --exit-zero --max-complexity=10 --max-line-length=127 --statistics + - name: Test with pytest + run: | + pytest From 334440eb388532bfa46f9ce07d6bb5cbeda80e7b Mon Sep 17 00:00:00 2001 From: Ray Luo Date: Fri, 29 Jan 2021 18:25:30 -0800 Subject: [PATCH 07/19] Disable flake8 for now --- .github/workflows/python-package.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/python-package.yml b/.github/workflows/python-package.yml index 8e7d43e1..423b519f 100644 --- a/.github/workflows/python-package.yml +++ b/.github/workflows/python-package.yml @@ -30,9 +30,9 @@ jobs: - name: Lint with flake8 run: | # stop the build if there are Python syntax errors or undefined names - flake8 . --count --select=E9,F63,F7,F82 --show-source --statistics + #flake8 . --count --select=E9,F63,F7,F82 --show-source --statistics # exit-zero treats all errors as warnings. The GitHub editor is 127 chars wide - flake8 . --count --exit-zero --max-complexity=10 --max-line-length=127 --statistics + #flake8 . --count --exit-zero --max-complexity=10 --max-line-length=127 --statistics - name: Test with pytest run: | pytest From e30b3053eec2fff84d971127b62cd8f8f2f14d0b Mon Sep 17 00:00:00 2001 From: Ray Luo Date: Fri, 29 Jan 2021 19:01:47 -0800 Subject: [PATCH 08/19] Fake a TRAVIS env --- .github/workflows/python-package.yml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/.github/workflows/python-package.yml b/.github/workflows/python-package.yml index 423b519f..2a27f1b3 100644 --- a/.github/workflows/python-package.yml +++ b/.github/workflows/python-package.yml @@ -10,6 +10,9 @@ on: jobs: build: + env: + # Fake a TRAVIS env so that the pre-existing test cases would behave like before + TRAVIS: true runs-on: ubuntu-latest strategy: From 0b4d3d3fb17c9dd28cbd0f24d8ea88858fb37642 Mon Sep 17 00:00:00 2001 From: Ray Luo Date: Tue, 2 Feb 2021 14:40:04 -0800 Subject: [PATCH 09/19] Use env vars to enable e2e tests --- .github/workflows/python-package.yml | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/.github/workflows/python-package.yml b/.github/workflows/python-package.yml index 2a27f1b3..5015740c 100644 --- a/.github/workflows/python-package.yml +++ b/.github/workflows/python-package.yml @@ -13,6 +13,11 @@ jobs: env: # Fake a TRAVIS env so that the pre-existing test cases would behave like before TRAVIS: true + LAB_APP_CLIENT_ID: ${{ secrets.LAB_APP_CLIENT_ID }} + LAB_APP_CLIENT_SECRET: ${{ secrets.LAB_APP_CLIENT_SECRET }} + LAB_OBO_CLIENT_SECRET: ${{ secrets.LAB_OBO_CLIENT_SECRET }} + LAB_OBO_CONFIDENTIAL_CLIENT_ID: ${{ secrets.LAB_OBO_CONFIDENTIAL_CLIENT_ID }} + LAB_OBO_PUBLIC_CLIENT_ID: ${{ secrets.LAB_OBO_PUBLIC_CLIENT_ID }} runs-on: ubuntu-latest strategy: From 290bff408fe3b431e98909413aa8baab67544666 Mon Sep 17 00:00:00 2001 From: Ray Luo Date: Tue, 2 Feb 2021 16:45:44 -0800 Subject: [PATCH 10/19] Cache dependencies, although the gain is insignificant for this repo --- .github/workflows/python-package.yml | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/.github/workflows/python-package.yml b/.github/workflows/python-package.yml index 5015740c..c841b10d 100644 --- a/.github/workflows/python-package.yml +++ b/.github/workflows/python-package.yml @@ -30,6 +30,20 @@ jobs: uses: actions/setup-python@v2 with: python-version: ${{ matrix.python-version }} + + # Derived from https://github.com/actions/cache/blob/main/examples.md#using-pip-to-get-cache-location + # However, a before-and-after test shows no improvement in this repo, + # possibly because the bottlenect was not in downloading those small python deps. + - name: Get pip cache dir from pip 20.1+ + id: pip-cache + run: | + echo "::set-output name=dir::$(pip cache dir)" + - name: pip cache + uses: actions/cache@v2 + with: + path: ${{ steps.pip-cache.outputs.dir }} + key: ${{ runner.os }}-py${{ matrix.python-version }}-pip-${{ hashFiles('**/setup.py', '**/requirements.txt') }} + - name: Install dependencies run: | python -m pip install --upgrade pip From b0ff1ce199555abbb3e9a49bd233d21d0570a21c Mon Sep 17 00:00:00 2001 From: Ray Luo Date: Tue, 2 Feb 2021 17:00:05 -0800 Subject: [PATCH 11/19] Enable tests on all python versions we supported --- .github/workflows/python-package.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/python-package.yml b/.github/workflows/python-package.yml index c841b10d..bc983d46 100644 --- a/.github/workflows/python-package.yml +++ b/.github/workflows/python-package.yml @@ -22,7 +22,7 @@ jobs: runs-on: ubuntu-latest strategy: matrix: - python-version: [2.7, 3.7] + python-version: [2.7, 3.5, 3.6, 3.7, 3.8, 3.9] steps: - uses: actions/checkout@v2 From 516513861b372ab268e7d7313d55e9413fa24bb3 Mon Sep 17 00:00:00 2001 From: Ray Luo Date: Tue, 2 Feb 2021 20:40:36 -0800 Subject: [PATCH 12/19] Refine OBO test case's guidance message --- tests/test_e2e.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/test_e2e.py b/tests/test_e2e.py index 20624e63..94e8e17b 100644 --- a/tests/test_e2e.py +++ b/tests/test_e2e.py @@ -620,13 +620,13 @@ def test_adfs2019_onprem_acquire_token_interactive(self): @unittest.skipUnless( os.getenv("LAB_OBO_CLIENT_SECRET"), - "Need LAB_OBO_CLIENT SECRET from https://msidlabs.vault.azure.net/secrets/TodoListServiceV2-OBO/c58ba97c34ca4464886943a847d1db56") + "Need LAB_OBO_CLIENT_SECRET from https://aka.ms/GetLabSecret?Secret=TodoListServiceV2-OBO") @unittest.skipUnless( os.getenv("LAB_OBO_CONFIDENTIAL_CLIENT_ID"), - "Confidential client id can be found here https://docs.msidlab.com/flows/onbehalfofflow.html") + "Need LAB_OBO_CONFIDENTIAL_CLIENT_ID from https://docs.msidlab.com/flows/onbehalfofflow.html") @unittest.skipUnless( os.getenv("LAB_OBO_PUBLIC_CLIENT_ID"), - "Public client id can be found here https://docs.msidlab.com/flows/onbehalfofflow.html") + "Need LAB_OBO_PUBLIC_CLIENT_ID from https://docs.msidlab.com/flows/onbehalfofflow.html") def test_acquire_token_obo(self): config = self.get_lab_user(usertype="cloud") From 886c3f36d71fcf1d6a303ddefa16b9964f46ac15 Mon Sep 17 00:00:00 2001 From: Ray Luo Date: Tue, 2 Feb 2021 21:05:58 -0800 Subject: [PATCH 13/19] Add prompt parameter into interactive sample --- sample/interactive_sample.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/sample/interactive_sample.py b/sample/interactive_sample.py index 38593315..2e5b1cf6 100644 --- a/sample/interactive_sample.py +++ b/sample/interactive_sample.py @@ -55,12 +55,14 @@ print("A local browser window will be open for you to sign in. CTRL+C to cancel.") result = app.acquire_token_interactive( config["scope"], - login_hint=config.get("username"), # You can use this parameter to pre-fill + login_hint=config.get("username"), # Optional. + # If you know the username ahead of time, this parameter can pre-fill # the username (or email address) field of the sign-in page for the user, - # if you know the username ahead of time. # Often, apps use this parameter during reauthentication, # after already extracting the username from an earlier sign-in # by using the preferred_username claim from returned id_token_claims. + + #prompt="select_account", # Optional. It forces to show account selector page ) if "access_token" in result: From cd7537d7bb024004c365cc2f74d95844f5d01599 Mon Sep 17 00:00:00 2001 From: Jiashuo Li Date: Wed, 3 Feb 2021 13:19:58 +0800 Subject: [PATCH 14/19] Pass kwargs to acquire_token_by_refresh_token (#298) Pass kwargs to acquire_token_by_refresh_token Co-authored-by: Ray Luo --- msal/application.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/msal/application.py b/msal/application.py index 1a5f1fec..5d12e42f 100644 --- a/msal/application.py +++ b/msal/application.py @@ -952,7 +952,7 @@ def _validate_ssh_cert_input_data(self, data): "you must include a string parameter named 'key_id' " "which identifies the key in the 'req_cnf' argument.") - def acquire_token_by_refresh_token(self, refresh_token, scopes): + def acquire_token_by_refresh_token(self, refresh_token, scopes, **kwargs): """Acquire token(s) based on a refresh token (RT) obtained from elsewhere. You use this method only when you have old RTs from elsewhere, @@ -975,6 +975,7 @@ def acquire_token_by_refresh_token(self, refresh_token, scopes): * A dict contains "error" and some other keys, when error happened. * A dict contains no "error" key means migration was successful. """ + self._validate_ssh_cert_input_data(kwargs.get("data", {})) return self.client.obtain_token_by_refresh_token( refresh_token, scope=decorate_scope(scopes, self.client_id), @@ -986,7 +987,7 @@ def acquire_token_by_refresh_token(self, refresh_token, scopes): rt_getter=lambda rt: rt, on_updating_rt=False, on_removing_rt=lambda rt_item: None, # No OP - ) + **kwargs) class PublicClientApplication(ClientApplication): # browser app or mobile app @@ -1305,4 +1306,3 @@ def acquire_token_on_behalf_of(self, user_assertion, scopes, claims_challenge=No self.ACQUIRE_TOKEN_ON_BEHALF_OF_ID), }, **kwargs) - From 25ffca37153733b60a14f3157f2ceeea37d0a334 Mon Sep 17 00:00:00 2001 From: Ray Luo Date: Wed, 3 Feb 2021 20:47:02 -0800 Subject: [PATCH 15/19] Better error msg when aud and client_id mismatch --- oauth2cli/oidc.py | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/oauth2cli/oidc.py b/oauth2cli/oidc.py index eb2e80aa..75f23276 100644 --- a/oauth2cli/oidc.py +++ b/oauth2cli/oidc.py @@ -47,7 +47,7 @@ def decode_id_token(id_token, client_id=None, issuer=None, nonce=None, now=None) if _now + skew < decoded.get("nbf", _now - 1): # nbf is optional per JWT specs # This is not an ID token validation, but a JWT validation # https://tools.ietf.org/html/rfc7519#section-4.1.5 - err = "0. The ID token is not yet valid" + err = "0. The ID token is not yet valid." if issuer and issuer != decoded["iss"]: # https://openid.net/specs/openid-connect-discovery-1_0.html#ProviderConfigurationResponse err = ('2. The Issuer Identifier for the OpenID Provider, "%s", ' @@ -57,7 +57,11 @@ def decode_id_token(id_token, client_id=None, issuer=None, nonce=None, now=None) valid_aud = client_id in decoded["aud"] if isinstance( decoded["aud"], list) else client_id == decoded["aud"] if not valid_aud: - err = "3. The aud (audience) Claim must contain this client's client_id." + err = ( + "3. The aud (audience) claim must contain this client's client_id " + '"%s", case-sensitively. Was your client_id in wrong casing?' + # Some IdP accepts wrong casing request but issues right casing IDT + ) % client_id # Per specs: # 6. If the ID Token is received via direct communication between # the Client and the Token Endpoint (which it is during _obtain_token()), @@ -67,9 +71,9 @@ def decode_id_token(id_token, client_id=None, issuer=None, nonce=None, now=None) err = "9. The current time MUST be before the time represented by the exp Claim." if nonce and nonce != decoded.get("nonce"): err = ("11. Nonce must be the same value " - "as the one that was sent in the Authentication Request") + "as the one that was sent in the Authentication Request.") if err: - raise RuntimeError("%s id_token was: %s" % ( + raise RuntimeError("%s The id_token was: %s" % ( err, json.dumps(decoded, indent=2))) return decoded From 0d4b8c26272fdd4b7a8f93a13c438add889b7fe1 Mon Sep 17 00:00:00 2001 From: Ray Luo Date: Sat, 6 Feb 2021 17:10:31 -0800 Subject: [PATCH 16/19] Backport the customizable on_obtaining_tokens() --- oauth2cli/oauth2.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/oauth2cli/oauth2.py b/oauth2cli/oauth2.py index fd764082..9e30e7f4 100644 --- a/oauth2cli/oauth2.py +++ b/oauth2cli/oauth2.py @@ -734,6 +734,7 @@ def __init__(self, def _obtain_token( self, grant_type, params=None, data=None, also_save_rt=False, + on_obtaining_tokens=None, *args, **kwargs): _data = data.copy() # to prevent side effect resp = super(Client, self)._obtain_token( @@ -753,7 +754,7 @@ def _obtain_token( # 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({ + (on_obtaining_tokens or self.on_obtaining_tokens)({ "client_id": self.client_id, "scope": scope, "token_endpoint": self.configuration["token_endpoint"], @@ -767,6 +768,7 @@ def obtain_token_by_refresh_token(self, token_item, scope=None, rt_getter=lambda token_item: token_item["refresh_token"], on_removing_rt=None, on_updating_rt=None, + on_obtaining_tokens=None, **kwargs): # type: (Union[str, dict], Union[str, list, set, tuple], Callable) -> dict """This is an overload which will trigger token storage callbacks. From 770bab31cf682f30c3d4e96aa7053e484f990d84 Mon Sep 17 00:00:00 2001 From: Ray Luo Date: Sat, 6 Feb 2021 17:17:50 -0800 Subject: [PATCH 17/19] Allow auth code flow to work inside a local container --- oauth2cli/authcode.py | 53 +++++++++++++++++++++++++++++-------------- oauth2cli/oauth2.py | 2 ++ 2 files changed, 38 insertions(+), 17 deletions(-) diff --git a/oauth2cli/authcode.py b/oauth2cli/authcode.py index d1951cbe..71e3f07c 100644 --- a/oauth2cli/authcode.py +++ b/oauth2cli/authcode.py @@ -33,19 +33,9 @@ def obtain_auth_code(listen_port, auth_uri=None): # Historically only used in t ).get("code") -def _browse(auth_uri): +def _browse(auth_uri): # throws ImportError, possibly webbrowser.Error in future import webbrowser # Lazy import. Some distro may not have this. - controller = webbrowser.get() # Get a default controller - # Some Linux Distro does not setup default browser properly, - # so we try to explicitly use some popular browser, if we found any. - for browser in ["chrome", "firefox", "safari", "windows-default"]: - try: - controller = webbrowser.get(browser) - break - except webbrowser.Error: - pass # This browser is not installed. Try next one. - logger.info("Please open a browser on THIS device to visit: %s" % auth_uri) - controller.open(auth_uri) + return webbrowser.open(auth_uri) # Use default browser. Customizable by $BROWSER def _qs2kv(qs): @@ -130,14 +120,16 @@ def get_port(self): return self._server.server_address[1] def get_auth_response(self, auth_uri=None, timeout=None, state=None, - welcome_template=None, success_template=None, error_template=None): - """Wait and return the auth response, or None when timeout. + welcome_template=None, success_template=None, error_template=None, + auth_uri_callback=None, + ): + """Wait and return the auth response. Raise RuntimeError when timeout. :param str auth_uri: If provided, this function will try to open a local browser. :param int timeout: In seconds. None means wait indefinitely. :param str state: - You may provide the state you used in auth_url, + You may provide the state you used in auth_uri, then we will use it to validate incoming response. :param str welcome_template: If provided, your end user will see it instead of the auth_uri. @@ -152,6 +144,10 @@ def get_auth_response(self, auth_uri=None, timeout=None, state=None, The page will be displayed when authentication encountered error. Placeholders can be any of these: https://tools.ietf.org/html/rfc6749#section-5.2 + :param callable auth_uri_callback: + A function with the shape of lambda auth_uri: ... + When a browser was unable to be launch, this function will be called, + so that the app could tell user to manually visit the auth_uri. :return: The auth response of the first leg of Auth Code flow, typically {"code": "...", "state": "..."} or {"error": "...", ...} @@ -164,8 +160,31 @@ def get_auth_response(self, auth_uri=None, timeout=None, state=None, logger.debug("Abort by visit %s", abort_uri) self._server.welcome_page = Template(welcome_template or "").safe_substitute( auth_uri=auth_uri, abort_uri=abort_uri) - if auth_uri: - _browse(welcome_uri if welcome_template else auth_uri) + if auth_uri: # Now attempt to open a local browser to visit it + _uri = welcome_uri if welcome_template else auth_uri + logger.info("Open a browser on this device to visit: %s" % _uri) + browser_opened = False + try: + browser_opened = _browse(_uri) + except: # Had to use broad except, because the potential + # webbrowser.Error is purposely undefined outside of _browse(). + # Absorb and proceed. Because browser could be manually run elsewhere. + logger.exception("_browse(...) unsuccessful") + if not browser_opened: + if not auth_uri_callback: + logger.warning( + "Found no browser in current environment. " + "If this program is being run inside a container " + "which has access to host network " + "(i.e. started by `docker run --net=host -it ...`), " + "you can use browser on host to visit the following link. " + "Otherwise, this auth attempt would either timeout " + "(current timeout setting is {timeout}) " + "or be aborted by CTRL+C. Auth URI: {auth_uri}".format( + auth_uri=_uri, timeout=timeout)) + else: # Then it is the auth_uri_callback()'s job to inform the user + auth_uri_callback(_uri) + self._server.success_template = Template(success_template or "Authentication completed. You can close this window now.") self._server.error_template = Template(error_template or diff --git a/oauth2cli/oauth2.py b/oauth2cli/oauth2.py index 9e30e7f4..267bdc44 100644 --- a/oauth2cli/oauth2.py +++ b/oauth2cli/oauth2.py @@ -580,6 +580,7 @@ def obtain_token_by_browser( welcome_template=None, success_template=None, auth_params=None, + auth_uri_callback=None, **kwargs): """A native app can use this method to obtain token via a local browser. @@ -637,6 +638,7 @@ def obtain_token_by_browser( timeout=timeout, welcome_template=welcome_template, success_template=success_template, + auth_uri_callback=auth_uri_callback, ) except PermissionError: if 0 < listen_port < 1024: From a8bf23602f7d5b27adcc17639d65871c1fc79708 Mon Sep 17 00:00:00 2001 From: Ray Luo Date: Mon, 8 Feb 2021 23:07:01 -0800 Subject: [PATCH 18/19] MSAL Python 1.9.0 Bumping version number --- msal/application.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/msal/application.py b/msal/application.py index 5d12e42f..a1f50038 100644 --- a/msal/application.py +++ b/msal/application.py @@ -21,7 +21,7 @@ # The __init__.py will import this. Not the other way around. -__version__ = "1.8.0" +__version__ = "1.9.0" logger = logging.getLogger(__name__) From 2616d89c4a0e57826bacf49087408fed2560509c Mon Sep 17 00:00:00 2001 From: Ray Luo Date: Mon, 8 Feb 2021 23:40:56 -0800 Subject: [PATCH 19/19] We will use github actions for release --- .github/workflows/python-package.yml | 33 ++++++++++++++++++++++++++-- 1 file changed, 31 insertions(+), 2 deletions(-) diff --git a/.github/workflows/python-package.yml b/.github/workflows/python-package.yml index bc983d46..c6b90bfd 100644 --- a/.github/workflows/python-package.yml +++ b/.github/workflows/python-package.yml @@ -1,7 +1,7 @@ # This workflow will install Python dependencies, run tests and lint with a variety of Python versions # For more information see: https://help.github.com/actions/language-and-framework-guides/using-python-with-github-actions -name: Python package +name: CI/CD on: push: @@ -9,7 +9,7 @@ on: branches: [ dev ] jobs: - build: + ci: env: # Fake a TRAVIS env so that the pre-existing test cases would behave like before TRAVIS: true @@ -19,6 +19,7 @@ jobs: LAB_OBO_CONFIDENTIAL_CLIENT_ID: ${{ secrets.LAB_OBO_CONFIDENTIAL_CLIENT_ID }} LAB_OBO_PUBLIC_CLIENT_ID: ${{ secrets.LAB_OBO_PUBLIC_CLIENT_ID }} + # Derived from https://docs.github.com/en/actions/guides/building-and-testing-python#starting-with-the-python-workflow-template runs-on: ubuntu-latest strategy: matrix: @@ -58,3 +59,31 @@ jobs: - name: Test with pytest run: | pytest + + cd: + needs: ci + if: github.event_name == 'push' && (startsWith(github.ref, 'refs/tags') || github.ref == 'refs/heads/main') + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v2 + - name: Set up Python 3.9 + uses: actions/setup-python@v2 + with: + python-version: 3.9 + - name: Build a package for release + run: | + python -m pip install build --user + python -m build --sdist --wheel --outdir dist/ . + - name: Publish to TestPyPI + uses: pypa/gh-action-pypi-publish@v1.4.2 + if: github.ref == 'refs/heads/main' + with: + user: __token__ + password: ${{ secrets.TEST_PYPI_API_TOKEN }} + repository_url: https://test.pypi.org/legacy/ + - name: Publish to PyPI + if: startsWith(github.ref, 'refs/tags') + uses: pypa/gh-action-pypi-publish@v1.4.2 + with: + user: __token__ + password: ${{ secrets.PYPI_API_TOKEN }}