Prevent duplicate test submissions.

htdocs/js/GatewayQuiz/gateway.js

@@ -8,6 +8,23 @@
 (() => {
 	if (!document.gwquiz) return;
 
+	// Only allow the form to be submitted once. The page remains displayed while the request is pending, and so if the
+	// server response is slow the user might click on "Grade Test" again which would send another submission without
+	// confirmation.
+	let formSubmitted = false;
+	document.gwquiz.addEventListener('submit', (evt) => {
+		if (formSubmitted) evt.preventDefault();
+		formSubmitted = true;
+	});
+
+	// If the page is restored from the back-forward cache, then allow the form to be submitted again. The server will
+	// refuse to grade a submission from this page if the test was graded in the meantime.
+	window.addEventListener('pageshow', (evt) => {
+		if (!evt.persisted) return;
+		formSubmitted = false;
+		actuallySubmit = false;
+	});
+
 	// Gateway timer
 	const timerDiv = document.getElementById('gwTimer'); // The timer div element
 	let actuallySubmit = false; // This needs to be set to true to allow an actual submission.
@@ -386,7 +403,7 @@
 			// Prevent the link from being followed.
 			evt.preventDefault();
 			document.gwquiz.newPage.value = pageChangeLink.dataset.pageNumber;
-			document.gwquiz.submit();
+			document.gwquiz.requestSubmit();
 		});
 	});
 

lib/WeBWorK/ContentGenerator/GatewayQuiz.pm

@@ -11,7 +11,7 @@ deal with versioning sets
 use Mojo::Promise;
 use Mojo::JSON qw(encode_json decode_json);
 
-use WeBWorK::Utils                    qw(encodeAnswers decodeAnswers wwRound);
+use WeBWorK::Utils                    qw(encodeAnswers decodeAnswers max wwRound);
 use WeBWorK::Utils::DateTime          qw(before between after);
 use WeBWorK::Utils::Files             qw(path_is_subdir);
 use WeBWorK::Utils::Instructor        qw(assignSetVersionToUser);
@@ -227,6 +227,39 @@ sub can_gradeUnsubmittedTest ($c, $user, $permissionLevel, $effectiveUser, $set,
 		&& (after($set->due_date + $c->ce->{gatewayGracePeriod}) && !$set->version_last_attempt_time);
 }
 
