Commit 88227abbc3ec9b5fe99aebc4349a410e7468a881
1 parent
5940f0c6
Exists in
master
fixed asserting hash as true
Showing
3 changed files
with
39 additions
and
61 deletions
Show diff stats
lib/recaptcha_verification.rb
@@ -10,7 +10,6 @@ class RecaptchaVerification | @@ -10,7 +10,6 @@ class RecaptchaVerification | ||
10 | if recaptcha_challenge_field == nil || recaptcha_response_field == nil | 10 | if recaptcha_challenge_field == nil || recaptcha_response_field == nil |
11 | return hash_error(_('Captcha validation error'), 500, nil, _('Missing captcha data')) | 11 | return hash_error(_('Captcha validation error'), 500, nil, _('Missing captcha data')) |
12 | end | 12 | end |
13 | - | ||
14 | verify_hash = { | 13 | verify_hash = { |
15 | "privatekey" => private_key, | 14 | "privatekey" => private_key, |
16 | "remoteip" => remote_ip, | 15 | "remoteip" => remote_ip, |
@@ -22,15 +21,12 @@ class RecaptchaVerification | @@ -22,15 +21,12 @@ class RecaptchaVerification | ||
22 | https.use_ssl = true | 21 | https.use_ssl = true |
23 | request = Net::HTTP::Post.new(uri.path) | 22 | request = Net::HTTP::Post.new(uri.path) |
24 | request.set_form_data(verify_hash) | 23 | request.set_form_data(verify_hash) |
25 | - # begin | ||
26 | - result = https.request(request).body.split("\n") | ||
27 | - # rescue Exception => e | ||
28 | - # return hash_error(_('Internal captcha validation error'), 500, nil, "Error validating Googles' recaptcha version 1: #{e.message}") | ||
29 | - # end | ||
30 | - return true if result[0] == "true" | ||
31 | - return hash_error(_("Wrong captcha text, please try again"), 403, nil, "Error validating Googles' recaptcha version 1: #{result[1]}") if result[1] == "incorrect-captcha-sol" | 24 | + body = https.request(request).body |
25 | + captcha_result = JSON.parse(body) | ||
26 | + return true if captcha_result["success"] | ||
27 | + return hash_error(_("Wrong captcha text, please try again"), 403, nil, "Error validating Googles' recaptcha version 1: #{captcha_result["error-codes"]}") if captcha_result["error-codes"] == "incorrect-captcha-sol" | ||
32 | #Catches all errors at the end | 28 | #Catches all errors at the end |
33 | - return hash_error(_("Internal recaptcha validation error"), 500, nil, "Error validating Googles' recaptcha version 1: #{result[1]}") | 29 | + return hash_error(_("Internal recaptcha validation error"), 500, nil, "Error validating Googles' recaptcha version 1: #{captcha_result["error-codes"]}") |
34 | end | 30 | end |
35 | 31 | ||
36 | # return true or a hash with the error | 32 | # return true or a hash with the error |
@@ -47,40 +43,10 @@ class RecaptchaVerification | @@ -47,40 +43,10 @@ class RecaptchaVerification | ||
47 | https.use_ssl = true | 43 | https.use_ssl = true |
48 | request = Net::HTTP::Post.new(uri.path) | 44 | request = Net::HTTP::Post.new(uri.path) |
49 | request.set_form_data(verify_hash) | 45 | request.set_form_data(verify_hash) |
50 | - # begin | ||
51 | - body = https.request(request).body | ||
52 | - # rescue Exception => e | ||
53 | - # return hash_error(_('Internal captcha validation error'), 500, nil, "recaptcha error: #{e.message}") | ||
54 | - # end | 46 | + body = https.request(request).body |
55 | captcha_result = JSON.parse(body) | 47 | captcha_result = JSON.parse(body) |
56 | return true if captcha_result["success"] | 48 | return true if captcha_result["success"] |
57 | return hash_error(_("Wrong captcha text, please try again"), 403, body, captcha_result["error-codes"]) | 49 | return hash_error(_("Wrong captcha text, please try again"), 403, body, captcha_result["error-codes"]) |
58 | end | 50 | end |
59 | 51 | ||
60 | - # return true or a hash with the error | ||
61 | - # :user_message, :status, :log_message, :javascript_console_message | ||
62 | - def verify_serpro_captcha(client_id, token, captcha_text, verify_uri) | ||
63 | - msg_icve = _('Internal captcha validation error') | ||
64 | - msg_esca = 'Environment recaptcha_plugin_attributes' | ||
65 | - return hash_error(msg_icve, 500, nil, "#{msg_esca} verify_uri not defined") if verify_uri.nil? | ||
66 | - return hash_error(msg_icve, 500, nil, "#{msg_esca} client_id not defined") if client_id.nil? | ||
67 | - return hash_error(_("Error processing token validation"), 500, nil, _("Missing Serpro's Captcha token")) unless token | ||
68 | - return hash_error(_('Captcha text has not been filled'), 403) unless captcha_text | ||
69 | - uri = URI(verify_uri) | ||
70 | - http = Net::HTTP.new(uri.host, uri.port) | ||
71 | - request = Net::HTTP::Post.new(uri.path) | ||
72 | - verify_string = "#{client_id}&#{token}&#{captcha_text}" | ||
73 | - request.body = verify_string | ||
74 | - body = http.request(request).body | ||
75 | - return true if body == '1' | ||
76 | - return hash_error(_("Internal captcha validation error"), 500, body, "Unable to reach Serpro's Captcha validation service") if body == "Activity timed out" | ||
77 | - return hash_error(_("Wrong captcha text, please try again"), 403) if body == '0' | ||
78 | - return hash_error(_("Serpro's captcha token not found"), 500) if body == '2' | ||
79 | - return hash_error(_("No data sent to validation server or other serious problem"), 500) if body == -1 | ||
80 | - #Catches all errors at the end | ||
81 | - return hash_error(_("Internal captcha validation error"), 500, nil, "Error validating Serpro's captcha service returned: #{body}") | ||
82 | - end | ||
83 | - | ||
84 | - | ||
85 | - | ||
86 | end | 52 | end |
test/test_helper.rb
@@ -30,9 +30,7 @@ class ActiveSupport::TestCase | @@ -30,9 +30,7 @@ class ActiveSupport::TestCase | ||
30 | remoteip: "127.0.0.1"} | 30 | remoteip: "127.0.0.1"} |
31 | end | 31 | end |
32 | 32 | ||
33 | - return_body = "{ | ||
34 | - \"success\": #{pass} | ||
35 | - }" | 33 | + return_body = "{\"success\": #{pass} }" |
36 | 34 | ||
37 | stub_request(:post, @verify_uri). | 35 | stub_request(:post, @verify_uri). |
38 | with(:body => body, | 36 | with(:body => body, |
test/unit/recaptcha_verification_test.rb
@@ -17,17 +17,16 @@ class RecaptchaVerificationTest < ActiveSupport::TestCase | @@ -17,17 +17,16 @@ class RecaptchaVerificationTest < ActiveSupport::TestCase | ||
17 | @params = {} | 17 | @params = {} |
18 | @params[:remoteip] = @remoteip | 18 | @params[:remoteip] = @remoteip |
19 | if version.to_i == 1 | 19 | if version.to_i == 1 |
20 | - #wont go to google thanks to webmock | 20 | + # won't go to google thanks to webmock |
21 | @verify_uri = 'https://www.google.com/recaptcha/api/verify' | 21 | @verify_uri = 'https://www.google.com/recaptcha/api/verify' |
22 | @params[:privatekey] = @environment.recaptcha_private_key | 22 | @params[:privatekey] = @environment.recaptcha_private_key |
23 | @params[:challenge] = "challenge" | 23 | @params[:challenge] = "challenge" |
24 | @params[:response] = "response" | 24 | @params[:response] = "response" |
25 | - | ||
26 | @params[:recaptcha_challenge_field] = @params[:challenge] | 25 | @params[:recaptcha_challenge_field] = @params[:challenge] |
27 | @params[:recaptcha_response_field] = @params[:response] | 26 | @params[:recaptcha_response_field] = @params[:response] |
28 | end | 27 | end |
29 | if version.to_i == 2 | 28 | if version.to_i == 2 |
30 | - #wont go to google thanks to webmock | 29 | + # won't go to google thanks to webmock |
31 | @verify_uri = 'https://www.google.com/recaptcha/api/siteverify' | 30 | @verify_uri = 'https://www.google.com/recaptcha/api/siteverify' |
32 | @params[:secret] = @environment.recaptcha_private_key | 31 | @params[:secret] = @environment.recaptcha_private_key |
33 | @params[:response] = "response" | 32 | @params[:response] = "response" |
@@ -55,7 +54,8 @@ class RecaptchaVerificationTest < ActiveSupport::TestCase | @@ -55,7 +54,8 @@ class RecaptchaVerificationTest < ActiveSupport::TestCase | ||
55 | setup_captcha(version) | 54 | setup_captcha(version) |
56 | validate_captcha(version) | 55 | validate_captcha(version) |
57 | r = RecaptchaPlugin.new.test_captcha(@remoteip, @params, @environment) | 56 | r = RecaptchaPlugin.new.test_captcha(@remoteip, @params, @environment) |
58 | - assert r | 57 | + assert_not_kind_of Hash, r |
58 | + assert_equal true, r | ||
59 | end | 59 | end |
60 | 60 | ||
61 | should 'fail recaptcha version 1' do | 61 | should 'fail recaptcha version 1' do |
@@ -63,16 +63,16 @@ class RecaptchaVerificationTest < ActiveSupport::TestCase | @@ -63,16 +63,16 @@ class RecaptchaVerificationTest < ActiveSupport::TestCase | ||
63 | setup_captcha(version) | 63 | setup_captcha(version) |
64 | validate_captcha(version, false) | 64 | validate_captcha(version, false) |
65 | r = RecaptchaPlugin.new.test_captcha(@remoteip, @params, @environment) | 65 | r = RecaptchaPlugin.new.test_captcha(@remoteip, @params, @environment) |
66 | - assert r | 66 | + assert_kind_of Hash, r |
67 | end | 67 | end |
68 | 68 | ||
69 | should 'pass recaptcha version 2' do | 69 | should 'pass recaptcha version 2' do |
70 | version = 2 | 70 | version = 2 |
71 | setup_captcha(version) | 71 | setup_captcha(version) |
72 | validate_captcha(version) | 72 | validate_captcha(version) |
73 | - rp = RecaptchaPlugin.new | ||
74 | - r = rp.test_captcha(@remoteip, @params, @environment) | ||
75 | - assert r | 73 | + r = RecaptchaPlugin.new.test_captcha(@remoteip, @params, @environment) |
74 | + assert_not_kind_of Hash, r | ||
75 | + assert_equal true, r | ||
76 | end | 76 | end |
77 | 77 | ||
78 | should 'fail recaptcha version 2' do | 78 | should 'fail recaptcha version 2' do |
@@ -80,6 +80,7 @@ class RecaptchaVerificationTest < ActiveSupport::TestCase | @@ -80,6 +80,7 @@ class RecaptchaVerificationTest < ActiveSupport::TestCase | ||
80 | setup_captcha(version) | 80 | setup_captcha(version) |
81 | validate_captcha(version, false) | 81 | validate_captcha(version, false) |
82 | r = RecaptchaPlugin.new.test_captcha(@remoteip, @params, @environment) | 82 | r = RecaptchaPlugin.new.test_captcha(@remoteip, @params, @environment) |
83 | + assert_kind_of Hash, r | ||
83 | assert_equal r[:user_message], _("Wrong captcha text, please try again") | 84 | assert_equal r[:user_message], _("Wrong captcha text, please try again") |
84 | end | 85 | end |
85 | 86 | ||
@@ -109,7 +110,6 @@ class RecaptchaVerificationTest < ActiveSupport::TestCase | @@ -109,7 +110,6 @@ class RecaptchaVerificationTest < ActiveSupport::TestCase | ||
109 | assert_equal json["message"], _("Wrong captcha text, please try again") | 110 | assert_equal json["message"], _("Wrong captcha text, please try again") |
110 | end | 111 | end |
111 | 112 | ||
112 | - | ||
113 | should 'fail captcha if user has not filled recaptcha_verify_uri v1 text' do | 113 | should 'fail captcha if user has not filled recaptcha_verify_uri v1 text' do |
114 | version = 1 | 114 | version = 1 |
115 | setup_captcha(version) | 115 | setup_captcha(version) |
@@ -124,17 +124,31 @@ class RecaptchaVerificationTest < ActiveSupport::TestCase | @@ -124,17 +124,31 @@ class RecaptchaVerificationTest < ActiveSupport::TestCase | ||
124 | article = create_article('Article 1') | 124 | article = create_article('Article 1') |
125 | params = {} | 125 | params = {} |
126 | params[:value] = 1 | 126 | params[:value] = 1 |
127 | + post "/api/v1/articles/#{article.id}/vote?#{params.to_query}" | ||
128 | + json = JSON.parse(last_response.body) | ||
129 | + assert_equal 401, last_response.status | ||
130 | + end | ||
127 | 131 | ||
132 | + should 'not perform a vote if recaptcha 1 fails' do | ||
133 | + version = 1 | ||
134 | + setup_captcha(version) | ||
135 | + validate_captcha(version, false) | ||
136 | + post "/api/v1/login-captcha?#{@params.to_query}" | ||
137 | + json = JSON.parse(last_response.body) | ||
138 | + article = create_article('Article 1') | ||
139 | + params = {} | ||
140 | + params[:private_token] = json['private_token'] | ||
141 | + params[:value] = 1 | ||
128 | post "/api/v1/articles/#{article.id}/vote?#{params.to_query}" | 142 | post "/api/v1/articles/#{article.id}/vote?#{params.to_query}" |
129 | json = JSON.parse(last_response.body) | 143 | json = JSON.parse(last_response.body) |
130 | assert_equal 401, last_response.status | 144 | assert_equal 401, last_response.status |
131 | end | 145 | end |
132 | 146 | ||
133 | - should 'perform a vote on an article identified by id' do | ||
134 | - version = 2 | 147 | + should 'perform a vote on an article identified by id using recaptcha 1' do |
148 | + version = 1 | ||
135 | setup_captcha(version) | 149 | setup_captcha(version) |
136 | validate_captcha(version) | 150 | validate_captcha(version) |
137 | - post "/api/v1/login-captcha?#{params.to_query}" | 151 | + post "/api/v1/login-captcha?#{@params.to_query}" |
138 | json = JSON.parse(last_response.body) | 152 | json = JSON.parse(last_response.body) |
139 | article = create_article('Article 1') | 153 | article = create_article('Article 1') |
140 | params = {} | 154 | params = {} |
@@ -146,10 +160,10 @@ class RecaptchaVerificationTest < ActiveSupport::TestCase | @@ -146,10 +160,10 @@ class RecaptchaVerificationTest < ActiveSupport::TestCase | ||
146 | assert_equal true, json['vote'] | 160 | assert_equal true, json['vote'] |
147 | end | 161 | end |
148 | 162 | ||
149 | - should 'not perform a vote if recaptcha 2 fails' do | 163 | + should 'perform a vote on an article identified by id using recaptcha 2' do |
150 | version = 2 | 164 | version = 2 |
151 | setup_captcha(version) | 165 | setup_captcha(version) |
152 | - validate_captcha(version, false) | 166 | + validate_captcha(version) |
153 | post "/api/v1/login-captcha?#{@params.to_query}" | 167 | post "/api/v1/login-captcha?#{@params.to_query}" |
154 | json = JSON.parse(last_response.body) | 168 | json = JSON.parse(last_response.body) |
155 | article = create_article('Article 1') | 169 | article = create_article('Article 1') |
@@ -158,11 +172,12 @@ class RecaptchaVerificationTest < ActiveSupport::TestCase | @@ -158,11 +172,12 @@ class RecaptchaVerificationTest < ActiveSupport::TestCase | ||
158 | params[:value] = 1 | 172 | params[:value] = 1 |
159 | post "/api/v1/articles/#{article.id}/vote?#{params.to_query}" | 173 | post "/api/v1/articles/#{article.id}/vote?#{params.to_query}" |
160 | json = JSON.parse(last_response.body) | 174 | json = JSON.parse(last_response.body) |
161 | - assert_equal 401, last_response.status | 175 | + assert_not_equal 401, last_response.status |
176 | + assert_equal true, json['vote'] | ||
162 | end | 177 | end |
163 | 178 | ||
164 | - should 'not perform a vote if recaptcha 1 fails' do | ||
165 | - version = 1 | 179 | + should 'not perform a vote if recaptcha 2 fails' do |
180 | + version = 2 | ||
166 | setup_captcha(version) | 181 | setup_captcha(version) |
167 | validate_captcha(version, false) | 182 | validate_captcha(version, false) |
168 | post "/api/v1/login-captcha?#{@params.to_query}" | 183 | post "/api/v1/login-captcha?#{@params.to_query}" |
@@ -176,5 +191,4 @@ class RecaptchaVerificationTest < ActiveSupport::TestCase | @@ -176,5 +191,4 @@ class RecaptchaVerificationTest < ActiveSupport::TestCase | ||
176 | assert_equal 401, last_response.status | 191 | assert_equal 401, last_response.status |
177 | end | 192 | end |
178 | 193 | ||
179 | - | ||
180 | end | 194 | end |