Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

replace recaptcha with self-hosted captcha #281

Merged
merged 3 commits into from Feb 8, 2019

Conversation

Projects
None yet
4 participants
@chappjc
Copy link
Member

commented Nov 21, 2018

This replaces Google's reCAPTCHA with a self hosted captcha.

NOTE: This is needs review and testing. It works for me though.

Create controllers/captcha.go, where the captcha GET and POST handlers
are implemented for retrieving images and verifying responses.
Create CaptchaHandler type with image size spec, and add it as a field
of MainController.
Create ApplyCaptcha captcha middleware.
Add GET route for /captchas for serving the captcha images.
Add POST route for /verifyhuman for verifying captcha responses and
redirecting to the previous page with the session updated.
Modify http handlers to display new captcha: PasswordReset, SignUp, and
Settings.
Modify http handlers to check for captcha verification:
PasswordResetPost, SignUpPost, and SettingsPost.
Unrelated: remove unused empty responseHeaderMap.
Comment on how insane core/(*Application).Route is.

@chappjc chappjc referenced this pull request Nov 22, 2018

Closed

privacy: Self-host CAPTCHA #279

@chappjc chappjc force-pushed the chappjc:captcha-self-hosted branch from 4c49f36 to 996f7bf Nov 23, 2018

@chappjc

This comment has been minimized.

Copy link
Member Author

commented Nov 23, 2018

Force pushed 996f7bf. Tested and working. The pages don't look very pretty, but's that's about the best I can do.

@chappjc

This comment has been minimized.

Copy link
Member Author

commented Nov 23, 2018

Currently it looks a bit like this (showing Password Reset page before solving captcha):

image

Sign up:

image

The idea is the forms protected by captcha don't show up until the captcha is solved. It's not just the forms are not there, of course, but that the POST handler that deals with the usual forms does not process the other non-captcha data. This is the purpose of the ApplyCaptcha middleware.

@chappjc

This comment has been minimized.

Copy link
Member Author

commented Nov 23, 2018

Setting the captcha length (now 6), characters (now just 10 numerals), captcha expiry (now 10 minutes), and other captcha parameters may be worth setting in the future. Although because of limitations in the captcha API, we may just want to fork the captcha repo.

@chappjc
Copy link
Member Author

left a comment

Annotated for other reviewers.

session.Values["CaptchaDone"] = true
status = http.StatusFound
} else {
session.Values["CaptchaDone"] = false

This comment has been minimized.

Copy link
@chappjc

chappjc Nov 23, 2018

Author Member

Should probably do a session.AddFlash("Captcha verification failed. Try again.") here too

"github.com/zenazn/goji/web"
)

