From ca0ee1d904476f25cba283c842248028128f8ec2 Mon Sep 17 00:00:00 2001 From: technorat2 Date: Mon, 13 Jul 2026 07:30:46 -0400 Subject: [PATCH 1/2] Retry once after an API 401 and bound Mobile Link HTTP timeouts Ported from pjordanandrsn/ha-generac#4 by @technorat2 (commits 7e3b067 "Improve authentication and entity reporting" and 40ae00b "Retry transient Mobile Link timeouts"), adapted to current upstream: - GeneracAuth.force_refresh(): mint a new access token when the resource server rejects the current one; get_endpoint() retries the request once with it. - Bound every auth and API request (45 s total, 10 s connect, 30 s read) and retry a timed-out API GET once. - Reauth/reconfigure replace stale credentials instead of merging them into the new entry data. - Coordinator treats OSError (timeouts, connection failures) as a transient UpdateFailed rather than an unexpected error. Adaptation notes: the login flow's dedicated session now carries the timeout once instead of per request; the discarded-key set names the credentials upstream's retired cookie/password flows actually stored (password, session_cookie). The resource server Authorization scheme stays Bearer. Co-authored-by: Jordan Anderson Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01K2RdXuPegztHA1uMG2fbtk --- custom_components/generac/api.py | 81 ++++++++++++++++-------- custom_components/generac/auth.py | 12 +++- custom_components/generac/config_flow.py | 15 ++++- custom_components/generac/coordinator.py | 5 ++ 4 files changed, 83 insertions(+), 30 deletions(-) diff --git a/custom_components/generac/api.py b/custom_components/generac/api.py index 604dcbc..ec69694 100644 --- a/custom_components/generac/api.py +++ b/custom_components/generac/api.py @@ -8,6 +8,7 @@ returning identical payloads for the endpoints we use. The iOS app uses `/api/v5`; we follow suit for futureproofing. """ +import asyncio import json import logging @@ -16,6 +17,7 @@ from .auth import GeneracAuth from .auth import InvalidGrantError +from .auth import REQUEST_TIMEOUT from .auth import USER_AGENT_API from .const import ALLOWED_DEVICES from .const import API_BASE @@ -23,7 +25,7 @@ from .models import ApparatusDetail from .models import Item -TIMEOUT = 10 +REQUEST_RETRY_DELAY = 1 _LOGGER: logging.Logger = logging.getLogger(__package__) @@ -131,29 +133,56 @@ async def get_endpoint(self, endpoint: str): } url = API_BASE + endpoint - try: - async with self._session.get(url, headers=headers) as response: - if response.status == 204: - return None - - if response.status == 401: - raise SessionExpiredException(f"API returned 401 for {endpoint}") - - if response.status != 200: - body = "" - try: - body = (await response.text())[:200] - except Exception: - pass - raise SessionExpiredException( - f"API returned status code {response.status} for " - f"{endpoint}: {body}" + for attempt in range(2): + try: + async with self._session.get( + url, headers=headers, timeout=REQUEST_TIMEOUT + ) as response: + if response.status == 204: + return None + + if response.status == 401 and attempt == 0: + _LOGGER.warning( + "Mobile Link rejected the access token for %s; " + "forcing one token refresh", + endpoint, + ) + try: + access_token = await self._auth.force_refresh() + except InvalidGrantError as ex: + raise InvalidCredentialsException(str(ex)) from ex + headers["Authorization"] = f"Bearer {access_token}" + continue + + if response.status == 401: + raise SessionExpiredException( + f"API returned 401 for {endpoint} after token refresh" + ) + + if response.status != 200: + body = "" + try: + body = (await response.text())[:200] + except Exception: + pass + raise SessionExpiredException( + f"API returned status code {response.status} for " + f"{endpoint}: {body}" + ) + + data = await response.json() + _LOGGER.debug("getEndpoint %s", json.dumps(data)) + return data + except SessionExpiredException: + raise + except asyncio.TimeoutError as ex: + if attempt == 0: + _LOGGER.warning( + "Mobile Link request timed out for %s; retrying once", + endpoint, ) - - data = await response.json() - _LOGGER.debug("getEndpoint %s", json.dumps(data)) - return data - except SessionExpiredException: - raise - except Exception as ex: - raise IOError(f"GET {url} failed: {type(ex).__name__}: {ex}") from ex + await asyncio.sleep(REQUEST_RETRY_DELAY) + continue + raise IOError(f"GET {url} timed out after retry") from ex + except Exception as ex: + raise IOError(f"GET {url} failed: {type(ex).__name__}: {ex}") from ex diff --git a/custom_components/generac/auth.py b/custom_components/generac/auth.py index 86bde14..473f2e6 100644 --- a/custom_components/generac/auth.py +++ b/custom_components/generac/auth.py @@ -40,6 +40,7 @@ from cryptography.hazmat.primitives.asymmetric.utils import decode_dss_signature _LOGGER = logging.getLogger(__name__) +REQUEST_TIMEOUT = aiohttp.ClientTimeout(total=45, connect=10, sock_read=30) AUTH0_DOMAIN = "auth.ecobee.com" AUTHORIZE_URL = f"https://{AUTH0_DOMAIN}/authorize" @@ -686,7 +687,7 @@ def __init__( # GeneracAuth.login's docstring for why this must stay isolated # from the shared session. We own it and close it in aclose(). self._login_session: Optional[aiohttp.ClientSession] = aiohttp.ClientSession( - cookie_jar=aiohttp.CookieJar(unsafe=True) + cookie_jar=aiohttp.CookieJar(unsafe=True), timeout=REQUEST_TIMEOUT ) # Populated when a code challenge pauses the flow. self._mfa_type: Optional[str] = None @@ -995,6 +996,13 @@ async def ensure_access_token(self) -> str: assert self._access_token is not None return self._access_token + async def force_refresh(self) -> str: + """Refresh once after the resource server rejects an access token.""" + async with self._refresh_lock: + await self._refresh() + assert self._access_token is not None + return self._access_token + async def _refresh(self) -> None: body = { "grant_type": "refresh_token", @@ -1011,7 +1019,7 @@ async def _post(nonce: str | None) -> tuple[int, dict, str | None]: "User-Agent": USER_AGENT_API, } async with self._session.post( - TOKEN_URL, json=body, headers=headers + TOKEN_URL, json=body, headers=headers, timeout=REQUEST_TIMEOUT ) as resp: text = await resp.text() try: diff --git a/custom_components/generac/config_flow.py b/custom_components/generac/config_flow.py index eda495e..128a392 100644 --- a/custom_components/generac/config_flow.py +++ b/custom_components/generac/config_flow.py @@ -56,6 +56,17 @@ "otp": "your authenticator app", "email": "email", } +# Credentials from the retired cookie / password flows. Reauth and +# reconfigure replace them instead of merging them into the new entry data. +_DISCARDED_AUTH_KEYS = frozenset({CONF_PASSWORD, "session_cookie"}) + + +def _replace_auth_data(existing: dict, new_data: dict) -> dict: + """Replace discarded credentials instead of retaining stale secrets.""" + retained = { + key: value for key, value in existing.items() if key not in _DISCARDED_AUTH_KEYS + } + return {**retained, **new_data} def _map_runtime_error(msg: str) -> str: @@ -260,7 +271,7 @@ async def _finish_reconfigure(self, entry_data: dict): # unload, surfacing as "failed to unload". self.hass.config_entries.async_update_entry( entry, - data={**entry.data, **entry_data}, + data=_replace_auth_data(entry.data, entry_data), options=new_options, ) self._reload_if_setup_failed(entry) @@ -325,7 +336,7 @@ async def _finish_reauth(self, entry_data: dict): # reload here would race the listener-driven one in the loaded case and # surface as "failed to unload". self.hass.config_entries.async_update_entry( - entry, data={**entry.data, **entry_data} + entry, data=_replace_auth_data(entry.data, entry_data) ) self._reload_if_setup_failed(entry) return self.async_abort(reason="reauth_successful") diff --git a/custom_components/generac/coordinator.py b/custom_components/generac/coordinator.py index f4949d3..52f7422 100644 --- a/custom_components/generac/coordinator.py +++ b/custom_components/generac/coordinator.py @@ -60,6 +60,11 @@ async def _async_update_data(self): _LOGGER.warning("Generac API session error: %s", ex) self.is_online = False raise UpdateFailed(f"API session error: {ex}") from ex + except OSError as ex: + # Network timeouts and connection failures are transient and do + # not indicate that the saved credentials need reauthentication. + _LOGGER.warning("Generac cloud request temporarily failed: %s", ex) + raise UpdateFailed(str(ex)) from ex except Exception as exception: _LOGGER.exception("Unexpected error refreshing Generac data") self.is_online = False From 088e5ac88fe66c6fa05921260c0939de265a6711 Mon Sep 17 00:00:00 2001 From: Jordan Anderson Date: Wed, 28 Oct 2026 19:20:02 +0000 Subject: [PATCH 2/2] Keep reauth reachable when the forced refresh is rejected Follow-up to the ported 401 retry. get_endpoint() called force_refresh() inside the transport `except Exception` block, so an InvalidGrantError raised by the token endpoint was converted to InvalidCredentialsException and then wrapped again as IOError. The coordinator treated that as a transient failure and Home Assistant never started reauthentication. - Move the forced refresh out of the transport try/except. A revoked refresh token now surfaces as InvalidCredentialsException (ConfigEntryAuthFailed). A token-endpoint outage keeps its own type (UpdateFailed). - force_refresh(rejected_token): callers that were rejected with the same token share one refresh instead of each spending the refresh token. - Retry transient connection failures (aiohttp.ClientConnectionError) as well as timeouts. Each request gets at most one transport retry and at most one refresh-and-retry. - The coordinator marks itself offline on transient OSError failures, as every other failure branch does. - Make the config flow's async_remove a synchronous callback that schedules aclose(). Home Assistant calls it without awaiting, so the async override never ran and a login session paused on the MFA form leaked when the flow was abandoned. Tests cover a valid token, an expired token refreshed, 401 followed by a successful refresh, a persistent 401, a revoked refresh token, a refresh outage, a timeout then success, repeated timeouts and connection failures, concurrent callers, refresh-token rotation, secret hygiene on failed refreshes, the bounded timeouts, stale legacy credentials being dropped on reauth/reconfigure, and an abandoned MFA flow closing its login session. One end-to-end test goes from an API 401 through the real GeneracAuth/GeneracApiClient to ConfigEntryAuthFailed. Co-authored-by: technorat2 Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01K2RdXuPegztHA1uMG2fbtk --- custom_components/generac/api.py | 122 +++++++++-------- custom_components/generac/auth.py | 11 +- custom_components/generac/config_flow.py | 19 ++- custom_components/generac/coordinator.py | 1 + tests/test_api.py | 138 +++++++++++++++++++- tests/test_auth.py | 159 +++++++++++++++++++++++ tests/test_config_flow.py | 86 ++++++++++++ tests/test_coordinator.py | 68 ++++++++++ 8 files changed, 536 insertions(+), 68 deletions(-) diff --git a/custom_components/generac/api.py b/custom_components/generac/api.py index ec69694..76cfe3e 100644 --- a/custom_components/generac/api.py +++ b/custom_components/generac/api.py @@ -114,6 +114,62 @@ async def get_endpoint(self, endpoint: str): except InvalidGrantError as ex: raise InvalidCredentialsException(str(ex)) from ex + url = API_BASE + endpoint + refreshed = False + retried = False + while True: + try: + status, data = await self._get(url, access_token) + except (asyncio.TimeoutError, aiohttp.ClientConnectionError) as ex: + # GET is idempotent: retry a transient transport failure once. + if retried: + raise IOError( + f"GET {url} failed after retry: {type(ex).__name__}: {ex}" + ) from ex + retried = True + _LOGGER.warning( + "Mobile Link request for %s failed (%s); retrying once", + endpoint, + type(ex).__name__, + ) + await asyncio.sleep(REQUEST_RETRY_DELAY) + continue + except Exception as ex: + raise IOError(f"GET {url} failed: {type(ex).__name__}: {ex}") from ex + + if status == 204: + return None + + if status == 401: + if refreshed: + raise SessionExpiredException( + f"API returned 401 for {endpoint} after token refresh" + ) + refreshed = True + _LOGGER.warning( + "Mobile Link rejected the access token for %s; " + "forcing one token refresh", + endpoint, + ) + # Outside the transport try/except above on purpose: an + # InvalidGrantError here must reach the coordinator as + # InvalidCredentialsException (-> reauth), not as IOError. + try: + access_token = await self._auth.force_refresh(access_token) + except InvalidGrantError as ex: + raise InvalidCredentialsException(str(ex)) from ex + continue + + if status != 200: + raise SessionExpiredException( + f"API returned status code {status} for {endpoint}: {data}" + ) + + _LOGGER.debug("getEndpoint %s", json.dumps(data)) + return data + + async def _get(self, url: str, access_token: str) -> tuple[int, object]: + """GET `url`; return (status, JSON body for 200, text excerpt otherwise).""" # Bearer is intentional here, even though this access token is # DPoP-*bound* (authorize sends dpop_jkt, so the token carries a # cnf.jkt claim). Probed 2026-06-25 (probe_rs_dpop.py): the MobileLink @@ -131,58 +187,16 @@ async def get_endpoint(self, endpoint: str): "Accept": "application/json", "User-Agent": USER_AGENT_API, } - - url = API_BASE + endpoint - for attempt in range(2): + async with self._session.get( + url, headers=headers, timeout=REQUEST_TIMEOUT + ) as response: + if response.status == 200: + return response.status, await response.json() + if response.status in (204, 401): + return response.status, None + body = "" try: - async with self._session.get( - url, headers=headers, timeout=REQUEST_TIMEOUT - ) as response: - if response.status == 204: - return None - - if response.status == 401 and attempt == 0: - _LOGGER.warning( - "Mobile Link rejected the access token for %s; " - "forcing one token refresh", - endpoint, - ) - try: - access_token = await self._auth.force_refresh() - except InvalidGrantError as ex: - raise InvalidCredentialsException(str(ex)) from ex - headers["Authorization"] = f"Bearer {access_token}" - continue - - if response.status == 401: - raise SessionExpiredException( - f"API returned 401 for {endpoint} after token refresh" - ) - - if response.status != 200: - body = "" - try: - body = (await response.text())[:200] - except Exception: - pass - raise SessionExpiredException( - f"API returned status code {response.status} for " - f"{endpoint}: {body}" - ) - - data = await response.json() - _LOGGER.debug("getEndpoint %s", json.dumps(data)) - return data - except SessionExpiredException: - raise - except asyncio.TimeoutError as ex: - if attempt == 0: - _LOGGER.warning( - "Mobile Link request timed out for %s; retrying once", - endpoint, - ) - await asyncio.sleep(REQUEST_RETRY_DELAY) - continue - raise IOError(f"GET {url} timed out after retry") from ex - except Exception as ex: - raise IOError(f"GET {url} failed: {type(ex).__name__}: {ex}") from ex + body = (await response.text())[:200] + except Exception: + pass + return response.status, body diff --git a/custom_components/generac/auth.py b/custom_components/generac/auth.py index 473f2e6..35d3c46 100644 --- a/custom_components/generac/auth.py +++ b/custom_components/generac/auth.py @@ -996,9 +996,16 @@ async def ensure_access_token(self) -> str: assert self._access_token is not None return self._access_token - async def force_refresh(self) -> str: - """Refresh once after the resource server rejects an access token.""" + async def force_refresh(self, rejected_token: str) -> str: + """Refresh once after the resource server rejects an access token. + + Callers pass the token that was rejected. If another caller already + replaced it while we waited for the lock, reuse that token instead of + spending a second refresh on the same rejection. + """ async with self._refresh_lock: + if self._access_token is not None and self._access_token != rejected_token: + return self._access_token await self._refresh() assert self._access_token is not None return self._access_token diff --git a/custom_components/generac/config_flow.py b/custom_components/generac/config_flow.py index 128a392..8b46f1e 100644 --- a/custom_components/generac/config_flow.py +++ b/custom_components/generac/config_flow.py @@ -413,13 +413,18 @@ async def _discard_login_flow(self) -> None: if flow is not None: await flow.aclose() - async def async_remove(self) -> None: - """Close the login session if the flow is abandoned mid-MFA.""" - if self._login_flow is not None: - try: - await self._login_flow.aclose() - except Exception: # noqa: BLE001 - _LOGGER.debug("Error closing login flow on remove", exc_info=True) + @callback + def async_remove(self) -> None: + """Close the login session if the flow is abandoned mid-MFA. + + HA calls this synchronously when the flow is removed, so an async + override would never run. Schedule the close instead; aclose() is + idempotent and never raises. + """ + flow = self._login_flow + self._login_flow = None + if flow is not None: + self.hass.async_create_task(flow.aclose()) @staticmethod @callback diff --git a/custom_components/generac/coordinator.py b/custom_components/generac/coordinator.py index 52f7422..9f8d483 100644 --- a/custom_components/generac/coordinator.py +++ b/custom_components/generac/coordinator.py @@ -64,6 +64,7 @@ async def _async_update_data(self): # Network timeouts and connection failures are transient and do # not indicate that the saved credentials need reauthentication. _LOGGER.warning("Generac cloud request temporarily failed: %s", ex) + self.is_online = False raise UpdateFailed(str(ex)) from ex except Exception as exception: _LOGGER.exception("Unexpected error refreshing Generac data") diff --git a/tests/test_api.py b/tests/test_api.py index d4183e3..8e1ea1c 100644 --- a/tests/test_api.py +++ b/tests/test_api.py @@ -4,11 +4,14 @@ from unittest.mock import AsyncMock from unittest.mock import MagicMock +import aiohttp import pytest +from custom_components.generac import api as api_module from custom_components.generac.api import GeneracApiClient from custom_components.generac.api import InvalidCredentialsException from custom_components.generac.api import SessionExpiredException from custom_components.generac.auth import InvalidGrantError +from custom_components.generac.auth import REQUEST_TIMEOUT from custom_components.generac.const import ALLOWED_DEVICES from custom_components.generac.const import API_BASE from custom_components.generac.const import DEVICE_TYPE_UNKNOWN @@ -30,6 +33,28 @@ def _acm(response): return cm +def _acm_raising(exc): + """An async-cm whose __aenter__ raises, like a timeout mid-request.""" + cm = MagicMock() + cm.__aenter__ = AsyncMock(side_effect=exc) + cm.__aexit__ = AsyncMock(return_value=None) + return cm + + +@pytest.fixture(autouse=True) +def no_retry_delay(monkeypatch): + """Keep the transient-failure retry from sleeping in tests.""" + monkeypatch.setattr(api_module, "REQUEST_RETRY_DELAY", 0) + + +def _ok(payload): + """A 200 JSON response.""" + response = AsyncMock(status=200) + response.headers = {"Content-Type": "application/json"} + response.json = AsyncMock(return_value=payload) + return response + + @pytest.fixture def mock_session(): """Fixture for aiohttp ClientSession.""" @@ -43,6 +68,7 @@ def mock_auth(): """Fixture for a GeneracAuth-like double that hands out a static AT.""" auth = MagicMock() auth.ensure_access_token = AsyncMock(return_value="fake-at") + auth.force_refresh = AsyncMock(return_value="fresh-at") return auth @@ -70,10 +96,12 @@ async def test_get_endpoint_uses_bearer_and_v5_base(client, mock_session, mock_a assert result == {"key": "value"} mock_auth.ensure_access_token.assert_awaited_once() + mock_auth.force_refresh.assert_not_awaited() args, kwargs = mock_session.get.call_args assert args[0] == f"{API_BASE}/test" assert kwargs["headers"]["Authorization"] == "Bearer fake-at" assert kwargs["headers"]["Accept"] == "application/json" + assert kwargs["timeout"] is REQUEST_TIMEOUT async def test_get_endpoint_no_content(client, mock_session): @@ -83,10 +111,62 @@ async def test_get_endpoint_no_content(client, mock_session): assert result is None -async def test_get_endpoint_session_expired_401(client, mock_session): - """401 from API raises SessionExpiredException.""" +async def test_get_endpoint_401_forces_one_refresh_and_retries( + client, mock_session, mock_auth +): + """A 401 forces one token refresh, then the request is retried with it.""" + mock_session.get.side_effect = [ + _acm(AsyncMock(status=401)), + _acm(_ok({"key": "value"})), + ] + + assert await client.get_endpoint("/test") == {"key": "value"} + + mock_auth.force_refresh.assert_awaited_once_with("fake-at") + assert mock_session.get.call_count == 2 + first, second = mock_session.get.call_args_list + assert first.kwargs["headers"]["Authorization"] == "Bearer fake-at" + assert second.kwargs["headers"]["Authorization"] == "Bearer fresh-at" + + +async def test_get_endpoint_session_expired_401(client, mock_session, mock_auth): + """A 401 that persists after the forced refresh raises SessionExpiredException.""" mock_session.get.return_value = _acm(AsyncMock(status=401)) - with pytest.raises(SessionExpiredException): + with pytest.raises(SessionExpiredException, match="after token refresh"): + await client.get_endpoint("/test") + mock_auth.force_refresh.assert_awaited_once() + assert mock_session.get.call_count == 2 + + +async def test_get_endpoint_401_invalid_grant_is_not_wrapped( + client, mock_session, mock_auth +): + """A rejected refresh token during the forced refresh must stay an auth error. + + Regression: the original patch called force_refresh() inside the + transport `except Exception` block, so this surfaced as IOError and the + coordinator never started reauth. + """ + mock_session.get.return_value = _acm(AsyncMock(status=401)) + mock_auth.force_refresh = AsyncMock(side_effect=InvalidGrantError("rt revoked")) + + with pytest.raises(InvalidCredentialsException) as exc_info: + await client.get_endpoint("/test") + assert not isinstance(exc_info.value, OSError) + assert isinstance(exc_info.value.__cause__, InvalidGrantError) + assert mock_session.get.call_count == 1 + + +async def test_get_endpoint_401_transient_refresh_failure_propagates( + client, mock_session, mock_auth +): + """A token-endpoint outage during the forced refresh keeps its own type.""" + mock_session.get.return_value = _acm(AsyncMock(status=401)) + mock_auth.force_refresh = AsyncMock( + side_effect=RuntimeError("token refresh failed: 503") + ) + + with pytest.raises(RuntimeError, match="503"): await client.get_endpoint("/test") @@ -98,10 +178,58 @@ async def test_get_endpoint_server_error(client, mock_session): async def test_get_endpoint_io_error_on_timeout(client, mock_session): - """Network timeout becomes IOError.""" + """A timeout that repeats on the single retry becomes IOError.""" mock_session.get.side_effect = asyncio.TimeoutError - with pytest.raises(IOError): + with pytest.raises(IOError, match="after retry"): + await client.get_endpoint("/test") + assert mock_session.get.call_count == 2 + + +async def test_get_endpoint_timeout_then_success(client, mock_session, mock_auth): + """One timeout is retried once and the second response is returned.""" + mock_session.get.side_effect = [ + _acm_raising(asyncio.TimeoutError()), + _acm(_ok({"key": "value"})), + ] + assert await client.get_endpoint("/test") == {"key": "value"} + assert mock_session.get.call_count == 2 + mock_auth.force_refresh.assert_not_awaited() + + +@pytest.mark.parametrize( + "error", + [ + aiohttp.ServerDisconnectedError(), + aiohttp.ClientConnectionError("connection reset"), + ], +) +async def test_get_endpoint_connection_error_then_success(client, mock_session, error): + """A transient connection failure is retried once.""" + mock_session.get.side_effect = [error, _acm(_ok([1]))] + assert await client.get_endpoint("/test") == [1] + assert mock_session.get.call_count == 2 + + +async def test_get_endpoint_repeated_connection_error(client, mock_session): + """Connection failures on both attempts become IOError.""" + mock_session.get.side_effect = aiohttp.ServerDisconnectedError() + with pytest.raises(IOError, match="ServerDisconnectedError"): await client.get_endpoint("/test") + assert mock_session.get.call_count == 2 + + +async def test_get_endpoint_timeout_and_401_each_retry_once( + client, mock_session, mock_auth +): + """The transport retry and the 401 refresh retry are separate, single budgets.""" + mock_session.get.side_effect = [ + asyncio.TimeoutError(), + _acm(AsyncMock(status=401)), + _acm(_ok({"key": "value"})), + ] + assert await client.get_endpoint("/test") == {"key": "value"} + assert mock_session.get.call_count == 3 + mock_auth.force_refresh.assert_awaited_once_with("fake-at") async def test_get_endpoint_json_decode_error(client, mock_session): diff --git a/tests/test_auth.py b/tests/test_auth.py index 7aeec3a..a59b360 100644 --- a/tests/test_auth.py +++ b/tests/test_auth.py @@ -1,7 +1,10 @@ """Tests for GeneracAuth refresh + Auth0 ULP form-parser error handling.""" from __future__ import annotations +import asyncio import json +import logging +import time from unittest.mock import AsyncMock from unittest.mock import MagicMock @@ -11,10 +14,12 @@ from custom_components.generac.auth import _post_login_form from custom_components.generac.auth import DPoPKey from custom_components.generac.auth import GeneracAuth +from custom_components.generac.auth import GeneracLoginFlow from custom_components.generac.auth import IDENTIFIER_URL from custom_components.generac.auth import InvalidCredentialsError from custom_components.generac.auth import InvalidGrantError from custom_components.generac.auth import PASSWORD_URL +from custom_components.generac.auth import REQUEST_TIMEOUT def _acm(resp): @@ -338,3 +343,157 @@ async def test_identifier_and_password_steps_strip_email_whitespace(): assert "username=user%40example.com" in pw_body # Password whitespace is preserved verbatim assert "password=+pass+" in pw_body + + +# --------------------------------------------------------------------------- +# Forced refresh after a resource-server 401, timeouts, secret hygiene +# --------------------------------------------------------------------------- + + +def _ok_token(access_token: str, expires_in: int = 3600): + """A 200 token response that yields to the loop, so callers interleave.""" + resp = _token_resp(200, {"access_token": access_token, "expires_in": expires_in}) + body = resp.text.return_value + + async def _text(): + await asyncio.sleep(0) + return body + + resp.text = _text + return _acm(resp) + + +@pytest.mark.asyncio +async def test_ensure_access_token_reuses_valid_token(): + """A non-expired access token is returned without a refresh.""" + auth = _make_auth() + auth._access_token = "at-VALID" + auth._access_token_exp = time.time() + 3600 + auth._session.post = MagicMock() + + assert await auth.ensure_access_token() == "at-VALID" + auth._session.post.assert_not_called() + + +@pytest.mark.asyncio +async def test_ensure_access_token_refreshes_expired_token(): + """An expired access token is refreshed with the bounded timeout.""" + auth = _make_auth() + auth._access_token = "at-EXPIRED" + auth._access_token_exp = time.time() - 1 + auth._session.post = MagicMock(return_value=_ok_token("at-NEW")) + + assert await auth.ensure_access_token() == "at-NEW" + auth._session.post.assert_called_once() + assert auth._session.post.call_args.kwargs["timeout"] is REQUEST_TIMEOUT + + +@pytest.mark.asyncio +async def test_concurrent_callers_share_one_expiry_refresh(): + """Concurrent callers that see an expired token trigger one refresh.""" + auth = _make_auth() + auth._session.post = MagicMock(return_value=_ok_token("at-NEW")) + + tokens = await asyncio.gather(*(auth.ensure_access_token() for _ in range(5))) + + assert tokens == ["at-NEW"] * 5 + auth._session.post.assert_called_once() + + +@pytest.mark.asyncio +async def test_force_refresh_refreshes_rejected_token(): + """force_refresh() mints a new token even if the old one looks unexpired.""" + auth = _make_auth() + auth._access_token = "at-REJECTED" + auth._access_token_exp = time.time() + 3600 + auth._session.post = MagicMock(return_value=_ok_token("at-NEW")) + + assert await auth.force_refresh("at-REJECTED") == "at-NEW" + auth._session.post.assert_called_once() + + +@pytest.mark.asyncio +async def test_concurrent_force_refresh_shares_one_refresh(): + """Callers rejected with the same token share one refresh (no RT stampede).""" + auth = _make_auth() + auth._access_token = "at-REJECTED" + auth._access_token_exp = time.time() + 3600 + auth._session.post = MagicMock(return_value=_ok_token("at-NEW")) + + tokens = await asyncio.gather( + *(auth.force_refresh("at-REJECTED") for _ in range(5)) + ) + + assert tokens == ["at-NEW"] * 5 + auth._session.post.assert_called_once() + + +@pytest.mark.asyncio +async def test_force_refresh_rotation_persists_once(): + """A rotated refresh token from a forced refresh is persisted exactly once.""" + auth = _make_auth(refresh_token="rt-OLD") + auth._access_token = "at-REJECTED" + auth._session.post = MagicMock( + return_value=_acm( + _token_resp( + 200, + { + "access_token": "at-NEW", + "expires_in": 3600, + "refresh_token": "rt-NEW", + }, + ) + ) + ) + persist_cb = AsyncMock() + auth.set_refresh_token_persist_callback(persist_cb) + + await asyncio.gather(*(auth.force_refresh("at-REJECTED") for _ in range(3))) + + persist_cb.assert_awaited_once_with("rt-NEW") + assert auth.refresh_token == "rt-NEW" + + +@pytest.mark.asyncio +async def test_force_refresh_invalid_grant_raises(): + """A revoked refresh token during a forced refresh raises InvalidGrantError.""" + auth = _make_auth(refresh_token="rt-REVOKED") + auth._access_token = "at-REJECTED" + auth._session.post = MagicMock( + return_value=_acm(_token_resp(403, {"error": "invalid_grant"})) + ) + + with pytest.raises(InvalidGrantError): + await auth.force_refresh("at-REJECTED") + + +@pytest.mark.asyncio +@pytest.mark.parametrize( + ("status", "body", "expected"), + [ + (403, {"error": "invalid_grant"}, InvalidGrantError), + (500, {"error": "server_error"}, RuntimeError), + ], +) +async def test_failed_refresh_does_not_expose_secrets(caplog, status, body, expected): + """Neither the exception nor the logs carry the refresh token or DPoP key.""" + caplog.set_level(logging.DEBUG) + auth = _make_auth(refresh_token="rt-SECRET-VALUE") + auth._session.post = MagicMock(return_value=_acm(_token_resp(status, body))) + + with pytest.raises(expected) as exc_info: + await auth.ensure_access_token() + + pem_body = auth.pem_str.splitlines()[1] + for text in (str(exc_info.value), caplog.text): + assert "rt-SECRET-VALUE" not in text + assert pem_body not in text + + +async def test_login_session_has_bounded_timeout(): + """The isolated login session bounds every universal-login request.""" + flow = GeneracLoginFlow(MagicMock(), "user@example.com", "pw") + try: + assert flow._login_session.timeout is REQUEST_TIMEOUT + finally: + await flow.aclose() diff --git a/tests/test_config_flow.py b/tests/test_config_flow.py index e343028..d213c36 100644 --- a/tests/test_config_flow.py +++ b/tests/test_config_flow.py @@ -6,6 +6,7 @@ import pytest from custom_components.generac.auth import DPoPKey from custom_components.generac.auth import InvalidCredentialsError +from custom_components.generac.auth import MfaRequiredError from custom_components.generac.const import CONF_DPOP_PEM from custom_components.generac.const import CONF_PASSWORD from custom_components.generac.const import CONF_REFRESH_TOKEN @@ -487,3 +488,88 @@ async def test_reconfigure_does_not_double_reload_when_loaded( await hass.async_block_till_done() mock_reload.assert_not_called() + + +@pytest.mark.parametrize("source", ["reauth", "reconfigure"]) +async def test_credential_update_drops_stale_legacy_secrets( + hass: HomeAssistant, source: str +) -> None: + """Reauth/reconfigure replace credentials instead of merging old secrets in. + + Entries created by the retired cookie/password flows still carry + `password` and `session_cookie`; a plain dict merge kept them forever. + """ + entry = MockConfigEntry( + domain=DOMAIN, + unique_id="user@example.com", + data={ + CONF_USERNAME: "user@example.com", + CONF_PASSWORD: "legacy-password", + "session_cookie": "legacy-cookie", + CONF_REFRESH_TOKEN: "stale-rt", + CONF_DPOP_PEM: DPoPKey.generate().to_pem_str(), + }, + options={}, + ) + entry.add_to_hass(hass) + + # Only reauth takes the entry data at init. A reconfigure flow treats init + # data as submitted user input and would log in with the legacy password. + init_kwargs = {"data": entry.data} if source == "reauth" else {} + result = await hass.config_entries.flow.async_init( + DOMAIN, + context={"source": source, "entry_id": entry.entry_id}, + **init_kwargs, + ) + assert result["type"] == "form" + user_input = {CONF_PASSWORD: "new-pw"} + if source == "reconfigure": + user_input[CONF_USERNAME] = "user@example.com" + + new_auth = _mock_auth(refresh_token="fresh-rt") + with patch( + "custom_components.generac.config_flow.GeneracAuth.login", + AsyncMock(return_value=new_auth), + ), patch("custom_components.generac.async_setup_entry", return_value=True): + result2 = await hass.config_entries.flow.async_configure( + result["flow_id"], user_input + ) + await hass.async_block_till_done() + + assert result2["type"] == "abort" + assert dict(entry.data) == { + CONF_USERNAME: "user@example.com", + CONF_REFRESH_TOKEN: "fresh-rt", + CONF_DPOP_PEM: new_auth.pem_str, + } + + +async def test_abandoned_mfa_flow_closes_login_session(hass: HomeAssistant) -> None: + """Aborting a flow paused on the MFA form closes the open login session. + + HA calls FlowHandler.async_remove synchronously; an async override was + never awaited, so the session leaked. + """ + result = await hass.config_entries.flow.async_init( + DOMAIN, context={"source": config_entries.SOURCE_USER} + ) + login_flow = MagicMock() + login_flow.mfa_type = "sms" + login_flow.aclose = AsyncMock() + with patch( + "custom_components.generac.config_flow.GeneracAuth.login", + AsyncMock( + side_effect=MfaRequiredError(login_flow, "sms", "https://x", "state") + ), + ): + result2 = await hass.config_entries.flow.async_configure( + result["flow_id"], + {CONF_USERNAME: "user@example.com", CONF_PASSWORD: "hunter2"}, + ) + assert result2["type"] == "form" + assert result2["step_id"] == "mfa" + + hass.config_entries.flow.async_abort(result["flow_id"]) + await hass.async_block_till_done() + + login_flow.aclose.assert_awaited_once() diff --git a/tests/test_coordinator.py b/tests/test_coordinator.py index 1ae6571..b88c0bb 100644 --- a/tests/test_coordinator.py +++ b/tests/test_coordinator.py @@ -1,10 +1,15 @@ """Test the Generac data update coordinator.""" +import json +import time from datetime import timedelta from unittest.mock import AsyncMock from unittest.mock import MagicMock import pytest +from custom_components.generac.api import GeneracApiClient from custom_components.generac.api import InvalidCredentialsException +from custom_components.generac.auth import DPoPKey +from custom_components.generac.auth import GeneracAuth from custom_components.generac.auth import InvalidGrantError from custom_components.generac.const import CONF_SCAN_INTERVAL from custom_components.generac.const import DEFAULT_SCAN_INTERVAL @@ -87,3 +92,66 @@ async def test_coordinator_invalid_grant_raises_auth_failed(hass): with pytest.raises(ConfigEntryAuthFailed): await coordinator._async_update_data() assert not coordinator.is_online + + +@pytest.mark.parametrize( + "error", + [IOError("GET failed after retry"), TimeoutError("token endpoint timed out")], +) +async def test_coordinator_transient_network_error_is_update_failed(hass, error): + """Timeouts/connection failures retry next poll; they never start reauth.""" + config_entry = MagicMock() + config_entry.options = {} + client = MagicMock() + client.async_get_data = AsyncMock(side_effect=error) + coordinator = GeneracDataUpdateCoordinator(hass, client, config_entry) + with pytest.raises(UpdateFailed): + await coordinator._async_update_data() + assert not coordinator.is_online + + +def _acm(response): + cm = MagicMock() + cm.__aenter__ = AsyncMock(return_value=response) + cm.__aexit__ = AsyncMock(return_value=None) + return cm + + +def _token_resp(status: int, body: dict): + resp = MagicMock() + resp.status = status + resp.text = AsyncMock(return_value=json.dumps(body)) + resp.headers = MagicMock() + resp.headers.get = MagicMock(return_value=None) + return resp + + +async def test_api_401_with_revoked_refresh_token_starts_reauth(hass): + """End to end: API 401 -> forced refresh -> invalid_grant -> reauth. + + Uses the real GeneracAuth and GeneracApiClient over a mocked session so + the exception travels through every layer the coordinator depends on. + """ + session = MagicMock() + session.get = MagicMock(return_value=_acm(AsyncMock(status=401))) + session.post = MagicMock( + return_value=_acm( + _token_resp( + 403, + {"error": "invalid_grant", "error_description": "revoked"}, + ) + ) + ) + auth = GeneracAuth(session, "rt-REVOKED", DPoPKey.generate()) + auth._access_token = "at-REJECTED" + auth._access_token_exp = time.time() + 3600 + config_entry = MagicMock() + config_entry.options = {} + coordinator = GeneracDataUpdateCoordinator( + hass, GeneracApiClient(session, auth), config_entry + ) + + with pytest.raises(ConfigEntryAuthFailed): + await coordinator._async_update_data() + assert not coordinator.is_online + session.post.assert_called_once()