Skip to content

Commit 3c5d5ed

Browse files
kinoroyclaude
andcommitted
Fix 2FA authentication regression in 2.x
In 1.6.x, Client.srpLogin drove the entire interactive two-factor flow internally and only returned once the session was fully authenticated. In 2.x, login moved to XcodesLoginKit and srpLogin now returns an AuthenticationState instead of completing the flow, expecting the caller to handle the second factor. The CLI's login closures discarded that state, so 2FA accounts were left with an unauthenticated (pre-2FA) cookie jar: no verification-code prompt appeared and downloads failed with 403 Unauthorized. Restore the interactive completion for the paths 1.6.x supported: trusted-device codes, SMS auto-sent, and SMS phone-number selection. Security-key and federated/not-developer states surface clear errors. The flow is modeled with injected closures (Dependencies) rather than a concrete Client to match the existing dependency-injection style and to keep it unit-testable. Adds coverage for each branch. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 parent 516b846 commit 3c5d5ed

3 files changed

Lines changed: 339 additions & 2 deletions

File tree

Sources/XcodesKit/Environment.swift

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -296,7 +296,8 @@ public struct Network: Sendable {
296296
downloadTask = { loginClient.urlSession.downloadTask(with: $0, to: $1, resumingWith: $2) }
297297
validateSession = { _ = try await loginClient.validateSession() }
298298
login = { accountName, password in
299-
_ = try await loginClient.srpLogin(accountName: accountName, password: password)
299+
let state = try await loginClient.srpLogin(accountName: accountName, password: password)
300+
try await TwoFactorAuthentication.completeIfNeeded(state, dependencies: .liveDependencies(client: loginClient))
300301
}
301302
checkIsFederated = { accountName in
302303
try await loginClient.checkIsFederated(accountName: accountName)
@@ -371,7 +372,8 @@ public struct Network: Sendable {
371372
self.downloadTask = downloadTask ?? { loginClient.urlSession.downloadTask(with: $0, to: $1, resumingWith: $2) }
372373
self.validateSession = validateSession ?? { _ = try await loginClient.validateSession() }
373374
self.login = login ?? { accountName, password in
374-
_ = try await loginClient.srpLogin(accountName: accountName, password: password)
375+
let state = try await loginClient.srpLogin(accountName: accountName, password: password)
376+
try await TwoFactorAuthentication.completeIfNeeded(state, dependencies: .liveDependencies(client: loginClient))
375377
}
376378
self.checkIsFederated = checkIsFederated ?? { accountName in
377379
try await loginClient.checkIsFederated(accountName: accountName)
Lines changed: 132 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,132 @@
1+
import Foundation
2+
import Rainbow
3+
import XcodesKit
4+
import XcodesLoginKit
5+
6+
/// Drives the interactive two-factor portion of an Apple sign-in from the command line.
7+
///
8+
/// `XcodesLoginKit.Client.srpLogin(accountName:password:)` no longer prompts for a verification code
9+
/// itself; instead it returns the next ``AuthenticationState``. UI clients (like the SwiftUI app) are
10+
/// expected to present their own second-factor screen and then call `submitSecurityCode` /
11+
/// `requestSMSSecurityCode`. Without an equivalent here, the CLI silently stopped at the pre-2FA session
12+
/// in 2.x, leaving the cookie jar unauthenticated and causing 403s on download. This restores the
13+
/// interactive flow that shipped in xcodes 1.6.x for trusted-device codes and SMS codes.
14+
enum TwoFactorAuthentication {
15+
/// The operations needed to complete a second-factor challenge.
16+
///
17+
/// Modeled as closures rather than a concrete `Client` so the flow can be exercised in tests and to
18+
/// match the dependency-injection style used by ``Environment``.
19+
struct Dependencies: Sendable {
20+
var submitSecurityCode: @Sendable (SecurityCode, AppleSessionData) async throws -> AuthenticationState
21+
var requestSMSSecurityCode: @Sendable (AuthOptionsResponse.TrustedPhoneNumber, AuthOptionsResponse, AppleSessionData) async throws -> AuthenticationState
22+
23+
/// Builds dependencies backed by a live login client.
24+
static func liveDependencies(client: XcodesLoginKit.Client) -> Dependencies {
25+
Dependencies(
26+
submitSecurityCode: { code, sessionData in
27+
try await client.submitSecurityCode(code, sessionData: sessionData)
28+
},
29+
requestSMSSecurityCode: { phoneNumber, authOptions, sessionData in
30+
try await client.requestSMSSecurityCode(to: phoneNumber, authOptions: authOptions, sessionData: sessionData)
31+
}
32+
)
33+
}
34+
}
35+
36+
/// Completes any outstanding second-factor challenge for a freshly attempted login.
37+
///
38+
/// - Parameters:
39+
/// - state: The state returned by `srpLogin` (or a subsequent step).
40+
/// - dependencies: The operations used to submit codes and request SMS messages.
41+
static func completeIfNeeded(_ state: AuthenticationState, dependencies: Dependencies) async throws {
42+
switch state {
43+
case .authenticated:
44+
return
45+
case let .waitingForSecondFactor(option, authOptions, sessionData):
46+
try await handleTwoFactor(option: option, authOptions: authOptions, sessionData: sessionData, dependencies: dependencies)
47+
case .waitingForFederatedAuthentication:
48+
// Federated accounts are detected and handled by AppleSessionService before srpLogin runs,
49+
// so reaching here means the federated flow wasn't completed.
50+
throw AuthenticationError.federatedAuthenticationRequired
51+
case .notAppleDeveloper:
52+
throw AuthenticationError.notDeveloperAppleId
53+
case .unauthenticated:
54+
throw AuthenticationError.notAuthorized
55+
}
56+
}
57+
58+
private static func handleTwoFactor(option: TwoFactorOption, authOptions: AuthOptionsResponse, sessionData: AppleSessionData, dependencies: Dependencies) async throws {
59+
Current.logging.log("Two-factor authentication is enabled for this account.\n")
60+
61+
switch option {
62+
// An SMS code was sent automatically to the account's single trusted phone number.
63+
case let .smsSent(phoneNumber):
64+
try await submitSMSCode(authOptions: authOptions, phoneNumber: phoneNumber, sessionData: sessionData, dependencies: dependencies)
65+
// No code was sent automatically; the user must pick a phone number first.
66+
case .smsPendingChoice:
67+
try await handleWithPhoneNumberSelection(authOptions: authOptions, sessionData: sessionData, dependencies: dependencies)
68+
// A code is shown on the account's trusted devices.
69+
case .codeSent:
70+
try await submitDeviceCode(authOptions: authOptions, sessionData: sessionData, dependencies: dependencies)
71+
// A physical security key is required, which the CLI has never supported (1.6.x threw here too).
72+
case .securityKey:
73+
throw XcodesKitError("This account requires a hardware security key for authentication, which xcodes does not support on the command line. Use the Xcodes app to sign in with a security key.")
74+
}
75+
}
76+
77+
/// Prompts for a trusted-device code, allowing the user to fall back to SMS by entering "sms".
78+
private static func submitDeviceCode(authOptions: AuthOptionsResponse, sessionData: AppleSessionData, dependencies: Dependencies) async throws {
79+
let securityCodeLength = authOptions.securityCode?.length ?? 0
80+
let code = Current.shell.readLine(prompt: """
81+
Enter "sms" without quotes to exit this prompt and choose a phone number to send an SMS security code to.
82+
Enter the \(securityCodeLength) digit code from one of your trusted devices:
83+
""") ?? ""
84+
85+
if code == "sms" {
86+
try await handleWithPhoneNumberSelection(authOptions: authOptions, sessionData: sessionData, dependencies: dependencies)
87+
return
88+
}
89+
90+
_ = try await dependencies.submitSecurityCode(.device(code: code), sessionData)
91+
}
92+
93+
/// Lists the trusted phone numbers, requests an SMS to the chosen one, then submits the code.
94+
private static func handleWithPhoneNumberSelection(authOptions: AuthOptionsResponse, sessionData: AppleSessionData, dependencies: Dependencies) async throws {
95+
// 2FA requires at least one trusted phone number, but inform the user rather than crashing if absent.
96+
guard let trustedPhoneNumbers = authOptions.trustedPhoneNumbers, trustedPhoneNumbers.isEmpty == false else {
97+
throw XcodesKitError("Your account doesn't have any trusted phone numbers, but they're required for two-factor authentication. See https://support.apple.com/en-ca/HT204915.")
98+
}
99+
100+
let phoneNumber = selectPhoneNumberInteractively(from: trustedPhoneNumbers)
101+
_ = try await dependencies.requestSMSSecurityCode(phoneNumber, authOptions, sessionData)
102+
try await submitSMSCode(authOptions: authOptions, phoneNumber: phoneNumber, sessionData: sessionData, dependencies: dependencies)
103+
}
104+
105+
private static func submitSMSCode(authOptions: AuthOptionsResponse, phoneNumber: AuthOptionsResponse.TrustedPhoneNumber, sessionData: AppleSessionData, dependencies: Dependencies) async throws {
106+
guard let length = authOptions.securityCode?.length else {
107+
throw XcodesKitError("Expected security code info but didn't receive any.")
108+
}
109+
110+
let code = Current.shell.readLine(prompt: "Enter the \(length) digit code sent to \(phoneNumber.numberWithDialCode): ") ?? ""
111+
_ = try await dependencies.submitSecurityCode(.sms(code: code, phoneNumberId: phoneNumber.id), sessionData)
112+
}
113+
114+
private static func selectPhoneNumberInteractively(from trustedPhoneNumbers: [AuthOptionsResponse.TrustedPhoneNumber]) -> AuthOptionsResponse.TrustedPhoneNumber {
115+
Current.logging.log("Trusted phone numbers:")
116+
for (index, phoneNumber) in trustedPhoneNumbers.enumerated() {
117+
Current.logging.log("\(index + 1): \(phoneNumber.numberWithDialCode)")
118+
}
119+
120+
let possibleSelection = Current.shell.readLine(prompt: "Select a trusted phone number to receive a code via SMS: ")
121+
guard
122+
let possibleSelection,
123+
let selection = Int(possibleSelection),
124+
trustedPhoneNumbers.indices.contains(selection - 1)
125+
else {
126+
Current.logging.log("Not a valid phone number index. Expecting a whole number between 1-\(trustedPhoneNumbers.count), but was given \(possibleSelection ?? "nothing").\n".red)
127+
return selectPhoneNumberInteractively(from: trustedPhoneNumbers)
128+
}
129+
130+
return trustedPhoneNumbers[selection - 1]
131+
}
132+
}
Lines changed: 203 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,203 @@
1+
@testable import XcodesCLIKit
2+
import Foundation
3+
import XcodesLoginKit
4+
import XCTest
5+
6+
/// `AppleSession` only exposes a `Decodable` initializer, so build the authenticated state from JSON.
7+
private let authenticatedState: AuthenticationState = {
8+
let json = Data(#"{"user":{"fullName":"Test User"}}"#.utf8)
9+
let session = try! JSONDecoder().decode(AppleSession.self, from: json)
10+
return .authenticated(session)
11+
}()
12+
13+
final class TwoFactorAuthenticationTests: XCTestCase {
14+
override func setUp() {
15+
super.setUp()
16+
Current = .mock
17+
}
18+
19+
private func authOptions(
20+
trustedPhoneNumbers: [AuthOptionsResponse.TrustedPhoneNumber]? = nil,
21+
codeLength: Int = 6
22+
) -> AuthOptionsResponse {
23+
AuthOptionsResponse(
24+
trustedPhoneNumbers: trustedPhoneNumbers,
25+
trustedDevices: nil,
26+
securityCode: .init(length: codeLength)
27+
)
28+
}
29+
30+
private let sessionData = AppleSessionData(serviceKey: "service", sessionID: "session", scnt: "scnt")
31+
32+
// MARK: Already authenticated
33+
34+
func test_CompleteIfNeeded_AlreadyAuthenticated_DoesNothing() async throws {
35+
let submitCalled = LockedBox(false)
36+
let dependencies = TwoFactorAuthentication.Dependencies(
37+
submitSecurityCode: { _, _ in submitCalled.set(true); return authenticatedState },
38+
requestSMSSecurityCode: { _, _, _ in authenticatedState }
39+
)
40+
41+
try await TwoFactorAuthentication.completeIfNeeded(authenticatedState, dependencies: dependencies)
42+
43+
XCTAssertFalse(submitCalled.value)
44+
}
45+
46+
// MARK: Trusted device code
47+
48+
func test_CompleteIfNeeded_TrustedDeviceCode_SubmitsEnteredCode() async throws {
49+
Current.shell.readLine = { _ in "123456" }
50+
51+
let submittedCode = LockedBox<SecurityCode?>(nil)
52+
let dependencies = TwoFactorAuthentication.Dependencies(
53+
submitSecurityCode: { code, _ in submittedCode.set(code); return authenticatedState },
54+
requestSMSSecurityCode: { _, _, _ in XCTFail("Should not request SMS"); return authenticatedState }
55+
)
56+
57+
let state = AuthenticationState.waitingForSecondFactor(.codeSent, authOptions(), sessionData)
58+
try await TwoFactorAuthentication.completeIfNeeded(state, dependencies: dependencies)
59+
60+
guard case let .device(code) = submittedCode.value else {
61+
return XCTFail("Expected a trusted-device code, got \(String(describing: submittedCode.value))")
62+
}
63+
XCTAssertEqual(code, "123456")
64+
}
65+
66+
func test_CompleteIfNeeded_TrustedDeviceCode_EnteringSMS_FallsBackToPhoneSelection() async throws {
67+
// First prompt (device code) -> "sms"; second prompt (phone selection) -> "1"; third prompt (SMS code) -> "654321".
68+
let scripted = ["sms", "1", "654321"]
69+
let index = LockedBox(0)
70+
Current.shell.readLine = { _ in
71+
let i = index.incrementAfterRead()
72+
return i < scripted.count ? scripted[i] : nil
73+
}
74+
75+
let smsRequested = LockedBox(false)
76+
let submittedCode = LockedBox<SecurityCode?>(nil)
77+
let phoneNumber = AuthOptionsResponse.TrustedPhoneNumber(id: 7, numberWithDialCode: "+1 (•••) •••-1234")
78+
let dependencies = TwoFactorAuthentication.Dependencies(
79+
submitSecurityCode: { code, _ in submittedCode.set(code); return authenticatedState },
80+
requestSMSSecurityCode: { _, _, _ in smsRequested.set(true); return authenticatedState }
81+
)
82+
83+
let state = AuthenticationState.waitingForSecondFactor(.codeSent, authOptions(trustedPhoneNumbers: [phoneNumber]), sessionData)
84+
try await TwoFactorAuthentication.completeIfNeeded(state, dependencies: dependencies)
85+
86+
XCTAssertTrue(smsRequested.value)
87+
guard case let .sms(code, phoneNumberId) = submittedCode.value else {
88+
return XCTFail("Expected an SMS code, got \(String(describing: submittedCode.value))")
89+
}
90+
XCTAssertEqual(code, "654321")
91+
XCTAssertEqual(phoneNumberId, 7)
92+
}
93+
94+
// MARK: SMS automatically sent
95+
96+
func test_CompleteIfNeeded_SMSSent_SubmitsCodeForThatNumber() async throws {
97+
Current.shell.readLine = { _ in "987654" }
98+
99+
let phoneNumber = AuthOptionsResponse.TrustedPhoneNumber(id: 3, numberWithDialCode: "+1 (•••) •••-9999")
100+
let submittedCode = LockedBox<SecurityCode?>(nil)
101+
let dependencies = TwoFactorAuthentication.Dependencies(
102+
submitSecurityCode: { code, _ in submittedCode.set(code); return authenticatedState },
103+
requestSMSSecurityCode: { _, _, _ in XCTFail("SMS already sent automatically"); return authenticatedState }
104+
)
105+
106+
let state = AuthenticationState.waitingForSecondFactor(.smsSent(phoneNumber), authOptions(trustedPhoneNumbers: [phoneNumber]), sessionData)
107+
try await TwoFactorAuthentication.completeIfNeeded(state, dependencies: dependencies)
108+
109+
guard case let .sms(code, phoneNumberId) = submittedCode.value else {
110+
return XCTFail("Expected an SMS code, got \(String(describing: submittedCode.value))")
111+
}
112+
XCTAssertEqual(code, "987654")
113+
XCTAssertEqual(phoneNumberId, 3)
114+
}
115+
116+
// MARK: SMS phone number selection
117+
118+
func test_CompleteIfNeeded_SMSPendingChoice_RequestsAndSubmitsForSelectedNumber() async throws {
119+
let scripted = ["2", "111222"]
120+
let index = LockedBox(0)
121+
Current.shell.readLine = { _ in
122+
let i = index.incrementAfterRead()
123+
return i < scripted.count ? scripted[i] : nil
124+
}
125+
126+
let phoneNumbers = [
127+
AuthOptionsResponse.TrustedPhoneNumber(id: 1, numberWithDialCode: "+1 (•••) •••-1111"),
128+
AuthOptionsResponse.TrustedPhoneNumber(id: 2, numberWithDialCode: "+1 (•••) •••-2222"),
129+
]
130+
let requestedPhoneID = LockedBox<Int?>(nil)
131+
let submittedCode = LockedBox<SecurityCode?>(nil)
132+
let dependencies = TwoFactorAuthentication.Dependencies(
133+
submitSecurityCode: { code, _ in submittedCode.set(code); return authenticatedState },
134+
requestSMSSecurityCode: { phone, _, _ in requestedPhoneID.set(phone.id); return authenticatedState }
135+
)
136+
137+
let state = AuthenticationState.waitingForSecondFactor(.smsPendingChoice, authOptions(trustedPhoneNumbers: phoneNumbers), sessionData)
138+
try await TwoFactorAuthentication.completeIfNeeded(state, dependencies: dependencies)
139+
140+
XCTAssertEqual(requestedPhoneID.value, 2)
141+
guard case let .sms(code, phoneNumberId) = submittedCode.value else {
142+
return XCTFail("Expected an SMS code, got \(String(describing: submittedCode.value))")
143+
}
144+
XCTAssertEqual(code, "111222")
145+
XCTAssertEqual(phoneNumberId, 2)
146+
}
147+
148+
func test_CompleteIfNeeded_SMSPendingChoice_InvalidSelection_RetriesUntilValid() async throws {
149+
// "0" and "9" are out of range, then "1" selects the first number.
150+
let scripted = ["0", "9", "1", "555000"]
151+
let index = LockedBox(0)
152+
Current.shell.readLine = { _ in
153+
let i = index.incrementAfterRead()
154+
return i < scripted.count ? scripted[i] : nil
155+
}
156+
157+
let phoneNumber = AuthOptionsResponse.TrustedPhoneNumber(id: 5, numberWithDialCode: "+1 (•••) •••-5555")
158+
let requestedPhoneID = LockedBox<Int?>(nil)
159+
let dependencies = TwoFactorAuthentication.Dependencies(
160+
submitSecurityCode: { _, _ in authenticatedState },
161+
requestSMSSecurityCode: { phone, _, _ in requestedPhoneID.set(phone.id); return authenticatedState }
162+
)
163+
164+
let state = AuthenticationState.waitingForSecondFactor(.smsPendingChoice, authOptions(trustedPhoneNumbers: [phoneNumber]), sessionData)
165+
try await TwoFactorAuthentication.completeIfNeeded(state, dependencies: dependencies)
166+
167+
XCTAssertEqual(requestedPhoneID.value, 5)
168+
}
169+
170+
// MARK: Unsupported / error states
171+
172+
func test_CompleteIfNeeded_SecurityKey_Throws() async {
173+
let dependencies = TwoFactorAuthentication.Dependencies(
174+
submitSecurityCode: { _, _ in authenticatedState },
175+
requestSMSSecurityCode: { _, _, _ in authenticatedState }
176+
)
177+
178+
// securityKey requires an fsaChallenge in a real response, but the handler rejects it before
179+
// inspecting authOptions, so an empty options object is sufficient here.
180+
let state = AuthenticationState.waitingForSecondFactor(.securityKey, authOptions(), sessionData)
181+
182+
do {
183+
try await TwoFactorAuthentication.completeIfNeeded(state, dependencies: dependencies)
184+
XCTFail("Expected security-key handling to throw")
185+
} catch {
186+
// Expected.
187+
}
188+
}
189+
190+
func test_CompleteIfNeeded_NotAppleDeveloper_Throws() async {
191+
let dependencies = TwoFactorAuthentication.Dependencies(
192+
submitSecurityCode: { _, _ in authenticatedState },
193+
requestSMSSecurityCode: { _, _, _ in authenticatedState }
194+
)
195+
196+
do {
197+
try await TwoFactorAuthentication.completeIfNeeded(.notAppleDeveloper, dependencies: dependencies)
198+
XCTFail("Expected notAppleDeveloper to throw")
199+
} catch {
200+
XCTAssertEqual(error as? AuthenticationError, .notDeveloperAppleId)
201+
}
202+
}
203+
}

0 commit comments

Comments
 (0)