Fix foscam to work again with non-admin accounts and make RTSP port configurable again (#45975)

* Do not require admin account for foscam cameras.

Foscam cameras require admin account for getting the MAC address,
requiring an admin account in the integration is not desirable as
an operator one is good enough (and a good practice).

Old entries using the MAC address as unique_id are migrated to the
new unique_id format so everything is consistent.

Also fixed unhandled invalid responses from the camera in the
config flow process.

* Make RTSP port configurable again as some cameras reports wrong port

* Remove periods from new log lines

* Set new Config Flow version to 2 and adjust the entity migration

* Create a proper error message for the InvalidResponse exception

* Change crafted unique_id to use entry_id in the entity

* Abort if same host and port is already configured

* Fix entry tracking to use entry_id instead of unique_id

* Remove unique_id from mocked config entry in tests

homeassistant/components/foscam/__init__.py

@@ -1,10 +1,15 @@
 """The foscam component."""
 import asyncio
 
-from homeassistant.config_entries import ConfigEntry
+from libpyfoscam import FoscamCamera
-from homeassistant.core import HomeAssistant
 
-from .const import DOMAIN, SERVICE_PTZ, SERVICE_PTZ_PRESET
+from homeassistant.config_entries import ConfigEntry
+from homeassistant.const import CONF_HOST, CONF_PASSWORD, CONF_PORT, CONF_USERNAME
+from homeassistant.core import HomeAssistant, callback
+from homeassistant.helpers.entity_registry import async_migrate_entries
+
+from .config_flow import DEFAULT_RTSP_PORT
+from .const import CONF_RTSP_PORT, DOMAIN, LOGGER, SERVICE_PTZ, SERVICE_PTZ_PRESET
 
 PLATFORMS = ["camera"]
 
@@ -22,7 +27,7 @@ async def async_setup_entry(hass: HomeAssistant, entry: ConfigEntry):
             hass.config_entries.async_forward_entry_setup(entry, component)
         )
 
-    hass.data[DOMAIN][entry.unique_id] = entry.data
+    hass.data[DOMAIN][entry.entry_id] = entry.data
 
     return True
 
@@ -39,10 +44,50 @@ async def async_unload_entry(hass: HomeAssistant, entry: ConfigEntry):
     )
 
     if unload_ok:
-        hass.data[DOMAIN].pop(entry.unique_id)
+        hass.data[DOMAIN].pop(entry.entry_id)
 
         if not hass.data[DOMAIN]:
             hass.services.async_remove(domain=DOMAIN, service=SERVICE_PTZ)
             hass.services.async_remove(domain=DOMAIN, service=SERVICE_PTZ_PRESET)
 
     return unload_ok
+
+
+async def async_migrate_entry(hass, config_entry: ConfigEntry):
+    """Migrate old entry."""
+    LOGGER.debug("Migrating from version %s", config_entry.version)
+
+    if config_entry.version == 1:
+        # Change unique id
+        @callback
+        def update_unique_id(entry):
+            return {"new_unique_id": config_entry.entry_id}
+
+        await async_migrate_entries(hass, config_entry.entry_id, update_unique_id)
+
+        config_entry.unique_id = None
+
+        # Get RTSP port from the camera or use the fallback one and store it in data
+        camera = FoscamCamera(
+            config_entry.data[CONF_HOST],
+            config_entry.data[CONF_PORT],
+            config_entry.data[CONF_USERNAME],
+            config_entry.data[CONF_PASSWORD],
+            verbose=False,
+        )
+
+        ret, response = await hass.async_add_executor_job(camera.get_port_info)
+
+        rtsp_port = DEFAULT_RTSP_PORT
+
+        if ret != 0:
+            rtsp_port = response.get("rtspPort") or response.get("mediaPort")
+
+        config_entry.data = {**config_entry.data, CONF_RTSP_PORT: rtsp_port}
+
+        # Change entry version
+        config_entry.version = 2
+
+    LOGGER.info("Migration to version %s successful", config_entry.version)
+
+    return True

