From 1496f32e289d95340f33a3c8231ac549eb8b2d3a Mon Sep 17 00:00:00 2001 From: Matt Kiazyk Date: Thu, 14 Oct 2021 15:43:16 -0500 Subject: [PATCH 1/2] Better handling of when AppleId is not a developer --- Xcodes/AppleAPI/Sources/AppleAPI/Client.swift | 34 +++++++++++++++---- Xcodes/Backend/AppState+Install.swift | 8 ++++- Xcodes/Backend/AppState+Update.swift | 7 +++- Xcodes/Backend/AppState.swift | 12 ++++--- 4 files changed, 49 insertions(+), 12 deletions(-) diff --git a/Xcodes/AppleAPI/Sources/AppleAPI/Client.swift b/Xcodes/AppleAPI/Sources/AppleAPI/Client.swift index 462286d..a45d300 100644 --- a/Xcodes/AppleAPI/Sources/AppleAPI/Client.swift +++ b/Xcodes/AppleAPI/Sources/AppleAPI/Client.swift @@ -148,11 +148,12 @@ public class Client { /// Use the olympus session endpoint to see if the existing session is still valid public func validateSession() -> AnyPublisher { return Current.network.dataTask(with: URLRequest.olympusSession) - .tryMap { (data, response) in - guard - let jsonObject = (try? JSONSerialization.jsonObject(with: data)) as? [String: Any], - jsonObject["provider"] != nil - else { throw AuthenticationError.invalidSession } + .map(\.data) + .decode(type: AppleSession.self, decoder: JSONDecoder()) + .tryMap { session in + if session.provider == nil { + throw AuthenticationError.notDeveloperAppleId + } } .eraseToAnyPublisher() } @@ -174,6 +175,7 @@ public enum AuthenticationState: Equatable { case unauthenticated case waitingForSecondFactor(TwoFactorOption, AuthOptionsResponse, AppleSessionData) case authenticated + case notAppleDeveloper } public enum AuthenticationError: Swift.Error, LocalizedError, Equatable { @@ -186,7 +188,8 @@ public enum AuthenticationError: Swift.Error, LocalizedError, Equatable { case accountUsesUnknownAuthenticationKind(String?) case accountLocked(String) case badStatusCode(statusCode: Int, data: Data, response: HTTPURLResponse) - + case notDeveloperAppleId + public var errorDescription: String? { switch self { case .invalidSession: @@ -212,6 +215,8 @@ public enum AuthenticationError: Swift.Error, LocalizedError, Equatable { return message case let .badStatusCode(statusCode, _, _): return "Received an unexpected status code: \(statusCode). If you continue to have problems, please submit a bug report in the Help menu." + case .notDeveloperAppleId: + return "You are not registered as an Apple Developer. Please visit Apple Developer Registration. https://developer.apple.com/register/" } } } @@ -362,3 +367,20 @@ public enum SecurityCode { } } } + +/// Object returned from olympus/v1/session +/// Used to check Provider, and show name +/// If Provider is nil, we can assume the Apple User is NOT an Apple Developer and can't download Xcode. +public struct AppleSession: Decodable, Equatable { + public let user: AppleUser + public let provider: AppleProvider? +} + +public struct AppleProvider: Decodable, Equatable { + public let providerId: Int + public let name: String +} + +public struct AppleUser: Decodable, Equatable { + public let fullName: String +} diff --git a/Xcodes/Backend/AppState+Install.swift b/Xcodes/Backend/AppState+Install.swift index d295d68..28449ee 100644 --- a/Xcodes/Backend/AppState+Install.swift +++ b/Xcodes/Backend/AppState+Install.swift @@ -40,9 +40,15 @@ extension AppState { } private func install(_ installationType: InstallationType, downloader: Downloader, attemptNumber: Int) -> AnyPublisher { + // We need to check if the Apple ID that is logged in is an Apple Developer + // Since users can use xcodereleases, we don't check for Apple ID on a xcode list refresh + // If the Apple Id is not a developer, the download action will try and download a xip that is invalid, causing a `xcode13.0.xip is damaged and can't be expanded.` Logger.appState.info("Using \(downloader) downloader") - return getXcodeArchive(installationType, downloader: downloader) + return validateSession() + .flatMap { _ in + self.getXcodeArchive(installationType, downloader: downloader) + } .flatMap { xcode, url -> AnyPublisher in self.installArchivedXcode(xcode, at: url) } diff --git a/Xcodes/Backend/AppState+Update.swift b/Xcodes/Backend/AppState+Update.swift index 62a327d..ed998af 100644 --- a/Xcodes/Backend/AppState+Update.swift +++ b/Xcodes/Backend/AppState+Update.swift @@ -71,7 +71,12 @@ extension AppState { private func updateAvailableXcodes(from dataSource: DataSource) -> AnyPublisher<[AvailableXcode], Error> { switch dataSource { case .apple: - return signInIfNeeded() + return signInIfNeeded() + .flatMap { [unowned self] in + // this will check to see if the Apple ID is a valid Apple Developer or not. + // If it's not, we can't use the Apple source to get xcode info. + self.validateSession() + } .flatMap { [unowned self] in self.releasedXcodes().combineLatest(self.prereleaseXcodes()) } .receive(on: DispatchQueue.main) .map { releasedXcodes, prereleaseXcodes in diff --git a/Xcodes/Backend/AppState.swift b/Xcodes/Backend/AppState.swift index 6c3c48b..c14ebf4 100644 --- a/Xcodes/Backend/AppState.swift +++ b/Xcodes/Backend/AppState.swift @@ -116,8 +116,9 @@ class AppState: ObservableObject { .receive(on: DispatchQueue.main) .handleEvents(receiveCompletion: { completion in if case .failure = completion { - self.authenticationState = .unauthenticated - self.presentedSheet = .signIn + // this is causing some awkwardness with showing an alert with the error and also popping up the sign in view + // self.authenticationState = .unauthenticated + // self.presentedSheet = .signIn } }) .eraseToAnyPublisher() @@ -227,7 +228,7 @@ class AppState: ObservableObject { self.authError = error case .finished: switch self.authenticationState { - case .authenticated, .unauthenticated: + case .authenticated, .unauthenticated, .notAppleDeveloper: self.presentedSheet = nil self.secondFactorData = nil case let .waitingForSecondFactor(option, authOptions, sessionData): @@ -315,7 +316,7 @@ class AppState: ObservableObject { self.$authenticationState .filter { state in switch state { - case .authenticated, .unauthenticated: return true + case .authenticated, .unauthenticated, .notAppleDeveloper: return true case .waitingForSecondFactor: return false } } @@ -324,6 +325,9 @@ class AppState: ObservableObject { if state == .unauthenticated { throw AuthenticationError.invalidSession } + if state == .notAppleDeveloper { + throw AuthenticationError.notDeveloperAppleId + } return Void() } } From e11cdd119846c0e7e8720cbdcb0a9bc79a5ef484 Mon Sep 17 00:00:00 2001 From: Matt Kiazyk Date: Tue, 19 Oct 2021 10:54:47 -0500 Subject: [PATCH 2/2] Fix up Tests - modify validateSession to use our network wrapper. --- Xcodes/Backend/AppState.swift | 4 ++-- Xcodes/Backend/Environment.swift | 6 +++++- XcodesTests/AppStateTests.swift | 4 ++++ XcodesTests/Environment+Mock.swift | 5 +++++ 4 files changed, 16 insertions(+), 3 deletions(-) diff --git a/Xcodes/Backend/AppState.swift b/Xcodes/Backend/AppState.swift index c14ebf4..f6090cb 100644 --- a/Xcodes/Backend/AppState.swift +++ b/Xcodes/Backend/AppState.swift @@ -10,7 +10,7 @@ import os.log class AppState: ObservableObject { private let client = AppleAPI.Client() - + // MARK: - Published Properties @Published var authenticationState: AuthenticationState = .unauthenticated @@ -112,7 +112,7 @@ class AppState: ObservableObject { // MARK: - Authentication func validateSession() -> AnyPublisher { - return client.validateSession() + return Current.network.validateSession() .receive(on: DispatchQueue.main) .handleEvents(receiveCompletion: { completion in if case .failure = completion { diff --git a/Xcodes/Backend/Environment.swift b/Xcodes/Backend/Environment.swift index 6d8af24..ef93c4a 100644 --- a/Xcodes/Backend/Environment.swift +++ b/Xcodes/Backend/Environment.swift @@ -169,7 +169,7 @@ private func _installedXcodes(destination: Path) -> [InstalledXcode] { public struct Network { private static let client = AppleAPI.Client() - public var dataTask: (URLRequest) -> AnyPublisher = { + public var dataTask: (URLRequest) -> AnyPublisher = { AppleAPI.Current.network.session.dataTaskPublisher(for: $0) .mapError { $0 as Error } .eraseToAnyPublisher() @@ -183,6 +183,10 @@ public struct Network { public func downloadTask(with url: URL, to saveLocation: URL, resumingWith resumeData: Data?) -> (progress: Progress, publisher: AnyPublisher<(saveLocation: URL, response: URLResponse), Error>) { return downloadTask(url, saveLocation, resumeData) } + + public var validateSession: () -> AnyPublisher = { + return client.validateSession() + } } public struct Keychain { diff --git a/XcodesTests/AppStateTests.swift b/XcodesTests/AppStateTests.swift index 0c4c165..c708d9d 100644 --- a/XcodesTests/AppStateTests.swift +++ b/XcodesTests/AppStateTests.swift @@ -81,6 +81,10 @@ class AppStateTests: XCTestCase { return true } } + Xcodes.Current.network.validateSession = { + return Just(()) + .setFailureType(to: Error.self).eraseToAnyPublisher() + } Xcodes.Current.network.dataTask = { urlRequest in // Don't have a valid session if urlRequest.url! == URLRequest.olympusSession.url! { diff --git a/XcodesTests/Environment+Mock.swift b/XcodesTests/Environment+Mock.swift index ed91706..8755da5 100644 --- a/XcodesTests/Environment+Mock.swift +++ b/XcodesTests/Environment+Mock.swift @@ -68,6 +68,11 @@ extension Network { .setFailureType(to: Error.self) .eraseToAnyPublisher() ) + }, + validateSession: { + return Just(()) + .setFailureType(to: Error.self) + .eraseToAnyPublisher() } ) }