[web] throw early in UserCreator.createNewUser with missing analyticsId (#34664)

* [web] throw early in UserCreator.createNewUser with missing analyticsId

* [migrations] rerun back-fill of db.users.analyticsId

GitOrigin-RevId: eabf16326192fca4dbea00ad643cee4315c426c6

services/web/app/src/Features/User/UserCreator.mjs

@@ -54,6 +54,9 @@ async function recordRegistrationEvent(user) {
 }
 
 async function createNewUser(attributes, options = {}) {
+  if (!attributes.analyticsId) {
+    throw new Error('bug: attributes.analyticsId is missing')
+  }
   let user = new User()
 
   if (attributes.first_name == null || attributes.first_name === '') {

services/web/app/src/Features/User/UserRegistrationHandler.mjs

@@ -100,6 +100,7 @@ const UserRegistrationHandler = {
       user = await UserRegistrationHandler.registerNewUser({
         email,
         password: crypto.randomBytes(32).toString('hex'),
+        analyticsId: crypto.randomUUID(),
       })
     } catch (error) {
       if (error.message === 'EmailAlreadyRegistered') {

services/web/modules/launchpad/app/src/LaunchpadController.mjs

@@ -128,6 +128,7 @@ function registerExternalAuthAdmin(authMethod) {
       password: crypto.randomBytes(32).toString('hex'),
       first_name: email,
       last_name: '',
+      analyticsId: crypto.randomUUID(),
     }
     logger.debug(
       { email, authMethod },
@@ -212,7 +213,7 @@ async function registerAdmin(req, res) {
       .json({ message: { type: 'error', text: invalidPassword.message } })
   }
 
-  const body = { email, password }
+  const body = { email, password, analyticsId: crypto.randomUUID() }
 
   const user = await UserRegistrationHandler.promises.registerNewUser(body)
 

services/web/modules/launchpad/test/unit/src/LaunchpadController.test.mjs

@@ -71,6 +71,19 @@ describe('LaunchpadController', function () {
       })
     )
 
+    vi.doMock('crypto', () => ({
+      default: (ctx.crypto = {
+        randomUUID: sinon
+          .stub()
+          .returns('8055c676-bcc7-4e64-a66f-8069f9a0bd92'),
+        randomBytes: sinon.stub().returns({
+          toString: () => {
+            return (ctx.password = 'mock-password123')
+          },
+        }),
+      }),
+    }))
+
     ctx.LaunchpadController = (await import(modulePath)).default
 
     ctx.email = 'bob@smith.com'
@@ -344,7 +357,11 @@ describe('LaunchpadController', function () {
           1
         )
         ctx.UserRegistrationHandler.promises.registerNewUser
-          .calledWith({ email: ctx.email, password: ctx.password })
+          .calledWith({
+            email: ctx.email,
+            password: ctx.password,
+            analyticsId: '8055c676-bcc7-4e64-a66f-8069f9a0bd92',
+          })
           .should.equal(true)
       })
 
@@ -603,7 +620,11 @@ describe('LaunchpadController', function () {
           1
         )
         ctx.UserRegistrationHandler.promises.registerNewUser
-          .calledWith({ email: ctx.email, password: ctx.password })
+          .calledWith({
+            email: ctx.email,
+            password: ctx.password,
+            analyticsId: '8055c676-bcc7-4e64-a66f-8069f9a0bd92',
+          })
           .should.equal(true)
       })
 
@@ -649,7 +670,11 @@ describe('LaunchpadController', function () {
           1
         )
         ctx.UserRegistrationHandler.promises.registerNewUser
-          .calledWith({ email: ctx.email, password: ctx.password })
+          .calledWith({
+            email: ctx.email,
+            password: ctx.password,
+            analyticsId: '8055c676-bcc7-4e64-a66f-8069f9a0bd92',
+          })
           .should.equal(true)
       })
     })
@@ -693,7 +718,11 @@ describe('LaunchpadController', function () {
           1
         )
         ctx.UserRegistrationHandler.promises.registerNewUser
-          .calledWith({ email: ctx.email, password: ctx.password })
+          .calledWith({
+            email: ctx.email,
+            password: ctx.password,
+            analyticsId: '8055c676-bcc7-4e64-a66f-8069f9a0bd92',
+          })
           .should.equal(true)
       })
 

services/web/scripts/devcontainer_setup.mjs

