Implement override for sender address in email messages

dlglin · dlglin · commit f3881825f348 · 2024-07-19T21:11:31.000Z
diff --git a/conf/defaults.config b/conf/defaults.config
@@ -42,6 +42,16 @@ include("VERSION");  # get WW version
 # Mail Settings
 ################################################################################
 
+# $generic_sender_name will be used as the "From:" name on all feedback emails
+# sent without a defined user.
+$generic_sender_name = '';
+
+# The following variables will override the "From:" address in messages sent by
+# the named feature.
+$feedback_sender_email = ''; # For student feedback
+$instructor_sender_email = ''; # For instructors emailing students
+$jitar_sender_email = ''; # For notifications of incomplete JiTaR sets
+
 # By default, feedback is sent to all users who have permission to
 # receive_feedback in a course. If this list is non-empty, feedback is also sent
 # to the addresses specified here.
diff --git a/conf/localOverrides.conf.dist b/conf/localOverrides.conf.dist
@@ -38,6 +38,24 @@
 # Additional mail settings in defaults.config can be overridden here
 ################################################################################
 
+# $generic_sender_name will be used as the "From:" name on all feedback emails
+# sent without a defined user.
+
+$generic_sender_name = '';
+
+# The following variables will override the "From:" address in messages sent by
+# the named feature.
+# When one of these is set, all messages sent by that feature will use the
+# supplied email address as the "From:" address, and the address of the relevant
+# user will be set as the "Reply-to:" address for the message.
+# These may be required if you have provided an SMTP username and password in
+# site.conf.  They can also be used to improve email verification and avoid
+# messages getting filtered as spam.
+
+$feedback_sender_email = ''; # For student feedback
+$instructor_sender_email = ''; # For instructors emailing students
+$jitar_sender_email = ''; # For notifications of incomplete JiTaR sets
+
 # By default, feedback is sent to all users who have permission to
 # receive_feedback in a course. If this list is non-empty, feedback is also sent
 # to the addresses specified here.
diff --git a/conf/site.conf.dist b/conf/site.conf.dist
@@ -287,7 +287,11 @@ $mail{tls_allowed} = 0;
 
 # SMTP Authentication
 # If your SMTP server requires authentication you can provide the username and password
-# for the account on the mail server
+# for the account on the mail server.
+# If you set these credentials, you may need to define the variables $feedback_sender_email,
+# $instructor_sender_email and $jitar_sender_email in localOverrides.conf, as some SMTP
+# servers require the "From:" address of outgoing emails to match this username.  Setting
+# those sender variables will then put the user's email address in the "Reply-to:" field.
 
 #$mail{smtpUsername} = '';
 #$mail{smtpPassword} = '';
diff --git a/courses.dist/modelCourse/templates/email/welcome.msg b/courses.dist/modelCourse/templates/email/welcome.msg
@@ -1,6 +1,7 @@
 ##  template for a Welcome message to be emailed to class (delete this line)
+## Note that the From: address will be replaced by the email address of the account
+## from which the message is sent.
 From: teacher@somewhere.edu (Jan Teacher)
-Reply-To: teacher@somewhere.edu
 Subject: online homework for Math 123
 Message:
     Hi $FN,
diff --git a/lib/Mojolicious/WeBWorK/Tasks/SendInstructorEmail.pm b/lib/Mojolicious/WeBWorK/Tasks/SendInstructorEmail.pm
@@ -79,9 +79,18 @@ sub mail_message_to_recipients ($job, $ce, $db, $mail_data) {
 			$mail_data->{merge_data}
 		);
 
