closes CNVS-18870
CNVS-18870 as described in the ticket description is not a bug. see
comments on the ticket for more details. but while investigating and
confirming that, it became obvious that the odd structure and scattered
implementation of the CSRF protection was both making it hard to reason
about and easy to introduce new bugs. after the refactor, we still:
* don't perform CSRF validation on GET requests
* don't perform it on token-authenticated API requests
* do perform it on session-authenticated API requests
* do perform it on non-API requests regardless of authentication method
additionally, we now:
* don't perform CSRF validation on HEAD requests
finally, we _don't_ support a csrf_token in the session anymore. that's
been deprecated forever; we can remove the code now.
test-plan:
- should not perform CSRF validation for:
- GET requests
- token-authenticated POST requests to API endpoints (path prefixed
by /api/) without an authenticity_token parameter or X-CSRF-Token
header
- token-authenticated POST requests to API endpoints even with an
authenticity_token parameter
- token-authenticated POST requests to API endpoints even with an
X-CSRF-Token header
- should perform CSRF validation for:
- POST requests to non-API endpoints
- session-authenticated POST requests to API endpoints
- when CSRF validation should occur, but the user has cookies off:
- POST requests to non-API endpoints should redirect to a "need
cookies" page
- XHR POST requests to non-API endpoints should not redirect
- POST requests to API endpoints should not redirect
Change-Id: I3dbb3a68623bc9d03a3e744a9d4e1f038a32709c
Reviewed-on: https://gerrit.instructure.com/65103
Reviewed-by: Simon Williams <simon@instructure.com>
Tested-by: Jenkins
QA-Review: August Thornton <august@instructure.com>
Product-Review: Jacob Fugal <jacob@instructure.com>
371ec30967