Implement JS client class for our API. (#1289)

api/features_api.py

@@ -41,5 +41,4 @@ class FeaturesAPI(basehandlers.APIHandler):
     feature.put()
     ramcache.flush_all()
 
-    # Callers don't use the JSON response for this API call.
     return {'message': 'Done'}

framework/basehandlers.py

@@ -23,6 +23,7 @@ import re
 
 import flask
 import flask.views
+import werkzeug.exceptions
 
 from google.appengine.api import users
 from google.appengine.ext import db
@@ -42,6 +43,10 @@ import django
 django.setup()
 
 
+# Our API responses are prefixed with this ro prevent attacks that
+# exploit <script src="...">.  See go/xssi.
+XSSI_PREFIX = ')]}\'\n';
+
 class BaseHandler(flask.views.MethodView):
 
   @property
@@ -127,12 +132,19 @@ class APIHandler(BaseHandler):
         }
     return headers
 
+  def defensive_jsonify(self, handler_data):
+    """Return a Flask Response object with a JSON string prefixed with junk."""
+    body = json.dumps(handler_data)
+    return flask.current_app.response_class(
+        XSSI_PREFIX + body,
+        mimetype=flask.current_app.config['JSONIFY_MIMETYPE'])
+
   def get(self, *args, **kwargs):
     """Handle an incoming HTTP GET request."""
     headers = self.get_headers()
     ramcache.check_for_distributed_invalidation()
     handler_data = self.do_get(*args, **kwargs)
-    return flask.jsonify(handler_data), headers
+    return self.defensive_jsonify(handler_data), headers
 
   def post(self, *args, **kwargs):
     """Handle an incoming HTTP POST request."""
@@ -140,7 +152,7 @@ class APIHandler(BaseHandler):
     headers = self.get_headers()
     ramcache.check_for_distributed_invalidation()
     handler_data = self.do_post(*args, **kwargs)
-    return flask.jsonify(handler_data), headers
+    return self.defensive_jsonify(handler_data), headers
 
   def patch(self, *args, **kwargs):
     """Handle an incoming HTTP PATCH request."""
@@ -148,7 +160,7 @@ class APIHandler(BaseHandler):
     headers = self.get_headers()
     ramcache.check_for_distributed_invalidation()
     handler_data = self.do_patch(*args, **kwargs)
-    return flask.jsonify(handler_data), headers
+    return self.defensive_jsonify(handler_data), headers
 
   def delete(self, *args, **kwargs):
     """Handle an incoming HTTP DELETE request."""
@@ -156,7 +168,7 @@ class APIHandler(BaseHandler):
     headers = self.get_headers()
     ramcache.check_for_distributed_invalidation()
     handler_data = self.do_delete(*args, **kwargs)
-    return flask.jsonify(handler_data), headers
+    return self.defensive_jsonify(handler_data), headers
 
   def _get_valid_methods(self):
     """For 405 responses, list methods the concrete handler implements."""
@@ -196,7 +208,12 @@ class APIHandler(BaseHandler):
     user = self.get_current_user(required=True)
     if not user:
       self.abort(403, msg='Sign in required')
-    token = self.get_param('token', required=False)
+    token = self.request.headers.get('X-Xsrf-Token')
+    if not token:
+      try:
+        token = self.get_param('token', required=False)
+      except werkzeug.exceptions.BadRequest:
+        pass  # Raised when the request has no body.
     if not token:
       # TODO(jrobbins): start enforcing in next release
       logging.info("would do self.abort(400, msg='Missing XSRF token')")
@@ -275,11 +292,13 @@ class FlaskHandler(BaseHandler):
         'dismissed_cues': json.dumps(user_pref.dismissed_cues),
       }
       common_data['xsrf_token'] = xsrf.generate_token(user.email())
+      common_data['xsrf_token_expires'] = xsrf.token_expires_sec()
     else:
       common_data['user'] = None
       common_data['login'] = (
           'Sign in', users.create_login_url(dest_url=current_path))
       common_data['xsrf_token'] = xsrf.generate_token(None)
+      common_data['xsrf_token_expires'] = 0
     return common_data
 
   def render(self, template_data, template_path):

framework/basehandlers_test.py

@@ -18,6 +18,7 @@ from __future__ import print_function
 import unittest
 import testing_config  # Must be imported before the module under test.
 
+import json
 import mock
 import flask
 import flask.views
@@ -302,6 +303,27 @@ class APIHandlerTests(unittest.TestCase):
   def setUp(self):
     self.handler = basehandlers.APIHandler()
 
