Edgewall Software
Modify

Ticket #8884 (closed defect: fixed)

Opened 2 years ago

Last modified 2 years ago

TICKET_CREATE doesn't permit the state change

Reported by: denis.marzadro@… Owned by: rblank
Priority: high Milestone: 0.11.7
Component: ticket system Version: 0.11.4
Severity: critical Keywords:
Cc: massimiliano.bernabe@…, ecarter
Release Notes:
API Changes:

Description (last modified by rblank) (diff)

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.

Attachments

8884-workflow-fixes-r8952.patch (5.5 KB) - added by rblank 2 years ago.
Fix for both issues (direct change to status and re-opening with TICKET_CREATE and TICKET_APPEND).
8884-workflow-fixes-r8981.patch (5.9 KB) - added by rblank 2 years ago.
Updated with feedback from this ticket and IRC.
8884-worflow-fixes-0.11-r8965.patch (6.2 KB) - added by rblank 2 years ago.
Backport to 0.11-stable.

Download all attachments as: .zip

Change History

comment:1 Changed 2 years ago by rblank

  • Description modified (diff)
  • Milestone set to 0.12
  • Owner set to rblank

Verified on trunk.

comment:2 Changed 2 years ago by rblank

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.

comment:3 Changed 2 years ago by 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)

comment:4 follow-up: Changed 2 years ago by rblank

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.

comment:5 in reply to: ↑ 4 Changed 2 years ago by rblank

Replying to rblank:

currently require TICKET_MODIFY

I meant TICKET_CHGPROP, of course.

comment:6 follow-up: Changed 2 years ago by rblank

  • Cc ecarter added

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?

comment:7 in reply to: ↑ 6 Changed 2 years ago by ecarter

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.

Changed 2 years ago by rblank

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

comment:8 Changed 2 years ago by rblank

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)

comment:9 follow-up: Changed 2 years ago by cboos

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

comment:10 Changed 2 years ago by 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?

comment:11 in reply to: ↑ 9 Changed 2 years ago by rblank

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

Changed 2 years ago by rblank

Updated with feedback from this ticket and IRC.

comment:12 Changed 2 years ago by rblank

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.

comment:13 Changed 2 years ago by ecarter

The updated patch looks good to me.

comment:14 Changed 2 years ago by rblank

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

Changed 2 years ago by rblank

Backport to 0.11-stable.

comment:15 Changed 2 years ago by rblank

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

comment:16 Changed 2 years ago by rblank

  • Milestone changed from 0.12 to 0.11.7
  • Resolution set to fixed
  • Status changed from new to closed

Backport tested on 2.3 and applied in [8993].

View

Add a comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
The resolution will be deleted. Next status will be 'reopened'
to The owner will be changed from rblank. Next status will be 'closed'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.