From 9d0ca76783befbdc50cc3599ae37e2344761d9c1 Mon Sep 17 00:00:00 2001 From: Christian Biesinger Date: Tue, 14 Dec 2021 23:31:35 +0000 Subject: [PATCH] [FedCM] Support AbortSignal to abort a get operation This allows a RP to cancel a pending get, e.g. when a user clicks a different sign in button. The dialog will close when abort is called. Bug: 1272025 Change-Id: Ibcc92189f493f2b148c1d473a8a836a06a406b68 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3318279 Commit-Queue: Christian Biesinger Reviewed-by: Yi Gu Reviewed-by: Robert Sesek Reviewed-by: Ken Buchanan Cr-Commit-Position: refs/heads/main@{#951721} --- .../webid/federated_auth_request_impl.cc | 9 ++++ .../webid/federated_auth_request_impl.h | 1 + .../webid/federated_auth_request_service.cc | 4 ++ .../webid/federated_auth_request_service.h | 1 + .../mojom/webid/federated_auth_request.mojom | 4 ++ .../credentials_container.cc | 24 +++++++++ .../credential-management/fedcm.https.html | 52 +++++++++++++++++++ .../support/fedcm-mock.js | 38 +++++++++----- 8 files changed, 119 insertions(+), 14 deletions(-) diff --git a/content/browser/webid/federated_auth_request_impl.cc b/content/browser/webid/federated_auth_request_impl.cc index cc674e90904da4..5bfc6d9a7924c2 100644 --- a/content/browser/webid/federated_auth_request_impl.cc +++ b/content/browser/webid/federated_auth_request_impl.cc @@ -110,6 +110,15 @@ void FederatedAuthRequestImpl::RequestIdToken( weak_ptr_factory_.GetWeakPtr())); } +void FederatedAuthRequestImpl::CancelTokenRequest() { + if (!auth_request_callback_) + return; + + // Dialog will be hidden by the destructor for request_dialog_controller_, + // triggered by CompleteRequest. + CompleteRequest(RequestIdTokenStatus::kErrorCanceled, ""); +} + void FederatedAuthRequestImpl::Revoke( const GURL& provider, const std::string& client_id, diff --git a/content/browser/webid/federated_auth_request_impl.h b/content/browser/webid/federated_auth_request_impl.h index 89ca82fa8f8e9b..98f1f3ae5a91a7 100644 --- a/content/browser/webid/federated_auth_request_impl.h +++ b/content/browser/webid/federated_auth_request_impl.h @@ -46,6 +46,7 @@ class CONTENT_EXPORT FederatedAuthRequestImpl { blink::mojom::RequestMode mode, bool prefer_auto_sign_in, blink::mojom::FederatedAuthRequest::RequestIdTokenCallback); + void CancelTokenRequest(); void Revoke(const GURL& provider, const std::string& client_id, const std::string& account_id, diff --git a/content/browser/webid/federated_auth_request_service.cc b/content/browser/webid/federated_auth_request_service.cc index efa45b6f0039a9..2fd7f362b658cb 100644 --- a/content/browser/webid/federated_auth_request_service.cc +++ b/content/browser/webid/federated_auth_request_service.cc @@ -55,6 +55,10 @@ void FederatedAuthRequestService::RequestIdToken( std::move(callback)); } +void FederatedAuthRequestService::CancelTokenRequest() { + impl_->CancelTokenRequest(); +} + void FederatedAuthRequestService::Revoke(const GURL& provider, const std::string& client_id, const std::string& account_id, diff --git a/content/browser/webid/federated_auth_request_service.h b/content/browser/webid/federated_auth_request_service.h index b182d2885ca45e..9240775b4e6381 100644 --- a/content/browser/webid/federated_auth_request_service.h +++ b/content/browser/webid/federated_auth_request_service.h @@ -48,6 +48,7 @@ class CONTENT_EXPORT FederatedAuthRequestService blink::mojom::RequestMode mode, bool prefer_auto_sign_in, RequestIdTokenCallback) override; + void CancelTokenRequest() override; void Revoke(const GURL& provider, const std::string& client_id, const std::string& account_id, diff --git a/third_party/blink/public/mojom/webid/federated_auth_request.mojom b/third_party/blink/public/mojom/webid/federated_auth_request.mojom index fd33a7c2e06a0d..11ffe04c6cf589 100644 --- a/third_party/blink/public/mojom/webid/federated_auth_request.mojom +++ b/third_party/blink/public/mojom/webid/federated_auth_request.mojom @@ -21,6 +21,7 @@ enum RequestIdTokenStatus { kErrorInvalidSigninResponse, kErrorInvalidAccountsResponse, kErrorInvalidTokenResponse, + kErrorCanceled, kError, }; @@ -68,6 +69,9 @@ interface FederatedAuthRequest { bool prefer_auto_sign_in) => (RequestIdTokenStatus status, string? id_token); + // Cancels the pending token request, if any. + CancelTokenRequest(); + // Revokes a token for the specified |account_id| from |provider| for // the RP identified by |client_id|. Revoke(url.mojom.Url provider, diff --git a/third_party/blink/renderer/modules/credentialmanager/credentials_container.cc b/third_party/blink/renderer/modules/credentialmanager/credentials_container.cc index 17317b5f239c78..214b3a50729edb 100644 --- a/third_party/blink/renderer/modules/credentialmanager/credentials_container.cc +++ b/third_party/blink/renderer/modules/credentialmanager/credentials_container.cc @@ -481,6 +481,16 @@ void AbortOtpRequest(ScriptState* script_state) { webotp_service->Abort(); } +// Abort an ongoing FederatedCredential get() operation. +void AbortFederatedCredentialRequest(ScriptState* script_state) { + if (!script_state->ContextIsValid()) + return; + + auto* fedcm_get_request = + CredentialManagerProxy::From(script_state)->FedCmGetRequest(); + fedcm_get_request->CancelTokenRequest(); +} + void OnStoreComplete(std::unique_ptr scoped_resolver) { auto* resolver = scoped_resolver->Release(); AssertSecurityRequirementsBeforeResponse( @@ -886,6 +896,11 @@ void OnRequestIdToken(ScriptPromiseResolver* resolver, "Provider's token response is invalid")); return; } + case RequestIdTokenStatus::kErrorCanceled: { + resolver->Reject(MakeGarbageCollected( + DOMExceptionCode::kAbortError, "Request has been aborted")); + return; + } case RequestIdTokenStatus::kError: { resolver->Reject(MakeGarbageCollected( DOMExceptionCode::kNetworkError, "Error retrieving an id token.")); @@ -1151,6 +1166,15 @@ ScriptPromise CredentialsContainer::get( return promise; } DCHECK(options->federated()->hasPreferAutoSignIn()); + if (options->hasSignal()) { + if (options->signal()->aborted()) { + resolver->Reject(MakeGarbageCollected( + DOMExceptionCode::kAbortError, "Request has been aborted.")); + return promise; + } + options->signal()->AddAlgorithm(WTF::Bind( + &AbortFederatedCredentialRequest, WrapPersistent(script_state))); + } bool prefer_auto_sign_in = options->federated()->preferAutoSignIn(); auto* fedcm_get_request = CredentialManagerProxy::From(script_state)->FedCmGetRequest(); diff --git a/third_party/blink/web_tests/external/wpt/credential-management/fedcm.https.html b/third_party/blink/web_tests/external/wpt/credential-management/fedcm.https.html index 655bff8c2360df..71ff0ab70f4925 100644 --- a/third_party/blink/web_tests/external/wpt/credential-management/fedcm.https.html +++ b/third_party/blink/web_tests/external/wpt/credential-management/fedcm.https.html @@ -101,6 +101,58 @@ assert_equals(token, "a_token"); }, "nonce is not required in FederatedIdentityProvider."); + fedcm_test(async (t, mock) => { + let controller = new AbortController(); + mock.returnPendingPromise(); + let aborted = false; + const token_promise = navigator.credentials.get({ + federated: { + providers: [{ + url: 'https://idp.test', + clientId: '1', + }], + }, + signal: controller.signal, + }); + assert_equals(aborted, false); + controller.abort(); + try { + await token_promise; + } catch (e) { + aborted = true; + assert_equals(e.name, "AbortError"); + } + assert_equals(aborted, true); + }, "test the abort signal"); + + fedcm_test(async (t, mock) => { + let controller = new AbortController(); + mock.returnPendingPromise(); + let aborted = false; + const token_promise = navigator.credentials.get({ + federated: { + providers: [{ + url: 'https://idp.test', + clientId: '1', + }], + }, + signal: controller.signal, + }); + assert_equals(aborted, false); + controller.abort(); + try { + await token_promise; + } catch (e) { + aborted = true; + assert_equals(e.name, "AbortError"); + } + assert_equals(aborted, true); + + mock.returnIdToken("a_token"); + const token = await navigator.credentials.get(test_options); + assert_equals(token, "a_token"); + }, "get after abort should work"); + promise_test(function(t) { // Logout API not supported yet. return promise_rejects_dom(t, "NotSupportedError", diff --git a/third_party/blink/web_tests/external/wpt/credential-management/support/fedcm-mock.js b/third_party/blink/web_tests/external/wpt/credential-management/support/fedcm-mock.js index 318419792c2986..2fa351446c2601 100644 --- a/third_party/blink/web_tests/external/wpt/credential-management/support/fedcm-mock.js +++ b/third_party/blink/web_tests/external/wpt/credential-management/support/fedcm-mock.js @@ -2,20 +2,6 @@ import { RequestMode, RequestIdTokenStatus, LogoutStatus, RevokeStatus, Federate function toMojoIdTokenStatus(status) { return RequestIdTokenStatus["k" + status]; -// switch(status) { -// case "Success": return RequestIdTokenStatus.kSuccess; -// case "ApprovalDeclined": return RequestIdTokenStatus.kApprovalDeclined; -// case "ErrorTooManyRequests": return RequestIdTokenStatus.kErrorTooManyRequests; -// case "ErrorWebIdNotSupportedByProvider": return RequestIdTokenStatus.kErrorWebIdNotSupportedByProvider; -// case "ErrorFetchingWellKnown": return RequestIdTokenStatus.kErrorFetchingWellKnown; -// case "ErrorInvalidWellKnown": return RequestIdTokenStatus.kErrorInvalidWellKnown; -// case "ErrorFetchingSignin": return RequestIdTokenStatus.kErrorFetchingSignin; -// case "ErrorInvalidSigninResponse": return RequestIdTokenStatus.kErrorInvalidSigninResponse; -// case "ErrorInvalidAccountsResponse": return RequestIdTokenStatus.kErrorInvalidAccountsResponse; -// case "ErrorInvalidTokenResponse": return RequestIdTokenStatus.kErrorInvalidTokenResponse; -// case "Error": return RequestIdTokenStatus.kError; -// default: throw new Error(`Invalid status: ${status}`); -// } } // A mock service for responding to federated auth requests. @@ -31,12 +17,15 @@ export class MockFederatedAuthRequest { this.status_ = RequestIdTokenStatus.kError; this.logoutStatus_ = LogoutStatus.kError; this.revokeStatus_ = RevokeStatus.kError; + this.returnPending_ = false; + this.pendingPromiseResolve_ = null; } // Causes the subsequent `navigator.credentials.get()` to resolve with the token. returnIdToken(token) { this.status_ = RequestIdTokenStatus.kSuccess; this.idToken_ = token; + this.returnPending_ = false; } // Causes the subsequent `navigator.credentials.get()` to reject with the error. @@ -45,6 +34,13 @@ export class MockFederatedAuthRequest { throw new Error("Success is not a valid error"); this.status_ = toMojoIdTokenStatus(error); this.idToken_ = null; + this.returnPending_ = false; + } + + // Causes the subsequent `navigator.credentials.get()` to return a pending promise + // that can be cancelled using `cancelTokenRequest()`. + returnPendingPromise() { + this.returnPending_ = true; } // Causes the subsequent `FederatedCredential.revoke` to reject with this @@ -59,12 +55,26 @@ export class MockFederatedAuthRequest { // Implements // RequestIdToken(url.mojom.Url provider, string id_request, RequestMode mode) => (RequestIdTokenStatus status, string? id_token); async requestIdToken(provider, idRequest, mode) { + if (this.returnPending_) { + this.pendingPromise_ = new Promise((resolve, reject) => { + this.pendingPromiseResolve_ = resolve; + }); + return this.pendingPromise_; + } return Promise.resolve({ status: this.status_, idToken: this.idToken_ }); } + async cancelTokenRequest() { + this.pendingPromiseResolve_({ + status: toMojoIdTokenStatus("ErrorCanceled"), + idToken: null + }); + this.pendingPromiseResolve_ = null; + } + async logout(logout_endpoints) { return Promise.resolve({ status: this.logoutStatus_