refactor(db): clients.secret to clients.hashedSecret, remove clients.whitelisted

Closes #155
Closes #267

Author: seanmonstar

lib/db/memory.js

@@ -19,7 +19,7 @@ const MAX_TTL = config.get('expiration.accessToken');
  *   clients: {
  *     <id>: {
  *       id: <id>,
- *       secret: <string>,
+ *       hashedSecret: <string>,
  *       name: <string>,
  *       imageUri: <string>,
  *       redirectUri: <string>,
@@ -133,7 +133,7 @@ MemoryStore.prototype = {
     client.canGrant = !!client.canGrant;
     client.trusted = !!client.trusted;
     this.clients[hex] = client;
-    client.secret = client.hashedSecret;
+    client.hashedSecret = client.hashedSecret;
     return P.resolve(client);
   },
   updateClient: function updateClient(client) {
@@ -149,7 +149,7 @@ MemoryStore.prototype = {
       if (key === 'id') {
         // nothing
       } else if (key === 'hashedSecret') {
-        old.secret = buf(client[key]);
+        old.hashedSecret = buf(client[key]);
       } else if (client[key] !== undefined) {
         old[key] = client[key];
       }

lib/db/mysql/index.js

@@ -112,9 +112,9 @@ MysqlStore.connect = function mysqlConnect(options) {
 
 const QUERY_CLIENT_REGISTER =
   'INSERT INTO clients ' +
-  '(id, name, imageUri, secret, redirectUri, termsUri, privacyUri, ' +
-  ' whitelisted, trusted, canGrant) ' +
-  'VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?);';
+  '(id, name, imageUri, hashedSecret, redirectUri, termsUri, privacyUri, ' +
+  ' trusted, canGrant) ' +
+  'VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?);';
 const QUERY_CLIENT_DEVELOPER_INSERT =
   'INSERT INTO clientDevelopers ' +
   '(rowId, developerId, clientId) ' +
@@ -135,17 +135,17 @@ const QUERY_DEVELOPER_INSERT =
   'VALUES (?, ?);';
 const QUERY_CLIENT_GET = 'SELECT * FROM clients WHERE id=?';
 const QUERY_CLIENT_LIST = 'SELECT id, name, redirectUri, imageUri, ' +
-  'termsUri, privacyUri, canGrant, whitelisted, trusted ' +
+  'termsUri, privacyUri, canGrant, trusted ' +
   'FROM clients, clientDevelopers, developers ' +
   'WHERE clients.id = clientDevelopers.clientId AND ' +
   'developers.developerId = clientDevelopers.developerId AND ' +
   'developers.email =?;';
 const QUERY_CLIENT_UPDATE = 'UPDATE clients SET ' +
   'name=COALESCE(?, name), imageUri=COALESCE(?, imageUri), ' +
-  'secret=COALESCE(?, secret), redirectUri=COALESCE(?, redirectUri), ' +
+  'hashedSecret=COALESCE(?, hashedSecret), ' +
+  'redirectUri=COALESCE(?, redirectUri), ' +
   'termsUri=COALESCE(?, termsUri), privacyUri=COALESCE(?, privacyUri), ' +
-  'whitelisted=COALESCE(?, whitelisted), trusted=COALESCE(?, trusted), ' +
-  'canGrant=COALESCE(?, canGrant) ' +
+  'trusted=COALESCE(?, trusted), canGrant=COALESCE(?, canGrant) ' +
   'WHERE id=?';
 const QUERY_CLIENT_DELETE = 'DELETE FROM clients WHERE id=?';
 const QUERY_CODE_INSERT =
@@ -215,8 +215,7 @@ MysqlStore.prototype = {
       client.redirectUri,
       client.termsUri || '',
       client.privacyUri || '',
-      !!client.trusted,  // XXX TODO: we have duplicate columns while we're
-      !!client.trusted,  // in the process of renaming whitelisted=>trusted.
+      !!client.trusted,
       !!client.canGrant
     ]).then(function() {
       logger.debug('registerClient.success', { id: hex(id) });
@@ -299,7 +298,7 @@ MysqlStore.prototype = {
     if (!client.id) {
       return P.reject(new Error('Update client needs an id'));
     }
-    var secret = client.hashedSecret || client.secret || null;
+    var secret = client.hashedSecret;
     if (secret) {
       secret = buf(secret);
     }
@@ -311,8 +310,7 @@ MysqlStore.prototype = {
       client.redirectUri,
       client.termsUri,
       client.privacyUri,
-      client.trusted,  // XXX TODO: we have duplicate columns while we're
-      client.trusted,  // in the process of renaming whitelisted => trusted.
+      client.trusted,
       client.canGrant,
 
       // WHERE

lib/db/mysql/patch.js

@@ -6,5 +6,5 @@
 // Update this if you add a new patch, and don't forget to update
 // the documentation for the current schema in ../schema.sql.
 
-module.exports.level = 6;
+module.exports.level = 7;
 

lib/db/mysql/patches/patch-006-007.sql

@@ -0,0 +1,7 @@
+-- Change clients.secret to clients.hashedSecret
+-- Drop whitelisted column
+
+ALTER TABLE clients CHANGE secret hashedSecret BINARY(32);
+ALTER TABLE clients DROP COLUMN whitelisted;
+
+UPDATE dbMetadata SET value = '7' WHERE name = 'schema-patch-level';

lib/db/mysql/patches/patch-007-006.sql

@@ -0,0 +1,8 @@
+-- Change clients.hashedSecret to clients.secret
+--  (commented out to avoid accidentally running this in production...)
+
+-- ALTER TABLE clients CHANGE hashedSecret secret BINARY(32);
+-- ALTER TABLE clients ADD COLUMN whitelisted BOOLEAN DEFAULT FALSE;
+-- UPDATE clients SET whitelisted=truested;
+
+-- UPDATE dbMetadata SET value = '6' WHERE name = 'schema-patch-level';

lib/db/mysql/schema.sql

@@ -9,13 +9,12 @@
 
 CREATE TABLE IF NOT EXISTS clients (
   id BINARY(8) PRIMARY KEY,
-  secret BINARY(32) NOT NULL,
+  hashedSecret BINARY(32) NOT NULL,
   name VARCHAR(256) NOT NULL,
   imageUri VARCHAR(256) NOT NULL,
   redirectUri VARCHAR(256) NOT NULL,
   termsUri VARCHAR(256) NOT NULL,
   privacyUri VARCHAR(256) NOT NULL,
-  whitelisted BOOLEAN DEFAULT FALSE,
   canGrant BOOLEAN DEFAULT FALSE,
   createdAt TIMESTAMP NOT NULL DEFAULT CURRENT_TIMESTAMP,
   trusted BOOLEAN DEFAULT FALSE

lib/routes/client/list.js

@@ -18,8 +18,6 @@ function serialize(client) {
     terms_uri: client.termsUri,
     privacy_uri: client.privacyUri,
     can_grant: client.canGrant,
-    // XXX TODO: a future PR will remove legacy "whitelisted" attr
-    whitelisted: client.trusted,
     trusted: client.trusted
   };
 }
@@ -40,8 +38,6 @@ module.exports = {
           terms_uri: Joi.string().allow('').required(),
           privacy_uri: Joi.string().allow('').required(),
           can_grant: Joi.boolean().required(),
-          // XXX TODO: a future PR will remove legacy "whitelisted" attr
-          whitelisted: Joi.boolean().required(),
           trusted: Joi.boolean().required()
         })
       )

lib/routes/client/register.js

@@ -25,8 +25,6 @@ module.exports = {
       terms_uri: Joi.string().max(256).allow(''),
       privacy_uri: Joi.string().max(256).allow(''),
       can_grant: Joi.boolean(),
-      // XXX TODO: a future PR will remove legacy "whitelisted" property
-      whitelisted: Joi.boolean(),
       trusted: Joi.boolean()
     }
   },
@@ -40,8 +38,6 @@ module.exports = {
       terms_uri: Joi.string().required().allow(''),
       privacy_uri: Joi.string().required().allow(''),
       can_grant: Joi.boolean().required(),
-      // XXX TODO: a future PR will remove legacy "whitelisted" property
-      whitelisted: Joi.boolean().required(),
       trusted: Joi.boolean().required()
     }
   },
@@ -57,11 +53,7 @@ module.exports = {
       termsUri: payload.terms_uri || '',
       privacyUri: payload.privacy_uri || '',
       canGrant: !!payload.can_grant,
-      // XXX TODO: a future PR will remove legacy "whitelisted" property.
-      // Accept both for now for API b/w compat.
-      trusted: !!(typeof payload.trusted !== 'undefined' ?
-                    payload.trusted :
-                    payload.whitelisted)
+      trusted: !!payload.trusted
     };
     var developerEmail = req.auth.credentials.email;
     var developerId = null;
@@ -91,8 +83,6 @@ module.exports = {
           terms_uri: client.termsUri,
           privacy_uri: client.privacyUri,
           can_grant: client.canGrant,
-          // XXX TODO: a future PR will remove legacy "whitelisted" property
-          whitelisted: client.trusted,
           trusted: client.trusted
         }).code(201);
       }, reply);

lib/routes/token.js

@@ -28,7 +28,7 @@ function confirmClient(id, secret) {
     }
 
     var submitted = hex(encrypt.hash(buf(secret)));
-    var stored = hex(client.secret);
+    var stored = hex(client.hashedSecret);
     if (submitted !== stored) {
       logger.info('client.mismatchSecret', { client: id });
       logger.verbose('client.mismatchSecret.details', {

test/api.js

@@ -881,7 +881,7 @@ describe('/v1', function() {
             hashedSecret: encrypt.hash(secret2),
             redirectUri: 'https://example.domain',
             imageUri: 'https://example.foo.domain/logo.png',
-            whitelisted: true
+            trusted: true
           };
           return db.registerClient(client2).then(function(c) {
             id2 = c.id.toString('hex');
@@ -1256,9 +1256,6 @@ describe('/v1', function() {
             assert(client.image_uri === '');
             assert(client.can_grant === false);
             assert(client.trusted === false);
-            // XXX TODO: future PR will remove legacy "whitelisted" attr,
-            // it's here for now for API b/w compat
-            assert(client.whitelisted === false);
             return db.getClient(client.id).then(function(klient) {
               assert.equal(klient.id.toString('hex'), client.id);
               assert.equal(klient.name, client.name);