Merge ~roadmr/canonical-identity-provider:2fa-always-generate-paper-backup into canonical-identity-provider:master

[Daniel Manrique]

Automatically generate and show a paper backup device when a non-paper device is added and no backup device existed.

This is controllable with a feature flag.

As discussed with design team, this has an "ugly view" that should be tweaked before going to production. Feature flag can be enabled on staging for testing, or the feature can be tested locally as well.

QA tips:

- Enable flag in staging
- Add a device on an account with no paper backup devices (e.g. delete your backup first or create a new account).
- After verifying your new device you will be inconditionally shown a set of just-generated backup codes with a prompt to print or store them.

Possibly pending things:

- We're adding two devices so two flash messages are shown, this could be consolidated into one if the current behavior is ugly.
- Ugly view can be invoked separately on any existing paper device (aids with easier QA) like so:

https://login.ubuntu.com/device-print/some-numeric-id?source=auto

[Hasan Ammar (hasanammar)]

Code looks good. Tested locally and works as expected.

review: Approve

diff --git a/src/webui/templates/device/new-paper-device-confirmation.html b/src/webui/templates/device/new-paper-device-confirmation.html
new file mode 100644
index 0000000..1fed783
--- /dev/null
+++ b/src/webui/templates/device/new-paper-device-confirmation.html
@@ -0,0 +1,6 @@
+{% extends "device/print-codes.html" %}
+{% load i18n %}
+{%block header_intro %}
+<h2>Automatically generated printable backup codes</h2>
+<p><b>{% trans "In addition to the device you just added, we have generated the following list of codes as backup in case your primary device becomes lost or unusable." %}</b></p>
+{% endblock %}
diff --git a/src/webui/templates/device/print-codes.html b/src/webui/templates/device/print-codes.html
index 0073c4f..e3d55e7 100644
--- a/src/webui/templates/device/print-codes.html
+++ b/src/webui/templates/device/print-codes.html
@@ -16,7 +16,9 @@ Affero General Public License version 3 (see the file LICENSE).
 {% block text_title %}{% trans "My account" %}{% endblock %}
 
 {% block content %}
+  {% block header_intro %}
   <h2>{% trans "Printable backup codes" %}</h2>
+  {% endblock %}
   <p>{% trans "Print this list of backup codes and keep them safe." %}</p>
   <p>{% trans "The codes must be used in the order shown below." %}</p>
   <p>{% trans "Each code can be used only once, please cross it out once you've used it." %}</p>
diff --git a/src/webui/tests/test_views_devices.py b/src/webui/tests/test_views_devices.py
index dc764d6..4c1586b 100644
--- a/src/webui/tests/test_views_devices.py
+++ b/src/webui/tests/test_views_devices.py
@@ -349,7 +349,8 @@ class DeviceAdditionViewTestCase(DeviceViewsTestCaseBase):
 
     def assert_successful_post(
             self, name='Some device', counter=1, device_type='generic',
-            oath_mode='hotp', hex_key=KEY, otp='755224'):
+            oath_mode='hotp', hex_key=KEY, otp='755224',
+            check_redirect=True):
 
         data = {'type': device_type, 'name': name}
         expected_type = device_type
@@ -366,12 +367,14 @@ class DeviceAdditionViewTestCase(DeviceViewsTestCaseBase):
         self.assertEqual(devices.count(), 1)
         device = devices[0]
 
-        if device_type == 'paper':
-            redirect_to = reverse(
-                'device-print', kwargs=dict(device_id=device.id))
-        else:
-            redirect_to = reverse('device-list')
-        self.assertRedirects(response, redirect_to, status_code=303)
+        if check_redirect:
+            if device_type == 'paper':
+                redirect_to = reverse(
+                    'device-print', kwargs=dict(device_id=device.id))
+            else:
+                redirect_to = reverse('device-list')
+
+            self.assertRedirects(response, redirect_to, status_code=303)
 
         self.assertLess(before, self.client.session[twofactor.TWOFACTOR_LOGIN])
         self.assertLess(self.client.session[twofactor.TWOFACTOR_LOGIN], after)
@@ -482,6 +485,74 @@ class DeviceAdditionViewTestCase(DeviceViewsTestCaseBase):
         self.assert_successful_post(
             name='Printable Backup Codes', counter=0, device_type='paper')
 
