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 Linkplay mTLS/HTTPS and improve logging #124307

Merged
merged 13 commits into from
Sep 3, 2024
18 changes: 11 additions & 7 deletions homeassistant/components/linkplay/__init__.py
Original file line number Diff line number Diff line change
@@ -1,15 +1,17 @@
"""Support for LinkPlay devices."""

from aiohttp import ClientSession
from linkplay.bridge import LinkPlayBridge
from linkplay.discovery import linkplay_factory_bridge
from linkplay.discovery import linkplay_factory_httpapi_bridge
from linkplay.exceptions import LinkPlayRequestException

from homeassistant.config_entries import ConfigEntry
from homeassistant.const import CONF_HOST
from homeassistant.core import HomeAssistant
from homeassistant.exceptions import ConfigEntryNotReady
from homeassistant.helpers.aiohttp_client import async_get_clientsession

from .const import PLATFORMS
from .utils import async_get_client_session


class LinkPlayData:
Expand All @@ -24,13 +26,15 @@
async def async_setup_entry(hass: HomeAssistant, entry: LinkPlayConfigEntry) -> bool:
"""Async setup hass config entry. Called when an entry has been setup."""

session = async_get_clientsession(hass)
if (
bridge := await linkplay_factory_bridge(entry.data[CONF_HOST], session)
) is None:
session: ClientSession = await async_get_client_session(hass)
bridge: LinkPlayBridge | None = None

Check warning on line 30 in homeassistant/components/linkplay/__init__.py

View check run for this annotation

Codecov / codecov/patch

homeassistant/components/linkplay/__init__.py#L29-L30

Added lines #L29 - L30 were not covered by tests

try:
bridge = await linkplay_factory_httpapi_bridge(entry.data[CONF_HOST], session)
except LinkPlayRequestException as exception:

Check warning on line 34 in homeassistant/components/linkplay/__init__.py

View check run for this annotation

Codecov / codecov/patch

homeassistant/components/linkplay/__init__.py#L32-L34

Added lines #L32 - L34 were not covered by tests
raise ConfigEntryNotReady(
f"Failed to connect to LinkPlay device at {entry.data[CONF_HOST]}"
)
) from exception

entry.runtime_data = LinkPlayData()
entry.runtime_data.bridge = bridge
Expand Down
36 changes: 28 additions & 8 deletions homeassistant/components/linkplay/config_flow.py
Original file line number Diff line number Diff line change
@@ -1,16 +1,22 @@
"""Config flow to configure LinkPlay component."""

import logging
from typing import Any

from linkplay.discovery import linkplay_factory_bridge
from aiohttp import ClientSession
from linkplay.bridge import LinkPlayBridge
from linkplay.discovery import linkplay_factory_httpapi_bridge
from linkplay.exceptions import LinkPlayRequestException
import voluptuous as vol

from homeassistant.components import zeroconf
from homeassistant.config_entries import ConfigFlow, ConfigFlowResult
from homeassistant.const import CONF_HOST, CONF_MODEL
from homeassistant.helpers.aiohttp_client import async_get_clientsession

from .const import DOMAIN
from .utils import async_get_client_session

_LOGGER = logging.getLogger(__name__)


class LinkPlayConfigFlow(ConfigFlow, domain=DOMAIN):
Expand All @@ -25,10 +31,15 @@
) -> ConfigFlowResult:
"""Handle Zeroconf discovery."""

session = async_get_clientsession(self.hass)
bridge = await linkplay_factory_bridge(discovery_info.host, session)
session: ClientSession = await async_get_client_session(self.hass)
bridge: LinkPlayBridge | None = None

if bridge is None:
try:
bridge = await linkplay_factory_httpapi_bridge(discovery_info.host, session)
except LinkPlayRequestException:
_LOGGER.exception(

Check warning on line 40 in homeassistant/components/linkplay/config_flow.py

View check run for this annotation

Codecov / codecov/patch

homeassistant/components/linkplay/config_flow.py#L39-L40

Added lines #L39 - L40 were not covered by tests
"Failed to connect to LinkPlay device at %s", discovery_info.host
)
return self.async_abort(reason="cannot_connect")

self.data[CONF_HOST] = discovery_info.host
Expand Down Expand Up @@ -66,8 +77,18 @@
"""Handle a flow initialized by the user."""
errors: dict[str, str] = {}
if user_input:
session = async_get_clientsession(self.hass)
bridge = await linkplay_factory_bridge(user_input[CONF_HOST], session)
session: ClientSession = await async_get_client_session(self.hass)
bridge: LinkPlayBridge | None = None

