[Rt-commit] rt branch, 4.6/customrole-rights, created. rt-4.4.1-18-g27a1ff8

Shawn Moore shawn at bestpractical.com
Wed Jun 7 12:56:08 EDT 2017


The branch, 4.6/customrole-rights has been created
        at  27a1ff8d73ebd637b29e477579c740d83884548f (commit)

- Log -----------------------------------------------------------------
commit 57d7f2e9f9a0972797869ea68578894e67e4a673
Author: Shawn M Moore <shawn at bestpractical.com>
Date:   Fri Jun 2 22:09:01 2017 +0000

    Factor out a ContextObject role from CFs for reuse in custom roles
    
    This is in support of adding CF-like permissions to custom roles

diff --git a/lib/RT/CustomField.pm b/lib/RT/CustomField.pm
index cb37113..090d11c 100644
--- a/lib/RT/CustomField.pm
+++ b/lib/RT/CustomField.pm
@@ -58,7 +58,8 @@ use base 'RT::Record';
 
 use Role::Basic 'with';
 with "RT::Record::Role::Rights",
-     "RT::Record::Role::LookupType";
+     "RT::Record::Role::LookupType",
+     "RT::Record::Role::ContextObject";
 
 sub Table {'CustomFields'}
 
@@ -917,112 +918,6 @@ sub UnlimitedValues {
     }
 }
 
