From 471c61fd69c98c1f51f2aa03feb861ef9146ac33 Mon Sep 17 00:00:00 2001 From: Neil Lalonde Date: Wed, 6 Feb 2013 19:25:21 -0500 Subject: [PATCH 1/2] Add honeypot and challenge to signup form --- .../discourse/models/user.js.coffee | 4 +- .../modal/create_account.js.handlebars | 8 ++++ .../views/modal/create_account_view.js.coffee | 15 ++++++- .../stylesheets/application/modal.css.scss | 3 ++ app/controllers/users_controller.rb | 18 ++++++++ config/routes.rb | 1 + spec/controllers/users_controller_spec.rb | 41 ++++++++++++++++++- 7 files changed, 86 insertions(+), 4 deletions(-) diff --git a/app/assets/javascripts/discourse/models/user.js.coffee b/app/assets/javascripts/discourse/models/user.js.coffee index 681ac2fc91c..b90c1ea3aac 100644 --- a/app/assets/javascripts/discourse/models/user.js.coffee +++ b/app/assets/javascripts/discourse/models/user.js.coffee @@ -190,9 +190,9 @@ window.Discourse.User.reopenClass error: (xhr) -> promise.reject(xhr) promise - createAccount: (name, email, password, username) -> + createAccount: (name, email, password, username, passwordConfirm, challenge) -> $.ajax url: '/users' dataType: 'json' - data: {name: name, email: email, password: password, username: username} + data: {name: name, email: email, password: password, username: username, password_confirmation: passwordConfirm, challenge: challenge} type: 'POST' diff --git a/app/assets/javascripts/discourse/templates/modal/create_account.js.handlebars b/app/assets/javascripts/discourse/templates/modal/create_account.js.handlebars index 737db276fa0..71735338033 100644 --- a/app/assets/javascripts/discourse/templates/modal/create_account.js.handlebars +++ b/app/assets/javascripts/discourse/templates/modal/create_account.js.handlebars @@ -49,6 +49,14 @@ {{/if}} + + + + {{view Ember.TextField valueBinding="view.accountPasswordConfirm" type="password" id="new-account-password-confirmation"}} + {{view Ember.TextField valueBinding="view.accountChallenge" id="new-account-challenge"}} + + + diff --git a/app/assets/javascripts/discourse/views/modal/create_account_view.js.coffee b/app/assets/javascripts/discourse/views/modal/create_account_view.js.coffee index 7f6d5098dd0..a0f17695609 100644 --- a/app/assets/javascripts/discourse/views/modal/create_account_view.js.coffee +++ b/app/assets/javascripts/discourse/views/modal/create_account_view.js.coffee @@ -3,6 +3,8 @@ window.Discourse.CreateAccountView = window.Discourse.ModalBodyView.extend Disco title: Em.String.i18n('create_account.title') uniqueUsernameValidation: null complete: false + accountPasswordConfirm: 0 + accountChallenge: 0 submitDisabled: (-> @@ -22,6 +24,8 @@ window.Discourse.CreateAccountView = window.Discourse.ModalBodyView.extend Disco # If blank, fail without a reason return Discourse.InputValidation.create(failed: true) if @blank('accountName') + @fetchConfirmationValue() if @get('accountPasswordConfirm') == 0 + # If too short return Discourse.InputValidation.create(failed: true, reason: Em.String.i18n('user.name.too_short')) if @get('accountName').length < 3 @@ -120,13 +124,22 @@ window.Discourse.CreateAccountView = window.Discourse.ModalBodyView.extend Disco ).property('accountPassword') + fetchConfirmationValue: -> + $.ajax + url: '/users/hp.json', + success: (json) => + @set('accountPasswordConfirm', json.value) + @set('accountChallenge', json.challenge.split("").reverse().join("")) + createAccount: -> name = @get('accountName') email = @get('accountEmail') password = @get('accountPassword') username = @get('accountUsername') + passwordConfirm = @get('accountPasswordConfirm') + challenge = @get('accountChallenge') - Discourse.User.createAccount(name, email, password, username).then (result) => + Discourse.User.createAccount(name, email, password, username, passwordConfirm, challenge).then (result) => if result.success @flash(result.message) diff --git a/app/assets/stylesheets/application/modal.css.scss b/app/assets/stylesheets/application/modal.css.scss index 2f628966200..7361893f99a 100644 --- a/app/assets/stylesheets/application/modal.css.scss +++ b/app/assets/stylesheets/application/modal.css.scss @@ -152,6 +152,9 @@ margin-bottom: 20px; } } + .password-confirmation { + display: none; + } } #move-selected { diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index 877470553c5..82138519c5f 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -123,6 +123,12 @@ class UsersController < ApplicationController end def create + + if params[:password_confirmation] != honeypot_value or params[:challenge] != challenge_value.try(:reverse) + # Don't give any indication that we caught you in the honeypot + return render(:json => {success: true, active: false, message: I18n.t("login.activate_email", email: params[:email]) }) + end + user = User.new user.name = params[:name] user.email = params[:email] @@ -183,6 +189,10 @@ class UsersController < ApplicationController render json: {errors: [I18n.t("mothership.access_token_problem")]} end + def get_honeypot_value + render json: {value: honeypot_value, challenge: challenge_value} + end + # all avatars are funneled through here def avatar @@ -320,6 +330,14 @@ class UsersController < ApplicationController private + def honeypot_value + Digest::SHA1::hexdigest("#{Discourse.current_hostname}:#{Discourse::Application.config.secret_token}")[0,15] + end + + def challenge_value + '3019774c067cc2b' + end + def fetch_user_from_params username_lower = params[:username].downcase username_lower.gsub!(/\.json$/, '') diff --git a/config/routes.rb b/config/routes.rb index 3331be7f080..ce4e76db22b 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -80,6 +80,7 @@ Discourse::Application.routes.draw do put 'users/password-reset/:token' => 'users#password_reset' get 'users/activate-account/:token' => 'users#activate_account' get 'users/authorize-email/:token' => 'users#authorize_email' + get 'users/hp' => 'users#get_honeypot_value' get 'user_preferences' => 'users#user_preferences_redirect' get 'users/:username/private-messages' => 'user_actions#private_messages', :format => false, :constraints => {:username => USERNAME_ROUTE_FORMAT} diff --git a/spec/controllers/users_controller_spec.rb b/spec/controllers/users_controller_spec.rb index ac076be5ef1..b67f6f7af2e 100644 --- a/spec/controllers/users_controller_spec.rb +++ b/spec/controllers/users_controller_spec.rb @@ -2,6 +2,11 @@ require 'spec_helper' describe UsersController do + before do + UsersController.any_instance.stubs(:honeypot_value).returns(nil) + UsersController.any_instance.stubs(:challenge_value).returns(nil) + end + describe '.show' do let!(:user) { log_in } @@ -339,7 +344,41 @@ describe UsersController do User.where(username: @user.username).first.active.should be_false end end - + + shared_examples_for 'honeypot fails' do + it 'should not create a new user' do + expect { + xhr :post, :create, create_params + }.to_not change { User.count } + end + + it 'should not send an email' do + User.any_instance.expects(:enqueue_welcome_message).never + xhr :post, :create, create_params + end + + it 'should say it was successful' do + xhr :post, :create, create_params + json = JSON::parse(response.body) + json["success"].should be_true + end + end + + context 'when honeypot value is wrong' do + before do + UsersController.any_instance.stubs(:honeypot_value).returns('abc') + end + let(:create_params) { {:name => @user.name, :username => @user.username, :password => "strongpassword", :email => @user.email, :password_confirmation => 'wrong'} } + it_should_behave_like 'honeypot fails' + end + + context 'when challenge answer is wrong' do + before do + UsersController.any_instance.stubs(:challenge_value).returns('abc') + end + let(:create_params) { {:name => @user.name, :username => @user.username, :password => "strongpassword", :email => @user.email, :challenge => 'abc'} } + it_should_behave_like 'honeypot fails' + end end context '.username' do From 1c29b040c5b92157fdc1a601f8a2567967d08e64 Mon Sep 17 00:00:00 2001 From: Aaron Chambers Date: Thu, 7 Feb 2013 00:46:57 +0000 Subject: [PATCH 2/2] A tiny typo fix. It all adds up... --- lib/site_setting_extension.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/site_setting_extension.rb b/lib/site_setting_extension.rb index 8a1bd859997..89e49d9aebd 100644 --- a/lib/site_setting_extension.rb +++ b/lib/site_setting_extension.rb @@ -31,8 +31,8 @@ module SiteSettingExtension end # just like a setting, except that it is available in javascript via DiscourseSession - def client_setting(name, defualt = nil, type = nil) - setting(name,defualt,type) + def client_setting(name, default = nil, type = nil) + setting(name,default,type) @@client_settings ||= [] @@client_settings << name end