fix: trigger 2FA push notification after SRP authentication by TeroPihlaja · Pull Request #260 · timlaing/pyicloud

TeroPihlaja · 2026-05-26T07:17:52Z

Problem

After SRP /signin/complete raises PyiCloud2FARequiredException, pyicloud calls _get_mfa_auth_options() to retrieve auth options but never explicitly requests delivery of the verification code.

Apple does not automatically push a code to the user's trusted devices for API-based (non-browser) sessions. As a result, users waiting for the 6-digit code on their iPhone/Mac receive nothing — authentication stalls indefinitely.

This was observed with a real Apple ID password on an HSA2 account (the SRP path). The GET /appleauth/auth call fetches the challenge parameters, but without an explicit GET /verify/trusteddevice call, Apple's servers never trigger the device push.

Fix

Add _request_2fa_code() which:

GET /verify/trusteddevice — explicitly triggers Apple to push the 6-digit code to all trusted devices (iPhone, Mac, etc.)
PUT /verify/phone (SMS fallback) — requests an SMS code if a trusted phone number is present in _auth_data

Call it automatically from _srp_authentication() immediately after _get_mfa_auth_options() populates _auth_data.

Why not call the existing `request_2fa_code()`?

The public request_2fa_code() routes through the trusted-device bridge (WebSocket) or SMS. When neither is available (no bridge boot context, no SMS mode reported), it returns False and no code is delivered. The GET /verify/trusteddevice endpoint is the simpler, direct path to trigger the user-visible push notification independently of the bridge flow.

Tests

Three new tests in tests/test_base.py:

test_private_request_2fa_code_triggers_trusted_device_push — verifies the GET /verify/trusteddevice call is made
test_private_request_2fa_code_sends_sms_when_phone_available — verifies SMS fallback via PUT /verify/phone
test_srp_authentication_calls_request_2fa_code_when_2fa_required — integration: _srp_authentication() invokes _request_2fa_code() when Apple signals 2FA required

🤖 Generated with Claude Code

After SRP /signin/complete raises PyiCloud2FARequiredException, pyicloud retrieved auth options via GET /appleauth/auth but never explicitly requested delivery of the verification code. Apple does not auto-push for API-based (non-browser) sessions, so users received no code on their trusted devices. Add _request_2fa_code() which calls GET /verify/trusteddevice to trigger the push notification and falls back to PUT /verify/phone (SMS) when a trusted phone number is available. Call it automatically from _srp_authentication() after _get_mfa_auth_options() sets _auth_data. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

coderabbitai · 2026-05-26T07:18:05Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 959777e1-9901-416f-bc36-33d8de6f67e6

📥 Commits

Reviewing files that changed from the base of the PR and between 180e57c and 7d98db8.

📒 Files selected for processing (2)

pyicloud/base.py
tests/test_base.py

📝 Walkthrough

Summary by CodeRabbit

Release Notes

Bug Fixes
- Improved two-factor authentication during sign-in by adding automatic fallback mechanisms for code delivery.
Tests
- Added comprehensive unit tests covering two-factor authentication request and authentication flows.

Walkthrough

This PR adds explicit 2FA code delivery for SRP-based authentication. When a sign-in attempt requires 2FA, the code now calls a new _request_2fa_code() method that attempts a trusted-device push and falls back to SMS before constructing MFA authentication options.

Changes

2FA Code Delivery for SRP Authentication

Layer / File(s)	Summary
2FA code request implementation and verification `pyicloud/base.py`, `tests/test_base.py`	New `_request_2fa_code()` method requests 2FA delivery via trusted-device push first, then SMS fallback using the trusted phone number. Tests verify both delivery paths and error handling.
SRP authentication integration `pyicloud/base.py`, `tests/test_base.py`	When `PyiCloud2FARequiredException` occurs during SRP sign-in completion, `_request_2fa_code()` is called before populating MFA request options. Integration test confirms the method is invoked in the SRP auth flow.

Sequence Diagram

sequenceDiagram
  participant SRPAuth as SRP Auth Handler
  participant RequestMethod as _request_2fa_code()
  participant TrustedDeviceAPI as Trusted-Device Push
  participant SMSAPI as SMS Verify Phone
  RequestMethod->>TrustedDeviceAPI: GET /verify/trusteddevice<br/>(with Accept: application/json)
  alt Push succeeds
    TrustedDeviceAPI-->>RequestMethod: Push sent
  else Push fails
    TrustedDeviceAPI-->>RequestMethod: Request error
    RequestMethod->>SMSAPI: PUT /verify/phone<br/>(phoneId + mode: sms)
    SMSAPI-->>RequestMethod: SMS sent or error logged
  end

Estimated Code Review Effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly Related PRs

timlaing/pyicloud#233: Both PRs change the trusted-device 2FA delivery flow: the main PR adds _request_2fa_code() to trigger trusted-device pushes (with SMS fallback), whilst #233 updates hsa2_bridge.py parsing/bootstrapping to handle Apple's changed trusted-device bridge payload fields (flowid/sessionUUID).

Suggested Labels

bugfix

Poem

