Merge lp:~fgallina/django-openid-auth/more-backend-tests into lp:django-openid-auth

[Fabián Ezequiel Gallina]

Added more tests `auth.OpenIDBackend`

Some of the cases were indirectly asserted in views tests but not all
the flows seemed properly covered, this should get us a solid
groundwork for any changes to come, exercizing the `authenticate`
method which is the main entrypoint of the backend.

Also included:
 - Ensure Django 1.7 runs on older Tox versions
 - Cleanup lint errors on auth module

[Facundo Batista (facundo)]

Tests FTW!

review: Approve

[Fabián Ezequiel Gallina (fgallina)]

LGTM

review: Approve

=== modified file 'django_openid_auth/auth.py'
--- django_openid_auth/auth.py	2015-09-21 20:42:53 +0000
+++ django_openid_auth/auth.py	2016-08-11 11:45:39 +0000
@@ -30,8 +30,6 @@
 
 from __future__ import unicode_literals
 
-__metaclass__ = type
-
 import re
 
 from django.conf import settings
@@ -54,7 +52,7 @@
 User = get_user_model()
 
 
-class OpenIDBackend:
+class OpenIDBackend(object):
     """A django.contrib.auth backend that authenticates the user based on
     an OpenID response."""
 

=== modified file 'django_openid_auth/tests/test_auth.py'
--- django_openid_auth/tests/test_auth.py	2015-04-23 20:39:02 +0000
+++ django_openid_auth/tests/test_auth.py	2016-08-11 11:45:39 +0000
@@ -28,19 +28,35 @@
 
 from __future__ import unicode_literals
 
+import re
+
 from django.contrib.auth.models import Group, Permission, User
 from django.test import TestCase
 from django.test.utils import override_settings
 
-from openid.consumer.consumer import SuccessResponse
+from openid.consumer.consumer import (
+    CancelResponse,
+    FailureResponse,
+    SetupNeededResponse,
+    SuccessResponse,
+)
+
 from openid.consumer.discover import OpenIDServiceEndpoint
+from openid.extensions import pape
 from openid.message import Message, OPENID2_NS
 
 from django_openid_auth.auth import OpenIDBackend
+from django_openid_auth.exceptions import (
+    DuplicateUsernameViolation,
+    MissingPhysicalMultiFactor,
+    MissingUsernameViolation,
+    RequiredAttributeNotReturned,
+)
 from django_openid_auth.models import UserOpenID
 from django_openid_auth.teams import ns_uri as TEAMS_NS
 from django_openid_auth.tests.helpers import override_session_serializer
 
+
 SREG_NS = "http://openid.net/sreg/1.0"
 AX_NS = "http://openid.net/srv/ax/1.0"
 
@@ -60,6 +76,7 @@
     def make_openid_response(self, sreg_args=None, teams_args=None):
         endpoint = OpenIDServiceEndpoint()
         endpoint.claimed_id = 'some-id'
+        endpoint.server_url = 'http://example.com/'
 
         message = Message(OPENID2_NS)
         if sreg_args is not None:
@@ -77,6 +94,9 @@
             fullname="Some User", nickname="someuser", email="[e-mail]",
             first=None, last=None, verified=False):
         endpoint = OpenIDServiceEndpoint()
+        endpoint.claimed_id = 'some-id'
+        endpoint.server_url = 'http://example.com/'
+
         message = Message(OPENID2_NS)
         attributes = [
             ("nickname", schema + "namePerson/friendly", nickname),
@@ -112,34 +132,28 @@
             user=user, claimed_id=claimed_id, display_id=display_id)
         return user_openid
 
-    def assert_account_verified(self, user, initially_verified, verified):
-        # set user's verification status
+    def _assert_account_verified(self, user, expected):
         permission = Permission.objects.get(codename='account_verified')
