Fix group membership by scratch variables for Visitor. Fixes bug #12195. Kudos to trex for the base patch. Also fix the same problem in the IP based group membership.
This commit is contained in:
Colin Kuskie
parent
8ab2dcca05
commit
e65368c7c1
4 changed files with 71 additions and 31 deletions
|
|
@ -1,6 +1,7 @@
|
|||
7.10.23
|
||||
- fixed #12225: Stock asset, multiple instances on a page
|
||||
- fixed #12229: Indexed thingy data has gateway url prepended to it
|
||||
- fixed #12195: Visitor group by scratch membership shared among all Visitors (Dale Trexel)
|
||||
|
||||
7.10.22
|
||||
- rfe #12223: Add date type to content profiling (metadata)
|
||||
|
|
|
|||
|
|
@ -1091,12 +1091,19 @@ Membership will always be false if no IpFilter has been set
|
|||
|
||||
id of the user to check for membership
|
||||
|
||||
=head3 sessionId
|
||||
|
||||
id of the session to check for user data. If no sessionId is passed in, then the
|
||||
group's session will be used to find one.
|
||||
|
||||
|
||||
=cut
|
||||
|
||||
sub hasIpUser {
|
||||
my $self = shift;
|
||||
my $userId = shift;
|
||||
my $session = $self->session;
|
||||
my $userId = shift;
|
||||
my $userSessionId = shift || $session->getId;
|
||||
|
||||
my $IpFilter = $self->ipFilter();
|
||||
return 0 unless ($IpFilter && $userId);
|
||||
|
|
@ -1104,9 +1111,9 @@ sub hasIpUser {
|
|||
$IpFilter =~ s/\s//g;
|
||||
my @filters = split /;/, $IpFilter;
|
||||
|
||||
my @ips = $session->db->buildArray(
|
||||
q{ select lastIP from userSession where expires > ? and userId = ? }
|
||||
,[ time(), $userId ]
|
||||
my @ips = $session->db->buildArray(
|
||||
q{ select lastIP from userSession where expires > ? and userId = ? and sessionId=?}
|
||||
,[ time(), $userId, $userSessionId, ]
|
||||
);
|
||||
|
||||
foreach my $ip (@ips) {
|
||||
|
|
@ -1207,7 +1214,7 @@ sub hasLDAPUser {
|
|||
|
||||
#-------------------------------------------------------------------
|
||||
|
||||
=head2 hasScratchUser ( userId )
|
||||
=head2 hasScratchUser ( userId, [ $sessionId ] )
|
||||
|
||||
Determine if the user passed in is a member of this group via session scratch
|
||||
variable settings and this group's scratchFilter.
|
||||
|
|
@ -1218,12 +1225,18 @@ If no scratchFilter has been set for this group, membership will always be false
|
|||
|
||||
id of the user to check for membership
|
||||
|
||||
=head3 sessionId
|
||||
|
||||
id of the session for the user being checked for membership. If no sessionId is passed in, then the
|
||||
group's session will be used to find one.
|
||||
|
||||
=cut
|
||||
|
||||
sub hasScratchUser {
|
||||
my $self = shift;
|
||||
my $userId = shift;
|
||||
my $session = $self->session;
|
||||
my $userId = shift;
|
||||
my $userSessionId = shift || $self->session;
|
||||
|
||||
my $scratchFilter = $self->scratchFilter();
|
||||
return 0 unless ($scratchFilter && $userId);
|
||||
|
|
@ -1232,7 +1245,7 @@ sub hasScratchUser {
|
|||
my @filters = split /;/, $scratchFilter;
|
||||
|
||||
my @scratchClauses = ();
|
||||
my @scratchPlaceholders = ( $userId, time() );
|
||||
my @scratchPlaceholders = ( $userSessionId, $userId, time() );
|
||||
foreach my $filter (@filters) {
|
||||
my ($name, $value) = split /=/, $filter;
|
||||
push @scratchClauses, "(s.name=? AND s.value=?)";
|
||||
|
|
@ -1246,6 +1259,7 @@ sub hasScratchUser {
|
|||
from
|
||||
userSession u, userSessionScratch s
|
||||
where
|
||||
u.sessionId = ? AND
|
||||
u.sessionId=s.sessionId AND
|
||||
u.userId = ? AND
|
||||
u.expires > ? AND
|
||||
|
|
@ -1273,6 +1287,7 @@ sub hasUser {
|
|||
my $self = shift;
|
||||
my $session = $self->session;
|
||||
my $user = shift || WebGUI::User->new($session,3); #Check the admin account if no user is passed in
|
||||
my $uSessionId = $user->session->getId;
|
||||
my $gid = $self->getId;
|
||||
my $db = $session->db;
|
||||
|
||||
|
|
@ -1359,9 +1374,9 @@ sub hasUser {
|
|||
my $groupToCheck = __PACKAGE__->new($session,$groupIdInGroup);
|
||||
### Check the 'has' method for each of the 'other' group methods available for this user
|
||||
### perform checks in a least -> most expensive manner. If we find the user, stow the cache and return true
|
||||
if( $groupToCheck->hasIpUser($uid)
|
||||
if( $groupToCheck->hasIpUser($uid, $uSessionId)
|
||||
|| $groupToCheck->hasKarmaUser($uid)
|
||||
|| $groupToCheck->hasScratchUser($uid)
|
||||
|| $groupToCheck->hasScratchUser($uid, $uSessionId)
|
||||
|| $groupToCheck->hasDatabaseUser($uid)
|
||||
|| $groupToCheck->hasLDAPUser($uid)
|
||||
) {
|
||||
|
|
|
|||
22
t/Group.t
22
t/Group.t
|
|
@ -640,15 +640,17 @@ WebGUI::Test->addToCleanup(@sessionBank);
|
|||
|
||||
#isInGroup test
|
||||
foreach my $scratchTest (@scratchTests) {
|
||||
is($scratchTest->{user}->isInGroup($gS->getId), $scratchTest->{expect}, $scratchTest->{comment});
|
||||
is($scratchTest->{user}->isInGroup($gS->getId), $scratchTest->{expect}, $scratchTest->{comment});
|
||||
}
|
||||
|
||||
WebGUI::Cache->new($session, $gS->getId)->delete(); ##Delete cached key for testing
|
||||
$session->stow->delete("isInGroup");
|
||||
|
||||
#hasScratchUser test
|
||||
foreach my $scratchTest (@scratchTests) {
|
||||
is($gS->hasScratchUser($scratchTest->{user}->getId), $scratchTest->{expect}, $scratchTest->{comment}." - hasScratchUser");
|
||||
foreach my $idx (0..$#scratchTests) {
|
||||
my $scratchTest = $scratchTests[$idx];
|
||||
my $sessionId = $sessionBank[$idx]->getId;
|
||||
is($gS->hasScratchUser($scratchTest->{user}->getId, $sessionId), $scratchTest->{expect}, $scratchTest->{comment}." - hasScratchUser");
|
||||
}
|
||||
|
||||
|
||||
|
|
@ -666,7 +668,7 @@ cmp_bag(
|
|||
|
||||
{ ##Add scope to force cleanup
|
||||
|
||||
note "Checking for user Visitor session leak";
|
||||
note "Checking for user Visitor session leak with scratch";
|
||||
|
||||
my $remoteSession = WebGUI::Test->newSession;
|
||||
$remoteSession->user({userId => 1});
|
||||
|
|
@ -681,13 +683,12 @@ cmp_bag(
|
|||
my $localSession = WebGUI::Test->newSession;
|
||||
WebGUI::Test->addToCleanup($localScratchGroup, $remoteSession, $localSession);
|
||||
$localSession->user({userId => 1});
|
||||
$remoteSession->scratch->set('local','ok');
|
||||
$localSession->scratch->set('local','ok');
|
||||
$localScratchGroup->clearCaches;
|
||||
|
||||
ok $localSession->user->isInGroup($localScratchGroup->getId), 'Local Visitor is in the scratch group';
|
||||
|
||||
$remoteSession->stow->delete('isInGroup');
|
||||
$localScratchGroup->clearCaches;
|
||||
ok !$remoteSession->user->isInGroup($localScratchGroup->getId), 'Remote Visitor is not in the scratch group, even though a different Visitor passed';
|
||||
|
||||
}
|
||||
|
|
@ -710,8 +711,9 @@ foreach my $idx (0..$#ipTests) {
|
|||
##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];
|
||||
##Assign this user and session to this test to be fetched later
|
||||
$ipTests[$idx]->{user} = $tcps[$idx];
|
||||
$ipTests[$idx]->{session} = $sessionBank[$idx];
|
||||
}
|
||||
WebGUI::Test->addToCleanup(@tcps);
|
||||
WebGUI::Test->addToCleanup(@sessionBank);
|
||||
|
|
@ -734,7 +736,7 @@ cmp_bag(
|
|||
);
|
||||
|
||||
is_deeply(
|
||||
[ (map { $gI->hasIpUser($_->{user}->getId) } @ipTests) ],
|
||||
[ (map { $gI->hasIpUser($_->{user}->getId, $_->{session}->getId) } @ipTests) ],
|
||||
[ (map { $_->{expect} } @ipTests) ],
|
||||
'hasIpUsers for group with IP filter'
|
||||
);
|
||||
|
|
@ -745,7 +747,7 @@ foreach my $ipTest (@ipTests) {
|
|||
|
||||
{ ##Add scope to force cleanup
|
||||
|
||||
note "Checking for user Visitor session leak";
|
||||
note "Checking for user Visitor session leak via IP address";
|
||||
|
||||
$ENV{REMOTE_ADDR} = '191.168.1.1';
|
||||
my $remoteSession = WebGUI::Test->newSession;
|
||||
|
|
|
|||
|
|
@ -20,27 +20,49 @@ use WebGUI::Group;
|
|||
|
||||
#----------------------------------------------------------------------------
|
||||
# Init
|
||||
my $session = WebGUI::Test->session;
|
||||
|
||||
my $session1 = WebGUI::Test->session;
|
||||
my $session2 = WebGUI::Session->open(WebGUI::Test::root, WebGUI::Test::file);
|
||||
|
||||
#----------------------------------------------------------------------------
|
||||
# Tests
|
||||
### Updates by DRT test that group membership is restricted by user session
|
||||
### ...specifically for Visitors to have separate scratch group memberships
|
||||
|
||||
plan tests => 5; # Increment this number for each test you create
|
||||
plan tests => 14; # Increment this number for each test you create
|
||||
|
||||
my $group = WebGUI::Group->new($session, 'new');
|
||||
my $group = WebGUI::Group->new($session1, 'new');
|
||||
WebGUI::Test->addToCleanup($group);
|
||||
$group->scratchFilter("itchy=test_value");
|
||||
is( $group->scratchFilter(), "itchy=test_value",'Group->scratchFilter is properly set and retrieved');
|
||||
$group->groupCacheTimeout(0);
|
||||
is( $group->groupCacheTimeout(), 0, 'set groupCacheTimeout to 0');
|
||||
|
||||
$session->user({userId => 1});
|
||||
ok( !$session->user->isInGroup($group->getId), 'Visitor is NOT in the group BEFORE scratch value is set');
|
||||
$session->scratch->set('itchy', 'test_value');
|
||||
is ($group->hasScratchUser($session->user->getId), 1, 'Group->hasScratchUser correctly returns 1 immediately after scratch is set');
|
||||
$session1->user({userId => 1});
|
||||
$session2->user({userId => 1});
|
||||
|
||||
##Simulate another page view by clearing stow, which is volatile
|
||||
$session->stow->deleteAll;
|
||||
$session->scratch->delete('itchy');
|
||||
is ($group->hasScratchUser($session->user->getId), 0, 'after clearing scratch, user is not in the group any longer');
|
||||
### Test group membership before scratch is set
|
||||
### NOTE: test hasScratchUser first, because isInGroup sets stow & cache
|
||||
is ($group->hasScratchUser($session1->user->getId,$session1->user->session->getId), 0, 'Group->hasScratchUser correctly returns 0 for Visitor 1 before scratch is set');
|
||||
is ($group->hasScratchUser($session2->user->getId,$session2->user->session->getId), 0, 'Group->hasScratchUser correctly returns 0 for Visitor 2 before scratch is set');
|
||||
ok( !$session1->user->isInGroup($group->getId), 'user1->isInGroup correctly returns 0 before scratch is set');
|
||||
ok( !$session2->user->isInGroup($group->getId), 'user2->isInGroup correctly returns 0 before scratch is set');
|
||||
|
||||
### Test group membership after scratch is set
|
||||
### Clear stow, which is volatile, to simulate new page view
|
||||
$session1->stow->deleteAll;
|
||||
$session2->stow->deleteAll;
|
||||
$session1->scratch->set('itchy', 'test_value');
|
||||
is ($group->hasScratchUser($session1->user->getId,$session1->user->session->getId), 1, 'Group->hasScratchUser correctly returns 1 for Visitor 1 after scratch for Visitor 1 is set');
|
||||
is ($group->hasScratchUser($session2->user->getId,$session2->user->session->getId), 0, 'Group->hasScratchUser correctly returns 0 for Visitor 2 after scratch for Visitor 1 is set');
|
||||
ok( $session1->user->isInGroup($group->getId), 'user1->isInGroup correctly returns 1 after scratch for Visitor 1 is set');
|
||||
ok( !$session2->user->isInGroup($group->getId), 'user2->isInGroup correctly returns 0 after scratch for Visitor 1 is set');
|
||||
|
||||
### Test group membership after scratch is deleted
|
||||
### Clear stow, which is volatile, to simulate new page view
|
||||
$session1->stow->deleteAll;
|
||||
$session2->stow->deleteAll;
|
||||
$session1->scratch->delete('itchy');
|
||||
is ($group->hasScratchUser($session1->user->getId,$session1->user->session->getId), 0, 'Group->hasScratchUser for Visitor 1 correctly returns 0 after clearing scratch for Visitor 1');
|
||||
is ($group->hasScratchUser($session2->user->getId,$session2->user->session->getId), 0, 'Group->hasScratchUser for Visitor 2 correctly returns 0 after clearing scratch for Visitor 1');
|
||||
ok( !$session1->user->isInGroup($group->getId), 'user1->isInGroup correctly returns 0 after scratch for Visitor 1 is deleted');
|
||||
ok( !$session2->user->isInGroup($group->getId), 'user2->isInGroup correctly returns 0 after scratch for Visitor 1 is deleted');
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue