[Rt-commit] rt branch, 4.4/custom-role-rights, created. rt-4.4.4-469-g1bd54a46ea

? sunnavy sunnavy at bestpractical.com
Fri May 21 18:28:47 EDT 2021


The branch, 4.4/custom-role-rights has been created
        at  1bd54a46ea00fc895bf156678aac5c981dde567f (commit)

- Log -----------------------------------------------------------------
commit e675acf519548f1814ad08a9d4a93a6e4ae45076
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 bfa88703c2..39ef613164 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'}
 
@@ -1019,112 +1020,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 afeb44959e..7968057fcf 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 e63426dac1..e0bc54cfe8 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
 
@@ -349,6 +350,8 @@ sub NotAddedTo {
         ->NotAddedTo( CustomRole => $self );
 }
 
+sub IsGlobal { return 0 }
+
 =head2 AddToObject
 
 Adds (applies) this custom role to the provided object (ObjectId).
@@ -488,6 +491,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 b6f2431dd1..451fc51916 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
@@ -190,6 +193,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 0000000000..64bcb46ac8
--- /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 0000000000..7dd7c3ab32
--- /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 108918efd7e1fbaba9b1487a7260b04c64c965cf
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 f9d629ea04..3cf3c88354 100644
--- a/lib/RT/Asset.pm
+++ b/lib/RT/Asset.pm
@@ -528,7 +528,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
@@ -543,7 +543,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 6c14c5c8b3..f5b9af802e 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 378f521819..b0c8c890b1 100644
--- a/lib/RT/Interface/Web.pm
+++ b/lib/RT/Interface/Web.pm
@@ -4157,8 +4157,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 c85ad437d9..f8dc7a3572 100644
--- a/lib/RT/Transaction.pm
+++ b/lib/RT/Transaction.pm
@@ -883,9 +883,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;
     }
 
@@ -1478,6 +1476,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 cfb78f2e01..8e49af9293 100644
--- a/share/html/Asset/Elements/EditRoleMembers
+++ b/share/html/Asset/Elements/EditRoleMembers
@@ -49,6 +49,7 @@
 $Object
 $Role
 $Recursively => 0
+$Readonly    => 0
 </%args>
 <%init>
 my $Group = $Object->RoleGroup($Role);
@@ -58,12 +59,18 @@ my $field_name = "RemoveRoleMember-" . $Group->Name;
 % my $Users = $Group->UserMembersObj( Recursively => $Recursively );
 % if ($Object->Role($Role)->{Single}) {
 % my $user = $Users->First || RT->Nobody;
+% if ($Readonly) {
+<& /Elements/ShowUser, User => $Users->First &>
+% } else {
 <input type="text" value="<% $user->Name %>" name="SetRoleMember-<% $Role %>" id="SetRoleMember-<% $Role %>" 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>
@@ -73,7 +80,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 dc324291ab..f6d605dcb2 100644
--- a/share/html/Elements/ColumnMap
+++ b/share/html/Elements/ColumnMap
@@ -148,14 +148,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);
                     }
@@ -166,14 +166,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 6214be51a5..5f021c2fc0 100644
--- a/share/html/Elements/SelectWatcherType
+++ b/share/html/Elements/SelectWatcherType
@@ -75,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 c5a10c7fcd..5cd4141a08 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', AutocompleteType => 'Principals' &>
 </td></tr>
