Merge pull request #512 from sharelatex/ja-project-name-validation

Refactor project name validation into one place and restrict /s
This commit is contained in:
James Allen 2017-06-05 11:55:39 +01:00 committed by GitHub
commit d7981fd2d4
24 changed files with 204 additions and 114 deletions

View file

@ -176,7 +176,7 @@ module.exports = EditorController =
callback?()
renameProject: (project_id, newName, callback = (err) ->) ->
ProjectDetailsHandler.renameProject project_id, newName, ->
ProjectDetailsHandler.renameProject project_id, newName, (err) ->
if err?
logger.err err:err, project_id:project_id, newName:newName, "error renaming project"
return callback(err)

View file

@ -25,6 +25,10 @@ module.exports = ErrorController =
else if error instanceof Errors.TooManyRequestsError
logger.warn {err: error, url: req.url}, "too many requests error"
res.sendStatus(429)
else if error instanceof Errors.InvalidNameError
logger.warn {err: error, url: req.url}, "invalid name error"
res.status(400)
res.send(error.message)
else
logger.error err: error, url:req.url, method:req.method, user:user, "error passed to top level next middlewear"
ErrorController.serverError req, res

View file

@ -5,7 +5,6 @@ NotFoundError = (message) ->
return error
NotFoundError.prototype.__proto__ = Error.prototype
ServiceNotConfiguredError = (message) ->
error = new Error(message)
error.name = "ServiceNotConfiguredError"
@ -13,7 +12,6 @@ ServiceNotConfiguredError = (message) ->
return error
ServiceNotConfiguredError.prototype.__proto__ = Error.prototype
TooManyRequestsError = (message) ->
error = new Error(message)
error.name = "TooManyRequestsError"
@ -21,8 +19,15 @@ TooManyRequestsError = (message) ->
return error
TooManyRequestsError.prototype.__proto__ = Error.prototype
InvalidNameError = (message) ->
error = new Error(message)
error.name = "InvalidNameError"
error.__proto__ = InvalidNameError.prototype
return error
InvalidNameError.prototype.__proto__ = Error.prototype
module.exports = Errors =
NotFoundError: NotFoundError
ServiceNotConfiguredError: ServiceNotConfiguredError
TooManyRequestsError: TooManyRequestsError
InvalidNameError: InvalidNameError

View file

@ -101,7 +101,7 @@ module.exports = ProjectController =
res.send(project_id:project._id)
newProject: (req, res)->
newProject: (req, res, next)->
user_id = AuthenticationController.getLoggedInUserId(req)
projectName = req.body.projectName?.trim()
template = req.body.template
@ -113,25 +113,17 @@ module.exports = ProjectController =
else
projectCreationHandler.createBasicProject user_id, projectName, cb
], (err, project)->
if err?
logger.error err: err, project: project, user: user_id, name: projectName, templateType: template, "error creating project"
res.sendStatus 500
else
logger.log project: project, user: user_id, name: projectName, templateType: template, "created project"
res.send {project_id:project._id}
return next(err) if err?
logger.log project: project, user: user_id, name: projectName, templateType: template, "created project"
res.send {project_id:project._id}
renameProject: (req, res)->
renameProject: (req, res, next)->
project_id = req.params.Project_id
newName = req.body.newProjectName
if newName.length > 150
return res.sendStatus 400
editorController.renameProject project_id, newName, (err)->
if err?
logger.err err:err, project_id:project_id, newName:newName, "problem renaming project"
res.sendStatus 500
else
res.sendStatus 200
return next(err) if err?
res.sendStatus 200
projectListPage: (req, res, next)->
timer = new metrics.Timer("project-list")

View file

