Commit ac6fb40d authored by Jordy Greenblatt's avatar Jordy Greenblatt Committed by Commit Bot

[CrOS MultiDevice] Stop Setup Flow from accumulating password attempts

The current HEAD behavior of the Setup Flow password page is to allow
the user to repeatedly request forward navigation while the IPC for a
password check is in progress. Then these requests for a queue so that
the next time the user changes the contexts of the password input
field, the next request in the queue is sent causing an accidental
password check/setup attempt.

This CL adds a field to the password page to allow it to determine
whether it is in the middle of waiting for a password check to complete
and if so, prevent any forward navigation attempt (including from the
'enter' key) and disable the button so the user knows it is out of
commission while waiting for the IPC response.

The CL also removes some now unused code surrounding expired auth
tokens which made it harder to follow the logic flow for the IPC and
callback:

In the original design, the user would enter their password before they
got to the start setup page and would therefore have to hold onto an
auth token throughout the flow. In particular, it was necessary to set
a timeout leading to a callback to erase the token once it was expired.
In the current design, however, the auth token is used to complete the
setup flow as soon as it is successfully obtained so there is no case
in which a user would have time to let their token expire.

Bug: 890958
Change-Id: Ia82af98ebaeeed0429c2da114d6e667130100c99
Reviewed-on: https://chromium-review.googlesource.com/c/1260215Reviewed-by: default avatarKyle Horimoto <khorimoto@chromium.org>
Commit-Queue: Jordy Greenblatt <jordynass@chromium.org>
Cr-Commit-Position: refs/heads/master@{#596491}
parent ee5a0f98
...@@ -18,7 +18,8 @@ Polymer({ ...@@ -18,7 +18,8 @@ Polymer({
*/ */
forwardButtonDisabled: { forwardButtonDisabled: {
type: Boolean, type: Boolean,
computed: 'shouldForwardButtonBeDisabled_(inputValue_, passwordInvalid_)', computed: 'shouldForwardButtonBeDisabled_(' +
'inputValue_, passwordInvalid_, waitingForPasswordCheck_)',
notify: true, notify: true,
}, },
...@@ -44,7 +45,11 @@ Polymer({ ...@@ -44,7 +45,11 @@ Polymer({
* Authentication token; retrieved using the quickUnlockPrivate API. * Authentication token; retrieved using the quickUnlockPrivate API.
* @type {string} * @type {string}
*/ */
authToken: {type: String, value: '', notify: true}, authToken: {
type: String,
value: '',
notify: true,
},
/** @private {string} */ /** @private {string} */
profilePhotoUrl_: { profilePhotoUrl_: {
...@@ -59,7 +64,10 @@ Polymer({ ...@@ -59,7 +64,10 @@ Polymer({
}, },
/** @private {!QuickUnlockPrivate} */ /** @private {!QuickUnlockPrivate} */
quickUnlockPrivate_: {type: Object, value: chrome.quickUnlockPrivate}, quickUnlockPrivate_: {
type: Object,
value: chrome.quickUnlockPrivate,
},
/** @private {string} */ /** @private {string} */
inputValue_: { inputValue_: {
...@@ -73,17 +81,17 @@ Polymer({ ...@@ -73,17 +81,17 @@ Polymer({
type: Boolean, type: Boolean,
value: false, value: false,
}, },
/** @private {boolean} */
waitingForPasswordCheck_: {
type: Boolean,
value: false,
},
}, },
/** @private {?multidevice_setup.BrowserProxy} */ /** @private {?multidevice_setup.BrowserProxy} */
browserProxy_: null, browserProxy_: null,
/**
* Function which clears this.authToken after it has expired.
* @private {number|undefined}
*/
clearAccountPasswordTimeout_: undefined,
clearPasswordTextInput: function() { clearPasswordTextInput: function() {
this.$.passwordInput.value = ''; this.$.passwordInput.value = '';
}, },
...@@ -103,10 +111,14 @@ Polymer({ ...@@ -103,10 +111,14 @@ Polymer({
/** Overridden from UiPageContainerBehavior. */ /** Overridden from UiPageContainerBehavior. */
getCanNavigateToNextPage: function() { getCanNavigateToNextPage: function() {
clearTimeout(this.clearAccountPasswordTimeout_);
return new Promise((resolve) => { return new Promise((resolve) => {
if (this.waitingForPasswordCheck_) {
resolve(false /* canNavigate */);
return;
}
this.waitingForPasswordCheck_ = true;
this.quickUnlockPrivate_.getAuthToken(this.inputValue_, (tokenInfo) => { this.quickUnlockPrivate_.getAuthToken(this.inputValue_, (tokenInfo) => {
this.waitingForPasswordCheck_ = false;
if (chrome.runtime.lastError) { if (chrome.runtime.lastError) {
this.passwordInvalid_ = true; this.passwordInvalid_ = true;
// Select the password text if the user entered an incorrect password. // Select the password text if the user entered an incorrect password.
...@@ -114,20 +126,8 @@ Polymer({ ...@@ -114,20 +126,8 @@ Polymer({
resolve(false /* canNavigate */); resolve(false /* canNavigate */);
return; return;
} }
this.authToken = tokenInfo.token; this.authToken = tokenInfo.token;
this.passwordInvalid_ = false; this.passwordInvalid_ = false;
// Subtract time from the exiration time to account for IPC delays.
// Treat values less than the minimum as 0 for testing.
const IPC_SECONDS = 2;
const lifetimeMs = tokenInfo.lifetimeSeconds > IPC_SECONDS ?
(tokenInfo.lifetimeSeconds - IPC_SECONDS) * 1000 :
0;
this.clearAccountPasswordTimeout_ = setTimeout(() => {
this.authToken = '';
}, lifetimeMs);
resolve(true /* canNavigate */); resolve(true /* canNavigate */);
}); });
}); });
...@@ -155,6 +155,7 @@ Polymer({ ...@@ -155,6 +155,7 @@ Polymer({
* @private * @private
*/ */
shouldForwardButtonBeDisabled_: function() { shouldForwardButtonBeDisabled_: function() {
return this.passwordInvalid_ || !this.inputValue_; return this.passwordInvalid_ || !this.inputValue_ ||
this.waitingForPasswordCheck_;
}, },
}); });
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