Bug 783437 - Add conditions acceptance to token server client; r=rnewman
authorGregory Szorc <gps@mozilla.com>
Tue, 28 Aug 2012 13:34:33 -0700
changeset 103710 e3adff2d69d789fe1fdd39615bda88a13810f370
parent 103709 e25785cfd8ad507dbd2dcced1031c3a1d187934d
child 103711 2464ccbf691f1dc08f219ff83ea392eaaeecb6e7
push id23373
push useremorley@mozilla.com
push dateWed, 29 Aug 2012 13:35:47 +0000
treeherdermozilla-central@213cafe746cc [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersrnewman
bugs783437
milestone18.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 783437 - Add conditions acceptance to token server client; r=rnewman
services/common/tests/unit/test_tokenserverclient.js
services/common/tokenserverclient.js
--- a/services/common/tests/unit/test_tokenserverclient.js
+++ b/services/common/tests/unit/test_tokenserverclient.js
@@ -13,16 +13,17 @@ function run_test() {
 add_test(function test_working_bid_exchange() {
   _("Ensure that working BrowserID token exchange works as expected.");
 
   let service = "http://example.com/foo";
 
   let server = httpd_setup({
     "/1.0/foo/1.0": function(request, response) {
       do_check_true(request.hasHeader("accept"));
+      do_check_false(request.hasHeader("x-conditions-accepted"));
       do_check_eq("application/json", request.getHeader("accept"));
 
       response.setStatusLine(request.httpVersion, 200, "OK");
       response.setHeader("Content-Type", "application/json");
 
       let body = JSON.stringify({
         id:           "id",
         key:          "key",
@@ -65,54 +66,271 @@ add_test(function test_invalid_arguments
     } catch (ex) {
       do_check_true(ex instanceof TokenServerClientError);
     }
   }
 
   run_next_test();
 });
 
-add_test(function test_error_404() {
-  _("Ensure that 404 responses result in error.");
+add_test(function test_conditions_required_response_handling() {
+  _("Ensure that a conditions required response is handled properly.");
+
+  let description = "Need to accept conditions";
+  let tosURL = "http://example.com/tos";
+
+  let server = httpd_setup({
+    "/1.0/foo/1.0": function(request, response) {
+      do_check_false(request.hasHeader("x-conditions-accepted"));
+
+      response.setStatusLine(request.httpVersion, 403, "Forbidden");
+      response.setHeader("Content-Type", "application/json");
+
+      let body = JSON.stringify({
+        errors: [{description: description, location: "body", name: ""}],
+        urls: {tos: tosURL}
+      });
+      response.bodyOutputStream.write(body, body.length);
+    }
+  });
+
+  let client = new TokenServerClient();
+  let url = TEST_SERVER_URL + "1.0/foo/1.0";
+
+  function onResponse(error, token) {
+    do_check_true(error instanceof TokenServerClientServerError);
+    do_check_eq(error.cause, "conditions-required");
+    do_check_null(token);
+
+    do_check_eq(error.urls.tos, tosURL);
+
+    server.stop(run_next_test);
+  }
+
+  client.getTokenFromBrowserIDAssertion(url, "assertion", onResponse);
+});
+
+add_test(function test_invalid_403_no_content_type() {
+  _("Ensure that a 403 without content-type is handled properly.");
+
+  let server = httpd_setup({
+    "/1.0/foo/1.0": function(request, response) {
+      response.setStatusLine(request.httpVersion, 403, "Forbidden");
+      // No Content-Type header by design.
+
+      let body = JSON.stringify({
+        errors: [{description: "irrelevant", location: "body", name: ""}],
+        urls: {foo: "http://bar"}
+      });
+      response.bodyOutputStream.write(body, body.length);
+    }
+  });
+
+  let client = new TokenServerClient();
+  let url = TEST_SERVER_URL + "1.0/foo/1.0";
+
+  function onResponse(error, token) {
+    do_check_true(error instanceof TokenServerClientServerError);
+    do_check_eq(error.cause, "malformed-response");
+    do_check_null(token);
+
+    do_check_null(error.urls);
+
+    server.stop(run_next_test);
+  }
+
+  client.getTokenFromBrowserIDAssertion(url, "assertion", onResponse);
+});
+
+add_test(function test_invalid_403_bad_json() {
+  _("Ensure that a 403 with JSON that isn't proper is handled properly.");
+
+  let server = httpd_setup({
+    "/1.0/foo/1.0": function(request, response) {
+      response.setStatusLine(request.httpVersion, 403, "Forbidden");
+      response.setHeader("Content-Type", "application/json; charset=utf-8");
+
+      let body = JSON.stringify({
+        foo: "bar"
+      });
+      response.bodyOutputStream.write(body, body.length);
+    }
+  });
+
+  let client = new TokenServerClient();
+  let url = TEST_SERVER_URL + "1.0/foo/1.0";
+
+  function onResponse(error, token) {
+    do_check_true(error instanceof TokenServerClientServerError);
+    do_check_eq(error.cause, "malformed-response");
+    do_check_null(token);
+    do_check_null(error.urls);
+
+    server.stop(run_next_test);
+  }
+
+  client.getTokenFromBrowserIDAssertion(url, "assertion", onResponse);
+});
+
+add_test(function test_403_no_urls() {
+  _("Ensure that a 403 without a urls field is handled properly.");
+
+  let server = httpd_setup({
+    "/1.0/foo/1.0": function(request, response) {
+      response.setStatusLine(request.httpVersion, 403, "Forbidden");
+      response.setHeader("Content-Type", "application/json; charset=utf-8");
+
+      let body = "{}";
+      response.bodyOutputStream.write(body, body.length);
+    }
+  });
+
+  let client = new TokenServerClient();
+  let url = TEST_SERVER_URL + "1.0/foo/1.0";
+
+  client.getTokenFromBrowserIDAssertion(url, "assertion",
+                                        function onResponse(error, result) {
+    do_check_true(error instanceof TokenServerClientServerError);
+    do_check_eq(error.cause, "malformed-response");
+    do_check_null(result);
+
+    server.stop(run_next_test);
+
+  });
+});
+
+add_test(function test_send_conditions_accepted() {
+  _("Ensures that the condition acceptance header is sent when asked.");
+
+  let server = httpd_setup({
+    "/1.0/foo/1.0": function(request, response) {
+      do_check_true(request.hasHeader("x-conditions-accepted"));
+      do_check_eq(request.getHeader("x-conditions-accepted"), "1");
+
+      response.setStatusLine(request.httpVersion, 200, "OK");
+      response.setHeader("Content-Type", "application/json");
+
+      let body = JSON.stringify({
+        id:           "id",
+        key:          "key",
+        api_endpoint: "http://example.com/",
+        uid:          "uid",
+      });
+      response.bodyOutputStream.write(body, body.length);
+    }
+  });
+
+  let client = new TokenServerClient();
+  let url = TEST_SERVER_URL + "1.0/foo/1.0";
+
+  function onResponse(error, token) {
+    do_check_null(error);
+
+    // Other tests validate other things.
+
+    server.stop(run_next_test);
+  }
+
+  client.getTokenFromBrowserIDAssertion(url, "assertion", onResponse, true);
+});
+
+add_test(function test_error_404_empty() {
+  _("Ensure that 404 responses without proper response are handled properly.");
 
   let server = httpd_setup();
 
   let client = new TokenServerClient();
   let url = TEST_SERVER_URL + "foo";
   client.getTokenFromBrowserIDAssertion(url, "assertion", function(error, r) {
-    do_check_neq(null, error);
-    do_check_eq("TokenServerClientServerError", error.name);
+    do_check_true(error instanceof TokenServerClientServerError);
+    do_check_eq(error.cause, "malformed-response");
+
     do_check_neq(null, error.response);
-    do_check_eq(null, r);
+    do_check_null(r);
 
     server.stop(run_next_test);
   });
 });
 
+add_test(function test_error_404_proper_response() {
+  _("Ensure that a Cornice error report for 404 is handled properly.");
+
+  let server = httpd_setup({
+    "/1.0/foo/1.0": function(request, response) {
+      response.setStatusLine(request.httpVersion, 404, "Not Found");
+      response.setHeader("Content-Type", "application/json; charset=utf-8");
+
+      let body = JSON.stringify({
+        status: 404,
+        errors: [{description: "No service", location: "body", name: ""}],
+      });
+
+      response.bodyOutputStream.write(body, body.length);
+    }
+  });
+
+  function onResponse(error, token) {
+    do_check_true(error instanceof TokenServerClientServerError);
+    do_check_eq(error.cause, "unknown-service");
+    do_check_null(token);
+
+    server.stop(run_next_test);
+  }
+
+  let client = new TokenServerClient();
+  let url = TEST_SERVER_URL + "1.0/foo/1.0";
+  client.getTokenFromBrowserIDAssertion(url, "assertion", onResponse);
+});
+
 add_test(function test_bad_json() {
   _("Ensure that malformed JSON is handled properly.");
 
   let server = httpd_setup({
     "/1.0/foo/1.0": function(request, response) {
       response.setStatusLine(request.httpVersion, 200, "OK");
       response.setHeader("Content-Type", "application/json");
 
       let body = '{"id": "id", baz}'
       response.bodyOutputStream.write(body, body.length);
     }
   });
 
   let client = new TokenServerClient();
   let url = TEST_SERVER_URL + "1.0/foo/1.0";
   client.getTokenFromBrowserIDAssertion(url, "assertion", function(error, r) {
-    _(error);
+    do_check_neq(null, error);
+    do_check_eq("TokenServerClientServerError", error.name);
+    do_check_eq(error.cause, "malformed-response");
+    do_check_neq(null, error.response);
+    do_check_eq(null, r);
+
+    server.stop(run_next_test);
+  });
+});
+
+add_test(function test_400_response() {
+  _("Ensure HTTP 400 is converted to malformed-request.");
+
+  let server = httpd_setup({
+    "/1.0/foo/1.0": function(request, response) {
+      response.setStatusLine(request.httpVersion, 400, "Bad Request");
+      response.setHeader("Content-Type", "application/json; charset=utf-8");
+
+      let body = "{}"; // Actual content may not be used.
+      response.bodyOutputStream.write(body, body.length);
+    }
+  });
+
+  let client = new TokenServerClient();
+  let url = TEST_SERVER_URL + "1.0/foo/1.0";
+  client.getTokenFromBrowserIDAssertion(url, "assertion", function(error, r) {
     do_check_neq(null, error);
     do_check_eq("TokenServerClientServerError", error.name);
     do_check_neq(null, error.response);
-    do_check_eq(null, r);
+    do_check_eq(error.cause, "malformed-request");
 
     server.stop(run_next_test);
   });
 });
 
 add_test(function test_unhandled_media_type() {
   _("Ensure that unhandled media types throw an error.");
 
@@ -125,17 +343,17 @@ add_test(function test_unhandled_media_t
       response.bodyOutputStream.write(body, body.length);
     }
   });
 
   let url = TEST_SERVER_URL + "1.0/foo/1.0";
   let client = new TokenServerClient();
   client.getTokenFromBrowserIDAssertion(url, "assertion", function(error, r) {
     do_check_neq(null, error);
-    do_check_eq("TokenServerClientError", error.name);
+    do_check_eq("TokenServerClientServerError", error.name);
     do_check_neq(null, error.response);
     do_check_eq(null, r);
 
     server.stop(run_next_test);
   });
 });
 
 add_test(function test_rich_media_types() {
--- a/services/common/tokenserverclient.js
+++ b/services/common/tokenserverclient.js
@@ -3,17 +3,17 @@
  * You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 "use strict";
 
 const EXPORTED_SYMBOLS = [
   "TokenServerClient",
   "TokenServerClientError",
   "TokenServerClientNetworkError",
-  "TokenServerClientServerError"
+  "TokenServerClientServerError",
 ];
 
 const {classes: Cc, interfaces: Ci, utils: Cu, results: Cr} = Components;
 
 Cu.import("resource://services-common/log4moz.js");
 Cu.import("resource://services-common/preferences.js");
 Cu.import("resource://services-common/rest.js");
 Cu.import("resource://services-common/utils.js");
@@ -48,24 +48,46 @@ function TokenServerClientNetworkError(e
 TokenServerClientNetworkError.prototype = new TokenServerClientError();
 TokenServerClientNetworkError.prototype.constructor =
   TokenServerClientNetworkError;
 
 /**
  * Represents a TokenServerClient error that occurred on the server.
  *
  * This type will be encountered for all non-200 response codes from the
- * server.
+ * server. The type of error is strongly enumerated and is stored in the
+ * `cause` property. This property can have the following string values:
+ *
+ *   conditions-required -- The server is requesting that the client
+ *     agree to service conditions before it can obtain a token. The
+ *     conditions that must be presented to the user and agreed to are in
+ *     the `urls` mapping on the instance. Keys of this mapping are
+ *     identifiers. Values are string URLs.
+ *
+ *   invalid-credentials -- A token could not be obtained because
+ *     the credentials presented by the client were invalid.
+ *
+ *   unknown-service -- The requested service was not found.
+ *
+ *   malformed-request -- The server rejected the request because it
+ *     was invalid. If you see this, code in this file is likely wrong.
+ *
+ *   malformed-response -- The response from the server was not what was
+ *     expected.
+ *
+ *   general -- A general server error has occurred. Clients should
+ *     interpret this as an opaque failure.
  *
  * @param message
  *        (string) Error message.
  */
-function TokenServerClientServerError(message) {
+function TokenServerClientServerError(message, cause="general") {
   this.name = "TokenServerClientServerError";
   this.message = message || "Server error.";
+  this.cause = cause;
 }
 TokenServerClientServerError.prototype = new TokenServerClientError();
 TokenServerClientServerError.prototype.constructor =
   TokenServerClientServerError;
 
 /**
  * Represents a client to the Token Server.
  *
@@ -80,92 +102,138 @@ TokenServerClientServerError.prototype.c
  * If you are tempted to implement this API in the future, consider this your
  * warning that you may be doing it wrong and that you should store full URIs
  * instead.
  *
  * Areas to Improve:
  *
  *  - The server sends a JSON response on error. The client does not currently
  *    parse this. It might be convenient if it did.
- *  - Currently all non-200 status codes are rolled into one error type. It
+ *  - Currently most non-200 status codes are rolled into one error type. It
  *    might be helpful if callers had a richer API that communicated who was
  *    at fault (e.g. differentiating a 503 from a 401).
  */
 function TokenServerClient() {
   this._log = Log4Moz.repository.getLogger("Common.TokenServerClient");
   this._log.level = Log4Moz.Level[Prefs.get("logger.level")];
 }
 TokenServerClient.prototype = {
   /**
    * Logger instance.
    */
   _log: null,
 
   /**
    * Obtain a token from a BrowserID assertion against a specific URL.
    *
-   * This asynchronously obtains the token. The callback receives 2 arguments.
-   * The first signifies an error and is a TokenServerClientError (or derived)
-   * type when an error occurs. If an HTTP response was seen, a RESTResponse
-   * instance will be stored in the "response" property of this object.
+   * This asynchronously obtains the token. The callback receives 2 arguments:
+   *
+   *   (TokenServerClientError | null) If no token could be obtained, this
+   *     will be a TokenServerClientError instance describing why. The
+   *     type seen defines the type of error encountered. If an HTTP response
+   *     was seen, a RESTResponse instance will be stored in the `response`
+   *     property of this object. If there was no error and a token is
+   *     available, this will be null.
    *
-   * The second argument to the callback is a map containing the results from
-   * the server. This map has the following keys:
+   *   (map | null) On success, this will be a map containing the results from
+   *     the server. If there was an error, this will be null. The map has the
+   *     following properties:
+   *
+   *       id       (string) HTTP MAC public key identifier.
+   *       key      (string) HTTP MAC shared symmetric key.
+   *       endpoint (string) URL where service can be connected to.
+   *       uid      (string) user ID for requested service.
+   *       duration (string) the validity duration of the issued token.
+   *
+   * Terms of Service Acceptance
+   * ---------------------------
    *
-   *   id       (string) HTTP MAC public key identifier.
-   *   key      (string) HTTP MAC shared symmetric key.
-   *   endpoint (string) URL where service can be connected to.
-   *   uid      (string) user ID for requested service.
-   *   duration (string) the validity duration of the issued token. 
+   * Some services require users to accept terms of service before they can
+   * obtain a token. If a service requires ToS acceptance, the error passed
+   * to the callback will be a `TokenServerClientServerError` with the
+   * `cause` property set to "conditions-required". The `urls` property of that
+   * instance will be a map of string keys to string URL values. The user-agent
+   * should prompt the user to accept the content at these URLs.
    *
-   * e.g.
+   * Clients signify acceptance of the terms of service by sending a token
+   * request with additional metadata. This is controlled by the
+   * `conditionsAccepted` argument to this function. Clients only need to set
+   * this flag once per service and the server remembers acceptance. If
+   * the conditions for the service change, the server may request
+   * clients agree to terms again. Therefore, clients should always be
+   * prepared to handle a conditions required response.
+   *
+   * Clients should not blindly send acceptance to conditions. Instead, clients
+   * should set `conditionsAccepted` if and only if the server asks for
+   * acceptance, the conditions are displayed to the user, and the user agrees
+   * to them.
+   *
+   * Example Usage
+   * -------------
    *
    *   let client = new TokenServerClient();
    *   let assertion = getBrowserIDAssertionFromSomewhere();
    *   let url = "https://token.services.mozilla.com/1.0/sync/2.0";
    *
    *   client.getTokenFromBrowserIDAssertion(url, assertion,
-   *                                         function(error, result) {
+   *                                         function onResponse(error, result) {
    *     if (error) {
-   *       // Do error handling.
+   *       if (error.cause == "conditions-required") {
+   *         promptConditionsAcceptance(error.urls, function onAccept() {
+   *           client.getTokenFromBrowserIDAssertion(url, assertion,
+   *           onResponse, true);
+   *         }
+   *         return;
+   *       }
+   *
+   *       // Do other error handling.
    *       return;
    *     }
    *
    *     let {
    *       id: id, key: key, uid: uid, endpoint: endpoint, duration: duration
    *     } = result;
    *     // Do stuff with data and carry on.
    *   });
    *
    * @param  url
    *         (string) URL to fetch token from.
    * @param  assertion
    *         (string) BrowserID assertion to exchange token for.
    * @param  cb
    *         (function) Callback to be invoked with result of operation.
+   * @param  conditionsAccepted
+   *         (bool) Whether to send acceptance to service conditions.
    */
   getTokenFromBrowserIDAssertion:
-    function getTokenFromBrowserIDAssertion(url, assertion, cb) {
+    function getTokenFromBrowserIDAssertion(url, assertion, cb,
+                                            conditionsAccepted=false) {
     if (!url) {
       throw new TokenServerClientError("url argument is not valid.");
     }
 
     if (!assertion) {
       throw new TokenServerClientError("assertion argument is not valid.");
     }
 
     if (!cb) {
       throw new TokenServerClientError("cb argument is not valid.");
     }
 
     this._log.debug("Beginning BID assertion exchange: " + url);
 
     let req = new RESTRequest(url);
-    req.setHeader("accept", "application/json");
-    req.setHeader("authorization", "Browser-ID " + assertion);
+    req.setHeader("Accept", "application/json");
+    req.setHeader("Authorization", "Browser-ID " + assertion);
+
+    if (conditionsAccepted) {
+      // Value is irrelevant.
+      req.setHeader("X-Conditions-Accepted", "1");
+    }
+
     let client = this;
     req.get(function onResponse(error) {
       if (error) {
         cb(new TokenServerClientNetworkError(error), null);
         return;
       }
 
       let self = this;
@@ -202,53 +270,104 @@ TokenServerClient.prototype = {
    * Handler to process token request responses.
    *
    * @param response
    *        RESTResponse from token HTTP request.
    * @param cb
    *        The original callback passed to the public API.
    */
   _processTokenResponse: function processTokenResponse(response, cb) {
-    this._log.debug("Got token response.");
+    this._log.debug("Got token response: " + response.status);
 
-    if (!response.success) {
-      this._log.info("Non-200 response code to token request: " +
-                     response.status);
-      this._log.debug("Response body: " + response.body);
-      let error = new TokenServerClientServerError("Non 200 response code: " +
-                                                   response.status);
-      error.response = response;
-      cb(error, null);
-      return;
-    }
+    // Responses should *always* be JSON, even in the case of 4xx and 5xx
+    // errors. If we don't see JSON, the server is likely very unhappy.
+    let ct = response.headers["content-type"] || "";
+    if (ct != "application/json" && !ct.startsWith("application/json;")) {
+      this._log.warn("Did not receive JSON response. Misconfigured server?");
+      this._log.debug("Content-Type: " + ct);
+      this._log.debug("Body: " + response.body);
 
-    let ct = response.headers["content-type"];
-    if (ct != "application/json" && ct.indexOf("application/json;") != 0) {
-      let error =  new TokenServerClientError("Unsupported media type: " + ct);
+      let error = new TokenServerClientServerError("Non-JSON response.",
+                                                   "malformed-response");
       error.response = response;
       cb(error, null);
       return;
     }
 
     let result;
     try {
       result = JSON.parse(response.body);
     } catch (ex) {
-      let error = new TokenServerClientServerError("Invalid JSON returned " +
-                                                   "from server.");
+      this._log.warn("Invalid JSON returned by server: " + response.body);
+      let error = new TokenServerClientServerError("Malformed JSON.",
+                                                   "malformed-response");
       error.response = response;
       cb(error, null);
       return;
     }
 
-    for each (let k in ["id", "key", "api_endpoint", "uid"]) {
+    // The service shouldn't have any 3xx, so we don't need to handle those.
+    if (response.status != 200) {
+      // We /should/ have a Cornice error report in the JSON. We log that to
+      // help with debugging.
+      if ("errors" in result) {
+        // This could throw, but this entire function is wrapped in a try. If
+        // the server is sending something not an array of objects, it has
+        // failed to keep its contract with us and there is little we can do.
+        for (let error of result.errors) {
+          this._log.info("Server-reported error: " + JSON.stringify(error));
+        }
+      }
+
+      let error = new TokenServerClientServerError();
+      error.response = response;
+
+      if (response.status == 400) {
+        error.message = "Malformed request.";
+        error.cause = "malformed-request";
+      } else if (response.status == 401) {
+        error.message("Authentication failed.");
+        error.cause = "invalid-credentials";
+      }
+
+      // 403 should represent a "condition acceptance needed" response.
+      //
+      // The extra validation of "urls" is important. We don't want to signal
+      // conditions required unless we are absolutely sure that is what the
+      // server is asking for.
+      else if (response.status == 403) {
+        if (!("urls" in result)) {
+          this._log.warn("403 response without proper fields!");
+          this._log.warn("Response body: " + response.body);
+
+          error.message = "Missing JSON fields.";
+          error.cause = "malformed-response";
+        } else if (typeof(result.urls) != "object") {
+          error.message = "urls field is not a map.";
+          error.cause = "malformed-response";
+        } else {
+          error.message = "Conditions must be accepted.";
+          error.cause = "conditions-required";
+          error.urls = result.urls;
+        }
+      } else if (response.status == 404) {
+        error.message = "Unknown service.";
+        error.cause = "unknown-service";
+      }
+
+      cb(error, null);
+      return;
+    }
+
+    for (let k of ["id", "key", "api_endpoint", "uid"]) {
       if (!(k in result)) {
         let error = new TokenServerClientServerError("Expected key not " +
                                                      " present in result: " +
                                                      k);
+        error.cause = "malformed-response";
         error.response = response;
         cb(error, null);
         return;
       }
     }
 
     this._log.debug("Successful token response: " + result.id);
     cb(null, {