From 785cc28f38b0ca4dc7bada26a3448f1470b93fe7 Mon Sep 17 00:00:00 2001 From: Glenn Rice Date: Fri, 16 Feb 2024 19:44:32 -0600 Subject: [PATCH] Add an authentication session. This requires another change to the `key` table. The `set_id` column has been removed, and in its place is a `session` key. This column is a JSON column and holds all session values. Note there are really two sessions that can be used. A cookie session (for this the Mojolicious session is used) or a database session. Which is used is determined by the `manage_session_via` course environment setting. The database session works by saving a hash in the stash key named 'webwork2.database_session'. This hash is saved to the database in the `after_dispatch` hook. This means that regardless of which session is used, session values can be set anytime after the session is created in authentication and before the `after_dispatch` hook is called. That is pretty much at anytime in a content generator module. Do not directly use the `Mojolicious::Controller::session` method (which will only set cookie session values) except in the authentication modules. Instead call the `WeBWorK::Authen::session` method which takes care of setting the values for the correct session type (database or cookie). One value the session now holds is what the `set_id` column stored before. In addition proctor authentication is completely reworked. Instead of the hack to use a separate proctor key, session values are used. There are some additional security measures implemented to make it harder for a student to hijack the session and gain access to a test after a proctoring session without having submitted the test as observed in #2243 and #2244. First, the proctor username and password (and the submit button) must come from a POST request. Parameters from a GET request are ignored for these things. It is a little harder to construct a POST request than a GET request. Second, the data saved in the session is specific to the set and version. So it is not possible for a student to open a new version of a test anymore, only to regain access to the version that was being worked. Third, the proctor_user is no longer saved in a hidden field in the GatewayQuiz form. If proctor authorization has been granted, then the authorization is saved in the session, and so the proctor_user is not needed. Some of the derived authentication modules will need some changes to work with this. Of course the LTIAdvantage, LTIAdvanced, and Proctor modules have already been changed and tested. Looking at the LDAP and CAS code, it seems that they should still work with this, but I can't test those. Most likely Cosign, Moodle, and Shibboleth will need changes. I don't know if anyone still uses the Moodle module. They really shouldn't, and should use LTI instead. I think that module should be deleted. --- lib/Mojolicious/WeBWorK.pm | 1 + lib/WeBWorK.pm | 6 +- lib/WeBWorK/Authen.pm | 188 ++++++++++---- lib/WeBWorK/Authen/LTIAdvanced.pm | 41 +-- lib/WeBWorK/Authen/LTIAdvantage.pm | 19 +- lib/WeBWorK/Authen/Proctor.pm | 244 +++++++++--------- lib/WeBWorK/ContentGenerator.pm | 19 +- lib/WeBWorK/ContentGenerator/GatewayQuiz.pm | 64 ++--- lib/WeBWorK/DB.pm | 67 +++-- lib/WeBWorK/DB/Record/Key.pm | 2 +- lib/WeBWorK/Debug.pm | 3 +- .../ContentGenerator/GatewayQuiz.html.ep | 1 - .../ContentGenerator/LoginProctor.html.ep | 6 +- 13 files changed, 337 insertions(+), 324 deletions(-) diff --git a/lib/Mojolicious/WeBWorK.pm b/lib/Mojolicious/WeBWorK.pm index 6546073950..cdb3865b44 100644 --- a/lib/Mojolicious/WeBWorK.pm +++ b/lib/Mojolicious/WeBWorK.pm @@ -172,6 +172,7 @@ sub startup ($app) { $SIG{__WARN__} = $c->stash->{orig_sig_warn} if defined $c->stash->{orig_sig_warn}; if ($c->isa('WeBWorK::ContentGenerator') && $c->ce) { + $c->authen->store_session if $c->authen; writeTimingLogEntry( $c->ce, '[' . $c->url_for . ']', diff --git a/lib/WeBWorK.pm b/lib/WeBWorK.pm index d0756cf6fd..2c6c42ada2 100644 --- a/lib/WeBWorK.pm +++ b/lib/WeBWorK.pm @@ -240,7 +240,7 @@ async sub dispatch ($c) { # we need to also check on the proctor. Note that in the gateway quiz # module this is double checked to be sure that someone isn't taking a # proctored quiz but calling the unproctored ContentGenerator. - if ($c->current_route =~ /^proctored_gateway_quiz|proctored_gateway_proctor_login$/) { + if ($c->current_route =~ /^(proctored_gateway_quiz|proctored_gateway_proctor_login)$/) { my $proctor_authen_module = WeBWorK::Authen::class($ce, 'proctor_module'); runtime_use $proctor_authen_module; my $authenProctor = $proctor_authen_module->new($c); @@ -251,6 +251,10 @@ async sub dispatch ($c) { await WeBWorK::ContentGenerator::LoginProctor->new($c)->go; return 0; } + } else { + # If any other page is opened, then revoke proctor authorization if it has been granted. + # Otherwise the student will be able to re-enter the test without again obtaining proctor authorization. + delete $c->authen->session->{proctor_authorization_granted}; } return 1; } else { diff --git a/lib/WeBWorK/Authen.pm b/lib/WeBWorK/Authen.pm index 299bba9e6c..403785b66e 100644 --- a/lib/WeBWorK/Authen.pm +++ b/lib/WeBWorK/Authen.pm @@ -329,7 +329,7 @@ sub get_credentials { } if (defined $cookieUser) { - $self->{user_id} = $cookieUser; + $self->{user_id} = trim($cookieUser); $self->{session_key} = $cookieKey; $self->{cookie_timestamp} = $cookieTimeStamp; $self->{login_type} = "normal"; @@ -470,7 +470,7 @@ sub maybe_send_cookie { return if $c->{rpc}; - my ($cookie_user, $cookie_key, $cookie_timestamp, $setID) = $self->fetchCookie; + my ($cookie_user, $cookie_key, $cookie_timestamp) = $self->fetchCookie; # Send a cookie if any of these conditions are met: @@ -501,7 +501,7 @@ sub maybe_send_cookie { ); if ($used_cookie || $unused_valid_cookie || $user_requests_cookie || $session_management_via_cookies) { - $self->sendCookie($self->{user_id}, $self->{session_key}, $setID); + $self->sendCookie($self->{user_id}, $self->{session_key}); } else { $self->killCookie; } @@ -613,35 +613,140 @@ sub unexpired_session_exists { return defined $Key && time <= $Key->timestamp + $self->{c}->ce->{sessionKeyTimeout}; } -# Clobbers any existing session for this $userID. -# A random key is generated, and that key is returned. -# When this is called in Proctor.pm, the actual user id is passed in via $trueUserID. -# The $userID is modified in that case and will not work in the hasPermissions call. +# Uses an existing session and session key if a key was found previously with a valid timestamp. Otherwise a random key +# is generated, and a new session and session key created. The key from the session is returned in any case. sub create_session { - my ($self, $userID, $trueUserID) = @_; + my ($self, $userID) = @_; my $c = $self->{c}; my $ce = $c->ce; my $db = $c->db; + my $newKey; + + if (!$c->stash->{'webwork2.database_session'} || !$c->stash->{'webwork2.database_session'}{user_id}) { + my @chars = @{ $ce->{sessionKeyChars} }; + srand; + $newKey = join('', @chars[ map rand(@chars), 1 .. $ce->{sessionKeyLength} ]); + $c->stash->{'webwork2.database_session'} = + { user_id => $userID, key => $newKey, timestamp => time, session => {} }; + } else { + $newKey = $c->stash->{'webwork2.database_session'}{key}; + } + + # If navigation is restricted, then set the set_id in the session. + $self->session(set_id => $c->stash->{setID}) + if $c->stash->{setID} && !$c->authz->hasPermissions($userID, 'navigation_allowed'); + + return $newKey; +} + +=head2 session + +This method can be used to get or set values in the session. Note that if +C is "session_cookie" then the Mojolicous cookie session +is used. If C is "key", then only the session in the +database is used. Note that database session is really a hash stored in +C<< $c->stash->{'webwork2.database_session} >> that has the following structure: + + { user_id => $userID, key => $key, timestamp => $timestamp, session => {} } + +Only keys in the C sub-hash can be set with this method. The +C, C, and C should be set directly in the +C hash. + +A single value from the session can be obtained as follows. + + $authen->session('key1'); - my @chars = @{ $ce->{sessionKeyChars} }; +Values can be set as in the following examples. - srand; - my $newKey = join('', @chars[ map rand(@chars), 1 .. $ce->{sessionKeyLength} ]); + $authen->session(key1 => 'value 1', key2 => 'value 2'); + $authen->session({ key1 => 'value 1', key2 => 'value 2' }); - my $setID = !$c->authz->hasPermissions($trueUserID // $userID, 'navigation_allowed') ? $c->stash('setID') : ''; +The entire session can be obtained as a hash reference as follows. - my $Key = $db->newKey(user_id => $userID, key => $newKey, timestamp => time, set_id => $setID); + my $session = $authen->session; - # DBFIXME this should be a REPLACE - eval { $db->deleteKey($userID) }; - eval { $db->addKey($Key) }; - if ($@) { - warn "Difficulty adding key for userID $userID: $@"; - eval { $db->putKey($Key) }; - warn "Couldn't put key for userid $userID either: $@" if $@; +=cut + +sub session { + my ($self, @params) = @_; + my $c = $self->{c}; + + # If session_management_via is not "session_cookie" (so should be "key"), then use the database session. + if ($c->ce->{session_management_via} ne 'session_cookie') { + $c->stash->{'webwork2.database_session'} //= { user_id => $self->{user_id}, session => {} }; + my $session = $c->stash->{'webwork2.database_session'}{session}; + + # Note that the return values are the same as those returned by the + # Mojolicious::Controller::session method in the following cases. + + # Return the session hash. + return $session unless @params; + + # Get session values. + return $session->{ $params[0] } unless @params > 1 || ref $params[0]; + + # Set session values. + my $values = ref $params[0] ? $params[0] : {@params}; + @$session{ keys %$values } = values %$values; + + return $c; } - return $newKey; + # If session_management_via is "session_cookie", then use the Mojolicious cookie session. + return $c->session(@params); +} + +=head2 store_session + +Store the database session. This is called after the current request has been +dispatched (in the C hook). This allows database session values +to be set or modified at any point before that is done. + +=cut + +sub store_session { + my $self = shift; + my $db = $self->{c}->db; + + if (my $session = $self->{c}->stash->{'webwork2.database_session'}) { + if ($WeBWorK::Debug::Enabled) { + debug('Saving database session. The database session contains'); + debug("\t$_ => $session->{$_}") for grep { $_ ne 'session' } keys %$session; + debug("\tsession => {"); + debug("\t\t$_ => $session->{session}{$_}") for keys %{ $session->{session} }; + debug("\t}"); + } + + my $key = $db->newKey($session); + # DBFIXME: This should be a REPLACE (but SQL::Abstract does not have REPLACE -- SQL::Abstract::mysql does!). + eval { $db->deleteKey($session->{user_id}) }; + eval { $db->addKey($key) }; + if ($@) { + warn "Difficulty adding key for userID $session->{user_id}: $@"; + eval { $db->putKey($key) }; + warn "Couldn't put key for userid $session->{user_id} either: $@" if $@; + } + } elsif ($self->{user_id}) { + debug('Deleting database session.'); + eval { $db->deleteKey($self->{user_id}) }; + } + + if ($WeBWorK::Debug::Enabled) { + # The cookie will actually be sent by the next line of the Mojolcious::Controller::rendered method after the + # after_dispatch hook in which this method is called. + my $cookieSession = $self->{c}->session; + if (keys %$cookieSession) { + if ($cookieSession->{expires} && $cookieSession->{expires} == 1) { + debug('The cookie session is expired.'); + } else { + debug('The cookie session contains'); + debug("\t$_ => $cookieSession->{$_}") for keys %$cookieSession; + } + } + } + + return; } =head2 check_session @@ -667,11 +772,16 @@ sub check_session { my $keyMatches = defined $possibleKey && $possibleKey eq $Key->key; - my $timestampValid = time <= $Key->timestamp + $ce->{sessionKeyTimeout}; + my $currentTime = time; + + my $timestampValid = + $ce->{session_management_via} eq 'session_cookie' && defined $self->{cookie_timestamp} + ? $currentTime <= $self->{cookie_timestamp} + $ce->{sessionKeyTimeout} + : $currentTime <= $Key->timestamp + $ce->{sessionKeyTimeout}; if ($keyMatches && $timestampValid && $updateTimestamp) { - $Key->timestamp(time); - $db->putKey($Key); + $Key->timestamp($currentTime); + $self->{c}->stash->{'webwork2.database_session'} = { $Key->toHash } if $keyMatches && $timestampValid; } return (1, $keyMatches, $timestampValid); @@ -696,8 +806,8 @@ sub killSession { } $self->forget_verification; - $self->killCookie if $ce->{session_management_via} eq 'session_cookie'; - $c->{db}->deleteKey($self->{user_id}) if defined $self->{user_id}; + $self->killCookie; + delete $c->stash->{'webwork2.database_session'}; return; } @@ -705,7 +815,7 @@ sub killSession { # Cookie management # Note that this does not really "fetch" the session cookie. It just gets -# the user_id, key, timestamp, and set_id from the cookie. +# the user_id, key, and timestamp from the session cookie. sub fetchCookie { my $self = shift; my $c = $self->{c}; @@ -716,11 +826,10 @@ sub fetchCookie { my $userID = $c->session->{user_id}; my $key = $c->session->{key}; my $timestamp = $c->session->{timestamp}; - my $setID = $c->session->{set_id}; if ($userID && $key) { debug(qq{fetchCookie: Returning userID="$userID", key="$key", timestamp="}, $timestamp, '"'); - return ($userID, $key, $timestamp, $setID); + return ($userID, $key, $timestamp); } else { debug('fetchCookie: Session cookie does not contain valid information. Returning nothing.'); return; @@ -730,7 +839,7 @@ sub fetchCookie { # Note that this does not actually "send" the cookie. It merely sets the default session values in the cookie. # The session cookie is actually sent by Mojolicious when the response is rendered. sub sendCookie { - my ($self, $userID, $key, $setID) = @_; + my ($self, $userID, $key) = @_; my $c = $self->{c}; my $ce = $c->ce; @@ -738,15 +847,10 @@ sub sendCookie { my $courseID = $c->stash('courseID'); - # This sets the setID in the cookie on initial login. - $setID = $c->stash('setID') - if !$setID && $c->authen->was_verified && !$c->authz->hasPermissions($userID, 'navigation_allowed'); - $c->session( user_id => $userID, key => $key, timestamp => time, - $setID ? (set_id => $setID) : (), # Set how long the browser should retain the cookie. expiration => $ce->{CookieLifeTime} eq 'session' ? 0 : $ce->{CookieLifeTime} ); @@ -760,20 +864,6 @@ sub killCookie { return; } -# This method is only used for a user that does not have the navigation_allowed permission, -# and is used to restrict that user to a specific set that the user is authenticated with. -sub get_session_set_id { - my $self = shift; - my $setID; - - if ($self->{c}->ce->{session_management_via} eq 'key') { - my $Key = $self->{c}->db->getKey($self->{c}->param('user')); - return $Key->set_id; - } else { - return $self->{c}->session->{set_id}; - } -} - # Utilities sub write_log_entry { diff --git a/lib/WeBWorK/Authen/LTIAdvanced.pm b/lib/WeBWorK/Authen/LTIAdvanced.pm index e06280b3be..3f7a1f91ab 100644 --- a/lib/WeBWorK/Authen/LTIAdvanced.pm +++ b/lib/WeBWorK/Authen/LTIAdvanced.pm @@ -92,10 +92,9 @@ sub request_has_data_for_this_verification_module { my $self = shift; my $c = $self->{c}; - # See comment in get_credentials() if ($c->{rpc}) { - debug("LTIAdvanced returning 1 because it is an rpc call"); - return 1; + debug("LTIAdvanced returning 0 because it is an rpc call"); + return 0; } # This module insists that the course is configured for LTI 1.3. @@ -125,19 +124,6 @@ sub get_credentials { debug("LTIAdvanced::get_credentials has been called\n"); - # This next part is necessary because some parts of webwork (e.g., - # WebworkWebservice.pm) need to replace the get_credentials() routine, - # but only replace the one in the parent class (out of caution, - # presumably). Therefore, we end up here even when authenticating - # for WebworkWebservice.pm. This would cause authentication failures - # when authenticating javascript web service requests (e.g., the - # Library Browser). - # Similar changes are needed in check_user() and verify_normal_user(). - if ($c->{rpc}) { - debug("falling back to superclass get_credentials (rpc call)"); - return $self->SUPER::get_credentials(@_); - } - ## Printing parameters to main page can help people set things up ## so we dont use the debug channel here if ($ce->{debug_lti_parameters}) { @@ -307,12 +293,6 @@ sub check_user { debug("LTIAdvanced::check_user has been called for user_id = |$user_id|"); - # See comment in get_credentials() - if ($c->{rpc}) { - #debug("falling back to superclass check_user (rpc call)"); - return $self->SUPER::check_user(@_); - } - if (!defined($user_id) || (defined $user_id && $user_id eq "")) { $self->{log_error} .= "no user id specified"; $self->{error} = $c->maketext( @@ -381,17 +361,6 @@ sub verify_normal_user { debug("LTIAdvanced::verify_normal_user called for user |$user_id|"); - # See comment in get_credentials() - if ($c->{rpc}) { - #debug("falling back to superclass verify_normal_user (rpc call)"); - return $self->SUPER::verify_normal_user(@_); - } - - # Call check_session in order to destroy any existing session cookies and Key table sessions - my ($sessionExists, $keyMatches, $timestampValid) = $self->check_session($user_id, $session_key, 0); - - debug("sessionExists='", $sessionExists, "' keyMatches='", $keyMatches, "' timestampValid='", $timestampValid, "'"); - my $auth_result = $self->authenticate; debug("auth_result=|${auth_result}|"); @@ -420,12 +389,6 @@ sub authenticate { my $self = shift; my ($c, $user) = map { $self->{$_}; } ('c', 'user_id'); - # See comment in get_credentials() - if ($c->{rpc}) { - #debug("falling back to superclass authenticate (rpc call)"); - return $self->SUPER::authenticate(@_); - } - debug("LTIAdvanced::authenticate called for user |$user|"); debug "ref(c) = |" . ref($c) . "|"; debug "ref of c->{paramcache} = |" . ref($c->{paramcache}) . "|"; diff --git a/lib/WeBWorK/Authen/LTIAdvantage.pm b/lib/WeBWorK/Authen/LTIAdvantage.pm index 49ad069138..04eaac4bd5 100644 --- a/lib/WeBWorK/Authen/LTIAdvantage.pm +++ b/lib/WeBWorK/Authen/LTIAdvantage.pm @@ -116,15 +116,13 @@ sub verify ($self) { # # To save this to the database a user_id value is needed that is unique for this request, satisfies the database # constrains on user_id's, and won't collide with webwork user_id's. So the login_hint (the LMS user id) and - # courseID are joined with ',set_id:' (hacking into the existing login proctor hack -- what an ugly hack to - # begin with). That means the LMS user id will also be needed to get the information back from the database, - # and so this is included in the state as well. + # courseID are joined with ',set_id:'. That means the LMS user id will also be needed to get the information + # back from the database, and so this is included in the state as well. # # To make matters worse, courseID's can contain hyphens, but user_id's can not. Fortunately courseID's can not # contain ampersats, while user_id's can. So the hyphens in the courseID are replaced with ampersats. # - # Finally, hack into the existing "nonce" key hack. The actual state and nonce are joined with a tab - # character and saved in the set_id field. Since the key value is "nonce", the database will not check + # Finally, hack into the existing "nonce" key hack. Since the key value is "nonce", the database will not check # to see that the user_id exists in the user table. my $key_id = join(',set_id:', $c->param('login_hint'), $c->stash->{courseID} =~ s/-/@/gr); @@ -137,7 +135,7 @@ sub verify ($self) { user_id => $key_id, key => 'nonce', timestamp => time, - set_id => join("\t", $c->stash->{LTIState}, $c->stash->{LTINonce}) + session => { lti_state => $c->stash->{LTIState}, lti_nonce => $c->stash->{LTINonce} } ); $c->db->addKey($key); @@ -161,7 +159,7 @@ sub get_credentials ($self) { # See the comments about the hacks involved here in the WeBWorK::Authen::LTIAdvantage::verify method above. my $key_id = join(',set_id:', $c->stash->{lti_lms_user_id}, $c->stash->{courseID} =~ s/-/@/gr); my $key = $c->db->getKey($key_id); - ($c->stash->{LTIState}, $c->stash->{LTINonce}) = split "\t", $key->set_id; + ($c->stash->{LTIState}, $c->stash->{LTINonce}) = ($key->session->{lti_state}, $key->session->{lti_nonce}); $c->db->deleteKey($key_id); $self->purge_old_state_keys; @@ -196,7 +194,6 @@ sub get_credentials ($self) { if (!defined $claims->{'https://purl.imsglobal.org/spec/lti/claim/deployment_id'} || $claims->{'https://purl.imsglobal.org/spec/lti/claim/deployment_id'} ne $ce->{LTI}{v1p3}{DeploymentID}) { - $c->log->info($claims->{'https://purl.imsglobal.org/spec/lti/claim/deployment_id'}); $self->{error} = $c->maketext( 'There was an error during the login process. Please speak to your instructor or system administrator.'); warn "Incorrect deployment id received in response.\n" if $ce->{debug_lti_parameters}; @@ -414,7 +411,7 @@ sub purge_old_state_keys ($self) { my $db = $c->db; my $time = time; - my @userIDs = $db->listKeys(); + my @userIDs = $db->listKeys; my @keys = $db->getKeys(@userIDs); my $modCourseID = $ce->{courseName} =~ s/-/@/gr; @@ -479,10 +476,6 @@ sub verify_normal_user ($self) { debug("LTIAdvantage::verify_normal_user called for user |$user_id|"); - # Call check_session in order to destroy any existing session cookies and key table sessions. - my ($sessionExists, $keyMatches, $timestampValid) = $self->check_session($user_id, $session_key, 0); - debug('sessionExists="', $sessionExists, '" keyMatches="', $keyMatches, '" timestampValid="', $timestampValid, '"'); - my $auth_result = $self->authenticate; debug("auth_result=|${auth_result}|"); diff --git a/lib/WeBWorK/Authen/Proctor.pm b/lib/WeBWorK/Authen/Proctor.pm index e9952e99aa..56ccc0df35 100644 --- a/lib/WeBWorK/Authen/Proctor.pm +++ b/lib/WeBWorK/Authen/Proctor.pm @@ -14,7 +14,7 @@ ################################################################################ package WeBWorK::Authen::Proctor; -use base qw/WeBWorK::Authen/; +use base 'WeBWorK::Authen'; =head1 NAME @@ -24,10 +24,15 @@ WeBWorK::Authen::Proctor - Authenticate gateway test proctors. use strict; use warnings; -use WeBWorK::Debug; + +use WeBWorK::Utils qw(x); use WeBWorK::DB::Utils qw(grok_vsetID); -use constant GENERIC_ERROR_MESSAGE => 'Invalid user ID or password.'; +use constant GENERIC_ERROR_MESSAGE => x('Invalid user ID or password.'); + +# Note that throughout this module only parameters in the request body_params are accepted (other than the +# effectiveUser). This means that only parameters for a POST request are allowed. GET request parameters are ignored. +# This is a security measure as it is more difficult to engineer a fake POST request than a GET request. sub verify { my $self = shift; @@ -36,7 +41,7 @@ sub verify { # At this point the usual authentication has already occurred and the user has been verified. If the # use_grade_auth_proctor option is set to 'No', then proctor authorization is not not needed. So return # 1 here to skip proctor authorization and proceed on to the GatewayQuiz module which will grade the test. - if ($c->param('submitAnswers')) { + if ($c->req->body_params->param('submitAnswers')) { my ($setName, $versionNum) = grok_vsetID($c->stash('setID')); my $userSet = $c->db->getMergedSetVersion($c->param('effectiveUser'), $setName, $versionNum); return 1 if $userSet && $userSet->use_grade_auth_proctor eq 'No'; @@ -45,162 +50,157 @@ sub verify { return $self->SUPER::verify(@_); } -# this is similar to the method in the base class, with these differences: +# This is similar to the method in the base class, with these differences: # 1. no guest logins # 2. no cookie -# 3. user_id/session_key/password come from params proctor_user/proctor_key/proctor_passwd +# 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 $ce = $c->ce; - my $db = $c->db; + my $c = $self->{c}; my ($set_id, $version_id) = grok_vsetID($c->stash('setID')); - # at least the user ID is available in request parameters - if (defined $c->param('proctor_user')) { - my $student_user_id = $c->param('effectiveUser'); - $self->{user_id} = $c->param('proctor_user'); - if ($self->{user_id} eq $set_id) { - $self->{user_id} = "set_id:$set_id"; - } - $self->{session_key} = $c->param('proctor_key'); - $self->{password} = $c->param('proctor_passwd'); - $self->{login_type} = - $c->param('submitAnswers') ? "proctor_grading:$student_user_id" : "proctor_login:$student_user_id"; + 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; + $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'; return 1; + } elsif ($c->authen->session('proctor_authorization_granted') && !$c->req->body_params->param('submitAnswers')) { + $self->{login_type} = 'proctor_login'; + $self->{credential_source} = 'session'; + return 1; } + + return 0; } -# duplicates method in superclass, adding additional check for permission -# to proctor quizzes +# If proctor authorization is granted a proctor user is not needed. So skip checking the user. sub check_user { - my $self = shift; - my $c = $self->{c}; - my $ce = $c->ce; - my $db = $c->db; - my $authz = $c->authz; - - my $submitAnswers = $c->param('submitAnswers'); - my $user_id = $self->{user_id}; - my $past_proctor_id = $c->param('past_proctor_user') || $user_id; - - # for set-level authentication we prepended "set_id:" - my $show_user_id = $user_id; - $show_user_id =~ s/^set_id://; - - if (defined $user_id and ($user_id eq '' || $show_user_id eq '')) { - $self->{log_error} = 'no user id specified'; - $self->{error} = 'You must specify a user ID.'; - return 0; - } + my $self = shift; + return 1 if $self->{credential_source} eq 'session'; + return $self->SUPER::check_user; +} - my $User = $db->getUser($user_id); +# This is similar to the method in the base class except that instead of creating a session, this just sets the +# "proctor_authorization_granted" value in the session. Note that the session used is the session of the original +# authentication module for this request. Furthermore it checks the proctor user permissions instead of the usual user +# login permissions. +sub verify_normal_user { + my $self = shift; + my $c = $self->{c}; - unless ($User) { - $self->{log_error} = 'user unknown'; - $self->{error} = GENERIC_ERROR_MESSAGE; - return 0; - } + # If the test is being submitted, then proctor credentials are always required. Note that if use_grade_auth_proctor + # is 'No', then the verify method will have returned 1, and this never happens. For an ongoing login session, only + # a key with versioned set information is accepted, and that version must match the requested set version. The set + # id will not have a version when opening a new version. For that new proctor credentials are required. + if ($self->{login_type} eq 'proctor_login' + && $c->stash('setID') =~ /,v\d+$/ + && $c->authen->session('proctor_authorization_granted') + && $c->authen->session('proctor_authorization_granted') eq $c->stash('setID')) + { + return 1; + } else { + my $auth_result = $self->authenticate; - # proctors may be tas, instructors, or proctors; if the last, they - # do not have the behavior course_access, so we don't bother to - # check that here. they must, however, be able to login, which - # it seems to me is an overlap between course permissions and - # course status behaviors. + if ($auth_result > 0) { + my $db = $c->db; + my $authz = $c->authz; - unless ($authz->hasPermissions($user_id, 'login')) { - $self->{log_error} = 'user not permitted to login'; - $self->{error} = GENERIC_ERROR_MESSAGE; - return 0; - } + my $user_id = $self->{user_id}; - if ($submitAnswers) { - unless ($authz->hasPermissions($user_id, 'proctor_quiz_grade')) { - # only set the error if this proctor is different - # than the past proctor, implying that we have - # tried to grade with a new proctor id - if ($past_proctor_id ne $user_id) { - $self->{log_error} = 'user not permitted to proctor quiz grading.'; - $self->{error} = - "User $show_user_id is not authorized to proctor test grade submissions in this course."; + # 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')) { + $self->{log_error} = 'user not permitted to login'; + $self->{error} = $c->maketext(GENERIC_ERROR_MESSAGE); + return 0; } - return 0; - } - } else { - # Need a UserSet to determine if it is configured to skip grade proctor - # authorization to grade the quiz. 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 ($setName, $versionNum) = grok_vsetID($c->stash('setID')); - my $userSet = $db->getMergedSet($c->param('effectiveUser'), $setName); - 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')) - ) - { - # Set the error based on if a single set password was required, a grade - # 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} = 'This quiz requires a set password to start, and the password was invalid.'; - } elsif ($userSet->use_grade_auth_proctor ne 'Yes') { - $self->{log_error} = - 'grade proctor required to login and user is not permitted to proctor quiz grading.'; - $self->{error} = "This quiz requires a grade proctor to start, and user $show_user_id is not " - . 'authorized to proctor test grade submissions in this course.'; + # 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')) { + $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); + return 0; + } + $c->authen->session('proctor_authorization_granted' => $c->stash('setID')); } else { - $self->{log_error} = 'user not permitted to proctor quiz logins.'; - $self->{error} = "User $show_user_id is not authorized to proctor test logins in this course."; + # 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 ($setName, $versionNum) = grok_vsetID($c->stash('setID')); + my $userSet = $db->getMergedSet($c->param('effectiveUser'), $setName); + 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')) + ) + { + # 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') { + $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 + ); + } 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); + } + return 0; + } + $c->authen->session('proctor_authorization_granted' => $c->stash('setID')); + } + return 1; + } else { + delete $c->authen->session->{'proctor_authorization_granted'}; + if ($auth_result == 0) { + $self->{log_error} = "authentication failed"; + $self->{error} = $c->maketext(GENERIC_ERROR_MESSAGE); } return 0; } } } -# this is similar to the method in the base class, excpet that the parameters -# proctor_user, proctor_key, and proctor_passwd are used +# 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 { my $self = shift; my $c = $self->{c}; $c->param('proctor_user', $self->{user_id}); - $c->param('proctor_key', $self->{session_key}); $c->param('proctor_passwd', ''); -} - -# rewrite the userID to include both the proctor's and the student's user ID -# and then call the default create_session method. -sub create_session { - my ($self, $userID, $newKey) = @_; - return $self->SUPER::create_session($self->proctor_key_id($userID), $newKey, $userID); + return; } -# rewrite the userID to include both the proctor's and the student's user ID -# and then call the default check_session method. -sub check_session { - my ($self, $userID, $possibleKey, $updateTimestamp) = @_; - - return $self->SUPER::check_session($self->proctor_key_id($userID), $possibleKey, $updateTimestamp); -} - -# proctor key ID rewriting helper -sub proctor_key_id { - my ($self, $userID, $newKey) = @_; - my $c = $self->{c}; - - my $proctor_key_id = $c->param('effectiveUser') . ',' . $userID; - $proctor_key_id .= ',g' if $self->{login_type} =~ /^proctor_grading/; - - return $proctor_key_id; -} +# Disable the session for proctors (instead use the session of the user authentication module). +sub create_session { } +sub check_session { } -# disable cookie functionality for proctors +# Disable cookie functionality for proctors. sub maybe_send_cookie { } +sub maybe_kill_cookie { } sub fetchCookie { } sub sendCookie { } sub killCookie { } diff --git a/lib/WeBWorK/ContentGenerator.pm b/lib/WeBWorK/ContentGenerator.pm index 6d5b3be00e..22254e641d 100644 --- a/lib/WeBWorK/ContentGenerator.pm +++ b/lib/WeBWorK/ContentGenerator.pm @@ -530,8 +530,8 @@ sub links ($c) { my $restricted_navigation = $authen->was_verified && !$authz->hasPermissions($userID, 'navigation_allowed'); # If navigation is restricted and the setID was not in the route stash, - # then get the setID this user is restricted to view from the authen cookie. - $setID = $authen->get_session_set_id if (!$setID && $restricted_navigation); + # then get the setID this user is restricted to view from the session. + $setID = $authen->session('set_id') if !$setID && $restricted_navigation; my $prettyProblemID = $problemID; @@ -1089,21 +1089,6 @@ sub hidden_authen_fields ($c, $id_prefix = undef) { return $c->hidden_fields('user', 'effectiveUser', 'key', 'theme'); } -=item hidden_proctor_authen_fields() - -Use hidden_fields to return hidden tags for request fields used in -proctor authentication. - -=cut - -sub hidden_proctor_authen_fields ($c) { - if ($c->param('proctor_user')) { - return $c->hidden_fields('proctor_user', 'proctor_key'); - } else { - return ''; - } -} - =item url_args(@fields) Return a hash containing values for each field mentioned in @fields, or all diff --git a/lib/WeBWorK/ContentGenerator/GatewayQuiz.pm b/lib/WeBWorK/ContentGenerator/GatewayQuiz.pm index aa91832b46..6627f02c5f 100644 --- a/lib/WeBWorK/ContentGenerator/GatewayQuiz.pm +++ b/lib/WeBWorK/ContentGenerator/GatewayQuiz.pm @@ -392,7 +392,7 @@ async sub pre_header_initialize ($c) { $tmplSet = $db->getMergedSet($effectiveUserID, $setID); # Now that is has been validated that this is a gateway test, save the assignment test for the processing of - # proctor keys for graded proctored tests. If a set was not obtained from the database, store a fake value here + # graded proctored tests. If a set was not obtained from the database, store a fake value here # to be able to continue. $c->{assignment_type} = $tmplSet->assignment_type || 'gateway'; @@ -500,7 +500,8 @@ async sub pre_header_initialize ($c) { my @setVersions = $db->getSetVersions(map { [ $effectiveUserID, $setID,, $_ ] } @setVersionIDs); for (@setVersions) { $totalNumVersions++; - $currentNumVersions++ if (!$timeInterval || $_->version_creation_time() > ($c->submitTime - $timeInterval)); + $currentNumVersions++ + if (!$timeInterval || $_->version_creation_time() > ($c->submitTime - $timeInterval)); } } @@ -644,14 +645,19 @@ async sub pre_header_initialize ($c) { $c->{invalidSet} = 'This set is closed. No new set versions may be taken.'; } - # If the set or problem is invalid, then delete any proctor keys if any and return. + # If the proctor session key does not have a set version id, then add it. This occurs when a student + # initialy enters a proctored test, since the version id is not determined until just above. + if ($c->authen->session('proctor_authorization_granted') + && $c->authen->session('proctor_authorization_granted') !~ /,v\d+$/) + { + if ($setVersionNumber) { $c->authen->session(proctor_authorization_granted => "$setID,v$setVersionNumber"); } + else { delete $c->authen->session->{proctor_authorization_granted}; } + } + + # If the set or problem is invalid, then delete any proctor session keys and return. if ($c->{invalidSet} || $c->{invalidProblem}) { if (defined $c->{assignment_type} && $c->{assignment_type} eq 'proctored_gateway') { - my $proctorID = $c->param('proctor_user'); - if ($proctorID) { - eval { $db->deleteKey("$effectiveUserID,$proctorID"); }; - eval { $db->deleteKey("$effectiveUserID,$proctorID,g"); }; - } + delete $c->authen->session->{proctor_authorization_granted}; } return; } @@ -878,32 +884,13 @@ async sub pre_header_initialize ($c) { # and answers can be recorded, then save the last answer for future reference. # Also save the persistent data to the database even when the last answer is not saved. - # First, deal with answers being submitted for a proctored exam. Delete the proctor keys that authorized the - # grading, so that it isn't possible to log in and take another proctored test without being reauthorized. - if ($c->{submitAnswers} && $c->{assignment_type} eq 'proctored_gateway') { - my $proctorID = $c->param('proctor_user'); - - # If there are no attempts left, delete all proctor keys for this user. - if ($set->attempts_per_version > 0 - && $set->attempts_per_version - 1 - $problem->num_correct - $problem->num_incorrect <= 0) - { - eval { $db->deleteAllProctorKeys($effectiveUserID); }; - } else { - # Otherwise, delete only the grading key. - eval { $db->deleteKey("$effectiveUserID,$proctorID,g"); }; - # In this case there may be a past login proctor key that can be kept so that another login to continue - # working the test is not needed. - if ($c->param('past_proctor_user') && $c->param('past_proctor_key')) { - $c->param('proctor_user', scalar $c->param('past_proctor_user')); - $c->param('proctor_key', scalar $c->param('past_proctor_key')); - } - } - # This is unsubtle, but we'd rather not have bogus keys sitting around. - if ($@) { - die "ERROR RESETTING PROCTOR GRADING KEY(S): $@\n"; - } - - } + # Deal with answers being submitted for a proctored exam. If there are no attempts left, then delete the + # proctor session key so that it isn't possible to start another proctored test without being reauthorized. + delete $c->authen->session->{proctor_authorization_granted} + if ($c->{submitAnswers} + && $c->{assignment_type} eq 'proctored_gateway' + && $set->attempts_per_version > 0 + && $set->attempts_per_version - 1 - $problem->num_correct - $problem->num_incorrect <= 0); my @pureProblems = $db->getAllProblemVersions($effectiveUserID, $setID, $versionID); for my $i (0 .. $#problems) { @@ -1204,7 +1191,8 @@ async sub pre_header_initialize ($c) { $c->{submitAnswers} && ( $will{recordAnswers} - || (!$set->version_last_attempt_time && $c->submitTime > $set->due_date + $ce->{gatewayGracePeriod}) + || (!$set->version_last_attempt_time + && $c->submitTime > $set->due_date + $ce->{gatewayGracePeriod}) ) ) || ( @@ -1228,7 +1216,8 @@ async sub pre_header_initialize ($c) { $c->{submitAnswers} && ( $will{recordAnswers} - || (!$set->version_last_attempt_time && $c->submitTime > $set->due_date + $ce->{gatewayGracePeriod}) + || (!$set->version_last_attempt_time + && $c->submitTime > $set->due_date + $ce->{gatewayGracePeriod}) ) ); @@ -1280,7 +1269,8 @@ async sub pre_header_initialize ($c) { my $pScore = 0; if (ref $pg) { # If a pg object is available, then use the pg recorded score and save it in the @probStatus array. - $pScore = compute_reduced_score($ce, $problems[$i], $set, $pg->{state}{recorded_score}, $c->submitTime); + $pScore = + compute_reduced_score($ce, $problems[$i], $set, $pg->{state}{recorded_score}, $c->submitTime); $probStatus[$i] = $pScore if $pScore > $probStatus[$i]; } else { # If a pg object is not available, then use the saved problem status. diff --git a/lib/WeBWorK/DB.pm b/lib/WeBWorK/DB.pm index 4d59f2504c..02fa05f33a 100644 --- a/lib/WeBWorK/DB.pm +++ b/lib/WeBWorK/DB.pm @@ -99,6 +99,7 @@ use Carp; use Data::Dumper; use Scalar::Util qw/blessed/; use HTML::Entities qw( encode_entities ); +use Mojo::JSON qw(encode_json decode_json); use WeBWorK::DB::Schema; use WeBWorK::DB::Utils qw/make_vsetID grok_vsetID grok_setID_from_vsetID_sql @@ -774,11 +775,18 @@ sub deletePermissionLevel { BEGIN { *Key = gen_schema_accessor("key"); - *newKey = gen_new("key"); *countKeysWhere = gen_count_where("key"); *existsKeyWhere = gen_exists_where("key"); *listKeysWhere = gen_list_where("key"); - *getKeysWhere = gen_get_records_where("key"); + # FIXME: getKeysWhere is never used, but if it is used the "session" in the returned keys is not JSON decoded. + *getKeysWhere = gen_get_records_where("key"); +} + +sub newKey { + my ($self, @values) = @_; + my $key = $self->{key}{record}->new(@values); + $key->session({}) unless ref($key->session) eq 'HASH'; + return $key; } sub countKeys { return scalar shift->listKeys(@_) } @@ -799,50 +807,41 @@ sub existsKey { sub getKey { my ($self, $userID) = shift->checkArgs(\@_, qw/user_id/); - return ($self->getKeys($userID))[0]; + my ($key) = $self->{key}->gets([$userID]); + $key->session(decode_json($key->session)) if $key; + return $key; } sub getKeys { my ($self, @userIDs) = shift->checkArgs(\@_, qw/user_id*/); - return $self->{key}->gets(map { [$_] } @userIDs); + my @keys = $self->{key}->gets(map { [$_] } @userIDs); + $_->session(decode_json($_->session)) for @keys; + return @keys; } sub addKey { - # PROCTORING - allow comma in keyfields my ($self, $Key) = shift->checkArgs(\@_, qw/VREC:key/); - # PROCTORING - check for both user and proctor - # we allow for two entries for proctor keys, one of the form - # userid,proctorid (which authorizes login), and the other - # of the form userid,proctorid,g (which authorizes grading) - # (having two of these means that a proctored test will require - # authorization for both login and grading). - if ($Key->user_id =~ /([^,]+)(?:,([^,]*))?(,g)?/) { - my ($userID, $proctorID) = ($1, $2); - croak "addKey: user $userID not found" - # unless $self->{user}->exists($userID); - unless $Key->key eq "nonce" or $self->{user}->exists($userID); - croak "addKey: proctor $proctorID not found" - # unless $self->{user}->exists($proctorID); - unless $Key->key eq "nonce" or $self->{user}->exists($proctorID); - } else { - croak "addKey: user ", $Key->user_id, " not found" - # unless $self->{user}->exists($Key->user_id); - unless $Key->key eq "nonce" or $self->{user}->exists($Key->user_id); - } + croak "addKey: user ", $Key->user_id, " not found" + unless $Key->key eq "nonce" || $self->{user}->exists($Key->user_id); + + my $keyCopy = $self->newKey($Key); + $keyCopy->session(encode_json($Key->session)) if ref($Key->session) eq 'HASH'; - eval { return $self->{key}->add($Key); }; + my $result = eval { $self->{key}->add($keyCopy) }; if (my $ex = caught WeBWorK::DB::Ex::RecordExists) { croak "addKey: key exists (perhaps you meant to use putKey?)"; } elsif ($@) { die $@; } + return $result; } sub putKey { - # PROCTORING - allow comma in keyfields my ($self, $Key) = shift->checkArgs(\@_, qw/VREC:key/); - my $rows = $self->{key}->put($Key); # DBI returns 0E0 for 0. + my $keyCopy = $self->newKey($Key); + $keyCopy->session(encode_json($Key->session)) if ref($Key->session) eq 'HASH'; + my $rows = $self->{key}->put($keyCopy); # DBI returns 0E0 for 0. if ($rows == 0) { croak "putKey: key not found (perhaps you meant to use addKey?)"; } else { @@ -855,13 +854,6 @@ sub deleteKey { return $self->{key}->delete($userID); } -sub deleteAllProctorKeys { - my ($self, $userID) = shift->checkArgs(\@_, qw/user_id/); - my $where = [ user_id_like => "$userID,%" ]; - - return $self->{key}->delete_where($where); -} - ################################################################################ # setting functions ################################################################################ @@ -2361,11 +2353,10 @@ sub check_user_id { # (valid characters are [-a-zA-Z0-9_.,@]) return 0; } } -# the (optional) second argument to checkKeyfields is to support versioned -# (gateway) sets, which may include commas in certain fields (in particular, -# set names (e.g., setDerivativeGateway,v1) and user names (e.g., -# username,proctorname) +# The (optional) second argument to checkKeyfields is to support versioned +# (gateway) sets, which may include commas in certain fields (in particular, +# set names (e.g., setDerivativeGateway,v1) sub checkKeyfields($;$) { my ($Record, $versioned) = @_; foreach my $keyfield ($Record->KEYFIELDS) { diff --git a/lib/WeBWorK/DB/Record/Key.pm b/lib/WeBWorK/DB/Record/Key.pm index 1f8315f4a6..e2c038739a 100644 --- a/lib/WeBWorK/DB/Record/Key.pm +++ b/lib/WeBWorK/DB/Record/Key.pm @@ -30,7 +30,7 @@ BEGIN { user_id => { type => "VARCHAR(100) NOT NULL", key => 1 }, key => { type => "TEXT" }, timestamp => { type => "BIGINT" }, - set_id => { type => "TINYTEXT" }, + session => { type => "TEXT NOT NULL DEFAULT '{}'" }, ); } diff --git a/lib/WeBWorK/Debug.pm b/lib/WeBWorK/Debug.pm index bf55fcd1fa..865de908c4 100644 --- a/lib/WeBWorK/Debug.pm +++ b/lib/WeBWorK/Debug.pm @@ -94,9 +94,10 @@ Write @messages to the debugging log. sub debug { my @message = @_; - @message = undefstr('###UNDEF###', @message); if ($Enabled) { + @message = undefstr('###UNDEF###', @message); + my ($package, $filename, $line, $subroutine) = caller(1); return if defined $AllowSubroutineOutput and not $subroutine =~ m/$AllowSubroutineOutput/; return if defined $DenySubroutineOutput and $subroutine =~ m/$DenySubroutineOutput/; diff --git a/templates/ContentGenerator/GatewayQuiz.html.ep b/templates/ContentGenerator/GatewayQuiz.html.ep index c74428452b..124e95b1cd 100644 --- a/templates/ContentGenerator/GatewayQuiz.html.ep +++ b/templates/ContentGenerator/GatewayQuiz.html.ep @@ -388,7 +388,6 @@ % <%= form_for $action, name => 'gwquiz', method => 'POST', class => 'problem-main-form', begin =%> <%= $c->hidden_authen_fields =%> - <%= $c->hidden_proctor_authen_fields =%> % % # Hacks to use a javascript link to trigger previews and jump to subsequent pages of a multipage test. <%= hidden_field pageChangeHack => '' =%> diff --git a/templates/ContentGenerator/LoginProctor.html.ep b/templates/ContentGenerator/LoginProctor.html.ep index bca4805257..02635f8309 100644 --- a/templates/ContentGenerator/LoginProctor.html.ep +++ b/templates/ContentGenerator/LoginProctor.html.ep @@ -42,16 +42,12 @@ %= 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|force_password_authen|proctor_user|proctor_key|proctor_passwd)$/ } - % param; + % grep { !/^(user|effectiveUser|passwd|key|force_passwd_authen|proctor_user|proctor_passwd)$/ } param; % if (@fields_to_print) { <%= $c->hidden_fields(@fields_to_print) %> % } <%= $c->hidden_authen_fields =%> % - <%= hidden_field past_proctor_user => param('past_proctor_user') || param('proctor_user') =%> - <%= hidden_field past_proctor_key => param('past_proctor_key') || param('proctor_key') =%> - % % if (param('submitAnswers') % || ($userSet->restricted_login_proctor eq '' || $userSet->restricted_login_proctor eq 'No')) % {