Fix 2FA authentication regression causing 403 on download (#480)

Sources/XcodesKit/Environment.swift

@@ -296,7 +296,8 @@ public struct Network: Sendable {
             downloadTask = { loginClient.urlSession.downloadTask(with: $0, to: $1, resumingWith: $2) }
             validateSession = { _ = try await loginClient.validateSession() }
             login = { accountName, password in
-                _ = try await loginClient.srpLogin(accountName: accountName, password: password)
+                let state = try await loginClient.srpLogin(accountName: accountName, password: password)
+                try await TwoFactorAuthentication.completeIfNeeded(state, dependencies: .liveDependencies(client: loginClient))
             }
             checkIsFederated = { accountName in
                 try await loginClient.checkIsFederated(accountName: accountName)
@@ -371,7 +372,8 @@ public struct Network: Sendable {
         self.downloadTask = downloadTask ?? { loginClient.urlSession.downloadTask(with: $0, to: $1, resumingWith: $2) }
         self.validateSession = validateSession ?? { _ = try await loginClient.validateSession() }
         self.login = login ?? { accountName, password in
-            _ = try await loginClient.srpLogin(accountName: accountName, password: password)
+            let state = try await loginClient.srpLogin(accountName: accountName, password: password)
+            try await TwoFactorAuthentication.completeIfNeeded(state, dependencies: .liveDependencies(client: loginClient))
         }
         self.checkIsFederated = checkIsFederated ?? { accountName in
             try await loginClient.checkIsFederated(accountName: accountName)

Sources/XcodesKit/TwoFactorAuthentication.swift

@@ -0,0 +1,132 @@
+import Foundation
+import Rainbow
+import XcodesKit
+import XcodesLoginKit
+
+/// Drives the interactive two-factor portion of an Apple sign-in from the command line.
+///
+/// `XcodesLoginKit.Client.srpLogin(accountName:password:)` no longer prompts for a verification code
+/// itself; instead it returns the next ``AuthenticationState``. UI clients (like the SwiftUI app) are
+/// expected to present their own second-factor screen and then call `submitSecurityCode` /
+/// `requestSMSSecurityCode`. Without an equivalent here, the CLI silently stopped at the pre-2FA session
+/// in 2.x, leaving the cookie jar unauthenticated and causing 403s on download. This restores the
+/// interactive flow that shipped in xcodes 1.6.x for trusted-device codes and SMS codes.
+enum TwoFactorAuthentication {
+    /// The operations needed to complete a second-factor challenge.
+    ///
+    /// Modeled as closures rather than a concrete `Client` so the flow can be exercised in tests and to
+    /// match the dependency-injection style used by ``Environment``.
+    struct Dependencies: Sendable {
+        var submitSecurityCode: @Sendable (SecurityCode, AppleSessionData) async throws -> AuthenticationState
+        var requestSMSSecurityCode: @Sendable (AuthOptionsResponse.TrustedPhoneNumber, AuthOptionsResponse, AppleSessionData) async throws -> AuthenticationState
+
+        /// Builds dependencies backed by a live login client.
+        static func liveDependencies(client: XcodesLoginKit.Client) -> Dependencies {
+            Dependencies(
+                submitSecurityCode: { code, sessionData in
+                    try await client.submitSecurityCode(code, sessionData: sessionData)
+                },
+                requestSMSSecurityCode: { phoneNumber, authOptions, sessionData in
+                    try await client.requestSMSSecurityCode(to: phoneNumber, authOptions: authOptions, sessionData: sessionData)
+                }
+            )
+        }
+    }
+
+    /// Completes any outstanding second-factor challenge for a freshly attempted login.
+    ///
+    /// - Parameters:
+    ///   - state: The state returned by `srpLogin` (or a subsequent step).
+    ///   - dependencies: The operations used to submit codes and request SMS messages.
+    static func completeIfNeeded(_ state: AuthenticationState, dependencies: Dependencies) async throws {
+        switch state {
+        case .authenticated:
+            return
+        case let .waitingForSecondFactor(option, authOptions, sessionData):
+            try await handleTwoFactor(option: option, authOptions: authOptions, sessionData: sessionData, dependencies: dependencies)
+        case .waitingForFederatedAuthentication:
+            // Federated accounts are detected and handled by AppleSessionService before srpLogin runs,
+            // so reaching here means the federated flow wasn't completed.
+            throw AuthenticationError.federatedAuthenticationRequired
+        case .notAppleDeveloper:
+            throw AuthenticationError.notDeveloperAppleId
+        case .unauthenticated:
+            throw AuthenticationError.notAuthorized
+        }
+    }
+
+    private static func handleTwoFactor(option: TwoFactorOption, authOptions: AuthOptionsResponse, sessionData: AppleSessionData, dependencies: Dependencies) async throws {
+        Current.logging.log("Two-factor authentication is enabled for this account.\n")
+
+        switch option {
+        // An SMS code was sent automatically to the account's single trusted phone number.
+        case let .smsSent(phoneNumber):
+            try await submitSMSCode(authOptions: authOptions, phoneNumber: phoneNumber, sessionData: sessionData, dependencies: dependencies)
+        // No code was sent automatically; the user must pick a phone number first.
+        case .smsPendingChoice:
+            try await handleWithPhoneNumberSelection(authOptions: authOptions, sessionData: sessionData, dependencies: dependencies)
+        // A code is shown on the account's trusted devices.
+        case .codeSent:
+            try await submitDeviceCode(authOptions: authOptions, sessionData: sessionData, dependencies: dependencies)
+        // A physical security key is required, which the CLI has never supported (1.6.x threw here too).
+        case .securityKey:
+            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.")
+        }
+    }
+
+    /// Prompts for a trusted-device code, allowing the user to fall back to SMS by entering "sms".
+    private static func submitDeviceCode(authOptions: AuthOptionsResponse, sessionData: AppleSessionData, dependencies: Dependencies) async throws {
+        let securityCodeLength = authOptions.securityCode?.length ?? 0
+        let code = Current.shell.readLine(prompt: """
+        Enter "sms" without quotes to exit this prompt and choose a phone number to send an SMS security code to.
+        Enter the \(securityCodeLength) digit code from one of your trusted devices:
+        """) ?? ""
+
+        if code == "sms" {
+            try await handleWithPhoneNumberSelection(authOptions: authOptions, sessionData: sessionData, dependencies: dependencies)
+            return
+        }
+
+        _ = try await dependencies.submitSecurityCode(.device(code: code), sessionData)
+    }
+
+    /// Lists the trusted phone numbers, requests an SMS to the chosen one, then submits the code.
+    private static func handleWithPhoneNumberSelection(authOptions: AuthOptionsResponse, sessionData: AppleSessionData, dependencies: Dependencies) async throws {
+        // 2FA requires at least one trusted phone number, but inform the user rather than crashing if absent.
+        guard let trustedPhoneNumbers = authOptions.trustedPhoneNumbers, trustedPhoneNumbers.isEmpty == false else {
+            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.")
+        }
+
+        let phoneNumber = selectPhoneNumberInteractively(from: trustedPhoneNumbers)
+        _ = try await dependencies.requestSMSSecurityCode(phoneNumber, authOptions, sessionData)
+        try await submitSMSCode(authOptions: authOptions, phoneNumber: phoneNumber, sessionData: sessionData, dependencies: dependencies)
+    }
+
+    private static func submitSMSCode(authOptions: AuthOptionsResponse, phoneNumber: AuthOptionsResponse.TrustedPhoneNumber, sessionData: AppleSessionData, dependencies: Dependencies) async throws {
+        guard let length = authOptions.securityCode?.length else {
+            throw XcodesKitError("Expected security code info but didn't receive any.")
+        }
+
+        let code = Current.shell.readLine(prompt: "Enter the \(length) digit code sent to \(phoneNumber.numberWithDialCode): ") ?? ""
+        _ = try await dependencies.submitSecurityCode(.sms(code: code, phoneNumberId: phoneNumber.id), sessionData)
+    }
+
+    private static func selectPhoneNumberInteractively(from trustedPhoneNumbers: [AuthOptionsResponse.TrustedPhoneNumber]) -> AuthOptionsResponse.TrustedPhoneNumber {
+        Current.logging.log("Trusted phone numbers:")
+        for (index, phoneNumber) in trustedPhoneNumbers.enumerated() {
+            Current.logging.log("\(index + 1): \(phoneNumber.numberWithDialCode)")
+        }
+
+        let possibleSelection = Current.shell.readLine(prompt: "Select a trusted phone number to receive a code via SMS: ")
+        guard
+            let possibleSelection,
+            let selection = Int(possibleSelection),
+            trustedPhoneNumbers.indices.contains(selection - 1)
+        else {
+            Current.logging.log("Not a valid phone number index. Expecting a whole number between 1-\(trustedPhoneNumbers.count), but was given \(possibleSelection ?? "nothing").\n".red)
+            return selectPhoneNumberInteractively(from: trustedPhoneNumbers)
+        }
+
+        return trustedPhoneNumbers[selection - 1]
+    }
+}

Tests/XcodesKitTests/TwoFactorAuthenticationTests.swift

@@ -0,0 +1,203 @@
+@testable import XcodesCLIKit
+import Foundation
+import XcodesLoginKit
+import XCTest
+
+/// `AppleSession` only exposes a `Decodable` initializer, so build the authenticated state from JSON.
+private let authenticatedState: AuthenticationState = {
+    let json = Data(#"{"user":{"fullName":"Test User"}}"#.utf8)
+    let session = try! JSONDecoder().decode(AppleSession.self, from: json)
+    return .authenticated(session)
+}()
+
+final class TwoFactorAuthenticationTests: XCTestCase {
+    override func setUp() {
+        super.setUp()
+        Current = .mock
+    }
+
+    private func authOptions(
+        trustedPhoneNumbers: [AuthOptionsResponse.TrustedPhoneNumber]? = nil,
+        codeLength: Int = 6
+    ) -> AuthOptionsResponse {
+        AuthOptionsResponse(
+            trustedPhoneNumbers: trustedPhoneNumbers,
+            trustedDevices: nil,
+            securityCode: .init(length: codeLength)
+        )
+    }
+
+    private let sessionData = AppleSessionData(serviceKey: "service", sessionID: "session", scnt: "scnt")
+
+    // MARK: Already authenticated
+
+    func test_CompleteIfNeeded_AlreadyAuthenticated_DoesNothing() async throws {
+        let submitCalled = LockedBox(false)
+        let dependencies = TwoFactorAuthentication.Dependencies(
+            submitSecurityCode: { _, _ in submitCalled.set(true); return authenticatedState },
+            requestSMSSecurityCode: { _, _, _ in authenticatedState }
+        )
+
+        try await TwoFactorAuthentication.completeIfNeeded(authenticatedState, dependencies: dependencies)
+
+        XCTAssertFalse(submitCalled.value)
+    }
+
+    // MARK: Trusted device code
+
+    func test_CompleteIfNeeded_TrustedDeviceCode_SubmitsEnteredCode() async throws {
+        Current.shell.readLine = { _ in "123456" }
+
+        let submittedCode = LockedBox<SecurityCode?>(nil)
+        let dependencies = TwoFactorAuthentication.Dependencies(
+            submitSecurityCode: { code, _ in submittedCode.set(code); return authenticatedState },
+            requestSMSSecurityCode: { _, _, _ in XCTFail("Should not request SMS"); return authenticatedState }
+        )
+
+        let state = AuthenticationState.waitingForSecondFactor(.codeSent, authOptions(), sessionData)
+        try await TwoFactorAuthentication.completeIfNeeded(state, dependencies: dependencies)
+
+        guard case let .device(code) = submittedCode.value else {
+            return XCTFail("Expected a trusted-device code, got \(String(describing: submittedCode.value))")
+        }
+        XCTAssertEqual(code, "123456")
+    }
+
+    func test_CompleteIfNeeded_TrustedDeviceCode_EnteringSMS_FallsBackToPhoneSelection() async throws {
+        // First prompt (device code) -> "sms"; second prompt (phone selection) -> "1"; third prompt (SMS code) -> "654321".
+        let scripted = ["sms", "1", "654321"]
+        let index = LockedBox(0)
+        Current.shell.readLine = { _ in
+            let i = index.incrementAfterRead()
+            return i < scripted.count ? scripted[i] : nil
+        }
+
+        let smsRequested = LockedBox(false)
+        let submittedCode = LockedBox<SecurityCode?>(nil)
+        let phoneNumber = AuthOptionsResponse.TrustedPhoneNumber(id: 7, numberWithDialCode: "+1 (•••) •••-1234")
+        let dependencies = TwoFactorAuthentication.Dependencies(
+            submitSecurityCode: { code, _ in submittedCode.set(code); return authenticatedState },
+            requestSMSSecurityCode: { _, _, _ in smsRequested.set(true); return authenticatedState }
+        )
+
+        let state = AuthenticationState.waitingForSecondFactor(.codeSent, authOptions(trustedPhoneNumbers: [phoneNumber]), sessionData)
+        try await TwoFactorAuthentication.completeIfNeeded(state, dependencies: dependencies)
+
+        XCTAssertTrue(smsRequested.value)
+        guard case let .sms(code, phoneNumberId) = submittedCode.value else {
+            return XCTFail("Expected an SMS code, got \(String(describing: submittedCode.value))")
+        }
+        XCTAssertEqual(code, "654321")
+        XCTAssertEqual(phoneNumberId, 7)
+    }
+
+    // MARK: SMS automatically sent
+
+    func test_CompleteIfNeeded_SMSSent_SubmitsCodeForThatNumber() async throws {
+        Current.shell.readLine = { _ in "987654" }
+
+        let phoneNumber = AuthOptionsResponse.TrustedPhoneNumber(id: 3, numberWithDialCode: "+1 (•••) •••-9999")
+        let submittedCode = LockedBox<SecurityCode?>(nil)
+        let dependencies = TwoFactorAuthentication.Dependencies(
+            submitSecurityCode: { code, _ in submittedCode.set(code); return authenticatedState },
+            requestSMSSecurityCode: { _, _, _ in XCTFail("SMS already sent automatically"); return authenticatedState }
+        )
+
+        let state = AuthenticationState.waitingForSecondFactor(.smsSent(phoneNumber), authOptions(trustedPhoneNumbers: [phoneNumber]), sessionData)
+        try await TwoFactorAuthentication.completeIfNeeded(state, dependencies: dependencies)
+
+        guard case let .sms(code, phoneNumberId) = submittedCode.value else {
+            return XCTFail("Expected an SMS code, got \(String(describing: submittedCode.value))")
+        }
+        XCTAssertEqual(code, "987654")
+        XCTAssertEqual(phoneNumberId, 3)
+    }
+
+    // MARK: SMS phone number selection
+
+    func test_CompleteIfNeeded_SMSPendingChoice_RequestsAndSubmitsForSelectedNumber() async throws {
+        let scripted = ["2", "111222"]
+        let index = LockedBox(0)
+        Current.shell.readLine = { _ in
+            let i = index.incrementAfterRead()
+            return i < scripted.count ? scripted[i] : nil
+        }
+
+        let phoneNumbers = [
+            AuthOptionsResponse.TrustedPhoneNumber(id: 1, numberWithDialCode: "+1 (•••) •••-1111"),
+            AuthOptionsResponse.TrustedPhoneNumber(id: 2, numberWithDialCode: "+1 (•••) •••-2222"),
+        ]
+        let requestedPhoneID = LockedBox<Int?>(nil)
+        let submittedCode = LockedBox<SecurityCode?>(nil)
+        let dependencies = TwoFactorAuthentication.Dependencies(
+            submitSecurityCode: { code, _ in submittedCode.set(code); return authenticatedState },
+            requestSMSSecurityCode: { phone, _, _ in requestedPhoneID.set(phone.id); return authenticatedState }
+        )
+
+        let state = AuthenticationState.waitingForSecondFactor(.smsPendingChoice, authOptions(trustedPhoneNumbers: phoneNumbers), sessionData)
+        try await TwoFactorAuthentication.completeIfNeeded(state, dependencies: dependencies)
+
+        XCTAssertEqual(requestedPhoneID.value, 2)
+        guard case let .sms(code, phoneNumberId) = submittedCode.value else {
+            return XCTFail("Expected an SMS code, got \(String(describing: submittedCode.value))")
+        }
+        XCTAssertEqual(code, "111222")
+        XCTAssertEqual(phoneNumberId, 2)
+    }
+
+    func test_CompleteIfNeeded_SMSPendingChoice_InvalidSelection_RetriesUntilValid() async throws {
+        // "0" and "9" are out of range, then "1" selects the first number.
+        let scripted = ["0", "9", "1", "555000"]
+        let index = LockedBox(0)
+        Current.shell.readLine = { _ in
+            let i = index.incrementAfterRead()
+            return i < scripted.count ? scripted[i] : nil
+        }
+
+        let phoneNumber = AuthOptionsResponse.TrustedPhoneNumber(id: 5, numberWithDialCode: "+1 (•••) •••-5555")
+        let requestedPhoneID = LockedBox<Int?>(nil)
+        let dependencies = TwoFactorAuthentication.Dependencies(
+            submitSecurityCode: { _, _ in authenticatedState },
+            requestSMSSecurityCode: { phone, _, _ in requestedPhoneID.set(phone.id); return authenticatedState }
+        )
+
+        let state = AuthenticationState.waitingForSecondFactor(.smsPendingChoice, authOptions(trustedPhoneNumbers: [phoneNumber]), sessionData)
+        try await TwoFactorAuthentication.completeIfNeeded(state, dependencies: dependencies)
+
+        XCTAssertEqual(requestedPhoneID.value, 5)
+    }
+
+    // MARK: Unsupported / error states
+
+    func test_CompleteIfNeeded_SecurityKey_Throws() async {
+        let dependencies = TwoFactorAuthentication.Dependencies(
+            submitSecurityCode: { _, _ in authenticatedState },
+            requestSMSSecurityCode: { _, _, _ in authenticatedState }
+        )
+
+        // securityKey requires an fsaChallenge in a real response, but the handler rejects it before
+        // inspecting authOptions, so an empty options object is sufficient here.
+        let state = AuthenticationState.waitingForSecondFactor(.securityKey, authOptions(), sessionData)
+
+        do {
+            try await TwoFactorAuthentication.completeIfNeeded(state, dependencies: dependencies)
+            XCTFail("Expected security-key handling to throw")
+        } catch {
+            // Expected.
+        }
+    }
+
+    func test_CompleteIfNeeded_NotAppleDeveloper_Throws() async {
+        let dependencies = TwoFactorAuthentication.Dependencies(
+            submitSecurityCode: { _, _ in authenticatedState },
+            requestSMSSecurityCode: { _, _, _ in authenticatedState }
+        )
+
+        do {
+            try await TwoFactorAuthentication.completeIfNeeded(.notAppleDeveloper, dependencies: dependencies)
+            XCTFail("Expected notAppleDeveloper to throw")
+        } catch {
+            XCTAssertEqual(error as? AuthenticationError, .notDeveloperAppleId)
+        }
+    }
+}