Skip to content

Commit 1406f44

Browse files
Fix: Convert scopes set to list in Credentials.__init__ (#1898)
This PR addresses an issue where passing a `set` of scopes to `Credentials` causes `to_json` to fail and introduces inconsistent mutability behavior if handled via a property accessor. Changes: - Modified `google.oauth2.credentials.Credentials.__init__` to convert `scopes` to a `list` if it is a `set`. - Updated `google.oauth2.credentials.Credentials.scopes` property docstring to `Optional[Sequence[str]]`. - Added a regression test `test_init_with_set_scopes` in `tests/oauth2/test_credentials.py` to verify the fix, ensuring type conversion, object stability, and serialization support. - Updated `tests/test__oauth2client.py` to relax strict equality checks on `scopes` (comparing as `sets`) to accommodate the new behavior where internal storage is enforced as a `list`. This PR supercedes PR #1145 which potentially introduced a mutability inconsistency and failed to address a type correctness issue. This PR avoids both of those concerns. 1. Mutability Inconsistency: If `._scopes` is a `set`, the `scopes` property returns a new `list` every time it is accessed. * If the user modifies this list (e.g. `creds.scopes.append("new_scope")`), the modification is lost because the next access returns a fresh list **from the underlying set**. * If `_scopes` was originally a list (passed in `__init__`), `creds.scopes` returns the same list reference, so modifications are preserved. This inconsistent behavior between "set-initialized" and "list-initialized" credentials is a potential bug trap. 2. Type correctness: The scopes argument in `__init__` is documented as `Sequence[str]`. A set is not a `Sequence` (it's not indexable/ordered). While Python is lenient, it is better to normalize the input at the boundary (`__init__`) rather than in the accessor. Moving the conversion logic to `__init__` ensures: * _scopes is always a list (or `None`). * Access to .scopes always returns the stored list (consistent mutability). * `to_json` works correctly because it accesses `self.scopes` (or self._scopes). --- *PR created automatically by Jules for task [15605314498929918698](https://jules.google.com/task/15605314498929918698) started by @chalmerlowe* --------- Co-authored-by: google-labs-jules[bot] <161369871+google-labs-jules[bot]@users.noreply.github.com> Co-authored-by: Chalmer Lowe <[email protected]>
1 parent cccad82 commit 1406f44

File tree

5 files changed

+34
-6
lines changed

5 files changed

+34
-6
lines changed

google/auth/transport/_aiohttp_requests.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,7 @@ def data(self):
7979

8080
async def raw_content(self):
8181
if self._raw_content is None:
82-
self._raw_content = await self._response.read()
82+
self._raw_content = await self._response.content.read()
8383
return self._raw_content
8484

8585
async def content(self):

google/oauth2/credentials.py

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -141,7 +141,10 @@ def __init__(
141141
self.expiry = expiry
142142
self._refresh_token = refresh_token
143143
self._id_token = id_token
144-
self._scopes = scopes
144+
if scopes is not None and isinstance(scopes, set):
145+
self._scopes = list(scopes)
146+
else:
147+
self._scopes = scopes
145148
self._default_scopes = default_scopes
146149
self._granted_scopes = granted_scopes
147150
self._token_uri = token_uri
@@ -207,7 +210,7 @@ def refresh_token(self):
207210

208211
@property
209212
def scopes(self):
210-
"""Optional[str]: The OAuth 2.0 permission scopes."""
213+
"""Optional[Sequence[str]]: The OAuth 2.0 permission scopes."""
211214
return self._scopes
212215

213216
@property

tests/oauth2/test_credentials.py

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,31 @@ def make_credentials(cls):
5555
enable_reauth_refresh=True,
5656
)
5757

58+
def test_init_with_set_scopes(self):
59+
# Regression test for https://github.com/googleapis/google-auth-library-python/issues/1145
60+
scopes_set = {"a", "b"}
61+
creds = credentials.Credentials(token="token", scopes=scopes_set)
62+
63+
# Verify scopes are converted to a list
64+
assert isinstance(creds.scopes, list)
65+
assert set(creds.scopes) == scopes_set
66+
67+
# Verify internal storage is a list (ensure consistent mutability)
68+
assert isinstance(creds._scopes, list)
69+
70+
# Verify consistency (property returns the same object)
71+
assert creds.scopes is creds.scopes
72+
73+
# Verify modifications persist
74+
creds.scopes.append("c")
75+
assert "c" in creds.scopes
76+
assert len(creds.scopes) == 3
77+
78+
# Verify to_json works
79+
json_output = creds.to_json()
80+
data = json.loads(json_output)
81+
assert set(data["scopes"]) == {"a", "b", "c"}
82+
5883
def test_default_state(self):
5984
credentials = self.make_credentials()
6085
assert not credentials.valid

tests/test__oauth2client.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@ def test__convert_oauth2_credentials():
5656
assert new_credentials._client_id == old_credentials.client_id
5757
assert new_credentials._client_secret == old_credentials.client_secret
5858
assert new_credentials._token_uri == old_credentials.token_uri
59-
assert new_credentials.scopes == old_credentials.scopes
59+
assert set(new_credentials.scopes) == set(old_credentials.scopes)
6060

6161

6262
def test__convert_service_account_credentials():

tests_async/transport/test_aiohttp_requests.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ def test__is_compressed_not(self):
4040
@pytest.mark.asyncio
4141
async def test_raw_content(self):
4242
mock_response = mock.AsyncMock()
43-
mock_response.read.return_value = mock.sentinel.read
43+
mock_response.content.read.return_value = mock.sentinel.read
4444
combined_response = aiohttp_requests._CombinedResponse(response=mock_response)
4545
raw_content = await combined_response.raw_content()
4646
assert raw_content == mock.sentinel.read
@@ -53,7 +53,7 @@ async def test_raw_content(self):
5353
@pytest.mark.asyncio
5454
async def test_content(self):
5555
mock_response = mock.AsyncMock()
56-
mock_response.read.return_value = mock.sentinel.read
56+
mock_response.content.read.return_value = mock.sentinel.read
5757
combined_response = aiohttp_requests._CombinedResponse(response=mock_response)
5858
content = await combined_response.content()
5959
assert content == mock.sentinel.read

0 commit comments

Comments
 (0)