[fedcm] Autofill emails only if the user accepts the modal dialog prompt
For verified email addresses, we'll show the FedCM modal dialog to get
some confirmation that the user wants to share their email address
(along with other fields, such as names and profile pictures).
When the modal dialog is shown, the user has the choice to reject or
dismiss it. When that happens, we want to make sure that the field does
not get auto-filled (in addition to failing the promise of the
conditional request).
For the password field recommendation, on the other hand, a modal isn't
necessary, and the token can be auto-granted without requiring extra
prompting.
To accomplish these differences, we introduce:
- A boolean parameter to OnFederatedTokenReceivedCallback, to let the
browser and password autofill managers know if the request succeeded
- A boolean autogrant parameter to NotifyAutofillSuggestionAccepted to
let the FederatedAuthAutofillSource know whether to request
additional permission or not
And the necessary implementation in the FederatedAuthAutofillSource to
accomplish the semantics described above.
Bug: 380367784
Change-Id: I8f5bf5d75738ad0f6e1d2545ecc8f2697cfe8806
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6484645
Commit-Queue: Sam Goto <goto@chromium.org>
Auto-Submit: Sam Goto <goto@chromium.org>
Reviewed-by: Jihad Hanna <jihadghanna@google.com>
Cr-Commit-Position: refs/heads/main@{#1453194}
diff --git a/components/autofill/content/browser/content_identity_credential_delegate.cc b/components/autofill/content/browser/content_identity_credential_delegate.cc
index 100fcb6..9fa3e63 100644
--- a/components/autofill/content/browser/content_identity_credential_delegate.cc
+++ b/components/autofill/content/browser/content_identity_credential_delegate.cc
@@ -95,6 +95,7 @@
void ContentIdentityCredentialDelegate::NotifySuggestionAccepted(
const Suggestion& suggestion,
+ bool show_modal,
OnFederatedTokenReceivedCallback callback) const {
content::FederatedAuthAutofillSource* source = source_.Run();
@@ -106,7 +107,7 @@
suggestion.GetPayload<Suggestion::IdentityCredentialPayload>();
source->NotifyAutofillSuggestionAccepted(
- payload.config_url, payload.account_id, std::move(callback));
+ payload.config_url, payload.account_id, show_modal, std::move(callback));
}
} // namespace autofill
diff --git a/components/autofill/content/browser/content_identity_credential_delegate.h b/components/autofill/content/browser/content_identity_credential_delegate.h
index b202b7a..d76a1b4 100644
--- a/components/autofill/content/browser/content_identity_credential_delegate.h
+++ b/components/autofill/content/browser/content_identity_credential_delegate.h
@@ -32,6 +32,7 @@
void NotifySuggestionAccepted(
const Suggestion& suggestion,
+ bool show_modal,
OnFederatedTokenReceivedCallback callback) const override;
private:
diff --git a/components/autofill/content/browser/content_identity_credential_delegate_unittest.cc b/components/autofill/content/browser/content_identity_credential_delegate_unittest.cc
index 45af418..633e0da4 100644
--- a/components/autofill/content/browser/content_identity_credential_delegate_unittest.cc
+++ b/components/autofill/content/browser/content_identity_credential_delegate_unittest.cc
@@ -35,6 +35,7 @@
NotifyAutofillSuggestionAccepted,
(const GURL& idp,
const std::string& account_id,
+ bool show_modal,
OnFederatedTokenReceivedCallback callback),
(override));
};
diff --git a/components/autofill/core/browser/integrators/identity_credential/identity_credential_delegate.h b/components/autofill/core/browser/integrators/identity_credential/identity_credential_delegate.h
index 2627f02..4d2aa49 100644
--- a/components/autofill/core/browser/integrators/identity_credential/identity_credential_delegate.h
+++ b/components/autofill/core/browser/integrators/identity_credential/identity_credential_delegate.h
@@ -32,8 +32,12 @@
public:
// For 3P logins usually the identity provider needs to issue a federated
// token such as ID token, access token etc. to complete the flow. This
- // callback is triggered when the browser receives such a token.
- using OnFederatedTokenReceivedCallback = base::OnceClosure;
+ // callback is triggered when the browser receives such a token or fails
+ // to produce one (e.g. the request is aborted or the user dismisses the
+ // modal dialog).
+ // The boolean parameter passed to the callback tells the caller whether
+ // the request was successful or not.
+ using OnFederatedTokenReceivedCallback = base::OnceCallback<void(bool)>;
virtual ~IdentityCredentialDelegate() = default;
// Generates verified Autofill suggestions from identity credential requests.
@@ -47,12 +51,20 @@
const FieldType& field_type) const = 0;
// Notifies the delegate that a suggestion from an identity credential
- // conditional request was accepted. After a user selects the suggestion from
+ // conditional request was accepted.
+ // The `callback` will be called when a federated token is received or fails
+ // to be provided.
+ // When `show_modal` is used, the user gets shown a modal dialog that gathers
+ // further confirmation (e.g. while providing a verified email address). The
+ // suggestion is filled (or not) when the callback is called (successfully
+ // or not).
+ // When `show_modal` is false, after a user selects the suggestion from
// the autofill dropdown UI, we should enter a loading state similar to the
- // selecting passkeys UX.The callback will be called when a federated token is
- // received. Once it's called, the loading menu will be hidden.
+ // selecting passkeys UX. Once the callback is called, the loading menu will
+ // be hidden.
virtual void NotifySuggestionAccepted(
const Suggestion& suggestion,
+ bool show_modal,
OnFederatedTokenReceivedCallback callback) const = 0;
};
diff --git a/components/autofill/core/browser/integrators/identity_credential/mock_identity_credential_delegate.h b/components/autofill/core/browser/integrators/identity_credential/mock_identity_credential_delegate.h
index b0191d8..1d0bbc2 100644
--- a/components/autofill/core/browser/integrators/identity_credential/mock_identity_credential_delegate.h
+++ b/components/autofill/core/browser/integrators/identity_credential/mock_identity_credential_delegate.h
@@ -24,6 +24,7 @@
MOCK_METHOD(void,
NotifySuggestionAccepted,
(const Suggestion& suggestion,
+ bool show_modal,
OnFederatedTokenReceivedCallback callback),
(const override));
};
diff --git a/components/autofill/core/browser/ui/autofill_external_delegate.cc b/components/autofill/core/browser/ui/autofill_external_delegate.cc
index 222e677..34d9d04 100644
--- a/components/autofill/core/browser/ui/autofill_external_delegate.cc
+++ b/components/autofill/core/browser/ui/autofill_external_delegate.cc
@@ -845,19 +845,28 @@
}
break;
case SuggestionType::kIdentityCredential: {
- // TODO(crbug.com/380367784): support filling and loading state.
if (const IdentityCredentialDelegate* identity_credential_delegate =
manager_->client().GetIdentityCredentialDelegate()) {
identity_credential_delegate->NotifySuggestionAccepted(
- suggestion, base::NullCallback());
+ suggestion, /*show_modal=*/true,
+ base::BindOnce(
+ [](base::WeakPtr<AutofillExternalDelegate> delegate,
+ const Suggestion& suggestion, bool accepted) {
+ if (!delegate || !accepted) {
+ return;
+ }
- VerifiedProfile profile =
- suggestion.GetPayload<Suggestion::IdentityCredentialPayload>()
- .fields;
- manager_->FillOrPreviewForm(
- mojom::ActionPersistence::kFill, query_form_,
- query_field_.global_id(), &profile,
- TriggerSourceFromSuggestionTriggerSource(trigger_source_));
+ VerifiedProfile profile =
+ suggestion
+ .GetPayload<Suggestion::IdentityCredentialPayload>()
+ .fields;
+ delegate->manager_->FillOrPreviewForm(
+ mojom::ActionPersistence::kFill, delegate->query_form_,
+ delegate->query_field_.global_id(), &profile,
+ TriggerSourceFromSuggestionTriggerSource(
+ delegate->trigger_source_));
+ },
+ GetWeakPtr(), suggestion));
}
break;
}
diff --git a/components/autofill/core/browser/ui/autofill_external_delegate_unittest.cc b/components/autofill/core/browser/ui/autofill_external_delegate_unittest.cc
index 6f1c395..cb9a50f 100644
--- a/components/autofill/core/browser/ui/autofill_external_delegate_unittest.cc
+++ b/components/autofill/core/browser/ui/autofill_external_delegate_unittest.cc
@@ -1283,7 +1283,46 @@
FillOrPreviewForm(mojom::ActionPersistence::kFill,
HasQueriedFormId(), IsQueriedFieldId(), _, _));
// Expect that the delegate gets notified.
- EXPECT_CALL(mock, NotifySuggestionAccepted(suggestion, _));
+ EXPECT_CALL(mock, NotifySuggestionAccepted)
+ .WillOnce(::testing::WithArgs<1, 2>(
+ [](bool show_modal,
+ IdentityCredentialDelegate::OnFederatedTokenReceivedCallback
+ callback) {
+ // Email verifications prompt the user
+ EXPECT_TRUE(show_modal);
+ // Pretend that the user has accepted the prompt
+ std::move(callback).Run(/*accepted=*/true);
+ }));
+
+ // Test fill.
+ external_delegate().DidAcceptSuggestion(suggestion,
+ SuggestionPosition{.row = 0});
+}
+
+// Test that an accepted verified email autofill suggestion will not fill the
+// form if the user later rejects the prompt.
+TEST_F(AutofillExternalDelegateTest,
+ TestVerifiedEmailSuggestion_PromptRejectedNoFill) {
+ IssueOnQuery();
+ const Suggestion suggestion = test::CreateAutofillSuggestion(
+ SuggestionType::kIdentityCredential, u"John Legend",
+ Suggestion::IdentityCredentialPayload());
+
+ // Set up a mock identity credential delegate.
+ MockIdentityCredentialDelegate mock;
+ ON_CALL(client(), GetIdentityCredentialDelegate).WillByDefault(Return(&mock));
+
+ // Expect that the delegate gets notified.
+ EXPECT_CALL(mock, NotifySuggestionAccepted)
+ .WillOnce(::testing::WithArgs<1, 2>(
+ [](bool show_modal,
+ IdentityCredentialDelegate::OnFederatedTokenReceivedCallback
+ callback) {
+ // Email verifications prompt the user
+ EXPECT_TRUE(show_modal);
+ // Pretend that the user has rejected the prompt
+ std::move(callback).Run(/*accepted=*/false);
+ }));
// Test fill.
external_delegate().DidAcceptSuggestion(suggestion,
diff --git a/components/password_manager/core/browser/password_autofill_manager.cc b/components/password_manager/core/browser/password_autofill_manager.cc
index c0f3edc..458524eb 100644
--- a/components/password_manager/core/browser/password_autofill_manager.cc
+++ b/components/password_manager/core/browser/password_autofill_manager.cc
@@ -234,8 +234,19 @@
identity_credential_delegate =
autofill_client_->GetIdentityCredentialDelegate()) {
identity_credential_delegate->NotifySuggestionAccepted(
- suggestion, base::BindOnce(&PasswordAutofillManager::HidePopup,
- weak_ptr_factory_.GetWeakPtr()));
+ suggestion, /*show_modal=*/false,
+ base::BindOnce(
+ [](base::WeakPtr<PasswordAutofillManager> manager,
+ bool accepted) {
+ if (!manager) {
+ return;
+ }
+ // When notifying the delegate, no extra permission prompts
+ // are requested. The pop-up in its loading state is hidden
+ // regardless of the accepted result.
+ manager->HidePopup();
+ },
+ weak_ptr_factory_.GetWeakPtr()));
}
UpdatePopup(PrepareLoadingStateSuggestions(
std::move(last_popup_open_args_).suggestions, suggestion));
diff --git a/content/browser/webid/federated_auth_request_impl.cc b/content/browser/webid/federated_auth_request_impl.cc
index 30f529e..332b664 100644
--- a/content/browser/webid/federated_auth_request_impl.cc
+++ b/content/browser/webid/federated_auth_request_impl.cc
@@ -1934,15 +1934,17 @@
void FederatedAuthRequestImpl::NotifyAutofillSuggestionAccepted(
const GURL& idp,
const std::string& account_id,
+ bool show_modal,
OnFederatedTokenReceivedCallback callback) {
+ token_received_callback_for_autofill_ = std::move(callback);
+
// Currently the verified email flow opens a modal UI upon notification and
// the autofill dropdown UI gets dismissed immediately. i.e. it doesn't need a
// valid callback. However, if a user is presented a full federated account,
// upon the account selection we'd proceed with fetching tokens directly and
// update he autofill dropdown UI to a loading UI.
- if (!callback.is_null()) {
+ if (!show_modal) {
OnAccountSelected(idp, account_id, true);
- token_received_callback_for_autofill_ = std::move(callback);
return;
}
// TODO(crbug.com/380367784): The third argument of OnAccountSelected checks
@@ -3196,6 +3198,11 @@
devtools_instrumentation::DidCloseFedCmDialog(render_frame_host());
}
+ if (token_received_callback_for_autofill_) {
+ std::move(token_received_callback_for_autofill_)
+ .Run(result == FederatedAuthRequestResult::kSuccess);
+ }
+
if (!should_delay_callback || should_complete_request_immediately_) {
CleanUp();
webid::GetPageData(render_frame_host().GetPage())
@@ -3312,9 +3319,6 @@
rp_mode_ = RpMode::kPassive;
private_key_.reset();
disclosures_.clear();
- if (token_received_callback_for_autofill_) {
- std::move(token_received_callback_for_autofill_).Run();
- }
}
void FederatedAuthRequestImpl::AddDevToolsIssue(
diff --git a/content/browser/webid/federated_auth_request_impl.h b/content/browser/webid/federated_auth_request_impl.h
index f542cf75..6b90f6c 100644
--- a/content/browser/webid/federated_auth_request_impl.h
+++ b/content/browser/webid/federated_auth_request_impl.h
@@ -137,6 +137,7 @@
void NotifyAutofillSuggestionAccepted(
const GURL& idp,
const std::string& account_id,
+ bool show_modal,
OnFederatedTokenReceivedCallback callback) override;
// To be called on the FederatedAuthRequest object corresponding to a
diff --git a/content/browser/webid/webid_browsertest.cc b/content/browser/webid/webid_browsertest.cc
index f3c149c..a21ef0b1 100644
--- a/content/browser/webid/webid_browsertest.cc
+++ b/content/browser/webid/webid_browsertest.cc
@@ -2125,7 +2125,7 @@
auto account = (*suggestions)[0];
source->NotifyAutofillSuggestionAccepted(
account->identity_provider->idp_metadata.config_url, account->id,
- base::NullCallback());
+ /*show_modal=*/true, base::NullCallback());
// Wait for the user to accept the prompt.
modal_loop.Run();
diff --git a/content/public/browser/federated_auth_autofill_source.h b/content/public/browser/federated_auth_autofill_source.h
index da962b7..3de9766 100644
--- a/content/public/browser/federated_auth_autofill_source.h
+++ b/content/public/browser/federated_auth_autofill_source.h
@@ -19,10 +19,13 @@
// from federated accounts and (b) handle when the suggestion gets selected.
class FederatedAuthAutofillSource {
public:
- // For 3P logins usually the identity provider needs to issue a federated
+ // For 3P logins the identity provider needs to issue a federated
// token such as ID token, access token etc. to complete the flow. This
- // callback is triggered when the browser receives such a token.
- using OnFederatedTokenReceivedCallback = base::OnceClosure;
+ // callback is triggered when the browser receives such a token or when
+ // an error occurs (e.g. the user cancels the request, the website aborts
+ // the request or the IdP's server fails to produce a token), with the
+ // parameter passed to the callback indicating a successful response or not.
+ using OnFederatedTokenReceivedCallback = base::OnceCallback<void(bool)>;
FederatedAuthAutofillSource() = default;
virtual ~FederatedAuthAutofillSource() = default;
@@ -32,11 +35,16 @@
GetAutofillSuggestions() const = 0;
// This is called when a suggestion from an identity credential conditional
// request was accepted. e.g. a user has selected the suggestion from the
- // autofill dropdown UI. The callback will be called when a federated token is
+ // autofill dropdown UI.
+ // `show_modal` determines whether the acceptance of the suggestion is
+ // sufficient permission to return a result or if extra dialogs need to be
+ // shown to the user.
+ // The `callback` will be called when a federated token is
// received. Once it's called, the autofill dropdown will be hidden.
virtual void NotifyAutofillSuggestionAccepted(
const GURL& idp,
const std::string& account_id,
+ bool show_modal,
OnFederatedTokenReceivedCallback callback) = 0;
// Returns the a data source for autofill if there is a pending conditional
