A standard user login page with a destination query string such as "user/login?destination=node/1234" and the CAPTCHA validation will take you to "node/1234", the destination page, instead of staying on the login page if your username and password are correct but the CAPTCHA fails.
Though the failed CAPTCHA validation prevents the user from logging in, it is desirable to stay on the user login page to give the user another chance to try.
The following code in captcha.module is supposed to do this
// If CAPTCHA was on a login form: stop validating, quit the current request
// and forward to the current page (like a reload) to prevent loging in.
// We do that because the log in procedure, which happens after
// captcha_validate(), does not check error conditions of extra form
// elements like the CAPTCHA.
if ($form_id == 'user_login' || $form_id == 'user_login_block') {
drupal_goto($_GET['q']);
}
However, drupal_goto doesn't seem to work though the execution gets to this point.
Since this is such an undesirable behavior, I'd consider it a bug and hope there's a solution for it.
Comment | File | Size | Author |
---|---|---|---|
#7 | 960270-login-and-destination-3.patch | 944 bytes | soxofaan |
#1 | 960270_login_and_destination_01.patch | 788 bytes | soxofaan |
Comments
Comment #1
soxofaan CreditAttribution: soxofaan commentedConfirmed.
The problem seems that drupal_goto is being too helpful and ignores the original path (
$_GET['q']
, user login form in this case) when there is a destination (node/1234).Attached patch tries to workaround this, but I'm not a big fan my self as it feels a bit dirty.
Comment #2
WinstonC CreditAttribution: WinstonC commentedThanks a lot, soxofaan. The patch works pretty much the way I wanted for now. However, for the better usability, it would be more desirable if the data entered by the user can stay in the form even after the CAPTCHA validation failed. In this case, the user ID and password. Try to imagine what would happen if the form has a lot more input fields. The user would be very frustrated to see all the data are lost just because he/she misread some of the distorted letters.
Comment #3
soxofaan CreditAttribution: soxofaan commentedTotally agree. But I'm afraid the bug is in Drupal core here as far as I remember: the user login form (up o D6) does not follow the traditional Drupal form processing workflow (especially validation), hence the workaround
I'm not sure this can be fixed withing the CAPTCHA module alone.
Need more investigation
Comment #4
jweowu CreditAttribution: jweowu commentedReviewed and tested.
There's a rogue space after the equals sign in
$destination = drupal_get_destination();
The issue in #2 could be logged separately, but I agree with #3 -- I doubt anything can be done about it. Better to request the core bug fix.
Comment #5
soxofaan CreditAttribution: soxofaan commentedrelated issue for D7: #1219928: Can't return to login form if only captcha failed
Comment #6
soxofaan CreditAttribution: soxofaan commentedBased on the fix #1219928: Can't return to login form if only captcha failed: apparently the code
in captcha.module is actually not necessary.
As far as I can remember/guess, that code was introduced during the beta period of Drupal6 to work around an issue in Drupal at that time. But after it has been fixed before Drupal 6.0 even, the workaround was never removed.
Removing the workaround seems to fix all the issues of this thread, with one minor remaining issue:
when you enter correct username and password, but wrong CAPTCHA response, you get two error messages:
This is because when the login validator function
user_login_final_validate
observers a login error (throughuser_authenticate
), it always thinks this is because of username/pasword error, not because of another field.Comment #7
soxofaan CreditAttribution: soxofaan commentedattached patch removes old workaround
let's see what the test bot thinks of this
Comment #8
soxofaan CreditAttribution: soxofaan commentedcool, no failed tests
committed and pushed:
http://drupalcode.org/project/captcha.git/commit/5f74f778599e51a42461bf7...