Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add a consistent 5 minute clock skew accomodation #145

Merged
merged 2 commits into from
Mar 23, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions google/auth/_helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.

Expand Down
6 changes: 4 additions & 2 deletions google/auth/credentials.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down
17 changes: 10 additions & 7 deletions google/auth/jwt.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down Expand Up @@ -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))

Expand Down
6 changes: 4 additions & 2 deletions tests/compute_engine/test_credentials.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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 = [{
Expand All @@ -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 ==
Expand Down
3 changes: 2 additions & 1 deletion tests/oauth2/test_credentials.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
5 changes: 3 additions & 2 deletions tests/test_app_engine.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
import mock
import pytest

from google.auth import _helpers
from google.auth import app_engine


Expand Down Expand Up @@ -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
Expand All @@ -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

Expand Down
16 changes: 15 additions & 1 deletion tests/test_credentials.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@

import datetime

from google.auth import _helpers
from google.auth import credentials


Expand All @@ -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
Expand Down