From 01e3409eb477bf619630744745fb90d82cfe32d6 Mon Sep 17 00:00:00 2001 From: Miguel Serrano Date: Mon, 3 Jul 2023 13:11:39 +0200 Subject: [PATCH] Merge pull request #13485 from overleaf/msm-share-modal-fetch-tokens [web] Fetch share tokens instead of sending via websocket GitOrigin-RevId: f97bb91ca3ceb410fe860bf1c7802d8157d9f8b4 --- .../Collaborators/CollaboratorsController.js | 36 +++ .../Collaborators/CollaboratorsGetter.js | 36 +++ .../Collaborators/CollaboratorsRouter.js | 11 + .../src/Features/Editor/EditorController.js | 15 +- .../Features/Editor/EditorHttpController.js | 2 - .../src/Features/Project/ProjectController.js | 3 +- .../Features/Project/ProjectDetailsHandler.js | 3 +- .../Features/Project/ProjectEditorHandler.js | 1 - .../TokenAccess/TokenAccessHandler.js | 17 -- .../components/link-sharing.js | 31 ++- .../react-share-project-modal-controller.js | 10 - .../js/shared/context/mock/mock-ide.js | 4 - .../js/shared/context/project-context.js | 7 - .../stories/share-project-modal.stories.js | 7 +- .../test/acceptance/src/TokenAccessTests.js | 250 ++++++++++++++++++ .../CollaboratorsControllerTests.js | 4 + .../Collaborators/CollaboratorsGetterTests.js | 61 +++++ .../unit/src/Editor/EditorControllerTests.js | 20 +- .../src/Editor/EditorHttpControllerTests.js | 1 - .../src/Project/ProjectControllerTests.js | 1 - .../src/Project/ProjectDetailsHandlerTests.js | 18 -- .../TokenAccess/TokenAccessHandlerTests.js | 31 --- 22 files changed, 435 insertions(+), 134 deletions(-) diff --git a/services/web/app/src/Features/Collaborators/CollaboratorsController.js b/services/web/app/src/Features/Collaborators/CollaboratorsController.js index 69a6583693..94b5687322 100644 --- a/services/web/app/src/Features/Collaborators/CollaboratorsController.js +++ b/services/web/app/src/Features/Collaborators/CollaboratorsController.js @@ -11,6 +11,7 @@ const Errors = require('../Errors/Errors') const logger = require('@overleaf/logger') const { expressify } = require('../../util/promises') const { hasAdminAccess } = require('../Helpers/AdminAuthorizationHelper') +const TokenAccessHandler = require('../TokenAccess/TokenAccessHandler') module.exports = { removeUserFromProject: expressify(removeUserFromProject), @@ -18,6 +19,7 @@ module.exports = { getAllMembers: expressify(getAllMembers), setCollaboratorInfo: expressify(setCollaboratorInfo), transferOwnership: expressify(transferOwnership), + getShareTokens: expressify(getShareTokens), } async function removeUserFromProject(req, res, next) { @@ -114,3 +116,37 @@ async function _removeUserIdFromProject(projectId, userId) { ) await TagsHandler.promises.removeProjectFromAllTags(userId, projectId) } + +async function getShareTokens(req, res) { + const projectId = req.params.Project_id + const userId = SessionManager.getLoggedInUserId(req.session) + + let tokens + if (userId) { + tokens = await CollaboratorsGetter.promises.getPublicShareTokens( + ObjectId(userId), + ObjectId(projectId) + ) + } else { + // anonymous access, the token is already available in the session + const readOnly = TokenAccessHandler.getRequestToken(req, projectId) + tokens = { readOnly } + } + if (!tokens) { + return res.sendStatus(403) + } + + if (tokens.readOnly || tokens.readAndWrite) { + logger.info( + { + projectId, + userId: userId || 'anonymous', + ip: req.ip, + tokens: Object.keys(tokens), + }, + 'project tokens accessed' + ) + } + + res.json(tokens) +} diff --git a/services/web/app/src/Features/Collaborators/CollaboratorsGetter.js b/services/web/app/src/Features/Collaborators/CollaboratorsGetter.js index 45bad4ded4..06a84d1102 100644 --- a/services/web/app/src/Features/Collaborators/CollaboratorsGetter.js +++ b/services/web/app/src/Features/Collaborators/CollaboratorsGetter.js @@ -25,6 +25,7 @@ module.exports = { getInvitedCollaboratorCount: callbackify(getInvitedCollaboratorCount), getProjectsUserIsMemberOf: callbackify(getProjectsUserIsMemberOf), isUserInvitedMemberOfProject: callbackify(isUserInvitedMemberOfProject), + getPublicShareTokens: callbackify(getPublicShareTokens), userIsTokenMember: callbackify(userIsTokenMember), getAllInvitedMembers: callbackify(getAllInvitedMembers), promises: { @@ -37,6 +38,7 @@ module.exports = { getInvitedCollaboratorCount, getProjectsUserIsMemberOf, isUserInvitedMemberOfProject, + getPublicShareTokens, userIsTokenMember, getAllInvitedMembers, }, @@ -133,6 +135,40 @@ async function isUserInvitedMemberOfProject(userId, projectId) { return false } +async function getPublicShareTokens(userId, projectId) { + const memberInfo = await Project.findOne( + { + _id: projectId, + }, + { + isOwner: { $eq: ['$owner_ref', userId] }, + hasTokenReadOnlyAccess: { + $and: [ + { $in: [userId, '$tokenAccessReadOnly_refs'] }, + { $eq: ['$publicAccesLevel', PublicAccessLevels.TOKEN_BASED] }, + ], + }, + tokens: 1, + } + ) + .lean() + .exec() + + if (!memberInfo) { + return null + } + + if (memberInfo.isOwner) { + return memberInfo.tokens + } else if (memberInfo.hasTokenReadOnlyAccess) { + return { + readOnly: memberInfo.tokens.readOnly, + } + } else { + return {} + } +} + async function getProjectsUserIsMemberOf(userId, fields) { const limit = pLimit(2) const [readAndWrite, readOnly, tokenReadAndWrite, tokenReadOnly] = diff --git a/services/web/app/src/Features/Collaborators/CollaboratorsRouter.js b/services/web/app/src/Features/Collaborators/CollaboratorsRouter.js index e21545295f..9a66bc0144 100644 --- a/services/web/app/src/Features/Collaborators/CollaboratorsRouter.js +++ b/services/web/app/src/Features/Collaborators/CollaboratorsRouter.js @@ -22,6 +22,10 @@ const rateLimiters = { points: 200, duration: 60 * 10, }), + getProjectTokens: new RateLimiter('get-project-tokens', { + points: 200, + duration: 60 * 10, + }), } module.exports = { @@ -139,5 +143,12 @@ module.exports = { CollaboratorsInviteController.acceptInvite, AnalyticsRegistrationSourceMiddleware.clearSource() ) + + webRouter.get( + '/project/:Project_id/tokens', + RateLimiterMiddleware.rateLimit(rateLimiters.getProjectTokens), + AuthorizationMiddleware.ensureUserCanReadProject, + CollaboratorsController.getShareTokens + ) }, } diff --git a/services/web/app/src/Features/Editor/EditorController.js b/services/web/app/src/Features/Editor/EditorController.js index 87dfe7f208..4224bbed72 100644 --- a/services/web/app/src/Features/Editor/EditorController.js +++ b/services/web/app/src/Features/Editor/EditorController.js @@ -581,20 +581,7 @@ const EditorController = { { newAccessLevel } ) if (newAccessLevel === PublicAccessLevels.TOKEN_BASED) { - ProjectDetailsHandler.ensureTokensArePresent( - projectId, - function (err, tokens) { - if (err) { - return callback(err) - } - EditorRealTimeController.emitToRoom( - projectId, - 'project:tokens:changed', - { tokens } - ) - callback() - } - ) + ProjectDetailsHandler.ensureTokensArePresent(projectId, callback) } else { callback() } diff --git a/services/web/app/src/Features/Editor/EditorHttpController.js b/services/web/app/src/Features/Editor/EditorHttpController.js index df1e2b1351..c0fe056472 100644 --- a/services/web/app/src/Features/Editor/EditorHttpController.js +++ b/services/web/app/src/Features/Editor/EditorHttpController.js @@ -67,8 +67,6 @@ async function joinProject(req, res, next) { if (!project) { return res.sendStatus(403) } - // Hide access tokens if this is not the project owner - TokenAccessHandler.protectTokens(project, privilegeLevel) // Hide sensitive data if the user is restricted if (isRestrictedUser) { project.owner = { _id: project.owner._id } diff --git a/services/web/app/src/Features/Project/ProjectController.js b/services/web/app/src/Features/Project/ProjectController.js index 4ec958f735..c7d8e68ca1 100644 --- a/services/web/app/src/Features/Project/ProjectController.js +++ b/services/web/app/src/Features/Project/ProjectController.js @@ -343,7 +343,7 @@ const ProjectController = { const userId = SessionManager.getLoggedInUserId(req.session) ProjectGetter.findAllUsersProjects( userId, - 'name lastUpdated publicAccesLevel archived trashed owner_ref tokens', + 'name lastUpdated publicAccesLevel archived trashed owner_ref', (err, projects) => { if (err != null) { return next(err) @@ -1091,7 +1091,6 @@ const ProjectController = { // If a project is simultaneously trashed and archived, we will consider it archived but not trashed. const trashed = ProjectHelper.isTrashed(project, userId) && !archived - TokenAccessHandler.protectTokens(project, accessLevel) const model = { id: project._id, name: project.name, diff --git a/services/web/app/src/Features/Project/ProjectDetailsHandler.js b/services/web/app/src/Features/Project/ProjectDetailsHandler.js index 90c5cca7ee..1c90ec9e0b 100644 --- a/services/web/app/src/Features/Project/ProjectDetailsHandler.js +++ b/services/web/app/src/Features/Project/ProjectDetailsHandler.js @@ -207,14 +207,13 @@ async function ensureTokensArePresent(projectId) { project.tokens.readOnly != null && project.tokens.readAndWrite != null ) { - return project.tokens + return } await _generateTokens(project) await Project.updateOne( { _id: projectId }, { $set: { tokens: project.tokens } } ).exec() - return project.tokens } async function clearTokens(projectId) { diff --git a/services/web/app/src/Features/Project/ProjectEditorHandler.js b/services/web/app/src/Features/Project/ProjectEditorHandler.js index bf151215aa..00b7eb22f7 100644 --- a/services/web/app/src/Features/Project/ProjectEditorHandler.js +++ b/services/web/app/src/Features/Project/ProjectEditorHandler.js @@ -36,7 +36,6 @@ module.exports = ProjectEditorHandler = { ), members: [], invites: this.buildInvitesView(invites), - tokens: project.tokens, imageName: project.imageName != null ? Path.basename(project.imageName) diff --git a/services/web/app/src/Features/TokenAccess/TokenAccessHandler.js b/services/web/app/src/Features/TokenAccess/TokenAccessHandler.js index 0c261460d2..16ef4248f7 100644 --- a/services/web/app/src/Features/TokenAccess/TokenAccessHandler.js +++ b/services/web/app/src/Features/TokenAccess/TokenAccessHandler.js @@ -246,22 +246,6 @@ const TokenAccessHandler = { }) }, - protectTokens(project, privilegeLevel) { - if (!project || !project.tokens) { - return - } - if (privilegeLevel === PrivilegeLevels.OWNER) { - return - } - if (privilegeLevel !== PrivilegeLevels.READ_AND_WRITE) { - project.tokens.readAndWrite = '' - project.tokens.readAndWritePrefix = '' - } - if (privilegeLevel !== PrivilegeLevels.READ_ONLY) { - project.tokens.readOnly = '' - } - }, - getV1DocPublishedInfo(token, callback) { // default to allowing access if (!Settings.apis.v1 || !Settings.apis.v1.url) { @@ -304,7 +288,6 @@ TokenAccessHandler.promises = promisifyAll(TokenAccessHandler, { '_projectFindOne', 'grantSessionTokenAccess', 'getRequestToken', - 'protectTokens', ], multiResult: { validateTokenForAnonymousAccess: ['isValidReadAndWrite', 'isValidReadOnly'], diff --git a/services/web/frontend/js/features/share-project-modal/components/link-sharing.js b/services/web/frontend/js/features/share-project-modal/components/link-sharing.js index 07f762d958..512c1154e7 100644 --- a/services/web/frontend/js/features/share-project-modal/components/link-sharing.js +++ b/services/web/frontend/js/features/share-project-modal/components/link-sharing.js @@ -1,4 +1,4 @@ -import { useCallback, useState } from 'react' +import { useCallback, useState, useEffect } from 'react' import PropTypes from 'prop-types' import { Button, Col, Row } from 'react-bootstrap' import { Trans } from 'react-i18next' @@ -10,6 +10,8 @@ import CopyLink from '../../../shared/components/copy-link' import { useProjectContext } from '../../../shared/context/project-context' import * as eventTracking from '../../../infrastructure/event-tracking' import { useUserContext } from '../../../shared/context/user-context' +import { getJSON } from '../../../infrastructure/fetch-json' +import useAbortController from '../../../shared/hooks/use-abort-controller' export default function LinkSharing({ canAddCollaborators }) { const [inflight, setInflight] = useState(false) @@ -27,8 +29,7 @@ export default function LinkSharing({ canAddCollaborators }) { ) .then(() => { // NOTE: not calling `updateProject` here as it receives data via - // project:publicAccessLevel:changed and project:tokens:changed - // over the websocket connection + // project:publicAccessLevel:changed over the websocket connection // TODO: eventTracking.sendMB('project-make-token-based') when newPublicAccessLevel is 'tokenBased' }) .finally(() => { @@ -106,7 +107,17 @@ PrivateSharing.propTypes = { } function TokenBasedSharing({ setAccessLevel, inflight, canAddCollaborators }) { - const { tokens } = useProjectContext() + const { _id: projectId } = useProjectContext() + + const [tokens, setTokens] = useState(null) + + const { signal } = useAbortController() + + useEffect(() => { + getJSON(`/project/${projectId}/tokens`, { signal }) + .then(data => setTokens(data)) + .catch(error => console.error(error)) + }, [projectId, signal]) return ( @@ -194,7 +205,17 @@ LegacySharing.propTypes = { } export function ReadOnlyTokenLink() { - const { tokens } = useProjectContext() + const { _id: projectId } = useProjectContext() + + const [tokens, setTokens] = useState(null) + + const { signal } = useAbortController() + + useEffect(() => { + getJSON(`/project/${projectId}/tokens`, { signal }) + .then(data => setTokens(data)) + .catch(error => console.error(error)) + }, [projectId, signal]) return ( diff --git a/services/web/frontend/js/features/share-project-modal/controllers/react-share-project-modal-controller.js b/services/web/frontend/js/features/share-project-modal/controllers/react-share-project-modal-controller.js index ba28d12a26..5b88a169cb 100644 --- a/services/web/frontend/js/features/share-project-modal/controllers/react-share-project-modal-controller.js +++ b/services/web/frontend/js/features/share-project-modal/controllers/react-share-project-modal-controller.js @@ -31,16 +31,6 @@ export default App.controller( }) } - /* tokens */ - - ide.socket.on('project:tokens:changed', data => { - if (data.tokens != null) { - $scope.$applyAsync(() => { - $scope.project.tokens = data.tokens - }) - } - }) - ide.socket.on('project:membership:changed', data => { if (data.members) { listProjectMembers($scope.project._id) diff --git a/services/web/frontend/js/shared/context/mock/mock-ide.js b/services/web/frontend/js/shared/context/mock/mock-ide.js index b4a1bc2582..52747022c6 100644 --- a/services/web/frontend/js/shared/context/mock/mock-ide.js +++ b/services/web/frontend/js/shared/context/mock/mock-ide.js @@ -27,10 +27,6 @@ export const getMockIde = () => { zotero: false, }, publicAccessLevel: '', - tokens: { - readOnly: '', - readAndWrite: '', - }, owner: { _id: '', email: '', diff --git a/services/web/frontend/js/shared/context/project-context.js b/services/web/frontend/js/shared/context/project-context.js index 91e63a44de..cf8e53cf4c 100644 --- a/services/web/frontend/js/shared/context/project-context.js +++ b/services/web/frontend/js/shared/context/project-context.js @@ -28,10 +28,6 @@ export const projectShape = { versioning: PropTypes.bool, }), publicAccessLevel: PropTypes.string, - tokens: PropTypes.shape({ - readOnly: PropTypes.string, - readAndWrite: PropTypes.string, - }), owner: PropTypes.shape({ _id: PropTypes.string.isRequired, email: PropTypes.string.isRequired, @@ -81,7 +77,6 @@ export function ProjectProvider({ children }) { invites, features, publicAccesLevel: publicAccessLevel, - tokens, owner, } = project || projectFallback @@ -94,7 +89,6 @@ export function ProjectProvider({ children }) { invites, features, publicAccessLevel, - tokens, owner, } }, [ @@ -105,7 +99,6 @@ export function ProjectProvider({ children }) { invites, features, publicAccessLevel, - tokens, owner, ]) diff --git a/services/web/frontend/stories/share-project-modal.stories.js b/services/web/frontend/stories/share-project-modal.stories.js index 07bfe4cd30..aaf52a8751 100644 --- a/services/web/frontend/stories/share-project-modal.stories.js +++ b/services/web/frontend/stories/share-project-modal.stories.js @@ -39,7 +39,6 @@ export const LinkSharingLoading = args => { project: { ...args.project, publicAccesLevel: 'tokenBased', - tokens: undefined, }, }) @@ -160,6 +159,12 @@ function setupFetchMock(fetchMock) { fetchMock // list contacts .get('express:/user/contacts', { contacts }, { delay }) + // access tokens + .get( + 'express:/project/:projectId/tokens', + { tokens: project.tokens }, + { delay } + ) // change privacy setting .post('express:/project/:projectId/settings/admin', 200, { delay }) // update project member (e.g. set privilege level) diff --git a/services/web/test/acceptance/src/TokenAccessTests.js b/services/web/test/acceptance/src/TokenAccessTests.js index cbd961375c..35ac5ec155 100644 --- a/services/web/test/acceptance/src/TokenAccessTests.js +++ b/services/web/test/acceptance/src/TokenAccessTests.js @@ -225,6 +225,18 @@ const tryAnonContentAccess = (user, projectId, token, test, callback) => { ) } +const tryFetchProjectTokens = (user, projectId, callback) => { + user.request.get( + { url: `/project/${projectId}/tokens`, json: true }, + (error, response, body) => { + if (error) { + return callback(error) + } + callback(null, response, body) + } + ) +} + describe('TokenAccess', function () { beforeEach(function (done) { this.timeout(90000) @@ -293,6 +305,55 @@ describe('TokenAccess', function () { done ) }) + + it('should deny access to access tokens', function (done) { + tryFetchProjectTokens(this.other1, this.projectId, (error, response) => { + expect(error).to.equal(null) + expect(response.statusCode).to.equal(403) + done() + }) + }) + }) + + describe('owner', function () { + beforeEach(function (done) { + this.projectName = `token-owner-test${Math.random()}` + this.owner.createProject(this.projectName, (err, projectId) => { + if (err != null) { + return done(err) + } + this.projectId = projectId + this.owner.makeTokenBased(this.projectId, err => { + if (err != null) { + return done(err) + } + this.owner.getProject(this.projectId, (err, project) => { + if (err != null) { + return done(err) + } + this.tokens = project.tokens + done() + }) + }) + }) + }) + + it('should be able to fetch read-only and read-write tokens', function (done) { + tryFetchProjectTokens( + this.owner, + this.projectId, + (error, response, body) => { + expect(error).to.equal(null) + expect(response.statusCode).to.equal(200) + expect(body).to.deep.equal({ + readOnly: this.tokens.readOnly, + readAndWrite: this.tokens.readAndWrite, + readAndWritePrefix: this.tokens.readAndWritePrefix, + }) + done() + } + ) + }) }) describe('read-only token', function () { @@ -407,6 +468,42 @@ describe('TokenAccess', function () { ) }) + it('should allow the user to fetch the read-only token', function (done) { + async.series( + [ + cb => { + // accept token + tryReadOnlyTokenAccept( + this.other1, + this.tokens.readOnly, + (response, body) => { + expect(response.statusCode).to.equal(200) + }, + (response, body) => { + expect(response.statusCode).to.equal(200) + expect(body.redirect).to.equal(`/project/${this.projectId}`) + expect(body.tokenAccessGranted).to.equal('readOnly') + }, + cb + ) + }, + cb => { + tryFetchProjectTokens( + this.other1, + this.projectId, + (error, response, body) => { + expect(error).to.equal(null) + expect(response.statusCode).to.equal(200) + expect(body).to.deep.equal({ readOnly: this.tokens.readOnly }) + cb() + } + ) + }, + ], + done + ) + }) + it('should redirect the admin to the project (with rw access)', function (done) { async.series( [ @@ -496,6 +593,14 @@ describe('TokenAccess', function () { ) }) }) + + it('should deny access to access tokens', function (done) { + tryFetchProjectTokens(this.other1, this.projectId, (error, response) => { + expect(error).to.equal(null) + expect(response.statusCode).to.equal(403) + done() + }) + }) }) describe('anonymous read-only token', function () { @@ -580,6 +685,40 @@ describe('TokenAccess', function () { ) }) + it('should allow the anonymous user to fetch the read-only token', function (done) { + async.series( + [ + cb => + tryReadOnlyTokenAccess( + this.anon, + this.tokens.readOnly, + (response, body) => { + expect(response.statusCode).to.equal(200) + }, + (response, body) => { + expect(response.statusCode).to.equal(200) + expect(body.redirect).to.equal(`/project/${this.projectId}`) + expect(body.grantAnonymousAccess).to.equal('readOnly') + }, + cb + ), + cb => { + tryFetchProjectTokens( + this.anon, + this.projectId, + (error, response, body) => { + expect(error).to.equal(null) + expect(response.statusCode).to.equal(200) + expect(body).to.deep.equal({ readOnly: this.tokens.readOnly }) + cb() + } + ) + }, + ], + done + ) + }) + describe('made private again', function () { beforeEach(function (done) { this.owner.makePrivate(this.projectId, () => setTimeout(done, 1000)) @@ -632,6 +771,14 @@ describe('TokenAccess', function () { done ) }) + + it('should deny access to access tokens', function (done) { + tryFetchProjectTokens(this.anon, this.projectId, (error, response) => { + expect(error).to.equal(null) + expect(response.statusCode).to.equal(403) + done() + }) + }) }) }) @@ -742,6 +889,40 @@ describe('TokenAccess', function () { ) }) + it('fetching access tokens returns an empty object', function (done) { + async.series( + [ + cb => + tryReadAndWriteTokenAccept( + this.other1, + this.tokens.readAndWrite, + (response, body) => { + expect(response.statusCode).to.equal(200) + }, + (response, body) => { + expect(response.statusCode).to.equal(200) + expect(body.redirect).to.equal(`/project/${this.projectId}`) + expect(body.tokenAccessGranted).to.equal('readAndWrite') + }, + cb + ), + cb => { + tryFetchProjectTokens( + this.other1, + this.projectId, + (error, response, body) => { + expect(error).to.equal(null) + expect(response.statusCode).to.equal(200) + expect(body).to.deep.equal({}) + cb() + } + ) + }, + ], + done + ) + }) + describe('upgrading from a read-only token', function () { beforeEach(function (done) { this.owner.createProject( @@ -928,6 +1109,18 @@ describe('TokenAccess', function () { done ) }) + + it('should deny access to access tokens', function (done) { + tryFetchProjectTokens( + this.other1, + this.projectId, + (error, response) => { + expect(error).to.equal(null) + expect(response.statusCode).to.equal(403) + done() + } + ) + }) }) }) @@ -1006,6 +1199,14 @@ describe('TokenAccess', function () { ) }) + it('should deny access to access tokens', function (done) { + tryFetchProjectTokens(this.anon, this.projectId, (error, response) => { + expect(error).to.equal(null) + expect(response.statusCode).to.equal(403) + done() + }) + }) + it('should require login if project does not exist', function (done) { async.series( [ @@ -1164,6 +1365,18 @@ describe('TokenAccess', function () { done ) }) + + it('should deny access to access tokens', function (done) { + tryFetchProjectTokens( + this.anon, + this.projectId, + (error, response) => { + expect(error).to.equal(null) + expect(response.statusCode).to.equal(403) + done() + } + ) + }) }) it('should 404 if project does not exist', function (done) { @@ -1458,5 +1671,42 @@ describe('TokenAccess', function () { done ) }) + + it('should deny access to access tokens with a 404', function (done) { + async.series( + [ + // delete project + cb => { + this.owner.deleteProject(this.projectId, cb) + }, + cb => { + // use read-only token + tryReadOnlyTokenAccess( + this.other1, + this.tokens.readOnly, + (response, body) => { + expect(response.statusCode).to.equal(200) + }, + (response, body) => { + expect(response.statusCode).to.equal(404) + }, + cb + ) + }, + cb => { + tryFetchProjectTokens( + this.other1, + this.projectId, + (error, response) => { + expect(error).to.equal(null) + expect(response.statusCode).to.equal(404) + cb() + } + ) + }, + ], + done + ) + }) }) }) diff --git a/services/web/test/unit/src/Collaborators/CollaboratorsControllerTests.js b/services/web/test/unit/src/Collaborators/CollaboratorsControllerTests.js index ad56d6495a..22c34a31a0 100644 --- a/services/web/test/unit/src/Collaborators/CollaboratorsControllerTests.js +++ b/services/web/test/unit/src/Collaborators/CollaboratorsControllerTests.js @@ -50,6 +50,9 @@ describe('CollaboratorsController', function () { transferOwnership: sinon.stub().resolves(), }, } + this.TokenAccessHandler = { + getRequestToken: sinon.stub().returns('access-token'), + } this.CollaboratorsController = SandboxedModule.require(MODULE_PATH, { requires: { @@ -61,6 +64,7 @@ describe('CollaboratorsController', function () { '../../Features/Errors/HttpErrorHandler': this.HttpErrorHandler, '../Tags/TagsHandler': this.TagsHandler, '../Authentication/SessionManager': this.SessionManager, + '../TokenAccess/TokenAccessHandler': this.TokenAccessHandler, }, }) }) diff --git a/services/web/test/unit/src/Collaborators/CollaboratorsGetterTests.js b/services/web/test/unit/src/Collaborators/CollaboratorsGetterTests.js index 9857c9fde0..318888dcdd 100644 --- a/services/web/test/unit/src/Collaborators/CollaboratorsGetterTests.js +++ b/services/web/test/unit/src/Collaborators/CollaboratorsGetterTests.js @@ -30,6 +30,11 @@ describe('CollaboratorsGetter', function () { tokenAccessReadAndWrite_refs: [this.readWriteTokenRef], tokenAccessReadOnly_refs: [this.readOnlyTokenRef], publicAccesLevel: 'tokenBased', + tokens: { + readOnly: 'ro', + readAndWrite: 'rw', + readAndWritePrefix: 'pre', + }, } this.UserGetter = { @@ -359,4 +364,60 @@ describe('CollaboratorsGetter', function () { expect(isMember).to.be.false }) }) + + describe('getPublicShareTokens', function () { + const userMock = ObjectId() + + it('should return null when the project is not found', async function () { + this.ProjectMock.expects('findOne').chain('exec').resolves(undefined) + const tokens = + await this.CollaboratorsGetter.promises.getPublicShareTokens( + userMock, + this.project._id + ) + expect(tokens).to.be.null + }) + + it('should return an empty object when the user is not owner or read-only collaborator', async function () { + this.ProjectMock.expects('findOne').chain('exec').resolves(this.project) + const tokens = + await this.CollaboratorsGetter.promises.getPublicShareTokens( + userMock, + this.project._id + ) + expect(tokens).to.deep.equal({}) + }) + + describe('when the user is a read-only token collaborator', function () { + it('should return the read-only token', async function () { + this.ProjectMock.expects('findOne') + .chain('exec') + .resolves({ hasTokenReadOnlyAccess: true, ...this.project }) + + const tokens = + await this.CollaboratorsGetter.promises.getPublicShareTokens( + userMock, + this.project._id + ) + expect(tokens).to.deep.equal({ readOnly: tokens.readOnly }) + }) + }) + + describe('when the user is the owner of the project', function () { + beforeEach(function () { + this.ProjectMock.expects('findOne') + .chain('exec') + .resolves({ isOwner: true, ...this.project }) + }) + + it('should return all the tokens', async function () { + const tokens = + await this.CollaboratorsGetter.promises.getPublicShareTokens( + userMock, + this.project._id + ) + expect(tokens).to.deep.equal(tokens) + }) + }) + }) }) diff --git a/services/web/test/unit/src/Editor/EditorControllerTests.js b/services/web/test/unit/src/Editor/EditorControllerTests.js index c52c82cc79..4b453386bc 100644 --- a/services/web/test/unit/src/Editor/EditorControllerTests.js +++ b/services/web/test/unit/src/Editor/EditorControllerTests.js @@ -873,7 +873,7 @@ describe('EditorController', function () { this.newAccessLevel = 'private' this.ProjectDetailsHandler.ensureTokensArePresent = sinon .stub() - .yields(null, this.tokens) + .yields() return this.EditorController.setPublicAccessLevel( this.project_id, this.newAccessLevel, @@ -898,14 +898,6 @@ describe('EditorController', function () { .calledWith(this.project_id) .should.equal(false) }) - - it('should not broadcast a token change', function () { - return this.EditorRealTimeController.emitToRoom - .calledWith(this.project_id, 'project:tokens:changed', { - tokens: this.tokens, - }) - .should.equal(false) - }) }) describe('when setting to tokenBased', function () { @@ -914,7 +906,7 @@ describe('EditorController', function () { this.tokens = { readOnly: 'aaa', readAndWrite: '42bbb' } this.ProjectDetailsHandler.ensureTokensArePresent = sinon .stub() - .yields(null, this.tokens) + .yields() return this.EditorController.setPublicAccessLevel( this.project_id, this.newAccessLevel, @@ -939,14 +931,6 @@ describe('EditorController', function () { .calledWith(this.project_id) .should.equal(true) }) - - it('should broadcast the token change too', function () { - return this.EditorRealTimeController.emitToRoom - .calledWith(this.project_id, 'project:tokens:changed', { - tokens: this.tokens, - }) - .should.equal(true) - }) }) }) diff --git a/services/web/test/unit/src/Editor/EditorHttpControllerTests.js b/services/web/test/unit/src/Editor/EditorHttpControllerTests.js index a7f8ed7c08..e8a443018f 100644 --- a/services/web/test/unit/src/Editor/EditorHttpControllerTests.js +++ b/services/web/test/unit/src/Editor/EditorHttpControllerTests.js @@ -113,7 +113,6 @@ describe('EditorHttpController', function () { this.Metrics = { inc: sinon.stub() } this.TokenAccessHandler = { getRequestToken: sinon.stub().returns(this.token), - protectTokens: sinon.stub(), } this.SessionManager = { getLoggedInUserId: sinon.stub().returns(this.user._id), diff --git a/services/web/test/unit/src/Project/ProjectControllerTests.js b/services/web/test/unit/src/Project/ProjectControllerTests.js index f048da68bd..4d7372f306 100644 --- a/services/web/test/unit/src/Project/ProjectControllerTests.js +++ b/services/web/test/unit/src/Project/ProjectControllerTests.js @@ -90,7 +90,6 @@ describe('ProjectController', function () { } this.TokenAccessHandler = { getRequestToken: sinon.stub().returns(this.token), - protectTokens: sinon.stub(), } this.CollaboratorsGetter = { userIsTokenMember: sinon.stub().callsArgWith(2, null, false), diff --git a/services/web/test/unit/src/Project/ProjectDetailsHandlerTests.js b/services/web/test/unit/src/Project/ProjectDetailsHandlerTests.js index fe4d2885ca..d11626f6e7 100644 --- a/services/web/test/unit/src/Project/ProjectDetailsHandlerTests.js +++ b/services/web/test/unit/src/Project/ProjectDetailsHandlerTests.js @@ -511,13 +511,6 @@ describe('ProjectDetailsHandler', function () { await this.handler.promises.ensureTokensArePresent(this.project._id) expect(this.ProjectModel.updateOne).not.to.have.been.called }) - - it('should produce the tokens without error', async function () { - const tokens = await this.handler.promises.ensureTokensArePresent( - this.project._id - ) - expect(tokens).to.deep.equal(this.project.tokens) - }) }) describe('when tokens are missing', function () { @@ -566,17 +559,6 @@ describe('ProjectDetailsHandler', function () { } ) }) - - it('should produce the tokens without error', async function () { - const tokens = await this.handler.promises.ensureTokensArePresent( - this.project._id - ) - expect(tokens).to.deep.equal({ - readOnly: this.readOnlyToken, - readAndWrite: this.readAndWriteToken, - readAndWritePrefix: this.readAndWriteTokenPrefix, - }) - }) }) }) diff --git a/services/web/test/unit/src/TokenAccess/TokenAccessHandlerTests.js b/services/web/test/unit/src/TokenAccess/TokenAccessHandlerTests.js index e5b1c21191..d85e2d867f 100644 --- a/services/web/test/unit/src/TokenAccess/TokenAccessHandlerTests.js +++ b/services/web/test/unit/src/TokenAccess/TokenAccessHandlerTests.js @@ -513,37 +513,6 @@ describe('TokenAccessHandler', function () { }) }) - describe('protectTokens', function () { - beforeEach(function () { - return (this.project = { - tokens: { - readAndWrite: 'rw', - readOnly: 'ro', - readAndWritePrefix: 'pre', - }, - }) - }) - - it('should hide write token from read-only user', function () { - this.TokenAccessHandler.protectTokens(this.project, 'readOnly') - expect(this.project.tokens.readAndWrite).to.equal('') - expect(this.project.tokens.readAndWritePrefix).to.equal('') - return expect(this.project.tokens.readOnly).to.equal('ro') - }) - - it('should hide read token from read-write user', function () { - this.TokenAccessHandler.protectTokens(this.project, 'readAndWrite') - expect(this.project.tokens.readAndWrite).to.equal('rw') - return expect(this.project.tokens.readOnly).to.equal('') - }) - - it('should leave tokens in place for owner', function () { - this.TokenAccessHandler.protectTokens(this.project, 'owner') - expect(this.project.tokens.readAndWrite).to.equal('rw') - return expect(this.project.tokens.readOnly).to.equal('ro') - }) - }) - describe('getDocPublishedInfo', function () { beforeEach(function () { return (this.callback = sinon.stub())