Store API key in login manager, and add temp field in prefs

chrome/content/zotero/preferences/preferences_sync.js

@@ -29,11 +29,13 @@ Zotero_Preferences.Sync = {
 	init: function () {
 		this.updateStorageSettings(null, null, true);
 		
-		document.getElementById('sync-password').value = Zotero.Sync.Server.password;
+		document.getElementById('sync-api-key').value = Zotero.Sync.Data.Local.getAPIKey();
-		var pass = Zotero.Sync.Storage.WebDAV.password;
+		
-		if (pass) {
+		// TEMP: Disabled
-			document.getElementById('storage-password').value = pass;
+		//var pass = Zotero.Sync.Storage.WebDAV.password;
-		}
+		//if (pass) {
+		//	document.getElementById('storage-password').value = pass;
+		//}
 	},
 	
 	updateStorageSettings: function (enabled, protocol, skipWarnings) {

chrome/content/zotero/preferences/preferences_sync.xul

@@ -63,6 +63,7 @@
 								</columns>
 								
 								<rows>
+									<!--
 									<row>
 										<label value="&zotero.preferences.sync.username;"/>
 										<textbox preference="pref-sync-username"
@@ -73,16 +74,27 @@
 										<textbox id="sync-password" type="password"
 											onchange="Zotero.Sync.Server.password = this.value"/>
 									</row>
+									-->
+									<row>
+										<label value="API Key (temp)"/>
+										<textbox id="sync-api-key" maxlength="24" size="25"
+											onchange="Zotero.Sync.Data.Local.setAPIKey(this.value)"/>
+									</row>
 									<row>
 										<box/>
-										<checkbox label="&zotero.preferences.sync.syncAutomatically;" preference="pref-sync-autosync"/>
+										<!--<checkbox label="&zotero.preferences.sync.syncAutomatically;" preference="pref-sync-autosync"/>-->
+										<checkbox label="&zotero.preferences.sync.syncAutomatically;"
+											disabled="true"/>
 									</row>
 									<row>
 										<box/>
 										<vbox>
-											<checkbox label="&zotero.preferences.sync.syncFullTextContent;"
+											<!--<checkbox label="&zotero.preferences.sync.syncFullTextContent;"
 												preference="pref-sync-fulltext-enabled"
-												tooltiptext="&zotero.preferences.sync.syncFullTextContent.desc;"/>
+												tooltiptext="&zotero.preferences.sync.syncFullTextContent.desc;"/>-->
+											<checkbox label="&zotero.preferences.sync.syncFullTextContent;"
+												tooltiptext="&zotero.preferences.sync.syncFullTextContent.desc;"
+												disabled="true"/>
 										</vbox>
 									</row>
 									<!--
@@ -124,7 +136,7 @@
 									oncommand="Zotero_Preferences.Sync.updateStorageSettings(null, this.value)">
 								<menupopup>
 									<menuitem label="Zotero" value="zotero"/>
-									<menuitem label="WebDAV" value="webdav"/>
+									<menuitem label="WebDAV" value="webdav" disabled="true"/><!-- TEMP -->
 								</menupopup>
 							</menulist>
 						</hbox>

chrome/content/zotero/xpcom/sync/syncLocal.js

@@ -28,7 +28,7 @@ if (!Zotero.Sync.Data) {
 }
 
 Zotero.Sync.Data.Local = {
-	_loginManagerHost: 'https://api.zotero.org',
+	_loginManagerHost: 'chrome://zotero',
 	_loginManagerRealm: 'Zotero Web API',
 	_lastSyncTime: null,
 	_lastClassicSyncTime: null,
@@ -42,41 +42,25 @@ Zotero.Sync.Data.Local = {
 	
 	
 	getAPIKey: function () {
-		var apiKey = Zotero.Prefs.get('devAPIKey');
+		Zotero.debug("Getting API key");
-		if (apiKey) {
+		var login = this._getAPIKeyLoginInfo();
-			return apiKey;
+		return login ? login.password : "";
-		}
-		var loginManager = Components.classes["@mozilla.org/login-manager;1"]
-			.getService(Components.interfaces.nsILoginManager);
-		var logins = loginManager.findLogins(
-			{}, this._loginManagerHost, null, this._loginManagerRealm
-		);
-		// Get API from returned array of nsILoginInfo objects
-		if (logins.length) {
-			return logins[0].password;
-		}
-		if (!apiKey) {
-			let username = Zotero.Prefs.get('sync.server.username');
-			if (username) {
-				let password = Zotero.Sync.Data.Local.getLegacyPassword(username);
-				if (!password) {
-					return false;
-				}
-				throw new Error("Unimplemented");
-				// Get API key from server
-				
-				// Store API key
-				
-				// Remove old logins and username pref
-			}
-		}
-		return apiKey;
 	},
 	
 	
 	setAPIKey: function (apiKey) {
 		var loginManager = Components.classes["@mozilla.org/login-manager;1"]
 			.getService(Components.interfaces.nsILoginManager);
+		
+		var oldLoginInfo = this._getAPIKeyLoginInfo();
+		
+		// Clear old login
+		if (oldLoginInfo && (!apiKey || apiKey === "")) {
+			Zotero.debug("Clearing old API key");
+			loginManager.removeLogin(oldLoginInfo);
+			return;
+		}
+		
 		var nsLoginInfo = new Components.Constructor("@mozilla.org/login-manager/loginInfo;1",
 				Components.interfaces.nsILoginInfo, "init");
 		var loginInfo = new nsLoginInfo(
@@ -85,10 +69,53 @@ Zotero.Sync.Data.Local = {
 			this._loginManagerRealm,
 			'API Key',
 			apiKey,
-			"",
+			'',
-			""
+			''
 		);
-		loginManager.addLogin(loginInfo);
+		if (!oldLoginInfo) {
+			Zotero.debug("Setting API key");
+			loginManager.addLogin(loginInfo);
+		}
+		else {
+			Zotero.debug("Replacing API key");
+			loginManager.modifyLogin(oldLoginInfo, loginInfo);
+		}
+	},
+	
+	
+	/**
+	 * @return {nsILoginInfo|false}
+	 */
+	_getAPIKeyLoginInfo: function () {
+		var loginManager = Components.classes["@mozilla.org/login-manager;1"]
+			.getService(Components.interfaces.nsILoginManager);
+		try {
+			var logins = loginManager.findLogins(
+				{},
+				this._loginManagerHost,
+				null,
+				this._loginManagerRealm
+			);
+		}
+		catch (e) {
+			Zotero.logError(e);
+			if (Zotero.isStandalone) {
+				var msg = Zotero.getString('sync.error.loginManagerCorrupted1', Zotero.appName) + "\n\n"
+					+ Zotero.getString('sync.error.loginManagerCorrupted2', [Zotero.appName, Zotero.appName]);
+			}
+			else {
+				var msg = Zotero.getString('sync.error.loginManagerInaccessible') + "\n\n"
+					+ Zotero.getString('sync.error.checkMasterPassword', Zotero.appName) + "\n\n"
+					+ Zotero.getString('sync.error.corruptedLoginManager', Zotero.appName);
+			}
+			var ps = Components.classes["@mozilla.org/embedcomp/prompt-service;1"]
+				.getService(Components.interfaces.nsIPromptService);
+			ps.alert(null, Zotero.getString('general.error'), msg);
+			return false;
+		}
+		
+		// Get API from returned array of nsILoginInfo objects
+		return logins.length ? logins[0] : false;
 	},
 	
 	
@@ -104,6 +131,7 @@ Zotero.Sync.Data.Local = {
 			var logins = loginManager.findLogins({}, loginManagerHost, null, loginManagerRealm);
 		}
 		catch (e) {
+			Zotero.logError(e);
 			return '';
 		}
 		

chrome/content/zotero/xpcom/sync/syncRunner.js

@@ -38,7 +38,10 @@ Zotero.Sync.Runner_Module = function (options = {}) {
 	
 	this.baseURL = options.baseURL || ZOTERO_CONFIG.API_URL;
 	this.apiVersion = options.apiVersion || ZOTERO_CONFIG.API_VERSION;
-	this.apiKey = options.apiKey || Zotero.Sync.Data.Local.getAPIKey();
+	
+	// Allows tests to set apiKey in options or as property, overriding login manager
+	var _apiKey = options.apiKey;
+	Zotero.defineProperty(this, 'apiKey', { set: val => _apiKey = val });
 	
 	Components.utils.import("resource://zotero/concurrentCaller.js");
 	this.caller = new ConcurrentCaller(4);
@@ -66,11 +69,15 @@ Zotero.Sync.Runner_Module = function (options = {}) {
 	var _errors = [];
 	
 	
-	this.getAPIClient = function () {
+	this.getAPIClient = function (options = {}) {
+		if (!options.apiKey) {
+			throw new Error("apiKey not provided");
+		}
+		
 		return new Zotero.Sync.APIClient({
 			baseURL: this.baseURL,
 			apiVersion: this.apiVersion,
-			apiKey: this.apiKey,
+			apiKey: options.apiKey,
 			caller: this.caller
 		});
 	}
@@ -112,7 +119,8 @@ Zotero.Sync.Runner_Module = function (options = {}) {
 		// Purge deleted objects so they don't cause sync errors (e.g., long tags)
 		yield Zotero.purgeDataObjects(true);
 		
-		if (!this.apiKey) {
+		var apiKey = yield _getAPIKey();
+		if (!apiKey) {
 			let msg = "API key not set";
 			let e = new Zotero.Error(msg, 0, { dialogButtonText: null })
 			this.updateIcons(e);
@@ -129,7 +137,7 @@ Zotero.Sync.Runner_Module = function (options = {}) {
 		this.updateIcons('animate');
 		
 		try {
-			let client = this.getAPIClient();
+			let client = this.getAPIClient({ apiKey });
 			
 			let keyInfo = yield this.checkAccess(client, options);
 			if (!keyInfo) {
@@ -603,13 +611,19 @@ Zotero.Sync.Runner_Module = function (options = {}) {
 			return false;
 		}
 		
+		var apiKey = yield _getAPIKey();
+		if (!apiKey) {
+			Zotero.debug("API key not set -- skipping download");
+			return false;
+		}
+		
 		// TEMP
 		var options = {};
 		
 		var itemID = item.id;
 		var modeClass = Zotero.Sync.Storage.Local.getClassForLibrary(item.libraryID);
 		var controller = new modeClass({
-			apiClient: this.getAPIClient()
+			apiClient: this.getAPIClient({apiKey })
 		});
 		
 		// TODO: verify WebDAV on-demand?
@@ -1160,4 +1174,33 @@ Zotero.Sync.Runner_Module = function (options = {}) {
 		_currentLastSyncLabel.value = Zotero.getString('sync.status.lastSync') + " " + msg;
 		_currentLastSyncLabel.hidden = false;
 	}
+	
+	
+	var _getAPIKey = Zotero.Promise.coroutine(function* () {
+		// Set as .apiKey on Runner in tests
+		return _apiKey
+			// Set in login manager
+			|| Zotero.Sync.Data.Local.getAPIKey()
+			// Fallback to old username/password
+			|| (yield _getAPIKeyFromLogin());
+	})
+	
+	
+	var _getAPIKeyFromLogin = Zotero.Promise.coroutine(function* () {
+		var apiKey = "";
+		let username = Zotero.Prefs.get('sync.server.username');
+		if (username) {
+			let password = Zotero.Sync.Data.Local.getLegacyPassword(username);
+			if (!password) {
+				return "";
+			}
+			throw new Error("Unimplemented");
+			// Get API key from server
+			
+			// Store API key
+			
+			// Remove old logins and username pref
+		}
+		return apiKey;
+	})
 }

test/tests/syncLocalTest.js

@@ -1,6 +1,25 @@
 "use strict";
 
 describe("Zotero.Sync.Data.Local", function() {
+	describe("#getAPIKey()/#setAPIKey()", function () {
+		it("should get and set an API key", function* () {
+			var apiKey1 = Zotero.Utilities.randomString(24);
+			var apiKey2 = Zotero.Utilities.randomString(24);
+			Zotero.Sync.Data.Local.setAPIKey(apiKey1);
+			assert.equal(Zotero.Sync.Data.Local.getAPIKey(apiKey1), apiKey1);
+			Zotero.Sync.Data.Local.setAPIKey(apiKey2);
+			assert.equal(Zotero.Sync.Data.Local.getAPIKey(apiKey2), apiKey2);
+		})
+		
+		
+		it("should clear an API key by setting an empty string", function* () {
+			var apiKey = Zotero.Utilities.randomString(24);
+			Zotero.Sync.Data.Local.setAPIKey(apiKey);
+			Zotero.Sync.Data.Local.setAPIKey("");
+			assert.strictEqual(Zotero.Sync.Data.Local.getAPIKey(apiKey), "");
+		})
+	})
+	
 	describe("#processSyncCacheForObjectType()", function () {
 		var types = Zotero.DataObjectUtilities.getTypes();
 		

test/tests/syncRunnerTest.js

@@ -109,10 +109,7 @@ describe("Zotero.Sync.Runner", function () {
 		yield Zotero.DB.queryAsync("DELETE FROM settings WHERE setting='account'");
 		yield Zotero.Users.init();
 		
-		var runner = new Zotero.Sync.Runner_Module({
+		var runner = new Zotero.Sync.Runner_Module({ baseURL, apiKey });
-			baseURL: baseURL,
-			apiKey: apiKey
-		});
 		
 		Components.utils.import("resource://zotero/concurrentCaller.js");
 		var caller = new ConcurrentCaller(1);
@@ -172,7 +169,7 @@ describe("Zotero.Sync.Runner", function () {
 		it("should check key access", function* () {
 			spy = sinon.spy(runner, "checkUser");
 			setResponse('keyInfo.fullAccess');
-			var json = yield runner.checkAccess(runner.getAPIClient());
+			var json = yield runner.checkAccess(runner.getAPIClient({ apiKey }));
 			sinon.assert.calledWith(spy, 1, "Username");
 			var compare = {};
 			Object.assign(compare, responses.keyInfo.fullAccess.json);
@@ -208,7 +205,7 @@ describe("Zotero.Sync.Runner", function () {
 			
 			setResponse('userGroups.groupVersions');
 			var libraries = yield runner.checkLibraries(
-				runner.getAPIClient(), false, responses.keyInfo.fullAccess.json
+				runner.getAPIClient({ apiKey }), false, responses.keyInfo.fullAccess.json
 			);
 			assert.lengthOf(libraries, 4);
 			assert.sameMembers(
@@ -232,13 +229,16 @@ describe("Zotero.Sync.Runner", function () {
 			
 			setResponse('userGroups.groupVersions');
 			var libraries = yield runner.checkLibraries(
-				runner.getAPIClient(), false, responses.keyInfo.fullAccess.json, [userLibraryID]
+				runner.getAPIClient({ apiKey }),
+				false,
+				responses.keyInfo.fullAccess.json,
+				[userLibraryID]
 			);
 			assert.lengthOf(libraries, 1);
 			assert.sameMembers(libraries, [userLibraryID]);
 			
 			var libraries = yield runner.checkLibraries(
-				runner.getAPIClient(),
+				runner.getAPIClient({ apiKey }),
 				false,
 				responses.keyInfo.fullAccess.json,
 				[userLibraryID, publicationsLibraryID]
@@ -247,7 +247,7 @@ describe("Zotero.Sync.Runner", function () {
 			assert.sameMembers(libraries, [userLibraryID, publicationsLibraryID]);
 			
 			var libraries = yield runner.checkLibraries(
-				runner.getAPIClient(),
+				runner.getAPIClient({ apiKey }),
 				false,
 				responses.keyInfo.fullAccess.json,
 				[group1.libraryID]
@@ -275,7 +275,7 @@ describe("Zotero.Sync.Runner", function () {
 			setResponse('groups.ownerGroup');
 			setResponse('groups.memberGroup');
 			var libraries = yield runner.checkLibraries(
-				runner.getAPIClient(), false, responses.keyInfo.fullAccess.json
+				runner.getAPIClient({ apiKey }), false, responses.keyInfo.fullAccess.json
 			);
 			assert.lengthOf(libraries, 4);
 			assert.sameMembers(
@@ -316,7 +316,7 @@ describe("Zotero.Sync.Runner", function () {
 			setResponse('groups.ownerGroup');
 			setResponse('groups.memberGroup');
 			var libraries = yield runner.checkLibraries(
-				runner.getAPIClient(),
+				runner.getAPIClient({ apiKey }),
 				false,
 				responses.keyInfo.fullAccess.json,
 				[group1.libraryID, group2.libraryID]
@@ -337,7 +337,7 @@ describe("Zotero.Sync.Runner", function () {
 			setResponse('groups.ownerGroup');
 			setResponse('groups.memberGroup');
 			var libraries = yield runner.checkLibraries(
-				runner.getAPIClient(), false, responses.keyInfo.fullAccess.json
+				runner.getAPIClient({ apiKey }), false, responses.keyInfo.fullAccess.json
 			);
 			assert.lengthOf(libraries, 4);
 			var groupData1 = responses.groups.ownerGroup;
@@ -368,7 +368,7 @@ describe("Zotero.Sync.Runner", function () {
 				assert.include(text, group1.name);
 			});
 			var libraries = yield runner.checkLibraries(
-				runner.getAPIClient(), false, responses.keyInfo.fullAccess.json
+				runner.getAPIClient({ apiKey }), false, responses.keyInfo.fullAccess.json
 			);
 			assert.lengthOf(libraries, 3);
 			assert.sameMembers(libraries, [userLibraryID, publicationsLibraryID, group2.libraryID]);
@@ -386,7 +386,7 @@ describe("Zotero.Sync.Runner", function () {
 				assert.include(text, group.name);
 			}, "extra1");
 			var libraries = yield runner.checkLibraries(
-				runner.getAPIClient(), false, responses.keyInfo.fullAccess.json
+				runner.getAPIClient({ apiKey }), false, responses.keyInfo.fullAccess.json
 			);
 			assert.lengthOf(libraries, 3);
 			assert.sameMembers(libraries, [userLibraryID, publicationsLibraryID, group.libraryID]);
@@ -403,7 +403,7 @@ describe("Zotero.Sync.Runner", function () {
 				assert.include(text, group.name);
 			}, "cancel");
 			var libraries = yield runner.checkLibraries(
-				runner.getAPIClient(), false, responses.keyInfo.fullAccess.json
+				runner.getAPIClient({ apiKey }), false, responses.keyInfo.fullAccess.json
 			);
 			assert.lengthOf(libraries, 0);
 			assert.isTrue(Zotero.Groups.exists(groupData.json.id));
@@ -632,8 +632,6 @@ describe("Zotero.Sync.Runner", function () {
 			});
 			
 			yield runner.sync({
-				baseURL: baseURL,
-				apiKey: apiKey,
 				onError: e => { throw e },
 			});