Merge lp:~jamestait/canonical-identity-provider/lp1648775-force-exhausted-paper-renewal into lp:canonical-identity-provider/release

[James Tait]

Redirect the user to the device list page and remind them to generate and print new paper codes if their current and only list is near exhaustion.

Redirect the user to the device list page and remind them to generate and
print new paper codes if their current and only list is near exhaustion.

The TwoFactorLoginTestCase has been updated to include the login token in the
URLs - I believe this is valid because redirection to the twofactor view will
now always include that token. The token is then passed between views to
enable the "Continue" link from the device-print view to redirect the user to
their original location in the RP.

Note that if the user has two sets of printed codes and only one is close to exhaustion, they will not be forced to renew the codes. In a follow-up branch, they will be prompted to renew via email.

[Ricardo Kirkner (ricardokirkner)]

review: Approve

=== modified file 'src/identityprovider/tests/utils.py'
--- src/identityprovider/tests/utils.py	2016-12-22 17:43:18 +0000
+++ src/identityprovider/tests/utils.py	2017-04-24 17:17:28 +0000
@@ -66,7 +66,7 @@
   Please print a new list for the following devices."""
 
 
-def assert_exausted_warning(testcase, devices, response):
+def assert_exhausted_warning(testcase, devices, response):
     """Helper to assert warning is correctly displayed"""
     dom = PyQuery(response.content)
     warning = dom.find('#exhausted_warning')
 
=== modified file 'src/webui/templates/common/printed_codes_nearly_exhausted_warning.html'
--- src/webui/templates/common/printed_codes_nearly_exhausted_warning.html	2013-04-17 13:23:35 +0000
+++ src/webui/templates/common/printed_codes_nearly_exhausted_warning.html	2017-04-24 17:17:28 +0000
@@ -1,11 +1,11 @@
-{% load i18n %}
+{% load i18n url_with_token %}
 
 {% if paper_devices_needing_renewal %}
 <div id="exhausted_warning" class="message warning">
   <p>{% blocktrans %}Your printed list of backup codes is nearly used up.
   Please print a new list for the following devices.{% endblocktrans %}</p>
   {% for device in paper_devices_needing_renewal %}
-  <p>{{ device.name }} <a href="{% url 'device-print' device.id %}" class="btn-sm"><span>{% trans 'Generate New Codes' %}</span></a></p>
+  <p>{{ device.name }} <a href="{% url_with_token 'device-print' device_id=device.id %}" class="btn-sm"><span>{% trans 'Generate New Codes' %}</span></a></p>
   {% endfor %}
 </div>
 {% endif %}
 
=== modified file 'src/webui/templates/device/print-codes.html'
--- src/webui/templates/device/print-codes.html	2014-12-09 21:41:56 +0000
+++ src/webui/templates/device/print-codes.html	2017-04-24 17:17:28 +0000
@@ -1,5 +1,5 @@
 {% extends "base.html" %}
-{% load i18n %}
+{% load i18n url_with_token %}
 
 {% comment %}
 Copyright 2012 Canonical Ltd.  This software is licensed under the GNU
@@ -30,9 +30,13 @@
   <button class="btn cta" id="printbtn" onclick="window.print()" data-qa-id="print_codes">
     <span>{% trans "Print Codes" %}</span>
   </button>
+  {% if token and not needs_renewal %}
+  <a class="cta secondary" href="{% url_with_token 'server-decide' %}" data-qa-id="continue_to_rp">{% trans "Continue" %}</a>
+  {% else %}
   <a class="cta secondary" href="{% url 'device-list' %}" data-qa-id="cancel_print_codes">{% trans "Go back" %}</a>
+  {% endif %}
   {% if generation_enabled %}
-  <a class="btn cta secondary print-new-codes" href="{% url 'device-generate' device_id %}" data-qa-id="generate_codes">
+  <a class="btn cta secondary print-new-codes" href="{% url_with_token 'device-generate' device_id=device_id %}" data-qa-id="generate_codes">
     <span>{% trans "Generate new codes" %}</span>
   </a>
   {% endif %}
 
=== modified file 'src/webui/tests/test_views_account.py'
--- src/webui/tests/test_views_account.py	2017-02-26 13:52:43 +0000
+++ src/webui/tests/test_views_account.py	2017-04-24 17:17:28 +0000
@@ -38,7 +38,7 @@
     MISSING_BACKUP_DEVICE,
     AuthenticatedTestCase,
     SSOBaseTestCase,
-    assert_exausted_warning,
+    assert_exhausted_warning,
 )
 from identityprovider.views.testing import MockLaunchpad
 from webui.constants import EMAIL_EXISTS_ERROR
@@ -639,10 +639,10 @@
 
         if (self.twofactor_enabled and self.devices_count > 0 and
                 self.paper_device_exhausted):
-            assert_exausted_warning(self, devices, response)
+            assert_exhausted_warning(self, devices, response)
         else:
             with self.assertRaises(AssertionError):
-                assert_exausted_warning(self, devices, response)
+                assert_exhausted_warning(self, devices, response)
 
     def test_device_preferences_with_twofactor_disabled(self):
         name = 'webui.views.account.is_twofactor_enabled'
 
=== modified file 'src/webui/tests/test_views_devices.py'
--- src/webui/tests/test_views_devices.py	2017-01-26 17:26:43 +0000
+++ src/webui/tests/test_views_devices.py	2017-04-24 17:17:28 +0000
@@ -25,7 +25,7 @@
     MISSING_BACKUP_DEVICE,
     AuthenticatedTestCase,
     SSOBaseTransactionTestCase,
-    assert_exausted_warning,
+    assert_exhausted_warning,
     test_concurrently,
 )
 
@@ -152,7 +152,7 @@
 
         response = self.client.get(self.url)
         with self.assertRaises(AssertionError):
-            assert_exausted_warning(self, [device], response)
+            assert_exhausted_warning(self, [device], response)
 
     def test_codes_exhausted_warning_exhausted(self):
         counter = (settings.TWOFACTOR_PAPER_CODES -
@@ -161,7 +161,17 @@
             account=self.account, device_type='paper', counter=counter)
 
         response = self.client.get(self.url)
-        assert_exausted_warning(self, [device], response)
+        assert_exhausted_warning(self, [device], response)
+
+    def test_codes_exhausted_warning_exhausted_with_login_token(self):
+        counter = (settings.TWOFACTOR_PAPER_CODES -
+                   settings.TWOFACTOR_PAPER_CODES_WARN_RENEWAL + 1)
+        device = self.factory.make_device(
+            account=self.account, device_type='paper', counter=counter)
+
+        url = reverse('device-list', kwargs=dict(token='a' * 16))
+        response = self.client.get(url)
+        assert_exhausted_warning(self, [device], response)
 
     def test_device_list_no_devices(self):
         assert self.account.devices.count() == 0
@@ -582,6 +592,18 @@
             self.assertEqual(len(strike), 0)
             self.assertEqual(elem.text.strip(), code)
 
+    def test_device_print_with_login_token(self):
+        counter = (settings.TWOFACTOR_PAPER_CODES -
+                   settings.TWOFACTOR_PAPER_CODES_WARN_RENEWAL + 1)
+        device = self.factory.make_device(
+            account=self.account, device_type='paper', counter=counter)
+
+        url = reverse('device-print', kwargs=dict(device_id=device.id,
+                                                  token='a' * 16))
+        response = self.client.get(url)
+
+        self.assertContains(response, 'Go back')
+
     def test_twofactor_disabled(self):
         self.assert_twofactor_disabled(self.print_url)
         self.assert_twofactor_disabled(self.generate_url)
 
=== modified file 'src/webui/tests/test_views_ui.py'
--- src/webui/tests/test_views_ui.py	2017-04-13 21:09:42 +0000
+++ src/webui/tests/test_views_ui.py	2017-04-24 17:17:28 +0000
@@ -1594,6 +1594,9 @@
 
 class TwoFactorLoginTestCase(BaseTwoFactorLoginTestCase):
 
+    url = reverse('twofactor', kwargs=dict(token='a' * 16))
+    logout_link = 'a[href="/aaaaaaaaaaaaaaaa/+logout"]'
+
     def setUp(self):
         super(TwoFactorLoginTestCase, self).setUp()
         self.mock_site = self.patch(
@@ -1640,7 +1643,7 @@
 
     def test_when_user_requires_twofactor_redirected(self):
         self.mock_site.return_value = False
-        r = self.do_login()
+        r = self.do_login(token='a' * 16)
         self.assertRedirects(r, self.url)
 
     def test_when_user_requires_twofactor_redirected_with_token(self):
@@ -1705,6 +1708,39 @@
         self.assertRedirects(r, reverse('account-edit'))
         mock_auth.assert_called_once_with(self.account, '123456')
 
+    def test_twofactor_step_redirected_to_device_list_near_exhaustion(self):
+        counter = (settings.TWOFACTOR_PAPER_CODES -
+                   settings.TWOFACTOR_PAPER_CODES_WARN_RENEWAL + 1)
+        self.factory.make_device(
+            account=self.account, device_type='paper', counter=counter)
+        mock_auth = self.patch(
+            'webui.views.ui.twofactor.authenticate', return_value=True)
+
+        self.client.login(username=self.email, password=self.password)
+        r = self.do_twofactor(oath_token='123456')
+
+        mock_auth.assert_called_once_with(self.account, '123456')
+        url = reverse('device-list', kwargs=dict(token='a' * 16))
+        self.assertRedirects(r, url)
+
+    def test_twofactor_step_redirected_to_next_near_exhaustion_of_one(self):
+        """If we have paper devices not near exhaustion, don't force refresh"""
+        counter = (settings.TWOFACTOR_PAPER_CODES -
+                   settings.TWOFACTOR_PAPER_CODES_WARN_RENEWAL + 1)
+        self.factory.make_device(
+            account=self.account, device_type='paper', counter=counter)
+        self.factory.make_device(
+            account=self.account, device_type='paper')
+        mock_auth = self.patch(
+            'webui.views.ui.twofactor.authenticate', return_value=True)
+
+        self.client.login(username=self.email, password=self.password)
+        r = self.do_twofactor(next_url=reverse('account-edit'),
+                              oath_token='123456')
+
+        self.assertRedirects(r, reverse('account-edit'))
+        mock_auth.assert_called_once_with(self.account, '123456')
+
     def test_twofactor_step_illformed_token_displays_error(self):
         self.client.login(username=self.email, password=self.password)
         response = self.do_twofactor(next_url=reverse('account-edit'),
 
=== modified file 'src/webui/urls.py'
--- src/webui/urls.py	2017-02-26 01:27:07 +0000
+++ src/webui/urls.py	2017-04-24 17:17:28 +0000
@@ -12,6 +12,7 @@
     'authtoken': '(?P<authtoken>%s)' % AUTHTOKEN_PATTERN,
     'email_address': '(?P<email_address>.+)',
     'fingerprint': '(?P<fingerprint>.+)',
+    'device_id': '(?P<device_id>.+)',
 }
 
 urlpatterns = patterns(
@@ -135,16 +136,21 @@
 urlpatterns += patterns(
     'webui.views.devices',
     url(r'^device-list$', 'device_list', name='device-list'),
+    url(r'^%(token)s/device-list$' % repls, 'device_list', name='device-list'),
     url(r'^device-addition$', 'device_addition', name='device-addition'),
-    url(r'^device-removal/(?P<device_id>.+)$', 'device_removal',
+    url(r'^device-removal/%(device_id)s$' % repls, 'device_removal',
         name='device-removal'),
-    url(r'^device-rename/(?P<device_id>.+)$', 'device_rename',
+    url(r'^device-rename/%(device_id)s$' % repls, 'device_rename',
         name='device-rename'),
     url(r'^\+device-help$', 'device_help', name='device-help'),
-    url(r'^device-print/(?P<device_id>.+)$', 'device_print',
-        name='device-print'),
-    url(r'^device-generate/(?P<device_id>.+)$', 'device_generate',
+    url(r'^device-print/%(device_id)s$' % repls, 'device_print',
+        name='device-print'),
+    url(r'^%(token)s/device-print/%(device_id)s$' % repls, 'device_print',
+        name='device-print'),
+    url(r'^device-generate/%(device_id)s$' % repls, 'device_generate',
         name='device-generate'),
+    url(r'^%(token)s/device-generate/%(device_id)s$' % repls,
+        'device_generate', name='device-generate'),
 )
 
 urlpatterns += patterns(
 
=== modified file 'src/webui/views/devices.py'
--- src/webui/views/devices.py	2017-01-26 17:26:43 +0000
+++ src/webui/views/devices.py	2017-04-24 17:17:28 +0000
@@ -95,13 +95,14 @@
 @sso_login_required
 @require_twofactor_enabled
 @allow_only('GET')
-def device_list(request):
+def device_list(request, token=None):
     paper_renewals = list(request.user.paper_devices_needing_renewal)
     context = context_with_current_section(
         request, device_addition_path=reverse(DEVICE_ADDITION),
         devices=request.user.devices.all(),
         need_backup_device_warning=request.user.need_backup_device_warning,
-        paper_devices_needing_renewal=paper_renewals
+        paper_devices_needing_renewal=paper_renewals,
+        token=token
     )
     return render(request, 'device/list.html', context)
 
@@ -220,7 +221,7 @@
 @sso_login_required(require_twofactor=True, require_twofactor_freshness=True)
 @require_twofactor_enabled
 @allow_only('GET')
-def device_print(request, device_id):
+def device_print(request, device_id, token=None):
     device = get_object_or_404(
         AuthenticationDevice, id=device_id, account=request.user,
         device_type='paper')
@@ -229,6 +230,8 @@
     remaining_codes = settings.TWOFACTOR_PAPER_CODES - details.position
     generation_enabled = (
         remaining_codes <= settings.TWOFACTOR_PAPER_CODES_ALLOW_GENERATION)
+    needs_renewal = (
+        remaining_codes < settings.TWOFACTOR_PAPER_CODES_WARN_RENEWAL)
 
     if generation_enabled:
         messages.warning(request, DEVICE_GENERATION_WARNING)
@@ -239,7 +242,10 @@
         counter=details.position,
         device_id=device.id,
         generation_enabled=generation_enabled,
+        needs_renewal=needs_renewal,
     )
+    if token is not None:
+        context['token'] = token
     return TemplateResponse(request, 'device/print-codes.html', context)
 
 
@@ -258,7 +264,7 @@
 @sso_login_required(require_twofactor=True, require_twofactor_freshness=True)
 @require_twofactor_enabled
 @allow_only('GET', 'POST')
-def device_generate(request, device_id):
+def device_generate(request, device_id, token=None):
     device = get_object_or_404(
         AuthenticationDevice, id=device_id, account=request.user,
         device_type='paper')
@@ -271,12 +277,16 @@
             request,
             codes=details.codes,
             device_id=device.id,
+            token=token,
         )
         return TemplateResponse(request, 'device/generate-codes.html', context)
 
     device.counter = details.start
     device.save()
-    return HttpResponseRedirect(reverse('device-print', args=(device.id,)))
+    kwargs = {'device_id': device.id}
+    if token:
+        kwargs['token'] = token
+    return HttpResponseRedirect(reverse('device-print', kwargs=kwargs))
 
 
 @sso_login_required(require_twofactor=True, require_twofactor_freshness=True)
 
=== modified file 'src/webui/views/ui.py'
--- src/webui/views/ui.py	2017-04-18 20:14:37 +0000
+++ src/webui/views/ui.py	2017-04-24 17:17:28 +0000
@@ -347,6 +347,15 @@
         limitlogin().reset_count(request)
 
         next_url = request.POST.get('next')
+        paper_devices = request.user.devices.filter(device_type='paper')
+        # We only force renewal if all paper devices are at or near exhaustion
+        needs_renewal = (paper_devices and len(paper_devices) ==
+                         len(list(request.user.paper_devices_needing_renewal)))
+        if token and needs_renewal:
+            url = reverse('device-list', kwargs=dict(token=token))
+            if next_url:
+                url += "?next=" + urlquote(next_url)
+            return HttpResponseRedirect(url)
         if next_url and is_safe_redirect_url(next_url):
             return HttpResponseRedirect(next_url)
         elif token: