[WebAuthN] A focused document should be required
authorjiewen_tan@apple.com <jiewen_tan@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 3 May 2019 23:41:36 +0000 (23:41 +0000)
committerjiewen_tan@apple.com <jiewen_tan@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 3 May 2019 23:41:36 +0000 (23:41 +0000)
https://bugs.webkit.org/show_bug.cgi?id=197543
<rdar://problem/50430989>

Reviewed by Brent Fulgham.

Source/WebCore:

This patch adds a check to see if the invoking document is focused before
calling into WebAuthN. This patch also removes some out-to-dated comments.

Test: http/wpt/webauthn/public-key-credential-unfocused-document.https.html

* Modules/credentialmanagement/CredentialsContainer.cpp:
(WebCore::CredentialsContainer::get):
(WebCore::CredentialsContainer::isCreate):
* Modules/webauthn/AuthenticatorCoordinator.cpp:
(WebCore::AuthenticatorCoordinator::create const):
(WebCore::AuthenticatorCoordinator::discoverFromExternalSource const):

LayoutTests:

* http/wpt/webauthn/public-key-credential-same-origin-with-ancestors.https.html:
* http/wpt/webauthn/public-key-credential-unfocused-document.https-expected.txt: Added.
* http/wpt/webauthn/public-key-credential-unfocused-document.https.html: Copied from LayoutTests/http/wpt/webauthn/public-key-credential-same-origin-with-ancestors.https.html.
* http/wpt/webauthn/resources/last-layer-frame.https.html:
* http/wpt/webauthn/resources/second-layer-frame.https.html:

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@244938 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog
LayoutTests/http/wpt/webauthn/public-key-credential-same-origin-with-ancestors.https.html
LayoutTests/http/wpt/webauthn/public-key-credential-unfocused-document.https-expected.txt [new file with mode: 0644]
LayoutTests/http/wpt/webauthn/public-key-credential-unfocused-document.https.html [new file with mode: 0644]
LayoutTests/http/wpt/webauthn/resources/last-layer-frame.https.html
LayoutTests/http/wpt/webauthn/resources/second-layer-frame.https.html
Source/WebCore/ChangeLog
Source/WebCore/Modules/credentialmanagement/CredentialsContainer.cpp
Source/WebCore/Modules/webauthn/AuthenticatorCoordinator.cpp

index 5a53ad1..db50337 100644 (file)
@@ -1,3 +1,17 @@
+2019-05-03  Jiewen Tan  <jiewen_tan@apple.com>
+
+        [WebAuthN] A focused document should be required
+        https://bugs.webkit.org/show_bug.cgi?id=197543
+        <rdar://problem/50430989>
+
+        Reviewed by Brent Fulgham.
+
+        * http/wpt/webauthn/public-key-credential-same-origin-with-ancestors.https.html:
+        * http/wpt/webauthn/public-key-credential-unfocused-document.https-expected.txt: Added.
+        * http/wpt/webauthn/public-key-credential-unfocused-document.https.html: Copied from LayoutTests/http/wpt/webauthn/public-key-credential-same-origin-with-ancestors.https.html.
+        * http/wpt/webauthn/resources/last-layer-frame.https.html:
+        * http/wpt/webauthn/resources/second-layer-frame.https.html:
+
 2019-05-03  Youenn Fablet  <youenn@apple.com>
 
         [iOS] set the default maximum camera count to 1 for enumerateDevices
