Add recaptcha keys to as site-configurable keys #10736

Zhao-Andy · 2020-10-08T13:37:35Z

What type of PR is this? (check all applicable)

Feature

Description

This adds Google ReCAPTCHA keys into SiteConfig, so that Forem admins are able to add ReCAPTCHA without having to go through the .env file.

QA Instructions, Screenshots, Recordings

Visit /admin/config
Find the Google ReCAPTCHA field
Update the field to anything
Verify that it updated

Added tests?

yes

Added to documentation?

to add to forem admin docs

[optional] Are there any post deployment tasks we need to perform?

No

[optional] What gif best describes this PR or how it makes you feel?

Zhao-Andy · 2020-10-08T13:46:45Z

app/views/admin/configs/show.html.erb

@@ -464,6 +464,32 @@
            </div>
          </div>

+          <div class="card mt-3">


I just stuck this under Google Analytics because alphabetical, but maybe we want to make it a required field in the first Forem setup steps?

Maybe we rename the section header to Google Analytics and ReCaptcha to make it easier to find 🙂

Actually I think it would be easier to find as a separate section, and also because I think it's more clear in the docs if these sections are separate. We can revisit in the future though!

Zhao-Andy · 2020-10-08T14:49:02Z

Converted to draft while I add some additional things.

benhalpern · 2020-10-08T15:06:48Z

Looks good to me so far.... I guess this means recaptcha should probably be mandatory? Because certain parts of the site just won't work if this is wrong.

Let's make sure we cover ample instructions. (E.g. v2 vs v3)

Zhao-Andy · 2020-10-08T15:08:30Z

@benhalpern we started a discussion in Slack and in a different repo, but basically I made it optional and added to the admin docs with instructions.

Ridhwana · 2020-10-08T14:37:16Z

app/views/admin/configs/show.html.erb

@@ -464,6 +464,32 @@
            </div>
          </div>

+          <div class="card mt-3">


Maybe we rename the section header to Google Analytics and ReCaptcha to make it easier to find 🙂

Ridhwana · 2020-10-08T19:03:21Z

app/controllers/feedback_messages_controller.rb

@@ -31,7 +31,9 @@ def create
  private

  def recaptcha_verified?
-    recaptcha_params = { secret_key: ApplicationConfig["RECAPTCHA_SECRET"] }
+    return true if SiteConfig.recaptcha_site_key.blank? && SiteConfig.recaptcha_secret_key.blank?


@Zhao-Andy perhaps its late and i'm tired but isn't recaptcha_verified? supposed to return false if the SiteConfig.recaptcha_site_key or secret key is blank?

All good :)

I want to return true because if they're blank, we're assuming they don't want to use / haven't set recaptcha. This allows them to submit abuse reports without recaptcha.

I was thinking maybe this should be another SiteConfig variable that's just a boolean, like SiteConfig.recaptcha_enabled but that seemed unnecessary.

(non-blocking, suggestion): hmm I see what you mean. I think I was confused by the method name vs. what we doing in that specific line of code. When we return true from that line, it's not because it was verified but rather because we don't want to use recaptcha which makes the naming of the function confusing.

Perhaps there's some way to make this easier to read, like extracting that condition into its own descriptive function.

app/lib/constants/site_config.rb

Co-authored-by: Julianna Tetreault <32834804+juliannatetreault@users.noreply.github.com>

app/lib/constants/site_config.rb

Co-authored-by: Julianna Tetreault <32834804+juliannatetreault@users.noreply.github.com>

app/views/admin/configs/show.html.erb

Co-authored-by: Julianna Tetreault <32834804+juliannatetreault@users.noreply.github.com>

juliannatetreault

Left a few small, non-blocking comments, but otherwise, everything is looking great!

app/views/admin/configs/show.html.erb

Co-authored-by: Julianna Tetreault <32834804+juliannatetreault@users.noreply.github.com>

Ridhwana

This looks great! Thanks @Zhao-Andy 🚀

juliannatetreault

Thanks for making these changes and writing the thorough docs for this. I tested everything locally and it worked great! 🎉

jacobherrington

Looks good! One question, but it's probably not blocking.

jacobherrington · 2020-10-08T20:44:09Z

app/controllers/feedback_messages_controller.rb

  def recaptcha_verified?
-    recaptcha_params = { secret_key: ApplicationConfig["RECAPTCHA_SECRET"] }
+    recaptcha_params = { secret_key: SiteConfig.recaptcha_secret_key }


jacobherrington · 2020-10-08T20:44:49Z

