fix(review): Disable stale routes and validate credentials
SchiZzA
parent
2b266f33b4
commit
0dfdd6c9e9
|
|
@ -160,6 +160,7 @@ def register_path(
|
|||
sticky=True,
|
||||
)
|
||||
else:
|
||||
routes.activate()
|
||||
routes.set_ingress_observer(coordinator_h.record_dispatch)
|
||||
_LOGGER.info("We have already registered routes: %s", routes.show_enabled())
|
||||
return True
|
||||
|
|
@ -195,6 +196,7 @@ async def async_setup_entry(hass: HomeAssistant, entry: SWSConfigEntry) -> bool:
|
|||
|
||||
if routes is not None:
|
||||
_LOGGER.debug("We have routes registered, will try to switch dispatcher.")
|
||||
routes.activate()
|
||||
routes.switch_route(coordinator.received_data, DEFAULT_URL if not _wslink else WSLINK_URL, enabled=_legacy)
|
||||
routes.set_ecowitt_enabled(_ecowitt_path, coordinator.received_ecowitt_data, _ecowitt_enabled)
|
||||
# Rebind the sticky health route to the new coordinator so /station/health
|
||||
|
|
@ -270,4 +272,13 @@ async def async_unload_entry(hass: HomeAssistant, entry: SWSConfigEntry) -> bool
|
|||
aiohttp routes stay registered and the dispatcher is re-wired on the next setup.
|
||||
"""
|
||||
|
||||
return await hass.config_entries.async_unload_platforms(entry, PLATFORMS)
|
||||
unload_ok = await hass.config_entries.async_unload_platforms(entry, PLATFORMS)
|
||||
|
||||
if unload_ok:
|
||||
domain_data = hass.data.get(DOMAIN)
|
||||
routes = domain_data.get("routes") if isinstance(domain_data, dict) else None
|
||||
if isinstance(routes, Routes):
|
||||
routes.deactivate()
|
||||
setattr(entry, "runtime_data", None)
|
||||
|
||||
return unload_ok
|
||||
|
|
|
|||
|
|
@ -67,6 +67,17 @@ def _ha_url_placeholders(hass: HomeAssistant) -> tuple[str, str]:
|
|||
return url.host or "UNKNOWN", str(url.port)
|
||||
|
||||
|
||||
def _validate_ecowitt_webhook(user_input: dict[str, Any], errors: dict[str, str]) -> None:
|
||||
"""Reject enabling Ecowitt without a webhook id.
|
||||
|
||||
The id is the endpoint's only credential, so an empty one would leave
|
||||
`/weatherhub/<id>` unauthenticated (`received_ecowitt_data` rejects it outright).
|
||||
"""
|
||||
webhook = user_input.get(ECOWITT_WEBHOOK_ID, "")
|
||||
if user_input.get(ECOWITT_ENABLED) and (not isinstance(webhook, str) or not webhook.strip()):
|
||||
errors[ECOWITT_WEBHOOK_ID] = "ecowitt_webhook_required"
|
||||
|
||||
|
||||
class ConfigOptionsFlowHandler(OptionsFlow):
|
||||
"""Handle WeatherStation ConfigFlow."""
|
||||
|
||||
|
|
@ -276,11 +287,12 @@ class ConfigOptionsFlowHandler(OptionsFlow):
|
|||
webhook = secrets.token_hex(8)
|
||||
|
||||
if user_input is not None:
|
||||
_validate_ecowitt_webhook(user_input, errors)
|
||||
# Both endpoints remap onto the same internal sensor keys, so enabling
|
||||
# Ecowitt while the legacy endpoint is active would corrupt those entities.
|
||||
if user_input.get(ECOWITT_ENABLED) and self.user_data.get(LEGACY_ENABLED):
|
||||
errors["base"] = ERROR_MUTUALLY_EXCLUSIVE
|
||||
else:
|
||||
if not errors:
|
||||
return self.async_create_entry(title=DOMAIN, data=self.retain_data(user_input))
|
||||
|
||||
host, port = _ha_url_placeholders(self.hass)
|
||||
|
|
@ -405,33 +417,39 @@ class ConfigFlowHandler(ConfigFlow, domain=DOMAIN):
|
|||
async def async_step_ecowitt(self, user_input: Any = None) -> ConfigFlowResult:
|
||||
"""Ecowitt stations setup."""
|
||||
|
||||
if user_input is None:
|
||||
webhook = secrets.token_hex(8)
|
||||
host, port = _ha_url_placeholders(self.hass)
|
||||
errors: dict[str, str] = {}
|
||||
|
||||
ecowitt_schema = {
|
||||
vol.Required(ECOWITT_WEBHOOK_ID, default=webhook): str,
|
||||
vol.Optional(ECOWITT_ENABLED, default=True): bool,
|
||||
}
|
||||
if user_input is not None:
|
||||
_validate_ecowitt_webhook(user_input, errors)
|
||||
if not errors:
|
||||
options: dict[str, Any] = {
|
||||
**user_input,
|
||||
LEGACY_ENABLED: False,
|
||||
WSLINK: False,
|
||||
API_ID: "",
|
||||
API_KEY: "",
|
||||
}
|
||||
|
||||
return self.async_show_form(
|
||||
step_id="ecowitt",
|
||||
data_schema=vol.Schema(ecowitt_schema),
|
||||
description_placeholders={
|
||||
"url": host,
|
||||
"port": port,
|
||||
"webhook_id": webhook,
|
||||
},
|
||||
)
|
||||
options: dict[str, Any] = {
|
||||
**user_input,
|
||||
LEGACY_ENABLED: False,
|
||||
WSLINK: False,
|
||||
API_ID: "",
|
||||
API_KEY: "",
|
||||
return self.async_create_entry(title=DOMAIN, data=options, options=options)
|
||||
|
||||
webhook = user_input.get(ECOWITT_WEBHOOK_ID, "") if user_input is not None else secrets.token_hex(8)
|
||||
host, port = _ha_url_placeholders(self.hass)
|
||||
|
||||
ecowitt_schema = {
|
||||
vol.Required(ECOWITT_WEBHOOK_ID, default=webhook): str,
|
||||
vol.Optional(ECOWITT_ENABLED, default=True): bool,
|
||||
}
|
||||
|
||||
return self.async_create_entry(title=DOMAIN, data=options, options=options)
|
||||
return self.async_show_form(
|
||||
step_id="ecowitt",
|
||||
data_schema=vol.Schema(ecowitt_schema),
|
||||
description_placeholders={
|
||||
"url": host,
|
||||
"port": port,
|
||||
"webhook_id": webhook,
|
||||
},
|
||||
errors=errors,
|
||||
)
|
||||
|
||||
@staticmethod
|
||||
@callback
|
||||
|
|
|
|||
|
|
@ -63,6 +63,16 @@ class Routes:
|
|||
"""Initialize dispatcher storage."""
|
||||
self.routes: dict[str, RouteInfo] = {}
|
||||
self._ingress_observer: IngressObserver | None = None
|
||||
self.active: bool = True
|
||||
|
||||
def activate(self) -> None:
|
||||
"""Allow registered routes to dispatch to their configured handlers."""
|
||||
self.active = True
|
||||
|
||||
def deactivate(self) -> None:
|
||||
"""Stop registered routes from dispatching to config-entry handlers."""
|
||||
self.active = False
|
||||
self._ingress_observer = None
|
||||
|
||||
def _resolve_route(self, request: Request) -> RouteInfo | None:
|
||||
"""Find the matching RouteInfo for a request.
|
||||
|
|
@ -141,6 +151,12 @@ class Routes:
|
|||
self._ingress_observer(request, False, "route_not_registered")
|
||||
return await unregistered(request)
|
||||
|
||||
if not self.active:
|
||||
_LOGGER.debug("Route (%s):%s received while integration is not loaded.", request.method, request.path)
|
||||
if self._ingress_observer is not None:
|
||||
self._ingress_observer(request, False, "integration_unloaded")
|
||||
return Response(text="Integration is not loaded.", status=503)
|
||||
|
||||
if self._ingress_observer is not None:
|
||||
self._ingress_observer(
|
||||
request,
|
||||
|
|
@ -196,6 +212,9 @@ class Routes:
|
|||
def show_enabled(self) -> str:
|
||||
"""Return a human-readable description of the currently enabled route."""
|
||||
|
||||
if not self.active:
|
||||
return "No routes are enabled."
|
||||
|
||||
enabled_routes = {
|
||||
f"Dispatcher enabled for ({route.route.method}):{route.url_path}, with handler: {route.handler}"
|
||||
for route in self.routes.values()
|
||||
|
|
@ -207,7 +226,7 @@ class Routes:
|
|||
|
||||
def path_enabled(self, url_path: str) -> bool:
|
||||
"""Return whether any route registered for `url_path` is enabled."""
|
||||
return any(route.enabled for route in self.routes.values() if route.url_path == url_path)
|
||||
return self.active and any(route.enabled for route in self.routes.values() if route.url_path == url_path)
|
||||
|
||||
def snapshot(self) -> dict[str, Any]:
|
||||
"""Return a compact routing snapshot for diagnostics."""
|
||||
|
|
@ -215,7 +234,7 @@ class Routes:
|
|||
key: {
|
||||
"path": route.url_path,
|
||||
"method": route.route.method,
|
||||
"enabled": route.enabled,
|
||||
"enabled": self.active and route.enabled,
|
||||
"sticky": route.sticky,
|
||||
}
|
||||
for key, route in self.routes.items()
|
||||
|
|
|
|||
|
|
@ -4,6 +4,7 @@
|
|||
"valid_credentials_api": "Provide valid API ID.",
|
||||
"valid_credentials_key": "Provide valid API KEY.",
|
||||
"valid_credentials_match": "API ID and API KEY should not be the same.",
|
||||
"ecowitt_webhook_required": "Provide a webhook ID before enabling Ecowitt data.",
|
||||
"protocols_mutually_exclusive": "The legacy (PWS/WSLink) and Ecowitt endpoints cannot be enabled at the same time - they feed the same sensor entities, which would mix up units and blank readings. Disable one of them first."
|
||||
},
|
||||
"step": {
|
||||
|
|
@ -50,6 +51,7 @@
|
|||
"valid_credentials_api": "Provide valid API ID.",
|
||||
"valid_credentials_key": "Provide valid API KEY.",
|
||||
"valid_credentials_match": "API ID and API KEY should not be the same.",
|
||||
"ecowitt_webhook_required": "Provide a webhook ID before enabling Ecowitt data.",
|
||||
"windy_id_required": "Windy API ID is required if you want to enable this function.",
|
||||
"windy_pw_required": "Windy API password is required if you want to enable this function.",
|
||||
"windy_key_required": "Windy station ID and password are required if you want to enable this function.",
|
||||
|
|
|
|||
|
|
@ -1 +0,0 @@
|
|||
../dev/custom_components/sws12500
|
||||
|
|
@ -4,6 +4,7 @@
|
|||
"valid_credentials_api": "Vyplňte platné API ID.",
|
||||
"valid_credentials_key": "Vyplňte platný API KEY.",
|
||||
"valid_credentials_match": "API ID a API KEY nesmějí být stejné!",
|
||||
"ecowitt_webhook_required": "Před zapnutím dat Ecowitt zadejte webhook ID.",
|
||||
"protocols_mutually_exclusive": "Starý endpoint (PWS/WSLink) a Ecowitt nelze zapnout současně – plní stejné entity senzorů, což by pomíchalo jednotky a mazalo naměřené hodnoty. Nejdřív jeden z nich vypni."
|
||||
},
|
||||
"step": {
|
||||
|
|
@ -50,6 +51,7 @@
|
|||
"valid_credentials_api": "Vyplňte platné API ID",
|
||||
"valid_credentials_key": "Vyplňte platný API KEY",
|
||||
"valid_credentials_match": "API ID a API KEY nesmějí být stejné!",
|
||||
"ecowitt_webhook_required": "Před zapnutím dat Ecowitt zadejte webhook ID.",
|
||||
"windy_id_required": "Je vyžadováno Windy ID, pokud chcete aktivovat přeposílání dat na Windy",
|
||||
"windy_pw_required": "Je vyžadován Windy KEY, pokud chcete aktivovat přeposílání dat na Windy",
|
||||
"windy_key_required": "Pro aktivaci je vyžadováno Windy ID i heslo stanice.",
|
||||
|
|
|
|||
|
|
@ -4,6 +4,7 @@
|
|||
"valid_credentials_api": "Provide valid API ID.",
|
||||
"valid_credentials_key": "Provide valid API KEY.",
|
||||
"valid_credentials_match": "API ID and API KEY should not be the same.",
|
||||
"ecowitt_webhook_required": "Provide a webhook ID before enabling Ecowitt data.",
|
||||
"protocols_mutually_exclusive": "The legacy (PWS/WSLink) and Ecowitt endpoints cannot be enabled at the same time - they feed the same sensor entities, which would mix up units and blank readings. Disable one of them first."
|
||||
},
|
||||
"step": {
|
||||
|
|
@ -50,6 +51,7 @@
|
|||
"valid_credentials_api": "Provide valid API ID.",
|
||||
"valid_credentials_key": "Provide valid API KEY.",
|
||||
"valid_credentials_match": "API ID and API KEY should not be the same.",
|
||||
"ecowitt_webhook_required": "Provide a webhook ID before enabling Ecowitt data.",
|
||||
"windy_id_required": "Windy API ID is required if you want to enable this function.",
|
||||
"windy_pw_required": "Windy API password is required if you want to enable this function.",
|
||||
"windy_key_required": "Windy station ID and password are required if you want to enable this function.",
|
||||
|
|
|
|||
|
|
@ -233,6 +233,35 @@ async def test_config_flow_user_invalid_credentials_match(
|
|||
assert result2["errors"]["base"] == "valid_credentials_match"
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
("user_input", "field", "error"),
|
||||
[
|
||||
({API_ID: "", API_KEY: "ok_key", WSLINK: False, DEV_DBG: False}, API_ID, "valid_credentials_api"),
|
||||
({API_ID: "ok_id", API_KEY: "", WSLINK: False, DEV_DBG: False}, API_KEY, "valid_credentials_key"),
|
||||
],
|
||||
)
|
||||
@pytest.mark.asyncio
|
||||
async def test_config_flow_user_rejects_empty_pws_credentials(
|
||||
hass,
|
||||
enable_custom_integrations,
|
||||
user_input,
|
||||
field,
|
||||
error,
|
||||
) -> None:
|
||||
result = await hass.config_entries.flow.async_init(
|
||||
DOMAIN, context={"source": config_entries.SOURCE_USER}
|
||||
)
|
||||
form = await hass.config_entries.flow.async_configure(
|
||||
result["flow_id"], user_input={"next_step_id": "pws"}
|
||||
)
|
||||
|
||||
result2 = await hass.config_entries.flow.async_configure(form["flow_id"], user_input=user_input)
|
||||
|
||||
assert result2["type"] == "form"
|
||||
assert result2["step_id"] == "pws"
|
||||
assert result2["errors"][field] == error
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_options_flow_init_menu(hass, enable_custom_integrations) -> None:
|
||||
"""Options flow shows menu with expected steps."""
|
||||
|
|
@ -475,6 +504,16 @@ async def test_options_flow_ecowitt_uses_get_url_placeholders_and_webhook_defaul
|
|||
assert placeholders["port"] == "8123"
|
||||
assert placeholders["webhook_id"] # generated
|
||||
|
||||
bad = await hass.config_entries.options.async_configure(
|
||||
init["flow_id"],
|
||||
user_input={
|
||||
ECOWITT_WEBHOOK_ID: "",
|
||||
ECOWITT_ENABLED: True,
|
||||
},
|
||||
)
|
||||
assert bad["type"] == "form"
|
||||
assert bad["errors"][ECOWITT_WEBHOOK_ID] == "ecowitt_webhook_required"
|
||||
|
||||
done = await hass.config_entries.options.async_configure(
|
||||
init["flow_id"],
|
||||
user_input={
|
||||
|
|
@ -594,6 +633,16 @@ async def test_config_flow_ecowitt_initial_setup(hass, enable_custom_integration
|
|||
assert placeholders["url"] == "example.local"
|
||||
assert placeholders["webhook_id"]
|
||||
|
||||
bad = await hass.config_entries.flow.async_configure(
|
||||
form["flow_id"],
|
||||
user_input={
|
||||
ECOWITT_WEBHOOK_ID: "",
|
||||
ECOWITT_ENABLED: True,
|
||||
},
|
||||
)
|
||||
assert bad["type"] == "form"
|
||||
assert bad["errors"][ECOWITT_WEBHOOK_ID] == "ecowitt_webhook_required"
|
||||
|
||||
done = await hass.config_entries.flow.async_configure(
|
||||
form["flow_id"],
|
||||
user_input={
|
||||
|
|
|
|||
|
|
@ -369,6 +369,35 @@ async def test_async_unload_entry_returns_true_on_success(hass_with_http):
|
|||
hass_with_http.config_entries.async_unload_platforms.assert_awaited_once()
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_async_unload_entry_deactivates_shared_routes(hass_with_http):
|
||||
entry = MockConfigEntry(domain=DOMAIN, data={}, options={API_ID: "id", API_KEY: "key"})
|
||||
entry.add_to_hass(hass_with_http)
|
||||
|
||||
coordinator = WeatherDataUpdateCoordinator(hass_with_http, entry)
|
||||
coordinator_health = HealthCoordinator(hass_with_http, entry)
|
||||
register_path(hass_with_http, coordinator, coordinator_health, entry)
|
||||
entry.runtime_data = SWSRuntimeData(
|
||||
coordinator=coordinator,
|
||||
health_coordinator=coordinator_health,
|
||||
last_options=dict(entry.options),
|
||||
)
|
||||
|
||||
routes = hass_with_http.data[DOMAIN]["routes"]
|
||||
assert routes.path_enabled(DEFAULT_URL) is True
|
||||
|
||||
hass_with_http.config_entries.async_unload_platforms = AsyncMock(return_value=True)
|
||||
|
||||
ok = await async_unload_entry(hass_with_http, entry)
|
||||
|
||||
assert ok is True
|
||||
assert routes.path_enabled(DEFAULT_URL) is False
|
||||
assert getattr(entry, "runtime_data", None) is None
|
||||
|
||||
response = await routes.dispatch(SimpleNamespace(method="GET", path=DEFAULT_URL))
|
||||
assert response.status == 503
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_async_unload_entry_returns_false_on_failure(hass_with_http):
|
||||
entry = MockConfigEntry(domain=DOMAIN, data={}, options={API_ID: "id", API_KEY: "key"})
|
||||
|
|
|
|||
Loading…
Reference in New Issue