search

plone / plone.protect

HTTP protection utilities for the Plone CMS
https://pypi.org/project/plone.protect/
7 stars 8 forks source link

Include protect.js for Plone 4? And Plone 5? #42

Closed mauritsvanrees closed 8 years ago

mauritsvanrees commented 8 years ago

Basically, see https://github.com/plone/plone4.csrffixes/issues/19 And possibly related thread on community.plone.org: https://community.plone.org/t/plone-protect-and-ajax-post-requests/2010

In short: Plone 4.3.9 has all fixes from plone4.csrffixes included, except the plone.protect pin and the transform which adds protect.js and blesses a few read-on-writes. These read-on-writes are presumably already blessed by plone.protect 3.x or have been fixed in their respective packages. But the javascript still seems needed for best experience.

Should we add this script on plone.protect master and perhaps include it only when the Plone version is 4.x? My guess is that adding it to the auto transform would work and is not too invasive. Or is this something we should do in for example CMFPlone?

mauritsvanrees commented 8 years ago

In https://github.com/plone/buildout.coredev/pull/176#issuecomment-218605691 @idgserpro asks a similar question, but slight broader. Basically: should we include the protect.js file in all Plone versions, so also in Plone 5? @vangheem, do you know this?

idgserpro commented 8 years ago

We vote for adding protect.js to Plone 5 as well. All add-ons that need to be both compatible to Plone 4 and 5 will benefit from this. Somewhat related: https://community.plone.org/t/the-incredible-and-sad-tale-of-innocent-plone-5-and-its-heartless-add-ons/1437

@hvelarde FYI

vangheem commented 8 years ago

I'm indifferent to adding it to plone 5. I personally think people should know what they are doing about CSRF and get ugly errors when they write unsafe things; however, it will make things easier...

If added to plone 5, some of the plone 4 specific bits should be removed.

idgserpro commented 8 years ago

We already have some "magic" happening in plone.protect transforms, automatically adding tokens to our forms. The idea is to have this same ease of use but now in AJAX calls.

Imagine you have a Plone 4 addon, using plone4.csrffixes. You have 50 ajax calls. You want to make it compatible with Plone 5. You have two options:

Our opinion is: if you don't need to explicitly add tokens to forms, you shouldn't need this in AJAX calls either. This is consistent with plone.protect README as well:

Automatic CSRF Protection

Since version 3, plone.protect provides automatic CSRF protection. It does this by automatically including the auth token to all internal forms when the user requesting the page is logged in.

vangheem commented 8 years ago

Alright, as I said, I'm indifferent.

idgserpro commented 8 years ago

@mauritsvanrees Could you please add protect.js in plone.protect?

mauritsvanrees commented 8 years ago

See pull request https://github.com/plone/plone.protect/pull/49 which finally passes the tests.

idgserpro commented 8 years ago

@mauritsvanrees fantastic! You are thinking pin plone.protect 3 in Plone 4.3? Or will we still depend of plone4.csrffixes?

mauritsvanrees commented 8 years ago

No, plone.protect will stay at 2.x. Moving to 3.x causes too many test failures. See also pull request #48.

frisi commented 8 years ago

@mauritsvanrees this change breaks plone sites that follow the instructions on https://plone.org/security/hotfix/20151006 as both packages register protect.js

zope.configuration.config.ConfigurationConflictError: Conflicting configuration actions
  For: ('resource', u'protect.js', <InterfaceClass zope.publisher.interfaces.browser.IBrowserRequest>, <InterfaceClass zope.publisher.interfaces.browser.IDefaultBrowserLayer>)
    File "plone.protect-3.0.19-py2.7.egg/plone/protect/configure.zcml", line 38.4-41.10
          <browser:resource
              name="protect.js"
              file="protect.js"
              />
    File "plone4.csrffixes-1.0.9-py2.7.egg/plone4/csrffixes/configure.zcml", line 16.2-19.6
        <browser:resource
          name="protect.js"
          file="protect.js"
          />

until today i thought i'd need to add plone4.csrffixes to my instance eggs if i want to activate the hotfix https://plone.org/security/hotfix/20151006

what's the official way to get around the above error? we either need to update the documentation that we need to pin plone.protect = 3.0.18 or that plone4.csrfprotect is not needed anymore (did not check if the package actually does additional stuff and is still needed)

mauritsvanrees commented 8 years ago

True, this gives a problem. This pull request should fix it: https://github.com/plone/plone4.csrffixes/pull/22 After this is merged, and a new release of plone4.csrffixes is made, you can use the two together again. Workaround for now would be to keep using plone.protect 3.0.18.

plone.protect 3.x is still missing a few fixes, see https://github.com/plone/plone.protect/pull/48