Skip to content

Commit

Permalink
[autofill] Wait for username on webauthn forms
Browse files Browse the repository at this point in the history
Have the password autofill wait for username if a form field is
annotated as `autofill="webauthn"` and the
WebAuthenticationConditionalUI flag is enabled. This is the first part
of an integration between WebAuthn Conditional UI and password autofill.

PR spec: w3c/webauthn#1576

Design doc:
https://docs.google.com/document/d/1KzEWP0aoLMZ0asfw6d3-7UHJ6csTtxLA478EgptCvkk

Bug: 1171985
Change-Id: I4c9879e36fc3923555731da8f7bf4a957e080e71
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3107526
Commit-Queue: Nina Satragno <[email protected]>
Reviewed-by: Vasilii Sukhanov <[email protected]>
Cr-Commit-Position: refs/heads/main@{#914458}
  • Loading branch information
nsatragno authored and Chromium LUCI CQ committed Aug 23, 2021
1 parent f343705 commit 16a9ddd
Show file tree
Hide file tree
Showing 12 changed files with 100 additions and 4 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
#include "base/bind.h"
#include "base/callback_helpers.h"
#include "base/command_line.h"
#include "base/feature_list.h"
#include "base/memory/ptr_util.h"
#include "base/memory/ref_counted.h"
#include "base/memory/singleton.h"
Expand Down Expand Up @@ -98,6 +99,7 @@
#include "content/public/browser/ssl_status.h"
#include "content/public/browser/storage_partition.h"
#include "content/public/browser/web_contents.h"
#include "content/public/common/content_features.h"
#include "content/public/common/content_switches.h"
#include "extensions/buildflags/buildflags.h"
#include "google_apis/gaia/gaia_urls.h"
Expand Down Expand Up @@ -948,6 +950,10 @@ FieldInfoManager* ChromePasswordManagerClient::GetFieldInfoManager() const {
return FieldInfoManagerFactory::GetForBrowserContext(profile_);
}

bool ChromePasswordManagerClient::IsWebAuthnAutofillEnabled() const {
return base::FeatureList::IsEnabled(features::kWebAuthConditionalUI);
}

void ChromePasswordManagerClient::AutomaticGenerationAvailable(
const autofill::password_generation::PasswordGenerationUIData& ui_data) {
if (!password_manager::bad_message::CheckChildProcessSecurityPolicyForURL(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -233,6 +233,7 @@ class ChromePasswordManagerClient
bool IsIsolationForPasswordSitesEnabled() const override;
bool IsNewTabPage() const override;
password_manager::FieldInfoManager* GetFieldInfoManager() const override;
bool IsWebAuthnAutofillEnabled() const override;

// autofill::mojom::PasswordGenerationDriver overrides.
void AutomaticGenerationAvailable(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ constexpr char kAutocompleteCurrentPassword[] = "current-password";
constexpr char kAutocompleteNewPassword[] = "new-password";
constexpr char kAutocompleteCreditCardPrefix[] = "cc-";
constexpr char kAutocompleteOneTimePassword[] = "one-time-code";
constexpr char kAutocompleteWebAuthn[] = "webauthn";

// The autocomplete attribute has one of the following structures:
// [section-*] [shipping|billing] [type_hint] field_type
Expand All @@ -50,9 +51,9 @@ constexpr char kAutocompleteOneTimePassword[] = "one-time-code";
// https://html.spec.whatwg.org/multipage/form-control-infrastructure.html#autofilling-form-controls%3A-the-autocomplete-attribute).
// For password forms, only the field_type is relevant. So parsing the attribute
// amounts to just taking the last token. If that token is one of "username",
// "current-password" or "new-password", this returns an appropriate enum value.
// If the token starts with a "cc-" prefix or is "one-time-code" token, this
// returns kNonPassword.
// "current-password", "new-password" or "webauthn", this returns an appropriate
// enum value. If the token starts with a "cc-" prefix or is "one-time-code"
// token, this returns kNonPassword.
// Otherwise, returns kNone.
AutocompleteFlag ExtractAutocompleteFlag(const std::string& attribute) {
std::vector<base::StringPiece> tokens =
Expand All @@ -68,6 +69,8 @@ AutocompleteFlag ExtractAutocompleteFlag(const std::string& attribute) {
return AutocompleteFlag::kCurrentPassword;
if (base::LowerCaseEqualsASCII(field_type, kAutocompleteNewPassword))
return AutocompleteFlag::kNewPassword;
if (base::LowerCaseEqualsASCII(field_type, kAutocompleteWebAuthn))
return AutocompleteFlag::kWebAuthn;

if (base::LowerCaseEqualsASCII(field_type, kAutocompleteOneTimePassword) ||
base::StartsWith(field_type, kAutocompleteCreditCardPrefix,
Expand Down Expand Up @@ -202,6 +205,10 @@ struct SignificantFields {
// True if the current form has only username, but no passwords.
bool is_single_username = false;

// True if the current form accepts webauthn crendentials from an active
// webauthn request.
bool accepts_webauthn_credentials = false;

// Returns true if some password field is present. This is the minimal
// requirement for a successful creation of a PasswordForm is present.
bool HasPasswords() const {
Expand Down Expand Up @@ -480,6 +487,7 @@ void ParseUsingAutocomplete(const std::vector<ProcessedField>& processed_fields,
result->confirmation_password = processed_field.field;
}
break;
case AutocompleteFlag::kWebAuthn:
case AutocompleteFlag::kNonPassword:
case AutocompleteFlag::kNone:
break;
Expand Down Expand Up @@ -976,6 +984,8 @@ std::unique_ptr<PasswordForm> AssemblePasswordForm(
significant_fields.is_new_password_reliable;
result->only_for_fallback = significant_fields.is_fallback;
result->submission_event = form_data.submission_event;
result->accepts_webauthn_credentials =
significant_fields.accepts_webauthn_credentials;

for (const FormFieldData& field : form_data.fields) {
if (field.form_control_type == "password" &&
Expand Down Expand Up @@ -1093,6 +1103,15 @@ std::unique_ptr<PasswordForm> FormDataParser::Parse(const FormData& form_data,
base::FeatureList::IsEnabled(
features::kTreatNewPasswordHeuristicsAsReliable));

if (mode == Mode::kFilling) {
for (const auto& field : processed_fields) {
if (field.autocomplete_flag == AutocompleteFlag::kWebAuthn) {
significant_fields.accepts_webauthn_credentials = true;
break;
}
}
}

base::UmaHistogramEnumeration("PasswordManager.UsernameDetectionMethod",
method);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ enum class AutocompleteFlag {
kUsername,
kCurrentPassword,
kNewPassword,
kWebAuthn,
// Represents the whole family of cc-* flags + OTP flag.
kNonPassword
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,7 @@ struct FormParsingTestCase {
SubmissionIndicatorEvent submission_event = SubmissionIndicatorEvent::NONE;
absl::optional<bool> is_new_password_reliable;
bool form_has_autofilled_value = false;
bool accepts_webauthn_credentials = false;
};

// Returns numbers which are distinct from each other within the scope of one
Expand Down Expand Up @@ -368,6 +369,9 @@ void CheckTestData(const std::vector<FormParsingTestCase>& test_cases) {
EXPECT_EQ(*test_case.is_new_password_reliable,
parsed_form->is_new_password_reliable);
}
EXPECT_EQ(test_case.accepts_webauthn_credentials &&
mode == FormDataParser::Mode::kFilling,
parsed_form->accepts_webauthn_credentials);
EXPECT_EQ(test_case.form_has_autofilled_value,
parsed_form->form_has_autofilled_value);

Expand Down Expand Up @@ -2852,6 +2856,29 @@ TEST(FormParserTest, UsernameWithTypePasswordAndServerPredictions) {
});
}

// Tests that if a field is marked as autofill="webauthn" then the
// `accepts_webauthn_credentials` flag is set.
TEST(FormParserTest, AcceptsWebAuthnCredentials) {
CheckTestData({
{
.description_for_logging = "Field tagged with autofill=\"webauthn\"",
.fields =
{
{.role = ElementRole::USERNAME,
.autocomplete_attribute = "webauthn",
.value = u"rosalina",
.name = u"username",
.form_control_type = "text"},
{.role = ElementRole::CURRENT_PASSWORD,
.value = u"luma",
.name = u"password",
.form_control_type = "password"},
},
.accepts_webauthn_credentials = true,
},
});
}

} // namespace

} // namespace password_manager
4 changes: 4 additions & 0 deletions components/password_manager/core/browser/password_form.h
Original file line number Diff line number Diff line change
Expand Up @@ -343,6 +343,10 @@ struct PasswordForm {
// password generation eligibility.
bool is_new_password_reliable = false;

// True iff the form may be filled with webauthn credentials from an active
// webauthn request.
bool accepts_webauthn_credentials = false;

// Serialized to prefs, so don't change numeric values!
// These values are persisted to logs. Entries should not be renumbered and
// numeric values should never be reused.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -240,6 +240,10 @@ LikelyFormFilling SendFillInformationToRenderer(
wait_for_username_reason = WaitForUsernameReason::kInsecureOrigin;
} else if (IsFillOnAccountSelectFeatureEnabled()) {
wait_for_username_reason = WaitForUsernameReason::kFoasFeature;
} else if (observed_form.accepts_webauthn_credentials &&
client->IsWebAuthnAutofillEnabled()) {
wait_for_username_reason =
WaitForUsernameReason::kAcceptsWebAuthnCredentials;
}

// Record no "FirstWaitForUsernameReason" metrics for a form that is not meant
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@ class MockPasswordManagerClient : public StubPasswordManagerClient {
(const GURL&),
(const, override));
MOCK_METHOD(bool, IsCommittedMainFrameSecure, (), (const, override));
MOCK_METHOD(bool, IsWebAuthnAutofillEnabled, (), (const, override));
};

PasswordForm CreateForm(std::u16string username,
Expand Down Expand Up @@ -263,6 +264,27 @@ TEST_F(PasswordFormFillingTest, TestFillOnLoadSuggestion) {
}
}

#if !defined(ANDROID) && !defined(OS_IOS)
TEST_F(PasswordFormFillingTest, DontFillOnLoadWebAuthnCredentials) {
observed_form_.accepts_webauthn_credentials = true;
for (bool webauthn_autofill_enabled : {false, true}) {
PasswordFormFillData fill_data;
EXPECT_CALL(client_, IsWebAuthnAutofillEnabled())
.WillOnce(Return(webauthn_autofill_enabled));
EXPECT_CALL(driver_, FillPasswordForm(_)).WillOnce(SaveArg<0>(&fill_data));
EXPECT_CALL(client_, PasswordWasAutofilled);
LikelyFormFilling likely_form_filling = SendFillInformationToRenderer(
&client_, &driver_, observed_form_, {&saved_match_}, federated_matches_,
&saved_match_, /*blocked_by_user=*/false, metrics_recorder_.get());
if (webauthn_autofill_enabled) {
EXPECT_EQ(LikelyFormFilling::kFillOnAccountSelect, likely_form_filling);
} else {
EXPECT_EQ(LikelyFormFilling::kFillOnPageLoad, likely_form_filling);
}
}
}
#endif

// Test autofill when username and password are prefilled. Overwrite password
// if server side classification thought the username was a placeholder or the
// classification failed. Do not overwrite if username doesn't look like a
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -230,7 +230,9 @@ class PasswordFormMetricsRecorder
kPasswordPrefilled = 8,
// A credential exists for affiliated website.
kAffiliatedWebsite = 9,
kMaxValue = kAffiliatedWebsite,
// The form may accept WebAuthn credentials.
kAcceptsWebAuthnCredentials = 10,
kMaxValue = kAcceptsWebAuthnCredentials,
};

// Used in UMA histogram, please do NOT reorder.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -167,4 +167,8 @@ network::mojom::NetworkContext* PasswordManagerClient::GetNetworkContext()
bool PasswordManagerClient::IsUnderAdvancedProtection() const {
return false;
}

bool PasswordManagerClient::IsWebAuthnAutofillEnabled() const {
return false;
}
} // namespace password_manager
Original file line number Diff line number Diff line change
Expand Up @@ -421,6 +421,9 @@ class PasswordManagerClient {
// Returns a FieldInfoManager associated with the current profile.
virtual FieldInfoManager* GetFieldInfoManager() const = 0;

// Returns true if integration between WebAuthn and Autofill is enabled.
virtual bool IsWebAuthnAutofillEnabled() const;

// Returns if the Autofill Assistant UI is shown.
virtual bool IsAutofillAssistantUIVisible() const = 0;

Expand Down
3 changes: 3 additions & 0 deletions tools/metrics/histograms/enums.xml
Original file line number Diff line number Diff line change
Expand Up @@ -63935,6 +63935,9 @@ Called by update_net_trust_anchors.py.-->
A credential exists for an affiliated matched site but not for the current
security origin.
</int>
<int value="10" label="Accepts WebAuthn credentials">
The form acccepts WebAuthn credentials from an active WebAuthn request.
</int>
</enum>

<enum name="PasswordManagerHttpCredentialType">
Expand Down

0 comments on commit 16a9ddd

Please sign in to comment.