type CaptchaHandler struct {

This comment has been minimized.

Copy link
@chappjc

chappjc Nov 23, 2018

Author Member

I need to doc all these exported things. And copyright the file.

@@ -5,6 +5,7 @@ import (
"encoding/hex"
"errors"
"fmt"
"github.com/dchest/captcha"

This comment has been minimized.

Copy link
@chappjc

chappjc Nov 23, 2018

Author Member

This import should be with the other github imports, not with the std lib imports.

@@ -145,6 +143,11 @@ func NewMainController(params *chaincfg.Params, adminIPs []string,
return nil, err
}

ch := &CaptchaHandler{
ImgHeight: 127,
ImgWidth: 257,

This comment has been minimized.

Copy link
@chappjc

chappjc Nov 23, 2018

Author Member

Pulled out of my ass and could be set in the config file, but that's not really needed now.

if controller.smtpHost == "" {
c.Env["SMTPDisabled"] = true
}
c.Env["CaptchaID"] = captcha.New()

This comment has been minimized.

Copy link
@chappjc

chappjc Nov 23, 2018

Author Member

No reload button next to the captcha. To get a new one, all you have to do is reload the page with the protected forms.

@@ -227,6 +227,10 @@ func runMain() int {
app.Get("/signup", application.Route(controller, "SignUp"))
app.Post("/signup", application.Route(controller, "SignUpPost"))

// Captcha
app.Get("/captchas/*", controller.CaptchaServe)
app.Post("/verifyhuman", controller.CaptchaVerify)

This comment has been minimized.

Copy link
@chappjc

chappjc Nov 23, 2018

Author Member

The application.Route function is a shit show. Not using it.

if err := saveSession(c, w, r); err != nil {
log.Errorf("Can't save session: %v", err)
}

if respHeader, exists := c.Env["ResponseHeaderMap"]; exists {

This comment has been minimized.

Copy link
@chappjc

chappjc Nov 23, 2018

Author Member

This was a relic from a much older feature that was partially ripped out.

@chappjc chappjc force-pushed the chappjc:captcha-self-hosted branch from 05d7cb4 to 9f33e7f Nov 23, 2018

@honeycrypto

This comment has been minimized.

Copy link

commented Nov 24, 2018

Why to overcomplicate things? Recaptcha works great.

@chappjc

This comment has been minimized.

Copy link
Member Author

commented Nov 24, 2018

@honeycrypto Please see #279

@dajohi dajohi changed the title replace recaptcha with self-hosted captcha [WIP] replace recaptcha with self-hosted captcha Jan 11, 2019

@chappjc chappjc changed the title [WIP] replace recaptcha with self-hosted captcha replace recaptcha with self-hosted captcha Jan 30, 2019

@teknico

This comment has been minimized.

Copy link
Contributor

commented Jan 31, 2019

This branch works for me too.

Under a local deployment I was able to sign up with the captcha image: I got the confirmation email and activated the account. Looking at the database I confirmed that the account got id no. 1 (configured as admin), and logging in I was indeed able to see the low-fee ticket page.

(The local deployment consists of one dcrd instance, one dcrwallet for fees, two voting dcrwallet, two stakepoold, one dcrstakepool and one mysqld. To make all the daemons run in the same namespace without virtual machines nor containers, I changed the various listening ports in the daemons' configuration.)

@chappjc

This comment has been minimized.

Copy link
Member Author

commented Feb 1, 2019

Thanks for the feedback @teknico.

@dajohi

This comment has been minimized.

Copy link
Member

commented Feb 1, 2019

@chappjc please rebase on top of current master

@chappjc chappjc force-pushed the chappjc:captcha-self-hosted branch from 9f33e7f to ed129c7 Feb 4, 2019

@chappjc

This comment has been minimized.

Copy link
Member Author

commented Feb 4, 2019

@dajohi rebased

chappjc added some commits Nov 21, 2018

replace recaptcha with self-hosted captcha
Create controllers/captcha.go, where the captcha GET and POST handlers
are implemented for retrieving images and verifying responses.
Create CaptchaHandler type with image size spec, and add it as a field
of MainController.
Create ApplyCaptcha captcha middleware.
Add GET route for /captchas for serving the captcha images.
Add POST route for /verifyhuman for verifying captcha responses and
redirecting to the previous page with the session udpated.
Modify http handlers to display new captcha: PasswordReset, SignIn, and
Settings.
Modify http handlers to check for captcha verification:
PasswordResetPost, SignInPost, and SettingsPost.
Unrelated: remove unused empty responseHeaderMap.
Comment on how insane core/(*Application).Route is.
remove the recaptcha.js script from the footer
Remove the old config file stuff.

@chappjc chappjc force-pushed the chappjc:captcha-self-hosted branch from ed129c7 to 04cc822 Feb 8, 2019

@chappjc

This comment has been minimized.

Copy link
Member Author

commented Feb 8, 2019

@dajohi rebased again

captcha: failure must redirect with StatusFound
Add a flash message in the CaptchaVerify middleware.
@dajohi

dajohi approved these changes Feb 8, 2019

Copy link
Member

left a comment

ok

@dajohi dajohi merged commit 9ec3776 into decred:master Feb 8, 2019

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@chappjc chappjc deleted the chappjc:captcha-self-hosted branch Feb 8, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.