Merge pull request #19626 from overleaf/ii-invite-remove-unncecessary-fields

[web] Clean up props exposed to the /invite and /invites endpoints

GitOrigin-RevId: dc4163e04d5785e141d5e322bfb0a626e67d637f

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

@@ -14,7 +14,7 @@ const CollaboratorsInviteHandler = {
   async getAllInvites(projectId) {
     logger.debug({ projectId }, 'fetching invites for project')
     const invites = await ProjectInvite.find({ projectId })
-      .select('_id email sendingUserId projectId privileges createdAt expires')
+      .select('_id email privileges')
       .exec()
     logger.debug(
       { projectId, count: invites.length },
@@ -102,15 +102,7 @@ const CollaboratorsInviteHandler = {
       privileges,
     })
     invite = await invite.save()
-    invite = _.pick(invite.toObject(), [
-      'email',
-      'sendingUserId',
-      'projectId',
-      'privileges',
-      '_id',
-      'createdAt',
-      'expires',
-    ])
+    invite = invite.toObject()
 
     // Send email and notification in background
     CollaboratorsInviteHandler._sendMessages(projectId, sendingUser, {
@@ -120,7 +112,7 @@ const CollaboratorsInviteHandler = {
       logger.err({ err, projectId, email }, 'error sending messages for invite')
     })
 
-    return invite
+    return _.pick(invite, ['_id', 'email', 'privileges'])
   },
 
   async revokeInvite(projectId, inviteId) {

services/web/app/src/Features/Project/ProjectEditorHandler.js

@@ -141,16 +141,6 @@ module.exports = ProjectEditorHandler = {
     if (invites == null) {
       return []
     }
-    return invites.map(invite =>
-      _.pick(invite, [
-        '_id',
-        'createdAt',
-        'email',
-        'expires',
-        'privileges',
-        'projectId',
-        'sendingUserId',
-      ])
-    )
+    return invites.map(invite => _.pick(invite, ['_id', 'email', 'privileges']))
   },
 }

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

@@ -97,14 +97,14 @@ const tryFollowInviteLink = (user, link, callback) => {
   )
 }
 
-const tryAcceptInvite = (user, invite, callback) => {
+const tryAcceptInvite = (user, invite, projectId, callback) => {
   user.getCsrfToken(err => {
     if (err) {
       return callback(err)
     }
     user.request.post(
       {
-        uri: `/project/${invite.projectId}/invite/token/${invite.token}/accept`,
+        uri: `/project/${projectId}/invite/token/${invite.token}/accept`,
         json: {
           token: invite.token,
         },
@@ -273,22 +273,22 @@ const expectRegistrationRedirectToInvite = (user, link, callback) => {
   })
 }
 
-const expectInviteRedirectToProject = (user, link, invite, callback) => {
+const expectInviteRedirectToProject = (user, link, projectId, callback) => {
   // view invite, redirect straight to project
   tryFollowInviteLink(user, link, (err, response) => {
     expect(err).not.to.exist
     expect(response.statusCode).to.equal(302)
-    expect(response.headers.location).to.equal(`/project/${invite.projectId}`)
+    expect(response.headers.location).to.equal(`/project/${projectId}`)
     callback()
   })
 }
 
-const expectAcceptInviteAndRedirect = (user, invite, callback) => {
+const expectAcceptInviteAndRedirect = (user, invite, projectId, callback) => {
   // should accept the invite and redirect to project
-  tryAcceptInvite(user, invite, (err, response) => {
+  tryAcceptInvite(user, invite, projectId, (err, response) => {
     expect(err).not.to.exist
     expect(response.statusCode).to.equal(302)
-    expect(response.headers.location).to.equal(`/project/${invite.projectId}`)
+    expect(response.headers.location).to.equal(`/project/${projectId}`)
     callback()
   })
 }
@@ -549,8 +549,14 @@ describe('ProjectInviteTests', function () {
           Async.series(
             [
               cb => expectInvitePage(this.user, this.link, cb),
-              cb => expectAcceptInviteAndRedirect(this.user, this.invite, cb),
-              cb => expectProjectAccess(this.user, this.invite.projectId, cb),
+              cb =>
+                expectAcceptInviteAndRedirect(
+                  this.user,
+                  this.invite,
+                  this.projectId,
+                  cb
+                ),
+              cb => expectProjectAccess(this.user, this.projectId, cb),
             ],
             done
           )
@@ -560,15 +566,15 @@ describe('ProjectInviteTests', function () {
           it('should just redirect to the project page', function (done) {
             Async.series(
               [
-                cb => expectProjectAccess(this.user, this.invite.projectId, cb),
+                cb => expectProjectAccess(this.user, this.projectId, cb),
                 cb =>
                   expectInviteRedirectToProject(
                     this.user,
                     this.link,
-                    this.invite,
+                    this.projectId,
                     cb
                   ),
-                cb => expectProjectAccess(this.user, this.invite.projectId, cb),
+                cb => expectProjectAccess(this.user, this.projectId, cb),
               ],
               done
             )
@@ -588,10 +594,11 @@ describe('ProjectInviteTests', function () {
                           throw err
                         }
                         this.secondInvite = invite
+                        const token = generateTokenSpy.getCall(1).returnValue
                         this.secondLink =
                           CollaboratorsEmailHandler._buildInviteUrl(
                             this.fakeProject,
-                            invite
+                            { ...invite, token }
                           )
                         cb()
                       }
@@ -601,11 +608,10 @@ describe('ProjectInviteTests', function () {
                     expectInviteRedirectToProject(
                       this.user,
                       this.secondLink,
-                      this.secondInvite,
+                      this.projectId,
                       cb
                     ),
-                  cb =>
-                    expectProjectAccess(this.user, this.invite.projectId, cb),
+                  cb => expectProjectAccess(this.user, this.projectId, cb),
                   cb =>
                     revokeInvite(
                       this.sendingUser,
@@ -626,7 +632,7 @@ describe('ProjectInviteTests', function () {
           Async.series(
             [
               cb => expectInvitePage(this.user, this.link, cb),
-              cb => expectNoProjectAccess(this.user, this.invite.projectId, cb),
+              cb => expectNoProjectAccess(this.user, this.projectId, cb),
             ],
             done
           )
@@ -642,8 +648,8 @@ describe('ProjectInviteTests', function () {
                 )
                 expectInvalidInvitePage(this.user, link, cb)
               },
-              cb => expectNoProjectAccess(this.user, this.invite.projectId, cb),
-              cb => expectNoProjectAccess(this.user, this.invite.projectId, cb),
+              cb => expectNoProjectAccess(this.user, this.projectId, cb),
+              cb => expectNoProjectAccess(this.user, this.projectId, cb),
             ],
             done
           )
@@ -653,8 +659,14 @@ describe('ProjectInviteTests', function () {
           Async.series(
             [
               cb => expectInvitePage(this.user, this.link, cb),
-              cb => expectAcceptInviteAndRedirect(this.user, this.invite, cb),
-              cb => expectProjectAccess(this.user, this.invite.projectId, cb),
+              cb =>
+                expectAcceptInviteAndRedirect(
+                  this.user,
+                  this.invite,
+                  this.projectId,
+                  cb
+                ),
+              cb => expectProjectAccess(this.user, this.projectId, cb),
             ],
             done
           )
@@ -681,8 +693,14 @@ describe('ProjectInviteTests', function () {
               cb =>
                 expectRegistrationRedirectToInvite(this.user, this.link, cb),
               cb => expectInvitePage(this.user, this.link, cb),
-              cb => expectAcceptInviteAndRedirect(this.user, this.invite, cb),
-              cb => expectProjectAccess(this.user, this.invite.projectId, cb),
+              cb =>
+                expectAcceptInviteAndRedirect(
+                  this.user,
+                  this.invite,
+                  this.projectId,
+                  cb
+                ),
+              cb => expectProjectAccess(this.user, this.projectId, cb),
             ],
             done
           )
@@ -700,7 +718,7 @@ describe('ProjectInviteTests', function () {
           Async.series(
             [
               cb => expectInviteRedirectToRegister(this.user, this.link, cb),
-              cb => expectNoProjectAccess(this.user, this.invite.projectId, cb),
+              cb => expectNoProjectAccess(this.user, this.projectId, cb),
             ],
             done
           )
@@ -714,7 +732,7 @@ describe('ProjectInviteTests', function () {
           Async.series(
             [
               cb => expectInvalidInvitePage(this.user, badLink, cb),
-              cb => expectNoProjectAccess(this.user, this.invite.projectId, cb),
+              cb => expectNoProjectAccess(this.user, this.projectId, cb),
             ],
             done
           )
@@ -730,7 +748,7 @@ describe('ProjectInviteTests', function () {
           Async.series(
             [
               cb => expectInviteRedirectToRegister(this.user, this.link, cb),
-              cb => expectNoProjectAccess(this.user, this.invite.projectId, cb),
+              cb => expectNoProjectAccess(this.user, this.projectId, cb),
             ],
             done
           )
@@ -743,7 +761,7 @@ describe('ProjectInviteTests', function () {
               cb => expectLoginPage(this.user, cb),
               cb => expectLoginRedirectToInvite(this.user, this.link, cb),
               cb => expectInvitePage(this.user, this.link, cb),
-              cb => expectNoProjectAccess(this.user, this.invite.projectId, cb),
+              cb => expectNoProjectAccess(this.user, this.projectId, cb),
             ],
             done
           )
@@ -756,8 +774,14 @@ describe('ProjectInviteTests', function () {
               cb => expectLoginPage(this.user, cb),
               cb => expectLoginRedirectToInvite(this.user, this.link, cb),
               cb => expectInvitePage(this.user, this.link, cb),
-              cb => expectAcceptInviteAndRedirect(this.user, this.invite, cb),
-              cb => expectProjectAccess(this.user, this.invite.projectId, cb),
+              cb =>
+                expectAcceptInviteAndRedirect(
+                  this.user,
+                  this.invite,
+                  this.projectId,
+                  cb
+                ),
+              cb => expectProjectAccess(this.user, this.projectId, cb),
             ],
             done
           )
@@ -769,7 +793,7 @@ describe('ProjectInviteTests', function () {
           Async.series(
             [
               cb => expectInviteRedirectToRegister(this.user, this.link, cb),
-              cb => expectNoProjectAccess(this.user, this.invite.projectId, cb),
+              cb => expectNoProjectAccess(this.user, this.projectId, cb),
             ],
             done
           )
@@ -783,7 +807,7 @@ describe('ProjectInviteTests', function () {
           Async.series(
             [
               cb => expectInvalidInvitePage(this.user, badLink, cb),
-              cb => expectNoProjectAccess(this.user, this.invite.projectId, cb),
+              cb => expectNoProjectAccess(this.user, this.projectId, cb),
             ],
             done
           )

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

@@ -5,6 +5,7 @@ const MockRequest = require('../helpers/MockRequest')
 const MockResponse = require('../helpers/MockResponse')
 const { ObjectId } = require('mongodb')
 const Errors = require('../../../../app/src/Features/Errors/Errors')
+const _ = require('lodash')
 
 const MODULE_PATH =
   '../../../../app/src/Features/Collaborators/CollaboratorsInviteController.js'
@@ -34,6 +35,7 @@ describe('CollaboratorsInviteController', function () {
       privileges: this.privileges,
       createdAt: new Date(),
     }
+    this.inviteReducedData = _.pick(this.invite, ['_id', 'email', 'privileges'])
     this.project = {
       _id: this.projectId,
       owner_ref: this.projectOwner._id,
@@ -82,7 +84,7 @@ describe('CollaboratorsInviteController', function () {
     this.CollaboratorsInviteHandler = {
       promises: {
         getAllInvites: sinon.stub(),
-        inviteToProject: sinon.stub().resolves(this.invite),
+        inviteToProject: sinon.stub().resolves(this.inviteReducedData),
         getInviteByToken: sinon.stub().resolves(this.invite),
         generateNewInvite: sinon.stub().resolves(this.invite),
         revokeInvite: sinon.stub().resolves(this.invite),
@@ -237,7 +239,7 @@ describe('CollaboratorsInviteController', function () {
         it('should produce json response', function () {
           this.res.json.callCount.should.equal(1)
           expect(this.res.json.firstCall.args[0]).to.deep.equal({
-            invite: this.invite,
+            invite: this.inviteReducedData,
           })
         })
 
@@ -362,7 +364,7 @@ describe('CollaboratorsInviteController', function () {
           it('should produce json response', function () {
             this.res.json.callCount.should.equal(1)
             expect(this.res.json.firstCall.args[0]).to.deep.equal({
-              invite: this.invite,
+              invite: this.inviteReducedData,
             })
           })
 
@@ -436,7 +438,7 @@ describe('CollaboratorsInviteController', function () {
       it('should produce json response', function () {
         this.res.json.callCount.should.equal(1)
         expect(this.res.json.firstCall.args[0]).to.deep.equal({
-          invite: this.invite,
+          invite: this.inviteReducedData,
         })
       })
 

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

@@ -86,16 +86,6 @@ 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 () {
@@ -243,13 +233,7 @@ describe('CollaboratorsInviteHandler', function () {
         expect(invite).to.not.equal(null)
         expect(invite).to.not.equal(undefined)
         expect(invite).to.be.instanceof(Object)
-        expect(invite).to.have.all.keys([
-          '_id',
-          'email',
-          'sendingUserId',
-          'projectId',
-          'privileges',
-        ])
+        expect(invite).to.have.all.keys(['_id', 'email', 'privileges'])
       })
 
       it('should have generated a random token', async function () {
@@ -413,12 +397,17 @@ describe('CollaboratorsInviteHandler', function () {
 
   describe('generateNewInvite', function () {
     beforeEach(function () {
+      this.fakeInviteToProjectObject = {
+        _id: new ObjectId(),
+        email: this.email,
+        privileges: this.privileges,
+      }
       this.CollaboratorsInviteHandler.promises.revokeInvite = sinon
         .stub()
         .resolves(this.fakeInvite)
       this.CollaboratorsInviteHandler.promises.inviteToProject = sinon
         .stub()
-        .resolves(this.newFakeInvite)
+        .resolves(this.fakeInviteToProjectObject)
       this.call = async () => {
         return await this.CollaboratorsInviteHandler.promises.generateNewInvite(
           this.projectId,
@@ -456,7 +445,7 @@ describe('CollaboratorsInviteHandler', function () {
 
       it('should return the invite', async function () {
         const invite = await this.call()
-        expect(invite).to.deep.equal(this.newFakeInvite)
+        expect(invite).to.deep.equal(this.fakeInviteToProjectObject)
       })
     })
 

services/web/test/unit/src/Project/ProjectEditorHandlerTests.js

@@ -218,7 +218,9 @@ describe('ProjectEditorHandler', function () {
       it('should include invites', function () {
         expect(this.result.invites).to.exist
         this.result.invites.should.deep.equal(
-          this.invites.map(invite => _.omit(invite, 'token'))
+          this.invites.map(invite =>
+            _.pick(invite, ['_id', 'email', 'privileges'])
+          )
         )
       })