diff --git a/services/web/app/src/Features/Collaborators/CollaboratorsInviteController.js b/services/web/app/src/Features/Collaborators/CollaboratorsInviteController.js index 027a7bafad..efaccabbcb 100644 --- a/services/web/app/src/Features/Collaborators/CollaboratorsInviteController.js +++ b/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( diff --git a/services/web/app/src/Features/Collaborators/CollaboratorsInviteHandler.js b/services/web/app/src/Features/Collaborators/CollaboratorsInviteHandler.js index 05540b0389..1fcc67207b 100644 --- a/services/web/app/src/Features/Collaborators/CollaboratorsInviteHandler.js +++ b/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( diff --git a/services/web/app/src/Features/Collaborators/CollaboratorsInviteHelper.js b/services/web/app/src/Features/Collaborators/CollaboratorsInviteHelper.js index c03471dbdf..305f93eac0 100644 --- a/services/web/app/src/Features/Collaborators/CollaboratorsInviteHelper.js +++ b/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, } diff --git a/services/web/app/src/Features/Collaborators/CollaboratorsRouter.js b/services/web/app/src/Features/Collaborators/CollaboratorsRouter.js index b00c1b717c..e639580aca 100644 --- a/services/web/app/src/Features/Collaborators/CollaboratorsRouter.js +++ b/services/web/app/src/Features/Collaborators/CollaboratorsRouter.js @@ -123,7 +123,7 @@ module.exports = { }), AuthenticationController.requireLogin(), AuthorizationMiddleware.ensureUserCanAdminProject, - CollaboratorsInviteController.resendInvite + CollaboratorsInviteController.generateNewInvite ) webRouter.get( diff --git a/services/web/app/src/models/ProjectInvite.js b/services/web/app/src/models/ProjectInvite.js index c7c0d44a79..a7bfbad1e0 100644 --- a/services/web/app/src/models/ProjectInvite.js +++ b/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, diff --git a/services/web/frontend/js/features/share-project-modal/components/invite.jsx b/services/web/frontend/js/features/share-project-modal/components/invite.jsx index 1304f19106..c44f712564 100644 --- a/services/web/frontend/js/features/share-project-modal/components/invite.jsx +++ b/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 ( diff --git a/services/web/migrations/20240531082910_remove_project_invite_tokens.js b/services/web/migrations/20240531082910_remove_project_invite_tokens.js new file mode 100644 index 0000000000..e980816a6f --- /dev/null +++ b/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 => {} diff --git a/services/web/test/acceptance/src/ProjectInviteTests.js b/services/web/test/acceptance/src/ProjectInviteTests.js index a5a5066d78..6d4dc7f308 100644 --- a/services/web/test/acceptance/src/ProjectInviteTests.js +++ b/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) { diff --git a/services/web/test/unit/src/Collaborators/CollaboratorsInviteControllerTests.js b/services/web/test/unit/src/Collaborators/CollaboratorsInviteControllerTests.js index cb2f8b36a9..da07b111b4 100644 --- a/services/web/test/unit/src/Collaborators/CollaboratorsInviteControllerTests.js +++ b/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 ) }) diff --git a/services/web/test/unit/src/Collaborators/CollaboratorsInviteHandlerTests.js b/services/web/test/unit/src/Collaborators/CollaboratorsInviteHandlerTests.js index c575b83e10..ff17f1109a 100644 --- a/services/web/test/unit/src/Collaborators/CollaboratorsInviteHandlerTests.js +++ b/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 ) })