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.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

soxofaan’s picture

Component: Code » User interface
Status: Active » Needs review
FileSize
788 bytes

Confirmed.

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.

WinstonC’s picture

Thanks 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.

soxofaan’s picture

it would be more desirable if the data entered by the user can stay in the form even after the CAPTCHA validation failed

Totally 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

      // 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']);
      }

I'm not sure this can be fixed withing the CAPTCHA module alone.
Need more investigation

jweowu’s picture

Status: Needs review » Reviewed & tested by the community

Reviewed 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.

soxofaan’s picture

soxofaan’s picture

Status: Reviewed & tested by the community » Needs work

Based on the fix #1219928: Can't return to login form if only captcha failed: apparently the code

      // 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']);
      }
    }

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:

  • The answer you entered for the CAPTCHA was not correct.
  • Sorry, unrecognized username or password. Have you forgotten your password?

This is because when the login validator function user_login_final_validate observers a login error (through user_authenticate), it always thinks this is because of username/pasword error, not because of another field.

soxofaan’s picture

Status: Needs work » Needs review
FileSize
944 bytes

attached patch removes old workaround
let's see what the test bot thinks of this

soxofaan’s picture

Status: Needs review » Fixed

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.