From 8d668d1349733e51fbfbd65d2ab474d70c967997 Mon Sep 17 00:00:00 2001 From: Glenn Rice Date: Thu, 13 Aug 2026 17:55:27 -0500 Subject: [PATCH] Clean up the `lib/WeBWorK/Authz.pm` file. This is primarily just code clean up, but there are a few of notable changes to the code. First, the `setCachedUser` method no longer attempts to look up the database permission record twice in the event that an exception occurs on the first lookup. That was rather stupid code. If an exception was thrown the first time, it is most likely going to be thrown again the second time. I think the reason this was done was because it used to be the case that when the database `getPermissionLevel` method was called and a record in the database did not exist, the record would be created. However, that is no longer the case, and even if it were that would not throw an exception and would return the permission record on the first call. So just plain stupid code here. Second, the `setCachedUser` method no longer refuses to cache the user if it detects that the request is a LTI 1.1 authentication request. That should not have been done, and results in extra database calls for any LTI 1.1 authentication request. In addition the method does not croak if the `oauth_nonce` parameter is detected and the `lis_person_sourced_id` parameter (or one of its misspellings) or the `lis_person_contact_email_primary` parameter is not present. Third, the `hasPermissions` method no longer returns 1 for a user that does not have a permission level defined in the database if the `lis_person_sourcedid` parameter is present. It just returns 0 as it does for any other case in which the permission level is not defined in the database. This is actually a potential security vulnerability. If the LTIAdvantage code is not set up right, this could result in a user without sufficient permissions gaining access to things the user does not have permission for. --- lib/WeBWorK/Authz.pm | 466 ++++++++++++------------------------------- 1 file changed, 129 insertions(+), 337 deletions(-) diff --git a/lib/WeBWorK/Authz.pm b/lib/WeBWorK/Authz.pm index e5587f4e48..98b32f79bd 100644 --- a/lib/WeBWorK/Authz.pm +++ b/lib/WeBWorK/Authz.pm @@ -1,4 +1,5 @@ package WeBWorK::Authz; +use Mojo::Base -signatures; =head1 NAME @@ -6,33 +7,36 @@ WeBWorK::Authz - check user permissions. =head1 SYNOPSIS - # create new authorizer -- $c is a WeBWorK::Controller object. - my $authz = new WeBWorK::Authz($c); +Create a new authorizer object. A WeBWorK::Controller object must be provided. - # tell authorizer to cache permission level of user spammy. - $authz->setCachedUser("spammy"); + my $authz = new WeBWorK::Authz($c); - # this call will use the cached data. - if ($authz->hasPermissions("spammy", "eat_breakfast")) { - eat_breakfast(); - } +Cache the permission level for the user spammy. - # this call will not use the cached data, and will cause a database lookup. - if ($authz->hasPermissions("hammy", "go_to_bed")) { - go_to_bed(); - } + $authz->setCachedUser('spammy'); + +This call will use the cached data. + + if ($authz->hasPermissions('spammy', 'eat_breakfast')) { + eat_breakfast(); + } + +This call will not use the cached data, and will cause a database lookup. + + if ($authz->hasPermissions('hammy', 'go_to_bed')) { + go_to_bed(); + } =head1 DESCRIPTION -WeBWorK::Authen determines if a user is authorized to perform a specific +C determines if a user is authorized to perform a specific activity, based on the user's PermissionLevel record in the WeBWorK database and -the contents of the %permissionLevels hash in the course environment. - -=head2 Format of the %permissionLevels hash +the contents of the C<%permissionLevels> hash in the course environment. -%permissionLevels maps text strings describing activities to numeric permission -levels. The definitive list of activities is contained in the default version of -%permissionLevels, in the file F. +The C<%permissionLevels> hash in the course environment maps text strings +describing activities to numeric permission levels. The definitive list of +activities is contained in the default version of the C<%permissionLevels> hash +defined in the file F. A user is able to engage in an activity if their permission level is greater than or equal to the level associated with the activity. If the level associated @@ -41,63 +45,41 @@ activity, regardless of their permission level. =cut -use strict; -use warnings; -use version; - -use Carp qw/croak/; +use Net::IP; +use Scalar::Util qw(weaken); use WeBWorK::Utils::DateTime qw(before); use WeBWorK::Utils::Sets qw(restricted_set_message); use WeBWorK::Authen::Proctor; -use Net::IP; -use Scalar::Util qw(weaken); - -################################################################################ =head1 CONSTRUCTOR -=over +Usage: C<< WeBWorK::Authz->new($c) >> -=item WeBWorK::Authz->new($c) - -Creates a new authorizer instance. $c is a WeBWorK::Controller object. It must -already have its C and C fields set. +Creates a new authorizer instance. C<$c> is a C object. It +must already have its C and C fields set. =cut -sub new { - my ($invocant, $c) = @_; - my $class = ref($invocant) || $invocant; - my $self = { c => $c, }; +sub new ($invocant, $c) { + my $self = bless { c => $c }, ref($invocant) || $invocant; weaken $self->{c}; - - $c->{permission_retrieval_error} = 0; - bless $self, $class; return $self; } -=back - -=cut - -################################################################################ - =head1 METHODS -=over +=head2 setCachedUser -=item setCachedUser($userID) +Usage: C<< $authz->setCachedUser($userID) >> -Caches the PermissionLevel of the user $userID in an existing authorizer. If a -user's PermissionLevel is cached, it will be used whenever hasPermissions() is -called on the same user. Only one user can be cached at a time. This is used by -WeBWorK to cache the "real" user. +Caches the PermissionLevel of the user C<$userID> in an existing authorizer. If +a user's PermissionLevel is cached, it will be used whenever C +is called on the same user. Only one user can be cached at a time. =cut -sub setCachedUser { - my ($self, $userID) = @_; +sub setCachedUser ($self, $userID) { my $c = $self->{c}; my $db = $c->db; @@ -106,250 +88,103 @@ sub setCachedUser { if (defined $userID) { $self->{userID} = $userID; - if (!$db->existsUser($userID) && defined($c->param("lis_person_sourcedid"))) { - # This is a new user referred via an LTI link. - # Do not attempt to cache the permission here. - # Rather, the LTI authentication module should cache the permission. - return 1; - } - my $PermissionLevel; - my $tryAgain = 1; - my $count = 0; - while ($tryAgain && $count < 2) { - eval { - $PermissionLevel = $db->getPermissionLevel($userID); # checked - }; - if ($@) { - $count++; - } else { - $tryAgain = 0; - } - } - if (defined $PermissionLevel - and defined $PermissionLevel->permission - and $PermissionLevel->permission ne "") - { - # cache the permission level record in this request to avoid later database calls - $self->{PermissionLevel} = $PermissionLevel; - } elsif (defined($c->param("lis_person_sourcedid")) - or defined($c->param("lis_person_sourced_id")) - or defined($c->param("lis_person_source_id")) - or defined($c->param("lis_person_sourceid")) - or defined($c->param("lis_person_contact_email_primary"))) - { - # This is a new user referred via an LTI link. - # Do not attempt to cache the permission here. - # Rather, the LTI authentication module should cache the permission. - return 1; - } elsif (defined($c->param("oauth_nonce"))) { - # This is a LTI attempt that doesn't have an lis_person_sourcedid username. - croak( - "Your request did not specify your username. Perhaps you were attempting to authenticate via LTI but the LTI tool did not transmit " - . "any variant of the lis_person_sourced_id parameter and did not transmit the lis_person_contact_email_primary parameter." - ); - - } else { - if ($c->{permission_retrieval_error} == 0) { - $c->{permission_retrieval_error} = 1; - croak "Unable to retrieve your permissions, perhaps due to a collision " - . "between your request and that of another user " - . "(or possibly an unfinished request of yours). " - . "Please press the BACK button on your browser and try again."; - } - } - } else { - warn "setCachedUser() called with userID undefined."; + my $permissionLevel = $db->getPermissionLevel($userID); + $self->{PermissionLevel} = $permissionLevel + if defined $permissionLevel && defined $permissionLevel->permission && $permissionLevel->permission ne ''; } return; } -=item hasPermissions($userID, $activity) +=head2 hasPermissions + +Usage: C<< $authz->hasPermissions($userID, $activity, $exactness) >> -Checks the %permissionLevels hash in the course environment to determine if the -user $userID has permission to engage in the activity $activity. If the user's -permission level is greater than or equal to the level associated with $activity, -a true value is returned. Otherwise, a false value is returned. +Checks the C<%permissionLevels> hash in the course environment to determine if +the user C<$userID> has permission to engage in the activity C<$activity>. If +the user's permission level is greater than or equal to the level associated +with C<$activity>, a true value is returned. Otherwise, a false value is +returned. -If $userID has been cached using the setCachedUser() call, the cached data is -used. Otherwise, the user's PermissionLevel is looked up in the WeBWorK -database. +If C<$userID> has been cached using the C call, the cached data +is used. Otherwise, the user's permission level is looked up in the database. If the user does not have a PermissionLevel record, the permission level record -is empty, or the activity does not appear in %permissionLevels, hasPermissions() -assumes that the user does not have permission. +is empty, or the activity does not appear in C<%permissionLevels>, +C assumes that the user does not have permission. + +If the optional C<$exactness> argument is provided it must be S<'equal'>, and in +that case C will only return true if the user's permission level +is equal to the level associated with C<$activity>. =cut -# This currently only uses two of it's arguments, but it accepts any number, in -# case in the future calculating certain permissions requires more information. -sub hasPermissions { - my ($self, $userID, $activity, $exactness) = @_; - croak 'hasPermissions called with the incorrect number of arguments' unless defined $activity; - croak 'hasPermissions called with an incorrect exactness argument' if defined $exactness && $exactness ne 'equal'; +sub hasPermissions ($self, $userID, $activity, $exactness = undef) { + $exactness //= 'ge'; - if (!defined($exactness)) { $exactness = 'ge'; } my $c = $self->{c}; my $ce = $c->ce; my $db = $c->db; - # this may need to be changed if we get other permission level data sources - return 0 unless defined $db; + return 0 unless defined $db && defined $userID && $userID ne ''; - # this may need to be changed if we want to control what unauthenticated users - # can do with the permissions system - return 0 unless defined $userID and $userID ne ""; + my $permissionLevelRecord; - my $PermissionLevel; - - if (not defined($self->{userID})) { - #warn "self->{userID} is undefined"; - $self->setCachedUser($userID); - } + $self->setCachedUser($userID) unless defined $self->{userID}; my $cachedUserID = $self->{userID}; - if (defined $cachedUserID and $cachedUserID ne "" and $cachedUserID eq $userID) { - # this is the same user -- we can skip the database call - $PermissionLevel = $self->{PermissionLevel}; + if (defined $cachedUserID && $cachedUserID ne '' && $cachedUserID eq $userID) { + $permissionLevelRecord = $self->{PermissionLevel}; } else { - # a different user, or no user was defined before - #my $prettyCachedUserID = defined $cachedUserID ? "'$cachedUserID'" : "undefined"; - #warn "hasPermissions called with user $userID , but cached user is $prettyCachedUserID. Accessing database.\n"; - $PermissionLevel = $db->getPermissionLevel($userID); # checked + $permissionLevelRecord = $db->getPermissionLevel($userID); } - my $permission_level; + return 0 unless defined $permissionLevelRecord; - if (defined $PermissionLevel) { - $permission_level = $PermissionLevel->permission; - } elsif (defined($c->param("lis_person_sourcedid"))) { - # This is an LTI login. Let's see if the LITBasic authentication module will handle this. - #return 1; - } else { - # uh, oh. this user has no permission level record! - if ($c->{permission_retrieval_error} != 1) { - warn "User '$userID' has no PermissionLevel record -- assuming no permission."; - } - return 0; - } - - unless (defined $permission_level and $permission_level ne "") { - warn "User '$userID' has empty permission level -- assuming no permission."; - return 0; - } + my $permissionLevel = $permissionLevelRecord->permission; + return 0 unless defined $permissionLevel && $permissionLevel ne ''; my $userRoles = $ce->{userRoles}; my $permissionLevels = $ce->{permissionLevels}; if (exists $permissionLevels->{$activity}) { - my $activity_role = $permissionLevels->{$activity}; - if (defined $activity_role) { - if (exists $userRoles->{$activity_role}) { - my $role_permlevel = $userRoles->{$activity_role}; - # Elevate all permissions greater than a student in the admin course to the - # create_and_delete_courses level. This way a user either has access to all - # or only student level permissions tools in the admin course. - if (defined($ce->{courseName}) && $ce->{courseName} eq $ce->{admin_course_id}) { - my $admin_permlevel = $userRoles->{ $permissionLevels->{create_and_delete_courses} }; - $role_permlevel = $admin_permlevel - if $role_permlevel > $userRoles->{student} && $role_permlevel < $admin_permlevel; - } - if (defined $role_permlevel) { - if ($exactness eq 'ge') { - return $permission_level >= $role_permlevel; - } elsif ($exactness eq 'equal') { - return $permission_level == $role_permlevel; - } else { - return 0; - } + my $activityRole = $permissionLevels->{$activity}; + if (defined $activityRole && exists $userRoles->{$activityRole}) { + my $rolePermissionLevel = $userRoles->{$activityRole}; + # Elevate all permissions greater than a student in the admin course to the + # create_and_delete_courses level. This way a user either has access to all + # or only student level permissions tools in the admin course. + if (defined $ce->{courseName} && $ce->{courseName} eq $ce->{admin_course_id}) { + my $admin_permlevel = $userRoles->{ $permissionLevels->{create_and_delete_courses} }; + $rolePermissionLevel = $admin_permlevel + if $rolePermissionLevel > $userRoles->{student} && $rolePermissionLevel < $admin_permlevel; + } + if (defined $rolePermissionLevel) { + if ($exactness eq 'ge') { + return $permissionLevel >= $rolePermissionLevel; + } elsif ($exactness eq 'equal') { + return $permissionLevel == $rolePermissionLevel; } else { - # warn "Role '$activity_role' has undefined permission level -- assuming no permission."; return 0; } } else { - # warn "Role '$activity_role' for activity '$activity' not found in \%userRoles -- assuming no permission."; return 0; } } else { - # warn "Undefined Role, -- assuming no one has permission to perform $activity."; - return 0; # undefined $activity_role, no one has permission to perform $activity + return 0; } } else { - # warn "Activity '$activity' not found in \%permissionLevels -- assuming no permission."; return 0; } } -######################### IU Addition ############### -sub hasExactPermissions { - my ($self, $userID, $activity) = @_; +# Set level authorization routines. + +sub checkSet ($self) { my $c = $self->{c}; my $ce = $c->ce; my $db = $c->db; - # my $Permission = $db->getPermissionLevel($user); # checked - # return 0 unless defined $Permission; - # my $permissionLevel = $Permission->permission(); - -## - my $PermissionLevel; - - if (not defined($self->{userID})) { - #warn "self->{userID} is undefined"; - $self->setCachedUser($userID); - } - - my $cachedUserID = $self->{userID}; - if (defined $cachedUserID and $cachedUserID ne "" and $cachedUserID eq $userID) { - # this is the same user -- we can skip the database call - $PermissionLevel = $self->{PermissionLevel}; - } else { - # a different user, or no user was defined before - #my $prettyCachedUserID = defined $cachedUserID ? "'$cachedUserID'" : "undefined"; - #warn "hasPermissions called with user $userID , but cached user is $prettyCachedUserID. Accessing database.\n"; - $PermissionLevel = $db->getPermissionLevel($userID); # checked - } - - my $permission_level; - - if (defined $PermissionLevel) { - $permission_level = $PermissionLevel->permission; - } else { - # uh, oh. this user has no permission level record! - if ($c->{permission_retrieval_error} != 1) { - warn "User '$userID' has no PermissionLevel record -- assuming no permission."; - } - return 0; - } - - unless (defined $permission_level and $permission_level ne "") { - warn "User '$userID' has empty permission level -- assuming no permission."; - return 0; - } - -## - - my $permissionLevels = $ce->{permissionLevels}; - if (exists $permissionLevels->{$activity}) { - if (defined $permissionLevels->{$activity}) { - return $permission_level == $permissionLevels->{$activity}; - } else { - return 0; - } - } else { - die "Activity '$activity' not found in %permissionLevels. Can't continue.\n"; - } -} -####################################################### - -#### set-level authorization routines - -sub checkSet { - my $self = shift; - my $c = $self->{c}; - my $ce = $c->ce; - my $db = $c->db; - my $node_name = $c->current_route; # First check to see if we have to worried about set-level access restrictions. @@ -357,8 +192,8 @@ sub checkSet { # To check set restrictions we need a set and a user. my $setName = $c->stash('setID'); - my $userName = $c->param("user"); - my $effectiveUserName = $c->param("effectiveUser"); + my $userName = $c->param('user'); + my $effectiveUserName = $c->param('effectiveUser'); # If there is no input userName, then the content generator will be forcing a login, so just bail. return 0 if (!$userName || !$effectiveUserName); @@ -372,24 +207,24 @@ sub checkSet { if ($set && $set->set_id eq $setName && $set->user_id eq $effectiveUserName && $set->version_id eq $verNum) { # If we have all of this, then we can just use this set and skip the rest. - } elsif ($setName eq 'Undefined_Set' && $self->hasPermissions($userName, "access_instructor_tools")) { + } elsif ($setName eq 'Undefined_Set' && $self->hasPermissions($userName, 'access_instructor_tools')) { # This is the case of previewing a problem from a 'try it' link. return 0; } else { if ($db->existsSetVersion($effectiveUserName, $setName, $verNum)) { $set = $db->getMergedSetVersion($effectiveUserName, $setName, $verNum); } else { - return $c->maketext("Requested version ([_1]) of set '[_2]' is not assigned to user [_3].", + return $c->maketext('Requested version ([_1]) of set "[_2]" is not assigned to user [_3].', $verNum, $setName, $effectiveUserName); } } if (!$set) { - return $c->maketext("Requested set '[_1]' could not be found in the database for user [_2].", + return $c->maketext('Requested set "[_1]" could not be found in the database for user [_2].', $setName, $effectiveUserName); } # Don't allow versioned sets to be viewed from the problem-list page. if ($node_name eq 'problem_list') { - return $c->maketext("Requested version ([_1]) of set '[_2]' cannot be directly accessed.", $verNum, + return $c->maketext('Requested version ([_1]) of set "[_2]" cannot be directly accessed.', $verNum, $setName); } } else { @@ -398,16 +233,16 @@ sub checkSet { } else { if ($db->existsUserSet($effectiveUserName, $setName)) { $set = $db->getMergedSet($effectiveUserName, $setName); - } elsif ($setName eq 'Undefined_Set' && $self->hasPermissions($userName, "access_instructor_tools")) { + } elsif ($setName eq 'Undefined_Set' && $self->hasPermissions($userName, 'access_instructor_tools')) { # This is the case of the library browser, when we don't actually have a set to look at. This only # happens for instructor tool users. return 0; } else { - return $c->maketext("Requested set '[_1]' is not assigned to user [_2].", $setName, $effectiveUserName); + return $c->maketext('Requested set "[_1]" is not assigned to user [_2].', $setName, $effectiveUserName); } } if (!$set) { - return $c->maketext("Requested set '[_1]' could not be found in the database for user [_2].", + return $c->maketext('Requested set "[_1]" could not be found in the database for user [_2].', $setName, $effectiveUserName); } } @@ -415,7 +250,7 @@ sub checkSet { $self->{merged_set} = $set; # Save restricted set messages to show to instructors if they exist. - my $canViewUnopened = $self->hasPermissions($userName, "view_unopened_sets"); + my $canViewUnopened = $self->hasPermissions($userName, 'view_unopened_sets'); my @restrictedSetMessages; # Now we know that the set is assigned to the appropriate user. @@ -423,15 +258,15 @@ sub checkSet { # Check to make sure that the set is visible, and that the user is allowed to view hidden sets. my $visible = $set && $set->visible ne '0' && $set->visible ne '1' ? 1 : $set->visible; - if (!$visible && !$self->hasPermissions($userName, "view_hidden_sets")) { + if (!$visible && !$self->hasPermissions($userName, 'view_hidden_sets')) { $c->{viewSetCheck} = 'hidden'; - return $c->maketext("Requested set '[_1]' is not available.", $setName); + return $c->maketext('Requested set "[_1]" is not available.', $setName); } # Check to see if the user is trying to access a set that is not open. if (before($set->open_date) && !$canViewUnopened) { $c->{viewSetCheck} = 'not-open'; - return $c->maketext("Requested set '[_1]' is not available yet.", $setName); + return $c->maketext('Requested set "[_1]" is not available yet.', $setName); } # Check to see if conditional release conditions have been met. @@ -446,15 +281,15 @@ sub checkSet { } # Check to be sure that gateways are being sent to the correct content generator. - if (defined($set->assignment_type) && $set->assignment_type =~ /gateway/ && $node_name eq 'problem_detail') { + if (defined $set->assignment_type && $set->assignment_type =~ /gateway/ && $node_name eq 'problem_detail') { return $c->maketext( - "Requested set '[_1]' is a test but the regular homework assignment content " + 'Requested set "[_1]" is a test but the regular homework assignment content ' . 'generator [_2] was called. Try re-entering the set from the problem sets listing page.', $setName, $node_name ); - } elsif ((!defined($set->assignment_type) || $set->assignment_type eq 'default') && $node_name =~ /gateway/) { + } elsif ((!defined $set->assignment_type || $set->assignment_type eq 'default') && $node_name =~ /gateway/) { return $c->maketext( - "Requested set '[_1]' is a homework assignment but the test content generator [_2] was called. " + 'Requested set "[_1]" is a homework assignment but the test content generator [_2] was called. ' . 'Try re-entering the set from the problem sets listing page.', $setName, $node_name ); @@ -463,10 +298,10 @@ sub checkSet { # Check if the user is entering a proctored assignment that the proctor has authenticated. This is necessary to # make sure that someone doesn't use the unproctored url path to obtain access to a proctored assignment. # Allow ProblemSet.pm to list the proctored quiz versions. - if (defined($set->assignment_type) + if (defined $set->assignment_type && $set->assignment_type =~ /proctored/ && $node_name ne 'problem_list' - && !WeBWorK::Authen::Proctor->new($c, $ce, $db)->verify()) + && !WeBWorK::Authen::Proctor->new($c, $ce, $db)->verify) { return $c->maketext( 'Requested set "[_1]" is a proctored test, but no valid proctor authorization has been obtained.', @@ -499,111 +334,68 @@ sub checkSet { return 0; } -sub invalidIPAddress { - # this exists as a separate routine because we need to check multiple - # sets in Hardcopy; having this routine to check the set allows us to do - # that for all sets individually there. +sub invalidIPAddress ($self, $set) { + return 0 if !defined $set->restrict_ip || $set->restrict_ip eq '' || $set->restrict_ip eq 'No'; - my $self = shift; - my $set = shift; + my $c = $self->{c}; + my $db = $c->db; - my $c = $self->{c}; - my $db = $c->db; - my $ce = $c->ce; - my $setName = $set->set_id; - my $userName = $c->param("user"); - my $effectiveUserName = $c->param("effectiveUser"); + # Make sure that the non-versioned set name is used. + my $setName = $set->set_id =~ s/,v\d+$//r; - return 0 - if (!defined($set->restrict_ip) - || $set->restrict_ip eq '' - || $set->restrict_ip eq 'No'); + my $restrictType = $set->restrict_ip; + my @restrictAddresses = map { $db->listLocationAddresses($_) } + map { $_->location_id } $db->getAllMergedSetLocations($c->param('effectiveUser'), $setName); my $clientIP = Net::IP->new($c->tx->remote_address); - # make sure that we're using the non-versioned set name - $setName =~ s/,v\d+$//; - - my $restrictType = $set->restrict_ip; - my @restrictLocations = $db->getAllMergedSetLocations($effectiveUserName, $setName); - my @locationIDs = (map { $_->location_id } @restrictLocations); - my @restrictAddresses = (map { $db->listLocationAddresses($_) } @locationIDs); - - # if there are no addresses in the locations, return an error that - # says this + # If there are no addresses in the locations, return an error. return $c->maketext( 'Client ip address [_1] is not allowed to work this assignment, because the assignment has ip address ' . 'restrictions and there are no allowed locations associated with the restriction. Contact your ' . 'professor to have this problem resolved.', - $clientIP->ip() - ) if (!@restrictAddresses); - - # build a set of IP objects to match against - my @restrictIPs = (map { Net::IP->new($_) } @restrictAddresses); + $clientIP->ip + ) unless @restrictAddresses; - # and check the clientAddress against these: is $clientIP - # in @restrictIPs? + # Check to see if the clientAddress is a restricted IP address. my $inRestrict = 0; - foreach my $rIP (@restrictIPs) { - if ($rIP->overlaps($clientIP) == $IP_B_IN_A_OVERLAP - || $rIP->overlaps($clientIP) == $IP_IDENTICAL) - { - $inRestrict = $rIP->ip(); + for my $rIP (map { Net::IP->new($_) } @restrictAddresses) { + if ($rIP->overlaps($clientIP) == $IP_B_IN_A_OVERLAP || $rIP->overlaps($clientIP) == $IP_IDENTICAL) { + $inRestrict = $rIP->ip; last; } } - # this is slightly complicated by having to check relax_restrict_ip + # This is slightly complicated by having to check relax_restrict_ip. my $badIP = ''; if ($restrictType eq 'RestrictTo' && !$inRestrict) { $badIP = $c->maketext( 'Client ip address [_1] is not in the list of addresses from which this assignment may be worked.', - $clientIP->ip()); + $clientIP->ip); } elsif ($restrictType eq 'DenyFrom' && $inRestrict) { $badIP = $c->maketext( 'Client ip address [_1] is in the list of addresses from which this assignment may not be worked.', - $clientIP->ip()); + $clientIP->ip); } else { return 0; } - # if we're here, we failed the IP check, and so need to consider - # if ip restrictions were relaxed. the set we were passed in - # is either the merged userset or the merged versioned userset, - # depending on whether the set is versioned or not - + # If this is reached, then the IP check failed. Now determine if ip restrictions were relaxed. my $relaxRestrict = $set->relax_restrict_ip; - return $badIP if ($relaxRestrict eq 'No'); + return $badIP if $relaxRestrict eq 'No'; if ($set->assignment_type =~ /gateway/) { if ($relaxRestrict eq 'AfterAnswerDate') { - # in this case we need to go and get the userset, - # not the versioned set (which we already have) - # drat! my $userset = $db->getMergedSet($set->user_id, $setName); return (!$userset || before($userset->answer_date) ? $badIP : 0); } else { - # this is easier; just look at the current answer date return (before($set->answer_date) ? $badIP : 0); } } else { - # the set isn't versioned, so assume that $relaxRestrict - # is 'AfterAnswerDate', regardless of what it actually - # is; 'AfterVersionAnswerDate' doesn't make sense in - # this case - return (before($set->answer_date) ? $badIP : 0); + # The set isn't versioned, so assume that $relaxRestrict is 'AfterAnswerDate', regardless of what it actually + # is. 'AfterVersionAnswerDate' doesn't make sense in this case. + return before($set->answer_date) ? $badIP : 0; } } -=back - -=cut - -=head1 AUTHOR - -Written by Dennis Lambe, malsyned at math.rochester.edu. Modified by Sam -Hathaway, sh002i at math.rochester.edu. - -=cut - 1;