Checkpoint before final conversion.

Added getIpUsers method to Group.pm
Group->getUsers and User->isInGroup both use getIpUsers
Tests added to Group.t to verify that getIpUsers works.

lib/WebGUI/Group.pm

@@ -487,7 +487,6 @@ sub getScratchUsers {
 	my $scratchFilter;
 	return [] unless $scratchFilter = $self->scratchFilter();
 
-	my $sessionId = $self->session->db->quote($self->session->getId());
 	my $time = $self->session->datetime->time();
 
 	$scratchFilter =~ s/\s//g;
@@ -511,6 +510,42 @@ EOQ
 	return $self->session->db->buildArrayRef($query, [ @scratchPlaceholders ]);
 }	
 
+#-------------------------------------------------------------------
+
+=head2 getIpUsers ( )
+
+Get the set of users allowed to be in this group via the lastIP recorded in
+the user's session and this group's IpFilter.  The set is returned as an array ref.
+
+If no IpFilter has been set for this group, returns an empty array ref.
+
+=cut
+
+sub getIpUsers {
+	my $self = shift;
+	my $IpFilter;
+	return [] unless $IpFilter = $self->ipFilter();
+
+	my $time = $self->session->datetime->time();
+
+	$IpFilter =~ s/\s//g;
+	my @filters = split /;/, $IpFilter;
+
+	my $query = "select userId,lastIP from userSession where expires > ?";
+
+	my $sth = $self->session->db->read($query, [ $self->session->datetime->time() ]);
+	my %localCache = ();
+	my @ipUsers = ();
+	while (my ($userId, $lastIP) = $sth->array() ) {
+		if (!exists $localCache{$lastIP}) {
+			$localCache{$lastIP} = isInSubnet($lastIP, \@filters);	
+		}
+		push @ipUsers, $userId if $localCache{$lastIP};
+	}
+	return \@ipUsers;
+}	
+
+
 #-------------------------------------------------------------------
 
 =head2 find ( session, name )
@@ -672,6 +707,7 @@ sub getUsers {
 				@{ $extGroup->getDatabaseUsers() },
 				@{ $extGroup->getKarmaUsers() },
 				@{ $extGroup->getScratchUsers() },
+				@{ $extGroup->getIpUsers() },
 			;
 		}
 	}
