Admin: 2 revisions imported

2016-04-06T20:39:27Z

2 revisions imported

New page

I was reading this code in RT::User_Overlay.pm::[[SetOwner]]():

<nowiki>if ( $self->OwnerObj->Id == $RT::Nobody->Id ) {
unless ( $self->CurrentUserHasRight('ModifyTicket')
|| $self->CurrentUserHasRight('TakeTicket') ) {
return ( 0, $self->loc("Permission Denied") );
}
}

# see if it's a steal
elsif ( $self->OwnerObj->Id != $RT::Nobody->Id
&& $self->OwnerObj->Id != $self->CurrentUser->id ) {

unless ( $self->CurrentUserHasRight('ModifyTicket')
|| $self->CurrentUserHasRight('StealTicket') ) {
return ( 0, $self->loc("Permission Denied") );
}
}
else {
unless ( $self->CurrentUserHasRight('ModifyTicket') ) {
return ( 0, $self->loc("Permission Denied") );
}
}
my $NewOwnerObj = RT::User->new( $self->CurrentUser );
my $OldOwnerObj = $self->OwnerObj;

$NewOwnerObj->Load($NewOwner);
if ( !$NewOwnerObj->Id ) {
return ( 0, $self->loc("That user does not exist") );
}

# If this ticket has an owner and it's not us, and we're not
# stealing it or forcing it, we're trying to assign it to someone
# that isn't us.
if ( ( $Type ne 'Steal' ) and ( $Type ne 'Force' ) and
( $self->OwnerObj->Id != $RT::Nobody->Id ) and
( $self->CurrentUser->Id ne $self->OwnerObj->Id() )
) {
return ( 0,
$self->loc("You can only reassign tickets that you own or that are unowned" ) );
}

#If we've specified a new owner and that user can't modify the ticket
elsif ( ( $NewOwnerObj->Id )
and ( !$NewOwnerObj->HasRight( Right => 'OwnTicket',
Object => $self ) )
) {
return ( 0, $self->loc("That user may not own tickets in that queue") );
}

#If the ticket has an owner and it's the new owner, we don't need
#To do anything
elsif ( ( $self->OwnerObj )
and ( $NewOwnerObj->Id eq $self->OwnerObj->Id ) ) {
return ( 0, $self->loc("That user already owns that ticket") );
}

</nowiki>

... with an eye to understanding which permissions did what. Here's the current logic in plain English, referring to [[CurrentUser]] as "you" and tickets owned by the Nobody user as "unowned":

1. Check this logic; if you fail, no change takes place
If it's unowned:
You must have TakeTicket or ModifyTicket to change the owner
Otherwise, if it's not unowned and not owned by you:
You must have StealTicket or ModifyTicket to change the owner
Otherwise:
You must have ModifyTicket to change the owner

2. The new owner must exist in RT

3. Check this set of logic

If you're not stealing the ticket and you're not forcing an ownership
change and the ticket is not unowned and not owned by you:
You can't change the owner no matter what permissions you have
Otherwise, if the new owner exists and doesn't have the OwnTicket right:
You can't change the owner to them no matter what
Otherwise, if you're changing the owner to the person who already
owns this ticket:
You can't change the owner because it's unnecessary

4. Change the owner to the new owner.

Reading the logic in section 1 I've inferred that [[ModifyTicket]] should allow you to change the ownership in any way you like as long as the new owner exists and has the [[OwnTicket]] right. The [[ModifyTicket]] wiki page seems to back this up. /'''Please correct me if I'm mistaken.'''/

It seems like there are three things that should be addressed:

# We should check the new owner's existence and [[OwnTicket]] right first; if it fails, it renders the rest of the logic moot.
# /*We should have a [[GiveTicket]] right*/ -- this is the big change proposed here
# You should be able to assign an owned ticket to someone if you have [[StealTicket]]+[[GiveTicket]]

Further, it seems like there are 7 possibly valid from/to ownership transitions:

* Nobody -> Self
** A '''Take''' transaction which should require [[TakeTicket]]
* Nobody -> Other
** A '''Give''' transaction (really '''Take+Give''') which should require [[TakeTicket]] and [[GiveTicket]]
* Self -> Nobody
** An '''Untake''' transaction which should not require any right
* Other -> Nobody
** An '''Untake''' transaction /(currently Give)/ (really Steal+Untake) which should require [[StealTicket]]
* Self -> Other
** A '''Give''' transaction which should require [[GiveTicket]]
* Other -> Self
** A '''Steal''' transaction which should require [[StealTicket]]
* Other A -> Other B
** A '''Give''' transaction (really Steal+Give) which should require [[StealTicket]] and [[GiveTicket]]

So we could choose to set the transaction type internally based on the order of

Thus, it seems like the logic should go:

1. New owner must exist and have OwnTicket

2. Check this logic; if you fail, no change takes place:
If it's currently unowned:
You must have TakeTicket or ModifyTicket
Otherwise, if the old owner isn't you:
You must have StealTicket or ModifyTicket

If the new owner isn't Nobody and isn't you:
You must have GiveTicket or ModifyTicket

3. Change the owner to the new owner

In code, this might resemble (warning: this was written in the wiki ... trust it not):

my $NewOwnerObj = RT::User->new( $self->CurrentUser );
my $OldOwnerObj = $self->OwnerObj;

$NewOwnerObj->Load($NewOwner);
if ( ! $NewOwnerObj->Id ) {
return ( 0, $self->loc("That user does not exist") );
} elsif ( ! $NewOwnerObj->HasRight('OwnTicket') ) {
return ( 0, $self->loc("That user may not own tickets in that queue") );
}

if ( $OldOwnerObj->Id == $RT::Nobody->Id )
unless ( $self->CurrentUserHasRight('TakeTicket') ||
$self->CurrentUserHasRight('ModifyTicket') ) {
return ( 0, $self->loc("Permission Denied") );
}

$Type = "Take";
} elsif ( $OldOwnerObj->Id != $self->CurrentUser->Id ) {
unless ( $self->CurrentUserHasRight('StealTicket') ||
$self->CurrentUserHasRight('ModifyTicket') ) {
return ( 0, $self->loc("Permission Denied") );
}

$Type = "Steal";
}

if ( $NewOwnerObj->Id == $RT::Nobody->Id ) {
$Type = "Untake";
} elsif ( $NewOwnerObj->Id != $self->CurrentUser->Id ) {
unless ( $self->CurrentUserHasRight('GiveTicket') ||
$self->CurrentUserHasRight('ModifyTicket') ) {
return ( 0, $self->loc("Permission Denied") );
}

$Type = "Give";
}

...

TicketOwnershipPermissions - Revision history

Admin: 2 revisions imported