4.3.x csrffixes partial

[mauritsvanrees]

This leans heavily on pull request #150.

Difference is that I keep plone.protect to the 2.x series and do not include plone4.csrffixes.

I include a branch of plone.protect with minimal forward compatibility so you can import IDisableCSRFProtection (which has no effect, but at least gives no ImportError) and can use @@authenticator/token. See https://github.com/plone/plone.protect/tree/2.x-forward-compat

Basically the idea is: move the patches from plone4.csrffixes into their correct places, which is what we normally do with hotfixes.
If someone wants csrf protection, then adding plone.protect 3.x to a buildout should be enough, without needing extra packages or pins.

If we want to, this can be a stepping stone for #150. That pull request suffers from too many test failures at the moment, which makes it hard to go forward.

And it is not just test setup problems, but 'live' ones: create a Plone Site, go to @@markup-controlpanel, and you get the confirmation page with the csrf warning. This is because an annotation for wicked is initialised on first load. This is a write-on-read that should be fixed (or grudgingly accepted). Whether we use the csrf protection from the newer plone.protect or not, does not really matter for this: this protection simply makes it clearer that there is a write-on-read problem.

[mauritsvanrees]

Jenkins: http://jenkins.plone.org/job/pull-request-5.0/877/

[mauritsvanrees]

BTW, I am wondering if we need to do something with protect.js from plone4.csrffixes. With the above changes, this should be the only thing from that package that is not merged.

[gforcada]

@mauritsvanrees you are testing this pull request against plone 5.0, while it should be 4.3! :-)

[mauritsvanrees]

Haha, good catch. :-)

[mauritsvanrees]

Running at http://jenkins.plone.org/job/pull-request-4.3/229/

[mauritsvanrees]

Getting HTTP Error 400: Bad Request in a plone.app.users test that goes fine locally... I tried one fix but this did not help.

[gforcada]

@mauritsvanrees no idea about the 400 error, but given that you are working on branches, just put debug information on the test to see which URL and content is there? that could help

[mauritsvanrees]

Ha, this now works. Looks ready for merge. Well, not this coredev branch: the individual package branches should be merged.

So to summarise what we would change here:

• Several straightforward write-on-read fixes.
• plone.protect 2.x with forward compatibility for 3.x. So not full csrf protection.
• plone.contentrules master change, so must also work on Plone 5: Fix a write on read situation plone.contentrules#4
• Use Products.PlonePAS master.
• Products.PlonePAS master change, so must also work on Plone 5: Let cleanId return a string when getting a unicode. Products.PlonePAS#15

[gforcada]

@mauritsvanrees cool! thanks a lot for you effort on this!

[esteele]

Merged.

[idgserpro]

@mauritsvanrees Guys, I think we have an interesting issue here.

plone4.csrffixes: The package aims to backport the auto CSRF implementation from Plone 5 to Plone 4.

In protect.js, we don't actually have just backports but a feature of automatically setting a token in xhr, which we don't have in plone.protect 3.x branch (we have just a README note about how to do it, not an implementation per se).

So, with packages that need to be Plone 4 and 5 compatible, when using Plone 4 and plone4.csrffixes for ajax requests everything is fine, but when using Plone 5 we need to manually do this with two approaches:

• Adding _authenticator variables across all or ajax calls, getting it's value from an input in a form;
• Copy protect.js from plone4.csrffixes.

Our question is: will this implementation be added to plone.protect, or do we need to manually add to our packages? Are we missing something here?

[mauritsvanrees]

See plone/plone.protect#42 where we can discuss this last question.