Bug 1476330 - Get but don't save passwords if password saving is disabled. r=MakeMyDay
authorPhilipp Kewisch <mozilla@kewis.ch>
Sat, 01 Sep 2018 10:00:05 +0200
changeset 33701 0eb273edd4ab0ec05bf53914f56e1eee12e10795
parent 33700 8ebed1915704a6107678b2d933790d22496bcb39
child 33702 77212297c915ec15d690788eeac94a726d6319c3
push id388
push userclokep@gmail.com
push dateMon, 28 Jan 2019 20:54:56 +0000
reviewersMakeMyDay
bugs1476330
Bug 1476330 - Get but don't save passwords if password saving is disabled. r=MakeMyDay
calendar/base/modules/utils/calAuthUtils.jsm
calendar/providers/caldav/calDavCalendar.js
calendar/providers/gdata/modules/gdataSession.jsm
calendar/providers/wcap/calWcapSession.js
calendar/test/unit/test_auth_utils.js
calendar/test/unit/xpcshell-shared.ini
--- a/calendar/base/modules/utils/calAuthUtils.jsm
+++ b/calendar/base/modules/utils/calAuthUtils.jsm
@@ -246,16 +246,21 @@ var calauth = {
      * @param {String} aRealm       The password realm (unused on branch)
      */
     passwordManagerSave: function(aUsername, aPassword, aOrigin, aRealm) {
         cal.ASSERT(aUsername);
         cal.ASSERT(aPassword);
 
         let origin = this._ensureOrigin(aOrigin);
 
+        if (!Services.logins.getLoginSavingEnabled(origin)) {
+            throw new Components.Exception("Password saving is disabled for " + origin,
+                                           Cr.NS_ERROR_NOT_AVAILABLE);
+        }
+
         try {
             let logins = Services.logins.findLogins({}, origin, null, aRealm);
 
             let newLoginInfo = Components.classes["@mozilla.org/login-manager/loginInfo;1"]
                                          .createInstance(Components.interfaces.nsILoginInfo);
             newLoginInfo.init(origin, null, aRealm, aUsername, aPassword, "", "");
             if (logins.length > 0) {
                 Services.logins.modifyLogin(logins[0], newLoginInfo);
@@ -283,20 +288,16 @@ var calauth = {
 
         if (typeof aPassword != "object") {
             throw new Components.Exception("", Components.results.NS_ERROR_XPC_NEED_OUT_OBJECT);
         }
 
         let origin = this._ensureOrigin(aOrigin);
 
         try {
-            if (!Services.logins.getLoginSavingEnabled(origin)) {
-                return false;
-            }
-
             let logins = Services.logins.findLogins({}, origin, null, aRealm);
             for (let loginInfo of logins) {
                 if (loginInfo.username == aUsername) {
                     aPassword.value = loginInfo.password;
                     return true;
                 }
             }
         } catch (exc) {
--- a/calendar/providers/caldav/calDavCalendar.js
+++ b/calendar/providers/caldav/calDavCalendar.js
@@ -1638,18 +1638,20 @@ calDavCalendar.prototype = {
                         try {
                             let origin = "oauth:" + sessionId;
                             if (val) {
                                 cal.auth.passwordManagerSave(sessionId, val, origin, pwMgrId);
                             } else {
                                 cal.auth.passwordManagerRemove(sessionId, origin, pwMgrId);
                             }
                         } catch (e) {
-                            // User might have cancelled the master password prompt, thats ok
-                            if (e.result != Components.results.NS_ERROR_ABORT) {
+                            // User might have cancelled the master password prompt, or password saving
+                            // could be disabled. That is ok, throw for everything else.
+                            if (e.result != Components.results.NS_ERROR_ABORT &&
+                                e.result != Components.results.NS_ERROR_NOT_AVAILABLE) {
                                 throw e;
                             }
                         }
                         return (this.mRefreshToken = val);
                     },
                     enumerable: true
                 });
             }
--- a/calendar/providers/gdata/modules/gdataSession.jsm
+++ b/calendar/providers/gdata/modules/gdataSession.jsm
@@ -172,18 +172,20 @@ calGoogleSession.prototype = {
                 try {
                     let origin = "oauth:" + sessionId;
                     if (val) {
                         cal.auth.passwordManagerSave(sessionId, val, origin, pwMgrId);
                     } else {
                         cal.auth.passwordManagerRemove(sessionId, origin, pwMgrId);
                     }
                 } catch (e) {
-                    // User might have cancelled the master password prompt, thats ok
-                    if (e.result != Components.results.NS_ERROR_ABORT) {
+                    // User might have cancelled the master password prompt, or password saving
+                    // could be disabled. That is ok, throw for everything else.
+                    if (e.result != Components.results.NS_ERROR_ABORT &&
+                        e.result != Components.results.NS_ERROR_NOT_AVAILABLE) {
                         throw e;
                     }
                 }
                 return (this.mRefreshToken = val);
             },
             enumerable: true
         });
 
--- a/calendar/providers/wcap/calWcapSession.js
+++ b/calendar/providers/wcap/calWcapSession.js
@@ -300,17 +300,26 @@ calWcapSession.prototype = {
                             respFunc(new Components.Exception(errorToString(calIWcapErrors.WCAP_LOGIN_FAILED),
                                                               calIWcapErrors.WCAP_LOGIN_FAILED));
                         }
                     } else if (loginerr) {
                         respFunc(loginerr);
                     } else {
                         if (outSavePW.value) {
                             // so try to remove old pw from db first:
-                            cal.auth.passwordManagerSave(outUser.value, outPW.value, this.uri.spec, "wcap login");
+                            try {
+                                cal.auth.passwordManagerSave(outUser.value, outPW.value, this.uri.spec, "wcap login");
+                            } catch (e) {
+                                // User might have cancelled the master password prompt, or password saving
+                                // could be disabled. That is ok, throw for everything else.
+                                if (e.result != Components.results.NS_ERROR_ABORT &&
+                                    e.result != Components.results.NS_ERROR_NOT_AVAILABLE) {
+                                    throw e;
+                                }
+                            }
                         }
                         this.credentials.userId = outUser.value;
                         this.credentials.pw = outPW.value; // eslint-disable-line id-length
                         this.setupSession(sessionId, request, (setuperr) => respFunc(setuperr, sessionId));
                     }
                 };
 
                 if (outPW.value) {
new file mode 100644
--- /dev/null
+++ b/calendar/test/unit/test_auth_utils.js
@@ -0,0 +1,98 @@
+/* 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/. */
+
+const USERNAME = "fred";
+const PASSWORD = "********";
+const ORIGIN = "https://origin";
+const REALM = "realm";
+
+function run_test() {
+    do_get_profile();
+    run_next_test();
+}
+
+function checkLoginCount(total) {
+    Assert.equal(total, Services.logins.countLogins("", "", ""));
+}
+
+/**
+ * Tests the passwordManager{Get,Save,Remove} functions
+ */
+add_task(async function test_password_manager() {
+    await Services.logins.initializationPromise;
+    checkLoginCount(0);
+
+    // Save the password
+    cal.auth.passwordManagerSave(USERNAME, PASSWORD, ORIGIN, REALM);
+    checkLoginCount(1);
+
+    // Save again, should modify the existing login
+    cal.auth.passwordManagerSave(USERNAME, PASSWORD, ORIGIN, REALM);
+    checkLoginCount(1);
+
+    // Retrieve the saved password
+    let passout = {};
+    let found = cal.auth.passwordManagerGet(USERNAME, passout, ORIGIN, REALM);
+    Assert.equal(passout.value, PASSWORD);
+    Assert.ok(found);
+    checkLoginCount(1);
+
+    // Retrieving should still happen with signon saving disabled, but saving should not
+    Services.prefs.setBoolPref("signon.rememberSignons", false);
+    passout = {};
+    found = cal.auth.passwordManagerGet(USERNAME, passout, ORIGIN, REALM);
+    Assert.equal(passout.value, PASSWORD);
+    Assert.ok(found);
+
+    Assert.throws(() => cal.auth.passwordManagerSave(USERNAME, PASSWORD, ORIGIN, REALM),
+                 /NS_ERROR_NOT_AVAILABLE/);
+    Services.prefs.clearUserPref("signon.rememberSignons");
+    checkLoginCount(1);
+
+    // Remove the password
+    found = cal.auth.passwordManagerRemove(USERNAME, ORIGIN, REALM);
+    checkLoginCount(0);
+    Assert.ok(found);
+
+    // Really gone?
+    found = cal.auth.passwordManagerRemove(USERNAME, ORIGIN, REALM);
+    checkLoginCount(0);
+    Assert.ok(!found);
+});
+
+/**
+ * Tests various origins that can be passed to passwordManagerSave
+ */
+add_task(async function test_password_manager_origins() {
+    await Services.logins.initializationPromise;
+    checkLoginCount(0);
+
+    // The scheme of the origin should be normalized to lowercase, this won't add any new passwords
+    cal.auth.passwordManagerSave(USERNAME, PASSWORD, "OAUTH:xpcshell@example.com", REALM);
+    checkLoginCount(1);
+    cal.auth.passwordManagerSave(USERNAME, PASSWORD, "oauth:xpcshell@example.com", REALM);
+    checkLoginCount(1);
+
+    // Make sure that the prePath isn't used for oauth, because that is only the scheme
+    let found = cal.auth.passwordManagerGet(USERNAME, {}, "oauth:", REALM);
+    Assert.ok(!found);
+
+    // Save a https url with a path (only prePath should be used)
+    cal.auth.passwordManagerSave(USERNAME, PASSWORD, "https://example.com/withpath", REALM);
+    found = cal.auth.passwordManagerGet(USERNAME, {}, "https://example.com", REALM);
+    Assert.ok(found);
+    checkLoginCount(2);
+
+    // Entering something that is not an URL should assume https
+    cal.auth.passwordManagerSave(USERNAME, PASSWORD, "example.net", REALM);
+    found = cal.auth.passwordManagerGet(USERNAME, {}, "https://example.net", REALM);
+    Assert.ok(found);
+    checkLoginCount(3);
+
+    // Cleanup
+    cal.auth.passwordManagerRemove(USERNAME, "oauth:xpcshell@example.com", REALM);
+    cal.auth.passwordManagerRemove(USERNAME, "https://example.com", REALM);
+    cal.auth.passwordManagerRemove(USERNAME, "https://example.net", REALM);
+    checkLoginCount(0);
+});
--- a/calendar/test/unit/xpcshell-shared.ini
+++ b/calendar/test/unit/xpcshell-shared.ini
@@ -1,13 +1,14 @@
 [test_alarm.js]
 [test_alarmservice.js]
 [test_alarmutils.js]
 [test_attachment.js]
 [test_attendee.js]
+[test_auth_utils.js]
 [test_bug1199942.js]
 [test_bug1204255.js]
 [test_bug1209399.js]
 [test_bug272411.js]
 [test_bug343792.js]
 [test_bug350845.js]
 [test_bug356207.js]
 [test_bug485571.js]