Enable cacheable captcha support (once 2449209 is committed)

Thanks for the report. What is probably needed is something to ensure that reCAPTCHA pages are never cached. It would help to look at how the math CAPTCHA does it.

I'm changing the status of this one as I think its fairly important to scale up. It's not the greatest to get the feeling that there is a possiblity for attacks on our sites using reCaptcha

The CAPTCHA session reuse attack is just a way of getting around the CAPTCHA. It doesn't open the site up to other attacks.

Wouldn't turning off "Cache pages for anonymous users" @ admin/config/development/performance mitigate this issue ?

@Jedd Casella: Yes, I expect that would solve the problem.

Here is an issue in Captcha module to try to get reCAPTCHA+caching to work: #2449209: Add support for cacheable captcha's (recaptcha). This can be fixed in either of the two modules, so I am thinking both issues should be kept open.

Is this a dupe of the fix http://drupalcode.org/project/recaptcha.git/commit/071f79c ?

No, because the fix is applied, and the problem persists.

This may be been resolved by #1395184: Forms with AJAX trigger "CAPTCHA session reuse attack detected." error. Please verify that the problem still happens.

I implemented proper support for recaptcha on cached pages.

This patch will only do something once #2449209: Add support for cacheable captcha's (recaptcha) is committed. Still posting it now, so I can apply the patch for a client.

It however can be applied at any time.

Looks good and works well, feel free to move this to RTBC once the dependency is committed.

I guess this is now for v2.

Dependency is committed. Should we RTBC based on #15?

Yep :)

+1 for RTBC

+1 or is there anything else blocking this?

1. Do we need to implement the same in D8?
2. Has someone and idea about the schedule of next CAPTCHA module release? I would only commit this when we know there is an upcomming release. Otherwise I create a new release and people may start facing issues or is this 100% backward compatible?

#22:

1. It depends on how captcha / recaptcha are implemented in Drupal 8.
2. No, idea, but the only effect is that it will continue to disable the page cache as now if the old captcha version is used.

The setting of the property has no effect if an older captcha version is used (as that property did not exist back then), so this is safe to commit whenever.

Maybe add a comment to the release notes:

To make use of the functionality you need to use captcha-7.x-1.x-dev for now.

Or something like that.

But in general this can go in without any risk:

- For older captcha versions: Nothing happens.
- For newer captcha versions: It works.

No other effects.

1. It depends on how captcha / recaptcha are implemented in Drupal 8.

I'm not aware there is a difference to D7 and captcha module also disables the cache on captcha enabled pages.

I guess eventually that patch will be ported to 8.x then. It might be the same or rely on e.g. placeholdering to at least have dynamic_page_cache enabled.

In any case it will probably be different than the patch here, so I would suggest to get this in and postpone an 8.x issue on a potential upstream update for 8.x?

That would mean we are running into a regression. We should first fix D8, than backport.

