From 510c1f5ca57c38a977e48c704afe5459cbb1dd0b Mon Sep 17 00:00:00 2001 From: Colin Kuskie Date: Wed, 5 Apr 2006 18:35:26 +0000 Subject: [PATCH] Modify User.pm and Group.pm to consider external means of defining users, such as a dbQuery. This would happened coincidently before, but required doing a User->isInGroup and only added members who were searched for rather than all members. Modify Group.t to test this. Update POD in SQL.pm. --- lib/WebGUI/Group.pm | 51 ++++++++++++++++++++++++++++++++++++++++++++- lib/WebGUI/SQL.pm | 2 -- lib/WebGUI/User.pm | 47 ++++++++--------------------------------- t/Group.t | 19 ++++++++++++----- 4 files changed, 73 insertions(+), 46 deletions(-) diff --git a/lib/WebGUI/Group.pm b/lib/WebGUI/Group.pm index e3f369742..ae6d83386 100755 --- a/lib/WebGUI/Group.pm +++ b/lib/WebGUI/Group.pm @@ -419,6 +419,45 @@ sub expireOffset { } +#------------------------------------------------------------------- + +=head2 externalUsers ( ) + +Get the set of users allowed to be in this group via external means, such as database +queries, LDAP and/or IP filtering. + +=cut + +sub externalUsers { + my $self = shift; + my $gid = $self->getId; + my @externalUsers = (); + my $isInGroup = $self->session->stow->get('isInGroup'); + ### Check external database + if ($self->get("dbQuery") && defined $self->get("databaseLinkId")) { + my $dbLink = WebGUI::DatabaseLink->new($self->session,$self->get("databaseLinkId")); + my $dbh = $dbLink->db; + if (defined $dbh) { + my $query = $self->get("dbQuery"); + WebGUI::Macro::process($self->session,\$query); + my $sth = $dbh->unconditionalRead($query); + unless ($sth->errorCode < 1) { + $self->session->errorHandler->warn("There was a problem with the database query for group ID $gid."); + } + else { + while(my ($userId)=$sth->array) { + push @externalUsers, $userId; + $isInGroup->{$userId}{$gid} = 1; + } + } + $sth->finish; + $dbLink->disconnect; + } + } + $self->session->stow->set("isInGroup",$isInGroup); + return \@externalUsers; +} + #------------------------------------------------------------------- =head2 find ( session, name ) @@ -512,6 +551,8 @@ sub getGroupsIn { elsif (!$isRecursive and exists($gotGroupsInGroup->{direct}{$self->getId})) { return $gotGroupsInGroup->{direct}{$self->getId}; } + ##We call updateUsers here because we get a free trip with recursion and many other User and Group + ##methods call getGroupsIn. my $groups = $self->session->db->buildArrayRef("select groupId from groupGroupings where inGroup=?",[$self->getId]); if ($isRecursive) { $loopCount++; @@ -556,6 +597,7 @@ sub getUsers { my $recursive = shift; my $withoutExpired = shift; my $clause; + my @externalUsers = (); if ($withoutExpired) { $clause = "expireDate > ".$self->session->datetime->time()." and "; } @@ -571,9 +613,16 @@ sub getUsers { $clause .= " OR groupId IN (".$self->session->db->quoteAndJoin($groups).")"; } } + ##Have to iterate twice due to the withoutExpired clause. + foreach my $groupId (@{ $groups }) { + push @externalUsers, @{ WebGUI::Group->new($self->session, $groupId)->externalUsers() }; + } } $clause .= ")"; - return $self->session->db->buildArrayRef("select userId from groupings where $clause"); + my @localUsers = $self->session->db->buildArray("select userId from groupings where $clause"); + push @externalUsers, @{ $self->externalUsers() }; + my @users = ( @localUsers, @externalUsers ); + return \@users; } diff --git a/lib/WebGUI/SQL.pm b/lib/WebGUI/SQL.pm index e0235951a..aa4f34cf8 100644 --- a/lib/WebGUI/SQL.pm +++ b/lib/WebGUI/SQL.pm @@ -356,8 +356,6 @@ sub errorMessage { Increments an incrementer of the specified type and returns the value. -B This is not a regular method, but is an exported subroutine. - =head3 idName Specify the name of one of the incrementers in the incrementer table. diff --git a/lib/WebGUI/User.pm b/lib/WebGUI/User.pm index 447527cb0..2dc363a40 100644 --- a/lib/WebGUI/User.pm +++ b/lib/WebGUI/User.pm @@ -208,7 +208,7 @@ sub getGroups { if (exists $gotGroupsForUser->{$self->userId}) { return $gotGroupsForUser->{$self->userId}; } else { - my @groups = $self->session->db->buildArray("select groupId from groupings where userId=".$self->session->db->quote($self->userId)." $clause"); + my @groups = $self->session->db->buildArray("select groupId from groupings where userId=? $clause", [$self->userId]); my $isInGroup = $self->session->stow->get("isInGroup"); foreach my $gid (@groups) { $isInGroup->{$self->userId}{$gid} = 1; @@ -247,7 +247,6 @@ Returns a boolean (0|1) value signifying that the user has the required privileg The group that you wish to verify against the user. Defaults to group with Id 3 (the Admin group). =cut - sub isInGroup { my (@data, $groupId); my ($self, $gid, $secondRun) = @_; @@ -308,42 +307,6 @@ sub isInGroup { $self->session->stow->set("isInGroup",$isInGroup); return 1; } - } - ### Check external database - if ($group->get("dbQuery") && defined $group->get("databaseLinkId")) { - # skip if not logged in and query contains a User macro - unless ($group->get("dbQuery") =~ /\^User/i && $uid eq '1') { - my $dbLink = WebGUI::DatabaseLink->new($self->session,$group->get("databaseLinkId")); - my $dbh = $dbLink->db; - if (defined $dbh) { - if ($group->get("dbQuery") =~ /select 1/i) { - my $query = $group->get("dbQuery"); - WebGUI::Macro::process($self->session,\$query); - my $sth = $dbh->unconditionalRead($query); - unless ($sth->errorCode < 1) { - $self->session->errorHandler->warn("There was a problem with the database query for group ID $gid."); - } else { - my ($result) = $sth->array; - if ($result == 1) { - $isInGroup->{$uid}{$gid} = 1; - if ($group->get("dbCacheTimeout") > 0) { - $group->deleteUsers([$uid]); - $group->addUsers([$uid],$group->get("dbCacheTimeout")); - } - } else { - $isInGroup->{$uid}{$gid} = 0; - $group->deleteUsers([$uid]) if ($group->get("dbCacheTimeout") > 0); - } - } - $sth->finish; - } else { - $self->session->errorHandler->warn("Database query for group ID $gid must use 'select 1'"); - } - $dbLink->disconnect; - $self->session->stow->set("isInGroup",$isInGroup); - return 1 if ($isInGroup->{$uid}{$gid}); - } - } } ### Check ldap if ($group->get("ldapGroup") && $group->get("ldapGroupProperty")) { @@ -386,6 +349,14 @@ sub isInGroup { } } + if ($group->get("dbQuery") && defined $group->get("databaseLinkId")) { + my @externalUsers = @{ $group->externalUsers() } ; + foreach my $extUserId ( @externalUsers ) { + $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}) { diff --git a/t/Group.t b/t/Group.t index 6fe45ddb2..aea0fa9c2 100644 --- a/t/Group.t +++ b/t/Group.t @@ -18,7 +18,7 @@ use WebGUI::Utility; use WebGUI::User; use WebGUI::Group; -use Test::More tests => 75; # increment this value for each test you create +use Test::More tests => 78; # increment this value for each test you create use Test::Deep; my $session = WebGUI::Test->session; @@ -223,17 +223,26 @@ my $mobUsers = $session->db->buildArrayRef('select userId from myUserTable'); cmp_bag($mobUsers, [map {$_->userId} @mob], 'verify SQL table built correctly'); is( $gY->databaseLinkId, 0, "Group Y's databaseLinkId is set to WebGUI"); -$gY->dbQuery(q!select 1 from myUserTable where userId='^#();'!); +$gY->dbQuery(q!select userId from myUserTable!); is( $session->stow->get('isInGroup'), undef, 'setting dbQuery clears cached isInGroup'); $session->user({userId => $mob[0]->userId}); is( $session->user->userId, $mob[0]->userId, 'mob[0] set to be current user'); $session->errorHandler->warn('checking mob[0] group membership'); is( $mob[0]->isInGroup($gY->getId), 1, 'mob[0] is in group Y after setting dbQuery'); -$session->user({userId => $mob[1]->userId}); -is( $mob[0]->isInGroup($gZ->getId), 1, 'mob[0] is not in group Z'); +is( $mob[0]->isInGroup($gZ->getId), 1, 'mob[0] isInGroup Z'); -ok( !isIn($mob[0]->userId, $gZ->getUsers(1)), 'mob[0] not in list of group Z users'); +ok( isIn($mob[0]->userId, @{ $gY->getUsers() }), 'mob[0] in list of group Y users'); +ok( !isIn($mob[0]->userId, @{ $gZ->getUsers() }), 'mob[0] not in list of group Z users'); + +$session->errorHandler->warn('getUsers checks'); +$session->errorHandler->warn('mob[0] userId = '. $mob[0]->userId); +ok( isIn($mob[0]->userId, @{ $gZ->getUsers(1) }), 'mob[0] in list of group Z users, recursively'); + +SKIP: { + skip("need to test expiration date in groupings interacting with recursive or not", 1); + ok(undef, "expiration date in groupings for getUser"); +} END { (defined $gX and ref $gX eq 'WebGUI::Group') and $gX->delete;