Closed AndreyNikiforov closed 1 month ago
I suspect something is changing on the Apple side. Similar issue reported in downstream project
Hello, exactly the same issue here and and I didn't change something for a while, I also suspect a change on Apple side :/
same issue for me, tested on mac and docker/linux
Joining the club! Docker instances with 3 accounts behaving the same.
same error in the library that was forked into icloudpd
same issue in other tools using Apple auth
I tried to compare requests icloudpd
makes to the ones happening in a browser accessing icloud.com and they are diverged over the years. Unless band-aid is found, the whole auth flow may need to be redone in icloudpd
to match new web auth flow used by icloud.com. New flow seems to be using SRP-6a/GSA, not just different urls. On a bright side, it most likely will be easy to support accounts without enabled MFA.
same issue for me, on linux
C:\Users\5257\Desktop\PyCharm\pythonProject.venv\Scripts\python.exe C:\Users\5257\Desktop\PyCharm\pythonProject\2.0000000000\设备名称.py 登录失败: ('Invalid email/password combination.', ICloudPyAPIResponseException('Service Temporarily Unavailable (503)'))
Same here. :(
Same error here for a few days now. It says 'Invalid email/password combination' but I can log into apple ID / account in my web browser with the same credentials.
Looks like already authenticated accounts (with active sessions) continue to work fine, any way to work around that by creating a session via the website and using that to authenticate instead?
I tried by logging into iCloud and opening Photos. Unfortunately, the same error occurred
See the same issue when trying to re-authenticate accounts. Ones where the cookie hasn't expired yet continue to work without issue
In another repo they are talking about the same, and think they have solved it! https://github.com/fastlane/fastlane/issues/26368
I have managed to fix the authentication by implementing the new SRP flow. See commit https://github.com/icloud-photos-downloader/icloud_photos_downloader/commit/4bcb2ac46a585205cbf3886b3df78179b34b18b1
Still the tests should be fixed and the code should be better organized.
Just want to share it as a reference if anyone else is working on this issue.
btw srp===1.0.21
should be added to dependencies
sorry @iowk, but how can i test that? i tried installing with pip and docker, and cant find the base.py to replace with that modified file, in order to test it. Ty!
sorry @iowk, but how can i test that? i tried installing with pip and docker, and cant find the base.py to replace with that modified file, in order to test it. Ty!
I think you can just replace src/pyicloud_ipd/base.py
and run the install process.
Also you'll have to add "srp===1.0.21",
to the dependencies list in pyproject.yml
i'm sorry, i never compiled a python exec from source, so that's the thing, how is the install process for Docker for example?
Update: already got the compiled python executable, not the docker image as i get that some executable is missing and i have no clue why, will update when i test it, ty for the effort and fast solution.
flawlessly working, thank you @iowk, hope someone already merges a PR on the master branch. Ty!
I have managed to fix the authentication by implementing the new SRP flow. See commit 4bcb2ac
Still the tests should be fixed and the code should be better organized.
Just want to share it as a reference if anyone else is working on this issue.
btw
srp===1.0.21
should be added to dependencies
Awesome!
@iowk Will you have time to adjust tests accordingly and submit PR? That would be very much appreciated by all the folks using icloudpd
and downstream projects. TIA
reported
Sure, I am glad to work on this. It might take a while to adjust tests since I am not very familiar with the framework.
@AndreyNikiforov To fix the tests, I think all the .yml files should be adjusted since the password is no longer directly sent to the server.
I tried to fix one of them here https://github.com/icloud-photos-downloader/icloud_photos_downloader/commit/f830709ebc17ea8fce4bd2569cb4f81882e896e6. I do not include the keys in the requests since they are randomly generated, and it might take quite some effort to mock the SRP client and server.
Could you please help me check if I am on the right track?
If so, I will continue to adjust the other .yml files correspondingly. Thank you.
@AndreyNikiforov To fix the tests, I think all the .yml files should be adjusted since the password is no longer directly sent to the server.
I tried to fix one of them here f830709. I do not include the keys in the requests since they are randomly generated, and it might take quite some effort to mock the SRP client and server.
Could you please help me check if I am on the right track?
If so, I will continue to adjust the other .yml files correspondingly. Thank you.
@iowk thanks for taking a look at fixing tests. A number of thoughts:
Questions:
- behavior tests are using auth as well, so they need to be adjusted - that is primary concern, right?
Yes
- is it possible/reasonable to use saved cookies for auth for all behavior tests? we will "skip" auth flow completely in those tests, right?
Edit: I update my thoughts in the next comment
I think it makes much sense to use saved cookies for auth. We will probably have to run the srp flow first to generate cookies, and run the behavior test itself. Something like:
def run_srp_auth(yml_file: str) -> int:
with vcr.use_cassette(os.path.join(self.vcr_path, yml_file)):
runner = CliRunner(env={"CLIENT_ID": "DE309E26-942E-11E8-92F5-14109FE0B321"})
result = runner.invoke(
main,
[
"--username",
"jdoe@gmail.com",
"--password",
"password1",
"--no-progress-bar",
"--cookie-directory",
cookie_dir,
"--auth-only",
],
)
return result.exit_code
)
def test_listing_albums(self) -> None:
base_dir = os.path.join(self.fixtures_path, inspect.stack()[0][3])
cookie_dir = os.path.join(base_dir, "cookie")
data_dir = os.path.join(base_dir, "data")
for dir in [base_dir, cookie_dir, data_dir]:
recreate_path(dir)
assert run_auth("srp_auth_flow.yml") == 0
with vcr.use_cassette(os.path.join(self.vcr_path, "listing_albums.yml")):
# Pass fixed client ID via environment variable
runner = CliRunner(env={"CLIENT_ID": "DE309E26-942E-11E8-92F5-14109FE0B321"})
result = runner.invoke(
main,
[
"--username",
"jdoe@gmail.com",
"--list-albums",
"--no-progress-bar",
"--cookie-directory",
cookie_dir,
],
)
...
Another way is to prepare dummy cookie files, like test/data/jdoegmailcom
and test/data/jdoegmailcom.session
, and copy them to cookie_dir
before the behavior tests.
Edit: Even with saved cookies, we still need to mock the response of _validate_token()
. That is, copy successful_auth.yml
to other .yml fils.
- what auth tests are still valuable if we continue to use vcr and mock srp? we will not be testing srp itself, right?
I think all the auth tests are still valuable since they focus on the flow after srp (2fa, 2sa, etc.).
Regarding testing srp itself, we can actually provide a fixed initial key a
to the srp user so that it's not random. Then get b
and salt
from the mock server, and test if the calculted session keys m1
, m2
are the same as our precalculated values.
Nontheless, I don't think there's too much worth of doing it since I will be precalculating m1
, m2
using the current code. It seems meaningless to test the code with what the code calculated.
- is it possible/reasonable to use saved cookies for auth for all behavior tests? we will "skip" auth flow completely in those tests, right?
After some more investigation, I think the better way is to mock the cookies directly for behavior tests. I create a new function _load_session_data
and mock it for tests. The same goes for _validate_token
.
This way, we can remove auth-related iteractions from behavior tests' cassettes by loading cookies through mocked functions.
I have fixed some tests here https://github.com/icloud-photos-downloader/icloud_photos_downloader/commit/d8439551f9c564c4efe1e594639144a3f54c087b. I would like to know your opinions. Thanks!
After some more investigation, I think the better way is to mock the cookies directly for behavior tests. I create a new function
_load_session_data
and mock it for tests. The same goes for_validate_token
.This way, we can remove auth-related iteractions from behavior tests' cassettes by loading cookies through mocked functions.
I have fixed some tests here d843955. I would like to know your opinions. Thanks!
If we are to use persisted credentials (cookies) for behavior tests, we most likely need another (small) change - perform session validation with hydrated credentials at the beginning of the auth flow. That way starting icloudpd
with non expired cookies will not ask for password and just work. I forgot about that when suggested auth loading approach. Should not be hard to implement, but may be tricky if the rest of auth is broken.
If we have saved cookies, we may be able to initialize cookie file (==clone into cookie_dir of each test) and do not use mocks at all - easier to modify 100 of test. WDYT?
I don't know about _validate_token
call. I assumed if it worked with vcr then it should continue to work the same way (==no need to mock it).
If we are to use persisted credentials (cookies) for behavior tests, we most likely need another (small) change - perform session validation with hydrated credentials at the beginning of the auth flow. That way starting icloudpd with non expired cookies will not ask for password and just work.
Acquiring password is not a problem because we are already using -p
in behavior tests. We already perform session validation at the start of authenticate()
, that is, to run _validate_token()
If we have saved cookies, we may be able to initialize cookie file (==clone into cookie_dir of each test) and do not use mocks at all - easier to modify 100 of test. WDYT?
Yes, I agree it is easier.
I don't know about
_validate_token
call. I assumed if it worked with vcr then it should continue to work the same way (==no need to mock it).
It worked because we used to auth with password, so POST to auth/signin
and setup/ws/1/accountLogin
are present in all cassettes.
If we auth with cookie, _validate_token()
will be called, so POST to setup/ws/1/validate
should be added to all cassettes. It is also easy to do if you prefer not to use mocks.
@iowk I have changed all non-auth tests to use stored cookies in #971 , so you can focus on auth test only.
I still not 100% sure how we can do srp testing (what part to mock and what not). I hope it will clearer from your code.
Thanks for pushing resolution of this issue forward.
@AndreyNikiforov I think test_listing_albums.py
and test_listing_libraries.py
are still not using stored cookies. I can change it in my PR.
Regarding srp testing, the auth-related cassettes will replace signin
with signin/init
and signin/complete
.
In the cassettes, signin
requests used to include password, but I am not planning to include keys (a
, m1
, m2
) in signin/init
and signin/complete
requests since they are random.
The response of signin/complete
will be similar to what was in signin
.
I will submit a PR later today based on what you changed. Thanks.
@AndreyNikiforov I think
test_listing_albums.py
andtest_listing_libraries.py
are still not using stored cookies. I can change it in my PR.Regarding srp testing, the auth-related cassettes will replace
signin
withsignin/init
andsignin/complete
.In the cassettes,
signin
requests used to include password, but I am not planning to include keys (a
,m1
,m2
) insignin/init
andsignin/complete
requests since they are random.The response of
signin/complete
will be similar to what was insignin
.I will submit a PR later today based on what you changed. Thanks.
keep working on auth tests. ill fix what i missed. if you want to take on these missed fixes too, better to have them as separate pr imo
I submitted the PR with auth tests fixed.
All tests except test_listing_albums
, test_listing_library
and test_listing_library_error
should pass at the moment.
I submitted the PR with auth tests fixed.
Thanks! There are comments about exceptions that needs to be addressed
All tests except
test_listing_albums
,test_listing_library
andtest_listing_library_error
should pass at the moment.
Submitted #973 with fixes. Strangely, these tests were green on my orig PR #971 but that is for another digging.
Thanks everyone for their work! I probably don't understand quite what the status is — I tried to reinstall icloudpd using pip, reentered my credentials and it's throwing a bad username/password error (I'm sure they are correct).
Am I missing a step?
~$ /var/services/homes/ds-admin/icloudpd-venv/bin/icloud --username "xxx"
Running in MAIN
Enter iCloud password for xxx:
Bad username or password for xxx
Submitted #973 with fixes. Strangely, these tests were green on my orig PR #971 but that is for another digging.
Thanks, I just rebased my branch.
It makes sense that the tests pass on PR #971 because the auth flow is not changed to srp. The reason they fail on my branch is the new srp flow.
Thanks for all the effort in fixing this one guys
Thanks for all the work here! I just installed 0.1.24
and ran:
icloudpd --username '<email>' --password '<password>' --auth-only
but I'm getting the following error response:
pyicloud_ipd.exceptions.PyiCloudAPIResponseException: {
"serviceErrors" : [ {
"code" : "-20101",
"message" : "Enter the email or phone number and password for your Apple Account.",
"suppressDismissal" : false
} ]
} (401)
Is there something else I need to configure? I tried removing ~/.pyicloud/
as well.
"message" : "Enter the email or phone number and
@scttnlsn Pls open new issue, so it is easier for folks with the same issue to connect.
I tried new version with 2fa account (that was failing earlier) before sending 1.24.0 and it worked for downloading and --auth-only
, so it is confirmed that error in this post fixed at least for some accounts.
Opened a new issue here: https://github.com/icloud-photos-downloader/icloud_photos_downloader/issues/975
An observation for the behaviour of the fix in my case. I had to authenticate twice because the first attempt failed. I run icloudpd like this:
icloudpd \
--username $APPLE_EMAIL \
--directory $PHOTOS_DIR \
--auth-only
The first time I run it after updating for the 503 fix, I got the 2FA prompt, entered the code, and then icloudpd failed with:
Please enter two-factor authentication code or device index (a) to send SMS with a code: <removed>
2024-10-26 08:05:12 ERROR Bad Gateway (502)
Traceback (most recent call last):
File "starters/icloudpd.py", line 6, in <module>
File "click/core.py", line 1157, in __call__
File "click/core.py", line 1078, in main
File "click/core.py", line 1434, in invoke
File "click/core.py", line 783, in invoke
File "icloudpd/base.py", line 745, in main
File "icloudpd/base.py", line 1183, in core
File "icloudpd/authentication.py", line 83, in authenticate_
File "icloudpd/authentication.py", line 199, in request_2fa
File "pyicloud_ipd/base.py", line 516, in validate_2fa_code
File "requests/sessions.py", line 637, in post
File "pyicloud_ipd/session.py", line 122, in request
File "pyicloud_ipd/session.py", line 196, in _raise_error
pyicloud_ipd.exceptions.PyiCloudAPIResponseException: Bad Gateway (502)
[4949] Failed to execute script 'icloudpd' due to unhandled exception!
After a minute I tried to run it a second time (no reinstall or anything) and it worked like a charm and logged me in correctly.
I don't have any weird home network configuration, there is no proxy in the way or anything like that. What I did differently (although I'm doubtful it had anything to do with it) was that in the first case I approved it on iPhone and after typing the code, I didn't close the 2FA popup and kept it running, while for the second try, I confirmed 2FA on a Mac and closed the popup before hitting enter in terminal.
If no one else sees this behaviour, then it was just some transient and maybe unrelated issue.
And thank you for maintaining this project. 😄
Summary
One of my accounts fails authentication
Another account works fine