From 859b0bd3a8d2a69899dec4389e5aa4c93a03c141 Mon Sep 17 00:00:00 2001 From: Yevhen Date: Mon, 30 Mar 2020 15:29:17 +0200 Subject: [PATCH] fix: Removes references to organisations after deletion (LLC-26) (#1511) --- lib/models/organisation-test.js | 63 +++++ lib/models/organisation.js | 224 +++++++++++------- .../fixtures/organisation.fixtures.js | 6 + .../test-utils/fixtures/user.fixtures.js | 9 + lib/models/test-utils/testModelsHelper.js | 8 + lib/models/uploadSchema.js | 12 + 6 files changed, 231 insertions(+), 91 deletions(-) create mode 100644 lib/models/test-utils/fixtures/organisation.fixtures.js create mode 100644 lib/models/test-utils/fixtures/user.fixtures.js create mode 100644 lib/models/test-utils/testModelsHelper.js diff --git a/lib/models/organisation-test.js b/lib/models/organisation-test.js index a193f34209..d9c993168b 100644 --- a/lib/models/organisation-test.js +++ b/lib/models/organisation-test.js @@ -2,10 +2,73 @@ import chai, { expect } from 'chai'; import chaiAsPromised from 'chai-as-promised'; import { Error } from 'mongoose'; import Organisation from './organisation'; +import User from './user'; +import Role from './role'; +import { createOrganisation, createUser } from './test-utils/testModelsHelper'; +import { userFixture } from './test-utils/fixtures/user.fixtures'; +import { orgFixture } from './test-utils/fixtures/organisation.fixtures'; chai.use(chaiAsPromised); describe('organisation model', () => { + afterEach(async () => { + await Role.deleteMany({}); + await Organisation.deleteMany({}); + await User.deleteMany({}); + }); + + it('should properly create organisation', async () => { + const user = await createUser(userFixture); + const organisation = await createOrganisation(orgFixture); + + const userAfterOrganisationCreated = await User.findOne({ + email: user.email + }); + + const createdUsersOrganisation = userAfterOrganisationCreated + .organisations + .find(id => id.toString() === organisation._id.toString()); + const createdUsersOrganisationSetting = userAfterOrganisationCreated + .organisationSettings + .find(orgSetting => + orgSetting.organisation.toString() === organisation._id.toString() + ); + const createdUsersRoles = await Role.find({ + organisation: organisation._id + }); + + await expect(createdUsersOrganisation).to.not.be.undefined; + await expect(createdUsersOrganisationSetting).to.not.be.undefined; + await expect(createdUsersRoles).to.be.an('array').to.have.lengthOf(2); + }); + + it('should properly delete organisation', async () => { + const user = await createUser(userFixture); + const organisation = await createOrganisation(orgFixture); + + await organisation.remove(); + + const userAfterOrganisationDeleted = await User.findOne({ + email: user.email + }); + + const deletedUsersOrganisation = userAfterOrganisationDeleted + .organisations + .find(id => id.toString() === organisation._id.toString()); + const deletedUsersOrganisationSetting = userAfterOrganisationDeleted + .organisationSettings + .find(orgSetting => + orgSetting.organisation.toString() === organisation._id.toString() + ); + const deletedUsersRoles = await Role.find({ + organisation: organisation._id + }); + + await expect(deletedUsersRoles).to.be.empty; + await expect(deletedUsersOrganisation).to.be.undefined; + await expect(deletedUsersOrganisationSetting).to.be.undefined; + }); + it('should not throw a ValidationError when settings.LOCKOUT_ATTEMPS is larger than 0', async () => { const f = Organisation.create({ settings: { diff --git a/lib/models/organisation.js b/lib/models/organisation.js index 8a4a099348..8a1b17e927 100644 --- a/lib/models/organisation.js +++ b/lib/models/organisation.js @@ -9,7 +9,6 @@ import filterByOrg from 'lib/models/plugins/filterByOrg'; import keys from 'lodash/keys'; import union from 'lodash/union'; import map from 'lodash/map'; -import find from 'lodash/find'; import isUndefined from 'lodash/isUndefined'; import { VIEW_PUBLIC_DASHBOARDS, @@ -22,8 +21,62 @@ import { update$dteTimezoneInDB } from 'lib/helpers/update$dteTimezoneInDB'; let Organisation; +/** + * @typedef {object} ExpirationNotifications + * @property {string} weekBeforeNotificationSent + * @property {string} expirationNotificationSent + */ + +/** + * @typedef {object} UsageStats + * @property {Date} RUN_DATETIME + * @property {Boolean} HAS_CHILDREN + * @property {Number} OWN_COUNT + * @property {Number} TOTAL_COUNT + * @property {Number} OWN_ESTIMATED_BYTES + * @property {Number} TOTAL_ESTIMATED_BYTES + */ + +/** + * @typedef {object} Settings + * @property {Boolean} LOCKOUT_ENABLED + * @property {Number} LOCKOUT_ATTEMPS + * @property {Number} LOCKOUT_SECONDS + * @property {Boolean} PASSWORD_HISTORY_CHECK + * @property {Number} PASSWORD_HISTORY_TOTAL + * @property {Number} PASSWORD_MIN_LENGTH + * @property {Boolean} PASSWORD_REQUIRE_ALPHA + * @property {Boolean} PASSWORD_REQUIRE_NUMBER + * @property {Boolean} PASSWORD_USE_CUSTOM_REGEX + * @property {String} PASSWORD_CUSTOM_REGEX + * @property {String} PASSWORD_CUSTOM_MESSAGE + */ + +/** + * Plain object structure without mongoose model methods + * + * @typedef {object} Organisation + * @property {string} name + * @property {string} logoPath + * @property {UploadSchema} logo TODO: define type + * @property {string} customColors + * @property {Organisation} parent + * @property {*} owner TODO: define type + * @property {Date} expiration + * @property {ExpirationNotifications} expirationNotifications + * @property {UsageStats} usageStats + * @property {Settings} settings + * @property {string} timezone + */ + +/** @typedef {module:mongoose.Model} OrganisationModel */ + +export const EMAIL_NOOP = 'EMAIL_NOOP'; +export const EMAIL_PROCESSING = 'EMAIL_PROCESSING'; +export const EMAIL_SENT = 'EMAIL_SENT'; +export const EMAIL_STATUS = [EMAIL_NOOP, EMAIL_PROCESSING, EMAIL_SENT]; + async function validateLockoutAttempt(value) { - // only validate field if lockout is enabled if (this.settings && this.settings.LOCKOUT_ENABLED) { if (value < 1) { throw new Error('A user should be allowed at least one attempt'); @@ -33,7 +86,6 @@ async function validateLockoutAttempt(value) { } async function validateLockoutSeconds(value) { - // only validate field if lockout is enabled if (this.settings && this.settings.LOCKOUT_ENABLED) { if (value < 5) { throw new Error('Must be at least 5 seconds'); @@ -43,7 +95,6 @@ async function validateLockoutSeconds(value) { } async function validateHistoryTotal(value) { - // only validate field if lockout is enabled if (this.settings && this.settings.LOCKOUT_ENABLED) { if (value < 1) { throw new Error('At least one password must be stored and checked with this setting enabled'); @@ -71,12 +122,6 @@ async function validateMinPasswordLength(value) { return true; } -export const EMAIL_NOOP = 'EMAIL_NOOP'; -export const EMAIL_PROCESSING = 'EMAIL_PROCESSING'; -export const EMAIL_SENT = 'EMAIL_SENT'; - -export const EMAIL_STATUS = [EMAIL_NOOP, EMAIL_PROCESSING, EMAIL_SENT]; - const schema = new mongoose.Schema({ name: { type: String }, logoPath: { type: String }, @@ -105,7 +150,6 @@ const schema = new mongoose.Schema({ }, settings: { LOCKOUT_ENABLED: { type: Boolean, default: true }, - // number of attempts before lock out LOCKOUT_ATTEMPS: { type: Number, default: 5, @@ -113,7 +157,6 @@ const schema = new mongoose.Schema({ validator: validateLockoutAttempt, }, }, - // 30 minute lock out period LOCKOUT_SECONDS: { type: Number, default: 1800, @@ -176,6 +219,11 @@ schema.plugin(auditRemove, { auditName: 'OrganisationAudit' }); +/** + * @param req + * @param res + * @param next + */ schema.statics.readScopeChecks = function runReadScopeChecks(req, res, next) { // we should always be allowed to read orgs return next(); @@ -183,8 +231,8 @@ schema.statics.readScopeChecks = function runReadScopeChecks(req, res, next) { /** * Returns an array of org ids that the given client can view limited to the level - * @param {Object} orgIds ids to start with - * @param {Object} totalIds ids found so far + * @param {Object} stepIds ids to start with + * @param {Object} cumulativeIds ids found so far * @param {Number} level How many levels deep to look for children * - default 0 (you can see your orgs) * @param {Function} cb Callback to be called with the result @@ -210,108 +258,102 @@ schema.statics.limitIdsByOrg = function limitIdsByOrg( } }; -schema.post('init', function handleInit() { - this._original = this.toObject(); -}); - -schema.pre('save', async function preSave(next) { - const User = getConnection().model('User'); +const createRoles = async (orgModel) => { const Role = getConnection().model('Role'); - // https://github.com/Automattic/mongoose/issues/1474 - this.wasNew = this.isNew; + await Role.create({ + title: 'Observer', + owner_id: orgModel.owner, + organisation: orgModel._id, + scopes: [ + VIEW_PUBLIC_DASHBOARDS, + VIEW_PUBLIC_VISUALISATIONS, + VIEW_PUBLIC_JOURNEYS + ] + }); + + return await Role.create({ + title: 'Admin', + owner_id: orgModel.owner, + organisation: orgModel._id, + scopes: [ALL] + }); +}; - const organisation = this; - const objectId = mongoose.Types.ObjectId; +const updateOwner = async (orgModel, User, adminRole) => { + const owner = await User.findById(orgModel.owner); - if (!isUndefined(this._oldExpiration) && this.expiration !== this._oldExpiration) { - this.expirationNotifications.expirationNotificationSent = EMAIL_NOOP; - this.expirationNotifications.weekBeforeNotificationSent = EMAIL_NOOP; + if (!owner) { + return; } - // https://github.com/Automattic/mongoose/issues/1474 - if (organisation.wasNew) { - const createAdminRole = await Role.create({ - title: 'Admin', - owner_id: organisation.owner, - organisation: organisation._id, - scopes: [ALL] - }); - - const createObserverRole = await Role.create({ - title: 'Observer', - owner_id: organisation.owner, - organisation: organisation._id, - scopes: [ - VIEW_PUBLIC_DASHBOARDS, - VIEW_PUBLIC_VISUALISATIONS, - VIEW_PUBLIC_JOURNEYS - ] - }); + owner.organisations = union(owner.organisations || [], [orgModel._id]); + owner.organisationSettings = union( + owner.organisationSettings || [], + [{ + organisation: orgModel._id, + roles: [adminRole._id], + }] + ); - const owner = await User.findById(objectId(organisation.owner)); - if (owner) { - // add this org to the creator's allowed orgs - const newRoles = [createAdminRole._id]; + await owner.save(); +}; - const existingOrganisationSettings = owner.organisationSettings || []; - let updatedSettings = existingOrganisationSettings; +const createDependencies = async (orgModel, User) => { + const adminRole = await createRoles(orgModel); + await updateOwner(orgModel, User, adminRole); +}; - // check if the user has settings for this org - const existingOS = find(existingOrganisationSettings, setting => - setting.organisation.toString() === organisation._id.toString() - ); +const removeDependencies = async (orgModel) => { + const Role = getConnection().model('Role'); + const User = getConnection().model('User'); - if (existingOS) { - const updatedOS = existingOS; - // if the setting already exists, update it - if (updatedOS.roles) { - // union the roles if exist - updatedOS.roles = union(updatedOS.roles, newRoles); - } else { - // set the new roles if no previous roles exist - updatedOS.roles = newRoles; - } - - // loop through to create the updated settings array - updatedSettings = map(updatedSettings, (os) => { - if (os.organisation.toString() === organisation._id.toString()) { - // return the updated settings into the array - return updatedOS; - } - }); - } else { - // insert a new item - updatedSettings.push({ - organisation: organisation._id, - roles: newRoles - }); + await Role.deleteMany({ organisation: [orgModel._id] }); + await User.updateMany( + {}, + { + $pull: { + organisations: orgModel._id, + organisationSettings: { organisation: orgModel._id }, } + } + ); +}; - // update the settings on the owner - owner.organisations = union(owner.organisations, [organisation._id]); - owner.organisationSettings = updatedSettings; - await owner.save(); - } +schema.post('init', function handleInit() { + this._original = this.toObject(); +}); + +schema.pre('save', async function preSave(next) { + const User = getConnection().model('User'); + + if (!isUndefined(this._oldExpiration) && this.expiration !== this._oldExpiration) { + this.expirationNotifications.expirationNotificationSent = EMAIL_NOOP; + this.expirationNotifications.weekBeforeNotificationSent = EMAIL_NOOP; + } - await Promise.all([createAdminRole, createObserverRole]); + if (!isUndefined(this._oldTimezone) && this.timezone !== this._oldTimezone) { + await update$dteTimezoneInDB(this._id, this.timezone); } - // Update users settings await User.updateMany( - { ownerOrganisation: organisation._id }, + { ownerOrganisation: this._id }, { ownerOrganisationSettings: this.settings }, { runValidators: false }, ); - // Update timezone offset of queries - if (!isUndefined(this._oldTimezone) && this.timezone !== this._oldTimezone) { - await update$dteTimezoneInDB(organisation._id, this.timezone); + if (this.isNew) { + await createDependencies(this, User); } next(); }); +schema.post('remove', async (orgModel, next) => { + await removeDependencies(orgModel); + next(); +}); + Organisation = getConnection().model('Organisation', schema, 'organisations'); export default Organisation; diff --git a/lib/models/test-utils/fixtures/organisation.fixtures.js b/lib/models/test-utils/fixtures/organisation.fixtures.js new file mode 100644 index 0000000000..df420bd3c2 --- /dev/null +++ b/lib/models/test-utils/fixtures/organisation.fixtures.js @@ -0,0 +1,6 @@ +import { userFixture } from './user.fixtures'; + +export const orgFixture = { + name: 'test organisation', + owner: userFixture._id +}; diff --git a/lib/models/test-utils/fixtures/user.fixtures.js b/lib/models/test-utils/fixtures/user.fixtures.js new file mode 100644 index 0000000000..5cae7be8a9 --- /dev/null +++ b/lib/models/test-utils/fixtures/user.fixtures.js @@ -0,0 +1,9 @@ +import { ObjectId } from 'mongodb'; + +export const userFixture = { + _id: new ObjectId(), + email: 'test@gmail.com', + scopes: 'site_admin', + password: 'test_password_123', + verified: true +}; diff --git a/lib/models/test-utils/testModelsHelper.js b/lib/models/test-utils/testModelsHelper.js new file mode 100644 index 0000000000..cf7895d06b --- /dev/null +++ b/lib/models/test-utils/testModelsHelper.js @@ -0,0 +1,8 @@ +import User from '../user'; +import Organisation from '../organisation'; + +export const createUser = async user => + await new User(user).save(); + +export const createOrganisation = async organisation => + await new Organisation(organisation).save(); diff --git a/lib/models/uploadSchema.js b/lib/models/uploadSchema.js index 7f1cfcfbf0..4c6ae53c09 100644 --- a/lib/models/uploadSchema.js +++ b/lib/models/uploadSchema.js @@ -1,6 +1,18 @@ import mongoose from 'mongoose'; import timestamps from 'mongoose-timestamp'; +/** + * Plain object structure without mongoose model methods + * + * @typedef {object} UploadSchema + * @property {string} mime + * @property {string} key + * @property {string} repo + */ + +/** + * @type {module:mongoose.Schema} + */ const schema = new mongoose.Schema({ mime: { type: String }, key: { type: String },