Allow WebAuthn .create() to wait for previous cancel requests to finish

https://bugs.webkit.org/show_bug.cgi?id=257176 rdar://109936742 Reviewed by Pascoe. Allow WebAuthn .create() to monitor and wait for previous cancel requests to finish, to avoid conflicts * Source/WebCore/Modules/webauthn/AuthenticatorCoordinator.cpp: (WebCore::AuthenticatorCoordinator::create): (WebCore::AuthenticatorCoordinator::discoverFromExternalSource): * Source/WebCore/Modules/webauthn/AuthenticatorCoordinator.h: * Source/WebCore/Modules/webauthn/AuthenticatorCoordinatorClient.h: * Source/WebKit/UIProcess/WebAuthentication/Cocoa/WebAuthenticatorCoordinatorProxy.mm: (WebKit::WebAuthenticatorCoordinatorProxy::performRequest): * Source/WebKit/UIProcess/WebAuthentication/WebAuthenticatorCoordinatorProxy.cpp: (WebKit::WebAuthenticatorCoordinatorProxy::~WebAuthenticatorCoordinatorProxy): (WebKit::WebAuthenticatorCoordinatorProxy::cancel): * Source/WebKit/UIProcess/WebAuthentication/WebAuthenticatorCoordinatorProxy.h: * Source/WebKit/UIProcess/WebAuthentication/WebAuthenticatorCoordinatorProxy.messages.in: * Source/WebKit/WebProcess/WebAuthentication/WebAuthenticatorCoordinator.cpp: (WebKit::WebAuthenticatorCoordinator::cancel): * Source/WebKit/WebProcess/WebAuthentication/WebAuthenticatorCoordinator.h: Canonical link: https://commits.webkit.org/273918@main
WebKit · Feb 1, 2024 · ded025f · ded025f
1 parent 4fe554a
commit ded025f
Show file tree

Hide file tree

Showing 9 changed files with 86 additions and 19 deletions.
diff --git a/Source/WebCore/Modules/webauthn/AuthenticatorCoordinator.cpp b/Source/WebCore/Modules/webauthn/AuthenticatorCoordinator.cpp
@@ -177,6 +177,22 @@ void AuthenticatorCoordinator::create(const Document& document, CredentialCreati
         return;
     }
 
