Skip to content

Commit

Permalink
Digital Credentials: implement visibility and focus requirements
Browse files Browse the repository at this point in the history
https://bugs.webkit.org/show_bug.cgi?id=275782
rdar://130821648

Reviewed by Andy Estes.

Adds visibility, focus, and user activation checks as per:
See WICG/digital-credentials#129

* LayoutTests/http/wpt/identity/identitycredentialscontainer-get-basics.https.html:
* LayoutTests/http/wpt/identity/identitycredentialscontainer-get-hidden.https-expected.txt: Added.
* LayoutTests/http/wpt/identity/identitycredentialscontainer-get-hidden.https.html: Added.
* LayoutTests/imported/w3c/web-platform-tests/digital-credentials/get-user-activation.https-expected.txt: Added.
* LayoutTests/imported/w3c/web-platform-tests/digital-credentials/get-user-activation.https.html: Added.
* LayoutTests/platform/ios/TestExpectations:
* Source/WebCore/Modules/credentialmanagement/CredentialsContainer.cpp:
(WebCore::CredentialsContainer::scopeAndCrossOriginParent const):
(WebCore::CredentialsContainer::get):
(WebCore::CredentialsContainer::isCreate):
(WebCore::CredentialsContainer::preventSilentAccess const):
(WebCore::CredentialsContainer::performCommonChecks):
* Source/WebCore/Modules/credentialmanagement/CredentialsContainer.h:
(WebCore::CredentialsContainer::document const):
* Source/WebCore/Modules/identity/IdentityCredentialsContainer.cpp:
(WebCore::IdentityCredentialsContainer::get):

Canonical link: https://commits.webkit.org/281003@main
  • Loading branch information
marcoscaceres committed Jul 16, 2024
1 parent 2997bda commit aadfa72
Show file tree
Hide file tree
Showing 9 changed files with 137 additions and 12 deletions.
Original file line number Diff line number Diff line change
@@ -1,44 +1,64 @@
<!DOCTYPE html>
<meta charset="utf-8">
<title>Digital Credential API: get() default behavior checks.</title>
<script src="/resources/testdriver.js"></script>
<script src="/resources/testdriver-vendor.js"></script>
<script src="/resources/testharness.js"></script>
<script src="/resources/testharnessreport.js"></script>
<body></body>
<script>
promise_setup(async () => {
if (document.visibilityState === "hidden") {
await new Promise((resolve) => {
document.onvisibilitychange = resolve;
testRunner.setPageVisibility("visible");
});
}
assert_equals(document.visibilityState, "visible", "should be visible");
});

