Skip to content
This repository was archived by the owner on Mar 26, 2025. It is now read-only.

Add recaptcha #1637

Merged
merged 6 commits into from
Jan 27, 2022
Merged

Add recaptcha #1637

merged 6 commits into from
Jan 27, 2022

Conversation

Pomax
Copy link
Contributor

@Pomax Pomax commented Jan 25, 2022

This PR covers the client-side recaptcha work outlined in mozilla/network-pulse-api#762

This gates the login route behind recaptcha validation, if USE_RECAPTCHA is enabled.

Requires mozilla/network-pulse-api#766 on the API-side

Testing

  • start with the test instructions on Add recaptcha to the API server network-pulse-api#766
  • clone the repo
  • make sure you have Node v16 installed, or if you (wisely) use nvm, set it to use v16 for the duration of testing. v17 upgraded its SSL library in a way that broke a million codebases and making things Node 17 compatible is a whole other can of worms.
  • run npm install
  • create an .env file and set it to:
PULSE_API=http://localhost:8000/api/pulse
PULSE_API_HOST=http://localhost:8000
PULSE_LOGIN_URL=http://localhost:8000/login
USE_RECAPTCHA=True
RECAPTCHA_KEY=therecaptchasitekey

where the reCAPTCHA site key is the localhost (v3) site key found on https://www.google.com/recaptcha/admin

  • run npm start

You should now be able to load up http://localhost:3000 and click the signin/signup link in the nav bar. Doing so should show you print statements pertaining to recaptcha in your API server terminal fly by. If all went well, you should now be asked to pick how you want to authenticate. Note: that is all you need to see. Actually logging in won't work (due to the gsuite callback URL not being a localhost URL for obvious reasons =D)

Please play around with what happens with USE_RECAPTCHA turned off only on the client, only on the server, and off for both: this should fail, pass, and pass respectively.

@Pomax Pomax temporarily deployed to network-puls-disable-us-ly4ejk January 25, 2022 21:53 Inactive
@Pomax Pomax temporarily deployed to network-puls-disable-us-ly4ejk January 26, 2022 00:32 Inactive
Copy link
Contributor

@danielfmiranda danielfmiranda left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi Pomax, just tested this alongside the other PR and can confirm that I get the expected results when using recaptha and also when turning it off for one app or the other, as well as both at the same time.

Approving!

@tbrlpld
Copy link

tbrlpld commented Jan 26, 2022

I was surprised to find that USE_RECAPTCHA=False does not deactivate recaptcha. It still appears to work. Removing the key from the .env file does have the intended effect.

@tbrlpld
Copy link

tbrlpld commented Jan 26, 2022

I can also confirm the expected behaviour

@Pomax Pomax temporarily deployed to network-puls-disable-us-ly4ejk January 27, 2022 17:26 Inactive
@Pomax Pomax temporarily deployed to network-puls-disable-us-ly4ejk January 27, 2022 17:31 Inactive
@Pomax Pomax merged commit 9c99295 into master Jan 27, 2022
@Pomax Pomax deleted the disable-user-signup branch January 27, 2022 17:32
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants