[Rt-commit] rt branch, 3.9-double_acl_cache, created. rt-3.9.6-141-g0666190
Ruslan Zakirov
ruz at bestpractical.com
Wed Nov 24 20:07:29 EST 2010
The branch, 3.9-double_acl_cache has been created
at 0666190c8adac5db278ff4a103d843b586c90892 (commit)
- Log -----------------------------------------------------------------
commit 01449f7e2baf025d837482a45bff6d27a91a8967
Author: Ruslan Zakirov <ruz at bestpractical.com>
Date: Wed Nov 24 23:17:02 2010 +0300
move some functions into RT::Test
diff --git a/lib/RT/Test.pm b/lib/RT/Test.pm
index 1334c21..186562e 100644
--- a/lib/RT/Test.pm
+++ b/lib/RT/Test.pm
@@ -588,6 +588,18 @@ sub load_or_create_queue {
return $obj;
}
+sub delete_queue_watchers {
+ my $self = shift;
+ my @queues = @_;
+
+ foreach my $q ( @queues ) {
+ foreach my $t (qw(Cc AdminCc) ) {
+ $q->DeleteWatcher( Type => $t, PrincipalId => $_->MemberId )
+ foreach @{ $q->$t()->MembersObj->ItemsArrayRef };
+ }
+ }
+}
+
sub create_tickets {
local $Test::Builder::Level = $Test::Builder::Level + 1;
@@ -655,6 +667,21 @@ sub create_ticket {
return $ticket;
}
+sub delete_tickets {
+ my $self = shift;
+ my $query = shift;
+ my $tickets = RT::Tickets->new( RT->SystemUser );
+ if ( $query ) {
+ $tickets->FromSQL( $query );
+ }
+ else {
+ $tickets->UnLimit;
+ }
+ while ( my $ticket = $tickets->Next ) {
+ $ticket->Delete;
+ }
+}
+
=head2 load_or_create_custom_field
=cut
diff --git a/t/api/rights_show_ticket.t b/t/api/rights_show_ticket.t
index 3ce1026..62f62c4 100644
--- a/t/api/rights_show_ticket.t
+++ b/t/api/rights_show_ticket.t
@@ -239,24 +239,8 @@ sub create_tickets_set{
return @res;
}
-sub cleanup { delete_tickets(); delete_watchers() };
-
-sub delete_tickets {
- my $tickets = RT::Tickets->new( RT->SystemUser );
- $tickets->FromSQL( "Queue = $qa_id OR Queue = $qb_id" );
- while ( my $ticket = $tickets->Next ) {
- $ticket->Delete;
- }
-}
-
-sub delete_watchers {
- foreach my $q ($queue_a, $queue_b) {
- foreach my $u ($user_a, $user_b) {
- foreach my $t (qw(Cc AdminCc) ) {
- $q->DeleteWatcher( Type => $t, PrincipalId => $u->id )
- if $q->IsWatcher( Type => $t, PrincipalId => $u->id );
- }
- }
- }
-}
+sub cleanup {
+ RT::Test->delete_tickets( "Queue = $qa_id OR Queue = $qb_id" );
+ RT::Test->delete_queue_watchers( $queue_a, $queue_b );
+};
commit e65867eec12b53bad81f6c463b59c2f2d055b527
Author: Ruslan Zakirov <ruz at bestpractical.com>
Date: Thu Nov 25 04:02:02 2010 +0300
factor out query generators out HasRight and friends
diff --git a/lib/RT/Principal_Overlay.pm b/lib/RT/Principal_Overlay.pm
index 477662d..55289d0 100755
--- a/lib/RT/Principal_Overlay.pm
+++ b/lib/RT/Principal_Overlay.pm
@@ -352,20 +352,32 @@ sub _HasGroupRight {
EquivObjects => [],
@_
);
- my $right = $args{'Right'};
my $query
= "SELECT ACL.id, ACL.ObjectType, ACL.ObjectId "
- . "FROM ACL, Principals, CachedGroupMembers WHERE "
- .
+ . $self->_HasGroupRightQuery( %args );
- # Only find superuser or rights with the name $right
- "(ACL.RightName = 'SuperUser' "
- . ( $right ne 'SuperUser' ? "OR ACL.RightName = '$right'" : '' )
- . ") "
+ $self->_Handle->ApplyLimits( \$query, 1 );
+ my ( $hit, $obj, $id ) = $self->_Handle->FetchResult($query);
+ return (0) unless $hit;
+
+ $obj .= "-$id" if $id;
+ return ( 1, $obj );
+}
+
+sub _HasGroupRightQuery {
+ my $self = shift;
+ my %args = (
+ Right => undef,
+ EquivObjects => [],
+ @_
+ );
+
+ my $query
+ = "FROM ACL, Principals, CachedGroupMembers WHERE "
# Never find disabled groups.
- . "AND Principals.id = ACL.PrincipalId "
+ . "Principals.id = ACL.PrincipalId "
. "AND Principals.PrincipalType = 'Group' "
. "AND Principals.Disabled = 0 "
@@ -375,8 +387,7 @@ sub _HasGroupRight {
# as is the case when we want to look up group rights
. "AND CachedGroupMembers.GroupId = ACL.PrincipalId "
. "AND CachedGroupMembers.GroupId = Principals.id "
- . "AND CachedGroupMembers.MemberId = "
- . $self->Id . " "
+ . "AND CachedGroupMembers.MemberId = ". $self->Id . " "
. "AND CachedGroupMembers.Disabled = 0 ";
my @clauses;
@@ -393,13 +404,13 @@ sub _HasGroupRight {
if (@clauses) {
$query .= " AND (" . join( ' OR ', @clauses ) . ")";
}
-
- $self->_Handle->ApplyLimits( \$query, 1 );
- my ( $hit, $obj, $id ) = $self->_Handle->FetchResult($query);
- return (0) unless $hit;
-
- $obj .= "-$id" if $id;
- return ( 1, $obj );
+ if ( my $right = $args{'Right'} ) {
+ # Only find superuser or rights with the name $right
+ $query .= " AND (ACL.RightName = 'SuperUser' "
+ . ( $right ne 'SuperUser' ? "OR ACL.RightName = '$right'" : '' )
+ . ") ";
+ }
+ return $query;
}
sub _HasRoleRight {
@@ -413,7 +424,25 @@ sub _HasRoleRight {
return 0 unless @roles;
my $query = "SELECT Groups.id "
- . "FROM Groups, Principals, CachedGroupMembers WHERE "
+ . $self->_HasRoleRightQuery( %args, Roles => \@roles );
+
+ $self->_Handle->ApplyLimits( \$query, 1 );
+ my ($hit) = $self->_Handle->FetchResult($query);
+ return (1) if $hit;
+
+ return 0;
+}
+
+sub _HasRoleRightQuery {
+ my $self = shift;
+ my %args = ( Right => undef,
+ EquivObjects => [],
+ Roles => undef,
+ @_
+ );
+
+ my $query =
+ " FROM Groups, Principals, CachedGroupMembers WHERE "
# Never find disabled things
. "Principals.Disabled = 0 " . "AND CachedGroupMembers.Disabled = 0 "
@@ -428,8 +457,11 @@ sub _HasRoleRight {
# as is the case when we want to look up group rights
. "AND Principals.id = CachedGroupMembers.GroupId "
. "AND CachedGroupMembers.MemberId = " . $self->Id . " "
+ ;
- . "AND (" . join( ' OR ', map "Groups.Type = '$_'", @roles ) . ")";
+ if ( $args{'Roles'} ) {
+ $query .= "AND (" . join( ' OR ', map "Groups.Type = '$_'", @{ $args{'Roles'} } ) . ")";
+ }
my (@object_clauses);
foreach my $obj ( @{ $args{'EquivObjects'} } ) {
@@ -446,12 +478,7 @@ sub _HasRoleRight {
push @object_clauses, "($clause)";
}
$query .= " AND (" . join( ' OR ', @object_clauses ) . ")";
-
- $self->_Handle->ApplyLimits( \$query, 1 );
- my ($hit) = $self->_Handle->FetchResult($query);
- return (1) if $hit;
-
- return 0;
+ return $query;
}
=head2 RolesWithRight
@@ -481,17 +508,40 @@ sub RolesWithRight {
return () if $args{'Right'} eq 'ExecuteCode'
and RT->Config->Get('DisallowExecuteCode');
- my $query = "SELECT DISTINCT PrincipalType FROM ACL"
+ my $query = "SELECT DISTINCT PrincipalType "
+ . $self->_RolesWithRightQuery( %args );
+
+ my $roles = $RT::Handle->dbh->selectcol_arrayref($query);
+ unless ($roles) {
+ $RT::Logger->warning( $RT::Handle->dbh->errstr );
+ return ();
+ }
+ return @$roles;
+}
- # Only find superuser or rights with the requested right
- . " WHERE ( RightName = '" . $args{'Right'} . "' "
+sub _RolesWithRightQuery {
+ my $self = shift;
+ my %args = ( Right => undef,
+ IncludeSystemRights => 1,
+ IncludeSuperusers => 1,
+ EquivObjects => [],
+ @_
+ );
- # Check SuperUser if we were asked to
- . ( $args{'IncludeSuperusers'} ? "OR RightName = 'SuperUser' " : '' )
- . ")"
+ my $query = " FROM ACL WHERE"
# we need only roles
- . " AND PrincipalType != 'Group'";
+ . " PrincipalType != 'Group'";
+
+ if ( my $right = $args{'Right'} ) {
+ $query .=
+ # Only find superuser or rights with the requested right
+ " AND ( RightName = '$right' "
+
+ # Check SuperUser if we were asked to
+ . ( $args{'IncludeSuperusers'} ? "OR RightName = 'SuperUser' " : '' )
+ . ")";
+ }
# skip rights granted on system level if we were asked to
unless ( $args{'IncludeSystemRights'} ) {
@@ -513,19 +563,10 @@ sub RolesWithRight {
$query .= " AND (" . join( ' OR ', @object_clauses ) . ")"
if @object_clauses;
- my $roles = $RT::Handle->dbh->selectcol_arrayref($query);
- unless ($roles) {
- $RT::Logger->warning( $RT::Handle->dbh->errstr );
- return ();
- }
- return @$roles;
+ return $query;
}
-
-
-
-
=head2 InvalidateACLCache
Cleans out and reinitializes the user rights cache
commit 6fd6943d47728e376c394c5755d1b8b7f7c9aba4
Author: Ruslan Zakirov <ruz at bestpractical.com>
Date: Thu Nov 25 04:02:45 2010 +0300
Principal->HasRights method
diff --git a/lib/RT/Principal_Overlay.pm b/lib/RT/Principal_Overlay.pm
index 55289d0..12e30a6 100755
--- a/lib/RT/Principal_Overlay.pm
+++ b/lib/RT/Principal_Overlay.pm
@@ -323,6 +323,78 @@ sub HasRight {
return ($hitcount);
}
+sub HasRights {
+ my $self = shift;
+ my %args = (
+ Object => undef,
+ EquivObjects => undef,
+ @_
+ );
+ return {} if $self->__Value('Disabled');
+
+ my $object = $args{'Object'};
+ unless ( eval { $object->id } ) {
+ $RT::Logger->crit("HasRights called with no valid object");
+ }
+
+ my $cache_key = $self->id .';:;'. ref($object) .'-'. $object->id;
+ my $cached = $_ACL_CACHE->fetch($cache_key);
+ return $cached if $cached;
+
+ push @{ $args{'EquivObjects'} }, $object;
+ unshift @{ $args{'EquivObjects'} },
+ $args{'Object'}->ACLEquivalenceObjects;
+ unshift @{ $args{'EquivObjects'} }, $RT::System
+ unless $self->can('_IsOverrideGlobalACL')
+ && $self->_IsOverrideGlobalACL( $object );
+
+ my %res = ();
+ {
+ my $query
+ = "SELECT DISTINCT ACL.RightName "
+ . $self->_HasGroupRightQuery(
+ EquivObjects => $args{'EquivObjects'}
+ );
+ my $rights = $RT::Handle->dbh->selectcol_arrayref($query);
+ unless ($rights) {
+ $RT::Logger->warning( $RT::Handle->dbh->errstr );
+ return ();
+ }
+ $res{$_} = 1 foreach @$rights;
+ }
+ my $roles;
+ {
+ my $query
+ = "SELECT DISTINCT Groups.Type "
+ . $self->_HasRoleRightQuery(
+ EquivObjects => $args{'EquivObjects'}
+ );
+ $roles = $RT::Handle->dbh->selectcol_arrayref($query);
+ unless ($roles) {
+ $RT::Logger->warning( $RT::Handle->dbh->errstr );
+ return ();
+ }
+ }
+ if ( @$roles ) {
+ my $query
+ = "SELECT DISTINCT ACL.RightName "
+ . $self->_RolesWithRightQuery(
+ EquivObjects => $args{'EquivObjects'}
+ )
+ . ' AND ('. join( ' OR ', map "PrincipalType = '$_'", @$roles ) .')'
+ ;
+ my $rights = $RT::Handle->dbh->selectcol_arrayref($query);
+ unless ($rights) {
+ $RT::Logger->warning( $RT::Handle->dbh->errstr );
+ return ();
+ }
+ $res{$_} = 1 foreach @$rights;
+ }
+
+ $_ACL_CACHE->store( $cache_key, \%res );
+ return \%res;
+}
+
=head2 _HasRight
Low level HasRight implementation, use HasRight method instead.
commit bd7c5b6d9cecb9b1ef6609dad4bba2afa208bf74
Author: Ruslan Zakirov <ruz at bestpractical.com>
Date: Thu Nov 25 04:03:29 2010 +0300
use new cache entry in HasRight
diff --git a/lib/RT/Principal_Overlay.pm b/lib/RT/Principal_Overlay.pm
index 12e30a6..bd07048 100755
--- a/lib/RT/Principal_Overlay.pm
+++ b/lib/RT/Principal_Overlay.pm
@@ -286,6 +286,14 @@ sub HasRight {
return (undef);
}
+ {
+ my $cached = $_ACL_CACHE->fetch(
+ $self->id .';:;'. ref($args{'Object'}) .'-'. $args{'Object'}->id
+ );
+ return $cached->{'SuperUser'} || $cached->{ $args{'Right'} }
+ if $cached;
+ }
+
unshift @{ $args{'EquivObjects'} },
$args{'Object'}->ACLEquivalenceObjects;
commit 7d891dc81eb9085bfdb6fa284cece98d263b8c85
Author: Ruslan Zakirov <ruz at bestpractical.com>
Date: Thu Nov 25 04:05:02 2010 +0300
fill ACL cache when ticket is about to be displayed
diff --git a/share/html/Ticket/Display.html b/share/html/Ticket/Display.html
index 06e2272..bb31114 100755
--- a/share/html/Ticket/Display.html
+++ b/share/html/Ticket/Display.html
@@ -134,6 +134,8 @@ if ($ARGS{'id'} eq 'new') {
} else {
$TicketObj ||= LoadTicket($ARGS{'id'});
+ $TicketObj->CurrentUser->PrincipalObj->HasRights( Object => $TicketObj );
+
$m->callback( CallbackName => 'BeforeProcessArguments',
TicketObj => $TicketObj, Tickets => $Tickets,
ActionsRef => \@Actions, ARGSRef => \%ARGS );
commit 23615bf43358e6ae524cea4ece86609199d815e2
Author: Ruslan Zakirov <ruz at bestpractical.com>
Date: Thu Nov 25 04:05:55 2010 +0300
use HasRights in tabs
diff --git a/share/html/Elements/Tabs b/share/html/Elements/Tabs
index 5152d55..f3725f6 100755
--- a/share/html/Elements/Tabs
+++ b/share/html/Elements/Tabs
@@ -323,39 +323,28 @@ if ( $request_path !~ qr{^/SelfService/} ) {
$tabs->child( history => title => loc('History') => path => "/Ticket/History.html?id=" . $id );
- my %can = (
- ModifyTicket => $obj->CurrentUserHasRight('ModifyTicket'),
- Watch => $obj->CurrentUserHasRight('Watch'),
- WatchAsAdminCc => $obj->CurrentUserHasRight('WatchAsAdminCc'),
- OwnTicket => $obj->CurrentUserHasRight('OwnTicket'),
- StealTicket => $obj->CurrentUserHasRight('StealTicket'),
- TakeTicket => $obj->CurrentUserHasRight('TakeTicket'),
- ReplyToTicket => $obj->CurrentUserHasRight('ReplyToTicket'),
- CommentOnTicket => $obj->CurrentUserHasRight('CommentOnTicket'),
- ModifyCustomField => $obj->CurrentUserHasRight('ModifyCustomField'),
- _ModifyOwner => (
- $obj->CurrentUserHasRight('OwnTicket')
- || $obj->CurrentUserHasRight('TakeTicket')
- || $obj->CurrentUserHasRight('StealTicket')
- )
- );
+ my %can = %{ $obj->CurrentUser->PrincipalObj->HasRights( Object => $obj ) };
+ $can{'_ModifyOwner'} = $can{'OwnTicket'} || $can{'TakeTicket'} || $can{'StealTicket'};
+ my $can = sub {
+ $can{$_[0]} || $can{'SuperUser'}
+ };
# comment out until we can do it for an individual custom field
- #if ( $can{ModifyTicket} || $can{ModifyCustomField} ) {
+ #if ( $can->('ModifyTicket') || $can->('ModifyCustomField') ) {
$tabs->child( basics => title => loc('Basics'), path => "/Ticket/Modify.html?id=" . $id, );
#}
- if ( $can{ModifyTicket} || $can{_ModifyOwner} || $can{Watch} || $can{WatchAsAdminCc} ) {
+ if ( $can->('ModifyTicket') || $can->('_ModifyOwner') || $can->('Watch') || $can->('WatchAsAdminCc') ) {
$tabs->child( people => title => loc('People'), path => "/Ticket/ModifyPeople.html?id=" . $id,);
}
- if ( $can{ModifyTicket} ) {
+ if ( $can->('ModifyTicket') ) {
$tabs->child( dates => title => loc('Dates'), path => "/Ticket/ModifyDates.html?id=" . $id, );
$tabs->child( links => title => loc('Links'), path => "/Ticket/ModifyLinks.html?id=" . $id, );
}
- #if ( $can{ModifyTicket} || $can{ModifyCustomField} || $can{_ModifyOwner} ) {
+ #if ( $can->('ModifyTicket') || $can->('ModifyCustomField') || $can->('_ModifyOwner') ) {
$tabs->child( jumbo => title => loc('Jumbo'), path => "/Ticket/ModifyAll.html?id=" . $id, );
#}
@@ -363,15 +352,15 @@ if ( $request_path !~ qr{^/SelfService/} ) {
$tabs->child( reminders => title => loc('Reminders'), path => "/Ticket/Reminders.html?id=" . $id, );
}
- if ( $can{'ModifyTicket'} or $can{'ReplyToTicket'} ) {
+ if ( $can->('ModifyTicket') or $can->('ReplyToTicket') ) {
$actions->child( reply => title => loc('Reply'), path => "/Ticket/Update.html?Action=Respond;id=" . $id,);
}
- if ( $obj->CurrentUserHasRight('ForwardMessage') ) {
+ if ( $can->('ForwardMessage') ) {
$actions->child( forward => title => loc('Forward'), path => "/Ticket/Forward.html?id=" . $id, );
}
- if ( $can{'ModifyTicket'} ) {
+ if ( $can->('ModifyTicket') ) {
my $current = $obj->Status;
my $lifecycle = $obj->QueueObj->lifecycle;
my $i = 1;
@@ -381,8 +370,7 @@ if ( $request_path !~ qr{^/SelfService/} ) {
next unless $lifecycle->is_transition( $current => $next );
my $check = $lifecycle->check_right( $current => $next );
- $can{$check} = $obj->CurrentUserHasRight($check) unless exists $can{$check};
- next unless $can{$check};
+ next unless $can->($check);
my $action = $info->{'update'} || '';
@@ -402,20 +390,20 @@ if ( $request_path !~ qr{^/SelfService/} ) {
}
}
- if ( $can{OwnTicket} ) {
+ if ( $can->('OwnTicket') ) {
if ( $obj->OwnerObj->Id == RT->Nobody->id
- && ( $can{'ModifyTicket'} or $can{'TakeTicket'} ) ) {
+ && ( $can->('ModifyTicket') or $can->('TakeTicket') ) ) {
$actions->child( take => path => "/Ticket/Display.html?Action=Take;id=" . $id, title => loc('Take'));
}
elsif ( $obj->OwnerObj->id != RT->Nobody->id
&& $obj->OwnerObj->id != $session{CurrentUser}->id
- && ( $can{'ModifyTicket'} or $can{'StealTicket'} ) ) {
+ && ( $can->('ModifyTicket') or $can->('StealTicket') ) ) {
$actions->child( steal => title => loc('Steal'), path => "/Ticket/Display.html?Action=Steal;id=" . $id,);
}
}
- if ( $can{'ModifyTicket'} || $can{'CommentOnTicket'} ) {
+ if ( $can->('ModifyTicket') || $can->('CommentOnTicket') ) {
$actions->child( comment => title => loc('Comment'), path => "/Ticket/Update.html?Action=Comment;id=" . $id);
}
commit 0666190c8adac5db278ff4a103d843b586c90892
Author: Ruslan Zakirov <ruz at bestpractical.com>
Date: Thu Nov 25 04:06:27 2010 +0300
simple tests for HasRights
diff --git a/t/api/has_rights.t b/t/api/has_rights.t
new file mode 100644
index 0000000..d87704f
--- /dev/null
+++ b/t/api/has_rights.t
@@ -0,0 +1,43 @@
+use RT::Test nodata => 1, tests => 7;
+
+use strict;
+use warnings;
+
+my $queue = RT::Test->load_or_create_queue( Name => 'A' );
+ok $queue && $queue->id, 'loaded or created queue_a';
+my $qid = $queue->id;
+
+my $user = RT::Test->load_or_create_user(
+ Name => 'user',
+ Password => 'password',
+ EmailAddress => 'test at example.com',
+);
+ok $user && $user->id, 'loaded or created user';
+
+{
+ cleanup();
+ RT::Test->set_rights(
+ { Principal => 'Everyone', Right => [qw(SeeQueue)] },
+ { Principal => 'Cc', Right => [qw(ShowTicket)] },
+ );
+ my ($t) = RT::Test->create_tickets(
+ { Queue => $queue->id },
+ { },
+ );
+ my $rights = $user->PrincipalObj->HasRights( Object => $t );
+ is_deeply( $rights, { SeeQueue => 1 }, 'got it' );
+
+ ($t) = RT::Test->create_tickets(
+ { Queue => $queue->id },
+ { Cc => $user->EmailAddress },
+ );
+ ok($t->Cc->HasMember( $user->id ), 'user is cc');
+ $rights = $user->PrincipalObj->HasRights( Object => $t );
+ is_deeply( $rights, { SeeQueue => 1, ShowTicket => 1 }, 'got it' )
+}
+
+sub cleanup {
+ RT::Test->delete_tickets( "Queue = $qid" );
+ RT::Test->delete_queue_watchers( $queue );
+};
+
-----------------------------------------------------------------------
More information about the Rt-commit
mailing list