Add optional reCAPTCHA for registration #335
Conversation
| @@ -24,7 +24,7 @@ RUN \ | |||
| alembic \ | |||
| "coloredlogs==5.0" \ | |||
| youtube-dl \ | |||
| && apk --no-cache del py3-pip | |||
| requests | |||
There was a problem hiding this comment.
Deleting pip caused the requests library to not be importable.
There was a problem hiding this comment.
You should install py3-requests from the alpine package manager instead of using pip.
Leave the call to uninstall pip, it slims down the final build image.
| @@ -22,6 +22,7 @@ | |||
| <link rel='apple-touch-startup-image' href='img/apple-touch-startup-image-1668x2224.png' media='(min-device-width: 834px) and (max-device-width: 834px) and (-webkit-min-device-pixel-ratio: 2) and (orientation: portrait)'/> | |||
| <link rel='apple-touch-startup-image' href='img/apple-touch-startup-image-2048x2732.png' media='(min-device-width: 1024px) and (max-device-width: 1024px) and (-webkit-min-device-pixel-ratio: 2) and (orientation: portrait)'/> | |||
| <link rel='manifest' href='manifest.json'/> | |||
| <script src="https://www.google.com/recaptcha/api.js"></script> | |||
There was a problem hiding this comment.
I would prefer that we don't load the reCAPTCHA js library if the feature is disabled.
There was a problem hiding this comment.
Not entirely sure how to do that on the base template to be honest.
|
Please address my review comments |
|
@sgsunder Check that you submitted the review, I don't see anything |
| @@ -22,6 +22,7 @@ | |||
| <link rel='apple-touch-startup-image' href='img/apple-touch-startup-image-1668x2224.png' media='(min-device-width: 834px) and (max-device-width: 834px) and (-webkit-min-device-pixel-ratio: 2) and (orientation: portrait)'/> | |||
| <link rel='apple-touch-startup-image' href='img/apple-touch-startup-image-2048x2732.png' media='(min-device-width: 1024px) and (max-device-width: 1024px) and (-webkit-min-device-pixel-ratio: 2) and (orientation: portrait)'/> | |||
| <link rel='manifest' href='manifest.json'/> | |||
| <script src="https://www.google.com/recaptcha/api.js"></script> | |||
There was a problem hiding this comment.
I don't think this is a good way to import this library, we should instead be adding it to vendor.min.js
- Find the recaptcha library on npm: https://www.npmjs.com/search?q=recaptcha
- Add library to
package.jsonandpackage-lock.json - Add library to the
external_jsarray inbuild.js
There was a problem hiding this comment.
Also, please make sure that whatever recaptcha library you use is GPLv3 compatible
| @@ -38,6 +38,7 @@ | |||
|
|
|||
| <div class='messages'></div> | |||
| <div class='buttons'> | |||
| <% if(ctx.enableRecaptcha) print(`<div id="recaptcha"></div><br>`); %> | |||
There was a problem hiding this comment.
Better way:
<% if(ctx.enableRecaptcha) { %>
<div id="recaptcha"></div><br>
<% } %>
| uri.formatApiLink("user", this._orig._name), | ||
| detail, | ||
| files | ||
| ) | ||
| : api.post(uri.formatApiLink("users"), detail, files); |
There was a problem hiding this comment.
Please install and enable pre-commit so that the autoformatting tools will maintain consistent formatting
| @@ -24,7 +24,7 @@ RUN \ | |||
| alembic \ | |||
| "coloredlogs==5.0" \ | |||
| youtube-dl \ | |||
| && apk --no-cache del py3-pip | |||
| requests | |||
There was a problem hiding this comment.
You should install py3-requests from the alpine package manager instead of using pip.
Leave the call to uninstall pip, it slims down the final build image.
| "secret": config.config["recaptcha_secret"], | ||
| "response": ctx.get_param_as_string("recaptchaToken", default=""), | ||
| }) | ||
|
|
There was a problem hiding this comment.
Can we add error handling if the call to the Google API fails (like if their site is down and throws a HTTP 504)?
|
Regarding the Docker Cloud CI failure: Make sure that any changes you make to |
This added a recaptcha button to the registration page. The button only appears if recaptcha is enabled in the config and the user is not logged in.
I included a couple links in the config to the recaptcha docs. All that's needed to use it is to enable it in the config and provide the two tokens. By default the recaptcha is disabled.
Captcha
Captcha error message