Merge lp:~twom/canonical-identity-provider/2fa-qrcode-fix into lp:canonical-identity-provider/release

[Tom Wardill]

Mark the output of the qrcode template tag as safe.

Mark the output of the qrcode template tag as safe.

Avoids encoding the google url, but the data is quoted explicitly, so should be safe.

Change notice for django:
https://docs.djangoproject.com/en/dev/releases/1.9/#simple-tag-now-wraps-tag-output-in-conditional-escape

Final output of string:
https://chart.googleapis.com/chart?chs=250x250&chld=L|0&cht=qr&chl=otpauth%3A//totp/UbuntuSSO/zl%40beyfvift.org%3Fsecret%3DABJYTWOCRT5P7QK4NEKTAYN2FWU5PHKI%26period%3D30

[Ricardo Kirkner (ricardokirkner)]

LGTM

review: Approve

[Colin Watson (cjwatson)]

review: Approve

[Otto Co-Pilot (otto-copilot)]

Running landing tests failed
https://jenkins.ols.canonical.com/online-services/job/canonical-identity-provider/86/

=== modified file 'src/identityprovider/templatetags/qrcode.py'
--- src/identityprovider/templatetags/qrcode.py	2016-12-14 10:53:13 +0000
+++ src/identityprovider/templatetags/qrcode.py	2018-03-16 13:31:48 +0000
@@ -2,12 +2,14 @@
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 from base64 import b32encode
-from urllib import quote
+from urllib import quote_plus
 from django import template
 from django.conf import settings
+from django.utils.safestring import mark_safe
 
 register = template.Library()
-URL = 'https://chart.googleapis.com/chart?chs=250x250&chld=L|0&cht=qr&chl=%s'
+URL = ('https://chart.googleapis.com/chart'
+       '?chs=250x250&chld=L%%7C0&cht=qr&chl=%s')
 
 
 def _encode(s):
@@ -30,4 +32,5 @@
         otp_url += '&period={}'.format(settings.TOTP_PERIOD)
 
     # note: for https, you *must* use this domain
-    return URL % quote(otp_url)
+    # mark this as safe as otherwise django will urlquote our data for us
+    return mark_safe(URL % quote_plus(otp_url))

=== modified file 'src/identityprovider/tests/test_templatetags.py'
--- src/identityprovider/tests/test_templatetags.py	2014-11-28 19:08:01 +0000
+++ src/identityprovider/tests/test_templatetags.py	2018-03-16 13:31:48 +0000
@@ -1,12 +1,12 @@
 # Copyright 2010 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
+from django.template import Context, Template
 from django.test import TestCase
 
 from identityprovider.templatetags.google_analytics import expand_ga_commands
 from identityprovider.templatetags.static_url import static_url
 
-
 SNIPPETS = [
     None,
     '',
@@ -56,3 +56,32 @@
         url = '/+relative'
         with self.settings(RELATIVE_URL=url):
             self.assertEqual(static_url('relative'), url)
+
+
+class QRCodeURLTestCase(TestCase):
+
+    def test_not_encoded(self):
+        """The url string should be marked as safe as we can't escape the &"""
+        qr_data = {
+            'oath_mode': 'totp',
+            'ident': u'UbuntuSSO/[e-mail]',
+            'hex_key': 'A10EE5139362F3150BFABB7DDBEC5EE6FA2CA989',
+        }
+
+        # Render the template tag so we see the final result
+        # as calling the method won't result in the string being encoded
+        template = Template(
+            '{% load qrcode %}{% qrcode_url mode ident hex_key %}'
+        )
+        context = Context(qr_data)
+        result = template.render(context)
+
+        # Ideally, we should just decode the string here and check if it
+        # changes but the email address is correctly encoded,
+        # so that changes if so.
+        # instead, check the whole string we have generated
+        expected_result = (
+            u'https://chart.googleapis.com/chart?chs=250x250&'
+            u'chld=L%7C0&cht=qr&chl=otpauth%3A%2F%2F%2FUbuntuSSO%2Fzl%40'
+            u'beyfvift.org%3Fsecret%3DUEHOKE4TMLZRKC72XN65X3C6435CZKMJ')
+        self.assertEqual(result, expected_result)

=== modified file 'src/webui/tests/test_views_devices.py'
--- src/webui/tests/test_views_devices.py	2018-02-09 20:56:16 +0000
+++ src/webui/tests/test_views_devices.py	2018-03-16 13:31:48 +0000
@@ -5,7 +5,7 @@
 
 import re
 from base64 import b16encode, b32encode
-from urllib import quote as urlquote
+from urllib import quote_plus as urlquote
 
 import mock
 from django.conf import settings