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

Return 403 instead of 500 if no CSRF token given #3134

Merged
merged 2 commits into from Jul 13, 2014

Conversation

Projects
None yet
2 participants
Contributor

kdazzle commented Jul 6, 2014

Not supplying a CSRF token shouldn't return a 500 response because it isn't a server error. The response status code should definitely be in the 400's, because it's the client's fault. And it should be a 403 because the client is forbidden from making that request without the appropriate credential (the CSRF token), though the request may be otherwise valid.

http://en.wikipedia.org/wiki/List_of_HTTP_status_codes

@kdazzle kdazzle Return 403 instead of 500 if no CSRF token given
Not supplying a CSRF token shouldn't return a 500 response because it isn't a server error. The response status code should definitely be in the 400's, because it's the client's fault. And it should be a 403 because the client is forbidden from making that request without the appropriate credential (the CSRF token), though the request may be otherwise valid.

http://en.wikipedia.org/wiki/List_of_HTTP_status_codes
05fcc09
Contributor

narfbg commented Jul 7, 2014

Makes sense. Could you add a changelog entry for this and remove the empty line at EOF?

Contributor

kdazzle commented Jul 13, 2014

@narfbg Thanks for the response! I removed that line and stuck an entry under the Security section of the changelog.

@narfbg narfbg added a commit that referenced this pull request Jul 13, 2014

@narfbg narfbg Merge pull request #3134 from kdazzle/patch-1
Return 403 instead of 500 if no CSRF token given
466af6c

@narfbg narfbg merged commit 466af6c into bcit-ci:develop Jul 13, 2014

1 check passed

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

@kdazzle kdazzle deleted the kdazzle:patch-1 branch Jul 13, 2014

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment