Add Picnic re-auth flow (#62938)

* Add re-auth handler for Picnic

* Extracted authentication part so right form/errors can be shown during re-auth flow

* Add tests for Picnic's re-authentication flow

* Simplify re-auth flow by using the same step as step_user

* Use user step also for re-auth flow instead of having an authenticate step

* Add check for when re-auth is done with different account

* Remove unnecessary else in Picnic config flow

* Fix the step id in the translation strings file

homeassistant/components/picnic/config_flow.py

@@ -9,6 +9,7 @@ import requests
 import voluptuous as vol
 
 from homeassistant import config_entries, core, exceptions
+from homeassistant.config_entries import SOURCE_REAUTH
 from homeassistant.const import CONF_ACCESS_TOKEN, CONF_PASSWORD, CONF_USERNAME
 
 from .const import CONF_COUNTRY_CODE, COUNTRY_CODES, DOMAIN
@@ -71,8 +72,12 @@ class ConfigFlow(config_entries.ConfigFlow, domain=DOMAIN):
 
     VERSION = 1
 
+    async def async_step_reauth(self, _):
+        """Perform the re-auth step upon an API authentication error."""
+        return await self.async_step_user()
+
     async def async_step_user(self, user_input=None):
-        """Handle the initial step."""
+        """Handle the authentication step, this is the generic step for both `step_user` and `step_reauth`."""
         if user_input is None:
             return self.async_show_form(
                 step_id="user", data_schema=STEP_USER_DATA_SCHEMA
@@ -90,17 +95,25 @@ class ConfigFlow(config_entries.ConfigFlow, domain=DOMAIN):
             _LOGGER.exception("Unexpected exception")
             errors["base"] = "unknown"
         else:
-            # Set the unique id and abort if it already exists
-            await self.async_set_unique_id(info["unique_id"])
-            self._abort_if_unique_id_configured()
-
-            return self.async_create_entry(
-                title=info["title"],
             data = {
                 CONF_ACCESS_TOKEN: auth_token,
                 CONF_COUNTRY_CODE: user_input[CONF_COUNTRY_CODE],
-                },
+            }
-            )
+            existing_entry = await self.async_set_unique_id(info["unique_id"])
+
+            # Abort if we're adding a new config and the unique id is already in use, else create the entry
+            if self.source != SOURCE_REAUTH:
+                self._abort_if_unique_id_configured()
+                return self.async_create_entry(title=info["title"], data=data)
+
+            # In case of re-auth, only continue if an exiting account exists with the same unique id
+            if existing_entry:
+                self.hass.config_entries.async_update_entry(existing_entry, data=data)
+                await self.hass.config_entries.async_reload(existing_entry.entry_id)
+                return self.async_abort(reason="reauth_successful")
+
+            # Set the error because the account is different
+            errors["base"] = "different_account"
 
         return self.async_show_form(
             step_id="user", data_schema=STEP_USER_DATA_SCHEMA, errors=errors

homeassistant/components/picnic/strings.json

@@ -12,10 +12,12 @@
     "error": {
       "cannot_connect": "[%key:common::config_flow::error::cannot_connect%]",
       "invalid_auth": "[%key:common::config_flow::error::invalid_auth%]",
-      "unknown": "[%key:common::config_flow::error::unknown%]"
+      "unknown": "[%key:common::config_flow::error::unknown%]",
+      "different_account": "Account should be the same as used for setting up the integration"
     },
     "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%]"
     }
   }
 }

homeassistant/components/picnic/translations/en.json

@@ -1,10 +1,12 @@
 {
     "config": {
         "abort": {
-            "already_configured": "Device is already configured"
+            "already_configured": "Device is already configured",
+            "reauth_successful": "Re-authentication was successful"
         },
         "error": {
             "cannot_connect": "Failed to connect",
+            "different_account": "Account should be the same as used for setting up the integration",
             "invalid_auth": "Invalid authentication",
             "unknown": "Unexpected error"
         },
@@ -17,6 +19,5 @@
                 }
             }
         }
-    },
+    }
-    "title": "Picnic"
 }

tests/components/picnic/test_config_flow.py

@@ -1,23 +1,20 @@
 """Test the Picnic config flow."""
 from unittest.mock import patch
 
+import pytest
 from python_picnic_api.session import PicnicAuthError
 import requests
 
-from homeassistant import config_entries
+from homeassistant import config_entries, data_entry_flow
 from homeassistant.components.picnic.const import CONF_COUNTRY_CODE, DOMAIN
 from homeassistant.const import CONF_ACCESS_TOKEN
 
