Skip to content

C
coopwire-hypothesis
Commit 11ac0e25 authored by Robert Knight's avatar Robert Knight
Browse files
Options

Replace Angular $http service with `window.fetch` in OAuthClient 

As part of the migration away from AngularJS, replace `$http` with
`fetch` in the OAuthClient class which handles interactions with h's
OAuth endpoints.

For testing use the fetch-mock library that is already used in h's
frontend tests. The current version (v7) has dependencies that are
written in ES6 and not transpiled, and so don't work in PhantomJS [1]. As a
workaround, use v6 of the library for the time being.

This is part of #974.

[1] We _do_ transpile our own code from ES6 -> ES5 but that transform is
    not applied to dependencies.
parent cdc6a27f
Show whitespace changes
Inline Side-by-side
Showing with 122 additions and 78 deletions
package.json
View file @ 11ac0e25
......@@ -53,6 +53,7 @@
"exorcist": "^1.0.1",
"express": "^4.14.1",
"extend": "^3.0.2",
"fetch-mock": "6",
"gulp": "^4.0.0",
"gulp-batch": "^1.0.5",
"gulp-changed": "^3.2.0",
......
src/sidebar/services/oauth-auth.js
View file @ 11ac0e25
......@@ -153,7 +153,7 @@ function auth(
return Promise.resolve(client);
}
return apiRoutes.links().then(links => {
client = new OAuthClient($http, {
client = new OAuthClient({
clientId: settings.oauthClientId,
authorizationEndpoint: links['oauth.authorize'],
revokeEndpoint: links['oauth.revoke'],
......
src/sidebar/services/test/oauth-auth-test.js
View file @ 11ac0e25
......@@ -88,8 +88,7 @@ describe('sidebar.oauth-auth', function() {
authorize: sinon.stub().returns(Promise.resolve(null)),
};
FakeOAuthClient = ($http, config) => {
fakeClient.$http = $http;
FakeOAuthClient = config => {
fakeClient.config = config;
return fakeClient;
};
......@@ -123,7 +122,6 @@ describe('sidebar.oauth-auth', function() {
it('configures an OAuthClient correctly', () => {
// Call a method which will trigger construction of the `OAuthClient`.
return auth.tokenGetter().then(() => {
assert.equal(fakeClient.$http, fakeHttp);
assert.deepEqual(fakeClient.config, {
clientId: 'the-client-id',
tokenEndpoint: 'https://hypothes.is/api/token',
......
src/sidebar/util/oauth-client.js
View file @ 11ac0e25
......@@ -15,12 +15,12 @@ const random = require('./random');
/**
* Return a new TokenInfo object from the given tokenUrl endpoint response.
* @param {Object} response - The HTTP response from a POST to the tokenUrl
* endpoint (an Angular $http response object).
* @returns {TokenInfo}
* @param {Response} response - The HTTP response from a POST to the tokenUrl
* endpoint.
* @returns {Promise<TokenInfo>}
*/
function tokenInfoFrom(response) {
const data = response.data;
return response.json().then(data => {
return {
accessToken: data.access_token,
......@@ -31,6 +31,7 @@ function tokenInfoFrom(response) {
refreshToken: data.refresh_token,
};
});
}
/**
......@@ -62,12 +63,9 @@ class OAuthClient {
/**
* Create a new OAuthClient
*
* @param {Object} $http - HTTP client
* @param {Config} config
*/
constructor($http, config) {
this.$http = $http;
constructor(config) {
this.clientId = config.clientId;
this.tokenEndpoint = config.tokenEndpoint;
this.authorizationEndpoint = config.authorizationEndpoint;
......@@ -208,11 +206,19 @@ class OAuthClient {
* @param {Object} data - Parameter dictionary
*/
_formPost(url, data) {
data = queryString.stringify(data);
const requestConfig = {
headers: { 'Content-Type': 'application/x-www-form-urlencoded' },
// The `fetch` API has native support for sending form data by setting
// the `body` option to a `FormData` instance. We are not using that here
// because our test environment has very limited `FormData` support and it
// is simpler just to format the data manually.
const formData = queryString.stringify(data);
const headers = {
'Content-Type': 'application/x-www-form-urlencoded',
};
return this.$http.post(url, data, requestConfig);
return fetch(url, {
method: 'POST',
headers,
body: formData,
});
}
/**
......
src/sidebar/util/test/oauth-client-test.js
View file @ 11ac0e25
'use strict';
const fetchMock = require('fetch-mock');
const { stringify } = require('query-string');
const sinon = require('sinon');
......@@ -8,13 +9,10 @@ const FakeWindow = require('./fake-window');
const fixtures = {
tokenResponse: {
status: 200,
data: {
access_token: 'access-token',
refresh_token: 'refresh-token',
expires_in: 360,
},
},
parsedToken: {
accessToken: 'access-token',
......@@ -24,14 +22,9 @@ const fixtures = {
// `Date.now() === 0`.
expiresAt: 350000,
},
formPostParams: {
headers: { 'Content-Type': 'application/x-www-form-urlencoded' },
},
};
describe('sidebar.util.oauth-client', () => {
let fakeHttp;
let client;
let clock;
const config = {
......@@ -43,42 +36,59 @@ describe('sidebar.util.oauth-client', () => {
};
beforeEach(() => {
fakeHttp = {
post: sinon.stub().returns(Promise.resolve({ status: 200 })),
};
clock = sinon.useFakeTimers();
fetchMock.catch(() => {
throw new Error('Unexpected fetch call');
});
client = new OAuthClient(fakeHttp, config);
client = new OAuthClient(config);
});
afterEach(() => {
fetchMock.restore();
clock.restore();
});
/**
* Check that a POST request was made with the given URL-encoded form data.
*
* @param {string} expectedBody
*/
function assertFormPost(expectedBody) {
assert.isTrue(fetchMock.called());
const [, options] = fetchMock.lastCall();
assert.deepEqual(options, {
body: expectedBody,
method: 'POST',
headers: { 'Content-Type': 'application/x-www-form-urlencoded' },
});
}
describe('#exchangeAuthCode', () => {
it('makes a POST request to the authorization endpoint', () => {
fakeHttp.post.returns(Promise.resolve(fixtures.tokenResponse));
it('makes a POST request to the token endpoint', () => {
fetchMock.post(config.tokenEndpoint, {
body: fixtures.tokenResponse,
});
return client.exchangeAuthCode('letmein').then(() => {
const expectedBody =
'client_id=1234-5678&code=letmein&grant_type=authorization_code';
assert.calledWith(
fakeHttp.post,
'https://annota.te/api/token',
expectedBody,
fixtures.formPostParams
);
assertFormPost(expectedBody);
});
});
it('resolves with the parsed token data', () => {
fakeHttp.post.returns(Promise.resolve(fixtures.tokenResponse));
fetchMock.post(config.tokenEndpoint, {
body: fixtures.tokenResponse,
});
return client.exchangeAuthCode('letmein').then(token => {
assert.deepEqual(token, fixtures.parsedToken);
});
});
it('rejects if the request fails', () => {
fakeHttp.post.returns(Promise.resolve({ status: 400 }));
fetchMock.post(config.tokenEndpoint, {
status: 400,
});
return client.exchangeAuthCode('unknowncode').catch(err => {
assert.equal(err.message, 'Authorization code exchange failed');
});
......@@ -87,22 +97,21 @@ describe('sidebar.util.oauth-client', () => {
describe('#exchangeGrantToken', () => {
it('makes a POST request to the token endpoint', () => {
fakeHttp.post.returns(Promise.resolve(fixtures.tokenResponse));
fetchMock.post(config.tokenEndpoint, {
body: fixtures.tokenResponse,
});
return client.exchangeGrantToken('letmein').then(() => {
const expectedBody =
'assertion=letmein' +
'&grant_type=urn%3Aietf%3Aparams%3Aoauth%3Agrant-type%3Ajwt-bearer';
assert.calledWith(
fakeHttp.post,
'https://annota.te/api/token',
expectedBody,
fixtures.formPostParams
);
assertFormPost(expectedBody);
});
});
it('resolves with the parsed token data', () => {
fakeHttp.post.returns(Promise.resolve(fixtures.tokenResponse));
fetchMock.post(config.tokenEndpoint, {
body: fixtures.tokenResponse,
});
return client.exchangeGrantToken('letmein').then(token => {
assert.deepEqual(token, fixtures.parsedToken);
......@@ -110,7 +119,9 @@ describe('sidebar.util.oauth-client', () => {
});
it('rejects if the request fails', () => {
fakeHttp.post.returns(Promise.resolve({ status: 400 }));
fetchMock.post(config.tokenEndpoint, {
status: 400,
});
return client.exchangeGrantToken('unknowntoken').catch(err => {
assert.equal(err.message, 'Failed to retrieve access token');
});
......@@ -119,23 +130,21 @@ describe('sidebar.util.oauth-client', () => {
describe('#refreshToken', () => {
it('makes a POST request to the token endpoint', () => {
fakeHttp.post.returns(Promise.resolve(fixtures.tokenResponse));
fetchMock.post(config.tokenEndpoint, {
body: fixtures.tokenResponse,
});
return client.refreshToken('valid-refresh-token').then(() => {
const expectedBody =
'grant_type=refresh_token&refresh_token=valid-refresh-token';
assert.calledWith(
fakeHttp.post,
'https://annota.te/api/token',
expectedBody,
fixtures.formPostParams
);
assertFormPost(expectedBody);
});
});
it('resolves with the parsed token data', () => {
fakeHttp.post.returns(Promise.resolve(fixtures.tokenResponse));
fetchMock.post(config.tokenEndpoint, {
body: fixtures.tokenResponse,
});
return client.refreshToken('valid-refresh-token').then(token => {
assert.deepEqual(token, fixtures.parsedToken);
......@@ -143,7 +152,7 @@ describe('sidebar.util.oauth-client', () => {
});
it('rejects if the request fails', () => {
fakeHttp.post.returns(Promise.resolve({ status: 400 }));
fetchMock.post(config.tokenEndpoint, { status: 400 });
return client.refreshToken('invalid-token').catch(err => {
assert.equal(err.message, 'Failed to refresh access token');
});
......@@ -152,26 +161,23 @@ describe('sidebar.util.oauth-client', () => {
describe('#revokeToken', () => {
it('makes a POST request to the revoke endpoint', () => {
fakeHttp.post.returns(Promise.resolve(fixtures.tokenResponse));
fetchMock.post(config.revokeEndpoint, {
body: fixtures.tokenResponse,
});
return client.revokeToken('valid-access-token').then(() => {
const expectedBody = 'token=valid-access-token';
assert.calledWith(
fakeHttp.post,
'https://annota.te/oauth/revoke',
expectedBody,
fixtures.formPostParams
);
assertFormPost(expectedBody);
});
});
it('resolves if the request succeeds', () => {
fakeHttp.post.returns(Promise.resolve({ status: 200 }));
fetchMock.post(config.revokeEndpoint, { status: 200 });
return client.revokeToken('valid-access-token');
});
it('rejects if the request fails', () => {
fakeHttp.post.returns(Promise.resolve({ status: 400 }));
fetchMock.post(config.revokeEndpoint, { status: 400 });
return client.revokeToken('invalid-token').catch(err => {
assert.equal(err.message, 'failed');
});
......
yarn.lock
View file @ 11ac0e25
......@@ -1641,6 +1641,15 @@ babel-plugin-transform-strict-mode@^6.24.1:
babel-runtime "^6.22.0"
babel-types "^6.24.1"
babel-polyfill@^6.26.0:
version "6.26.0"
resolved "https://registry.yarnpkg.com/babel-polyfill/-/babel-polyfill-6.26.0.tgz#379937abc67d7895970adc621f284cd966cf2153"
integrity sha1-N5k3q8Z9eJWXCtxiHyhM2WbPIVM=
dependencies:
babel-runtime "^6.26.0"
core-js "^2.5.0"
regenerator-runtime "^0.10.5"
babel-preset-env@^1.7.0:
version "1.7.0"
resolved "https://registry.yarnpkg.com/babel-preset-env/-/babel-preset-env-1.7.0.tgz#dea79fa4ebeb883cd35dab07e260c1c9c04df77a"
......@@ -2637,7 +2646,7 @@ copy-props@^2.0.1:
each-props "^1.3.0"
is-plain-object "^2.0.1"
core-js@^2.2.0, core-js@^2.4.0, core-js@^2.5.7:
core-js@^2.2.0, core-js@^2.4.0, core-js@^2.5.0, core-js@^2.5.7:
version "2.6.5"
resolved "https://registry.yarnpkg.com/core-js/-/core-js-2.6.5.tgz#44bc8d249e7fb2ff5d00e0341a7ffb94fbf67895"
integrity sha512-klh/kDpwX8hryYL14M9w/xei6vrv6sE8gTHDG7/T/+SEovB/G4ejwcfE/CBzO6Edsu+OETZMZ3wcX/EjUkrl5A==
......@@ -3790,6 +3799,15 @@ fd-slicer@~1.0.1:
dependencies:
pend "~1.2.0"
fetch-mock@6:
version "6.5.2"
resolved "https://registry.yarnpkg.com/fetch-mock/-/fetch-mock-6.5.2.tgz#b3842b305c13ea0f81c85919cfaa7de387adfa3e"
integrity sha512-EIvbpCLBTYyDLu4HJiqD7wC8psDwTUaPaWXNKZbhNO/peUYKiNp5PkZGKRJtnTxaPQu71ivqafvjpM7aL+MofQ==
dependencies:
babel-polyfill "^6.26.0"
glob-to-regexp "^0.4.0"
path-to-regexp "^2.2.1"
figures@^2.0.0:
version "2.0.0"
resolved "https://registry.yarnpkg.com/figures/-/figures-2.0.0.tgz#3ab1a2d2a62c8bfb431a0c94cb797a2fce27c962"
......@@ -4174,6 +4192,11 @@ glob-stream@^6.1.0:
to-absolute-glob "^2.0.0"
unique-stream "^2.0.2"
glob-to-regexp@^0.4.0:
version "0.4.0"
resolved "https://registry.yarnpkg.com/glob-to-regexp/-/glob-to-regexp-0.4.0.tgz#49bd677b1671022bd10921c3788f23cdebf9c7e6"
integrity sha512-fyPCII4vn9Gvjq2U/oDAfP433aiE64cyP/CJjRJcpVGjqqNdioUYn9+r0cSzT1XPwmGAHuTT7iv+rQT8u/YHKQ==
glob-watcher@^5.0.0:
version "5.0.3"
resolved "https://registry.yarnpkg.com/glob-watcher/-/glob-watcher-5.0.3.tgz#88a8abf1c4d131eb93928994bc4a593c2e5dd626"
......@@ -7018,6 +7041,11 @@ path-to-regexp@^1.7.0:
dependencies:
isarray "0.0.1"
path-to-regexp@^2.2.1:
version "2.4.0"
resolved "https://registry.yarnpkg.com/path-to-regexp/-/path-to-regexp-2.4.0.tgz#35ce7f333d5616f1c1e1bfe266c3aba2e5b2e704"
integrity sha512-G6zHoVqC6GGTQkZwF4lkuEyMbVOjoBKAEybQUypI1WTkqinCOrq2x6U2+phkJ1XsEMTy4LjtwPI7HW+NVrRR2w==
path-type@^1.0.0:
version "1.1.0"
resolved "https://registry.yarnpkg.com/path-type/-/path-type-1.1.0.tgz#59c44f7ee491da704da415da5a4070ba4f8fe441"
......@@ -7541,6 +7569,11 @@ regenerate@^1.2.1, regenerate@^1.4.0:
resolved "https://registry.yarnpkg.com/regenerate/-/regenerate-1.4.0.tgz#4a856ec4b56e4077c557589cae85e7a4c8869a11"
integrity sha512-1G6jJVDWrt0rK99kBjvEtziZNCICAuvIPkSiUFIQxVP06RCVpq3dmDo2oi6ABpYaDYaTRr67BEhL8r1wgEZZKg==
regenerator-runtime@^0.10.5:
version "0.10.5"
resolved "https://registry.yarnpkg.com/regenerator-runtime/-/regenerator-runtime-0.10.5.tgz#336c3efc1220adcedda2c9fab67b5a7955a33658"
integrity sha1-M2w+/BIgrc7dosn6tntaeVWjNlg=
regenerator-runtime@^0.11.0:
version "0.11.1"
resolved "https://registry.yarnpkg.com/regenerator-runtime/-/regenerator-runtime-0.11.1.tgz#be05ad7f9bf7d22e056f9726cee5017fbf19e2e9"
......
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