Merge branch 'master' into websocket-token-auth

* master:
  Remove auth => session dependency

src/sidebar/app-controller.js

@@ -132,7 +132,7 @@ module.exports = function AppController(
     });
     drafts.discard();
     $scope.accountDialog.visible = false;
-    auth.logout();
+    session.logout();
   };
 
   $scope.clearSelection = function () {

src/sidebar/auth.js

 'use strict';
 
-var INITIAL_TOKEN = {
-  // The user ID which the current cached token is valid for
-  userid: undefined,
-  // Promise for the API token for 'userid'.
-  // This is initialized when fetchOrReuseToken() is called and
-  // reset when logging out via logout()
-  token: undefined,
-};
-
-/**
- * Fetches a new API token for the current logged-in user.
- *
- * @return {Promise} - A promise for a new JWT token.
- */
-function fetchToken($http, session, settings) {
-  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 = {
-    headers: headers,
-    // Skip JWT authorization for the token request itself.
-    skipAuthorization: true,
-  };
-
-  return $http.get(tokenUrl, config).then(function (response) {
-    return response.data;
-  });
-}
+var NULL_TOKEN = Promise.resolve(null);
 
 /**
  * Service for fetching and caching access tokens for the Hypothesis API.
  */
 // @ngInject
-function auth($http, flash, jwtHelper, session, settings) {
+function auth($http, jwtHelper, settings) {
 
-  var cachedToken = INITIAL_TOKEN;
+  var cachedToken = NULL_TOKEN;
 
   /**
-   * Fetches or returns a cached JWT API token for the current user.
+   * Fetch a new API token for the current logged-in user.
    *
-   * @return {Promise} - A promise for a JWT API token for the current
-   *                     user.
-   */
-  // @ngInject
-  function fetchOrReuseToken($http, jwtHelper, session, settings) {
-    function refreshToken() {
-      return fetchToken($http, session, settings).then(function (token) {
-        return token;
-      });
-    }
-
-    var userid;
-
-    return session.load()
-      .then(function (data) {
-        userid = data.userid;
-        if (userid === cachedToken.userid && cachedToken.token) {
-          return cachedToken.token;
-        } else {
-          cachedToken = {
-            userid: userid,
-            token: refreshToken(),
-          };
-          return cachedToken.token;
-        }
-      })
-      .then(function (token) {
-        if (jwtHelper.isTokenExpired(token)) {
-          cachedToken = {
-            userid: userid,
-            token: refreshToken(),
-          };
-          return cachedToken.token;
-        } else {
-          return token;
-        }
-      });
-  }
-
-  function clearCache() {
-    cachedToken = INITIAL_TOKEN;
-  }
-
-  /**
-   * Log out from the API and clear any cached tokens.
+   * The user is authenticated using their session cookie.
    *
-   * @return {Promise<void>} - A promise for when logout has completed.
+   * @return {Promise<string>} - A promise for a new JWT token.
    */
-  function logout() {
-    return session.logout({}).$promise
-      .then(function() {
-        clearCache();
-      })
-      .catch(function(err) {
-        flash.error('Log out failed!');
-        throw err;
-      });
+  function fetchToken() {
+    var tokenUrl = new URL('token', settings.apiUrl).href;
+    return $http.get(tokenUrl, {}).then(function (response) {
+      return response.data;
+    });
   }
 
   /**
-   * Return an access token for authenticating API requests.
+   * Fetch or return a cached JWT API token for the current user.
    *
-   * @return {Promise<string>}
+   * @return {Promise<string>} - A promise for a JWT API token for the current
+   *                             user.
    */
   function tokenGetter() {
-    return fetchOrReuseToken($http, jwtHelper, session, settings);
+    return cachedToken.then(function (token) {
+      if (!token || jwtHelper.isTokenExpired(token)) {
+        cachedToken = fetchToken();
+        return cachedToken;
+      } else {
+        return token;
+      }
+    });
+  }
+
+  function clearCache() {
+    cachedToken = NULL_TOKEN;
   }
 
   return {
     clearCache: clearCache,
     tokenGetter: tokenGetter,
-    logout: logout,
   };
 }
 

src/sidebar/session.js

@@ -48,7 +48,8 @@ function sessionActions(options) {
  *
  * @ngInject
  */
-function session($http, $resource, $rootScope, annotationUI, flash, raven, settings) {
+function session($http, $resource, $rootScope, annotationUI, auth,
+                 flash, raven, settings) {
   // Headers sent by every request made by the session service.
   var headers = {};
   var actions = sessionActions({
@@ -104,7 +105,7 @@ function session($http, $resource, $rootScope, annotationUI, flash, raven, setti
    *              the response, update() can be used to update the client
    *              when new state has been pushed to it by the server.
    */
-  resource.update = function (model) {
+  function update(model) {
     var prevSession = annotationUI.getState().session;
 
     var isInitialLoad = !prevSession.csrf;
@@ -128,6 +129,8 @@ function session($http, $resource, $rootScope, annotationUI, flash, raven, setti
     });
 
     if (userChanged) {
+      auth.clearCache();
+
       $rootScope.$broadcast(events.USER_CHANGED, {
         initialLoad: isInitialLoad,
         userid: model.userid,
@@ -151,7 +154,7 @@ function session($http, $resource, $rootScope, annotationUI, flash, raven, setti
 
     // Return the model
     return model;
-  };
+  }
 
   function process(data, headersGetter, status) {
     if (status < 200 || status >= 500) {
@@ -179,14 +182,23 @@ function session($http, $resource, $rootScope, annotationUI, flash, raven, setti
       }
     }
 
-    return resource.update(model);
+    return update(model);
+  }
+
+  function logout() {
+    return resource.logout().$promise.then(function () {
+      auth.clearCache();
+    }).catch(function (err) {
+      flash.error('Log out failed');
+      throw err;
+    });
   }
 
   return {
     dismissSidebarTutorial: resource.dismiss_sidebar_tutorial,
     load: resource.load,
     login: resource.login,
-    logout: resource.logout,
+    logout: logout,
 
     // For the moment, we continue to expose the session state as a property on
     // this service. In future, other services which access the session state
@@ -195,7 +207,7 @@ function session($http, $resource, $rootScope, annotationUI, flash, raven, setti
       return annotationUI.getState().session;
     },
 
-    update: resource.update,
+    update: update,
   };
 }
 

src/sidebar/test/app-controller-test.js

@@ -58,9 +58,7 @@ describe('AppController', function () {
       clearSelectedAnnotations: sandbox.spy(),
     };
 
-    fakeAuth = {
-      logout: sandbox.stub().returns(Promise.resolve()),
-    };
+    fakeAuth = {};
 
     fakeDrafts = {
       contains: sandbox.stub(),
@@ -88,6 +86,7 @@ describe('AppController', function () {
 
     fakeSession = {
       load: sandbox.stub().returns(Promise.resolve({userid: null})),
+      logout: sandbox.stub(),
     };
 
     fakeGroups = {focus: sandbox.spy()};
@@ -254,7 +253,13 @@ describe('AppController', function () {
     });
   });
 
-  describe('logout()', function () {
+  describe('#logout()', function () {
+    it('calls session.logout()', function () {
+      createController();
+      $scope.logout();
+      assert.called(fakeSession.logout);
+    });
+
     it('prompts the user if there are drafts', function () {
       fakeDrafts.count.returns(1);
       createController();

src/sidebar/test/auth-test.js

@@ -6,20 +6,15 @@ describe('auth', function () {
   var fakeHttp;
   var fakeJwtHelper;
   var fakeSettings;
-  var fakeSession;
   var fakeTokens = ['token-one', 'token-two'];
   var fakeTokenIndex;
 
   beforeEach(function () {
     fakeTokenIndex = 0;
     fakeHttp = {
-      defaults: {xsrfHeaderName: 'X-CSRF-Token'},
       get: sinon.spy(function (url, config) {
-        assert.equal(config.skipAuthorization, true);
         assert.equal(url, 'https://test.hypothes.is/api/token');
-        assert.deepEqual(config.headers, {
-          'X-CSRF-Token': fakeSession.state.csrf,
-        });
+        assert.deepEqual(config, {});
 
         var result = {status: 200, data: fakeTokens[fakeTokenIndex]};
         ++fakeTokenIndex;
@@ -27,29 +22,16 @@ describe('auth', function () {
       }),
     };
     fakeJwtHelper = {isTokenExpired: sinon.stub()};
-    fakeSession = {
-      load: sinon.spy(function () {
-        return Promise.resolve(fakeSession.state);
-      }),
-      logout: sinon.spy(function () {
-        return {$promise: Promise.resolve()};
-      }),
-      state: {
-        csrf: 'fake-csrf-token',
-      },
-    };
     fakeSettings = {
       apiUrl: 'https://test.hypothes.is/api/',
     };
   });
 
   function authFactory() {
-    var fakeFlash = { error: sinon.stub() };
-    return auth(fakeHttp, fakeFlash, fakeJwtHelper, fakeSession, fakeSettings);
+    return auth(fakeHttp, fakeJwtHelper, fakeSettings);
   }
 
   describe('#tokenGetter', function () {
-
     it('should fetch and return a new token', function () {
       var auth = authFactory();
       return auth.tokenGetter().then(function (token) {
@@ -80,11 +62,13 @@ describe('auth', function () {
         assert.equal(token, fakeTokens[1]);
       });
     });
+  });
 
-    it('should fetch a new token if the userid changes', function () {
+  describe('#clearCache', function () {
+    it('should remove existing cached tokens', function () {
       var auth = authFactory();
       return auth.tokenGetter().then(function () {
-        fakeSession.state.userid = 'new-user-id';
+        auth.clearCache();
         return auth.tokenGetter();
       }).then(function (token) {
         assert.calledTwice(fakeHttp.get);
@@ -92,13 +76,4 @@ describe('auth', function () {
       });
     });
   });
-
-  describe('#logout', function () {
-    it('should call session.logout', function () {
-      var auth = authFactory();
-      return auth.logout().then(function () {
-        assert.called(fakeSession.logout);
-      });
-    });
-  });
 });

src/sidebar/test/session-test.js

@@ -6,10 +6,11 @@ var events = require('../events');
 
 var mock = angular.mock;
 
-describe('h:session', function () {
+describe('session', function () {
   var $httpBackend;
   var $rootScope;
 
+  var fakeAuth;
   var fakeFlash;
   var fakeRaven;
   var sandbox;
@@ -32,6 +33,9 @@ describe('h:session', function () {
         state = session;
       },
     };
+    fakeAuth = {
+      clearCache: sandbox.spy(),
+    };
     fakeFlash = {error: sandbox.spy()};
     fakeRaven = {
       setUserInfo: sandbox.spy(),
@@ -39,6 +43,7 @@ describe('h:session', function () {
 
     mock.module('h', {
       annotationUI: fakeAnnotationUI,
+      auth: fakeAuth,
       flash: fakeFlash,
       raven: fakeRaven,
       settings: {
@@ -237,6 +242,11 @@ describe('h:session', function () {
       assert.calledOnce(userChangeCallback);
     });
 
+    it('clears the API token cache when the user changes', function () {
+      session.update({userid: 'different-user', csrf: 'dummytoken'});
+      assert.called(fakeAuth.clearCache);
+    });
+
     it('updates the user ID for Sentry error reports', function () {
       session.update({
         userid: 'anne',
@@ -256,4 +266,29 @@ describe('h:session', function () {
       $httpBackend.flush();
     });
   });
+
+  describe('#logout()', function () {
+    beforeEach(function () {
+      var url = 'https://test.hypothes.is/root/app?__formid__=logout';
+      $httpBackend.expectPOST(url).respond(200, {
+        model: {
+          userid: 'logged-out-id',
+        },
+      });
+    });
+
+    it('logs the user out on the service and updates the session', function () {
+      session.logout().then(function () {
+        assert.equal(session.state.userid, 'logged-out-id');
+      });
+      $httpBackend.flush();
+    });
+
+    it('clears the API access token cache', function () {
+      session.logout().then(function () {
+        assert.called(fakeAuth.clearCache);
+      });
+      $httpBackend.flush();
+    });
+  });
 });