From e6899416e13214df63ccc5edc035039e318613fe Mon Sep 17 00:00:00 2001 From: Erik Montnemery Date: Mon, 17 Jan 2022 18:14:26 +0100 Subject: [PATCH] Suppress Alexa state reports when not authorized (#64064) --- homeassistant/components/alexa/config.py | 17 ++- homeassistant/components/alexa/smart_home.py | 2 +- .../components/alexa/smart_home_http.py | 2 +- .../components/alexa/state_report.py | 8 +- .../components/cloud/alexa_config.py | 3 +- homeassistant/components/cloud/http_api.py | 4 +- tests/components/alexa/test_state_report.py | 37 +++++- tests/components/cloud/test_alexa_config.py | 109 +++++++++++++++++- 8 files changed, 169 insertions(+), 13 deletions(-) diff --git a/homeassistant/components/alexa/config.py b/homeassistant/components/alexa/config.py index ef69860c23e..613fd948366 100644 --- a/homeassistant/components/alexa/config.py +++ b/homeassistant/components/alexa/config.py @@ -1,5 +1,6 @@ """Config helpers for Alexa.""" from abc import ABC, abstractmethod +import logging from homeassistant.core import callback from homeassistant.helpers.storage import Store @@ -9,6 +10,8 @@ from .state_report import async_enable_proactive_mode STORE_AUTHORIZED = "authorized" +_LOGGER = logging.getLogger(__name__) + class AbstractConfig(ABC): """Hold the configuration for Alexa.""" @@ -102,13 +105,25 @@ class AbstractConfig(ABC): """Return authorization status.""" return self._store.authorized - def set_authorized(self, authorized): + async def set_authorized(self, authorized): """Set authorization status. - Set when an incoming message is received from Alexa. - Unset if state reporting fails """ self._store.set_authorized(authorized) + if self.should_report_state != self.is_reporting_states: + if self.should_report_state: + _LOGGER.debug("Enable proactive mode") + try: + await self.async_enable_proactive_mode() + except Exception: + # We failed to enable proactive mode, unset authorized flag + self._store.set_authorized(False) + raise + else: + _LOGGER.debug("Disable proactive mode") + await self.async_disable_proactive_mode() class AlexaConfigStore: diff --git a/homeassistant/components/alexa/smart_home.py b/homeassistant/components/alexa/smart_home.py index caa484ad7ca..7d144619bc9 100644 --- a/homeassistant/components/alexa/smart_home.py +++ b/homeassistant/components/alexa/smart_home.py @@ -31,7 +31,7 @@ async def async_handle_message(hass, config, request, context=None, enabled=True "Alexa API not enabled in Home Assistant configuration" ) - config.set_authorized(True) + await config.set_authorized(True) if directive.has_endpoint: directive.load_entity(hass, config) diff --git a/homeassistant/components/alexa/smart_home_http.py b/homeassistant/components/alexa/smart_home_http.py index 7a6f89414e7..bcb422c4c70 100644 --- a/homeassistant/components/alexa/smart_home_http.py +++ b/homeassistant/components/alexa/smart_home_http.py @@ -39,7 +39,7 @@ class AlexaConfig(AbstractConfig): @property def should_report_state(self): """Return if we should proactively report states.""" - return self._auth is not None + return self._auth is not None and self.authorized @property def endpoint(self): diff --git a/homeassistant/components/alexa/state_report.py b/homeassistant/components/alexa/state_report.py index fff82c6b286..78cf50a697e 100644 --- a/homeassistant/components/alexa/state_report.py +++ b/homeassistant/components/alexa/state_report.py @@ -117,7 +117,11 @@ async def async_send_changereport_message( try: token = await config.async_get_access_token() except (RequireRelink, NoTokenAvailable): - config.set_authorized(False) + await config.set_authorized(False) + _LOGGER.error( + "Error when sending ChangeReport to Alexa, could not get access token" + ) + return headers = {"Authorization": f"Bearer {token}"} @@ -170,7 +174,7 @@ async def async_send_changereport_message( alexa_properties, invalidate_access_token=False, ) - config.set_authorized(False) + await config.set_authorized(False) _LOGGER.error( "Error when sending ChangeReport to Alexa: %s: %s", diff --git a/homeassistant/components/cloud/alexa_config.py b/homeassistant/components/cloud/alexa_config.py index edf6111204d..d17d80f0516 100644 --- a/homeassistant/components/cloud/alexa_config.py +++ b/homeassistant/components/cloud/alexa_config.py @@ -75,7 +75,7 @@ class CloudAlexaConfig(alexa_config.AbstractConfig): @property def should_report_state(self): """Return if states should be proactively reported.""" - return self._prefs.alexa_report_state + return self._prefs.alexa_report_state and self.authorized @property def endpoint(self): @@ -159,7 +159,6 @@ class CloudAlexaConfig(alexa_config.AbstractConfig): if resp.status == HTTPStatus.BAD_REQUEST: if body["reason"] in ("RefreshTokenNotFound", "UnknownRegion"): if self.should_report_state: - await self._prefs.async_update(alexa_report_state=False) persistent_notification.async_create( self.hass, f"There was an error reporting state to Alexa ({body['reason']}). " diff --git a/homeassistant/components/cloud/http_api.py b/homeassistant/components/cloud/http_api.py index 0400f01a39f..028960bffed 100644 --- a/homeassistant/components/cloud/http_api.py +++ b/homeassistant/components/cloud/http_api.py @@ -372,10 +372,10 @@ async def websocket_update_prefs(hass, connection, msg): "Please go to the Alexa app and re-link the Home Assistant " "skill and then try to enable state reporting.", ) - alexa_config.set_authorized(False) + await alexa_config.set_authorized(False) return - alexa_config.set_authorized(True) + await alexa_config.set_authorized(True) await cloud.client.prefs.async_update(**changes) diff --git a/tests/components/alexa/test_state_report.py b/tests/components/alexa/test_state_report.py index 923d91ed4dd..cbadb8697b8 100644 --- a/tests/components/alexa/test_state_report.py +++ b/tests/components/alexa/test_state_report.py @@ -1,8 +1,10 @@ """Test report state.""" -from unittest.mock import patch +from unittest.mock import AsyncMock, patch + +import pytest from homeassistant import core -from homeassistant.components.alexa import state_report +from homeassistant.components.alexa import errors, state_report from . import TEST_URL, get_default_config @@ -99,6 +101,37 @@ async def test_report_state_unsets_authorized_on_error(hass, aioclient_mock): config._store.set_authorized.assert_called_once_with(False) +@pytest.mark.parametrize("exc", [errors.NoTokenAvailable, errors.RequireRelink]) +async def test_report_state_unsets_authorized_on_access_token_error( + hass, aioclient_mock, exc +): + """Test proactive state unsets authorized on error.""" + aioclient_mock.post(TEST_URL, text="", status=202) + + hass.states.async_set( + "binary_sensor.test_contact", + "on", + {"friendly_name": "Test Contact Sensor", "device_class": "door"}, + ) + + config = get_default_config() + + await state_report.async_enable_proactive_mode(hass, config) + + hass.states.async_set( + "binary_sensor.test_contact", + "off", + {"friendly_name": "Test Contact Sensor", "device_class": "door"}, + ) + + config._store.set_authorized.assert_not_called() + + with patch.object(config, "async_get_access_token", AsyncMock(side_effect=exc)): + # To trigger event listener + await hass.async_block_till_done() + config._store.set_authorized.assert_called_once_with(False) + + async def test_report_state_instance(hass, aioclient_mock): """Test proactive state reports with instance.""" aioclient_mock.post(TEST_URL, text="", status=202) diff --git a/tests/components/cloud/test_alexa_config.py b/tests/components/cloud/test_alexa_config.py index 237aad09e4d..0b5000a9ad4 100644 --- a/tests/components/cloud/test_alexa_config.py +++ b/tests/components/cloud/test_alexa_config.py @@ -4,6 +4,7 @@ from unittest.mock import AsyncMock, Mock, patch import pytest +from homeassistant.components.alexa import errors from homeassistant.components.cloud import ALEXA_SCHEMA, alexa_config from homeassistant.helpers.aiohttp_client import async_get_clientsession from homeassistant.helpers.entity_registry import EVENT_ENTITY_REGISTRY_UPDATED @@ -89,6 +90,7 @@ async def test_alexa_config_report_state(hass, cloud_prefs, cloud_stub): hass, ALEXA_SCHEMA({}), "mock-user-id", cloud_prefs, cloud_stub ) await conf.async_initialize() + await conf.set_authorized(True) assert cloud_prefs.alexa_report_state is False assert conf.should_report_state is False @@ -147,6 +149,107 @@ async def test_alexa_config_invalidate_token(hass, cloud_prefs, aioclient_mock): assert len(aioclient_mock.mock_calls) == 2 +@pytest.mark.parametrize( + "reject_reason,expected_exception", + [ + ("RefreshTokenNotFound", errors.RequireRelink), + ("UnknownRegion", errors.RequireRelink), + ("OtherReason", errors.NoTokenAvailable), + ], +) +async def test_alexa_config_fail_refresh_token( + hass, + cloud_prefs, + aioclient_mock, + reject_reason, + expected_exception, +): + """Test Alexa config failing to refresh token.""" + + aioclient_mock.post( + "http://example/alexa_token", + json={ + "access_token": "mock-token", + "event_endpoint": "http://example.com/alexa_endpoint", + "expires_in": 30, + }, + ) + aioclient_mock.post("http://example.com/alexa_endpoint", text="", status=202) + conf = alexa_config.CloudAlexaConfig( + hass, + ALEXA_SCHEMA({}), + "mock-user-id", + cloud_prefs, + Mock( + alexa_access_token_url="http://example/alexa_token", + auth=Mock(async_check_token=AsyncMock()), + websession=async_get_clientsession(hass), + ), + ) + await conf.async_initialize() + await conf.set_authorized(True) + + assert cloud_prefs.alexa_report_state is False + assert conf.should_report_state is False + assert conf.is_reporting_states is False + + hass.states.async_set("fan.test_fan", "off") + + # Enable state reporting + await cloud_prefs.async_update(alexa_report_state=True) + await hass.async_block_till_done() + + assert cloud_prefs.alexa_report_state is True + assert conf.should_report_state is True + assert conf.is_reporting_states is True + + # Change states to trigger event listener + hass.states.async_set("fan.test_fan", "on") + await hass.async_block_till_done() + + # Invalidate the token and try to fetch another + conf.async_invalidate_access_token() + aioclient_mock.clear_requests() + aioclient_mock.post( + "http://example/alexa_token", + json={"reason": reject_reason}, + status=400, + ) + + # Change states to trigger event listener + hass.states.async_set("fan.test_fan", "off") + await hass.async_block_till_done() + + # Check state reporting is still wanted in cloud prefs, but disabled for Alexa + assert cloud_prefs.alexa_report_state is True + assert conf.should_report_state is False + assert conf.is_reporting_states is False + + # Simulate we're again authorized, but token update fails + with pytest.raises(expected_exception): + await conf.set_authorized(True) + + assert cloud_prefs.alexa_report_state is True + assert conf.should_report_state is False + assert conf.is_reporting_states is False + + # Simulate we're again authorized and token update succeeds + # State reporting should now be re-enabled for Alexa + aioclient_mock.clear_requests() + aioclient_mock.post( + "http://example/alexa_token", + json={ + "access_token": "mock-token", + "event_endpoint": "http://example.com/alexa_endpoint", + "expires_in": 30, + }, + ) + await conf.set_authorized(True) + assert cloud_prefs.alexa_report_state is True + assert conf.should_report_state is True + assert conf.is_reporting_states is True + + @contextlib.contextmanager def patch_sync_helper(): """Patch sync helper. @@ -257,9 +360,11 @@ async def test_alexa_entity_registry_sync(hass, mock_cloud_login, cloud_prefs): async def test_alexa_update_report_state(hass, cloud_prefs, cloud_stub): """Test Alexa config responds to reporting state.""" - await alexa_config.CloudAlexaConfig( + conf = alexa_config.CloudAlexaConfig( hass, ALEXA_SCHEMA({}), "mock-user-id", cloud_prefs, cloud_stub - ).async_initialize() + ) + await conf.async_initialize() + await conf.set_authorized(True) with patch( "homeassistant.components.cloud.alexa_config.CloudAlexaConfig.async_sync_entities",