Make API key mandatory for PI-Hole (#85885)

add reauth, so make api-key mandatory

tests/components/pi_hole/__init__.py

@@ -3,7 +3,13 @@ from unittest.mock import AsyncMock, MagicMock, patch
 
 from hole.exceptions import HoleError
 
-from homeassistant.components.pi_hole.const import CONF_STATISTICS_ONLY
+from homeassistant.components.pi_hole.const import (
+    CONF_STATISTICS_ONLY,
+    DEFAULT_LOCATION,
+    DEFAULT_NAME,
+    DEFAULT_SSL,
+    DEFAULT_VERIFY_SSL,
+)
 from homeassistant.const import (
     CONF_API_KEY,
     CONF_HOST,
@@ -47,7 +53,16 @@ API_KEY = "apikey"
 SSL = False
 VERIFY_SSL = True
 
-CONF_DATA = {
+CONFIG_DATA_DEFAULTS = {
+    CONF_HOST: f"{HOST}:{PORT}",
+    CONF_LOCATION: DEFAULT_LOCATION,
+    CONF_NAME: DEFAULT_NAME,
+    CONF_SSL: DEFAULT_SSL,
+    CONF_VERIFY_SSL: DEFAULT_VERIFY_SSL,
+    CONF_API_KEY: API_KEY,
+}
+
+CONFIG_DATA = {
     CONF_HOST: f"{HOST}:{PORT}",
     CONF_LOCATION: LOCATION,
     CONF_NAME: NAME,
@@ -56,34 +71,35 @@ CONF_DATA = {
     CONF_VERIFY_SSL: VERIFY_SSL,
 }
 
-CONF_CONFIG_FLOW_USER = {
+CONFIG_FLOW_USER = {
     CONF_HOST: HOST,
     CONF_PORT: PORT,
+    CONF_API_KEY: API_KEY,
     CONF_LOCATION: LOCATION,
     CONF_NAME: NAME,
-    CONF_STATISTICS_ONLY: False,
     CONF_SSL: SSL,
     CONF_VERIFY_SSL: VERIFY_SSL,
 }
 
-CONF_CONFIG_FLOW_API_KEY = {
+CONFIG_FLOW_API_KEY = {
     CONF_API_KEY: API_KEY,
 }
 
-CONF_CONFIG_ENTRY = {
+CONFIG_ENTRY = {
     CONF_HOST: f"{HOST}:{PORT}",
     CONF_LOCATION: LOCATION,
     CONF_NAME: NAME,
-    CONF_STATISTICS_ONLY: False,
     CONF_API_KEY: API_KEY,
     CONF_SSL: SSL,
     CONF_VERIFY_SSL: VERIFY_SSL,
 }
 
+CONFIG_ENTRY_IMPORTED = {**CONFIG_ENTRY, CONF_STATISTICS_ONLY: False}
+
 SWITCH_ENTITY_ID = "switch.pi_hole"
 
 
-def _create_mocked_hole(raise_exception=False, has_versions=True):
+def _create_mocked_hole(raise_exception=False, has_versions=True, has_data=True):
     mocked_hole = MagicMock()
     type(mocked_hole).get_data = AsyncMock(
         side_effect=HoleError("") if raise_exception else None
@@ -93,7 +109,10 @@ def _create_mocked_hole(raise_exception=False, has_versions=True):
     )
     type(mocked_hole).enable = AsyncMock()
     type(mocked_hole).disable = AsyncMock()
-    mocked_hole.data = ZERO_DATA
+    if has_data:
+        mocked_hole.data = ZERO_DATA
+    else:
+        mocked_hole.data = []
     if has_versions:
         mocked_hole.versions = SAMPLE_VERSIONS
     else:

tests/components/pi_hole/test_config_flow.py

@@ -2,28 +2,26 @@
 import logging
 from unittest.mock import patch
 
-from homeassistant.components.pi_hole.const import CONF_STATISTICS_ONLY, DOMAIN
+from homeassistant.components.pi_hole.const import DOMAIN
 from homeassistant.config_entries import SOURCE_IMPORT, SOURCE_USER
 from homeassistant.const import CONF_API_KEY
+from homeassistant.core import HomeAssistant
 from homeassistant.data_entry_flow import FlowResultType
 
 from . import (
-    CONF_CONFIG_ENTRY,
-    CONF_CONFIG_FLOW_API_KEY,
-    CONF_CONFIG_FLOW_USER,
-    CONF_DATA,
+    CONFIG_DATA,
+    CONFIG_DATA_DEFAULTS,
+    CONFIG_ENTRY,
+    CONFIG_ENTRY_IMPORTED,
+    CONFIG_FLOW_USER,
     NAME,
+    ZERO_DATA,
     _create_mocked_hole,
     _patch_config_flow_hole,
+    _patch_init_hole,
 )
 
-
-def _flow_next(hass, flow_id):
-    return next(
-        flow
-        for flow in hass.config_entries.flow.async_progress()
-        if flow["flow_id"] == flow_id
-    )
+from tests.common import MockConfigEntry
 
 
 def _patch_setup():
@@ -33,41 +31,41 @@ def _patch_setup():
     )
 
 
-async def test_flow_import(hass, caplog):
+async def test_flow_import(hass: HomeAssistant, caplog):
     """Test import flow."""
     mocked_hole = _create_mocked_hole()
     with _patch_config_flow_hole(mocked_hole), _patch_setup():
         result = await hass.config_entries.flow.async_init(
-            DOMAIN, context={"source": SOURCE_IMPORT}, data=CONF_DATA
+            DOMAIN, context={"source": SOURCE_IMPORT}, data=CONFIG_DATA
         )
         assert result["type"] == FlowResultType.CREATE_ENTRY
         assert result["title"] == NAME
-        assert result["data"] == CONF_CONFIG_ENTRY
+        assert result["data"] == CONFIG_ENTRY_IMPORTED
 
         # duplicated server
         result = await hass.config_entries.flow.async_init(
-            DOMAIN, context={"source": SOURCE_IMPORT}, data=CONF_DATA
+            DOMAIN, context={"source": SOURCE_IMPORT}, data=CONFIG_DATA
         )
         assert result["type"] == FlowResultType.ABORT
         assert result["reason"] == "already_configured"
 
 
-async def test_flow_import_invalid(hass, caplog):
+async def test_flow_import_invalid(hass: HomeAssistant, caplog):
     """Test import flow with invalid server."""
     mocked_hole = _create_mocked_hole(True)
     with _patch_config_flow_hole(mocked_hole), _patch_setup():
         result = await hass.config_entries.flow.async_init(
-            DOMAIN, context={"source": SOURCE_IMPORT}, data=CONF_DATA
+            DOMAIN, context={"source": SOURCE_IMPORT}, data=CONFIG_DATA
         )
         assert result["type"] == FlowResultType.ABORT
         assert result["reason"] == "cannot_connect"
         assert len([x for x in caplog.records if x.levelno == logging.ERROR]) == 1
 
 
-async def test_flow_user(hass):
+async def test_flow_user(hass: HomeAssistant):
     """Test user initialized flow."""
-    mocked_hole = _create_mocked_hole()
-    with _patch_config_flow_hole(mocked_hole), _patch_setup():
+    mocked_hole = _create_mocked_hole(has_data=False)
+    with _patch_config_flow_hole(mocked_hole), _patch_init_hole(mocked_hole):
         result = await hass.config_entries.flow.async_init(
             DOMAIN,
             context={"source": SOURCE_USER},
@@ -75,69 +73,68 @@ async def test_flow_user(hass):
         assert result["type"] == FlowResultType.FORM
         assert result["step_id"] == "user"
         assert result["errors"] == {}
-        _flow_next(hass, result["flow_id"])
 
         result = await hass.config_entries.flow.async_configure(
             result["flow_id"],
-            user_input=CONF_CONFIG_FLOW_USER,
+            user_input=CONFIG_FLOW_USER,
         )
         assert result["type"] == FlowResultType.FORM
-        assert result["step_id"] == "api_key"
-        assert result["errors"] is None
-        _flow_next(hass, result["flow_id"])
+        assert result["step_id"] == "user"
+        assert result["errors"] == {CONF_API_KEY: "invalid_auth"}
 
+        mocked_hole.data = ZERO_DATA
         result = await hass.config_entries.flow.async_configure(
             result["flow_id"],
-            user_input=CONF_CONFIG_FLOW_API_KEY,
+            user_input=CONFIG_FLOW_USER,
         )
         assert result["type"] == FlowResultType.CREATE_ENTRY
         assert result["title"] == NAME
-        assert result["data"] == CONF_CONFIG_ENTRY
+        assert result["data"] == CONFIG_ENTRY
 
         # duplicated server
         result = await hass.config_entries.flow.async_init(
             DOMAIN,
             context={"source": SOURCE_USER},
-            data=CONF_CONFIG_FLOW_USER,
+            data=CONFIG_FLOW_USER,
         )
         assert result["type"] == FlowResultType.ABORT
         assert result["reason"] == "already_configured"
 
 
-async def test_flow_statistics_only(hass):
-    """Test user initialized flow with statistics only."""
-    mocked_hole = _create_mocked_hole()
-    with _patch_config_flow_hole(mocked_hole), _patch_setup():
-        result = await hass.config_entries.flow.async_init(
-            DOMAIN,
-            context={"source": SOURCE_USER},
-        )
-        assert result["type"] == FlowResultType.FORM
-        assert result["step_id"] == "user"
-        assert result["errors"] == {}
-        _flow_next(hass, result["flow_id"])
-
-        user_input = {**CONF_CONFIG_FLOW_USER}
-        user_input[CONF_STATISTICS_ONLY] = True
-        config_entry_data = {**CONF_CONFIG_ENTRY}
-        config_entry_data[CONF_STATISTICS_ONLY] = True
-        config_entry_data.pop(CONF_API_KEY)
-        result = await hass.config_entries.flow.async_configure(
-            result["flow_id"],
-            user_input=user_input,
-        )
-        assert result["type"] == FlowResultType.CREATE_ENTRY
-        assert result["title"] == NAME
-        assert result["data"] == config_entry_data
-
-
 async def test_flow_user_invalid(hass):
     """Test user initialized flow with invalid server."""
     mocked_hole = _create_mocked_hole(True)
     with _patch_config_flow_hole(mocked_hole), _patch_setup():
         result = await hass.config_entries.flow.async_init(
-            DOMAIN, context={"source": SOURCE_USER}, data=CONF_CONFIG_FLOW_USER
+            DOMAIN, context={"source": SOURCE_USER}, data=CONFIG_FLOW_USER
         )
         assert result["type"] == FlowResultType.FORM
         assert result["step_id"] == "user"
         assert result["errors"] == {"base": "cannot_connect"}
+
+
+async def test_flow_reauth(hass: HomeAssistant):
+    """Test reauth flow."""
+    mocked_hole = _create_mocked_hole(has_data=False)
+    entry = MockConfigEntry(domain=DOMAIN, data=CONFIG_DATA_DEFAULTS)
+    entry.add_to_hass(hass)
+    with _patch_init_hole(mocked_hole), _patch_config_flow_hole(mocked_hole):
+        assert not await hass.config_entries.async_setup(entry.entry_id)
+
+        flows = hass.config_entries.flow.async_progress()
+
+        assert len(flows) == 1
+        assert flows[0]["step_id"] == "reauth_confirm"
+        assert flows[0]["context"]["entry_id"] == entry.entry_id
+
+        mocked_hole.data = ZERO_DATA
+
+        result = await hass.config_entries.flow.async_configure(
+            flows[0]["flow_id"],
+            user_input={CONF_API_KEY: "newkey"},
+        )
+
+        await hass.async_block_till_done()
+        assert result["type"] == FlowResultType.ABORT
+        assert result["reason"] == "reauth_successful"
+        assert entry.data[CONF_API_KEY] == "newkey"

tests/components/pi_hole/test_init.py

@@ -7,27 +7,16 @@ from hole.exceptions import HoleError
 from homeassistant.components import pi_hole, switch
 from homeassistant.components.pi_hole.const import (
     CONF_STATISTICS_ONLY,
-    DEFAULT_LOCATION,
-    DEFAULT_NAME,
-    DEFAULT_SSL,
-    DEFAULT_VERIFY_SSL,
     SERVICE_DISABLE,
     SERVICE_DISABLE_ATTR_DURATION,
 )
-from homeassistant.const import (
-    ATTR_ENTITY_ID,
-    CONF_API_KEY,
-    CONF_HOST,
-    CONF_LOCATION,
-    CONF_NAME,
-    CONF_SSL,
-    CONF_VERIFY_SSL,
-)
+from homeassistant.config_entries import ConfigEntryState
+from homeassistant.const import ATTR_ENTITY_ID, CONF_API_KEY, CONF_HOST
+from homeassistant.core import HomeAssistant
 from homeassistant.setup import async_setup_component
 
 from . import (
-    CONF_CONFIG_ENTRY,
-    CONF_DATA,
+    CONFIG_DATA_DEFAULTS,
     SWITCH_ENTITY_ID,
     _create_mocked_hole,
     _patch_config_flow_hole,
@@ -37,7 +26,7 @@ from . import (
 from tests.common import MockConfigEntry
 
 
-async def test_setup_minimal_config(hass):
+async def test_setup_minimal_config(hass: HomeAssistant):
     """Tests component setup with minimal config."""
     mocked_hole = _create_mocked_hole()
     with _patch_config_flow_hole(mocked_hole), _patch_init_hole(mocked_hole):
@@ -88,7 +77,7 @@ async def test_setup_minimal_config(hass):
     assert state.state == "off"
 
 
-async def test_setup_name_config(hass):
+async def test_setup_name_config(hass: HomeAssistant):
     """Tests component setup with a custom name."""
     mocked_hole = _create_mocked_hole()
     with _patch_config_flow_hole(mocked_hole), _patch_init_hole(mocked_hole):
@@ -106,7 +95,7 @@ async def test_setup_name_config(hass):
     )
 
 
-async def test_switch(hass, caplog):
+async def test_switch(hass: HomeAssistant, caplog):
     """Test Pi-hole switch."""
     mocked_hole = _create_mocked_hole()
     with _patch_config_flow_hole(mocked_hole), _patch_init_hole(mocked_hole):
@@ -154,7 +143,7 @@ async def test_switch(hass, caplog):
         assert errors[-1].message == "Unable to disable Pi-hole: Error2"
 
 
-async def test_disable_service_call(hass):
+async def test_disable_service_call(hass: HomeAssistant):
     """Test disable service call with no Pi-hole named."""
     mocked_hole = _create_mocked_hole()
     with _patch_config_flow_hole(mocked_hole), _patch_init_hole(mocked_hole):
@@ -180,21 +169,14 @@ async def test_disable_service_call(hass):
 
         await hass.async_block_till_done()
 
-        mocked_hole.disable.assert_called_once_with(1)
+        mocked_hole.disable.assert_called_with(1)
 
 
-async def test_unload(hass):
+async def test_unload(hass: HomeAssistant):
     """Test unload entities."""
     entry = MockConfigEntry(
         domain=pi_hole.DOMAIN,
-        data={
-            CONF_NAME: DEFAULT_NAME,
-            CONF_HOST: "pi.hole",
-            CONF_LOCATION: DEFAULT_LOCATION,
-            CONF_SSL: DEFAULT_SSL,
-            CONF_VERIFY_SSL: DEFAULT_VERIFY_SSL,
-            CONF_STATISTICS_ONLY: True,
-        },
+        data={**CONFIG_DATA_DEFAULTS, CONF_HOST: "pi.hole"},
     )
     entry.add_to_hass(hass)
     mocked_hole = _create_mocked_hole()
@@ -208,32 +190,25 @@ async def test_unload(hass):
     assert entry.entry_id not in hass.data[pi_hole.DOMAIN]
 
 
-async def test_migrate(hass):
-    """Test migrate from old config entry."""
-    entry = MockConfigEntry(domain=pi_hole.DOMAIN, data=CONF_DATA)
-    entry.add_to_hass(hass)
-
+async def test_remove_obsolete(hass: HomeAssistant):
+    """Test removing obsolete config entry parameters."""
     mocked_hole = _create_mocked_hole()
-    with _patch_config_flow_hole(mocked_hole), _patch_init_hole(mocked_hole):
-        await hass.config_entries.async_setup(entry.entry_id)
-        await hass.async_block_till_done()
-
-    assert entry.data == CONF_CONFIG_ENTRY
-
-
-async def test_migrate_statistics_only(hass):
-    """Test migrate from old config entry with statistics only."""
-    conf_data = {**CONF_DATA}
-    conf_data[CONF_API_KEY] = ""
-    entry = MockConfigEntry(domain=pi_hole.DOMAIN, data=conf_data)
+    entry = MockConfigEntry(
+        domain=pi_hole.DOMAIN, data={**CONFIG_DATA_DEFAULTS, CONF_STATISTICS_ONLY: True}
+    )
     entry.add_to_hass(hass)
+    with _patch_init_hole(mocked_hole):
+        assert await hass.config_entries.async_setup(entry.entry_id)
+        assert CONF_STATISTICS_ONLY not in entry.data
 
+
+async def test_missing_api_key(hass: HomeAssistant):
+    """Tests start reauth flow if api key is missing."""
     mocked_hole = _create_mocked_hole()
-    with _patch_config_flow_hole(mocked_hole), _patch_init_hole(mocked_hole):
-        await hass.config_entries.async_setup(entry.entry_id)
-        await hass.async_block_till_done()
-
-    config_entry_data = {**CONF_CONFIG_ENTRY}
-    config_entry_data[CONF_STATISTICS_ONLY] = True
-    config_entry_data[CONF_API_KEY] = ""
-    assert entry.data == config_entry_data
+    data = CONFIG_DATA_DEFAULTS.copy()
+    data.pop(CONF_API_KEY)
+    entry = MockConfigEntry(domain=pi_hole.DOMAIN, data=data)
+    entry.add_to_hass(hass)
+    with _patch_init_hole(mocked_hole):
+        assert not await hass.config_entries.async_setup(entry.entry_id)
+        assert entry.state == ConfigEntryState.SETUP_ERROR