RT::Users->WhoBelongToGroups applies LimitToPrivileged filter

Hi all

I’ve noticed that the WhoBelongToGroups method of user search class
RT::Users unconditionally calls LimitToPrivileged, making it useless for
when you want to find both privileged and unprivileged users, or when
you want to find only unprivileged users where you’ve already called
LimitToUnprivileged . This behaviour isn’t documented and there’s a comment:

# Unprivileged users can't be granted real system rights.
# is this really the right thing to be saying?
$self->LimitToPrivileged();

that suggests uncertainty about whether it’s correct.

I’m running 4.07 but this code remains in 4.10 and in 4.2 git master HEAD.

The attached patch documents the current behaviour and adds a flag to
override it per-call. It applies to 4.0 and master HEAD. A pull request
has been sent with the changes:

Filed as RT ticket 22989.

Anyone who wants to apply this change in the mean time can do so by
grabbing lib/RT/Users.pm, extracting the WhoBelongToGroups method from
it, and adding it to $localrtlib/RT/Users_Local.pm like this:

use strict;
no warnings qw(redefine);
package RT::Users;

paste WhoBelongToGroups method here

1;

Craig Ringer http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

0001-Allow-RT-Users-WhoBelongToGroups-to-optionally-retur.patch (2.17 KB)

Hi all

I’ve noticed that the WhoBelongToGroups method of user search class
RT::Users unconditionally calls LimitToPrivileged, making it useless
for when you want to find both privileged and unprivileged users, or
when you want to find only unprivileged users where you’ve already
called LimitToUnprivileged . This behaviour isn’t documented and
there’s a comment:

# Unprivileged users can't be granted real system rights.
# is this really the right thing to be saying?
$self->LimitToPrivileged();

BTW, I’m about to put up another extension that needs the behaviour in
the patch.

If I want to make it run on unpatched RT for easier distribution and
installation how would you recommend doing this? I surely don’t want to
distribute a Users_Local.pm in my extension.

I think I could hot-insert the method into RT::Users with something like:

{
package RT::Users;
sub WhoBelongToGroupsIncludingPrivileged {
# …
}
}

in my module. It doesn’t have to be visible to other modules, so it’d
essentially be a method of RT::Users that’s private to my extension.

Is this a sane approach? My Perl is still a bit rusty, so I wanted to
check before putting anything out in the wild.

Craig Ringer http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

I think I could hot-insert the method into RT::Users with something like:

{
package RT::Users;
sub WhoBelongToGroupsIncludingPrivileged {
# …
}
}

in my module. It doesn’t have to be visible to other modules, so it’d
essentially be a method of RT::Users that’s private to my extension.

That injects the method into RT::Users. It will override the existing
method and all code will use it once your extension is loaded. It is
not private, as you describe.

That injects the method into RT::Users. It will override the existing
method and all code will use it once your extension is loaded. It is
not private, as you describe.

Yes, but the name isn’t referenced anywhere else so the additional
method won’t affect any other code. Private in effect because of that.

Craig Ringer http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

That injects the method into RT::Users. It will override the existing
method and all code will use it once your extension is loaded. It is
not private, as you describe.

Yes, but the name isn’t referenced anywhere else so the additional
method won’t affect any other code. Private in effect because of that.

Ah, I missed that in your example you renamed the method.

Hello,

So far we have been using short comments with Time Worked to record time in
RT, but that is not very convenient as time records are being e-mailed to
everyone just like any other comments. I am thinking of separating most time
entries into their own transaction type to keep them inside RT only, but
that would also allow us to do better reporting.

I created two simple overlay files to implement the very basic functionality
including SeeTimeEntry privilege:

https://gist.github.com/mushak/720ec19d7623ed16ac1c Transaction_Overlay.pm
https://gist.github.com/mushak/78e85a8795dd362f4e0a Queue_Overlay.pm

Is this a correct way of approaching this problem?
Can I encounter any issues during upgrades after adding new transaction
types?

I also created a sample transaction using the following code:

my $Transaction = RT::Transaction->new($RT::SystemUser);
my ( $id, $msg ) = $Transaction->Create(
Ticket => $Ticket->Id,
Type => ‘TimeEntry’,
TimeTaken => 13,
ActivateScrips => 0
);

The transaction now appears in the ticket’s history, but the ticket’s time
worked was not updated. What else do I need to do to have that updated as
well?

Best regards,
Maciek

So far we have been using short comments with Time Worked to record time in
RT, but that is not very convenient as time records are being e-mailed to
everyone just like any other comments. I am thinking of separating most time
entries into their own transaction type to keep them inside RT only, but
that would also allow us to do better reporting.

This already exists in core RT. On the Comment page, simply enter time
worked without any message and click the “Update Ticket” button.

You may also be interested in the RT::Extension::QuickAddTimeWorked
plugin which makes this process easier.

Thomas,

This already exists in core RT. On the Comment page, simply enter time worked without
any message and click the “Update Ticket” button.

Thanks. I know this exists, but I’m thinking of further extending this functionality later which would for example result in storing serialized structure rather than just text in such transactions. Using plain comments would mean all that information would be shown in ticket history and that would become nothing but noise. I would also like to be able to show all time entries to customers, but not necessarily all comments. So I am wondering whether my approach for adding/using custom transaction type is correct or what are the risks of doing that. I’m asking this because of the following comment in Transaction.pm:

{{{ sub Create

=head2 Create

Create a new transaction.

This routine should never be called by anything other than RT::Ticket.
It should not be called
from client code. Ever. Not ever. If you do this, we will hunt you down and break your kneecaps.

Maciek

Thanks. I know this exists, but I’m thinking of further extending
this functionality later which would for example result in storing
serialized structure rather than just text in such transactions.

That’s well outside the scope of RT’s normal time tracking
functionality. Why do you need a serialized structure?

Using plain comments would mean all that information would be shown
in ticket history and that would become nothing but noise. I would
also like to be able to show all time entries to customers, but not
necessarily all comments.

When you do as I described and enter time worked on the Comment page
without any actual comment message, a comment is never recorded. Only a
TimeWorked transaction is recorded. So your customers will see those
entries with ShowTicket and yet still not see comments (provided you
don’t grant them ShowTicketComments).

I think you’re misunderstanding how the update form works when you have
an empty message and some amount of time worked. It doesn’t create the
normal reply/comment transaction.

So I am wondering whether my approach for
adding/using custom transaction type is correct or what are the risks
of doing that. I’m asking this because of the following comment in
Transaction.pm:

Transactions are one of the most used parts of RT, and some of the
hairiest to customize. It’s a bit better in 4.2, but that’s not
released yet. If you go down this path, be ready to write Perl and
support your customizations down the road. I suggest doing it as a
cleanly written extension.

Calling the _NewTransaction method provided by RT::Record is preferred
over RT::Transaction->Create; you should listen to the documentation.
grep the code for examples.