Repository: GoogleCloudPlatform/google-auth-library-python
Pull request #158 (2017-05-05 23:14:41)

Fix clock skew calculations
Previously, the clock skew adjusted in the wrong direction. It would cause us to consider credentials whose expiration time had already passed according to the system clock to still be sent to the server. This is the opposite of what we wanted to happen. This fixes it so that we report that credentials are expired slightly before the system clock thinks they've expired.


Patch:
diff --git a/google/auth/credentials.py b/google/auth/credentials.py
index 6fb89d8..74d6788 100644
--- a/google/auth/credentials.py
+++ b/google/auth/credentials.py
@@ -56,10 +56,13 @@ class Credentials(object):
         Note that credentials can be invalid but not expired becaue Credentials
         with :attr:`expiry` set to None is considered to never expire.
         """
-        # 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
+        if not self.expiry:
+            return False
+
+        # Remove 5 minutes from expiry to err on the side of reporting
+        # expiration early so that we avoid the 401-refresh-retry loop.
+        skewed_expiry = self.expiry - _helpers.CLOCK_SKEW
+        return _helpers.utcnow() >= skewed_expiry
 
     @property
     def valid(self):


Test patch:
diff --git a/tests/test_app_engine.py b/tests/test_app_engine.py
index 136a914..2b957f0 100644
--- a/tests/test_app_engine.py
+++ b/tests/test_app_engine.py
@@ -112,10 +112,10 @@ class TestCredentials(object):
 
     @mock.patch(
         'google.auth._helpers.utcnow',
-        return_value=datetime.datetime.min + _helpers.CLOCK_SKEW)
+        return_value=datetime.datetime.min)
     def test_refresh(self, now_mock, app_identity_mock):
         token = 'token'
-        ttl = 100
+        ttl = _helpers.CLOCK_SKEW_SECS + 100
         app_identity_mock.get_access_token.return_value = token, ttl
         credentials = app_engine.Credentials(scopes=['email'])
 
diff --git a/tests/test_credentials.py b/tests/test_credentials.py
index c2ff844..b5a540d 100644
--- a/tests/test_credentials.py
+++ b/tests/test_credentials.py
@@ -38,21 +38,19 @@ 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.
+    # Set the expiration to one second more than now plus the clock skew
+    # accomodation. These credentials should be valid.
     credentials.expiry = (
-        datetime.datetime.utcnow() -
+        datetime.datetime.utcnow() +
+        _helpers.CLOCK_SKEW +
         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))
+    # Set the credentials expiration to now. Because of the clock skew
+    # accomodation, these credentials should report as expired.
+    credentials.expiry = datetime.datetime.utcnow()
 
     assert not credentials.valid
     assert credentials.expired
diff --git a/tests/test_iam.py b/tests/test_iam.py
index f776788..97fa907 100644
--- a/tests/test_iam.py
+++ b/tests/test_iam.py
@@ -20,6 +20,7 @@ import mock
 import pytest
 from six.moves import http_client
 
+from google.auth import _helpers
 from google.auth import exceptions
 from google.auth import iam
 from google.auth import transport
@@ -42,7 +43,7 @@ def make_credentials():
             super(CredentialsImpl, self).__init__()
             self.token = 'token'
             # Force refresh
-            self.expiry = datetime.datetime.min
+            self.expiry = datetime.datetime.min + _helpers.CLOCK_SKEW
 
         def refresh(self, request):
             pass
diff --git a/tests/transport/test_grpc.py b/tests/transport/test_grpc.py
index 7a3cc0a..a494ee5 100644
--- a/tests/transport/test_grpc.py
+++ b/tests/transport/test_grpc.py
@@ -17,6 +17,7 @@ import datetime
 import mock
 import pytest
 
+from google.auth import _helpers
 from google.auth import credentials
 try:
     import google.auth.transport.grpc
@@ -56,7 +57,7 @@ class TestAuthMetadataPlugin(object):
 
     def test_call_refresh(self):
         credentials = MockCredentials()
-        credentials.expiry = datetime.datetime.min
+        credentials.expiry = datetime.datetime.min + _helpers.CLOCK_SKEW
         request = mock.Mock()
 
         plugin = google.auth.transport.grpc.AuthMetadataPlugin(