Handle 2FA-required errors separately from password-reset errors#169005
Open
TeroPihlaja wants to merge 11 commits intohome-assistant:devfrom
Open
Handle 2FA-required errors separately from password-reset errors#169005TeroPihlaja wants to merge 11 commits intohome-assistant:devfrom
TeroPihlaja wants to merge 11 commits intohome-assistant:devfrom
Conversation
…Cloud setup When iCloud login fails, distinguish between two scenarios: 1. **Password needs resetting** — OAuth fails, credentials are invalid. Log ERROR directing user to re-enter password. 2. **2FA verification code needed** — Login succeeded but FMIP service requires additional MFA (common when the Apple account has MFA enabled but no verification code was cached yet). Log WARNING with gentler language directing user to enter verification code via the UI configuration menu. Added handler for `PyiCloudAuthRequiredException` which is raised when the FMIP service specifically requires re-authentication even after the main iCloud login succeeded. Apply the same 2FA-aware logging here. This provides clearer, more actionable error messages and prevents users from resetting their password when they only need to verify a code.
Contributor
|
Hey there @Quentame, @nzapponi, mind taking a look at this pull request as it has been labeled with an integration ( Code owner commandsCode owners of
|
Contributor
There was a problem hiding this comment.
Pull request overview
Improves the iCloud integration’s setup error handling so 2FA-required states are distinguished from invalid-credential states, and prevents crashes when iCloud requires re-authentication after initial login.
Changes:
- Add handling for
PyiCloudAuthRequiredExceptionduring account setup. - Differentiate logging between “2FA required” vs “password invalid / re-login required” scenarios.
- Adjust the
PyiCloudFailedLoginExceptionpath to emit a 2FA-specific warning when applicable.
Author
|
Could a maintainer please add the |
pyicloud is a third-party package and must be imported before the homeassistant first-party imports. Also removes the unused PyiCloudFailedLoginException import. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
ruff isort places from-imports before bare imports alphabetically within the same section. pyicloud (p-y-i) sorts before pytest (p-y-t). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The constructor signature is __init__(self, response), not a message string. Pass None since the test only verifies the exception is caught, not the response contents. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The constructor is __init__(self, apple_id: str, response: Response). Pass both required arguments to instantiate correctly. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Catch PyiCloudAuthRequiredException also when accessing api.devices.user_info, not only during PyiCloudService construction — prevents an unhandled exception crash if FMIP re-auth is required after the initial login succeeded - Add test for this new code path - Fix inline comment on PyiCloudFailedLoginException handler to reflect that it can indicate 2FA requirement, not just invalid credentials Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…es handler - Extract shared PyiCloudAuthRequiredException logging/reauth logic into _handle_auth_required(requires_2fa) to prevent the two handlers diverging - Third handler (devices.user_info) now checks requires_2fa before logging, so it correctly logs warning vs error like the other handlers do - Add _LOGGER assertions to test_setup_auth_required_exception_from_devices to verify error is logged (not warning) when requires_2fa is False Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…gicMock type type(MagicMock()).user_info = PropertyMock(...) mutates the global MagicMock class and leaks into other tests. Replace with a scoped inner class whose user_info property raises PyiCloudAuthRequiredException directly. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The existing tests only exercised the error branch (requires_2fa=False). Testing the helper directly avoids the setup() complexity where requires_2fa=True in the first try block would trigger PyiCloudFailedLoginException instead. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Set self.api = None before the try block so exception handlers never see a stale API instance if PyiCloudService() raises mid-construction - Route the requires_2fa=True branch of PyiCloudFailedLoginException through _handle_auth_required(True), eliminating the duplicate 2FA warning message; only the unique bad-password error stays inline Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
When iCloud login fails, the integration treated all failures the same way — logging a generic "Your password is no longer working" error regardless of the actual cause. This was confusing for users who only needed to verify a 2FA code (which is a temporary config state) versus those with truly invalid credentials.
Additionally,
PyiCloudAuthRequiredExceptionwas not caught, causing the setup flow to crash if FMIP service required re-authentication even after the main login succeeded.Changes
account.py:PyiCloudAuthRequiredExceptionimportPyiCloudFailedLoginExceptionhandler to checkapi.requires_2fafirst:True: log WARNING with user-friendly message directing them to enter their verification codeFalse: log ERROR with message to reset password (original behavior)except PyiCloudAuthRequiredException:block with the same 2FA-aware logging logicResult: Users now see actionable, contextual error messages instead of generic password-reset prompts when they only need to verify a code.
Type of change
Checklist
python -m pytest tests/components/icloud/Co-authored with Claude Sonnet 4.6