diff --git a/Xcodes.xcodeproj/project.pbxproj b/Xcodes.xcodeproj/project.pbxproj index e5e975a..94ceedb 100644 --- a/Xcodes.xcodeproj/project.pbxproj +++ b/Xcodes.xcodeproj/project.pbxproj @@ -9,6 +9,7 @@ /* Begin PBXBuildFile section */ 536CFDD2263C94DE00026CE0 /* SignedInView.swift in Sources */ = {isa = PBXBuildFile; fileRef = 536CFDD1263C94DE00026CE0 /* SignedInView.swift */; }; 536CFDD4263C9A8000026CE0 /* XcodesSheet.swift in Sources */ = {isa = PBXBuildFile; fileRef = 536CFDD3263C9A8000026CE0 /* XcodesSheet.swift */; }; + 53CBAB2C263DCC9100410495 /* XcodesAlert.swift in Sources */ = {isa = PBXBuildFile; fileRef = 53CBAB2B263DCC9100410495 /* XcodesAlert.swift */; }; 63EAA4EB259944450046AB8F /* ProgressButton.swift in Sources */ = {isa = PBXBuildFile; fileRef = 63EAA4EA259944450046AB8F /* ProgressButton.swift */; }; CA11E7BA2598476C00D2EE1C /* XcodeCommands.swift in Sources */ = {isa = PBXBuildFile; fileRef = CA11E7B92598476C00D2EE1C /* XcodeCommands.swift */; }; CA2518EC25A7FF2B00F08414 /* AppStateUpdateTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = CA2518EB25A7FF2B00F08414 /* AppStateUpdateTests.swift */; }; @@ -162,6 +163,7 @@ /* Begin PBXFileReference section */ 536CFDD1263C94DE00026CE0 /* SignedInView.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SignedInView.swift; sourceTree = ""; }; 536CFDD3263C9A8000026CE0 /* XcodesSheet.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = XcodesSheet.swift; sourceTree = ""; }; + 53CBAB2B263DCC9100410495 /* XcodesAlert.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = XcodesAlert.swift; sourceTree = ""; }; 63EAA4EA259944450046AB8F /* ProgressButton.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ProgressButton.swift; sourceTree = ""; }; CA11E7B92598476C00D2EE1C /* XcodeCommands.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = XcodeCommands.swift; sourceTree = ""; }; CA2518EB25A7FF2B00F08414 /* AppStateUpdateTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = AppStateUpdateTests.swift; sourceTree = ""; }; @@ -316,6 +318,7 @@ 63EAA4EA259944450046AB8F /* ProgressButton.swift */, CA452BAF259FD9770072DFA4 /* ProgressIndicator.swift */, 536CFDD3263C9A8000026CE0 /* XcodesSheet.swift */, + 53CBAB2B263DCC9100410495 /* XcodesAlert.swift */, ); path = Common; sourceTree = ""; @@ -789,6 +792,7 @@ CABFA9C52592EEEA00380FEE /* FileManager+.swift in Sources */, CABFA9CD2592EEEA00380FEE /* Foundation.swift in Sources */, CA9FF8872595607900E47BAF /* InstalledXcode.swift in Sources */, + 53CBAB2C263DCC9100410495 /* XcodesAlert.swift in Sources */, CA42DD6E25AEA8B200BC0B0C /* Logger.swift in Sources */, CA61A6E0259835580008926E /* Xcode.swift in Sources */, CAE4247F259A666100B8B246 /* MainWindow.swift in Sources */, diff --git a/Xcodes/Backend/AppState+Install.swift b/Xcodes/Backend/AppState+Install.swift index 4b68976..d295d68 100644 --- a/Xcodes/Backend/AppState+Install.swift +++ b/Xcodes/Backend/AppState+Install.swift @@ -208,6 +208,7 @@ extension AppState { .handleEvents(receiveCompletion: { [unowned self] completion in if case let .failure(error) = completion { self.error = error + self.presentedAlert = .generic(title: "Unable to install archived Xcode", message: error.legibleLocalizedDescription) } }) .catch { _ in @@ -344,6 +345,7 @@ extension AppState { receiveCompletion: { completion in if case let .failure(error) = completion { self.error = error + self.presentedAlert = .generic(title: "Unable to perform post install steps", message: error.legibleLocalizedDescription) } }, receiveValue: {} @@ -396,6 +398,7 @@ extension AppState { ) } } + self.presentedAlert = .privilegedHelper } return helperInstallConsentSubject diff --git a/Xcodes/Backend/AppState+Update.swift b/Xcodes/Backend/AppState+Update.swift index db8bc0a..62a327d 100644 --- a/Xcodes/Backend/AppState+Update.swift +++ b/Xcodes/Backend/AppState+Update.swift @@ -4,6 +4,7 @@ import Path import Version import SwiftSoup import struct XCModel.Xcode +import AppleAPI extension AppState { @@ -43,7 +44,11 @@ extension AppState { receiveCompletion: { [unowned self] completion in switch completion { case let .failure(error): - self.error = error + // Prevent setting the app state error if it is an invalid session, we will present the sign in view instead + if error as? AuthenticationError != .invalidSession { + self.error = error + self.presentedAlert = .generic(title: "Unable to update selected Xcode", message: error.legibleLocalizedDescription) + } case .finished: Current.defaults.setDate(Current.date(), forKey: "lastUpdated") } diff --git a/Xcodes/Backend/AppState.swift b/Xcodes/Backend/AppState.swift index f928faa..1426042 100644 --- a/Xcodes/Backend/AppState.swift +++ b/Xcodes/Backend/AppState.swift @@ -45,7 +45,7 @@ class AppState: ObservableObject { @Published var isProcessingAuthRequest = false @Published var secondFactorData: SecondFactorData? @Published var xcodeBeingConfirmedForUninstallation: Xcode? - @Published var xcodeBeingConfirmedForInstallCancellation: Xcode? + @Published var presentedAlert: XcodesAlert? @Published var helperInstallState: HelperInstallState = .notInstalled /// Whether the user is being prepared for the helper installation alert with an explanation. /// This closure will be performed after the user chooses whether or not to proceed. @@ -133,7 +133,7 @@ class AppState: ObservableObject { authError = nil signIn(username: username, password: password) .sink( - receiveCompletion: { _ in }, + receiveCompletion: { _ in }, receiveValue: { _ in } ) .store(in: &cancellables) @@ -209,13 +209,8 @@ class AppState: ObservableObject { private func handleAuthenticationFlowCompletion(_ completion: Subscribers.Completion) { switch completion { case let .failure(error): - if case .invalidUsernameOrPassword = error as? AuthenticationError, - let username = savedUsername { - // 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) - Current.defaults.removeObject(forKey: "username") - } - + // remove saved username and any stored keychain password if authentication fails so it doesn't try again. + clearLoginCredentials() Logger.appState.error("Authentication error: \(error.legibleDescription)") self.authError = error case .finished: @@ -230,10 +225,7 @@ class AppState: ObservableObject { } func signOut() { - if let username = savedUsername { - try? Current.keychain.remove(username) - } - Current.defaults.removeObject(forKey: "username") + clearLoginCredentials() AppleAPI.Current.network.session.configuration.httpCookieStorage?.removeCookies(since: .distantPast) authenticationState = .unauthenticated } @@ -256,6 +248,7 @@ class AppState: ObservableObject { guard userConsented else { return } self.installHelperIfNecessary(shouldPrepareUserForHelperInstallation: false) } + presentedAlert = .privilegedHelper return } @@ -264,6 +257,7 @@ class AppState: ObservableObject { receiveCompletion: { [unowned self] completion in if case let .failure(error) = completion { self.error = error + self.presentedAlert = .generic(title: "Unable to install helper", message: error.legibleLocalizedDescription) } }, receiveValue: {} @@ -342,6 +336,7 @@ class AppState: ObservableObject { // Prevent setting the app state error if it is an invalid session, we will present the sign in view instead if error as? AuthenticationError != .invalidSession { self.error = error + self.presentedAlert = .generic(title: "Unable to install Xcode", message: error.legibleLocalizedDescription) } if let index = self.allXcodes.firstIndex(where: { $0.id == id }) { self.allXcodes[index].installState = .notInstalled @@ -385,6 +380,7 @@ class AppState: ObservableObject { receiveCompletion: { [unowned self] completion in if case let .failure(error) = completion { self.error = error + self.presentedAlert = .generic(title: "Unable to uninstall Xcode", message: error.legibleLocalizedDescription) } self.uninstallPublisher = nil }, @@ -416,6 +412,7 @@ class AppState: ObservableObject { guard userConsented else { return } self.select(id: id, shouldPrepareUserForHelperInstallation: false) } + presentedAlert = .privilegedHelper return } @@ -435,6 +432,7 @@ class AppState: ObservableObject { receiveCompletion: { [unowned self] completion in if case let .failure(error) = completion { self.error = error + self.presentedAlert = .generic(title: "Unable to select Xcode", message: error.legibleLocalizedDescription) } self.selectPublisher = nil }, @@ -565,6 +563,15 @@ class AppState: ObservableObject { .eraseToAnyPublisher() } + /// removes saved username and credentials stored in keychain + private func clearLoginCredentials() { + if let username = savedUsername { + try? Current.keychain.remove(username) + } + Current.defaults.removeObject(forKey: "username") + + } + // MARK: - Nested Types struct AlertContent: Identifiable { diff --git a/Xcodes/Backend/XcodeCommands.swift b/Xcodes/Backend/XcodeCommands.swift index 7f32377..58ee191 100644 --- a/Xcodes/Backend/XcodeCommands.swift +++ b/Xcodes/Backend/XcodeCommands.swift @@ -61,7 +61,7 @@ struct CancelInstallButton: View { private func cancelInstall() { guard let xcode = xcode else { return } - appState.xcodeBeingConfirmedForInstallCancellation = xcode + appState.presentedAlert = .cancelInstall(xcode: xcode) } } diff --git a/Xcodes/Frontend/Common/XcodesAlert.swift b/Xcodes/Frontend/Common/XcodesAlert.swift new file mode 100644 index 0000000..c57d089 --- /dev/null +++ b/Xcodes/Frontend/Common/XcodesAlert.swift @@ -0,0 +1,15 @@ +import Foundation + +enum XcodesAlert: Identifiable { + case cancelInstall(xcode: Xcode) + case privilegedHelper + case generic(title: String, message: String) + + var id: Int { + switch self { + case .cancelInstall: return 1 + case .privilegedHelper: return 2 + case .generic: return 3 + } + } +} diff --git a/Xcodes/Frontend/MainWindow.swift b/Xcodes/Frontend/MainWindow.swift index 954b221..fe87402 100644 --- a/Xcodes/Frontend/MainWindow.swift +++ b/Xcodes/Frontend/MainWindow.swift @@ -48,51 +48,9 @@ struct MainWindow: View { .environmentObject(appState) } } - // This overlay is only here to work around the one-alert-per-view limitation - .overlay( - Color.clear - // This particular alert could be added specifically to InstallationStepView and CancelInstallButton _except_ for when CancelInstallButton is used in the Xcode CommandMenu, so it's here for now. - .alert(item: $appState.xcodeBeingConfirmedForInstallCancellation) { xcode in - Alert(title: Text("Are you sure you want to stop the installation of Xcode \(xcode.description)?"), - message: Text("Any progress will be discarded."), - primaryButton: .destructive(Text("Stop Installation"), action: { self.appState.cancelInstall(id: xcode.id) }), - secondaryButton: .cancel(Text("Cancel"))) - } - ) - // This overlay is only here to work around the one-alert-per-view limitation - .overlay( - Color.clear - .alert(isPresented: $appState.isPreparingUserForActionRequiringHelper.isNotNil) { - Alert( - title: Text("Privileged Helper"), - message: Text("Xcodes uses a separate privileged helper to perform tasks as root. These are things that would require sudo on the command line, including post-install steps and switching Xcode versions with xcode-select.\n\nYou'll be prompted for your macOS account password to install it."), - primaryButton: .default(Text("Install"), action: { - // The isPreparingUserForActionRequiringHelper closure is set to nil by the alert's binding when its dismissed. - // We need to capture it to be invoked after that happens. - let helperAction = appState.isPreparingUserForActionRequiringHelper - DispatchQueue.main.async { - // This really shouldn't be nil, but sometimes this alert is being shown twice and I don't know why. - // There are some DispatchQueue.main.async's scattered around which make this better but in some situations it's still happening. - // When that happens, the second time the user clicks an alert button isPreparingUserForActionRequiringHelper will be nil. - // To at least not crash, we're using ? - helperAction?(true) - } - }), - secondaryButton: .cancel { - // The isPreparingUserForActionRequiringHelper closure is set to nil by the alert's binding when its dismissed. - // We need to capture it to be invoked after that happens. - let helperAction = appState.isPreparingUserForActionRequiringHelper - DispatchQueue.main.async { - // This really shouldn't be nil, but sometimes this alert is being shown twice and I don't know why. - // There are some DispatchQueue.main.async's scattered around which make this better but in some situations it's still happening. - // When that happens, the second time the user clicks an alert button isPreparingUserForActionRequiringHelper will be nil. - // To at least not crash, we're using ? - helperAction?(false) - } - } - ) - } - ) + .alert(item: $appState.presentedAlert, content: { presentedAlert in + alert(for: presentedAlert) + }) // I'm expecting to be able to use this modifier on a List row, but using it at the top level here is the only way that has made XcodeCommands work so far. // FB8954571 focusedValue(_:_:) on List row doesn't propagate value to @FocusedValue .focusedValue(\.selectedXcode, SelectedXcode(appState.allXcodes.first { $0.id == selectedXcodeID })) @@ -136,6 +94,63 @@ struct MainWindow: View { .frame(width: 400) } } + + private func alert(for alertType: XcodesAlert) -> Alert { + switch alertType { + case let .cancelInstall(xcode): + return Alert( + title: Text("Are you sure you want to stop the installation of Xcode \(xcode.description)?"), + message: Text("Any progress will be discarded."), + primaryButton: .destructive( + Text("Stop Installation"), + action: { + self.appState.cancelInstall(id: xcode.id) + } + ), + secondaryButton: .cancel(Text("Cancel")) + ) + case .privilegedHelper: + return Alert( + title: Text("Privileged Helper"), + message: Text("Xcodes uses a separate privileged helper to perform tasks as root. These are things that would require sudo on the command line, including post-install steps and switching Xcode versions with xcode-select.\n\nYou'll be prompted for your macOS account password to install it."), + primaryButton: .default(Text("Install"), action: { + // The isPreparingUserForActionRequiringHelper closure is set to nil by the alert's binding when its dismissed. + // We need to capture it to be invoked after that happens. + let helperAction = appState.isPreparingUserForActionRequiringHelper + DispatchQueue.main.async { + // This really shouldn't be nil, but sometimes this alert is being shown twice and I don't know why. + // There are some DispatchQueue.main.async's scattered around which make this better but in some situations it's still happening. + // When that happens, the second time the user clicks an alert button isPreparingUserForActionRequiringHelper will be nil. + // To at least not crash, we're using ? + helperAction?(true) + appState.presentedAlert = nil + } + }), + secondaryButton: .cancel { + // The isPreparingUserForActionRequiringHelper closure is set to nil by the alert's binding when its dismissed. + // We need to capture it to be invoked after that happens. + let helperAction = appState.isPreparingUserForActionRequiringHelper + DispatchQueue.main.async { + // This really shouldn't be nil, but sometimes this alert is being shown twice and I don't know why. + // There are some DispatchQueue.main.async's scattered around which make this better but in some situations it's still happening. + // When that happens, the second time the user clicks an alert button isPreparingUserForActionRequiringHelper will be nil. + // To at least not crash, we're using ? + helperAction?(false) + appState.presentedAlert = nil + } + } + ) + case let .generic(title, message): + return Alert( + title: Text(title), + message: Text(message), + dismissButton: .default( + Text("Ok"), + action: { appState.presentedAlert = nil } + ) + ) + } + } } struct MainWindow_Previews: PreviewProvider { diff --git a/Xcodes/Frontend/XcodeList/XcodeListViewRow.swift b/Xcodes/Frontend/XcodeList/XcodeListViewRow.swift index 9b0c645..1b7e477 100644 --- a/Xcodes/Frontend/XcodeList/XcodeListViewRow.swift +++ b/Xcodes/Frontend/XcodeList/XcodeListViewRow.swift @@ -112,7 +112,7 @@ struct XcodeListViewRow: View { InstallationStepRowView( installationStep: installationStep, highlighted: selected, - cancel: { appState.xcodeBeingConfirmedForInstallCancellation = xcode } + cancel: { appState.presentedAlert = .cancelInstall(xcode: xcode) } ) } }