diff --git a/reviewboard/accounts/backends/http_digest.py b/reviewboard/accounts/backends/http_digest.py index 469ef80efb310dcc64ed9a56537c546401b08487..e0ba30d0cd1561cd35849fdcf7c93cfc20f81d3f 100644 --- a/reviewboard/accounts/backends/http_digest.py +++ b/reviewboard/accounts/backends/http_digest.py @@ -75,32 +75,22 @@ class HTTPDigestBackend(BaseAuthBackend): """ username = username.strip() - filename = settings.DIGEST_FILE_LOCATION digest_text = '%s:%s:%s' % (username, settings.DIGEST_REALM, password) - digest_password = hashlib.md5(digest_text).hexdigest() + digest_password = hashlib.md5(digest_text.encode('utf-8')).hexdigest() - try: - with open(filename, 'r') as fp: - for line_no, line in enumerate(fp): - try: - user, realm, passwd = line.strip().split(':') + user_info = self._htpasswd_get_user(username) - if user == username and passwd == digest_password: - return self.get_or_create_user(username=username, - request=request) - except ValueError as e: - logger.exception('Error parsing HTTP Digest password ' - 'file "%s" at line %d: %s', - filename, line_no, e) - break - except IOError as e: - logger.exception('Could not open the HTTP Digest password ' - 'file "%s": %s', - filename, e) + if user_info is not None: + if (user_info['username'] == username and + user_info['passwd'] == digest_password): + return self.get_or_create_user(username=username, + request=request, + htpassword_user_info=user_info) return None - def get_or_create_user(self, username, request=None): + def get_or_create_user(self, username, request=None, + htpassword_user_info={}): """Return an existing user or create one if it doesn't exist. This does not authenticate the user. @@ -116,6 +106,9 @@ class HTTPDigestBackend(BaseAuthBackend): request (django.http.HttpRequest, unused): The HTTP request from the client. This is unused. + htpassword_user_info (dict): + The parsed user info in the digest password file. + Returns: django.contrib.auth.models.User: The resulting user, or ``None`` if one could not be found. @@ -123,6 +116,50 @@ class HTTPDigestBackend(BaseAuthBackend): try: user = User.objects.get(username=username) except User.DoesNotExist: + if htpassword_user_info is None: + user_info = self._htpasswd_get_user(username=username) + + if user_info is None: + return None + user = User.objects.create_user(username=username) return user + + def _htpasswd_get_user(self, username): + """Return the user info if it is found in the digest password file. + + Args: + username (unicode): + The username of the user to look up. + + Returns: + dict: + The parsed user info in the digest password file. + """ + filename = settings.DIGEST_FILE_LOCATION + + try: + with open(filename, 'r') as fp: + for line_no, line in enumerate(fp): + try: + user, realm, passwd = line.strip().split(':') + + if user == username: + return { + 'username': user, + 'realm': realm, + 'passwd': passwd, + } + + except ValueError as e: + logger.exception('Error parsing HTTP Digest password ' + 'file "%s" at line %d: %s', + filename, line_no, e) + break + except IOError as e: + logger.exception('Could not open the HTTP Digest password ' + 'file "%s": %s', + filename, e) + + return None diff --git a/reviewboard/accounts/tests/test_http_digest_auth_backend.py b/reviewboard/accounts/tests/test_http_digest_auth_backend.py new file mode 100644 index 0000000000000000000000000000000000000000..b99b026f86bc43eb179f0f75f4332802d5b57adf --- /dev/null +++ b/reviewboard/accounts/tests/test_http_digest_auth_backend.py @@ -0,0 +1,196 @@ +"""Unit tests for HTTPDigestBackend.""" + +import hashlib +import kgb +import tempfile + +from django.conf import settings +from django.contrib.auth.models import User + +from reviewboard.accounts.backends import HTTPDigestBackend +from reviewboard.testing import TestCase + + +class HTTPDigestBackendTests(kgb.SpyAgency, TestCase): + """Unit tests for HTTPDigestBackendTests.""" + + def setUp(self): + super(HTTPDigestBackendTests, self).setUp() + + self.backend = HTTPDigestBackend() + + def test_authenticate_with_valid_user(self): + """Testing HTTPDigestBackend.authenticate with valid user + credentials + """ + _digest_text = '%s:%s:%s' % ('test-user', + settings.DIGEST_REALM, + 'test-pass') + _digest_passwd = hashlib.md5(_digest_text.encode('utf-8')).hexdigest() + + _user_info = { + 'username': 'test-user', + 'realm': 'test-realm', + 'passwd': _digest_passwd, + } + + self.spy_on(self.backend._htpasswd_get_user, + call_fake=lambda *args, **kwargs: _user_info) + self.spy_on(self.backend.get_or_create_user, + call_fake=lambda *args, **kwargs: + User(username='test-user')) + + user = self.backend.authenticate(request=None, + username='test-user', + password='test-pass') + + self.assertIsNotNone(user) + self.assertEqual(user.username, 'test-user') + + self.assertSpyCalledWith(self.backend._htpasswd_get_user, + 'test-user') + self.assertSpyCalledWith(self.backend.get_or_create_user, + username='test-user', + request=None, + htpassword_user_info=_user_info) + + def test_authenticate_with_invalid_user(self): + """Testing HTTPDigestBackend.authenticate with bad user credentials""" + self.spy_on(self.backend._htpasswd_get_user, + call_fake=lambda *args, **kwargs: None) + + user = self.backend.authenticate(request=None, + username='test-user', + password='test-pass') + self.assertIsNone(user) + + def test_get_or_create_user_with_htpassword_user_info_and_in_db(self): + """Testing HTTPDigestBackend.get_or_create_user with + htpassword_user_info and user is in database + """ + _user_info = { + 'username': 'test-user', + 'realm': 'test-realm', + 'passwd': 'test_pass' + } + + self.spy_on(User.objects.get, + call_fake=lambda *args, **kwargs: + User(username='test-user')) + + user = self.backend.get_or_create_user(username='test-user', + request=None, + htpassword_user_info=_user_info) + + self.assertIsNotNone(user) + self.assertEqual(user.username, 'test-user') + + self.assertSpyCalledWith(User.objects.get, username='test-user') + + def test_get_or_create_user_with_htpassword_user_info_not_in_db(self): + """Testing HTTPDigestBackend.get_or_create_user with + htpassword_user_info and user is not found in database + """ + _user_info = { + 'username': 'test-user', + 'realm': 'test-realm', + 'passwd': 'test_pass' + } + + self.spy_on(User.objects.get, call_fake=self._raise_user_error) + self.spy_on(User.objects.create_user, + call_fake=lambda *args, **kwargs: + User(username='test-user')) + + user = self.backend.get_or_create_user(username='test-user', + request=None, + htpassword_user_info=_user_info) + + self.assertIsNotNone(user) + self.assertEqual(user.username, 'test-user') + + self.assertSpyCalledWith(User.objects.get, username='test-user') + self.assertSpyRaised(User.objects.get, User.DoesNotExist) + self.assertSpyCalledWith(User.objects.create_user, + username='test-user') + + def test_get_or_create_user_without_htpassword_user_info_and_found(self): + """Testing HTTPDigestBackend.get_or_create_user without + htpassword_user_info, user is not found in database but is found in + the digest password file + """ + _user_info = { + 'username': 'test-user', + 'realm': 'test-realm', + 'passwd': 'test_pass' + } + + self.spy_on(User.objects.get, call_fake=self._raise_user_error) + self.spy_on(self.backend._htpasswd_get_user, + call_fake=lambda *args, **kwargs: _user_info) + self.spy_on(User.objects.create_user, + call_fake=lambda *args, **kwargs: + User(username='test-user')) + + user = self.backend.get_or_create_user(username='test-user', + request=None, + htpassword_user_info=None) + + self.assertIsNotNone(user) + self.assertEqual(user.username, 'test-user') + + self.assertSpyCalledWith(User.objects.get, username='test-user') + self.assertSpyRaised(User.objects.get, User.DoesNotExist) + self.assertSpyCalledWith(self.backend._htpasswd_get_user, + 'test-user') + self.assertSpyCalledWith(User.objects.create_user, + username='test-user') + + def test_get_or_create_user_without_htpassword_user_info_not_found(self): + """Testing HTTPDigestBackend.get_or_create_user without + htpassword_user_info and user is not found in both the database + and digest password file + """ + self.spy_on(User.objects.get, call_fake=self._raise_user_error) + self.spy_on(self.backend._htpasswd_get_user, + call_fake=lambda *args, **kwargs: None) + + user = self.backend.get_or_create_user(username='test-user', + request=None, + htpassword_user_info=None) + + self.assertIsNone(user) + + def test_htpasswd_get_user_user_found(self): + """Testing HTTPDigestBackend.htpasswd_get_user_found with user found + in the digest password file + """ + temp_file = tempfile.NamedTemporaryFile(mode='w') + + temp_file.write('test-user:test-realm:test-pass\n' + 'test-user2:test-realm2:test-pass2\n') + temp_file.seek(0) + + with self.settings(DIGEST_FILE_LOCATION=temp_file.name): + user = self.backend._htpasswd_get_user(username='test-user') + + self.assertIsNotNone(user) + self.assertEqual(user['username'], 'test-user') + + def test_htpasswd_get_user_user_not_found(self): + """Testing HTTPDigestBackend.htpasswd_get_user_found with user not + found in the digest password file + """ + temp_file2 = tempfile.NamedTemporaryFile(mode='w') + + temp_file2.write('test-user:test-realm:test-pass\n' + 'test-user2:test-realm2:test-pass2\n') + temp_file2.seek(0) + + with self.settings(DIGEST_FILE_LOCATION=temp_file2.name): + user = self.backend._htpasswd_get_user(username='bad-user') + + self.assertIsNone(user) + + def _raise_user_error(self, *args, **kwargs): + raise User.DoesNotExist