Commit 22181c80 authored by Nick Stenning's avatar Nick Stenning

Remove "assertion" GET param from token requests

When fetching a JWT from the server, the client needs to supply the
session CSRF token in order to prevent third-party pages from being able
to fetch and use tokens without the user's permission.

Previously, we supplied the CSRF token in an "assertion" GET parameter
(partially in an attempt to make this look a bit like an OAuth token
issuance API) but in Pyramid 1.7 this isn't allowed. (This is good:
allowing the CSRF to be passed as a GET parameter makes it easier to
construct a cross-domain attack which retrieves a token for the user).

This commit moves the CSRF token into a request header, which works
because there are only two legitimate situations in which this request
is made:

- from an embed iframe, which is on the same origin as the service
- from a Chrome extension iframe, which is permitted to make
  cross-origin XHR requests to URLs specified in the manifest (in our
  case, `<all_urls>`).

Note that we cannot rely on Angular's built-in CSRF support here,
because it does not operate for cross-domain requests.
parent 8281804b
...@@ -5,8 +5,6 @@ ...@@ -5,8 +5,6 @@
* API requests and logging out of the API. * API requests and logging out of the API.
*/ */
var queryString = require('query-string');
var INITIAL_TOKEN = { var INITIAL_TOKEN = {
// The user ID which the current cached token is valid for // The user ID which the current cached token is valid for
userid: undefined, userid: undefined,
...@@ -26,16 +24,19 @@ var cachedToken = INITIAL_TOKEN; ...@@ -26,16 +24,19 @@ var cachedToken = INITIAL_TOKEN;
// @ngInject // @ngInject
function fetchToken($http, session, settings) { function fetchToken($http, session, settings) {
var tokenUrl = new URL('token', settings.apiUrl).href; var tokenUrl = new URL('token', settings.apiUrl).href;
// Explicitly include the CSRF token in the headers. This won't be done
// automatically in the extension as this will be a cross-domain request, and
// Angular's CSRF support doesn't operate automatically cross-domain.
var headers = {};
headers[$http.defaults.xsrfHeaderName] = session.state.csrf;
var config = { var config = {
params: { headers: headers,
assertion: session.state.csrf,
},
// Skip JWT authorization for the token request itself. // Skip JWT authorization for the token request itself.
skipAuthorization: true, skipAuthorization: true,
transformRequest: function (data) {
return queryString.stringify(data);
},
}; };
return $http.get(tokenUrl, config).then(function (response) { return $http.get(tokenUrl, config).then(function (response) {
return response.data; return response.data;
}); });
......
...@@ -13,10 +13,13 @@ describe('auth', function () { ...@@ -13,10 +13,13 @@ describe('auth', function () {
beforeEach(function () { beforeEach(function () {
fakeTokenIndex = 0; fakeTokenIndex = 0;
fakeHttp = { fakeHttp = {
defaults: {xsrfHeaderName: 'X-CSRF-Token'},
get: sinon.spy(function (url, config) { get: sinon.spy(function (url, config) {
assert.equal(config.skipAuthorization, true); assert.equal(config.skipAuthorization, true);
assert.equal(url, 'https://test.hypothes.is/api/token'); assert.equal(url, 'https://test.hypothes.is/api/token');
assert.equal(config.params.assertion, fakeSession.state.csrf); assert.deepEqual(config.headers, {
'X-CSRF-Token': fakeSession.state.csrf,
});
var result = {status: 200, data: fakeTokens[fakeTokenIndex]}; var result = {status: 200, data: fakeTokens[fakeTokenIndex]};
++fakeTokenIndex; ++fakeTokenIndex;
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment