Merge lp:~canonical-isd-hackers/canonical-identity-provider/930377-add-device into lp:canonical-identity-provider/release

[Michael Foord]

Adding a two factor device authenticates the user with that device.

[David Owen (dsowen)]

Test changes look good. It's not every day you see a decent use for globals, but I think this is one.

review: Approve

=== modified file 'identityprovider/models/twofactor.py'
--- identityprovider/models/twofactor.py	2012-02-07 14:39:20 +0000
+++ identityprovider/models/twofactor.py	2012-02-14 18:17:19 +0000
@@ -79,5 +79,3 @@
             self.counter = new_counter
             self.save()
         return valid
-
-

=== modified file 'identityprovider/tests/acceptance/devices/add_device.py'
--- identityprovider/tests/acceptance/devices/add_device.py	2012-02-13 16:12:21 +0000
+++ identityprovider/tests/acceptance/devices/add_device.py	2012-02-14 18:17:19 +0000
@@ -4,7 +4,9 @@
 from sst.actions import *
 
 import helpers
-from devices import click_add_device_button, click_add_new_device_link
+from devices import (
+    click_add_device_button, click_add_new_device_link, click_remove_button
+)
 
 
 config.set_base_url_from_env()
@@ -100,3 +102,15 @@
 assert_table_has_rows('device-list', 1)
 assert_table_row_contains_text('device-list', 0,
     ['My Generic Device', 'Remove'], regex=True)
+
+# Click on the remove button
+click_remove_button()
+
+# Click on ok - this will only work if adding a device also authenticates us.
+# If not we will need to authenticate and this next step will fail.
+click_button(get_element(tag='button', text='Remove this device'))
+# Check we are back on the device-list page
+assert_url('/device-list')
+
+# Check that our device has been removed
+assert_table_has_rows('device-list', 0)
\ No newline at end of file

=== modified file 'identityprovider/tests/acceptance/devices/add_two_devices.py'
--- identityprovider/tests/acceptance/devices/add_two_devices.py	2012-02-13 17:07:37 +0000
+++ identityprovider/tests/acceptance/devices/add_two_devices.py	2012-02-14 18:17:19 +0000
@@ -19,6 +19,9 @@
 # Add an authentication device
 aes_key = add_device('This device')
 
+# Logout and back in so we are no longer 2f-authenticated
+helpers.logout_and_in()
+
 # Click on "Add a new authentication device" link
 click_add_new_device_link()
 

=== modified file 'identityprovider/tests/acceptance/devices/prefs.py'
--- identityprovider/tests/acceptance/devices/prefs.py	2012-02-13 16:12:21 +0000
+++ identityprovider/tests/acceptance/devices/prefs.py	2012-02-14 18:17:19 +0000
@@ -11,7 +11,6 @@
 # TODO: remove once staging / production have two-factor authentication
 check_flags('TWOFACTOR')
 
-
 # Create account and login
 helpers.register_account()
 
@@ -42,6 +41,9 @@
 assert_attribute(get_element(tag="input", name="two-factor", value="always"),
                  'checked', None)
 
+# Logout and back in so we are no longer 2f-authenticated
+helpers.logout_and_in()
+
 # Change the preference to always require 2 factor authentication and save
 set_radio_value(get_element(tag="input", name="two-factor", value="always"))
 click_button("authentication-prefs")

=== modified file 'identityprovider/tests/acceptance/devices/remove_device.py'
--- identityprovider/tests/acceptance/devices/remove_device.py	2012-02-13 16:12:21 +0000
+++ identityprovider/tests/acceptance/devices/remove_device.py	2012-02-14 18:17:19 +0000
@@ -5,12 +5,8 @@
 from oath import hotp
 
 import helpers
-from devices import add_device
-
-
-def click_remove_button():
-    # this button is actually a link
-    click_link(get_element(tag='a', text_regex='Remove'))
+from devices import add_device, click_remove_button
+
 
 config.set_base_url_from_env()
 # TODO: remove once staging / production have two-factor authentication
