Bug 557589 - code audit and create unit test plan for service.js [r=mconnor]

Part 4: Get rid of superfluous attribute, introduce constants for password/passphrase realms, add/improve tests for login(), logout(), persistLogin().

services/sync/modules/constants.js

@@ -53,6 +53,8 @@ PREFS_BRANCH:                          "services.sync.",
 
 // Host "key" to access Weave Identity in the password manager
 PWDMGR_HOST:                           "chrome://weave",
+PWDMGR_PASSWORD_REALM:                 "Mozilla Services Password",
+PWDMGR_PASSPHRASE_REALM:               "Mozilla Services Encryption Passphrase",
 
 // Sync intervals for various clients configurations
 SINGLE_USER_SYNC:                      24 * 60 * 60 * 1000, // 1 day

services/sync/modules/service.js

@@ -103,9 +103,6 @@ WeaveSvc.prototype = {
   _loggedIn: false,
   keyGenEnabled: true,
 
-  // object for caching public and private keys
-  _keyPair: {},
-
   get username() {
     return Svc.Prefs.get("username", "").toLowerCase();
   },
@@ -287,11 +284,11 @@ WeaveSvc.prototype = {
       this._log.info("Weave Sync disabled");
 
     // Create Weave identities (for logging in, and for encryption)
-    ID.set('WeaveID', new Identity('Mozilla Services Password', this.username));
+    ID.set('WeaveID', new Identity(PWDMGR_PASSWORD_REALM, this.username));
     Auth.defaultAuthenticator = new BasicAuthenticator(ID.get('WeaveID'));
 
     ID.set('WeaveCryptoID',
-           new Identity('Mozilla Services Encryption Passphrase', this.username));
+           new Identity(PWDMGR_PASSPHRASE_REALM, this.username));
 
     this._updateCachedURLs();
 
@@ -764,7 +761,6 @@ WeaveSvc.prototype = {
 
     this._log.info("Logging out");
     this._loggedIn = false;
-    this._keyPair = {};
 
     // Cancel the sync timer now that we're logged out
     this._checkSyncStatus();

services/sync/tests/unit/test_service_login.js

@@ -6,9 +6,11 @@ Cu.import("resource://services-sync/util.js");
 
 function login_handler(request, response) {
   // btoa('johndoe:ilovejane') == am9obmRvZTppbG92ZWphbmU=
+  // btoa('janedoe:ilovejohn') == amFuZWRvZTppbG92ZWpvaG4=
   let body;
-  if (request.hasHeader("Authorization") &&
-      request.getHeader("Authorization") == "Basic am9obmRvZTppbG92ZWphbmU=") {
+  let header = request.getHeader("Authorization");
+  if (header == "Basic am9obmRvZTppbG92ZWphbmU="
+      || header == "Basic amFuZWRvZTppbG92ZWpvaG4=") {
     body = "{}";
     response.setStatusLine(request.httpVersion, 200, "OK");
   } else {
@@ -23,7 +25,8 @@ function run_test() {
   Log4Moz.repository.rootLogger.addAppender(new Log4Moz.DumpAppender());
 
   let server = httpd_setup({
-    "/1.0/johndoe/info/collections": login_handler
+    "/1.0/johndoe/info/collections": login_handler,
+    "/1.0/janedoe/info/collections": login_handler
   });
 
   try {
@@ -58,6 +61,32 @@ function run_test() {
     do_check_true(Weave.Service.isLoggedIn);
     do_check_true(Svc.Prefs.get("autoconnect"));
 
+    _("We can also pass username, password and passphrase to login().");
+    Weave.Service.login("janedoe", "incorrectpassword", "bar");
+    do_check_eq(Weave.Service.username, "janedoe");
+    do_check_eq(Weave.Service.password, "incorrectpassword");
+    do_check_eq(Weave.Service.passphrase, "bar");
+    do_check_eq(Status.service, LOGIN_FAILED);
+    do_check_eq(Status.login, LOGIN_FAILED_LOGIN_REJECTED);
+    do_check_false(Weave.Service.isLoggedIn);
+
+    _("Try again with correct password.");
+    Weave.Service.login("janedoe", "ilovejohn");
+    do_check_eq(Status.service, STATUS_OK);
+    do_check_eq(Status.login, LOGIN_SUCCEEDED);
+    do_check_true(Weave.Service.isLoggedIn);
+    do_check_true(Svc.Prefs.get("autoconnect"));
+
+    _("Logout.");
+    Weave.Service.logout();
+    do_check_false(Weave.Service.isLoggedIn);
+    do_check_false(Svc.Prefs.get("autoconnect"));
+
+    _("Logging out again won't do any harm.");
+    Weave.Service.logout();
+    do_check_false(Weave.Service.isLoggedIn);
+    do_check_false(Svc.Prefs.get("autoconnect"));
+
   } finally {
     Svc.Prefs.resetBranch("");
     server.stop(function() {});

services/sync/tests/unit/test_service_persistLogin.js

@@ -0,0 +1,39 @@
+Cu.import("resource://services-sync/service.js");
+Cu.import("resource://services-sync/constants.js");
+
+function run_test() {
+  try {
+    Weave.Service.username = "johndoe";
+    Weave.Service.password = "ilovejane";
+    Weave.Service.passphrase = "my preciousss";
+
+    _("Confirm initial environment is empty.");
+    let logins = Weave.Svc.Login.findLogins({}, PWDMGR_HOST, null,
+                                        PWDMGR_PASSWORD_REALM);
+    do_check_eq(logins.length, 0);
+    logins = Weave.Svc.Login.findLogins({}, PWDMGR_HOST, null,
+                                        PWDMGR_PASSPHRASE_REALM);
+    do_check_eq(logins.length, 0);
+
+    _("Persist logins to the login service");
+    Weave.Service.persistLogin();
+
+    _("The password has been persisted in the login service.");
+    logins = Weave.Svc.Login.findLogins({}, PWDMGR_HOST, null,
+                                        PWDMGR_PASSWORD_REALM);
+    do_check_eq(logins.length, 1);
+    do_check_eq(logins[0].username, "johndoe");
+    do_check_eq(logins[0].password, "ilovejane");
+
+    _("The passphrase has been persisted in the login service.");
+    logins = Weave.Svc.Login.findLogins({}, PWDMGR_HOST, null,
+                                        PWDMGR_PASSPHRASE_REALM);
+    do_check_eq(logins.length, 1);
+    do_check_eq(logins[0].username, "johndoe");
+    do_check_eq(logins[0].password, "my preciousss");
+
+  } finally {
+    Weave.Svc.Prefs.resetBranch("");
+    Weave.Svc.Login.removeAllLogins();
+  }
+}