Attachment 369216 Details for Bug 191517 – Patch

[patch] Patch

bug-191517-20190506185808.patch (text/plain), 37.36 KB, created by Jiewen Tan on 2019-05-06 18:58:09 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Jiewen Tan

Created: 2019-05-06 18:58:09 PDT

Size: 37.36 KB

patch

obsolete

>Subversion Revision: 244938
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index 5bcf7c4065aeac11d4432e0027407933aa3af51a..a9eb43a23d669e967afcb2d83b95c1dc10786262 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,24 @@
>+2019-05-06  Jiewen Tan  <jiewen_tan@apple.com>
>+
>+        [WebAuthN] A new request should always suppress the pending request if any
>+        https://bugs.webkit.org/show_bug.cgi?id=191517
>+        <rdar://problem/46888222>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Blocking new requests from the same page when there is a pending request could DoS the
>+        WebAuthN API in the period between [the page is refreshed, the pending request is
>+        hanedled/timeout]. Therefore, the policy is then made to always suppress the pending
>+        request whenever there is a new one to keep the policy of handling one request at a time.
>+
>+        Covered by new tests in existing files.
>+
>+        * Modules/webauthn/AuthenticatorCoordinatorClient.cpp:
>+        (WebCore::AuthenticatorCoordinatorClient::requestReply):
>+        (WebCore::AuthenticatorCoordinatorClient::setRequestCompletionHandler):
>+        (WebCore::AuthenticatorCoordinatorClient::addQueryCompletionHandler):
>+        * Modules/webauthn/AuthenticatorCoordinatorClient.h:
>+
> 2019-05-02  Jiewen Tan  <jiewen_tan@apple.com>
> 
>         [WebAuthN] A focused document should be required
>diff --git a/Source/WebKit/ChangeLog b/Source/WebKit/ChangeLog
>index dce1b4f002946f33e69d79fc09af50be207c6403..1145857b38d26ec57cd747b95d4fae9369152371 100644
>--- a/Source/WebKit/ChangeLog
>+++ b/Source/WebKit/ChangeLog
>@@ -1,3 +1,58 @@
>+2019-05-06  Jiewen Tan  <jiewen_tan@apple.com>
>+
>+        [WebAuthN] A new request should always suppress the pending request if any
>+        https://bugs.webkit.org/show_bug.cgi?id=191517
>+        <rdar://problem/46888222>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Back then, blocking new requests from random web pages in UI Process when there
>+        is a pending request is good to prevent background tabs from DoS the foreground
>+        tab. However, in r244938, WebAuthN API is restircted to only allow focused document
>+        to call. Therefore, the above issue doesn't exist anymore.
>+
>+        Apart from the vanished benefit, this behavoir actually blocks new pages to use
>+        WebAuhtN API in the period between [the previous initating page is closed, the pending
>+        request is hanedled/timeout].
>+
>+        Also, it makes sense to have the current focused document to preempt the pending request.
>+        Therefore, the policy is then made to always suppress the pending request whenever
>+        there is a new one to keep the policy of handling one request at a time.
>+
>+        The current implementation doesn't cancel the pending request in any authenticators explicitly.
>+        Therefore, the new request could potential not reach the authenticator if it is busy handling
>+        the previous one. Since the authenticator could be busy hanldling reqeusts from other applications
>+        at anytime, this case is no much difference than it. Hence, there is no incentives to address
>+        this issue at this moment. It will be addressed in Bug 191523.
>+
>+        For atomicity of the request/response pair, i.e, preventing using the old response for the new request,
>+        there are two safe guards:
>+        1) In web process, each request to UI process is paired with an incremental ID, and therefore an old
>+        response from UI process would have a different ID than the current request, which will then be ignored.
>+        2) In UI process, all responses from authenticators will be piped to the main run loop for processing.
>+        Therefore, when the new request comes in, the old response is either processed or waiting in the pipe.
>+        To prevent the latter being processed, the new request will immediately destroy any authenticators bound
>+        to the old response in the current run loop. Hence, in the next run loop when dealing the old response,
>+        the lambda will have no where to hand the response over.
>+
>+        * UIProcess/WebAuthentication/AuthenticatorManager.cpp:
>+        (WebKit::AuthenticatorManager::makeCredential):
>+        (WebKit::AuthenticatorManager::getAssertion):
>+        (WebKit::AuthenticatorManager::clearStateAsync):
>+        (WebKit::AuthenticatorManager::clearState):
>+        (WebKit::AuthenticatorManager::timeOutTimerFired):
>+        * UIProcess/WebAuthentication/AuthenticatorManager.h:
>+        * UIProcess/WebAuthentication/WebAuthenticatorCoordinatorProxy.cpp:
>+        (WebKit::WebAuthenticatorCoordinatorProxy::makeCredential):
>+        (WebKit::WebAuthenticatorCoordinatorProxy::getAssertion):
>+        (WebKit::WebAuthenticatorCoordinatorProxy::requestReply):
>+        * UIProcess/WebAuthentication/WebAuthenticatorCoordinatorProxy.h:
>+        * UIProcess/WebAuthentication/WebAuthenticatorCoordinatorProxy.messages.in:
>+        * WebProcess/WebAuthentication/WebAuthenticatorCoordinator.cpp:
>+        (WebKit::WebAuthenticatorCoordinator::makeCredential):
>+        (WebKit::WebAuthenticatorCoordinator::getAssertion):
>+        * WebProcess/WebAuthentication/WebAuthenticatorCoordinator.messages.in:
>+
> 2019-05-03  Chris Dumez  <cdumez@apple.com>
> 
>         Unresponsive Service Worker processes should get killed
>diff --git a/Source/WebCore/Modules/webauthn/AuthenticatorCoordinatorClient.cpp b/Source/WebCore/Modules/webauthn/AuthenticatorCoordinatorClient.cpp
>index 3c658a32fcbbd0d17a711dfaf2bb6a1926a132a9..e1df91287de527bf07a4482f39168e71440bd635 100644
>--- a/Source/WebCore/Modules/webauthn/AuthenticatorCoordinatorClient.cpp
>+++ b/Source/WebCore/Modules/webauthn/AuthenticatorCoordinatorClient.cpp
>@@ -32,8 +32,10 @@
> 
> namespace WebCore {
> 
>-void AuthenticatorCoordinatorClient::requestReply(const WebCore::PublicKeyCredentialData& data, const WebCore::ExceptionData& exception)
>+void AuthenticatorCoordinatorClient::requestReply(uint64_t messageId, const WebCore::PublicKeyCredentialData& data, const WebCore::ExceptionData& exception)
> {
>+    if (messageId != m_accumulatedRequestMessageId - 1)
>+        return;
>     m_pendingCompletionHandler(data, exception);
> }
> 
>@@ -43,20 +45,18 @@ void AuthenticatorCoordinatorClient::isUserVerifyingPlatformAuthenticatorAvailab
>     handler(result);
> }
> 
>-bool AuthenticatorCoordinatorClient::setRequestCompletionHandler(RequestCompletionHandler&& handler)
>+uint64_t AuthenticatorCoordinatorClient::setRequestCompletionHandler(RequestCompletionHandler&& handler)
> {
>-    if (m_pendingCompletionHandler) {
>-        handler({ }, { NotAllowedError, "A request is pending."_s });
>-        return false;
>-    }
>+    if (m_pendingCompletionHandler)
>+        m_pendingCompletionHandler({ }, { NotAllowedError, "This request is suppressed by a new request."_s });
> 
>     m_pendingCompletionHandler = WTFMove(handler);
>-    return true;
>+    return m_accumulatedRequestMessageId++;
> }
> 
> uint64_t AuthenticatorCoordinatorClient::addQueryCompletionHandler(QueryCompletionHandler&& handler)
> {
>-    uint64_t messageId = m_accumulatedMessageId++;
>+    uint64_t messageId = m_accumulatedQueryMessageId++;
>     auto addResult = m_pendingQueryCompletionHandlers.add(messageId, WTFMove(handler));
>     ASSERT_UNUSED(addResult, addResult.isNewEntry);
>     return messageId;
>diff --git a/Source/WebCore/Modules/webauthn/AuthenticatorCoordinatorClient.h b/Source/WebCore/Modules/webauthn/AuthenticatorCoordinatorClient.h
>index 162aaea8b4c406b38c63e737a967adacc787f9b9..5bc442f299600665a6db14ee88c068ad1c0c600e 100644
>--- a/Source/WebCore/Modules/webauthn/AuthenticatorCoordinatorClient.h
>+++ b/Source/WebCore/Modules/webauthn/AuthenticatorCoordinatorClient.h
>@@ -56,18 +56,19 @@ public:
>     virtual void isUserVerifyingPlatformAuthenticatorAvailable(QueryCompletionHandler&&) = 0;
> 
>     // Receivers.
>-    void requestReply(const WebCore::PublicKeyCredentialData&, const WebCore::ExceptionData&);
>+    void requestReply(uint64_t messageId, const WebCore::PublicKeyCredentialData&, const WebCore::ExceptionData&);
>     void isUserVerifyingPlatformAuthenticatorAvailableReply(uint64_t messageId, bool);
> 
> protected:
>-    // Only one request is allowed at one time. It returns false whenever there is an existing pending request.
>-    // And invokes the provided handler with NotAllowedError.
>-    bool setRequestCompletionHandler(RequestCompletionHandler&&);
>+    // Only one request is allowed at one time. A new reqeuest will suppress the pending request if any.
>+    // A message id that is tied to the request wil be generated each time to prevent mismatching responses.
>+    uint64_t setRequestCompletionHandler(RequestCompletionHandler&&);
>     uint64_t addQueryCompletionHandler(QueryCompletionHandler&&);
> 
> private:
>+    uint64_t m_accumulatedRequestMessageId { 1 };
>     RequestCompletionHandler m_pendingCompletionHandler;
>-    uint64_t m_accumulatedMessageId { 1 };
>+    uint64_t m_accumulatedQueryMessageId { 1 };
>     HashMap<uint64_t, QueryCompletionHandler> m_pendingQueryCompletionHandlers;
> };
> 
>diff --git a/Source/WebKit/UIProcess/WebAuthentication/AuthenticatorManager.cpp b/Source/WebKit/UIProcess/WebAuthentication/AuthenticatorManager.cpp
>index 84e28a8983c2c7ab8eeeca87103d6eee5545e35f..aae9ccbacfe961e698e0296e6afe3934f6ed3f05 100644
>--- a/Source/WebKit/UIProcess/WebAuthentication/AuthenticatorManager.cpp
>+++ b/Source/WebKit/UIProcess/WebAuthentication/AuthenticatorManager.cpp
>@@ -129,8 +129,9 @@ void AuthenticatorManager::makeCredential(const Vector<uint8_t>& hash, const Pub
>     using namespace AuthenticatorManagerInternal;
> 
>     if (m_pendingCompletionHandler) {
>-        callback(ExceptionData { NotAllowedError, "A request is pending."_s });
>-        return;
>+        m_pendingCompletionHandler(ExceptionData { NotAllowedError, "This request is cancelled by a new request."_s });
>+        clearState();
>+        m_requestTimeOutTimer.stop();
>     }
> 
>     // 1. Save request for async operations.
>@@ -147,8 +148,9 @@ void AuthenticatorManager::getAssertion(const Vector<uint8_t>& hash, const Publi
>     using namespace AuthenticatorManagerInternal;
> 
>     if (m_pendingCompletionHandler) {
>-        callback(ExceptionData { NotAllowedError, "A request is pending."_s });
>-        return;
>+        m_pendingCompletionHandler(ExceptionData { NotAllowedError, "This request is cancelled by a new request."_s });
>+        clearState();
>+        m_requestTimeOutTimer.stop();
>     }
> 
>     // 1. Save request for async operations.
>@@ -166,13 +168,18 @@ void AuthenticatorManager::clearStateAsync()
>     RunLoop::main().dispatch([weakThis = makeWeakPtr(*this)] {
>         if (!weakThis)
>             return;
>-        weakThis->m_pendingRequestData = { };
>-        ASSERT(!weakThis->m_pendingCompletionHandler);
>-        weakThis->m_services.clear();
>-        weakThis->m_authenticators.clear();
>+        weakThis->clearState();
>     });
> }
> 
>+void AuthenticatorManager::clearState()
>+{
>+    m_pendingRequestData = { };
>+    ASSERT(!m_pendingCompletionHandler);
>+    m_services.clear();
>+    m_authenticators.clear();
>+}
>+
> void AuthenticatorManager::authenticatorAdded(Ref<Authenticator>&& authenticator)
> {
>     ASSERT(RunLoop::isMain());
>@@ -231,7 +238,7 @@ void AuthenticatorManager::timeOutTimerFired()
> {
>     ASSERT(m_requestTimeOutTimer.isActive());
>     m_pendingCompletionHandler((ExceptionData { NotAllowedError, "Operation timed out."_s }));
>-    clearStateAsync();
>+    clearState();
> }
> 
> } // namespace WebKit
>diff --git a/Source/WebKit/UIProcess/WebAuthentication/AuthenticatorManager.h b/Source/WebKit/UIProcess/WebAuthentication/AuthenticatorManager.h
>index 7fc4a1ebd901a6cc44f3f4068eb6b37cfe48541a..3de42dee88d19cfc5c91b14cfc9fb21f806cea6e 100644
>--- a/Source/WebKit/UIProcess/WebAuthentication/AuthenticatorManager.h
>+++ b/Source/WebKit/UIProcess/WebAuthentication/AuthenticatorManager.h
>@@ -62,6 +62,7 @@ protected:
>     Callback& pendingCompletionHandler() { return m_pendingCompletionHandler; }
>     RunLoop::Timer<AuthenticatorManager>& requestTimeOutTimer() { return m_requestTimeOutTimer; }
>     void clearStateAsync(); // To void cyclic dependence.
>+    void clearState();
> 
> private:
>     // AuthenticatorTransportService::Observer
>@@ -79,7 +80,7 @@ private:
>     void initTimeOutTimer(const Optional<unsigned>& timeOutInMs);
>     void timeOutTimerFired();
> 
>-    // Request: We only allow one request per time.
>+    // Request: We only allow one request per time. A new request will cancel the pending one if any.
>     WebAuthenticationRequestData m_pendingRequestData;
>     Callback m_pendingCompletionHandler;
>     RunLoop::Timer<AuthenticatorManager> m_requestTimeOutTimer;
>diff --git a/Source/WebKit/UIProcess/WebAuthentication/WebAuthenticatorCoordinatorProxy.cpp b/Source/WebKit/UIProcess/WebAuthentication/WebAuthenticatorCoordinatorProxy.cpp
>index d6b8d74e007b812edef09e03beda2fd5a4ed15d7..b5a5c8234d8f45584ad6aa8efd16bdf5583df407 100644
>--- a/Source/WebKit/UIProcess/WebAuthentication/WebAuthenticatorCoordinatorProxy.cpp
>+++ b/Source/WebKit/UIProcess/WebAuthentication/WebAuthenticatorCoordinatorProxy.cpp
>@@ -53,33 +53,33 @@ WebAuthenticatorCoordinatorProxy::~WebAuthenticatorCoordinatorProxy()
>     m_webPageProxy.process().removeMessageReceiver(Messages::WebAuthenticatorCoordinatorProxy::messageReceiverName(), m_webPageProxy.pageID());
> }
> 
>-void WebAuthenticatorCoordinatorProxy::makeCredential(const Vector<uint8_t>& hash, const WebCore::PublicKeyCredentialCreationOptions& options)
>+void WebAuthenticatorCoordinatorProxy::makeCredential(uint64_t messageId, const Vector<uint8_t>& hash, const WebCore::PublicKeyCredentialCreationOptions& options)
> {
>-    auto callback = [weakThis = makeWeakPtr(*this)] (Variant<WebCore::PublicKeyCredentialData, WebCore::ExceptionData>&& result) {
>+    auto callback = [messageId, weakThis = makeWeakPtr(*this)] (Variant<WebCore::PublicKeyCredentialData, WebCore::ExceptionData>&& result) {
>         ASSERT(RunLoop::isMain());
>         if (!weakThis)
>             return;
> 
>         WTF::switchOn(result, [&](const WebCore::PublicKeyCredentialData& data) {
>-            weakThis->requestReply(data, { });
>+            weakThis->requestReply(messageId, data, { });
>         }, [&](const  WebCore::ExceptionData& exception) {
>-            weakThis->requestReply({ }, exception);
>+            weakThis->requestReply(messageId, { }, exception);
>         });
>     };
>     m_webPageProxy.websiteDataStore().authenticatorManager().makeCredential(hash, options, WTFMove(callback));
> }
> 
>-void WebAuthenticatorCoordinatorProxy::getAssertion(const Vector<uint8_t>& hash, const WebCore::PublicKeyCredentialRequestOptions& options)
>+void WebAuthenticatorCoordinatorProxy::getAssertion(uint64_t messageId, const Vector<uint8_t>& hash, const WebCore::PublicKeyCredentialRequestOptions& options)
> {
>-    auto callback = [weakThis = makeWeakPtr(*this)] (Variant<WebCore::PublicKeyCredentialData, WebCore::ExceptionData>&& result) {
>+    auto callback = [messageId, weakThis = makeWeakPtr(*this)] (Variant<WebCore::PublicKeyCredentialData, WebCore::ExceptionData>&& result) {
>         ASSERT(RunLoop::isMain());
>         if (!weakThis)
>             return;
> 
>         WTF::switchOn(result, [&](const WebCore::PublicKeyCredentialData& data) {
>-            weakThis->requestReply(data, { });
>+            weakThis->requestReply(messageId, data, { });
>         }, [&](const  WebCore::ExceptionData& exception) {
>-            weakThis->requestReply({ }, exception);
>+            weakThis->requestReply(messageId, { }, exception);
>         });
>     };
>     m_webPageProxy.websiteDataStore().authenticatorManager().getAssertion(hash, options, WTFMove(callback));
>@@ -90,9 +90,9 @@ void WebAuthenticatorCoordinatorProxy::isUserVerifyingPlatformAuthenticatorAvail
>     m_webPageProxy.send(Messages::WebAuthenticatorCoordinator::IsUserVerifyingPlatformAuthenticatorAvailableReply(messageId, LocalService::isAvailable()));
> }
> 
>-void WebAuthenticatorCoordinatorProxy::requestReply(const WebCore::PublicKeyCredentialData& data, const WebCore::ExceptionData& exception)
>+void WebAuthenticatorCoordinatorProxy::requestReply(uint64_t messageId, const WebCore::PublicKeyCredentialData& data, const WebCore::ExceptionData& exception)
> {
>-    m_webPageProxy.send(Messages::WebAuthenticatorCoordinator::RequestReply(data, exception));
>+    m_webPageProxy.send(Messages::WebAuthenticatorCoordinator::RequestReply(messageId, data, exception));
> }
> 
> } // namespace WebKit
>diff --git a/Source/WebKit/UIProcess/WebAuthentication/WebAuthenticatorCoordinatorProxy.h b/Source/WebKit/UIProcess/WebAuthentication/WebAuthenticatorCoordinatorProxy.h
>index ba1aef5f58b59e382690a93f286176fe904b42d9..85ce6906111442bc679cc7a775b8682dd15728c9 100644
>--- a/Source/WebKit/UIProcess/WebAuthentication/WebAuthenticatorCoordinatorProxy.h
>+++ b/Source/WebKit/UIProcess/WebAuthentication/WebAuthenticatorCoordinatorProxy.h
>@@ -54,12 +54,12 @@ private:
>     void didReceiveMessage(IPC::Connection&, IPC::Decoder&) override;
> 
>     // Receivers.
>-    void makeCredential(const Vector<uint8_t>& hash, const WebCore::PublicKeyCredentialCreationOptions&);
>-    void getAssertion(const Vector<uint8_t>& hash, const WebCore::PublicKeyCredentialRequestOptions&);
>+    void makeCredential(uint64_t messageId, const Vector<uint8_t>& hash, const WebCore::PublicKeyCredentialCreationOptions&);
>+    void getAssertion(uint64_t messageId, const Vector<uint8_t>& hash, const WebCore::PublicKeyCredentialRequestOptions&);
>     void isUserVerifyingPlatformAuthenticatorAvailable(uint64_t messageId);
> 
>     // Senders.
>-    void requestReply(const WebCore::PublicKeyCredentialData&, const WebCore::ExceptionData&);
>+    void requestReply(uint64_t messageId, const WebCore::PublicKeyCredentialData&, const WebCore::ExceptionData&);
> 
>     WebPageProxy& m_webPageProxy;
> };
>diff --git a/Source/WebKit/UIProcess/WebAuthentication/WebAuthenticatorCoordinatorProxy.messages.in b/Source/WebKit/UIProcess/WebAuthentication/WebAuthenticatorCoordinatorProxy.messages.in
>index 87c66776e61a05b1bad5292dd115213aafa3abe1..117cd8808f0de57562c1d81a1182b57d565002b8 100644
>--- a/Source/WebKit/UIProcess/WebAuthentication/WebAuthenticatorCoordinatorProxy.messages.in
>+++ b/Source/WebKit/UIProcess/WebAuthentication/WebAuthenticatorCoordinatorProxy.messages.in
>@@ -26,8 +26,8 @@
> 
> messages -> WebAuthenticatorCoordinatorProxy {
> 
>-    MakeCredential(Vector<uint8_t> hash, struct WebCore::PublicKeyCredentialCreationOptions options);
>-    GetAssertion(Vector<uint8_t> hash, struct WebCore::PublicKeyCredentialRequestOptions options);
>+    MakeCredential(uint64_t messageId, Vector<uint8_t> hash, struct WebCore::PublicKeyCredentialCreationOptions options);
>+    GetAssertion(uint64_t messageId, Vector<uint8_t> hash, struct WebCore::PublicKeyCredentialRequestOptions options);
>     IsUserVerifyingPlatformAuthenticatorAvailable(uint64_t messageId);
> }
> 
>diff --git a/Source/WebKit/WebProcess/WebAuthentication/WebAuthenticatorCoordinator.cpp b/Source/WebKit/WebProcess/WebAuthentication/WebAuthenticatorCoordinator.cpp
>index 90ba4c57c6414a878af27bdb583a07e5a424a8f2..85659166cc21ed96264074b7e8642b2a86218ca5 100644
>--- a/Source/WebKit/WebProcess/WebAuthentication/WebAuthenticatorCoordinator.cpp
>+++ b/Source/WebKit/WebProcess/WebAuthentication/WebAuthenticatorCoordinator.cpp
>@@ -50,18 +50,14 @@ WebAuthenticatorCoordinator::~WebAuthenticatorCoordinator()
> 
> void WebAuthenticatorCoordinator::makeCredential(const Vector<uint8_t>& hash, const WebCore::PublicKeyCredentialCreationOptions& options, WebCore::RequestCompletionHandler&& handler)
> {
>-    if (!setRequestCompletionHandler(WTFMove(handler)))
>-        return;
>-
>-    m_webPage.send(Messages::WebAuthenticatorCoordinatorProxy::MakeCredential(hash, options));
>+    auto messageId = setRequestCompletionHandler(WTFMove(handler));
>+    m_webPage.send(Messages::WebAuthenticatorCoordinatorProxy::MakeCredential(messageId, hash, options));
> }
> 
> void WebAuthenticatorCoordinator::getAssertion(const Vector<uint8_t>& hash, const WebCore::PublicKeyCredentialRequestOptions& options, WebCore::RequestCompletionHandler&& handler)
> {
>-    if (!setRequestCompletionHandler(WTFMove(handler)))
>-        return;
>-
>-    m_webPage.send(Messages::WebAuthenticatorCoordinatorProxy::GetAssertion(hash, options));
>+    auto messageId = setRequestCompletionHandler(WTFMove(handler));
>+    m_webPage.send(Messages::WebAuthenticatorCoordinatorProxy::GetAssertion(messageId, hash, options));
> }
> 
> void WebAuthenticatorCoordinator::isUserVerifyingPlatformAuthenticatorAvailable(WebCore::QueryCompletionHandler&& handler)
>diff --git a/Source/WebKit/WebProcess/WebAuthentication/WebAuthenticatorCoordinator.messages.in b/Source/WebKit/WebProcess/WebAuthentication/WebAuthenticatorCoordinator.messages.in
>index 53ec1935199ffbd93e6c5b9f3966252275fafee1..18700ca3c190384beaa1210cb03be8ed389aa813 100644
>--- a/Source/WebKit/WebProcess/WebAuthentication/WebAuthenticatorCoordinator.messages.in
>+++ b/Source/WebKit/WebProcess/WebAuthentication/WebAuthenticatorCoordinator.messages.in
>@@ -26,7 +26,7 @@
> 
> messages -> WebAuthenticatorCoordinator {
> 
>-    RequestReply(struct WebCore::PublicKeyCredentialData data, struct WebCore::ExceptionData exception);
>+    RequestReply(uint64_t messageId, struct WebCore::PublicKeyCredentialData data, struct WebCore::ExceptionData exception);
>     IsUserVerifyingPlatformAuthenticatorAvailableReply(uint64_t messageId, bool result);
> }
> 
>diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
>index db50337bf1fc1eb2e63ba6da004d866925c7643d..829210661f5066717484d20b25c69fc4591453b5 100644
>--- a/LayoutTests/ChangeLog
>+++ b/LayoutTests/ChangeLog
>@@ -1,3 +1,21 @@
>+2019-05-06  Jiewen Tan  <jiewen_tan@apple.com>
>+
>+        [WebAuthN] A new request should always suppress the pending request if any
>+        https://bugs.webkit.org/show_bug.cgi?id=191517
>+        <rdar://problem/46888222>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        * http/wpt/webauthn/public-key-credential-create-failure.https-expected.txt:
>+        * http/wpt/webauthn/public-key-credential-create-failure.https.html:
>+        * http/wpt/webauthn/public-key-credential-create-success-hid.https-expected.txt:
>+        * http/wpt/webauthn/public-key-credential-create-success-hid.https.html:
>+        * http/wpt/webauthn/public-key-credential-get-failure.https-expected.txt:
>+        * http/wpt/webauthn/public-key-credential-get-failure.https.html:
>+        * http/wpt/webauthn/public-key-credential-get-success-hid.https-expected.txt:
>+        * http/wpt/webauthn/public-key-credential-get-success-hid.https.html:
>+        * http/wpt/webauthn/resources/new-page.html: Added.
>+
> 2019-05-03  Jiewen Tan  <jiewen_tan@apple.com>
> 
>         [WebAuthN] A focused document should be required
>diff --git a/LayoutTests/http/wpt/webauthn/public-key-credential-create-failure.https-expected.txt b/LayoutTests/http/wpt/webauthn/public-key-credential-create-failure.https-expected.txt
>index e2604ae0d9acef2b234fa525849a2902bb5bbef0..de826522d90b03053ec7de6208c30968b588f504 100644
>--- a/LayoutTests/http/wpt/webauthn/public-key-credential-create-failure.https-expected.txt
>+++ b/LayoutTests/http/wpt/webauthn/public-key-credential-create-failure.https-expected.txt
>@@ -2,4 +2,8 @@
> PASS PublicKeyCredential's [[create]] with timeout 
> PASS PublicKeyCredential's [[create]] with a mismatched RP ID 
> PASS PublicKeyCredential's [[create]] with an empty pubKeyCredParams 
>+PASS PublicKeyCredential's [[create]] with two consecutive requests 
>+PASS PublicKeyCredential's [[create]] with two consecutive requests (2) 
>+PASS PublicKeyCredential's [[create]] with new requests in a new page 
>+PASS PublicKeyCredential's [[create]] with new requests in a new page (2) 
> 
>diff --git a/LayoutTests/http/wpt/webauthn/public-key-credential-create-failure.https.html b/LayoutTests/http/wpt/webauthn/public-key-credential-create-failure.https.html
>index 4e131cbe4e2cbb4d71951309352fcd25d5580750..ddf6838195726410aafb0b76f82ead5e00ccbe2c 100644
>--- a/LayoutTests/http/wpt/webauthn/public-key-credential-create-failure.https.html
>+++ b/LayoutTests/http/wpt/webauthn/public-key-credential-create-failure.https.html
>@@ -70,4 +70,93 @@
>         return promiseRejects(t, "NotSupportedError",
>             navigator.credentials.create(options), "No desired properties of the to be created credential are provided.");
>     }, "PublicKeyCredential's [[create]] with an empty pubKeyCredParams");
>+
>+    promise_test(function(t) {
>+        const options = {
>+            publicKey: {
>+                rp: {
>+                    name: "example.com"
>+                },
>+                user: {
>+                    name: "John Appleseed",
>+                    id: asciiToUint8Array("123456"),
>+                    displayName: "John",
>+                },
>+                challenge: asciiToUint8Array("123456"),
>+                pubKeyCredParams: [{ type: "public-key", alg: -7 }],
>+            }
>+        };
>+
>+        const result = promiseRejects(t, "NotAllowedError", navigator.credentials.create(options), "This request is suppressed by a new request.");
>+        promiseRejects(t, "NotAllowedError", navigator.credentials.create(options), "This request is suppressed by a new request.");
>+        return result;
>+    }, "PublicKeyCredential's [[create]] with two consecutive requests");
>+
>+    promise_test(function(t) {
>+        const creationOptions = {
>+            publicKey: {
>+                rp: {
>+                    name: "example.com"
>+                },
>+                user: {
>+                    name: "John Appleseed",
>+                    id: asciiToUint8Array("123456"),
>+                    displayName: "John",
>+                },
>+                challenge: asciiToUint8Array("123456"),
>+                pubKeyCredParams: [{ type: "public-key", alg: -7 }],
>+            }
>+        };
>+        const result = promiseRejects(t, "NotAllowedError", navigator.credentials.create(creationOptions), "This request is suppressed by a new request.");
>+
>+        const requestOptions = {
>+            publicKey: {
>+                challenge: asciiToUint8Array("123456")
>+            }
>+        };
>+        promiseRejects(t, "NotAllowedError", navigator.credentials.get(requestOptions), "This request is suppressed by a new request.");
>+        return result;
>+    }, "PublicKeyCredential's [[create]] with two consecutive requests (2)");
>+
>+    promise_test(function(t) {
>+        const options = {
>+            publicKey: {
>+                rp: {
>+                    name: "example.com"
>+                },
>+                user: {
>+                    name: "John Appleseed",
>+                    id: asciiToUint8Array("123456"),
>+                    displayName: "John",
>+                },
>+                challenge: asciiToUint8Array("123456"),
>+                pubKeyCredParams: [{ type: "public-key", alg: -7 }],
>+            }
>+        };
>+
>+        const result = promiseRejects(t, "NotAllowedError", navigator.credentials.create(options), "This request is cancelled by a new request.");
>+        window.open("./resources/new-page.html?isCreation=true");
>+        return result;
>+    }, "PublicKeyCredential's [[create]] with new requests in a new page");
>+
>+    promise_test(function(t) {
>+        const options = {
>+            publicKey: {
>+                rp: {
>+                    name: "example.com"
>+                },
>+                user: {
>+                    name: "John Appleseed",
>+                    id: asciiToUint8Array("123456"),
>+                    displayName: "John",
>+                },
>+                challenge: asciiToUint8Array("123456"),
>+                pubKeyCredParams: [{ type: "public-key", alg: -7 }],
>+            }
>+        };
>+
>+        const result = promiseRejects(t, "NotAllowedError", navigator.credentials.create(options), "This request is cancelled by a new request.");
>+        window.open("./resources/new-page.html?isCreation=false");
>+        return result;
>+    }, "PublicKeyCredential's [[create]] with new requests in a new page (2)");
> </script>
>diff --git a/LayoutTests/http/wpt/webauthn/public-key-credential-create-success-hid.https-expected.txt b/LayoutTests/http/wpt/webauthn/public-key-credential-create-success-hid.https-expected.txt
>index 27fc4a3258f42eba19e51b35616e7bdec478bc16..59b2df747c7b99ca40d7c7ff0340b3bf189911dd 100644
>--- a/LayoutTests/http/wpt/webauthn/public-key-credential-create-success-hid.https-expected.txt
>+++ b/LayoutTests/http/wpt/webauthn/public-key-credential-create-success-hid.https-expected.txt
>@@ -5,4 +5,5 @@ PASS PublicKeyCredential's [[create]] with requireResidentKey { false } in a moc
> PASS PublicKeyCredential's [[create]] with userVerification { 'preferred' } in a mock local authenticator. 
> PASS PublicKeyCredential's [[create]] with userVerification { 'discouraged' } in a mock local authenticator. 
> PASS PublicKeyCredential's [[create]] with mixed options in a mock local authenticator. 
>+PASS PublicKeyCredential's [[create]] with two consecutive requests. 
> 
>diff --git a/LayoutTests/http/wpt/webauthn/public-key-credential-create-success-hid.https.html b/LayoutTests/http/wpt/webauthn/public-key-credential-create-success-hid.https.html
>index ecb954e7ec1f54e3c6d80bab75f62ad33ed401ee..751eee9b0e5ab742e0515df7a1977dc5fe106016 100644
>--- a/LayoutTests/http/wpt/webauthn/public-key-credential-create-success-hid.https.html
>+++ b/LayoutTests/http/wpt/webauthn/public-key-credential-create-success-hid.https.html
>@@ -170,4 +170,27 @@
>             checkResult(credential);
>         });
>     }, "PublicKeyCredential's [[create]] with mixed options in a mock local authenticator.");
>+
>+    promise_test(t => {
>+        const options = {
>+            publicKey: {
>+                rp: {
>+                    name: "localhost",
>+                },
>+                user: {
>+                    name: "John Appleseed",
>+                    id: Base64URL.parse(testUserhandleBase64),
>+                    displayName: "Appleseed",
>+                },
>+                challenge: Base64URL.parse("MTIzNDU2"),
>+                pubKeyCredParams: [{ type: "public-key", alg: -7 }],
>+                timeout: 100
>+            }
>+        };
>+
>+        promiseRejects(t, "NotAllowedError", navigator.credentials.create(options), "This request is suppressed by a new request.");
>+        return navigator.credentials.create(options).then(credential => {
>+            checkResult(credential);
>+        });
>+    }, "PublicKeyCredential's [[create]] with two consecutive requests.");
> </script>
>diff --git a/LayoutTests/http/wpt/webauthn/public-key-credential-get-failure.https-expected.txt b/LayoutTests/http/wpt/webauthn/public-key-credential-get-failure.https-expected.txt
>index d0fa61f84232f213000cbbc3b01336eefb186635..432ac646ebb149c41fa01c0c0f00617c1617437e 100644
>--- a/LayoutTests/http/wpt/webauthn/public-key-credential-get-failure.https-expected.txt
>+++ b/LayoutTests/http/wpt/webauthn/public-key-credential-get-failure.https-expected.txt
>@@ -5,4 +5,8 @@ PASS PublicKeyCredential's [[get]] with a mismatched APP ID (invalid URLs)
> PASS PublicKeyCredential's [[get]] with a mismatched APP ID (different protocols) 
> PASS PublicKeyCredential's [[get]] with a mismatched APP ID (different sites 1) 
> PASS PublicKeyCredential's [[get]] with a mismatched APP ID (different sites 2) 
>+PASS PublicKeyCredential's [[get]] with two consecutive requests 
>+PASS PublicKeyCredential's [[get]] with two consecutive requests (2) 
>+PASS PublicKeyCredential's [[get]] with new requests in a new page 
>+PASS PublicKeyCredential's [[get]] with new requests in a new page (2) 
> 
>diff --git a/LayoutTests/http/wpt/webauthn/public-key-credential-get-failure.https.html b/LayoutTests/http/wpt/webauthn/public-key-credential-get-failure.https.html
>index 8547fd8e040fbe22d01ebe6f82f58b27986edb89..4214eb20745c10166c3e484e4ac889c822f8058f 100644
>--- a/LayoutTests/http/wpt/webauthn/public-key-credential-get-failure.https.html
>+++ b/LayoutTests/http/wpt/webauthn/public-key-credential-get-failure.https.html
>@@ -79,4 +79,67 @@
>         return promiseRejects(t, "SecurityError",
>             navigator.credentials.get(options), "The origin of the document is not authorized for the provided App ID.");
>     }, "PublicKeyCredential's [[get]] with a mismatched APP ID (different sites 2)");
>+
>+    promise_test(t => {
>+        const options = {
>+            publicKey: {
>+                challenge: asciiToUint8Array("123456")
>+            }
>+        };
>+
>+        const result = promiseRejects(t, "NotAllowedError", navigator.credentials.get(options), "This request is suppressed by a new request.");
>+        promiseRejects(t, "NotAllowedError", navigator.credentials.get(options), "This request is suppressed by a new request.");
>+        return result;
>+    }, "PublicKeyCredential's [[get]] with two consecutive requests");
>+
>+    promise_test(function(t) {
>+        const requestOptions = {
>+            publicKey: {
>+                challenge: asciiToUint8Array("123456")
>+            }
>+        };
>+        const result = promiseRejects(t, "NotAllowedError", navigator.credentials.get(requestOptions), "This request is suppressed by a new request.");
>+
>+        const creationOptions = {
>+            publicKey: {
>+                rp: {
>+                    name: "example.com"
>+                },
>+                user: {
>+                    name: "John Appleseed",
>+                    id: asciiToUint8Array("123456"),
>+                    displayName: "John",
>+                },
>+                challenge: asciiToUint8Array("123456"),
>+                pubKeyCredParams: [{ type: "public-key", alg: -7 }],
>+            }
>+        };
>+        promiseRejects(t, "NotAllowedError", navigator.credentials.create(creationOptions), "This request is suppressed by a new request.");
>+
>+        return result;
>+    }, "PublicKeyCredential's [[get]] with two consecutive requests (2)");
>+
>+    promise_test(t => {
>+        const options = {
>+            publicKey: {
>+                challenge: asciiToUint8Array("123456")
>+            }
>+        };
>+
>+        const result = promiseRejects(t, "NotAllowedError", navigator.credentials.get(options), "This request is cancelled by a new request.");
>+        window.open("./resources/new-page.html?isCreation=false");
>+        return result;
>+    }, "PublicKeyCredential's [[get]] with new requests in a new page");
>+
>+    promise_test(t => {
>+        const options = {
>+            publicKey: {
>+                challenge: asciiToUint8Array("123456")
>+            }
>+        };
>+
>+        const result = promiseRejects(t, "NotAllowedError", navigator.credentials.get(options), "This request is cancelled by a new request.");
>+        window.open("./resources/new-page.html?isCreation=true");
>+        return result;
>+    }, "PublicKeyCredential's [[get]] with new requests in a new page (2)");
> </script>
>diff --git a/LayoutTests/http/wpt/webauthn/public-key-credential-get-success-hid.https-expected.txt b/LayoutTests/http/wpt/webauthn/public-key-credential-get-success-hid.https-expected.txt
>index 9058e84277c9dc5fd92747437a63d2ec7c2dcda4..43c306cb015699d2f2207d9bec6f1014adf1fe97 100644
>--- a/LayoutTests/http/wpt/webauthn/public-key-credential-get-success-hid.https-expected.txt
>+++ b/LayoutTests/http/wpt/webauthn/public-key-credential-get-success-hid.https-expected.txt
>@@ -4,4 +4,5 @@ PASS PublicKeyCredential's [[get]] with matched allow credentials in a mock hid
> PASS PublicKeyCredential's [[get]] with userVerification { preferred } in a mock hid authenticator. 
> PASS PublicKeyCredential's [[get]] with userVerification { discouraged } in a mock hid authenticator. 
> PASS PublicKeyCredential's [[get]] with mixed options in a mock hid authenticator. 
>+PASS PublicKeyCredential's [[get]] with two consecutive requests. 
> 
>diff --git a/LayoutTests/http/wpt/webauthn/public-key-credential-get-success-hid.https.html b/LayoutTests/http/wpt/webauthn/public-key-credential-get-success-hid.https.html
>index 29910e965203b1ef0ac5a60f36e4f5b2fc7131f9..bc9894e3952ae85a068f6dfa5c83284866dccc23 100644
>--- a/LayoutTests/http/wpt/webauthn/public-key-credential-get-success-hid.https.html
>+++ b/LayoutTests/http/wpt/webauthn/public-key-credential-get-success-hid.https.html
>@@ -98,4 +98,18 @@
>             return checkResult(credential);
>         });
>     }, "PublicKeyCredential's [[get]] with mixed options in a mock hid authenticator.");
>+
>+    promise_test(t => {
>+        const options = {
>+            publicKey: {
>+                challenge: Base64URL.parse("MTIzNDU2"),
>+                timeout: 100
>+            }
>+        };
>+
>+        promiseRejects(t, "NotAllowedError", navigator.credentials.get(options), "This request is suppressed by a new request.");
>+        return navigator.credentials.get(options).then(credential => {
>+            return checkResult(credential);
>+        });
>+    }, "PublicKeyCredential's [[get]] with two consecutive requests.");
> </script>
>diff --git a/LayoutTests/http/wpt/webauthn/resources/new-page.html b/LayoutTests/http/wpt/webauthn/resources/new-page.html
>new file mode 100644
>index 0000000000000000000000000000000000000000..f0903d3b60a19467d9bf3af3aa343275f40f3360
>--- /dev/null
>+++ b/LayoutTests/http/wpt/webauthn/resources/new-page.html
>@@ -0,0 +1,30 @@
>+<script src="./util.js"></script>
>+<script>
>+    const url = new URL(window.location.href);
>+    const isCreation = url.searchParams.get("creation");
>+
>+    const makeOptions = {
>+        publicKey: {
>+            rp: {
>+                name: "example.com"
>+            },
>+            user: {
>+                name: "John Appleseed",
>+                id: asciiToUint8Array("123456"),
>+                displayName: "Appleseed",
>+            },
>+            challenge: asciiToUint8Array("123456"),
>+            pubKeyCredParams: [{ type: "public-key", alg: -7 }]
>+        }
>+    };
>+    const requestOptions = {
>+        publicKey: {
>+            challenge: asciiToUint8Array("123456"),
>+        }
>+    };
>+
>+    if (isCreation == "true")
>+        navigator.credentials.create(makeOptions);
>+    else
>+        navigator.credentials.get(requestOptions);
>+</script>

Flags:

bfulgham: review+

Actions: View | Formatted Diff | Diff

Attachments on bug 191517: 369216 | 369332