-
-
Notifications
You must be signed in to change notification settings - Fork 35.8k
Fix PG&E and Duquesne Light Company in Opower #149658
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’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
Can you elaborate a bit on why you would need to use a separate addon? |
|
Now pge.com requires some headers (nifmwh7vqx-a to nifmwh7vqx-z) whose values are computed in JavaScript so it's hard to do that in Python. The add-on runs a headless browser that takes care of the login flow and returns the opower access token that the Opower integration needs. That token is only valid for a short time (under 30 minutes) and we don't have the refresh token so every 12 hours that the Opower integration refreshes data it delegates the login flow to the add-on to extract a fresh access token for opower.com. |
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.
A few strings that need to be converted from title- to sentence-case.
Co-authored-by: Norbert Rittel <norbert@rittel.de>
|
Drafting it because I'm close to performing the PG&E login without the need of a separate add-on. |
|
I removed the dependency to the login service. This is ready for review. I hope it can be reviewed and merged before the August release. Thanks! |
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.
Pull Request Overview
This PR upgrades the Opower library from version 0.12.4 to 0.15.1 to fix authentication issues with Pacific Gas & Electric (PG&E) and Duquesne Light Company, and adds support for interactive multi-factor authentication (MFA) flow.
- Upgrades Opower library dependency to version 0.15.1
- Implements interactive MFA flow with support for code selection and submission
- Refactors config flow to use separate steps for utility selection, credentials, MFA options, and MFA code
Reviewed Changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
requirements_all.txt |
Updates Opower library version from 0.12.4 to 0.15.1 |
requirements_test_all.txt |
Updates test requirements for Opower library |
homeassistant/components/opower/manifest.json |
Updates integration requirements to use new Opower version |
homeassistant/components/opower/const.py |
Adds new constant for login data storage |
homeassistant/components/opower/coordinator.py |
Updates to handle MfaChallenge exception and pass login data |
homeassistant/components/opower/config_flow.py |
Major refactor to support multi-step MFA flow |
homeassistant/components/opower/strings.json |
Updates UI strings for new MFA flow steps |
tests/components/opower/test_config_flow.py |
Comprehensive test updates for new MFA flow functionality |
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.
Thanks, @tronikos 👍
../Frenck
Co-authored-by: Norbert Rittel <norbert@rittel.de>
| }, | ||
| "mfa_code": { | ||
| "title": "Enter security code", | ||
| "description": "A security code has been sent via your selected method. Please enter it below to complete login.", |
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.
Noticed this (post-merge) while doing the translations in Lokalise:
As the totp_secret string above talks about using an authenticator app I wonder if "… has been sent …" here is a wording that does fit for that use case?
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.
Currently only one utility uses this flow that can either email you or text you the security code. But you are right, the MFA could be done via an authenticator app. In fact I expect someone from the community to change the current utility that takes TOTP secret to work with a security code in which case this description here would be confusing. In addition, there are utilities that don't ask you how you want to receive the code and I've code it if mfa_options is empty to come directly to this step to enter the security code. So in that scenario the description here would also be confusing since they didn't select any method. I changed the string in #150247 to avoid these issues. Thanks!
| "mfa": { | ||
| "description": "The TOTP secret below is not one of the 6-digit time-based numeric codes. It is a string of around 16 characters containing the shared secret that enables your authenticator app to generate the correct time-based code at the appropriate time. See the documentation.", | ||
| "credentials": { | ||
| "title": "Enter Credentials", |
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 also overlooked the missing sentence-casing here.
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.
Breaking change
Proposed change
Bump Opower library
tronikos/opower@v0.12.4...v0.15.1
It includes fixes for the following utilities:
The changes in the integration are:
Type of change
Additional information
Checklist
ruff format 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.To help with the load of incoming pull requests: