Merge lp:~jaboing/canonical-identity-provider/sso into lp:canonical-identity-provider/release

[Julien Funk]

Removed TWOFACTOR flags and updated scripts to work for Dev/Production/Staging - added function 'delete_device' to devices.py and function 'login_to_isdqa_account' to helpers.py. All are tested pass and should now run as part of the default acceptance suite.

Removed TWOFACTOR flags and updated scripts to work for Dev/Production/Staging - added function 'delete_device' to devices.py and function 'login_to_isdqa_account' to helpers.py. All are tested pass and should now run as part of the default acceptance suite.

[Ricardo Kirkner (ricardokirkner)]

A few comments and questions

- why not refactor the if statement into the register_account helper? (the register_account helper would then be 'smart' enough to created the account if needed or log in with the sanctified account if not); this also reduces the changes required

- l.141-142: the double call to delete_device is intentional?

- removing the TWOFACTOR flag will make this run everywhere (dev, staging, prod), so are we sure these tests pass against staging/prod?

- in login_to_isdqa_account you create the account if it's missing; but I don't see how this new account gets the necessary twofactor bits enabled (unless register_account does this by default). Why can't we always call login_to_isdqa_account, both on staging/prod and dev, and get rid of the if statement alltogether?

[Julien Funk (jaboing)]

- I added a new helper login_or_register_account() which is smart enough to create the account or log in with the sanctified account

- the double call is indeed intentional, I clarified this in the comment

- I ahventested all the tests against production/staging/dev already and they all pass happily

- I think you misunderstood the login_to_isdqa_account function, it does not create a new account at all, it simply logs into the existing isdqa account. The reason we cannot call this on dev is because the isdqa account does not exist by default on the developer machines and instead a new account must be created in the local database. On staging and production however the isdqa account exists already.

Will merge shortly.

[Ricardo Kirkner (ricardokirkner)]

This looks much better, thanks. However a few more small issues:

l. 205-206: this seems oddly indented. is this correct? (looks like this was supposed to be indented one extra level maybe? (inside the delete_device function?)

l. 247: login_to_isdqa_account is calling register_account when on staging when the password doesn't match. Why do we register an account on staging but not on production? Shouldn't both environments be consistent?

review: Needs Fixing

[Ricardo Kirkner (ricardokirkner)]

LGTM. Thanks!

review: Approve

=== modified file 'identityprovider/tests/acceptance/devices/add_device.py'
--- identityprovider/tests/acceptance/devices/add_device.py	2012-03-02 10:22:00 +0000
+++ identityprovider/tests/acceptance/devices/add_device.py	2012-04-09 20:00:32 +0000
@@ -10,11 +10,8 @@
 
 
 config.set_base_url_from_env()
-# TODO: remove once staging / production have two-factor authentication
-check_flags('TWOFACTOR')
 
-# Create account and login
-helpers.register_account()
+helpers.login_or_register_account()
 
 # Go to the authentication devices page
 click_link('devices-link')
 
=== modified file 'identityprovider/tests/acceptance/devices/add_device_google.py'
--- identityprovider/tests/acceptance/devices/add_device_google.py	2012-02-22 20:32:44 +0000
+++ identityprovider/tests/acceptance/devices/add_device_google.py	2012-04-09 20:00:32 +0000
@@ -4,19 +4,19 @@
 from canonical.isd.tests.sst import config
 from oath import hotp
 from sst.actions import *
+from django.conf import settings
 
 import helpers
 from devices import (
-    click_add_device_button, click_add_new_device_link
+    click_add_device_button, 
+    click_add_new_device_link,
+    delete_device,
 )
 
 
 config.set_base_url_from_env()
-# TODO: remove once staging / production have two-factor authentication
-check_flags('TWOFACTOR')
 
-# Create account and login
-email = helpers.register_account()
+email = helpers.login_or_register_account()
 
 # Go to the authentication devices page
 click_link('devices-link')
@@ -60,3 +60,6 @@
 assert_table_has_rows('device-list', 1)
 assert_table_row_contains_text('device-list', 0,
     ['My Google Device', 'Delete'], regex=True)
+
+# clean up afterward, delete device
+delete_device()
 
=== modified file 'identityprovider/tests/acceptance/devices/add_device_yubikey.py'
--- identityprovider/tests/acceptance/devices/add_device_yubikey.py	2012-02-22 20:32:44 +0000
+++ identityprovider/tests/acceptance/devices/add_device_yubikey.py	2012-04-09 20:00:32 +0000
@@ -3,18 +3,18 @@
 from oath import hotp
 from sst.actions import *
 
+from django.conf import settings
 import helpers
 from devices import (
-    click_add_device_button, click_add_new_device_link
+    click_add_device_button, 
+    click_add_new_device_link,
+    delete_device,
 )
 
 
 config.set_base_url_from_env()
-# TODO: remove once staging / production have two-factor authentication
-check_flags('TWOFACTOR')
 
-# Create account and login
-email = helpers.register_account()
+email = helpers.login_or_register_account()
 
 # Go to the authentication devices page
 click_link('devices-link')
@@ -43,3 +43,5 @@
 assert_table_row_contains_text('device-list', 0,
     ['My YubiKey', 'Delete'], regex=True)
 
+# clean up afterward, delete device
+delete_device()
 
=== modified file 'identityprovider/tests/acceptance/devices/add_two_devices.py'
--- identityprovider/tests/acceptance/devices/add_two_devices.py	2012-03-02 10:22:00 +0000
+++ identityprovider/tests/acceptance/devices/add_two_devices.py	2012-04-09 20:00:32 +0000
@@ -5,16 +5,16 @@
 
 import helpers
 from devices import (
-    add_device, click_add_device_button, click_add_new_device_link
+    add_device, 
+    click_add_device_button, 
+    click_add_new_device_link,
+    delete_device,
 )
 
 
 config.set_base_url_from_env()
-# TODO: remove once staging / production have two-factor authentication
-check_flags('TWOFACTOR')
 
-# Create account and login
-helpers.register_account()
+helpers.login_or_register_account()
 
 # Add an authentication device
 aes_key = add_device('This device')
@@ -50,3 +50,8 @@
 assert_table_has_rows('device-list', 2)
 assert_table_row_contains_text('device-list', 1,
     ['This device (1)', 'Rename Delete'])
+
+# clean up afterward, delete 2 devices
+delete_device()
+delete_device()
+
 
=== modified file 'identityprovider/tests/acceptance/devices/delete_device.py'
--- identityprovider/tests/acceptance/devices/delete_device.py	2012-02-22 20:32:44 +0000
+++ identityprovider/tests/acceptance/devices/delete_device.py	2012-04-09 20:00:32 +0000
@@ -9,12 +9,8 @@
 
 
 config.set_base_url_from_env()
-# TODO: remove once staging / production have two-factor authentication
-check_flags('TWOFACTOR')
-
-
-# Create account and login
-helpers.register_account()
+
+helpers.login_or_register_account()
 
 # Add an authentication device
 aes_key = add_device('This device')
 
=== modified file 'identityprovider/tests/acceptance/devices/rename_device.py'
--- identityprovider/tests/acceptance/devices/rename_device.py	2012-02-13 16:12:21 +0000
+++ identityprovider/tests/acceptance/devices/rename_device.py	2012-04-09 20:00:32 +0000
@@ -7,19 +7,17 @@
     assert_device,
     assert_no_device,
     rename_device,
+    delete_device,
 )
 
 config.set_base_url_from_env()