+  def test_get_headers(self):
+    """We always use some standard headers."""
+    actual = self.handler.get_headers()
+    self.assertEqual(
+        {'Strict-Transport-Security':
+             'max-age=63072000; includeSubDomains; preload',
+         'Access-Control-Allow-Origin': '*',
+         'X-UA-Compatible': 'IE=Edge,chrome=1',
+         },
+        actual)
+
+  def test_defensive_jsonify(self):
+    """We prefix our JSON responses with defensive characters."""
+    handler_data = {'one': 1, 'two': 2}
+    with test_app.test_request_context('/path'):
+      actual = self.handler.defensive_jsonify(handler_data)
+
+    actual_sent_text = actual.response[0]
+    self.assertTrue(actual_sent_text.startswith(basehandlers.XSSI_PREFIX))
+    self.assertIn(json.dumps(handler_data), actual_sent_text)
+
   def test_do_get(self):
     """If a subclass does not implement do_get(), raise NotImplementedError."""
     with self.assertRaises(NotImplementedError):
@@ -335,13 +357,25 @@ class APIHandlerTests(unittest.TestCase):
     self.check_bad_HTTP_method(self.handler.do_delete)
 
   @mock.patch('framework.basehandlers.APIHandler.validate_token')
-  def test_require_signed_in_and_xsrf_token__OK(self, mock_validate_token):
-    """User is signed in and has a token."""
+  def test_require_signed_in_and_xsrf_token__OK_body(self, mock_validate_token):
+    """User is signed in and has a token in the request body."""
     testing_config.sign_in('user@example.com', 111)
-    params = {'token': 'valid token'}
+    params = {'token': 'valid body token'}
     with test_app.test_request_context('/path', json=params):
       self.handler.require_signed_in_and_xsrf_token()
-    mock_validate_token.assert_called_once()
+    mock_validate_token.assert_called_once_with(
+        'valid body token', 'user@example.com')
+
+  @mock.patch('framework.basehandlers.APIHandler.validate_token')
+  def test_require_signed_in_and_xsrf_token__OK_header(self, mock_validate_token):
+    """User is signed in and has a token in the request header."""
+    testing_config.sign_in('user@example.com', 111)
+    headers = {'X-Xsrf-Token': 'valid header token'}
+    params = {}
+    with test_app.test_request_context('/path', headers=headers, json=params):
+      self.handler.require_signed_in_and_xsrf_token()
+    mock_validate_token.assert_called_once_with(
+        'valid header token', 'user@example.com')
 
   @unittest.skip('TODO(jrobbins): enable after next release')
   @mock.patch('framework.basehandlers.APIHandler.validate_token')

static/js-src/admin/process_overview_form.js

@@ -8,14 +8,12 @@ if (document.querySelector('.delete-button')) {
       return;
     }
 
-    fetch(`/api/v0/features/${e.currentTarget.dataset.id}`, {
-      method: 'DELETE',
-      credentials: 'include',
-    }).then((resp) => {
-      if (resp.status === 200) {
+    window.csClient.doDelete(`/features/${e.currentTarget.dataset.id}`)
+     .then((resp) => {
+       if (resp.message === 'Done') {
         location.href = '/features';
       }
-    });
+     });
   });
 }
 

static/js-src/cs-client.js

