Fix clock skew calculations (#158)

Previously, the clock skew adjusted in the wrong direction. It would cause us to consider credentials whose expiration time had already pass 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.

google/auth/credentials.py

@@ -56,10 +56,13 @@ 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.
         """
-        # 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):

tests/test_app_engine.py

@@ -112,10 +112,10 @@ def test_service_account_email_explicit(self, app_identity_mock):
 
     @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'])
 

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

tests/test_iam.py

@@ -20,6 +20,7 @@
 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 __init__(self):
             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

tests/transport/test_grpc.py

@@ -17,6 +17,7 @@
 import mock
 import pytest
 
+from google.auth import _helpers
 from google.auth import credentials
 try:
     import google.auth.transport.grpc
@@ -56,7 +57,7 @@ def test_call_no_refresh(self):
 
     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(