Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Issue 590 - Implement CSRF protection #594

Closed
wants to merge 3 commits into from
Closed

Issue 590 - Implement CSRF protection #594

wants to merge 3 commits into from

Conversation

rodfersou
Copy link
Member

closes #590

@@ -18,6 +18,12 @@ parts +=
rebuild_i18n-sh
robot

eggs +=
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rodfersou please remove from here

@@ -66,3 +72,7 @@ scripts =
coverage =
# use latest version of setuptools
setuptools =
plone.keyring = 3.0.1
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rodfersou if needed, this should be included in versions-4.3.x.cfg instead

@@ -18,6 +18,9 @@ parts +=
rebuild_i18n-sh
robot

test-eggs +=
plone4.csrffixes

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@hvelarde should I run this just in 4.3 version? If so, I need to figure out when CSRF is enabled into RF test.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rodfersou no, we have to test this in all Plone versions supported by the package; figuring out how is the hard part.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok, then I'll replicate the pins into all versions 4.x files

@rodfersou rodfersou force-pushed the issue_590 branch 4 times, most recently from 37bac47 to faf6008 Compare March 2, 2016 15:28
@rodfersou
Copy link
Member Author

@hvelarde finally green

@@ -18,6 +18,9 @@ parts +=
rebuild_i18n-sh
robot

test-eggs +=
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rodfersou this should be added on a version basis also or we will end with the package installed in Plone 5

@hvelarde
Copy link
Member

hvelarde commented Mar 2, 2016

@idgserpro comments?

hvelarde referenced this pull request in plone/Products.CMFPlone Mar 3, 2016
@rodfersou rodfersou force-pushed the issue_590 branch 4 times, most recently from 38866d8 to 24264e1 Compare March 4, 2016 15:40
@idgserpro idgserpro force-pushed the issue_590 branch 5 times, most recently from 3b54d43 to 5cdea1c Compare April 19, 2016 13:33
@idgserpro idgserpro force-pushed the issue_590 branch 4 times, most recently from c21e49d to f4711a1 Compare April 19, 2016 17:44
@idgserpro idgserpro force-pushed the issue_590 branch 2 times, most recently from d10d4b2 to 64fa546 Compare May 6, 2016 14:46
@idgserpro idgserpro force-pushed the issue_590 branch 5 times, most recently from b24c711 to ed518b9 Compare May 10, 2016 19:05
Fix some broken builds:

https://travis-ci.org/collective/collective.cover/builds/128352379
https://travis-ci.org/collective/collective.cover/builds/128367247
https://travis-ci.org/collective/collective.cover/builds/128375390

Errors:

KEYWORD BuiltIn . Sleep 2s, Wait for content tree to load
Documentation:
Pauses the test executed for the given time.
Start / End / Elapsed:	20160418 15:49:18.810 / 20160418 15:49:20.813 / 00:00:02.003
15:49:20.812	INFO	Slept 2 seconds
15:49:20.812	INFO	Wait for content tree to load
00:00:00.581KEYWORD Selenium2Library . Input Text css=#content-trees input, file

Documentation:
Types the given `text` into text field identified by `locator`.
Start / End / Elapsed:	20160418 15:49:20.813 / 20160418 15:49:21.394 / 00:00:00.581
00:00:00.060KEYWORD Selenium2Library . Capture Page Screenshot
15:49:20.814	INFO	Typing text 'file' into text field 'css=#content-trees input'
15:49:21.394	FAIL	ElementNotVisibleException: Message: Element is not currently visible and so may not be interacted with
In essence, this is needed for Plone 5 only. Plone 4 installations that
have plone4.csrffixes and plone.protect >= 3.0.x don't need it.

Check

plone/plone.protect#42

for mode details.
@idgserpro
Copy link
Member

The authenticator is only necessary in Plone 5. See:

#590
plone/plone.protect#42

@idgserpro idgserpro closed this May 23, 2016
@idgserpro idgserpro deleted the issue_590 branch May 23, 2016 17:06
@hvelarde
Copy link
Member

@idgserpro please restore the branch and reopen the PR; this is work that will need to be retaken at some point in the future; makes no harm to have it here.

@idgserpro idgserpro restored the issue_590 branch May 24, 2016 12:17
@idgserpro idgserpro reopened this May 24, 2016
@idgserpro
Copy link
Member

@hvelarde I don't see sense in it but it is done. I think we have to concentrate here. Sorry.

@idgserpro
Copy link
Member

protect.js was added in plone.protect:

plone/plone.protect#49

@idgserpro idgserpro closed this Jun 10, 2016
@hvelarde hvelarde deleted the issue_590 branch June 10, 2016 15:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Implement CSRF protection for Plone 5
3 participants