From 974069bf1c2c31dc591bf29258235986122437bb Mon Sep 17 00:00:00 2001 From: Jakob Ackermann Date: Fri, 9 Feb 2024 11:26:16 +0000 Subject: [PATCH] Merge pull request #16979 from overleaf/jpa-join-project-remove-sl-1 [misc] joinProject: pass userId and anonymous access token in body 1/2 GitOrigin-RevId: 5d7832246c7262c004c2cd465d62488384b35ee3 --- package-lock.json | 2 + services/real-time/app/js/WebApiManager.js | 5 +- services/real-time/package.json | 1 + services/real-time/test/setup.js | 4 + .../test/unit/js/WebApiManagerTests.js | 64 +++++++++++++- .../Features/Editor/EditorHttpController.js | 5 +- .../app/src/Features/Editor/EditorRouter.js | 2 +- .../test/acceptance/src/TokenAccessTests.js | 83 ++++++++++++++++++- .../src/Editor/EditorHttpControllerTests.js | 5 ++ 9 files changed, 165 insertions(+), 6 deletions(-) diff --git a/package-lock.json b/package-lock.json index 6953ce76c3..70ba86f2b4 100644 --- a/package-lock.json +++ b/package-lock.json @@ -45571,6 +45571,7 @@ "mocha": "^10.2.0", "sandboxed-module": "~0.3.0", "sinon": "^9.2.4", + "sinon-chai": "^3.7.0", "timekeeper": "0.0.4", "typescript": "^5.0.4", "uid-safe": "^2.1.5" @@ -54587,6 +54588,7 @@ "request": "^2.88.2", "sandboxed-module": "~0.3.0", "sinon": "^9.2.4", + "sinon-chai": "^3.7.0", "socket.io": "github:overleaf/socket.io#0.9.19-overleaf-10", "socket.io-client": "github:overleaf/socket.io-client#0.9.17-overleaf-5", "timekeeper": "0.0.4", diff --git a/services/real-time/app/js/WebApiManager.js b/services/real-time/app/js/WebApiManager.js index e33f05d418..943d8021b1 100644 --- a/services/real-time/app/js/WebApiManager.js +++ b/services/real-time/app/js/WebApiManager.js @@ -27,7 +27,10 @@ module.exports = { pass: settings.apis.web.pass, sendImmediately: true, }, - json: true, + json: { + userId, + anonymousAccessToken: user.anonymousAccessToken, + }, jar: false, headers, }, diff --git a/services/real-time/package.json b/services/real-time/package.json index c451ff8dcd..80935b6566 100644 --- a/services/real-time/package.json +++ b/services/real-time/package.json @@ -44,6 +44,7 @@ "mocha": "^10.2.0", "sandboxed-module": "~0.3.0", "sinon": "^9.2.4", + "sinon-chai": "^3.7.0", "timekeeper": "0.0.4", "typescript": "^5.0.4", "uid-safe": "^2.1.5" diff --git a/services/real-time/test/setup.js b/services/real-time/test/setup.js index 6d09ee0c24..1bee8e2fde 100644 --- a/services/real-time/test/setup.js +++ b/services/real-time/test/setup.js @@ -1,9 +1,13 @@ const chai = require('chai') const SandboxedModule = require('sandboxed-module') const sinon = require('sinon') +const chaiAsPromised = require('chai-as-promised') +const sinonChai = require('sinon-chai') // Chai configuration chai.should() +chai.use(chaiAsPromised) +chai.use(sinonChai) // Global stubs const sandbox = sinon.createSandbox() diff --git a/services/real-time/test/unit/js/WebApiManagerTests.js b/services/real-time/test/unit/js/WebApiManagerTests.js index 451c1ccedf..3cb8e22d7f 100644 --- a/services/real-time/test/unit/js/WebApiManagerTests.js +++ b/services/real-time/test/unit/js/WebApiManagerTests.js @@ -68,7 +68,10 @@ describe('WebApiManager', function () { pass: this.settings.apis.web.pass, sendImmediately: true, }, - json: true, + json: { + userId: this.user_id, + anonymousAccessToken: undefined, + }, jar: false, headers: {}, }) @@ -91,6 +94,65 @@ describe('WebApiManager', function () { }) }) + describe('with anon user', function () { + beforeEach(function () { + this.user_id = 'anonymous-user' + this.token = 'a-ro-token' + this.user = { + _id: this.user_id, + anonymousAccessToken: this.token, + } + this.response = { + project: { name: 'Test project' }, + privilegeLevel: 'readOnly', + isRestrictedUser: true, + isTokenMember: false, + isInvitedMember: false, + } + this.request.post = sinon + .stub() + .yields(null, { statusCode: 200 }, this.response) + this.WebApiManager.joinProject( + this.project_id, + this.user, + this.callback + ) + }) + + it('should send a request to web to join the project', function () { + this.request.post.should.have.been.calledWith({ + url: `${this.settings.apis.web.url}/project/${this.project_id}/join`, + qs: { + user_id: this.user_id, + }, + auth: { + user: this.settings.apis.web.user, + pass: this.settings.apis.web.pass, + sendImmediately: true, + }, + json: { + userId: this.user_id, + anonymousAccessToken: this.token, + }, + jar: false, + headers: { 'x-sl-anonymous-access-token': this.token }, + }) + }) + + it('should return the project, privilegeLevel, and restricted flag', function () { + this.callback.should.have.been.calledWith( + null, + this.response.project, + this.response.privilegeLevel, + { + isRestrictedUser: this.response.isRestrictedUser, + isTokenMember: this.response.isTokenMember, + isInvitedMember: this.response.isInvitedMember, + } + ) + }) + }) + describe('when web replies with a 403', function () { beforeEach(function () { this.request.post = sinon diff --git a/services/web/app/src/Features/Editor/EditorHttpController.js b/services/web/app/src/Features/Editor/EditorHttpController.js index 2fb9e354af..83080ba6a4 100644 --- a/services/web/app/src/Features/Editor/EditorHttpController.js +++ b/services/web/app/src/Features/Editor/EditorHttpController.js @@ -57,7 +57,7 @@ const unsupportedSpellcheckLanguages = [ async function joinProject(req, res, next) { const projectId = req.params.Project_id - let userId = req.query.user_id // keep schema in sync with router + let userId = req.body.userId || req.query.user_id // keep schema in sync with router if (userId === 'anonymous-user') { userId = null } @@ -177,7 +177,8 @@ async function _buildJoinProjectView(req, projectId, userId) { await CollaboratorsGetter.promises.getInvitedMembersWithPrivilegeLevels( projectId ) - const token = req.headers['x-sl-anonymous-access-token'] + const token = + req.body.anonymousAccessToken || req.headers['x-sl-anonymous-access-token'] const privilegeLevel = await AuthorizationManager.promises.getPrivilegeLevelForProject( userId, diff --git a/services/web/app/src/Features/Editor/EditorRouter.js b/services/web/app/src/Features/Editor/EditorRouter.js index eda8d9c891..f57121f421 100644 --- a/services/web/app/src/Features/Editor/EditorRouter.js +++ b/services/web/app/src/Features/Editor/EditorRouter.js @@ -71,7 +71,7 @@ module.exports = { RateLimiterMiddleware.rateLimit(rateLimiters.joinProject, { params: ['Project_id'], // keep schema in sync with controller - getUserId: req => req.query.user_id, + getUserId: req => req.body.userId || req.query.user_id, }), EditorHttpController.joinProject ) diff --git a/services/web/test/acceptance/src/TokenAccessTests.js b/services/web/test/acceptance/src/TokenAccessTests.js index 453e78e06b..d037ffe3c3 100644 --- a/services/web/test/acceptance/src/TokenAccessTests.js +++ b/services/web/test/acceptance/src/TokenAccessTests.js @@ -160,7 +160,15 @@ const _doTryTokenAccept = ( }) } -const tryContentAccess = (user, projcetId, test, callback) => { +const tryContentAccess = (user, projectId, test, callback) => { + tryContentAccessQuery(user, projectId, test, err1 => { + tryContentAccessBody(user, projectId, test, err2 => { + callback(err1 || err2) + }) + }) +} + +const tryContentAccessQuery = (user, projcetId, test, callback) => { // The real-time service calls this end point to determine the user's // permissions. let userId @@ -191,7 +199,47 @@ const tryContentAccess = (user, projcetId, test, callback) => { ) } +const tryContentAccessBody = (user, projcetId, test, callback) => { + // The real-time service calls this end point to determine the user's + // permissions. + let userId + if (user.id != null) { + userId = user.id + } else { + userId = 'anonymous-user' + } + request.post( + { + url: `/project/${projcetId}/join`, + auth: { + user: settings.apis.web.user, + pass: settings.apis.web.pass, + sendImmediately: true, + }, + json: { + userId, + }, + jar: false, + }, + (error, response, body) => { + if (error != null) { + return callback(error) + } + test(response, body) + callback() + } + ) +} + const tryAnonContentAccess = (user, projectId, token, test, callback) => { + tryAnonContentAccessHeader(user, projectId, token, test, err1 => { + tryAnonContentAccessBody(user, projectId, token, test, err2 => { + callback(err1 || err2) + }) + }) +} + +const tryAnonContentAccessHeader = (user, projectId, token, test, callback) => { // The real-time service calls this end point to determine the user's // permissions. let userId @@ -225,6 +273,39 @@ const tryAnonContentAccess = (user, projectId, token, test, callback) => { ) } +const tryAnonContentAccessBody = (user, projectId, token, test, callback) => { + // The real-time service calls this end point to determine the user's + // permissions. + let userId + if (user.id != null) { + userId = user.id + } else { + userId = 'anonymous-user' + } + request.post( + { + url: `/project/${projectId}/join`, + auth: { + user: settings.apis.web.user, + pass: settings.apis.web.pass, + sendImmediately: true, + }, + json: { + userId, + anonymousAccessToken: token, + }, + jar: false, + }, + (error, response, body) => { + if (error != null) { + return callback(error) + } + test(response, body) + callback() + } + ) +} + const tryFetchProjectTokens = (user, projectId, callback) => { user.request.get( { url: `/project/${projectId}/tokens`, json: true }, diff --git a/services/web/test/unit/src/Editor/EditorHttpControllerTests.js b/services/web/test/unit/src/Editor/EditorHttpControllerTests.js index 80fc1cd704..2b1b8cb500 100644 --- a/services/web/test/unit/src/Editor/EditorHttpControllerTests.js +++ b/services/web/test/unit/src/Editor/EditorHttpControllerTests.js @@ -167,6 +167,7 @@ describe('EditorHttpController', function () { beforeEach(function () { this.req.params = { Project_id: this.project._id } this.req.query = { user_id: this.user._id } + this.req.body = { userId: this.user._id } }) describe('successfully', function () { @@ -251,6 +252,10 @@ describe('EditorHttpController', function () { beforeEach(function (done) { this.token = 'token' this.TokenAccessHandler.getRequestToken.returns(this.token) + this.req.body = { + userId: 'anonymous-user', + anonymousAccessToken: this.token, + } this.req.query = { user_id: 'anonymous-user' } this.req.headers = { 'x-sl-anonymous-access-token': this.token } this.res.callback = done