Bug 1052247 - Enforce that OAuth is done over HTTPS in FxAccountsOAuthClient. r?markh
It now follows the setting of the identity.fxaccounts.allowHttp preference.
MozReview-Commit-ID: 9646Xi48QMP
--- a/services/fxaccounts/FxAccounts.jsm
+++ b/services/fxaccounts/FxAccounts.jsm
@@ -1286,16 +1286,17 @@ FxAccountsInternal.prototype = {
},
_rejectWhenVerified(currentState, error) {
currentState.whenVerifiedDeferred.reject(error);
delete currentState.whenVerifiedDeferred;
},
requiresHttps() {
+ // Also used in FxAccountsOAuthGrantClient.jsm.
let allowHttp = Services.prefs.getBoolPref("identity.fxaccounts.allowHttp", false);
return allowHttp !== true;
},
promiseAccountsSignUpURI() {
return FxAccountsConfig.promiseAccountsSignUpURI();
},
--- a/services/fxaccounts/FxAccountsOAuthGrantClient.jsm
+++ b/services/fxaccounts/FxAccountsOAuthGrantClient.jsm
@@ -10,21 +10,24 @@
this.EXPORTED_SYMBOLS = ["FxAccountsOAuthGrantClient", "FxAccountsOAuthGrantClientError"];
const {classes: Cc, interfaces: Ci, utils: Cu, results: Cr} = Components;
Cu.import("resource://gre/modules/Log.jsm");
Cu.import("resource://gre/modules/FxAccountsCommon.js");
Cu.import("resource://services-common/rest.js");
+Cu.import("resource://gre/modules/Services.jsm");
Cu.importGlobalProperties(["URL"]);
const AUTH_ENDPOINT = "/authorization";
const DESTROY_ENDPOINT = "/destroy";
+// This is the same pref that's used by FxAccounts.jsm.
+const ALLOW_HTTP_PREF = "identity.fxaccounts.allowHttp";
/**
* Create a new FxAccountsOAuthClient for browser some service.
*
* @param {Object} options Options
* @param {Object} options.parameters
* @param {String} options.parameters.client_id
* OAuth id returned from client registration
@@ -40,16 +43,21 @@ this.FxAccountsOAuthGrantClient = functi
this.parameters = options;
try {
this.serverURL = new URL(this.parameters.serverURL);
} catch (e) {
throw new Error("Invalid 'serverURL'");
}
+ let forceHTTPS = !Services.prefs.getBoolPref(ALLOW_HTTP_PREF, false);
+ if (forceHTTPS && this.serverURL.protocol != "https:") {
+ throw new Error("'serverURL' must be HTTPS");
+ }
+
log.debug("FxAccountsOAuthGrantClient Initialized");
};
this.FxAccountsOAuthGrantClient.prototype = {
/**
* Retrieves an OAuth access token for the signed in user
*
--- a/services/fxaccounts/tests/xpcshell/test_accounts.js
+++ b/services/fxaccounts/tests/xpcshell/test_accounts.js
@@ -1192,17 +1192,17 @@ add_test(function test_getOAuthToken() {
let alice = getTestUser("alice");
alice.verified = true;
let getTokenFromAssertionCalled = false;
fxa.internal._d_signCertificate.resolve("cert1");
// create a mock oauth client
let client = new FxAccountsOAuthGrantClient({
- serverURL: "http://example.com/v1",
+ serverURL: "https://example.com/v1",
client_id: "abc123"
});
client.getTokenFromAssertion = function() {
getTokenFromAssertionCalled = true;
return Promise.resolve({ access_token: "token" });
};
fxa.setSignedInUser(alice).then(
@@ -1224,17 +1224,17 @@ add_test(function test_getOAuthTokenScop
let alice = getTestUser("alice");
alice.verified = true;
let getTokenFromAssertionCalled = false;
fxa.internal._d_signCertificate.resolve("cert1");
// create a mock oauth client
let client = new FxAccountsOAuthGrantClient({
- serverURL: "http://example.com/v1",
+ serverURL: "https://example.com/v1",
client_id: "abc123"
});
client.getTokenFromAssertion = function(assertion, scopeString) {
equal(scopeString, "foo bar");
getTokenFromAssertionCalled = true;
return Promise.resolve({ access_token: "token" });
};
@@ -1257,17 +1257,17 @@ add_task(async function test_getOAuthTok
let alice = getTestUser("alice");
alice.verified = true;
let numTokenFromAssertionCalls = 0;
fxa.internal._d_signCertificate.resolve("cert1");
// create a mock oauth client
let client = new FxAccountsOAuthGrantClient({
- serverURL: "http://example.com/v1",
+ serverURL: "https://example.com/v1",
client_id: "abc123"
});
client.getTokenFromAssertion = function() {
numTokenFromAssertionCalls += 1;
return Promise.resolve({ access_token: "token" });
};
await fxa.setSignedInUser(alice);
@@ -1290,17 +1290,17 @@ add_task(async function test_getOAuthTok
let alice = getTestUser("alice");
alice.verified = true;
let numTokenFromAssertionCalls = 0;
fxa.internal._d_signCertificate.resolve("cert1");
// create a mock oauth client
let client = new FxAccountsOAuthGrantClient({
- serverURL: "http://example.com/v1",
+ serverURL: "https://example.com/v1",
client_id: "abc123"
});
client.getTokenFromAssertion = function() {
numTokenFromAssertionCalls += 1;
return Promise.resolve({ access_token: "token" });
};
await fxa.setSignedInUser(alice);
@@ -1388,17 +1388,17 @@ add_test(function test_getOAuthToken_net
let fxa = new MockFxAccounts();
let alice = getTestUser("alice");
alice.verified = true;
fxa.internal._d_signCertificate.resolve("cert1");
// create a mock oauth client
let client = new FxAccountsOAuthGrantClient({
- serverURL: "http://example.com/v1",
+ serverURL: "https://example.com/v1",
client_id: "abc123"
});
client.getTokenFromAssertion = function() {
return Promise.reject(new FxAccountsOAuthGrantClientError({
error: ERROR_NETWORK,
errno: ERRNO_NETWORK
}));
};
@@ -1417,17 +1417,17 @@ add_test(function test_getOAuthToken_aut
let fxa = new MockFxAccounts();
let alice = getTestUser("alice");
alice.verified = true;
fxa.internal._d_signCertificate.resolve("cert1");
// create a mock oauth client
let client = new FxAccountsOAuthGrantClient({
- serverURL: "http://example.com/v1",
+ serverURL: "https://example.com/v1",
client_id: "abc123"
});
client.getTokenFromAssertion = function() {
return Promise.reject(new FxAccountsOAuthGrantClientError({
error: ERROR_INVALID_FXA_ASSERTION,
errno: ERRNO_INVALID_FXA_ASSERTION
}));
};
@@ -1446,17 +1446,17 @@ add_test(function test_getOAuthToken_unk
let fxa = new MockFxAccounts();
let alice = getTestUser("alice");
alice.verified = true;
fxa.internal._d_signCertificate.resolve("cert1");
// create a mock oauth client
let client = new FxAccountsOAuthGrantClient({
- serverURL: "http://example.com/v1",
+ serverURL: "https://example.com/v1",
client_id: "abc123"
});
client.getTokenFromAssertion = function() {
return Promise.reject("boom");
};
fxa.setSignedInUser(alice).then(() => {
fxa.getOAuthToken({ scope: "profile", client })
--- a/services/fxaccounts/tests/xpcshell/test_oauth_grant_client.js
+++ b/services/fxaccounts/tests/xpcshell/test_oauth_grant_client.js
@@ -2,17 +2,17 @@
* http://creativecommons.org/publicdomain/zero/1.0/ */
"use strict";
Cu.import("resource://gre/modules/FxAccountsCommon.js");
Cu.import("resource://gre/modules/FxAccountsOAuthGrantClient.jsm");
const CLIENT_OPTIONS = {
- serverURL: "http://127.0.0.1:9010/v1",
+ serverURL: "https://127.0.0.1:9010/v1",
client_id: "abc123"
};
const STATUS_SUCCESS = 200;
/**
* Mock request responder
* @param {String} response
@@ -131,17 +131,17 @@ add_test(function serverErrorResponse()
do_check_eq(e.error, "Bad Request");
do_check_eq(e.message, "Unauthorized");
run_next_test();
});
});
add_test(function networkErrorResponse() {
let client = new FxAccountsOAuthGrantClient({
- serverURL: "http://domain.dummy",
+ serverURL: "https://domain.dummy",
client_id: "abc123"
});
Services.prefs.setBoolPref("identity.fxaccounts.skipDeviceRegistration", true);
client.getTokenFromAssertion("assertion", "scope")
.catch(function(e) {
do_check_eq(e.name, "FxAccountsOAuthGrantClientError");
do_check_eq(e.code, null);
do_check_eq(e.errno, ERRNO_NETWORK);
@@ -200,24 +200,36 @@ add_test(function incorrectErrno() {
add_test(function constructorTests() {
validationHelper(undefined,
"Error: Missing configuration options");
validationHelper({},
"Error: Missing 'serverURL' parameter");
- validationHelper({ serverURL: "http://example.com" },
+ validationHelper({ serverURL: "https://example.com" },
+ "Error: Missing 'client_id' parameter");
+
+ validationHelper({ serverURL: "https://example.com" },
"Error: Missing 'client_id' parameter");
validationHelper({ client_id: "123ABC" },
"Error: Missing 'serverURL' parameter");
- validationHelper({ client_id: "123ABC", serverURL: "badUrl" },
- "Error: Invalid 'serverURL'");
+ validationHelper({ client_id: "123ABC", serverURL: "http://example.com" },
+ "Error: 'serverURL' must be HTTPS");
+
+ try {
+ Services.prefs.setBoolPref("identity.fxaccounts.allowHttp", true);
+ validationHelper({ client_id: "123ABC", serverURL: "http://example.com" }, null);
+ } finally {
+ Services.prefs.clearUserPref("identity.fxaccounts.allowHttp");
+ }
+
+
run_next_test();
});
add_test(function errorTests() {
let error1 = new FxAccountsOAuthGrantClientError();
do_check_eq(error1.name, "FxAccountsOAuthGrantClientError");
do_check_eq(error1.code, null);
@@ -245,25 +257,44 @@ add_test(function errorTests() {
do_check_eq(fields2.errno, statusCode);
do_check_eq(fields2.error, "Error");
do_check_eq(fields2.message, "Something");
do_check_true(error2.toString().indexOf("Something") >= 0);
run_next_test();
});
+
+add_test(function networkErrorResponse() {
+ let client = new FxAccountsOAuthGrantClient({
+ serverURL: "https://domain.dummy",
+ client_id: "abc123"
+ });
+ Services.prefs.setBoolPref("identity.fxaccounts.skipDeviceRegistration", true);
+ client.getTokenFromAssertion("assertion", "scope")
+ .catch(function(e) {
+ do_check_eq(e.name, "FxAccountsOAuthGrantClientError");
+ do_check_eq(e.code, null);
+ do_check_eq(e.errno, ERRNO_NETWORK);
+ do_check_eq(e.error, ERROR_NETWORK);
+ run_next_test();
+ }).catch(() => {}).then(() =>
+ Services.prefs.clearUserPref("identity.fxaccounts.skipDeviceRegistration"));
+});
+
+
/**
* Quick way to test the "FxAccountsOAuthGrantClient" constructor.
*
* @param {Object} options
* FxAccountsOAuthGrantClient constructor options
* @param {String} expected
- * Expected error message
+ * Expected error message, or null if it's expected to pass.
* @returns {*}
*/
function validationHelper(options, expected) {
try {
new FxAccountsOAuthGrantClient(options);
} catch (e) {
return do_check_eq(e.toString(), expected);
}
- throw new Error("Validation helper error");
+ return do_check_eq(expected, null);
}
--- a/services/fxaccounts/tests/xpcshell/test_oauth_grant_client_server.js
+++ b/services/fxaccounts/tests/xpcshell/test_oauth_grant_client_server.js
@@ -2,16 +2,17 @@
* http://creativecommons.org/publicdomain/zero/1.0/ */
// A test of FxAccountsOAuthGrantClient but using a real server it can
// hit.
"use strict";
Cu.import("resource://gre/modules/FxAccountsCommon.js");
Cu.import("resource://gre/modules/FxAccountsOAuthGrantClient.jsm");
+Cu.import("resource://gre/modules/Services.jsm");
// handlers for our server.
var numTokenFetches;
var activeTokens;
function authorize(request, response) {
response.setStatusLine("1.1", 200, "OK");
let token = "token" + numTokenFetches;
@@ -46,24 +47,29 @@ function startServer() {
function promiseStopServer(server) {
return new Promise(resolve => {
server.stop(resolve);
});
}
add_task(async function getAndRevokeToken() {
+ Services.prefs.setBoolPref("identity.fxaccounts.allowHttp", true);
let server = startServer();
- let clientOptions = {
- serverURL: "http://localhost:" + server.identity.primaryPort + "/v1",
- client_id: "abc123",
- }
+ try {
+ let clientOptions = {
+ serverURL: "http://localhost:" + server.identity.primaryPort + "/v1",
+ client_id: "abc123",
+ }
- let client = new FxAccountsOAuthGrantClient(clientOptions);
- let result = await client.getTokenFromAssertion("assertion", "scope");
- equal(result.access_token, "token0");
- equal(numTokenFetches, 1, "we hit the server to fetch a token");
- await client.destroyToken("token0");
- equal(activeTokens.size, 0, "We hit the server to revoke it");
- await promiseStopServer(server);
+ let client = new FxAccountsOAuthGrantClient(clientOptions);
+ let result = await client.getTokenFromAssertion("assertion", "scope");
+ equal(result.access_token, "token0");
+ equal(numTokenFetches, 1, "we hit the server to fetch a token");
+ await client.destroyToken("token0");
+ equal(activeTokens.size, 0, "We hit the server to revoke it");
+ } finally {
+ await promiseStopServer(server);
+ Services.prefs.clearUserPref("identity.fxaccounts.allowHttp");
+ }
});
// XXX - TODO - we should probably add more tests for unexpected responses etc.