Bug 22821 - Use reply-to address for item notes notifications if available to avoid being flagged as spam
Summary: Use reply-to address for item notes notifications if available to avoid being...
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: OPAC (show other bugs)
Version: Main
Hardware: All All
: P5 - low normal (vote)
Assignee: Martin Renvoize
QA Contact: Jonathan Druart
URL:
Keywords:
Depends on: 10269 19496 22823
Blocks:
  Show dependency treegraph
 
Reported: 2019-05-01 15:31 UTC by Martin Renvoize
Modified: 2020-11-30 21:48 UTC (History)
5 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Trivial patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Prior to this patch when a patron added an item note the system would send a notification email to the branchemail with a from address of the patrons email. This patch updates the notification system to use the 'reply-to' address in preference to the 'branchemail' if it is defined and it also sets the 'from' address to the 'branchemail' and adds the patron email as a 'reply-to' as this was the intended functionality.
Version(s) released in:
20.05.00


Attachments
Bug 22821: Add reply_address field to message_queue table (1.63 KB, patch)
2020-02-25 10:09 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 22821: Schema Update (1.42 KB, patch)
2020-02-25 10:09 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 22821: Use 'reply-to' address in issue notes (3.34 KB, patch)
2020-02-25 10:09 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 22821: Correct to_address for issue notes (1.77 KB, patch)
2020-02-25 10:09 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 22821: Add reply_address field to message_queue table (1.63 KB, patch)
2020-02-25 10:16 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 22821: Schema Update (1.42 KB, patch)
2020-02-25 10:16 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 22821: Use 'reply-to' address in issue notes (5.79 KB, patch)
2020-02-25 10:16 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 22821: Correct to_address for issue notes (1.77 KB, patch)
2020-02-25 10:16 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 22821: Add reply_address field to message_queue table (1.71 KB, patch)
2020-02-28 00:30 UTC, Myka Kennedy Stephens
Details | Diff | Splinter Review
Bug 22821: Schema Update (1.49 KB, patch)
2020-02-28 00:30 UTC, Myka Kennedy Stephens
Details | Diff | Splinter Review
Bug 22821: Use 'reply-to' address in issue notes (5.86 KB, patch)
2020-02-28 00:30 UTC, Myka Kennedy Stephens
Details | Diff | Splinter Review
Bug 22821: Correct to_address for issue notes (1.84 KB, patch)
2020-02-28 00:30 UTC, Myka Kennedy Stephens
Details | Diff | Splinter Review
Bug 22821: Use 'reply-to' address in issue notes (2.84 KB, patch)
2020-03-02 14:14 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 22821: Correct to_address for issue notes (1.84 KB, patch)
2020-03-02 14:14 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 22821: Use 'reply-to' address in issue notes (2.92 KB, patch)
2020-03-18 09:49 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 22821: Correct to_address for issue notes (1.91 KB, patch)
2020-03-18 09:49 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 22821: Rename method with ->inbound_email_address (1.53 KB, patch)
2020-03-18 09:49 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 24722: reserves.priority must default to 1 at DB level (3.14 KB, patch)
2020-03-18 10:08 UTC, Jonathan Druart
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Martin Renvoize 2019-05-01 15:31:48 UTC
In bug 19496 a fallback order for the email addresses (both to and from) was added. This fallback order appears to be incorrect per my understanding of the system preferences.

> my $to_address = $branch->branchemail || $branch->branchreplyto || C4::Context->ReplytoDefault || C4::Context->preference('KohaAdminEmailAddress');

My understanding is that branchemail is used for the 'from' address in emails from the branch, however it may not be an address which is accessible by the librarians (as to be allowed through spam filters for most popular email service the 'from' address domain has to match the domain of the sender server).. as such 'reply-to' address was added to allow for a library to send 'from' a proper domain but have 'replies' go to an email address they control.

> my $from_address = $patron->email || $patron->emailpro || $patron->B_email;

