From cd7b38a7bb349f105008615cde27b867b04e092a Mon Sep 17 00:00:00 2001 From: Bu Sun Kim Date: Mon, 17 Jun 2019 15:50:07 -0700 Subject: [PATCH 1/6] Make code_verifier optional. --- google_auth_oauthlib/flow.py | 27 ++++++++++++--------------- 1 file changed, 12 insertions(+), 15 deletions(-) diff --git a/google_auth_oauthlib/flow.py b/google_auth_oauthlib/flow.py index d4336d7..c6bdc7b 100644 --- a/google_auth_oauthlib/flow.py +++ b/google_auth_oauthlib/flow.py @@ -108,8 +108,7 @@ def __init__( creation time. Otherwise, it will need to be set using :attr:`redirect_uri`. code_verifier (str): random string of 43-128 chars used to verify - the key exchange.using PKCE. Auto-generated if not provided. - + the key exchange.using PKCE. .. _client secrets: https://developers.google.com/api-client-library/python/guide /aaa_client_secrets @@ -160,7 +159,9 @@ def from_client_config(cls, client_config, scopes, **kwargs): client_config, scopes, **kwargs)) redirect_uri = kwargs.get('redirect_uri', None) - return cls(session, client_type, client_config, redirect_uri) + code_verifier = kwargs.get('code_verifier', None) + + return cls(session, client_type, client_config, redirect_uri, code_verifier) @classmethod def from_client_secrets_file(cls, client_secrets_file, scopes, **kwargs): @@ -217,18 +218,14 @@ def authorization_url(self, **kwargs): specify the ``state`` when constructing the :class:`Flow`. """ kwargs.setdefault('access_type', 'offline') - if not self.code_verifier: - chars = ascii_letters+digits+'-._~' - rnd = SystemRandom() - random_verifier = [rnd.choice(chars) for _ in range(0, 128)] - self.code_verifier = ''.join(random_verifier) - code_hash = hashlib.sha256() - code_hash.update(str.encode(self.code_verifier)) - unencoded_challenge = code_hash.digest() - b64_challenge = urlsafe_b64encode(unencoded_challenge) - code_challenge = b64_challenge.decode().split('=')[0] - kwargs.setdefault('code_challenge', code_challenge) - kwargs.setdefault('code_challenge_method', 'S256') + if self.code_verifier: + code_hash = hashlib.sha256() + code_hash.update(str.encode(self.code_verifier)) + unencoded_challenge = code_hash.digest() + b64_challenge = urlsafe_b64encode(unencoded_challenge) + code_challenge = b64_challenge.decode().split('=')[0] + kwargs.setdefault('code_challenge', code_challenge) + kwargs.setdefault('code_challenge_method', 'S256') url, state = self.oauth2session.authorization_url( self.client_config['auth_uri'], **kwargs) From fe76232242a9be11a9cb23beddee24afd78fa6c4 Mon Sep 17 00:00:00 2001 From: Bu Sun Kim Date: Mon, 17 Jun 2019 16:00:56 -0700 Subject: [PATCH 2/6] Update tests. --- tests/test_flow.py | 83 +++++++++++++++++++++++++++++++++++----------- 1 file changed, 64 insertions(+), 19 deletions(-) diff --git a/tests/test_flow.py b/tests/test_flow.py index c8a2390..30689c5 100644 --- a/tests/test_flow.py +++ b/tests/test_flow.py @@ -88,7 +88,6 @@ def test_redirect_uri(self, instance): def test_authorization_url(self, instance): scope = 'scope_one' instance.oauth2session.scope = [scope] - instance.code_verifier = 'amanaplanacanalpanama' authorization_url_patch = mock.patch.object( instance.oauth2session, 'authorization_url', wraps=instance.oauth2session.authorization_url) @@ -101,11 +100,9 @@ def test_authorization_url(self, instance): authorization_url_spy.assert_called_with( CLIENT_SECRETS_INFO['web']['auth_uri'], access_type='offline', - prompt='consent', - code_challenge='2yN0TOdl0gkGwFOmtfx3f913tgEaLM2d2S0WlmG1Z6Q', - code_challenge_method='S256') + prompt='consent') - def test_authorization_url_access_type(self, instance): + def test_authorization_url_code_verifier(self, instance): scope = 'scope_one' instance.oauth2session.scope = [scope] instance.code_verifier = 'amanaplanacanalpanama' @@ -114,34 +111,35 @@ def test_authorization_url_access_type(self, instance): wraps=instance.oauth2session.authorization_url) with authorization_url_patch as authorization_url_spy: - url, _ = instance.authorization_url(access_type='meep') + url, _ = instance.authorization_url(prompt='consent') assert CLIENT_SECRETS_INFO['web']['auth_uri'] in url assert scope in url authorization_url_spy.assert_called_with( CLIENT_SECRETS_INFO['web']['auth_uri'], - access_type='meep', + access_type='offline', + prompt='consent', code_challenge='2yN0TOdl0gkGwFOmtfx3f913tgEaLM2d2S0WlmG1Z6Q', code_challenge_method='S256') - def test_authorization_url_generated_verifier(self, instance): + def test_authorization_url_access_type(self, instance): scope = 'scope_one' instance.oauth2session.scope = [scope] - authorization_url_path = mock.patch.object( + instance.code_verifier = 'amanaplanacanalpanama' + authorization_url_patch = mock.patch.object( instance.oauth2session, 'authorization_url', wraps=instance.oauth2session.authorization_url) - with authorization_url_path as authorization_url_spy: - instance.authorization_url() + with authorization_url_patch as authorization_url_spy: + url, _ = instance.authorization_url(access_type='meep') - _, kwargs = authorization_url_spy.call_args_list[0] - assert kwargs['code_challenge_method'] == 'S256' - assert len(instance.code_verifier) == 128 - assert len(kwargs['code_challenge']) == 43 - valid_verifier = r'^[A-Za-z0-9-._~]*$' - valid_challenge = r'^[A-Za-z0-9-_]*$' - assert re.match(valid_verifier, instance.code_verifier) - assert re.match(valid_challenge, kwargs['code_challenge']) + assert CLIENT_SECRETS_INFO['web']['auth_uri'] in url + assert scope in url + authorization_url_spy.assert_called_with( + CLIENT_SECRETS_INFO['web']['auth_uri'], + access_type='meep', + code_challenge='2yN0TOdl0gkGwFOmtfx3f913tgEaLM2d2S0WlmG1Z6Q', + code_challenge_method='S256') def test_fetch_token(self, instance): instance.code_verifier = 'amanaplanacanalpanama' @@ -219,6 +217,21 @@ def set_token(*args, **kwargs): with fetch_token_patch as fetch_token_mock: yield fetch_token_mock + @mock.patch('google_auth_oauthlib.flow.input', autospec=True) + def test_run_console(self, input_mock, instance, mock_fetch_token): + input_mock.return_value = mock.sentinel.code + credentials = instance.run_console() + + assert credentials.token == mock.sentinel.access_token + assert credentials._refresh_token == mock.sentinel.refresh_token + assert credentials.id_token == mock.sentinel.id_token + + mock_fetch_token.assert_called_with( + CLIENT_SECRETS_INFO['web']['token_uri'], + client_secret=CLIENT_SECRETS_INFO['web']['client_secret'], + code=mock.sentinel.code, + code_verifier=None) + @mock.patch('google_auth_oauthlib.flow.input', autospec=True) def test_run_console(self, input_mock, instance, mock_fetch_token): input_mock.return_value = mock.sentinel.code @@ -242,6 +255,38 @@ def test_run_local_server( auth_redirect_url = urllib.parse.urljoin( 'http://localhost:60452', self.REDIRECT_REQUEST_PATH) + + with concurrent.futures.ThreadPoolExecutor(max_workers=1) as pool: + future = pool.submit(partial( + instance.run_local_server, port=60452)) + + while not future.done(): + try: + requests.get(auth_redirect_url) + except requests.ConnectionError: # pragma: NO COVER + pass + + credentials = future.result() + + assert credentials.token == mock.sentinel.access_token + assert credentials._refresh_token == mock.sentinel.refresh_token + assert credentials.id_token == mock.sentinel.id_token + assert webbrowser_mock.open.called + + expected_auth_response = auth_redirect_url.replace('http', 'https') + mock_fetch_token.assert_called_with( + CLIENT_SECRETS_INFO['web']['token_uri'], + client_secret=CLIENT_SECRETS_INFO['web']['client_secret'], + authorization_response=expected_auth_response, + code_verifier=None) + + @pytest.mark.webtest + @mock.patch('google_auth_oauthlib.flow.webbrowser', autospec=True) + def test_run_local_server_code_verifier( + self, webbrowser_mock, instance, mock_fetch_token): + auth_redirect_url = urllib.parse.urljoin( + 'http://localhost:60452', + self.REDIRECT_REQUEST_PATH) instance.code_verifier = 'amanaplanacanalpanama' with concurrent.futures.ThreadPoolExecutor(max_workers=1) as pool: From 500dc00542bc7222298323d1b4d8a053f1a47ca9 Mon Sep 17 00:00:00 2001 From: Bu Sun Kim Date: Tue, 18 Jun 2019 15:35:09 -0700 Subject: [PATCH 3/6] Add option to autogen code_verifier. --- google_auth_oauthlib/flow.py | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/google_auth_oauthlib/flow.py b/google_auth_oauthlib/flow.py index c6bdc7b..4fedd28 100644 --- a/google_auth_oauthlib/flow.py +++ b/google_auth_oauthlib/flow.py @@ -95,7 +95,7 @@ class Flow(object): def __init__( self, oauth2session, client_type, client_config, - redirect_uri=None, code_verifier=None): + redirect_uri=None, code_verifier=None, autogenerate_code_verifier=False): """ Args: oauth2session (requests_oauthlib.OAuth2Session): @@ -109,6 +109,8 @@ def __init__( :attr:`redirect_uri`. code_verifier (str): random string of 43-128 chars used to verify the key exchange.using PKCE. + autogenerate_code_verifier (bool): If true, auto-generate a + code_verifier. .. _client secrets: https://developers.google.com/api-client-library/python/guide /aaa_client_secrets @@ -121,6 +123,7 @@ def __init__( """requests_oauthlib.OAuth2Session: The OAuth 2.0 session.""" self.redirect_uri = redirect_uri self.code_verifier = code_verifier + self.autogenerate_code_verifier = autogenerate_code_verifier @classmethod def from_client_config(cls, client_config, scopes, **kwargs): @@ -154,14 +157,17 @@ def from_client_config(cls, client_config, scopes, **kwargs): raise ValueError( 'Client secrets must be for a web or installed app.') + # these args cannot be passed to requests_oauthlib.OAuth2Session + code_verifier = kwargs.pop('code_verifier', None) + autogenerate_code_verifier = kwargs.pop('autogenerate_code_verifier', None) + session, client_config = ( google_auth_oauthlib.helpers.session_from_client_config( client_config, scopes, **kwargs)) redirect_uri = kwargs.get('redirect_uri', None) - code_verifier = kwargs.get('code_verifier', None) - return cls(session, client_type, client_config, redirect_uri, code_verifier) + return cls(session, client_type, client_config, redirect_uri, code_verifier, autogenerate_code_verifier) @classmethod def from_client_secrets_file(cls, client_secrets_file, scopes, **kwargs): @@ -218,6 +224,12 @@ def authorization_url(self, **kwargs): specify the ``state`` when constructing the :class:`Flow`. """ kwargs.setdefault('access_type', 'offline') + if self.autogenerate_code_verifier: + chars = ascii_letters+digits+'-._~' + rnd = SystemRandom() + random_verifier = [rnd.choice(chars) for _ in range(0, 128)] + self.code_verifier = ''.join(random_verifier) + if self.code_verifier: code_hash = hashlib.sha256() code_hash.update(str.encode(self.code_verifier)) From 109d2a3d68e9ae2ad96e3b9482e9758bb3b52ba8 Mon Sep 17 00:00:00 2001 From: Bu Sun Kim Date: Tue, 18 Jun 2019 15:52:48 -0700 Subject: [PATCH 4/6] Re-add test for autogenerated code verifier. --- tests/test_flow.py | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/tests/test_flow.py b/tests/test_flow.py index 30689c5..f4252de 100644 --- a/tests/test_flow.py +++ b/tests/test_flow.py @@ -141,6 +141,26 @@ def test_authorization_url_access_type(self, instance): code_challenge='2yN0TOdl0gkGwFOmtfx3f913tgEaLM2d2S0WlmG1Z6Q', code_challenge_method='S256') + def test_authorization_url_generated_verifier(self): + scope = 'scope_one' + instance = flow.Flow.from_client_config( + CLIENT_SECRETS_INFO, scopes=[scope], autogenerate_code_verifier=True) + authorization_url_path = mock.patch.object( + instance.oauth2session, 'authorization_url', + wraps=instance.oauth2session.authorization_url) + + with authorization_url_path as authorization_url_spy: + instance.authorization_url() + + _, kwargs = authorization_url_spy.call_args_list[0] + assert kwargs['code_challenge_method'] == 'S256' + assert len(instance.code_verifier) == 128 + assert len(kwargs['code_challenge']) == 43 + valid_verifier = r'^[A-Za-z0-9-._~]*$' + valid_challenge = r'^[A-Za-z0-9-_]*$' + assert re.match(valid_verifier, instance.code_verifier) + assert re.match(valid_challenge, kwargs['code_challenge']) + def test_fetch_token(self, instance): instance.code_verifier = 'amanaplanacanalpanama' fetch_token_patch = mock.patch.object( From 35dabdb7d559cebba477cfa07b9f909c29ded86b Mon Sep 17 00:00:00 2001 From: Bu Sun Kim Date: Mon, 24 Jun 2019 12:28:18 -0700 Subject: [PATCH 5/6] Fix lint. --- google_auth_oauthlib/flow.py | 15 ++++++++++++--- tests/test_flow.py | 18 ++---------------- 2 files changed, 14 insertions(+), 19 deletions(-) diff --git a/google_auth_oauthlib/flow.py b/google_auth_oauthlib/flow.py index 4fedd28..5ad3062 100644 --- a/google_auth_oauthlib/flow.py +++ b/google_auth_oauthlib/flow.py @@ -95,7 +95,8 @@ class Flow(object): def __init__( self, oauth2session, client_type, client_config, - redirect_uri=None, code_verifier=None, autogenerate_code_verifier=False): + redirect_uri=None, code_verifier=None, + autogenerate_code_verifier=False): """ Args: oauth2session (requests_oauthlib.OAuth2Session): @@ -159,7 +160,8 @@ def from_client_config(cls, client_config, scopes, **kwargs): # these args cannot be passed to requests_oauthlib.OAuth2Session code_verifier = kwargs.pop('code_verifier', None) - autogenerate_code_verifier = kwargs.pop('autogenerate_code_verifier', None) + autogenerate_code_verifier = kwargs.pop( + 'autogenerate_code_verifier', None) session, client_config = ( google_auth_oauthlib.helpers.session_from_client_config( @@ -167,7 +169,14 @@ def from_client_config(cls, client_config, scopes, **kwargs): redirect_uri = kwargs.get('redirect_uri', None) - return cls(session, client_type, client_config, redirect_uri, code_verifier, autogenerate_code_verifier) + return cls( + session, + client_type, + client_config, + redirect_uri, + code_verifier, + autogenerate_code_verifier + ) @classmethod def from_client_secrets_file(cls, client_secrets_file, scopes, **kwargs): diff --git a/tests/test_flow.py b/tests/test_flow.py index f4252de..106f609 100644 --- a/tests/test_flow.py +++ b/tests/test_flow.py @@ -144,7 +144,8 @@ def test_authorization_url_access_type(self, instance): def test_authorization_url_generated_verifier(self): scope = 'scope_one' instance = flow.Flow.from_client_config( - CLIENT_SECRETS_INFO, scopes=[scope], autogenerate_code_verifier=True) + CLIENT_SECRETS_INFO, scopes=[scope], + autogenerate_code_verifier=True) authorization_url_path = mock.patch.object( instance.oauth2session, 'authorization_url', wraps=instance.oauth2session.authorization_url) @@ -237,21 +238,6 @@ def set_token(*args, **kwargs): with fetch_token_patch as fetch_token_mock: yield fetch_token_mock - @mock.patch('google_auth_oauthlib.flow.input', autospec=True) - def test_run_console(self, input_mock, instance, mock_fetch_token): - input_mock.return_value = mock.sentinel.code - credentials = instance.run_console() - - assert credentials.token == mock.sentinel.access_token - assert credentials._refresh_token == mock.sentinel.refresh_token - assert credentials.id_token == mock.sentinel.id_token - - mock_fetch_token.assert_called_with( - CLIENT_SECRETS_INFO['web']['token_uri'], - client_secret=CLIENT_SECRETS_INFO['web']['client_secret'], - code=mock.sentinel.code, - code_verifier=None) - @mock.patch('google_auth_oauthlib.flow.input', autospec=True) def test_run_console(self, input_mock, instance, mock_fetch_token): input_mock.return_value = mock.sentinel.code From 17e6fd236d267a56fc02578af63314d5f4fad4a4 Mon Sep 17 00:00:00 2001 From: Bu Sun Kim Date: Fri, 2 Aug 2019 13:32:02 -0700 Subject: [PATCH 6/6] Retrigger Travis.