Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ref #2659 public HTTP Basic credentials extraction #2662

Merged
merged 8 commits into from
Sep 1, 2016
77 changes: 46 additions & 31 deletions pyramid/authentication.py
Original file line number Diff line number Diff line change
Expand Up @@ -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]

Expand All @@ -1145,46 +1145,61 @@ 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
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This crosses 79 chars, but I will fix it in the merge.

# 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):
return native_(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
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see the point in this fallback. It's trying to reproduce all of the logic, but likely with bugs and inconsistencies from the original request.authorization code. This looks like a maintenance nightmare.

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