Join GitHub today
GitHub is home to over 20 million developers working together to host and review code, manage projects, and build software together.
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
Already on GitHub? Sign in to your account
Log the hostname the reCAPTCHA was completed on #946
+11
−2
Conversation
Some checks were not successful
Sytest Dendron (Commit)
— Build #325 origin/dbkr/log_recaptcha_hostname failed in 6 min 45 sec
Sytest Postgres (Commit)
— Build #1160 origin/dbkr/log_recaptcha_hostname failed in 6 min 21 sec
Sytest SQLite (Commit)
— Build #1184 origin/dbkr/log_recaptcha_hostname failed in 5 min 22 sec
Flake8 + Packaging (Commit)
— Build #1213 origin/dbkr/log_recaptcha_hostname succeeded in 37 sec
Unit Tests (Commit)
— Build #1250 origin/dbkr/log_recaptcha_hostname succeeded in 2 min 14 sec
dbkr
assigned
NegativeMjark
Jul 22, 2016
NegativeMjark
commented on an outdated diff
Jul 22, 2016
synapse/handlers/auth.py
| @@ -279,8 +279,17 @@ def _check_recaptcha(self, authdict, clientip): | ||
| data = pde.response | ||
| resp_body = simplejson.loads(data) | ||
| - if 'success' in resp_body and resp_body['success']: | ||
| - defer.returnValue(True) | ||
| + if 'success' in resp_body: | ||
| + # Note that we do NOT check the hostname here: we explicitly | ||
| + # intend the CAPTCHA to be presented by whatever client the | ||
| + # user is using, we just care that they have completed a CAPTCHA. | ||
| + logger.info( | ||
| + "%s reCAPTCHA from hostname %s", | ||
| + "Successful" if resp_body['success'] else "Failed", | ||
| + resp_body['hostname'] |
NegativeMjark
Contributor
|
|
LGTM other than the possibility for KeyErrors |
Some checks were not successful
Sytest Dendron (Commit)
— Build #326 origin/dbkr/log_recaptcha_hostname failed in 6 min 24 sec
Flake8 + Packaging (Commit)
— Build #1214 origin/dbkr/log_recaptcha_hostname succeeded in 34 sec
Sytest Postgres (Commit)
— Build #1161 origin/dbkr/log_recaptcha_hostname succeeded in 6 min 53 sec
Sytest SQLite (Commit)
— Build #1185 origin/dbkr/log_recaptcha_hostname succeeded in 5 min 47 sec
Unit Tests (Commit)
— Build #1251 origin/dbkr/log_recaptcha_hostname succeeded in 2 min 29 sec
|
Done. Are these tests still 'expected' to fail? |
No, but they're now failing due to changes that were snuck in while the CI was broken :(. |
|
LGTM |
dbkr
merged commit 4fcdf7b
into
develop
Jul 25, 2016
8 of 10 checks passed
Sytest Dendron (Commit)
Build #326 origin/dbkr/log_recaptcha_hostname failed in 6 min 24 sec
Details
Sytest Dendron (Merged PR)
Build finished.
Details
Flake8 + Packaging (Commit)
Build #1214 origin/dbkr/log_recaptcha_hostname succeeded in 34 sec
Details
Flake8 + Packaging (Merged PR)
Build finished.
Details
Sytest Postgres (Commit)
Build #1161 origin/dbkr/log_recaptcha_hostname succeeded in 6 min 53 sec
Details
Sytest Postgres (Merged PR)
Build finished.
Details
Sytest SQLite (Commit)
Build #1185 origin/dbkr/log_recaptcha_hostname succeeded in 5 min 47 sec
Details
Sytest SQLite (Merged PR)
Build finished.
Details
Unit Tests (Commit)
Build #1251 origin/dbkr/log_recaptcha_hostname succeeded in 2 min 29 sec
Details
Unit Tests (Merged PR)
Build finished.
Details
richvdh
deleted the
dbkr/log_recaptcha_hostname
branch
Dec 1, 2016
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
dbkr commentedJul 22, 2016
This could be useful information to have in the logs. Also comment about how & why we don't verify the hostname.