From 5149e817da8dca9d2e3f76335a22264351679703 Mon Sep 17 00:00:00 2001 From: Brandon Evans Date: Tue, 22 Dec 2020 22:06:18 -0700 Subject: [PATCH] Store credentials in the keychain Like xcodes, storing the username in defaults so we know which item to look up in the keychain later. This also fixes the Xcode list update logic to not only validate the session but login with saved credentials if it fails. --- Xcodes/Backend/AppState.swift | 113 ++++++++++++++---- Xcodes/Backend/Environment.swift | 18 +++ Xcodes/Frontend/XcodeList/XcodeListView.swift | 15 ++- Xcodes/SettingsView.swift | 32 +---- 4 files changed, 120 insertions(+), 58 deletions(-) diff --git a/Xcodes/Backend/AppState.swift b/Xcodes/Backend/AppState.swift index 33e00cb..fc5e81d 100644 --- a/Xcodes/Backend/AppState.swift +++ b/Xcodes/Backend/AppState.swift @@ -4,6 +4,7 @@ import Combine import Path import PromiseKit import LegibleError +import KeychainAccess class AppState: ObservableObject { private let list = XcodeList() @@ -20,6 +21,7 @@ class AppState: ObservableObject { func validateSession() -> AnyPublisher { return client.validateSession() + .receive(on: DispatchQueue.main) .handleEvents(receiveCompletion: { completion in if case .failure = completion { self.authenticationState = .unauthenticated @@ -29,20 +31,50 @@ class AppState: ObservableObject { .eraseToAnyPublisher() } - func login(username: String, password: String) { - client.login(accountName: username, password: password) - .receive(on: DispatchQueue.main) - .sink( - receiveCompletion: { completion in - self.handleAuthenticationFlowCompletion(completion) - }, - receiveValue: { authenticationState in - self.authenticationState = authenticationState + func loginIfNeeded() -> AnyPublisher { + validateSession() + .catch { (error) -> AnyPublisher in + guard + let username = Current.defaults.string(forKey: "username"), + let password = try? Current.keychain.getString(username) + else { + return Fail(error: error) + .eraseToAnyPublisher() } + + return self.login(username: username, password: password) + .map { _ in Void() } + .eraseToAnyPublisher() + } + .eraseToAnyPublisher() + } + + func login(username: String, password: String) { + login(username: username, password: password) + .sink( + receiveCompletion: { _ in }, + receiveValue: { _ in } ) .store(in: &cancellables) } + func login(username: String, password: String) -> AnyPublisher { + try? Current.keychain.set(password, key: username) + Current.defaults.set(username, forKey: "username") + + return client.login(accountName: username, password: password) + .receive(on: DispatchQueue.main) + .handleEvents( + receiveOutput: { authenticationState in + self.authenticationState = authenticationState + }, + receiveCompletion: { completion in + self.handleAuthenticationFlowCompletion(completion) + } + ) + .eraseToAnyPublisher() + } + func handleTwoFactorOption(_ option: TwoFactorOption, authOptions: AuthOptionsResponse, serviceKey: String, sessionID: String, scnt: String) { self.presentingSignInAlert = false self.secondFactorData = SecondFactorData( @@ -89,6 +121,12 @@ class AppState: ObservableObject { private func handleAuthenticationFlowCompletion(_ completion: Subscribers.Completion) { switch completion { case let .failure(error): + if case .invalidUsernameOrPassword = error as? AuthenticationError, + let username = Current.defaults.string(forKey: "username") { + // remove any keychain password if we fail to log with an invalid username or password so it doesn't try again. + try? Current.keychain.remove(username) + } + self.error = AlertContent(title: "Error signing in", message: error.legibleLocalizedDescription) case .finished: switch self.authenticationState { @@ -101,30 +139,55 @@ class AppState: ObservableObject { } } + func logOut() { + let username = Current.defaults.string(forKey: "username") + Current.defaults.removeObject(forKey: "username") + if let username = username { + try? Current.keychain.remove(username) + } + AppleAPI.Current.network.session.configuration.httpCookieStorage?.removeCookies(since: .distantPast) + authenticationState = .unauthenticated + } + // MARK: - Load Xcode Versions func update() { - // Treat this implementation as a placeholder that can be thrown away. - // It's only here to make it easy to see that auth works. update() - .sink(receiveCompletion: { _ in }, - receiveValue: { _ in }) + .sink( + receiveCompletion: { _ in }, + receiveValue: { _ in } + ) .store(in: &cancellables) } - public func update() -> AnyPublisher<[Xcode], Error> { - // Wrap the Promise API in a Publisher for now - return Deferred { - Future { promise in - self.list.update() - .done { promise(.success($0)) } - .catch { promise(.failure($0)) } + public func update() -> AnyPublisher<[Xcode], Never> { + loginIfNeeded() + .flatMap { + // Wrap the Promise API in a Publisher for now + Deferred { + Future { promise in + self.list.update() + .done { promise(.success($0)) } + .catch { promise(.failure($0)) } + } + } + .handleEvents( + receiveCompletion: { completion in + if case let .failure(error) = completion { + self.error = AlertContent(title: "Update Error", message: error.legibleLocalizedDescription) + } + } + ) } - } - .handleEvents(receiveOutput: { [unowned self] xcodes in - self.updateAllVersions(xcodes) - }) - .eraseToAnyPublisher() + .catch { _ in + Just(self.list.availableXcodes) + } + .handleEvents( + receiveOutput: { [unowned self] xcodes in + self.updateAllVersions(xcodes) + } + ) + .eraseToAnyPublisher() } private func updateAllVersions(_ xcodes: [Xcode]) { diff --git a/Xcodes/Backend/Environment.swift b/Xcodes/Backend/Environment.swift index 2595a3d..d5d1467 100644 --- a/Xcodes/Backend/Environment.swift +++ b/Xcodes/Backend/Environment.swift @@ -18,6 +18,7 @@ public struct Environment { public var network = Network() public var logging = Logging() public var keychain = Keychain() + public var defaults = Defaults() } public var Current = Environment() @@ -152,3 +153,20 @@ public struct Keychain { try remove(key) } } + +public struct Defaults { + public var string: (String) -> String? = { UserDefaults.standard.string(forKey: $0) } + public func string(forKey key: String) -> String? { + string(key) + } + + public var set: (Any?, String) -> Void = { UserDefaults.standard.set($0, forKey: $1) } + public func set(_ value: Any?, forKey key: String) { + set(value, key) + } + + public var removeObject: (String) -> Void = { UserDefaults.standard.removeObject(forKey: $0) } + public func removeObject(forKey key: String) { + removeObject(key) + } +} diff --git a/Xcodes/Frontend/XcodeList/XcodeListView.swift b/Xcodes/Frontend/XcodeList/XcodeListView.swift index 1247397..7594be8 100644 --- a/Xcodes/Frontend/XcodeList/XcodeListView.swift +++ b/Xcodes/Frontend/XcodeList/XcodeListView.swift @@ -95,12 +95,15 @@ struct XcodeListView: View { message: Text(verbatim: error.message), dismissButton: .default(Text("OK"))) } - .alert(item: self.$rowBeingConfirmedForUninstallation) { row in - Alert(title: Text("Uninstall Xcode \(row.title)?"), - message: Text("It will be moved to the Trash, but won't be emptied."), - primaryButton: .destructive(Text("Uninstall"), action: { self.appState.uninstall(id: row.id) }), - secondaryButton: .cancel(Text("Cancel"))) - } + /* + Removing this for now, because it's overriding the error alert that's being worked on above. + .alert(item: self.$rowBeingConfirmedForUninstallation) { row in + Alert(title: Text("Uninstall Xcode \(row.title)?"), + message: Text("It will be moved to the Trash, but won't be emptied."), + primaryButton: .destructive(Text("Uninstall"), action: { self.appState.uninstall(id: row.id) }), + secondaryButton: .cancel(Text("Cancel"))) + } + **/ .sheet(isPresented: $appState.secondFactorData.isNotNil) { secondFactorView(appState.secondFactorData!) .environmentObject(appState) diff --git a/Xcodes/SettingsView.swift b/Xcodes/SettingsView.swift index 597ad6d..62d7eb2 100644 --- a/Xcodes/SettingsView.swift +++ b/Xcodes/SettingsView.swift @@ -8,21 +8,15 @@ struct SettingsView: View { VStack(alignment: .leading) { GroupBox(label: Text("Apple ID")) { VStack(alignment: .leading) { - switch appState.authenticationState { - case .authenticated: - Text("Signed in") - Button("Sign Out", action: {}) - - case .unauthenticated: + if let username = Current.defaults.string(forKey: "username") { + Text(username) + Button("Sign Out", action: appState.logOut) + } else { Button("Sign In", action: { self.appState.presentingSignInAlert = true }) .sheet(isPresented: $appState.presentingSignInAlert) { SignInCredentialsView(isPresented: $appState.presentingSignInAlert) .environmentObject(appState) } - - case .waitingForSecondFactor: - Button("Signing In...", action: {}) - .disabled(true) } } .frame(maxWidth: .infinity, alignment: .leading) @@ -40,23 +34,7 @@ struct SettingsView_Previews: PreviewProvider { static var previews: some View { Group { SettingsView() - .environmentObject(configure(AppState()) { - $0.authenticationState = .authenticated - }) - - SettingsView() - .environmentObject(configure(AppState()) { - $0.authenticationState = .unauthenticated - }) - - SettingsView() - .environmentObject(configure(AppState()) { - $0.authenticationState = .waitingForSecondFactor( - TwoFactorOption.codeSent, - AuthOptionsResponse(trustedPhoneNumbers: nil, trustedDevices: nil, securityCode: .init(length: 6)), - AppleSessionData(serviceKey: "", sessionID: "", scnt: "") - ) - }) + .environmentObject(AppState()) } } }