From 86525bf30295b7e5b5bff929694d435a0dc98960 Mon Sep 17 00:00:00 2001
From: Jaimos Skriletz <jaimosskriletz@gmail.com>
Date: Tue, 6 Aug 2024 15:01:40 -0600
Subject: [PATCH] Don't require $authen{admin_module} is a subset of
 $authen{user_module}.

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.
---
 conf/localOverrides.conf.dist | 13 ++++++-------
 lib/WeBWorK/Authen.pm         | 17 ++++-------------
 2 files changed, 10 insertions(+), 20 deletions(-)

diff --git a/conf/localOverrides.conf.dist b/conf/localOverrides.conf.dist
index 6ea88b25cf..5c5e1796ff 100644
--- a/conf/localOverrides.conf.dist
+++ b/conf/localOverrides.conf.dist
@@ -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.
diff --git a/lib/WeBWorK/Authen.pm b/lib/WeBWorK/Authen.pm
index c5de75dd0e..002e5753c3 100644
--- a/lib/WeBWorK/Authen.pm
+++ b/lib/WeBWorK/Authen.pm
@@ -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.");
@@ -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;