core icon indicating copy to clipboard operation
core copied to clipboard

Fix Login issue with Pycloud

Open PaulCavill opened this issue 1 year ago • 8 comments

Upgrade pyicloud version and update field descriptions to reflect that the main account password is required for login.

pyicloud has now been taken over by @timlaing, updated to support apple login flow.

Breaking change

Proposed change

Type of change

Add new requirment pyicloudng module to fix the login flow due to Apple updating login process.

  • [ ] Dependency upgrade
  • [x] Bugfix (non-breaking change which fixes an issue)
  • [ ] New integration (thank you!)
  • [ ] New feature (which adds functionality to an existing integration)
  • [ ] Deprecation (breaking change to happen in the future)
  • [ ] Breaking change (fix/feature causing existing functionality to break)
  • [ ] Code quality improvements to existing code or addition of tests

Additional information

  • This PR fixes or closes issue: fixes #
  • This PR is related to issue: #128830
  • Link to documentation pull request:

Checklist

  • [x] The code change is tested and works locally.
  • [ ] Local tests pass. Your PR cannot be merged unless tests pass
  • [x] There is no commented out code in this PR.
  • [ ] I have followed the development checklist
  • [ ] I have followed the perfect PR recommendations
  • [ ] The code has been formatted using Ruff (ruff format homeassistant tests)
  • [ ] Tests have been added to verify that the new code works.

If user exposed functionality or configuration variables are added/changed:

If the code communicates with devices, web services, or third-party tools:

  • [ ] The manifest file has all fields filled out correctly.
    Updated and included derived files by running: python3 -m script.hassfest.
  • [ ] New or updated dependencies have been added to requirements_all.txt.
    Updated by running python3 -m script.gen_requirements_all.
  • [ ] For the updated dependencies - a link to the changelog, or at minimum a diff between library versions is added to the PR description.

To help with the load of incoming pull requests:

PaulCavill avatar Oct 23 '24 22:10 PaulCavill

Please take a look at the requested changes, and use the Ready for review button when you are done, thanks :+1:

Learn more about our pull request process.

home-assistant[bot] avatar Oct 23 '24 22:10 home-assistant[bot]

Hey there @quentame, @nzapponi, mind taking a look at this pull request as it has been labeled with an integration (icloud) you are listed as a code owner for? Thanks!

Code owner commands

Code owners of icloud can trigger bot actions by commenting:

  • @home-assistant close Closes the pull request.
  • @home-assistant rename Awesome new title Renames the pull request.
  • @home-assistant reopen Reopen the pull request.
  • @home-assistant unassign icloud Removes the current integration label and assignees on the pull request, add the integration domain after the command.
  • @home-assistant add-label needs-more-information Add a label (needs-more-information, problem in dependency, problem in custom component) to the pull request.
  • @home-assistant remove-label needs-more-information Remove a label (needs-more-information, problem in dependency, problem in custom component) on the pull request.

home-assistant[bot] avatar Oct 23 '24 22:10 home-assistant[bot]


Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

coderabbitai[bot] avatar Oct 23 '24 22:10 coderabbitai[bot]

Update: New code ready for review, just pending a maintainer action.

https://github.com/picklepete/pyicloud/pull/459

PaulCavill avatar Nov 13 '24 10:11 PaulCavill

New maintainer request is underway. https://github.com/pypi/support/issues/5377

PaulCavill avatar Jan 04 '25 20:01 PaulCavill

Maybe we can wait another few weeks for the new maintainer to get access. https://github.com/pypi/support/issues/5377

PaulCavill avatar Jan 29 '25 09:01 PaulCavill

There hasn't been any activity on this pull request recently. This pull request has been automatically marked as stale because of that and will be closed if no further activity occurs within 7 days. If you are the author of this PR, please leave a comment if you want to keep it open. Also, please rebase your PR onto the latest dev branch to ensure that it's up to date with the latest changes. Thank you for your contribution!

github-actions[bot] avatar Jun 14 '25 15:06 github-actions[bot]

New Version of https://pypi.org/project/pyicloud released

PaulCavill avatar Jun 14 '25 22:06 PaulCavill

Plaese put the changelog in the PR description

joostlek avatar Jun 16 '25 21:06 joostlek

We're still in discussion, please don't put it ready for review until we resolved those

joostlek avatar Jun 17 '25 09:06 joostlek

@joostlek Would you like anymore information or changes

