Skip to content

Commit

Permalink
fix: remove token_info call from token refresh path (#1595)
Browse files Browse the repository at this point in the history 
* fix: remove token_info call from token refresh path

* update secret

* 🦉 Updates from OwlBot post-processor

See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md

---------

Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com>
  • Loading branch information
@sai-sunder-s @gcf-owl-bot
sai-sunder-s and gcf-owl-bot[bot] authored Sep 16, 2024
1 parent 2910b6b commit afb9e5a
Show file tree
Hide file tree
Showing 3 changed files with 9 additions and 123 deletions.
30 changes: 0 additions & 30 deletions google/oauth2/credentials.py
View file
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):
"""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
Binary file not shown.
102 changes: 9 additions & 93 deletions tests/oauth2/test_credentials.py
View file
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

0 comments on commit afb9e5a

Please sign in to comment.