Skip to content

auth: fall back to trusted-device 2FA when Apple rejects SMS (#283) - #315

Open
Nick-800 wants to merge 2 commits into
xtool-org:mainfrom
Nick-800:fix/sms-2fa-fallback
Open

Nick-800 wants to merge 2 commits into
xtool-org:mainfrom
Nick-800:fix/sms-2fa-fallback

Conversation

@Nick-800

@Nick-800 Nick-800 commented Oct 11, 2026 •

Copy link
Copy Markdown

Summary

  • Parse Apple's <xmlui><alert> responses (previously swallowed by no-op decoders) and surface them as a typed GrandSlamTwoFactorAuthenticateOperation.Error.smsRejected.
  • When the SMS endpoint rejects (e.g. title="No Trusted Phone Numbers"), transparently fall back to the trusted-device 2FA flow instead of hanging on Code: for an SMS that will never arrive.

Why

Closes #283. Reporter captured the following response from POST https://gsa.apple.com/auth/verify/phone/put?mode=sms on an account that does have a trusted phone number on file:

<xmlui action="showModal">
  <alert title="No Trusted Phone Numbers"
         message="You do not have any trusted phone numbers associated with your Apple Account...">
    <button name="ak-button" ak-action="cancel"></button>
    <button httpMethod="GET" url="https://gsa.apple.com/iforgot/phone/add..."></button>
  </alert>
</xmlui>

Today both GrandSlamSMSAuthRequest.Decoder and GrandSlamValidateSMSRequest.Decoder are no-ops, so the alert is dropped on the floor. The operation then calls delegate.fetchCode() and the user waits forever. The reporter's workaround was to register an iPhone on the Apple ID so the trusted-device channel is used — this PR does that automatically.

Changes

  • Sources/XKit/GrandSlam/Requests/BuddyMLAlert.swift (new) — tiny XMLParser-based helper that extracts the first <alert title=… message=…> from a buddyML body. Foundation-only, works on macOS and Linux.
  • Sources/XKit/GrandSlam/Operations/GrandSlamTwoFactorAuthenticateOperation.swift — new smsRejected(title:message:) error case (with LocalizedError descriptions matching GrandSlamAuthenticateOperation.Error); performSMSAuth catches it and falls back to performTrustedDeviceAuth().
  • Sources/XKit/GrandSlam/Requests/Concrete/GrandSlamTrustedDeviceRequest.swift — wire the parser into GrandSlamSMSAuthRequest.Decoder.
  • Sources/XKit/GrandSlam/Requests/Concrete/GrandSlamValidateRequest.swift — same for GrandSlamValidateSMSRequest.Decoder.
  • Tests/XToolTests/BuddyMLAlertParserTests.swift (new) — swift-testing cases covering the issue 2FA code never comes when using xtool setup. #283 fixture, non-alert buddyML, empty data, malformed XML, and multi-alert payloads.

Non-goals

  • Doesn't attempt to list trusted phone numbers via GET /auth/verify/phone. That endpoint's response shape is undocumented and hard to exercise without a real failing Apple account — the fallback achieves the same user outcome without reverse-engineering it. The TODO at the top of performSMSAuth is clarified rather than removed.
  • Doesn't touch GrandSlamTrustedDeviceRequest.Decoder / GrandSlamSecondaryAuthRequest.Decoder — we don't have captured alert payloads from those endpoints and didn't want to change behavior speculatively.

Test plan

  • swift build — clean, no warnings.
  • swift test — 19 tests pass (5 new).
  • make lint — 0 violations in 139 files.
  • End-to-end verification against a real Apple account — this flow can't be exercised in CI. If a maintainer or @necoarcc can confirm xtool setup on an account that previously hit "No Trusted Phone Numbers" now falls back to the trusted-device code, that would be greatly appreciated.

Follows the same "surface the swallowed error" direction as #246.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Bug Fixes
    • Improved two-factor authentication error messages for incorrect verification codes and cancellations.
    • When SMS authentication is rejected, the app now displays the rejection details and falls back to trusted-device authentication.
    • Rejection alerts from the authentication service are now recognized during both SMS request and verification, helping show the relevant details and continue with an available authentication method.

…rg#283)

Parse the <xmlui><alert> body Apple returns from
`/auth/verify/phone/put?mode=sms` (previously discarded by an empty
decoder) and surface it as a typed `smsRejected` error. In
`performSMSAuth`, catch this error and transparently retry via the
trusted-device flow, logging a one-line notice.

Previously `xtool setup` hung forever on accounts that received the
"No Trusted Phone Numbers" alert, because the code silently prompted
for a verification code that Apple never sent.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Oct 11, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 871d36cf-465b-4232-9210-bf4d46b12cce
📥 Commits

Reviewing files that changed from the base of the PR and between aae2f6c and 1fdec3e.

📒 Files selected for processing: 1
  • Sources/XKit/GrandSlam/Operations/GrandSlamTwoFactorAuthenticateOperation.swift

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review.

📝 Walkthrough

SMS response decoders now detect BuddyML alerts and convert them to SMS-rejected errors. The two-factor operation provides localized error descriptions and falls back to trusted-device authentication when SMS authentication is rejected.

Changes: SMS authentication handling

Layer / File(s) Summary
Parse and decode SMS rejection alerts
Sources/XKit/GrandSlam/Requests/BuddyMLAlert.swift, Sources/XKit/GrandSlam/Requests/Concrete/GrandSlamTrustedDeviceRequest.swift, Sources/XKit/GrandSlam/Requests/Concrete/GrandSlamValidateRequest.swift, Tests/XToolTests/BuddyMLAlertParserTests.swift
The parser captures the first BuddyML alert and returns nil when input is empty, malformed, or has no alert. The request decoders throw an SMS-rejected error when an alert is present. Tests cover alert parsing and selection.
Handle SMS rejection in two-factor authentication
Sources/XKit/GrandSlam/Operations/GrandSlamTwoFactorAuthenticateOperation.swift
The operation supplies localized descriptions for authentication errors. It catches SMS rejection, writes a fallback message to standard error, and runs trusted-device authentication. Other errors are rethrown.

Priority: ➖ Normal

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Bug fix · Severity of issue fixed: Medium

Sequence Diagram(s)

sequenceDiagram
  participant SMSDecoder as GrandSlamSMSAuthRequest.Decoder
  participant AlertParser as BuddyMLAlertParser
  participant TwoFactorOperation as GrandSlamTwoFactorAuthenticateOperation
  participant TrustedDeviceAuth as Trusted-device authentication
  SMSDecoder->>AlertParser: Parse SMS response data
  AlertParser-->>SMSDecoder: Return alert when present
  SMSDecoder-->>TwoFactorOperation: Throw smsRejected with alert title and message
  TwoFactorOperation->>TrustedDeviceAuth: Run authentication after SMS rejection
Loading

Suggested reviewers: rahul05ranjan

Merge Risk: ⚪ Minimal · up to 1fdec

The change falls back to trusted-device authentication when SMS is rejected. Available evidence does not establish a reachable validation failure that is mislabeled, so no concrete issue requiring resolution before merge remains.

Pre-merge checks: Passed 4
Passed Passed checks: 4 passed
Check name Status Explanation
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title clearly and concisely describes the main change: fallback to trusted-device 2FA when Apple rejects SMS authentication.
Linked Issues check Passed Issue #283 reports that xtool setup prompts for an SMS code that never arrives. The SMS decoder and validation decoder now parse BuddyML alerts and throw smsRejected. performSMSAuth() catches SM…
Out of Scope Changes check Passed The changed operation, BuddyML parser, SMS decoders, and parser tests directly support the Issue #283 failure and the trusted-device fallback. The validation error mapping and stderr notice support th…
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

@rahul05ranjan rahul05ranjan left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Traced this by reading the code (nothing executed — no Swift toolchain on my machine); the flow and the fallback placement both look right to me.

The subtle part is handled correctly: after falling back you return before validateCode(phoneNumberID:), so the SMS validate request never fires with a phone ID Apple just rejected. Since GrandSlamClient.send runs R.Decoder.decode(data:) unconditionally on the response body, throwing from the previously no-op SMS decoders is a workable channel for this, and the parser being conservative (nil on non-alert, malformed, or empty input) means the old behavior is preserved for normal responses.

Two things I'd want the author to consider:

  • GrandSlamValidateSMSRequest.Decoder now throws smsRejected for any alert body — and that decoder also runs during code validation, not just the initial send. There's no captured evidence of what the securitycode endpoint returns for a wrong code (the -21669 GrandSlamOperationError path in validateCode covers the plist-style errors), so if Apple signals a bad SMS code with a buddyML alert, the user gets the raw alert title as a fatal error instead of "Incorrect verification code." That's still an improvement over pre-PR behavior — the no-op decoder would have treated the failed validation as success and blown up downstream — but mapping alerts during validate to .incorrectVerificationCode might be closer to intent. A captured payload from the validate endpoint would settle it.
  • The print("SMS 2FA unavailable...") goes to stdout, the same stream as the Code: prompt from Console.prompt, so the message lands right where the user will type. XKit has precedent for prints (XADIProvider, LoggingMiddleware) so it's not inconsistent, but stderr is the conventional place for a status notice that isn't part of the prompt flow. Not a blocker.

The #if canImport(FoundationXML) guard is right for this repo's platform set — on Linux XMLParser only comes from FoundationXML, macOS has it in Foundation — and corelibs-foundation implements abortParsing(), so stopsAtFirstAlert should behave the same on all three platforms. That's the main cross-platform risk with XMLParser and it's handled.

Addresses the two soft suggestions from @rahul05ranjan on PR xtool-org#315:

- `validateCode(phoneNumberID:)` now catches `.smsRejected` thrown by
  `GrandSlamValidateSMSRequest.Decoder` and rethrows as
  `.incorrectVerificationCode`. Previously a buddyML alert returned by
  Apple's `/securitycode` endpoint would surface as a fatal alert title;
  during the validate step the overwhelmingly likely cause is a wrong
  code, so this matches runtime intent.
- The SMS-fallback notice in `performSMSAuth()` now writes to stderr via
  `FileHandle.standardError.write`, so it no longer interleaves with the
  stdout `Code:` prompt from `Console.prompt`.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@Nick-800

Copy link
Copy Markdown
Author

Thanks for the careful trace, @rahul05ranjan! Both points addressed in 1fdec3e:

  1. Validate-step mapping. validateCode(phoneNumberID:) now catches .smsRejected after the existing -21669 catch and rethrows as .incorrectVerificationCode. Agreed that without a captured payload from /securitycode it's a judgment call, but during validate "incorrect code" is the overwhelmingly likely cause and matches the existing error surface. The decoder stays strict (parses the alert for any downstream caller), the operation applies the semantic mapping.
  2. Stderr for the fallback notice. Switched the print("SMS 2FA unavailable…") in performSMSAuth() to FileHandle.standardError.write(…) so it no longer lands on the same stream as the subsequent Code: prompt.

Gates still clean (swift build, swift test 19/19, make lint 0 violations).

@Quhuy007

Copy link
Copy Markdown

I'm experiencing a very similar SMS 2FA authentication failure on xtool 1.22.2 under WSL.

Environment

  • Host: Windows with WSL (Linux x86_64)
  • Swift: 6.4.1
  • xtool: 1.22.2
  • Authentication: Apple ID password mode
  • Apple ID: Free account with one trusted phone number

Actual error

POST https://gsa.apple.com/auth/verify/phone/put?mode=sms
failed: HTTP 403

<xmlui>
  <alert message="Could not connect to iCloud"
         cancelButtonTitle="OK"/>
</xmlui>

Additional checks

  • I can sign in successfully at account.apple.com.
  • xtool auth status confirms I am not authenticated.
  • The WSL TLS trust issue has been resolved.
  • curl -I https://gsa.apple.com now returns HTTP 404 without certificate verification errors.
  • Retrying password authentication still produces the same HTTP 403 at the SMS 2FA endpoint.

Question

Would PR #315 handle an HTTP 403 response containing Could not connect to iCloud, or does it only handle SMS rejection messages such as No Trusted Phone Numbers?

Is there an officially supported workaround to use trusted-device 2FA instead of SMS authentication in this situation?

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2FA code never comes when using xtool setup.

3 participants