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

fix: rename CLOCK_SKEW and separate client/server user case #863

Merged
merged 2 commits into from Sep 10, 2021
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
7 changes: 5 additions & 2 deletions google/auth/_helpers.py
Expand Up @@ -20,8 +20,11 @@
import urllib


CLOCK_SKEW_SECS = 60 # 60 seconds
CLOCK_SKEW = datetime.timedelta(seconds=CLOCK_SKEW_SECS)
# Token server doesn't provide a new a token when doing refresh unless the
# token is expiring within 30 seconds, so refresh threshold should not be
# more than 30 seconds. Otherwise auth lib will send tons of refresh requests
# until 30 seconds before the expiration, and cause a spike of CPU usage.
REFRESH_THRESHOLD = datetime.timedelta(seconds=20)


def copy_docstring(source_class):
Expand Down
2 changes: 1 addition & 1 deletion google/auth/credentials.py
Expand Up @@ -62,7 +62,7 @@ def expired(self):

# Remove 10 seconds 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
skewed_expiry = self.expiry - _helpers.REFRESH_THRESHOLD
return _helpers.utcnow() >= skewed_expiry

@property
Expand Down
14 changes: 9 additions & 5 deletions google/auth/jwt.py
Expand Up @@ -167,12 +167,14 @@ def decode_header(token):
return header


def _verify_iat_and_exp(payload):
def _verify_iat_and_exp(payload, clock_skew_in_seconds=0):
"""Verifies the ``iat`` (Issued At) and ``exp`` (Expires) claims in a token
payload.

Args:
payload (Mapping[str, str]): The JWT payload.
clock_skew_in_seconds (int): The clock skew used for `iat` and `exp`
validation.

Raises:
ValueError: if any checks failed.
Expand All @@ -188,7 +190,7 @@ def _verify_iat_and_exp(payload):
iat = payload["iat"]
# Err on the side of accepting a token that is slightly early to account
# for clock skew.
earliest = iat - _helpers.CLOCK_SKEW_SECS
earliest = iat - clock_skew_in_seconds
if now < earliest:
raise ValueError(
"Token used too early, {} < {}. Check that your computer's clock is set correctly.".format(
Expand All @@ -200,12 +202,12 @@ def _verify_iat_and_exp(payload):
exp = payload["exp"]
# 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
latest = exp + clock_skew_in_seconds
if latest < now:
raise ValueError("Token expired, {} < {}".format(latest, now))


def decode(token, certs=None, verify=True, audience=None):
def decode(token, certs=None, verify=True, audience=None, clock_skew_in_seconds=0):
"""Decode and verify a JWT.

Args:
Expand All @@ -221,6 +223,8 @@ def decode(token, certs=None, verify=True, audience=None):
audience (str or list): The audience claim, 'aud', that this JWT should
contain. Or a list of audience claims. If None then the JWT's 'aud'
parameter is not verified.
clock_skew_in_seconds (int): The clock skew used for `iat` and `exp`
validation.

Returns:
Mapping[str, str]: The deserialized JSON payload in the JWT.
Expand Down Expand Up @@ -271,7 +275,7 @@ def decode(token, certs=None, verify=True, audience=None):
raise ValueError("Could not verify token signature.")

# Verify the issued at and created times in the payload.
_verify_iat_and_exp(payload)
_verify_iat_and_exp(payload, clock_skew_in_seconds)

# Check audience.
if audience is not None:
Expand Down
4 changes: 2 additions & 2 deletions google/oauth2/credentials.py
Expand Up @@ -270,7 +270,7 @@ def refresh(self, request):
raise exceptions.RefreshError(
"The refresh_handler returned expiry is not a datetime object."
)
if _helpers.utcnow() >= expiry - _helpers.CLOCK_SKEW:
if _helpers.utcnow() >= expiry - _helpers.REFRESH_THRESHOLD:
raise exceptions.RefreshError(
"The credentials returned by the refresh_handler are "
"already expired."
Expand Down Expand Up @@ -359,7 +359,7 @@ def from_authorized_user_info(cls, info, scopes=None):
expiry.rstrip("Z").split(".")[0], "%Y-%m-%dT%H:%M:%S"
)
else:
expiry = _helpers.utcnow() - _helpers.CLOCK_SKEW
expiry = _helpers.utcnow() - _helpers.REFRESH_THRESHOLD

# process scopes, which needs to be a seq
if scopes is None and "scopes" in info:
Expand Down
4 changes: 2 additions & 2 deletions tests/compute_engine/test_credentials.py
Expand Up @@ -64,7 +64,7 @@ def test_default_state(self):

