Merge lp:~alecu/ubuntu-sso-client/timestamp-autofix into lp:ubuntu-sso-client

[Alejandro J. Cura]

Do a HEAD request on the server to get accurate timestamp (LP: #692597)

Do a HEAD request on the server to get accurate timestamp (LP: #692597)

[lp:~alecu/ubuntu-sso-client/timestamp-autofix]

796. By Alejandro J. Cura on 2011-10-06

fixing punctuation.

[Diego Sarmentero (diegosarmentero)]

+1

review: Approve

[Natalia Bidart (nataliabidart)]

Looks good, I have the same concerns I pointed out in the storage-protocol branch.

review: Approve

=== modified file 'ubuntu_sso/account.py'
--- ubuntu_sso/account.py	2011-03-16 01:36:07 +0000
+++ ubuntu_sso/account.py	2011-10-06 21:50:30 +0000
@@ -1,6 +1,7 @@
 # -*- coding: utf-8 -*-
 
 # Author: Natalia Bidart <[e-mail]>
+# Author: Alejandro J. Cura <[e-mail]>
 #
 # Copyright 2010 Canonical Ltd.
 #
@@ -31,6 +32,7 @@
 from oauth import oauth
 
 from ubuntu_sso.logger import setup_logging
+from ubuntu_sso.utils import timestamp_checker
 
 
 logger = setup_logging("ubuntu_sso.account")
@@ -39,6 +41,27 @@
 SSO_STATUS_ERROR = 'error'
 
 
+class TimestampedAuthorizer(OAuthAuthorizer):
+    """Includes a custom timestamp on OAuth signatures."""
+
+    def __init__(self, get_timestamp, *args, **kwargs):
+        """Store the get_timestamp method, and move on."""
+        super(TimestampedAuthorizer, self).__init__(*args, **kwargs)
+        self.get_timestamp = get_timestamp
+
+    # pylint: disable=C0103
+    def authorizeRequest(self, absolute_uri, method, body, headers):
+        """Override authorizeRequest including the timestamp."""
+        parameters = {"oauth_timestamp": self.get_timestamp()}
+        oauth_request = oauth.OAuthRequest.from_consumer_and_token(
+            self.consumer, self.access_token, http_url=absolute_uri,
+            parameters=parameters)
+        oauth_request.sign_request(
+            oauth.OAuthSignatureMethod_PLAINTEXT(),
+            self.consumer, self.access_token)
+        headers.update(oauth_request.to_header(self.oauth_realm))
+
+
 class InvalidEmailError(Exception):
     """The email is not valid."""
 
@@ -181,9 +204,11 @@
         if sso_service is None:
             oauth_token = oauth.OAuthToken(token['token'],
                                            token['token_secret'])
-            authorizer = OAuthAuthorizer(token['consumer_key'],
-                                         token['consumer_secret'],
-                                         oauth_token)
+            authorizer = TimestampedAuthorizer(
+                                        timestamp_checker.get_faithful_time,
+                                        token['consumer_key'],
+                                        token['consumer_secret'],
+                                        oauth_token)
             sso_service = self.sso_service_class(authorizer, self.service_url)
 
         me_info = sso_service.accounts.me()
@@ -203,9 +228,10 @@
                            token_name=token_name)
 
         oauth_token = oauth.OAuthToken(token['token'], token['token_secret'])
-        authorizer = OAuthAuthorizer(token['consumer_key'],
-                                     token['consumer_secret'],
-                                     oauth_token)
+        authorizer = TimestampedAuthorizer(timestamp_checker.get_faithful_time,
+                                           token['consumer_key'],
+                                           token['consumer_secret'],
+                                           oauth_token)
         sso_service = self.sso_service_class(authorizer, self.service_url)
         result = sso_service.accounts.validate_email(email_token=email_token)
         logger.info('validate_email: email: %r result: %r', email, result)

=== modified file 'ubuntu_sso/tests/test_account.py'
--- ubuntu_sso/tests/test_account.py	2011-09-02 13:05:06 +0000
+++ ubuntu_sso/tests/test_account.py	2011-10-06 21:50:30 +0000
@@ -1,6 +1,7 @@
 # -*- coding: utf-8 -*-
 #
 # Author: Natalia Bidart <[e-mail]>
+# Author: Alejandro J. Cura <[e-mail]>
 #
 # Copyright 2010 Canonical Ltd.
 #
@@ -24,15 +25,36 @@
 # pylint: disable=F0401
 from lazr.restfulclient.errors import HTTPError
 # pylint: enable=F0401
+from oauth import oauth
 from twisted.trial.unittest import TestCase
 
-from ubuntu_sso.account import (Account, AuthenticationError, EmailTokenError,
-    InvalidEmailError, InvalidPasswordError, NewPasswordError, SERVICE_URL,
-    RegistrationError, ResetPasswordTokenError,
-    SSO_STATUS_OK, SSO_STATUS_ERROR)
-from ubuntu_sso.tests import (APP_NAME, CAPTCHA_ID, CAPTCHA_PATH,
-    CAPTCHA_SOLUTION, EMAIL, EMAIL_TOKEN, NAME, PASSWORD, RESET_PASSWORD_TOKEN,
-    TOKEN, TOKEN_NAME)
+from ubuntu_sso.account import (
+    Account,
+    AuthenticationError,
+    EmailTokenError,
+    InvalidEmailError,
+    InvalidPasswordError,
+    NewPasswordError,
+    SERVICE_URL,
+    RegistrationError,
+    ResetPasswordTokenError,
+    SSO_STATUS_OK,
+    SSO_STATUS_ERROR,
+    TimestampedAuthorizer,
+)
+from ubuntu_sso.tests import (
+    APP_NAME,
+    CAPTCHA_ID,
+    CAPTCHA_PATH,
+    CAPTCHA_SOLUTION,
+    EMAIL,
+    EMAIL_TOKEN,
+    NAME,
+    PASSWORD,
+    RESET_PASSWORD_TOKEN,
+    TOKEN,
+    TOKEN_NAME,
+)
 
 
 CANT_RESET_PASSWORD_CONTENT = "CanNotResetPassowrdError: " \
@@ -145,6 +167,36 @@
         self.accounts = FakedAccounts()
 
 
+class TimestampedAuthorizerTestCase(TestCase):
+    """Test suite for the TimestampedAuthorizer."""
+
+    def test_authorize_request_includes_timestamp(self):
+        """The authorizeRequest method includes the timestamp."""
+        fromcandt_call = []
+        fake_uri = "http://protocultura.net"
+        fake_timestamp = 1234
+        get_fake_timestamp = lambda: fake_timestamp
+        original_oauthrequest = oauth.OAuthRequest
+
+        class FakeOAuthRequest(oauth.OAuthRequest):
+            """A Fake OAuthRequest class."""
+
+            @staticmethod
+            def from_consumer_and_token(*args, **kwargs):
+                """A fake from_consumer_and_token."""
+                fromcandt_call.append((args, kwargs))
+                builder = original_oauthrequest.from_consumer_and_token
+                return builder(*args, **kwargs)
+
+        self.patch(oauth, "OAuthRequest", FakeOAuthRequest)
+
+        authorizer = TimestampedAuthorizer(get_fake_timestamp, "ubuntuone")
+        authorizer.authorizeRequest(fake_uri, "POST", None, {})
+        call_kwargs = fromcandt_call[0][1]
+        parameters = call_kwargs["parameters"]
+        self.assertEqual(parameters["oauth_timestamp"], fake_timestamp)
+
+
 class AccountTestCase(TestCase):
     """Test suite for the SSO login processor."""
 

=== modified file 'ubuntu_sso/tests/test_credentials.py'
--- ubuntu_sso/tests/test_credentials.py	2011-08-22 16:21:36 +0000
+++ ubuntu_sso/tests/test_credentials.py	2011-10-06 21:50:30 +0000
@@ -20,6 +20,7 @@
 
 import logging
 import os
+import time
 import urllib2
 
 from twisted.internet import defer
@@ -373,6 +374,8 @@
             return response
 
         self.patch(credentials.urllib2, 'urlopen', faked_urlopen)
+        self.patch(credentials.utils.timestamp_checker, "get_faithful_time",
+                   time.time)
 
     @defer.inlineCallbacks
     def test_ping_url_if_url_is_none(self):

=== modified file 'ubuntu_sso/utils/__init__.py'
--- ubuntu_sso/utils/__init__.py	2011-08-12 12:54:36 +0000
+++ ubuntu_sso/utils/__init__.py	2011-10-06 21:50:30 +0000
@@ -2,7 +2,7 @@
 
 # Author: Alejandro J. Cura <[e-mail]>
 #
-# Copyright 2010 Canonical Ltd.
+# Copyright 2010, 2011 Canonical Ltd.
 #
 # This program is free software: you can redistribute it and/or modify it
 # under the terms of the GNU General Public License version 3, as published
@@ -19,9 +19,71 @@
 """Utility modules that may find use outside ubuntu_sso."""
 
 import cgi
+import time
+import urllib2
 
 from oauth import oauth
 from urlparse import urlparse
+from twisted.web import http
+
+from ubuntu_sso.logger import setup_logging
+logger = setup_logging("ubuntu_sso.utils")
+
+
+class RequestHead(urllib2.Request):
+    """A request with the method set to HEAD."""
+
+    _request_method = "HEAD"
+
+    def get_method(self):
+        """Return the desired method."""
+        return self._request_method
+
+
+class SyncTimestampChecker(object):
+    """A timestamp that's regularly checked with a server."""
+
+    CHECKING_INTERVAL = 60 * 60  # in seconds
+    ERROR_INTERVAL = 30  # in seconds
+    SERVER_URL = "http://one.ubuntu.com/"
+
+    def __init__(self):
+        """Initialize this instance."""
+        self.next_check = time.time()
+        self.skew = 0
+
+    def get_server_time(self):
+        """Get the time at the server."""
+        headers = {"Cache-Control": "no-cache"}
+        request = RequestHead(self.SERVER_URL, headers=headers)
+        response = urllib2.urlopen(request)
+        date_string = response.info()["Date"]
+        timestamp = http.stringToDatetime(date_string)
+        return timestamp
+
+    def get_faithful_time(self):
+        """Get an accurate timestamp."""
+        local_time = time.time()
+        if local_time >= self.next_check:
+            try:
+                server_time = self.get_server_time()
+                self.next_check = local_time + self.CHECKING_INTERVAL
+                self.skew = server_time - local_time
+                logger.debug("Calculated server-local time skew: %r",
+                             self.skew)
+            #pylint: disable=W0703
+            except Exception, server_error:
+                logger.debug("Error while verifying the server time skew: %r",
+                             server_error)
+                self.next_check = local_time + self.ERROR_INTERVAL
+        logger.debug("Using corrected timestamp: %r",
+                     http.datetimeToString(local_time + self.skew))
+        return int(local_time + self.skew)
+
+
+# pylint: disable=C0103
+timestamp_checker = SyncTimestampChecker()
+# pylint: enable=C0103
 
 
 def oauth_headers(url, credentials, http_method='GET'):
@@ -37,6 +99,7 @@
     url = url.encode('utf-8')
     _, _, _, _, query, _ = urlparse(url)
     parameters = dict(cgi.parse_qsl(query))
+    parameters["oauth_timestamp"] = timestamp_checker.get_faithful_time()
 
     consumer = oauth.OAuthConsumer(credentials['consumer_key'],
                                    credentials['consumer_secret'])

=== modified file 'ubuntu_sso/utils/tests/test_oauth_headers.py'
--- ubuntu_sso/utils/tests/test_oauth_headers.py	2011-08-12 12:54:36 +0000
+++ ubuntu_sso/utils/tests/test_oauth_headers.py	2011-10-06 21:50:30 +0000
@@ -1,6 +1,7 @@
 # -*- coding: utf-8 -*-
 
 # Author: Natalia B. Bidart <[e-mail]>
+# Author: Alejandro J. Cura <[e-mail]>
 #
 # Copyright 2011 Canonical Ltd.
 #
@@ -18,9 +19,16 @@
 
 """Tests for the oauth_headers helper function."""
 
+import time
+
+from twisted.application import internet, service
+from twisted.internet import defer
+from twisted.internet.threads import deferToThread
 from twisted.trial.unittest import TestCase
+from twisted.web import server, resource
 
-from ubuntu_sso.utils import oauth, oauth_headers
+from ubuntu_sso import utils
+from ubuntu_sso.utils import oauth, oauth_headers, SyncTimestampChecker
 from ubuntu_sso.tests import TOKEN
 
 
@@ -44,6 +52,21 @@
     """Test suite for the oauth_headers method."""
 
     url = u'http://example.com'
+    fake_timestamp_value = 1
+
+    @defer.inlineCallbacks
+    def setUp(self):
+        """Initialize this test suite."""
+        yield super(SignWithCredentialsTestCase, self).setUp()
+        self.timestamp_called = False
+
+        def fake_timestamp():
+            """A fake timestamp that records the call."""
+            self.timestamp_called = True
+            return self.fake_timestamp_value
+
+        self.patch(utils.timestamp_checker, "get_faithful_time",
+                   fake_timestamp)
 
     def build_header(self, url, http_method='GET'):
         """Build an Oauth header for comparison."""
@@ -105,5 +128,131 @@
         oauth_headers(url=self.url + path, credentials=TOKEN)
 
         self.assertIn('parameters', FakedOAuthRequest.params)
-        self.assertEqual(FakedOAuthRequest.params['parameters'],
-                         {'foo': 'bar'})
+        params = FakedOAuthRequest.params['parameters']
+        del(params["oauth_timestamp"])
+        self.assertEqual(params, {'foo': 'bar'})
+
+    def test_oauth_headers_uses_timestamp_checker(self):
+        """The oauth_headers function uses the timestamp_checker."""
+        oauth_headers(u"http://protocultura.net", TOKEN)
+        self.assertTrue(self.timestamp_called,
+                        "the timestamp MUST be requested.")
+
+
+class RootResource(resource.Resource):
+    """A root resource that logs the number of calls."""
+
+    isLeaf = True
+
+    def __init__(self, *args, **kwargs):
+        """Initialize this fake instance."""
+        resource.Resource.__init__(self, *args, **kwargs)
+        self.count = 0
+        self.request_headers = []
+
+    # pylint: disable=C0103
+    def render_HEAD(self, request):
+        """Increase the counter on each render."""
+        self.count += 1
+        self.request_headers.append(request.requestHeaders)
+        return ""
+
+
+class MockWebServer(object):
+    """A mock webserver for testing."""
+
+    def __init__(self):
+        """Start up this instance."""
+        # pylint: disable=E1101
+        self.root = RootResource()
+        site = server.Site(self.root)
+        application = service.Application('web')
+        self.service_collection = service.IServiceCollection(application)
+        self.tcpserver = internet.TCPServer(0, site)
+        self.tcpserver.setServiceParent(self.service_collection)
+        self.service_collection.startService()
+
+    def get_url(self):
+        """Build the url for this mock server."""
+        # pylint: disable=W0212
+        port_num = self.tcpserver._port.getHost().port
+        return "http://localhost:%d/" % port_num
+
+    def stop(self):
+        """Shut it down."""
+        # pylint: disable=E1101
+        self.service_collection.stopService()
+
+
+class FakedError(Exception):
+    """A mock, test, sample, and fake exception."""
+
+
+class TimestampCheckerTestCase(TestCase):
+    """Tests for the timestamp checker."""
+
+    def setUp(self):
+        """Initialize a fake webserver."""
+        self.ws = MockWebServer()
+        self.addCleanup(self.ws.stop)
+        self.patch(SyncTimestampChecker, "SERVER_URL", self.ws.get_url())
+
+    @defer.inlineCallbacks
+    def test_returned_value_is_int(self):
+        """The returned value is an integer."""
+        checker = SyncTimestampChecker()
+        timestamp = yield deferToThread(checker.get_faithful_time)
+        self.assertEqual(type(timestamp), int)
+
+    @defer.inlineCallbacks
+    def test_first_call_does_head(self):
+        """The first call gets the clock from our web."""
+        checker = SyncTimestampChecker()
+        yield deferToThread(checker.get_faithful_time)
+        self.assertEqual(self.ws.root.count, 1)
+
+    @defer.inlineCallbacks
+    def test_second_call_is_cached(self):
+        """For the second call, the time is cached."""
+        checker = SyncTimestampChecker()
+        yield deferToThread(checker.get_faithful_time)
+        yield deferToThread(checker.get_faithful_time)
+        self.assertEqual(self.ws.root.count, 1)
+
+    @defer.inlineCallbacks
+    def test_after_timeout_cache_expires(self):
+        """After some time, the cache expires."""
+        fake_timestamp = 1
+        self.patch(time, "time", lambda: fake_timestamp)
+        checker = SyncTimestampChecker()
+        yield deferToThread(checker.get_faithful_time)
+        fake_timestamp += SyncTimestampChecker.CHECKING_INTERVAL
+        yield deferToThread(checker.get_faithful_time)
+        self.assertEqual(self.ws.root.count, 2)
+
+    @defer.inlineCallbacks
+    def test_server_date_sends_nocache_headers(self):
+        """Getting the server date sends the no-cache headers."""
+        checker = SyncTimestampChecker()
+        yield deferToThread(checker.get_server_time)
+        assert len(self.ws.root.request_headers) == 1
+        headers = self.ws.root.request_headers[0]
+        result = headers.getRawHeaders("Cache-Control")
+        self.assertEqual(result, ["no-cache"])
+
+    @defer.inlineCallbacks
+    def test_server_error_means_skew_not_updated(self):
+        """When server can't be reached, the skew is not updated."""
+        fake_timestamp = 1
+        self.patch(time, "time", lambda: fake_timestamp)
+        checker = SyncTimestampChecker()
+
+        def failing_get_server_time():
+            """Let's fail while retrieving the server time."""
+            raise FakedError()
+
+        self.patch(checker, "get_server_time", failing_get_server_time)
+        yield deferToThread(checker.get_faithful_time)
+        self.assertEqual(checker.skew, 0)
+        self.assertEqual(checker.next_check,
+                         fake_timestamp + SyncTimestampChecker.ERROR_INTERVAL)