From 5bae90496f7df95800b47b5b9fd8642e59c9b9ea Mon Sep 17 00:00:00 2001 From: John Schanck Date: Tue, 17 Oct 2023 00:21:26 +0000 Subject: [PATCH] Bug 1857982 - propagate reason from abort controller. r=saschanaz Differential Revision: https://phabricator.services.mozilla.com/D190498 --- dom/webauthn/WebAuthnManager.cpp | 86 +++++++++++++------ dom/webauthn/WebAuthnManager.h | 26 ++++-- .../enrollment.https.html.ini | 3 - .../createcredential-abort.https.html.ini | 12 --- .../getcredential-abort.https.html.ini | 12 --- 5 files changed, 79 insertions(+), 60 deletions(-) delete mode 100644 testing/web-platform/meta/webauthn/createcredential-abort.https.html.ini delete mode 100644 testing/web-platform/meta/webauthn/getcredential-abort.https.html.ini diff --git a/dom/webauthn/WebAuthnManager.cpp b/dom/webauthn/WebAuthnManager.cpp index 8015819731af..a06271faff62 100644 --- a/dom/webauthn/WebAuthnManager.cpp +++ b/dom/webauthn/WebAuthnManager.cpp @@ -177,7 +177,7 @@ nsresult RelaxSameOrigin(nsPIDOMWindowInner* aParent, **********************************************************************/ void WebAuthnManager::ClearTransaction() { - if (!mTransaction.isNothing()) { + if (mTransaction.isSome()) { StopListeningForVisibilityEvents(); } @@ -185,20 +185,10 @@ void WebAuthnManager::ClearTransaction() { Unfollow(); } -void WebAuthnManager::RejectTransaction(const nsresult& aError) { - if (!NS_WARN_IF(mTransaction.isNothing())) { - mTransaction.ref().mPromise->MaybeReject(aError); - } - - ClearTransaction(); -} - -void WebAuthnManager::CancelTransaction(const nsresult& aError) { +void WebAuthnManager::CancelParent() { if (!NS_WARN_IF(!mChild || mTransaction.isNothing())) { mChild->SendRequestCancel(mTransaction.ref().mId); } - - RejectTransaction(aError); } void WebAuthnManager::HandleVisibilityChange() { @@ -244,13 +234,7 @@ already_AddRefed WebAuthnManager::MakeCredential( // Otherwise, the user may well have clicked away, so let's // abort the old transaction and take over control from here. - CancelTransaction(NS_ERROR_ABORT); - } - - // Abort the request if aborted flag is already set. - if (aSignal.WasPassed() && aSignal.Value().Aborted()) { - promise->MaybeReject(NS_ERROR_DOM_ABORT_ERR); - return promise.forget(); + CancelTransaction(NS_ERROR_DOM_ABORT_ERR); } nsString origin; @@ -452,11 +436,30 @@ already_AddRefed WebAuthnManager::MakeCredential( return promise.forget(); } + // Abort the request if aborted flag is already set. + if (aSignal.WasPassed() && aSignal.Value().Aborted()) { + AutoJSAPI jsapi; + if (!jsapi.Init(global)) { + promise->MaybeReject(NS_ERROR_DOM_ABORT_ERR); + return promise.forget(); + } + JSContext* cx = jsapi.cx(); + JS::Rooted reason(cx); + aSignal.Value().GetReason(cx, &reason); + promise->MaybeReject(reason); + return promise.forget(); + } + WebAuthnMakeCredentialInfo info( origin, NS_ConvertUTF8toUTF16(rpId), challenge, clientDataJSON, adjustedTimeout, excludeList, rpInfo, userInfo, coseAlgos, extensions, authSelection, attestation, context->Top()->Id()); + // Set up the transaction state (including event listeners, etc). Fallible + // operations should not be performed below this line, as we must not leave + // the transaction state partially initialized. Once the transaction state is + // initialized the only valid ways to end the transaction are + // CancelTransaction, RejectTransaction, and FinishMakeCredential. #ifdef XP_WIN if (!WinWebAuthnManager::AreWebAuthNApisAvailable()) { ListenForVisibilityEvents(); @@ -503,13 +506,7 @@ already_AddRefed WebAuthnManager::GetAssertion( // Otherwise, the user may well have clicked away, so let's // abort the old transaction and take over control from here. - CancelTransaction(NS_ERROR_ABORT); - } - - // Abort the request if aborted flag is already set. - if (aSignal.WasPassed() && aSignal.Value().Aborted()) { - promise->MaybeReject(NS_ERROR_DOM_ABORT_ERR); - return promise.forget(); + CancelTransaction(NS_ERROR_DOM_ABORT_ERR); } nsString origin; @@ -656,11 +653,30 @@ already_AddRefed WebAuthnManager::GetAssertion( return promise.forget(); } + // Abort the request if aborted flag is already set. + if (aSignal.WasPassed() && aSignal.Value().Aborted()) { + AutoJSAPI jsapi; + if (!jsapi.Init(global)) { + promise->MaybeReject(NS_ERROR_DOM_ABORT_ERR); + return promise.forget(); + } + JSContext* cx = jsapi.cx(); + JS::Rooted reason(cx); + aSignal.Value().GetReason(cx, &reason); + promise->MaybeReject(reason); + return promise.forget(); + } + WebAuthnGetAssertionInfo info(origin, NS_ConvertUTF8toUTF16(rpId), challenge, clientDataJSON, adjustedTimeout, allowList, extensions, aOptions.mUserVerification, context->Top()->Id()); + // Set up the transaction state (including event listeners, etc). Fallible + // operations should not be performed below this line, as we must not leave + // the transaction state partially initialized. Once the transaction state is + // initialized the only valid ways to end the transaction are + // CancelTransaction, RejectTransaction, and FinishGetAssertion. #ifdef XP_WIN if (!WinWebAuthnManager::AreWebAuthNApisAvailable()) { ListenForVisibilityEvents(); @@ -704,7 +720,7 @@ already_AddRefed WebAuthnManager::Store(const Credential& aCredential, // Otherwise, the user may well have clicked away, so let's // abort the old transaction and take over control from here. - CancelTransaction(NS_ERROR_ABORT); + CancelTransaction(NS_ERROR_DOM_ABORT_ERR); } promise->MaybeReject(NS_ERROR_DOM_NOT_SUPPORTED_ERR); @@ -822,7 +838,21 @@ void WebAuthnManager::RequestAborted(const uint64_t& aTransactionId, } void WebAuthnManager::RunAbortAlgorithm() { - CancelTransaction(NS_ERROR_DOM_ABORT_ERR); + if (NS_WARN_IF(mTransaction.isNothing())) { + return; + } + + nsCOMPtr global = do_QueryInterface(mParent); + + AutoJSAPI jsapi; + if (!jsapi.Init(global)) { + CancelTransaction(NS_ERROR_DOM_ABORT_ERR); + return; + } + JSContext* cx = jsapi.cx(); + JS::Rooted reason(cx); + Signal()->GetReason(cx, &reason); + CancelTransaction(reason); } } // namespace mozilla::dom diff --git a/dom/webauthn/WebAuthnManager.h b/dom/webauthn/WebAuthnManager.h index 8271512a69fd..05175c8087cf 100644 --- a/dom/webauthn/WebAuthnManager.h +++ b/dom/webauthn/WebAuthnManager.h @@ -112,17 +112,33 @@ class WebAuthnManager final : public WebAuthnManagerBase, public AbortFollower { void RunAbortAlgorithm() override; protected: - // Cancels the current transaction (by sending a Cancel message to the - // parent) and rejects it by calling RejectTransaction(). - void CancelTransaction(const nsresult& aError); // Upon a visibility change, makes note of it in the current transaction. void HandleVisibilityChange() override; private: virtual ~WebAuthnManager(); - // Rejects the current transaction and calls ClearTransaction(). - void RejectTransaction(const nsresult& aError); + // Send a Cancel message to the parent, reject the promise with the given + // reason (an nsresult or JS::Handle), and clear the transaction. + template + void CancelTransaction(const T& aReason) { + CancelParent(); + RejectTransaction(aReason); + } + + // Reject the promise with the given reason (an nsresult or JS::Value), and + // clear the transaction. + template + void RejectTransaction(const T& aReason) { + if (!NS_WARN_IF(mTransaction.isNothing())) { + mTransaction.ref().mPromise->MaybeReject(aReason); + } + + ClearTransaction(); + } + + // Send a Cancel message to the parent. + void CancelParent(); // Clears all information we have about the current transaction. void ClearTransaction(); diff --git a/testing/web-platform/meta/secure-payment-confirmation/enrollment.https.html.ini b/testing/web-platform/meta/secure-payment-confirmation/enrollment.https.html.ini index d396e0e624b8..33eedc5cf852 100644 --- a/testing/web-platform/meta/secure-payment-confirmation/enrollment.https.html.ini +++ b/testing/web-platform/meta/secure-payment-confirmation/enrollment.https.html.ini @@ -21,6 +21,3 @@ [Payment credential does not allow residentKey to be "discouraged".] expected: FAIL - - [Payment credential abort reason with Error] - expected: FAIL diff --git a/testing/web-platform/meta/webauthn/createcredential-abort.https.html.ini b/testing/web-platform/meta/webauthn/createcredential-abort.https.html.ini deleted file mode 100644 index ab1b2545154c..000000000000 --- a/testing/web-platform/meta/webauthn/createcredential-abort.https.html.ini +++ /dev/null @@ -1,12 +0,0 @@ -[createcredential-abort.https.html] - [navigator.credentials.create() after abort reason] - expected: FAIL - - [navigator.credentials.create() before abort reason] - expected: FAIL - - [navigator.credentials.create() after abort reason with Error] - expected: FAIL - - [navigator.credentials.create() before abort reason with Error] - expected: FAIL diff --git a/testing/web-platform/meta/webauthn/getcredential-abort.https.html.ini b/testing/web-platform/meta/webauthn/getcredential-abort.https.html.ini deleted file mode 100644 index 1ea6b6c7318b..000000000000 --- a/testing/web-platform/meta/webauthn/getcredential-abort.https.html.ini +++ /dev/null @@ -1,12 +0,0 @@ -[getcredential-abort.https.html] - [navigator.credentials.get() after abort reason] - expected: FAIL - - [navigator.credentials.get() before abort reason] - expected: FAIL - - [navigator.credentials.get() after abort reason with Error] - expected: FAIL - - [navigator.credentials.get() before abort reason with Error] - expected: FAIL