Skip to content

Commit

Permalink
Don't require $authen{admin_module} is a subset of $authen{user_module}.
Browse files Browse the repository at this point in the history
This changes the logic of how $authen{admin_module} is used to restrict
modules for the admin course. Instead of looping over all user_modules,
and requiring that module also be listed in admin_modules, only loop over
the modules listed in admin_modules when logging into the admin course.

This fixes an issue with LDAP (and maybe other authen modules), where
if they are listed in the user_module list, they have to also be listed
in the admin_module list, otherwise the error message generated for
this causes the authentication to fail.
  • Loading branch information
somiaj committed Aug 7, 2024
1 parent 0e92922 commit 7e8035e
Show file tree
Hide file tree
Showing 3 changed files with 12 additions and 23 deletions.
5 changes: 2 additions & 3 deletions conf/defaults.config
Original file line number Diff line number Diff line change
Expand Up @@ -744,9 +744,8 @@ $authen{user_module} = {"*" => "WeBWorK::Authen::Basic_TheLastOption"};
# A string or a hash is accepted, as above.
$authen{proctor_module} = "WeBWorK::Authen::Proctor";

# List of authentication modules that may be used to enter the admin course.
# This should always be an array reference with a subset of the modules named
# in $authen{user_module}.
# This is used instead of $authen{user_module} to determine which authentication
# module to use when logging into the admin course.
$authen{admin_module} = ['WeBWorK::Authen::Basic_TheLastOption'];

################################################################################
Expand Down
13 changes: 6 additions & 7 deletions conf/localOverrides.conf.dist
Original file line number Diff line number Diff line change
Expand Up @@ -485,19 +485,18 @@ $mail{feedbackRecipients} = [
#$authen{proctor_module} = "WeBWorK::Authen::Proctor";

# List of authentication modules that may be used to enter the admin course.
# This should be a non-empty sublist of whatever is in $authen{user_module}.
# This is used instead of $authen{user_module} when logging into the admin course.
# Since the admin course provides overall power to add/delete courses, access
# to this course should be protected by the best possible authentication you
# have available to you. The current default is
# WeBWorK::Authen::Basic_TheLastOption which is simple password based
# authentication for a password locally stored in your WeBWorK server's
# database. On one hand, this is necessary as the initial setting, as it is the
# only option available when a new server is being installed. However, since
# this option does not make use of multi-factor authentication or provide any
# capabilities to prevent dictionary attacks, etc. At the very least you should
# use a very strong password. If you have the option to use a more secure
# authentication approach to the admin course (one which you are confident
# cannot be spoofed) that is preferable.
# only option available when a new server is being installed, on the other hand,
# this option does not provide any capabilities to prevent dictionary attacks, etc.
# At the very least you should use a very strong password with two factor authentication.
# If you have the option to use a more secure authentication approach to the admin course
# (one which you are confident cannot be spoofed) that is preferable.
#
# Note that if you include authentication module config files further down,
# those may override the setting of $authen{admin_module} here.
Expand Down
17 changes: 4 additions & 13 deletions lib/WeBWorK/Authen.pm
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,10 @@ sub call_next_authen_method {
my $c = $self->{c};
my $ce = $c->{ce};

my $user_authen_module = WeBWorK::Authen::class($ce, "user_module");
my $user_authen_module =
$ce->{courseName} eq $ce->{admin_course_id}
? WeBWorK::Authen::class($ce, "admin_module")
: WeBWorK::Authen::class($ce, "user_module");
if (!defined $user_authen_module || $user_authen_module eq '') {
$self->{error} = $c->maketext(
"No authentication method found for your request. If this recurs, please speak with your instructor.");
Expand All @@ -155,18 +158,6 @@ sub verify {
debug('BEGIN VERIFY');

return $self->call_next_authen_method if !$self->request_has_data_for_this_verification_module;
my $authen_ref = ref($self);
if ($c->ce->{courseName} eq $c->ce->{admin_course_id}
&& !(grep {/^$authen_ref$/} @{ $c->ce->{authen}{admin_module} }))
{
$self->write_log_entry("Cannot authenticate into admin course using $authen_ref.");
$c->stash(
authen_error => $c->maketext(
'There was an error during the login process. Please speak to your instructor or system administrator.'
)
);
return $self->call_next_authen_method();
}

$self->{was_verified} = $self->do_verify;

Expand Down

0 comments on commit 7e8035e

Please sign in to comment.