From 92da38bb6b28533a11a7992081e745d46a4b51f7 Mon Sep 17 00:00:00 2001 From: James Allen Date: Mon, 30 Jan 2017 14:36:06 +0100 Subject: [PATCH 1/6] Add migration to remove holding accounts --- .../5_remove_holding_accounts.coffee | 53 +++++++++++++++++++ server-ce/migrations/about_migrations.md | 11 +++- 2 files changed, 62 insertions(+), 2 deletions(-) create mode 100644 server-ce/migrations/5_remove_holding_accounts.coffee diff --git a/server-ce/migrations/5_remove_holding_accounts.coffee b/server-ce/migrations/5_remove_holding_accounts.coffee new file mode 100644 index 0000000000..c9ab2a041b --- /dev/null +++ b/server-ce/migrations/5_remove_holding_accounts.coffee @@ -0,0 +1,53 @@ +Settings = require "settings-sharelatex" +mongojs = require("mongojs") +ObjectId = mongojs.ObjectId +db = mongojs(Settings.mongo.url, ['users', 'projects']) +async = require "async" + +findHoldingAccounts = (callback = (error, users) ->) -> + db.users.find({holdingAccount: true}, {holdingAccount: 1, email: 1}, callback) + +deleteUserProjects = (user_id, callback = (error) ->) -> + # Holding accounts can't own projects, so only remove from + # collaberator_refs and readOnly_refs + console.log "[Removing user from projects]", user_id + db.projects.update { + $or: [ + {collaberator_refs: user_id}, + {readOnly_refs: user_id} + ] + }, { + $pull: { + collaberator_refs: user_id, + readOnly_refs: user_id + } + }, { + multi: true + }, (error, result) -> + console.log "[Removed user from projects]", user_id, result + callback(error) + +deleteUser = (user_id, callback = (error) ->) -> + if !user_id? + throw new Error("must have user_id") + console.log "[Removing user]", user_id + db.users.remove {_id: user_id}, (error, result) -> + console.log "[Removed user]", user_id, result + callback(error) + +exports.migrate = (client, done=()->) -> + findHoldingAccounts (error, users) -> + throw error if error? + console.log "[Got list of holding accounts]", users.map (u) -> u._id + jobs = users.map (u) -> + (cb) -> + deleteUserProjects u._id, (error) -> + return cb(error) if error? + deleteUser u._id, cb + async.series jobs, (error) -> + throw error if error? + console.log "[Removed holding accounts]" + done() + +exports.rollback = (client, done) -> + done() diff --git a/server-ce/migrations/about_migrations.md b/server-ce/migrations/about_migrations.md index 146cb9c047..97f91442b1 100644 --- a/server-ce/migrations/about_migrations.md +++ b/server-ce/migrations/about_migrations.md @@ -1,2 +1,9 @@ -* if migration is stopped mid way it will start at the beginging next time -* to see the run migrations do db.getCollection('_migrations').find() you can't do db._migrations.find() \ No newline at end of file +If migration is stopped mid way it will start at the beginging next time + +To see the run migrations do db.getCollection('_migrations').find() you can't do db._migrations.find() + +When testing, to roll back a migration run: + +``` +./node_modules/east/bin/east rollback 5 --adapter east-mongo --url mongodb://localhost:27017/sharelatex +``` From a53939827592b8b4dc9a5169e9601638774d4b4c Mon Sep 17 00:00:00 2001 From: James Allen Date: Mon, 30 Jan 2017 15:40:31 +0100 Subject: [PATCH 2/6] Add in dry run option for holding account migration and log out removed users --- .../5_remove_holding_accounts.coffee | 116 +++++++++++------- 1 file changed, 73 insertions(+), 43 deletions(-) diff --git a/server-ce/migrations/5_remove_holding_accounts.coffee b/server-ce/migrations/5_remove_holding_accounts.coffee index c9ab2a041b..a4afe59582 100644 --- a/server-ce/migrations/5_remove_holding_accounts.coffee +++ b/server-ce/migrations/5_remove_holding_accounts.coffee @@ -4,50 +4,80 @@ ObjectId = mongojs.ObjectId db = mongojs(Settings.mongo.url, ['users', 'projects']) async = require "async" -findHoldingAccounts = (callback = (error, users) ->) -> - db.users.find({holdingAccount: true}, {holdingAccount: 1, email: 1}, callback) +module.exports = HoldingAccountMigration = + DRY_RUN: true -deleteUserProjects = (user_id, callback = (error) ->) -> - # Holding accounts can't own projects, so only remove from - # collaberator_refs and readOnly_refs - console.log "[Removing user from projects]", user_id - db.projects.update { - $or: [ - {collaberator_refs: user_id}, - {readOnly_refs: user_id} - ] - }, { - $pull: { - collaberator_refs: user_id, - readOnly_refs: user_id - } - }, { - multi: true - }, (error, result) -> - console.log "[Removed user from projects]", user_id, result - callback(error) + findHoldingAccounts: (callback = (error, users) ->) -> + db.users.find({holdingAccount: true}, {holdingAccount: 1, email: 1}, callback) -deleteUser = (user_id, callback = (error) ->) -> - if !user_id? - throw new Error("must have user_id") - console.log "[Removing user]", user_id - db.users.remove {_id: user_id}, (error, result) -> - console.log "[Removed user]", user_id, result - callback(error) + deleteUserProjects: (user_id, callback = (error) ->) -> + # Holding accounts can't own projects, so only remove from + # collaberator_refs and readOnly_refs + console.log "[Removing user from projects]", user_id + db.projects.find { + $or: [ + {collaberator_refs: user_id}, + {readOnly_refs: user_id} + ] + }, { collaberator_refs: 1, readOnly_refs: 1 }, (error, projects = []) -> + return callback(error) if error? + jobs = projects.map (project) -> + (cb) -> + console.log "[Removing user from project]", user_id, JSON.stringify(project) + if !project._id? + throw new Error("no project id") + + if !HoldingAccountMigration.DRY_RUN + db.projects.update { + _id: project._id + }, { + $pull: { + collaberator_refs: user_id, + readOnly_refs: user_id + } + }, (error, result) -> + return cb(error) if error? + console.log "[Removed user from project]", user_id, project._id, result + cb() + else + console.log "[Would have removed user from project]", user_id, project._id + cb() + + async.series jobs, callback -exports.migrate = (client, done=()->) -> - findHoldingAccounts (error, users) -> - throw error if error? - console.log "[Got list of holding accounts]", users.map (u) -> u._id - jobs = users.map (u) -> - (cb) -> - deleteUserProjects u._id, (error) -> - return cb(error) if error? - deleteUser u._id, cb - async.series jobs, (error) -> + deleteUser: (user_id, callback = (error) ->) -> + if !user_id? + throw new Error("must have user_id") + db.users.find {_id: user_id}, (error, user) -> + return callback(error) if error? + if !user? + throw new Error("expected user") + console.log "[Removing user]", user_id, JSON.stringify(user) + if !HoldingAccountMigration.DRY_RUN + db.users.remove {_id: user_id}, (error, result) -> + console.log "[Removed user]", user_id, result + callback(error) + else + console.log "[Would have removed user]", user_id + callback() + + run: (done) -> + HoldingAccountMigration.findHoldingAccounts (error, users) -> throw error if error? - console.log "[Removed holding accounts]" - done() - -exports.rollback = (client, done) -> - done() + console.log "[Got list of holding accounts]", users.map (u) -> u._id + jobs = users.map (u) -> + (cb) -> + HoldingAccountMigration.deleteUserProjects u._id, (error) -> + return cb(error) if error? + HoldingAccountMigration.deleteUser u._id, cb + async.series jobs, (error) -> + throw error if error? + console.log "[Removed holding accounts]" + done() + + migrate: (client, done=()->) -> + HoldingAccountMigration.DRY_RUN = false + HoldingAccountMigration.run(done) + + rollback: (client, done) -> + done() From 303dcba51b3ff8f7d376a7b3721274d5c2efa518 Mon Sep 17 00:00:00 2001 From: James Allen Date: Thu, 23 Mar 2017 11:34:45 +0000 Subject: [PATCH 3/6] Address Henry's comments about robustness --- .../5_remove_holding_accounts.coffee | 32 +++++++++---------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/server-ce/migrations/5_remove_holding_accounts.coffee b/server-ce/migrations/5_remove_holding_accounts.coffee index a4afe59582..1b503656c6 100644 --- a/server-ce/migrations/5_remove_holding_accounts.coffee +++ b/server-ce/migrations/5_remove_holding_accounts.coffee @@ -24,7 +24,7 @@ module.exports = HoldingAccountMigration = jobs = projects.map (project) -> (cb) -> console.log "[Removing user from project]", user_id, JSON.stringify(project) - if !project._id? + if !project?._id? throw new Error("no project id") if !HoldingAccountMigration.DRY_RUN @@ -48,31 +48,31 @@ module.exports = HoldingAccountMigration = deleteUser: (user_id, callback = (error) ->) -> if !user_id? throw new Error("must have user_id") - db.users.find {_id: user_id}, (error, user) -> - return callback(error) if error? - if !user? - throw new Error("expected user") - console.log "[Removing user]", user_id, JSON.stringify(user) - if !HoldingAccountMigration.DRY_RUN - db.users.remove {_id: user_id}, (error, result) -> - console.log "[Removed user]", user_id, result - callback(error) - else - console.log "[Would have removed user]", user_id + if !HoldingAccountMigration.DRY_RUN + db.users.remove {_id: user_id, holdingAccount: true}, (error, result) -> + return callback(error) if error? + console.log "[Removed user]", user_id, result + if result.n != 1 + return callback(new Error("failed to remove user as expected")) callback() + else + console.log "[Would have removed user]", user_id + callback() - run: (done) -> + run: (done = () ->) -> HoldingAccountMigration.findHoldingAccounts (error, users) -> throw error if error? console.log "[Got list of holding accounts]", users.map (u) -> u._id jobs = users.map (u) -> (cb) -> - HoldingAccountMigration.deleteUserProjects u._id, (error) -> + HoldingAccountMigration.deleteUser u._id, (error) -> return cb(error) if error? - HoldingAccountMigration.deleteUser u._id, cb + HoldingAccountMigration.deleteUserProjects u._id, (error) -> + return cb(error) if error? + setTimeout cb, 200 # Small delay to not hammer DB async.series jobs, (error) -> throw error if error? - console.log "[Removed holding accounts]" + console.log "[FINISHED]" done() migrate: (client, done=()->) -> From 51dd24574a4f7bb73bf6c7b33ea569c7afe44a81 Mon Sep 17 00:00:00 2001 From: James Allen Date: Thu, 23 Mar 2017 11:50:46 +0000 Subject: [PATCH 4/6] Improve logging --- server-ce/migrations/5_remove_holding_accounts.coffee | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/server-ce/migrations/5_remove_holding_accounts.coffee b/server-ce/migrations/5_remove_holding_accounts.coffee index 1b503656c6..687b1ee3b7 100644 --- a/server-ce/migrations/5_remove_holding_accounts.coffee +++ b/server-ce/migrations/5_remove_holding_accounts.coffee @@ -60,16 +60,17 @@ module.exports = HoldingAccountMigration = callback() run: (done = () ->) -> + console.log "[Getting list of holding accounts]" HoldingAccountMigration.findHoldingAccounts (error, users) -> throw error if error? - console.log "[Got list of holding accounts]", users.map (u) -> u._id + console.log "[Got #{users.length} holding accounts]" jobs = users.map (u) -> (cb) -> HoldingAccountMigration.deleteUser u._id, (error) -> return cb(error) if error? HoldingAccountMigration.deleteUserProjects u._id, (error) -> return cb(error) if error? - setTimeout cb, 200 # Small delay to not hammer DB + setTimeout cb, 50 # Small delay to not hammer DB async.series jobs, (error) -> throw error if error? console.log "[FINISHED]" From ba3333303cde4c94bc748a6db8abaccdd98a0f95 Mon Sep 17 00:00:00 2001 From: James Allen Date: Thu, 23 Mar 2017 11:56:46 +0000 Subject: [PATCH 5/6] More loggin --- server-ce/migrations/5_remove_holding_accounts.coffee | 2 ++ 1 file changed, 2 insertions(+) diff --git a/server-ce/migrations/5_remove_holding_accounts.coffee b/server-ce/migrations/5_remove_holding_accounts.coffee index 687b1ee3b7..3e45d41605 100644 --- a/server-ce/migrations/5_remove_holding_accounts.coffee +++ b/server-ce/migrations/5_remove_holding_accounts.coffee @@ -64,8 +64,10 @@ module.exports = HoldingAccountMigration = HoldingAccountMigration.findHoldingAccounts (error, users) -> throw error if error? console.log "[Got #{users.length} holding accounts]" + i = 0 jobs = users.map (u) -> (cb) -> + console.log "[Removing user #{i++}/#{users.length}]" HoldingAccountMigration.deleteUser u._id, (error) -> return cb(error) if error? HoldingAccountMigration.deleteUserProjects u._id, (error) -> From 930aa9c5724ddc9ae45a62e0d99f9a37bb31a5a9 Mon Sep 17 00:00:00 2001 From: James Allen Date: Fri, 24 Mar 2017 13:46:31 +0000 Subject: [PATCH 6/6] Don't find holding accounts with a hashed password --- server-ce/migrations/5_remove_holding_accounts.coffee | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server-ce/migrations/5_remove_holding_accounts.coffee b/server-ce/migrations/5_remove_holding_accounts.coffee index 3e45d41605..494669859d 100644 --- a/server-ce/migrations/5_remove_holding_accounts.coffee +++ b/server-ce/migrations/5_remove_holding_accounts.coffee @@ -8,7 +8,7 @@ module.exports = HoldingAccountMigration = DRY_RUN: true findHoldingAccounts: (callback = (error, users) ->) -> - db.users.find({holdingAccount: true}, {holdingAccount: 1, email: 1}, callback) + db.users.find({holdingAccount: true, hashedPassword: { $exists: false }}, {holdingAccount: 1, email: 1}, callback) deleteUserProjects: (user_id, callback = (error) ->) -> # Holding accounts can't own projects, so only remove from