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
Change History
comment:1 Changed 2 years ago by rblank
- Description modified (diff)
- Milestone set to 0.12
- Owner set to rblank
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: ↓ 5 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
comment:6 follow-up: ↓ 7 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
- Attachment 8884-workflow-fixes-r8952.patch added
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: ↓ 11 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
- Attachment 8884-workflow-fixes-r8981.patch added
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
- Attachment 8884-worflow-fixes-0.11-r8965.patch added
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].



Verified on trunk.