Commit e05ba783 authored by Robert Knight's avatar Robert Knight

Get OAuth authorization and revocation endpoints from /api/links

Use the "apiRoutes" service to get the URLs of the `/oauth/authorize`
and `/oauth/revoke` endpoints from `/api/links` instead of from
"oauthAuthorizeUrl" and "oauthRevokeUrl" in app settings.

This makes the client's behavior more consistent in terms of getting all
links to pages within the service from the `/api/links` route.

It also paves the way to enabling the client to use multiple annotation
services, each of which is defined by a single entrypoint (the `/api`
route) from which all other API routes and links into the service are
obtained.
parent ba2de7c8
...@@ -28,7 +28,8 @@ var serviceConfig = require('./service-config'); ...@@ -28,7 +28,8 @@ var serviceConfig = require('./service-config');
* an opaque access token. * an opaque access token.
*/ */
// @ngInject // @ngInject
function auth($http, $rootScope, $window, flash, localStorage, random, settings) { function auth($http, $rootScope, $window,
apiRoutes, flash, localStorage, random, settings) {
/** /**
* Authorization code from auth popup window. * Authorization code from auth popup window.
...@@ -336,7 +337,8 @@ function auth($http, $rootScope, $window, flash, localStorage, random, settings) ...@@ -336,7 +337,8 @@ function auth($http, $rootScope, $window, flash, localStorage, random, settings)
var left = $window.screen.width / 2 - width / 2; var left = $window.screen.width / 2 - width / 2;
var top = $window.screen.height /2 - height / 2; var top = $window.screen.height /2 - height / 2;
var authUrl = settings.oauthAuthorizeUrl; return apiRoutes.links().then(links => {
var authUrl = links['oauth.authorize'];
authUrl += '?' + queryString.stringify({ authUrl += '?' + queryString.stringify({
client_id: settings.oauthClientId, client_id: settings.oauthClientId,
origin: $window.location.origin, origin: $window.location.origin,
...@@ -352,7 +354,8 @@ function auth($http, $rootScope, $window, flash, localStorage, random, settings) ...@@ -352,7 +354,8 @@ function auth($http, $rootScope, $window, flash, localStorage, random, settings)
}).replace(/&/g, ','); }).replace(/&/g, ',');
$window.open(authUrl, 'Login to Hypothesis', authWindowSettings); $window.open(authUrl, 'Login to Hypothesis', authWindowSettings);
return authResponse.then((resp) => { return authResponse;
}).then((resp) => {
// Save the auth code. It will be exchanged for an access token when the // Save the auth code. It will be exchanged for an access token when the
// next API request is made. // next API request is made.
authCode = resp.code; authCode = resp.code;
...@@ -366,8 +369,10 @@ function auth($http, $rootScope, $window, flash, localStorage, random, settings) ...@@ -366,8 +369,10 @@ function auth($http, $rootScope, $window, flash, localStorage, random, settings)
* This revokes and then forgets any OAuth credentials that the user has. * This revokes and then forgets any OAuth credentials that the user has.
*/ */
function logout() { function logout() {
return tokenInfoPromise.then(token => { return Promise.all([tokenInfoPromise, apiRoutes.links()])
return formPost(settings.oauthRevokeUrl, { .then(([token, links]) => {
var revokeUrl = links['oauth.revoke'];
return formPost(revokeUrl, {
token: token.accessToken, token: token.accessToken,
}); });
}).then(() => { }).then(() => {
......
...@@ -56,6 +56,7 @@ describe('sidebar.oauth-auth', function () { ...@@ -56,6 +56,7 @@ describe('sidebar.oauth-auth', function () {
var $rootScope; var $rootScope;
var auth; var auth;
var nowStub; var nowStub;
var fakeApiRoutes;
var fakeHttp; var fakeHttp;
var fakeFlash; var fakeFlash;
var fakeLocalStorage; var fakeLocalStorage;
...@@ -104,6 +105,13 @@ describe('sidebar.oauth-auth', function () { ...@@ -104,6 +105,13 @@ describe('sidebar.oauth-auth', function () {
post: sinon.stub().returns(successfulFirstAccessTokenPromise), post: sinon.stub().returns(successfulFirstAccessTokenPromise),
}; };
fakeApiRoutes = {
links: sinon.stub().returns(Promise.resolve({
'oauth.authorize': 'https://hypothes.is/oauth/authorize/',
'oauth.revoke': 'https://hypothes.is/oauth/revoke/',
})),
};
fakeFlash = { fakeFlash = {
error: sinon.stub(), error: sinon.stub(),
}; };
...@@ -114,9 +122,7 @@ describe('sidebar.oauth-auth', function () { ...@@ -114,9 +122,7 @@ describe('sidebar.oauth-auth', function () {
fakeSettings = { fakeSettings = {
apiUrl: 'https://hypothes.is/api/', apiUrl: 'https://hypothes.is/api/',
oauthAuthorizeUrl: 'https://hypothes.is/oauth/authorize/',
oauthClientId: 'the-client-id', oauthClientId: 'the-client-id',
oauthRevokeUrl: 'https://hypothes.is/oauth/revoke/',
services: [{ services: [{
authority: 'publisher.org', authority: 'publisher.org',
grantToken: 'a.jwt.token', grantToken: 'a.jwt.token',
...@@ -134,6 +140,7 @@ describe('sidebar.oauth-auth', function () { ...@@ -134,6 +140,7 @@ describe('sidebar.oauth-auth', function () {
angular.mock.module('app', { angular.mock.module('app', {
$http: fakeHttp, $http: fakeHttp,
$window: fakeWindow, $window: fakeWindow,
apiRoutes: fakeApiRoutes,
flash: fakeFlash, flash: fakeFlash,
localStorage: fakeLocalStorage, localStorage: fakeLocalStorage,
random: fakeRandom, random: fakeRandom,
...@@ -500,6 +507,8 @@ describe('sidebar.oauth-auth', function () { ...@@ -500,6 +507,8 @@ describe('sidebar.oauth-auth', function () {
it('opens the auth endpoint in a popup window', () => { it('opens the auth endpoint in a popup window', () => {
auth.login(); auth.login();
return fakeApiRoutes.links().then((links) => {
var authUrl = links['oauth.authorize'];
var params = { var params = {
client_id: fakeSettings.oauthClientId, client_id: fakeSettings.oauthClientId,
origin: 'client.hypothes.is', origin: 'client.hypothes.is',
...@@ -507,7 +516,7 @@ describe('sidebar.oauth-auth', function () { ...@@ -507,7 +516,7 @@ describe('sidebar.oauth-auth', function () {
response_type: 'code', response_type: 'code',
state: 'notrandom', state: 'notrandom',
}; };
var expectedAuthUrl = `${fakeSettings.oauthAuthorizeUrl}?${stringify(params)}`; var expectedAuthUrl = `${authUrl}?${stringify(params)}`;
assert.calledWith( assert.calledWith(
fakeWindow.open, fakeWindow.open,
expectedAuthUrl, expectedAuthUrl,
...@@ -515,6 +524,7 @@ describe('sidebar.oauth-auth', function () { ...@@ -515,6 +524,7 @@ describe('sidebar.oauth-auth', function () {
'height=400,left=312,top=184,width=400' 'height=400,left=312,top=184,width=400'
); );
}); });
});
it('ignores auth responses if the state does not match', () => { it('ignores auth responses if the state does not match', () => {
var loggedIn = false; var loggedIn = false;
......
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