Merge lp:~nataliabidart/ubuntu-sso-client/handle-user-not-validated-stable into lp:ubuntu-sso-client/stable-1-0

[Natalia Bidart]

* Added a new DBus signal UserNotValidated to indicate when a user is registered but not validated (LP: #667899).
* Added new workflow so email validation is requested if necessary.

To test, follow the same procedure as in

https://code.launchpad.net/~nataliabidart/ubuntu-sso-client/handle-user-not-validated/+merge/39779

but use instead the ApplicationCredentials interface in the /credentials object path. Run the "login_or_register_to_get_credentials" method and pass parameters like these:

'Ubuntu One', '', '', 0

The rest of the workflow is the same.

[Roman Yepishev (rye)]

Works as advertised. This branch is awesome too.

review: Approve (fieldtest)

[Alejandro J. Cura (alecu)]

I approve of this branch.

review: Approve

=== modified file 'ubuntu_sso/gui.py'
--- ubuntu_sso/gui.py	2010-10-01 13:14:34 +0000
+++ ubuntu_sso/gui.py	2010-11-01 21:13:47 +0000
@@ -252,6 +252,8 @@
         self.tc_uri = tc_uri
         self.help_text = help_text
         self.close_callback = close_callback
+        self.user_email = None
+        self.user_password = None
 
         ui_filename = get_data_file('ui.glade')
         builder = gtk.Builder()
@@ -357,6 +359,8 @@
              self._filter_by_app_name(self.on_logged_in),
             'LoginError':
              self._filter_by_app_name(self.on_login_error),
+            'UserNotValidated':
+             self._filter_by_app_name(self.on_user_not_validated),
             'PasswordResetTokenSent':
              self._filter_by_app_name(self.on_password_reset_token_sent),
             'PasswordResetError':
@@ -417,8 +421,8 @@
 
             match = self.bus.add_signal_receiver(method, signal_name=signal,
                                                  dbus_interface=iface)
-            logger.info('Connecting signal %r with method %r at iface %r.' \
-                        'Match: %r', signal, method, iface, match)
+            logger.debug('Connecting signal %r with method %r at iface %r.' \
+                         'Match: %r', signal, method, iface, match)
             self._signals_receivers[(iface, signal)] = method
 
     def _debug(self, *args, **kwargs):
@@ -725,8 +729,8 @@
         remove = self.bus.remove_signal_receiver
         for (iface, signal) in self._signals_receivers.keys():
             method = self._signals_receivers.pop((iface, signal))
-            logger.info('Removing signal %r with method %r at iface %r.',
-                        signal, method, iface)
+            logger.debug('Removing signal %r with method %r at iface %r.',
+                         signal, method, iface)
             remove(method, signal_name=signal, dbus_interface=iface)
 
         # hide the main window
@@ -804,6 +808,8 @@
             return
 
         self._set_current_page(self.processing_vbox)
+        self.user_email = email1
+        self.user_password = password1
 
         logger.info('Calling register_user with email %r, password <hidden>,' \
                     ' captcha_id %r and captcha_solution %r.', email1,
@@ -832,8 +838,8 @@
             self.email_token_entry.set_warning(self.FIELD_REQUIRED)
             return
 
-        email = self.email1_entry.get_text()
-        password = self.password1_entry.get_text()
+        email = self.user_email
+        password = self.user_password
         f = self.backend.validate_email
         logger.info('Calling validate_email with email %r, password <hidden>' \
                     ', app_name %r and email_token %r.', email, self.app_name,
@@ -871,6 +877,8 @@
           reply_handler=NO_OP, error_handler=NO_OP)
 
         self._set_current_page(self.processing_vbox)
+        self.user_email = email
+        self.user_password = password
 
     def on_login_back_button_clicked(self, *args, **kwargs):
         """User wants to go to the previous page."""
@@ -1061,6 +1069,11 @@
                                      msg))
 
     @log_call