+# Save the time of a test submission to the set version's version_last_attempt_time, but only if the test has not been
+# submitted since the test page was rendered.  Returns 1 if the time was saved, and 0 if the test was submitted since
+# the page was rendered. This is used to protect against various forms of resubmission being graded and replacing the
+# first valid submission.  This is also used by the LoginProctor module when a proctored test is submitted.  In that
+# case the versionLastAttemptTime parameter is updated to the new value so that the proctor login form forwards it to
+# this module when the proctor authorizes grading.
+sub recordSubmissionTime ($c, $set, $problem) {
+	my $attemptNumber = $c->param('attemptNumber');
+	return 0
+		if defined $attemptNumber
+		&& $attemptNumber =~ /^\d+$/
+		&& $attemptNumber != $problem->num_correct + $problem->num_incorrect;
+
+	my $expectedTime = $c->param('versionLastAttemptTime');
+	$expectedTime = $set->version_last_attempt_time || 0 unless defined $expectedTime && $expectedTime =~ /^\d+$/;
+	my $submissionTime = max(int($c->submitTime), $expectedTime + 1);
+
+	return 0
+		if $c->db->{set_version}->update_where(
+			{ version_last_attempt_time => $submissionTime },
+			{
+				user_id                   => $set->user_id,
+				set_id                    => $set->set_id,
+				version_id                => $set->version_id,
+				version_last_attempt_time => $expectedTime
+			}
+		) == 0;
+
+	$c->{submissionTime} = $submissionTime;
+	$c->param(versionLastAttemptTime => $submissionTime);
+	return 1;
+}
+
 sub can_showScore ($c, $user, $permissionLevel, $effectiveUser, $set, $problem, $tmplSet) {
 	return
 		$c->authz->hasPermissions($user->user_id, 'view_hidden_work')
@@ -799,9 +832,18 @@ async sub pre_header_initialize ($c) {
 		useMathQuill       => $user->useMathQuill ne '' ? $user->useMathQuill : $ce->{pg}{options}{useMathQuill},
 	);
 
-	# Does the user have permission to use certain options?
 	my @args = ($user, $permissionLevel, $effectiveUser, $set, $problem, $tmplSet);
-	my %can  = (
+
+	if ($want{recordAnswers} && $c->can_recordAnswers(@args) && !$c->recordSubmissionTime($set, $problem)) {
+		$c->{submitAnswers} = 0;
+		$want{recordAnswers} = 0;
+		$c->addbadmessage(
+			$c->maketext(
+				'This submission was not graded because this test was already submitted from another page or window.')
+		);
+	}
+
+	my %can = (
 		showOldAnswers        => $c->can_showOldAnswers(@args),
 		showCorrectAnswers    => $c->can_showCorrectAnswers(@args),
 		showProblemGrader     => $c->can_showProblemGrader(@args),
@@ -981,6 +1023,29 @@ async sub pre_header_initialize ($c) {
 		}
 
 		my @pureProblems = $db->getAllProblemVersions($effectiveUserID, $setID, $versionID);
+
+		# Another submission may occur from a page rendered after this submission arrived but before it finishes
+		# grading.  So before saving any scores, save the new number of attempts for one problem, but only if it still
+		# has the number of attempts that it had when this request started.  This is done in a single database update,
+		# so only one submission can be recorded for each attempt.
+		my $alreadyGraded = 0;
+		if ($c->{submitAnswers} && $will{recordAnswers}) {
+			my $firstProblem = $problems[ $probOrder[0] ];
+			my $state        = $pg_results[ $probOrder[0] ]{state};
+			$alreadyGraded = $db->{problem_version}->update_where(
+				{ num_correct => $state->{num_of_correct_ans}, num_incorrect => $state->{num_of_incorrect_ans} },
+				{
+					user_id       => $effectiveUserID,
+					set_id        => $setID,
+					version_id    => $versionID,
+					problem_id    => $firstProblem->problem_id,
+					num_correct   => $firstProblem->num_correct   || [ 0, undef ],
+					num_incorrect => $firstProblem->num_incorrect || [ 0, undef ]
+				}
+			) == 0;
+			$will{recordAnswers} = 0 if $alreadyGraded;
+		}
+
 		for my $i (0 .. $#problems) {
 			# Process each problem.
 			my $pureProblem = $pureProblems[ $probOrder[$i] ];
@@ -1058,7 +1123,11 @@ async sub pre_header_initialize ($c) {
 						. $problem->num_incorrect);
 			} elsif ($c->{submitAnswers}) {
 				# This is the case answers were submitted but cannot be saved. Report an error message.
-				if ($c->{isClosed}) {
+				if ($alreadyGraded) {
+					$scoreRecordedMessage[ $probOrder[$i] ] = $c->maketext(
+						'Your score was not recorded because this test was already submitted from another page or '
+							. 'window.');
+				} elsif ($c->{isClosed}) {
 					$scoreRecordedMessage[ $probOrder[$i] ] =
 						$c->maketext('Your score was not recorded because this problem set version is not open.');
 				} elsif ($problem->num_correct + $problem->num_incorrect >= $set->attempts_per_version) {
@@ -1169,7 +1238,7 @@ async sub pre_header_initialize ($c) {
 		)
 	{
 		# Save the submission time if we're recording the answer, or if the first submission occurs after the due_date.
-		$set->version_last_attempt_time($timeNowInt)
+		$set->version_last_attempt_time($c->{submissionTime} // $timeNowInt)
 			if (
 				$c->{submitAnswers}
 				&& (

lib/WeBWorK/ContentGenerator/LoginProctor.pm

@@ -48,23 +48,18 @@ async sub initialize ($c) {
 			# This should never happen.
 			die 'Request to grade a set version before any tests have been taken.' if $noSetVersions;
 
-			# Determine if answers can be recorded, and set last_attempt_time if appropriate.
+			# Determine if answers can be recorded, and if so record the submission time so that the time spent waiting
+			# for proctor authorization does not count against the time limit.  If that fails because the test was
+			# already submitted, then this submission will not be graded after proctor authorization.
+			my $problem = $db->getMergedProblemVersion($effectiveUserID, $c->stash->{setID},
+				$versionNum, ($db->listProblemVersions($effectiveUserID, $c->stash->{setID}, $versionNum))[0]);
 			if (WeBWorK::ContentGenerator::GatewayQuiz::can_recordAnswers(
-				$c,
-				$db->getUser($userID),
-				$db->getPermissionLevel($userID),
-				$c->{effectiveUser},
-				$c->stash->{userSet},
-				$db->getMergedProblemVersion(
-					$effectiveUserID, $c->stash->{setID},
-					$versionNum, ($db->listProblemVersions($effectiveUserID, $c->stash->{setID}, $versionNum))[0]
-				)
+				$c,                               $db->getUser($userID),
+				$db->getPermissionLevel($userID), $c->{effectiveUser},
+				$c->stash->{userSet},             $problem
 			))
 			{
-				$c->stash->{userSet}->version_last_attempt_time(int($c->submitTime));
-				# FIXME: This saves all of the merged set data into the set_user table.  We live with this in other
-				# places for versioned sets, but it's not ideal.
-				$db->putSetVersion($c->stash->{userSet});
+				WeBWorK::ContentGenerator::GatewayQuiz::recordSubmissionTime($c, $c->stash->{userSet}, $problem);
 			}
 		}
 	}

templates/ContentGenerator/GatewayQuiz.html.ep

@@ -425,6 +425,8 @@
 		<%= hidden_field courseID => $ce->{courseName} =%>
 		%
 		<%= hidden_field startTime => param('startTime') || time =%>
+		<%= hidden_field versionLastAttemptTime => $c->{set}->version_last_attempt_time || 0 =%>
+		<%= hidden_field attemptNumber => $c->{attemptNumber} =%>
 		% if ($numProbPerPage && $numPages > 1) {
 			<%= hidden_field newPage => '' =%>
 			<%= hidden_field currentPage => $pageNumber =%>