@@ -681,6 +717,7 @@ sub getUsers {
 		@{ $self->getDatabaseUsers() },
 		@{ $self->getKarmaUsers() },
 		@{ $self->getScratchUsers() },
+		@{ $self->getIpUsers() },
 	;
 	my @users = ( @localUsers, @externalUsers );
 	return \@users;

lib/WebGUI/User.pm

@@ -255,20 +255,11 @@ sub isInGroup {
         ### The following several checks are to increase performance. If this section were removed, everything would continue to work as normal. 
         return 1 if ($gid eq '7');		# everyone is in the everyone group
         return 1 if ($gid eq '1' && $uid eq '1'); 	# visitors are in the visitors group
+        return 1 if ($gid eq '2' && $uid ne '1'); 	# if you're not a visitor, then you're a registered user
+        return 1 if ($uid eq '3');  #Admin is in every group
         ### Get data for auxillary checks.
 	my $group = WebGUI::Group->new($self->session,$gid);
 	my $isInGroup = $self->session->stow->get("isInGroup");
-        ### Check IP Address
-        if ($group->get("ipFilter")) {
-		my $ipFilter = $group->get("ipFilter");
-                $ipFilter =~ s/\s+//g;
-                my @ips = split(",",$ipFilter);
-		my $ipMatch = WebGUI::Utility::isInSubnet($self->session->env->get("REMOTE_ADDR"), [ @ips ]);
-		return 1 if $ipMatch;
-        }
-        return 0 if ($uid eq '1');  #Visitor is in no other groups
-        return 1 if ($uid eq '3');  #Admin is in every group
-        return 1 if ($gid eq '2' && $uid ne '1'); 	# if you're not a visitor, then you're a registered user
         ### Look to see if we've already looked up this group. 
         if ($isInGroup->{$uid}{$gid} eq '1') {
                 return 1;
@@ -348,6 +339,13 @@ sub isInGroup {
 		$self->session->stow->set("isInGroup",$isInGroup);
 		return 1 if ($isInGroup->{$uid}{$gid});
 	}
+        if (my @ipUsers = @{ $group->getIpUsers() }) {
+		foreach my $extUserId ( @ipUsers ) {
+			$isInGroup->{$extUserId}{$gid} = 1;
+		}
+		$self->session->stow->set("isInGroup",$isInGroup);
+		return 1 if ($isInGroup->{$uid}{$gid});
+	}
         ### Check for groups of groups.
         my $groups = $group->getGroupsIn(1);
         foreach (@{$groups}) {

t/Group.t

@@ -50,7 +50,31 @@ my @scratchTests = (
 			},
 );
 
-plan tests => (85 + scalar(@scratchTests)); # increment this value for each test you create
+my @ipTests = (
+			{
+				ip => '192.168.0.2',
+				comment => 'good IP address',
+				expect  => 1,
+			},
+			{
+				ip => '10.0.0.2',
+				comment => 'bad IP address',
+				expect  => 0,
+			},
+			{
+				ip => '192.168.0.128',
+				comment => 'another good IP address',
+				expect  => 1,
+			},
+			{
+				ip => '172.17.10.20',
+				comment => 'another bad IP address',
+				expect  => 0,
+			},
+);
+
+
+plan tests => (87 + scalar(@scratchTests) + scalar(@ipTests)); # increment this value for each test you create
 
 my $session = WebGUI::Test->session;
 
@@ -397,6 +421,51 @@ cmp_bag(
 	'getUsers for group with scratch'
 );
 
+foreach my $subSession (@sessionBank) {
+	$subSession->db->write("DELETE FROM userSessionScratch WHERE sessionId=?",[ $subSession->getId]);
+}
+
+@sessionBank = ();
+my @tcps =  ();
+
+foreach my $idx (0..$#ipTests) {
+	##Set the ip to be used by the session for this user
+	my $ip = $ipTests[$idx]->{ip};
+	$ENV{REMOTE_ADDR} = $ip;
+	##Create a new session
+	$sessionBank[$idx] = WebGUI::Session->open(WebGUI::Test->root, WebGUI::Test->file);
+
+	##Create a new user and make this session's default user that user
+	$tcps[$idx] = WebGUI::User->new($sessionBank[$idx], "new");
+	$sessionBank[$idx]->user({user => $tcps[$idx]});
+
+	##Name this user for convenience
+	$tcps[$idx]->username("tcp$idx");
+
+	##Assign this user to this test to be fetched later
+	$ipTests[$idx]->{user} = $tcps[$idx];
+}
+
+my $gI = WebGUI::Group->new($session, "new");
+$gI->name('Group I');
+$gI->ipFilter('192.168.0.0/24');
+
+cmp_bag(
+	$gI->getIpUsers,
+	[ (map { $_->{user}->userId() }  grep { $_->{expect} } @ipTests) ],
+	'getIpUsers'
+);
+
+cmp_bag(
+	$gI->getUsers,
+	[ (map { $_->{user}->userId() }  grep { $_->{expect} } @ipTests) ],
+	'getUsers for group with IP filter'
+);
+
+foreach my $ipTest (@ipTests) {
+	is($ipTest->{user}->isInGroup($gI->getId), $ipTest->{expect}, $ipTest->{comment});
+}
+
 SKIP: {
 	skip("need to test expiration date in groupings interacting with recursive or not", 1);
 	ok(undef, "expiration date in groupings for getUser");
@@ -406,7 +475,7 @@ END {
 	foreach my $testGroup ($gX, $gY, $gZ, $gA, $gB, $gC, $g, $gK, $gS) {
 		$testGroup->delete if (defined $testGroup and ref $testGroup eq 'WebGUI::Group');
 	}
-	foreach my $dude ($user, @crowd, @mob, @chameleons, @itchies) {
+	foreach my $dude ($user, @crowd, @mob, @chameleons, @itchies, @tcps) {
 		$dude->delete if (defined $dude and ref $dude eq 'WebGUI::User');
 	}
 	$session->db->dbh->do('DROP TABLE IF EXISTS myUserTable');