+    if (createOptions.signal) {
+        createOptions.signal->addAlgorithm([weakThis = WeakPtr { *this }](JSC::JSValue) mutable {
+            if (!weakThis)
+                return;
+            weakThis->m_isCancelling = true;
+            weakThis->m_client->cancel([weakThis = WTFMove(weakThis)] () mutable {
+                if (!weakThis)
+                    return;
+                if (auto queuedRequest = WTFMove(weakThis->m_queuedRequest)) {
+                    weakThis->m_isCancelling = false;
+                    queuedRequest();
+                }
+            });
+        });
+    }
+
     auto callback = [weakThis = WeakPtr { *this }, clientDataJson = WTFMove(clientDataJson), promise = WTFMove(promise), abortSignal = WTFMove(abortSignal)] (AuthenticatorResponseData&& data, AuthenticatorAttachment attachment, ExceptionData&& exception) mutable {
         if (abortSignal && abortSignal->aborted()) {
             promise.reject(Exception { ExceptionCode::AbortError, "Aborted by AbortSignal."_s });
@@ -191,8 +207,21 @@ void AuthenticatorCoordinator::create(const Document& document, CredentialCreati
         ASSERT(!exception.message.isNull());
         promise.reject(exception.toException());
     };
+
+    if (m_isCancelling) {
+        m_queuedRequest = [weakThis = WeakPtr { *this }, weakFrame = WeakPtr { *frame }, clientDataJsonHash = WTFMove(clientDataJsonHash), createOptions = WTFMove(createOptions), callback = WTFMove(callback)]() mutable {
+            if (!weakThis || !weakFrame)
+                return;
+            const auto options = createOptions.publicKey.value();
+            RefPtr frame = weakFrame.get();
+            if (!frame)
+                return;
+            weakThis->m_client->makeCredential(*weakFrame, clientDataJsonHash, options, createOptions.mediation, WTFMove(callback));
+        };
+        return;
+    }
     // Async operations are dispatched and handled in the messenger.
-    m_client->makeCredential(*frame, callerOrigin, clientDataJsonHash, options, createOptions.mediation, WTFMove(callback));
+    m_client->makeCredential(*frame, clientDataJsonHash, options, createOptions.mediation, WTFMove(callback));
 }
 
 void AuthenticatorCoordinator::discoverFromExternalSource(const Document& document, CredentialRequestOptions&& requestOptions, const ScopeAndCrossOriginParent& scopeAndCrossOriginParent, CredentialPromise&& promise)
@@ -251,10 +280,18 @@ void AuthenticatorCoordinator::discoverFromExternalSource(const Document& docume
     }
 
     if (requestOptions.signal) {
-        requestOptions.signal->addAlgorithm([weakThis = WeakPtr { *this }](JSC::JSValue) {
+        requestOptions.signal->addAlgorithm([weakThis = WeakPtr { *this }](JSC::JSValue) mutable {
             if (!weakThis)
                 return;
-            weakThis->m_client->cancel();
+            weakThis->m_isCancelling = true;
+            weakThis->m_client->cancel([weakThis = WTFMove(weakThis)] () mutable {
+                if (!weakThis)
+                    return;
+                if (auto queuedRequest = WTFMove(weakThis->m_queuedRequest)) {
+                    weakThis->m_isCancelling = false;
+                    queuedRequest();
+                }
+            });
         });
     }
 
@@ -272,8 +309,21 @@ void AuthenticatorCoordinator::discoverFromExternalSource(const Document& docume
         ASSERT(!exception.message.isNull());
         promise.reject(exception.toException());
     };
+
+    if (m_isCancelling) {
+        m_queuedRequest = [weakThis = WeakPtr { *this }, weakFrame = WeakPtr { *frame }, clientDataJsonHash = WTFMove(clientDataJsonHash), requestOptions = WTFMove(requestOptions), scopeAndCrossOriginParent, callback = WTFMove(callback)]() mutable {
+            if (!weakThis || !weakFrame)
+                return;
+            const auto options = requestOptions.publicKey.value();
+            RefPtr frame = weakFrame.get();
+            if (!frame)
+                return;
+            weakThis->m_client->getAssertion(*weakFrame, clientDataJsonHash, options, requestOptions.mediation, scopeAndCrossOriginParent, WTFMove(callback));
+        };
+        return;
+    }
     // Async operations are dispatched and handled in the messenger.
-    m_client->getAssertion(*frame, callerOrigin, clientDataJsonHash, options, requestOptions.mediation, scopeAndCrossOriginParent, WTFMove(callback));
+    m_client->getAssertion(*frame, clientDataJsonHash, options, requestOptions.mediation, scopeAndCrossOriginParent, WTFMove(callback));
 }
 
 void AuthenticatorCoordinator::isUserVerifyingPlatformAuthenticatorAvailable(const Document& document, DOMPromiseDeferred<IDLBoolean>&& promise) const

diff --git a/Source/WebCore/Modules/webauthn/AuthenticatorCoordinator.h b/Source/WebCore/Modules/webauthn/AuthenticatorCoordinator.h
@@ -28,6 +28,7 @@
 #if ENABLE(WEB_AUTHN)
 
 #include "IDLTypes.h"
+#include <wtf/CompletionHandler.h>
 #include <wtf/Forward.h>
 #include <wtf/Noncopyable.h>
 #include <wtf/WeakPtr.h>
@@ -74,6 +75,8 @@ class AuthenticatorCoordinator final : public CanMakeWeakPtr<AuthenticatorCoordi
     AuthenticatorCoordinator() = default;
 
     std::unique_ptr<AuthenticatorCoordinatorClient> m_client;
+    bool m_isCancelling = false;
+    CompletionHandler<void()> m_queuedRequest;
 };
 
 } // namespace WebCore

diff --git a/Source/WebCore/Modules/webauthn/AuthenticatorCoordinatorClient.h b/Source/WebCore/Modules/webauthn/AuthenticatorCoordinatorClient.h
@@ -61,12 +61,12 @@ class AuthenticatorCoordinatorClient : public CanMakeWeakPtr<AuthenticatorCoordi
     AuthenticatorCoordinatorClient() = default;
     virtual ~AuthenticatorCoordinatorClient() = default;
 
-    virtual void makeCredential(const LocalFrame&, const SecurityOrigin&, const Vector<uint8_t>&, const PublicKeyCredentialCreationOptions&, MediationRequirement, RequestCompletionHandler&&) = 0;
-    virtual void getAssertion(const LocalFrame&, const SecurityOrigin&, const Vector<uint8_t>&, const PublicKeyCredentialRequestOptions&, MediationRequirement, const ScopeAndCrossOriginParent&, RequestCompletionHandler&&) = 0;
+    virtual void makeCredential(const LocalFrame&, const Vector<uint8_t>&, const PublicKeyCredentialCreationOptions&, MediationRequirement, RequestCompletionHandler&&) = 0;
+    virtual void getAssertion(const LocalFrame&, const Vector<uint8_t>&, const PublicKeyCredentialRequestOptions&, MediationRequirement, const ScopeAndCrossOriginParent&, RequestCompletionHandler&&) = 0;
     virtual void isConditionalMediationAvailable(const SecurityOrigin&, QueryCompletionHandler&&) = 0;
     virtual void isUserVerifyingPlatformAuthenticatorAvailable(const SecurityOrigin&, QueryCompletionHandler&&) = 0;
     virtual void getClientCapabilities(const SecurityOrigin&, CapabilitiesCompletionHandler&&) = 0;
-    virtual void cancel() = 0;
+    virtual void cancel(CompletionHandler<void()>&&) = 0;
 };
 
 } // namespace WebCore

diff --git a/Source/WebKit/UIProcess/WebAuthentication/Cocoa/WebAuthenticatorCoordinatorProxy.mm b/Source/WebKit/UIProcess/WebAuthentication/Cocoa/WebAuthenticatorCoordinatorProxy.mm
@@ -326,6 +326,9 @@ - (void)authorizationController:(ASAuthorizationController *)controller didCompl
                 response.signature = toArrayBuffer(credential.get().signature);
                 response.userHandle = toArrayBuffer(credential.get().userID);
             }
+            if (auto cancelHandler = WTFMove(m_cancelHandler))
+                cancelHandler();
+
             if (!m_paused) {
                 m_completionHandler(response, attachment, exceptionData);
                 m_delegate.clear();
@@ -915,8 +918,17 @@ static inline void getArePasskeysDisallowedForRelyingParty(const WebCore::Securi
     });
 }
 
