From 94210fc461471379908655ae0b97997a3a26e446 Mon Sep 17 00:00:00 2001 From: Ruben Vereecken Date: Thu, 10 Jul 2014 18:00:32 +0200 Subject: [PATCH] Anonymous users are now silently renamed upon signup in case of conflict --- server/users/User.coffee | 41 +++++++++++++++----- server/users/user_handler.coffee | 3 +- test/server/functional/user.spec.coffee | 51 +++++++++++++++++-------- 3 files changed, 68 insertions(+), 27 deletions(-) diff --git a/server/users/User.coffee b/server/users/User.coffee index fc7440ef3..13f4c11fb 100644 --- a/server/users/User.coffee +++ b/server/users/User.coffee @@ -30,6 +30,9 @@ UserSchema.methods.isAdmin = -> p = @get('permissions') return p and 'admin' in p +UserSchema.methods.isAnonymous = -> + @get 'anonymous' + UserSchema.methods.trackActivity = (activityName, increment) -> now = new Date() increment ?= parseInt increment or 1 @@ -109,6 +112,30 @@ UserSchema.statics.updateMailChimp = (doc, callback) -> mc?.lists.subscribe params, onSuccess, onFailure +UserSchema.statics.unconflictName = unconflictName = (name, done) -> + User.findOne {slug: _.str.slugify(name)}, (err, otherUser) -> + return done err if err? + return done null, name unless otherUser + suffix = _.random(0, 9) + '' + unconflictName name + suffix, done + +UserSchema.methods.register = (done) -> + @set('anonymous', false) + @set('permissions', ['admin']) if not isProduction + if (name = @get 'name')? and name isnt '' + unconflictName name, (err, uniqueName) => + return done err if err + @set 'name', uniqueName + done() + else done() + data = + email_id: sendwithus.templates.welcome_email + recipient: + address: @get 'email' + sendwithus.api.send data, (err, result) -> + log.error "sendwithus post-save error: #{err}, result: #{result}" if err + + UserSchema.pre('save', (next) -> @set('emailLower', @get('email')?.toLowerCase()) @set('nameLower', @get('name')?.toLowerCase()) @@ -116,16 +143,10 @@ UserSchema.pre('save', (next) -> if @get('password') @set('passwordHash', User.hashPassword(pwd)) @set('password', undefined) - if @get('email') and @get('anonymous') - @set('anonymous', false) - @set('permissions', ['admin']) if not isProduction - data = - email_id: sendwithus.templates.welcome_email - recipient: - address: @get 'email' - sendwithus.api.send data, (err, result) -> - log.error "sendwithus post-save error: #{err}, result: #{result}" if err - next() + if @get('email') and @get('anonymous') # a user registers + @register next + else + next() ) UserSchema.post 'save', (doc) -> diff --git a/server/users/user_handler.coffee b/server/users/user_handler.coffee index 45b0253f0..c145855b9 100644 --- a/server/users/user_handler.coffee +++ b/server/users/user_handler.coffee @@ -104,7 +104,8 @@ UserHandler = class UserHandler extends Handler return callback(null, req, user) unless req.body.name nameLower = req.body.name?.toLowerCase() return callback(null, req, user) unless nameLower - return callback(null, req, user) if nameLower is user.get('nameLower') and not user.get('anonymous') + return callback(null, req, user) if user.get 'anonymous' # anonymous users can have any name + return callback(null, req, user) if nameLower is user.get('nameLower') User.findOne({nameLower: nameLower, anonymous: false}).exec (err, otherUser) -> log.error "Database error setting user name: #{err}" if err return callback(res: 'Database error.', code: 500) if err diff --git a/test/server/functional/user.spec.coffee b/test/server/functional/user.spec.coffee index b60b944f4..e8bfb2b51 100644 --- a/test/server/functional/user.spec.coffee +++ b/test/server/functional/user.spec.coffee @@ -110,24 +110,13 @@ describe 'POST /db/user', -> it 'should allow multiple anonymous users with same name', (done) -> createAnonNameUser('Jim', done) - it 'should not allow setting existing user name to anonymous user', (done) -> - - createAnonUser = -> - request.post getURL('/auth/logout'), -> - request.get getURL('/auth/whoami'), -> - req = request.post(getURL('/db/user'), (err, response) -> - expect(response.statusCode).toBe(409) - done() - ) - form = req.form() - form.append('name', 'Jim') - + it 'should allow setting existing user name to anonymous user', (done) -> req = request.post(getURL('/db/user'), (err, response, body) -> expect(response.statusCode).toBe(200) request.get getURL('/auth/whoami'), (request, response, body) -> res = JSON.parse(response.body) expect(res.anonymous).toBeFalsy() - createAnonUser() + createAnonNameUser 'Jim', done ) form = req.form() form.append('email', 'new@user.com') @@ -212,6 +201,39 @@ ghlfarghlarghlfarghlarghlfarghlarghlfarghlarghlfarghlarghlfarghlarghlfarghlarghl form.append('email', 'New@email.com') form.append('name', 'Wilhelm') + it 'should not allow two users with the same name slug', (done) -> + loginSam (sam) -> + samsName = sam.get 'name' + sam.set 'name', 'admin' + request.put {uri:getURL(urlUser + '/' + sam.id), json: sam.toObject()}, (err, response) -> + expect(err).toBeNull() + expect(response.statusCode).toBe 409 + + sam.set 'name', samsName + done() + + it 'should silently rename an anonymous user if their name conflicts upon signup', (done) -> + request.post getURL('/auth/logout'), -> + request.get getURL('/auth/whoami'), -> + req = request.post getURL('/db/user'), (err, response) -> + expect(response.statusCode).toBe(200) + request.get getURL('/auth/whoami'), (err, response) -> + expect(err).toBeNull() + guy = JSON.parse(response.body) + expect(guy.anonymous).toBeTruthy() + expect(guy.name).toEqual 'admin' + + guy.email = 'blub@blub' # Email means registration + req = request.post {url: getURL('/db/user'), json: guy}, (err, response) -> + expect(err).toBeNull() + finalGuy = response.body + expect(finalGuy.anonymous).toBeFalsy() + expect(finalGuy.name).not.toEqual guy.name + expect(finalGuy.name.length).toBe guy.name.length + 1 + done() + form = req.form() + form.append('name', 'admin') + describe 'GET /db/user', -> it 'logs in as admin', (done) -> @@ -293,6 +315,3 @@ describe 'GET /db/user', -> - - -