We cannot spoof send emails from the patrons email address for the same reasons as outlined above. The email must come from the koha system but may happily contain the above fallback in the reply-to header to achieve what I believe is wanted.
Comment 1 Martin Renvoize 2019-05-02 06:42:20 UTC
I am more than happy to mentor someone who might like to work on this, just reach out here or on IRC (I'm ashimema there)
Comment 2 Myka Kennedy Stephens 2020-02-23 00:04:31 UTC
I think our library is experiencing this bug. When a patron submits a note on an item through the OPAC, Koha sends the email to make it look like it came from the patron's email address. We are able to receive these emails if it is a patron email with the same domain (lancasterseminary.edu). A patron from a different domain (e.g., yahoo.com) had their OPAC note email returned to them because it couldn't be authenticated as coming from their domain. OPAC note emails need to come from the email address set in Koha admin instead of trying to look like they came from the patron.

Are we on the same page, Martin? Or do I need to submit this as a separate bug?

Thanks,
Myka
Comment 3 Martin Renvoize 2020-02-24 19:22:43 UTC
Hi Myka, 

I think you are indeed experiencing this bug.. well.. half of it at least, I appear to explain two issues I suspect to be happening.

I must admit, I had hoped someone would pick this up nearer the time I reported it.. I did try to encourage a few people to take a look.

I'll try and put a quick patch together myself if you're willing to test it.
Comment 4 Martin Renvoize 2020-02-25 10:09:41 UTC
Created attachment 99559 [details] [review]
Bug 22821: Add reply_address field to message_queue table
Comment 5 Martin Renvoize 2020-02-25 10:09:44 UTC
Created attachment 99560 [details] [review]
Bug 22821: Schema Update
Comment 6 Martin Renvoize 2020-02-25 10:09:46 UTC
Created attachment 99561 [details] [review]
Bug 22821: Use 'reply-to' address in issue notes

This patch updates the issue notices to not pass from_address and so
default to the branch email of the user for from_address. We now pass
the patrons email (cascading through email, emailpro, B_email) into the
reply_address field so mail servers are less likely to report emails as
spam and mail clients still have access to the patron address via the
reply-to header when staff want to hit 'reply' and have the email go to
the user.
Comment 7 Martin Renvoize 2020-02-25 10:09:49 UTC
Created attachment 99562 [details] [review]
Bug 22821: Correct to_address for issue notes

This patch updates the controller and service code to use the newly
introduced 'get_effective_email' routine from bug 22823 to obtain the
branches most appropriate email address.
Comment 8 Martin Renvoize 2020-02-25 10:16:32 UTC
Created attachment 99563 [details] [review]
Bug 22821: Add reply_address field to message_queue table
Comment 9 Martin Renvoize 2020-02-25 10:16:35 UTC
Created attachment 99564 [details] [review]
Bug 22821: Schema Update
Comment 10 Martin Renvoize 2020-02-25 10:16:38 UTC
Created attachment 99565 [details] [review]
Bug 22821: Use 'reply-to' address in issue notes

This patch updates the issue notices to not pass from_address and so
default to the branch email of the user for from_address. We now pass
the patrons email (cascading through email, emailpro, B_email) into the
reply_address field so mail servers are less likely to report emails as
spam and mail clients still have access to the patron address via the
reply-to header when staff want to hit 'reply' and have the email go to
the user.
Comment 11 Martin Renvoize 2020-02-25 10:16:41 UTC
Created attachment 99566 [details] [review]
Bug 22821: Correct to_address for issue notes

This patch updates the controller and service code to use the newly
introduced 'get_effective_email' routine from bug 22823 to obtain the
branches most appropriate email address.
Comment 12 Myka Kennedy Stephens 2020-02-28 00:30:42 UTC
Created attachment 99732 [details] [review]
Bug 22821: Add reply_address field to message_queue table

Signed-off-by: Myka Kennedy Stephens <mkstephens@lancasterseminary.edu>
Comment 13 Myka Kennedy Stephens 2020-02-28 00:30:45 UTC
Created attachment 99733 [details] [review]
Bug 22821: Schema Update

Signed-off-by: Myka Kennedy Stephens <mkstephens@lancasterseminary.edu>
Comment 14 Myka Kennedy Stephens 2020-02-28 00:30:48 UTC
Created attachment 99734 [details] [review]
Bug 22821: Use 'reply-to' address in issue notes

This patch updates the issue notices to not pass from_address and so
default to the branch email of the user for from_address. We now pass
the patrons email (cascading through email, emailpro, B_email) into the
reply_address field so mail servers are less likely to report emails as
spam and mail clients still have access to the patron address via the
reply-to header when staff want to hit 'reply' and have the email go to
the user.

Signed-off-by: Myka Kennedy Stephens <mkstephens@lancasterseminary.edu>
Comment 15 Myka Kennedy Stephens 2020-02-28 00:30:51 UTC
Created attachment 99735 [details] [review]
Bug 22821: Correct to_address for issue notes

This patch updates the controller and service code to use the newly
introduced 'get_effective_email' routine from bug 22823 to obtain the
branches most appropriate email address.

Signed-off-by: Myka Kennedy Stephens <mkstephens@lancasterseminary.edu>
Comment 16 Myka Kennedy Stephens 2020-02-28 00:39:04 UTC
These patches seem to fix the problem. Here was my test plan:

1. Created a simple SQL report (SELECT * FROM message_queue)
2. Ran report - results empty, no reply_address field
3. Applied patches
4. Restarted everything, updatedatabase, dbic
5. Ran report - results empty, new reply_address field appeared
6. Enabled AllowCheckoutNotes
7. Checked out an item to user with email address
8. Logged into OPAC as user
9. Entered a note for checked out item
10. Ran report - note email in results with user's email address appearing in reply_address field

Thank you, Martin!
Comment 17 Martin Renvoize 2020-02-28 15:40:29 UTC
Excellent testing Myka, thankyou :)
Comment 18 Martin Renvoize 2020-03-02 13:51:28 UTC
I'm going to move the reply-to handling into a submission for bug 10269 as that way it can then be a simple dependency for a bunch of other bugs that want it.
Comment 19 Martin Renvoize 2020-03-02 14:14:20 UTC
Created attachment 99908 [details] [review]
Bug 22821: Use 'reply-to' address in issue notes

