From 2f1200bef94b45f3f8cade7084afb7b58ffb672a Mon Sep 17 00:00:00 2001 From: Yegor Dolgopolov Date: Fri, 4 Sep 2026 21:21:13 -0700 Subject: [PATCH] fix(sdk): resend the OAuth resource on token refresh Refreshes went to the token endpoint without the RFC 8707 resource, so an authorization server that applies a default resource to resource-less requests could re-bind the refreshed REST token to another audience. OAuthCredential now carries the resource from the code exchange, persists it, and passes it to authlib's refresh_token. Credentials stored before this release have no resource and keep the previous behavior. --- CHANGELOG.md | 1 + .../discolike/src/discolike/_credentials.py | 4 ++ packages/discolike/src/discolike/_oauth.py | 28 +++++++++-- packages/discolike/tests/test_config.py | 27 ++++++++++- packages/discolike/tests/test_oauth.py | 47 +++++++++++++++++++ 5 files changed, 102 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 935ea40..eb18ec8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,7 @@ ## Unreleased +- SDK: OAuth refreshes now resend the RFC 8707 `resource` the token was issued for. `OAuthCredential` gains an optional `resource` field, filled in by `exchange_code` and persisted to the config file; credentials stored by earlier releases load with `resource=None` and refresh as before until the next `discolike auth login`. Without it, an authorization server configured with a default resource could re-bind a refreshed REST token to another audience. - SDK (behavior change, no code change): company `address.state` now comes back from the API as the subdivision name ("California", "Tokyo") instead of the ISO code ("CA", "13"). `CompanyAddress.state` is still `str | None` and needs no migration, but anything joining or grouping on that value as a code has to resolve it. The contact's own `state` is unchanged and stays a code. - SDK: state filters accept a code or a name, resolved server-side against the countries you selected. Discover/count still take one `country` value, but that value may be a region alias (`EU`, `APAC`, `DACH`) and the state resolves against every member. Contacts state filters accept multiple countries and drop a value they cannot resolve rather than erroring. - SDK: `MatchCompanyParams.state` works for any country with subdivisions, not just the US, and takes a code or a name. diff --git a/packages/discolike/src/discolike/_credentials.py b/packages/discolike/src/discolike/_credentials.py index c08644a..80e9521 100644 --- a/packages/discolike/src/discolike/_credentials.py +++ b/packages/discolike/src/discolike/_credentials.py @@ -18,6 +18,9 @@ class OAuthCredential: expires_at: float client_id: str token_endpoint: str + # RFC 8707 resource the token was issued for; resent on refresh so an authorization server with a + # default resource cannot re-bind the refreshed token. None for credentials stored before 0.3.3. + resource: str | None = None def expires_within(self, seconds: float, *, now: float | None = None) -> bool: current = time.time() if now is None else now @@ -31,6 +34,7 @@ def from_config(cls, data: dict[str, Any]) -> OAuthCredential: expires_at=float(data["expires_at"]), client_id=str(data["client_id"]), token_endpoint=str(data["token_endpoint"]), + resource=str(data["resource"]) if data.get("resource") else None, ) def to_config(self) -> dict[str, Any]: diff --git a/packages/discolike/src/discolike/_oauth.py b/packages/discolike/src/discolike/_oauth.py index 7289cad..9e39663 100644 --- a/packages/discolike/src/discolike/_oauth.py +++ b/packages/discolike/src/discolike/_oauth.py @@ -111,7 +111,12 @@ def _client_kwargs( def _credential_from_token( - token: dict[str, Any], *, client_id: str, token_endpoint: str, fallback_refresh_token: str | None + token: dict[str, Any], + *, + client_id: str, + token_endpoint: str, + resource: str | None, + fallback_refresh_token: str | None, ) -> OAuthCredential: # Exceptions raised here may be logged by SDK consumers; never attach live tokens to them. safe_payload = {key: value for key, value in token.items() if key not in TOKEN_KEYS} @@ -128,9 +133,18 @@ def _credential_from_token( expires_at=float(token["expires_at"]), client_id=client_id, token_endpoint=token_endpoint, + resource=resource, ) +def _refresh_kwargs(credential: OAuthCredential) -> dict[str, Any]: + """Resend the resource the token was issued for; credentials stored before 0.3.3 have none.""" + kwargs: dict[str, Any] = {"refresh_token": credential.refresh_token} + if credential.resource: + kwargs["resource"] = credential.resource + return kwargs + + def discover(base_url: str, *, client: httpx2.Client) -> AuthServerMetadata: payload = _payload(client.get(base_url.rstrip("/") + METADATA_PATH)) return AuthServerMetadata( @@ -178,7 +192,11 @@ def exchange_code( with TokenClient(**kwargs) as client: token = client.fetch_token(metadata.token_endpoint, code=code, code_verifier=code_verifier, resource=resource) return _credential_from_token( - token, client_id=client_id, token_endpoint=metadata.token_endpoint, fallback_refresh_token=None + token, + client_id=client_id, + token_endpoint=metadata.token_endpoint, + resource=resource, + fallback_refresh_token=None, ) @@ -188,11 +206,12 @@ def refresh( """Rotates the tokens; any failure means the session is gone and the user must log in again.""" try: with TokenClient(**_client_kwargs(client_id=credential.client_id, transport=transport)) as client: - token = client.refresh_token(credential.token_endpoint, refresh_token=credential.refresh_token) + token = client.refresh_token(credential.token_endpoint, **_refresh_kwargs(credential)) return _credential_from_token( token, client_id=credential.client_id, token_endpoint=credential.token_endpoint, + resource=credential.resource, fallback_refresh_token=credential.refresh_token, ) except AuthenticationError as exc: @@ -204,11 +223,12 @@ async def refresh_async( ) -> OAuthCredential: try: async with AsyncTokenClient(**_client_kwargs(client_id=credential.client_id, transport=transport)) as client: - token = await client.refresh_token(credential.token_endpoint, refresh_token=credential.refresh_token) + token = await client.refresh_token(credential.token_endpoint, **_refresh_kwargs(credential)) return _credential_from_token( token, client_id=credential.client_id, token_endpoint=credential.token_endpoint, + resource=credential.resource, fallback_refresh_token=credential.refresh_token, ) except AuthenticationError as exc: diff --git a/packages/discolike/tests/test_config.py b/packages/discolike/tests/test_config.py index 5daa832..7f28b50 100644 --- a/packages/discolike/tests/test_config.py +++ b/packages/discolike/tests/test_config.py @@ -60,7 +60,12 @@ def test_binary_garbage_config_returns_empty(isolated_config) -> None: def _oauth_credential() -> OAuthCredential: return OAuthCredential( - access_token="at", refresh_token="rt", expires_at=1.0, client_id="c", token_endpoint="https://t/token" + access_token="at", + refresh_token="rt", + expires_at=1.0, + client_id="c", + token_endpoint="https://t/token", + resource="https://api.example.com/v1", ) @@ -74,10 +79,30 @@ def test_save_and_load_oauth_credential(isolated_config) -> None: "expires_at": 1.0, "client_id": "c", "token_endpoint": "https://t/token", + "resource": "https://api.example.com/v1", } assert load_credential() == _oauth_credential() +def test_load_oauth_credential_saved_before_resource_was_stored(isolated_config) -> None: + """Config written by 0.3.x has no `resource`; it must still load, and refresh then omits the field.""" + save_config( + { + "auth_method": "oauth", + "oauth": { + "access_token": "at", + "refresh_token": "rt", + "expires_at": 1.0, + "client_id": "c", + "token_endpoint": "https://t/token", + }, + } + ) + loaded = load_credential() + assert isinstance(loaded, OAuthCredential) + assert loaded.resource is None + + def test_save_api_key_credential_keeps_legacy_shape(isolated_config) -> None: save_credential(ApiKeyCredential(api_key="dk-1")) assert load_config() == {"auth_method": "api_key", "api_key": "dk-1"} diff --git a/packages/discolike/tests/test_oauth.py b/packages/discolike/tests/test_oauth.py index d3ad1c0..efc4d6e 100644 --- a/packages/discolike/tests/test_oauth.py +++ b/packages/discolike/tests/test_oauth.py @@ -142,6 +142,7 @@ def handler(request: httpx2.Request) -> httpx2.Response: assert credential.refresh_token == "rt" assert credential.client_id == "client-abc" assert credential.token_endpoint == METADATA.token_endpoint + assert credential.resource == BASE_URL assert int(before) + 3600 <= credential.expires_at <= time.time() + 3600 @@ -293,3 +294,49 @@ def test_credential_config_roundtrip_and_expiry() -> None: assert OAuthCredential.from_config(credential.to_config()) == credential assert credential.expires_within(60, now=950.0) assert not credential.expires_within(60, now=900.0) + + +def test_refresh_sends_resource_and_keeps_it_on_rotated_credential() -> None: + """Without `resource` a server with a default resource may re-bind the refreshed token elsewhere.""" + credential = OAuthCredential( + access_token="old", + refresh_token="rt-old", + expires_at=0.0, + client_id="c", + token_endpoint=METADATA.token_endpoint, + resource=BASE_URL, + ) + seen: list[httpx2.Request] = [] + + def rotating(request: httpx2.Request) -> httpx2.Response: + seen.append(request) + return httpx2.Response(200, json={"access_token": "new", "refresh_token": "rt-new", "expires_in": 60}) + + rotated = refresh(credential, transport=transport_for(rotating)) + assert form(seen[0]) == { + "grant_type": "refresh_token", + "refresh_token": "rt-old", + "client_id": "c", + "resource": BASE_URL, + } + assert rotated.resource == BASE_URL + + +async def test_refresh_async_sends_resource_and_keeps_it_on_rotated_credential() -> None: + credential = OAuthCredential( + access_token="old", + refresh_token="rt-old", + expires_at=0.0, + client_id="c", + token_endpoint=METADATA.token_endpoint, + resource=BASE_URL, + ) + seen: list[httpx2.Request] = [] + + def rotating(request: httpx2.Request) -> httpx2.Response: + seen.append(request) + return httpx2.Response(200, json={"access_token": "new", "refresh_token": "rt-new", "expires_in": 60}) + + rotated = await refresh_async(credential, transport=transport_for(rotating)) + assert form(seen[0])["resource"] == BASE_URL + assert rotated.resource == BASE_URL