app/controllers/feedback_messages_controller.rb

@@ -30,8 +30,12 @@ def create

  private

+  def recaptcha_disabled?
+    SiteConfig.recaptcha_site_key.blank? && SiteConfig.recaptcha_secret_key.blank?


What happens if one of these is set and not the other? Does recaptcha need both?

Yeah, it needs both. The site_key is used to display the tag, and the secret_key is used to validate the request.

So should this be SiteConfig.recaptcha_site_key.blank? || SiteConfig.recaptcha_secret_key.blank?

I might be misunderstanding, but if only one is supplied this method should return true, right?

* Add recaptcha keys to as site-configurable keys * Add recaptcha to site config view * Use site config key * Make ReCAPTCHA tag optional if keys are blank * Use proper capitalization of recaptcha Co-authored-by: Julianna Tetreault <32834804+juliannatetreault@users.noreply.github.com> * Refactor logic for readability * Use proper capitalization of recaptcha Co-authored-by: Julianna Tetreault <32834804+juliannatetreault@users.noreply.github.com> * Use proper capitalization of recaptcha Co-authored-by: Julianna Tetreault <32834804+juliannatetreault@users.noreply.github.com> * Use proper capitalization of recaptcha Co-authored-by: Julianna Tetreault <32834804+juliannatetreault@users.noreply.github.com> Co-authored-by: Julianna Tetreault <32834804+juliannatetreault@users.noreply.github.com>

