From 3c5d5edd8be366af3334058e28c6eece83e10a79 Mon Sep 17 00:00:00 2001 From: Kino Roy Date: Tue, 16 Jun 2026 12:50:50 -0700 Subject: [PATCH] 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 --- Sources/XcodesKit/Environment.swift | 6 +- .../XcodesKit/TwoFactorAuthentication.swift | 132 ++++++++++++ .../TwoFactorAuthenticationTests.swift | 203 ++++++++++++++++++ 3 files changed, 339 insertions(+), 2 deletions(-) create mode 100644 Sources/XcodesKit/TwoFactorAuthentication.swift create mode 100644 Tests/XcodesKitTests/TwoFactorAuthenticationTests.swift diff --git a/Sources/XcodesKit/Environment.swift b/Sources/XcodesKit/Environment.swift index a443f7a..3b5a637 100644 --- a/Sources/XcodesKit/Environment.swift +++ b/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) diff --git a/Sources/XcodesKit/TwoFactorAuthentication.swift b/Sources/XcodesKit/TwoFactorAuthentication.swift new file mode 100644 index 0000000..2eef55c --- /dev/null +++ b/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] + } +} diff --git a/Tests/XcodesKitTests/TwoFactorAuthenticationTests.swift b/Tests/XcodesKitTests/TwoFactorAuthenticationTests.swift new file mode 100644 index 0000000..be2c8bd --- /dev/null +++ b/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(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(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(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(nil) + let submittedCode = LockedBox(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(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) + } + } +}