Skip to content

Commit

Permalink
avoid parsing json from empty responses #837 (#847)
Browse files Browse the repository at this point in the history
  • Loading branch information
AndreyNikiforov authored May 27, 2024
1 parent 2bb14a0 commit d53533b
Show file tree
Hide file tree
Showing 6 changed files with 68 additions and 28 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
## Unreleased

- feature: added parameter `--keep-unicode-in-filenames` with default `false` for compatibility [#845](https://github.com/icloud-photos-downloader/icloud_photos_downloader/issues/845)
- fix: avoid parsing json from empty responses [#837](https://github.com/icloud-photos-downloader/icloud_photos_downloader/issues/837)

## 1.17.7 (2024-05-25)

Expand Down
26 changes: 13 additions & 13 deletions src/icloudpd/authentication.py
Original file line number Diff line number Diff line change
Expand Up @@ -44,17 +44,17 @@ def authenticate_(
if icloud.requires_2fa:
if raise_error_on_2sa:
raise TwoStepAuthRequiredError(
"Two-step/two-factor authentication is required"
"Two-factor authentication is required"
)
logger.info("Two-step/two-factor authentication is required (2fa)")
logger.info("Two-factor authentication is required (2fa)")
request_2fa(icloud, logger)

elif icloud.requires_2sa:
if raise_error_on_2sa:
raise TwoStepAuthRequiredError(
"Two-step/two-factor authentication is required"
"Two-step authentication is required"
)
logger.info("Two-step/two-factor authentication is required (2sa)")
logger.info("Two-step authentication is required (2sa)")
request_2sa(icloud, logger)

return icloud
Expand Down Expand Up @@ -85,12 +85,12 @@ def request_2sa(icloud: PyiCloudService, logger: logging.Logger) -> None:

device = devices[device_index]
if not icloud.send_verification_code(device):
logger.error("Failed to send two-factor authentication code")
logger.error("Failed to send two-step authentication code")
sys.exit(1)

code = click.prompt("Please enter two-factor authentication code")
code = click.prompt("Please enter two-step authentication code")
if not icloud.validate_verification_code(device, code):
logger.error("Failed to verify two-factor authentication code")
logger.error("Failed to verify two-step authentication code")
sys.exit(1)
logger.info(
"Great, you're all set up. The script can now be run without "
Expand Down Expand Up @@ -122,7 +122,7 @@ def request_2fa(icloud: PyiCloudService, logger: logging.Logger) -> None:
# pylint: enable-msg=consider-using-f-string

index_str = f"..{len(devices) - 1}" if len(devices) > 1 else ""
code = click.prompt(
code:int = click.prompt(
f"Please enter two-factor authentication code or device index (0{index_str}) to send SMS with a code",
type=click.IntRange(
0,
Expand All @@ -139,11 +139,11 @@ def request_2fa(icloud: PyiCloudService, logger: logging.Logger) -> None:
type=click.IntRange(
0,
999999))
if not icloud.validate_verification_code(device, code):
if not icloud.validate_verification_code(device, str(code)):
logger.error("Failed to verify two-factor authentication code")
sys.exit(1)
else:
if not icloud.validate_2fa_code(code):
if not icloud.validate_2fa_code(str(code)):
logger.error("Failed to verify two-factor authentication code")
sys.exit(1)
else:
Expand All @@ -152,13 +152,13 @@ def request_2fa(icloud: PyiCloudService, logger: logging.Logger) -> None:
type=click.IntRange(
0,
999999))
if not icloud.validate_2fa_code(code):
if not icloud.validate_2fa_code(str(code)):
logger.error("Failed to verify two-factor authentication code")
sys.exit(1)
logger.info(
"Great, you're all set up. The script can now be run without "
"user interaction until 2SA expires.\n"
"user interaction until 2FA expires.\n"
"You can set up email notifications for when "
"the two-step authentication expires.\n"
"the two-factor authentication expires.\n"
"(Use --help to view information about SMTP options.)"
)
2 changes: 1 addition & 1 deletion src/pyicloud_ipd/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ def request(self, method: str, url, **kwargs):
return response

try:
data = response.json()
data = response.json() if response.status_code != 204 else {}
except: # pylint: disable=bare-except
request_logger.warning("Failed to parse response with JSON mimetype")
return response
Expand Down
50 changes: 45 additions & 5 deletions tests/test_authentication.py
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ def test_2sa_required(self) -> None:
)

self.assertTrue(
"Two-step/two-factor authentication is required"
"Two-step authentication is required"
in str(context.exception)
)

Expand All @@ -93,7 +93,7 @@ def test_2fa_required(self) -> None:
)

self.assertTrue(
"Two-step/two-factor authentication is required"
"Two-factor authentication is required"
in str(context.exception)
)

Expand Down Expand Up @@ -145,7 +145,7 @@ def test_successful_token_validation(self) -> None:
self.assertIn("INFO Authentication completed successfully", self._caplog.text)
assert result.exit_code == 0

def test_password_prompt(self) -> None:
def test_password_prompt_2sa(self) -> None:
base_dir = os.path.join(self.fixtures_path, inspect.stack()[0][3])
cookie_dir = os.path.join(base_dir, "cookie")

Expand All @@ -170,17 +170,57 @@ def test_password_prompt(self) -> None:
)
self.assertIn("DEBUG Authenticating...", self._caplog.text)
self.assertIn(
"INFO Two-step/two-factor authentication is required",
"INFO Two-step authentication is required",
self._caplog.text,
)
self.assertIn(" 0: SMS to *******03", result.output)
self.assertIn("Please choose an option: [0]: 0", result.output)
self.assertIn(
"Please enter two-factor authentication code: 654321", result.output
"Please enter two-step authentication code: 654321", result.output
)
self.assertIn(
"INFO Great, you're all set up. The script can now be run without "
"user interaction until 2SA expires.",
self._caplog.text,
)
assert result.exit_code == 0

def test_password_prompt_2fa(self) -> None:
base_dir = os.path.join(self.fixtures_path, inspect.stack()[0][3])
cookie_dir = os.path.join(base_dir, "cookie")

for dir in [base_dir, cookie_dir]:
recreate_path(dir)

with vcr.use_cassette(os.path.join(self.vcr_path, "2fa_flow_valid_code.yml")):
runner = CliRunner(env={
"CLIENT_ID": "DE309E26-942E-11E8-92F5-14109FE0B321"
})
result = runner.invoke(
main,
[
"--username",
"[email protected]",
"--no-progress-bar",
"--cookie-directory",
cookie_dir,
"--auth-only",
],
input="password1\n654321\n",
)
self.assertIn("DEBUG Authenticating...", self._caplog.text)
self.assertIn(
"INFO Two-factor authentication is required",
self._caplog.text,
)
self.assertIn(" 0: SMS to *******03", result.output)
self.assertIn(
"Please enter two-factor authentication code or device index (0) to send SMS with a code: 654321", result.output
)
self.assertIn(
"INFO Great, you're all set up. The script can now be run without "
"user interaction until 2FA expires.",
self._caplog.text,
)
self.assertNotIn("Failed to parse response with JSON mimetype", self._caplog.text)
assert result.exit_code == 0
14 changes: 7 additions & 7 deletions tests/test_two_step_auth.py
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ def test_2sa_flow_invalid_code(self) -> None:
input="0\n901431\n",
)
self.assertIn(
"ERROR Failed to verify two-factor authentication code",
"ERROR Failed to verify two-step authentication code",
self._caplog.text,
)

Expand Down Expand Up @@ -83,13 +83,13 @@ def test_2sa_flow_valid_code(self) -> None:
)
self.assertIn("DEBUG Authenticating...", self._caplog.text)
self.assertIn(
"INFO Two-step/two-factor authentication is required",
"INFO Two-step authentication is required",
self._caplog.text,
)
self.assertIn(" 0: SMS to *******03", result.output)
self.assertIn("Please choose an option: [0]: 0", result.output)
self.assertIn(
"Please enter two-factor authentication code: 654321", result.output
"Please enter two-step authentication code: 654321", result.output
)
self.assertIn(
"INFO Great, you're all set up. The script can now be run without "
Expand Down Expand Up @@ -129,13 +129,13 @@ def test_2sa_flow_failed_send_code(self) -> None:
)
self.assertIn("DEBUG Authenticating...", self._caplog.text)
self.assertIn(
"INFO Two-step/two-factor authentication is required",
"INFO Two-step authentication is required",
self._caplog.text,
)
self.assertIn(" 0: SMS to *******03", result.output)
self.assertIn("Please choose an option: [0]: 0", result.output)
self.assertIn(
"ERROR Failed to send two-factor authentication code",
"ERROR Failed to send two-step authentication code",
self._caplog.text,
)
assert result.exit_code == 1
Expand Down Expand Up @@ -199,15 +199,15 @@ def test_2fa_flow_valid_code(self) -> None:
)
self.assertIn("DEBUG Authenticating...", self._caplog.text)
self.assertIn(
"INFO Two-step/two-factor authentication is required",
"INFO Two-factor authentication is required",
self._caplog.text,
)
self.assertIn(
"Please enter two-factor authentication code or device index (0) to send SMS with a code: 654321", result.output
)
self.assertIn(
"INFO Great, you're all set up. The script can now be run without "
"user interaction until 2SA expires.",
"user interaction until 2FA expires.",
self._caplog.text,
)
assert result.exit_code == 0
3 changes: 1 addition & 2 deletions tests/vcr_cassettes/2fa_flow_valid_code.yml
Original file line number Diff line number Diff line change
Expand Up @@ -185,7 +185,7 @@ interactions:
uri: https://idmsa.apple.com/appleauth/auth/verify/trusteddevice/securitycode
response:
body:
string: !!python/unicode '{}'
string: !!binary
headers:
Cache-Control:
- 'no-cache'
Expand Down Expand Up @@ -213,7 +213,6 @@ interactions:
X-Content-Type-Options: ['nosniff']
X-FRAME-OPTIONS: ['DENY']
X-XSS-Protection: ['1; mode=block']
content-length: ['203']
scnt: ['scnt-1234567890']
vary: ['accept-encoding']
status:
Expand Down

0 comments on commit d53533b

Please sign in to comment.