[Rt-commit] rt branch, 4.0/processacls-bulletproofing, created. rt-4.0.0rc6-14-g53191ff

[Thomas Sibley]

The branch, 4.0/processacls-bulletproofing has been created
        at  53191ff446a08ecfede37c50578ddb0d2eff1c3a (commit)

- Log -----------------------------------------------------------------
commit 9ecb06b948d638121c5e762809f0b64d1b30d626
Author: Thomas Sibley <trs at bestpractical.com>
Date:   Fri Mar 4 12:28:31 2011 -0500

    Merge rights if a principal is "added" that already exists
    
    This can happen if an admin misses the user/group in the list and types
    their name to give them rights.  Previously (and rather unintuitively)
    the existing rights were revoked and the newly added rights granted.
    
    See also [rt3 #16426].

diff --git a/lib/RT/Interface/Web.pm b/lib/RT/Interface/Web.pm
index 6205d70..63c511f 100644
--- a/lib/RT/Interface/Web.pm
+++ b/lib/RT/Interface/Web.pm
@@ -1713,8 +1713,20 @@ sub ProcessACLs {
         # Turn our addprincipal rights spec into a real one
         for my $arg (keys %$ARGSref) {
             next unless $arg =~ /^SetRights-addprincipal-(.+?-\d+)$/;
-            $ARGSref->{"SetRights-$principal_id-$1"} = $ARGSref->{$arg};
-            push @check, "$principal_id-$1";
+
+            my $tuple = "$principal_id-$1";
+            my $key   = "SetRights-$tuple";
+
+            # If we have it already, that's odd, but merge them
+            if (grep { $_ eq $tuple } @check) {
+                $ARGSref->{$key} = [
+                    (ref $ARGSref->{$key} eq 'ARRAY' ? @{$ARGSref->{$key}} : $ARGSref->{$key}),
+                    (ref $ARGSref->{$arg} eq 'ARRAY' ? @{$ARGSref->{$arg}} : $ARGSref->{$arg}),
+                ];
+            } else {
+                $ARGSref->{$key} = $ARGSref->{$arg};
+                push @check, $tuple;
+            }
         }
     }
 

commit 53191ff446a08ecfede37c50578ddb0d2eff1c3a
Author: Thomas Sibley <trs at bestpractical.com>
Date:   Fri Mar 4 12:33:08 2011 -0500

    Make sure our check list of principals is unique
    
    This is would be unexpected, but otherwise we unnecessarily check ACLs
    more than once.

diff --git a/lib/RT/Interface/Web.pm b/lib/RT/Interface/Web.pm
index 63c511f..4f56a94 100644
--- a/lib/RT/Interface/Web.pm
+++ b/lib/RT/Interface/Web.pm
@@ -69,6 +69,7 @@ use RT::Interface::Web::Menu;
 use RT::Interface::Web::Session;
 use Digest::MD5 ();
 use Encode qw();
+use List::MoreUtils qw();
 
 =head2 SquishedCSS $style
 
@@ -1742,7 +1743,7 @@ sub ProcessACLs {
         $state{$tuple} = { map { $_ => 1 } @rights };
     }
 
-    foreach my $tuple (@check) {
+    foreach my $tuple (List::MoreUtils::uniq @check) {
         next unless $tuple =~ /^(\d+)-(.+?)-(\d+)$/;
 
         my ( $principal_id, $object_type, $object_id ) = ( $1, $2, $3 );

-----------------------------------------------------------------------