From 6d931f0948eceabcd9ea052b52e47b3107d3a973 Mon Sep 17 00:00:00 2001 From: Eric Mc Sween <5454374+emcsween@users.noreply.github.com> Date: Tue, 13 Jun 2023 07:15:13 -0400 Subject: [PATCH] Merge pull request #13427 from overleaf/em-td-websocket-invites Fix disclosure of invites and invite tokens through the websocket GitOrigin-RevId: cf4925f4faeaaa9202055b52f32e5a80f313946a --- .../Features/Editor/EditorHttpController.js | 1 + .../Features/Project/ProjectEditorHandler.js | 55 +++++------ .../src/Editor/EditorHttpControllerTests.js | 2 + .../src/Project/ProjectEditorHandlerTests.js | 93 ++++++++----------- 4 files changed, 65 insertions(+), 86 deletions(-) diff --git a/services/web/app/src/Features/Editor/EditorHttpController.js b/services/web/app/src/Features/Editor/EditorHttpController.js index 17e445ad38..df1e2b1351 100644 --- a/services/web/app/src/Features/Editor/EditorHttpController.js +++ b/services/web/app/src/Features/Editor/EditorHttpController.js @@ -73,6 +73,7 @@ async function joinProject(req, res, next) { if (isRestrictedUser) { project.owner = { _id: project.owner._id } project.members = [] + project.invites = [] } // Only show the 'renamed or deleted' message once if (project.deletedByExternalDataSource) { diff --git a/services/web/app/src/Features/Project/ProjectEditorHandler.js b/services/web/app/src/Features/Project/ProjectEditorHandler.js index 81d6db6afb..bf151215aa 100644 --- a/services/web/app/src/Features/Project/ProjectEditorHandler.js +++ b/services/web/app/src/Features/Project/ProjectEditorHandler.js @@ -1,16 +1,3 @@ -/* eslint-disable - max-len, -*/ -// TODO: This file was created by bulk-decaffeinate. -// Fix any style issues and re-enable lint. -/* - * decaffeinate suggestions: - * DS101: Remove unnecessary use of Array.from - * DS102: Remove unnecessary code created because of implicit returns - * DS205: Consider reworking code to avoid use of IIFEs - * DS207: Consider shorter variations of null checks - * Full docs: https://github.com/decaffeinate/decaffeinate/blob/master/docs/suggestions.md - */ let ProjectEditorHandler const _ = require('underscore') const Path = require('path') @@ -48,7 +35,7 @@ module.exports = ProjectEditorHandler = { deletedDocsFromDocstore ), members: [], - invites, + invites: this.buildInvitesView(invites), tokens: project.tokens, imageName: project.imageName != null @@ -56,12 +43,6 @@ module.exports = ProjectEditorHandler = { : undefined, } - if (result.invites == null) { - result.invites = [] - } - result.invites.forEach(invite => { - delete invite.token - }) ;({ owner, ownerFeatures, members } = this.buildOwnerAndMembersViews(members)) result.owner = owner @@ -100,7 +81,7 @@ module.exports = ProjectEditorHandler = { let owner = null let ownerFeatures = null const filteredMembers = [] - for (const member of Array.from(members || [])) { + for (const member of members || []) { if (member.privilegeLevel === 'owner') { ownerFeatures = member.user.features owner = this.buildUserModelView(member.user, 'owner') @@ -129,24 +110,15 @@ module.exports = ProjectEditorHandler = { }, buildFolderModelView(folder) { - let file const fileRefs = _.filter(folder.fileRefs || [], file => file != null) return { _id: folder._id, name: folder.name, - folders: Array.from(folder.folders || []).map(childFolder => + folders: (folder.folders || []).map(childFolder => this.buildFolderModelView(childFolder) ), - fileRefs: (() => { - const result = [] - for (file of Array.from(fileRefs)) { - result.push(this.buildFileModelView(file)) - } - return result - })(), - docs: Array.from(folder.docs || []).map(doc => - this.buildDocModelView(doc) - ), + fileRefs: fileRefs.map(file => this.buildFileModelView(file)), + docs: (folder.docs || []).map(doc => this.buildDocModelView(doc)), } }, @@ -165,4 +137,21 @@ module.exports = ProjectEditorHandler = { name: doc.name, } }, + + buildInvitesView(invites) { + if (invites == null) { + return [] + } + return invites.map(invite => + _.pick(invite, [ + '_id', + 'createdAt', + 'email', + 'expires', + 'privileges', + 'projectId', + 'sendingUserId', + ]) + ) + }, } diff --git a/services/web/test/unit/src/Editor/EditorHttpControllerTests.js b/services/web/test/unit/src/Editor/EditorHttpControllerTests.js index 74095b3b5e..a7f8ed7c08 100644 --- a/services/web/test/unit/src/Editor/EditorHttpControllerTests.js +++ b/services/web/test/unit/src/Editor/EditorHttpControllerTests.js @@ -25,11 +25,13 @@ describe('EditorHttpController', function () { other_property: true, }, members: [{ one: 1 }, { two: 2 }], + invites: [{ three: 3 }, { four: 4 }], } this.reducedProjectView = { _id: this.projectView._id, owner: { _id: this.projectView.owner._id }, members: [], + invites: [], } this.doc = { _id: new ObjectId(), name: 'excellent-original-idea.tex' } this.file = { _id: new ObjectId() } diff --git a/services/web/test/unit/src/Project/ProjectEditorHandlerTests.js b/services/web/test/unit/src/Project/ProjectEditorHandlerTests.js index ade0c59a6b..93ba9895e2 100644 --- a/services/web/test/unit/src/Project/ProjectEditorHandlerTests.js +++ b/services/web/test/unit/src/Project/ProjectEditorHandlerTests.js @@ -1,15 +1,4 @@ -/* eslint-disable - max-len, - no-return-assign, -*/ -// TODO: This file was created by bulk-decaffeinate. -// Fix any style issues and re-enable lint. -/* - * decaffeinate suggestions: - * DS101: Remove unnecessary use of Array.from - * DS102: Remove unnecessary code created because of implicit returns - * Full docs: https://github.com/decaffeinate/decaffeinate/blob/master/docs/suggestions.md - */ +const _ = require('lodash') const { expect } = require('chai') const modulePath = '../../../../app/src/Features/Project/ProjectEditorHandler' @@ -109,38 +98,38 @@ describe('ProjectEditorHandler', function () { this.deletedDocsFromDocstore = [ { _id: 'deleted-doc-id-from-docstore', name: 'docstore.tex' }, ] - return (this.handler = SandboxedModule.require(modulePath)) + this.handler = SandboxedModule.require(modulePath) }) describe('buildProjectModelView', function () { describe('with owner, members and invites included', function () { beforeEach(function () { - return (this.result = this.handler.buildProjectModelView( + this.result = this.handler.buildProjectModelView( this.project, this.members, this.invites, this.deletedDocsFromDocstore - )) + ) }) it('should include the id', function () { expect(this.result._id).to.exist - return this.result._id.should.equal('project-id') + this.result._id.should.equal('project-id') }) it('should include the name', function () { expect(this.result.name).to.exist - return this.result.name.should.equal('Project Name') + this.result.name.should.equal('Project Name') }) it('should include the root doc id', function () { expect(this.result.rootDoc_id).to.exist - return this.result.rootDoc_id.should.equal('file-id') + this.result.rootDoc_id.should.equal('file-id') }) it('should include the public access level', function () { expect(this.result.publicAccesLevel).to.exist - return this.result.publicAccesLevel.should.equal('private') + this.result.publicAccesLevel.should.equal('private') }) it('should include the owner', function () { @@ -149,7 +138,7 @@ describe('ProjectEditorHandler', function () { this.result.owner.email.should.equal('owner@sharelatex.com') this.result.owner.first_name.should.equal('Owner') this.result.owner.last_name.should.equal('ShareLaTeX') - return this.result.owner.privileges.should.equal('owner') + this.result.owner.privileges.should.equal('owner') }) it('should include the deletedDocs', function () { @@ -164,14 +153,9 @@ describe('ProjectEditorHandler', function () { ]) }) - it('invites should not include the token', function () { - expect(this.result.invites[0].token).not.to.exist - expect(this.result.invites[1].token).not.to.exist - }) - it('should gather readOnly_refs and collaberators_refs into a list of members', function () { const findMember = id => { - for (const member of Array.from(this.result.members)) { + for (const member of this.result.members) { if (member._id === id) { return member } @@ -193,7 +177,7 @@ describe('ProjectEditorHandler', function () { findMember('read-write-id').privileges.should.equal('readAndWrite') findMember('read-write-id').first_name.should.equal('Read') findMember('read-write-id').last_name.should.equal('Write') - return findMember('read-write-id').email.should.equal( + findMember('read-write-id').email.should.equal( 'read-write@sharelatex.com' ) }) @@ -203,12 +187,12 @@ describe('ProjectEditorHandler', function () { this.result.rootFolder[0].name.should.equal('') this.result.rootFolder[0].folders[0]._id.should.equal('sub-folder-id') - return this.result.rootFolder[0].folders[0].name.should.equal('folder') + this.result.rootFolder[0].folders[0].name.should.equal('folder') }) it('should not duplicate folder contents', function () { this.result.rootFolder[0].docs.length.should.equal(0) - return this.result.rootFolder[0].fileRefs.length.should.equal(0) + this.result.rootFolder[0].fileRefs.length.should.equal(0) }) it('should include files in the project', function () { @@ -221,8 +205,8 @@ describe('ProjectEditorHandler', function () { this.result.rootFolder[0].folders[0].fileRefs[0].created.should.equal( this.created ) - return expect(this.result.rootFolder[0].folders[0].fileRefs[0].size).not - .to.exist + expect(this.result.rootFolder[0].folders[0].fileRefs[0].size).not.to + .exist }) it('should include docs in the project but not the lines', function () { @@ -230,13 +214,20 @@ describe('ProjectEditorHandler', function () { this.result.rootFolder[0].folders[0].docs[0].name.should.equal( 'main.tex' ) - return expect(this.result.rootFolder[0].folders[0].docs[0].lines).not.to - .exist + expect(this.result.rootFolder[0].folders[0].docs[0].lines).not.to.exist }) it('should include invites', function () { expect(this.result.invites).to.exist - return this.result.invites.should.deep.equal(this.invites) + this.result.invites.should.deep.equal( + this.invites.map(invite => _.omit(invite, 'token')) + ) + }) + + it('invites should not include the token', function () { + for (const invite of this.result.invites) { + expect(invite.token).not.to.exist + } }) }) @@ -269,7 +260,7 @@ describe('ProjectEditorHandler', function () { [], [] ) - return result.deletedByExternalDataSource.should.equal(false) + result.deletedByExternalDataSource.should.equal(false) }) it('should set the deletedByExternalDataSource flag to false when it is false', function () { @@ -279,7 +270,7 @@ describe('ProjectEditorHandler', function () { [], [] ) - return result.deletedByExternalDataSource.should.equal(false) + result.deletedByExternalDataSource.should.equal(false) }) it('should set the deletedByExternalDataSource flag to true when it is true', function () { @@ -290,7 +281,7 @@ describe('ProjectEditorHandler', function () { [], [] ) - return result.deletedByExternalDataSource.should.equal(true) + result.deletedByExternalDataSource.should.equal(true) }) }) @@ -302,12 +293,12 @@ describe('ProjectEditorHandler', function () { compileGroup: 'priority', compileTimeout: 96, } - return (this.result = this.handler.buildProjectModelView( + this.result = this.handler.buildProjectModelView( this.project, this.members, [], [] - )) + ) }) it('should copy the owner features to the project', function () { @@ -320,7 +311,7 @@ describe('ProjectEditorHandler', function () { this.result.features.compileGroup.should.equal( this.owner.features.compileGroup ) - return this.result.features.compileTimeout.should.equal( + this.result.features.compileTimeout.should.equal( this.owner.features.compileTimeout ) }) @@ -389,13 +380,11 @@ describe('ProjectEditorHandler', function () { compileGroup: 'priority', compileTimeout: 22, } - return (this.result = this.handler.buildOwnerAndMembersViews( - this.members - )) + this.result = this.handler.buildOwnerAndMembersViews(this.members) }) it('should produce an object with the right keys', function () { - return expect(this.result).to.have.all.keys([ + expect(this.result).to.have.all.keys([ 'owner', 'ownerFeatures', 'members', @@ -406,15 +395,13 @@ describe('ProjectEditorHandler', function () { this.result.members.length.should.equal(this.members.length - 1) expect(this.result.owner._id).to.equal(this.owner._id) expect(this.result.owner.email).to.equal(this.owner.email) - return expect( + expect( this.result.members.filter(m => m._id === this.owner._id).length ).to.equal(0) }) it('should extract the ownerFeatures from the owner object', function () { - return expect(this.result.ownerFeatures).to.deep.equal( - this.owner.features - ) + expect(this.result.ownerFeatures).to.deep.equal(this.owner.features) }) describe('when there is no owner', function () { @@ -423,13 +410,13 @@ describe('ProjectEditorHandler', function () { this.membersWithoutOwner = this.members.filter( m => m.user._id !== this.owner._id ) - return (this.result = this.handler.buildOwnerAndMembersViews( + this.result = this.handler.buildOwnerAndMembersViews( this.membersWithoutOwner - )) + ) }) it('should produce an object with the right keys', function () { - return expect(this.result).to.have.all.keys([ + expect(this.result).to.have.all.keys([ 'owner', 'ownerFeatures', 'members', @@ -438,11 +425,11 @@ describe('ProjectEditorHandler', function () { it('should not separate out an owner', function () { this.result.members.length.should.equal(this.membersWithoutOwner.length) - return expect(this.result.owner).to.equal(null) + expect(this.result.owner).to.equal(null) }) it('should not extract the ownerFeatures from the owner object', function () { - return expect(this.result.ownerFeatures).to.equal(null) + expect(this.result.ownerFeatures).to.equal(null) }) }) })