From 3a3ffde3dd25cf4bca4d3c16936bbd845f401867 Mon Sep 17 00:00:00 2001 From: Max Radermacher Date: Tue, 26 May 2026 18:24:40 -0500 Subject: [PATCH] Remove indirection for some methods --- .../Provisioning/ProvisioningCoordinatorImpl.swift | 7 +++++-- .../UserInterface/ProvisioningController.swift | 1 + .../RegistrationCoordinatorDependencies.swift | 2 ++ .../Registration/RegistrationCoordinatorImpl.swift | 2 +- .../Provisioning/ProvisioningCoordinatorTest.swift | 1 + .../Registration/RegistrationCoordinatorTest.swift | 1 + SignalServiceKit/Account/MasterKeySyncManager.swift | 4 +++- .../Environment/DependenciesBridge.swift | 2 +- .../Megaphones/ExperienceUpgradeManifest.swift | 9 ++++++--- .../KeyBackupService/SecureValueRecoveryMock.swift | 10 ---------- .../AccountAttributesUpdaterImpl.swift | 2 +- .../SecureValueRecovery/SVRLocalStorage.swift | 6 +++--- .../SecureValueRecovery/SecureValueRecovery.swift | 6 ------ .../SecureValueRecovery2Impl.swift | 12 +----------- .../Storage/Database/GRDBSchemaMigrator.swift | 3 ++- 15 files changed, 28 insertions(+), 40 deletions(-) diff --git a/Signal/Provisioning/ProvisioningCoordinatorImpl.swift b/Signal/Provisioning/ProvisioningCoordinatorImpl.swift index 71d20cc3bb..454dd859d2 100644 --- a/Signal/Provisioning/ProvisioningCoordinatorImpl.swift +++ b/Signal/Provisioning/ProvisioningCoordinatorImpl.swift @@ -25,6 +25,7 @@ class ProvisioningCoordinatorImpl: ProvisioningCoordinator { private let signalService: OWSSignalServiceProtocol private let storageServiceManager: StorageServiceManager private let svr: SecureValueRecovery + private let svrLocalStorage: SVRLocalStorage private let syncManager: SyncManagerProtocol private let threadStore: ThreadStore private let tsAccountManager: TSAccountManager @@ -47,6 +48,7 @@ class ProvisioningCoordinatorImpl: ProvisioningCoordinator { signalService: OWSSignalServiceProtocol, storageServiceManager: StorageServiceManager, svr: SecureValueRecovery, + svrLocalStorage: SVRLocalStorage, syncManager: SyncManagerProtocol, threadStore: ThreadStore, tsAccountManager: TSAccountManager, @@ -68,6 +70,7 @@ class ProvisioningCoordinatorImpl: ProvisioningCoordinator { self.signalService = signalService self.storageServiceManager = storageServiceManager self.svr = svr + self.svrLocalStorage = svrLocalStorage self.syncManager = syncManager self.threadStore = threadStore self.tsAccountManager = tsAccountManager @@ -479,7 +482,7 @@ class ProvisioningCoordinatorImpl: ProvisioningCoordinator { didLinkNSync: Bool, ) async throws(CompleteProvisioningError) { let hasBackedUpMasterKey = self.db.read { tx in - self.svr.hasBackedUpMasterKey(transaction: tx) + self.svrLocalStorage.isMasterKeyBackedUp(tx: tx) } let capabilities = AccountAttributes.Capabilities(hasSVRBackups: hasBackedUpMasterKey) do { @@ -702,7 +705,7 @@ class ProvisioningCoordinatorImpl: ProvisioningCoordinator { let phoneNumberDiscoverability = tsAccountManager.phoneNumberDiscoverability(tx: tx) - let hasSVRBackups = svr.hasBackedUpMasterKey(transaction: tx) + let hasSVRBackups = svrLocalStorage.isMasterKeyBackedUp(tx: tx) return AccountAttributes( isManualMessageFetchEnabled: isManualMessageFetchEnabled, diff --git a/Signal/Provisioning/UserInterface/ProvisioningController.swift b/Signal/Provisioning/UserInterface/ProvisioningController.swift index e8a007ac95..009bef8774 100644 --- a/Signal/Provisioning/UserInterface/ProvisioningController.swift +++ b/Signal/Provisioning/UserInterface/ProvisioningController.swift @@ -51,6 +51,7 @@ class ProvisioningController: NSObject { signalService: SSKEnvironment.shared.signalServiceRef, storageServiceManager: SSKEnvironment.shared.storageServiceManagerRef, svr: DependenciesBridge.shared.svr, + svrLocalStorage: DependenciesBridge.shared.svrLocalStorage, syncManager: SSKEnvironment.shared.syncManagerRef, threadStore: ThreadStoreImpl(), tsAccountManager: DependenciesBridge.shared.tsAccountManager, diff --git a/Signal/Registration/RegistrationCoordinatorDependencies.swift b/Signal/Registration/RegistrationCoordinatorDependencies.swift index 134105d1f2..834822372c 100644 --- a/Signal/Registration/RegistrationCoordinatorDependencies.swift +++ b/Signal/Registration/RegistrationCoordinatorDependencies.swift @@ -41,6 +41,7 @@ public struct RegistrationCoordinatorDependencies { public let signalService: OWSSignalServiceProtocol public let storageServiceManager: RegistrationCoordinatorImpl.Shims.StorageServiceManager public let svr: SecureValueRecovery + public let svrLocalStorage: SVRLocalStorage public let svrAuthCredentialStore: SVRAuthCredentialStorage public let timeoutProvider: RegistrationCoordinatorImpl.Shims.TimeoutProvider public let tsAccountManager: TSAccountManager @@ -88,6 +89,7 @@ public struct RegistrationCoordinatorDependencies { signalService: SSKEnvironment.shared.signalServiceRef, storageServiceManager: RegistrationCoordinatorImpl.Wrappers.StorageServiceManager(SSKEnvironment.shared.storageServiceManagerRef), svr: DependenciesBridge.shared.svr, + svrLocalStorage: DependenciesBridge.shared.svrLocalStorage, svrAuthCredentialStore: DependenciesBridge.shared.svrCredentialStorage, timeoutProvider: RegistrationCoordinatorImpl.Wrappers.TimeoutProvider(), tsAccountManager: DependenciesBridge.shared.tsAccountManager, diff --git a/Signal/Registration/RegistrationCoordinatorImpl.swift b/Signal/Registration/RegistrationCoordinatorImpl.swift index ca184b59cd..341fbcf0a1 100644 --- a/Signal/Registration/RegistrationCoordinatorImpl.swift +++ b/Signal/Registration/RegistrationCoordinatorImpl.swift @@ -2313,7 +2313,7 @@ public class RegistrationCoordinatorImpl: RegistrationCoordinator { // If we have a local master key, theres no need to restore after registration. // (we will still back up though) inMemoryState.shouldRestoreSVRMasterKeyAfterRegistration = localMasterKey == nil - inMemoryState.didHaveSVRBackupsPriorToReg = deps.svr.hasBackedUpMasterKey(transaction: tx) + inMemoryState.didHaveSVRBackupsPriorToReg = deps.svrLocalStorage.isMasterKeyBackedUp(tx: tx) } // MARK: - SVR Auth Credential Candidates Pathway diff --git a/Signal/test/Provisioning/ProvisioningCoordinatorTest.swift b/Signal/test/Provisioning/ProvisioningCoordinatorTest.swift index cfe3968a83..0cf2616fef 100644 --- a/Signal/test/Provisioning/ProvisioningCoordinatorTest.swift +++ b/Signal/test/Provisioning/ProvisioningCoordinatorTest.swift @@ -90,6 +90,7 @@ public class ProvisioningCoordinatorTest: XCTestCase { signalService: signalServiceMock, storageServiceManager: storageServiceManagerMock, svr: svrMock, + svrLocalStorage: SVRLocalStorage(), syncManager: syncManagerMock, threadStore: threadStoreMock, tsAccountManager: tsAccountManagerMock, diff --git a/Signal/test/Registration/RegistrationCoordinatorTest.swift b/Signal/test/Registration/RegistrationCoordinatorTest.swift index 9e27d17423..d810711961 100644 --- a/Signal/test/Registration/RegistrationCoordinatorTest.swift +++ b/Signal/test/Registration/RegistrationCoordinatorTest.swift @@ -160,6 +160,7 @@ public class RegistrationCoordinatorTest { signalService: mockSignalService, storageServiceManager: storageServiceManagerMock, svr: svr, + svrLocalStorage: SVRLocalStorage(), svrAuthCredentialStore: svrAuthCredentialStore, timeoutProvider: timeoutProviderMock, tsAccountManager: tsAccountManagerMock, diff --git a/SignalServiceKit/Account/MasterKeySyncManager.swift b/SignalServiceKit/Account/MasterKeySyncManager.swift index 7fea54b0c2..759f3d19b5 100644 --- a/SignalServiceKit/Account/MasterKeySyncManager.swift +++ b/SignalServiceKit/Account/MasterKeySyncManager.swift @@ -80,7 +80,9 @@ class MasterKeySyncManagerImpl: MasterKeySyncManager { } private func runStartupJobsForLinkedDevice(tx: DBWriteTransaction) { - if svr.hasMasterKey(transaction: tx) { + let accountKeyStore = DependenciesBridge.shared.accountKeyStore + + if accountKeyStore.getMasterKey(tx: tx) != nil { // No need to sync; we have the master key. return } diff --git a/SignalServiceKit/Environment/DependenciesBridge.swift b/SignalServiceKit/Environment/DependenciesBridge.swift index 3dffed98f0..853385a380 100644 --- a/SignalServiceKit/Environment/DependenciesBridge.swift +++ b/SignalServiceKit/Environment/DependenciesBridge.swift @@ -175,7 +175,7 @@ public class DependenciesBridge { public let storyRecipientManager: StoryRecipientManager public let storyRecipientStore: StoryRecipientStore public let subscriptionConfigManager: SubscriptionConfigManager - let svrLocalStorage: SVRLocalStorage + public let svrLocalStorage: SVRLocalStorage public let threadAssociatedDataStore: ThreadAssociatedDataStore public let threadRemover: ThreadRemover public let threadReplyInfoStore: ThreadReplyInfoStore diff --git a/SignalServiceKit/Megaphones/ExperienceUpgradeManifest.swift b/SignalServiceKit/Megaphones/ExperienceUpgradeManifest.swift index d85da8b54b..3f37aaf038 100644 --- a/SignalServiceKit/Megaphones/ExperienceUpgradeManifest.swift +++ b/SignalServiceKit/Megaphones/ExperienceUpgradeManifest.swift @@ -530,10 +530,13 @@ public enum ExperienceUpgradeManifest: Codable, Equatable, Hashable { public static func checkPreconditionsForIntroducingPins(transaction: DBReadTransaction) -> Bool { // The PIN setup flow requires an internet connection and you to not already have a PIN + let accountKeyStore = DependenciesBridge.shared.accountKeyStore + let tsAccountManager = DependenciesBridge.shared.tsAccountManager + let reachabilityManager = SSKEnvironment.shared.reachabilityManagerRef if - SSKEnvironment.shared.reachabilityManagerRef.isReachable, - DependenciesBridge.shared.tsAccountManager.registrationState(tx: transaction).isRegisteredPrimaryDevice, - !DependenciesBridge.shared.svr.hasMasterKey(transaction: transaction) + reachabilityManager.isReachable, + tsAccountManager.registrationState(tx: transaction).isRegisteredPrimaryDevice, + accountKeyStore.getMasterKey(tx: transaction) == nil { return true } diff --git a/SignalServiceKit/Mocks/KeyBackupService/SecureValueRecoveryMock.swift b/SignalServiceKit/Mocks/KeyBackupService/SecureValueRecoveryMock.swift index 5ef54bfb19..5d91ac6906 100644 --- a/SignalServiceKit/Mocks/KeyBackupService/SecureValueRecoveryMock.swift +++ b/SignalServiceKit/Mocks/KeyBackupService/SecureValueRecoveryMock.swift @@ -17,16 +17,6 @@ public class SecureValueRecoveryMock: SecureValueRecovery { public func refreshCredentialsIfNecessary() async throws { } - public var hasBackedUpMasterKey: Bool = false - - public func hasBackedUpMasterKey(transaction: DBReadTransaction) -> Bool { - return hasBackedUpMasterKey - } - - public func hasMasterKey(transaction: DBReadTransaction) -> Bool { - return SVRLocalStorage().getIsMasterKeyBackedUp(transaction) - } - public var reglockToken: String? public var backupMasterKeyMock: ((_ pin: String, _ masterKey: MasterKey, _ force: Bool, _ authMethod: SVR.AuthMethod) -> Promise)? diff --git a/SignalServiceKit/Network/API/Requests/AccountAttributes/AccountAttributesUpdaterImpl.swift b/SignalServiceKit/Network/API/Requests/AccountAttributes/AccountAttributesUpdaterImpl.swift index a91215586c..dec4faa1e4 100644 --- a/SignalServiceKit/Network/API/Requests/AccountAttributes/AccountAttributesUpdaterImpl.swift +++ b/SignalServiceKit/Network/API/Requests/AccountAttributes/AccountAttributesUpdaterImpl.swift @@ -140,7 +140,7 @@ public class AccountAttributesUpdaterImpl: AccountAttributesUpdater { } // has non-nil value if isRegistered is true. - let hasBackedUpMasterKey = self.svrLocalStorage.getIsMasterKeyBackedUp(tx) + let hasBackedUpMasterKey = self.svrLocalStorage.isMasterKeyBackedUp(tx: tx) let capabilities = AccountAttributes.Capabilities(hasSVRBackups: hasBackedUpMasterKey) let lastAttributeRequestToken = self.kvStore.getData(Keys.latestUpdateRequestToken, transaction: tx) diff --git a/SignalServiceKit/SecureValueRecovery/SVRLocalStorage.swift b/SignalServiceKit/SecureValueRecovery/SVRLocalStorage.swift index 20e7f90aff..9f8e3b82a4 100644 --- a/SignalServiceKit/SecureValueRecovery/SVRLocalStorage.swift +++ b/SignalServiceKit/SecureValueRecovery/SVRLocalStorage.swift @@ -6,10 +6,10 @@ import Foundation /// Stores state related to SVR; e.g. do we have backups at all, etc. -struct SVRLocalStorage { +public struct SVRLocalStorage { let completedBackupStore = KeyValueStore(collection: "SVR.Completed") - func getIsMasterKeyBackedUp(_ transaction: DBReadTransaction) -> Bool { - return !completedBackupStore.allKeys(transaction: transaction).isEmpty + public func isMasterKeyBackedUp(tx: DBReadTransaction) -> Bool { + return !completedBackupStore.allKeys(transaction: tx).isEmpty } } diff --git a/SignalServiceKit/SecureValueRecovery/SecureValueRecovery.swift b/SignalServiceKit/SecureValueRecovery/SecureValueRecovery.swift index 2dd228c1d5..70da9d1a72 100644 --- a/SignalServiceKit/SecureValueRecovery/SecureValueRecovery.swift +++ b/SignalServiceKit/SecureValueRecovery/SecureValueRecovery.swift @@ -103,12 +103,6 @@ public protocol SecureValueRecovery { /// Performs periodic credential refresh to help with re-registration. func refreshCredentialsIfNecessary() async throws - /// Indicates whether or not we have a master key locally - func hasMasterKey(transaction: DBReadTransaction) -> Bool - - /// Indicates whether or not we have a master key stored in SVR - func hasBackedUpMasterKey(transaction: DBReadTransaction) -> Bool - /// Loads the users key, if any, from the SVR into the database. func restoreKeys(pin: String, authMethod: SVR.AuthMethod) async -> SVR.RestoreKeysResult diff --git a/SignalServiceKit/SecureValueRecovery/SecureValueRecovery2Impl.swift b/SignalServiceKit/SecureValueRecovery/SecureValueRecovery2Impl.swift index ffbb8bfbcf..1d4b0075e1 100644 --- a/SignalServiceKit/SecureValueRecovery/SecureValueRecovery2Impl.swift +++ b/SignalServiceKit/SecureValueRecovery/SecureValueRecovery2Impl.swift @@ -44,7 +44,7 @@ public class SecureValueRecovery2Impl: SecureValueRecovery { // MARK: - Periodic Backups public func refreshCredentialsIfNecessary() async throws { - let hasBackedUp = self.db.read { tx in self.hasBackedUpMasterKey(transaction: tx) } + let hasBackedUp = self.db.read { tx in self.localStorage.isMasterKeyBackedUp(tx: tx) } guard hasBackedUp else { // If we've never backed up, don't refresh periodically. (If we eventually // perform a backup, we'll cache those credential after fetching them.) @@ -61,16 +61,6 @@ public class SecureValueRecovery2Impl: SecureValueRecovery { } } - // MARK: - Key Existence - - public func hasMasterKey(transaction: DBReadTransaction) -> Bool { - return accountKeyStore.getMasterKey(tx: transaction) != nil - } - - public func hasBackedUpMasterKey(transaction: DBReadTransaction) -> Bool { - return localStorage.getIsMasterKeyBackedUp(transaction) - } - // MARK: - Key Management private let backupQueue = ConcurrentTaskQueue(concurrentLimit: 1) diff --git a/SignalServiceKit/Storage/Database/GRDBSchemaMigrator.swift b/SignalServiceKit/Storage/Database/GRDBSchemaMigrator.swift index 1cfe9b31b2..a923b966aa 100644 --- a/SignalServiceKit/Storage/Database/GRDBSchemaMigrator.swift +++ b/SignalServiceKit/Storage/Database/GRDBSchemaMigrator.swift @@ -5181,7 +5181,8 @@ public class GRDBSchemaMigrator { // migration, we want the crash logs reflect where it occurred. migrator.registerMigration(.dataMigration_enableV2RegistrationLockIfNecessary) { transaction in - if DependenciesBridge.shared.svr.hasMasterKey(transaction: transaction) { + let accountKeyStore = DependenciesBridge.shared.accountKeyStore + if accountKeyStore.getMasterKey(tx: transaction) != nil { KeyValueStore(collection: "kOWS2FAManager_Collection") .setBool(true, key: "isRegistrationLockV2Enabled", transaction: transaction) }