From d5eab6a56fafa21b76d5f734257afe91c0ef2e4b Mon Sep 17 00:00:00 2001 From: Ludeeus Date: Mon, 24 Jan 2022 11:02:08 +0000 Subject: [PATCH 1/3] Add repo scope option to GitHub integration --- .../components/github/config_flow.py | 70 ++++++++++++++++++- homeassistant/components/github/const.py | 1 + .../components/github/coordinator.py | 8 ++- homeassistant/components/github/strings.json | 19 ++++- .../components/github/translations/en.json | 19 ++++- 5 files changed, 112 insertions(+), 5 deletions(-) diff --git a/homeassistant/components/github/config_flow.py b/homeassistant/components/github/config_flow.py index f0e27283355..81525e27cde 100644 --- a/homeassistant/components/github/config_flow.py +++ b/homeassistant/components/github/config_flow.py @@ -27,6 +27,7 @@ import homeassistant.helpers.config_validation as cv from .const import ( CLIENT_ID, CONF_ACCESS_TOKEN, + CONF_REPO_SCOPE, CONF_REPOSITORIES, DEFAULT_REPOSITORIES, DOMAIN, @@ -78,6 +79,8 @@ class ConfigFlow(config_entries.ConfigFlow, domain=DOMAIN): self._device: GitHubDeviceAPI | None = None self._login: GitHubLoginOauthModel | None = None self._login_device: GitHubLoginDeviceModel | None = None + self._repo_scope: bool | None = None + self._reauth = False async def async_step_user( self, @@ -87,6 +90,18 @@ class ConfigFlow(config_entries.ConfigFlow, domain=DOMAIN): if self._async_current_entries(): return self.async_abort(reason="already_configured") + if not user_input: + return self.async_show_form( + step_id="user", + data_schema=vol.Schema( + { + vol.Optional(CONF_REPO_SCOPE, default=False): cv.boolean, + } + ), + ) + + self._repo_scope = user_input[CONF_REPO_SCOPE] + return await self.async_step_device(user_input) async def async_step_device( @@ -94,6 +109,8 @@ class ConfigFlow(config_entries.ConfigFlow, domain=DOMAIN): user_input: dict[str, Any] | None = None, ) -> FlowResult: """Handle device steps.""" + if existing_entry := await self.async_set_unique_id(DOMAIN): + self._repo_scope = existing_entry.options.get(CONF_REPO_SCOPE, False) async def _wait_for_login() -> None: # mypy is not aware that we can't get here without having these set already @@ -118,7 +135,9 @@ class ConfigFlow(config_entries.ConfigFlow, domain=DOMAIN): ) try: - response = await self._device.register() + response = await self._device.register( + **{"scope": "repo" if self._repo_scope else ""} + ) self._login_device = response.data except GitHubException as exception: LOGGER.exception(exception) @@ -141,6 +160,18 @@ class ConfigFlow(config_entries.ConfigFlow, domain=DOMAIN): LOGGER.exception(exception) return self.async_show_progress_done(next_step_id="could_not_register") + if existing_entry: + # mypy is not aware that we can't get here without having this set already + assert self._login is not None + + self.hass.config_entries.async_update_entry( + existing_entry, data={CONF_ACCESS_TOKEN: self._login.access_token} + ) + await self.hass.config_entries.async_reload(existing_entry.entry_id) + return self.async_show_progress_done( + next_step_id="async_step_reauth_completed" + ) + return self.async_show_progress_done(next_step_id="repositories") async def async_step_repositories( @@ -170,9 +201,31 @@ class ConfigFlow(config_entries.ConfigFlow, domain=DOMAIN): return self.async_create_entry( title="", data={CONF_ACCESS_TOKEN: self._login.access_token}, - options={CONF_REPOSITORIES: user_input[CONF_REPOSITORIES]}, + options={ + CONF_REPOSITORIES: user_input[CONF_REPOSITORIES], + CONF_REPO_SCOPE: self._repo_scope, + }, ) + async def async_step_reauth( + self, + user_input: dict[str, Any] | None = None, + ) -> FlowResult: + """Perform reauth upon an API authentication error.""" + 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.""" + if user_input is None: + return self.async_show_form( + step_id="reauth_confirm", + data_schema=vol.Schema({}), + ) + return await self.async_step_device() + async def async_step_could_not_register( self, user_input: dict[str, Any] | None = None, @@ -180,6 +233,13 @@ class ConfigFlow(config_entries.ConfigFlow, domain=DOMAIN): """Handle issues that need transition await from progress step.""" return self.async_abort(reason="could_not_register") + async def async_step_reauth_completed( + self, + user_input: dict[str, Any] | None = None, + ) -> FlowResult: + """Abort with success message for reauth complete.""" + return self.async_abort(reason="reauth_successful") + @staticmethod @callback def async_get_options_flow( @@ -222,6 +282,12 @@ class OptionsFlowHandler(config_entries.OptionsFlow): CONF_REPOSITORIES, default=configured_repositories, ): cv.multi_select({k: k for k in repositories}), + vol.Optional( + CONF_REPO_SCOPE, + default=self.config_entry.options.get( + CONF_REPO_SCOPE, False + ), + ): cv.boolean, } ), ) diff --git a/homeassistant/components/github/const.py b/homeassistant/components/github/const.py index 7a0c471ab03..dba27d2e3a8 100644 --- a/homeassistant/components/github/const.py +++ b/homeassistant/components/github/const.py @@ -18,6 +18,7 @@ DEFAULT_UPDATE_INTERVAL = timedelta(seconds=300) CONF_ACCESS_TOKEN = "access_token" CONF_REPOSITORIES = "repositories" +CONF_REPO_SCOPE = "repo_scope" class IssuesPulls(NamedTuple): diff --git a/homeassistant/components/github/coordinator.py b/homeassistant/components/github/coordinator.py index 790a8680926..5f4eb8afefd 100644 --- a/homeassistant/components/github/coordinator.py +++ b/homeassistant/components/github/coordinator.py @@ -13,9 +13,10 @@ from aiogithubapi import ( from homeassistant.config_entries import ConfigEntry from homeassistant.core import HomeAssistant, T +from homeassistant.exceptions import ConfigEntryAuthFailed from homeassistant.helpers.update_coordinator import DataUpdateCoordinator, UpdateFailed -from .const import DEFAULT_UPDATE_INTERVAL, DOMAIN, LOGGER, IssuesPulls +from .const import CONF_REPO_SCOPE, DEFAULT_UPDATE_INTERVAL, DOMAIN, LOGGER, IssuesPulls CoordinatorKeyType = Literal["information", "release", "issue", "commit"] @@ -23,6 +24,8 @@ CoordinatorKeyType = Literal["information", "release", "issue", "commit"] class GitHubBaseDataUpdateCoordinator(DataUpdateCoordinator[T]): """Base class for GitHub data update coordinators.""" + config_entry: ConfigEntry + def __init__( self, hass: HomeAssistant, @@ -61,6 +64,9 @@ class RepositoryInformationDataUpdateCoordinator( async def fetch_data(self) -> GitHubRepositoryModel: """Get the latest data from GitHub.""" result = await self._client.repos.get(self.repository) + scope = "repo" if self.config_entry.options.get(CONF_REPO_SCOPE, False) else "" + if scope != result.headers.x_oauth_scopes: + raise ConfigEntryAuthFailed("Invalid OAuth scopes") return result.data diff --git a/homeassistant/components/github/strings.json b/homeassistant/components/github/strings.json index ac80086ed3a..6b025664b65 100644 --- a/homeassistant/components/github/strings.json +++ b/homeassistant/components/github/strings.json @@ -1,6 +1,12 @@ { "config": { "step": { + "user": { + "description": "Granting full access to repositories is needed if you want to use this integration with private repositories.", + "data": { + "repo_scope": "Grant full access to repositories" + } + }, "repositories": { "title": "Configure repositories", "data": { @@ -13,7 +19,18 @@ }, "abort": { "already_configured": "[%key:common::config_flow::abort::already_configured_service%]", - "could_not_register": "Could not register integration with GitHub" + "could_not_register": "Could not register integration with GitHub", + "reauth_successful": "[%key:common::config_flow::abort::reauth_successful%]" + } + }, + "options": { + "step": { + "init": { + "data": { + "repositories": "[%key:component::github::config::step::repositories::data::repositories%]", + "repo_scope": "[%key:component::github::config::step::user::data::repo_scope%]" + } + } } } } \ No newline at end of file diff --git a/homeassistant/components/github/translations/en.json b/homeassistant/components/github/translations/en.json index a082de3af67..91f599cfd2a 100644 --- a/homeassistant/components/github/translations/en.json +++ b/homeassistant/components/github/translations/en.json @@ -2,7 +2,8 @@ "config": { "abort": { "already_configured": "Service is already configured", - "could_not_register": "Could not register integration with GitHub" + "could_not_register": "Could not register integration with GitHub", + "reauth_successful": "Re-authentication was successful" }, "progress": { "wait_for_device": "1. Open {url} \n2.Paste the following key to authorize the integration: \n```\n{code}\n```\n" @@ -13,6 +14,22 @@ "repositories": "Select repositories to track." }, "title": "Configure repositories" + }, + "user": { + "data": { + "repo_scope": "Grant full access to repositories" + }, + "description": "Granting full access to repositories is needed if you want to use this integration with private repositories." + } + } + }, + "options": { + "step": { + "init": { + "data": { + "repo_scope": "Grant full access to repositories", + "repositories": "Select repositories to track." + } } } } From 3ff5bbe0fd9c4fd113d82193af0ca02b9f580af1 Mon Sep 17 00:00:00 2001 From: Ludeeus Date: Mon, 24 Jan 2022 12:02:11 +0000 Subject: [PATCH 2/3] Fix current tests --- .../components/github/coordinator.py | 11 ++++---- homeassistant/components/github/strings.json | 4 +++ .../components/github/translations/en.json | 4 +++ tests/components/github/conftest.py | 4 ++- tests/components/github/test_config_flow.py | 28 +++++++++++++++++++ 5 files changed, 44 insertions(+), 7 deletions(-) diff --git a/homeassistant/components/github/coordinator.py b/homeassistant/components/github/coordinator.py index 81c3e1376a2..d2d61485e69 100644 --- a/homeassistant/components/github/coordinator.py +++ b/homeassistant/components/github/coordinator.py @@ -64,11 +64,6 @@ class GitHubBaseDataUpdateCoordinator(DataUpdateCoordinator[T]): async def _async_update_data(self) -> T: try: response = await self.fetch_data() - scope = ( - "repo" if self.config_entry.options.get(CONF_REPO_SCOPE, False) else "" - ) - if scope != response.headers.x_oauth_scopes: - raise ConfigEntryAuthFailed("Invalid OAuth scopes") except GitHubNotModifiedException: LOGGER.debug( "Content for %s with %s not modified", @@ -92,7 +87,11 @@ class RepositoryInformationDataUpdateCoordinator( async def fetch_data(self) -> GitHubResponseModel[GitHubRepositoryModel]: """Get the latest data from GitHub.""" - return await self._client.repos.get(self.repository, **{"etag": self._etag}) + response = await self._client.repos.get(self.repository, **{"etag": self._etag}) + scope = "repo" if self.config_entry.options.get(CONF_REPO_SCOPE, False) else "" + if scope != response.headers.x_oauth_scopes: + raise ConfigEntryAuthFailed("Invalid OAuth scopes") + return response class RepositoryReleaseDataUpdateCoordinator( diff --git a/homeassistant/components/github/strings.json b/homeassistant/components/github/strings.json index 6b025664b65..6834f6e2e54 100644 --- a/homeassistant/components/github/strings.json +++ b/homeassistant/components/github/strings.json @@ -12,6 +12,10 @@ "data": { "repositories": "Select repositories to track." } + }, + "reauth_confirm": { + "title": "[%key:common::config_flow::title::reauth%]", + "description": "The GitHub integration needs to re-authenticate your account to match the selected scopes." } }, "progress": { diff --git a/homeassistant/components/github/translations/en.json b/homeassistant/components/github/translations/en.json index 91f599cfd2a..dfb01d7a277 100644 --- a/homeassistant/components/github/translations/en.json +++ b/homeassistant/components/github/translations/en.json @@ -9,6 +9,10 @@ "wait_for_device": "1. Open {url} \n2.Paste the following key to authorize the integration: \n```\n{code}\n```\n" }, "step": { + "reauth_confirm": { + "description": "The GitHub integration needs to re-authenticate your account to match the selected scopes.", + "title": "Reauthenticate Integration" + }, "repositories": { "data": { "repositories": "Select repositories to track." diff --git a/tests/components/github/conftest.py b/tests/components/github/conftest.py index 04b53da6b91..82cee3aefc8 100644 --- a/tests/components/github/conftest.py +++ b/tests/components/github/conftest.py @@ -6,6 +6,7 @@ import pytest from homeassistant.components.github.const import ( CONF_ACCESS_TOKEN, + CONF_REPO_SCOPE, CONF_REPOSITORIES, DOMAIN, ) @@ -23,8 +24,9 @@ def mock_config_entry() -> MockConfigEntry: return MockConfigEntry( title="", domain=DOMAIN, + unique_id=DOMAIN, data={CONF_ACCESS_TOKEN: MOCK_ACCESS_TOKEN}, - options={CONF_REPOSITORIES: [TEST_REPOSITORY]}, + options={CONF_REPOSITORIES: [TEST_REPOSITORY], CONF_REPO_SCOPE: False}, ) diff --git a/tests/components/github/test_config_flow.py b/tests/components/github/test_config_flow.py index dad97472620..a5be8a721d7 100644 --- a/tests/components/github/test_config_flow.py +++ b/tests/components/github/test_config_flow.py @@ -7,6 +7,7 @@ from homeassistant import config_entries from homeassistant.components.github.config_flow import starred_repositories from homeassistant.components.github.const import ( CONF_ACCESS_TOKEN, + CONF_REPO_SCOPE, CONF_REPOSITORIES, DEFAULT_REPOSITORIES, DOMAIN, @@ -62,6 +63,17 @@ async def test_full_user_flow_implementation( context={"source": config_entries.SOURCE_USER}, ) + assert result["step_id"] == "user" + assert result["type"] == "form" + assert "flow_id" in result + + result = await hass.config_entries.flow.async_configure( + result["flow_id"], + user_input={ + CONF_REPO_SCOPE: False, + }, + ) + assert result["step_id"] == "device" assert result["type"] == RESULT_TYPE_SHOW_PROGRESS assert "flow_id" in result @@ -92,10 +104,19 @@ async def test_flow_with_registration_failure( "https://github.com/login/device/code", exc=GitHubException("Registration failed"), ) + result = await hass.config_entries.flow.async_init( DOMAIN, context={"source": config_entries.SOURCE_USER}, ) + + result = await hass.config_entries.flow.async_configure( + result["flow_id"], + user_input={ + CONF_REPO_SCOPE: False, + }, + ) + assert result["type"] == RESULT_TYPE_ABORT assert result.get("reason") == "could_not_register" @@ -124,6 +145,13 @@ async def test_flow_with_activation_failure( DOMAIN, context={"source": config_entries.SOURCE_USER}, ) + result = await hass.config_entries.flow.async_configure( + result["flow_id"], + user_input={ + CONF_REPO_SCOPE: False, + }, + ) + assert result["step_id"] == "device" assert result["type"] == RESULT_TYPE_SHOW_PROGRESS From 4c78115fb350793cc2f8d0c543a3219a3930212e Mon Sep 17 00:00:00 2001 From: Ludeeus Date: Mon, 24 Jan 2022 14:00:02 +0000 Subject: [PATCH 3/3] use async_get_entry --- homeassistant/components/github/config_flow.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/homeassistant/components/github/config_flow.py b/homeassistant/components/github/config_flow.py index 81525e27cde..75d048c64c7 100644 --- a/homeassistant/components/github/config_flow.py +++ b/homeassistant/components/github/config_flow.py @@ -109,7 +109,9 @@ class ConfigFlow(config_entries.ConfigFlow, domain=DOMAIN): user_input: dict[str, Any] | None = None, ) -> FlowResult: """Handle device steps.""" - if existing_entry := await self.async_set_unique_id(DOMAIN): + if existing_entry := self.hass.config_entries.async_get_entry( + self.context["entry_id"] + ): self._repo_scope = existing_entry.options.get(CONF_REPO_SCOPE, False) async def _wait_for_login() -> None: