diff --git a/services/web/app/src/Features/Collaborators/CollaboratorsGetter.js b/services/web/app/src/Features/Collaborators/CollaboratorsGetter.js index 10c8e53757..a3543ae614 100644 --- a/services/web/app/src/Features/Collaborators/CollaboratorsGetter.js +++ b/services/web/app/src/Features/Collaborators/CollaboratorsGetter.js @@ -16,9 +16,6 @@ module.exports = { getMemberIdsWithPrivilegeLevels: callbackify(getMemberIdsWithPrivilegeLevels), getMemberIds: callbackify(getMemberIds), getInvitedMemberIds: callbackify(getInvitedMemberIds), - getInvitedMembersWithPrivilegeLevels: callbackify( - getInvitedMembersWithPrivilegeLevels - ), getInvitedMembersWithPrivilegeLevelsFromFields: callbackify( getInvitedMembersWithPrivilegeLevelsFromFields ), @@ -36,7 +33,6 @@ module.exports = { getMemberIdsWithPrivilegeLevels, getMemberIds, getInvitedMemberIds, - getInvitedMembersWithPrivilegeLevels, getInvitedMembersWithPrivilegeLevelsFromFields, getMemberIdPrivilegeLevel, getInvitedEditCollaboratorCount, @@ -95,11 +91,40 @@ class ProjectAccess { this.#publicAccessLevel = project.publicAccesLevel } + /** + * @return {Promise<{ownerMember: LoadedProjectMember|undefined, members: LoadedProjectMember[]}>} + */ + async loadOwnerAndInvitedMembers() { + const all = await _loadMembers( + this.#members.filter(m => m.source !== Sources.TOKEN) + ) + return { + ownerMember: all.find(m => m.privilegeLevel === PrivilegeLevels.OWNER), + members: all.filter(m => m.privilegeLevel !== PrivilegeLevels.OWNER), + } + } + /** * @return {Promise} */ async loadInvitedMembers() { - return _loadMembers(this.#members.filter(m => m.source !== Sources.TOKEN)) + return _loadMembers( + this.#members.filter( + m => + m.source !== Sources.TOKEN && + m.privilegeLevel !== PrivilegeLevels.OWNER + ) + ) + } + + /** + * @return {Promise} + */ + async loadOwner() { + const [owner] = await _loadMembers( + this.#members.filter(m => m.privilegeLevel === PrivilegeLevels.OWNER) + ) + return owner } /** @@ -248,10 +273,6 @@ async function getInvitedMemberIds(projectId) { return (await getProjectAccess(projectId)).invitedMemberIds() } -async function getInvitedMembersWithPrivilegeLevels(projectId) { - return await (await getProjectAccess(projectId)).loadInvitedMembers() -} - async function getInvitedMembersWithPrivilegeLevelsFromFields( ownerId, collaboratorIds, @@ -397,10 +418,9 @@ async function dangerouslyGetAllProjectsUserIsMemberOf(userId, fields) { async function getAllInvitedMembers(projectId) { try { - const { members } = ProjectEditorHandler.buildOwnerAndMembersViews( - await (await getProjectAccess(projectId)).loadInvitedMembers() - ) - return members + const projectAccess = await getProjectAccess(projectId) + const invitedMembers = await projectAccess.loadInvitedMembers() + return invitedMembers.map(ProjectEditorHandler.buildUserModelView) } catch (err) { throw OError.tag(err, 'error getting members for project', { projectId }) } @@ -526,6 +546,7 @@ function _getMemberIdsWithPrivilegeLevelsFromFields( * @private */ async function _loadMembers(members) { + if (members.length === 0) return [] const userIds = Array.from(new Set(members.map(m => m.id))) const users = new Map() for (const user of await UserGetter.promises.getUsers(userIds, { diff --git a/services/web/app/src/Features/Editor/EditorHttpController.js b/services/web/app/src/Features/Editor/EditorHttpController.js index def7face04..f44b57f069 100644 --- a/services/web/app/src/Features/Editor/EditorHttpController.js +++ b/services/web/app/src/Features/Editor/EditorHttpController.js @@ -42,12 +42,6 @@ async function joinProject(req, res, next) { if (!project) { return res.sendStatus(403) } - // Hide sensitive data if the user is restricted - if (isRestrictedUser) { - project.owner = { _id: project.owner._id } - project.members = [] - project.invites = [] - } // Only show the 'renamed or deleted' message once if (project.deletedByExternalDataSource) { await ProjectDeleter.promises.unmarkAsDeletedByExternalSource(projectId) @@ -75,7 +69,6 @@ async function _buildJoinProjectView(req, projectId, userId) { throw new Errors.NotFoundError('project not found') } const projectAccess = new ProjectAccess(project) - const members = await projectAccess.loadInvitedMembers() const token = req.body.anonymousAccessToken const privilegeLevel = await AuthorizationManager.promises.getPrivilegeLevelForProjectWithProjectAccess( @@ -87,8 +80,6 @@ async function _buildJoinProjectView(req, projectId, userId) { if (privilegeLevel == null || privilegeLevel === PrivilegeLevels.NONE) { return { project: null, privilegeLevel: null, isRestrictedUser: false } } - const invites = - await CollaboratorsInviteGetter.promises.getAllInvites(projectId) const isTokenMember = projectAccess.isUserTokenMember(userId) const isInvitedMember = projectAccess.isUserInvitedMember(userId) const isRestrictedUser = AuthorizationManager.isRestrictedUser( @@ -97,11 +88,23 @@ async function _buildJoinProjectView(req, projectId, userId) { isTokenMember, isInvitedMember ) + let ownerMember + let members = [] + let invites = [] + if (isRestrictedUser) { + ownerMember = await projectAccess.loadOwner() + } else { + ;({ ownerMember, members } = + await projectAccess.loadOwnerAndInvitedMembers()) + invites = await CollaboratorsInviteGetter.promises.getAllInvites(projectId) + } return { project: ProjectEditorHandler.buildProjectModelView( project, + ownerMember, members, - invites + invites, + isRestrictedUser ), privilegeLevel, isTokenMember, diff --git a/services/web/app/src/Features/Project/ProjectEditorHandler.js b/services/web/app/src/Features/Project/ProjectEditorHandler.js index 05e5beba09..3d3d300e66 100644 --- a/services/web/app/src/Features/Project/ProjectEditorHandler.js +++ b/services/web/app/src/Features/Project/ProjectEditorHandler.js @@ -6,8 +6,13 @@ const Features = require('../../infrastructure/Features') module.exports = ProjectEditorHandler = { trackChangesAvailable: false, - buildProjectModelView(project, members, invites) { - let owner, ownerFeatures + buildProjectModelView( + project, + ownerMember, + members, + invites, + isRestrictedUser + ) { const result = { _id: project._id, name: project.name, @@ -20,20 +25,23 @@ module.exports = ProjectEditorHandler = { description: project.description, spellCheckLanguage: project.spellCheckLanguage, deletedByExternalDataSource: project.deletedByExternalDataSource || false, - members: [], - invites: this.buildInvitesView(invites), imageName: project.imageName != null ? Path.basename(project.imageName) : undefined, } - ;({ owner, ownerFeatures, members } = - this.buildOwnerAndMembersViews(members)) - result.owner = owner - result.members = members + if (isRestrictedUser) { + result.owner = { _id: project.owner_ref } + result.members = [] + result.invites = [] + } else { + result.owner = this.buildUserModelView(ownerMember) + result.members = members.map(this.buildUserModelView) + result.invites = this.buildInvitesView(invites) + } - result.features = _.defaults(ownerFeatures || {}, { + result.features = _.defaults(ownerMember?.user?.features || {}, { collaborators: -1, // Infinite versioning: false, dropbox: false, @@ -62,25 +70,6 @@ module.exports = ProjectEditorHandler = { return result }, - buildOwnerAndMembersViews(members) { - let owner = null - let ownerFeatures = null - const filteredMembers = [] - for (const member of members || []) { - if (member.privilegeLevel === 'owner') { - ownerFeatures = member.user.features - owner = this.buildUserModelView(member) - } else { - filteredMembers.push(this.buildUserModelView(member)) - } - } - return { - owner, - ownerFeatures, - members: filteredMembers, - } - }, - buildUserModelView(member) { const user = member.user return { diff --git a/services/web/test/unit/src/Collaborators/CollaboratorsGetterTests.js b/services/web/test/unit/src/Collaborators/CollaboratorsGetterTests.js index dda99e04f3..10542c4564 100644 --- a/services/web/test/unit/src/Collaborators/CollaboratorsGetterTests.js +++ b/services/web/test/unit/src/Collaborators/CollaboratorsGetterTests.js @@ -62,7 +62,7 @@ describe('CollaboratorsGetter', function () { }, } this.ProjectEditorHandler = { - buildOwnerAndMembersViews: sinon.stub(), + buildUserModelView: sinon.stub(), } this.CollaboratorsGetter = SandboxedModule.require(MODULE_PATH, { requires: { @@ -204,30 +204,6 @@ describe('CollaboratorsGetter', function () { }) }) - describe('getInvitedMembersWithPrivilegeLevels', function () { - beforeEach(function () { - this.UserGetter.promises.getUsers.resolves([ - { _id: this.readOnlyRef1 }, - { _id: this.readOnlyTokenRef }, - { _id: this.readWriteRef2 }, - { _id: this.readWriteTokenRef }, - { _id: this.reviewer1Ref }, - ]) - }) - - it('should return an array of invited members with their privilege levels', async function () { - const result = - await this.CollaboratorsGetter.promises.getInvitedMembersWithPrivilegeLevels( - this.project._id - ) - expect(result).to.have.deep.members([ - { user: { _id: this.readOnlyRef1 }, privilegeLevel: 'readOnly' }, - { user: { _id: this.readWriteRef2 }, privilegeLevel: 'readAndWrite' }, - { user: { _id: this.reviewer1Ref }, privilegeLevel: 'review' }, - ]) - }) - }) - describe('getMemberIdPrivilegeLevel', function () { it('should return the privilege level if it exists', async function () { const level = @@ -401,20 +377,21 @@ describe('CollaboratorsGetter', function () { { user: this.readWriteUser, privilegeLevel: 'readAndWrite' }, { user: this.reviewUser, privilegeLevel: 'review' }, ] - this.views = { - owner: this.owningUser, - ownerFeatures: this.owningUser.features, - members: [ - { _id: this.readWriteUser._id, email: this.readWriteUser.email }, - { _id: this.reviewUser._id, email: this.reviewUser.email }, - ], - } + this.memberViews = [ + { _id: this.readWriteUser._id, email: this.readWriteUser.email }, + { _id: this.reviewUser._id, email: this.reviewUser.email }, + ] this.UserGetter.promises.getUsers.resolves([ this.owningUser, this.readWriteUser, this.reviewUser, ]) - this.ProjectEditorHandler.buildOwnerAndMembersViews.returns(this.views) + this.ProjectEditorHandler.buildUserModelView + .withArgs(this.members[1]) + .returns(this.memberViews[0]) + this.ProjectEditorHandler.buildUserModelView + .withArgs(this.members[2]) + .returns(this.memberViews[1]) this.result = await this.CollaboratorsGetter.promises.getAllInvitedMembers( this.project._id @@ -422,15 +399,18 @@ describe('CollaboratorsGetter', function () { }) it('should produce a list of members', function () { - expect(this.result).to.deep.equal(this.views.members) + expect(this.result).to.deep.equal(this.memberViews) }) - it('should call ProjectEditorHandler.buildOwnerAndMembersViews', function () { - expect(this.ProjectEditorHandler.buildOwnerAndMembersViews).to.have.been - .calledOnce + it('should call ProjectEditorHandler.buildUserModelView', function () { + expect(this.ProjectEditorHandler.buildUserModelView).to.have.been + .calledTwice expect( - this.ProjectEditorHandler.buildOwnerAndMembersViews - ).to.have.been.calledWith(this.members) + this.ProjectEditorHandler.buildUserModelView + ).to.have.been.calledWith(this.members[1]) + expect( + this.ProjectEditorHandler.buildUserModelView + ).to.have.been.calledWith(this.members[2]) }) }) diff --git a/services/web/test/unit/src/Editor/EditorHttpControllerTests.js b/services/web/test/unit/src/Editor/EditorHttpControllerTests.js index f9fcf4362e..7fc08c45d3 100644 --- a/services/web/test/unit/src/Editor/EditorHttpControllerTests.js +++ b/services/web/test/unit/src/Editor/EditorHttpControllerTests.js @@ -20,6 +20,12 @@ describe('EditorHttpController', function () { _id: new ObjectId(), projects: {}, } + this.members = [ + { user: { _id: 'owner', features: {} }, privilegeLevel: 'owner' }, + { user: { _id: 'one' }, privilegeLevel: 'readOnly' }, + ] + this.ownerMember = this.members[0] + this.invites = [{ _id: 'three' }, { _id: 'four' }] this.projectView = { _id: this.project._id, owner: { @@ -27,7 +33,10 @@ describe('EditorHttpController', function () { email: 'owner@example.com', other_property: true, }, - members: [{ one: 1 }, { two: 2 }], + members: [ + { _id: 'owner', privileges: 'owner' }, + { _id: 'one', privileges: 'readOnly' }, + ], invites: [{ three: 3 }, { four: 4 }], } this.reducedProjectView = { @@ -56,10 +65,16 @@ describe('EditorHttpController', function () { .resolves('owner'), }, } + const members = this.members + const ownerMember = this.ownerMember this.CollaboratorsGetter = { ProjectAccess: class { - loadInvitedMembers() { - return [] + loadOwnerAndInvitedMembers() { + return { members, ownerMember } + } + + loadOwner() { + return ownerMember } isUserTokenMember() { @@ -71,9 +86,6 @@ describe('EditorHttpController', function () { } }, promises: { - getInvitedMembersWithPrivilegeLevels: sinon - .stub() - .resolves(['members', 'mock']), isUserInvitedMemberOfProject: sinon.stub().resolves(false), }, } @@ -82,22 +94,23 @@ describe('EditorHttpController', function () { userIsTokenMember: sinon.stub().resolves(false), }, } + this.invites = [ + { + _id: 'invite_one', + email: 'user-one@example.com', + privileges: 'readOnly', + projectId: this.project._id, + }, + { + _id: 'invite_two', + email: 'user-two@example.com', + privileges: 'readOnly', + projectId: this.project._id, + }, + ] this.CollaboratorsInviteGetter = { promises: { - getAllInvites: sinon.stub().resolves([ - { - _id: 'invite_one', - email: 'user-one@example.com', - privileges: 'readOnly', - projectId: this.project._id, - }, - { - _id: 'invite_two', - email: 'user-two@example.com', - privileges: 'readOnly', - projectId: this.project._id, - }, - ]), + getAllInvites: sinon.stub().resolves(this.invites), }, } this.EditorController = { @@ -195,6 +208,18 @@ describe('EditorHttpController', function () { this.EditorHttpController.joinProject(this.req, this.res) }) + it('should request a full view', function () { + expect( + this.ProjectEditorHandler.buildProjectModelView + ).to.have.been.calledWith( + this.project, + this.ownerMember, + this.members, + this.invites, + false + ) + }) + it('should return the project and privilege level', function () { expect(this.res.json).to.have.been.calledWith({ project: this.projectView, @@ -231,6 +256,9 @@ describe('EditorHttpController', function () { describe('with a restricted user', function () { beforeEach(function (done) { + this.ProjectEditorHandler.buildProjectModelView.returns( + this.reducedProjectView + ) this.AuthorizationManager.isRestrictedUser.returns(true) this.AuthorizationManager.promises.getPrivilegeLevelForProjectWithProjectAccess.resolves( 'readOnly' @@ -239,6 +267,12 @@ describe('EditorHttpController', function () { this.EditorHttpController.joinProject(this.req, this.res) }) + it('should request a restricted view', function () { + expect( + this.ProjectEditorHandler.buildProjectModelView + ).to.have.been.calledWith(this.project, this.ownerMember, [], [], true) + }) + it('should mark the user as restricted, and hide details of owner', function () { expect(this.res.json).to.have.been.calledWith({ project: this.reducedProjectView, @@ -268,6 +302,9 @@ describe('EditorHttpController', function () { beforeEach(function (done) { this.token = 'token' this.TokenAccessHandler.getRequestToken.returns(this.token) + this.ProjectEditorHandler.buildProjectModelView.returns( + this.reducedProjectView + ) this.req.body = { userId: 'anonymous-user', anonymousAccessToken: this.token, @@ -282,6 +319,12 @@ describe('EditorHttpController', function () { this.EditorHttpController.joinProject(this.req, this.res) }) + it('should request a restricted view', function () { + expect( + this.ProjectEditorHandler.buildProjectModelView + ).to.have.been.calledWith(this.project, this.ownerMember, [], [], true) + }) + it('should mark the user as restricted', function () { expect(this.res.json).to.have.been.calledWith({ project: this.reducedProjectView, diff --git a/services/web/test/unit/src/Project/ProjectEditorHandlerTests.js b/services/web/test/unit/src/Project/ProjectEditorHandlerTests.js index 0fb5b5fce4..8456fe2227 100644 --- a/services/web/test/unit/src/Project/ProjectEditorHandlerTests.js +++ b/services/web/test/unit/src/Project/ProjectEditorHandlerTests.js @@ -8,6 +8,7 @@ describe('ProjectEditorHandler', function () { beforeEach(function () { this.project = { _id: 'project-id', + owner_ref: 'owner-id', name: 'Project Name', rootDoc_id: 'file-id', publicAccesLevel: 'private', @@ -43,16 +44,19 @@ describe('ProjectEditorHandler', function () { }, ], } + this.ownerMember = { + user: (this.owner = { + _id: 'owner-id', + first_name: 'Owner', + last_name: 'Overleaf', + email: 'owner@overleaf.com', + features: { + compileTimeout: 240, + }, + }), + privilegeLevel: 'owner', + } this.members = [ - { - user: (this.owner = { - _id: 'owner-id', - first_name: 'Owner', - last_name: 'Overleaf', - email: 'owner@overleaf.com', - }), - privilegeLevel: 'owner', - }, { user: { _id: 'read-only-id', @@ -96,8 +100,10 @@ describe('ProjectEditorHandler', function () { beforeEach(function () { this.result = this.handler.buildProjectModelView( this.project, + this.ownerMember, this.members, - this.invites + this.invites, + false ) }) @@ -206,6 +212,93 @@ describe('ProjectEditorHandler', function () { expect(invite.token).not.to.exist } }) + + it('should have the correct features', function () { + expect(this.result.features.compileTimeout).to.equal(240) + }) + }) + + describe('with a restricted user', function () { + beforeEach(function () { + this.result = this.handler.buildProjectModelView( + this.project, + this.ownerMember, + [], + [], + true + ) + }) + + it('should include the id', function () { + expect(this.result._id).to.exist + this.result._id.should.equal('project-id') + }) + + it('should include the name', function () { + expect(this.result.name).to.exist + this.result.name.should.equal('Project Name') + }) + + it('should include the root doc id', function () { + expect(this.result.rootDoc_id).to.exist + this.result.rootDoc_id.should.equal('file-id') + }) + + it('should include the public access level', function () { + expect(this.result.publicAccesLevel).to.exist + this.result.publicAccesLevel.should.equal('private') + }) + + it('should hide the owner', function () { + expect(this.result.owner).to.deep.equal({ _id: 'owner-id' }) + }) + + it('should hide members', function () { + this.result.members.length.should.equal(0) + }) + + it('should include folders in the project', function () { + this.result.rootFolder[0]._id.should.equal('root-folder-id') + this.result.rootFolder[0].name.should.equal('') + + this.result.rootFolder[0].folders[0]._id.should.equal('sub-folder-id') + 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) + this.result.rootFolder[0].fileRefs.length.should.equal(0) + }) + + it('should include files in the project', function () { + this.result.rootFolder[0].folders[0].fileRefs[0]._id.should.equal( + 'file-id' + ) + this.result.rootFolder[0].folders[0].fileRefs[0].name.should.equal( + 'image.png' + ) + this.result.rootFolder[0].folders[0].fileRefs[0].created.should.equal( + this.created + ) + 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 () { + this.result.rootFolder[0].folders[0].docs[0]._id.should.equal('doc-id') + this.result.rootFolder[0].folders[0].docs[0].name.should.equal( + 'main.tex' + ) + expect(this.result.rootFolder[0].folders[0].docs[0].lines).not.to.exist + }) + + it('should hide invites', function () { + expect(this.result.invites).to.have.length(0) + }) + + it('should have the correct features', function () { + expect(this.result.features.compileTimeout).to.equal(240) + }) }) describe('deletedByExternalDataSource', function () { @@ -213,8 +306,10 @@ describe('ProjectEditorHandler', function () { delete this.project.deletedByExternalDataSource const result = this.handler.buildProjectModelView( this.project, + this.ownerMember, this.members, - [] + [], + false ) result.deletedByExternalDataSource.should.equal(false) }) @@ -222,8 +317,10 @@ describe('ProjectEditorHandler', function () { it('should set the deletedByExternalDataSource flag to false when it is false', function () { const result = this.handler.buildProjectModelView( this.project, + this.ownerMember, this.members, - [] + [], + false ) result.deletedByExternalDataSource.should.equal(false) }) @@ -232,8 +329,10 @@ describe('ProjectEditorHandler', function () { this.project.deletedByExternalDataSource = true const result = this.handler.buildProjectModelView( this.project, + this.ownerMember, this.members, - [] + [], + false ) result.deletedByExternalDataSource.should.equal(true) }) @@ -249,8 +348,10 @@ describe('ProjectEditorHandler', function () { } this.result = this.handler.buildProjectModelView( this.project, + this.ownerMember, this.members, - [] + [], + false ) }) @@ -278,8 +379,10 @@ describe('ProjectEditorHandler', function () { } this.result = this.handler.buildProjectModelView( this.project, + this.ownerMember, this.members, - [] + [], + false ) }) it('should not emit trackChangesState', function () { @@ -302,8 +405,10 @@ describe('ProjectEditorHandler', function () { this.project.track_changes = dbEntry this.result = this.handler.buildProjectModelView( this.project, + this.ownerMember, this.members, - [] + [], + false ) }) it(`should set trackChangesState=${expected}`, function () { @@ -322,66 +427,4 @@ describe('ProjectEditorHandler', function () { }) }) }) - - describe('buildOwnerAndMembersViews', function () { - beforeEach(function () { - this.owner.features = { - versioning: true, - collaborators: 3, - compileGroup: 'priority', - compileTimeout: 22, - } - this.result = this.handler.buildOwnerAndMembersViews(this.members) - }) - - it('should produce an object with the right keys', function () { - expect(this.result).to.have.all.keys([ - 'owner', - 'ownerFeatures', - 'members', - ]) - }) - - it('should separate the owner from the members', 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) - 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 () { - expect(this.result.ownerFeatures).to.deep.equal(this.owner.features) - }) - - describe('when there is no owner', function () { - beforeEach(function () { - // remove the owner from members list - this.membersWithoutOwner = this.members.filter( - m => m.user._id !== this.owner._id - ) - this.result = this.handler.buildOwnerAndMembersViews( - this.membersWithoutOwner - ) - }) - - it('should produce an object with the right keys', function () { - expect(this.result).to.have.all.keys([ - 'owner', - 'ownerFeatures', - 'members', - ]) - }) - - it('should not separate out an owner', function () { - this.result.members.length.should.equal(this.membersWithoutOwner.length) - expect(this.result.owner).to.equal(null) - }) - - it('should not extract the ownerFeatures from the owner object', function () { - expect(this.result.ownerFeatures).to.equal(null) - }) - }) - }) })