Port the NoCaptcha implementation of ReCaptcha from gh-5315 #5323

nikosdion · 2014-12-04T14:44:17Z

Executive summary

This is a fork of the work done in gh-5315 by @nonumber to implement the new NoCaptcha feature of reCAPTCHA. Since the new noCAPTCHA API is incompatible with the parameters of the old reCAPTCHA implementation we have no implemented it as a new plugin called "Captcha-NoCaptcha". The same tests as gh-5315 apply to this PR.

@nonumber

Close gh-5315 # Executive summary This is a fork of the work done in gh-5315 by @nonumber to implement the new NoCaptcha feature of reCAPTCHA. Since the new noCAPTCHA API is incompatible with the parameters of the old reCAPTCHA implementation we have no implemented it as a new plugin called "Captcha-NoCaptcha". The same tests as gh-5315 apply to this PR.

brianteeman · 2014-12-04T15:27:21Z

administrator/language/en-GB/en-GB.plg_captcha_nocaptcha.ini

+PLG_NOCAPTCHA_PUBLIC_KEY_LABEL="Site key"
+PLG_NOCAPTCHA_PUBLIC_KEY_DESC="Used in the JavaScript code that is served to your users. See the plugin description for instructions on getting a site key."
+PLG_NOCAPTCHA_PRIVATE_KEY_LABEL="Secret key"
+PLG_NOCAPTCHA_PRIVATE_KEY_DESC="Used in the communication between your server and the ReCaptha server. Be sure to keep it a secret. See the plugin description for instructions on getting a secret key."


ReCaptha ?? Shouldnt that be reCAPTCHA

Add you write it reCAPTCHA on line 6 and ReCaptha here
So it should be consistent and spelt correctly

brianteeman · 2014-12-04T15:35:08Z

Great work - thanks - I made a few small comments on the english language file

This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/5323.

nikosdion · 2014-12-04T15:39:31Z

@brianteeman I used the same capitalization as the reCAPTCHA plugin. Yeah, it's inconsistent with the product branding but I had two choices:

be inconsistent with Joomla!; or
be inconsistent with reCAPTCHA

Regarding index.html, it is not required by JED for third party extensions. The core plugins still have those files.

brianteeman · 2014-12-04T15:47:18Z

The same caps but not the same spelling ;)

re index.html see #3788

On 4 December 2014 at 15:39, Nicholas K. Dionysopoulos <
notifications@github.com> wrote:

@brianteeman https://github.com/brianteeman I used the same
capitalization as the reCAPTCHA plugin. Yeah, it's inconsistent with the
product branding but I had two choices:

be inconsistent with Joomla!; or

be inconsistent with reCAPTCHA

Regarding index.html, it is not required by JED for third party
extensions. The core plugins still have those files.

—
Reply to this email directly or view it on GitHub
#5323 (comment).

Brian Teeman
Co-founder Joomla! and OpenSourceMatters Inc.
http://brian.teeman.net/

waader · 2014-12-04T15:53:06Z

@test success! I tested with postgresql and mssql on the user registration form.

peterlose · 2014-12-04T16:00:46Z

installation/sql/mysql/joomla.sql

@@ -610,6 +610,7 @@ INSERT INTO `#__extensions` (`extension_id`, `name`, `type`, `element`, `folder`
 (449, 'plg_authentication_cookie', 'plugin', 'cookie', 'authentication', 0, 1, 1, 0, '', '', '', '', 0, '0000-00-00 00:00:00', 0, 0),
 (450, 'plg_twofactorauth_yubikey', 'plugin', 'yubikey', 'twofactorauth', 0, 0, 1, 0, '', '', '', '', 0, '0000-00-00 00:00:00', 0, 0),
 (451, 'plg_search_tags', 'plugin', 'tags', 'search', 0, 1, 1, 0, '', '{"search_limit":"50","show_tagged_items":"1"}', '', '', 0, '0000-00-00 00:00:00', 0, 0),
+(452, 'plg_captcha_nocaptcha', 'plugin', 'nocaptcha', 'captcha', 0, 0, 1, 0, '', '{"public_key":"","private_key":"","theme":"clean"}', '', '', 0, '0000-00-00 00:00:00', 0, 0),


Shoundn't the theme be light instead of clean? The new API only supports light and dark

peterlose · 2014-12-04T16:01:27Z

Could we perhaps include the data-type attribute?

nikosdion · 2014-12-04T16:17:30Z

I fixed the language strings and the default theme. I am not sure what you mean regarding the data-type attribute @losedk

peterlose · 2014-12-04T16:22:07Z

@nikosdion Cool!

Included a screenshot from the API documentation:

Perhaps it could be a setting in the plugin?

nikosdion · 2014-12-04T16:25:54Z

I think it shouldn't. We have not implemented it for the old reCAPTCHA plugin. Moreover –if I understand it correctly– Google will serve the correct type (image or audio) depending on the user's accessibility settings. I wonder if we have any people with impaired vision who can test it for us using their real world browser settings?

peterlose · 2014-12-04T16:27:28Z

Point taken! :)

