Add captcha, this should fix #228

ivaldi · Oct 11, 2016 · 95c6822 · 95c6822
1 parent dc920d9
commit 95c6822
Show file tree

Hide file tree

Showing 9 changed files with 126 additions and 31 deletions.
diff --git a/Gemfile b/Gemfile
@@ -84,3 +84,6 @@ gem 'fancybox2-rails', '~> 0.2.8'
 
 # gravatar for user avatar images
 gem 'gravatar_image_tag'
+
+# Captcha for brimir
+gem "recaptcha", require: "recaptcha/rails"
diff --git a/Gemfile.lock b/Gemfile.lock
@@ -209,6 +209,8 @@ GEM
     rb-fsevent (0.9.7)
     rb-inotify (0.9.7)
       ffi (>= 0.5.0)
+    recaptcha (3.3.0)
+      json
     responders (2.2.0)
       railties (>= 4.2.0, < 5.1)
     sass (3.4.22)
@@ -283,6 +285,7 @@ DEPENDENCIES
   rails (~> 4.2.0)
   rails-i18n
   rake
+  recaptcha
   sass-rails (~> 5.0.0)
   select2-rails (~> 3.5)
   spring

diff --git a/README.md b/README.md
@@ -44,6 +44,13 @@ If you want to use LDAP, configure config/ldap.yml accordingly, then change the
     bin/rails console production
     u = User.new({ email: 'your@email.address', password: 'somepassword', password_confirmation: 'somepassword' }); u.agent = true; u.save!
 
+IMPORTANT Captcha Notice:
+If you want to use recaptcha in production you have to go to
+https://www.google.com/recaptcha, create your private and public keys and export these to your production environment, by running:
+
+    export RECAPTCHA_PUBLIC_KEY="[YOUR_KEY]"
+    export RECAPTCHA_PRIVATE_KEY="["YOUR_KEY"]"
+
 Updating
 --------
 First download the new code in the same directory by unpacking a release tarball or by running `git pull` (when you cloned the repo earlier). After updating code run the following commands to install necessary gem updates, migrate the database and regenerate precompiled assets.