@ -6,6 +6,7 @@ ObjectId = require('mongoose').Types.ObjectId
Project = require('../../models/Project').Project
Folder = require('../../models/Folder').Folder
ProjectEntityHandler = require('./ProjectEntityHandler')
ProjectDetailsHandler = require('./ProjectDetailsHandler')
User = require('../../models/User').User
fs = require('fs')
Path = require "path"
@ -15,19 +16,21 @@ module.exports = ProjectCreationHandler =
createBlankProject : (owner_id, projectName, callback = (error, project) ->)->
metrics.inc("project-creation")
logger.log owner_id:owner_id, projectName:projectName, "creating blank project"
rootFolder = new Folder {'name':'rootFolder'}
project = new Project
owner_ref : new ObjectId(owner_id)
name : projectName
if Settings.currentImageName?
project.imageName = Settings.currentImageName
project.rootFolder[0] = rootFolder
User.findById owner_id, "ace.spellCheckLanguage", (err, user)->
project.spellCheckLanguage = user.ace.spellCheckLanguage
project.save (err)->
return callback(err) if err?
callback err, project
ProjectDetailsHandler.validateProjectName projectName, (error) ->
return callback(error) if error?
logger.log owner_id:owner_id, projectName:projectName, "creating blank project"
rootFolder = new Folder {'name':'rootFolder'}
project = new Project
owner_ref : new ObjectId(owner_id)
name : projectName
if Settings.currentImageName?
project.imageName = Settings.currentImageName
project.rootFolder[0] = rootFolder
User.findById owner_id, "ace.spellCheckLanguage", (err, user)->
project.spellCheckLanguage = user.ace.spellCheckLanguage
project.save (err)->
return callback(err) if err?
callback err, project
createBasicProject : (owner_id, projectName, callback = (error, project) ->)->
self = @

View file

@ -7,8 +7,7 @@ _ = require("underscore")
PublicAccessLevels = require("../Authorization/PublicAccessLevels")
Errors = require("../Errors/Errors")
module.exports =
module.exports = ProjectDetailsHandler =
getDetails: (project_id, callback)->
ProjectGetter.getProject project_id, {name:true, description:true, compiler:true, features:true, owner_ref:true}, (err, project)->
if err?
@ -39,16 +38,29 @@ module.exports =
callback(err)
renameProject: (project_id, newName, callback = ->)->
logger.log project_id: project_id, newName:newName, "renaming project"
ProjectGetter.getProject project_id, {name:true}, (err, project)->
if err? or !project?
logger.err err:err, project_id:project_id, "error getting project or could not find it todo project rename"
return callback(err)
oldProjectName = project.name
Project.update _id:project_id, {name: newName}, (err, project)=>
if err?
ProjectDetailsHandler.validateProjectName newName, (error) ->
return callback(error) if error?
logger.log project_id: project_id, newName:newName, "renaming project"
ProjectGetter.getProject project_id, {name:true}, (err, project)->
if err? or !project?
logger.err err:err, project_id:project_id, "error getting project or could not find it todo project rename"
return callback(err)
tpdsUpdateSender.moveEntity {project_id:project_id, project_name:oldProjectName, newProjectName:newName}, callback
oldProjectName = project.name
Project.update _id:project_id, {name: newName}, (err, project)=>
if err?
return callback(err)
tpdsUpdateSender.moveEntity {project_id:project_id, project_name:oldProjectName, newProjectName:newName}, callback
MAX_PROJECT_NAME_LENGTH: 150
validateProjectName: (name, callback = (error) ->) ->
if name.length == 0
return callback(new Errors.InvalidNameError("Project name cannot be blank"))
else if name.length > @MAX_PROJECT_NAME_LENGTH
return callback(new Errors.InvalidNameError("Project name is too long"))
else if name.indexOf("/") > -1
return callback(new Errors.InvalidNameError("Project name cannot not contain / characters"))
else
return callback()
setPublicAccessLevel : (project_id, newAccessLevel, callback = ->)->
logger.log project_id: project_id, level: newAccessLevel, "set public access level"

View file

@ -1,6 +1,5 @@
Project = require('../../models/Project').Project
logger = require('logger-sharelatex')
Project = require("../../models/Project").Project
module.exports =
markAsUpdated : (project_id, callback)->

View file

@ -167,6 +167,8 @@ script(type='text/ng-template', id='cloneProjectModalTemplate')
.modal-header
h3 #{translate("copy_project")}
.modal-body
.alert.alert-danger(ng-show="state.error.message") {{ state.error.message}}
.alert.alert-danger(ng-show="state.error && !state.error.message") #{translate("generic_something_went_wrong")}
form(name="cloneProjectForm", novalidate)
.form-group
label #{translate("new_name")}