try:
bridge = await linkplay_factory_httpapi_bridge(
user_input[CONF_HOST], session
)
except LinkPlayRequestException:
_LOGGER.exception(
"Failed to connect to LinkPlay device at %s", user_input[CONF_HOST]
)
errors["base"] = "cannot_connect"

if bridge is not None:
self.data[CONF_HOST] = user_input[CONF_HOST]
Expand All @@ -83,7 +104,6 @@
data={CONF_HOST: self.data[CONF_HOST]},
)

errors["base"] = "cannot_connect"
return self.async_show_form(
step_id="user",
data_schema=vol.Schema({vol.Required(CONF_HOST): str}),
Expand Down
1 change: 1 addition & 0 deletions homeassistant/components/linkplay/const.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,3 +4,4 @@

DOMAIN = "linkplay"
PLATFORMS = [Platform.MEDIA_PLAYER]
CONF_SESSION = "session"
Copy link
Member

@MartinHjelmare MartinHjelmare Sep 4, 2024

Choose a reason for hiding this comment

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

We don't prefix constants with CONF_ unless it's a key used for configuration. Use eg DATA_SESSION instead.

11 changes: 11 additions & 0 deletions homeassistant/components/linkplay/media_player.py
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,17 @@
PlayingMode.XLR: "XLR",
PlayingMode.HDMI: "HDMI",
PlayingMode.OPTICAL_2: "Optical 2",
PlayingMode.EXTERN_BLUETOOTH: "External Bluetooth",
PlayingMode.PHONO: "Phono",
PlayingMode.ARC: "ARC",
PlayingMode.COAXIAL_2: "Coaxial 2",
PlayingMode.TF_CARD_1: "SD Card 1",
PlayingMode.TF_CARD_2: "SD Card 2",
PlayingMode.CD: "CD",
PlayingMode.DAB: "DAB Radio",
PlayingMode.FM: "FM Radio",
PlayingMode.RCA: "RCA",
PlayingMode.UDISK: "USB",
}

SOURCE_MAP_INV: dict[str, PlayingMode] = {v: k for k, v in SOURCE_MAP.items()}
Expand Down
22 changes: 22 additions & 0 deletions homeassistant/components/linkplay/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,16 @@

from typing import Final

from aiohttp import ClientSession
from linkplay.utils import async_create_unverified_client_session

from homeassistant.core import HomeAssistant
from homeassistant.helpers.aiohttp_client import (
_async_register_default_clientsession_shutdown,
dukeofphilberg marked this conversation as resolved.
Show resolved Hide resolved
)

from .const import CONF_SESSION, DOMAIN

MANUFACTURER_ARTSOUND: Final[str] = "ArtSound"
MANUFACTURER_ARYLIC: Final[str] = "Arylic"
MANUFACTURER_IEAST: Final[str] = "iEAST"
Expand Down Expand Up @@ -44,3 +54,15 @@
return MANUFACTURER_IEAST, MODELS_IEAST_AUDIOCAST_M5
case _:
return MANUFACTURER_GENERIC, MODELS_GENERIC


async def async_get_client_session(hass: HomeAssistant) -> ClientSession:
"""Get a ClientSession that can be used with LinkPlay devices."""
hass.data.setdefault(DOMAIN, {})
if CONF_SESSION not in hass.data[DOMAIN]:
clientsession: ClientSession = await async_create_unverified_client_session()
dukeofphilberg marked this conversation as resolved.
Show resolved Hide resolved
_async_register_default_clientsession_shutdown(hass, clientsession)
return clientsession

Check warning on line 65 in homeassistant/components/linkplay/utils.py

View check run for this annotation

Codecov / codecov/patch

homeassistant/components/linkplay/utils.py#L61-L65

Added lines #L61 - L65 were not covered by tests

session: ClientSession = hass.data[DOMAIN][CONF_SESSION]
return session

Check warning on line 68 in homeassistant/components/linkplay/utils.py

View check run for this annotation

Codecov / codecov/patch

homeassistant/components/linkplay/utils.py#L67-L68

Added lines #L67 - L68 were not covered by tests
9 changes: 7 additions & 2 deletions tests/components/linkplay/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
from collections.abc import Generator
from unittest.mock import AsyncMock, patch

