Merge branch 'api_captcha_with_frontend_console_feedback' into api

Evandro Junior
2 parents af9a4c2b 88a604ed
Showing 4 changed files with 120 additions and 107 deletions Show diff stats
lib/noosfero/api/helpers.rb
lib/noosfero/api/session.rb
test/unit/api/helpers_test.rb
test/unit/api/session_test.rb
+require 'grape'
+
   module Noosfero;
     module API
@@ -37,6 +39,10 @@
         @environment
       end
+      def logger
+        Noosfero::API::API.logger
+      end
+
       def limit
         limit = params[:limit].to_i
         limit = default_limit if limit <= 0
@@ -202,20 +208,6 @@
         attrs
       end
-      def verify_recaptcha_v2(remote_ip, g_recaptcha_response, private_key, api_recaptcha_verify_uri)
-        verify_hash = {
-          "secret"    => private_key,
-          "remoteip"  => remote_ip,
-          "response"  => g_recaptcha_response
-        }
-        uri = URI(api_recaptcha_verify_uri)
-        https = Net::HTTP.new(uri.host, uri.port)
-        https.use_ssl = true
-        request = Net::HTTP::Post.new(uri.path)
-        request.set_form_data(verify_hash)
-        JSON.parse(https.request(request).body)
-      end
-
       ##########################################
       #              error helpers             #
       ##########################################
@@ -251,8 +243,19 @@
         render_api_error!(_('Method Not Allowed'), 405)
       end
-      def render_api_error!(message, status)
-        error!({'message' => message, :code => status}, status)
+      # javascript_console_message is supposed to be executed as console.log()
+      def render_api_error!(user_message, status, log_message = nil, javascript_console_message = nil)
+        message_hash = {'message' => user_message, :code => status}
+        message_hash[:javascript_console_message] = javascript_console_message if javascript_console_message.present?
+        log_msg = "#{status}, User message: #{user_message}"
+        log_msg = "#{log_message}, #{log_msg}" if log_message.present?
+        log_msg = "#{log_msg}, Javascript Console Message: #{javascript_console_message}" if javascript_console_message.present?
+        logger.error log_msg
+        if javascript_console_message.present?
+          error!(message_hash, status)
+        else
+          error!(user_message, status)
+        end
       end
       def render_api_errors!(messages)
@@ -319,10 +322,13 @@
       def test_captcha(remote_ip, params, environment)
         d = environment.api_captcha_settings
         return true unless d[:enabled] == true
+        msg_icve = _('Internal captcha validation error')
+        msg_eacs = 'Environment api_captcha_settings'
+        s = 500
         if d[:provider] == 'google'
-          raise ArgumentError, "Environment api_captcha_settings private_key not defined" if d[:private_key].nil?
-          raise ArgumentError, "Environment api_captcha_settings version not defined" unless d[:version] == 1 || d[:version] == 2
+          return render_api_error!(msg_icve, s, nil, "#{msg_eacs} private_key not defined") if d[:private_key].nil?
+          return render_api_error!(msg_icve, s, nil, "#{msg_eacs} version not defined") unless d[:version] == 1 || d[:version] == 2
           if d[:version]  == 1
             d[:verify_uri] ||= 'https://www.google.com/recaptcha/api/verify'
             return verify_recaptcha_v1(remote_ip, d[:private_key], d[:verify_uri], params[:recaptcha_challenge_field], params[:recaptcha_response_field])
@@ -333,15 +339,15 @@
           end
         end
         if d[:provider] == 'serpro'
-          raise ArgumentError, "Environment api_captcha_settings verify_uri not defined" if d[:verify_uri].nil?
+          return render_api_error!(msg_icve, s, nil, "#{msg_eacs} verify_uri not defined") if d[:verify_uri].nil?
           return verify_serpro_captcha(d[:serpro_client_id], params[:txtToken_captcha_serpro_gov_br], params[:captcha_text], d[:verify_uri])
         end
