From 8e0f591cc1df0de5c18c9a6490be195028342d20 Mon Sep 17 00:00:00 2001 From: Glenn Rice Date: Fri, 1 Dec 2023 21:53:42 -0600 Subject: [PATCH] Switch from using Mojolicious templates to a text .msg file and basic interpolation. Mojolicious templates allow to much, and as such are a security vulnerability. Too bad. This calls the WeBWorK::Utils::processEmailMessage method again. Then interpolates some achievement specific variable in after that. More variables may be needed, but this is a start. --- .../notifications/default.html.ep | 12 ----- .../achievements/notifications/default.msg | 10 +++++ .../WeBWorK/Tasks/AchievementNotification.pm | 44 +++++++++++-------- .../AchievementNotificationEditor.pm | 7 ++- .../AchievementList/default_table.html.ep | 2 +- .../existing_form.html.ep | 4 +- ...uctorAchievementNotificationEditor.html.ep | 25 ++++++----- 7 files changed, 57 insertions(+), 47 deletions(-) delete mode 100644 courses.dist/modelCourse/templates/achievements/notifications/default.html.ep create mode 100644 courses.dist/modelCourse/templates/achievements/notifications/default.msg diff --git a/courses.dist/modelCourse/templates/achievements/notifications/default.html.ep b/courses.dist/modelCourse/templates/achievements/notifications/default.html.ep deleted file mode 100644 index f3e0ba02e2..0000000000 --- a/courses.dist/modelCourse/templates/achievements/notifications/default.html.ep +++ /dev/null @@ -1,12 +0,0 @@ -<%= $user->first_name %> - -Congratulations, you just earned the "<%= $achievement->{name} %>" achievement! - -<%= $achievement->{description} %> - -% if ($nextLevelPoints) { -You have <%= $nextLevelPoints - $pointsEarned %> points remaining until your next level-up. - -% } -Great job! ---Prof. X diff --git a/courses.dist/modelCourse/templates/achievements/notifications/default.msg b/courses.dist/modelCourse/templates/achievements/notifications/default.msg new file mode 100644 index 0000000000..291aadb95e --- /dev/null +++ b/courses.dist/modelCourse/templates/achievements/notifications/default.msg @@ -0,0 +1,10 @@ +$FN + +Congratulations, you just earned the $ACHIEVEMENT_NAME achievement! + +$ACHIEVEMENT_DESCRIPTION + +You have $POINTS_TO_NEXT_LEVEL points remaining until your next level-up. + +Great job! +--Prof. X diff --git a/lib/Mojolicious/WeBWorK/Tasks/AchievementNotification.pm b/lib/Mojolicious/WeBWorK/Tasks/AchievementNotification.pm index 7ef964c93e..01e8fbb75d 100644 --- a/lib/Mojolicious/WeBWorK/Tasks/AchievementNotification.pm +++ b/lib/Mojolicious/WeBWorK/Tasks/AchievementNotification.pm @@ -18,12 +18,13 @@ use Mojo::Base 'Minion::Job', -signatures; use Email::Stuffer; use Email::Sender::Transport::SMTP; +use Mojo::File; use WeBWorK::Debug qw(debug); use WeBWorK::CourseEnvironment; use WeBWorK::DB; use WeBWorK::Localize; -use WeBWorK::Utils qw(createEmailSenderTransportSMTP); +use WeBWorK::Utils qw(processEmailMessage createEmailSenderTransportSMTP); # send student notification that they have earned an achievement sub run ($job, $mail_data) { @@ -60,25 +61,32 @@ sub send_achievement_notification ($job, $ce, $db, $mail_data) { my $user_record = $db->getUser($mail_data->{recipient}); die "Record for user $mail_data->{recipient} not found\n" unless ($user_record); die "User $mail_data->{recipient} does not have an email address -- skipping\n" - unless ($user_record->email_address =~ /\S/); - - my $template = "$ce->{courseDirs}{achievement_notifications}/$mail_data->{achievement}{email_template}"; - my $renderer = Mojo::Template->new(vars => 1); - - # what other data might need to be passed to the template? - my $body = $renderer->render_file( - $template, - { - ce => $ce, # holds achievement URLs - achievement => $mail_data->{achievement}, # full db record - setID => $mail_data->{set_id}, - nextLevelPoints => $mail_data->{nextLevelPoints}, - pointsEarned => $mail_data->{pointsEarned}, - user => $user_record, - user_status => $ce->status_abbrev_to_name($user_record->status) - } + unless $user_record->email_address =~ /\S/; + + my $body = processEmailMessage( + Mojo::File->new("$ce->{courseDirs}{achievement_notifications}/$mail_data->{achievement}{email_template}") + ->slurp('UTF-8'), + $user_record, $ce->status_abbrev_to_name($user_record->status) ); + { + # Additional macros that can be used in the email message. + # What other data might need to be passed to the template? + my $ACHIEVEMENT_NAME = $mail_data->{achievement}{name}; + my $ACHIEVEMENT_DESCRIPTION = $mail_data->{achievement}{description}; + my $SET_ID = $mail_data->{set_id}; + my $NEXT_LEVEL_POINTS = $mail_data->{nextLevelPoints}; + my $POINTS_EARNED = $mail_data->{pointsEarned}; + my $POINTS_TO_NEXT_LEVEL = $$NEXT_LEVEL_POINTS > $POINTS_EARNED ? $NEXT_LEVEL_POINTS - $POINTS_EARNED : 0; + + $body =~ s/\$ACHIEVEMENT_NAME/$ACHIEVEMENT_NAME/g; + $body =~ s/\$ACHIEVEMENT_DESCRIPTION/$ACHIEVEMENT_DESCRIPTION/g; + $body =~ s/\$SET_ID/$SET_ID/g; + $body =~ s/\$NEXT_LEVEL_POINTS/$NEXT_LEVEL_POINTS/g; + $body =~ s/\$POINTS_EARNED/$POINTS_EARNED/g; + $body =~ s/\$POINTS_TO_NEXT_LEVEL/$POINTS_TO_NEXT_LEVEL/g; + } + my $email = Email::Stuffer->to($user_record->email_address)->from($from)->subject($mail_data->{subject})->text_body($body) ->header('X-Remote-Host' => $mail_data->{remote_host}); diff --git a/lib/WeBWorK/ContentGenerator/Instructor/AchievementNotificationEditor.pm b/lib/WeBWorK/ContentGenerator/Instructor/AchievementNotificationEditor.pm index 9f26f87011..85caf68c68 100644 --- a/lib/WeBWorK/ContentGenerator/Instructor/AchievementNotificationEditor.pm +++ b/lib/WeBWorK/ContentGenerator/Instructor/AchievementNotificationEditor.pm @@ -53,8 +53,7 @@ sub pre_header_initialize ($c) { } $c->{sourceFilePath} = - $c->ce->{courseDirs}{achievement_notifications} . '/' - . ($c->{achievement}->email_template || 'default.html.ep'); + $c->ce->{courseDirs}{achievement_notifications} . '/' . ($c->{achievement}->email_template || 'default.msg'); my $actionID = $c->param('action'); @@ -199,8 +198,8 @@ sub save_as_handler ($c) { $c->addbadmessage($c->maketext('Achievement notification contents is empty.')) unless $c->stash->{achievementNotification}; - # Rescue the user in case they forgot to end the file name with .html.ep - $new_file_name =~ s/(\.html)?(\.ep)?$/.html.ep/; + # Rescue the user in case they forgot to end the file name with .msg + $new_file_name =~ s/(\.msg)?$/.msg/; # Construct the output file path. my $outputFilePath = $c->ce->{courseDirs}{achievement_notifications} . '/' . $new_file_name; diff --git a/templates/ContentGenerator/Instructor/AchievementList/default_table.html.ep b/templates/ContentGenerator/Instructor/AchievementList/default_table.html.ep index 72da91f0f7..714596999d 100644 --- a/templates/ContentGenerator/Instructor/AchievementList/default_table.html.ep +++ b/templates/ContentGenerator/Instructor/AchievementList/default_table.html.ep @@ -65,7 +65,7 @@ url_for('instructor_achievement_notification', achievementID => $achievement_id), ), begin =%> <%= $_->email_template - ? maketext('Edit Email Template [_1]', $_->email_template =~ s/\.html\.ep//r) + ? maketext('Edit Email Template [_1]', $_->email_template =~ s/\.msg//r) : maketext('Enable Email Notification') =%> <% end =%> diff --git a/templates/ContentGenerator/Instructor/AchievementNotificationEditor/existing_form.html.ep b/templates/ContentGenerator/Instructor/AchievementNotificationEditor/existing_form.html.ep index 882400c279..b95231f182 100644 --- a/templates/ContentGenerator/Instructor/AchievementNotificationEditor/existing_form.html.ep +++ b/templates/ContentGenerator/Instructor/AchievementNotificationEditor/existing_form.html.ep @@ -6,8 +6,8 @@
% my $relativeSourceFilePath = $c->getRelativeSourceFilePath($c->{sourceFilePath}); <%= select_field 'action.existing.target_file' => [ - map { [ $_ =~ s/(\.html)?\.ep//r => $_, $_ eq $relativeSourceFilePath ? (selected => undef) : () ] } - @{ path($ce->{courseDirs}{achievement_notifications})->list->grep(qr/\.ep$/) + map { [ $_ =~ s/\.msg$//r => $_, $_ eq $relativeSourceFilePath ? (selected => undef) : () ] } + @{ path($ce->{courseDirs}{achievement_notifications})->list->grep(qr/\.msg$/) ->map(sub { $c->getRelativeSourceFilePath($_) })->to_array } ], id => 'action.existing.target_file_id', class => 'form-select form-select-sm d-inline w-auto', diff --git a/templates/HelpFiles/InstructorAchievementNotificationEditor.html.ep b/templates/HelpFiles/InstructorAchievementNotificationEditor.html.ep index a12b112f8c..cedb3d60fa 100644 --- a/templates/HelpFiles/InstructorAchievementNotificationEditor.html.ep +++ b/templates/HelpFiles/InstructorAchievementNotificationEditor.html.ep @@ -43,16 +43,21 @@

<%= maketext('Template Substitutions') %>

- <%== maketext('The following variables are available for use in the template. These variables can be used inside ' - . '[_1] tags. For example, [_2] will insert the description of the achievement into the email body.', - '<%= ... %>', '<%= $achievement->description %>') =%> + <%== maketext('The following variables are available for use in the template.') =%>