fix: device flow client_id, form-encoded auth body, expired token fallback - #314
Conversation
7b55319 to
34ae9cf
Compare
|
@manW13-be willing to fix the CI/Lint and tests? |
|
Probably related to Home Assistant issue: home-assistant/core#173222 |
|
Hi @wmalgadey, The CI/lint issue is now fixed — Could you take a look when you have a moment? Thanks! |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #314 +/- ##
==========================================
- Coverage 76.11% 75.71% -0.40%
==========================================
Files 37 37
Lines 2211 2224 +13
Branches 136 139 +3
==========================================
+ Hits 1683 1684 +1
- Misses 486 498 +12
Partials 42 42 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
sorry to bother you again, but I already did merge changes related to the home id :/ Could you rebase your changes and add the variants again? |
There was a problem hiding this comment.
Pull request overview
Fixes multiple OAuth2 device-flow and token-refresh issues when talking to login.tado.com, including correct form-encoded auth requests, a proper device-flow fallback when saved tokens are expired, and support for providing a per-instance client_id.
Changes:
- Add
client_idas an optional constructor parameter (threaded throughTado→Http) instead of relying on patching a module global. - Switch OAuth2 token/device-flow calls to
application/x-www-form-urlencodedrequest bodies (not query params + empty JSON). - Improve device verification URL generation and token-expiration fallback behavior.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
tests/test_http.py |
Updates the refresh-token request matcher to expect form-encoded body parameters. |
PyTado/interface/interface.py |
Exposes optional client_id on the public Tado interface and passes it through to Http. |
PyTado/http.py |
Implements client_id plumbing, device-flow fallback on refresh failure, form-encoded auth requests, and improved /me home-id extraction logic. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if response.get("homeIds"): | ||
| return int(response["homeIds"][0]) | ||
|
|
||
| raise TadoException(f"Cannot extract home ID from /me response: {response}") |
| token_response = self._session.request( | ||
| method="post", | ||
| url="https://login.tado.com/oauth2/token", | ||
| params={ | ||
| "client_id": CLIENT_ID_DEVICE, | ||
| "device_code": self._device_flow_data["device_code"], | ||
| "grant_type": "urn:ietf:params:oauth:grant-type:device_code", | ||
| }, | ||
| data=urlencode( | ||
| { |
| visit_url = ( | ||
| self._device_flow_data["verification_uri"] | ||
| + "?" | ||
| + urlencode({"user_code": self._user_code, "client_id": self._client_id}) | ||
| ) |
…lback - Add `client_id` parameter to `Http.__init__` and `Tado.__init__`, defaulting to `CLIENT_ID_DEVICE`. Removes the need to monkey-patch the module global for custom client IDs. - Fix verification URL to include `client_id` query param (required by login.tado.com — omitting it causes `missing_client_id` rejection). - Fix `_refresh_token`, `_login_device_flow`, and device polling to use form-encoded body (`application/x-www-form-urlencoded`) instead of query params + empty JSON body. The Tado auth server requires a form body; the previous format silently failed. - Add missing `else` branch in `__init__`: when a saved token exists but the refresh fails (expired), fall back to a new device flow instead of staying in `NOT_STARTED` indefinitely.
Token refresh now sends params as form-encoded body, not query string. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2bcafe3 to
bfae767
Compare
|
Done — rebased on master (including #331). 69 tests pass, black is clean. |
…device_ready() guard 1. _check_device_activation(): revert form-encoded body to query params. The Tado token endpoint requires params= for device_code polling — data=urlencode() returns authorization_pending regardless of auth state. 2. _get_id(): add home.id and homeIds[0] fallbacks after existing homes/homeId checks, and include the raw response in the TadoException message for easier debugging. 3. Http.__init__: wrap _device_ready() in try/except. If /me returns an empty body (e.g. Tado rate-limiting, 429) the constructor no longer crashes — it wipes the stale token and starts a fresh device flow. Note: _device_ready() in device_activation() (line 641) is intentionally left unwrapped so errors propagate to the caller. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Summary
Four bugs affecting the OAuth2 device flow and token refresh, confirmed against
login.tado.comin production.Bug 1 — Verification URL missing
client_iddevice_verification_url()returned a URL with onlyuser_code. Opening it in a browser causeslogin.tado.comto reject the session withmissing_client_id. Fixed: bothuser_codeandclient_idare now included.Bug 2 —
NOT_STARTEDwhen saved token is expiredWhen a token file existed but the refresh failed (expired token), there was no fallback — status stayed
NOT_STARTEDindefinitely. Fixed: added the missingelsebranch to start a new device flow in that case.Bug 3 —
client_idas constructor parameterThe only way to pass a custom
client_idwas to monkey-patchPyTado.const.CLIENT_ID_DEVICE, a module-level global shared across all instances. Fixed:Http.__init__andTado.__init__now acceptclient_id=None, defaulting toCLIENT_ID_DEVICEif not provided.Bug 4 — Auth requests sent as query string + empty JSON body
_refresh_token,_login_device_flow, and device polling sent parameters viaparams=(query string) withdata=json.dumps({})andContent-Type: application/json. The Tado auth server requiresapplication/x-www-form-urlencoded. Fixed on all three call sites.Test plan
user_codeandclient_idNOT_STARTEDTado(client_id="custom-id")works without patchingPyTado.constlogin.tado.com)