Edgewall Software
Modify

Opened 10 years ago

Closed 10 years ago

#8884 closed defect (fixed)

TICKET_CREATE doesn't permit the state change

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

Description (last modified by Remy Blank)

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 (3)

8884-workflow-fixes-r8952.patch (5.5 KB ) - added by Remy Blank 10 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 Remy Blank 10 years ago.
Updated with feedback from this ticket and IRC.
8884-worflow-fixes-0.11-r8965.patch (6.2 KB ) - added by Remy Blank 10 years ago.
Backport to 0.11-stable.

Download all attachments as: .zip

Change History (19)

comment:1 by Remy Blank, 10 years ago

Description: modified (diff)
Milestone: 0.12
Owner: set to Remy Blank

Verified on trunk.

comment:2 by Remy Blank, 10 years ago

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 by anonymous, 10 years ago

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 by Remy Blank, 10 years ago

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.

in reply to:  4 comment:5 by Remy Blank, 10 years ago

Replying to rblank:

currently require TICKET_MODIFY

I meant TICKET_CHGPROP, of course.

comment:6 by Remy Blank, 10 years ago

Cc: Eli Carter 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?

in reply to:  6 comment:7 by Eli Carter, 10 years ago

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.

by Remy Blank, 10 years ago

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

comment:8 by Remy Blank, 10 years ago

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 by Christian Boos, 10 years ago

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 by anonymous, 10 years ago

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?

in reply to:  9 comment:11 by Remy Blank, 10 years ago

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

by Remy Blank, 10 years ago

Updated with feedback from this ticket and IRC.

comment:12 by Remy Blank, 10 years ago

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 by Eli Carter, 10 years ago

The updated patch looks good to me.

comment:14 by Remy Blank, 10 years ago

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

by Remy Blank, 10 years ago

Backport to 0.11-stable.

comment:15 by Remy Blank, 10 years ago

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 by Remy Blank, 10 years ago

Milestone: 0.120.11.7
Resolution: fixed
Status: newclosed

Backport tested on 2.3 and applied in [8993].

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain Remy Blank.
The resolution will be deleted. Next status will be 'reopened'.
to as closed The owner will be changed from Remy Blank to the specified user.

Add Comment


E-mail address and name can be saved in the Preferences .
 
Note: See TracTickets for help on using tickets.