Bug 1313849 – Stop using nsIDialogParamBlock in setp12password.xul. r?keeler
nsIDialogParamBlock isn't a great API, and is best avoided.
This patch also splits password.js into two files that implement the
functionality of changepassword.xul and setp12password.xul
respectively, and adds a test.
MozReview-Commit-ID: A1GlnIFl8h
--- a/security/manager/pki/nsNSSDialogs.cpp
+++ b/security/manager/pki/nsNSSDialogs.cpp
@@ -265,48 +265,46 @@ nsNSSDialogs::ChooseCertificate(nsIInter
if (NS_FAILED(rv)) {
return rv;
}
}
return NS_OK;
}
-NS_IMETHODIMP
-nsNSSDialogs::SetPKCS12FilePassword(nsIInterfaceRequestor *ctx,
- nsAString &_password,
- bool *_retval)
+NS_IMETHODIMP
+nsNSSDialogs::SetPKCS12FilePassword(nsIInterfaceRequestor* ctx,
+ /*out*/ nsAString& password,
+ /*out*/ bool* confirmedPassword)
{
- nsresult rv;
- *_retval = true;
+ // |ctx| is allowed to be null.
+ NS_ENSURE_ARG(confirmedPassword);
+
// Get the parent window for the dialog
nsCOMPtr<mozIDOMWindowProxy> parent = do_GetInterface(ctx);
- nsCOMPtr<nsIDialogParamBlock> block =
- do_CreateInstance(NS_DIALOGPARAMBLOCK_CONTRACTID);
- if (!block) return NS_ERROR_FAILURE;
- // open up the window
- rv = nsNSSDialogHelper::openDialog(parent,
+ nsCOMPtr<nsIWritablePropertyBag2> retVals = new nsHashPropertyBag();
+ nsresult rv =
+ nsNSSDialogHelper::openDialog(parent,
"chrome://pippki/content/setp12password.xul",
- block);
- if (NS_FAILED(rv)) return rv;
- // see if user canceled
- int32_t status;
- rv = block->GetInt(1, &status);
- if (NS_FAILED(rv)) return rv;
- *_retval = (status == 0) ? false : true;
- if (*_retval) {
- // retrieve the password
- char16_t *pw;
- rv = block->GetString(2, &pw);
- if (NS_SUCCEEDED(rv)) {
- _password = pw;
- free(pw);
- }
+ retVals);
+ if (NS_FAILED(rv)) {
+ return rv;
}
- return rv;
+
+ rv = retVals->GetPropertyAsBool(NS_LITERAL_STRING("confirmedPassword"),
+ confirmedPassword);
+ if (NS_FAILED(rv)) {
+ return rv;
+ }
+
+ if (!*confirmedPassword) {
+ return NS_OK;
+ }
+
+ return retVals->GetPropertyAsAString(NS_LITERAL_STRING("password"), password);
}
NS_IMETHODIMP
nsNSSDialogs::GetPKCS12FilePassword(nsIInterfaceRequestor* ctx,
nsAString& _password,
bool* _retval)
{
*_retval = false;
rename from security/manager/pki/resources/content/password.js
rename to security/manager/pki/resources/content/changepassword.js
--- a/security/manager/pki/resources/content/password.js
+++ b/security/manager/pki/resources/content/changepassword.js
@@ -119,25 +119,16 @@ function process()
if (params) {
// Return value 0 means "canceled"
params.SetInt(1, 0);
}
checkPasswords();
}
-function onP12Load(disableOkButton)
-{
- document.documentElement.getButton("accept").disabled = disableOkButton;
- pw1 = document.getElementById("pw1");
- params = window.arguments[0].QueryInterface(nsIDialogParamBlock);
- // Select first password field
- document.getElementById('pw1').focus();
-}
-
function setPassword()
{
var pk11db = Components.classes[nsPK11TokenDB].getService(nsIPK11TokenDB);
var token = pk11db.findTokenByName(tokenName);
var oldpwbox = document.getElementById("oldpw");
var initpw = oldpwbox.getAttribute("inited");
var bundle = document.getElementById("pippki_bundle");
@@ -201,26 +192,16 @@ function setPassword()
// Return value 1 means "successfully executed ok"
params.SetInt(1, 1);
}
// Terminate dialog
return success;
}
-function setP12Password()
-{
- // grab what was entered
- params.SetString(2, pw1.value);
- // Return value
- params.SetInt(1, 1);
- // Terminate dialog
- return true;
-}
-
function setPasswordStrength()
{
// We weigh the quality of the password by checking the number of:
// - Characters
// - Numbers
// - Non-alphanumeric chars
// - Upper and lower case characters
--- a/security/manager/pki/resources/content/changepassword.xul
+++ b/security/manager/pki/resources/content/changepassword.xul
@@ -10,17 +10,18 @@
<dialog id="set_password" title="&setPassword.title;"
xmlns="http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul"
buttons="accept,cancel"
ondialogaccept="return setPassword();"
onload="onLoad();">
<stringbundle id="pippki_bundle" src="chrome://pippki/locale/pippki.properties"/>
-<script type="application/javascript" src="chrome://pippki/content/password.js"/>
+<script type="application/javascript"
+ src="chrome://pippki/content/changepassword.js"/>
<hbox align="center">
<label value="&setPassword.tokenName.label;: "/>
<label id="tokenName" />
<menulist id="tokenMenu" oncommand="onMenuChange()">
<menupopup/>
</menulist>
</hbox>
new file mode 100644
--- /dev/null
+++ b/security/manager/pki/resources/content/setp12password.js
@@ -0,0 +1,128 @@
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+"use strict";
+
+/**
+ * @file Implements the functionality of setp12password.xul: a dialog that lets
+ * the user confirm the password to set on a PKCS #12 file.
+ * @argument {nsISupports} window.arguments[0]
+ * Object to set the return values of calling the dialog on, queryable
+ * to the underlying type of SetP12PasswordReturnValues.
+ */
+
+/**
+ * @typedef SetP12PasswordReturnValues
+ * @type nsIWritablePropertyBag2
+ * @property {Boolean} confirmedPassword
+ * Set to true if the user entered two matching passwords and
+ * confirmed the dialog.
+ * @property {String} password
+ * The password the user entered. Undefined value if
+ * |confirmedPassword| is not true.
+ */
+
+const { classes: Cc, interfaces: Ci, utils: Cu, results: Cr } = Components;
+
+/**
+ * onload() handler.
+ */
+function onLoad() {
+ // Ensure the first password textbox has focus.
+ document.getElementById("pw1").focus();
+}
+
+/**
+ * ondialogaccept() handler.
+ *
+ * @returns {Boolean} true to make the dialog close, false otherwise.
+ */
+function onDialogAccept() {
+ let password = document.getElementById("pw1").value;
+
+ let retVals = window.arguments[0].QueryInterface(Ci.nsIWritablePropertyBag2);
+ retVals.setPropertyAsBool("confirmedPassword", true);
+ retVals.setPropertyAsAString("password", password);
+ return true;
+}
+
+/**
+ * ondialogcancel() handler.
+ *
+ * @returns {Boolean} true to make the dialog close, false otherwise.
+ */
+function onDialogCancel() {
+ let retVals = window.arguments[0].QueryInterface(Ci.nsIWritablePropertyBag2);
+ retVals.setPropertyAsBool("confirmedPassword", false);
+ return true;
+}
+
+/**
+ * Calculates the strength of the given password, suitable for use in updating
+ * a progress bar that represents said strength.
+ *
+ * The strength of the password is calculated by checking the number of:
+ * - Characters
+ * - Numbers
+ * - Non-alphanumeric chars
+ * - Upper case characters
+ *
+ * @param {String} password
+ * The password to calculate the strength of.
+ * @returns {Number}
+ * The strength of the password in the range [0, 100].
+ */
+function getPasswordStrength(password) {
+ let lengthStrength = password.length;
+ if (lengthStrength > 5) {
+ lengthStrength = 5;
+ }
+
+ let nonNumericChars = password.replace(/[0-9]/g, "");
+ let numericStrength = password.length - nonNumericChars.length;
+ if (numericStrength > 3) {
+ numericStrength = 3;
+ }
+
+ let nonSymbolChars = password.replace(/\W/g, "");
+ let symbolStrength = password.length - nonSymbolChars.length;
+ if (symbolStrength > 3) {
+ symbolStrength = 3;
+ }
+
+ let nonUpperAlphaChars = password.replace(/[A-Z]/g, "");
+ let upperAlphaStrength = password.length - nonUpperAlphaChars.length;
+ if (upperAlphaStrength > 3) {
+ upperAlphaStrength = 3;
+ }
+
+ let strength = (lengthStrength * 10) - 20 + (numericStrength * 10) +
+ (symbolStrength * 15) + (upperAlphaStrength * 10);
+ if (strength < 0) {
+ strength = 0;
+ }
+ if (strength > 100) {
+ strength = 100;
+ }
+
+ return strength;
+}
+
+/**
+ * oninput() handler for both password textboxes.
+ *
+ * @param {Boolean} recalculatePasswordStrength
+ * Whether to recalculate the strength of the first password.
+ */
+function onPasswordInput(recalculatePasswordStrength) {
+ let pw1 = document.getElementById("pw1").value;
+
+ if (recalculatePasswordStrength) {
+ document.getElementById("pwmeter").value = getPasswordStrength(pw1);
+ }
+
+ // Disable the accept button if the two passwords don't match, and enable it
+ // if the passwords do match.
+ let pw2 = document.getElementById("pw2").value;
+ document.documentElement.getButton("accept").disabled = (pw1 != pw2);
+}
--- a/security/manager/pki/resources/content/setp12password.xul
+++ b/security/manager/pki/resources/content/setp12password.xul
@@ -2,50 +2,50 @@
<!-- This Source Code Form is subject to the terms of the Mozilla Public
- License, v. 2.0. If a copy of the MPL was not distributed with this
- file, You can obtain one at http://mozilla.org/MPL/2.0/. -->
<?xml-stylesheet href="chrome://global/skin/" type="text/css"?>
<!DOCTYPE dialog SYSTEM "chrome://pippki/locale/pippki.dtd">
-<dialog id="setp12password" title="&pkcs12.setpassword.title;"
- xmlns="http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul"
- style="width: 48em;"
- buttons="accept,cancel"
- ondialogaccept="return setP12Password();"
- onload="onP12Load(true);">
+<dialog id="setp12password"
+ title="&pkcs12.setpassword.title;"
+ xmlns="http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul"
+ style="width: 48em;"
+ buttons="accept,cancel"
+ ondialogaccept="return onDialogAccept();"
+ ondialogcancel="return onDialogCancel();"
+ onload="onLoad();">
- <script type="application/javascript" src="chrome://pippki/content/password.js"/>
+ <script type="application/javascript"
+ src="chrome://pippki/content/setp12password.js"/>
<description>&pkcs12.setpassword.message;</description>
<separator />
<grid>
<columns> <column/> <column/> </columns>
<rows>
<row>
- <label value="&pkcs12.setpassword.label1;"/>
- <textbox id="pw1" type="password"
- oninput="setPasswordStrength(); checkPasswords();"/>
+ <label value="&pkcs12.setpassword.label1;"/>
+ <textbox id="pw1" type="password" oninput="onPasswordInput(true);"/>
</row>
<row>
- <label value="&pkcs12.setpassword.label2;"/>
- <textbox id="pw2" type="password"
- oninput="checkPasswords();"/>
+ <label value="&pkcs12.setpassword.label2;"/>
+ <textbox id="pw2" type="password" oninput="onPasswordInput(false);"/>
</row>
</rows>
</grid>
<separator/>
<description>&pkcs12.setpassword.reminder;</description>
<separator/>
<label value="&setPassword.meter.label;"/>
<grid style="margin: 4px;">
<rows> <row/> </rows>
<columns>
<column style="margin: 5px;">
- <progressmeter flex="1" id="pwmeter" mode="determined" value="0%"
+ <progressmeter flex="1" id="pwmeter" mode="determined" value="0"
orient="horizontal"
- style="width: 20em; foreground-color: red"/>
+ style="width: 20em; foreground-color: red"/>
</column>
</columns>
</grid>
-
</dialog>
--- a/security/manager/pki/resources/jar.mn
+++ b/security/manager/pki/resources/jar.mn
@@ -9,16 +9,17 @@ pippki.jar:
content/pippki/OrphanOverlay.xul (content/OrphanOverlay.xul)
content/pippki/OthersOverlay.xul (content/OthersOverlay.xul)
content/pippki/WebSitesOverlay.xul (content/WebSitesOverlay.xul)
content/pippki/certDump.xul (content/certDump.xul)
content/pippki/certManager.js (content/certManager.js)
content/pippki/certManager.xul (content/certManager.xul)
content/pippki/certViewer.js (content/certViewer.js)
content/pippki/certViewer.xul (content/certViewer.xul)
+ content/pippki/changepassword.js (content/changepassword.js)
content/pippki/changepassword.xul (content/changepassword.xul)
content/pippki/choosetoken.js (content/choosetoken.js)
content/pippki/choosetoken.xul (content/choosetoken.xul)
content/pippki/clientauthask.js (content/clientauthask.js)
content/pippki/clientauthask.xul (content/clientauthask.xul)
content/pippki/createCertInfo.js (content/createCertInfo.js)
content/pippki/createCertInfo.xul (content/createCertInfo.xul)
content/pippki/deletecert.js (content/deletecert.js)
@@ -27,16 +28,16 @@ pippki.jar:
content/pippki/device_manager.xul (content/device_manager.xul)
content/pippki/downloadcert.js (content/downloadcert.js)
content/pippki/downloadcert.xul (content/downloadcert.xul)
content/pippki/editcacert.js (content/editcacert.js)
content/pippki/editcacert.xul (content/editcacert.xul)
content/pippki/exceptionDialog.js (content/exceptionDialog.js)
* content/pippki/exceptionDialog.xul (content/exceptionDialog.xul)
content/pippki/load_device.xul (content/load_device.xul)
- content/pippki/password.js (content/password.js)
content/pippki/pippki.js (content/pippki.js)
content/pippki/protectedAuth.js (content/protectedAuth.js)
content/pippki/protectedAuth.xul (content/protectedAuth.xul)
content/pippki/resetpassword.js (content/resetpassword.js)
content/pippki/resetpassword.xul (content/resetpassword.xul)
+ content/pippki/setp12password.js (content/setp12password.js)
content/pippki/setp12password.xul (content/setp12password.xul)
content/pippki/viewCertDetails.xul (content/viewCertDetails.xul)
--- a/security/manager/ssl/tests/mochitest/browser/browser.ini
+++ b/security/manager/ssl/tests/mochitest/browser/browser.ini
@@ -7,11 +7,12 @@ support-files =
[browser_bug627234_perwindowpb.js]
[browser_certificateManagerLeak.js]
[browser_certViewer.js]
[browser_clientAuth_connection.js]
[browser_clientAuth_ui.js]
[browser_deleteCert_ui.js]
[browser_downloadCert_ui.js]
[browser_editCACertTrust.js]
-# An earlier attempting at landing this test resulted in frequent intermittent
-# failures, almost entirely on Linux.
+# An earlier attempt at landing this test resulted in frequent intermittent
+# failures, almost entirely on Linux. See Bug 1309519.
skip-if = os == "linux"
+[browser_exportP12_passwordUI.js]
new file mode 100644
--- /dev/null
+++ b/security/manager/ssl/tests/mochitest/browser/browser_exportP12_passwordUI.js
@@ -0,0 +1,142 @@
+// Any copyright is dedicated to the Public Domain.
+// http://creativecommons.org/publicdomain/zero/1.0/
+"use strict";
+
+// Tests that the UI for setting the password on a to be exported PKCS #12 file:
+// 1. Correctly requires the password to be typed in twice as confirmation.
+// 2. Calculates and displays the strength of said password.
+
+/**
+ * @typedef {TestCase}
+ * @type Object
+ * @property {String} name
+ * The name of the test case for display purposes.
+ * @property {String} password1
+ * The password to enter into the first password textbox.
+ * @property {String} password2
+ * The password to enter into the second password textbox.
+ * @property {String} strength
+ * The expected strength of the password in the range [0, 100].
+ */
+
+/**
+ * A list of test cases representing various inputs to the password textboxes.
+ * @type TestCase[]
+ */
+const TEST_CASES = [
+ { name: "empty",
+ password1: "",
+ password2: "",
+ strength: "0" },
+ { name: "match-weak",
+ password1: "foo",
+ password2: "foo",
+ strength: "10" },
+ { name: "match-medium",
+ password1: "foo123",
+ password2: "foo123",
+ strength: "60" },
+ { name: "match-strong",
+ password1: "fooBARBAZ 1234567890`~!@#$%^&*()-_=+{[}]|\\:;'\",<.>/?一二三",
+ password2: "fooBARBAZ 1234567890`~!@#$%^&*()-_=+{[}]|\\:;'\",<.>/?一二三",
+ strength: "100" },
+ { name: "mismatch-weak",
+ password1: "foo",
+ password2: "bar",
+ strength: "10" },
+ { name: "mismatch-medium",
+ password1: "foo123",
+ password2: "bar",
+ strength: "60" },
+ { name: "mismatch-strong",
+ password1: "fooBARBAZ 1234567890`~!@#$%^&*()-_=+{[}]|\\:;'\",<.>/?一二三",
+ password2: "bar",
+ strength: "100" },
+];
+
+/**
+ * Opens the dialog shown to set the password on a PKCS #12 file being exported.
+ *
+ * @returns {Promise}
+ * A promise that resolves when the dialog has finished loading, with
+ * an array consisting of:
+ * 1. The window of the opened dialog.
+ * 2. The return value nsIWritablePropertyBag2 passed to the dialog.
+ */
+function openSetP12PasswordDialog() {
+ let returnVals = Cc["@mozilla.org/hash-property-bag;1"]
+ .createInstance(Ci.nsIWritablePropertyBag2);
+ let win = window.openDialog("chrome://pippki/content/setp12password.xul", "",
+ "", returnVals);
+ return new Promise((resolve, reject) => {
+ win.addEventListener("load", function onLoad() {
+ win.removeEventListener("load", onLoad);
+ resolve([win, returnVals]);
+ });
+ });
+}
+
+// Tests that the first password textbox is the element that is initially
+// focused.
+add_task(function* testFocus() {
+ let [win, retVals] = yield openSetP12PasswordDialog();
+ Assert.equal(win.document.activeElement,
+ win.document.getElementById("pw1").inputField,
+ "First password textbox should have focus");
+ yield BrowserTestUtils.closeWindow(win);
+});
+
+// Tests that the password strength algorithm used is reasonable, and that the
+// Accept button is only enabled if the two passwords match.
+add_task(function* testPasswordStrengthAndEquality() {
+ let [win, retVals] = yield openSetP12PasswordDialog();
+ let password1Textbox = win.document.getElementById("pw1");
+ let password2Textbox = win.document.getElementById("pw2");
+ let strengthProgressBar = win.document.getElementById("pwmeter");
+
+ for (let testCase of TEST_CASES) {
+ password1Textbox.value = testCase.password1;
+ password2Textbox.value = testCase.password2;
+ // Setting the value of the password textboxes via |.value| apparently
+ // doesn't cause the oninput handlers to be called, so we do it here.
+ password1Textbox.oninput();
+ password2Textbox.oninput();
+
+ Assert.equal(win.document.documentElement.getButton("accept").disabled,
+ password1Textbox.value != password2Textbox.value,
+ "Actual and expected accept button disable state should " +
+ `match for ${testCase.name}`);
+ Assert.equal(strengthProgressBar.value, testCase.strength,
+ "Actual and expected strength value should match for" +
+ `${testCase.name}`);
+ }
+
+ yield BrowserTestUtils.closeWindow(win);
+});
+
+// Test that the right values are returned when the dialog is accepted.
+add_task(function* testAcceptDialogReturnValues() {
+ let [win, retVals] = yield openSetP12PasswordDialog();
+ const password = "fooBAR 1234567890`~!@#$%^&*()-_=+{[}]|\\:;'\",<.>/?一二三";
+ win.document.getElementById("pw1").value = password;
+ win.document.getElementById("pw2").value = password;
+ info("Accepting dialog");
+ win.document.getElementById("setp12password").acceptDialog();
+ yield BrowserTestUtils.windowClosed(win);
+
+ Assert.ok(retVals.get("confirmedPassword"),
+ "Return value should signal user confirmed a password");
+ Assert.equal(retVals.get("password"), password,
+ "Actual and expected password should match");
+});
+
+// Test that the right values are returned when the dialog is canceled.
+add_task(function* testCancelDialogReturnValues() {
+ let [win, retVals] = yield openSetP12PasswordDialog();
+ info("Canceling dialog");
+ win.document.getElementById("setp12password").cancelDialog();
+ yield BrowserTestUtils.windowClosed(win);
+
+ Assert.ok(!retVals.get("confirmedPassword"),
+ "Return value should signal user didn't confirm a password");
+});