New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add Reauth flow to Wallbox integration #58743
Add Reauth flow to Wallbox integration #58743
Conversation
@bdraco I added the discussed reauth flow to the wallbox component in this PR. Maybe you have time for a quick review? Thnx |
Compensating for timedrift in my devcontainer, making a new commit with the right date/time. Requested changes were done in a previous commit.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good. One small comment about how to tell the user the reauth doesn't match
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please resolve conflicts. Thanks 馃憤
also change string for invalid serial
Test failure is unrelated. I'll restart the CI |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One small tweak, and this looks good to merge 馃憤
Co-authored-by: J. Nick Koston <nick@koston.org>
* Add Reauth flow to Wallbox integration * Review comments processed * Fixed tests * Added test for reauth invalid * Commit to compensate for timedrift, show changes Compensating for timedrift in my devcontainer, making a new commit with the right date/time. Requested changes were done in a previous commit. * remove reauth schema * Update homeassistant/components/wallbox/__init__.py Co-authored-by: J. Nick Koston <nick@koston.org> Co-authored-by: J. Nick Koston <nick@koston.org>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please address the comments in a new PR. Thanks!
@@ -47,14 +59,27 @@ async def async_step_user(self, user_input=None): | |||
errors = {} | |||
|
|||
try: | |||
info = await validate_input(self.hass, user_input) | |||
await self.async_set_unique_id(user_input["station"]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please only wrap the lines that can raise in the try... except block.
|
||
return self.async_show_form( | ||
step_id="user", data_schema=STEP_USER_DATA_SCHEMA, errors=errors | ||
step_id="user", | ||
data_schema=STEP_USER_DATA_SCHEMA, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Normally we should only ask the user for the required info to re-authenticate in the reauth flow. We shouldn't ask the user again for info that should stay the same like station id.
info = await validate_input(self.hass, user_input) | ||
return self.async_create_entry(title=info["title"], data=user_input) | ||
if user_input["station"] == self._reauth_entry.data[CONF_STATION]: | ||
self.hass.config_entries.async_update_entry( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should validate the new credentials in the reauth flow before updating the config entry data.
@@ -7,15 +7,23 @@ | |||
"username": "[%key:common::config_flow::data::username%]", | |||
"password": "[%key:common::config_flow::data::password%]" | |||
} | |||
}, | |||
"reauth_confirm": { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We're missing this step.
Proposed change
I'm adding a reauthentication flow to the wallbox component. It's triggered by an HTML 403 error (so that will usually be a password change). The flow is triggered by calling ConfigEntryAuthFailed from the authentication method of the Wallbox class.
Type of change
Additional information
Checklist
black --fast homeassistant tests
)If user exposed functionality or configuration variables are added/changed:
If the code communicates with devices, web services, or third-party tools:
Updated and included derived files by running:
python3 -m script.hassfest
.requirements_all.txt
.Updated by running
python3 -m script.gen_requirements_all
..coveragerc
.The integration reached or maintains the following Integration Quality Scale:
To help with the load of incoming pull requests: