Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix proctor login for login_proctor users with Proctor status. #2327

Merged
merged 1 commit into from
Feb 23, 2024

Conversation

drgrice1
Copy link
Member

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.

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.
@drgrice1 drgrice1 force-pushed the bugfix/login_proctor branch from 1cebe43 to 57f13f8 Compare February 21, 2024 14:14
@Alex-Jordan Alex-Jordan merged commit 65a1b74 into openwebwork:develop Feb 23, 2024
2 checks passed
@drgrice1 drgrice1 deleted the bugfix/login_proctor branch February 23, 2024 10:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants