diff --git a/BitwardenKit/Core/Platform/Services/Mocks/MockUserCryptoManagementClientService+Mocks.swift b/BitwardenKit/Core/Platform/Services/Mocks/MockUserCryptoManagementClientService+Mocks.swift new file mode 100644 index 0000000000..e9db920fde --- /dev/null +++ b/BitwardenKit/Core/Platform/Services/Mocks/MockUserCryptoManagementClientService+Mocks.swift @@ -0,0 +1,27 @@ +import BitwardenSdkMocks + +public extension MockUserCryptoManagementClientService { + /// The `MockPinSettingsClientProtocol` wired to `pinSettingsReturnValue`. + var mockPinSettings: MockPinSettingsClientProtocol { + guard let mock = pinSettingsReturnValue as? MockPinSettingsClientProtocol else { + preconditionFailure( + "pinSettingsReturnValue is not a MockPinSettingsClientProtocol. " + + "Use MockUserCryptoManagementClientService.withMocks() to ensure it is wired correctly.", + ) + } + return mock + } + + /// Creates a `MockUserCryptoManagementClientService` with nested mocks pre-wired as return values. + /// + /// - Parameter pinSettings: The mock to return from `pinSettings()`. Defaults to a new + /// `MockPinSettingsClientProtocol`. + /// + static func withMocks( + pinSettings: MockPinSettingsClientProtocol = MockPinSettingsClientProtocol(), + ) -> MockUserCryptoManagementClientService { + let mock = MockUserCryptoManagementClientService() + mock.pinSettingsReturnValue = pinSettings + return mock + } +} diff --git a/BitwardenKit/Core/Platform/Services/UserCryptoManagementClientService.swift b/BitwardenKit/Core/Platform/Services/UserCryptoManagementClientService.swift new file mode 100644 index 0000000000..14ac746292 --- /dev/null +++ b/BitwardenKit/Core/Platform/Services/UserCryptoManagementClientService.swift @@ -0,0 +1,27 @@ +import BitwardenSdk +import Foundation + +/// A protocol for a service that handles SDK-managed user crypto state tasks. This is similar to +/// `UserCryptoManagementClientProtocol` but returns protocols so they can be mocked for testing. +/// +public protocol UserCryptoManagementClientService: AnyObject, Sendable { // sourcery: AutoMockable + /// Changes the user's KDF settings. + /// + func changeKdf(password: String, newKdf: Kdf) async throws + + /// Returns the PIN settings sub-client. + /// + func pinSettings() -> PinSettingsClientProtocol + + /// Backfills the server with the id of the user's current user key, if missing. + /// + func userKeyIdBackfill() async throws +} + +// MARK: UserCryptoManagementClient + +extension UserCryptoManagementClient: UserCryptoManagementClientService { + public func pinSettings() -> PinSettingsClientProtocol { + pinSettings() as PinSettingsClient + } +} diff --git a/BitwardenSdkMocks/BitwardenSdkMocks.swift b/BitwardenSdkMocks/BitwardenSdkMocks.swift index 7f02677044..760b7b9983 100644 --- a/BitwardenSdkMocks/BitwardenSdkMocks.swift +++ b/BitwardenSdkMocks/BitwardenSdkMocks.swift @@ -19,6 +19,7 @@ extension Fido2CredentialStore {} extension FoldersClientProtocol {} extension GeneratorClientsProtocol {} extension PasswordHistoryClientProtocol {} +extension PinSettingsClientProtocol {} extension PoliciesClientProtocol {} extension RegistrationClientProtocol {} extension SendClientProtocol {} diff --git a/BitwardenShared/Core/Auth/Repositories/AuthRepository.swift b/BitwardenShared/Core/Auth/Repositories/AuthRepository.swift index 7ff536f81e..67ee847e66 100644 --- a/BitwardenShared/Core/Auth/Repositories/AuthRepository.swift +++ b/BitwardenShared/Core/Auth/Repositories/AuthRepository.swift @@ -746,7 +746,12 @@ extension DefaultAuthRepository: AuthRepository { } func clearPins() async throws { - try await stateService.clearPins() + guard await configService.getFeatureFlag(.sdkManagedPinUnlock) else { + try await stateService.clearPins() + return + } + + try await clientService.userCryptoManagement().pinSettings().unsetPin() } func deleteAccount(otp: String?, passwordText: String?) async throws { @@ -1041,11 +1046,17 @@ extension DefaultAuthRepository: AuthRepository { } func setPins(_ pin: String, requirePasswordAfterRestart: Bool) async throws { - let enrollPinResponse = try await clientService.crypto().enrollPin(pin: pin) - try await stateService.setPinKeys( - enrollPinResponse: enrollPinResponse, - requirePasswordAfterRestart: requirePasswordAfterRestart, - ) + guard await configService.getFeatureFlag(.sdkManagedPinUnlock) else { + let enrollPinResponse = try await clientService.crypto().enrollPin(pin: pin) + try await stateService.setPinKeys( + enrollPinResponse: enrollPinResponse, + requirePasswordAfterRestart: requirePasswordAfterRestart, + ) + return + } + + let lockType: PinLockType = requirePasswordAfterRestart ? .afterFirstUnlock : .beforeFirstUnlock + try await clientService.userCryptoManagement().pinSettings().setPin(pin: pin, lockType: lockType) } func setLastActiveAccountTime() async throws { @@ -1161,22 +1172,27 @@ extension DefaultAuthRepository: AuthRepository { } func unlockVaultWithPIN(pin: String) async throws { - if let pinProtectedUserKeyEnvelope = try await stateService.pinProtectedUserKeyEnvelope() { - try await unlockVault( - method: .pinEnvelope( - pin: pin, - pinProtectedUserKeyEnvelope: pinProtectedUserKeyEnvelope, - ), - ) - } else { - // This is needed to support unlocking with a legacy pin protected user key. Once the - // vault is unlocked, the user's pin protected user key is migrated to a pin protected - // user key envelope. - guard let pinProtectedUserKey = try await stateService.pinProtectedUserKey() else { - throw StateServiceError.noPinProtectedUserKey + guard await configService.getFeatureFlag(.sdkManagedPinUnlock) else { + if let pinProtectedUserKeyEnvelope = try await stateService.pinProtectedUserKeyEnvelope() { + try await unlockVault( + method: .pinEnvelope( + pin: pin, + pinProtectedUserKeyEnvelope: pinProtectedUserKeyEnvelope, + ), + ) + } else { + // This is needed to support unlocking with a legacy pin protected user key. Once the + // vault is unlocked, the user's pin protected user key is migrated to a pin protected + // user key envelope. + guard let pinProtectedUserKey = try await stateService.pinProtectedUserKey() else { + throw StateServiceError.noPinProtectedUserKey + } + try await unlockVault(method: .pin(pin: pin, pinProtectedUserKey: pinProtectedUserKey)) } - try await unlockVault(method: .pin(pin: pin, pinProtectedUserKey: pinProtectedUserKey)) + return } + + try await unlockVault(method: .pinState(pin: pin)) } func validatePassword(_ password: String) async throws -> Bool { @@ -1204,14 +1220,18 @@ extension DefaultAuthRepository: AuthRepository { } func validatePin(pin: String) async throws -> Bool { - guard let pinProtectedUserKeyEnvelope = try await stateService.pinProtectedUserKeyEnvelope() else { - return false + guard await configService.getFeatureFlag(.sdkManagedPinUnlock) else { + guard let pinProtectedUserKeyEnvelope = try await stateService.pinProtectedUserKeyEnvelope() else { + return false + } + + return try await clientService.auth().validatePinProtectedUserKeyEnvelope( + pin: pin, + pinProtectedUserKeyEnvelope: pinProtectedUserKeyEnvelope, + ) } - return try await clientService.auth().validatePinProtectedUserKeyEnvelope( - pin: pin, - pinProtectedUserKeyEnvelope: pinProtectedUserKeyEnvelope, - ) + return try await clientService.userCryptoManagement().pinSettings().validatePin(pin: pin) } func verifyOtp(_ otp: String) async throws { @@ -1452,7 +1472,7 @@ extension DefaultAuthRepository: AuthRepository { // Note: We handle all errors broadly here because the SDK doesn't provide specific // error types to distinguish key rotation failures from other errors. Clearing the // PIN keys on any error is the safest approach to maintain data consistency. - try await stateService.clearPins() + try await clearPins() // Return `nil` instead of throwing to avoid erroring out of the unlock process. return nil } diff --git a/BitwardenShared/Core/Auth/Repositories/AuthRepositoryTests.swift b/BitwardenShared/Core/Auth/Repositories/AuthRepositoryTests.swift index 7987112909..32286499fc 100644 --- a/BitwardenShared/Core/Auth/Repositories/AuthRepositoryTests.swift +++ b/BitwardenShared/Core/Auth/Repositories/AuthRepositoryTests.swift @@ -249,7 +249,8 @@ class AuthRepositoryTests: BitwardenTestCase { // swiftlint:disable:this type_bo XCTAssertFalse(result) } - /// `.clearPins()` clears the user's pins. + /// `.clearPins()` clears the user's legacy pins when the `sdkManagedPinUnlock` feature flag + /// is disabled. func test_clearPins() async throws { stateService.activeAccount = Account.fixture() let userId = Account.fixture().profile.userId @@ -262,6 +263,21 @@ class AuthRepositoryTests: BitwardenTestCase { // swiftlint:disable:this type_bo XCTAssertNil(stateService.pinProtectedUserKeyValue[userId]) XCTAssertNil(stateService.encryptedPinByUserId[userId]) XCTAssertNil(stateService.accountVolatileData[userId]?.pinProtectedUserKey) + XCTAssertFalse(clientService.mockUserCryptoManagement.mockPinSettings.unsetPinCalled) + } + + /// `.clearPins()` clears the SDK-managed PIN state when the `sdkManagedPinUnlock` feature + /// flag is enabled, leaving legacy PIN state untouched. + func test_clearPins_sdkManagedPinUnlock() async throws { + configService.featureFlagsBool[.sdkManagedPinUnlock] = true + stateService.activeAccount = Account.fixture() + let userId = Account.fixture().profile.userId + stateService.pinProtectedUserKeyValue[userId] = "123" + + try await subject.clearPins() + + XCTAssertTrue(clientService.mockUserCryptoManagement.mockPinSettings.unsetPinCalled) + XCTAssertEqual(stateService.pinProtectedUserKeyValue[userId], "123") } /// `convertNewUserToKeyConnector()` converts a new user to key connector and unlocks the vault. @@ -2302,7 +2318,8 @@ class AuthRepositoryTests: BitwardenTestCase { // swiftlint:disable:this type_bo } } - /// `setPins(_:requirePasswordAfterRestart:)` sets the user's pins. + /// `setPins(_:requirePasswordAfterRestart:)` sets the user's legacy pins when the + /// `sdkManagedPinUnlock` feature flag is disabled. func test_setPins() async throws { let account = Account.fixture() stateService.activeAccount = account @@ -2321,6 +2338,7 @@ class AuthRepositoryTests: BitwardenTestCase { // swiftlint:disable:this type_bo XCTAssertEqual(stateService.encryptedPinByUserId[userId], "userKeyEncryptedPin") XCTAssertEqual(stateService.pinProtectedUserKeyEnvelopeValue[userId], "pinProtectedUserKeyEnvelope") XCTAssertNil(stateService.pinProtectedUserKeyValue[userId]) + XCTAssertFalse(clientService.mockUserCryptoManagement.mockPinSettings.setPinCalled) } /// `setPins(_:requirePasswordAfterRestart:)` throws an error if one occurs. @@ -2332,6 +2350,47 @@ class AuthRepositoryTests: BitwardenTestCase { // swiftlint:disable:this type_bo } } + /// `setPins(_:requirePasswordAfterRestart:)` sets the SDK-managed PIN with a + /// `.afterFirstUnlock` lock type when the `sdkManagedPinUnlock` feature flag is enabled and + /// master password is required after restart, without touching legacy PIN state. + func test_setPins_sdkManagedPinUnlock() async throws { + configService.featureFlagsBool[.sdkManagedPinUnlock] = true + let account = Account.fixture() + stateService.activeAccount = account + + try await subject.setPins("123", requirePasswordAfterRestart: true) + + let setPinArguments = clientService.mockUserCryptoManagement.mockPinSettings.setPinReceivedArguments + XCTAssertEqual(setPinArguments?.pin, "123") + XCTAssertEqual(setPinArguments?.lockType, .afterFirstUnlock) + XCTAssertFalse(clientService.mockCrypto.enrollPinCalled) + XCTAssertNil(stateService.pinProtectedUserKeyEnvelopeValue[account.profile.userId]) + } + + /// `setPins(_:requirePasswordAfterRestart:)` sets the SDK-managed PIN lock type to + /// `.beforeFirstUnlock` when master password isn't required after restart. + func test_setPins_sdkManagedPinUnlock_beforeFirstUnlock() async throws { + configService.featureFlagsBool[.sdkManagedPinUnlock] = true + stateService.activeAccount = .fixture() + + try await subject.setPins("123", requirePasswordAfterRestart: false) + + let setPinArguments = clientService.mockUserCryptoManagement.mockPinSettings.setPinReceivedArguments + XCTAssertEqual(setPinArguments?.pin, "123") + XCTAssertEqual(setPinArguments?.lockType, .beforeFirstUnlock) + } + + /// `setPins(_:requirePasswordAfterRestart:)` throws an error from the SDK-managed PIN + /// enrollment call when the `sdkManagedPinUnlock` feature flag is enabled. + func test_setPins_sdkManagedPinUnlock_error() async throws { + configService.featureFlagsBool[.sdkManagedPinUnlock] = true + clientService.mockUserCryptoManagement.mockPinSettings.setPinThrowableError = BitwardenTestError.example + + await assertAsyncThrows(error: BitwardenTestError.example) { + try await subject.setPins("123", requirePasswordAfterRestart: true) + } + } + /// `.shouldPerformMasterPasswordReprompt(reprompt:)` when reprompt password /// and master password hash exists. func test_shouldPerformMasterPasswordReprompt_true() async throws { @@ -3325,6 +3384,37 @@ class AuthRepositoryTests: BitwardenTestCase { // swiftlint:disable:this type_bo XCTAssertEqual(stateService.manuallyLockedAccounts["1"], false) } + /// `unlockVaultWithPIN(_:)` unlocks the vault via the SDK-managed PIN state when the + /// `sdkManagedPinUnlock` feature flag is enabled. + func test_unlockVaultWithPIN_sdkManagedPinUnlock() async throws { + configService.featureFlagsBool[.sdkManagedPinUnlock] = true + let account = Account.fixture() + stateService.activeAccount = account + stateService.accountEncryptionKeys = [ + "1": AccountEncryptionKeys( + cryptographicState: .fixtureV2(), + encryptedUserKey: "USER_KEY", + ), + ] + + try await subject.unlockVaultWithPIN(pin: "123") + + XCTAssertEqual( + clientService.mockCrypto.initializeUserCryptoReceivedReq, + InitUserCryptoRequest( + userId: "1", + kdfParams: .pbkdf2(iterations: UInt32(Constants.pbkdf2Iterations)), + email: "user@bitwarden.com", + accountCryptographicState: .fixtureV2(), + method: .pinState(pin: "123"), + upgradeToken: nil, + ), + ) + XCTAssertFalse(vaultTimeoutService.isLocked(userId: "1")) + XCTAssertTrue(vaultTimeoutService.unlockVaultHadUserInteraction) + XCTAssertEqual(stateService.manuallyLockedAccounts["1"], false) + } + /// `unlockVaultWithPassword` restores the biometric key after a successful unlock when biometrics is enabled. func test_unlockVaultWithPassword_restoresBiometricKeyWhenEnabled() async throws { let account = Account.fixture(profile: .fixture( @@ -3608,6 +3698,20 @@ class AuthRepositoryTests: BitwardenTestCase { // swiftlint:disable:this type_bo XCTAssertFalse(isPinValid) } + /// `validatePin(_:)` validates via the SDK-managed PIN settings client when the + /// `sdkManagedPinUnlock` feature flag is enabled. + func test_validatePin_sdkManagedPinUnlock() async throws { + configService.featureFlagsBool[.sdkManagedPinUnlock] = true + stateService.activeAccount = .fixture() + clientService.mockUserCryptoManagement.mockPinSettings.validatePinReturnValue = true + + let isPinValid = try await subject.validatePin(pin: "123") + + XCTAssertTrue(isPinValid) + XCTAssertEqual(clientService.mockUserCryptoManagement.mockPinSettings.validatePinReceivedPin, "123") + XCTAssertFalse(clientService.mockAuth.validatePinProtectedUserKeyEnvelopeCalled) + } + /// `validatePin(_:)` throws if the there is no active account. func test_validatePin_noActiveAccount() async throws { await assertAsyncThrows(error: StateServiceError.noActiveAccount) { diff --git a/BitwardenShared/Core/Platform/Models/Enum/FeatureFlag.swift b/BitwardenShared/Core/Platform/Models/Enum/FeatureFlag.swift index 10b6a89bd3..857f0796c6 100644 --- a/BitwardenShared/Core/Platform/Models/Enum/FeatureFlag.swift +++ b/BitwardenShared/Core/Platform/Models/Enum/FeatureFlag.swift @@ -65,6 +65,13 @@ extension FeatureFlag: @retroactive CaseIterable { /// Flag to enable/disable Premium upgrade path. static let premiumUpgradePath = FeatureFlag(rawValue: "pm-31697-premium-upgrade-path") + /// Flag to enable/disable SDK-managed PIN unlock. + /// + /// When enabled, PIN unlock, enrollment, and validation route through the Bitwarden SDK's + /// client-managed PIN state (`InitUserCryptoMethod.pinState`, `PinSettingsClient`) instead + /// of the legacy iOS-managed PIN-protected key envelope. + static let sdkManagedPinUnlock = FeatureFlag(rawValue: "pm-31059-sdk-pin-unlock") + /// Flag to enable/disable the Send Controls policy, which supersedes the `disableSend` and /// `sendOptions` policies when active. static let sendControls = FeatureFlag(rawValue: "pm-31885-send-controls") @@ -91,6 +98,7 @@ extension FeatureFlag: @retroactive CaseIterable { .organizationUserNotificationBanner, .policiesInAcceptedState, .premiumUpgradePath, + .sdkManagedPinUnlock, .sendControls, .vfo1Foundation, ] diff --git a/BitwardenShared/Core/Platform/Services/ClientService.swift b/BitwardenShared/Core/Platform/Services/ClientService.swift index 0cd5f6d063..0836cb5486 100644 --- a/BitwardenShared/Core/Platform/Services/ClientService.swift +++ b/BitwardenShared/Core/Platform/Services/ClientService.swift @@ -73,6 +73,13 @@ protocol ClientService { /// func sends(for userId: String?) async throws -> SendClientProtocol + /// Returns a `UserCryptoManagementClientService` for SDK-managed user crypto state tasks. + /// + /// - Parameter userId: The user ID mapped to the client instance. + /// - Returns: A `UserCryptoManagementClientService` for SDK-managed user crypto state tasks. + /// + func userCryptoManagement(for userId: String?) async throws -> UserCryptoManagementClientService + /// Returns a `VaultClientService` for vault data tasks. /// /// - Parameter userId: The user ID mapped to the client instance. @@ -140,6 +147,12 @@ extension ClientService { try await sends(for: nil) } + /// Returns a `UserCryptoManagementClientService` for SDK-managed user crypto state tasks. + /// + func userCryptoManagement() async throws -> UserCryptoManagementClientService { + try await userCryptoManagement(for: nil) + } + /// Returns a `VaultClientService` for vault data tasks. /// func vault() async throws -> VaultClientService { @@ -252,6 +265,10 @@ actor DefaultClientService: ClientService { try await client(for: userId).sends() } + func userCryptoManagement(for userId: String?) async throws -> UserCryptoManagementClientService { + try await client(for: userId).userCryptoManagement() + } + func vault(for userId: String?) async throws -> VaultClientService { try await client(for: userId).vault() } @@ -381,6 +398,9 @@ protocol BitwardenSdkClient { /// Returns sends operations. func sends() -> SendClientProtocol + /// Returns SDK-managed user crypto state operations. + func userCryptoManagement() -> UserCryptoManagementClientService + /// Returns vault operations. func vault() -> VaultClientService } @@ -420,6 +440,10 @@ extension Client: BitwardenSdkClient { sends() as SendClient } + func userCryptoManagement() -> UserCryptoManagementClientService { + userCryptoManagement() as UserCryptoManagementClient + } + func vault() -> VaultClientService { vault() as VaultClient } diff --git a/BitwardenShared/Core/Platform/Services/ClientServiceTests.swift b/BitwardenShared/Core/Platform/Services/ClientServiceTests.swift index df8bb636f9..70f14eeac6 100644 --- a/BitwardenShared/Core/Platform/Services/ClientServiceTests.swift +++ b/BitwardenShared/Core/Platform/Services/ClientServiceTests.swift @@ -374,6 +374,17 @@ final class ClientServiceTests: BitwardenTestCase { // swiftlint:disable:this ty XCTAssertNotIdentical(sends, user2Sends) } + /// `userCryptoManagement(for:)` returns a new `UserCryptoManagementClientService` for every user. + func test_userCryptoManagement() async throws { + stateService.activeAccount = .fixture(profile: .fixture(userId: "1")) + + let userCryptoManagement = try await subject.userCryptoManagement() + XCTAssertIdentical(userCryptoManagement, clientBuilder.clients.first?.userCryptoManagementClient) + + let user2UserCryptoManagement = try await subject.userCryptoManagement(for: "2") + XCTAssertNotIdentical(userCryptoManagement, user2UserCryptoManagement) + } + /// `vault(for:)` returns a new `VaultClientProtocol` for every user. func test_vault() async throws { stateService.activeAccount = .fixture(profile: .fixture(userId: "1")) diff --git a/BitwardenShared/Core/Platform/Services/TestHelpers/MockClientBuilder.swift b/BitwardenShared/Core/Platform/Services/TestHelpers/MockClientBuilder.swift index fcd4490e0d..5fee33c3e6 100644 --- a/BitwardenShared/Core/Platform/Services/TestHelpers/MockClientBuilder.swift +++ b/BitwardenShared/Core/Platform/Services/TestHelpers/MockClientBuilder.swift @@ -28,6 +28,7 @@ class MockClient: BitwardenSdkClient { var platformClient = MockPlatformClientService.withMocks() var policiesClient = MockPoliciesClientProtocol() var sendClient = MockSendClientProtocol() + var userCryptoManagementClient = MockUserCryptoManagementClientService.withMocks() var vaultClient = MockVaultClientService() func auth() -> any AuthClientService { @@ -66,6 +67,10 @@ class MockClient: BitwardenSdkClient { sendClient } + func userCryptoManagement() -> any UserCryptoManagementClientService { + userCryptoManagementClient + } + func vault() -> any VaultClientService { vaultClient } diff --git a/BitwardenShared/Core/Platform/Services/TestHelpers/MockClientService.swift b/BitwardenShared/Core/Platform/Services/TestHelpers/MockClientService.swift index 061ebe99ce..9398728f81 100644 --- a/BitwardenShared/Core/Platform/Services/TestHelpers/MockClientService.swift +++ b/BitwardenShared/Core/Platform/Services/TestHelpers/MockClientService.swift @@ -18,6 +18,7 @@ class MockClientService: ClientService { var mockPlatformIsPreAuth = false var mockPolicies: MockPoliciesClientProtocol var mockSends: MockSendClientProtocol + var mockUserCryptoManagement: MockUserCryptoManagementClientService var mockVault: MockVaultClientService var platformCallCount = 0 var platformError: Error? @@ -38,6 +39,7 @@ class MockClientService: ClientService { mock.encryptBufferClosure = { _, buffer in buffer } return mock }(), + userCryptoManagement: MockUserCryptoManagementClientService = .withMocks(), vault: MockVaultClientService = MockVaultClientService(), ) { mockAuth = auth @@ -47,6 +49,7 @@ class MockClientService: ClientService { mockPlatform = platform mockPolicies = policies mockSends = sends + mockUserCryptoManagement = userCryptoManagement mockVault = vault } @@ -93,6 +96,10 @@ class MockClientService: ClientService { mockSends } + func userCryptoManagement(for userId: String?) -> UserCryptoManagementClientService { + mockUserCryptoManagement + } + func vault(for userId: String?) -> VaultClientService { mockVault }