From b5e2604041c6aaa54b4d6d4b51a8d2f7dc844d5b Mon Sep 17 00:00:00 2001 From: Mathias Jakobsen Date: Tue, 27 Sep 2022 12:23:36 +0100 Subject: [PATCH] [web] Upgrade restricted user access if they are invited members (#9401) * [web] Upgrade restricted user access if they are invited members Previously, if a user joined a project via a read-only link and later on joined the project via an invite, we would still treat them as restricted users, disabling chat and commenting. This patch changes that, so that we do *not* consider an invited user restricted. GitOrigin-RevId: e2acdfd29cc0687cb7276310a9c96d697087b21a --- .../Authorization/AuthorizationManager.js | 23 ++++++++++++++++--- .../Collaborators/CollaboratorsGetter.js | 3 +++ .../Features/Editor/EditorHttpController.js | 8 ++++++- .../src/Features/Project/ProjectController.js | 11 ++++++++- .../AuthorizationManagerTests.js | 20 ++++++++-------- .../src/Editor/EditorHttpControllerTests.js | 3 ++- .../src/Project/ProjectControllerTests.js | 2 ++ 7 files changed, 55 insertions(+), 15 deletions(-) diff --git a/services/web/app/src/Features/Authorization/AuthorizationManager.js b/services/web/app/src/Features/Authorization/AuthorizationManager.js index 08dfa82900..15272ac3ca 100644 --- a/services/web/app/src/Features/Authorization/AuthorizationManager.js +++ b/services/web/app/src/Features/Authorization/AuthorizationManager.js @@ -11,12 +11,19 @@ const Errors = require('../Errors/Errors') const { hasAdminAccess } = require('../Helpers/AdminAuthorizationHelper') const Settings = require('@overleaf/settings') -function isRestrictedUser(userId, privilegeLevel, isTokenMember) { +function isRestrictedUser( + userId, + privilegeLevel, + isTokenMember, + isInvitedMember +) { if (privilegeLevel === PrivilegeLevels.NONE) { return true } return ( - privilegeLevel === PrivilegeLevels.READ_ONLY && (isTokenMember || !userId) + privilegeLevel === PrivilegeLevels.READ_ONLY && + (isTokenMember || !userId) && + !isInvitedMember ) } @@ -30,7 +37,17 @@ async function isRestrictedUserForProject(userId, projectId, token) { userId, projectId ) - return isRestrictedUser(userId, privilegeLevel, isTokenMember) + const isInvitedMember = + await CollaboratorsGetter.promises.isUserInvitedMemberOfProject( + userId, + projectId + ) + return isRestrictedUser( + userId, + privilegeLevel, + isTokenMember, + isInvitedMember + ) } async function getPublicAccessLevel(projectId) { diff --git a/services/web/app/src/Features/Collaborators/CollaboratorsGetter.js b/services/web/app/src/Features/Collaborators/CollaboratorsGetter.js index 39077d28bf..45bad4ded4 100644 --- a/services/web/app/src/Features/Collaborators/CollaboratorsGetter.js +++ b/services/web/app/src/Features/Collaborators/CollaboratorsGetter.js @@ -118,6 +118,9 @@ async function getInvitedCollaboratorCount(projectId) { } async function isUserInvitedMemberOfProject(userId, projectId) { + if (!userId) { + return false + } const members = await getMemberIdsWithPrivilegeLevels(projectId) for (const member of members) { if ( diff --git a/services/web/app/src/Features/Editor/EditorHttpController.js b/services/web/app/src/Features/Editor/EditorHttpController.js index 316f9c6e52..33a38016ab 100644 --- a/services/web/app/src/Features/Editor/EditorHttpController.js +++ b/services/web/app/src/Features/Editor/EditorHttpController.js @@ -131,10 +131,16 @@ async function _buildJoinProjectView(req, projectId, userId) { userId, projectId ) + const isInvitedMember = + await CollaboratorsGetter.promises.isUserInvitedMemberOfProject( + userId, + projectId + ) const isRestrictedUser = AuthorizationManager.isRestrictedUser( userId, privilegeLevel, - isTokenMember + isTokenMember, + isInvitedMember ) return { project: ProjectEditorHandler.buildProjectModelView( diff --git a/services/web/app/src/Features/Project/ProjectController.js b/services/web/app/src/Features/Project/ProjectController.js index 8a7e635ebf..7bcfef1e22 100644 --- a/services/web/app/src/Features/Project/ProjectController.js +++ b/services/web/app/src/Features/Project/ProjectController.js @@ -833,6 +833,13 @@ const ProjectController = { } CollaboratorsGetter.userIsTokenMember(userId, projectId, cb) }, + isInvitedMember(cb) { + CollaboratorsGetter.isUserInvitedMemberOfProject( + userId, + projectId, + cb + ) + }, brandVariation: [ 'project', (results, cb) => { @@ -1060,6 +1067,7 @@ const ProjectController = { subscription, userIsMemberOfGroupSubscription, isTokenMember, + isInvitedMember, brandVariation, newSourceEditorAssignment, pdfjsAssignment, @@ -1220,7 +1228,8 @@ const ProjectController = { isRestrictedTokenMember: AuthorizationManager.isRestrictedUser( userId, privilegeLevel, - isTokenMember + isTokenMember, + isInvitedMember ), languages: Settings.languages, learnedWords, diff --git a/services/web/test/unit/src/Authorization/AuthorizationManagerTests.js b/services/web/test/unit/src/Authorization/AuthorizationManagerTests.js index 607d4eb949..5731f495f6 100644 --- a/services/web/test/unit/src/Authorization/AuthorizationManagerTests.js +++ b/services/web/test/unit/src/Authorization/AuthorizationManagerTests.js @@ -65,17 +65,19 @@ describe('AuthorizationManager', function () { describe('isRestrictedUser', function () { it('should produce the correct values', function () { const notRestrictedScenarios = [ - [null, 'readAndWrite', false], - ['id', 'readAndWrite', true], - ['id', 'readOnly', false], + [null, 'readAndWrite', false, false], + ['id', 'readAndWrite', true, false], + ['id', 'readAndWrite', true, true], + ['id', 'readOnly', false, false], + ['id', 'readOnly', false, true], ] const restrictedScenarios = [ - [null, 'readOnly', false], - ['id', 'readOnly', true], - [null, false, true], - [null, false, false], - ['id', false, true], - ['id', false, false], + [null, 'readOnly', false, false], + ['id', 'readOnly', true, false], + [null, false, true, false], + [null, false, false, false], + ['id', false, true, false], + ['id', false, false, false], ] for (const notRestrictedArgs of notRestrictedScenarios) { expect( diff --git a/services/web/test/unit/src/Editor/EditorHttpControllerTests.js b/services/web/test/unit/src/Editor/EditorHttpControllerTests.js index 07b79295da..58f990f36d 100644 --- a/services/web/test/unit/src/Editor/EditorHttpControllerTests.js +++ b/services/web/test/unit/src/Editor/EditorHttpControllerTests.js @@ -59,6 +59,7 @@ describe('EditorHttpController', function () { getInvitedMembersWithPrivilegeLevels: sinon .stub() .resolves(['members', 'mock']), + isUserInvitedMemberOfProject: sinon.stub().resolves(false), }, } this.CollaboratorsHandler = { @@ -234,7 +235,7 @@ describe('EditorHttpController', function () { this.req.query = { user_id: 'anonymous-user' } this.res.json.callsFake(() => done()) this.AuthorizationManager.isRestrictedUser - .withArgs(null, 'readOnly', false) + .withArgs(null, 'readOnly', false, false) .returns(true) this.AuthorizationManager.promises.getPrivilegeLevelForProject .withArgs(null, this.project._id, this.token) diff --git a/services/web/test/unit/src/Project/ProjectControllerTests.js b/services/web/test/unit/src/Project/ProjectControllerTests.js index cb29b8f8d6..b7d7b3d526 100644 --- a/services/web/test/unit/src/Project/ProjectControllerTests.js +++ b/services/web/test/unit/src/Project/ProjectControllerTests.js @@ -96,6 +96,7 @@ describe('ProjectController', function () { } this.CollaboratorsGetter = { userIsTokenMember: sinon.stub().callsArgWith(2, null, false), + isUserInvitedMemberOfProject: sinon.stub().callsArgWith(2, null, true), } this.ProjectEntityHandler = {} this.NotificationBuilder = { @@ -1014,6 +1015,7 @@ describe('ProjectController', function () { }) it('should add isRestrictedTokenMember', function (done) { + this.AuthorizationManager.isRestrictedUser.returns(false) this.res.render = (pageName, opts) => { opts.isRestrictedTokenMember.should.exist opts.isRestrictedTokenMember.should.equal(false)