+from tests.common import MockConfigEntry
 
-async def test_form(hass):
-    """Test we get the form."""
-
-    result = await hass.config_entries.flow.async_init(
-        DOMAIN, context={"source": config_entries.SOURCE_USER}
-    )
-    assert result["type"] == "form"
-    assert result["errors"] is None
 
+@pytest.fixture
+def picnic_api():
+    """Create PicnicAPI mock with set response data."""
     auth_token = "af3wh738j3fa28l9fa23lhiufahu7l"
     auth_data = {
         "user_id": "f29-2a6-o32n",
@@ -29,13 +26,27 @@ async def test_form(hass):
     }
     with patch(
         "homeassistant.components.picnic.config_flow.PicnicAPI",
-    ) as mock_picnic, patch(
+    ) as picnic_mock:
+        picnic_mock().session.auth_token = auth_token
+        picnic_mock().get_user.return_value = auth_data
+
+        yield picnic_mock
+
+
+async def test_form(hass, picnic_api):
+    """Test we get the form and a config entry is created."""
+
+    result = await hass.config_entries.flow.async_init(
+        DOMAIN, context={"source": config_entries.SOURCE_USER}
+    )
+    assert result["type"] == data_entry_flow.RESULT_TYPE_FORM
+    assert result["step_id"] == "user"
+    assert result["errors"] is None
+
+    with patch(
         "homeassistant.components.picnic.async_setup_entry",
         return_value=True,
     ) as mock_setup_entry:
-        mock_picnic().session.auth_token = auth_token
-        mock_picnic().get_user.return_value = auth_data
-
         result2 = await hass.config_entries.flow.async_configure(
             result["flow_id"],
             {
@@ -49,14 +60,14 @@ async def test_form(hass):
     assert result2["type"] == "create_entry"
     assert result2["title"] == "Teststreet 123b"
     assert result2["data"] == {
-        CONF_ACCESS_TOKEN: auth_token,
+        CONF_ACCESS_TOKEN: picnic_api().session.auth_token,
         CONF_COUNTRY_CODE: "NL",
     }
     assert len(mock_setup_entry.mock_calls) == 1
 
 
 async def test_form_invalid_auth(hass):
-    """Test we handle invalid auth."""
+    """Test we handle invalid authentication."""
     result = await hass.config_entries.flow.async_init(
         DOMAIN, context={"source": config_entries.SOURCE_USER}
     )
@@ -74,12 +85,12 @@ async def test_form_invalid_auth(hass):
             },
         )
 
-    assert result2["type"] == "form"
+    assert result2["type"] == data_entry_flow.RESULT_TYPE_FORM
     assert result2["errors"] == {"base": "invalid_auth"}
 
 
 async def test_form_cannot_connect(hass):
-    """Test we handle cannot connect error."""
+    """Test we handle connection errors."""
     result = await hass.config_entries.flow.async_init(
         DOMAIN, context={"source": config_entries.SOURCE_USER}
     )
@@ -97,7 +108,7 @@ async def test_form_cannot_connect(hass):
             },
         )
 
-    assert result2["type"] == "form"
+    assert result2["type"] == data_entry_flow.RESULT_TYPE_FORM
     assert result2["errors"] == {"base": "cannot_connect"}
 
 
@@ -120,5 +131,150 @@ async def test_form_exception(hass):
             },
         )
 
-    assert result2["type"] == "form"
+    assert result2["type"] == data_entry_flow.RESULT_TYPE_FORM
     assert result2["errors"] == {"base": "unknown"}
