From 111050bea936646325caa3a2a7b84860597dd41c Mon Sep 17 00:00:00 2001 From: Paulus Schoutsen Date: Sat, 8 Feb 2020 04:10:59 -0800 Subject: [PATCH] Clean up core services (#31509) * Clean up core services * Fix conversation test --- .../components/conversation/__init__.py | 19 ++- homeassistant/components/group/__init__.py | 7 +- .../components/homeassistant/__init__.py | 53 ++++--- homeassistant/components/intent/__init__.py | 19 ++- tests/components/conversation/test_init.py | 8 +- tests/components/homeassistant/test_init.py | 143 +++++++----------- tests/components/intent/test_init.py | 94 ++++++++++++ 7 files changed, 222 insertions(+), 121 deletions(-) diff --git a/homeassistant/components/conversation/__init__.py b/homeassistant/components/conversation/__init__.py index 158a365981b..91031c141dd 100644 --- a/homeassistant/components/conversation/__init__.py +++ b/homeassistant/components/conversation/__init__.py @@ -131,9 +131,22 @@ class ConversationProcessView(http.HomeAssistantView): """Send a request for processing.""" hass = request.app["hass"] - intent_result = await _async_converse( - hass, data["text"], data.get("conversation_id"), self.context(request) - ) + try: + intent_result = await _async_converse( + hass, data["text"], data.get("conversation_id"), self.context(request) + ) + except intent.IntentError as err: + _LOGGER.error("Error handling intent: %s", err) + return self.json( + { + "success": False, + "error": { + "code": str(err.__class__.__name__).lower(), + "message": str(err), + }, + }, + status_code=500, + ) return self.json(intent_result) diff --git a/homeassistant/components/group/__init__.py b/homeassistant/components/group/__init__.py index fc37f904e0d..7257959700f 100644 --- a/homeassistant/components/group/__init__.py +++ b/homeassistant/components/group/__init__.py @@ -13,6 +13,8 @@ from homeassistant.const import ( ATTR_NAME, CONF_ICON, CONF_NAME, + ENTITY_MATCH_ALL, + ENTITY_MATCH_NONE, SERVICE_RELOAD, STATE_CLOSED, STATE_HOME, @@ -134,7 +136,10 @@ def expand_entity_ids(hass: HomeAssistantType, entity_ids: Iterable[Any]) -> Lis """ found_ids: List[str] = [] for entity_id in entity_ids: - if not isinstance(entity_id, str): + if not isinstance(entity_id, str) or entity_id in ( + ENTITY_MATCH_NONE, + ENTITY_MATCH_ALL, + ): continue entity_id = entity_id.lower() diff --git a/homeassistant/components/homeassistant/__init__.py b/homeassistant/components/homeassistant/__init__.py index 8aa1d7e020a..17ab9ba3b44 100644 --- a/homeassistant/components/homeassistant/__init__.py +++ b/homeassistant/components/homeassistant/__init__.py @@ -6,6 +6,7 @@ from typing import Awaitable import voluptuous as vol +from homeassistant.auth.permissions.const import CAT_ENTITIES, POLICY_CONTROL import homeassistant.config as conf_util from homeassistant.const import ( ATTR_ENTITY_ID, @@ -19,8 +20,8 @@ from homeassistant.const import ( SERVICE_TURN_ON, ) import homeassistant.core as ha -from homeassistant.exceptions import HomeAssistantError -from homeassistant.helpers import config_validation as cv, intent +from homeassistant.exceptions import HomeAssistantError, Unauthorized, UnknownUser +from homeassistant.helpers import config_validation as cv from homeassistant.helpers.service import async_extract_entity_ids _LOGGER = logging.getLogger(__name__) @@ -74,23 +75,16 @@ async def async_setup(hass: ha.HomeAssistant, config: dict) -> Awaitable[bool]: await asyncio.wait(tasks) - hass.services.async_register(ha.DOMAIN, SERVICE_TURN_OFF, async_handle_turn_service) - hass.services.async_register(ha.DOMAIN, SERVICE_TURN_ON, async_handle_turn_service) - hass.services.async_register(ha.DOMAIN, SERVICE_TOGGLE, async_handle_turn_service) - hass.helpers.intent.async_register( - intent.ServiceIntentHandler( - intent.INTENT_TURN_ON, ha.DOMAIN, SERVICE_TURN_ON, "Turned {} on" - ) + service_schema = vol.Schema({ATTR_ENTITY_ID: cv.entity_ids}, extra=vol.ALLOW_EXTRA) + + hass.services.async_register( + ha.DOMAIN, SERVICE_TURN_OFF, async_handle_turn_service, schema=service_schema ) - hass.helpers.intent.async_register( - intent.ServiceIntentHandler( - intent.INTENT_TURN_OFF, ha.DOMAIN, SERVICE_TURN_OFF, "Turned {} off" - ) + hass.services.async_register( + ha.DOMAIN, SERVICE_TURN_ON, async_handle_turn_service, schema=service_schema ) - hass.helpers.intent.async_register( - intent.ServiceIntentHandler( - intent.INTENT_TOGGLE, ha.DOMAIN, SERVICE_TOGGLE, "Toggled {}" - ) + hass.services.async_register( + ha.DOMAIN, SERVICE_TOGGLE, async_handle_turn_service, schema=service_schema ) async def async_handle_core_service(call): @@ -118,6 +112,25 @@ async def async_setup(hass: ha.HomeAssistant, config: dict) -> Awaitable[bool]: async def async_handle_update_service(call): """Service handler for updating an entity.""" + if call.context.user_id: + user = await hass.auth.async_get_user(call.context.user_id) + + if user is None: + raise UnknownUser( + context=call.context, + permission=POLICY_CONTROL, + user_id=call.context.user_id, + ) + + for entity in call.data[ATTR_ENTITY_ID]: + if not user.permissions.check_entity(entity, POLICY_CONTROL): + raise Unauthorized( + context=call.context, + permission=POLICY_CONTROL, + user_id=call.context.user_id, + perm_category=CAT_ENTITIES, + ) + tasks = [ hass.helpers.entity_component.async_update_entity(entity) for entity in call.data[ATTR_ENTITY_ID] @@ -126,13 +139,13 @@ async def async_setup(hass: ha.HomeAssistant, config: dict) -> Awaitable[bool]: if tasks: await asyncio.wait(tasks) - hass.services.async_register( + hass.helpers.service.async_register_admin_service( ha.DOMAIN, SERVICE_HOMEASSISTANT_STOP, async_handle_core_service ) - hass.services.async_register( + hass.helpers.service.async_register_admin_service( ha.DOMAIN, SERVICE_HOMEASSISTANT_RESTART, async_handle_core_service ) - hass.services.async_register( + hass.helpers.service.async_register_admin_service( ha.DOMAIN, SERVICE_CHECK_CONFIG, async_handle_core_service ) hass.services.async_register( diff --git a/homeassistant/components/intent/__init__.py b/homeassistant/components/intent/__init__.py index bdf612b2e83..37761e88347 100644 --- a/homeassistant/components/intent/__init__.py +++ b/homeassistant/components/intent/__init__.py @@ -5,7 +5,8 @@ import voluptuous as vol from homeassistant.components import http from homeassistant.components.http.data_validator import RequestDataValidator -from homeassistant.core import HomeAssistant +from homeassistant.const import SERVICE_TOGGLE, SERVICE_TURN_OFF, SERVICE_TURN_ON +from homeassistant.core import DOMAIN as HA_DOMAIN, HomeAssistant from homeassistant.helpers import config_validation as cv, integration_platform, intent from .const import DOMAIN @@ -22,6 +23,22 @@ async def async_setup(hass: HomeAssistant, config: dict): hass, DOMAIN, _async_process_intent ) + hass.helpers.intent.async_register( + intent.ServiceIntentHandler( + intent.INTENT_TURN_ON, HA_DOMAIN, SERVICE_TURN_ON, "Turned {} on" + ) + ) + hass.helpers.intent.async_register( + intent.ServiceIntentHandler( + intent.INTENT_TURN_OFF, HA_DOMAIN, SERVICE_TURN_OFF, "Turned {} off" + ) + ) + hass.helpers.intent.async_register( + intent.ServiceIntentHandler( + intent.INTENT_TOGGLE, HA_DOMAIN, SERVICE_TOGGLE, "Toggled {}" + ) + ) + return True diff --git a/tests/components/conversation/test_init.py b/tests/components/conversation/test_init.py index f84d2109095..737d99cbddd 100644 --- a/tests/components/conversation/test_init.py +++ b/tests/components/conversation/test_init.py @@ -201,11 +201,9 @@ async def test_toggle_intent(hass, sentence): async def test_http_api(hass, hass_client): """Test the HTTP conversation API.""" - result = await async_setup_component(hass, "homeassistant", {}) - assert result - - result = await async_setup_component(hass, "conversation", {}) - assert result + assert await async_setup_component(hass, "homeassistant", {}) + assert await async_setup_component(hass, "conversation", {}) + assert await async_setup_component(hass, "intent", {}) client = await hass_client() hass.states.async_set("light.kitchen", "off") diff --git a/tests/components/homeassistant/test_init.py b/tests/components/homeassistant/test_init.py index 6c2b7f78e24..0f7dc7ed309 100644 --- a/tests/components/homeassistant/test_init.py +++ b/tests/components/homeassistant/test_init.py @@ -4,6 +4,8 @@ import asyncio import unittest from unittest.mock import Mock, patch +import pytest +import voluptuous as vol import yaml from homeassistant import config @@ -11,9 +13,12 @@ import homeassistant.components as comps from homeassistant.components.homeassistant import ( SERVICE_CHECK_CONFIG, SERVICE_RELOAD_CORE_CONFIG, + SERVICE_SET_LOCATION, ) from homeassistant.const import ( ATTR_ENTITY_ID, + ENTITY_MATCH_ALL, + ENTITY_MATCH_NONE, EVENT_CORE_CONFIG_UPDATE, SERVICE_HOMEASSISTANT_RESTART, SERVICE_HOMEASSISTANT_STOP, @@ -24,9 +29,8 @@ from homeassistant.const import ( STATE_ON, ) import homeassistant.core as ha -from homeassistant.exceptions import HomeAssistantError +from homeassistant.exceptions import HomeAssistantError, Unauthorized from homeassistant.helpers import entity -import homeassistant.helpers.intent as intent from homeassistant.setup import async_setup_component from tests.common import ( @@ -249,95 +253,6 @@ class TestComponentsCore(unittest.TestCase): assert not mock_stop.called -async def test_turn_on_intent(hass): - """Test HassTurnOn intent.""" - result = await async_setup_component(hass, "homeassistant", {}) - assert result - - hass.states.async_set("light.test_light", "off") - calls = async_mock_service(hass, "light", SERVICE_TURN_ON) - - response = await intent.async_handle( - hass, "test", "HassTurnOn", {"name": {"value": "test light"}} - ) - await hass.async_block_till_done() - - assert response.speech["plain"]["speech"] == "Turned test light on" - assert len(calls) == 1 - call = calls[0] - assert call.domain == "light" - assert call.service == "turn_on" - assert call.data == {"entity_id": ["light.test_light"]} - - -async def test_turn_off_intent(hass): - """Test HassTurnOff intent.""" - result = await async_setup_component(hass, "homeassistant", {}) - assert result - - hass.states.async_set("light.test_light", "on") - calls = async_mock_service(hass, "light", SERVICE_TURN_OFF) - - response = await intent.async_handle( - hass, "test", "HassTurnOff", {"name": {"value": "test light"}} - ) - await hass.async_block_till_done() - - assert response.speech["plain"]["speech"] == "Turned test light off" - assert len(calls) == 1 - call = calls[0] - assert call.domain == "light" - assert call.service == "turn_off" - assert call.data == {"entity_id": ["light.test_light"]} - - -async def test_toggle_intent(hass): - """Test HassToggle intent.""" - result = await async_setup_component(hass, "homeassistant", {}) - assert result - - hass.states.async_set("light.test_light", "off") - calls = async_mock_service(hass, "light", SERVICE_TOGGLE) - - response = await intent.async_handle( - hass, "test", "HassToggle", {"name": {"value": "test light"}} - ) - await hass.async_block_till_done() - - assert response.speech["plain"]["speech"] == "Toggled test light" - assert len(calls) == 1 - call = calls[0] - assert call.domain == "light" - assert call.service == "toggle" - assert call.data == {"entity_id": ["light.test_light"]} - - -async def test_turn_on_multiple_intent(hass): - """Test HassTurnOn intent with multiple similar entities. - - This tests that matching finds the proper entity among similar names. - """ - result = await async_setup_component(hass, "homeassistant", {}) - assert result - - hass.states.async_set("light.test_light", "off") - hass.states.async_set("light.test_lights_2", "off") - hass.states.async_set("light.test_lighter", "off") - calls = async_mock_service(hass, "light", SERVICE_TURN_ON) - - response = await intent.async_handle( - hass, "test", "HassTurnOn", {"name": {"value": "test lights"}} - ) - await hass.async_block_till_done() - - assert response.speech["plain"]["speech"] == "Turned test lights 2 on" - assert len(calls) == 1 - call = calls[0] - assert call.domain == "light" - assert call.service == "turn_on" - assert call.data == {"entity_id": ["light.test_lights_2"]} - - async def test_turn_on_to_not_block_for_domains_without_service(hass): """Test if turn_on is blocking domain with no service.""" await async_setup_component(hass, "homeassistant", {}) @@ -411,3 +326,49 @@ async def test_setting_location(hass): assert len(events) == 1 assert hass.config.latitude == 30 assert hass.config.longitude == 40 + + +async def test_require_admin(hass, hass_read_only_user): + """Test services requiring admin.""" + await async_setup_component(hass, "homeassistant", {}) + + for service in ( + SERVICE_HOMEASSISTANT_RESTART, + SERVICE_HOMEASSISTANT_STOP, + SERVICE_CHECK_CONFIG, + SERVICE_RELOAD_CORE_CONFIG, + ): + with pytest.raises(Unauthorized): + await hass.services.async_call( + ha.DOMAIN, + service, + {}, + context=ha.Context(user_id=hass_read_only_user.id), + blocking=True, + ) + assert False, f"Should have raises for {service}" + + with pytest.raises(Unauthorized): + await hass.services.async_call( + ha.DOMAIN, + SERVICE_SET_LOCATION, + {"latitude": 0, "longitude": 0}, + context=ha.Context(user_id=hass_read_only_user.id), + blocking=True, + ) + + +async def test_turn_on_off_toggle_schema(hass, hass_read_only_user): + """Test the schemas for the turn on/off/toggle services.""" + await async_setup_component(hass, "homeassistant", {}) + + for service in SERVICE_TURN_ON, SERVICE_TURN_OFF, SERVICE_TOGGLE: + for invalid in None, "nothing", ENTITY_MATCH_ALL, ENTITY_MATCH_NONE: + with pytest.raises(vol.Invalid): + await hass.services.async_call( + ha.DOMAIN, + service, + {"entity_id": invalid}, + context=ha.Context(user_id=hass_read_only_user.id), + blocking=True, + ) diff --git a/tests/components/intent/test_init.py b/tests/components/intent/test_init.py index 56344b6affe..723736f35bc 100644 --- a/tests/components/intent/test_init.py +++ b/tests/components/intent/test_init.py @@ -2,6 +2,7 @@ import pytest from homeassistant.components.cover import SERVICE_OPEN_COVER +from homeassistant.const import SERVICE_TOGGLE, SERVICE_TURN_OFF, SERVICE_TURN_ON from homeassistant.helpers import intent from homeassistant.setup import async_setup_component @@ -74,3 +75,96 @@ async def test_cover_intents_loading(hass): assert call.domain == "cover" assert call.service == "open_cover" assert call.data == {"entity_id": "cover.garage_door"} + + +async def test_turn_on_intent(hass): + """Test HassTurnOn intent.""" + result = await async_setup_component(hass, "homeassistant", {}) + result = await async_setup_component(hass, "intent", {}) + assert result + + hass.states.async_set("light.test_light", "off") + calls = async_mock_service(hass, "light", SERVICE_TURN_ON) + + response = await intent.async_handle( + hass, "test", "HassTurnOn", {"name": {"value": "test light"}} + ) + await hass.async_block_till_done() + + assert response.speech["plain"]["speech"] == "Turned test light on" + assert len(calls) == 1 + call = calls[0] + assert call.domain == "light" + assert call.service == "turn_on" + assert call.data == {"entity_id": ["light.test_light"]} + + +async def test_turn_off_intent(hass): + """Test HassTurnOff intent.""" + result = await async_setup_component(hass, "homeassistant", {}) + result = await async_setup_component(hass, "intent", {}) + assert result + + hass.states.async_set("light.test_light", "on") + calls = async_mock_service(hass, "light", SERVICE_TURN_OFF) + + response = await intent.async_handle( + hass, "test", "HassTurnOff", {"name": {"value": "test light"}} + ) + await hass.async_block_till_done() + + assert response.speech["plain"]["speech"] == "Turned test light off" + assert len(calls) == 1 + call = calls[0] + assert call.domain == "light" + assert call.service == "turn_off" + assert call.data == {"entity_id": ["light.test_light"]} + + +async def test_toggle_intent(hass): + """Test HassToggle intent.""" + result = await async_setup_component(hass, "homeassistant", {}) + result = await async_setup_component(hass, "intent", {}) + assert result + + hass.states.async_set("light.test_light", "off") + calls = async_mock_service(hass, "light", SERVICE_TOGGLE) + + response = await intent.async_handle( + hass, "test", "HassToggle", {"name": {"value": "test light"}} + ) + await hass.async_block_till_done() + + assert response.speech["plain"]["speech"] == "Toggled test light" + assert len(calls) == 1 + call = calls[0] + assert call.domain == "light" + assert call.service == "toggle" + assert call.data == {"entity_id": ["light.test_light"]} + + +async def test_turn_on_multiple_intent(hass): + """Test HassTurnOn intent with multiple similar entities. + + This tests that matching finds the proper entity among similar names. + """ + result = await async_setup_component(hass, "homeassistant", {}) + result = await async_setup_component(hass, "intent", {}) + assert result + + hass.states.async_set("light.test_light", "off") + hass.states.async_set("light.test_lights_2", "off") + hass.states.async_set("light.test_lighter", "off") + calls = async_mock_service(hass, "light", SERVICE_TURN_ON) + + response = await intent.async_handle( + hass, "test", "HassTurnOn", {"name": {"value": "test lights"}} + ) + await hass.async_block_till_done() + + assert response.speech["plain"]["speech"] == "Turned test lights 2 on" + assert len(calls) == 1 + call = calls[0] + assert call.domain == "light" + assert call.service == "turn_on" + assert call.data == {"entity_id": ["light.test_lights_2"]}