Skip to content

Commit

Permalink
settings_account: Don't redirect to login page during password change.
Browse files Browse the repository at this point in the history
This handles a rare race condition that occurs when the session hash
is not updated by the backend during the password change process.
This mostly occurs in puppeteer tests, but could occur to a user.
  • Loading branch information
priyank-p authored and timabbott committed Aug 30, 2020
1 parent 5548ab8 commit 9884226
Show file tree
Hide file tree
Showing 3 changed files with 32 additions and 5 deletions.
8 changes: 7 additions & 1 deletion frontend_tests/node_tests/settings_org.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
"use strict";

const rewiremock = require("rewiremock/node");

set_global("$", global.make_zjquery());

const noop = () => {};
Expand Down Expand Up @@ -77,7 +79,11 @@ set_global("list_render", _list_render);
const settings_config = zrequire("settings_config");
const settings_bots = zrequire("settings_bots");
zrequire("stream_data");
zrequire("settings_account");
rewiremock.proxy(() => zrequire("settings_account"), {
// Setup is only imported to set the
// setup.password_change_in_progress flag.
"../../static/js/setup": {},
});
zrequire("settings_org");
zrequire("settings_ui");
zrequire("dropdown_list_widget");
Expand Down
7 changes: 7 additions & 0 deletions static/js/settings_account.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ const render_settings_api_key_modal = require("../templates/settings/api_key_mod
const render_settings_custom_user_profile_field = require("../templates/settings/custom_user_profile_field.hbs");
const render_settings_dev_env_email_access = require("../templates/settings/dev_env_email_access.hbs");

const setup = require("./setup");

exports.update_email = function (new_email) {
const email_input = $("#email_value");

Expand Down Expand Up @@ -445,10 +447,15 @@ exports.set_up = function () {
}
}

setup.password_change_in_progress = true;
const opts = {
success_continuation() {
setup.password_change_in_progress = false;
overlays.close_modal("#change_password_modal");
},
error_continuation() {
setup.password_change_in_progress = false;
},
error_msg_element: change_password_error,
};
settings_ui.do_settings_change(
Expand Down
22 changes: 18 additions & 4 deletions static/js/setup.js
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
"use strict";

const util = require("./util");
// Miscellaneous early setup.

// Miscellaneous early setup.
exports.password_change_in_progress = false;
$(() => {
if (util.is_mobile()) {
// Disable the tutorial; it's ugly on mobile.
Expand Down Expand Up @@ -35,10 +36,23 @@ $(() => {

// For some reason, jQuery wants this to be attached to an element.
$(document).ajaxError((event, xhr) => {
if (exports.password_change_in_progress) {
// The backend for handling password change API requests
// will replace the user's session; this results in a
// brief race where any API request will fail with a 401
// error after the old session is deactivated but before
// the new one has been propagated to the browser. So we
// skip our normal HTTP 401 error handling if we're in the
// process of executing a password change.
return;
}

if (xhr.status === 401) {
// We got logged out somehow, perhaps from another window or a session timeout.
// We could display an error message, but jumping right to the login page seems
// smoother and conveys the same information.
// We got logged out somehow, perhaps from another window
// changing the user's password, or a session timeout. We
// could display an error message, but jumping right to
// the login page conveys the same information with a
// smoother re-login experience.
window.location.replace(page_params.login_page);
}
});
Expand Down

0 comments on commit 9884226

Please sign in to comment.