@@ -23,6 +19,9 @@
 # Add an authentication device
 aes_key = add_device('This device')
 
+# Logout and back in so we are no longer 2f-authenticated
+helpers.logout_and_in()
+
 # Click on the remove button
 click_remove_button()
 

=== modified file 'identityprovider/tests/acceptance/shared/devices.py'
--- identityprovider/tests/acceptance/shared/devices.py	2012-02-13 15:58:27 +0000
+++ identityprovider/tests/acceptance/shared/devices.py	2012-02-14 18:17:19 +0000
@@ -11,6 +11,11 @@
         get_element(tag='a', text_regex='^Add a new authentication device'))
 
 
+def click_remove_button():
+    # this button is actually a link
+    click_link(get_element(tag='a', text_regex='Remove'))
+
+
 def assert_device(name):
     assert_element(tag="td", text=name)
 

=== modified file 'identityprovider/tests/acceptance/shared/helpers.py'
--- identityprovider/tests/acceptance/shared/helpers.py	2012-02-13 15:58:27 +0000
+++ identityprovider/tests/acceptance/shared/helpers.py	2012-02-14 18:17:19 +0000
@@ -8,6 +8,9 @@
 from sst.actions import *
 
 
+_LAST_EMAIL = None
+_LAST_PASSWORD = None
+
 def register_account(email_address=None, displayname="My Name",
                      password="Admin007", fetch_email=True, verify=True):
     """Register an account and verify the token by default.
@@ -16,6 +19,8 @@
 
     If you pass in an `email_address` then the verification token is returned.
     """
+    global _LAST_EMAIL, _LAST_PASSWORD
+
     return_email = False
     if email_address is None:
         return_email = True
@@ -38,6 +43,10 @@
         if verify:
             write_textfield(get_element(name='confirmation_code'), vcode)
             click_button(get_element(css_class='btn'))
+
+    _LAST_EMAIL = email_address
+    _LAST_PASSWORD = password
+
     if return_email:
         return email_address
     return vcode
@@ -101,6 +110,15 @@
     click_button(get_element(name='continue'))
 
 
+def logout_and_in():
+    """Log out and then back in again, using the email and password from the
+    last registered account. Returns to the initial url after login."""
+    url = get_current_url()
+    logout()
+    login(_LAST_EMAIL, _LAST_PASSWORD)
+    go_to(url)
+
+
 def logout():
     go_to('/+logout')
 

=== modified file 'identityprovider/tests/unit/test_devices.py'
--- identityprovider/tests/unit/test_devices.py	2012-02-14 13:45:37 +0000
+++ identityprovider/tests/unit/test_devices.py	2012-02-14 18:17:19 +0000
@@ -114,6 +114,7 @@
         mock_request = self._get_mock_request()
         mock_request.method = 'POST'
         mock_request.POST.update(post_data)
+        mock_request.session = {twofactor.TWOFACTOR_KEY: True}
         return mock_request
 
     def _post_device_addition(self, mock_request):
@@ -146,6 +147,8 @@
             otp='755224',
             hex_key=self.KEY
         )
+        mock_request.session = {}
+        mock_request.user.has_twofactor_devices.return_value = False
 
         response, MockDevice = self._post_device_addition(mock_request)
 
@@ -158,6 +161,7 @@
             account=mock_request.user,
             counter=1
         )
+        self.assertEqual(mock_request.session[twofactor.TWOFACTOR_KEY], True)
 
     def test_unrecognised_type(self):
         mock_request = self._get_post_request(

=== modified file 'identityprovider/views/devices.py'
--- identityprovider/views/devices.py	2012-02-14 13:31:35 +0000
+++ identityprovider/views/devices.py	2012-02-14 18:17:19 +0000
@@ -163,6 +163,7 @@
                     key=hex_key,
                     counter=new_counter
                 )
+                twofactor.login(request)
                 return HttpResponseSeeOther(reverse(DEVICE_LIST))
             # Otherwise, set the error flag and fall through...
             error = _("The one-time password didn't match the AES key.")