From 5ce959c1d55f100d9635c42a302a431374d1f58f Mon Sep 17 00:00:00 2001 From: Erwin Kooi Date: Wed, 3 Oct 2018 21:02:49 -0400 Subject: [PATCH 1/5] https://github.com/auth0/auth0-python/issues/119 #Hacktoberfest --- auth0/v3/authentication/authorize_client.py | 45 +++++++++++++------ .../authentication/test_authorize_client.py | 43 ++++++++++++++++++ 2 files changed, 74 insertions(+), 14 deletions(-) diff --git a/auth0/v3/authentication/authorize_client.py b/auth0/v3/authentication/authorize_client.py index 66fb64e2..064c23de 100644 --- a/auth0/v3/authentication/authorize_client.py +++ b/auth0/v3/authentication/authorize_client.py @@ -1,8 +1,9 @@ +from urllib.parse import urlencode, urlunparse, quote_plus + from .base import AuthenticationBase class AuthorizeClient(AuthenticationBase): - """Authorize Client Args: @@ -12,22 +13,38 @@ class AuthorizeClient(AuthenticationBase): def __init__(self, domain): self.domain = domain - def authorize(self, client_id, audience=None, state=None, redirect_uri=None, - response_type='code', scope='openid'): + def _defaults(self, params): + params.setdefault('response_type', 'code') + params.setdefault('scope', 'openid') + return params + + def get_authorize_url(self, quote_via=quote_plus, **kwargs): + """ + :param quote_via: callable + :param client_id: str + :param audience: str + :param state: str + :param redirect_uri: str + :param response_type: str + :param scope: str + :return: str + """ + params = urlencode(self._defaults(kwargs), doseq=True, quote_via=quote_via) + return urlunparse(['https', self.domain, '/authorize', None, params, None]) + + def authorize(self, **kwargs): """Authorization code grant This is the OAuth 2.0 grant that regular web apps utilize in order to access an API. - """ - params = { - 'client_id': client_id, - 'audience': audience, - 'response_type': response_type, - 'scope': scope, - 'state': state, - 'redirect_uri': redirect_uri - } + :param client_id: str + :param audience: str + :param state: str + :param redirect_uri: str + :param response_type: str + :param scope: str + :return: Response + """ return self.get( 'https://%s/authorize' % self.domain, - params=params) - + params=self._defaults(kwargs)) diff --git a/auth0/v3/test/authentication/test_authorize_client.py b/auth0/v3/test/authentication/test_authorize_client.py index 1bc54ec2..6a76a716 100644 --- a/auth0/v3/test/authentication/test_authorize_client.py +++ b/auth0/v3/test/authentication/test_authorize_client.py @@ -1,10 +1,53 @@ import unittest +from urllib.parse import quote + import mock from ...authentication.authorize_client import AuthorizeClient class TestAuthorizeClient(unittest.TestCase): + def test_get_authorize_url(self): + expected_result = 'https://my.domain.com/authorize' \ + '?client_id=cid' \ + '&audience=https%3A%2F%2Ftest.com%2Fapi' \ + '&state=st+ate' \ + '&redirect_uri=http%3A%2F%2Flocalhost%3Fcallback' \ + '&response_type=code' \ + '&scope=openid' + + a = AuthorizeClient('my.domain.com') + + result = a.get_authorize_url( + client_id='cid', + audience='https://test.com/api', + state='st ate', + redirect_uri='http://localhost?callback', + ) + + self.assertEqual(result, expected_result) + + def test_get_authorize_url_quote(self): + expected_result = 'https://my.domain.com/authorize' \ + '?client_id=cid' \ + '&audience=https%3A%2F%2Ftest.com%2Fapi%3Ffoo%3Dbar' \ + '&state=st%20ate' \ + '&redirect_uri=http%3A%2F%2Flocalhost%3Fcallback' \ + '&response_type=code' \ + '&scope=openid' + + a = AuthorizeClient('my.domain.com') + + result = a.get_authorize_url( + quote_via=quote, + client_id='cid', + audience='https://test.com/api?foo=bar', + state='st ate', + redirect_uri='http://localhost?callback', + ) + + self.assertEqual(result, expected_result) + @mock.patch('auth0.v3.authentication.authorize_client.AuthorizeClient.get') def test_login(self, mock_get): From e94c12eb9e881ede0150f6b7c6b921ff988bf246 Mon Sep 17 00:00:00 2001 From: Erwin Kooi Date: Wed, 3 Oct 2018 21:30:27 -0400 Subject: [PATCH 2/5] python2 compatible --- auth0/v3/authentication/authorize_client.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/auth0/v3/authentication/authorize_client.py b/auth0/v3/authentication/authorize_client.py index 064c23de..d654cafa 100644 --- a/auth0/v3/authentication/authorize_client.py +++ b/auth0/v3/authentication/authorize_client.py @@ -1,5 +1,4 @@ -from urllib.parse import urlencode, urlunparse, quote_plus - +from requests.compat import urlencode, urlunparse, quote_plus from .base import AuthenticationBase From e296cacbe619fab50e728c2525e389e18c9ac3e5 Mon Sep 17 00:00:00 2001 From: Erwin Kooi Date: Wed, 3 Oct 2018 21:32:40 -0400 Subject: [PATCH 3/5] python2 compatible --- auth0/v3/test/authentication/test_authorize_client.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/auth0/v3/test/authentication/test_authorize_client.py b/auth0/v3/test/authentication/test_authorize_client.py index 6a76a716..f8fbddfc 100644 --- a/auth0/v3/test/authentication/test_authorize_client.py +++ b/auth0/v3/test/authentication/test_authorize_client.py @@ -1,5 +1,5 @@ import unittest -from urllib.parse import quote +from requests.compat import quote import mock from ...authentication.authorize_client import AuthorizeClient From b8b9277c51e0dbe5e6c9b8d5253447fe5ff39d60 Mon Sep 17 00:00:00 2001 From: Erwin Kooi Date: Wed, 3 Oct 2018 21:55:25 -0400 Subject: [PATCH 4/5] python2 compatible urlencode with quote_via --- auth0/v3/authentication/authorize_client.py | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/auth0/v3/authentication/authorize_client.py b/auth0/v3/authentication/authorize_client.py index d654cafa..1f6732bd 100644 --- a/auth0/v3/authentication/authorize_client.py +++ b/auth0/v3/authentication/authorize_client.py @@ -1,6 +1,9 @@ +import sys from requests.compat import urlencode, urlunparse, quote_plus from .base import AuthenticationBase +_ver = '{}{}{}'.format(*sys.version_info) + class AuthorizeClient(AuthenticationBase): """Authorize Client @@ -28,7 +31,11 @@ def get_authorize_url(self, quote_via=quote_plus, **kwargs): :param scope: str :return: str """ - params = urlencode(self._defaults(kwargs), doseq=True, quote_via=quote_via) + + params = urlencode(self._defaults(kwargs), doseq=True, quote_via=quote_via) \ + if _ver > '34' \ + else '&'.join(['{}={}'.format(quote_via(k, safe=''), quote_via(v, safe='')) + for k, v in self._defaults(kwargs).items()]) return urlunparse(['https', self.domain, '/authorize', None, params, None]) def authorize(self, **kwargs): From 838beb4b58980be6ddca000f84333b56855bd7f0 Mon Sep 17 00:00:00 2001 From: Erwin Kooi Date: Wed, 3 Oct 2018 22:37:40 -0400 Subject: [PATCH 5/5] unit test urlencode > allow random order of query parameters --- auth0/v3/authentication/authorize_client.py | 54 +++++++-------- .../authentication/test_authorize_client.py | 69 ++++++++++++------- 2 files changed, 71 insertions(+), 52 deletions(-) diff --git a/auth0/v3/authentication/authorize_client.py b/auth0/v3/authentication/authorize_client.py index 1f6732bd..8650399d 100644 --- a/auth0/v3/authentication/authorize_client.py +++ b/auth0/v3/authentication/authorize_client.py @@ -15,42 +15,40 @@ class AuthorizeClient(AuthenticationBase): def __init__(self, domain): self.domain = domain - def _defaults(self, params): - params.setdefault('response_type', 'code') - params.setdefault('scope', 'openid') - return params - - def get_authorize_url(self, quote_via=quote_plus, **kwargs): + def get_authorize_url(self, client_id, audience=None, state=None, redirect_uri=None, + response_type='code', scope='openid', quote_via=quote_plus): """ - :param quote_via: callable - :param client_id: str - :param audience: str - :param state: str - :param redirect_uri: str - :param response_type: str - :param scope: str - :return: str + use quote_via=urllib.quote to to urlencode spaces into "%20", the default is "+" """ - - params = urlencode(self._defaults(kwargs), doseq=True, quote_via=quote_via) \ + params = { + 'client_id': client_id, + 'audience': audience, + 'response_type': response_type, + 'scope': scope, + 'state': state, + 'redirect_uri': redirect_uri + } + query = urlencode(params, doseq=True, quote_via=quote_via) \ if _ver > '34' \ else '&'.join(['{}={}'.format(quote_via(k, safe=''), quote_via(v, safe='')) - for k, v in self._defaults(kwargs).items()]) - return urlunparse(['https', self.domain, '/authorize', None, params, None]) + for k, v in params.items()]) + return urlunparse(['https', self.domain, '/authorize', None, query, None]) - def authorize(self, **kwargs): + def authorize(self, client_id, audience=None, state=None, redirect_uri=None, + response_type='code', scope='openid'): """Authorization code grant This is the OAuth 2.0 grant that regular web apps utilize in order to access an API. - - :param client_id: str - :param audience: str - :param state: str - :param redirect_uri: str - :param response_type: str - :param scope: str - :return: Response """ + params = { + 'client_id': client_id, + 'audience': audience, + 'response_type': response_type, + 'scope': scope, + 'state': state, + 'redirect_uri': redirect_uri + } + return self.get( 'https://%s/authorize' % self.domain, - params=self._defaults(kwargs)) + params=params) diff --git a/auth0/v3/test/authentication/test_authorize_client.py b/auth0/v3/test/authentication/test_authorize_client.py index f8fbddfc..2546b39e 100644 --- a/auth0/v3/test/authentication/test_authorize_client.py +++ b/auth0/v3/test/authentication/test_authorize_client.py @@ -1,5 +1,5 @@ import unittest -from requests.compat import quote +from requests.compat import quote, urlparse import mock from ...authentication.authorize_client import AuthorizeClient @@ -8,45 +8,66 @@ class TestAuthorizeClient(unittest.TestCase): def test_get_authorize_url(self): - expected_result = 'https://my.domain.com/authorize' \ - '?client_id=cid' \ - '&audience=https%3A%2F%2Ftest.com%2Fapi' \ - '&state=st+ate' \ - '&redirect_uri=http%3A%2F%2Flocalhost%3Fcallback' \ - '&response_type=code' \ - '&scope=openid' + expected_result = urlparse( + 'https://my.domain.com/authorize' + '?client_id=cid' + '&audience=https%3A%2F%2Ftest.com%2Fapi' + '&state=st+ate' + '&redirect_uri=http%3A%2F%2Flocalhost%3Fcallback' + '&response_type=code' + '&scope=openid+profile') a = AuthorizeClient('my.domain.com') - result = a.get_authorize_url( + actual_result = urlparse(a.get_authorize_url( client_id='cid', audience='https://test.com/api', state='st ate', redirect_uri='http://localhost?callback', - ) + scope='openid profile' + )) - self.assertEqual(result, expected_result) + self.assertEqual(actual_result.scheme, expected_result.scheme) + self.assertEqual(actual_result.hostname, expected_result.hostname) + self.assertEqual(actual_result.path, expected_result.path) - def test_get_authorize_url_quote(self): - expected_result = 'https://my.domain.com/authorize' \ - '?client_id=cid' \ - '&audience=https%3A%2F%2Ftest.com%2Fapi%3Ffoo%3Dbar' \ - '&state=st%20ate' \ - '&redirect_uri=http%3A%2F%2Flocalhost%3Fcallback' \ - '&response_type=code' \ - '&scope=openid' + # there is no guarantee the order of items in the query is equal to the method call, that's okay + expected_query = expected_result.query.split('&') + actual_query = actual_result.query.split('&') + self.assertEqual(sorted(expected_query), sorted(actual_query)) + def test_get_authorize_url_quote(self): + """ + sometimes we want to urlencode spaces into %20, we can do that with quote_via + """ + expected_result = urlparse( + 'https://my.domain.com/authorize' + '?client_id=cid' + '&audience=https%3A%2F%2Ftest.com%2Fapi%3Ffoo%3Dbar' + '&state=st%20ate' + '&redirect_uri=http%3A%2F%2Flocalhost%3Fcallback%3D%23123' + '&response_type=code' + '&scope=openid%20profile' + ) a = AuthorizeClient('my.domain.com') - result = a.get_authorize_url( - quote_via=quote, + actual_result = urlparse(a.get_authorize_url( client_id='cid', audience='https://test.com/api?foo=bar', state='st ate', - redirect_uri='http://localhost?callback', - ) + redirect_uri='http://localhost?callback=#123', + quote_via=quote, + scope='openid profile' + )) + + self.assertEqual(actual_result.scheme, expected_result.scheme) + self.assertEqual(actual_result.hostname, expected_result.hostname) + self.assertEqual(actual_result.path, expected_result.path) - self.assertEqual(result, expected_result) + # there is no guarantee the order of items in the query is equal to the method call, that's okay + expected_query = expected_result.query.split('&') + actual_query = actual_result.query.split('&') + self.assertEqual(sorted(expected_query), sorted(actual_query)) @mock.patch('auth0.v3.authentication.authorize_client.AuthorizeClient.get') def test_login(self, mock_get):