Add a timeout during OAuth token exchange and additional debug logging (#85911)

This commit is contained in:
Allen Porter 2023-01-16 10:52:43 -08:00 committed by GitHub
parent c5dedb7a79
commit 1afb4897a8
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 81 additions and 6 deletions

View File

@ -211,6 +211,7 @@ class NestFlowHandler(
async def async_oauth_create_entry(self, data: dict[str, Any]) -> FlowResult: async def async_oauth_create_entry(self, data: dict[str, Any]) -> FlowResult:
"""Complete OAuth setup and finish pubsub or finish.""" """Complete OAuth setup and finish pubsub or finish."""
_LOGGER.debug("Finishing post-oauth configuration")
assert self.config_mode != ConfigMode.LEGACY, "Step only supported for SDM API" assert self.config_mode != ConfigMode.LEGACY, "Step only supported for SDM API"
self._data.update(data) self._data.update(data)
if self.source == SOURCE_REAUTH: if self.source == SOURCE_REAUTH:
@ -459,6 +460,7 @@ class NestFlowHandler(
async def async_step_finish(self, data: dict[str, Any] | None = None) -> FlowResult: async def async_step_finish(self, data: dict[str, Any] | None = None) -> FlowResult:
"""Create an entry for the SDM flow.""" """Create an entry for the SDM flow."""
_LOGGER.debug("Creating/updating configuration entry")
assert self.config_mode != ConfigMode.LEGACY, "Step only supported for SDM API" assert self.config_mode != ConfigMode.LEGACY, "Step only supported for SDM API"
# Update existing config entry when in the reauth flow. # Update existing config entry when in the reauth flow.
if entry := self._async_reauth_entry(): if entry := self._async_reauth_entry():

View File

@ -41,6 +41,9 @@ MY_AUTH_CALLBACK_PATH = "https://my.home-assistant.io/redirect/oauth"
CLOCK_OUT_OF_SYNC_MAX_SEC = 20 CLOCK_OUT_OF_SYNC_MAX_SEC = 20
OAUTH_AUTHORIZE_URL_TIMEOUT_SEC = 30
OAUTH_TOKEN_TIMEOUT_SEC = 30
class AbstractOAuth2Implementation(ABC): class AbstractOAuth2Implementation(ABC):
"""Base class to abstract OAuth2 authentication.""" """Base class to abstract OAuth2 authentication."""
@ -194,6 +197,7 @@ class LocalOAuth2Implementation(AbstractOAuth2Implementation):
if self.client_secret is not None: if self.client_secret is not None:
data["client_secret"] = self.client_secret data["client_secret"] = self.client_secret
_LOGGER.debug("Sending token request to %s", self.token_url)
resp = await session.post(self.token_url, data=data) resp = await session.post(self.token_url, data=data)
if resp.status >= 400 and _LOGGER.isEnabledFor(logging.DEBUG): if resp.status >= 400 and _LOGGER.isEnabledFor(logging.DEBUG):
body = await resp.text() body = await resp.text()
@ -283,9 +287,10 @@ class AbstractOAuth2FlowHandler(config_entries.ConfigFlow, metaclass=ABCMeta):
return self.async_external_step_done(next_step_id=next_step) return self.async_external_step_done(next_step_id=next_step)
try: try:
async with async_timeout.timeout(10): async with async_timeout.timeout(OAUTH_AUTHORIZE_URL_TIMEOUT_SEC):
url = await self.async_generate_authorize_url() url = await self.async_generate_authorize_url()
except asyncio.TimeoutError: except asyncio.TimeoutError as err:
_LOGGER.error("Timeout generating authorize url: %s", err)
return self.async_abort(reason="authorize_url_timeout") return self.async_abort(reason="authorize_url_timeout")
except NoURLAvailableError: except NoURLAvailableError:
return self.async_abort( return self.async_abort(
@ -303,7 +308,17 @@ class AbstractOAuth2FlowHandler(config_entries.ConfigFlow, metaclass=ABCMeta):
self, user_input: dict[str, Any] | None = None self, user_input: dict[str, Any] | None = None
) -> FlowResult: ) -> FlowResult:
"""Create config entry from external data.""" """Create config entry from external data."""
token = await self.flow_impl.async_resolve_external_data(self.external_data) _LOGGER.debug("Creating config entry from external data")
try:
async with async_timeout.timeout(OAUTH_TOKEN_TIMEOUT_SEC):
token = await self.flow_impl.async_resolve_external_data(
self.external_data
)
except asyncio.TimeoutError as err:
_LOGGER.error("Timeout resolving OAuth token: %s", err)
return self.async_abort(reason="oauth2_timeout")
# Force int for non-compliant oauth2 providers # Force int for non-compliant oauth2 providers
try: try:
token["expires_in"] = int(token["expires_in"]) token["expires_in"] = int(token["expires_in"])
@ -436,7 +451,7 @@ class OAuth2AuthorizeCallbackView(http.HomeAssistantView):
await hass.config_entries.flow.async_configure( await hass.config_entries.flow.async_configure(
flow_id=state["flow_id"], user_input=user_input flow_id=state["flow_id"], user_input=user_input
) )
_LOGGER.debug("Resumed OAuth configuration flow")
return web.Response( return web.Response(
headers={"content-type": "text/html"}, headers={"content-type": "text/html"},
text="<script>window.close()</script>", text="<script>window.close()</script>",

View File

@ -71,6 +71,7 @@
"no_devices_found": "No devices found on the network", "no_devices_found": "No devices found on the network",
"webhook_not_internet_accessible": "Your Home Assistant instance needs to be accessible from the internet to receive webhook messages.", "webhook_not_internet_accessible": "Your Home Assistant instance needs to be accessible from the internet to receive webhook messages.",
"oauth2_error": "Received invalid token data.", "oauth2_error": "Received invalid token data.",
"oauth2_timeout": "Timeout resolving OAuth token.",
"oauth2_missing_configuration": "The component is not configured. Please follow the documentation.", "oauth2_missing_configuration": "The component is not configured. Please follow the documentation.",
"oauth2_missing_credentials": "The integration requires application credentials.", "oauth2_missing_credentials": "The integration requires application credentials.",
"oauth2_authorize_url_timeout": "Timeout generating authorize URL.", "oauth2_authorize_url_timeout": "Timeout generating authorize URL.",

View File

@ -134,8 +134,9 @@ async def test_abort_if_authorization_timeout(
flow = flow_handler() flow = flow_handler()
flow.hass = hass flow.hass = hass
with patch.object( with patch(
local_impl, "async_generate_authorize_url", side_effect=asyncio.TimeoutError "homeassistant.helpers.config_entry_oauth2_flow.async_timeout.timeout",
side_effect=asyncio.TimeoutError,
): ):
result = await flow.async_step_user() result = await flow.async_step_user()
@ -278,6 +279,62 @@ async def test_abort_if_oauth_rejected(
assert result["description_placeholders"] == {"error": "access_denied"} assert result["description_placeholders"] == {"error": "access_denied"}
async def test_abort_on_oauth_timeout_error(
hass,
flow_handler,
local_impl,
hass_client_no_auth,
aioclient_mock,
current_request_with_host,
):
"""Check timeout during oauth token exchange."""
flow_handler.async_register_implementation(hass, local_impl)
config_entry_oauth2_flow.async_register_implementation(
hass, TEST_DOMAIN, MockOAuth2Implementation()
)
result = await hass.config_entries.flow.async_init(
TEST_DOMAIN, context={"source": config_entries.SOURCE_USER}
)
assert result["type"] == data_entry_flow.FlowResultType.FORM
assert result["step_id"] == "pick_implementation"
# Pick implementation
result = await hass.config_entries.flow.async_configure(
result["flow_id"], user_input={"implementation": TEST_DOMAIN}
)
state = config_entry_oauth2_flow._encode_jwt(
hass,
{
"flow_id": result["flow_id"],
"redirect_uri": "https://example.com/auth/external/callback",
},
)
assert result["type"] == data_entry_flow.FlowResultType.EXTERNAL_STEP
assert result["url"] == (
f"{AUTHORIZE_URL}?response_type=code&client_id={CLIENT_ID}"
"&redirect_uri=https://example.com/auth/external/callback"
f"&state={state}&scope=read+write"
)
client = await hass_client_no_auth()
resp = await client.get(f"/auth/external/callback?code=abcd&state={state}")
assert resp.status == 200
assert resp.headers["content-type"] == "text/html; charset=utf-8"
with patch(
"homeassistant.helpers.config_entry_oauth2_flow.async_timeout.timeout",
side_effect=asyncio.TimeoutError,
):
result = await hass.config_entries.flow.async_configure(result["flow_id"])
assert result["type"] == data_entry_flow.FlowResultType.ABORT
assert result["reason"] == "oauth2_timeout"
async def test_step_discovery(hass, flow_handler, local_impl): async def test_step_discovery(hass, flow_handler, local_impl):
"""Check flow triggers from discovery.""" """Check flow triggers from discovery."""
flow_handler.async_register_implementation(hass, local_impl) flow_handler.async_register_implementation(hass, local_impl)