Delete the captcha registration after retrieving data in Comment/Add and Message/Reply #4417
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…and Message/Reply When a validation error is encountered, a new template with a new captcha will be sent. However the logic within the captcha controller prevents a callback from being added for a specific captcha ID if one is already registered. This leads to the previous captcha callback being reused for another attempt. This does not work, because a single instance of reCAPTCHA may only be used once, thus erroring out if the callback is invoked a second time. Fix this issue by deleting the captcha callback once we used it once. Upon another failure another template will be sent, re-registering a new and valid captcha. It was also considered caching the return value, however this will cause issues if the user mistypes a captcha as they will be unable to correct the error, due to the same value being returned on the next attempt. Ideally the `getData()` function would automatically delete the callback, making it a single-use callback by design. This might break API users relying on this current (broken) behavior, though. The whole (AJAX) CAPTCHA API looks broken beyond repair. It also relies on the jQuery parts being available. It should be cleanly refactored in a future version.
dtdesign
approved these changes
Jul 26, 2021
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
When a validation error is encountered, a new template with a new captcha will
be sent. However the logic within the captcha controller prevents a callback
from being added for a specific captcha ID if one is already registered. This
leads to the previous captcha callback being reused for another attempt.
This does not work, because a single instance of reCAPTCHA may only be used
once, thus erroring out if the callback is invoked a second time.
Fix this issue by deleting the captcha callback once we used it once. Upon
another failure another template will be sent, re-registering a new and valid
captcha.
It was also considered caching the return value, however this will cause issues
if the user mistypes a captcha as they will be unable to correct the error, due
to the same value being returned on the next attempt.
Ideally the
getData()function would automatically delete the callback,making it a single-use callback by design. This might break API users relying
on this current (broken) behavior, though.
The whole (AJAX) CAPTCHA API looks broken beyond repair. It also relies on the
jQuery parts being available. It should be cleanly refactored in a future
version.