diff --git a/docs/changelog/7.x.x.txt b/docs/changelog/7.x.x.txt index d65300ce1..f64b07ce5 100644 --- a/docs/changelog/7.x.x.txt +++ b/docs/changelog/7.x.x.txt @@ -3,6 +3,7 @@ - fix: SQLForm file upload broken - fix: Newsletter can contain duplicate threads - fix: Captcha unreadable when using Image::Magick + - fix: WebGUI::Group->new doesn't check group validity 7.4.7 - fix: misspelled i18n in webgui password recovery diff --git a/lib/WebGUI/Asset/Post/Thread.pm b/lib/WebGUI/Asset/Post/Thread.pm index 1484e8a09..dfd7d7762 100644 --- a/lib/WebGUI/Asset/Post/Thread.pm +++ b/lib/WebGUI/Asset/Post/Thread.pm @@ -778,9 +778,11 @@ Negates the subscribe method. =cut sub unsubscribe { - my $self = shift; - my $group = WebGUI::Group->new($self->session,$self->get("subscriptionGroupId")); - $group->deleteUsers([$self->session->user->userId]); + my $self = shift; + my $group = WebGUI::Group->new($self->session,$self->get("subscriptionGroupId")); + return + if !$group; + $group->deleteUsers([$self->session->user->userId]); } diff --git a/lib/WebGUI/Asset/Wobject/Collaboration.pm b/lib/WebGUI/Asset/Wobject/Collaboration.pm index a781ce84f..a0d3c8921 100644 --- a/lib/WebGUI/Asset/Wobject/Collaboration.pm +++ b/lib/WebGUI/Asset/Wobject/Collaboration.pm @@ -1138,7 +1138,9 @@ sub processPropertiesFromFormPost { sub purge { my $self = shift; my $group = WebGUI::Group->new($self->session, $self->get("subscriptionGroupId")); - $group->delete; + if ($group) { + $group->delete; + } if ($self->get("getMailCronId")) { my $cron = WebGUI::Workflow::Cron->new($self->session, $self->get("getMailCronId")); $cron->delete if defined $cron; @@ -1262,8 +1264,16 @@ Subscribes a user to this collaboration system. sub subscribe { my $self = shift; - my $group = WebGUI::Group->new($self->session,$self->get("subscriptionGroupId")); - $group->addUsers([$self->session->user->userId]); + my $group; + my $subscriptionGroup = $self->get('subscriptionGroupId'); + if ($subscriptionGroup) { + $group = WebGUI::Group->new($self->session,$subscriptionGroup); + } + if (!$group) { + $self->createSubscriptionGroup; + $group = WebGUI::Group->new($self->session,$self->get('subscriptionGroupId')); + } + $group->addUsers([$self->session->user->userId]); } #------------------------------------------------------------------- @@ -1277,7 +1287,9 @@ Unsubscribes a user from this collaboration system sub unsubscribe { my $self = shift; my $group = WebGUI::Group->new($self->session,$self->get("subscriptionGroupId")); - $group->deleteUsers([$self->session->user->userId],[$self->get("subscriptionGroupId")]); + return + unless $group; + $group->deleteUsers([$self->session->user->userId],[$self->get("subscriptionGroupId")]); } diff --git a/lib/WebGUI/Group.pm b/lib/WebGUI/Group.pm index 32cea0bdc..7714e4f3e 100755 --- a/lib/WebGUI/Group.pm +++ b/lib/WebGUI/Group.pm @@ -981,15 +981,18 @@ If you specified "new" for groupId, you can use this property to specify an id y =cut sub new { - my ($class, %group); - tie %group, 'Tie::CPHash'; - $class = shift; - my $self = {}; - $self->{_session} = shift; - $self->{_groupId} = shift; - my $override = shift; - my $cached = $self->{_session}->stow->get("groupObj"); + my ($class, %group); + tie %group, 'Tie::CPHash'; + my $self = {}; + + $class = shift; + $self->{_session} = shift; + $self->{_groupId} = shift; + my $override = shift; + + my $cached = $self->{_session}->stow->get("groupObj"); return $cached->{$self->{_groupId}} if ($cached->{$self->{_groupId}}); + bless $self, $class; if ($self->{_groupId} eq "new") { $self->_create($override); @@ -997,6 +1000,18 @@ sub new { elsif ($self->{_groupId} eq "") { $self->{_group} = $self->_defaults(); } + else { + # Check if the groupId is valid. If not return undef + my ($groupExists) = $self->{_session}->db->quickArray('select groupId from groups where groupId=?', [ + $self->{_groupId}, + ]); + unless ($groupExists) { + $self->{_session}->errorHandler->warn('WebGUI::Group->new called with a non-existant groupId:' + .'['.$self->{_groupId}.']'); + return undef; + } + } + $cached->{$self->{_groupId}} = $self; $self->{_session}->stow->set("groupObj", $cached); return $self; diff --git a/lib/WebGUI/Operation/Group.pm b/lib/WebGUI/Operation/Group.pm index 3ec37e15e..6c3a6ccdd 100644 --- a/lib/WebGUI/Operation/Group.pm +++ b/lib/WebGUI/Operation/Group.pm @@ -82,6 +82,8 @@ sub canEditGroup { return 1 if canEditAll($session, $user); my $group = WebGUI::Group->new($session,$groupId); + return + unless $group; return $user->isInGroup( $session->setting->get("groupIdAdminGroupAdmin") ) && $group->userIsAdmin( $user->userId ) ; @@ -464,7 +466,7 @@ sub www_editGroupSave { return $session->privilege->adminOnly unless canEditGroup($session, $gid); my $g = WebGUI::Group->new($session, $gid); - # We don't want them to use an existing name. If needed, we'll ad a number to the name to keep it unique. + # We don't want them to use an existing name. If needed, we'll add a number to the name to keep it unique. my $groupName = $session->form->process("groupName"); while (my $existingGroupId = WebGUI::Group->find($session, $groupName)->getId) { last @@ -499,50 +501,52 @@ sub www_editGroupSave { #------------------------------------------------------------------- sub www_editGrouping { - my $session = shift; - return $session->privilege->adminOnly() unless (canEditGroup($session,$session->form->process("gid"))); - my $i18n = WebGUI::International->new($session); - my $f = WebGUI::HTMLForm->new($session); - $f->submit; - $f->hidden( - -name => "op", - -value => "editGroupingSave" - ); - $f->hidden( - -name => "uid", - -value => $session->form->process("uid") - ); - $f->hidden( - -name => "gid", - -value => $session->form->process("gid") - ); - my $u = WebGUI::User->new($session,$session->form->process("uid")); - my $g = WebGUI::Group->new($session,$session->form->process("gid")); - $f->readOnly( - -value => $u->username, - -label => $i18n->get(50), - -hoverHelp => $i18n->get('50 description'), - ); - $f->readOnly( - -value => $g->name, - -label => $i18n->get(84), - -hoverHelp => $i18n->get('84 description'), - ); - my $group = WebGUI::Group->new($session,$session->form->process("gid")); - $f->date( - -name => "expireDate", - -label => $i18n->get(369), - -hoverHelp => $i18n->get('369 description'), - -value => $group->userGroupExpireDate($session->form->process("uid")), - ); - $f->yesNo( - -name=>"groupAdmin", - -label=>$i18n->get(977), - -hoverHelp=>$i18n->get('977 description'), - -value=>$group->userIsAdmin($session->form->process("uid")) - ); - $f->submit; - return _submenu($session,$f->print,'370'); + my $session = shift; + my $uid = $session->form->process('uid'); + my $gid = $session->form->process('gid'); + return $session->privilege->adminOnly() + unless canEditGroup($session, $gid); + my $i18n = WebGUI::International->new($session); + my $f = WebGUI::HTMLForm->new($session); + $f->submit; + $f->hidden( + -name => "op", + -value => "editGroupingSave" + ); + $f->hidden( + -name => "uid", + -value => $uid, + ); + $f->hidden( + -name => "gid", + -value => $gid, + ); + my $u = WebGUI::User->new($session,$uid); + my $g = WebGUI::Group->new($session,$gid); + $f->readOnly( + -value => $u->username, + -label => $i18n->get(50), + -hoverHelp => $i18n->get('50 description'), + ); + $f->readOnly( + -value => $g && $g->name, + -label => $i18n->get(84), + -hoverHelp => $i18n->get('84 description'), + ); + $f->date( + -name => "expireDate", + -label => $i18n->get(369), + -hoverHelp => $i18n->get('369 description'), + -value => $g && $g->userGroupExpireDate($uid), + ); + $f->yesNo( + -name=>"groupAdmin", + -label=>$i18n->get(977), + -hoverHelp=>$i18n->get('977 description'), + -value=> $g && $g->userIsAdmin($uid), + ); + $f->submit; + return _submenu($session,$f->print,'370'); } #------------------------------------------------------------------- diff --git a/lib/WebGUI/User.pm b/lib/WebGUI/User.pm index fa65e6abd..ec3b0e2be 100644 --- a/lib/WebGUI/User.pm +++ b/lib/WebGUI/User.pm @@ -297,6 +297,8 @@ sub isInGroup { return $isInGroup->{$uid}{$gid} if exists $isInGroup->{$uid}{$gid}; ### Lookup the actual groupings. my $group = WebGUI::Group->new($self->session,$gid); + # Cope with non-existant groups. Default to the admin group if the groupId is invalid. + $group = WebGUI::Group->new($self->session, 3) unless $group; ### Check for groups of groups. my $users = $group->getAllUsers(); foreach my $user (@{$users}) {