-
-=head2 ACLEquivalenceObjects
-
-Returns list of objects via which users can get rights on this custom field. For custom fields
-these objects can be set using L<ContextObject|/"ContextObject and SetContextObject">.
-
-=cut
-
-sub ACLEquivalenceObjects {
-    my $self = shift;
-
-    my $ctx = $self->ContextObject
-        or return;
-    return ($ctx, $ctx->ACLEquivalenceObjects);
-}
-
-=head2 ContextObject and SetContextObject
-
-Set or get a context for this object. It can be ticket, queue or another
-object this CF added to. Used for ACL control, for example
-SeeCustomField can be granted on queue level to allow people to see all
-fields added to the queue.
-
-=cut
-
-sub SetContextObject {
-    my $self = shift;
-    return $self->{'context_object'} = shift;
-}
-  
-sub ContextObject {
-    my $self = shift;
-    return $self->{'context_object'};
-}
-
-sub ValidContextType {
-    my $self = shift;
-    my $class = shift;
-
-    my %valid;
-    $valid{$_}++ for split '-', $self->LookupType;
-    delete $valid{'RT::Transaction'};
-
-    return $valid{$class};
-}
-
-=head2 LoadContextObject
-
-Takes an Id for a Context Object and loads the right kind of RT::Object
-for this particular Custom Field (based on the LookupType) and returns it.
-This is a good way to ensure you don't try to use a Queue as a Context
-Object on a User Custom Field.
-
-=cut
-
-sub LoadContextObject {
-    my $self = shift;
-    my $type = shift;
-    my $contextid = shift;
-
-    unless ( $self->ValidContextType($type) ) {
-        RT->Logger->debug("Invalid ContextType $type for Custom Field ".$self->Id);
-        return;
-    }
-
-    my $context_object = $type->new( $self->CurrentUser );
-    my ($id, $msg) = $context_object->LoadById( $contextid );
-    unless ( $id ) {
-        RT->Logger->debug("Invalid ContextObject id: $msg");
-        return;
-    }
-    return $context_object;
-}
-
-=head2 ValidateContextObject
-
-Ensure that a given ContextObject applies to this Custom Field.  For
-custom fields that are assigned to Queues or to Classes, this checks
-that the Custom Field is actually added to that object.  For Global
-Custom Fields, it returns true as long as the Object is of the right
-type, because you may be using your permissions on a given Queue of
-Class to see a Global CF.  For CFs that are only added globally, you
-don't need a ContextObject.
-
-=cut
-
-sub ValidateContextObject {
-    my $self = shift;
-    my $object = shift;
-
-    return 1 if $self->IsGlobal;
-
-    # global only custom fields don't have objects
-    # that should be used as context objects.
-    return if $self->IsOnlyGlobal;
-
-    # Otherwise, make sure we weren't passed a user object that we're
-    # supposed to treat as a queue.
-    return unless $self->ValidContextType(ref $object);
-
-    # Check that it is added correctly
-    my ($added_to) = grep {ref($_) eq $self->RecordClassFromLookupType} ($object, $object->ACLEquivalenceObjects);
-    return unless $added_to;
-    return $self->IsAdded($added_to->id);
-}
-
 sub _Set {
     my $self = shift;
     my %args = @_;
diff --git a/lib/RT/CustomFields.pm b/lib/RT/CustomFields.pm
index 7ef55da..f25cb15 100644
--- a/lib/RT/CustomFields.pm
+++ b/lib/RT/CustomFields.pm
@@ -72,6 +72,9 @@ use base 'RT::SearchBuilder';
 
 use RT::CustomField;
 
+use Role::Basic 'with';
+with "RT::SearchBuilder::Role::ContextObject";
+
 sub Table { 'CustomFields'}
 
 sub _Init {
@@ -359,31 +362,6 @@ sub ApplySortOrder {
 }
 
 
-=head2 ContextObject
-
-Returns context object for this collection of custom fields,
-but only if it's defined.
-
-=cut
-
-sub ContextObject {
-    my $self = shift;
-    return $self->{'context_object'};
-}
-
-
-=head2 SetContextObject
-
-Sets context object for this collection of custom fields.
-
-=cut
-
-sub SetContextObject {
-    my $self = shift;
-    return $self->{'context_object'} = shift;
-}
-
-
 sub _OCFAlias {
     my $self = shift;
     return RT::ObjectCustomFields->new( $self->CurrentUser )
@@ -410,21 +388,6 @@ sub AddRecord {
     return $self->SUPER::AddRecord( $record );
 }
 
-=head2 NewItem
-
-Returns an empty new RT::CustomField item
-Overrides <RT::SearchBuilder/NewItem> to make sure </ContextObject>
-is inherited.
-
-=cut
-
-sub NewItem {
-    my $self = shift;
-    my $res = RT::CustomField->new($self->CurrentUser);
-    $res->SetContextObject($self->ContextObject);
-    return $res;
-}
-
 =head2 LimitToCatalog
 
 Takes a numeric L<RT::Catalog> ID.  Limits the L<RT::CustomFields> collection
diff --git a/lib/RT/CustomRole.pm b/lib/RT/CustomRole.pm
index 3efa899..1d3f540 100644
--- a/lib/RT/CustomRole.pm
+++ b/lib/RT/CustomRole.pm
@@ -56,7 +56,8 @@ use RT::CustomRoles;
 use RT::ObjectCustomRole;
 
 use Role::Basic 'with';
-with "RT::Record::Role::LookupType";
+with "RT::Record::Role::LookupType",
+     "RT::Record::Role::ContextObject";
 
 =head1 NAME
 
@@ -352,6 +353,8 @@ sub NotAddedTo {
         ->NotAddedTo( CustomRole => $self );
 }
 
+sub IsGlobal { return 0 }
+
 =head2 AddToObject
 
 Adds (applies) this custom role to the provided object (ObjectId).
@@ -483,6 +486,15 @@ sub GroupType {
     return 'RT::CustomRole-' . $self->id;
 }
 
+=head2 CurrentUserCanSee
+
+=cut
+
+sub CurrentUserCanSee {
+    my $self = shift;
+    return 1;
+}
+
 =head2 id
 
 Returns the current value of id.
diff --git a/lib/RT/CustomRoles.pm b/lib/RT/CustomRoles.pm
index f5dea59..3af2419 100644
--- a/lib/RT/CustomRoles.pm
+++ b/lib/RT/CustomRoles.pm
@@ -54,6 +54,9 @@ use base 'RT::SearchBuilder';
 
 use RT::CustomRole;
 
+use Role::Basic 'with';
+with "RT::SearchBuilder::Role::ContextObject";
+
 =head1 NAME
 
 RT::CustomRoles - collection of RT::CustomRole records
@@ -185,6 +188,24 @@ sub ApplySortOrder {
     } );
 }
 
+=head2 AddRecord
+
+Overrides the collection to ensure that only custom roles the user can
+see are returned; also propagates down the L</ContextObject>.
+
+=cut
+
+sub AddRecord {
+    my $self = shift;
+    my ($record) = @_;
+
+    $record->SetContextObject( $self->ContextObject );
+
+    return unless $record->CurrentUserCanSee;
+
+    return $self->SUPER::AddRecord( $record );
+}
+
 RT::Base->_ImportOverlays();
 
 1;
diff --git a/lib/RT/Record/Role/ContextObject.pm b/lib/RT/Record/Role/ContextObject.pm
new file mode 100644
index 0000000..64bcb46
--- /dev/null
+++ b/lib/RT/Record/Role/ContextObject.pm
@@ -0,0 +1,185 @@
+# BEGIN BPS TAGGED BLOCK {{{
+#
+# COPYRIGHT:
+#
+# This software is Copyright (c) 1996-2017 Best Practical Solutions, LLC
+#                                          <sales at bestpractical.com>
+#
+# (Except where explicitly superseded by other copyright notices)
+#
+#
+# LICENSE:
+#
+# This work is made available to you under the terms of Version 2 of
+# the GNU General Public License. A copy of that license should have
+# been provided with this software, but in any event can be snarfed
+# from www.gnu.org.
+#
+# This work is distributed in the hope that it will be useful, but
+# WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+# General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
+# 02110-1301 or visit their web page on the internet at
+# http://www.gnu.org/licenses/old-licenses/gpl-2.0.html.
+#
+#
+# CONTRIBUTION SUBMISSION POLICY:
+#
+# (The following paragraph is not intended to limit the rights granted
+# to you to modify and distribute this software under the terms of
+# the GNU General Public License and is only of importance to you if
+# you choose to contribute your changes and enhancements to the
+# community by submitting them to Best Practical Solutions, LLC.)
+#
+# By intentionally submitting any modifications, corrections or
+# derivatives to this work, or any other work intended for use with
+# Request Tracker, to Best Practical Solutions, LLC, you confirm that
+# you are the copyright holder for those contributions and you grant
+# Best Practical Solutions,  LLC a nonexclusive, worldwide, irrevocable,
+# royalty-free, perpetual, license to use, copy, create derivative
+# works based on those contributions, and sublicense and distribute
+# those contributions and any derivatives thereof.
+#
+# END BPS TAGGED BLOCK }}}
+
+package RT::Record::Role::ContextObject;
+
+use strict;
+use warnings;
+use 5.010;
+
+use Role::Basic;
+use Scalar::Util qw(blessed);
+
+=head1 NAME
+
+RT::Record::Role::ContextObject - Common methods for records which have a ContextObject
+
+=head1 DESCRIPTION
+
+=head1 REQUIRES
+
+=head2 L<RT::Record::Role>
+
+=head2 L<RT::Record::Role::LookupType>
+
+=head2 IsGlobal
+
+=head2 IsOnlyGlobal
+
+=head2 IsAdded
+
+=cut
+
+with 'RT::Record::Role';
+
+=head2 ACLEquivalenceObjects
+
+Adds the context object's ACLEquivalenceObjects.
+
+=cut
+
+sub ACLEquivalenceObjects {
+    my $self = shift;
+
+    my $ctx = $self->ContextObject
+        or return;
+    return ($ctx, $ctx->ACLEquivalenceObjects);
+}
+
+=head2 ContextObject and SetContextObject
+
+Set or get a context for this object. It can be ticket, queue or another
+object. Used for ACL control, for example
+SeeCustomField can be granted on queue level to allow people to see all
+fields added to the queue.
+
+=cut
+
+sub SetContextObject {
+    my $self = shift;
+    return $self->{'context_object'} = shift;
+}
+
+sub ContextObject {
+    my $self = shift;
+    return $self->{'context_object'};
+}
+
+sub ValidContextType {
+    my $self = shift;
+    my $class = shift;
+
+    my %valid;
+    $valid{$_}++ for split '-', $self->LookupType;
+    delete $valid{'RT::Transaction'};
+
+    return $valid{$class};
+}
+
+=head2 LoadContextObject
+
+Takes an Id for a Context Object and loads the right kind of RT::Object
+for this particular Custom Field (based on the LookupType) and returns it.
+This is a good way to ensure you don't try to use a Queue as a Context
+Object on a User Custom Field.
+
+=cut
+
+sub LoadContextObject {
+    my $self = shift;
+    my $type = shift;
+    my $contextid = shift;
+
+    unless ( $self->ValidContextType($type) ) {
+        RT->Logger->debug("Invalid ContextType $type for Custom Field ".$self->Id);
+        return;
+    }
+
+    my $context_object = $type->new( $self->CurrentUser );
+    my ($id, $msg) = $context_object->LoadById( $contextid );
+    unless ( $id ) {
+        RT->Logger->debug("Invalid ContextObject id: $msg");
+        return;
+    }
+    return $context_object;
+}
+
+=head2 ValidateContextObject
+
+Ensure that a given ContextObject applies to this Custom Field.  For
+custom fields that are assigned to Queues or to Classes, this checks
+that the Custom Field is actually added to that object.  For Global
+Custom Fields, it returns true as long as the Object is of the right
+type, because you may be using your permissions on a given Queue of
+Class to see a Global CF.  For CFs that are only added globally, you
+don't need a ContextObject.
+
+=cut
+
+sub ValidateContextObject {
+    my $self = shift;
+    my $object = shift;
+
+    return 1 if $self->IsGlobal;
+
+    # global only custom fields don't have objects
+    # that should be used as context objects.
+    return if $self->IsOnlyGlobal;
+
+    # Otherwise, make sure we weren't passed a user object that we're
+    # supposed to treat as a queue.
+    return unless $self->ValidContextType(ref $object);
+
+    # Check that it is added correctly
+    my ($added_to) = grep {ref($_) eq $self->RecordClassFromLookupType} ($object, $object->ACLEquivalenceObjects);
+    return unless $added_to;
+    return $self->IsAdded($added_to->id);
+}
+
+1;
+
diff --git a/lib/RT/SearchBuilder/Role/ContextObject.pm b/lib/RT/SearchBuilder/Role/ContextObject.pm
new file mode 100644
index 0000000..7dd7c3a
--- /dev/null
+++ b/lib/RT/SearchBuilder/Role/ContextObject.pm
@@ -0,0 +1,106 @@
+# BEGIN BPS TAGGED BLOCK {{{
+#
+# COPYRIGHT:
+#
+# This software is Copyright (c) 1996-2017 Best Practical Solutions, LLC
+#                                          <sales at bestpractical.com>
+#
+# (Except where explicitly superseded by other copyright notices)
+#
+#
+# LICENSE:
+#
+# This work is made available to you under the terms of Version 2 of
+# the GNU General Public License. A copy of that license should have
+# been provided with this software, but in any event can be snarfed
+# from www.gnu.org.
+#
+# This work is distributed in the hope that it will be useful, but
+# WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+# General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
+# 02110-1301 or visit their web page on the internet at
+# http://www.gnu.org/licenses/old-licenses/gpl-2.0.html.
+#
+#
+# CONTRIBUTION SUBMISSION POLICY:
+#
+# (The following paragraph is not intended to limit the rights granted
+# to you to modify and distribute this software under the terms of
+# the GNU General Public License and is only of importance to you if
+# you choose to contribute your changes and enhancements to the
+# community by submitting them to Best Practical Solutions, LLC.)
+#
+# By intentionally submitting any modifications, corrections or
+# derivatives to this work, or any other work intended for use with
+# Request Tracker, to Best Practical Solutions, LLC, you confirm that
+# you are the copyright holder for those contributions and you grant
+# Best Practical Solutions,  LLC a nonexclusive, worldwide, irrevocable,
+# royalty-free, perpetual, license to use, copy, create derivative
+# works based on those contributions, and sublicense and distribute
+# those contributions and any derivatives thereof.
+#
+# END BPS TAGGED BLOCK }}}
+
+use strict;
+use warnings;
+
+package RT::SearchBuilder::Role::ContextObject;
+use Role::Basic;
+use Scalar::Util qw(blessed);
+
+=head1 NAME
+
+RT::SearchBuilder::Role::ContextObject - Common methods for collections with a ContextObject
+
+=head1 REQUIRES
+
+=head2 L<RT::SearchBuilder::Role>
+
+=cut
+
+with 'RT::SearchBuilder::Role';
+
+=head2 ContextObject
+
+Returns context object for this collection.
+
+=cut
+
+sub ContextObject {
+    my $self = shift;
+    return $self->{'context_object'};
+}
+
+=head2 SetContextObject
+
+Sets context object for this collection.
+
+=cut
+
+sub SetContextObject {
+    my $self = shift;
+    return $self->{'context_object'} = shift;
+}
+
+=head2 NewItem
+
+Returns an empty new record item
+Overrides <RT::SearchBuilder/NewItem> to make sure </ContextObject>
+is inherited.
+
+=cut
+
+sub NewItem {
+    my $self = shift;
+    my $item = $self->RecordClass->new($self->CurrentUser);
+    $item->SetContextObject($self->ContextObject);
+    return $item;
+}
+
+1;
+

commit 1e2d9f7926e296bb0b87ff8d0cdd2c58e393aa9c
Author: Shawn M Moore <shawn at bestpractical.com>
Date:   Fri Jun 2 22:27:13 2017 +0000

    Additional hookpoints and context required for custom role rights
    
    There are no visible changes in this commit, but it enables subsequent
    commits

diff --git a/lib/RT/Asset.pm b/lib/RT/Asset.pm
index 9c4c5fd..d9e5840 100644
--- a/lib/RT/Asset.pm
+++ b/lib/RT/Asset.pm
@@ -527,7 +527,7 @@ sub AddRoleMember {
     return (0, $self->loc("No permission to modify this asset"))
         unless $self->CurrentUserHasRight("ModifyAsset");
 
-    return $self->_AddRoleMember(@_);
+    return $self->_AddRoleMember(ACL => sub { $self->_HasModifyRoleMemberRight(@_) }, @_);
 }
 
 =head2 DeleteRoleMember
@@ -542,7 +542,14 @@ sub DeleteRoleMember {
     return (0, $self->loc("No permission to modify this asset"))
         unless $self->CurrentUserHasRight("ModifyAsset");
 
-    return $self->_DeleteRoleMember(@_);
+    return $self->_DeleteRoleMember(ACL => sub { $self->_HasModifyRoleMemberRight(@_) }, @_);
+}
+
+sub _HasModifyRoleMemberRight {
+    my $self = shift;
+    my ($type, $principal) = @_;
+
+    return 1;
 }
 
 =head2 RoleGroup
diff --git a/lib/RT/Catalog.pm b/lib/RT/Catalog.pm
index c6b457c..733c136 100644
--- a/lib/RT/Catalog.pm
+++ b/lib/RT/Catalog.pm
@@ -368,6 +368,24 @@ sub RoleGroup {
     }
 }
 
