Repository navigation
Conversation
…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>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📝 Walkthrough
Merge Risk: ⚪ Minimal · up to
|
rahul05ranjan
left a comment
There was a problem hiding this comment.
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.Decodernow throwssmsRejectedfor any alert body — and that decoder also runs during code validation, not just the initial send. There's no captured evidence of what thesecuritycodeendpoint returns for a wrong code (the -21669GrandSlamOperationErrorpath invalidateCodecovers 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.incorrectVerificationCodemight 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 theCode:prompt fromConsole.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>
|
Thanks for the careful trace, @rahul05ranjan! Both points addressed in
Gates still clean ( |
|
I'm experiencing a very similar SMS 2FA authentication failure on xtool 1.22.2 under WSL. Environment
Actual error Additional checks
Question Would PR #315 handle an HTTP 403 response containing Is there an officially supported workaround to use trusted-device 2FA instead of SMS authentication in this situation? |
Summary
<xmlui><alert>responses (previously swallowed by no-op decoders) and surface them as a typedGrandSlamTwoFactorAuthenticateOperation.Error.smsRejected.title="No Trusted Phone Numbers"), transparently fall back to the trusted-device 2FA flow instead of hanging onCode: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=smson an account that does have a trusted phone number on file:Today both
GrandSlamSMSAuthRequest.DecoderandGrandSlamValidateSMSRequest.Decoderare no-ops, so the alert is dropped on the floor. The operation then callsdelegate.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) — tinyXMLParser-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— newsmsRejected(title:message:)error case (withLocalizedErrordescriptions matchingGrandSlamAuthenticateOperation.Error);performSMSAuthcatches it and falls back toperformTrustedDeviceAuth().Sources/XKit/GrandSlam/Requests/Concrete/GrandSlamTrustedDeviceRequest.swift— wire the parser intoGrandSlamSMSAuthRequest.Decoder.Sources/XKit/GrandSlam/Requests/Concrete/GrandSlamValidateRequest.swift— same forGrandSlamValidateSMSRequest.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
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 ofperformSMSAuthis clarified rather than removed.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.xtool setupon 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