+
+
+async def test_form_already_configured(hass, picnic_api):
+    """Test that an entry with unique id can only be added once."""
+    # Create a mocked config entry and make sure to use the same user_id as set for the picnic_api mock response.
+    MockConfigEntry(
+        domain=DOMAIN,
+        unique_id=picnic_api().get_user()["user_id"],
+        data={CONF_ACCESS_TOKEN: "a3p98fsen.a39p3fap", CONF_COUNTRY_CODE: "NL"},
+    ).add_to_hass(hass)
+
+    result_init = await hass.config_entries.flow.async_init(
+        DOMAIN, context={"source": config_entries.SOURCE_USER}
+    )
+
+    result_configure = await hass.config_entries.flow.async_configure(
+        result_init["flow_id"],
+        {
+            "username": "test-username",
+            "password": "test-password",
+            "country_code": "NL",
+        },
+    )
+    await hass.async_block_till_done()
+
+    assert result_configure["type"] == data_entry_flow.RESULT_TYPE_ABORT
+    assert result_configure["reason"] == "already_configured"
+
+
+async def test_step_reauth(hass, picnic_api):
+    """Test the re-auth flow."""
+    # Create a mocked config entry
+    conf = {CONF_ACCESS_TOKEN: "a3p98fsen.a39p3fap", CONF_COUNTRY_CODE: "NL"}
+
+    MockConfigEntry(
+        domain=DOMAIN,
+        unique_id=picnic_api().get_user()["user_id"],
+        data=conf,
+    ).add_to_hass(hass)
+
+    # Init a re-auth flow
+    result_init = await hass.config_entries.flow.async_init(
+        DOMAIN, context={"source": config_entries.SOURCE_REAUTH}, data=conf
+    )
+    assert result_init["type"] == data_entry_flow.RESULT_TYPE_FORM
+    assert result_init["step_id"] == "user"
+
+    with patch(
+        "homeassistant.components.picnic.async_setup_entry",
+        return_value=True,
+    ):
+        result_configure = await hass.config_entries.flow.async_configure(
+            result_init["flow_id"],
+            {
+                "username": "test-username",
+                "password": "test-password",
+                "country_code": "NL",
+            },
+        )
+        await hass.async_block_till_done()
+
+    # Check that the returned flow has type abort because of successful re-authentication
+    assert result_configure["type"] == data_entry_flow.RESULT_TYPE_ABORT
+    assert result_configure["reason"] == "reauth_successful"
+
+    assert len(hass.config_entries.async_entries()) == 1
+
+
+async def test_step_reauth_failed(hass):
+    """Test the re-auth flow when authentication fails."""
+    # Create a mocked config entry
+    user_id = "f29-2a6-o32n"
+    conf = {CONF_ACCESS_TOKEN: "a3p98fsen.a39p3fap", CONF_COUNTRY_CODE: "NL"}
+
+    MockConfigEntry(
+        domain=DOMAIN,
+        unique_id=user_id,
+        data=conf,
+    ).add_to_hass(hass)
+
+    # Init a re-auth flow
+    result_init = await hass.config_entries.flow.async_init(
+        DOMAIN, context={"source": config_entries.SOURCE_REAUTH}, data=conf
+    )
+    assert result_init["type"] == data_entry_flow.RESULT_TYPE_FORM
+    assert result_init["step_id"] == "user"
+
+    with patch(
+        "homeassistant.components.picnic.config_flow.PicnicHub.authenticate",
+        side_effect=PicnicAuthError,
+    ):
+        result_configure = await hass.config_entries.flow.async_configure(
+            result_init["flow_id"],
+            {
+                "username": "test-username",
+                "password": "test-password",
+                "country_code": "NL",
+            },
+        )
+        await hass.async_block_till_done()
+
+    # Check that the returned flow has type form with error set
+    assert result_configure["type"] == "form"
+    assert result_configure["errors"] == {"base": "invalid_auth"}
+
+    assert len(hass.config_entries.async_entries()) == 1
+
+
+async def test_step_reauth_different_account(hass, picnic_api):
+    """Test the re-auth flow when authentication is done with a different account."""
+    # Create a mocked config entry, unique_id should be different that the user id in the api response
+    conf = {CONF_ACCESS_TOKEN: "a3p98fsen.a39p3fap", CONF_COUNTRY_CODE: "NL"}
+
+    MockConfigEntry(
+        domain=DOMAIN,
+        unique_id="3fpawh-ues-af3ho",
+        data=conf,
+    ).add_to_hass(hass)
+
+    # Init a re-auth flow
+    result_init = await hass.config_entries.flow.async_init(
+        DOMAIN, context={"source": config_entries.SOURCE_REAUTH}, data=conf
+    )
+    assert result_init["type"] == data_entry_flow.RESULT_TYPE_FORM
+    assert result_init["step_id"] == "user"
+
+    with patch(
+        "homeassistant.components.picnic.async_setup_entry",
+        return_value=True,
+    ):
+        result_configure = await hass.config_entries.flow.async_configure(
+            result_init["flow_id"],
+            {
+                "username": "test-username",
+                "password": "test-password",
+                "country_code": "NL",
+            },
+        )
+        await hass.async_block_till_done()
+
+    # Check that the returned flow has type form with error set
+    assert result_configure["type"] == "form"
+    assert result_configure["errors"] == {"base": "different_account"}
+
+    assert len(hass.config_entries.async_entries()) == 1