From 7872f87dd74fb4e2b610bb589facc0f763f153ae Mon Sep 17 00:00:00 2001 From: Paulus Schoutsen Date: Wed, 5 Jan 2022 23:45:02 -0800 Subject: [PATCH] Allow registering a webhook as local only (#63516) --- .../components/google_assistant/helpers.py | 1 + homeassistant/components/webhook/__init__.py | 29 ++++++- tests/components/webhook/test_init.py | 78 +++++++++++++++---- 3 files changed, 90 insertions(+), 18 deletions(-) diff --git a/homeassistant/components/google_assistant/helpers.py b/homeassistant/components/google_assistant/helpers.py index f0fb1f0f3ac..667871a3473 100644 --- a/homeassistant/components/google_assistant/helpers.py +++ b/homeassistant/components/google_assistant/helpers.py @@ -288,6 +288,7 @@ class AbstractConfig(ABC): "Local Support for " + user_agent_id, webhook_id, self._handle_local_webhook, + local_only=True, ) setup_webhook_ids.append(webhook_id) except ValueError: diff --git a/homeassistant/components/webhook/__init__.py b/homeassistant/components/webhook/__init__.py index c16d1b93015..645af24bce2 100644 --- a/homeassistant/components/webhook/__init__.py +++ b/homeassistant/components/webhook/__init__.py @@ -3,6 +3,7 @@ from __future__ import annotations from collections.abc import Awaitable, Callable from http import HTTPStatus +from ipaddress import ip_address import logging import secrets @@ -15,6 +16,7 @@ from homeassistant.core import HomeAssistant, callback from homeassistant.helpers.network import get_url from homeassistant.helpers.typing import ConfigType from homeassistant.loader import bind_hass +from homeassistant.util import network from homeassistant.util.aiohttp import MockRequest _LOGGER = logging.getLogger(__name__) @@ -38,6 +40,8 @@ def async_register( name: str, webhook_id: str, handler: Callable[[HomeAssistant, str, Request], Awaitable[Response | None]], + *, + local_only=False, ) -> None: """Register a webhook.""" handlers = hass.data.setdefault(DOMAIN, {}) @@ -45,7 +49,12 @@ def async_register( if webhook_id in handlers: raise ValueError("Handler is already defined!") - handlers[webhook_id] = {"domain": domain, "name": name, "handler": handler} + handlers[webhook_id] = { + "domain": domain, + "name": name, + "handler": handler, + "local_only": local_only, + } @callback @@ -101,6 +110,17 @@ async def async_handle_webhook(hass, webhook_id, request): _LOGGER.debug("%s", content) return Response(status=HTTPStatus.OK) + if webhook["local_only"]: + try: + remote = ip_address(request.remote) + except ValueError: + _LOGGER.debug("Unable to parse remote ip %s", request.remote) + return Response(status=HTTPStatus.OK) + + if not network.is_local(remote): + _LOGGER.warning("Received remote request for local webhook %s", webhook_id) + return Response(status=HTTPStatus.OK) + try: response = await webhook["handler"](hass, webhook_id, request) if response is None: @@ -145,7 +165,12 @@ def websocket_list(hass, connection, msg): """Return a list of webhooks.""" handlers = hass.data.setdefault(DOMAIN, {}) result = [ - {"webhook_id": webhook_id, "domain": info["domain"], "name": info["name"]} + { + "webhook_id": webhook_id, + "domain": info["domain"], + "name": info["name"], + "local_only": info["local_only"], + } for webhook_id, info in handlers.items() ] diff --git a/tests/components/webhook/test_init.py b/tests/components/webhook/test_init.py index f00c20af74a..5114bab5634 100644 --- a/tests/components/webhook/test_init.py +++ b/tests/components/webhook/test_init.py @@ -1,8 +1,11 @@ """Test the webhook component.""" from http import HTTPStatus +from ipaddress import ip_address +from unittest.mock import patch import pytest +from homeassistant.components import webhook from homeassistant.config import async_process_ha_core_config from homeassistant.setup import async_setup_component @@ -17,19 +20,19 @@ def mock_client(hass, hass_client): async def test_unregistering_webhook(hass, mock_client): """Test unregistering a webhook.""" hooks = [] - webhook_id = hass.components.webhook.async_generate_id() + webhook_id = webhook.async_generate_id() async def handle(*args): """Handle webhook.""" hooks.append(args) - hass.components.webhook.async_register("test", "Test hook", webhook_id, handle) + webhook.async_register(hass, "test", "Test hook", webhook_id, handle) resp = await mock_client.post(f"/api/webhook/{webhook_id}") assert resp.status == HTTPStatus.OK assert len(hooks) == 1 - hass.components.webhook.async_unregister(webhook_id) + webhook.async_unregister(hass, webhook_id) resp = await mock_client.post(f"/api/webhook/{webhook_id}") assert resp.status == HTTPStatus.OK @@ -42,14 +45,14 @@ async def test_generate_webhook_url(hass): hass, {"external_url": "https://example.com"}, ) - url = hass.components.webhook.async_generate_url("some_id") + url = webhook.async_generate_url(hass, "some_id") assert url == "https://example.com/api/webhook/some_id" async def test_async_generate_path(hass): """Test generating just the path component of the url correctly.""" - path = hass.components.webhook.async_generate_path("some_id") + path = webhook.async_generate_path("some_id") assert path == "/api/webhook/some_id" @@ -61,7 +64,7 @@ async def test_posting_webhook_nonexisting(hass, mock_client): async def test_posting_webhook_invalid_json(hass, mock_client): """Test posting to a nonexisting webhook.""" - hass.components.webhook.async_register("test", "Test hook", "hello", None) + webhook.async_register(hass, "test", "Test hook", "hello", None) resp = await mock_client.post("/api/webhook/hello", data="not-json") assert resp.status == HTTPStatus.OK @@ -69,13 +72,13 @@ async def test_posting_webhook_invalid_json(hass, mock_client): async def test_posting_webhook_json(hass, mock_client): """Test posting a webhook with JSON data.""" hooks = [] - webhook_id = hass.components.webhook.async_generate_id() + webhook_id = webhook.async_generate_id() async def handle(*args): """Handle webhook.""" hooks.append((args[0], args[1], await args[2].text())) - hass.components.webhook.async_register("test", "Test hook", webhook_id, handle) + webhook.async_register(hass, "test", "Test hook", webhook_id, handle) resp = await mock_client.post(f"/api/webhook/{webhook_id}", json={"data": True}) assert resp.status == HTTPStatus.OK @@ -88,13 +91,13 @@ async def test_posting_webhook_json(hass, mock_client): async def test_posting_webhook_no_data(hass, mock_client): """Test posting a webhook with no data.""" hooks = [] - webhook_id = hass.components.webhook.async_generate_id() + webhook_id = webhook.async_generate_id() async def handle(*args): """Handle webhook.""" hooks.append(args) - hass.components.webhook.async_register("test", "Test hook", webhook_id, handle) + webhook.async_register(hass, "test", "Test hook", webhook_id, handle) resp = await mock_client.post(f"/api/webhook/{webhook_id}") assert resp.status == HTTPStatus.OK @@ -108,13 +111,13 @@ async def test_posting_webhook_no_data(hass, mock_client): async def test_webhook_put(hass, mock_client): """Test sending a put request to a webhook.""" hooks = [] - webhook_id = hass.components.webhook.async_generate_id() + webhook_id = webhook.async_generate_id() async def handle(*args): """Handle webhook.""" hooks.append(args) - hass.components.webhook.async_register("test", "Test hook", webhook_id, handle) + webhook.async_register(hass, "test", "Test hook", webhook_id, handle) resp = await mock_client.put(f"/api/webhook/{webhook_id}") assert resp.status == HTTPStatus.OK @@ -127,13 +130,13 @@ async def test_webhook_put(hass, mock_client): async def test_webhook_head(hass, mock_client): """Test sending a head request to a webhook.""" hooks = [] - webhook_id = hass.components.webhook.async_generate_id() + webhook_id = webhook.async_generate_id() async def handle(*args): """Handle webhook.""" hooks.append(args) - hass.components.webhook.async_register("test", "Test hook", webhook_id, handle) + webhook.async_register(hass, "test", "Test hook", webhook_id, handle) resp = await mock_client.head(f"/api/webhook/{webhook_id}") assert resp.status == HTTPStatus.OK @@ -143,6 +146,37 @@ async def test_webhook_head(hass, mock_client): assert hooks[0][2].method == "HEAD" +async def test_webhook_local_only(hass, mock_client): + """Test posting a webhook with local only.""" + hooks = [] + webhook_id = webhook.async_generate_id() + + async def handle(*args): + """Handle webhook.""" + hooks.append((args[0], args[1], await args[2].text())) + + webhook.async_register( + hass, "test", "Test hook", webhook_id, handle, local_only=True + ) + + resp = await mock_client.post(f"/api/webhook/{webhook_id}", json={"data": True}) + assert resp.status == HTTPStatus.OK + assert len(hooks) == 1 + assert hooks[0][0] is hass + assert hooks[0][1] == webhook_id + assert hooks[0][2] == '{"data": true}' + + # Request from remote IP + with patch( + "homeassistant.components.webhook.ip_address", + return_value=ip_address("123.123.123.123"), + ): + resp = await mock_client.post(f"/api/webhook/{webhook_id}", json={"data": True}) + assert resp.status == HTTPStatus.OK + # No hook received + assert len(hooks) == 1 + + async def test_listing_webhook( hass, hass_ws_client, hass_access_token, enable_custom_integrations ): @@ -150,7 +184,8 @@ async def test_listing_webhook( assert await async_setup_component(hass, "webhook", {}) client = await hass_ws_client(hass, hass_access_token) - hass.components.webhook.async_register("test", "Test hook", "my-id", None) + webhook.async_register(hass, "test", "Test hook", "my-id", None) + webhook.async_register(hass, "test", "Test hook", "my-2", None, local_only=True) await client.send_json({"id": 5, "type": "webhook/list"}) @@ -158,5 +193,16 @@ async def test_listing_webhook( assert msg["id"] == 5 assert msg["success"] assert msg["result"] == [ - {"webhook_id": "my-id", "domain": "test", "name": "Test hook"} + { + "webhook_id": "my-id", + "domain": "test", + "name": "Test hook", + "local_only": False, + }, + { + "webhook_id": "my-2", + "domain": "test", + "name": "Test hook", + "local_only": True, + }, ]