[Rt-commit] rt branch, 4.0/group-dashboards, updated. rt-4.0.1-136-g469aff8

Alex Vandiver alexmv at bestpractical.com
Wed Jul 20 20:56:10 EDT 2011


The branch, 4.0/group-dashboards has been updated
       via  469aff87d02d1fa6115c55cf93f5350274e3be96 (commit)
       via  d13be25f579c898303e5e027498bce07597ce79d (commit)
       via  415d6c197467f1e21d81bea2f2486b00b356d17c (commit)
      from  2e85388e291dffa14b9817d0bb1305d1b4be5b0f (commit)

Summary of changes:
 lib/RT/Dashboard.pm       |   16 +++++++--
 lib/RT/Groups.pm          |    5 ++-
 t/web/dashboards-groups.t |   83 +++++++++++++++++++++++++++++++++++++++------
 3 files changed, 89 insertions(+), 15 deletions(-)

- Log -----------------------------------------------------------------
commit 415d6c197467f1e21d81bea2f2486b00b356d17c
Author: Alex Vandiver <alexmv at bestpractical.com>
Date:   Wed Jul 20 20:27:10 2011 -0400

    Having SeeGroupDashboard on a group is insufficient; you must also be a member
    
    This restriction is currently enforced in RT::SharedSetting->_GetObject,
    and means that granting SeeGroupDashboard globally (on RT::System) means
    "you can see group dashboards for all groups you are in", as opposed to
    the previous implementation which caused it to mean "you can see all
    group dashboards."
    
    We may move eventually to SeeGroupDashboard globally having the latter
    meaning, but that would be a semantic change, and would require other UI
    changes.

diff --git a/lib/RT/Dashboard.pm b/lib/RT/Dashboard.pm
index 1aa9bf8..0822b0b 100644
--- a/lib/RT/Dashboard.pm
+++ b/lib/RT/Dashboard.pm
@@ -385,17 +385,26 @@ sub ObjectsForLoading {
     );
     my @objects;
 
+    # If you've been granted the SeeOwnDashboard global right (which you
+    # could have by way of global user right or global group right), you
+    # get to see your own dashboards
     my $CurrentUser = $self->CurrentUser;
     push @objects, $CurrentUser->UserObj
         if $CurrentUser->HasRight(Object => $RT::System, Right => 'SeeOwnDashboard');
 
-
+    # Find groups for which: (a) you are a member of the group, and (b)
+    # you have been granted SeeGroupDashboard on (by any means), and (c)
+    # have at least one dashboard
     my $groups = RT::Groups->new($CurrentUser);
     $groups->LimitToUserDefinedGroups;
     $groups->ForWhichCurrentUserHasRight(
         Right             => 'SeeGroupDashboard',
         %args,
     );
