From 57f13f83805a3ed7b893ec74455b0b6bc5ce0ddf Mon Sep 17 00:00:00 2001 From: Glenn Rice Date: Thu, 15 Feb 2024 08:23:40 -0600 Subject: [PATCH] Fix proctor login for `login_proctor` users with `Proctor` status. This was broken by #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. --- lib/WeBWorK/Authen.pm | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/lib/WeBWorK/Authen.pm b/lib/WeBWorK/Authen.pm index 104ac603c7..5717911e33 100644 --- a/lib/WeBWorK/Authen.pm +++ b/lib/WeBWorK/Authen.pm @@ -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; @@ -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";