Implement CSRF protection for Plone 5

[hvelarde]

In 657f1e6 @alecpm started some work to implement CSRF protection into the package; we need to review it and finish the work.

[idgserpro]

@hvelarde @rodfersou the authenticator is only necessary in Plone 5. See:

plone/plone.protect#42

In Plone 4 with plone4.csrffixes and Plone 4 without plone4.csrffixes it's not necessary. I removed the authenticator and the tests passed with plone4.csrffixes.

As the cc is not working in Plone 5 still do not think it should be fixed now.

I suggest close the PR #594 and change the title of this issue to be fix in Plone 5. Do you agree?

[hvelarde]

I have no opinion on this at this time.

[idgserpro]

Guys, any opinion on this matter? Specially about #590 (comment)

@alecpm @rodfersou @frapell @jpgimenez @djowett @fredvd

[rodfersou]

@idgserpro digging into the threads I found what you are talking about.. protect.js automatically add the token for ajax calls.

But my question is.. what is the difference beetween the X-CSRF-TOKEN header and the _authenticator parameter that we add in this pull request?

I think we should wait for the decicion of this issue before continue with this work.

About your experiment removing the authentication and keeping the tests that didn't fail, that's interesting, it should fail..

[djowett]

Sorry, no time to form an opinion on this one. Glad you guys are working on it.

Sent from my Sony Xperia™ smartphone

---- Rodrigo Ferreira de Souza wrote ----

@idgserpro digging into the threads I found what you are talking about.. protect.js automatically add the token for ajax calls.

But my question is.. what is the difference beetween the X-CSRF-TOKEN header and the _authenticator parameter that we add in this pull request?

I think we should wait for the decicion of this issue before continue with this work.

About your experiment removing the authentication and keeping the tests that didn't fail, that's interesting, it should fail..

—
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub

[idgserpro]

But my question is.. what is the difference beetween the X-CSRF-TOKEN header and the _authenticator parameter that we add in this pull request?

@rodfersou From an end user perspective, none. You can see in this commit that if _authenticator is not found, it checks for X-CSRF-TOKEN to add csrf protection.

From an implementation perspective, this means less code and being less error prone, since all AJAX calls will have the token in the request instead of manually adding it to your javascript. The way it's done now, if you add another call to your javascript you need to remember to add, again, _authenticator to your request.

The experiment don't fail in Plone 4.3 because of plone4.csrffixes, this package adds the X-CRSF-TOKEN in all requests.

[idgserpro]

See too: https://github.com/plone/plone.protect#headers

[hvelarde]

so we can add the authenticator based on the Plone version used, right? we can leave this pending until we can make more advances on Plone 5 compatibility.

[idgserpro]

@hvelarde right. I'll change the title of this issue to make this clearer.

[idgserpro]

protect.js was added in plone.protect:

plone/plone.protect#49