Move new CSRF docs from Classic UI to Backend?

[stevepiercy]

@plone/volto-team did not respond in a timely manner to https://github.com/plone/documentation/pull/1414, so I'm opening this issue here to ask the questions:

We created a new page Cross-Site Request Forgery (CSRF) which is located under "Classic UI".

1. Should this be moved under "Backend"?
2. Should it be updated with any content specific to Volto?

[tiberiuichim]

Sorry, I didn't see that there was input required on the other ticket.

There's no CSRF protection, as far as I know. @sneridagh maybe you know more about this?

[sneridagh]

There is nothing related in the Volto side. There is a good explanation on why this is not needed in the frontend, I can't elaborate now a complete one, but we can try to fill in the blanks. I think the gist of it is that you can't tamper in a link the headers needed to be injected in the API call (token, Accept). Neither the CORS allows you to send anything under another domain thet is JS-based.

@stevepiercy so go ahead and move it under backend.

[stevepiercy]

OK, I'll move it under backend. Let's keep this issue open, though, to ensure we do fill in the blanks.

Here's an abridged curl request when logging into https://demo.plone.org/ with potentially relevant headers that might help fill in the blanks for Volto.

curl 'https://demo.plone.org/++api++/@login' \
  -H 'authority: demo.plone.org' \
  -H 'accept: application/json' \
  -H 'origin: https://demo.plone.org' \
  -H 'sec-fetch-dest: empty' \
  -H 'sec-fetch-mode: cors' \
  -H 'sec-fetch-site: same-origin' \
  -H 'sec-gpc: 1' \

We also need to change some language in the CSRF page, when it gets moved under Backend. It talks about Plone doing automatic CSRF protection, but it sounds like that is done only in Classic UI, not "Plone". Plone 6 has two frontends, so we need to explicitly state it applies only to Classic UI. Is that correct? @jensens?

[jensens]

In the first place Plone does the hard-protection in the backend on database level, no matter who writes (ClassicUI, Volto, Scripts, name it). It is a subscriber on the before-commit event and it gets grip unless it is disabled like described in the docs.

So, question is

• is it disabled for RestAPI somewhere?
• If not, how is it managed?

And whatever is the answer, it needs a good piece of documentation.

[tiberiuichim]

plone.restapi has quite a few lines like this: https://github.com/plone/plone.restapi/blob/b33eeb990fb33957d4a85827ee23cb3aa07368cc/src/plone/restapi/services/content/add.py#L44-L45

[stevepiercy]

It is a subscriber on the before-commit event and it gets grip unless it is disabled like described in the docs.

I'm sorry, "gets grip"? Do you mean "takes hold" or "takes effect"?

plone.restapi has quite a few lines like this: https://github.com/plone/plone.restapi/blob/b33eeb990fb33957d4a85827ee23cb3aa07368cc/src/plone/restapi/services/content/add.py#L44-L45

That looks like CSRF may be disabled, not is absolutely disabled.

https://github.com/plone/plone.protect/blob/master/plone/protect/interfaces.py#L22

[tiberiuichim]

@stevepiercy as far as I understand it, CSRF is specifically disabled by plone.restapi in most of its code that commits to database: add/update content, etc. @tisto can you confirm this?

[davisagli]

My understanding is that CSRF checks are not really needed for a JSON web service as long as the service is adequately validating the request's Content-Type (to make sure the attacker isn't sending JSON from a web form misidentified as text/plain) and as long as the server hasn't opened up the CORS policy for XHR requests too widely.

[davisagli]

(I'm curious why plone.restapi is disabling CSRF in specific endpoints rather than doing it generally in plone.rest though. It seems like something that should be handled at a framework level.)

[sneridagh]

@davisagli I vaguely remember that there was a reason for that but I can’t remember now. Maybe @tisto can recall it.

[jensens]

I'm sorry, "gets grip"? Do you mean "takes hold" or "takes effect"?

@stevepiercy Sorry, should have looked this up, yes.

According to what @davisagli says I tend to think we are doing it wrong. I think we better should not disable protection in restapi at all, but add a check in plone.protect if it is a ReST request and CORS is configured properly and only then allow writes without the header set.

[nitish-singh07]

is this issue still open ?@stevepiercy

[stevepiercy]

@nitish-singh07 yes it is still open, by virtue of the green icon with "Open" in its name. See Item 4 under Things not to do.

However the maintainers have not reached an agreement on what, if anything, needs to be written for documentation.

Because of ongoing discussion, this is not a good item for first-time contributors, so I changed its difficulty level to 42 lvl: moderate.