bitwarden/ios
1
0
Fork 0
mirror of https://github.com/bitwarden/ios.git synced 2025-12-10 17:46:07 -06:00
Code Issues Packages Projects Releases 32 Wiki Activity

BIT-2316: Fix vault timeout never lock migration (#639)

Browse Source
This commit is contained in:
Matt Czech 2024-05-14 11:24:51 -05:00 committed by GitHub
parent eb275cb716
commit 8976c485b5
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
13 changed files with 218 additions and 25 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
Bitwarden/Application/Support/Info.plist
View File

@ -4,6 +4,8 @@
<dict>
<key>BitwardenAppIdentifier</key>
<string>$(BASE_BUNDLE_ID)</string>
<key>BitwardenKeychainAccessGroup</key>
<string>$(AppIdentifierPrefix)$(BASE_BUNDLE_ID)</string>
<key>CADisableMinimumFrameDurationOnPhone</key>
<true/>
<key>CFBundleDevelopmentRegion</key>

2
BitwardenActionExtension/Application/Support/Info.plist
View File

@ -4,6 +4,8 @@
<dict>
<key>BitwardenAppIdentifier</key>
<string>$(BASE_BUNDLE_ID)</string>
<key>BitwardenKeychainAccessGroup</key>
<string>$(AppIdentifierPrefix)$(BASE_BUNDLE_ID)</string>
<key>CFBundleDisplayName</key>
<string>Autofill with Bitwarden</string>
<key>CFBundleName</key>

2
BitwardenAutoFillExtension/Application/Support/Info.plist
View File

@ -4,6 +4,8 @@
<dict>
<key>BitwardenAppIdentifier</key>
<string>$(BASE_BUNDLE_ID)</string>
<key>BitwardenKeychainAccessGroup</key>
<string>$(AppIdentifierPrefix)$(BASE_BUNDLE_ID)</string>
<key>CFBundleDisplayName</key>
<string>Bitwarden</string>
<key>CFBundleName</key>

2
BitwardenShareExtension/Application/Support/Info.plist
View File

@ -4,6 +4,8 @@
<dict>
<key>BitwardenAppIdentifier</key>
<string>$(BASE_BUNDLE_ID)</string>
<key>BitwardenKeychainAccessGroup</key>
<string>$(AppIdentifierPrefix)$(BASE_BUNDLE_ID)</string>
<key>CFBundleDisplayName</key>
<string>Bitwarden Share</string>
<key>CFBundleName</key>

12
BitwardenShared/Core/Auth/Services/KeychainRepository.swift
View File

@ -183,18 +183,20 @@ class DefaultKeychainRepository: KeychainRepository {
///
let appIdService: AppIdService
/// An identifier for this application and extensions.
/// ie: "LTZ2PFU5D6.com.8bit.bitwarden"
/// An identifier for the keychain service used by the application and extensions.
///
/// Example: "com.8bit.bitwarden".
///
var appSecAttrService: String {
Bundle.main.appIdentifier
}
/// An identifier for this application group and extensions
/// ie: "group.LTZ2PFU5D6.com.8bit.bitwarden"
/// An identifier for the keychain access group used by the application group and extensions.
///
/// Example: "LTZ2PFU5D6.com.8bit.bitwarden"
///
var appSecAttrAccessGroup: String {
Bundle.main.groupIdentifier
Bundle.main.keychainAccessGroup
}
/// The keychain service used by the repository

2
BitwardenShared/Core/Auth/Services/KeychainRepositoryTests.swift
View File

@ -51,7 +51,7 @@ final class KeychainRepositoryTests: BitwardenTestCase { // swiftlint:disable:th
///
func test_appSecAttrAccessGroup() {
XCTAssertEqual(
Bundle.main.groupIdentifier,
Bundle.main.keychainAccessGroup,
subject.appSecAttrAccessGroup
)
}

5
BitwardenShared/Core/Platform/Extensions/Bundle+Extensions.swift
View File

@ -32,4 +32,9 @@ extension Bundle {
var groupIdentifier: String {
"group." + appIdentifier
}
/// Return's the app's access group identifier for storing keychain items.
var keychainAccessGroup: String {
infoDictionary?["BitwardenKeychainAccessGroup"] as? String ?? appIdentifier
}
}

96
BitwardenShared/Core/Platform/Services/MigrationService.swift
View File

@ -26,6 +26,9 @@ class DefaultMigrationService {
/// The repository used to manage keychain items.
let keychainRepository: KeychainRepository
/// The service name associated with the app's keychain items.
let keychainServiceName: String
/// The shared UserDefaults instance (NOTE: this should be the standard one just for the app,
/// not one in the app group).
let standardUserDefaults: UserDefaults
@ -38,17 +41,20 @@ class DefaultMigrationService {
/// - appSettingsStore: The service used by the application to persist app setting values.
/// - errorReporter: The service used by the application to report non-fatal errors.
/// - keychainRepository: The repository used to manage keychain items.
/// - keychainServiceName: The service name associated with the app's keychain items.
/// - standardUserDefaults: The shared UserDefaults instance.
///
init(
appSettingsStore: AppSettingsStore,
errorReporter: ErrorReporter,
keychainRepository: KeychainRepository,
keychainServiceName: String = Bundle.main.appIdentifier,
standardUserDefaults: UserDefaults = .standard
) {
self.appSettingsStore = appSettingsStore
self.errorReporter = errorReporter
self.keychainRepository = keychainRepository
self.keychainServiceName = keychainServiceName
self.standardUserDefaults = standardUserDefaults
}
@ -90,6 +96,80 @@ class DefaultMigrationService {
try await keychainRepository.setRefreshToken(tokens.refreshToken, userId: accountId)
}
}
/// Performs migration 2.
///
/// Notes:
/// - Migrate Keychain items, migrating data in kSecAttrGeneric to kSecValueData.
///
private func performMigration2() async throws {
let query = [
kSecClass: kSecClassGenericPassword,
kSecMatchLimit: kSecMatchLimitAll,
kSecAttrService: keychainServiceName,
kSecReturnData: true,
kSecReturnAttributes: true,
] as CFDictionary
var keychainItems: AnyObject?
let status = SecItemCopyMatching(query, &keychainItems)
guard status == errSecSuccess else {
Logger.application.error("Error searching for keychain items: \(status)")
return
}
if let keychainItems = keychainItems as? NSArray {
for keychainItem in keychainItems {
guard let itemDictionary = keychainItem as? NSDictionary else { continue }
let query = [
kSecClass: kSecClassGenericPassword,
kSecAttrAccount: itemDictionary[kSecAttrAccount],
kSecAttrService: keychainServiceName,
] as CFDictionary
var attributesToUpdate: [CFString: Any] = [
kSecAttrAccessGroup: Bundle.main.keychainAccessGroup,
]
if let genericData = itemDictionary[kSecAttrGeneric] as? Data,
!genericData.isEmpty,
itemDictionary[kSecValueData] == nil {
// Migrate data from kSecAttrGeneric to kSecValueData.
attributesToUpdate[kSecValueData] = genericData
attributesToUpdate[kSecAttrGeneric] = Data()
}
let status = SecItemUpdate(query, attributesToUpdate as CFDictionary)
guard status == errSecSuccess else {
Logger.application.error("Error updating keychain item: \(status)")
continue
}
}
}
}
}
extension DefaultMigrationService {
/// The list of migrations that can be performed.
var migrations: [() async throws -> Void] {
[
performMigration1,
performMigration2,
]
}
/// Performs a single migration for a migration version number.
///
/// - Note: `performMigrations()` should be used in almost all cases to perform the full set of
/// migrations. This exists to allow tests to perform a single migration.
///
/// - Parameter version: The migration version to perform.
///
func performMigration(version: Int) async throws {
let migrationIndex = version - 1
guard migrationIndex >= 0, migrationIndex < migrations.count else { return }
try await migrations[migrationIndex]()
appSettingsStore.migrationVersion = version
}
}
extension DefaultMigrationService: MigrationService {
@ -97,16 +177,14 @@ extension DefaultMigrationService: MigrationService {
var migrationVersion = appSettingsStore.migrationVersion
defer { appSettingsStore.migrationVersion = migrationVersion }
// The list of migrations that can be performed.
let migrations: [(version: Int, method: () async throws -> Void)] = [
(1, performMigration1),
]
do {
for migration in migrations where migrationVersion < migration.version {
try await migration.method()
migrationVersion = migration.version
Logger.application.info("Completed data migration \(migration.version)")
for (migrationIndex, migration) in migrations.enumerated() {
let version = migrationIndex + 1
guard migrationVersion < version else { continue }
try await migration()
migrationVersion = version
Logger.application.info("Completed data migration \(version)")
}
} catch {
errorReporter.log(error: error)

69
BitwardenShared/Core/Platform/Services/MigrationServiceTests.swift
View File

@ -22,11 +22,13 @@ class MigrationServiceTests: BitwardenTestCase {
standardUserDefaults = UserDefaults(suiteName: "test")
standardUserDefaults.removeObject(forKey: "MSAppCenterCrashesIsEnabled")
SecItemDelete([kSecClass: kSecClassGenericPassword] as CFDictionary)
subject = DefaultMigrationService(
appSettingsStore: appSettingsStore,
errorReporter: errorReporter,
keychainRepository: keychainRepository,
keychainServiceName: "com.bitwarden.test",
standardUserDefaults: standardUserDefaults
)
}
@ -43,6 +45,15 @@ class MigrationServiceTests: BitwardenTestCase {
// MARK: Tests
/// `performMigrations()` performs all migrations and updates the migration version.
func test_performMigrations() async throws {
appSettingsStore.migrationVersion = 0
await subject.performMigrations()
XCTAssertEqual(appSettingsStore.migrationVersion, subject.migrations.count)
}
/// `performMigrations()` logs an error to the error reporter if one occurs.
func test_performMigrations_error() async throws {
appSettingsStore.migrationVersion = 0
@ -91,7 +102,7 @@ class MigrationServiceTests: BitwardenTestCase {
appSettingsStore.notificationsLastRegistrationDates[userId] = Date()
}
await subject.performMigrations()
try await subject.performMigration(version: 1)
XCTAssertEqual(appSettingsStore.migrationVersion, 1)
@ -121,7 +132,7 @@ class MigrationServiceTests: BitwardenTestCase {
func test_performMigrations_1_withAppCenterCrashesKey_false() async throws {
appSettingsStore.migrationVersion = 0
standardUserDefaults.setValue(false, forKey: "MSAppCenterCrashesIsEnabled")
await subject.performMigrations()
try await subject.performMigration(version: 1)
XCTAssertFalse(errorReporter.isEnabled)
}
@ -130,7 +141,7 @@ class MigrationServiceTests: BitwardenTestCase {
func test_performMigrations_1_withAppCenterCrashesKey_true() async throws {
appSettingsStore.migrationVersion = 0
standardUserDefaults.setValue(true, forKey: "MSAppCenterCrashesIsEnabled")
await subject.performMigrations()
try await subject.performMigration(version: 1)
XCTAssertTrue(errorReporter.isEnabled)
}
@ -139,11 +150,61 @@ class MigrationServiceTests: BitwardenTestCase {
appSettingsStore.migrationVersion = 0
appSettingsStore.state = nil
await subject.performMigrations()
try await subject.performMigration(version: 1)
XCTAssertEqual(appSettingsStore.migrationVersion, 1)
XCTAssertNil(appSettingsStore.state)
XCTAssertTrue(keychainRepository.deleteAllItemsCalled)
XCTAssertTrue(errorReporter.isEnabled)
}
/// `performMigrations()` for migration 2 migrates keychain data in kSecAttrGeneric to kSecValueData.
func test_performMigrations_2() async throws {
let itemsToAdd: [(account: String, value: String)] = [
("TEST_ACCOUNT_1", "secret"),
("TEST_ACCOUNT_2", "password"),
]
for item in itemsToAdd {
SecItemAdd(
[
kSecClass: kSecClassGenericPassword,
kSecAttrAccount: item.account,
kSecAttrService: "com.bitwarden.test",
kSecAttrGeneric: Data(item.value.utf8),
] as CFDictionary,
nil
)
}
try await subject.performMigration(version: 2)
var copyResult: AnyObject?
SecItemCopyMatching(
[
kSecClass: kSecClassGenericPassword,
kSecAttrService: "com.bitwarden.test",
kSecMatchLimit: kSecMatchLimitAll,
kSecReturnData: true,
kSecReturnAttributes: true,
] as CFDictionary,
&copyResult
)
let keychainItems = try XCTUnwrap(copyResult as? [[CFString: Any]])
XCTAssertEqual(keychainItems.count, 2)
let item1 = try XCTUnwrap(keychainItems[0])
XCTAssertEqual(item1[kSecAttrAccessGroup] as? String, Bundle.main.keychainAccessGroup)
XCTAssertEqual(item1[kSecAttrAccount] as? String, "TEST_ACCOUNT_1")
XCTAssertEqual(item1[kSecAttrGeneric] as? Data, Data())
XCTAssertEqual(item1[kSecValueData] as? Data, Data("secret".utf8))
let item2 = try XCTUnwrap(keychainItems[1])
XCTAssertEqual(item2[kSecAttrAccessGroup] as? String, Bundle.main.keychainAccessGroup)
XCTAssertEqual(item2[kSecAttrAccount] as? String, "TEST_ACCOUNT_2")
XCTAssertEqual(item2[kSecAttrGeneric] as? Data, Data())
XCTAssertEqual(item2[kSecValueData] as? Data, Data("password".utf8))
XCTAssertEqual(appSettingsStore.migrationVersion, 2)
}
}

6
BitwardenShared/Core/Platform/Services/ServiceContainer.swift
View File

@ -276,7 +276,11 @@ public class ServiceContainer: Services { // swiftlint:disable:this type_body_le
)
let timeProvider = CurrentTime()
let stateService = DefaultStateService(appSettingsStore: appSettingsStore, dataStore: dataStore)
let stateService = DefaultStateService(
appSettingsStore: appSettingsStore,
dataStore: dataStore,
keychainRepository: keychainRepository
)
let clientBuilder = DefaultClientBuilder(errorReporter: errorReporter)
let clientService = DefaultClientService(

11
BitwardenShared/Core/Platform/Services/StateService.swift
View File

@ -945,6 +945,9 @@ actor DefaultStateService: StateService { // swiftlint:disable:this type_body_le
/// A subject containing the last sync time mapped to user ID.
private var lastSyncTimeByUserIdSubject = CurrentValueSubject<[String: Date], Never>([:])
/// A service used to access data in the keychain.
private let keychainRepository: KeychainRepository
/// A subject containing whether to show the website icons.
private var showWebIconsSubject: CurrentValueSubject<Bool, Never>
@ -955,13 +958,16 @@ actor DefaultStateService: StateService { // swiftlint:disable:this type_body_le
/// - Parameters:
/// - appSettingsStore: The service that persists app settings.
/// - dataStore: The data store that handles performing data requests.
/// - keychainRepository: A service used to access data in the keychain.
///
init(
appSettingsStore: AppSettingsStore,
dataStore: DataStore
dataStore: DataStore,
keychainRepository: KeychainRepository
) {
self.appSettingsStore = appSettingsStore
self.dataStore = dataStore
self.keychainRepository = keychainRepository
appThemeSubject = CurrentValueSubject(AppTheme(appSettingsStore.appTheme))
showWebIconsSubject = CurrentValueSubject(!appSettingsStore.disableWebIcons)
@ -1148,7 +1154,8 @@ actor DefaultStateService: StateService { // swiftlint:disable:this type_body_le
func getVaultTimeout(userId: String?) async throws -> SessionTimeoutValue {
let userId = try getAccount(userId: userId).profile.userId
guard let rawValue = appSettingsStore.vaultTimeout(userId: userId) else {
return .fifteenMinutes
let userAuthKey = try? await keychainRepository.getUserAuthKeyValue(for: .neverLock(userId: userId))
return userAuthKey == nil ? .fifteenMinutes : .never
}
return SessionTimeoutValue(rawValue: rawValue)
}

30
BitwardenShared/Core/Platform/Services/StateServiceTests.swift
View File

@ -8,6 +8,7 @@ class StateServiceTests: BitwardenTestCase { // swiftlint:disable:this type_body
var appSettingsStore: MockAppSettingsStore!
var dataStore: DataStore!
var keychainRepository: MockKeychainRepository!
var subject: DefaultStateService!
// MARK: Setup & Teardown
@ -17,8 +18,13 @@ class StateServiceTests: BitwardenTestCase { // swiftlint:disable:this type_body
appSettingsStore = MockAppSettingsStore()
dataStore = DataStore(errorReporter: MockErrorReporter(), storeType: .memory)
keychainRepository = MockKeychainRepository()
subject = DefaultStateService(appSettingsStore: appSettingsStore, dataStore: dataStore)
subject = DefaultStateService(
appSettingsStore: appSettingsStore,
dataStore: dataStore,
keychainRepository: keychainRepository
)
}
override func tearDown() {
@ -26,6 +32,7 @@ class StateServiceTests: BitwardenTestCase { // swiftlint:disable:this type_body
appSettingsStore = nil
dataStore = nil
keychainRepository = nil
subject = nil
}
@ -663,6 +670,27 @@ class StateServiceTests: BitwardenTestCase { // swiftlint:disable:this type_body
XCTAssertEqual(vaultTimeout, .custom(20))
}
/// `.getVaultTimeout(userId:)` gets the default vault timeout for the user if a value isn't set.
func test_getVaultTimeout_default() async throws {
appSettingsStore.vaultTimeout["1"] = nil
await subject.addAccount(.fixture(profile: .fixture(userId: "1")))
let vaultTimeout = try await subject.getVaultTimeout()
XCTAssertEqual(vaultTimeout, .fifteenMinutes)
}
/// `.getVaultTimeout(userId:)` gets the user's vault timeout when it's set to never lock.
func test_getVaultTimeout_neverLock() async throws {
appSettingsStore.vaultTimeout["1"] = nil
keychainRepository.mockStorage[keychainRepository.formattedKey(for: .neverLock(userId: "1"))] = "NEVER_LOCK_KEY"
await subject.addAccount(.fixture(profile: .fixture(userId: "1")))
let vaultTimeout = try await subject.getVaultTimeout()
XCTAssertEqual(vaultTimeout, .never)
}
/// `lastSyncTimePublisher()` returns a publisher for the user's last sync time.
func test_lastSyncTimePublisher() async throws {
await subject.addAccount(.fixture(profile: .fixture(userId: "1")))

4
BitwardenShared/Core/Platform/Services/Stores/TestHelpers/MockAppSettingsStore.swift
View File

@ -36,7 +36,7 @@ class MockAppSettingsStore: AppSettingsStore {
var shouldTrustDevice = [String: Bool?]()
var timeoutAction = [String: Int]()
var twoFactorTokens = [String: String]()
var vaultTimeout = [String: Int?]()
var vaultTimeout = [String: Int]()
var state: State? {
didSet {
activeIdSubject.send(state?.activeUserId)
@ -229,7 +229,7 @@ class MockAppSettingsStore: AppSettingsStore {
}
func vaultTimeout(userId: String) -> Int? {
vaultTimeout[userId] ?? 0
vaultTimeout[userId]
}
func activeAccountIdPublisher() -> AnyPublisher<String?, Never> {