@@ -14,6 +14,7 @@ import OError from '@overleaf/o-error'
 import fs from 'node:fs'
 import Path from 'node:path'
 import { fileURLToPath } from 'node:url'
+import crypto from 'node:crypto'
 
 const MONOREPO = Path.dirname(
   Path.dirname(Path.dirname(Path.dirname(fileURLToPath(import.meta.url))))
@@ -42,6 +43,7 @@ async function createUser(
     const user = await UserRegistrationHandler.promises.registerNewUser({
       email,
       password: PASSWORD,
+      analyticsId: crypto.randomUUID(),
     })
     userId = user._id
   } catch (err) {

services/web/scripts/e2e_test_setup.mjs

@@ -11,6 +11,7 @@ import UserDeleter from '../app/src/Features/User/UserDeleter.mjs'
 import UserRegistrationHandler from '../app/src/Features/User/UserRegistrationHandler.mjs'
 import HistoryManager from '../app/src/Features/History/HistoryManager.mjs'
 import ProjectCreationHandler from '../app/src/Features/Project/ProjectCreationHandler.mjs'
+import crypto from 'node:crypto'
 
 const MONOREPO = Path.dirname(
   Path.dirname(Path.dirname(Path.dirname(fileURLToPath(import.meta.url))))
@@ -24,6 +25,7 @@ async function createUser(email) {
   const user = await UserRegistrationHandler.promises.registerNewUser({
     email,
     password: process.env.CYPRESS_DEFAULT_PASSWORD,
+    analyticsId: crypto.randomUUID(),
   })
   const features = email.startsWith('free+')
     ? Settings.defaultFeatures

services/web/test/acceptance/src/ProjectStructureMongoLockTest.mjs

@@ -20,6 +20,7 @@ import ProjectEntityMongoUpdateHandler from '../../../app/src/Features/Project/P
 import UserCreator from '../../../app/src/Features/User/UserCreator.mjs'
 import { expect } from 'chai'
 import _ from 'lodash'
+import crypto from 'node:crypto'
 
 // These tests are neither acceptance tests nor unit tests. It's difficult to
 // test/verify that our locking is doing what we hope.
@@ -46,6 +47,7 @@ describe('ProjectStructureMongoLock', function () {
       const userDetails = {
         holdingAccount: false,
         email: 'test@example.com',
+        analyticsId: crypto.randomUUID(),
       }
       UserCreator.createNewUser(userDetails, {}, (err, user) => {
         this.user = user

services/web/test/acceptance/src/helpers/UserHelper.mjs

@@ -8,6 +8,7 @@ import UserUpdater from '../../../../app/src/Features/User/UserUpdater.mjs'
 import moment from 'moment'
 import fetch from 'node-fetch'
 import mongodb from 'mongodb-legacy'
+import Crypto from 'node:crypto'
 
 import { UserAuditLogEntry } from '../../../../app/src/models/UserAuditLogEntry.mjs'
 
@@ -248,6 +249,10 @@ class UserHelper {
       delete attributes.password
     }
 
+    if (!attributes.analyticsId) {
+      attributes.analyticsId = Crypto.randomUUID()
+    }
+
     userHelper.user = await UserCreator.promises.createNewUser(attributes)
 
     return userHelper

services/web/test/unit/src/User/UserCreator.test.mjs

@@ -102,13 +102,17 @@ describe('UserCreator', function () {
     ctx.UserCreator = (await import(modulePath)).default
 
     ctx.email = 'bob.oswald@gmail.com'
+    ctx.attributes = {
+      email: ctx.email,
+      analyticsId: '8055c676-bcc7-4e64-a66f-8069f9a0bd92',
+    }
   })
 
   describe('createNewUser', function () {
     describe('with callbacks', function () {
       it('should take the opts and put them in the model', async function (ctx) {
         const user = await ctx.UserCreator.promises.createNewUser({
-          email: ctx.email,
+          ...ctx.attributes,
           holdingAccount: true,
         })
         assert.equal(user.email, ctx.email)
@@ -118,7 +122,7 @@ describe('UserCreator', function () {
 
       it('should use the start of the email if the first name is empty string', async function (ctx) {
         const user = await ctx.UserCreator.promises.createNewUser({
-          email: ctx.email,
+          ...ctx.attributes,
           holdingAccount: true,
           first_name: '',
         })
@@ -129,7 +133,7 @@ describe('UserCreator', function () {
 
       it('should use the first name if passed', async function (ctx) {
         const user = await ctx.UserCreator.promises.createNewUser({
-          email: ctx.email,
+          ...ctx.attributes,
           holdingAccount: true,
           first_name: 'fiiirstname',
         })
@@ -140,7 +144,7 @@ describe('UserCreator', function () {
 
       it('should use the last name if passed', async function (ctx) {
         const user = await ctx.UserCreator.promises.createNewUser({
-          email: ctx.email,
+          ...ctx.attributes,
           holdingAccount: true,
           last_name: 'lastNammmmeee',
         })
@@ -150,9 +154,9 @@ describe('UserCreator', function () {
       })
 
       it('should set emails attribute', async function (ctx) {
-        const user = await ctx.UserCreator.promises.createNewUser({
-          email: ctx.email,
-        })
+        const user = await ctx.UserCreator.promises.createNewUser(
+          ctx.attributes
+        )
         user.email.should.equal(ctx.email)
         user.emails.length.should.equal(1)
         user.emails[0].email.should.equal(ctx.email)
@@ -161,9 +165,8 @@ describe('UserCreator', function () {
       })
 
       describe('with affiliations feature', function () {
-        let attributes, user
+        let user
         beforeEach(function (ctx) {
-          attributes = { email: ctx.email }
           ctx.Features.hasFeature = sinon
             .stub()
             .withArgs('affiliations')
@@ -172,7 +175,7 @@ describe('UserCreator', function () {
 
         describe('when v1 affiliations API does not return an error', function () {
           beforeEach(async function (ctx) {
-            user = await ctx.UserCreator.promises.createNewUser(attributes)
+            user = await ctx.UserCreator.promises.createNewUser(ctx.attributes)
           })
 
           it('should flag that affiliation is unchecked', function () {
@@ -198,7 +201,7 @@ describe('UserCreator', function () {
         describe('when v1 affiliations API does return an error', function () {
           beforeEach(async function (ctx) {
             ctx.UserUpdater.promises.addAffiliationForNewUser.rejects()
-            user = await ctx.UserCreator.promises.createNewUser(attributes)
+            user = await ctx.UserCreator.promises.createNewUser(ctx.attributes)
           })
 
           it('should flag that affiliation is unchecked', function () {
@@ -228,7 +231,7 @@ describe('UserCreator', function () {
         describe('when v1 affiliations API returns an error and requireAffiliation=true', function () {
           beforeEach(async function (ctx) {
             ctx.UserUpdater.promises.addAffiliationForNewUser.rejects()
-            user = await ctx.UserCreator.promises.createNewUser(attributes)
+            user = await ctx.UserCreator.promises.createNewUser(ctx.attributes)
           })
 
           it('should flag that affiliation is unchecked', function () {
@@ -257,8 +260,7 @@ describe('UserCreator', function () {
       })
 
       it('should not add affiliation when without affiliation feature', async function (ctx) {
-        const attributes = { email: ctx.email }
-        await ctx.UserCreator.promises.createNewUser(attributes)
+        await ctx.UserCreator.promises.createNewUser(ctx.attributes)
         sinon.assert.notCalled(
           ctx.UserUpdater.promises.addAffiliationForNewUser
         )
@@ -268,7 +270,7 @@ describe('UserCreator', function () {
     describe('with promises', function () {
       it('should take the opts and put them in the model', async function (ctx) {
         const opts = {
-          email: ctx.email,
+          ...ctx.attributes,
           holdingAccount: true,
         }
         const user = await ctx.UserCreator.promises.createNewUser(opts)
@@ -282,8 +284,9 @@ describe('UserCreator', function () {
           .stub()
           .withArgs('affiliations')
           .returns(true)
-        const attributes = { email: ctx.email }
-        const user = await ctx.UserCreator.promises.createNewUser(attributes)
+        const user = await ctx.UserCreator.promises.createNewUser(
+          ctx.attributes
+        )
         sinon.assert.calledOnce(
           ctx.UserUpdater.promises.addAffiliationForNewUser
         )
@@ -296,8 +299,7 @@ describe('UserCreator', function () {
 
       it('should not add affiliation when without affiliation feature', async function (ctx) {
         ctx.Features.hasFeature = sinon.stub().returns(false)
-        const attributes = { email: ctx.email }
-        await ctx.UserCreator.promises.createNewUser(attributes)
+        await ctx.UserCreator.promises.createNewUser(ctx.attributes)
         sinon.assert.notCalled(
           ctx.UserUpdater.promises.addAffiliationForNewUser
         )
@@ -305,7 +307,7 @@ describe('UserCreator', function () {
 
       it('should include SAML provider ID with email', async function (ctx) {
         const attributes = {
-          email: ctx.email,
+          ...ctx.attributes,
           samlIdentifiers: [{ email: ctx.email, providerId: '1' }],
         }
         const user = await ctx.UserCreator.promises.createNewUser(attributes)
@@ -313,9 +315,9 @@ describe('UserCreator', function () {
       })
 
       it('should fire an analytics event and user property on registration', async function (ctx) {
-        const user = await ctx.UserCreator.promises.createNewUser({
-          email: ctx.email,
-        })
+        const user = await ctx.UserCreator.promises.createNewUser(
+          ctx.attributes
+        )
         assert.equal(user.email, ctx.email)
         sinon.assert.calledWith(
           ctx.Analytics.recordEventForUserInBackground,
@@ -331,9 +333,9 @@ describe('UserCreator', function () {
 
       it('should schedule post registration jobs on registration with saas feature', async function (ctx) {
         ctx.Features.hasFeature = sinon.stub().withArgs('saas').returns(true)
-        const user = await ctx.UserCreator.promises.createNewUser({
-          email: ctx.email,
-        })
+        const user = await ctx.UserCreator.promises.createNewUser(
+          ctx.attributes
+        )
         assert.equal(user.email, ctx.email)
         sinon.assert.calledWith(
           ctx.UserOnboardingEmailManager.scheduleOnboardingEmail,
@@ -347,8 +349,7 @@ describe('UserCreator', function () {
       })
 
       it('should not schedule post registration checks when without saas feature', async function (ctx) {
-        const attributes = { email: ctx.email }
-        await ctx.UserCreator.promises.createNewUser(attributes)
+        await ctx.UserCreator.promises.createNewUser(ctx.attributes)
         sinon.assert.notCalled(
           ctx.UserOnboardingEmailManager.scheduleOnboardingEmail
         )
@@ -357,6 +358,14 @@ describe('UserCreator', function () {
             .schedulePostRegistrationAnalytics
         )
       })
+
+      it('should throw without analyticsId', async function (ctx) {
+        await expect(
+          ctx.UserCreator.promises.createNewUser({ email: ctx.email })
+        ).to.eventually.be.rejectedWith(
+          /bug: attributes.analyticsId is missing/
+        )
+      })
     })
   })
 })

services/web/test/unit/src/User/UserRegistrationHandler.test.mjs

@@ -68,7 +68,11 @@ describe('UserRegistrationHandler', function () {
     }))
 
     vi.doMock('crypto', () => ({
-      default: (ctx.crypto = {}),
+      default: (ctx.crypto = {
+        randomUUID: sinon
+          .stub()
+          .returns('8055c676-bcc7-4e64-a66f-8069f9a0bd92'),
+      }),
     }))
 
     vi.doMock('../../../../app/src/Features/Email/EmailHandler', () => ({
@@ -287,6 +291,7 @@ describe('UserRegistrationHandler', function () {
         sinon.assert.calledWith(ctx.handler.promises.registerNewUser, {
           email: ctx.email,
           password: ctx.password,
+          analyticsId: '8055c676-bcc7-4e64-a66f-8069f9a0bd92',
         })
       })
 

tools/migrations/20260616070000_back_fill_users_analyticsId.mjs

@@ -0,0 +1,24 @@
+import { db } from './lib/mongodb.mjs'
+import { batchedUpdate } from '@overleaf/mongo-utils/batchedUpdate.js'
+
+const tags = ['saas', 'server-ce', 'server-pro', 'nonblocking']
+
+const migrate = async () => {
+  await batchedUpdate(db.users, { analyticsId: { $exists: false } }, [
+    { $set: { analyticsId: { $toString: '$_id' } } },
+  ])
+}
+
+const rollback = async () => {
+  await batchedUpdate(
+    db.users,
+    { $expr: { $eq: [{ $strLenCP: '$analyticsId' }, 24] } },
+    { $unset: { analyticsId: 1 } }
+  )
+}
+
+export default {
+  tags,
+  migrate,
+  rollback,
+}