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

Reset captcha value #375

Merged
merged 3 commits into from May 21, 2019
Merged

Reset captcha value #375

merged 3 commits into from May 21, 2019

Conversation

degeri
Copy link
Member

@degeri degeri commented May 8, 2019

This will reset the the captcha to "false" after each sensitive request.
fixes #343

This will reset the value of the captcha after each sensitive request fixes decred#343
@dajohi dajohi requested a review from jholdstock May 9, 2019 17:15
Copy link
Member

@jholdstock jholdstock left a comment

Choose a reason for hiding this comment

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

discussed on matrix:

If we want to force the user to complete a captcha for every action which sends an email, we dont really need to store CaptchaDone in the users session. Rather than flipping it between true/false, we can just assume it is always false
We still need c.Env["CaptchaDone"] so the template knows whether to draw the captcha or not, but we dont need session.Values["CaptchaDone"] at all

@degeri
Copy link
Member Author

degeri commented May 20, 2019

Looked at it again.

We cant remove session.Values["CaptchaDone"] right now cause there are are actually two requests 1. Captcha solve -> 2. Sensitive action, this requires that we "track" that a captcha is solved and then invalidate it such that it can only be used once.

For a "proper" fix we need to completely remove c.Env["CaptchaDone"] and session.Values["CaptchaDone"] and load up the captcha and the sensitive form in a single page / request (Would require some UX changes too). That needs be a whole another PR imo.

@jholdstock
Copy link
Member

Discussed further on matrix. Need to set both env and session vars. Also, inside function SettingsPost() we need to set the vars in an else statement, to be consistent with the other two instances.

Should be good to merge after this.

Address @jholdstock comments.
Make travis happy.
@dajohi dajohi merged commit 3d621ef into decred:master May 21, 2019
girino added a commit to girino/dcrstakepool that referenced this pull request Sep 7, 2019
* commit '3d621efb38e1d969b837bdbc0d7992ed9ac1211f':
  Reset captcha value (decred#375)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Reset captcha value in session after signup/passwordreset action
3 participants