-        if initially_verified:
-            user.user_permissions.add(permission)
-        else:
-            user.user_permissions.remove(permission)
-
-        user = User.objects.get(pk=user.pk)
-        has_perm = user.has_perm('django_openid_auth.account_verified')
-        assert has_perm == initially_verified
-
-        if hasattr(user, '_perm_cache'):
-            del user._perm_cache
-
-        # get a response including verification status
-        response = self.make_response_ax()
-        data = dict(first_name=u"Some56789012345678901234567890123",
-                    last_name=u"User56789012345678901234567890123",
-                    email=u"[e-mail]",
-                    account_verified=verified)
-        self.backend.update_user_details(user, data, response)
-
-        # refresh object from the database
-        user = User.objects.get(pk=user.pk)
-        # check the verification status
-        self.assertEqual(
-            user.has_perm('django_openid_auth.account_verified'), verified)
+        perm_label = '%s.%s' % (permission.content_type.app_label,
+                                permission.codename)
+        # Always invalidate the per-request perm cache
+        for attr in user.__dict__.keys():
+            if attr.endswith('_perm_cache'):
+                delattr(user, attr)
+
+        self.assertEqual(user.has_perm(perm_label), expected)
+
+    def assert_account_not_verified(self, user):
+        self._assert_account_verified(user, False)
+
+    def assert_account_verified(self, user):
+        self._assert_account_verified(user, True)
+
+    def assert_no_users_created(self, expected_count=0):
+        current_count = User.objects.count()
+        msg = 'New users found (expected: %i, current: %i)' % (
+            expected_count, current_count)
+        self.assertEqual(current_count, expected_count, msg)
 
     def test_extract_user_details_sreg(self):
         expected = {
@@ -217,23 +231,53 @@
         self.assertEqual("Some56789012345678901234567890",  user.first_name)
         self.assertEqual("User56789012345678901234567890",  user.last_name)
 
+    def _test_update_user_perms_account_verified(
+            self, user, initially_verified, verified):
+        # set user's verification status
+        permission = Permission.objects.get(codename='account_verified')
+        if initially_verified:
+            user.user_permissions.add(permission)
+        else:
+            user.user_permissions.remove(permission)
+
+        if initially_verified:
+            self.assert_account_verified(user)
+        else:
+            self.assert_account_not_verified(user)
+
+        # get a response including verification status
+        response = self.make_response_ax()
+        data = dict(first_name=u"Some56789012345678901234567890123",
+                    last_name=u"User56789012345678901234567890123",
+                    email=u"[e-mail]",
+                    account_verified=verified)
+        self.backend.update_user_details(user, data, response)
+
+        # refresh object from the database
+        user = User.objects.get(pk=user.pk)
+
+        if verified:
+            self.assert_account_verified(user)
+        else:
+            self.assert_account_not_verified(user)
+
     def test_update_user_perms_initially_verified_then_verified(self):
-        self.assert_account_verified(
+        self._test_update_user_perms_account_verified(
             self.make_user_openid().user,
             initially_verified=True, verified=True)
 
     def test_update_user_perms_initially_verified_then_unverified(self):
-        self.assert_account_verified(
+        self._test_update_user_perms_account_verified(
             self.make_user_openid().user,
             initially_verified=True, verified=False)
 
     def test_update_user_perms_initially_not_verified_then_verified(self):
-        self.assert_account_verified(
+        self._test_update_user_perms_account_verified(
             self.make_user_openid().user,
             initially_verified=False, verified=True)
 
     def test_update_user_perms_initially_not_verified_then_unverified(self):
-        self.assert_account_verified(
+        self._test_update_user_perms_account_verified(
             self.make_user_openid().user,
             initially_verified=False, verified=False)
 
@@ -266,6 +310,7 @@
                 expected,
                 self.backend._get_preferred_username(nick, email))
 
+    @override_settings(OPENID_USE_EMAIL_FOR_USERNAME=False)
     def test_preferred_username_no_email_munging(self):
         for nick, email, expected in [
                 ('nickcomesfirst', '[e-mail]', 'nickcomesfirst'),
@@ -388,3 +433,277 @@
         user = self.backend.authenticate(openid_response=response)
 
         self.assertIsNone(user)
+
+    def test_auth_no_response(self):
+        self.assertIsNone(self.backend.authenticate())
+        self.assert_no_users_created()
+
+    def test_auth_cancel_response(self):
+        response = CancelResponse(OpenIDServiceEndpoint())
+
+        self.assertIsNone(self.backend.authenticate(openid_response=response))
+        self.assert_no_users_created()
+
+    def test_auth_failure_response(self):
+        response = FailureResponse(OpenIDServiceEndpoint())
+
+        self.assertIsNone(self.backend.authenticate(openid_response=response))
+        self.assert_no_users_created()
+
+    def test_auth_setup_needed_response(self):
+        response = SetupNeededResponse(OpenIDServiceEndpoint())
+
+        self.assertIsNone(self.backend.authenticate(openid_response=response))
+        self.assert_no_users_created()
+
+    @override_settings(OPENID_CREATE_USERS=False)
+    def test_auth_no_create_users(self):
+        response = self.make_openid_response(
+            sreg_args=dict(email='[e-mail]'),
+            teams_args=dict(is_member='foo'))
+        user = self.backend.authenticate(openid_response=response)
+
+        self.assertIsNone(user)
+        self.assert_no_users_created()
+
+    @override_settings(OPENID_CREATE_USERS=False)
+    def test_auth_no_create_users_existing_user(self):
+        response = self.make_openid_response(
+            sreg_args=dict(email='[e-mail]'),
+            teams_args=dict(is_member='foo'))
+        existing_openid = self.make_user_openid(
+            claimed_id=response.identity_url)
+        expected_user_count = User.objects.count()
+
+        user = self.backend.authenticate(openid_response=response)
+
+        self.assertIsNotNone(user)
+        self.assertEqual(user, existing_openid.user)
+        self.assert_no_users_created(expected_count=expected_user_count)
+
+    @override_settings(
+        OPENID_UPDATE_DETAILS_FROM_SREG=True,
+        OPENID_VALID_VERIFICATION_SCHEMES={
+            'http://example.com/': {'token_via_email'}})
+    def test_auth_update_details_from_sreg(self):
+        first_name = 'a' * 31
+        last_name = 'b' * 31
+        email = '[e-mail]'
+        response = self.make_response_ax(
+            fullname=first_name + ' ' + last_name,
+            nickname='newnickname',
+            email=email,
+            first=first_name,
+            last=last_name,
+            verified=True,
+        )
+        existing_openid = self.make_user_openid(
+            claimed_id=response.identity_url)
+        original_username = existing_openid.user.username
+        expected_user_count = User.objects.count()
+
+        self.assert_account_not_verified(existing_openid.user)
+
+        user = self.backend.authenticate(openid_response=response)
+
+        self.assertEqual(user, existing_openid.user)
+        self.assertEqual(
+            user.username, original_username,
+            'Username must not be updated unless OPENID_FOLLOW_RENAMES=True.')
+        self.assertEqual(user.email, email)
+        self.assertEqual(user.first_name, first_name[:30])
+        self.assertEqual(user.last_name, last_name[:30])
+        self.assert_account_verified(user)
+        self.assert_no_users_created(expected_count=expected_user_count)
+
+    @override_settings(
+        OPENID_UPDATE_DETAILS_FROM_SREG=True,
+        OPENID_VALID_VERIFICATION_SCHEMES={
+            'http://example.com/': {'token_via_email'}})
+    def test_auth_update_details_from_sreg_unverifies_account(self):
+        first_name = 'a' * 31
+        last_name = 'b' * 31
+        email = '[e-mail]'
+        kwargs = dict(
+            fullname=first_name + ' ' + last_name,
+            nickname='newnickname',
+            email=email,
+            first=first_name,
+            last=last_name,
+            verified=True,
+        )
+        verified_response = self.make_response_ax(**kwargs)
+        verified_user = self.backend.authenticate(
+            openid_response=verified_response)
+
+        self.assert_account_verified(verified_user)
+        expected_user_count = User.objects.count()
+
+        kwargs['verified'] = False
+        unverified_response = self.make_response_ax(**kwargs)
+        unverified_user = self.backend.authenticate(
+            openid_response=unverified_response)
+
+        self.assertEqual(verified_user, unverified_user)
+        self.assert_account_not_verified(unverified_user)
+        self.assert_no_users_created(expected_count=expected_user_count)
+
+    @override_settings(OPENID_PHYSICAL_MULTIFACTOR_REQUIRED=True)
+    def test_physical_multifactor_required_not_given(self):
+        response = self.make_openid_response()
+
+        with self.assertRaises(MissingPhysicalMultiFactor):
+            self.backend.authenticate(openid_response=response)
+
+        self.assertTrue(
+            UserOpenID.objects.filter(claimed_id='some-id').exists(),
+            'User must be created anyways.')
+
+    @override_settings(OPENID_PHYSICAL_MULTIFACTOR_REQUIRED=True)
+    def test_physical_multifactor_required_invalid_auth_policy(self):
+        response = self.make_openid_response()
+        message = response.message
+        message.setArg(
+            pape.ns_uri, 'auth_policies',
+            pape.AUTH_MULTI_FACTOR + ' ' + pape.AUTH_PHISHING_RESISTANT)
+        response = SuccessResponse(
+            response.endpoint, message,
+            signed_fields=message.toPostArgs().keys())
+
+        with self.assertRaises(MissingPhysicalMultiFactor):
+            self.backend.authenticate(openid_response=response)
+
+        self.assertTrue(
+            UserOpenID.objects.filter(claimed_id='some-id').exists(),
+            'User must be created anyways.')
+
+    @override_settings(OPENID_PHYSICAL_MULTIFACTOR_REQUIRED=True)
+    def test_physical_multifactor_required_valid_auth_policy(self):
+        response = self.make_openid_response()
+        message = response.message
+        message.setArg(
+            pape.ns_uri, 'auth_policies',
+            pape.AUTH_MULTI_FACTOR_PHYSICAL)
+        response = SuccessResponse(
+            response.endpoint, message,
+            signed_fields=message.toPostArgs().keys())
+
+        user = self.backend.authenticate(openid_response=response)
+
+        self.assertIsNotNone(user)
+
+    @override_settings(OPENID_STRICT_USERNAMES=True)
+    def test_auth_strict_usernames(self):
+        username = 'nickname'
+        response = self.make_openid_response(
+            sreg_args=dict(nickname=username, email='[e-mail]'),
+            teams_args=dict(is_member='foo'))
+        user = self.backend.authenticate(openid_response=response)
+
+        self.assertIsNotNone(user, 'User must be created')
+        self.assertEqual(user.username, username)
+
+    @override_settings(OPENID_STRICT_USERNAMES=True)
+    def test_auth_strict_usernames_no_nickname(self):
+        response = self.make_openid_response(
+            sreg_args=dict(nickname='', email='[e-mail]'),
+            teams_args=dict(is_member='foo'))
+
+        msg = re.escape(
+            "An attribute required for logging in was not returned (nickname)")
+
+        with self.assertRaisesRegexp(RequiredAttributeNotReturned, msg):
+            self.backend.authenticate(openid_response=response)
+
+        self.assert_no_users_created()
+
+    @override_settings(
+        OPENID_STRICT_USERNAMES=True,
+        OPENID_UPDATE_DETAILS_FROM_SREG=True)
+    def test_auth_strict_usernames_conflict(self):
+        existing_openid = self.make_user_openid()
+        expected_user_count = User.objects.count()
+
+        response = self.make_openid_response(
+            sreg_args=dict(
+                nickname=existing_openid.user.username, email='[e-mail]'),
+            teams_args=dict(is_member='foo'))
+
+        with self.assertRaises(DuplicateUsernameViolation):
+            self.backend.authenticate(openid_response=response)
+
+        self.assert_no_users_created(expected_count=expected_user_count)
+
+    @override_settings(
+        OPENID_FOLLOW_RENAMES=True,
+        OPENID_STRICT_USERNAMES=True,
+        OPENID_UPDATE_DETAILS_FROM_SREG=True)
+    def test_auth_follow_renames(self):
+        new_username = 'new'
+        original_response = self.make_openid_response(
+            sreg_args=dict(nickname='username', email='[e-mail]'),
+            teams_args=dict(is_member='foo'))
+        rename_response = self.make_openid_response(
+            sreg_args=dict(nickname=new_username, email='[e-mail]'),
+            teams_args=dict(is_member='foo'))
+        user = self.backend.authenticate(openid_response=original_response)
+        expected_user_count = User.objects.count()
+
+        self.assertIsNotNone(user, 'User must be created')
+
+        renamed_user = self.backend.authenticate(
+            openid_response=rename_response)
+
+        self.assertEqual(user.pk, renamed_user.pk)
+        self.assertEqual(renamed_user.username, new_username)
+        self.assert_no_users_created(expected_count=expected_user_count)
+
+    @override_settings(
+        OPENID_FOLLOW_RENAMES=True,
+        OPENID_STRICT_USERNAMES=True,
+        OPENID_UPDATE_DETAILS_FROM_SREG=True)
+    def test_auth_follow_renames_strict_usernames_no_nickname(self):
+        response = self.make_openid_response(
+            sreg_args=dict(nickname='nickame', email='[e-mail]'),
+            teams_args=dict(is_member='foo'))
+        user = self.backend.authenticate(openid_response=response)
+        expected_user_count = User.objects.count()
+
+        self.assertIsNotNone(user, 'User must be created')
+
+        response = self.make_openid_response(
+            sreg_args=dict(nickname='', email='[e-mail]'),
+            teams_args=dict(is_member='foo'))
+
+        # XXX: Check possibilities to normalize this error into a
+        # `RequiredAttributeNotReturned`.
+        with self.assertRaises(MissingUsernameViolation):
+            self.backend.authenticate(openid_response=response)
+
+        self.assert_no_users_created(expected_count=expected_user_count)
+
+    @override_settings(
+        OPENID_FOLLOW_RENAMES=True,
+        OPENID_STRICT_USERNAMES=True,
+        OPENID_UPDATE_DETAILS_FROM_SREG=True)
+    def test_auth_follow_renames_strict_usernames_rename_conflict(self):
+        existing_openid = self.make_user_openid()
+        original_username = 'nickame'
+        good_response = self.make_openid_response(
+            sreg_args=dict(nickname=original_username, email='[e-mail]'),
+            teams_args=dict(is_member='foo'))
+        conflict_response = self.make_openid_response(
+            sreg_args=dict(
+                nickname=existing_openid.user.username, email='[e-mail]'),
+            teams_args=dict(is_member='foo'))
+        user = self.backend.authenticate(openid_response=good_response)
+        expected_user_count = User.objects.count()
+
+        self.assertIsNotNone(user, 'First request should succeed')
+
+        with self.assertRaises(DuplicateUsernameViolation):
+            self.backend.authenticate(openid_response=conflict_response)
+
+        db_user = User.objects.get(pk=user.pk)
+        self.assertEqual(db_user.username, original_username)
+        self.assert_no_users_created(expected_count=expected_user_count)

=== modified file 'tox.ini'
--- tox.ini	2016-08-10 21:32:13 +0000
+++ tox.ini	2016-08-11 11:45:39 +0000
@@ -10,7 +10,7 @@
 [testenv:py2.7-django1.7]
 basepython = python2.7
 deps =
-    django >= 1.7, < 1.8
+    django == 1.7.11
     {[testenv]deps}
 
 [testenv:py2.7-django1.8]