-
-
Notifications
You must be signed in to change notification settings - Fork 32.3k
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
Allow users to update their iCloud password when auth fails #39138
Conversation
bd88662
to
9395d87
Compare
9a34e63
to
89d16fa
Compare
I updated the logic to use |
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.
Did not check the tests
BTW @Quentame I had made a suggestion in #36120 that's a bit ugly but would resolve that issue. On setup, when there is 2FA, we can store the timestamp for when that occurred in the config entry and then setup a followup callback to trigger a fresh token grab in the future (let's say 1 month from the last refresh). |
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.
LGTM
test failure seems to be unrelated to this PR |
This comment has been minimized.
This comment has been minimized.
I am not used to see persistant notifications, but: |
@JulienFloris answered here #41315 |
I appreciate the work being done on this. When it's time to re-authenticate iCloud I have to remove and re-add the integration. It would be so much quicker if I could just enter the code to authenticate when the authentication expires. |
…ssal of persistent notification
Co-authored-by: Martin Hjelmare <marhje52@gmail.com>
I think we are good on this PR, going to go ahead and merge it so that I can work on #36120 as it appears to be affecting a lot of users |
Proposed change
I recently changed my iCloud password and noticed I had to delete and recreate the integration to update it. This change sets up a new discovery config flow when authentication fails that will update the password of the existing config entry. I saw multiple suggestions for how to handle this in home-assistant/architecture#377 but there hasn't been consensus yet, and I saw this pattern in other integrations such as
airvisual
andazure_devops
so I thought it was a safe approach. One thing I did differently here was to create a frontend notification for the user so that the user had more context of what to do.I tried to minimize changes to existing code, and verified that the exception that is being used to capture auth failing is always due to an incorrect email/password combo.
Type of change
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: