Ticket #1490 (closed Bug: fixed)

Opened 11 years ago

Sendto button mis-configured in HEAD

Reported by: limi Owned by:
Priority: critical Milestone: 2.1
Component: General Version:
Keywords: Cc:

Description

We have removed the allow_sendto attribute (because it's a proper action now), but is still checks for the attribute in the action. Just remove the condition in the setup (I'm to tired to do it right now :)

Change History

comment:1 Changed 11 years ago by limi

The plot thickens :)

The SendTo form itself also checks for the allow_sendto attribute. It should check for the action being visible instead :)

comment:2 Changed 11 years ago by maikroeder

Todo:

  1. Remove the property in migrations
  2. Remove the condition in the sendto action
  3. Change all Page Templates and Python Code to check for the visible state of the action

comment:3 Changed 10 years ago by limi

This is an ugly that I believe still exists. Should be terminated before 2.0.

comment:4 Changed 10 years ago by pieterb

See also:  http://www.plone.org/collector/1338: msgid bug in sendto template

comment:5 Changed 10 years ago by gerrykirk

updated code in migration scripts, removed setting on portal properties and prefs form, removed condition on action.

was not able to figure out how to check visible property in sendto.cpy.

migration changes need testing

comment:6 Changed 10 years ago by gerrykirk

It occurred to me that the migration script should toggle the visible property of the action, based on the site property value. Not sure where that should be done in the migration code. Right now, the property gets deleted in onex_tobeta2.py and the action added in plone2_base.py.

Also, is it necessary to check the visible property of the sendto action in sendto.cpy?

comment:7 Changed 10 years ago by limi

  • Status changed from new to closed
  • Resolution set to fixed

It's no longer a prefs option, it's controlled purely with actions. Since it's such a small change (and not intrusive), I don't think we have to fix this in the migrations. People need to know how the new approach works anyway.

comment:8 Changed 10 years ago by Anonymous User

If I append the following to visible documents on my Plone site:

sendto?send_from_address=andrew@…&send_to_address=andrew@…&comment=get_your_nrop_at_my_url

It successfully sends the e-mail, even though I have set the mailto action to not be visible. This works whether I am logged in to my Plone 2.0 Final instance or not.

(BTW: That's not my real e-mail. This is: andrew at bennettsathome dot com)

Notice that sendto isn't even the form - it's the success action of the form.

comment:9 Changed 9 years ago by tiran

  • Status changed from closed to new
  • Resolution fixed deleted

The bug still exists and I think it is a critical bug. The best solution for the whole sendto feature / spam issue would be a new permission like "Allow sendto". All actions and scripts should be protected by the permission. That much easier and more secure than fiddling around with the visible state of an action.

comment:10 Changed 9 years ago by tiran

Also readplone_form_scripts/sendto.cpy

4405 shh42 # need to check visible state of 'sendto' action in portal_actions 4405 shh42 # but I couldn't figure out how - update collector issue #1490 4405 shh42 # when implemented 4405 shh42 at = getToolByName(context, 'portal_actions') 4405 shh42 4405 shh42 #if not sendto_action.visible : 4405 shh42 if 0: 5785 shh42 return state.set( 5785 shh42 status='failure', 5785 shh42 portal_status_message='You are not allowed to send this link.')

comment:11 Changed 9 years ago by limi

Please also verify that the new contact-info template doesn't have any security problems. It allows mailing the site admin with requests, but hopefully can't be exploited in a way so that a query string can overwrite the To: address or similar.

comment:12 Changed 9 years ago by Anonymous User

This issue is resolved in svn, but the collector doesn't like me following up on this one issue for some reason, so I can't close it. Please somebody close it.

alecm

comment:13 Changed 9 years ago by limi

  • Status changed from new to closed
  • Resolution set to fixed

Fixed.

comment:14 Changed 22 months ago by davisagli

  • Component changed from Infrastructure to General
Note: See TracTickets for help on using tickets.