Commit e9af01ff authored by Nina Satragno's avatar Nina Satragno Committed by Chromium LUCI CQ

[fido] Settings error when new PIN same as current

Show an error message on the Security Keys Set PIN settings page when
the new PIN is the same as the current PIN. Under some circumstances,
keys won't accept a PIN that is equal, so show an error early to be
safe.

The string provided was wider than the input, so to avoid shifting the
confirm input to the right, the error text is allowed to overflow with
white-space: nowrap (see the screenshot).

UXR review: go/force-pin-change-ux
Screenshot:
https://storage.cloud.google.com/chromium-translation-screenshots/a775ab5fe5a6302c4f83dc1b4863d21ff7b20ff0

Tbr: cpu@chromium.org
Fixed: 1152510
Change-Id: Id17f114c240469608fb0ef785bc5db4daba11a6c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2615458Reviewed-by: default avatardpapad <dpapad@chromium.org>
Reviewed-by: default avatarTheodore Olsauskas-Warren <sauski@google.com>
Commit-Queue: Nina Satragno <nsatragno@chromium.org>
Cr-Commit-Position: refs/heads/master@{#844291}
parent 8457934b
...@@ -3295,6 +3295,9 @@ ...@@ -3295,6 +3295,9 @@
<message name="IDS_SETTINGS_SECURITY_KEYS_PIN_INCORRECT_RETRIES_PL" desc="A message shown to the user when they enter an incorrect PIN for a security key. PINs, in this context, are short, often numeric codes that are often used with, for example, ATM cards. Security keys are external devices used to authenticate people."> <message name="IDS_SETTINGS_SECURITY_KEYS_PIN_INCORRECT_RETRIES_PL" desc="A message shown to the user when they enter an incorrect PIN for a security key. PINs, in this context, are short, often numeric codes that are often used with, for example, ATM cards. Security keys are external devices used to authenticate people.">
Incorrect PIN. You have <ph name="RETRIES">$1<ex>8</ex></ph> attempts remaining. Incorrect PIN. You have <ph name="RETRIES">$1<ex>8</ex></ph> attempts remaining.
</message> </message>
<message name="IDS_SETTINGS_SECURITY_KEYS_SAME_PIN_AS_CURRENT" desc="Error message. Displayed when the user attempts to set a new PIN for their security key, and the new PIN is the same as the currently set PIN. PINs, in this context, are short, often numeric codes that are often used with, for example, ATM cards. Security keys are external devices used to authenticate people.">
Create a new PIN that's different from your current PIN
</message>
<message name="IDS_SETTINGS_SECURITY_KEYS_NEW_PIN" desc="A message shown when a user is trying to set a PIN on a security key. PINs, in this context, are short, often numeric codes that are often used with, for example, ATM cards. Security keys are external devices used to authenticate people."> <message name="IDS_SETTINGS_SECURITY_KEYS_NEW_PIN" desc="A message shown when a user is trying to set a PIN on a security key. PINs, in this context, are short, often numeric codes that are often used with, for example, ATM cards. Security keys are external devices used to authenticate people.">
{MIN_PIN_LENGTH, plural, {MIN_PIN_LENGTH, plural,
=1 {Enter your new PIN. A PIN must be at least one character long and can contain letters, numbers, and other characters.} =1 {Enter your new PIN. A PIN must be at least one character long and can contain letters, numbers, and other characters.}
......
a775ab5fe5a6302c4f83dc1b4863d21ff7b20ff0
\ No newline at end of file
...@@ -13,6 +13,11 @@ ...@@ -13,6 +13,11 @@
flex-direction: row; flex-direction: row;
} }
#newPINRow cr-input {
width: 8em;
--cr-input-error-white-space: nowrap;
}
paper-spinner-lite { paper-spinner-lite {
padding-bottom: 12px; padding-bottom: 12px;
} }
......
...@@ -18,8 +18,7 @@ import '../settings_shared_css.m.js'; ...@@ -18,8 +18,7 @@ import '../settings_shared_css.m.js';
import {I18nBehavior} from 'chrome://resources/js/i18n_behavior.m.js'; import {I18nBehavior} from 'chrome://resources/js/i18n_behavior.m.js';
import {PluralStringProxyImpl} from 'chrome://resources/js/plural_string_proxy.js'; import {PluralStringProxyImpl} from 'chrome://resources/js/plural_string_proxy.js';
import {IronA11yAnnouncer} from 'chrome://resources/polymer/v3_0/iron-a11y-announcer/iron-a11y-announcer.js'; import {html, Polymer} from 'chrome://resources/polymer/v3_0/polymer/polymer_bundled.min.js';
import {afterNextRender, html, Polymer} from 'chrome://resources/polymer/v3_0/polymer/polymer_bundled.min.js';
import {loadTimeData} from '../i18n_setup.js'; import {loadTimeData} from '../i18n_setup.js';
...@@ -200,10 +199,6 @@ Polymer({ ...@@ -200,10 +199,6 @@ Polymer({
this.browserProxy_ = SecurityKeysPINBrowserProxyImpl.getInstance(); this.browserProxy_ = SecurityKeysPINBrowserProxyImpl.getInstance();
this.$.dialog.showModal(); this.$.dialog.showModal();
afterNextRender(this, function() {
IronA11yAnnouncer.requestAvailability();
});
this.browserProxy_.startSetPIN().then( this.browserProxy_.startSetPIN().then(
({done, error, currentMinPinLength, newMinPinLength, retries}) => { ({done, error, currentMinPinLength, newMinPinLength, retries}) => {
if (done) { if (done) {
...@@ -397,7 +392,6 @@ Polymer({ ...@@ -397,7 +392,6 @@ Polymer({
this.isValidPIN_(this.currentPIN_, this.currentMinPinLength_); this.isValidPIN_(this.currentPIN_, this.currentMinPinLength_);
if (this.currentPINError_ !== '') { if (this.currentPINError_ !== '') {
this.focusOn_(this.$.currentPIN); this.focusOn_(this.$.currentPIN);
this.fire('iron-announce', {text: this.currentPINError_});
this.fire('ui-ready'); // for test synchronization. this.fire('ui-ready'); // for test synchronization.
return; return;
} }
...@@ -406,7 +400,6 @@ Polymer({ ...@@ -406,7 +400,6 @@ Polymer({
this.newPINError_ = this.isValidPIN_(this.newPIN_, this.newMinPinLength_); this.newPINError_ = this.isValidPIN_(this.newPIN_, this.newMinPinLength_);
if (this.newPINError_ !== '') { if (this.newPINError_ !== '') {
this.focusOn_(this.$.newPIN); this.focusOn_(this.$.newPIN);
this.fire('iron-announce', {text: this.newPINError_});
this.fire('ui-ready'); // for test synchronization. this.fire('ui-ready'); // for test synchronization.
return; return;
} }
...@@ -414,7 +407,13 @@ Polymer({ ...@@ -414,7 +407,13 @@ Polymer({
if (this.newPIN_ !== this.confirmPIN_) { if (this.newPIN_ !== this.confirmPIN_) {
this.confirmPINError_ = this.i18n('securityKeysPINMismatch'); this.confirmPINError_ = this.i18n('securityKeysPINMismatch');
this.focusOn_(this.$.confirmPIN); this.focusOn_(this.$.confirmPIN);
this.fire('iron-announce', {text: this.confirmPINError_}); this.fire('ui-ready'); // for test synchronization.
return;
}
if (this.newPIN_ === this.currentPIN_) {
this.newPINError_ = this.i18n('securityKeysSamePINAsCurrent');
this.focusOn_(this.$.newPIN);
this.fire('ui-ready'); // for test synchronization. this.fire('ui-ready'); // for test synchronization.
return; return;
} }
...@@ -441,7 +440,6 @@ Polymer({ ...@@ -441,7 +440,6 @@ Polymer({
this.mismatchError_(/** @type {number} */ (this.retries_)); this.mismatchError_(/** @type {number} */ (this.retries_));
this.setPINButtonValid_ = true; this.setPINButtonValid_ = true;
this.focusOn_(this.$.currentPIN); this.focusOn_(this.$.currentPIN);
this.fire('iron-announce', {text: this.currentPINError_});
this.fire('ui-ready'); // for test synchronization. this.fire('ui-ready'); // for test synchronization.
} else { } else {
// Unknown error. // Unknown error.
......
...@@ -2405,6 +2405,8 @@ void AddSecurityKeysStrings(content::WebUIDataSource* html_source) { ...@@ -2405,6 +2405,8 @@ void AddSecurityKeysStrings(content::WebUIDataSource* html_source) {
{"securityKeysTouchToContinue", {"securityKeysTouchToContinue",
IDS_SETTINGS_SECURITY_KEYS_TOUCH_TO_CONTINUE}, IDS_SETTINGS_SECURITY_KEYS_TOUCH_TO_CONTINUE},
{"securityKeysSetPinButton", IDS_SETTINGS_SECURITY_KEYS_SET_PIN_BUTTON}, {"securityKeysSetPinButton", IDS_SETTINGS_SECURITY_KEYS_SET_PIN_BUTTON},
{"securityKeysSamePINAsCurrent",
IDS_SETTINGS_SECURITY_KEYS_SAME_PIN_AS_CURRENT},
}; };
AddLocalizedStringsBulk(html_source, kSecurityKeysStrings); AddLocalizedStringsBulk(html_source, kSecurityKeysStrings);
bool win_native_api_available = false; bool win_native_api_available = false;
......
...@@ -4,6 +4,7 @@ ...@@ -4,6 +4,7 @@
// clang-format off // clang-format off
import {webUIListenerCallback} from 'chrome://resources/js/cr.m.js'; import {webUIListenerCallback} from 'chrome://resources/js/cr.m.js';
import {loadTimeData} from 'chrome://resources/js/load_time_data.m.js';
import {PromiseResolver} from 'chrome://resources/js/promise_resolver.m.js'; import {PromiseResolver} from 'chrome://resources/js/promise_resolver.m.js';
import {flush} from 'chrome://resources/polymer/v3_0/polymer/polymer_bundled.min.js'; import {flush} from 'chrome://resources/polymer/v3_0/polymer/polymer_bundled.min.js';
import {BioEnrollDialogPage, CredentialManagementDialogPage, Ctap2Status, ResetDialogPage, SampleStatus, SecurityKeysBioEnrollProxyImpl, SecurityKeysCredentialBrowserProxyImpl, SecurityKeysPINBrowserProxyImpl,SecurityKeysResetBrowserProxyImpl, SetPINDialogPage} from 'chrome://settings/lazy_load.js'; import {BioEnrollDialogPage, CredentialManagementDialogPage, Ctap2Status, ResetDialogPage, SampleStatus, SecurityKeysBioEnrollProxyImpl, SecurityKeysCredentialBrowserProxyImpl, SecurityKeysPINBrowserProxyImpl,SecurityKeysResetBrowserProxyImpl, SetPINDialogPage} from 'chrome://settings/lazy_load.js';
...@@ -550,6 +551,14 @@ suite('SecurityKeysSetPINDialog', function() { ...@@ -550,6 +551,14 @@ suite('SecurityKeysSetPINDialog', function() {
assertFalse(dialog.$.newPIN.invalid); assertFalse(dialog.$.newPIN.invalid);
assertFalse(dialog.$.confirmPIN.invalid); assertFalse(dialog.$.confirmPIN.invalid);
setChangePINEntries(validNewPIN, validNewPIN, validNewPIN);
assertFalse(dialog.$.currentPIN.invalid);
assertTrue(dialog.$.newPIN.invalid);
assertEquals(
dialog.$.newPIN.errorMessage,
loadTimeData.getString('securityKeysSamePINAsCurrent'));
assertFalse(dialog.$.confirmPIN.invalid);
let setPINResolver = new PromiseResolver(); let setPINResolver = new PromiseResolver();
browserProxy.setResponseFor('setPIN', setPINResolver.promise); browserProxy.setResponseFor('setPIN', setPINResolver.promise);
setPINEntry(dialog.$.currentPIN, validCurrentPIN); setPINEntry(dialog.$.currentPIN, validCurrentPIN);
......
...@@ -81,7 +81,12 @@ ...@@ -81,7 +81,12 @@
form with cr-inputs that can be invalid, this space is also desired form with cr-inputs that can be invalid, this space is also desired
in order to have consistent spacing. in order to have consistent spacing.
If spacing is not needed, apply "--cr-input-error-display: none". */ If spacing is not needed, apply "--cr-input-error-display: none".
When grouping cr-inputs horizontally, it might be helpful to set
--cr-input-error-white-space to "nowrap" and set a fixed width for
each cr-input so that a long error label does not shift the inputs
forward. */
color: var(--cr-input-error-color); color: var(--cr-input-error-color);
display: var(--cr-input-error-display, block); display: var(--cr-input-error-display, block);
font-size: var(--cr-form-field-label-font-size); font-size: var(--cr-form-field-label-font-size);
...@@ -89,6 +94,7 @@ ...@@ -89,6 +94,7 @@
line-height: var(--cr-form-field-label-line-height); line-height: var(--cr-form-field-label-line-height);
margin: 8px 0; margin: 8px 0;
visibility: hidden; visibility: hidden;
white-space: var(--cr-input-error-white-space);
} }
:host([invalid]) #error { :host([invalid]) #error {
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment