From 76a7aaddd95ee22909af7875100996f626af8bcd Mon Sep 17 00:00:00 2001 From: Tony Li Date: Sun, 14 Jun 2026 18:23:46 +1200 Subject: [PATCH 01/13] Format files with swift-format ahead of keychain changes --- .../ShareExtensionService.swift | 33 +++-- .../WordPressData/Swift/Blog+Swift.swift | 28 ++-- .../Swift/SharedDataIssueSolver.swift | 39 ++++-- .../WordPressData/Swift/WPAccount.swift | 12 +- .../Services/StatsWidgetsService.swift | 127 +++++++++++------- .../Tests/Jetpack/DataMigratorTests.swift | 46 +++++-- .../Jetpack/SharedDataIssueSolverTests.swift | 23 +++- .../Jetpack/Utility/DataMigrator.swift | 25 ++-- .../Classes/Services/CredentialsService.swift | 4 +- .../Services/NotificationSupportService.swift | 6 +- .../Editor/EditorConfiguration+Blog.swift | 26 ++-- .../Sources/NotificationService.swift | 93 ++++++++----- 12 files changed, 303 insertions(+), 159 deletions(-) diff --git a/Modules/Sources/ShareExtensionCore/ShareExtensionService.swift b/Modules/Sources/ShareExtensionCore/ShareExtensionService.swift index 38d92720cf9a..2e0b2ec3c7f1 100644 --- a/Modules/Sources/ShareExtensionCore/ShareExtensionService.swift +++ b/Modules/Sources/ShareExtensionCore/ShareExtensionService.swift @@ -153,11 +153,13 @@ public final class ShareExtensionService { /// Retrieves the WordPress.com OAuth Token, meant for Extension usage. /// public func retrieveShareExtensionToken() -> String? { - guard let oauth2Token = try? SFHFKeychainUtils.getPasswordForUsername( - configuration.keychainTokenKey, - andServiceName: configuration.keychainServiceName, - accessGroup: appKeychainAccessGroup - ) else { + guard + let oauth2Token = try? SFHFKeychainUtils.getPasswordForUsername( + configuration.keychainTokenKey, + andServiceName: configuration.keychainServiceName, + accessGroup: appKeychainAccessGroup + ) + else { return nil } @@ -167,11 +169,13 @@ public final class ShareExtensionService { /// Retrieves the WordPress.com Username, meant for Extension usage. /// public func retrieveShareExtensionUsername() -> String? { - guard let oauth2Token = try? SFHFKeychainUtils.getPasswordForUsername( - configuration.keychainUsernameKey, - andServiceName: configuration.keychainServiceName, - accessGroup: appKeychainAccessGroup - ) else { + guard + let oauth2Token = try? SFHFKeychainUtils.getPasswordForUsername( + configuration.keychainUsernameKey, + andServiceName: configuration.keychainServiceName, + accessGroup: appKeychainAccessGroup + ) + else { return nil } @@ -186,7 +190,8 @@ public final class ShareExtensionService { } if let siteID = userDefaults.object(forKey: configuration.userDefaultsPrimarySiteID) as? Int, - let siteName = userDefaults.object(forKey: configuration.userDefaultsPrimarySiteName) as? String { + let siteName = userDefaults.object(forKey: configuration.userDefaultsPrimarySiteName) as? String + { return (siteID, siteName) } @@ -202,12 +207,14 @@ public final class ShareExtensionService { } if let siteID = userDefaults.object(forKey: configuration.userDefaultsLastUsedSiteID) as? Int, - let siteName = userDefaults.object(forKey: configuration.userDefaultsLastUsedSiteName) as? String { + let siteName = userDefaults.object(forKey: configuration.userDefaultsLastUsedSiteName) as? String + { return (siteID, siteName) } if let siteID = userDefaults.object(forKey: configuration.userDefaultsPrimarySiteID) as? Int, - let siteName = userDefaults.object(forKey: configuration.userDefaultsPrimarySiteName) as? String { + let siteName = userDefaults.object(forKey: configuration.userDefaultsPrimarySiteName) as? String + { return (siteID, siteName) } diff --git a/Modules/Sources/WordPressData/Swift/Blog+Swift.swift b/Modules/Sources/WordPressData/Swift/Blog+Swift.swift index eda7c4828f0f..9a4d00cefcbd 100644 --- a/Modules/Sources/WordPressData/Swift/Blog+Swift.swift +++ b/Modules/Sources/WordPressData/Swift/Blog+Swift.swift @@ -66,7 +66,8 @@ extension Blog { @objc public var password: String? { get { guard let username, !username.isEmpty, - let xmlrpc, !xmlrpc.isEmpty else { + let xmlrpc, !xmlrpc.isEmpty + else { return nil } if let password = try? keychain.getPassword(for: username, serviceName: xmlrpc) { @@ -181,11 +182,13 @@ extension Blog { @objc public var timeZone: TimeZone? { let oneHourInSeconds: Double = 3600 if let name = getOptionString(name: "timezone"), !name.isEmpty, - let timeZone = TimeZone(identifier: name) { + let timeZone = TimeZone(identifier: name) + { return timeZone } if let gmtOffset = getOptionValue("gmt_offset") as? NSNumber, - let timeZone = TimeZone(secondsFromGMT: Int(gmtOffset.doubleValue * oneHourInSeconds)) { + let timeZone = TimeZone(secondsFromGMT: Int(gmtOffset.doubleValue * oneHourInSeconds)) + { return timeZone } if let value = getOptionValue("time_zone") { @@ -205,7 +208,8 @@ extension Blog { for protectionSpace in storage.allCredentials.keys { if protectionSpace.host == url.host && protectionSpace.port == (url.port ?? 80) - && protectionSpace.authenticationMethod == NSURLAuthenticationMethodHTTPBasic { + && protectionSpace.authenticationMethod == NSURLAuthenticationMethodHTTPBasic + { return true } } @@ -217,13 +221,15 @@ extension Blog { @objc public var logDescription: String { let extra: String if let account { - extra = " wp.com account: \(account.username) blogId: \(dotComID?.intValue ?? 0) plan: \(planTitle ?? "") (\(planID?.intValue ?? 0))" + extra = + " wp.com account: \(account.username) blogId: \(dotComID?.intValue ?? 0) plan: \(planTitle ?? "") (\(planID?.intValue ?? 0))" } else if let jetpack { extra = " jetpack: \(jetpack)" } else { extra = "" } - return "" + return + "" } // MARK: - Misc @@ -266,9 +272,10 @@ extension Blog { /// The blog's categories sorted alphabetically by name (case-insensitive). @objc public var sortedCategories: [PostCategory] { - (categories ?? []).sorted { - $0.categoryName.caseInsensitiveCompare($1.categoryName) == .orderedAscending - } + (categories ?? []) + .sorted { + $0.categoryName.caseInsensitiveCompare($1.categoryName) == .orderedAscending + } } /// The set of allowed file types for uploads, derived from blog options. @@ -286,7 +293,8 @@ extension Blog { public var siteVisibility: SiteVisibility { get { guard let rawValue = settings?.privacy?.intValue, - let visibility = SiteVisibility(rawValue: rawValue) else { + let visibility = SiteVisibility(rawValue: rawValue) + else { return .unknown } return visibility diff --git a/Modules/Sources/WordPressData/Swift/SharedDataIssueSolver.swift b/Modules/Sources/WordPressData/Swift/SharedDataIssueSolver.swift index 2010e58c6819..6353f634dbf7 100644 --- a/Modules/Sources/WordPressData/Swift/SharedDataIssueSolver.swift +++ b/Modules/Sources/WordPressData/Swift/SharedDataIssueSolver.swift @@ -9,11 +9,13 @@ public final class SharedDataIssueSolver { private let localFileStore: LocalFileStore private let appGroupName: String - public init(contextManager: CoreDataStack = ContextManager.shared, - keychainUtils: KeychainAccessible = KeychainUtils(), - sharedDefaults: UserPersistentRepository? = UserDefaults(suiteName: BuildSettings.current.appGroupName), - localFileStore: LocalFileStore = FileManager.default, - appGroupName: String = BuildSettings.current.appGroupName) { + public init( + contextManager: CoreDataStack = ContextManager.shared, + keychainUtils: KeychainAccessible = KeychainUtils(), + sharedDefaults: UserPersistentRepository? = UserDefaults(suiteName: BuildSettings.current.appGroupName), + localFileStore: LocalFileStore = FileManager.default, + appGroupName: String = BuildSettings.current.appGroupName + ) { self.contextManager = contextManager self.keychainUtils = keychainUtils self.sharedDefaults = sharedDefaults @@ -34,18 +36,29 @@ public final class SharedDataIssueSolver { /// public func migrateAuthKey(for username: String) { guard BuildSettings.current.brand == .jetpack, - let token = try? keychainUtils.getPassword(for: username, serviceName: WPAccountConstants.authToken.rawValue) else { + let token = try? keychainUtils.getPassword( + for: username, + serviceName: WPAccountConstants.authToken.rawValue + ) + else { return } // If the token has already been migrated, no need to resolve the issue again. // There might also be a possibility that the user logged in to JP by themselves. In which, we won't need to migrate. - if let _ = try? keychainUtils.getPassword(for: username, serviceName: WPAccountConstants.authToken.valueForJetpack) { + if let _ = try? keychainUtils.getPassword( + for: username, + serviceName: WPAccountConstants.authToken.valueForJetpack + ) { return } // if authToken for the account username exists, move it to the authToken location for JP. - try? keychainUtils.setPassword(for: username, to: token, serviceName: WPAccountConstants.authToken.valueForJetpack) + try? keychainUtils.setPassword( + for: username, + to: token, + serviceName: WPAccountConstants.authToken.valueForJetpack + ) } public func migrateExtensionsData() { @@ -137,9 +150,13 @@ private extension SharedDataIssueSolver { ] fileNames.forEach { fileName in - guard let sourceURL = localFileStore.containerURL(forAppGroup: appGroupName)?.appendingPathComponent(fileName.rawValue), - let targetURL = localFileStore.containerURL(forAppGroup: appGroupName)?.appendingPathComponent(fileName.valueForJetpack), - localFileStore.fileExists(at: sourceURL) else { + guard + let sourceURL = localFileStore.containerURL(forAppGroup: appGroupName)? + .appendingPathComponent(fileName.rawValue), + let targetURL = localFileStore.containerURL(forAppGroup: appGroupName)? + .appendingPathComponent(fileName.valueForJetpack), + localFileStore.fileExists(at: sourceURL) + else { return } diff --git a/Modules/Sources/WordPressData/Swift/WPAccount.swift b/Modules/Sources/WordPressData/Swift/WPAccount.swift index 45542d7ec325..9526b6cf0850 100644 --- a/Modules/Sources/WordPressData/Swift/WPAccount.swift +++ b/Modules/Sources/WordPressData/Swift/WPAccount.swift @@ -86,7 +86,7 @@ public class WPAccount: NSManagedObject { // MARK: - Entity Name @objc public override class func entityName() -> String { - return "Account" + "Account" } // MARK: - Lifecycle @@ -139,7 +139,10 @@ public class WPAccount: NSManagedObject { do { try keychain.setPassword(for: username, to: authToken, serviceName: keychainServiceName) } catch { - WPLogError("Error while updating or deleting WordPressComOAuthKeychainServiceName token: %@", error.localizedDescription) + WPLogError( + "Error while updating or deleting WordPressComOAuthKeychainServiceName token: %@", + error.localizedDescription + ) } } @@ -163,7 +166,10 @@ public class WPAccount: NSManagedObject { do { return try keychain.getPassword(for: username, serviceName: serviceName) } catch { - WPLogError("Error while retrieving WordPressComOAuthKeychainServiceName token: %@", error.localizedDescription) + WPLogError( + "Error while retrieving WordPressComOAuthKeychainServiceName token: %@", + error.localizedDescription + ) throw error } } diff --git a/Sources/JetpackStatsWidgets/Sources/Services/StatsWidgetsService.swift b/Sources/JetpackStatsWidgets/Sources/Services/StatsWidgetsService.swift index b8cd9089919d..4d34b423765c 100644 --- a/Sources/JetpackStatsWidgets/Sources/Services/StatsWidgetsService.swift +++ b/Sources/JetpackStatsWidgets/Sources/Services/StatsWidgetsService.swift @@ -29,8 +29,10 @@ class StatsWidgetsService { private var state: State = .ready - func fetchStats(for widgetData: HomeWidgetData, - completion: @escaping (Result) -> Void) { + func fetchStats( + for widgetData: HomeWidgetData, + completion: @escaping (Result) -> Void + ) { guard !state.isLoading else { return @@ -48,8 +50,10 @@ class StatsWidgetsService { } } - private func fetchTodayStats(widgetData: HomeWidgetTodayData, - completion: @escaping (Result) -> Void) { + private func fetchTodayStats( + widgetData: HomeWidgetTodayData, + completion: @escaping (Result) -> Void + ) { getInsight(widgetData: widgetData) { [weak self] (insight: StatsTodayInsight?, error) in guard let self else { @@ -68,15 +72,19 @@ class StatsWidgetsService { return } - let newWidgetData = HomeWidgetTodayData(siteID: widgetData.siteID, - siteName: widgetData.siteName, - url: widgetData.url, - timeZone: widgetData.timeZone, - date: Date(), - stats: TodayWidgetStats(views: insight.viewsCount, - visitors: insight.visitorsCount, - likes: insight.likesCount, - comments: insight.commentsCount)) + let newWidgetData = HomeWidgetTodayData( + siteID: widgetData.siteID, + siteName: widgetData.siteName, + url: widgetData.url, + timeZone: widgetData.timeZone, + date: Date(), + stats: TodayWidgetStats( + views: insight.viewsCount, + visitors: insight.visitorsCount, + likes: insight.likesCount, + comments: insight.commentsCount + ) + ) completion(.success(newWidgetData)) DispatchQueue.main.async { // update the item in the local cache @@ -86,8 +94,10 @@ class StatsWidgetsService { } } - private func fetchAllTimeStats(widgetData: HomeWidgetAllTimeData, - completion: @escaping (Result) -> Void) { + private func fetchAllTimeStats( + widgetData: HomeWidgetAllTimeData, + completion: @escaping (Result) -> Void + ) { getInsight(widgetData: widgetData) { [weak self] (insight: StatsAllTimesInsight?, error) in @@ -101,16 +111,20 @@ class StatsWidgetsService { return } - let newWidgetData = HomeWidgetAllTimeData(siteID: widgetData.siteID, - siteName: widgetData.siteName, - url: widgetData.url, - timeZone: widgetData.timeZone, - date: Date(), - stats: AllTimeWidgetStats(views: - insight?.viewsCount, - visitors: insight?.visitorsCount, - posts: insight?.postsCount, - bestViews: insight?.bestViewsPerDayCount)) + let newWidgetData = HomeWidgetAllTimeData( + siteID: widgetData.siteID, + siteName: widgetData.siteName, + url: widgetData.url, + timeZone: widgetData.timeZone, + date: Date(), + stats: AllTimeWidgetStats( + views: + insight?.viewsCount, + visitors: insight?.visitorsCount, + posts: insight?.postsCount, + bestViews: insight?.bestViewsPerDayCount + ) + ) completion(.success(newWidgetData)) DispatchQueue.main.async { // update the item in the local cache @@ -120,25 +134,31 @@ class StatsWidgetsService { } } - private func fetchThisWeekStats(widgetData: HomeWidgetThisWeekData, - completion: @escaping (Result) -> Void) { + private func fetchThisWeekStats( + widgetData: HomeWidgetThisWeekData, + completion: @escaping (Result) -> Void + ) { // Get the current date in the site's time zone. let siteTimeZone = widgetData.timeZone let weekEndingDate = Date().convert(from: siteTimeZone).normalizedDate() // Include an extra day. It's needed to get the dailyChange for the last day. - getData(widgetData: widgetData, - for: .day, - endingOn: weekEndingDate, - limit: ThisWeekWidgetStats.maxDaysToDisplay + 1) { [weak self] (summary: StatsSummaryTimeIntervalData?, error: Error?) in + getData( + widgetData: widgetData, + for: .day, + endingOn: weekEndingDate, + limit: ThisWeekWidgetStats.maxDaysToDisplay + 1 + ) { [weak self] (summary: StatsSummaryTimeIntervalData?, error: Error?) in guard let self else { return } if let error { - DDLogError("This Week Widget: Error fetching summary: \(String(describing: error.localizedDescription))") + DDLogError( + "This Week Widget: Error fetching summary: \(String(describing: error.localizedDescription))" + ) completion(.failure(error)) self.state = .error return @@ -156,16 +176,18 @@ class StatsWidgetsService { summaryData: summaryData.map { ThisWeekWidgetStats.Input( periodStartDate: $0.periodStartDate, - viewsCount: $0.viewsCount) + viewsCount: $0.viewsCount + ) } ) ) ) completion(.success(newWidgetData)) - DispatchQueue.global().async { - HomeWidgetThisWeekData.setItem(item: newWidgetData) - } + DispatchQueue.global() + .async { + HomeWidgetThisWeekData.setItem(item: newWidgetData) + } self.state = .ready } } @@ -190,10 +212,14 @@ private extension StatsWidgetsService { ) { do { self.service = try createStatsService(for: widgetData) - self.service?.getInsight(limit: limit, completion: { [weak self] in - completion($0, $1) - self?.service = nil - }) + self.service? + .getInsight( + limit: limit, + completion: { [weak self] in + completion($0, $1) + self?.service = nil + } + ) } catch { completion(nil, error) self.state = .error @@ -210,10 +236,17 @@ private extension StatsWidgetsService { ) { do { self.service = try createStatsService(for: widgetData) - self.service?.getData(for: period, unit: unit, endingOn: endingOn, limit: limit, completion: { [weak self] in - completion($0, $1) - self?.service = nil - }) + self.service? + .getData( + for: period, + unit: unit, + endingOn: endingOn, + limit: limit, + completion: { [weak self] in + completion($0, $1) + self?.service = nil + } + ) } catch { completion(nil, error) self.state = .error @@ -227,6 +260,10 @@ private extension StatsWidgetsService { accessGroup: BuildSettings.current.appKeychainAccessGroup ) let wpApi = WordPressComRestApi(oAuthToken: token) - return StatsServiceRemoteV2(wordPressComRestApi: wpApi, siteID: widgetData.siteID, siteTimezone: widgetData.timeZone) + return StatsServiceRemoteV2( + wordPressComRestApi: wpApi, + siteID: widgetData.siteID, + siteTimezone: widgetData.timeZone + ) } } diff --git a/Tests/KeystoneTests/Tests/Jetpack/DataMigratorTests.swift b/Tests/KeystoneTests/Tests/Jetpack/DataMigratorTests.swift index 239796887fe1..1dd9abe933bb 100644 --- a/Tests/KeystoneTests/Tests/Jetpack/DataMigratorTests.swift +++ b/Tests/KeystoneTests/Tests/Jetpack/DataMigratorTests.swift @@ -83,7 +83,10 @@ class DataMigratorTests: XCTestCase { let migratorError = getExportDataMigratorError(migrator) // Then - XCTAssertEqual(migratorError, DataMigrationError.databaseExportError(underlyingError: DataMigrationError.sharedUserDefaultsNil)) + XCTAssertEqual( + migratorError, + DataMigrationError.databaseExportError(underlyingError: DataMigrationError.sharedUserDefaultsNil) + ) } func test_importData_givenDataIsNotExported_shouldFail() { @@ -239,7 +242,7 @@ private final class CoreDataStackMock: CoreDataStack { } func newDerivedContext() -> NSManagedObjectContext { - return mainContext + mainContext } func saveContextAndWait(_ context: NSManagedObjectContext) {} @@ -247,7 +250,11 @@ private final class CoreDataStackMock: CoreDataStack { func save(_ context: NSManagedObjectContext, completion completionBlock: (() -> Void)?, on queue: DispatchQueue) {} func performAndSave(_ aBlock: @escaping (NSManagedObjectContext) -> Void) {} - func performAndSave(_ aBlock: @escaping (NSManagedObjectContext) -> Void, completion: (() -> Void)?, on queue: DispatchQueue) {} + func performAndSave( + _ aBlock: @escaping (NSManagedObjectContext) -> Void, + completion: (() -> Void)?, + on queue: DispatchQueue + ) {} } // MARK: - Helpers @@ -259,11 +266,18 @@ private extension DataMigratorTests { static let defaultsWrapperKey = "defaults_staging_dictionary" } - func createContext(for model: NSManagedObjectModel = NSManagedObjectModel.mergedModel(from: [Bundle.wordPressData])!, - type: String = NSInMemoryStoreType, - at location: URL? = nil) throws -> NSManagedObjectContext { + func createContext( + for model: NSManagedObjectModel = NSManagedObjectModel.mergedModel(from: [Bundle.wordPressData])!, + type: String = NSInMemoryStoreType, + at location: URL? = nil + ) throws -> NSManagedObjectContext { let persistentStoreCoordinator = NSPersistentStoreCoordinator(managedObjectModel: model) - try persistentStoreCoordinator.addPersistentStore(ofType: type, configurationName: nil, at: location, options: nil) + try persistentStoreCoordinator.addPersistentStore( + ofType: type, + configurationName: nil, + at: location, + options: nil + ) let managedObjectContext = NSManagedObjectContext(concurrencyType: .mainQueueConcurrencyType) managedObjectContext.persistentStoreCoordinator = persistentStoreCoordinator @@ -271,7 +285,7 @@ private extension DataMigratorTests { } func createFileContext(for model: NSManagedObjectModel, at location: URL) throws -> NSManagedObjectContext { - return try createContext(for: model, type: NSSQLiteStoreType, at: location) + try createContext(for: model, type: NSSQLiteStoreType, at: location) } func getExportDataMigratorError(_ migrator: DataMigrator) -> DataMigrationError? { @@ -289,8 +303,9 @@ private extension DataMigratorTests { func getModelNames() -> [String] { guard let modelFileURL = Bundle.wordPressData.url(forResource: "WordPress", withExtension: "momd"), - let versionInfo = NSDictionary(contentsOf: modelFileURL.appendingPathComponent("VersionInfo.plist")), - let modelNames = (versionInfo["NSManagedObjectModel_VersionHashes"] as? [String: AnyObject])?.keys else { + let versionInfo = NSDictionary(contentsOf: modelFileURL.appendingPathComponent("VersionInfo.plist")), + let modelNames = (versionInfo["NSManagedObjectModel_VersionHashes"] as? [String: AnyObject])?.keys + else { return [] } let sortedModelNames = modelNames.sorted { $0.compare($1, options: .numeric) == .orderedAscending } @@ -311,7 +326,11 @@ private extension DataMigratorTests { let momdPaths = Bundle.wordPressData.paths(forResourcesOfType: "momd", inDirectory: nil) for path in momdPaths { - if let url = Bundle.wordPressData.url(forResource: name, withExtension: "mom", subdirectory: URL(fileURLWithPath: path).lastPathComponent) { + if let url = Bundle.wordPressData.url( + forResource: name, + withExtension: "mom", + subdirectory: URL(fileURLWithPath: path).lastPathComponent + ) { return url } } @@ -322,8 +341,9 @@ private extension DataMigratorTests { func getRecentObjectModels() -> (current: NSManagedObjectModel?, previous: NSManagedObjectModel?) { let models = getModelNames() guard models.count > 1, - let currentModel = getModelObject(for: models[models.count - 1]), - let previousModel = getModelObject(for: models[models.count - 2]) else { + let currentModel = getModelObject(for: models[models.count - 1]), + let previousModel = getModelObject(for: models[models.count - 2]) + else { return (current: nil, previous: nil) } return (current: currentModel, previous: previousModel) diff --git a/Tests/KeystoneTests/Tests/Jetpack/SharedDataIssueSolverTests.swift b/Tests/KeystoneTests/Tests/Jetpack/SharedDataIssueSolverTests.swift index 503a73f709e4..21a920098ee4 100644 --- a/Tests/KeystoneTests/Tests/Jetpack/SharedDataIssueSolverTests.swift +++ b/Tests/KeystoneTests/Tests/Jetpack/SharedDataIssueSolverTests.swift @@ -120,7 +120,12 @@ private extension SharedDataIssueSolverTests { func createInMemoryContext() throws -> NSManagedObjectContext { let managedObjectModel = NSManagedObjectModel.mergedModel(from: [Bundle.main])! let persistentStoreCoordinator = NSPersistentStoreCoordinator(managedObjectModel: managedObjectModel) - try persistentStoreCoordinator.addPersistentStore(ofType: NSInMemoryStoreType, configurationName: nil, at: nil, options: nil) + try persistentStoreCoordinator.addPersistentStore( + ofType: NSInMemoryStoreType, + configurationName: nil, + at: nil, + options: nil + ) let managedObjectContext = NSManagedObjectContext(concurrencyType: .mainQueueConcurrencyType) managedObjectContext.persistentStoreCoordinator = persistentStoreCoordinator @@ -138,7 +143,7 @@ private final class CoreDataStackMock: CoreDataStack { } func newDerivedContext() -> NSManagedObjectContext { - return mainContext + mainContext } func saveContextAndWait(_ context: NSManagedObjectContext) {} @@ -146,13 +151,17 @@ private final class CoreDataStackMock: CoreDataStack { func save(_ context: NSManagedObjectContext, completion completionBlock: (() -> Void)?, on queue: DispatchQueue) {} func performAndSave(_ aBlock: @escaping (NSManagedObjectContext) -> Void) {} - func performAndSave(_ aBlock: @escaping (NSManagedObjectContext) -> Void, completion: (() -> Void)?, on queue: DispatchQueue) {} + func performAndSave( + _ aBlock: @escaping (NSManagedObjectContext) -> Void, + completion: (() -> Void)?, + on queue: DispatchQueue + ) {} } // MARK: - Mock Local File Store private final class MockLocalFileStore: LocalFileStore { - var fileShouldExistClosure: (URL?) -> Bool = { _ in return false } + var fileShouldExistClosure: (URL?) -> Bool = { _ in false } var removeItemCallCount: Int = 0 var copyItemCallCount: Int = 0 @@ -160,15 +169,15 @@ private final class MockLocalFileStore: LocalFileStore { var copyShouldThrowError: Bool = false func fileExists(at url: URL) -> Bool { - return fileShouldExistClosure(url) + fileShouldExistClosure(url) } func save(contents: Data, at url: URL) -> Bool { - return true + true } func containerURL(forAppGroup appGroup: String) -> URL? { - return URL(string: "/dev/null") + URL(string: "/dev/null") } func removeItem(at url: URL) throws { diff --git a/WordPress/Classes/Jetpack/Utility/DataMigrator.swift b/WordPress/Classes/Jetpack/Utility/DataMigrator.swift index a76c3ff2bd27..233bc6d830e9 100644 --- a/WordPress/Classes/Jetpack/Utility/DataMigrator.swift +++ b/WordPress/Classes/Jetpack/Utility/DataMigrator.swift @@ -28,13 +28,18 @@ final class DataMigrator { private let crashLogger: CrashLogging? private let appGroupName: String - init(coreDataStack: CoreDataStack = ContextManager.shared, - backupLocation: URL? = FileManager.default.containerURL(forSecurityApplicationGroupIdentifier: BuildSettings.current.appGroupName)?.appendingPathComponent("WordPress.sqlite"), - keychainUtils: KeychainAccessible = KeychainUtils(), - localDefaults: UserPersistentRepository = UserDefaults.standard, - sharedDefaults: UserPersistentRepository? = UserDefaults(suiteName: BuildSettings.current.appGroupName), - crashLogger: CrashLogging? = .main, - appGroupName: String = BuildSettings.current.appGroupName) { + init( + coreDataStack: CoreDataStack = ContextManager.shared, + backupLocation: URL? = FileManager.default.containerURL( + forSecurityApplicationGroupIdentifier: BuildSettings.current.appGroupName + )? + .appendingPathComponent("WordPress.sqlite"), + keychainUtils: KeychainAccessible = KeychainUtils(), + localDefaults: UserPersistentRepository = UserDefaults.standard, + sharedDefaults: UserPersistentRepository? = UserDefaults(suiteName: BuildSettings.current.appGroupName), + crashLogger: CrashLogging? = .main, + appGroupName: String = BuildSettings.current.appGroupName + ) { self.coreDataStack = coreDataStack self.backupLocation = backupLocation self.keychainUtils = keychainUtils @@ -101,7 +106,8 @@ extension DataMigrator: ContentDataMigrating { func deleteExportedData() { guard let backupLocation, - let sharedDefaults else { + let sharedDefaults + else { return } @@ -168,7 +174,8 @@ private extension DataMigrator { func populateFromSharedDefaults() throws { guard let sharedDefaults, - let temporaryDictionary = sharedDefaults.dictionary(forKey: DefaultsWrapper.dictKey) else { + let temporaryDictionary = sharedDefaults.dictionary(forKey: DefaultsWrapper.dictKey) + else { throw DataMigrationError.sharedUserDefaultsNil } for (key, value) in temporaryDictionary { diff --git a/WordPress/Classes/Services/CredentialsService.swift b/WordPress/Classes/Services/CredentialsService.swift index 70110d9d105b..63d1e3488d5a 100644 --- a/WordPress/Classes/Services/CredentialsService.swift +++ b/WordPress/Classes/Services/CredentialsService.swift @@ -7,7 +7,7 @@ protocol CredentialsProvider { struct KeychainCredentialsProvider: CredentialsProvider { func getPassword(username: String, service: String) -> String? { - return try? SFHFKeychainUtils.getPasswordForUsername(username, andServiceName: service) + try? SFHFKeychainUtils.getPasswordForUsername(username, andServiceName: service) } } @@ -19,6 +19,6 @@ class CredentialsService { } func getOAuthToken(site: JetpackSiteRef) -> String? { - return provider.getPassword(username: site.username, service: BuildSettings.current.authKeychainServiceName) + provider.getPassword(username: site.username, service: BuildSettings.current.authKeychainServiceName) } } diff --git a/WordPress/Classes/Services/NotificationSupportService.swift b/WordPress/Classes/Services/NotificationSupportService.swift index b9cf64848724..2904f8f0ed7b 100644 --- a/WordPress/Classes/Services/NotificationSupportService.swift +++ b/WordPress/Classes/Services/NotificationSupportService.swift @@ -15,8 +15,10 @@ final class NotificationSupportService { ) } - init(appKeychainAccessGroup: String, - configuration: NotificationServiceExtensionConfiguration) { + init( + appKeychainAccessGroup: String, + configuration: NotificationServiceExtensionConfiguration + ) { self.appKeychainAccessGroup = appKeychainAccessGroup self.configuration = configuration } diff --git a/WordPress/Classes/Utility/Editor/EditorConfiguration+Blog.swift b/WordPress/Classes/Utility/Editor/EditorConfiguration+Blog.swift index bd24cb4c353a..d4b8f6c85a54 100644 --- a/WordPress/Classes/Utility/Editor/EditorConfiguration+Blog.swift +++ b/WordPress/Classes/Utility/Editor/EditorConfiguration+Blog.swift @@ -14,7 +14,8 @@ extension EditorConfiguration { if applicationPassword != nil { siteApiRootString = selfHostedApiUrl } else { - siteApiRootString = shouldUseWPComRestApi ? blog.wordPressComRestApi?.baseURL.absoluteString : selfHostedApiUrl + siteApiRootString = + shouldUseWPComRestApi ? blog.wordPressComRestApi?.baseURL.absoluteString : selfHostedApiUrl } let siteId = blog.dotComID?.stringValue @@ -48,15 +49,15 @@ extension EditorConfiguration { siteURL: siteURL, siteApiRoot: siteApiRoot ) - .setSiteApiNamespace(siteApiNamespace) - .setNamespaceExcludedPaths(["/wpcom/v2/following/recommendations", "/wpcom/v2/following/mine"]) - .setAuthHeader(authHeader) - .setShouldUseThemeStyles(GutenbergSettings().isThemeStylesEnabled(for: blog)) - // Limited to Jetpack-connected sites until editor assets endpoint is available in WordPress core - .setShouldUsePlugins(Self.shouldEnablePlugins(for: blog, appPassword: applicationPassword)) - .setLocale(WordPressComLanguageDatabase.shared.deviceLanguage.slug) - .setEnableNetworkLogging(ExtensiveLogging.enabled) - .setNetworkFallbackMode(.automatic) + .setSiteApiNamespace(siteApiNamespace) + .setNamespaceExcludedPaths(["/wpcom/v2/following/recommendations", "/wpcom/v2/following/mine"]) + .setAuthHeader(authHeader) + .setShouldUseThemeStyles(GutenbergSettings().isThemeStylesEnabled(for: blog)) + // Limited to Jetpack-connected sites until editor assets endpoint is available in WordPress core + .setShouldUsePlugins(Self.shouldEnablePlugins(for: blog, appPassword: applicationPassword)) + .setLocale(WordPressComLanguageDatabase.shared.deviceLanguage.slug) + .setEnableNetworkLogging(ExtensiveLogging.enabled) + .setNetworkFallbackMode(.automatic) // Build editor assets endpoint var editorAssetsEndpoint = siteApiRoot @@ -77,8 +78,7 @@ extension EditorConfiguration { // Requires a Jetpack until editor assets endpoint is available in WordPress core. // Requires a WP.com Simple site or an application password to authenticate all REST // API requests, including those originating from non-core blocks. - return RemoteFeatureFlag.newGutenbergPlugins.enabled() && - blog.isAccessibleThroughWPCom && - (blog.isHostedAtWPcom || appPassword != nil) + RemoteFeatureFlag.newGutenbergPlugins.enabled() && blog.isAccessibleThroughWPCom + && (blog.isHostedAtWPcom || appPassword != nil) } } diff --git a/WordPress/WordPressNotificationServiceExtension/Sources/NotificationService.swift b/WordPress/WordPressNotificationServiceExtension/Sources/NotificationService.swift index 4698e6a412b1..7860e0878a40 100644 --- a/WordPress/WordPressNotificationServiceExtension/Sources/NotificationService.swift +++ b/WordPress/WordPressNotificationServiceExtension/Sources/NotificationService.swift @@ -41,7 +41,10 @@ class NotificationService: UNNotificationServiceExtension { // MARK: UNNotificationServiceExtension - override func didReceive(_ request: UNNotificationRequest, withContentHandler contentHandler: @escaping (UNNotificationContent) -> Void) { + override func didReceive( + _ request: UNNotificationRequest, + withContentHandler contentHandler: @escaping (UNNotificationContent) -> Void + ) { self.contentHandler = contentHandler self.bestAttemptContent = request.content.mutableCopy() as? UNMutableNotificationContent @@ -58,7 +61,8 @@ class NotificationService: UNNotificationServiceExtension { let apsAlert = notificationContent.apsAlert, let notificationType = notificationContent.type, let notificationKind = NotificationKind(rawValue: notificationType), - token != nil else { + token != nil + else { let hasToken = token != nil tracks.trackNotificationMalformed(hasToken: hasToken, notificationBody: request.content.body) @@ -90,7 +94,8 @@ class NotificationService: UNNotificationServiceExtension { // comment likes will always have a bolded title. if notificationContent.title.isEmpty, !notificationContent.body.isEmpty, - !NotificationKind.omitsRichNotificationBody(notificationKind) { + !NotificationKind.omitsRichNotificationBody(notificationKind) + { notificationContent.title = notificationContent.body notificationContent.body = "" } @@ -112,7 +117,8 @@ class NotificationService: UNNotificationServiceExtension { gravatarURLString: nil, notificationIdentifier: nil, notificationReadStatus: true, - noticon: nil) + noticon: nil + ) notificationContent.userInfo[CodingUserInfoKey.richNotificationViewModel.rawValue] = viewModel.data contentHandler(notificationContent) @@ -132,14 +138,18 @@ class NotificationService: UNNotificationServiceExtension { service.loadNotes(noteIds: [noteID]) { [self, tracks] error, notifications in if let error { - tracks.trackNotificationRetrievalFailed(notificationIdentifier: noteID, errorDescription: error.localizedDescription) + tracks.trackNotificationRetrievalFailed( + notificationIdentifier: noteID, + errorDescription: error.localizedDescription + ) contentHandler(notificationContent) return } guard let remoteNotifications = notifications, remoteNotifications.count == 1, - let notification = remoteNotifications.first else { + let notification = remoteNotifications.first + else { contentHandler(notificationContent) return } @@ -152,21 +162,25 @@ class NotificationService: UNNotificationServiceExtension { gravatarURLString: notification.icon, notificationIdentifier: notification.notificationId, notificationReadStatus: notification.read, - noticon: notification.noticon) + noticon: notification.noticon + ) // Only populate title / body for notification kinds with rich body content if !NotificationKind.omitsRichNotificationBody(notificationKind) { notificationContent.title = contentFormatter.attributedSubject?.string ?? apsAlert // Improve the notification body by trimming whitespace and reducing any multiple blank lines - notificationContent.body = contentFormatter.attributedBody?.string.condenseWhitespace().truncate(with: 256) ?? "" + notificationContent.body = + contentFormatter.attributedBody?.string.condenseWhitespace().truncate(with: 256) ?? "" } notificationContent.userInfo[CodingUserInfoKey.richNotificationViewModel.rawValue] = viewModel.data tracks.trackNotificationAssembled() - let iconURL = NotificationKind.isNotificationIconSupported(notificationKind) ? notification.icon.flatMap(URL.init) : nil + let iconURL = + NotificationKind.isNotificationIconSupported(notificationKind) + ? notification.icon.flatMap(URL.init) : nil // If the notification contains any image media, download it and // attach it to the notification. @@ -184,7 +198,10 @@ class NotificationService: UNNotificationServiceExtension { guard let self, let data, let fileExtension, - let fileURL = self.saveMediaAttachment(data: data, fileName: String(format: "%@.%@", identifier, fileExtension)) + let fileURL = self.saveMediaAttachment( + data: data, + fileName: String(format: "%@.%@", identifier, fileExtension) + ) else { return } @@ -192,7 +209,8 @@ class NotificationService: UNNotificationServiceExtension { let imageAttachment = try? UNNotificationAttachment( identifier: identifier, url: fileURL, - options: nil) + options: nil + ) if let imageAttachment { notificationContent.attachments = [imageAttachment] @@ -207,7 +225,8 @@ class NotificationService: UNNotificationServiceExtension { notificationService?.wordPressComRestApi.invalidateAndCancelTasks() if let contentHandler, - let bestAttemptContent { + let bestAttemptContent + { contentHandler(bestAttemptContent) } @@ -265,12 +284,15 @@ private extension NotificationService { let directoryPath = URL.Helpers.temporaryDirectory(named: ProcessInfo.processInfo.globallyUniqueString) do { - try FileManager.default.createDirectory(at: directoryPath, withIntermediateDirectories: true, attributes: nil) + try FileManager.default.createDirectory( + at: directoryPath, + withIntermediateDirectories: true, + attributes: nil + ) let fileURL = directoryPath.appendingPathComponent(fileName) try data.write(to: fileURL) return fileURL - } - catch { + } catch { return nil } } @@ -298,11 +320,13 @@ private extension NotificationService { /// - Returns: the token if found; `nil` otherwise /// func readExtensionToken() -> String? { - guard let oauthToken = try? SFHFKeychainUtils.getPasswordForUsername( - configuration.keychainTokenKey, - andServiceName: configuration.keychainServiceName, - accessGroup: appKeychainAccessGroup - ) else { + guard + let oauthToken = try? SFHFKeychainUtils.getPasswordForUsername( + configuration.keychainTokenKey, + andServiceName: configuration.keychainServiceName, + accessGroup: appKeychainAccessGroup + ) + else { debugPrint("Unable to retrieve Notification Service Extension OAuth token") return nil } @@ -315,11 +339,13 @@ private extension NotificationService { /// - Returns: the username if found; `nil` otherwise /// func readExtensionUsername() -> String? { - guard let username = try? SFHFKeychainUtils.getPasswordForUsername( - configuration.keychainUsernameKey, - andServiceName: configuration.keychainServiceName, - accessGroup: appKeychainAccessGroup - ) else { + guard + let username = try? SFHFKeychainUtils.getPasswordForUsername( + configuration.keychainUsernameKey, + andServiceName: configuration.keychainServiceName, + accessGroup: appKeychainAccessGroup + ) + else { debugPrint("Unable to retrieve Notification Service Extension username") return nil } @@ -332,11 +358,13 @@ private extension NotificationService { /// - Returns: the userID if found; `nil` otherwise /// func readExtensionUserID() -> String? { - guard let userID = try? SFHFKeychainUtils.getPasswordForUsername( - configuration.keychainUserIDKey, - andServiceName: configuration.keychainServiceName, - accessGroup: appKeychainAccessGroup - ) else { + guard + let userID = try? SFHFKeychainUtils.getPasswordForUsername( + configuration.keychainUserIDKey, + andServiceName: configuration.keychainServiceName, + accessGroup: appKeychainAccessGroup + ) + else { debugPrint("Unable to retrieve Notification Service Extension userID") return nil } @@ -353,5 +381,8 @@ private extension NotificationService { return content } - static let viewMilestoneTitle = AppLocalizedString("You hit a milestone 🚀", comment: "Title for a view milestone push notification") + static let viewMilestoneTitle = AppLocalizedString( + "You hit a milestone 🚀", + comment: "Title for a view milestone push notification" + ) } From 81e2bf20b6056629b82f40ad281acaa90404c3d9 Mon Sep 17 00:00:00 2001 From: Tony Li Date: Sun, 14 Jun 2026 18:43:05 +1200 Subject: [PATCH 02/13] Define the per-app and shared keychain access group build settings --- Modules/Sources/BuildSettingsKit/BuildSettings+Live.swift | 1 + Modules/Sources/BuildSettingsKit/BuildSettings+Preview.swift | 1 + Modules/Sources/BuildSettingsKit/BuildSettings.swift | 4 ++++ Sources/Jetpack/Info.plist | 2 ++ Sources/WordPress/Info.plist | 2 ++ config/Common.alpha.xcconfig | 2 +- config/Common.debug.xcconfig | 2 +- config/Common.release.xcconfig | 2 +- config/Jetpack.alpha.xcconfig | 1 + config/Jetpack.debug.xcconfig | 1 + config/Jetpack.release.xcconfig | 1 + config/Reader.alpha.xcconfig | 1 + config/Reader.debug.xcconfig | 1 + config/Reader.release.xcconfig | 1 + config/WordPress.alpha.xcconfig | 1 + config/WordPress.debug.xcconfig | 1 + config/WordPress.release.xcconfig | 1 + 17 files changed, 22 insertions(+), 3 deletions(-) diff --git a/Modules/Sources/BuildSettingsKit/BuildSettings+Live.swift b/Modules/Sources/BuildSettingsKit/BuildSettings+Live.swift index 526e8f434070..56b6bbfcef6a 100644 --- a/Modules/Sources/BuildSettingsKit/BuildSettings+Live.swift +++ b/Modules/Sources/BuildSettingsKit/BuildSettings+Live.swift @@ -10,6 +10,7 @@ extension BuildSettings { pushNotificationAppID = bundle.infoValue(forKey: "WPPushNotificationAppID") appGroupName = bundle.infoValue(forKey: "WPAppGroupName") appKeychainAccessGroup = bundle.infoValue(forKey: "WPAppKeychainAccessGroup") + sharedKeychainAccessGroup = bundle.object(forInfoDictionaryKey: "WPSharedKeychainAccessGroup") as? String eventNamePrefix = bundle.infoValue(forKey: "WPEventNamePrefix") explatPlatform = bundle.infoValue(forKey: "WPExplatPlatform") itunesAppID = bundle.infoValue(forKey: "WPItunesAppID") diff --git a/Modules/Sources/BuildSettingsKit/BuildSettings+Preview.swift b/Modules/Sources/BuildSettingsKit/BuildSettings+Preview.swift index 7b0b72872d85..fedbdd1bf0a1 100644 --- a/Modules/Sources/BuildSettingsKit/BuildSettings+Preview.swift +++ b/Modules/Sources/BuildSettingsKit/BuildSettings+Preview.swift @@ -9,6 +9,7 @@ extension BuildSettings { pushNotificationAppID: "xcpreview_push_notification_id", appGroupName: "xcpreview_app_group_name", appKeychainAccessGroup: "xcpreview_app_keychain_access_group", + sharedKeychainAccessGroup: "xcpreview_shared_keychain_access_group", eventNamePrefix: "xcpreview", explatPlatform: "xcpreview", itunesAppID: "1234567890", diff --git a/Modules/Sources/BuildSettingsKit/BuildSettings.swift b/Modules/Sources/BuildSettingsKit/BuildSettings.swift index bca0495766ad..439fdbabd164 100644 --- a/Modules/Sources/BuildSettingsKit/BuildSettings.swift +++ b/Modules/Sources/BuildSettingsKit/BuildSettings.swift @@ -25,6 +25,10 @@ public struct BuildSettings: Sendable { public var pushNotificationAppID: String public var appGroupName: String public var appKeychainAccessGroup: String + /// The legacy cross-app keychain group shared by the WordPress and + /// Jetpack apps. nil where the app has no shared-group entitlement + /// (Reader): the key is simply absent from that app's Info.plist. + public var sharedKeychainAccessGroup: String? public var eventNamePrefix: String public var explatPlatform: String public var itunesAppID: String diff --git a/Sources/Jetpack/Info.plist b/Sources/Jetpack/Info.plist index 5e59e5044a0c..b840ff661d1e 100644 --- a/Sources/Jetpack/Info.plist +++ b/Sources/Jetpack/Info.plist @@ -653,6 +653,8 @@ ${WP_APP_GROUP_NAME} WPAppKeychainAccessGroup ${WP_APP_KEYCHAIN_ACCESS_GROUP} + WPSharedKeychainAccessGroup + ${WP_SHARED_KEYCHAIN_ACCESS_GROUP} WPAppURLScheme ${WP_APP_URL_SCHEME} WPAuthKeychainServiceName diff --git a/Sources/WordPress/Info.plist b/Sources/WordPress/Info.plist index 40582e9aa0af..671bd0006942 100644 --- a/Sources/WordPress/Info.plist +++ b/Sources/WordPress/Info.plist @@ -578,6 +578,8 @@ ${WP_APP_GROUP_NAME} WPAppKeychainAccessGroup ${WP_APP_KEYCHAIN_ACCESS_GROUP} + WPSharedKeychainAccessGroup + ${WP_SHARED_KEYCHAIN_ACCESS_GROUP} WPAppURLScheme ${WP_APP_URL_SCHEME} WPAuthKeychainServiceName diff --git a/config/Common.alpha.xcconfig b/config/Common.alpha.xcconfig index 2a0341d12978..1ee3f73db0a4 100644 --- a/config/Common.alpha.xcconfig +++ b/config/Common.alpha.xcconfig @@ -3,5 +3,5 @@ DEVELOPMENT_TEAM = 99KV9Z6BKV WP_APP_GROUP_NAME = group.org.wordpress.alpha -WP_APP_KEYCHAIN_ACCESS_GROUP = 99KV9Z6BKV.org.wordpress.alpha +WP_SHARED_KEYCHAIN_ACCESS_GROUP = 99KV9Z6BKV.org.wordpress.alpha WP_JETPACK_APP_URL_SCHEME = jpalpha diff --git a/config/Common.debug.xcconfig b/config/Common.debug.xcconfig index 5852376609aa..93d5aef30d40 100644 --- a/config/Common.debug.xcconfig +++ b/config/Common.debug.xcconfig @@ -4,5 +4,5 @@ DEVELOPMENT_TEAM = PZYM8XX95Q CODE_SIGN_IDENTITY=Apple Development: Created via API (886NX39KP6) WP_APP_GROUP_NAME = group.org.wordpress -WP_APP_KEYCHAIN_ACCESS_GROUP = 3TMU3BH3NK.org.wordpress +WP_SHARED_KEYCHAIN_ACCESS_GROUP = 3TMU3BH3NK.org.wordpress WP_JETPACK_APP_URL_SCHEME = jpdebug diff --git a/config/Common.release.xcconfig b/config/Common.release.xcconfig index db55ac0d03dd..baf04f0f8fe9 100644 --- a/config/Common.release.xcconfig +++ b/config/Common.release.xcconfig @@ -4,5 +4,5 @@ DEVELOPMENT_TEAM = PZYM8XX95Q CODE_SIGN_IDENTITY=Apple Distribution: Automattic, Inc. (${DEVELOPMENT_TEAM}) WP_APP_GROUP_NAME = group.org.wordpress -WP_APP_KEYCHAIN_ACCESS_GROUP = 3TMU3BH3NK.org.wordpress +WP_SHARED_KEYCHAIN_ACCESS_GROUP = 3TMU3BH3NK.org.wordpress WP_JETPACK_APP_URL_SCHEME = jetpack diff --git a/config/Jetpack.alpha.xcconfig b/config/Jetpack.alpha.xcconfig index c5b040eaad64..fcefc285a9fa 100644 --- a/config/Jetpack.alpha.xcconfig +++ b/config/Jetpack.alpha.xcconfig @@ -4,3 +4,4 @@ WP_PUSH_NOTIFICATION_APP_ID = com.jetpack.alpha WP_BUILD_CONFIGURATION = alpha WP_APP_URL_SCHEME = ${WP_JETPACK_APP_URL_SCHEME} +WP_APP_KEYCHAIN_ACCESS_GROUP = 99KV9Z6BKV.org.wordpress.alpha.jetpack diff --git a/config/Jetpack.debug.xcconfig b/config/Jetpack.debug.xcconfig index be99f837d875..c6232f0369d4 100644 --- a/config/Jetpack.debug.xcconfig +++ b/config/Jetpack.debug.xcconfig @@ -4,3 +4,4 @@ WP_PUSH_NOTIFICATION_APP_ID = com.jetpack.appstore.dev WP_BUILD_CONFIGURATION = debug WP_APP_URL_SCHEME = ${WP_JETPACK_APP_URL_SCHEME} +WP_APP_KEYCHAIN_ACCESS_GROUP = 3TMU3BH3NK.org.wordpress.jetpack diff --git a/config/Jetpack.release.xcconfig b/config/Jetpack.release.xcconfig index dc670c3ee62b..a96e111d34de 100644 --- a/config/Jetpack.release.xcconfig +++ b/config/Jetpack.release.xcconfig @@ -4,3 +4,4 @@ WP_PUSH_NOTIFICATION_APP_ID = com.jetpack.appstore WP_BUILD_CONFIGURATION = release WP_APP_URL_SCHEME = ${WP_JETPACK_APP_URL_SCHEME} +WP_APP_KEYCHAIN_ACCESS_GROUP = 3TMU3BH3NK.org.wordpress.jetpack diff --git a/config/Reader.alpha.xcconfig b/config/Reader.alpha.xcconfig index 17b40f97ba3b..3308778fc082 100644 --- a/config/Reader.alpha.xcconfig +++ b/config/Reader.alpha.xcconfig @@ -6,5 +6,6 @@ PRODUCT_BUNDLE_IDENTIFIER = com.automattic.readerapp WP_PUSH_NOTIFICATION_APP_ID = TBD WP_BUILD_CONFIGURATION = alpha WP_APP_URL_SCHEME = wpreader +WP_APP_KEYCHAIN_ACCESS_GROUP = 99KV9Z6BKV.org.wordpress.alpha.reader CODE_SIGN_ENTITLEMENTS = ../Sources/Reader/Reader.entitlements diff --git a/config/Reader.debug.xcconfig b/config/Reader.debug.xcconfig index 6dbe490d37a1..bbc19c4bc6b9 100644 --- a/config/Reader.debug.xcconfig +++ b/config/Reader.debug.xcconfig @@ -6,6 +6,7 @@ PRODUCT_BUNDLE_IDENTIFIER = com.automattic.readerapp WP_PUSH_NOTIFICATION_APP_ID = TBD WP_BUILD_CONFIGURATION = debug WP_APP_URL_SCHEME = wpreader +WP_APP_KEYCHAIN_ACCESS_GROUP = 3TMU3BH3NK.org.wordpress.reader PROVISIONING_PROFILE_SPECIFIER = Reader Development CODE_SIGN_ENTITLEMENTS = ../Sources/Reader/Reader.entitlements diff --git a/config/Reader.release.xcconfig b/config/Reader.release.xcconfig index b2e88a878233..ff80d6c556bc 100644 --- a/config/Reader.release.xcconfig +++ b/config/Reader.release.xcconfig @@ -6,6 +6,7 @@ PRODUCT_BUNDLE_IDENTIFIER = com.automattic.readerapp WP_PUSH_NOTIFICATION_APP_ID = TBD WP_BUILD_CONFIGURATION = release WP_APP_URL_SCHEME = wpreader +WP_APP_KEYCHAIN_ACCESS_GROUP = 3TMU3BH3NK.org.wordpress.reader PROVISIONING_PROFILE_SPECIFIER = match AppStore ${PRODUCT_BUNDLE_IDENTIFIER} CODE_SIGN_ENTITLEMENTS = ../Sources/Reader/Reader.entitlements diff --git a/config/WordPress.alpha.xcconfig b/config/WordPress.alpha.xcconfig index 9a52d299471c..5c1e51e1d6a3 100644 --- a/config/WordPress.alpha.xcconfig +++ b/config/WordPress.alpha.xcconfig @@ -4,3 +4,4 @@ WP_PUSH_NOTIFICATION_APP_ID = org.wordpress.alpha WP_BUILD_CONFIGURATION = alpha WP_APP_URL_SCHEME = wpalpha +WP_APP_KEYCHAIN_ACCESS_GROUP = 99KV9Z6BKV.org.wordpress.alpha.wordpress diff --git a/config/WordPress.debug.xcconfig b/config/WordPress.debug.xcconfig index 30c0938a5e1f..aa405759f405 100644 --- a/config/WordPress.debug.xcconfig +++ b/config/WordPress.debug.xcconfig @@ -4,3 +4,4 @@ WP_PUSH_NOTIFICATION_APP_ID = org.wordpress.appstore.dev WP_BUILD_CONFIGURATION = debug WP_APP_URL_SCHEME = wpdebug +WP_APP_KEYCHAIN_ACCESS_GROUP = 3TMU3BH3NK.org.wordpress.wordpress diff --git a/config/WordPress.release.xcconfig b/config/WordPress.release.xcconfig index ab24ee2d95f2..d1eb30e66b8c 100644 --- a/config/WordPress.release.xcconfig +++ b/config/WordPress.release.xcconfig @@ -4,3 +4,4 @@ WP_PUSH_NOTIFICATION_APP_ID = org.wordpress.appstore WP_BUILD_CONFIGURATION = release WP_APP_URL_SCHEME = wordpress +WP_APP_KEYCHAIN_ACCESS_GROUP = 3TMU3BH3NK.org.wordpress.wordpress From 1cd24304b348145afffa5624c42a49af40511061 Mon Sep 17 00:00:00 2001 From: Tony Li Date: Sun, 14 Jun 2026 18:43:05 +1200 Subject: [PATCH 03/13] Add app-private and shared keychain wrappers with read-repair --- Modules/Package.swift | 1 + .../Keychain/AppKeychain.swift | 126 ++++++++++++++++ .../Keychain/AuthTokenServiceNames.swift | 9 ++ .../KeychainErrorClassification.swift | 19 +++ .../Keychain/SharedKeychain.swift | 62 ++++++++ .../AppKeychainTests.swift | 142 ++++++++++++++++++ .../KeychainErrorClassificationTests.swift | 22 +++ .../WordPressSharedTests/KeychainStub.swift | 84 +++++++++++ .../SharedKeychainTests.swift | 47 ++++++ 9 files changed, 512 insertions(+) create mode 100644 Modules/Sources/WordPressShared/Keychain/AppKeychain.swift create mode 100644 Modules/Sources/WordPressShared/Keychain/AuthTokenServiceNames.swift create mode 100644 Modules/Sources/WordPressShared/Keychain/KeychainErrorClassification.swift create mode 100644 Modules/Sources/WordPressShared/Keychain/SharedKeychain.swift create mode 100644 Modules/Tests/WordPressSharedTests/AppKeychainTests.swift create mode 100644 Modules/Tests/WordPressSharedTests/KeychainErrorClassificationTests.swift create mode 100644 Modules/Tests/WordPressSharedTests/KeychainStub.swift create mode 100644 Modules/Tests/WordPressSharedTests/SharedKeychainTests.swift diff --git a/Modules/Package.swift b/Modules/Package.swift index e3dbf2d02e42..577f7b0c398c 100644 --- a/Modules/Package.swift +++ b/Modules/Package.swift @@ -238,6 +238,7 @@ let package = Package( .target( name: "WordPressShared", dependencies: [ + "BuildSettingsKit", .product(name: "SwiftSoup", package: "SwiftSoup"), .target(name: "SFHFKeychainUtils"), .target(name: "WordPressSharedObjC") diff --git a/Modules/Sources/WordPressShared/Keychain/AppKeychain.swift b/Modules/Sources/WordPressShared/Keychain/AppKeychain.swift new file mode 100644 index 000000000000..3fdf67e378bb --- /dev/null +++ b/Modules/Sources/WordPressShared/Keychain/AppKeychain.swift @@ -0,0 +1,126 @@ +import BuildSettingsKit +import Foundation +import SFHFKeychainUtils + +/// Keychain access scoped to this app family's private access group (the +/// app and its extensions, e.g. "3TMU3BH3NK.org.wordpress.jetpack"). +/// +/// Routing rules: +/// - reads: private group first, then a read-only fallback to the legacy +/// shared group (transition only; removed once pre-change app +/// versions are negligible) +/// - writes: private group, always +/// - deletes: both groups, so a logout cannot resurrect a credential +/// through the fallback read +/// +/// Use `SharedKeychain` instead for the WordPress-to-Jetpack migration +/// contract, the only data deliberately shared across apps. +public final class AppKeychain: KeychainAccessible { + private let privateGroup: String + private let sharedGroup: String? + private let keychainUtils: SFHFKeychainUtils.Type + + public convenience init() { + let settings = BuildSettings.current + self.init( + privateGroup: settings.appKeychainAccessGroup, + sharedGroup: settings.sharedKeychainAccessGroup + ) + } + + init( + privateGroup: String, + sharedGroup: String?, + keychainUtils: SFHFKeychainUtils.Type = SFHFKeychainUtils.self + ) { + self.privateGroup = privateGroup + self.sharedGroup = sharedGroup + self.keychainUtils = keychainUtils + } + + public func getPassword(for username: String, serviceName: String) throws -> String { + do { + return try keychainUtils.getPasswordForUsername( + username, + andServiceName: serviceName, + accessGroup: privateGroup + ) + } catch { + // A real failure (for example errSecInteractionNotAllowed while the + // device is locked) must surface: the fallback is long-lived now, + // so masking it as not-found would be permanent. Fall back only on + // a genuine not-found of the private read, and only when a shared + // group exists. + guard !isRealKeychainFailure(error), let sharedGroup else { throw error } + let value = try keychainUtils.getPasswordForUsername( + username, + andServiceName: serviceName, + accessGroup: sharedGroup + ) + // Read-repair: migrate the item into the private group so future + // reads stop depending on the shared-group fallback. Best-effort, + // the read already succeeded; the next read retries the repair. + try? keychainUtils.storeUsername( + username, + andPassword: value, + forServiceName: serviceName, + accessGroup: privateGroup, + updateExisting: true + ) + return value + } + } + + public func setPassword(for username: String, to newValue: String?, serviceName: String) throws { + guard let newValue else { + // Delete from both groups, attempting the private-group delete even + // when the shared-group delete fails. A logout must not leave a + // credential in the shared group, where the fallback read (and + // read-repair) could otherwise resurrect it. Delete the shared group + // first: it is the only group the fallback reads, so an interruption + // between the two deletes can never leave the resurrectable + // "private empty, shared present" state. Surface the first real + // failure only after both deletes have been attempted. + var deleteFailure: Error? + if let sharedGroup { + do { + try deleteIgnoringNotFound(username, serviceName: serviceName, accessGroup: sharedGroup) + } catch { + deleteFailure = error + } + } + do { + try deleteIgnoringNotFound(username, serviceName: serviceName, accessGroup: privateGroup) + } catch { + deleteFailure = deleteFailure ?? error + } + if let deleteFailure { + throw deleteFailure + } + return + } + try keychainUtils.storeUsername( + username, + andPassword: newValue, + forServiceName: serviceName, + accessGroup: privateGroup, + updateExisting: true + ) + } + + private func deleteIgnoringNotFound(_ username: String, serviceName: String, accessGroup: String) throws { + do { + try keychainUtils.deleteItem( + forUsername: username, + andServiceName: serviceName, + accessGroup: accessGroup + ) + } catch { + // Deleting a missing item is expected: the item usually exists + // in only one of the two groups. Anything else (for example + // errSecInteractionNotAllowed while the device is locked) must + // surface, or a logout could silently leave a credential behind. + guard !isRealKeychainFailure(error) else { throw error } + } + } +} diff --git a/Modules/Sources/WordPressShared/Keychain/AuthTokenServiceNames.swift b/Modules/Sources/WordPressShared/Keychain/AuthTokenServiceNames.swift new file mode 100644 index 000000000000..24250444e983 --- /dev/null +++ b/Modules/Sources/WordPressShared/Keychain/AuthTokenServiceNames.swift @@ -0,0 +1,9 @@ +/// Keychain service names for the WP.com OAuth tokens. These form a +/// cross-app contract (see `SharedKeychain`): the WordPress app publishes +/// its token under `wordPress` for the Jetpack app to import, the group +/// sweep preserves that item, and each app's copy skips the other's token. +/// Old app versions hardcode both values, so they must never change. +public enum AuthTokenServiceNames { + public static let wordPress = "public-api.wordpress.com" + public static let jetpack = "jetpack.public-api.wordpress.com" +} diff --git a/Modules/Sources/WordPressShared/Keychain/KeychainErrorClassification.swift b/Modules/Sources/WordPressShared/Keychain/KeychainErrorClassification.swift new file mode 100644 index 000000000000..deb93f9beca8 --- /dev/null +++ b/Modules/Sources/WordPressShared/Keychain/KeychainErrorClassification.swift @@ -0,0 +1,19 @@ +import Foundation +import Security + +/// The error domain `SFHFKeychainUtils` uses for its own failures. Shared so +/// the classifier and its tests cannot drift to different spellings. +let sfhfKeychainErrorDomain = "SFHFKeychainUtilsErrorDomain" + +/// Classifies errors thrown by `SFHFKeychainUtils`. +/// +/// `SFHFKeychainUtils` populates an `NSError` in its own domain (with the raw +/// `OSStatus` as the code) only for real failures. A not-found surfaces either +/// as a nil result that Swift bridges to a generic non-SFHF error (reads), or +/// as an SFHF error whose code is `errSecItemNotFound` (deletes). So a real +/// failure is exactly an SFHF-domain error whose code is not `errSecItemNotFound`. +public func isRealKeychainFailure(_ error: Error) -> Bool { + let nsError = error as NSError + return nsError.domain == sfhfKeychainErrorDomain + && nsError.code != Int(errSecItemNotFound) +} diff --git a/Modules/Sources/WordPressShared/Keychain/SharedKeychain.swift b/Modules/Sources/WordPressShared/Keychain/SharedKeychain.swift new file mode 100644 index 000000000000..194abc385e83 --- /dev/null +++ b/Modules/Sources/WordPressShared/Keychain/SharedKeychain.swift @@ -0,0 +1,62 @@ +import BuildSettingsKit +import Foundation +import SFHFKeychainUtils + +/// Keychain access scoped to the legacy cross-app shared access group +/// ("3TMU3BH3NK.org.wordpress"). +/// +/// The ONLY permitted users are the WordPress-to-Jetpack migration contract: +/// 1. DataMigrator.exportData (WordPress: publish the WP.com token) +/// 2. DataMigrator.deleteExportedData (WordPress: remove it on logout) +/// 3. SharedDataIssueSolver.migrateAuthKey (Jetpack: read the token) +/// +/// Anything else belongs in `AppKeychain`. A new SharedKeychain call site +/// means a new cross-app data flow; treat it as a design change. +public final class SharedKeychain: KeychainAccessible { + private let group: String + private let keychainUtils: SFHFKeychainUtils.Type + + /// Fails where the app has no shared-group entitlement (Reader). + public convenience init?() { + self.init(group: BuildSettings.current.sharedKeychainAccessGroup) + } + + init?(group: String?, keychainUtils: SFHFKeychainUtils.Type = SFHFKeychainUtils.self) { + guard let group else { return nil } + self.group = group + self.keychainUtils = keychainUtils + } + + public func getPassword(for username: String, serviceName: String) throws -> String { + try keychainUtils.getPasswordForUsername( + username, + andServiceName: serviceName, + accessGroup: group + ) + } + + public func setPassword(for username: String, to newValue: String?, serviceName: String) throws { + if let newValue { + try keychainUtils.storeUsername( + username, + andPassword: newValue, + forServiceName: serviceName, + accessGroup: group, + updateExisting: true + ) + } else { + do { + try keychainUtils.deleteItem( + forUsername: username, + andServiceName: serviceName, + accessGroup: group + ) + } catch { + // Deleting an already-absent item is success: the migration + // cleanup path removes a token that may legitimately be gone. + // Real failures must surface, same as AppKeychain. + guard !isRealKeychainFailure(error) else { throw error } + } + } + } +} diff --git a/Modules/Tests/WordPressSharedTests/AppKeychainTests.swift b/Modules/Tests/WordPressSharedTests/AppKeychainTests.swift new file mode 100644 index 000000000000..9361de7a2117 --- /dev/null +++ b/Modules/Tests/WordPressSharedTests/AppKeychainTests.swift @@ -0,0 +1,142 @@ +import Security +import Testing +@testable import WordPressShared + +extension KeychainStubSuites { + @Suite(.serialized) + struct AppKeychainTests { + private let privateGroup = "team.private" + private let sharedGroup = "team.shared" + + private func makeKeychain(sharedGroup: String? = "team.shared") -> AppKeychain { + AppKeychain(privateGroup: privateGroup, sharedGroup: sharedGroup, keychainUtils: KeychainStub.self) + } + + init() { + KeychainStub.reset() + } + + @Test func readPrefersPrivateGroup() throws { + KeychainStub.seed(group: privateGroup, service: "svc", username: "user", password: "private-pw") + KeychainStub.seed(group: sharedGroup, service: "svc", username: "user", password: "shared-pw") + + let value = try makeKeychain().getPassword(for: "user", serviceName: "svc") + #expect(value == "private-pw") + } + + @Test func readFallsBackToSharedGroup() throws { + KeychainStub.seed(group: sharedGroup, service: "svc", username: "user", password: "shared-pw") + + let value = try makeKeychain().getPassword(for: "user", serviceName: "svc") + #expect(value == "shared-pw") + } + + @Test func readThrowsWhenMissingEverywhere() { + #expect(throws: (any Error).self) { + try makeKeychain().getPassword(for: "user", serviceName: "svc") + } + } + + @Test func readDoesNotFallBackWithoutSharedGroup() { + KeychainStub.seed(group: sharedGroup, service: "svc", username: "user", password: "shared-pw") + + #expect(throws: (any Error).self) { + try makeKeychain(sharedGroup: nil).getPassword(for: "user", serviceName: "svc") + } + } + + @Test func writeTargetsPrivateGroupOnly() throws { + try makeKeychain().setPassword(for: "user", to: "new-pw", serviceName: "svc") + + #expect(KeychainStub.password(group: privateGroup, service: "svc", username: "user") == "new-pw") + #expect(KeychainStub.password(group: sharedGroup, service: "svc", username: "user") == nil) + } + + @Test func deleteRemovesFromBothGroups() throws { + KeychainStub.seed(group: privateGroup, service: "svc", username: "user", password: "pw") + KeychainStub.seed(group: sharedGroup, service: "svc", username: "user", password: "pw") + + try makeKeychain().setPassword(for: "user", to: nil, serviceName: "svc") + + #expect(KeychainStub.password(group: privateGroup, service: "svc", username: "user") == nil) + #expect(KeychainStub.password(group: sharedGroup, service: "svc", username: "user") == nil) + } + + @Test func deleteSucceedsWhenItemOnlyInSharedGroup() throws { + KeychainStub.seed(group: sharedGroup, service: "svc", username: "user", password: "pw") + + try makeKeychain().setPassword(for: "user", to: nil, serviceName: "svc") + + #expect(KeychainStub.password(group: sharedGroup, service: "svc", username: "user") == nil) + } + + @Test func deleteRethrowsRealFailures() { + KeychainStub.seed(group: privateGroup, service: "svc", username: "user", password: "pw") + KeychainStub.deleteError = NSError(domain: sfhfKeychainErrorDomain, code: Int(errSecInteractionNotAllowed)) + + #expect(throws: (any Error).self) { + try makeKeychain().setPassword(for: "user", to: nil, serviceName: "svc") + } + } + + @Test func deleteClearsSharedGroupEvenWhenPrivateDeleteFails() throws { + KeychainStub.seed(group: privateGroup, service: "svc", username: "user", password: "pw") + KeychainStub.seed(group: sharedGroup, service: "svc", username: "user", password: "pw") + KeychainStub.deleteErrors[privateGroup] = NSError( + domain: sfhfKeychainErrorDomain, + code: Int(errSecInteractionNotAllowed) + ) + + #expect(throws: (any Error).self) { + try makeKeychain().setPassword(for: "user", to: nil, serviceName: "svc") + } + // The private delete failed, but the shared delete must still run so + // the fallback read cannot resurrect the credential. + #expect(KeychainStub.password(group: sharedGroup, service: "svc", username: "user") == nil) + } + + @Test func readRepairWritesValueIntoPrivateGroup() throws { + KeychainStub.seed(group: sharedGroup, service: "svc", username: "user", password: "shared-pw") + + let value = try makeKeychain().getPassword(for: "user", serviceName: "svc") + + #expect(value == "shared-pw") + #expect(KeychainStub.password(group: privateGroup, service: "svc", username: "user") == "shared-pw") + } + + @Test func repairedValueIsServedFromPrivateGroup() throws { + KeychainStub.seed(group: sharedGroup, service: "svc", username: "user", password: "shared-pw") + let keychain = makeKeychain() + + _ = try keychain.getPassword(for: "user", serviceName: "svc") + // Change the shared copy; a second read must come from the repaired + // private copy, not the shared group. + KeychainStub.seed(group: sharedGroup, service: "svc", username: "user", password: "changed") + + #expect(try keychain.getPassword(for: "user", serviceName: "svc") == "shared-pw") + } + + @Test func realPrivateReadFailureRethrowsWithoutFallback() { + KeychainStub.readErrors[privateGroup] = NSError( + domain: sfhfKeychainErrorDomain, + code: Int(errSecInteractionNotAllowed) + ) + KeychainStub.seed(group: sharedGroup, service: "svc", username: "user", password: "shared-pw") + + #expect(throws: (any Error).self) { + try makeKeychain().getPassword(for: "user", serviceName: "svc") + } + #expect(KeychainStub.password(group: privateGroup, service: "svc", username: "user") == nil) + } + + @Test func writeThroughFailureIsSwallowedAndValueReturned() throws { + KeychainStub.seed(group: sharedGroup, service: "svc", username: "user", password: "shared-pw") + KeychainStub.storeError = NSError(domain: sfhfKeychainErrorDomain, code: Int(errSecInteractionNotAllowed)) + + let value = try makeKeychain().getPassword(for: "user", serviceName: "svc") + + #expect(value == "shared-pw") + #expect(KeychainStub.password(group: privateGroup, service: "svc", username: "user") == nil) + } + } +} diff --git a/Modules/Tests/WordPressSharedTests/KeychainErrorClassificationTests.swift b/Modules/Tests/WordPressSharedTests/KeychainErrorClassificationTests.swift new file mode 100644 index 000000000000..6b2d57e78df5 --- /dev/null +++ b/Modules/Tests/WordPressSharedTests/KeychainErrorClassificationTests.swift @@ -0,0 +1,22 @@ +import Foundation +import Security +import Testing + +@testable import WordPressShared + +struct KeychainErrorClassificationTests { + @Test func realSFHFFailureIsReal() { + let error = NSError(domain: sfhfKeychainErrorDomain, code: Int(errSecInteractionNotAllowed)) + #expect(isRealKeychainFailure(error)) + } + + @Test func sfhfNotFoundIsBenign() { + let error = NSError(domain: sfhfKeychainErrorDomain, code: Int(errSecItemNotFound)) + #expect(!isRealKeychainFailure(error)) + } + + @Test func synthesizedReadNotFoundIsBenign() { + enum ReadError: Error { case notFound } + #expect(!isRealKeychainFailure(ReadError.notFound)) + } +} diff --git a/Modules/Tests/WordPressSharedTests/KeychainStub.swift b/Modules/Tests/WordPressSharedTests/KeychainStub.swift new file mode 100644 index 000000000000..da865fbfa239 --- /dev/null +++ b/Modules/Tests/WordPressSharedTests/KeychainStub.swift @@ -0,0 +1,84 @@ +import Foundation +import Security +import SFHFKeychainUtils +import Testing +@testable import WordPressShared + +/// Parent suite that serializes every keychain suite against the others. +/// They all share `KeychainStub`'s class-level state, and `.serialized` on +/// an individual suite only orders the tests inside it; without a common +/// serialized ancestor, separate suites still run in parallel and race on +/// the stub. +@Suite(.serialized) enum KeychainStubSuites {} + +/// In-memory SFHFKeychainUtils replacement keyed by access group. +/// Class-level state: any suite using it must nest in `KeychainStubSuites`. +final class KeychainStub: SFHFKeychainUtils { + /// group -> service -> username -> password. + /// nil access groups are stored under `defaultGroup`. + nonisolated(unsafe) static var groups: [String: [String: [String: String]]] = [:] + nonisolated(unsafe) static var storeError: Error? + nonisolated(unsafe) static var deleteError: Error? + nonisolated(unsafe) static var readErrors: [String: Error] = [:] + nonisolated(unsafe) static var deleteErrors: [String: Error] = [:] + + static let defaultGroup = "" + + enum StubError: Error { + case notFound + } + + static func reset() { + groups = [:] + storeError = nil + deleteError = nil + readErrors = [:] + deleteErrors = [:] + } + + static func seed(group: String, service: String, username: String, password: String) { + groups[group, default: [:]][service, default: [:]][username] = password + } + + static func password(group: String, service: String, username: String) -> String? { + groups[group]?[service]?[username] + } + + override class func getPasswordForUsername( + _ username: String!, + andServiceName serviceName: String!, + accessGroup: String! + ) throws -> String { + let group = accessGroup ?? defaultGroup + if let error = readErrors[group] { throw error } + guard let value = groups[group]?[serviceName]?[username] else { + throw StubError.notFound + } + return value + } + + override class func storeUsername( + _ username: String!, + andPassword password: String!, + forServiceName serviceName: String!, + accessGroup: String!, + updateExisting: Bool + ) throws { + if let storeError { throw storeError } + groups[accessGroup ?? defaultGroup, default: [:]][serviceName, default: [:]][username] = password + } + + override class func deleteItem( + forUsername username: String!, + andServiceName serviceName: String!, + accessGroup: String! + ) throws { + let group = accessGroup ?? defaultGroup + if let error = deleteErrors[group] { throw error } + if let deleteError { throw deleteError } + guard groups[group]?[serviceName]?[username] != nil else { + throw NSError(domain: sfhfKeychainErrorDomain, code: Int(errSecItemNotFound)) + } + groups[group]?[serviceName]?[username] = nil + } +} diff --git a/Modules/Tests/WordPressSharedTests/SharedKeychainTests.swift b/Modules/Tests/WordPressSharedTests/SharedKeychainTests.swift new file mode 100644 index 000000000000..185a1f4ed688 --- /dev/null +++ b/Modules/Tests/WordPressSharedTests/SharedKeychainTests.swift @@ -0,0 +1,47 @@ +import Testing +@testable import WordPressShared + +extension KeychainStubSuites { + @Suite(.serialized) + struct SharedKeychainTests { + private let sharedGroup = "team.shared" + + init() { + KeychainStub.reset() + } + + @Test func initFailsWithoutGroup() { + #expect(SharedKeychain(group: nil, keychainUtils: KeychainStub.self) == nil) + } + + @Test func readTargetsSharedGroup() throws { + KeychainStub.seed(group: sharedGroup, service: "svc", username: "user", password: "pw") + let keychain = try #require(SharedKeychain(group: sharedGroup, keychainUtils: KeychainStub.self)) + + let value = try keychain.getPassword(for: "user", serviceName: "svc") + #expect(value == "pw") + } + + @Test func writeTargetsSharedGroup() throws { + let keychain = try #require(SharedKeychain(group: sharedGroup, keychainUtils: KeychainStub.self)) + + try keychain.setPassword(for: "user", to: "pw", serviceName: "svc") + #expect(KeychainStub.password(group: sharedGroup, service: "svc", username: "user") == "pw") + } + + @Test func nilValueDeletesFromSharedGroup() throws { + KeychainStub.seed(group: sharedGroup, service: "svc", username: "user", password: "pw") + let keychain = try #require(SharedKeychain(group: sharedGroup, keychainUtils: KeychainStub.self)) + + try keychain.setPassword(for: "user", to: nil, serviceName: "svc") + #expect(KeychainStub.password(group: sharedGroup, service: "svc", username: "user") == nil) + } + + @Test func nilValueSucceedsWhenItemMissing() throws { + let keychain = try #require(SharedKeychain(group: sharedGroup, keychainUtils: KeychainStub.self)) + + try keychain.setPassword(for: "user", to: nil, serviceName: "svc") + #expect(KeychainStub.password(group: sharedGroup, service: "svc", username: "user") == nil) + } + } +} From 3d8ad9ea10095a851ee44825c133591f10275c6a Mon Sep 17 00:00:00 2001 From: Tony Li Date: Sun, 14 Jun 2026 18:43:05 +1200 Subject: [PATCH 04/13] Route the app's keychain access through AppKeychain --- .../WordPressData/Swift/Blog+SelfHosted.swift | 14 ++-- .../WordPressData/Swift/Blog+Swift.swift | 2 +- .../WordPressData/Swift/WPAccount.swift | 4 +- Tests/KeystoneTests/Helpers/BlogBuilder.swift | 31 ++++++--- .../ApplicationPasswordRepository.swift | 2 +- .../Classes/Services/CredentialsService.swift | 4 +- .../Services/NotificationSupportService.swift | 68 ++++++++----------- .../Editor/EditorConfiguration+Blog.swift | 2 +- 8 files changed, 64 insertions(+), 63 deletions(-) diff --git a/Modules/Sources/WordPressData/Swift/Blog+SelfHosted.swift b/Modules/Sources/WordPressData/Swift/Blog+SelfHosted.swift index a9f4b8e5fe0b..260a1ee3e47b 100644 --- a/Modules/Sources/WordPressData/Swift/Blog+SelfHosted.swift +++ b/Modules/Sources/WordPressData/Swift/Blog+SelfHosted.swift @@ -23,7 +23,7 @@ public extension Blog { xmlrpcEndpointURL: URL, blogID: TaggedManagedObjectID?, in contextManager: ContextManager, - using keychainImplementation: KeychainAccessible = KeychainUtils() + using keychainImplementation: KeychainAccessible = AppKeychain() ) async throws -> TaggedManagedObjectID { try await contextManager.performAndSave { context in let blog = @@ -69,13 +69,13 @@ public extension Blog { /// Retrieve Application Tokens /// - func getApplicationToken(using keychainImplementation: KeychainAccessible = KeychainUtils()) throws -> String { + func getApplicationToken(using keychainImplementation: KeychainAccessible = AppKeychain()) throws -> String { try keychainImplementation.getPassword(for: self.getUsername(), serviceName: self.getUrlString()) } /// Delete Application Token /// - func deleteApplicationToken(using keychainImplementation: KeychainAccessible = KeychainUtils()) throws { + func deleteApplicationToken(using keychainImplementation: KeychainAccessible = AppKeychain()) throws { try? keychainImplementation.setPassword(for: self.getUsername(), to: nil, serviceName: self.getUrlString()) } @@ -89,7 +89,7 @@ public extension Blog { /// func setApplicationToken( _ newValue: String, - using keychainImplementation: KeychainAccessible = KeychainUtils() + using keychainImplementation: KeychainAccessible = AppKeychain() ) throws { try keychainImplementation.setPassword(for: self.getUsername(), to: newValue, serviceName: self.getUrlString()) } @@ -104,7 +104,7 @@ public extension Blog { } /// A null-safe replacement for `Blog.password(get)` - func getPassword(using keychainImplementation: KeychainAccessible = KeychainUtils()) throws -> String { + func getPassword(using keychainImplementation: KeychainAccessible = AppKeychain()) throws -> String { try keychainImplementation.getPassword( for: self.getUsername(), serviceName: self.getXMLRPCEndpoint().absoluteString @@ -112,7 +112,7 @@ public extension Blog { } /// A null-safe replacement for `Blog.password(set)` - func setPassword(to newValue: String, using keychainImplementation: KeychainAccessible = KeychainUtils()) throws { + func setPassword(to newValue: String, using keychainImplementation: KeychainAccessible = AppKeychain()) throws { try keychainImplementation.setPassword( for: self.getUsername(), to: newValue, @@ -272,7 +272,7 @@ extension WordPressSite { /// /// For self-hosted sites, application password credentials are required. /// Sites without them cannot be represented as a `WordPressSite`. - public init(blog: Blog, keychain: KeychainAccessible = KeychainUtils()) throws { + public init(blog: Blog, keychain: KeychainAccessible = AppKeychain()) throws { let siteURL = try blog.getUrl() self.blogId = TaggedManagedObjectID(blog) self.siteURL = siteURL diff --git a/Modules/Sources/WordPressData/Swift/Blog+Swift.swift b/Modules/Sources/WordPressData/Swift/Blog+Swift.swift index 9a4d00cefcbd..fb3145193f64 100644 --- a/Modules/Sources/WordPressData/Swift/Blog+Swift.swift +++ b/Modules/Sources/WordPressData/Swift/Blog+Swift.swift @@ -56,7 +56,7 @@ extension Blog { /// Injectable keychain for testability. var keychain: any KeychainAccessible { get { - objc_getAssociatedObject(self, &blogKeychainKey) as? (any KeychainAccessible) ?? KeychainUtils() + objc_getAssociatedObject(self, &blogKeychainKey) as? (any KeychainAccessible) ?? AppKeychain() } set { objc_setAssociatedObject(self, &blogKeychainKey, newValue, .OBJC_ASSOCIATION_RETAIN_NONATOMIC) diff --git a/Modules/Sources/WordPressData/Swift/WPAccount.swift b/Modules/Sources/WordPressData/Swift/WPAccount.swift index 9526b6cf0850..15fcd266137f 100644 --- a/Modules/Sources/WordPressData/Swift/WPAccount.swift +++ b/Modules/Sources/WordPressData/Swift/WPAccount.swift @@ -30,7 +30,7 @@ public class WPAccount: NSManagedObject { private var cachedToken: String? - lazy var keychain: any KeychainAccessible = KeychainUtils() + lazy var keychain: any KeychainAccessible = AppKeychain() lazy var keychainServiceName: String = BuildSettings.current.authKeychainServiceName lazy var keychainMigration: any AuthKeyMigrationProtocol = AuthKeyMigration() @@ -151,7 +151,7 @@ public class WPAccount: NSManagedObject { forUsername: username, serviceName: BuildSettings.current.authKeychainServiceName, migration: AuthKeyMigration(), - keychain: KeychainUtils() + keychain: AppKeychain() ) } diff --git a/Tests/KeystoneTests/Helpers/BlogBuilder.swift b/Tests/KeystoneTests/Helpers/BlogBuilder.swift index 4ac68c7e3391..8e6be36c9940 100644 --- a/Tests/KeystoneTests/Helpers/BlogBuilder.swift +++ b/Tests/KeystoneTests/Helpers/BlogBuilder.swift @@ -29,7 +29,7 @@ final class BlogBuilder { } func with(atomic: Bool) -> Self { - return set(blogOption: "is_wpcom_atomic", value: atomic ? 1 : 0) + set(blogOption: "is_wpcom_atomic", value: atomic ? 1 : 0) } func with(isHostedAtWPCom: Bool) -> Self { @@ -38,7 +38,7 @@ final class BlogBuilder { } func with(supportsDomains: Bool) -> Self { - return with(isHostedAtWPCom: supportsDomains) + with(isHostedAtWPCom: supportsDomains) .with(isAdmin: supportsDomains) } @@ -61,7 +61,7 @@ final class BlogBuilder { } func with(wordPressVersion: String) -> Self { - return set(blogOption: "software_version", value: wordPressVersion) + set(blogOption: "software_version", value: wordPressVersion) } func with(username: String) -> Self { @@ -83,7 +83,9 @@ final class BlogBuilder { func with(siteName: String) -> Self { if blog.settings == nil { - blog.settings = NSEntityDescription.insertNewObject(forEntityName: BlogSettings.entityName(), into: context) as! BlogSettings + blog.settings = + NSEntityDescription.insertNewObject(forEntityName: BlogSettings.entityName(), into: context) + as! BlogSettings } blog.settings?.name = siteName return self @@ -102,7 +104,8 @@ final class BlogBuilder { func withAnAccount(username: String = "test_user", authToken: String = "authtoken") -> Self { // Add Account - let account = NSEntityDescription.insertNewObject(forEntityName: WPAccount.entityName(), into: context) as! WPAccount + let account = + NSEntityDescription.insertNewObject(forEntityName: WPAccount.entityName(), into: context) as! WPAccount account.displayName = "displayName" account.username = username account.authToken = authToken @@ -153,7 +156,9 @@ final class BlogBuilder { func with(domainCount: Int, of type: DomainType, domainName: String = "") -> Self { var domains: [ManagedDomain] = [] for _ in 0.. Self { + func withMappedDomain( + originalUrl: String = "http://domain1.com", + mappedDomainUrl: String = "http://domain2.com" + ) -> Self { set(blogOption: "unmapped_url", value: originalUrl) set(blogOption: "home_url", value: mappedDomainUrl) @@ -189,7 +197,7 @@ final class BlogBuilder { } func with(isWPForTeamsSite: Bool) -> Self { - return set(blogOption: "is_wpforteams_site", value: isWPForTeamsSite) + set(blogOption: "is_wpforteams_site", value: isWPForTeamsSite) } func with(connections: Set) -> Self { @@ -206,7 +214,7 @@ final class BlogBuilder { @discardableResult func build() -> Blog { - return blog + blog } @discardableResult @@ -226,7 +234,7 @@ final class BlogBuilder { return self } - func withApplicationPassword(_ password: String, using keychain: KeychainAccessible = KeychainUtils()) -> Self { + func withApplicationPassword(_ password: String, using keychain: KeychainAccessible = AppKeychain()) -> Self { do { try blog.setApplicationToken(password, using: keychain) } catch { @@ -248,7 +256,8 @@ extension Blog { return } - let account = NSEntityDescription.insertNewObject(forEntityName: WPAccount.entityName(), into: context) as! WPAccount + let account = + NSEntityDescription.insertNewObject(forEntityName: WPAccount.entityName(), into: context) as! WPAccount account.username = "foo" account.addBlogsObject(self) } diff --git a/WordPress/Classes/Services/ApplicationPasswordRepository.swift b/WordPress/Classes/Services/ApplicationPasswordRepository.swift index 2a7baa715d9d..5dbd224efb3d 100644 --- a/WordPress/Classes/Services/ApplicationPasswordRepository.swift +++ b/WordPress/Classes/Services/ApplicationPasswordRepository.swift @@ -32,7 +32,7 @@ import WordPressCore actor ApplicationPasswordRepository { static let shared: ApplicationPasswordRepository = .init( coreDataStack: ContextManager.shared, - keychain: KeychainUtils() + keychain: AppKeychain() ) private let coreDataStack: CoreDataStackSwift diff --git a/WordPress/Classes/Services/CredentialsService.swift b/WordPress/Classes/Services/CredentialsService.swift index 63d1e3488d5a..688c4550a38e 100644 --- a/WordPress/Classes/Services/CredentialsService.swift +++ b/WordPress/Classes/Services/CredentialsService.swift @@ -1,5 +1,5 @@ import BuildSettingsKit -import SFHFKeychainUtils +import WordPressShared protocol CredentialsProvider { func getPassword(username: String, service: String) -> String? @@ -7,7 +7,7 @@ protocol CredentialsProvider { struct KeychainCredentialsProvider: CredentialsProvider { func getPassword(username: String, service: String) -> String? { - try? SFHFKeychainUtils.getPasswordForUsername(username, andServiceName: service) + try? AppKeychain().getPassword(for: username, serviceName: service) } } diff --git a/WordPress/Classes/Services/NotificationSupportService.swift b/WordPress/Classes/Services/NotificationSupportService.swift index 2904f8f0ed7b..321245f81ccb 100644 --- a/WordPress/Classes/Services/NotificationSupportService.swift +++ b/WordPress/Classes/Services/NotificationSupportService.swift @@ -1,26 +1,24 @@ import Foundation import BuildSettingsKit -import SFHFKeychainUtils import NotificationServiceExtensionCore +import WordPressShared final class NotificationSupportService { - private let appKeychainAccessGroup: String private let configuration: NotificationServiceExtensionConfiguration + private let keychain: any KeychainAccessible convenience init() { - let settings = BuildSettings.current self.init( - appKeychainAccessGroup: settings.appKeychainAccessGroup, - configuration: settings.notificationServiceExtensionConfiguration + configuration: BuildSettings.current.notificationServiceExtensionConfiguration ) } init( - appKeychainAccessGroup: String, - configuration: NotificationServiceExtensionConfiguration + configuration: NotificationServiceExtensionConfiguration, + keychain: any KeychainAccessible = AppKeychain() ) { - self.appKeychainAccessGroup = appKeychainAccessGroup self.configuration = configuration + self.keychain = keychain } /// Sets the OAuth Token that should be used by the Notification Service Extension to access WPCOM. @@ -29,12 +27,10 @@ final class NotificationSupportService { /// func storeToken(_ authToken: String) { do { - try SFHFKeychainUtils.storeUsername( - configuration.keychainTokenKey, - andPassword: authToken, - forServiceName: configuration.keychainServiceName, - accessGroup: appKeychainAccessGroup, - updateExisting: true + try keychain.setPassword( + for: configuration.keychainTokenKey, + to: authToken, + serviceName: configuration.keychainServiceName ) } catch { DDLogDebug("Error while saving Notification Service Extension OAuth token: \(error)") @@ -47,12 +43,10 @@ final class NotificationSupportService { /// func storeUsername(_ username: String) { do { - try SFHFKeychainUtils.storeUsername( - configuration.keychainUsernameKey, - andPassword: username, - forServiceName: configuration.keychainServiceName, - accessGroup: appKeychainAccessGroup, - updateExisting: true + try keychain.setPassword( + for: configuration.keychainUsernameKey, + to: username, + serviceName: configuration.keychainServiceName ) } catch { DDLogDebug("Error while saving Notification Service Extension username: \(error)") @@ -65,12 +59,10 @@ final class NotificationSupportService { /// func storeUserID(_ userID: String) { do { - try SFHFKeychainUtils.storeUsername( - configuration.keychainUserIDKey, - andPassword: userID, - forServiceName: configuration.keychainServiceName, - accessGroup: appKeychainAccessGroup, - updateExisting: true + try keychain.setPassword( + for: configuration.keychainUserIDKey, + to: userID, + serviceName: configuration.keychainServiceName ) } catch { DDLogDebug("Error while saving Notification Service Extension userID: \(error)") @@ -81,10 +73,10 @@ final class NotificationSupportService { /// func deleteServiceExtensionToken() { do { - try SFHFKeychainUtils.deleteItem( - forUsername: configuration.keychainTokenKey, - andServiceName: configuration.keychainServiceName, - accessGroup: appKeychainAccessGroup + try keychain.setPassword( + for: configuration.keychainTokenKey, + to: nil, + serviceName: configuration.keychainServiceName ) } catch { DDLogDebug("Error while removing Notification Service Extension OAuth token: \(error)") @@ -95,10 +87,10 @@ final class NotificationSupportService { /// func deleteServiceExtensionUsername() { do { - try SFHFKeychainUtils.deleteItem( - forUsername: configuration.keychainUsernameKey, - andServiceName: configuration.keychainServiceName, - accessGroup: appKeychainAccessGroup + try keychain.setPassword( + for: configuration.keychainUsernameKey, + to: nil, + serviceName: configuration.keychainServiceName ) } catch { DDLogDebug("Error while removing Notification Service Extension username: \(error)") @@ -109,10 +101,10 @@ final class NotificationSupportService { /// func deleteServiceExtensionUserID() { do { - try SFHFKeychainUtils.deleteItem( - forUsername: configuration.keychainUserIDKey, - andServiceName: configuration.keychainServiceName, - accessGroup: appKeychainAccessGroup + try keychain.setPassword( + for: configuration.keychainUserIDKey, + to: nil, + serviceName: configuration.keychainServiceName ) } catch { DDLogDebug("Error while removing Notification Service Extension userID: \(error)") diff --git a/WordPress/Classes/Utility/Editor/EditorConfiguration+Blog.swift b/WordPress/Classes/Utility/Editor/EditorConfiguration+Blog.swift index d4b8f6c85a54..dd53bcaa557d 100644 --- a/WordPress/Classes/Utility/Editor/EditorConfiguration+Blog.swift +++ b/WordPress/Classes/Utility/Editor/EditorConfiguration+Blog.swift @@ -5,7 +5,7 @@ import WordPressShared import Support extension EditorConfiguration { - init(blog: Blog, postType: PostTypeDetails, keychain: KeychainAccessible = KeychainUtils()) { + init(blog: Blog, postType: PostTypeDetails, keychain: KeychainAccessible = AppKeychain()) { let selfHostedApiUrl = blog.restApiRootURL ?? blog.url(withPath: "wp-json/") let applicationPassword = try? blog.getApplicationToken(using: keychain) let shouldUseWPComRestApi = applicationPassword == nil && blog.isAccessibleThroughWPCom From ade30c7849d2c11b38a07fe0d7410707b9151ea0 Mon Sep 17 00:00:00 2001 From: Tony Li Date: Sun, 14 Jun 2026 18:43:05 +1200 Subject: [PATCH 05/13] Route the share extension through AppKeychain --- .../ShareExtensionService.swift | 61 ++++++++----------- 1 file changed, 27 insertions(+), 34 deletions(-) diff --git a/Modules/Sources/ShareExtensionCore/ShareExtensionService.swift b/Modules/Sources/ShareExtensionCore/ShareExtensionService.swift index 2e0b2ec3c7f1..31bab8b3ad20 100644 --- a/Modules/Sources/ShareExtensionCore/ShareExtensionService.swift +++ b/Modules/Sources/ShareExtensionCore/ShareExtensionService.swift @@ -1,29 +1,28 @@ import Foundation import BuildSettingsKit -import SFHFKeychainUtils +import WordPressShared public final class ShareExtensionService { private let appGroupName: String - private let appKeychainAccessGroup: String private let configuration: ShareExtensionConfiguration + private let keychain: any KeychainAccessible public convenience init() { let settings = BuildSettings.current self.init( appGroupName: settings.appGroupName, - appKeychainAccessGroup: settings.appKeychainAccessGroup, configuration: settings.shareExtensionConfiguration ) } public init( appGroupName: String, - appKeychainAccessGroup: String, - configuration: ShareExtensionConfiguration + configuration: ShareExtensionConfiguration, + keychain: any KeychainAccessible = AppKeychain() ) { self.appGroupName = appGroupName - self.appKeychainAccessGroup = appKeychainAccessGroup self.configuration = configuration + self.keychain = keychain } /// Sets the OAuth Token that should be used by the Share Extension to hit the Dotcom Backend. @@ -32,12 +31,10 @@ public final class ShareExtensionService { /// public func storeToken(_ oauth2Token: String) { do { - try SFHFKeychainUtils.storeUsername( - configuration.keychainTokenKey, - andPassword: oauth2Token, - forServiceName: configuration.keychainServiceName, - accessGroup: appKeychainAccessGroup, - updateExisting: true + try keychain.setPassword( + for: configuration.keychainTokenKey, + to: oauth2Token, + serviceName: configuration.keychainServiceName ) } catch { print("Error while saving Share Extension OAuth bearer token: \(error)") @@ -50,12 +47,10 @@ public final class ShareExtensionService { /// public func storeUsername(_ username: String) { do { - try SFHFKeychainUtils.storeUsername( - configuration.keychainUsernameKey, - andPassword: username, - forServiceName: configuration.keychainServiceName, - accessGroup: appKeychainAccessGroup, - updateExisting: true + try keychain.setPassword( + for: configuration.keychainUsernameKey, + to: username, + serviceName: configuration.keychainServiceName ) } catch { print("Error while saving Share Extension OAuth bearer token: \(error)") @@ -121,20 +116,20 @@ public final class ShareExtensionService { /// public func removeShareExtensionConfiguration() { do { - try SFHFKeychainUtils.deleteItem( - forUsername: configuration.keychainTokenKey, - andServiceName: configuration.keychainServiceName, - accessGroup: appKeychainAccessGroup + try keychain.setPassword( + for: configuration.keychainTokenKey, + to: nil, + serviceName: configuration.keychainServiceName ) } catch { print("Error while removing Share Extension OAuth2 bearer token: \(error)") } do { - try SFHFKeychainUtils.deleteItem( - forUsername: configuration.keychainUsernameKey, - andServiceName: configuration.keychainServiceName, - accessGroup: appKeychainAccessGroup + try keychain.setPassword( + for: configuration.keychainUsernameKey, + to: nil, + serviceName: configuration.keychainServiceName ) } catch { print("Error while removing Share Extension Username: \(error)") @@ -154,10 +149,9 @@ public final class ShareExtensionService { /// public func retrieveShareExtensionToken() -> String? { guard - let oauth2Token = try? SFHFKeychainUtils.getPasswordForUsername( - configuration.keychainTokenKey, - andServiceName: configuration.keychainServiceName, - accessGroup: appKeychainAccessGroup + let oauth2Token = try? keychain.getPassword( + for: configuration.keychainTokenKey, + serviceName: configuration.keychainServiceName ) else { return nil @@ -170,10 +164,9 @@ public final class ShareExtensionService { /// public func retrieveShareExtensionUsername() -> String? { guard - let oauth2Token = try? SFHFKeychainUtils.getPasswordForUsername( - configuration.keychainUsernameKey, - andServiceName: configuration.keychainServiceName, - accessGroup: appKeychainAccessGroup + let oauth2Token = try? keychain.getPassword( + for: configuration.keychainUsernameKey, + serviceName: configuration.keychainServiceName ) else { return nil From 4b5a040fcf686975aaa59df5b5a7a339e3cd2122 Mon Sep 17 00:00:00 2001 From: Tony Li Date: Sun, 14 Jun 2026 18:43:05 +1200 Subject: [PATCH 06/13] Route the notification service extension through AppKeychain --- .../Sources/NotificationService.swift | 42 ++++++++++--------- 1 file changed, 23 insertions(+), 19 deletions(-) diff --git a/WordPress/WordPressNotificationServiceExtension/Sources/NotificationService.swift b/WordPress/WordPressNotificationServiceExtension/Sources/NotificationService.swift index 7860e0878a40..db527863b7e6 100644 --- a/WordPress/WordPressNotificationServiceExtension/Sources/NotificationService.swift +++ b/WordPress/WordPressNotificationServiceExtension/Sources/NotificationService.swift @@ -2,7 +2,6 @@ import UIKit import BuildSettingsKit import FormattableContentKit import NotificationServiceExtensionCore -import SFHFKeychainUtils import TracksMini import UserNotifications import WordPressKit @@ -28,14 +27,16 @@ class NotificationService: UNNotificationServiceExtension { private var notificationService: NotificationSyncServiceRemote? private let configuration: NotificationServiceExtensionConfiguration - - private let appKeychainAccessGroup: String + /// Deliberately lazy: creating `AppKeychain` reads `BuildSettings`, + /// which fatally asserts unless `BuildSettings.configure(secrets:)` + /// has run first. A stored default would be evaluated before this + /// type's init body performs that configuration. + private lazy var keychain: any KeychainAccessible = AppKeychain() override init() { BuildSettings.configure(secrets: ApiCredentials.toSecrets()) tracks = Tracks() configuration = BuildSettings.current.notificationServiceExtensionConfiguration - appKeychainAccessGroup = BuildSettings.current.appKeychainAccessGroup super.init() } @@ -321,11 +322,12 @@ private extension NotificationService { /// func readExtensionToken() -> String? { guard - let oauthToken = try? SFHFKeychainUtils.getPasswordForUsername( - configuration.keychainTokenKey, - andServiceName: configuration.keychainServiceName, - accessGroup: appKeychainAccessGroup - ) + let oauthToken = + try? keychain + .getPassword( + for: configuration.keychainTokenKey, + serviceName: configuration.keychainServiceName + ) else { debugPrint("Unable to retrieve Notification Service Extension OAuth token") return nil @@ -340,11 +342,12 @@ private extension NotificationService { /// func readExtensionUsername() -> String? { guard - let username = try? SFHFKeychainUtils.getPasswordForUsername( - configuration.keychainUsernameKey, - andServiceName: configuration.keychainServiceName, - accessGroup: appKeychainAccessGroup - ) + let username = + try? keychain + .getPassword( + for: configuration.keychainUsernameKey, + serviceName: configuration.keychainServiceName + ) else { debugPrint("Unable to retrieve Notification Service Extension username") return nil @@ -359,11 +362,12 @@ private extension NotificationService { /// func readExtensionUserID() -> String? { guard - let userID = try? SFHFKeychainUtils.getPasswordForUsername( - configuration.keychainUserIDKey, - andServiceName: configuration.keychainServiceName, - accessGroup: appKeychainAccessGroup - ) + let userID = + try? keychain + .getPassword( + for: configuration.keychainUserIDKey, + serviceName: configuration.keychainServiceName + ) else { debugPrint("Unable to retrieve Notification Service Extension userID") return nil From 3b30617d9523b96d30b061b631ba5fbdf53cf5ca Mon Sep 17 00:00:00 2001 From: Tony Li Date: Sun, 14 Jun 2026 18:43:05 +1200 Subject: [PATCH 07/13] Route stats widget credentials through AppKeychain --- .../Services/StatsWidgetsService.swift | 11 +++++----- .../Tests/Stores/StatsWidgetsStoreTests.swift | 19 ++++++++++------- .../Classes/Stores/StatsWidgetsStore.swift | 21 +++++++++---------- 3 files changed, 26 insertions(+), 25 deletions(-) diff --git a/Sources/JetpackStatsWidgets/Sources/Services/StatsWidgetsService.swift b/Sources/JetpackStatsWidgets/Sources/Services/StatsWidgetsService.swift index 4d34b423765c..20a1ad83265b 100644 --- a/Sources/JetpackStatsWidgets/Sources/Services/StatsWidgetsService.swift +++ b/Sources/JetpackStatsWidgets/Sources/Services/StatsWidgetsService.swift @@ -1,6 +1,5 @@ import Foundation import CocoaLumberjackSwift -import SFHFKeychainUtils import BuildSettingsKit import WordPressKit import JetpackStatsWidgetsCore @@ -254,11 +253,11 @@ private extension StatsWidgetsService { } private func createStatsService(for widgetData: HomeWidgetData) throws -> StatsServiceRemoteV2 { - let token = try SFHFKeychainUtils.getPasswordForUsername( - WidgetStatsConfiguration.keychainTokenKey, - andServiceName: WidgetStatsConfiguration.keychainServiceName, - accessGroup: BuildSettings.current.appKeychainAccessGroup - ) + let token = try AppKeychain() + .getPassword( + for: WidgetStatsConfiguration.keychainTokenKey, + serviceName: WidgetStatsConfiguration.keychainServiceName + ) let wpApi = WordPressComRestApi(oAuthToken: token) return StatsServiceRemoteV2( wordPressComRestApi: wpApi, diff --git a/Tests/KeystoneTests/Tests/Stores/StatsWidgetsStoreTests.swift b/Tests/KeystoneTests/Tests/Stores/StatsWidgetsStoreTests.swift index 97b29cae5ac1..dcafc9fbb1d5 100644 --- a/Tests/KeystoneTests/Tests/Stores/StatsWidgetsStoreTests.swift +++ b/Tests/KeystoneTests/Tests/Stores/StatsWidgetsStoreTests.swift @@ -7,7 +7,6 @@ import JetpackStatsWidgetsCore class StatsWidgetsStoreTests: CoreDataTestCase { private var sut: StatsWidgetsStore! private var appGroupName: String! - private let appKeychainAccessGroup = "xctest_appKeychainAccessGroup" override func setUp() { super.setUp() @@ -17,8 +16,7 @@ class StatsWidgetsStoreTests: CoreDataTestCase { deleteHomeWidgetData() sut = StatsWidgetsStore( coreDataStack: contextManager, - appGroupName: appGroupName, - appKeychainAccessGroup: appKeychainAccessGroup + appGroupName: appGroupName ) } @@ -36,7 +34,10 @@ class StatsWidgetsStoreTests: CoreDataTestCase { .build() XCTAssertFalse(statsWidgetsHaveData()) - NotificationCenter.default.post(name: NSNotification.Name(rawValue: WordPressAuthenticationManager.WPSigninDidFinishNotification), object: nil) + NotificationCenter.default.post( + name: NSNotification.Name(rawValue: WordPressAuthenticationManager.WPSigninDidFinishNotification), + object: nil + ) XCTAssertTrue(statsWidgetsHaveData()) } @@ -46,7 +47,10 @@ class StatsWidgetsStoreTests: CoreDataTestCase { .withAnAccount() .isHostedAtWPcom() .build() - NotificationCenter.default.post(name: NSNotification.Name(rawValue: WordPressAuthenticationManager.WPSigninDidFinishNotification), object: nil) + NotificationCenter.default.post( + name: NSNotification.Name(rawValue: WordPressAuthenticationManager.WPSigninDidFinishNotification), + object: nil + ) NotificationCenter.default.post(name: .wpAccountDefaultWordPressComAccountChanged, object: nil) @@ -58,9 +62,8 @@ class StatsWidgetsStoreTests: CoreDataTestCase { private extension StatsWidgetsStoreTests { private func statsWidgetsHaveData() -> Bool { - hasData(for: HomeWidgetTodayData.self) && - hasData(for: HomeWidgetThisWeekData.self) && - hasData(for: HomeWidgetAllTimeData.self) + hasData(for: HomeWidgetTodayData.self) && hasData(for: HomeWidgetThisWeekData.self) + && hasData(for: HomeWidgetAllTimeData.self) } private func deleteHomeWidgetData() { diff --git a/WordPress/Classes/Stores/StatsWidgetsStore.swift b/WordPress/Classes/Stores/StatsWidgetsStore.swift index cc6682512d69..de40219e9904 100644 --- a/WordPress/Classes/Stores/StatsWidgetsStore.swift +++ b/WordPress/Classes/Stores/StatsWidgetsStore.swift @@ -1,24 +1,24 @@ import Foundation import JetpackStatsWidgetsCore import BuildSettingsKit -import SFHFKeychainUtils import WidgetKit import WordPressData import WordPressKit +import WordPressShared class StatsWidgetsStore { private let coreDataStack: CoreDataStack private let appGroupName: String - private let appKeychainAccessGroup: String + private let keychain: any KeychainAccessible init( coreDataStack: CoreDataStack = ContextManager.shared, appGroupName: String = BuildSettings.current.appGroupName, - appKeychainAccessGroup: String = BuildSettings.current.appKeychainAccessGroup + keychain: any KeychainAccessible = AppKeychain() ) { self.coreDataStack = coreDataStack self.appGroupName = appGroupName - self.appKeychainAccessGroup = appKeychainAccessGroup + self.keychain = keychain observeAccountChangesForWidgets() observeAccountSignInForWidgets() @@ -452,13 +452,12 @@ private extension StatsWidgetsStore { guard let token = AccountHelper.authToken else { return } do { - try SFHFKeychainUtils.storeUsername( - WidgetStatsConfiguration.keychainTokenKey, - andPassword: token, - forServiceName: WidgetStatsConfiguration.keychainServiceName, - accessGroup: appKeychainAccessGroup, - updateExisting: true - ) + try keychain + .setPassword( + for: WidgetStatsConfiguration.keychainTokenKey, + to: token, + serviceName: WidgetStatsConfiguration.keychainServiceName + ) } catch { DDLogDebug("Error while saving Widgets OAuth token: \(error)") } From e1b1c23f05db1fe9318c8b5fb56620fd578cb7fd Mon Sep 17 00:00:00 2001 From: Tony Li Date: Sun, 14 Jun 2026 18:43:06 +1200 Subject: [PATCH 08/13] Move the WordPress-to-Jetpack auth handoff onto the shared keychain --- .../Swift/SharedDataIssueSolver.swift | 42 +++--- .../Tests/Jetpack/DataMigratorTests.swift | 136 +++++++++++++++++- .../Jetpack/SharedDataIssueSolverTests.swift | 9 +- .../Jetpack/Utility/DataMigrationError.swift | 21 +-- .../Jetpack/Utility/DataMigrator.swift | 73 +++++++++- 5 files changed, 235 insertions(+), 46 deletions(-) diff --git a/Modules/Sources/WordPressData/Swift/SharedDataIssueSolver.swift b/Modules/Sources/WordPressData/Swift/SharedDataIssueSolver.swift index 6353f634dbf7..1c16687c15b9 100644 --- a/Modules/Sources/WordPressData/Swift/SharedDataIssueSolver.swift +++ b/Modules/Sources/WordPressData/Swift/SharedDataIssueSolver.swift @@ -4,20 +4,23 @@ import BuildSettingsKit public final class SharedDataIssueSolver { private let contextManager: CoreDataStack - private let keychainUtils: KeychainAccessible + private let appKeychain: KeychainAccessible + private let sharedKeychain: KeychainAccessible? private let sharedDefaults: UserPersistentRepository? private let localFileStore: LocalFileStore private let appGroupName: String public init( contextManager: CoreDataStack = ContextManager.shared, - keychainUtils: KeychainAccessible = KeychainUtils(), + appKeychain: KeychainAccessible = AppKeychain(), + sharedKeychain: KeychainAccessible? = SharedKeychain(), sharedDefaults: UserPersistentRepository? = UserDefaults(suiteName: BuildSettings.current.appGroupName), localFileStore: LocalFileStore = FileManager.default, appGroupName: String = BuildSettings.current.appGroupName ) { self.contextManager = contextManager - self.keychainUtils = keychainUtils + self.appKeychain = appKeychain + self.sharedKeychain = sharedKeychain self.sharedDefaults = sharedDefaults self.localFileStore = localFileStore self.appGroupName = appGroupName @@ -35,10 +38,15 @@ public final class SharedDataIssueSolver { /// both apps are logged in with the same account. /// public func migrateAuthKey(for username: String) { + // Explicitly the shared group: this is the one deliberate cross-app + // keychain read in the codebase. The WordPress app publishes the + // token there at export time (and pre-change versions wrote it + // there by default). guard BuildSettings.current.brand == .jetpack, - let token = try? keychainUtils.getPassword( + let sharedKeychain, + let token = try? sharedKeychain.getPassword( for: username, - serviceName: WPAccountConstants.authToken.rawValue + serviceName: AuthTokenServiceNames.wordPress ) else { return @@ -46,18 +54,18 @@ public final class SharedDataIssueSolver { // If the token has already been migrated, no need to resolve the issue again. // There might also be a possibility that the user logged in to JP by themselves. In which, we won't need to migrate. - if let _ = try? keychainUtils.getPassword( + if let _ = try? appKeychain.getPassword( for: username, - serviceName: WPAccountConstants.authToken.valueForJetpack + serviceName: AuthTokenServiceNames.jetpack ) { return } // if authToken for the account username exists, move it to the authToken location for JP. - try? keychainUtils.setPassword( + try? appKeychain.setPassword( for: username, to: token, - serviceName: WPAccountConstants.authToken.valueForJetpack + serviceName: AuthTokenServiceNames.jetpack ) } @@ -106,22 +114,6 @@ fileprivate protocol MigratableConstant { var valueForJetpack: String { get } } -// MARK: - Account Auth Token Helpers - -private extension SharedDataIssueSolver { - - enum WPAccountConstants: String, MigratableConstant { - case authToken = "public-api.wordpress.com" - - var valueForJetpack: String { - switch self { - case .authToken: - return "jetpack.public-api.wordpress.com" - } - } - } -} - // MARK: - Today Widget Helpers private extension SharedDataIssueSolver { diff --git a/Tests/KeystoneTests/Tests/Jetpack/DataMigratorTests.swift b/Tests/KeystoneTests/Tests/Jetpack/DataMigratorTests.swift index 1dd9abe933bb..d5c06b1e3736 100644 --- a/Tests/KeystoneTests/Tests/Jetpack/DataMigratorTests.swift +++ b/Tests/KeystoneTests/Tests/Jetpack/DataMigratorTests.swift @@ -1,12 +1,15 @@ +import Security import XCTest @testable import WordPress +@testable import WordPressData class DataMigratorTests: XCTestCase { private var context: NSManagedObjectContext! private var migrator: DataMigrator! private var coreDataStack: CoreDataStackMock! - private var keychainUtils: KeychainUtilsMock! + private var appKeychain: KeychainUtilsMock! + private var sharedKeychain: KeychainUtilsMock! private var sharedUserDefaults: InMemoryUserDefaults! private var localUserDefaults: InMemoryUserDefaults! private let appGroupName = "xctest_app_group_name" @@ -16,13 +19,16 @@ class DataMigratorTests: XCTestCase { context = try! createContext() coreDataStack = CoreDataStackMock(mainContext: context) - keychainUtils = KeychainUtilsMock() + appKeychain = KeychainUtilsMock() + sharedKeychain = KeychainUtilsMock() sharedUserDefaults = InMemoryUserDefaults() localUserDefaults = InMemoryUserDefaults() + UserSettings.defaultDotComUUID = nil migrator = DataMigrator( coreDataStack: coreDataStack, backupLocation: URL(string: "/dev/null"), - keychainUtils: keychainUtils, + appKeychain: appKeychain, + sharedKeychain: sharedKeychain, localDefaults: localUserDefaults, sharedDefaults: sharedUserDefaults, crashLogger: nil, @@ -30,6 +36,11 @@ class DataMigratorTests: XCTestCase { ) } + override func tearDown() { + UserSettings.defaultDotComUUID = nil + super.tearDown() + } + func testExportSucceeds() { // When var successful = false @@ -73,7 +84,8 @@ class DataMigratorTests: XCTestCase { migrator = DataMigrator( coreDataStack: coreDataStack, backupLocation: URL(string: "/dev/null"), - keychainUtils: keychainUtils, + appKeychain: appKeychain, + sharedKeychain: sharedKeychain, sharedDefaults: nil, crashLogger: nil, appGroupName: appGroupName @@ -107,6 +119,116 @@ class DataMigratorTests: XCTestCase { wait(for: [expect], timeout: 1) } + // MARK: Auth token handoff tests + + func testExportPublishesAuthTokenToSharedKeychain() { + let account = AccountBuilder(context) + .with(username: "exportuser") + .build() + UserSettings.defaultDotComUUID = account.uuid + appKeychain.passwords["public-api.wordpress.com"] = ["exportuser": "token-123"] + + let expectation = expectation(description: "export completes") + migrator.exportData { _ in expectation.fulfill() } + waitForExpectations(timeout: 1) + + XCTAssertEqual(sharedKeychain.passwords["public-api.wordpress.com"]?["exportuser"], "token-123") + XCTAssertEqual(sharedUserDefaults.string(forKey: "wp_data_migration_exported_username"), "exportuser") + } + + func testExportSucceedsWhenLoggedOut() { + let expectation = expectation(description: "export completes") + var exportResult: Result? + migrator.exportData { result in + exportResult = result + expectation.fulfill() + } + waitForExpectations(timeout: 1) + + guard case .success = exportResult else { + return XCTFail("export should succeed without an account") + } + XCTAssertNil(sharedKeychain.passwords["public-api.wordpress.com"]) + } + + func testExportFailsWhenTokenPublishFails() { + let account = AccountBuilder(context) + .with(username: "exportuser") + .build() + UserSettings.defaultDotComUUID = account.uuid + appKeychain.passwords["public-api.wordpress.com"] = ["exportuser": "token-123"] + sharedKeychain.setPasswordError = NSError(domain: "test", code: 1) + + let expectation = expectation(description: "export completes") + var exportResult: Result? + migrator.exportData { result in + exportResult = result + expectation.fulfill() + } + waitForExpectations(timeout: 1) + + guard case .failure = exportResult else { + return XCTFail("export should fail when the token publish fails") + } + } + + func testExportSucceedsWhenTokenMissing() { + let account = AccountBuilder(context) + .with(username: "exportuser") + .build() + UserSettings.defaultDotComUUID = account.uuid + appKeychain.getPasswordError = NSError(domain: "test", code: 1) + + let expectation = expectation(description: "export completes") + var exportResult: Result? + migrator.exportData { result in + exportResult = result + expectation.fulfill() + } + waitForExpectations(timeout: 1) + + guard case .success = exportResult else { + return XCTFail("export should stay best-effort when there is no token to publish") + } + XCTAssertNil(sharedKeychain.passwords["public-api.wordpress.com"]) + } + + func testExportFailsWhenTokenReadFails() { + let account = AccountBuilder(context) + .with(username: "exportuser") + .build() + UserSettings.defaultDotComUUID = account.uuid + // A real keychain failure carries SFHFKeychainUtils' error domain + // with the raw OSStatus, unlike the generic not-found error. + appKeychain.getPasswordError = NSError( + domain: "SFHFKeychainUtilsErrorDomain", + code: Int(errSecInteractionNotAllowed) + ) + + let expectation = expectation(description: "export completes") + var exportResult: Result? + migrator.exportData { result in + exportResult = result + expectation.fulfill() + } + waitForExpectations(timeout: 1) + + guard case .failure = exportResult else { + return XCTFail("export should fail when the token read fails with a real keychain error") + } + XCTAssertNil(sharedKeychain.passwords["public-api.wordpress.com"]) + } + + func testDeleteExportedDataRemovesPublishedToken() { + sharedKeychain.passwords["public-api.wordpress.com"] = ["exportuser": "token-123"] + sharedUserDefaults.set("exportuser", forKey: "wp_data_migration_exported_username") + + migrator.deleteExportedData() + + XCTAssertNil(sharedKeychain.passwords["public-api.wordpress.com"]?["exportuser"]) + XCTAssertNil(sharedUserDefaults.string(forKey: "wp_data_migration_exported_username")) + } + // MARK: Exported data deletion tests func test_deleteExportedData_shouldMarkDataNotReadyToMigrate() { @@ -154,7 +276,8 @@ class DataMigratorTests: XCTestCase { migrator = DataMigrator( coreDataStack: coreDataStack, backupLocation: backupLocation, - keychainUtils: keychainUtils, + appKeychain: appKeychain, + sharedKeychain: sharedKeychain, localDefaults: localUserDefaults, sharedDefaults: sharedUserDefaults, crashLogger: nil, @@ -204,7 +327,8 @@ class DataMigratorTests: XCTestCase { migrator = DataMigrator( coreDataStack: coreDataStack, backupLocation: backupLocation, - keychainUtils: keychainUtils, + appKeychain: appKeychain, + sharedKeychain: sharedKeychain, localDefaults: localUserDefaults, sharedDefaults: sharedUserDefaults, crashLogger: nil, diff --git a/Tests/KeystoneTests/Tests/Jetpack/SharedDataIssueSolverTests.swift b/Tests/KeystoneTests/Tests/Jetpack/SharedDataIssueSolverTests.swift index 21a920098ee4..eff824c2aa93 100644 --- a/Tests/KeystoneTests/Tests/Jetpack/SharedDataIssueSolverTests.swift +++ b/Tests/KeystoneTests/Tests/Jetpack/SharedDataIssueSolverTests.swift @@ -7,7 +7,8 @@ class SharedDataIssueSolverTests: XCTestCase { private var context: NSManagedObjectContext! private var contextManager: CoreDataStackMock! - private var keychainUtils: KeychainUtilsMock! + private var appKeychain: KeychainUtilsMock! + private var sharedKeychain: KeychainUtilsMock! private var sharedUserDefaults: InMemoryUserDefaults! private var mockLocalStore: MockLocalFileStore! private var sharedDataIssueSolver: SharedDataIssueSolver! @@ -17,12 +18,14 @@ class SharedDataIssueSolverTests: XCTestCase { context = try! createInMemoryContext() contextManager = CoreDataStackMock(mainContext: context) - keychainUtils = KeychainUtilsMock() + appKeychain = KeychainUtilsMock() + sharedKeychain = KeychainUtilsMock() sharedUserDefaults = InMemoryUserDefaults() mockLocalStore = MockLocalFileStore() sharedDataIssueSolver = SharedDataIssueSolver( contextManager: contextManager, - keychainUtils: keychainUtils, + appKeychain: appKeychain, + sharedKeychain: sharedKeychain, sharedDefaults: sharedUserDefaults, localFileStore: mockLocalStore, appGroupName: "xctest_app_group_name" diff --git a/WordPress/Classes/Jetpack/Utility/DataMigrationError.swift b/WordPress/Classes/Jetpack/Utility/DataMigrationError.swift index 39e15849733f..576f71f64fda 100644 --- a/WordPress/Classes/Jetpack/Utility/DataMigrationError.swift +++ b/WordPress/Classes/Jetpack/Utility/DataMigrationError.swift @@ -3,6 +3,7 @@ import Foundation enum DataMigrationError { case databaseImportError(underlyingError: Error) case databaseExportError(underlyingError: Error) + case keychainExportError(underlyingError: Error) case backupLocationNil case sharedUserDefaultsNil case dataNotReadyToImport @@ -17,11 +18,12 @@ extension DataMigrationError: LocalizedError, CustomNSError { case .dataNotReadyToImport: return "The data wasn't ready to import" case .databaseImportError(let error): return "Import Failed: \(error.localizedDescription)" case .databaseExportError(let error): return "Export Failed: \(error.localizedDescription)" + case .keychainExportError(let error): return "Keychain export failed: \(error.localizedDescription)" } } static var errorDomain: String { - return String(describing: DataMigrationError.self) + String(describing: DataMigrationError.self) } var errorCode: Int { @@ -31,17 +33,20 @@ extension DataMigrationError: LocalizedError, CustomNSError { case .sharedUserDefaultsNil: return 201 case .databaseImportError(let error): return 1000 + (error as NSError).code case .databaseExportError(let error): return 2000 + (error as NSError).code + case .keychainExportError(let error): return 3000 + (error as NSError).code } } var errorUserInfo: [String: Any] { switch self { - case .databaseExportError(let error), .databaseImportError(let error): + case .databaseExportError(let error), .databaseImportError(let error), .keychainExportError(let error): let nsError = error as NSError - return ["underlying-error-domain": nsError.domain, - "underlying-error-code": nsError.code, - "underlying-error-message": nsError.localizedDescription, - "underlying-error-user-info": nsError.userInfo] + return [ + "underlying-error-domain": nsError.domain, + "underlying-error-code": nsError.code, + "underlying-error-message": nsError.localizedDescription, + "underlying-error-user-info": nsError.userInfo + ] default: return [:] } @@ -50,13 +55,13 @@ extension DataMigrationError: LocalizedError, CustomNSError { extension DataMigrationError: CustomDebugStringConvertible { var debugDescription: String { - return "[\(Self.errorDomain)] \(localizedDescription)" + "[\(Self.errorDomain)] \(localizedDescription)" } } extension DataMigrationError: Equatable { - static func ==(left: DataMigrationError, right: DataMigrationError) -> Bool { + static func == (left: DataMigrationError, right: DataMigrationError) -> Bool { let leftNSError = left as NSError let rightNSError = right as NSError return leftNSError == rightNSError diff --git a/WordPress/Classes/Jetpack/Utility/DataMigrator.swift b/WordPress/Classes/Jetpack/Utility/DataMigrator.swift index 233bc6d830e9..d1a63b48227e 100644 --- a/WordPress/Classes/Jetpack/Utility/DataMigrator.swift +++ b/WordPress/Classes/Jetpack/Utility/DataMigrator.swift @@ -22,7 +22,8 @@ protocol ContentDataMigrating { final class DataMigrator { private let coreDataStack: CoreDataStack private let backupLocation: URL? - private let keychainUtils: KeychainAccessible + private let appKeychain: KeychainAccessible + private let sharedKeychain: KeychainAccessible? private let localDefaults: UserPersistentRepository private let sharedDefaults: UserPersistentRepository? private let crashLogger: CrashLogging? @@ -34,7 +35,8 @@ final class DataMigrator { forSecurityApplicationGroupIdentifier: BuildSettings.current.appGroupName )? .appendingPathComponent("WordPress.sqlite"), - keychainUtils: KeychainAccessible = KeychainUtils(), + appKeychain: KeychainAccessible = AppKeychain(), + sharedKeychain: KeychainAccessible? = SharedKeychain(), localDefaults: UserPersistentRepository = UserDefaults.standard, sharedDefaults: UserPersistentRepository? = UserDefaults(suiteName: BuildSettings.current.appGroupName), crashLogger: CrashLogging? = .main, @@ -42,7 +44,8 @@ final class DataMigrator { ) { self.coreDataStack = coreDataStack self.backupLocation = backupLocation - self.keychainUtils = keychainUtils + self.appKeychain = appKeychain + self.sharedKeychain = sharedKeychain self.localDefaults = localDefaults self.sharedDefaults = sharedDefaults self.crashLogger = crashLogger @@ -64,6 +67,14 @@ extension DataMigrator: ContentDataMigrating { completion?(.failure(error)) return } + do { + try publishAuthTokenToSharedKeychain() + } catch { + let error = DataMigrationError.keychainExportError(underlyingError: error) + log(error: error) + completion?(.failure(error)) + return + } BloggingRemindersScheduler.handleRemindersMigration(appGroupName: appGroupName) isDataReadyToMigrate = true @@ -94,7 +105,8 @@ extension DataMigrator: ContentDataMigrating { let sharedDataIssueSolver = SharedDataIssueSolver( contextManager: coreDataStack, - keychainUtils: keychainUtils, + appKeychain: appKeychain, + sharedKeychain: sharedKeychain, sharedDefaults: sharedDefaults, appGroupName: appGroupName ) @@ -115,6 +127,11 @@ extension DataMigrator: ContentDataMigrating { // this serves as the first stopgap that prevents the migration process on the Jetpack side. isDataReadyToMigrate = false + // Remove the token published for the migration handoff. This is an + // improvement over the pre-change behavior, where the token lingered + // in the shared group after logout. + removePublishedAuthToken() + // remove database backup try? coreDataStack.removeBackupData(from: backupLocation) @@ -135,6 +152,54 @@ private extension DataMigrator { static let dictKey = "defaults_staging_dictionary" } + static let exportedUsernameKey = "wp_data_migration_exported_username" + + /// Publishes the WP.com auth token to the shared keychain group so the + /// Jetpack app (old or new versions) can import it. + /// + /// A MISSING token keeps the pre-change best-effort behavior (that user + /// was already broken before the keychain split). A real keychain + /// failure, on the read or the write, throws and must fail the export: + /// the published token is the only way the login reaches Jetpack, so a + /// "successful" export without it would migrate the content and land + /// the user signed out. + func publishAuthTokenToSharedKeychain() throws { + guard let account = try? WPAccount.lookupDefaultWordPressComAccount(in: coreDataStack.mainContext), + let sharedKeychain + else { + return + } + let username = account.username + let token: String + do { + token = try appKeychain.getPassword(for: username, serviceName: AuthTokenServiceNames.wordPress) + } catch { + guard isRealKeychainFailure(error) else { + crashLogger?.logMessage("Keychain token unavailable during migration export", level: .info) + return + } + crashLogger?.logError(error, userInfo: ["context": "migration-token-handoff"], level: .error) + throw error + } + do { + try sharedKeychain.setPassword(for: username, to: token, serviceName: AuthTokenServiceNames.wordPress) + sharedDefaults?.set(username, forKey: Self.exportedUsernameKey) + } catch { + crashLogger?.logError(error, userInfo: ["context": "migration-token-handoff"], level: .error) + throw error + } + } + + func removePublishedAuthToken() { + guard let sharedDefaults, + let username = sharedDefaults.string(forKey: Self.exportedUsernameKey) + else { + return + } + try? sharedKeychain?.setPassword(for: username, to: nil, serviceName: AuthTokenServiceNames.wordPress) + sharedDefaults.removeObject(forKey: Self.exportedUsernameKey) + } + /// Convenience wrapper to check whether the export data is ready to be imported. /// The value is stored in the App Group space so it is accessible from both apps. var isDataReadyToMigrate: Bool { From 1512759d5a0c30d973d8f89cd0ba1366cc48100e Mon Sep 17 00:00:00 2001 From: Tony Li Date: Sun, 14 Jun 2026 18:43:06 +1200 Subject: [PATCH 09/13] Move the WordPress app and its extensions onto a private keychain access group --- WordPress/WordPress-Alpha.entitlements | 1 + WordPress/WordPress.entitlements | 1 + .../WordPressDraftActionExtension-Alpha.entitlements | 1 + .../WordPressDraftActionExtension.entitlements | 1 + .../WordPressNotificationServiceExtension-Alpha.entitlements | 1 + .../WordPressNotificationServiceExtension.entitlements | 1 + .../WordPressShareExtension/WordPressShare-Alpha.entitlements | 1 + WordPress/WordPressShareExtension/WordPressShare.entitlements | 1 + 8 files changed, 8 insertions(+) diff --git a/WordPress/WordPress-Alpha.entitlements b/WordPress/WordPress-Alpha.entitlements index 746c03fe2ddc..6df9777ecf46 100644 --- a/WordPress/WordPress-Alpha.entitlements +++ b/WordPress/WordPress-Alpha.entitlements @@ -13,6 +13,7 @@ keychain-access-groups + 99KV9Z6BKV.org.wordpress.alpha.wordpress 99KV9Z6BKV.org.wordpress.alpha diff --git a/WordPress/WordPress.entitlements b/WordPress/WordPress.entitlements index 01c82000c649..5a08bd31c83c 100644 --- a/WordPress/WordPress.entitlements +++ b/WordPress/WordPress.entitlements @@ -33,6 +33,7 @@ keychain-access-groups + 3TMU3BH3NK.org.wordpress.wordpress 3TMU3BH3NK.org.wordpress diff --git a/WordPress/WordPressDraftActionExtension/WordPressDraftActionExtension-Alpha.entitlements b/WordPress/WordPressDraftActionExtension/WordPressDraftActionExtension-Alpha.entitlements index dca30c5b176d..f4ecb8806a25 100644 --- a/WordPress/WordPressDraftActionExtension/WordPressDraftActionExtension-Alpha.entitlements +++ b/WordPress/WordPressDraftActionExtension/WordPressDraftActionExtension-Alpha.entitlements @@ -8,6 +8,7 @@ keychain-access-groups + 99KV9Z6BKV.org.wordpress.alpha.wordpress 99KV9Z6BKV.org.wordpress.alpha diff --git a/WordPress/WordPressDraftActionExtension/WordPressDraftActionExtension.entitlements b/WordPress/WordPressDraftActionExtension/WordPressDraftActionExtension.entitlements index 7eb5daa80a0d..916b942b0ccf 100644 --- a/WordPress/WordPressDraftActionExtension/WordPressDraftActionExtension.entitlements +++ b/WordPress/WordPressDraftActionExtension/WordPressDraftActionExtension.entitlements @@ -8,6 +8,7 @@ keychain-access-groups + 3TMU3BH3NK.org.wordpress.wordpress 3TMU3BH3NK.org.wordpress diff --git a/WordPress/WordPressNotificationServiceExtension/WordPressNotificationServiceExtension-Alpha.entitlements b/WordPress/WordPressNotificationServiceExtension/WordPressNotificationServiceExtension-Alpha.entitlements index dca30c5b176d..f4ecb8806a25 100644 --- a/WordPress/WordPressNotificationServiceExtension/WordPressNotificationServiceExtension-Alpha.entitlements +++ b/WordPress/WordPressNotificationServiceExtension/WordPressNotificationServiceExtension-Alpha.entitlements @@ -8,6 +8,7 @@ keychain-access-groups + 99KV9Z6BKV.org.wordpress.alpha.wordpress 99KV9Z6BKV.org.wordpress.alpha diff --git a/WordPress/WordPressNotificationServiceExtension/WordPressNotificationServiceExtension.entitlements b/WordPress/WordPressNotificationServiceExtension/WordPressNotificationServiceExtension.entitlements index 7eb5daa80a0d..916b942b0ccf 100644 --- a/WordPress/WordPressNotificationServiceExtension/WordPressNotificationServiceExtension.entitlements +++ b/WordPress/WordPressNotificationServiceExtension/WordPressNotificationServiceExtension.entitlements @@ -8,6 +8,7 @@ keychain-access-groups + 3TMU3BH3NK.org.wordpress.wordpress 3TMU3BH3NK.org.wordpress diff --git a/WordPress/WordPressShareExtension/WordPressShare-Alpha.entitlements b/WordPress/WordPressShareExtension/WordPressShare-Alpha.entitlements index dca30c5b176d..f4ecb8806a25 100644 --- a/WordPress/WordPressShareExtension/WordPressShare-Alpha.entitlements +++ b/WordPress/WordPressShareExtension/WordPressShare-Alpha.entitlements @@ -8,6 +8,7 @@ keychain-access-groups + 99KV9Z6BKV.org.wordpress.alpha.wordpress 99KV9Z6BKV.org.wordpress.alpha diff --git a/WordPress/WordPressShareExtension/WordPressShare.entitlements b/WordPress/WordPressShareExtension/WordPressShare.entitlements index 7eb5daa80a0d..916b942b0ccf 100644 --- a/WordPress/WordPressShareExtension/WordPressShare.entitlements +++ b/WordPress/WordPressShareExtension/WordPressShare.entitlements @@ -8,6 +8,7 @@ keychain-access-groups + 3TMU3BH3NK.org.wordpress.wordpress 3TMU3BH3NK.org.wordpress From b5993bed503e4bf37ce91aa73ec3de4d73ac66ae Mon Sep 17 00:00:00 2001 From: Tony Li Date: Sun, 14 Jun 2026 18:43:06 +1200 Subject: [PATCH 10/13] Move the Jetpack app and its extensions onto a private keychain access group --- .../JetpackStatsWidgets.entitlements | 1 + ...tpackStatsWidgetsRelease-Alpha.entitlements | 3 ++- WordPress/Jetpack/JetpackDebug.entitlements | 1 + .../Jetpack/JetpackRelease-Alpha.entitlements | 1 + WordPress/Jetpack/JetpackRelease.entitlements | 1 + WordPress/WordPress.xcodeproj/project.pbxproj | 18 +++++++++--------- ...packDraftActionExtension-Alpha.entitlements | 15 +++++++++++++++ .../JetpackDraftActionExtension.entitlements | 15 +++++++++++++++ ...ficationServiceExtension-Alpha.entitlements | 15 +++++++++++++++ ...ckNotificationServiceExtension.entitlements | 15 +++++++++++++++ .../JetpackShare-Alpha.entitlements | 15 +++++++++++++++ .../JetpackShare.entitlements | 15 +++++++++++++++ 12 files changed, 105 insertions(+), 10 deletions(-) create mode 100644 WordPress/WordPressDraftActionExtension/JetpackDraftActionExtension-Alpha.entitlements create mode 100644 WordPress/WordPressDraftActionExtension/JetpackDraftActionExtension.entitlements create mode 100644 WordPress/WordPressNotificationServiceExtension/JetpackNotificationServiceExtension-Alpha.entitlements create mode 100644 WordPress/WordPressNotificationServiceExtension/JetpackNotificationServiceExtension.entitlements create mode 100644 WordPress/WordPressShareExtension/JetpackShare-Alpha.entitlements create mode 100644 WordPress/WordPressShareExtension/JetpackShare.entitlements diff --git a/Sources/JetpackStatsWidgets/JetpackStatsWidgets.entitlements b/Sources/JetpackStatsWidgets/JetpackStatsWidgets.entitlements index 7eb5daa80a0d..dce8e1a27cf8 100644 --- a/Sources/JetpackStatsWidgets/JetpackStatsWidgets.entitlements +++ b/Sources/JetpackStatsWidgets/JetpackStatsWidgets.entitlements @@ -8,6 +8,7 @@ keychain-access-groups + 3TMU3BH3NK.org.wordpress.jetpack 3TMU3BH3NK.org.wordpress diff --git a/Sources/JetpackStatsWidgets/JetpackStatsWidgetsRelease-Alpha.entitlements b/Sources/JetpackStatsWidgets/JetpackStatsWidgetsRelease-Alpha.entitlements index 5517320d7801..49a5972cc9c3 100644 --- a/Sources/JetpackStatsWidgets/JetpackStatsWidgetsRelease-Alpha.entitlements +++ b/Sources/JetpackStatsWidgets/JetpackStatsWidgetsRelease-Alpha.entitlements @@ -8,7 +8,8 @@ keychain-access-groups - $(AppIdentifierPrefix)99KV9Z6BKV.org.wordpress.alpha + 99KV9Z6BKV.org.wordpress.alpha.jetpack + 99KV9Z6BKV.org.wordpress.alpha diff --git a/WordPress/Jetpack/JetpackDebug.entitlements b/WordPress/Jetpack/JetpackDebug.entitlements index 11eb0930bd31..4e42a7383c25 100644 --- a/WordPress/Jetpack/JetpackDebug.entitlements +++ b/WordPress/Jetpack/JetpackDebug.entitlements @@ -24,6 +24,7 @@ keychain-access-groups + 3TMU3BH3NK.org.wordpress.jetpack 3TMU3BH3NK.org.wordpress diff --git a/WordPress/Jetpack/JetpackRelease-Alpha.entitlements b/WordPress/Jetpack/JetpackRelease-Alpha.entitlements index 486afcb0595c..12bd1bd7bdbb 100644 --- a/WordPress/Jetpack/JetpackRelease-Alpha.entitlements +++ b/WordPress/Jetpack/JetpackRelease-Alpha.entitlements @@ -20,6 +20,7 @@ keychain-access-groups + 99KV9Z6BKV.org.wordpress.alpha.jetpack 99KV9Z6BKV.org.wordpress.alpha diff --git a/WordPress/Jetpack/JetpackRelease.entitlements b/WordPress/Jetpack/JetpackRelease.entitlements index 11eb0930bd31..4e42a7383c25 100644 --- a/WordPress/Jetpack/JetpackRelease.entitlements +++ b/WordPress/Jetpack/JetpackRelease.entitlements @@ -24,6 +24,7 @@ keychain-access-groups + 3TMU3BH3NK.org.wordpress.jetpack 3TMU3BH3NK.org.wordpress diff --git a/WordPress/WordPress.xcodeproj/project.pbxproj b/WordPress/WordPress.xcodeproj/project.pbxproj index e6a0ca6a0a5a..131838cbf642 100644 --- a/WordPress/WordPress.xcodeproj/project.pbxproj +++ b/WordPress/WordPress.xcodeproj/project.pbxproj @@ -4658,7 +4658,7 @@ CLANG_WARN_OBJC_ROOT_CLASS = YES_ERROR; CLANG_WARN_UNREACHABLE_CODE = YES; CLANG_WARN__DUPLICATE_METHOD_MATCH = YES; - CODE_SIGN_ENTITLEMENTS = WordPressShareExtension/WordPressShare.entitlements; + CODE_SIGN_ENTITLEMENTS = WordPressShareExtension/JetpackShare.entitlements; COPY_PHASE_STRIP = NO; ENABLE_BITCODE = NO; ENABLE_STRICT_OBJC_MSGSEND = YES; @@ -4714,7 +4714,7 @@ CLANG_WARN_OBJC_ROOT_CLASS = YES_ERROR; CLANG_WARN_UNREACHABLE_CODE = YES; CLANG_WARN__DUPLICATE_METHOD_MATCH = YES; - CODE_SIGN_ENTITLEMENTS = WordPressShareExtension/WordPressShare.entitlements; + CODE_SIGN_ENTITLEMENTS = WordPressShareExtension/JetpackShare.entitlements; CODE_SIGN_IDENTITY = "Apple Distribution: Automattic, Inc. (PZYM8XX95Q)"; COPY_PHASE_STRIP = NO; ENABLE_BITCODE = NO; @@ -4766,7 +4766,7 @@ CLANG_WARN_OBJC_ROOT_CLASS = YES_ERROR; CLANG_WARN_UNREACHABLE_CODE = YES; CLANG_WARN__DUPLICATE_METHOD_MATCH = YES; - CODE_SIGN_ENTITLEMENTS = "WordPressShareExtension/WordPressShare-Alpha.entitlements"; + CODE_SIGN_ENTITLEMENTS = "WordPressShareExtension/JetpackShare-Alpha.entitlements"; CODE_SIGN_IDENTITY = "iPhone Distribution: Automattic, Inc."; COPY_PHASE_STRIP = NO; ENABLE_BITCODE = NO; @@ -4815,7 +4815,7 @@ CLANG_WARN_DOCUMENTATION_COMMENTS = NO; CLANG_WARN_OBJC_ROOT_CLASS = YES_ERROR; CLANG_WARN_UNGUARDED_AVAILABILITY = YES; - CODE_SIGN_ENTITLEMENTS = WordPressDraftActionExtension/WordPressDraftActionExtension.entitlements; + CODE_SIGN_ENTITLEMENTS = WordPressDraftActionExtension/JetpackDraftActionExtension.entitlements; CODE_SIGN_STYLE = Manual; COPY_PHASE_STRIP = NO; ENABLE_BITCODE = NO; @@ -4862,7 +4862,7 @@ CLANG_WARN_DOCUMENTATION_COMMENTS = NO; CLANG_WARN_OBJC_ROOT_CLASS = YES_ERROR; CLANG_WARN_UNGUARDED_AVAILABILITY = YES; - CODE_SIGN_ENTITLEMENTS = WordPressDraftActionExtension/WordPressDraftActionExtension.entitlements; + CODE_SIGN_ENTITLEMENTS = WordPressDraftActionExtension/JetpackDraftActionExtension.entitlements; CODE_SIGN_IDENTITY = "Apple Distribution: Automattic, Inc. (PZYM8XX95Q)"; CODE_SIGN_STYLE = Manual; COPY_PHASE_STRIP = NO; @@ -4905,7 +4905,7 @@ CLANG_WARN_DOCUMENTATION_COMMENTS = NO; CLANG_WARN_OBJC_ROOT_CLASS = YES_ERROR; CLANG_WARN_UNGUARDED_AVAILABILITY = YES; - CODE_SIGN_ENTITLEMENTS = "WordPressDraftActionExtension/WordPressDraftActionExtension-Alpha.entitlements"; + CODE_SIGN_ENTITLEMENTS = "WordPressDraftActionExtension/JetpackDraftActionExtension-Alpha.entitlements"; CODE_SIGN_IDENTITY = "iPhone Distribution: Automattic, Inc."; CODE_SIGN_STYLE = Manual; COPY_PHASE_STRIP = NO; @@ -4947,7 +4947,7 @@ CLANG_WARN_DOCUMENTATION_COMMENTS = YES; CLANG_WARN_OBJC_ROOT_CLASS = YES_ERROR; CLANG_WARN_UNGUARDED_AVAILABILITY = YES_AGGRESSIVE; - CODE_SIGN_ENTITLEMENTS = WordPressNotificationServiceExtension/WordPressNotificationServiceExtension.entitlements; + CODE_SIGN_ENTITLEMENTS = WordPressNotificationServiceExtension/JetpackNotificationServiceExtension.entitlements; COPY_PHASE_STRIP = NO; GCC_C_LANGUAGE_STANDARD = gnu11; GCC_DYNAMIC_NO_PIC = NO; @@ -4988,7 +4988,7 @@ CLANG_WARN_DOCUMENTATION_COMMENTS = YES; CLANG_WARN_OBJC_ROOT_CLASS = YES_ERROR; CLANG_WARN_UNGUARDED_AVAILABILITY = YES_AGGRESSIVE; - CODE_SIGN_ENTITLEMENTS = WordPressNotificationServiceExtension/WordPressNotificationServiceExtension.entitlements; + CODE_SIGN_ENTITLEMENTS = WordPressNotificationServiceExtension/JetpackNotificationServiceExtension.entitlements; CODE_SIGN_IDENTITY = "Apple Distribution: Automattic, Inc. (PZYM8XX95Q)"; COPY_PHASE_STRIP = NO; ENABLE_NS_ASSERTIONS = NO; @@ -5025,7 +5025,7 @@ CLANG_WARN_DOCUMENTATION_COMMENTS = YES; CLANG_WARN_OBJC_ROOT_CLASS = YES_ERROR; CLANG_WARN_UNGUARDED_AVAILABILITY = YES_AGGRESSIVE; - CODE_SIGN_ENTITLEMENTS = "WordPressNotificationServiceExtension/WordPressNotificationServiceExtension-Alpha.entitlements"; + CODE_SIGN_ENTITLEMENTS = "WordPressNotificationServiceExtension/JetpackNotificationServiceExtension-Alpha.entitlements"; CODE_SIGN_IDENTITY = "iPhone Distribution: Automattic, Inc."; COPY_PHASE_STRIP = NO; ENABLE_NS_ASSERTIONS = NO; diff --git a/WordPress/WordPressDraftActionExtension/JetpackDraftActionExtension-Alpha.entitlements b/WordPress/WordPressDraftActionExtension/JetpackDraftActionExtension-Alpha.entitlements new file mode 100644 index 000000000000..49a5972cc9c3 --- /dev/null +++ b/WordPress/WordPressDraftActionExtension/JetpackDraftActionExtension-Alpha.entitlements @@ -0,0 +1,15 @@ + + + + + com.apple.security.application-groups + + group.org.wordpress.alpha + + keychain-access-groups + + 99KV9Z6BKV.org.wordpress.alpha.jetpack + 99KV9Z6BKV.org.wordpress.alpha + + + diff --git a/WordPress/WordPressDraftActionExtension/JetpackDraftActionExtension.entitlements b/WordPress/WordPressDraftActionExtension/JetpackDraftActionExtension.entitlements new file mode 100644 index 000000000000..dce8e1a27cf8 --- /dev/null +++ b/WordPress/WordPressDraftActionExtension/JetpackDraftActionExtension.entitlements @@ -0,0 +1,15 @@ + + + + + com.apple.security.application-groups + + group.org.wordpress + + keychain-access-groups + + 3TMU3BH3NK.org.wordpress.jetpack + 3TMU3BH3NK.org.wordpress + + + diff --git a/WordPress/WordPressNotificationServiceExtension/JetpackNotificationServiceExtension-Alpha.entitlements b/WordPress/WordPressNotificationServiceExtension/JetpackNotificationServiceExtension-Alpha.entitlements new file mode 100644 index 000000000000..49a5972cc9c3 --- /dev/null +++ b/WordPress/WordPressNotificationServiceExtension/JetpackNotificationServiceExtension-Alpha.entitlements @@ -0,0 +1,15 @@ + + + + + com.apple.security.application-groups + + group.org.wordpress.alpha + + keychain-access-groups + + 99KV9Z6BKV.org.wordpress.alpha.jetpack + 99KV9Z6BKV.org.wordpress.alpha + + + diff --git a/WordPress/WordPressNotificationServiceExtension/JetpackNotificationServiceExtension.entitlements b/WordPress/WordPressNotificationServiceExtension/JetpackNotificationServiceExtension.entitlements new file mode 100644 index 000000000000..dce8e1a27cf8 --- /dev/null +++ b/WordPress/WordPressNotificationServiceExtension/JetpackNotificationServiceExtension.entitlements @@ -0,0 +1,15 @@ + + + + + com.apple.security.application-groups + + group.org.wordpress + + keychain-access-groups + + 3TMU3BH3NK.org.wordpress.jetpack + 3TMU3BH3NK.org.wordpress + + + diff --git a/WordPress/WordPressShareExtension/JetpackShare-Alpha.entitlements b/WordPress/WordPressShareExtension/JetpackShare-Alpha.entitlements new file mode 100644 index 000000000000..49a5972cc9c3 --- /dev/null +++ b/WordPress/WordPressShareExtension/JetpackShare-Alpha.entitlements @@ -0,0 +1,15 @@ + + + + + com.apple.security.application-groups + + group.org.wordpress.alpha + + keychain-access-groups + + 99KV9Z6BKV.org.wordpress.alpha.jetpack + 99KV9Z6BKV.org.wordpress.alpha + + + diff --git a/WordPress/WordPressShareExtension/JetpackShare.entitlements b/WordPress/WordPressShareExtension/JetpackShare.entitlements new file mode 100644 index 000000000000..dce8e1a27cf8 --- /dev/null +++ b/WordPress/WordPressShareExtension/JetpackShare.entitlements @@ -0,0 +1,15 @@ + + + + + com.apple.security.application-groups + + group.org.wordpress + + keychain-access-groups + + 3TMU3BH3NK.org.wordpress.jetpack + 3TMU3BH3NK.org.wordpress + + + From 9fd902db1f29b54fcad041bb0b0e67e6125724ad Mon Sep 17 00:00:00 2001 From: Tony Li Date: Sun, 14 Jun 2026 18:43:06 +1200 Subject: [PATCH 11/13] Move the Reader app onto a private keychain access group --- Sources/Reader/Info.plist | 2 +- Sources/Reader/Reader.entitlements | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Sources/Reader/Info.plist b/Sources/Reader/Info.plist index 79f54d2c6dc6..f4d60e9f0440 100644 --- a/Sources/Reader/Info.plist +++ b/Sources/Reader/Info.plist @@ -11,7 +11,7 @@ WPAppURLScheme ${WP_APP_URL_SCHEME} WPAuthKeychainServiceName - jetpack.public-api.wordpress.com + reader.public-api.wordpress.com WPBuildConfiguration ${WP_BUILD_CONFIGURATION} WPEventNamePrefix diff --git a/Sources/Reader/Reader.entitlements b/Sources/Reader/Reader.entitlements index c040628806ec..9ee1e3b4f334 100644 --- a/Sources/Reader/Reader.entitlements +++ b/Sources/Reader/Reader.entitlements @@ -15,7 +15,7 @@ keychain-access-groups - 3TMU3BH3NK.org.wordpress + $(WP_APP_KEYCHAIN_ACCESS_GROUP) From 21b4dee8609447c00b7ecacf2923fff140bca1bb Mon Sep 17 00:00:00 2001 From: Tony Li Date: Mon, 15 Jun 2026 14:53:07 +1200 Subject: [PATCH 12/13] Deprecate KeychainUtils in favor of AppKeychain and SharedKeychain --- .../Sources/WordPressShared/Keychain/KeychainUtils.swift | 7 +++++++ .../Tests/WordPressSharedTests/KeychainUtilsTests.swift | 1 + 2 files changed, 8 insertions(+) diff --git a/Modules/Sources/WordPressShared/Keychain/KeychainUtils.swift b/Modules/Sources/WordPressShared/Keychain/KeychainUtils.swift index f48c59023f89..b78e800626fd 100644 --- a/Modules/Sources/WordPressShared/Keychain/KeychainUtils.swift +++ b/Modules/Sources/WordPressShared/Keychain/KeychainUtils.swift @@ -1,5 +1,11 @@ import SFHFKeychainUtils +@available( + *, + deprecated, + message: + "Use AppKeychain for app-private data, or SharedKeychain for the cross-app migration contract. KeychainUtils operates on the default access group, which made every keychain item cross-app by accident." +) @objcMembers public class KeychainUtils: NSObject { @@ -26,6 +32,7 @@ public class KeychainUtils: NSObject { } } +@available(*, deprecated) extension KeychainUtils: KeychainAccessible { public func getPassword(for username: String, serviceName: String) throws -> String { try self.keychainUtils.getPasswordForUsername(username, andServiceName: serviceName) diff --git a/Modules/Tests/WordPressSharedTests/KeychainUtilsTests.swift b/Modules/Tests/WordPressSharedTests/KeychainUtilsTests.swift index 245e2c7f9da5..85a88c6c4ddf 100644 --- a/Modules/Tests/WordPressSharedTests/KeychainUtilsTests.swift +++ b/Modules/Tests/WordPressSharedTests/KeychainUtilsTests.swift @@ -10,6 +10,7 @@ class KeychainUtilsTests: XCTestCase { SFHFKeychainUtilsMock.configure(with: [:]) } + @available(*, deprecated, message: "Exercises the deprecated KeychainUtils.copyKeychain") func testCopyingPasswordsBetweenKeychains() { let service = "service" let username = "username" From 72488b5510852e4e03c4d41d7b79c85127573c9d Mon Sep 17 00:00:00 2001 From: Jeremy Massel <1123407+jkmassel@users.noreply.github.com> Date: Mon, 15 Jun 2026 11:52:21 -0600 Subject: [PATCH 13/13] Fix logout credential resurrection and crash on keychain entitlement drift Two keychain hardening changes surfaced while reviewing this PR. AppKeychain.setPassword(nil) deleted the shared group, caught any failure, then deleted the private group regardless. A real shared-delete failure with a successful private delete left the credential "private empty, shared present": the next read fell back to the shared copy and read-repaired it into the private group, resurrecting a logged-out token and re-exposing it in the cross-app group. The shared-first ordering only guarded against an interruption between the two deletes, not a thrown shared-delete error. Require the shared delete to succeed before touching the private copy. Add the mirror test; the suite only covered the reverse direction (private fails, shared succeeds). A keychain access group declared in Info.plist/BuildSettings but missing from the entitlement fails every operation with errSecMissingEntitlement, blocking login and leaking tokens cross-app. requireDeclaredAccessGroupsAreEntitled() probes each declared group at launch and crashes on that signal in release and beta too, so a misconfigured build cannot ship past TestFlight. Wired into WordPressAppDelegate.willFinishLaunching, shared by WordPress, Jetpack, and Reader. No-op on the Simulator; crashes only on the precise errSecMissingEntitlement code so a correct build is never falsely bricked. --- .../Keychain/AppKeychain.swift | 87 ++++++++++++++----- .../AppKeychainTests.swift | 24 +++++ .../Classes/System/WordPressAppDelegate.swift | 7 ++ 3 files changed, 96 insertions(+), 22 deletions(-) diff --git a/Modules/Sources/WordPressShared/Keychain/AppKeychain.swift b/Modules/Sources/WordPressShared/Keychain/AppKeychain.swift index 3fdf67e378bb..e31c617ff27d 100644 --- a/Modules/Sources/WordPressShared/Keychain/AppKeychain.swift +++ b/Modules/Sources/WordPressShared/Keychain/AppKeychain.swift @@ -1,5 +1,6 @@ import BuildSettingsKit import Foundation +import Security import SFHFKeychainUtils /// Keychain access scoped to this app family's private access group (the @@ -38,6 +39,59 @@ public final class AppKeychain: KeychainAccessible { self.keychainUtils = keychainUtils } + /// Requires that every keychain access group this app declares + /// (`WPAppKeychainAccessGroup`, plus `WPSharedKeychainAccessGroup` when + /// present) is actually granted by the app's `keychain-access-groups` + /// entitlement. Call once, early, at launch. + /// + /// A group declared in `Info.plist`/`BuildSettings` but missing from the + /// entitlement makes every keychain operation against it fail with + /// `errSecMissingEntitlement`: the app can neither persist nor read + /// credentials (login cannot complete) and a logout can leak the token into + /// the cross-app group. That is a blatant build misconfiguration, so this + /// crashes hard — in release/beta too, not just debug. A TestFlight build + /// and the App Store build share the same entitlements, so the crash + /// surfaces on the first launch of any affected build and blocks it from + /// ever being promoted to production. + /// + /// Crashes only on the precise `errSecMissingEntitlement` signal (not on + /// lock-state or not-found errors), so a correctly configured build cannot + /// be falsely bricked. No-op on the Simulator, which does not enforce + /// access-group entitlements. + public static func requireDeclaredAccessGroupsAreEntitled() { + #if !targetEnvironment(simulator) + let settings = BuildSettings.current + var groups = [settings.appKeychainAccessGroup] + if let sharedGroup = settings.sharedKeychainAccessGroup { + groups.append(sharedGroup) + } + for group in groups where !isAccessGroupEntitled(group) { + fatalError( + """ + Keychain access group '\(group)' is declared (Info.plist / BuildSettings) but is not \ + granted by the app's keychain-access-groups entitlement. Every keychain operation against \ + it fails with errSecMissingEntitlement — login cannot persist and logout can leak tokens \ + into the cross-app group. Fix the target's .entitlements before release. + """ + ) + } + #endif + } + + /// Whether the signed entitlements grant access to `accessGroup`. Issues a + /// benign scoped query: the keychain rejects an un-entitled group with + /// `errSecMissingEntitlement`. Any other status (a hit, `errSecItemNotFound` + /// on an empty group, or a transient lock-state error) means the + /// access-group filter was accepted, i.e. the group is entitled. + private static func isAccessGroupEntitled(_ accessGroup: String) -> Bool { + let query: [CFString: Any] = [ + kSecClass: kSecClassGenericPassword, + kSecAttrAccessGroup: accessGroup, + kSecMatchLimit: kSecMatchLimitOne + ] + return SecItemCopyMatching(query as CFDictionary, nil) != errSecMissingEntitlement + } + public func getPassword(for username: String, serviceName: String) throws -> String { do { return try keychainUtils.getPasswordForUsername( @@ -73,30 +127,19 @@ public final class AppKeychain: KeychainAccessible { public func setPassword(for username: String, to newValue: String?, serviceName: String) throws { guard let newValue else { - // Delete from both groups, attempting the private-group delete even - // when the shared-group delete fails. A logout must not leave a - // credential in the shared group, where the fallback read (and - // read-repair) could otherwise resurrect it. Delete the shared group - // first: it is the only group the fallback reads, so an interruption - // between the two deletes can never leave the resurrectable - // "private empty, shared present" state. Surface the first real - // failure only after both deletes have been attempted. - var deleteFailure: Error? + // A logout must never end with the credential present in the shared + // group but absent from the private group: the fallback read would + // resurrect it (and read-repair it back into the private group). + // Delete the shared group first and let a real failure propagate + // before the private copy is touched. Deleting shared-first also + // keeps an interruption between the two deletes safe. Worst case on a + // real failure is that both copies remain — logout did not complete + // and the caller retries — never the resurrectable "private empty, + // shared present" state. if let sharedGroup { - do { - try deleteIgnoringNotFound(username, serviceName: serviceName, accessGroup: sharedGroup) - } catch { - deleteFailure = error - } - } - do { - try deleteIgnoringNotFound(username, serviceName: serviceName, accessGroup: privateGroup) - } catch { - deleteFailure = deleteFailure ?? error - } - if let deleteFailure { - throw deleteFailure + try deleteIgnoringNotFound(username, serviceName: serviceName, accessGroup: sharedGroup) } + try deleteIgnoringNotFound(username, serviceName: serviceName, accessGroup: privateGroup) return } try keychainUtils.storeUsername( diff --git a/Modules/Tests/WordPressSharedTests/AppKeychainTests.swift b/Modules/Tests/WordPressSharedTests/AppKeychainTests.swift index 9361de7a2117..aa917a12fa40 100644 --- a/Modules/Tests/WordPressSharedTests/AppKeychainTests.swift +++ b/Modules/Tests/WordPressSharedTests/AppKeychainTests.swift @@ -138,5 +138,29 @@ extension KeychainStubSuites { #expect(value == "shared-pw") #expect(KeychainStub.password(group: privateGroup, service: "svc", username: "user") == nil) } + + @Test func sharedDeleteFailureMustNotEmptyPrivateGroup() { + // Logout where the shared-group delete hits a real failure while the + // private-group delete would succeed. The credential must NOT be left + // in the resurrectable "private empty, shared present" state: a later + // getPassword would miss the private group, fall back to the still- + // present shared copy, and read-repair the logged-out credential back + // into the private group. + KeychainStub.seed(group: privateGroup, service: "svc", username: "user", password: "pw") + KeychainStub.seed(group: sharedGroup, service: "svc", username: "user", password: "pw") + KeychainStub.deleteErrors[sharedGroup] = NSError( + domain: sfhfKeychainErrorDomain, + code: Int(errSecInteractionNotAllowed) + ) + + #expect(throws: (any Error).self) { + try makeKeychain().setPassword(for: "user", to: nil, serviceName: "svc") + } + // The shared delete failed, so the shared copy survives. The private + // copy must therefore also survive — otherwise the fallback read + // resurrects the credential. (Before the fix, the private delete ran + // and emptied the private group, leaving the resurrectable state.) + #expect(KeychainStub.password(group: privateGroup, service: "svc", username: "user") == "pw") + } } } diff --git a/WordPress/Classes/System/WordPressAppDelegate.swift b/WordPress/Classes/System/WordPressAppDelegate.swift index 2d313ec3b604..9e3c54b621e4 100644 --- a/WordPress/Classes/System/WordPressAppDelegate.swift +++ b/WordPress/Classes/System/WordPressAppDelegate.swift @@ -112,6 +112,13 @@ public class WordPressAppDelegate: UIResponder, UIApplicationDelegate { // Start CrashLogging as soon as possible (in case a crash happens during startup) try? loggingStack.start() + // Fail fast on a keychain entitlement misconfiguration. A declared + // access group that isn't actually granted breaks login (the token + // can't be persisted or read) and can leak tokens into the cross-app + // group on logout. Crash here — before any keychain access — so an + // affected build can't get past beta. No-op on the Simulator. + AppKeychain.requireDeclaredAccessGroupsAreEntitled() + // Configure WPCom API overrides configureWordPressComApi()