TicketOwnershipPermissions

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

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") );
 }
 
 

... 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:

1. We should check the new owner's existence and OwnTicket right first; if it fails, it renders the rest of the logic moot.
2. /*We should have a GiveTicket right*/ -- this is the big change proposed here
3. 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";
}

...