🐰 Two-factor flows now flow with grace,
When SRP needs a second face,
We push first, then SMS we send,
On which delivery we depend!

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The pull request title clearly and specifically describes the main change: adding explicit 2FA push notification triggering after SRP authentication.
Description check	✅ Passed	The pull request description is comprehensive and directly related to the changeset, explaining the problem, the fix, and the rationale for the implementation.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 Pylint (4.0.5)

pyicloud/base.py

Traceback (most recent call last):
File "/usr/local/bin/pylint", line 8, in
sys.exit(run_pylint())
^^^^^^^^^^^^
File "/usr/local/lib/python3.11/dist-packages/pylint/init.py", line 34, in run_pylint
PylintRun(argv or sys.argv[1:])
File "/usr/local/lib/python3.11/dist-packages/pylint/lint/run.py", line 239, in init
linter.check(args)
File "/usr/local/lib/python3.11/dist-packages/pylint/lint/pylinter.py", line 698, in check
check_parallel(
File "/usr/local/lib/python3.11/dist-packages/pylint/lint/parallel.py", line 141, in check_parallel
with ProcessPoolExecutor(
^^^^^^^^^^^^^^^^^^^^
File "/usr/lib/python3.11/concurrent/futures/process.py", line 706, in init
self._call_queue = _SafeQueue(
^^^^^^^^^^^
File "/usr/lib/python3.11/concurrent/futures/process.py", line 168, in init
super().init(max_size, ctx=ctx)
File "/usr/lib/python3.11/multiprocessing/queues.py", line 43, in init
self._rlock = ctx.Lock()
^^^^^^^^^^
File "/usr/lib/python3.11/multiprocessing/context.py", line 68, in Lock
return Lock(ctx=self.get_context())
^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/usr/lib/python3.11/multiprocessing/synchronize.py", line 162, in init
SemLock.init(self, SEMAPHORE, 1, 1, ctx=ctx)
File "/usr/lib/python3.11/multiprocessing/synchronize.py", line 57, in init
sl = self._semlock = _multiprocessing.SemLock(
^^^^^^^^^^^^^^^^^^^^^^^^^
FileNotFoundError: [Errno 2] No such file or directory

tests/test_base.py

Traceback (most recent call last):
File "/usr/local/bin/pylint", line 8, in
sys.exit(run_pylint())
^^^^^^^^^^^^
File "/usr/local/lib/python3.11/dist-packages/pylint/init.py", line 34, in run_pylint
PylintRun(argv or sys.argv[1:])
File "/usr/local/lib/python3.11/dist-packages/pylint/lint/run.py", line 239, in init
linter.check(args)
File "/usr/local/lib/python3.11/dist-packages/pylint/lint/pylinter.py", line 698, in check
check_parallel(
File "/usr/local/lib/python3.11/dist-packages/pylint/lint/parallel.py", line 141, in check_parallel
with ProcessPoolExecutor(
^^^^^^^^^^^^^^^^^^^^
File "/usr/lib/python3.11/concurrent/futures/process.py", line 706, in init
self._call_queue = _SafeQueue(
^^^^^^^^^^^
File "/usr/lib/python3.11/concurrent/futures/process.py", line 168, in init
super().init(max_size, ctx=ctx)
File "/usr/lib/python3.11/multiprocessing/queues.py", line 43, in init
self._rlock = ctx.Lock()
^^^^^^^^^^
File "/usr/lib/python3.11/multiprocessing/context.py", line 68, in Lock
return Lock(ctx=self.get_context())
^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/usr/lib/python3.11/multiprocessing/synchronize.py", line 162, in init
SemLock.init(self, SEMAPHORE, 1, 1, ctx=ctx)
File "/usr/lib/python3.11/multiprocessing/synchronize.py", line 57, in init
sl = self._semlock = _multiprocessing.SemLock(
^^^^^^^^^^^^^^^^^^^^^^^^^
FileNotFoundError: [Errno 2] No such file or directory

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

timlaing · 2026-05-26T20:43:23Z

@TeroPihlaja many thanks for your support and contribution

coderabbitai Bot added the bugfix label May 26, 2026

coderabbitai Bot approved these changes May 26, 2026

View reviewed changes

timlaing approved these changes May 26, 2026

View reviewed changes

timlaing merged commit 65d544d into timlaing:main May 26, 2026
12 of 13 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix: trigger 2FA push notification after SRP authentication#260

fix: trigger 2FA push notification after SRP authentication#260
timlaing merged 1 commit into
timlaing:mainfrom
TeroPihlaja:fix/explicit-2fa-push-trigger

TeroPihlaja commented May 26, 2026

Uh oh!

coderabbitai Bot commented May 26, 2026 •

edited

Loading

Summary by CodeRabbit

Release Notes

Walkthrough

Changes

Sequence Diagram

Estimated Code Review Effort

Possibly Related PRs

Suggested Labels

Poem

Uh oh!

timlaing commented May 26, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

TeroPihlaja commented May 26, 2026

Problem

Fix

Why not call the existing request_2fa_code()?

Tests

Uh oh!

coderabbitai Bot commented May 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by CodeRabbit

Release Notes

Walkthrough

Changes

Sequence Diagram

Estimated Code Review Effort

Possibly Related PRs

Suggested Labels

Poem

Uh oh!

timlaing commented May 26, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Why not call the existing `request_2fa_code()`?

coderabbitai Bot commented May 26, 2026 •

edited

Loading