From 1d51e56272105d281ee7edb1403e90f3c5cc71d9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Bachmann?= Date: Fri, 4 Sep 2026 13:07:21 +0200 Subject: [PATCH] Harden bridge state and reauthentication handling --- .../mb_secure/alarm_control_panel.py | 2 + custom_components/mb_secure/binary_sensor.py | 6 +- custom_components/mb_secure/config_flow.py | 12 ++- custom_components/mb_secure/entity.py | 7 +- custom_components/mb_secure/switch.py | 6 +- .../mb_secure/translations/en.json | 3 +- tests/test_config_flow.py | 74 ++++++++++++- tests/test_coordinator.py | 66 +++++++++++- tests/test_entities.py | 100 +++++++++++++++++- 9 files changed, 264 insertions(+), 12 deletions(-) diff --git a/custom_components/mb_secure/alarm_control_panel.py b/custom_components/mb_secure/alarm_control_panel.py index 4841c0f..f1818ff 100644 --- a/custom_components/mb_secure/alarm_control_panel.py +++ b/custom_components/mb_secure/alarm_control_panel.py @@ -38,6 +38,8 @@ async def async_setup_entry( class MBSecureAlarmEntity(MBSecureEntity, AlarmControlPanelEntity): """Represent one normalized security area.""" + _attr_code_arm_required = False + def __init__( self, coordinator: MBSecureCoordinator, bridge_id: str, object_id: str ) -> None: diff --git a/custom_components/mb_secure/binary_sensor.py b/custom_components/mb_secure/binary_sensor.py index f1e2a60..7dc02f0 100644 --- a/custom_components/mb_secure/binary_sensor.py +++ b/custom_components/mb_secure/binary_sensor.py @@ -46,4 +46,8 @@ class MBSecurePointEntity(MBSecureEntity, BinarySensorEntity): item = self.bridge_object if item is None: return None - return item.state in {"active", "on", "open", "triggered"} + if item.state in {"active", "on", "open", "triggered"}: + return True + if item.state in {"clear", "closed", "inactive", "off"}: + return False + return None diff --git a/custom_components/mb_secure/config_flow.py b/custom_components/mb_secure/config_flow.py index bc3fdff..4e8afac 100644 --- a/custom_components/mb_secure/config_flow.py +++ b/custom_components/mb_secure/config_flow.py @@ -160,13 +160,23 @@ class MBSecureConfigFlow(ConfigFlow, domain=DOMAIN): if user_input is not None: reauth_entry = self._get_reauth_entry() candidate = {**reauth_entry.data, CONF_TOKEN: user_input[CONF_TOKEN]} - error, _ = await self._async_validate(candidate) + error, info = await self._async_validate(candidate) if error is None: + assert info is not None + if info.bridge_id is None: + errors["base"] = "missing_bridge_id" + return self._show_reauth_form(errors) + await self.async_set_unique_id(info.bridge_id) + self._abort_if_unique_id_mismatch(reason="wrong_account") return self.async_update_reload_and_abort( reauth_entry, data_updates={CONF_TOKEN: user_input[CONF_TOKEN]}, ) errors["base"] = error + return self._show_reauth_form(errors) + + def _show_reauth_form(self, errors: dict[str, str]) -> ConfigFlowResult: + """Show the bridge token replacement form.""" return self.async_show_form( step_id="reauth_confirm", data_schema=vol.Schema( diff --git a/custom_components/mb_secure/entity.py b/custom_components/mb_secure/entity.py index e67923b..082767b 100644 --- a/custom_components/mb_secure/entity.py +++ b/custom_components/mb_secure/entity.py @@ -56,7 +56,12 @@ class MBSecureEntity(CoordinatorEntity[MBSecureCoordinator]): @property def available(self) -> bool: """Report availability from both transport and topology state.""" - return super().available and self.bridge_object is not None + return ( + super().available + and self.bridge_object is not None + and self.coordinator.info is not None + and self.coordinator.info.connected + ) @property def name(self) -> str | None: diff --git a/custom_components/mb_secure/switch.py b/custom_components/mb_secure/switch.py index 6e9edb1..2db37b9 100644 --- a/custom_components/mb_secure/switch.py +++ b/custom_components/mb_secure/switch.py @@ -48,7 +48,11 @@ class MBSecureOutputEntity(MBSecureEntity, SwitchEntity): item = self.bridge_object if item is None: return None - return item.state == "on" + if item.state == "on": + return True + if item.state == "off": + return False + return None async def async_turn_on(self, **kwargs: Any) -> None: """Turn the output on.""" diff --git a/custom_components/mb_secure/translations/en.json b/custom_components/mb_secure/translations/en.json index 7a5acbf..3317c2a 100644 --- a/custom_components/mb_secure/translations/en.json +++ b/custom_components/mb_secure/translations/en.json @@ -36,7 +36,8 @@ }, "abort": { "already_configured": "This bridge is already configured", - "invalid_discovery": "The discovered bridge information is invalid" + "invalid_discovery": "The discovered bridge information is invalid", + "wrong_account": "The token belongs to a different bridge installation" } } } diff --git a/tests/test_config_flow.py b/tests/test_config_flow.py index 0cd049b..2283040 100644 --- a/tests/test_config_flow.py +++ b/tests/test_config_flow.py @@ -2,11 +2,12 @@ from unittest.mock import patch -from homeassistant.config_entries import SOURCE_HASSIO, SOURCE_USER +from homeassistant.config_entries import SOURCE_HASSIO, SOURCE_REAUTH, SOURCE_USER from homeassistant.const import CONF_HOST, CONF_PORT from homeassistant.core import HomeAssistant from homeassistant.data_entry_flow import FlowResultType from homeassistant.helpers.service_info.hassio import HassioServiceInfo +from pytest_homeassistant_custom_component.common import MockConfigEntry from custom_components.mb_secure.api import BridgeAuthenticationError from custom_components.mb_secure.const import CONF_TOKEN, DOMAIN @@ -146,3 +147,74 @@ async def test_hassio_discovery_rejects_incomplete_data( assert result["type"] is FlowResultType.ABORT assert result["reason"] == "invalid_discovery" + + +async def test_reauth_updates_token_for_same_bridge(hass: HomeAssistant) -> None: + """A replacement token is accepted only for the configured bridge.""" + entry = MockConfigEntry( + domain=DOMAIN, + data=USER_INPUT, + unique_id="bridge-installation-1", + ) + entry.add_to_hass(hass) + result = await hass.config_entries.flow.async_init( + DOMAIN, + context={"source": SOURCE_REAUTH, "entry_id": entry.entry_id}, + data=entry.data, + ) + info = BridgeInfo( + bridge_id="bridge-installation-1", + bridge_version="0.1.0", + api_version=1, + connected=True, + capabilities=frozenset(), + ) + new_token = "y" * 32 + + with patch( + "custom_components.mb_secure.api.BridgeClient.async_get_info", + return_value=info, + ): + result = await hass.config_entries.flow.async_configure( + result["flow_id"], {CONF_TOKEN: new_token} + ) + + assert result["type"] is FlowResultType.ABORT + assert result["reason"] == "reauth_successful" + assert entry.data[CONF_TOKEN] == new_token + + +async def test_reauth_rejects_token_for_different_bridge( + hass: HomeAssistant, +) -> None: + """A valid token from another installation cannot replace the token.""" + entry = MockConfigEntry( + domain=DOMAIN, + data=USER_INPUT, + unique_id="bridge-installation-1", + ) + entry.add_to_hass(hass) + result = await hass.config_entries.flow.async_init( + DOMAIN, + context={"source": SOURCE_REAUTH, "entry_id": entry.entry_id}, + data=entry.data, + ) + info = BridgeInfo( + bridge_id="bridge-installation-2", + bridge_version="0.1.0", + api_version=1, + connected=True, + capabilities=frozenset(), + ) + + with patch( + "custom_components.mb_secure.api.BridgeClient.async_get_info", + return_value=info, + ): + result = await hass.config_entries.flow.async_configure( + result["flow_id"], {CONF_TOKEN: "y" * 32} + ) + + assert result["type"] is FlowResultType.ABORT + assert result["reason"] == "wrong_account" + assert entry.data[CONF_TOKEN] == USER_INPUT[CONF_TOKEN] diff --git a/tests/test_coordinator.py b/tests/test_coordinator.py index a0a2719..a2cf3d2 100644 --- a/tests/test_coordinator.py +++ b/tests/test_coordinator.py @@ -1,6 +1,6 @@ """Tests for MB-Secure snapshot and event coordination.""" -from unittest.mock import AsyncMock, MagicMock +from unittest.mock import AsyncMock, MagicMock, patch import pytest from homeassistant.core import HomeAssistant @@ -8,7 +8,10 @@ from homeassistant.exceptions import ConfigEntryAuthFailed from homeassistant.helpers.update_coordinator import UpdateFailed from pytest_homeassistant_custom_component.common import MockConfigEntry -from custom_components.mb_secure.api import BridgeAuthenticationError +from custom_components.mb_secure.api import ( + BridgeAuthenticationError, + BridgeConnectionError, +) from custom_components.mb_secure.const import CONF_TOKEN, DOMAIN from custom_components.mb_secure.coordinator import MBSecureCoordinator from custom_components.mb_secure.models import BridgeInfo, BridgeSnapshot, DomainEvent @@ -91,6 +94,65 @@ async def test_snapshot_rejects_changed_bridge_identity( client.async_get_snapshot.assert_not_awaited() +async def test_bridge_restart_accepts_a_new_snapshot_revision( + hass: HomeAssistant, +) -> None: + """A full snapshot reconciles revision reset after a bridge restart.""" + client = MagicMock() + client.async_get_info = AsyncMock( + return_value=BridgeInfo( + bridge_id="bridge-installation-1", + bridge_version="0.1.0", + api_version=1, + connected=True, + capabilities=frozenset(), + ) + ) + client.async_get_snapshot = AsyncMock(return_value=snapshot(revision=0)) + instance, _ = coordinator(hass, client) + instance.async_set_updated_data(snapshot(revision=20)) + + result = await instance._async_update_data() + + assert result.revision == 0 + + +async def test_event_stream_reconnects_after_connection_loss( + hass: HomeAssistant, +) -> None: + """The event listener reconnects and resumes contiguous updates.""" + client = MagicMock() + attempts = 0 + + async def events(): + nonlocal attempts + attempts += 1 + if attempts == 1: + if False: + yield + raise BridgeConnectionError + yield DomainEvent( + revision=2, + event_type="state_changed", + object_type="area", + object_id="area-1", + state="armed_away", + ) + raise BridgeAuthenticationError + + client.async_events = events + instance, entry = coordinator(hass, client) + instance.async_set_updated_data(snapshot()) + entry.async_start_reauth = MagicMock() + + with patch("custom_components.mb_secure.coordinator.asyncio.sleep") as sleep: + await instance._async_event_loop() + + sleep.assert_awaited_once_with(1) + assert attempts == 2 + assert instance.data.areas["area-1"].state == "armed_away" + + async def test_contiguous_event_updates_coordinator_data( hass: HomeAssistant, ) -> None: diff --git a/tests/test_entities.py b/tests/test_entities.py index c880bde..fa16e48 100644 --- a/tests/test_entities.py +++ b/tests/test_entities.py @@ -1,7 +1,7 @@ """Tests for MB-Secure devices and entities.""" from copy import deepcopy -from unittest.mock import patch +from unittest.mock import AsyncMock, patch from homeassistant.core import HomeAssistant from homeassistant.helpers import device_registry as dr @@ -82,7 +82,12 @@ def snapshot_payload() -> dict[str, object]: } -async def setup_entry(hass: HomeAssistant) -> MockConfigEntry: +async def setup_entry( + hass: HomeAssistant, + *, + connected: bool = True, + payload: dict[str, object] | None = None, +) -> MockConfigEntry: """Set up an entry against mocked neutral Bridge API responses.""" entry = MockConfigEntry( domain=DOMAIN, @@ -94,10 +99,10 @@ async def setup_entry(hass: HomeAssistant) -> MockConfigEntry: bridge_id="bridge-installation-1", bridge_version="0.1.0", api_version=1, - connected=True, + connected=connected, capabilities=frozenset({"snapshot", "events", "commands"}), ) - snapshot = BridgeSnapshot.from_dict(snapshot_payload()) + snapshot = BridgeSnapshot.from_dict(payload or snapshot_payload()) with ( patch( "custom_components.mb_secure.api.BridgeClient.async_get_info", @@ -190,3 +195,90 @@ async def test_snapshot_topology_changes_add_and_remove_entities( assert "bridge-installation-1:points:point-1" not in unique_ids assert "bridge-installation-1:points:point-2" in unique_ids assert hass.states.get(old_entry) is None + + +async def test_panel_connection_loss_marks_all_entities_unavailable( + hass: HomeAssistant, +) -> None: + """A reachable bridge does not hide loss of its panel connection.""" + entry = await setup_entry(hass, connected=False) + registry = er.async_get(hass) + + for entity in er.async_entries_for_config_entry(registry, entry.entry_id): + state = hass.states.get(entity.entity_id) + assert state is not None + assert state.state == "unavailable" + + +async def test_unknown_binary_states_remain_unknown(hass: HomeAssistant) -> None: + """Unknown neutral states are never converted into a false clear state.""" + payload = snapshot_payload() + points = payload["points"] + outputs = payload["outputs"] + assert isinstance(points, list) and isinstance(outputs, list) + points[0]["state"] = "future_state" + outputs[0]["state"] = "future_state" + entry = await setup_entry(hass, payload=payload) + registry = er.async_get(hass) + + for unique_id in ( + "bridge-installation-1:points:point-1", + "bridge-installation-1:outputs:output-1", + ): + entity_id = next( + entity.entity_id + for entity in er.async_entries_for_config_entry(registry, entry.entry_id) + if entity.unique_id == unique_id + ) + state = hass.states.get(entity_id) + assert state is not None + assert state.state == "unknown" + + +async def test_entity_services_use_scoped_bridge_commands( + hass: HomeAssistant, +) -> None: + """Entity services send only normalized commands for their own object IDs.""" + entry = await setup_entry(hass) + coordinator = entry.runtime_data + coordinator.client.async_arm_area = AsyncMock() + coordinator.client.async_disarm_area = AsyncMock() + coordinator.client.async_set_output = AsyncMock() + coordinator.async_request_refresh = AsyncMock() + registry = er.async_get(hass) + + area_entity_id = registry.async_get_entity_id( + "alarm_control_panel", + DOMAIN, + "bridge-installation-1:areas:area-1", + ) + output_entity_id = registry.async_get_entity_id( + "switch", + DOMAIN, + "bridge-installation-1:outputs:output-1", + ) + assert area_entity_id is not None and output_entity_id is not None + + await hass.services.async_call( + "alarm_control_panel", + "alarm_arm_away", + {"entity_id": area_entity_id}, + blocking=True, + ) + await hass.services.async_call( + "alarm_control_panel", + "alarm_disarm", + {"entity_id": area_entity_id}, + blocking=True, + ) + await hass.services.async_call( + "switch", + "turn_on", + {"entity_id": output_entity_id}, + blocking=True, + ) + + coordinator.client.async_arm_area.assert_awaited_once_with("area-1", "away") + coordinator.client.async_disarm_area.assert_awaited_once_with("area-1") + coordinator.client.async_set_output.assert_awaited_once_with("output-1", "on") + assert coordinator.async_request_refresh.await_count == 3