+    # When the auto backup deviceflag is set...
+    @switches(TWOFACTOR_AUTO_BACKUP_DEVICE=True)
+    def test_successful_post_generic_no_paper_auto_add(self):
+        devices = self.account.devices.all()
+        self.assertEqual(devices.count(), 0)
+        # Adding the account's first device...
+        response = self.assert_successful_post(
+            device_type='generic', check_redirect=False)
+        devices = self.account.devices.all()
+        self.assertEqual(devices.filter(device_type="generic:hotp").count(), 1)
+        # Adds a paper backup device magically
+        self.assertEqual(devices.filter(device_type="paper").count(), 1)
+        backup_device = devices.filter(device_type="paper").first()
+        # And the hex key is different
+        keys = devices.values_list('key')
+        self.assertNotEqual(keys[0], keys[1])
+        redirect_to = (
+            reverse(
+                'device-print',
+                kwargs=dict(device_id=backup_device.id)) +
+            "?source=auto"
+        )
+        self.assertRedirects(response, redirect_to, status_code=303)
+
+    # When the auto backup deviceflag is unset...
+    @switches(TWOFACTOR_AUTO_BACKUP_DEVICE=False)
+    def test_successful_post_generic_no_paper_no_auto_add(self):
+        devices = self.account.devices.all()
+        self.assertEqual(devices.count(), 0)
+        # Adding the account's first device...
+        self.assert_successful_post(device_type='generic')
+        devices = self.account.devices.all()
+        self.assertEqual(devices.filter(device_type="generic:hotp").count(), 1)
+        # Does not add a paper backup device
+        self.assertEqual(devices.filter(device_type="paper").count(), 0)
+
+    # When the auto backup deviceflag is set...
+    @switches(TWOFACTOR_AUTO_BACKUP_DEVICE=True)
+    def test_successful_post_generic_paper_no_add(self):
+        # And a paper backup device already exists...
+        self.factory.make_device(
+            account=self.account, device_type='paper')
+        devices = self.account.devices.all()
+        self.assertEqual(devices.count(), 1)
+        # Simulating having authenticated with this existing device...
+        self.do_2fa_login(devices.first())
+        # and Adding the account's first real device...
+        self.assert_successful_post(device_type='generic')
+        devices = self.account.devices.all()
+        self.assertEqual(devices.filter(device_type="generic:hotp").count(), 1)
+        # Does not add additional paper devices.
+        self.assertEqual(devices.filter(device_type="paper").count(), 1)
+
+    # When the auto backup deviceflag is set...
+    @switches(TWOFACTOR_AUTO_BACKUP_DEVICE=True)
+    def test_successful_post_paper_main_no_add(self):
+        devices = self.account.devices.all()
+        self.assertEqual(devices.count(), 0)
+        # Adding the account's first device as a paper device...
+        # (note verification uses counter=0 vs. the default of 1 because
+        # we don't test the first code for paper devices.
+        self.assert_successful_post(
+            device_type='paper', name='Printable Backup Codes', counter=0)
+        devices = self.account.devices.all()
+        self.assertEqual(devices.count(), 1)
+        # Does not add additional paper devices.
+        self.assertEqual(devices.filter(device_type="paper").count(), 1)
+
     def test_unrecognised_type(self):
         data = {'type': 'foobar', 'hex_key': KEY, 'name': 'Some device',
                 'otp': 'some otp key'}
@@ -696,6 +767,15 @@ class DevicePrintViewTestCase(DeviceViewsTestCaseBase):
         for i in range(settings.TWOFACTOR_PAPER_CODES_ALLOW_GENERATION):
             self.assert_generation_warning_and_button(i)
 
+    def test_autogenerated_template_parameters(self):
+        # If ?source=auto is passed as a query parameter...
+        response = self.client.get(self.print_url + "?source=auto")
+        # Use a different template to render the response
+        self.assertTemplateUsed(
+            response, 'device/new-paper-device-confirmation.html')
+        # and context should contain NO current section
+        self.assertIsNone(response.context['current_section'])
+
 
 class DeviceAuthenticationFailuretestCase(DeviceViewsTestCaseBase):
 
diff --git a/src/webui/views/devices.py b/src/webui/views/devices.py
index 9e78d4b..3f7716b 100644
--- a/src/webui/views/devices.py
+++ b/src/webui/views/devices.py
@@ -176,6 +176,27 @@ def _device_addition_standard(request, device_type):
             if accepted:
                 _create_device(request, device_name, hex_key, new_device_value,
                                device_type, oath_type)
+                # If the flag is active and the account has no paper devices,
+                # force add one.
+                if gargoyle.is_active('TWOFACTOR_AUTO_BACKUP_DEVICE', request):
+                    paper_devices = request.user.devices.filter(
+                            device_type='paper')
+                    if paper_devices.count() < 1:
+                        backup_hex_key = generate_key(20)
+                        device_name = get_unique_device_name_for_user(
+                            device_types['paper'],
+                            request.user)
+                        backup_device = _create_device(
+                            request, device_name, backup_hex_key, 0, 'paper')
+                        # If a device was auto-added, send to a nice page.
+                        # Very similar to device-print view with some tweaks
+                        # for auto-generated device.
+                        return HttpResponseSeeOther(reverse(
+                            'device-print',
+                            args=(backup_device.id,),
+                            ) + "?source=auto"
+                        )
+
                 return HttpResponseSeeOther(reverse(DEVICE_LIST))
 
             # Otherwise, set the error flag and fall through...
@@ -226,12 +247,17 @@ def device_print(request, device_id, token=None):
         AuthenticationDevice, id=device_id, account=request.user,
         device_type='paper')
 
+    source = request.GET.get('source', '')
+
     details = _codes_for_position(device)
     remaining_codes = settings.TWOFACTOR_PAPER_CODES - details.position
+    # don't show generation or renewal link for source=auto
     generation_enabled = (
-        remaining_codes <= settings.TWOFACTOR_PAPER_CODES_ALLOW_GENERATION)
+        remaining_codes <= settings.TWOFACTOR_PAPER_CODES_ALLOW_GENERATION
+        and source != "auto")
     needs_renewal = (
-        remaining_codes < settings.TWOFACTOR_PAPER_CODES_WARN_RENEWAL)
+        remaining_codes < settings.TWOFACTOR_PAPER_CODES_WARN_RENEWAL
+        and source != "auto")
 
     if generation_enabled:
         messages.warning(request, DEVICE_GENERATION_WARNING)
@@ -246,7 +272,12 @@ def device_print(request, device_id, token=None):
     )
     if token is not None:
         context['token'] = token
-    return TemplateResponse(request, 'device/print-codes.html', context)
+    if source == "auto":
+        context['current_section'] = None
+        template = "device/new-paper-device-confirmation.html",
+    else:
+        template = "device/print-codes.html"
+    return TemplateResponse(request, template, context)
 
 
 def _codes_for_position(device, next_page=False):