View file

@ -98,6 +98,8 @@ script(type='text/ng-template', id='renameProjectModalTemplate')
) ×
h3 #{translate("rename_project")}
.modal-body
.alert.alert-danger(ng-show="state.error.message") {{ state.error.message}}
.alert.alert-danger(ng-show="state.error && !state.error.message") #{translate("generic_something_went_wrong")}
form(name="renameProjectForm", novalidate)
input.form-control(
type="text",
@ -111,8 +113,10 @@ script(type='text/ng-template', id='renameProjectModalTemplate')
button.btn.btn-default(ng-click="cancel()") #{translate("cancel")}
button.btn.btn-primary(
ng-click="rename()",
ng-disabled="renameProjectForm.$invalid"
) #{translate("rename")}
ng-disabled="renameProjectForm.$invalid || state.inflight"
)
span(ng-show="!state.inflight") #{translate("rename")}
span(ng-show="state.inflight") #{translate("renaming")}...
script(type='text/ng-template', id='cloneProjectModalTemplate')
.modal-header
@ -123,6 +127,8 @@ script(type='text/ng-template', id='cloneProjectModalTemplate')
) ×
h3 #{translate("copy_project")}
.modal-body
.alert.alert-danger(ng-show="state.error.message") {{ state.error.message}}
.alert.alert-danger(ng-show="state.error && !state.error.message") #{translate("generic_something_went_wrong")}
form(name="cloneProjectForm", novalidate)
.form-group
label #{translate("new_name")}
@ -155,6 +161,8 @@ script(type='text/ng-template', id='newProjectModalTemplate')
) ×
h3 #{translate("new_project")}
.modal-body
.alert.alert-danger(ng-show="state.error.message") {{ state.error.message}}
.alert.alert-danger(ng-show="state.error && !state.error.message") #{translate("generic_something_went_wrong")}
form(novalidate, name="newProjectForm")
input.form-control(
type="text",

View file

@ -6,6 +6,7 @@ define [
projectName: ide.$scope.project.name + " (Copy)"
$scope.state =
inflight: false
error: false
$modalInstance.opened.then () ->
$timeout () ->
@ -20,9 +21,16 @@ define [
$scope.clone = () ->
$scope.state.inflight = true
$scope.state.error = false
cloneProject($scope.inputs.projectName)
.then (data) ->
.success (data) ->
window.location = "/project/#{data.data.project_id}"
.error (body, statusCode) ->
$scope.state.inflight = false
if statusCode == 400
$scope.state.error = { message: body }
else
$scope.state.error = true
$scope.cancel = () ->
$modalInstance.dismiss('cancel')

View file

@ -19,12 +19,18 @@ define [
$scope.finishRenaming = () ->
$scope.state.renaming = false
newName = $scope.inputs.name
if !newName? or newName.length == 0 or newName.length > MAX_PROJECT_NAME_LENGTH
return
if $scope.project.name == newName
return
oldName = $scope.project.name
$scope.project.name = newName
settings.saveProjectSettings({name: $scope.project.name})
.error (response, statusCode) ->
$scope.project.name = oldName
if statusCode == 400
ide.showGenericMessageModal("Error renaming project", response)
else
ide.showGenericMessageModal("Error renaming project", "Please try again in a moment")
console.log arguments
ide.socket.on "projectNameUpdated", (name) ->
$scope.$apply () ->

View file

@ -12,8 +12,7 @@ define [
# End of tracking code.
data._csrf = window.csrfToken
ide.$http.post "/user/settings", data
return ide.$http.post "/user/settings", data
saveProjectSettings: (data) ->
# Tracking code.
@ -24,9 +23,8 @@ define [
# End of tracking code.
data._csrf = window.csrfToken
ide.$http.post "/project/#{ide.project_id}/settings", data
return ide.$http.post "/project/#{ide.project_id}/settings", data
saveProjectAdminSettings: (data) ->
# Tracking code.
for key in Object.keys(data)
@ -36,8 +34,6 @@ define [
# End of tracking code.
data._csrf = window.csrfToken
ide.$http.post "/project/#{ide.project_id}/settings/admin", data
return ide.$http.post "/project/#{ide.project_id}/settings/admin", data
}
]

View file

@ -1,9 +1,13 @@
define [
"base"
], (App) ->
App.controller 'RenameProjectModalController', ($scope, $modalInstance, $timeout, projectName) ->
App.controller 'RenameProjectModalController', ($scope, $modalInstance, $timeout, project, queuedHttp) ->
$scope.inputs =
projectName: projectName
projectName: project.name
$scope.state =
inflight: false
error: false
$modalInstance.opened.then () ->
$timeout () ->
@ -11,7 +15,20 @@ define [
, 200
$scope.rename = () ->
$modalInstance.close($scope.inputs.projectName)
$scope.state.inflight = true
$scope.state.error = false
$scope
.renameProject(project, $scope.inputs.projectName)
.success () ->
$scope.state.inflight = false
$scope.state.error = false
$modalInstance.close()
.error (body, statusCode) ->
$scope.state.inflight = false
if statusCode == 400
$scope.state.error = { message: body }
else
$scope.state.error = true
$scope.cancel = () ->
$modalInstance.dismiss('cancel')
@ -21,6 +38,7 @@ define [
projectName: project.name + " (Copy)"
$scope.state =
inflight: false
error: false
$modalInstance.opened.then () ->
$timeout () ->
@ -31,9 +49,16 @@ define [
$scope.state.inflight = true
$scope
.cloneProject(project, $scope.inputs.projectName)
.then (project_id) ->
.success () ->
$scope.state.inflight = false
$modalInstance.close(project_id)
$scope.state.error = false
$modalInstance.close()
.error (body, statusCode) ->
$scope.state.inflight = false
if statusCode == 400
$scope.state.error = { message: body }
else
$scope.state.error = true
$scope.cancel = () ->
$modalInstance.dismiss('cancel')
@ -43,6 +68,7 @@ define [
projectName: ""
$scope.state =
inflight: false
error: false
$modalInstance.opened.then () ->
$timeout () ->
@ -51,11 +77,19 @@ define [
$scope.create = () ->
$scope.state.inflight = true
$scope.state.error = false
$scope
.createProject($scope.inputs.projectName, template)
.then (project_id) ->
.success (data) ->
$scope.state.inflight = false
$modalInstance.close(project_id)
$scope.state.error = false
$modalInstance.close(data.project_id)
.error (body, statusCode) ->
$scope.state.inflight = false
if statusCode == 400
$scope.state.error = { message: body }
else
$scope.state.error = true
$scope.cancel = () ->
$modalInstance.dismiss('cancel')

View file

@ -256,9 +256,7 @@ define [
)
$scope.createProject = (name, template = "none") ->
deferred = $q.defer()
queuedHttp
return queuedHttp
.post("/project/new", {
_csrf: window.csrfToken
projectName: name
@ -273,13 +271,7 @@ define [
# to the rest of the app
}
$scope.updateVisibleProjects()
deferred.resolve(data.project_id)
)
.error((data, status, headers, config) ->
deferred.reject()
)
return deferred.promise
$scope.openCreateProjectModal = (template = "none") ->
event_tracking.send 'project-list-page-interaction', 'new-project', template
@ -294,15 +286,13 @@ define [
modalInstance.result.then (project_id) ->
window.location = "/project/#{project_id}"
MAX_PROJECT_NAME_LENGTH = 150
$scope.renameProject = (project, newName) ->
if !newName? or newName.length == 0 or newName.length > MAX_PROJECT_NAME_LENGTH
return
project.name = newName
queuedHttp.post "/project/#{project.id}/rename", {
newProjectName: project.name
return queuedHttp.post "/project/#{project.id}/rename", {
newProjectName: newName,
_csrf: window.csrfToken
}
.success () ->
project.name = newName
$scope.openRenameProjectModal = () ->
project = $scope.getFirstSelectedProject()
@ -312,16 +302,11 @@ define [
templateUrl: "renameProjectModalTemplate"
controller: "RenameProjectModalController"
resolve:
projectName: () -> project.name
)
modalInstance.result.then(
(newName) ->
$scope.renameProject(project, newName)
project: () -> project
scope: $scope
)
$scope.cloneProject = (project, cloneName) ->
deferred = $q.defer()
event_tracking.send 'project-list-page-interaction', 'project action', 'Clone'
queuedHttp
.post("/project/#{project.id}/clone", {
@ -337,13 +322,7 @@ define [
# to the rest of the app
}
$scope.updateVisibleProjects()
deferred.resolve(data.project_id)
)
.error((data, status, headers, config) ->
deferred.reject()
)
return deferred.promise
$scope.openCloneProjectModal = () ->
project = $scope.getFirstSelectedProject()

View file

@ -19,24 +19,29 @@ define [
processPendingRequests()
queuedHttp = (args...) ->
deferred = $q.defer()
promise = deferred.promise
# We can't use Angular's $q.defer promises, because it only passes
# a single argument on error, and $http passes multiple.
promise = {}
successCallbacks = []
errorCallbacks = []
# Adhere to the $http promise conventions
promise.success = (callback) ->
promise.then(callback)
successCallbacks.push callback
return promise
promise.error = (callback) ->
promise.catch(callback)
errorCallbacks.push callback
return promise
doRequest = () ->
$http(args...)
.success (successArgs...) ->
deferred.resolve(successArgs...)
.error (errorArgs...) ->
deferred.reject(errorArgs...)
.success (args...) ->
for cb in successCallbacks
cb(args...)
.error (args...) ->
for cb in errorCallbacks
cb(args...)
pendingRequests.push doRequest
processPendingRequests()

View file

@ -547,7 +547,7 @@ describe "EditorController", ->
@err = "errro"
@window_id = "kdsjklj290jlk"
@newName = "new name here"
@ProjectDetailsHandler.renameProject = sinon.stub().callsArgWith(2, @err)
@ProjectDetailsHandler.renameProject = sinon.stub().callsArg(2)
@EditorRealTimeController.emitToRoom = sinon.stub()
it "should call the EditorController", (done)->

View file

@ -280,20 +280,12 @@ describe "ProjectController", ->
done()
@ProjectController.renameProject @req, @res
it "should send a 500 if there is a problem", (done)->
@EditorController.renameProject.callsArgWith(2, "problem")
@res.sendStatus = (code)=>
code.should.equal 500
@EditorController.renameProject.calledWith(@project_id, @newProjectName).should.equal true
it "should send an error to next() if there is a problem", (done)->
@EditorController.renameProject.callsArgWith(2, error = new Error("problem"))
next = (e)=>
e.should.equal error
done()
@ProjectController.renameProject @req, @res
it "should return an error if the name is over 150 chars", (done)->
@req.body.newProjectName = "EDMUBEEBKBXUUUZERMNSXFFWIBHGSDAWGMRIQWJBXGWSBVWSIKLFPRBYSJEKMFHTRZBHVKJSRGKTBHMJRXPHORFHAKRNPZGGYIOTEDMUBEEBKBXUUUZERMNSXFFWIBHGSDAWGMRIQWJBXGWSBVWSIKLFPRBYSJEKMFHTRZBHVKJSRGKTBHMJRXPHORFHAKRNPZGGYIOT"
@res.sendStatus = (code)=>
code.should.equal 400
done()
@ProjectController.renameProject @req, @res
@ProjectController.renameProject @req, @res, next
describe "loadEditor", ->
beforeEach ->

View file

@ -34,6 +34,8 @@ describe 'ProjectCreationHandler', ->
addDoc: sinon.stub().callsArgWith(4, null, {_id: docId})
addFile: sinon.stub().callsArg(4)
setRootDoc: sinon.stub().callsArg(2)
@ProjectDetailsHandler =
validateProjectName: sinon.stub().yields()
@user =
first_name:"first name here"
@ -48,6 +50,7 @@ describe 'ProjectCreationHandler', ->
'../../models/Project':{Project:@ProjectModel}
'../../models/Folder':{Folder:@FolderModel}
'./ProjectEntityHandler':@ProjectEntityHandler
"./ProjectDetailsHandler":@ProjectDetailsHandler
"settings-sharelatex": @Settings = {}
'logger-sharelatex': {log:->}
"metrics-sharelatex": {
@ -96,6 +99,18 @@ describe 'ProjectCreationHandler', ->
it 'should return the error to the callback', ->
should.exist @callback.args[0][0]
describe "with an invalid name", ->
beforeEach ->
@ProjectDetailsHandler.validateProjectName = sinon.stub().yields(new Error("bad name"))
@handler.createBlankProject ownerId, projectName, @callback
it 'should return the error to the callback', ->
should.exist @callback.args[0][0]
it 'should not try to create the project', ->
@ProjectModel::save.called.should.equal false
describe 'Creating a basic project', ->
beforeEach ->

View file

@ -4,6 +4,7 @@ Errors = require "../../../../app/js/Features/Errors/Errors"
SandboxedModule = require('sandboxed-module')
sinon = require('sinon')
assert = require("chai").assert
expect = require("chai").expect
require('chai').should()
describe 'ProjectDetailsHandler', ->
@ -95,6 +96,7 @@ describe 'ProjectDetailsHandler', ->
describe "renameProject", ->
beforeEach ->
@handler.validateProjectName = sinon.stub().yields()
@ProjectModel.update.callsArgWith(2)
@newName = "new name here"
@ -108,7 +110,34 @@ describe 'ProjectDetailsHandler', ->
@handler.renameProject @project_id, @newName, =>
@tpdsUpdateSender.moveEntity.calledWith({project_id:@project_id, project_name:@project.name, newProjectName:@newName}).should.equal true
done()
it "should not do anything with an invalid name", (done) ->
@handler.validateProjectName = sinon.stub().yields(new Error("invalid name"))
@handler.renameProject @project_id, @newName, =>
@tpdsUpdateSender.moveEntity.called.should.equal false
@ProjectModel.update.called.should.equal false
done()
describe "validateProjectName", ->
it "should reject empty names", (done) ->
@handler.validateProjectName "", (error) ->
expect(error).to.exist
done()
it "should reject empty names with /s", (done) ->
@handler.validateProjectName "foo/bar", (error) ->
expect(error).to.exist
done()
it "should reject long names", (done) ->
@handler.validateProjectName new Array(1000).join("a"), (error) ->
expect(error).to.exist
done()
it "should accept normal names", (done) ->
@handler.validateProjectName "foobar", (error) ->
expect(error).to.not.exist
done()
describe "setPublicAccessLevel", ->
beforeEach ->

View file

@ -80,6 +80,7 @@ describe 'ProjectEntityHandler', ->
'./ProjectUpdateHandler': @projectUpdater
"./ProjectGetter": @ProjectGetter
"settings-sharelatex":@settings
"../Cooldown/CooldownManager": @CooldownManager = {}
describe 'mkdirp', ->

View file

@ -70,7 +70,6 @@ describe 'ProjectLocator', ->
it 'should give error if element could not be found', (done)->
@locator.findElement {project_id:project._id, element_id:"ddsd432nj42", type:"docs"}, (err, foundElement, path, parentFolder)->
console.log err
err.should.deep.equal new Errors.NotFoundError("entity not found")
done()

View file

@ -11,6 +11,7 @@ describe 'ProjectUpdateHandler', ->
@ProjectModel.update = sinon.stub().callsArg(3)
@handler = SandboxedModule.require modulePath, requires:
'../../models/Project':{Project:@ProjectModel}
'logger-sharelatex' : { log: sinon.stub() }
describe 'marking a project as recently updated', ->
it 'should send an update to mongo', (done)->

View file

@ -11,6 +11,7 @@ describe "UserLocator", ->
@UserLocator = SandboxedModule.require modulePath, requires:
"../../infrastructure/mongojs": db: @db = { users: {} }
"metrics-sharelatex": timeAsyncMethod: sinon.stub()
'logger-sharelatex' : { log: sinon.stub() }
@db.users =
findOne : sinon.stub().callsArgWith(1, null, @user)

View file

@ -96,7 +96,6 @@ describe "UserRegistrationHandler", ->
it "should return email registered in the error if there is a non holdingAccount there", (done)->
@User.findOne.callsArgWith(1, null, @user = {holdingAccount:false})
@handler.registerNewUser @passingRequest, (err, user)=>
console.log err, user
err.should.deep.equal new Error("EmailAlreadyRegistered")
user.should.deep.equal @user
done()