Skip to content

Commit

Permalink
Fix proctor login for login_proctor users with Proctor status.
Browse files Browse the repository at this point in the history
This was broken by openwebwork#2252.  The issue is that `login_proctor` users with
`Proctor` status do not have the `allow_course_access` behavior.  Thus
the check on line 509 of `lib/WeBWorK/Authen.pm` fails.  So this skips
that check if the `login_type` starts with "proctor".  This will only be
true if the `WeBWorK::Authen::Proctor` module is the current
authentication module and the `proctor_user` parameter is set.  Note
that the proctor authen module already checks all of the necessary
permissions, so this check is not needed.  When the check on line 509 of
`lib/WeBWorK/Authen.pm` was in the `check_user` method of
`WeBWorK::Authen` this check was not even done by the
`WeBWorK::Authen::Proctor` module because that module overrides the
`check_user` method.

To test this create a proctored test that is set with a "Proctor
Authorization Type" of "Both Start and Grade", and create a
`login_proctor` user with the `Proctor` status. Proctor authentication
should succeed if the `login_proctor` user's credentials are entered on
the proctor login page.  For the develop branch this not only fails, but
the message "This user is not allowed to log in to this course" is
shown.  While that message is true, it is not applicable here.  That is
the whole point of a `login_proctor`.  Note that set level proctors
(hidden users created if you set the "Password" in the "Proctoring
Parameters" for a test) automatically have "Proctor" status.  So set
level proctoring is completely broken.

Also move session creation and setting of `$self->{initial_login}` to 1
until after the checks now on lines 509-521 of `lib/WeBWorK/Authen.pm`.
Those things should not be done if `verify_normal_user` is returning 0.
  • Loading branch information
drgrice1 committed Feb 15, 2024
1 parent 24f276d commit 1cebe43
Showing 1 changed file with 5 additions and 3 deletions.
8 changes: 5 additions & 3 deletions lib/WeBWorK/Authen.pm
Original file line number Diff line number Diff line change
Expand Up @@ -505,10 +505,10 @@ sub verify_normal_user {
my $auth_result = $self->authenticate;

if ($auth_result > 0) {
$self->{session_key} = $self->create_session($user_id);
$self->{initial_login} = 1;
# deny certain roles (dropped students, proctor roles)
unless ($c->ce->status_abbrev_has_behavior($c->db->getUser($user_id)->status, "allow_course_access")) {
unless ($self->{login_type} =~ /^proctor/
|| $c->ce->status_abbrev_has_behavior($c->db->getUser($user_id)->status, "allow_course_access"))
{
$self->{log_error} = "user not allowed course access";
$self->{error} = "This user is not allowed to log in to this course";
return 0;
Expand All @@ -519,6 +519,8 @@ sub verify_normal_user {
$self->{error} = "This user is not allowed to log in to this course";
return 0;
}
$self->{session_key} = $self->create_session($user_id);
$self->{initial_login} = 1;
return 1;
} elsif ($auth_result == 0) {
$self->{log_error} = "authentication failed";
Expand Down

0 comments on commit 1cebe43

Please sign in to comment.