8000 [PM-8388] Update pin verification to use the new SDK function. by fedemkr · Pull Request #773 · bitwarden/ios · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

[PM-8388] Update pin verification to use the new SDK function. #773

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Jul 30, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 3 additions & 24 deletions BitwardenShared/Core/Auth/Repositories/AuthRepository.swift
Original file line number Diff line number Diff line change
Expand Up @@ -233,7 +233,7 @@ protocol AuthRepository: AnyObject {
/// Validates thes user's entered PIN.
/// - Parameter pin: Pin to validate.
/// - Returns: `true` if valid, `false` otherwise.
func validatePin(pin: String) async -> Bool
func validatePin(pin: String) async throws -> Bool

/// Verifies that the entered one-time password matches the one sent to the user.
///
Expand Down Expand Up @@ -779,33 +779,12 @@ extension DefaultAuthRepository: AuthRepository {
}
}

func validatePin(pin: String) async -> Bool {
func validatePin(pin: String) async throws -> Bool {
guard let pinProtectedUserKey = try? await stateService 8000 .pinProtectedUserKey() else {
return false
}

// HACK: As the SDK doesn't provide a way to directly validate the pin yet, we have this method
// which just tries to initialize the user crypto and if it succeeds then the PIN is correct, otherwise
// the PIN is incorrect.

do {
let account = try await stateService.getActiveAccount()
let encryptionKeys = try await stateService.getAccountEncryptionKeys()

try await clientService.crypto().initializeUserCrypto(
req: InitUserCryptoRequest(
kdfParams: account.kdf.sdkKdf,
email: account.profile.email,
privateKey: encryptionKeys.encryptedPrivateKey,
method: .pin(pin: pin, pinProtectedUserKey: pinProtectedUserKey)
)
)
try await organizationService.initializeOrganizationCrypto()

return true
} catch {
return false
}
return try await clientService.auth().validatePin(pin: pin, pinProtectedUserKey: pinProtectedUserKey)
}

func verifyOtp(_ otp: String) async throws {
Expand Down
84 changes: 25 additions & 59 deletions BitwardenShared/Core/Auth/Repositories/AuthRepositoryTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -1564,53 +1564,25 @@ class AuthRepositoryTests: BitwardenTestCase { // swiftlint:disable:this type_bo
XCTAssertFalse(isValid)
}

/// `validatePin(_:)` returns `true` if the pin is valid when initializing the user crypto.
/// `validatePin(_:)` returns `true` if the pin is valid.
func test_validatePin() async throws {
let account = Account.fixture()
stateService.activeAccount = account

stateService.accountEncryptionKeys = [
"1": AccountEncryptionKeys(encryptedPrivateKey: "PRIVATE_KEY", encryptedUserKey: "USER_KEY"),
]

stateService.encryptedPinByUserId[account.profile.userId] = "123"
stateService.pinProtectedUserKeyValue[account.profile.userId] = "123"

let isPinValid = await subject.validatePin(pin: "123")
clientService.mockAuth.validatePinResult = .success(true)

let isPinValid = try await subject.validatePin(pin: "123")

XCTAssertEqual(
clientService.mockCrypto.initializeUserCryptoRequest,
InitUserCryptoRequest(
kdfParams: .pbkdf2(iterations: UInt32(Constants.pbkdf2Iterations)),
email: "user@bitwarden.com",
privateKey: "PRIVATE_KEY",
method: .pin(pin: "123", pinProtectedUserKey: "123")
)
)
XCTAssertTrue(isPinValid)
}

/// `validatePin(_:)` returns `false` if the there is no active account.
func test_validatePin_noActiveAccount() async throws {
let account = Account.fixture()

stateService.accountEncryptionKeys = [
"1": AccountEncryptionKeys(encryptedPrivateKey: "PRIVATE_KEY", encryptedUserKey: "USER_KEY"),
]

stateService.encryptedPinByUserId[account.profile.userId] = "123"

let isPinValid = await subject.validatePin(pin: "123")
let isPinValid = try await subject.validatePin(pin: "123")

XCTAssertNotEqual(
clientService.mockCrypto.initializeUserCryptoRequest,
InitUserCryptoRequest(
kdfParams: .pbkdf2(iterations: UInt32(Constants.pbkdf2Iterations)),
email: "user@bitwarden.com",
privateKey: "PRIVATE_KEY",
method: .pin(pin: "123", pinProtectedUserKey: "123")
)
)
XCTAssertFalse(isPinValid)
}

Expand All @@ -1619,45 +1591,39 @@ class AuthRepositoryTests: BitwardenTestCase { // swiftlint:disable:this type_bo
let account = Account.fixture()
stateService.activeAccount = account

stateService.accountEncryptionKeys = [
"1": AccountEncryptionKeys(encryptedPrivateKey: "PRIVATE_KEY", encryptedUserKey: "USER_KEY"),
]

stateService.encryptedPinByUserId[account.profile.userId] = "123"

let isPinValid = await subject.validatePin(pin: "123")
let isPinValid = try await subject.validatePin(pin: "123")

XCTAssertNotEqual(
clientService.mockCrypto.initializeUserCryptoRequest,
InitUserCryptoRequest(
kdfParams: .pbkdf2(iterations: UInt32(Constants.pbkdf2Iterations)),
email: "user@bitwarden.com",
privateKey: "PRIVATE_KEY",
method: .pin(pin: "123", pinProtectedUserKey: "123")
)
)
XCTAssertFalse(isPinValid)
}

/// `validatePin(_:)` returns `false` if initializing user crypto throws.
func test_validatePin_initializeUserCryptoThrows() async throws {
/// `validatePin(_:)` returns `false` if the pin is not valid.
func test_validatePin_notValid() async throws {
let account = Account.fixture()
stateService.activeAccount = account

stateService.accountEncryptionKeys = [
"1": AccountEncryptionKeys(encryptedPrivateKey: "PRIVATE_KEY", encryptedUserKey: "USER_KEY"),
]

stateService.encryptedPinByUserId[account.profile.userId] = "123"
stateService.pinProtectedUserKeyValue[account.profile.userId] = "123"

clientService.mockCrypto.initializeUserCryptoResult = .failure(BitwardenTestError.example)
clientService.mockAuth.validatePinResult = .success(false)

let isPinValid = await subject.validatePin(pin: "123")
let isPinValid = try await subject.validatePin(pin: "123")

XCTAssertFalse(isPinValid)
}

/// `validatePin(_:)` throws when validating.
func test_validatePin_throws() async throws {
let account = Account.fixture()
stateService.activeAccount = account

stateService.pinProtectedUserKeyValue[account.profile.userId] = "123"

clientService.mockAuth.validatePinResult = .failure(BitwardenTestError.example)

await assertAsyncThrows(error: BitwardenTestError.example) {
_ = try await subject.validatePin(pin: "123")
}
}

/// `validatePin(_:)` returns `false` if initializing org crypto throws.
func test_validatePin_initializeOrgCryptoThrows() async throws {
let account = Account.fixture()
Expand All @@ -1672,7 +1638,7 @@ class AuthRepositoryTests: BitwardenTestCase { // swiftlint:disable:this type_bo

organizationService.initializeOrganizationCryptoError = BitwardenTestError.example

let isPinValid = await subject.validatePin(pin: "123")
let isPinValid = try await subject.validatePin(pin: "123")

XCTAssertFalse(isPinValid)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ class MockAuthRepository: AuthRepository { // swiftlint:disable:this type_body_l
var validatePasswordPasswords = [String]()
var validatePasswordResult: Result<Bool, Error> = .success(true)

var validatePinResult: Bool = true
var validatePinResult: Result<Bool, Error> = .success(false)

var vaultTimeout = [String: SessionTimeoutValue]()

Expand Down Expand Up @@ -305,8 +305,8 @@ class MockAuthRepository: AuthRepository { // swiftlint:disable:this type_body_l
return try validatePasswordResult.get()
}

func validatePin(pin: String) async -> Bool {
validatePinResult
func validatePin(pin: String) async throws -> Bool {
try validatePinResult.get()
}

func verifyOtp(_ otp: String) async throws {
Expand Down
29 changes: 18 additions & 11 deletions BitwardenShared/UI/Auth/Utilities/UserVerificationHelper.swift
Original file line number Diff line number Diff line change
Expand Up @@ -159,18 +159,25 @@ extension DefaultUserVerificationHelper: UserVerificationHelper {
continuation.resume(throwing: UserVerificationError.cancelled)
},
settingUp: false,
completion: { pin in
guard await self.authRepository.validatePin(pin: pin) else {
self.userVerificationDelegate?.showAlert(
.defaultAlert(title: Localizations.invalidPIN),
onDismissed: {
continuation.resume(returning: .notVerified)
}
)
return
}
completion: { [weak self] pin in
guard let self else { return }

do {
guard try await authRepository.validatePin(pin: pin) else {
userVerificationDelegate?.showAlert(
.defaultAlert(title: Localizations.invalidPIN),
onDismissed: {
continuation.resume(returning: .notVerified)
}
)
return
}

continuation.resume(returning: .verified)
continuation.resume(returning: .verified)
} catch {
errorReporter.log(error: error)
continuation.resume(returning: .unableToPerform)
}
}
)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import XCTest

// MARK: - UserVerificationHelperTests

class UserVerificationHelperTests: BitwardenTestCase {
class UserVerificationHelperTests: BitwardenTestCase { // swiftlint:disable:this type_body_length
// MARK: Types

typealias VerifyFunction = () async throws -> UserVerificationResult
Expand Down Expand Up @@ -298,7 +298,7 @@ class UserVerificationHelperTests: BitwardenTestCase {
/// `verifyPin()` with verified PIN.
func test_verifyPin_verified() async throws {
authRepository.isPinUnlockAvailableResult = .success(true)
authRepository.validatePinResult = true
authRepository.validatePinResult = .success(true)

let task = Task {
try await self.subject.verifyPin()
Expand All @@ -318,7 +318,7 @@ class UserVerificationHelperTests: BitwardenTestCase {
/// `verifyPin()` with not verified PIN.
func test_verifyPin_notVerified() async throws {
authRepository.isPinUnlockAvailableResult = .success(true)
authRepository.validatePinResult = false
authRepository.validatePinResult = .success(false)

let task = Task {
try await self.subject.verifyPin()
Expand Down Expand Up @@ -348,6 +348,31 @@ class UserVerificationHelperTests: BitwardenTestCase {
XCTAssertEqual(result, .notVerified)
}

/// `verifyPin()` with throwing pin verification returns unable to perform.
func test_verifyPin_throwsUnableToPerform() async throws {
authRepository.isPinUnlockAvailableResult = .success(true)
authRepository.validatePinResult = .failure(BitwardenTestError.example)

let task = Task {
try await self.subject.verifyPin()
}

try await waitForAsync {
!self.userVerificationDelegate.alertShown.isEmpty
}

try await enterPinInAlertAndSubmit()

try await waitForAsync {
!self.errorReporter.errors.isEmpty
}

let result = try await task.value

XCTAssertEqual(errorReporter.errors.last as? BitwardenTestError, .example)
XCTAssertEqual(result, .unableToPerform)
}

// MARK: Private

private func enterMasterPasswordInAlertAndSubmit() async throws {
Expand Down Expand Up @@ -387,4 +412,4 @@ class MockUserVerificationHelperDelegate: UserVerificationDelegate {
showAlert(alert)
alertOnDismissed = onDismissed
}
}
} // swiftlint:disable:this file_length
Loading
0