Merge pull request #514 from hypothesis/oauth-code-grant-type

Use correct request params when exchanging auth code for tokens

src/sidebar/oauth-auth.js

@@ -141,11 +141,8 @@ function auth($http, $window, flash, localStorage, random, settings) {
 
   // Exchange the JWT grant token for an access token.
   // See https://tools.ietf.org/html/rfc7523#section-4
-  function exchangeToken(grantToken) {
+  function exchangeJWT(grantToken) {
     var data = {
-      // FIXME: This should be set to the appropriate grant type if we are
-      //        exchanging an authorization code for a grant token, which
-      //        is the case for first-party accounts.
       grant_type: 'urn:ietf:params:oauth:grant-type:jwt-bearer',
       assertion: grantToken,
     };
@@ -157,6 +154,24 @@ function auth($http, $window, flash, localStorage, random, settings) {
     });
   }
 
+  /**
+   * Exchange an authorization code from the `/oauth/authorize` endpoint for
+   * access and refresh tokens.
+   */
+  function exchangeAuthCode(code) {
+    var data = {
+      client_id: settings.oauthClientId,
+      grant_type: 'authorization_code',
+      code,
+    };
+    return postToTokenUrl(data).then((response) => {
+      if (response.status !== 200) {
+        throw new Error('Authorization code exchange failed');
+      }
+      return tokenInfoFrom(response);
+    });
+  }
+
   /**
    * Exchange the refresh token for a new access token and refresh token pair.
    * See https://tools.ietf.org/html/rfc6749#section-6
@@ -226,7 +241,7 @@ function auth($http, $window, flash, localStorage, random, settings) {
 
       if (grantToken) {
         // Exchange host-page provided grant token for a new access token.
-        accessTokenPromise = exchangeToken(grantToken).then(function (tokenInfo) {
+        accessTokenPromise = exchangeJWT(grantToken).then((tokenInfo) => {
           refreshAccessTokenBeforeItExpires(tokenInfo, { persist: false });
           return tokenInfo.accessToken;
         }).catch(function(err) {
@@ -237,7 +252,7 @@ function auth($http, $window, flash, localStorage, random, settings) {
       } else if (authCode) {
         // Exchange authorization code retrieved from login popup for a new
         // access token.
-        accessTokenPromise = exchangeToken(authCode).then((tokenInfo) => {
+        accessTokenPromise = exchangeAuthCode(authCode).then((tokenInfo) => {
           saveToken(tokenInfo);
           refreshAccessTokenBeforeItExpires(tokenInfo, { persist: true });
           return tokenInfo.accessToken;

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

@@ -506,8 +506,9 @@ describe('sidebar.oauth-auth', function () {
       }).then(() => {
         // 2. Verify that auth code is exchanged for access & refresh tokens.
         var expectedBody =
-          'assertion=acode' +
-          '&grant_type=urn%3Aietf%3Aparams%3Aoauth%3Agrant-type%3Ajwt-bearer';
+          'client_id=the-client-id' +
+          '&code=acode' +
+          '&grant_type=authorization_code';
         assert.calledWith(fakeHttp.post, 'https://hypothes.is/api/token', expectedBody, {
           headers: {'Content-Type': 'application/x-www-form-urlencoded'},
         });
@@ -527,6 +528,24 @@ describe('sidebar.oauth-auth', function () {
         assert.equal(err.message, 'Authorization window was closed');
       });
     });
+
+    it('rejects if auth code exchange fails', () => {
+      var loggedIn = auth.login();
+
+      // Successful response from authz popup.
+      fakeWindow.sendMessage({
+        type: 'authorization_response',
+        code: 'acode',
+        state: 'notrandom',
+      });
+
+      // Error response from auth code => token exchange.
+      fakeHttp.post.returns(Promise.resolve({status: 400}));
+
+      return loggedIn.catch(err => {
+        assert.equal(err.message, 'Authorization code exchange failed');
+      });
+    });
   });
 
   // Advance time forward so that any current access tokens will have expired.