diff --git a/.typos.toml b/.typos.toml index 44a547f652..a4bad401be 100644 --- a/.typos.toml +++ b/.typos.toml @@ -30,6 +30,7 @@ crypted = "crypted" crypted_ = "crypted_" cryptedPassword = "cryptedPassword" crypted_password = "crypted_password" +isCryptedPassword = "isCryptedPassword" # Variable name for the paired select-all checkboxes. pairedSelectAlls = "pairedSelectAlls" # Function name (write JSON to file). diff --git a/bin/dump-past-answers.pl b/bin/dump-past-answers.pl index 49ec33863b..9d86a90466 100755 --- a/bin/dump-past-answers.pl +++ b/bin/dump-past-answers.pl @@ -144,7 +144,7 @@ sub write_past_answers_csv { $row[1] = $courseID; - my @users = $db->getUsersWhere({ user_id => { not_like => 'set_id:%' } }); + my @users = $db->getUsersWhere; for my $user (@users) { my $userID = $user->user_id; diff --git a/bin/importClassList.pl b/bin/importClassList.pl index e175e31fce..40efabbb03 100755 --- a/bin/importClassList.pl +++ b/bin/importClassList.pl @@ -74,7 +74,7 @@ sub importUsersFromCSV { print "Saw user_id = $user_id\n"; - unless (WeBWorK::DB::check_user_id($user_id)) { # try to catch lines with bad characters + unless (eval { WeBWorK::DB::check_user_id($user_id) }) { # try to catch lines with bad characters push @skipped, $user_id; next; } diff --git a/lib/Caliper/Entity.pm b/lib/Caliper/Entity.pm index f9fd191cd6..7739bccde1 100644 --- a/lib/Caliper/Entity.pm +++ b/lib/Caliper/Entity.pm @@ -131,7 +131,7 @@ sub problem_set ($c, $set_id) { time_limit_cap => $problem_set->time_limit_cap, restrict_ip => $problem_set->restrict_ip, relax_restrict_ip => $problem_set->relax_restrict_ip, - restricted_login_proctor => $problem_set->restricted_login_proctor, + restricted_login_password => $problem_set->restricted_login_password, hide_hint => $problem_set->hide_hint, restrict_prob_progression => $problem_set->restrict_prob_progression } diff --git a/lib/Mojolicious/WeBWorK/Tasks/LMSRosterSync.pm b/lib/Mojolicious/WeBWorK/Tasks/LMSRosterSync.pm index 94203be472..7482f326b7 100644 --- a/lib/Mojolicious/WeBWorK/Tasks/LMSRosterSync.pm +++ b/lib/Mojolicious/WeBWorK/Tasks/LMSRosterSync.pm @@ -86,7 +86,7 @@ async sub synchronizeRoster ($job) { my (@messages, @addedUsers, @userAchievementRecordsToAdd, @globalAchievementRecordsToAdd, %usersInLMSCourse); my $updatedUsers = 0; - my %users = map { $_->user_id => $_ } $db->getUsersWhere({ user_id => { not_like => 'set_id:%' } }); + my %users = map { $_->user_id => $_ } $db->getUsersWhere; my @achievements = $db->getAchievementsWhere({ enabled => 1 }, ['achievement_id']); @@ -263,7 +263,7 @@ async sub synchronizeRoster ($job) { if @globalAchievementRecordsToAdd; my %permissionLevels = - map { $_->user_id => $_->permission } $db->getPermissionLevelsWhere({ user_id => { not_like => 'set_id:%' } }); + map { $_->user_id => $_->permission } $db->getPermissionLevelsWhere; # Mark all users not in the LMS roster and at or below the LTIAccountCreationCutoff as dropped. my @droppedUsers; diff --git a/lib/WeBWorK/Authen/Proctor.pm b/lib/WeBWorK/Authen/Proctor.pm index c72ecede03..26e9063941 100644 --- a/lib/WeBWorK/Authen/Proctor.pm +++ b/lib/WeBWorK/Authen/Proctor.pm @@ -10,8 +10,9 @@ WeBWorK::Authen::Proctor - Authenticate gateway test proctors. use strict; use warnings; -use WeBWorK::Utils qw(x); +use WeBWorK::Utils qw(x utf8Crypt isCryptedPassword); use WeBWorK::Utils::Sets qw(test_version_is_complete); +use WeBWorK::Debug qw(debug); use constant GENERIC_ERROR_MESSAGE => x('Invalid user ID or password.'); @@ -68,37 +69,34 @@ sub verify { return $self->SUPER::verify(@_); } -# This is similar to the method in the base class, with these differences: -# 1. no guest logins -# 2. no cookie -# 3. no session key -# 4. user_id/password come from POST request params proctor_user/proctor_passwd sub get_credentials { my ($self) = @_; my $c = $self->{c}; - my $set_id = $c->stash('setID'); - - if (defined $c->req->body_params->param('proctor_user')) { - $self->{user_id} = $c->req->body_params->param('proctor_user'); - $self->{user_id} = "set_id:$set_id" if $self->{user_id} eq $set_id; + if (defined $c->req->body_params->param('proctor_user') || $c->req->body_params->param('set_level_proctor')) { + $self->{user_id} = $c->req->body_params->param('proctor_user') // ''; + $self->{set_level_proctor} = 1 if $c->req->body_params->param('set_level_proctor'); $self->{password} = $c->req->body_params->param('proctor_passwd'); $self->{login_type} = $c->req->body_params->param('submitAnswers') ? 'proctor_grading' : 'proctor_login'; $self->{credential_source} = 'params'; + debug('proctor credential source: "params", ', + $self->{set_level_proctor} ? 'using set level proctor' : qq{user: "$self->{user_id}"}); return 1; } elsif ($c->authen->session('proctor_authorization_granted') && !$c->req->body_params->param('submitAnswers')) { $self->{login_type} = 'proctor_login'; $self->{credential_source} = 'session'; + debug('proctor credential source: "session"'); return 1; } return 0; } -# If proctor authorization is granted a proctor user is not needed. So skip checking the user. +# If proctor authorization is granted or if a set level proctor password is used then a proctor user is not needed. +# So skip checking the user. sub check_user { my $self = shift; - return 1 if $self->{credential_source} eq 'session'; + return 1 if $self->{credential_source} eq 'session' || $self->{set_level_proctor}; return $self->SUPER::check_user; } @@ -136,15 +134,9 @@ sub verify_normal_user { my $db = $c->db; my $authz = $c->authz; - my $user_id = $self->{user_id}; - - # Prepended "set_id:" for set-level authentication. - my $show_user_id = $user_id; - $show_user_id =~ s/^set_id://; - # A proctor user may have the Proctor status which does not have the course_access behavior. # So don't check that here. However, the user must be able to login. - unless ($authz->hasPermissions($user_id, 'login')) { + unless ($self->{set_level_proctor} || $authz->hasPermissions($self->{user_id}, 'login')) { $self->{log_error} = 'user not permitted to login'; $self->{error} = $c->maketext(GENERIC_ERROR_MESSAGE); return 0; @@ -153,11 +145,11 @@ sub verify_normal_user { # As mentioned above the setID will not have the set version number if a new version is being opened. That # will be added to the proctor session key by the GatewayQuiz module later. if ($self->{login_type} eq 'proctor_grading') { - unless ($authz->hasPermissions($user_id, 'proctor_quiz_grade')) { + unless ($authz->hasPermissions($self->{user_id}, 'proctor_quiz_grade')) { $self->{log_error} = 'user not permitted to proctor test grade submissions'; $self->{error} = $c->maketext('User [_1] is not authorized to proctor test grade submissions in this course.', - $show_user_id); + $self->{user_id}); return 0; } $c->authen->session('proctor_authorization_granted' => $c->stash('setID')); @@ -166,32 +158,37 @@ sub verify_normal_user { # A UserSet is needed to determine if it is configured to skip grade proctor authorization. Require a # grade_proctor permission level to start a quiz that skips authorization to grade it. This ensures that # a grade proctor level of authorization is always required. - my $userSet = $db->getMergedSet($c->param('effectiveUser'), $c->stash('setID')); - unless ( - $authz->hasPermissions($user_id, 'proctor_quiz_grade') - || (($userSet->use_grade_auth_proctor eq 'Yes' || $userSet->restricted_login_proctor eq 'Yes') - && $authz->hasPermissions($user_id, 'proctor_quiz_login')) + my $userSet = $self->{userSet} // $db->getMergedSet($c->param('effectiveUser'), $c->stash('setID')); + + # Only accept the set level proctor password if set, and reject any other credentials. + if ($userSet->restricted_login_password ne '' && !$self->{set_level_proctor}) { + $self->{log_error} = 'invalid set password to start quiz.'; + $self->{error} = $c->maketext(GENERIC_ERROR_MESSAGE); + return 0; + } + + if ( + $userSet->restricted_login_password eq '' + && !$authz->hasPermissions($self->{user_id}, 'proctor_quiz_grade') + && ($userSet->use_grade_auth_proctor eq 'No' + || !$authz->hasPermissions($self->{user_id}, 'proctor_quiz_login')) + ) { - # Set the error based on if a single set password was required, a grade - # proctor was required to start, or a login proctor was required. - if ($userSet->restricted_login_proctor eq 'Yes') { - $self->{log_error} = 'invalid set password to start quiz.'; - $self->{error} = - $c->maketext('This quiz requires a set password to start, and the password was invalid.'); - } elsif ($userSet->use_grade_auth_proctor ne 'Yes') { + # Set the error based on if a grade proctor was required to start, or a login proctor was required. + if ($userSet->use_grade_auth_proctor eq 'No') { $self->{log_error} = 'grade proctor required to login and user is not permitted to proctor quiz grading.'; $self->{error} = $c->maketext( 'This quiz requires a grade proctor to start, and user [_1] is ' . 'not authorized to proctor test grade submissions in this course.', - $show_user_id + $self->{user_id} ); } else { $self->{log_error} = 'user not permitted to proctor quiz logins.'; $self->{error} = - $c->maketext("User [_1] is not authorized to proctor test logins in this course.", - $show_user_id); + $c->maketext('User [_1] is not authorized to proctor test logins in this course.', + $self->{user_id}); } return 0; } @@ -203,7 +200,7 @@ sub verify_normal_user { delete $c->authen->session->{'proctor_authorization_granted'}; delete $c->authen->session->{'proctor_authorization_version'}; if ($auth_result == 0) { - $self->{log_error} = "authentication failed"; + $self->{log_error} = 'authentication failed'; $self->{error} = $c->maketext(GENERIC_ERROR_MESSAGE); } return 0; @@ -211,6 +208,45 @@ sub verify_normal_user { } } +# This checks the password for the set level proctor if the set is configured for that. +# Otherwise it checks the password using the base class. +sub authenticate { + my $self = shift; + + return -1 unless defined $self->{password}; + + if ($self->{set_level_proctor}) { + my $c = $self->{c}; + + # First verify that this set is configured to use a set level proctor. + my $userSet = $c->db->getMergedSet($c->param('effectiveUser'), $c->stash('setID')); + if (!$userSet || $userSet->restricted_login_password eq '') { + $self->write_log_entry('SET LEVEL PROCTOR: set does not have a set level proctor password'); + return 0; + } + + debug('Checking set level proctor password.'); + my $setPassword = $userSet->restricted_login_password; + my $password = $self->{password} =~ s/^\s*|\s*$//gr; + + # Set level proctor passwords migrated from courses created before these passwords were stored in the set are + # crypt hashes. + if ($setPassword eq $password + || (isCryptedPassword($setPassword) && utf8Crypt($password, $setPassword) eq $setPassword)) + { + $self->write_log_entry('SET LEVEL PROCTOR: password accepted'); + $self->{userSet} = $userSet; + return 1; + } else { + $self->write_log_entry('SET LEVEL PROCTOR: password rejected'); + return 0; + } + } + + debug('Checking proctor password.'); + return $self->SUPER::authenticate; +} + # This is similar to the method in the base class, except that the parameters # proctor_user and proctor_passwd are used and there is no session_key. sub set_params { diff --git a/lib/WeBWorK/ContentGenerator/API/CourseActions.pm b/lib/WeBWorK/ContentGenerator/API/CourseActions.pm index 9d530f1744..5d81e2b812 100644 --- a/lib/WeBWorK/ContentGenerator/API/CourseActions.pm +++ b/lib/WeBWorK/ContentGenerator/API/CourseActions.pm @@ -81,7 +81,7 @@ sub listUsers ($c) { my $db = $c->db; my $ce = $c->ce; - my @userInfo = map { unbless($_) } $db->getUsersWhere({ user_id => { not_like => 'set_id:%' } }); + my @userInfo = map { unbless($_) } $db->getUsersWhere; my $numGlobalSets = $db->countGlobalSets; for my $user (@userInfo) { diff --git a/lib/WeBWorK/ContentGenerator/API/SetActions.pm b/lib/WeBWorK/ContentGenerator/API/SetActions.pm index 10008da41b..e3356b2f89 100644 --- a/lib/WeBWorK/ContentGenerator/API/SetActions.pm +++ b/lib/WeBWorK/ContentGenerator/API/SetActions.pm @@ -115,7 +115,7 @@ sub updateSetProperties ($c) { $set->time_limit_cap($c->req->param('time_limit_cap')); $set->restrict_ip($c->req->param('restrict_ip')); $set->relax_restrict_ip($c->req->param('relax_restrict_ip')); - $set->restricted_login_proctor($c->req->param('restricted_login_proctor')); + $set->restricted_login_password($c->req->param('restricted_login_password')); $db->putGlobalSet($set); diff --git a/lib/WeBWorK/ContentGenerator/AchievementsLeaderboard.pm b/lib/WeBWorK/ContentGenerator/AchievementsLeaderboard.pm index 9be5a882f5..adf9b29ce0 100644 --- a/lib/WeBWorK/ContentGenerator/AchievementsLeaderboard.pm +++ b/lib/WeBWorK/ContentGenerator/AchievementsLeaderboard.pm @@ -25,18 +25,17 @@ sub initialize ($c) { return unless $c->authz->hasPermissions($c->{userName}, 'view_leaderboard'); - # Get list of all users (except set-level proctors) and achievements. + # Get list of all users and achievements. my @achievements = sortAchievements($db->getAchievementsWhere); my %achievementsById = map { $_->achievement_id => $_ } @achievements; my %globalUserAchievements = - map { $_->user_id => $_ } $db->getGlobalUserAchievementsWhere({ user_id => { not_like => 'set_id:%' } }); + map { $_->user_id => $_ } $db->getGlobalUserAchievementsWhere; $c->{showUserNames} = $c->authz->hasPermissions($c->{userName}, 'view_leaderboard_usernames'); $c->{showLevels} = 0; # Hide level column unless at least one user has a level achievement. my %allUserAchievements; for ($db->getUserAchievementsWhere({ - user_id => { not_like => 'set_id:%' }, achievement_id => [ map { $_->achievement_id } grep { $_->category ne 'level' } @achievements ], })) { @@ -44,7 +43,7 @@ sub initialize ($c) { } my @rows; - for my $user ($db->getUsersWhere({ user_id => { not_like => 'set_id:%' } })) { + for my $user ($db->getUsersWhere) { # Only include users who can be shown in stats. next unless $ce->status_abbrev_has_behavior($user->status, 'include_in_stats'); diff --git a/lib/WeBWorK/ContentGenerator/GatewayQuiz.pm b/lib/WeBWorK/ContentGenerator/GatewayQuiz.pm index 6099a44e32..2647606547 100644 --- a/lib/WeBWorK/ContentGenerator/GatewayQuiz.pm +++ b/lib/WeBWorK/ContentGenerator/GatewayQuiz.pm @@ -1334,10 +1334,8 @@ sub nav ($c, $args) { my %users = map { $_->[0] => 1 } @userVersions; my %allUserRecords = - map { $_->user_id => $_ } - grep { $users{ $_->user_id } } - $c->db->getUsersWhere( - { -and => { user_id => { not_like => 'set_id:%' } }, user_id => { '!=' => $userID } }); + map { $_->user_id => $_ } + grep { $users{ $_->user_id } } $c->db->getUsersWhere({ user_id => { '!=' => $userID } }); my $filter = $c->param('studentNavFilter'); diff --git a/lib/WeBWorK/ContentGenerator/Hardcopy.pm b/lib/WeBWorK/ContentGenerator/Hardcopy.pm index 2148619092..e499d008d6 100644 --- a/lib/WeBWorK/ContentGenerator/Hardcopy.pm +++ b/lib/WeBWorK/ContentGenerator/Hardcopy.pm @@ -356,7 +356,7 @@ sub display_form ($c) { if ($perm_multiuser && $perm_multiset) { # Get all users for selection. - @users = $db->getUsersWhere({ user_id => { not_like => 'set_id:%' } }); + @users = $db->getUsersWhere; # Get sets for selection. # Note that we are getting GlobalSets instead of using the list of UserSets assigned to the diff --git a/lib/WeBWorK/ContentGenerator/Instructor/AchievementList.pm b/lib/WeBWorK/ContentGenerator/Instructor/AchievementList.pm index 406b4ce4b4..dbbe54bb47 100644 --- a/lib/WeBWorK/ContentGenerator/Instructor/AchievementList.pm +++ b/lib/WeBWorK/ContentGenerator/Instructor/AchievementList.pm @@ -236,7 +236,7 @@ sub score_handler ($c) { # First get everything that is needed from the database. my @achievements = sortAchievements($db->getAchievements(@achievementsToScore)); - my @users = $db->getUsersWhere({ user_id => { not_like => 'set_id:%' } }, [qw(section last_name)]); + my @users = $db->getUsersWhere({}, [qw(section last_name)]); my %globalUserAchievements = map { $_->user_id => $_ } $db->getGlobalUserAchievementsWhere; diff --git a/lib/WeBWorK/ContentGenerator/Instructor/AchievementUserEditor.pm b/lib/WeBWorK/ContentGenerator/Instructor/AchievementUserEditor.pm index fa407fa750..099bf0ea74 100644 --- a/lib/WeBWorK/ContentGenerator/Instructor/AchievementUserEditor.pm +++ b/lib/WeBWorK/ContentGenerator/Instructor/AchievementUserEditor.pm @@ -22,7 +22,7 @@ sub initialize ($c) { return unless $authz->hasPermissions($user, 'edit_achievements'); $c->stash->{userRecords} = - [ $db->getUsersWhere({ user_id => { not_like => 'set_id:%' } }, [qw/section last_name first_name/]) ]; + [ $db->getUsersWhere({}, [qw/section last_name first_name/]) ]; $c->stash->{userAchievementRecords} = { map { $_->user_id => $_ } $db->getUserAchievementsWhere({ achievement_id => $achievementID }) }; diff --git a/lib/WeBWorK/ContentGenerator/Instructor/Assigner.pm b/lib/WeBWorK/ContentGenerator/Instructor/Assigner.pm index 699e9d9214..1139bedfba 100644 --- a/lib/WeBWorK/ContentGenerator/Instructor/Assigner.pm +++ b/lib/WeBWorK/ContentGenerator/Instructor/Assigner.pm @@ -23,11 +23,10 @@ sub initialize ($c) { unless $authz->hasPermissions($user, 'access_instructor_tools') && $authz->hasPermissions($user, 'assign_problem_sets'); - # Get all users except the set level proctors, and restrict to the sections or recitations that are allowed for the + # Get all users, and restrict to the sections or recitations that are allowed for the # user if such restrictions are defined. $c->stash->{users} = [ $db->getUsersWhere({ - user_id => { not_like => 'set_id:%' }, $ce->{viewable_sections}{$user} || $ce->{viewable_recitations}{$user} ? ( -or => [ diff --git a/lib/WeBWorK/ContentGenerator/Instructor/Index.pm b/lib/WeBWorK/ContentGenerator/Instructor/Index.pm index 5492dc0b26..fa7b2f95f9 100644 --- a/lib/WeBWorK/ContentGenerator/Instructor/Index.pm +++ b/lib/WeBWorK/ContentGenerator/Instructor/Index.pm @@ -162,12 +162,11 @@ sub initialize ($c) { return; } - # Get all users except the set level proctors, and restrict to the sections or recitations that are allowed for the - # user if such restrictions are defined. This list is sorted by last_name, then first_name, then user_id. + # Get all users, and restrict to the sections or recitations that are allowed for the user if such restrictions are + # defined. This list is sorted by last_name, then first_name, then user_id. $c->stash->{users} = [ $db->getUsersWhere( { - user_id => { not_like => 'set_id:%' }, $ce->{viewable_sections}{$userID} || $ce->{viewable_recitations}{$userID} ? ( -or => [ diff --git a/lib/WeBWorK/ContentGenerator/Instructor/ProblemSetDetail.pm b/lib/WeBWorK/ContentGenerator/Instructor/ProblemSetDetail.pm index 4d859c4a8f..4880a77d7e 100644 --- a/lib/WeBWorK/ContentGenerator/Instructor/ProblemSetDetail.pm +++ b/lib/WeBWorK/ContentGenerator/Instructor/ProblemSetDetail.pm @@ -10,7 +10,7 @@ specific user/set information as well as problem information use Exporter qw(import); -use WeBWorK::Utils qw(cryptPassword x); +use WeBWorK::Utils qw(x isCryptedPassword); use WeBWorK::Utils::Files qw(surePathToFile readFile); use WeBWorK::Utils::Instructor qw(assignProblemToAllSetUsers addProblemToSet); use WeBWorK::Utils::JITAR qw(seq_to_jitar_id jitar_id_to_seq); @@ -23,7 +23,8 @@ our @EXPORT_OK = qw(FIELD_PROPERTIES); use constant SET_FIELDS => [ qw(set_header hardcopy_header open_date reduced_scoring_date due_date answer_date visible description enable_reduced_scoring restricted_release restricted_status restrict_ip relax_restrict_ip - assignment_type use_grade_auth_proctor attempts_per_version version_time_limit time_limit_cap + assignment_type use_grade_auth_proctor restricted_login_password attempts_per_version version_time_limit + time_limit_cap versions_per_interval time_interval problem_randorder problems_per_page hide_score:hide_score_by_problem hide_work hide_hint restrict_prob_progression email_instructor) ]; @@ -435,6 +436,20 @@ use constant FIELD_PROPERTIES => { . 'authorization or a password specific to this set to start and no authorization to grade.' ), }, + restricted_login_password => { + name => x('Password'), + type => 'edit', + size => 10, + override => 'any', + help_text => x( + 'Provide a password to have a password students must enter to start a proctored test. Alternatively, ' + . 'leave this blank if you would like to have a proctor level user enter their username and ' + . q{password on the student's screen for authentication. When editing the set for specific } + . 'users, a different password can be set for those users. Note that if the password is changed ' + . 'after students have started the test, then the new password will be needed if a student needs ' + . 'to be authorized again to continue the test.' + ), + }, restrict_prob_progression => { name => x('Restrict Problem Progression'), type => 'choose', @@ -467,11 +482,8 @@ use constant FIELD_PROPERTIES => { ), }, - # In addition to the set fields above, there are a number of things - # that are set but aren't in this table: - # any set proctor information (which is in the user tables), and - # any set location restriction information (which is in the - # location tables) + # In addition to the set fields above, any set location restriction information is set but isn't in this table + # (it is in the location tables). # Problem information source_file => { @@ -637,7 +649,6 @@ sub fieldTable ($c, $userID, $setID, $problemID, $globalRecord, $userRecord = un my $ipFields = ''; my $numLocations = 0; - # Needed for set-level proctor my $proctorFields = ''; my @fieldOrder; @@ -861,6 +872,13 @@ sub fieldHTML ($c, $userID, $setID, $problemID, $globalRecord, $userRecord, $fie $globalValue = $properties{default}; } + # A password migrated from an older course is a hash that should not be shown, so show ******** instead. That is + # submitted back if left unchanged, and the hashed password is then kept. + if ($field eq 'restricted_login_password') { + $globalValue = '********' if isCryptedPassword($globalValue); + $userValue = '********' if isCryptedPassword($userValue); + } + if ($properties{convertby}) { $globalValue = $globalValue / $properties{convertby} if $globalValue; $userValue = $userValue / $properties{convertby} if $userValue; @@ -1188,10 +1206,14 @@ sub extraSetFields ($c, $userID, $setID, $globalRecord, $userRecord, $forUsers) $c->fieldHTML($userID, $setID, undef, $globalRecord, $userRecord, 'use_grade_auth_proctor') )->join('') ), - $forUsers ? '' : $c->include( - 'ContentGenerator/Instructor/ProblemSetDetail/restricted_login_proctor_password_row', - globalRecord => $globalRecord, - proctorClass => $proctorClass + # The password is never read from a set version, so don't show it when editing set versions. + $forUsers && $userRecord->version_id ? '' : $c->tag( + 'tr', + class => $proctorClass, + $c->c( + map { $c->tag('td', $_) } + $c->fieldHTML($userID, $setID, undef, $globalRecord, $userRecord, 'restricted_login_password') + )->join('') ) )->join(''); } @@ -1481,10 +1503,6 @@ sub initialize ($c) { # Save general set information (including headers) if ($forUsers) { - # Note that we don't deal with the proctor user fields here, with the assumption that it can't be possible - # to change them for users. - # FIXME: This is not the most robust treatment of the problem - my @userRecords = $db->getUserSetsWhere({ user_id => [@editForUser], set_id => $setID }); # If editing a set version, we want to edit that instead of the userset, so get it too. my $userSet = $userRecords[0]; @@ -1503,7 +1521,14 @@ sub initialize ($c) { $field eq 'restricted_release' ? join(',', grep {$_} @paramValues) : (@paramValues > 1 && $paramValues[0] eq 'numeric') ? $paramValues[1] : $paramValues[0]; - if ($param && $param ne '') { + + if ($field eq 'restricted_login_password' && defined $param) { + $param =~ s/^\s*|\s*$//g; + # ******** is shown in place of a hashed password, and in that case it is left unchanged. + next if $param eq '********'; + } + + if (defined $param && $param ne '') { $param = $param * $properties{$field}->{convertby} if $properties{$field}{convertby}; # Special case: Does field fill in multiple values? @@ -1589,6 +1614,12 @@ sub initialize ($c) { $param = $param * $properties{$field}->{convertby}; } + if ($field eq 'restricted_login_password') { + $param =~ s/^\s*|\s*$//g; + # ******** is shown in place of a hashed password, and in that case it is left unchanged. + next if $param eq '********'; + } + if ($field =~ /:/) { my @values = split(/:/, $param); my @fields = split(/:/, $field); @@ -1613,6 +1644,8 @@ sub initialize ($c) { )); } + $setRecord->restricted_login_password('') if $setRecord->assignment_type ne 'proctored_gateway'; + $db->putGlobalSet($setRecord); if ($ce->{LTIVersion} @@ -1652,71 +1685,6 @@ sub initialize ($c) { $db->deleteGlobalSetLocation($setID, $_); } } - - # Save proctored problem proctor user information - if ($c->param("set.$setID.restricted_login_proctor_password") - && $setRecord->assignment_type eq 'proctored_gateway') - { - # In this case add a set-level proctor or update the password. - my $procID = "set_id:$setID"; - my $pass = $c->param("set.$setID.restricted_login_proctor_password"); - # Should we carefully check in this case that the user and password exist? The code in the add stanza - # is pretty careful to be sure that there's a one-to-one correspondence between the existence of the - # user and the setting of the set restricted_login_proctor field, so we assume that just checking the - # latter here is sufficient. - if ($setRecord->restricted_login_proctor eq 'Yes') { - if ($pass ne '********') { - # A new password was submitted. So save it. - my $dbPass = eval { $db->getPassword($procID) }; - if ($@) { - $c->addbadmessage($c->maketext( - 'Error getting old set-proctor password from the database: [_1]. ' - . 'No update to the password was done.', - $@ - )); - } else { - $dbPass->password(cryptPassword($pass)); - $db->putPassword($dbPass); - } - } - } else { - $setRecord->restricted_login_proctor('Yes'); - my $procUser = $db->newUser(); - $procUser->user_id($procID); - $procUser->last_name("Proctor"); - $procUser->first_name("Login"); - $procUser->student_id("loginproctor"); - $procUser->status($ce->status_name_to_abbrevs('Proctor')); - my $procPerm = $db->newPermissionLevel; - $procPerm->user_id($procID); - $procPerm->permission($ce->{userRoles}{login_proctor}); - my $procPass = $db->newPassword; - $procPass->user_id($procID); - $procPass->password(cryptPassword($pass)); - - eval { $db->addUser($procUser) }; - if ($@) { - $c->addbadmessage($c->maketext("Error adding set-level proctor: [_1]", $@)); - } else { - $db->addPermissionLevel($procPerm); - $db->addPassword($procPass); - } - - # Set the restricted_login_proctor set field - $db->putGlobalSet($setRecord); - } - - } else { - # If the parameter isn't set, or if the assignment type is not 'proctored_gateway', then ensure that - # there is no set-level proctor defined. - if ($setRecord->restricted_login_proctor eq 'Yes') { - - $setRecord->restricted_login_proctor('No'); - $db->deleteUser("set_id:$setID"); - $db->putGlobalSet($setRecord); - - } - } } # Save problem information diff --git a/lib/WeBWorK/ContentGenerator/Instructor/ProblemSetList.pm b/lib/WeBWorK/ContentGenerator/Instructor/ProblemSetList.pm index b4dc785399..6b87aff5b0 100644 --- a/lib/WeBWorK/ContentGenerator/Instructor/ProblemSetList.pm +++ b/lib/WeBWorK/ContentGenerator/Instructor/ProblemSetList.pm @@ -469,25 +469,11 @@ sub create_handler ($c) { $db->addGlobalProblem($_); } - # also copy any set_location restrictions and set-level proctor - # information + # Also copy any set_location restrictions. foreach ($db->getAllGlobalSetLocations($oldSetID)) { $_->set_id($newSetID); $db->addGlobalSetLocation($_); } - if ($newSetRecord->restricted_login_proctor eq 'Yes') { - my $procUser = $db->getUser("set_id:$oldSetID"); - $procUser->user_id("set_id:$newSetID"); - eval { $db->addUser($procUser) }; - if (!$@) { - my $procPerm = $db->getPermissionLevel("set_id:$oldSetID"); - $procPerm->user_id("set_id:$newSetID"); - $db->addPermissionLevel($procPerm); - my $procPass = $db->getPassword("set_id:$oldSetID"); - $procPass->user_id("set_id:$newSetID"); - $db->addPassword($procPass); - } - } } # Assign set to current active user. diff --git a/lib/WeBWorK/ContentGenerator/Instructor/Scoring.pm b/lib/WeBWorK/ContentGenerator/Instructor/Scoring.pm index 5db0c635fa..7c92e9f467 100644 --- a/lib/WeBWorK/ContentGenerator/Instructor/Scoring.pm +++ b/lib/WeBWorK/ContentGenerator/Instructor/Scoring.pm @@ -51,7 +51,7 @@ sub initialize ($c) { # Get all users sorted by last_name, then first_name, then user_id. debug("pre-fetching users"); - my @Users = $db->getUsersWhere({ user_id => { not_like => 'set_id:%' } }, qw/last_name first_name user_id/); + my @Users = $db->getUsersWhere({}, qw/last_name first_name user_id/); my %Users; my @sortedUserIDs; for my $User (@Users) { diff --git a/lib/WeBWorK/ContentGenerator/Instructor/SendMail.pm b/lib/WeBWorK/ContentGenerator/Instructor/SendMail.pm index 81250c9e62..db75e321ac 100644 --- a/lib/WeBWorK/ContentGenerator/Instructor/SendMail.pm +++ b/lib/WeBWorK/ContentGenerator/Instructor/SendMail.pm @@ -81,11 +81,11 @@ sub initialize ($c) { $c->{preview_user} = $classList[0] && $classList[0] ne $userID ? $db->getUser($classList[0]) : $user; # Gather database data - # Get all users except set level proctors and practice users. If the current user has restrictions on viewable - # sections or recitations, those are filtered out as well. The users are sorted by user_id. + # Get all users except practice users. If the current user has restrictions on viewable sections or recitations, + # those are filtered out as well. The users are sorted by user_id. my @Users = $db->getUsersWhere( { - user_id => [ -and => { not_like => 'set_id:%' }, { not_like => "$ce->{practiceUserPrefix}\%" } ], + user_id => { not_like => "$ce->{practiceUserPrefix}\%" }, $ce->{viewable_sections}{$userID} || $ce->{viewable_recitations}{$userID} ? ( -or => [ diff --git a/lib/WeBWorK/ContentGenerator/Instructor/ShowAnswers.pm b/lib/WeBWorK/ContentGenerator/Instructor/ShowAnswers.pm index 53c780ff01..5f7c0e1342 100644 --- a/lib/WeBWorK/ContentGenerator/Instructor/ShowAnswers.pm +++ b/lib/WeBWorK/ContentGenerator/Instructor/ShowAnswers.pm @@ -233,10 +233,9 @@ sub getInstructorData ($c) { my $ce = $c->ce; my $user = $c->authen->{user_id}; - # Get all users except the set level proctors, and restrict to the sections or recitations that are allowed for + # Get all users, and restrict to the sections or recitations that are allowed for # the user if such restrictions are defined. my @users = $db->getUsersWhere({ - user_id => { not_like => 'set_id:%' }, $ce->{viewable_sections}{$user} || $ce->{viewable_recitations}{$user} ? ( -or => [ diff --git a/lib/WeBWorK/ContentGenerator/Instructor/Stats.pm b/lib/WeBWorK/ContentGenerator/Instructor/Stats.pm index 9f2aa9679b..749fcc4278 100644 --- a/lib/WeBWorK/ContentGenerator/Instructor/Stats.pm +++ b/lib/WeBWorK/ContentGenerator/Instructor/Stats.pm @@ -35,15 +35,14 @@ sub initialize ($c) { $c->{problemRecord} = $problemRecord; } - # Cache a list of all users except set level proctors and practice users, and restrict to the sections - # or recitations that are allowed for the user if such restrictions are defined. This list is sorted by - # last_name, then first_name, then user_id. This is used in multiple places in this module, and is used - # on every page except the main page, so it is done here to prevent extra database access. + # Cache a list of all users except practice users, and restrict to the sections or recitations that are allowed + # for the user if such restrictions are defined. This list is sorted by last_name, then first_name, then + # user_id. This is used in multiple places in this module, and is used on every page except the main page, so + # it is done here to prevent extra database access. $c->{student_records} = [ $db->getUsersWhere( { - user_id => - [ -and => { not_like => 'set_id:%' }, { not_like => "$ce->{practiceUserPrefix}\%" } ], + user_id => { not_like => "$ce->{practiceUserPrefix}\%" }, $ce->{viewable_sections}{$user} || $ce->{viewable_recitations}{$user} ? ( -or => [ diff --git a/lib/WeBWorK/ContentGenerator/Instructor/StudentProgress.pm b/lib/WeBWorK/ContentGenerator/Instructor/StudentProgress.pm index 4e6c346d60..38ec0c8afc 100644 --- a/lib/WeBWorK/ContentGenerator/Instructor/StudentProgress.pm +++ b/lib/WeBWorK/ContentGenerator/Instructor/StudentProgress.pm @@ -21,14 +21,14 @@ sub initialize ($c) { # Check permissions return unless $c->authz->hasPermissions($user, "access_instructor_tools"); - # Cache a list of all users except set level proctors and practice users, and restrict to the sections or - # recitations that are allowed for the user if such restrictions are defined. This list is sorted by last_name, - # then first_name, then user_id. This is used in multiple places in this module, and is guaranteed to be used at - # least once. So it is done here to prevent extra database access. + # Cache a list of all users except practice users, and restrict to the sections or recitations that are allowed for + # the user if such restrictions are defined. This list is sorted by last_name, then first_name, then user_id. This + # is used in multiple places in this module, and is guaranteed to be used at least once. So it is done here to + # prevent extra database access. $c->{student_records} = [ $db->getUsersWhere( { - user_id => [ -and => { not_like => 'set_id:%' }, { not_like => "$ce->{practiceUserPrefix}\%" } ], + user_id => { not_like => "$ce->{practiceUserPrefix}\%" }, $ce->{viewable_sections}{$user} || $ce->{viewable_recitations}{$user} ? ( -or => [ diff --git a/lib/WeBWorK/ContentGenerator/Instructor/UserList.pm b/lib/WeBWorK/ContentGenerator/Instructor/UserList.pm index 154a32dab5..9f14fa0df0 100644 --- a/lib/WeBWorK/ContentGenerator/Instructor/UserList.pm +++ b/lib/WeBWorK/ContentGenerator/Instructor/UserList.pm @@ -177,14 +177,12 @@ async sub initialize ($c) { return; } - # Get a list of all users except set-level proctors from the database. - my @allUsersDB = $db->getUsersWhere({ user_id => { not_like => 'set_id:%' } }); + # Get a list of all users from the database. + my @allUsersDB = $db->getUsersWhere; - my %permissionLevels = - map { $_->user_id => $_->permission } $db->getPermissionLevelsWhere({ user_id => { not_like => 'set_id:%' } }); + my %permissionLevels = map { $_->user_id => $_->permission } $db->getPermissionLevelsWhere; - my %passwordRecords = - map { $_->user_id => $_ } $db->getPasswordsWhere({ user_id => { not_like => 'set_id:%' } }); + my %passwordRecords = map { $_->user_id => $_ } $db->getPasswordsWhere; # Add permission level, a password exists field, and a must reset password field to the user record hash. for my $user (@allUsersDB) { @@ -733,11 +731,11 @@ sub importUsersFromCSV ($c, $fileName, $replaceExisting, $fallbackPasswordSource my %record = %$record; my $user_id = $record{user_id}; - unless (WeBWorK::DB::check_user_id($user_id)) { # try to catch lines with bad characters + unless (eval { WeBWorK::DB::check_user_id($user_id) }) { # try to catch lines with bad characters push @skipped, $user_id; next; } - if ($user_id eq $c->authen->{user_id}) { # don't replace yourself!! + if ($user_id eq $c->authen->{user_id}) { # don't replace yourself!! push @skipped, $user_id; next; } diff --git a/lib/WeBWorK/ContentGenerator/Instructor/UsersAssignedToSet.pm b/lib/WeBWorK/ContentGenerator/Instructor/UsersAssignedToSet.pm index 60592b6076..003fb93c17 100644 --- a/lib/WeBWorK/ContentGenerator/Instructor/UsersAssignedToSet.pm +++ b/lib/WeBWorK/ContentGenerator/Instructor/UsersAssignedToSet.pm @@ -36,8 +36,7 @@ sub initialize ($c) { return unless $c->stash->{set_record}; # Get all user records and cache them for later use. - my @userRecords = - $db->getUsersWhere({ user_id => { not_like => 'set_id:%' } }, [qw/section last_name first_name/]); + my @userRecords = $db->getUsersWhere({}, [qw/section last_name first_name/]); my %currentUsers = map { $ce->status_abbrev_has_behavior($_->status, 'include_in_assignment') ? ($_->user_id => 1) : () } @userRecords; diff --git a/lib/WeBWorK/DB.pm b/lib/WeBWorK/DB.pm index 07be455adc..d3bc7527c0 100644 --- a/lib/WeBWorK/DB.pm +++ b/lib/WeBWorK/DB.pm @@ -468,13 +468,12 @@ sub newUser { sub countUsers { return scalar shift->listUsers(@_) } -# Note: This returns a list of user_ids for all users except set level proctors. sub listUsers { my ($self) = shift->checkArgs(\@_); if (wantarray) { - return map {@$_} $self->{user}->get_fields_where(['user_id'], { user_id => { not_like => 'set_id:%' } }); + return map {@$_} $self->{user}->get_fields_where(['user_id']); } else { - return $self->{user}->count_where({ user_id => { not_like => 'set_id:%' } }); + return $self->{user}->count_where; } } @@ -2141,9 +2140,10 @@ sub getAllMergedProblemVersions { # utilities ################################################################################ -sub check_user_id { # (valid characters are [-a-zA-Z0-9_.,@]) +# Valid characters for the user_id column are [-a-zA-Z0-9_.,@]. +sub check_user_id { my $value = shift; - if ($value =~ m/^[-a-zA-Z0-9_.@]*,?(set_id:)?[-a-zA-Z0-9_.@]*(,g)?$/) { + if ($value =~ m/^[-a-zA-Z0-9_.,@]*$/) { return 1; } else { croak "invalid characters in user_id field: '$value' (valid characters are [-a-zA-Z0-9_.,@])"; @@ -2179,7 +2179,7 @@ sub validateKeyfieldValue { . "' (valid characters are [0-9])" unless $value =~ m/^[0-9]*$/; } elsif ($keyfield eq "user_id" || $keyfield eq 'userID') { - check_user_id($value); # (valid characters are [-a-zA-Z0-9_.,]) see above. + check_user_id($value); } elsif ($keyfield eq "ip_mask") { croak "invalid characters in '$keyfield' field: '$value' (valid characters are [-a-zA-Z0-9_.,])" unless $value =~ m/^[-a-fA-F0-9_.:\/]*$/; diff --git a/lib/WeBWorK/DB/Record/Set.pm b/lib/WeBWorK/DB/Record/Set.pm index 741dd58393..fa1fd324a0 100644 --- a/lib/WeBWorK/DB/Record/Set.pm +++ b/lib/WeBWorK/DB/Record/Set.pm @@ -39,7 +39,7 @@ BEGIN { time_limit_cap => { type => "ENUM('0','1')" }, restrict_ip => { type => "ENUM('No','RestrictTo','DenyFrom') DEFAULT 'No'" }, relax_restrict_ip => { type => "ENUM('No','AfterAnswerDate','AfterVersionAnswerDate') DEFAULT 'No'" }, - restricted_login_proctor => { type => "ENUM('No','Yes')" }, + restricted_login_password => { type => "TEXT" }, use_grade_auth_proctor => { type => "ENUM('No','Yes') DEFAULT 'Yes'" }, hide_hint => { type => "INT" }, restrict_prob_progression => { type => "INT" }, diff --git a/lib/WeBWorK/DB/Record/UserSet.pm b/lib/WeBWorK/DB/Record/UserSet.pm index d334358c42..0a2f15ea55 100644 --- a/lib/WeBWorK/DB/Record/UserSet.pm +++ b/lib/WeBWorK/DB/Record/UserSet.pm @@ -42,7 +42,7 @@ BEGIN { time_limit_cap => { type => "ENUM('0','1')" }, restrict_ip => { type => "ENUM('No','RestrictTo','DenyFrom')" }, relax_restrict_ip => { type => "ENUM('No','AfterAnswerDate','AfterVersionAnswerDate')" }, - restricted_login_proctor => { type => "ENUM('No','Yes')" }, + restricted_login_password => { type => "TEXT" }, use_grade_auth_proctor => { type => "ENUM('No','Yes')" }, hide_hint => { type => "INT" }, restrict_prob_progression => { type => "INT" }, diff --git a/lib/WeBWorK/HTML/ScrollingRecordList.pm b/lib/WeBWorK/HTML/ScrollingRecordList.pm index 2c766d8cdf..c4d758837a 100644 --- a/lib/WeBWorK/HTML/ScrollingRecordList.pm +++ b/lib/WeBWorK/HTML/ScrollingRecordList.pm @@ -92,8 +92,7 @@ sub scrollingRecordList ($options, @records) { sub userData ($c, @users) { my %permissionName = reverse %{ $c->ce->{userRoles} }; - my %permissions = map { $_->user_id => $permissionName{ $_->permission } } - $c->db->getPermissionLevelsWhere({ user_id => { not_like => 'set_id:%' } }); + my %permissions = map { $_->user_id => $permissionName{ $_->permission } } $c->db->getPermissionLevelsWhere; my @records = map { { value => $_->user_id, diff --git a/lib/WeBWorK/HTML/StudentNav.pm b/lib/WeBWorK/HTML/StudentNav.pm index be8caf88af..ba1b6affb1 100644 --- a/lib/WeBWorK/HTML/StudentNav.pm +++ b/lib/WeBWorK/HTML/StudentNav.pm @@ -15,9 +15,7 @@ sub studentNav ($c, $setID) { return '' unless $c->authz->hasPermissions($userID, 'become_student'); # Find all users (except the current user) sorted by last_name, then first_name, then user_id. - my @allUserRecords = - $c->db->getUsersWhere({ -and => { user_id => { not_like => 'set_id:%' } }, user_id => { '!=' => $userID } }, - [qw/last_name first_name user_id/]); + my @allUserRecords = $c->db->getUsersWhere({ user_id => { '!=' => $userID } }, [qw/last_name first_name user_id/]); # If $setID is provided, only show users assigned to given set. if ($setID) { diff --git a/lib/WeBWorK/Utils.pm b/lib/WeBWorK/Utils.pm index ea8e583483..22e2e9b32b 100644 --- a/lib/WeBWorK/Utils.pm +++ b/lib/WeBWorK/Utils.pm @@ -22,6 +22,7 @@ our @EXPORT_OK = qw( wwRound cryptPassword utf8Crypt + isCryptedPassword undefstr sortByName sortAchievements @@ -156,6 +157,10 @@ sub utf8Crypt ($clearPassword, $hash) { return $cryptedPassword; } +sub isCryptedPassword ($password) { + return ($password // '') =~ m{^\$[0-9a-z]+\$} ? 1 : 0; +} + sub undefstr ($default, @values) { return map { defined $_ ? $_ : $default } @values[ 0 .. $#values ]; } @@ -707,6 +712,13 @@ called on the UTF-8 encoded version of C<$clearPassword>. Note that C<$hash> can be a salt or a password hash generated by a previous call of this method with a salt.. +=head2 isCryptedPassword + +Usage: C + +Returns 1 if C<$password> looks like a password hash returned by C in the +modular crypt format "$id$salt$hash", and 0 otherwise. + =head2 undefstr Usage: C diff --git a/lib/WeBWorK/Utils/CourseDBIntegrityCheck.pm b/lib/WeBWorK/Utils/CourseDBIntegrityCheck.pm index 4689c89771..1d09997a88 100644 --- a/lib/WeBWorK/Utils/CourseDBIntegrityCheck.pm +++ b/lib/WeBWorK/Utils/CourseDBIntegrityCheck.pm @@ -258,12 +258,13 @@ sub updateTableFields { my $schema_obj = $db->{$table}; - my %newKeyFields; + my (%newFields, %newKeyFields); # Add fields for my $field_name (keys %$fieldStatus) { if ($fieldStatus->{$field_name}[0] == ONLY_IN_A) { if ($schema_obj->can('add_column_field') && $schema_obj->add_column_field($field_name)) { + $newFields{$field_name} = 1; if ($fieldStatus->{$field_name}[1]) { push(@messages, [ "Added primary key column '$field_name' to table '$table'", 1 ]); $newKeyFields{$field_name} = 1; @@ -286,6 +287,13 @@ sub updateTableFields { } } + # This must be done before fields are dropped since it uses the old restricted_login_proctor field. + push(@messages, $self->migrateSetLevelProctorPasswords) + if $table eq 'set' + && $newFields{restricted_login_password} + && $fieldStatus->{restricted_login_proctor} + && $fieldStatus->{restricted_login_proctor}[0] == ONLY_IN_B; + # Drop fields if listed in $delete_field_names. for my $field_name (@$delete_field_names) { if ($fieldStatus->{$field_name} && $fieldStatus->{$field_name}[0] == ONLY_IN_B) { @@ -357,6 +365,60 @@ sub migrateSetIdToSetIdVersionId { return @results; } +# Set level proctor passwords used to be the password of a "set_id:" user, and were used for sets with the +# restricted_login_proctor field set to 'Yes'. Copy the password of that user into the restricted_login_password field +# of the set. Then delete all of these users. +sub migrateSetLevelProctorPasswords { + my $self = shift; + my $db = $self->db; + my $record = $db->{set}; + + my @messages; + + my %passwords = map { substr($_->[0], 7) => $_->[1] } + $db->{password}->get_fields_where([qw(user_id password)], { user_id => { like => 'set_id:%' } }); + + my @fields = $record->fields; + my %values; + @values{@fields} = (0 .. $#fields); + + my @keyfields = $record->keyfields; + my %where; + @where{@keyfields} = map { $values{$_} } @keyfields; + + my ($stmt, @val_order) = $record->sql->update($record->table, \%values); + (substr($stmt, length($stmt), 0), my @where_order) = $record->sql->where(\%where); + + my $rows_i = $record->get_fields_where_i([@fields], { restricted_login_proctor => 'Yes' }); + my $sth = $record->dbh->prepare_cached($stmt, undef, 3); + + until ($rows_i->is_exhausted) { + my $fieldValues = $rows_i->value; + + my $setID = $fieldValues->[ $values{set_id} ]; + my $password = $passwords{$setID}; + next unless defined $password && $password =~ /\S/; + + my @bind_vals = @$fieldValues[@where_order]; + $fieldValues->[ $values{restricted_login_password} ] = $password; + unshift(@bind_vals, @$fieldValues[@val_order]); + + $record->dbh->debug_stmt($sth, @bind_vals); + $sth->execute(@bind_vals); + push(@messages, [ "Migrated the set level proctor password for set '$setID'", 1 ]); + } + $sth->finish; + + my $deleted = 0; + for my $table (qw(user password permission)) { + my $result = $db->{$table}->delete_where({ user_id => { like => 'set_id:%' } }); + $deleted = $result if $table eq 'user'; + } + push(@messages, [ 'Deleted ' . ($deleted + 0) . ' obsolete set level proctor users', 1 ]) if $deleted > 0; + + return @messages; +} + # Database locking utilities # Create a lock named 'webwork.dbugrade' that times out after 10 seconds. diff --git a/lib/WeBWorK/Utils/CourseManagement.pm b/lib/WeBWorK/Utils/CourseManagement.pm index 75b5850149..7d6c568ce4 100644 --- a/lib/WeBWorK/Utils/CourseManagement.pm +++ b/lib/WeBWorK/Utils/CourseManagement.pm @@ -353,8 +353,7 @@ sub addCourse { # as the role might be customized in that file. my @non_student_ids = map {@$_} ($db0->listPermissionLevelsWhere({ - permission => { '!=' => $options{copyConfig} ? $ce0->{userRoles}{student} : $ce->{userRoles}{student} }, - user_id => { not_like => 'set_id:%' } + permission => { '!=' => $options{copyConfig} ? $ce0->{userRoles}{student} : $ce->{userRoles}{student} } })); for my $user_id (@non_student_ids) { @@ -395,20 +394,6 @@ sub addCourse { eval { $db->addGlobalSetLocation($location) }; warn $@ if $@; } - - # Copy the set level proctor user for this set if there is one (despite the for loop there can only be one). - for my $setProctor ($db0->getUsersWhere({ user_id => 'set_id:' . $set->set_id })) { - eval { $db->addUser($setProctor) }; - warn $@ if $@; - - my $password = $db0->getPassword($setProctor->user_id); - eval { $db->addPassword($password) } if $password; - warn $@ if $@; - - my $permission = $db0->getPermissionLevel($setProctor->user_id); - eval { $db->addPermissionLevel($permission) } if $permission; - warn $@ if $@; - } } if ($options{copyNonStudents}) { foreach my $userTriple (@users) { diff --git a/lib/WeBWorK/Utils/Instructor.pm b/lib/WeBWorK/Utils/Instructor.pm index de6c3f03b7..e425480a74 100644 --- a/lib/WeBWorK/Utils/Instructor.pm +++ b/lib/WeBWorK/Utils/Instructor.pm @@ -259,7 +259,7 @@ sub assignSetToAllUsers { my ($db, $ce, $setID) = @_; debug("$setID: getting user list"); - my @userRecords = $db->getUsersWhere({ user_id => { not_like => 'set_id:%' } }); + my @userRecords = $db->getUsersWhere; debug("$setID: (done with that)"); return assignSetToGivenUsers($db, $ce, $setID, 0, @userRecords); diff --git a/templates/ContentGenerator/Instructor/ProblemSetDetail/restricted_login_proctor_password_row.html.ep b/templates/ContentGenerator/Instructor/ProblemSetDetail/restricted_login_proctor_password_row.html.ep deleted file mode 100644 index 45611dd0f8..0000000000 --- a/templates/ContentGenerator/Instructor/ProblemSetDetail/restricted_login_proctor_password_row.html.ep +++ /dev/null @@ -1,36 +0,0 @@ -% # This assumes that the login proctor password is something that can only be changed for the global set. -% -% # If the set doesn't require a login proctor, then we can assume that one doesn't exist. -% # Otherwise, we need to check the database to find if there's an already defined password. -% # In that case set the form parameter so it is picked up by the tag helper below. -% if ($globalRecord->restricted_login_proctor eq 'Yes' && $db->existsPassword("set_id:$setID")) { - % param("set.$setID.restricted_login_proctor_password", '********'); -% } -% - - - <%= label_for "set.$setID.restricted_login_proctor_password" => maketext('Password') =%> - - - - - <%= maketext('Proctor Password Help') =%> - - - - <%= text_field - "set.$setID.restricted_login_proctor_password" => '', - id => "set.$setID.restricted_login_proctor_password", - size => 10, - class => 'form-control form-control-sm' =%> - - diff --git a/templates/ContentGenerator/LoginProctor.html.ep b/templates/ContentGenerator/LoginProctor.html.ep index fe4af28282..b69e688263 100644 --- a/templates/ContentGenerator/LoginProctor.html.ep +++ b/templates/ContentGenerator/LoginProctor.html.ep @@ -42,15 +42,13 @@ %= form_for current_route, method => 'POST', begin % # Add the form data posted to the requested URI in hidden fields. % my @fields_to_print = - % grep { !/^(user|effectiveUser|passwd|key|proctor_user|proctor_passwd)$/ } param; + % grep { !/^(user|effectiveUser|passwd|key|proctor_user|proctor_passwd|set_level_proctor)$/ } param; % if (@fields_to_print) { <%= $c->hidden_fields(@fields_to_print) %> % } <%= $c->hidden_authen_fields =%> % - % if (param('submitAnswers') - % || ($userSet->restricted_login_proctor eq '' || $userSet->restricted_login_proctor eq 'No')) - % { + % if (param('submitAnswers') || $userSet->restricted_login_password eq '') { % # The user info and username field for the proctor. - <%= hidden_field proctor_user => "set_id:$setID" =%> + <%= hidden_field set_level_proctor => 1 =%> % } % % # The password field for the proctor.