diff --git a/bin/addcourse b/bin/addcourse index 79c1bcbbc7..ecbc9b0ded 100755 --- a/bin/addcourse +++ b/bin/addcourse @@ -58,7 +58,7 @@ BEGIN { use lib "$ENV{WEBWORK_ROOT}/lib"; use WeBWorK::CourseEnvironment; -use WeBWorK::Utils qw(runtime_use cryptPassword); +use WeBWorK::Utils qw(runtime_use cryptPassword isValidEmailAddress); use WeBWorK::Utils::CourseManagement qw(addCourse); use WeBWorK::File::Classlist qw(parse_classlist); use WeBWorK::DB::Record::User; @@ -101,6 +101,14 @@ if ($users) { my %record = %$record; my $user_id = $record{user_id}; + if (defined $record{email_address} + && $record{email_address} ne '' + && !isValidEmailAddress($record{email_address})) + { + warn qq{Invalid email address "$record{email_address}" for user $user_id will not be set.\n}; + $record{email_address} = ''; + } + # Set the default status if the status field is not set. $record{status} = $ce->{statuses}{Enrolled}{abbrevs}[0] unless $record{status}; diff --git a/bin/importClassList.pl b/bin/importClassList.pl index 52316f17cf..e175e31fce 100755 --- a/bin/importClassList.pl +++ b/bin/importClassList.pl @@ -17,7 +17,7 @@ BEGIN use WeBWorK::DB qw(check_user_id); use WeBWorK::File::Classlist qw(parse_classlist); -use WeBWorK::Utils qw(cryptPassword); +use WeBWorK::Utils qw(cryptPassword isValidEmailAddress); use WeBWorK::File::Classlist qw(parse_classlist); if ((scalar(@ARGV) != 2)) { @@ -89,6 +89,14 @@ sub importUsersFromCSV { next; } + if (defined $record{email_address} + && $record{email_address} ne '' + && !isValidEmailAddress($record{email_address})) + { + warn "Invalid email address '$record{email_address}' for user $user_id will not be set.\n"; + $record{email_address} = ''; + } + # set default status is status field is "empty" $record{status} = $default_status_abbrev unless defined $record{status} and $record{status} ne ""; diff --git a/lib/WeBWorK/ContentGenerator/API/CourseActions.pm b/lib/WeBWorK/ContentGenerator/API/CourseActions.pm index 3d5eaefa73..9552b73aae 100644 --- a/lib/WeBWorK/ContentGenerator/API/CourseActions.pm +++ b/lib/WeBWorK/ContentGenerator/API/CourseActions.pm @@ -8,7 +8,7 @@ use Mojo::JSON qw(false true); use WeBWorK::DB; use WeBWorK::DB::Utils qw(initializeUserProblem); -use WeBWorK::Utils qw(cryptPassword); +use WeBWorK::Utils qw(cryptPassword isValidEmailAddress); use WeBWorK::Utils::CourseManagement qw(addCourse); use WeBWorK::Utils::Files qw(surePathToFile path_is_subdir); use WeBWorK::ConfigValues qw(getConfigValues); @@ -133,6 +133,9 @@ sub addUser ($c) { # Add a new user. my $ce = $c->ce; + return $c->renderError('The email_address parameter is not a valid email address.') + if $c->req->param('email_address') && !isValidEmailAddress($c->req->param('email_address')); + my $new_student = $db->newUser(user_id => $c->req->param('user_id'), status => $ce->{statuses}{Enrolled}{abbrevs}[0]); $new_student->first_name($c->req->param('first_name')) if $c->req->param('first_name'); diff --git a/lib/WeBWorK/ContentGenerator/CourseAdmin.pm b/lib/WeBWorK/ContentGenerator/CourseAdmin.pm index eef18d5af9..5738232c6f 100644 --- a/lib/WeBWorK/ContentGenerator/CourseAdmin.pm +++ b/lib/WeBWorK/ContentGenerator/CourseAdmin.pm @@ -15,7 +15,7 @@ use Time::localtime; use WeBWorK::CourseEnvironment; use WeBWorK::Debug qw(debug); -use WeBWorK::Utils qw(cryptPassword trim_spaces); +use WeBWorK::Utils qw(cryptPassword trim_spaces isValidEmailAddress); use WeBWorK::Utils::CourseManagement qw( addCourse renameCourse @@ -275,6 +275,10 @@ sub add_course_validate ($c) { $_ ); } + + my $email = trim_spaces($c->param("add_initial_email_$_")); + push @errors, $c->maketext('The email address for user number [_1] is not valid.', $_) + if $email ne '' && !isValidEmailAddress($email); } return @errors; diff --git a/lib/WeBWorK/ContentGenerator/Feedback.pm b/lib/WeBWorK/ContentGenerator/Feedback.pm index ee77fbe4d8..21569e5e42 100644 --- a/lib/WeBWorK/ContentGenerator/Feedback.pm +++ b/lib/WeBWorK/ContentGenerator/Feedback.pm @@ -12,7 +12,7 @@ use Email::Stuffer; use Try::Tiny; use WeBWorK::Upload; -use WeBWorK::Utils qw(createEmailSenderTransportSMTP fetchEmailRecipients formatEmailSubject); +use WeBWorK::Utils qw(createEmailSenderTransportSMTP fetchEmailRecipients formatEmailSubject isValidEmailAddress); # request parameters used # @@ -91,7 +91,7 @@ sub initialize ($c) { $c->stash->{send_error} = $c->maketext('No Sender specified.'); return; } - unless ($from =~ /^[a-zA-Z0-9.!#$%&\'*+\/=?^_`~\-]+@[a-zA-Z0-9\-]+\.[a-zA-Z0-9.\-]+$/) { + unless (isValidEmailAddress($from)) { $c->stash->{send_error} = $c->maketext('Sender is not a valid email address.'); return; } diff --git a/lib/WeBWorK/ContentGenerator/Instructor/AddUsers.pm b/lib/WeBWorK/ContentGenerator/Instructor/AddUsers.pm index d4543315aa..b5935578cc 100644 --- a/lib/WeBWorK/ContentGenerator/Instructor/AddUsers.pm +++ b/lib/WeBWorK/ContentGenerator/Instructor/AddUsers.pm @@ -7,7 +7,7 @@ WeBWorK::ContentGenerator::Instructor::AddUsers - Menu interface for adding user =cut -use WeBWorK::Utils qw/cryptPassword trim_spaces/; +use WeBWorK::Utils qw/cryptPassword trim_spaces isValidEmailAddress/; use WeBWorK::Utils::Instructor qw(assignSetsToUsers); sub initialize ($c) { @@ -48,7 +48,11 @@ sub initialize ($c) { $newUser->comment(trim_spaces($c->param("comment_$i"))); $newUser->status($c->param("status_$i")); - eval { $db->addUser($newUser) }; + eval { + die $c->maketext('Invalid email address.') . "\n" + if $newUser->email_address ne '' && !isValidEmailAddress($newUser->email_address); + $db->addUser($newUser); + }; if ($@) { push( @{ $c->{studentEntryReport} }, diff --git a/lib/WeBWorK/ContentGenerator/Instructor/UserList.pm b/lib/WeBWorK/ContentGenerator/Instructor/UserList.pm index c1fe85a1f4..18b5ee9fab 100644 --- a/lib/WeBWorK/ContentGenerator/Instructor/UserList.pm +++ b/lib/WeBWorK/ContentGenerator/Instructor/UserList.pm @@ -48,7 +48,7 @@ Export users: use Mojo::File; use WeBWorK::File::Classlist qw(parse_classlist write_classlist); -use WeBWorK::Utils qw(cryptPassword x); +use WeBWorK::Utils qw(cryptPassword isValidEmailAddress x); use constant HIDE_USERS_THRESHOLD => 200; use constant EDIT_FORMS => [qw(save_edit cancel_edit)]; @@ -106,11 +106,21 @@ use constant FIELDS => [ # a subroutine that takes the parameter value as its only argument, and returns a translatable error string if the # parameter value is not valid for the field, and 0 otherwise. use constant FIELD_PROPERTIES => { - user_id => { name => x('Login Name') }, - first_name => { name => x('First Name'), type => 'text', attributes => { size => 10 } }, - last_name => { name => x('Last Name'), type => 'text', attributes => { size => 10 } }, - email_address => { name => x('Email Address'), type => 'text', attributes => { size => 20 } }, - student_id => { name => x('Student ID'), type => 'text', attributes => { size => 11 } }, + user_id => { name => x('Login Name') }, + first_name => { name => x('First Name'), type => 'text', attributes => { size => 10 } }, + last_name => { name => x('Last Name'), type => 'text', attributes => { size => 10 } }, + email_address => { + name => x('Email Address'), + type => 'text', + attributes => { size => 20 }, + perlValidate => sub { + my $value = shift; + return $value =~ /\S/ && !isValidEmailAddress($value) + ? (x('Email address for [_1] unchanged. The value given is not a valid email address.'))[0] + : 0; + } + }, + student_id => { name => x('Student ID'), type => 'text', attributes => { size => 11 } }, status => { name => x('Enrollment Status'), type => 'status' }, accommodation_time_factor => { name => x('Accommodation Time Factor'), @@ -709,7 +719,7 @@ sub importUsersFromCSV ($c, $fileName, $replaceExisting, $fallbackPasswordSource my $default_permission_level = $ce->{default_permission_level}; my (@usersToInsert, @passwordsToInsert, @permissionsToInsert, - @usersToUpdate, @passwordsToUpdate, @permissionsToUpdate, @skipped); + @usersToUpdate, @passwordsToUpdate, @permissionsToUpdate, @skipped, @invalidEmailUsers); # get list of hashrefs representing lines in classlist file my @classlist = parse_classlist("$ce->{courseDirs}{templates}/$fileName"); @@ -739,6 +749,14 @@ sub importUsersFromCSV ($c, $fileName, $replaceExisting, $fallbackPasswordSource next; } + if (defined $record{email_address} + && $record{email_address} ne '' + && !isValidEmailAddress($record{email_address})) + { + push @invalidEmailUsers, $user_id; + $record{email_address} = ''; + } + # set default status is status field is "empty" $record{status} = $default_status_abbrev unless defined $record{status} && $record{status} ne ''; @@ -789,6 +807,14 @@ sub importUsersFromCSV ($c, $fileName, $replaceExisting, $fallbackPasswordSource } } + $c->addbadmessage($c->maketext( + 'The email [plural,_1,address was,addresses were] not valid for the following [plural,_1,user,users] ' + . 'and [plural,_1,was,were] not set: [_2]', + scalar(@invalidEmailUsers), + join(', ', @invalidEmailUsers) + )) + if @invalidEmailUsers; + $db->User->insert_records(\@usersToInsert) if @usersToInsert; $db->User->update_records(\@usersToUpdate) if @usersToUpdate; $db->Password->insert_records(\@passwordsToInsert) if @passwordsToInsert; diff --git a/lib/WeBWorK/ContentGenerator/Options.pm b/lib/WeBWorK/ContentGenerator/Options.pm index 7208281f4d..dbc66c29aa 100644 --- a/lib/WeBWorK/ContentGenerator/Options.pm +++ b/lib/WeBWorK/ContentGenerator/Options.pm @@ -7,7 +7,7 @@ WeBWorK::ContentGenerator::Options - Change user options. =cut -use WeBWorK::Utils qw(cryptPassword utf8Crypt); +use WeBWorK::Utils qw(cryptPassword utf8Crypt isValidEmailAddress); use WeBWorK::Localize; sub page_title ($c) { @@ -128,17 +128,24 @@ sub initialize ($c) { my $newA = $c->param('newAddress'); if ($changeOptions && $authz->hasPermissions($userID, 'change_email_address') && $newA) { - my $oldA = $c->{effectiveUser}->email_address; - $c->{effectiveUser}->email_address($newA); - eval { $db->putUser($c->{effectiveUser}) }; - if ($@) { - $c->{effectiveUser}->email_address($oldA); - $c->log->error("Unable to save new email address for $userID: $@"); - $c->addbadmessage($c->maketext('Your email address has not been changed due to an internal error.')); + if (!isValidEmailAddress($newA)) { + $c->addbadmessage( + $c->maketext( + 'Your email address has not been changed. The new email address you entered is not valid.') + ); } else { - $c->param('currAddress', $c->param('newAddress')); - $c->param('newAddress', undef); - $c->addgoodmessage($c->maketext('Your email address has been changed.')); + my $oldA = $c->{effectiveUser}->email_address; + $c->{effectiveUser}->email_address($newA); + eval { $db->putUser($c->{effectiveUser}) }; + if ($@) { + $c->{effectiveUser}->email_address($oldA); + $c->log->error("Unable to save new email address for $userID: $@"); + $c->addbadmessage($c->maketext('Your email address has not been changed due to an internal error.')); + } else { + $c->param('currAddress', $newA); + $c->param('newAddress', undef); + $c->addgoodmessage($c->maketext('Your email address has been changed.')); + } } } diff --git a/lib/WeBWorK/Utils.pm b/lib/WeBWorK/Utils.pm index a9fee1d831..de42b34be0 100644 --- a/lib/WeBWorK/Utils.pm +++ b/lib/WeBWorK/Utils.pm @@ -1,6 +1,7 @@ package WeBWorK::Utils; use Mojo::Base 'Exporter', -signatures; +use Email::Address::XS; use Email::Sender::Transport::SMTP; use Mojo::JSON qw(from_json to_json); use Mojo::Util qw(b64_encode b64_decode encode decode); @@ -31,6 +32,7 @@ our @EXPORT_OK = qw( fetchEmailRecipients processEmailMessage createEmailSenderTransportSMTP + isValidEmailAddress generateURLs formatEmailSubject getAssetURL @@ -373,6 +375,10 @@ sub createEmailSenderTransportSMTP ($ce) { }); } +sub isValidEmailAddress ($address) { + return Email::Address::XS->parse_bare_address($address // '')->is_valid; +} + sub generateURLs ($c, %params) { my $db = $c->db; my $userName = $c->param('user'); @@ -803,6 +809,12 @@ Usage: C This returns an C object for use in sending emails. A valid C object must be provided in C<$ce>. +=head2 isValidEmailAddress + +Usage: C + +Returns true if C<$address> is a valid email address, and false otherwise. + =head2 generateURLs Usage: C