From 8da063d64027923a62a06ef6faa4ecdb94780192 Mon Sep 17 00:00:00 2001 From: Jessica Lawshe <5312836+lawshe@users.noreply.github.com> Date: Tue, 24 Oct 2023 08:47:19 -0500 Subject: [PATCH] Merge pull request #15326 from overleaf/jel-link-sharing [web] Add prefix of token hash to link sharing URLs GitOrigin-RevId: 4b764c076a335768ab261dd1e181d90ce00fd1a2 --- .../Collaborators/CollaboratorsController.js | 12 + .../TokenAccess/TokenAccessController.js | 14 +- .../TokenAccess/TokenAccessHandler.js | 27 +++ .../components/link-sharing.jsx | 20 +- .../components/share-modal-body.jsx | 8 +- services/web/frontend/js/main/token-access.js | 2 +- .../test/acceptance/src/TokenAccessTests.js | 12 +- .../components/share-project-modal.test.jsx | 15 ++ .../CollaboratorsControllerTests.js | 1 + .../TokenAccess/TokenAccessControllerTests.js | 42 +++- .../TokenAccess/TokenAccessHandlerTests.js | 217 +++++++++++------- 11 files changed, 262 insertions(+), 108 deletions(-) diff --git a/services/web/app/src/Features/Collaborators/CollaboratorsController.js b/services/web/app/src/Features/Collaborators/CollaboratorsController.js index c3af1ba18b..a86f4fface 100644 --- a/services/web/app/src/Features/Collaborators/CollaboratorsController.js +++ b/services/web/app/src/Features/Collaborators/CollaboratorsController.js @@ -167,5 +167,17 @@ async function getShareTokens(req, res) { ) } + if (tokens.readOnly) { + tokens.readOnlyHashPrefix = TokenAccessHandler.createTokenHashPrefix( + tokens.readOnly + ) + } + + if (tokens.readAndWrite) { + tokens.readAndWriteHashPrefix = TokenAccessHandler.createTokenHashPrefix( + tokens.readAndWrite + ) + } + res.json(tokens) } diff --git a/services/web/app/src/Features/TokenAccess/TokenAccessController.js b/services/web/app/src/Features/TokenAccess/TokenAccessController.js index 87a4723448..4e0fe30b89 100644 --- a/services/web/app/src/Features/TokenAccess/TokenAccessController.js +++ b/services/web/app/src/Features/TokenAccess/TokenAccessController.js @@ -96,6 +96,7 @@ async function tokenAccessPage(req, res, next) { return res.redirect(302, docPublishedInfo.published_path) } } + res.render('project/token/access', { postUrl: makePostUrl(token), }) @@ -225,12 +226,15 @@ async function checkAndGetProjectOrResponseAction( async function grantTokenAccessReadAndWrite(req, res, next) { const { token } = req.params - const { confirmedByUser } = req.body + const { confirmedByUser, tokenHashPrefix } = req.body const userId = SessionManager.getLoggedInUserId(req.session) if (!TokenAccessHandler.isReadAndWriteToken(token)) { return res.sendStatus(400) } const tokenType = TokenAccessHandler.TOKEN_TYPES.READ_AND_WRITE + + TokenAccessHandler.checkTokenHashPrefix(token, tokenHashPrefix, tokenType) + try { const [project, action] = await checkAndGetProjectOrResponseAction( tokenType, @@ -268,6 +272,7 @@ async function grantTokenAccessReadAndWrite(req, res, next) { userId, project._id ) + return res.json({ redirect: `/project/${project._id}`, tokenAccessGranted: tokenType, @@ -285,12 +290,16 @@ async function grantTokenAccessReadAndWrite(req, res, next) { async function grantTokenAccessReadOnly(req, res, next) { const { token } = req.params - const { confirmedByUser } = req.body + const { confirmedByUser, tokenHashPrefix } = req.body const userId = SessionManager.getLoggedInUserId(req.session) if (!TokenAccessHandler.isReadOnlyToken(token)) { return res.sendStatus(400) } + const tokenType = TokenAccessHandler.TOKEN_TYPES.READ_ONLY + + TokenAccessHandler.checkTokenHashPrefix(token, tokenHashPrefix, tokenType) + const docPublishedInfo = await TokenAccessHandler.promises.getV1DocPublishedInfo(token) if (docPublishedInfo.allow === false) { @@ -333,6 +342,7 @@ async function grantTokenAccessReadOnly(req, res, next) { userId, project._id ) + return res.json({ redirect: `/project/${project._id}`, tokenAccessGranted: tokenType, diff --git a/services/web/app/src/Features/TokenAccess/TokenAccessHandler.js b/services/web/app/src/Features/TokenAccess/TokenAccessHandler.js index 6af5ca2045..ee8235158b 100644 --- a/services/web/app/src/Features/TokenAccess/TokenAccessHandler.js +++ b/services/web/app/src/Features/TokenAccess/TokenAccessHandler.js @@ -2,6 +2,7 @@ const { Project } = require('../../models/Project') const PublicAccessLevels = require('../Authorization/PublicAccessLevels') const PrivilegeLevels = require('../Authorization/PrivilegeLevels') const { ObjectId } = require('mongodb') +const Metrics = require('@overleaf/metrics') const Settings = require('@overleaf/settings') const logger = require('@overleaf/logger') const V1Api = require('../V1/V1Api') @@ -279,6 +280,32 @@ const TokenAccessHandler = { callback(null, body) }) }, + + createTokenHashPrefix(token) { + const hash = crypto.createHash('sha256') + hash.update(token) + return hash.digest('hex').slice(0, 6) + }, + + checkTokenHashPrefix(token, tokenHashPrefix, type) { + let hashPrefixStatus + + if (!tokenHashPrefix) { + hashPrefixStatus = 'missing' + } else { + const hashPrefix = TokenAccessHandler.createTokenHashPrefix(token) + if (hashPrefix === tokenHashPrefix.replace('#', '')) { + hashPrefixStatus = 'match' + } else { + hashPrefixStatus = 'mismatch' + } + } + + Metrics.inc('link-sharing.hash-check', { + path: type, + status: hashPrefixStatus, + }) + }, } TokenAccessHandler.promises = promisifyAll(TokenAccessHandler, { diff --git a/services/web/frontend/js/features/share-project-modal/components/link-sharing.jsx b/services/web/frontend/js/features/share-project-modal/components/link-sharing.jsx index 13c0ec5074..a37a631b27 100644 --- a/services/web/frontend/js/features/share-project-modal/components/link-sharing.jsx +++ b/services/web/frontend/js/features/share-project-modal/components/link-sharing.jsx @@ -15,7 +15,7 @@ import { getJSON } from '../../../infrastructure/fetch-json' import useAbortController from '../../../shared/hooks/use-abort-controller' import { debugConsole } from '@/utils/debugging' -export default function LinkSharing({ canAddCollaborators }) { +export default function LinkSharing() { const [inflight, setInflight] = useState(false) const { monitorRequest } = useShareProjectContext() @@ -61,7 +61,6 @@ export default function LinkSharing({ canAddCollaborators }) { ) @@ -81,10 +80,6 @@ export default function LinkSharing({ canAddCollaborators }) { } } -LinkSharing.propTypes = { - canAddCollaborators: PropTypes.bool, -} - function PrivateSharing({ setAccessLevel, inflight, projectId }) { const { t } = useTranslation() return ( @@ -117,7 +112,7 @@ PrivateSharing.propTypes = { projectId: PropTypes.string, } -function TokenBasedSharing({ setAccessLevel, inflight, canAddCollaborators }) { +function TokenBasedSharing({ setAccessLevel, inflight }) { const { t } = useTranslation() const { _id: projectId } = useProjectContext() @@ -152,6 +147,7 @@ function TokenBasedSharing({ setAccessLevel, inflight, canAddCollaborators }) { {t('anyone_with_link_can_edit')} @@ -160,6 +156,7 @@ function TokenBasedSharing({ setAccessLevel, inflight, canAddCollaborators }) { {t('anyone_with_link_can_view')} @@ -172,7 +169,6 @@ function TokenBasedSharing({ setAccessLevel, inflight, canAddCollaborators }) { TokenBasedSharing.propTypes = { setAccessLevel: PropTypes.func.isRequired, inflight: PropTypes.bool, - canAddCollaborators: PropTypes.bool, } function LegacySharing({ accessLevel, setAccessLevel, inflight }) { @@ -229,6 +225,7 @@ export function ReadOnlyTokenLink() { {t('anyone_with_link_can_view')} @@ -238,7 +235,7 @@ export function ReadOnlyTokenLink() { ) } -function AccessToken({ token, path, tooltipId }) { +function AccessToken({ token, tokenHashPrefix, path, tooltipId }) { const { t } = useTranslation() const { isAdmin } = useUserContext() @@ -254,7 +251,9 @@ function AccessToken({ token, path, tooltipId }) { if (isAdmin) { origin = window.ExposedSettings.siteUrl } - const link = `${origin}${path}${token}` + const link = `${origin}${path}${token}${ + tokenHashPrefix ? `#${tokenHashPrefix}` : '' + }` return (
@@ -266,6 +265,7 @@ function AccessToken({ token, path, tooltipId }) { AccessToken.propTypes = { token: PropTypes.string, + tokenHashPrefix: PropTypes.string, tooltipId: PropTypes.string.isRequired, path: PropTypes.string.isRequired, } diff --git a/services/web/frontend/js/features/share-project-modal/components/share-modal-body.jsx b/services/web/frontend/js/features/share-project-modal/components/share-modal-body.jsx index 85a36650d3..02938c37f4 100644 --- a/services/web/frontend/js/features/share-project-modal/components/share-modal-body.jsx +++ b/services/web/frontend/js/features/share-project-modal/components/share-modal-body.jsx @@ -69,7 +69,7 @@ export default function ShareModalBody() { {isProjectOwner && ( <>
- + )} @@ -108,7 +108,7 @@ export default function ShareModalBody() { {isProjectOwner && ( <>
- + )} @@ -122,9 +122,7 @@ export default function ShareModalBody() { default: return ( <> - {isProjectOwner && ( - - )} + {isProjectOwner && } diff --git a/services/web/frontend/js/main/token-access.js b/services/web/frontend/js/main/token-access.js index 64ffb7b7ff..68cb1075ab 100644 --- a/services/web/frontend/js/main/token-access.js +++ b/services/web/frontend/js/main/token-access.js @@ -43,13 +43,13 @@ App.controller('TokenAccessPageController', [ const parsedData = JSON.parse(textData) const { postUrl, csrfToken } = parsedData $scope.accessInFlight = true - $http({ method: 'POST', url: postUrl, data: { _csrf: csrfToken, confirmedByUser, + tokenHashPrefix: window.location.hash, }, }).then( function successCallback(response) { diff --git a/services/web/test/acceptance/src/TokenAccessTests.js b/services/web/test/acceptance/src/TokenAccessTests.js index 35ac5ec155..d8e548113a 100644 --- a/services/web/test/acceptance/src/TokenAccessTests.js +++ b/services/web/test/acceptance/src/TokenAccessTests.js @@ -345,11 +345,14 @@ describe('TokenAccess', function () { (error, response, body) => { expect(error).to.equal(null) expect(response.statusCode).to.equal(200) - expect(body).to.deep.equal({ + expect(body).to.include({ readOnly: this.tokens.readOnly, readAndWrite: this.tokens.readAndWrite, readAndWritePrefix: this.tokens.readAndWritePrefix, }) + expect(body.readOnlyHashPrefix).to.exist + expect(body.readAndWriteHashPrefix).to.exist + expect(Object.keys(body).length).to.equal(5) done() } ) @@ -494,7 +497,8 @@ describe('TokenAccess', function () { (error, response, body) => { expect(error).to.equal(null) expect(response.statusCode).to.equal(200) - expect(body).to.deep.equal({ readOnly: this.tokens.readOnly }) + expect(body).to.include({ readOnly: this.tokens.readOnly }) + expect(body.readOnlyHashPrefix).to.exist cb() } ) @@ -709,7 +713,9 @@ describe('TokenAccess', function () { (error, response, body) => { expect(error).to.equal(null) expect(response.statusCode).to.equal(200) - expect(body).to.deep.equal({ readOnly: this.tokens.readOnly }) + expect(body).to.include({ readOnly: this.tokens.readOnly }) + expect(body.readOnlyHashPrefix).to.exist + expect(Object.keys(body).length).to.equal(2) cb() } ) diff --git a/services/web/test/frontend/features/share-project-modal/components/share-project-modal.test.jsx b/services/web/test/frontend/features/share-project-modal/components/share-project-modal.test.jsx index b5cb253f70..5996e6ac47 100644 --- a/services/web/test/frontend/features/share-project-modal/components/share-project-modal.test.jsx +++ b/services/web/test/frontend/features/share-project-modal/components/share-project-modal.test.jsx @@ -146,6 +146,14 @@ describe('', function () { }) it('handles access level "tokenBased"', async function () { + const tokens = { + readAndWrite: '6862414195fwtbrtrdtskb', + readAndWritePrefix: '6862414195', + readOnly: 'wrnjfzkysqkr', + readAndWriteHashPrefix: 'taEVki', + readOnlyHashPrefix: 'j2xYbL', + } + fetchMock.get(`/project/${project._id}/tokens`, tokens) renderWithEditorContext(, { scope: { project: { ...project, publicAccesLevel: 'tokenBased' } }, }) @@ -157,6 +165,13 @@ describe('', function () { .not.to.be.null expect(screen.queryByText('Anyone with this link can edit this project')) .not.to.be.null + + screen.getByText( + `https://www.test-overleaf.com/${tokens.readAndWrite}#${tokens.readAndWriteHashPrefix}` + ) + screen.getByText( + `https://www.test-overleaf.com/read/${tokens.readOnly}#${tokens.readOnlyHashPrefix}` + ) }) it('handles legacy access level "readAndWrite"', async function () { diff --git a/services/web/test/unit/src/Collaborators/CollaboratorsControllerTests.js b/services/web/test/unit/src/Collaborators/CollaboratorsControllerTests.js index 9eb99105b3..0f50b76c84 100644 --- a/services/web/test/unit/src/Collaborators/CollaboratorsControllerTests.js +++ b/services/web/test/unit/src/Collaborators/CollaboratorsControllerTests.js @@ -23,6 +23,7 @@ describe('CollaboratorsController', function () { removeUserFromProject: sinon.stub().resolves(), setCollaboratorPrivilegeLevel: sinon.stub().resolves(), }, + createTokenHashPrefix: sinon.stub().returns('abc123'), } this.CollaboratorsGetter = { promises: { diff --git a/services/web/test/unit/src/TokenAccess/TokenAccessControllerTests.js b/services/web/test/unit/src/TokenAccess/TokenAccessControllerTests.js index 015550e956..0beb92d794 100644 --- a/services/web/test/unit/src/TokenAccess/TokenAccessControllerTests.js +++ b/services/web/test/unit/src/TokenAccess/TokenAccessControllerTests.js @@ -29,6 +29,7 @@ describe('TokenAccessController', function () { isReadAndWriteToken: sinon.stub().returns(true), isReadOnlyToken: sinon.stub().returns(true), tokenAccessEnabledForProject: sinon.stub().returns(true), + checkTokenHashPrefix: sinon.stub(), promises: { addReadAndWriteUserToProject: sinon.stub().resolves(), addReadOnlyUserToProject: sinon.stub().resolves(), @@ -78,7 +79,7 @@ describe('TokenAccessController', function () { describe('normal case', function () { beforeEach(function (done) { this.req.params = { token: this.token } - this.req.body = { confirmedByUser: true } + this.req.body = { confirmedByUser: true, tokenHashPrefix: 'prefix' } this.res.callback = done this.TokenAccessController.grantTokenAccessReadAndWrite( this.req, @@ -104,6 +105,12 @@ describe('TokenAccessController', function () { { privileges: 'readAndWrite' } ) }) + + it('checks token hash', function () { + expect( + this.TokenAccessHandler.checkTokenHashPrefix + ).to.have.been.calledWith(this.token, 'prefix', 'readAndWrite') + }) }) describe('when the access was already granted', function () { @@ -124,13 +131,38 @@ describe('TokenAccessController', function () { .called }) }) + + describe('hash prefix missing in request', function () { + beforeEach(function (done) { + this.req.params = { token: this.token } + this.req.body = { confirmedByUser: true } + this.res.callback = done + this.TokenAccessController.grantTokenAccessReadAndWrite( + this.req, + this.res, + done + ) + }) + + it('grants read and write access', function () { + expect( + this.TokenAccessHandler.promises.addReadAndWriteUserToProject + ).to.have.been.calledWith(this.user._id, this.project._id) + }) + + it('sends missing hash to metrics', function () { + expect( + this.TokenAccessHandler.checkTokenHashPrefix + ).to.have.been.calledWith(this.token, undefined, 'readAndWrite') + }) + }) }) describe('grantTokenAccessReadOnly', function () { describe('normal case', function () { beforeEach(function (done) { this.req.params = { token: this.token } - this.req.body = { confirmedByUser: true } + this.req.body = { confirmedByUser: true, tokenHashPrefix: 'prefix' } this.res.callback = done this.TokenAccessController.grantTokenAccessReadOnly( this.req, @@ -156,6 +188,12 @@ describe('TokenAccessController', function () { { privileges: 'readOnly' } ) }) + + it('sends checks if hash prefix matches', function () { + expect( + this.TokenAccessHandler.checkTokenHashPrefix + ).to.have.been.calledWith(this.token, 'prefix', 'readOnly') + }) }) describe('when the access was already granted', function () { diff --git a/services/web/test/unit/src/TokenAccess/TokenAccessHandlerTests.js b/services/web/test/unit/src/TokenAccess/TokenAccessHandlerTests.js index d85e2d867f..feeb79b16a 100644 --- a/services/web/test/unit/src/TokenAccess/TokenAccessHandlerTests.js +++ b/services/web/test/unit/src/TokenAccess/TokenAccessHandlerTests.js @@ -1,16 +1,3 @@ -/* eslint-disable - n/handle-callback-err, - max-len, - no-return-assign, - no-unused-vars, -*/ -// TODO: This file was created by bulk-decaffeinate. -// Fix any style issues and re-enable lint. -/* - * decaffeinate suggestions: - * DS102: Remove unnecessary code created because of implicit returns - * Full docs: https://github.com/decaffeinate/decaffeinate/blob/master/docs/suggestions.md - */ const SandboxedModule = require('sandboxed-module') const path = require('path') const sinon = require('sinon') @@ -32,10 +19,11 @@ describe('TokenAccessHandler', function () { } this.userId = ObjectId() this.req = {} - return (this.TokenAccessHandler = SandboxedModule.require(modulePath, { + this.TokenAccessHandler = SandboxedModule.require(modulePath, { requires: { mongodb: { ObjectId }, '../../models/Project': { Project: (this.Project = {}) }, + '@overleaf/metrics': (this.Metrics = { inc: sinon.stub() }), '@overleaf/settings': (this.settings = {}), '../V1/V1Api': (this.V1Api = { request: sinon.stub(), @@ -45,7 +33,7 @@ describe('TokenAccessHandler', function () { recordEventForUser: sinon.stub(), }), }, - })) + }) }) describe('getTokenType', function () { @@ -119,14 +107,15 @@ describe('TokenAccessHandler', function () { describe('addReadOnlyUserToProject', function () { beforeEach(function () { - return (this.Project.updateOne = sinon.stub().callsArgWith(2, null)) + this.Project.updateOne = sinon.stub().callsArgWith(2, null) }) it('should call Project.updateOne', function (done) { - return this.TokenAccessHandler.addReadOnlyUserToProject( + this.TokenAccessHandler.addReadOnlyUserToProject( this.userId, this.projectId, err => { + expect(err).to.not.exist expect(this.Project.updateOne.callCount).to.equal(1) expect( this.Project.updateOne.calledWith({ @@ -142,36 +131,36 @@ describe('TokenAccessHandler', function () { 'project-joined', { mode: 'read-only' } ) - return done() + done() } ) }) it('should not produce an error', function (done) { - return this.TokenAccessHandler.addReadOnlyUserToProject( + this.TokenAccessHandler.addReadOnlyUserToProject( this.userId, this.projectId, err => { expect(err).to.not.exist - return done() + done() } ) }) describe('when Project.updateOne produces an error', function () { beforeEach(function () { - return (this.Project.updateOne = sinon + this.Project.updateOne = sinon .stub() - .callsArgWith(2, new Error('woops'))) + .callsArgWith(2, new Error('woops')) }) it('should produce an error', function (done) { - return this.TokenAccessHandler.addReadOnlyUserToProject( + this.TokenAccessHandler.addReadOnlyUserToProject( this.userId, this.projectId, err => { expect(err).to.exist - return done() + done() } ) }) @@ -180,14 +169,15 @@ describe('TokenAccessHandler', function () { describe('addReadAndWriteUserToProject', function () { beforeEach(function () { - return (this.Project.updateOne = sinon.stub().callsArgWith(2, null)) + this.Project.updateOne = sinon.stub().callsArgWith(2, null) }) it('should call Project.updateOne', function (done) { - return this.TokenAccessHandler.addReadAndWriteUserToProject( + this.TokenAccessHandler.addReadAndWriteUserToProject( this.userId, this.projectId, err => { + expect(err).to.not.exist expect(this.Project.updateOne.callCount).to.equal(1) expect( this.Project.updateOne.calledWith({ @@ -203,36 +193,36 @@ describe('TokenAccessHandler', function () { 'project-joined', { mode: 'read-write' } ) - return done() + done() } ) }) it('should not produce an error', function (done) { - return this.TokenAccessHandler.addReadAndWriteUserToProject( + this.TokenAccessHandler.addReadAndWriteUserToProject( this.userId, this.projectId, err => { expect(err).to.not.exist - return done() + done() } ) }) describe('when Project.updateOne produces an error', function () { beforeEach(function () { - return (this.Project.updateOne = sinon + this.Project.updateOne = sinon .stub() - .callsArgWith(2, new Error('woops'))) + .callsArgWith(2, new Error('woops')) }) it('should produce an error', function (done) { - return this.TokenAccessHandler.addReadAndWriteUserToProject( + this.TokenAccessHandler.addReadAndWriteUserToProject( this.userId, this.projectId, err => { expect(err).to.exist - return done() + done() } ) }) @@ -241,7 +231,7 @@ describe('TokenAccessHandler', function () { describe('grantSessionTokenAccess', function () { beforeEach(function () { - return (this.req = { session: {}, headers: {} }) + this.req = { session: {}, headers: {} } }) it('should add the token to the session', function (done) { @@ -253,7 +243,7 @@ describe('TokenAccessHandler', function () { expect( this.req.session.anonTokenAccess[this.projectId.toString()] ).to.equal(this.token) - return done() + done() }) }) @@ -267,27 +257,28 @@ describe('TokenAccessHandler', function () { }) it('should try to find projects with both kinds of token', function (done) { - return this.TokenAccessHandler.validateTokenForAnonymousAccess( + this.TokenAccessHandler.validateTokenForAnonymousAccess( this.projectId, this.token, (err, allowed) => { + expect(err).to.not.exist expect( this.TokenAccessHandler.getProjectByToken.callCount ).to.equal(1) - return done() + done() } ) }) it('should allow read-only access', function (done) { - return this.TokenAccessHandler.validateTokenForAnonymousAccess( + this.TokenAccessHandler.validateTokenForAnonymousAccess( this.projectId, this.token, (err, rw, ro) => { expect(err).to.not.exist expect(rw).to.equal(false) expect(ro).to.equal(true) - return done() + done() } ) }) @@ -309,27 +300,28 @@ describe('TokenAccessHandler', function () { }) it('should try to find projects with both kinds of token', function (done) { - return this.TokenAccessHandler.validateTokenForAnonymousAccess( + this.TokenAccessHandler.validateTokenForAnonymousAccess( this.projectId, this.token, (err, rw, ro) => { + expect(err).to.not.exist expect( this.TokenAccessHandler.getProjectByToken.callCount ).to.equal(1) - return done() + done() } ) }) it('should not allow read-and-write access', function (done) { - return this.TokenAccessHandler.validateTokenForAnonymousAccess( + this.TokenAccessHandler.validateTokenForAnonymousAccess( this.projectId, this.token, (err, rw, ro) => { expect(err).to.not.exist expect(rw).to.equal(false) expect(ro).to.equal(false) - return done() + done() } ) }) @@ -341,27 +333,28 @@ describe('TokenAccessHandler', function () { }) it('should try to find projects with both kinds of token', function (done) { - return this.TokenAccessHandler.validateTokenForAnonymousAccess( + this.TokenAccessHandler.validateTokenForAnonymousAccess( this.projectId, this.token, (err, rw, ro) => { + expect(err).to.not.exist expect( this.TokenAccessHandler.getProjectByToken.callCount ).to.equal(1) - return done() + done() } ) }) it('should allow read-and-write access', function (done) { - return this.TokenAccessHandler.validateTokenForAnonymousAccess( + this.TokenAccessHandler.validateTokenForAnonymousAccess( this.projectId, this.token, (err, rw, ro) => { expect(err).to.not.exist expect(rw).to.equal(true) expect(ro).to.equal(false) - return done() + done() } ) }) @@ -376,27 +369,28 @@ describe('TokenAccessHandler', function () { }) it('should try to find projects with both kinds of token', function (done) { - return this.TokenAccessHandler.validateTokenForAnonymousAccess( + this.TokenAccessHandler.validateTokenForAnonymousAccess( this.projectId, this.token, (err, allowed) => { + expect(err).to.not.exist expect( this.TokenAccessHandler.getProjectByToken.callCount ).to.equal(1) - return done() + done() } ) }) it('should not allow any access', function (done) { - return this.TokenAccessHandler.validateTokenForAnonymousAccess( + this.TokenAccessHandler.validateTokenForAnonymousAccess( this.projectId, this.token, (err, rw, ro) => { expect(err).to.not.exist expect(rw).to.equal(false) expect(ro).to.equal(false) - return done() + done() } ) }) @@ -410,20 +404,22 @@ describe('TokenAccessHandler', function () { }) it('should try to find projects with both kinds of token', function (done) { - return this.TokenAccessHandler.validateTokenForAnonymousAccess( + this.TokenAccessHandler.validateTokenForAnonymousAccess( this.projectId, this.token, (err, allowed) => { + expect(err).to.exist + expect(allowed).to.not.exist expect( this.TokenAccessHandler.getProjectByToken.callCount ).to.equal(1) - return done() + done() } ) }) it('should produce an error and not allow access', function (done) { - return this.TokenAccessHandler.validateTokenForAnonymousAccess( + this.TokenAccessHandler.validateTokenForAnonymousAccess( this.projectId, this.token, (err, rw, ro) => { @@ -431,7 +427,7 @@ describe('TokenAccessHandler', function () { expect(err).to.be.instanceof(Error) expect(rw).to.equal(undefined) expect(ro).to.equal(undefined) - return done() + done() } ) }) @@ -439,7 +435,7 @@ describe('TokenAccessHandler', function () { describe('when project is not set to token-based access', function () { beforeEach(function () { - return (this.project.publicAccesLevel = 'private') + this.project.publicAccesLevel = 'private' }) describe('for read-and-write project', function () { @@ -453,14 +449,14 @@ describe('TokenAccessHandler', function () { }) it('should not allow any access', function (done) { - return this.TokenAccessHandler.validateTokenForAnonymousAccess( + this.TokenAccessHandler.validateTokenForAnonymousAccess( this.projectId, this.token, (err, rw, ro) => { expect(err).to.not.exist expect(rw).to.equal(false) expect(ro).to.equal(false) - return done() + done() } ) }) @@ -477,14 +473,14 @@ describe('TokenAccessHandler', function () { }) it('should not allow any access', function (done) { - return this.TokenAccessHandler.validateTokenForAnonymousAccess( + this.TokenAccessHandler.validateTokenForAnonymousAccess( this.projectId, this.token, (err, rw, ro) => { expect(err).to.not.exist expect(rw).to.equal(false) expect(ro).to.equal(false) - return done() + done() } ) }) @@ -498,14 +494,14 @@ describe('TokenAccessHandler', function () { }) it('should not allow any access', function (done) { - return this.TokenAccessHandler.validateTokenForAnonymousAccess( + this.TokenAccessHandler.validateTokenForAnonymousAccess( this.projectId, null, (err, rw, ro) => { expect(err).to.not.exist expect(rw).to.equal(false) expect(ro).to.equal(false) - return done() + done() } ) }) @@ -515,21 +511,18 @@ describe('TokenAccessHandler', function () { describe('getDocPublishedInfo', function () { beforeEach(function () { - return (this.callback = sinon.stub()) + this.callback = sinon.stub() }) describe('when v1 api not set', function () { beforeEach(function () { this.settings.apis = { v1: undefined } - return this.TokenAccessHandler.getV1DocPublishedInfo( - this.token, - this.callback - ) + this.TokenAccessHandler.getV1DocPublishedInfo(this.token, this.callback) }) it('should not check access and return default info', function () { expect(this.V1Api.request.called).to.equal(false) - return expect( + expect( this.callback.calledWith(null, { allow: true, }) @@ -539,7 +532,7 @@ describe('TokenAccessHandler', function () { describe('when v1 api is set', function () { beforeEach(function () { - return (this.settings.apis = { v1: { url: 'v1Url' } }) + this.settings.apis = { v1: { url: 'v1Url' } } }) describe('on V1Api.request success', function () { @@ -547,7 +540,7 @@ describe('TokenAccessHandler', function () { this.V1Api.request = sinon .stub() .callsArgWith(1, null, null, 'mock-data') - return this.TokenAccessHandler.getV1DocPublishedInfo( + this.TokenAccessHandler.getV1DocPublishedInfo( this.token, this.callback ) @@ -559,23 +552,21 @@ describe('TokenAccessHandler', function () { url: `/api/v1/sharelatex/docs/${this.token}/is_published`, }) ).to.equal(true) - return expect(this.callback.calledWith(null, 'mock-data')).to.equal( - true - ) + expect(this.callback.calledWith(null, 'mock-data')).to.equal(true) }) }) describe('on V1Api.request error', function () { beforeEach(function () { this.V1Api.request = sinon.stub().callsArgWith(1, 'error') - return this.TokenAccessHandler.getV1DocPublishedInfo( + this.TokenAccessHandler.getV1DocPublishedInfo( this.token, this.callback ) }) it('should callback with error', function () { - return expect(this.callback.calledWith('error')).to.equal(true) + expect(this.callback.calledWith('error')).to.equal(true) }) }) }) @@ -583,12 +574,12 @@ describe('TokenAccessHandler', function () { describe('getV1DocInfo', function () { beforeEach(function () { - return (this.callback = sinon.stub()) + this.callback = sinon.stub() }) describe('when v1 api not set', function () { beforeEach(function () { - return this.TokenAccessHandler.getV1DocInfo( + this.TokenAccessHandler.getV1DocInfo( this.token, this.v2UserId, this.callback @@ -597,7 +588,7 @@ describe('TokenAccessHandler', function () { it('should not check access and return default info', function () { expect(this.V1Api.request.called).to.equal(false) - return expect( + expect( this.callback.calledWith(null, { exists: true, exported: false, @@ -608,7 +599,7 @@ describe('TokenAccessHandler', function () { describe('when v1 api is set', function () { beforeEach(function () { - return (this.settings.apis = { v1: 'v1' }) + this.settings.apis = { v1: 'v1' } }) describe('on V1Api.request success', function () { @@ -616,7 +607,7 @@ describe('TokenAccessHandler', function () { this.V1Api.request = sinon .stub() .callsArgWith(1, null, null, 'mock-data') - return this.TokenAccessHandler.getV1DocInfo( + this.TokenAccessHandler.getV1DocInfo( this.token, this.v2UserId, this.callback @@ -629,16 +620,14 @@ describe('TokenAccessHandler', function () { url: `/api/v1/sharelatex/docs/${this.token}/info`, }) ).to.equal(true) - return expect(this.callback.calledWith(null, 'mock-data')).to.equal( - true - ) + expect(this.callback.calledWith(null, 'mock-data')).to.equal(true) }) }) describe('on V1Api.request error', function () { beforeEach(function () { this.V1Api.request = sinon.stub().callsArgWith(1, 'error') - return this.TokenAccessHandler.getV1DocInfo( + this.TokenAccessHandler.getV1DocInfo( this.token, this.v2UserId, this.callback @@ -646,9 +635,67 @@ describe('TokenAccessHandler', function () { }) it('should callback with error', function () { - return expect(this.callback.calledWith('error')).to.equal(true) + expect(this.callback.calledWith('error')).to.equal(true) }) }) }) }) + + describe('createTokenHashPrefix', function () { + it('creates a prefix of the hash', function () { + const prefix = + this.TokenAccessHandler.createTokenHashPrefix('zxpxjrwdtsgd') + expect(prefix.length).to.equal(6) + }) + }) + + describe('checkTokenHashPrefix', function () { + it('sends "match" to metrics when prefix matches the prefix of the hash of the token', function () { + const token = 'zxpxjrwdtsgd' + const prefix = this.TokenAccessHandler.createTokenHashPrefix(token) + + this.TokenAccessHandler.checkTokenHashPrefix(token, prefix, 'readOnly') + + expect(this.Metrics.inc).to.have.been.calledWith( + 'link-sharing.hash-check', + { + path: 'readOnly', + status: 'match', + } + ) + }) + it('sends "mismatch" to metrics when prefix does not match the prefix of the hash of the token', function () { + const token = 'zxpxjrwdtsgd' + const prefix = this.TokenAccessHandler.createTokenHashPrefix(token) + + this.TokenAccessHandler.checkTokenHashPrefix( + 'anothertoken', + prefix, + 'readOnly' + ) + + expect(this.Metrics.inc).to.have.been.calledWith( + 'link-sharing.hash-check', + { + path: 'readOnly', + status: 'mismatch', + } + ) + }) + it('sends "missing" to metrics when prefix is undefined', function () { + this.TokenAccessHandler.checkTokenHashPrefix( + 'anothertoken', + undefined, + 'readOnly' + ) + + expect(this.Metrics.inc).to.have.been.calledWith( + 'link-sharing.hash-check', + { + path: 'readOnly', + status: 'missing', + } + ) + }) + }) })