@fabianx (#25)
8.x is already benefiting from dynamic_page_cache without any patches. If the patches are ported, the end result should be full page cache.

@hass (#26)
Captcha maintainer already merged the patch in D7. Why not apply the recaptcha patch? There is no reason why the patches can't be ported to D8, but delaying the D7 merge is not going to make this faster in my opinion.

i think the rules are simple. We fix latest drupal, than backport. Otherwise fixes will never go into current versions, but only past.

I‘d really like to commit this asap, but we need to make sure we are not running into a regression. If nobody in here is interrested in D8, we will not see a D8 patch in next century. So, i make this a requirement for the D7 patch to get things done. Very simple.

There is another case #2893656: Getting a "CAPTCHA session reuse attack detected" error. that shows a bug if recaptcha is cached in D8. I guess we will see the same bug once this patch is committed. Aside of this bug the recaptcha seems to be cached in D8. So we do not need to write code for D8 to implement caching, but we need to figure out why the session reuse attack detected.

I did some research on this. I said in #27 that "8.x is already benefiting from dynamic_page_cache without any patches". That's not correct at all.

In 8.x image captcha and math captha do not benefit from dynamic_page_cache or full page_cache. It is possible that they can be made to benefit from dynamic_page_cache, but I have no idea how. Full page_cache shouldn't be possible because of how the module relies on the so called "captcha session" that it must create for each individual captcha challenge and store in mysql database.
This is how the caching is disabled in general:

        $result['form']['captcha_response'] = [
          '#type' => 'textfield',
          '#title' => t('Math question'),
          ...
          '#cache' => ['max-age' => 0],
        ];
        \Drupal::service('page_cache_kill_switch')->trigger();

In 8.x recaptcha already does benefit from full page_cache and it is working correctly in the sense that it is not preventing caching of the page, it is validating the challenge correctly, and it has a different challenge for each visit of the page. The only problem is that captcha module really likes to send the totally useless "CAPTCHA session reuse attack detected" message. The related issue #2893656-8: Getting a "CAPTCHA session reuse attack detected" error. currently has patches that fix the message by disabling cache. That's not ideal.

I ported the solution to D8. The captcha path is in #2974083: Port to D8: support for cacheable captcha (recaptcha). Adding the recaptcha patch here. The D7 is still RTBC. D8 needs review I guess.

Bot seems to be testing in D7. Re-uploading.

RTBC, but we are back waiting on upstream for 8.x

Failing test is irrelevant.

Failing patches cannot committed.

The failing test is not caused by the patch. It fails in all other d8 issues as well.

Retesting as https://www.drupal.org/node/147903/qa is all green. Maybe a bot issue.

Last test in https://www.drupal.org/node/147903/qa is from 8th of March. Maybe it would fail if it runs now?

How can we test with an automated test if the caching works?

Caching worked for d8 before this patch. It's just that the error was displayed, and it needed to be suppressed.

How can we test that a page is cached? Make an http request to a page with a form. Inspect the headers. There should be a header such as Cache-Control: public, max-age=1800.

It's a bit harder to test for the message, because it appeared the second time that the same cached form was submitted. And also - we would need to solve the recaptcha somehow.

Hopefully it never fails.

Sorry that I keep facing below error message (e.g. at /user/login):

CAPTCHA validation error: unknown CAPTCHA session ID. Contact the site administrator if this problem persists.

Most likely #2893656-8: Getting a "CAPTCHA session reuse attack detected" error. related issue, and so backport changes as below:

diff -u b/recaptcha.module b/recaptcha.module
--- b/recaptcha.module
+++ b/recaptcha.module
@@ -85,9 +85,10 @@
             '#value' => 'Google no captcha',
           ];

-          // As the validate callback does not depend on sid or solution, this
-          // captcha type can be displayed on cached pages.
-          $captcha['cacheable'] = TRUE;
+          // Prevent "CAPTCHA validation error: unknown CAPTCHA session ID"
+          $captcha['cacheable'] = FALSE;
+          $captcha['form']['captcha_response']['#cache'] = ['max-age' => 0];
+          \Drupal::service('page_cache_kill_switch')->trigger();

           // Check if reCAPTCHA use globally is enabled.
           $recaptcha_src = 'https://www.google.com/recaptcha/api.js';

Hopefully this fix the problem :-(

Hi! I've faced with same error, only on '/user/register' page. I use recaptcha 8.x-2.4 and last dev version of captcha module.
In headers I see X-Drupal-Cache: HIT.
Flushing of Drupal's cache fixes errors, but I'm not sure how it can be reproduced.
We don't use any additional caching on the site.
Any thoughts? Thanks!

I see that patch from #54 isn't committed. Should we reopen this issue?

@56

Patch #54 actually disables caching so it doesn't match the title of this issue very well. I don't think this issue should be reopened again as it's too big and has too many different considerations to it. I believe a new issue is better.

I'm also having the same error. The webform is embedded on a field, maybe this could be a problem?

I am using the contact_block module and I have placed a contact form on my contact page. The contact form has a captcha on it and I get the "captcha session reuse attack detected error". It'll be great to have a fix. For now, I'm going to disable the captcha even though it'll make me very sad to do so.