@@ -107,7 +114,6 @@ Some users have made requests for the following features. If you would like to c
 - Desktop notifications using web notifications (#218).
 - Custom ticket statuses, all via database. (#217)
 - Filter on to/cc/bcc without verified addresses. (#227)
-- Add captcha for non-signed in ticket creation. (#228)
 - IMAP or POP3 pull mechanism for new tickets. (#249)
 - Notes field for customer account, to add info about them, such as website url.
 

diff --git a/app/controllers/tickets_controller.rb b/app/controllers/tickets_controller.rb
@@ -30,9 +30,9 @@ def show
     @agents = User.agents
 
     draft = @ticket.replies
-        .where('user_id IS NULL OR user_id = ?', current_user.id)
-        .where(draft: true)
-        .first
+      .where('user_id IS NULL OR user_id = ?', current_user.id)
+      .where(draft: true)
+      .first
 
     @replies = @ticket.replies.chronologically.without_drafts.select do |reply|
       can? :show, reply
@@ -55,9 +55,9 @@ def show
       format.eml do
         begin
           send_file @ticket.raw_message.path(:original),
-              filename: "ticket-#{@ticket.id}.eml",
-              type: 'text/plain',
-              disposition: :attachment
+            filename: "ticket-#{@ticket.id}.eml",
+            type: 'text/plain',
+            disposition: :attachment
         rescue
           raise ActiveRecord::RecordNotFound
         end
@@ -84,7 +84,7 @@ def index
     respond_to do |format|
       format.html do
         @tickets = @tickets.paginate(page: params[:page],
-            per_page: current_user.per_page)
+                                     per_page: current_user.per_page)
       end
       format.csv do
         @tickets = @tickets.includes(:status_changes)
@@ -159,26 +159,23 @@ def create
       @ticket = Ticket.new(ticket_params)
     end
 
-    if !@ticket.nil? && @ticket.save
-      NotificationMailer.incoming_message(@ticket, params[:message])
+    if Ticket.recaptcha_keys_present? && verify_recaptcha
+      ticket_save_and_sent_notification
+    elsif !Ticket.recaptcha_keys_present?
+      ticket_save_and_sent_notification
     end
 
     respond_to do |format|
       format.html do
 
-        if !@ticket.nil? && @ticket.valid?
-
-          if current_user.nil?
-            render 'create'
-          else
-            redirect_to ticket_url(@ticket), notice: I18n::translate(:ticket_added)
-          end
-
+        # if recpatcha is enabled, set conditionals
+        if Ticket.recaptcha_keys_present? && verify_recaptcha 
+          ticket_respond_to_html
+        elsif !Ticket.recaptcha_keys_present?
+          ticket_respond_to_html
         else
-          @email_addresses = EmailAddress.verified.ordered
-          render 'new'
+          render_new_with_email_addresses
         end
-
       end
 
       format.json do
@@ -193,5 +190,30 @@ def create
 
       format.js { render }
     end
+
+  end
+
+  private
+  def ticket_respond_to_html
+    if !@ticket.nil? && @ticket.valid?
+      if current_user.nil?
+        render 'create'
+      else
+        redirect_to ticket_url(@ticket), notice: I18n::translate(:ticket_added)
+      end
+    else
+      render_new_with_email_addresses
+    end
+  end
+
+  def render_new_with_email_addresses
+    @email_addresses = EmailAddress.verified.ordered
+    render 'new'
+  end 
+
+  def ticket_save_and_sent_notification
+    if !@ticket.nil? && @ticket.save
+      NotificationMailer.incoming_message(@ticket, params[:message])
+    end
   end
 end
diff --git a/app/models/ticket.rb b/app/models/ticket.rb
@@ -164,6 +164,15 @@ def to
     to_email_address.try :email
   end
 
+  def self.recaptcha_keys_present?
+    if Recaptcha.configuration.public_key.blank? &&
+        Recaptcha.configuration.private_key.blank?
+      false
+    else
+      true
+    end
+  end
+
   protected
     def create_status_change
       status_changes.create! status: self.status

diff --git a/app/views/tickets/_form.html.erb b/app/views/tickets/_form.html.erb
@@ -54,6 +54,12 @@
 
   <%= render 'attachments/form', f: f %>
 
+  <% if Ticket.recaptcha_keys_present? %>
+    <p>
+      <%= recaptcha_tags %>
+    </p>
+  <% end %>
+
   <p>
     <%= f.submit class: 'button regular radius' %>
   </p>

diff --git a/config/initializers/recaptcha.rb b/config/initializers/recaptcha.rb
@@ -0,0 +1,14 @@
+# these will only work locally 
+# Create your own at https://www.google.com/recaptcha
+
+if Rails.env == "production"
+  Recaptcha.configure do |config|
+    config.public_key   = Rails.application.secrets[:recaptcha_public_key]
+    config.private_key  = Rails.application.secrets[:recaptcha_private_key]
+  end
+else
+  Recaptcha.configure do |config|
+    config.public_key   = Rails.application.secrets[:recaptcha_public_key]
+    config.private_key  = Rails.application.secrets[:recaptcha_private_key]
+  end
+end
diff --git a/config/secrets.yml b/config/secrets.yml
@@ -14,15 +14,21 @@ development:
   secret_key_base: 2db2964f8fcc1eaebf8db0116a54889c81ea3e7baa83cca54c67c25c6d1b1901a60093e2b9189a40c979fc8b85bf1295c6b2a3e86fbdbe2464f5cf02fd306007
   google_client_id: 1032949297619-5nqftn8e540bnohtqjiv3pg2qvg7uvtj.apps.googleusercontent.com
   google_client_secret: 1L43_SExBR35OHLi4jnLOkzr
+  recaptcha_public_key: 6LeIxAcTAAAAAJcZVRqyHh71UMIEGNQ_MXjiZKhI
+  recaptcha_private_key: 6LeIxAcTAAAAAGG-vFI1TnRWxMZNFuojJ4WifJWe
 
 test:
   secret_key_base: e1dd1f2fa691750f01e19de0ab7066263251d06a3c4a33ab5dfa6ab0500cd4911ab9e53c118587e91ccd9577dacb13b00b70692c79fc3818ee09a9395d372b3c
   google_client_id:
   google_client_secret:
+  recaptcha_public_key: 6LeIxAcTAAAAAJcZVRqyHh71UMIEGNQ_MXjiZKhI
+  recaptcha_private_key: 6LeIxAcTAAAAAGG-vFI1TnRWxMZNFuojJ4WifJWe
 
 # Do not keep production secrets in the repository,
 # instead read values from the environment.
 production:
   secret_key_base: <%= ENV["SECRET_KEY_BASE"] %>
   google_client_id:
   google_client_secret:
+  recaptcha_public_key:  <%= ENV["RECAPTCHA_PUBLIC_KEY"] %>
+  recaptcha_private_key: <%= ENV["RECAPTCHA_PRIVATE_KEY"] %>
diff --git a/test/controllers/tickets_controller_test.rb b/test/controllers/tickets_controller_test.rb
@@ -76,9 +76,9 @@ class TicketsControllerTest < ActionController::TestCase
     assert_no_difference 'ActionMailer::Base.deliveries.size' do
       assert_no_difference 'Ticket.count' do
         post :create, ticket: {
-            from: 'invalid',
-            content: '',
-            subject: '',
+          from: 'invalid',
+          content: '',
+          subject: '',
         }
 
         assert_response :success
@@ -93,9 +93,9 @@ class TicketsControllerTest < ActionController::TestCase
     assert_difference 'ActionMailer::Base.deliveries.size', User.agents.count do
       assert_difference 'Ticket.count' do
         post :create, ticket: {
-            from: 'test@test.nl',
-            content: @ticket.content,
-            subject: @ticket.subject,
+          from: 'test@test.nl',
+          content: @ticket.content,
+          subject: @ticket.subject,
         }
 
         assert_response :success
@@ -111,9 +111,9 @@ class TicketsControllerTest < ActionController::TestCase
     assert_difference 'ActionMailer::Base.deliveries.size', User.agents.count do
       assert_difference 'Ticket.count', 1 do
         post :create, ticket: {
-            from: 'test@test.nl',
-            content: @ticket.content,
-            subject: @ticket.subject,
+          from: 'test@test.nl',
+          content: @ticket.content,
+          subject: @ticket.subject,
         }
 
         assert_redirected_to ticket_url(assigns(:ticket))
@@ -166,7 +166,7 @@ class TicketsControllerTest < ActionController::TestCase
 
     # should have selected same outgoing address as original received
     assert_select 'option[selected="selected"]' +
-        "[value=\"#{email_addresses(:brimir).id}\"]"
+      "[value=\"#{email_addresses(:brimir).id}\"]"
 
     # should contain this for internal note switch
     assert_select '[data-notified-users]'
@@ -320,6 +320,32 @@ class TicketsControllerTest < ActionController::TestCase
     end
   end
 
+  test 'shown captcha should not break new ticket page when not signed in' do
+    # Stub keys Recaptcha
+    Recaptcha.configuration.public_key = '6LeIxAcTAAAAAJcZVRqyHh71UMIEGNQ_MXjiZKhI'
+    Recaptcha.configuration.private_key = '6LeIxAcTAAAAAGG-vFI1TnRWxMZNFuojJ4WifJWe'
+
+    if Ticket.recaptcha_keys_present?
+
+      get :new
+
+      assert_response :success
+    end
+  end
+
+  test 'not shown captcha should not break new ticket page when not signed' do
+    # Stub keys Recaptcha
+    Recaptcha.configuration.public_key = ''
+    Recaptcha.configuration.private_key = ''
+
+    if !(Ticket.recaptcha_keys_present?)
+
+      get :new
+
+      assert_response :success
+    end
+  end
+
   test 'should get new ticket form in correct language' do
     I18n.locale = :nl
     get :new