From 84c08edcf3359000e76cc2c80c26c424bbde2193 Mon Sep 17 00:00:00 2001 From: James Allen Date: Fri, 7 Nov 2014 12:31:47 +0000 Subject: [PATCH] Factor out common joinProject logic to provide and HTTP end point for the real-time API --- .../Features/Editor/EditorController.coffee | 61 ++++++++----- .../Editor/EditorHttpController.coffee | 17 ++++ .../Features/Editor/EditorRouter.coffee | 8 +- .../Features/Project/ProjectDeleter.coffee | 13 ++- services/web/app/coffee/router.coffee | 2 +- .../Editor/EditorControllerTests.coffee | 89 +++++++++++++------ .../Editor/EditorHttpControllerTests.coffee | 53 +++++++++++ .../coffee/Project/ProjectDeleterTests.coffee | 26 ++---- 8 files changed, 192 insertions(+), 77 deletions(-) diff --git a/services/web/app/coffee/Features/Editor/EditorController.coffee b/services/web/app/coffee/Features/Editor/EditorController.coffee index 07b9588bcf..e12f6a9d04 100644 --- a/services/web/app/coffee/Features/Editor/EditorController.coffee +++ b/services/web/app/coffee/Features/Editor/EditorController.coffee @@ -7,6 +7,7 @@ ProjectOptionsHandler = require('../Project/ProjectOptionsHandler') ProjectDetailsHandler = require('../Project/ProjectDetailsHandler') ProjectDeleter = require("../Project/ProjectDeleter") ProjectGetter = require('../Project/ProjectGetter') +UserGetter = require('../User/UserGetter') CollaboratorsHandler = require("../Collaborators/CollaboratorsHandler") DocumentUpdaterHandler = require('../DocumentUpdater/DocumentUpdaterHandler') LimitationsManager = require("../Subscription/LimitationsManager") @@ -33,34 +34,48 @@ module.exports = EditorController = joinProject: (client, user, project_id, callback) -> logger.log user_id:user._id, project_id:project_id, "user joining project" Metrics.inc "editor.join-project" + EditorController.buildJoinProjectView project_id, user._id, (error, project, privilegeLevel, protocolVersion) -> + return callback(error) if error? + if !privilegeLevel + callback new Error("Not authorized") + else + client.join(project_id) + client.set("project_id", project_id) + client.set("owner_id", project.owner._id) + client.set("user_id", user._id) + client.set("first_name", user.first_name) + client.set("last_name", user.last_name) + client.set("email", user.email) + client.set("connected_time", new Date()) + client.set("signup_date", user.signUpDate) + client.set("login_count", user.loginCount) + AuthorizationManager.setPrivilegeLevelOnClient client, privilegeLevel + + callback null, project, privilegeLevel, EditorController.protocolVersion + + # can be done after the connection has happened + ConnectedUsersManager.updateUserPosition project_id, client.id, user, null, -> + + # Only show the 'renamed or deleted' message once + if project.deletedByExternalDataSource + ProjectDeleter.unmarkAsDeletedByExternalSource project_id + + buildJoinProjectView: (project_id, user_id, callback = (error, project, privilegeLevel) ->) -> ProjectGetter.getProjectWithoutDocLines project_id, (error, project) -> return callback(error) if error? ProjectGetter.populateProjectWithUsers project, (error, project) -> return callback(error) if error? - AuthorizationManager.getPrivilegeLevelForProject project, user, - (error, canAccess, privilegeLevel) -> - if error? or !canAccess - callback new Error("Not authorized") + UserGetter.getUser user_id, { isAdmin: true }, (error, user) -> + return callback(error) if error? + AuthorizationManager.getPrivilegeLevelForProject project, user, (error, canAccess, privilegeLevel) -> + return callback(error) if error? + if !canAccess + callback null, null, false else - client.join(project_id) - client.set("project_id", project_id) - client.set("owner_id", project.owner_ref._id) - client.set("user_id", user._id) - client.set("first_name", user.first_name) - client.set("last_name", user.last_name) - client.set("email", user.email) - client.set("connected_time", new Date()) - client.set("signup_date", user.signUpDate) - client.set("login_count", user.loginCount) - AuthorizationManager.setPrivilegeLevelOnClient client, privilegeLevel - callback null, ProjectEditorHandler.buildProjectModelView(project), privilegeLevel, EditorController.protocolVersion - - # can be done affter the connection has happened - ConnectedUsersManager.updateUserPosition project_id, client.id, user, null, -> - - # Only show the 'renamed or deleted' message once - ProjectDeleter.unmarkAsDeletedByExternalSource project - + callback(null, + ProjectEditorHandler.buildProjectModelView(project), + privilegeLevel + ) leaveProject: (client, user) -> self = @ diff --git a/services/web/app/coffee/Features/Editor/EditorHttpController.coffee b/services/web/app/coffee/Features/Editor/EditorHttpController.coffee index 97f61e9b86..0bec9d8fae 100644 --- a/services/web/app/coffee/Features/Editor/EditorHttpController.coffee +++ b/services/web/app/coffee/Features/Editor/EditorHttpController.coffee @@ -1,9 +1,26 @@ ProjectEntityHandler = require "../Project/ProjectEntityHandler" +ProjectDeleter = require "../Project/ProjectDeleter" logger = require "logger-sharelatex" EditorRealTimeController = require "./EditorRealTimeController" EditorController = require "./EditorController" +Metrics = require('../../infrastructure/Metrics') module.exports = EditorHttpController = + joinProject: (req, res, next) -> + project_id = req.params.Project_id + user_id = req.query.user_id + logger.log {user_id, project_id}, "join project request" + Metrics.inc "editor.join-project" + EditorController.buildJoinProjectView project_id, user_id, (error, project, privilegeLevel) -> + return next(error) if error? + res.json { + project: project + privilegeLevel: privilegeLevel + } + # Only show the 'renamed or deleted' message once + if project.deletedByExternalDataSource + ProjectDeleter.unmarkAsDeletedByExternalSource project_id + restoreDoc: (req, res, next) -> project_id = req.params.Project_id doc_id = req.params.doc_id diff --git a/services/web/app/coffee/Features/Editor/EditorRouter.coffee b/services/web/app/coffee/Features/Editor/EditorRouter.coffee index 9163b4d3cd..a7295722ac 100644 --- a/services/web/app/coffee/Features/Editor/EditorRouter.coffee +++ b/services/web/app/coffee/Features/Editor/EditorRouter.coffee @@ -2,7 +2,7 @@ EditorHttpController = require('./EditorHttpController') SecurityManager = require('../../managers/SecurityManager') module.exports = - apply: (app) -> + apply: (app, httpAuth) -> app.post '/project/:Project_id/doc', SecurityManager.requestCanModifyProject, EditorHttpController.addDoc app.post '/project/:Project_id/folder', SecurityManager.requestCanModifyProject, EditorHttpController.addFolder @@ -14,3 +14,9 @@ module.exports = app.delete '/project/:Project_id/folder/:entity_id', SecurityManager.requestCanModifyProject, EditorHttpController.deleteFolder app.post '/project/:Project_id/doc/:doc_id/restore', SecurityManager.requestCanModifyProject, EditorHttpController.restoreDoc + + # Called by the real-time API to load up the current project state. + # This is a post request because it's more than just a getting of data. We take actions + # whenever a user joins a project, like updating the deleted status. + app.post '/project/:Project_id/join', httpAuth, EditorHttpController.joinProject + app.ignoreCsrf('post', '/project/:Project_id/join') \ No newline at end of file diff --git a/services/web/app/coffee/Features/Project/ProjectDeleter.coffee b/services/web/app/coffee/Features/Project/ProjectDeleter.coffee index 9a7decec28..6fdba733e7 100644 --- a/services/web/app/coffee/Features/Project/ProjectDeleter.coffee +++ b/services/web/app/coffee/Features/Project/ProjectDeleter.coffee @@ -16,14 +16,11 @@ module.exports = ProjectDeleter = require('../Editor/EditorController').notifyUsersProjectHasBeenDeletedOrRenamed project_id, -> callback() - unmarkAsDeletedByExternalSource: (project, callback = (error) ->) -> - logger.log project_id: "removing flag marking project as deleted by external data source" - if project.deletedByExternalDataSource - conditions = {_id:project._id.toString()} - update = {deletedByExternalDataSource: false} - Project.update conditions, update, {}, callback - else - callback() + unmarkAsDeletedByExternalSource: (project_id, callback = (error) ->) -> + logger.log project_id: project_id, "removing flag marking project as deleted by external data source" + conditions = {_id:project_id.toString()} + update = {deletedByExternalDataSource: false} + Project.update conditions, update, {}, callback deleteUsersProjects: (owner_id, callback)-> logger.log owner_id:owner_id, "deleting users projects" diff --git a/services/web/app/coffee/router.coffee b/services/web/app/coffee/router.coffee index 269854e6bd..aaf81a8f16 100644 --- a/services/web/app/coffee/router.coffee +++ b/services/web/app/coffee/router.coffee @@ -60,7 +60,7 @@ module.exports = class Router app.get '/register', UserPagesController.registerPage app.post '/register', UserController.register - EditorRouter.apply(app) + EditorRouter.apply(app, httpAuth) CollaboratorsRouter.apply(app) SubscriptionRouter.apply(app) UploadsRouter.apply(app) diff --git a/services/web/test/UnitTests/coffee/Editor/EditorControllerTests.coffee b/services/web/test/UnitTests/coffee/Editor/EditorControllerTests.coffee index 9d15369aa7..023add50f5 100644 --- a/services/web/test/UnitTests/coffee/Editor/EditorControllerTests.coffee +++ b/services/web/test/UnitTests/coffee/Editor/EditorControllerTests.coffee @@ -16,7 +16,9 @@ describe "EditorController", -> @doc_id = "test-doc-id" @source = "dropbox" - @projectModelView = "projectModelView" + @projectModelView = + _id: @project_id + owner:{_id:"something"} @user = _id: @user_id = "user-id" @@ -65,6 +67,7 @@ describe "EditorController", -> '../Project/ProjectDetailsHandler': @ProjectDetailsHandler '../Project/ProjectDeleter' : @ProjectDeleter '../Project/ProjectGetter' : @ProjectGetter = {} + '../User/UserGetter': @UserGetter = {} '../Collaborators/CollaboratorsHandler': @CollaboratorsHandler '../DocumentUpdater/DocumentUpdaterHandler' : @DocumentUpdaterHandler '../Subscription/LimitationsManager' : @LimitationsManager @@ -85,8 +88,6 @@ describe "EditorController", -> beforeEach -> sinon.spy(@client, "set") sinon.spy(@client, "get") - @ProjectGetter.getProjectWithoutDocLines = sinon.stub().callsArgWith(1, null, @project) - @ProjectGetter.populateProjectWithUsers = sinon.stub().callsArgWith(1, null, @project) @AuthorizationManager.setPrivilegeLevelOnClient = sinon.stub() @EditorRealTimeController.emitToRoom = sinon.stub() @ConnectedUsersManager.updateUserPosition.callsArgWith(4) @@ -94,20 +95,9 @@ describe "EditorController", -> describe "when authorized", -> beforeEach -> - @AuthorizationManager.getPrivilegeLevelForProject = - sinon.stub().callsArgWith(2, null, true, "owner") + @EditorController.buildJoinProjectView = sinon.stub().callsArgWith(2, null, @projectModelView, "owner") @EditorController.joinProject(@client, @user, @project_id, @callback) - it "should find the project without doc lines", -> - @ProjectGetter.getProjectWithoutDocLines - .calledWith(@project_id) - .should.equal true - - it "should populate the user references in the project", -> - @ProjectGetter.populateProjectWithUsers - .calledWith(@project) - .should.equal true - it "should set the privilege level on the client", -> @AuthorizationManager.setPrivilegeLevelOnClient .calledWith(@client, "owner") @@ -119,21 +109,15 @@ describe "EditorController", -> it "should set the project_id of the client", -> @client.set.calledWith("project_id", @project_id).should.equal true - it "should return the project model view, privilege level and protocol version", -> - @callback.calledWith(null, @projectModelView, "owner", @EditorController.protocolVersion).should.equal true - it "should mark the user as connected with the ConnectedUsersManager", -> @ConnectedUsersManager.updateUserPosition.calledWith(@project_id, @client.id, @user, null).should.equal true - - it "should remove the flag to send a user a message about the project being deleted", -> - @ProjectDeleter.unmarkAsDeletedByExternalSource - .calledWith(@project) - .should.equal true + + it "should return the project model view, privilege level and protocol version", -> + @callback.calledWith(null, @projectModelView, "owner", @EditorController.protocolVersion).should.equal true describe "when not authorized", -> beforeEach -> - @AuthorizationManager.getPrivilegeLevelForProject = - sinon.stub().callsArgWith(2, null, false) + @EditorController.buildJoinProjectView = sinon.stub().callsArgWith(2, null, null, false) @EditorController.joinProject(@client, @user, @project_id, @callback) it "should not set the privilege level on the client", -> @@ -148,6 +132,61 @@ describe "EditorController", -> it "should return an error", -> @callback.calledWith(sinon.match.truthy).should.equal true + + describe "when the project is marked as deleted", -> + beforeEach -> + @projectModelView.deletedByExternalDataSource = true + @EditorController.buildJoinProjectView = sinon.stub().callsArgWith(2, null, @projectModelView, "owner") + @EditorController.joinProject(@client, @user, @project_id, @callback) + + it "should remove the flag to send a user a message about the project being deleted", -> + @ProjectDeleter.unmarkAsDeletedByExternalSource + .calledWith(@project_id) + .should.equal true + + describe "buildJoinProjectView", -> + beforeEach -> + @ProjectGetter.getProjectWithoutDocLines = sinon.stub().callsArgWith(1, null, @project) + @ProjectGetter.populateProjectWithUsers = sinon.stub().callsArgWith(1, null, @project) + @UserGetter.getUser = sinon.stub().callsArgWith(2, null, @user) + + describe "when authorized", -> + beforeEach -> + @AuthorizationManager.getPrivilegeLevelForProject = + sinon.stub().callsArgWith(2, null, true, "owner") + @EditorController.buildJoinProjectView(@project_id, @user_id, @callback) + + it "should find the project without doc lines", -> + @ProjectGetter.getProjectWithoutDocLines + .calledWith(@project_id) + .should.equal true + + it "should populate the user references in the project", -> + @ProjectGetter.populateProjectWithUsers + .calledWith(@project) + .should.equal true + + it "should look up the user", -> + @UserGetter.getUser + .calledWith(@user_id, { isAdmin: true }) + .should.equal true + + it "should check the privilege level", -> + @AuthorizationManager.getPrivilegeLevelForProject + .calledWith(@project, @user) + .should.equal true + + it "should return the project model view, privilege level and protocol version", -> + @callback.calledWith(null, @projectModelView, "owner", @EditorController.protocolVersion).should.equal true + + describe "when not authorized", -> + beforeEach -> + @AuthorizationManager.getPrivilegeLevelForProject = + sinon.stub().callsArgWith(2, null, false, null) + @EditorController.buildJoinProjectView(@project_id, @user_id, @callback) + + it "should return false in the callback", -> + @callback.calledWith(null, null, false, @EditorController.protocolVersion).should.equal true describe "leaveProject", -> diff --git a/services/web/test/UnitTests/coffee/Editor/EditorHttpControllerTests.coffee b/services/web/test/UnitTests/coffee/Editor/EditorHttpControllerTests.coffee index dda8a5fed8..fce17bb9e3 100644 --- a/services/web/test/UnitTests/coffee/Editor/EditorHttpControllerTests.coffee +++ b/services/web/test/UnitTests/coffee/Editor/EditorHttpControllerTests.coffee @@ -7,16 +7,69 @@ describe "EditorHttpController", -> beforeEach -> @EditorHttpController = SandboxedModule.require modulePath, requires: '../Project/ProjectEntityHandler' : @ProjectEntityHandler = {} + '../Project/ProjectDeleter' : @ProjectDeleter = {} "./EditorRealTimeController": @EditorRealTimeController = {} "logger-sharelatex": @logger = { log: sinon.stub(), error: sinon.stub() } "./EditorController": @EditorController = {} + '../../infrastructure/Metrics': @Metrics = {inc: sinon.stub()} + @project_id = "mock-project-id" @doc_id = "mock-doc-id" + @user_id = "mock-user-id" @parent_folder_id = "mock-folder-id" @req = {} @res = send: sinon.stub() json: sinon.stub() + + describe "joinProject", -> + beforeEach -> + @req.params = + Project_id: @project_id + @req.query = + user_id: @user_id + @projectView = { + _id: @project_id + } + @EditorController.buildJoinProjectView = sinon.stub().callsArgWith(2, null, @projectView, "owner") + @ProjectDeleter.unmarkAsDeletedByExternalSource = sinon.stub() + + describe "successfully", -> + beforeEach -> + @EditorHttpController.joinProject @req, @res + + it "should get the project view", -> + @EditorController.buildJoinProjectView + .calledWith(@project_id, @user_id) + .should.equal true + + it "should return the project and privilege level", -> + @res.json + .calledWith({ + project: @projectView + privilegeLevel: "owner" + }) + .should.equal true + + it "should not try to unmark the project as deleted", -> + @ProjectDeleter.unmarkAsDeletedByExternalSource + .called + .should.equal false + + it "should send an inc metric", -> + @Metrics.inc + .calledWith("editor.join-project") + .should.equal true + + describe "when the project is marked as deleted", -> + beforeEach -> + @projectView.deletedByExternalDataSource = true + @EditorHttpController.joinProject @req, @res + + it "should unmark the project as deleted", -> + @ProjectDeleter.unmarkAsDeletedByExternalSource + .calledWith(@project_id) + .should.equal true describe "restoreDoc", -> beforeEach -> diff --git a/services/web/test/UnitTests/coffee/Project/ProjectDeleterTests.coffee b/services/web/test/UnitTests/coffee/Project/ProjectDeleterTests.coffee index 99f2a5352b..13f5d2a694 100644 --- a/services/web/test/UnitTests/coffee/Project/ProjectDeleterTests.coffee +++ b/services/web/test/UnitTests/coffee/Project/ProjectDeleterTests.coffee @@ -4,7 +4,7 @@ SandboxedModule = require('sandboxed-module') sinon = require('sinon') -describe 'Project deleter', -> +describe 'ProjectDeleter', -> beforeEach -> @project_id = "12312" @@ -55,24 +55,12 @@ describe 'Project deleter', -> @project = { _id: @project_id } - - describe "when the project does not have the flag set", -> - beforeEach -> - @project.deletedByExternalDataSource = false - @deleter.unmarkAsDeletedByExternalSource @project, @callback - - it "should not update the project", -> - @Project.update.called.should.equal false - - describe "when the project does have the flag set", -> - beforeEach -> - @project.deletedByExternalDataSource = true - @deleter.unmarkAsDeletedByExternalSource @project, @callback - - it "should remove the flag from the project", -> - @Project.update - .calledWith({_id: @project_id}, {deletedByExternalDataSource:false}) - .should.equal true + @deleter.unmarkAsDeletedByExternalSource @project_id, @callback + + it "should remove the flag from the project", -> + @Project.update + .calledWith({_id: @project_id}, {deletedByExternalDataSource:false}) + .should.equal true describe "deleteUsersProjects", ->