From bf839687ad68f5dd1f72111d1876062868a06461 Mon Sep 17 00:00:00 2001 From: Robbie Trencheny Date: Tue, 12 Mar 2019 22:04:27 -0700 Subject: [PATCH] Mobile App: Registration schema improvements (#21850) * Update registration schema to add os_name (required) and make app_name required * Ensure that a provided app_component is valid and available * Ensure that component DEPENDENCIES declares mobile_app * Update homeassistant/helpers/config_validation.py * Standardize error responses * Dont generalize REGISTER_BAD_COMPONENT * Fix tests after merge --- homeassistant/components/mobile_app/const.py | 9 ++++++- .../components/mobile_app/helpers.py | 14 +++++++++- .../components/mobile_app/http_api.py | 27 +++++++++++++++---- .../components/mobile_app/webhook.py | 14 +++++----- tests/components/mobile_app/const.py | 2 ++ tests/components/mobile_app/test_http_api.py | 25 +++++++++++++++++ 6 files changed, 76 insertions(+), 15 deletions(-) diff --git a/homeassistant/components/mobile_app/const.py b/homeassistant/components/mobile_app/const.py index d2f32b8877c..60b4cde4708 100644 --- a/homeassistant/components/mobile_app/const.py +++ b/homeassistant/components/mobile_app/const.py @@ -26,6 +26,7 @@ ATTR_APP_VERSION = 'app_version' ATTR_DEVICE_NAME = 'device_name' ATTR_MANUFACTURER = 'manufacturer' ATTR_MODEL = 'model' +ATTR_OS_NAME = 'os_name' ATTR_OS_VERSION = 'os_version' ATTR_SUPPORTS_ENCRYPTION = 'supports_encryption' @@ -40,6 +41,10 @@ ATTR_WEBHOOK_ENCRYPTED = 'encrypted' ATTR_WEBHOOK_ENCRYPTED_DATA = 'encrypted_data' ATTR_WEBHOOK_TYPE = 'type' +ERR_INVALID_COMPONENT = 'invalid_component' +ERR_RENDER_FAILURE = 'render_failure' +ERR_SAVE_FAILURE = 'save_failure' + WEBHOOK_TYPE_CALL_SERVICE = 'call_service' WEBHOOK_TYPE_FIRE_EVENT = 'fire_event' WEBHOOK_TYPE_RENDER_TEMPLATE = 'render_template' @@ -50,15 +55,17 @@ WEBHOOK_TYPES = [WEBHOOK_TYPE_CALL_SERVICE, WEBHOOK_TYPE_FIRE_EVENT, WEBHOOK_TYPE_RENDER_TEMPLATE, WEBHOOK_TYPE_UPDATE_LOCATION, WEBHOOK_TYPE_UPDATE_REGISTRATION] + REGISTRATION_SCHEMA = vol.Schema({ vol.Optional(ATTR_APP_COMPONENT): cv.string, vol.Optional(ATTR_APP_DATA, default={}): dict, vol.Required(ATTR_APP_ID): cv.string, - vol.Optional(ATTR_APP_NAME): cv.string, + vol.Required(ATTR_APP_NAME): cv.string, vol.Required(ATTR_APP_VERSION): cv.string, vol.Required(ATTR_DEVICE_NAME): cv.string, vol.Required(ATTR_MANUFACTURER): cv.string, vol.Required(ATTR_MODEL): cv.string, + vol.Required(ATTR_OS_NAME): cv.string, vol.Optional(ATTR_OS_VERSION): cv.string, vol.Required(ATTR_SUPPORTS_ENCRYPTION, default=False): cv.boolean, }) diff --git a/homeassistant/components/mobile_app/helpers.py b/homeassistant/components/mobile_app/helpers.py index 28d8a797a32..1f67170a72c 100644 --- a/homeassistant/components/mobile_app/helpers.py +++ b/homeassistant/components/mobile_app/helpers.py @@ -3,7 +3,7 @@ import logging import json from typing import Callable, Dict, Tuple -from aiohttp.web import Response +from aiohttp.web import json_response, Response from homeassistant.core import Context from homeassistant.helpers.typing import HomeAssistantType @@ -84,6 +84,18 @@ def empty_okay_response(headers: Dict = None, status: int = 200) -> Response: headers=headers) +def error_response(code: str, message: str, status: int = 400, + headers: dict = None) -> Response: + """Return an error Response.""" + return json_response({ + 'success': False, + 'error': { + 'code': code, + 'message': message + } + }, status=status, headers=headers) + + def supports_encryption() -> bool: """Test if we support encryption.""" try: diff --git a/homeassistant/components/mobile_app/http_api.py b/homeassistant/components/mobile_app/http_api.py index 15e1385359e..30083cc86b1 100644 --- a/homeassistant/components/mobile_app/http_api.py +++ b/homeassistant/components/mobile_app/http_api.py @@ -13,12 +13,14 @@ from homeassistant.const import (HTTP_CREATED, HTTP_INTERNAL_SERVER_ERROR, from homeassistant.exceptions import HomeAssistantError from homeassistant.helpers.storage import Store from homeassistant.helpers.typing import HomeAssistantType +from homeassistant.loader import get_component -from .const import (DATA_REGISTRATIONS, ATTR_SUPPORTS_ENCRYPTION, +from .const import (ATTR_APP_COMPONENT, ATTR_SUPPORTS_ENCRYPTION, CONF_CLOUDHOOK_URL, CONF_SECRET, CONF_USER_ID, - DOMAIN, REGISTRATION_SCHEMA) + DATA_REGISTRATIONS, DOMAIN, ERR_INVALID_COMPONENT, + ERR_SAVE_FAILURE, REGISTRATION_SCHEMA) -from .helpers import supports_encryption, savable_state +from .helpers import error_response, supports_encryption, savable_state from .webhook import setup_registration @@ -44,6 +46,20 @@ class RegistrationsView(HomeAssistantView): """Handle the POST request for registration.""" hass = request.app['hass'] + if ATTR_APP_COMPONENT in data: + component = get_component(hass, data[ATTR_APP_COMPONENT]) + if component is None: + fmt_str = "{} is not a valid component." + msg = fmt_str.format(data[ATTR_APP_COMPONENT]) + return error_response(ERR_INVALID_COMPONENT, msg) + + if (hasattr(component, 'DEPENDENCIES') is False or + (hasattr(component, 'DEPENDENCIES') and + DOMAIN not in component.DEPENDENCIES)): + fmt_str = "{} is not compatible with mobile_app." + msg = fmt_str.format(data[ATTR_APP_COMPONENT]) + return error_response(ERR_INVALID_COMPONENT, msg) + webhook_id = generate_secret() if hass.components.cloud.async_active_subscription(): @@ -64,8 +80,9 @@ class RegistrationsView(HomeAssistantView): try: await self._store.async_save(savable_state(hass)) except HomeAssistantError: - return self.json_message("Error saving registration.", - HTTP_INTERNAL_SERVER_ERROR) + return error_response(ERR_SAVE_FAILURE, + "Error saving registration", + status=HTTP_INTERNAL_SERVER_ERROR) setup_registration(hass, self._store, data) diff --git a/homeassistant/components/mobile_app/webhook.py b/homeassistant/components/mobile_app/webhook.py index a14354e4ae3..61188b50e1b 100644 --- a/homeassistant/components/mobile_app/webhook.py +++ b/homeassistant/components/mobile_app/webhook.py @@ -25,13 +25,13 @@ from .const import (ATTR_APP_COMPONENT, DATA_DELETED_IDS, DATA_REGISTRATIONS, ATTR_TEMPLATE, ATTR_TEMPLATE_VARIABLES, ATTR_WEBHOOK_DATA, ATTR_WEBHOOK_ENCRYPTED, ATTR_WEBHOOK_ENCRYPTED_DATA, ATTR_WEBHOOK_TYPE, - CONF_SECRET, DOMAIN, WEBHOOK_PAYLOAD_SCHEMA, - WEBHOOK_SCHEMAS, WEBHOOK_TYPE_CALL_SERVICE, - WEBHOOK_TYPE_FIRE_EVENT, WEBHOOK_TYPE_RENDER_TEMPLATE, - WEBHOOK_TYPE_UPDATE_LOCATION, + CONF_SECRET, DOMAIN, ERR_RENDER_FAILURE, + WEBHOOK_PAYLOAD_SCHEMA, WEBHOOK_SCHEMAS, + WEBHOOK_TYPE_CALL_SERVICE, WEBHOOK_TYPE_FIRE_EVENT, + WEBHOOK_TYPE_RENDER_TEMPLATE, WEBHOOK_TYPE_UPDATE_LOCATION, WEBHOOK_TYPE_UPDATE_REGISTRATION) -from .helpers import (_decrypt_payload, empty_okay_response, +from .helpers import (_decrypt_payload, empty_okay_response, error_response, registration_context, safe_registration, savable_state, webhook_response) @@ -135,9 +135,7 @@ async def handle_webhook(store: Store, hass: HomeAssistantType, _LOGGER.error("Error when rendering template during mobile_app " "webhook (device name: %s): %s", registration[ATTR_DEVICE_NAME], ex) - return webhook_response(({"error": str(ex)}), - status=HTTP_BAD_REQUEST, - registration=registration, headers=headers) + return error_response(ERR_RENDER_FAILURE, str(ex), headers=headers) if webhook_type == WEBHOOK_TYPE_UPDATE_LOCATION: try: diff --git a/tests/components/mobile_app/const.py b/tests/components/mobile_app/const.py index 63b37932104..919a2a6e1fb 100644 --- a/tests/components/mobile_app/const.py +++ b/tests/components/mobile_app/const.py @@ -28,6 +28,7 @@ REGISTER = { 'device_name': 'Test 1', 'manufacturer': 'mobile_app', 'model': 'Test', + 'os_name': 'Linux', 'os_version': '1.0', 'supports_encryption': True } @@ -40,6 +41,7 @@ REGISTER_CLEARTEXT = { 'device_name': 'Test 1', 'manufacturer': 'mobile_app', 'model': 'Test', + 'os_name': 'Linux', 'os_version': '1.0', 'supports_encryption': False } diff --git a/tests/components/mobile_app/test_http_api.py b/tests/components/mobile_app/test_http_api.py index 3ff93bdfa75..195d33e15b2 100644 --- a/tests/components/mobile_app/test_http_api.py +++ b/tests/components/mobile_app/test_http_api.py @@ -63,3 +63,28 @@ async def test_registration(hass_client, authed_api_client): # noqa: F811 decrypted_data = decrypted_data.decode("utf-8") assert json.loads(decrypted_data) == {'rendered': 'Hello world'} + + +async def test_register_invalid_component(authed_api_client): # noqa: F811 + """Test that registration with invalid component fails.""" + resp = await authed_api_client.post( + '/api/mobile_app/registrations', json={ + 'app_component': 'will_never_be_valid', + 'app_data': {'foo': 'bar'}, + 'app_id': 'io.homeassistant.mobile_app_test', + 'app_name': 'Mobile App Tests', + 'app_version': '1.0.0', + 'device_name': 'Test 1', + 'manufacturer': 'mobile_app', + 'model': 'Test', + 'os_name': 'Linux', + 'os_version': '1.0', + 'supports_encryption': True + } + ) + + assert resp.status == 400 + register_json = await resp.json() + assert 'error' in register_json + assert register_json['success'] is False + assert register_json['error']['code'] == 'invalid_component'