Просмотр исходного кода

[web] Clear hardcoded password in external SP auth (#33597)

registerExternalAuthAdmin() now generates a random password on admin registration.

A migration clears the password for existing installs only in CE/SP

GitOrigin-RevId: 94a82d35dc8cd46915c31fb24f477c19367025eb
Miguel Serrano 2 месяцев назад
Родитель
Сommit
107189cd5f

+ 3 - 2
services/web/modules/launchpad/app/src/LaunchpadController.mjs

@@ -2,6 +2,7 @@ import OError from '@overleaf/o-error'
 import { expressify } from '@overleaf/promise-utils'
 import Settings from '@overleaf/settings'
 import Path from 'node:path'
+import crypto from 'node:crypto'
 import logger from '@overleaf/logger'
 import UserRegistrationHandler from '../../../../app/src/Features/User/UserRegistrationHandler.mjs'
 import EmailHandler from '../../../../app/src/Features/Email/EmailHandler.mjs'
@@ -124,12 +125,12 @@ function registerExternalAuthAdmin(authMethod) {
 
     const body = {
       email,
-      password: 'password_here',
+      password: crypto.randomBytes(32).toString('hex'),
       first_name: email,
       last_name: '',
     }
     logger.debug(
-      { body, authMethod },
+      { email, authMethod },
       'creating admin account for specified external-auth user'
     )
 

+ 30 - 24
services/web/modules/launchpad/test/unit/src/LaunchpadController.test.mjs

@@ -759,14 +759,16 @@ describe('LaunchpadController', function () {
         ctx.UserRegistrationHandler.promises.registerNewUser.callCount.should.equal(
           1
         )
-        ctx.UserRegistrationHandler.promises.registerNewUser
-          .calledWith({
-            email: ctx.email,
-            password: 'password_here',
-            first_name: ctx.email,
-            last_name: '',
-          })
-          .should.equal(true)
+        const call =
+          ctx.UserRegistrationHandler.promises.registerNewUser.firstCall
+        expect(call.args[0]).to.include({
+          email: ctx.email,
+          first_name: ctx.email,
+          last_name: '',
+        })
+        expect(call.args[0].password).to.be.a('string')
+        expect(call.args[0].password).to.not.equal('password_here')
+        expect(call.args[0].password.length).to.be.at.least(16)
       })
 
       it('should have updated the user to make them an admin', function (ctx) {
@@ -964,14 +966,16 @@ describe('LaunchpadController', function () {
         ctx.UserRegistrationHandler.promises.registerNewUser.callCount.should.equal(
           1
         )
-        ctx.UserRegistrationHandler.promises.registerNewUser
-          .calledWith({
-            email: ctx.email,
-            password: 'password_here',
-            first_name: ctx.email,
-            last_name: '',
-          })
-          .should.equal(true)
+        const call =
+          ctx.UserRegistrationHandler.promises.registerNewUser.firstCall
+        expect(call.args[0]).to.include({
+          email: ctx.email,
+          first_name: ctx.email,
+          last_name: '',
+        })
+        expect(call.args[0].password).to.be.a('string')
+        expect(call.args[0].password).to.not.equal('password_here')
+        expect(call.args[0].password.length).to.be.at.least(16)
       })
 
       it('should not call update', function (ctx) {
@@ -1015,14 +1019,16 @@ describe('LaunchpadController', function () {
         ctx.UserRegistrationHandler.promises.registerNewUser.callCount.should.equal(
           1
         )
-        ctx.UserRegistrationHandler.promises.registerNewUser
-          .calledWith({
-            email: ctx.email,
-            password: 'password_here',
-            first_name: ctx.email,
-            last_name: '',
-          })
-          .should.equal(true)
+        const call =
+          ctx.UserRegistrationHandler.promises.registerNewUser.firstCall
+        expect(call.args[0]).to.include({
+          email: ctx.email,
+          first_name: ctx.email,
+          last_name: '',
+        })
+        expect(call.args[0].password).to.be.a('string')
+        expect(call.args[0].password).to.not.equal('password_here')
+        expect(call.args[0].password.length).to.be.at.least(16)
       })
     })
   })

+ 39 - 0
tools/migrations/20260511150000_reset_hardcoded_admin_password.mjs

@@ -0,0 +1,39 @@
+import bcrypt from 'bcrypt'
+import { db } from './lib/mongodb.mjs'
+import { batchedUpdate } from '@overleaf/mongo-utils/batchedUpdate.js'
+import { promiseMapWithLimit } from '@overleaf/promise-utils'
+
+const tags = ['server-ce', 'server-pro']
+
+const HARDCODED_PASSWORD = 'password_here'
+const CONCURRENCY = parseInt(process.env.CONCURRENCY, 10) || 10
+
+const migrate = async () => {
+  await batchedUpdate(
+    db.users,
+    { hashedPassword: { $type: 'string' } },
+    async function (batch) {
+      await promiseMapWithLimit(CONCURRENCY, batch, async user => {
+        const match = await bcrypt.compare(
+          HARDCODED_PASSWORD,
+          user.hashedPassword
+        )
+        if (match) {
+          await db.users.updateOne(
+            { _id: user._id, hashedPassword: user.hashedPassword },
+            { $unset: { hashedPassword: 1 } }
+          )
+        }
+      })
+    },
+    { hashedPassword: 1 }
+  )
+}
+
+const rollback = async () => {}
+
+export default {
+  tags,
+  migrate,
+  rollback,
+}

+ 1 - 0
tools/migrations/package.json

@@ -11,6 +11,7 @@
     "@overleaf/o-error": "workspace:*",
     "@overleaf/promise-utils": "workspace:*",
     "@overleaf/settings": "workspace:*",
+    "bcrypt": "^6.0.0",
     "east": "2.0.3",
     "mongodb": "6.12.0"
   }

+ 1 - 0
yarn.lock

@@ -6770,6 +6770,7 @@ __metadata:
     "@overleaf/o-error": "workspace:*"
     "@overleaf/promise-utils": "workspace:*"
     "@overleaf/settings": "workspace:*"
+    bcrypt: "npm:^6.0.0"
     east: "npm:2.0.3"
     mongodb: "npm:6.12.0"
   languageName: unknown