-# TODO: remove once staging / production have two-factor authentication
-check_flags('TWOFACTOR')
-
 
 # happy path
 
-# create an account and log in
-helpers.register_account(password='Password1')
-# add a new device
-name = 'device'
+helpers.login_or_register_account()
+
+# add a new device with unicode character and white space
+name = 'device name'
 add_device(name)
 # make sure device exists
 assert_device(name)
@@ -56,3 +54,7 @@
     'The name must contain at least one non-whitespace character.')
 # cancel and go back
 click_link(get_element(tag='a', text='cancel'))
+
+# clean up afterward, delete device
+delete_device()
+
 
=== modified file 'identityprovider/tests/acceptance/shared/devices.py'
--- identityprovider/tests/acceptance/shared/devices.py	2012-02-22 20:32:44 +0000
+++ identityprovider/tests/acceptance/shared/devices.py	2012-04-09 20:00:32 +0000
@@ -15,8 +15,9 @@
 
 
 def click_delete_button():
+    # Clicks the first Delete button it finds
     # this button is actually a link
-    click_link(get_element(tag='a', text_regex='Delete'))
+    click_link(get_elements(tag='a', text_regex='Delete')[0])
 
 
 def assert_device(name):
@@ -47,6 +48,15 @@
     click_add_device_button()
     return aes_key
 
+def delete_device():
+    # Click on the delete button
+    click_delete_button()
+    # Click on ok
+    click_button(get_element(tag='button', text='Delete this device'))
+    # Check we are back on the device-list page
+    assert_url('/device-list')
+    # Check that our device has been deleted
+    fails(get_element, 'device-list')
 
 def rename_device(name, new_name):
     click_link(get_element(tag='a', text_regex='Rename'))
 
=== modified file 'identityprovider/tests/acceptance/shared/helpers.py'
--- identityprovider/tests/acceptance/shared/helpers.py	2012-03-30 16:01:27 +0000
+++ identityprovider/tests/acceptance/shared/helpers.py	2012-04-09 20:00:32 +0000
@@ -105,6 +105,15 @@
         assert_title('Complete email address validation')
         click_button(get_element(name='continue'))
 
+def login_or_register_account():
+    # if developer instance, create an account and log in
+    if get_base_url() == 'http://localhost:8000/':
+        email = register_account()
+    # else use the sanctified QA Account
+    else:
+        login_to_isdqa_account()
+        email = settings.QA_ACCOUNT_EMAIL
+    return email
 
 def login(email, password):
     go_to('/')
@@ -198,6 +207,21 @@
         register_account(settings.TEST_ACCOUNT_EMAIL, "Test Account",
             settings.TEST_ACCOUNT_PASSWORD)
 
+def login_to_isdqa_account():
+    from sst import actions
+
+    global _LAST_EMAIL, _LAST_PASSWORD
+
+    login(settings.QA_ACCOUNT_EMAIL, settings.QA_ACCOUNT_PASSWORD)
+    # Wait for the reload
+    wait_for(exists_element, id='ubuntu-header')
+
+    if exists_element(tag='span', css_class='error', text="Password didn't match."):
+        actions._raise("This test requires a test account to be present")
+ 
+    _LAST_EMAIL = settings.QA_ACCOUNT_EMAIL
+    _LAST_PASSWORD = settings.QA_ACCOUNT_PASSWORD
+
 
 def login_from_redirect(email=settings.QA_ACCOUNT_EMAIL,
         password=settings.QA_ACCOUNT_PASSWORD):