Skip to content
This repository has been archived by the owner on Nov 25, 2024. It is now read-only.

Switch the default config option values for recaptcha_sitekey_class and recaptcha_form_field #2939

Merged
merged 2 commits into from
Jan 16, 2023

Conversation

anoadragon453
Copy link
Member

Attempting to use the web auth fallback mechanism for Google ReCAPTCHA with the default setting for client_api.recaptcha_sitekey_class of "g-recaptcha-response" results in no captcha being rendered:

image

I cross-checked the captcha code between dendrite.matrix.org's fallback page and matrix-client.matrix.org's one (which both use the same captcha public key) and noticed a discrepancy in the class attribute of the div that renders the captcha. ReCAPTCHA's docs state to use "g-recaptcha" as the class for the submit button.

I noticed this when user @parappanon:parappa.party reported that they were also seeing no captcha being rendered on their Dendrite instance. Changing client_api.recaptcha_sitekey_class to "g-recaptcha" caused their captcha to render properly as well.

There may have been a change in the class name from ReCAPTCHA v2 to v3? The docs for v2 also request one uses "g-recaptcha" though.

Thus I propose changing the default setting to unbreak people's recaptcha auth fallback pages. Should fix dendrite.matrix.org as well.

…recaptcha`

This appears to unbreak recaptcha rendering on recaptcha web auth fallback pages.
@anoadragon453 anoadragon453 requested a review from a team as a code owner January 14, 2023 16:34
@codecov
Copy link

codecov bot commented Jan 14, 2023

Codecov Report

Base: 36.56% // Head: 36.60% // Increases project coverage by +0.04% 🎉

Coverage data is based on head (697403b) compared to base (477a44f).
Patch coverage: 100.00% of modified lines in pull request are covered.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2939      +/-   ##
==========================================
+ Coverage   36.56%   36.60%   +0.04%     
==========================================
  Files         494      494              
  Lines       54676    54676              
==========================================
+ Hits        19993    20016      +23     
+ Misses      32114    32098      -16     
+ Partials     2569     2562       -7     
Flag Coverage Δ
unittests 36.60% <100.00%> (+0.04%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
setup/config/config_clientapi.go 73.13% <100.00%> (ø)
syncapi/streams/stream_pdu.go 30.98% <0.00%> (-0.24%) ⬇️
roomserver/internal/query/query.go 25.79% <0.00%> (+0.43%) ⬆️
roomserver/storage/shared/storage.go 45.45% <0.00%> (+0.56%) ⬆️
userapi/storage/postgres/account_data_table.go 82.69% <0.00%> (+1.92%) ⬆️
userapi/consumers/roomserver.go 43.38% <0.00%> (+2.37%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@anoadragon453 anoadragon453 changed the title Change default ReCAPTCHA div class from g-recaptcha-response to g-recaptcha Switch the default config option values for recaptcha_sitekey_class and recaptcha_form_field Jan 14, 2023
@anoadragon453
Copy link
Member Author

@S7evinK I've updated this PR to also change the default of the client_api.recaptcha_form_field option. My theory is that the default values for the recaptcha_sitekey_class and recaptcha_form_field options were accidentally swapped.

@anoadragon453 anoadragon453 requested a review from S7evinK January 14, 2023 18:23
@anoadragon453
Copy link
Member Author

@S7evinK CI failures appear unrelated to the PR. Also heads up I don't have perms to merge this PR, if you're waiting on me to do so :)

@S7evinK
Copy link
Contributor

S7evinK commented Jan 16, 2023

Yep, they are related to matrix-org/complement@8550c22 I think (at least for Complement)

@S7evinK S7evinK added T-Defect Bugs, crashes, hangs, security vulnerabilities, or other reported issues. S-Minor Blocks non-critical functionality, workarounds exist. labels Jan 16, 2023
@S7evinK S7evinK merged commit eeeb301 into matrix-org:main Jan 16, 2023
@anoadragon453 anoadragon453 deleted the patch-1 branch January 16, 2023 11:52
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
S-Minor Blocks non-critical functionality, workarounds exist. T-Defect Bugs, crashes, hangs, security vulnerabilities, or other reported issues.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants