diff --git a/migrations/20160405083451-users-drop-username-null.js b/migrations/20160405083451-users-drop-username-null.js new file mode 100644 index 000000000..aa941c5bb --- /dev/null +++ b/migrations/20160405083451-users-drop-username-null.js @@ -0,0 +1,15 @@ +dbm = dbm || require('db-migrate'); + +exports.up = function(db, callback) { + db.runSql('ALTER TABLE users MODIFY username VARCHAR(254) UNIQUE', [], function (error) { + if (error) console.error(error); + callback(error); + }); +}; + +exports.down = function(db, callback) { + db.runSql('ALTER TABLE users MODIFY username VARCHAR(254) NOT NULL UNIQUE', [], function (error) { + if (error) console.error(error); + callback(error); + }); +}; diff --git a/migrations/schema.sql b/migrations/schema.sql index 7c2e6617a..b5975c734 100644 --- a/migrations/schema.sql +++ b/migrations/schema.sql @@ -11,7 +11,7 @@ CREATE TABLE IF NOT EXISTS users( id VARCHAR(128) NOT NULL UNIQUE, - username VARCHAR(254) NOT NULL UNIQUE, + username VARCHAR(254) UNIQUE, email VARCHAR(254) NOT NULL UNIQUE, password VARCHAR(1024) NOT NULL, salt VARCHAR(512) NOT NULL, diff --git a/src/test/database-test.js b/src/test/database-test.js index 50cbc9aa6..21fc65f55 100644 --- a/src/test/database-test.js +++ b/src/test/database-test.js @@ -47,7 +47,7 @@ describe('database', function () { var USER_1 = { id: 'uuid456', - username: 'uuid456', + username: '', password: 'secret', email: 'safe2@me.com', salt: 'tata', @@ -57,18 +57,28 @@ describe('database', function () { displayName: 'Herbert Heidelberg' }; + var USER_2 = { + id: 'uuid457', + username: '', + password: 'secret', + email: 'safe3@me.com', + salt: 'tata', + createdAt: 'sometime back', + modifiedAt: 'now', + resetToken: '', + displayName: 'Herbert Heidelberg' + }; + it('can add user', function (done) { - userdb.add(USER_0.id, USER_0, function (error) { - expect(!error).to.be.ok(); - done(); - }); + userdb.add(USER_0.id, USER_0, done); }); it('can add another user', function (done) { - userdb.add(USER_1.id, USER_1, function (error) { - expect(!error).to.be.ok(); - done(); - }); + userdb.add(USER_1.id, USER_1, done); + }); + + it('can add another user with empty username', function (done) { + userdb.add(USER_2.id, USER_2, done); }); it('cannot add same user again', function (done) { @@ -123,13 +133,22 @@ describe('database', function () { it('can get all with group ids', function (done) { userdb.getAllWithGroupIds(function (error, all) { expect(error).to.not.be.ok(); - expect(all.length).to.equal(2); - var user0Copy = _.extend({}, USER_0); - user0Copy.groupIds = [ ]; - expect(all[0]).to.eql(user0Copy); - var user1Copy = _.extend({}, USER_1); - user1Copy.groupIds = [ ]; - expect(all[1]).to.eql(user1Copy); + expect(all.length).to.equal(3); + + var userCopy; + + userCopy = _.extend({}, USER_0); + userCopy.groupIds = [ ]; + expect(all[0]).to.eql(userCopy); + + userCopy = _.extend({}, USER_1); + userCopy.groupIds = [ ]; + expect(all[1]).to.eql(userCopy); + + userCopy = _.extend({}, USER_2); + userCopy.groupIds = [ ]; + expect(all[2]).to.eql(userCopy); + done(); }); }); @@ -145,7 +164,7 @@ describe('database', function () { it('counts the users', function (done) { userdb.count(function (error, count) { expect(error).to.not.be.ok(); - expect(count).to.equal(2); + expect(count).to.equal(3); done(); }); }); @@ -185,7 +204,7 @@ describe('database', function () { it('did remove the user', function (done) { userdb.count(function (error, count) { - expect(count).to.equal(1); + expect(count).to.equal(2); done(); }); }); diff --git a/src/userdb.js b/src/userdb.js index 78a21a5b4..8403ff6a2 100644 --- a/src/userdb.js +++ b/src/userdb.js @@ -25,6 +25,15 @@ var assert = require('assert'), var USERS_FIELDS = [ 'id', 'username', 'email', 'password', 'salt', 'createdAt', 'modifiedAt', 'resetToken', 'displayName' ].join(','); +function postProcess(result) { + assert.strictEqual(typeof result, 'object'); + + // The username may be null or undefined in the db, let's ensure it is a string + result.username = result.username || ''; + + return result; +} + function get(userId, callback) { assert.strictEqual(typeof userId, 'string'); assert.strictEqual(typeof callback, 'function'); @@ -33,7 +42,7 @@ function get(userId, callback) { 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]); + callback(null, postProcess(result[0])); }); } @@ -45,7 +54,7 @@ function getByUsername(username, callback) { 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]); + callback(null, postProcess(result[0])); }); } @@ -57,7 +66,7 @@ function getByEmail(email, callback) { 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]); + callback(null, postProcess(result[0])); }); } @@ -70,7 +79,7 @@ function getOwner(callback) { 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]); + callback(null, postProcess(result[0])); }); } @@ -84,7 +93,7 @@ function getByResetToken(resetToken, callback) { 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]); + callback(null, postProcess(result[0])); }); } @@ -100,6 +109,8 @@ function getAllWithGroupIds(callback) { result.groupIds = result.groupIds ? result.groupIds.split(',') : [ ]; }); + results.forEach(postProcess); + callback(null, results); }); } @@ -107,10 +118,11 @@ function getAllWithGroupIds(callback) { function getAllAdmins(callback) { assert.strictEqual(typeof callback, 'function'); - database.query('SELECT ' + USERS_FIELDS + ' FROM users, groupMembers WHERE groupMembers.groupId = ? AND users.id = groupMembers.userId', - [ groups.ADMIN_GROUP_ID ], function (error, results) { + database.query('SELECT ' + USERS_FIELDS + ' FROM users, groupMembers WHERE groupMembers.groupId = ? AND users.id = groupMembers.userId', [ groups.ADMIN_GROUP_ID ], function (error, results) { if (error) return callback(new DatabaseError(DatabaseError.INTERNAL_ERROR, error)); + results.forEach(postProcess); + callback(null, results); }); } @@ -127,9 +139,8 @@ function add(userId, user, callback) { assert.strictEqual(typeof user.displayName, 'string'); assert.strictEqual(typeof callback, 'function'); - var data = [ userId, user.username, user.password, user.email, user.salt, user.createdAt, user.modifiedAt, user.resetToken, user.displayName ]; - database.query('INSERT INTO users (id, username, password, email, salt, createdAt, modifiedAt, resetToken, displayName) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?)', - data, function (error, result) { + var data = [ userId, user.username || null, user.password, user.email, user.salt, user.createdAt, user.modifiedAt, user.resetToken, user.displayName ]; + database.query('INSERT INTO users (id, username, password, email, salt, createdAt, modifiedAt, resetToken, displayName) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?)', data, function (error, result) { if (error && error.code === 'ER_DUP_ENTRY') return callback(new DatabaseError(DatabaseError.ALREADY_EXISTS, error)); if (error || result.affectedRows !== 1) return callback(new DatabaseError(DatabaseError.INTERNAL_ERROR, error)); @@ -165,7 +176,7 @@ function getByAccessToken(accessToken, callback) { 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]); + callback(null, postProcess(result[0])); }); } @@ -186,7 +197,12 @@ function update(userId, user, callback) { var fields = [ ]; for (var k in user) { fields.push(k + ' = ?'); - args.push(user[k]); + + if (k === 'username') { + args.push(user.username || null); + } else { + args.push(user[k]); + } } args.push(userId);