* chore: update the profile to be more dynamic. Maps out the group + field name + value for field * feat: add the correct input type to the form tying it to the correct method to update * feat: update the value and the label * feat: update the profile with a submit * refactor: ensure we dont show empty groups on the settings/profile * fix: this shoudl be dribbble * fix: use attributes! to refresh the cache to ensure that we font run into the case where Profile.attributes is [] * feat: show group description only when it exists * chore: add some classes * feat: use the profile update service to update the relevant user information * chore: rename variable * chore: use a scope to not return empty groups * fix: only update user if the profile sync is update is successful. Also capture errors * feat: style the crayons field checkbox * refactor: rename the attribute to user instead of profile * refactor: service object * Fix CodeClimate warning and rearrange code * Fix service object, add missing attribute * Fix flash and add redirect * Fix service object + spec * Fix user edits profile spec * Disable spec * Fix more specs * Undo vim fail * Add data update scripts * Update where clause in data update script * Rename method * Make sync_to_user conditional * Add after update actions and Honeycomb * feat: handle checkbox allow chcekbox to check * Don't set experience cookie in ProfilesController * Clean up Profiles::Update object * Fix spec * Remove Twitch integration (#10732) * Remove Twitch integration * Ignore columns * Change admin password in docs (#10735) Update default admin password in documentation * Do not require meta_keywords to be set (#10721) * Remove extra character from meta_keywords in /listings/index.html.erb * Remove meta_keywords from MANDATORY_CONFIGS * Add and use meta_keywords_tag helper * Use modern tag syntax instead of deprecated syntax * Add and use meta_keywords_default helper * Add and use meta_keywords_article helper * Remove * from meta_keywords_field.label * Update meta_keyword specs to account for no keywords being set * Generalizes Connect welcome message and settings nav tab and refactors tests (#10741) [deploy] * [deploy] fix: Clear chat input after Enter key submit (#10487) * fix: Clear chat input after Enter key submit * fix: Rename Composer component * test: Add integration tests for input value * After pressing Enter * After clicking Send * After clicking Save edit * After clicking Close edit * Remove tabindex of -1 on upload image input in the write a post editor (#10543) * Remove tabindex of -1 on upload image input in the write a post editor * Remove negative tabindex on image uploader's outer button * Give tabindex of -1 to outer button on image uploader * Update buttons.scss * [deploy] Add recaptcha keys to as site-configurable keys (#10736) * Add recaptcha keys to as site-configurable keys * Add recaptcha to site config view * Use site config key * Make ReCAPTCHA tag optional if keys are blank * Use proper capitalization of recaptcha Co-authored-by: Julianna Tetreault <32834804+juliannatetreault@users.noreply.github.com> * Refactor logic for readability * Use proper capitalization of recaptcha Co-authored-by: Julianna Tetreault <32834804+juliannatetreault@users.noreply.github.com> * Use proper capitalization of recaptcha Co-authored-by: Julianna Tetreault <32834804+juliannatetreault@users.noreply.github.com> * Use proper capitalization of recaptcha Co-authored-by: Julianna Tetreault <32834804+juliannatetreault@users.noreply.github.com> Co-authored-by: Julianna Tetreault <32834804+juliannatetreault@users.noreply.github.com> * Add Section for Series/Collections (#10719) * Add Section for Series/Collections I added a small section in this area of the docs to share some information about collections/ series. There is not much information in the documentation about collections or series and I felt like this would be beneficial to others looking to help with the Series feature of the website. * Updated wording of collections section. * Articles API: filtering by tags (#3654) (#10614) * Articles API: filtering by tags (#3654) * Address CR comments (#3654) * Add test case * Move tests * Update docs * Use _any_ option for querying by tags (#3654) * Move Imgproxy endpoint config to Images::Optimizer (#10742) * Link Email Account creation UI to existing Create Account UI (#10746) [deploy] * Conditional button to link to email accout creation * Rename email signup state param * Test for email account creation button scenarios * Add forem meta tags (#10747) * Add forem meta tags * Add docs * Update docs/technical-overview/compatibility.md Co-authored-by: Jacob Herrington <jacobherringtondeveloper@gmail.com> * Update docs/technical-overview/compatibility.md Co-authored-by: Jacob Herrington <jacobherringtondeveloper@gmail.com> * Update docs/technical-overview/compatibility.md Co-authored-by: Jacob Herrington <jacobherringtondeveloper@gmail.com> * Update docs/technical-overview/compatibility.md Co-authored-by: Jacob Herrington <jacobherringtondeveloper@gmail.com> * Remove Twitch-related columns from users table (#10750) * [deploy] Long text goes outside cards (#10740) * Add word-break to crayons-card Add word-break to crayons-card this will allow words to be broken if the word is too long but will wrap normally otherwise. * Use overflow-wrap: anywhere; `break-word` is deprecated though `anywhere` does not work in IE * Removes superfluous sign_in from users_onboarding_spec.rb (#10757) [deploy] * [deploy] Allow glitch id starting with ~ (#10544) * [deploy] crayons prep (#10737) * Add DEV special case for following hiring tag * Move image validation to Profiles::Update object * spec: move over the tests for validating images * Move follow_hiring_tag into the service object Co-authored-by: Ridhwana <ridhwana.khan16@gmail.com> Co-authored-by: Andrew Bone <AndrewB05@gmail.com> Co-authored-by: Vaidehi Joshi <vaidehi.sj@gmail.com> Co-authored-by: Julianna Tetreault <32834804+juliannatetreault@users.noreply.github.com> Co-authored-by: Robin Gagnon <me@reobin.dev> Co-authored-by: Manaswini <58681405+Manaswini1832@users.noreply.github.com> Co-authored-by: Andy Zhao <17884966+Zhao-Andy@users.noreply.github.com> Co-authored-by: Christopher Wray <53663762+cwray-tech@users.noreply.github.com> Co-authored-by: Rafal Trojanowski <rt.trojanowski@gmail.com> Co-authored-by: Mac Siri <mac@dev.to> Co-authored-by: Josh Puetz <josh@dev.to> Co-authored-by: Ben Halpern <bendhalpern@gmail.com> Co-authored-by: Jacob Herrington <jacobherringtondeveloper@gmail.com> Co-authored-by: irmela <irmela@berlin-coding.de> Co-authored-by: ludwiczakpawel <ludwiczakpawel@gmail.com>

Zhao-Andy added 2 commits October 7, 2020 17:25

Add recaptcha keys to as site-configurable keys

7013708

Add recaptcha to site config view

1c35fb1

pr-triage bot added the PR: unreviewed bot applied label for PR's with no review label Oct 8, 2020

Zhao-Andy requested review from Ridhwana and juliannatetreault October 8, 2020 13:37

Zhao-Andy commented Oct 8, 2020

View reviewed changes

Zhao-Andy marked this pull request as draft October 8, 2020 14:48

Zhao-Andy added 2 commits October 8, 2020 10:56

Use site config key

1602377

Make ReCAPTCHA tag optional if keys are blank

b7194fd

pr-triage bot added PR: draft bot applied label for PR's that are a work in progress and removed PR: unreviewed bot applied label for PR's with no review labels Oct 8, 2020

Zhao-Andy marked this pull request as ready for review October 8, 2020 15:08

pr-triage bot added PR: unreviewed bot applied label for PR's with no review and removed PR: draft bot applied label for PR's that are a work in progress labels Oct 8, 2020

Zhao-Andy requested a review from jacobherrington October 8, 2020 18:48

Ridhwana reviewed Oct 8, 2020

View reviewed changes

juliannatetreault reviewed Oct 8, 2020

View reviewed changes

app/lib/constants/site_config.rb Outdated Show resolved Hide resolved

Zhao-Andy and others added 2 commits October 8, 2020 15:33

Use proper capitalization of recaptcha

aa18e92

Co-authored-by: Julianna Tetreault <32834804+juliannatetreault@users.noreply.github.com>

Refactor logic for readability

b2a29d6

juliannatetreault reviewed Oct 8, 2020

View reviewed changes

app/lib/constants/site_config.rb Outdated Show resolved Hide resolved

Use proper capitalization of recaptcha

db38b4c

Co-authored-by: Julianna Tetreault <32834804+juliannatetreault@users.noreply.github.com>

juliannatetreault reviewed Oct 8, 2020

View reviewed changes

app/views/admin/configs/show.html.erb Outdated Show resolved Hide resolved

Use proper capitalization of recaptcha

3490ff6

Co-authored-by: Julianna Tetreault <32834804+juliannatetreault@users.noreply.github.com>

juliannatetreault reviewed Oct 8, 2020

View reviewed changes

app/views/admin/configs/show.html.erb Outdated Show resolved Hide resolved

app/views/admin/configs/show.html.erb Outdated Show resolved Hide resolved

Use proper capitalization of recaptcha

9922711

Co-authored-by: Julianna Tetreault <32834804+juliannatetreault@users.noreply.github.com>

Ridhwana approved these changes Oct 8, 2020

View reviewed changes

pr-triage bot removed the PR: unreviewed bot applied label for PR's with no review label Oct 8, 2020

pr-triage bot added the PR: partially-approved bot applied label for PR's where a single reviewer approves changes label Oct 8, 2020

juliannatetreault approved these changes Oct 8, 2020

View reviewed changes

jacobherrington reviewed Oct 8, 2020

View reviewed changes

pr-triage bot added PR: reviewed-approved bot applied label for PR's where reviewer approves changes and removed PR: partially-approved bot applied label for PR's where a single reviewer approves changes labels Oct 8, 2020

Zhao-Andy merged commit 6c20a70 into forem:master Oct 8, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add recaptcha keys to as site-configurable keys #10736

Add recaptcha keys to as site-configurable keys #10736

Zhao-Andy commented Oct 8, 2020

Zhao-Andy Oct 8, 2020

Ridhwana Oct 8, 2020 •

edited

Zhao-Andy Oct 8, 2020

Zhao-Andy commented Oct 8, 2020

benhalpern commented Oct 8, 2020

Zhao-Andy commented Oct 8, 2020

Ridhwana Oct 8, 2020 •

edited

Ridhwana Oct 8, 2020 •

edited

Zhao-Andy Oct 8, 2020

Ridhwana Oct 8, 2020

Zhao-Andy Oct 8, 2020

juliannatetreault left a comment

Ridhwana left a comment

juliannatetreault left a comment

jacobherrington left a comment

jacobherrington Oct 8, 2020

jacobherrington Oct 8, 2020

Zhao-Andy Oct 8, 2020

jacobherrington Oct 9, 2020 •

edited

Add recaptcha keys to as site-configurable keys #10736

Add recaptcha keys to as site-configurable keys #10736

Conversation

Zhao-Andy commented Oct 8, 2020

What type of PR is this? (check all applicable)

Description

QA Instructions, Screenshots, Recordings

Added tests?

Added to documentation?

[optional] Are there any post deployment tasks we need to perform?

[optional] What gif best describes this PR or how it makes you feel?

Choose a reason for hiding this comment

Ridhwana Oct 8, 2020 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Zhao-Andy commented Oct 8, 2020

benhalpern commented Oct 8, 2020

Zhao-Andy commented Oct 8, 2020

Ridhwana Oct 8, 2020 • edited

Choose a reason for hiding this comment

Ridhwana Oct 8, 2020 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

juliannatetreault left a comment

Choose a reason for hiding this comment

Ridhwana left a comment

Choose a reason for hiding this comment

juliannatetreault left a comment

Choose a reason for hiding this comment

jacobherrington left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jacobherrington Oct 9, 2020 • edited

Choose a reason for hiding this comment

Ridhwana Oct 8, 2020 •

edited

Ridhwana Oct 8, 2020 •

edited

Ridhwana Oct 8, 2020 •

edited

jacobherrington Oct 9, 2020 •

edited