[fido] Polish security key settings page

Polish the security key settings page:
* Wire up the forcePinChange flag. When a user attempts using a key
  with the forcePinChange flag on, they are shown a dialog explaining
  they have to set a new PIN.
* Dialogs that require the user to set a new PIN will now show a default
  button taking the user directly to the set PIN dialog.
* Fix the string shown when the user is required to set a new PIN on the
  bio enrollment dialog.

TBR'ing cpu@ for grdp updates.

UXW review: go/force-pin-change-ux

Tbr: cpu@chromium.org
Fixed: 1143427,1150131
Change-Id: Ice18acb7546cde4cd53d4dd4488636263325a50d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2561702Reviewed-by: Theodore Olsauskas-Warren <sauski@google.com>
Reviewed-by: Martin Kreichgauer <martinkr@google.com>
Reviewed-by: dpapad <dpapad@chromium.org>
Commit-Queue: Nina Satragno <nsatragno@chromium.org>
Cr-Commit-Position: refs/heads/master@{#832399}

chrome/app/settings_strings.grdp

@@ -3204,6 +3204,9 @@
   <message name="IDS_SETTINGS_SECURITY_KEYS_SET_PIN" desc="A header that appears in the Settings subpage for security keys (which are external devices for user authentication). PINs are often four-digit codes, commonly used to authorise the use of ATM cards. A similar mechanism can be used with security keys and this is the heading for the section about settings them.">
     Create a PIN
   </message>
+  <message name="IDS_SETTINGS_SECURITY_KEYS_SET_PIN_BUTTON" desc="The label of a button that appears in the Settings subpage for security keys (which are external devices for user authentication). The button takes the user to a page where they can set or change their PIN. PINs are often four-digit codes, commonly used to authorise the use of ATM cards. A similar mechanism can be used with security keys.">
+    Set PIN
+  </message>
   <message name="IDS_SETTINGS_SECURITY_KEYS_SET_PIN_DESC" desc="A description that appears in the Settings subpage for security keys (which are external devices for user authentication). PINs are often four-digit codes, commonly used to authorise the use of ATM cards. A similar mechanism can be used with security keys and this is the description for the section about settings them.">
     Protect your security key with a PIN (Personal Identification Number)
   </message>
@@ -3386,10 +3389,16 @@
   <message name="IDS_SETTINGS_SECURITY_KEYS_BIO_ENROLLMENT_DELETE" desc="The label for a button that when clicked deletes a fingerprint from a security key (an authentication hardware device) with a built-in fingerprint reader.">
     Delete this fingerprint
   </message>
+  <message name="IDS_SETTINGS_SECURITY_KEYS_BIO_NO_PIN" desc="An error message shown when a user attempts to enroll a fingerprint on their security key (an authentication hardware device), but the user has not set up a PIN (short, often numeric codes that are often used with, for example, ATM cards) for that security key.">
+    Your security key is not protected with a PIN. To manage fingerprints, first create a PIN.
+  </message>
   <message name="IDS_SETTINGS_SECURITY_KEYS_TOUCH_TO_CONTINUE" desc="A label instructing the user to physically touch the activation button on their security key (an authentication hardware device).">
     To continue, insert and touch your security key
   </message>
   <message name="IDS_SETTINGS_SECURITY_KEYS_PIN_PROMPT" desc="A label instructing the user to enter the PIN (short, often numeric codes that are often used with, for example, ATM cards) for their security key (an authentication hardware device).">
     Enter the PIN for your security key. If you don’t know the PIN, you’ll need to reset the security key.
   </message>
+  <message name="IDS_SETTINGS_SECURITY_KEYS_FORCE_PIN_CHANGE" desc="An error message shown when a user attempts to use a security key (an authentication hardware device) that has a default PIN (short, often numeric codes that are often used with, for example, ATM cards) set. Before using the key, the user needs to change the PIN to a new code.">
+    To use your new security key, set a new PIN
+  </message>
 </grit-part>

chrome/app/settings_strings_grdp/IDS_SETTINGS_SECURITY_KEYS_BIO_NO_PIN.png.sha1

+eb44b9901d0b216dffdf900a4622130c193ae9af
\ No newline at end of file

chrome/app/settings_strings_grdp/IDS_SETTINGS_SECURITY_KEYS_FORCE_PIN_CHANGE.png.sha1

+c16d9b56a804083f1767c81d54d9f02e0fc65433
\ No newline at end of file

chrome/app/settings_strings_grdp/IDS_SETTINGS_SECURITY_KEYS_SET_PIN_BUTTON.png.sha1

+eb44b9901d0b216dffdf900a4622130c193ae9af
\ No newline at end of file

chrome/browser/resources/settings/privacy_page/security_keys_bio_enroll_dialog.html

@@ -110,7 +110,7 @@
             hidden="[[!confirmButtonVisible_]]"
             disabled="[[confirmButtonDisabled_]]"
             on-click="confirmButtonClick_">
-          $i18n{continue}
+          [[confirmButtonLabel_]]
         </cr-button>
         <cr-button id="doneButton" class="action-button"
             on-click="done_" hidden="[[!doneButtonVisible_]]">

chrome/browser/resources/settings/privacy_page/security_keys_bio_enroll_dialog.js

@@ -63,6 +63,9 @@ Polymer({
     /** @private */
     confirmButtonVisible_: Boolean,
 
+    /** @private */
+    confirmButtonLabel_: String,
+
     /** @private */
     deleteInProgress_: Boolean,
 
@@ -104,6 +107,9 @@ Polymer({
   /** @private {string} */
   recentEnrollmentId_: '',
 
+  /** @private {boolean} */
+  showSetPINButton_: false,
+
   /** @override */
   attached() {
     afterNextRender(this, function() {
@@ -125,9 +131,11 @@ Polymer({
   /**
    * @private
    * @param {string} error
+   * @param {boolean=} requiresPINChange
    */
-  onError_(error) {
+  onError_(error, requiresPINChange = false) {
     this.errorMsg_ = error;
+    this.showSetPINButton_ = requiresPINChange;
     this.dialogPage_ = BioEnrollDialogPage.ERROR;
   },
 
@@ -175,6 +183,7 @@ Polymer({
         this.cancelButtonVisible_ = true;
         this.cancelButtonDisabled_ = false;
         this.confirmButtonVisible_ = true;
+        this.confirmButtonLabel_ = this.i18n('continue');
         this.confirmButtonDisabled_ = false;
         this.doneButtonVisible_ = false;
         this.$.pin.focus();
@@ -193,14 +202,16 @@ Polymer({
       case BioEnrollDialogPage.CHOOSE_NAME:
         this.cancelButtonVisible_ = false;
         this.confirmButtonVisible_ = true;
+        this.confirmButtonLabel_ = this.i18n('continue');
         this.confirmButtonDisabled_ = !this.recentEnrollmentName_.length;
         this.doneButtonVisible_ = false;
         this.$.enrollmentName.focus();
         break;
       case BioEnrollDialogPage.ERROR:
-        this.cancelButtonVisible_ = false;
-        this.confirmButtonVisible_ = false;
-        this.doneButtonVisible_ = true;
+        this.cancelButtonVisible_ = true;
+        this.confirmButtonVisible_ = this.showSetPINButton_;
+        this.confirmButtonLabel_ = this.i18n('securityKeysSetPinButton');
+        this.doneButtonVisible_ = false;
         break;
       default:
         assertNotReached();
@@ -303,6 +314,10 @@ Polymer({
       case BioEnrollDialogPage.CHOOSE_NAME:
         this.renameNewEnrollment_();
         break;
+      case BioEnrollDialogPage.ERROR:
+        this.$.dialog.close();
+        this.fire('bio-enroll-set-pin');
+        break;
       default:
         assertNotReached();
     }

chrome/browser/resources/settings/privacy_page/security_keys_credential_management_dialog.js

@@ -95,6 +95,9 @@ Polymer({
   /** @private {?Set<string>} */
   checkedCredentialIds_: null,
 
+  /** @private {boolean} */
+  showSetPINButton_: false,
+
   /** @override */
   attached() {
     this.$.dialog.showModal();
@@ -112,9 +115,11 @@ Polymer({
   /**
    * @private
    * @param {string} error
+   * @param {boolean=} requiresPINChange
    */
-  onError_(error) {
+  onError_(error, requiresPINChange = false) {
     this.errorMsg_ = error;
+    this.showSetPINButton_ = requiresPINChange;
     this.dialogPage_ = CredentialManagementDialogPage.ERROR;
   },
 
@@ -175,9 +180,10 @@ Polymer({
         this.closeButtonVisible_ = false;
         break;
       case CredentialManagementDialogPage.ERROR:
-        this.cancelButtonVisible_ = false;
-        this.confirmButtonVisible_ = false;
-        this.closeButtonVisible_ = true;
+        this.cancelButtonVisible_ = true;
+        this.confirmButtonLabel_ = this.i18n('securityKeysSetPinButton');
+        this.confirmButtonVisible_ = this.showSetPINButton_;
+        this.closeButtonVisible_ = false;
         break;
       default:
         assertNotReached();
@@ -194,6 +200,10 @@ Polymer({
       case CredentialManagementDialogPage.CREDENTIALS:
         this.deleteSelectedCredentials_();
         break;
+      case CredentialManagementDialogPage.ERROR:
+        this.$.dialog.close();
+        this.fire('credential-management-set-pin');
+        break;
       default:
         assertNotReached();
     }
@@ -274,10 +284,10 @@ Polymer({
     this.confirmButtonDisabled_ = true;
     this.deleteInProgress_ = true;
     this.browserProxy_.deleteCredentials(Array.from(this.checkedCredentialIds_))
-        .then((err) => {
+        .then((error) => {
           this.confirmButtonDisabled_ = false;
           this.deleteInProgress_ = false;
-          this.onError_(err);
+          this.onError_(error);
         });
   },
 });

chrome/browser/resources/settings/privacy_page/security_keys_subpage.html

@@ -33,6 +33,7 @@
 
     <template is="dom-if" if="[[showCredentialManagementDialog_]]" restamp>
       <settings-security-keys-credential-management-dialog
+        on-credential-management-set-pin="onSetPIN_"
         on-close="onCredentialManagementDialogClosed_">
       </settings-security-keys-credential-management-dialog>
     </template>
@@ -44,6 +45,7 @@
 
     <template is="dom-if" if="[[showBioEnrollDialog_]]" restamp>
       <settings-security-keys-bio-enroll-dialog
+          on-bio-enroll-set-pin="onSetPIN_"
           on-close="onBioEnrollDialogClosed_">
       </settings-security-keys-bio-enroll-dialog>
     </template>

chrome/browser/ui/webui/settings/settings_localized_strings_provider.cc

@@ -2379,6 +2379,7 @@ void AddSecurityKeysStrings(content::WebUIDataSource* html_source) {
       {"securityKeysTitle", IDS_SETTINGS_SECURITY_KEYS_TITLE},
       {"securityKeysTouchToContinue",
        IDS_SETTINGS_SECURITY_KEYS_TOUCH_TO_CONTINUE},
+      {"securityKeysSetPinButton", IDS_SETTINGS_SECURITY_KEYS_SET_PIN_BUTTON},
   };
   AddLocalizedStringsBulk(html_source, kSecurityKeysStrings);
   bool win_native_api_available = false;

chrome/browser/ui/webui/settings/settings_security_key_handler.cc

@@ -19,7 +19,6 @@
 #include "device/fido/bio/enrollment_handler.h"
 #include "device/fido/credential_management.h"
 #include "device/fido/credential_management_handler.h"
-#include "device/fido/fido_discovery_factory.h"
 #include "device/fido/pin.h"
 #include "device/fido/reset_request_handler.h"
 #include "device/fido/set_pin_request_handler.h"
@@ -53,6 +52,12 @@ base::DictionaryValue EncodeEnrollment(const std::vector<uint8_t>& id,
 
 namespace settings {
 
+SecurityKeysHandlerBase::SecurityKeysHandlerBase() = default;
+SecurityKeysHandlerBase::SecurityKeysHandlerBase(
+    std::unique_ptr<device::FidoDiscoveryFactory> discovery_factory)
+    : discovery_factory_(std::move(discovery_factory)) {}
+SecurityKeysHandlerBase::~SecurityKeysHandlerBase() = default;
+
 void SecurityKeysHandlerBase::OnJavascriptAllowed() {}
 
 void SecurityKeysHandlerBase::OnJavascriptDisallowed() {
@@ -285,8 +290,34 @@ void SecurityKeysResetHandler::OnResetFinished(
 }
 
 SecurityKeysCredentialHandler::SecurityKeysCredentialHandler() = default;
+SecurityKeysCredentialHandler::SecurityKeysCredentialHandler(
+    std::unique_ptr<device::FidoDiscoveryFactory> discovery_factory)
+    : SecurityKeysHandlerBase(std::move(discovery_factory)) {}
 SecurityKeysCredentialHandler::~SecurityKeysCredentialHandler() = default;
 
+void SecurityKeysCredentialHandler::HandleStart(const base::ListValue* args) {
+  DCHECK_EQ(State::kNone, state_);
+  DCHECK_CURRENTLY_ON(BrowserThread::UI);
+  DCHECK_EQ(1u, args->GetSize());
+  DCHECK(!credential_management_);
+
+  AllowJavascript();
+  DCHECK(callback_id_.empty());
+  callback_id_ = args->GetList()[0].GetString();
+
+  state_ = State::kStart;
+  credential_management_ =
+      std::make_unique<device::CredentialManagementHandler>(
+          discovery_factory(), supported_transports(),
+          base::BindOnce(
+              &SecurityKeysCredentialHandler::OnCredentialManagementReady,
+              weak_factory_.GetWeakPtr()),
+          base::BindRepeating(&SecurityKeysCredentialHandler::OnGatherPIN,
+                              weak_factory_.GetWeakPtr()),
+          base::BindOnce(&SecurityKeysCredentialHandler::OnFinished,
+                         weak_factory_.GetWeakPtr()));
+}
+
 void SecurityKeysCredentialHandler::RegisterMessages() {
   web_ui()->RegisterMessageCallback(
       "securityKeyCredentialManagementStart",
@@ -316,38 +347,12 @@ void SecurityKeysCredentialHandler::Close() {
   // Invalidate all existing WeakPtrs so that no stale callbacks occur.
   weak_factory_.InvalidateWeakPtrs();
   state_ = State::kNone;
-  discovery_factory_.reset();
   credential_management_.reset();
   callback_id_.clear();
   credential_management_provide_pin_cb_.Reset();
   DCHECK(!credential_management_provide_pin_cb_);
 }
 
-void SecurityKeysCredentialHandler::HandleStart(const base::ListValue* args) {
-  DCHECK_EQ(State::kNone, state_);
-  DCHECK_CURRENTLY_ON(BrowserThread::UI);
-  DCHECK_EQ(1u, args->GetSize());
-  DCHECK(!credential_management_);
-  DCHECK(!discovery_factory_);
-
-  AllowJavascript();
-  DCHECK(callback_id_.empty());
-  callback_id_ = args->GetList()[0].GetString();
-
-  state_ = State::kStart;
-  discovery_factory_ = std::make_unique<device::FidoDiscoveryFactory>();
-  credential_management_ =
-      std::make_unique<device::CredentialManagementHandler>(
-          discovery_factory_.get(), supported_transports(),
-          base::BindOnce(
-              &SecurityKeysCredentialHandler::OnCredentialManagementReady,
-              weak_factory_.GetWeakPtr()),
-          base::BindRepeating(&SecurityKeysCredentialHandler::OnGatherPIN,
-                              weak_factory_.GetWeakPtr()),
-          base::BindOnce(&SecurityKeysCredentialHandler::OnFinished,
-                         weak_factory_.GetWeakPtr()));
-}
-
 void SecurityKeysCredentialHandler::HandlePIN(const base::ListValue* args) {
   DCHECK_EQ(State::kPIN, state_);
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
@@ -506,7 +511,7 @@ void SecurityKeysCredentialHandler::OnFinished(
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
 
   int error;
-
+  bool requires_pin_change = false;
   switch (status) {
     case device::CredentialManagementStatus::kSoftPINBlock:
       error = IDS_SETTINGS_SECURITY_KEYS_PIN_SOFT_LOCK;
@@ -519,23 +524,53 @@ void SecurityKeysCredentialHandler::OnFinished(
       error = IDS_SETTINGS_SECURITY_KEYS_NO_CREDENTIAL_MANAGEMENT;
       break;
     case device::CredentialManagementStatus::kNoPINSet:
+      requires_pin_change = true;
       error = IDS_SETTINGS_SECURITY_KEYS_CREDENTIAL_MANAGEMENT_NO_PIN;
       break;
     case device::CredentialManagementStatus::kAuthenticatorResponseInvalid:
       error = IDS_SETTINGS_SECURITY_KEYS_CREDENTIAL_MANAGEMENT_ERROR;
       break;
+    case device::CredentialManagementStatus::kForcePINChange:
+      requires_pin_change = true;
+      error = IDS_SETTINGS_SECURITY_KEYS_FORCE_PIN_CHANGE;
+      break;
     case device::CredentialManagementStatus::kSuccess:
       error = IDS_SETTINGS_SECURITY_KEYS_CREDENTIAL_MANAGEMENT_REMOVED;
       break;
   }
 
   FireWebUIListener("security-keys-credential-management-finished",
-                    base::Value(l10n_util::GetStringUTF8(std::move(error))));
+                    base::Value(l10n_util::GetStringUTF8(error)),
+                    base::Value(requires_pin_change));
 }
 
 SecurityKeysBioEnrollmentHandler::SecurityKeysBioEnrollmentHandler() = default;
+SecurityKeysBioEnrollmentHandler::SecurityKeysBioEnrollmentHandler(
+    std::unique_ptr<device::FidoDiscoveryFactory> discovery_factory)
+    : SecurityKeysHandlerBase(std::move(discovery_factory)) {}
 SecurityKeysBioEnrollmentHandler::~SecurityKeysBioEnrollmentHandler() = default;
 
+void SecurityKeysBioEnrollmentHandler::HandleStart(
+    const base::ListValue* args) {
+  DCHECK_CURRENTLY_ON(BrowserThread::UI);
+  DCHECK_EQ(state_, State::kNone);
+  DCHECK_EQ(1u, args->GetSize());
+  DCHECK(callback_id_.empty());
+
+  AllowJavascript();
+  state_ = State::kStart;
+  callback_id_ = args->GetList()[0].GetString();
+  bio_ = std::make_unique<device::BioEnrollmentHandler>(
+      supported_transports(),
+      base::BindOnce(&SecurityKeysBioEnrollmentHandler::OnReady,
+                     weak_factory_.GetWeakPtr()),
+      base::BindOnce(&SecurityKeysBioEnrollmentHandler::OnError,
+                     weak_factory_.GetWeakPtr()),
+      base::BindRepeating(&SecurityKeysBioEnrollmentHandler::OnGatherPIN,
+                          weak_factory_.GetWeakPtr()),
+      discovery_factory());
+}
+
 void SecurityKeysBioEnrollmentHandler::RegisterMessages() {
   web_ui()->RegisterMessageCallback(
       "securityKeyBioEnrollStart",
@@ -577,35 +612,11 @@ void SecurityKeysBioEnrollmentHandler::RegisterMessages() {
 void SecurityKeysBioEnrollmentHandler::Close() {
   weak_factory_.InvalidateWeakPtrs();
   state_ = State::kNone;
-  discovery_factory_.reset();
   bio_.reset();
   callback_id_.clear();
-  discovery_factory_.reset();
   provide_pin_cb_.Reset();
 }
 
-void SecurityKeysBioEnrollmentHandler::HandleStart(
-    const base::ListValue* args) {
-  DCHECK_CURRENTLY_ON(BrowserThread::UI);
-  DCHECK_EQ(state_, State::kNone);
-  DCHECK_EQ(1u, args->GetSize());
-  DCHECK(callback_id_.empty());
-
-  AllowJavascript();
-  state_ = State::kStart;
-  callback_id_ = args->GetList()[0].GetString();
-  discovery_factory_ = std::make_unique<device::FidoDiscoveryFactory>();
-  bio_ = std::make_unique<device::BioEnrollmentHandler>(
-      supported_transports(),
-      base::BindOnce(&SecurityKeysBioEnrollmentHandler::OnReady,
-                     weak_factory_.GetWeakPtr()),
-      base::BindOnce(&SecurityKeysBioEnrollmentHandler::OnError,
-                     weak_factory_.GetWeakPtr()),
-      base::BindRepeating(&SecurityKeysBioEnrollmentHandler::OnGatherPIN,
-                          weak_factory_.GetWeakPtr()),
-      discovery_factory_.get());
-}
-
 void SecurityKeysBioEnrollmentHandler::OnReady() {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
   DCHECK(bio_);
@@ -622,6 +633,7 @@ void SecurityKeysBioEnrollmentHandler::OnError(
   state_ = State::kNone;
 
   int error;
+  bool requires_pin_change = false;
   switch (status) {
     case device::BioEnrollmentStatus::kSoftPINBlock:
       error = IDS_SETTINGS_SECURITY_KEYS_PIN_SOFT_LOCK;
@@ -633,18 +645,24 @@ void SecurityKeysBioEnrollmentHandler::OnError(
       error = IDS_SETTINGS_SECURITY_KEYS_NO_BIOMETRIC_ENROLLMENT;
       break;
     case device::BioEnrollmentStatus::kNoPINSet:
-      error = IDS_SETTINGS_SECURITY_KEYS_CREDENTIAL_MANAGEMENT_NO_PIN;
+      requires_pin_change = true;
+      error = IDS_SETTINGS_SECURITY_KEYS_BIO_NO_PIN;
       break;
     case device::BioEnrollmentStatus::kAuthenticatorResponseInvalid:
       error = IDS_SETTINGS_SECURITY_KEYS_CREDENTIAL_MANAGEMENT_ERROR;
       break;
+    case device::BioEnrollmentStatus::kForcePINChange:
+      requires_pin_change = true;
+      error = IDS_SETTINGS_SECURITY_KEYS_FORCE_PIN_CHANGE;
+      break;
     case device::BioEnrollmentStatus::kSuccess:
       error = IDS_SETTINGS_SECURITY_KEYS_CREDENTIAL_MANAGEMENT_REMOVED;
       break;
   }
 
   FireWebUIListener("security-keys-bio-enroll-error",
-                    base::Value(l10n_util::GetStringUTF8(error)));
+                    base::Value(l10n_util::GetStringUTF8(error)),
+                    base::Value(requires_pin_change));
 
   // If |callback_id_| is not empty, there is an ongoing operation,
   // which means there is an unresolved Promise. Reject it so that

chrome/browser/ui/webui/settings/settings_security_key_handler.h

@@ -12,9 +12,9 @@
 #include "base/memory/weak_ptr.h"
 #include "base/optional.h"
 #include "chrome/browser/ui/webui/settings/settings_page_ui_handler.h"
-#include "device/fido/fido_constants.h"
-
 #include "device/fido/bio/enrollment.h"
+#include "device/fido/fido_constants.h"
+#include "device/fido/fido_discovery_factory.h"
 
 namespace base {
 class ListValue;
@@ -22,7 +22,6 @@ class ListValue;
 
 namespace device {
 struct AggregatedEnumerateCredentialsResponse;
-class FidoDiscoveryFactory;
 class CredentialManagementHandler;
 enum class CredentialManagementStatus;
 class SetPINRequestHandler;
@@ -40,14 +39,25 @@ class SecurityKeysHandlerBase : public SettingsPageUIHandler {
   SecurityKeysHandlerBase& operator=(const SecurityKeysHandlerBase&) = delete;
 
  protected:
-  SecurityKeysHandlerBase() = default;
+  SecurityKeysHandlerBase();
+  explicit SecurityKeysHandlerBase(
+      std::unique_ptr<device::FidoDiscoveryFactory> discovery_factory);
+  ~SecurityKeysHandlerBase() override;
 
   // Subclasses must implement close to invalidate all pending callbacks.
   virtual void Close() = 0;
 
+  // Returns the discovery factory to be used for the request.
+  device::FidoDiscoveryFactory* discovery_factory() {
+    return discovery_factory_.get();
+  }
+
  private:
   void OnJavascriptAllowed() override;
   void OnJavascriptDisallowed() override;
+
+  std::unique_ptr<device::FidoDiscoveryFactory> discovery_factory_ =
+      std::make_unique<device::FidoDiscoveryFactory>();
 };
 
 // SecurityKeysPINHandler processes messages from the "Create a PIN" dialog of
@@ -130,6 +140,11 @@ class SecurityKeysCredentialHandler : public SecurityKeysHandlerBase {
   SecurityKeysCredentialHandler();
   ~SecurityKeysCredentialHandler() override;
 
+ protected:
+  explicit SecurityKeysCredentialHandler(
+      std::unique_ptr<device::FidoDiscoveryFactory> discovery_factory);
+  void HandleStart(const base::ListValue* args);
+
  private:
   enum class State {
     kNone,
@@ -143,7 +158,6 @@ class SecurityKeysCredentialHandler : public SecurityKeysHandlerBase {
   void RegisterMessages() override;
   void Close() override;
 
-  void HandleStart(const base::ListValue* args);
   void HandlePIN(const base::ListValue* args);
   void HandleEnumerate(const base::ListValue* args);
   void HandleDelete(const base::ListValue* args);
@@ -164,7 +178,6 @@ class SecurityKeysCredentialHandler : public SecurityKeysHandlerBase {
   State state_ = State::kNone;
   base::OnceCallback<void(std::string)> credential_management_provide_pin_cb_;
 
-  std::unique_ptr<device::FidoDiscoveryFactory> discovery_factory_;
   std::unique_ptr<device::CredentialManagementHandler> credential_management_;
 
   std::string callback_id_;
@@ -180,6 +193,11 @@ class SecurityKeysBioEnrollmentHandler : public SecurityKeysHandlerBase {
   SecurityKeysBioEnrollmentHandler();
   ~SecurityKeysBioEnrollmentHandler() override;
 
+ protected:
+  explicit SecurityKeysBioEnrollmentHandler(
+      std::unique_ptr<device::FidoDiscoveryFactory> discovery_factory);
+  void HandleStart(const base::ListValue* args);
+
  private:
   enum class State {
     kNone,
@@ -195,7 +213,6 @@ class SecurityKeysBioEnrollmentHandler : public SecurityKeysHandlerBase {
   void RegisterMessages() override;
   void Close() override;
 
-  void HandleStart(const base::ListValue* args);
   void OnReady();
   void OnError(device::BioEnrollmentStatus status);
   void OnGatherPIN(uint32_t min_pin_length,
@@ -229,7 +246,6 @@ class SecurityKeysBioEnrollmentHandler : public SecurityKeysHandlerBase {
   State state_ = State::kNone;
   std::string callback_id_;
   base::OnceCallback<void(std::string)> provide_pin_cb_;
-  std::unique_ptr<device::FidoDiscoveryFactory> discovery_factory_;
   std::unique_ptr<device::BioEnrollmentHandler> bio_;
   base::WeakPtrFactory<SecurityKeysBioEnrollmentHandler> weak_factory_{this};
 };

chrome/browser/ui/webui/settings/settings_security_key_handler_unittest.cc

+// Copyright 2020 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "chrome/browser/ui/webui/settings/settings_security_key_handler.h"
+
+#include <memory>
+
+#include "chrome/grit/generated_resources.h"
+#include "chrome/test/base/chrome_render_view_host_test_harness.h"
+#include "content/public/test/test_web_ui.h"
+#include "device/fido/fake_fido_discovery.h"
+#include "device/fido/fido_discovery_factory.h"
+#include "device/fido/fido_types.h"
+#include "device/fido/virtual_fido_device_factory.h"
+#include "testing/gtest/include/gtest/gtest.h"
+#include "ui/base/l10n/l10n_util.h"
+
+namespace settings {
+
+namespace {
+
+class TestSecurityKeysCredentialHandler : public SecurityKeysCredentialHandler {
+ public:
+  explicit TestSecurityKeysCredentialHandler(content::TestWebUI* web_ui)
+      : SecurityKeysCredentialHandler(
+            std::make_unique<device::test::VirtualFidoDeviceFactory>()) {
+    set_web_ui(web_ui);
+    AllowJavascriptForTesting();
+  }
+
+  using SecurityKeysCredentialHandler::HandleStart;
+
+  device::test::VirtualFidoDeviceFactory* GetDiscoveryFactory() {
+    return static_cast<device::test::VirtualFidoDeviceFactory*>(
+        discovery_factory());
+  }
+};
+
+class TestSecurityKeysBioEnrollmentHandler
+    : public SecurityKeysBioEnrollmentHandler {
+ public:
+  explicit TestSecurityKeysBioEnrollmentHandler(content::TestWebUI* web_ui)
+      : SecurityKeysBioEnrollmentHandler(
+            std::make_unique<device::test::VirtualFidoDeviceFactory>()) {
+    set_web_ui(web_ui);
+    AllowJavascriptForTesting();
+  }
+
+  using SecurityKeysBioEnrollmentHandler::HandleStart;
+
+  device::test::VirtualFidoDeviceFactory* GetDiscoveryFactory() {
+    return static_cast<device::test::VirtualFidoDeviceFactory*>(
+        discovery_factory());
+  }
+};
+
+}  // namespace
+
+class SecurityKeysCredentialHandlerTest
+    : public ChromeRenderViewHostTestHarness {
+ protected:
+  void SetUp() override {
+    ChromeRenderViewHostTestHarness::SetUp();
+    web_ui_ = std::make_unique<content::TestWebUI>();
+    web_ui_->set_web_contents(web_contents());
+    handler_ =
+        std::make_unique<TestSecurityKeysCredentialHandler>(web_ui_.get());
+    web_ui_->ClearTrackedCalls();
+  }
+
+  std::unique_ptr<TestSecurityKeysCredentialHandler> handler_;
+  std::unique_ptr<content::TestWebUI> web_ui_;
+};
+
+TEST_F(SecurityKeysCredentialHandlerTest, TestForcePINChange) {
+  handler_->GetDiscoveryFactory()->mutable_state()->force_pin_change = true;
+  handler_->GetDiscoveryFactory()->mutable_state()->pin = "1234";
+  device::VirtualCtap2Device::Config config;
+  config.pin_support = true;
+  config.pin_uv_auth_token_support = true;
+  config.min_pin_length_support = true;
+  config.credential_management_support = true;
+  config.ctap2_versions = {device::Ctap2Version::kCtap2_1};
+  handler_->GetDiscoveryFactory()->SetCtap2Config(config);
+
+  std::string callback_id("start_callback_id");
+  base::ListValue args;
+  args.Append(callback_id);
+  handler_->HandleStart(&args);
+  base::RunLoop().RunUntilIdle();
+
+  EXPECT_EQ(web_ui_->call_data()[0]->arg1()->GetString(),
+            "security-keys-credential-management-finished");
+  EXPECT_EQ(
+      web_ui_->call_data()[0]->arg2()->GetString(),
+      l10n_util::GetStringUTF8(IDS_SETTINGS_SECURITY_KEYS_FORCE_PIN_CHANGE));
+  EXPECT_EQ(web_ui_->call_data()[0]->arg3()->GetBool(), true);
+}
+
+class SecurityKeysBioEnrollmentHandlerTest
+    : public ChromeRenderViewHostTestHarness {
+ protected:
+  void SetUp() override {
+    ChromeRenderViewHostTestHarness::SetUp();
+    web_ui_ = std::make_unique<content::TestWebUI>();
+    web_ui_->set_web_contents(web_contents());
+    handler_ =
+        std::make_unique<TestSecurityKeysBioEnrollmentHandler>(web_ui_.get());
+    web_ui_->ClearTrackedCalls();
+  }
+
+  std::unique_ptr<TestSecurityKeysBioEnrollmentHandler> handler_;
+  std::unique_ptr<content::TestWebUI> web_ui_;
+};
+
+TEST_F(SecurityKeysBioEnrollmentHandlerTest, TestForcePINChange) {
+  handler_->GetDiscoveryFactory()->mutable_state()->force_pin_change = true;
+  handler_->GetDiscoveryFactory()->mutable_state()->pin = "1234";
+  device::VirtualCtap2Device::Config config;
+  config.internal_uv_support = true;
+  config.bio_enrollment_support = true;
+  config.pin_support = true;
+  config.pin_uv_auth_token_support = true;
+  config.min_pin_length_support = true;
+  config.credential_management_support = true;
+  config.ctap2_versions = {device::Ctap2Version::kCtap2_1};
+  handler_->GetDiscoveryFactory()->SetCtap2Config(config);
+
+  std::string callback_id("start_callback_id");
+  base::ListValue args;
+  args.Append(callback_id);
+  handler_->HandleStart(&args);
+  base::RunLoop().RunUntilIdle();
+
+  EXPECT_EQ(web_ui_->call_data()[0]->arg1()->GetString(),
+            "security-keys-bio-enroll-error");
+  EXPECT_EQ(
+      web_ui_->call_data()[0]->arg2()->GetString(),
+      l10n_util::GetStringUTF8(IDS_SETTINGS_SECURITY_KEYS_FORCE_PIN_CHANGE));
+  EXPECT_EQ(web_ui_->call_data()[0]->arg3()->GetBool(), true);
+}
+
+}  // namespace settings

chrome/test/BUILD.gn

@@ -4671,6 +4671,7 @@ test("unit_tests") {
       "../browser/ui/webui/settings/safety_check_handler_unittest.cc",
       "../browser/ui/webui/settings/settings_cookies_view_handler_unittest.cc",
       "../browser/ui/webui/settings/settings_manage_profile_handler_unittest.cc",
+      "../browser/ui/webui/settings/settings_security_key_handler_unittest.cc",
       "../browser/ui/webui/settings/site_settings_handler_unittest.cc",
       "../browser/ui/webui/settings/site_settings_helper_unittest.cc",
       "../browser/ui/webui/settings_utils_unittest.cc",

chrome/test/data/webui/settings/security_keys_subpage_test.js

@@ -628,11 +628,35 @@ suite('SecurityKeysCredentialManagement', function() {
     assertShown(allDivs, dialog, 'initial');
     startResolver.resolve([currentMinPinLength]);
 
-    const errorString = 'foo bar baz';
+    const error = 'foo bar baz';
     webUIListenerCallback(
-        'security-keys-credential-management-finished', errorString);
+        'security-keys-credential-management-finished', error);
     assertShown(allDivs, dialog, 'error');
-    assertTrue(dialog.$.error.textContent.trim().includes(errorString));
+    assertTrue(dialog.$.error.textContent.trim().includes(error));
+  });
+
+  test('PINChangeError', async function() {
+    const startResolver = new PromiseResolver();
+    browserProxy.setResponseFor(
+        'startCredentialManagement', startResolver.promise);
+
+    document.body.appendChild(dialog);
+    await browserProxy.whenCalled('startCredentialManagement');
+    assertShown(allDivs, dialog, 'initial');
+    startResolver.resolve([currentMinPinLength]);
+
+    const error = 'foo bar baz';
+    webUIListenerCallback(
+        'security-keys-credential-management-finished', error,
+        true /* requiresPINChange */);
+    assertShown(allDivs, dialog, 'error');
+    assertFalse(dialog.$.confirmButton.hidden);
+    assertFalse(dialog.$.confirmButton.disabled);
+    assertTrue(dialog.$.error.textContent.trim().includes(error));
+
+    const setPinEvent = eventToPromise('credential-management-set-pin', dialog);
+    dialog.$.confirmButton.click();
+    await setPinEvent;
   });
 
   test('Credentials', async function() {
@@ -754,10 +778,33 @@ suite('SecurityKeysBioEnrollment', function() {
     assertShown(allDivs, dialog, 'initial');
     resolver.resolve([currentMinPinLength]);
 
-    const errorString = 'foo bar baz';
-    webUIListenerCallback('security-keys-bio-enroll-error', errorString);
+    const error = 'foo bar baz';
+    webUIListenerCallback('security-keys-bio-enroll-error', error);
     assertShown(allDivs, dialog, 'error');
-    assertTrue(dialog.$.error.textContent.trim().includes(errorString));
+    assertTrue(dialog.$.confirmButton.hidden);
+    assertTrue(dialog.$.error.textContent.trim().includes(error));
+  });
+
+  test('PINChangeError', async function() {
+    const resolver = new PromiseResolver();
+    browserProxy.setResponseFor('startBioEnroll', resolver.promise);
+
+    document.body.appendChild(dialog);
+    await browserProxy.whenCalled('startBioEnroll');
+    assertShown(allDivs, dialog, 'initial');
+    resolver.resolve([currentMinPinLength]);
+
+    const error = 'something about setting a new PIN';
+    webUIListenerCallback(
+        'security-keys-bio-enroll-error', error, true /* requiresPINChange */);
+    assertShown(allDivs, dialog, 'error');
+    assertFalse(dialog.$.confirmButton.hidden);
+    assertFalse(dialog.$.confirmButton.disabled);
+    assertTrue(dialog.$.error.textContent.trim().includes(error));
+
+    const setPinEvent = eventToPromise('bio-enroll-set-pin', dialog);
+    dialog.$.confirmButton.click();
+    await setPinEvent;
   });
 
   test('Enrollments', async function() {

device/fido/bio/enrollment_handler.cc

@@ -165,6 +165,11 @@ void BioEnrollmentHandler::OnTouch(FidoAuthenticator* authenticator) {
     return;
   }
 
+  if (authenticator->ForcePINChange()) {
+    Finish(BioEnrollmentStatus::kForcePINChange);
+    return;
+  }
+
   authenticator_ = authenticator;
   state_ = State::kGettingRetries;
   authenticator_->GetPinRetries(base::BindOnce(

device/fido/bio/enrollment_handler.h

@@ -28,6 +28,7 @@ enum class BioEnrollmentStatus {
   kHardPINBlock,
   kNoPINSet,
   kAuthenticatorMissingBioEnrollment,
+  kForcePINChange,
 };
 
 // BioEnrollmentHandler exercises the CTAP2.1 authenticatorBioEnrollment

device/fido/bio/enrollment_handler_unittest.cc

@@ -101,6 +101,24 @@ TEST_F(BioEnrollmentHandlerTest, NoPINSupport) {
   EXPECT_EQ(error_callback_.value(), BioEnrollmentStatus::kNoPINSet);
 }
 
+// Tests bio enrollment handler against device with the forcePINChange flag on.
+TEST_F(BioEnrollmentHandlerTest, ForcePINChange) {
+  VirtualCtap2Device::Config config;
+  config.pin_support = true;
+  config.bio_enrollment_preview_support = true;
+  config.min_pin_length_support = true;
+  config.pin_uv_auth_token_support = true;
+  config.ctap2_versions = {Ctap2Version::kCtap2_1};
+  virtual_device_factory_.mutable_state()->force_pin_change = true;
+
+  virtual_device_factory_.SetCtap2Config(config);
+
+  auto handler = MakeHandler();
+  error_callback_.WaitForCallback();
+
+  EXPECT_EQ(error_callback_.value(), BioEnrollmentStatus::kForcePINChange);
+}
+
 // Tests enrollment handler PIN soft block.
 TEST_F(BioEnrollmentHandlerTest, SoftPINBlock) {
   VirtualCtap2Device::Config config;

device/fido/credential_management_handler.cc

@@ -77,6 +77,13 @@ void CredentialManagementHandler::OnTouch(FidoAuthenticator* authenticator) {
     return;
   }
 
+  if (authenticator->ForcePINChange()) {
+    state_ = State::kFinished;
+    std::move(finished_callback_)
+        .Run(CredentialManagementStatus::kForcePINChange);
+    return;
+  }
+
   authenticator_ = authenticator;
   authenticator_->GetPinRetries(
       base::BindOnce(&CredentialManagementHandler::OnRetriesResponse,

device/fido/credential_management_handler.h

@@ -32,6 +32,7 @@ enum class CredentialManagementStatus {
   kHardPINBlock,
   kAuthenticatorMissingCredentialManagement,
   kNoPINSet,
+  kForcePINChange,
 };
 
 // CredentialManagementHandler implements the authenticatorCredentialManagement

device/fido/credential_management_handler_unittest.cc

@@ -113,8 +113,28 @@ TEST_F(CredentialManagementHandlerTest, Test) {
   EXPECT_FALSE(finished_callback_.was_called());
 }
 
+TEST_F(CredentialManagementHandlerTest, TestForcePINChange) {
+  virtual_device_factory_.mutable_state()->pin = kPIN;
+  virtual_device_factory_.mutable_state()->force_pin_change = true;
+
+  VirtualCtap2Device::Config ctap_config;
+  ctap_config.pin_support = true;
+  ctap_config.resident_key_support = true;
+  ctap_config.credential_management_support = true;
+  ctap_config.min_pin_length_support = true;
+  ctap_config.pin_uv_auth_token_support = true;
+  ctap_config.ctap2_versions = {Ctap2Version::kCtap2_1};
+  virtual_device_factory_.SetCtap2Config(ctap_config);
+  virtual_device_factory_.SetSupportedProtocol(device::ProtocolVersion::kCtap2);
+
+  auto handler = MakeHandler();
+  finished_callback_.WaitForCallback();
+  ASSERT_EQ(finished_callback_.value(),
+            CredentialManagementStatus::kForcePINChange);
+}
+
 TEST_F(CredentialManagementHandlerTest,
-       EnmerateCredentialResponse_TruncatedUTF8) {
+       EnumerateCredentialResponse_TruncatedUTF8) {
   // Webauthn says[1] that authenticators may truncate strings in user entities.
   // Since authenticators aren't going to do UTF-8 processing, that means that
   // they may truncate a multi-byte code point and thus produce an invalid