Skip to content

Commit

Permalink
[FedCM] Support AbortSignal to abort a get operation
Browse files Browse the repository at this point in the history
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 <cbiesinger@chromium.org>
Reviewed-by: Yi Gu <yigu@chromium.org>
Reviewed-by: Robert Sesek <rsesek@chromium.org>
Reviewed-by: Ken Buchanan <kenrb@chromium.org>
Cr-Commit-Position: refs/heads/main@{#951721}
  • Loading branch information
cbiesinger authored and Chromium LUCI CQ committed Dec 14, 2021
1 parent 8f6aab4 commit 9d0ca76
Show file tree
Hide file tree
Showing 8 changed files with 119 additions and 14 deletions.
9 changes: 9 additions & 0 deletions content/browser/webid/federated_auth_request_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
1 change: 1 addition & 0 deletions content/browser/webid/federated_auth_request_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
4 changes: 4 additions & 0 deletions content/browser/webid/federated_auth_request_service.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
1 change: 1 addition & 0 deletions content/browser/webid/federated_auth_request_service.h
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ enum RequestIdTokenStatus {
kErrorInvalidSigninResponse,
kErrorInvalidAccountsResponse,
kErrorInvalidTokenResponse,
kErrorCanceled,
kError,
};

Expand Down Expand Up @@ -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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<ScopedPromiseResolver> scoped_resolver) {
auto* resolver = scoped_resolver->Release();
AssertSecurityRequirementsBeforeResponse(
Expand Down Expand Up @@ -886,6 +896,11 @@ void OnRequestIdToken(ScriptPromiseResolver* resolver,
"Provider's token response is invalid"));
return;
}
case RequestIdTokenStatus::kErrorCanceled: {
resolver->Reject(MakeGarbageCollected<DOMException>(
DOMExceptionCode::kAbortError, "Request has been aborted"));
return;
}
case RequestIdTokenStatus::kError: {
resolver->Reject(MakeGarbageCollected<DOMException>(
DOMExceptionCode::kNetworkError, "Error retrieving an id token."));
Expand Down Expand Up @@ -1151,6 +1166,15 @@ ScriptPromise CredentialsContainer::get(
return promise;
}
DCHECK(options->federated()->hasPreferAutoSignIn());
if (options->hasSignal()) {
if (options->signal()->aborted()) {
resolver->Reject(MakeGarbageCollected<DOMException>(
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();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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.
Expand All @@ -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
Expand All @@ -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_
Expand Down

0 comments on commit 9d0ca76

Please sign in to comment.