From 22a0b57fb5a01e771ec95f95735405ad457edd20 Mon Sep 17 00:00:00 2001 From: tronikos Date: Fri, 9 Dec 2022 17:21:12 +0000 Subject: [PATCH 1/7] Remove duplicate object key in strings.json --- homeassistant/components/google_sheets/strings.json | 4 ---- 1 file changed, 4 deletions(-) diff --git a/homeassistant/components/google_sheets/strings.json b/homeassistant/components/google_sheets/strings.json index 33230038cdf9a..602301758f8b8 100644 --- a/homeassistant/components/google_sheets/strings.json +++ b/homeassistant/components/google_sheets/strings.json @@ -4,10 +4,6 @@ "pick_implementation": { "title": "[%key:common::config_flow::title::oauth2_pick_implementation%]" }, - "reauth_confirm": { - "title": "[%key:common::config_flow::title::reauth%]", - "description": "The Google Sheets integration needs to re-authenticate your account" - }, "auth": { "title": "Link Google Account" }, From 63e24f84cc1c0250ee5fa57c7f624bbed12e0221 Mon Sep 17 00:00:00 2001 From: tronikos Date: Fri, 9 Dec 2022 17:27:34 +0000 Subject: [PATCH 2/7] Remove async_entry_has_scopes check This is not needed. This was copied from google calendar integration where it was needed to reauth when the scope changed. --- .../components/google_sheets/__init__.py | 10 +------ .../components/google_sheets/config_flow.py | 4 +-- .../components/google_sheets/const.py | 1 - tests/components/google_sheets/test_init.py | 26 ------------------- 4 files changed, 3 insertions(+), 38 deletions(-) diff --git a/homeassistant/components/google_sheets/__init__.py b/homeassistant/components/google_sheets/__init__.py index 19f5ce81f5c34..3b4ecfc649fc3 100644 --- a/homeassistant/components/google_sheets/__init__.py +++ b/homeassistant/components/google_sheets/__init__.py @@ -21,7 +21,7 @@ import homeassistant.helpers.config_validation as cv from homeassistant.helpers.selector import ConfigEntrySelector -from .const import DATA_CONFIG_ENTRY, DEFAULT_ACCESS, DOMAIN +from .const import DATA_CONFIG_ENTRY, DOMAIN DATA = "data" WORKSHEET = "worksheet" @@ -51,9 +51,6 @@ async def async_setup_entry(hass: HomeAssistant, entry: ConfigEntry) -> bool: raise ConfigEntryNotReady from err except aiohttp.ClientError as err: raise ConfigEntryNotReady from err - - if not async_entry_has_scopes(hass, entry): - raise ConfigEntryAuthFailed("Required scopes are not present, reauth required") hass.data.setdefault(DOMAIN, {})[entry.entry_id] = session await async_setup_service(hass) @@ -61,11 +58,6 @@ async def async_setup_entry(hass: HomeAssistant, entry: ConfigEntry) -> bool: return True -def async_entry_has_scopes(hass: HomeAssistant, entry: ConfigEntry) -> bool: - """Verify that the config entry desired scope is present in the oauth token.""" - return DEFAULT_ACCESS in entry.data.get(CONF_TOKEN, {}).get("scope", "").split(" ") - - async def async_unload_entry(hass: HomeAssistant, entry: ConfigEntry) -> bool: """Unload a config entry.""" hass.data[DOMAIN].pop(entry.entry_id) diff --git a/homeassistant/components/google_sheets/config_flow.py b/homeassistant/components/google_sheets/config_flow.py index 3805ee9d38b12..a4ec42e6e248a 100644 --- a/homeassistant/components/google_sheets/config_flow.py +++ b/homeassistant/components/google_sheets/config_flow.py @@ -13,7 +13,7 @@ from homeassistant.data_entry_flow import FlowResult from homeassistant.helpers import config_entry_oauth2_flow -from .const import DEFAULT_ACCESS, DEFAULT_NAME, DOMAIN +from .const import DEFAULT_NAME, DOMAIN _LOGGER = logging.getLogger(__name__) @@ -36,7 +36,7 @@ def logger(self) -> logging.Logger: def extra_authorize_data(self) -> dict[str, Any]: """Extra data that needs to be appended to the authorize url.""" return { - "scope": DEFAULT_ACCESS, + "scope": "https://www.googleapis.com/auth/drive.file", # Add params to ensure we get back a refresh token "access_type": "offline", "prompt": "consent", diff --git a/homeassistant/components/google_sheets/const.py b/homeassistant/components/google_sheets/const.py index f8f065972f997..56b29ed64309b 100644 --- a/homeassistant/components/google_sheets/const.py +++ b/homeassistant/components/google_sheets/const.py @@ -7,4 +7,3 @@ DATA_CONFIG_ENTRY: Final = "config_entry" DEFAULT_NAME = "Google Sheets" -DEFAULT_ACCESS = "https://www.googleapis.com/auth/drive.file" diff --git a/tests/components/google_sheets/test_init.py b/tests/components/google_sheets/test_init.py index f77edcbb4915b..14fe96fe6b47f 100644 --- a/tests/components/google_sheets/test_init.py +++ b/tests/components/google_sheets/test_init.py @@ -95,32 +95,6 @@ async def test_setup_success( assert not len(hass.services.async_services().get(DOMAIN, {})) -@pytest.mark.parametrize( - "scopes", - [ - [], - [ - "https://www.googleapis.com/auth/drive.file+plus+extra" - ], # Required scope is a prefix - ["https://www.googleapis.com/auth/drive.readonly"], - ], - ids=["no_scope", "required_scope_prefix", "other_scope"], -) -async def test_missing_required_scopes_requires_reauth( - hass: HomeAssistant, setup_integration: ComponentSetup -) -> None: - """Test that reauth is invoked when required scopes are not present.""" - await setup_integration() - - entries = hass.config_entries.async_entries(DOMAIN) - assert len(entries) == 1 - assert entries[0].state is ConfigEntryState.SETUP_ERROR - - flows = hass.config_entries.flow.async_progress() - assert len(flows) == 1 - assert flows[0]["step_id"] == "reauth_confirm" - - @pytest.mark.parametrize("expires_at", [time.time() - 3600], ids=["expired"]) async def test_expired_token_refresh_success( hass: HomeAssistant, From 776e26d9a610c826cca2181e65c86b47b0ccfa85 Mon Sep 17 00:00:00 2001 From: tronikos Date: Fri, 9 Dec 2022 17:29:01 +0000 Subject: [PATCH 3/7] Remove unused constant in application_credentials --- .../components/google_sheets/application_credentials.py | 5 ----- 1 file changed, 5 deletions(-) diff --git a/homeassistant/components/google_sheets/application_credentials.py b/homeassistant/components/google_sheets/application_credentials.py index c54356b659e8c..5294282559537 100644 --- a/homeassistant/components/google_sheets/application_credentials.py +++ b/homeassistant/components/google_sheets/application_credentials.py @@ -1,14 +1,9 @@ """application_credentials platform for Google Sheets.""" - import oauth2client from homeassistant.components.application_credentials import AuthorizationServer from homeassistant.core import HomeAssistant -AUTHORIZATION_SERVER = AuthorizationServer( - oauth2client.GOOGLE_AUTH_URI, oauth2client.GOOGLE_TOKEN_URI -) - async def async_get_authorization_server(hass: HomeAssistant) -> AuthorizationServer: """Return authorization server.""" From a508a2ff7241b9eb8a43f4e367eaba69441b6eb1 Mon Sep 17 00:00:00 2001 From: tronikos Date: Fri, 9 Dec 2022 17:31:40 +0000 Subject: [PATCH 4/7] Move constant to the file used --- homeassistant/components/google_sheets/__init__.py | 3 ++- homeassistant/components/google_sheets/const.py | 3 --- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/homeassistant/components/google_sheets/__init__.py b/homeassistant/components/google_sheets/__init__.py index 3b4ecfc649fc3..4c13bba45c380 100644 --- a/homeassistant/components/google_sheets/__init__.py +++ b/homeassistant/components/google_sheets/__init__.py @@ -21,9 +21,10 @@ import homeassistant.helpers.config_validation as cv from homeassistant.helpers.selector import ConfigEntrySelector -from .const import DATA_CONFIG_ENTRY, DOMAIN +from .const import DOMAIN DATA = "data" +DATA_CONFIG_ENTRY = "config_entry" WORKSHEET = "worksheet" SERVICE_APPEND_SHEET = "append_sheet" diff --git a/homeassistant/components/google_sheets/const.py b/homeassistant/components/google_sheets/const.py index 56b29ed64309b..6937f53f78f40 100644 --- a/homeassistant/components/google_sheets/const.py +++ b/homeassistant/components/google_sheets/const.py @@ -1,9 +1,6 @@ """Constants for Google Sheets integration.""" from __future__ import annotations -from typing import Final - DOMAIN = "google_sheets" -DATA_CONFIG_ENTRY: Final = "config_entry" DEFAULT_NAME = "Google Sheets" From 2a9f2bc33e81f9b0d7d085cbbdca220d1e1c61a9 Mon Sep 17 00:00:00 2001 From: tronikos Date: Fri, 9 Dec 2022 17:33:53 +0000 Subject: [PATCH 5/7] fix warning use-implicit-booleaness-not-len --- tests/components/google_sheets/test_init.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/components/google_sheets/test_init.py b/tests/components/google_sheets/test_init.py index 14fe96fe6b47f..951b98f2f419b 100644 --- a/tests/components/google_sheets/test_init.py +++ b/tests/components/google_sheets/test_init.py @@ -92,7 +92,7 @@ async def test_setup_success( assert not hass.data.get(DOMAIN) assert entries[0].state is ConfigEntryState.NOT_LOADED - assert not len(hass.services.async_services().get(DOMAIN, {})) + assert not hass.services.async_services().get(DOMAIN, {}) @pytest.mark.parametrize("expires_at", [time.time() - 3600], ids=["expired"]) From 40914bb8720d83650239be3ad4302368bc9a7d79 Mon Sep 17 00:00:00 2001 From: tronikos Date: Fri, 9 Dec 2022 21:25:22 +0000 Subject: [PATCH 6/7] Remove not accessed parameters --- tests/components/google_sheets/test_init.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/tests/components/google_sheets/test_init.py b/tests/components/google_sheets/test_init.py index 951b98f2f419b..2b9dc6df5ade6 100644 --- a/tests/components/google_sheets/test_init.py +++ b/tests/components/google_sheets/test_init.py @@ -99,7 +99,6 @@ async def test_setup_success( async def test_expired_token_refresh_success( hass: HomeAssistant, setup_integration: ComponentSetup, - scopes: list[str], aioclient_mock: AiohttpClientMocker, ) -> None: """Test expired token is refreshed.""" @@ -142,7 +141,6 @@ async def test_expired_token_refresh_success( async def test_expired_token_refresh_failure( hass: HomeAssistant, setup_integration: ComponentSetup, - scopes: list[str], aioclient_mock: AiohttpClientMocker, status: http.HTTPStatus, expected_state: ConfigEntryState, From 6a0fe0492f2de39cdd77cb69eb90d5729c1be9a7 Mon Sep 17 00:00:00 2001 From: tronikos Date: Sat, 10 Dec 2022 04:10:01 +0000 Subject: [PATCH 7/7] Revert "Remove async_entry_has_scopes check" This reverts commit 63e24f84cc1c0250ee5fa57c7f624bbed12e0221. --- .../components/google_sheets/__init__.py | 10 ++++++- .../components/google_sheets/config_flow.py | 4 +-- .../components/google_sheets/const.py | 1 + tests/components/google_sheets/test_init.py | 26 +++++++++++++++++++ 4 files changed, 38 insertions(+), 3 deletions(-) diff --git a/homeassistant/components/google_sheets/__init__.py b/homeassistant/components/google_sheets/__init__.py index 4c13bba45c380..803b737283b33 100644 --- a/homeassistant/components/google_sheets/__init__.py +++ b/homeassistant/components/google_sheets/__init__.py @@ -21,7 +21,7 @@ import homeassistant.helpers.config_validation as cv from homeassistant.helpers.selector import ConfigEntrySelector -from .const import DOMAIN +from .const import DEFAULT_ACCESS, DOMAIN DATA = "data" DATA_CONFIG_ENTRY = "config_entry" @@ -52,6 +52,9 @@ async def async_setup_entry(hass: HomeAssistant, entry: ConfigEntry) -> bool: raise ConfigEntryNotReady from err except aiohttp.ClientError as err: raise ConfigEntryNotReady from err + + if not async_entry_has_scopes(hass, entry): + raise ConfigEntryAuthFailed("Required scopes are not present, reauth required") hass.data.setdefault(DOMAIN, {})[entry.entry_id] = session await async_setup_service(hass) @@ -59,6 +62,11 @@ async def async_setup_entry(hass: HomeAssistant, entry: ConfigEntry) -> bool: return True +def async_entry_has_scopes(hass: HomeAssistant, entry: ConfigEntry) -> bool: + """Verify that the config entry desired scope is present in the oauth token.""" + return DEFAULT_ACCESS in entry.data.get(CONF_TOKEN, {}).get("scope", "").split(" ") + + async def async_unload_entry(hass: HomeAssistant, entry: ConfigEntry) -> bool: """Unload a config entry.""" hass.data[DOMAIN].pop(entry.entry_id) diff --git a/homeassistant/components/google_sheets/config_flow.py b/homeassistant/components/google_sheets/config_flow.py index a4ec42e6e248a..3805ee9d38b12 100644 --- a/homeassistant/components/google_sheets/config_flow.py +++ b/homeassistant/components/google_sheets/config_flow.py @@ -13,7 +13,7 @@ from homeassistant.data_entry_flow import FlowResult from homeassistant.helpers import config_entry_oauth2_flow -from .const import DEFAULT_NAME, DOMAIN +from .const import DEFAULT_ACCESS, DEFAULT_NAME, DOMAIN _LOGGER = logging.getLogger(__name__) @@ -36,7 +36,7 @@ def logger(self) -> logging.Logger: def extra_authorize_data(self) -> dict[str, Any]: """Extra data that needs to be appended to the authorize url.""" return { - "scope": "https://www.googleapis.com/auth/drive.file", + "scope": DEFAULT_ACCESS, # Add params to ensure we get back a refresh token "access_type": "offline", "prompt": "consent", diff --git a/homeassistant/components/google_sheets/const.py b/homeassistant/components/google_sheets/const.py index 6937f53f78f40..71ef8f4a4f48f 100644 --- a/homeassistant/components/google_sheets/const.py +++ b/homeassistant/components/google_sheets/const.py @@ -4,3 +4,4 @@ DOMAIN = "google_sheets" DEFAULT_NAME = "Google Sheets" +DEFAULT_ACCESS = "https://www.googleapis.com/auth/drive.file" diff --git a/tests/components/google_sheets/test_init.py b/tests/components/google_sheets/test_init.py index 2b9dc6df5ade6..4c70854409905 100644 --- a/tests/components/google_sheets/test_init.py +++ b/tests/components/google_sheets/test_init.py @@ -95,6 +95,32 @@ async def test_setup_success( assert not hass.services.async_services().get(DOMAIN, {}) +@pytest.mark.parametrize( + "scopes", + [ + [], + [ + "https://www.googleapis.com/auth/drive.file+plus+extra" + ], # Required scope is a prefix + ["https://www.googleapis.com/auth/drive.readonly"], + ], + ids=["no_scope", "required_scope_prefix", "other_scope"], +) +async def test_missing_required_scopes_requires_reauth( + hass: HomeAssistant, setup_integration: ComponentSetup +) -> None: + """Test that reauth is invoked when required scopes are not present.""" + await setup_integration() + + entries = hass.config_entries.async_entries(DOMAIN) + assert len(entries) == 1 + assert entries[0].state is ConfigEntryState.SETUP_ERROR + + flows = hass.config_entries.flow.async_progress() + assert len(flows) == 1 + assert flows[0]["step_id"] == "reauth_confirm" + + @pytest.mark.parametrize("expires_at", [time.time() - 3600], ids=["expired"]) async def test_expired_token_refresh_success( hass: HomeAssistant,