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 105744 e3adff2d69d789fe1fdd39615bda88a13810f370
parent 105743 e25785cfd8ad507dbd2dcced1031c3a1d187934d
child 105745 2464ccbf691f1dc08f219ff83ea392eaaeecb6e7
push id55
push usershu@rfrn.org
push dateThu, 30 Aug 2012 01:33:09 +0000
reviewersrnewman
bugs783437
milestone18.0a1
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, {