Skip to content

Commit

Permalink
PM-11720 - TDE User Without MP Cannot Enable Autofill For Account (#908)
Browse files Browse the repository at this point in the history
  • Loading branch information
phil-livefront authored Sep 6, 2024
1 parent c3f3a13 commit f37d588
Show file tree
Hide file tree
Showing 5 changed files with 141 additions and 16 deletions.
40 changes: 25 additions & 15 deletions BitwardenShared/Core/Auth/Repositories/AuthRepository.swift
Original file line number Diff line number Diff line change
Expand Up @@ -935,11 +935,9 @@ extension DefaultAuthRepository: AuthRepository {
// No-op: nothing extra to do for decryptedKey.
break
case .deviceKey:
// No-op: nothing extra (for now).
break
try await configureBiometricUnlockIfRequired()
case .keyConnector:
// No-op: nothing extra to do for Key Connector.
break
try await configureBiometricUnlockIfRequired()
case let .password(password, _):
let hashedPassword = try await authService.hashPassword(
password: password,
Expand All @@ -956,17 +954,7 @@ extension DefaultAuthRepository: AuthRepository {
try await stateService.setPinProtectedUserKeyToMemory(pinProtectedUserKey)
}

// Re-enable biometrics, if required.
let biometricUnlockStatus = try? await biometricsRepository.getBiometricUnlockStatus()
switch biometricUnlockStatus {
case .available(_, true, false):
try await biometricsRepository.configureBiometricIntegrity()
try await biometricsRepository.setBiometricUnlockKey(
authKey: clientService.crypto().getUserEncryptionKey()
)
default:
break
}
try await configureBiometricUnlockIfRequired()
case .pin:
// No-op: nothing extra to do for pin unlock.
break
Expand Down Expand Up @@ -1027,8 +1015,30 @@ extension DefaultAuthRepository: AuthRepository {
try await stateService.setForcePasswordResetReason(nil)
}

/// Returns the provided user ID if it exists, otherwise fetches the active account's ID.
///
/// - Parameter maybeId: The optional user ID to check.
/// - Returns: The user ID if provided, otherwise the active account's ID.
/// - Throws: An error if fetching the active account ID fails.
///
private func userIdOrActive(_ maybeId: String?) async throws -> String {
if let maybeId { return maybeId }
return try await stateService.getActiveAccountId()
}


Check warning on line 1029 in BitwardenShared/Core/Auth/Repositories/AuthRepository.swift

View workflow job for this annotation

GitHub Actions / Build (Production) / Build

(consecutiveBlankLines) Replace consecutive blank lines with a single blank line.

Check warning on line 1029 in BitwardenShared/Core/Auth/Repositories/AuthRepository.swift

View workflow job for this annotation

GitHub Actions / Build (Beta) / Build

(consecutiveBlankLines) Replace consecutive blank lines with a single blank line.
/// This method checks the biometric unlock status, and if biometric unlock is available but not
/// fully configured (i.e., it doesn't have a valid integrity), it sets up biometric integrity and configures
/// the biometric unlock key.
///
/// - Throws: An error if configuring biometric integrity or setting the biometric unlock key fails.
///
private func configureBiometricUnlockIfRequired() async throws {
if case .available(_, true, false) = try? await biometricsRepository.getBiometricUnlockStatus() {
try await biometricsRepository.configureBiometricIntegrity()
try await biometricsRepository.setBiometricUnlockKey(
authKey: clientService.crypto().getUserEncryptionKey()
)
}
}
}
70 changes: 70 additions & 0 deletions BitwardenShared/Core/Auth/Repositories/AuthRepositoryTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -1035,6 +1035,7 @@ class AuthRepositoryTests: BitwardenTestCase { // swiftlint:disable:this type_bo
try await subject.unlockVaultWithNeverlockKey()
}
XCTAssertFalse(vaultTimeoutService.unlockVaultHadUserInteraction)
XCTAssertFalse(biometricsRepository.didConfigureBiometricIntegrity)
}

/// `test_unlockVaultWithDeviceKey` attempts to unlock the vault using the device key from the keychain.
Expand Down Expand Up @@ -1063,6 +1064,38 @@ class AuthRepositoryTests: BitwardenTestCase { // swiftlint:disable:this type_bo
XCTAssertTrue(vaultTimeoutService.unlockVaultHadUserInteraction)
}

/// `test_unlockVaultWithDeviceKey` attempts to unlock the vault using the device key from the keychain.
func test_unlockVaultWithDeviceKey_successWithBiometricsEnabled() async throws {
let active = Account.fixtureWithTDE()
stateService.activeAccount = active
keychainService.mockStorage = [
keychainService.formattedKey(
for: KeychainItem.deviceKey(
userId: active.profile.userId
)
):
"pasta",
]

biometricsRepository.biometricUnlockStatus = .success(
.available(.faceID, enabled: true, hasValidIntegrity: false)
)

stateService.accountEncryptionKeys = [
active.profile.userId: .init(
encryptedPrivateKey: "secret",
encryptedUserKey: "recipe"
),
]
clientService.mockCrypto.getUserEncryptionKeyResult = .success("sauce")
clientService.mockCrypto.initializeUserCryptoResult = .success(())
await assertAsyncDoesNotThrow {
try await subject.unlockVaultWithDeviceKey()
}
XCTAssertTrue(vaultTimeoutService.unlockVaultHadUserInteraction)
XCTAssertTrue(biometricsRepository.didConfigureBiometricIntegrity)
}

/// `test_unlockVaultWithDeviceKey` attempts to unlock the vault using the device key from the keychain.
func test_unlockVaultWithDeviceKey_error() async throws {
let active = Account.fixture()
Expand Down Expand Up @@ -1457,6 +1490,43 @@ class AuthRepositoryTests: BitwardenTestCase { // swiftlint:disable:this type_bo
)
XCTAssertFalse(keyConnectorService.convertNewUserToKeyConnectorCalled)
XCTAssertTrue(vaultTimeoutService.unlockVaultHadUserInteraction)
XCTAssertFalse(biometricsRepository.didConfigureBiometricIntegrity)
}

/// `unlockVaultWithKeyConnectorKey()` unlocks the user's vault with their key connector key.
func test_unlockVaultWithKeyConnectorKeyWithBiometricsEnabled() async {
clientService.mockCrypto.initializeUserCryptoResult = .success(())
keyConnectorService.getMasterKeyFromKeyConnectorResult = .success("key")
stateService.accountEncryptionKeys = [
"1": AccountEncryptionKeys(
encryptedPrivateKey: "private",
encryptedUserKey: "user"
),
]
stateService.activeAccount = .fixture()
biometricsRepository.biometricUnlockStatus = .success(
.available(.faceID, enabled: true, hasValidIntegrity: false)
)

await assertAsyncDoesNotThrow {
try await subject.unlockVaultWithKeyConnectorKey(
keyConnectorURL: URL(string: "https://example.com")!,
orgIdentifier: "org-id"
)
}

XCTAssertEqual(
clientService.mockCrypto.initializeUserCryptoRequest,
InitUserCryptoRequest(
kdfParams: KdfConfig().sdkKdf,
email: "[email protected]",
privateKey: "private",
method: .keyConnector(masterKey: "key", userKey: "user")
)
)
XCTAssertFalse(keyConnectorService.convertNewUserToKeyConnectorCalled)
XCTAssertTrue(vaultTimeoutService.unlockVaultHadUserInteraction)
XCTAssertTrue(biometricsRepository.didConfigureBiometricIntegrity)
}

/// `unlockVaultWithKeyConnectorKey()` converts a new user to use key connector and unlocks the
Expand Down
1 change: 1 addition & 0 deletions BitwardenShared/UI/Auth/AuthRouter.swift
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ final class AuthRouter: NSObject, Router {
// MARK: Types

typealias Services = HasAuthRepository
& HasBiometricsRepository
& HasClientService
& HasConfigService
& HasErrorReporter
Expand Down
37 changes: 37 additions & 0 deletions BitwardenShared/UI/Auth/AuthRouterTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ final class AuthRouterTests: BitwardenTestCase { // swiftlint:disable:this type_
// MARK: Properties

var authRepository: MockAuthRepository!
var biometricsRepository: MockBiometricsRepository!
var configService: MockConfigService!
var errorReporter: MockErrorReporter!
var stateService: MockStateService!
Expand All @@ -20,6 +21,7 @@ final class AuthRouterTests: BitwardenTestCase { // swiftlint:disable:this type_
super.setUp()

authRepository = MockAuthRepository()
biometricsRepository = MockBiometricsRepository()
configService = MockConfigService()
errorReporter = MockErrorReporter()
stateService = MockStateService()
Expand All @@ -29,6 +31,7 @@ final class AuthRouterTests: BitwardenTestCase { // swiftlint:disable:this type_
isInAppExtension: false,
services: ServiceContainer.withMocks(
authRepository: authRepository,
biometricsRepository: biometricsRepository,
configService: configService,
errorReporter: errorReporter,
stateService: stateService,
Expand All @@ -41,6 +44,7 @@ final class AuthRouterTests: BitwardenTestCase { // swiftlint:disable:this type_
super.tearDown()

authRepository = nil
biometricsRepository = nil
configService = nil
errorReporter = nil
stateService = nil
Expand Down Expand Up @@ -118,6 +122,39 @@ final class AuthRouterTests: BitwardenTestCase { // swiftlint:disable:this type_
XCTAssertEqual(route, .complete)
}

/// `handleAndRoute(_ :)` redirects `.accountBecameActive()` to `.enterpriseSingleSignOn`
/// when the account is unlocked.
func test_handleAndRoute_accountBecameActive_noMpAndTDE_withBiometricsEnabled() async {
let active = Account.fixture(
profile: .fixture(
userDecryptionOptions: UserDecryptionOptions(
hasMasterPassword: false,
keyConnectorOption: nil,
trustedDeviceOption: nil
)
)
)
stateService.activeAccount = active

biometricsRepository.biometricUnlockStatus = .success(
.available(.faceID, enabled: true, hasValidIntegrity: false)
)
stateService.isAuthenticated = [
active.profile.userId: true,
]

authRepository.isLockedResult = .success(true)
let route = await subject.handleAndRoute(
.accountBecameActive(
active,
animated: true,
attemptAutomaticBiometricUnlock: true,
didSwitchAccountAutomatically: false
)
)
XCTAssertEqual(route, .enterpriseSingleSignOn(email: "[email protected]"))
}

/// `handleAndRoute(_ :)` redirects `.accountBecameActive()` to `.vaultUnlock` when checking if
/// an account is authenticated fails.
func test_handleAndRoute_accountBecameActive_logout_isAuthenticatedError() async {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -354,7 +354,14 @@ extension AuthRouter {
return .landingSoftLoggedOut(email: activeAccount.profile.email)
}

// Otherwise, return `.vaultUnlock`.
let hasMasterPassword = activeAccount.profile.userDecryptionOptions?.hasMasterPassword == true

if !hasMasterPassword {
let biometricUnlockStatus = try await services.biometricsRepository.getBiometricUnlockStatus()
if case .available(_, true, false) = biometricUnlockStatus {
return .enterpriseSingleSignOn(email: activeAccount.profile.email)
}
}
return .vaultUnlock(
activeAccount,
animated: animated,
Expand Down

0 comments on commit f37d588

Please sign in to comment.