-void WebAuthenticatorCoordinatorProxy::cancel()
+void WebAuthenticatorCoordinatorProxy::cancel(CompletionHandler<void()>&& handler)
 {
+#if HAVE(WEB_AUTHN_AS_MODERN)
+    if (m_completionHandler || m_delegate) {
+#else
+    if (m_proxy) {
+#endif
+        m_cancelHandler = WTFMove(handler);
+    } else
+        handler();
+
     if (m_proxy) {
         [m_proxy cancelCurrentRequest];
         m_proxy.clear();

diff --git a/Source/WebKit/UIProcess/WebAuthentication/WebAuthenticatorCoordinatorProxy.cpp b/Source/WebKit/UIProcess/WebAuthentication/WebAuthenticatorCoordinatorProxy.cpp
@@ -55,7 +55,7 @@ WebAuthenticatorCoordinatorProxy::WebAuthenticatorCoordinatorProxy(WebPageProxy&
 WebAuthenticatorCoordinatorProxy::~WebAuthenticatorCoordinatorProxy()
 {
 #if HAVE(UNIFIED_ASC_AUTH_UI)
-    cancel();
+    cancel([]() { });
 #endif // HAVE(UNIFIED_ASC_AUTH_UI)
     m_webPageProxy.process().removeMessageReceiver(Messages::WebAuthenticatorCoordinatorProxy::messageReceiverName(), m_webPageProxy.webPageID());
 }
@@ -127,8 +127,9 @@ void WebAuthenticatorCoordinatorProxy::handleRequest(WebAuthenticationRequestDat
 
 
 #if !HAVE(UNIFIED_ASC_AUTH_UI)
-void WebAuthenticatorCoordinatorProxy::cancel()
+void WebAuthenticatorCoordinatorProxy::cancel(CompletionHandler<void()>&& completionHandler)
 {
+    completionHandler();
 }
 
 void WebAuthenticatorCoordinatorProxy::isUserVerifyingPlatformAuthenticatorAvailable(const SecurityOriginData&, QueryCompletionHandler&& handler)

diff --git a/Source/WebKit/UIProcess/WebAuthentication/WebAuthenticatorCoordinatorProxy.h b/Source/WebKit/UIProcess/WebAuthentication/WebAuthenticatorCoordinatorProxy.h
@@ -94,7 +94,7 @@ class WebAuthenticatorCoordinatorProxy : public IPC::MessageReceiver {
     void isUserVerifyingPlatformAuthenticatorAvailable(const WebCore::SecurityOriginData&, QueryCompletionHandler&&);
     void isConditionalMediationAvailable(const WebCore::SecurityOriginData&, QueryCompletionHandler&&);
     void getClientCapabilities(const WebCore::SecurityOriginData&, CapabilitiesCompletionHandler&&);
-    void cancel();
+    void cancel(CompletionHandler<void()>&&);
 
     void handleRequest(WebAuthenticationRequestData&&, RequestCompletionHandler&&);
 
@@ -124,6 +124,7 @@ class WebAuthenticatorCoordinatorProxy : public IPC::MessageReceiver {
 
     RetainPtr<ASCAuthorizationRemotePresenter> m_presenter;
     RetainPtr<ASCAgentProxy> m_proxy;
+    CompletionHandler<void()> m_cancelHandler;
 #endif // HAVE(UNIFIED_ASC_AUTH_UI)
 };
 

diff --git a/Source/WebKit/UIProcess/WebAuthentication/WebAuthenticatorCoordinatorProxy.messages.in b/Source/WebKit/UIProcess/WebAuthentication/WebAuthenticatorCoordinatorProxy.messages.in
@@ -31,7 +31,7 @@ messages -> WebAuthenticatorCoordinatorProxy NotRefCounted {
     isConditionalMediationAvailable(WebCore::SecurityOriginData origin) -> (bool result)
     IsUserVerifyingPlatformAuthenticatorAvailable(WebCore::SecurityOriginData origin) -> (bool result)
     GetClientCapabilities(WebCore::SecurityOriginData origin) -> (Vector<KeyValuePair<String, bool>> result)
-    Cancel()
+    Cancel() -> ()
 }
 
 #endif
diff --git a/Source/WebKit/WebProcess/WebAuthentication/WebAuthenticatorCoordinator.cpp b/Source/WebKit/WebProcess/WebAuthentication/WebAuthenticatorCoordinator.cpp
@@ -67,7 +67,7 @@ WebAuthenticatorCoordinator::WebAuthenticatorCoordinator(WebPage& webPage)
 {
 }
 
-void WebAuthenticatorCoordinator::makeCredential(const LocalFrame& frame, const SecurityOrigin&, const Vector<uint8_t>& hash, const PublicKeyCredentialCreationOptions& options, MediationRequirement mediation, RequestCompletionHandler&& handler)
+void WebAuthenticatorCoordinator::makeCredential(const LocalFrame& frame, const Vector<uint8_t>& hash, const PublicKeyCredentialCreationOptions& options, MediationRequirement mediation, RequestCompletionHandler&& handler)
 {
     auto webFrame = WebFrame::fromCoreFrame(frame);
     if (!webFrame)
@@ -76,7 +76,7 @@ void WebAuthenticatorCoordinator::makeCredential(const LocalFrame& frame, const
     m_webPage.sendWithAsyncReply(Messages::WebAuthenticatorCoordinatorProxy::MakeCredential(webFrame->frameID(), webFrame->info(), hash, options), WTFMove(handler));
 }
 
-void WebAuthenticatorCoordinator::getAssertion(const LocalFrame& frame, const SecurityOrigin&, const Vector<uint8_t>& hash, const PublicKeyCredentialRequestOptions& options, MediationRequirement mediation, const ScopeAndCrossOriginParent& scopeAndCrossOriginParent, RequestCompletionHandler&& handler)
+void WebAuthenticatorCoordinator::getAssertion(const LocalFrame& frame, const Vector<uint8_t>& hash, const PublicKeyCredentialRequestOptions& options, MediationRequirement mediation, const ScopeAndCrossOriginParent& scopeAndCrossOriginParent, RequestCompletionHandler&& handler)
 {
     auto webFrame = WebFrame::fromCoreFrame(frame);
     if (!webFrame)
@@ -95,9 +95,9 @@ void WebAuthenticatorCoordinator::isUserVerifyingPlatformAuthenticatorAvailable(
     m_webPage.sendWithAsyncReply(Messages::WebAuthenticatorCoordinatorProxy::IsUserVerifyingPlatformAuthenticatorAvailable(origin.data()), WTFMove(handler));
 }
 
-void WebAuthenticatorCoordinator::cancel()
+void WebAuthenticatorCoordinator::cancel(CompletionHandler<void()>&& handler)
 {
-    m_webPage.send(Messages::WebAuthenticatorCoordinatorProxy::Cancel());
+    m_webPage.sendWithAsyncReply(Messages::WebAuthenticatorCoordinatorProxy::Cancel(), WTFMove(handler));
 }
 
 void WebAuthenticatorCoordinator::getClientCapabilities(const SecurityOrigin& origin, CapabilitiesCompletionHandler&& handler)

diff --git a/Source/WebKit/WebProcess/WebAuthentication/WebAuthenticatorCoordinator.h b/Source/WebKit/WebProcess/WebAuthentication/WebAuthenticatorCoordinator.h
@@ -40,12 +40,12 @@ class WebAuthenticatorCoordinator final : public WebCore::AuthenticatorCoordinat
 
 private:
     // WebCore::AuthenticatorCoordinatorClient
-    void makeCredential(const WebCore::LocalFrame&, const WebCore::SecurityOrigin&, const Vector<uint8_t>&, const WebCore::PublicKeyCredentialCreationOptions&, WebCore::MediationRequirement, WebCore::RequestCompletionHandler&&) final;
-    void getAssertion(const WebCore::LocalFrame&, const WebCore::SecurityOrigin&, const Vector<uint8_t>& hash, const WebCore::PublicKeyCredentialRequestOptions&, WebCore::MediationRequirement, const std::pair<WebAuthn::Scope, std::optional<WebCore::SecurityOriginData>>&, WebCore::RequestCompletionHandler&&) final;
+    void makeCredential(const WebCore::LocalFrame&, const Vector<uint8_t>&, const WebCore::PublicKeyCredentialCreationOptions&, WebCore::MediationRequirement, WebCore::RequestCompletionHandler&&) final;
+    void getAssertion(const WebCore::LocalFrame&, const Vector<uint8_t>& hash, const WebCore::PublicKeyCredentialRequestOptions&, WebCore::MediationRequirement, const std::pair<WebAuthn::Scope, std::optional<WebCore::SecurityOriginData>>&, WebCore::RequestCompletionHandler&&) final;
     void isConditionalMediationAvailable(const WebCore::SecurityOrigin&, WebCore::QueryCompletionHandler&&) final;
     void isUserVerifyingPlatformAuthenticatorAvailable(const WebCore::SecurityOrigin&, WebCore::QueryCompletionHandler&&) final;
     void getClientCapabilities(const WebCore::SecurityOrigin&, WebCore::CapabilitiesCompletionHandler&&) final;
-    void cancel() final;
+    void cancel(CompletionHandler<void()>&&) final;
 
     WebPage& m_webPage;
 };