[Rt-commit] r18941 - in rt/3.999/trunk/lib/RT: .

ruz at bestpractical.com ruz at bestpractical.com
Thu Mar 26 04:55:41 EDT 2009


Author: ruz
Date: Thu Mar 26 04:55:41 2009
New Revision: 18941

Modified:
   rt/3.999/trunk/lib/RT/Bootstrap.pm
   rt/3.999/trunk/lib/RT/IsPrincipal.pm
   rt/3.999/trunk/lib/RT/Model/ACE.pm
   rt/3.999/trunk/lib/RT/Model/ACECollection.pm
   rt/3.999/trunk/lib/RT/Model/Group.pm
   rt/3.999/trunk/lib/RT/Model/Principal.pm
   rt/3.999/trunk/lib/RT/Model/User.pm

Log:
* principal_id -> principal on ACE
* load_by_values -> load_by_cols, it was a mistake
* add acl_equivalence_group to IsPrincipal role
  so we can avoid checking type of the object
* canonicalize_principal -> principal_to_acl_group
* _createacl_equivalence_group -> create_acl_equivalence

Modified: rt/3.999/trunk/lib/RT/Bootstrap.pm
==============================================================================
--- rt/3.999/trunk/lib/RT/Bootstrap.pm	(original)
+++ rt/3.999/trunk/lib/RT/Bootstrap.pm	Thu Mar 26 04:55:41 2009
@@ -111,11 +111,11 @@
         else {
             my $ace = RT::Model::ACE->new( current_user => RT->system_user );
             my ( $val, $msg ) = $ace->_bootstrap_create(
-                principal_id   => acl_equiv_group_id( RT->system_user->id ),
-                type => 'Group',
-                right_name     => 'SuperUser',
-                object_type    => 'RT::System',
-                object_id      => 1,
+                principal   => acl_equiv_group_id( RT->system_user->id ),
+                type        => 'Group',
+                right_name  => 'SuperUser',
+                object_type => 'RT::System',
+                object_id   => 1,
             );
             return ( $val, $msg ) unless $val;
         }

Modified: rt/3.999/trunk/lib/RT/IsPrincipal.pm
==============================================================================
--- rt/3.999/trunk/lib/RT/IsPrincipal.pm	(original)
+++ rt/3.999/trunk/lib/RT/IsPrincipal.pm	Thu Mar 26 04:55:41 2009
@@ -24,6 +24,22 @@
     return shift->principal->disabled;
 }
 
+=head2 acl_equivalence_group
+
+=cut
+
+sub acl_equivalence_group {
+    my $self = shift;
+
+    my $res = RT::Model::Group->new( current_user => $self->current_user );
+    $res->load_acl_equivalence( $self );
+    unless ( $res->id ) {
+        Jifty->log->fatal( "No ACL equiv group for principal #". $self->id );
+        return (undef, _("No ACL equiv group for principal #%1", $self->id));
+    }
+    return $res;
+}
+
 =head2 principal 
 
 Returns L<RT::Model::Principal/|"the principal object"> for this record.

Modified: rt/3.999/trunk/lib/RT/Model/ACE.pm
==============================================================================
--- rt/3.999/trunk/lib/RT/Model/ACE.pm	(original)
+++ rt/3.999/trunk/lib/RT/Model/ACE.pm	Thu Mar 26 04:55:41 2009
@@ -75,14 +75,18 @@
 
 use Jifty::DBI::Schema;
 use Jifty::DBI::Record schema {
-    column
-        type => max_length is 25,
-        type is 'varchar(25)', default is '';
-    column principal_id => references RT::Model::Principal;
+    column type =>
+        max_length is 25,
+        type is 'varchar(25)',
+        default is ''
+    ;
+    column principal => references RT::Model::Principal;
     column right_name => max_length is 25, type is 'varchar(25)', is mandatory;
-    column
-        object_type => max_length is 25,
-        type is 'varchar(25)', default is '';
+    column object_type =>
+        type is 'varchar(25)',
+        max_length is 25,
+        default is '',
+    ;
     column object_id => type is 'int', default is '0';
 };
 
@@ -113,74 +117,67 @@
 
 
 
-=head2 load_by_values PARAMHASH
+=head2 load_by_cols PARAMHASH
 
 Load an ACE by specifying a paramhash with the following fields:
 
-              principal_id => undef,
-              type => undef,
-	      right_name => undef,
+    principal  => undef,
+    type       => undef,
+    right_name => undef,
 
-        And either:
+And either:
 
-	      object => undef,
+    object => undef,
 
-            OR
+or
 
-	      object_type => undef,
-	      object_id => undef
+    object_type => undef,
+    object_id   => undef
 
 =cut
 
-sub load_by_values {
+sub load_by_cols {
     my $self = shift;
     my %args = (
-        principal_id   => undef,
-        type => undef,
-        right_name     => undef,
-        object         => undef,
-        object_id      => undef,
-        object_type    => undef,
+        principal   => undef,
+        type        => undef,
+        right_name  => undef,
+        object      => undef,
+        object_id   => undef,
+        object_type => undef,
         @_
     );
 
-    my $princ_obj;
-    ( $princ_obj, $args{'type'} ) = $self->canonicalize_principal( $args{'principal_id'}, $args{'type'} );
-
-    unless ( $princ_obj->id ) {
-        return ( 0, _( 'Principal %1 not found.', $args{'principal_id'} ) );
-    }
-
     my ( $object, $object_type, $object_id ) = $self->_parse_object_arg(%args);
     unless ($object) {
         return ( 0, _("System error. Right not granted.") );
     }
 
+    my ($acl_group, $msg) = $self->principal_to_acl_group( $args{'principal'} );
+    unless ( $acl_group ) {
+        return ( 0, $msg );
+    }
+
     $self->load_by_cols(
-        principal_id   => $princ_obj->id,
-        type => $args{'type'},
-        right_name     => $args{'right_name'},
-        object_type    => $object_type,
-        object_id      => $object_id
+        principal   => $acl_group->id,
+        type        => $args{'type'} || 'Group',
+        right_name  => $args{'right_name'},
+        object_type => $object_type,
+        object_id   => $object_id
     );
-
-    #If we couldn't load it.
     unless ( $self->id ) {
         return ( 0, _("ACE not found") );
     }
 
     # if we could
     return ( $self->id, _("Right Loaded") );
-
 }
 
-
-
 =head2 create <PARAMS>
 
 PARAMS is a parameter hash with the following elements:
 
-   principal_id => The id of an RT::Model::Principal object
+   principal => The id of an RT::Model::Principal object
    type => "User" "Group" or any Role type
    right_name => the name of a right. in any case
 
@@ -206,10 +203,10 @@
 sub create {
     my $self = shift;
     my %args = (
-        principal_id   => undef,
-        type => undef,
-        right_name     => undef,
-        object         => undef,
+        principal  => undef,
+        type       => undef,
+        right_name => undef,
+        object     => undef,
         @_
     );
 
@@ -217,7 +214,7 @@
         return ( 0, _('No right specified') );
     }
 
-    #if we haven't specified any sort of right, we're talking about a global right
+    #if we haven't specified any sort of object, we're talking about a global right
     if (   !defined $args{'object'}
         && !defined $args{'object_id'}
         && !defined $args{'object_type'} )
@@ -229,19 +226,14 @@
         return ( 0, _("System error. Right not granted.") );
     }
 