brianteeman · 2014-12-04T16:28:31Z

@test Almost but there are issues with the captcha responsiveness which you can see if you use Isis and the testing data (or have anything in both left and right columns)

This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/5323.

brianteeman · 2014-12-04T16:30:37Z

Correction - the old captcha plugin has the exact same issues. And as you;ve corrected the test I am marking this as a successful test

This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/5323.

nikosdion · 2014-12-04T16:34:19Z

FYI @brianteeman the responsiveness is one part Google problem and one part Joomla! issue. If/when Joomla! switches to Bootstrap 3 this issue will go away. BS3 renders forms differently on extra small widths. Instead of having the label left and the fields to the right it moves the label on top of the fields and this fixes the layout.

mbabker · 2014-12-04T20:15:19Z

Installed the plugin on developer.joomla.org, working fine.

zero-24 · 2014-12-04T20:27:06Z

@nikosdion can you disable the CS stuff for the google lib and fix the CS stuff for the new file?

FILE: .../travis/build/joomla/joomla-cms/plugins/captcha/nocaptcha/nocaptcha.php
--------------------------------------------------------------------------------
FOUND 6 ERROR(S) AFFECTING 4 LINE(S)
--------------------------------------------------------------------------------
 31 | ERROR | Expected 2 spaces after the longest type
 31 | ERROR | Expected 2 spaces after the longest variable name
 69 | ERROR | Expected 2 spaces after the longest type
 71 | ERROR | Expected 2 spaces after the longest variable name
 86 | ERROR | Expected 2 spaces after the longest type
 86 | ERROR | Expected 2 spaces after the longest variable name
--------------------------------------------------------------------------------

See: https://travis-ci.org/joomla/joomla-cms/jobs/42998686

zero-24 · 2014-12-04T20:50:20Z

@nikosdion i have add a CS PR to fix the issues: https://github.com/nikosdion/joomla-cms/pull/3

Fix CS for Port the NoCaptcha implementation of ReCaptcha from gh-5315

nikosdion · 2014-12-04T20:59:01Z

Thank you! I just merged your PR.

@nikosdion

``` FILE: .../travis/build/joomla/joomla-cms/plugins/captcha/nocaptcha/nocaptcha.php -------------------------------------------------------------------------------- FOUND 2 ERROR(S) AFFECTING 2 LINE(S) -------------------------------------------------------------------------------- 31 | ERROR | Expected 2 spaces after the longest type 71 | ERROR | Expected 2 spaces after the longest variable name -------------------------------------------------------------------------------- ``` Sorry @nikosdion

zero-24 · 2014-12-04T21:14:00Z

Thanks but it looks like i have missed two errors. https://github.com/nikosdion/joomla-cms/pull/4 Sorry @nikosdion

hmm looks like i have missed two errors

nikosdion · 2014-12-04T21:25:21Z

There you go, merged :)

zero-24 · 2014-12-04T21:42:27Z

thanks Travis is happy now 😄

jessicadunbar · 2014-12-04T21:43:18Z

Thank you! I tested it on developer.joomla.org and everything tested fine.

nikosdion · 2014-12-04T21:44:56Z

Awesome! I guess we can set it RTC now that we have tested it on project property and Travis is happy? :)

zero-24 · 2014-12-04T21:51:26Z

moving to RTC but i think @Bakual @phproberto @mbabker @dbhurley or others form the PLT need to accept it bevor we can merge this 😄

This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/5323.

brianteeman · 2014-12-04T21:52:18Z

@nikosdion Can you correct the spelling mistake in the english as I commented on github. Once thats done I will set it RTC

This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/5323.

Corrected spelling mistake

Update en-GB.plg_captcha_nocaptcha.ini

brianteeman · 2014-12-04T23:04:31Z

Thanks for merging that spelling correction

infograf768 · 2014-12-06T05:54:41Z

Why add deprecated strings in a new plugin?

infograf768 · 2014-12-06T05:56:05Z

@committer
a new line has to be added in en-GB.install.xml for the new plugin if merged in 3.4.0 beta