@@ -0,0 +1,104 @@
+/**
+ * Copyright 2021 Google Inc. All rights reserved.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+(function(exports) {
+'use strict';
+
+
+class ChromeStatusClient {
+  constructor(token, tokenExpiresSec) {
+    this.token = token;
+    this.tokenExpiresSec = tokenExpiresSec;
+    this.baseUrl = '/api/v0'; // Same scheme, host, and port.
+  }
+
+  /**
+   * Refresh the XSRF token if necessary.
+   */
+  async ensureTokenIsValid() {
+    if (ChromeStatusClient.isTokenExpired(this.tokenExpiresSec)) {
+      const refreshResponse = await this.doFetch(
+        '/currentuser/token', 'POST', null);
+      this.token = refreshResponse.token;
+      this.tokenExpiresSec = refreshResponse.tokenExpiresSec;
+    }
+  }
+
+  static isTokenExpired(tokenExpiresSec) {
+    const tokenExpiresDate = new Date(tokenExpiresSec * 1000);
+    return tokenExpiresDate < new Date();
+  }
+
+  async doFetch(resource, httpMethod, body, includeToken=true) {
+    const url = this.baseUrl + resource;
+    let headers = {
+      'accept': 'application/json',
+      'content-type': 'application/json',
+    };
+    if (includeToken) {
+      headers['X-Xsrf-Token'] = this.token;
+    }
+    let options = {
+      method: httpMethod,
+      credentials: 'same-origin',
+      headers: headers,
+    };
+    if (body !== null) {
+      options['body'] = JSON.stringify(body);
+    }
+
+    const response = await fetch(url, options);
+
+    if (response.status !== 200) {
+      throw new Error(
+          `Got error response from server: ${response.status}`);
+    }
+    const rawResponseText = await response.text();
+    const XSSIPrefix = ')]}\'\n';
+    if (!rawResponseText.startsWith(XSSIPrefix)) {
+      console.log(rawResponseText);
+      throw new Error(
+          `Response does not start with XSSI prefix: ${XSSIPrefix}`);
+    }
+    return JSON.parse(rawResponseText.substr(XSSIPrefix.length));
+  }
+
+  doGet(resource, body) {
+    // GET's do not use token.
+    return this.doFetch(resource, 'GET', body, false);
+  }
+
+  doPost(resource, body) {
+    return this.ensureTokenIsValid().then(() => {
+      return this.doFetch(resource, 'POST', body);
+    });
+  }
+
+  doPatch(resource, body) {
+    return this.ensureTokenIsValid().then(() => {
+      return this.doFetch(resource, 'PATCH', body);
+    });
+  }
+
+  doDelete(resource) {
+    return this.ensureTokenIsValid().then(() => {
+      return this.doFetch(resource, 'DELETE', null);
+    });
+  }
+};
+
+exports.ChromeStatusClient = ChromeStatusClient;
+})(window);

static/js-src/cues.js

@@ -4,15 +4,8 @@
 
 class CuesService {
   static dismissCue(cue) {
-    const url = location.hostname == 'localhost' ?
-      'https://www.chromestatus.com/api/v0/currentuser/cues' :
-      '/api/v0/currentuser/cues';
-    return fetch(url, {
-      method: 'POST',
-      headers: {'Content-Type': 'application/json'},
-      body: JSON.stringify({cue: cue}),
-    })
-      .then((res) => res.json);
+    return window.csClient.doPost('/currentuser/cues', {cue: cue})
+      .then((res) => res);
     // TODO: catch((error) => { display message }
   }
 }

static/js-src/notifications.js

@@ -4,28 +4,16 @@
 
 class StarService {
   static getStars() {
-    const url = location.hostname == 'localhost' ?
-      'https://www.chromestatus.com/api/v0/currentuser/stars' :
-      '/api/v0/currentuser/stars';
-    return fetch(url, {
-      method: 'GET',
-      headers: {'Content-Type': 'application/json'},
-    })
-      .then((res) => res.json())
+    return window.csClient.doGet('/currentuser/stars')
       .then((res) => res.featureIds);
     // TODO: catch((error) => { display message }
   }
 
   static setStar(featureId, starred) {
-    const url = location.hostname == 'localhost' ?
-      'https://www.chromestatus.com/api/v0/currentuser/stars' :
-      '/api/v0/currentuser/stars';
-    return fetch(url, {
-      method: 'POST',
-      headers: {'Content-Type': 'application/json'},
-      body: JSON.stringify({featureId: featureId, starred: starred}),
-    })
-      .then((res) => res.json);
+    return window.csClient.doPost(
+      '/currentuser/stars',
+      {featureId: featureId, starred: starred})
+      .then((res) => res);
     // TODO: catch((error) => { display message }
   }
 }

templates/_base.html

@@ -57,9 +57,19 @@ limitations under the License.
   </style>
 
   {% block css %}{% endblock %}
-  
-  {# Metric is need by sw registration and page code. #}
-  <script>{% inline_file "/static/js/metric.min.js" %}</script>
+
+  {# Loaded immediately because it is used by JS code on the page. #}
+  <script>
+    {% inline_file "/static/js/metric.min.js" %}
+    {% inline_file "/static/js/cs-client.min.js" %}
+
+    window.CS_env = {
+        token: '{{xsrf_token}}',
+        tokenExpiresSec: {{xsrf_token_expires}},
+    };
+    window.csClient = new ChromeStatusClient(
+        window.CS_env.token, window.CS_env.tokenExpiresSec);
+  </script>
 
   <script src="https://unpkg.com/@webcomponents/webcomponentsjs/webcomponents-loader.js" defer></script>
   <script type="module">