-    # {{{ Validate the principal
-    my $princ_obj;
-    ( $princ_obj, $args{'type'} ) = $self->canonicalize_principal( $args{'principal_id'}, $args{'type'} );
-
-    unless ( $princ_obj->id ) {
-        return ( 0, _( 'Principal %1 not found.', $args{'principal_id'} ) );
+    my ($acl_group, $msg) = $self->principal_to_acl_group( $args{'principal'} );
+    unless ( $acl_group ) {
+        return ( 0, $msg );
     }
 
-    # }}}
-
     # {{{ Check the ACL
 
-    if ( ref( $args{'object'} ) eq 'RT::Model::Group' ) {
+    if ( $args{'object'}->isa('RT::Model::Group') ) {
         unless (
             $self->current_user->has_right(
                 object => $args{'object'},
@@ -292,22 +284,22 @@
 
     # Make sure the right doesn't already exist.
     $self->load_by_cols(
-        principal_id   => $princ_obj->id,
-        type => $args{'type'},
-        right_name     => $args{'right_name'},
-        object_type    => $args{'object_type'},
-        object_id      => $args{'object_id'},
+        principal   => $acl_group->id,
+        type        => $args{'type'},
+        right_name  => $args{'right_name'},
+        object_type => $args{'object_type'},
+        object_id   => $args{'object_id'},
     );
     if ( $self->id ) {
         return ( 0, _('That principal already has that right') );
     }
 
     my $id = $self->SUPER::create(
-        principal_id   => $princ_obj->id,
-        type => $args{'type'},
-        right_name     => $args{'right_name'},
-        object_type    => ref( $args{'object'} ),
-        object_id      => $args{'object'}->id,
+        principal   => $acl_group->id,
+        type        => $args{'type'},
+        right_name  => $args{'right_name'},
+        object_type => ref( $args{'object'} ),
+        object_id   => $args{'object'}->id,
     );
 
     #Clear the key cache. TODO someday we may want to just clear a little bit of the keycache space.
@@ -396,7 +388,7 @@
         my $user = RT::Model::User->new( current_user => $self->current_user );
         $user->load( $args{'UserId'} );
         delete $args{'UserId'};
-        $args{'principal_id'}   = $user->principal_id;
+        $args{'principal'}   = $user->principal_id;
         $args{'type'} = 'User';
     }
 
@@ -491,44 +483,28 @@
     }
 }
 
-=head2 canonicalize_principal (principal_id, type)
+=head2 principal_to_acl_group
 
-Takes a principal id and an optional principal type.
-
-If the principal is a user, resolves it to the proper acl equivalence group.
-Returns a tuple of  (RT::Model::Principal, type)  for the principal we really want to work with
+Takes a principal either an object or id. Resolves it to the proper acl
+equivalence group. Returns a tuple of (L<RT::Model::Group>, message). On
+errors object is empty and message is the error.
 
 =cut
 
-sub canonicalize_principal {
-    my $self       = shift;
-    my $princ_id   = shift;
-    my $princ_type = shift || 'Group';
-
-    my $princ_obj = RT::Model::Principal->new( current_user => RT->system_user );
-    $princ_obj->load($princ_id);
-
-    unless ( $princ_obj->id ) {
-        use Carp;
-        Jifty->log->fatal(Carp::cluck);
-        Jifty->log->fatal("Can't load a principal for id $princ_id");
-        return ( $princ_obj, undef );
-    }
-
-    # rights never get granted to users. they get granted to their
-    # ACL equivalence groups
-    if ( $princ_type eq 'User' ) {
-        my $equiv_group = RT::Model::Group->new( current_user => $self->current_user );
-        $equiv_group->load_acl_equivalence($princ_obj);
-        unless ( $equiv_group->id ) {
-            Jifty->log->fatal( "No ACL equiv group for princ " . $princ_obj->id );
-            return ( RT::Model::Principal->new( current_user => RT->system_user ), undef );
-        }
-        $princ_obj  = $equiv_group->principal();
-        $princ_type = 'Group';
+sub principal_to_acl_group {
+    my $self = shift;
+    my $principal = shift;
 
+    return $principal->acl_equivalence_group
+        if blessed $principal;
+
+    my $tmp = RT::Model::Principal->new( $self->current_user );
+    $tmp->load( $principal );
+    unless ( $tmp->id ) {
+        return (undef, _( 'Principal %1 not found.', $principal ));
+        return undef;
     }
-    return ( $princ_obj, $princ_type );
+    return $principal->acl_equivalence_group;
 }
 
 sub _parse_object_arg {
@@ -544,7 +520,7 @@
         Jifty->log->fatal( "Method called with an object_type or an object_id and object args" );
         return ();
     } elsif ( $args{'object'} && !UNIVERSAL::can( $args{'object'}, 'id' ) ) {
-        Jifty->log->fatal("Method called called object that has no id method");
+        Jifty->log->fatal("Method called with object that has no id method");
         return ();
     } elsif ( $args{'object'} ) {
         my $obj = $args{'object'};

Modified: rt/3.999/trunk/lib/RT/Model/ACECollection.pm
==============================================================================
--- rt/3.999/trunk/lib/RT/Model/ACECollection.pm	(original)
+++ rt/3.999/trunk/lib/RT/Model/ACECollection.pm	Thu Mar 26 04:55:41 2009
@@ -105,7 +105,6 @@
         entry_aggregator => 'OR',
         quote_value      => 0
     );
-
 }
 
 
@@ -170,7 +169,7 @@
         my $cgm = $self->new_alias('CachedGroupMembers');
         $self->join(
             alias1  => 'main',
-            column1 => 'principal_id',
+            column1 => 'principal',
             alias2  => $cgm,
             column2 => 'group_id'
         );
@@ -203,7 +202,7 @@
             $args{'id'} = $group->principal_id;
         }
         $self->limit(
-            column           => 'principal_id',
+            column           => 'principal',
             operator         => '=',
             value            => $args{'id'},
             entry_aggregator => 'OR'

Modified: rt/3.999/trunk/lib/RT/Model/Group.pm
==============================================================================
--- rt/3.999/trunk/lib/RT/Model/Group.pm	(original)
+++ rt/3.999/trunk/lib/RT/Model/Group.pm	Thu Mar 26 04:55:41 2009
@@ -226,7 +226,6 @@
 
     return $self->load_by_cols(
         domain   => 'ACLEquivalence',
-        type     => 'UserEquiv',
         instance => $principal,
     );
 }
@@ -405,7 +404,7 @@
 
 
 
-=head2 _createacl_equivalence_group { Principal }
+=head2 create_acl_equivalence { Principal }
 
 A helper subroutine which creates a group containing only 
 an individual user. This gets used by the ACL system to check rights.
@@ -415,7 +414,7 @@
 
 =cut
 
-sub _createacl_equivalence_group {
+sub create_acl_equivalence {
     my $self  = shift;
     my $princ = shift;
     my ( $id, $msg ) = $self->_create(
@@ -1020,6 +1019,7 @@
 
 }
 
+sub acl_equivalence_group { return $_[0] }
 
 sub basic_columns {
     ( [ name => 'name' ], [ description => 'description' ], );

Modified: rt/3.999/trunk/lib/RT/Model/Principal.pm
==============================================================================
--- rt/3.999/trunk/lib/RT/Model/Principal.pm	(original)
+++ rt/3.999/trunk/lib/RT/Model/Principal.pm	Thu Mar 26 04:55:41 2009
@@ -157,10 +157,10 @@
     # If it's a user, we really want to grant the right to their
     # user equivalence group
     return $ace->create(
-        right_name    => $args{'right'},
-        object        => $args{'object'},
-        type          => $type,
-        principal_id  => $self->id,
+        right_name => $args{'right'},
+        object     => $args{'object'},
+        type       => $type,
+        principal  => $self,
     );
 }
 
@@ -198,23 +198,23 @@
     my $type = $self->_get_principal_type_for_acl();
 
     my $ace = RT::Model::ACE->new( current_user => $self->current_user );
-    $ace->load_by_values(
-        right_name     => $args{'right'},
-        object         => $args{'object'},
-        type => $type,
-        principal_id   => $self->id
+    $ace->load_by_cols(
+        right_name => $args{'right'},
+        object     => $args{'object'},
+        type       => $type,
+        principal  => $self,
     );
 
     unless ( $ace->id ) {
         return ( 0, _("ACE not found") );
     }
-    return ( $ace->delete );
+    return $ace->delete;
 }
 
 
 
 
-=head2 sub has_right (right => 'right' object => undef)
+=head2 has_right (right => 'right' object => undef)
 
 
 Checks to see whether this principal has the right "right" for the object
@@ -359,13 +359,13 @@
         "(ACL.right_name = 'SuperUser' OR ACL.right_name = '$right') "
 
         # Never find disabled groups.
-        . "AND Principals.id = ACL.principal_id " . "AND Principals.type = 'Group' " . "AND Principals.disabled = 0 "
+        . "AND Principals.id = ACL.principal " . "AND Principals.type = 'Group' " . "AND Principals.disabled = 0 "
 
         # See if the principal is a member of the group recursively or _is the rightholder_
         # never find recursively disabled group members
         # also, check to see if the right is being granted _directly_ to this principal,
         #  as is the case when we want to look up group rights
-        . "AND CachedGroupMembers.group_id  = ACL.principal_id "
+        . "AND CachedGroupMembers.group_id  = ACL.principal "
         . "AND CachedGroupMembers.group_id  = Principals.id "
         . "AND CachedGroupMembers.member_id = "
         . $self->id . " "
@@ -496,6 +496,8 @@
 }
 
 
+sub acl_equivalence_group { $_[0]->object->acl_equivalence_group }
+
 
 =head2 _reference_id
 

Modified: rt/3.999/trunk/lib/RT/Model/User.pm
==============================================================================
--- rt/3.999/trunk/lib/RT/Model/User.pm	(original)
+++ rt/3.999/trunk/lib/RT/Model/User.pm	Thu Mar 26 04:55:41 2009
@@ -245,7 +245,7 @@
     }
 
     my $aclstash = RT::Model::Group->new( current_user => $self->current_user );
-    my $stash_id = $aclstash->_createacl_equivalence_group($principal);
+    my $stash_id = $aclstash->create_acl_equivalence($principal);
 
     unless ($stash_id) {
         Jifty->handle->rollback();
@@ -460,7 +460,7 @@
 
     my $aclstash = RT::Model::Group->new( current_user => $self->current_user );
 
-    my $stash_id = $aclstash->_createacl_equivalence_group($principal);
+    my $stash_id = $aclstash->create_acl_equivalence($principal);
 
     unless ($stash_id) {
         Jifty->handle->rollback();


More information about the Rt-commit mailing list