-		my $email =
-			Email::Stuffer->to($user_record->email_address)->from($mail_data->{from})->subject($mail_data->{subject})
-			->text_body($msg)->header('X-Remote-Host' => $mail_data->{remote_host});
+		my $email;
+		if ($ce->{instructor_sender_email}) {
+			$email =
+				Email::Stuffer->to($user_record->email_address)
+				->from($mail_data->{from_name} . ' <' . $ce->{instructor_sender_email} . '>')
+				->reply_to($mail_data->{from})->subject($mail_data->{subject})->text_body($msg)
+				->header('X-Remote-Host' => $mail_data->{remote_host});
+		} else {
+			$email =
+				Email::Stuffer->to($user_record->email_address)->from($mail_data->{from})
+				->subject($mail_data->{subject})->text_body($msg)->header('X-Remote-Host' => $mail_data->{remote_host});
+		}
 
 		eval {
 			$email->send_or_die({
diff --git a/lib/WeBWorK/ContentGenerator/Feedback.pm b/lib/WeBWorK/ContentGenerator/Feedback.pm
@@ -185,10 +185,17 @@ $emailableURL
 			$msg .= "***** Data about the environment: *****\n\n" . Dumper($ce) . "\n\n";
 		}
 
-		my $email =
-			Email::Stuffer->to(join(',', @recipients))->from($sender)->subject($subject)->text_body($msg)
-			->header('X-Remote-Host' => $remote_host);
-
+		my $from_name = $user ? $user->full_name : $ce->{generic_sender_name};
+		my $email;
+		if ($ce->{feedback_sender_email}) {
+			$email =
+				Email::Stuffer->to(join(',', @recipients))->from("$from_name <$ce->{feedback_sender_email}>")
+				->reply_to($sender)->subject($subject)->text_body($msg)->header('X-Remote-Host' => $remote_host);
+		} else {
+			$email =
+				Email::Stuffer->to(join(',', @recipients))->from($sender)->subject($subject)->text_body($msg)
+				->header('X-Remote-Host' => $remote_host);
+		}
 		# Extra headers
 		$email->header('X-WeBWorK-Route',  $route)    if defined $route;
 		$email->header('X-WeBWorK-Course', $courseID) if defined $courseID;
diff --git a/lib/WeBWorK/ContentGenerator/Instructor/SendMail.pm b/lib/WeBWorK/ContentGenerator/Instructor/SendMail.pm
@@ -87,8 +87,8 @@ sub initialize ($c) {
 
 	# Store data
 	$c->{defaultPreviewUser} = $ur;
-	$c->{defaultFrom}        = $ur->rfc822_mailbox;
-	$c->{defaultReply}       = $ur->rfc822_mailbox;
+	$c->{from}               = $ur->rfc822_mailbox;
+	$c->{from_name}          = $ur->full_name;
 	$c->{defaultSubject}     = $c->stash('courseID') . ' notice';
 	$c->{merge_file}         = $mergefile // '';
 
@@ -196,13 +196,11 @@ sub initialize ($c) {
 	}
 
 	# Get inputs
-	my ($from, $replyTo, $r_text, $subject);
+	my ($r_text, $subject);
 	if ($input_source eq 'file') {
 		if ($input_file) {
-			($from, $replyTo, $subject, $r_text) = $c->read_input_file("$emailDirectory/$input_file");
+			($subject, $r_text) = $c->read_input_file("$emailDirectory/$input_file");
 		} else {
-			$from    = $c->{defaultFrom};
-			$replyTo = $c->{defaultReply};
 			$subject = $c->{defaultSubject};
 
 			# If action is openMessage and no file was found, then 'None' was selected.
@@ -212,16 +210,12 @@ sub initialize ($c) {
 				$c->param('savefilename', 'default.msg') if $c->param('savefilename');
 			}
 		}
-		$c->param('from',    $from)    if $from;
-		$c->param('replyTo', $replyTo) if $replyTo;
 		$c->param('subject', $subject) if $subject;
 		$c->param('body',    $$r_text) if $r_text;
 	} elsif ($input_source eq 'form') {
 		# read info from the form
 		# bail if there is no message body
 
-		$from    = $c->param('from');
-		$replyTo = $c->param('replyTo');
 		$subject = $c->param('subject');
 		my $body = $c->param('body');
 		# Sanity check: body must contain non-white space when previewing message.
@@ -233,8 +227,6 @@ sub initialize ($c) {
 	my $remote_host = $c->tx->remote_address || "UNKNOWN";
 
 	# Store data
-	$c->{from}        = $from;
-	$c->{replyTo}     = $replyTo;
 	$c->{subject}     = $subject;
 	$c->{remote_host} = $remote_host;
 	$c->{r_text}      = $r_text;
@@ -280,8 +272,7 @@ sub initialize ($c) {
 		$temp_body =~ s/\r\n/\n/g;
 		$temp_body = join(
 			"\n",
-			"From: $from",
-			"Reply-To: $replyTo",
+			"From: $c->{from}",
 			"Subject: $subject",
 			"Content-Type: text/plain; charset=UTF-8",
 			"Message:",
@@ -315,15 +306,6 @@ sub initialize ($c) {
 			return;
 		}
 
-		# verify format of Reply-to address (zero or more valid rfc2822/ref5322 addresses)
-		if (defined $c->{replyTo} and $c->{replyTo} ne "") {
-			my @parsed_replyto_addrs = Email::Address::XS->parse($c->{replyTo});
-			unless (@parsed_replyto_addrs > 0) {
-				$c->addbadmessage($c->maketext("Invalid Reply-to address."));
-				return;
-			}
-		}
-
 		# Check that recipients have been selected.
 		unless (@{ $c->{ra_send_to} }) {
 			$c->addbadmessage(
@@ -353,7 +335,7 @@ sub initialize ($c) {
 				text        => ${ $c->{r_text} // \'' },
 				merge_data  => $c->{rh_merge_data},
 				from        => $c->{from},
-				defaultFrom => $c->{defaultFrom},
+				from_name   => $c->{from_name},
 				remote_host => $c->{remote_host},
 			} ],
 			{ notes => { courseID => $c->stash('courseID') } }
@@ -393,10 +375,9 @@ sub print_preview ($c) {
 	# Note that this escaping is done in the Mojolicious template automatically.
 	$msg = join(
 		"",
-		"To: ",       $c->{preview_user}->email_address, "\n",
-		"From: ",     $c->{from},                        "\n",
-		"Reply-To: ", $c->{replyTo},                     "\n",
-		"Subject: ",  $c->{subject},                     "\n",
+		"To: ",      $c->{preview_user}->email_address, "\n",
+		"From: ",    $c->{from},                        "\n",
+		"Subject: ", $c->{subject},                     "\n",
 		# In a real mails we would UTF-8 encode the message and give the Content-Type header. For the preview which is
 		# displayed as html, just add the header, but do NOT use Encode::encode("UTF-8",$msg).
 		"Content-Type: text/plain; charset=UTF-8\n\n",
@@ -435,7 +416,7 @@ sub saveMessageFile ($c, $body, $msgFileName) {
 sub read_input_file ($c, $filePath) {
 	my ($text, @text);
 	my $header = '';
-	my ($subject, $from, $replyTo);
+	my $subject;
 
 	open my $FILE, "<:encoding(UTF-8)", $filePath
 		or do { $c->addbadmessage($c->maketext(q{Can't open [_1]}, $filePath)); return };
@@ -445,17 +426,12 @@ sub read_input_file ($c, $filePath) {
 	$text = join('', <$FILE>);
 	close $FILE;
 
-	$text   =~ s/^\s*//;           # remove initial white space if any.
-	$header =~ /^From:\s(.*)$/m;
-	$from = $1 || $c->{defaultFrom};
-
-	$header =~ /^Reply-To:\s(.*)$/m;
-	$replyTo = $1 || $c->{defaultReply};
+	$text =~ s/^\s*//;    # remove initial white space if any.
 
 	$header =~ /^Subject:\s(.*)$/m;
 	$subject = $1 || $c->{defaultSubject};
 
-	return ($from, $replyTo, $subject, \$text);
+	return ($subject, \$text);
 }
 
 sub get_message_file_names ($c) {
diff --git a/lib/WeBWorK/Utils/ProblemProcessing.pm b/lib/WeBWorK/Utils/ProblemProcessing.pm
@@ -464,7 +464,13 @@ Recitation: $recitation
 Comment:    $comment
 /;
 
-	my $email = Email::Stuffer->to(join(',', @recipients))->from($sender)->subject($subject)->text_body($msg);
+	my $email;
+	if ($ce->{jitar_sender_email}) {
+		$email = Email::Stuffer->to(join(',', @recipients))->from("$full_name <$ce->{jitar_sender_email}>")
+			->reply_to($sender)->subject($subject)->text_body($msg);
+	} else {
+		$email = Email::Stuffer->to(join(',', @recipients))->from($sender)->subject($subject)->text_body($msg);
+	}
 
 	# Extra headers
 	$email->header('X-WeBWorK-Course: ', $courseID) if defined $courseID;
diff --git a/templates/ContentGenerator/Instructor/SendMail/main_form.html.ep b/templates/ContentGenerator/Instructor/SendMail/main_form.html.ep
@@ -34,15 +34,7 @@
 						<%= label_for from => maketext('From:'),
 							class => 'col-sm-3 col-form-label col-form-label-sm' =%>
 						<div class="col-sm-9">
-							<%= text_field from => $c->{from}, id => 'from',
-								class => 'form-control form-control-sm' =%>
-						</div>
-					</div>
-					<div class="row mb-1">
-						<%= label_for replyTo => maketext('Reply-To:'),
-							class => 'col-sm-3 col-form-label col-form-label-sm' =%>
-						<div class="col-sm-9">
-							<%= text_field replyTo => $c->{replyTo}, id => 'replyTo',
+							<%= text_field from => $c->{from}, id => 'from', readonly => undef, disabled => undef,
 								class => 'form-control form-control-sm' =%>
 						</div>
 					</div>