+    $groups->WithMember(
+        Recursively => 1,
+        PrincipalId => $CurrentUser->UserObj->PrincipalId
+    );
     my $attrs = $groups->Join(
         ALIAS1 => 'main',
         FIELD1 => 'id',
@@ -412,10 +421,11 @@ sub ObjectsForLoading {
         FIELD => 'Name',
         VALUE => 'Dashboard',
     );
-
     push @objects, @{ $groups->ItemsArrayRef };
 
-
+    # Finally, if you have been granted the SeeDashboard right (which
+    # you could have by way of global user right or global group right),
+    # you can see system dashboards.
     push @objects, RT::System->new($CurrentUser)
         if $CurrentUser->HasRight(Object => $RT::System, Right => 'SeeDashboard');
 

commit d13be25f579c898303e5e027498bce07597ce79d
Author: Alex Vandiver <alexmv at bestpractical.com>
Date:   Wed Jul 20 20:31:14 2011 -0400

    Ensure that we do not return disabled groups

diff --git a/lib/RT/Groups.pm b/lib/RT/Groups.pm
index e4b916f..6a5b510 100644
--- a/lib/RT/Groups.pm
+++ b/lib/RT/Groups.pm
@@ -348,7 +348,10 @@ sub ForWhichCurrentUserHasRight {
         @_,
     );
 
-    # Groups which are the target object of an ACL with that right, or
+    # Non-disabled groups...
+    $self->LimitToEnabled;
+
+    # ...which are the target object of an ACL with that right, or
     # where the target is the system object (a global right)
     my $acl = $self->_JoinACL( %args );
     $self->_AddSubClause(

commit 469aff87d02d1fa6115c55cf93f5350274e3be96
Author: Alex Vandiver <alexmv at bestpractical.com>
Date:   Wed Jul 20 20:33:41 2011 -0400

    Tests for superuser rights, and group membership interactions

diff --git a/t/web/dashboards-groups.t b/t/web/dashboards-groups.t
index 0c16798..6726d17 100644
--- a/t/web/dashboards-groups.t
+++ b/t/web/dashboards-groups.t
@@ -1,7 +1,7 @@
 #!/usr/bin/perl -w
 use strict;
 
-use RT::Test nodata => 1, tests => 48;
+use RT::Test nodata => 1, tests => 64;
 my ($baseurl, $m) = RT::Test->started_ok;
 
 my $url = $m->rt_base_url;
@@ -109,26 +109,87 @@ my ($group) = grep {$_->isa("RT::Group") and $_->Id == $inner_group->Id}
     RT::Dashboard->new($currentuser)->_PrivacyObjects;
 ok($group, "Found the group in  the privacy objects list");
 
+my @loading = map {ref($_)."-".$_->Id} RT::Dashboard->new($currentuser)->ObjectsForLoading;
+is_deeply(
+    \@loading,
+    ["RT::User-".$user_obj->Id, "RT::Group-".$inner_group->Id],
+    "We can load from ourselves (SeeOwnDashboard) and a group we are with SeeGroupDashboard"
+);
 
-($group) = grep {$_->isa("RT::Group") and $_->Id == $inner_group->Id}
-    RT::Dashboard->new($currentuser)->ObjectsForLoading;
-ok($group, "Found the group in the objects for loading");
-
-
-# With superuser, the dashboards of groups we're not in should not show
-# up in the menu, but should in the dashboard list.
+# If you are granted SeeGroupDashboard globally, you can only see
+# dashboards in groups you are in.
 $user_obj->PrincipalObj->RevokeRight(
     Right  => 'SeeGroupDashboard',
     Object => $inner_group,
 );
 $user_obj->PrincipalObj->GrantRight(
-    Right  => 'SuperUser',
+    Right  => 'SeeGroupDashboard',
     Object => RT->System,
 );
+$m->get_ok("/Dashboards/index.html");
+$m->content_contains("inner dashboard", "Having SeeGroupDashboard gobally is fine for groups you are in");
+ at loading = map {ref($_)."-".$_->Id} RT::Dashboard->new($currentuser)->ObjectsForLoading;
+is_deeply(
+    \@loading,
+    ["RT::User-".$user_obj->Id, "RT::Group-".$inner_group->Id],
+    "SeeGroupDashboard globally still works for groups you are in"
+);
+
 $inner_group->DeleteMember($user_obj->PrincipalObj->Id);
 ok(!$outer_group->HasMemberRecursively($user_obj->PrincipalId), "outer no longer has user recursively");
 ok(!$inner_group->HasMemberRecursively($user_obj->PrincipalId), "inner no longer has user recursively");
 $m->get_ok("/Dashboards/index.html");
-$m->content_contains("inner dashboard", "Superuser can see dashboards in their own groups");
+$m->content_lacks("inner dashboard", "But global SeeGroupDashboard isn't enough for other groups");
+$m->no_warnings_ok;
+ at loading = map {ref($_)."-".$_->Id} RT::Dashboard->new($currentuser)->ObjectsForLoading;
+is_deeply(
+    \@loading,
+    ["RT::User-".$user_obj->Id],
+    "We only have our SeeOwnDashboard right, as we are no longer in inner"
+);
+
+# Similarly, if you're a SuperUser, you still only see dashboards for
+# groups you belong to
+$user_obj->PrincipalObj->RevokeRight(
+    Right  => 'SeeGroupDashboard',
+    Object => RT->System,
+);
+$user_obj->PrincipalObj->GrantRight(
+    Right  => 'SuperUser',
+    Object => RT->System,
+);
+$m->get_ok("/Dashboards/index.html");
+$m->content_lacks("inner dashboard", "Superuser can't see dashboards in groups they're not in");
+ at loading = map {ref($_)."-".$_->Id} RT::Dashboard->new($currentuser)->ObjectsForLoading;
+is_deeply(
+    \@loading,
+    ["RT::User-".$user_obj->Id, "RT::System-1"],
+    "We pick up the system-level SeeDashboard right from superuser"
+);
+ at loading = map {ref($_)."-".$_->Id} RT::Dashboard->new($currentuser)->ObjectsForLoading(IncludeSuperusers => 0);
+is_deeply(
+    \@loading,
+    ["RT::User-".$user_obj->Id, "RT::System-1"],
+    "IncludeSuperusers only cuts out _group_ dashboard objects for loading, not user and system ones"
+);
+
+$inner_group->AddMember($user_obj->PrincipalId);
+$m->get_ok("/Dashboards/index.html");
+$m->content_contains("inner dashboard", "Superuser can see dashboards in groups they are in");
+ at loading = map {ref($_)."-".$_->Id} RT::Dashboard->new($currentuser)->ObjectsForLoading;
+is_deeply(
+    \@loading,
+    ["RT::User-".$user_obj->Id, "RT::Group-".$inner_group->Id, "RT::System-1"],
+    "Becoming a member of the group makes it a possibility"
+);
+ at loading = map {ref($_)."-".$_->Id} RT::Dashboard->new($currentuser)->ObjectsForLoading(IncludeSuperusers => 0);
+is_deeply(
+    \@loading,
+    ["RT::User-".$user_obj->Id, "RT::System-1"],
+    "But only via superuser"
+);
+
+$m->get_ok("/Dashboards/index.html");
+$m->content_contains("inner dashboard", "The dashboards list includes superuser rights");
 $m->get_ok("/index.html");
-$m->content_lacks("inner dashboard", "Also in the menu");
+$m->content_lacks("inner dashboard", "But the menu skips them");

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


More information about the Rt-commit mailing list