From 50bcae88b7366c1343524d177c1a7f40f8533f4a Mon Sep 17 00:00:00 2001 From: Orest Bolohan Date: Mon, 2 Jun 2014 15:04:34 -0700 Subject: [PATCH 1/5] Modify the OAuth2Credentials.get_access_token() method to return the expiration information, in addition to the access token. --- oauth2client/client.py | 18 +++++++++++++----- tests/test_appengine.py | 3 ++- tests/test_gce.py | 5 +++-- tests/test_oauth2client.py | 15 +++++++++------ tests/test_service_account.py | 17 ++++++++++------- 5 files changed, 37 insertions(+), 21 deletions(-) diff --git a/oauth2client/client.py b/oauth2client/client.py index 6279d7a..aedfecf 100644 --- a/oauth2client/client.py +++ b/oauth2client/client.py @@ -600,18 +600,16 @@ class OAuth2Credentials(Credentials): return False def get_access_token(self, http=None): - """Return the access token. + """Return the access token and its expiration information. If the token does not exist, get one. If the token expired, refresh it. """ - if self.access_token and not self.access_token_expired: - return self.access_token - else: + if not self.access_token or self.access_token_expired: if not http: http = httplib2.Http() self.refresh(http) - return self.access_token + return {'access_token': self.access_token, 'expires_in': self._expires_in()} def set_store(self, store): """Set the Storage for the credential. @@ -625,6 +623,16 @@ class OAuth2Credentials(Credentials): """ self.store = store + def _expires_in(self): + """Get in how many seconds does the token expire.""" + if self.token_expiry: + now = datetime.datetime.utcnow() + if self.token_expiry > now: + time_delta = self.token_expiry - now + return int(round(time_delta.days * 86400.0 + + time_delta.seconds + + time_delta.microseconds * 0.000001)) + def _updateFromCredential(self, other): """Update this Credential from another instance.""" self.__dict__.update(other.__getstate__()) diff --git a/tests/test_appengine.py b/tests/test_appengine.py index e75ca88..d783036 100644 --- a/tests/test_appengine.py +++ b/tests/test_appengine.py @@ -245,7 +245,8 @@ class TestAppAssertionCredentials(unittest.TestCase): credentials = AppAssertionCredentials(['dummy_scope']) token = credentials.get_access_token() - self.assertEqual('a_token_123', token) + self.assertEqual('a_token_123', token['access_token']) + self.assertEqual(None, token['expires_in']) class TestFlowModel(db.Model): diff --git a/tests/test_gce.py b/tests/test_gce.py index ec87512..450bda1 100644 --- a/tests/test_gce.py +++ b/tests/test_gce.py @@ -125,8 +125,9 @@ class AssertionCredentialsTests(unittest.TestCase): http = httplib2.Http() http.request = httplib2_request - self.assertEquals('this-is-a-token', - credentials.get_access_token(http=http)) + token = credentials.get_access_token(http=http) + self.assertEqual('this-is-a-token', token['access_token']) + self.assertEqual(None, token['expires_in']) m.UnsetStubs() m.VerifyAll() diff --git a/tests/test_oauth2client.py b/tests/test_oauth2client.py index 4d9771d..58b3533 100644 --- a/tests/test_oauth2client.py +++ b/tests/test_oauth2client.py @@ -590,21 +590,24 @@ class BasicCredentialsTests(unittest.TestCase): ({'status': '200'}, simplejson.dumps(token_response_second)), ]) - self.assertEqual('first_token', - self.credentials.get_access_token(http=http)) + token = self.credentials.get_access_token(http=http) + self.assertEqual('first_token', token['access_token']) + self.assertEqual(1, token['expires_in']) self.assertFalse(self.credentials.access_token_expired) self.assertEqual(token_response_first, self.credentials.token_response) - self.assertEqual('first_token', - self.credentials.get_access_token(http=http)) + token = self.credentials.get_access_token(http=http) + self.assertEqual('first_token', token['access_token']) + self.assertEqual(1, token['expires_in']) self.assertFalse(self.credentials.access_token_expired) self.assertEqual(token_response_first, self.credentials.token_response) time.sleep(1) self.assertTrue(self.credentials.access_token_expired) - self.assertEqual('second_token', - self.credentials.get_access_token(http=http)) + token = self.credentials.get_access_token(http=http) + self.assertEqual('second_token', token['access_token']) + self.assertEqual(1, token['expires_in']) self.assertFalse(self.credentials.access_token_expired) self.assertEqual(token_response_second, self.credentials.token_response) diff --git a/tests/test_service_account.py b/tests/test_service_account.py index f31394f..20d5fdf 100644 --- a/tests/test_service_account.py +++ b/tests/test_service_account.py @@ -100,21 +100,24 @@ class ServiceAccountCredentialsTests(unittest.TestCase): ({'status': '200'}, simplejson.dumps(token_response_first)), ({'status': '200'}, simplejson.dumps(token_response_second)), ]) - - self.assertEqual('first_token', - self.credentials.get_access_token(http=http)) + + token = self.credentials.get_access_token(http=http) + self.assertEqual('first_token', token['access_token']) + self.assertEqual(1, token['expires_in']) self.assertFalse(self.credentials.access_token_expired) self.assertEqual(token_response_first, self.credentials.token_response) - self.assertEqual('first_token', - self.credentials.get_access_token(http=http)) + token = self.credentials.get_access_token(http=http) + self.assertEqual('first_token', token['access_token']) + self.assertEqual(1, token['expires_in']) self.assertFalse(self.credentials.access_token_expired) self.assertEqual(token_response_first, self.credentials.token_response) time.sleep(1) self.assertTrue(self.credentials.access_token_expired) - self.assertEqual('second_token', - self.credentials.get_access_token(http=http)) + token = self.credentials.get_access_token(http=http) + self.assertEqual('second_token', token['access_token']) + self.assertEqual(1, token['expires_in']) self.assertFalse(self.credentials.access_token_expired) self.assertEqual(token_response_second, self.credentials.token_response) From dff7414cb7926d1467fe7334b6d97febccd63637 Mon Sep 17 00:00:00 2001 From: Orest Bolohan Date: Mon, 2 Jun 2014 17:19:54 -0700 Subject: [PATCH 2/5] Make OAuth2Crdentials.get_access_token() method return a collections.namedtuple rather than a dictionary. --- oauth2client/client.py | 7 ++++++- tests/test_appengine.py | 4 ++-- tests/test_gce.py | 4 ++-- tests/test_oauth2client.py | 12 ++++++------ tests/test_service_account.py | 12 ++++++------ 5 files changed, 22 insertions(+), 17 deletions(-) diff --git a/oauth2client/client.py b/oauth2client/client.py index aedfecf..c4e31cd 100644 --- a/oauth2client/client.py +++ b/oauth2client/client.py @@ -31,6 +31,7 @@ import time import urllib import urlparse +from collections import namedtuple from oauth2client import GOOGLE_AUTH_URI from oauth2client import GOOGLE_REVOKE_URI from oauth2client import GOOGLE_TOKEN_URI @@ -75,6 +76,9 @@ SERVICE_ACCOUNT = 'service_account' # The environment variable pointing the file with local Default Credentials. GOOGLE_CREDENTIALS_DEFAULT = 'GOOGLE_CREDENTIALS_DEFAULT' +# The access token along with the seconds in which it expires. +AccessTokenInfo = namedtuple('AccessTokenInfo', ['access_token', 'expires_in']) + class Error(Exception): """Base error for this module.""" @@ -609,7 +613,8 @@ class OAuth2Credentials(Credentials): if not http: http = httplib2.Http() self.refresh(http) - return {'access_token': self.access_token, 'expires_in': self._expires_in()} + return AccessTokenInfo(access_token=self.access_token, + expires_in=self._expires_in()) def set_store(self, store): """Set the Storage for the credential. diff --git a/tests/test_appengine.py b/tests/test_appengine.py index d783036..2bea272 100644 --- a/tests/test_appengine.py +++ b/tests/test_appengine.py @@ -245,8 +245,8 @@ class TestAppAssertionCredentials(unittest.TestCase): credentials = AppAssertionCredentials(['dummy_scope']) token = credentials.get_access_token() - self.assertEqual('a_token_123', token['access_token']) - self.assertEqual(None, token['expires_in']) + self.assertEqual('a_token_123', token.access_token) + self.assertEqual(None, token.expires_in) class TestFlowModel(db.Model): diff --git a/tests/test_gce.py b/tests/test_gce.py index 450bda1..60e876e 100644 --- a/tests/test_gce.py +++ b/tests/test_gce.py @@ -126,8 +126,8 @@ class AssertionCredentialsTests(unittest.TestCase): http.request = httplib2_request token = credentials.get_access_token(http=http) - self.assertEqual('this-is-a-token', token['access_token']) - self.assertEqual(None, token['expires_in']) + self.assertEqual('this-is-a-token', token.access_token) + self.assertEqual(None, token.expires_in) m.UnsetStubs() m.VerifyAll() diff --git a/tests/test_oauth2client.py b/tests/test_oauth2client.py index 58b3533..64a104a 100644 --- a/tests/test_oauth2client.py +++ b/tests/test_oauth2client.py @@ -591,14 +591,14 @@ class BasicCredentialsTests(unittest.TestCase): ]) token = self.credentials.get_access_token(http=http) - self.assertEqual('first_token', token['access_token']) - self.assertEqual(1, token['expires_in']) + self.assertEqual('first_token', token.access_token) + self.assertEqual(1, token.expires_in) self.assertFalse(self.credentials.access_token_expired) self.assertEqual(token_response_first, self.credentials.token_response) token = self.credentials.get_access_token(http=http) - self.assertEqual('first_token', token['access_token']) - self.assertEqual(1, token['expires_in']) + self.assertEqual('first_token', token.access_token) + self.assertEqual(1, token.expires_in) self.assertFalse(self.credentials.access_token_expired) self.assertEqual(token_response_first, self.credentials.token_response) @@ -606,8 +606,8 @@ class BasicCredentialsTests(unittest.TestCase): self.assertTrue(self.credentials.access_token_expired) token = self.credentials.get_access_token(http=http) - self.assertEqual('second_token', token['access_token']) - self.assertEqual(1, token['expires_in']) + self.assertEqual('second_token', token.access_token) + self.assertEqual(1, token.expires_in) self.assertFalse(self.credentials.access_token_expired) self.assertEqual(token_response_second, self.credentials.token_response) diff --git a/tests/test_service_account.py b/tests/test_service_account.py index 20d5fdf..952483c 100644 --- a/tests/test_service_account.py +++ b/tests/test_service_account.py @@ -102,14 +102,14 @@ class ServiceAccountCredentialsTests(unittest.TestCase): ]) token = self.credentials.get_access_token(http=http) - self.assertEqual('first_token', token['access_token']) - self.assertEqual(1, token['expires_in']) + self.assertEqual('first_token', token.access_token) + self.assertEqual(1, token.expires_in) self.assertFalse(self.credentials.access_token_expired) self.assertEqual(token_response_first, self.credentials.token_response) token = self.credentials.get_access_token(http=http) - self.assertEqual('first_token', token['access_token']) - self.assertEqual(1, token['expires_in']) + self.assertEqual('first_token', token.access_token) + self.assertEqual(1, token.expires_in) self.assertFalse(self.credentials.access_token_expired) self.assertEqual(token_response_first, self.credentials.token_response) @@ -117,7 +117,7 @@ class ServiceAccountCredentialsTests(unittest.TestCase): self.assertTrue(self.credentials.access_token_expired) token = self.credentials.get_access_token(http=http) - self.assertEqual('second_token', token['access_token']) - self.assertEqual(1, token['expires_in']) + self.assertEqual('second_token', token.access_token) + self.assertEqual(1, token.expires_in) self.assertFalse(self.credentials.access_token_expired) self.assertEqual(token_response_second, self.credentials.token_response) From e0c51b98dd18a3a878560ada983eec221419168d Mon Sep 17 00:00:00 2001 From: Orest Bolohan Date: Tue, 3 Jun 2014 08:43:35 -0700 Subject: [PATCH 3/5] Applied some of Craig's suggestions. --- oauth2client/client.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/oauth2client/client.py b/oauth2client/client.py index c4e31cd..31663ab 100644 --- a/oauth2client/client.py +++ b/oauth2client/client.py @@ -629,7 +629,7 @@ class OAuth2Credentials(Credentials): self.store = store def _expires_in(self): - """Get in how many seconds does the token expire.""" + """Return the number of seconds until this token expires.""" if self.token_expiry: now = datetime.datetime.utcnow() if self.token_expiry > now: @@ -637,6 +637,8 @@ class OAuth2Credentials(Credentials): return int(round(time_delta.days * 86400.0 + time_delta.seconds + time_delta.microseconds * 0.000001)) + else: + return 0 def _updateFromCredential(self, other): """Update this Credential from another instance.""" From 4bcd15ed0e0b672bdd6d17ef950366c7d8a9b157 Mon Sep 17 00:00:00 2001 From: Orest Bolohan Date: Tue, 3 Jun 2014 10:39:19 -0700 Subject: [PATCH 4/5] Tweak the computation of seconds from a datetime.timedelta object, so that we consider only the whole seconds. --- oauth2client/client.py | 6 +++--- tests/test_oauth2client.py | 13 +++++++------ tests/test_service_account.py | 13 +++++++------ 3 files changed, 17 insertions(+), 15 deletions(-) diff --git a/oauth2client/client.py b/oauth2client/client.py index 31663ab..575fa49 100644 --- a/oauth2client/client.py +++ b/oauth2client/client.py @@ -634,9 +634,9 @@ class OAuth2Credentials(Credentials): now = datetime.datetime.utcnow() if self.token_expiry > now: time_delta = self.token_expiry - now - return int(round(time_delta.days * 86400.0 + - time_delta.seconds + - time_delta.microseconds * 0.000001)) + # TODO(orestica): return time_delta.total_seconds() + # once dropping support for Python 2.6 + return time_delta.days * 86400 + time_delta.seconds else: return 0 diff --git a/tests/test_oauth2client.py b/tests/test_oauth2client.py index 64a104a..72e3b4e 100644 --- a/tests/test_oauth2client.py +++ b/tests/test_oauth2client.py @@ -583,8 +583,9 @@ class BasicCredentialsTests(unittest.TestCase): self.assertEqual('foobar', instance.token_response) def test_get_access_token(self): - token_response_first = {'access_token': 'first_token', 'expires_in': 1} - token_response_second = {'access_token': 'second_token', 'expires_in': 1} + S = 2 # number of seconds in which the token expires + token_response_first = {'access_token': 'first_token', 'expires_in': S} + token_response_second = {'access_token': 'second_token', 'expires_in': S} http = HttpMockSequence([ ({'status': '200'}, simplejson.dumps(token_response_first)), ({'status': '200'}, simplejson.dumps(token_response_second)), @@ -592,22 +593,22 @@ class BasicCredentialsTests(unittest.TestCase): token = self.credentials.get_access_token(http=http) self.assertEqual('first_token', token.access_token) - self.assertEqual(1, token.expires_in) + self.assertEqual(S - 1, token.expires_in) self.assertFalse(self.credentials.access_token_expired) self.assertEqual(token_response_first, self.credentials.token_response) token = self.credentials.get_access_token(http=http) self.assertEqual('first_token', token.access_token) - self.assertEqual(1, token.expires_in) + self.assertEqual(S - 1, token.expires_in) self.assertFalse(self.credentials.access_token_expired) self.assertEqual(token_response_first, self.credentials.token_response) - time.sleep(1) + time.sleep(S) self.assertTrue(self.credentials.access_token_expired) token = self.credentials.get_access_token(http=http) self.assertEqual('second_token', token.access_token) - self.assertEqual(1, token.expires_in) + self.assertEqual(S - 1, token.expires_in) self.assertFalse(self.credentials.access_token_expired) self.assertEqual(token_response_second, self.credentials.token_response) diff --git a/tests/test_service_account.py b/tests/test_service_account.py index 952483c..56313d3 100644 --- a/tests/test_service_account.py +++ b/tests/test_service_account.py @@ -94,8 +94,9 @@ class ServiceAccountCredentialsTests(unittest.TestCase): self.assertEqual('dummy_scope', new_credentials._scopes) def test_access_token(self): - token_response_first = {'access_token': 'first_token', 'expires_in': 1} - token_response_second = {'access_token': 'second_token', 'expires_in': 1} + S = 2 # number of seconds in which the token expires + token_response_first = {'access_token': 'first_token', 'expires_in': S} + token_response_second = {'access_token': 'second_token', 'expires_in': S} http = HttpMockSequence([ ({'status': '200'}, simplejson.dumps(token_response_first)), ({'status': '200'}, simplejson.dumps(token_response_second)), @@ -103,21 +104,21 @@ class ServiceAccountCredentialsTests(unittest.TestCase): token = self.credentials.get_access_token(http=http) self.assertEqual('first_token', token.access_token) - self.assertEqual(1, token.expires_in) + self.assertEqual(S - 1, token.expires_in) self.assertFalse(self.credentials.access_token_expired) self.assertEqual(token_response_first, self.credentials.token_response) token = self.credentials.get_access_token(http=http) self.assertEqual('first_token', token.access_token) - self.assertEqual(1, token.expires_in) + self.assertEqual(S - 1, token.expires_in) self.assertFalse(self.credentials.access_token_expired) self.assertEqual(token_response_first, self.credentials.token_response) - time.sleep(1) + time.sleep(S) self.assertTrue(self.credentials.access_token_expired) token = self.credentials.get_access_token(http=http) self.assertEqual('second_token', token.access_token) - self.assertEqual(1, token.expires_in) + self.assertEqual(S - 1, token.expires_in) self.assertFalse(self.credentials.access_token_expired) self.assertEqual(token_response_second, self.credentials.token_response) From 95830b0a52be1e64d60b331e917b0e8618a559a9 Mon Sep 17 00:00:00 2001 From: Orest Bolohan Date: Tue, 3 Jun 2014 15:36:17 -0700 Subject: [PATCH 5/5] Add additional docstring for the OAuth2Credentials._expires_in() method, explaining what values are returned when token_expiry is in the past or None. --- oauth2client/client.py | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/oauth2client/client.py b/oauth2client/client.py index 575fa49..a3af4f8 100644 --- a/oauth2client/client.py +++ b/oauth2client/client.py @@ -629,7 +629,14 @@ class OAuth2Credentials(Credentials): self.store = store def _expires_in(self): - """Return the number of seconds until this token expires.""" + """Return the number of seconds until this token expires. + + If token_expiry is in the past, this method will return 0, meaning the + token has already expired. + If token_expiry is None, this method will return None. Note that returning + 0 in such a case would not be fair: the token may still be valid; + we just don't know anything about it. + """ if self.token_expiry: now = datetime.datetime.utcnow() if self.token_expiry > now: