Merge lp:~nataliabidart/ubuntu/maverick/ubuntu-sso-client/ubuntu-sso-client-1.0.7 into lp:ubuntu/maverick-proposed/ubuntu-sso-client

[Natalia Bidart]

* New upstream release (1.0.6, 1.0.7):

[ Natalia B. Bidart <email address hidden> ]
  * 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.
  * The verify email page should be always built, not only on registration.
[ Alejandro J. Cura <email address hidden> ]
  * Store credentials on the keyring *only* from the main thread
  (LP: #656545).

* New upstream release (1.0.5):

[ Natalia B. Bidart <email address hidden> ]
  * Credentials are removed if the pinging to the server fails or any
  other exception occurs (LP: #660516).

=== modified file 'PKG-INFO'
--- PKG-INFO	2010-10-12 10:07:55 +0000
+++ PKG-INFO	2010-11-04 12:34:40 +0000
@@ -1,6 +1,6 @@
 Metadata-Version: 1.1
 Name: ubuntu-sso-client
-Version: 1.0.4
+Version: 1.0.7
 Summary: Ubuntu Single Sign-On client
 Home-page: https://launchpad.net/ubuntu-sso-client
 Author: Natalia Bidart

=== modified file 'debian/changelog'
--- debian/changelog	2010-10-12 10:07:55 +0000
+++ debian/changelog	2010-11-04 12:34:40 +0000
@@ -1,3 +1,26 @@
+ubuntu-sso-client (1.0.7-0ubuntu1) UNRELEASED; urgency=low
+
+  * New upstream release (1.0.6, 1.0.7):
+
+    [ Natalia B. Bidart <[e-mail]> ]
+      * 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.
+      * The verify email page should be always built, not only on registration.
+
+    [ Alejandro J. Cura <[e-mail]> ]
+      * Store credentials on the keyring *only* from the main thread (LP:
+      #656545).
+
+  * New upstream release (1.0.5):
+
+    [ Natalia B. Bidart <[e-mail]> ]
+
+      * Credentials are removed if the pinging to the server fails or any
+      other exception occurs (LP: #660516).
+
+ -- Natalia Bidart (nessita) <[e-mail]>  Thu, 04 Nov 2010 09:21:00 -0300
+
 ubuntu-sso-client (1.0.4-0ubuntu1) maverick-proposed; urgency=low
 
   * New upstream release:

=== modified file 'setup.py'
--- setup.py	2010-10-12 10:07:55 +0000
+++ setup.py	2010-11-04 12:34:40 +0000
@@ -86,7 +86,7 @@
 
 DistUtilsExtra.auto.setup(
     name='ubuntu-sso-client',
-    version='1.0.4',
+    version='1.0.7',
     license='GPL v3',
     author='Natalia Bidart',
     author_email='[e-mail]',

=== modified file 'ubuntu_sso/gui.py'
--- ubuntu_sso/gui.py	2010-10-01 14:57:22 +0000
+++ ubuntu_sso/gui.py	2010-11-04 12:34:40 +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()
@@ -319,13 +321,13 @@
         self._append_page(self._build_login_page())
         self._append_page(self._build_request_password_token_page())
         self._append_page(self._build_set_new_password_page())
+        self._append_page(self._build_verify_email_page())
 
         window_size = None
         if not login_only:
             window_size = (550, 500)
             self._append_page(self._build_enter_details_page())
             self._append_page(self._build_tc_page())
-            self._append_page(self._build_verify_email_page())
             self.login_button.grab_focus()
             self._set_current_page(self.enter_details_vbox)
         else:
@@ -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-09-14 19:28:09 +0000
+++ ubuntu_sso/main.py	2010-11-04 12:34:40 +0000
@@ -55,6 +55,7 @@
 logger = setup_logging("ubuntu_sso.main")
 PING_URL = "https://one.ubuntu.com/oauth/sso-finished-so-get-tokens/"
 SERVICE_URL = "https://login.ubuntu.com/api/1.0"
+NO_OP = lambda *args, **kwargs: None
 
 
 class NoDefaultConfigError(Exception):
@@ -96,10 +97,16 @@
     """The new password could not be set."""
 
 
-def keyring_store_credentials(app_name, credentials):
+def keyring_store_credentials(app_name, credentials, callback, *cb_args):
     """Store the credentials in the keyring."""
-    logger.info('keyring_store_credentials: app_name "%s".', app_name)
-    Keyring(app_name).set_ubuntusso_attr(credentials)
+
+    def _inner():
+        """Store the credentials, and trigger the callback."""
+        logger.info('keyring_store_credentials: app_name "%s".', app_name)
+        Keyring(app_name).set_ubuntusso_attr(credentials)
+        callback(*cb_args)
+
+    gobject.idle_add(_inner)
 
 
 def keyring_get_credentials(app_name):
@@ -209,6 +216,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 +339,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 +361,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 +385,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 +402,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 +417,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,11 +455,16 @@
         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)
-            keyring_store_credentials(app_name, credentials)
-            return email
-        blocking(f, app_name, self.EmailValidated, self.EmailValidationError)
+            credentials = self.processor.validate_email(email, password,
+                                                      email_token, token_name)
+
+            def _email_stored():
+                """The email was stored, so call the signal."""
+                self.EmailValidated(app_name, email)
+
+            keyring_store_credentials(app_name, credentials, _email_stored)
+
+        blocking(f, app_name, NO_OP, self.EmailValidationError)
 
     # request_password_reset_token signals
     @dbus.service.signal(DBUS_IFACE_USER_NAME, signature="ss")
@@ -440,7 +485,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 +508,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 +526,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,
@@ -510,7 +555,8 @@
 
     def _login_success_cb(self, dialog, app_name, email):
         """Handles the response from the UI dialog."""
-        logger.info('Login successful for app %r, email %r', app_name, email)
+        logger.info('Login successful for app %r, email %r. Still pending to '
+                    'ping server and send result signal.', app_name, email)
         try:
             creds = keyring_get_credentials(app_name)
             self._ping_url(app_name, email, creds)
@@ -518,6 +564,7 @@
         except:  # pylint: disable=W0702
             msg = "Problem getting the credentials from the keyring."
             logger.exception(msg)
+            self.clear_token(app_name)
             self.CredentialsError(app_name, msg, traceback.format_exc())
 
     def _login_error_cb(self, dialog, app_name, error):
@@ -639,6 +686,7 @@
 
         'app_name' is the name of the application.
         """
+        logger.info('Clearing credentials for app %r.', app_name)
         try:
             creds = Keyring(app_name)
             creds.delete_ubuntusso_attr()

=== modified file 'ubuntu_sso/tests/test_gui.py'
--- ubuntu_sso/tests/test_gui.py	2010-09-08 19:25:02 +0000
+++ ubuntu_sso/tests/test_gui.py	2010-11-04 12:34:40 +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)."""
@@ -1005,6 +1011,17 @@
                           dict(reply_handler=gui.NO_OP,
                                error_handler=gui.NO_OP)))
 
+    def test_on_verify_token_button_clicked(self):
+        """Verify token uses cached user_email and user_password."""
+        self.ui.user_email = '[e-mail]'
+        self.ui.user_password = 'yadda-yedda'
+        self.ui.on_verify_token_button_clicked()
+        self.assertEqual(self.ui.backend._called['validate_email'],
+                         ((APP_NAME, self.ui.user_email,
+                           self.ui.user_password, EMAIL_TOKEN),
+                          dict(reply_handler=gui.NO_OP,
+                               error_handler=gui.NO_OP)))
+
     def test_on_verify_token_button_shows_processing_page(self):
         """Verify token button triggers call to backend."""
         self.click_verify_email_with_valid_data()
@@ -1079,7 +1096,7 @@
 
 
 class VerifyEmailValidationTestCase(UbuntuSSOClientTestCase):
-    """Test suite for the user registration (verify email page)."""
+    """Test suite for the user registration validation (verify email page)."""
 
     def setUp(self):
         """Init."""
@@ -1114,6 +1131,20 @@
         self.assert_warnings_visibility()
 
 
+class VerifyEmailLoginOnlyTestCase(VerifyEmailTestCase):
+    """Test suite for the user login (verify email page)."""
+
+    kwargs = dict(app_name=APP_NAME, tc_uri=TC_URI, help_text=HELP_TEXT,
+                  login_only=True)
+
+
+class VerifyEmailValidationLoginOnlyTestCase(VerifyEmailValidationTestCase):
+    """Test suite for the user login validation (verify email page)."""
+
+    kwargs = dict(app_name=APP_NAME, tc_uri=TC_URI, help_text=HELP_TEXT,
+                  login_only=True)
+
+
 class RegistrationValidationTestCase(UbuntuSSOClientTestCase):
     """Test suite for the user registration  validations."""
 
@@ -1328,6 +1359,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 +1414,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 +1825,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 +1907,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-09-08 19:25:02 +0000
+++ ubuntu_sso/tests/test_main.py	2010-11-04 12:34:40 +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):
@@ -380,12 +417,13 @@
         mockbus._register_object_path(ARGS)
         self.mockprocessorclass = None
 
-        def ksc(k, val):
+        def ksc(k, val, callback, *cb_args):
             """Assert over token and app_name."""
             self.assertEqual(k, APP_NAME)
             self.assertEqual(val, TOKEN)
             self.keyring_was_set = True
             self.keyring_values = k, val
+            callback(*cb_args)
 
         self.patch(ubuntu_sso.main, "keyring_store_credentials", ksc)
         self.keyring_was_set = False
@@ -519,8 +557,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 +576,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 +630,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 +660,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):
@@ -802,7 +871,7 @@
         self.assertEqual(result["errtype"], e.__class__.__name__)
 
 
