From dd906df7b796d01ad9d398e57248e6eb8c26f6f8 Mon Sep 17 00:00:00 2001 From: Jakob Ackermann Date: Thu, 16 Feb 2023 10:41:58 +0000 Subject: [PATCH] Merge pull request #11360 from overleaf/jpa-encrypt-2fa-secret [web] two-factor-authentication: encrypt the secret in the db GitOrigin-RevId: 86642e13d917b239012229f685ad0210039a6706 --- services/web/app/src/models/User.js | 2 +- .../20230124092607_clear_old_2fa_setup.js | 30 ++++++++++++++ services/web/scripts/helpers/batchedUpdate.js | 39 ++++++++++++------- .../acceptance/config/settings.test.saas.js | 8 ++++ 4 files changed, 64 insertions(+), 15 deletions(-) create mode 100644 services/web/migrations/20230124092607_clear_old_2fa_setup.js diff --git a/services/web/app/src/models/User.js b/services/web/app/src/models/User.js index 16c2698f1f..732c1a8e30 100644 --- a/services/web/app/src/models/User.js +++ b/services/web/app/src/models/User.js @@ -167,7 +167,7 @@ const UserSchema = new Schema({ twoFactorAuthentication: { createdAt: { type: Date }, enrolledAt: { type: Date }, - secret: { type: String }, + secretEncrypted: { type: String }, }, onboardingEmailSentAt: { type: Date }, splitTests: Schema.Types.Mixed, diff --git a/services/web/migrations/20230124092607_clear_old_2fa_setup.js b/services/web/migrations/20230124092607_clear_old_2fa_setup.js new file mode 100644 index 0000000000..d72eeb12a5 --- /dev/null +++ b/services/web/migrations/20230124092607_clear_old_2fa_setup.js @@ -0,0 +1,30 @@ +const { batchedUpdate } = require('../scripts/helpers/batchedUpdate') + +exports.tags = ['saas'] + +const batchedUpdateOptions = { + VERBOSE_LOGGING: 'true', + BATCH_SIZE: '1', +} + +exports.migrate = async () => { + await batchedUpdate( + 'users', + { 'twoFactorAuthentication.secret': { $exists: true } }, + { $unset: { twoFactorAuthentication: true } }, + null, + null, + batchedUpdateOptions + ) +} + +exports.rollback = async () => { + await batchedUpdate( + 'users', + { 'twoFactorAuthentication.secretEncrypted': { $exists: true } }, + { $unset: { twoFactorAuthentication: true } }, + null, + null, + batchedUpdateOptions + ) +} diff --git a/services/web/scripts/helpers/batchedUpdate.js b/services/web/scripts/helpers/batchedUpdate.js index 5100186368..7b21d3ba07 100644 --- a/services/web/scripts/helpers/batchedUpdate.js +++ b/services/web/scripts/helpers/batchedUpdate.js @@ -1,18 +1,27 @@ const { ReadPreference, ObjectId } = require('mongodb') const { db, waitForDb } = require('../../app/src/infrastructure/mongodb') -const BATCH_DESCENDING = process.env.BATCH_DESCENDING === 'true' -const BATCH_SIZE = parseInt(process.env.BATCH_SIZE, 10) || 1000 -const VERBOSE_LOGGING = process.env.VERBOSE_LOGGING === 'true' +let BATCH_DESCENDING +let BATCH_SIZE +let VERBOSE_LOGGING let BATCH_LAST_ID -if (process.env.BATCH_LAST_ID) { - BATCH_LAST_ID = ObjectId(process.env.BATCH_LAST_ID) -} else if (process.env.BATCH_RANGE_START) { - BATCH_LAST_ID = ObjectId(process.env.BATCH_RANGE_START) -} let BATCH_RANGE_END -if (process.env.BATCH_RANGE_END) { - BATCH_RANGE_END = ObjectId(process.env.BATCH_RANGE_END) +refreshGlobalOptionsForBatchedUpdate() + +function refreshGlobalOptionsForBatchedUpdate(options = {}) { + options = Object.assign({}, options, process.env) + + BATCH_DESCENDING = options.BATCH_DESCENDING === 'true' + BATCH_SIZE = parseInt(options.BATCH_SIZE, 10) || 1000 + VERBOSE_LOGGING = options.VERBOSE_LOGGING === 'true' + if (options.BATCH_LAST_ID) { + BATCH_LAST_ID = ObjectId(options.BATCH_LAST_ID) + } else if (options.BATCH_RANGE_START) { + BATCH_LAST_ID = ObjectId(options.BATCH_RANGE_START) + } + if (options.BATCH_RANGE_END) { + BATCH_RANGE_END = ObjectId(options.BATCH_RANGE_END) + } } async function getNextBatch(collection, query, maxId, projection, options) { @@ -56,13 +65,15 @@ async function batchedUpdate( query, update, projection, - options + findOptions, + batchedUpdateOptions ) { + refreshGlobalOptionsForBatchedUpdate(batchedUpdateOptions) await waitForDb() const collection = db[collectionName] - options = options || {} - options.readPreference = ReadPreference.SECONDARY + findOptions = findOptions || {} + findOptions.readPreference = ReadPreference.SECONDARY projection = projection || { _id: 1 } let nextBatch @@ -74,7 +85,7 @@ async function batchedUpdate( query, maxId, projection, - options + findOptions )).length ) { maxId = nextBatch[nextBatch.length - 1]._id diff --git a/services/web/test/acceptance/config/settings.test.saas.js b/services/web/test/acceptance/config/settings.test.saas.js index a1201883a0..d9f46081d1 100644 --- a/services/web/test/acceptance/config/settings.test.saas.js +++ b/services/web/test/acceptance/config/settings.test.saas.js @@ -56,6 +56,14 @@ const overrides = { // Disable contentful module. contentful: undefined, + + twoFactorAuthentication: { + accessTokenEncryptorOptions: { + cipherPasswords: { + '2023.1-v3': 'this-is-just-a-test-secret', + }, + }, + }, } module.exports = baseApp.mergeWith(baseTest.mergeWith(overrides))