Undefined array key 2 in Drupal\friendlycaptcha\SiteVerify->verify() when using local endpoint

I can't reproduce this.

What's happening there is that the friendlycaptcha_captcha_validation reads the input with $requestStack->getCurrentRequest()->request->get('frc-captcha-solution') and then explodes that value with this:

[$signature, $puzzle, $solutions] = explode('.', $solution);

Your error message indicates that the string doesn't contain 3 parts but only 2.

There is no other operation involved. So, that looks like the request from the browser doesn't send a proper value in frc-captcha-solution, but I don't see how that could be the case. Don't see that this could be related to the local endpoint.

Do you have the friendly-challenge JS library installed, if so, which version?

I'm asking, because the request that should contain frc-captcha-solution is generated by that library.

Do you see any javascript errors in your browser console?

Grevil is on it. We're seeing this in a client project. The friendlycaptcha UI is present and working for me.
Perhaps it happens if JS is disabled, e.g. on crawler access or similar edge cases. We'll provide more information asap.

Yea, I can not reproduce this issue either. If we have JS disabled, The captcha will automatically always fail and a text is displayed to the user, to disable Javascript:

You need Javascript for CAPTCHA verification to submit this form.
This question is for testing whether or not you are a human visitor and to prevent automated spam submissions.

I guess this is caused by some kind of bot, or someone trying to break through the captcha and brute force a login.

But we should fix it nonetheless. On it.

Just had a look at the logs and they seem to be regular users, not bots. From different countries, but none of them seems suspicious.

The MR looks great, except I wouldn't log a critical. Malformed submissions will be negative to the user, but it's not a system failure. I would just log that as info. Anything above warning would trigger an alert in enterprise environments, that's probably not what we want.

Alright, the issue should be fixed now!

I also just realized another few pain points concerning the logging:

• We always log any failures as critical, "info" should be enough, as we don't want a bunch of critical log messages inside the log, just because our site is being attacked by bots.
• We are logging inside verify() and later on again in friendlycaptcha_captcha_validation(). Instead, let's pass a logging message inside returnSolutionInvalid, returnSolutionTimeoutOrDuplicate, returnErrorEmptySolution and returnMalformedSolution to returnResponse, so our returnResponse would look like this: private static function returnResponse(bool $success, string $errorId = NULL, string $error = NULL). Our original "$error" in returnResponse is already an errorId anyway, this way everything is way cleaner.

I'll create a follow up issue for that.

Created the follow-up issue here: 📌 Adjust the logging for the local endpoint implementation Active .

@jurgenhaas I agree! But I would rather clean that up in the follow-up issue! Hence, there are multiple critical log calls that should be info calls in my opinion:

Line 179:

$this->logger->critical('Signature mismatch. Calculated "%calculated", given "%given"', [</li>
        '%calculated' => $calculated,
        '%given' => $signature,
      ]);

Line 187:

$this->logger->critical('Puzzle "%puzzle" was already successfully used before', ['%puzzle' => $puzzleHex]);

Line 198:

      $this->logger->critical('Puzzle is too old (%age seconds, allowed: %expiry', [
        '%age' => $age,
        '%expiry' => $expiry,
      ]);

Line 212:

$this->logger->critical('Solution seen in this request before');

Line 221:

$this->logger->critical($currentSolution . ' (index: ' . $solutionIndex . ') is invalid (' . $first4Int . ' >= ' . $t . ')');

@Grevil sounds good to me.

Thank you all! Agree with both!

So we fix the error, but don't know about the reason for the root cause yet, right?
So should we merge this after @jurgenhaas review but leave it open to check the reason? Or what would you suggest?

Just set to RTBC.

@Anybody I wouldn't worry about the root too much TBH. I could imagine that the same issue has been there for ages, just that it was invisible since the invalid request went to the remote service and triggered the error there instead of locally. Receiving invalid requests on public URLs seems reasonable, so I would probably close this issue if it were mine.

I would agree with @jurgenhaas, @Anybody should do the final decision here, merging...

Thanks everyone! :)

Let's wait with the release after 📌 Adjust the logging for the local endpoint implementation Active is fixed.

I'll be right on it after my break!

PS: Unsure if I already offered that, but @jurgenhaas wouldn't it make sense to have you as Co-Maintainer? ;)

Unsure if I already offered that

You haven't ;-) but that's no problem. I'm happy to collaborate, regardless of being a co-maintainer or not. Not that I didn't have a lot of modules on my list already. I leave that to you, though.

It's out of the question - welcome to the team! :)

Thank you, happy to be part of the team.

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