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

Add HEOS options flow for optional authentication #134105

Open
wants to merge 11 commits into
base: dev
Choose a base branch
from
41 changes: 31 additions & 10 deletions homeassistant/components/heos/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,23 @@
from datetime import timedelta
import logging

from pyheos import Heos, HeosError, HeosOptions, HeosPlayer, const as heos_const
from pyheos import (
Credentials,
Heos,
HeosError,
HeosOptions,
HeosPlayer,
const as heos_const,
)

from homeassistant.config_entries import ConfigEntry
from homeassistant.const import CONF_HOST, EVENT_HOMEASSISTANT_STOP, Platform
from homeassistant.const import (
CONF_HOST,
CONF_PASSWORD,
CONF_USERNAME,
EVENT_HOMEASSISTANT_STOP,
Platform,
)
from homeassistant.core import CALLBACK_TYPE, HomeAssistant, callback
from homeassistant.exceptions import ConfigEntryNotReady, HomeAssistantError
from homeassistant.helpers import device_registry as dr, entity_registry as er
Expand Down Expand Up @@ -56,9 +69,22 @@ async def async_setup_entry(hass: HomeAssistant, entry: HeosConfigEntry) -> bool
hass.config_entries.async_update_entry(entry, unique_id=DOMAIN)

host = entry.data[CONF_HOST]
credentials: Credentials | None = None
if entry.options:
credentials = Credentials(
entry.options[CONF_USERNAME], entry.options[CONF_PASSWORD]
)

