From bf056c899032d474d1a2f1546d15ff00ea264afd Mon Sep 17 00:00:00 2001 From: Anders Kaseorg Date: Fri, 23 Apr 2021 12:27:13 -0700 Subject: [PATCH] js: Extract password_quality module; remove zxcvbn from globals. Signed-off-by: Anders Kaseorg --- .eslintrc.json | 3 +- frontend_tests/node_tests/password.js | 34 ++++------------ static/js/common.js | 56 --------------------------- static/js/password_quality.js | 49 +++++++++++++++++++++++ static/js/portico/signup.js | 9 +++-- static/js/settings_account.js | 16 ++++---- tools/webpack.assets.json | 7 +--- webpack.config.ts | 9 ----- 8 files changed, 72 insertions(+), 111 deletions(-) create mode 100644 static/js/password_quality.js diff --git a/.eslintrc.json b/.eslintrc.json index c9a0e4c132..0113608209 100644 --- a/.eslintrc.json +++ b/.eslintrc.json @@ -115,8 +115,7 @@ { "files": ["static/js/**"], "globals": { - "StripeCheckout": false, - "zxcvbn": false + "StripeCheckout": false } }, { diff --git a/frontend_tests/node_tests/password.js b/frontend_tests/node_tests/password.js index 582234f2d0..f9abeb1c76 100644 --- a/frontend_tests/node_tests/password.js +++ b/frontend_tests/node_tests/password.js @@ -2,12 +2,10 @@ const {strict: assert} = require("assert"); -const {set_global, with_field, zrequire} = require("../zjsunit/namespace"); +const {zrequire} = require("../zjsunit/namespace"); const {run_test} = require("../zjsunit/test"); -set_global("zxcvbn", require("zxcvbn")); - -const common = zrequire("common"); +const {password_quality, password_warning} = zrequire("password_quality"); function password_field(min_length, min_guesses) { const self = {}; @@ -51,41 +49,25 @@ run_test("basics w/progress bar", () => { })(); password = "z!X4@S_&"; - accepted = common.password_quality(password, bar, password_field(10, 80000)); + accepted = password_quality(password, bar, password_field(10, 80000)); assert(!accepted); assert.equal(bar.w, "39.7%"); assert.equal(bar.added_class, "bar-danger"); - warning = common.password_warning(password, password_field(10)); + warning = password_warning(password, password_field(10)); assert.equal(warning, "translated: Password should be at least 10 characters long"); password = "foo"; - accepted = common.password_quality(password, bar, password_field(2, 200)); + accepted = password_quality(password, bar, password_field(2, 200)); assert(accepted); assert.equal(bar.w, "10.390277164940581%"); assert.equal(bar.added_class, "bar-success"); - warning = common.password_warning(password, password_field(2)); + warning = password_warning(password, password_field(2)); assert.equal(warning, "translated: Password is too weak"); password = "aaaaaaaa"; - accepted = common.password_quality(password, bar, password_field(6, 1e100)); + accepted = password_quality(password, bar, password_field(6, 1e100)); assert(!accepted); assert.equal(bar.added_class, "bar-danger"); - warning = common.password_warning(password, password_field(6)); + warning = password_warning(password, password_field(6)); assert.equal(warning, 'Repeats like "aaa" are easy to guess'); }); - -run_test("zxcvbn undefined", () => { - // According to common.js, we load zxcvbn.js asynchronously, so the - // variable might not be set. This just gets line coverage on the - // defensive code. - - const password = "aaaaaaaa"; - const progress_bar = undefined; - - with_field(global, "zxcvbn", undefined, () => { - const accepted = common.password_quality(password, progress_bar, password_field(6, 1e100)); - assert(accepted === undefined); - const warning = common.password_warning(password, password_field(6)); - assert(warning === undefined); - }); -}); diff --git a/static/js/common.js b/static/js/common.js index 94107afc33..5ccf3c0720 100644 --- a/static/js/common.js +++ b/static/js/common.js @@ -12,62 +12,6 @@ export function autofocus(selector) { }); } -// Return a boolean indicating whether the password is acceptable. -// Also updates a Bootstrap progress bar control (a jQuery object) -// if provided. -// -// Assumes that zxcvbn.js has been loaded. -// -// This is in common.js because we want to use it from the signup page -// and also from the in-app password change interface. -export function password_quality(password, bar, password_field) { - // We load zxcvbn.js asynchronously, so the variable might not be set. - if (typeof zxcvbn === "undefined") { - return undefined; - } - - const min_length = password_field.data("minLength"); - const min_guesses = password_field.data("minGuesses"); - - const result = zxcvbn(password); - const acceptable = password.length >= min_length && result.guesses >= min_guesses; - - if (bar !== undefined) { - const t = result.crack_times_seconds.offline_slow_hashing_1e4_per_second; - let bar_progress = Math.min(1, Math.log(1 + t) / 22); - - // Even if zxcvbn loves your short password, the bar should be - // filled at most 1/3 of the way, because we won't accept it. - if (!acceptable) { - bar_progress = Math.min(bar_progress, 0.33); - } - - // The bar bottoms out at 10% so there's always something - // for the user to see. - bar.width(90 * bar_progress + 10 + "%") - .removeClass("bar-success bar-danger") - .addClass(acceptable ? "bar-success" : "bar-danger"); - } - - return acceptable; -} - -export function password_warning(password, password_field) { - if (typeof zxcvbn === "undefined") { - return undefined; - } - - const min_length = password_field.data("minLength"); - - if (password.length < min_length) { - return $t( - {defaultMessage: "Password should be at least {length} characters long"}, - {length: min_length}, - ); - } - return zxcvbn(password).feedback.warning || $t({defaultMessage: "Password is too weak"}); -} - export function phrase_match(query, phrase) { // match "tes" to "test" and "stream test" but not "hostess" let i; diff --git a/static/js/password_quality.js b/static/js/password_quality.js new file mode 100644 index 0000000000..b837d86a38 --- /dev/null +++ b/static/js/password_quality.js @@ -0,0 +1,49 @@ +import zxcvbn from "zxcvbn"; + +import {$t} from "./i18n"; + +// Note: this module is loaded asynchronously from the app with +// import() to keep zxcvbn out of the initial page load. Do not +// import it synchronously from the app. + +// Return a boolean indicating whether the password is acceptable. +// Also updates a Bootstrap progress bar control (a jQuery object) +// if provided. +export function password_quality(password, bar, password_field) { + const min_length = password_field.data("minLength"); + const min_guesses = password_field.data("minGuesses"); + + const result = zxcvbn(password); + const acceptable = password.length >= min_length && result.guesses >= min_guesses; + + if (bar !== undefined) { + const t = result.crack_times_seconds.offline_slow_hashing_1e4_per_second; + let bar_progress = Math.min(1, Math.log(1 + t) / 22); + + // Even if zxcvbn loves your short password, the bar should be + // filled at most 1/3 of the way, because we won't accept it. + if (!acceptable) { + bar_progress = Math.min(bar_progress, 0.33); + } + + // The bar bottoms out at 10% so there's always something + // for the user to see. + bar.width(90 * bar_progress + 10 + "%") + .removeClass("bar-success bar-danger") + .addClass(acceptable ? "bar-success" : "bar-danger"); + } + + return acceptable; +} + +export function password_warning(password, password_field) { + const min_length = password_field.data("minLength"); + + if (password.length < min_length) { + return $t( + {defaultMessage: "Password should be at least {length} characters long"}, + {length: min_length}, + ); + } + return zxcvbn(password).feedback.warning || $t({defaultMessage: "Password is too weak"}); +} diff --git a/static/js/portico/signup.js b/static/js/portico/signup.js index e1f68c0acb..15ccfd0a7d 100644 --- a/static/js/portico/signup.js +++ b/static/js/portico/signup.js @@ -1,6 +1,7 @@ import $ from "jquery"; import * as common from "../common"; +import {password_quality, password_warning} from "../password_quality"; $(() => { // NB: this file is included on multiple pages. In each context, @@ -10,17 +11,17 @@ $(() => { if (password_field.length > 0) { $.validator.addMethod( "password_strength", - (value) => common.password_quality(value, undefined, password_field), - () => common.password_warning(password_field.val(), password_field), + (value) => password_quality(value, undefined, password_field), + () => password_warning(password_field.val(), password_field), ); // Reset the state of the password strength bar if the page // was just reloaded due to a validation failure on the backend. - common.password_quality(password_field.val(), $("#pw_strength .bar"), password_field); + password_quality(password_field.val(), $("#pw_strength .bar"), password_field); password_field.on("input", function () { // Update the password strength bar even if we aren't validating // the field yet. - common.password_quality($(this).val(), $("#pw_strength .bar"), $(this)); + password_quality($(this).val(), $("#pw_strength .bar"), $(this)); }); } diff --git a/static/js/settings_account.js b/static/js/settings_account.js index b753c16031..31edc6333b 100644 --- a/static/js/settings_account.js +++ b/static/js/settings_account.js @@ -23,6 +23,8 @@ import * as setup from "./setup"; import * as ui_report from "./ui_report"; import * as user_pill from "./user_pill"; +let password_quality; // Loaded asynchronously + export function update_email(new_email) { const email_input = $("#email_value"); @@ -393,7 +395,7 @@ export function set_up() { "#new_password + .password_visibility_toggle", ); $("#old_password, #new_password").val(""); - common.password_quality("", $("#pw_strength .bar"), $("#new_password")); + password_quality?.("", $("#pw_strength .bar"), $("#new_password")); } clear_password_change(); @@ -415,8 +417,7 @@ export function set_up() { if (page_params.realm_password_auth_enabled !== false) { // zxcvbn.js is pretty big, and is only needed on password // change, so load it asynchronously. - const {default: zxcvbn} = await import("zxcvbn"); - window.zxcvbn = zxcvbn; + password_quality = (await import("./password_quality")).password_quality; $("#pw_strength .bar").removeClass("fade"); } }); @@ -443,15 +444,14 @@ export function set_up() { const new_pw_field = $("#new_password"); const new_pw = data.new_password; if (new_pw !== "") { - const password_ok = common.password_quality(new_pw, undefined, new_pw_field); - if (password_ok === undefined) { - // zxcvbn.js didn't load, for whatever reason. + if (password_quality === undefined) { + // password_quality didn't load, for whatever reason. settings_change_error( "An internal error occurred; try reloading the page. " + "Sorry for the trouble!", ); return; - } else if (!password_ok) { + } else if (!password_quality(new_pw, undefined, new_pw_field)) { settings_change_error($t_html({defaultMessage: "New password is too weak"})); return; } @@ -481,7 +481,7 @@ export function set_up() { $("#new_password").on("input", () => { const field = $("#new_password"); - common.password_quality(field.val(), $("#pw_strength .bar"), field); + password_quality?.(field.val(), $("#pw_strength .bar"), field); }); $("#change_full_name_button").on("click", (e) => { diff --git a/tools/webpack.assets.json b/tools/webpack.assets.json index 3b2b9e6439..7e66d200a2 100644 --- a/tools/webpack.assets.json +++ b/tools/webpack.assets.json @@ -65,12 +65,7 @@ "./static/styles/portico/integrations.css" ], "signup": ["./static/js/bundles/portico", "jquery-validation", "./static/js/portico/signup"], - "register": [ - "./static/js/bundles/portico", - "jquery-validation", - "./static/js/portico/signup", - "zxcvbn/dist/zxcvbn" - ], + "register": ["./static/js/bundles/portico", "jquery-validation", "./static/js/portico/signup"], "confirm-preregistrationuser": [ "./static/js/bundles/common", "./static/js/portico/confirm-preregistrationuser" diff --git a/webpack.config.ts b/webpack.config.ts index 5bb5c0ad37..f16317dc24 100644 --- a/webpack.config.ts +++ b/webpack.config.ts @@ -83,15 +83,6 @@ export default (_env: unknown, argv: {mode?: string}): webpack.Configuration[] = ], use: [cacheLoader, "babel-loader"], }, - // Uses script-loader on minified files so we don't change global variables in them. - // Also has the effect of making processing these files fast - // Currently the source maps don't work with these so use unminified files - // if debugging is required. - { - // We dont want to match admin.js - test: /(\.min|min\.|zxcvbn)\.js/, - use: [cacheLoader, "script-loader"], - }, // regular css files { test: /\.css$/,