[rt-devel] SetOwner issues

[Michael van Elst]

Hi,

I am using RT3.0.1 and probably didn't set up all access rights by
the book.

In my installation only the owner of a ticket is allowed to modify it.
If you want to modify a ticket you first have to take it (assuming your
rights are sufficient for that) and you have to give away a ticket
to allow others to modify it.

This causes two issues in the SetOwner function in Ticket_Overlay.pm.

1.

Giving a ticket away first removes the current user from the list
of ticket owners and then adds the new owner to this list.
In a second step the ticket field that denotes the owner (which is
different from the above lists) is changed to reflect the change.

This fails when you are only allowed to modify a ticket as the owner
of the ticket because you have removed your access rights before you
modify the ticket fields. If your database doesn't support rollbacks
you even end with the owner list already changed but the ticket
unchanged and you might have no way to fix that besides poking the SQL
tables.

Reversing the operation makes this safe. First modify the ticket
contents and then give away your rights and pass them to the new
owner. My code looks like:

    my ( $old_owner ) = $self->OwnerGroup->MembersObj->First;
    unless ($old_owner) {
        $RT::Handle->Rollback();
        return(0, $self->loc("Could not find owner. "));
    }

    my ( $add_id, $add_msg ) = $self->OwnerGroup->_AddMember(
                                       PrincipalId => $NewOwnerObj->PrincipalId,
                                       InsideTransaction => 1 );
    unless ($add_id) {
        $RT::Handle->Rollback();
        return(0, $self->loc("Could not add owner. "). $add_msg);
    }

    # We call set twice with slightly different arguments, so
    # as to not have an SQL transaction span two RT transactions

    my ( $val, $msg ) = $self->_Set(
        Field           => 'Owner',
        RecordTransaction => 0,
        Value           => $NewOwnerObj->Id,
        TimeTaken       => 0,
        TransactionType => $Type
    );

    unless ($val) {
        $RT::Handle->Rollback;
        return(0, $self->loc("Could not set owner. "). $msg);
    }

    my ($del_id, $del_msg) = $old_owner->Delete();
    unless ($del_id) {
        $RT::Handle->Rollback();
        return(0, $self->loc("Could not remove owner. "). $del_msg);
    }

    $RT::Handle->Commit();


2.

SetOwner() checks if the current user is allowed to modify a ticket.
If a ticket is owned by 'Nobody' then you cannot take it unless you
already have the 'ModifyTicket' right. Which means that you don't
need to own the ticket in the first place. I'm not sure what Ownership
is good for if it doesn't add privileges. So I changed the test to:

    unless ( $self->CurrentUserHasRight('ModifyTicket')
             or ( $self->CurrentUserHasRight('OwnTicket') and
                  $self->OwnerObj->Id == $RT::Nobody->Id ) ) {
        return ( 0, $self->loc("Permission Denied") );
    }

This way you can restrict modifications to a ticket to the owner
and you still allow to take tickets from 'Nobody'.


Greetings,
-- 
    ,eM""=.            a"-.                         Michael van Elst
   dWWMWM" -          :GM==;                        mlelstv at dev.de.cw.net
  :WWMWMw=--.          "W='  cable & wireless
   9WWMm==-.
    "-Wmw-"  CABLE & WIRELESS