From e9d0fa21fade26b833af42b1bd377ff82d2eb0ed Mon Sep 17 00:00:00 2001 From: Mark Hammond Date: Tue, 24 Mar 2015 14:25:30 +1100 Subject: [PATCH] Bug 1131416 - module for the readinglist sync engine to use to talk to the server. r=adw --- browser/app/profile/firefox.js | 1 + .../components/readinglist/ServerClient.jsm | 166 ++++++++++++++ browser/components/readinglist/moz.build | 6 +- .../readinglist/test/xpcshell/head.js | 49 ++++ .../test/xpcshell/test_ServerClient.js | 209 ++++++++++++++++++ .../readinglist/test/xpcshell/xpcshell.ini | 1 + 6 files changed, 428 insertions(+), 4 deletions(-) create mode 100644 browser/components/readinglist/ServerClient.jsm create mode 100644 browser/components/readinglist/test/xpcshell/test_ServerClient.js diff --git a/browser/app/profile/firefox.js b/browser/app/profile/firefox.js index 639057f61455..1d9c011c2d25 100644 --- a/browser/app/profile/firefox.js +++ b/browser/app/profile/firefox.js @@ -1884,3 +1884,4 @@ pref("browser.readinglist.enabled", true); pref("browser.readinglist.sidebarEverOpened", false); // Enable the readinglist engine by default. pref("readinglist.scheduler.enabled", true); +pref("readinglist.server", "https://readinglist.services.mozilla.com/v1"); diff --git a/browser/components/readinglist/ServerClient.jsm b/browser/components/readinglist/ServerClient.jsm new file mode 100644 index 000000000000..6f6c677acba7 --- /dev/null +++ b/browser/components/readinglist/ServerClient.jsm @@ -0,0 +1,166 @@ +/* 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/. */ + +// The client used to access the ReadingList server. + +"use strict"; + +const { classes: Cc, interfaces: Ci, utils: Cu } = Components; + +Cu.import("resource://gre/modules/XPCOMUtils.jsm"); +Cu.import("resource://gre/modules/Services.jsm"); +Cu.import("resource://gre/modules/Log.jsm"); +Cu.import("resource://gre/modules/Task.jsm"); + +XPCOMUtils.defineLazyModuleGetter(this, "RESTRequest", "resource://services-common/rest.js"); +XPCOMUtils.defineLazyModuleGetter(this, "CommonUtils", "resource://services-common/utils.js"); +XPCOMUtils.defineLazyModuleGetter(this, "fxAccounts", "resource://gre/modules/FxAccounts.jsm"); + +let log = Log.repository.getLogger("readinglist.serverclient"); + +const OAUTH_SCOPE = "readinglist"; // The "scope" on the oauth token we request. + +this.EXPORTED_SYMBOLS = [ + "ServerClient", +]; + +// utf-8 joy. rest.js, which we use for the underlying requests, does *not* +// encode the request as utf-8 even though it wants to know the encoding. +// It does, however, explicitly decode the response. This seems insane, but is +// what it is. +// The end result being we need to utf-8 the request and let the response take +// care of itself. +function objectToUTF8Json(obj) { + // FTR, unescape(encodeURIComponent(JSON.stringify(obj))) also works ;) + return CommonUtils.encodeUTF8(JSON.stringify(obj)); +} + +function ServerClient(fxa = fxAccounts) { + this.fxa = fxa; +} + +ServerClient.prototype = { + + request(options) { + return this._request(options.path, options.method, options.body, options.headers); + }, + + get serverURL() { + return Services.prefs.getCharPref("readinglist.server"); + }, + + _getURL(path) { + let result = this.serverURL; + // we expect the path to have a leading slash, so remove any trailing + // slashes on the pref. + if (result.endsWith("/")) { + result = result.slice(0, -1); + } + return result + path; + }, + + // Hook points for testing. + _getToken() { + // Assume token-caching is in place - if it's not we should avoid doing + // this each request. + return this.fxa.getOAuthToken({scope: OAUTH_SCOPE}); + }, + + _removeToken(token) { + // XXX - remove this check once tokencaching landsin FxA. + if (!this.fxa.removeCachedOAuthToken) { + dump("XXX - token caching support is yet to land - can't remove token!"); + return; + } + return this.fxa.removeCachedOAuthToken({token}); + }, + + // Converts an error from the RESTRequest object to an error we export. + _convertRestError(error) { + return error; // XXX - errors? + }, + + // Converts an error from a try/catch handler to an error we export. + _convertJSError(error) { + return error; // XXX - errors? + }, + + /* + * Perform a request - handles authentication + */ + _request: Task.async(function* (path, method, body, headers) { + let token = yield this._getToken(); + let response = yield this._rawRequest(path, method, body, headers, token); + log.debug("initial request got status ${status}", response); + if (response.status == 401) { + // an auth error - assume our token has expired or similar. + this._removeToken(token); + token = yield this._getToken(); + response = yield this._rawRequest(path, method, body, headers, token); + log.debug("retry of request got status ${status}", response); + } + return response; + }), + + /* + * Perform a request *without* abstractions such as auth etc + * + * On success (which *includes* non-200 responses) returns an object like: + * { + * status: 200, # http status code + * headers: {}, # header values keyed by header name. + * body: {}, # parsed json + } + */ + + _rawRequest(path, method, body, headers, oauthToken) { + return new Promise((resolve, reject) => { + let url = this._getURL(path); + log.debug("dispatching request to", url); + let request = new RESTRequest(url); + method = method.toUpperCase(); + + request.setHeader("Accept", "application/json"); + request.setHeader("Content-Type", "application/json; charset=utf-8"); + request.setHeader("Authorization", "Bearer " + oauthToken); + // and additional header specified for this request. + if (headers) { + for (let [headerName, headerValue] in Iterator(headers)) { + log.trace("Caller specified header: ${headerName}=${headerValue}", {headerName, headerValue}); + request.setHeader(headerName, headerValue); + } + } + + request.onComplete = error => { + if (error) { + return reject(this._convertRestError(error)); + } + + let response = request.response; + log.debug("received response status: ${status} ${statusText}", response); + // Handle response status codes we know about + let result = { + status: response.status, + headers: response.headers + }; + try { + if (response.body) { + result.body = JSON.parse(response.body); + } + } catch (e) { + log.info("Failed to parse JSON body |${body}|: ${e}", + {body: response.body, e}); + // We don't reject due to this (and don't even make a huge amount of + // log noise - eg, a 50X error from a load balancer etc may not write + // JSON. + } + + resolve(result); + } + // We are assuming the body has already been decoded and thus contains + // unicode, but the server expects utf-8. encodeURIComponent does that. + request.dispatch(method, objectToUTF8Json(body)); + }); + }, +}; diff --git a/browser/components/readinglist/moz.build b/browser/components/readinglist/moz.build index 89d9ee43a0b8..db1a99bb3b85 100644 --- a/browser/components/readinglist/moz.build +++ b/browser/components/readinglist/moz.build @@ -6,6 +6,8 @@ JAR_MANIFESTS += ['jar.mn'] EXTRA_JS_MODULES.readinglist += [ 'ReadingList.jsm', + 'Scheduler.jsm', + 'ServerClient.jsm', 'SQLiteStore.jsm', ] @@ -17,9 +19,5 @@ BROWSER_CHROME_MANIFESTS += ['test/browser/browser.ini'] XPCSHELL_TESTS_MANIFESTS += ['test/xpcshell/xpcshell.ini'] -EXTRA_JS_MODULES.readinglist += [ - 'Scheduler.jsm', -] - with Files('**'): BUG_COMPONENT = ('Firefox', 'Reading List') diff --git a/browser/components/readinglist/test/xpcshell/head.js b/browser/components/readinglist/test/xpcshell/head.js index caf9f95a9552..65dd71c94925 100644 --- a/browser/components/readinglist/test/xpcshell/head.js +++ b/browser/components/readinglist/test/xpcshell/head.js @@ -5,3 +5,52 @@ const {classes: Cc, interfaces: Ci, utils: Cu, results: Cr} = Components; Cu.import("resource://gre/modules/XPCOMUtils.jsm"); Cu.import("resource://gre/modules/Services.jsm"); + +do_get_profile(); // fxa needs a profile directory for storage. + +Cu.import("resource://gre/modules/FxAccounts.jsm"); +Cu.import("resource://gre/modules/FxAccountsClient.jsm"); + +// Create a mocked FxAccounts object with a signed-in, verified user. +function* createMockFxA() { + + function MockFxAccountsClient() { + this._email = "nobody@example.com"; + this._verified = false; + + this.accountStatus = function(uid) { + let deferred = Promise.defer(); + deferred.resolve(!!uid && (!this._deletedOnServer)); + return deferred.promise; + }; + + this.signOut = function() { return Promise.resolve(); }; + + FxAccountsClient.apply(this); + } + + MockFxAccountsClient.prototype = { + __proto__: FxAccountsClient.prototype + } + + function MockFxAccounts() { + return new FxAccounts({ + fxAccountsClient: new MockFxAccountsClient(), + getAssertion: () => Promise.resolve("assertion"), + }); + } + + let fxa = new MockFxAccounts(); + let credentials = { + email: "foo@example.com", + uid: "1234@lcip.org", + assertion: "foobar", + sessionToken: "dead", + kA: "beef", + kB: "cafe", + verified: true + }; + + yield fxa.setSignedInUser(credentials); + return fxa; +} diff --git a/browser/components/readinglist/test/xpcshell/test_ServerClient.js b/browser/components/readinglist/test/xpcshell/test_ServerClient.js new file mode 100644 index 000000000000..8528e4685ada --- /dev/null +++ b/browser/components/readinglist/test/xpcshell/test_ServerClient.js @@ -0,0 +1,209 @@ +/* Any copyright is dedicated to the Public Domain. + * http://creativecommons.org/publicdomain/zero/1.0/ */ + +"use strict"; + +Cu.import("resource://testing-common/httpd.js"); +Cu.import("resource:///modules/readinglist/ServerClient.jsm"); +Cu.import("resource://gre/modules/Log.jsm"); + +let appender = new Log.DumpAppender(); +for (let logName of ["FirefoxAccounts", "readinglist.serverclient"]) { + Log.repository.getLogger(logName).addAppender(appender); +} + +// Some test servers we use. +let Server = function(handlers) { + this._server = null; + this._handlers = handlers; +} + +Server.prototype = { + start() { + this._server = new HttpServer(); + for (let [path, handler] in Iterator(this._handlers)) { + // httpd.js seems to swallow exceptions + let thisHandler = handler; + let wrapper = (request, response) => { + try { + thisHandler(request, response); + } catch (ex) { + print("**** Handler for", path, "failed:", ex, ex.stack); + throw ex; + } + } + this._server.registerPathHandler(path, wrapper); + } + this._server.start(-1); + }, + + stop() { + return new Promise(resolve => { + this._server.stop(resolve); + this._server = null; + }); + }, + + get host() { + return "http://localhost:" + this._server.identity.primaryPort; + }, +}; + +// An OAuth server that hands out tokens. +function OAuthTokenServer() { + let server; + let handlers = { + "/v1/authorization": (request, response) => { + response.setStatusLine("1.1", 200, "OK"); + let token = "token" + server.numTokenFetches; + print("Test OAuth server handing out token", token); + server.numTokenFetches += 1; + server.activeTokens.add(token); + response.write(JSON.stringify({access_token: token})); + }, + "/v1/destroy": (request, response) => { + // Getting the body seems harder than it should be! + let sis = Cc["@mozilla.org/scriptableinputstream;1"] + .createInstance(Ci.nsIScriptableInputStream); + sis.init(request.bodyInputStream); + let body = JSON.parse(sis.read(sis.available())); + sis.close(); + let token = body.token; + ok(server.activeTokens.delete(token)); + print("after destroy have", server.activeTokens.size, "tokens left.") + response.setStatusLine("1.1", 200, "OK"); + response.write('{}'); + }, + } + server = new Server(handlers); + server.numTokenFetches = 0; + server.activeTokens = new Set(); + return server; +} + +// The tests. +function run_test() { + run_next_test(); +} + +// Arrange for the first token we hand out to be rejected - the client should +// notice the 401 and silently get a new token and retry the request. +add_task(function testAuthRetry() { + let handlers = { + "/v1/batch": (request, response) => { + // We know the first token we will get is "token0", so we simulate that + // "expiring" by only accepting "token1". Then we just echo the response + // back. + let authHeader; + try { + authHeader = request.getHeader("Authorization"); + } catch (ex) {} + if (authHeader != "Bearer token1") { + response.setStatusLine("1.1", 401, "Unauthorized"); + response.write("wrong token"); + return; + } + response.setStatusLine("1.1", 200, "OK"); + response.write(JSON.stringify({ok: true})); + } + }; + let rlserver = new Server(handlers); + rlserver.start(); + let authServer = OAuthTokenServer(); + authServer.start(); + try { + Services.prefs.setCharPref("readinglist.server", rlserver.host + "/v1"); + Services.prefs.setCharPref("identity.fxaccounts.remote.oauth.uri", authServer.host + "/v1"); + + let fxa = yield createMockFxA(); + let sc = new ServerClient(fxa); + + let response = yield sc.request({ + path: "/batch", + method: "post", + body: {foo: "bar"}, + }); + equal(response.status, 200, "got the 200 we expected"); + equal(authServer.numTokenFetches, 2, "took 2 tokens to get the 200") + deepEqual(response.body, {ok: true}); + } finally { + yield authServer.stop(); + yield rlserver.stop(); + } +}); + +// Check that specified headers are seen by the server, and that server headers +// in the response are seen by the client. +add_task(function testHeaders() { + let handlers = { + "/v1/batch": (request, response) => { + ok(request.hasHeader("x-foo"), "got our foo header"); + equal(request.getHeader("x-foo"), "bar", "foo header has the correct value"); + response.setHeader("Server-Sent-Header", "hello"); + response.setStatusLine("1.1", 200, "OK"); + response.write("{}"); + } + }; + let rlserver = new Server(handlers); + rlserver.start(); + try { + Services.prefs.setCharPref("readinglist.server", rlserver.host + "/v1"); + + let fxa = yield createMockFxA(); + let sc = new ServerClient(fxa); + sc._getToken = () => Promise.resolve(); + + let response = yield sc.request({ + path: "/batch", + method: "post", + headers: {"X-Foo": "bar"}, + body: {foo: "bar"}}); + equal(response.status, 200, "got the 200 we expected"); + equal(response.headers["server-sent-header"], "hello", "got the server header"); + } finally { + yield rlserver.stop(); + } +}); + +// Check that unicode ends up as utf-8 in requests, and vice-versa in responses. +// (Note the ServerClient assumes all strings in and out are UCS, and thus have +// already been encoded/decoded (ie, it never expects to receive stuff already +// utf-8 encoded, and never returns utf-8 encoded responses.) +add_task(function testUTF8() { + let handlers = { + "/v1/hello": (request, response) => { + // Get the body as bytes. + let sis = Cc["@mozilla.org/scriptableinputstream;1"] + .createInstance(Ci.nsIScriptableInputStream); + sis.init(request.bodyInputStream); + let body = sis.read(sis.available()); + sis.close(); + // The client sent "{"copyright: "\xa9"} where \xa9 is the copyright symbol. + // It should have been encoded as utf-8 which is \xc2\xa9 + equal(body, '{"copyright":"\xc2\xa9"}', "server saw utf-8 encoded data"); + // and just write it back unchanged. + response.setStatusLine("1.1", 200, "OK"); + response.write(body); + } + }; + let rlserver = new Server(handlers); + rlserver.start(); + try { + Services.prefs.setCharPref("readinglist.server", rlserver.host + "/v1"); + + let fxa = yield createMockFxA(); + let sc = new ServerClient(fxa); + sc._getToken = () => Promise.resolve(); + + let body = {copyright: "\xa9"}; // see above - \xa9 is the copyright symbol + let response = yield sc.request({ + path: "/hello", + method: "post", + body: body + }); + equal(response.status, 200, "got the 200 we expected"); + deepEqual(response.body, body); + } finally { + yield rlserver.stop(); + } +}); diff --git a/browser/components/readinglist/test/xpcshell/xpcshell.ini b/browser/components/readinglist/test/xpcshell/xpcshell.ini index f89f63d89996..d10cbc176f16 100644 --- a/browser/components/readinglist/test/xpcshell/xpcshell.ini +++ b/browser/components/readinglist/test/xpcshell/xpcshell.ini @@ -3,5 +3,6 @@ head = head.js firefox-appdir = browser [test_ReadingList.js] +[test_ServerClient.js] [test_scheduler.js] [test_SQLiteStore.js]