Skip to content

Commit 4269035

Browse files
committed
refactor(config_flow): defer credential storage until OAuth succeeds
Replaces the eager-replace approach from the prior PR. The form submit no longer mutates the application_credentials store; instead it stages the new credentials on self._pending_credential, builds an in-memory LocalOAuth2Implementation, sets self.flow_impl, and jumps to async_step_auth. The actual store mutation runs in async_oauth_create_entry on the OAuth-success path. Two bugs this fixes: 1. Reconfigure with bad creds left the entry broken. The old flow deleted the original cred before OAuth ran; if OAuth failed at Xthings (bad secret, user rejection, etc.), the existing entry's stored token was no longer refreshable against any valid client and the integration looped on "failed to unload". With deferred storage, OAuth failure leaves the original cred intact and the entry keeps working. 2. First-time install showed two consecutive credential prompts. HA's stock "missing_credentials" flow added the cred via its own UI, then restarted the flow; our async_step_user routing then rendered our form on top, asking for the same cred a second time. async_step_user now always renders our form directly, bypassing HA's stock prompt. The unused scope step (whose value was never actually applied to extra_authorize_data) is dropped. Trade-offs noted: - Drops the never-applied API scope field (DEFAULT_API_SCOPE is hard-coded into extra_authorize_data, matching the prior runtime behavior). - Drops the credential_import_failed error path from the form, since imports now happen during async_oauth_create_entry; if the import fails there, HA aborts the flow with the standard oauth_error reason. Test coverage: 15 tests in test_config_flow_reconfigure.py (form rendering, user-step routing, submit-does-not-touch-store, pending- credential staging, validation, reconfigure routing, commit-on-success, delete-then-import for matching client_id, no-pending no-op, source- based entry update for reconfigure and reauth).
1 parent 4c34a29 commit 4269035

3 files changed

Lines changed: 377 additions & 239 deletions

File tree

custom_components/u_tec/config_flow.py

Lines changed: 97 additions & 95 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,6 @@
3838
from utec_py.devices.switch import Switch as UhomeSwitch
3939

4040
from .const import (
41-
CONF_API_SCOPE,
4241
CONF_HA_DEVICES,
4342
CONF_OPTIMISTIC_LIGHTS,
4443
CONF_OPTIMISTIC_LOCKS,
@@ -47,14 +46,8 @@
4746
CONF_PUSH_ENABLED,
4847
DEFAULT_API_SCOPE,
4948
DOMAIN,
50-
)
51-
52-
STEP_USER_DATA_SCHEMA = vol.Schema(
53-
{
54-
# vol.Required(CONF_CLIENT_ID): str,
55-
# vol.Optional(CONF_PUSH_ENABLED, default="push_enabled"): BooleanSelector(),
56-
vol.Optional(CONF_API_SCOPE, default=DEFAULT_API_SCOPE): str,
57-
}
49+
OAUTH2_AUTHORIZE,
50+
OAUTH2_TOKEN,
5851
)
5952

6053