from aiohttp import ClientSession
from linkplay.bridge import LinkPlayBridge, LinkPlayDevice
import pytest

Expand All @@ -14,11 +15,15 @@

@pytest.fixture
def mock_linkplay_factory_bridge() -> Generator[AsyncMock]:
"""Mock for linkplay_factory_bridge."""
"""Mock for linkplay_factory_httpapi_bridge."""

with (
patch(
"homeassistant.components.linkplay.config_flow.linkplay_factory_bridge"
"homeassistant.components.linkplay.config_flow.async_get_client_session",
return_value=AsyncMock(spec=ClientSession),
),
patch(
"homeassistant.components.linkplay.config_flow.linkplay_factory_httpapi_bridge",
) as factory,
):
bridge = AsyncMock(spec=LinkPlayBridge)
Expand Down
26 changes: 12 additions & 14 deletions tests/components/linkplay/test_config_flow.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,9 @@
from ipaddress import ip_address
from unittest.mock import AsyncMock

from linkplay.exceptions import LinkPlayRequestException
import pytest

from homeassistant.components.linkplay.const import DOMAIN
from homeassistant.components.zeroconf import ZeroconfServiceInfo
from homeassistant.config_entries import SOURCE_USER, SOURCE_ZEROCONF
Expand Down Expand Up @@ -47,10 +50,9 @@
)


@pytest.mark.usefixtures("mock_linkplay_factory_bridge", "mock_setup_entry")
async def test_user_flow(
hass: HomeAssistant,
mock_linkplay_factory_bridge: AsyncMock,
mock_setup_entry: AsyncMock,
) -> None:
"""Test user setup config flow."""
result = await hass.config_entries.flow.async_init(
Expand All @@ -74,10 +76,9 @@ async def test_user_flow(
assert result["result"].unique_id == UUID


@pytest.mark.usefixtures("mock_linkplay_factory_bridge")
async def test_user_flow_re_entry(
hass: HomeAssistant,
mock_linkplay_factory_bridge: AsyncMock,
mock_setup_entry: AsyncMock,
) -> None:
"""Test user setup config flow when an entry with the same unique id already exists."""

Expand Down Expand Up @@ -105,10 +106,9 @@ async def test_user_flow_re_entry(
assert result["reason"] == "already_configured"


@pytest.mark.usefixtures("mock_linkplay_factory_bridge", "mock_setup_entry")
async def test_zeroconf_flow(
hass: HomeAssistant,
mock_linkplay_factory_bridge: AsyncMock,
mock_setup_entry: AsyncMock,
) -> None:
"""Test Zeroconf flow."""
result = await hass.config_entries.flow.async_init(
Expand All @@ -133,10 +133,9 @@ async def test_zeroconf_flow(
assert result["result"].unique_id == UUID


@pytest.mark.usefixtures("mock_linkplay_factory_bridge")
async def test_zeroconf_flow_re_entry(
hass: HomeAssistant,
mock_linkplay_factory_bridge: AsyncMock,
mock_setup_entry: AsyncMock,
) -> None:
"""Test Zeroconf flow when an entry with the same unique id already exists."""

Expand All @@ -160,16 +159,15 @@ async def test_zeroconf_flow_re_entry(
assert result["reason"] == "already_configured"


@pytest.mark.usefixtures("mock_setup_entry")
async def test_flow_errors(
hass: HomeAssistant,
mock_linkplay_factory_bridge: AsyncMock,
mock_setup_entry: AsyncMock,
) -> None:
"""Test flow when the device cannot be reached."""

# Temporarily store bridge in a separate variable and set factory to return None
bridge = mock_linkplay_factory_bridge.return_value
mock_linkplay_factory_bridge.return_value = None
# Temporarily make the mock_linkplay_factory_bridge throw an exception
mock_linkplay_factory_bridge.side_effect = (LinkPlayRequestException("Error"),)

result = await hass.config_entries.flow.async_init(
DOMAIN,
Expand All @@ -188,8 +186,8 @@ async def test_flow_errors(
assert result["step_id"] == "user"
assert result["errors"] == {"base": "cannot_connect"}

# Make linkplay_factory_bridge return a mock bridge again
mock_linkplay_factory_bridge.return_value = bridge
# Make mock_linkplay_factory_bridge_exception no longer throw an exception
mock_linkplay_factory_bridge.side_effect = None

result = await hass.config_entries.flow.async_configure(
result["flow_id"],
Expand Down