Skip to content

Commit 1b23a7e

Browse files
authored
Merge pull request #481 from kinoroy/fix-2fa-authentication-regression
Fix 2FA authentication regression causing 403 on download (#480)
2 parents 516b846 + 3c5d5ed commit 1b23a7e

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)