[web] skip fetching members and invites for restricted users (#25673)

* [web] hide sensitive data from joinProject when building project view

* [web] skip fetching members and invites for restricted users

* [web] fix owner features in joinProject view

* [web] separate invited members from owner

* [web] skip fetching users with empty members  list

* [web] split await chain

Co-authored-by: Antoine Clausse <antoine.clausse@overleaf.com>

* [web] remove spurious parentheses

* [web] remove dead code

Co-authored-by: Antoine Clausse <antoine.clausse@overleaf.com>

---------

Co-authored-by: Antoine Clausse <antoine.clausse@overleaf.com>
GitOrigin-RevId: 5b4d874f974971e9c14d7412620805f8ebf63541
This commit is contained in:
Jakob Ackermann
2025-06-02 13:38:40 +02:00
committed by Copybot
parent 93c221775e
commit b45ffbf055
6 changed files with 268 additions and 189 deletions

View File

@@ -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<LoadedProjectMember[]>}
*/
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<LoadedProjectMember|undefined>}
*/
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, {

View File

@@ -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,

View File

@@ -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 {

View File

@@ -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])
})
})

View File

@@ -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,

View File

@@ -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)
})
})
})
})