+    def on_user_not_validated(self, app_name, email, *args, **kwargs):
+        """User was not validated."""
+        self.on_user_registered(app_name, email)
+
+    @log_call
     def on_password_reset_token_sent(self, app_name, email, *args, **kwargs):
         """Password reset token was successfully sent."""
         msg = self.SET_NEW_PASSWORD_LABEL % {'email': email}
 
=== modified file 'ubuntu_sso/main.py'
--- ubuntu_sso/main.py	2010-10-18 17:34:04 +0000
+++ ubuntu_sso/main.py	2010-11-01 21:13:47 +0000
@@ -209,6 +209,27 @@
                      'token_name: %r', credentials['consumer_key'], token_name)
         return credentials
 
+    def is_validated(self, email, password, token_name, sso_service=None):
+        """Return if user with 'email' and 'password' is validated."""
+        if sso_service is None:
+            token = self.login(email=email, password=password,
+                               token_name=token_name)
+
+            oauth_token = oauth.OAuthToken(token['token'],
+                                           token['token_secret'])
+            authorizer = OAuthAuthorizer(token['consumer_key'],
+                                         token['consumer_secret'],
+                                         oauth_token)
+            sso_service = self.sso_service_class(authorizer, self.service_url)
+
+        me_info = sso_service.accounts.me()
+        key = 'preferred_email'
+        result = key in me_info and me_info[key] != None
+
+        logger.debug('is_validated: email: %r token_name: %r, result: %r.',
+                     email, token_name, result)
+        return result
+
     def validate_email(self, email, password, email_token, token_name):
         """Validate an email token for user with 'email' and 'password'."""
         logger.debug('validate_email: email: %r password: <hidden>, '
@@ -311,12 +332,8 @@
         dbus.service.Object.__init__(self, object_path="/sso",
                                      bus_name=bus_name)
         self.sso_login_processor_class = sso_login_processor_class
-        self.sso_service_class = sso_service_class
-
-    def processor(self):
-        """Create a login processor with the given class and service class."""
-        return self.sso_login_processor_class(
-                                    sso_service_class=self.sso_service_class)
+        self.processor = self.sso_login_processor_class(
+                                    sso_service_class=sso_service_class)
 
     # generate_capcha signals
     @dbus.service.signal(DBUS_IFACE_USER_NAME, signature="ss")
@@ -337,7 +354,7 @@
         """Call the matching method in the processor."""
         def f():
             """Inner function that will be run in a thread."""
-            return self.processor().generate_captcha(filename)
+            return self.processor.generate_captcha(filename)
         blocking(f, app_name, self.CaptchaGenerated,
                  self.CaptchaGenerationError)
 
@@ -361,8 +378,8 @@
         """Call the matching method in the processor."""
         def f():
             """Inner function that will be run in a thread."""
-            return self.processor().register_user(email, password,
-                                                  captcha_id, captcha_solution)
+            return self.processor.register_user(email, password,
+                                                captcha_id, captcha_solution)
         blocking(f, app_name, self.UserRegistered, self.UserRegistrationError)
 
     # login signals
@@ -378,6 +395,12 @@
         logger.debug('SSOLogin: emitting LoginError with '
                      'app_name "%s" and error %r', app_name, error)
 
+    @dbus.service.signal(DBUS_IFACE_USER_NAME, signature="ss")
+    def UserNotValidated(self, app_name, result):
+        """Signal thrown when the user is not validated."""
+        logger.debug('SSOLogin: emitting UserNotValidated with app_name "%s" '
+                     'and result %r', app_name, result)
+
     @dbus.service.method(dbus_interface=DBUS_IFACE_USER_NAME,
                          in_signature='sss')
     def login(self, app_name, email, password):
@@ -387,13 +410,23 @@
             token_name = get_token_name(app_name)
             logger.debug('login: token_name %r, email %r, password <hidden>.',
                          token_name, email)
-            credentials = self.processor().login(email, password, token_name)
+            credentials = self.processor.login(email, password, token_name)
             logger.debug('login returned not None credentials? %r.',
                          credentials is not None)
-            assert credentials is not None
-            keyring_store_credentials(app_name, credentials)
-            return email
-        blocking(f, app_name, self.LoggedIn, self.LoginError)
+            return credentials
+
+        def success_cb(app_name, credentials):
+            """Login finished successfull."""
+            token_name = get_token_name(app_name)
+            is_validated = self.processor.is_validated(email, password,
+                                                       token_name)
+            logger.debug('user is validated? %r.', is_validated)
+            if is_validated:
+                keyring_store_credentials(app_name, credentials)
+                self.LoggedIn(app_name, email)
+            else:
+                self.UserNotValidated(app_name, email)
+        blocking(f, app_name, success_cb, self.LoginError)
 
     # validate_email signals
     @dbus.service.signal(DBUS_IFACE_USER_NAME, signature="ss")
@@ -415,8 +448,8 @@
         def f():
             """Inner function that will be run in a thread."""
             token_name = get_token_name(app_name)
-            credentials = self.processor().validate_email(email, password,
-                                                   email_token, token_name)
+            credentials = self.processor.validate_email(email, password,
+                                                      email_token, token_name)
             keyring_store_credentials(app_name, credentials)
             return email
         blocking(f, app_name, self.EmailValidated, self.EmailValidationError)
@@ -440,7 +473,7 @@
         """Call the matching method in the processor."""
         def f():
             """Inner function that will be run in a thread."""
-            return self.processor().request_password_reset_token(email)
+            return self.processor.request_password_reset_token(email)
         blocking(f, app_name, self.PasswordResetTokenSent,
                  self.PasswordResetError)
 
@@ -463,8 +496,8 @@
         """Call the matching method in the processor."""
         def f():
             """Inner function that will be run in a thread."""
-            return self.processor().set_new_password(email, token,
-                                                     new_password)
+            return self.processor.set_new_password(email, token,
+                                                   new_password)
         blocking(f, app_name, self.PasswordChanged, self.PasswordChangeError)
 
 
@@ -481,19 +514,19 @@
     @dbus.service.signal(DBUS_IFACE_CRED_NAME, signature="s")
     def AuthorizationDenied(self, app_name):
         """Signal thrown when the user denies the authorization."""
-        logger.info('SSOLogin: emitting AuthorizationDenied with app_name '
-                    '"%s"', app_name)
+        logger.info('SSOCredentials: emitting AuthorizationDenied with '
+                    'app_name "%s"', app_name)
 
     @dbus.service.signal(DBUS_IFACE_CRED_NAME, signature="sa{ss}")
     def CredentialsFound(self, app_name, credentials):
         """Signal thrown when the credentials are found."""
-        logger.info('SSOLogin: emitting CredentialsFound with app_name "%s"',
-                    app_name)
+        logger.info('SSOCredentials: emitting CredentialsFound with '
+                    'app_name "%s"', app_name)
 
     @dbus.service.signal(DBUS_IFACE_CRED_NAME, signature="sss")
     def CredentialsError(self, app_name, error_message, detailed_error):
         """Signal thrown when there is a problem finding the credentials."""
-        logger.debug('SSOCredentials: emitting CredentialsError with app_name '
+        logger.error('SSOCredentials: emitting CredentialsError with app_name '
                      '"%s" and error_message %r', app_name, error_message)
 
     @dbus.service.method(dbus_interface=DBUS_IFACE_CRED_NAME,
 
=== modified file 'ubuntu_sso/tests/test_gui.py'
--- ubuntu_sso/tests/test_gui.py	2010-09-07 13:57:35 +0000
+++ ubuntu_sso/tests/test_gui.py	2010-11-01 21:13:47 +0000
@@ -856,6 +856,12 @@
         self.ui.join_ok_button.clicked()
         self.assertTrue(self._called)
 
+    def test_user_and_pass_are_cached(self):
+        """Username and password are temporarly cached for further use."""
+        self.click_join_with_valid_data()
+        self.assertEqual(self.ui.user_email, EMAIL)
+        self.assertEqual(self.ui.user_password, PASSWORD)
+
 
 class NoTermsAndConditionsTestCase(UbuntuSSOClientTestCase):
     """Test suite for the user registration (with no t&c link)."""
@@ -1328,6 +1334,12 @@
         self.ui.on_login_error(app_name=APP_NAME, error=self.error)
         self.assert_pages_visibility(login=True)
 
+    def test_on_user_not_validated_morphs_to_verify_page(self):
+        """On user not validated, the verify page is shown."""
+        self.click_connect_with_valid_data()
+        self.ui.on_user_not_validated(app_name=APP_NAME, email=EMAIL)
+        self.assert_pages_visibility(verify_email=True)
+
     def test_on_login_error_a_warning_is_shown(self):
         """On user login error, a warning is shown with proper wording."""
         self.click_connect_with_valid_data()
@@ -1377,6 +1389,12 @@
         self.ui.login_ok_button.clicked()
         self.assertTrue(self._called)
 
+    def test_user_and_pass_are_cached(self):
+        """Username and password are temporarly cached for further use."""
+        self.click_connect_with_valid_data()
+        self.assertEqual(self.ui.user_email, EMAIL)
+        self.assertEqual(self.ui.user_password, PASSWORD)
+
 
 class LoginValidationTestCase(UbuntuSSOClientTestCase):
     """Test suite for the user login validation."""
@@ -1782,7 +1800,7 @@
         """All the backend signals are listed to be binded."""
         for sig in ('CaptchaGenerated', 'CaptchaGenerationError',
                     'UserRegistered', 'UserRegistrationError',
-                    'LoggedIn', 'LoginError',
+                    'LoggedIn', 'LoginError', 'UserNotValidated',
                     'EmailValidated', 'EmailValidationError',
                     'PasswordResetTokenSent', 'PasswordResetError',
                     'PasswordChanged', 'PasswordChangeError'):
@@ -1864,6 +1882,13 @@
         self.ui._signals['LoginError'](mismatch_app_name, 'dummy')
         self.assertFalse(self._called)
 
+    def test_on_user_not_validated_is_not_called(self):
+        """on_user_not_validated is not called if incorrect app_name."""
+        self.patch(self.ui, 'on_user_not_validated', self._set_called)
+        mismatch_app_name = self.ui.app_name * 2
+        self.ui._signals['UserNotValidated'](mismatch_app_name, 'dummy')
+        self.assertFalse(self._called)
+
     def test_on_password_reset_token_sent_is_not_called(self):
         """on_password_reset_token_sent is not called if incorrect app_name."""
         self.patch(self.ui, 'on_password_reset_token_sent', self._set_called)
 
=== modified file 'ubuntu_sso/tests/test_main.py'
--- ubuntu_sso/tests/test_main.py	2010-10-18 17:34:04 +0000
+++ ubuntu_sso/tests/test_main.py	2010-11-01 21:13:47 +0000
@@ -152,6 +152,9 @@
 class FakedAccounts(object):
     """Fake the accounts service."""
 
+    def __init__(self):
+        self.preferred_email = EMAIL
+
     def validate_email(self, email_token):
         """Fake the email validation. Return a fix result."""
         if email_token is None:
@@ -164,6 +167,17 @@
         else:
             return STATUS_EMAIL_OK
 
+    # pylint: disable=E0202, C0103
+
+    def me(self):
+        """Fake the 'me' information."""
+        return {u'username': u'Wh46bKY',
+                u'preferred_email': self.preferred_email,
+                u'displayname': u'',
+                u'unverified_emails': [u'[e-mail]'],
+                u'verified_emails': [],
+                u'openid_identifier': u'Wh46bKY'}
+
 
 class FakedSSOServer(object):
     """Fake an SSO server."""
@@ -279,6 +293,29 @@
         result = self.processor.login(**self.login_kwargs)
         self.assertEqual(TOKEN, result, 'authentication was successful.')
 
+    # is_validated
+
+    def test_is_validated(self):
+        """If preferred email is not None, user is validated."""
+        result = self.processor.is_validated(**self.login_kwargs)
+        self.assertTrue(result, 'user must be validated.')
+
+    def test_is_not_validated(self):
+        """If preferred email is None, user is not validated."""
+        service = FakedSSOServer(None, None)
+        service.accounts.preferred_email = None
+        result = self.processor.is_validated(sso_service=service,
+                                             **self.login_kwargs)
+        self.assertFalse(result, 'user must not be validated.')
+
+    def test_is_not_validated_empty_result(self):
+        """If preferred email is None, user is not validated."""
+        service = FakedSSOServer(None, None)
+        service.accounts.me = lambda: {}
+        result = self.processor.is_validated(sso_service=service,
+                                             **self.login_kwargs)
+        self.assertFalse(result, 'user must not be validated.')
+
     # validate_email
 
     def test_validate_email_if_status_ok(self):
@@ -519,8 +556,11 @@
     def test_login(self):
         """Test that the login method works ok."""
         d = Deferred()
-        self.create_mock_processor().login(EMAIL, PASSWORD, TOKEN_NAME)
+        processor = self.create_mock_processor()
+        processor.login(EMAIL, PASSWORD, TOKEN_NAME)
         self.mocker.result(TOKEN)
+        processor.is_validated(EMAIL, PASSWORD, TOKEN_NAME)
+        self.mocker.result(True)
         self.patch(ubuntu_sso.main, "blocking", self.fake_ok_blocking)
         self.mocker.replay()
 
@@ -535,13 +575,40 @@
                           sso_login_processor_class=self.mockprocessorclass)
         self.patch(client, "LoggedIn", verify)
         self.patch(client, "LoginError", d.errback)
+        self.patch(client, "UserNotValidated", d.errback)
+        client.login(APP_NAME, EMAIL, PASSWORD)
+        return d
+
+    def test_login_user_not_validated(self):
+        """Test that the login sends EmailNotValidated signal."""
+        d = Deferred()
+        processor = self.create_mock_processor()
+        processor.login(EMAIL, PASSWORD, TOKEN_NAME)
+        self.mocker.result(TOKEN)
+        processor.is_validated(EMAIL, PASSWORD, TOKEN_NAME)
+        self.mocker.result(False)
+        self.patch(ubuntu_sso.main, "blocking", self.fake_ok_blocking)
+        self.mocker.replay()
+
+        def verify(app_name, email):
+            """The actual test."""
+            self.assertEqual(app_name, APP_NAME)
+            self.assertEqual(email, EMAIL)
+            self.assertFalse(self.keyring_was_set, "Keyring should not be set")
+            d.callback("Ok")
+
+        client = SSOLogin(self.mockbusname,
+                          sso_login_processor_class=self.mockprocessorclass)
+        self.patch(client, "LoggedIn", d.errback)
+        self.patch(client, "LoginError", d.errback)
+        self.patch(client, "UserNotValidated", verify)
         client.login(APP_NAME, EMAIL, PASSWORD)
         return d
 
     def test_login_error(self):
         """Test that the login method fails as expected."""
         d = Deferred()
-        self.mockprocessorclass = self.mocker.mock()
+        self.create_mock_processor()
         self.patch(ubuntu_sso.main, "blocking", self.fake_err_blocking)
 
         def fake_gtn(*args):
@@ -562,6 +629,7 @@
                           sso_login_processor_class=self.mockprocessorclass)
         self.patch(client, "LoggedIn", d.errback)
         self.patch(client, "LoginError", verify)
+        self.patch(client, "UserNotValidated", d.errback)
         client.login(APP_NAME, EMAIL, PASSWORD)
         return d
 
@@ -591,7 +659,7 @@
     def test_validate_email_error(self):
         """Test that the validate_email method fails as expected."""
         d = Deferred()
-        self.mockprocessorclass = self.mocker.mock()
+        self.create_mock_processor()
         self.patch(ubuntu_sso.main, "blocking", self.fake_err_blocking)
 
         def fake_gtn(*args):