From c895f874184371ff5798748b1c7234128ca4b25e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dariusz=20G=C3=B3recki?= Date: Fri, 1 Jul 2016 09:42:30 +0100 Subject: [PATCH 1/5] ref #2659 public HTTP Basic credentials extraction --- pyramid/authentication.py | 77 +++++++++++++++++++++++---------------- 1 file changed, 46 insertions(+), 31 deletions(-) diff --git a/pyramid/authentication.py b/pyramid/authentication.py index 9bf1de62e6..95d6a710da 100644 --- a/pyramid/authentication.py +++ b/pyramid/authentication.py @@ -1128,7 +1128,7 @@ def __init__(self, check, realm='Realm', debug=False): def unauthenticated_userid(self, request): """ The userid parsed from the ``Authorization`` request header.""" - credentials = self._get_credentials(request) + credentials = extract_http_basic_credentials(request) if credentials: return credentials[0] @@ -1145,42 +1145,14 @@ def forget(self, request): return [('WWW-Authenticate', 'Basic realm="%s"' % self.realm)] def callback(self, username, request): - # Username arg is ignored. Unfortunately _get_credentials winds up + # Username arg is ignored. Unfortunately extract_http_basic_credentials winds up # getting called twice when authenticated_userid is called. Avoiding # that, however, winds up duplicating logic from the superclass. - credentials = self._get_credentials(request) + credentials = extract_http_basic_credentials(request) if credentials: username, password = credentials return self.check(username, password, request) - def _get_credentials(self, request): - authorization = request.headers.get('Authorization') - if not authorization: - return None - try: - authmeth, auth = authorization.split(' ', 1) - except ValueError: # not enough values to unpack - return None - if authmeth.lower() != 'basic': - return None - - try: - authbytes = b64decode(auth.strip()) - except (TypeError, binascii.Error): # can't decode - return None - - # try utf-8 first, then latin-1; see discussion in - # https://github.com/Pylons/pyramid/issues/898 - try: - auth = authbytes.decode('utf-8') - except UnicodeDecodeError: - auth = authbytes.decode('latin-1') - - try: - username, password = auth.split(':', 1) - except ValueError: # not enough values to unpack - return None - return username, password class _SimpleSerializer(object): def loads(self, bstruct): @@ -1188,3 +1160,46 @@ def loads(self, bstruct): def dumps(self, appstruct): return bytes_(appstruct) + + +def extract_http_basic_credentials(request): + """ A helper function for extraction of HTTP Basic credentials + from a given `request`. + + ``request`` + The request object + """ + try: + # First try authorization extraction logic from WebOb + try: + authmeth, auth = request.authorization + except AttributeError: # Probably a DummyRequest + authorization = request.headers.get('Authorization') + if not authorization: + return None + + authmeth, auth = authorization.split(' ', 1) + except (ValueError, TypeError): + # not enough values to unpack or None is not iterable + return None + + if authmeth.lower() != 'basic': + return None + + try: + authbytes = b64decode(auth.strip()) + except (TypeError, binascii.Error): # can't decode + return None + + # try utf-8 first, then latin-1; see discussion in + # https://github.com/Pylons/pyramid/issues/898 + try: + auth = authbytes.decode('utf-8') + except UnicodeDecodeError: + auth = authbytes.decode('latin-1') + + try: + username, password = auth.split(':', 1) + except ValueError: # not enough values to unpack + return None + return username, password From 362a5812733ccc97336cea9ba5b1deebc3120ab2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dariusz=20G=C3=B3recki?= Date: Sat, 2 Jul 2016 15:16:45 +0100 Subject: [PATCH 2/5] Remove WebOb related logic --- pyramid/authentication.py | 17 ++++++----------- 1 file changed, 6 insertions(+), 11 deletions(-) diff --git a/pyramid/authentication.py b/pyramid/authentication.py index 95d6a710da..c6fd8f466e 100644 --- a/pyramid/authentication.py +++ b/pyramid/authentication.py @@ -1169,18 +1169,13 @@ def extract_http_basic_credentials(request): ``request`` The request object """ + authorization = request.headers.get('Authorization') + if not authorization: + return None + try: - # First try authorization extraction logic from WebOb - try: - authmeth, auth = request.authorization - except AttributeError: # Probably a DummyRequest - authorization = request.headers.get('Authorization') - if not authorization: - return None - - authmeth, auth = authorization.split(' ', 1) - except (ValueError, TypeError): - # not enough values to unpack or None is not iterable + authmeth, auth = authorization.split(' ', 1) + except ValueError: # not enough values to unpack return None if authmeth.lower() != 'basic': From 744bf0565a15a40f5c04cc8b0c1fe84a2ca489da Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dariusz=20G=C3=B3recki?= Date: Fri, 15 Jul 2016 17:15:51 +0100 Subject: [PATCH 3/5] Add myself to contributors list & PEP8 --- CONTRIBUTORS.txt | 3 +++ pyramid/authentication.py | 7 ++++--- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/CONTRIBUTORS.txt b/CONTRIBUTORS.txt index 4edf1b4e93..492c2fac2c 100644 --- a/CONTRIBUTORS.txt +++ b/CONTRIBUTORS.txt @@ -256,3 +256,6 @@ Contributors - Amos Latteier, 2015/10/22 - Rami Chousein, 2015/10/28 + +- Dariusz Gorecki, 2016/07/15 + diff --git a/pyramid/authentication.py b/pyramid/authentication.py index c6fd8f466e..2a54be34dc 100644 --- a/pyramid/authentication.py +++ b/pyramid/authentication.py @@ -1145,9 +1145,10 @@ def forget(self, request): return [('WWW-Authenticate', 'Basic realm="%s"' % self.realm)] def callback(self, username, request): - # Username arg is ignored. Unfortunately extract_http_basic_credentials winds up - # getting called twice when authenticated_userid is called. Avoiding - # that, however, winds up duplicating logic from the superclass. + # Username arg is ignored. Unfortunately + # extract_http_basic_credentials winds up getting called twice when + # authenticated_userid is called. Avoiding that, however, + # winds up duplicating logic from the superclass. credentials = extract_http_basic_credentials(request) if credentials: username, password = credentials From 830bcb8aea8d9c842ef1ccd9a80470836f4c6442 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dariusz=20G=C3=B3recki?= Date: Wed, 10 Aug 2016 11:46:45 +0100 Subject: [PATCH 4/5] Add docs & explict tests --- docs/api/authentication.rst | 3 ++ pyramid/authentication.py | 4 +- pyramid/tests/test_authentication.py | 61 ++++++++++++++++++++++++++++ 3 files changed, 66 insertions(+), 2 deletions(-) diff --git a/docs/api/authentication.rst b/docs/api/authentication.rst index 19d08618bd..de2c734913 100644 --- a/docs/api/authentication.rst +++ b/docs/api/authentication.rst @@ -35,4 +35,7 @@ Helper Classes :members: +Helper Functions +~~~~~~~~~~~~~~~~ + .. autofunction:: extract_http_basic_credentials diff --git a/pyramid/authentication.py b/pyramid/authentication.py index 712cef08e9..46909d84e9 100644 --- a/pyramid/authentication.py +++ b/pyramid/authentication.py @@ -1120,10 +1120,10 @@ def dumps(self, appstruct): def extract_http_basic_credentials(request): """ A helper function for extraction of HTTP Basic credentials - from a given `request`. + from a given :term:`request`. ``request`` - The request object + The :term:`request` object """ authorization = request.headers.get('Authorization') if not authorization: diff --git a/pyramid/tests/test_authentication.py b/pyramid/tests/test_authentication.py index 0a22e59652..53747b6f01 100644 --- a/pyramid/tests/test_authentication.py +++ b/pyramid/tests/test_authentication.py @@ -1476,6 +1476,67 @@ def test_forget(self): self.assertEqual(policy.forget(None), [ ('WWW-Authenticate', 'Basic realm="SomeRealm"')]) + +class TestExtractHTTPBasicCredentials(unittest.TestCase): + def _get_func(self): + from pyramid.authentication import extract_http_basic_credentials + return extract_http_basic_credentials + + def test_no_auth_header(self): + request = testing.DummyRequest() + fn = self._get_func() + + self.assertIsNone(fn(request)) + + def test_invalid_payload(self): + import base64 + request = testing.DummyRequest() + request.headers['Authorization'] = 'Basic %s' % base64.b64encode( + bytes_('chrisrpassword')).decode('ascii') + fn = self._get_func() + self.assertIsNone(fn(request)) + + def test_not_a_basic_auth_scheme(self): + import base64 + request = testing.DummyRequest() + request.headers['Authorization'] = 'OtherScheme %s' % base64.b64encode( + bytes_('chrisr:password')).decode('ascii') + fn = self._get_func() + self.assertIsNone(fn(request)) + + def test_no_base64_encoding(self): + request = testing.DummyRequest() + request.headers['Authorization'] = 'Basic ...' + fn = self._get_func() + self.assertIsNone(fn(request)) + + def test_latin1_payload(self): + import base64 + request = testing.DummyRequest() + inputs = (b'm\xc3\xb6rk\xc3\xb6:' + b'm\xc3\xb6rk\xc3\xb6password').decode('utf-8') + request.headers['Authorization'] = 'Basic %s' % ( + base64.b64encode(inputs.encode('latin-1')).decode('latin-1')) + fn = self._get_func() + self.assertEqual(fn(request), ( + b'm\xc3\xb6rk\xc3\xb6'.decode('utf-8'), + b'm\xc3\xb6rk\xc3\xb6password'.decode('utf-8') + )) + + def test_utf8_payload(self): + import base64 + request = testing.DummyRequest() + inputs = (b'm\xc3\xb6rk\xc3\xb6:' + b'm\xc3\xb6rk\xc3\xb6password').decode('utf-8') + request.headers['Authorization'] = 'Basic %s' % ( + base64.b64encode(inputs.encode('utf-8')).decode('latin-1')) + fn = self._get_func() + self.assertEqual(fn(request), ( + b'm\xc3\xb6rk\xc3\xb6'.decode('utf-8'), + b'm\xc3\xb6rk\xc3\xb6password'.decode('utf-8') + )) + + class TestSimpleSerializer(unittest.TestCase): def _makeOne(self): from pyramid.authentication import _SimpleSerializer From 693cb098a7bc8fbff5fb97c1ac031d0b6e397060 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dariusz=20G=C3=B3recki?= Date: Thu, 11 Aug 2016 12:04:28 +0100 Subject: [PATCH 5/5] Add this feature to chenges & small improvement --- CHANGES.txt | 5 +++++ pyramid/authentication.py | 13 +++++++++++-- pyramid/tests/test_authentication.py | 12 ++++++++++++ 3 files changed, 28 insertions(+), 2 deletions(-) diff --git a/CHANGES.txt b/CHANGES.txt index 8cb4c602e6..a614a44997 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -17,6 +17,11 @@ Backward Incompatibilities Features -------- +- The `_get_credentials` private method of `BasicAuthAuthenticationPolicy` + has been extracted into standalone function `extract_http_basic_credentials` + in `pyramid.authentication` module, this function extracts HTTP Basic + credentials from `request` object, and returns them as a named tuple. + Bug Fixes --------- diff --git a/pyramid/authentication.py b/pyramid/authentication.py index 46909d84e9..034da9e46b 100644 --- a/pyramid/authentication.py +++ b/pyramid/authentication.py @@ -1,6 +1,7 @@ import binascii from codecs import utf_8_decode from codecs import utf_8_encode +from collections import namedtuple import hashlib import base64 import re @@ -1118,9 +1119,16 @@ def dumps(self, appstruct): return bytes_(appstruct) +http_basic_credentials = namedtuple('http_basic_credentials', + ['username', 'password']) + + def extract_http_basic_credentials(request): """ A helper function for extraction of HTTP Basic credentials - from a given :term:`request`. + from a given :term:`request`. Returned values: + + - ``None`` - when credentials couldn't be extracted + - ``namedtuple`` with extracted ``username`` and ``password`` attributes ``request`` The :term:`request` object @@ -1153,4 +1161,5 @@ def extract_http_basic_credentials(request): username, password = auth.split(':', 1) except ValueError: # not enough values to unpack return None - return username, password + + return http_basic_credentials(username, password) diff --git a/pyramid/tests/test_authentication.py b/pyramid/tests/test_authentication.py index 53747b6f01..32923c9ab5 100644 --- a/pyramid/tests/test_authentication.py +++ b/pyramid/tests/test_authentication.py @@ -1536,6 +1536,18 @@ def test_utf8_payload(self): b'm\xc3\xb6rk\xc3\xb6password'.decode('utf-8') )) + def test_namedtuple_return(self): + import base64 + request = testing.DummyRequest() + request.headers['Authorization'] = 'Basic %s' % base64.b64encode( + bytes_('chrisr:pass')).decode('ascii') + fn = self._get_func() + result = fn(request) + + self.assertEqual(result.username, 'chrisr') + self.assertEqual(result.password, 'pass') + + class TestSimpleSerializer(unittest.TestCase): def _makeOne(self):