diff options
Diffstat (limited to 'rt/lib/RT')
| -rw-r--r-- | rt/lib/RT/Config.pm | 26 | ||||
| -rw-r--r-- | rt/lib/RT/Groups_Overlay.pm | 33 | ||||
| -rwxr-xr-x | rt/lib/RT/Record.pm | 1 | ||||
| -rw-r--r-- | rt/lib/RT/SearchBuilder.pm | 62 | ||||
| -rw-r--r-- | rt/lib/RT/Ticket_Overlay.pm | 143 | ||||
| -rw-r--r-- | rt/lib/RT/User_Overlay.pm | 67 | ||||
| -rw-r--r-- | rt/lib/RT/Users_Overlay.pm | 136 |
7 files changed, 270 insertions, 198 deletions
diff --git a/rt/lib/RT/Config.pm b/rt/lib/RT/Config.pm index 4e66c3cff..3f8581cc5 100644 --- a/rt/lib/RT/Config.pm +++ b/rt/lib/RT/Config.pm @@ -333,6 +333,22 @@ our %META = ( }, }, + RTAddressRegexp => { + Type => 'SCALAR', + PostLoadCheck => sub { + my $self = shift; + my $value = $self->Get('RTAddressRegexp'); + return if $value; + + $RT::Logger->error( + 'The RTAddressRegexp option is not set in the config.' + .' Not setting this option results in additional SQL queries to' + .' check whether each address belongs to RT or not.' + .' It is especially important to set this option if RT recieves' + .' emails on addresses that are not in the database or config.' + ); + }, + }, # User overridable mail options EmailFrequency => { Section => 'Mail', #loc @@ -349,6 +365,16 @@ our %META = ( ] } }, + NotifyActor => { + Section => 'Mail', #loc + Overridable => 1, + SortOrder => 2, + Widget => '/Widgets/Form/Boolean', + WidgetArguments => { + Description => 'Outgoing mail', #loc + Hints => 'Should RT send you mail for ticket updates you make?', #loc + } + }, # this tends to break extensions that stash links in ticket update pages Organization => { diff --git a/rt/lib/RT/Groups_Overlay.pm b/rt/lib/RT/Groups_Overlay.pm index 407b905b9..1b3ef51e2 100644 --- a/rt/lib/RT/Groups_Overlay.pm +++ b/rt/lib/RT/Groups_Overlay.pm @@ -88,6 +88,7 @@ sub _Init { my $self = shift; $self->{'table'} = "Groups"; $self->{'primary_key'} = "id"; + $self->{'with_disabled_column'} = 1; my @result = $self->SUPER::_Init(@_); @@ -129,7 +130,7 @@ sub PrincipalsAlias { } -# {{{ LimiToSystemInternalGroups +# {{{ LimitToSystemInternalGroups =head2 LimitToSystemInternalGroups @@ -148,7 +149,7 @@ sub LimitToSystemInternalGroups { # }}} -# {{{ LimiToUserDefinedGroups +# {{{ LimitToUserDefinedGroups =head2 LimitToUserDefinedGroups @@ -167,7 +168,7 @@ sub LimitToUserDefinedGroups { # }}} -# {{{ LimiToPersonalGroupsFor +# {{{ LimitToPersonalGroupsFor =head2 LimitToPersonalGroupsFor PRINCIPAL_ID @@ -377,12 +378,13 @@ Only find items that haven\'t been disabled sub LimitToEnabled { my $self = shift; - - $self->Limit( ALIAS => $self->PrincipalsAlias, - FIELD => 'Disabled', - VALUE => '0', - OPERATOR => '=', - ); + + $self->{'handled_disabled_column'} = 1; + $self->Limit( + ALIAS => $self->PrincipalsAlias, + FIELD => 'Disabled', + VALUE => '0', + ); } # }}} @@ -397,13 +399,14 @@ Only find items that have been deleted. sub LimitToDeleted { my $self = shift; - $self->{'find_disabled_rows'} = 1; - $self->Limit( ALIAS => $self->PrincipalsAlias, - FIELD => 'Disabled', - OPERATOR => '=', - VALUE => 1, - ); + $self->{'handled_disabled_column'} = $self->{'find_disabled_rows'} = 1; + $self->Limit( + ALIAS => $self->PrincipalsAlias, + FIELD => 'Disabled', + VALUE => 1, + ); } + # }}} # {{{ sub Next diff --git a/rt/lib/RT/Record.pm b/rt/lib/RT/Record.pm index 4f38b0405..bee94420d 100755 --- a/rt/lib/RT/Record.pm +++ b/rt/lib/RT/Record.pm @@ -1562,6 +1562,7 @@ sub CustomFields { $cfs->LimitToGlobalOrObjectId( $self->_LookupId( $self->CustomFieldLookupType ) ); + $cfs->ApplySortOrder; return $cfs; } diff --git a/rt/lib/RT/SearchBuilder.pm b/rt/lib/RT/SearchBuilder.pm index 527952b67..671d880c7 100644 --- a/rt/lib/RT/SearchBuilder.pm +++ b/rt/lib/RT/SearchBuilder.pm @@ -93,10 +93,9 @@ Only find items that haven't been disabled sub LimitToEnabled { my $self = shift; - - $self->Limit( FIELD => 'Disabled', - VALUE => '0', - OPERATOR => '=' ); + + $self->{'handled_disabled_column'} = 1; + $self->Limit( FIELD => 'Disabled', VALUE => '0' ); } =head2 LimitToDeleted @@ -107,12 +106,19 @@ Only find items that have been deleted. sub LimitToDeleted { my $self = shift; - - $self->{'find_disabled_rows'} = 1; - $self->Limit( FIELD => 'Disabled', - OPERATOR => '=', - VALUE => '1' - ); + + $self->{'handled_disabled_column'} = $self->{'find_disabled_rows'} = 1; + $self->Limit( FIELD => 'Disabled', VALUE => '1' ); +} + +=head2 FindAllRows + +Find all matching rows, regardless of whether they are disabled or not + +=cut + +sub FindAllRows { + shift->{'find_disabled_rows'} = 1; } =head2 LimitAttribute PARAMHASH @@ -262,16 +268,6 @@ sub LimitCustomField { ); } -=head2 FindAllRows - -Find all matching rows, regardless of whether they are disabled or not - -=cut - -sub FindAllRows { - shift->{'find_disabled_rows'} = 1; -} - =head2 Limit PARAMHASH This Limit sub calls SUPER::Limit, but defaults "CASESENSITIVE" to 1, thus @@ -323,6 +319,32 @@ sub ItemsArrayRef { return $self->ItemsOrderBy($self->SUPER::ItemsArrayRef()); } +# make sure that Disabled rows never get seen unless +# we're explicitly trying to see them. + +sub _DoSearch { + my $self = shift; + + if ( $self->{'with_disabled_column'} + && !$self->{'handled_disabled_column'} + && !$self->{'find_disabled_rows'} + ) { + $self->LimitToEnabled; + } + return $self->SUPER::_DoSearch(@_); +} +sub _DoCount { + my $self = shift; + + if ( $self->{'with_disabled_column'} + && !$self->{'handled_disabled_column'} + && !$self->{'find_disabled_rows'} + ) { + $self->LimitToEnabled; + } + return $self->SUPER::_DoCount(@_); +} + eval "require RT::SearchBuilder_Vendor"; die $@ if ($@ && $@ !~ qr{^Can't locate RT/SearchBuilder_Vendor.pm}); eval "require RT::SearchBuilder_Local"; diff --git a/rt/lib/RT/Ticket_Overlay.pm b/rt/lib/RT/Ticket_Overlay.pm index f4664fd72..83737c168 100644 --- a/rt/lib/RT/Ticket_Overlay.pm +++ b/rt/lib/RT/Ticket_Overlay.pm @@ -135,6 +135,11 @@ our %LINKDIRMAP = ( sub LINKTYPEMAP { return \%LINKTYPEMAP } sub LINKDIRMAP { return \%LINKDIRMAP } +our %MERGE_CACHE = ( + effective => {}, + merged => {}, +); + # {{{ sub Load =head2 Load @@ -148,47 +153,46 @@ Otherwise, returns the ticket id. sub Load { my $self = shift; my $id = shift; + $id = '' unless defined $id; - #TODO modify this routine to look at EffectiveId and do the recursive load - # thing. be careful to cache all the interim tickets we try so we don't loop forever. + # TODO: modify this routine to look at EffectiveId and + # do the recursive load thing. be careful to cache all + # the interim tickets we try so we don't loop forever. # FIXME: there is no TicketBaseURI option in config my $base_uri = RT->Config->Get('TicketBaseURI') || ''; #If it's a local URI, turn it into a ticket id - if ( $base_uri && defined $id && $id =~ /^$base_uri(\d+)$/ ) { + if ( $base_uri && $id =~ /^$base_uri(\d+)$/ ) { $id = $1; } - #If it's a remote URI, we're going to punt for now - elsif ( $id =~ '://' ) { + unless ( $id =~ /^\d+$/ ) { + $RT::Logger->debug("Tried to load a bogus ticket id: '$id'"); return (undef); } - #If we have an integer URI, load the ticket - if ( defined $id && $id =~ /^\d+$/ ) { - my ($ticketid,$msg) = $self->LoadById($id); + $id = $MERGE_CACHE{'effective'}{ $id } + if $MERGE_CACHE{'effective'}{ $id }; - unless ($self->Id) { - $RT::Logger->debug("$self tried to load a bogus ticket: $id"); - return (undef); - } - } - - #It's not a URI. It's not a numerical ticket ID. Punt! - else { - $RT::Logger->debug("Tried to load a bogus ticket id: '$id'"); + my ($ticketid, $msg) = $self->LoadById( $id ); + unless ( $self->Id ) { + $RT::Logger->debug("$self tried to load a bogus ticket: $id"); return (undef); } #If we're merged, resolve the merge. - if ( ( $self->EffectiveId ) and ( $self->EffectiveId != $self->Id ) ) { - $RT::Logger->debug ("We found a merged ticket.". $self->id ."/".$self->EffectiveId); - return ( $self->Load( $self->EffectiveId ) ); + if ( $self->EffectiveId && $self->EffectiveId != $self->Id ) { + $RT::Logger->debug( + "We found a merged ticket. " + . $self->id ."/". $self->EffectiveId + ); + my $real_id = $self->Load( $self->EffectiveId ); + $MERGE_CACHE{'effective'}{ $id } = $real_id; + return $real_id; } #Ok. we're loaded. lets get outa here. - return ( $self->Id ); - + return $self->Id; } # }}} @@ -1160,12 +1164,20 @@ sub _AddWatcher { my $principal = RT::Principal->new($self->CurrentUser); if ($args{'Email'}) { + if ( RT::EmailParser->IsRTAddress( $args{'Email'} ) ) { + return (0, $self->loc("[_1] is an address RT receives mail at. Adding it as a '[_2]' would create a mail loop", $args{'Email'}, $self->loc($args{'Type'}))); + } my $user = RT::User->new($RT::SystemUser); my ($pid, $msg) = $user->LoadOrCreateByEmail( $args{'Email'} ); $args{'PrincipalId'} = $pid if $pid; } if ($args{'PrincipalId'}) { $principal->Load($args{'PrincipalId'}); + if ( $principal->id and $principal->IsUser and my $email = $principal->Object->EmailAddress ) { + return (0, $self->loc("[_1] is an address RT receives mail at. Adding it as a '[_2]' would create a mail loop", $email, $self->loc($args{'Type'}))) + if RT::EmailParser->IsRTAddress( $email ); + + } } @@ -1696,8 +1708,9 @@ sub IsOwner { =head2 TransactionAddresses -Returns a composite hashref of the results of L<RT::Transaction/Addresses> for all this ticket's Create, Comment or Correspond transactions. -The keys are C<To>, C<Cc> and C<Bcc>. The values are lists of C<Email::Address> objects. +Returns a composite hashref of the results of L<RT::Transaction/Addresses> for +all this ticket's Create, Comment or Correspond transactions. The keys are +stringified email addresses. Each value is an L<Email::Address> object. NOTE: For performance reasons, this method might want to skip transactions and go straight for attachments. But to make that work right, we're going to need to go and walk around the access control in Attachment.pm's sub _Value. @@ -2230,13 +2243,13 @@ sub _RecordNote { my $addresses = join ', ', ( map { RT::User->CanonicalizeEmailAddress( $_->address ) } Email::Address->parse( $args{ $type . 'MessageTo' } ) ); - $args{'MIMEObj'}->head->add( 'RT-Send-' . $type, $addresses ); + $args{'MIMEObj'}->head->add( 'RT-Send-' . $type, Encode::encode_utf8( $addresses ) ); } } foreach my $argument (qw(Encrypt Sign)) { $args{'MIMEObj'}->head->add( - "X-RT-$argument" => $args{ $argument } + "X-RT-$argument" => Encode::encode_utf8( $args{ $argument } ) ) if defined $args{ $argument }; } @@ -2282,34 +2295,35 @@ sub _Links { my $field = shift; my $type = shift || ""; - unless ( $self->{"$field$type"} ) { - $self->{"$field$type"} = new RT::Links( $self->CurrentUser ); - - #not sure what this ACL was supposed to do... but returning the - # bare (unlimited) RT::Links certainly seems wrong, it causes the - # $Ticket->Customers method during creation to return results for every - # ticket... - #if ( $self->CurrentUserHasRight('ShowTicket') ) { - - # Maybe this ticket is a merged ticket - my $Tickets = new RT::Tickets( $self->CurrentUser ); - # at least to myself - $self->{"$field$type"}->Limit( FIELD => $field, - VALUE => $self->URI, - ENTRYAGGREGATOR => 'OR' ); - $Tickets->Limit( FIELD => 'EffectiveId', - VALUE => $self->EffectiveId ); - while (my $Ticket = $Tickets->Next) { - $self->{"$field$type"}->Limit( FIELD => $field, - VALUE => $Ticket->URI, - ENTRYAGGREGATOR => 'OR' ); - } - $self->{"$field$type"}->Limit( FIELD => 'Type', - VALUE => $type ) - if ($type); - #} + my $cache_key = "$field$type"; + return $self->{ $cache_key } if $self->{ $cache_key }; + + my $links = $self->{ $cache_key } + = RT::Links->new( $self->CurrentUser ); + unless ( $self->CurrentUserHasRight('ShowTicket') ) { + $links->Limit( FIELD => 'id', VALUE => 0 ); + return $links; } - return ( $self->{"$field$type"} ); + + # Maybe this ticket is a merge ticket + my $limit_on = 'Local'. $field; + # at least to myself + $links->Limit( + FIELD => $limit_on, + VALUE => $self->id, + ENTRYAGGREGATOR => 'OR', + ); + $links->Limit( + FIELD => $limit_on, + VALUE => $_, + ENTRYAGGREGATOR => 'OR', + ) foreach $self->Merged; + $links->Limit( + FIELD => 'Type', + VALUE => $type, + ) if $type; + + return $links; } # }}} @@ -2551,8 +2565,6 @@ sub _AddLink { MergeInto take the id of the ticket to merge this ticket into. - - =cut sub MergeInto { @@ -2564,7 +2576,7 @@ sub MergeInto { } # Load up the new ticket. - my $MergeInto = RT::Ticket->new($RT::SystemUser); + my $MergeInto = RT::Ticket->new($self->CurrentUser); $MergeInto->Load($ticket_id); # make sure it exists. @@ -2577,6 +2589,11 @@ sub MergeInto { return ( 0, $self->loc("Permission Denied") ); } + delete $MERGE_CACHE{'effective'}{ $self->id }; + delete @{ $MERGE_CACHE{'merged'} }{ + $ticket_id, $MergeInto->id, $self->id + }; + $RT::Handle->BeginTransaction(); # We use EffectiveId here even though it duplicates information from @@ -2726,18 +2743,22 @@ Returns list of tickets' ids that's been merged into this ticket. sub Merged { my $self = shift; - my $mergees = new RT::Tickets( $self->CurrentUser ); + my $id = $self->id; + return @{ $MERGE_CACHE{'merged'}{ $id } } + if $MERGE_CACHE{'merged'}{ $id }; + + my $mergees = RT::Tickets->new( $self->CurrentUser ); $mergees->Limit( FIELD => 'EffectiveId', - OPERATOR => '=', - VALUE => $self->Id, + VALUE => $id, ); $mergees->Limit( FIELD => 'id', OPERATOR => '!=', - VALUE => $self->Id, + VALUE => $id, ); - return map $_->id, @{ $mergees->ItemsArrayRef || [] }; + return @{ $MERGE_CACHE{'merged'}{ $id } ||= [] } + = map $_->id, @{ $mergees->ItemsArrayRef || [] }; } # }}} diff --git a/rt/lib/RT/User_Overlay.pm b/rt/lib/RT/User_Overlay.pm index 0f2856816..2115c3299 100644 --- a/rt/lib/RT/User_Overlay.pm +++ b/rt/lib/RT/User_Overlay.pm @@ -916,6 +916,42 @@ sub _GenerateRandomNextChar { return ($i); } +sub SafeSetPassword { + my $self = shift; + my %args = ( + Current => undef, + New => undef, + Confirmation => undef, + @_, + ); + return (1) unless defined $args{'New'} && length $args{'New'}; + + my %cond = $self->CurrentUserRequireToSetPassword; + + unless ( $cond{'CanSet'} ) { + return (0, $self->loc('You can not set password.') .' '. $cond{'Reason'} ); + } + + my $error = ''; + if ( $cond{'RequireCurrent'} && !$self->CurrentUser->IsPassword($args{'Current'}) ) { + if ( defined $args{'Current'} && length $args{'Current'} ) { + $error = $self->loc("Please enter your current password correctly."); + } + else { + $error = $self->loc("Please enter your current password."); + } + } elsif ( $args{'New'} ne $args{'Confirmation'} ) { + $error = $self->loc("Passwords do not match."); + } + + if ( $error ) { + $error .= ' '. $self->loc('Password has not been set.'); + return (0, $error); + } + + return $self->SetPassword( $args{'New'} ); +} + =head3 SetPassword Takes a string. Checks the string's length and sets this user's password @@ -1034,7 +1070,7 @@ sub IsPassword { } # if it's a historical password we say ok. - if ($self->__Value('Password') eq crypt($value, $self->__Value('Password')) + if ($self->__Value('Password') eq crypt(encode_utf8($value), $self->__Value('Password')) or $self->_GeneratePasswordBase64($value) eq $self->__Value('Password')) { # ...but upgrade the legacy password inplace. @@ -1047,6 +1083,35 @@ sub IsPassword { return (undef); } +sub CurrentUserRequireToSetPassword { + my $self = shift; + + my %res = ( + CanSet => 1, + Reason => '', + RequireCurrent => 1, + ); + + if ( RT->Config->Get('WebExternalAuth') + && !RT->Config->Get('WebFallbackToInternalAuth') + ) { + $res{'CanSet'} = 0; + $res{'Reason'} = $self->loc("External authentication enabled."); + } + elsif ( !$self->CurrentUser->HasPassword ) { + if ( $self->CurrentUser->id == ($self->id||0) ) { + # don't require current password if user has no + $res{'RequireCurrent'} = 0; + } + else { + $res{'CanSet'} = 0; + $res{'Reason'} = $self->loc("Your password is not set."); + } + } + + return %res; +} + =head3 AuthToken Returns an authentication string associated with the user. This diff --git a/rt/lib/RT/Users_Overlay.pm b/rt/lib/RT/Users_Overlay.pm index 32fb393cf..ea12dbe7f 100644 --- a/rt/lib/RT/Users_Overlay.pm +++ b/rt/lib/RT/Users_Overlay.pm @@ -73,11 +73,10 @@ no warnings qw(redefine); sub _Init { my $self = shift; $self->{'table'} = 'Users'; - $self->{'primary_key'} = 'id'; + $self->{'primary_key'} = 'id'; + $self->{'with_disabled_column'} = 1; - - - my @result = $self->SUPER::_Init(@_); + my @result = $self->SUPER::_Init(@_); # By default, order by name $self->OrderBy( ALIAS => 'main', FIELD => 'Name', @@ -114,46 +113,41 @@ sub PrincipalsAlias { } -# {{{ sub _DoSearch - -=head2 _DoSearch +=head2 LimitToEnabled - A subclass of DBIx::SearchBuilder::_DoSearch that makes sure that _Disabled rows never get seen unless -we're explicitly trying to see them. +Only find items that haven\'t been disabled =cut -sub _DoSearch { +# XXX: should be generalized +sub LimitToEnabled { my $self = shift; - #unless we really want to find disabled rows, make sure we\'re only finding enabled ones. - unless ( $self->{'find_disabled_rows'} ) { - $self->LimitToEnabled(); - } - return ( $self->SUPER::_DoSearch(@_) ); - + $self->{'handled_disabled_column'} = 1; + $self->Limit( + ALIAS => $self->PrincipalsAlias, + FIELD => 'Disabled', + VALUE => '0', + ); } -# }}} -# {{{ sub LimitToEnabled - -=head2 LimitToEnabled +=head2 LimitToDeleted -Only find items that haven\'t been disabled +Only find items that have been deleted. =cut -# XXX: should be generalized -sub LimitToEnabled { +sub LimitToDeleted { my $self = shift; - - $self->Limit( ALIAS => $self->PrincipalsAlias, - FIELD => 'Disabled', - VALUE => '0', - OPERATOR => '=' ); + + $self->{'handled_disabled_column'} = $self->{'find_disabled_rows'} = 1; + $self->Limit( + ALIAS => $self->PrincipalsAlias, + FIELD => 'Disabled', + VALUE => 1, + ); } -# }}} # {{{ LimitToEmail @@ -378,7 +372,8 @@ sub WhoHaveRight { return (undef); } - my @from_role = $self->Clone->_WhoHaveRoleRightSplitted( %args ); + my $from_role = $self->Clone; + $from_role->WhoHaveRoleRight( %args ); my $from_group = $self->Clone; $from_group->WhoHaveGroupRight( %args ); @@ -387,8 +382,8 @@ sub WhoHaveRight { use DBIx::SearchBuilder 1.50; #no version on ::Union :( use DBIx::SearchBuilder::Union; my $union = new DBIx::SearchBuilder::Union; - $union->add( $_ ) foreach @from_role; $union->add( $from_group ); + $union->add( $from_role ); %$self = %$union; bless $self, ref($union); @@ -410,57 +405,14 @@ sub WhoHaveRoleRight @_ ); - my $groups = $self->_JoinGroups( %args ); - my $acl = $self->_JoinACL( %args ); - - $self->Limit( ALIAS => $acl, - FIELD => 'PrincipalType', - VALUE => "$groups.Type", - QUOTEVALUE => 0, - ); - - # no system user - $self->Limit( ALIAS => $self->PrincipalsAlias, - FIELD => 'id', - OPERATOR => '!=', - VALUE => $RT::SystemUser->id - ); - my @objects = $self->_GetEquivObjects( %args ); - unless ( @objects ) { - unless ( $args{'IncludeSystemRights'} ) { - $self->_AddSubClause( WhichObjects => "($acl.ObjectType != 'RT::System')" ); - } + my @roles = RT::Principal->RolesWithRight( %args ); + unless ( @roles ) { + $self->_AddSubClause( "WhichRole", "(main.id = 0)" ); return; } - my ($groups_clauses, $acl_clauses) = $self->_RoleClauses( $groups, $acl, @objects ); - $self->_AddSubClause( "WhichObject", "(". join( ' OR ', @$groups_clauses ) .")" ); - $self->_AddSubClause( "WhichRole", "(". join( ' OR ', @$acl_clauses ) .")" ); - - return; -} - -sub _WhoHaveRoleRightSplitted { - my $self = shift; - my %args = ( - Right => undef, - Object => undef, - IncludeSystemRights => undef, - IncludeSuperusers => undef, - IncludeSubgroupMembers => 1, - EquivObjects => [ ], - @_ - ); - my $groups = $self->_JoinGroups( %args ); - my $acl = $self->_JoinACL( %args ); - - $self->Limit( ALIAS => $acl, - FIELD => 'PrincipalType', - VALUE => "$groups.Type", - QUOTEVALUE => 0, - ); # no system user $self->Limit( ALIAS => $self->PrincipalsAlias, @@ -469,35 +421,21 @@ sub _WhoHaveRoleRightSplitted { VALUE => $RT::SystemUser->id ); - my @objects = $self->_GetEquivObjects( %args ); - unless ( @objects ) { - unless ( $args{'IncludeSystemRights'} ) { - $self->_AddSubClause( WhichObjects => "($acl.ObjectType != 'RT::System')" ); - } - return $self; - } + $self->_AddSubClause( "WhichRole", "(". join( ' OR ', map "$groups.Type = '$_'", @roles ) .")" ); - my ($groups_clauses, $acl_clauses) = $self->_RoleClauses( $groups, $acl, @objects ); - $self->_AddSubClause( "WhichRole", "(". join( ' OR ', @$acl_clauses ) .")" ); - - my @res; - foreach ( @$groups_clauses ) { - my $tmp = $self->Clone; - $tmp->_AddSubClause( WhichObject => $_ ); - push @res, $tmp; - } + my @groups_clauses = $self->_RoleClauses( $groups, @objects ); + $self->_AddSubClause( "WhichObject", "(". join( ' OR ', @groups_clauses ) .")" ) + if @groups_clauses; - return @res; + return; } sub _RoleClauses { my $self = shift; my $groups = shift; - my $acl = shift; my @objects = @_; my @groups_clauses; - my @acl_clauses; foreach my $obj ( @objects ) { my $type = ref($obj)? ref($obj): $obj; my $id; @@ -509,12 +447,8 @@ sub _RoleClauses { # field to integer and drop this quotes. $role_clause .= " AND $groups.Instance = '$id'" if $id; push @groups_clauses, "($role_clause)"; - - my $object_clause = "$acl.ObjectType = '$type'"; - $object_clause .= " AND $acl.ObjectId = $id" if $id; - push @acl_clauses, "($object_clause)"; } - return (\@groups_clauses, \@acl_clauses); + return @groups_clauses; } # XXX: should be generalized |