diff --git a/share/html/Ticket/Elements/EditWatchers b/share/html/Ticket/Elements/EditWatchers
index cfc454de7f..9bf46ca18b 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' ) ) { 
 <& /Elements/ShowUser, User => $member &> <& /Elements/ShowUserEmailFrequency, User => $member, Ticket => $TicketObj &>
 % } else {
@@ -76,4 +79,5 @@ my $Members = $Watchers->MembersObj;
 <%ARGS>
 $TicketObj => undef
 $Watchers => undef
+$Readonly => 0
 </%ARGS>

commit 213179f2bd9123fcb147a83b495ade7948af48d7
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 f5b9af802e..da5a50dee6 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/Group.pm b/lib/RT/Group.pm
index 7d5a02407a..bd0f738cb2 100644
--- a/lib/RT/Group.pm
+++ b/lib/RT/Group.pm
@@ -1743,6 +1743,7 @@ sub _CustomRoleObj {
         if ($id) {
             my $role = RT::CustomRole->new($self->CurrentUser);
             $role->Load($id);
+            $role->SetContextObject( $self->{context_object} ) if $self->{context_object};
             return $role;
         }
     }
diff --git a/lib/RT/Queue.pm b/lib/RT/Queue.pm
index bf0b158a22..133419c46d 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 f347bcc743..a28dab8448 100644
--- a/lib/RT/Record/Role/Roles.pm
+++ b/lib/RT/Record/Role/Roles.pm
@@ -349,6 +349,7 @@ sub RoleGroup {
             Object  => $self,
             Name    => $name,
         );
+        $group->{context_object} = $self;
     }
     return $group;
 }
@@ -810,6 +811,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 f6d605dcb2..62e34043a4 100644
--- a/share/html/Elements/ColumnMap
+++ b/share/html/Elements/ColumnMap
@@ -172,6 +172,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 17c09b5af3..40c87b56da 100644
--- a/share/html/Search/Elements/BuildFormatString
+++ b/share/html/Search/Elements/BuildFormatString
@@ -128,6 +128,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 4d120305c4..7c79bfd099 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 85e04c881c..ab6354f601 100644
--- a/share/html/Ticket/Create.html
+++ b/share/html/Ticket/Create.html
@@ -186,6 +186,7 @@
 </tr>
 
 % my $roles = $QueueObj->CustomRoles;