@@ -87,8 +80,7 @@ class UhomeOAuth2FlowHandler(
8780
def __init__(self) -> None:
8881
"""Initialize Uhome OAuth2 flow."""
8982
super().__init__()
90-
self._api_scope = None
91-
self.data = {}
83+
self._pending_credential: ClientCredential | None = None
9284

9385
@property
9486
def logger(self) -> logging.Logger:
@@ -98,29 +90,18 @@ def logger(self) -> logging.Logger:
9890
@property
9991
def extra_authorize_data(self) -> dict[str, Any]:
10092
"""Extra data that needs to be appended to the authorize url."""
101-
return {"scope": self._api_scope or DEFAULT_API_SCOPE}
93+
return {"scope": DEFAULT_API_SCOPE}
10294

10395
async def async_step_user(self, user_input=None) -> ConfigFlowResult:
104-
"""Prompt the user to enter their client credentials and API scope."""
96+
"""Entry point for initial setup.
97+
98+
Always renders the credential form (blank or prefilled from any existing
99+
application_credentials entry) — bypasses HA's stock "missing_credentials"
100+
prompt entirely so the user only ever sees one credential form per setup.
101+
"""
105102
if self._async_current_entries():
106103
return self.async_abort(reason="single_instance_allowed")
107-
108-
# Issue #50 recovery: if credentials are already stored from a prior
109-
# failed attempt, route the user to the inline replace form instead
110-
# of the scope-only user form. This avoids the hidden Application
111-
# Credentials menu detour for fixing typos.
112-
if self._get_existing_credential() is not None:
113-
return await self.async_step_replace_credentials()
114-
115-
if user_input is not None:
116-
self.data = user_input
117-
return await self.async_step_pick_implementation()
118-
119-
errors = {}
120-
121-
return self.async_show_form(
122-
step_id="user", data_schema=STEP_USER_DATA_SCHEMA, errors=errors
123-
)
104+
return await self.async_step_replace_credentials()
124105

125106
def _get_existing_credential(self) -> dict | None:
126107
"""Return the first stored credential dict for this domain, or None."""
@@ -135,15 +116,18 @@ def _get_existing_credential(self) -> dict | None:
135116
async def async_step_replace_credentials(
136117
self, user_input: dict[str, Any] | None = None
137118
) -> ConfigFlowResult:
138-
"""Render and process the inline replace-credentials form.
139-
140-
Used both for issue #50 recovery (initial setup with stale creds in
141-
HA's app-creds store) and for reconfigure of working entries.
142-
143-
For items whose client_id matches the new one, we delete BEFORE import
144-
so the import is not a no-op (async_import_item returns early on duplicate
145-
suggested_id). For items with a different client_id, import first then
146-
delete — so a partial failure leaves the entry pointing at a valid cred.
119+
"""Render the credential form and start OAuth with an in-memory implementation.
120+
121+
Used by initial setup (via async_step_user), issue #50 recovery (stale creds
122+
in HA's app-creds store from a prior failed attempt), and reconfigure of a
123+
working entry (via async_step_reconfigure).
124+
125+
The application_credentials store is NOT mutated here. We build an in-memory
126+
LocalOAuth2Implementation with the entered creds, set self.flow_impl directly,
127+
and jump to async_step_auth — bypassing async_step_pick_implementation. The
128+
actual store update happens in async_oauth_create_entry, on the OAuth-success
129+
path only. Consequence: if OAuth fails, any existing credential is untouched
130+
and a working config entry continues to refresh against valid creds.
147131
"""
148132
if user_input is not None:
149133
client_id = (user_input.get("client_id") or "").strip()
@@ -155,61 +139,16 @@ async def async_step_replace_credentials(
155139
errors={"base": "empty_credentials"},
156140
)
157141

158-
# Snapshot existing items grouped by whether their client_id matches.
159-
storage = self.hass.data.get(APP_CREDS_DATA)
160-
matching_ids: list[str] = []
161-
other_ids: list[str] = []
162-
if storage is not None:
163-
for item in storage.async_items():
164-
if item.get(APP_CREDS_DOMAIN) != DOMAIN:
165-
continue
166-
if item.get(APP_CREDS_CLIENT_ID) == client_id:
167-
matching_ids.append(item[APP_CREDS_ID])
168-
else:
169-
other_ids.append(item[APP_CREDS_ID])
170-
171-
# For matching client_ids, delete BEFORE import so the import is not
172-
# a no-op. Acceptable safety tradeoff: between delete and import there
173-
# is no cred for this domain, but no concurrent code runs (single
174-
# coroutine, no foreign awaits in the gap).
175-
for item_id in matching_ids:
176-
try:
177-
await storage.async_delete_item(item_id)
178-
except Exception as err: # noqa: BLE001
179-
_LOGGER.warning(
180-
"Failed to delete pre-existing u_tec credential %s before re-import: %s",
181-
item_id,
182-
err,
183-
)
184-
185-
try:
186-
await async_import_client_credential(
187-
self.hass,
188-
DOMAIN,
189-
ClientCredential(client_id, client_secret),
190-
"u_tec",
191-
)
192-
except Exception as err: # noqa: BLE001
193-
_LOGGER.error("Failed to import new u_tec credential: %s", err)
194-
return self._show_replace_form(
195-
client_id=client_id,
196-
errors={"base": "credential_import_failed"},
197-
)
198-
199-
# For non-matching client_ids, delete AFTER import (safe order —
200-
# entry still references a valid cred while old ones are cleaned up).
201-
if storage is not None:
202-
for item_id in other_ids:
203-
try:
204-
await storage.async_delete_item(item_id)
205-
except Exception as err: # noqa: BLE001
206-
_LOGGER.warning(
207-
"Failed to delete stale u_tec credential %s: %s",
208-
item_id,
209-
err,
210-
)
211-
212-
return await self.async_step_pick_implementation()
142+
self._pending_credential = ClientCredential(client_id, client_secret)
143+
self.flow_impl = config_entry_oauth2_flow.LocalOAuth2Implementation(
144+
self.hass,
145+
DOMAIN,
146+
client_id,
147+
client_secret,
148+
OAUTH2_AUTHORIZE,
149+
OAUTH2_TOKEN,
150+
)
151+
return await self.async_step_auth()
213152

214153
return self._show_replace_form(client_id=None)
215154

@@ -238,7 +177,17 @@ def _show_replace_form(
238177
async def async_oauth_create_entry(
239178
self, data: dict
240179
) -> ConfigFlowResult:
241-
"""Create or update the config entry depending on the flow source."""
180+
"""Create or update the config entry depending on the flow source.
181+
182+
If credentials were entered via async_step_replace_credentials and OAuth
183+
succeeded, commit them to the application_credentials store now and rewrite
184+
the entry's auth_implementation reference to the standard "u_tec" auth_domain
185+
(which is what async_get_implementations resolves against).
186+
"""
187+
if self._pending_credential is not None:
188+
await self._commit_pending_credential()
189+
data = {**data, "auth_implementation": "u_tec"}
190+
242191
if self.source == SOURCE_RECONFIGURE:
243192
entry = self._get_reconfigure_entry()
244193
return self.async_update_reload_and_abort(
@@ -270,6 +219,59 @@ async def async_oauth_create_entry(
270219
title=self.flow_impl.name, data=data, options=options
271220
)
272221

222+
async def _commit_pending_credential(self) -> None:
223+
"""Persist the deferred credential to the application_credentials store.
224+
225+
Called from async_oauth_create_entry on the OAuth-success path. For items
226+
whose client_id matches the new one, we delete BEFORE import (otherwise
227+
async_import_item is a no-op on duplicate suggested_id and the secret
228+
wouldn't update). For items with a different client_id, we import first
229+
then delete — preserving a valid cred for the entry's auth_implementation
230+
reference throughout.
231+
"""
232+
assert self._pending_credential is not None # guarded by caller
233+
new_client_id = self._pending_credential.client_id
234+
235+
storage = self.hass.data.get(APP_CREDS_DATA)
236+
matching_ids: list[str] = []
237+
other_ids: list[str] = []
238+
if storage is not None:
239+
for item in storage.async_items():
240+
if item.get(APP_CREDS_DOMAIN) != DOMAIN:
241+
continue
242+
if item.get(APP_CREDS_CLIENT_ID) == new_client_id:
243+
matching_ids.append(item[APP_CREDS_ID])
244+
else:
245+
other_ids.append(item[APP_CREDS_ID])
246+
247+
for item_id in matching_ids:
248+
try:
249+
await storage.async_delete_item(item_id)
250+
except Exception as err: # noqa: BLE001
251+
_LOGGER.warning(
252+
"Failed to delete pre-existing u_tec credential %s before re-import: %s",
253+
item_id,
254+
err,
255+
)
256+
257+
await async_import_client_credential(
258+
self.hass,
259+
DOMAIN,
260+
self._pending_credential,
261+
"u_tec",
262+
)
263+
264+
if storage is not None:
265+
for item_id in other_ids:
266+
try:
267+
await storage.async_delete_item(item_id)
268+
except Exception as err: # noqa: BLE001
269+
_LOGGER.warning(
270+
"Failed to delete stale u_tec credential %s: %s",
271+
item_id,
272+
err,
273+
)
274+
273275
async def async_step_reauth(
274276
self, entry_data: Mapping[str, vol.Any]
275277
) -> ConfigFlowResult:

custom_components/u_tec/strings.json

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,8 +9,8 @@
99
"description": "The Uhome integration needs to re-authenticate your account"
1010
},
1111
"replace_credentials": {
12-
"title": "Update U-Tec credentials",
13-
"description": "Enter your Client ID and Client Secret from the Xthings app (My Account → OpenAPI). Submitting will replace any previously stored credentials and start a new authorization. The Client Secret is not displayed for security; please re-enter it.",
12+
"title": "U-Tec credentials",
13+
"description": "Enter your Client ID and Client Secret from the Xthings app (My Account → OpenAPI). Submitting will start the U-Tec authorization. The Client Secret is not displayed; if you've used this integration before, please re-enter it.",
1414
"data": {
1515
"client_id": "Client ID",
1616
"client_secret": "Client Secret"

0 commit comments

Comments
 (0)