From 73d7d80731a5f18915e4e871111b752d1137ff66 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sun, 31 Jan 2021 10:43:00 -1000 Subject: [PATCH] Add timeout to lutron_caseta to prevent it blocking startup (#45769) --- .../components/lutron_caseta/__init__.py | 26 ++++++-- .../components/lutron_caseta/config_flow.py | 47 ++++++++++----- .../components/lutron_caseta/const.py | 2 + .../lutron_caseta/test_config_flow.py | 60 +++++++++++++------ 4 files changed, 97 insertions(+), 38 deletions(-) diff --git a/homeassistant/components/lutron_caseta/__init__.py b/homeassistant/components/lutron_caseta/__init__.py index 7526d4874f6..f349dde8921 100644 --- a/homeassistant/components/lutron_caseta/__init__.py +++ b/homeassistant/components/lutron_caseta/__init__.py @@ -1,10 +1,12 @@ """Component for interacting with a Lutron Caseta system.""" import asyncio import logging +import ssl from aiolip import LIP from aiolip.data import LIPMode from aiolip.protocol import LIP_BUTTON_PRESS +import async_timeout from pylutron_caseta.smartbridge import Smartbridge import voluptuous as vol @@ -29,6 +31,7 @@ from .const import ( BRIDGE_DEVICE_ID, BRIDGE_LEAP, BRIDGE_LIP, + BRIDGE_TIMEOUT, BUTTON_DEVICES, CONF_CA_CERTS, CONF_CERTFILE, @@ -94,15 +97,26 @@ async def async_setup_entry(hass, config_entry): keyfile = hass.config.path(config_entry.data[CONF_KEYFILE]) certfile = hass.config.path(config_entry.data[CONF_CERTFILE]) ca_certs = hass.config.path(config_entry.data[CONF_CA_CERTS]) + bridge = None - bridge = Smartbridge.create_tls( - hostname=host, keyfile=keyfile, certfile=certfile, ca_certs=ca_certs - ) + try: + bridge = Smartbridge.create_tls( + hostname=host, keyfile=keyfile, certfile=certfile, ca_certs=ca_certs + ) + except ssl.SSLError: + _LOGGER.error("Invalid certificate used to connect to bridge at %s.", host) + return False - await bridge.connect() - if not bridge.is_connected(): + timed_out = True + try: + with async_timeout.timeout(BRIDGE_TIMEOUT): + await bridge.connect() + timed_out = False + except asyncio.TimeoutError: + _LOGGER.error("Timeout while trying to connect to bridge at %s.", host) + + if timed_out or not bridge.is_connected(): await bridge.close() - _LOGGER.error("Unable to connect to Lutron Caseta bridge at %s", host) raise ConfigEntryNotReady _LOGGER.debug("Connected to Lutron Caseta bridge via LEAP at %s", host) diff --git a/homeassistant/components/lutron_caseta/config_flow.py b/homeassistant/components/lutron_caseta/config_flow.py index 806096d6717..30cbc03ac47 100644 --- a/homeassistant/components/lutron_caseta/config_flow.py +++ b/homeassistant/components/lutron_caseta/config_flow.py @@ -2,7 +2,9 @@ import asyncio import logging import os +import ssl +import async_timeout from pylutron_caseta.pairing import PAIR_CA, PAIR_CERT, PAIR_KEY, async_pair from pylutron_caseta.smartbridge import Smartbridge import voluptuous as vol @@ -15,6 +17,7 @@ from homeassistant.core import callback from .const import ( ABORT_REASON_ALREADY_CONFIGURED, ABORT_REASON_CANNOT_CONNECT, + BRIDGE_TIMEOUT, CONF_CA_CERTS, CONF_CERTFILE, CONF_KEYFILE, @@ -50,6 +53,8 @@ class LutronCasetaFlowHandler(config_entries.ConfigFlow, domain=DOMAIN): """Initialize a Lutron Caseta flow.""" self.data = {} self.lutron_id = None + self.tls_assets_validated = False + self.attempted_tls_validation = False async def async_step_user(self, user_input=None): """Handle a flow initialized by the user.""" @@ -92,11 +97,16 @@ class LutronCasetaFlowHandler(config_entries.ConfigFlow, domain=DOMAIN): self._configure_tls_assets() + if ( + not self.attempted_tls_validation + and await self.hass.async_add_executor_job(self._tls_assets_exist) + and await self.async_validate_connectable_bridge_config() + ): + self.tls_assets_validated = True + self.attempted_tls_validation = True + if user_input is not None: - if ( - await self.hass.async_add_executor_job(self._tls_assets_exist) - and await self.async_validate_connectable_bridge_config() - ): + if self.tls_assets_validated: # If we previous paired and the tls assets already exist, # we do not need to go though pairing again. return self.async_create_entry(title=self.bridge_id, data=self.data) @@ -207,6 +217,8 @@ class LutronCasetaFlowHandler(config_entries.ConfigFlow, domain=DOMAIN): async def async_validate_connectable_bridge_config(self): """Check if we can connect to the bridge with the current config.""" + bridge = None + try: bridge = Smartbridge.create_tls( hostname=self.data[CONF_HOST], @@ -214,16 +226,23 @@ class LutronCasetaFlowHandler(config_entries.ConfigFlow, domain=DOMAIN): certfile=self.hass.config.path(self.data[CONF_CERTFILE]), ca_certs=self.hass.config.path(self.data[CONF_CA_CERTS]), ) - - await bridge.connect() - if not bridge.is_connected(): - return False - - await bridge.close() - return True - except Exception: # pylint: disable=broad-except - _LOGGER.exception( - "Unknown exception while checking connectivity to bridge %s", + except ssl.SSLError: + _LOGGER.error( + "Invalid certificate used to connect to bridge at %s.", self.data[CONF_HOST], ) return False + + connected_ok = False + try: + with async_timeout.timeout(BRIDGE_TIMEOUT): + await bridge.connect() + connected_ok = bridge.is_connected() + except asyncio.TimeoutError: + _LOGGER.error( + "Timeout while trying to connect to bridge at %s.", + self.data[CONF_HOST], + ) + + await bridge.close() + return connected_ok diff --git a/homeassistant/components/lutron_caseta/const.py b/homeassistant/components/lutron_caseta/const.py index 4226be36f05..fcc647f00ba 100644 --- a/homeassistant/components/lutron_caseta/const.py +++ b/homeassistant/components/lutron_caseta/const.py @@ -33,3 +33,5 @@ ACTION_RELEASE = "release" CONF_TYPE = "type" CONF_SUBTYPE = "subtype" + +BRIDGE_TIMEOUT = 35 diff --git a/tests/components/lutron_caseta/test_config_flow.py b/tests/components/lutron_caseta/test_config_flow.py index 8a33fab670b..58377c8e085 100644 --- a/tests/components/lutron_caseta/test_config_flow.py +++ b/tests/components/lutron_caseta/test_config_flow.py @@ -1,5 +1,6 @@ """Test the Lutron Caseta config flow.""" import asyncio +import ssl from unittest.mock import AsyncMock, patch from pylutron_caseta.pairing import PAIR_CA, PAIR_CERT, PAIR_KEY @@ -21,6 +22,14 @@ from homeassistant.const import CONF_HOST from tests.common import MockConfigEntry +EMPTY_MOCK_CONFIG_ENTRY = { + CONF_HOST: "", + CONF_KEYFILE: "", + CONF_CERTFILE: "", + CONF_CA_CERTS: "", +} + + MOCK_ASYNC_PAIR_SUCCESS = { PAIR_KEY: "mock_key", PAIR_CERT: "mock_cert", @@ -115,21 +124,34 @@ async def test_bridge_cannot_connect(hass): async def test_bridge_cannot_connect_unknown_error(hass): """Test checking for connection and encountering an unknown error.""" - entry_mock_data = { - CONF_HOST: "", - CONF_KEYFILE: "", - CONF_CERTFILE: "", - CONF_CA_CERTS: "", - } - with patch.object(Smartbridge, "create_tls") as create_tls: mock_bridge = MockBridge() - mock_bridge.connect = AsyncMock(side_effect=Exception()) + mock_bridge.connect = AsyncMock(side_effect=asyncio.TimeoutError) create_tls.return_value = mock_bridge result = await hass.config_entries.flow.async_init( DOMAIN, context={"source": config_entries.SOURCE_IMPORT}, - data=entry_mock_data, + data=EMPTY_MOCK_CONFIG_ENTRY, + ) + + assert result["type"] == "form" + assert result["step_id"] == STEP_IMPORT_FAILED + assert result["errors"] == {"base": ERROR_CANNOT_CONNECT} + + result = await hass.config_entries.flow.async_configure(result["flow_id"], {}) + + assert result["type"] == data_entry_flow.RESULT_TYPE_ABORT + assert result["reason"] == CasetaConfigFlow.ABORT_REASON_CANNOT_CONNECT + + +async def test_bridge_invalid_ssl_error(hass): + """Test checking for connection and encountering invalid ssl certs.""" + + with patch.object(Smartbridge, "create_tls", side_effect=ssl.SSLError): + result = await hass.config_entries.flow.async_init( + DOMAIN, + context={"source": config_entries.SOURCE_IMPORT}, + data=EMPTY_MOCK_CONFIG_ENTRY, ) assert result["type"] == "form" @@ -351,23 +373,25 @@ async def test_form_user_reuses_existing_assets_when_pairing_again(hass, tmpdir) assert result["errors"] is None assert result["step_id"] == "user" - result2 = await hass.config_entries.flow.async_configure( - result["flow_id"], - { - CONF_HOST: "1.1.1.1", - }, - ) - await hass.async_block_till_done() + with patch.object(Smartbridge, "create_tls") as create_tls: + create_tls.return_value = MockBridge(can_connect=True) + result2 = await hass.config_entries.flow.async_configure( + result["flow_id"], + { + CONF_HOST: "1.1.1.1", + }, + ) + await hass.async_block_till_done() + assert result2["type"] == "form" assert result2["step_id"] == "link" - with patch.object(Smartbridge, "create_tls") as create_tls, patch( + with patch( "homeassistant.components.lutron_caseta.async_setup", return_value=True ), patch( "homeassistant.components.lutron_caseta.async_setup_entry", return_value=True, ): - create_tls.return_value = MockBridge(can_connect=True) result3 = await hass.config_entries.flow.async_configure( result2["flow_id"], {},