Skip to content

Commit

Permalink
[PM-10450] Fix FIdo2 never lock user verification not appearing (#787)
Browse files Browse the repository at this point in the history
  • Loading branch information
fedemkr authored Aug 2, 2024
1 parent 42acf6f commit 72a2018
Show file tree
Hide file tree
Showing 6 changed files with 50 additions and 5 deletions.
16 changes: 16 additions & 0 deletions BitwardenAutoFillExtension/CredentialProviderViewController.swift
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import AuthenticationServices
import BitwardenSdk
import BitwardenShared
import Combine
import OSLog

/// An `ASCredentialProviderViewController` that implements credential autofill.
Expand All @@ -11,12 +12,21 @@ class CredentialProviderViewController: ASCredentialProviderViewController {
/// The app's theme.
var appTheme: AppTheme = .default

/// A subject containing whether the controller did appear.
private var didAppearSubject = CurrentValueSubject<Bool, Never>(false)

/// The processor that manages application level logic.
private var appProcessor: AppProcessor?

/// The context of the credential provider to see how the extension is being used.
private var context: CredentialProviderContext?

override func viewDidAppear(_ animated: Bool) {
super.viewDidAppear(animated)

didAppearSubject.send(true)
}

// MARK: ASCredentialProviderViewController

override func prepareCredentialList(for serviceIdentifiers: [ASCredentialServiceIdentifier]) {
Expand Down Expand Up @@ -307,6 +317,12 @@ extension CredentialProviderViewController: Fido2AppExtensionDelegate {
extensionContext.completeRegistrationRequest(using: asPasskeyRegistrationCredential)
}

func getDidAppearPublisher() -> AsyncPublisher<AnyPublisher<Bool, Never>> {
didAppearSubject
.eraseToAnyPublisher()
.values
}

func setUserInteractionRequired() {
context?.flowFailedBecauseUserInteractionRequired = true
cancel(error: ASExtensionError(.userInteractionRequired))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1579,8 +1579,6 @@ class AuthRepositoryTests: BitwardenTestCase { // swiftlint:disable:this type_bo

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

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

XCTAssertFalse(isPinValid)
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import AuthenticationServices
import Combine

/// A delegate that is used to handle actions and retrieve information from within an Autofill extension
/// on Fido2 flows.
Expand All @@ -19,6 +20,9 @@ public protocol Fido2AppExtensionDelegate: AppExtensionDelegate {
@available(iOSApplicationExtension 17.0, *)
func completeRegistrationRequest(asPasskeyRegistrationCredential: ASPasskeyRegistrationCredential)

/// Gets a publisher for when `didAppear` happens.
func getDidAppearPublisher() -> AsyncPublisher<AnyPublisher<Bool, Never>>

/// Marks that user interaction is required.
func setUserInteractionRequired()
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import AuthenticationServices
import Combine
import Foundation

@testable import BitwardenShared
Expand All @@ -8,6 +9,7 @@ class MockFido2AppExtensionDelegate: MockAppExtensionDelegate, Fido2AppExtension
var completeAssertionRequestMocker = InvocationMocker<ASPasskeyAssertionCredential>()
var completeRegistrationRequestMocker = InvocationMocker<ASPasskeyRegistrationCredential>()
var extensionMode: AutofillExtensionMode = .configureAutofill
var didAppearPublisher = CurrentValueSubject<Bool, Never>(false)
var setUserInteractionRequiredCalled = false

var flowWithUserInteraction: Bool = true
Expand All @@ -20,6 +22,12 @@ class MockFido2AppExtensionDelegate: MockAppExtensionDelegate, Fido2AppExtension
completeRegistrationRequestMocker.invoke(param: asPasskeyRegistrationCredential)
}

func getDidAppearPublisher() -> AsyncPublisher<AnyPublisher<Bool, Never>> {
didAppearPublisher
.eraseToAnyPublisher()
.values
}

func setUserInteractionRequired() {
setUserInteractionRequiredCalled = true
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -129,9 +129,15 @@ class AppProcessorFido2Tests: BitwardenTestCase {
func test_onNeedsUserInteraction_flowWithUserInteraction() async {
appExtensionDelegate.flowWithUserInteraction = true

await assertAsyncDoesNotThrow {
let taskResult = Task {
try await subject.onNeedsUserInteraction()
}

appExtensionDelegate.didAppearPublisher.send(true)

await assertAsyncDoesNotThrow {
try await taskResult.value
}
XCTAssertFalse(appExtensionDelegate.setUserInteractionRequiredCalled)
}

Expand Down
17 changes: 15 additions & 2 deletions BitwardenShared/UI/Platform/Application/AppProcessor.swift
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import AuthenticationServices
import BitwardenSdk
import Combine
import Foundation
import OSLog
import UIKit

/// The `AppProcessor` processes actions received at the application level and contains the logic
Expand Down Expand Up @@ -423,11 +424,23 @@ extension AppProcessor: Fido2UserInterfaceHelperDelegate {
}

func onNeedsUserInteraction() async throws {
if let fido2AppExtensionDelegate = appExtensionDelegate as? Fido2AppExtensionDelegate,
!fido2AppExtensionDelegate.flowWithUserInteraction {
guard let fido2AppExtensionDelegate = appExtensionDelegate as? Fido2AppExtensionDelegate else {
return
}

if !fido2AppExtensionDelegate.flowWithUserInteraction {
fido2AppExtensionDelegate.setUserInteractionRequired()
throw Fido2Error.userInteractionRequired
}

// WORKAROUND: We need to wait until the view controller appears in order to perform any
// action that needs user interaction or it might not show the prompt to the user.
// E.g. without this there are certain devices that don't show the FaceID prompt
// and the user only sees the screen dimming a bit and failing the flow.
for await didAppear in fido2AppExtensionDelegate.getDidAppearPublisher() {
guard didAppear else { continue }
return
}
}

func showAlert(_ alert: Alert) {
Expand Down

0 comments on commit 72a2018

Please sign in to comment.