+=head2 CustomRoles
+
+Returns an L<RT::CustomRoles> object containing all catalog-specific roles.
+
+=cut
+
+sub CustomRoles {
+    my $self = shift;
+
+    my $roles = RT::CustomRoles->new( $self->CurrentUser );
+    if ( $self->CurrentUserHasRight('ShowCatalog') ) {
+        $roles->LimitToObjectId( $self->Id );
+        $roles->LimitToLookupType(RT::Asset->CustomFieldLookupType);
+        $roles->ApplySortOrder;
+    }
+    return ($roles);
+}
+
 =head2 AssetCustomFields
 
 Returns an L<RT::CustomFields> object containing all global and
diff --git a/lib/RT/Interface/Web.pm b/lib/RT/Interface/Web.pm
index e3cf905..00e278a 100644
--- a/lib/RT/Interface/Web.pm
+++ b/lib/RT/Interface/Web.pm
@@ -3920,8 +3920,8 @@ sub GetPrincipalsMap {
         elsif (/Roles/) {
             my $roles = RT::Groups->new($session{'CurrentUser'});
 
-            if ($object->isa("RT::CustomField")) {
-                # If we're a custom field, show the global roles for our LookupType.
+            if ($object->DOES("RT::Record::Role::LookupType")) {
+                # show the global roles for our LookupType.
                 my $class = $object->RecordClassFromLookupType;
                 if ($class and $class->DOES("RT::Record::Role::Roles")) {
                     $roles->LimitToRolesForObject(RT->System);
diff --git a/lib/RT/Transaction.pm b/lib/RT/Transaction.pm
index 9afdedf..c2a3382 100644
--- a/lib/RT/Transaction.pm
+++ b/lib/RT/Transaction.pm
@@ -871,9 +871,7 @@ sub _CanonicalizeRoleName {
     my $self = shift;
     my $role_name = shift;
 
-    if ($role_name =~ /^RT::CustomRole-(\d+)$/) {
-        my $role = RT::CustomRole->new($self->CurrentUser);
-        $role->Load($1);
+    if (my $role = $self->Object->CustomRoleObj($role_name)) {
         return $role->Name;
     }
 
@@ -1426,6 +1424,11 @@ sub CurrentUserCanSee {
         return 0 unless $cf->CurrentUserCanSee;
     }
 
+    # Ditto custom role
+    if ( ($type eq 'AddWatcher' || $type eq 'DelWatcher' || $type eq 'SetWatcher') && (my $role = $self->Object->CustomRoleObj($self->__Value('Field')))) {
+        return 0 unless $role->CurrentUserCanSee;
+    }
+
     # Transactions that might have changed the ->Object's visibility to
     # the current user are marked readable
     return 1 if $self->{ _object_is_readable };
diff --git a/share/html/Asset/Elements/EditRoleMembers b/share/html/Asset/Elements/EditRoleMembers
index 16c4c79..3605f49 100644
--- a/share/html/Asset/Elements/EditRoleMembers
+++ b/share/html/Asset/Elements/EditRoleMembers
@@ -48,6 +48,7 @@
 <%args>
 $Group       => undef
 $Recursively => 0
+$Readonly    => 0
 </%args>
 <%init>
 my $field_name = "RemoveRoleMember-" . $Group->Name;
@@ -55,12 +56,18 @@ my $field_name = "RemoveRoleMember-" . $Group->Name;
 <ul class="role-members">
 % my $Users = $Group->UserMembersObj( Recursively => $Recursively );
 % if ($Group->SingleMemberRoleGroup) {
+% if ($Readonly) {
+<& /Elements/ShowUser, User => $Users->First &>
+% } else {
 <input type="text" value="<% $Users->First->Name %>" name="SetRoleMember-<% $Group->Name %>" id="SetRoleMember-<% $Group->Name %>" data-autocomplete="Users" data-autocomplete-return="Name" /><br />
+% }
 % } else {
 % while ( my $user = $Users->Next ) {
 <li>
   <label>
+% if (!$Readonly) {
     <input type="checkbox" name="<% $field_name %>" value="<% $user->PrincipalId %>">
+% }
     <& /Elements/ShowUser, User => $user &>
   </label>
 </li>
@@ -70,7 +77,9 @@ my $field_name = "RemoveRoleMember-" . $Group->Name;
 % while (my $group = $Groups->Next) {
 <li>
   <label>
+% if (!$Readonly) {
     <input type="checkbox" name="<% $field_name %>" value="<% $group->PrincipalId %>">
+% }
     <&|/l&>Group</&>: <% $group->Name %>
   </label>
 </li>
diff --git a/share/html/Elements/ColumnMap b/share/html/Elements/ColumnMap
index c744ddc..7391fa6 100644
--- a/share/html/Elements/ColumnMap
+++ b/share/html/Elements/ColumnMap
@@ -149,14 +149,14 @@ $WCOLUMN_MAP = $COLUMN_MAP = {
             my $object = shift;
             my $role_name = pop;
 
-            my $role_type = do {
+            my $role_obj = do {
                 # Cache the role object on a per-request basis, to avoid
                 # having to load it for every row
                 my $key = "RT::CustomRole-" . $role_name;
 
-                my $role_type = $m->notes($key);
-                if (!$role_type) {
-                    my $role_obj = RT::CustomRole->new($object->CurrentUser);
+                my $role_obj = $m->notes($key);
+                if (!$role_obj) {
+                    $role_obj = RT::CustomRole->new($object->CurrentUser);
                     if ($role_name =~ /^\d+$/) {
                         $role_obj->Load($role_name);
                     }
@@ -167,14 +167,14 @@ $WCOLUMN_MAP = $COLUMN_MAP = {
                     RT->Logger->notice("Unable to load custom role $role_name")
                         unless $role_obj->Id;
 
-                    $role_type = $role_obj->GroupType;
-                    $m->notes($key, $role_type);
+                    $m->notes($key, $role_obj);
                 }
 
-                $role_type;
+                $role_obj;
             };
 
-            return if !$role_type;
+            my $role_type = $role_obj->GroupType;
+            return unless $role_type && $role_obj->CurrentUserCanSee;
             return \($m->scomp("/Elements/ShowPrincipal", Object => $object->RoleGroup($role_type) ) );
         },
     },
diff --git a/share/html/Elements/SelectWatcherType b/share/html/Elements/SelectWatcherType
index 338fc23..e74db95 100644
--- a/share/html/Elements/SelectWatcherType
+++ b/share/html/Elements/SelectWatcherType
@@ -67,6 +67,7 @@ else {
     @types = $Queue->Roles(Single => 0);
 }
 
+my $Object = $Ticket || $Queue;
 </%INIT>
 <%ARGS>
 $AllowNull => 1
@@ -74,4 +75,5 @@ $Default=>undef
 $Scope => 'ticket'
 $Name => 'WatcherType'
 $Queue => undef
+$Ticket => undef
 </%ARGS>
diff --git a/share/html/Ticket/Elements/AddWatchers b/share/html/Ticket/Elements/AddWatchers
index 1cbb6fc..73db5b0 100644
--- a/share/html/Ticket/Elements/AddWatchers
+++ b/share/html/Ticket/Elements/AddWatchers
@@ -60,8 +60,9 @@
 % while (my $u = $Users->Next ) {
 <tr>
 <td><&/Elements/SelectWatcherType,
-    Name  => "Ticket-AddWatcher-Principal-". $u->PrincipalId,
-    Queue => $Ticket->QueueObj,
+    Name   => "Ticket-AddWatcher-Principal-". $u->PrincipalId,
+    Queue  => $Ticket->QueueObj,
+    Ticket => $Ticket,
 &></td>
 <td><& '/Elements/ShowUser', User => $u, style=>'verbose' &></td>
 </tr>
@@ -77,8 +78,9 @@
 % while (my $g = $Groups->Next ) {
 <tr>
 <td><& /Elements/SelectWatcherType,
-    Name  => "Ticket-AddWatcher-Principal-".$g->PrincipalId,
-    Queue => $Ticket->QueueObj,
+    Name   => "Ticket-AddWatcher-Principal-".$g->PrincipalId,
+    Queue  => $Ticket->QueueObj,
+    Ticket => $Ticket,
 &></td>
 <td><%$g->Name%> (<%$g->Description%>)</td>
 </tr>
@@ -94,7 +96,7 @@
 % for my $email (@extras) {
 % $counter++;
 <tr><td>
-<&/Elements/SelectWatcherType, Name => "WatcherTypeEmail".$counter, Queue => $Ticket->QueueObj &>
+<&/Elements/SelectWatcherType, Name => "WatcherTypeEmail".$counter, Queue => $Ticket->QueueObj, Ticket => $Ticket &>
 </td><td>
 <input type="hidden" name="WatcherAddressEmail<%$counter%>" value="<%$email->format%>">
 <%$email->format%>
@@ -102,7 +104,7 @@
 % }
 % for my $i (1 .. 3) {
 <tr><td>
-<&/Elements/SelectWatcherType, Name => "WatcherTypeEmail" . $i, Queue => $Ticket->QueueObj &>
+<&/Elements/SelectWatcherType, Name => "WatcherTypeEmail" . $i, Queue => $Ticket->QueueObj, Ticket => $Ticket &>
 </td><td>
 <& /Elements/EmailInput, Name => 'WatcherAddressEmail' . $i, Size => '20' &>
 </td></tr>
diff --git a/share/html/Ticket/Elements/EditWatchers b/share/html/Ticket/Elements/EditWatchers
index 1c88d72..b4f3fb7 100644
--- a/share/html/Ticket/Elements/EditWatchers
+++ b/share/html/Ticket/Elements/EditWatchers
@@ -53,7 +53,10 @@
 % while ( my $watcher = $Members->Next ) {
 % my $member = $watcher->MemberObj->Object;
 <li>
+% unless ($Readonly) {
 <input type="checkbox" class="checkbox" name="Ticket-DeleteWatcher-Type-<% $Watchers->Name %>-Principal-<% $watcher->MemberId %>" value="1" unchecked />
+% }
+
 % if ( $member->isa( 'RT::User' ) ) { 
 % if ( $session{CurrentUser}->HasRight( Right => 'AdminUsers', Object => $RT::System ) &&
 %      $session{CurrentUser}->HasRight( Right => 'ShowConfigTab', Object =>$RT::System ) ) {
@@ -82,4 +85,5 @@ my $Members = $Watchers->MembersObj;
 <%ARGS>
 $TicketObj => undef
 $Watchers => undef
+$Readonly => 0
 </%ARGS>

commit bdb83b4635d169d49dc5b04a49f74cc5c1b2dd7b
Author: Shawn M Moore <shawn at bestpractical.com>
Date:   Fri Jun 2 22:17:33 2017 +0000

    Provide context object for custom roles
    
    This enables better permission checking

diff --git a/lib/RT/Catalog.pm b/lib/RT/Catalog.pm
index 733c136..32d3409 100644
--- a/lib/RT/Catalog.pm
+++ b/lib/RT/Catalog.pm
@@ -379,6 +379,7 @@ sub CustomRoles {
 
     my $roles = RT::CustomRoles->new( $self->CurrentUser );
     if ( $self->CurrentUserHasRight('ShowCatalog') ) {
+        $roles->SetContextObject( $self );
         $roles->LimitToObjectId( $self->Id );
         $roles->LimitToLookupType(RT::Asset->CustomFieldLookupType);
         $roles->ApplySortOrder;
diff --git a/lib/RT/Queue.pm b/lib/RT/Queue.pm
index fddc90c..059046e 100644
--- a/lib/RT/Queue.pm
+++ b/lib/RT/Queue.pm
@@ -480,6 +480,7 @@ sub CustomRoles {
 
     my $roles = RT::CustomRoles->new( $self->CurrentUser );
     if ( $self->CurrentUserHasRight('SeeQueue') ) {
+        $roles->SetContextObject( $self );
         $roles->LimitToObjectId( $self->Id );
         $roles->LimitToLookupType(RT::Ticket->CustomFieldLookupType);
         $roles->ApplySortOrder;
diff --git a/lib/RT/Record/Role/Roles.pm b/lib/RT/Record/Role/Roles.pm
index dc0b552..56cf640 100644
--- a/lib/RT/Record/Role/Roles.pm
+++ b/lib/RT/Record/Role/Roles.pm
@@ -750,6 +750,7 @@ sub CustomRoleObj {
     if (my ($id) = $name =~ /^RT::CustomRole-(\d+)$/) {
         my $role = RT::CustomRole->new($self->CurrentUser);
         $role->Load($id);
+        $role->SetContextObject($self);
         return $role;
     }
 
diff --git a/share/html/Elements/ColumnMap b/share/html/Elements/ColumnMap
index 7391fa6..055686f 100644
--- a/share/html/Elements/ColumnMap
+++ b/share/html/Elements/ColumnMap
@@ -173,6 +173,8 @@ $WCOLUMN_MAP = $COLUMN_MAP = {
                 $role_obj;
             };
 
+            $role_obj->SetContextObject($object);
+
             my $role_type = $role_obj->GroupType;
             return unless $role_type && $role_obj->CurrentUserCanSee;
             return \($m->scomp("/Elements/ShowPrincipal", Object => $object->RoleGroup($role_type) ) );
diff --git a/share/html/Search/Elements/BuildFormatString b/share/html/Search/Elements/BuildFormatString
index 9c9bf80..9b98ea4 100644
--- a/share/html/Search/Elements/BuildFormatString
+++ b/share/html/Search/Elements/BuildFormatString
@@ -124,6 +124,7 @@ foreach my $id (keys %queues) {
     $queue->Load($id);
     next unless $queue->Id;
     $CustomRoles->LimitToObjectId($queue->Id);
+    $CustomRoles->SetContextObject( $queue ) if keys %queues == 1;
 }
 while ( my $Role = $CustomRoles->Next ) {
     push @fields, "CustomRole.{" . $Role->Name . "}";
diff --git a/share/html/Search/Elements/PickCustomRoles b/share/html/Search/Elements/PickCustomRoles
index a1ebe52..b74155f 100644
--- a/share/html/Search/Elements/PickCustomRoles
+++ b/share/html/Search/Elements/PickCustomRoles
@@ -57,6 +57,7 @@ foreach my $id (keys %queues) {
     $queue->Load($id);
     next unless $queue->Id;
     $CustomRoles->LimitToObjectId($queue->Id);
+    $CustomRoles->SetContextObject( $queue ) if keys %queues == 1;
 }
 $m->callback(
     CallbackName => 'MassageCustomRoles',
diff --git a/share/html/Ticket/Create.html b/share/html/Ticket/Create.html
index e396857..4ea3192 100644
--- a/share/html/Ticket/Create.html
+++ b/share/html/Ticket/Create.html
@@ -185,6 +185,7 @@
 </tr>
 
 % my $roles = $QueueObj->CustomRoles;
+% $roles->SetContextObject($QueueObj);
 % $roles->LimitToMultipleValue;
 % while (my $role = $roles->Next) {
 <tr>
diff --git a/share/html/Ticket/Elements/EditBasics b/share/html/Ticket/Elements/EditBasics
index 89d5ffe..c7d6af1 100644
--- a/share/html/Ticket/Elements/EditBasics
+++ b/share/html/Ticket/Elements/EditBasics
@@ -139,6 +139,7 @@ my @role_fields;
 
 unless ($ExcludeCustomRoles) {
     my $roles = $QueueObj->CustomRoles;
+    $roles->SetContextObject($TicketObj || $QueueObj);
     $roles->LimitToSingleValue;
     while (my $role = $roles->Next) {
         push @role_fields, {
diff --git a/share/html/Ticket/Elements/EditPeople b/share/html/Ticket/Elements/EditPeople
index 4d2adc5..9ac039b 100644
--- a/share/html/Ticket/Elements/EditPeople
+++ b/share/html/Ticket/Elements/EditPeople
@@ -73,6 +73,7 @@
 
 % my @role_fields;
 % my $single_roles = $Ticket->QueueObj->CustomRoles;
+% $single_roles->SetContextObject($Ticket);
 % $single_roles->LimitToSingleValue;
 % while (my $role = $single_roles->Next) {
 <tr>
@@ -105,6 +106,7 @@
 </tr>
 
 % my $multi_roles = $Ticket->QueueObj->CustomRoles;
+% $multi_roles->SetContextObject($Ticket);
 % $multi_roles->LimitToMultipleValue;
 % while (my $role = $multi_roles->Next) {
 % my $group = $Ticket->RoleGroup($role->GroupType);
diff --git a/share/html/Ticket/Elements/ShowPeople b/share/html/Ticket/Elements/ShowPeople
index f31e907..ab95338 100644
--- a/share/html/Ticket/Elements/ShowPeople
+++ b/share/html/Ticket/Elements/ShowPeople
@@ -56,6 +56,7 @@
   </tr>
 
 % my $single_roles = $Ticket->QueueObj->CustomRoles;
+% $single_roles->SetContextObject($Ticket);
 % $single_roles->LimitToSingleValue;
 % while (my $role = $single_roles->Next) {
 %     my $group = $Ticket->RoleGroup($role->GroupType);
@@ -85,6 +86,7 @@
   </tr>
 
 % my $multi_roles = $Ticket->QueueObj->CustomRoles;
+% $multi_roles->SetContextObject($Ticket);
 % $multi_roles->LimitToMultipleValue;
 % while (my $role = $multi_roles->Next) {
   <tr>

commit ef64f1e70d0fbc54ad41771812d1862ac28b0807
Author: Shawn M Moore <shawn at bestpractical.com>
Date:   Fri Jun 2 22:18:19 2017 +0000

    Avoid throwing error on invalid roles
    
    If there's no acl callback available, assume no permission

diff --git a/lib/RT/Record/Role/Roles.pm b/lib/RT/Record/Role/Roles.pm
index 56cf640..4ca766a 100644
--- a/lib/RT/Record/Role/Roles.pm
+++ b/lib/RT/Record/Role/Roles.pm
@@ -695,7 +695,7 @@ sub _AddRolesOnCreate {
             my $group = $self->RoleGroup($role);
             my @left;
             for my $principal (@{$roles->{$role}}) {
-                if ($acls{$role}->($principal)) {
+                if ($acls{$role} && $acls{$role}->($principal)) {
                     next if $group->HasMember($principal);
                     my ($ok, $msg) = $group->_AddMember(
                         PrincipalId       => $principal->id,

commit dd1f5a37aa7b5bae27ce9fa1aec0631da4bc5ca0
Author: Shawn M Moore <shawn at bestpractical.com>
Date:   Fri Jun 2 22:22:51 2017 +0000

    Infrastructure for granting rights for custom roles

diff --git a/lib/RT/Catalog.pm b/lib/RT/Catalog.pm
index 32d3409..d478941 100644
--- a/lib/RT/Catalog.pm
+++ b/lib/RT/Catalog.pm
@@ -86,6 +86,8 @@ __PACKAGE__->AddRight( Staff   => ModifyAsset  => 'Modify assets' ); #loc
 __PACKAGE__->AddRight( General => SeeCustomField        => 'View custom field values' ); # loc
 __PACKAGE__->AddRight( Staff   => ModifyCustomField     => 'Modify custom field values' ); # loc
 __PACKAGE__->AddRight( Staff   => SetInitialCustomField => 'Add custom field values only at object creation time'); # loc
+__PACKAGE__->AddRight( General => SeeCustomRole         => 'View custom role members'); # loc
+__PACKAGE__->AddRight( Staff   => ModifyCustomRole      => 'Modify custom role members'); # loc
 
 RT::ACE->RegisterCacheHandler(sub {
     my %args = (
diff --git a/lib/RT/CustomRole.pm b/lib/RT/CustomRole.pm
index 1d3f540..baafb1c 100644
--- a/lib/RT/CustomRole.pm
+++ b/lib/RT/CustomRole.pm
@@ -56,9 +56,13 @@ use RT::CustomRoles;
 use RT::ObjectCustomRole;
 
 use Role::Basic 'with';
-with "RT::Record::Role::LookupType",
+with "RT::Record::Role::Rights",
+     "RT::Record::Role::LookupType",
      "RT::Record::Role::ContextObject";
 
+__PACKAGE__->AddRight( General => SeeCustomRole         => 'View custom roles'); # loc
+__PACKAGE__->AddRight( Staff   => ModifyCustomRole      => 'Add, modify and delete custom role members'); # loc
+
 =head1 NAME
 
 RT::CustomRole - user-defined role groups
diff --git a/lib/RT/Queue.pm b/lib/RT/Queue.pm
index 059046e..61abca0 100644
--- a/lib/RT/Queue.pm
+++ b/lib/RT/Queue.pm
@@ -111,6 +111,8 @@ __PACKAGE__->AddRight( Staff   => SetInitialCustomField => 'Add custom field val
 __PACKAGE__->AddRight( Admin   => AssignCustomFields    => 'Assign and remove queue custom fields' ); # loc
 __PACKAGE__->AddRight( Admin   => ModifyTemplate        => 'Modify Scrip templates' ); # loc
 __PACKAGE__->AddRight( Admin   => ShowTemplate          => 'View Scrip templates' ); # loc
+__PACKAGE__->AddRight( General => SeeCustomRole         => 'View custom role members'); # loc
+__PACKAGE__->AddRight( Staff   => ModifyCustomRole      => 'Modify custom role members'); # loc
 
 __PACKAGE__->AddRight( Admin   => ModifyScrips        => 'Modify Scrips' ); # loc
 __PACKAGE__->AddRight( Admin   => ShowScrips          => 'View Scrips' ); # loc
diff --git a/share/html/Admin/CustomRoles/GroupRights.html b/share/html/Admin/CustomRoles/GroupRights.html
new file mode 100644
index 0000000..9d17e5a
--- /dev/null
+++ b/share/html/Admin/CustomRoles/GroupRights.html
@@ -0,0 +1,78 @@
+%# BEGIN BPS TAGGED BLOCK {{{
+%#
+%# COPYRIGHT:
+%#
+%# This software is Copyright (c) 1996-2017 Best Practical Solutions, LLC
+%#                                          <sales at bestpractical.com>
+%#
+%# (Except where explicitly superseded by other copyright notices)
+%#
+%#
+%# LICENSE:
+%#
+%# This work is made available to you under the terms of Version 2 of
+%# the GNU General Public License. A copy of that license should have
+%# been provided with this software, but in any event can be snarfed
+%# from www.gnu.org.
+%#
+%# This work is distributed in the hope that it will be useful, but
+%# WITHOUT ANY WARRANTY; without even the implied warranty of
+%# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+%# General Public License for more details.
+%#
+%# You should have received a copy of the GNU General Public License
+%# along with this program; if not, write to the Free Software
+%# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
+%# 02110-1301 or visit their web page on the internet at
+%# http://www.gnu.org/licenses/old-licenses/gpl-2.0.html.
+%#
+%#
+%# CONTRIBUTION SUBMISSION POLICY:
+%#
+%# (The following paragraph is not intended to limit the rights granted
+%# to you to modify and distribute this software under the terms of
+%# the GNU General Public License and is only of importance to you if
+%# you choose to contribute your changes and enhancements to the
+%# community by submitting them to Best Practical Solutions, LLC.)
+%#
+%# By intentionally submitting any modifications, corrections or
+%# derivatives to this work, or any other work intended for use with
+%# Request Tracker, to Best Practical Solutions, LLC, you confirm that
+%# you are the copyright holder for those contributions and you grant
+%# Best Practical Solutions,  LLC a nonexclusive, worldwide, irrevocable,
+%# royalty-free, perpetual, license to use, copy, create derivative
+%# works based on those contributions, and sublicense and distribute
+%# those contributions and any derivatives thereof.
+%#
+%# END BPS TAGGED BLOCK }}}
+<& /Admin/Elements/Header, Title => $title &>
+<& /Elements/Tabs &>
+<& /Elements/ListActions, actions => \@results &>
+
+  <form method="post" action="GroupRights.html" id="ModifyGroupRights" name="ModifyGroupRights">
+    <input type="hidden" class="hidden" name="id" value="<% $CustomRole->id %>" />
+
+    <& /Admin/Elements/EditRights, Context => $CustomRole, Principals => \@principals &>
+    <& /Elements/Submit, Label => loc('Save Changes') &>
+  </form>
+
+<%INIT>
+
+if (!defined $id) {
+    $m->comp("/Elements/Error", Why => loc("No custom role defined"));
+}
+
+my $CustomRole = RT::CustomRole->new($session{'CurrentUser'});
+$CustomRole->Load($id) || $m->comp("/Elements/Error", Why => loc("Couldn't load custom role [_1]",$id));
+
+my @results = ProcessACLs( \%ARGS );
+
+my $title = loc('Modify group rights for custom role [_1]', $CustomRole->Name);
+
+# Principal collections
+my @principals = GetPrincipalsMap($CustomRole, qw(System Roles Groups));
+</%INIT>
+
+<%ARGS>
+$id => undef
+</%ARGS>
diff --git a/share/html/Admin/CustomRoles/UserRights.html b/share/html/Admin/CustomRoles/UserRights.html
new file mode 100644
index 0000000..a096533
--- /dev/null
+++ b/share/html/Admin/CustomRoles/UserRights.html
@@ -0,0 +1,78 @@
+%# BEGIN BPS TAGGED BLOCK {{{
+%#
+%# COPYRIGHT:
+%#
+%# This software is Copyright (c) 1996-2017 Best Practical Solutions, LLC
+%#                                          <sales at bestpractical.com>
+%#
+%# (Except where explicitly superseded by other copyright notices)
+%#
+%#
+%# LICENSE:
+%#
+%# This work is made available to you under the terms of Version 2 of
+%# the GNU General Public License. A copy of that license should have
+%# been provided with this software, but in any event can be snarfed
+%# from www.gnu.org.
+%#
+%# This work is distributed in the hope that it will be useful, but
+%# WITHOUT ANY WARRANTY; without even the implied warranty of
+%# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+%# General Public License for more details.
+%#
+%# You should have received a copy of the GNU General Public License
+%# along with this program; if not, write to the Free Software
+%# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
+%# 02110-1301 or visit their web page on the internet at
+%# http://www.gnu.org/licenses/old-licenses/gpl-2.0.html.
+%#
+%#
+%# CONTRIBUTION SUBMISSION POLICY:
+%#
+%# (The following paragraph is not intended to limit the rights granted
+%# to you to modify and distribute this software under the terms of
+%# the GNU General Public License and is only of importance to you if
+%# you choose to contribute your changes and enhancements to the
+%# community by submitting them to Best Practical Solutions, LLC.)
+%#
+%# By intentionally submitting any modifications, corrections or
+%# derivatives to this work, or any other work intended for use with
+%# Request Tracker, to Best Practical Solutions, LLC, you confirm that
+%# you are the copyright holder for those contributions and you grant
+%# Best Practical Solutions,  LLC a nonexclusive, worldwide, irrevocable,
+%# royalty-free, perpetual, license to use, copy, create derivative
+%# works based on those contributions, and sublicense and distribute
+%# those contributions and any derivatives thereof.
+%#
+%# END BPS TAGGED BLOCK }}}
+<& /Admin/Elements/Header, Title => $title &>
+<& /Elements/Tabs &>
+<& /Elements/ListActions, actions => \@results &>
+
+  <form method="post" action="UserRights.html" id="ModifyUserRights" name="ModifyUserRights">
+    <input type="hidden" class="hidden" name="id" value="<% $CustomRole->id %>" />
+
+    <& /Admin/Elements/EditRights, Context => $CustomRole, Principals => \@principals &>
+    <& /Elements/Submit, Label => loc('Save Changes') &>
+  </form>
+
+<%INIT>
+
+if (!defined $id) {
+    $m->comp("/Elements/Error", Why => loc("No custom role defined"));
+}
+
+my $CustomRole = RT::CustomRole->new($session{'CurrentUser'});
+$CustomRole->Load($id) || $m->comp("/Elements/Error", Why => loc("Couldn't load custom role [_1]",$id));
+
+my @results = ProcessACLs( \%ARGS );
+
+my $title = loc('Modify user rights for custom role [_1]', $CustomRole->Name);
+
+# Principal collections
+my @principals = GetPrincipalsMap($CustomRole, qw(Users));
+</%INIT>
+
+<%ARGS>
+$id => undef
+</%ARGS>
diff --git a/share/html/Elements/Tabs b/share/html/Elements/Tabs
index 866e8f2..8084843 100644
--- a/share/html/Elements/Tabs
+++ b/share/html/Elements/Tabs
@@ -416,8 +416,10 @@ my $build_admin_menu = sub {
 
             if ( $obj and $obj->id ) {
                 my $tabs = PageMenu();
-                $tabs->child( basics       => title => loc('Basics'),       path => "/Admin/CustomRoles/Modify.html?id=".$id );
-                $tabs->child( 'applies-to' => title => loc('Applies to'),   path => "/Admin/CustomRoles/Objects.html?id=" . $id );
+                $tabs->child( basics         => title => loc('Basics'),       path => "/Admin/CustomRoles/Modify.html?id=".$id );
+                $tabs->child( 'group-rights' => title => loc('Group Rights'), path => "/Admin/CustomRoles/GroupRights.html?id=".$id );
+                $tabs->child( 'user-rights'  => title => loc('User Rights'),  path => "/Admin/CustomRoles/UserRights.html?id=".$id );
+                $tabs->child( 'applies-to'   => title => loc('Applies to'),   path => "/Admin/CustomRoles/Objects.html?id=".$id );
             }
         }
     }

commit 27a1ff8d73ebd637b29e477579c740d83884548f
Author: Shawn M Moore <shawn at bestpractical.com>
Date:   Fri Jun 2 22:25:11 2017 +0000

    Enforce SeeCustomRole and ModifyCustomRole

diff --git a/lib/RT/Asset.pm b/lib/RT/Asset.pm
index d9e5840..7b61580 100644
--- a/lib/RT/Asset.pm
+++ b/lib/RT/Asset.pm
@@ -325,7 +325,19 @@ sub Create {
     }
 
     # Figure out users for roles
-    push @non_fatal_errors, $self->_AddRolesOnCreate( $roles, map { $_ => sub {1} } $self->Roles );
+    my %acls = (map { $_ => sub {1} } $self->Roles(UserDefined => 0));
+
+    my $custom_roles = $catalog->CustomRoles;
+    while (my $role = $custom_roles->Next) {
+        $acls{$role->GroupType} = sub {
+            return $self->CurrentUser->HasRight(
+                Right  => 'ModifyCustomRole',
+                Object => $role,
+            );
+        };
+    }
+
+    push @non_fatal_errors, $self->_AddRolesOnCreate( $roles, %acls );
 
     # Add CFs
     foreach my $key (keys %args) {
@@ -549,6 +561,10 @@ sub _HasModifyRoleMemberRight {
     my $self = shift;
     my ($type, $principal) = @_;
 
+    if (my $role = $self->CustomRoleObj($type)) {
+        return $role->CurrentUserHasRight('ModifyCustomRole');
+    }
+
     return 1;
 }
 
diff --git a/lib/RT/CustomRole.pm b/lib/RT/CustomRole.pm
index baafb1c..04f0735 100644
--- a/lib/RT/CustomRole.pm
+++ b/lib/RT/CustomRole.pm
@@ -492,11 +492,13 @@ sub GroupType {
 
 =head2 CurrentUserCanSee
 
+If the user has SeeCustomRole they can see this custom role and its members.
+
 =cut
 
 sub CurrentUserCanSee {
     my $self = shift;
-    return 1;
+    return $self->CurrentUserHasRight('SeeCustomRole');
 }
 
 =head2 id
diff --git a/lib/RT/Ticket.pm b/lib/RT/Ticket.pm
index 6457b5b..73989f5 100644
--- a/lib/RT/Ticket.pm
+++ b/lib/RT/Ticket.pm
@@ -511,6 +511,16 @@ sub Create {
         },
     );
 
+    my $custom_roles = $QueueObj->CustomRoles;
+    while (my $role = $custom_roles->Next) {
+        $acls{$role->GroupType} = sub {
+            return $self->CurrentUser->HasRight(
+                Right  => 'ModifyCustomRole',
+                Object => $role,
+            );
+        };
+    }
+
     # Populate up the role groups.  This call modifies $roles.
     push @non_fatal_errors, $self->_AddRolesOnCreate( $roles, %acls );
 
@@ -658,6 +668,10 @@ sub _HasModifyWatcherRight {
     my $self = shift;
     my ($type, $principal) = @_;
 
+    if (my $role = $self->CustomRoleObj($type)) {
+        return $role->CurrentUserHasRight('ModifyCustomRole');
+    }
+
     # ModifyTicket works in any case
     return 1 if $self->CurrentUserHasRight('ModifyTicket');
     # If the watcher isn't the current user then the current user has no right
diff --git a/share/html/Asset/Elements/EditCatalogPeople b/share/html/Asset/Elements/EditCatalogPeople
index ca16584..bead9b0 100644
--- a/share/html/Asset/Elements/EditCatalogPeople
+++ b/share/html/Asset/Elements/EditCatalogPeople
@@ -51,9 +51,14 @@ $Object
 <%init>
 </%init>
 % for my $role ($Object->Roles( ACLOnly => 0 )) {
+% my $custom_role = $Object->CustomRoleObj($role);
+% next if $custom_role && !$custom_role->CurrentUserHasRight('SeeCustomRole');
 <div class="role-<% CSSClass($role) %> role">
   <h3><% $Object->LabelForRole($role) %></h3>
-  <& EditRoleMembers, Group => $Object->RoleGroup($role) &>
+  <& EditRoleMembers,
+       Group => $Object->RoleGroup($role),
+       Readonly => ($custom_role && !$custom_role->CurrentUserHasRight('ModifyCustomRole'))
+  &>
 </div>
 % }
 <em><&|/l&>(Check box to delete)</&></em>
diff --git a/share/html/Asset/Elements/EditPeople b/share/html/Asset/Elements/EditPeople
index aefe52c..ad10f4d 100644
--- a/share/html/Asset/Elements/EditPeople
+++ b/share/html/Asset/Elements/EditPeople
@@ -47,6 +47,8 @@
 %# END BPS TAGGED BLOCK }}}
 <table border="0" cellpadding="0" cellspacing="0">
 % for my $role ( $AssetObj->Roles(_Catalog => $CatalogObj) ) {
+% my $custom_role = $CatalogObj->CustomRoleObj($role);
+% next if $custom_role && !$custom_role->CurrentUserHasRight('ModifyCustomRole');
 <tr class="asset-people-<% CSSClass($role) %>">
 <td class="label">
 <% $AssetObj->LabelForRole($role) %>:
@@ -56,7 +58,6 @@
 </td>
 </tr>
 
-% my $custom_role = $AssetObj->CustomRoleObj($role);
 % if ($custom_role && $custom_role->EntryHint) {
 <tr>
   <td class="label"> </td>
diff --git a/share/html/Asset/Elements/SelectRoleType b/share/html/Asset/Elements/SelectRoleType
index 0ec3b94..d182366 100644
--- a/share/html/Asset/Elements/SelectRoleType
+++ b/share/html/Asset/Elements/SelectRoleType
@@ -55,6 +55,8 @@ $AllowNull  => 0
   <option value=""></option>
 % }
 % for my $role ($Object->Roles( ACLOnly => 0, Single => 0 )) {
+% my $custom_role = $Object->CustomRoleObj($role);
+% next if $custom_role && !$custom_role->CurrentUserHasRight('ModifyCustomRole');
   <option value="<% $role %>"><% $Object->LabelForRole($role) %></option>
 % }
 </select>
diff --git a/share/html/Asset/Elements/ShowPeople b/share/html/Asset/Elements/ShowPeople
index 12b7cd9..e9c6b97 100644
--- a/share/html/Asset/Elements/ShowPeople
+++ b/share/html/Asset/Elements/ShowPeople
@@ -53,6 +53,8 @@ my $CatalogObj = $AssetObj->CatalogObj;
 </%init>
 <table>
 % for my $role ($AssetObj->Roles) {
+% my $custom_role = $AssetObj->CustomRoleObj($role);
+% next if $custom_role && !$custom_role->CurrentUserHasRight('SeeCustomRole');
 <tr><td class="label"><% $AssetObj->LabelForRole($role) %>:
 % if ($AssetObj->Role($role)->{Single}) {
 %      my $users = $AssetObj->RoleGroup($role)->UserMembersObj(Recursively => 0);
diff --git a/share/html/Elements/SelectWatcherType b/share/html/Elements/SelectWatcherType
index e74db95..791a8ef 100644
--- a/share/html/Elements/SelectWatcherType
+++ b/share/html/Elements/SelectWatcherType
@@ -50,6 +50,8 @@
 <option value="">-</option>
 % }
 %for my $value (@types) {
+% my $custom_role = $Object->CustomRoleObj($value);
+% next if $custom_role && !$custom_role->CurrentUserHasRight('ModifyCustomRole');
 <option value="<%$value%>"
 % if (defined($Default) && $value eq $Default) {
 selected="selected"
diff --git a/share/html/Ticket/Create.html b/share/html/Ticket/Create.html
index 4ea3192..ea8206a 100644
--- a/share/html/Ticket/Create.html
+++ b/share/html/Ticket/Create.html
@@ -188,6 +188,7 @@
 % $roles->SetContextObject($QueueObj);
 % $roles->LimitToMultipleValue;
 % while (my $role = $roles->Next) {
+% next unless $role->CurrentUserHasRight('ModifyCustomRole');
 <tr>
 <td class="label">
 <% $role->Name %>:
diff --git a/share/html/Ticket/Elements/EditBasics b/share/html/Ticket/Elements/EditBasics
index c7d6af1..5177ef3 100644
--- a/share/html/Ticket/Elements/EditBasics
+++ b/share/html/Ticket/Elements/EditBasics
@@ -142,6 +142,7 @@ unless ($ExcludeCustomRoles) {
     $roles->SetContextObject($TicketObj || $QueueObj);
     $roles->LimitToSingleValue;
     while (my $role = $roles->Next) {
+        next unless $role->CurrentUserHasRight('ModifyCustomRole');
         push @role_fields, {
             name => $role->Name,
             comp => '/Elements/SingleUserRoleInput',
diff --git a/share/html/Ticket/Elements/EditPeople b/share/html/Ticket/Elements/EditPeople
index 9ac039b..8a0ee80 100644
--- a/share/html/Ticket/Elements/EditPeople
+++ b/share/html/Ticket/Elements/EditPeople
@@ -78,7 +78,16 @@
 % while (my $role = $single_roles->Next) {
 <tr>
   <td class="label"><% $role->Name %>:</td>
+% if ($role->CurrentUserHasRight('ModifyCustomRole')) {
   <td class="value"><& /Elements/SingleUserRoleInput, role => $role, Ticket => $Ticket &></td>
+% } else {
+% my $group = $Ticket->RoleGroup($role->GroupType);
+% my $users = $group->UserMembersObj( Recursively => 0 );
+%# $users can be empty for tickets created before the custom role is added to the queue,
+%# so fall back to nobody
+%     my $user = $users->First || RT->Nobody;
+    <td class="value"><& /Elements/ShowUser, User => $user, Ticket => $Ticket &></td>
+% }
 </tr>
 
 % }
@@ -112,7 +121,7 @@
 % my $group = $Ticket->RoleGroup($role->GroupType);
 <tr>
   <td class="label"><% $role->Name %>:</td>
-  <td class="value"><& EditWatchers, TicketObj => $Ticket, Watchers => $group &></td>
+  <td class="value"><& EditWatchers, TicketObj => $Ticket, Watchers => $group, Readonly => !$role->CurrentUserHasRight('ModifyCustomRole') &></td>
 </tr>
 % }
 
diff --git a/t/customroles/web-assets.t b/t/customroles/web-assets.t
index a550b6c..a43292b 100644
--- a/t/customroles/web-assets.t
+++ b/t/customroles/web-assets.t
@@ -98,6 +98,15 @@ diag "Grant permissions on Licensee";
     }, "submitted rights form");
     $m->text_contains("Granted right 'ShowAsset' to Licensee");
 
+    my $privileged = RT::Group->new(RT->SystemUser);
+    $privileged->LoadSystemInternalGroup('Privileged');
+    $m->submit_form_ok({
+        with_fields => {
+            "SetRights-" . $privileged->Id . '-RT::Catalog-' . $catalog->id => 'SeeCustomRole',
+        },
+    }, "submitted rights form");
+    $m->text_contains("Granted right 'SeeCustomRole' to Privileged");
+
     RT::Principal::InvalidateACLCache();
 }
 

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


More information about the rt-commit mailing list