feat(codes): Delete authorization codes when revoking client access. (#…

…578); r=philbooth

lib/db/memory.js

@@ -292,13 +292,13 @@ MemoryStore.prototype = {
   },
 
   /**
-   * Delete all non-expired tokens for some clientId and uid.
+   * Delete all authorization grants for some clientId and uid.
    *
    * @param {String} clientId Client ID
    * @param {String} uid User Id as Hex
-   * @returns {Promise}
+   e @returns {Promise}
    */
-  deleteActiveClientTokens: function deleteActiveClientTokens(clientId, uid) {
+  deleteClientAuthorization: function deleteClientAuthorization(clientId, uid) {
     if (! clientId || ! uid) {
       return P.reject(new Error('clientId and uid are required'));
     }
@@ -314,6 +314,7 @@ MemoryStore.prototype = {
       }
     }
 
+    deleteToken(this.codes);
     deleteToken(this.tokens);
     deleteToken(this.refreshTokens);
 

lib/db/mysql/index.js

@@ -196,6 +196,8 @@ const QUERY_ACTIVE_CLIENT_TOKENS_BY_UID =
   'SELECT tokens.clientId AS id, tokens.createdAt, tokens.scope, clients.name ' +
   'FROM tokens LEFT OUTER JOIN clients ON clients.id = tokens.clientId ' +
   'WHERE tokens.userId=? AND tokens.expiresAt > NOW() AND clients.canGrant = 0;';
+const DELETE_ACTIVE_CODES_BY_CLIENT_AND_UID =
+  'DELETE FROM codes WHERE clientId=? AND userId=?';
 const DELETE_ACTIVE_TOKENS_BY_CLIENT_AND_UID =
   'DELETE FROM tokens WHERE clientId=? AND userId=?';
 const DELETE_ACTIVE_REFRESH_TOKENS_BY_CLIENT_AND_UID =
@@ -467,13 +469,18 @@ MysqlStore.prototype = {
   },
 
   /**
-   * Delete all tokens for some clientId and uid.
+   * Delete all authorization grants for some clientId and uid.
    *
    * @param {String} clientId Client ID
    * @param {String} uid User Id as Hex
    * @returns {Promise}
    */
-  deleteActiveClientTokens: function deleteActiveClientTokens(clientId, uid) {
+  deleteClientAuthorization: function deleteClientAuthorization(clientId, uid) {
+    const deleteCodes = this._write(DELETE_ACTIVE_CODES_BY_CLIENT_AND_UID, [
+      buf(clientId),
+      buf(uid)
+    ]);
+
     const deleteTokens = this._write(DELETE_ACTIVE_TOKENS_BY_CLIENT_AND_UID, [
       buf(clientId),
       buf(uid)
@@ -485,6 +492,7 @@ MysqlStore.prototype = {
     ]);
 
     return P.all([
+      deleteCodes,
       deleteTokens,
       deleteRefreshTokens
     ]);

lib/routes/client-tokens/delete.js

@@ -12,7 +12,7 @@ module.exports = {
   },
   handler: function activeServices(req, reply) {
     var clientId = req.params.client_id;
-    return db.deleteActiveClientTokens(clientId, req.auth.credentials.user)
+    return db.deleteClientAuthorization(clientId, req.auth.credentials.user)
       .done(function() {
         reply({});
       }, reply);

test/api.js

@@ -26,45 +26,27 @@ const STALE_AUTH_AT = AUTH_AT - (2 * 24 * 60 * 60);
 const AMR = ['pwd', 'email'];
 const AAL = 1;
 const ACR = 'AAL1';
-const VERIFY_GOOD = JSON.stringify({
-  status: 'okay',
-  email: USERID + '@' + config.get('browserid.issuer'),
-  issuer: config.get('browserid.issuer'),
-  idpClaims: {
-    'fxa-verifiedEmail': VEMAIL,
-    'fxa-lastAuthAt': AUTH_AT,
-    'fxa-generation': 123456,
-    'fxa-tokenVerified': true,
-    'fxa-amr': AMR,
-    'fxa-aal': AAL
-  }
-});
-const VERIFY_GOOD_BUT_STALE = JSON.stringify({
-  status: 'okay',
-  email: USERID + '@' + config.get('browserid.issuer'),
-  issuer: config.get('browserid.issuer'),
-  idpClaims: {
-    'fxa-verifiedEmail': VEMAIL,
-    'fxa-lastAuthAt': STALE_AUTH_AT,
-    'fxa-generation': 123456,
-    'fxa-tokenVerified': true,
-    'fxa-amr': AMR,
-    'fxa-aal': AAL
-  }
-});
-const VERIFY_GOOD_BUT_UNVERIFIED = JSON.stringify({
-  status: 'okay',
-  email: USERID + '@' + config.get('browserid.issuer'),
-  issuer: config.get('browserid.issuer'),
-  idpClaims: {
-    'fxa-verifiedEmail': VEMAIL,
-    'fxa-lastAuthAt': AUTH_AT,
-    'fxa-generation': 123456,
-    'fxa-tokenVerified': false,
-    'fxa-amr': AMR,
-    'fxa-aal': AAL
-  }
-});
+
+function mockVerifierResult(opts) {
+  opts = opts || {};
+  return JSON.stringify({
+    status: opts.status || 'okay',
+    email: (opts.uid ||  USERID) + '@' + config.get('browserid.issuer'),
+    issuer: opts.issuer || config.get('browserid.issuer'),
+    idpClaims: {
+      'fxa-verifiedEmail': opts.vemail || VEMAIL,
+      'fxa-lastAuthAt': opts.authAt || AUTH_AT,
+      'fxa-generation': opts.generation || 123456,
+      'fxa-tokenVerified': opts.hasOwnProperty('tokenVerified') ? opts.tokenVerified : true,
+      'fxa-amr': opts.amr || AMR,
+      'fxa-aal': opts.aal || AAL
+    }
+  });
+}
+
+const VERIFY_GOOD = mockVerifierResult();
+const VERIFY_GOOD_BUT_STALE = mockVerifierResult({ authAt: STALE_AUTH_AT });
+const VERIFY_GOOD_BUT_UNVERIFIED = mockVerifierResult({ tokenVerified: false });
 
 const MAX_TTL_S = config.get('expiration.accessToken') / 1000;
 
@@ -3418,6 +3400,41 @@ describe('/v1', function() {
           });
       });
 
+      it('deletes outstanding authorization codes for the client', () => {
+        let code;
+        mockAssertion().reply(200, mockVerifierResult({ uid: user1.uid }));
+        return Server.api.post({
+          url: '/authorization',
+          payload: authParams({
+            scope: 'profile',
+          })
+        }).then(res => {
+          code = res.result.code;
+          assert.ok(code, 'an authorization code was generated');
+          return Server.api.delete({
+            url: '/client-tokens/' + clientId.toString('hex'),
+            headers: {
+              authorization: 'Bearer ' + tokenWithClientWrite
+            }
+          });
+        }).then(res => {
+          return Server.api.post({
+            url: '/token',
+            payload: {
+              client_id: clientId,
+              client_secret: secret,
+              code,
+            }
+          });
+        }).then(res => {
+          assert.equal(res.statusCode, 400);
+          assert.equal(res.result.code, 400);
+          assert.equal(res.result.errno, 105);
+          assert.equal(res.result.message, 'Unknown code');
+          assertSecurityHeaders(res);
+        });
+      });
+
       it('errors for invalid tokens', function() {
         return Server.api.delete({
           url: '/client-tokens/' + clientId,

test/db/index.js

@@ -605,12 +605,12 @@ describe('db', function() {
       });
     });
 
-    describe('deleteActiveClientTokens', function() {
+    describe('deleteClientAuthorization', function() {
       var clientId = buf(randomString(8));
       var userId = buf(randomString(16));
 
       it('should delete client tokens', function() {
-        return db.deleteActiveClientTokens(clientId, userId)
+        return db.deleteClientAuthorization(clientId, userId)
           .then(
             function(result) {
               assert.ok(result);