homeassistant/components/foscam/camera.py

@@ -15,7 +15,14 @@ from homeassistant.const import (
 )
 from homeassistant.helpers import config_validation as cv, entity_platform
 
-from .const import CONF_STREAM, DOMAIN, LOGGER, SERVICE_PTZ, SERVICE_PTZ_PRESET
+from .const import (
+    CONF_RTSP_PORT,
+    CONF_STREAM,
+    DOMAIN,
+    LOGGER,
+    SERVICE_PTZ,
+    SERVICE_PTZ_PRESET,
+)
 
 PLATFORM_SCHEMA = PLATFORM_SCHEMA.extend(
     {
@@ -24,7 +31,7 @@ PLATFORM_SCHEMA = PLATFORM_SCHEMA.extend(
         vol.Required(CONF_USERNAME): cv.string,
         vol.Optional(CONF_NAME, default="Foscam Camera"): cv.string,
         vol.Optional(CONF_PORT, default=88): cv.port,
-        vol.Optional("rtsp_port"): cv.port,
+        vol.Optional(CONF_RTSP_PORT): cv.port,
     }
 )
 
@@ -71,6 +78,7 @@ async def async_setup_platform(hass, config, async_add_entities, discovery_info=
         CONF_USERNAME: config[CONF_USERNAME],
         CONF_PASSWORD: config[CONF_PASSWORD],
         CONF_STREAM: "Main",
+        CONF_RTSP_PORT: config.get(CONF_RTSP_PORT, 554),
     }
 
     hass.async_create_task(
@@ -134,8 +142,8 @@ class HassFoscamCamera(Camera):
         self._username = config_entry.data[CONF_USERNAME]
         self._password = config_entry.data[CONF_PASSWORD]
         self._stream = config_entry.data[CONF_STREAM]
-        self._unique_id = config_entry.unique_id
+        self._unique_id = config_entry.entry_id
-        self._rtsp_port = None
+        self._rtsp_port = config_entry.data[CONF_RTSP_PORT]
         self._motion_status = False
 
     async def async_added_to_hass(self):
@@ -145,7 +153,13 @@ class HassFoscamCamera(Camera):
             self._foscam_session.get_motion_detect_config
         )
 
-        if ret != 0:
+        if ret == -3:
+            LOGGER.info(
+                "Can't get motion detection status, camera %s configured with non-admin user",
+                self._name,
+            )
+
+        elif ret != 0:
             LOGGER.error(
                 "Error getting motion detection status of %s: %s", self._name, ret
             )
@@ -153,17 +167,6 @@ class HassFoscamCamera(Camera):
         else:
             self._motion_status = response == 1
 
-        # Get RTSP port
-        ret, response = await self.hass.async_add_executor_job(
-            self._foscam_session.get_port_info
-        )
-
-        if ret != 0:
-            LOGGER.error("Error getting RTSP port of %s: %s", self._name, ret)
-
-        else:
-            self._rtsp_port = response.get("rtspPort") or response.get("mediaPort")
-
     @property
     def unique_id(self):
         """Return the entity unique ID."""
@@ -205,6 +208,11 @@ class HassFoscamCamera(Camera):
             ret = self._foscam_session.enable_motion_detection()
 
             if ret != 0:
+                if ret == -3:
+                    LOGGER.info(
+                        "Can't set motion detection status, camera %s configured with non-admin user",
+                        self._name,
+                    )
                 return
 
             self._motion_status = True
@@ -220,6 +228,11 @@ class HassFoscamCamera(Camera):
             ret = self._foscam_session.disable_motion_detection()
 
             if ret != 0:
+                if ret == -3:
+                    LOGGER.info(
+                        "Can't set motion detection status, camera %s configured with non-admin user",
+                        self._name,
+                    )
                 return
 
             self._motion_status = False

homeassistant/components/foscam/config_flow.py

@@ -1,6 +1,10 @@
 """Config flow for foscam integration."""
 from libpyfoscam import FoscamCamera
-from libpyfoscam.foscam import ERROR_FOSCAM_AUTH, ERROR_FOSCAM_UNAVAILABLE
+from libpyfoscam.foscam import (
+    ERROR_FOSCAM_AUTH,
+    ERROR_FOSCAM_UNAVAILABLE,
+    FOSCAM_SUCCESS,
+)
 import voluptuous as vol
 
 from homeassistant import config_entries, exceptions
@@ -13,12 +17,13 @@ from homeassistant.const import (
 )
 from homeassistant.data_entry_flow import AbortFlow
 
-from .const import CONF_STREAM, LOGGER
+from .const import CONF_RTSP_PORT, CONF_STREAM, LOGGER
 from .const import DOMAIN  # pylint:disable=unused-import
 
 STREAMS = ["Main", "Sub"]
 
 DEFAULT_PORT = 88
+DEFAULT_RTSP_PORT = 554
 
 
 DATA_SCHEMA = vol.Schema(
@@ -28,6 +33,7 @@ DATA_SCHEMA = vol.Schema(
         vol.Required(CONF_USERNAME): str,
         vol.Required(CONF_PASSWORD): str,
         vol.Required(CONF_STREAM, default=STREAMS[0]): vol.In(STREAMS),
+        vol.Required(CONF_RTSP_PORT, default=DEFAULT_RTSP_PORT): int,
     }
 )
 
@@ -35,7 +41,7 @@ DATA_SCHEMA = vol.Schema(
 class ConfigFlow(config_entries.ConfigFlow, domain=DOMAIN):
     """Handle a config flow for foscam."""
 
-    VERSION = 1
+    VERSION = 2
     CONNECTION_CLASS = config_entries.CONN_CLASS_LOCAL_POLL
 
     async def _validate_and_create(self, data):
@@ -43,6 +49,14 @@ class ConfigFlow(config_entries.ConfigFlow, domain=DOMAIN):
 
         Data has the keys from DATA_SCHEMA with values provided by the user.
         """
+
+        for entry in self.hass.config_entries.async_entries(DOMAIN):
+            if (
+                entry.data[CONF_HOST] == data[CONF_HOST]
+                and entry.data[CONF_PORT] == data[CONF_PORT]
+            ):
+                raise AbortFlow("already_configured")
+
         camera = FoscamCamera(
             data[CONF_HOST],
             data[CONF_PORT],
@@ -52,7 +66,7 @@ class ConfigFlow(config_entries.ConfigFlow, domain=DOMAIN):
         )
 
         # Validate data by sending a request to the camera
-        ret, response = await self.hass.async_add_executor_job(camera.get_dev_info)
+        ret, _ = await self.hass.async_add_executor_job(camera.get_product_all_info)
 
         if ret == ERROR_FOSCAM_UNAVAILABLE:
             raise CannotConnect
@@ -60,10 +74,23 @@ class ConfigFlow(config_entries.ConfigFlow, domain=DOMAIN):
         if ret == ERROR_FOSCAM_AUTH:
             raise InvalidAuth
 
-        await self.async_set_unique_id(response["mac"])
+        if ret != FOSCAM_SUCCESS:
-        self._abort_if_unique_id_configured()
+            LOGGER.error(
+                "Unexpected error code from camera %s:%s: %s",
+                data[CONF_HOST],
+                data[CONF_PORT],
+                ret,
+            )
+            raise InvalidResponse
 
-        name = data.pop(CONF_NAME, response["devName"])
+        # Try to get camera name (only possible with admin account)
+        ret, response = await self.hass.async_add_executor_job(camera.get_dev_info)
+
+        dev_name = response.get(
+            "devName", f"Foscam {data[CONF_HOST]}:{data[CONF_PORT]}"
+        )
+
+        name = data.pop(CONF_NAME, dev_name)
 
         return self.async_create_entry(title=name, data=data)
 
@@ -81,6 +108,9 @@ class ConfigFlow(config_entries.ConfigFlow, domain=DOMAIN):
             except InvalidAuth:
                 errors["base"] = "invalid_auth"
 
+            except InvalidResponse:
+                errors["base"] = "invalid_response"
+
             except AbortFlow:
                 raise
 
@@ -105,6 +135,12 @@ class ConfigFlow(config_entries.ConfigFlow, domain=DOMAIN):
             LOGGER.error("Error importing foscam platform config: invalid auth.")
             return self.async_abort(reason="invalid_auth")
 
+        except InvalidResponse:
+            LOGGER.exception(
+                "Error importing foscam platform config: invalid response from camera."
+            )
+            return self.async_abort(reason="invalid_response")
+
         except AbortFlow:
             raise
 
@@ -121,3 +157,7 @@ class CannotConnect(exceptions.HomeAssistantError):
 
 class InvalidAuth(exceptions.HomeAssistantError):
     """Error to indicate there is invalid auth."""
+
+
+class InvalidResponse(exceptions.HomeAssistantError):
+    """Error to indicate there is invalid response."""

homeassistant/components/foscam/const.py

@@ -5,6 +5,7 @@ LOGGER = logging.getLogger(__package__)
 
 DOMAIN = "foscam"
 
+CONF_RTSP_PORT = "rtsp_port"
 CONF_STREAM = "stream"
 
 SERVICE_PTZ = "ptz"

homeassistant/components/foscam/strings.json

@@ -8,6 +8,7 @@
           "port": "[%key:common::config_flow::data::port%]",
           "username": "[%key:common::config_flow::data::username%]",
           "password": "[%key:common::config_flow::data::password%]",
+          "rtsp_port": "RTSP port",
           "stream": "Stream"
         }
       }
@@ -15,6 +16,7 @@
     "error": {
       "cannot_connect": "[%key:common::config_flow::error::cannot_connect%]",
       "invalid_auth": "[%key:common::config_flow::error::invalid_auth%]",
+      "invalid_response": "Invalid response from the device",
       "unknown": "[%key:common::config_flow::error::unknown%]"
     },
     "abort": {

homeassistant/components/foscam/translations/en.json

@@ -6,6 +6,7 @@
         "error": {
             "cannot_connect": "Failed to connect",
             "invalid_auth": "Invalid authentication",
+            "invalid_response": "Invalid response from the device",
             "unknown": "Unexpected error"
         },
         "step": {
@@ -14,6 +15,7 @@
                     "host": "Host",
                     "password": "Password",
                     "port": "Port",
+                    "rtsp_port": "RTSP port",
                     "stream": "Stream",
                     "username": "Username"
                 }

tests/components/foscam/test_config_flow.py

@@ -1,7 +1,12 @@
 """Test the Foscam config flow."""
 from unittest.mock import patch
 
-from libpyfoscam.foscam import ERROR_FOSCAM_AUTH, ERROR_FOSCAM_UNAVAILABLE
+from libpyfoscam.foscam import (
+    ERROR_FOSCAM_AUTH,
+    ERROR_FOSCAM_CMD,
+    ERROR_FOSCAM_UNAVAILABLE,
+    ERROR_FOSCAM_UNKNOWN,
+)
 
 from homeassistant import config_entries, data_entry_flow, setup
 from homeassistant.components.foscam import config_flow
@@ -14,6 +19,13 @@ VALID_CONFIG = {
     config_flow.CONF_USERNAME: "admin",
     config_flow.CONF_PASSWORD: "1234",
     config_flow.CONF_STREAM: "Main",
+    config_flow.CONF_RTSP_PORT: 554,
+}
+OPERATOR_CONFIG = {
+    config_flow.CONF_USERNAME: "operator",
+}
+INVALID_RESPONSE_CONFIG = {
+    config_flow.CONF_USERNAME: "interr",
 }
 CAMERA_NAME = "Mocked Foscam Camera"
 CAMERA_MAC = "C0:C1:D0:F4:B4:D4"
@@ -23,26 +35,39 @@ def setup_mock_foscam_camera(mock_foscam_camera):
     """Mock FoscamCamera simulating behaviour using a base valid config."""
 
     def configure_mock_on_init(host, port, user, passwd, verbose=False):
-        return_code = 0
+        product_all_info_rc = 0
-        data = {}
+        dev_info_rc = 0
+        dev_info_data = {}
 
         if (
             host != VALID_CONFIG[config_flow.CONF_HOST]
             or port != VALID_CONFIG[config_flow.CONF_PORT]
         ):
-            return_code = ERROR_FOSCAM_UNAVAILABLE
+            product_all_info_rc = dev_info_rc = ERROR_FOSCAM_UNAVAILABLE
 
         elif (
-            user != VALID_CONFIG[config_flow.CONF_USERNAME]
+            user
+            not in [
+                VALID_CONFIG[config_flow.CONF_USERNAME],
+                OPERATOR_CONFIG[config_flow.CONF_USERNAME],
+                INVALID_RESPONSE_CONFIG[config_flow.CONF_USERNAME],
+            ]
             or passwd != VALID_CONFIG[config_flow.CONF_PASSWORD]
         ):
-            return_code = ERROR_FOSCAM_AUTH
+            product_all_info_rc = dev_info_rc = ERROR_FOSCAM_AUTH
+
+        elif user == INVALID_RESPONSE_CONFIG[config_flow.CONF_USERNAME]:
+            product_all_info_rc = dev_info_rc = ERROR_FOSCAM_UNKNOWN
+
+        elif user == OPERATOR_CONFIG[config_flow.CONF_USERNAME]:
+            dev_info_rc = ERROR_FOSCAM_CMD
 
         else:
-            data["devName"] = CAMERA_NAME
+            dev_info_data["devName"] = CAMERA_NAME
-            data["mac"] = CAMERA_MAC
+            dev_info_data["mac"] = CAMERA_MAC
 
-        mock_foscam_camera.get_dev_info.return_value = (return_code, data)
+        mock_foscam_camera.get_product_all_info.return_value = (product_all_info_rc, {})
+        mock_foscam_camera.get_dev_info.return_value = (dev_info_rc, dev_info_data)
 
         return mock_foscam_camera
 
@@ -142,12 +167,44 @@ async def test_user_cannot_connect(hass):
         assert result["errors"] == {"base": "cannot_connect"}
 
 
+async def test_user_invalid_response(hass):
+    """Test we handle invalid response error from user input."""
+    await setup.async_setup_component(hass, "persistent_notification", {})
+
+    result = await hass.config_entries.flow.async_init(
+        config_flow.DOMAIN, context={"source": config_entries.SOURCE_USER}
+    )
+    assert result["type"] == data_entry_flow.RESULT_TYPE_FORM
+    assert result["errors"] == {}
+
+    with patch(
+        "homeassistant.components.foscam.config_flow.FoscamCamera",
+    ) as mock_foscam_camera:
+        setup_mock_foscam_camera(mock_foscam_camera)
+
+        invalid_response = VALID_CONFIG.copy()
+        invalid_response[config_flow.CONF_USERNAME] = INVALID_RESPONSE_CONFIG[
+            config_flow.CONF_USERNAME
+        ]
+
+        result = await hass.config_entries.flow.async_configure(
+            result["flow_id"],
+            invalid_response,
+        )
+
+        await hass.async_block_till_done()
+
+        assert result["type"] == data_entry_flow.RESULT_TYPE_FORM
+        assert result["errors"] == {"base": "invalid_response"}
+
+
 async def test_user_already_configured(hass):
     """Test we handle already configured from user input."""
     await setup.async_setup_component(hass, "persistent_notification", {})
 
     entry = MockConfigEntry(
-        domain=config_flow.DOMAIN, data=VALID_CONFIG, unique_id=CAMERA_MAC
+        domain=config_flow.DOMAIN,
+        data=VALID_CONFIG,
     )
     entry.add_to_hass(hass)
 
@@ -201,6 +258,8 @@ async def test_user_unknown_exception(hass):
 
 async def test_import_user_valid(hass):
     """Test valid config from import."""
+    await setup.async_setup_component(hass, "persistent_notification", {})
+
     with patch(
         "homeassistant.components.foscam.config_flow.FoscamCamera",
     ) as mock_foscam_camera, patch(
@@ -229,6 +288,8 @@ async def test_import_user_valid(hass):
 
 async def test_import_user_valid_with_name(hass):
     """Test valid config with extra name from import."""
+    await setup.async_setup_component(hass, "persistent_notification", {})
+
     with patch(
         "homeassistant.components.foscam.config_flow.FoscamCamera",
     ) as mock_foscam_camera, patch(
@@ -261,10 +322,7 @@ async def test_import_user_valid_with_name(hass):
 
 async def test_import_invalid_auth(hass):
     """Test we handle invalid auth from import."""
-    entry = MockConfigEntry(
+    await setup.async_setup_component(hass, "persistent_notification", {})
-        domain=config_flow.DOMAIN, data=VALID_CONFIG, unique_id=CAMERA_MAC
-    )
-    entry.add_to_hass(hass)
 
     with patch(
         "homeassistant.components.foscam.config_flow.FoscamCamera",
@@ -287,11 +345,8 @@ async def test_import_invalid_auth(hass):
 
 
 async def test_import_cannot_connect(hass):
-    """Test we handle invalid auth from import."""
+    """Test we handle cannot connect error from import."""
-    entry = MockConfigEntry(
+    await setup.async_setup_component(hass, "persistent_notification", {})
-        domain=config_flow.DOMAIN, data=VALID_CONFIG, unique_id=CAMERA_MAC
-    )
-    entry.add_to_hass(hass)
 
     with patch(
         "homeassistant.components.foscam.config_flow.FoscamCamera",
@@ -313,10 +368,39 @@ async def test_import_cannot_connect(hass):
         assert result["reason"] == "cannot_connect"
 
 
+async def test_import_invalid_response(hass):
+    """Test we handle invalid response error from import."""
+    await setup.async_setup_component(hass, "persistent_notification", {})
+
+    with patch(
+        "homeassistant.components.foscam.config_flow.FoscamCamera",
+    ) as mock_foscam_camera:
+        setup_mock_foscam_camera(mock_foscam_camera)
+
+        invalid_response = VALID_CONFIG.copy()
+        invalid_response[config_flow.CONF_USERNAME] = INVALID_RESPONSE_CONFIG[
+            config_flow.CONF_USERNAME
+        ]
+
+        result = await hass.config_entries.flow.async_init(
+            config_flow.DOMAIN,
+            context={"source": config_entries.SOURCE_IMPORT},
+            data=invalid_response,
+        )
+
+        await hass.async_block_till_done()
+
+        assert result["type"] == data_entry_flow.RESULT_TYPE_ABORT
+        assert result["reason"] == "invalid_response"
+
+
 async def test_import_already_configured(hass):
     """Test we handle already configured from import."""
+    await setup.async_setup_component(hass, "persistent_notification", {})
+
     entry = MockConfigEntry(
-        domain=config_flow.DOMAIN, data=VALID_CONFIG, unique_id=CAMERA_MAC
+        domain=config_flow.DOMAIN,
+        data=VALID_CONFIG,
     )
     entry.add_to_hass(hass)