This patch updates the issue notices to not pass from_address and so
default to the branch email of the user for from_address. We now pass
the patrons email (cascading through email, emailpro, B_email) into the
reply_address field so mail servers are less likely to report emails as
spam and mail clients still have access to the patron address via the
reply-to header when staff want to hit 'reply' and have the email go to
the user.

Signed-off-by: Myka Kennedy Stephens <mkstephens@lancasterseminary.edu>
Comment 20 Martin Renvoize 2020-03-02 14:14:23 UTC
Created attachment 99909 [details] [review]
Bug 22821: Correct to_address for issue notes

This patch updates the controller and service code to use the newly
introduced 'get_effective_email' routine from bug 22823 to obtain the
branches most appropriate email address.

Signed-off-by: Myka Kennedy Stephens <mkstephens@lancasterseminary.edu>
Comment 21 Jonathan Druart 2020-03-18 09:38:23 UTC
 84             my $to_address = $branch->branchemail || $branch->branchreplyto || C4::Context->ReplytoDefault || C4::Context->preference('KohaAdminEmailAddress');

The C4::Context->ReplytoDefault was super wrong!
Comment 22 Jonathan Druart 2020-03-18 09:49:36 UTC
Created attachment 100938 [details] [review]
Bug 22821: Use 'reply-to' address in issue notes

This patch updates the issue notices to not pass from_address and so
default to the branch email of the user for from_address. We now pass
the patrons email (cascading through email, emailpro, B_email) into the
reply_address field so mail servers are less likely to report emails as
spam and mail clients still have access to the patron address via the
reply-to header when staff want to hit 'reply' and have the email go to
the user.

Signed-off-by: Myka Kennedy Stephens <mkstephens@lancasterseminary.edu>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 23 Jonathan Druart 2020-03-18 09:49:40 UTC
Created attachment 100939 [details] [review]
Bug 22821: Correct to_address for issue notes

This patch updates the controller and service code to use the newly
introduced 'get_effective_email' routine from bug 22823 to obtain the
branches most appropriate email address.

Signed-off-by: Myka Kennedy Stephens <mkstephens@lancasterseminary.edu>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 24 Jonathan Druart 2020-03-18 09:49:44 UTC
Created attachment 100940 [details] [review]
Bug 22821: Rename method with ->inbound_email_address

The method has been renamed in the previous bug report.

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 25 Jonathan Druart 2020-03-18 10:08:15 UTC Comment hidden (obsolete)
Comment 26 Martin Renvoize 2020-03-18 15:44:57 UTC
Nice work everyone!

Pushed to master for 20.05
Comment 27 Martin Renvoize 2020-03-18 16:30:00 UTC
Nice work everyone!

Pushed to master for 20.05
Comment 28 Katrin Fischer 2020-03-19 07:31:54 UTC
I feel the description of the bug is not correct on this one, but not sure what it should be changed to - Martin, could you have a look?
Comment 29 Katrin Fischer 2020-03-20 07:13:07 UTC
Given it a go with what I think it does - feel free to correct :)
Comment 30 Joy Nelson 2020-04-03 20:13:11 UTC
not backported to 19.11.x  due to dependencies