index 21f147b..c01edc0 100644 (file)
@@ -12,7 +12,7 @@
 <body>
     <script>
         promise_test(t => {
-            return withCrossOriginIframe("last-layer-frame.https.html").then((message) => {
+            return withCrossOriginIframe("last-layer-frame.https.html?shouldFocus=true&exceptionMessage=The origin of the document is not the same as its ancestors.").then((message) => {
                 assert_equals(message.data, "PASS.");
             });
         }, "Tests that a frame that doesn't share the same origin with all its ancestors could not access the API.");
diff --git a/LayoutTests/http/wpt/webauthn/public-key-credential-unfocused-document.https-expected.txt b/LayoutTests/http/wpt/webauthn/public-key-credential-unfocused-document.https-expected.txt
new file mode 100644 (file)
index 0000000..02b71fb
--- /dev/null
@@ -0,0 +1,4 @@
+  
+
+PASS Tests that a frame that doesn't have the focus could not access the API. 
+
diff --git a/LayoutTests/http/wpt/webauthn/public-key-credential-unfocused-document.https.html b/LayoutTests/http/wpt/webauthn/public-key-credential-unfocused-document.https.html
new file mode 100644 (file)
index 0000000..b78c708
--- /dev/null
@@ -0,0 +1,22 @@
+<!DOCTYPE html>
+<html>
+<head>
+    <meta charset="utf-8">
+    <title>Web Authentication API: Tests that a frame that doesn't have the focus could not access the API.</title>
+    <script src="/resources/testharness.js"></script>
+    <script src="/resources/testharnessreport.js"></script>
+    <script src="/common/utils.js"></script>
+    <script src="/common/get-host-info.sub.js"></script>
+    <script src="./resources/util.js"></script>
+</head>
+<body>
+    <iframe src=""></iframe>
+    <script>
+        promise_test(t => {
+            return withCrossOriginIframe("last-layer-frame.https.html?shouldFocus=false&exceptionMessage=The document is not focused.").then((message) => {
+                assert_equals(message.data, "PASS.");
+            });
+        }, "Tests that a frame that doesn't have the focus could not access the API.");
+    </script>
+</body>
+</html>
index 6a4da4c..df787ae 100644 (file)
@@ -1,48 +1,61 @@
-<script src="./util.js"></script>
-<script>
-function messageToTop(message) {
-    top.postMessage(message, "*");
-}
+<!DOCTYPE html>
+<html>
+<head>
+    <script src="./util.js"></script>
+</head>
+<body>
+    <input type="text" id="input">
+    <script>
+        const url = new URL(window.location.href);
+        const shouldFocus = url.searchParams.get("shouldFocus");
+        const exceptionMessage = url.searchParams.get("exceptionMessage");
 
-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"),
-    }
-};
+        function messageToTop(message) {
+            top.postMessage(message, "*");
+        }
 
+        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"),
+            }
+        };
 
-navigator.credentials.create(makeOptions).then(
-    function(value) {
-        messageToTop("Access granted. " + value);
-    },
-    function(exception) {
-        if (exception.name == "NotAllowedError" && exception.message == "The origin of the document is not the same as its ancestors.")
-            return navigator.credentials.get(requestOptions);
-        else
-            messageToTop("Throw " + exception.name  + ".");
-    }
-).then(function(value) {
-        messageToTop("Access granted. " + value);
-    },
-    function(exception) {
-        if (exception.name == "NotAllowedError" && exception.message == "The origin of the document is not the same as its ancestors.")
-            messageToTop("PASS.");
-        else
-            messageToTop("Throw " + exception.name  + ".");
-    }
-);
-</script>
+        if (shouldFocus == "true")
+            input.focus();
+        navigator.credentials.create(makeOptions).then(
+            function(value) {
+                messageToTop("Access granted. " + value);
+            },
+            function(exception) {
+                if (exception.name == "NotAllowedError" && exception.message == exceptionMessage)
+                    return navigator.credentials.get(requestOptions);
+                else
+                    messageToTop("Throw " + exception.name  + ".");
+            }
+        ).then(function(value) {
+                messageToTop("Access granted. " + value);
+            },
+            function(exception) {
+                if (exception.name == "NotAllowedError" && exception.message == exceptionMessage)
+                    messageToTop("PASS.");
+                else
+                    messageToTop("Throw " + exception.name  + ".");
+            }
+        );
+    </script>
+</body>
+</html>
index 3cf1630..a38dc96 100644 (file)
@@ -3,6 +3,6 @@
 <head>
 </head>
 <body>
