Merge lp:~canonical-isd-hackers/canonical-identity-provider/2f-reset into lp:canonical-identity-provider/release

[David Owen]

Reset the user's preference to "only when a site asks" when removing the last two-factor device. Additionally, if the flag is set for "required" during authentication but the account has no devices, log an error and proceed with normal (not two-factor) authentication.

=== modified file 'identityprovider/models/account.py'
--- identityprovider/models/account.py	2012-01-18 16:24:23 +0000
+++ identityprovider/models/account.py	2012-02-02 17:13:33 +0000
@@ -312,6 +312,12 @@
         except Consumer.DoesNotExist:
             return []
 
+    def has_twofactor_devices(self):
+        """Returns True if this Account has any associated two-factor
+        devices (including one-time pads)."""
+        # TODO: Test for one-time pads.
+        return self.devices.exists()
+
     def save(self, force_insert=False, force_update=False, **kwargs):
         if settings.READ_ONLY_MODE:
             return
 
=== modified file 'identityprovider/tests/unit/test_devices.py'
--- identityprovider/tests/unit/test_devices.py	2012-01-27 12:31:00 +0000
+++ identityprovider/tests/unit/test_devices.py	2012-02-02 17:13:33 +0000
@@ -252,6 +252,30 @@
 
         self.assertEqual(AuthenticationDevice.objects.all().count(), 0)
 
+    def test_preference_reset(self):
+        account = self._get_account()
+        account.twofactor_required = True
+        mock_request = self._get_mock_request()
+        mock_request.user = account
+        mock_request.method = 'POST'
+
+        device1 = AuthenticationDevice.objects.create(
+            account=account,
+            key='some key',
+            name='First device'
+        )
+        device2 = AuthenticationDevice.objects.create(
+            account=account,
+            key='some key',
+            name='Second device'
+        )
+
+        device_removal(mock_request, device1.id)
+        self.assertTrue(account.twofactor_required)
+
+        device_removal(mock_request, device2.id)
+        self.assertFalse(account.twofactor_required)
+
 
 class TestPrefs(TwoFactorTestCase):
 
 
=== modified file 'identityprovider/tests/unit/test_views_ui.py'
--- identityprovider/tests/unit/test_views_ui.py	2012-01-30 14:42:31 +0000
+++ identityprovider/tests/unit/test_views_ui.py	2012-02-02 17:13:33 +0000
@@ -1196,6 +1196,20 @@
         oath = inputs[0]
         self.assertEqual(oath.attrib['autocomplete'], 'off')
 
+    def test_user_requires_twofactor_auth(self):
+        def f(require2f, has_devices, returns, logs):
+            with patch('logging.warning') as mock_warning:
+                account = Mock()
+                account.twofactor_required = require2f
+                account.has_twofactor_devices.return_value = has_devices
+                r = ui.user_requires_twofactor_auth(account)
+                self.assertEqual(returns, r)
+                self.assertEqual(logs, mock_warning.called)
+        f(require2f=False, has_devices=False, returns=False, logs=False)
+        f(require2f=False, has_devices=True, returns=False, logs=False)
+        f(require2f=True, has_devices=False, returns=False, logs=True)
+        f(require2f=True, has_devices=True, returns=True, logs=False)
+
     @patch('identityprovider.views.ui.user_requires_twofactor_auth')
     def test_when_user_requires_twofactor_redirected(self, mock_user):
         mock_user.return_value = True
 
=== modified file 'identityprovider/views/devices.py'
--- identityprovider/views/devices.py	2012-01-30 14:42:31 +0000
+++ identityprovider/views/devices.py	2012-02-02 17:13:33 +0000
@@ -201,9 +201,15 @@
         ))
 
     device.delete()
+
     # We should probably send an e-mail to the user stating which
     # device was removed. As a security measure, this would be much
     # stronger if bugs #784813, #784817, and #784818 were done.
+
+    if not request.user.has_twofactor_devices():
+        request.user.twofactor_required = False
+        request.user.save()
+
     return HttpResponseSeeOther('/device-list')
 
 
 
=== modified file 'identityprovider/views/ui.py'
--- identityprovider/views/ui.py	2012-01-30 14:42:31 +0000
+++ identityprovider/views/ui.py	2012-02-02 17:13:33 +0000
@@ -102,7 +102,11 @@
 def user_requires_twofactor_auth(account):
     if not gargoyle.is_active('TWOFACTOR'):
         return False
-    return account.twofactor_required
+    if account.twofactor_required:
+        if account.has_twofactor_devices():
+            return True
+        logging.warning('Account %s requires two-factor but has no devices' % account.openid_identifier)
+    return False