update request_forgery_protection docs

[donbobka]

I made some documentation changes in request_forgery_protection.rb:

1. Due to protect_from_forgery by default use :null_session method, some lines of documentation are obsolete.
2. Change controller name from ApplicationController to ApiController. ApplicationController can mislead developers, because usually rails application consist of non-api controllers and api controllers

[rafaelfranca]

It is better to keep ApplicationController. Even if the application consist on api an non-api it is still correct and doesn't need to explain what is a ApiController and how it would be used.

[donbobka]

Agree

[donbobka]

I've just updated commit

[donbobka]

I've just updated docs:

• delete action some_api_action from ApplicationController, because actions in ApplicationController is bad practice.
• change explanation, what you should do in case of api application

[sponomarev]

👍

[donbobka]

IMO following sentence is wrong:

Only HTML and JavaScript requests are checked, so this will not protect your XML API (presumably you'll have a different authentication scheme there anyway).

From here

In code there is no limitation to CSRF-token checks only for html/javascript

[rafaelfranca]

Yes. I think you are right. @jeremy do you know if this sentence is true (or were)?

[jeremy]

It is true. Only requests that maintain a session are checked, which means HTML for most apps. APIs typically aren't session oriented, so there will be no CSRF token to check against: https://github.com/rails/rails/blob/master/actionpack/lib/action_controller/metal/request_forgery_protection.rb#L248-L262

[donbobka]

@jeremy, in code that you noticed there is no limitation for html or javascript, besides exclusion HEAD and GET requests:

!protect_against_forgery? || request.get? || request.head?

APIs typically aren't session oriented, so there will be no CSRF token to check against

Rails has a good method for API CSRF protection: :null_session

Look at commit 6efc5bf where this sentence appeared. Version before changes is more truthful, because it's that what makes the code.

All requests are checked except GET requests as these should be idempotent.

IMO: First and second paragraphs should be about standard CSRF protection that you will get in new rails application. And third paragraph should explain API case of rails usage.

[jeremy]

Correct, there is no explicit format check in the code.

The docs intend to communicate that session-oriented requests, like those made from a browser, are verified for authenticity.

That happens to include HTML and JS requests, but it isn't limited to these. It's only written this way to be understandable to Rails users: that "browser requests" need CSRF checks. The docs may certainly be updated to to reflect this. They should be accurate about how CSRF decisions are made but should be very easy to understand.

[donbobka]

Why are JS and HTML requests highlights here as something special?

What about a following variant:

All requests are checked except GET requests as these should be idempotent. Keep in mind that all session-oriented requests should be CSRF protected, including Javascript and HTML requests.`

[sponomarev]

@donbobka looks very sensible and clear

[donbobka]

@rafaelfranca What do you think about @a86ee8a?

[rafaelfranca]

Sorry for the delay of 4 years 😱! Can you rebase this PR?

[donbobka]

@rafaelfranca I rebased it, but there are no changes against current master. And I see that changes already merged
https://github.com/rails/rails/blame/master/actionpack/lib/action_controller/metal/request_forgery_protection.rb#L18

So, I think we can close it.

[rafaelfranca]

Cool! I'll close. Thanks