Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support password less PI-Hole installations #86184

Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 0 additions & 4 deletions homeassistant/components/pi_hole/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
26 changes: 23 additions & 3 deletions homeassistant/components/pi_hole/config_flow.py
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,6 @@ async def async_step_user(
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(
Expand All @@ -70,6 +69,9 @@ async def async_step_user(
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",
Expand All @@ -79,7 +81,6 @@ async def async_step_user(
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,
Expand All @@ -100,6 +101,25 @@ async def async_step_user(
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)
Expand Down Expand Up @@ -141,7 +161,7 @@ async def _async_try_connect(self) -> dict[str, str]:
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()
Expand Down
6 changes: 5 additions & 1 deletion homeassistant/components/pi_hole/strings.json
Original file line number Diff line number Diff line change
Expand Up @@ -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}",
Expand Down
8 changes: 0 additions & 8 deletions homeassistant/components/pi_hole/translations/en.json
Original file line number Diff line number Diff line change
Expand Up @@ -23,22 +23,14 @@
},
"user": {
"data": {
"api_key": "API Key",
"host": "Host",
"location": "Location",
"name": "Name",
"port": "Port",
"ssl": "Uses an SSL certificate",
"statistics_only": "Statistics Only",
"verify_ssl": "Verify SSL certificate"
}
}
}
},
"issues": {
"deprecated_yaml": {
"description": "Configuring PI-Hole using YAML is being removed.\n\nYour existing YAML configuration has been imported into the UI automatically.\n\nRemove the PI-Hole YAML configuration from your configuration.yaml file and restart Home Assistant to fix this issue.",
"title": "The PI-Hole YAML configuration is being removed"
}
}
}
14 changes: 12 additions & 2 deletions tests/components/pi_hole/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -73,14 +73,17 @@
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,
Expand All @@ -89,6 +92,13 @@
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"


Expand Down
37 changes: 30 additions & 7 deletions tests/components/pi_hole/test_config_flow.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,9 @@

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,
Expand All @@ -20,8 +22,8 @@
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):
result = await hass.config_entries.flow.async_init(
Expand All @@ -37,17 +39,17 @@ async def test_flow_user(hass: HomeAssistant):
user_input=CONFIG_FLOW_USER,
)
assert result["type"] == FlowResultType.FORM
assert result["step_id"] == "user"
assert result["errors"] == {CONF_API_KEY: "invalid_auth"}
assert result["step_id"] == "api_key"
assert result["errors"] == {}
mib1185 marked this conversation as resolved.
Show resolved Hide resolved

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

# duplicated server
result = await hass.config_entries.flow.async_init(
Expand All @@ -59,6 +61,27 @@ 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):
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(
mib1185 marked this conversation as resolved.
Show resolved Hide resolved
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


async def test_flow_user_invalid(hass: HomeAssistant):
"""Test user initialized flow with invalid server."""
mocked_hole = _create_mocked_hole(True)
Expand Down
15 changes: 1 addition & 14 deletions tests/components/pi_hole/test_init.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,7 @@
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 (
Expand Down Expand Up @@ -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