From 867ad8ab4b6c4e5f53ba59912187dade397e8f75 Mon Sep 17 00:00:00 2001 From: Brandon Evans Date: Tue, 22 Dec 2020 16:41:59 -0700 Subject: [PATCH 1/3] Add SettingsView --- Xcodes.xcodeproj/project.pbxproj | 8 +++ Xcodes/Backend/Configure.swift | 5 ++ Xcodes/Frontend/XcodeList/XcodeListView.swift | 5 -- Xcodes/SettingsView.swift | 62 +++++++++++++++++++ Xcodes/XcodesApp.swift | 13 ++-- 5 files changed, 83 insertions(+), 10 deletions(-) create mode 100644 Xcodes/Backend/Configure.swift create mode 100644 Xcodes/SettingsView.swift diff --git a/Xcodes.xcodeproj/project.pbxproj b/Xcodes.xcodeproj/project.pbxproj index dc3dcf1..0867a06 100644 --- a/Xcodes.xcodeproj/project.pbxproj +++ b/Xcodes.xcodeproj/project.pbxproj @@ -39,6 +39,8 @@ CABFA9F32592F0E400380FEE /* PMKFoundation in Frameworks */ = {isa = PBXBuildFile; productRef = CABFA9F22592F0E400380FEE /* PMKFoundation */; }; CABFA9F82592F0F900380FEE /* KeychainAccess in Frameworks */ = {isa = PBXBuildFile; productRef = CABFA9F72592F0F900380FEE /* KeychainAccess */; }; CABFA9FD2592F13300380FEE /* LegibleError in Frameworks */ = {isa = PBXBuildFile; productRef = CABFA9FC2592F13300380FEE /* LegibleError */; }; + CABFAA2C2592FBFC00380FEE /* SettingsView.swift in Sources */ = {isa = PBXBuildFile; fileRef = CABFAA2A2592FBFC00380FEE /* SettingsView.swift */; }; + CABFAA2D2592FBFC00380FEE /* Configure.swift in Sources */ = {isa = PBXBuildFile; fileRef = CABFAA2B2592FBFC00380FEE /* Configure.swift */; }; CAD2E7A22449574E00113D76 /* XcodesApp.swift in Sources */ = {isa = PBXBuildFile; fileRef = CAD2E7A12449574E00113D76 /* XcodesApp.swift */; }; CAD2E7A42449574E00113D76 /* XcodeListView.swift in Sources */ = {isa = PBXBuildFile; fileRef = CAD2E7A32449574E00113D76 /* XcodeListView.swift */; }; CAD2E7A62449575000113D76 /* Assets.xcassets in Resources */ = {isa = PBXBuildFile; fileRef = CAD2E7A52449575000113D76 /* Assets.xcassets */; }; @@ -90,6 +92,8 @@ CABFA9B92592EEEA00380FEE /* Models.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = Models.swift; sourceTree = ""; }; CABFA9BA2592EEEA00380FEE /* DateFormatter+.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "DateFormatter+.swift"; sourceTree = ""; }; CABFA9D42592EF6300380FEE /* DECISIONS.md */ = {isa = PBXFileReference; lastKnownFileType = net.daringfireball.markdown; path = DECISIONS.md; sourceTree = ""; }; + CABFAA2A2592FBFC00380FEE /* SettingsView.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; name = SettingsView.swift; path = Xcodes/SettingsView.swift; sourceTree = SOURCE_ROOT; }; + CABFAA2B2592FBFC00380FEE /* Configure.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; name = Configure.swift; path = Xcodes/Backend/Configure.swift; sourceTree = SOURCE_ROOT; }; CAD2E79E2449574E00113D76 /* Xcodes.app */ = {isa = PBXFileReference; explicitFileType = wrapper.application; includeInIndex = 0; path = Xcodes.app; sourceTree = BUILT_PRODUCTS_DIR; }; CAD2E7A12449574E00113D76 /* XcodesApp.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = XcodesApp.swift; sourceTree = ""; }; CAD2E7A32449574E00113D76 /* XcodeListView.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = XcodeListView.swift; sourceTree = ""; }; @@ -164,6 +168,7 @@ isa = PBXGroup; children = ( CA378F982466567600A58CE0 /* AppState.swift */, + CABFAA2B2592FBFC00380FEE /* Configure.swift */, CABFA9BA2592EEEA00380FEE /* DateFormatter+.swift */, CABFA9B22592EEEA00380FEE /* Entry+.swift */, CABFA9A92592EEE900380FEE /* Environment.swift */, @@ -187,6 +192,7 @@ children = ( CAA1CB50255A5D16003FD669 /* SignIn */, CABFAA142592F73000380FEE /* XcodeList */, + CABFAA2A2592FBFC00380FEE /* SettingsView.swift */, ); path = Frontend; sourceTree = ""; @@ -394,11 +400,13 @@ CABFA9CE2592EEEA00380FEE /* Version+Xcode.swift in Sources */, CAA1CB49255A5C97003FD669 /* SignInSMSView.swift in Sources */, CAA1CB35255A5AD5003FD669 /* SignInCredentialsView.swift in Sources */, + CABFAA2D2592FBFC00380FEE /* Configure.swift in Sources */, CA73510D257BFCEF00EA9CF8 /* NSAttributedString+.swift in Sources */, CABFA9C22592EEEA00380FEE /* Promise+.swift in Sources */, CAA1CB4D255A5CFD003FD669 /* SignInPhoneListView.swift in Sources */, CABFA9CF2592EEEA00380FEE /* Process.swift in Sources */, CABFA9C72592EEEA00380FEE /* Entry+.swift in Sources */, + CABFAA2C2592FBFC00380FEE /* SettingsView.swift in Sources */, CABFA9C92592EEEA00380FEE /* URLRequest+Apple.swift in Sources */, CABFA9CC2592EEEA00380FEE /* Path+.swift in Sources */, CAD2E7A22449574E00113D76 /* XcodesApp.swift in Sources */, diff --git a/Xcodes/Backend/Configure.swift b/Xcodes/Backend/Configure.swift new file mode 100644 index 0000000..46e3d8c --- /dev/null +++ b/Xcodes/Backend/Configure.swift @@ -0,0 +1,5 @@ +public func configure(_ subject: Subject, configuration: (inout Subject) -> Void) -> Subject { + var copy = subject + configuration(©) + return copy +} diff --git a/Xcodes/Frontend/XcodeList/XcodeListView.swift b/Xcodes/Frontend/XcodeList/XcodeListView.swift index c6ab09a..1247397 100644 --- a/Xcodes/Frontend/XcodeList/XcodeListView.swift +++ b/Xcodes/Frontend/XcodeList/XcodeListView.swift @@ -67,11 +67,6 @@ struct XcodeListView: View { } .toolbar { ToolbarItemGroup(placement: .primaryAction) { - Button("Login", action: { self.appState.presentingSignInAlert = true }) - .sheet(isPresented: $appState.presentingSignInAlert) { - SignInCredentialsView(isPresented: $appState.presentingSignInAlert) - .environmentObject(appState) - } Button(action: appState.update) { Image(systemName: "arrow.clockwise") } diff --git a/Xcodes/SettingsView.swift b/Xcodes/SettingsView.swift new file mode 100644 index 0000000..597ad6d --- /dev/null +++ b/Xcodes/SettingsView.swift @@ -0,0 +1,62 @@ +import AppleAPI +import SwiftUI + +struct SettingsView: View { + @EnvironmentObject var appState: AppState + + var body: some 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: + 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) + } + Spacer() + } + .padding() + .navigationTitle("Settings") + .frame(width: 300) + .frame(minHeight: 300) + } +} + +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: "") + ) + }) + } + } +} diff --git a/Xcodes/XcodesApp.swift b/Xcodes/XcodesApp.swift index d7e97ae..5f27dbc 100644 --- a/Xcodes/XcodesApp.swift +++ b/Xcodes/XcodesApp.swift @@ -5,11 +5,14 @@ struct XcodesApp: App { @StateObject private var appState = AppState() var body: some Scene { - Group { - WindowGroup("Xcodes") { - XcodeListView() - .environmentObject(appState) - } + WindowGroup("Xcodes") { + XcodeListView() + .environmentObject(appState) + } + + Settings { + SettingsView() + .environmentObject(appState) } } } From 5149e817da8dca9d2e3f76335a22264351679703 Mon Sep 17 00:00:00 2001 From: Brandon Evans Date: Tue, 22 Dec 2020 22:06:18 -0700 Subject: [PATCH 2/3] 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()) } } } From bfb56660bfc24672ac19adf5adb7dbdc94e33656 Mon Sep 17 00:00:00 2001 From: Brandon Evans Date: Tue, 22 Dec 2020 22:11:42 -0700 Subject: [PATCH 3/3] Sign In instead of Login --- Xcodes/Backend/AppState.swift | 14 +++++++------- Xcodes/Backend/Environment.swift | 7 ------- Xcodes/Frontend/SignIn/SignInCredentialsView.swift | 2 +- Xcodes/SettingsView.swift | 2 +- 4 files changed, 9 insertions(+), 16 deletions(-) diff --git a/Xcodes/Backend/AppState.swift b/Xcodes/Backend/AppState.swift index fc5e81d..4ee7fb3 100644 --- a/Xcodes/Backend/AppState.swift +++ b/Xcodes/Backend/AppState.swift @@ -31,7 +31,7 @@ class AppState: ObservableObject { .eraseToAnyPublisher() } - func loginIfNeeded() -> AnyPublisher { + func signInIfNeeded() -> AnyPublisher { validateSession() .catch { (error) -> AnyPublisher in guard @@ -42,15 +42,15 @@ class AppState: ObservableObject { .eraseToAnyPublisher() } - return self.login(username: username, password: password) + return self.signIn(username: username, password: password) .map { _ in Void() } .eraseToAnyPublisher() } .eraseToAnyPublisher() } - func login(username: String, password: String) { - login(username: username, password: password) + func signIn(username: String, password: String) { + signIn(username: username, password: password) .sink( receiveCompletion: { _ in }, receiveValue: { _ in } @@ -58,7 +58,7 @@ class AppState: ObservableObject { .store(in: &cancellables) } - func login(username: String, password: String) -> AnyPublisher { + func signIn(username: String, password: String) -> AnyPublisher { try? Current.keychain.set(password, key: username) Current.defaults.set(username, forKey: "username") @@ -139,7 +139,7 @@ class AppState: ObservableObject { } } - func logOut() { + func signOut() { let username = Current.defaults.string(forKey: "username") Current.defaults.removeObject(forKey: "username") if let username = username { @@ -161,7 +161,7 @@ class AppState: ObservableObject { } public func update() -> AnyPublisher<[Xcode], Never> { - loginIfNeeded() + signInIfNeeded() .flatMap { // Wrap the Promise API in a Publisher for now Deferred { diff --git a/Xcodes/Backend/Environment.swift b/Xcodes/Backend/Environment.swift index d5d1467..166cbab 100644 --- a/Xcodes/Backend/Environment.swift +++ b/Xcodes/Backend/Environment.swift @@ -122,13 +122,6 @@ public struct Network { public func downloadTask(with convertible: URLRequestConvertible, to saveLocation: URL, resumingWith resumeData: Data?) -> (progress: Progress, promise: Promise<(saveLocation: URL, response: URLResponse)>) { return downloadTask(convertible, saveLocation, resumeData) } - -// public var validateSession: () -> Promise = client.validateSession -// -// public var login: (String, String) -> Promise = { client.login(accountName: $0, password: $1) } -// public func login(accountName: String, password: String) -> Promise { -// login(accountName, password) -// } } public struct Logging { diff --git a/Xcodes/Frontend/SignIn/SignInCredentialsView.swift b/Xcodes/Frontend/SignIn/SignInCredentialsView.swift index 05a2a2b..7b7c49c 100644 --- a/Xcodes/Frontend/SignIn/SignInCredentialsView.swift +++ b/Xcodes/Frontend/SignIn/SignInCredentialsView.swift @@ -28,7 +28,7 @@ struct SignInCredentialsView: View { Spacer() Button("Cancel") { isPresented = false } .keyboardShortcut(.cancelAction) - Button("Next") { appState.login(username: username, password: password) } + Button("Next") { appState.signIn(username: username, password: password) } .disabled(username.isEmpty) .keyboardShortcut(.defaultAction) } diff --git a/Xcodes/SettingsView.swift b/Xcodes/SettingsView.swift index 62d7eb2..373561f 100644 --- a/Xcodes/SettingsView.swift +++ b/Xcodes/SettingsView.swift @@ -10,7 +10,7 @@ struct SettingsView: View { VStack(alignment: .leading) { if let username = Current.defaults.string(forKey: "username") { Text(username) - Button("Sign Out", action: appState.logOut) + Button("Sign Out", action: appState.signOut) } else { Button("Sign In", action: { self.appState.presentingSignInAlert = true }) .sheet(isPresented: $appState.presentingSignInAlert) {