Add the new language files to the install.xml & remove the not longer used lang strings

nikosdion · 2014-12-06T10:28:46Z

@infograf768 Issues fixed thanks to @zero-24's speedy PR :)

infograf768 · 2014-12-06T11:24:54Z

Works great!
Looks like we are covered for all these languages;
https://developers.google.com/recaptcha/docs/language

jissues-bot added the PR-staging label Dec 4, 2014

brianteeman reviewed Dec 4, 2014
View reviewed changes

peterlose reviewed Dec 4, 2014
View reviewed changes

Nicholas K. Dionysopoulos added 2 commits December 4, 2014 18:12

Capitalisation consistency in language strings

eb0df81

Wrong default parameter (clean instead of light)

bbd4bd9

zero-24 added 2 commits December 4, 2014 21:44

exclude new recaptchalib

3bb1921

CS for the new file

c1022f6

Merge pull request #3 from zero-24/patch-5

b086ba4

Fix CS for Port the NoCaptcha implementation of ReCaptcha from gh-5315

Merge pull request #4 from zero-24/patch-5

1f1b636

hmm looks like i have missed two errors

brianteeman and others added 2 commits December 4, 2014 22:30

Update en-GB.plg_captcha_nocaptcha.ini

3de4303

Corrected spelling mistake

Merge pull request #5 from brianteeman/patch-3

d7b4d2a

Update en-GB.plg_captcha_nocaptcha.ini

jissues-bot added the RTC This Pull Request is Ready To Commit label Dec 4, 2014

zero-24 and others added 3 commits December 6, 2014 11:22

Add the new language files to the install.xml

a45a776

remove unused lang strings

94ec22d

Merge pull request #6 from zero-24/patch-1

f0b6170

Add the new language files to the install.xml & remove the not longer used lang strings

infograf768 closed this in 4921769 Dec 7, 2014

infograf768 added the Feature label Dec 7, 2014

infograf768 added this to the Joomla! 3.4.0 milestone Dec 7, 2014

zero-24 removed the RTC This Pull Request is Ready To Commit label Oct 14, 2015

joomla-cms-bot added the Language Change This is for Translators label Oct 14, 2015

810 mentioned this pull request Nov 7, 2015

J3.5 Beta1 #8304

Closed

Uh oh!

Port the NoCaptcha implementation of ReCaptcha from gh-5315 #5323

Port the NoCaptcha implementation of ReCaptcha from gh-5315 #5323

Uh oh!

Conversation

nikosdion commented Dec 4, 2014

Executive summary

Uh oh!

brianteeman Dec 4, 2014

Choose a reason for hiding this comment

Uh oh!

brianteeman Dec 4, 2014

Choose a reason for hiding this comment

Uh oh!

brianteeman commented Dec 4, 2014

Uh oh!

nikosdion commented Dec 4, 2014

Uh oh!

brianteeman commented Dec 4, 2014

Uh oh!

waader commented Dec 4, 2014

Uh oh!

peterlose Dec 4, 2014

Choose a reason for hiding this comment

Uh oh!

peterlose commented Dec 4, 2014

Uh oh!

nikosdion commented Dec 4, 2014

Uh oh!

peterlose commented Dec 4, 2014

Uh oh!

nikosdion commented Dec 4, 2014

Uh oh!

peterlose commented Dec 4, 2014

Uh oh!

brianteeman commented Dec 4, 2014

Uh oh!

brianteeman commented Dec 4, 2014

Uh oh!

nikosdion commented Dec 4, 2014

Uh oh!

mbabker commented Dec 4, 2014

Uh oh!

zero-24 commented Dec 4, 2014

Uh oh!

zero-24 commented Dec 4, 2014

Uh oh!

nikosdion commented Dec 4, 2014

Uh oh!

zero-24 commented Dec 4, 2014

Uh oh!

nikosdion commented Dec 4, 2014

Uh oh!

zero-24 commented Dec 4, 2014

Uh oh!

jessicadunbar commented Dec 4, 2014

Uh oh!

nikosdion commented Dec 4, 2014

Uh oh!

zero-24 commented Dec 4, 2014

Uh oh!

brianteeman commented Dec 4, 2014

Uh oh!

brianteeman commented Dec 4, 2014

Uh oh!

infograf768 commented Dec 6, 2014

Uh oh!

infograf768 commented Dec 6, 2014

Uh oh!

nikosdion commented Dec 6, 2014

Uh oh!

infograf768 commented Dec 6, 2014

Uh oh!

Uh oh!