Add a consistent 5 minute clock skew accomodation (#145)

Author: Jon Wayne Parrott

google/auth/_helpers.py

@@ -22,6 +22,10 @@
 from six.moves import urllib
 
 
+CLOCK_SKEW_SECS = 300  # 5 minutes in seconds
+CLOCK_SKEW = datetime.timedelta(seconds=CLOCK_SKEW_SECS)
+
+
 def copy_docstring(source_class):
     """Decorator that copies a method's docstring from another class.
 

google/auth/credentials.py

@@ -56,8 +56,10 @@ def expired(self):
         Note that credentials can be invalid but not expired becaue Credentials
         with :attr:`expiry` set to None is considered to never expire.
         """
-        now = _helpers.utcnow()
-        return self.expiry is not None and self.expiry <= now
+        # Err on the side of reporting expiration early so that we avoid
+        # the 403-refresh-retry loop.
+        adjusted_now = _helpers.utcnow() - _helpers.CLOCK_SKEW
+        return self.expiry is not None and self.expiry <= adjusted_now
 
     @property
     def valid(self):

google/auth/jwt.py

@@ -52,8 +52,7 @@
 import google.auth.credentials
 
 
-_DEFAULT_TOKEN_LIFETIME_SECS = 3600  # 1 hour in sections
-_CLOCK_SKEW_SECS = 300  # 5 minutes in seconds
+_DEFAULT_TOKEN_LIFETIME_SECS = 3600  # 1 hour in seconds
 
 
 def encode(signer, payload, header=None, key_id=None):
@@ -161,21 +160,25 @@ def _verify_iat_and_exp(payload):
     """
     now = _helpers.datetime_to_secs(_helpers.utcnow())
 
-    # Make sure the iat and exp claims are present
+    # Make sure the iat and exp claims are present.
     for key in ('iat', 'exp'):
         if key not in payload:
             raise ValueError(
                 'Token does not contain required claim {}'.format(key))
 
-    # Make sure the token wasn't issued in the future
+    # Make sure the token wasn't issued in the future.
     iat = payload['iat']
-    earliest = iat - _CLOCK_SKEW_SECS
+    # Err on the side of accepting a token that is slightly early to account
+    # for clock skew.
+    earliest = iat - _helpers.CLOCK_SKEW_SECS
     if now < earliest:
         raise ValueError('Token used too early, {} < {}'.format(now, iat))
 
-    # Make sure the token wasn't issue in the past
+    # Make sure the token wasn't issued in the past.
     exp = payload['exp']
-    latest = exp + _CLOCK_SKEW_SECS
+    # Err on the side of accepting a token that is slightly out of date
+    # to account for clow skew.
+    latest = exp + _helpers.CLOCK_SKEW_SECS
     if latest < now:
         raise ValueError('Token expired, {} < {}'.format(latest, now))
 

tests/compute_engine/test_credentials.py

@@ -17,6 +17,7 @@
 import mock
 import pytest
 
+from google.auth import _helpers
 from google.auth import exceptions
 from google.auth.compute_engine import credentials
 
@@ -38,7 +39,8 @@ def test_default_state(self):
         assert self.credentials.service_account_email == 'default'
 
     @mock.patch(
-        'google.auth._helpers.utcnow', return_value=datetime.datetime.min)
+        'google.auth._helpers.utcnow',
+        return_value=datetime.datetime.min + _helpers.CLOCK_SKEW)
     @mock.patch('google.auth.compute_engine._metadata.get')
     def test_refresh_success(self, get_mock, now_mock):
         get_mock.side_effect = [{
@@ -57,7 +59,7 @@ def test_refresh_success(self, get_mock, now_mock):
         # Check that the credentials have the token and proper expiration
         assert self.credentials.token == 'token'
         assert self.credentials.expiry == (
-            datetime.datetime.min + datetime.timedelta(seconds=500))
+            now_mock() + datetime.timedelta(seconds=500))
 
         # Check the credential info
         assert (self.credentials.service_account_email ==

tests/oauth2/test_credentials.py

@@ -53,7 +53,8 @@ def test_create_scoped(self):
 
     @mock.patch('google.oauth2._client.refresh_grant', autospec=True)
     @mock.patch(
-        'google.auth._helpers.utcnow', return_value=datetime.datetime.min)
+        'google.auth._helpers.utcnow',
+        return_value=datetime.datetime.min + _helpers.CLOCK_SKEW)
     def test_refresh_success(self, now_mock, refresh_grant_mock):
         token = 'token'
         expiry = _helpers.utcnow() + datetime.timedelta(seconds=500)

tests/test_app_engine.py

@@ -17,6 +17,7 @@
 import mock
 import pytest
 
+from google.auth import _helpers
 from google.auth import app_engine
 
 
@@ -111,7 +112,7 @@ def test_service_account_email_explicit(self, app_identity_mock):
 
     @mock.patch(
         'google.auth._helpers.utcnow',
-        return_value=datetime.datetime.min)
+        return_value=datetime.datetime.min + _helpers.CLOCK_SKEW)
     def test_refresh(self, now_mock, app_identity_mock):
         token = 'token'
         ttl = 100
@@ -124,7 +125,7 @@ def test_refresh(self, now_mock, app_identity_mock):
             credentials.scopes, credentials._service_account_id)
         assert credentials.token == token
         assert credentials.expiry == (
-            datetime.datetime.min + datetime.timedelta(seconds=ttl))
+            now_mock() + datetime.timedelta(seconds=ttl))
         assert credentials.valid
         assert not credentials.expired
 

tests/test_credentials.py

@@ -14,6 +14,7 @@
 
 import datetime
 
+from google.auth import _helpers
 from google.auth import credentials
 
 
@@ -37,8 +38,21 @@ def test_expired_and_valid():
     assert credentials.valid
     assert not credentials.expired
 
+    # Set the expiration in the past, but because of clock skew accomodation
+    # the credentials should still be valid.
     credentials.expiry = (
-        datetime.datetime.utcnow() - datetime.timedelta(seconds=60))
+        datetime.datetime.utcnow() -
+        datetime.timedelta(seconds=1))
+
+    assert credentials.valid
+    assert not credentials.expired
+
+    # Set the credentials far enough in the past to exceed the clock skew
+    # accomodation. They should now be expired.
+    credentials.expiry = (
+        datetime.datetime.utcnow() -
+        _helpers.CLOCK_SKEW -
+        datetime.timedelta(seconds=1))
 
     assert not credentials.valid
     assert credentials.expired