Merge pull request #19071 from overleaf/ii-invite-token-remove-token-field

[web] Remove the token field from `projectInvites` collection

GitOrigin-RevId: fe8395e1d1a0ba2daad600b7e3be657f40151a8e

services/web/app/src/Features/Collaborators/CollaboratorsInviteController.js

@@ -220,7 +220,7 @@ const CollaboratorsInviteController = {
     res.sendStatus(204)
   },
 
-  async resendInvite(req, res) {
+  async generateNewInvite(req, res) {
     const projectId = req.params.Project_id
     const inviteId = req.params.invite_id
     const user = SessionManager.getSessionUser(req.session)
@@ -234,7 +234,7 @@ const CollaboratorsInviteController = {
       return res.sendStatus(429)
     }
 
-    const invite = await CollaboratorsInviteHandler.promises.resendInvite(
+    const invite = await CollaboratorsInviteHandler.promises.generateNewInvite(
       projectId,
       sendingUser,
       inviteId
@@ -253,7 +253,7 @@ const CollaboratorsInviteController = {
       )
     }
 
-    res.sendStatus(201)
+    res.status(201).json({ newInviteId: invite._id })
   },
 
   async viewInvite(req, res) {
@@ -394,7 +394,9 @@ module.exports = {
   getAllInvites: expressify(CollaboratorsInviteController.getAllInvites),
   inviteToProject: expressify(CollaboratorsInviteController.inviteToProject),
   revokeInvite: expressify(CollaboratorsInviteController.revokeInvite),
-  resendInvite: expressify(CollaboratorsInviteController.resendInvite),
+  generateNewInvite: expressify(
+    CollaboratorsInviteController.generateNewInvite
+  ),
   viewInvite: expressify(CollaboratorsInviteController.viewInvite),
   acceptInvite: expressify(CollaboratorsInviteController.acceptInvite),
   _checkShouldInviteEmail: callbackify(

services/web/app/src/Features/Collaborators/CollaboratorsInviteHandler.js

@@ -1,4 +1,4 @@
-const { callbackify, promisify } = require('util')
+const { callbackify } = require('util')
 const { ProjectInvite } = require('../../models/ProjectInvite')
 const logger = require('@overleaf/logger')
 const CollaboratorsEmailHandler = require('./CollaboratorsEmailHandler')
@@ -6,16 +6,16 @@ const CollaboratorsHandler = require('./CollaboratorsHandler')
 const CollaboratorsInviteHelper = require('./CollaboratorsInviteHelper')
 const UserGetter = require('../User/UserGetter')
 const ProjectGetter = require('../Project/ProjectGetter')
-const Crypto = require('crypto')
 const NotificationsBuilder = require('../Notifications/NotificationsBuilder')
 const PrivilegeLevels = require('../Authorization/PrivilegeLevels')
-
-const randomBytes = promisify(Crypto.randomBytes)
+const _ = require('lodash')
 
 const CollaboratorsInviteHandler = {
   async getAllInvites(projectId) {
     logger.debug({ projectId }, 'fetching invites for project')
-    const invites = await ProjectInvite.find({ projectId }).exec()
+    const invites = await ProjectInvite.find({ projectId })
+      .select('_id email sendingUserId projectId privileges createdAt expires')
+      .exec()
     logger.debug(
       { projectId, count: invites.length },
       'found invites for project'
@@ -92,25 +92,31 @@ const CollaboratorsInviteHandler = {
       { projectId, sendingUserId: sendingUser._id, email, privileges },
       'adding invite'
     )
-    const buffer = await randomBytes(24)
-    const token = buffer.toString('hex')
+    const token = CollaboratorsInviteHelper.generateToken()
     const tokenHmac = CollaboratorsInviteHelper.hashInviteToken(token)
     let invite = new ProjectInvite({
       email,
-      token,
       tokenHmac,
       sendingUserId: sendingUser._id,
       projectId,
       privileges,
     })
     invite = await invite.save()
+    invite = _.pick(invite.toObject(), [
+      'email',
+      'sendingUserId',
+      'projectId',
+      'privileges',
+      '_id',
+      'createdAt',
+      'expires',
+    ])
 
     // Send email and notification in background
-    CollaboratorsInviteHandler._sendMessages(
-      projectId,
-      sendingUser,
-      invite
-    ).catch(err => {
+    CollaboratorsInviteHandler._sendMessages(projectId, sendingUser, {
+      ...invite,
+      token,
+    }).catch(err => {
       logger.err({ err, projectId, email }, 'error sending messages for invite')
     })
 
@@ -134,25 +140,24 @@ const CollaboratorsInviteHandler = {
     return invite
   },
 
-  async resendInvite(projectId, sendingUser, inviteId) {
-    logger.debug({ projectId, inviteId }, 'resending invite email')
-    const invite = await ProjectInvite.findOne({
-      _id: inviteId,
-      projectId,
-    }).exec()
+  async generateNewInvite(projectId, sendingUser, inviteId) {
+    logger.debug({ projectId, inviteId }, 'generating new invite email')
+    const invite = await this.revokeInvite(projectId, inviteId)
 
     if (invite == null) {
-      logger.warn({ projectId, inviteId }, 'no invite found, nothing to resend')
+      logger.warn(
+        { projectId, inviteId },
+        'no invite found, nothing to generate'
+      )
       return null
     }
 
-    await CollaboratorsInviteHandler._sendMessages(
+    return await this.inviteToProject(
       projectId,
       sendingUser,
-      invite
+      invite.email,
+      invite.privileges
     )
-
-    return invite
   },
 
   async getInviteByToken(projectId, tokenString) {
@@ -199,7 +204,7 @@ module.exports = {
   getInviteCount: callbackify(CollaboratorsInviteHandler.getInviteCount),
   inviteToProject: callbackify(CollaboratorsInviteHandler.inviteToProject),
   revokeInvite: callbackify(CollaboratorsInviteHandler.revokeInvite),
-  resendInvite: callbackify(CollaboratorsInviteHandler.resendInvite),
+  generateNewInvite: callbackify(CollaboratorsInviteHandler.generateNewInvite),
   getInviteByToken: callbackify(CollaboratorsInviteHandler.getInviteByToken),
   acceptInvite: callbackify(CollaboratorsInviteHandler.acceptInvite),
   _trySendInviteNotification: callbackify(

services/web/app/src/Features/Collaborators/CollaboratorsInviteHelper.js

@@ -1,5 +1,10 @@
 const Crypto = require('crypto')
 
+function generateToken() {
+  const buffer = Crypto.randomBytes(24)
+  return buffer.toString('hex')
+}
+
 function hashInviteToken(token) {
   return Crypto.createHmac('sha256', 'overleaf-token-invite')
     .update(token)
@@ -7,5 +12,6 @@ function hashInviteToken(token) {
 }
 
 module.exports = {
+  generateToken,
   hashInviteToken,
 }

services/web/app/src/Features/Collaborators/CollaboratorsRouter.js

@@ -123,7 +123,7 @@ module.exports = {
       }),
       AuthenticationController.requireLogin(),
       AuthorizationMiddleware.ensureUserCanAdminProject,
-      CollaboratorsInviteController.resendInvite
+      CollaboratorsInviteController.generateNewInvite
     )
 
     webRouter.get(

services/web/app/src/models/ProjectInvite.js

@@ -14,7 +14,6 @@ const ExpiryDate = function () {
 const ProjectInviteSchema = new Schema(
   {
     email: String,
-    token: String,
     tokenHmac: String,
     sendingUserId: ObjectId,
     projectId: ObjectId,

services/web/frontend/js/features/share-project-modal/components/invite.jsx

@@ -44,21 +44,34 @@ Invite.propTypes = {
 
 function ResendInvite({ invite }) {
   const { t } = useTranslation()
-  const { monitorRequest } = useShareProjectContext()
-  const { _id: projectId } = useProjectContext()
+  const { updateProject, monitorRequest } = useShareProjectContext()
+  const { _id: projectId, invites } = useProjectContext()
 
   // const buttonRef = useRef(null)
   //
   const handleClick = useCallback(
     () =>
-      monitorRequest(() => resendInvite(projectId, invite)).finally(() => {
-        // NOTE: disabled as react-bootstrap v0.33.1 isn't forwarding the ref to the `button`
-        // if (buttonRef.current) {
-        //   buttonRef.current.blur()
-        // }
-        document.activeElement.blur()
-      }),
-    [invite, monitorRequest, projectId]
+      monitorRequest(() => resendInvite(projectId, invite))
+        .then(({ newInviteId }) => {
+          const updatedInvites = invites.map(existing => {
+            if (existing === invite) {
+              // Update the invitation id for the project
+              existing._id = newInviteId
+            }
+            return existing
+          })
+          updateProject({
+            invites: updatedInvites,
+          })
+        })
+        .finally(() => {
+          // NOTE: disabled as react-bootstrap v0.33.1 isn't forwarding the ref to the `button`
+          // if (buttonRef.current) {
+          //   buttonRef.current.blur()
+          // }
+          document.activeElement.blur()
+        }),
+    [invite, monitorRequest, projectId, invites, updateProject]
   )
 
   return (

services/web/migrations/20240531082910_remove_project_invite_tokens.js

@@ -0,0 +1,15 @@
+/* eslint-disable no-unused-vars */
+
+const Helpers = require('./lib/helpers')
+
+exports.tags = ['server-ce', 'server-pro', 'saas']
+
+exports.migrate = async client => {
+  const { db } = client
+  await Helpers.assertDependency(
+    '20240524135408_add_token_hmac_project_invite_tokens'
+  )
+  await db.projectInvites.updateMany({}, { $unset: { token: 1 } })
+}
+
+exports.rollback = async client => {}

services/web/test/acceptance/src/ProjectInviteTests.js

@@ -3,8 +3,12 @@ const Async = require('async')
 const User = require('./helpers/User')
 const settings = require('@overleaf/settings')
 const CollaboratorsEmailHandler = require('../../../app/src/Features/Collaborators/CollaboratorsEmailHandler')
+const CollaboratorsInviteHelper = require('../../../app/src/Features/Collaborators/CollaboratorsInviteHelper')
 const Features = require('../../../app/src/infrastructure/Features')
 const cheerio = require('cheerio')
+const sinon = require('sinon')
+
+let generateTokenSpy
 
 const createInvite = (sendingUser, projectId, email, callback) => {
   sendingUser.getCsrfToken(err => {
@@ -55,6 +59,8 @@ const createProjectAndInvite = (owner, projectName, email, callback) => {
       if (err) {
         return callback(err)
       }
+      // attach the token to the invite
+      invite.token = generateTokenSpy.getCall(0).returnValue
       const link = CollaboratorsEmailHandler._buildInviteUrl(project, invite)
       callback(null, project, invite, link)
     })
@@ -316,6 +322,9 @@ describe('ProjectInviteTests', function () {
     this.user = new User()
     this.site_admin = new User({ email: `admin+${Math.random()}@example.com` })
     this.email = `smoketestuser+${Math.random()}@example.com`
+
+    generateTokenSpy = sinon.spy(CollaboratorsInviteHelper, 'generateToken')
+
     Async.series(
       [
         cb => this.sendingUser.login(cb),
@@ -340,6 +349,10 @@ describe('ProjectInviteTests', function () {
     )
   })
 
+  afterEach(function () {
+    generateTokenSpy.restore()
+  })
+
   describe('creating invites', function () {
     describe('creating two invites', function () {
       beforeEach(function (done) {

services/web/test/unit/src/Collaborators/CollaboratorsInviteControllerTests.js

@@ -84,7 +84,7 @@ describe('CollaboratorsInviteController', function () {
         getAllInvites: sinon.stub(),
         inviteToProject: sinon.stub().resolves(this.invite),
         getInviteByToken: sinon.stub().resolves(this.invite),
-        resendInvite: sinon.stub().resolves(this.invite),
+        generateNewInvite: sinon.stub().resolves(this.invite),
         revokeInvite: sinon.stub().resolves(this.invite),
         acceptInvite: sinon.stub(),
       },
@@ -1257,7 +1257,7 @@ describe('CollaboratorsInviteController', function () {
     })
   })
 
-  describe('resendInvite', function () {
+  describe('generateNewInvite', function () {
     beforeEach(function () {
       this.req.params = {
         Project_id: this.projectId,
@@ -1268,23 +1268,26 @@ describe('CollaboratorsInviteController', function () {
         .resolves(true)
     })
 
-    describe('when resendInvite does not produce an error', function () {
+    describe('when generateNewInvite does not produce an error', function () {
       beforeEach(function (done) {
         this.res.callback = () => done()
-        this.CollaboratorsInviteController.resendInvite(
+        this.CollaboratorsInviteController.generateNewInvite(
           this.req,
           this.res,
           this.next
         )
       })
 
-      it('should produce a 201 response', function () {
-        this.res.sendStatus.callCount.should.equal(1)
-        this.res.sendStatus.calledWith(201).should.equal(true)
+      it('should produce a 201 response with new invitation id', function () {
+        this.res.status.callCount.should.equal(1)
+        this.res.status.calledWith(201).should.equal(true)
+        expect(this.res.json.firstCall.args[0]).to.deep.equal({
+          newInviteId: this.invite._id,
+        })
       })
 
-      it('should have called resendInvite', function () {
-        this.CollaboratorsInviteHandler.promises.resendInvite.callCount.should.equal(
+      it('should have called generateNewInvite', function () {
+        this.CollaboratorsInviteHandler.promises.generateNewInvite.callCount.should.equal(
           1
         )
       })
@@ -1309,13 +1312,13 @@ describe('CollaboratorsInviteController', function () {
       })
     })
 
-    describe('when resendInvite produces an error', function () {
+    describe('when generateNewInvite produces an error', function () {
       beforeEach(function (done) {
-        this.CollaboratorsInviteHandler.promises.resendInvite.rejects(
+        this.CollaboratorsInviteHandler.promises.generateNewInvite.rejects(
           new Error('woops')
         )
         this.next.callsFake(() => done())
-        this.CollaboratorsInviteController.resendInvite(
+        this.CollaboratorsInviteController.generateNewInvite(
           this.req,
           this.res,
           this.next
@@ -1331,8 +1334,8 @@ describe('CollaboratorsInviteController', function () {
         this.next.calledWith(sinon.match.instanceOf(Error)).should.equal(true)
       })
 
-      it('should have called resendInvite', function () {
-        this.CollaboratorsInviteHandler.promises.resendInvite.callCount.should.equal(
+      it('should have called generateNewInvite', function () {
+        this.CollaboratorsInviteHandler.promises.generateNewInvite.callCount.should.equal(
           1
         )
       })

services/web/test/unit/src/Collaborators/CollaboratorsInviteHandlerTests.js

@@ -43,6 +43,7 @@ describe('CollaboratorsInviteHandler', function () {
     this.NotificationsBuilder = { promises: {} }
     this.tokenHmac = 'jkhajkefhaekjfhkfg'
     this.CollaboratorsInviteHelper = {
+      generateToken: sinon.stub().returns(this.Crypto.randomBytes(24)),
       hashInviteToken: sinon.stub().returns(this.tokenHmac),
     }
 
@@ -85,6 +86,16 @@ describe('CollaboratorsInviteHandler', function () {
       privileges: this.privileges,
       createdAt: new Date(),
     }
+    this.newFakeInvite = {
+      _id: new ObjectId(),
+      email: this.email,
+      token: 'new-token',
+      tokenHmac: 'new-hmac-token',
+      sendingUserId: this.sendingUserId,
+      projectId: this.projectId,
+      privileges: this.privileges,
+      createdAt: new Date(),
+    }
   })
 
   describe('getInviteCount', function () {
@@ -158,6 +169,7 @@ describe('CollaboratorsInviteHandler', function () {
         { _id: new ObjectId(), two: 2 },
       ]
       this.ProjectInvite.find.returns({
+        select: sinon.stub().returnsThis(),
         exec: sinon.stub().resolves(this.fakeInvites),
       })
       this.call = async () => {
@@ -188,6 +200,7 @@ describe('CollaboratorsInviteHandler', function () {
     describe('when ProjectInvite.find produces an error', function () {
       beforeEach(function () {
         this.ProjectInvite.find.returns({
+          select: sinon.stub().returnsThis(),
           exec: sinon.stub().rejects(new Error('woops')),
         })
       })
@@ -201,6 +214,14 @@ describe('CollaboratorsInviteHandler', function () {
   describe('inviteToProject', function () {
     beforeEach(function () {
       this.ProjectInvite.prototype.save.callsFake(async function () {
+        Object.defineProperty(this, 'toObject', {
+          value: function () {
+            return this
+          },
+          writable: true,
+          configurable: true,
+          enumerable: false,
+        })
         return this
       })
       this.CollaboratorsInviteHandler.promises._sendMessages = sinon
@@ -225,8 +246,6 @@ describe('CollaboratorsInviteHandler', function () {
         expect(invite).to.have.all.keys([
           '_id',
           'email',
-          'token',
-          'tokenHmac',
           'sendingUserId',
           'projectId',
           'privileges',
@@ -392,16 +411,16 @@ describe('CollaboratorsInviteHandler', function () {
     })
   })
 
-  describe('resendInvite', function () {
+  describe('generateNewInvite', function () {
     beforeEach(function () {
-      this.ProjectInvite.findOne.returns({
-        exec: sinon.stub().resolves(this.fakeInvite),
-      })
-      this.CollaboratorsInviteHandler.promises._sendMessages = sinon
+      this.CollaboratorsInviteHandler.promises.revokeInvite = sinon
         .stub()
-        .resolves()
+        .resolves(this.fakeInvite)
+      this.CollaboratorsInviteHandler.promises.inviteToProject = sinon
+        .stub()
+        .resolves(this.newFakeInvite)
       this.call = async () => {
-        return await this.CollaboratorsInviteHandler.promises.resendInvite(
+        return await this.CollaboratorsInviteHandler.promises.generateNewInvite(
           this.projectId,
           this.sendingUser,
           this.inviteId
@@ -410,44 +429,51 @@ describe('CollaboratorsInviteHandler', function () {
     })
 
     describe('when all goes well', function () {
-      it('should call ProjectInvite.findOne', async function () {
+      it('should call revokeInvite', async function () {
         await this.call()
-        this.ProjectInvite.findOne.callCount.should.equal(1)
-        this.ProjectInvite.findOne
-          .calledWith({ _id: this.inviteId, projectId: this.projectId })
+        this.CollaboratorsInviteHandler.promises.revokeInvite.callCount.should.equal(
+          1
+        )
+        this.CollaboratorsInviteHandler.promises.revokeInvite
+          .calledWith(this.projectId, this.inviteId)
           .should.equal(true)
       })
 
-      it('should have called _sendMessages', async function () {
+      it('should have called inviteToProject', async function () {
         await this.call()
-        this.CollaboratorsInviteHandler.promises._sendMessages.callCount.should.equal(
+        this.CollaboratorsInviteHandler.promises.inviteToProject.callCount.should.equal(
           1
         )
-        this.CollaboratorsInviteHandler.promises._sendMessages
-          .calledWith(this.projectId, this.sendingUser, this.fakeInvite)
+        this.CollaboratorsInviteHandler.promises.inviteToProject
+          .calledWith(
+            this.projectId,
+            this.sendingUser,
+            this.fakeInvite.email,
+            this.fakeInvite.privileges
+          )
           .should.equal(true)
       })
 
       it('should return the invite', async function () {
         const invite = await this.call()
-        expect(invite).to.deep.equal(this.fakeInvite)
+        expect(invite).to.deep.equal(this.newFakeInvite)
       })
     })
 
-    describe('when findOne produces an error', function () {
+    describe('when revokeInvite produces an error', function () {
       beforeEach(function () {
-        this.ProjectInvite.findOne.returns({
-          exec: sinon.stub().rejects(new Error('woops')),
-        })
+        this.CollaboratorsInviteHandler.promises.revokeInvite = sinon
+          .stub()
+          .rejects(new Error('woops'))
       })
 
       it('should produce an error', async function () {
         await expect(this.call()).to.be.rejectedWith(Error)
       })
 
-      it('should not have called _sendMessages', async function () {
+      it('should not have called inviteToProject', async function () {
         await expect(this.call()).to.be.rejected
-        this.CollaboratorsInviteHandler.promises._sendMessages.callCount.should.equal(
+        this.CollaboratorsInviteHandler.promises.inviteToProject.callCount.should.equal(
           0
         )
       })
@@ -455,14 +481,14 @@ describe('CollaboratorsInviteHandler', function () {
 
     describe('when findOne does not find an invite', function () {
       beforeEach(function () {
-        this.ProjectInvite.findOne.returns({
-          exec: sinon.stub().resolves(null),
-        })
+        this.CollaboratorsInviteHandler.promises.revokeInvite = sinon
+          .stub()
+          .resolves(null)
       })
 
-      it('should not have called _sendMessages', async function () {
+      it('should not have called inviteToProject', async function () {
         await this.call()
-        this.CollaboratorsInviteHandler.promises._sendMessages.callCount.should.equal(
+        this.CollaboratorsInviteHandler.promises.inviteToProject.callCount.should.equal(
           0
         )
       })