Merge lp:~nataliabidart/ubuntu-sso-client/ping-url-can-be-none into lp:ubuntu-sso-client

[Natalia Bidart]

* Credentials should not be cleared if the ping wasn't made due to empty ping url (LP: #676679).

[Chad Miller (cmiller)]

review: Approve

[dobey (dobey)]

review: Approve

[Ubuntu One Auto Pilot (otto-pilot)]

There was a problem validating some authors of the branch. Authors must be either one of the listed Launchpad users, or a member of one of the listed teams on Launchpad.

Persons or Teams:

    contributor-agreement-canonical
    ubuntuone-hackers

Unaccepted Authors:

    Natalia B. Bidart <email address hidden>

[Ubuntu One Auto Pilot (otto-pilot)]

There was a problem validating some authors of the branch. Authors must be either one of the listed Launchpad users, or a member of one of the listed teams on Launchpad.

Persons or Teams:

    contributor-agreement-canonical
    ubuntuone-hackers

Unaccepted Authors:

    Natalia B. Bidart <email address hidden>

=== modified file 'ubuntu_sso/credentials.py'
--- ubuntu_sso/credentials.py	2010-11-17 19:37:12 +0000
+++ ubuntu_sso/credentials.py	2010-11-18 20:09:09 +0000
@@ -194,11 +194,14 @@
         creds = yield self.find_credentials()
         if creds is not None:
             assert len(creds) > 0, 'Creds are empty! This should not happen'
-            status = self._ping_url(app_name, email, creds)
-            if status is None:
-                yield self.clear_credentials()
-            else:
-                self.success_cb(creds)
+            # ping a server with the credentials if we were requested to
+            if self.ping_url is not None:
+                status = self._ping_url(app_name, email, creds)
+                if status is None:
+                    yield self.clear_credentials()
+                    return
+
+            self.success_cb(creds)
 
     def _login_error_cb(self, dialog, app_name, error):
         """Handle UI error when login/registration failed."""
@@ -215,28 +218,28 @@
     def _ping_url(self, app_name, email, credentials):
         """Ping the self.ping_url with the email attached.
 
-        Sign the request with the user credentials.
+        Sign the request with the user credentials. The self.ping_url must be
+        defined if this method is being called.
 
         """
-        logger.info('Maybe pinging server for app_name "%s", ping_url: "%s", '
+        logger.info('Pinging server for app_name "%s", ping_url: "%s", '
                     'email "%s".', app_name, self.ping_url, email)
-        if self.ping_url is not None:
-            url = self.ping_url + email
-            consumer = oauth.OAuthConsumer(credentials['consumer_key'],
-                                           credentials['consumer_secret'])
-            token = oauth.OAuthToken(credentials['token'],
-                                     credentials['token_secret'])
-            get_request = oauth.OAuthRequest.from_consumer_and_token
-            oauth_req = get_request(oauth_consumer=consumer, token=token,
-                                    http_method='GET', http_url=url)
-            oauth_req.sign_request(oauth.OAuthSignatureMethod_HMAC_SHA1(),
-                                   consumer, token)
-            request = urllib2.Request(url, headers=oauth_req.to_header())
-            logger.debug('Opening the url "%s" with urllib2.urlopen.',
-                         request.get_full_url())
-            response = urllib2.urlopen(request)
-            logger.debug('Url opened. Response: %s.', response.code)
-            return response.code
+        url = self.ping_url + email
+        consumer = oauth.OAuthConsumer(credentials['consumer_key'],
+                                       credentials['consumer_secret'])
+        token = oauth.OAuthToken(credentials['token'],
+                                 credentials['token_secret'])
+        get_request = oauth.OAuthRequest.from_consumer_and_token
+        oauth_req = get_request(oauth_consumer=consumer, token=token,
+                                http_method='GET', http_url=url)
+        oauth_req.sign_request(oauth.OAuthSignatureMethod_HMAC_SHA1(),
+                               consumer, token)
+        request = urllib2.Request(url, headers=oauth_req.to_header())
+        logger.debug('Opening the url "%s" with urllib2.urlopen.',
+                     request.get_full_url())
+        response = urllib2.urlopen(request)
+        logger.debug('Url opened. Response: %s.', response.code)
+        return response.code
 
     @handle_exceptions(msg='Problem opening the Ubuntu SSO user interface')
     def _show_ui(self, login_only):

=== modified file 'ubuntu_sso/tests/test_credentials.py'
--- ubuntu_sso/tests/test_credentials.py	2010-11-16 20:20:56 +0000
+++ ubuntu_sso/tests/test_credentials.py	2010-11-18 20:09:09 +0000
@@ -323,6 +323,23 @@
 
         self.assertEqual(self._url_pinged, ((APP_NAME, EMAIL, TOKEN), {}))
 
+    @inlineCallbacks
+    def test_no_ping_url_is_success(self):
+        """Auth success + cred found + no ping url, success_cb is called.
+
+        Credentials are NOT removed.
+
+        """
+        self.patch(credentials.Keyring, 'get_credentials',
+                   lambda kr, app: defer.succeed(TOKEN))  # auth success
+        self.patch(self.obj, 'clear_credentials', self._set_called)
+        self.obj.ping_url = None
+
+        yield self.obj._login_success_cb(dialog=None, app_name=APP_NAME,
+                                         email=EMAIL)
+
+        self.assertEqual(self._called, (('success', APP_NAME, TOKEN), {}))
+
 
 class CredentialsLoginErrorTestCase(CredentialsTestCase):
     """Test suite for the Credentials login error callback."""