TICKET_CREATE doesn't permit the state change

[denis.marzadro@…]

trac.ini:

...
reopen = closed -> reopened 
reopen.permissions = TICKET_CREATE
reopen.operations = del_resolution
...

We set permissions for every authenticated user, and with above configuration (coming from the official examples) users with only TICKET_CREATE permission are not able to reopen any ticket. The system response is "Warning: No permission to change ticket fields." "Changed by guest status changed from closed to reopened resolution risolto deleted"

The only way to make endusers able to reopen a ticket, is to give them the TICKET_CHGPROP permission, but then they are able to change every properties any time and we don't want to give them this possibility.

[Remy Blank]

Verified on trunk.

[Remy Blank]

Currently, modifying the state of a ticket requires either TICKET_APPEND or TICKET_CHGPROP. I could probably work around this, but I'm not sure it's a good idea. Does it really make sense to allow someone to re-open a ticket, but to prevent her from adding a comment as to why she did?

So I suggest a "worksforme" with the suggestion to add the TICKET_APPEND permission to your users.

[anonymous]

Perhaps i didn't explain very well the problem: i have some external users that i need to enable just to create and reopen ticket. So, I gave them the TICKET_VIEW, TICKET_CREATE, TICKET_APPEND permissions but with only these permissions they are not able to reopen a ticket because trac system notify they are not enable to change the ticket properties (i think state and maybe the resolution description)

[Remy Blank]

Ah, sorry, I should have checked that. Indeed, even with TICKET_APPEND, it's not possible to re-open a closed ticket. And you are correct, changing any properties (including status and resolution) currently require TICKET_MODIFY.

[Remy Blank]

Replying to rblank:

currently require TICKET_MODIFY

I meant TICKET_CHGPROP, of course.

[Remy Blank]

Mmh, it's even worse than I thought. If a user has TICKET_CHGPROP, but is not allowed to change the status according to the workflow, she can bypass the workflow by crafting a special ticket page in the browser (e.g. with FireBug) including a field_status field. I assume the same applies to the resolution field. For example, to close a ticket:

<input name="field_status" value="closed" type="hidden"/>

I wonder if we shouldn't disallow direct changes to the status and resolution fields, and only allow that from the workflow.

Eli, could please you comment on that?

[Eli Carter]

Replying to rblank:

Mmh, it's even worse than I thought. If a user has TICKET_CHGPROP, but is not allowed to change the status according to the workflow, she can bypass the workflow by crafting a special ticket page in the browser (e.g. with FireBug) including a field_status field. I assume the same applies to the resolution field. For example, to close a ticket:

Oh, that's not good. And I think we need to fix that in 0.11-stable as well as trunk.

[Remy Blank]

Fix for both issues (direct change to status and re-opening with TICKET_CREATE and TICKET_APPEND).

[Remy Blank]

The patch above (against trunk) fixes the two issues, plus an additional related one:

• A user having TICKET_APPEND and TICKET_CREATE can now reopen a closed ticket with the default workflow.
• The status and resolution fields cannot be changed directly anymore (and neither can time and changetime).
• The initial status for new tickets is forced to new, something that was previously done through a hidden <input>.

Ok to apply? Eli?

(I can backport the change to 0.11-stable if desired)

[Christian Boos]

Patch looks good, two remarks though:

• as you have workflow_fields = ('resolution', 'status', 'time', 'changetime'), time and changetime are not really workflow fields, so maybe rather use protected_fields terminology? (or programmatic_fields?), and # Fields that can't be modified directly by the user.
• as you talk about 0.11-stable, remember the possible Python 2.3 breakage when replacing dict([]) vs. dict(genexpr) ;-)

[anonymous]

first of all thanks for working on the ticket so quickly, then … maybe i didn't understand well the patch:

it would be useful (ate least for me) to be able to differently address the new tickets from the reopened ones, so i thought to define workflow as

… reopen = closed → reopened

or changing resolution (and then useing it in specific reports)

… reopen.operations = set_resolution reopen.set_resolution = riaperto

does this patch let me do this?

[Remy Blank]

Replying to cboos:

• as you talk about 0.11-stable, remember the possible Python 2.3 breakage when replacing dict([]) vs. dict(genexpr) ;-)

Mmh… I'm sure I would have thought of that ;-)

Replying to anonymous:

does this patch let me do this?

The resolution field should be empty for tickets that are not closed, so I'm not sure you will be able to do what you describe. But at least people with TICKET_CREATE will be able to reopen tickets (as advertised).

[Remy Blank]

Updated with feedback from this ticket and IRC.

[Remy Blank]

The patch above integrates the feedback in comment:9 (except for an import of set for Python 2.3). It also makes the permission checks more explicit, instead of using an opaque cnt variable for counting allowed permissions.

[Eli Carter]

The updated patch looks good to me.

[Remy Blank]

Thanks for the review, Eli! Patch applied in [8982]. I'm working on the 0.11-stable backport (the patch doesn't apply cleanly).

[Remy Blank]

Backport to 0.11-stable.

[Remy Blank]

The patch above is a backport to 0.11-stable. Unfortunately, my Python 2.3 installation seems to be broken at the moment, so I would be grateful if somebody could test the patch on 2.3 (2.6 works fine).

[Remy Blank]

Backport tested on 2.3 and applied in [8993].