# Setting all_progress_events=False ensures that we only receive a
# media position update upon start of playback or when media changes
controller = Heos(HeosOptions(host, all_progress_events=False, auto_reconnect=True))
controller = Heos(
HeosOptions(
host,
all_progress_events=False,
auto_reconnect=True,
credentials=credentials,
)
)
try:
await controller.connect()
# Auto reconnect only operates if initial connection was successful.
Expand All @@ -82,13 +108,8 @@ async def disconnect_controller(event):
if controller.is_signed_in:
favorites = await controller.get_favorites()
else:
_LOGGER.warning(
(
"%s is not logged in to a HEOS account and will be unable to"
" retrieve HEOS favorites: Use the 'heos.sign_in' service to"
" sign-in to a HEOS account"
),
host,
_LOGGER.info(
"The HEOS System is not logged in: Enter credentials in the integration options to access favorites and streaming services"
)
inputs = await controller.get_input_sources()
except HeosError as error:
Expand Down
93 changes: 89 additions & 4 deletions homeassistant/components/heos/config_flow.py
andrewsayre marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
@@ -1,17 +1,27 @@
"""Config flow to configure Heos."""

from typing import TYPE_CHECKING, Any
import logging
from typing import TYPE_CHECKING, Any, cast
from urllib.parse import urlparse

from pyheos import Heos, HeosError, HeosOptions
from pyheos import CommandFailedError, Heos, HeosError, HeosOptions
import voluptuous as vol

from homeassistant.components import ssdp
from homeassistant.config_entries import ConfigFlow, ConfigFlowResult
from homeassistant.const import CONF_HOST
from homeassistant.config_entries import (
ConfigEntry,
ConfigFlow,
ConfigFlowResult,
OptionsFlow,
)
from homeassistant.const import CONF_HOST, CONF_PASSWORD, CONF_USERNAME
from homeassistant.core import callback
from homeassistant.helpers import selector

from .const import DOMAIN

_LOGGER = logging.getLogger(__name__)


def format_title(host: str) -> str:
"""Format the title for config entries."""
Expand All @@ -36,6 +46,12 @@ class HeosFlowHandler(ConfigFlow, domain=DOMAIN):

VERSION = 1

@staticmethod
@callback
def async_get_options_flow(config_entry: ConfigEntry) -> OptionsFlow:
"""Create the options flow."""
return HeosOptionsFlowHandler()

async def async_step_ssdp(
self, discovery_info: ssdp.SsdpServiceInfo
) -> ConfigFlowResult:
Expand Down Expand Up @@ -100,3 +116,72 @@ async def async_step_reconfigure(
data_schema=vol.Schema({vol.Required(CONF_HOST, default=host): str}),
errors=errors,
)


OPTIONS_SCHEMA = vol.Schema(
{
vol.Optional(CONF_USERNAME): selector.TextSelector(),
vol.Optional(CONF_PASSWORD): selector.TextSelector(
selector.TextSelectorConfig(type=selector.TextSelectorType.PASSWORD)
),
}
)


class HeosOptionsFlowHandler(OptionsFlow):
"""Define HEOS options flow."""

async def async_step_init(
self, user_input: dict[str, Any] | None = None
) -> ConfigFlowResult:
"""Manage the options."""
errors: dict[str, str] = {}
if user_input is not None:
authentication = CONF_USERNAME in user_input or CONF_PASSWORD in user_input
if authentication and CONF_USERNAME not in user_input:
errors[CONF_USERNAME] = "username_missing"
if authentication and CONF_PASSWORD not in user_input:
errors[CONF_PASSWORD] = "password_missing"
Comment on lines +140 to +144
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we can avoid doing this using vol.All in the schema, I'm just not sure if we have a prebuilt validator for this

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How do I use vol.All this way? I searched the code base and didn't see other examples.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

zwave_js has a couple of validators check has_at_least_one_node https://github.com/home-assistant/core/blob/dev/homeassistant/components/zwave_js/services.py#L263

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using vol.All inside vol.Schema or at the top level causes a ValueError: dictionary update sequence element #0 has length 3; 2 is required at

data["data_schema"] = voluptuous_serialize.convert(

Here's what I have (that does not work):

def has_both_or_neither(val: dict[str, Any]) -> dict[str, Any]:
    """Validate that at least one node is specified."""
    if val.get(CONF_USERNAME) ^ val.get(CONF_PASSWORD):
        raise vol.Invalid(
            "Either both or neither of username and password must be specified"
        )
    return val


OPTIONS_SCHEMA = vol.Schema(
    vol.All(
        {
            vol.Optional(CONF_USERNAME): selector.TextSelector(),
            vol.Optional(CONF_PASSWORD): selector.TextSelector(
                selector.TextSelectorConfig(type=selector.TextSelectorType.PASSWORD)
            ),
        },
        has_both_or_neither,
    )
)

I couldn't find any other options flow schemas that use vol.All this way nor use custom validation functions. FWIW, I followed the same pattern that the OpenSky integration did:

async def async_step_init(
self, user_input: dict[str, Any] | None = None
) -> ConfigFlowResult:
"""Initialize form."""
errors: dict[str, str] = {}
if user_input is not None:
authentication = CONF_USERNAME in user_input or CONF_PASSWORD in user_input
if authentication and CONF_USERNAME not in user_input:
errors["base"] = "username_missing"
if authentication and CONF_PASSWORD not in user_input:
errors["base"] = "password_missing"
if user_input[CONF_CONTRIBUTING_USER] and not authentication:
errors["base"] = "no_authentication"
if authentication and not errors:
opensky = OpenSky(session=async_get_clientsession(self.hass))
try:
await opensky.authenticate(
BasicAuth(
login=user_input[CONF_USERNAME],
password=user_input[CONF_PASSWORD],
),
contributing_user=user_input[CONF_CONTRIBUTING_USER],
)
except OpenSkyUnauthenticatedError:
errors["base"] = "invalid_auth"
if not errors:
return self.async_create_entry(data=user_input)
return self.async_show_form(
step_id="init",
errors=errors,
data_schema=self.add_suggested_values_to_schema(
vol.Schema(
{
vol.Required(CONF_RADIUS): vol.Coerce(float),
vol.Optional(CONF_ALTITUDE): vol.Coerce(float),
vol.Optional(CONF_USERNAME): str,
vol.Optional(CONF_PASSWORD): str,
vol.Optional(CONF_CONTRIBUTING_USER, default=False): bool,
}
),
user_input or self.config_entry.options,
),
)

What do you suggest?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

actually, I was looking for vol.Inclusive (but just found it again) https://github.com/home-assistant/core/blob/dev/homeassistant/components/android_ip_webcam/config_flow.py#L23-L24, which should achieve what we want


if not errors:
heos = cast(
Heos, self.config_entry.runtime_data.controller_manager.controller
)

try:
if user_input:
# Attempt to login
try:
await heos.sign_in(
user_input[CONF_USERNAME], user_input[CONF_PASSWORD]
)
_LOGGER.info(
"Successfully signed-in to HEOS Account: %s",
heos.signed_in_username,
)
except CommandFailedError as err:
if err.error_id in (6, 8, 10): # Auth-specific errors
errors["base"] = "invalid_auth"
_LOGGER.info(
"Failed to sign-in to HEOS Account: %s", err
)
else:
raise # Re-raise unexpected error
else:
# Log out
await heos.sign_out()
andrewsayre marked this conversation as resolved.
Show resolved Hide resolved
_LOGGER.info("Successfully signed-out of HEOS Account")
except HeosError:
errors["base"] = "unknown"
_LOGGER.exception("Unexpected error occurred during sign-in/out")

if not errors:
return self.async_create_entry(data=user_input)

return self.async_show_form(
errors=errors,
step_id="init",
data_schema=self.add_suggested_values_to_schema(
OPTIONS_SCHEMA, user_input or self.config_entry.options
),
)
5 changes: 1 addition & 4 deletions homeassistant/components/heos/quality_scale.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -33,10 +33,7 @@ rules:
status: todo
comment: Actions currently only log and instead should raise exceptions.
config-entry-unloading: done
docs-configuration-parameters:
status: done
comment: |
The integration doesn't provide any additional configuration parameters.
docs-configuration-parameters: done
docs-installation-parameters: done
entity-unavailable: done
integration-owner: done
Expand Down
22 changes: 22 additions & 0 deletions homeassistant/components/heos/strings.json
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,28 @@
"single_instance_allowed": "[%key:common::config_flow::abort::single_instance_allowed%]"
}
},
"options": {
"step": {
"init": {
"title": "HEOS Options",
"description": "You can sign-in to your HEOS Account to access favorites, streaming services, and other features. Clearing the credentials will sign-out of your account.",
"data": {
"username": "[%key:common::config_flow::data::username%]",
"password": "[%key:common::config_flow::data::password%]"
},
"data_description": {
"username": "The username or e-mail address of your HEOS Account.",
"password": "The password to your HEOS Account."
}
}
},
"error": {
"username_missing": "Username is missing",
"password_missing": "Password is missing",
"invalid_auth": "[%key:common::config_flow::error::invalid_auth%]",
"unknown": "[%key:common::config_flow::error::unknown%]"
}
},
"services": {
"sign_in": {
"name": "Sign in",
Expand Down
53 changes: 46 additions & 7 deletions tests/components/heos/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,21 +18,60 @@
import pytest_asyncio

from homeassistant.components import ssdp
from homeassistant.components.heos import DOMAIN
from homeassistant.const import CONF_HOST
from homeassistant.components.heos import (
CONF_PASSWORD,
DOMAIN,
ControllerManager,
GroupManager,
HeosRuntimeData,
SourceManager,
)
from homeassistant.const import CONF_HOST, CONF_USERNAME

from tests.common import MockConfigEntry


@pytest.fixture(name="config_entry")
def config_entry_fixture():
def config_entry_fixture(heos_runtime_data):
"""Create a mock HEOS config entry."""
return MockConfigEntry(
entry = MockConfigEntry(
domain=DOMAIN,
data={CONF_HOST: "127.0.0.1"},
title="HEOS System (via 127.0.0.1)",
unique_id=DOMAIN,
)
entry.runtime_data = heos_runtime_data
return entry


@pytest.fixture(name="config_entry_options")
def config_entry_options_fixture(heos_runtime_data):
"""Create a mock HEOS config entry with options."""
entry = MockConfigEntry(
domain=DOMAIN,
data={CONF_HOST: "127.0.0.1"},
title="HEOS System (via 127.0.0.1)",
options={CONF_USERNAME: "user", CONF_PASSWORD: "pass"},
unique_id=DOMAIN,
)
entry.runtime_data = heos_runtime_data
return entry


@pytest.fixture(name="heos_runtime_data")
def heos_runtime_data_fixture(controller_manager, players):
"""Create a mock HeosRuntimeData fixture."""
return HeosRuntimeData(
controller_manager, Mock(GroupManager), Mock(SourceManager), players
)


@pytest.fixture(name="controller_manager")
def controller_manager_fixture(controller):
"""Create a mock controller manager fixture."""
mock_controller_manager = Mock(ControllerManager)
mock_controller_manager.controller = controller
return mock_controller_manager


@pytest.fixture(name="controller")
Expand All @@ -43,6 +82,7 @@ def controller_fixture(
mock_heos = Mock(Heos)
for player in players.values():
player.heos = mock_heos
mock_heos.return_value = mock_heos
mock_heos.dispatcher = dispatcher
mock_heos.get_players.return_value = players
mock_heos.players = players
Expand All @@ -55,11 +95,10 @@ def controller_fixture(
mock_heos.connection_state = const.STATE_CONNECTED
mock_heos.get_groups.return_value = group
mock_heos.create_group.return_value = None
mock = Mock(return_value=mock_heos)

with (
patch("homeassistant.components.heos.Heos", new=mock),
patch("homeassistant.components.heos.config_flow.Heos", new=mock),
patch("homeassistant.components.heos.Heos", new=mock_heos),
patch("homeassistant.components.heos.config_flow.Heos", new=mock_heos),
):
yield mock_heos

Expand Down
Loading