From e10f33e30f0c69ad74c942ed7f27c5dc60b456c4 Mon Sep 17 00:00:00 2001 From: Daniel Lockyer Date: Thu, 5 May 2022 16:50:27 +0100 Subject: [PATCH] Added `users.status` validation to the schema refs https://github.com/TryGhost/Toolbox/issues/309 - this commit adds a validation array of valid user `status` values to the schema - this also includes a migration to update users with invalid statuses to `inactive`, which I've seen with `invited` and `invited-pending` statuses that pre-dated proper invitations - this also deletes tests that were wrong and written 7 years ago before invites was added --- ...5-09-14-17-cleanup-invalid-users-status.js | 15 +++ core/server/data/schema/schema.js | 21 +++- test/regression/models/model_users.test.js | 100 ------------------ 3 files changed, 32 insertions(+), 104 deletions(-) create mode 100644 core/server/data/migrations/versions/5.0/2022-05-09-14-17-cleanup-invalid-users-status.js diff --git a/core/server/data/migrations/versions/5.0/2022-05-09-14-17-cleanup-invalid-users-status.js b/core/server/data/migrations/versions/5.0/2022-05-09-14-17-cleanup-invalid-users-status.js new file mode 100644 index 0000000000..7b5a257f96 --- /dev/null +++ b/core/server/data/migrations/versions/5.0/2022-05-09-14-17-cleanup-invalid-users-status.js @@ -0,0 +1,15 @@ +const logging = require('@tryghost/logging'); +const {createTransactionalMigration} = require('../../utils'); + +module.exports = createTransactionalMigration( + async function up(knex) { + const affectedRows = await knex('users') + .update('status', 'inactive') + .whereNotIn('status', ['active', 'inactive', 'locked', 'warn-1', 'warn-2', 'warn-3', 'warn-4']); + + logging.info(`Updated ${affectedRows} users with invalid statuses to 'inactive'`); + }, + async function down() { + // no-op: we don't want to revert users back to an invalid status + } +); \ No newline at end of file diff --git a/core/server/data/schema/schema.js b/core/server/data/schema/schema.js index c2bdb26b85..6e538510e8 100644 --- a/core/server/data/schema/schema.js +++ b/core/server/data/schema/schema.js @@ -119,10 +119,23 @@ module.exports = { facebook: {type: 'string', maxlength: 2000, nullable: true}, twitter: {type: 'string', maxlength: 2000, nullable: true}, accessibility: {type: 'text', maxlength: 65535, nullable: true}, - // @TODO: add isIn validation here to control for all possible status values. - // The ones that come up by reviewing the user model are: - // 'active', 'inactive', 'locked', 'warn-1', 'warn-2', 'warn-3', 'warn-4' - status: {type: 'string', maxlength: 50, nullable: false, defaultTo: 'active'}, + status: { + type: 'string', + maxlength: 50, + nullable: false, + defaultTo: 'active', + validations: { + isIn: [[ + 'active', + 'inactive', + 'locked', + 'warn-1', + 'warn-2', + 'warn-3', + 'warn-4' + ]] + } + }, // NOTE: unused at the moment and reserved for future features locale: {type: 'string', maxlength: 6, nullable: true}, visibility: { diff --git a/test/regression/models/model_users.test.js b/test/regression/models/model_users.test.js index 17311bee33..752404cc54 100644 --- a/test/regression/models/model_users.test.js +++ b/test/regression/models/model_users.test.js @@ -231,21 +231,6 @@ describe('User Model', function run() { }); }); - it('can invite user', function (done) { - const userData = testUtils.DataGenerator.forModel.users[4]; - - UserModel.add(_.extend({}, userData, {status: 'invited'}), context).then(function (createdUser) { - should.exist(createdUser); - createdUser.attributes.password.should.not.equal(userData.password, 'password was hashed'); - createdUser.attributes.email.should.eql(userData.email, 'email address correct'); - - Object.keys(eventsTriggered).length.should.eql(1); - should.exist(eventsTriggered['user.added']); - - done(); - }).catch(done); - }); - it('can add active user', function (done) { const userData = testUtils.DataGenerator.forModel.users[4]; @@ -331,91 +316,6 @@ describe('User Model', function run() { done(); }); }); - - it('can edit invited user', function (done) { - const userData = testUtils.DataGenerator.forModel.users[4]; - let userId; - - UserModel.add(_.extend({}, userData, {status: 'invited'}), context).then(function (createdUser) { - should.exist(createdUser); - createdUser.attributes.password.should.not.equal(userData.password, 'password was hashed'); - createdUser.attributes.email.should.eql(userData.email, 'email address correct'); - createdUser.attributes.status.should.equal('invited'); - - userId = createdUser.attributes.id; - - Object.keys(eventsTriggered).length.should.eql(1); - should.exist(eventsTriggered['user.added']); - - return UserModel.edit({website: 'http://some.newurl.com'}, {id: userId}); - }).then(function (createdUser) { - createdUser.attributes.status.should.equal('invited'); - - Object.keys(eventsTriggered).length.should.eql(2); - should.exist(eventsTriggered['user.edited']); - - done(); - }).catch(done); - }); - - it('can activate invited user', function (done) { - const userData = testUtils.DataGenerator.forModel.users[4]; - let userId; - - UserModel.add(_.extend({}, userData, {status: 'invited'}), context).then(function (createdUser) { - should.exist(createdUser); - createdUser.attributes.password.should.not.equal(userData.password, 'password was hashed'); - createdUser.attributes.email.should.eql(userData.email, 'email address correct'); - createdUser.attributes.status.should.equal('invited'); - - userId = createdUser.attributes.id; - - Object.keys(eventsTriggered).length.should.eql(1); - should.exist(eventsTriggered['user.added']); - - return UserModel.edit({status: 'active'}, {id: userId}); - }).then(function (createdUser) { - createdUser.attributes.status.should.equal('active'); - - Object.keys(eventsTriggered).length.should.eql(3); - should.exist(eventsTriggered['user.activated']); - should.exist(eventsTriggered['user.edited']); - - done(); - }).catch(done); - }); - - it('can destroy invited user', function (done) { - const userData = testUtils.DataGenerator.forModel.users[4]; - let userId; - - UserModel.add(_.extend({}, userData, {status: 'invited'}), context).then(function (createdUser) { - should.exist(createdUser); - createdUser.attributes.password.should.not.equal(userData.password, 'password was hashed'); - createdUser.attributes.email.should.eql(userData.email, 'email address correct'); - createdUser.attributes.status.should.equal('invited'); - - userId = {id: createdUser.attributes.id}; - - Object.keys(eventsTriggered).length.should.eql(1); - should.exist(eventsTriggered['user.added']); - - // Destroy the user - return UserModel.destroy(userId); - }).then(function (response) { - response.toJSON().should.be.empty(); - - Object.keys(eventsTriggered).length.should.eql(2); - should.exist(eventsTriggered['user.deleted']); - - // Double check we can't find the user again - return UserModel.findOne(userId); - }).then(function (newResults) { - should.equal(newResults, null); - - done(); - }).catch(done); - }); }); describe('Password change', function () {