diff --git a/services/web/app.coffee b/services/web/app.coffee index 3d45307e5d..8496af8f17 100644 --- a/services/web/app.coffee +++ b/services/web/app.coffee @@ -18,18 +18,6 @@ argv = require("optimist") .usage("Usage: $0") .argv -Server.app.use (error, req, res, next) -> - if error?.code is 'EBADCSRFTOKEN' - logger.log err: error,url:req.url, method:req.method, user:req?.sesson?.user, "invalid csrf" - res.sendStatus(403) - return - logger.error err: error, url:req.url, method:req.method, user:req?.sesson?.user, "error passed to top level next middlewear" - res.statusCode = error.status or 500 - if res.statusCode == 500 - res.end("Oops, something went wrong with your request, sorry. If this continues, please contact us at #{Settings.adminEmail}") - else - res.end() - if Settings.catchErrors process.removeAllListeners "uncaughtException" process.on "uncaughtException", (error) -> diff --git a/services/web/app/coffee/Features/Authorization/AuthorizationManager.coffee b/services/web/app/coffee/Features/Authorization/AuthorizationManager.coffee index db49881bbf..eb6f564fea 100644 --- a/services/web/app/coffee/Features/Authorization/AuthorizationManager.coffee +++ b/services/web/app/coffee/Features/Authorization/AuthorizationManager.coffee @@ -3,6 +3,7 @@ Project = require("../../models/Project").Project User = require("../../models/User").User PrivilegeLevels = require("./PrivilegeLevels") PublicAccessLevels = require("./PublicAccessLevels") +Errors = require("../Errors/Errors") module.exports = AuthorizationManager = # Get the privilege level that the user has for the project @@ -14,8 +15,10 @@ module.exports = AuthorizationManager = getPublicAccessLevel = () -> Project.findOne { _id: project_id }, { publicAccesLevel: 1 }, (error, project) -> return callback(error) if error? + if !project? + return callback new Errors.NotFoundError("no project found with id #{project_id}") if project.publicAccesLevel == PublicAccessLevels.READ_ONLY - return callback null, PrivilegeLevels.READ_ONLY + return callback null, PrivilegeLevels.READ_ONLY, true else if project.publicAccesLevel == PublicAccessLevels.READ_AND_WRITE return callback null, PrivilegeLevels.READ_AND_WRITE, true else diff --git a/services/web/app/coffee/Features/Authorization/AuthorizationMiddlewear.coffee b/services/web/app/coffee/Features/Authorization/AuthorizationMiddlewear.coffee index 2db1632ecf..4888db0c8a 100644 --- a/services/web/app/coffee/Features/Authorization/AuthorizationMiddlewear.coffee +++ b/services/web/app/coffee/Features/Authorization/AuthorizationMiddlewear.coffee @@ -1,6 +1,8 @@ AuthorizationManager = require("./AuthorizationManager") async = require "async" logger = require "logger-sharelatex" +ObjectId = require("mongojs").ObjectId +Errors = require "../Errors/Errors" module.exports = AuthorizationMiddlewear = ensureUserCanReadMultipleProjects: (req, res, next) -> @@ -83,6 +85,8 @@ module.exports = AuthorizationMiddlewear = project_id = req.params?.project_id or req.params?.Project_id if !project_id? return callback(new Error("Expected project_id in request parameters")) + if !ObjectId.isValid(project_id) + return callback(new Errors.NotFoundError("invalid project_id: #{project_id}")) AuthorizationMiddlewear._getUserId req, (error, user_id) -> return callback(error) if error? callback(null, user_id, project_id) diff --git a/services/web/app/coffee/Features/Collaborators/CollaboratorsHandler.coffee b/services/web/app/coffee/Features/Collaborators/CollaboratorsHandler.coffee index cb6459c85f..71737eecff 100644 --- a/services/web/app/coffee/Features/Collaborators/CollaboratorsHandler.coffee +++ b/services/web/app/coffee/Features/Collaborators/CollaboratorsHandler.coffee @@ -7,12 +7,13 @@ ContactManager = require "../Contacts/ContactManager" CollaboratorsEmailHandler = require "./CollaboratorsEmailHandler" async = require "async" PrivilegeLevels = require "../Authorization/PrivilegeLevels" +Errors = require "../Errors/Errors" module.exports = CollaboratorsHandler = getMemberIdsWithPrivilegeLevels: (project_id, callback = (error, members) ->) -> Project.findOne { _id: project_id }, { owner_ref: 1, collaberator_refs: 1, readOnly_refs: 1 }, (error, project) -> return callback(error) if error? - return callback null, null if !project? + return callback new Errors.NotFoundError("no project found with id #{project_id}") if !project? members = [] members.push { id: project.owner_ref.toString(), privilegeLevel: PrivilegeLevels.OWNER } for member_id in project.readOnly_refs or [] diff --git a/services/web/app/coffee/Features/Errors/ErrorController.coffee b/services/web/app/coffee/Features/Errors/ErrorController.coffee index d0589ba5ed..a0ce6c85c9 100644 --- a/services/web/app/coffee/Features/Errors/ErrorController.coffee +++ b/services/web/app/coffee/Features/Errors/ErrorController.coffee @@ -1,5 +1,24 @@ +Errors = require "./Errors" +logger = require "logger-sharelatex" + module.exports = ErrorController = notFound: (req, res)-> - res.statusCode = 404 + res.status(404) res.render 'general/404', - title: "page_not_found" \ No newline at end of file + title: "page_not_found" + + serverError: (req, res)-> + res.status(500) + res.render 'general/500', + title: "Server Error" + + handleError: (error, req, res, next) -> + if error?.code is 'EBADCSRFTOKEN' + logger.log err: error,url:req.url, method:req.method, user:req?.sesson?.user, "invalid csrf" + res.sendStatus(403) + return + logger.error err: error, url:req.url, method:req.method, user:req?.sesson?.user, "error passed to top level next middlewear" + if error instanceof Errors.NotFoundError + ErrorController.notFound req, res + else + ErrorController.serverError req, res \ No newline at end of file diff --git a/services/web/app/coffee/Features/Errors/Errors.coffee b/services/web/app/coffee/Features/Errors/Errors.coffee new file mode 100644 index 0000000000..0bbff1f19b --- /dev/null +++ b/services/web/app/coffee/Features/Errors/Errors.coffee @@ -0,0 +1,9 @@ +NotFoundError = (message) -> + error = new Error(message) + error.name = "NotFoundError" + error.__proto__ = NotFoundError.prototype + return error +NotFoundError.prototype.__proto__ = Error.prototype + +module.exports = Errors = + NotFoundError: NotFoundError \ No newline at end of file diff --git a/services/web/app/coffee/infrastructure/Server.coffee b/services/web/app/coffee/infrastructure/Server.coffee index 4a035ba007..fea8752bb2 100644 --- a/services/web/app/coffee/infrastructure/Server.coffee +++ b/services/web/app/coffee/infrastructure/Server.coffee @@ -30,6 +30,8 @@ OldAssetProxy = require("./OldAssetProxy") translations = require("translations-sharelatex").setup(Settings.i18n) Modules = require "./Modules" +ErrorController = require "../Features/Errors/ErrorController" + metrics.mongodb.monitor(Path.resolve(__dirname + "/../../../node_modules/mongojs/node_modules/mongodb"), logger) metrics.mongodb.monitor(Path.resolve(__dirname + "/../../../node_modules/mongoose/node_modules/mongodb"), logger) @@ -136,6 +138,8 @@ app.use(webRouter) router = new Router(webRouter, apiRouter) +app.use ErrorController.handleError + module.exports = app: app server: server diff --git a/services/web/app/views/general/500.jade b/services/web/app/views/general/500.jade new file mode 100644 index 0000000000..12983b923b --- /dev/null +++ b/services/web/app/views/general/500.jade @@ -0,0 +1,16 @@ +extends ../layout + +block content + .content + .container + .row + .col-md-8.col-md-offset-2.text-center + .page-header + h2 Oh dear, something went wrong. + p: img(src="/img/lion-sad-128.png", alt="Sad Lion") + p + | Something went wrong with your request, sorry. Our staff are probably looking into this, but it continues, please contact us at #{settings.adminEmail} + p + a(href="/") + i.fa.fa-arrow-circle-o-left + | #{translate("take_me_home")} diff --git a/services/web/public/img/lion-sad-128.png b/services/web/public/img/lion-sad-128.png new file mode 100644 index 0000000000..e5e58c42b0 Binary files /dev/null and b/services/web/public/img/lion-sad-128.png differ diff --git a/services/web/test/UnitTests/coffee/Authorization/AuthorizationManagerTests.coffee b/services/web/test/UnitTests/coffee/Authorization/AuthorizationManagerTests.coffee index b1a464829c..1e3b6b0ebe 100644 --- a/services/web/test/UnitTests/coffee/Authorization/AuthorizationManagerTests.coffee +++ b/services/web/test/UnitTests/coffee/Authorization/AuthorizationManagerTests.coffee @@ -4,6 +4,7 @@ should = chai.should() expect = chai.expect modulePath = "../../../../app/js/Features/Authorization/AuthorizationManager.js" SandboxedModule = require('sandboxed-module') +Errors = require "../../../../app/js/Features/Errors/Errors.js" describe "AuthorizationManager", -> beforeEach -> @@ -11,6 +12,7 @@ describe "AuthorizationManager", -> "../Collaborators/CollaboratorsHandler": @CollaboratorsHandler = {} "../../models/Project": Project: @Project = {} "../../models/User": User: @User = {} + "../Errors/Errors": Errors @user_id = "user-id-1" @project_id = "project-id-1" @callback = sinon.stub() @@ -91,6 +93,16 @@ describe "AuthorizationManager", -> it "should return the public privilege level", -> @callback.calledWith(null, "readAndWrite", true).should.equal true + + describe "when the project doesn't exist", -> + beforeEach -> + @Project.findOne + .withArgs({ _id: @project_id }, { publicAccesLevel: 1 }) + .yields(null, null) + + it "should return a NotFoundError", -> + @AuthorizationManager.getPrivilegeLevelForProject @user_id, @project_id, (error) -> + error.should.be.instanceof Errors.NotFoundError describe "canUserReadProject", -> beforeEach -> diff --git a/services/web/test/UnitTests/coffee/Authorization/AuthorizationMiddlewearTests.coffee b/services/web/test/UnitTests/coffee/Authorization/AuthorizationMiddlewearTests.coffee index 7e1ca2d5ef..bc62e603de 100644 --- a/services/web/test/UnitTests/coffee/Authorization/AuthorizationMiddlewearTests.coffee +++ b/services/web/test/UnitTests/coffee/Authorization/AuthorizationMiddlewearTests.coffee @@ -4,16 +4,21 @@ should = chai.should() expect = chai.expect modulePath = "../../../../app/js/Features/Authorization/AuthorizationMiddlewear.js" SandboxedModule = require('sandboxed-module') +Errors = require "../../../../app/js/Features/Errors/Errors.js" describe "AuthorizationMiddlewear", -> beforeEach -> @AuthorizationMiddlewear = SandboxedModule.require modulePath, requires: "./AuthorizationManager": @AuthorizationManager = {} "logger-sharelatex": {log: () ->} + "mongojs": ObjectId: @ObjectId = {} + "../Errors/Errors": Errors @user_id = "user-id-123" @project_id = "project-id-123" @req = {} @res = {} + @ObjectId.isValid = sinon.stub() + @ObjectId.isValid.withArgs(@project_id).returns true @next = sinon.stub() METHODS_TO_TEST = { @@ -90,6 +95,17 @@ describe "AuthorizationMiddlewear", -> @AuthorizationMiddlewear.redirectToRestricted .calledWith(@req, @res, @next) .should.equal true + + describe "with malformed project id", -> + beforeEach -> + @req.params = + project_id: "blah" + @ObjectId.isValid = sinon.stub().returns false + + it "should return a not found error", (done) -> + @AuthorizationMiddlewear[middlewearMethod] @req, @res, (error) -> + error.should.be.instanceof Errors.NotFoundError + done() describe "ensureUserIsSiteAdmin", -> beforeEach -> diff --git a/services/web/test/UnitTests/coffee/Collaborators/CollaboratorsHandlerTests.coffee b/services/web/test/UnitTests/coffee/Collaborators/CollaboratorsHandlerTests.coffee index f659cfebe6..632b43e7f2 100644 --- a/services/web/test/UnitTests/coffee/Collaborators/CollaboratorsHandlerTests.coffee +++ b/services/web/test/UnitTests/coffee/Collaborators/CollaboratorsHandlerTests.coffee @@ -5,6 +5,7 @@ path = require('path') sinon = require('sinon') modulePath = path.join __dirname, "../../../../app/js/Features/Collaborators/CollaboratorsHandler" expect = require("chai").expect +Errors = require "../../../../app/js/Features/Errors/Errors.js" describe "CollaboratorsHandler", -> beforeEach -> @@ -16,6 +17,7 @@ describe "CollaboratorsHandler", -> "../../models/Project": Project: @Project = {} "../Project/ProjectEntityHandler": @ProjectEntityHandler = {} "./CollaboratorsEmailHandler": @CollaboratorsEmailHandler = {} + "../Errors/Errors": Errors @project_id = "mock-project-id" @user_id = "mock-user-id" @@ -24,25 +26,35 @@ describe "CollaboratorsHandler", -> @callback = sinon.stub() describe "getMemberIdsWithPrivilegeLevels", -> - beforeEach -> - @Project.findOne = sinon.stub() - @Project.findOne.withArgs({_id: @project_id}, {owner_ref: 1, collaberator_refs: 1, readOnly_refs: 1}).yields(null, @project = { - owner_ref: [ "owner-ref" ] - readOnly_refs: [ "read-only-ref-1", "read-only-ref-2" ] - collaberator_refs: [ "read-write-ref-1", "read-write-ref-2" ] - }) - @CollaboratorHandler.getMemberIdsWithPrivilegeLevels @project_id, @callback + describe "with project", -> + beforeEach -> + @Project.findOne = sinon.stub() + @Project.findOne.withArgs({_id: @project_id}, {owner_ref: 1, collaberator_refs: 1, readOnly_refs: 1}).yields(null, @project = { + owner_ref: [ "owner-ref" ] + readOnly_refs: [ "read-only-ref-1", "read-only-ref-2" ] + collaberator_refs: [ "read-write-ref-1", "read-write-ref-2" ] + }) + @CollaboratorHandler.getMemberIdsWithPrivilegeLevels @project_id, @callback - it "should return an array of member ids with their privilege levels", -> - @callback - .calledWith(null, [ - { id: "owner-ref", privilegeLevel: "owner" } - { id: "read-only-ref-1", privilegeLevel: "readOnly" } - { id: "read-only-ref-2", privilegeLevel: "readOnly" } - { id: "read-write-ref-1", privilegeLevel: "readAndWrite" } - { id: "read-write-ref-2", privilegeLevel: "readAndWrite" } - ]) - .should.equal true + it "should return an array of member ids with their privilege levels", -> + @callback + .calledWith(null, [ + { id: "owner-ref", privilegeLevel: "owner" } + { id: "read-only-ref-1", privilegeLevel: "readOnly" } + { id: "read-only-ref-2", privilegeLevel: "readOnly" } + { id: "read-write-ref-1", privilegeLevel: "readAndWrite" } + { id: "read-write-ref-2", privilegeLevel: "readAndWrite" } + ]) + .should.equal true + + describe "with a missing project", -> + beforeEach -> + @Project.findOne = sinon.stub().yields(null, null) + + it "should return a NotFoundError", (done) -> + @CollaboratorHandler.getMemberIdsWithPrivilegeLevels @project_id, (error) -> + error.should.be.instanceof Errors.NotFoundError + done() describe "getMemberIds", -> beforeEach -> diff --git a/services/web/test/acceptance/coffee/AuthorizationTests.coffee b/services/web/test/acceptance/coffee/AuthorizationTests.coffee index 0cacffb5cf..5d54151483 100644 --- a/services/web/test/acceptance/coffee/AuthorizationTests.coffee +++ b/services/web/test/acceptance/coffee/AuthorizationTests.coffee @@ -1,83 +1,8 @@ -request = require("request") expect = require("chai").expect -async = require "async" -settings = require("settings-sharelatex") -{db} = require("../../../app/js/infrastructure/mongojs") - -count = 0 -BASE_URL = "http://localhost:3000" - -request = request.defaults({ - baseUrl: BASE_URL, - followRedirect: false -}) - -class User - constructor: (options = {}) -> - @email = "acceptance-test-#{count}@example.com" - @password = "acceptance-test-#{count}-password" - count++ - @jar = request.jar() - @request = request.defaults({ - jar: @jar - }) - - login: (callback = (error) ->) -> - @getCsrfToken (error) => - return callback(error) if error? - @request.post { - url: "/register" # Register will log in, but also ensure user exists - json: - email: @email - password: @password - }, (error, response, body) => - return callback(error) if error? - db.users.findOne {email: @email}, (error, user) => - return callback(error) if error? - @id = user?._id?.toString() - callback() - - createProject: (name, callback = (error, project_id) ->) -> - @request.post { - url: "/project/new", - json: - projectName: name - }, (error, response, body) -> - return callback(error) if error? - if !body?.project_id? - console.error "SOMETHING WENT WRONG CREATING PROJECT", response.statusCode, response.headers["location"], body - callback(null, body.project_id) - - addUserToProject: (project_id, email, privileges, callback = (error, user) ->) -> - @request.post { - url: "/project/#{project_id}/users", - json: {email, privileges} - }, (error, response, body) -> - return callback(error) if error? - callback(null, body.user) - - makePublic: (project_id, level, callback = (error) ->) -> - @request.post { - url: "/project/#{project_id}/settings/admin", - json: - publicAccessLevel: level - }, (error, response, body) -> - return callback(error) if error? - callback(null) - - getCsrfToken: (callback = (error) ->) -> - @request.get { - url: "/register" - }, (err, response, body) => - return callback(error) if error? - csrfMatches = body.match("window.csrfToken = \"(.*?)\";") - if !csrfMatches? - return callback(new Error("no csrf token found")) - @request = @request.defaults({ - headers: - "x-csrf-token": csrfMatches[1] - }) - callback() +async = require("async") +User = require "./helpers/User" +request = require "./helpers/request" +settings = require "settings-sharelatex" try_read_access = (user, project_id, test, callback) -> async.series [ diff --git a/services/web/test/acceptance/coffee/ProjectCRUDTests.coffee b/services/web/test/acceptance/coffee/ProjectCRUDTests.coffee new file mode 100644 index 0000000000..19e5f445f7 --- /dev/null +++ b/services/web/test/acceptance/coffee/ProjectCRUDTests.coffee @@ -0,0 +1,21 @@ +expect = require("chai").expect +async = require("async") +User = require "./helpers/User" + +describe "Project CRUD", -> + before (done) -> + @user = new User() + @user.login done + + describe "when project doesn't exist", -> + it "should return 404", (done) -> + @user.request.get "/project/aaaaaaaaaaaaaaaaaaaaaaaa", (err, res, body) -> + expect(res.statusCode).to.equal 404 + done() + + describe "when project has malformed id", -> + it "should return 404", (done) -> + @user.request.get "/project/blah", (err, res, body) -> + expect(res.statusCode).to.equal 404 + done() + \ No newline at end of file diff --git a/services/web/test/acceptance/coffee/helpers/User.coffee b/services/web/test/acceptance/coffee/helpers/User.coffee new file mode 100644 index 0000000000..f1c38029d7 --- /dev/null +++ b/services/web/test/acceptance/coffee/helpers/User.coffee @@ -0,0 +1,74 @@ +request = require("./request") +settings = require("settings-sharelatex") +{db} = require("../../../../app/js/infrastructure/mongojs") + +count = 0 + +class User + constructor: (options = {}) -> + @email = "acceptance-test-#{count}@example.com" + @password = "acceptance-test-#{count}-password" + count++ + @jar = request.jar() + @request = request.defaults({ + jar: @jar + }) + + login: (callback = (error) ->) -> + @getCsrfToken (error) => + return callback(error) if error? + @request.post { + url: "/register" # Register will log in, but also ensure user exists + json: + email: @email + password: @password + }, (error, response, body) => + return callback(error) if error? + db.users.findOne {email: @email}, (error, user) => + return callback(error) if error? + @id = user?._id?.toString() + callback() + + createProject: (name, callback = (error, project_id) ->) -> + @request.post { + url: "/project/new", + json: + projectName: name + }, (error, response, body) -> + return callback(error) if error? + if !body?.project_id? + console.error "SOMETHING WENT WRONG CREATING PROJECT", response.statusCode, response.headers["location"], body + callback(null, body.project_id) + + addUserToProject: (project_id, email, privileges, callback = (error, user) ->) -> + @request.post { + url: "/project/#{project_id}/users", + json: {email, privileges} + }, (error, response, body) -> + return callback(error) if error? + callback(null, body.user) + + makePublic: (project_id, level, callback = (error) ->) -> + @request.post { + url: "/project/#{project_id}/settings/admin", + json: + publicAccessLevel: level + }, (error, response, body) -> + return callback(error) if error? + callback(null) + + getCsrfToken: (callback = (error) ->) -> + @request.get { + url: "/register" + }, (err, response, body) => + return callback(error) if error? + csrfMatches = body.match("window.csrfToken = \"(.*?)\";") + if !csrfMatches? + return callback(new Error("no csrf token found")) + @request = @request.defaults({ + headers: + "x-csrf-token": csrfMatches[1] + }) + callback() + +module.exports = User \ No newline at end of file diff --git a/services/web/test/acceptance/coffee/helpers/request.coffee b/services/web/test/acceptance/coffee/helpers/request.coffee new file mode 100644 index 0000000000..879acd843a --- /dev/null +++ b/services/web/test/acceptance/coffee/helpers/request.coffee @@ -0,0 +1,5 @@ +BASE_URL = "http://localhost:3000" +module.exports = require("request").defaults({ + baseUrl: BASE_URL, + followRedirect: false +}) \ No newline at end of file