From e1512fd3e195cd96654a7f0e1903e31d3576e7d8 Mon Sep 17 00:00:00 2001 From: Michael <35783820+mib1185@users.noreply.github.com> Date: Fri, 20 Jan 2023 13:02:44 +0100 Subject: [PATCH] Support password less PI-Hole installations (#86184) fixes undefined --- homeassistant/components/pi_hole/__init__.py | 4 -- .../components/pi_hole/config_flow.py | 26 ++++++++-- homeassistant/components/pi_hole/strings.json | 6 ++- .../components/pi_hole/translations/en.json | 1 - tests/components/pi_hole/__init__.py | 20 +++++++- tests/components/pi_hole/test_config_flow.py | 48 ++++++++++++++++--- tests/components/pi_hole/test_init.py | 15 +----- 7 files changed, 88 insertions(+), 32 deletions(-) diff --git a/homeassistant/components/pi_hole/__init__.py b/homeassistant/components/pi_hole/__init__.py index ac42410604f..49f1697adc6 100644 --- a/homeassistant/components/pi_hole/__init__.py +++ b/homeassistant/components/pi_hole/__init__.py @@ -62,10 +62,6 @@ async def async_setup_entry(hass: HomeAssistant, entry: ConfigEntry) -> bool: entry_data.pop(CONF_STATISTICS_ONLY) hass.config_entries.async_update_entry(entry, data=entry_data) - # start reauth to force api key is present - if CONF_API_KEY not in entry.data: - raise ConfigEntryAuthFailed - _LOGGER.debug("Setting up %s integration with host %s", DOMAIN, host) session = async_get_clientsession(hass, verify_tls) diff --git a/homeassistant/components/pi_hole/config_flow.py b/homeassistant/components/pi_hole/config_flow.py index 48cf93cbe33..136e851429d 100644 --- a/homeassistant/components/pi_hole/config_flow.py +++ b/homeassistant/components/pi_hole/config_flow.py @@ -55,7 +55,6 @@ class PiHoleFlowHandler(config_entries.ConfigFlow, domain=DOMAIN): CONF_LOCATION: user_input[CONF_LOCATION], CONF_SSL: user_input[CONF_SSL], CONF_VERIFY_SSL: user_input[CONF_VERIFY_SSL], - CONF_API_KEY: user_input[CONF_API_KEY], } self._async_abort_entries_match( @@ -70,6 +69,9 @@ class PiHoleFlowHandler(config_entries.ConfigFlow, domain=DOMAIN): title=user_input[CONF_NAME], data=self._config ) + if CONF_API_KEY in errors: + return await self.async_step_api_key() + user_input = user_input or {} return self.async_show_form( step_id="user", @@ -79,7 +81,6 @@ class PiHoleFlowHandler(config_entries.ConfigFlow, domain=DOMAIN): vol.Required( CONF_PORT, default=user_input.get(CONF_PORT, 80) ): vol.Coerce(int), - vol.Required(CONF_API_KEY): str, vol.Required( CONF_NAME, default=user_input.get(CONF_NAME, DEFAULT_NAME) ): str, @@ -100,6 +101,25 @@ class PiHoleFlowHandler(config_entries.ConfigFlow, domain=DOMAIN): errors=errors, ) + async def async_step_api_key( + self, user_input: dict[str, Any] | None = None + ) -> FlowResult: + """Handle step to setup API key.""" + errors = {} + if user_input is not None: + self._config[CONF_API_KEY] = user_input[CONF_API_KEY] + if not (errors := await self._async_try_connect()): + return self.async_create_entry( + title=self._config[CONF_NAME], + data=self._config, + ) + + return self.async_show_form( + step_id="api_key", + data_schema=vol.Schema({vol.Required(CONF_API_KEY): str}), + errors=errors, + ) + async def async_step_reauth(self, entry_data: Mapping[str, Any]) -> FlowResult: """Perform reauth upon an API authentication error.""" self._config = dict(entry_data) @@ -141,7 +161,7 @@ class PiHoleFlowHandler(config_entries.ConfigFlow, domain=DOMAIN): session, location=self._config[CONF_LOCATION], tls=self._config[CONF_SSL], - api_token=self._config[CONF_API_KEY], + api_token=self._config.get(CONF_API_KEY), ) try: await pi_hole.get_data() diff --git a/homeassistant/components/pi_hole/strings.json b/homeassistant/components/pi_hole/strings.json index 120ab8cb80a..2f04b8fe47e 100644 --- a/homeassistant/components/pi_hole/strings.json +++ b/homeassistant/components/pi_hole/strings.json @@ -7,11 +7,15 @@ "port": "[%key:common::config_flow::data::port%]", "name": "[%key:common::config_flow::data::name%]", "location": "[%key:common::config_flow::data::location%]", - "api_key": "[%key:common::config_flow::data::api_key%]", "ssl": "[%key:common::config_flow::data::ssl%]", "verify_ssl": "[%key:common::config_flow::data::verify_ssl%]" } }, + "api_key": { + "data": { + "api_key": "[%key:common::config_flow::data::api_key%]" + } + }, "reauth_confirm": { "title": "PI-Hole [%key:common::config_flow::title::reauth%]", "description": "Please enter a new api key for PI-Hole at {host}/{location}", diff --git a/homeassistant/components/pi_hole/translations/en.json b/homeassistant/components/pi_hole/translations/en.json index 815182731c2..d3561a08e6c 100644 --- a/homeassistant/components/pi_hole/translations/en.json +++ b/homeassistant/components/pi_hole/translations/en.json @@ -18,7 +18,6 @@ }, "user": { "data": { - "api_key": "API Key", "host": "Host", "location": "Location", "name": "Name", diff --git a/tests/components/pi_hole/__init__.py b/tests/components/pi_hole/__init__.py index 677a742726f..8295f933d46 100644 --- a/tests/components/pi_hole/__init__.py +++ b/tests/components/pi_hole/__init__.py @@ -73,14 +73,17 @@ CONFIG_DATA = { CONFIG_FLOW_USER = { CONF_HOST: HOST, CONF_PORT: PORT, - CONF_API_KEY: API_KEY, CONF_LOCATION: LOCATION, CONF_NAME: NAME, CONF_SSL: SSL, CONF_VERIFY_SSL: VERIFY_SSL, } -CONFIG_ENTRY = { +CONFIG_FLOW_API_KEY = { + CONF_API_KEY: API_KEY, +} + +CONFIG_ENTRY_WITH_API_KEY = { CONF_HOST: f"{HOST}:{PORT}", CONF_LOCATION: LOCATION, CONF_NAME: NAME, @@ -89,6 +92,13 @@ CONFIG_ENTRY = { CONF_VERIFY_SSL: VERIFY_SSL, } +CONFIG_ENTRY_WITHOUT_API_KEY = { + CONF_HOST: f"{HOST}:{PORT}", + CONF_LOCATION: LOCATION, + CONF_NAME: NAME, + CONF_SSL: SSL, + CONF_VERIFY_SSL: VERIFY_SSL, +} SWITCH_ENTITY_ID = "switch.pi_hole" @@ -121,3 +131,9 @@ def _patch_config_flow_hole(mocked_hole): return patch( "homeassistant.components.pi_hole.config_flow.Hole", return_value=mocked_hole ) + + +def _patch_setup_hole(): + return patch( + "homeassistant.components.pi_hole.async_setup_entry", return_value=True + ) diff --git a/tests/components/pi_hole/test_config_flow.py b/tests/components/pi_hole/test_config_flow.py index 9cc818df60f..05df5c2d322 100644 --- a/tests/components/pi_hole/test_config_flow.py +++ b/tests/components/pi_hole/test_config_flow.py @@ -8,22 +8,25 @@ from homeassistant.data_entry_flow import FlowResultType from . import ( CONFIG_DATA_DEFAULTS, - CONFIG_ENTRY, + CONFIG_ENTRY_WITH_API_KEY, + CONFIG_ENTRY_WITHOUT_API_KEY, + CONFIG_FLOW_API_KEY, CONFIG_FLOW_USER, NAME, ZERO_DATA, _create_mocked_hole, _patch_config_flow_hole, _patch_init_hole, + _patch_setup_hole, ) from tests.common import MockConfigEntry -async def test_flow_user(hass: HomeAssistant): - """Test user initialized flow.""" +async def test_flow_user_with_api_key(hass: HomeAssistant): + """Test user initialized flow with api key needed.""" mocked_hole = _create_mocked_hole(has_data=False) - with _patch_config_flow_hole(mocked_hole): + with _patch_config_flow_hole(mocked_hole), _patch_setup_hole() as mock_setup: result = await hass.config_entries.flow.async_init( DOMAIN, context={"source": SOURCE_USER}, @@ -37,17 +40,26 @@ async def test_flow_user(hass: HomeAssistant): user_input=CONFIG_FLOW_USER, ) assert result["type"] == FlowResultType.FORM - assert result["step_id"] == "user" + assert result["step_id"] == "api_key" + assert result["errors"] == {} + + result = await hass.config_entries.flow.async_configure( + result["flow_id"], + user_input={CONF_API_KEY: "some_key"}, + ) + assert result["type"] == FlowResultType.FORM + assert result["step_id"] == "api_key" 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=CONFIG_FLOW_USER, + user_input=CONFIG_FLOW_API_KEY, ) assert result["type"] == FlowResultType.CREATE_ENTRY assert result["title"] == NAME - assert result["data"] == CONFIG_ENTRY + assert result["data"] == CONFIG_ENTRY_WITH_API_KEY + mock_setup.assert_called_once() # duplicated server result = await hass.config_entries.flow.async_init( @@ -59,6 +71,28 @@ async def test_flow_user(hass: HomeAssistant): assert result["reason"] == "already_configured" +async def test_flow_user_without_api_key(hass: HomeAssistant): + """Test user initialized flow without api key needed.""" + mocked_hole = _create_mocked_hole() + with _patch_config_flow_hole(mocked_hole), _patch_setup_hole() as mock_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"] == {} + + result = await hass.config_entries.flow.async_configure( + result["flow_id"], + user_input=CONFIG_FLOW_USER, + ) + assert result["type"] == FlowResultType.CREATE_ENTRY + assert result["title"] == NAME + assert result["data"] == CONFIG_ENTRY_WITHOUT_API_KEY + mock_setup.assert_called_once() + + async def test_flow_user_invalid(hass: HomeAssistant): """Test user initialized flow with invalid server.""" mocked_hole = _create_mocked_hole(True) diff --git a/tests/components/pi_hole/test_init.py b/tests/components/pi_hole/test_init.py index c739f286cb4..52ca64a63af 100644 --- a/tests/components/pi_hole/test_init.py +++ b/tests/components/pi_hole/test_init.py @@ -10,8 +10,7 @@ from homeassistant.components.pi_hole.const import ( SERVICE_DISABLE, SERVICE_DISABLE_ATTR_DURATION, ) -from homeassistant.config_entries import ConfigEntryState -from homeassistant.const import ATTR_ENTITY_ID, CONF_API_KEY, CONF_HOST, CONF_NAME +from homeassistant.const import ATTR_ENTITY_ID, CONF_HOST, CONF_NAME from homeassistant.core import HomeAssistant from . import ( @@ -199,15 +198,3 @@ async def test_remove_obsolete(hass: HomeAssistant): 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() - 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