New module that incorporates reCaptcha in user-password login form

[GoogleCodeExporter]

A new module that includes a reCaptcha to the username-password login form.
It is possible to require reCaptcha every time or only after a number of failed 
login attempts.
Information on the user (and the number of failed login attempts) is extracted 
from LDAP.

Modifications to employ other user data sources (instead of LDAP), or other 
captcha systems (and not reCaptcha) should be fairly easy.

Original issue reported on code.google.com by dmbor...@gmail.com on 5 Jun 2012 at 10:10

Attachments:

• userpasswordcaptcha-module.tar.gz

[GoogleCodeExporter]

Hi,

this patch copies rather large portitions of the SSP code rather than extending 
existing classes, which has the unfortunate side effect of duplicating a lot of 
the code, which in turn increases the maintenance required for this module. 
From a quick look at it, I'd like to see some changes:

- The config in config-templates/mod_userpasswordcaptcha.php should probably be 
in config/authsources.php instead.
- The errors dictionary should not be copied. Instead, the errors specific for 
this module should be moved into a separate dictionary.
- Since you already have a custom template for this module, the htmlinject hook 
should not be necessary. Instead, just add it to the template.
- The sspmod_userpasswordcaptcha_Auth_UserPassBase class should extend the 
existing UserPassBase class. It may be necessary to modify the base class 
slightly in order to make it easier to override the parts that you need.
- The sspmod_userpasswordcaptcha_failedLogins class should probably be moved 
into the spmod_userpasswordcaptcha_Auth_UserPassBase class.
- The same goes for the sspmod_userpasswordcaptcha_reCAPTCHA class.
- I do not think the ucmTheme should be included?
- Since you reuse the "stage id" of the normal UserPassBase class in loadState 
and saveState, your module can easily be bypassed by just altering the URL to 
the normal loginuserpass.php page. You need to change the code to use a custom 
stage.
- I think it would be better to move the code for deciding whether to show a 
captcha (and require a captcha answer) into the class that actually handles the 
authentication. By adding an extra parameter to the login-function 
($hasValidCaptcha), and adding a specific exception that says "user should 
provide a captcha on the next login attempt", I think the javascript can be 
removed, and the code would be easier to adapt to other login methods than LDAP.

Unfortunately, I do not have time to make these changes. If you want to make 
them, it would be very good, but if not, we can just leave the code here on the 
issue tracker until a later time.

Original comment by olavmrk@gmail.com on 5 Jun 2012 at 2:09

[GoogleCodeExporter]

Hi,
Thanks for spending some time taking a look at this.
You are right in that most of the code is a replica, and some things would
be best done by modifying the core code, but I've tried to modify files
only in the module's directory. For example, all lib/Auth files are there
because I need to call www/loginuserpass.php. If you could indicate a
better way to do this without modifying code outside this module I'd be
glad to do it.

I have some questions:

- config-templates/mod_userpasswordcaptcha.php is to be copied to config/.
I didn't want to modify existing code and that's the reason for the
separate file. Of course I can integrate it with authsources.php

- I'd also like to have dictionary files with just the added text, but I
don't know how to do this. In templates/loginuserpass.php the error texts
may be from both the normal login process (username or password incorrect)
and from the captcha process. But the code does not give the option (or has
a fallback). Either error messages are from the main dictionary or from
userpasswordcaptcha ($this->t('errors:title_' . $this->data['errorcode'] .
'}');). One option would be to include the new captcha errors in the main
dictionary, but again I didn't want to modify files outside the module.

- The htmlinject hook puts code in the <head> of the form. The template I
have (templates/loginuserpass.php) modifies the <body>. Once I delete
ucmTheme, i thought the easiest way to get some code into the <head> is
htmlinject hook. Maybe you could point to a better solution.

- I'll try to merge reCAPTCHA and failedLogins classes in the
sspmod_userpasswordcaptcha_Auth_UserPassBase class. I still don't know how
to avoid copying a big chunk of the original sspmod_core_Auth_UserPassBase
class without modifying files outside the new module directory.

- I've deleted the ucmTheme directory.

- I've modified STAGEID to
sspmod_userpasswordcaptcha_Auth_UserPassBase.state, but then I get the
following error:
      The page isn't redirecting properly
      Firefox has detected that the server is redirecting the request for
this address in a way that will never complete.
      This problem can sometimes be caused by disabling or refusing to
accept cookies.
  How should I do this?

- The way you propose to get the captcha shown is also valid, but has one
drawback. If the first login attempt already requires captcha (because the
user previously failed too many times), it will not be shown to the user
until he/she tries to log in. It's a minor drawback. I'll think about it.

Thanks for the input,
David

Original comment by dmbor...@gmail.com on 6 Jun 2012 at 12:18

[GoogleCodeExporter]

> - I'd also like to have dictionary files with just the added text, but I
> don't know how to do this. In templates/loginuserpass.php the error texts
> may be from both the normal login process (username or password incorrect)
> and from the captcha process. But the code does not give the option (or has
> a fallback). Either error messages are from the main dictionary or from
> userpasswordcaptcha ($this->t('errors:title_' . $this->data['errorcode'] .
> '}');). One option would be to include the new captcha errors in the main
> dictionary, but again I didn't want to modify files outside the module.

Since you control the template, changing it to use your own dictionary for that 
specific error should be possible.

> - The htmlinject hook puts code in the <head> of the form. The template I
> have (templates/loginuserpass.php) modifies the <body>. Once I delete
> ucmTheme, i thought the easiest way to get some code into the <head> is
> htmlinject hook. Maybe you could point to a better solution.

You can also do this from the template: $this->data['head'] = [...]
That line must be inserted before the header-template is included.

> - I'll try to merge reCAPTCHA and failedLogins classes in the
> sspmod_userpasswordcaptcha_Auth_UserPassBase class. I still don't know how
> to avoid copying a big chunk of the original sspmod_core_Auth_UserPassBase
> class without modifying files outside the new module directory.

I think some modifications may be necessary, but it should mostly be small 
changes that make it easier to extend this class.

> - I've modified STAGEID to
> sspmod_userpasswordcaptcha_Auth_UserPassBase.state, but then I get the
> following error:
>       The page isn't redirecting properly
>       Firefox has detected that the server is redirecting the request for
> this address in a way that will never complete.
>       This problem can sometimes be caused by disabling or refusing to
> accept cookies.
>   How should I do this?

You probably have some reference to the old stage ID somewhere, which causes it 
to fail when loading the state.

> - The way you propose to get the captcha shown is also valid, but has one
> drawback. If the first login attempt already requires captcha (because the
> user previously failed too many times), it will not be shown to the user
> until he/she tries to log in. It's a minor drawback. I'll think about it.

I agree that it is a bit of an inconvinience, but I think it is rare that the 
user will experience it. Most of the time they will get the captcha after 
repeated login attempts. Only rarely will it be on the first attempt. (It will 
happen in the case where they have given up earlier, or in the case of attacks.)

Original comment by olavmrk@gmail.com on 7 Jun 2012 at 7:42

[GoogleCodeExporter]

Original comment by jaim...@gmail.com on 26 Feb 2014 at 2:03

• Added labels: Type-Enhancement

[GoogleCodeExporter]

Nothing has happened here for well over a year. Closing it as WontFix. It can 
be resubmitted as a pull request to GitHub later.

Original comment by olavmrk@gmail.com on 27 Feb 2014 at 9:53

• Changed state: WontFix