Store the set level proctor password in the set

.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).

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;

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;
 		}

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
 		}

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;

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,14 +200,53 @@ 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;
 		}
 	}
 }
 
+# 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 {

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) {

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);
 

lib/WeBWorK/ContentGenerator/AchievementsLeaderboard.pm

@@ -25,26 +25,25 @@ 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 ],
 	}))
 	{
 		$allUserAchievements{ $_->user_id }{ $_->achievement_id } = $_;
 	}
 
 	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');
 

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');
 

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

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;
 

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 }) };
 

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 => [

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 => [