Skip to content

Commit

Permalink
Make the admin_module separate from the user_module for authentic…
Browse files Browse the repository at this point in the history
…ation.

When signing in to the admin course the course `$authen{admin_module}`
is used, and for all other courses the `$auten{user_module}` is used.
So the two lists are completely separate.  Neither needs to be a subset
of the other.

Also fix the `net_ldap_options` course environment variable usage.  It
is `net_ldap_options` in the `authen_ldap.conf` file, but was
`net_ldap_opts` in the code.  Furthermore, the variable was dereferenced
as an array, but is actually a hash which causes an error.
  • Loading branch information
drgrice1 committed Aug 7, 2024
1 parent 86525bf commit f6eecaa
Show file tree
Hide file tree
Showing 8 changed files with 30 additions and 48 deletions.
2 changes: 1 addition & 1 deletion conf/authen_CAS.conf.dist
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ $authen{user_module} = {
};

# 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.
Expand Down
13 changes: 2 additions & 11 deletions conf/authen_LTI.conf.dist
Original file line number Diff line number Diff line change
Expand Up @@ -46,19 +46,10 @@ $authen{user_module} = [
];

# 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.
# have available to you.
$authen{admin_module} = [
#'WeBWorK::Authen::LTIAdvantage',
#'WeBWorK::Authen::LTIAdvanced',
Expand Down
22 changes: 10 additions & 12 deletions conf/authen_ldap.conf.dist
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,10 @@
########################################################################################

# Set LDAP as the authentication module to use.
$authen{user_module} = {
"*" => "WeBWorK::Authen::LDAP",
};
$authen{user_module} = { "*" => "WeBWorK::Authen::LDAP" };

# 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.
Expand Down Expand Up @@ -41,23 +39,23 @@ $authen{ldap_options} = {
# Edit the data below:
net_ldap_base => "ou=people,dc=myschool,dc=edu",

# Use a Bind account if set to 1
bindAccount => 0,
# Use a Bind account if set to 1
bindAccount => 0,

searchDN => "cn=search,DC=youredu,DC=edu",
bindPassword => "password",
searchDN => "cn=search,DC=youredu,DC=edu",
bindPassword => "password",

# The LDAP module searches for a DN whose RDN matches the username
# entered by the user. The net_ldap_rdn setting tells the LDAP
# backend what part of your LDAP schema you want to use as the RDN.
# The correct value for net_ldap_rdn will depend on your LDAP setup.
#

# Uncomment this line if you use Active Directory.
#net_ldap_rdn => "sAMAccountName",
#

# Uncomment this line if your schema uses uid as an RDN.
#net_ldap_rdn => "uid",
#

# By default, net_ldap_rdn is set to "sAMAccountName".

# If failover = "all", then all LDAP failures will be checked
Expand All @@ -69,4 +67,4 @@ $authen{ldap_options} = {
failover => "all",
};

1; #final line of the file to reassure perl that it was read properly.
1; #final line of the file to reassure perl that it was read properly.
3 changes: 1 addition & 2 deletions conf/defaults.config
Original file line number Diff line number Diff line change
Expand Up @@ -745,8 +745,7 @@ $authen{user_module} = {"*" => "WeBWorK::Authen::Basic_TheLastOption"};
$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} when logging into the admin course.
$authen{admin_module} = ['WeBWorK::Authen::Basic_TheLastOption'];

################################################################################
Expand Down
6 changes: 2 additions & 4 deletions conf/localOverrides.conf.dist
Original file line number Diff line number Diff line change
Expand Up @@ -475,9 +475,8 @@ $mail{feedbackRecipients} = [
# responds affirmatively will be used.

#$authen{user_module} = {
# sql_moodle => "WeBWorK::Authen::Moodle",
# sql_ldap => "WeBWorK::Authen::LDAP"
# "*" => "WeBWorK::Authen::Basic_TheLastOption"
# "*" => "WeBWorK::Authen::LDAP"
# "*" => "WeBWorK::Authen::Basic_TheLastOption"
#};

# Select the authentication module to use for proctor logins.
Expand All @@ -502,7 +501,6 @@ $mail{feedbackRecipients} = [
# those may override the setting of $authen{admin_module} here.

#$authen{admin_module} = [
# 'WeBWorK::Authen::Moodle',
# 'WeBWorK::Authen::LDAP',
# 'WeBWorK::Authen::Basic_TheLastOption'
#];
Expand Down
5 changes: 3 additions & 2 deletions lib/WeBWorK.pm
Original file line number Diff line number Diff line change
Expand Up @@ -153,11 +153,12 @@ async sub dispatch ($c) {
my $authz = WeBWorK::Authz->new($c);
$c->authz($authz);
my $user_authen_module = WeBWorK::Authen::class($ce, 'user_module');
my $user_authen_module =
WeBWorK::Authen::class($ce, $ce->{courseName} eq $ce->{admin_course_id} ? 'admin_module' : 'user_module');
runtime_use $user_authen_module;
my $authen = $user_authen_module->new($c);
debug("Using user_authen_module $user_authen_module: $authen\n");
debug("Using authentication module $user_authen_module: $authen\n");
$c->authen($authen);
if ($routeCaptures{courseID}) {
Expand Down
5 changes: 2 additions & 3 deletions lib/WeBWorK/Authen.pm
Original file line number Diff line number Diff line change
Expand Up @@ -131,9 +131,8 @@ sub call_next_authen_method {
my $ce = $c->{ce};

my $user_authen_module =
$ce->{courseName} eq $ce->{admin_course_id}
? WeBWorK::Authen::class($ce, "admin_module")
: WeBWorK::Authen::class($ce, "user_module");
WeBWorK::Authen::class($ce, $ce->{courseName} eq $ce->{admin_course_id} ? 'admin_module' : '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 Down
22 changes: 9 additions & 13 deletions lib/WeBWorK/Authen/LDAP.pm
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,9 @@ use base qw/WeBWorK::Authen/;

use strict;
use warnings;
use WeBWorK::Debug;
use Net::LDAP qw/LDAP_INVALID_CREDENTIALS/;

use WeBWorK::Debug qw(debug);
use Net::LDAP qw(LDAP_INVALID_CREDENTIALS);

sub checkPassword {
my ($self, $userID, $possibleClearPassword) = @_;
Expand All @@ -32,10 +33,8 @@ sub checkPassword {
my $ret = $self->ldap_authen_uid($userID, $possibleClearPassword);
return 1 if ($ret == 1);

#return 0 if ($userID !~ /admin/);

# optional: fail over to superclass checkPassword
if (($failover eq "all" or $failover eq "1") || ($failover eq "local" and $ret < 0)) {
if ($failover eq "all" || $failover eq "1" || ($failover eq "local" && $ret < 0)) {
$self->write_log_entry("AUTH LDAP: authentication failed, deferring to superclass");
return $self->SUPER::checkPassword($userID, $possibleClearPassword);
}
Expand All @@ -48,20 +47,17 @@ sub ldap_authen_uid {
my ($self, $uid, $password) = @_;
my $ce = $self->{c}->ce;
my $hosts = $ce->{authen}{ldap_options}{net_ldap_hosts};
my $opts = $ce->{authen}{ldap_options}{net_ldap_opts};
my $opts = $ce->{authen}{ldap_options}{net_ldap_options} // {};
my $base = $ce->{authen}{ldap_options}{net_ldap_base};
my $searchdn = $ce->{authen}{ldap_options}{searchDN};
my $bindAccount = $ce->{authen}{ldap_options}{bindAccount};
my $bindpassword = $ce->{authen}{ldap_options}{bindPassword};
# Be backwards-compatible with releases that hardcode this value.
my $rdn = "sAMAccountName";
if (defined $ce->{authen}{ldap_options}{net_ldap_rdn}) {
$rdn = $ce->{authen}{ldap_options}{net_ldap_rdn};
}
my $rdn = $ce->{authen}{ldap_options}{net_ldap_rdn} // 'sAMAccountName';

# connect to LDAP server
my $ldap = new Net::LDAP($hosts, @$opts);
if (not defined $ldap) {
my $ldap = Net::LDAP->new($hosts, ref($opts) eq 'HASH' ? %$opts : ());
if (!defined $ldap) {
warn "AUTH LDAP: couldn't connect to any of ", join(", ", @$hosts), ".\n";
return 0;
}
Expand Down Expand Up @@ -100,7 +96,7 @@ sub ldap_authen_uid {
return -1;
}
my $dn = $msg->shift_entry->dn;
if (not defined $dn) {
if (!defined $dn) {
warn "AUTH LDAP: got null DN when looking up UID '$uid'.\n";
return 0;
}
Expand Down

0 comments on commit f6eecaa

Please sign in to comment.