-class KeyringCredentialsTestCase(MockerTestCase):
+class KeyringCredentialsTestCase(TestCase, MockerTestCase):
     """Check the functions that access the keyring."""
 
     # Invalid name (should match ([a-z_][a-z0-9_]*|[A-Z_][A-Z0-9_]*)$)
@@ -810,15 +879,19 @@
 
     def test_keyring_store_cred(self):
         """Verify the method that stores credentials."""
+        idle_add = lambda f, *args, **kwargs: f(*args, **kwargs)
+        self.patch(gobject, "idle_add", idle_add)
         token_value = TOKEN
         mockKeyringClass = self.mocker.replace("ubuntu_sso.keyring.Keyring")
         mockKeyringClass(APP_NAME)
         mockKeyring = self.mocker.mock()
+        callback = self.mocker.mock()
         self.mocker.result(mockKeyring)
         mockKeyring.set_ubuntusso_attr(token_value)
+        callback(1, 2, 3)
         self.mocker.replay()
 
-        keyring_store_credentials(APP_NAME, token_value)
+        keyring_store_credentials(APP_NAME, token_value, callback, 1, 2, 3)
 
     def test_keyring_get_cred(self):
         """The method returns the right token."""
@@ -1367,6 +1440,24 @@
         self.assertEqual(self.calls[0][0], 'CredentialsError')
         self.assertEqual(self.calls[0][1][0], APP_NAME)
 
+    def test_credentials_are_not_stored_if_ping_failed(self):
+        """Credentials are not stored if the ping fails."""
+
+        def fail(*args, **kwargs):
+            """Raise an exception."""
+            self.args = AssertionError((args, kwargs))
+            # pylint: disable=E0702
+            raise self.args
+
+        self.patch(self.client, '_ping_url', fail)
+        self._patch('clear_token')
+
+        self.client._login_success_cb(None, APP_NAME, EMAIL)
+
+        self.assertEqual(len(self.calls), 1)
+        self.assertEqual(self.calls[0][0], 'clear_token')
+        self.assertEqual(self.calls[0][1][0], APP_NAME)
+
 
 class EnvironOverridesTestCase(TestCase):
     """Some URLs can be set from the environment for testing/QA purposes."""