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

Re-Auth Config Flow for vesync #137398

Draft
wants to merge 3 commits into
base: dev
Choose a base branch
from
Draft
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: 2 additions & 2 deletions homeassistant/components/vesync/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
from homeassistant.config_entries import ConfigEntry
from homeassistant.const import CONF_PASSWORD, CONF_USERNAME, Platform
from homeassistant.core import HomeAssistant, ServiceCall
from homeassistant.exceptions import ConfigEntryAuthFailed
from homeassistant.helpers import entity_registry as er
from homeassistant.helpers.dispatcher import async_dispatcher_send

Expand Down Expand Up @@ -46,8 +47,7 @@ async def async_setup_entry(hass: HomeAssistant, config_entry: ConfigEntry) -> b
login = await hass.async_add_executor_job(manager.login)

if not login:
_LOGGER.error("Unable to login to the VeSync server")
return False
raise ConfigEntryAuthFailed

hass.data[DOMAIN] = {}
hass.data[DOMAIN][VS_MANAGER] = manager
Expand Down
35 changes: 34 additions & 1 deletion homeassistant/components/vesync/config_flow.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
"""Config flow utilities."""

from collections.abc import Mapping
from typing import Any

from pyvesync import VeSync
Expand Down Expand Up @@ -36,7 +37,7 @@ def _show_form(self, errors: dict[str, str] | None = None) -> ConfigFlowResult:
)

async def async_step_user(
self, user_input: dict[str, Any] | None = None
self, user_input: Mapping[str, Any] | None = None
cdnninja marked this conversation as resolved.
Show resolved Hide resolved
) -> ConfigFlowResult:
"""Handle a flow start."""
if self._async_current_entries():
Expand All @@ -57,3 +58,35 @@ async def async_step_user(
title=username,
data={CONF_USERNAME: username, CONF_PASSWORD: password},
)

async def async_step_reauth(
self, entry_data: Mapping[str, Any]
) -> ConfigFlowResult:
"""Handle re-authentication with vesync."""
return await self.async_step_reauth_confirm()

async def async_step_reauth_confirm(
self, user_input: dict[str, Any] | None = None
) -> ConfigFlowResult:
"""Confirm re-authentication with vesync."""

if user_input:
reauth_entry = self._get_reauth_entry()
username = user_input[CONF_USERNAME]
password = user_input[CONF_PASSWORD]
data = {
**reauth_entry.data,
CONF_USERNAME: username,
CONF_PASSWORD: password,
}

manager = VeSync(username, password)
login = await self.hass.async_add_executor_job(manager.login)
if login:
return self.async_update_reload_and_abort(reauth_entry, data=data)
Comment on lines +85 to +86
Copy link
Member

Choose a reason for hiding this comment

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

If we can't login, I think we should show an error. We should also take in account any exceptions raised because we could not connect to VeSync (for whatever reason)

Copy link
Contributor

Choose a reason for hiding this comment

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

I think the usual pattern to raise ConfigEntryAuthFailed which causes HomeAssistant to present authentication flow.


return self.async_show_form(
step_id="reauth_confirm",
data_schema=DATA_SCHEMA,
description_placeholders={"name": "VeSync"},
)
11 changes: 10 additions & 1 deletion homeassistant/components/vesync/strings.json
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,22 @@
"username": "[%key:common::config_flow::data::email%]",
"password": "[%key:common::config_flow::data::password%]"
}
},
"reauth_confirm": {
"title": "[%key:common::config_flow::title::reauth%]",
"description": "The vesync integration needs to re-authenticate your account",
"data": {
"username": "[%key:common::config_flow::data::email%]",
"password": "[%key:common::config_flow::data::password%]"
}
}
},
"error": {
"invalid_auth": "[%key:common::config_flow::error::invalid_auth%]"
},
"abort": {
"single_instance_allowed": "[%key:common::config_flow::abort::single_instance_allowed%]"
"single_instance_allowed": "[%key:common::config_flow::abort::single_instance_allowed%]",
"reauth_successful": "[%key:common::config_flow::abort::reauth_successful%]"
}
},
"entity": {
Expand Down
48 changes: 48 additions & 0 deletions tests/components/vesync/test_config_flow.py
Original file line number Diff line number Diff line change
Expand Up @@ -48,3 +48,51 @@ async def test_config_flow_user_input(hass: HomeAssistant) -> None:
assert result["type"] is FlowResultType.CREATE_ENTRY
assert result["data"][CONF_USERNAME] == "user"
assert result["data"][CONF_PASSWORD] == "pass"


async def test_reauth_flow(hass: HomeAssistant) -> None:
"""Test a successful reauth flow."""
mock_entry = MockConfigEntry(
domain=DOMAIN,
unique_id="test-username",
)
mock_entry.add_to_hass(hass)

result = await mock_entry.start_reauth_flow(hass)

assert result["step_id"] == "reauth_confirm"
assert result["type"] is FlowResultType.FORM
with patch("pyvesync.vesync.VeSync.login", return_value=True):
result2 = await hass.config_entries.flow.async_configure(
result["flow_id"],
{CONF_USERNAME: "new-username", CONF_PASSWORD: "new-password"},
)

assert result2["type"] is FlowResultType.ABORT
assert result2["reason"] == "reauth_successful"
assert mock_entry.data == {
CONF_USERNAME: "new-username",
CONF_PASSWORD: "new-password",
}


async def test_reauth_flow_invalid_auth(hass: HomeAssistant) -> None:
"""Test an authorization error reauth flow."""

mock_entry = MockConfigEntry(
domain=DOMAIN,
unique_id="test-username",
)
mock_entry.add_to_hass(hass)

result = await mock_entry.start_reauth_flow(hass)
assert result["step_id"] == "reauth_confirm"
assert result["type"] is FlowResultType.FORM

with patch("pyvesync.vesync.VeSync.login", return_value=False):
result2 = await hass.config_entries.flow.async_configure(
result["flow_id"],
{CONF_USERNAME: "new-username", CONF_PASSWORD: "new-password"},
)

assert result2["type"] is FlowResultType.FORM
Comment on lines +79 to +98
Copy link
Member

Choose a reason for hiding this comment

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

Let's have this test repatched so it will succeed (and thus end in an ABORT), this way we test that the flow is able to finish

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Wouldn't that be the test_reauth_flow test further up? It completes the reconfig, where as this one is a reauth with invalid details.

Loading