diff --git a/CODEOWNERS b/CODEOWNERS index c7bc33d244f..9890a8f3502 100644 --- a/CODEOWNERS +++ b/CODEOWNERS @@ -568,8 +568,8 @@ build.json @home-assistant/supervisor /tests/components/isy994/ @bdraco @shbatm /homeassistant/components/izone/ @Swamp-Ig /tests/components/izone/ @Swamp-Ig -/homeassistant/components/jellyfin/ @j-stienstra -/tests/components/jellyfin/ @j-stienstra +/homeassistant/components/jellyfin/ @j-stienstra @ctalkington +/tests/components/jellyfin/ @j-stienstra @ctalkington /homeassistant/components/jewish_calendar/ @tsvi /tests/components/jewish_calendar/ @tsvi /homeassistant/components/juicenet/ @jesserockz diff --git a/homeassistant/components/jellyfin/__init__.py b/homeassistant/components/jellyfin/__init__.py index a58108b05ab..0126c05e4f2 100644 --- a/homeassistant/components/jellyfin/__init__.py +++ b/homeassistant/components/jellyfin/__init__.py @@ -6,7 +6,7 @@ from homeassistant.core import HomeAssistant from homeassistant.exceptions import ConfigEntryNotReady from .client_wrapper import CannotConnect, InvalidAuth, create_client, validate_input -from .const import DATA_CLIENT, DOMAIN +from .const import CONF_CLIENT_DEVICE_ID, DATA_CLIENT, DOMAIN _LOGGER = logging.getLogger(__name__) @@ -15,7 +15,13 @@ async def async_setup_entry(hass: HomeAssistant, entry: ConfigEntry) -> bool: """Set up Jellyfin from a config entry.""" hass.data.setdefault(DOMAIN, {}) - client = create_client() + if CONF_CLIENT_DEVICE_ID not in entry.data: + entry_data = entry.data.copy() + entry_data[CONF_CLIENT_DEVICE_ID] = entry.entry_id + hass.config_entries.async_update_entry(entry, data=entry_data) + + client = create_client(device_id=entry.data[CONF_CLIENT_DEVICE_ID]) + try: await validate_input(hass, dict(entry.data), client) except CannotConnect as ex: diff --git a/homeassistant/components/jellyfin/client_wrapper.py b/homeassistant/components/jellyfin/client_wrapper.py index 9f6380e2181..65de5d4232e 100644 --- a/homeassistant/components/jellyfin/client_wrapper.py +++ b/homeassistant/components/jellyfin/client_wrapper.py @@ -3,7 +3,6 @@ from __future__ import annotations import socket from typing import Any -import uuid from jellyfin_apiclient_python import Jellyfin, JellyfinClient from jellyfin_apiclient_python.api import API @@ -34,22 +33,19 @@ async def validate_input( return userid -def create_client() -> JellyfinClient: +def create_client(device_id: str, device_name: str | None = None) -> JellyfinClient: """Create a new Jellyfin client.""" + if device_name is None: + device_name = socket.gethostname() + jellyfin = Jellyfin() + client = jellyfin.get_client() - _setup_client(client) - return client - - -def _setup_client(client: JellyfinClient) -> None: - """Configure the Jellyfin client with a number of required properties.""" - player_name = socket.gethostname() - client_uuid = str(uuid.uuid4()) - - client.config.app(USER_APP_NAME, CLIENT_VERSION, player_name, client_uuid) + client.config.app(USER_APP_NAME, CLIENT_VERSION, device_name, device_id) client.config.http(USER_AGENT) + return client + def _connect(client: JellyfinClient, url: str, username: str, password: str) -> str: """Connect to the Jellyfin server and assert that the user can login.""" @@ -75,6 +71,7 @@ def _login( ) -> None: """Assert that the user can log in to the Jellyfin server.""" response = connection_manager.login(url, username, password) + if "AccessToken" not in response: raise InvalidAuth diff --git a/homeassistant/components/jellyfin/config_flow.py b/homeassistant/components/jellyfin/config_flow.py index 6820031ed7b..51553f1a6f2 100644 --- a/homeassistant/components/jellyfin/config_flow.py +++ b/homeassistant/components/jellyfin/config_flow.py @@ -9,9 +9,10 @@ import voluptuous as vol from homeassistant import config_entries from homeassistant.const import CONF_PASSWORD, CONF_URL, CONF_USERNAME from homeassistant.data_entry_flow import FlowResult +from homeassistant.util.uuid import random_uuid_hex from .client_wrapper import CannotConnect, InvalidAuth, create_client, validate_input -from .const import DOMAIN +from .const import CONF_CLIENT_DEVICE_ID, DOMAIN _LOGGER = logging.getLogger(__name__) @@ -24,11 +25,20 @@ STEP_USER_DATA_SCHEMA = vol.Schema( ) +def _generate_client_device_id() -> str: + """Generate a random UUID4 string to identify ourselves.""" + return random_uuid_hex() + + class ConfigFlow(config_entries.ConfigFlow, domain=DOMAIN): """Handle a config flow for Jellyfin.""" VERSION = 1 + def __init__(self) -> None: + """Initialize the Jellyfin config flow.""" + self.client_device_id: str | None = None + async def async_step_user( self, user_input: dict[str, Any] | None = None ) -> FlowResult: @@ -39,7 +49,10 @@ class ConfigFlow(config_entries.ConfigFlow, domain=DOMAIN): errors: dict[str, str] = {} if user_input is not None: - client = create_client() + if self.client_device_id is None: + self.client_device_id = _generate_client_device_id() + + client = create_client(device_id=self.client_device_id) try: userid = await validate_input(self.hass, user_input, client) except CannotConnect: @@ -54,7 +67,8 @@ class ConfigFlow(config_entries.ConfigFlow, domain=DOMAIN): self._abort_if_unique_id_configured() return self.async_create_entry( - title=user_input[CONF_URL], data=user_input + title=user_input[CONF_URL], + data={CONF_CLIENT_DEVICE_ID: self.client_device_id, **user_input}, ) return self.async_show_form( diff --git a/homeassistant/components/jellyfin/const.py b/homeassistant/components/jellyfin/const.py index 1f679fd43c8..182144806d2 100644 --- a/homeassistant/components/jellyfin/const.py +++ b/homeassistant/components/jellyfin/const.py @@ -9,6 +9,8 @@ CLIENT_VERSION: Final = "1.0" COLLECTION_TYPE_MOVIES: Final = "movies" COLLECTION_TYPE_MUSIC: Final = "music" +CONF_CLIENT_DEVICE_ID: Final = "client_device_id" + DATA_CLIENT: Final = "client" ITEM_KEY_COLLECTION_TYPE: Final = "CollectionType" diff --git a/homeassistant/components/jellyfin/manifest.json b/homeassistant/components/jellyfin/manifest.json index 48f4cf0c837..e2189bed2cb 100644 --- a/homeassistant/components/jellyfin/manifest.json +++ b/homeassistant/components/jellyfin/manifest.json @@ -5,6 +5,6 @@ "documentation": "https://www.home-assistant.io/integrations/jellyfin", "requirements": ["jellyfin-apiclient-python==1.8.1"], "iot_class": "local_polling", - "codeowners": ["@j-stienstra"], + "codeowners": ["@j-stienstra", "@ctalkington"], "loggers": ["jellyfin_apiclient_python"] } diff --git a/tests/components/jellyfin/conftest.py b/tests/components/jellyfin/conftest.py new file mode 100644 index 00000000000..c1d9634aede --- /dev/null +++ b/tests/components/jellyfin/conftest.py @@ -0,0 +1,89 @@ +"""Fixtures for Jellyfin integration tests.""" +from __future__ import annotations + +from collections.abc import Generator +from unittest.mock import AsyncMock, MagicMock, create_autospec, patch + +from jellyfin_apiclient_python import JellyfinClient +from jellyfin_apiclient_python.api import API +from jellyfin_apiclient_python.configuration import Config +from jellyfin_apiclient_python.connection_manager import ConnectionManager +import pytest + +from .const import ( + MOCK_SUCCESFUL_CONNECTION_STATE, + MOCK_SUCCESFUL_LOGIN_RESPONSE, + MOCK_USER_SETTINGS, +) + + +@pytest.fixture +def mock_setup_entry() -> Generator[AsyncMock, None, None]: + """Mock setting up a config entry.""" + with patch( + "homeassistant.components.jellyfin.async_setup_entry", return_value=True + ) as setup_mock: + yield setup_mock + + +@pytest.fixture +def mock_client_device_id() -> Generator[None, MagicMock, None]: + """Mock generating device id.""" + with patch( + "homeassistant.components.jellyfin.config_flow._generate_client_device_id" + ) as id_mock: + id_mock.return_value = "TEST-UUID" + yield id_mock + + +@pytest.fixture +def mock_auth() -> MagicMock: + """Return a mocked ConnectionManager.""" + jf_auth = create_autospec(ConnectionManager) + jf_auth.connect_to_address.return_value = MOCK_SUCCESFUL_CONNECTION_STATE + jf_auth.login.return_value = MOCK_SUCCESFUL_LOGIN_RESPONSE + + return jf_auth + + +@pytest.fixture +def mock_api() -> MagicMock: + """Return a mocked API.""" + jf_api = create_autospec(API) + jf_api.get_user_settings.return_value = MOCK_USER_SETTINGS + + return jf_api + + +@pytest.fixture +def mock_config() -> MagicMock: + """Return a mocked JellyfinClient.""" + jf_config = create_autospec(Config) + jf_config.data = {} + + return jf_config + + +@pytest.fixture +def mock_client( + mock_config: MagicMock, mock_auth: MagicMock, mock_api: MagicMock +) -> MagicMock: + """Return a mocked JellyfinClient.""" + jf_client = create_autospec(JellyfinClient) + jf_client.auth = mock_auth + jf_client.config = mock_config + jf_client.jellyfin = mock_api + + return jf_client + + +@pytest.fixture +def mock_jellyfin(mock_client: MagicMock) -> Generator[None, MagicMock, None]: + """Return a mocked Jellyfin.""" + with patch( + "homeassistant.components.jellyfin.client_wrapper.Jellyfin", autospec=True + ) as jellyfin_mock: + jf = jellyfin_mock.return_value + jf.get_client.return_value = mock_client + + yield jf diff --git a/tests/components/jellyfin/test_config_flow.py b/tests/components/jellyfin/test_config_flow.py index e898f8ac5ce..be90e521ac1 100644 --- a/tests/components/jellyfin/test_config_flow.py +++ b/tests/components/jellyfin/test_config_flow.py @@ -1,17 +1,15 @@ """Test the jellyfin config flow.""" -from unittest.mock import patch +from unittest.mock import MagicMock from homeassistant import config_entries, data_entry_flow -from homeassistant.components.jellyfin.const import DOMAIN +from homeassistant.components.jellyfin.const import CONF_CLIENT_DEVICE_ID, DOMAIN from homeassistant.const import CONF_PASSWORD, CONF_URL, CONF_USERNAME from homeassistant.core import HomeAssistant from .const import ( - MOCK_SUCCESFUL_CONNECTION_STATE, MOCK_SUCCESFUL_LOGIN_RESPONSE, MOCK_UNSUCCESFUL_CONNECTION_STATE, MOCK_UNSUCCESFUL_LOGIN_RESPONSE, - MOCK_USER_SETTINGS, TEST_PASSWORD, TEST_URL, TEST_USERNAME, @@ -31,52 +29,52 @@ async def test_abort_if_existing_entry(hass: HomeAssistant): assert result["reason"] == "single_instance_allowed" -async def test_form(hass: HomeAssistant): +async def test_form( + hass: HomeAssistant, + mock_jellyfin: MagicMock, + mock_client: MagicMock, + mock_client_device_id: MagicMock, + mock_setup_entry: MagicMock, +): """Test the complete configuration form.""" result = await hass.config_entries.flow.async_init( DOMAIN, context={"source": config_entries.SOURCE_USER} ) + assert result["type"] == "form" assert result["errors"] == {} - with patch( - "homeassistant.components.jellyfin.client_wrapper.ConnectionManager.connect_to_address", - return_value=MOCK_SUCCESFUL_CONNECTION_STATE, - ) as mock_connect, patch( - "homeassistant.components.jellyfin.client_wrapper.ConnectionManager.login", - return_value=MOCK_SUCCESFUL_LOGIN_RESPONSE, - ) as mock_login, patch( - "homeassistant.components.jellyfin.async_setup_entry", - return_value=True, - ) as mock_setup_entry, patch( - "homeassistant.components.jellyfin.client_wrapper.API.get_user_settings", - return_value=MOCK_USER_SETTINGS, - ) as mock_set_id: - result2 = await hass.config_entries.flow.async_configure( - result["flow_id"], - { - CONF_URL: TEST_URL, - CONF_USERNAME: TEST_USERNAME, - CONF_PASSWORD: TEST_PASSWORD, - }, - ) - await hass.async_block_till_done() + result2 = await hass.config_entries.flow.async_configure( + result["flow_id"], + { + CONF_URL: TEST_URL, + CONF_USERNAME: TEST_USERNAME, + CONF_PASSWORD: TEST_PASSWORD, + }, + ) + await hass.async_block_till_done() assert result2["type"] == "create_entry" assert result2["title"] == TEST_URL assert result2["data"] == { + CONF_CLIENT_DEVICE_ID: "TEST-UUID", CONF_URL: TEST_URL, CONF_USERNAME: TEST_USERNAME, CONF_PASSWORD: TEST_PASSWORD, } - assert len(mock_connect.mock_calls) == 1 - assert len(mock_login.mock_calls) == 1 + assert len(mock_client.auth.connect_to_address.mock_calls) == 1 + assert len(mock_client.auth.login.mock_calls) == 1 assert len(mock_setup_entry.mock_calls) == 1 - assert len(mock_set_id.mock_calls) == 1 + assert len(mock_client.jellyfin.get_user_settings.mock_calls) == 1 -async def test_form_cannot_connect(hass: HomeAssistant): +async def test_form_cannot_connect( + hass: HomeAssistant, + mock_jellyfin: MagicMock, + mock_client: MagicMock, + mock_client_device_id: MagicMock, +): """Test we handle an unreachable server.""" result = await hass.config_entries.flow.async_init( DOMAIN, context={"source": config_entries.SOURCE_USER} @@ -84,27 +82,30 @@ async def test_form_cannot_connect(hass: HomeAssistant): assert result["type"] == "form" assert result["errors"] == {} - with patch( - "homeassistant.components.jellyfin.client_wrapper.ConnectionManager.connect_to_address", - return_value=MOCK_UNSUCCESFUL_CONNECTION_STATE, - ) as mock_connect: - result2 = await hass.config_entries.flow.async_configure( - result["flow_id"], - { - CONF_URL: TEST_URL, - CONF_USERNAME: TEST_USERNAME, - CONF_PASSWORD: TEST_PASSWORD, - }, - ) + mock_client.auth.connect_to_address.return_value = MOCK_UNSUCCESFUL_CONNECTION_STATE + + result2 = await hass.config_entries.flow.async_configure( + result["flow_id"], + { + CONF_URL: TEST_URL, + CONF_USERNAME: TEST_USERNAME, + CONF_PASSWORD: TEST_PASSWORD, + }, + ) await hass.async_block_till_done() assert result2["type"] == "form" assert result2["errors"] == {"base": "cannot_connect"} - assert len(mock_connect.mock_calls) == 1 + assert len(mock_client.auth.connect_to_address.mock_calls) == 1 -async def test_form_invalid_auth(hass: HomeAssistant): +async def test_form_invalid_auth( + hass: HomeAssistant, + mock_jellyfin: MagicMock, + mock_client: MagicMock, + mock_client_device_id: MagicMock, +): """Test that we can handle invalid credentials.""" result = await hass.config_entries.flow.async_init( DOMAIN, context={"source": config_entries.SOURCE_USER} @@ -112,31 +113,28 @@ async def test_form_invalid_auth(hass: HomeAssistant): assert result["type"] == "form" assert result["errors"] == {} - with patch( - "homeassistant.components.jellyfin.client_wrapper.ConnectionManager.connect_to_address", - return_value=MOCK_SUCCESFUL_CONNECTION_STATE, - ) as mock_connect, patch( - "homeassistant.components.jellyfin.client_wrapper.ConnectionManager.login", - return_value=MOCK_UNSUCCESFUL_LOGIN_RESPONSE, - ) as mock_login: - result2 = await hass.config_entries.flow.async_configure( - result["flow_id"], - { - CONF_URL: TEST_URL, - CONF_USERNAME: TEST_USERNAME, - CONF_PASSWORD: TEST_PASSWORD, - }, - ) - await hass.async_block_till_done() + mock_client.auth.login.return_value = MOCK_UNSUCCESFUL_LOGIN_RESPONSE + + result2 = await hass.config_entries.flow.async_configure( + result["flow_id"], + { + CONF_URL: TEST_URL, + CONF_USERNAME: TEST_USERNAME, + CONF_PASSWORD: TEST_PASSWORD, + }, + ) + await hass.async_block_till_done() assert result2["type"] == "form" assert result2["errors"] == {"base": "invalid_auth"} - assert len(mock_connect.mock_calls) == 1 - assert len(mock_login.mock_calls) == 1 + assert len(mock_client.auth.connect_to_address.mock_calls) == 1 + assert len(mock_client.auth.login.mock_calls) == 1 -async def test_form_exception(hass: HomeAssistant): +async def test_form_exception( + hass: HomeAssistant, mock_jellyfin: MagicMock, mock_client: MagicMock +): """Test we handle an unexpected exception during server setup.""" result = await hass.config_entries.flow.async_init( DOMAIN, context={"source": config_entries.SOURCE_USER} @@ -144,21 +142,71 @@ async def test_form_exception(hass: HomeAssistant): assert result["type"] == "form" assert result["errors"] == {} - with patch( - "homeassistant.components.jellyfin.client_wrapper.ConnectionManager.connect_to_address", - side_effect=Exception("UnknownException"), - ) as mock_connect: - result2 = await hass.config_entries.flow.async_configure( - result["flow_id"], - { - CONF_URL: TEST_URL, - CONF_USERNAME: TEST_USERNAME, - CONF_PASSWORD: TEST_PASSWORD, - }, - ) - await hass.async_block_till_done() + mock_client.auth.connect_to_address.side_effect = Exception("UnknownException") + + result2 = await hass.config_entries.flow.async_configure( + result["flow_id"], + { + CONF_URL: TEST_URL, + CONF_USERNAME: TEST_USERNAME, + CONF_PASSWORD: TEST_PASSWORD, + }, + ) + await hass.async_block_till_done() assert result2["type"] == "form" assert result2["errors"] == {"base": "unknown"} - assert len(mock_connect.mock_calls) == 1 + assert len(mock_client.auth.connect_to_address.mock_calls) == 1 + + +async def test_form_persists_device_id_on_error( + hass: HomeAssistant, + mock_jellyfin: MagicMock, + mock_client: MagicMock, + mock_client_device_id: MagicMock, +): + """Test that we can handle invalid credentials.""" + result = await hass.config_entries.flow.async_init( + DOMAIN, context={"source": config_entries.SOURCE_USER} + ) + assert result["type"] == "form" + assert result["errors"] == {} + + mock_client_device_id.return_value = "TEST-UUID-1" + mock_client.auth.login.return_value = MOCK_UNSUCCESFUL_LOGIN_RESPONSE + + result2 = await hass.config_entries.flow.async_configure( + result["flow_id"], + { + CONF_URL: TEST_URL, + CONF_USERNAME: TEST_USERNAME, + CONF_PASSWORD: TEST_PASSWORD, + }, + ) + await hass.async_block_till_done() + + assert result2["type"] == "form" + assert result2["errors"] == {"base": "invalid_auth"} + + mock_client_device_id.return_value = "TEST-UUID-2" + mock_client.auth.login.return_value = MOCK_SUCCESFUL_LOGIN_RESPONSE + + result3 = await hass.config_entries.flow.async_configure( + result2["flow_id"], + { + CONF_URL: TEST_URL, + CONF_USERNAME: TEST_USERNAME, + CONF_PASSWORD: TEST_PASSWORD, + }, + ) + await hass.async_block_till_done() + + assert result3 + assert result3["type"] == "create_entry" + assert result3["data"] == { + CONF_CLIENT_DEVICE_ID: "TEST-UUID-1", + CONF_URL: TEST_URL, + CONF_USERNAME: TEST_USERNAME, + CONF_PASSWORD: TEST_PASSWORD, + }