Bug 3280

Confirmed this is still an issue.

Not sure there is any good way around this.  To truly solve the issue, we'd need to be able to separate the spam from the ham (as it were), which is too subjective to handle systematically.

Perhaps instead of allowing arbitrary email addresses, we only allow either other borrowernumbers (so you can send to your friends) or a selection of one of the patron's own email addresses.  The latter would be much easier than the former.  This would reduce the flexibility of opac-sendbasket, but I think that flexibility is the fundamental security issue here.

Does opac/opac-sendshelf.pl should be have a separated bug report ?

As there have same features (send email to someone), corrections (or enhancements) should be consistent.

Kyle,
Your patch now removes the feature of an anonymous user putting books into the cart and mailing them. Would it not be enough to check if the cart is not empty instead of forcing login now? 
Should we discuss restricting all email functionality in Koha to authenticated users? Or add yet another pref :-)

Hi Marcel, not sure that was Kyle - there is bug 4274 for the problem. It used to be possible for not logged in users to mail the cart (I know it was in 3.2.x). Then it got changed some time after that. Now it requires you to log in before you can send items from the cart.

(In reply to comment #6)
> Hi Marcel, not sure that was Kyle - there is bug 4274 for the problem. It
> used to be possible for not logged in users to mail the cart (I know it was
> in 3.2.x). Then it got changed some time after that. Now it requires you to
> log in before you can send items from the cart.

It is still possible in master. I just did ??

Perhaps it changed again? :( I tested on 3.6.3.

(In reply to comment #8)
> Perhaps it changed again? :( I tested on 3.6.3.

No, it is not possible. Somehow I cannot reproduce anymore what I did before..

OK. The patch came from Frere Sebastien Marie. And current master does not allow anynomous users here (anymore).

One question remains: It works for me. But if you change the From address to the patron's address, could we have Relay Access Denied errors or similar? A mail server could reject outgoing mail if not sent from designated domains?

(En réponse au commentaire 10)
> One question remains: It works for me. But if you change the From address to
> the patron's address, could we have Relay Access Denied errors or similar? A
> mail server could reject outgoing mail if not sent from designated domains?

Yes, it is possible that mail will be reported as spam, if the patron address use SPF (spam prevention system based on whitelist of smtp authorised to send email for particuliar domain).

(En réponse au commentaire 11)
> (En réponse au commentaire 10)
> > One question remains: It works for me. But if you change the From address to
> > the patron's address, could we have Relay Access Denied errors or similar? A
> > mail server could reject outgoing mail if not sent from designated domains?
> 
> Yes, it is possible that mail will be reported as spam, if the patron
> address use SPF (spam prevention system based on whitelist of smtp
> authorised to send email for particuliar domain).

So it is possible to remove using GetFirstValidEmailAddress($borrowernumber) as From address, and inconditionnally use C4::Context->preference('KohaAdminEmailAddress').

But I think we should include *in the body* the name/address of the sender:
"This mail was sent to you from $user->{firstname} $user->{surname} <$user_email>"

So authentificated user will not abuse sending email using opac-sendbasket.pl

We could also add a Reply-To field with "$user->{firstname} $user->{surname} <$user_email>" ? (in order to prevent error like press the reply-to button and mail to KohaAdminEmailAddress instead of $user_email)

Created attachment 10548 [details] [review]
Signed patch for 3280

QA Comment:
Small patch. Code looks good to me. Idea of Reply-to address seems to me a good approach as compared to changing the From address.
Passed QA

(In reply to comment #3)
> Does opac/opac-sendshelf.pl should be have a separated bug report ?
> As there have same features (send email to someone), corrections (or
> enhancements) should be consistent.

Sebastien: You could also adjust the title of this report, take over the assignee field (you wrote the patch, not Galen) and submit another patch for send-shelf. That would be great!

(assignee changed, to reflect who sent the patch)

Because this now makes it impossible to send a cart without being logged in (which is probably a valid change) it is a big behaviour change which I dont want to spring on people mid cycle. Holding for 3.10

Hi Chris, but this is already the case in 3.6?

Ok, rereading through the bug report there is a bit that worries me:
- add field X-Orig-IP with IP of sender

IP addresses in web server logs have been issue of data privacy here - I wonder if it's ok adding the IP to the mail?

(In reply to comment #20)
> Ok, rereading through the bug report there is a bit that worries me:
> - add field X-Orig-IP with IP of sender
> 
> IP addresses in web server logs have been issue of data privacy here - I
> wonder if it's ok adding the IP to the mail?

Would not worry about that too much. Could be lots of ip addresses in the mail headers showing the route of this message from A to B.
Moreover, measures against spammers here should not be considered as privacy intruding. You can find this ip address also in your access log; that is on your server not just in the mail header..

(In reply to comment #21)
> (In reply to comment #20)
> > Ok, rereading through the bug report there is a bit that worries me:
> > - add field X-Orig-IP with IP of sender
> > 
> > IP addresses in web server logs have been issue of data privacy here - I
> > wonder if it's ok adding the IP to the mail?
> 
> Would not worry about that too much. Could be lots of ip addresses in the
> mail headers showing the route of this message from A to B.
+1, ans I suspect that, in France, it's legally requested to track this kind of things = the mail is sent by someone, the server admin must be able to answer any legal/court request

Created attachment 10595 [details] [review]
Followup for privacy issue

Paul: Katrin raised the question on privacy issue on x-orig-ip.
Since we did not have it for years, i would suggest to remove it now first. And come up with a solution for all parties later on.

Setting this to Signed off to get Paul's attention :-)

(In reply to comment #24)
> Setting this to Signed off to get Paul's attention :-)
Passed QA may even be a better idea..

OK, patch pushed and discussion started on mailing lists

(In reply to comment #26)
> OK, patch pushed and discussion started on mailing lists
Paul: I do not see the followup appearing in master ?

(In reply to comment #27)
> (In reply to comment #26)
> > OK, patch pushed and discussion started on mailing lists
> Paul: I do not see the followup appearing in master ?

Sorry, it was commited on my local computer, forgot to push yesterday, before leaving office. It's pushed now

I have forwarded this to a coworker and hoping to find out what we have to do soon. Whatever we do, we should not forget about the lists as they are also sent out using a similar technique.

This was released in 3.10.0