Merge pull request #501 from hypothesis/oauth-logout

Implement logout when using OAuth

src/sidebar/oauth-auth.js

@@ -33,6 +33,11 @@ function auth($http, $window, flash, localStorage, random, settings) {
   var accessTokenPromise;
   var tokenUrl = resolve('token', settings.apiUrl);
 
+  /**
+   * Timer ID of the current access token refresh timer.
+   */
+  var refreshTimer;
+
   /**
    * Show an error message telling the user that the access token has expired.
    */
@@ -76,14 +81,12 @@ function auth($http, $window, flash, localStorage, random, settings) {
     };
   }
 
-  // Post the given data to the tokenUrl endpoint as a form submission.
-  // Return a Promise for the access token response.
-  function postToTokenUrl(data) {
+  function formPost(url, data) {
     data = queryString.stringify(data);
     var requestConfig = {
       headers: {'Content-Type': 'application/x-www-form-urlencoded'},
     };
-    return $http.post(tokenUrl, data, requestConfig);
+    return $http.post(url, data, requestConfig);
   }
 
   function grantTokenFromHostPage() {
@@ -146,7 +149,7 @@ function auth($http, $window, flash, localStorage, random, settings) {
       grant_type: 'urn:ietf:params:oauth:grant-type:jwt-bearer',
       assertion: grantToken,
     };
-    return postToTokenUrl(data).then(function (response) {
+    return formPost(tokenUrl, data).then(function (response) {
       if (response.status !== 200) {
         throw new Error('Failed to retrieve access token');
       }
@@ -164,7 +167,7 @@ function auth($http, $window, flash, localStorage, random, settings) {
       grant_type: 'authorization_code',
       code,
     };
-    return postToTokenUrl(data).then((response) => {
+    return formPost(tokenUrl, data).then((response) => {
       if (response.status !== 200) {
         throw new Error('Authorization code exchange failed');
       }
@@ -182,7 +185,7 @@ function auth($http, $window, flash, localStorage, random, settings) {
    */
   function refreshAccessToken(refreshToken, options) {
     var data = { grant_type: 'refresh_token', refresh_token: refreshToken };
-    return postToTokenUrl(data).then((response) => {
+    return formPost(tokenUrl, data).then((response) => {
       var tokenInfo = tokenInfoFrom(response);
 
       if (options.persist) {
@@ -227,7 +230,7 @@ function auth($http, $window, flash, localStorage, random, settings) {
       }
     }
 
-    window.setTimeout(refreshAccessTokenIfNearExpiry, delay);
+    refreshTimer = $window.setTimeout(refreshAccessTokenIfNearExpiry, delay);
   }
 
   /**
@@ -279,11 +282,15 @@ function auth($http, $window, flash, localStorage, random, settings) {
     return accessTokenPromise;
   }
 
-  // clearCache() isn't implemented (or needed) yet for OAuth.
-  // In the future, for example when OAuth-authenticated users can login and
-  // logout of the client, this clearCache() will need to clear the access
-  // token and cancel any scheduled refresh token requests.
+  /**
+   * Forget any cached credentials.
+   */
   function clearCache() {
+    // Once cookie auth has been removed, the `clearCache` method can be removed
+    // from the public API of this service in favor of `logout`.
+    accessTokenPromise = Promise.resolve(null);
+    localStorage.removeItem(storageKey());
+    $window.clearTimeout(refreshTimer);
   }
 
   /**
@@ -352,9 +359,25 @@ function auth($http, $window, flash, localStorage, random, settings) {
     });
   }
 
+  /**
+   * Log out of the service (in the client only).
+   *
+   * This revokes and then forgets any OAuth credentials that the user has.
+   */
+  function logout() {
+    return accessTokenPromise.then(accessToken => {
+      return formPost(settings.oauthRevokeUrl, {
+        token: accessToken,
+      });
+    }).then(() => {
+      clearCache();
+    });
+  }
+
   return {
     clearCache,
     login,
+    logout,
     tokenGetter,
   };
 }

src/sidebar/session.js

@@ -150,7 +150,12 @@ function session($http, $q, $resource, $rootScope, analytics, annotationUI, auth
     lastLoadTime = Date.now();
 
     if (userChanged) {
-      if (!getAuthority()) {
+      if (!auth.login) {
+        // When using cookie-based auth, notify the auth service that the current
+        // login has changed and API tokens need to be invalidated.
+        //
+        // This is not needed for OAuth-based auth because all login/logout
+        // activities happen through the auth service itself.
         auth.clearCache();
       }
 
@@ -208,10 +213,32 @@ function session($http, $q, $resource, $rootScope, analytics, annotationUI, auth
     return update(model);
   }
 
+  /**
+   * Log the user out of the current session.
+   */
   function logout() {
-    return resource.logout().$promise.then(function () {
-      auth.clearCache();
-    }).catch(function (err) {
+    var loggedOut;
+
+    if (auth.logout) {
+      loggedOut = auth.logout().then(() => {
+        // When using OAuth, we have to explicitly re-fetch the logged-out
+        // user's profile.
+        // When using cookie-based auth, `resource.logout()` handles this
+        // automatically.
+        return reload();
+      });
+    } else {
+      loggedOut = resource.logout().$promise.then(() => {
+        // When using cookie-based auth, notify the auth service that the current
+        // login has changed and API tokens need to be invalidated.
+        //
+        // This is not needed for OAuth-based auth because all login/logout
+        // activities happen through the auth service itself.
+        auth.clearCache();
+      });
+    }
+
+    return loggedOut.catch(function (err) {
       flash.error('Log out failed');
       analytics.track(analytics.events.LOGOUT_FAILURE);
       return $q.reject(new Error(err));

src/sidebar/test/oauth-auth-test.js

@@ -20,6 +20,9 @@ class FakeWindow {
     };
 
     this.open = sinon.stub();
+
+    this.setTimeout = window.setTimeout.bind(window);
+    this.clearTimeout = window.clearTimeout.bind(window);
   }
 
   addEventListener(event, callback) {
@@ -55,12 +58,29 @@ describe('sidebar.oauth-auth', function () {
   var clock;
   var successfulFirstAccessTokenPromise;
 
+  /**
+   * Login and retrieve an auth code.
+   */
+  function login() {
+    var loggedIn = auth.login();
+    fakeWindow.sendMessage({
+      type: 'authorization_response',
+      code: 'acode',
+      state: 'notrandom',
+    });
+    return loggedIn;
+  }
+
   before(() => {
     angular.module('app', [])
       .service('auth', require('../oauth-auth'));
   });
 
   beforeEach(function () {
+    // Setup fake clock. This has to be done before setting up the `window`
+    // fake which makes use of timers.
+    clock = sinon.useFakeTimers();
+
     nowStub = sinon.stub(window.performance, 'now');
     nowStub.returns(300);
 
@@ -89,6 +109,7 @@ describe('sidebar.oauth-auth', function () {
       apiUrl: 'https://hypothes.is/api/',
       oauthAuthorizeUrl: 'https://hypothes.is/oauth/authorize/',
       oauthClientId: 'the-client-id',
+      oauthRevokeUrl: 'https://hypothes.is/oauth/revoke/',
       services: [{
         authority: 'publisher.org',
         grantToken: 'a.jwt.token',
@@ -100,6 +121,7 @@ describe('sidebar.oauth-auth', function () {
     fakeLocalStorage = {
       getObject: sinon.stub().returns(null),
       setObject: sinon.stub(),
+      removeItem: sinon.stub(),
     };
 
     angular.mock.module('app', {
@@ -114,8 +136,6 @@ describe('sidebar.oauth-auth', function () {
     angular.mock.inject((_auth_) => {
       auth = _auth_;
     });
-
-    clock = sinon.useFakeTimers();
   });
 
   afterEach(function () {
@@ -319,19 +339,6 @@ describe('sidebar.oauth-auth', function () {
   });
 
   describe('persistence of tokens to storage', () => {
-    /**
-     * Login and retrieve an auth code.
-     */
-    function login() {
-      var loggedIn = auth.login();
-      fakeWindow.sendMessage({
-        type: 'authorization_response',
-        code: 'acode',
-        state: 'notrandom',
-      });
-      return loggedIn;
-    }
-
     beforeEach(() => {
       fakeSettings.services = [];
     });
@@ -548,6 +555,45 @@ describe('sidebar.oauth-auth', function () {
     });
   });
 
+  describe('#logout', () => {
+    beforeEach(() => {
+      // logout() is only currently used when using the public
+      // Hypothesis service.
+      fakeSettings.services = [];
+
+      return login().then(() => {
+        return auth.tokenGetter();
+      }).then(token => {
+        assert.notEqual(token, null);
+
+        fakeHttp.post.reset();
+      });
+    });
+
+    it('forgets access tokens', () => {
+      return auth.logout().then(() => {
+        return auth.tokenGetter();
+      }).then(token => {
+        assert.equal(token, null);
+      });
+    });
+
+    it('removes cached tokens', () => {
+      return auth.logout().then(() => {
+        assert.calledWith(fakeLocalStorage.removeItem, TOKEN_KEY);
+      });
+    });
+
+    it('revokes tokens', () => {
+      return auth.logout().then(() => {
+        var expectedBody = 'token=firstAccessToken';
+        assert.calledWith(fakeHttp.post, 'https://hypothes.is/oauth/revoke/', expectedBody, {
+          headers: {'Content-Type': 'application/x-www-form-urlencoded'},
+        });
+      });
+    });
+  });
+
   // Advance time forward so that any current access tokens will have expired.
   function expireAccessToken () {
     clock.tick(DEFAULT_TOKEN_EXPIRES_IN_SECS * 1000);

src/sidebar/test/session-test.js

@@ -295,11 +295,8 @@ describe('session', function () {
       });
     });
 
-    it('does not clear the access token when the host page provides a grant token', function () {
-      fakeServiceConfig.returns({
-        authority: 'publisher.org',
-        grantToken: 'a.jwt.token',
-      });
+    it('does not clear the access token when using OAuth-based authorization', function () {
+      fakeAuth.login = Promise.resolve();
 
       session.update({userid: 'different-user', csrf: 'dummytoken'});
 
@@ -349,46 +346,84 @@ describe('session', function () {
     });
   });
 
-  describe('#logout()', function () {
-    var postExpectation;
-    beforeEach(function () {
-      var logoutUrl = 'https://test.hypothes.is/root/app?__formid__=logout';
-      postExpectation = $httpBackend.expectPOST(logoutUrl).respond(200, {
-        model: {
-          userid: 'logged-out-id',
-        },
+  describe('#logout', function () {
+    context('when using cookie auth', () => {
+      var postExpectation;
+      beforeEach(function () {
+        var logoutUrl = 'https://test.hypothes.is/root/app?__formid__=logout';
+        postExpectation = $httpBackend.expectPOST(logoutUrl).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');
+      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();
       });
-      $httpBackend.flush();
-    });
 
-    it('clears the API access token cache', function () {
-      session.logout().then(function () {
-        assert.called(fakeAuth.clearCache);
+      it('clears the API access token cache', function () {
+        session.logout().then(function () {
+          assert.called(fakeAuth.clearCache);
+        });
+        $httpBackend.flush();
       });
-      $httpBackend.flush();
-    });
 
-    it('tracks successful logout actions in analytics', function () {
-      session.logout().then(function () {
-        assert.calledWith(fakeAnalytics.track, fakeAnalytics.events.LOGOUT_SUCCESS);
+      it('tracks successful logout actions in analytics', function () {
+        session.logout().then(function () {
+          assert.calledWith(fakeAnalytics.track, fakeAnalytics.events.LOGOUT_SUCCESS);
+        });
+        $httpBackend.flush();
+      });
+
+      it('tracks unsuccessful logout actions in analytics', function () {
+        postExpectation.respond(500);
+
+        session.logout().catch(function(){
+          assert.calledWith(fakeAnalytics.track, fakeAnalytics.events.LOGOUT_FAILURE);
+        });
+
+        $httpBackend.flush();
       });
-      $httpBackend.flush();
     });
 
-    it('tracks unsuccessful logout actions in analytics', function () {
-      postExpectation.respond(500);
+    context('when using OAuth', () => {
+      beforeEach(() => {
+        var loggedIn = true;
 
-      session.logout().catch(function(){
-        assert.calledWith(fakeAnalytics.track, fakeAnalytics.events.LOGOUT_FAILURE);
+        fakeAuth.login = sinon.stub().returns(Promise.resolve());
+        fakeAuth.logout = sinon.spy(() => {
+          loggedIn = false;
+          return Promise.resolve();
+        });
+
+        // Fake profile response after logout.
+        fakeStore.profile.read = () => Promise.resolve({
+          userid: null,
+          loggedIn,
+        });
       });
 
-      $httpBackend.flush();
+      it('logs the user out', () => {
+        return session.logout().then(() => {
+          assert.called(fakeAuth.logout);
+        });
+      });
+
+      it('tracks successful logout actions in analytics', () => {
+        return session.logout().then(() => {
+          assert.calledWith(fakeAnalytics.track, fakeAnalytics.events.LOGOUT_SUCCESS);
+        });
+      });
+
+      it('updates the profile after logging out', () => {
+        return session.logout().then(() => {
+          assert.isFalse(session.state.loggedIn);
+        });
+      });
     });
   });
 });