promise_test(async (t) => {
await test_driver.bless();
await promise_rejects_dom(
t,
"NotSupportedError",
navigator.identity.get(),
"navigator.identity.get() with no argument."
);

await test_driver.bless();
await promise_rejects_dom(
t,
"NotSupportedError",
navigator.identity.get({}),
"navigator.identity.get() with empty dictionary."
);

await test_driver.bless();
await promise_rejects_js(
t,
TypeError,
navigator.identity.get({ digital: "wrong type" }),
"navigator.identity.get() with bogus digital type"
);

await test_driver.bless();
await promise_rejects_dom(
t,
"NotSupportedError",
navigator.identity.get({ bogus_key: "bogus" }),
"navigator.identity.get() with unknown key (same as passing empty dictionary)."
);

await test_driver.bless();
await promise_rejects_js(
t,
TypeError,
navigator.identity.get({ digital: { providers: [] } }),
"navigator.identity.get() with an empty list of providers"
);

await test_driver.bless();
await promise_rejects_js(
t,
TypeError,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@

PASS navigator.identity.get() can't be used with hidden documents.

Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
<!DOCTYPE html>
<meta charset="utf-8" />
<title>
Digital Credential API: get() can't be used with hidden documents.
</title>
<script src="/resources/testdriver.js"></script>
<script src="/resources/testdriver-vendor.js"></script>
<script src="/resources/testharness.js"></script>
<script src="/resources/testharnessreport.js"></script>
<body></body>
<script>
promise_setup(async () => {
if (document.visibilityState !== "hidden") {
await new Promise((resolve) => {
document.onvisibilitychange = resolve;
testRunner.setPageVisibility("hidden");
});
}
});

promise_test(async function (test) {
this.add_cleanup(() => {
testRunner.setPageVisibility("visible");
});

assert_equals(
document.visibilityState,
"hidden",
"now should be hidden"
);

await test_driver.bless();

assert_true(
navigator.userActivation.isActive,
"User activation should be active after test_driver.bless()."
);

await promise_rejects_dom(
test,
"NotAllowedError",
navigator.identity.get({})
);
}, "navigator.identity.get() can't be used with hidden documents.");
</script>
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@

PASS navigator.identity.get() calling the API without user activation should reject with NotAllowedError.
PASS navigator.identity.get() consumes user activation.

Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
<!DOCTYPE html>
<title>Digital Credential API: get() consumes user activation.</title>
<script src="/resources/testdriver.js"></script>
<script src="/resources/testdriver-vendor.js"></script>
<script src="/resources/testharness.js"></script>
<script src="/resources/testharnessreport.js"></script>
<body></body>
<script>
promise_test(async (t) => {
assert_false(
navigator.userActivation.isActive,
"User activation should not be active"
);
await promise_rejects_dom(t, "NotAllowedError", navigator.identity.get({}));
}, "navigator.identity.get() calling the API without user activation should reject with NotAllowedError.");

promise_test(async (t) => {
await test_driver.bless();
assert_true(
navigator.userActivation.isActive,
"User activation should be active after test_driver.bless()."
);
await promise_rejects_dom(
t,
"NotSupportedError",
navigator.identity.get({})
);
assert_false(
navigator.userActivation.isActive,
"User activation should be consumed after navigator.identity.get()."
);
}, "navigator.identity.get() consumes user activation.");
</script>
3 changes: 3 additions & 0 deletions LayoutTests/platform/ios/TestExpectations
Original file line number Diff line number Diff line change
Expand Up @@ -351,6 +351,9 @@ imported/w3c/web-platform-tests/html/semantics/embedded-content/media-elements
http/tests/cache/disk-cache/disk-cache-media-small.html
http/tests/media/audio-load-loadeddata.html [ Pass ]

# Digital Credentials
http/wpt/identity/identitycredentialscontainer-get-hidden.https.html [ Skip ]

# No touch events
fast/events/touch [ Skip ]
fast/shadow-dom/touch-event-ios.html [ Skip ]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,11 +51,11 @@ CredentialsContainer::CredentialsContainer(WeakPtr<Document, WeakPtrImplWithEven

ScopeAndCrossOriginParent CredentialsContainer::scopeAndCrossOriginParent() const
{
if (!m_document)
RefPtr document = this->document();
if (!document)
return std::pair { WebAuthn::Scope::CrossOrigin, std::nullopt };

bool isSameSite = true;
RefPtr document = m_document.get();
Ref origin = document->securityOrigin();
auto url = document->url();
std::optional<SecurityOriginData> crossOriginParent;
Expand Down Expand Up @@ -89,14 +89,13 @@ void CredentialsContainer::get(CredentialRequestOptions&& options, CredentialPro
return;
}

RefPtr document = m_document.get();
// The request will be aborted in WebAuthenticatorCoordinatorProxy if conditional mediation is not available.
if (options.mediation != MediationRequirement::Conditional && !document->hasFocus()) {
if (options.mediation != MediationRequirement::Conditional && !document()->hasFocus()) {
promise.reject(Exception { ExceptionCode::NotAllowedError, "The document is not focused."_s });
return;
}

document->page()->authenticatorCoordinator().discoverFromExternalSource(*document, WTFMove(options), scopeAndCrossOriginParent(), WTFMove(promise));
document()->page()->authenticatorCoordinator().discoverFromExternalSource(*document(), WTFMove(options), scopeAndCrossOriginParent(), WTFMove(promise));
}

void CredentialsContainer::store(const BasicCredential&, CredentialPromise&& promise)
Expand All @@ -118,17 +117,17 @@ void CredentialsContainer::isCreate(CredentialCreationOptions&& options, Credent
}

// Extra.
if (!m_document->hasFocus()) {
if (!document()->hasFocus()) {
promise.reject(Exception { ExceptionCode::NotAllowedError, "The document is not focused."_s });
return;
}

m_document->page()->authenticatorCoordinator().create(*m_document, WTFMove(options), scopeAndCrossOriginParent().first, WTFMove(options.signal), WTFMove(promise));
document()->page()->authenticatorCoordinator().create(*document(), WTFMove(options), scopeAndCrossOriginParent().first, WTFMove(options.signal), WTFMove(promise));
}

void CredentialsContainer::preventSilentAccess(DOMPromiseDeferred<void>&& promise) const
{
if (m_document && !m_document->isFullyActive()) {
if (document() && !document()->isFullyActive()) {
promise.reject(Exception { ExceptionCode::NotAllowedError, "The document is not fully active."_s });
return;
}
Expand All @@ -138,13 +137,12 @@ void CredentialsContainer::preventSilentAccess(DOMPromiseDeferred<void>&& promis
template<typename Options>
bool CredentialsContainer::performCommonChecks(const Options& options, CredentialPromise& promise)
{
RefPtr document = m_document.get();

if (!document) {
if (!document()) {
promise.reject(Exception { ExceptionCode::NotSupportedError });
return false;
}

RefPtr document = this->document();
if (!document->isFullyActive()) {
promise.reject(Exception { ExceptionCode::NotAllowedError, "The document is not fully active."_s });
return false;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,12 +65,12 @@ class CredentialsContainer : public RefCounted<CredentialsContainer> {

private:
ScopeAndCrossOriginParent scopeAndCrossOriginParent() const;

WeakPtr<Document, WeakPtrImplWithEventTargetData> m_document;

protected:
template<typename Options>
bool performCommonChecks(const Options&, CredentialPromise&);
const Document* document() const { return m_document.get(); }
};

} // namespace WebCore
Expand Down
19 changes: 19 additions & 0 deletions Source/WebCore/Modules/identity/IdentityCredentialsContainer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,8 @@
#include "ExceptionOr.h"
#include "JSDOMPromiseDeferred.h"
#include "JSDigitalCredential.h"
#include "LocalDOMWindow.h"
#include "VisibilityState.h"

namespace WebCore {
IdentityCredentialsContainer::IdentityCredentialsContainer(WeakPtr<Document, WeakPtrImplWithEventTargetData>&& document)
Expand All @@ -48,6 +50,23 @@ void IdentityCredentialsContainer::get(CredentialRequestOptions&& options, Crede
if (!performCommonChecks(options, promise))
return;

RefPtr document = this->document();
if (!document->hasFocus()) {
promise.reject(Exception { ExceptionCode::NotAllowedError, "The document is not focused."_s });
return;
}

if (document->visibilityState() != VisibilityState::Visible) {
promise.reject(Exception { ExceptionCode::NotAllowedError, "The document is not visible."_s });
return;
}

RefPtr window = document->domWindow();
if (!window || !window->consumeTransientActivation()) {
promise.reject(Exception { ExceptionCode::NotAllowedError, "Calling get() needs to be triggered by an activation triggering user event."_s });
return;
}

if (!options.digital) {
promise.reject(Exception { ExceptionCode::NotSupportedError, "Only digital member is supported."_s });
return;
Expand Down

0 comments on commit aadfa72

Please sign in to comment.