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

Add CSRF Security measures on login forms #40

Merged
merged 6 commits into from
Feb 4, 2014
Merged

Conversation

GeneralZero
Copy link
Contributor

just added the csrf middleware

as I requested in #39

just added the csrf middleware
@GeneralZero
Copy link
Contributor Author

Still in progress of testing don't commit

Changed Views to accept CSRF token
@sahat
Copy link
Owner

sahat commented Feb 3, 2014

I don't know much about CSRF attacks, but is adding hidden input field with csrf token value enough to protect against this attack?
screenshot 2014-02-03 13 11 59

@GeneralZero
Copy link
Contributor Author

CSRF is used to validate that the the page that was requested is the page that the information was sent back from.

This is used so a site cant make a post request to a website without requesting a page first.

When a ajax request is made from an external site it won't automatically do the operation (this may be change username, password, phone number, etc.). This happens because If the user is already signed in then the cookies are also sent in the request.

This is a great site describing CSRF if you want to read it
http://t.co/NjkAVofBlQ

@GeneralZero
Copy link
Contributor Author

I commented it out and forgot to remove it

@sahat
Copy link
Owner

sahat commented Feb 4, 2014

@GeneralZero please let me know when it's ok to merge.

@GeneralZero
Copy link
Contributor Author

It's good to go.

@sahat sahat merged commit dff5aa1 into sahat:master Feb 4, 2014
@dstroot
Copy link

dstroot commented Feb 8, 2014

Deleting your profile was missed - it's broken.

Needs this code:
image

@sahat
Copy link
Owner

sahat commented Feb 8, 2014

@dstroot Thanks for catching it. This should be fixed now.

@GeneralZero
Copy link
Contributor Author

Thanks for that

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.

3 participants