diff --git a/migrations/20190215215805-tokens-add-id.js b/migrations/20190215215805-tokens-add-id.js new file mode 100644 index 000000000..b2ec903c1 --- /dev/null +++ b/migrations/20190215215805-tokens-add-id.js @@ -0,0 +1,29 @@ +'use strict'; + +var async = require('async'); +var uuid = require('uuid'); + +exports.up = function(db, callback) { + async.series([ + db.runSql.bind(db, 'START TRANSACTION;'), + + db.runSql.bind(db, 'ALTER TABLE tokens ADD COLUMN id VARCHAR(128)'), + + function (done) { + db.runSql('SELECT * FROM tokens', function (error, tokens) { + async.eachSeries(tokens, function (token, iteratorDone) { + db.runSql('UPDATE tokens SET id=? WHERE accessToken=?', [ 'tid-'+uuid.v4(), token.accessToken ], iteratorDone); + }, done); + }); + }, + + db.runSql.bind(db, 'ALTER TABLE tokens MODIFY id VARCHAR(128) NOT NULL UNIQUE'), + db.runSql.bind(db, 'COMMIT'), + ], callback); +}; + +exports.down = function(db, callback) { + async.series([ + db.runSql.bind(db, 'ALTER TABLE tokens DROP COLUMN id'), + ], callback); +}; diff --git a/migrations/schema.sql b/migrations/schema.sql index e392fe7e9..68dfbde98 100644 --- a/migrations/schema.sql +++ b/migrations/schema.sql @@ -41,9 +41,10 @@ CREATE TABLE IF NOT EXISTS groupMembers( FOREIGN KEY(userId) REFERENCES users(id)); CREATE TABLE IF NOT EXISTS tokens( + id VARCHAR(128) NOT NULL UNIQUE, name VARCHAR(64) DEFAULT "", // description accessToken VARCHAR(128) NOT NULL UNIQUE, - identifier VARCHAR(128) NOT NULL, + identifier VARCHAR(128) NOT NULL, // resourceId: app id or user id clientId VARCHAR(128), scope VARCHAR(512) NOT NULL, expires BIGINT NOT NULL, // FIXME: make this a timestamp diff --git a/src/accesscontrol.js b/src/accesscontrol.js index b63bc2eba..fe63df851 100644 --- a/src/accesscontrol.js +++ b/src/accesscontrol.js @@ -121,7 +121,7 @@ function validateToken(accessToken, callback) { assert.strictEqual(typeof accessToken, 'string'); assert.strictEqual(typeof callback, 'function'); - tokendb.get(accessToken, function (error, token) { + tokendb.getByAccessToken(accessToken, function (error, token) { if (error && error.reason === DatabaseError.NOT_FOUND) return callback(null, null /* user */, 'Invalid Token'); // will end up as a 401 if (error) return callback(error); // this triggers 'internal error' in passport diff --git a/src/clients.js b/src/clients.js index 72b8e928f..499cf1189 100644 --- a/src/clients.js +++ b/src/clients.js @@ -276,16 +276,24 @@ function addTokenByUserId(clientId, userId, expiresAt, options, callback) { accesscontrol.scopesForUser(user, function (error, userScopes) { if (error) return callback(new ClientsError(ClientsError.INTERNAL_ERROR, error)); - var scope = accesscontrol.canonicalScopeString(result.scope); - var authorizedScopes = accesscontrol.intersectScopes(userScopes, scope.split(',')); + const scope = accesscontrol.canonicalScopeString(result.scope); + const authorizedScopes = accesscontrol.intersectScopes(userScopes, scope.split(',')); - var token = tokendb.generateToken(); + const token = { + id: 'tid-' + uuid.v4(), + accessToken: hat(8 * 32), + identifier: userId, + clientId: result.id, + expires: expiresAt, + scope: authorizedScopes.join(','), + name: name + }; - tokendb.add(token, userId, result.id, expiresAt, authorizedScopes.join(','), name, function (error) { + tokendb.add(token, function (error) { if (error) return callback(new ClientsError(ClientsError.INTERNAL_ERROR, error)); callback(null, { - accessToken: token, + accessToken: token.accessToken, tokenScopes: authorizedScopes, identifier: userId, clientId: result.id, @@ -347,5 +355,5 @@ function addDefaultClients(origin, callback) { } function removeTokenPrivateFields(token) { - return _.pick(token, 'identifier', 'clientId', 'scope', 'expires', 'name'); + return _.pick(token, 'id', 'identifier', 'clientId', 'scope', 'expires', 'name'); } diff --git a/src/test/database-test.js b/src/test/database-test.js index acddcfe18..ade95bfea 100644 --- a/src/test/database-test.js +++ b/src/test/database-test.js @@ -826,46 +826,42 @@ describe('database', function () { describe('token', function () { var TOKEN_0 = { + id: 'tid-0', name: 'token0', - accessToken: tokendb.generateToken(), + accessToken: hat(8 * 32), identifier: '0', clientId: 'clientid-0', expires: Date.now() + 60 * 60000, scope: 'clients' }; var TOKEN_1 = { + id: 'tid-1', name: 'token1', - accessToken: tokendb.generateToken(), + accessToken: hat(8 * 32), identifier: '1', clientId: 'clientid-1', expires: Number.MAX_SAFE_INTEGER, scope: 'settings' }; var TOKEN_2 = { + id: 'tid-2', name: 'token2', - accessToken: tokendb.generateToken(), + accessToken: hat(8 * 32), identifier: '2', clientId: 'clientid-2', expires: Date.now(), scope: 'apps' }; - it('add fails due to missing arguments', function () { - expect(function () { tokendb.add(TOKEN_0.accessToken, TOKEN_0.identifier, TOKEN_0.clientId, TOKEN_0.scope); }).to.throwError(); - expect(function () { tokendb.add(TOKEN_0.accessToken, TOKEN_0.identifier, TOKEN_0.clientId, function () {}); }).to.throwError(); - expect(function () { tokendb.add(TOKEN_0.accessToken, TOKEN_0.identifier, function () {}); }).to.throwError(); - expect(function () { tokendb.add(TOKEN_0.accessToken, function () {}); }).to.throwError(); - }); - it('add succeeds', function (done) { - tokendb.add(TOKEN_0.accessToken, TOKEN_0.identifier, TOKEN_0.clientId, TOKEN_0.expires, TOKEN_0.scope, TOKEN_0.name, function (error) { + tokendb.add(TOKEN_0, function (error) { expect(error).to.be(null); done(); }); }); it('add of same token fails', function (done) { - tokendb.add(TOKEN_0.accessToken, TOKEN_0.identifier, TOKEN_0.clientId, TOKEN_0.expires, TOKEN_0.scope, TOKEN_0.name, function (error) { + tokendb.add(TOKEN_0, function (error) { expect(error).to.be.a(DatabaseError); expect(error.reason).to.be(DatabaseError.ALREADY_EXISTS); done(); @@ -873,7 +869,16 @@ describe('database', function () { }); it('get succeeds', function (done) { - tokendb.get(TOKEN_0.accessToken, function (error, result) { + tokendb.get(TOKEN_0.id, function (error, result) { + expect(error).to.be(null); + expect(result).to.be.an('object'); + expect(result).to.be.eql(TOKEN_0); + done(); + }); + }); + + it('getByAccessToken succeeds', function (done) { + tokendb.getByAccessToken(TOKEN_0.accessToken, function (error, result) { expect(error).to.be(null); expect(result).to.be.an('object'); expect(result).to.be.eql(TOKEN_0); @@ -882,7 +887,7 @@ describe('database', function () { }); it('get of nonexisting token fails', function (done) { - tokendb.get(TOKEN_1.accessToken, function (error, result) { + tokendb.getByAccessToken(TOKEN_1.accessToken, function (error, result) { expect(error).to.be.a(DatabaseError); expect(error.reason).to.be(DatabaseError.NOT_FOUND); expect(result).to.not.be.ok(); @@ -901,8 +906,16 @@ describe('database', function () { }); }); + it('delete fails', function (done) { + tokendb.del(TOKEN_0.id + 'x', function (error) { + expect(error).to.be.a(DatabaseError); + expect(error.reason).to.be(DatabaseError.NOT_FOUND); + done(); + }); + }); + it('delete succeeds', function (done) { - tokendb.del(TOKEN_0.accessToken, function (error) { + tokendb.del(TOKEN_0.id, function (error) { expect(error).to.be(null); done(); }); @@ -918,7 +931,7 @@ describe('database', function () { }); it('delByIdentifier succeeds', function (done) { - tokendb.add(TOKEN_1.accessToken, TOKEN_1.identifier, TOKEN_1.clientId, TOKEN_1.expires, TOKEN_1.scope, '', function (error) { + tokendb.add(TOKEN_1, function (error) { expect(error).to.be(null); tokendb.delByIdentifier(TOKEN_1.identifier, function (error) { @@ -929,7 +942,7 @@ describe('database', function () { }); it('cannot delete previously delete record', function (done) { - tokendb.del(TOKEN_0.accessToken, function (error) { + tokendb.del(TOKEN_0.id, function (error) { expect(error).to.be.a(DatabaseError); expect(error.reason).to.be(DatabaseError.NOT_FOUND); done(); @@ -937,7 +950,7 @@ describe('database', function () { }); it('getByIdentifierAndClientId succeeds', function (done) { - tokendb.add(TOKEN_0.accessToken, TOKEN_0.identifier, TOKEN_0.clientId, TOKEN_0.expires, TOKEN_0.scope, TOKEN_0.name, function (error) { + tokendb.add(TOKEN_0, function (error) { expect(error).to.be(null); tokendb.getByIdentifierAndClientId(TOKEN_0.identifier, TOKEN_0.clientId, function (error, result) { @@ -951,14 +964,14 @@ describe('database', function () { }); it('delExpired succeeds', function (done) { - tokendb.add(TOKEN_2.accessToken, TOKEN_2.identifier, TOKEN_2.clientId, TOKEN_2.expires, TOKEN_2.scope, TOKEN_2.name, function (error) { + tokendb.add(TOKEN_2, function (error) { expect(error).to.be(null); tokendb.delExpired(function (error, result) { expect(error).to.not.be.ok(); expect(result).to.eql(1); - tokendb.get(TOKEN_2.accessToken, function (error, result) { + tokendb.getByAccessToken(TOKEN_2.accessToken, function (error, result) { expect(error).to.be.a(DatabaseError); expect(error.reason).to.be(DatabaseError.NOT_FOUND); expect(result).to.not.be.ok(); @@ -972,7 +985,7 @@ describe('database', function () { tokendb.delByIdentifierAndClientId(TOKEN_0.identifier, TOKEN_0.clientId, function (error) { expect(error).to.be(null); - tokendb.get(TOKEN_0.accessToken, function (error, result) { + tokendb.getByAccessToken(TOKEN_0.accessToken, function (error, result) { expect(error).to.be.a(DatabaseError); expect(error.reason).to.be(DatabaseError.NOT_FOUND); expect(result).to.not.be.ok(); @@ -982,13 +995,13 @@ describe('database', function () { }); it('delByClientId succeeds', function (done) { - tokendb.add(TOKEN_0.accessToken, TOKEN_0.identifier, TOKEN_0.clientId, TOKEN_0.expires, TOKEN_0.scope, TOKEN_0.name, function (error) { + tokendb.add(TOKEN_0, function (error) { expect(error).to.be(null); tokendb.delByClientId(TOKEN_0.clientId, function (error) { expect(error).to.not.be.ok(); - tokendb.get(TOKEN_0.accessToken, function (error, result) { + tokendb.getByAccessToken(TOKEN_0.accessToken, function (error, result) { expect(error).to.be.a(DatabaseError); expect(error.reason).to.be(DatabaseError.NOT_FOUND); expect(result).to.not.be.ok(); @@ -1840,7 +1853,7 @@ describe('database', function () { var yesterday = new Date(); yesterday.setDate(yesterday.getDate() - 1); - database.query('INSERT INTO eventlog (id, action, source, data, creationTime) VALUES (?, ?, ?, ?, ?)', [ 'anotherid', 'user.login2', JSON.stringify({ ip: '1.2.3.4' }), JSON.stringify({ appId: 'thatapp' }), yesterday ], function (error, result) { + database.query('INSERT INTO eventlog (id, action, source, data, creationTime) VALUES (?, ?, ?, ?, ?)', [ 'anotherid', 'user.login2', JSON.stringify({ ip: '1.2.3.4' }), JSON.stringify({ appId: 'thatapp' }), yesterday ], function (error) { expect(error).to.equal(null); eventlogdb.upsert('anotherid_new', 'user.login2', { ip: '1.2.3.4' }, { appId: 'thatapp' }, function (error, result) { diff --git a/src/test/janitor-test.js b/src/test/janitor-test.js index dce9aa0a8..316eca2d8 100644 --- a/src/test/janitor-test.js +++ b/src/test/janitor-test.js @@ -85,7 +85,7 @@ describe('janitor', function () { }); it('did not remove the non-expired token', function (done) { - tokendb.get(TOKEN_0.accessToken, function (error, result) { + tokendb.getByAccessToken(TOKEN_0.accessToken, function (error, result) { expect(error).to.be(null); expect(result).to.be.eql(TOKEN_0); done(); @@ -93,7 +93,7 @@ describe('janitor', function () { }); it('did remove the non-expired token', function (done) { - tokendb.get(TOKEN_1.accessToken, function (error, result) { + tokendb.getByAccessToken(TOKEN_1.accessToken, function (error, result) { expect(error).to.be.a(DatabaseError); expect(error.reason).to.be(DatabaseError.NOT_FOUND); expect(result).to.not.be.ok(); diff --git a/src/tokendb.js b/src/tokendb.js index 05e584dba..add34bc87 100644 --- a/src/tokendb.js +++ b/src/tokendb.js @@ -3,8 +3,8 @@ 'use strict'; exports = module.exports = { - generateToken: generateToken, get: get, + getByAccessToken: getByAccessToken, add: add, del: del, delByClientId: delByClientId, @@ -19,16 +19,11 @@ exports = module.exports = { var assert = require('assert'), database = require('./database.js'), - DatabaseError = require('./databaseerror'), - hat = require('./hat.js'); + DatabaseError = require('./databaseerror'); -var TOKENS_FIELDS = [ 'accessToken', 'identifier', 'clientId', 'scope', 'expires', 'name' ].join(','); +var TOKENS_FIELDS = [ 'id', 'accessToken', 'identifier', 'clientId', 'scope', 'expires', 'name' ].join(','); -function generateToken() { - return hat(8 * 32); // TODO: make this stronger -} - -function get(accessToken, callback) { +function getByAccessToken(accessToken, callback) { assert.strictEqual(typeof accessToken, 'string'); assert.strictEqual(typeof callback, 'function'); @@ -40,7 +35,24 @@ function get(accessToken, callback) { }); } -function add(accessToken, identifier, clientId, expires, scope, name, callback) { +function get(id, callback) { + assert.strictEqual(typeof id, 'string'); + assert.strictEqual(typeof callback, 'function'); + + database.query('SELECT ' + TOKENS_FIELDS + ' FROM tokens WHERE id = ?', [ id ], function (error, result) { + if (error) return callback(new DatabaseError(DatabaseError.INTERNAL_ERROR, error)); + if (result.length === 0) return callback(new DatabaseError(DatabaseError.NOT_FOUND)); + + callback(null, result[0]); + }); +} + +function add(token, callback) { + assert.strictEqual(typeof token, 'object'); + assert.strictEqual(typeof callback, 'function'); + + let { id, accessToken, identifier, clientId, expires, scope, name } = token; + assert.strictEqual(typeof accessToken, 'string'); assert.strictEqual(typeof identifier, 'string'); assert(typeof clientId === 'string' || clientId === null); @@ -49,8 +61,8 @@ function add(accessToken, identifier, clientId, expires, scope, name, callback) assert.strictEqual(typeof name, 'string'); assert.strictEqual(typeof callback, 'function'); - database.query('INSERT INTO tokens (accessToken, identifier, clientId, expires, scope, name) VALUES (?, ?, ?, ?, ?, ?)', - [ accessToken, identifier, clientId, expires, scope, name ], function (error, result) { + database.query('INSERT INTO tokens (id, accessToken, identifier, clientId, expires, scope, name) VALUES (?, ?, ?, ?, ?, ?, ?)', + [ id, accessToken, identifier, clientId, expires, scope, name ], function (error, result) { if (error && error.code === 'ER_DUP_ENTRY') return callback(new DatabaseError(DatabaseError.ALREADY_EXISTS)); if (error || result.affectedRows !== 1) return callback(new DatabaseError(DatabaseError.INTERNAL_ERROR, error)); @@ -58,11 +70,11 @@ function add(accessToken, identifier, clientId, expires, scope, name, callback) }); } -function del(accessToken, callback) { - assert.strictEqual(typeof accessToken, 'string'); +function del(id, callback) { + assert.strictEqual(typeof id, 'string'); assert.strictEqual(typeof callback, 'function'); - database.query('DELETE FROM tokens WHERE accessToken = ?', [ accessToken ], function (error, result) { + database.query('DELETE FROM tokens WHERE id = ?', [ id ], function (error, result) { if (error) return callback(new DatabaseError(DatabaseError.INTERNAL_ERROR, error)); if (result.affectedRows !== 1) return callback(new DatabaseError(DatabaseError.NOT_FOUND));