Skip to content

fix: propagate #1809 to oauth.Credentials #1875

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

Closed
mikeedjones wants to merge 3 commits into from
Closed

fix: propagate #1809 to oauth.Credentials #1875

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
a283a5b
fix: rename internal refresh token attribute to _refresh_token_val an…
mikeedjones Nov 23, 2025
bec5843
fix occasion when `_refresh_token` being treated like the old attribu…
mikeedjones Nov 23, 2025
0f52925
Merge branch 'main' into bugfix/propagate-change-of-refresh-token-to-…
mikeedjones Dec 9, 2025
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
2 changes: 1 addition & 1 deletion google/auth/external_account_authorized_user.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -343,7 +343,7 @@ def revoke(self, request):
request, self._refresh_token_val, "refresh_token", self._revoke_url
)
self.token = None
self._refresh_token = None
self._refresh_token_val = None
Copy link
Author

@mikeedjones mikeedjones Nov 23, 2025

Choose a reason for hiding this comment

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

I think this and the associated test was missed in #1809


@_helpers.copy_docstring(credentials.Credentials)
def get_cred_info(self):
Expand Down
6 changes: 3 additions & 3 deletions google/oauth2/_credentials_async.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ class Credentials(oauth2_credentials.Credentials):
@_helpers.copy_docstring(credentials.Credentials)
async def refresh(self, request):
if (
self._refresh_token is None
self._refresh_token_val is None
or self._token_uri is None
or self._client_id is None
or self._client_secret is None
Expand All @@ -70,7 +70,7 @@ async def refresh(self, request):
) = await reauth.refresh_grant(
request,
self._token_uri,
self._refresh_token,
self._refresh_token_val,
self._client_id,
self._client_secret,
scopes=self._scopes,
Expand All @@ -80,7 +80,7 @@ async def refresh(self, request):

self.token = access_token
self.expiry = expiry
self._refresh_token = refresh_token
self._refresh_token_val = refresh_token
self._id_token = grant_response.get("id_token")
self._rapt_token = rapt_token

Expand Down
17 changes: 10 additions & 7 deletions google/oauth2/credentials.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ def __init__(
super(Credentials, self).__init__()
self.token = token
self.expiry = expiry
self._refresh_token = refresh_token
self._refresh_token_val = refresh_token
self._id_token = id_token
self._scopes = scopes
self._default_scopes = default_scopes
Expand Down Expand Up @@ -178,7 +178,7 @@ def __setstate__(self, d):
all the attributes."""
self.token = d.get("token")
self.expiry = d.get("expiry")
self._refresh_token = d.get("_refresh_token")
self._refresh_token_val = d.get("_refresh_token_val")
self._id_token = d.get("_id_token")
self._scopes = d.get("_scopes")
self._default_scopes = d.get("_default_scopes")
Expand All @@ -203,7 +203,7 @@ def __setstate__(self, d):
@property
def refresh_token(self):
"""Optional[str]: The OAuth 2.0 refresh token."""
return self._refresh_token
return self._refresh_token_val

@property
def scopes(self):
Expand Down Expand Up @@ -350,6 +350,9 @@ def with_universe_domain(self, universe_domain):
def _metric_header_for_usage(self):
return metrics.CRED_TYPE_USER

def _refresh_token(self, request):
Copy link

@iostreamdoth iostreamdoth Dec 4, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

why not remove this and revert change on.

self._source_credentials._refresh_token(request) - > self._source_credentials.refresh(request)

I think using refresh() makes sense in place of _refresh_token

Copy link
Author

@mikeedjones mikeedjones Dec 5, 2025

Choose a reason for hiding this comment

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

_refresh_token has to be a method - _source_credentials._refresh_token. being called from google\auth\impersonated_credentials.py is causing #1850.

There is a bigger fix where "private" methods/attributes of source_credentials are not accessed, but that would be a much larger and more complicated change. This PR is only trying to align the interfaces of oauth2.credentials and auth.credentials since that was not done in #1809

Copy link
Author

@mikeedjones mikeedjones Dec 10, 2025

Choose a reason for hiding this comment

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

Apparently not! #1884

return self.refresh(request)

@_helpers.copy_docstring(credentials.Credentials)
def refresh(self, request):
if self._universe_domain != credentials.DEFAULT_UNIVERSE_DOMAIN:
Expand All @@ -368,7 +371,7 @@ def refresh(self, request):
# available. This is useful in general when tokens are obtained by calling
# some external process on demand. It is particularly useful for retrieving
# downscoped tokens from a token broker.
if self._refresh_token is None and self.refresh_handler:
if self._refresh_token_val is None and self.refresh_handler:
token, expiry = self.refresh_handler(request, scopes=scopes)
# Validate returned data.
if not isinstance(token, str):
Expand All @@ -389,7 +392,7 @@ def refresh(self, request):
return

if (
self._refresh_token is None
self._refresh_token_val is None
or self._token_uri is None
or self._client_id is None
or self._client_secret is None
Expand All @@ -409,7 +412,7 @@ def refresh(self, request):
) = reauth.refresh_grant(
request,
self._token_uri,
self._refresh_token,
self._refresh_token_val,
self._client_id,
self._client_secret,
scopes=scopes,
Expand All @@ -419,7 +422,7 @@ def refresh(self, request):

self.token = access_token
self.expiry = expiry
self._refresh_token = refresh_token
self._refresh_token_val = refresh_token
self._id_token = grant_response.get("id_token")
self._rapt_token = rapt_token

Expand Down
2 changes: 1 addition & 1 deletion tests/test__oauth2client.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ def test__convert_oauth2_credentials():
new_credentials = _oauth2client._convert_oauth2_credentials(old_credentials)

assert new_credentials.token == old_credentials.access_token
assert new_credentials._refresh_token == old_credentials.refresh_token
assert new_credentials._refresh_token_val == old_credentials.refresh_token
assert new_credentials._client_id == old_credentials.client_id
assert new_credentials._client_secret == old_credentials.client_secret
assert new_credentials._token_uri == old_credentials.token_uri
Expand Down
2 changes: 1 addition & 1 deletion tests/test_external_account_authorized_user.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -367,7 +367,7 @@ def test_revoke_auth_success(self):
),
)
assert creds.token is None
assert creds._refresh_token is None
assert creds._refresh_token_val is None

def test_revoke_without_revoke_url(self):
request = self.make_mock_request()
Expand Down
Loading