bitwarden · brant-livefront · Nov 13, 2024 · Nov 8, 2024 · Nov 11, 2024 · Nov 11, 2024
@@ -290,7 +290,7 @@ final class AuthenticatorBridgeItemServiceTests: AuthenticatorBridgeKitTestCase
         XCTAssertEqual(results[0], expectedItems)
     }
 
-    /// Verify that the shared items publisher publishes new lists when items are deleted..
+    /// Verify that the shared items publisher publishes new lists when items are deleted.
     ///
     func test_sharedItemsPublisher_withDeletes() async throws {
         let initialItems = AuthenticatorBridgeItemDataView.fixtures().sorted { $0.id < $1.id }

@@ -25,10 +25,10 @@ enum KeychainItem: Equatable {
     /// The keychain item for a user's refresh token.
     case refreshToken(userId: String)
 
-    /// The `SecAccessControlCreateFlags` protection level for this keychain item.
+    /// The `SecAccessControlCreateFlags` level for this keychain item.
     ///     If `nil`, no extra protection is applied.
     ///
-    var protection: SecAccessControlCreateFlags? {
+    var accessControlFlags: SecAccessControlCreateFlags? {
         switch self {
         case .accessToken,
              .authenticatorVaultKey,
@@ -42,6 +42,21 @@ enum KeychainItem: Equatable {
         }
     }
 
+    /// The protection level for this keychain item.
+    var protection: CFTypeRef {
+        switch self {
+        case .biometrics,
+             .deviceKey,
+             .neverLock,
+             .pendingAdminLoginRequest:
+            kSecAttrAccessibleWhenUnlockedThisDeviceOnly
+        case .accessToken,
+             .authenticatorVaultKey,
+             .refreshToken:
+            kSecAttrAccessibleAfterFirstUnlockThisDeviceOnly
+        }
+    }
+
     /// The storage key for this keychain item.
     ///
     var unformattedKey: String {
@@ -317,7 +332,8 @@ class DefaultKeychainRepository: KeychainRepository {
     ///
     func setValue(_ value: String, for item: KeychainItem) async throws {
         let accessControl = try keychainService.accessControl(
-            for: item.protection ?? []
+            protection: item.protection,
+            for: item.accessControlFlags ?? []
         )
         let query = await keychainQueryValues(
             for: item,

@@ -339,6 +339,8 @@ final class KeychainRepositoryTests: BitwardenTestCase { // swiftlint:disable:th
             String(data: XCTUnwrap(attributes[kSecValueData] as? Data), encoding: .utf8),
             "ACCESS_TOKEN"
         )
+        let protection = try XCTUnwrap(keychainService.accessControlProtection as? String)
+        XCTAssertEqual(protection, String(kSecAttrAccessibleAfterFirstUnlockThisDeviceOnly))
     }
 
     /// `setAccessToken(userId:)` throws an error if one occurs.
@@ -355,7 +357,7 @@ final class KeychainRepositoryTests: BitwardenTestCase { // swiftlint:disable:th
         keychainService.accessControlResult = .success(
             SecAccessControlCreateWithFlags(
                 nil,
-                kSecAttrAccessibleWhenUnlockedThisDeviceOnly,
+                kSecAttrAccessibleAfterFirstUnlockThisDeviceOnly,
                 [],
                 nil
             )!
@@ -368,6 +370,8 @@ final class KeychainRepositoryTests: BitwardenTestCase { // swiftlint:disable:th
             String(data: XCTUnwrap(attributes[kSecValueData] as? Data), encoding: .utf8),
             "AUTHENTICATOR_VAULT_KEY"
         )
+        let protection = try XCTUnwrap(keychainService.accessControlProtection as? String)
+        XCTAssertEqual(protection, String(kSecAttrAccessibleAfterFirstUnlockThisDeviceOnly))
     }
 
     /// `setAuthenticatorVaultKey(userId:)` throws an error if one occurs.
@@ -384,7 +388,7 @@ final class KeychainRepositoryTests: BitwardenTestCase { // swiftlint:disable:th
         keychainService.accessControlResult = .success(
             SecAccessControlCreateWithFlags(
                 nil,
-                kSecAttrAccessibleWhenUnlockedThisDeviceOnly,
+                kSecAttrAccessibleAfterFirstUnlockThisDeviceOnly,
                 [],
                 nil
             )!
@@ -397,6 +401,8 @@ final class KeychainRepositoryTests: BitwardenTestCase { // swiftlint:disable:th
             String(data: XCTUnwrap(attributes[kSecValueData] as? Data), encoding: .utf8),
             "REFRESH_TOKEN"
         )
+        let protection = try XCTUnwrap(keychainService.accessControlProtection as? String)
+        XCTAssertEqual(protection, String(kSecAttrAccessibleAfterFirstUnlockThisDeviceOnly))
     }
 
     /// `setRefreshToken(userId:)` throws an error if one occurs.
@@ -431,7 +437,7 @@ final class KeychainRepositoryTests: BitwardenTestCase { // swiftlint:disable:th
             SecAccessControlCreateWithFlags(
                 nil,
                 kSecAttrAccessibleWhenUnlockedThisDeviceOnly,
-                item.protection ?? [],
+                item.accessControlFlags ?? [],
                 nil
             )!
         )
@@ -450,13 +456,15 @@ final class KeychainRepositoryTests: BitwardenTestCase { // swiftlint:disable:th
             SecAccessControlCreateWithFlags(
                 nil,
                 kSecAttrAccessibleWhenUnlockedThisDeviceOnly,
-                item.protection ?? [],
+                item.accessControlFlags ?? [],
                 nil
             )!
         )
         keychainService.addResult = .success(())
         try await subject.setUserAuthKey(for: item, value: newKey)
         XCTAssertEqual(keychainService.accessControlFlags, .biometryCurrentSet)
+        let protection = try XCTUnwrap(keychainService.accessControlProtection as? String)
+        XCTAssertEqual(protection, String(kSecAttrAccessibleWhenUnlockedThisDeviceOnly))
     }
 
     /// `setUserAuthKey(_:)` succeeds quietly.
@@ -468,12 +476,14 @@ final class KeychainRepositoryTests: BitwardenTestCase { // swiftlint:disable:th
             SecAccessControlCreateWithFlags(
                 nil,
                 kSecAttrAccessibleWhenUnlockedThisDeviceOnly,
-                item.protection ?? [],
+                item.accessControlFlags ?? [],
                 nil
             )!
         )
         keychainService.addResult = .success(())
         try await subject.setUserAuthKey(for: item, value: newKey)
         XCTAssertEqual(keychainService.accessControlFlags, [])
+        let protection = try XCTUnwrap(keychainService.accessControlProtection as? String)
+        XCTAssertEqual(protection, String(kSecAttrAccessibleWhenUnlockedThisDeviceOnly))
     }
 }
@@ -8,10 +8,14 @@ import Foundation
 protocol KeychainService: AnyObject {
     /// Creates an access control for a given set of flags.
     ///
-    /// - Parameter flags: The `SecAccessControlCreateFlags` for the access control.
+    /// - Parameters:
+    ///   - protection: Protection class to be used for the item. Use one of the values that go with the
+    ///     `kSecAttrAccessible` attribute key.
+    ///   - flags: The `SecAccessControlCreateFlags` for the access control.
     /// - Returns: The SecAccessControl.
     ///
     func accessControl(
+        protection: CFTypeRef,
         for flags: SecAccessControlCreateFlags
     ) throws -> SecAccessControl
 
@@ -75,12 +79,13 @@ class DefaultKeychainService: KeychainService {
     // MARK: Methods
 
     func accessControl(
+        protection: CFTypeRef,
         for flags: SecAccessControlCreateFlags
     ) throws -> SecAccessControl {
         var error: Unmanaged<CFError>?
         let accessControl = SecAccessControlCreateWithFlags(
             nil,
-            kSecAttrAccessibleWhenUnlockedThisDeviceOnly,
+            protection,
             flags,
             &error
         )

diff --git a/BitwardenShared/Core/Auth/Services/KeychainServiceTests.swift b/BitwardenShared/Core/Auth/Services/KeychainServiceTests.swift
@@ -7,6 +7,60 @@ import XCTest
 class KeychainServiceErrorTests: BitwardenTestCase {
     // MARK: Tests
 
+    /// Creating an access control with no specific protection or flags results in the correct default values.
+    func test_accessControl_default() throws {
+        let subject = DefaultKeychainService()
+
+        let accessControl = try subject.accessControl(
+            protection: kSecAttrAccessibleWhenUnlockedThisDeviceOnly,
+            for: []
+        )
+        var error: Unmanaged<CFError>?
+        let expected = SecAccessControlCreateWithFlags(
+            nil,
+            kSecAttrAccessibleWhenUnlockedThisDeviceOnly,
+            [],
+            &error
+        )
+        XCTAssertEqual(accessControl, expected)
+    }
+
+    /// Specifying `.biometryCurrentSet` access control flag is reflected in the access control.
+    func test_accessControl_withBiometrics() throws {
+        let subject = DefaultKeychainService()
+
+        let accessControl = try subject.accessControl(
+            protection: kSecAttrAccessibleWhenUnlockedThisDeviceOnly,
+            for: .biometryCurrentSet
+        )
+        var error: Unmanaged<CFError>?
+        let expected = SecAccessControlCreateWithFlags(
+            nil,
+            kSecAttrAccessibleWhenUnlockedThisDeviceOnly,
+            .biometryCurrentSet,
+            &error
+        )
+        XCTAssertEqual(accessControl, expected)
+    }
+
+    /// Specifying a custom protection level is reflected in the access control.
+    func test_accessControl_withProtection() throws {
+        let subject = DefaultKeychainService()
+
+        let accessControl = try subject.accessControl(
+            protection: kSecAttrAccessibleAfterFirstUnlockThisDeviceOnly,
+            for: []
+        )
+        var error: Unmanaged<CFError>?
+        let expected = SecAccessControlCreateWithFlags(
+            nil,
+            kSecAttrAccessibleAfterFirstUnlockThisDeviceOnly,
+            [],
+            &error
+        )
+        XCTAssertEqual(accessControl, expected)
+    }
+
     /// `getter:errorUserInfo` gets the appropriate user info based on the error case.
     func test_errorUserInfo() {
         let errorAccessControlFailed = KeychainServiceError.accessControlFailed(nil)

@@ -140,7 +140,7 @@ class MockKeychainRepository: KeychainRepository {
 
     func setUserAuthKey(for item: KeychainItem, value: String) async throws {
         let formattedKey = formattedKey(for: item)
-        securityType = item.protection
+        securityType = item.accessControlFlags
         try setResult.get()
         mockStorage[formattedKey] = value
     }

@@ -6,6 +6,7 @@ class MockKeychainService {
     // MARK: Properties
 
     var accessControlFlags: SecAccessControlCreateFlags?
+    var accessControlProtection: CFTypeRef?
     var accessControlResult: Result<SecAccessControl, KeychainServiceError> = .failure(.accessControlFailed(nil))
     var addAttributes: CFDictionary?
     var addResult: Result<Void, KeychainServiceError> = .success(())
@@ -18,8 +19,9 @@ class MockKeychainService {
 // MARK: KeychainService
 
 extension MockKeychainService: KeychainService {
-    func accessControl(for flags: SecAccessControlCreateFlags) throws -> SecAccessControl {
+    func accessControl(protection: CFTypeRef, for flags: SecAccessControlCreateFlags) throws -> SecAccessControl {
            accessControlFlags = flags
+        accessControlProtection = protection
         return try accessControlResult.get()
     }
 

@@ -72,7 +72,7 @@ actor DefaultAuthenticatorSyncService: NSObject, AuthenticatorSyncService {
     /// A Task to hold the subscription that waits for sync to be turned on/off.
     private var syncSubscriber: Task<Void, Never>?
 
-    /// A Task to hold the subscription that waits for the vault to be locked/unlocked..
+    /// A Task to hold the subscription that waits for the vault to be locked/unlocked.
     private var vaultSubscriber: Task<Void, Never>?
 
     /// The service used by the application to manage vault access.
@@ -269,12 +269,18 @@ actor DefaultAuthenticatorSyncService: NSObject, AuthenticatorSyncService {
     /// - Parameter userId: The userId of the user whose sync is being enabled.
     ///
     private func enableSyncForUserId(_ userId: String) {
-        guard !vaultTimeoutService.isLocked(userId: userId) else { return }
-
         enableSyncTask = Task { [enableSyncTask] in
             _ = await enableSyncTask?.result
 
             do {
+                guard !vaultTimeoutService.isLocked(userId: userId) else {
+                    let authVaultKey = try? await keychainRepository.getAuthenticatorVaultKey(userId: userId)
+                    if authVaultKey != nil {
+                        subscribeToCipherUpdates(userId: userId)
+                    }
+                    return
+                }
+
                 try await createAuthenticatorKeyIfNeeded()
                 try await createAuthenticatorVaultKeyIfNeeded(userId: userId)
                 subscribeToCipherUpdates(userId: userId)

@@ -113,7 +113,7 @@ final class AuthenticatorSyncServiceTests: BitwardenTestCase { // swiftlint:disa
 
     /// When the user has subscribed to sync and has an unlocked vault, the
     /// `createAuthenticatorVaultKeyIfNeeded` method successfully handles an
-    /// error in retrieving the user's vault key..
+    /// error in retrieving the user's vault key.
        ///
     @MainActor
     func test_createAuthenticatorVaultKeyIfNeeded_cryptoError() async throws {
@@ -145,7 +145,7 @@ final class AuthenticatorSyncServiceTests: BitwardenTestCase { // swiftlint:disa
 
     /// When the user has subscribed to sync and has an unlocked vault, the
     /// `createAuthenticatorVaultKeyIfNeeded` method successfully handles an
-    /// error in storing the user's vault key in the keychain..
+    /// error in storing the user's vault key in the keychain.
     ///
     @MainActor
     func test_createAuthenticatorVaultKeyIfNeeded_keychainError() async throws {
@@ -708,6 +708,40 @@ final class AuthenticatorSyncServiceTests: BitwardenTestCase { // swiftlint:disa
         XCTAssertEqual(item.id, "1234")
     }
 
+    /// Verifies that the AuthSyncService uses the previously stored AuthenticatorVaultKey
+    /// when the user's vault is locked at the initial startup of the service - i.e. if the  AuthenticatorVaultKey
+    /// exists, there's no need to wait for vault unlock.
+    ///
+    @MainActor
+    func test_determineSyncForUserId_vaultLockedAtStartup() async throws {
+        setupInitialState(vaultLocked: true)
+        let key = try await clientService.crypto().getUserEncryptionKey()
+        try await keychainRepository.setAuthenticatorVaultKey(key, userId: "1")
+        await subject.start()
+
+        stateService.syncToAuthenticatorSubject.send(("1", true))
+        cipherDataStore.cipherSubjectByUserId["1"]?.send([
+            .fixture(
+                id: "1234",
+                login: .fixture(
+                    username: "masked@example.com",
+                    totp: "totp"
+                )
+            ),
+        ])
+
+        waitFor(authBridgeItemService.storedItems["1"]?.first != nil)
+
+        let item = try XCTUnwrap(authBridgeItemService.storedItems["1"]?.first)
+        XCTAssertEqual(item.accountDomain, "vault.bitwarden.com")
+        XCTAssertEqual(item.accountEmail, "user@bitwarden.com")
+        XCTAssertEqual(item.favorite, false)
+        XCTAssertEqual(item.id, "1234")
+        XCTAssertEqual(item.name, "Bitwarden")
+        XCTAssertEqual(item.totpKey, "totp")
+        XCTAssertEqual(item.username, "masked@example.com")
+    }
+
     /// When the `AuthenticatorBridgeItemService` throws an error , `getTemporaryTotpItem()`  returns `nil`.
     ///
     @MainActor
@@ -876,7 +910,7 @@ final class AuthenticatorSyncServiceTests: BitwardenTestCase { // swiftlint:disa
     }
 
     /// Verify that `writeCiphers()` correctly catches and logs errors that occur in `decryptTOTPs`. The user's vault is
-    /// re-locked at the end of error handling..
+    /// re-locked at the end of error handling.
     ///
     @MainActor
     func test_writeCiphers_vaultLockedDecryptTOTPsError() async throws {

@@ -155,7 +155,10 @@ class DefaultMigrationService {
                 // Set access control flags for biometric keys.
                 if let account = itemDictionary[kSecAttrAccount] as? String,
                    account.contains("userKeyBiometricUnlock_"),
-                   let accessControl = try? keychainService.accessControl(for: .biometryCurrentSet) {
+                   let accessControl = t
5ACE
ry? keychainService.accessControl(
+                       protection: kSecAttrAccessibleWhenUnlockedThisDeviceOnly,
+                       for: .biometryCurrentSet
+                   ) {
                     attributesToUpdate[kSecAttrAccessControl] = accessControl
                 }