fullfil Plone Styleguide Coding Conventions

[loechel]

Various Changes, due to a fix for disabling transforms on request while CSRF Protection should be disabled.

• Cleanup of package, to fullfil Plone Styleguide Coding Conventions
• Files Added for Styleguide Settings:
  • setup.cfg - isort settings
  • .editorconfig
• modified PloneTransform.transformIteratable (auto.py) to disable transform if CSRF Protection is explicite disabled.

[vangheem]

I don't think you want to do this. This also skips doing the transform to add tokens to intern forms.

[gforcada]

@loechel if you want to clean up a package, please do so in a different pull request that's only doing that

That will help get it merged right away :+1: some tips (if needed): https://github.com/plone/jenkins.plone.org/blob/master/docs/source/run-qa-on-package.rst

[jensens]

@loechel @vangheem state here? close this?

[loechel]

sorry, have been extremly busy the last week.

From my point of view we should not distignuish between diabling CSRF Protection via the environment settings which was alrady in, and by the Marker Interface.

I knew that my case, preparing Snippets for ESI is special, but I don't want to completely disable protection nor make the code much mor complicated, by a separate parser for Elements that do not provide a HTML Root Element and Doctype.

But if we want to discuss it more in detail I could revert this and just make the pull for stylguide clomplience like @gforcada sugessted.

[mauritsvanrees]

I first thought it made sense. But now I disagree.

Currently, the marker interface makes sure plone.protect does not complain about a write-on-read or a missing csrf token on the current request. Presumably, the marker interface is only set when we think it is safe to allow this. So instead of showing the confirmation/warning page, Plone shows the normal page. This page contains links and/or forms. The transform has changed these links and forms to contain the csrf token.

With your change, those csrf token are not added. So when the user fills in a form on this page, and posts a request, plone.protect will kick in on this new request, which presumably does not have the marker interface, and show the confirmation/warning page.

So the effect is that the confirmation page may be shown a request later.

If my reasoning is correct, then I am -1 on this. +1 for the style changes.

[mauritsvanrees]

The explicit transform skip from the title is just one line, right? If you agree, you could undo that change and keep this pull request (but change the title), if that makes sense. But maybe a separate one is clearer.

[loechel]

Ok, I updated the Pull Request, so that it functionally don't change anything, now it is just the style changes. I did also commented out the CHANGE.rst documentation for the change but did not remane the Pull request.

should we also revert https://github.com/plone/plone4.csrffixes/commit/2c247277c4758b7895685f7e43f5bfa6cdab6ca1 ? It does the same.

[jensens]

looks like it needs a rebase

[jensens]

I rebased and squashed down to two relevant commits. This is all about ocde style. If its green I'am going to merge.

[jensens]

rebase is more headache than reapplying the code style changes on a new branch. I close this in favor of #58.