diff --git a/CLAUDE.md b/CLAUDE.md index d3e46b2..3606719 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -4,7 +4,7 @@ - `SWIFT_DEFAULT_ACTOR_ISOLATION = MainActor` is set on both app and test targets — don't add explicit `@MainActor` to production code or test structs/functions, it's already the default -## Swift Testing Migration +## Swift Testing Conventions - Place `@Test` and `@Suite` annotations on the line **above** the declaration, not inline - Use `// when` and `// then` comment blocks; skip `// given` (assumed from context) diff --git a/PPPC Utility.xcodeproj/project.pbxproj b/PPPC Utility.xcodeproj/project.pbxproj index 11b3e67..69780f1 100644 --- a/PPPC Utility.xcodeproj/project.pbxproj +++ b/PPPC Utility.xcodeproj/project.pbxproj @@ -560,7 +560,10 @@ PRODUCT_BUNDLE_IDENTIFIER = "com.jamf.PPPC-UtilityTests"; PRODUCT_NAME = "$(TARGET_NAME)"; PROVISIONING_PROFILE_SPECIFIER = ""; + SWIFT_APPROACHABLE_CONCURRENCY = YES; + SWIFT_DEFAULT_ACTOR_ISOLATION = MainActor; SWIFT_OPTIMIZATION_LEVEL = "-Onone"; + SWIFT_STRICT_CONCURRENCY = complete; SWIFT_VERSION = 5.0; TEST_HOST = "$(BUILT_PRODUCTS_DIR)/PPPC Utility.app/Contents/MacOS/PPPC Utility"; }; @@ -585,6 +588,9 @@ PRODUCT_BUNDLE_IDENTIFIER = "com.jamf.PPPC-UtilityTests"; PRODUCT_NAME = "$(TARGET_NAME)"; PROVISIONING_PROFILE_SPECIFIER = ""; + SWIFT_APPROACHABLE_CONCURRENCY = YES; + SWIFT_DEFAULT_ACTOR_ISOLATION = MainActor; + SWIFT_STRICT_CONCURRENCY = complete; SWIFT_VERSION = 5.0; TEST_HOST = "$(BUILT_PRODUCTS_DIR)/PPPC Utility.app/Contents/MacOS/PPPC Utility"; }; @@ -727,7 +733,10 @@ PRODUCT_BUNDLE_IDENTIFIER = com.jamf.opensource.pppcutility; PRODUCT_NAME = "$(TARGET_NAME)"; PROVISIONING_PROFILE_SPECIFIER = ""; - SWIFT_VERSION = 5.0; + SWIFT_APPROACHABLE_CONCURRENCY = YES; + SWIFT_DEFAULT_ACTOR_ISOLATION = MainActor; + SWIFT_STRICT_CONCURRENCY = complete; + SWIFT_VERSION = 6.0; }; name = Debug; }; @@ -751,7 +760,10 @@ PRODUCT_BUNDLE_IDENTIFIER = com.jamf.opensource.pppcutility; PRODUCT_NAME = "$(TARGET_NAME)"; PROVISIONING_PROFILE_SPECIFIER = ""; - SWIFT_VERSION = 5.0; + SWIFT_APPROACHABLE_CONCURRENCY = YES; + SWIFT_DEFAULT_ACTOR_ISOLATION = MainActor; + SWIFT_STRICT_CONCURRENCY = complete; + SWIFT_VERSION = 6.0; }; name = Release; }; diff --git a/PPPC UtilityTests/ModelTests/ModelTests.swift b/PPPC UtilityTests/ModelTests/ModelTests.swift index 8e838cb..c2ad6f1 100644 --- a/PPPC UtilityTests/ModelTests/ModelTests.swift +++ b/PPPC UtilityTests/ModelTests/ModelTests.swift @@ -38,12 +38,12 @@ struct ModelTests { // MARK: - tests for getExecutableFrom* @Test - func getExecutableBasedOnIdentifierAndCodeRequirement_BundleIdentifierType() { + func getExecutableBasedOnIdentifierAndCodeRequirement_BundleIdentifierType() async { let identifier = "com.example.App" let codeRequirement = "testCodeRequirement" // when - let executable = model.getExecutableFrom(identifier: identifier, codeRequirement: codeRequirement) + let executable = await model.getExecutableFrom(identifier: identifier, codeRequirement: codeRequirement) // then #expect(executable.displayName == "App") @@ -52,12 +52,12 @@ struct ModelTests { } @Test - func getExecutableBasedOnIdentifierAndCodeRequirement_PathIdentifierType() { + func getExecutableBasedOnIdentifierAndCodeRequirement_PathIdentifierType() async { let identifier = "/myGreatPath/Awesome/Binary" let codeRequirement = "testCodeRequirement" // when - let executable = model.getExecutableFrom(identifier: identifier, codeRequirement: codeRequirement) + let executable = await model.getExecutableFrom(identifier: identifier, codeRequirement: codeRequirement) // then #expect(executable.displayName == "Binary") @@ -66,12 +66,12 @@ struct ModelTests { } @Test - func getExecutableFromComputerBasedOnIdentifier() { + func getExecutableFromComputerBasedOnIdentifier() async { let identifier = "com.apple.Safari" let codeRequirement = "randomReq" // when - let executable = model.getExecutableFrom(identifier: identifier, codeRequirement: codeRequirement) + let executable = await model.getExecutableFrom(identifier: identifier, codeRequirement: codeRequirement) // then #expect(executable.displayName == "Safari") @@ -80,10 +80,10 @@ struct ModelTests { } @Test - func getExecutableFromSelectedExecutables() { + func getExecutableFromSelectedExecutables() async { let expectedIdentifier = "com.something.1" - let executable = model.getExecutableFrom(identifier: expectedIdentifier, codeRequirement: "testReq") - let executableSecond = model.getExecutableFrom(identifier: "com.something.2", codeRequirement: "testReq2") + let executable = await model.getExecutableFrom(identifier: expectedIdentifier, codeRequirement: "testReq") + let executableSecond = await model.getExecutableFrom(identifier: "com.something.2", codeRequirement: "testReq2") model.selectedExecutables = [executable, executableSecond] // when @@ -97,11 +97,11 @@ struct ModelTests { } @Test - func getExecutableFromSelectedExecutables_Path() { + func getExecutableFromSelectedExecutables_Path() async { let expectedIdentifier = "/path/something/Special" - let executableOneMore = model.getExecutableFrom(identifier: "/path/something/Special1", codeRequirement: "testReq") - let executable = model.getExecutableFrom(identifier: expectedIdentifier, codeRequirement: "testReq") - let executableSecond = model.getExecutableFrom(identifier: "com.something.2", codeRequirement: "testReq2") + let executableOneMore = await model.getExecutableFrom(identifier: "/path/something/Special1", codeRequirement: "testReq") + let executable = await model.getExecutableFrom(identifier: expectedIdentifier, codeRequirement: "testReq") + let executableSecond = await model.getExecutableFrom(identifier: "com.something.2", codeRequirement: "testReq2") model.selectedExecutables = [executableOneMore, executable, executableSecond] // when @@ -184,11 +184,11 @@ struct ModelTests { // MARK: - tests for importProfile @Test - func importProfileUsingAuthorizationKeyAllow() { + func importProfileUsingAuthorizationKeyAllow() async { let profile = TCCProfileBuilder().buildProfile(authorization: .allow) // when - model.importProfile(tccProfile: profile) + await model.importProfile(tccProfile: profile) // then #expect(model.selectedExecutables.count == 1) @@ -196,11 +196,11 @@ struct ModelTests { } @Test - func importProfileUsingAuthorizationKeyDeny() { + func importProfileUsingAuthorizationKeyDeny() async { let profile = TCCProfileBuilder().buildProfile(authorization: .deny) // when - model.importProfile(tccProfile: profile) + await model.importProfile(tccProfile: profile) // then #expect(model.selectedExecutables.count == 1) @@ -208,11 +208,11 @@ struct ModelTests { } @Test - func importProfileUsingAuthorizationKeyAllowStandardUsers() { + func importProfileUsingAuthorizationKeyAllowStandardUsers() async { let profile = TCCProfileBuilder().buildProfile(authorization: .allowStandardUserToSetSystemService) // when - model.importProfile(tccProfile: profile) + await model.importProfile(tccProfile: profile) // then #expect(model.selectedExecutables.count == 1) @@ -220,11 +220,11 @@ struct ModelTests { } @Test - func importProfileUsingLegacyAllowKeyTrue() { + func importProfileUsingLegacyAllowKeyTrue() async { let profile = TCCProfileBuilder().buildProfile(allowed: true) // when - model.importProfile(tccProfile: profile) + await model.importProfile(tccProfile: profile) // then #expect(model.selectedExecutables.count == 1) @@ -232,11 +232,11 @@ struct ModelTests { } @Test - func importProfileUsingLegacyAllowKeyFalse() { + func importProfileUsingLegacyAllowKeyFalse() async { let profile = TCCProfileBuilder().buildProfile(allowed: false) // when - model.importProfile(tccProfile: profile) + await model.importProfile(tccProfile: profile) // then #expect(model.selectedExecutables.count == 1) @@ -244,11 +244,11 @@ struct ModelTests { } @Test - func importProfileUsingAuthorizationKeyThatIsInvalid() { + func importProfileUsingAuthorizationKeyThatIsInvalid() async { let profile = TCCProfileBuilder().buildProfile(authorization: "invalidkey") // when - model.importProfile(tccProfile: profile) + await model.importProfile(tccProfile: profile) // then #expect(model.selectedExecutables.count == 1) @@ -256,18 +256,18 @@ struct ModelTests { } @Test - func importProfileUsingAuthorizationKeyTranslatesToAppleEvents() { + func importProfileUsingAuthorizationKeyTranslatesToAppleEvents() async { let profile = TCCProfileBuilder().buildProfile(authorization: "deny") // when - model.importProfile(tccProfile: profile) + await model.importProfile(tccProfile: profile) // then #expect(model.selectedExecutables.count == 1) #expect(model.selectedExecutables.first?.policy.SystemPolicyAllFiles == "Deny") } - // MARK: - tests for profileToString + // MARK: - tests for policyFromString @Test func policyWhenUsingAllow() { diff --git a/PPPC UtilityTests/NetworkingTests/NetworkAuthManagerTests.swift b/PPPC UtilityTests/NetworkingTests/NetworkAuthManagerTests.swift index 4eba732..efa97e1 100644 --- a/PPPC UtilityTests/NetworkingTests/NetworkAuthManagerTests.swift +++ b/PPPC UtilityTests/NetworkingTests/NetworkAuthManagerTests.swift @@ -85,7 +85,7 @@ struct NetworkAuthManagerTests { networking.errorToThrow = NetworkingError.serverResponse(404, "No such page") // default is that bearer auth is supported. - let firstCheckBearerAuthSupported = await authManager.bearerAuthSupported() + let firstCheckBearerAuthSupported = authManager.bearerAuthSupported() #expect(firstCheckBearerAuthSupported) // when/then @@ -94,7 +94,7 @@ struct NetworkAuthManagerTests { } // The authManager should now know that bearer auth is not supported - let secondCheckBearerAuthSupported = await authManager.bearerAuthSupported() + let secondCheckBearerAuthSupported = authManager.bearerAuthSupported() #expect(!secondCheckBearerAuthSupported) } diff --git a/PPPC UtilityTests/TCCProfileImporterTests/TCCProfileImporterTests.swift b/PPPC UtilityTests/TCCProfileImporterTests/TCCProfileImporterTests.swift index 4f1af27..042d5ac 100644 --- a/PPPC UtilityTests/TCCProfileImporterTests/TCCProfileImporterTests.swift +++ b/PPPC UtilityTests/TCCProfileImporterTests/TCCProfileImporterTests.swift @@ -40,15 +40,13 @@ struct TCCProfileImporterTests { let tccProfileImporter = TCCProfileImporter() let resourceURL = try getResourceProfile(fileName: "TestTCCProfileSigned-Broken") - tccProfileImporter.decodeTCCProfile(fileUrl: resourceURL) { tccProfileResult in - switch tccProfileResult { - case .success: - Issue.record("Malformed profile should not succeed") - case .failure(let tccProfileError): - if case TCCProfileImportError.invalidProfileFile = tccProfileError { - } else { - Issue.record("Expected invalidProfileFile error, got \(tccProfileError)") - } + do { + _ = try tccProfileImporter.decodeTCCProfile(fileUrl: resourceURL) + Issue.record("Malformed profile should not succeed") + } catch { + if case TCCProfileImportError.invalidProfileFile = error { + } else { + Issue.record("Expected invalidProfileFile error, got \(error)") } } } @@ -59,13 +57,11 @@ struct TCCProfileImporterTests { let resourceURL = try getResourceProfile(fileName: "TestTCCUnsignedProfile-Empty") let expectedTCCProfileError = TCCProfileImportError.invalidProfileFile(description: "PayloadContent") - tccProfileImporter.decodeTCCProfile(fileUrl: resourceURL) { tccProfileResult in - switch tccProfileResult { - case .success: - Issue.record("Empty Content, it shouldn't be success") - case .failure(let tccProfileError): - #expect(tccProfileError.localizedDescription == expectedTCCProfileError.localizedDescription) - } + do { + _ = try tccProfileImporter.decodeTCCProfile(fileUrl: resourceURL) + Issue.record("Empty Content, it shouldn't be success") + } catch { + #expect(error.localizedDescription == expectedTCCProfileError.localizedDescription) } } @@ -74,15 +70,9 @@ struct TCCProfileImporterTests { let tccProfileImporter = TCCProfileImporter() let resourceURL = try getResourceProfile(fileName: "TestTCCUnsignedProfile") - tccProfileImporter.decodeTCCProfile(fileUrl: resourceURL) { tccProfileResult in - switch tccProfileResult { - case .success(let tccProfile): - #expect(!tccProfile.content.isEmpty) - #expect(!tccProfile.content[0].services.isEmpty) - case .failure(let tccProfileError): - Issue.record("Unable to read tccProfile \(tccProfileError.localizedDescription)") - } - } + let tccProfile = try tccProfileImporter.decodeTCCProfile(fileUrl: resourceURL) + #expect(!tccProfile.content.isEmpty) + #expect(!tccProfile.content[0].services.isEmpty) } @Test @@ -90,15 +80,9 @@ struct TCCProfileImporterTests { let tccProfileImporter = TCCProfileImporter() let resourceURL = try getResourceProfile(fileName: "TestTCCUnsignedProfile-allLower") - tccProfileImporter.decodeTCCProfile(fileUrl: resourceURL) { tccProfileResult in - switch tccProfileResult { - case .success(let tccProfile): - #expect(!tccProfile.content.isEmpty) - #expect(!tccProfile.content[0].services.isEmpty) - case .failure(let tccProfileError): - Issue.record("Unable to read tccProfile \(tccProfileError.localizedDescription)") - } - } + let tccProfile = try tccProfileImporter.decodeTCCProfile(fileUrl: resourceURL) + #expect(!tccProfile.content.isEmpty) + #expect(!tccProfile.content[0].services.isEmpty) } @Test @@ -107,13 +91,11 @@ struct TCCProfileImporterTests { let resourceURL = try getResourceProfile(fileName: "TestTCCUnsignedProfile-Broken") let expectedTCCProfileError = TCCProfileImportError.invalidProfileFile(description: "The given data was not a valid property list.") - tccProfileImporter.decodeTCCProfile(fileUrl: resourceURL) { tccProfileResult in - switch tccProfileResult { - case .success: - Issue.record("Broken Unsigned Profile, it shouldn't be success") - case .failure(let tccProfileError): - #expect(tccProfileError.localizedDescription == expectedTCCProfileError.localizedDescription) - } + do { + _ = try tccProfileImporter.decodeTCCProfile(fileUrl: resourceURL) + Issue.record("Broken Unsigned Profile, it shouldn't be success") + } catch { + #expect(error.localizedDescription == expectedTCCProfileError.localizedDescription) } } diff --git a/PPPC UtilityTests/TCCProfileImporterTests/TCCProfileTests.swift b/PPPC UtilityTests/TCCProfileImporterTests/TCCProfileTests.swift index 761e274..45e45f2 100644 --- a/PPPC UtilityTests/TCCProfileImporterTests/TCCProfileTests.swift +++ b/PPPC UtilityTests/TCCProfileImporterTests/TCCProfileTests.swift @@ -154,7 +154,7 @@ struct TCCProfileTests { // unit tests for handling both Auth and allowed keys should fail? @Test - func settingLegacyAllowValueNullifiesAuthorization() throws { + func settingLegacyAllowValueNullifiesAuthorization() { var tccPolicy = TCCPolicy(identifier: "id", codeRequirement: "req", receiverIdentifier: "recId", receiverCodeRequirement: "recreq") tccPolicy.authorization = .allow @@ -180,12 +180,12 @@ struct TCCProfileTests { } @Test - func jamfProAPIData() throws { + func jamfProAPIData() async throws { let tccProfile = TCCProfileBuilder().buildProfile(allowed: false, authorization: .allow) let expected = try loadTextFile(fileName: "TestTCCProfileForJamfProAPI").trimmingCharacters(in: .whitespacesAndNewlines) // when - let data = try tccProfile.jamfProAPIData(signingIdentity: nil, site: nil) + let data = try await tccProfile.jamfProAPIData(signingIdentity: nil, site: nil) // then let xmlString = String(data: data, encoding: .utf8) diff --git a/Source/AppDelegate.swift b/Source/AppDelegate.swift index ec32034..041ac5b 100644 --- a/Source/AppDelegate.swift +++ b/Source/AppDelegate.swift @@ -27,7 +27,7 @@ import Cocoa -@NSApplicationMain +@main class AppDelegate: NSObject, NSApplicationDelegate { func applicationDidFinishLaunching(_ aNotification: Notification) {} diff --git a/Source/Model/Model.swift b/Source/Model/Model.swift index 763927f..a590d97 100644 --- a/Source/Model/Model.swift +++ b/Source/Model/Model.swift @@ -37,34 +37,25 @@ import OSLog let logger = Logger.Model - func getAppleEventChoices(executable: Executable) -> [Executable] { + func getAppleEventChoices(executable: Executable) async -> [Executable] { var executables: [Executable] = [] - loadExecutable(url: URL(fileURLWithPath: "/System/Library/CoreServices/System Events.app")) { result in - switch result { - case .success(let executable): - executables.append(executable) - case .failure(let error): - self.logger.error("\(error)") - } + do { + executables.append(try await loadExecutable(url: URL(fileURLWithPath: "/System/Library/CoreServices/System Events.app"))) + } catch { + self.logger.error("\(error)") } - loadExecutable(url: URL(fileURLWithPath: "/System/Library/CoreServices/SystemUIServer.app")) { result in - switch result { - case .success(let executable): - executables.append(executable) - case .failure(let error): - self.logger.error("\(error)") - } + do { + executables.append(try await loadExecutable(url: URL(fileURLWithPath: "/System/Library/CoreServices/SystemUIServer.app"))) + } catch { + self.logger.error("\(error)") } - loadExecutable(url: URL(fileURLWithPath: "/System/Library/CoreServices/Finder.app")) { result in - switch result { - case .success(let executable): - executables.append(executable) - case .failure(let error): - self.logger.error("\(error)") - } + do { + executables.append(try await loadExecutable(url: URL(fileURLWithPath: "/System/Library/CoreServices/Finder.app"))) + } catch { + self.logger.error("\(error)") } let others = store.values.filter { $0 != executable && !Set(executables).contains($0) } @@ -87,17 +78,16 @@ struct IconFilePath { } typealias LoadExecutableResult = Result -typealias LoadExecutableCompletion = ((LoadExecutableResult) -> Void) extension Model { - func loadExecutable(url: URL, completion: @escaping LoadExecutableCompletion) { + func loadExecutable(url: URL) async throws -> Executable { let executable = Executable() if let bundle = Bundle(url: url) { switch populateFromBundle(executable, bundle: bundle, url: url) { case .failure(let error): - return completion(.failure(error)) + throw error case .success: break } @@ -106,15 +96,15 @@ extension Model { } if let alreadyFoundExecutable = store[executable.identifier] { - return completion(.success(alreadyFoundExecutable)) + return alreadyFoundExecutable } do { - executable.codeRequirement = try SecurityWrapper.copyDesignatedRequirement(url: url) + executable.codeRequirement = try await SecurityWrapper.copyDesignatedRequirement(url: url) store[executable.identifier] = executable - return completion(.success(executable)) + return executable } catch { - return completion(.failure(.codeRequirementError(description: error.localizedDescription))) + throw LoadExecutableError.codeRequirementError(description: error.localizedDescription) } } @@ -171,6 +161,7 @@ extension Model { return IconFilePath.unknown } } + } // MARK: Exporting Profile @@ -211,14 +202,14 @@ extension Model { services: services) } - func importProfile(tccProfile: TCCProfile) { + func importProfile(tccProfile: TCCProfile) async { if let content = tccProfile.content.first { self.cleanUpAndRemoveDependencies() self.importedTCCProfile = tccProfile for (key, policies) in content.services { - getExecutablesFromAllPolicies(policies: policies) + await getExecutablesFromAllPolicies(policies: policies) for policy in policies { let executable = getExecutableFromSelectedExecutables(bundleIdentifier: policy.identifier) @@ -227,7 +218,7 @@ extension Model { let rIdentifier = policy.receiverIdentifier, let rCodeRequirement = policy.receiverCodeRequirement { - let destination = getExecutableFrom(identifier: rIdentifier, codeRequirement: rCodeRequirement) + let destination = await getExecutableFrom(identifier: rIdentifier, codeRequirement: rCodeRequirement) let allowed: Bool = (policy.allowed == true || policy.authorization == TCCPolicyAuthorizationValue.allow) let appleEvent = AppleEventRule(source: source, destination: destination, value: allowed) executable?.appleEvents.appendIfNew(appleEvent) @@ -265,9 +256,9 @@ extension Model { return policy } - func getExecutablesFromAllPolicies(policies: [TCCPolicy]) { + func getExecutablesFromAllPolicies(policies: [TCCPolicy]) async { for tccPolicy in policies where getExecutableFromSelectedExecutables(bundleIdentifier: tccPolicy.identifier) == nil { - let executable = getExecutableFrom(identifier: tccPolicy.identifier, codeRequirement: tccPolicy.codeRequirement) + let executable = await getExecutableFrom(identifier: tccPolicy.identifier, codeRequirement: tccPolicy.codeRequirement) self.selectedExecutables.append(executable) } } @@ -279,21 +270,18 @@ extension Model { return nil } - func getExecutableFrom(identifier: String, codeRequirement: String) -> Executable { + func getExecutableFrom(identifier: String, codeRequirement: String) async -> Executable { var executable = Executable(identifier: identifier, codeRequirement: codeRequirement) - findExecutableOnComputerUsing(bundleIdentifier: identifier) { result in - switch result { - case .success(let goodExecutable): - executable = goodExecutable - case .failure(let error): - self.logger.error("\(error)") - } + do { + executable = try await findExecutable(bundleIdentifier: identifier) + } catch { + self.logger.error("\(error)") } return executable } - private func findExecutableOnComputerUsing(bundleIdentifier: String, completion: @escaping LoadExecutableCompletion) { + private func findExecutable(bundleIdentifier: String) async throws -> Executable { var urlToLoad: URL? if bundleIdentifier.contains("/") { urlToLoad = URL(string: "file://\(bundleIdentifier)") @@ -302,16 +290,9 @@ extension Model { } if let fileURL = urlToLoad { - self.loadExecutable(url: fileURL) { result in - switch result { - case .success(let executable): - return completion(.success(executable)) - case .failure(let error): - return completion(.failure(error)) - } - } + return try await self.loadExecutable(url: fileURL) } - return completion(.failure(.executableNotFound)) + throw LoadExecutableError.executableNotFound } private func cleanUpAndRemoveDependencies() { diff --git a/Source/Model/SigningIdentity.swift b/Source/Model/SigningIdentity.swift index 7ba021a..469412f 100644 --- a/Source/Model/SigningIdentity.swift +++ b/Source/Model/SigningIdentity.swift @@ -30,11 +30,11 @@ import Cocoa class SigningIdentity: NSObject { @objc dynamic var displayName: String - var reference: SecIdentity? + nonisolated(unsafe) var reference: SecIdentity? - init(name: String, reference: SecIdentity?) { - displayName = name - super.init() + nonisolated init(name: String, reference: SecIdentity?) { + self.displayName = name self.reference = reference + super.init() } } diff --git a/Source/Model/TCCProfile.swift b/Source/Model/TCCProfile.swift index 8d67958..2c01a38 100644 --- a/Source/Model/TCCProfile.swift +++ b/Source/Model/TCCProfile.swift @@ -160,11 +160,11 @@ public struct TCCProfile: Codable { /// - signingIdentity: A signing identity; can be nil to leave the profile unsigned. /// - site: A Jamf Pro site /// - Returns: XML data for use with the Jamf Pro API. - func jamfProAPIData(signingIdentity: SecIdentity?, site: (String, String)?) throws -> Data { + func jamfProAPIData(signingIdentity: SecIdentity?, site: (String, String)?) async throws -> Data { var profileText: String var profileData = try xmlData() if let identity = signingIdentity { - profileData = try SecurityWrapper.sign(data: profileData, using: identity) + profileData = try await SecurityWrapper.sign(data: profileData, using: identity) } profileText = String(data: profileData, encoding: .utf8) ?? "" diff --git a/Source/Networking/JamfProAPIClient.swift b/Source/Networking/JamfProAPIClient.swift index 99ac649..76dc3e2 100644 --- a/Source/Networking/JamfProAPIClient.swift +++ b/Source/Networking/JamfProAPIClient.swift @@ -79,7 +79,7 @@ class JamfProAPIClient: Networking { func load(request: URLRequest) async throws -> T { let result: T - if await authManager.bearerAuthSupported() { + if authManager.bearerAuthSupported() { do { result = try await loadBearerAuthorized(request: request) } catch AuthError.bearerAuthNotSupported { @@ -100,7 +100,7 @@ class JamfProAPIClient: Networking { func send(request: URLRequest) async throws -> Data { let result: Data - if await authManager.bearerAuthSupported() { + if authManager.bearerAuthSupported() { do { result = try await sendBearerAuthorized(request: request) } catch AuthError.bearerAuthNotSupported { diff --git a/Source/Networking/NetworkAuthManager.swift b/Source/Networking/NetworkAuthManager.swift index 2f39450..a939a90 100644 --- a/Source/Networking/NetworkAuthManager.swift +++ b/Source/Networking/NetworkAuthManager.swift @@ -46,8 +46,10 @@ enum AuthenticationInfo { case clientCreds(id: String, secret: String) } -/// This actor ensures that only one token refresh occurs at the same time. -actor NetworkAuthManager { +/// This class ensures that only one token refresh occurs at the same time. +/// With MainActor default isolation, all types are MainActor by default, +/// providing the same serialization that the actor previously offered. +class NetworkAuthManager { private let authInfo: AuthenticationInfo private var currentToken: Token? @@ -112,15 +114,15 @@ actor NetworkAuthManager { /// The default is that bearer authentication is supported. After the first network call attempting to use bearer auth, if the /// server does not actually support it this will return false. /// - Returns: True if bearer auth is supported. - func bearerAuthSupported() async -> Bool { + func bearerAuthSupported() -> Bool { return supportsBearerAuth } /// Properly encodes the username and password for use in Basic authentication. /// - /// This doesn't mutate any state and only accesses `let` constants so it doesn't need to be actor isolated. + /// This doesn't mutate any state and only accesses `let` constants so it doesn't need special isolation. /// - Returns: The encoded data string for use with Basic Auth. - nonisolated func basicAuthString() throws -> String { + func basicAuthString() throws -> String { guard case .basicAuth(let username, let password) = authInfo, !username.isEmpty && !password.isEmpty else { diff --git a/Source/Networking/UploadManager.swift b/Source/Networking/UploadManager.swift index 794dac9..109f8c2 100644 --- a/Source/Networking/UploadManager.swift +++ b/Source/Networking/UploadManager.swift @@ -23,61 +23,43 @@ struct UploadManager { case anyError(String) } - func verifyConnection(authManager: NetworkAuthManager, completionHandler: @escaping (Result) -> Void) { + func verifyConnection(authManager: NetworkAuthManager) async throws -> VerificationInfo { logger.info("Checking connection to Jamf Pro server") - Task { - let networking = JamfProAPIClient(serverUrlString: serverURL, tokenManager: authManager) - let result: Result + let networking = JamfProAPIClient(serverUrlString: serverURL, tokenManager: authManager) - do { - let version = try await networking.getJamfProVersion() + do { + let version = try await networking.getJamfProVersion() - // Must sign if Jamf Pro is less than v10.7.1 - let mustSign = (version.semantic() < SemanticVersion(major: 10, minor: 7, patch: 1)) + // Must sign if Jamf Pro is less than v10.7.1 + let mustSign = (version.semantic() < SemanticVersion(major: 10, minor: 7, patch: 1)) - let orgName = try await networking.getOrganizationName() + let orgName = try await networking.getOrganizationName() - result = .success(VerificationInfo(mustSign: mustSign, organization: orgName)) - } catch is AuthError { - logger.error("Invalid credentials.") - result = .failure(VerificationError.anyError("Invalid credentials.")) - } catch { - logger.error("Jamf Pro server is unavailable.") - result = .failure(VerificationError.anyError("Jamf Pro server is unavailable.")) - } - - completionHandler(result) + return VerificationInfo(mustSign: mustSign, organization: orgName) + } catch is AuthError { + logger.error("Invalid credentials.") + throw VerificationError.anyError("Invalid credentials.") + } catch { + logger.error("Jamf Pro server is unavailable.") + throw VerificationError.anyError("Jamf Pro server is unavailable.") } } - func upload(profile: TCCProfile, authMgr: NetworkAuthManager, siteInfo: (String, String)?, signingIdentity: SigningIdentity?, completionHandler: @escaping (Error?) -> Void) { + func upload(profile: TCCProfile, authMgr: NetworkAuthManager, siteInfo: (String, String)?, signingIdentity: SigningIdentity?) async throws { logger.info("Uploading profile: \(profile.displayName, privacy: .public)") let networking = JamfProAPIClient(serverUrlString: serverURL, tokenManager: authMgr) - Task { - let success: Error? - var identity: SecIdentity? - if let signingIdentity = signingIdentity { - logger.info("Signing profile with \(signingIdentity.displayName)") - identity = signingIdentity.reference - } - - do { - let profileData = try profile.jamfProAPIData(signingIdentity: identity, site: siteInfo) - - _ = try await networking.upload(computerConfigProfile: profileData) - - success = nil - logger.info("Uploaded successfully") - } catch { - logger.error("Error creating or uploading profile: \(error.localizedDescription)") - success = error - } - - DispatchQueue.main.async { - completionHandler(success) - } + var identity: SecIdentity? + if let signingIdentity = signingIdentity { + logger.info("Signing profile with \(signingIdentity.displayName)") + identity = signingIdentity.reference } + + let profileData = try await profile.jamfProAPIData(signingIdentity: identity, site: siteInfo) + + _ = try await networking.upload(computerConfigProfile: profileData) + + logger.info("Uploaded successfully") } } diff --git a/Source/SecurityWrapper.swift b/Source/SecurityWrapper.swift index 2de002c..4fdc1ec 100644 --- a/Source/SecurityWrapper.swift +++ b/Source/SecurityWrapper.swift @@ -27,10 +27,11 @@ import Foundation import Haversack +import Security struct SecurityWrapper { - static func execute(block: () -> (OSStatus)) throws { + nonisolated static func execute(block: () -> (OSStatus)) throws { let status = block() if status != 0 { throw NSError(domain: NSOSStatusErrorDomain, code: Int(status), userInfo: nil) @@ -71,7 +72,7 @@ struct SecurityWrapper { return nil } - static func copyDesignatedRequirement(url: URL) throws -> String { + @concurrent static func copyDesignatedRequirement(url: URL) async throws -> String { let flags = SecCSFlags(rawValue: 0) var staticCode: SecStaticCode? var requirement: SecRequirement? @@ -84,8 +85,7 @@ struct SecurityWrapper { return text! as String } - static func sign(data: Data, using identity: SecIdentity) throws -> Data { - + static func sign(data: Data, using identity: SecIdentity) async throws -> Data { var outputData: CFData? var encoder: CMSEncoder? try execute { CMSEncoderCreate(&encoder) } @@ -97,11 +97,11 @@ struct SecurityWrapper { return outputData! as Data } - static func loadSigningIdentities() throws -> [SigningIdentity] { + @concurrent static func loadSigningIdentities() async throws -> [SigningIdentity] { let haversack = Haversack() let query = IdentityQuery().matching(mustBeValidOnDate: Date()).returning(.reference) - let identities = try haversack.search(where: query) + let identities = try await haversack.search(where: query) return identities.compactMap { guard let secIdentity = $0.reference else { @@ -115,7 +115,7 @@ struct SecurityWrapper { } } - static func getCertificateCommonName(for identity: SecIdentity) throws -> String { + nonisolated static func getCertificateCommonName(for identity: SecIdentity) throws -> String { var certificate: SecCertificate? var commonName: CFString? try execute { SecIdentityCopyCertificate(identity, &certificate) } diff --git a/Source/SwiftUI/UploadInfoView.swift b/Source/SwiftUI/UploadInfoView.swift index c37bfc3..19de944 100644 --- a/Source/SwiftUI/UploadInfoView.swift +++ b/Source/SwiftUI/UploadInfoView.swift @@ -119,10 +119,12 @@ struct UploadInfoView: View { .keyboardShortcut(.cancelAction) Button(verifiedConnection ? "Upload" : "Check connection") { - if verifiedConnection { - performUpload() - } else { - verifyConnection() + Task { + if verifiedConnection { + await performUpload() + } else { + await verifyConnection() + } } } .keyboardShortcut(.defaultAction) @@ -272,7 +274,7 @@ struct UploadInfoView: View { return NetworkAuthManager(clientId: username, clientSecret: password) } - func verifyConnection() { + func verifyConnection() async { guard connectionInfoPassesValidation(setWarningInfo: true) else { return } @@ -280,31 +282,30 @@ struct UploadInfoView: View { networkOperationInfo = "Checking Jamf Pro server" let uploadMgr = UploadManager(serverURL: serverURL) - uploadMgr.verifyConnection(authManager: makeAuthManager()) { result in - if case .success(let success) = result { - mustSign = success.mustSign - organization = success.organization - verifiedConnectionHash = hashOfConnectionInfo - if saveToKeychain { - do { - try SecurityWrapper.saveCredentials( - username: username, - password: password, - server: serverURL) - } catch { - logger.error("Failed to save credentials with error: \(error.localizedDescription)") - } + do { + let info = try await uploadMgr.verifyConnection(authManager: makeAuthManager()) + mustSign = info.mustSign + organization = info.organization + verifiedConnectionHash = hashOfConnectionInfo + if saveToKeychain { + do { + try SecurityWrapper.saveCredentials( + username: username, + password: password, + server: serverURL) + } catch { + logger.error("Failed to save credentials with error: \(error.localizedDescription)") } - // Future on macOS 12+: focus on Payload Name field - } else if case .failure(let failure) = result, - case .anyError(let errorString) = failure - { - warningInfo = errorString - verifiedConnectionHash = 0 } - - networkOperationInfo = nil + } catch UploadManager.VerificationError.anyError(let errorString) { + warningInfo = errorString + verifiedConnectionHash = 0 + } catch { + warningInfo = error.localizedDescription + verifiedConnectionHash = 0 } + + networkOperationInfo = nil } private func dismissView() { @@ -317,7 +318,7 @@ struct UploadInfoView: View { } } - func performUpload() { + func performUpload() async { guard connectionInfoPassesValidation(setWarningInfo: true) else { return } @@ -342,20 +343,18 @@ struct UploadInfoView: View { } let uploadMgr = UploadManager(serverURL: serverURL) - uploadMgr.upload( - profile: profile, - authMgr: makeAuthManager(), - siteInfo: siteIdAndName, - signingIdentity: mustSign ? signingId : nil - ) { possibleError in - if let error = possibleError { - warningInfo = error.localizedDescription - } else { - Alert().display(header: "Success", message: "Profile uploaded succesfully") - dismissView() - } - networkOperationInfo = nil + do { + try await uploadMgr.upload( + profile: profile, + authMgr: makeAuthManager(), + siteInfo: siteIdAndName, + signingIdentity: mustSign ? signingId : nil) + Alert().display(header: "Success", message: "Profile uploaded succesfully") + dismissView() + } catch { + warningInfo = error.localizedDescription } + networkOperationInfo = nil } } diff --git a/Source/TCCProfileImporter/TCCProfileConfigurationPanel.swift b/Source/TCCProfileImporter/TCCProfileConfigurationPanel.swift index 75174e3..e46f02b 100644 --- a/Source/TCCProfileImporter/TCCProfileConfigurationPanel.swift +++ b/Source/TCCProfileImporter/TCCProfileConfigurationPanel.swift @@ -31,9 +31,12 @@ import Foundation class TCCProfileConfigurationPanel { /// Load TCC Profile data from file /// - /// - Parameter completion: TCCProfileImportCompletion - success with TCCProfile or failure with TCCProfileImport Error - func loadTCCProfileFromFile(importer: TCCProfileImporter, window: NSWindow, _ completion: @escaping TCCProfileImportCompletion) { - let openPanel = NSOpenPanel.init() + /// - Parameters: + /// - importer: The TCCProfileImporter to use + /// - window: The window to present the open panel in + /// - Returns: The decoded TCCProfile, or nil if the user cancelled + func loadTCCProfileFromFile(importer: TCCProfileImporter, window: NSWindow) async throws -> TCCProfile? { + let openPanel = NSOpenPanel() openPanel.allowedFileTypes = ["mobileconfig", "plist"] openPanel.allowsMultipleSelection = false openPanel.canChooseDirectories = false @@ -41,19 +44,13 @@ class TCCProfileConfigurationPanel { openPanel.canChooseFiles = true openPanel.title = "Open TCCProfile File" - openPanel.beginSheetModal(for: window) { (response) in - if response != .OK { - completion(.failure(.cancelled)) - } else { - if let result = openPanel.url { - importer.decodeTCCProfile(fileUrl: result) { tccProfileResult in - return completion(tccProfileResult) - } - } else { - completion(.failure(TCCProfileImportError.unableToOpenFile)) - } - } + let response = await openPanel.beginSheetModal(for: window) + guard response == .OK else { return nil } + + guard let fileUrl = openPanel.url else { + throw TCCProfileImportError.unableToOpenFile } + return try importer.decodeTCCProfile(fileUrl: fileUrl) } } diff --git a/Source/TCCProfileImporter/TCCProfileImporter.swift b/Source/TCCProfileImporter/TCCProfileImporter.swift index 0ac1111..6b61f49 100644 --- a/Source/TCCProfileImporter/TCCProfileImporter.swift +++ b/Source/TCCProfileImporter/TCCProfileImporter.swift @@ -27,46 +27,43 @@ import Foundation -typealias TCCProfileImportResult = Result -typealias TCCProfileImportCompletion = ((TCCProfileImportResult) -> Void) - /// Load tcc profiles public class TCCProfileImporter { // MARK: Load TCCProfile - /// Mapping & Decoding tcc profile + /// Mapping & Decoding tcc profile from data /// - /// - Parameter fileUrl: path with a file to load, completion: TCCProfileImportCompletion - success with TCCProfile or failure with TCCProfileImport Error - func decodeTCCProfile(data: Data, _ completion: @escaping TCCProfileImportCompletion) { + /// - Parameter data: The raw data to decode + /// - Returns: The decoded TCCProfile + func decodeTCCProfile(data: Data) throws -> TCCProfile { do { // Note that parse will ignore the signing portion of the data - let tccProfile = try TCCProfile.parse(from: data) - return completion(.success(tccProfile)) + return try TCCProfile.parse(from: data) } catch TCCProfile.ParseError.failedToCreateDecoder { - return completion(.failure(.decodeProfileError)) + throw TCCProfileImportError.decodeProfileError } catch let DecodingError.keyNotFound(codingKey, _) { - return completion(TCCProfileImportResult.failure(.invalidProfileFile(description: codingKey.stringValue))) + throw TCCProfileImportError.invalidProfileFile(description: codingKey.stringValue) } catch let DecodingError.typeMismatch(type, context) { let errorDescription = "Type \(type) mismatch: \(context.debugDescription) codingPath: \(context.codingPath)" - return completion(.failure(.invalidProfileFile(description: errorDescription))) + throw TCCProfileImportError.invalidProfileFile(description: errorDescription) } catch let error as NSError { let errorDescription = error.userInfo["NSDebugDescription"] as? String - return completion(.failure(.invalidProfileFile(description: errorDescription ?? error.localizedDescription))) + throw TCCProfileImportError.invalidProfileFile(description: errorDescription ?? error.localizedDescription) } } - /// Mapping & Decoding tcc profile + /// Mapping & Decoding tcc profile from a file URL /// - /// - Parameter fileUrl: path with a file to load, completion: TCCProfileImportCompletion - success with TCCProfile or failure with TCCProfileImport Error - func decodeTCCProfile(fileUrl: URL, _ completion: @escaping TCCProfileImportCompletion) { + /// - Parameter fileUrl: path with a file to load + /// - Returns: The decoded TCCProfile + func decodeTCCProfile(fileUrl: URL) throws -> TCCProfile { let data: Data do { data = try Data(contentsOf: fileUrl) - return decodeTCCProfile(data: data, completion) } catch { - return completion(.failure(.unableToOpenFile)) + throw TCCProfileImportError.unableToOpenFile } - + return try decodeTCCProfile(data: data) } } diff --git a/Source/View Controllers/OpenViewController.swift b/Source/View Controllers/OpenViewController.swift index 733315b..d9b0026 100644 --- a/Source/View Controllers/OpenViewController.swift +++ b/Source/View Controllers/OpenViewController.swift @@ -43,16 +43,16 @@ class OpenViewController: NSViewController, NSTableViewDataSource, NSTableViewDe // Reload executables current = Model.shared.current if let value = current { - choices = Model.shared.getAppleEventChoices(executable: value) + Task { + choices = await Model.shared.getAppleEventChoices(executable: value) + } } } func tableView(_ tableView: NSTableView, selectionIndexesForProposedSelection proposedSelectionIndexes: IndexSet) -> IndexSet { - DispatchQueue.main.async { - guard let index = proposedSelectionIndexes.first else { return } - self.completionBlock?([.success(self.choices[index])]) - self.dismiss(self) - } + guard let index = proposedSelectionIndexes.first else { return proposedSelectionIndexes } + self.completionBlock?([.success(self.choices[index])]) + self.dismiss(self) return proposedSelectionIndexes } @@ -64,13 +64,20 @@ class OpenViewController: NSViewController, NSTableViewDataSource, NSTableViewDe panel.directoryURL = URL(fileURLWithPath: "/Applications", isDirectory: true) panel.begin { response in if response == .OK { - var selections: [LoadExecutableResult] = [] - panel.urls.forEach { - Model.shared.loadExecutable(url: $0) { result in - selections.append(result) + Task { + var selections: [LoadExecutableResult] = [] + for url in panel.urls { + do { + let executable = try await Model.shared.loadExecutable(url: url) + selections.append(.success(executable)) + } catch { + if let loadError = error as? LoadExecutableError { + selections.append(.failure(loadError)) + } + } } + block?(selections) } - block?(selections) } } } diff --git a/Source/View Controllers/SaveViewController.swift b/Source/View Controllers/SaveViewController.swift index 0f8a525..dc45cab 100644 --- a/Source/View Controllers/SaveViewController.swift +++ b/Source/View Controllers/SaveViewController.swift @@ -85,10 +85,7 @@ class SaveViewController: NSViewController { panel.begin { response in if response == .OK { - // Let the save panel fully close itself before doing any work that may require keychain access. - DispatchQueue.main.async { - self.saveTo(url: panel.url!) - } + self.saveTo(url: panel.url!) } } } @@ -96,12 +93,14 @@ class SaveViewController: NSViewController { override func viewDidLoad() { super.viewDidLoad() payloadIdentifier = UUID().uuidString - do { - var identities = try SecurityWrapper.loadSigningIdentities() - identities.insert(SigningIdentity(name: "Not signed", reference: nil), at: 0) - identitiesPopUpAC.add(contentsOf: identities) - } catch { - logger.error("Error loading identities: \(error)") + Task { + do { + var identities = try await SecurityWrapper.loadSigningIdentities() + identities.insert(SigningIdentity(name: "Not signed", reference: nil), at: 0) + identitiesPopUpAC.add(contentsOf: identities) + } catch { + logger.error("Error loading identities: \(error)") + } } loadImportedTCCProfileInfo() @@ -120,9 +119,11 @@ class SaveViewController: NSViewController { defaultsController.removeObserver(self, forKeyPath: "values.organization", context: &SaveViewController.saveProfileKVOContext) } - override func observeValue(forKeyPath keyPath: String?, of object: Any?, change: [NSKeyValueChangeKey: Any]?, context: UnsafeMutableRawPointer?) { + nonisolated override func observeValue(forKeyPath keyPath: String?, of object: Any?, change: [NSKeyValueChangeKey: Any]?, context: UnsafeMutableRawPointer?) { if context == &SaveViewController.saveProfileKVOContext { - updateIsReadyToSave() + MainActor.assumeIsolated { + updateIsReadyToSave() + } } else { super.observeValue(forKeyPath: keyPath, of: object, change: change, context: context) } @@ -136,18 +137,20 @@ class SaveViewController: NSViewController { identifier: payloadIdentifier, displayName: payloadName, payloadDescription: payloadDescription ?? payloadName) - do { - var outputData = try profile.xmlData() - if let identity = identitiesPopUpAC.selectedObjects.first as? SigningIdentity, let ref = identity.reference { - logger.info("Signing profile with \(identity.displayName)") - outputData = try SecurityWrapper.sign(data: outputData, using: ref) + Task { + do { + var outputData = try profile.xmlData() + if let identity = identitiesPopUpAC.selectedObjects.first as? SigningIdentity, let ref = identity.reference { + logger.info("Signing profile with \(identity.displayName)") + outputData = try await SecurityWrapper.sign(data: outputData, using: ref) + } + try outputData.write(to: url) + logger.info("Saved successfully") + } catch { + logger.error("Error: \(error)") } - try outputData.write(to: url) - logger.info("Saved successfully") - } catch { - logger.error("Error: \(error)") + self.dismiss(nil) } - self.dismiss(nil) } func loadImportedTCCProfileInfo() { diff --git a/Source/View Controllers/TCCProfileViewController.swift b/Source/View Controllers/TCCProfileViewController.swift index 5dcdb1b..02f39d0 100644 --- a/Source/View Controllers/TCCProfileViewController.swift +++ b/Source/View Controllers/TCCProfileViewController.swift @@ -147,21 +147,23 @@ class TCCProfileViewController: NSViewController { let logger = Logger.TCCProfileViewController @IBAction func uploadAction(_ sender: NSButton) { - let identities: [SigningIdentity] - do { - identities = try SecurityWrapper.loadSigningIdentities() - } catch { - identities = [] - logger.error("Error loading identities: \(error.localizedDescription)") - } + Task { + let identities: [SigningIdentity] + do { + identities = try await SecurityWrapper.loadSigningIdentities() + } catch { + identities = [] + logger.error("Error loading identities: \(error.localizedDescription)") + } - let uploadView = UploadInfoView(signingIdentities: identities) { - // Dismiss the sheet when the UploadInfoView decides it is done - if let controller = self.presentedViewControllers?.first { - self.dismiss(controller) + let uploadView = UploadInfoView(signingIdentities: identities) { + // Dismiss the sheet when the UploadInfoView decides it is done + if let controller = self.presentedViewControllers?.first { + self.dismiss(controller) + } } + self.presentAsSheet(NSHostingController(rootView: uploadView)) } - self.presentAsSheet(NSHostingController(rootView: uploadView)) } fileprivate func showAlert(_ error: LocalizedError, for window: NSWindow) { @@ -181,15 +183,15 @@ class TCCProfileViewController: NSViewController { let tccProfileImporter = TCCProfileImporter() let tccConfigPanel = TCCProfileConfigurationPanel() - tccConfigPanel.loadTCCProfileFromFile(importer: tccProfileImporter, window: window) { [weak self] tccProfileResult in - guard let weakSelf = self else { return } - switch tccProfileResult { - case .success(let tccProfile): - weakSelf.model.importProfile(tccProfile: tccProfile) - case .failure(let tccProfileImportError): - if !tccProfileImportError.isCancelled { - weakSelf.showAlert(tccProfileImportError, for: window) + Task { + do { + if let tccProfile = try await tccConfigPanel.loadTCCProfileFromFile(importer: tccProfileImporter, window: window) { + await model.importProfile(tccProfile: tccProfile) } + } catch let error as TCCProfileImportError { + showAlert(error, for: window) + } catch { + showAlert(TCCProfileImportError.invalidProfileFile(description: error.localizedDescription), for: window) } } } @@ -204,19 +206,21 @@ class TCCProfileViewController: NSViewController { } panel.begin { response in if response == .OK { - panel.urls.forEach { - self.model.loadExecutable(url: $0) { [weak self] result in - switch result { - case .success(let executable): - guard self?.shouldExecutableBeAdded(executable) ?? false else { + Task { + for url in panel.urls { + do { + let executable = try await self.model.loadExecutable(url: url) + guard self.shouldExecutableBeAdded(executable) else { let error = LoadExecutableError.executableAlreadyExists - self?.showAlert(error, for: window) - return + self.showAlert(error, for: window) + continue } block(executable) - case .failure(let error): - self?.showAlert(error, for: window) - self?.logger.error("\(error)") + } catch { + if let loadError = error as? LoadExecutableError { + self.showAlert(loadError, for: window) + } + self.logger.error("\(error)") } } } @@ -251,9 +255,15 @@ class TCCProfileViewController: NSViewController { removableVolumesPopUpAC ]) - setupStandardUserAllowAndDeny(policies: [screenCapturePopUpAC, listenEventPopUpAC]) + setupStandardUserAllowAndDeny(policies: [ + screenCapturePopUpAC, + listenEventPopUpAC + ]) - setupDenyOnly(policies: [cameraPopUpAC, microphonePopUpAC]) + setupDenyOnly(policies: [ + cameraPopUpAC, + microphonePopUpAC + ]) setupDescriptions() @@ -408,31 +418,32 @@ extension TCCProfileViewController: NSTableViewDataSource { guard let window = self.view.window else { return false } - var addedAny = false - urls?.forEach { (url) in - model.loadExecutable(url: url) { [weak self] result in - switch result { - case .success(let newExecutable): - if tableView == self?.executablesTable { - guard self?.executablesAC.canInsert ?? false else { - return + guard let urls = urls, !urls.isEmpty else { return false } + + Task { + for url in urls { + do { + let newExecutable = try await model.loadExecutable(url: url) + if tableView == self.executablesTable { + guard self.executablesAC.canInsert else { + continue } - if self?.shouldExecutableBeAdded(newExecutable) ?? false { - self?.executablesAC.insert(newExecutable, atArrangedObjectIndex: row) - addedAny = true + if self.shouldExecutableBeAdded(newExecutable) { + self.executablesAC.insert(newExecutable, atArrangedObjectIndex: row) } } else { - self?.insertIntoAppleEvents(newExecutable) - addedAny = true + self.insertIntoAppleEvents(newExecutable) + } + } catch { + if let loadError = error as? LoadExecutableError { + self.showAlert(loadError, for: window) } - case .failure(let error): - self?.showAlert(error, for: window) - self?.logger.error("\(error)") + self.logger.error("\(error)") } } } - return addedAny + return true } } diff --git a/Source/Views/Alert.swift b/Source/Views/Alert.swift index 7c61b23..f833459 100644 --- a/Source/Views/Alert.swift +++ b/Source/Views/Alert.swift @@ -27,16 +27,14 @@ import Cocoa -class Alert: NSObject { +class Alert { func display(header: String, message: String) { - DispatchQueue.main.async { - let dialog: NSAlert = NSAlert() - dialog.messageText = header - dialog.informativeText = message - dialog.alertStyle = NSAlert.Style.warning - dialog.addButton(withTitle: "OK") - dialog.runModal() - } + let dialog: NSAlert = NSAlert() + dialog.messageText = header + dialog.informativeText = message + dialog.alertStyle = NSAlert.Style.warning + dialog.addButton(withTitle: "OK") + dialog.runModal() } /// Displays a message with a cancel button and returns true if OK was pressed diff --git a/docs/plans/approachable-concurrency.md b/docs/plans/approachable-concurrency.md new file mode 100644 index 0000000..7ea242f --- /dev/null +++ b/docs/plans/approachable-concurrency.md @@ -0,0 +1,243 @@ +# Approachable Concurrency — Implementation Plan + +**Issue:** https://github.com/jamf/PPPC-Utility/issues/141 + +## Problem + +PPPC Utility is on Swift 5.0 with zero concurrency checking. The goal is to adopt Swift 6.2 Approachable Concurrency so MainActor isolation is inferred by the compiler rather than manually annotated. + +## Approach + +**"Turn it on and see what breaks"** — enable Approachable Concurrency build settings first, then use compiler diagnostics to guide incremental fixes. + +## PR/Stage Boundaries + +**Every stage produces a buildable, functional app.** The build settings from Stage 1 are already applied and the app builds + tests pass with only warnings. Each subsequent stage resolves warnings and modernizes patterns — none are required for the app to function. + +| PR | Stage | Status | Description | +|----|-------|--------|-------------| +| 1 | Stage 1 | ✅ Done | Enable Approachable Concurrency build settings | +| 2 | Stage 2 | ✅ Done | Remove `NetworkAuthManager` actor → class | +| 3a | Stage 3a | ✅ Done | Remove 3 `DispatchQueue.main.async` wrappers | +| 3b | Stage 3b | ✅ Done | Convert `UploadManager` to async throws | +| 3c | Stage 3c | ✅ Done | Convert `Model.loadExecutable` to direct return | +| 3d | Stage 3d | ✅ Done | Convert `TCCProfileImporter` to direct return | +| 4 | Stage 4 | ✅ Done | Add `@concurrent` for background I/O | +| 5 | Stage 5 | ✅ Done | Enable Swift 6 language mode (warnings → errors) | + +PRs 3a–3d can be one PR or individual PRs — each is independently functional. PR 4 depends on PR 3c. PR 5 depends on all prior stages. + +## Stage 1: Enable Approachable Concurrency Build Settings ✅ + +Add to both app and test targets (Debug + Release): +- `SWIFT_DEFAULT_ACTOR_ISOLATION = MainActor` +- `SWIFT_APPROACHABLE_CONCURRENCY = YES` +- `SWIFT_STRICT_CONCURRENCY = complete` +- Keep `SWIFT_VERSION = 5.0` + +Build, capture all warnings/errors, categorize them. + +## Stage 1 Results: Compiler Diagnostics (reference) + +Build + tests PASSED with only warnings. 2 concurrency warning categories found: +1. `Token.isValid` cross-isolation (actor ↔ MainActor) +2. XCTestCase/NSObject override isolation mismatch (all test classes + SaveViewController) + +--- + +## Stage 2: Remove the `NetworkAuthManager` actor → class + +**Goal:** With `SWIFT_DEFAULT_ACTOR_ISOLATION = MainActor`, all types are MainActor by default. The `NetworkAuthManager` actor's synchronization purpose (single-flight token refresh) is now provided by MainActor serialization. Convert it to a regular class. + +### Changes: + +**`Source/Networking/NetworkAuthManager.swift`** +- `actor NetworkAuthManager` → `class NetworkAuthManager` +- `bearerAuthSupported()` — drop `async`, it was only async for actor isolation. Return type stays `Bool`. +- `nonisolated func basicAuthString()` → `func basicAuthString()` — no longer an actor, `nonisolated` is unnecessary +- `validToken(networking:)` — stays `async throws` (does async work internally) +- `refreshToken(networking:)` — stays `async throws` (does async work internally) +- The `Task { }` inside `refreshToken` inherits MainActor isolation under Approachable Concurrency, so mutations to `refreshTask`, `currentToken`, `supportsBearerAuth` remain safe + +**`Source/Networking/JamfProAPIClient.swift`** +- `await authManager.bearerAuthSupported()` → `authManager.bearerAuthSupported()` (2 locations, lines ~80, ~101) — drop `await` since no longer async + +**`Source/Networking/Networking.swift`** +- No changes needed — `authManager` property is `let`, and all calls to async methods already use `await` + +**`Source/SwiftUI/UploadInfoView.swift`** +- `makeAuthManager()` — no changes, just returns the new class instead of actor + +**`PPPC UtilityTests/NetworkingTests/NetworkAuthManagerTests.swift`** +- `await authManager.bearerAuthSupported()` → `authManager.bearerAuthSupported()` (2 locations, lines ~91, ~104) — drop `await` +- Tests that are `async throws` stay that way (they still `await` async methods like `validToken`) + +**Side effect:** Resolves the `Token.isValid` cross-isolation warning because `Token` and `NetworkAuthManager` are now both on MainActor — no isolation boundary to cross. + +--- + +## Stage 3: Remove `DispatchQueue.main.async` and convert `Task{}` + completion handlers to async + +**Goal:** With everything MainActor-isolated, manual dispatch to the main thread is redundant. Also, replace `Task{}` + completion handler patterns with direct async methods — callers can just `await`. + +### 3a. Remove `DispatchQueue.main.async` wrappers (3 simple locations) + +**1. `Source/Views/Alert.swift:32`** — `display(header:message:)` +- Remove `DispatchQueue.main.async { ... }` wrapper, keep the body inline +- `Alert` is MainActor by default, so `display()` is already on MainActor + +**2. `Source/View Controllers/OpenViewController.swift:51`** — `tableView(_:selectionIndexesForProposedSelection:)` +- Remove `DispatchQueue.main.async { ... }` wrapper, keep the body inline +- NSTableViewDelegate method called on MainActor + +**3. `Source/View Controllers/SaveViewController.swift:87`** — `savePressed(_:)` panel callback +- Remove `DispatchQueue.main.async { ... }` wrapper, keep `self.saveTo(url: panel.url!)` +- NSSavePanel.begin completion runs on main thread; SaveViewController is MainActor + +### 3b. Convert `UploadManager` from Task/completion to async (larger refactor) + +**Current pattern** — both methods use `Task{}` internally + call a `completionHandler`: +- `verifyConnection(authManager:completionHandler:)` — wraps async networking in `Task{}`, calls back via completion +- `upload(profile:authMgr:siteInfo:signingIdentity:completionHandler:)` — same pattern + `DispatchQueue.main.async` before callback + +**New pattern** — make both methods `async throws` directly: +- `func verifyConnection(authManager:) async throws -> VerificationInfo` — no Task, no completion handler, just returns +- `func upload(profile:authMgr:siteInfo:signingIdentity:) async throws` — no Task, no completion handler, throws on error + +**Caller update** — `UploadInfoView.swift`: +- `verifyConnection()` — currently synchronous, calls `uploadMgr.verifyConnection(...) { result in }`. Refactor to: + ```swift + func verifyConnection() async { ... let info = try await uploadMgr.verifyConnection(...) } + ``` + Button action becomes: `Task { await verifyConnection() }` + Note: This is the one place a `Task{}` remains necessary — bridging from SwiftUI button action to async. This is the idiomatic pattern. +- `performUpload()` — same pattern as above + +### 3c. Convert `Model.loadExecutable` from completion handler to direct return + +**Current:** `func loadExecutable(url:completion: @escaping LoadExecutableCompletion)` — synchronous with completion handler callback. This is not actually async work — it reads bundle info and code requirements synchronously. + +**New:** `func loadExecutable(url: URL) throws -> Executable` — direct return, throws on error. No completion handler needed since the work is synchronous. + +**Callers to update:** +- `Model.getAppleEventChoices(executable:)` — currently calls `loadExecutable(url:) { result in ... }` 3 times, switch on result. Simplify to `try loadExecutable(url:)` with do/catch. +- `Model.findExecutableOnComputerUsing(bundleIdentifier:completion:)` → `func findExecutable(bundleIdentifier:) throws -> Executable` — direct return +- `Model.getExecutableFrom(identifier:codeRequirement:)` — currently calls `findExecutableOnComputerUsing`. Update to use new direct call. +- `Model.getExecutablesFromAllPolicies(policies:)` — calls `getExecutableFrom`. Update. +- `TCCProfileViewController.promptForExecutables(_:)` — calls `model.loadExecutable(url:)`. Update to direct call. +- `TCCProfileViewController.tableView(_:acceptDrop:row:dropOperation:)` — calls `model.loadExecutable(url:)`. Update. +- `OpenViewController.prompt(_:)` — calls `Model.shared.loadExecutable(url:)`. Update. + +### 3d. Convert `TCCProfileImporter` from completion handler to direct return + +**Current:** +- `decodeTCCProfile(data:completion:)` — synchronous decode with completion callback +- `decodeTCCProfile(fileUrl:completion:)` — synchronous file read + decode with completion callback + +**New:** +- `func decodeTCCProfile(data: Data) throws -> TCCProfile` — direct return +- `func decodeTCCProfile(fileUrl: URL) throws -> TCCProfile` — direct return +- Remove `TCCProfileImportCompletion` typealias (no longer needed) + +**Caller update:** +- `TCCProfileConfigurationPanel.loadTCCProfileFromFile(importer:window:completion:)` — update to use direct calls inside the panel callback +- `TCCProfileViewController.importProfile(_:)` — update to use new direct return + +--- + +## Stage 4: Identify and annotate `@concurrent` for background work + +**Goal:** Find synchronous work that runs on MainActor and could block the UI. Mark it `@concurrent` to run on the cooperative thread pool. + +### Candidates analysis: + +**Networking layer (`Networking`, `JamfProAPIClient`)** +- `URLSession.shared.data(for:)` is an Apple async API — it suspends, doesn't block MainActor ✅ +- JSON decoding after `await` runs synchronously on MainActor — payloads are tiny ✅ +- **No `@concurrent` needed** for networking methods + +**`SecurityWrapper` (static methods)** +- `sign(data:using:)` — CMS signing could be slow for large profiles → **candidate for `@concurrent`** +- `copyDesignatedRequirement(url:)` — reads code signatures from disk → **candidate for `@concurrent`** +- `loadSigningIdentities()` — queries keychain for all identities → **candidate for `@concurrent`** +- `loadCredentials()` / `saveCredentials()` / `removeCredentials()` — keychain ops, typically fast → **possible candidate** + +**`Model.loadExecutable(url:)` (after Stage 3c refactor)** +- Reads bundle info from disk + calls `SecurityWrapper.copyDesignatedRequirement` → **candidate for `@concurrent`** +- After Stage 3c this is `throws -> Executable`. Making it `@concurrent async throws -> Executable` would move disk I/O off MainActor. +- Callers (`getAppleEventChoices`, `importProfile`, `promptForExecutables`, drag-drop handler) would need to `await`. + +**`TCCProfile.xmlData()` / `TCCProfile.jamfProAPIData()`** +- PropertyListEncoder/XML encoding — fast for typical profile sizes → **likely not needed** + +### Recommended `@concurrent` annotations: +1. `SecurityWrapper.copyDesignatedRequirement(url:)` → `@concurrent static func ... async throws` +2. `SecurityWrapper.sign(data:using:)` → `@concurrent static func ... async throws` +3. `SecurityWrapper.loadSigningIdentities()` → `@concurrent static func ... async throws` +4. `Model.loadExecutable(url:)` → `@concurrent func ... async throws -> Executable` (since it calls copyDesignatedRequirement) +5. All callers of the above need `await` +6. `Model.getAppleEventChoices` → `async` (calls loadExecutable), callers update +7. `Model.importProfile` → calls `getExecutableFrom` which calls `loadExecutable` — cascade of async + +### Note on remaining `Task{}` usage after all stages: +The only `Task{}` calls that should remain are at **UI entry points** where we bridge from synchronous UI callbacks to async: +- SwiftUI button actions in `UploadInfoView` (Task { await verifyConnection() }, Task { await performUpload() }) +- NSOpenPanel/NSSavePanel `.begin { }` callbacks if they need to call async methods +- These are idiomatic and unavoidable — UI callbacks are synchronous by nature + +## Key Guidelines (from CLAUDE.md & swift-concurrency skill) + +- Do NOT add explicit `@MainActor` annotations — isolation is inferred via build settings +- Do NOT annotate value types (structs, enums) as `Sendable` — they are Sendable by default +- Prefer `@concurrent` for background work instead of actor isolation opt-outs +- Avoid unnecessary `Task {}` — prefer async/await +- `nonisolated(nonsending)` for generic async utilities that should stay on caller's executor + +--- + +## Stage 5: Enable Swift 6 Language Mode — Full Strict Concurrency + +**Goal:** Flip `SWIFT_VERSION` from `5.0` to `6.0` so all concurrency warnings become hard errors. This is the capstone — the app and tests must compile with zero concurrency diagnostics. + +**Depends on:** Stages 1–4 complete (zero concurrency warnings). + +### 5a. Build setting change + +**`PPPC Utility.xcodeproj/project.pbxproj`** — 4 locations (app Debug, app Release, test Debug, test Release): +- `SWIFT_VERSION = 5.0` → `SWIFT_VERSION = 6.0` + +All other concurrency settings are already correct from Stage 1: +- `SWIFT_STRICT_CONCURRENCY = complete` ✅ +- `SWIFT_APPROACHABLE_CONCURRENCY = YES` ✅ +- `SWIFT_DEFAULT_ACTOR_ISOLATION = MainActor` ✅ + +### 5b. Fix override isolation mismatches (from Stage 1 diagnostics) + +Stage 1 Results identified two warning categories. Category 1 (`Token.isValid` cross-isolation) is resolved by Stage 2. Category 2 — **override isolation mismatches** — is not addressed by Stages 2–4 and becomes an error in Swift 6. + +The issue: With `SWIFT_DEFAULT_ACTOR_ISOLATION = MainActor`, our classes and their method overrides are MainActor-isolated. But some parent class methods from ObjC frameworks are not annotated for MainActor in the SDK. The compiler warns (Swift 5) / errors (Swift 6) about the isolation mismatch. + +**Known locations from Stage 1 results:** + +1. **`SaveViewController.swift`** — `override func observeValue(forKeyPath:of:change:context:)` + - NSObject's `observeValue` is not MainActor-annotated in the SDK + - Fix: Add `nonisolated` to the override, since KVO callbacks can fire from any context. The method body only accesses `self` (which is MainActor), so wrap the body access in `MainActor.assumeIsolated { }` — this is safe because KVO on `@objc dynamic` properties from MainActor objects delivers on MainActor. + - Alternative: If Swift 6.2 SDK has annotated this method by the time we get here, no fix needed — verify by building first. + +2. **Test classes** — `override func setUp()` in `ModelTests` + - XCTestCase lifecycle methods (`setUp`, `tearDown`) are `nonisolated` in the XCTest header. With default MainActor isolation, test subclasses are MainActor, creating a mismatch on the override. + - Fix: Add `nonisolated` to `override func setUp()`. If the body needs MainActor access, mark the override as `override func setUp() async throws` (XCTest supports async setUp in modern versions) or move setup work to a helper. + +### 5c. Verification + +1. **Build** both app and test targets (Debug + Release) — zero errors, zero warnings +2. **Run all tests** — all pass +3. **Manual smoke test** — launch app, add executables, configure TCC policies, save/export profile, import profile +4. Confirm no runtime isolation assertions (Swift 6 adds runtime checks for actor isolation violations) + +### What should NOT be in Stage 5 + +- No explicit `@MainActor` annotations — isolation is inferred via `SWIFT_DEFAULT_ACTOR_ISOLATION = MainActor` +- No `Sendable` annotations on value types (structs, enums) — they are Sendable by default +- No new `Task {}` wrappers — if something needs async bridging, it should have been handled in Stages 3–4 +- No behavioral changes — Stage 5 is purely a compiler enforcement change