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.

Sign up for GitHub

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

Jump to bottom

fix: remove token_info call from token refresh path #1595

Merged
merged 3 commits into from
Sep 16, 2024
Merged

fix: remove token_info call from token refresh path #1595

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
598082c
fix: remove token_info call from token refresh path
sai-sunder-s Sep 16, 2024
2a38a51
update secret
sai-sunder-s Sep 16, 2024
26af882
🦉 Updates from OwlBot post-processor
gcf-owl-bot[bot] Sep 16, 2024
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
30 changes: 0 additions & 30 deletions google/oauth2/credentials.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@
"""

from datetime import datetime
import http.client as http_client
import io
import json
import logging
Expand Down Expand Up @@ -351,33 +350,6 @@ def with_universe_domain(self, universe_domain):
def _metric_header_for_usage(self):
return metrics.CRED_TYPE_USER

def _set_account_from_access_token(self, request):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you alternatively perform this in the account property in case _account is not already set?

"""Obtain the account from token info endpoint and set the account field.

Args:
request (google.auth.transport.Request): A callable used to make
HTTP requests.
"""
# We only set the account if it's not yet set.
if self._account:
return

if not self.token:
return

# Make request to token info endpoint with the access token.
# If the token is invalid, it returns 400 error code.
# If the token is valid, it returns 200 status with a JSON. The account
# is the "email" field of the JSON.
token_info_url = "{}?access_token={}".format(
_GOOGLE_OAUTH2_TOKEN_INFO_ENDPOINT, self.token
)
response = request(method="GET", url=token_info_url)

if response.status == http_client.OK:
response_data = json.loads(response.data.decode("utf-8"))
self._account = response_data.get("email")

@_helpers.copy_docstring(credentials.Credentials)
def refresh(self, request):
if self._universe_domain != credentials.DEFAULT_UNIVERSE_DOMAIN:
Expand Down Expand Up @@ -414,7 +386,6 @@ def refresh(self, request):
)
self.token = token
self.expiry = expiry
self._set_account_from_access_token(request)
return

if (
Expand Down Expand Up @@ -451,7 +422,6 @@ def refresh(self, request):
self._refresh_token = refresh_token
self._id_token = grant_response.get("id_token")
self._rapt_token = rapt_token
self._set_account_from_access_token(request)

if scopes and "scope" in grant_response:
requested_scopes = frozenset(scopes)
Expand Down
Binary file modified system_tests/secrets.tar.enc
View file
Open in desktop
Binary file not shown.
102 changes: 9 additions & 93 deletions tests/oauth2/test_credentials.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,48 +71,6 @@ def test_default_state(self):
assert credentials.rapt_token == self.RAPT_TOKEN
assert credentials.refresh_handler is None

def test__set_account_from_access_token_no_token(self):
credentials = self.make_credentials()
assert not credentials.token
assert not credentials.account

credentials._set_account_from_access_token(mock.Mock())
assert not credentials.account

def test__set_account_from_access_token_account_already_set(self):
credentials = self.make_credentials()
credentials.token = "fake-token"
credentials._account = "fake-account"

credentials._set_account_from_access_token(mock.Mock())
assert credentials.account == "fake-account"

def test__set_account_from_access_token_error_response(self):
credentials = self.make_credentials()
credentials.token = "fake-token"
assert not credentials.account

mock_response = mock.Mock()
mock_response.status = 400
mock_request = mock.Mock(return_value=mock_response)
credentials._set_account_from_access_token(mock_request)
assert not credentials.account

def test__set_account_from_access_token_success(self):
credentials = self.make_credentials()
credentials.token = "fake-token"
assert not credentials.account

mock_response = mock.Mock()
mock_response.status = 200
mock_response.data = (
b'{"aud": "aud", "sub": "sub", "scope": "scope", "email": "fake-account"}'
)

mock_request = mock.Mock(return_value=mock_response)
credentials._set_account_from_access_token(mock_request)
assert credentials.account == "fake-account"

def test_get_cred_info(self):
credentials = self.make_credentials()
credentials._account = "fake-account"
Expand Down Expand Up @@ -205,15 +163,12 @@ def test_refresh_with_non_default_universe_domain(self):
"refresh is only supported in the default googleapis.com universe domain"
)

@mock.patch.object(
credentials.Credentials, "_set_account_from_access_token", autospec=True
)
@mock.patch("google.oauth2.reauth.refresh_grant", autospec=True)
@mock.patch(
"google.auth._helpers.utcnow",
return_value=datetime.datetime.min + _helpers.REFRESH_THRESHOLD,
)
def test_refresh_success(self, unused_utcnow, refresh_grant, set_account):
def test_refresh_success(self, unused_utcnow, refresh_grant):
token = "token"
new_rapt_token = "new_rapt_token"
expiry = _helpers.utcnow() + datetime.timedelta(seconds=500)
Expand Down Expand Up @@ -259,8 +214,6 @@ def test_refresh_success(self, unused_utcnow, refresh_grant, set_account):
# expired)
assert credentials.valid

set_account.assert_called_once()

def test_refresh_no_refresh_token(self):
request = mock.create_autospec(transport.Request)
credentials_ = credentials.Credentials(token=None, refresh_token=None)
Expand All @@ -270,16 +223,13 @@ def test_refresh_no_refresh_token(self):

request.assert_not_called()

@mock.patch.object(
credentials.Credentials, "_set_account_from_access_token", autospec=True
)
@mock.patch("google.oauth2.reauth.refresh_grant", autospec=True)
@mock.patch(
"google.auth._helpers.utcnow",
return_value=datetime.datetime.min + _helpers.REFRESH_THRESHOLD,
)
def test_refresh_with_refresh_token_and_refresh_handler(
self, unused_utcnow, refresh_grant, set_account
self, unused_utcnow, refresh_grant
):
token = "token"
new_rapt_token = "new_rapt_token"
Expand Down Expand Up @@ -339,15 +289,8 @@ def test_refresh_with_refresh_token_and_refresh_handler(
# higher priority.
refresh_handler.assert_not_called()

set_account.assert_called_once()

@mock.patch.object(
credentials.Credentials, "_set_account_from_access_token", autospec=True
)
@mock.patch("google.auth._helpers.utcnow", return_value=datetime.datetime.min)
def test_refresh_with_refresh_handler_success_scopes(
self, unused_utcnow, set_account
):
def test_refresh_with_refresh_handler_success_scopes(self, unused_utcnow):
expected_expiry = datetime.datetime.min + datetime.timedelta(seconds=2800)
refresh_handler = mock.Mock(return_value=("ACCESS_TOKEN", expected_expiry))
scopes = ["email", "profile"]
Expand All @@ -371,17 +314,11 @@ def test_refresh_with_refresh_handler_success_scopes(
assert creds.expiry == expected_expiry
assert creds.valid
assert not creds.expired
set_account.assert_called_once()
# Confirm refresh handler called with the expected arguments.
refresh_handler.assert_called_with(request, scopes=scopes)

@mock.patch.object(
credentials.Credentials, "_set_account_from_access_token", autospec=True
)
@mock.patch("google.auth._helpers.utcnow", return_value=datetime.datetime.min)
def test_refresh_with_refresh_handler_success_default_scopes(
self, unused_utcnow, set_account
):
def test_refresh_with_refresh_handler_success_default_scopes(self, unused_utcnow):
expected_expiry = datetime.datetime.min + datetime.timedelta(seconds=2800)
original_refresh_handler = mock.Mock(
return_value=("UNUSED_TOKEN", expected_expiry)
Expand Down Expand Up @@ -409,7 +346,6 @@ def test_refresh_with_refresh_handler_success_default_scopes(
assert creds.expiry == expected_expiry
assert creds.valid
assert not creds.expired
set_account.assert_called_once()
# default_scopes should be used since no developer provided scopes
# are provided.
refresh_handler.assert_called_with(request, scopes=default_scopes)
Expand Down Expand Up @@ -503,16 +439,13 @@ def test_refresh_with_refresh_handler_expired_token(self, unused_utcnow):
# Confirm refresh handler called with the expected arguments.
refresh_handler.assert_called_with(request, scopes=scopes)

@mock.patch.object(
credentials.Credentials, "_set_account_from_access_token", autospec=True
)
@mock.patch("google.oauth2.reauth.refresh_grant", autospec=True)
@mock.patch(
"google.auth._helpers.utcnow",
return_value=datetime.datetime.min + _helpers.REFRESH_THRESHOLD,
)
def test_credentials_with_scopes_requested_refresh_success(
self, unused_utcnow, refresh_grant, set_account
self, unused_utcnow, refresh_grant
):
scopes = ["email", "profile"]
default_scopes = ["https://www.googleapis.com/auth/cloud-platform"]
Expand Down Expand Up @@ -568,22 +501,18 @@ def test_credentials_with_scopes_requested_refresh_success(
assert creds.has_scopes(scopes)
assert creds.rapt_token == new_rapt_token
assert creds.granted_scopes == scopes
set_account.assert_called_once()

# Check that the credentials are valid (have a token and are not
# expired.)
assert creds.valid

@mock.patch.object(
credentials.Credentials, "_set_account_from_access_token", autospec=True
)
@mock.patch("google.oauth2.reauth.refresh_grant", autospec=True)
@mock.patch(
"google.auth._helpers.utcnow",
return_value=datetime.datetime.min + _helpers.REFRESH_THRESHOLD,
)
def test_credentials_with_only_default_scopes_requested(
self, unused_utcnow, refresh_grant, set_account
self, unused_utcnow, refresh_grant
):
default_scopes = ["email", "profile"]
token = "token"
Expand Down Expand Up @@ -637,22 +566,18 @@ def test_credentials_with_only_default_scopes_requested(
assert creds.has_scopes(default_scopes)
assert creds.rapt_token == new_rapt_token
assert creds.granted_scopes == default_scopes
set_account.assert_called_once()

# Check that the credentials are valid (have a token and are not
# expired.)
assert creds.valid

@mock.patch.object(
credentials.Credentials, "_set_account_from_access_token", autospec=True
)
@mock.patch("google.oauth2.reauth.refresh_grant", autospec=True)
@mock.patch(
"google.auth._helpers.utcnow",
return_value=datetime.datetime.min + _helpers.REFRESH_THRESHOLD,
)
def test_credentials_with_scopes_returned_refresh_success(
self, unused_utcnow, refresh_grant, set_account
self, unused_utcnow, refresh_grant
):
scopes = ["email", "profile"]
token = "token"
Expand Down Expand Up @@ -706,22 +631,18 @@ def test_credentials_with_scopes_returned_refresh_success(
assert creds.has_scopes(scopes)
assert creds.rapt_token == new_rapt_token
assert creds.granted_scopes == scopes
set_account.assert_called_once()

# Check that the credentials are valid (have a token and are not
# expired.)
assert creds.valid

@mock.patch.object(
credentials.Credentials, "_set_account_from_access_token", autospec=True
)
@mock.patch("google.oauth2.reauth.refresh_grant", autospec=True)
@mock.patch(
"google.auth._helpers.utcnow",
return_value=datetime.datetime.min + _helpers.REFRESH_THRESHOLD,
)
def test_credentials_with_only_default_scopes_requested_different_granted_scopes(
self, unused_utcnow, refresh_grant, set_account
self, unused_utcnow, refresh_grant
):
default_scopes = ["email", "profile"]
token = "token"
Expand Down Expand Up @@ -775,22 +696,18 @@ def test_credentials_with_only_default_scopes_requested_different_granted_scopes
assert creds.has_scopes(default_scopes)
assert creds.rapt_token == new_rapt_token
assert creds.granted_scopes == ["email"]
set_account.assert_called_once()

# Check that the credentials are valid (have a token and are not
# expired.)
assert creds.valid

@mock.patch.object(
credentials.Credentials, "_set_account_from_access_token", autospec=True
)
@mock.patch("google.oauth2.reauth.refresh_grant", autospec=True)
@mock.patch(
"google.auth._helpers.utcnow",
return_value=datetime.datetime.min + _helpers.REFRESH_THRESHOLD,
)
def test_credentials_with_scopes_refresh_different_granted_scopes(
self, unused_utcnow, refresh_grant, set_account
self, unused_utcnow, refresh_grant
):
scopes = ["email", "profile"]
scopes_returned = ["email"]
Expand Down Expand Up @@ -848,7 +765,6 @@ def test_credentials_with_scopes_refresh_different_granted_scopes(
assert creds.has_scopes(scopes)
assert creds.rapt_token == new_rapt_token
assert creds.granted_scopes == scopes_returned
set_account.assert_called_once()

# Check that the credentials are valid (have a token and are not
# expired.)
Expand Down