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.

Sign up for GitHub

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

Jump to bottom

notification moderator: link to commented page to moderate comments instead of links to approve or delete with CSRF confirmation #163

Merged
merged 8 commits into from
Dec 3, 2019
Merged

notification moderator: link to commented page to moderate comments instead of links to approve or delete with CSRF confirmation #163

merged 8 commits into from
Dec 3, 2019

Conversation

ksuess
Copy link
Member

@ksuess ksuess commented Sep 30, 2019

see #162

@mister-roboto
Copy link

@ksuess thanks for creating this Pull Request and help improve Plone!

To ensure that these changes do not break other parts of Plone, the Plone test suite matrix needs to pass.

Whenever you feel that the pull request is ready to be tested, either start all jenkins jobs pull requests by yourself, or simply add a comment in this pull request stating:

@jenkins-plone-org please run jobs

With this simple comment all the jobs will be started automatically.

Happy hacking!

@ksuess
Copy link
Member Author

ksuess commented Sep 30, 2019

@jenkins-plone-org please run jobs

@ksuess ksuess requested a review from agitator September 30, 2019 12:27
jensens
jensens requested changes Oct 3, 2019
View reviewed changes
Copy link
Member

@jensens jensens left a comment

Choose a reason for hiding this comment

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

overall LGTM, except disabling CSRF protection.

plone/app/discussion/browser/moderation.py Outdated Show resolved Hide resolved
@ksuess
Copy link
Member Author

ksuess commented Nov 22, 2019

@jenkins-plone-org please run jobs

@ksuess
Copy link
Member Author

ksuess commented Nov 25, 2019

It would be nice if moderator could click on "approve" in email, logs in and does not have to confirm that he really wants do this action.
I need some help with this CSRF plone.protect topic how to achieve this, without completly disabling CSRF protection.

@jensens
Copy link
Member

jensens commented Nov 25, 2019

The token hash contains the username, so it sending out an email with token for a different user does not work.

But disabling the CSRF protection for moderation is not what we want from a security point of view.

I would create an intermediate page (Do you want to approve the message .... ? Yes, No) with a link to actually approve a comment. Then on the page the CSRF token can be inserted.

@ksuess
Copy link
Member Author

ksuess commented Nov 29, 2019

So one option for a new moderator notification could be an email with

  1. New: also email of the commenter
  2. A link to an intermediate page. This could be the view of the commented page (at anchor of the comment):
    image
    On this view everything needed is there: the moderator can even edit the comment before approving, deleting or later also marking as spam. see issue Additional workflow state for spam comments #164

ksuess added 2 commits December 1, 2019 19:47
@ksuess
Notification moderator: email of commentator added. Link as is but re…
3d3c638 
…quest to log in for moderating comment added.

New moderator notification with email and link to commented page and request to login.
No links to approve and delete: due to CSRF direct links to modification of Plone objects result in request to confirm. So page with comments is presented to moderator.
If already logged in, moderator is on comment to moderate.
If not logged in, moderator is on login page with came_from.
@ksuess
Update moderation.py
296d591 
unused IDisableCSRFProtection removed
@ksuess
Copy link
Member Author

ksuess commented Dec 1, 2019

@jenkins-plone-org please run jobs

@ksuess
Copy link
Member Author

ksuess commented Dec 2, 2019

@jenkins-plone-org please run jobs

@ksuess
Copy link
Member Author

ksuess commented Dec 2, 2019

image

@ksuess ksuess changed the title publish only pending comment, else show status message notification moderator: link to commented page to moderate comments instead of links to approve or delete with CSRF confirmation Dec 2, 2019
@ksuess ksuess requested a review from jensens December 2, 2019 09:40
@jensens jensens merged commit 7469a4f into master Dec 3, 2019
@jensens jensens deleted the moderation branch December 3, 2019 12:51
@ksuess ksuess mentioned this pull request Dec 5, 2019
Closed
@vincentfretin vincentfretin mentioned this pull request Mar 20, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jensens jensens jensens approved these changes

@agitator agitator Awaiting requested review from agitator

Assignees
No one assigned
Labels
04 type: enhancement
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@ksuess @mister-roboto @jensens