PaulCavill avatar Jun 24 '25 21:06 PaulCavill

Hoping this update can hit soon since iCloud is FUBAR'd otherwise lol

rcmaehl avatar Jul 06 '25 07:07 rcmaehl

https://github.com/home-assistant/core/pull/129059#discussion_r2151639953 https://github.com/home-assistant/core/pull/129059#discussion_r2150923504 https://github.com/home-assistant/core/pull/129059#discussion_r2151638276

I still haven't received any answers so far

joostlek avatar Jul 21 '25 21:07 joostlek

#129059 (comment) #129059 (comment) #129059 (comment)

I still haven't received any answers so far

I have answered all 3 of them. Issue was my side.

PaulCavill avatar Jul 21 '25 21:07 PaulCavill

@joostlek Submodule removed.

PaulCavill avatar Jul 24 '25 11:07 PaulCavill

Okay I got good news and bad news, the PR itself looks good now. Except the library depends on a version of fido2 that is not compatible with the cryptography version we use. I see that fido2 has a new version which seems to fix this. So I think @timlaing should look into if fido2 is required, and if so, make sure it uses a version that works.

Since this PR has been going on for too long I am also not aiming to let it linger around any longer, feel free to send me a message on Discord if you need help with this!

joostlek avatar Aug 17 '25 15:08 joostlek

Was this even tested at all? I wanted to see if I could help fix the deps issue, but I couldn't even use app passwords to login with pyicloud directly, and it's still an open issue on their project.

The "fix" for app passwords is from a commit to a 3rd project, icloud_photos_downloader, that apparently didn't make it in any branch, which differs slightly from their current code, and that project also has no apparent support for app passwords (there are multiple issues about this closed without fix).

My understanding is that pyicloud logins like a normal web browser, and you can't use app passwords on the web. The only way it may seem to work is if you somehow end up getting cookies using your main password + 2FA, then change the password, but once the cookies expire and auth is re-triggered it will fail just like before.

TL;DR, fix need to be tested on a clean slate (removing the icloud dir form HA) or with pyicloud directly.

dermoth avatar Aug 21 '25 13:08 dermoth

@joostlek pyicloud 2.0.2 has been released to fix the fido issue.

PaulCavill avatar Aug 21 '25 21:08 PaulCavill

Was this even tested at all? I wanted to see if I could help fix the deps issue, but I couldn't even use app passwords to login with pyicloud directly, and it's still an open issue on their project.

The "fix" for app passwords is from a commit to a 3rd project, icloud_photos_downloader, that apparently didn't make it in any branch, which differs slightly from their current code, and that project also has no apparent support for app passwords (there are multiple issues about this closed without fix).

My understanding is that pyicloud logins like a normal web browser, and you can't use app passwords on the web. The only way it may seem to work is if you somehow end up getting cookies using your main password + 2FA, then change the password, but once the cookies expire and auth is re-triggered it will fail just like before.

TL;DR, fix need to be tested on a clean slate (removing the icloud dir form HA) or with pyicloud directly.

Apple remove support for App Passwords on web login, so currently the only way to auth is use your main password and MFA each month.

I have been running this for about 3 months with no issues. other than forcing pyicloud to install in the docker image, this is now solve in 2.0.2.

PaulCavill avatar Aug 21 '25 21:08 PaulCavill

Aight, kicked of the CI and updated the PR description, let's see :)

joostlek avatar Sep 05 '25 05:09 joostlek

https://github.com/home-assistant/core/pull/129059#pullrequestreview-3180614169

MartinHjelmare avatar Sep 05 '25 06:09 MartinHjelmare

We may want to mention in the breaking change release note that the integration requires re-authentication every month.

MartinHjelmare avatar Sep 05 '25 10:09 MartinHjelmare

What's left pending on this?

mcg88 avatar Sep 12 '25 21:09 mcg88

@PaulCavill can you please update the breaking change section as requested by @MartinHjelmare, once done, the PR can be merged 👍

emontnemery avatar Sep 13 '25 11:09 emontnemery

Please press the ready for review button when ready to merge.

MartinHjelmare avatar Sep 13 '25 20:09 MartinHjelmare

@PaulCavill please stop merging the dev branch into your feature branch. It removes the CI result and we have to run the CI again. We need to see that CI passes before we can merge.

We only need to merge dev if there's a merge conflict or there's something specific we need from the dev branch.

MartinHjelmare avatar Sep 14 '25 06:09 MartinHjelmare