Improve fitbit oauth import robustness (#102833)

* Improve fitbit oauth import robustness

* Improve sensor tests and remove unnecessary client check

* Fix oauth client id/secret config key checks

* Add executor for sync call

homeassistant/components/fitbit/sensor.py

@@ -8,6 +8,8 @@ import logging
 import os
 from typing import Any, Final, cast
 
+from fitbit import Fitbit
+from oauthlib.oauth2.rfc6749.errors import OAuth2Error
 import voluptuous as vol
 
 from homeassistant.components.application_credentials import (
@@ -567,34 +569,51 @@ async def async_setup_platform(
 
     if config_file is not None:
         _LOGGER.debug("Importing existing fitbit.conf application credentials")
-        await async_import_client_credential(
-            hass,
-            DOMAIN,
-            ClientCredential(
-                config_file[CONF_CLIENT_ID], config_file[CONF_CLIENT_SECRET]
-            ),
+
+        # Refresh the token before importing to ensure it is working and not
+        # expired on first initialization.
+        authd_client = Fitbit(
+            config_file[CONF_CLIENT_ID],
+            config_file[CONF_CLIENT_SECRET],
+            access_token=config_file[ATTR_ACCESS_TOKEN],
+            refresh_token=config_file[ATTR_REFRESH_TOKEN],
+            expires_at=config_file[ATTR_LAST_SAVED_AT],
+            refresh_cb=lambda x: None,
         )
-        result = await hass.config_entries.flow.async_init(
-            DOMAIN,
-            context={"source": SOURCE_IMPORT},
-            data={
-                "auth_implementation": DOMAIN,
-                CONF_TOKEN: {
-                    ATTR_ACCESS_TOKEN: config_file[ATTR_ACCESS_TOKEN],
-                    ATTR_REFRESH_TOKEN: config_file[ATTR_REFRESH_TOKEN],
-                    "expires_at": config_file[ATTR_LAST_SAVED_AT],
-                },
-                CONF_CLOCK_FORMAT: config[CONF_CLOCK_FORMAT],
-                CONF_UNIT_SYSTEM: config[CONF_UNIT_SYSTEM],
-                CONF_MONITORED_RESOURCES: config[CONF_MONITORED_RESOURCES],
-            },
-        )
-        translation_key = "deprecated_yaml_import"
-        if (
-            result.get("type") == FlowResultType.ABORT
-            and result.get("reason") == "cannot_connect"
-        ):
+        try:
+            await hass.async_add_executor_job(authd_client.client.refresh_token)
+        except OAuth2Error as err:
+            _LOGGER.debug("Unable to import fitbit OAuth2 credentials: %s", err)
             translation_key = "deprecated_yaml_import_issue_cannot_connect"
+        else:
+            await async_import_client_credential(
+                hass,
+                DOMAIN,
+                ClientCredential(
+                    config_file[CONF_CLIENT_ID], config_file[CONF_CLIENT_SECRET]
+                ),
+            )
+            result = await hass.config_entries.flow.async_init(
+                DOMAIN,
+                context={"source": SOURCE_IMPORT},
+                data={
+                    "auth_implementation": DOMAIN,
+                    CONF_TOKEN: {
+                        ATTR_ACCESS_TOKEN: config_file[ATTR_ACCESS_TOKEN],
+                        ATTR_REFRESH_TOKEN: config_file[ATTR_REFRESH_TOKEN],
+                        "expires_at": config_file[ATTR_LAST_SAVED_AT],
+                    },
+                    CONF_CLOCK_FORMAT: config[CONF_CLOCK_FORMAT],
+                    CONF_UNIT_SYSTEM: config[CONF_UNIT_SYSTEM],
+                    CONF_MONITORED_RESOURCES: config[CONF_MONITORED_RESOURCES],
+                },
+            )
+            translation_key = "deprecated_yaml_import"
+            if (
+                result.get("type") == FlowResultType.ABORT
+                and result.get("reason") == "cannot_connect"
+            ):
+                translation_key = "deprecated_yaml_import_issue_cannot_connect"
     else:
         translation_key = "deprecated_yaml_no_import"
 

tests/components/fitbit/test_config_flow.py

@@ -209,9 +209,17 @@ async def test_import_fitbit_config(
     fitbit_config_setup: None,
     sensor_platform_setup: Callable[[], Awaitable[bool]],
     issue_registry: ir.IssueRegistry,
+    requests_mock: Mocker,
 ) -> None:
     """Test that platform configuration is imported successfully."""
 
+    requests_mock.register_uri(
+        "POST",
+        OAUTH2_TOKEN,
+        status_code=HTTPStatus.OK,
+        json=SERVER_ACCESS_TOKEN,
+    )
+
     with patch(
         "homeassistant.components.fitbit.async_setup_entry", return_value=True
     ) as mock_setup:
@@ -256,6 +264,12 @@ async def test_import_fitbit_config_failure_cannot_connect(
 ) -> None:
     """Test platform configuration fails to import successfully."""
 
+    requests_mock.register_uri(
+        "POST",
+        OAUTH2_TOKEN,
+        status_code=HTTPStatus.OK,
+        json=SERVER_ACCESS_TOKEN,
+    )
     requests_mock.register_uri(
         "GET", PROFILE_API_URL, status_code=HTTPStatus.INTERNAL_SERVER_ERROR
     )
@@ -273,6 +287,43 @@ async def test_import_fitbit_config_failure_cannot_connect(
     assert issue.translation_key == "deprecated_yaml_import_issue_cannot_connect"
 
 
+@pytest.mark.parametrize(
+    "status_code",
+    [
+        (HTTPStatus.UNAUTHORIZED),
+        (HTTPStatus.INTERNAL_SERVER_ERROR),
+    ],
+)
+async def test_import_fitbit_config_cannot_refresh(
+    hass: HomeAssistant,
+    fitbit_config_setup: None,
+    sensor_platform_setup: Callable[[], Awaitable[bool]],
+    issue_registry: ir.IssueRegistry,
+    requests_mock: Mocker,
+    status_code: HTTPStatus,
+) -> None:
+    """Test platform configuration import fails when refreshing the token."""
+
+    requests_mock.register_uri(
+        "POST",
+        OAUTH2_TOKEN,
+        status_code=status_code,
+        json="",
+    )
+
+    with patch(
+        "homeassistant.components.fitbit.async_setup_entry", return_value=True
+    ) as mock_setup:
+        await sensor_platform_setup()
+
+    assert len(mock_setup.mock_calls) == 0
+
+    # Verify an issue is raised that we were unable to import configuration
+    issue = issue_registry.issues.get((DOMAIN, "deprecated_yaml"))
+    assert issue
+    assert issue.translation_key == "deprecated_yaml_import_issue_cannot_connect"
+
+
 async def test_import_fitbit_config_already_exists(
     hass: HomeAssistant,
     config_entry: MockConfigEntry,
@@ -281,9 +332,17 @@ async def test_import_fitbit_config_already_exists(
     fitbit_config_setup: None,
     sensor_platform_setup: Callable[[], Awaitable[bool]],
     issue_registry: ir.IssueRegistry,
+    requests_mock: Mocker,
 ) -> None:
     """Test that platform configuration is not imported if it already exists."""
 
+    requests_mock.register_uri(
+        "POST",
+        OAUTH2_TOKEN,
+        status_code=HTTPStatus.OK,
+        json=SERVER_ACCESS_TOKEN,
+    )
+
     # Verify existing config entry
     entries = hass.config_entries.async_entries(DOMAIN)
     assert len(entries) == 1

tests/components/fitbit/test_sensor.py

@@ -9,7 +9,7 @@ import pytest
 from requests_mock.mocker import Mocker
 from syrupy.assertion import SnapshotAssertion
 
-from homeassistant.components.fitbit.const import DOMAIN
+from homeassistant.components.fitbit.const import DOMAIN, OAUTH2_TOKEN
 from homeassistant.const import Platform
 from homeassistant.core import HomeAssistant
 from homeassistant.helpers import entity_registry as er
@@ -23,6 +23,7 @@ from homeassistant.util.unit_system import (
 from .conftest import (
     DEVICES_API_URL,
     PROFILE_USER_ID,
+    SERVER_ACCESS_TOKEN,
     TIMESERIES_API_URL_FORMAT,
     timeseries_response,
 )
@@ -55,6 +56,18 @@ def platforms() -> list[str]:
     return [Platform.SENSOR]
 
 
+@pytest.fixture(autouse=True)
+def mock_token_refresh(requests_mock: Mocker) -> None:
+    """Test that platform configuration is imported successfully."""
+
+    requests_mock.register_uri(
+        "POST",
+        OAUTH2_TOKEN,
+        status_code=HTTPStatus.OK,
+        json=SERVER_ACCESS_TOKEN,
+    )
+
+
 @pytest.mark.parametrize(
     (
         "monitored_resources",