-        raise ArgumentError, "Environment api_captcha_settings provider not defined"
+        return render_api_error!(msg_icve, s, nil, "#{msg_eacs} provider not defined")
       end
       def verify_recaptcha_v1(remote_ip, private_key, api_recaptcha_verify_uri, recaptcha_challenge_field, recaptcha_response_field)
         if recaptcha_challenge_field == nil || recaptcha_response_field == nil
-          return _('Missing captcha data')
+          return render_api_error!(_('Captcha validation error'), 500, nil, _('Missing captcha data'))
         end
         verify_hash = {
@@ -358,19 +364,14 @@
         begin
           body = https.request(request).body
         rescue Exception => e
-          logger = Logger.new(File.join(Rails.root, 'log', "#{ENV['RAILS_ENV'] || 'production'}_api.log"))
-          logger.error e
-          return _("Google recaptcha error: #{e.message}")
+          return render_api_error!(_('Internal captcha validation error'), 500, nil, "recaptcha error: #{e.message}")
         end
         body = JSON.parse(body)
         body == "true\nsuccess" ? true : body
       end
       def verify_recaptcha_v2(remote_ip, private_key, api_recaptcha_verify_uri, g_recaptcha_response)
-        if g_recaptcha_response == nil
-          return _('Missing captcha data')
-        end
-
+        return render_api_error!(_('Captcha validation error'), 500, nil, _('Missing captcha data')) if g_recaptcha_response == nil
         verify_hash = {
             "secret"    => private_key,
             "remoteip"  => remote_ip,
@@ -384,17 +385,15 @@
         begin
           body = https.request(request).body
         rescue Exception => e
-          logger = Logger.new(File.join(Rails.root, 'log', "#{ENV['RAILS_ENV'] || 'production'}_api.log"))
-          logger.error e
-          return _("Google recaptcha error: #{e.message}")
+          return render_api_error!(_('Internal captcha validation error'), 500, nil, "recaptcha error: #{e.message}")
         end
         captcha_result = JSON.parse(body)
         captcha_result["success"] ? true : captcha_result
       end
       def verify_serpro_captcha(client_id, token, captcha_text, verify_uri)
-        return _('Missing Serpro Captcha token') if token == nil
-        return _('Captcha text has not been filled') if captcha_text == nil
+        return render_api_error!(_("Error processing token validation"), 500, nil, "Missing Serpro's Captcha token") unless token
+        return render_api_error!(_('Captcha text has not been filled'), 403) unless captcha_text
         uri = URI(verify_uri)
         http = Net::HTTP.new(uri.host, uri.port)
         request = Net::HTTP::Post.new(uri.path)
@@ -403,13 +402,15 @@
         begin
           body = http.request(request).body
         rescue Exception => e
-          logger = Logger.new(File.join(Rails.root, 'log', "#{ENV['RAILS_ENV'] || 'production'}_api.log"))
-          logger.error e
-          return _("Serpro captcha error: #{e.message}")
+          return render_api_error!(_('Internal captcha validation error'), 500, nil, "Serpro captcha error: #{e.message}")
         end
-        return _("Wrong captcha text, please try again") if body == 0
-        return _("Token not found") if body == 2
-        body == '1' ? true : body
+        return true if body == '1'
+        return render_api_error!(_("Internal captcha validation error"), 500, body, "Unable to reach Serpro's Captcha validation service") if body == "Activity timed out"
+        return render_api_error!(_("Wrong captcha text, please try again"), 403) if body == 0
+        return render_api_error!(_("Serpro's captcha token not found"), 500) if body == 2
+        return render_api_error!(_("No data sent to validation server or other serious problem"), 500) if body == -1
+        #Catches all errors at the end
+        return render_api_error!(_("Internal captcha validation error"), 500, nil, "Error validating Serpro's captcha #{body}")
       end
     end
@@ -2,7 +2,6 @@ require &quot;uri&quot;
 module Noosfero
   module API
-
     class Session < Grape::API
       # Login to get token
@@ -40,13 +39,9 @@ module Noosfero
         unique_attributes! User, [:email, :login]
         attrs = attributes_for_keys [:email, :login, :password, :password_confirmation] + environment.signup_person_fields
         remote_ip = (request.respond_to?(:remote_ip) && request.remote_ip) || (env && env['REMOTE_ADDR'])
-
-        result = test_captcha(remote_ip, params, environment)
-        unless result == true
-          render_api_error!(result, 401)
-          return
-        end
-
+        # test_captcha will render_api_error! and exit in case of some problem
+        # this return is only improve the clarity of the execution path
+        return unless test_captcha(remote_ip, params, environment)
         user = User.new(attrs)
         if user.save
           user.generate_private_token! if user.activated?
@@ -113,7 +113,6 @@ class APIHelpersTest &lt; ActiveSupport::TestCase
     p = fast_create(Profile)
     a = fast_create(Article, :published => false, :profile_id => p.id)
     fast_create(Article, :profile_id => p.id)
-
     user.generate_private_token!
     User.expects(:find_by_private_token).returns(user)
     assert_equal 403, find_article(p.articles, a.id).last
@@ -162,57 +161,6 @@ class APIHelpersTest &lt; ActiveSupport::TestCase
     assert_nil make_conditions_with_parameter[:type]
   end
-  should 'do not test captcha when there are no settings' do
-    environment = Environment.new
-    assert test_captcha("127.0.0.1", {}, environment)
-  end
-
-  should 'do not test captcha when captcha is disabled on settings' do
-    environment = Environment.new
-    environment.api_captcha_settings = {
-        enabled: false,
-    }
-    assert test_captcha("127.0.0.1", {}, environment)
-  end
-
-  should 'fail display recaptcha v1' do
-    environment = Environment.new
-    environment.api_captcha_settings = {
-        enabled: true,
-        provider: 'google',
-        version:  1,
-        private_key:  '6LdsWAcTAAAAAB6maB_HalVyCc4asDAxPxloIMvY',
-        public_key:   '6LdsWAcTAAAAAChTUUD6yu9fCDhdIZzNd7F53zf-',
-        verify_uri:   'https://www.google.com/recaptcha/api/verify',
-    }
-    assert_equal test_captcha("127.0.0.1", {}, environment), "Missing captcha data"
-  end
-
-  should 'fail display recaptcha v2' do
-    environment = Environment.new
-    environment.api_captcha_settings = {
-        enabled: true,
-        provider: 'google',
-        version:  2,
-        private_key:  '6LdsWAcTAAAAAB6maB_HalVyCc4asDAxPxloIMvY',
-        public_key:   '6LdsWAcTAAAAAChTUUD6yu9fCDhdIZzNd7F53zf-',
-        verify_uri:   'https://www.google.com/recaptcha/api/siteverify',
-    }
-    assert_equal test_captcha("127.0.0.1", {}, environment), "Missing captcha data"
-  end
-
-  should 'fail display Serpro captcha' do
-    environment = Environment.new
-    environment.api_captcha_settings = {
-        enabled: true,
-        provider: 'serpro',
-        serpro_client_id:  '0000000000000000',
-        verify_uri:   'http://localhost/api/verify',
-    }
-    params = {}
-    params[:txtToken_captcha_serpro_gov_br] = '4324343'
-    assert_equal test_captcha("127.0.0.1", params, environment), _('Captcha text has not been filled')
-  end
   should 'render not_found if endpoint is unavailable' do
     Noosfero::API::API.stubs(:endpoint_unavailable?).returns(true)
@@ -232,11 +180,79 @@ class APIHelpersTest &lt; ActiveSupport::TestCase
     #assert_equal 403, find_article(p.articles, a.id).last
     #assert_equals [article1, article2], present_articles
+  end
+###### Captcha tests ######
-  end
+should 'do not test captcha when there are no settings' do
+  environment = Environment.new
+  assert test_captcha("127.0.0.1", {}, environment)
+end
+
+should 'do not test captcha when captcha is disabled on settings' do
+  environment = Environment.new
+  environment.api_captcha_settings = {
+      enabled: false,
+  }
+  assert test_captcha("127.0.0.1", {}, environment)
+end
+
+should 'fail display recaptcha v1' do
+  environment = Environment.new
+  environment.api_captcha_settings = {
+      enabled: true,
+      provider: 'google',
+      version:  1,
+      private_key:  '6LdsWAcTAAAAAB6maB_HalVyCc4asDAxPxloIMvY',
+      public_key:   '6LdsWAcTAAAAAChTUUD6yu9fCDhdIZzNd7F53zf-',
+      verify_uri:   'https://www.google.com/recaptcha/api/verify',
+  }
+  r = test_captcha('127.0.0.1', params, environment)
+  assert_equal(_("Missing captcha data"), r[0][:javascript_console_message])
+end
-  should 'captcha serpro say name or service not known' do
+should 'fail display recaptcha v2' do
+  environment = Environment.new
+  environment.api_captcha_settings = {
+      enabled: true,
+      provider: 'google',
+      version:  2,
+      private_key:  '6LdsWAcTAAAAAB6maB_HalVyCc4asDAxPxloIMvY',
+      public_key:   '6LdsWAcTAAAAAChTUUD6yu9fCDhdIZzNd7F53zf-',
+      verify_uri:   'https://www.google.com/recaptcha/api/siteverify',
+  }
+  r = test_captcha('127.0.0.1', params, environment)
+  assert_equal(_("Missing captcha data"), r[0][:javascript_console_message])
+end
+
+should 'verify if user filled Serpro\' captcha text' do
+  environment = Environment.new
+  environment.api_captcha_settings = {
+      enabled: true,
+      provider: 'serpro',
+      serpro_client_id:  '0000000000000000',
+      verify_uri:   'http://localhost/api/verify',
+  }
+  params = {}
+  params[:txtToken_captcha_serpro_gov_br] = '4324343'
+  assert_equal(_('Captcha text has not been filled'), test_captcha('127.0.0.1', params, environment)[0])
+end
+
+should 'verify if Serpro\' captcha token has been sent' do
+  environment = Environment.new
+  environment.api_captcha_settings = {
+      enabled: true,
+      provider: 'serpro',
+      serpro_client_id:  '0000000000000000',
+      verify_uri:   'http://localhost/api/verify',
+  }
+  params = {}
+  params[:captcha_text] = '4324343'
+  r = test_captcha('127.0.0.1', params, environment)
+  assert_equal(_("Missing Serpro's Captcha token"), r[0][:javascript_console_message])
+end
+
+should 'captcha serpro say name or service not known' do
     environment = Environment.new
     environment.api_captcha_settings = {
         enabled: true,
@@ -247,11 +263,11 @@ class APIHelpersTest &lt; ActiveSupport::TestCase
     params = {}
     params[:txtToken_captcha_serpro_gov_br] = '4324343'
     params[:captcha_text] = '4324343'
-    logger = Logger.new(File.join(Rails.root, 'log', 'test_api.log'))
-    stubs(:logger).returns(logger)
-    assert_equal test_captcha('127.0.0.1', params, environment), 'Serpro captcha error: getaddrinfo: Name or service not known'
-  end
+    r = test_captcha('127.0.0.1', params, environment)
+    assert_equal(_("Serpro captcha error: getaddrinfo: Name or service not known"), r[0][:javascript_console_message])
+end
+###### END Captcha tests ######
   protected
@@ -52,7 +52,7 @@ class SessionTest &lt; ActiveSupport::TestCase
     assert_equal 400, last_response.status
   end
-  should 'detected error, Name or service not known, for Serpro Captcha communication' do
+  should 'detected error, Name or service not known, for Serpro captcha communication' do
     environment = Environment.default
     environment.api_captcha_settings = {
         enabled: true,
@@ -64,7 +64,8 @@ class SessionTest &lt; ActiveSupport::TestCase
     params = {:login => "newuserapi", :password => "newuserapi", :password_confirmation => "newuserapi", :email => "newuserapi@email.com",
               :txtToken_captcha_serpro_gov_br => '4324343', :captcha_text => '4030320'}
     post "/api/v1/register?#{params.to_query}"
-    assert_equal "Serpro captcha error: getaddrinfo: Name or service not known", JSON.parse(last_response.body)["message"]
+    message = JSON.parse(last_response.body)['javascript_console_message']
+    assert_equal "Serpro captcha error: getaddrinfo: Name or service not known", message
   end
   # TODO: Add another test cases to check register situations