-    <iframe src="last-layer-frame.https.html"></iframe>
+    <iframe src="last-layer-frame.https.html?shouldFocus=true&exceptionMessage=The origin of the document is not the same as its ancestors."></iframe>
 </body>
 </html>
index d059a87..5bcf7c4 100644 (file)
@@ -1,3 +1,23 @@
+2019-05-02  Jiewen Tan  <jiewen_tan@apple.com>
+
+        [WebAuthN] A focused document should be required
+        https://bugs.webkit.org/show_bug.cgi?id=197543
+        <rdar://problem/50430989>
+
+        Reviewed by Brent Fulgham.
+
+        This patch adds a check to see if the invoking document is focused before
+        calling into WebAuthN. This patch also removes some out-to-dated comments.
+
+        Test: http/wpt/webauthn/public-key-credential-unfocused-document.https.html
+
+        * Modules/credentialmanagement/CredentialsContainer.cpp:
+        (WebCore::CredentialsContainer::get):
+        (WebCore::CredentialsContainer::isCreate):
+        * Modules/webauthn/AuthenticatorCoordinator.cpp:
+        (WebCore::AuthenticatorCoordinator::create const):
+        (WebCore::AuthenticatorCoordinator::discoverFromExternalSource const):
+
 2019-05-03  Devin Rousso  <drousso@apple.com>
 
         Web Inspector: DOM: rename "low power" to "display composited"
index 7081478..2f03895 100644 (file)
@@ -83,6 +83,12 @@ void CredentialsContainer::get(CredentialRequestOptions&& options, CredentialPro
         return;
     }
 
+    // Extra.
+    if (!m_document->hasFocus()) {
+        promise.reject(Exception { NotAllowedError, "The document is not focused."_s });
+        return;
+    }
+
     m_document->page()->authenticatorCoordinator().discoverFromExternalSource(m_document->securityOrigin(), options.publicKey.value(), doesHaveSameOriginAsItsAncestors(), WTFMove(options.signal), WTFMove(promise));
 }
 
@@ -112,6 +118,12 @@ void CredentialsContainer::isCreate(CredentialCreationOptions&& options, Credent
         return;
     }
 
+    // Extra.
+    if (!m_document->hasFocus()) {
+        promise.reject(Exception { NotAllowedError, "The document is not focused."_s });
+        return;
+    }
+
     m_document->page()->authenticatorCoordinator().create(m_document->securityOrigin(), options.publicKey.value(), doesHaveSameOriginAsItsAncestors(), WTFMove(options.signal), WTFMove(promise));
 }
 
index a4d20ac..747aab3 100644 (file)
@@ -166,10 +166,7 @@ void AuthenticatorCoordinator::create(const SecurityOrigin& callerOrigin, const
     auto clientDataJsonHash = produceClientDataJsonHash(clientDataJson);
 
     // Step 4, 17-21.
-    // Only platform attachments will be supported at this stage. Assuming one authenticator per device.
-    // Also, resident keys, user verifications and direct attestation are enforced at this tage.
-    // For better performance, transports of options.excludeCredentials are checked in LocalAuthenticator.
-    if (!m_client)  {
+    if (!m_client) {
         promise.reject(Exception { UnknownError, "Unknown internal error."_s });
         return;
     }
@@ -237,10 +234,7 @@ void AuthenticatorCoordinator::discoverFromExternalSource(const SecurityOrigin&
     auto clientDataJsonHash = produceClientDataJsonHash(clientDataJson);
 
     // Step 4, 14-19.
-    // Only platform attachments will be supported at this stage. Assuming one authenticator per device.
-    // Also, resident keys, user verifications and direct attestation are enforced at this tage.
-    // For better performance, filtering of options.allowCredentials is done in LocalAuthenticator.
-    if (!m_client)  {
+    if (!m_client) {
         promise.reject(Exception { UnknownError, "Unknown internal error."_s });
         return;
     }