From 182c40f16ecf10be92fe7d3677627889c6aeb515 Mon Sep 17 00:00:00 2001 From: sdb9696 <51370195+sdb9696@users.noreply.github.com> Date: Wed, 15 Nov 2023 03:49:27 +0000 Subject: [PATCH] Add reauth flow to ring integration (#103758) * Add reauth flow to ring integration * Refactor re-auth flow post review * Fix threading issue on device update --- homeassistant/components/ring/__init__.py | 37 ++++--- homeassistant/components/ring/config_flow.py | 81 +++++++++++--- homeassistant/components/ring/strings.json | 10 +- tests/components/ring/test_config_flow.py | 111 +++++++++++++++++++ tests/components/ring/test_init.py | 29 ++--- 5 files changed, 221 insertions(+), 47 deletions(-) diff --git a/homeassistant/components/ring/__init__.py b/homeassistant/components/ring/__init__.py index a0863836a6c..7e7bff1fa53 100644 --- a/homeassistant/components/ring/__init__.py +++ b/homeassistant/components/ring/__init__.py @@ -13,6 +13,7 @@ import ring_doorbell from homeassistant.config_entries import ConfigEntry from homeassistant.const import Platform, __version__ from homeassistant.core import HomeAssistant, ServiceCall, callback +from homeassistant.exceptions import ConfigEntryAuthFailed from homeassistant.helpers import device_registry as dr from homeassistant.helpers.event import async_track_time_interval from homeassistant.util.async_ import run_callback_threadsafe @@ -58,20 +59,20 @@ async def async_setup_entry(hass: HomeAssistant, entry: ConfigEntry) -> bool: try: await hass.async_add_executor_job(ring.update_data) - except ring_doorbell.AuthenticationError: - _LOGGER.error("Access token is no longer valid. Please set up Ring again") - return False + except ring_doorbell.AuthenticationError as err: + _LOGGER.warning("Ring access token is no longer valid, need to re-authenticate") + raise ConfigEntryAuthFailed(err) from err hass.data.setdefault(DOMAIN, {})[entry.entry_id] = { "api": ring, "devices": ring.devices(), "device_data": GlobalDataUpdater( - hass, "device", entry.entry_id, ring, "update_devices", timedelta(minutes=1) + hass, "device", entry, ring, "update_devices", timedelta(minutes=1) ), "dings_data": GlobalDataUpdater( hass, "active dings", - entry.entry_id, + entry, ring, "update_dings", timedelta(seconds=5), @@ -79,7 +80,7 @@ async def async_setup_entry(hass: HomeAssistant, entry: ConfigEntry) -> bool: "history_data": DeviceDataUpdater( hass, "history", - entry.entry_id, + entry, ring, lambda device: device.history(limit=10), timedelta(minutes=1), @@ -87,7 +88,7 @@ async def async_setup_entry(hass: HomeAssistant, entry: ConfigEntry) -> bool: "health_data": DeviceDataUpdater( hass, "health", - entry.entry_id, + entry, ring, lambda device: device.update_health_data(), timedelta(minutes=1), @@ -143,7 +144,7 @@ class GlobalDataUpdater: self, hass: HomeAssistant, data_type: str, - config_entry_id: str, + config_entry: ConfigEntry, ring: ring_doorbell.Ring, update_method: str, update_interval: timedelta, @@ -151,7 +152,7 @@ class GlobalDataUpdater: """Initialize global data updater.""" self.hass = hass self.data_type = data_type - self.config_entry_id = config_entry_id + self.config_entry = config_entry self.ring = ring self.update_method = update_method self.update_interval = update_interval @@ -188,8 +189,10 @@ class GlobalDataUpdater: getattr(self.ring, self.update_method) ) except ring_doorbell.AuthenticationError: - _LOGGER.error("Ring access token is no longer valid. Set up Ring again") - await self.hass.config_entries.async_unload(self.config_entry_id) + _LOGGER.warning( + "Ring access token is no longer valid, need to re-authenticate" + ) + self.config_entry.async_start_reauth(self.hass) return except ring_doorbell.RingTimeout: _LOGGER.warning( @@ -216,7 +219,7 @@ class DeviceDataUpdater: self, hass: HomeAssistant, data_type: str, - config_entry_id: str, + config_entry: ConfigEntry, ring: ring_doorbell.Ring, update_method: Callable[[ring_doorbell.Ring], Any], update_interval: timedelta, @@ -224,7 +227,7 @@ class DeviceDataUpdater: """Initialize device data updater.""" self.data_type = data_type self.hass = hass - self.config_entry_id = config_entry_id + self.config_entry = config_entry self.ring = ring self.update_method = update_method self.update_interval = update_interval @@ -277,9 +280,11 @@ class DeviceDataUpdater: try: data = info["data"] = self.update_method(info["device"]) except ring_doorbell.AuthenticationError: - _LOGGER.error("Ring access token is no longer valid. Set up Ring again") - self.hass.add_job( - self.hass.config_entries.async_unload(self.config_entry_id) + _LOGGER.warning( + "Ring access token is no longer valid, need to re-authenticate" + ) + self.hass.loop.call_soon_threadsafe( + self.config_entry.async_start_reauth, self.hass ) return except ring_doorbell.RingTimeout: diff --git a/homeassistant/components/ring/config_flow.py b/homeassistant/components/ring/config_flow.py index b22d59a78f5..222a18fa24f 100644 --- a/homeassistant/components/ring/config_flow.py +++ b/homeassistant/components/ring/config_flow.py @@ -1,4 +1,5 @@ """Config flow for Ring integration.""" +from collections.abc import Mapping import logging from typing import Any @@ -6,12 +7,19 @@ import ring_doorbell import voluptuous as vol from homeassistant import config_entries, core, exceptions -from homeassistant.const import __version__ as ha_version +from homeassistant.config_entries import ConfigEntry +from homeassistant.const import CONF_PASSWORD, CONF_USERNAME, __version__ as ha_version +from homeassistant.data_entry_flow import FlowResult from . import DOMAIN _LOGGER = logging.getLogger(__name__) +STEP_USER_DATA_SCHEMA = vol.Schema( + {vol.Required(CONF_USERNAME): str, vol.Required(CONF_PASSWORD): str} +) +STEP_REAUTH_DATA_SCHEMA = vol.Schema({vol.Required(CONF_PASSWORD): str}) + async def validate_input(hass: core.HomeAssistant, data): """Validate the user input allows us to connect.""" @@ -39,6 +47,7 @@ class RingConfigFlow(config_entries.ConfigFlow, domain=DOMAIN): VERSION = 1 user_pass: dict[str, Any] = {} + reauth_entry: ConfigEntry | None = None async def async_step_user(self, user_input=None): """Handle the initial step.""" @@ -46,34 +55,34 @@ class RingConfigFlow(config_entries.ConfigFlow, domain=DOMAIN): if user_input is not None: try: token = await validate_input(self.hass, user_input) - await self.async_set_unique_id(user_input["username"]) - - return self.async_create_entry( - title=user_input["username"], - data={"username": user_input["username"], "token": token}, - ) except Require2FA: self.user_pass = user_input return await self.async_step_2fa() - except InvalidAuth: errors["base"] = "invalid_auth" except Exception: # pylint: disable=broad-except _LOGGER.exception("Unexpected exception") errors["base"] = "unknown" + else: + await self.async_set_unique_id(user_input["username"]) + return self.async_create_entry( + title=user_input["username"], + data={"username": user_input["username"], "token": token}, + ) return self.async_show_form( - step_id="user", - data_schema=vol.Schema( - {vol.Required("username"): str, vol.Required("password"): str} - ), - errors=errors, + step_id="user", data_schema=STEP_USER_DATA_SCHEMA, errors=errors ) async def async_step_2fa(self, user_input=None): """Handle 2fa step.""" if user_input: + if self.reauth_entry: + return await self.async_step_reauth_confirm( + {**self.user_pass, **user_input} + ) + return await self.async_step_user({**self.user_pass, **user_input}) return self.async_show_form( @@ -81,6 +90,52 @@ class RingConfigFlow(config_entries.ConfigFlow, domain=DOMAIN): data_schema=vol.Schema({vol.Required("2fa"): str}), ) + async def async_step_reauth(self, entry_data: Mapping[str, Any]) -> FlowResult: + """Handle reauth upon an API authentication error.""" + self.reauth_entry = self.hass.config_entries.async_get_entry( + self.context["entry_id"] + ) + return await self.async_step_reauth_confirm() + + async def async_step_reauth_confirm( + self, user_input: dict[str, Any] | None = None + ) -> FlowResult: + """Dialog that informs the user that reauth is required.""" + errors = {} + assert self.reauth_entry is not None + + if user_input: + user_input[CONF_USERNAME] = self.reauth_entry.data[CONF_USERNAME] + try: + token = await validate_input(self.hass, user_input) + except Require2FA: + self.user_pass = user_input + return await self.async_step_2fa() + except InvalidAuth: + errors["base"] = "invalid_auth" + except Exception: # pylint: disable=broad-except + _LOGGER.exception("Unexpected exception") + errors["base"] = "unknown" + else: + data = { + CONF_USERNAME: user_input[CONF_USERNAME], + "token": token, + } + self.hass.config_entries.async_update_entry( + self.reauth_entry, data=data + ) + await self.hass.config_entries.async_reload(self.reauth_entry.entry_id) + return self.async_abort(reason="reauth_successful") + + return self.async_show_form( + step_id="reauth_confirm", + data_schema=STEP_REAUTH_DATA_SCHEMA, + errors=errors, + description_placeholders={ + CONF_USERNAME: self.reauth_entry.data[CONF_USERNAME] + }, + ) + class Require2FA(exceptions.HomeAssistantError): """Error to indicate we require 2FA.""" diff --git a/homeassistant/components/ring/strings.json b/homeassistant/components/ring/strings.json index b300e335b19..688e3141beb 100644 --- a/homeassistant/components/ring/strings.json +++ b/homeassistant/components/ring/strings.json @@ -13,6 +13,13 @@ "data": { "2fa": "Two-factor code" } + }, + "reauth_confirm": { + "title": "[%key:common::config_flow::title::reauth%]", + "description": "The Ring integration needs to re-authenticate your account {username}", + "data": { + "password": "[%key:common::config_flow::data::password%]" + } } }, "error": { @@ -20,7 +27,8 @@ "unknown": "[%key:common::config_flow::error::unknown%]" }, "abort": { - "already_configured": "[%key:common::config_flow::abort::already_configured_device%]" + "already_configured": "[%key:common::config_flow::abort::already_configured_device%]", + "reauth_successful": "[%key:common::config_flow::abort::reauth_successful%]" } }, "entity": { diff --git a/tests/components/ring/test_config_flow.py b/tests/components/ring/test_config_flow.py index 0c1578e2c8d..53c7e139a51 100644 --- a/tests/components/ring/test_config_flow.py +++ b/tests/components/ring/test_config_flow.py @@ -10,6 +10,8 @@ from homeassistant.const import CONF_PASSWORD, CONF_USERNAME from homeassistant.core import HomeAssistant from homeassistant.data_entry_flow import FlowResultType +from tests.common import MockConfigEntry + async def test_form( hass: HomeAssistant, @@ -108,3 +110,112 @@ async def test_form_2fa( "token": "new-foobar", } assert len(mock_setup_entry.mock_calls) == 1 + + +async def test_reauth( + hass: HomeAssistant, + mock_added_config_entry: MockConfigEntry, + mock_setup_entry: AsyncMock, + mock_ring_auth: Mock, +) -> None: + """Test reauth flow.""" + mock_added_config_entry.async_start_reauth(hass) + await hass.async_block_till_done() + + flows = hass.config_entries.flow.async_progress() + assert len(flows) == 1 + [result] = flows + assert result["step_id"] == "reauth_confirm" + + mock_ring_auth.fetch_token.side_effect = ring_doorbell.Requires2FAError + result2 = await hass.config_entries.flow.async_configure( + result["flow_id"], + user_input={ + CONF_PASSWORD: "other_fake_password", + }, + ) + + mock_ring_auth.fetch_token.assert_called_once_with( + "foo@bar.com", "other_fake_password", None + ) + assert result2["type"] == FlowResultType.FORM + assert result2["step_id"] == "2fa" + mock_ring_auth.fetch_token.reset_mock(side_effect=True) + mock_ring_auth.fetch_token.return_value = "new-foobar" + result3 = await hass.config_entries.flow.async_configure( + result2["flow_id"], + user_input={"2fa": "123456"}, + ) + + mock_ring_auth.fetch_token.assert_called_once_with( + "foo@bar.com", "other_fake_password", "123456" + ) + assert result3["type"] == FlowResultType.ABORT + assert result3["reason"] == "reauth_successful" + assert mock_added_config_entry.data == { + "username": "foo@bar.com", + "token": "new-foobar", + } + assert len(mock_setup_entry.mock_calls) == 1 + + +@pytest.mark.parametrize( + ("error_type", "errors_msg"), + [ + (ring_doorbell.AuthenticationError, "invalid_auth"), + (Exception, "unknown"), + ], + ids=["invalid-auth", "unknown-error"], +) +async def test_reauth_error( + hass: HomeAssistant, + mock_added_config_entry: MockConfigEntry, + mock_setup_entry: AsyncMock, + mock_ring_auth: Mock, + error_type, + errors_msg, +) -> None: + """Test reauth flow.""" + mock_added_config_entry.async_start_reauth(hass) + await hass.async_block_till_done() + + flows = hass.config_entries.flow.async_progress() + assert len(flows) == 1 + [result] = flows + assert result["step_id"] == "reauth_confirm" + + mock_ring_auth.fetch_token.side_effect = error_type + result2 = await hass.config_entries.flow.async_configure( + result["flow_id"], + user_input={ + CONF_PASSWORD: "error_fake_password", + }, + ) + await hass.async_block_till_done() + + mock_ring_auth.fetch_token.assert_called_once_with( + "foo@bar.com", "error_fake_password", None + ) + assert result2["type"] == FlowResultType.FORM + assert result2["errors"] == {"base": errors_msg} + + # Now test reauth can go on to succeed + mock_ring_auth.fetch_token.reset_mock(side_effect=True) + mock_ring_auth.fetch_token.return_value = "new-foobar" + result3 = await hass.config_entries.flow.async_configure( + result2["flow_id"], + user_input={ + CONF_PASSWORD: "other_fake_password", + }, + ) + + mock_ring_auth.fetch_token.assert_called_once_with( + "foo@bar.com", "other_fake_password", None + ) + assert result3["type"] == FlowResultType.ABORT + assert result3["reason"] == "reauth_successful" + assert mock_added_config_entry.data == { + "username": "foo@bar.com", + "token": "new-foobar", + } + assert len(mock_setup_entry.mock_calls) == 1 diff --git a/tests/components/ring/test_init.py b/tests/components/ring/test_init.py index 9fa79b21fab..6ad79623a12 100644 --- a/tests/components/ring/test_init.py +++ b/tests/components/ring/test_init.py @@ -9,7 +9,7 @@ from ring_doorbell import AuthenticationError, RingError, RingTimeout import homeassistant.components.ring as ring from homeassistant.components.ring import DOMAIN -from homeassistant.config_entries import ConfigEntryState +from homeassistant.config_entries import SOURCE_REAUTH, ConfigEntryState from homeassistant.core import HomeAssistant from homeassistant.setup import async_setup_component from homeassistant.util import dt as dt_util @@ -46,7 +46,6 @@ async def test_auth_failed_on_setup( hass: HomeAssistant, requests_mock: requests_mock.Mocker, mock_config_entry: MockConfigEntry, - caplog, ) -> None: """Test auth failure on setup entry.""" mock_config_entry.add_to_hass(hass) @@ -54,14 +53,10 @@ async def test_auth_failed_on_setup( "ring_doorbell.Ring.update_data", side_effect=AuthenticationError, ): - result = await hass.config_entries.async_setup(mock_config_entry.entry_id) + assert not any(mock_config_entry.async_get_active_flows(hass, {SOURCE_REAUTH})) + await hass.config_entries.async_setup(mock_config_entry.entry_id) await hass.async_block_till_done() - assert result is False - assert "Access token is no longer valid. Please set up Ring again" in [ - record.message for record in caplog.records if record.levelname == "ERROR" - ] - - assert DOMAIN not in hass.data + assert any(mock_config_entry.async_get_active_flows(hass, {SOURCE_REAUTH})) assert mock_config_entry.state is ConfigEntryState.SETUP_ERROR @@ -75,7 +70,7 @@ async def test_auth_failure_on_global_update( mock_config_entry.add_to_hass(hass) await hass.config_entries.async_setup(mock_config_entry.entry_id) await hass.async_block_till_done() - + assert not any(mock_config_entry.async_get_active_flows(hass, {SOURCE_REAUTH})) with patch( "ring_doorbell.Ring.update_devices", side_effect=AuthenticationError, @@ -83,11 +78,11 @@ async def test_auth_failure_on_global_update( async_fire_time_changed(hass, dt_util.now() + timedelta(minutes=20)) await hass.async_block_till_done() - assert "Ring access token is no longer valid. Set up Ring again" in [ - record.message for record in caplog.records if record.levelname == "ERROR" + assert "Ring access token is no longer valid, need to re-authenticate" in [ + record.message for record in caplog.records if record.levelname == "WARNING" ] - assert mock_config_entry.entry_id not in hass.data[DOMAIN] + assert any(mock_config_entry.async_get_active_flows(hass, {SOURCE_REAUTH})) async def test_auth_failure_on_device_update( @@ -100,7 +95,7 @@ async def test_auth_failure_on_device_update( mock_config_entry.add_to_hass(hass) await hass.config_entries.async_setup(mock_config_entry.entry_id) await hass.async_block_till_done() - + assert not any(mock_config_entry.async_get_active_flows(hass, {SOURCE_REAUTH})) with patch( "ring_doorbell.RingDoorBell.history", side_effect=AuthenticationError, @@ -108,11 +103,11 @@ async def test_auth_failure_on_device_update( async_fire_time_changed(hass, dt_util.now() + timedelta(minutes=20)) await hass.async_block_till_done() - assert "Ring access token is no longer valid. Set up Ring again" in [ - record.message for record in caplog.records if record.levelname == "ERROR" + assert "Ring access token is no longer valid, need to re-authenticate" in [ + record.message for record in caplog.records if record.levelname == "WARNING" ] - assert mock_config_entry.entry_id not in hass.data[DOMAIN] + assert any(mock_config_entry.async_get_active_flows(hass, {SOURCE_REAUTH})) @pytest.mark.parametrize(