+% $roles->SetContextObject($QueueObj);
 % $roles->LimitToMultipleValue;
 % $m->callback( CallbackName => 'ModifyCustomRoles', ARGSRef => \%ARGS, CustomRoles => $roles );
 % while (my $role = $roles->Next) {
diff --git a/share/html/Ticket/Elements/EditBasics b/share/html/Ticket/Elements/EditBasics
index ca09867a76..b30e7fa14c 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;
     $m->callback( CallbackName => 'ModifyCustomRoles', %ARGS, CustomRoles => $roles);
     while (my $role = $roles->Next) {
diff --git a/share/html/Ticket/Elements/EditPeople b/share/html/Ticket/Elements/EditPeople
index bdb276a5d5..03d0bff330 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;
 % $m->callback( CustomRoles => $single_roles, SingleRoles => 1, Ticket => $Ticket, %ARGS, CallbackName => 'ModifyCustomRoles' );
 % while (my $role = $single_roles->Next) {
@@ -112,6 +113,7 @@
 </tr>
 
 % my $multi_roles = $Ticket->QueueObj->CustomRoles;
+% $multi_roles->SetContextObject($Ticket);
 % $multi_roles->LimitToMultipleValue;
 % $m->callback( CustomRoles => $multi_roles, SingleRoles => 0, Ticket => $Ticket, %ARGS, CallbackName => 'ModifyCustomRoles' );
 % while (my $role = $multi_roles->Next) {
diff --git a/share/html/Ticket/Elements/ShowPeople b/share/html/Ticket/Elements/ShowPeople
index 418a0c41b4..256dfe06ca 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;
 % $m->callback( CustomRoles => $single_roles, SingleRoles => 1, Ticket => $Ticket, %ARGS, CallbackName => 'ModifyCustomRoles' );
 % while (my $role = $single_roles->Next) {
@@ -87,6 +88,7 @@
   </tr>
 
 % my $multi_roles = $Ticket->QueueObj->CustomRoles;
+% $multi_roles->SetContextObject($Ticket);
 % $multi_roles->LimitToMultipleValue;
 % $m->callback( CustomRoles => $multi_roles, SingleRoles => 0, Ticket => $Ticket, %ARGS, CallbackName => 'ModifyCustomRoles' );
 % while (my $role = $multi_roles->Next) {

commit 1ee772eff0c6fbb39e012aee826df314acc1dea4
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 a28dab8448..d083ac8a4e 100644
--- a/lib/RT/Record/Role/Roles.pm
+++ b/lib/RT/Record/Role/Roles.pm
@@ -756,7 +756,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 7e233a488001eada7c2f652f5bdb5fe778edacb1
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 da5a50dee6..4882eb50ae 100644
--- a/lib/RT/Catalog.pm
+++ b/lib/RT/Catalog.pm
@@ -86,6 +86,9 @@ __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
+__PACKAGE__->AddRight( Staff   => SetInitialCustomRole  => 'Add custom role members only at object creation time'); # loc
 
 RT::ACE->RegisterCacheHandler(sub {
     my %args = (
diff --git a/lib/RT/CustomRole.pm b/lib/RT/CustomRole.pm
index e0bc54cfe8..add262cd65 100644
--- a/lib/RT/CustomRole.pm
+++ b/lib/RT/CustomRole.pm
@@ -56,9 +56,15 @@ 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( Admin   => AdminCustomRoles      => 'Create, modify and delete custom roles'); # loc
+__PACKAGE__->AddRight( General => SeeCustomRole         => 'View custom roles'); # loc
+__PACKAGE__->AddRight( Staff   => ModifyCustomRole      => 'Add, modify and delete custom role members'); # loc
+__PACKAGE__->AddRight( Staff   => SetInitialCustomRole  => 'Add custom role members only at object creation time'); # loc
+
 =head1 NAME
 
 RT::CustomRole - user-defined role groups
diff --git a/lib/RT/Queue.pm b/lib/RT/Queue.pm
index 133419c46d..9e37b6f85b 100644
--- a/lib/RT/Queue.pm
+++ b/lib/RT/Queue.pm
@@ -111,6 +111,9 @@ __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( Staff   => SetInitialCustomRole  => 'Add custom role members only at object creation time'); # 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 0000000000..9d17e5a5bd
--- /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 0000000000..a096533379
--- /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 a6415e194f..233049c6c9 100644
--- a/share/html/Elements/Tabs
+++ b/share/html/Elements/Tabs
@@ -442,8 +442,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 3db9c0d591731beab64ae0d626d77fe173f50fef
Author: Shawn M Moore <shawn at bestpractical.com>
Date:   Fri Jun 2 22:25:11 2017 +0000

    Enforce SeeCustomRole, ModifyCustomRole, SetInitialCustomRole and AdminCustomRoles

diff --git a/lib/RT/Asset.pm b/lib/RT/Asset.pm
index 3cf3c88354..34304fab29 100644
--- a/lib/RT/Asset.pm
+++ b/lib/RT/Asset.pm
@@ -322,7 +322,22 @@ 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( ForCreation => 1 );
+    while (my $role = $custom_roles->Next) {
+        $acls{$role->GroupType} = sub {
+            for my $right (qw/ModifyCustomRole SetInitialCustomRole/) {
+                return 1 if $self->CurrentUser->HasRight(
+                    Right  => $right,
+                    Object => $role,
+                );
+            }
+            return 0;
+        };
+    }
+
+    push @non_fatal_errors, $self->_AddRolesOnCreate( $roles, %acls );
 
     # Add CFs
     foreach my $key (keys %args) {
@@ -524,6 +539,7 @@ Checks I<ModifyAsset> before calling L<RT::Record::Role::Roles/_AddRoleMember>.
 
 sub AddRoleMember {
     my $self = shift;
+    my %args = @_;
 
     return (0, $self->loc("No permission to modify this asset"))
         unless $self->CurrentUserHasRight("ModifyAsset");
@@ -550,6 +566,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/Catalog.pm b/lib/RT/Catalog.pm
index 4882eb50ae..77e1070434 100644
--- a/lib/RT/Catalog.pm
+++ b/lib/RT/Catalog.pm
@@ -371,7 +371,7 @@ sub RoleGroup {
     }
 }
 
-=head2 CustomRoles
+=head2 CustomRoles ForCreation => BOOL
 
 Returns an L<RT::CustomRoles> object containing all catalog-specific roles.
 
@@ -379,6 +379,7 @@ Returns an L<RT::CustomRoles> object containing all catalog-specific roles.
 
 sub CustomRoles {
     my $self = shift;
+    my %args = @_;
 
     my $roles = RT::CustomRoles->new( $self->CurrentUser );
     if ( $self->CurrentUserHasRight('ShowCatalog') ) {
@@ -387,6 +388,11 @@ sub CustomRoles {
         $roles->LimitToLookupType(RT::Asset->CustomFieldLookupType);
         $roles->ApplySortOrder;
     }
+
+    if ( $args{ForCreation} ) {
+        $roles->{include_set_initial} = 1;
+    }
+
     return ($roles);
 }
 
diff --git a/lib/RT/CustomRole.pm b/lib/RT/CustomRole.pm
index add262cd65..8d05721a88 100644
--- a/lib/RT/CustomRole.pm
+++ b/lib/RT/CustomRole.pm
@@ -499,11 +499,23 @@ sub GroupType {
 
 =head2 CurrentUserCanSee
 
+If the user has SeeCustomRole they can see this custom role and its members.
+
+Otherwise, if the user has SetInitialCustomRole and this is being used in a
+"create" context, then they can see this custom role and its details. This
+allows you to set up custom roles that are only visible on create pages and
+are then inaccessible.
+
 =cut
 
 sub CurrentUserCanSee {
     my $self = shift;
-    return 1;
+    return 1 if $self->CurrentUserHasRight('SeeCustomRole');
+
+    return 1 if $self->{include_set_initial}
+             && $self->CurrentUserHasRight('SetInitialCustomRole');
+
+    return 0;
 }
 
 =head2 id
@@ -582,6 +594,11 @@ Returns (1, 'Status message') on success and (0, 'Error Message') on failure.
 sub SetLookupType {
     my $self = shift;
     my $lookup = shift;
+
+    unless ( $self->CurrentUserHasRight('AdminCustomRoles') ) {
+        return ( 0, $self->loc('Permission Denied') );
+    }
+
     if ( $lookup ne $self->LookupType ) {
         # Okay... We need to invalidate our existing relationships
         RT::ObjectCustomRole->new($self->CurrentUser)->DeleteAll( CustomRole => $self );
@@ -738,6 +755,21 @@ sub SetDisabled {
     }
 }
 
+sub _Set {
+    my $self = shift;
+    my %args = @_;
+    unless ( $self->CurrentUserHasRight('AdminCustomRoles') ) {
+        return ( 0, $self->loc('Permission Denied') );
+    }
+    return $self->SUPER::_Set(@_);
+}
+
+sub _Value {
+    my $self = shift;
+    return undef unless $self->Id && $self->CurrentUserCanSee;
+    return $self->__Value(@_);
+}
+
 sub _CoreAccessible {
     {
         id =>
diff --git a/lib/RT/CustomRoles.pm b/lib/RT/CustomRoles.pm
index 451fc51916..e988b29ae7 100644
--- a/lib/RT/CustomRoles.pm
+++ b/lib/RT/CustomRoles.pm
@@ -205,6 +205,7 @@ sub AddRecord {
     my ($record) = @_;
 
     $record->SetContextObject( $self->ContextObject );
+    $record->{include_set_initial} = $self->{include_set_initial};
 
     return unless $record->CurrentUserCanSee;
 
diff --git a/lib/RT/Queue.pm b/lib/RT/Queue.pm
index 9e37b6f85b..7673647dea 100644
--- a/lib/RT/Queue.pm
+++ b/lib/RT/Queue.pm
@@ -472,7 +472,7 @@ sub TicketTransactionCustomFields {
     return ($cfs);
 }
 
-=head2 CustomRoles
+=head2 CustomRoles ForCreation => BOOL
 
 Returns an L<RT::CustomRoles> object containing all queue-specific roles.
 
@@ -480,6 +480,7 @@ Returns an L<RT::CustomRoles> object containing all queue-specific roles.
 
 sub CustomRoles {
     my $self = shift;
+    my %args = @_;
 
     my $roles = RT::CustomRoles->new( $self->CurrentUser );
     if ( $self->CurrentUserHasRight('SeeQueue') ) {
@@ -491,6 +492,11 @@ sub CustomRoles {
     else {
         $roles->Limit( FIELD => 'id', VALUE => 0, SUBCLAUSE => 'ACL' );
     }
+
+    if ( $args{ForCreation} ) {
+        $roles->{include_set_initial} = 1;
+    }
+
     return ($roles);
 }
 
diff --git a/lib/RT/Ticket.pm b/lib/RT/Ticket.pm
index add1c374d8..180b9fe651 100644
--- a/lib/RT/Ticket.pm
+++ b/lib/RT/Ticket.pm
@@ -513,6 +513,19 @@ sub Create {
         },
     );
 
+    my $custom_roles = $QueueObj->CustomRoles( ForCreation => 1 );
+    while (my $role = $custom_roles->Next) {
+        $acls{$role->GroupType} = sub {
+            for my $right (qw/ModifyCustomRole SetInitialCustomRole/) {
+                return 1 if $self->CurrentUser->HasRight(
+                    Right  => $right,
+                    Object => $role,
+                );
+            }
+            return 0;
+        };
+    }
+
     # Populate up the role groups.  This call modifies $roles.
     push @non_fatal_errors, $self->_AddRolesOnCreate( $roles, %acls );
 
@@ -660,6 +673,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 24adcb559c..b5894252e9 100644
--- a/share/html/Asset/Elements/EditCatalogPeople
+++ b/share/html/Asset/Elements/EditCatalogPeople
@@ -51,9 +51,11 @@ $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, Object => $Object, Role => $role &>
+  <& EditRoleMembers, Object => $Object, Role => $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 0e9f690294..b7b1f1ad7d 100644
--- a/share/html/Asset/Elements/EditPeople
+++ b/share/html/Asset/Elements/EditPeople
@@ -47,6 +47,14 @@
 %# END BPS TAGGED BLOCK }}}
 <table border="0" cellpadding="0" cellspacing="0">
 % for my $role ( $object->Roles ) {
+% my $custom_role = $object->CustomRoleObj($role);
+% next if $custom_role
+%    && (
+%    $AssetObj->Id
+%    ? !$custom_role->CurrentUserHasRight('ModifyCustomRole')
+%    : (        !$custom_role->CurrentUserHasRight('ModifyCustomRole')
+%            && !$custom_role->CurrentUserHasRight('SetInitialCustomRole') )
+%       );
 <tr class="asset-people-<% CSSClass($role) %>">
 <td class="label">
 <% $object->LabelForRole($role) %>:
@@ -56,7 +64,6 @@
 </td>
 </tr>
 
-% my $custom_role = $object->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 0dcf70fece..8eb7eef138 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 2dd9193b15..8523ec6d12 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 5f021c2fc0..1dc60f6d32 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"
@@ -68,6 +70,7 @@ else {
 }
 
 $m->callback( Types => \@types, %ARGS, CallbackName => 'ModifyWatcherTypes' );
+my $Object = $Ticket || $Queue;
 </%INIT>
 <%ARGS>
 $AllowNull => 1
diff --git a/share/html/Ticket/Create.html b/share/html/Ticket/Create.html
index ab6354f601..5703daa65a 100644
--- a/share/html/Ticket/Create.html
+++ b/share/html/Ticket/Create.html
@@ -185,11 +185,12 @@
   </td>
 </tr>
 
-% my $roles = $QueueObj->CustomRoles;
+% my $roles = $QueueObj->CustomRoles(ForCreation => 1);
 % $roles->SetContextObject($QueueObj);
 % $roles->LimitToMultipleValue;
 % $m->callback( CallbackName => 'ModifyCustomRoles', ARGSRef => \%ARGS, CustomRoles => $roles );
 % while (my $role = $roles->Next) {
+% next unless $role->CurrentUserHasRight('ModifyCustomRole') || $role->CurrentUserHasRight('SetInitialCustomRole');
 <tr>
 <td class="label">
 <% $role->Name %>:
diff --git a/share/html/Ticket/Elements/EditBasics b/share/html/Ticket/Elements/EditBasics
index b30e7fa14c..a4faf8e9fc 100644
--- a/share/html/Ticket/Elements/EditBasics
+++ b/share/html/Ticket/Elements/EditBasics
@@ -138,11 +138,19 @@ unless ( @fields ) {
 my @role_fields;
 
 unless ($ExcludeCustomRoles) {
-    my $roles = $QueueObj->CustomRoles;
+    my $roles = $QueueObj->CustomRoles( $TicketObj && $TicketObj->Id ? () : ( ForCreation => 1 ) );
     $roles->SetContextObject($TicketObj || $QueueObj);
     $roles->LimitToSingleValue;
     $m->callback( CallbackName => 'ModifyCustomRoles', %ARGS, CustomRoles => $roles);
     while (my $role = $roles->Next) {
+        if ( $TicketObj && $TicketObj->Id ) {
+            next unless $role->CurrentUserHasRight('ModifyCustomRole');
+        }
+        else {
+            next
+                unless $role->CurrentUserHasRight('ModifyCustomRole')
+                || $role->CurrentUserHasRight('SetInitialCustomRole');
+        }
         push @role_fields, {
             name => $role->Name,
             comp => '/Elements/SingleUserRoleInput',
diff --git a/share/html/Ticket/Elements/EditPeople b/share/html/Ticket/Elements/EditPeople
index 03d0bff330..777f3a6abe 100644
--- a/share/html/Ticket/Elements/EditPeople
+++ b/share/html/Ticket/Elements/EditPeople
@@ -79,7 +79,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>
 
 % }
@@ -122,7 +131,7 @@
   <td class="label">
     <input type="checkbox" class="checkbox" onclick="setCheckbox(this, /^Ticket-DeleteWatcher-Type-RT::CustomRole-<% $role->Id %>-/)">
     <% $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>
 % }
 

commit 7b11eca4c2297e178cf64e105d66c2b41d3365cd
Author: sunnavy <sunnavy at bestpractical.com>
Date:   Sat May 22 03:26:37 2021 +0800

    Update tests for the new added SeeCustomRole right

diff --git a/t/customroles/basic.t b/t/customroles/basic.t
index 10668f073f..48b3f3fc67 100644
--- a/t/customroles/basic.t
+++ b/t/customroles/basic.t
@@ -150,7 +150,7 @@ diag 'roles now added to queues' if $ENV{'TEST_VERBOSE'};
         ok( !$queue->HasRole( $sales->GroupType ), 'HasRole returns false for users without rights' );
     }
 
-    RT::Test->set_rights( { Principal => $alice->PrincipalObj, Right => ['SeeQueue'] } );
+    RT::Test->set_rights( { Principal => $alice->PrincipalObj, Right => ['SeeQueue', 'SeeCustomRole'] } );
 
     my @users = ( RT->SystemUser, $alice );
     for my $user ( @users ) {
diff --git a/t/customroles/web-assets.t b/t/customroles/web-assets.t
index 03118dc880..5cd919782b 100644
--- a/t/customroles/web-assets.t
+++ b/t/customroles/web-assets.t
@@ -98,6 +98,15 @@ diag "Grant permissions on Licensee";
     $m->text_contains("Granted right 'ShowAsset' to Licensee");
     $m->text_contains("Granted right 'ShowCatalog' 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();
 }
 
diff --git a/t/ticket/add-watchers.t b/t/ticket/add-watchers.t
index c7cb9196f7..7afbd6ed33 100644
--- a/t/ticket/add-watchers.t
+++ b/t/ticket/add-watchers.t
@@ -50,8 +50,10 @@ my $principal = $user->PrincipalObj;
 ok( $principal, "principal loaded" );
 $principal->GrantRight( Right => 'ShowTicket', Object => $queue );
 $principal->GrantRight( Right => 'SeeQueue'  , Object => $queue );
+$principal->GrantRight( Right => 'SeeCustomRole', Object => $queue );
 
 ok(  $user->HasRight( Right => 'SeeQueue',     Object => $queue ), "user can see queue" );
+ok(  $user->HasRight( Right => 'SeeCustomRole', Object => $queue ), "user can see custom roles" );
 ok(  $user->HasRight( Right => 'ShowTicket',   Object => $queue ), "user can show queue tickets" );
 ok( !$user->HasRight( Right => 'ModifyTicket', Object => $queue ), "user can't modify queue tickets" );
 ok( !$user->HasRight( Right => 'Watch',        Object => $queue ), "user can't watch queue tickets" );

commit 0fd15e8031fc14615dfbad49f76d4ea20bcc44d2
Author: sunnavy <sunnavy at bestpractical.com>
Date:   Sat May 22 05:49:53 2021 +0800

    Rename "AdminCustomRoles" to singular form "AdminCustomRole"
    
    Previously it was a system-only right, where plural form is fine. Since
    it's added to each custom role, singular form is more appropriate.

diff --git a/etc/upgrade/4.4.5/content b/etc/upgrade/4.4.5/content
index 45b13c1eee..331a10d00b 100644
--- a/etc/upgrade/4.4.5/content
+++ b/etc/upgrade/4.4.5/content
@@ -45,4 +45,17 @@ our @Final = (
             }
         }
     },
+    sub {
+        my $acl = RT::ACL->new( RT->SystemUser );
+        $acl->Limit(
+            FIELD => 'RightName',
+            VALUE => 'AdminCustomRoles',
+        );
+        while ( my $ace = $acl->Next ) {
+            my ( $ret, $msg ) = $ace->__Set( Field => 'RightName', Value => 'AdminCustomRole' );
+            if ( !$ret ) {
+                die "Couldn't update right name for ACE #" . $ace->id . ": $msg";
+            }
+        }
+    },
 );
diff --git a/lib/RT/CustomRole.pm b/lib/RT/CustomRole.pm
index 8d05721a88..35ab9fb22f 100644
--- a/lib/RT/CustomRole.pm
+++ b/lib/RT/CustomRole.pm
@@ -60,7 +60,7 @@ with "RT::Record::Role::Rights",
      "RT::Record::Role::LookupType",
      "RT::Record::Role::ContextObject";
 
-__PACKAGE__->AddRight( Admin   => AdminCustomRoles      => 'Create, modify and delete custom roles'); # loc
+__PACKAGE__->AddRight( Admin   => AdminCustomRole       => 'Create, modify and delete custom roles'); # loc
 __PACKAGE__->AddRight( General => SeeCustomRole         => 'View custom roles'); # loc
 __PACKAGE__->AddRight( Staff   => ModifyCustomRole      => 'Add, modify and delete custom role members'); # loc
 __PACKAGE__->AddRight( Staff   => SetInitialCustomRole  => 'Add custom role members only at object creation time'); # loc
@@ -106,7 +106,7 @@ sub Create {
         @_,
     );
 
-    unless ( $self->CurrentUser->HasRight(Object => $RT::System, Right => 'AdminCustomRoles') ) {
+    unless ( $self->CurrentUser->HasRight(Object => $RT::System, Right => 'AdminCustomRole') ) {
         return (0, $self->loc('Permission Denied'));
     }
 
@@ -293,7 +293,7 @@ Delete this object. You should Disable instead.
 sub Delete {
     my $self = shift;
 
-    unless ( $self->CurrentUserHasRight('AdminCustomRoles') ) {
+    unless ( $self->CurrentUserHasRight('AdminCustomRole') ) {
         return ( 0, $self->loc('Permission Denied') );
     }
 
@@ -396,7 +396,7 @@ sub AddToObject {
     $args{'ObjectId'} = $object->id;
 
     return ( 0, $self->loc('Permission Denied') )
-        unless $object->CurrentUserHasRight('AdminCustomRoles');
+        unless $object->CurrentUserHasRight('AdminCustomRole');
 
     my $rec = RT::ObjectCustomRole->new( $self->CurrentUser );
     my ( $status, $add ) = $rec->Add( %args, CustomRole => $self );
@@ -440,7 +440,7 @@ sub RemoveFromObject {
     $args{'ObjectId'} = $object->id;
 
     return ( 0, $self->loc('Permission Denied') )
-        unless $object->CurrentUserHasRight('AdminCustomRoles');
+        unless $object->CurrentUserHasRight('AdminCustomRole');
 
     my $rec = RT::ObjectCustomRole->new( $self->CurrentUser );
     $rec->LoadByCols( CustomRole => $self->id, ObjectId => $args{'ObjectId'} );
@@ -595,7 +595,7 @@ sub SetLookupType {
     my $self = shift;
     my $lookup = shift;
 
-    unless ( $self->CurrentUserHasRight('AdminCustomRoles') ) {
+    unless ( $self->CurrentUserHasRight('AdminCustomRole') ) {
         return ( 0, $self->loc('Permission Denied') );
     }
 
@@ -758,7 +758,7 @@ sub SetDisabled {
 sub _Set {
     my $self = shift;
     my %args = @_;
-    unless ( $self->CurrentUserHasRight('AdminCustomRoles') ) {
+    unless ( $self->CurrentUserHasRight('AdminCustomRole') ) {
         return ( 0, $self->loc('Permission Denied') );
     }
     return $self->SUPER::_Set(@_);
diff --git a/lib/RT/System.pm b/lib/RT/System.pm
index 05ef847388..bb18906cb9 100644
--- a/lib/RT/System.pm
+++ b/lib/RT/System.pm
@@ -83,7 +83,7 @@ use Data::GUID;
 __PACKAGE__->AddRight( Admin   => SuperUser           => 'Do anything and everything'); # loc
 __PACKAGE__->AddRight( Staff   => ShowUserHistory     => 'Show history of public user properties'); # loc
 __PACKAGE__->AddRight( Admin   => AdminUsers          => 'Create, modify and delete users'); # loc
-__PACKAGE__->AddRight( Admin   => AdminCustomRoles    => 'Create, modify and delete custom roles'); # loc
+__PACKAGE__->AddRight( Admin   => AdminCustomRole     => 'Create, modify and delete custom roles'); # loc
 __PACKAGE__->AddRight( Staff   => ModifySelf          => "Modify one's own RT account"); # loc
 __PACKAGE__->AddRight( Staff   => ShowArticlesMenu    => 'Show Articles menu'); # loc
 __PACKAGE__->AddRight( Admin   => ShowConfigTab       => 'Show Admin menu'); # loc
diff --git a/share/html/Elements/Tabs b/share/html/Elements/Tabs
index 233049c6c9..2bf74bf0f9 100644
--- a/share/html/Elements/Tabs
+++ b/share/html/Elements/Tabs
@@ -97,7 +97,7 @@ my $build_admin_menu = sub {
         $cfs->child( create => title => loc('Create'), path => "/Admin/CustomFields/Modify.html?Create=1" );
     }
 
-    if ( $session{'CurrentUser'}->HasRight( Object => RT->System, Right => 'AdminCustomRoles' ) ) {
+    if ( $session{'CurrentUser'}->HasRight( Object => RT->System, Right => 'AdminCustomRole' ) ) {
         my $roles = $admin->child( 'custom-roles' =>
             title       => loc('Custom Roles'),
             description => loc('Manage custom roles'),

commit 1bd54a46ea00fc895bf156678aac5c981dde567f
Author: sunnavy <sunnavy at bestpractical.com>
Date:   Sat May 22 06:02:15 2021 +0800

    Add docs for new custom role rights

diff --git a/docs/UPGRADING-4.4 b/docs/UPGRADING-4.4
index 9c712c57d6..d1d71879bb 100644
--- a/docs/UPGRADING-4.4
+++ b/docs/UPGRADING-4.4
@@ -667,6 +667,17 @@ After you run this, you may have significantly reduced the number of records
 in your CachedGroupMembers table, and may need to tell your database to
 refresh indexes/statistics.
 
+=item * New rights for custom roles
+
+Users now need C<SeeCustomRole> and C<ModifyCustomRole> to view and modify
+ticket/asset custom role members, respectively.
+
+C<SetInitialCustomRole> is to set custom role members on ticket/asset create
+only, quite like C<SetInitialCustomField>.
+
+C<AdminCustomRoles> is renamed to C<AdminCustomRole> and can be granted at
+custom role level too.
+
 =back
 
 =cut

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


More information about the rt-commit mailing list