@mock.patch(
"google.auth._helpers.utcnow",
return_value=datetime.datetime.min + _helpers.CLOCK_SKEW,
return_value=datetime.datetime.min + _helpers.REFRESH_THRESHOLD,
)
@mock.patch("google.auth.compute_engine._metadata.get", autospec=True)
def test_refresh_success(self, get, utcnow):
Expand Down Expand Up @@ -98,7 +98,7 @@ def test_refresh_success(self, get, utcnow):

@mock.patch(
"google.auth._helpers.utcnow",
return_value=datetime.datetime.min + _helpers.CLOCK_SKEW,
return_value=datetime.datetime.min + _helpers.REFRESH_THRESHOLD,
)
@mock.patch("google.auth.compute_engine._metadata.get", autospec=True)
def test_refresh_success_with_scopes(self, get, utcnow):
Expand Down
14 changes: 7 additions & 7 deletions tests/oauth2/test_credentials.py
Expand Up @@ -115,7 +115,7 @@ def test_invalid_refresh_handler(self):
@mock.patch("google.oauth2.reauth.refresh_grant", autospec=True)
@mock.patch(
"google.auth._helpers.utcnow",
return_value=datetime.datetime.min + _helpers.CLOCK_SKEW,
return_value=datetime.datetime.min + _helpers.REFRESH_THRESHOLD,
)
def test_refresh_success(self, unused_utcnow, refresh_grant):
token = "token"
Expand Down Expand Up @@ -175,7 +175,7 @@ def test_refresh_no_refresh_token(self):
@mock.patch("google.oauth2.reauth.refresh_grant", autospec=True)
@mock.patch(
"google.auth._helpers.utcnow",
return_value=datetime.datetime.min + _helpers.CLOCK_SKEW,
return_value=datetime.datetime.min + _helpers.REFRESH_THRESHOLD,
)
def test_refresh_with_refresh_token_and_refresh_handler(
self, unused_utcnow, refresh_grant
Expand Down Expand Up @@ -361,7 +361,7 @@ def test_refresh_with_refresh_handler_invalid_expiry(self):

@mock.patch("google.auth._helpers.utcnow", return_value=datetime.datetime.min)
def test_refresh_with_refresh_handler_expired_token(self, unused_utcnow):
expected_expiry = datetime.datetime.min + _helpers.CLOCK_SKEW
expected_expiry = datetime.datetime.min + _helpers.REFRESH_THRESHOLD
# Simulate refresh handler returns an expired token.
refresh_handler = mock.Mock(return_value=("TOKEN", expected_expiry))
scopes = ["email", "profile"]
Expand Down Expand Up @@ -391,7 +391,7 @@ def test_refresh_with_refresh_handler_expired_token(self, unused_utcnow):
@mock.patch("google.oauth2.reauth.refresh_grant", autospec=True)
@mock.patch(
"google.auth._helpers.utcnow",
return_value=datetime.datetime.min + _helpers.CLOCK_SKEW,
return_value=datetime.datetime.min + _helpers.REFRESH_THRESHOLD,
)
def test_credentials_with_scopes_requested_refresh_success(
self, unused_utcnow, refresh_grant
Expand Down Expand Up @@ -457,7 +457,7 @@ def test_credentials_with_scopes_requested_refresh_success(
@mock.patch("google.oauth2.reauth.refresh_grant", autospec=True)
@mock.patch(
"google.auth._helpers.utcnow",
return_value=datetime.datetime.min + _helpers.CLOCK_SKEW,
return_value=datetime.datetime.min + _helpers.REFRESH_THRESHOLD,
)
def test_credentials_with_only_default_scopes_requested(
self, unused_utcnow, refresh_grant
Expand Down Expand Up @@ -521,7 +521,7 @@ def test_credentials_with_only_default_scopes_requested(
@mock.patch("google.oauth2.reauth.refresh_grant", autospec=True)
@mock.patch(
"google.auth._helpers.utcnow",
return_value=datetime.datetime.min + _helpers.CLOCK_SKEW,
return_value=datetime.datetime.min + _helpers.REFRESH_THRESHOLD,
)
def test_credentials_with_scopes_returned_refresh_success(
self, unused_utcnow, refresh_grant
Expand Down Expand Up @@ -588,7 +588,7 @@ def test_credentials_with_scopes_returned_refresh_success(
@mock.patch("google.oauth2.reauth.refresh_grant", autospec=True)
@mock.patch(
"google.auth._helpers.utcnow",
return_value=datetime.datetime.min + _helpers.CLOCK_SKEW,
return_value=datetime.datetime.min + _helpers.REFRESH_THRESHOLD,
)
def test_credentials_with_scopes_refresh_failure_raises_refresh_error(
self, unused_utcnow, refresh_grant
Expand Down
4 changes: 3 additions & 1 deletion tests/test_credentials.py
Expand Up @@ -46,7 +46,9 @@ def test_expired_and_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() + _helpers.CLOCK_SKEW + datetime.timedelta(seconds=1)
datetime.datetime.utcnow()
+ _helpers.REFRESH_THRESHOLD
+ datetime.timedelta(seconds=1)
)

assert credentials.valid
Expand Down
4 changes: 3 additions & 1 deletion tests/test_downscoped.py
Expand Up @@ -669,7 +669,9 @@ def test_before_request_expired(self, utcnow):
# Set the expiration to one second more than now plus the clock skew
# accommodation. These credentials should be valid.
credentials.expiry = (
datetime.datetime.min + _helpers.CLOCK_SKEW + datetime.timedelta(seconds=1)
datetime.datetime.min
+ _helpers.REFRESH_THRESHOLD
+ datetime.timedelta(seconds=1)
)

assert credentials.valid
Expand Down
8 changes: 6 additions & 2 deletions tests/test_external_account.py
Expand Up @@ -976,7 +976,9 @@ def test_before_request_expired(self, utcnow):
# Set the expiration to one second more than now plus the clock skew
# accomodation. These credentials should be valid.
credentials.expiry = (
datetime.datetime.min + _helpers.CLOCK_SKEW + datetime.timedelta(seconds=1)
datetime.datetime.min
+ _helpers.REFRESH_THRESHOLD
+ datetime.timedelta(seconds=1)
)

assert credentials.valid
Expand Down Expand Up @@ -1027,7 +1029,9 @@ def test_before_request_impersonation_expired(self, utcnow):
# Set the expiration to one second more than now plus the clock skew
# accomodation. These credentials should be valid.
credentials.expiry = (
datetime.datetime.min + _helpers.CLOCK_SKEW + datetime.timedelta(seconds=1)
datetime.datetime.min
+ _helpers.REFRESH_THRESHOLD
+ datetime.timedelta(seconds=1)
)

assert credentials.valid
Expand Down
2 changes: 1 addition & 1 deletion tests/test_iam.py
Expand Up @@ -45,7 +45,7 @@ def __init__(self):
super(CredentialsImpl, self).__init__()
self.token = "token"
# Force refresh
self.expiry = datetime.datetime.min + _helpers.CLOCK_SKEW
self.expiry = datetime.datetime.min + _helpers.REFRESH_THRESHOLD

def refresh(self, request):
pass
Expand Down
6 changes: 3 additions & 3 deletions tests/test_impersonated_credentials.py
Expand Up @@ -211,11 +211,11 @@ def test_refresh_source_credentials(self, time_skew):
credentials = self.make_credentials(lifetime=None)

# Source credentials is refreshed only if it is expired within
# _helpers.CLOCK_SKEW from now. We add a time_skew to the expiry, so
# _helpers.REFRESH_THRESHOLD from now. We add a time_skew to the expiry, so
# source credentials is refreshed only if time_skew <= 0.
credentials._source_credentials.expiry = (
_helpers.utcnow()
+ _helpers.CLOCK_SKEW
+ _helpers.REFRESH_THRESHOLD
+ datetime.timedelta(seconds=time_skew)
)
credentials._source_credentials.token = "Token"
Expand All @@ -238,7 +238,7 @@ def test_refresh_source_credentials(self, time_skew):
assert not credentials.expired

# Source credentials is refreshed only if it is expired within
# _helpers.CLOCK_SKEW
# _helpers.REFRESH_THRESHOLD
if time_skew > 0:
source_cred_refresh.assert_not_called()
else:
Expand Down
34 changes: 32 additions & 2 deletions tests/test_jwt.py
Expand Up @@ -197,7 +197,7 @@ def test_decode_bad_token_too_early(token_factory):
}
)
with pytest.raises(ValueError) as excinfo:
jwt.decode(token, PUBLIC_CERT_BYTES)
jwt.decode(token, PUBLIC_CERT_BYTES, clock_skew_in_seconds=59)
assert excinfo.match(r"Token used too early")


Expand All @@ -210,10 +210,40 @@ def test_decode_bad_token_expired(token_factory):
}
)
with pytest.raises(ValueError) as excinfo:
jwt.decode(token, PUBLIC_CERT_BYTES)
jwt.decode(token, PUBLIC_CERT_BYTES, clock_skew_in_seconds=59)
assert excinfo.match(r"Token expired")


def test_decode_success_with_no_clock_skew(token_factory):
token = token_factory(
claims={
"exp": _helpers.datetime_to_secs(
_helpers.utcnow() + datetime.timedelta(seconds=1)
),
"iat": _helpers.datetime_to_secs(
_helpers.utcnow() - datetime.timedelta(seconds=1)
),
}
)

jwt.decode(token, PUBLIC_CERT_BYTES)


def test_decode_success_with_custom_clock_skew(token_factory):
token = token_factory(
claims={
"exp": _helpers.datetime_to_secs(
_helpers.utcnow() + datetime.timedelta(seconds=2)
),
"iat": _helpers.datetime_to_secs(
_helpers.utcnow() - datetime.timedelta(seconds=2)
),
}
)

jwt.decode(token, PUBLIC_CERT_BYTES, clock_skew_in_seconds=1)


def test_decode_bad_token_wrong_audience(token_factory):
token = token_factory()
audience = "audience2@example.com"
Expand Down
2 changes: 1 addition & 1 deletion tests/transport/test_grpc.py
Expand Up @@ -80,7 +80,7 @@ def test_call_no_refresh(self):

def test_call_refresh(self):
credentials = CredentialsStub()
credentials.expiry = datetime.datetime.min + _helpers.CLOCK_SKEW
credentials.expiry = datetime.datetime.min + _helpers.REFRESH_THRESHOLD
request = mock.create_autospec(transport.Request)

plugin = google.auth.transport.grpc.AuthMetadataPlugin(credentials, request)
Expand Down
8 changes: 4 additions & 4 deletions tests_async/oauth2/test_credentials_async.py
Expand Up @@ -62,7 +62,7 @@ def test_default_state(self):
@mock.patch("google.oauth2._reauth_async.refresh_grant", autospec=True)
@mock.patch(
"google.auth._helpers.utcnow",
return_value=datetime.datetime.min + _helpers.CLOCK_SKEW,
return_value=datetime.datetime.min + _helpers.REFRESH_THRESHOLD,
)
@pytest.mark.asyncio
async def test_refresh_success(self, unused_utcnow, refresh_grant):
Expand Down Expand Up @@ -124,7 +124,7 @@ async def test_refresh_no_refresh_token(self):
@mock.patch("google.oauth2._reauth_async.refresh_grant", autospec=True)
@mock.patch(
"google.auth._helpers.utcnow",
return_value=datetime.datetime.min + _helpers.CLOCK_SKEW,
return_value=datetime.datetime.min + _helpers.REFRESH_THRESHOLD,
)
@pytest.mark.asyncio
async def test_credentials_with_scopes_requested_refresh_success(
Expand Down Expand Up @@ -188,7 +188,7 @@ async def test_credentials_with_scopes_requested_refresh_success(
@mock.patch("google.oauth2._reauth_async.refresh_grant", autospec=True)
@mock.patch(
"google.auth._helpers.utcnow",
return_value=datetime.datetime.min + _helpers.CLOCK_SKEW,
return_value=datetime.datetime.min + _helpers.REFRESH_THRESHOLD,
)
@pytest.mark.asyncio
async def test_credentials_with_scopes_returned_refresh_success(
Expand Down Expand Up @@ -251,7 +251,7 @@ async def test_credentials_with_scopes_returned_refresh_success(
@mock.patch("google.oauth2._reauth_async.refresh_grant", autospec=True)
@mock.patch(
"google.auth._helpers.utcnow",
return_value=datetime.datetime.min + _helpers.CLOCK_SKEW,
return_value=datetime.datetime.min + _helpers.REFRESH_THRESHOLD,
)
@pytest.mark.asyncio
async def test_credentials_with_scopes_refresh_failure_raises_refresh_error(
Expand Down
4 changes: 3 additions & 1 deletion tests_async/test_credentials_async.py
Expand Up @@ -46,7 +46,9 @@ def test_expired_and_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() + _helpers.CLOCK_SKEW + datetime.timedelta(seconds=1)
datetime.datetime.utcnow()
+ _helpers.REFRESH_THRESHOLD
+ datetime.timedelta(seconds=1)
)

assert credentials.valid
Expand Down